[openipc-ai]

Two S3L fixes that came out of the review of #2763.

CPU target. s3l_lite_defconfig targeted a Cortex-A7 with VFPv4, but the S3L is a Cortex-A9 with NEON and VFPv3 (cpuinfo in OpenIPC/ipctool#266). With the old flags gcc emits sdiv and vfma, which an A9 doesn't have, so ordinary code can take SIGILL.

load_ambarella. The script was 0 bytes, so S70vendor ran it and nothing loaded. It now does the following, in the vendor's order:

1. Loads iav; modprobe pulls in dsp, hw_timer, imgproc and vout.
2. Creates /dev/iav and /dev/ucode, root-only, because iav registers no device class and mdev never sees them.
3. Loads the sensor driver, using sensor from the environment or ipcinfo -s. A stored unknown falls through to detection. A bare name means MIPI, because ipcinfo can't tell the interface; imx290_lvds and imx326_lvds reach the LVDS drivers.
4. Runs load_ucode /lib/firmware if the tool and microcode are present.

This does not produce video on its own. The microcode isn't in this package, and it needs a licensing decision. The stock load_ucode is a glibc armhf binary that won't run on this musl image. s3l_lite_defconfig selects no streamer, and #2763 lists more that video needs: the board's device tree, a 3A process, an exact 30 fps VIN and an ISP configuration. The author plans PRs for those. Without them the drivers load, the DSP stays down, and the script logs that video will not start.

Hardware evidence

• This PR's image has not run on a camera. No S3L board is available to us, and nothing in CI builds s3l_lite.
• The author reports, from their Amcrest IP4M-1026E, that a hand-run bring-up works in the same order, with /proc/devices showing 248 iav and 248 ucode, and modules.dep resolving iav to hw_timer imgproc vout dsp. ov4689_mipi needs no arguments.
• The same board runs a BR2_cortex_a9 NEON build with H.264 after a warm reboot, but that is a glibc armhf toolchain, not this musl softfp one. A cold boot doesn't stream yet.

Tested here

• Buildroot 2024.02.10 resolves the defconfig to -mcpu=cortex-a9 -mfpu=neon. Before, it was cortex-a7 and neon-vfpv4.
• The same C file compiled with each set of flags: the A7 flags emit sdiv and vfma.f64, and the A9 flags call __aeabi_idiv.
• The final script under dash with stubbed commands, 15 cases: load order, node creation and mode, each sensor spelling, stored unknown, unsupported and missing sensors, and no microcode. iav failing ran on the previous commit, and that code didn't change.

Not tested: a full s3l_lite image build, its size before and after, and a run of a binary built with the new flags. No toolchain is published yet (#2765).

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Target S3L’s Cortex-A9 and load Ambarella drivers at boot

🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Target Cortex-A9/NEON instead of A7/VFPv4 to avoid unsupported instructions.
• Load S3L drivers, create device nodes, and select the sensor during vendor boot.
• Load proprietary DSP microcode when available; otherwise log that video cannot start.

Diagram

graph TD
  A["S70vendor"] --> B["load_ambarella"] --> C["modprobe iav"] --> D["Device nodes"] --> E["Sensor lookup"] --> F["Sensor module"] --> G{"Microcode available?"}
  G -->|yes| H["load_ucode"]

Loading

High-Level Assessment

Keep S3L bring-up in the existing vendor-loader path rather than adding another init service. Conditional microcode loading respects the licensing boundary while allowing the packaged drivers to initialize; video remains unavailable without the proprietary files. Verify module dependency indexing and hardware behavior on a built image.

Files changed (2) +103 / -2

Enhancement (1) +99 / -0

load_ambarellaBring up S3L drivers through the vendor boot hook +99/-0

Bring up S3L drivers through the vendor boot hook

• Replaces the empty loader with a script that modprobes iav, creates its character-device nodes, and loads a supported sensor selected from the environment or ipcinfo. It loads DSP microcode only when the external loader and firmware are present, logging when video cannot start without them.

general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella

Bug fix (1) +4 / -2

s3l_lite_defconfigCorrect S3L CPU and FPU target +4/-2

Correct S3L CPU and FPU target

• Replaces Cortex-A7 and NEON-VFPv4 settings with Cortex-A9 and NEON/VFP settings. This targets instructions supported by the S3L rather than emitting A7 instructions that can cause SIGILL.

br-ext-chip-ambarella/configs/s3l_lite_defconfig

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. CPU target change reaches the whole image unverified 📘 Rule violation ☼ Reliability

Description

The s3l_lite_defconfig change replaces BR2_cortex_a7 and BR2_ARM_FPU_NEON_VFPV4 with a
Cortex-A9 NEON/VFP target for every package, while the new load_ambarella script loads drivers and
creates /dev/iav and /dev/ucode at boot. The PR reports only a single-file host compile and a
dash run with stubbed commands, with no full image build, image-size comparison, or camera run of
either change; any toolchain built from the old config would also need rebuilding.

Code

br-ext-chip-ambarella/configs/s3l_lite_defconfig[R3-6]

+BR2_cortex_a9=y
BR2_ARM_EABI=y
-BR2_ARM_FPU_NEON_VFPV4=y
+BR2_ARM_ENABLE_NEON=y
+BR2_ARM_ENABLE_VFP=y

Evidence

The defconfig diff changes the board-wide CPU and FPU selection, and the new boot script loads iav
and the sensor driver and creates device nodes. The PR description lists a full S3L image build and
camera testing under “Not tested”; its reported checks are a compile of one C file and a dash run
with stubbed commands, not before-and-after results from a real board.

Rule 3: Hardware evidence is present and honest
br-ext-chip-ambarella/configs/s3l_lite_defconfig[3-7]
br-ext-chip-ambarella/configs/s3l_lite_defconfig[3-5]
general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella[79-89]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes the image-wide S3L CPU/FPU target and adds a boot-time script that loads drivers and creates device nodes, but reports no full image build, image-size comparison, or run on a camera. The host-side checks do not verify the resulting firmware on hardware.
## Fix Focus Areas
- br-ext-chip-ambarella/configs/s3l_lite_defconfig[3-7]
- general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella[79-89]
## Recommended Fix
Build `make BOARD=s3l_lite` before and after the change, rebuilding any toolchain made from the old config as needed, and record both image sizes. Boot a real S3L camera with the new defconfig and script; include `dmesg` and `logger` output showing iav and the sensor loading and the device nodes appearing, results from `load_ambarella` with and without microcode, and a short run of a binary built with the new flags. Put the results in the PR description. If hardware is unavailable, say so plainly and mark the change as unverified on hardware.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Local users can access camera drivers ✓ Resolved

Description

make_node creates both IAV and microcode character devices with mode 0666, granting every local
user read and write access. When the loader starts the drivers, non-root processes, including
services configured to run as nobody, can open these device interfaces without a permission
boundary.

---

3. Some sensors get the wrong driver ✓ Resolved

Resolving commit: a4f6d05

Resolution reason

Still maps bare imx290/imx326 to MIPI (ipcinfo cannot tell interface), but adds an explicit path:
sensor values imx290_lvds/imx326_lvds map to the unsuffixed drivers, and explicitly configured
*_mipi names are preserved as-is. Users can now select the non-MIPI variant via the sensor
environment variable, which addresses the lack of a loader path to the unsuffixed driver. Also reads
sensor from fw_printenv and retries detection when the stored value is "unknown".

Description

sensor_module maps bare imx290 and imx326 names unconditionally to their MIPI modules. The
package also ships distinct unsuffixed modules, but a camera reporting either bare sensor name has
no path through this loader to select its unsuffixed driver.

---

View review recommended (2)

4. Suffixed sensor names cannot load ✓ Resolved

Description

load_sensor lowercases the stored sensor value but passes an interface-suffixed value unchanged to
a lookup that accepts only bare names. If the environment contains a value such as imx290_mipi,
the loader rejects it even though the corresponding module is installed.

---

5. A stale sensor value blocks detection ✓ Resolved

Description

load_sensor falls back to ipcinfo -s only when the U-Boot sensor value is empty. If an earlier
failed probe persisted sensor=unknown, the non-empty value prevents a new probe and is then
rejected by sensor_module, even after the underlying detection problem is fixed.

---

Tip of the day

💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗