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

[Graeme Russ <graeme.russ@gmail.com>]

On 25/05/11 00:19, Wolfgang Denk wrote:
> Dear Scott McNutt,
> 
> In message <4DDBB29D.2050102@psyent.com> you wrote:
>>
>> Why must get_timer() be used to perform "meaningful time measurement?"
> 
> Excellent question!  It was never intended to be used as such.

Because get_timer() as it currently stands can as it is assumed to return
milliseconds

> Also, neither udelay() nor get_timer() make any warranties about
> accuracy or precision. If they are a few percent off, that's perfectly
> fine. Even if they are 10% off this is not a big problem anywhere.
> 
>> If all of this is about time measurement, why not start with your dream
>> time measurement API, and go from there? And make it an optional
>> feature.
> 
> D'acore.
> 
>> If the existing API, that has been used successfully for years,
>> primarily as a mechanism for detecting a time-out, has issues,
>> then let's resolve _those_ issues and avoid the "car-boat" ... the
>> vehicle that floats like a car and handles like a boat. ;-)
> 
> :-)

OK, let's wind back - My original suggestion made no claim towards changing
what the API is used for, or how it looks to those who use it (for all
practical intents and purposes). I suggested:
 - Removing set_timer() and reset_timer()
 - Implement get_timer() as a platform independent function

Lets look at two real-world implementations of my suggested solution - One
which 'exposes' ticks and one which does not..

=============
Exposing ticks and tick_frequency to everyone via a 'tick' HAL

In /lib/timer.c

void prescaler(void)
{
	u32 ticks = get_ticks();
	u32 tick_frequency = get_tick_frequency();

	/* Bill's algorithm */

	/* result stored in gd->timer_in_ms; */
}

u32 get_timer(u32 base)
{
	prescaler(ticks, tick_frequency);

	return gd->timer_in_ms - base;
}

In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c

u32 get_ticks(void)
{
	u32 ticks;

	/* Get ticks from hardware counter */

	return ticks;
}

u32 get_tick_frequency(void)
{
	u32 tick_frequency;

	/* Determine tick frequency - likely very trivial */

	return tick_frequency;
}

=======================
Not exposing ticks and tick_frequency to everyone

In /lib/timer.c

void prescaler(u32 ticks, u32 tick_frequency)
{
	u32 current_ms;

	/* Bill's algorithm */

	/* result stored in gd->timer_in_ms; */
}

In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c

static u32 get_ticks(void)
{
	u32 ticks;

	/* Get ticks from hardware counter */

	return ticks;
}

static u32 get_tick_frequency(void)
{
	u32 tick_frequency;

	/* Determine tick frequency */

	return tick_frequency;
}

u32 get_timer(u32 base)
{
	u32 ticks = get_ticks();
	u32 tick_frequency = get_tick_frequency();

	prescaler(ticks, tick_frequency);

	return gd->timer_in_ms - base;
}

===============
I personally prefer the first - There is only one implementation of
get_timer() in the entire code and the platform implementer never has to
concern themselves with what the tick counter is used for. If the API gets
extended to include get_timer_in_seconds() there is ZERO impact on
platforms. Using the second method, any new feature would have to be
implemented on all platforms - and we all know how well that works ;)

And what about those few platforms that are actually capable of generating
a 1ms timebase (either via interrupts or natively in a hardware counter)
without the prescaler? Well, with prescaler() declared weak, all you need
to do in /arch/cpu/soc/timer.c or /arch/cpu/timer.c or
/board/<board>/timer.c is:

For platforms with a 1ms hardware counter:
void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
{
	gd->timer_in_ms = get_milliseconds();
}

For platforms with a 1ms interrupt source:
void timer_isr(void *unused)
{
	gd->timer_in_ms++;
}

void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
{
}


And finally, if the platform supports interrupts but either the hardware
counter has better accuracy than the interrupt generator or the interrupt
generator cannot generate 1ms interrupts, configure the interrupt generator
to fire at any rate better than the tick counter rollover listed in
previous post and:

void timer_isr(void *unused)
{
	/*
	 * We are here to stop the tick counter rolling over. All we
	 * need to do is kick the prescaler - get_timer() does that :)
	 */
	get_timer(0);
}

In summary, platform specific code reduces to:
 - For a platform that cannot generate 1ms interrupts AND the hardware
   counter is not in ms
   - Implement get_ticks() and get_tick_frequency()
   - Optionally implement as ISR to kick the prescaler
 - For a platform that can generate 1ms interrupts, or the hardware
   counter is in ms
   - Override the prescaler() function to do nothing

If none of the above not look 'simple', I invite you to have a look in
arch/arm ;)

Regards,

Graeme