Skip to content

gk710x: publish U-Boot in CPU order, like every other file - #50

Merged
dimerr merged 2 commits into
openipc-gk710xfrom
gk710x/cpu-order
Oct 22, 2026
Merged

dimerr merged 2 commits into
openipc-gk710xfrom
gk710x/cpu-order

Conversation

@dimerr

@dimerr dimerr commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Every published file is now in CPU order (U-Boot, uImage, rootfs), so they all load by tftp/mmc/usb and go to flash with sf write or SD recovery as they are. The programmer image is the whole flash laid out from those files and word-swapped as one; the openipc-branch PR from docs/gk710x-cpu-order adds the README recipe for it.

Why: the U-Boot release was in chip order while kernel and rootfs were not, so concatenating them for a programmer put U-Boot on the chip right and the kernel and rootfs unswapped. A GK7102C dump shows exactly that: U-Boot and env fine, 27 05 19 56 and hsqs raw at 0x50000/0x250000, and no kernel found.

  • gk710x-image.py writes CPU order.
  • gk_sflash.c no longer swaps on read. It still recognises a chip-order image written at 0 and swaps it back, so a programmer image fed to sf write or SD recovery lands right.
  • CLAUDE.md: the byte-order rules.

Tested in qemu-hisilicon:

  • gk7102: a programmer image built with dd + objcopy boots to login twice, and the env survives.
  • gk7102 and gk7102s (on a GK7102C dump), U-Boot only on the chip: SD recovery writes the CPU-order kernel and rootfs, and the flash reads back byte for byte as the files. Both boots reach login; on boot 2 all three images are already on the flash.
  • gk7102: the programmer image as an SD recovery image at 0 lands byte for byte (env and the jffs2 overlay aside) and boots.

CI's smoke step already swaps a CPU-order image for qemu.

The release U-Boot was in chip order -- every 32-bit word swapped, as
the chip holds it -- so that a programmer could write it as it is,
while OpenIPC/firmware's uImage and rootfs are in CPU order. A
programmer image made by concatenating the three put U-Boot on the
chip right and the kernel and rootfs unswapped, and the camera found no
kernel (a GK7102C dump shows exactly that).

One order for everything published: gk710x-image.py writes CPU order,
which tftp, mmc and usb load and sf write or SD recovery put on flash
as they are. A programmer or qemu flash-file image is the whole flash
laid out from those files and then word-swapped as one, e.g. with
objcopy --reverse-bytes=4 (the openipc branch's README shows how).

gk_sflash.c no longer swaps on read: through the driver the flash reads
as it was written, in CPU order. It still recognises a chip-order image
written at 0 by its swapped magic and swaps it back, so a programmer
image fed to sf write or SD recovery lands right as well.

Under qemu-hisilicon:
- gk7102: an 8 MiB programmer image built with dd + objcopy from the
  CPU-order U-Boot, uImage and rootfs boots to login, and the second
  boot loads the saved environment;
- gk7102 and gk7102s (a GK7102C flash dump): with only U-Boot on the
  chip, SD recovery writes the CPU-order kernel and rootfs images; the
  flash then reads back byte for byte as the three files, both boots
  reach login, and on the second the U-Boot, kernel and rootfs images
  are all "already on the flash";
- gk7102: the programmer image itself, as an SD recovery image at 0,
  lands on the chip byte for byte (env and the jffs2 overlay aside,
  which the boot then writes) and boots.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

gk710x: Publish U-Boot in CPU byte order

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Publish U-Boot in CPU order, matching kernel and rootfs images for direct flash writes.
• Stop swapping flash reads while retaining write compatibility with chip-order programmer images.
• Document whole-flash word swapping required for programmers and QEMU.
Diagram

graph TD
  B["Image builder"] --> P["CPU-order files"] --> D["SPI flash driver"] --> C[("Flash storage")]
  P --> L["Whole-flash layout"] --> S["Word swap"] --> Q["Programmer or QEMU"] --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Publish both CPU-order and chip-order images
  • ➕ Provides a ready-to-use programmer image without a separate conversion step.
  • ➖ Creates two release artifacts with different byte-order rules and a greater risk of choosing the wrong one.

Recommendation: Keep the PR's single CPU-order format for published components and convert the assembled flash image for programmer or QEMU use. This avoids mixing byte orders during direct writes or image assembly; publishing both formats would reintroduce ambiguity.

Files changed (3) +44 / -82

Bug fix (2) +21 / -50
gk_sflash.cRemove boot-image read swapping +15/-43

Remove boot-image read swapping

• Flash reads now return CPU-order data without a boot-image swap window, and probing no longer establishes one from existing flash contents. Write-time detection and conversion of chip-order images starting at offset zero remain in place.

drivers/mtd/spi/gk_sflash.c

gk710x-image.pyEmit the U-Boot release image in CPU order +6/-7

Emit the U-Boot release image in CPU order

• Writes the constructed boot header and U-Boot image without reversing each 32-bit word. Updates the module guidance to require whole-image word swapping for programmer and QEMU flash files.

openipc/gk710x-image.py

Documentation (1) +23 / -32
CLAUDE.mdDocument the CPU-order release and chip-order programmer workflow +23/-32

Document the CPU-order release and chip-order programmer workflow

• Replaces guidance that treated the U-Boot release as chip-order with rules for CPU-order published files and word-swapped whole-flash images. Clarifies read behavior, write-time compatibility, and QEMU input requirements.

CLAUDE.md

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

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Later boot sectors can be corrupted ✓ Resolved
Description
gk_sf_read now returns CPU-order bytes, so sf update can skip an unchanged sector at offset 0
without clearing the swap window left by an earlier chip-order write. If a later bootloader sector
differs in that same session, gk_sf_write swaps its CPU-order bytes under the stale window, and
the update completes without verifying the corrupted sector.
Code

drivers/mtd/spi/gk_sflash.c[L242-243]

-		if (addr < sf_swap_end)
-			w = swab32(w);
Evidence
The update loop compares sectors in order and returns without writing an identical sector. Only a
write at offset zero refreshes sf_swap_end; subsequent writes below that retained end are
word-swapped. Removing the read-side swap makes sector zero of a previously written chip-order image
compare equal to a CPU-order input, enabling this path.

cmd/sf.c[178-204]
cmd/sf.c[220-250]
drivers/mtd/spi/gk_sflash.c[208-245]
drivers/mtd/spi/gk_sflash.c[262-279]

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

## Issue description
After a chip-order write at offset zero, a CPU-order `sf update` can skip identical sector zero and leave the chip-order swap window active for changed later sectors.
## Fix Focus Areas
- drivers/mtd/spi/gk_sflash.c[208-215]
- drivers/mtd/spi/gk_sflash.c[262-279]
- cmd/sf.c[178-204]
## Recommended Fix
Ensure a compare-before-write skip at offset zero updates or clears the driver's swap state according to the update input, even though no sector is programmed. Verify that a changed later bootloader sector is written and read back in CPU order.

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



Remediation recommended

2. Recovery rewrites the same bootloader 🐞 Bug ☼ Reliability
Description
recovery_flash_holds compares a chip-order recovery payload directly with the CPU-order bytes that
gk_sf_read now returns, so it cannot recognize an already-flashed chip-order bootloader. When its
recovery card remains inserted, boot-time recovery erases and programs that bootloader again on
every start instead of taking the existing-image skip.
Code

drivers/mtd/spi/gk_sflash.c[L242-243]

-		if (addr < sf_swap_end)
-			w = swab32(w);
Evidence
Recovery compares flash reads byte-for-byte with the loaded payload before deciding to erase and
write. The driver recognizes chip-order boot magic and swaps that payload on write but no longer
swaps it on read; openipc_helper invokes recovery during boot.

common/openipc.c[398-414]
common/openipc.c[476-489]
common/openipc.c[992-998]
drivers/mtd/spi/gk_sflash.c[208-245]
drivers/mtd/spi/gk_sflash.c[262-279]

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

## Issue description
Recovery accepts a chip-order boot image at offset zero, but its unchanged-image check compares that payload against CPU-order flash reads and repeatedly rewrites it.
## Fix Focus Areas
- common/openipc.c[398-414]
- common/openipc.c[476-489]
- drivers/mtd/spi/gk_sflash.c[183-215]
## Recommended Fix
Make the recovery comparison account for the same chip-order conversion that the flash write applies, while retaining a direct comparison for CPU-order payloads. Test a second boot with a chip-order bootloader recovery image still present.

ⓘ 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 ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread drivers/mtd/spi/gk_sflash.c
Comment thread drivers/mtd/spi/gk_sflash.c
After a chip-order image was written at 0, the window stayed open for
any later write below its end. A CPU-order sf update in the same session
whose first sector was unchanged skipped it, so nothing at 0 closed the
window, and its later changed sectors were written swapped.

A write that does not start at 0 now keeps the window only if it starts
where the previous write ended -- the pieces of one image, as sf update
and SD recovery write them.

Under qemu-hisilicon gk7102: chip-order sf write at 0, then a CPU-order
sf update of a U-Boot differing at 0x20000, read back with the file's
CRC (before: a different one); the same with a saveenv in between; and a
chip-order sf update alone lands as the CPU-order file. A programmer
image still boots to login twice.

Reported by Qodo on #50.
@dimerr
dimerr merged commit 76c9f5c into openipc-gk710x Oct 22, 2026
10 checks passed
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