From 5ff90081a88cf25299de2427c54ea06a1df40ffa Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 14 Apr 2026 10:43:01 +1000 Subject: [PATCH 1/2] stm32/powerctrl: Add bounded waits and HSEM to WB55 MSI clock path. The MSI clock path in powerctrl_set_sysclk() (used by STM32WB and STM32WL) has six unbounded while loops that permanently hang CPU1 if any hardware flag fails to assert. On STM32WB55, it also modifies RCC registers without acquiring the RCC hardware semaphore, and does not check CPU2's power state before reducing SYSCLK below 32 MHz. Add a tick-based RCC_WAIT(cond, timeout_ms, result) macro using mp_hal_ticks_ms and replace all unbounded while loops. Timeout values match the STM32 HAL: 2ms for MSI/PLL/VOS, 100ms for HSE, 500ms for HSEM, 5000ms for clock switch. On timeout, return -MP_ETIMEDOUT. The macro evaluates cond at most once per iteration and breaks immediately on success, avoiding re-evaluation of side-effecting conditions like LL_HSEM_1StepLock (which performs a hardware lock attempt on every RLR register read). On STM32WB, acquire CFG_HW_RCC_SEMID around RCC modifications, matching SystemClock_Config() and powerctrl_low_power_prep_wb55(). Check C2DS/C2SB before SYSCLK < 32 MHz; wait up to 1 second for CPU2 to enter deep sleep, returning -MP_EPERM if it doesn't. The error is raised before any RCC modification, so SystemCoreClock and SysTick remain correct and the caller can retry. Also fix REGLPF wait polarity: the original code waited while the flag was clear (main regulator ready), matching neither the HAL implementation nor the RM0434 description. Now waits while set (low-power regulator still active), consistent with the STM32 HAL HAL_PWREx_DisableLowPowerRunMode(). Extend mp_machine_set_freq() to raise OSError for the new error codes. References: - RM0434 Rev 10 Section 7.2.17: RCC register access through HSEM - AN5289 Rev 8 Section 4.3: CPU2 clock requirements (HCLK2 >= 32 MHz) Signed-off-by: Andrew Leech --- ports/stm32/modmachine.c | 4 +++ ports/stm32/powerctrl.c | 72 ++++++++++++++++++++++++++++++++++++---- ports/stm32/powerctrl.h | 17 +++++++++- 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/ports/stm32/modmachine.c b/ports/stm32/modmachine.c index c1ac6e0862d28..66d870121055b 100644 --- a/ports/stm32/modmachine.c +++ b/ports/stm32/modmachine.c @@ -383,6 +383,10 @@ static void mp_machine_set_freq(size_t n_args, const mp_obj_t *args) { int ret = powerctrl_set_sysclk(sysclk, ahb, apb1, apb2); if (ret == -MP_EINVAL) { mp_raise_ValueError(MP_ERROR_TEXT("invalid freq")); + } else if (ret == -MP_ETIMEDOUT) { + mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("freq change timeout")); + } else if (ret == -MP_EPERM) { + mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("CPU2 active")); } else if (ret < 0) { MICROPY_BOARD_FATAL_ERROR("can't change freq"); } diff --git a/ports/stm32/powerctrl.c b/ports/stm32/powerctrl.c index a63c57f4a78bb..233f2f7b5f021 100644 --- a/ports/stm32/powerctrl.c +++ b/ports/stm32/powerctrl.c @@ -656,7 +656,10 @@ int powerctrl_set_sysclk(uint32_t sysclk, uint32_t ahb, uint32_t apb1, uint32_t // Exit LPR and wait for the regulator to be ready. LL_PWR_ExitLowPowerRunMode(); - while (!LL_PWR_IsActiveFlag_REGLPF()) { + int fail; + RCC_WAIT(LL_PWR_IsActiveFlag_REGLPF(), 2, fail); + if (fail) { + return -MP_ETIMEDOUT; } } } @@ -664,32 +667,74 @@ int powerctrl_set_sysclk(uint32_t sysclk, uint32_t ahb, uint32_t apb1, uint32_t // Select VOS1 if SYSCLK will increase beyond threshold. if (sysclk > VOS2_THRESHOLD) { LL_PWR_SetRegulVoltageScaling(LL_PWR_REGU_VOLTAGE_SCALE1); - while (LL_PWR_IsActiveFlag_VOS()) { + int fail; + RCC_WAIT(LL_PWR_IsActiveFlag_VOS(), 2, fail); + if (fail) { + return -MP_ETIMEDOUT; } } if (sysclk_mode == SYSCLK_MODE_HSE_64M) { - SystemClock_Config(); + int ret = SystemClock_Config(); + if (ret < 0) { + return ret; + } } else if (sysclk_mode == SYSCLK_MODE_MSI) { + int fail; + + #if defined(STM32WB) + // Acquire the RCC semaphore, matching SystemClock_Config() and + // powerctrl_low_power_prep_wb55(). RM0434 Section 7.2.17. + RCC_WAIT(LL_HSEM_1StepLock(HSEM, CFG_HW_RCC_SEMID), 500, fail); + if (fail) { + return -MP_ETIMEDOUT; + } + + // CPU2 requires HCLK2 >= 32 MHz when awake (AN5289 Section 4.3). + // C2HPRE is a divider, so HCLK2 can never exceed SYSCLK; there is + // no way to maintain 32 MHz HCLK2 from a lower SYSCLK. Wait up + // to 1 second for CPU2 to enter deep sleep before giving up. + // Returning EPERM here is safe: no RCC registers have been modified + // yet, so SystemCoreClock and SysTick remain correct. The caller + // can retry after a delay. + if (sysclk < 32000000 + && !LL_PWR_IsActiveFlag_C2DS() + && !LL_PWR_IsActiveFlag_C2SB()) { + RCC_WAIT(!(LL_PWR_IsActiveFlag_C2DS() || LL_PWR_IsActiveFlag_C2SB()), 1000, fail); + if (fail) { + LL_HSEM_ReleaseLock(HSEM, CFG_HW_RCC_SEMID, 0); + return -MP_EPERM; + } + } + #endif + // Set flash latency to maximum to ensure the latency is large enough for // both the current SYSCLK and the SYSCLK that will be selected below. LL_FLASH_SetLatency(FLASH_LATENCY_MAX); - while (LL_FLASH_GetLatency() != FLASH_LATENCY_MAX) { + RCC_WAIT(LL_FLASH_GetLatency() != FLASH_LATENCY_MAX, 2, fail); + if (fail) { + goto msi_fail; } // Before changing the MSIRANGE value, if MSI is on then it must also be ready. - while ((RCC->CR & (RCC_CR_MSIRDY | RCC_CR_MSION)) == RCC_CR_MSION) { + RCC_WAIT((RCC->CR & (RCC_CR_MSIRDY | RCC_CR_MSION)) == RCC_CR_MSION, 2, fail); + if (fail) { + goto msi_fail; } LL_RCC_MSI_SetRange(msirange << RCC_CR_MSIRANGE_Pos); // Clock SYSCLK from MSI. LL_RCC_SetSysClkSource(LL_RCC_SYS_CLKSOURCE_MSI); - while (LL_RCC_GetSysClkSource() != LL_RCC_SYS_CLKSOURCE_STATUS_MSI) { + RCC_WAIT(LL_RCC_GetSysClkSource() != LL_RCC_SYS_CLKSOURCE_STATUS_MSI, 5000, fail); + if (fail) { + goto msi_fail; } // Disable PLL to decrease power consumption. LL_RCC_PLL_Disable(); - while (LL_RCC_PLL_IsReady() != 0) { + RCC_WAIT(LL_RCC_PLL_IsReady() != 0, 2, fail); + if (fail) { + goto msi_fail; } LL_RCC_PLL_DisableDomain_SYS(); @@ -709,6 +754,19 @@ int powerctrl_set_sysclk(uint32_t sysclk, uint32_t ahb, uint32_t apb1, uint32_t // Update HAL state and SysTick. SystemCoreClockUpdate(); powerctrl_config_systick(); + + #if defined(STM32WB) + LL_HSEM_ReleaseLock(HSEM, CFG_HW_RCC_SEMID, 0); + #endif + } + + // Error cleanup for MSI path (entered via goto msi_fail). + if (0) { + msi_fail: + #if defined(STM32WB) + LL_HSEM_ReleaseLock(HSEM, CFG_HW_RCC_SEMID, 0); + #endif + return -MP_ETIMEDOUT; } // Return straight away if the clocks are already at the desired frequency. diff --git a/ports/stm32/powerctrl.h b/ports/stm32/powerctrl.h index 724ab58366f4a..40ddd7bdbbaa8 100644 --- a/ports/stm32/powerctrl.h +++ b/ports/stm32/powerctrl.h @@ -43,7 +43,22 @@ static inline void stm32_system_init(void) { } #endif -void SystemClock_Config(void); +int SystemClock_Config(void); + +// Bounded spin-wait for clock/power flag transitions. Timeout values +// should match the STM32 HAL for the relevant family: HSE 100ms, PLL 2ms, +// MSI 2ms, clock switch 5000ms, HSEM 500ms. +// Sets result to 1 on timeout, 0 on success. Evaluates cond at most once +// per iteration (no post-loop re-evaluation) so it is safe with +// side-effecting conditions like LL_HSEM_1StepLock. +#define RCC_WAIT(cond, timeout_ms, result) \ + do { \ + uint32_t _t0 = mp_hal_ticks_ms(); \ + (result) = 1; \ + while (mp_hal_ticks_ms() - _t0 < (timeout_ms)) { \ + if (!(cond)) { (result) = 0; break; } \ + } \ + } while (0) MP_NORETURN void powerctrl_mcu_reset(void); MP_NORETURN void powerctrl_enter_bootloader(uint32_t r0, uint32_t bl_addr); From 148b3971d90272c93d5b562698fcd514c2f6b5b8 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Tue, 14 Apr 2026 10:43:14 +1000 Subject: [PATCH 2/2] stm32/powerctrlboot: Add bounded waits to WB55 SystemClock_Config. The STM32WB SystemClock_Config() (64 MHz PLL path) has four unbounded while loops for HSEM acquire, HSE ready, PLL lock, and SYSCLK switch. Apply RCC_WAIT with appropriate timeouts to all four loops. Use separate cleanup labels (fail_clk48, fail_rcc) to release both CFG_HW_CLK48_CONFIG and CFG_HW_RCC semaphores correctly on post-CLK48-acquire failures. Change SystemClock_Config() from void to int across all STM32 targets for API consistency. Non-WB implementations return 0 unconditionally. Signed-off-by: Andrew Leech --- ports/stm32/main.c | 4 ++- ports/stm32/mboot/main.c | 8 +++-- ports/stm32/mboot/mboot.h | 4 +-- ports/stm32/powerctrlboot.c | 62 ++++++++++++++++++++++++++++--------- ports/stm32/system_stm32.c | 4 ++- 5 files changed, 61 insertions(+), 21 deletions(-) diff --git a/ports/stm32/main.c b/ports/stm32/main.c index 17111c6df983e..5d3cd7eb93822 100644 --- a/ports/stm32/main.c +++ b/ports/stm32/main.c @@ -435,7 +435,9 @@ void stm32_main(uint32_t reset_mode) { HAL_InitTick(TICK_INT_PRIORITY); // set the system clock to be HSE - SystemClock_Config(); + if (SystemClock_Config() < 0) { + MICROPY_BOARD_FATAL_ERROR("SystemClock_Config"); + } #if defined(STM32N6) risaf_init(); diff --git a/ports/stm32/mboot/main.c b/ports/stm32/mboot/main.c index 7251bc1562e90..84ea5c5df48ad 100644 --- a/ports/stm32/mboot/main.c +++ b/ports/stm32/mboot/main.c @@ -195,7 +195,7 @@ void systick_init(void) { #if defined(STM32F4) || defined(STM32F7) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // This function assumes that HSI is used as the system clock (see RCC->CFGR, SWS bits) // Enable Power Control clock @@ -269,11 +269,13 @@ void SystemClock_Config(void) { // whether we were started from DFU or from a power on reset. RCC->DCKCFGR2 = 0; #endif + + return 0; } #elif defined(STM32H7) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // This function assumes that HSI is used as the system clock (see RCC->CFGR, SWS bits) // Select VOS level as high voltage to give reliable operation @@ -362,6 +364,8 @@ void SystemClock_Config(void) { // Update clock value and reconfigure systick now that the frequency changed SystemCoreClockUpdate(); systick_init(); + + return 0; } #endif diff --git a/ports/stm32/mboot/mboot.h b/ports/stm32/mboot/mboot.h index bc1320c5cdb36..99f92c3018bf2 100644 --- a/ports/stm32/mboot/mboot.h +++ b/ports/stm32/mboot/mboot.h @@ -200,7 +200,7 @@ extern int32_t first_writable_flash_sector; void systick_init(void); void led_init(void); void mboot_ui_systick(void); -void SystemClock_Config(void); +int SystemClock_Config(void); uint32_t get_le32(const uint8_t *b); uint64_t get_le64(const uint8_t *b); @@ -224,7 +224,7 @@ static inline void mboot_entry_init_default(void) { #endif // set the system clock to be HSE - SystemClock_Config(); + (void)SystemClock_Config(); #if defined(STM32H7) // Ensure IRQs are enabled (needed coming out of ST bootloader on H7) diff --git a/ports/stm32/powerctrlboot.c b/ports/stm32/powerctrlboot.c index 7baaa6773209a..8b7b343a1f067 100644 --- a/ports/stm32/powerctrlboot.c +++ b/ports/stm32/powerctrlboot.c @@ -24,6 +24,7 @@ * THE SOFTWARE. */ +#include "py/mperrno.h" #include "py/mphal.h" #include "irq.h" #include "powerctrl.h" @@ -66,7 +67,7 @@ void powerctrl_config_systick(void) { #if defined(STM32F0) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Enable power control peripheral __HAL_RCC_PWR_CLK_ENABLE(); @@ -126,11 +127,13 @@ void SystemClock_Config(void) { SystemCoreClockUpdate(); powerctrl_config_systick(); + + return 0; } #elif defined(STM32G0) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Enable power control peripheral __HAL_RCC_PWR_CLK_ENABLE(); @@ -191,11 +194,13 @@ void SystemClock_Config(void) { | __HAL_RCC_CRS_RELOADVALUE_CALCULATE(48000000, 1000) << CRS_CFGR_RELOAD_Pos; #endif #endif + + return 0; } #elif defined(STM32H5) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Set power voltage scaling. __HAL_PWR_VOLTAGESCALING_CONFIG(PWR_REGULATOR_VOLTAGE_SCALE0); while (!__HAL_PWR_GET_FLAG(PWR_FLAG_VOSRDY)) { @@ -303,11 +308,13 @@ void SystemClock_Config(void) { #ifdef NDEBUG DBGMCU->CR = 0; #endif + + return 0; } #elif defined(STM32L0) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Enable power control peripheral __HAL_RCC_PWR_CLK_ENABLE(); @@ -356,11 +363,13 @@ void SystemClock_Config(void) { | __HAL_RCC_CRS_RELOADVALUE_CALCULATE(48000000, 1000) << CRS_CFGR_RELOAD_Pos; #endif #endif + + return 0; } #elif defined(STM32L1) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Enable power control peripheral __HAL_RCC_PWR_CLK_ENABLE(); @@ -412,11 +421,13 @@ void SystemClock_Config(void) { #if !defined(NDEBUG) DBGMCU->CR &= ~(DBGMCU_CR_DBG_SLEEP | DBGMCU_CR_DBG_STOP | DBGMCU_CR_DBG_STANDBY); #endif + + return 0; } #elif defined(STM32N6) -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Enable HSI. LL_RCC_HSI_Enable(); while (!LL_RCC_HSI_IsReady()) { @@ -534,24 +545,34 @@ void SystemClock_Config(void) { // Reconfigure clock state and SysTick. SystemCoreClockUpdate(); powerctrl_config_systick(); + + return 0; } #elif defined(STM32WB) -void SystemClock_Config(void) { - while (LL_HSEM_1StepLock(HSEM, CFG_HW_RCC_SEMID)) { +int SystemClock_Config(void) { + int fail; + + RCC_WAIT(LL_HSEM_1StepLock(HSEM, CFG_HW_RCC_SEMID), 500, fail); + if (fail) { + return -MP_ETIMEDOUT; } // Enable the 32MHz external oscillator RCC->CR |= RCC_CR_HSEON; - while (!(RCC->CR & RCC_CR_HSERDY)) { + RCC_WAIT(!(RCC->CR & RCC_CR_HSERDY), 100, fail); + if (fail) { + goto fail_rcc; } // Prevent CPU2 from disabling CLK48. // This semaphore protected access to the CLK48 configuration. // CPU1 should hold this semaphore while the USB peripheral is in use. // See AN5289 and https://github.com/micropython/micropython/issues/6316. - while (LL_HSEM_1StepLock(HSEM, CFG_HW_CLK48_CONFIG_SEMID)) { + RCC_WAIT(LL_HSEM_1StepLock(HSEM, CFG_HW_CLK48_CONFIG_SEMID), 500, fail); + if (fail) { + goto fail_rcc; } // Use HSE and the PLL to get a 64MHz SYSCLK @@ -566,8 +587,9 @@ void SystemClock_Config(void) { | (PLLM - 1) << RCC_PLLCFGR_PLLM_Pos | 3 << RCC_PLLCFGR_PLLSRC_Pos; RCC->CR |= RCC_CR_PLLON; - while (!(RCC->CR & RCC_CR_PLLRDY)) { - // Wait for PLL to lock + RCC_WAIT(!(RCC->CR & RCC_CR_PLLRDY), 2, fail); + if (fail) { + goto fail_clk48; } const uint32_t sysclk_src = 3; @@ -579,8 +601,9 @@ void SystemClock_Config(void) { // Select SYSCLK source RCC->CFGR |= sysclk_src << RCC_CFGR_SW_Pos; - while (((RCC->CFGR >> RCC_CFGR_SWS_Pos) & 0x3) != sysclk_src) { - // Wait for SYSCLK source to change + RCC_WAIT(((RCC->CFGR >> RCC_CFGR_SWS_Pos) & 0x3) != sysclk_src, 5000, fail); + if (fail) { + goto fail_clk48; } // Select PLLQ as 48MHz source for USB and RNG @@ -591,13 +614,20 @@ void SystemClock_Config(void) { // Release RCC semaphore LL_HSEM_ReleaseLock(HSEM, CFG_HW_RCC_SEMID, 0); + return 0; + +fail_clk48: + LL_HSEM_ReleaseLock(HSEM, CFG_HW_CLK48_CONFIG_SEMID, 0); +fail_rcc: + LL_HSEM_ReleaseLock(HSEM, CFG_HW_RCC_SEMID, 0); + return -MP_ETIMEDOUT; } #elif defined(STM32WL) #include "stm32wlxx_ll_utils.h" -void SystemClock_Config(void) { +int SystemClock_Config(void) { // Set flash latency (2 wait states, sysclk > 36MHz) LL_FLASH_SetLatency(LL_FLASH_LATENCY_2); while (LL_FLASH_GetLatency() != LL_FLASH_LATENCY_2) { @@ -688,6 +718,8 @@ void SystemClock_Config(void) { SystemCoreClockUpdate(); powerctrl_config_systick(); + + return 0; } #endif diff --git a/ports/stm32/system_stm32.c b/ports/stm32/system_stm32.c index 5e5dca3ce8d8b..564944418554b 100644 --- a/ports/stm32/system_stm32.c +++ b/ports/stm32/system_stm32.c @@ -162,7 +162,7 @@ * * Timers run from APBx if APBx_PRESC=1, else 2x APBx */ -MP_WEAK void SystemClock_Config(void) { +MP_WEAK int SystemClock_Config(void) { #if defined(STM32F7) // The DFU bootloader changes the clocksource register from its default power // on reset value, so we set it back here, so the clocksources are the same @@ -693,6 +693,8 @@ MP_WEAK void SystemClock_Config(void) { #ifdef MICROPY_HW_ANALOG_SWITCH_PC3 HAL_SYSCFG_AnalogSwitchConfig(SYSCFG_SWITCH_PC3, MICROPY_HW_ANALOG_SWITCH_PC3); #endif + + return 0; } #endif