[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Alex Shnitman]

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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: timer-patch.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20071016/201e6929/attachment.txt 

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Superior ability breeds superior ambition.
	-- Spock, "Space Seed", stardate 3141.9

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Dirk Behme]

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

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Don't worry about people stealing your ideas. If your ideas are  any
good, you'll have to ram them down people's throats."  - Howard Aiken

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Dirk Behme]

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>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: increase_timestamp_wraparound_time.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20071020/d65a00d9/attachment.txt 

[U-Boot-Users] PATCH: fix timer overflow in DaVinci

[Dirk Behme]

Peter Pearse wrote:
> Dirk, Alex
> 	Did this get tested? 

I looked again into it. As I now have real hardware, I can test it.

> If so did it pass?

Yes. At least for me ;) I tested with (x) >> 1 (division by 2, didn't 
want to wait ~42 minutes) and the wrap around time was doubled.

> 	Shall I treat it as a submitted patch?

Yes, please. Thanks for asking!

Best regards

Dirk

>>-----Original Message-----
>>From: u-boot-users-bounces at lists.sourceforge.net 
>>[mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
>>Of Dirk Behme
>>Sent: 20 October 2007 07:24
>>To: Alex Shnitman
>>Cc: u-boot-users at lists.sourceforge.net
>>Subject: Re: [U-Boot-Users] PATCH: fix timer overflow in DaVinci
>>
>>Wolfgang Denk wrote:
>>
>>>In message 
>>
>><265CBF1670611D47B47E67959D02EBE3C381D8@mngilex001.Jerusalem.m
> 
> angodsp.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>
>>
> 
> 
>