[U-Boot-Users] PATCH: fix timer overflow in DaVinci
Hi,
The get_timer() function in DaVinci's timer.c doesn't handle overflow -- it simply subtracts the "base" from the current time, but if the timer overflowed and the current time is smaller than base, a negative number results. The attached patch fixes that.
The bug manifests itself e.g. when writing large blocks of flash (which take many minutes to complete), and it so happens that a write operation starts before the overflow and then times out prematurely because of it.
Please CC replies to me because I'm not on the list...
Regards, --Alex
In message 265CBF1670611D47B47E67959D02EBE3C381D8@mngilex001.Jerusalem.mangodsp.com you wrote:
The get_timer() function in DaVinci's timer.c doesn't handle overflow -- it simply subtracts the "base" from the current time, but if the timer overflowed and the current time is smaller than base, a negative number results. The attached patch fixes that.
I think this is the wrong approach. get_timer() should not have to deal with wrap arounds, because get_timer_masked() is suppsoed to handle this internally. So please fix it there.
When reposting, please don't forhet to add your Signed-off-by: line.
Content-Type: text/plain; name="timer-patch.txt" Content-Transfer-Encoding: base64 Content-Description: timer-patch.txt Content-Disposition: attachment; filename="timer-patch.txt"
LS0tIGNwdS9hcm05MjZlanMvZGF2aW5jaS90aW1lci5jLm9yaWcJMjAwNy0xMC0xNiAyMTo1OTow MS4wMDAwMDAwMDAgKzAyMDAKKysrIGNwdS9hcm05MjZlanMvZGF2aW5jaS90aW1lci5jCTIwMDct MDktMTEgMjE6NTM6MDMuMDAwMDAwMDAwICswMzAwCkBAIC04Niw3ICs4NiwxMCBAQAogCiB1bG9u ZyBnZXRfdGltZXIodWxvbmcgYmFzZSkKIHsKLQlyZXR1cm4oZ2V0X3RpbWVyX21hc2tlZCgpIC0g YmFzZSk7CisJdWxvbmcgZ3RtID0gZ2V0X3RpbWVyX21hc2tlZCgpOworCWlmKGd0bSA8IGJhc2Up ICAvKiBvdmVyZmxvdzsgYXNzdW1lIG5vIG1vcmUgdGhhbiBvbmUgLS0gMTU5IHNlY29uZHMgKi8K KwkJZ3RtICs9ICgweGZmZmZmZmZmVUwgLyBUSU1FUl9MT0FEX1ZBTCk7CisJcmV0dXJuIGd0bSAt IGJhc2U7CiB9CiAKIHZvaWQgc2V0X3RpbWVyKHVsb25nIHQpCg==
And send your patch inline and as plain text, not as a base64 encoded attachment.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 265CBF1670611D47B47E67959D02EBE3C381D8@mngilex001.Jerusalem.mangodsp.com you wrote:
The get_timer() function in DaVinci's timer.c doesn't handle overflow -- it simply subtracts the "base" from the current time, but if the timer overflowed and the current time is smaller than base, a negative number results. The attached patch fixes that.
I think this is the wrong approach. get_timer() should not have to deal with wrap arounds, because get_timer_masked() is suppsoed to handle this internally. So please fix it there.
Just for my understanding:
In cpu/arm926ejs/davinci/timer.c the overflow of the timer itself is handled, but it is timestamp that overflows?
static ulong timestamp; ... ulong get_timer_raw(void) { ulong now = READ_TIMER;
if (now >= lastinc) { /* normal mode */ timestamp += now - lastinc; } else { /* overflow ... */ timestamp += now + TIMER_LOAD_VAL - lastinc; } lastinc = now; return timestamp; }
With READ_TIMER running at 27MHz and timestamp being 32bit timestamp overflows after ~159s?
Seems that code above is used in a lot timer modules, not only for DaVinci. Then, it depends on READ_TIMER frequency how fast timestamp overflows?
Best regards
Dirk
In message 47163D08.8050402@googlemail.com you wrote:
In cpu/arm926ejs/davinci/timer.c the overflow of the timer itself is handled, but it is timestamp that overflows?
It cannot wrap around, but that is not an overflow. It's an unsigned int.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 265CBF1670611D47B47E67959D02EBE3C381D8@mngilex001.Jerusalem.mangodsp.com you wrote:
The get_timer() function in DaVinci's timer.c doesn't handle overflow -- it simply subtracts the "base" from the current time, but if the timer overflowed and the current time is smaller than base, a negative number results. The attached patch fixes that.
I think this is the wrong approach. get_timer() should not have to deal with wrap arounds, because get_timer_masked() is suppsoed to handle this internally. So please fix it there.
Do you like to test this? It should decrease the counter values from the timer running at high frequency by division. With this, we should have some more time before timestamp wraps around.
Signed-off-by: Dirk Behme dirk.behme@gmail.com
Index: uboot_davinci/cpu/arm926ejs/davinci/timer.c =================================================================== --- uboot_davinci.orig/cpu/arm926ejs/davinci/timer.c +++ uboot_davinci/cpu/arm926ejs/davinci/timer.c @@ -61,6 +61,11 @@ davinci_timer *timer = (davinci_timer * #define TIMER_LOAD_VAL (CFG_HZ_CLOCK / CFG_HZ) #define READ_TIMER timer->tim34
+/* Timer runs with CFG_HZ_CLOCK, currently 27MHz. To avoid wrap + around of timestamp already after min ~159s, divide it, e.g. by 16. + timestamp will then wrap around all min ~42min */ +#define DIV(x) ((x) >> 4) + static ulong timestamp; static ulong lastinc;
@@ -101,20 +106,20 @@ void udelay(unsigned long usec)
void reset_timer_masked(void) { - lastinc = READ_TIMER; + lastinc = DIV(READ_TIMER); timestamp = 0; }
ulong get_timer_raw(void) { - ulong now = READ_TIMER; + ulong now = DIV(READ_TIMER);
if (now >= lastinc) { /* normal mode */ timestamp += now - lastinc; } else { /* overflow ... */ - timestamp += now + TIMER_LOAD_VAL - lastinc; + timestamp += now + DIV(TIMER_LOAD_VAL) - lastinc; } lastinc = now; return timestamp; @@ -122,7 +127,7 @@ ulong get_timer_raw(void)
ulong get_timer_masked(void) { - return(get_timer_raw() / TIMER_LOAD_VAL); + return(get_timer_raw() / DIV(TIMER_LOAD_VAL)); }
void udelay_masked(unsigned long usec)
participants (3)
-
Alex Shnitman -
Dirk Behme -
Wolfgang Denk