zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.88k stars 6.63k forks source link

pm: lptim: stm32h7: pending irq stops STANDBY #43372

Closed emillindq closed 2 years ago

emillindq commented 2 years ago

Describe the bug Recently LPTIM1 was selected to handle the clock APIs on STM32H7 MCUs (https://github.com/zephyrproject-rtos/zephyr/pull/41867). Since this driver doesn't have any pm support, upon entering system STANDBY (which I have provided myself for now since it's still WIP) the system stops executing after calling HAL_PWR_EnterSTANDBYMode() but it doesn't go into STANDBY according to the current draw and I am unable to wake it using the programmed WKUP pin.

After looking into the NVIC's ISPR reg, it turns out an interrupt from LPTIM1 is pending, and it also turns out the core won't go into STANDBY if there's any pending interrupts. If I manually disable LPTIM1 & clear the pending interrupts, it goes into STANDBY.

I think there should be some kind of power management in the LPTIM driver, which causes it do deinit on SOFT_OFF PM state. I also think the entire lptim driver is not implemented as it should since any other driver use the #define DT_DRV_COMPAT blaha and then DT_INST to get the driver's node. This also requires the driver to use the DEVICE_DT_INST_DEFINE which can receive a PM handler.

I'd love to get @erwango & @benediktibk feedback on this, and a possible solution I can implement.

Thanks!

To Reproduce

  1. Implement the pm_power_state_set function, add a busy-wait delay to make sure LPTIM1 interrupt comes, configure any WKUP pin, then call HAL_PWR_EnterSTANDBYMode()
  2. Implement pm_policy_next_state and say next state should be PM_STATE_SOFT_OFF
  3. Now try to wake up the MCU, not possible
  4. Remove delay, probably will be possible since interrupt hasn't come yet

Expected behavior LPTIM PM handler is called by PM when suspending the system and it should deinit the peripheral and clear pending interrupts

Impact No impact since I can just deinit it manuallly, but this must be addressed in the proper way.

Environment (please complete the following information):

Additional context I'm using the STM32H743BIT6 MCU

benediktibk commented 2 years ago

Hi @emillindq, I already went a little bit down this road of implement the power management on the H7 series. Unfortunately, this also differs within the H7 series, specifically the VOS switch. Combined with my current lack of time to continue on this task I am stuck at this point: https://github.com/zephyrproject-rtos/zephyr/pull/41869 https://github.com/zephyrproject-rtos/zephyr/pull/41871

I'm planning to continue this work (which I assume would also cover your issue), but so far I did not have the chance to do so.

In general I am confused of the issue which you have. For applications using power management the tickless kernel should be used, if I understood this correctly. In this case then LPTIM shouldn't fire and stop the system from going into standby.

emillindq commented 2 years ago

@benediktibk Thanks for your reply! Maybe I'm misunderstanding something, but with tickless kernel I am unable to sleep in the application since the kernel has no sense of time right?

I am using PM and ticks, and the reason for me to use PM is simply because I want to be able to suspend/resume devices and put the MCU in STANDBY using the PM API. I don't have a super-duper power-restricted system, I just want to be able to go into STANDBY :smile:

FYI and other stumbling across this issue, I solved my problem by

            /* Reset master switch for peripherals */
            HAL_DeInit();
            /* Disable NVIC interrupts */
            for (uint8_t i = 0; i < ARRAY_SIZE(NVIC->ICER); i++)
            {
                NVIC->ICER[i] = 0xFFFFFFFF;
            }
            /* Clear pending NVIC interrupts */
            for (uint8_t i = 0; i < ARRAY_SIZE(NVIC->ICPR); i++)
            {
                NVIC->ICPR[i] = 0xFFFFFFFF;
            }
benediktibk commented 2 years ago

Zephyr still has a sense of time with a tickless kernel, it just schedules the next wakeup time during idle to be whenever it will be necessary, e.g. when the next timeout of a thread will occur.