fix SP2305 detection

[conke]

No description provided.

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

PR Summary by Qodo

Fix SP2305 sensor detection mapping

🐞 Bug fix 🕐 Less than 5 minutes

AI Description

• Correctly identify chip ID 0x2735 as SP2305.
• Prevent mislabeling the sensor as OV2735 during SuperPix detection.

Diagram

graph TD
  A["detect_superpix_sensor()"] --> B["Read chip ID"] --> C["Match 0x2735"] --> D["Set sensor_id: SP2305"] --> E["Return true"]

Loading

High-Level Assessment

The direct mapping fix is appropriate and minimally invasive for correcting SP2305 identification. More structural changes (e.g., a centralized ID-to-name table) were considered but are unnecessary given the narrow scope of the issue.

Files changed (1) +1 / -1

Bug fix (1) +1 / -1

sensors.cFix chip ID 0x2735 to report SP2305 +1/-1

Fix chip ID 0x2735 to report SP2305

• Updates the SuperPix detection case for chip ID 0x2735 to set the sensor identifier string to "SP2305" instead of formatting it as an OV2735 ID. This prevents incorrect sensor naming during detection.

src/sensors.c

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

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Misleading OV2735 comment 🐞 Bug ⚙ Maintainability

Description

In detect_superpix_sensor(), the 0x2735 case now sets ctx->sensor_id to "SP2305" while the adjacent
comment still references "OV2735", leaving the code’s documentation inconsistent with its behavior.
This can mislead maintainers about what sensor/model is actually being reported for that register
value.

Code

src/sensors.c[909]

+        strcpy(ctx->sensor_id, "SP2305");

Evidence

The PR now maps res == 0x2735 to "SP2305", but the comment immediately above still labels this
case as OV2735. Elsewhere in the same function, SuperPix models are generally represented with the
SP%04x naming convention, so the unchanged OV2735 comment stands out as inconsistent with the new
returned ID.

src/sensors.c[903-916]

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 SuperPix detection switch case for `0x2735` now returns the model string `"SP2305"`, but the nearby comment still says `OV2735`, which is inconsistent and confusing for future maintenance.

### Issue Context
This PR changed the model string for `case 0x2735:` from `OV%04x` to a literal `SP2305`, but did not update the comment explaining the case.

### Fix Focus Areas
- src/sensors.c[907-910]

### Suggested fix
Update the comment to accurately describe why `0x2735` maps to `SP2305` (e.g., note that SP2305 reports an OV2735-compatible ID), or adjust the mapping/code so the comment and returned model name are consistent.

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

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

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

1. Misleading ov2735 comment 🐞 Bug ⚙ Maintainability

In detect_superpix_sensor(), the 0x2735 case now sets ctx->sensor_id to "SP2305" while the adjacent
comment still references "OV2735", leaving the code’s documentation inconsistent with its behavior.
This can mislead maintainers about what sensor/model is actually being reported for that register
value.

Agent Prompt

### Issue description
The SuperPix detection switch case for `0x2735` now returns the model string `"SP2305"`, but the nearby comment still says `OV2735`, which is inconsistent and confusing for future maintenance.

### Issue Context
This PR changed the model string for `case 0x2735:` from `OV%04x` to a literal `SP2305`, but did not update the comment explaining the case.

### Fix Focus Areas
- src/sensors.c[907-910]

### Suggested fix
Update the comment to accurately describe why `0x2735` maps to `SP2305` (e.g., note that SP2305 reports an OV2735-compatible ID), or adjust the mapping/code so the comment and returned model name are consistent.

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

[widgetii]

Thanks for the patch, but the PR description is empty, so I can't tell what problem this is fixing. Before we change detection behaviour I need the rationale:

• What device / SoC and which sensor module did you observe this on?
• How did you establish the part is an SP2305 and not an OV2735 — silkscreen on the sensor, module BOM, vendor driver name, something else?
• Please post ipctool i2cdump <sensor-i2c-addr> 0x00 0x0F (page 0, i.e. after writing 0xFD = 0) from that device, together with the full ipctool output.

The reason I'm asking rather than merging: as far as we can tell SP2305 and OV2735 are the same silicon sold under two brands, so 0x2735 doesn't identify one over the other — flipping the label just moves the wrong name onto somebody else's hardware.

• The OmniVision OV2735 datasheet (v2.1, 2017) documents exactly the probe we do here: P0:0x02 CHIP_ID = 0x27, P0:0x03 CHIP_ID = 0x35, P0:0x04 ECO_VER = 0x01, P0:0xFD page select. So 0x2735 is OV2735's own documented ID.
• SP2305 vendor drivers use an identical register map — e.g. XM's sp2305_cmos.c writes 0xfd=1, then 0x03/0x04 as exposure MSB/LSB, matching the OV2735 datasheet's P1:0x03 BUF_EXP_8MSB / P1:0x04 BUF_EXP_8LSB. Goke's own libsns_sp2305.so even prints ==ominivision sp2305 sensor HCG 1080P30fps(MIPI port) init success!==. OmniVision and SuperPix have been the same group (WillSemi) since 2018-19, and SP↔OV/OS silicon twins are routine.
• OV2735B is a third marketing name that most likely reports the same ID (separate Ingenic driver and yaml).

There's also a compatibility problem. ipcinfo --short-sensor output is consumed programmatically by the firmware load scripts, which case on the lowercased name:

• hisilicon-osdrv-hi3516cv200/.../load_hisilicon → ov2710_mipi | ov2735_i2c_1080p | ov2735)
• hisilicon-osdrv-hi3516cv300/.../load_hisilicon → ov2718_i2c|ov2735_i2c)
• ingenic-osdrv-t21/.../load_ingenic → "ov2735b")
• nothing anywhere matches sp2305

So as it stands this patch would drop those boards into the *) UNSUPPORTED sensor branch — no pinmux, no sensor clock, camera doesn't come up.

If your register dump shows something that genuinely differs between the two labels (ECO_VER at P0:0x04, or the package register at P2:0x35, are the candidates), we can distinguish them properly — the same approach requested in #112 — and I'd be glad to take that patch. If the two are bit-identical, then the correct output is one name plus an alias comment, and the firmware side needs a matching change before the name can flip.

[widgetii]

Closing this due to no response