[dimerr]

One recovery rule for every branch (common/openipc.c is to be byte for byte the same on openipc-hisilicon, openipc-xm and openipc-gk710x):

• script.txt still runs first;
• otherwise every *.img in the card's or stick's root, in directory order, that is a valid uImage (magic, header CRC, data CRC) whose load address + data size fit the flash, on an erase boundary, and fits in RAM, has its payload written at the load address. The wrapper header is never written. U-Boot at 0 included;
• an image already on the flash is skipped, then the normal boot continues (a card left in the slot rewrites nothing);
• the SPI NAND path (rootfs.ubifs.<soc>.img) is unchanged;
• the split firmware.bin path and the per-name lookups are gone.

Tested: under qemu-hisilicon on openipc-gk710x (gk7102, gk7102s) and openipc-xm (xm530), with this exact file: images written and booted; bad data CRC, non-uImage, past-the-end and off-boundary images skipped with a message; a second boot finds everything already on the flash.

Not tested: a HiSilicon build or boot. This host's gcc 15 cannot build the vendor tree, so this PR's CI is the first build of it on HiSilicon SoCs.

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

PR Summary by Qodo

Recover uImage payloads from SD or USB at their flash addresses

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Scan root-level *.img files instead of relying on firmware and SoC-specific filenames.
• Validate each uImage and write only its payload at its erase-aligned load address.
• Skip unchanged flash contents; retain script.txt precedence and the SPI NAND recovery path.

Diagram

graph TD
  M["SD or USB"] --> S{"script.txt present?"} -->|no| I["Scan root images"] --> V{"Valid uImage?"} -->|yes| C{"Already flashed?"} -->|no| F["Write SPI NOR"] --> I
  S -->|yes| R["Run script"] --> B["Continue boot"]
  V -->|no| I
  C -->|yes| I
  I -->|done| B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Explicit recovery manifest

• ➕ Makes permitted flash addresses and write order explicit.
• ➖ Requires a new card format and tooling; loses the simple self-describing image workflow.

Recommendation: The uImage load-address approach fits the goal of recovering varied payloads without filename rules. A manifest would offer tighter control over what can overwrite flash, but is a larger compatibility and tooling change; reviewers should assess whether recovery media are trusted.

Files changed (1) +90 / -150

Enhancement (1) +90 / -150

openipc.cReplace SPI NOR filename recovery with validated image discovery +90/-150

Replace SPI NOR filename recovery with validated image discovery

• Collects root-level *.img files in directory order, validates their uImage headers, payload CRCs, and destination bounds, then writes payloads—not wrappers—to SPI NOR at their load addresses. It skips matching flash contents and removes the split-firmware and named kernel/rootfs lookups. script.txt retains precedence, and SPI NAND rootfs recovery remains on its existing path.

common/openipc.c

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

Code Review by Qodo

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

1. Recovered kernels can remain unbootable 🐞 Bug ≡ Correctness

Description

recovery_write_img reads past the uImage header and writes only its payload, so a card containing
a single-wrapped kernel leaves no uImage header at the flash destination. The subsequent flash scan
recognizes kernels by uImage or FIT magic, and bootm loads from that same destination; on later
boots the payload comparison skips the image as already written.

Code

common/openipc.c[R456-457]

+  if (rfs_read(name, CONFIG_SYS_LOAD_ADDR, sizeof(hdr), len, &actread) ||
+      actread != len) {

Evidence

The new read starts after the header and the flash write uses only that buffer. The scanner requires
uImage or FIT magic at the kernel offset, while the configured boot command reads that offset and
invokes bootm.

common/openipc.c[421-473]
common/openipc.c[81-93]
common/openipc.c[166-178]
include/configs/openipc-common.h[183-184]
include/configs/openipc-common.h[217-228]

Agent prompt

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

## Issue description
A single-wrapped kernel recovery image is written without the header required by the flash scan and boot path.
## Fix Focus Areas
- common/openipc.c[456-473]
- common/openipc.c[81-93]
- include/configs/openipc-common.h[210-228]
## Recommended Fix
Keep the payload-only rule for images whose payload is already a bootable flash image, but handle single-wrapped kernels so the bytes written at the kernel offset retain the uImage header required by scanning and `bootm`. Add a recovery test that scans and boots the resulting flash contents.

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

---

2. FIT kernel cards no longer recover 🐞 Bug ≡ Correctness

Description

recovery_write_img rejects every file without uImage magic, and recovery_try no longer falls
back to the per-name FIT lookup. A card containing the previously accepted fitImage..img therefore
cannot restore its kernel, although the flash scanner and normal update command still support FIT
kernels.

Code

common/openipc.c[R427-430]

+      rfs_read(name, (ulong)&hdr, 0, sizeof(hdr), &actread) ||
+      actread != sizeof(hdr) || !image_check_magic(&hdr) ||
+      !image_check_hcrc(&hdr)) {
+    printf("Recovery: %s is not a uImage, skipped\n", name);

Evidence

The added uImage-only condition rejects FIT's FDT header, and the changed caller has no FIT
fallback. Existing kernel detection accepts FDT headers, and the environment's uk command also
falls back to a FIT image.

common/openipc.c[426-430]
common/openipc.c[559-575]
common/openipc.c[81-93]
include/configs/openipc-common.h[175-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
NOR card recovery now rejects FIT kernel files that the previous recovery path accepted and the boot path still recognizes.
## Fix Focus Areas
- common/openipc.c[426-430]
- common/openipc.c[565-574]
- common/openipc.c[81-93]
## Recommended Fix
Add an explicit, validated FIT recovery path with a defined flash destination, or retain the former FIT fallback alongside the new wrapped-image rule. Test recovery from a FIT-only card.

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

---

3. Shorter firmware leaves old overlay data 🐞 Bug ≡ Correctness

Description

recovery_write_img erases only the new payload's rounded-up length, replacing the split-firmware
recovery path that erased from the firmware start to the end of flash. When a shorter whole-firmware
image replaces a larger installation, bytes beyond that erase range remain; the split-layout update
path still erases to flash end specifically to avoid leaving an old overlay behind.

Code

common/openipc.c[R471-473]

+  printf("Recovery: writing %s (%lu bytes) at 0x%lx\n", name, len, addr);
+  if (spi_flash_erase(recovery_flash, addr, ALIGN(len, sect)) ||
   spi_flash_write(recovery_flash, addr, len, (void *)CONFIG_SYS_LOAD_ADDR)) {

Evidence

The new erase length ends with the payload, whereas the split-layout scan explicitly maintains an
end-of-flash erase for whole-firmware updates to prevent an old overlay surviving a shorter image.
The generated flash partitions include a trailing rootfs_data area.

common/openipc.c[465-473]
common/openipc.c[749-766]
common/openipc.c[869-883]

Agent prompt

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

## Issue description
A shorter whole-firmware recovery image leaves bytes from the previous installation beyond the new erase range.
## Fix Focus Areas
- common/openipc.c[465-473]
- common/openipc.c[749-766]
## Recommended Fix
For a whole-firmware recovery on a split-layout board, clear the remaining firmware area through the end of flash, including when the new payload already matches but stale trailing data remains. Keep bounded erases for independent kernel and rootfs images, and test replacement of a longer installation with a shorter one.

ⓘ 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 open the rule or relationship behind a finding in one click

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗