stm32/uart: Enable UART clock in low power modes.

[iabdalkader]

Summary

Prevents UART clock from stopping in low power modes (such as WFI). This is needed particularly on the N6, without it Bluetooth fails with:

[CYW43] cyw43_bluetooth_download_firmware: unexpected response 01

Testing

Only tested on N6, I don't think it has much effect on other MCUs as they don't seem to gate the clocks during WFI like the N6 does.

Trade-offs and Alternatives

Could only enable it for N6.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:   +80 +0.020% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

[dpgeorge]

I'm not sure why this is needed? In stm32_main() there are the following lines which enabled sleep for all UARTs (and other peripherals):

    // Enable some APB peripherals during sleep.
    LL_APB1_GRP1_EnableClockLowPower(0xffffffff); // I2C, I3C, LPTIM, SPI, TIM, UART, WWDG
    LL_APB2_GRP1_EnableClockLowPower(0xffffffff); // SAI, SPI, TIM, UART
    LL_APB4_GRP1_EnableClockLowPower(0xffffffff); // I2C, LPTIM, LPUART, RTC, SPI

(Note that the change here does impact pretty much all MCUs, because they define these macros.)

[iabdalkader]

I'm not sure why this is needed? In stm32_main() there are the following lines which enabled sleep for all UARTs (and other peripherals):

We don’t use the same main.c, so we don’t have that code, but I could easily add it. However, one problem with that approach is that there are no corresponding CLK_SLEEP_DISABLE()'s, so in low-power modes all clocks will remain enabled. And if you add CLK_SLEEP_DISABLE() to the drivers, it will not be re-enabled again.

(Note that the change here does impact pretty much all MCUs, because they define these macros.)

I understand. There are things we could possibly do to mitigate this, for example we could refactor the UART clock enable/disable function, and define a macro for CLK_SLEEP_ENABLE/DISABLE that's only actually defined for N6. This requires even more changes though.
Note: we don't have to merge this I was just pointing out the issue, we could just work around it in our code.

[dpgeorge]

However, one problem with that approach is that there are no corresponding CLK_SLEEP_DISABLE()'s, so in low-power modes all clocks will remain enabled.

Do you need to do CLK_SLEEP_DISABLE() to get the peripherals to shut down in STOP and STANDBY modes? I thought it was just for SLEEP mode (ie standard WFI/WFE).

On the one hand we want them to stay enabled during SLEEP (eg at the idle REPL). On the other hand, if you then want them to shut down during STOP/STANDBY then you can just deinit them and that will fully turn them off.

I'm a bit confused as to what the goal here is, what exactly is the issue with just enabling the sleep clocks unconditionally like is already done in stm32_main? If you also do that in your main code, I think it should work well.

[iabdalkader]

Yes, you're probably right. I will just add this to our init code.

[dpgeorge]

Just to add: all existing MCUs work fine in STOP/STANDBY with all peripherals having their sleep clock enabled (which is the default on them). So it should be the same for the N6, it should be OK to have the peripheral sleep clocks always enabled.

[iabdalkader]

    LL_APB1_GRP1_EnableClockLowPower(0xffffffff); // I2C, I3C, LPTIM, SPI, TIM, UART, WWDG
    LL_APB2_GRP1_EnableClockLowPower(0xffffffff); // SAI, SPI, TIM, UART
    LL_APB4_GRP1_EnableClockLowPower(0xffffffff); // I2C, LPTIM, LPUART, RTC, SPI

@dpgeorge Note that some of those bits are reserved. The HAL provides macros with all bits for each bus/group, ex: LL_APB1_GRP1_PERIPH_ALL.

[dpgeorge]

Note that some of those bits are reserved. The HAL provides macros with all bits for each bus/group, ex: LL_APB1_GRP1_PERIPH_ALL

OK, thanks. See #18087.