Conversation
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Adds initial i.MX 8QuadMax (imx8qm) platform support, including a new HAL, build/flash tooling, and unit tests that pin down critical register-translation and NOR-write semantics.
Changes:
- Introduces the i.MX8QM HAL (SCU bring-up, uSDHC↔SDHCI shim, FlexSPI NOR, optional MMU, DT fixups) plus linker script and example configs.
- Adds host unit tests for the uSDHC SDHCI-translation shim and FlexSPI NOR page/erase behavior (via extracted HAL code).
- Adds scripts and CI jobs to build, bundle into an NXP boot container, flash, and AHAB-sign images.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
hal/imx8qm.c |
Implements i.MX8QM HAL: SCU init, uSDHC shim, FlexSPI NOR, optional MMU teardown, DT fixups. |
hal/imx8qm.h |
Provides i.MX8QM register bases/constants used by C and AArch64 startup. |
hal/imx8qm.ld |
Defines BL33 DRAM-resident link map and reserved stack region. |
arch.mk |
Adds target-specific AArch64 flags and i.MX8QM build knobs/defaults. |
Makefile |
Adjusts main build outputs for imx8qm (no factory.bin; build bootloader + signed payload). |
src/boot_aarch64_start.S |
Adds simple EL2 vectors for imx8qm and stack/DTB handoff handling. |
src/sdhci.c |
Adds shutdown behavior to revert 1.8V signaling after UHS recovery; updates comment scope list. |
tools/unit-tests/unit-imx8qm-usdhc-shim.c |
New unit test pinning uSDHC↔SDHCI translation semantics and write ordering. |
tools/unit-tests/unit-imx8qm-flexspi-write.c |
New unit test covering NOR page-boundary splits, WREN-per-op, erase rounding, and read address forms. |
tools/unit-tests/Makefile |
Adds extracted-header generation and build rules for the new unit tests. |
tools/scripts/imx8qm/imx8qm-mkflashbin.sh |
Bundles wolfBoot + signed payload (+ optional DTB) into BL33 image and optionally invokes imx-mkimage. |
tools/scripts/imx8qm/imx8qm-flash.sh |
Adds safe SDP and removable-SD flashing helper. |
tools/scripts/imx8qm/imx8qm-ahab-sign.sh |
Adds AHAB signing helper using NXP CST. |
test-app/app_imx8qm.c |
Adds an i.MX8QM payload app validating EL and DTB handoff via x0. |
docs/Targets.md |
Documents the new imx8qm target, build variants, container flow, and limitations. |
.github/workflows/test-configs.yml |
Adds CI build matrix entries for multiple imx8qm variants. |
.gitignore |
Ignores newly built unit-test binaries. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 6 comments.
89e0436 to
b65885e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The SDHCI changes introduce a potential hang path around line resets and the CMD8 “ignore error” logic is overly broad and should be tightened to avoid masking real controller/bus failures.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
|
|
||
| static inline int timer_expired(uint64_t deadline) | ||
| { | ||
| return timer_get_count() > deadline; |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.
There was a problem hiding this comment.
🟡 Changes recommended
src/sdhci.c has a new error path that can fail a command without emitting a diagnostic, making failures harder to debug (see stored comment ID: 001).
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The updated SDHCI error-recovery loops introduce new long polling paths that should service the platform watchdog, and the repo should ignore the newly-added SDHCI cmd-inhibit unit-test binaries to avoid leaving untracked artifacts.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a new SoC port plus cross-cutting boot/SDHCI/AArch64 changes that are hardware-sensitive and require careful human validation beyond static review.
Review details
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Adds support for running wolfBoot as bare-metal BL33 on the i.MX 8QuadMax MEK, replacing U-Boot in the NXP AHAB container.
Added Components
hal/imx8qm.{c,h,ld}: HAL, register definitions, and linker script.config/examples/imx8qm-mek.config: Main config template (SD, eMMC, Linux, filesystem, chainload).config/examples/imx8qm-mek-qspi.config: FlexSPI NOR memory map.test-app/app_imx8qm.c,tools/scripts/imx8qm/packaging/signing helpers, target block inarch.mk, CI jobs, and host unit tests.Shared / Core Changes
src/sdhci.c:Command Inhibitwait and added reset on timeout to prevent hangs.CMD8responses as fatal (supports legacy SD v1.x cards).SDHCI_UHS_RECOVER_ON_INITopt-in flag.src/boot_aarch64_start.S&hal/cm4.c: Generalized EL2 vector table initialization across targets.src/boot_aarch64_start.S&include/aarch64_arch.h: Replaced hardcoded target check withWOLFBOOT_AARCH64_MMU_TEARDOWNmacro and centralized AArch64 timer/cache helpers.src/update_disk.c: Fixed bug where staged external images were verified against external flash instead of RAM after copy.Hardware & Test Status
Target Hardware: i.MX 8QuadMax MEK (MCIMX8QM-CPU, RevB)
[Install Certificate]form the CA-flagged SRKs require, and boots on this open part. Signature enforcement is unproven: an open part does not verify it, and no board here has fused SRKs.