From af293ba7f87704a82eb8c44b9026fec1957ad331 Mon Sep 17 00:00:00 2001 From: AI Dev Date: Fri, 9 Oct 2026 18:20:07 +0000 Subject: [PATCH] sensors: probing a streaming SP2305 no longer turns its picture white 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. --- src/sensors.c | 111 +++++++++++++++++++++++++++------------------ src/sensors_test.c | 84 ++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 45 deletions(-) diff --git a/src/sensors.c b/src/sensors.c index 9cd93870..2d3bef11 100644 --- a/src/sensors.c +++ b/src/sensors.c @@ -818,46 +818,31 @@ static int detect_omni_sensor(sensor_ctx_t *ctx, int fd, return true; } -static int detect_galaxycore_sensor(sensor_ctx_t *ctx, int fd, - unsigned char i2c_addr) { - if (i2c_change_addr(fd, i2c_addr) < 0) - return false; - - int prod_msb = i2c_read_register(fd, i2c_addr, 0x3f0, 2, 1); - int prod_lsb = i2c_read_register(fd, i2c_addr, 0x3f1, 2, 1); - - if (prod_msb == -1 || prod_lsb == -1) { - prod_msb = i2c_read_register(fd, i2c_addr, 0xf0, 1, 1); - prod_lsb = i2c_read_register(fd, i2c_addr, 0xf1, 1, 1); - } - - if (prod_msb == -1 || prod_lsb == -1) - return false; - - int res = prod_msb << 8 | prod_lsb; - - switch (res) { - case 0x2053: - case 0x2083: - case 0x2093: - case 0x4023: - case 0x4653: - sprintf(ctx->sensor_id, "GC%04x", res); - return true; - } - - prod_msb = i2c_read_register(fd, i2c_addr, 0xf0, 1, 1); +/* A GalaxyCore ID read a byte-address at a time, 0 when it is not one. */ +static int galaxycore_narrow_id(int fd, unsigned char i2c_addr) { + int prod_msb = i2c_read_register(fd, i2c_addr, 0xf0, 1, 1); if (prod_msb == -1) - return false; - - prod_lsb = i2c_read_register(fd, i2c_addr, 0xf1, 1, 1); + return 0; + int prod_lsb = i2c_read_register(fd, i2c_addr, 0xf1, 1, 1); if (prod_lsb == -1) - return false; - res = prod_msb << 8 | prod_lsb; + return 0; + return prod_msb << 8 | prod_lsb; +} - if (!res) +static int detect_galaxycore_sensor(sensor_ctx_t *ctx, int fd, + unsigned char i2c_addr) { + if (i2c_change_addr(fd, i2c_addr) < 0) return false; + /* The byte-address ID first. A two-byte address is a write as far as a + * part with one-byte addresses is concerned: reading 0x3f0 sends 0x03, + * 0xf0, and such a part stores 0xf0 in its register 0x03. On an SP2305 / + * OV2735 that is the exposure, and a camera that was streaming went white + * and stayed white until its streamer restarted -- the GC1004 this + * address was added for has its exposure at 0x03 too. Every part this + * probe has ever named by its byte-address ID is named here before + * anything is written to it. */ + int res = galaxycore_narrow_id(fd, i2c_addr); switch (res) { case 0x1004: case 0x1024: @@ -868,6 +853,7 @@ static int detect_galaxycore_sensor(sensor_ctx_t *ctx, int fd, case 0x2053: case 0x2063: case 0x2083: + case 0x2093: case 0x3003: case 0x4023: case 0x4653: @@ -876,25 +862,55 @@ static int detect_galaxycore_sensor(sensor_ctx_t *ctx, int fd, case 0x5603: sprintf(ctx->sensor_id, "GC%04x", res); return true; + } + + int prod_msb = i2c_read_register(fd, i2c_addr, 0x3f0, 2, 1); + int prod_lsb = i2c_read_register(fd, i2c_addr, 0x3f1, 2, 1); + if (prod_msb == -1 || prod_lsb == -1) + return false; + + int wide = prod_msb << 8 | prod_lsb; + switch (wide) { + case 0x2053: + case 0x2083: + case 0x2093: + case 0x4023: + case 0x4653: + sprintf(ctx->sensor_id, "GC%04x", wide); + return true; case 0xffff: // no response return false; - default: - SENSOR_ERR("GalaxyCore", res); - return false; } + if (res && res != 0xffff) + SENSOR_ERR("GalaxyCore", res); + return false; } +static int detect_superpix_on_page0(sensor_ctx_t *ctx, int fd, + unsigned char i2c_addr); + static int detect_superpix_sensor(sensor_ctx_t *ctx, int fd, unsigned char i2c_addr) { if (i2c_change_addr(fd, i2c_addr) < 0) return false; - // Set page 0 + /* The ID is on page 0, and the page is left as it was found. A streamer + * driving this sensor writes its exposure to page 1 and does not select + * the page again before every write, so a probe that leaves page 0 + * selected sends the next exposure to the wrong page. One data byte: two + * wrote 0x00 on into 0xFE as well. */ int page = i2c_read_register(fd, i2c_addr, 0xFD, 1, 1); if (page > 0) - i2c_write_register(fd, i2c_addr, 0xFD, 1, 0x00, 2); + i2c_write_register(fd, i2c_addr, 0xFD, 1, 0x00, 1); + int res = detect_superpix_on_page0(ctx, fd, i2c_addr); + if (page > 0) + i2c_write_register(fd, i2c_addr, 0xFD, 1, page, 1); + return res; +} +static int detect_superpix_on_page0(sensor_ctx_t *ctx, int fd, + unsigned char i2c_addr) { int prod_msb = i2c_read_register(fd, i2c_addr, 0x02, 1, 1); if (prod_msb == -1) return false; @@ -1174,17 +1190,22 @@ static bool get_sensor_id_i2c(sensor_ctx_t *ctx) { SENSOR_SMARTSENS)) { strcpy(ctx->vendor, "SmartSens"); detected = true; - } else if (detect_possible_sensors(ctx, fd, detect_galaxycore_sensor, - SENSOR_GALAXYCORE)) { - strcpy(ctx->vendor, "GalaxyCore"); - ctx->reg_width = 1; - detected = true; + /* SuperPix before GalaxyCore. On HiSilicon both are probed at the same + * device (0x78 and 0x79 are one 7-bit address), and the SuperPix probe + * only ever reads and writes a byte-address at a time, so a sensor of + * that family is named before any two-byte address reaches it -- which on + * a one-byte-address part is a register write. */ } else if (detect_possible_sensors(ctx, fd, detect_superpix_sensor, SENSOR_SUPERPIX)) { // vendor is set by the probe itself: this register family carries both // SuperPix and OmniVision part numbers ctx->reg_width = 1; detected = true; + } else if (detect_possible_sensors(ctx, fd, detect_galaxycore_sensor, + SENSOR_GALAXYCORE)) { + strcpy(ctx->vendor, "GalaxyCore"); + ctx->reg_width = 1; + detected = true; } else if (detect_possible_sensors(ctx, fd, detect_techpoint_adc, SENSOR_TECHPOINT)) { strcpy(ctx->vendor, "TechPoint"); diff --git a/src/sensors_test.c b/src/sensors_test.c index 95ddc946..b8fc6daa 100644 --- a/src/sensors_test.c +++ b/src/sensors_test.c @@ -500,6 +500,89 @@ static void check_probe_restores_console(void) { i2c_read_register = mock_read; } +/* ---- a streaming SP2305 shares its address with a GalaxyCore probe ------ */ + +/* On HiSilicon the GalaxyCore probe runs at 0x78 and the SuperPix one at 0x79: + * one 7-bit address, so both reach an SP2305 / OV2735. That part has one-byte + * register addresses, and a two-byte address is a write as far as it is + * concerned -- the GalaxyCore read of 0x3f0 stored 0xf0 in its register 0x03, + * which is the exposure, and a camera streaming at the time went white until + * its streamer restarted. So: named without a single two-byte address reaching + * it, and left on the register page it was found on. */ +static int sp_page; +static int sp_wide; +static int sp_wide_writes; +static int sp_bad_page_writes; + +static int sp_read(int fd, unsigned char addr, uint32_t reg, + unsigned int reg_width, unsigned int data_width) { + (void)fd; + (void)data_width; + if (addr != mock_addr) + return -1; + if (reg_width != 1) { + sp_wide++; + return -1; + } + if (reg == 0xFD) + return sp_page; + if (sp_page == 0 && reg == 0x02) + return 0x27; + if (sp_page == 0 && reg == 0x03) + return 0x35; + return 0x40; /* what this part answers at 0xf0/0xf1 on page 1 */ +} + +static int sp_write(int fd, unsigned char addr, uint32_t reg, + unsigned int reg_width, uint32_t data, + unsigned int data_width) { + (void)fd; + if (addr != mock_addr) + return -1; + if (reg_width != 1) + sp_wide_writes++; + if (reg == 0xFD) { + if (data_width != 1) + sp_bad_page_writes++; + sp_page = (int)data; + } + return 0; +} + +static void check_superpix_spared_by_galaxycore(void) { + static unsigned char addrs[] = {0x60, 0}; + static sensor_addr_t table[] = {{SENSOR_GALAXYCORE, addrs}, + {SENSOR_SUPERPIX, addrs}, + {0, NULL}}; + sensor_ctx_t ctx; + + setup_hal_fallback(); + open_i2c_sensor_fd = quiet_open; + i2c_read_register = sp_read; + i2c_write_register = sp_write; + i2c_change_addr = mock_change_addr; + possible_i2c_addrs = table; + + memset(&ctx, 0, sizeof(ctx)); + sp_page = 1; /* where the streamer keeps it */ + sp_wide = sp_wide_writes = sp_bad_page_writes = 0; + CHECK(getsensorid(&ctx)); + CHECK(!strcmp(ctx.sensor_id, "OV2735")); + CHECK(sp_wide == 0); + CHECK(sp_wide_writes == 0); + CHECK(sp_bad_page_writes == 0); + CHECK(sp_page == 1); + if (strcmp(ctx.sensor_id, "OV2735") || sp_wide || sp_page != 1) + fprintf(stderr, + " (SP2305 at a GalaxyCore address: got \"%s\", %d two-byte " + "reads, left on page %d)\n", + ctx.sensor_id, sp_wide, sp_page); + + possible_i2c_addrs = NULL; + i2c_read_register = mock_read; + i2c_write_register = mock_write; +} + int main(int argc, char **argv) { i2c_read_register = mock_read; i2c_write_register = mock_write; @@ -526,6 +609,7 @@ int main(int argc, char **argv) { check_techpoint(); check_vendor_dependent(); check_probe_restores_console(); + check_superpix_spared_by_galaxycore(); if (failures) fprintf(stderr, "sensors_test: %d failure(s)\n", failures);