| Message ID | 20160825184058.2071-1-afd@ti.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
On 8/25/2016 11:40 AM, Andrew F. Davis wrote: > When the inputed usec is too large we process it in chunks of > CONFIG_WD_PERIOD size. Subtracting this from usec until usec is > zero. If usec is not an integer multiple of CONFIG_WD_PERIOD it > will underflow and the condition will not become false when it > should. Fix this logic. > > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > lib/time.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/lib/time.c b/lib/time.c > index f37150f..4ec3cb9 100644 > --- a/lib/time.c > +++ b/lib/time.c > @@ -145,14 +145,14 @@ void __weak __udelay(unsigned long usec) > > void udelay(unsigned long usec) > { > - ulong kv; > + ulong kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; > + ulong elapsed = 0; > > do { > WATCHDOG_RESET(); > - kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; > - __udelay (kv); > - usec -= kv; > - } while(usec); > + __udelay(kv); > + elapsed += kv; > + } while (elapsed < usec); > } > > void mdelay(unsigned long msec) > The original code looks fine to me. Can you give an example of failure ? ie. If udelay is passed value of CONFIG_WD_PERIOD+1, the udelay sequence will be udelay(CONFIG_WD_PERIOD) udelay(1) whereas the need code does udelay(CONFIG_WD_PERIOD) udelay(CONFIG_WD_PERIOD)
On 08/25/2016 02:02 PM, Troy Kisky wrote: > On 8/25/2016 11:40 AM, Andrew F. Davis wrote: >> When the inputed usec is too large we process it in chunks of >> CONFIG_WD_PERIOD size. Subtracting this from usec until usec is >> zero. If usec is not an integer multiple of CONFIG_WD_PERIOD it >> will underflow and the condition will not become false when it >> should. Fix this logic. >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> --- >> lib/time.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/lib/time.c b/lib/time.c >> index f37150f..4ec3cb9 100644 >> --- a/lib/time.c >> +++ b/lib/time.c >> @@ -145,14 +145,14 @@ void __weak __udelay(unsigned long usec) >> >> void udelay(unsigned long usec) >> { >> - ulong kv; >> + ulong kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; >> + ulong elapsed = 0; >> >> do { >> WATCHDOG_RESET(); >> - kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; >> - __udelay (kv); >> - usec -= kv; >> - } while(usec); >> + __udelay(kv); >> + elapsed += kv; >> + } while (elapsed < usec); >> } >> >> void mdelay(unsigned long msec) >> > > The original code looks fine to me. Can you give an example of failure ? > ie. > If udelay is passed value of CONFIG_WD_PERIOD+1, the udelay sequence will be > > udelay(CONFIG_WD_PERIOD) > udelay(1) > > whereas the need code does > udelay(CONFIG_WD_PERIOD) > udelay(CONFIG_WD_PERIOD) > Hmm, I'm not sure where I saw the problem before, I think I may have tried to optimize it and broke it myself, oh well, this patch can safely be ignored. Thanks, Andrew
diff --git a/lib/time.c b/lib/time.c index f37150f..4ec3cb9 100644 --- a/lib/time.c +++ b/lib/time.c @@ -145,14 +145,14 @@ void __weak __udelay(unsigned long usec) void udelay(unsigned long usec) { - ulong kv; + ulong kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; + ulong elapsed = 0; do { WATCHDOG_RESET(); - kv = usec > CONFIG_WD_PERIOD ? CONFIG_WD_PERIOD : usec; - __udelay (kv); - usec -= kv; - } while(usec); + __udelay(kv); + elapsed += kv; + } while (elapsed < usec); } void mdelay(unsigned long msec)
When the inputed usec is too large we process it in chunks of CONFIG_WD_PERIOD size. Subtracting this from usec until usec is zero. If usec is not an integer multiple of CONFIG_WD_PERIOD it will underflow and the condition will not become false when it should. Fix this logic. Signed-off-by: Andrew F. Davis <afd@ti.com> --- lib/time.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)