Received-SPF: pass (google.com: domain of linux-crypto+bounces-4164-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <b2d0dfcb-37d6-4375-a4ad-ca96a5339840@foss.st.com>
Date: Tue, 14 May 2024 10:10:16 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] [RFC] clk: stm32mp1: Keep RNG1 clock always running
To: Marek Vasut <marex@denx.de>, <linux-crypto@vger.kernel.org>
CC: =?UTF-8?Q?Uwe_Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Gabriel Fernandez
	<gabriel.fernandez@foss.st.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Michael Turquette
	<mturquette@baylibre.com>,
        Olivia Mackall <olivia@selenic.com>, Rob Herring
	<robh@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Yang Yingliang
	<yangyingliang@huawei.com>,
        <linux-arm-kernel@lists.infradead.org>, <linux-clk@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>
References: <20240513220349.183568-1-marex@denx.de>
Content-Language: en-US
From: Gatien CHEVALLIER <gatien.chevallier@foss.st.com>
In-Reply-To: <20240513220349.183568-1-marex@denx.de>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

Hi Marek,

Strange indeed.
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.
Else it might just be a bad clock balance.

Can you describe the software ecosystem that you're running please?
(SCMI/no SCMI)?

Do you have the 3 fixes of stm32_rng.c that you've sent recently in your
software when testing?

What if you add a trace in a random generation function in random.c?

After this, I'll try to reproduce the issue.

Thanks,
Gatien


On 5/14/24 00:02, Marek Vasut wrote:
> 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(-)
> 
> 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),