hisilicon-{opensdk,osdrv-hi3516cv300}: ship+load sensor_spi, rmmod by open_*#2030
hisilicon-{opensdk,osdrv-hi3516cv300}: ship+load sensor_spi, rmmod by open_*#2030
Conversation
… open_* User-reported failure on hi3516cv300 / IMX323_i2c after upgrade to OpenIPC 2.6.04.28-lite — `Cannot init sensor`, `Cannot start SDK`. syslog shows the root cause: open_sensor_i2c: Unknown symbol i2c_new_device (err 0) open_sensor_i2c: Unknown symbol ssp_drv_exit (err 0) open_sensor_i2c: Unknown symbol ssp_drv_init (err 0) open_sensor_i2c: Unknown symbol ssp_get_ops (err 0) open_sensor_i2c: Unknown symbol i2c_unregister_device (err 0) The vendor shipped a single combined hi3516cv300_sensor.ko containing both I2C and SPI drivers. openhisilicon split them into open_sensor_i2c and open_sensor_spi (kernel/sensor_*/hi3516cv300/), with the i2c side keeping `extern` references to ssp_drv_init/exit/get_ops in the spi side. PR openhisilicon#59 (a416f9e) added EXPORT_SYMBOL for those three so the split would actually work. But on the firmware side, the cv300 install block in hisilicon-opensdk.mk only ever installed open_sensor_i2c.ko (renamed to hi3516cv300_sensor.ko) — open_sensor_spi.ko was built and discarded. As a result cv300 cameras ship a hi3516cv300_sensor.ko that demands ssp_drv_* with nothing exporting them, and the `i2c_new_device` / `i2c_unregister_device` references are also rejected because resolution stops at the first missing symbol. The user's IMX291 test in PR openhisilicon#59 happened to work because that path also loaded an SPI-side module (Sony sensor); IMX323 over I2C hits the bare hole. Two coordinated changes: 1. `general/package/hisilicon-opensdk/hisilicon-opensdk.mk`: install `open_sensor_spi.ko` as `hi_sensor_spi.ko` in the cv300 block, matching V3A (3519v101) and V3.5 (cv500) naming for the same module. 2. `general/package/hisilicon-osdrv-hi3516cv300/files/script/load_hisilicon`: `insmod hi_sensor_spi.ko` immediately before `insmod hi3516cv300_sensor.ko` (sensor_i2c). Add corresponding `rmmod -w open_sensor_spi` in remove_ko. Plus the same #62 rmmod-by-open_-name rename pattern that already shipped for cv500/cv100/3519v101/cv200 — verified per-module against /proc/modules on the actual cv300 camera (172.17.32.126): hi3516cv300_<base|sys|isp|vpss|venc|vedu|h264e|h265e|jpege|rc|chnl| ive|vgs|wdt|adec|aenc|ai|ao|aio|pwm> → open_<base> hi3516cv300_region → open_rgn hi3516cv300_viu → open_vi hi3516cv300_sensor → open_sensor_i2c hi_acodec → open_acodec hi_osal → open_osal hi_mipi → open_mipi_rx sys_config[.ko] → open_sys_config Strategy B blob-wrapped modules that retain vendor names — verified present in real-hardware lsmod under those names — left unchanged: `hi3516cv300_vou`, `hi_piris`. External / non-openhisilicon also kept: `cma_osal`. Refs OpenIPC/openhisilicon#62, OpenIPC/openhisilicon#66.
Pulls in:
75fac96 sensor_i2c/hi3516cv300: declare MODULE_LICENSE("GPL") (#67)
Required for the cv300 sensor_i2c module to pass kernel symbol
resolution against the GPL-only i2c_new_device / i2c_unregister_device
helpers — see this PR's earlier commit and OpenIPC/openhisilicon#67.
Live diagnosis on cv300 hardware (172.17.32.126, IMX323 i2c) showed
that MODULE_LICENSE alone wasn't enough — the cv300 port of these
two modules was missing module_init/module_exit registration entirely.
The init/exit functions exist as plain C functions (mipi.c lines 2209
and 2281; sensor.c's sensor_dev_init/exit) but were never wired up
through the module_init() / module_exit() macros, so insmod loaded the
.ko but the kernel never invoked their init paths. End result on the
camera:
- /dev/hi_mipi never created (mipi_init's osal_registerdevice never
runs) → user-space "warning: open hi_mipi dev failed"
- sensor_i2c "loads but does nothing" → SDK can't reach sensor
cv200, av100, 3519v101, cv500 all have these wrappers wired (see
kernel/mipi_rx/hi3516cv200/mipi.c:1520, kernel/sensor_i2c/hi3516cv200/
sensor_i2c.c:186, kernel/init/hi3516cv500/mipi_rx_init.c). cv300 was
a half-port from PR #38 / #59 — it shipped the bus drivers as functions
expected to be combined into one bigger module the way the vendor did,
but openhisilicon's split form left them inert.
mipi.c: register the existing mipi_init/mipi_exit through the module
macros and declare GPL license.
sensor.c: declare the three module params load_hisilicon already passes
on insmod (sensor_bus_type, sensor_clk_frequency, sensor_pinmux_mode);
add a no-op module_init/exit pair. The actual bus/sensor wiring stays
driven by user-space via the ISP callback path that already calls
sensor_dev_init() once g_stSensorDev[index]->stCtrlBus is populated —
the vendor's combined hi3516cv300_sensor.ko worked the same way.
Combined with the firmware-side fix in OpenIPC/firmware#2030 (ships
hi_sensor_spi.ko and pre-loads it before hi3516cv300_sensor.ko) and
the MODULE_LICENSE addition already in this PR, this brings cv300
end-to-end with the user's IMX323 i2c camera at 172.17.32.126.
Refs #62, #66.
…param wiring (#67) * ci: fail QEMU smoke test on silent module-load regressions The QEMU boot job's only assertion was `Starting crond: OK` — but load_hisilicon can exit mid-sequence (e.g., after report_error from a duplicate insmod) and still let init reach crond. CV200's QEMU log has been printing `insmod: can't insert 'hi3518e_sys.ko': Operation not permitted` for some time without the job ever turning red. Add three negative grep checks alongside the existing positive one, run on every platform in the matrix: - `insmod: can't insert` — direct module-load failure. - `Error: There's something wrong, please check!` — load_hisilicon's report_error exit (e.g., insmod hitting an already-loaded module due to a stale rmmod). - `Cannot get chip id` — majestic's downstream symptom when sys is inactive. Each failure path tails 50 lines of qemu-output.txt and names which signal hit, so the matrix immediately reveals which platforms are affected without having to download the artifact. Refs #62. * ci: replace insmod check with rmmod check to align with #62 The previous `insmod: can't insert` grep flagged a separate, pre-existing bug on hi3516cv200 / hi3516av100 (the sys.ko blob's init returns -EPERM when run under qemu-hisilicon's incomplete chip emulation — observed on vendor-blob-only runs pre-#2021 too). #62's fix won't clear that. The rmmod-by-vendor-name failure pattern *is* the #62 bug: rmmod: can't unload module 'mmz': No such file or directory rmmod: can't unload module 'hi_sensor_i2c': No such file or directory rmmod: can't unload module 'hi3516cv500_isp': No such file or directory `remove_detect()` rmmods by vendor name; openhisilicon source modules announce as open_* internally; the rmmods fail silently; the script exits at `SENSOR=unknown`; crond starts; old assertion stayed green. Switching to `^rmmod: can't unload` makes this PR a clean TDD harness for #62: hi3516cv100, hi3516cv500, hi3519v101 turn red on the actual bug, will turn green when source-module names are aligned. ev200 and gk7205v200 stay green (already aligned). cv200 / av100 / cv300 don't reach remove_detect in QEMU due to separate earlier failures and remain green here — they need their own issue/test cycle. * sensor_i2c/hi3516cv300: declare MODULE_LICENSE("GPL") User-reported failure on real hi3516cv300 hardware (IMX323 over I2C, OpenIPC 2.6.04.28-lite): open_sensor_i2c: Unknown symbol i2c_new_device (err 0) open_sensor_i2c: Unknown symbol i2c_unregister_device (err 0) The kernel's `i2c_new_device` / `i2c_unregister_device` are `EXPORT_SYMBOL_GPL`. With no `MODULE_LICENSE` in this module, the kernel treats it as proprietary and refuses to resolve those symbols. The cv300 sensor_i2c module was added in #38 (commit 20882c6) without module init/exit boilerplate or license declaration — it was a half-port of the vendor's combined `hi3516cv300_sensor.ko`, where the top-level wrapper would have lived. Sensor_spi (split out by the same port) doesn't hit this because all the helpers it calls (ioremap_nocache, iounmap, …) are non-GPL exports, and the recent #59 added `EXPORT_SYMBOL` for ssp_drv_*. The i2c branch of i2c_drv.c hits two GPL-only symbols that have always required this license tag. Verified live on hi3516cv300 hardware (172.17.32.126) reproducing the user's `Unknown symbol i2c_new_device` failure. Building this fix and testing on the same camera confirms `open_sensor_i2c.ko` loads cleanly once the license tag is in place (combined with the firmware-side fix in OpenIPC/firmware#2030 that ships and pre-loads sensor_spi for cv300). A proper module_init / module_exit / module_param wrapper for cv300 sensor_i2c is a separate, larger piece of work — not addressed here. The .ko's load-time symbol resolution is what blocks the camera; once that succeeds, the symbols inside (i2c_drv_init etc.) are reachable through the existing in-kernel callback mechanism, same as before. Refs #62, #66. * {mipi_rx,sensor_i2c}/hi3516cv300: add module_init/exit + MODULE_LICENSE Live diagnosis on cv300 hardware (172.17.32.126, IMX323 i2c) showed that MODULE_LICENSE alone wasn't enough — the cv300 port of these two modules was missing module_init/module_exit registration entirely. The init/exit functions exist as plain C functions (mipi.c lines 2209 and 2281; sensor.c's sensor_dev_init/exit) but were never wired up through the module_init() / module_exit() macros, so insmod loaded the .ko but the kernel never invoked their init paths. End result on the camera: - /dev/hi_mipi never created (mipi_init's osal_registerdevice never runs) → user-space "warning: open hi_mipi dev failed" - sensor_i2c "loads but does nothing" → SDK can't reach sensor cv200, av100, 3519v101, cv500 all have these wrappers wired (see kernel/mipi_rx/hi3516cv200/mipi.c:1520, kernel/sensor_i2c/hi3516cv200/ sensor_i2c.c:186, kernel/init/hi3516cv500/mipi_rx_init.c). cv300 was a half-port from PR #38 / #59 — it shipped the bus drivers as functions expected to be combined into one bigger module the way the vendor did, but openhisilicon's split form left them inert. mipi.c: register the existing mipi_init/mipi_exit through the module macros and declare GPL license. sensor.c: declare the three module params load_hisilicon already passes on insmod (sensor_bus_type, sensor_clk_frequency, sensor_pinmux_mode); add a no-op module_init/exit pair. The actual bus/sensor wiring stays driven by user-space via the ISP callback path that already calls sensor_dev_init() once g_stSensorDev[index]->stCtrlBus is populated — the vendor's combined hi3516cv300_sensor.ko worked the same way. Combined with the firmware-side fix in OpenIPC/firmware#2030 (ships hi_sensor_spi.ko and pre-loads it before hi3516cv300_sensor.ko) and the MODULE_LICENSE addition already in this PR, this brings cv300 end-to-end with the user's IMX323 i2c camera at 172.17.32.126. Refs #62, #66. * sensor_i2c/hi3516cv300: bus_type / pinmux_mode are strings, not ints UART trace on real cv300 hardware showed: insmod: can't insert 'hi3516cv300_sensor.ko': Invalid argument after the previous commit declared the script's params as int. The load_hisilicon script for cv300 actually passes: sensor_bus_type="i2c" (or "ssp") sensor_pinmux_mode="i2c_mipi" (or "ssp_mipi", "i2c_dc", "ssp_dc") sensor_clk_frequency=37125000 (numeric, stays int) The kernel module_param parser returns -EINVAL on `int` for non-numeric values, which is what we hit. Switch the two string params to charp. The clock frequency stays int. Without this, insmod fails, cv300_sensor.ko never loads, the rest of the boot continues but majestic dies on missing sensor → watchdog times out and the camera reboots in a loop. * Revert "mipi_rx/hi3516cv300: add module_init/exit + MODULE_LICENSE" Live UART trace on real cv300 hardware showed that registering mipi_init through module_init at insmod time panics the kernel — the SSH session resets, watchdog reboots, the camera enters a boot loop. cv200's mipi_init is wired the same way and works because cv200 also installs a platform_driver that's resolved its IRQ mapping via platform_get_irq() before mipi_init runs (cv200/mipi.c :1533). cv500 ships the same pattern out-of-band as a separate init/hi3516cv500/mipi_rx_init.c wrapper. cv300's mipi.c hardcodes mipi_irq=28 at compile time, but inside module_init the irq/register state isn't safely set up — request_irq or the prior mipi_drv_init_reg() ioremap'd register write panics the kernel before any printk reaches /dev/console. The proper fix is to add a dedicated init/hi3516cv300/mipi_rx_init.c that registers a platform_driver, gets the irq number from devicetree, and calls mipi_rx_mod_init() from the probe — exactly what cv500 does. That's separate work; reverting this drop here so the rest of the sensor_i2c fix in #67 lands cleanly without bricking cv300 cameras. The remaining cv300 user impact: open_sensor_i2c.ko now loads (the fix this PR addresses), but /dev/hi_mipi still isn't created, so majestic still fails on "open hi_mipi dev failed". That's a smaller scope than the unbootable-camera regression caused by leaving this module_init in. Refs #62, #66. --------- Co-authored-by: Vasiliy Yakovlev <vixand@openipc.org>
End-to-end cv300 verification on real hardware (10.216.128.52, IMX291 i2c)After merging companion PRs Majestic boot reaches: — versus the original user-reported failure:
Ready to merge?Once #67 + #68 land in openhisilicon main (#67 already merged), bumping this PR's |
f92dc97 to
4849ba2
Compare
Summary
User-reported failure on real
hi3516cv300hardware (IMX323 over I2C, OpenIPC2.6.04.28-lite):syslog isolates the trigger:
Root cause
The vendor shipped a single combined
hi3516cv300_sensor.kocontaining both I2C and SPI drivers. openhisilicon split them intoopen_sensor_i2candopen_sensor_spi(kernel/sensor_*/hi3516cv300/), with the i2c side keepingexternreferences tossp_drv_init/exit/get_opsin the spi side.OpenIPC/openhisilicon#59(a416f9e) addedEXPORT_SYMBOLfor those three so the split would actually work.But on the firmware side, the cv300 install block in
hisilicon-opensdk.mkonly ever installedopen_sensor_i2c.ko(renamed tohi3516cv300_sensor.ko) —open_sensor_spi.kowas built bykernel/hi3516cv300.kbuildline 159 then discarded by the install rule. So cv300 cameras ship ahi3516cv300_sensor.kothat demandsssp_drv_*with nothing exporting them, and resolution stops at the first missing symbol — surfacing as thei2c_new_device/i2c_unregister_device"Unknown symbol" lines too.The IMX291 test in #59 happened to work because that path also loaded an SPI-side module (Sony sensor i.e.
hi_ssp_sony.ko); IMX323 over I2C hits the bare hole.Two coordinated changes
general/package/hisilicon-opensdk/hisilicon-opensdk.mk: installopen_sensor_spi.koashi_sensor_spi.koin the cv300 block. Matches V3A (3519v101) and V3.5 (cv500) naming for the same module.general/package/hisilicon-osdrv-hi3516cv300/files/script/load_hisilicon:insmod hi_sensor_spi.koimmediately beforeinsmod hi3516cv300_sensor.kosossp_drv_*symbols are resolved when sensor_i2c links.rmmod -w open_sensor_spiinremove_ko.#62rmmod-by-open_*-name rename pattern that already shipped for cv500/cv100/3519v101/cv200 — verified per-module against/proc/moduleson the actual cv300 camera (172.17.32.126):.gnu.linkonce.this_module)hi3516cv300_{base,sys,isp,vpss,venc,vedu,h264e,h265e,jpege,rc,chnl,ive,vgs,wdt,adec,aenc,ai,ao,aio,pwm}open_<base>hi3516cv300_regionopen_rgnhi3516cv300_viuopen_vihi3516cv300_sensoropen_sensor_i2chi_acodecopen_acodechi_osalopen_osalhi_mipiopen_mipi_rxsys_config[.ko]open_sys_configStrategy B blob-wrapped modules left alone — verified present in real-hardware lsmod under their vendor names:
hi3516cv300_vou,hi_piris. External / non-openhisilicon also kept:cma_osal.Verification
Real-hardware lsmod on
172.17.32.126(the failing user's camera) confirms the loaded names:— with
open_sensor_i2cnotably missing becauseinsmod hi3516cv300_sensor.kofailed on the unresolved symbols inremove_ko/insert_ko above. After the fix this module joins the set andopen_sensor_spijoins it too.Test plan
releases/latesttarball: hi3516cv300 nightly producesopenipc.hi3516cv300-nor-lite.tgzcontaininghi_sensor_spi.koat/lib/modules/3.18.20/hisilicon/.172.17.32.126, IMX323 i2c): after upgrade,load_hisilicon -iloadsopen_sensor_spi,open_sensor_i2ccleanly, noUnknown symbollines in dmesg,lsmodshows both,majesticreachesSensor driver loadedand serves frames.Refs
OpenIPC/openhisilicon#62— root-cause investigation (umbrella for the rmmod-by-vendor-name issue across V1–V3.5).OpenIPC/openhisilicon#66— cv300Wrong chip-id: hiunknownin QEMU (separate emulation gap; this PR fixes the real-hardware path).OpenIPC/openhisilicon#59(a416f9e) — added theEXPORT_SYMBOLs that this PR finally lets be consumed.#2026(cv500),#2027(cv100),#2028(3519v101),#2029(cv200).