Skip to content

Commit 697f1ad

Browse files
authored
hal: re-enable the sensor clock before every probe, not once per process (#184)
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.
1 parent bd9997b commit 697f1ad

4 files changed

Lines changed: 27 additions & 0 deletions

File tree

src/hal/common.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ write_register_t spi_write_register;
2222
int (*i2c_change_addr)(int fd, unsigned char addr);
2323
float (*hal_temperature)();
2424
void (*hal_cleanup)();
25+
void (*hal_enable_sensor_clock)();
2526

2627
#ifndef STANDALONE_LIBRARY
2728
void (*hal_detect_ethernet)(cJSON *root);
@@ -223,6 +224,9 @@ void setup_hal_fallback() {
223224
i2c_write_register = universal_i2c_write_register;
224225
spi_write_register = universal_spi_write_register;
225226
hal_cleanup = universal_hal_cleanup;
227+
/* Cleared, not defaulted: most SoCs need nothing done to make the sensor
228+
* answer, and this runs before detection picks the one that does. */
229+
hal_enable_sensor_clock = NULL;
226230
#ifndef STANDALONE_LIBRARY
227231
hal_totalmem = default_totalmem;
228232
#endif

src/hal/common.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ extern write_register_t spi_write_register;
9191
extern float (*hal_temperature)();
9292
extern void (*hal_cleanup)();
9393

94+
/* Put the sensor in a state where it can answer a probe, where that takes
95+
* doing. Ingenic gates the sensor's clock, and whoever had the pipeline up
96+
* last may well have gated it off again on the way down — so this has to run
97+
* before every probe, not once per process. NULL where nothing is needed. */
98+
extern void (*hal_enable_sensor_clock)();
99+
94100
#ifndef STANDALONE_LIBRARY
95101
extern void (*hal_detect_ethernet)(cJSON *handle);
96102
extern unsigned long (*hal_totalmem)(unsigned long *media_mem);

src/hal/ingenic.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,12 @@ void setup_hal_ingenic() {
368368
ingenic_enable_sensor_clock();
369369
possible_i2c_addrs = ingenic_possible_i2c_addrs;
370370
open_i2c_sensor_fd = ingenic_open_i2c_fd;
371+
/* Also as a hook, because the call above only ever runs once: getchipname()
372+
* caches the chip id and returns before ever reaching here again. Anything
373+
* that gates the clock off afterwards — the vendor SDK does, on the way
374+
* down — would otherwise leave every later probe reading an unclocked
375+
* sensor and reporting that there is none. */
376+
hal_enable_sensor_clock = ingenic_enable_sensor_clock;
371377
#ifndef STANDALONE_LIBRARY
372378
hal_totalmem = ingenic_totalmem;
373379
#endif

src/sensors.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,6 +1194,17 @@ bool getsensorid(sensor_ctx_t *ctx) {
11941194
int current_i2c_adapter_nr;
11951195
if (!getchipname())
11961196
return NULL;
1197+
1198+
/* Every probe, not just the first. getchipname() sets the HAL up once and
1199+
* then returns its cached answer forever, so anything the setup did to
1200+
* make the sensor answerable was done once too. On Ingenic that is the
1201+
* sensor's clock, and the vendor SDK gates it off when it tears a pipeline
1202+
* down: a second probe in the same process then found an unclocked sensor
1203+
* and reported that the board has none. A fresh process got it right,
1204+
* which is what made it look like the hardware rather than us. */
1205+
if (hal_enable_sensor_clock)
1206+
hal_enable_sensor_clock();
1207+
11971208
// there is no platform specific i2c/spi access layer
11981209
if (!open_i2c_sensor_fd(i2c_adapter_nr))
11991210
return NULL;

0 commit comments

Comments
 (0)