Skip to content

sensors: probing a streaming SP2305 no longer turns its picture white - #238

Merged
openipc-ai merged 1 commit into
masterfrom
sensor-probe-spares-superpix
Oct 9, 2026
Merged

openipc-ai merged 1 commit into
masterfrom
sensor-probe-spares-superpix

Conversation

@openipc-ai

Copy link
Copy Markdown
Contributor

What happened

On HiSilicon the GalaxyCore probe runs at 0x78, the same 7-bit address as the SuperPix probe's 0x79, which is an SP2305 / OV2735. The GalaxyCore probe's first read used a two-byte register address, 0x3f0.

The SP2305 has one-byte addresses, so it took that read as a write: 0xf0, then 0xf1, into register 0x03. That is the high byte of its exposure.

On a camera that was streaming, the picture went white and stayed white until the streamer restarted. The sensor driver rewrites the exposure only when it changes, and in a bright scene it doesn't (OpenIPC/openhisilicon#255 fixes that side). ipctool -s and ipcinfo --short-sensor both did it, every time. This is very likely the random white-out in OpenIPC/majestic#295.

Changes

  • SuperPix probed before GalaxyCore. It only reads and writes one byte of address at a time, so a part in that register family is named before any two-byte address reaches it.
  • GalaxyCore tries the byte-address ID first (0xf0/0xf1) and only falls back to the two-byte one for an ID that doesn't name a part. The GC1004 that 0x78 was added for has one-byte addresses, and its exposure is at 0x03 too.
  • SuperPix page select fixed. It now selects page 0 with one data byte, not two (the second went on into 0xFE), and puts back the page it found. A streamer writes the exposure to page 1 without reselecting the page every time.

Tests

  • Fingerprint: src/sensors_golden.txt is unchanged; every ID names the same part as before.
  • New check in sensors_test: a scripted SP2305 on page 1, at an address shared with a GalaxyCore probe.
    • Against the old code it records two two-byte reads, a two-byte page write, and page 0 left selected.
    • Against this change it records none of them.
  • Other suites: all host tests in pr-build-check.yml pass.

On hardware

hi3516ev300 + SP2305/OV2735, streaming under an LED floodlight, auto-exposure at ~130 lines:

before after
ipctool -s "unexpected value for GalaxyCore", then picture white, register 0x03 = 0xf1 ov2735_i2c, register and picture unchanged
ipcinfo --short-sensor picture white ov2735, unchanged
full ipctool report — OmniVision OV2735 at 0x79, unchanged

On HiSilicon the GalaxyCore probe runs at 0x78, which is the same 7-bit
address as the SuperPix probe's 0x79 -- an SP2305 / OV2735. Its first
read was a two-byte address, 0x3f0. That sensor has one-byte addresses,
so it took the read as a write: 0xf0, then 0xf1, into register 0x03,
the high byte of its exposure. On a camera that was streaming, the
picture went white and stayed white -- the sensor's driver rewrites the
exposure only when it changes, and in a bright scene it does not --
until the streamer restarted. `ipctool -s` and `ipcinfo --short-sensor`
both did it, every time.

- SuperPix is probed before GalaxyCore. It only ever reads and writes a
  byte-address at a time, so a SuperPix-register-family part is named
  before any two-byte address reaches it.
- GalaxyCore reads its byte-address ID first and tries the two-byte one
  only for what that does not name. The GC1004 that 0x78 was added for
  has one-byte addresses and its exposure at 0x03 too.
- The SuperPix probe selects page 0 with one data byte, not two (the
  second went on into 0xFE), and puts back the page it found. A
  streamer writes the exposure to page 1 without selecting it every
  time.

The fingerprint is unchanged: every ID names the same part as before.
sensors_test gains a scripted SP2305 on page 1 at an address shared
with a GalaxyCore probe; against the old code it sees two two-byte
reads, a two-byte page write and page 0 left selected.

Measured on hi3516ev300 + SP2305/OV2735 while streaming under an LED
floodlight: `ipctool -s`, `ipcinfo --short-sensor` and a full report
name the sensor OV2735, and the exposure register and the picture are
unchanged. Before, each of the first two left it white.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Prevent sensor probing from whitening a streaming SP2305

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Probe SuperPix before GalaxyCore to identify shared-address sensors without hazardous two-byte
 reads.
• Preserve the sensor’s register page and use one-byte page writes during SuperPix detection.
• Add a streaming-SP2305 regression test while retaining the existing sensor-ID fingerprint.
Diagram

graph TD
  Scan["I²C sensor scan"] --> SuperPix["SuperPix probe"] --> PageID["Page-zero ID"] --> Restore["Restore page"] --> GalaxyCore["GalaxyCore probe"] --> Narrow["Byte-address ID"] --> Wide["Two-byte fallback"]
  Restore -- "matched" --> Identified["Sensor identified"]
  Restore -- "no match" --> GalaxyCore
  Narrow -- "matched" --> Identified
  Wide -- "matched" --> Identified
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Gate two-byte reads by sensor address metadata
  • ➕ Could prevent two-byte transactions at every address known to host one-byte-register sensors.
  • ➖ Requires reliable platform-specific address and register-width data.
  • ➖ Risks suppressing detection of GalaxyCore parts that need the two-byte ID path.

Recommendation: The targeted probe order and byte-first identification are appropriate because they protect the reported shared-address sensor while preserving existing GalaxyCore identification paths. Address-based gating is worth considering only if further collisions show the remaining two-byte fallback needs broader safeguards.

Files changed (2) +150 / -45

Bug fix (1) +66 / -45
sensors.cMake shared-address detection safer for streaming sensors +66/-45

Make shared-address detection safer for streaming sensors

• Moves SuperPix ahead of GalaxyCore and tries GalaxyCore byte-address IDs before two-byte IDs. SuperPix detection now uses one-byte page-select writes and restores the original page after reading the ID.

src/sensors.c

Tests (1) +84 / -0
sensors_test.cCover a streaming SP2305 at a shared probe address +84/-0

Cover a streaming SP2305 at a shared probe address

• Adds a scripted page-1 SP2305/OV2735 to the full sensor scan. The test checks its identification, absence of two-byte transactions or malformed page writes, and restoration of page 1.

src/sensors_test.c

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant