From 7aaa709b0a30302360fe2560cdc83616e4b1aab1 Mon Sep 17 00:00:00 2001 From: Ben Neigher Date: Sun, 16 Aug 2026 12:12:33 -0700 Subject: [PATCH 1/3] load_goke: opt-in sensor_dvp/sensor_mclk gate for DVP-wired boards open_sys_config picks MIPI or DVP pad routing from its chip= and g_cmos_yuv_flag= arguments. On a board that wires the sensor to the DVP pads, the MIPI arguments mux the i2c controller to pads the sensor is not connected to, so every address NACKs and no sensor is ever detected. Gated on an env var rather than $CHIP_TYPE deliberately: both wirings exist on gk7202v300, so keying this off the SoC name would fix DVP boards by breaking every MIPI one. Profiles opt in with 'fw_setenv sensor_dvp 1'; unset means the current behaviour is unchanged. sensor_mclk is gated the same way. open_sys_config reads MCLK only from its module parameters and defaults to 27 MHz at this chip=, so a sensor init table tuned for 24 MHz runs against the wrong clock -- and the ini's MCLK key is never consulted on this path, which makes sweeping it look like a ruled-out cause. Refs: OpenIPC/firmware#2074 --- .../files/script/load_goke | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/general/package/goke-osdrv-gk7205v200/files/script/load_goke b/general/package/goke-osdrv-gk7205v200/files/script/load_goke index a404047236..22cc364790 100755 --- a/general/package/goke-osdrv-gk7205v200/files/script/load_goke +++ b/general/package/goke-osdrv-gk7205v200/files/script/load_goke @@ -21,6 +21,22 @@ os_mem_size=${os_mem_size:=32} BOARD=demo YUV_TYPE0=0 # 0 -- raw, 1 --DC, 2 --bt1120, 3 --bt656 +# DVP-wired sensor boards -- OPT-IN, set by the device profile. +# +# open_sys_config selects MIPI or DVP pad routing from its chip= and +# g_cmos_yuv_flag= arguments. A board that wires the sensor to the DVP pads +# needs the DVP path; with the MIPI arguments the i2c controller is muxed to +# pads that are not connected to the sensor, so every address NACKs and no +# sensor is ever detected. +# +# Keyed off an env var rather than the SoC name on purpose: both wirings exist +# on gk7202v300, so testing $CHIP_TYPE here would fix DVP boards by breaking +# every MIPI one. Profiles opt in with `fw_setenv sensor_dvp 1`. +if [ "$(fw_printenv -n sensor_dvp 2>/dev/null)" = "1" ]; then + CHIP_TYPE=gk7205v200 + YUV_TYPE0=1 +fi + cd /lib/modules/$(uname -r)/goke/ ################################################################## @@ -165,6 +181,13 @@ insert_ko() { YUV_TYPE0=1 fi modprobe open_sys_config chip=$CHIP_TYPE sensors=$SENSOR g_cmos_yuv_flag=$YUV_TYPE0 board=$BOARD + # open_sys_config takes MCLK only from its module parameters and defaults to + # 27 MHz at this chip=. A sensor init table tuned for 24 MHz (the GC2053 + # ForCar tables are) then runs against the wrong clock. The ini's MCLK key is + # not consulted on this path, so the register is written directly. + case "$(fw_printenv -n sensor_mclk 2>/dev/null)" in + 24) devmem 0x120100F0 32 0x0000000D ;; + esac insert_osal insmod gk7205v200_base.ko insmod gk7205v200_sys.ko From 0318d51cd8ccc0f9943556173b8026d5b438821e Mon Sep 17 00:00:00 2001 From: Ben Neigher Date: Mon, 17 Aug 2026 12:42:07 -0700 Subject: [PATCH 2/3] =?UTF-8?q?load=5Fgoke:=20address=20review=20=E2=80=94?= =?UTF-8?q?=20separate=20the=20chip=3D=20selector,=20and=20do=20not=20writ?= =?UTF-8?q?e=20MCLK=20blind?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from review: 1. CHIP_TYPE role conflation. The opt-in was overwriting CHIP_TYPE, which is the DETECTED SoC, with the value wanted for open_sys_config's chip= pad-routing selector. Those are two different things that happen to coincide by default: a DVP board needs the gk7205v200 routing tables while still BEING a gk7202v300. Conflating them makes every existing and future CHIP_TYPE branch harder to reason about. Now a dedicated SYSCFG_CHIP carries the selector and CHIP_TYPE keeps meaning exactly one thing. 2. Silent MCLK write. devmem was called without checking that it exists or that the write succeeded. A wrong MCLK does not fail loudly — it yields a corrupted or absent image while every log line still looks healthy — so this is exactly the case that must not pass quietly. Both failures now log to daemon.err. Still opt-in and still keyed off env vars: with sensor_dvp unset the script is behaviourally identical to before. Refs: OpenIPC/firmware#2074 --- .../files/script/load_goke | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/general/package/goke-osdrv-gk7205v200/files/script/load_goke b/general/package/goke-osdrv-gk7205v200/files/script/load_goke index 22cc364790..4e39f3c405 100755 --- a/general/package/goke-osdrv-gk7205v200/files/script/load_goke +++ b/general/package/goke-osdrv-gk7205v200/files/script/load_goke @@ -32,8 +32,14 @@ YUV_TYPE0=0 # 0 -- raw, 1 --DC, 2 --bt1120, 3 --bt656 # Keyed off an env var rather than the SoC name on purpose: both wirings exist # on gk7202v300, so testing $CHIP_TYPE here would fix DVP boards by breaking # every MIPI one. Profiles opt in with `fw_setenv sensor_dvp 1`. +# SYSCFG_CHIP is the value handed to open_sys_config's chip= selector, which is +# NOT the same thing as CHIP_TYPE (the detected SoC). They coincide by default; +# a DVP board needs the gk7205v200 pad-routing tables while still BEING a +# gk7202v300. Keeping them separate leaves CHIP_TYPE meaning exactly one thing, +# so existing and future chip-specific branches are unaffected by this opt-in. +SYSCFG_CHIP=$CHIP_TYPE if [ "$(fw_printenv -n sensor_dvp 2>/dev/null)" = "1" ]; then - CHIP_TYPE=gk7205v200 + SYSCFG_CHIP=gk7205v200 YUV_TYPE0=1 fi @@ -107,7 +113,7 @@ insert_osal() { } insert_detect() { - modprobe open_sys_config chip=$CHIP_TYPE sensors=unknown g_cmos_yuv_flag=$YUV_TYPE0 board=$BOARD + modprobe open_sys_config chip=$SYSCFG_CHIP sensors=unknown g_cmos_yuv_flag=$YUV_TYPE0 board=$BOARD insert_osal insmod gk7205v200_base.ko modprobe open_isp @@ -180,13 +186,22 @@ insert_ko() { if [ "$SENSOR" == "bt656" ] || [ "$SENSOR" == "jxf23_dc" ]; then YUV_TYPE0=1 fi - modprobe open_sys_config chip=$CHIP_TYPE sensors=$SENSOR g_cmos_yuv_flag=$YUV_TYPE0 board=$BOARD + modprobe open_sys_config chip=$SYSCFG_CHIP sensors=$SENSOR g_cmos_yuv_flag=$YUV_TYPE0 board=$BOARD # open_sys_config takes MCLK only from its module parameters and defaults to # 27 MHz at this chip=. A sensor init table tuned for 24 MHz (the GC2053 # ForCar tables are) then runs against the wrong clock. The ini's MCLK key is # not consulted on this path, so the register is written directly. + # A wrong MCLK does not fail loudly — it produces a corrupted or absent + # image while every log line still looks healthy. So say something when the + # override cannot be applied, rather than continuing silently misconfigured. case "$(fw_printenv -n sensor_mclk 2>/dev/null)" in - 24) devmem 0x120100F0 32 0x0000000D ;; + 24) + if ! command -v devmem >/dev/null 2>&1; then + logger -p daemon.err -t load_goke "sensor_mclk=24 requested but devmem is missing; MCLK left at the chip default" + elif ! devmem 0x120100F0 32 0x0000000D; then + logger -p daemon.err -t load_goke "sensor_mclk=24: MCLK register write failed; sensor may not produce a usable image" + fi + ;; esac insert_osal insmod gk7205v200_base.ko From aa6c983421633c76b8b4dde15914f10f172a0880 Mon Sep 17 00:00:00 2001 From: Ben Neigher Date: Wed, 26 Aug 2026 19:02:04 -0700 Subject: [PATCH 3/3] load_goke: mask the MCLK field, and only borrow tables for gk7202v300 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses @widgetii's review, which checked this against the pinned SDK source rather than the diff. Both findings were correct. 1. The devmem write was a blind whole-word store to a register the driver only ever read-modify-writes. sensor_clock_config() is reg_write32(clock << 2, 0xF << 2, base+0xF0) — bits [5:2] and nothing else. 0x0000000D happens to put 0x3 (24MHz) in that field, so it worked, but it also cleared bits [31:6] and set bit 0. Now masks: (cur & ~0x3C) | (sel << 2). Also extended to the full documented table (74.25/72/54/24/37.125/36/27/12) instead of a lone magic constant, and an unsupported value now logs instead of doing nothing silently — that was the same failure class the rest of this PR guards against. 2. sensor_dvp=1 no longer forces gk7205v200 unconditionally. Only gk7202v300 lacks a DVP branch: its pinmux() case is MIPI-only and ignores g_cmos_yuv_flag entirely. gk7205v200 and gk7205v300 have their own DVP paths, and pushing a DVP-wired v300 onto the v200 VI pad table would be this very bug pointing the other way. Minor, also from review: logger now follows the file's convention (logger -s -p ... -t goke). The -s matters — a failed MCLK write is bring-up fatal and should reach the console the way 'SENSOR is not detected' does. The command -v devmem guard is kept as belt and braces even though CONFIG_DEVMEM=y, since the read is now load-bearing too. The insert_detect/insert_ko asymmetry is documented rather than changed: first-boot autodetect probes at the sensor table's default clock, which is fine because i2c is master-clocked, but it is the kind of thing that surprises people. Refs: OpenIPC/firmware#2074 --- .../files/script/load_goke | 63 +++++++++++++++---- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/general/package/goke-osdrv-gk7205v200/files/script/load_goke b/general/package/goke-osdrv-gk7205v200/files/script/load_goke index 4e39f3c405..c9a5e939a4 100755 --- a/general/package/goke-osdrv-gk7205v200/files/script/load_goke +++ b/general/package/goke-osdrv-gk7205v200/files/script/load_goke @@ -39,8 +39,16 @@ YUV_TYPE0=0 # 0 -- raw, 1 --DC, 2 --bt1120, 3 --bt656 # so existing and future chip-specific branches are unaffected by this opt-in. SYSCFG_CHIP=$CHIP_TYPE if [ "$(fw_printenv -n sensor_dvp 2>/dev/null)" = "1" ]; then - SYSCFG_CHIP=gk7205v200 YUV_TYPE0=1 + # Only gk7202v300 needs to borrow another chip's tables. Its pinmux() branch + # is MIPI-only — it ignores g_cmos_yuv_flag entirely and always calls + # i2c0_for_mipi_sensor_pin_mux/vi_mipi_rx_mux — so there is no DVP path + # reachable for it at any flag value. gk7205v200 and gk7205v300 each have + # their own DVP path already; forcing a v300 onto the v200 VI pad table would + # be exactly this bug pointing the other way. + if [ "$CHIP_TYPE" = "gk7202v300" ]; then + SYSCFG_CHIP=gk7205v200 + fi fi cd /lib/modules/$(uname -r)/goke/ @@ -191,18 +199,49 @@ insert_ko() { # 27 MHz at this chip=. A sensor init table tuned for 24 MHz (the GC2053 # ForCar tables are) then runs against the wrong clock. The ini's MCLK key is # not consulted on this path, so the register is written directly. - # A wrong MCLK does not fail loudly — it produces a corrupted or absent - # image while every log line still looks healthy. So say something when the - # override cannot be applied, rather than continuing silently misconfigured. - case "$(fw_printenv -n sensor_mclk 2>/dev/null)" in - 24) - if ! command -v devmem >/dev/null 2>&1; then - logger -p daemon.err -t load_goke "sensor_mclk=24 requested but devmem is missing; MCLK left at the chip default" - elif ! devmem 0x120100F0 32 0x0000000D; then - logger -p daemon.err -t load_goke "sensor_mclk=24: MCLK register write failed; sensor may not produce a usable image" + # CRG PERI_CRG60 @ 0x120100F0 — sns0 clock select is bits [5:2] ONLY: + # 0x0:74.25 0x1:72 0x2:54 0x3:24 0x4:37.125 0x5:36 0x6:27 0x7:12 (MHz) + # + # Read-modify-write just that field, matching what the driver itself does + # (sensor_clock_config() is reg_write32(clock << 2, 0xF << 2, base+0xF0)). + # Storing the whole word would also clear bits [31:6] and set bit 0, which + # nothing in the driver ever does — it happens to select the right clock, but + # by accident of the constant rather than by writing the field. + # + # A wrong MCLK does not fail loudly: it yields a corrupted or absent image + # while every log line still looks healthy. Hence -s, so a failure reaches the + # console the way "SENSOR is not detected" does. + # + # ASYMMETRY, deliberate but worth knowing: sensor_dvp applies to insert_detect + # too, this does not. First-boot autodetect therefore probes at the sensor + # table's default clock. i2c is master-clocked so detection still works, but + # it will surprise someone eventually. + mclk="$(fw_printenv -n sensor_mclk 2>/dev/null)" + if [ -n "$mclk" ]; then + case "$mclk" in + 74.25) mclk_sel=0 ;; + 72) mclk_sel=1 ;; + 54) mclk_sel=2 ;; + 24) mclk_sel=3 ;; + 37.125) mclk_sel=4 ;; + 36) mclk_sel=5 ;; + 27) mclk_sel=6 ;; + 12) mclk_sel=7 ;; + *) mclk_sel="" ;; + esac + if [ -z "$mclk_sel" ]; then + logger -s -p daemon.err -t goke "sensor_mclk=$mclk is not a supported MCLK (74.25/72/54/24/37.125/36/27/12); leaving the sensor table default" + elif ! command -v devmem >/dev/null 2>&1; then + logger -s -p daemon.err -t goke "sensor_mclk=$mclk requested but devmem is missing; leaving the sensor table default" + else + mclk_cur="$(devmem 0x120100F0)" + if [ -z "$mclk_cur" ]; then + logger -s -p daemon.err -t goke "sensor_mclk=$mclk: could not read PERI_CRG60; leaving the sensor table default" + elif ! devmem 0x120100F0 32 $(( (mclk_cur & ~0x3C) | (mclk_sel << 2) )); then + logger -s -p daemon.err -t goke "sensor_mclk=$mclk: MCLK write failed; sensor may not produce a usable image" fi - ;; - esac + fi + fi insert_osal insmod gk7205v200_base.ko insmod gk7205v200_sys.ko