* [U-Boot] [PATCH] timer: Fix possible underflow in udelay
@ 2016-08-25 18:40 Andrew F. Davis
2016-08-25 19:02 ` Troy Kisky
0 siblings, 1 reply; 3+ messages in thread
From: Andrew F. Davis @ 2016-08-25 18:40 UTC (permalink / raw)
To: u-boot
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)
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* [U-Boot] [PATCH] timer: Fix possible underflow in udelay
2016-08-25 18:40 [U-Boot] [PATCH] timer: Fix possible underflow in udelay Andrew F. Davis
@ 2016-08-25 19:02 ` Troy Kisky
2016-08-25 19:18 ` Andrew F. Davis
0 siblings, 1 reply; 3+ messages in thread
From: Troy Kisky @ 2016-08-25 19:02 UTC (permalink / raw)
To: u-boot
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)
^ permalink raw reply [flat|nested] 3+ messages in thread* [U-Boot] [PATCH] timer: Fix possible underflow in udelay
2016-08-25 19:02 ` Troy Kisky
@ 2016-08-25 19:18 ` Andrew F. Davis
0 siblings, 0 replies; 3+ messages in thread
From: Andrew F. Davis @ 2016-08-25 19:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-08-25 19:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-25 18:40 [U-Boot] [PATCH] timer: Fix possible underflow in udelay Andrew F. Davis
2016-08-25 19:02 ` Troy Kisky
2016-08-25 19:18 ` Andrew F. Davis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox