[widgetii]

OpenIPC's S3L firmware ships drivers for ar0230, ar0237, imx172, imx178,
imx226, imx274, imx290, imx326, ov2718 and ov4689, and load_ambarella asks
ipcinfo -s which one to load. All of them were already told apart but the
AR0230 (CHIP_VERSION_REG 0x3000 = 0x0056) and the two Sony sensors below.

Both byte orders are taken: the AR0237's 0x0950 arrives as 0x5009 through
the reader the S3L HAL uses, and the AR0230's ID would arrive the same way.

Not done: IMX172 and IMX326. Neither has an ID register, the vendor drivers
check nothing at probe, and without a board or a datasheet to hand there is
no reset default to tell them from the IMX226 that the // IMX326 too?
branch already reports; a guessed one would risk misnaming sensors that are
detected correctly today. A board with either can set sensor= in U-Boot.

Tested: sensors_test passes against the golden list (two AR0230 rows added).
On a Hikvision DS-2CD2423G0-I (S3Lm + AR0237) a static armhf ipcinfo still
reports chip s3l, family s3l and sensor ar0237. No AR0230 here, so its rows
are unproven on hardware.

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

PR Summary by Qodo

Detect AR0230 sensors on Ambarella S3L

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Identify AR0230 from either byte order of its OnSemi chip-version register.
• Add both IDs to the golden list to guard against detection regressions.

Diagram

graph TD
  Bus["I2C register 0x3000"] --> Probe["OnSemi probe"] --> Match{"AR0230 ID?"} -->|yes| Name["AR0230 result"] --> CLI["ipcinfo -s"]
  Match -->|no| Existing["Existing ID handling"]
  Golden["Golden ID rows"] -.-> Probe

Loading

High-Level Assessment

Explicitly matching both values in the existing OnSemi probe is the lowest-risk approach. Changing byte-order handling across the reader would affect other sensors without being necessary for this identification.

Files changed (2) +10 / -0

Bug fix (1) +8 / -0

sensors.cRecognize both AR0230 chip-version byte orders +8/-0

Recognize both AR0230 chip-version byte orders

• Maps register values 0x0056 and 0x5600 to AR0230 in the existing OnSemi probe. The accompanying comment explains the byte-swapped reading observed with the S3L HAL.

src/sensors.c

Tests (1) +2 / -0

sensors_golden.txtAdd AR0230 golden-list expectations +2/-0

Add AR0230 golden-list expectations

• Adds expected AR0230 names for both chip-version values to the sensor-detection fingerprint.

src/sensors_golden.txt

[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. Unverified 0x5600 alias may misname other onsemi-address parts 🐞 Bug ≡ Correctness

Description

The case 0x5600: label in detect_onsemi_sensor maps any 16-bit read of register 0x3000 equal to
0x5600 to AR0230, based on an untested byte-swap assumption. The PR's own description declines to
guess IMX172/IMX326 IDs because a wrong guess could misname sensors detected correctly today, and
this alias carries the same risk: any part reading 0x5600 at 0x3000 on addresses 0x20/0x30 would now
be named AR0230 before later probes run, where today it falls to default: break and is declined.

Code

src/sensors.c[R455-457]

+    case 0x0056:
+    case 0x5600:
+        sid = 0x0230;

Evidence

The alias is added to the onsemi probe, which runs before the later probes for the same address, so
a claim here is terminal. The PR admits the 0x0056 and 0x5600 rows are unproven on hardware (no
AR0230 available). The 0x5009 precedent was validated on a real DS-2CD2423G0-I, and no equivalent
evidence exists for 0x5600. The PR's own justification for leaving IMX172/IMX326 unnamed applies to
this alias too.

src/sensors.c[451-458]
src/hal/ambarella.c[9-11]
src/sensors_golden.txt[1101-1107]

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 `case 0x5600:` label in `detect_onsemi_sensor` names any 16-bit read of 0x3000 equal to 0x5600 as AR0230. The byte-swapped value is inferred, not observed on hardware, and this probe runs before others, so a non-AR0230 part reading 0x5600 would be misnamed.
## Fix Focus Areas
- src/sensors.c[451-458]
- src/sensors_golden.txt[1101-1107]
## Recommended Fix
Remove the `case 0x5600:` label and the `onsemi 5600 AR0230` golden row. Keep `case 0x0056:` with the datasheet value. Re-add the swapped form only after a board read confirms the S3L reader returns 0x5600 for an AR0230, and then update the comment to cite that capture.

ⓘ 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 hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[widgetii]

On the 0x5600 finding: keeping it, and here's why.

The swap isn't inferred; it's the S3L reader's measured behaviour. On the DS-2CD2423G0-I, register 0x3000 of the AR0237 holds 0x0950 and arrives as 0x5009 through universal_i2c_read_register, which returns the low byte first. That reader is the one ambarella_setup_hal installs. An AR0230's datasheet value, 0x0056, can therefore only arrive as 0x5600 on an S3L, the platform this PR exists for. Without that row, AR0230 isn't detected there at all.

The unproven part is narrower: whether an AR0230 reads its datasheet value. 0x0056 covers readers that don't swap. A false match on 0x5600 would need some other part at onsemi's 0x20/0x30 whose real ID is 0x5600, and none in this table is.