From f960c84f5bf73894fcfd67ba7e201fa3e8d590be Mon Sep 17 00:00:00 2001 From: Andrew Yong Date: Wed, 12 Aug 2026 14:29:52 +0800 Subject: [PATCH] fix(stm32wl): reset instead of hanging on faults (#11420) * fix(stm32wl): reset instead of hanging on faults Reset instead of hanging forever on three unrecoverable faults, each of which previously required a manual power cycle to recover: - HardFault_Handler_C: blinked SOS forever with no debugger attached; now resets once the fault registers are printed. - __wrap___assert_func: silently hung on an assert failure; now prints file/line/func/expr via debug_printf, then resets. - earlyBootCheck: silently hung if the jump into the bootloader ROM failed to take; now calls the bare NVIC_SystemReset(), not the HAL wrapper, since it runs pre-HAL_Init() and MSP/VTOR are already repointed at the bootloader by this point, so a return would unwind through a corrupted stack frame. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong * refactor(stm32wl): group fault-handling code Group the fault-handling code together and drop incidental cruft: - Move __wrap___assert_func next to HardFault_Handler_C and the other fault-reporting helpers. - Add banner comments separating linker-hack wrappers from fault-handling/recovery code, matching the existing Bootloader redirect banner. - Drop the forward declaration for debug_printf, no longer needed now that __wrap___assert_func sits below its definition. - Trim the Bootloader redirect banner comment to 1-3 lines. No behavior change. Assisted-by: Claude Sonnet 5 Signed-off-by: Andrew Yong --------- Signed-off-by: Andrew Yong --- src/platform/stm32wl/main-stm32wl.cpp | 80 ++++++--------------------- 1 file changed, 16 insertions(+), 64 deletions(-) diff --git a/src/platform/stm32wl/main-stm32wl.cpp b/src/platform/stm32wl/main-stm32wl.cpp index a2fed8357..50f7ae3d8 100644 --- a/src/platform/stm32wl/main-stm32wl.cpp +++ b/src/platform/stm32wl/main-stm32wl.cpp @@ -18,20 +18,9 @@ static bool stm32wlRtcValid = false; #endif // ─── Bootloader redirect ────────────────────────────────────────────────────── -// -// Why .noinit + constructor instead of TAMP backup registers: -// -// The STM32duino startup sequence initialises clocks which may call -// __HAL_RCC_BACKUPRESET_FORCE/RELEASE when configuring the LSE oscillator, -// wiping the entire backup domain (including TAMP->BKP0R) before setup() -// ever runs. The backup-register approach therefore cannot reliably survive -// a soft reset in this toolchain. -// -// Solution: store the magic in a .noinit SRAM variable. -// - NVIC_SystemReset() does NOT clear SRAM. -// - The linker script skips zero-init for .noinit sections. -// - __attribute__((constructor)) fires before main()/HAL_Init(), so we can -// intercept and jump before anything disturbs peripheral state. +// Uses .noinit SRAM instead of TAMP backup registers: STM32duino's clock init can wipe the +// backup domain via __HAL_RCC_BACKUPRESET_FORCE/RELEASE before setup() runs, but .noinit +// survives NVIC_SystemReset() and this constructor fires before HAL_Init() touches anything. #define BOOTLOADER_MAGIC 0xD00DB007UL #define SYS_MEM_BASE 0x1FFF0000UL @@ -58,8 +47,10 @@ __attribute__((constructor(101), used)) static void earlyBootCheck(void) SCB->VTOR = SYS_MEM_BASE; __set_MSP(*(volatile uint32_t *)SYS_MEM_BASE); ((void (*)(void))(*(volatile uint32_t *)(SYS_MEM_BASE + 4)))(); - while (1) - ; + // Should never be reached: the bootloader ROM does not return. A bare reset + // (rather than returning normally) avoids unwinding through this function's + // epilogue, which would restore registers relative to the now-repointed MSP. + NVIC_SystemReset(); } void enterDfuMode() @@ -169,15 +160,7 @@ void cpuDeepSleep(uint32_t msecToWake) #endif } -// Hacks to force more code and data out. - -// By default __assert_func uses fiprintf which pulls in stdio. -extern "C" void __wrap___assert_func(const char *, int, const char *, const char *) -{ - while (true) - ; - return; -} +// ─── Linker hacks to reduce code size ───────────────────────────────────────── // By default strerror has a lot of strings we probably don't use. Make it return an empty string instead. char empty = 0; @@ -197,6 +180,8 @@ extern "C" void __wrap__tzset_unlocked_r(struct _reent *reent_ptr) } #endif +// ─── Fault handling & recovery ──────────────────────────────────────────────── + // Taken from https://interrupt.memfault.com/blog/cortex-m-hardfault-debug typedef struct __attribute__((packed)) ContextStateFrame { uint32_t r0; @@ -233,32 +218,11 @@ static void debug_printf(const char *format, ...) uart_debug_write((uint8_t *)hardfault_message_buffer, min((unsigned int)length, sizeof(hardfault_message_buffer) - 1)); } -// N picked by guessing -#define DOT_TIME 1200000 -static void dot() +// By default __assert_func uses fiprintf which pulls in stdio. +extern "C" void __wrap___assert_func(const char *file, int line, const char *func, const char *failedexpr) { - digitalWrite(LED_POWER, LED_STATE_ON); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } - digitalWrite(LED_POWER, LED_STATE_OFF); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } -} - -static void dash() -{ - digitalWrite(LED_POWER, LED_STATE_ON); - for (volatile int i = 0; i < (DOT_TIME * 3); i++) { /* busy wait */ - } - digitalWrite(LED_POWER, LED_STATE_OFF); - for (volatile int i = 0; i < DOT_TIME; i++) { /* busy wait */ - } -} - -static void space() -{ - for (volatile int i = 0; i < (DOT_TIME * 3); i++) { /* busy wait */ - } + debug_printf("assert: %s:%d in %s: %s\r\n", file, line, func, failedexpr); + HAL_NVIC_SystemReset(); } // Disable optimizations for this function so "frame" argument @@ -277,17 +241,5 @@ extern "C" __attribute__((optimize("O0"))) void HardFault_Handler_C(sContextStat HALT_IF_DEBUGGING(); - // blink SOS forever - while (1) { - dot(); - dot(); - dot(); - dash(); - dash(); - dash(); - dot(); - dot(); - dot(); - space(); - } -} \ No newline at end of file + HAL_NVIC_SystemReset(); +}