[NXP: MIMXRT1064-EVK] Board Enablement - #66
alieissa-commits wants to merge 9 commits into
Conversation
…fication pipeline
570cd5c to
a3213fa
Compare
There was a problem hiding this comment.
Thanks for this, it is a substantial piece of work and parts of it are the best-structured contribution the pipeline has had. The CI job picks up the pinned, checksum-verified Arm toolchain and Renode steps from #53 and #55 rather than reinventing them, and it caches both. Three demos that build clean and run under multi-node emulation is a real result.
I am requesting changes on four things: two structural, one a claim that is ahead of its evidence, and one a test that cannot fail. Detail is in the inline comments, summary here.
Where it lands. docs/architecture.md sections 2 and 4 place new boards under targets/<Vendor>/<Board>/, started from templates/target/ and implementing the abstract interfaces in bsp/include/bsp/. This PR adds a root-level NXP/, alongside MXChip, OpenHW and STMicroelectronics, which that document labels [Pre-framework] and freezes. It also defines its own board_init(), console_write() and USER_LED_* macros in place of bsp_board_init, bsp_console_write and bsp_led_*. The framework exists to stop that legacy set growing, so this needs to move to targets/NXP/MIMXRT1064-EVK/ and implement the contract. The existing console_* functions can stay exactly as they are, sitting behind bsp_console_write.
fetch_sdk.sh reintroduces what #55 removed. Three of the five fetches track a moving ref, mcuxsdk-examples at main, getting-started at master and CMSIS_5 at its default branch, and not one of the five is checksummed. The CI step also runs the full CMSIS clone on every build with no cache. Pin each ref to a tag or a commit, verify what comes down, and cache on the pin, the way the toolchain steps in that same workflow file already do.
The verification claims are ahead of the verification. The passing Renode run emits 1,472 non existing peripheral warnings. Renode 1.16.1 ships platforms/cpus/imxrt1064.repl, which declares CCM, ANALOG, IOMUXC, IOMUXC_GPR, SRC and OCOTP as Tag stubs, two of them returning fixed values: CCM_CBCDR reads back 0x000A8200 and the analog register at 0x400D8030 reads back 0x80002001 no matter what the code writes. So BOARD_BootClockRUN(), the PLL bring-up and the pin mux are not merely unverified, the emulator reports success to anything that polls them. The 600 MHz in the banner is a compile-time constant assigned in the SDK's clock_config.c, not a readback. What this suite establishes is that the image boots and that ThreadX and NetX Duo work over Renode's Ethernet model. That is worth having, and worth saying precisely in the README.
The TRNG test cannot fail, which matters more than the rest because there is an open driver correctness question behind it. See the comments on app/trng.c and on both netx_trng_console files.
The description reads as a feature announcement rather than cause, fix and proof, and it describes the shell as multi-client when it is one socket in one thread serving one connection at a time. The numbering also skips 4. Please rewrite it around what changed and what the run actually asserts: on a squash with this many commits the PR title and body become the permanent history entry.
| @@ -0,0 +1,241 @@ | |||
| # Copyright (c) 2026 Eclipse ThreadX contributors | |||
There was a problem hiding this comment.
Blocking, placement. This whole tree should live at targets/NXP/MIMXRT1064-EVK/, not at the repository root.
docs/architecture.md section 4 describes onboarding a board as: create targets/<Vendor>/<Board_Name>/ from templates/target/, then implement the interfaces in bsp/include/bsp/. The root-level vendor directories are the pre-framework set that section 1 explicitly freezes, and adding a fourth reopens it.
Beyond the move, the board needs to implement bsp_board_init, bsp_led_init/on/off/toggle and bsp_console_init/write. board_init() and console_write() here are the right implementations, they just need to sit behind those names so the demos can be built against the contract rather than against this board.
| trap clean_temp EXIT | ||
|
|
||
| # 1. Download official NXP MIMXRT1064 DFP pack from NXP repository | ||
| PACK_URL="https://mcuxpresso.nxp.com/cmsis_pack/repo/NXP.MIMXRT1064_DFP.15.1.0.pack" |
There was a problem hiding this comment.
The pack version is pinned, good, but there is no checksum. #55 established pin plus SHA256 verification for every download in this repository, and this is the one path that does not follow it.
The same applies to the four curl calls and the clone below.
| echo "" | ||
|
|
||
| # 2. Download EVK-MIMXRT1064 Board Support Files from official NXP mcuxsdk-examples | ||
| RAW_BASE="https://raw.githubusercontent.com/nxp-mcuxpresso/mcuxsdk-examples/main/_boards/evkmimxrt1064" |
There was a problem hiding this comment.
Blocking. Ten build-critical files fetched from main. Any commit upstream silently changes what this board compiles, with nothing recording which version produced a given build.
Pin to a tag or a commit SHA and verify the downloads.
A second problem in the loops that follow: the if [ -f ... ]; then cp guard means a missing file is skipped silently rather than failing. That defeats set -e, so an upstream layout change produces a half-populated SDK and a confusing compile error instead of a clear failure here.
| # 3. Fetch CMSIS Core headers | ||
| echo "[INFO] Cloning CMSIS Core headers (depth=1)..." | ||
| CMSIS_CLONE_DIR="${TEMP_DIR}/cmsis_core_repo" | ||
| git clone --depth 1 https://github.com/ARM-software/CMSIS_5.git "${CMSIS_CLONE_DIR}" |
There was a problem hiding this comment.
Blocking. An unpinned clone of the CMSIS_5 default branch. Pin it to a release tag.
This is also uncached in CI, so every run re-clones the repository in full. #55 removed exactly this cost for the toolchain and Renode downloads; once the ref is pinned, actions/cache keyed on that pin will do the same here.
|
|
||
| # 5. Fetch official NetX Duo NXP Ethernet driver (100% stock upstream) | ||
| echo "[INFO] Downloading official NetX Duo NXP Ethernet driver..." | ||
| NETX_RAW_BASE="https://raw.githubusercontent.com/eclipse-threadx/getting-started/master/NXP/MIMXRT1060-EVK/lib/netx_driver" |
There was a problem hiding this comment.
Blocking. The NetX Ethernet driver and, just above, the KSZ8081 PHY driver are pulled from getting-started at master, unpinned.
Worth a separate decision from the two of us before this merges: getting-started is on the same archive track as iot-devkit, which the root README says will be retired. Making two build-critical drivers depend on its master HEAD points this board at a repository the project intends to stop maintaining. Vendoring the two drivers, as the NUCLEO target does with the STM32 HAL, may be the better answer.
| /** | ||
| * @brief _sbrk() allocates memory to the newlib heap and is used by malloc. | ||
| */ | ||
| void *_sbrk(ptrdiff_t incr) |
There was a problem hiding this comment.
_sbrk has no mutual exclusion, and there are no __malloc_lock / __malloc_unlock implementations anywhere in the board.
All three demos call printf from two or more threads, and threadx_basic also calls it from a TX_TIMER callback. newlib allocates the stdout buffer on the first buffered write, so two threads reaching that point together can corrupt the heap. This is the kind of defect that will not show up in a two-second emulation run and will show up on a desk after an hour.
Either implement the malloc lock hooks over a TX_MUTEX, or give the console its own mutex and make stdout unbuffered.
Separately: the bound here is __StackLimit, while the linker script also defines __heap_limit with the comment Add for _sbrk. One of the two should go.
| #include "tx_api.h" | ||
| #include <stdio.h> | ||
|
|
||
| #define HEARTBEAT_THREAD_STACK_SIZE 1024 |
There was a problem hiding this comment.
1 KB is too small for a thread that calls printf.
newlib's full vfprintf wants on the order of a kilobyte of stack by itself, and both threads here format %lu conversions. ThreadX stack checking is off by default, so an overflow corrupts whatever is adjacent rather than faulting, which makes it painful to diagnose later.
2 KB would be the safe floor. It is also worth enabling TX_ENABLE_STACK_CHECKING in the debug configuration so the suite catches this class of problem itself.
One more, unrelated to size: these stacks are uint8_t[] here and in netx_echo, but ULONG[] in netx_trng_console. ThreadX realigns internally so nothing breaks, but pick one.
| * Compatibility header: redirects MIMXRT1062.h from stock NetX Duo driver | ||
| * to MIMXRT1064 device registers without modifying vendor source files. | ||
| */ | ||
| #ifndef _MIMXRT1062_H_ |
There was a problem hiding this comment.
_MIMXRT1062_H_ is a reserved identifier: a leading underscore followed by a capital is reserved to the implementation in every C standard, and MISRA C 2012 Rule 21.2 rejects it.
MIMXRT1062_H is enough. The shim itself is a neat solution to keeping the vendor driver unmodified.
| ${SDK_DIR}/drivers/fsl_enet.c | ||
| ) | ||
|
|
||
| target_compile_definitions(netx_imxrt_driver_client |
There was a problem hiding this comment.
This eight-line definitions block is the fourth verbatim copy in this file, and the include list below is the fourth near-copy.
An INTERFACE library carrying both, linked by each target, would remove all of it. netx_imxrt_driver and netx_imxrt_driver_client differ only by NETX_CLIENT_NODE=1 and could otherwise share everything.
The block starting at the # 2. comment is also indented four spaces where nothing else in the file is.
| on: | ||
| push: | ||
| branches: [ main, master, dev, 'feat/**' ] | ||
| branches: [ main, master, dev, 'feat/**', 'test/**' ] |
There was a problem hiding this comment.
test/** is a branch-naming convention from this contribution rather than something the pipeline needs, and it changes the trigger for every job in the file. Please drop it.
workflow_dispatch below is a good addition, keep that.
[NXP: MIMXRT1064-EVK] Board Enablement
Overview
This PR introduces full board enablement and demonstration applications for the NXP i.MX RT1064-EVK evaluation board (ARM Cortex-M7 @ 600 MHz) running Eclipse ThreadX RTOS and the NetX Duo TCP/IP stack.
The port is designed around a decoupled static Board Support Package (
board_bsp) that isolates low-level hardware initializations (system clocks, MPU, caches, pin muxing, Ethernet MAC/PHY descriptors, and on-chip hardware TRNG) from application logic.Key Highlights & Features
Modular Multi-Demo Architecture:
threadx_basic: Simple multi-threading, kernel software timers, and User LED heartbeat blinking.netx_echo: Dual-node network verification testing ARP, ICMP Ping, UDP Echo, and TCP Echo over an emulated virtual switch.netx_trng_console: True Random Number Generator hardware entropy driver paired with a multi-client Telnet management shell. Supports optional-Seed <number>parameter for reproducible hardware entropy verification.build/app/demos/<demo_name>/.Simulation & Automated CI/CD:
test_headless.ps1andtest_headless.sh) for deterministic regression testing without a display.Cross-Platform Tooling:
Standards & Licensing Compliance:
NOTICE.mddocumenting upstream NXP MCUXpresso SDK (BSD 3-Clause) and official Arm CMSIS-Core (Apache-2.0).