ci: size the cv6xx NOR canvas to the variant it is built for

[eseverson]

Problem

create_hisi laid u-boot at 0 and the combined firmware.bin verbatim at the firmware offset, on a canvas hardcoded to 16384 KiB. Two things were wrong with that, one new and one old.

The new one: a lite variant is flashed to an 8 MiB part, and an image whose tail runs past the end of the chip cannot be written to one. The part size is now the sixth argument, defaulting to 16384 so every existing caller keeps its behaviour, and the cv6xx loop emits both variants at the size each is built for.

The old one: the boot images this job pairs with the firmware carry their partition table in a compiled-in env, and nothing rewrites it later. Decompressed from boot-hi3516cv610-*-nor.bin (a HiSilicon boot table with a gzip'd u-boot behind it):

cv6xx  256k(boot),64k(env),2048k(kernel),5120k(rootfs),7168k@0x50000(firmware),-(rootfs_data)
       bootnor: sf read ${kernaddr}=0x50000 ${kernsize}=0x200000; root=/dev/mtdblock3
dv500  512k(boot),256k(env),6144k(kernel),8192k(rootfs),14336k@0xC0000(firmware),-(rootfs_data)

So u-boot reads the kernel as one fixed slot and mounts root at a fixed offset behind it. firmware.bin is the FIT with the squashfs packed at the next 64 KiB boundary (board post-image.sh), which only agrees with that table when the FIT happens to end exactly at the slot. On every published cv6xx and dv500 image it does not: root=/dev/mtdblockN points 704 KiB before the squashfs on cv6xx ultimate and 512 KiB past it on dv500, and the cv6xx FIT is larger than its 2048 KiB slot, so sf read hands bootm a truncated image. sysupgrade already gets this right (do_update_firmware splits the blob at the FIT boundary and writes each half to its partition); a fresh whole-chip flash of the release .bin did not.

This does what sysupgrade does: split at the FIT boundary (the FIT's total size is the big-endian word at byte 4; the squashfs length is bytes_used in its superblock), put the FIT at the firmware offset and the rootfs at the rootfs partition, and measure each piece against the partition it goes into — u-boot against the boot partition (the env sits behind it, so the firmware offset is the wrong bound), the FIT against the kernel slot, the rootfs against the rootfs slot, the firmware partition against the part. Each failure prints a ::error:: and fails the step rather than uploading something that boots a truncated kernel or overwrites rootfs_data. The exact-size check on the assembled canvas stays as the backstop, and the scratch variables are local now.

The layout table is a case on $soc; adding a family means adding its u-boot's env line there. Consequence worth stating: with this in place, cv6xx ultimate on master fails the FIT check (2709 KiB in 2048) and hi3519dv500 ultimate fails the rootfs check (8832 KiB in 8192). Both are the same table disagreement the published images already carry, now reported instead of published. #2447 fits both cv6xx slots.

A refusal does not take the other SoCs' images with it. The step runs under Actions' default bash -e, and the Sigmastar, Allwinner and Ingenic images are assembled before the HiSilicon loops, so a bare return 1 there would have ended the step with their .bin files sitting in target/ and the upload never reached. The HiSilicon calls collect their failures (|| hisi_failed=1), the step exits 1 after the last one so the run is red, and Upload runs on always() so what did assemble ships. Exercised in the harness: a refused ultimate followed by a good lite leaves the lite .bin in target/ and the step red.

Hardware tested on

hi3516cv6xx (chip reports hi3516cv613, DDR3 128 MB), SIMICAM A314D / Vatilon H80, 8 MiB NOR.

This produces the bytes a user writes to a chip, so it was tested as that: the function from this branch, run against the #2447 lite build and the published boot-hi3516cv610-20s-nor.bin, produced openipc-hi3516cv610-ddr3-128m-nor-lite.bin, and that image was written to the camera's NOR in place (u-boot, erased env, FIT, rootfs, at the offsets above). It booted on the OpenIPC u-boot with its default env, the kernel reported the table from that env, /init found and mounted rootfs_data, and after the usual fw_setenv osmem 48M it streams. u-boot, FIT and squashfs read back byte-identical from their partitions. One board addition: rootfs_data was pre-staged with the device's overlay (an out-of-tree sensor driver, since this sensor has no driver in the tree) instead of left for /init to format; the rest of the image is the assembled one.

Evidence

$ python3 .github/scripts/lint-workflow-shell.py
all run blocks parse clean
$ python3 .github/scripts/lint-workflow-shell.py --self-test
self-test passed

# the function from this branch, against the #2447 lite build:
Created: target/openipc-hi3516cv610-ddr3-128m-nor-lite.bin (FIT 2048 KiB in 2048, rootfs 5076 KiB in 5120)
   u-boot at 0 ....................... byte-identical to boot-hi3516cv610-20s-nor.bin
   256K-320K (env) ................... erased
   FIT at 320K ....................... byte-identical to fitImage (2063177 B)
   rootfs at 2368K ................... byte-identical to rootfs.squashfs (5197824 B)
   everything after ................... erased

# and the three ways it refuses:
   rootfs superblock claiming 5300 KiB  -> ::error::hi3516cv6xx_lite: the rootfs is 5300 KiB, the rootfs partition is 5120 KiB
   FIT header claiming 2200 KiB         -> ::error::hi3516cv6xx_lite: the FIT is 2240 KiB, the kernel partition is 2048 KiB
   blob that is not a FIT               -> ::error::hi3516cv6xx_lite: firmware.bin does not start with a FIT image

On the camera, running that image:

# cat /proc/mtd
mtd0: 00040000 00010000 "u-boot"
mtd1: 00010000 00010000 "env"
mtd2: 00200000 00010000 "kernel"
mtd3: 00500000 00010000 "rootfs"
mtd4: 00700000 00010000 "firmware"
mtd5: 000b0000 00010000 "rootfs_data"

# cat /proc/cmdline
mem=48M console=ttyAMA0,115200 panic=20 root=/dev/mtdblock3 rootfstype=squashfs init=/init mtdparts=sfc:256k(u-boot),64k(env),2048k(kernel),5120k(rootfs),7168k@0x50000(firmware),-(rootfs_data)

dd if=/dev/mtd0 | head -c 241664  | md5sum  -> 37c9dd1798718e64371875a98d65ce4a   = boot-hi3516cv610-20s-nor.bin
dd if=/dev/mtd2 | head -c 2063177 | md5sum  -> ee9b6cca762d36abb82fa8bc86229a8a   = fitImage
dd if=/dev/mtd3 | head -c 5197824 | md5sum  -> 7732a4955365664028859353f2d68a29   = rootfs.squashfs

# netstat -ltn | awk 'NR>2{print $4}' | sort -u
0.0.0.0:22
0.0.0.0:554
0.0.0.0:80
# head -4 /proc/umap/venc
      id   width  height   type  by_frame   sequence
       0    2560    1440   H264         y        773
       1     640     360   H264         y        387

Scope

• No kernel patches, no model-specific files, no bring-up tooling
• Nothing in general/overlay/ or a shared load script
• No package source or version changes
• No LD_PRELOAD, no unbuildable binaries
• Existing callers unchanged by the new default

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

PR Summary by Qodo

Size CV6xx NOR release images by firmware variant

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Build CV6xx lite and ultimate images on 8 MiB and 16 MiB canvases.
• Reject firmware or bootloader payloads exceeding their assigned NOR regions.
• Preserve 16 MiB defaults for existing HiSilicon callers.

Diagram

graph TD
  A["CV6xx Loop"] --> B{"Variant?"} -->|lite| C["8 MiB Limit"] --> E["create_hisi"] --> F["Capacity Checks"] --> G["NOR Release"]
  B -->|ultimate| D["16 MiB Limit"] --> E

Loading

High-Level Assessment

The explicit variant-to-capacity mapping is appropriate because the workflow already controls the variant loop and the values directly represent supported hardware. Deriving capacity from artifact metadata could reduce duplication, but no such manifest is available and introducing one would be disproportionate; the optional argument also preserves existing Hi3519DV500 behavior.

Files changed (1) +41 / -8

Bug fix (1) +41 / -8

image.ymlBuild and validate variant-sized CV6xx NOR images +41/-8

Build and validate variant-sized CV6xx NOR images

• Extends 'create_hisi' with an optional NOR-size argument, validates bootloader and firmware payload boundaries, and confirms the assembled artifact exactly matches the target part. The CV6xx release loop now emits lite images on 8 MiB canvases and ultimate images on 16 MiB canvases while retaining the 16 MiB default for existing callers.

.github/workflows/image.yml

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

Code Review by Qodo

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

1. A size-check comment repeats the code ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The comment # and the canvas is still exactly the size of the part merely describes the stat
result and comparison immediately below it instead of recording why the post-write assertion exists.
A later maintainer gains no context beyond lines 177–180 themselves, adding noise around the
exact-size guard.

Code

.github/workflows/image.yml[176]

+              # and the canvas is still exactly the size of the part

Evidence

Compliance rule 33 requires new comments to explain rationale rather than restating the next
operation. The added comment at line 176 only states the exact-size condition enforced immediately
afterward.

CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code: CLAUDE.md: Write Comments That Explain Rationale Rather Than Restating Code
.github/workflows/image.yml[176-180]

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 comment above the final canvas-size assertion narrates the code rather than explaining the reason for this defensive check.
## Fix Focus Areas
- .github/workflows/image.yml[176-180]
## Recommended Fix
Remove the redundant comment or replace it with a concise rationale explaining that the assertion prevents an unexpectedly extended image from being uploaded as a flashable release.

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

---

2. Lite release images are never created 🐞 Bug ≡ Correctness

Description

The new lite iteration requests hi3516cv6xx_lite from the manifest, but the build matrix and
defconfig tree only define hi3516cv6xx_ultimate. On every scheduled or dispatched assembly,
firmware_url therefore returns empty and all four lite calls return before creating their release
files.

Code

.github/workflows/image.yml[178]

+            for variant in lite ultimate; do

Evidence

The workflow looks up an artifact by the exact ${soc}_${variant} key and skips when none exists,
while the matrix and available defconfig define only the ultimate cv6xx platform.

.github/workflows/image.yml[69-82]
.github/workflows/image.yml[137-140]
.github/scripts/ci-matrix.py[60-63]
br-ext-chip-hisilicon/configs/hi3516cv6xx_ultimate_defconfig[1-14]

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 image workflow now assembles cv6xx lite releases, but the repository never builds or publishes a `hi3516cv6xx_lite` artifact for it to consume.
## Fix Focus Areas
- .github/workflows/image.yml[176-183]
- .github/scripts/ci-matrix.py[60-63]
- br-ext-chip-hisilicon/configs/hi3516cv6xx_ultimate_defconfig[1-14]
## Recommended Fix
Add a cv6xx lite defconfig configured for the 8 MiB flash variant and register `hi3516cv6xx_lite` in the CI build matrix so the manifest contains the artifact requested by `firmware_url`. Keep the new loop only after that artifact is produced and published.

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

---

3. Oversized firmware escapes the 8 MiB image ✓ Resolved 🐞 Bug ≡ Correctness

Description

The variable-size initialization creates an 8192 KiB file, but the subsequent unbounded dd writes
can extend that file past its initial length. This occurs whenever the firmware blob exceeds the
7872 KiB available after the 320 KiB offset, and the cv6xx image-generation script imposes no
corresponding maximum.

Code

.github/workflows/image.yml[R149-151]

+              # $nor is the part size in KiB -- a lite variant is flashed to an
+              # 8 MiB part, so a 16 MiB canvas would be unwritable past its end.
+              dd if=/dev/zero bs=1K count="$nor" status=none | tr '\000' '\377' > "$release"

Evidence

The new canvas size is only used to prefill the output; both later writes use conv=notrunc without
a bound check. The firmware producer concatenates the FIT image and root filesystem and rounds the
result, but never verifies that it fits the 7872 KiB region available on an 8 MiB part.

.github/workflows/image.yml[149-153]
.github/workflows/image.yml[176-183]
br-ext-chip-hisilicon/board/hi3516cv6xx/post-image.sh[13-22]

Agent prompt

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

## Issue description
Creating an 8192 KiB canvas does not cap later writes, so an oversized firmware blob extends the release beyond the physical NOR size instead of failing assembly.
## Fix Focus Areas
- .github/workflows/image.yml[149-153]
- br-ext-chip-hisilicon/board/hi3516cv6xx/post-image.sh[13-22]
## Recommended Fix
Before writing, compare each input size and offset against `nor * 1024`, failing the workflow when a write would exceed the canvas. After assembly, assert that the release file remains exactly `nor * 1024` bytes.

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

---

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[eseverson]

The lite variant is being added in this PR: #2447

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

Code review by qodo was updated up to the latest commit b002127

[openipc-ai]

The sixth argument and the exact-size backstop are the right shape, and the hardcoded 16384 clearly had to go. But the bound the new check enforces is not the one that decides whether the image boots, and I think that has to be settled before this publishes an 8 MiB artifact.

The u-boot this job pairs with the firmware is a HiSilicon boot table followed by a gzip member (offset 40128 for cv610, 84768 for hi3519dv500). Decompressed, the compiled-in default env is:

# boot-hi3516cv610-*-nor.bin
kernaddr=0x50000   kernsize=0x200000   rootaddr=0x250000   rootsize=0x500000
mtdparts=sfc:256k(boot),64k(env),2048k(kernel),5120k(rootfs),7168k@0x50000(firmware),-(rootfs_data)
bootnor=... sf read ${baseaddr} ${kernaddr} ${kernsize}; bootm ${baseaddr}
bootargs=... root=/dev/mtdblock3 ...

# boot-hi3519dv500-*-nor.bin
kernaddr=0xC0000   kernsize=0x600000
mtdparts=512k(boot),256k(env),6144k(kernel),8192k(rootfs),14336k@0xC0000(firmware),-(rootfs_data)

Nothing rewrites that afterwards -- set_allocator only edits the mmz half of bootargs, and no overlay script touches mtdparts or kernsize -- so this is the table the camera boots with.

Measured against what this job currently assembles, and what it would assemble with #2447:

	FIT	kernel part	rootfs at	rootfs part at	blob	firmware part
cv6xx ultimate (published)	2709 KiB	2048 KiB ❌	+2752 KiB	+2048 KiB ❌	10496 KiB	7168 KiB ❌
cv6xx lite (#2447)	2496 KiB	2048 KiB ❌	+2496 KiB	+2048 KiB ❌	7680 KiB	7168 KiB ❌
hi3519dv500 ultimate (published)	5586 KiB	6144 KiB ✅	+5632 KiB	+6144 KiB ❌	14464 KiB	14336 KiB ❌

Two separate disagreements, both pre-existing and neither introduced here. post-image.sh packs the rootfs at the first 64 KiB boundary after the FIT; u-boot expects it at a fixed partition offset -- so root=/dev/mtdblock3 points 704 KiB before the squashfs on cv6xx ultimate and 512 KiB past it on hi3519dv500. And on cv6xx the FIT is larger than the 2048 KiB kernel partition, so sf read ${kernaddr} ${kernsize} hands bootm a truncated image.

general/overlay/usr/sbin/sysupgrade gets both right where this assembler does not: do_update_firmware splits the blob at the FIT boundary and writes each half to its own partition, and check_combined_fits dies rather than write when either half is oversized. So an over-large combined image fails safe over the air -- it only mis-lands on a fresh whole-chip flash of exactly the .bin this job publishes, which is the artifact #2447 is asking for on parts nobody can reach with a programmer twice.

I am requesting changes on the bound, not on the parameterisation. Everything else here should stay. If splitting the blob the way sysupgrade does is too much for one PR, checking fw_kib against the firmware partition size (7168 / 14336) rather than nor - off at least stops this publishing something that overruns into rootfs_data.

One process note: "Hardware tested on: not applicable -- CI machinery that assembles a release artifact" is a stretch against CLAUDE.md, which exempts "CI machinery that only selects, lints or tests". This produces the bytes a user writes to a chip. lint-workflow-shell.py and its self-test do pass here, for what that covers.

[eseverson]

Rewritten in a8ff89e0 around the bound you identified, and then some: create_hisi now does what sysupgrade's do_update_firmware does. It splits the blob at the FIT boundary (total size from the FDT header, squashfs length from bytes_used), puts the FIT at the firmware offset and the rootfs at the rootfs partition, and checks u-boot against the boot partition, the FIT against the kernel slot, the rootfs against the rootfs slot and the firmware partition against the part — each a ::error:: and a failed step. The layout table is a case on $soc carrying the two env lines you decompressed. Scratch variables are local.

Consequence, stated in the description: cv6xx ultimate on master now fails the FIT check and hi3519dv500 ultimate the rootfs check, which is the same disagreement the published images already carry, reported instead of published.

Hardware section is no longer "not applicable": the function from this branch, run against the #2447 build and the published 20s u-boot, produced the lite image, and that image was written to the H80's NOR in place. It boots on the OpenIPC u-boot with its default env and streams; u-boot, FIT and squashfs read back byte-identical from their partitions. The three refusal paths were exercised with doctored blobs; output in the description.

[openipc-ai]

The rewrite is the right one, and it is more than I asked for -- splitting at the FIT boundary and placing each half where the env looks is exactly what do_update_firmware does, and I checked the arithmetic lands where it should: 320 + 2048 = 2368 KiB = 0x250000 on cv6xx, 768 + 6144 = 6912 KiB on dv500, both matching the tables. bytes_used really is the little-endian u64 at superblock offset 40, the FDT total size really is the big-endian word at byte 4, and the four bounds are each the partition rather than the chip. My inline points are all addressed; the local line covers every scratch variable.

One thing I got wrong by omission in the first review, and it now blocks: the consequence is not "reported instead of published", it is "nothing is published".

• the Prepare step declares no shell:, so it runs under Actions' default bash -e
• create_hisi returns 1 as a plain command inside a for loop, so set -e ends the step
• Upload carries no if:, so it defaults to success() and is skipped

And the ordering decides the blast radius. Sigmastar and Allwinner assemble at line 238 and Ingenic at 243, both before the cv6xx loop at 257. Their .bin files are sitting in target/ when the first cv6xx ultimate call fails; the dv500 loop at 270 never runs at all; and the upload at 275 never fires. So on the first scheduled assembly after this merges, the image release gets no new artifacts for any SoC -- not two withheld.

Refusing to publish a broken image is right. Ending every other SoC's publication to do it is not, particularly when the two targets being refused are already broken on master, so this is not a regression being caught but a standing disagreement being surfaced.

Report and skip would do it: ::error:: on the target, return 0, and let the rest of the run finish. If the run should still go red -- and it probably should, or nobody will look -- collect the failures instead of returning out of the loop, exit "$rc" at the end of the step, and give Upload an if: always() so what did assemble still ships.

Nothing else outstanding from me. lint-workflow-shell.py and its self-test pass on a8ff89e0.

[eseverson]

Right, and thank you for tracing the blast radius rather than the symptom. cd896b7e: the HiSilicon calls collect (|| hisi_failed=1), the step exits 1 after the last one so the run stays red, and Upload is if: always() so the Sigmastar, Allwinner and Ingenic images that assembled before the refusal still ship. create_hisi also clears output/ at entry now, so a refused call cannot leave its blob for the next one. Exercised in the harness with a refused ultimate followed by a good lite: the lite .bin is in target/, the error is printed, the step ends non-zero. Linter and its self-test pass.

[openipc-ai]

The collect-and-continue shape is right, and rm -rf output at function entry is a catch I missed -- the old code only cleaned up on the success path, so a refused call left its extracted blob for the next one. Every refusal now gets its turn, the step still ends red, and what assembled still ships.

One thing left, and it is the same class of harm the PR exists to prevent.

always() does not mean "on failure". It means on failure and on cancellation. create_hisi builds $release in four stages -- erased canvas, u-boot, FIT, rootfs -- so a run cancelled between them leaves a partial .bin in target/: an 0xff canvas carrying a bootloader and no firmware. With if: always() the upload runs anyway and that lands in the image release as a flashable asset, next to the good ones and indistinguishable from them until someone writes it to a chip.

if: ${{ !cancelled() }} keeps everything you wanted from always() -- a refused image still fails the step, the images that did assemble still publish -- and lets a cancellation actually cancel. The ${{ }} is not optional there: if: !cancelled() on its own is invalid YAML, because a leading ! is a tag indicator.

Not a request, just so it is known rather than rediscovered: a wget failure still takes the return 0 path, so a download that fails is skipped silently and does not set hisi_failed. That predates this PR and I would leave it alone here.

Housekeeping while I was in it: I rebased this onto master as fdb2f321, since #2447 landed and the required checks are strict. Clean rebase, patch byte-identical, authorship unchanged. Its workflow runs were sitting at action_required -- fork PR -- so I approved them; this is the first CI this branch has had.

[openipc-ai]

Approving, with one disclosure: I pushed a commit onto this branch rather than hand it back for a single word.

fa47769a changes the upload condition to if: ${{ !cancelled() }} and moves the reasoning into the comment above the step. Nothing else in it. Your commit 85d943ed is untouched and still carries the work; I added mine on top so the change is attributable and one revert away if you disagree with it.

Why I did it rather than ask: master had moved again (#2454), the required checks are strict, so this needed another rebase and another full CI cycle no matter what. Folding the one-word fix into that cycle cost nothing and meant not merging a defect I had just written a review about. If you would rather it were yours, squash it into 85d943ed and force-push -- I will re-approve.

Everything else here is settled and I resolved the threads with what each turned into. For the record, what this PR ends up doing: it splits the combined blob at the FIT boundary, puts each half where the cv610 and dv500 envs actually look, refuses any image whose halves do not fit their slots, and keeps publishing the ones that do. cv6xx ultimate and hi3519dv500 will start failing that refusal on the first run -- both are genuinely broken against their own partition tables today, and the numbers are in my earlier review.

CI on fa47769a -- its fork runs needed approving again, so I did that too.