[widgetii]

On a Hi3516CV610 camera with a VS4388 (OpenIPC/majestic#357), ipctool reports no sensor. The ViSemi probe only tries 0x20, but that board's vendor firmware addresses the sensor at 0x6c:

• vs4388_i2c_init() does ioctl(fd, I2C_SLAVE_FORCE, 0x36);
• its ISP sync registers carry dev_addr = 0x6c.

Without a sensor name, majestic has nothing to choose a sensor profile by.

Change

visemi_addrs becomes {0x20, 0x6c}.

This can't misidentify another sensor. The ViSemi probe accepts only an exact 0x4388 at 0x3000 (16-bit). The OmniVision probe, which also tries 0x6c, runs before it and matches only its own IDs.

Tested

The CI test targets pass locally: cYAML_test, reginfo_test, longse_test, sensors_test, hal_hisi_test. Not yet run on that camera.

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

PR Summary by Qodo

hisi: detect VS4388 sensors at I²C address 0x6c

🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Probe 0x6c after 0x20 so Hi3516CV610 cameras can identify their VS4388 sensor.
• Preserve exact sensor-ID matching to avoid identifying unrelated devices at the shared address.

Diagram

graph TD
  A["HiSilicon address table"] --> B["0x20 then 0x6c"] --> C["ViSemi probe"] --> D{"ID is 0x4388?"}
  D -->|Yes| E["VS4388 identified"]
  D -->|No| F["Continue detection"]

Loading

High-Level Assessment

Adding 0x6c to the existing HiSilicon ViSemi address list is the narrowest fit: the probe already requires an exact VS4388 ID, and OmniVision is tried first at the shared address. Board-specific address configuration would add machinery for this isolated case. Hardware validation remains outstanding.

Files changed (1) +3 / -1

Bug fix (1) +3 / -1

hal_hisi.cAdd 0x6c to HiSilicon ViSemi probe addresses +3/-1

Add 0x6c to HiSilicon ViSemi probe addresses

• The ViSemi address list now tries 0x6c after 0x20, allowing detection of a VS4388 on the reported Hi3516CV610 camera. A comment records the vendor-firmware evidence for the additional address.

src/hal/hisi/hal_hisi.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. V2 cameras probe the wrong slave 🐞 Bug ≡ Correctness

Description

The shared visemi_addrs table now supplies 0x6c to V2/V2A, where setup_hal_hisi() installs
i2c_change_plain_addr() and passes that value directly to I2C_SLAVE_FORCE. Those builds
therefore probe slave 0x6c instead of the VS4388 slave 0x36, so a VS4388 on that path is missed
and any unrelated device at 0x6c receives the ViSemi ID reads.

Code

src/hal/hisi/hal_hisi.c[34]

+static unsigned char visemi_addrs[] = {0x20, 0x6c, 0};

Evidence

The changed address is used by every HiSilicon generation. V2/V2A select the plain address callback,
which forwards 0x6c unchanged to the kernel, while the generic HiSilicon reader used by the CV610
path shifts it to 0x36; the ViSemi detector invokes this callback before reading the ID.

src/hal/hisi/hal_hisi.c[32-34]
src/hal/hisi/hal_hisi.c[669-678]
src/hal/common.c[81-85]
src/hal/hisi/hal_hisi.c[293-304]
src/sensors.c[1088-1101]

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 shared `visemi_addrs` table contains the 8-bit encoded address `0x6c`, but V2/V2A configure `i2c_change_plain_addr()`, which passes the value directly to `I2C_SLAVE_FORCE`. This makes those generations probe 7-bit slave `0x6c` rather than the intended VS4388 slave `0x36`.
## Fix Focus Areas
- src/hal/hisi/hal_hisi.c[34-34]
- src/hal/hisi/hal_hisi.c[669-678]
- src/hal/common.c[81-85]
## Recommended Fix
Provide generation-appropriate ViSemi address handling or a V2/V2A-specific address table so the CV610 path continues converting `0x6c` to `0x36`, while V2/V2A pass the correct 7-bit value to `I2C_SLAVE_FORCE`. Add or update tests covering the ViSemi probe on each affected HiSilicon generation.

ⓘ 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 turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗