[openipc-ai]

Problem

load_ambarella sends load_ucode's output to /dev/null, so a boot log doesn't say which microcode the DSP got. On the IP4M-1026E, reading it back took a separate tool (#2782, which suggested this change), and #2767 lists seven builds of this microcode in the field.

Change

On success, load_ambarella logs the ucode version line through log_vendor. The per-file lines stay out of the log. A failure is handled as before: load_ucode's own error reaches stderr, and load_ucode failed is logged.

Hardware tested on

Ambarella S3L, Amcrest IP4M-1026E, run by @DeathCamel58, not by me: output. The console shows ambarella: ucode version 2018/10/28 284712.282649. That run was a warm netboot of the first version of this PR. The later fix below only changes the case where no version line comes back, and it hasn't run on a camera.

Evidence

QEMU, on qemu-bench's s3l machine, with s3l_lite's kernel, the real dsp.ko and the shipped microcode. QEMU doesn't model the DSP.

Before (the image built for #2782; this is the last 8 lines of the script's output, and rc is tail's):

=== load_ambarella
[    0.744780] dsp: module license 'Proprietary' taints kernel.
[    0.744994] Disabling lock debugging due to kernel taint
[    0.767293] Vout notice: vout0:0 probed!
[    0.767650] Vout notice: vout1:1 probed!
rc=0

After:

=== load_ambarella
ambarella: ucode version 2018/10/28 284712.282649
=== kernel errors
0

After, with orcme.bin removed:

=== load_ambarella
load_ucode: /lib/firmware/orcme.bin: No such file or directory
ambarella: load_ucode failed
=== kernel errors
0

After, with a stand-in load_ucode that exits 0 and prints no version line, which is what a failed version ioctl does. Without the fix this logged a blank line (Qodo's finding):

=== load_ambarella
ambarella: Microcode loaded, version unknown
=== kernel errors
0

STRICT=1 test_shell_parse.sh and STRICT=1 test_strip_shell_comments.sh pass. ci-matrix.py --stdin narrows this to s3l_lite.

Known limit

The line can scroll out of logread: on the IP4M-1026E the driver's kern.debug dumps filled the 64 KB syslog ring within 40 seconds of boot. The console keeps it. That's outside this PR.

Scope

• No kernel patches under general/package/all-patches/linux/ (those go to OpenIPC/linux)
• No files specific to a single retail camera model (those go to OpenIPC/builder)
• No probing or bring-up tooling (that goes to OpenIPC/ipctool)
• Nothing under general/overlay/ or in a shared load_<vendor> script hardcodes a value specific to my board
• Package sources come from an OpenIPC repository, and any version bump keeps at least the specificity of the pin it replaces (a new package should pin a full 40-character SHA)
• No LD_PRELOAD, and no binaries that cannot be rebuilt from source
• New code is selected by a defconfig, so CI actually builds it

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

PR Summary by Qodo

Log the Ambarella S3L microcode version at boot

✨ Enhancement 🕐 Less than 10 minutes

AI Description

• Record the loaded DSP microcode version in the boot log to help diagnose field builds.
• Keep per-file output suppressed and preserve existing failure reporting.

Diagram

graph TD
  A["load_microcode"] --> B["load_ucode"] --> C{"Success?"}
  C -->|yes| D["Select version"] --> E["log_vendor"]
  C -->|no| F["Failure message"] --> E

Loading

High-Level Assessment

Capturing the existing loader output and selecting its version line keeps the change in the boot script without altering microcode loading. Changing load_ucode's interface or reading the version separately would add scope without a clear benefit.

Files changed (1) +7 / -1

Enhancement (1) +7 / -1

load_ambarellaLog the DSP microcode version after a successful load +7/-1

Log the DSP microcode version after a successful load

• Captures load_ucode's stdout and sends only the ucode version line through log_vendor when loading succeeds. On failure, it retains the load_ucode failed log message; the loader's stderr remains visible.

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

[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. Boot log change has no hardware test 📘 Rule violation ◔ Observability

Description

The PR changes what load_ambarella writes to the boot log on every camera that runs it, by
replacing the /dev/null redirect with a log_vendor call that records the ucode version line
and a failure message. The PR description says the change was "Not run on a camera" and that the
evidence is QEMU output, which does not model the DSP, so the behaviour change has no board output
behind it. The author asks the IP4M-1026E owner to check the version line after merge, which leaves
the change unverified on the hardware it targets.

Code

general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella[R138-141]

+	# The version is the first thing to compare when the DSP misbehaves: cameras
+	# in the field run many different builds of this microcode (#2767).
+	if output=$(load_ucode "${ucode_dir}"); then
+		log_vendor "$(echo "${output}" | grep '^ucode version')"

Evidence

Failure Criteria of rule 3 are met: the description states the change was not tested on hardware,
and the diff alters what the firmware logs on a camera during boot. The rule's Success Criteria asks
for before/after output from a real camera, and the PR provides only QEMU output, which the
description itself says does not model the DSP.

Rule 3: Hardware evidence is present and honest
general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella[138-141]

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 boot log written by `load_ambarella` on Ambarella S3L cameras, but its only evidence is QEMU output, and the description says it was not run on a camera. Rule 3 requires before/after output from a real camera when a diff changes firmware behaviour.
## Fix Focus Areas
- general/package/legacy/ambarella-osdrv-s3l/files/script/load_ambarella[138-141]
## Recommended Fix
Boot an S3L camera (for example the IP4M-1026E) with this change, capture the `load_ambarella` boot log lines for both the success path (`ambarella: ucode version ...`) and the failure path (`ambarella: load_ucode failed`), and paste them into the PR description under the evidence heading. If no camera is available, state that plainly in the description and label the change as untested on hardware.

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

---

2. Version lookup failures leave a blank log ✓ Resolved

Description

load_microcode passes an empty string to log_vendor when the captured output has no `ucode
version line. load_ucode` treats a failed version ioctl as a successful load without printing that
line, so affected cameras boot with neither a version nor an explanation of why it is absent.

---

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[DeathCamel58]

Booted it on the IP4M-1026E: the version line is there.

How it was run. Netbooted, so the flash wasn't touched. The image is master d86e9fc1 with the SDK microcode from #2782 in /lib/firmware and this PR's load_ambarella (comment-stripped by the build, the same file otherwise). It's my bench profile: initramfs, the board DTB, and my Divinus S3L backend as the streamer. This was a warm reboot, but the script's path is the same from power-on.

Serial console during boot:

Loading vendor modules...
ambarella: ucode version 2018/10/28 284712.282649

After that, the DSP entered modes 0 and 1 and stream 0 came up (2688×1512 H.264), as on #2782.

One thing about where the line ends up. On this image it isn't in logread for long. syslogd keeps a 64 KB ring (SYSLOG_SIZE=64), and once a streamer starts, the S3L driver's per-command debug dumps (dsp_cmd: ... with every field, at kern.debug) fill it. Forty seconds after boot, logread started in the middle of those dumps from around the 2.5 s mark, and the ambarella: line, logged just before them, wasn't there. So the console has it, but a log pulled later may not. That's outside this PR, but it's worth knowing before someone asks a user for the version from logread.