From e5e0e778ca911368b01a7a753eaecb1dca16afa0 Mon Sep 17 00:00:00 2001 From: widgetii <6576495+widgetii@users.noreply.github.com> Date: Wed, 27 May 2026 16:25:12 +0300 Subject: [PATCH] kernel/sensor_i2c/hi3516cv200: don't clobber vendor hi_i2c_master_send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #162 (May 21, "kernel/hi3516cv200: bring up V2 generation against Linux 7.0") added compat shims to `kernel/sensor_i2c/hi3516cv200/sensor_i2c.c` so the module would build on mainline kernels: #ifndef hi_i2c_master_send #define hi_i2c_master_send i2c_master_send #endif #ifndef I2C_M_16BIT_REG #define I2C_M_16BIT_REG 0 #endif The `#ifndef I2C_M_16BIT_REG` check is correct — that symbol is a macro on OpenIPC's hisilicon-hi3516cv200 kernel (`include/uapi/linux/i2c.h` defines it as `0x0002`), so the preprocessor sees it and skips the fallback. The `#ifndef hi_i2c_master_send` check is **wrong**. `hi_i2c_master_send` is exported by `drivers/i2c/busses/i2c-hibvt.c` as an `extern` function (an EXPORT_SYMBOL'd symbol, not a preprocessor macro). The preprocessor cannot see extern function symbols, so `#ifndef hi_i2c_master_send` always evaluates to true and the `#define hi_i2c_master_send i2c_master_send` macro fires — silently routing every sensor write through `i2c_master_send` → `i2c_transfer` → `rt_mutex_trylock` instead of the vendor's `hi_i2c_master_send` → `hibvt_i2c_xfer` direct-xfer path. The vendor extern is IRQ-safe by design: `hibvt_i2c_xfer` is called directly with no i2c-core locking. The mainline path is **not** IRQ-safe: `rt_mutex_trylock` (kernel/locking/rtmutex.c:1545) WARNs and returns 0 in hardirq/softirq context, `i2c_transfer` returns `-EAGAIN`, and every sensor write inside `ISP_IntBottomHalf` (called from `ISP_ISR`) silently fails. Sensor exposure/gain registers never get updated, AE becomes a no-op control loop, and the visible symptom is per-sensor (OV2735 winds to max gain → image white; soi_f22 stays at boot-default → dim). Gate the entire mainline-fallback block on `I2C_M_16BIT_REG`, which the preprocessor can actually see. On vendor kernel the macro exists, the block is skipped, and the original vendor xfer is used. On mainline both fallbacks fire together — `hi_i2c_master_send` becomes `i2c_master_send` AND the 16-bit flag defines become 0, which is the consistent state #162 was attempting to reach in the first place. The asymmetric handling (function aliased but flags preserved) is what made the bug invisible at review — the build still worked, only the runtime path changed. Reported-by: moontwister (OpenIPC/firmware#2144, OV2735) Verified-on: hi3518ev200 + soi_f22 (dlab lab bench) — rt_mutex_trylock WARN count goes from 3+ to 0 with this single-file change. Image goes from dim+stuck (sensor stuck at boot default, ±0.3% JPEG variance) to bright+stable (AE actively converging, 12/15 burst at ±1% with three AE-transition outliers; AE Error narrows from -44 to 10). Fixes: abadb7cf ("kernel/hi3516cv200: bring up V2 generation against Linux 7.0 (issue #51) (#162)") --- kernel/sensor_i2c/hi3516cv200/sensor_i2c.c | 30 ++++++++++++++-------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/kernel/sensor_i2c/hi3516cv200/sensor_i2c.c b/kernel/sensor_i2c/hi3516cv200/sensor_i2c.c index d520e6a..d1582df 100644 --- a/kernel/sensor_i2c/hi3516cv200/sensor_i2c.c +++ b/kernel/sensor_i2c/hi3516cv200/sensor_i2c.c @@ -10,20 +10,28 @@ #include #include "isp_ext.h" -/* HiSi vendor i2c extensions don't exist in mainline. Map to standard - * i2c_master_send/recv. Drop the 16-bit reg/data flags — sensors that - * require 16-bit register addressing will be broken at runtime (not at - * build), needs follow-up kernel patch for true 16-bit support. */ -#ifndef hi_i2c_master_send +/* + * OpenIPC's hisilicon-hi3516cv200 kernel patches i2c with a vendor + * extension: `hi_i2c_master_send` calls `hibvt_i2c_xfer` directly, + * bypassing i2c-core's rt_mutex_trylock so it is safe from the ISP_ISR + * hot path. The vendor uapi i2c.h also defines I2C_M_16BIT_REG and + * I2C_M_16BIT_DATA so the controller can switch reg/data width per + * transfer. On mainline kernels neither exists; we fall back to the + * standard i2c API (which WARNs from atomic context via + * rt_mutex_trylock — see OpenIPC/firmware#2144) and lose true 16-bit + * sensor addressing until a kernel-side patch lands. + * + * Gate the entire compat shim on the I2C_M_16BIT_REG macro — it is + * the only macro symbol the vendor provides that the preprocessor can + * see. The function symbol `hi_i2c_master_send` is an extern, so a + * naive `#ifndef hi_i2c_master_send` triggers the macro fallback even + * on the vendor kernel, silently routing all sensor writes through + * rt_mutex_trylock and making AE writes fail from ISR context. + */ +#ifndef I2C_M_16BIT_REG #define hi_i2c_master_send i2c_master_send -#endif -#ifndef hi_i2c_master_recv #define hi_i2c_master_recv i2c_master_recv -#endif -#ifndef I2C_M_16BIT_REG #define I2C_M_16BIT_REG 0 -#endif -#ifndef I2C_M_16BIT_DATA #define I2C_M_16BIT_DATA 0 #endif