[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Thu, May 26, 2011 at 2:19 PM, Reinhard Meyer
<u-boot at emk-elektronik.de> wrote:
>
> Dear Graeme Russ,
>>
>> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
>> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
>> udelay() both end up calling get_timer_raw() (with udelay only having
>> millisecond resolution it seems). Some others can be configured to
>> increment the timer using an interrupt. ARM is, quite frankly, a complete
>> mess - It has a mass of *_timer_masked() functions which the core timer
>> functions are 'wafer thin' wrapper around, udelay() silently resets
>> the timebase trashing get_timer() loops etc.
>
> Please look at current master for at91.
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/at91/timer.c;h=a0876879d3907af553d832bea187a062a22b9bd4;hb=5d1ee00b1fe1180503f6dfc10e87a6c6e74778f3
>
> AT91 uses a 32 bit hardware register that by means of a prescaler is made
> to increment at a rate in the low megahertz range.

Yes, I see that now

> This results in a wrap approximately every 1000 seconds.
> Actually this would be sufficient for all known uses of udelay() and get_timer()
> timeout loops. However, this hardware register is extended to 64 bits by software
> every time it is read (by detecting rollovers).

Which makes it 100% compatible with my proposed solution - The software
prescaler will trigger the 64-bit extension and rollover detection

> Since a wrap of that 64 bit "tick" would occur after the earth has ended,
> it is simple to obtain milliseconds from it by doing a 64 bit division.

Which would be done in the common prescaler in /lib/

Currently, most ARM specific utilisations of get_timer() enforce a reset
of the tick counter by calling reset_timer() - Subsequent calls to
get_timer() then assume a start time of zero. Provided the internal timer
rolls over currectly, the initial call of get_timer(0) will reset the ms
timer and remove and 'glitch' present due to not calling the 'extender'
function between 32-bit rollovers which makes the reset_timer() call
unneccessary - I believe at91 behaves correctly in this regard.

In any case, the underlying assumption made by the ARM timer interface
(call reset_timer() first always) is inherently broken as not all users
of the timer API do this - They assume a sane behaviour of:

	start = get_timer(0);
	elapsed_time = get_timer(start);

Add to this udelay() resetting the timer make the following very broken:

	start = get_timer(0);
	while(condition) {
		udelay(delay);
	}
	elapsed_time = get_timer(start);

NOTE: In this case, if udelay() also calls the prescaler then no interrupt
triggered every 1000s would be required in the above example to get
correct elapsed_time even if the loop ran for several hours (provided
udelay() is called at least every 1000s

However, to allow timing of independent events with no intervening
udelay() or get_timer() calls, an 1000s interrupt to kick the prescaler is
all that is needed to make this particular implementation behave correctly.
Of course disabling interruts and not calling get_timer() or udelay() will
break the timer - But there is nothing that can be done about that)

Regards,

Graeme