[FIX] Improve D4XX streaming resilience and I2C failure handling by ymodlin · Pull Request #368 · realsenseai/realsense_mipi_platform_driver

ymodlin · 2026-02-19T18:59:50Z

Address failures observed during 13+ hour streaming stress tests where repeated start/stop cycles eventually cause catastrophic I2C bus failure (errno -121) and unrecoverable camera state requiring power cycle.

Key changes:

Add I2C circuit breaker: track consecutive errors via atomic counter and declare camera dead after 50 failures, short-circuiting all further I2C operations with -ENODEV to prevent bus thrashing
Rate-limit I2C error messages: demote per-retry warnings to dev_dbg, use dev_warn_ratelimited for final failures to prevent kernel log flooding (3,430+ identical messages observed in stress test logs)
Fix DFU release: skip I2C verification when camera is dead, reduce retries from 10 to 3, always return 0 to prevent file descriptor leaks
Verify GMSL link after HW reset recovery: replace blind 300ms sleep with active I2C polling (up to 1.2s) to confirm link is alive before declaring reset complete
Reset all sensor streaming state in HW reset recovery path to ensure ds5_configure() performs fresh setup on next stream-on instead of using stale cached state

Address failures observed during 13+ hour streaming stress tests where repeated start/stop cycles eventually cause catastrophic I2C bus failure (errno -121) and unrecoverable camera state requiring power cycle. Key changes: - Add I2C circuit breaker: track consecutive errors via atomic counter and declare camera dead after 50 failures, short-circuiting all further I2C operations with -ENODEV to prevent bus thrashing - Rate-limit I2C error messages: demote per-retry warnings to dev_dbg, use dev_warn_ratelimited for final failures to prevent kernel log flooding (3,430+ identical messages observed in stress test logs) - Fix DFU release: skip I2C verification when camera is dead, reduce retries from 10 to 3, always return 0 to prevent file descriptor leaks - Verify GMSL link after HW reset recovery: replace blind 300ms sleep with active I2C polling (up to 1.2s) to confirm link is alive before declaring reset complete - Reset all sensor streaming state in HW reset recovery path to ensure ds5_configure() performs fresh setup on next stream-on instead of using stale cached state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR enhances the D4XX camera driver's resilience during extended streaming operations by implementing an I2C circuit breaker pattern, improving error message hygiene, and adding active GMSL link verification after hardware resets. The changes address catastrophic I2C bus failures observed in 13+ hour stress tests where repeated errors eventually cause unrecoverable camera states.

Changes:

Implements I2C circuit breaker with atomic error counter that declares camera dead after 50 consecutive failures
Reduces kernel log noise by demoting retry warnings to debug level and rate-limiting final error messages
Improves DFU release reliability by skipping I2C checks when camera is dead and always returning success to prevent FD leaks
Replaces blind 300ms sleep after SERDES reset with active I2C polling (up to 1.2s) to verify GMSL link recovery
Resets all sensor streaming state during hardware reset recovery to ensure clean re-initialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T19:04:31Z

+		if (atomic_inc_return(&state->i2c_consec_errors) >=
+		    DS5_I2C_DEAD_THRESHOLD) {
+			state->camera_dead = true;
+			dev_err(&state->client->dev,
+				"camera unreachable after %d consecutive I2C errors\n",
+				DS5_I2C_DEAD_THRESHOLD);
+		}


Race condition: the camera_dead flag is set without proper synchronization. While i2c_consec_errors is atomic, the camera_dead boolean is not protected by any lock. Multiple threads could simultaneously trigger the threshold and all call atomic_inc_return, leading to multiple threads setting camera_dead to true and logging the error message. More critically, one thread could read camera_dead as false at line 552 while another thread is setting it to true at line 580, potentially causing inconsistent behavior. Consider using atomic_cmpxchg or protecting both the counter check and the flag assignment with the state mutex.

Copilot · 2026-02-19T19:04:31Z

 		state->dser_ops->reset_oneshot(state->dser_dev);
-		msleep(300);
+
+		/*
+		 * Verify GMSL link is re-established by polling I2C.
+		 * reset_oneshot itself sleeps 100ms internally; we add
+		 * settling time and verify the link is actually alive
+		 * rather than blindly sleeping a fixed 300ms.
+		 */
+		msleep(200);
+		for (i = 0; i < 10; i++) {
+			ret = ds5_read(state, DS5_FW_VERSION, &status);
+			if (ret == 0)
+				break;
+			dev_dbg(&state->client->dev,
+				"%s(): GMSL link not ready, retry %d\n",
+				__func__, i);
+			msleep(100);
+		}
+		if (ret < 0) {
+			dev_err(&state->client->dev,
+				"%s(): GMSL link failed to recover after reset_oneshot (%d)\n",
+				__func__, ret);
+			return -EIO;
+		}
+		dev_info(&state->client->dev,
+			"%s(): GMSL link verified after %d ms\n",
+			__func__, 200 + i * 100);


The circuit breaker has no recovery mechanism. Once camera_dead is set to true, it remains permanently true for the lifetime of the driver instance. Even if hardware reset recovery (ds5_hw_reset_with_recovery) succeeds and verifies the GMSL link is working (lines 2464-2481), the camera_dead flag is never cleared. This means the camera will remain unusable until the driver is unloaded and reloaded. Consider resetting both camera_dead and i2c_consec_errors after successful hardware reset recovery to allow the driver to recover from transient failures.

Copilot · 2026-02-19T19:04:31Z

+		 */
+		msleep(200);
+		for (i = 0; i < 10; i++) {
+			ret = ds5_read(state, DS5_FW_VERSION, &status);


The GMSL link verification uses status variable to read firmware version, but status is declared as u16 at line 2309 for use with hardware reset status checks. Reusing it here for firmware version reading is semantically confusing. While technically correct since both are u16 values, consider using a separate variable like fw_version for the GMSL link verification to improve code clarity and prevent future maintenance issues.

Copilot · 2026-02-19T19:04:32Z

+			return -EIO;
+		}
+		dev_info(&state->client->dev,
+			"%s(): GMSL link verified after %d ms\n",


The GMSL link verification loop increments i from 0 to 9, but the final success message reports timing as 200 + i * 100 milliseconds. If the link succeeds on the first attempt (i=0, ret=0 at line 2466), the message claims "GMSL link verified after 200 ms" when actually 200ms was the initial sleep before any polling attempts. This is misleading. Consider either adjusting the message to account for this (e.g., using 200 + (i > 0 ? i * 100 : 0)) or restructuring the timing calculation to be more accurate.

Suggested change

"%s(): GMSL link verified after %d ms\n",

"%s(): GMSL link verified after %d ms (including initial 200 ms settling delay)\n",

Copilot · 2026-02-19T19:04:32Z

+		for (i = 0; i < 10; i++) {
+			ret = ds5_read(state, DS5_FW_VERSION, &status);
+			if (ret == 0)
+				break;
+			dev_dbg(&state->client->dev,
+				"%s(): GMSL link not ready, retry %d\n",
+				__func__, i);
+			msleep(100);
+		}


The GMSL link verification polling (lines 2464-2472) will increment the consecutive error counter if I2C reads fail, potentially triggering the circuit breaker and setting camera_dead during legitimate recovery. Since ds5_read returns -ENODEV when camera_dead is true (line 644-645), once the threshold is hit during this polling loop, all subsequent reads will fail with -ENODEV, causing the recovery to fail at line 2473-2477 even if the hardware is actually recovering. This creates a catch-22 where recovery attempts can poison themselves. Consider temporarily suppressing the circuit breaker logic during hardware reset recovery, or resetting the error counter before attempting GMSL link verification.

Copilot · 2026-02-19T19:04:32Z

+		if (atomic_inc_return(&state->i2c_consec_errors) >=
+		    DS5_I2C_DEAD_THRESHOLD) {
+			state->camera_dead = true;
+			dev_err(&state->client->dev,


The circuit breaker threshold message is duplicated across all four I2C functions (ds5_write, ds5_raw_write, ds5_read, ds5_raw_read) with identical wording. If multiple I2C functions hit the threshold simultaneously or sequentially, the same "camera unreachable after 50 consecutive I2C errors" message will be logged multiple times. Since the goal is to reduce log noise (as mentioned in the PR description about 3,430+ identical messages), consider using a flag to ensure this critical message is only logged once, or at least using dev_err_ratelimited instead of dev_err.

Suggested change

dev_err(&state->client->dev,

dev_err_ratelimited(&state->client->dev,

Copilot · 2026-02-19T19:04:32Z

-		return ret;
+		return 0; /* release must succeed to avoid FD leak */
 	}
 	ret = ds5_read(state, DS5_FW_BUILD, &state->fw_build);


The function now unconditionally returns 0 even after the final ds5_read for FW_BUILD (line 5694) regardless of whether it succeeds or fails. While this prevents FD leaks (which is good), silently ignoring errors from reading FW_BUILD means the firmware version information could be incomplete or stale. If consistency is important, either document why FW_BUILD read failures are acceptable, or at least log a debug message when line 5694 fails so there's a record in the logs.

Suggested change

ret = ds5_read(state, DS5_FW_BUILD, &state->fw_build);

ret = ds5_read(state, DS5_FW_BUILD, &state->fw_build);

if (ret)

dev_dbg(&state->client->dev,

"%s(): failed to read FW_BUILD, firmware build info may be stale (err=%d)\n",

__func__, ret);

Copilot · 2026-02-19T19:04:33Z

 	do {
 		ret = ds5_read(state, DS5_FW_VERSION, &state->fw_version);
 		if (ret)
 			msleep_range(10);
-	} while (retry-- && ret != 0 );
+	} while (retry-- && ret != 0);


The retry loop at lines 5683-5687 will fail prematurely if the circuit breaker trips during retries. If the first 2 reads fail (decrementing retry to 1), and those failures cause i2c_consec_errors to hit the threshold, camera_dead will be set to true. The third retry will then return -ENODEV immediately at line 644 due to the camera_dead check, without actually attempting I2C communication. This means the actual retry count becomes unpredictable once the threshold is approached. Consider resetting or pausing the circuit breaker during DFU release retries, or document that the circuit breaker can cause early termination of retries.

Copilot AI review requested due to automatic review settings February 19, 2026 18:59

Copilot started reviewing on behalf of ymodlin February 19, 2026 19:00 View session

Bump version to 1.0.2.11

47995ff

Copilot AI reviewed Feb 19, 2026

View reviewed changes

ymodlin and others added 2 commits February 19, 2026 22:25

[FIX] Reset sensor pipe configuration on release

b17850a

Bump version to 1.0.2.12

b991538

ymodlin closed this Feb 24, 2026

ymodlin deleted the fix-reboot branch May 27, 2026 10:38

Navigation Menu

Uh oh!

[FIX] Improve D4XX streaming resilience and I2C failure handling - #368

[FIX] Improve D4XX streaming resilience and I2C failure handling#368
ymodlin wants to merge 4 commits into
devfrom
fix-reboot

ymodlin commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	"%s(): GMSL link verified after %d ms\n",
	"%s(): GMSL link verified after %d ms (including initial 200 ms settling delay)\n",

	dev_err(&state->client->dev,
	dev_err_ratelimited(&state->client->dev,

Uh oh!

Conversation

ymodlin commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants