* [PATCH] hwrng: stm32 - fix build warning @ 2016-05-23 12:44 Sudip Mukherjee 2016-05-23 20:35 ` Arnd Bergmann 0 siblings, 1 reply; 13+ messages in thread From: Sudip Mukherjee @ 2016-05-23 12:44 UTC (permalink / raw) To: linux-arm-kernel We have been getting build warning about: drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used uninitialized in this function On checking the code it turns out that sr can never be used uninitialized as sr is getting initialized in the while loop and while loop will always execute as the minimum value of max can be 32. So just initialize sr to 0 while declaring it to silence the compiler. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> --- build log at: https://travis-ci.org/sudipm-mukherjee/parport/jobs/132180906 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 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); -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-23 12:44 [PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee @ 2016-05-23 20:35 ` Arnd Bergmann 2016-05-24 7:59 ` Maxime Coquelin 2016-05-25 2:05 ` Sudip Mukherjee 0 siblings, 2 replies; 13+ messages in thread From: Arnd Bergmann @ 2016-05-23 20:35 UTC (permalink / raw) To: linux-arm-kernel On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote: > We have been getting build warning about: > drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': > drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used > uninitialized in this function > > On checking the code it turns out that sr can never be used > uninitialized as sr is getting initialized in the while loop and while > loop will always execute as the minimum value of max can be 32. > So just initialize sr to 0 while declaring it to silence the compiler. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> > --- I notice that you are using a really old compiler. While this warning seems to be valid in the sense that the compiler should figure out that the variable might be used uninitialized, please update your toolchain before reporting other such problems, as gcc-4.6 had a lot more false positives that newer ones (5.x or 6.x) have. > > build log at: > https://travis-ci.org/sudipm-mukherjee/parport/jobs/132180906 > > 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 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. Arnd ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-23 20:35 ` Arnd Bergmann @ 2016-05-24 7:59 ` Maxime Coquelin 2016-05-24 8:32 ` Arnd Bergmann 2016-05-25 2:05 ` Sudip Mukherjee 1 sibling, 1 reply; 13+ messages in thread From: Maxime Coquelin @ 2016-05-24 7:59 UTC (permalink / raw) To: linux-arm-kernel 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: >> We have been getting build warning about: >> drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': >> drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used >> uninitialized in this function >> >> On checking the code it turns out that sr can never be used >> uninitialized as sr is getting initialized in the while loop and while >> loop will always execute as the minimum value of max can be 32. >> So just initialize sr to 0 while declaring it to silence the compiler. >> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >> --- > > I notice that you are using a really old compiler. While this warning > seems to be valid in the sense that the compiler should figure out that > the variable might be used uninitialized, please update your toolchain > before reporting other such problems, as gcc-4.6 had a lot more false > positives that newer ones (5.x or 6.x) have. > >> >> build log at: >> https://travis-ci.org/sudipm-mukherjee/parport/jobs/132180906 >> >> 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 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. Maybe the better way is just to initialize sr with status register content? diff --git a/drivers/char/hw_random/stm32-rng.c b/drivers/char/hw_random/stm32-rng.c index 92a810648bd0..07a6659d0fe6 100644 --- a/drivers/char/hw_random/stm32-rng.c +++ b/drivers/char/hw_random/stm32-rng.c @@ -57,8 +57,8 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) 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; @@ -77,6 +77,8 @@ static int stm32_rng_read(struct hwrng *rng, void *data, size_t max, bool wait) retval += sizeof(u32); data += sizeof(u32); max -= sizeof(u32); + + sr = readl_relaxed(priv->base + RNG_SR); } if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), Regards, Maxime ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 7:59 ` Maxime Coquelin @ 2016-05-24 8:32 ` Arnd Bergmann 2016-05-24 8:50 ` Maxime Coquelin 0 siblings, 1 reply; 13+ messages in thread From: Arnd Bergmann @ 2016-05-24 8:32 UTC (permalink / raw) To: linux-arm-kernel 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. How about moving the error handling into the loop itself? 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); + 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); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 8:32 ` Arnd Bergmann @ 2016-05-24 8:50 ` Maxime Coquelin 2016-05-24 8:58 ` Arnd Bergmann 2016-05-24 10:09 ` Daniel Thompson 0 siblings, 2 replies; 13+ messages in thread From: Maxime Coquelin @ 2016-05-24 8:50 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 8:50 ` Maxime Coquelin @ 2016-05-24 8:58 ` Arnd Bergmann 2016-05-24 9:20 ` Maxime Coquelin 2016-05-24 10:09 ` Daniel Thompson 1 sibling, 1 reply; 13+ messages in thread From: Arnd Bergmann @ 2016-05-24 8:58 UTC (permalink / raw) To: linux-arm-kernel On Tuesday, May 24, 2016 10:50:17 AM CEST Maxime Coquelin wrote: > 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, > Yes, that looks good to me. Arnd ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 8:58 ` Arnd Bergmann @ 2016-05-24 9:20 ` Maxime Coquelin 2016-05-25 2:00 ` Sudip Mukherjee 0 siblings, 1 reply; 13+ messages in thread From: Maxime Coquelin @ 2016-05-24 9:20 UTC (permalink / raw) To: linux-arm-kernel 2016-05-24 10:58 GMT+02:00 Arnd Bergmann <arnd@arndb.de>: > On Tuesday, May 24, 2016 10:50:17 AM CEST Maxime Coquelin wrote: >> 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, >> > > Yes, that looks good to me. Thanks! Sudip, do you want to send the patch, or I manage to do it? Maxime ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 9:20 ` Maxime Coquelin @ 2016-05-25 2:00 ` Sudip Mukherjee 0 siblings, 0 replies; 13+ messages in thread From: Sudip Mukherjee @ 2016-05-25 2:00 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 24 May 2016 02:50 PM, Maxime Coquelin wrote: > 2016-05-24 10:58 GMT+02:00 Arnd Bergmann <arnd@arndb.de>: >> On Tuesday, May 24, 2016 10:50:17 AM CEST Maxime Coquelin wrote: >>> 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, >>> >> >> Yes, that looks good to me. > > Thanks! > Sudip, do you want to send the patch, or I manage to do it? Maybe you should send it, i have not done anything in reaching its final form. Regards Sudip ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 8:50 ` Maxime Coquelin 2016-05-24 8:58 ` Arnd Bergmann @ 2016-05-24 10:09 ` Daniel Thompson 2016-05-24 10:57 ` Maxime Coquelin 1 sibling, 1 reply; 13+ messages in thread From: Daniel Thompson @ 2016-05-24 10:09 UTC (permalink / raw) To: linux-arm-kernel On 24/05/16 09:50, Maxime Coquelin wrote: > 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; Minor quibble but I might prefer that the error handling/recovery actually be put on the error path itself (included in the if (sr != RNG_SR_DRDY) ). Daniel. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-24 10:09 ` Daniel Thompson @ 2016-05-24 10:57 ` Maxime Coquelin 0 siblings, 0 replies; 13+ messages in thread From: Maxime Coquelin @ 2016-05-24 10:57 UTC (permalink / raw) To: linux-arm-kernel 2016-05-24 12:09 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>: > On 24/05/16 09:50, Maxime Coquelin wrote: >> >> 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; > > > Minor quibble but I might prefer that the error handling/recovery actually > be put on the error path itself (included in the if (sr != RNG_SR_DRDY) ). Yes, it would be better. Regards, Maxime ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-23 20:35 ` Arnd Bergmann 2016-05-24 7:59 ` Maxime Coquelin @ 2016-05-25 2:05 ` Sudip Mukherjee 2016-05-25 10:06 ` Arnd Bergmann 1 sibling, 1 reply; 13+ messages in thread From: Sudip Mukherjee @ 2016-05-25 2:05 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 24 May 2016 02:05 AM, Arnd Bergmann wrote: > On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote: >> We have been getting build warning about: >> drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': >> drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used >> uninitialized in this function >> >> On checking the code it turns out that sr can never be used >> uninitialized as sr is getting initialized in the while loop and while >> loop will always execute as the minimum value of max can be 32. >> So just initialize sr to 0 while declaring it to silence the compiler. >> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >> --- > > I notice that you are using a really old compiler. While this warning > seems to be valid in the sense that the compiler should figure out that > the variable might be used uninitialized, please update your toolchain > before reporting other such problems, as gcc-4.6 had a lot more false > positives that newer ones (5.x or 6.x) have. yes, i need to upgrade gcc in my travis bot. But in my local system I am having gcc-4.8.4 and there also I am having this error and i am sure 4.8.4 is still being used by many people. Regards Sudip ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-25 2:05 ` Sudip Mukherjee @ 2016-05-25 10:06 ` Arnd Bergmann 2016-05-27 9:00 ` Sudip Mukherjee 0 siblings, 1 reply; 13+ messages in thread From: Arnd Bergmann @ 2016-05-25 10:06 UTC (permalink / raw) To: linux-arm-kernel On Wednesday, May 25, 2016 7:35:17 AM CEST Sudip Mukherjee wrote: > On Tuesday 24 May 2016 02:05 AM, Arnd Bergmann wrote: > > On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote: > >> We have been getting build warning about: > >> drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': > >> drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used > >> uninitialized in this function > >> > >> On checking the code it turns out that sr can never be used > >> uninitialized as sr is getting initialized in the while loop and while > >> loop will always execute as the minimum value of max can be 32. > >> So just initialize sr to 0 while declaring it to silence the compiler. > >> > >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> > >> --- > > > > I notice that you are using a really old compiler. While this warning > > seems to be valid in the sense that the compiler should figure out that > > the variable might be used uninitialized, please update your toolchain > > before reporting other such problems, as gcc-4.6 had a lot more false > > positives that newer ones (5.x or 6.x) have. > > yes, i need to upgrade gcc in my travis bot. But in my local system I am > having gcc-4.8.4 and there also I am having this error and i am sure > 4.8.4 is still being used by many people. Right, the change from gcc-4.8 to 4.9 is what drastically changed hte maybe-uninitialized warnings, introducing a number of additional warnings (many of them correct) but removing many others (mostly false positives). I tend to care only about the ones in 4.9+ for this reason. I haven't run statistics on this in a while, but I guess we could consider turning off this warning for 4.8 and earlier (though IIRC the switch to turn it off only appeared in 4.9). BTW, regarding your build infrastructure, I'd also recommend building with 'make -s' to make the output more compact. Arnd ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] hwrng: stm32 - fix build warning 2016-05-25 10:06 ` Arnd Bergmann @ 2016-05-27 9:00 ` Sudip Mukherjee 0 siblings, 0 replies; 13+ messages in thread From: Sudip Mukherjee @ 2016-05-27 9:00 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 25 May 2016 03:36 PM, Arnd Bergmann wrote: > On Wednesday, May 25, 2016 7:35:17 AM CEST Sudip Mukherjee wrote: >> On Tuesday 24 May 2016 02:05 AM, Arnd Bergmann wrote: >>> On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote: >>>> We have been getting build warning about: >>>> drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read': >>>> drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used >>>> uninitialized in this function >>>> >>>> On checking the code it turns out that sr can never be used >>>> uninitialized as sr is getting initialized in the while loop and while >>>> loop will always execute as the minimum value of max can be 32. >>>> So just initialize sr to 0 while declaring it to silence the compiler. >>>> >>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >>>> --- >>> >snip> > > BTW, regarding your build infrastructure, I'd also recommend building > with 'make -s' to make the output more compact. travis is having a timeout and if there is no output from the build within a time limit then it will cancel the build. I can use the option if i can increase the timeout limit. I will have a look at the options. Thanks for the idea. Regards Sudip ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-05-27 9:00 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-23 12:44 [PATCH] hwrng: stm32 - fix build warning Sudip Mukherjee 2016-05-23 20:35 ` Arnd Bergmann 2016-05-24 7:59 ` Maxime Coquelin 2016-05-24 8:32 ` Arnd Bergmann 2016-05-24 8:50 ` Maxime Coquelin 2016-05-24 8:58 ` Arnd Bergmann 2016-05-24 9:20 ` Maxime Coquelin 2016-05-25 2:00 ` Sudip Mukherjee 2016-05-24 10:09 ` Daniel Thompson 2016-05-24 10:57 ` Maxime Coquelin 2016-05-25 2:05 ` Sudip Mukherjee 2016-05-25 10:06 ` Arnd Bergmann 2016-05-27 9:00 ` Sudip Mukherjee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).