[dimerr]

• common/openipc.c byte for byte as on openipc-hisilicon, with the generic SD/USB recovery (the openipc-hisilicon PR from recovery/wrapper-payload).
• SPI NAND images gone: no XM530/XM550 board with SPI NAND has turned up, the kernel has no driver for it, and the boot path was a guess. Removed: xmsfc_nand.c, its hook, openipc/nand_config, the NAND build and CONFIG_SYS_MAX_NAND_DEVICE; CLAUDE.md updated. The NAND block in the shared openipc-common.h is left as is; it is never compiled now.

Needs the openipc-branch PR from ci/xm-nor-only merged first, or CI looks for -nand.bin.

Tested: xm510/xm530/xm550 build. On qemu-hisilicon xm530 from an SD card, over two boots: a kernel image was written at 0x50000; bad CRC and non-uImage files were skipped; U-Boot and then the kernel were found already on the flash; the saved env loaded.

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

PR Summary by Qodo

Add generic XM recovery and remove unsupported SPI NAND images

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

AI Description

• Recover valid SD/USB uImages to SPI NOR while skipping corrupt or already-installed payloads.
• Align flash scanning with shared OpenIPC behavior and remount FAT between filesystem calls.
• Remove unverified XM SPI NAND images, driver support, and build configuration.

Diagram

graph TD
  Media["SD or USB"] --> Script{"script.txt found?"} -->|no| List["List root images"] --> Valid{"Valid uImage?"} -->|yes| Flash["Compare or write NOR"] --> Scan["Scan firmware"] --> Boot["Boot firmware"]
  Script -->|yes| Run["Run script"] --> Scan
  Valid -->|skip| Scan

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Require a target-specific recovery manifest

• ➕ Could prevent a valid image for another target from overwriting XM flash.
• ➖ Requires new image tooling and breaks compatibility with the shared, generic OpenIPC recovery format.

Recommendation: Keep the shared generic recovery implementation and NOR-only builds: they remove an unverified boot path without introducing an XM-specific recovery fork. Because valid removable-media images can target U-Boot at flash address zero, recovery media should be trusted; consider target binding if that assumption changes. Merge the stated ci/xm-nor-only prerequisite before relying on CI.

Files changed (6) +375 / -158

Enhancement (1) +361 / -86

openipc.cSynchronize shared recovery and firmware scanning +361/-86

Synchronize shared recovery and firmware scanning

• Adds generic SD/USB recovery that validates uImage headers and CRCs, writes payloads at bounded flash addresses, and skips payloads already present. Remounts FAT for each filesystem operation and improves NOR scanning, with split-layout handling compiled when configured; NAND-specific paths return to the shared CONFIG_FMC_SPI_NAND gate.

common/openipc.c

Documentation (1) +12 / -38

CLAUDE.mdDocument NOR-only builds and SD recovery +12/-38

Document NOR-only builds and SD recovery

• Removes the speculative SPI NAND boot and test guidance. Documents NOR-only outputs and recovery from root-directory images on an SD card.

CLAUDE.md

Other (4) +2 / -34

build.shBuild only NOR images for XM targets +2/-21

Build only NOR images for XM targets

• Stops applying the NAND configuration and generating a NAND image with a synthetic boot header. XM510, XM530, and XM550 now each produce one NOR binary.

build.sh

KconfigRemove the XM SPI NAND configuration option +0/-9

Remove the XM SPI NAND configuration option

• Removes NAND_XMSFC, so XM530/XM550 builds can no longer select the unsupported controller driver.

drivers/mtd/nand/raw/Kconfig

MakefileRemove the XM SPI NAND driver build hook +0/-1

Remove the XM SPI NAND driver build hook

• Drops the object-file selection for the removed xmsfc_nand driver.

drivers/mtd/nand/raw/Makefile

xm530.hRemove the XM530 NAND device limit +0/-3

Remove the XM530 NAND device limit

• Drops CONFIG_SYS_MAX_NAND_DEVICE and its NAND-image comment from the shared XM530/XM550 board header.

include/configs/xm530.h

[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 crafted recovery image overwrites RAM ✓ Resolved

Description

recovery_write_img() accepts a uImage header with a zero payload length and passes that length to
rfs_read(), where zero means read the rest of the file rather than read nothing. A large .img
file on an SD card or USB stick can therefore write past the available RAM below U-Boot’s stack
before the read-length check rejects it.

Code

common/openipc.c[R456-457]

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

Evidence

The new image path checks RAM capacity using len, then forwards that value to fs_read. FAT
limits its read only when the requested maximum is greater than zero; with zero, it copies file
contents from the given offset before the caller can check actread.

common/openipc.c[433-459]
common/openipc.c[248-255]
fs/fs.c[573-612]
fs/fat/fat.c[322-345]
fs/fat/fat.c[375-417]

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 zero payload length bypasses the recovery RAM-size check but makes FAT read the entire remainder of the file into RAM.
## Fix Focus Areas
- common/openipc.c[433-456]
## Recommended Fix
Reject zero-length payloads before calling `rfs_read()`, and ensure the requested read length is the length checked against available RAM.

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

---

2. A card left in the slot can wipe saved settings on every boot 🐞 Bug ≡ Correctness

Description

recovery_write_img skips an image only when recovery_flash_holds finds every payload byte
already on the flash. If the image covers flash that the system writes while running (the env sector
for a whole-flash image at 0, or the rootfs_data overlay inside a full firmware image), the check
fails on every boot after the first, so the image is erased and rewritten each time and the saved
env or overlay settings are lost.

Code

common/openipc.c[R465-469]

+  /* A card left in the slot: nothing to do, the system boots */
+  if (recovery_flash_holds(addr, (void *)CONFIG_SYS_LOAD_ADDR, len)) {
+    printf("Recovery: %s is already on the flash at 0x%lx\n", name, addr);
+    return 0;
}

Evidence

The comment on recovery_write_img says the payload may be "a whole firmware, U-Boot itself", and the
only thing that stops a rewrite is a byte-for-byte match across the whole payload length
(recovery_flash_holds). firmware_recovery runs on every boot before firmware_scan (lines 979-980),
and nothing records that an image has already been applied. So any area inside the payload that
changes at runtime (env saved at CONFIG_ENV_OFFSET, the jffs2 overlay after the rootfs) makes the
next boot erase and rewrite it.

common/openipc.c[398-413]
common/openipc.c[415-479]
common/openipc.c[975-982]

Agent prompt

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

## Issue description
With a recovery card left inserted, an image whose payload covers the env sector or the rootfs_data overlay never compares equal to the flash after runtime writes. It is reflashed on every boot and user settings are wiped.
## Fix Focus Areas
- common/openipc.c[398-413]
- common/openipc.c[465-469]
## Recommended Fix
Record that an image was applied, e.g. store its header dcrc and load address in an env variable (`recovery_done`) after a successful write, and skip images whose dcrc matches. Alternatively, leave out of the comparison (and the write) the env range and anything past the rootfs end, or refuse images that overlap CONFIG_ENV_OFFSET..+CONFIG_ENV_SIZE unless they start at 0 and are U-Boot only.

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

---

3. Base config comment still mentions the removed NAND fragment ✓ Resolved

Description

The header of openipc/openipc_config still says build.sh appends nand_config for NAND images,
but this PR deletes that fragment and its line in build.sh. A later reader will look for a NAND
build path that no longer exists.

Code

build.sh[34]

-    [ "${kind}" = nand ] && fragments="${fragments} openipc/nand_config"

Evidence

openipc/openipc_config lines 1-3 still describe nand_config being appended, while build.sh no longer
adds it and openipc/nand_config is deleted.

openipc/openipc_config[1-3]
build.sh[30-41]

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 openipc_config header comment still refers to the removed nand_config fragment.
## Fix Focus Areas
- openipc/openipc_config[1-3]
## Recommended Fix
Change the comment to: "build.sh appends the per-SoC fragment and runs olddefconfig, so a later line overrides an earlier 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 ↗