2024-04-19 05:02:54

by Marek Vasut

[permalink] [raw]
Subject: [PATCH 1/3] hwrng: stm32 - use logical OR in conditional

The conditional is used to check whether err is non-zero OR whether
reg variable is non-zero after clearing bits from it. This should be
done using logical OR, not bitwise OR, fix it.

Fixes: 6b85a7e141cb ("hwrng: stm32 - implement STM32MP13x support")
Signed-off-by: Marek Vasut <[email protected]>
---
Cc: "Uwe Kleine-König" <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Gatien Chevallier <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Olivia Mackall <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Yang Yingliang <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/char/hw_random/stm32-rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 379bc245c5202..1cc61ef8ee54c 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -353,7 +353,7 @@ static int stm32_rng_init(struct hwrng *rng)
err = readl_relaxed_poll_timeout_atomic(priv->base + RNG_SR, reg,
reg & RNG_SR_DRDY,
10, 100000);
- if (err | (reg & ~RNG_SR_DRDY)) {
+ if (err || (reg & ~RNG_SR_DRDY)) {
clk_disable_unprepare(priv->clk);
dev_err((struct device *)priv->rng.priv,
"%s: timeout:%x SR: %x!\n", __func__, err, reg);
--
2.43.0



2024-04-19 05:02:58

by Marek Vasut

[permalink] [raw]
Subject: [PATCH 2/3] hwrng: stm32 - put IP into RPM suspend on failure

In case of an irrecoverable failure, put the IP into RPM suspend
to avoid RPM imbalance. I did not trigger this case, but it seems
it should be done based on reading the code.

Fixes: b17bc6eb7c2b ("hwrng: stm32 - rework error handling in stm32_rng_read()")
Signed-off-by: Marek Vasut <[email protected]>
---
Cc: "Uwe Kleine-König" <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Gatien Chevallier <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Olivia Mackall <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Yang Yingliang <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/char/hw_random/stm32-rng.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index 1cc61ef8ee54c..b6182f86d8a4b 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -220,7 +220,8 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
if (err && i > RNG_NB_RECOVER_TRIES) {
dev_err((struct device *)priv->rng.priv,
"Couldn't recover from seed error\n");
- return -ENOTRECOVERABLE;
+ retval = -ENOTRECOVERABLE;
+ goto exit_rpm;
}

continue;
@@ -238,7 +239,8 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
if (err && i > RNG_NB_RECOVER_TRIES) {
dev_err((struct device *)priv->rng.priv,
"Couldn't recover from seed error");
- return -ENOTRECOVERABLE;
+ retval = -ENOTRECOVERABLE;
+ goto exit_rpm;
}

continue;
@@ -250,6 +252,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
max -= sizeof(u32);
}

+exit_rpm:
pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);

--
2.43.0


2024-04-19 05:02:59

by Marek Vasut

[permalink] [raw]
Subject: [PATCH 3/3] hwrng: stm32 - repair clock handling

The clock management in this driver does not seem to be correct. The
struct hwrng .init callback enables the clock, but there is no matching
.cleanup callback to disable the clock. The clock get disabled as some
later point by runtime PM suspend callback.

Furthermore, both runtime PM and sleep suspend callbacks access registers
first and disable clock which are used for register access second. If the
IP is already in RPM suspend and the system enters sleep state, the sleep
callback will attempt to access registers while the register clock are
already disabled. This bug has been fixed once before already in commit
9bae54942b13 ("hwrng: stm32 - fix pm_suspend issue"), and regressed in
commit ff4e46104f2e ("hwrng: stm32 - rework power management sequences") .

Fix this slightly differently, disable register clock at the end of .init
callback, this way the IP is disabled after .init. On every access to the
IP, which really is only stm32_rng_read(), do pm_runtime_get_sync() which
is already done in stm32_rng_read() to bring the IP from RPM suspend, and
pm_runtime_mark_last_busy()/pm_runtime_put_sync_autosuspend() to put it
back into RPM suspend.

Change sleep suspend/resume callbacks to enable and disable register clock
around register access, as those cannot use the RPM suspend/resume callbacks
due to slightly different initialization in those sleep callbacks. This way,
the register access should always be performed with clock surely enabled.

Fixes: ff4e46104f2e ("hwrng: stm32 - rework power management sequences")
Signed-off-by: Marek Vasut <[email protected]>
---
Cc: "Uwe Kleine-König" <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Gatien Chevallier <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Olivia Mackall <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Yang Yingliang <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/char/hw_random/stm32-rng.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
index b6182f86d8a4b..0e903d6e22e30 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -363,6 +363,8 @@ static int stm32_rng_init(struct hwrng *rng)
return -EINVAL;
}

+ clk_disable_unprepare(priv->clk);
+
return 0;
}

@@ -387,6 +389,11 @@ static int __maybe_unused stm32_rng_runtime_suspend(struct device *dev)
static int __maybe_unused stm32_rng_suspend(struct device *dev)
{
struct stm32_rng_private *priv = dev_get_drvdata(dev);
+ int err;
+
+ err = clk_prepare_enable(priv->clk);
+ if (err)
+ return err;

if (priv->data->has_cond_reset) {
priv->pm_conf.nscr = readl_relaxed(priv->base + RNG_NSCR);
@@ -468,6 +475,8 @@ static int __maybe_unused stm32_rng_resume(struct device *dev)
writel_relaxed(reg, priv->base + RNG_CR);
}

+ clk_disable_unprepare(priv->clk);
+
return 0;
}

--
2.43.0