diff mbox series

[RFC] clk: stm32mp1: Keep RNG1 clock always running

Message ID 20240513220349.183568-1-marex@denx.de
State New
Headers show
Series [RFC] clk: stm32mp1: Keep RNG1 clock always running | expand

Commit Message

Marek Vasut May 13, 2024, 10:02 p.m. UTC
In case of STM32MP15xC/F SoC, in case the RNG1 is enabled in DT, the RNG1
clock are managed by the driver. The RNG1 clock are toggled off on entry
to suspend and back on on resume. For reason thus far unknown (could this
be some chip issue?), when the system goes through repeated suspend/resume
cycles, the system eventually hangs after a few such cycles.

This can be reproduced with CONFIG_PM_DEBUG 'pm_test' this way:
"
echo core > /sys/power/pm_test
while true ; do
    echo mem > /sys/power/state
    sleep 2 ;
done
"
The system locks up after about a minute and if WDT is active, resets.

If the RNG1 clock are kept enabled across suspend/resume, either using
this change, or by keeping the clock enabled in RNG driver suspend/resume
callbacks, the system does not lock up.

NOTE: This patch is a workaround. It would be good to know why does this
      change make the hang go away, whether this is a chip issue or some
      other problem ?

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
Cc: Gatien Chevallier <gatien.chevallier@foss.st.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Olivia Mackall <olivia@selenic.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Yang Yingliang <yangyingliang@huawei.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-clk@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
---
 drivers/char/hw_random/stm32-rng.c | 2 ++
 drivers/clk/stm32/clk-stm32mp1.c   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Gatien CHEVALLIER May 15, 2024, 9:16 a.m. UTC | #1
Hi Marek,

On 5/14/24 16:37, Marek Vasut wrote:
> On 5/14/24 10:10 AM, Gatien CHEVALLIER wrote:
>> Hi Marek,
> 
> Hi,
> 
>> Strange indeed.
> 
> Yes.
> 
>> A potential reason that comes to my mind would be that something tries 
>> to get a random number after the driver suspended and fails to do so.
> 
> Possibly.
> 
>> Else it might just be a bad clock balance.
> 
> I don't think so, this would be reported by the kernel and it would show 
> up in /sys/kernel/debug/clk/clk_summary as incrementing use count. It 
> would also not happen in a non-deterministic manner like this happens 
> here, the hang doesn't always happen after well defined suspend/resume 
> cycle count.
> 
>> Can you describe the software ecosystem that you're running please?
>> (SCMI/no SCMI)?
> 
> STM32MP157C DHCOM PDK2 with mainline U-Boot 2024.07-rc2 , no SCMI.
> 
>> Do you have the 3 fixes of stm32_rng.c that you've sent recently in your
>> software when testing?
> 
> Yes, but this happens even without them.
> 
>> What if you add a trace in a random generation function in random.c?
> 
> Do you have a function name or line number for me ?

I put a trace in _get_random_bytes() in drivers/char/random.c. I'm not
100% sure but this should be the entry point when getting a random number.

> 
>> After this, I'll try to reproduce the issue.
> 
> If you have a minute to test it on some ST MP15 board, that would be 
> real nice. Thanks !

I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the bootchain
but this should not have an impact here.

How did you manage to test using "echo core > /sys/power/pm_test"?
In kernel/power/suspend.c, enter_state(). If the pm_test_level is core,
then an error is fired with the following trace:
"Unsupported test mode for suspend to idle, please choose 
none/freezer/devices/platform."

I've tried using "echo devices > /sys/power/pm_test" so that I can at 
least test that the driver is put to sleep then wakes up. I do not
reproduce your issue.

[  169.026421] Filesystems sync: 0.013 seconds
[  169.031087] Freezing user space processes
[  169.036562] Freezing user space processes completed (elapsed 0.002 
seconds)
[  169.042238] OOM killer disabled.
[  169.045383] Freezing remaining freezable tasks
[  169.051408] Freezing remaining freezable tasks completed (elapsed 
0.001 seconds)
[  169.238226] dwc2 49000000.usb-otg: suspending usb gadget 
configfs-gadget.g1
[  169.270236] In stm32_rng_suspend
[  169.275501] PM: suspend debug: Waiting for 5 second(s).
[  174.283418] In stm32_rng_resume
[  174.284291] stm32-dwmac 5800a000.ethernet end0: configuring for 
phy/rgmii-id link mode
[  174.337714] dwmac4: Master AXI performs any burst length
[  174.341699] stm32-dwmac 5800a000.ethernet end0: No Safety Features 
support found
[  174.349138] stm32-dwmac 5800a000.ethernet end0: IEEE 1588-2008 
Advanced Timestamp supported
[  174.363442] dwc2 49000000.usb-otg: resuming usb gadget configfs-gadget.g1
[  174.667669] onboard-usb-hub 2-1: reset high-speed USB device number 2 
using ehci-platform
[  174.989075] OOM killer enabled.
[  174.990848] Restarting tasks ... done.
[  175.003976] random: crng reseeded on system resumption
[  175.009464] PM: suspend exit
[  175.011473] random: ASKING FOR 96 BYTES
[  175.011468] random: ASKING FOR 96 BYTES
[  175.015747] random: ASKING FOR 16 BYTES
[  175.044933] random: ASKING FOR 96 BYTES
[  175.059399] random: ASKING FOR 96 BYTES
[  175.070925] random: ASKING FOR 16 BYTES
[  175.079285] random: ASKING FOR 96 BYTES
[  175.082113] random: ASKING FOR 16 BYTES
[  175.096759] random: ASKING FOR 16 BYTES
[  175.098674] random: ASKING FOR 96 BYTES
[  175.295584] random: ASKING FOR 16 BYTES
[  175.302357] random: ASKING FOR 96 BYTES
[  175.311525] random: ASKING FOR 16 BYTES
[  175.312989] random: ASKING FOR 16 BYTES


Can you give it another shot with the trace so that we can ensure that
no random is asked after the driver is suspended in your case please?

Thanks,
Gatien
Marek Vasut May 16, 2024, 1:06 a.m. UTC | #2
On 5/15/24 11:16 AM, Gatien CHEVALLIER wrote:

Hi,

>>> What if you add a trace in a random generation function in random.c?
>>
>> Do you have a function name or line number for me ?
> 
> I put a trace in _get_random_bytes() in drivers/char/random.c. I'm not
> 100% sure but this should be the entry point when getting a random number.

You're right, there is a read attempt right before the hang, and 
__clk_is_enabled() returns 0 in stm32_read_rng() . In fact, it is the 
pm_runtime_get_sync() which is returning -EACCES instead of zero, and 
this is currently not checked so the failure is not detected before 
register access takes place, to register file with clock disabled, which 
triggers a hard hang.

I'll be sending a patch shortly, thanks for this hint !

>>> After this, I'll try to reproduce the issue.
>>
>> If you have a minute to test it on some ST MP15 board, that would be 
>> real nice. Thanks !
> 
> I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
> SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the bootchain
> but this should not have an impact here.
> 
> How did you manage to test using "echo core > /sys/power/pm_test"?
> In kernel/power/suspend.c, enter_state(). If the pm_test_level is core,
> then an error is fired with the following trace:
> "Unsupported test mode for suspend to idle, please choose 
> none/freezer/devices/platform."

Could this be firmware related ?

> I've tried using "echo devices > /sys/power/pm_test" so that I can at 
> least test that the driver is put to sleep then wakes up. I do not
> reproduce your issue.

Can you try 'processors' ?

I did also notice it sometimes takes much longer than a minute to hang, 
but eventually it does hang. Maybe let it cycle for an hour or a few ?

[...]
Gatien CHEVALLIER May 16, 2024, 7:42 a.m. UTC | #3
Hi Marek

On 5/16/24 03:06, Marek Vasut wrote:
> On 5/15/24 11:16 AM, Gatien CHEVALLIER wrote:
> 
> Hi,
> 
>>>> What if you add a trace in a random generation function in random.c?
>>>
>>> Do you have a function name or line number for me ?
>>
>> I put a trace in _get_random_bytes() in drivers/char/random.c. I'm not
>> 100% sure but this should be the entry point when getting a random 
>> number.
> 
> You're right, there is a read attempt right before the hang, and 
> __clk_is_enabled() returns 0 in stm32_read_rng() . In fact, it is the 
> pm_runtime_get_sync() which is returning -EACCES instead of zero, and 
> this is currently not checked so the failure is not detected before 
> register access takes place, to register file with clock disabled, which 
> triggers a hard hang.
> 
> I'll be sending a patch shortly, thanks for this hint !
> 

Great news, indeed the return code isn't checked. Let's use
pm_runtime_resume_and_get().

>>>> After this, I'll try to reproduce the issue.
>>>
>>> If you have a minute to test it on some ST MP15 board, that would be 
>>> real nice. Thanks !
>>
>> I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
>> SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the bootchain
>> but this should not have an impact here.
>>
>> How did you manage to test using "echo core > /sys/power/pm_test"?
>> In kernel/power/suspend.c, enter_state(). If the pm_test_level is core,
>> then an error is fired with the following trace:
>> "Unsupported test mode for suspend to idle, please choose 
>> none/freezer/devices/platform."
> 
> Could this be firmware related ?
> 
>> I've tried using "echo devices > /sys/power/pm_test" so that I can at 
>> least test that the driver is put to sleep then wakes up. I do not
>> reproduce your issue.
> 
> Can you try 'processors' ?
> 

Given this:
#ifdef CONFIG_PM_DEBUG
		if (pm_test_level != TEST_NONE && pm_test_level <= TEST_CPUS) {
			pr_warn("Unsupported test mode for suspend to idle, please choose 
none/freezer/devices/platform.\n");
			return -EAGAIN;
		}
#endif

and this

static const char * const pm_tests[__TEST_AFTER_LAST] = {
	[TEST_NONE] = "none",
	[TEST_CORE] = "core",
	[TEST_CPUS] = "processors",
	[TEST_PLATFORM] = "platform",
	[TEST_DEVICES] = "devices",
	[TEST_FREEZER] = "freezer",
};

I'm getting the error as well.

> I did also notice it sometimes takes much longer than a minute to hang, 
> but eventually it does hang. Maybe let it cycle for an hour or a few ?
> 

I'll let it loop for some time then for device pm state.

> [...]
> 

Thanks for investigating this.

Cheers,
Gatien
Marek Vasut May 16, 2024, 10:43 a.m. UTC | #4
On 5/16/24 9:42 AM, Gatien CHEVALLIER wrote:

Hi,

>>>>> What if you add a trace in a random generation function in random.c?
>>>>
>>>> Do you have a function name or line number for me ?
>>>
>>> I put a trace in _get_random_bytes() in drivers/char/random.c. I'm not
>>> 100% sure but this should be the entry point when getting a random 
>>> number.
>>
>> You're right, there is a read attempt right before the hang, and 
>> __clk_is_enabled() returns 0 in stm32_read_rng() . In fact, it is the 
>> pm_runtime_get_sync() which is returning -EACCES instead of zero, and 
>> this is currently not checked so the failure is not detected before 
>> register access takes place, to register file with clock disabled, 
>> which triggers a hard hang.
>>
>> I'll be sending a patch shortly, thanks for this hint !
>>
> 
> Great news, indeed the return code isn't checked. Let's use
> pm_runtime_resume_and_get().

Yes please.

I will wonder why we get EACCES though, that basically means we are 
suspending already. Is it safe to return -errno from rng read function 
in that case ?

>>>>> After this, I'll try to reproduce the issue.
>>>>
>>>> If you have a minute to test it on some ST MP15 board, that would be 
>>>> real nice. Thanks !
>>>
>>> I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
>>> SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the bootchain
>>> but this should not have an impact here.
>>>
>>> How did you manage to test using "echo core > /sys/power/pm_test"?
>>> In kernel/power/suspend.c, enter_state(). If the pm_test_level is core,
>>> then an error is fired with the following trace:
>>> "Unsupported test mode for suspend to idle, please choose 
>>> none/freezer/devices/platform."
>>
>> Could this be firmware related ?
>>
>>> I've tried using "echo devices > /sys/power/pm_test" so that I can at 
>>> least test that the driver is put to sleep then wakes up. I do not
>>> reproduce your issue.
>>
>> Can you try 'processors' ?
>>
> 
> Given this:
> #ifdef CONFIG_PM_DEBUG
>          if (pm_test_level != TEST_NONE && pm_test_level <= TEST_CPUS) {
>              pr_warn("Unsupported test mode for suspend to idle

You're supposed to be suspending to 'mem' , not 'idle' . Could that be it ?
Marek Vasut May 16, 2024, 8:01 p.m. UTC | #5
On 5/16/24 4:35 PM, Gatien CHEVALLIER wrote:

Hi,

>>>>>>> What if you add a trace in a random generation function in random.c?
>>>>>>
>>>>>> Do you have a function name or line number for me ?
>>>>>
>>>>> I put a trace in _get_random_bytes() in drivers/char/random.c. I'm not
>>>>> 100% sure but this should be the entry point when getting a random 
>>>>> number.
>>>>
>>>> You're right, there is a read attempt right before the hang, and 
>>>> __clk_is_enabled() returns 0 in stm32_read_rng() . In fact, it is 
>>>> the pm_runtime_get_sync() which is returning -EACCES instead of 
>>>> zero, and this is currently not checked so the failure is not 
>>>> detected before register access takes place, to register file with 
>>>> clock disabled, which triggers a hard hang.
>>>>
>>>> I'll be sending a patch shortly, thanks for this hint !
>>>>
>>>
>>> Great news, indeed the return code isn't checked. Let's use
>>> pm_runtime_resume_and_get().
>>
>> Yes please.
>>
>> I will wonder why we get EACCES though, that basically means we are 
>> suspending already. Is it safe to return -errno from rng read function 
>> in that case ?
> 
> The framework expects a function that can return an error code so I
> don't see why not. Else the framework would have an issue.
> 
> I still haven't figured out what is happening.
> 
> Could it be that the kernel is getting entropy with hwrng_fillfn()
> like it does periodically to feed the entropy pool and it happens at the
> same time as your pm test sequence?

Possibly. I use script as init which contains basically #!/bin/sh , 
mount of a few filesystems like dev, proc, sys, and then the pm_test 
sequence to avoid wasting time booting full userspace.

> FYI, I have been running your script with (echo devices > 
> /sys/power/pm_test) for 5 hours now and haven't been able to reproduce 
> the issue.

Maybe the 'devices' test is not enough and the deeper pm_test states 
have some sort of impact ?

>>>>>>> After this, I'll try to reproduce the issue.
>>>>>>
>>>>>> If you have a minute to test it on some ST MP15 board, that would 
>>>>>> be real nice. Thanks !
>>>>>
>>>>> I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
>>>>> SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the 
>>>>> bootchain
>>>>> but this should not have an impact here.
>>>>>
>>>>> How did you manage to test using "echo core > /sys/power/pm_test"?
>>>>> In kernel/power/suspend.c, enter_state(). If the pm_test_level is 
>>>>> core,
>>>>> then an error is fired with the following trace:
>>>>> "Unsupported test mode for suspend to idle, please choose 
>>>>> none/freezer/devices/platform."
>>>>
>>>> Could this be firmware related ?
>>>>
>>>>> I've tried using "echo devices > /sys/power/pm_test" so that I can 
>>>>> at least test that the driver is put to sleep then wakes up. I do not
>>>>> reproduce your issue.
>>>>
>>>> Can you try 'processors' ?
>>>>
>>>
>>> Given this:
>>> #ifdef CONFIG_PM_DEBUG
>>>          if (pm_test_level != TEST_NONE && pm_test_level <= TEST_CPUS) {
>>>              pr_warn("Unsupported test mode for suspend to idle
>>
>> You're supposed to be suspending to 'mem' , not 'idle' . Could that be 
>> it ?
> 
> Yes you're right, I've been missing that. I do not have "deep" available
> in /sys/power/mem_sleep... not upstreamed yet maybe... Have you coded a
> PSCI service for this in U-Boot?
> 
> I'm either missing something or I can't reproduce your setup.

The PSCI provider in U-Boot has been in place for years, there's no need 
to code anything, just compile it and that's all:

$ make stm32mp15_basic_defconfig && make -j`nproc`

This gets you u-boot-spl.stm32 and u-boot.itb as FSBL/SSBL .
Gatien CHEVALLIER May 17, 2024, 3:39 p.m. UTC | #6
On 5/16/24 22:01, Marek Vasut wrote:
> On 5/16/24 4:35 PM, Gatien CHEVALLIER wrote:
> 
> Hi,
> 
>>>>>>>> What if you add a trace in a random generation function in 
>>>>>>>> random.c?
>>>>>>>
>>>>>>> Do you have a function name or line number for me ?
>>>>>>
>>>>>> I put a trace in _get_random_bytes() in drivers/char/random.c. I'm 
>>>>>> not
>>>>>> 100% sure but this should be the entry point when getting a random 
>>>>>> number.
>>>>>
>>>>> You're right, there is a read attempt right before the hang, and 
>>>>> __clk_is_enabled() returns 0 in stm32_read_rng() . In fact, it is 
>>>>> the pm_runtime_get_sync() which is returning -EACCES instead of 
>>>>> zero, and this is currently not checked so the failure is not 
>>>>> detected before register access takes place, to register file with 
>>>>> clock disabled, which triggers a hard hang.
>>>>>
>>>>> I'll be sending a patch shortly, thanks for this hint !
>>>>>
>>>>
>>>> Great news, indeed the return code isn't checked. Let's use
>>>> pm_runtime_resume_and_get().
>>>
>>> Yes please.
>>>
>>> I will wonder why we get EACCES though, that basically means we are 
>>> suspending already. Is it safe to return -errno from rng read 
>>> function in that case ?
>>
>> The framework expects a function that can return an error code so I
>> don't see why not. Else the framework would have an issue.
>>
>> I still haven't figured out what is happening.
>>
>> Could it be that the kernel is getting entropy with hwrng_fillfn()
>> like it does periodically to feed the entropy pool and it happens at the
>> same time as your pm test sequence?
> 
> Possibly. I use script as init which contains basically #!/bin/sh , 
> mount of a few filesystems like dev, proc, sys, and then the pm_test 
> sequence to avoid wasting time booting full userspace.
> 
Ok,

The strangest thing is not being to enable the clock, maybe there's
something on the clock driver side. Tracking clock enable/disable
may lead to something.

>> FYI, I have been running your script with (echo devices > 
>> /sys/power/pm_test) for 5 hours now and haven't been able to reproduce 
>> the issue.
> 
> Maybe the 'devices' test is not enough and the deeper pm_test states 
> have some sort of impact ?
>

Maybe, I don't have the knowledge to confirm or invalidate this.
Tasks should be frozen before drivers are put to sleep so my instinct
would say no but you can't take it for granted :)

>>>>>>>> After this, I'll try to reproduce the issue.
>>>>>>>
>>>>>>> If you have a minute to test it on some ST MP15 board, that would 
>>>>>>> be real nice. Thanks !
>>>>>>
>>>>>> I tried to reproduce the issue you're facing on a STM32MP157C-DK2 no
>>>>>> SCMI on the 6.9-rc7 kernel tag. I uses OP-TEE and TF-A in the 
>>>>>> bootchain
>>>>>> but this should not have an impact here.
>>>>>>
>>>>>> How did you manage to test using "echo core > /sys/power/pm_test"?
>>>>>> In kernel/power/suspend.c, enter_state(). If the pm_test_level is 
>>>>>> core,
>>>>>> then an error is fired with the following trace:
>>>>>> "Unsupported test mode for suspend to idle, please choose 
>>>>>> none/freezer/devices/platform."
>>>>>
>>>>> Could this be firmware related ?
>>>>>
>>>>>> I've tried using "echo devices > /sys/power/pm_test" so that I can 
>>>>>> at least test that the driver is put to sleep then wakes up. I do not
>>>>>> reproduce your issue.
>>>>>
>>>>> Can you try 'processors' ?
>>>>>
>>>>
>>>> Given this:
>>>> #ifdef CONFIG_PM_DEBUG
>>>>          if (pm_test_level != TEST_NONE && pm_test_level <= 
>>>> TEST_CPUS) {
>>>>              pr_warn("Unsupported test mode for suspend to idle
>>>
>>> You're supposed to be suspending to 'mem' , not 'idle' . Could that 
>>> be it ?
>>
>> Yes you're right, I've been missing that. I do not have "deep" available
>> in /sys/power/mem_sleep... not upstreamed yet maybe... Have you coded a
>> PSCI service for this in U-Boot?
>>
>> I'm either missing something or I can't reproduce your setup.
> 
> The PSCI provider in U-Boot has been in place for years, there's no need 
> to code anything, just compile it and that's all:
> 
> $ make stm32mp15_basic_defconfig && make -j`nproc`
> 
> This gets you u-boot-spl.stm32 and u-boot.itb as FSBL/SSBL .

Ok thanks.

Best regards,
Gatien
Marek Vasut May 21, 2024, 10:27 a.m. UTC | #7
On 5/17/24 5:39 PM, Gatien CHEVALLIER wrote:

Hi,

>> Possibly. I use script as init which contains basically #!/bin/sh , 
>> mount of a few filesystems like dev, proc, sys, and then the pm_test 
>> sequence to avoid wasting time booting full userspace.
>>
> Ok,
> 
> The strangest thing is not being to enable the clock, maybe there's
> something on the clock driver side. Tracking clock enable/disable
> may lead to something.

I suspect the problem is that rng_read and runtime suspend/resume can 
run in parallel, that's why this problem occurs.

>>> FYI, I have been running your script with (echo devices > 
>>> /sys/power/pm_test) for 5 hours now and haven't been able to 
>>> reproduce the issue.
>>
>> Maybe the 'devices' test is not enough and the deeper pm_test states 
>> have some sort of impact ?
>>
> 
> Maybe, I don't have the knowledge to confirm or invalidate this.
> Tasks should be frozen before drivers are put to sleep so my instinct
> would say no but you can't take it for granted :)

Could it be the kernel that requires randomness ?
Gatien CHEVALLIER May 28, 2024, 1:55 p.m. UTC | #8
On 5/21/24 12:27, Marek Vasut wrote:
> On 5/17/24 5:39 PM, Gatien CHEVALLIER wrote:
> 
> Hi,
> 
>>> Possibly. I use script as init which contains basically #!/bin/sh , 
>>> mount of a few filesystems like dev, proc, sys, and then the pm_test 
>>> sequence to avoid wasting time booting full userspace.
>>>
>> Ok,
>>
>> The strangest thing is not being to enable the clock, maybe there's
>> something on the clock driver side. Tracking clock enable/disable
>> may lead to something.
> 
> I suspect the problem is that rng_read and runtime suspend/resume can 
> run in parallel, that's why this problem occurs.
> 

Hum, this looks strange... This would need to be confirmed in your
use case. That would mean that flags aren't synced at the entry of these
functions?

>>>> FYI, I have been running your script with (echo devices > 
>>>> /sys/power/pm_test) for 5 hours now and haven't been able to 
>>>> reproduce the issue.
>>>
>>> Maybe the 'devices' test is not enough and the deeper pm_test states 
>>> have some sort of impact ?
>>>
>>
>> Maybe, I don't have the knowledge to confirm or invalidate this.
>> Tasks should be frozen before drivers are put to sleep so my instinct
>> would say no but you can't take it for granted :)
> 
> Could it be the kernel that requires randomness ?

That can be confirmed by adding traces to the entry point in random.c.
Maybe activating CONFIG_WARN_ALL_UNSEEDED_RANDOM will help investigate
this. It will add verbosity if crng isn't ready.

Or maybe try calling directely rng_is_initialized() to see if the crng
is ready when your issue occurs.

Best regards,
Gatien
diff mbox series

Patch

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 7d0de8ab5e7f5..ec0314f05ff3e 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -403,6 +403,7 @@  static int __maybe_unused stm32_rng_suspend(struct device *dev)
 
 	writel_relaxed(priv->pm_conf.cr, priv->base + RNG_CR);
 
+	// Keeping the clock enabled across suspend/resume helps too
 	clk_disable_unprepare(priv->clk);
 
 	return 0;
@@ -434,6 +435,7 @@  static int __maybe_unused stm32_rng_resume(struct device *dev)
 	int err;
 	u32 reg;
 
+	// Keeping the clock enabled across suspend/resume helps too
 	err = clk_prepare_enable(priv->clk);
 	if (err)
 		return err;
diff --git a/drivers/clk/stm32/clk-stm32mp1.c b/drivers/clk/stm32/clk-stm32mp1.c
index 7e2337297402a..1a6e853d935fa 100644
--- a/drivers/clk/stm32/clk-stm32mp1.c
+++ b/drivers/clk/stm32/clk-stm32mp1.c
@@ -2000,7 +2000,7 @@  static const struct clock_config stm32mp1_clock_cfg[] = {
 	KCLK(SDMMC3_K, "sdmmc3_k", sdmmc3_src, 0, G_SDMMC3, M_SDMMC3),
 	KCLK(FMC_K, "fmc_k", fmc_src, 0, G_FMC, M_FMC),
 	KCLK(QSPI_K, "qspi_k", qspi_src, 0, G_QSPI, M_QSPI),
-	KCLK(RNG1_K, "rng1_k", rng_src, 0, G_RNG1, M_RNG1),
+	KCLK(RNG1_K, "rng1_k", rng_src, CLK_IS_CRITICAL, G_RNG1, M_RNG1),
 	KCLK(RNG2_K, "rng2_k", rng_src, 0, G_RNG2, M_RNG2),
 	KCLK(USBPHY_K, "usbphy_k", usbphy_src, 0, G_USBPHY, M_USBPHY),
 	KCLK(STGEN_K, "stgen_k", stgen_src, CLK_IS_CRITICAL, G_STGEN, M_STGEN),