Skip to content

ci: build the hi3536dv100 kernel modules against the board's kernel - #267

Merged
widgetii merged 4 commits into
mainfrom
ci-hi3536dv100
Oct 16, 2026
Merged

widgetii merged 4 commits into
mainfrom
ci-hi3536dv100

Conversation

@widgetii

@widgetii widgetii commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Problem

#266 added hi3536dv100, but CI doesn't build it. Every matrix entry goes through firmware's hisilicon-opensdk (br-hisilicon-opensdk), and that package doesn't know the family yet. So nothing would catch a broken kbuild line, a mirror file that stops compiling, or an OSAL export lost to an edit.

Change

  • A kmod_only: true matrix entry for hi3536dv100 / hi3536dv100_lite. The opensdk step skips kmod_only entries.
  • Build kernel modules (kernel-only family):
    • make BOARD=hi3536dv100_lite br-linux in firmware, then make -C <linux> M=$GITHUB_WORKSPACE/kernel CHIPARCH=hi3536dv100 with Buildroot's cross compiler.
    • The step fails on modpost's undefined!, because for external modules modpost only warns and make still exits 0.
  • Verify hi3536dv100 modules:
    • All 27 open_*.ko exist: the six built from source (osal, sys_config, wdt, hi_ir, hi_cipher, tlv_320aic31) and the 21 closed MPP modules relinked by hi3536dv100: build the closed MPP modules from the SDK's .ko files #268.
    • Each carries the board kernel's vermagic.
    • open_osal.ko exports the same 213 names as the SDK's prebuilt hi_osal.ko, checked against .github/expected/hi3536dv100-osal-exports.txt. Nothing in this tree imports them; the prebuilt MPP modules in firmware do, so a dropped export would otherwise only surface when one fails to load on a camera.

No existing entry changes behaviour.

Testing

Ran both new steps' run: blocks as written, extracted from the YAML, against a local hi3536dv100_lite firmware build. Only br-linux was stubbed, because the kernel was already built:

ok: open_osal.ko (102920 bytes)
ok: open_sys_config.ko (6752 bytes)
ok: open_wdt.ko (10732 bytes)
ok: open_hi_ir.ko (13400 bytes)
ok: open_hi_cipher.ko (22912 bytes)
ok: open_tlv_320aic31.ko (11628 bytes)
ok: open_osal.ko exports 213 symbols

Negative check: I added a call to a symbol nothing exports to a copy of hiir.c. make exited 0, and modpost printed WARNING: "openipc_ci_no_such_symbol" [.../open_hi_ir.ko] undefined!, which the new step turns into a failure.

Update

  • Rebased onto hi3536dv100: build the closed MPP modules from the SDK's .ko files #268, so the verify step now covers all 27 modules.
  • First CI run failed after the kernel built. The step looked for the cross compiler in output/host/bin. Firmware uses per-package directories, and output/host/ is assembled only at the end of a full build, so after br-linux alone the toolchain is in output/per-package/linux/host/bin.
    • My local check had run against a tree that had finished a full build, which is why it missed this.
    • Both steps now use the per-package path.
    • Re-checked in a layout with only per-package/ and build/: both steps pass, with 27 modules found.
  • Qodo's two findings are fixed: pipefail, and the export check by name.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Build and verify hi3536dv100 kernel modules in CI

⚙️ Configuration changes 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a kernel-only CI matrix entry because firmware’s opensdk package does not support hi3536dv100.
Diagram

graph TD
  Matrix["Build matrix"] --> Mode{"Kernel only?"} -->|yes| Kernel["Board kernel"] --> Kbuild["External kbuild"] --> Post["Undefined check"] --> Verify["Module verification"]
  Mode -->|no| SDK["OpenHisilicon build"]
Loading
High-Level Assessment

The kernel-only path is appropriate for CI coverage while firmware’s opensdk package lacks this family. Extending opensdk would be a broader cross-repository change and is unnecessary to validate these modules.

Files changed (1) +56 / -0

Other (1) +56 / -0
build.ymlAdd kernel-only hi3536dv100 build and verification +56/-0

Add kernel-only hi3536dv100 build and verification

• Adds a hi3536dv100_lite matrix entry that builds firmware’s kernel and compiles this repository’s modules against it, while leaving existing opensdk entries on their current path. Fails on modpost unresolved-symbol warnings and verifies all six modules, board-kernel vermagic, and the expected 213 OSAL exports.

.github/workflows/build.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. A failed module build can pass CI ✓ Resolved
Description
The kernel-module make command is piped to tee without pipefail, so the pipeline can return
success when make fails. If that failure does not print undefined!, the following check does not
detect it; the later file checks do not independently check the build command's exit status.
Code

.github/workflows/build.yml[R283-284]

+          make ARCH=arm CROSS_COMPILE="$PWD/$CROSS" -C "$KDIR" \
+            M="$GITHUB_WORKSPACE/kernel" CHIPARCH=${{ matrix.platform }} modules 2>&1 | tee kmod.log
Evidence
The new build runs make through tee, and its only subsequent failure test searches for
undefined!. The verification checks module files and metadata, not the exit status of make.

.github/workflows/build.yml[276-307]

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 `tee` pipeline masks a nonzero exit from the kernel-module build unless the log contains `undefined!`.
## Fix Focus Areas
- .github/workflows/build.yml[283-289]
## Recommended Fix
Enable `set -o pipefail` before the build pipeline, or capture and check `make`'s pipeline status explicitly. Keep the separate `undefined!` check because modpost can emit that warning while returning success.

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


2. Changed exports can pass the OSAL check ✓ Resolved
Description
The OSAL verification compares only the number of __ksymtab_ entries with 213, not their symbol
names. If an expected export is renamed or removed while another export is added, the count still
passes even though firmware's prebuilt modules may import the missing name.
Code

.github/workflows/build.yml[R314-315]

+          EXPORTS=$("$NM" "$GITHUB_WORKSPACE/kernel/open_osal.ko" | grep -c ' __ksymtab_')
+          if [ "$EXPORTS" -ne 213 ]; then
Evidence
The workflow counts matching symbol-table entries and tests only that count. The Hi3536 OSAL sources
declare named exports, so changing one required name while adding another leaves this test's input
unchanged.

.github/workflows/build.yml[309-319]
kernel/osal/hi3536dv100/osal_proc.c[150-150]
kernel/osal/hi3536dv100/osal_schedule.c[10-10]

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

## Issue description
An export count of 213 cannot establish that `open_osal.ko` provides the 213 names required by the SDK modules.
## Fix Focus Areas
- .github/workflows/build.yml[309-319]
## Recommended Fix
Maintain a reference list of the SDK's required OSAL export names and compare it with the names extracted from the built module. Fail when any required name is absent, even if the total count is unchanged.

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


Grey Divider

Tip of the day
💡 Did you know, you can pick a display mode instead of tweaking every Display option by hand

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml Outdated
widgetii added a commit that referenced this pull request Oct 16, 2026
Two Qodo findings on #267:
- make ran through tee without pipefail, so a failed build that never
  printed "undefined!" passed the step;
- the OSAL check compared only the count, so swapping one export for
  another still passed.

Set pipefail, and diff open_osal.ko's export names against
.github/expected/hi3536dv100-osal-exports.txt, which is the SDK's
prebuilt hi_osal.ko list. Locally, a renamed export and a make that fails
both now fail their step.
firmware's hisilicon-opensdk does not build hi3536dv100 yet, so the usual
br-hisilicon-opensdk step cannot cover it. A kmod_only matrix entry
builds firmware's kernel with br-linux, then builds kernel/ against it
with CHIPARCH=hi3536dv100.

modpost only warns about an import that nothing exports when it builds
external modules, and make still exits 0, so the step fails on
"undefined!" itself. The verify step checks that all six open_*.ko exist
and carry the board kernel's vermagic. It also checks that open_osal
still exports the 213 symbols the SDK's hi_osal.ko does: nothing in this
tree imports them, and a dropped one would only show up when a prebuilt
MPP module failed to load on a camera.
Two Qodo findings on #267:
- make ran through tee without pipefail, so a failed build that never
  printed "undefined!" passed the step;
- the OSAL check compared only the count, so swapping one export for
  another still passed.

Set pipefail, and diff open_osal.ko's export names against
.github/expected/hi3536dv100-osal-exports.txt, which is the SDK's
prebuilt hi_osal.ko list. Locally, a renamed export and a make that fails
both now fail their step.
…ll 27 modules

The first run on #267 failed after the kernel had built: the step looked
for the cross compiler in output/host/bin. Firmware builds with per-package
directories, and output/host/ is only assembled at the end of a full
build, so after br-linux alone the toolchain is in
output/per-package/linux/host/bin. The local check had run against a tree
that had finished a full build, which is why it passed.

With #268 in, the verify step lists all 27 modules: the six built from
source, and the 21 closed MPP modules relinked from the SDK's .ko files.
Checked in a layout with only per-package/ and build/, as CI has after
br-linux: both steps pass, with 27 modules found.
The second run on #267 built all 27 modules and found them all, then
failed the export check on order alone. The expected list was sorted in
en_US.UTF-8, while the runner sorts in C.UTF-8, and the two place '_'
differently. Sort both sides with LC_ALL=C. The step now passes under
either locale locally.
@widgetii
widgetii merged commit bb710cb into main Oct 16, 2026
36 checks passed
widgetii added a commit that referenced this pull request Oct 16, 2026
Two Qodo findings on #267:
- make ran through tee without pipefail, so a failed build that never
  printed "undefined!" passed the step;
- the OSAL check compared only the count, so swapping one export for
  another still passed.

Set pipefail, and diff open_osal.ko's export names against
.github/expected/hi3536dv100-osal-exports.txt, which is the SDK's
prebuilt hi_osal.ko list. Locally, a renamed export and a make that fails
both now fail their step.
widgetii added a commit that referenced this pull request Oct 16, 2026
…ll 27 modules

The first run on #267 failed after the kernel had built: the step looked
for the cross compiler in output/host/bin. Firmware builds with per-package
directories, and output/host/ is only assembled at the end of a full
build, so after br-linux alone the toolchain is in
output/per-package/linux/host/bin. The local check had run against a tree
that had finished a full build, which is why it passed.

With #268 in, the verify step lists all 27 modules: the six built from
source, and the 21 closed MPP modules relinked from the SDK's .ko files.
Checked in a layout with only per-package/ and build/, as CI has after
br-linux: both steps pass, with 27 modules found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant