From f65745ed60ff9cdc042828e9da8cdcdb95e8f66d Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Mon, 7 Sep 2026 16:11:56 +0000 Subject: [PATCH 1/3] fix(arm): refuse bounded waits before the tick, and name the boot stages Millis() does not advance until InitSysTick() runs in Board::Init(), so `(Millis() - start) > timeout` could never become true before that point. I2CBus's flag wait and Usart::Receive both compared against that frozen counter with no guard: a bus held low before the tick would have spun forever, with the timeout silently doing nothing. Both now return kInvalidState instead. Unreachable today, since both are called only after Board::Init() has run. It becomes reachable the moment anything talks to a device during bring-up, which is exactly what documenting an off-chip bring-up stage invites. docs/BOOT_FLOW.md is that documentation. The organising idea is that a peripheral's stage is decided by what can fail, not by what is convenient: - On-chip bring-up is register writes against hardware soldered into the die. Nothing can be absent and nothing can time out, so it cannot fail, and it lives in a constructor -- which is what makes an unconfigured peripheral unreachable. - Off-chip bring-up is I/O across a wire, where absence and silence are normal outcomes. It needs somewhere that can report one: Board::Init(). That structure already existed; what was missing was the contract, which board::Board now states where a reader meets it. The ADC is the case that shows the split is not "sensor or not" -- the peripheral is stage 1, the sensor wired to it is stage 2. Also corrects an invariant from the previous commit. mcu::Delay *does* work before the tick -- delay.cpp falls back to a DWT->CYCCNT spin -- so "bring-up must not call mcu::Delay" was wrong. The real boundary is narrower: a stage 1 constructor may wait, but may not talk, because every real transfer needs a timeout. Two follow-ups are written down rather than taken. A timebase valid from reset (Micros() on DWT->CYCCNT) would make timeouts work in every stage and dissolve this boundary; it is worth doing when a stage 1 peripheral needs a bounded wait. Explicit init levels in the style of Zephyr's PRE_KERNEL_1 / POST_KERNEL would replace the two-phase split; the trigger is two off-chip devices with an ordering constraint between them, and until then it would be a framework guarding a state that has not occurred. Known gaps recorded, not solved: common::Error has no payload, so Board::Init() cannot name which device failed; there is nowhere to report a bring-up failure to, since the console is a stage 3 resource; and an RTOS will want SysTick for itself. Costs 236 bytes of .text on i2c_demo for the guards. Co-Authored-By: Claude Opus 5 (1M context) --- docs/BOOT_FLOW.md | 137 ++++++++++++++++++ docs/PROJECT_PLAN.md | 34 +++++ src/libs/board/board.hpp | 15 ++ .../board/stm32f767zi_nucleo/nucleo_board.hpp | 7 +- src/libs/mcu/arm_cm7/i2c.cpp | 8 + src/libs/mcu/arm_cm7/usart.cpp | 6 + 6 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 docs/BOOT_FLOW.md diff --git a/docs/BOOT_FLOW.md b/docs/BOOT_FLOW.md new file mode 100644 index 0000000..a5759d5 --- /dev/null +++ b/docs/BOOT_FLOW.md @@ -0,0 +1,137 @@ +# Boot Flow + +How an application on this project gets from reset to running, and — the part +that matters when you add hardware — **which stage your new peripheral belongs +to and why**. + +The short version: a peripheral's stage is decided by *what can fail*, not by +what is convenient. + +## The sequence + +Verified against the code rather than assumed; the ordering here is the sort of +thing that is easy to state backwards, and this repo has done so twice. + +| # | Where | What happens | +|---|---|---| +| 1 | [`startup.s`](../src/libs/board/stm32f767zi_nucleo/startup.s) | `sp = _estack`, copy `.data` from flash, zero `.bss` | +| 2 | `bl SystemInit` → [`cortex_m7.cpp`](../src/libs/mcu/arm_cm7/cortex_m7.cpp) | Enable the FPU (`CPACR`), point `VTOR` at flash. **Core registers only.** | +| 3 | `bl __libc_init_array` | Static constructors run. `g_board` is built, and with it every on-chip peripheral. | +| 4 | `bl main` → [`main.cpp`](../src/libs/board/stm32f767zi_nucleo/main.cpp) | `app::AppMain(g_board)` → `RunApp` | +| 5 | `Board::Init()` | `InitSysTick()`, then off-chip bring-up | +| 6 | `App::Init()` then `App::Run()` | Application. `Uart::Init` lives here. `Run()` never returns. | + +Two consequences that are easy to get wrong: + +- **`SystemInit` runs *after* `.data` and `.bss` are ready**, not before. It is + restricted to core registers because that is all it needs, not because memory + is unusable. +- **No code in this project configures clocks.** The F767 runs on HSI at 16 MHz + out of reset and stays there. `RCC` is live from reset, and each driver + enables its own peripheral clock as its first action — which is what makes + step 3 able to touch registers at all. + +## The stages + +| Stage | Runs at | Nature | Can fail? | Timeouts work? | +|---|---|---|---|---| +| 0 — Core | step 2 | Core registers | No | No | +| 1 — On-chip | step 3 | Register writes, no dependencies | **No** | **No** | +| 2 — Off-chip | step 5 | I/O across a wire | **Yes, routinely** | **Yes** | +| 3 — Runtime | step 6 | Application, interrupts | Yes | Yes | + +### Why "can it fail" is the dividing line + +Bringing up an on-chip peripheral is a fixed sequence of register writes against +hardware that is soldered into the die. There is nothing to be absent, nothing +to time out, nothing to report. That is what lets it happen in a constructor — +and a constructor is worth having, because it means an unconfigured peripheral +cannot be reached. See the 2026-09-06 decision-log entry in +[`PROJECT_PLAN.md`](PROJECT_PLAN.md). + +Bringing up an off-chip device is I/O. The device may be unpopulated, wrongly +strapped, held in reset, or simply slow. Failure is a *normal outcome*, so it +needs somewhere that can report one — which a constructor is not. + +### The timebase is the hard boundary + +`mcu::Millis()` does not advance until `InitSysTick()` runs in stage 2. Before +that: + +- **`mcu::Delay` works.** It checks `SysTickRunning()` and falls back to a + `DWT->CYCCNT` busy-wait ([`delay.cpp`](../src/libs/mcu/arm_cm7/delay.cpp)). An + early delay is imprecise and burns cycles, but it is not a hang. +- **Bounded waits do not.** `(Millis() - start) > timeout` can never become true + against a frozen counter, so a naive timeout loop spins forever. The drivers + therefore *refuse* one this early — `I2CBus`'s flag wait and + `Usart::Receive` with a non-zero timeout both return `kInvalidState` when the + tick is not running, rather than hanging. + +So a stage-1 constructor may wait, but it may not *talk to anything*, because +every real transfer needs a timeout. That is the whole reason off-chip bring-up +is stage 2. + +## Where does my peripheral go? + +``` +Is it inside the MCU? +├─ Yes → Stage 1. A board member; its constructor configures it. +│ GPIO, I2C/SPI/UART controllers, ADC, timers. +│ Must not fail. Must not do I/O. +│ +└─ No, it is across a wire → Stage 2. Brought up in Board::Init(). + Sensors, flash chips, radios, anything with its own part number. + May fail, and should say so. +``` + +The ADC is the case that shows the split is not "sensor or not": the **ADC +peripheral** is stage 1, while the **sensor wired to it** is stage 2. + +### Worked examples + +| Device | Stage 1 (constructor) | Stage 2 (`Board::Init()`) | +|---|---|---| +| I2C temp/humidity sensor | `I2CBus` on its pins | Probe the device, read its ID, configure sampling | +| SPI NOR flash | `SpiBus` + a chip-select `GpioPin` | Read the JEDEC ID, verify capacity | +| Bluetooth module on UART | `Usart` on its pins, reset-line `GpioPin` | Release reset, wait, exchange a command, check the reply | +| ADC-based sensor | The ADC peripheral | Reference/calibration; the reading itself is stage 3 | + +`mcu::Usart` is the exception in the table above and worth understanding: it is +on-chip but still two-phase, because only the *application* knows the +`UartConfig` it wants, and `Init()` validates that config and can fail. Being +on-chip makes a constructor *possible*, not mandatory. + +## Known gaps + +Named because they are load-bearing for what comes next, not because they block +anything today. + +- **`Board::Init()` cannot say which device failed.** It returns + `std::expected`, and `common::Error` carries no payload. + With one off-chip device that is tolerable; with four it is not. This needs + solving before stage 2 has real occupants. +- **There is nowhere to report a failure to.** `main.cpp` spins with the error + in a register for a debugger to find, because the console is a stage-3 + resource brought up by the application. A board that fails bring-up cannot + currently tell anyone. +- **An RTOS will want the tick.** FreeRTOS drives `SysTick` itself, and + conventionally starts some drivers after the kernel does. Stage 3 is where + that lands, and it will likely revisit step 5's ordering. +- **Stage 1 has no ordering mechanism between peripherals.** It is declaration + order in the board class, which is well defined but implicit. Fine for + independent peripherals; a problem the first time one depends on another. + +## What would change this design + +Deliberately written down so the next step is a decision rather than a drift. + +- **A timebase valid from reset** — building `Micros()` on `DWT->CYCCNT`, which + runs from reset and needs no interrupt — would make timeouts work in every + stage and remove the hard boundary above. Worth doing when a stage-1 + peripheral genuinely needs a bounded wait. It is a timeout source, not an + uptime clock: the counter wraps in ~268 s at 16 MHz. +- **Explicit init levels** (Zephyr's `PRE_KERNEL_1` / `POST_KERNEL`, or a + registry with priorities) would replace the two-phase split. The trigger is + concrete: **two off-chip devices with an ordering constraint between them.** + Until then it is ceremony, and this project would rather show the seam than + hide it behind a framework. diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 3215638..3ce9682 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -129,6 +129,40 @@ is the open part. ## Decision Log +### 2026-09-07: Boot flow named in two stages, split on fallibility + +- Wrote `docs/BOOT_FLOW.md`. The stage a peripheral belongs to is decided by + **what can fail**, not by what is convenient: on-chip bring-up is register + writes against hardware soldered into the die, so it cannot fail and lives in + a constructor; off-chip bring-up is I/O across a wire, where absence and + silence are normal outcomes, so it needs somewhere that can report one -- + `Board::Init()`. +- The structure this describes already existed; what was missing was the + contract. `board::Board` now states it where a reader meets it. +- Corrected a claim made in the 2026-09-06 entry above. `mcu::Delay` *does* + work before the tick -- it falls back to a `DWT->CYCCNT` spin -- so + "bring-up must not call mcu::Delay" was wrong. The real boundary is narrower + and sharper: `Millis()` is frozen until `InitSysTick()`, so a *bounded* wait + can never end. Since every real bus transfer needs a timeout, stage 1 may + wait but may not talk. +- That was a live latent hang, not just a doc error. `I2CBus`'s flag wait and + `Usart::Receive` compared against a frozen `Millis()` with no guard, so a + stuck bus before the tick would have spun forever. Both now refuse with + `kInvalidState`. Unreachable today -- both are called only after + `Board::Init()` -- and reachable the moment anything talks to a device during + bring-up, which is exactly what stage 2 invites. +- Rejected, for now, an init-level registry in the style of Zephyr's + `PRE_KERNEL_1`/`POST_KERNEL`. With one board and no off-chip devices it would + be a framework guarding a state that has not occurred. Its trigger is written + down instead: two off-chip devices with an ordering constraint between them. +- Deferred a reset-valid timebase (`Micros()` on `DWT->CYCCNT`), which would + make timeouts work in every stage and dissolve the boundary entirely. Worth + doing when a stage-1 peripheral needs a bounded wait, and not before. +- Known gaps recorded rather than solved: `common::Error` has no payload, so + `Board::Init()` cannot name which device failed; there is nowhere to report a + bring-up failure to, since the console is a stage-3 resource; and an RTOS + will want `SysTick` for itself. + ### 2026-09-06: Peripheral configuration moved into the constructor (#37, #38) - `mcu::GpioPin` and `mcu::I2CBus` now configure the hardware as they are diff --git a/src/libs/board/board.hpp b/src/libs/board/board.hpp index 71eac0d..c1a4b58 100644 --- a/src/libs/board/board.hpp +++ b/src/libs/board/board.hpp @@ -10,9 +10,24 @@ namespace board { +/// @brief Everything an application is given of the hardware it runs on. +/// +/// A board comes up in two phases, and which one a peripheral belongs to is +/// settled by whether it can fail, not by what is convenient: +/// +/// - **Construction** brings up what is inside the MCU. Register writes with +/// no dependencies, so they cannot fail and need no timebase. +/// - **Init()** brings up what is on the other side of a wire, and is +/// fallible for that reason: a device can be absent, wrongly strapped, or +/// simply not answer. It also starts the system tick, which is what makes +/// bounded waits -- and so any real bus transfer -- legal from here on. +/// +/// docs/BOOT_FLOW.md has the whole sequence and the rules for each stage. struct Board { virtual ~Board() = default; + /// Bring up everything off-chip and start the timebase the drivers need. + /// Must have returned successfully before any accessor below is used. [[nodiscard]] virtual auto Init() -> std::expected = 0; [[nodiscard]] virtual auto UserLed1() -> mcu::OutputPin& = 0; [[nodiscard]] virtual auto UserLed2() -> mcu::OutputPin& = 0; diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp index 81bec27..6ec4adc 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp @@ -32,8 +32,11 @@ namespace board { /// peripheral has to poll a status bit that can time out, it needs a /// separate call -- see mcu::Usart, which stays two-phase for this reason /// as well as because only the application knows its UartConfig. -/// 2. Bring-up must not call mcu::Delay. InitSysTick() runs from Init(), -/// after main(), so a constructor that waited on the tick would hang. +/// 2. Bring-up must not talk to anything across a wire. mcu::Delay is fine +/// -- before the tick it falls back to a cycle-counter spin -- but a +/// *timeout* is not: Millis() is frozen until Init() starts the tick, so +/// a bounded wait cannot end and the drivers refuse one this early. That +/// makes off-chip bring-up Init()'s job. See docs/BOOT_FLOW.md. /// 3. This board must stay the only object with a dynamic initializer. /// Construction order within it is declaration order and well defined; /// order across translation units is not. `.init_array` holding one entry diff --git a/src/libs/mcu/arm_cm7/i2c.cpp b/src/libs/mcu/arm_cm7/i2c.cpp index 23e092a..fb3c5f2 100644 --- a/src/libs/mcu/arm_cm7/i2c.cpp +++ b/src/libs/mcu/arm_cm7/i2c.cpp @@ -102,6 +102,14 @@ auto EnablePeripheralClock(I2CId id) -> void { /// missing device, which is worth telling apart from a wedged bus. [[nodiscard]] auto WaitForFlag(I2C_TypeDef* registers, std::uint32_t flag) -> std::expected { + // Millis() does not advance until the board starts the tick, so the timeout + // below could never fire and a bus held low would spin forever. Refuse + // instead: a transfer this early is a boot-order mistake, not a bus fault. + // See docs/BOOT_FLOW.md -- talking to a device is stage 2 work. + if (!SysTickRunning()) { + return std::unexpected(common::Error::kInvalidState); + } + const std::uint32_t start = Millis(); while ((registers->ISR & flag) == 0U) { if ((registers->ISR & I2C_ISR_NACKF) != 0U) { diff --git a/src/libs/mcu/arm_cm7/usart.cpp b/src/libs/mcu/arm_cm7/usart.cpp index d3a3068..63eadde 100644 --- a/src/libs/mcu/arm_cm7/usart.cpp +++ b/src/libs/mcu/arm_cm7/usart.cpp @@ -255,6 +255,12 @@ auto Usart::Receive(std::span buffer, std::uint32_t timeout_ms) if (buffer.empty()) { return 0U; } + // A bounded wait needs a running tick: Millis() is frozen until the board + // starts it, so the timeout below would never fire. Waiting forever is still + // allowed, since that asks for no timeout in the first place. + if (timeout_ms != 0 && !SysTickRunning()) { + return std::unexpected(common::Error::kInvalidState); + } auto* registers = Registers(id_); const std::uint32_t start = Millis(); From 16821d0c9571b1bb89cc78a2d80cf3edd7f530ac Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Mon, 7 Sep 2026 16:19:51 +0000 Subject: [PATCH 2/3] docs: bring the README's status up to date Three commits overtook it. UART and I2C were still listed as placeholders that return an error, the architecture section said hardware boards were planned, and the technology stack named host emulation as the only target. The status table now distinguishes implemented from verified, which is a distinction this project should be making: GPIO/EXTI/SysTick and USART3 have been confirmed on the physical board, I2C has not. That matches the open task in PROJECT_PLAN.md's Milestone 2 rather than quietly claiming more. Also adds a Documentation section. docs/ has three files and the README linked none of them except in passing. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c59ca3b..93ec06f 100644 --- a/README.md +++ b/README.md @@ -45,8 +45,10 @@ Application (apps/) → Board (libs/board/) → MCU (libs/mcu/) → Platfo ``` - **apps/**: Example applications (blinky, uart_echo, i2c_demo) -- **libs/mcu/**: Hardware abstractions (Pin, UART, I2C, Delay) with host emulation -- **libs/board/**: Board-specific implementations (host today; hardware boards planned) +- **libs/mcu/**: Hardware abstractions (Pin, UART, I2C, Delay), with a host + emulation backend and a Cortex-M7 one +- **libs/board/**: Board-specific implementations (the host emulator and the + STM32F767ZI Nucleo) - **py/host-emulator/**: Python hardware simulator for desktop testing ## Build Commands @@ -103,7 +105,7 @@ cd py/host-emulator && uv run host-emulator | Compilers | Clang 18 (host), ARM GCC (embedded) | | Testing | Google Test, pytest | | IPC | ZeroMQ + JSON | -| Targets | Host emulation (hardware targets planned) | +| Targets | Host emulation, STM32F767ZI Nucleo (Cortex-M7) | ## Code Quality @@ -123,10 +125,19 @@ cd py/host-emulator && uv run host-emulator | Docker/DevContainer | ✅ Working | | CI/CD | ✅ Working | | ARM cross-compile (Cortex-M7) | ✅ Working | -| STM32F767ZI Nucleo: GPIO, EXTI, SysTick | ✅ Working | -| STM32F767ZI Nucleo: UART, I2C | 🚧 Placeholders that return an error | +| STM32F767ZI Nucleo: GPIO, EXTI, SysTick | ✅ Verified on hardware | +| STM32F767ZI Nucleo: UART (USART3 on the ST-LINK VCP) | ✅ Verified on hardware | +| STM32F767ZI Nucleo: I2C | ✅ Implemented, not yet verified on hardware | | Other boards (STM32F3, nRF52) | 📋 Planned | +## Documentation + +- [docs/BOOT_FLOW.md](docs/BOOT_FLOW.md) — how a board comes up, and which + stage a new peripheral belongs to +- [docs/HARDWARE.md](docs/HARDWARE.md) — flashing, debugging and the pin map +- [docs/PROJECT_PLAN.md](docs/PROJECT_PLAN.md) — milestones, and a decision log + recording why things are the way they are + ## Resources - [Correct-by-Construction](https://youtu.be/nLSm3Haxz0I) From bba1cfcca623a93b992bc2a250f8d778fc78d0cd Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Mon, 7 Sep 2026 16:27:29 +0000 Subject: [PATCH 3/3] docs: anchor the boot-flow follow-ups to #43 and #44 The two gaps BOOT_FLOW.md recorded now have issues. Point at them, so the trigger for each is one click from the document that describes it -- the same treatment 4c141fd gave the peripheral-init decisions. Co-Authored-By: Claude Opus 5 (1M context) --- docs/BOOT_FLOW.md | 4 ++-- docs/PROJECT_PLAN.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/BOOT_FLOW.md b/docs/BOOT_FLOW.md index a5759d5..0116fbb 100644 --- a/docs/BOOT_FLOW.md +++ b/docs/BOOT_FLOW.md @@ -106,7 +106,7 @@ on-chip makes a constructor *possible*, not mandatory. Named because they are load-bearing for what comes next, not because they block anything today. -- **`Board::Init()` cannot say which device failed.** It returns +- **`Board::Init()` cannot say which device failed** (#43). It returns `std::expected`, and `common::Error` carries no payload. With one off-chip device that is tolerable; with four it is not. This needs solving before stage 2 has real occupants. @@ -125,7 +125,7 @@ anything today. Deliberately written down so the next step is a decision rather than a drift. -- **A timebase valid from reset** — building `Micros()` on `DWT->CYCCNT`, which +- **A timebase valid from reset** (#44) — building `Micros()` on `DWT->CYCCNT`, which runs from reset and needs no interrupt — would make timeouts work in every stage and remove the hard boundary above. Worth doing when a stage-1 peripheral genuinely needs a bounded wait. It is a timeout source, not an diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 3ce9682..2df10b9 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -155,11 +155,11 @@ is the open part. `PRE_KERNEL_1`/`POST_KERNEL`. With one board and no off-chip devices it would be a framework guarding a state that has not occurred. Its trigger is written down instead: two off-chip devices with an ordering constraint between them. -- Deferred a reset-valid timebase (`Micros()` on `DWT->CYCCNT`), which would +- Deferred a reset-valid timebase (`Micros()` on `DWT->CYCCNT`, #44), which would make timeouts work in every stage and dissolve the boundary entirely. Worth doing when a stage-1 peripheral needs a bounded wait, and not before. - Known gaps recorded rather than solved: `common::Error` has no payload, so - `Board::Init()` cannot name which device failed; there is nowhere to report a + `Board::Init()` cannot name which device failed (#43); there is nowhere to report a bring-up failure to, since the console is a stage-3 resource; and an RTOS will want `SysTick` for itself.