From 49d7b1c8ff48b5e434a73bdf7e3f6b28a2e6c1d8 Mon Sep 17 00:00:00 2001 From: Dmitry Ilyin <6576495+widgetii@users.noreply.github.com> Date: Fri, 28 Aug 2026 08:54:43 +0300 Subject: [PATCH] hal: re-enable the sensor clock before every probe, not once per process getsensorid() reads the sensor over i2c, so the sensor has to be clocked to answer. On Ingenic that clock is turned on by setup_hal_ingenic(), which runs from hw_detect_system() inside getchipname() -- and getchipname() caches: if (*sysid) return sysid; so the HAL setup happens exactly once per process. That is fine while nothing takes the clock away again. Something does: a vendor SDK gates it off when it tears its pipeline down, leaving /proc/jz/clock/cgu_cim/enable reading "disabled". Every probe after that reads an unclocked sensor and reports that the board has none. The symptom is confusing in a specific way. A fresh process always gets the right answer, because it runs the HAL setup again on its first getchipname() -- so `ipcinfo -l` from a shell answers correctly at the very moment a long-lived caller is being told there is no sensor. That reads as a caller bug, or as flaky hardware, rather than as this. So make it a HAL hook and call it before each probe. NULL for every SoC that needs nothing done, which is all of them bar Ingenic today, and cleared in setup_hal_fallback() so detection cannot inherit a previous target's. Measured on a t31 (sc2332) with a caller that probes once per pipeline reload: before first probe finds sc2332_i2c, every later one finds nothing after sc2332_i2c on all of them, across repeated SDK teardowns Found from majestic, where it surfaced as "sensor autodetection failed" and then "Cannot start SDK" on every SIGHUP reload. --- src/hal/common.c | 4 ++++ src/hal/common.h | 6 ++++++ src/hal/ingenic.c | 6 ++++++ src/sensors.c | 11 +++++++++++ 4 files changed, 27 insertions(+) diff --git a/src/hal/common.c b/src/hal/common.c index 107badf..7de2996 100644 --- a/src/hal/common.c +++ b/src/hal/common.c @@ -22,6 +22,7 @@ write_register_t spi_write_register; int (*i2c_change_addr)(int fd, unsigned char addr); float (*hal_temperature)(); void (*hal_cleanup)(); +void (*hal_enable_sensor_clock)(); #ifndef STANDALONE_LIBRARY void (*hal_detect_ethernet)(cJSON *root); @@ -223,6 +224,9 @@ void setup_hal_fallback() { i2c_write_register = universal_i2c_write_register; spi_write_register = universal_spi_write_register; hal_cleanup = universal_hal_cleanup; + /* Cleared, not defaulted: most SoCs need nothing done to make the sensor + * answer, and this runs before detection picks the one that does. */ + hal_enable_sensor_clock = NULL; #ifndef STANDALONE_LIBRARY hal_totalmem = default_totalmem; #endif diff --git a/src/hal/common.h b/src/hal/common.h index f2c6a12..fb129cb 100644 --- a/src/hal/common.h +++ b/src/hal/common.h @@ -91,6 +91,12 @@ extern write_register_t spi_write_register; extern float (*hal_temperature)(); extern void (*hal_cleanup)(); +/* Put the sensor in a state where it can answer a probe, where that takes + * doing. Ingenic gates the sensor's clock, and whoever had the pipeline up + * last may well have gated it off again on the way down — so this has to run + * before every probe, not once per process. NULL where nothing is needed. */ +extern void (*hal_enable_sensor_clock)(); + #ifndef STANDALONE_LIBRARY extern void (*hal_detect_ethernet)(cJSON *handle); extern unsigned long (*hal_totalmem)(unsigned long *media_mem); diff --git a/src/hal/ingenic.c b/src/hal/ingenic.c index 376d473..d416bbf 100644 --- a/src/hal/ingenic.c +++ b/src/hal/ingenic.c @@ -368,6 +368,12 @@ void setup_hal_ingenic() { ingenic_enable_sensor_clock(); possible_i2c_addrs = ingenic_possible_i2c_addrs; open_i2c_sensor_fd = ingenic_open_i2c_fd; + /* Also as a hook, because the call above only ever runs once: getchipname() + * caches the chip id and returns before ever reaching here again. Anything + * that gates the clock off afterwards — the vendor SDK does, on the way + * down — would otherwise leave every later probe reading an unclocked + * sensor and reporting that there is none. */ + hal_enable_sensor_clock = ingenic_enable_sensor_clock; #ifndef STANDALONE_LIBRARY hal_totalmem = ingenic_totalmem; #endif diff --git a/src/sensors.c b/src/sensors.c index 797d15e..50c1dd0 100644 --- a/src/sensors.c +++ b/src/sensors.c @@ -1194,6 +1194,17 @@ bool getsensorid(sensor_ctx_t *ctx) { int current_i2c_adapter_nr; if (!getchipname()) return NULL; + + /* Every probe, not just the first. getchipname() sets the HAL up once and + * then returns its cached answer forever, so anything the setup did to + * make the sensor answerable was done once too. On Ingenic that is the + * sensor's clock, and the vendor SDK gates it off when it tears a pipeline + * down: a second probe in the same process then found an unclocked sensor + * and reported that the board has none. A fresh process got it right, + * which is what made it look like the hardware rather than us. */ + if (hal_enable_sensor_clock) + hal_enable_sensor_clock(); + // there is no platform specific i2c/spi access layer if (!open_i2c_sensor_fd(i2c_adapter_nr)) return NULL;