STM32L5: Entropy : Power Management not working due to entropy driver & stop mode

[wcappelle]

Describe the bug
When enabling power management with stop mode(any of the three) enabled for the STM32L5 soc, the stop mode will stop all clocks in the mcu going to that lp mode. when returning from stop mode, not all clocks are being restored, like the HSI48 clock which is being used by USB & RNG hardware blocks. When this happends, and the entropy driver needs to get new samples, it enables interrupts for the block and this starts a constant entry to the ISR due to a clock error detected int he RNG hardware block.
This endless loop in the stm32_rng_isr never gets resolved.

The problem could be resolved in the power.c of the soc to re-enable the HSI48 clock (or just use MSI as a clock source if USB is not present, the power consumption will like that much more :-) ).

Next to that, a pm constraint needs to be added to the entropy driver, preventing the mcu to go to deep sleep when the entropy driver is filling the random number pool.

Impact
no low power for the L5 and probably other families which uses the rng & stop mode (L4,...)

Environment (please complete the following information):

• OS: Linux
• Zephyr SDK, 13.1
• zephyr v2.7.99-3760-g4a0e5e9958

Additional context
I've fixed some of these thing on a local branch (at least to get it working for my case), but it's deviating a bit from the mainstream clock implementations & clock selections (using MSI, with own overriding of the weak power functions), but I could share at least the power constraints in the entropy driver.

[wcappelle]

I think this might need first some discussion on how to handle the diversity in clocks. I'm wondering why this clock configuration is scattered into the usb driver & entropy driver for the same clock source (HSI48).
With some direction, I could try to fix this (if not too much deviating from what I need to work ofc).

[erwango]

I think this might need first some discussion on how to handle the diversity in clocks.

About this point, I've started to work on this here: #42097.
H7 only for now, but that's the direction.

[wcappelle]

I think this might need first some discussion on how to handle the diversity in clocks.

About this point, I've started to work on this here: #42097. H7 only for now, but that's the direction.

@erwango look nice in point of view to the devicetree, however, looks like a lot of work to rework this in the clock_stm32_ll_common.
It also doesn't completely fit to a design where 1 clock mux is needed for more than 1 hw resource. For this mcu in particular there is 1 switch for 2 blocks being rng and USB:

I don't know how you can set this in your devicetree without creating another virtual clockdomain?

[erwango]

@erwango look nice in point of view to the devicetree, however, looks like a lot of work to rework this in the clock_stm32_ll_common.

Sure, this will require some driver rework, but I don't how to achieve a scalable multi clock solution otherwise.

It also doesn't completely fit to a design where 1 clock mux is needed for more than 1 hw resource. For this mcu in particular there is 1 switch for 2 blocks being rng and USB:

Indeed, for this kind of domain, we'll need to add new dedicated nodes. But this shouldn't be quite complex. Anyway, thanks for the pointer. As this is also present on H7 devices, I'll take some time to add one and see how it goes.

[erwango]

I don't know how you can set this in your devicetree without creating another virtual clockdomain?

So I've had a try here: erwango@57ea427
This adds stm32h7 per_ck as a virtual clock device:

&perck {
	clocks = <&rcc STM32_SRC_HSI_KER CKPER_SEL(0)>;
	status = "okay";
};

Works fine (even if not fully tested yet), main drawback is the cost (~220 bytes of flash)

A much simpler alternative would be to add clock configuration directly to the clock consumer end node:

	clocks = <&rcc STM32_CLOCK_BUS_APB2 0x00001000>,
		 <&rcc STM32_SRC_HSI_KER CKPER_SEL(0)>,
		 <&rcc STM32_SRC_CKPER SPI123_SEL(4)>;

Though, this proposal has 2 main drawbacks:

• If board dts is poorly configured, 2 consumers of the same internal clock (per_ck here) could set conflicting configurations
• This adds complexity on the consumers driver implementation side that should be able to differentiate between all these clocks

In the end the virtual clock device feels saner.

[wcappelle]

t to a design where 1 clock mux is needed for more than 1 hw resource. For this mcu in particular there is 1 switch for 2 blocks being rng and USB

I'm personally also in favor of the first approach.
The direction of your PR looks good for me. I think you're also solving the splitoff of all the source clocks (HSI/MSI/HSE/HSI48/..), since in the current clock intialization routines all is very tightly coupled: if you have MSI as clock source, it will disable HSI or HSE or any other clock it thinks it might not use. As far as I could see you can with your changes select HSI in combination with MSI for different hardware clock domains, and both will be enabled right?
This change would also make it possible to move the USB & RNG clocks outside of the drivers,and make these a user of the clock driver, and let the clock driver initialize the clock instead of usb & rng driver based on the need.

I've seen there is a split off for the clock drivers for u5,h7, is it more the idea to create different clock driver files per series (split off from the common file )?