Uh oh!
There was an error while loading. Please reload this page.
Moved the startup self-tests and RAM sizing behind new BSP interfaces - #56
Merged
fdesbiens merged 1 commit intoAug 31, 2026
Conversation
The framework's stated purpose is that application code can be built for any board implementing the `bsp/` contracts. Neither demo could be, because both `main.c` files reached past those contracts for two things no portable application can see: the board's memory extents, and the hardware specifics its startup self-tests assert on. Neither was ever application logic. The self-tests test the BSP - linker reservations, clock trees, interrupt controllers - so they belong in the BSP. Two new contracts move both behind the boundary: * `bsp/selftest.h` - `bsp_self_test()` runs the board's checks and reports each through an application-supplied callback, so message formatting (and therefore the choice between `printf()` and `bsp_console_write()`) stays with the application while the checks stay with the board. * `bsp/memory.h` - `bsp_ram_region()` reports what RAM the application may claim, clamped against whatever the board reserves. Both targets implement both, and both `main.c` files now include nothing but the C standard headers, `tx_api.h` and `<bsp/...>`. The NUCLEO byte pool comes out byte-identical at 88204 bytes. Prototyping `bsp_ram_region()` on the PolarFire first surfaced a latent bug there. Its `_sbrk()` bounded the heap at the end of DRAM, which is the same mistake the NUCLEO shipped with before eclipse-threadx#50: harmless only because nothing else claimed that memory. The moment `bsp_ram_region()` promises it to an application, an oversized `malloc()` could take memory holding thread stacks. The heap is now bounded against a documented 64 KB reservation that `bsp_ram_region()` skips, and a new self-test guards the bound the way the NUCLEO's test 4 does. Measured heap use is under 256 bytes, so the reservation has ample headroom. The PolarFire tick-rate compile-time check moved to `hwtimer.c`, next to the `TICK_CYCLES` constant it guards, since it needed ThreadX headers that a portable `main.c` should not have to pull in. Verified locally on both targets: clean builds plus `test_renode.py` green on Cortex-M4 and RV64. Both suites were also confirmed to still have teeth - weakening each target's `_sbrk()` bound back to the end of RAM makes them exit 1 (three failures on the NUCLEO, one on the PolarFire), so no assertion was lost in the move. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The framework's stated purpose is that application code can be built for any board that implements the
bsp/contracts. Neither demo could be. Bothmain.cfiles reached past those contracts for two things no portable application can see: the board's memory extents, and the hardware specifics its startup self-tests assert on.Neither was ever application logic. The self-tests test the BSP — linker reservations, clock trees, interrupt controllers — so they belong in the BSP. Moving them takes most of the contamination in
main.cwith them.What
Two new contracts in
bsp/include/bsp/:selftest.h—bsp_self_test(report, context)runs the board's checks and reports each through an application-supplied callback, returning the failure count. Message formatting — and therefore the choice betweenprintf()andbsp_console_write()— stays with the application while the checks stay with the board.memory.h—bsp_ram_region(first_unused, &base, &size)reports what RAM the application may claim, clamped against whatever the board reserves above ThreadX's first unused address.Both targets implement both. Both
main.cfiles now include nothing but C standard headers,tx_api.hand<bsp/...>. The NUCLEO byte pool comes out byte-identical at 88204 bytes, so there is no behaviour change on that target.The target template and
docs/architecture.mdgained the two interfaces, and the template shipsbsp_memory.c/bsp_selftest.cstubs so a new board has something to fill in. Neither restores the portability claim that #52 walked back — there is still noapps/directory, and that stays true until CI builds one for two architectures.One scope call worth flagging
Prototyping
bsp_ram_region()on the PolarFire first — as the design intended — surfaced a latent bug there. Its_sbrk()bounded the heap at the end of DRAM, which is the same mistake the NUCLEO shipped with before #50. On the PolarFire it was harmless only because nothing else claimed that memory: the LM75 demo uses static stacks. The momentbsp_ram_region()promises that DRAM to an application, one oversizedmalloc()could take memory holding thread stacks.Shipping the interface without fixing that would have made it a trap, so this PR also bounds the PolarFire heap against a documented 64 KB reservation that
bsp_ram_region()skips, and adds a self-test guarding the bound the way the NUCLEO's test 4 does. Measured heap use on that target is under 256 bytes, so the reservation has roughly 256x headroom.The PolarFire tick-rate compile-time check moved to
hwtimer.c, next to theTICK_CYCLESconstant it guards, because it needs ThreadX headers a portablemain.cshould not have to pull in.Verification
Built and Renode-tested locally on both architectures with the CI-pinned toolchains (Arm GNU 14.3.Rel1, xPack RISC-V GCC 14.3.0-1, Renode 1.16.1):
test_renode.pyBoth suites were also confirmed to have kept their teeth after the move. Weakening each target's
_sbrk()bound back to the end of RAM makes them exit 1 — three failures on the NUCLEO (matching what was documented when they were first written), one on the PolarFire's new guard — so no assertion was silently lost in the refactor.Notes
bsp_ram_region()'s invariant on both targets, and the PolarFire heap bound.main.cfiles regardless of whether a sharedapps/ever lands.