Skip to content

Commit 9e73bd5

Browse files
ymodlinclaude
ymodlin
and
claude
committed
[FIX] Improve D4XX streaming resilience and I2C failure handling
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>
1 parent 95c2627 commit 9e73bd5

1 file changed

Lines changed: 126 additions & 34 deletions

File tree

‎kernel/realsense/d4xx.c‎

Lines changed: 126 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ enum ds5_mux_pad {
194194
/* I2C retry configuration */
195195
#define DS5_I2C_RETRY_COUNT 5
196196
#define DS5_I2C_RETRY_DELAY_US 5000
197+
#define DS5_I2C_DEAD_THRESHOLD 50 /* consecutive errors before declaring dead */
197198

198199
/* DFU definition section */
199200
#define DFU_MAGIC_NUMBER "/0x01/0x02/0x03/0x04"
@@ -481,6 +482,9 @@ struct ds5 {
481482
struct i2c_client *dser_i2c;
482483
const struct dser_interface *dser_ops;
483484
#endif
485+
/* I2C health tracking */
486+
atomic_t i2c_consec_errors;
487+
bool camera_dead;
484488
};
485489

486490
struct ds5_counters {
@@ -545,6 +549,9 @@ static int ds5_write(struct ds5 *state, u16 reg, u16 val)
545549
int retry;
546550
u8 value[2];
547551

552+
if (state->camera_dead)
553+
return -ENODEV;
554+
548555
value[1] = val >> 8;
549556
value[0] = val & 0x00FF;
550557

@@ -557,20 +564,31 @@ static int ds5_write(struct ds5 *state, u16 reg, u16 val)
557564
if (ret == 0)
558565
break;
559566
if (retry < DS5_I2C_RETRY_COUNT - 1) {
560-
dev_warn(&state->client->dev,
567+
dev_dbg(&state->client->dev,
561568
"%s(): i2c write retry %d, 0x%04x = 0x%x, err %d\n",
562569
__func__, retry + 1, reg, val, ret);
563570
usleep_range(DS5_I2C_RETRY_DELAY_US,
564571
DS5_I2C_RETRY_DELAY_US + 500);
565572
}
566573
}
567-
if (ret < 0)
568-
dev_err(&state->client->dev,
574+
if (ret < 0) {
575+
dev_warn_ratelimited(&state->client->dev,
569576
"%s(): i2c write failed after %d retries, 0x%04x = 0x%x, err %d\n",
570577
__func__, DS5_I2C_RETRY_COUNT, reg, val, ret);
571-
else if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
572-
dev_dbg(&state->client->dev, "%s(): i2c write 0x%04x: 0x%x\n",
573-
__func__, reg, val);
578+
if (atomic_inc_return(&state->i2c_consec_errors) >=
579+
DS5_I2C_DEAD_THRESHOLD) {
580+
state->camera_dead = true;
581+
dev_err(&state->client->dev,
582+
"camera unreachable after %d consecutive I2C errors\n",
583+
DS5_I2C_DEAD_THRESHOLD);
584+
}
585+
} else {
586+
atomic_set(&state->i2c_consec_errors, 0);
587+
if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
588+
dev_dbg(&state->client->dev,
589+
"%s(): i2c write 0x%04x: 0x%x\n",
590+
__func__, reg, val);
591+
}
574592

575593
return ret;
576594
}
@@ -581,26 +599,39 @@ static int ds5_raw_write(struct ds5 *state, u16 reg,
581599
int ret;
582600
int retry;
583601

602+
if (state->camera_dead)
603+
return -ENODEV;
604+
584605
for (retry = 0; retry < DS5_I2C_RETRY_COUNT; retry++) {
585606
ret = regmap_raw_write(state->regmap, reg, val, val_len);
586607
if (ret == 0)
587608
break;
588609
if (retry < DS5_I2C_RETRY_COUNT - 1) {
589-
dev_warn(&state->client->dev,
610+
dev_dbg(&state->client->dev,
590611
"%s(): i2c raw write retry %d, 0x%04x size(%d), err %d\n",
591612
__func__, retry + 1, reg, (int)val_len, ret);
592613
usleep_range(DS5_I2C_RETRY_DELAY_US,
593614
DS5_I2C_RETRY_DELAY_US + 500);
594615
}
595616
}
596-
if (ret < 0)
597-
dev_err(&state->client->dev,
617+
if (ret < 0) {
618+
dev_warn_ratelimited(&state->client->dev,
598619
"%s(): i2c raw write failed after %d retries, 0x%04x size(%d), err %d\n",
599620
__func__, DS5_I2C_RETRY_COUNT, reg, (int)val_len, ret);
600-
else if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
601-
dev_dbg(&state->client->dev,
602-
"%s(): i2c raw write 0x%04x: %d bytes\n",
603-
__func__, reg, (int)val_len);
621+
if (atomic_inc_return(&state->i2c_consec_errors) >=
622+
DS5_I2C_DEAD_THRESHOLD) {
623+
state->camera_dead = true;
624+
dev_err(&state->client->dev,
625+
"camera unreachable after %d consecutive I2C errors\n",
626+
DS5_I2C_DEAD_THRESHOLD);
627+
}
628+
} else {
629+
atomic_set(&state->i2c_consec_errors, 0);
630+
if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
631+
dev_dbg(&state->client->dev,
632+
"%s(): i2c raw write 0x%04x: %d bytes\n",
633+
__func__, reg, (int)val_len);
634+
}
604635

605636
return ret;
606637
}
@@ -610,25 +641,39 @@ static int ds5_read(struct ds5 *state, u16 reg, u16 *val)
610641
int ret;
611642
int retry;
612643

644+
if (state->camera_dead)
645+
return -ENODEV;
646+
613647
for (retry = 0; retry < DS5_I2C_RETRY_COUNT; retry++) {
614648
ret = regmap_raw_read(state->regmap, reg, val, 2);
615649
if (ret == 0)
616650
break;
617651
if (retry < DS5_I2C_RETRY_COUNT - 1) {
618-
dev_warn(&state->client->dev,
652+
dev_dbg(&state->client->dev,
619653
"%s(): i2c read retry %d, 0x%04x, err %d\n",
620654
__func__, retry + 1, reg, ret);
621655
usleep_range(DS5_I2C_RETRY_DELAY_US,
622656
DS5_I2C_RETRY_DELAY_US + 500);
623657
}
624658
}
625-
if (ret < 0)
626-
dev_err(&state->client->dev,
659+
if (ret < 0) {
660+
dev_warn_ratelimited(&state->client->dev,
627661
"%s(): i2c read failed after %d retries, 0x%04x, err %d\n",
628662
__func__, DS5_I2C_RETRY_COUNT, reg, ret);
629-
else if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
630-
dev_dbg(&state->client->dev, "%s(): i2c read 0x%04x: 0x%x\n",
631-
__func__, reg, *val);
663+
if (atomic_inc_return(&state->i2c_consec_errors) >=
664+
DS5_I2C_DEAD_THRESHOLD) {
665+
state->camera_dead = true;
666+
dev_err(&state->client->dev,
667+
"camera unreachable after %d consecutive I2C errors\n",
668+
DS5_I2C_DEAD_THRESHOLD);
669+
}
670+
} else {
671+
atomic_set(&state->i2c_consec_errors, 0);
672+
if (state->dfu_dev.dfu_state_flag == DS5_DFU_IDLE)
673+
dev_dbg(&state->client->dev,
674+
"%s(): i2c read 0x%04x: 0x%x\n",
675+
__func__, reg, *val);
676+
}
632677

633678
return ret;
634679
}
@@ -638,22 +683,35 @@ static int ds5_raw_read(struct ds5 *state, u16 reg, void *val, size_t val_len)
638683
int ret;
639684
int retry;
640685

686+
if (state->camera_dead)
687+
return -ENODEV;
688+
641689
for (retry = 0; retry < DS5_I2C_RETRY_COUNT; retry++) {
642690
ret = regmap_raw_read(state->regmap, reg, val, val_len);
643691
if (ret == 0)
644692
break;
645693
if (retry < DS5_I2C_RETRY_COUNT - 1) {
646-
dev_warn(&state->client->dev,
694+
dev_dbg(&state->client->dev,
647695
"%s(): i2c raw read retry %d, 0x%04x size(%d), err %d\n",
648696
__func__, retry + 1, reg, (int)val_len, ret);
649697
usleep_range(DS5_I2C_RETRY_DELAY_US,
650698
DS5_I2C_RETRY_DELAY_US + 500);
651699
}
652700
}
653-
if (ret < 0)
654-
dev_err(&state->client->dev,
701+
if (ret < 0) {
702+
dev_warn_ratelimited(&state->client->dev,
655703
"%s(): i2c raw read failed after %d retries, 0x%04x size(%d), err %d\n",
656704
__func__, DS5_I2C_RETRY_COUNT, reg, (int)val_len, ret);
705+
if (atomic_inc_return(&state->i2c_consec_errors) >=
706+
DS5_I2C_DEAD_THRESHOLD) {
707+
state->camera_dead = true;
708+
dev_err(&state->client->dev,
709+
"camera unreachable after %d consecutive I2C errors\n",
710+
DS5_I2C_DEAD_THRESHOLD);
711+
}
712+
} else {
713+
atomic_set(&state->i2c_consec_errors, 0);
714+
}
657715

658716
return ret;
659717
}
@@ -2384,12 +2442,43 @@ static int ds5_hw_reset_with_recovery(struct ds5 *state)
23842442
sensor->pipe_configured = false;
23852443
sensor->pipe_id = -1;
23862444
}
2445+
2446+
/* Ensure all sensor streaming state is fully reset so
2447+
* ds5_configure() does a fresh setup on next stream-on */
2448+
for (i = 0; i < ARRAY_SIZE(sensors); i++)
2449+
sensors[i]->streaming = false;
2450+
23872451
mutex_unlock(&serdes_lock__);
23882452

23892453
dev_info(&state->client->dev,
23902454
"%s(): Re-initializing SERDES link\n", __func__);
23912455
state->dser_ops->reset_oneshot(state->dser_dev);
2392-
msleep(300);
2456+
2457+
/*
2458+
* Verify GMSL link is re-established by polling I2C.
2459+
* reset_oneshot itself sleeps 100ms internally; we add
2460+
* settling time and verify the link is actually alive
2461+
* rather than blindly sleeping a fixed 300ms.
2462+
*/
2463+
msleep(200);
2464+
for (i = 0; i < 10; i++) {
2465+
ret = ds5_read(state, DS5_FW_VERSION, &status);
2466+
if (ret == 0)
2467+
break;
2468+
dev_dbg(&state->client->dev,
2469+
"%s(): GMSL link not ready, retry %d\n",
2470+
__func__, i);
2471+
msleep(100);
2472+
}
2473+
if (ret < 0) {
2474+
dev_err(&state->client->dev,
2475+
"%s(): GMSL link failed to recover after reset_oneshot (%d)\n",
2476+
__func__, ret);
2477+
return -EIO;
2478+
}
2479+
dev_info(&state->client->dev,
2480+
"%s(): GMSL link verified after %d ms\n",
2481+
__func__, 200 + i * 100);
23932482
}
23942483
#endif
23952484

@@ -5560,20 +5649,21 @@ static int ds5_dfu_device_release(struct inode *inode, struct file *file)
55605649
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(
55615650
state->client->adapter);
55625651
#endif
5563-
int ret = 0, retry = 10;
5652+
int ret = 0, retry = 3;
55645653
mutex_lock(&state->lock);
55655654
state->dfu_dev.device_open_count--;
55665655
if (state->dfu_dev.dfu_state_flag != DS5_DFU_RECOVERY)
55675656
state->dfu_dev.dfu_state_flag = DS5_DFU_IDLE;
5568-
/* We disable this section as it has no effect when device in operational
5569-
mode and has not enough effect when device in recovery mode */
5570-
// if (state->dfu_dev.dfu_state_flag == DS5_DFU_DONE
5571-
// && state->dfu_dev.init_v4l_f)
5572-
// ds5_v4l_init(state->client, state);
5573-
// state->dfu_dev.init_v4l_f = 0;
55745657
if (state->dfu_dev.dfu_msg)
55755658
devm_kfree(&state->client->dev, state->dfu_dev.dfu_msg);
55765659
state->dfu_dev.dfu_msg = NULL;
5660+
5661+
/* Skip I2C verification if camera is unreachable */
5662+
if (state->camera_dead) {
5663+
mutex_unlock(&state->lock);
5664+
return 0;
5665+
}
5666+
55775667
#ifdef CONFIG_TEGRA_CAMERA_PLATFORM
55785668
/* get i2c controller and restore bus clock rate */
55795669
while (parent && i2c_parent_is_i2c_adapter(parent))
@@ -5594,16 +5684,16 @@ static int ds5_dfu_device_release(struct inode *inode, struct file *file)
55945684
ret = ds5_read(state, DS5_FW_VERSION, &state->fw_version);
55955685
if (ret)
55965686
msleep_range(10);
5597-
} while (retry-- && ret != 0 );
5687+
} while (retry-- && ret != 0);
55985688
if (ret) {
5599-
dev_warn(&state->client->dev,
5689+
dev_warn_ratelimited(&state->client->dev,
56005690
"%s(): no communication with d4xx\n", __func__);
56015691
mutex_unlock(&state->lock);
5602-
return ret;
5692+
return 0; /* release must succeed to avoid FD leak */
56035693
}
56045694
ret = ds5_read(state, DS5_FW_BUILD, &state->fw_build);
56055695
mutex_unlock(&state->lock);
5606-
return ret;
5696+
return 0;
56075697
};
56085698

56095699
static const struct file_operations ds5_device_file_ops = {
@@ -5829,6 +5919,8 @@ static int ds5_probe(struct i2c_client *c, const struct i2c_device_id *id)
58295919
return -ENOMEM;
58305920

58315921
mutex_init(&state->lock);
5922+
atomic_set(&state->i2c_consec_errors, 0);
5923+
state->camera_dead = false;
58325924

58335925
state->client = c;
58345926
dev_warn(&c->dev, "Probing driver for D4xx\n");

0 commit comments

Comments
 (0)