[dimerr]

Scanning a CV610 board with a GC4653 killed the sensor bus at once. On stock firmware the streamer then tripped the watchdog; on OpenIPC every later transfer on i2c-0 failed until the sensor was reset.

i2cdetect read register 0 with a one-byte address, and the GC4653 takes two. Single transfers on the board isolated it: i2cget 0x52 0x00 kills the bus and a pulse of the sensor reset in 0x11018440 recovers it, while a NACK at an empty address, 0x78 and the general call at 0x00 do nothing.

On HISI_OT the scan now writes one byte and a stop. A read alone also found the sensor, but it took about 500 ms per table line: without an interrupt, bsp-i2c waits for data on every address that does not answer, where a write fails on the NACK. A zero-length write is out too, as the driver loads len - 1 into its loop counter.

Tested on a Hi3516CV610 QFN board with a GC4653 on OpenIPC: the scan is immediate, finds the sensor, and the sensor still answers afterwards. Other CV610 sensors and a scan during a live stream are untested. Other chips keep the old probe.

Sensor detection still wedges this sensor: detect_galaxycore_sensor reads 0xf0 with a one-byte address first, since #238. That needs its own fix.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Prevent i2cdetect from wedging GC4653 on Hi3516CV610

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Probe HISI_OT addresses with a one-byte write and stop to avoid wedging GC4653 sensors.
• Preserve the existing register-read probe on other chips.
• Sensor detection remains affected; other CV610 sensors and live-stream scans remain untested.

Diagram

graph TD
  Scan["i2cdetect scan"] --> Address["Set I2C address"] --> Platform{"HISI_OT?"} -->|Yes| Write["One-byte write"] --> Bus["Sensor bus"]
  Platform -->|No| Read["Register read"] --> Bus

Loading

High-Level Assessment

The chip-specific branch is appropriate: it avoids the GC4653-wedging register read without changing probes on other chips. The reported read-only probe is too slow on unanswered addresses, and a zero-length write is incompatible with the driver. Validate other CV610 sensors separately; this does not fix the sensor-detection path.

Files changed (1) +23 / -1

Bug fix (1) +23 / -1

i2cspi.cUse a write-and-stop probe for HISI_OT scans +23/-1

Use a write-and-stop probe for HISI_OT scans

• Adds an i2cdetect probe helper that writes one byte on HISI_OT and retains the register-read probe elsewhere. The scan calls this helper after selecting each address, avoiding the transfer reported to wedge GC4653 on Hi3516CV610.

src/i2cspi.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Failed address changes mislabel devices 🐞 Bug ≡ Correctness

Description

i2cdetect() ignores failures from i2c_change_addr(), while the new HISI_OT write() probe uses
the slave address last selected on the file descriptor. If an address-selection ioctl fails but a
subsequent write succeeds, the scan reports the previous device at the current table address and
sends the probe byte to that previous device again.

Code

src/i2cspi.c[205]

+        return write(fd, &byte, 1) == 1 ? 0 : -1;

Evidence

The HISI_OT scan retains the default address selector, which returns -1 when I2C_SLAVE_FORCE
fails. The loop discards that result, whereas the new write probe has no address argument; the
former HISI read probe supplied the intended address in each I2C message.

src/i2cspi.c[225-228]
src/hal/common.c[72-78]
src/hal/hisi/hal_hisi.c[649-681]
src/hal/hisi/hal_hisi.c[319-332]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The HISI_OT write probe depends on the selected file-descriptor address, but the scan ignores address-selection failures and can report or write to the wrong device.
## Fix Focus Areas
- src/i2cspi.c[225-228]
## Recommended Fix
Check the return value of `i2c_change_addr()` in the scan loop. Treat a failed selection as a failed probe without calling `i2cdetect_probe()`; preserve the existing table formatting.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗