Refactor ds5 sensor configuration and streaming logic to improve stab… - #375
Conversation
4f7d47d to
daeed27
Compare
There was a problem hiding this comment.
Pull request overview
This pull request refactors the DS5 sensor configuration and streaming logic for the D4XX RealSense camera driver to improve stability and error handling, particularly for SERDES (Serializer/Deserializer) pipeline management over GMSL links.
Changes:
- Replaced boolean
pipe_configuredflag withPIPE_NOT_CONFIGUREDconstant for clearer pipe state tracking - Increased stream start/stop timeout from 700ms to 2000ms for better reliability under adverse conditions
- Refactored
ds5_mux_s_stream()with unified retry logic that properly handles firmware config rejections and streaming state transitions - Consolidated pipe cleanup in
ds5_hw_reset_with_recovery()by introducingds5_config_cache_clear()helper function
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| long-run.log | Kernel diagnostic log showing errors being addressed (refcount issues, timeouts, capture failures) |
| kernel/realsense/d4xx.c | Core driver changes: improved pipe state management, increased timeouts, refactored streaming logic with better error recovery |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| if (i >= DS5_START_MAX_COUNT) { | ||
| if (on == !(status & DS5_STATUS_STREAMING)) |
There was a problem hiding this comment.
The variable 'status' is used in the conditional check after the loop, but if the loop times out without a successful read, 'status' may be uninitialized. Initialize 'status' to a safe value (e.g., 0) before the loop to prevent undefined behavior.
| ret = -(ENOSR); | ||
| return ret; |
There was a problem hiding this comment.
Variable 'ret' is assigned but its value is not used before being overwritten or returned. The assignment at line 1786 should be simplified to just 'return -ENOSR;' or the intermediate variable should be removed.
| ret = -(ENOSR); | |
| return ret; | |
| return -ENOSR; |
| if (streaming == expected_streaming_state) | ||
| ts = jiffies; | ||
| for (timeout = ts + msecs_to_jiffies(DS5_START_MAX_TIME); | ||
| time_before(jiffies, timeout); msleep_range(i*DS5_START_POLL_TIME)) |
There was a problem hiding this comment.
The loop iterator variable 'i' is used in the sleep calculation but is never incremented in the first polling loop. This results in a busy-wait spin (zero sleep) until timeout. The loop should increment 'i' in the loop update expression or use a separate counter.
| time_before(jiffies, timeout); msleep_range(i*DS5_START_POLL_TIME)) | |
| time_before(jiffies, timeout); i++, msleep_range(i * DS5_START_POLL_TIME)) |
…ility and error handling
daeed27 to
39b6b8e
Compare
…ility and error handling