stm32/powerctrl: Add bounded waits, HSEM and CPU2 checks to STM32WB clock paths.#36
Open
andrewleech wants to merge 2 commits into
Open
stm32/powerctrl: Add bounded waits, HSEM and CPU2 checks to STM32WB clock paths.#36andrewleech wants to merge 2 commits into
andrewleech wants to merge 2 commits into
Conversation
f88930b to
dd6d4a1
Compare
|
Code size report: |
eb38f70 to
e14da59
Compare
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 <andrew.leech@planetinnovation.com.au>
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 <andrew.leech@planetinnovation.com.au>
e14da59 to
148b397
Compare
ce2c0c9 to
9f396bb
Compare
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 free
to 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.
Summary
I've been hitting intermittent WDT resets on a custom STM32WB55 board that changes SYSCLK at runtime via
machine.freq(). The device runs at 4 MHz MSI in a low-power idle state and boosts to 8/16 MHz for data collection and file I/O. The lockup was always silent, the last log message showed a successful frequency change then nothing until the watchdog fired.Digging into it, the MSI clock path in
powerctrl_set_sysclk()has six unboundedwhileloops waiting on hardware flags (regulator ready, VOS, flash latency, MSI ready, clock switch, PLL shutdown).SystemClock_Config()has another four (HSEM, HSE, PLL, clock switch). If any flag doesn't assert the CPU just spins forever.The MSI path is also missing a couple of protections that the PLL path and the stop-mode path already have. It modifies RCC registers without acquiring
CFG_HW_RCC_SEMID, so CPU2's wireless stack sequencer can collide on the same registers. And it doesn't check CPU2's power state before dropping SYSCLK below 32 MHz. The wireless stack needs HCLK2 >= 32 MHz when CPU2 is running (AN5289 Section 4.3), and no C2HPRE prescaler can multiply a sub-32 MHz SYSCLK up to that. If CPU2 hasn't entered deep sleep after BLE deactivation (the latency is unspecified per ST's docs), the HCLK2 violation can crash the wireless stack and leave HSEM semaphores held indefinitely, deadlocking CPU1.I ran a stress test cycling the device between temperature states that trigger these frequency transitions. Before this fix, 5 WDT resets in ~80 cycles over 10 minutes. After, 154 cycles over 20 minutes with zero clock-related resets.
This PR:
RCC_WAITbounded countdown macro inpowerctrl.h. Uses an iteration count rather thanHAL_GetTickbecause SysTick rate is changing during the clock transition itself (same approach asADC_WAITinmachine_adc.c). At 2 MHz MSI, 100000 iterations is ~200 ms.RCC_WAIT, returning-MP_ETIMEDOUTon timeout instead of hanging.CFG_HW_RCC_SEMIDaround the MSI path's RCC modifications on STM32WB, matchingSystemClock_Config()andpowerctrl_low_power_prep_wb55().C2DS/C2SBbefore SYSCLK < 32 MHz on STM32WB, returns-MP_EPERMif CPU2 is awake.SystemClock_Config()to returnintso failures propagate. Split cleanup labels (fail_clk48,fail_rcc) release both CLK48 and RCC semaphores correctly.mp_machine_set_freq()raisesOSErrorfor the new error codes instead of callingMICROPY_BOARD_FATAL_ERROR.References:
Testing
Tested on a custom STM32WB55 board with asyncio app, BLE wireless stack v1.22, and hardware watchdog.
Before: stress test cycling between 4 MHz idle and 8-16 MHz active, 5 WDT resets in 10 minutes (~80 cycles). Every lockup was during
machine.freq()itself, before any application code ran at the new frequency.After: same test, 154 cycles over 20 minutes, zero clock-related WDT resets. Both successful data collection cycles completed the full 8 -> 16 -> 8 MHz sequence cleanly.
Also verified:
machine.freq(4000000)with BLE active raisesOSError("can't change freq")as expected (CPU2 awake)SystemClock_Configstill works normallyTrade-offs and Alternatives
The
RCC_WAIT(cond); if (cond)pattern evaluates the condition twice at the boundary. For volatile register reads this is just a benign re-read. ForLL_HSEM_1StepLockthe second call on an already-held semaphore returns success (same-core re-lock per STM32WB HSEM behaviour), so it's safe but does one redundant MMIO access per successful acquire.Non-WB
SystemClock_Configimplementations (F0, G0, H5, L0, L1, N6, WL) still have unbounded waits in their own oscillator/PLL loops. They're updated to returnintfor API consistency but don't useRCC_WAIT. The WL MSI path does get the bounded waits though since it shares thepowerctrl_set_sysclkcode with WB.Generative AI
I used generative AI tools when creating this PR, but a human has checked the code and is responsible for the description above.