Skip to content

Commit 32795fe

Browse files
Nikolai-LCopilot
andauthored
Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent f065b0a commit 32795fe

2 files changed

Lines changed: 19 additions & 20 deletions

File tree

docs/hw-reset-followup-plan.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ The `reset_issue_dmesg.txt` log shows 35 consecutive userspace-triggered HW rese
1717

1818
### Steps
1919

20-
1. **Add tracking fields to `struct ds5`** (`kernel/realsense/d4xx.c`):
20+
1. **Add tracking fields to `struct ds5_dev`** (`kernel/realsense/d4xx.c`):
2121
- `int consecutive_reset_failures` — incremented when `ds5_hw_reset_with_recovery()` returns an error, reset to 0 on success
2222
- `unsigned long last_reset_jiffies` — timestamp of the last reset attempt
2323
- `int total_resets` — lifetime counter for diagnostics (never reset)
@@ -57,7 +57,7 @@ The `reset_issue_dmesg.txt` log shows 35 consecutive userspace-triggered HW rese
5757

5858
## Design Decisions
5959

60-
- **Circuit-breaker is per-instance, not per-camera**: Each `struct ds5` has its own counter. Since userspace sends HW reset to one instance (typically Depth at `9-001a`), this naturally tracks per-camera. Peer instances aren't reset independently.
60+
- **Circuit-breaker is per-camera, shared across instances**: The tracking fields live in `struct ds5_dev`, so all `struct ds5` instances for the same physical camera share a single counter and breaker state. If any instance (typically Depth at `9-001a`) trips the breaker, further HW reset requests from any instance for that camera are rejected until the breaker auto‑clears.
6161
- **Return -EBUSY for tripped breaker, -EAGAIN for cooldown**: Distinct error codes let userspace distinguish "permanently failed" from "try again later".
6262
- **Auto-clear after 60s inactivity**: Avoids permanent lockout. If the camera recovers externally (e.g., power cycle), the driver will accept resets again.
6363
- **Phase 1 dual-camera is diagnostic only**: No blocking behavior added — just logging. If field data shows disruption, we can add a sibling-streaming gate in a future commit.

kernel/realsense/d4xx.c

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2431,21 +2431,21 @@ static int ds5_hw_reset_serdes_recovery(struct ds5 *state, bool force_phase2)
24312431
* later. Do multiple reads with delays to catch oscillation.
24322432
*/
24332433
ret = ds5_read(state, DS5_FW_VERSION, &tmp);
2434-
if (ret == 0) {
2435-
int k;
2436-
bool stable = true;
2437-
2438-
for (k = 0; k < DS5_HW_RESET_STABILITY_READS; k++) {
2439-
msleep(DS5_HW_RESET_STABILITY_INTERVAL_MS);
2440-
ret = ds5_read(state, DS5_FW_VERSION, &tmp);
2441-
if (ret < 0) {
2442-
dev_warn(&state->client->dev,
2443-
"%s(): Phase 1 stability check %d/%d failed (err %d)\n",
2444-
__func__, k + 1, DS5_HW_RESET_STABILITY_READS, ret);
2445-
stable = false;
2446-
break;
2447-
}
2448-
}
2434+
if (ret == 0) {
2435+
int k;
2436+
bool stable = true;
2437+
2438+
for (k = 0; k < DS5_HW_RESET_STABILITY_READS; k++) {
2439+
msleep(DS5_HW_RESET_STABILITY_INTERVAL_MS);
2440+
ret = ds5_read(state, DS5_FW_VERSION, &tmp);
2441+
if (ret < 0) {
2442+
dev_warn(&state->client->dev,
2443+
"%s(): Phase 1 stability check %d/%d failed (err %d)\n",
2444+
__func__, k + 1, DS5_HW_RESET_STABILITY_READS, ret);
2445+
stable = false;
2446+
break;
2447+
}
2448+
}
24492449
if (stable) {
24502450
dev_info(&state->client->dev,
24512451
"%s(): Phase 1 succeeded, I2C link stable (%d consecutive reads OK)\n",
@@ -4052,8 +4052,8 @@ static int ds5_setup_and_link(struct ds5 *state)
40524052
int err = 0;
40534053
struct device *dev = &state->client->dev;
40544054

4055-
mutex_lock(&serdes_lock__);
40564055
ds5_init_global_slots_once();
4056+
mutex_lock(&serdes_lock__);
40574057

40584058
state->ds5_dev = NULL;
40594059
/* Look for existing DS5 instances */
@@ -4064,8 +4064,7 @@ static int ds5_setup_and_link(struct ds5 *state)
40644064
match = ds5_inited[i].ds5_primary &&
40654065
ds5_inited[i].ds5_primary->ser_dev == state->ser_dev;
40664066
mutex_unlock(&ds5_inited[i].lock);
4067-
if (match)
4068-
{ /* Same camera, different stream instance. */
4067+
if (match) { /* Same camera, different stream instance. */
40694068
state->serdes_primary = false;
40704069
state->ds5_dev = &ds5_inited[i];
40714070
break;

0 commit comments

Comments
 (0)