Repository navigation
fullhan: FH8626V100 and FH8652/FH8656/FH8658 boards, with FH865x media from the SDK objects - #2714
Conversation
PR Summary by QodoAdd FH8626V100 and FH865x lite boards with relinked FH865x media
AI Description
Diagram
High-Level Assessment
Files changed (114)
|
Code Review by Qodo
1. New boards lack hardware validation
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| # Packages | ||
| BR2_PACKAGE_DROPBEAR_OPENIPC=y | ||
| BR2_PACKAGE_FULLHAN_OSDRV_FH865X=y | ||
| BR2_PACKAGE_IPCTOOL=y |
There was a problem hiding this comment.
1. New boards lack hardware validation 📘 Rule violation ☼ Reliability
The four new defconfigs add firmware images, including a media-loading stack for the FH865x boards, without any real-camera test evidence. The PR explicitly states that no FH8626V100 or FH865x camera was tested, so boot and media behaviour remain unverified on every affected board.
Agent Prompt
## Issue description
The PR adds new camera firmware images and enables the FH865x media package, but the submitted evidence says no FH8626V100 or FH865x camera was tested.
## Fix Focus Areas
- br-ext-chip-fullhan/configs/fh8626v100_lite_defconfig[46-50]
- br-ext-chip-fullhan/configs/fh8652_lite_defconfig[49-52]
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[13-39]
## Recommended Fix
Test the affected boards on real hardware and add board-specific before/after output covering boot, media-driver loading, and streaming or clearly documented hardware limitations with the corresponding evidence.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| insmod $media | ||
| insmod isp.ko | ||
| insmod vpu.ko | ||
| insmod enc.ko | ||
| insmod jpeg.ko |
There was a problem hiding this comment.
2. Module dependencies bypass standard loading 📘 Rule violation ☼ Reliability
The shipped load_fullhan script invokes insmod directly for vmm.ko, xbus_rpc.ko, the selected media module, and each dependent codec module. Direct insertion bypasses the tree's dependency-resolving loader, so a changed or additional module dependency can make media startup fail even when the required module is present in the image.
Agent Prompt
## Issue description
The shipped FH865x loader uses `insmod` for all kernel modules instead of the repository-standard dependency-aware loader.
## Fix Focus Areas
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[17-17]
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[26-30]
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[35-39]
## Recommended Fix
Install the modules in the standard module directory and replace direct `insmod` calls with `modprobe`, passing module parameters in modprobe-compatible form so dependencies are resolved from the module metadata.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| insmod vmm.ko mmz=anonymous,0,0xA2000000,32M anony=1 | ||
|
|
||
| #reset sensor | ||
| set_gpio 13 0 |
There was a problem hiding this comment.
3. Other cameras inherit a sensor reset 📘 Rule violation ≡ Correctness
load_fullhan unconditionally drives GPIO 13 low and high around media loading instead of obtaining the sensor-reset pin from board-specific configuration. Because the FH8652, FH8656 and FH8658 defconfigs select the shared package, that sequence also runs on variants whose sensor wiring may use a different pin or reset method.
Agent Prompt
## Issue description
The shared FH865x `load_fullhan` script hardcodes GPIO 13 for sensor reset, applying that pin assignment to every board that selects the package.
## Fix Focus Areas
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[19-20]
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[32-33]
- br-ext-chip-fullhan/configs/fh8652_lite_defconfig[42-52]
## Recommended Fix
Remove the literal GPIO 13 assignment from the shared loader. Obtain the reset pin or method from applicable per-board pinmux, GPIO, or driver configuration, or from autodetection, and preserve the reset sequence for boards that define that pin.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if [ ! -d "/sys/class/gpio/GPIO$1" ]; then | ||
| echo $1 > /sys/class/gpio/export | ||
| echo out > /sys/class/gpio/GPIO$1/direction | ||
| fi |
There was a problem hiding this comment.
5. Fh865x sensors never receive reset 🐞 Bug ≡ Correctness
set_gpio() exports GPIO 13 but writes its direction and value through /sys/class/gpio/GPIO13 instead of the lowercase gpio13 path created by GPIO sysfs. Each FH865x image installs this loader, so both the sensor-reset and reset-release writes fail during media startup.
Agent Prompt
## Issue description
The FH865x loader uses uppercase GPIO sysfs paths, so its sensor reset writes target nonexistent files.
## Fix Focus Areas
- general/package/fullhan-osdrv-fh865x/files/script/load_fullhan[5-10]
## Recommended Fix
Use `/sys/class/gpio/gpio$1` consistently for the directory check, direction write, and value write; then verify the reset sequence on FH865x hardware.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Qodo review, point by point. Fixes are in 030322a.
Re-tested in qemu-hisilicon |
Thanks for the point-by-point update. Based on the explanation, commit
I can’t independently verify commit |
Both now run on the one fullhan-fh8852v200 4.9.129 kernel: FH8626V100 from OpenIPC/linux#53, FH865x from the ARCH_FH865x port. So the boards take OPENIPC_SOC_FAMILY=fh8852v200 and reuse its published kernel tarball and toolchain; only the kernel config differs. - fh8626v100.generic.config: the FH8852V200 config moved to ARCH_FH8626V100, with the v2 SADC it has. The AJL33PQ0866-only SD0 option stays off; that is one camera's pinmux, not the SoC's. - fh865x.generic.config: ARCH_FH865x, ARMv7, VFPv3/NEON, shared by fh8652, fh8656 and fh8658. Userland stays ARMv6 on the family toolchain, which a Cortex-A7 runs as is. These images carry no media stack yet. FH865x gets one in the next commit. FH8626V100 does not: its media drivers come from an FH8626V100_IPC SDK (V1.0.0 on 3.0.8, V2.0.0 on 4.9), and neither SDK has been published. The FH885xV200 SDK's media_process has no FH8626 chip table, and the factory-image modules are not something this tree takes (best_practices.md 2.4). Depends on the FH865x kernel port reaching fullhan-fh8852v200. Built locally against that branch: fh8652_lite uImage 1435/2048 KB, rootfs 2308/5120 KB; fh8626v100_lite 1434/2048 KB, 2312/5120 KB. In qemu-hisilicon the fh8652, fh8656, fh8658 and fh8626v100 machines boot them to the login prompt with a DHCP lease and a written JFFS2 overlay. Not run on a camera.
… objects
FH865x runs the FH885xV200 media code: the FH885XV200_IPC_V1.1.0 SDK
covers it (its media_process carries chip_fh865x, and its guides are
"FH885xV200 & FH865x"), and an FH8652 camera loads exactly that module
set. The SDK's prebuilt modules are ARMv6 builds for the FH885xV200
kernel, so they cannot load on the Cortex-A7 kernel. But the SDK also
ships them as objects to link against the board's kernel
(media_support/make_drivers). This package carries those objects, the
V1.1.0 release with FP13 over it, and links them against this kernel at
build time. Linked the same way against the FH885xV200 kernel, they
reproduce every module in fullhan-osdrv-fh8852v200 instruction for
instruction.
The libraries, sensors and ARC firmware are fullhan-osdrv-fh8852v200's,
which are that SDK's, so the boards take OPENIPC_SOC_FAMILY=fh8852v200
and majestic.fh8852v200. fh8652, fh8656 and fh8658 now select this
package and majestic.
load_fullhan uses the FH885xV200 memory layout (U-Boot gives FH865x
mem=32M too). The SDK has no FH865x build of the ARC firmware, only the
FH885xV200 one, which is the same code as the FH8652 camera's, a few
months newer. The ARC runs audio and the encoders' softcore, not video,
so when xbus_rpc cannot start it the script loads media_process_no_sc
and keeps video.
The kernel side of the ABI (exported v6 cache entry points, kmalloc
slots for 32-byte lines) is in the FH865x port in OpenIPC/linux.
Built locally: fh8652_lite uImage 1435/2048 KB, rootfs 4700/5120 KB.
In qemu-hisilicon's fh8652 machine it boots to the login prompt with
DHCP and the JFFS2 overlay. xbus_rpc stops at "firmware not ready",
since QEMU has no ARC, and then media_process_no_sc, isp, vpu, enc and
jpeg load. majestic starts ("Majestic Lite for Fullhan (fh8852v200)")
and gets no further, because QEMU has no ISP or VPU. Not run on an
FH865x camera.
- load_fullhan loads with modprobe; depmod indexes the fullhan/ directory. The exception is the fallback with media_process_no_sc, which goes in by path: depmod ties isp, vpu, enc and jpeg to media_process and xbus_rpc, and modprobe would pull those back in. - bgm.ko is no longer built: nothing loads it, as on FH885xV200. - The sensor-reset comments say why GPIO13 is held: it is the SoC's CIS_RSTN in the SDK's and the FH8652 camera's load scripts. In qemu-hisilicon's fh8652 machine (no ARC) modprobe xbus_rpc fails, then media_process_no_sc, isp, vpu, enc and jpeg load. rootfs 4672/5120 KB.
030322a to
effac50
Compare
Problem
New hardware support: Fullhan FH8626V100 and FH8652 / FH8656 / FH8658 (FH865x). There are no OpenIPC images for either. OpenIPC/u-boot already builds
u-boot-fh8626v100-nor.binandu-boot-fh865{2,6,8}-nor.bin(releasefullhan).Both families now run on the one
fullhan-fh8852v2004.9.129 kernel:So all four boards take
OPENIPC_SOC_FAMILY=fh8852v200, with its published kernel tarball, toolchain and majestic. Only the kernel config differs:fh8626v100.generic.config: the FH8852V200 config moved toARCH_FH8626V100, with its v2 SADC. The AJL33PQ0866-only SD0 option stays off; it is one camera's pinmux.fh865x.generic.config:ARCH_FH865x, ARMv7, VFPv3/NEON, shared by the three FH865x boards. The userland stays ARMv6 on the family toolchain, which a Cortex-A7 runs as is.Stacked on #2713; the base is its branch. Commits:
fullhan: add FH8626V100 and FH8652/FH8656/FH8658 lite boards: the boards, without media.fullhan-osdrv-fh865x: FH8652/FH8656/FH8658 media, linked from the SDK objects: FH865x runs the FH885xV200 media code. The FH885XV200_IPC_V1.1.0 SDK covers it: itsmedia_processcarrieschip_fh865x, and its guides are titled "FH885xV200 & FH865x". The SDK's prebuilt modules are ARMv6 builds and do not load on the Cortex-A7 kernel. The SDK also ships them as objects to link against the board's kernel (media_support/make_drivers)._shippedfiles and links them against the board's kernel at build time. Linked the same way against the FH885xV200 kernel, they reproduce all nine modules infullhan-osdrv-fh8852v200instruction for instruction.fullhan-osdrv-fh8852v200's, which are that SDK's.load_fullhanfalls back tomedia_process_no_scif the ARC does not start. The SDK has no FH865x ARC firmware build, and the ARC runs audio and the softcore, not video.FH8626V100 has no media in this PR. Its media drivers come from an
FH8626V100_IPCSDK (V1.0.0 on 3.0.8, V2.0.0 on 4.9) that has not been published. The FH885xV200 SDK'smedia_processhas no FH8626 chip table, and factory-image modules are not something this tree takes (best_practices.md §2.4, as with #2217).Hardware tested on
None. Not run on an FH8626V100 or FH865x camera; draft until it is.
Evidence
Built locally against OpenIPC/linux#158:
The relinked modules are ARMv7 builds with no unresolved symbols:
qemu-hisilicon,
test-fullhan-boot.sh(fh8626v100 needs OpenIPC/qemu-bench#217):Media path on fh8652 under qemu, from a shell. QEMU models no ARC, ISP or VPU, so this is as far as it can go:
Loading
dwc2registers the USB host on both (fh8652 irq 71, fh8626v100 irq 43).ipcinfo -creportsfh8652/fh8658with OpenIPC/ipctool#255 (fh8626v100without it).Scope
general/package/all-patches/linux/(those go to OpenIPC/linux)general/overlay/or in a sharedload_<vendor>script hardcodes a value specific to my boardLD_PRELOAD, and no binaries that cannot be rebuilt from source. Unticked deliberately:fullhan-osdrv-fh865xadds the SDK'smake_driversobject files. They are from the FH885XV200_IPC_V1.1.0 / FP13 vendor SDK release, the same releasefullhan-osdrv-fh8852v200's modules were built from, and they are linked at build time. There is noLD_PRELOAD.