Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1405955
| From | Maxime Coquelin <mcoquelin.stm32@gmail.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] hwrng: stm32 - fix build warning |
| Date | 2016-05-24 11:00 +0200 |
| Message-ID | <rCgm3-12f-29@gated-at.bofh.it> (permalink) |
| References | <rC1Gi-gw-13@gated-at.bofh.it> <rC4NQ-2ay-27@gated-at.bofh.it> <rCfpV-sX-35@gated-at.bofh.it> <rCg2C-Ve-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
2016-05-24 10:32 GMT+02:00 Arnd Bergmann <arnd@arndb.de>:
> On Tuesday, May 24, 2016 9:59:41 AM CEST Maxime Coquelin wrote:
>> 2016-05-23 22:35 GMT+02:00 Arnd Bergmann <arnd@arndb.de>:
>> > On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote:
>> >> diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
>> >> index 92a8106..0533370 100644
>> >> --- a/drivers/char/hw_random/stm32-rng.c
>> >> +++ b/drivers/char/hw_random/stm32-rng.c
>> >> @@ -52,7 +52,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>> >> {
>> >> struct stm32_rng_private *priv =
>> >> container_of(rng, struct stm32_rng_private, rng);
>> >> - u32 sr;
>> >> + u32 sr = 0;
>> >> int retval = 0;
>> >>
>> >> pm_runtime_get_sync((struct device *) priv->rng.priv);
>> >
>> > Does this work as well?
>> >
>> > diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
>> > index 92a810648bd0..5c836b0afa40 100644
>> > --- a/drivers/char/hw_random/stm32-rng.c
>> > +++ b/drivers/char/hw_random/stm32-rng.c
>> > @@ -79,7 +79,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>> > max -= sizeof(u32);
>> > }
>> >
>> > - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
>> > + if (WARN_ONCE(retval > 0 && (sr & (RNG_SR_SEIS | RNG_SR_CEIS)),
>> > "bad RNG status - %x\n", sr))
>> > writel_relaxed(0, priv->base + RNG_SR);
>> >
>> > I think it would be nicer to not add a bogus initialization.
>> Hmm, no sure this nicer.
>> The while loop can break before retval is incremented when sr value is
>> not expected (sr != RNG_SR_DRDY).
>> In that case, we certainly want to print sr value.
>
> Ah, you are right.
>
>> Maybe the better way is just to initialize sr with status register content?
>
>> pm_runtime_get_sync((struct device *) priv->rng.priv);
>>
>>+ sr = readl_relaxed(priv->base + RNG_SR);
>> while (max > sizeof(u32)) {
>>- sr = readl_relaxed(priv->base + RNG_SR);
>> if (!sr && wait) {
>> unsigned int timeout = RNG_TIMEOUT;
>
>
> I think that introduces a bug: you really want to read the status
> register on each loop iteration.
Actually, I read the status again at the end of the loop.
But my implementation isn't good anyway, because I read the status
register one time more every time.
>
> How about moving the error handling into the loop itself?
That would be better, indeed, but there is one problem with your below proposal:
>
> Arnd
>
>
> diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c
> index 92a810648bd0..fceacd809462 100644
> --- a/drivers/char/hw_random/stm32-rng.c
> +++ b/drivers/char/hw_random/stm32-rng.c
> @@ -59,6 +59,10 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>
> while (max > sizeof(u32)) {
> sr = readl_relaxed(priv->base + RNG_SR);
> + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
> + "bad RNG status - %x\n", sr))
> + writel_relaxed(0, priv->base + RNG_SR);
> +
The error handling should be moved after the last status register read.
> if (!sr && wait) {
> unsigned int timeout = RNG_TIMEOUT;
>
> @@ -79,10 +83,6 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> max -= sizeof(u32);
> }
>
> - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
> - "bad RNG status - %x\n", sr))
> - writel_relaxed(0, priv->base + RNG_SR);
> -
> pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
> pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);
diff --git a/drivers/char/hw_random/stm32-rng.c
b/drivers/char/hw_random/stm32-rng.c
index 92a810648bd0..2a0fc90e4dc3 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -68,6 +68,10 @@ static int stm32_rng_read(struct hwrng *rng, void
*data, size_t max, bool wait)
} while (!sr && --timeout);
}
+ if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
+ "bad RNG status - %x\n", sr))
+ writel_relaxed(0, priv->base + RNG_SR);
+
/* If error detected or data not ready... */
if (sr != RNG_SR_DRDY)
break;
@@ -79,10 +83,6 @@ static int stm32_rng_read(struct hwrng *rng, void
*data, size_t max, bool wait)
max -= sizeof(u32);
}
- if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
- "bad RNG status - %x\n", sr))
- writel_relaxed(0, priv->base + RNG_SR);
-
pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);
Thanks,
Maxime
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-05-23 19:20 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Arnd Bergmann <arnd@arndb.de> - 2016-05-23 22:40 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Maxime Coquelin <mcoquelin.stm32@gmail.com> - 2016-05-24 10:00 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Arnd Bergmann <arnd@arndb.de> - 2016-05-24 10:40 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Maxime Coquelin <mcoquelin.stm32@gmail.com> - 2016-05-24 11:00 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Arnd Bergmann <arnd@arndb.de> - 2016-05-24 11:00 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Maxime Coquelin <mcoquelin.stm32@gmail.com> - 2016-05-24 11:30 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-05-25 08:40 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Daniel Thompson <daniel.thompson@linaro.org> - 2016-05-24 12:10 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Maxime Coquelin <mcoquelin.stm32@gmail.com> - 2016-05-24 13:00 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-05-25 08:40 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Arnd Bergmann <arnd@arndb.de> - 2016-05-25 12:10 +0200
Re: [PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-05-27 15:40 +0200
csiph-web