Skip to content

cv6xx: keep the parser's first-table offset, and say where its bounds come from - #166

Merged
widgetii merged 1 commit into
masterfrom
cv6xx/parser-nits
Oct 25, 2026
Merged

widgetii merged 1 commit into
masterfrom
cv6xx/parser-nits

Conversation

@widgetii

Copy link
Copy Markdown
Member

Follow-up to #130, covering the two nits from its review.

Changes

  • CV6xxBootParts.first_table_offset replaces offset_32. build_ddr_table() used to read the DDR header's structure_length from file_data again to rebuild where table 0 starts, even though parse_cv6xx_boot() had already validated that. The parser now keeps the validated offset, so the two can't drift. offset_32 had no other users; the browser parser already kept tableOffset.

  • The parser's bounds are documented with the layouts they were set against, measured on the published images:

    Image GSL header GSL → DDR header Board map
    boot-hi3516cv608-nor.bin, boot-hi3516cv610-*-nor.bin 0x800 0x400 8 bytes, 1 table
    boot-hi3519dv500-dmebpro-nor.bin 0x1200 0x100 8 bytes, 1 table

    MAX_GSL_HEADER_OFFSET (64 KiB) and MAX_SECTION_GAP (4 KiB) leave room past both layouts. MAX_DDR_TABLES = 8 because the board map has 8 entries, so a ninth table could never be selected. The browser constants point at the Python comment.

Verified

  • No behaviour change: GSL, U-Boot and build_ddr_table() output are byte-identical to master for all six published images and board IDs 0–9.
  • pytest tests/ --ignore=tests/fuzz: 1097 passed, 3 skipped. Fuzz: 16 passed.
  • ruff and mypy are clean; node --test: 95 passed.

… come from

build_ddr_table() re-read the DDR header's structure length from the raw
file to rebuild the offset of table 0, which parse_cv6xx_boot() had
already validated. CV6xxBootParts now carries that offset instead of
offset_32, the only thing offset_32 was used for, so the two cannot read
the header differently.

The parser's bounds get the layouts they were set against: the GSL
header at 0x800 (hi3516cv608/cv610) or 0x1200 (hi3519dv500), 0x400 or
0x100 between the GSL and the DDR header, and an 8-byte board map that
caps the tables at eight. The browser copy points at those comments.

Output is byte-identical to master for every published cv608, cv610
and hi3519dv500 boot image and every board ID.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

CV6xx: Reuse the validated DDR table offset and document parser bounds

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Preserve the parser-validated first-table offset instead of rereading the DDR header during table
 construction.
• Document the published image layouts behind parser bounds and link the browser's matching
 constants.
Diagram

graph TD
  Image["CV6xx image"] --> Parser["Python parser"] --> Parts["Validated offsets"] --> Builder["DDR builder"] --> Table["Selected table"]
  Image --> Browser["Browser parser"]
  Parser -.->|bound rationale| Browser
Loading
High-Level Assessment

Keep the validated offset in the parsed parts: it removes a redundant header read without changing table selection. Recomputing or revalidating it in the builder would duplicate parser logic without a clear benefit.

Files changed (2) +26 / -11

Refactor (1) +24 / -11
hisilicon_cv6xx.pyCarry the validated first-table offset into DDR table construction +24/-11

Carry the validated first-table offset into DDR table construction

• Replaces the unused intermediate offset with the parser-validated first-table offset, which build_ddr_table() now uses without rereading the header. Comments explain the published layouts, field sizes, and board-map limits behind the existing parser bounds.

src/defib/protocol/hisilicon_cv6xx.py

Documentation (1) +2 / -0
protocol.jsLink browser parser bounds to their Python rationale +2/-0

Link browser parser bounds to their Python rationale

• Adds a comment identifying the matching Python constants as the source of the bounds rationale and asking that the two sets change together. Browser parsing behavior is unchanged.

web/protocol.js

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@widgetii
widgetii merged commit 6459158 into master Oct 25, 2026
13 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