[U-Boot] [RFC] ARM timing code refactoring

[Reinhard Meyer]

On 23.01.2011 21:59, Albert ARIBAUD wrote:
> Le 23/01/2011 20:35, Wolfgang Denk a écrit :
>
>> At the moment I would suggest to change the existing interface like
>> that:
>>
>> * Drop the set_timer() function.
>>
>> * Change get_timer() to take no argument, i. e.:
>>
>> unsigned long get_timer(void);
>>
>> get_timer() returns a monotonous upward counting time stamp with a
>> resolution of milliseconds. After reaching ULONG_MAX the timer wraps
>> around to 0.

Exactly that wrap makes the situation so complicated, since the simple code
u32 get_timer(void)
{
  return (ticks * 1000ULL) / tickspersec;
}
won't do that wrap.

>>
>> The get_timer() implementation may be interrupt based and is only
>> available after relocation.

Currently it is used before relocation in some places, I think I have
seen it in NAND drivers... That would have to be changed then.

>>
>> * Provide a fast, low-level, system dependent timer function
>>
>> unsigned long long get_ticks(void);
>>
>> get_ticks() returns a monotonous upward counting time stamp with a
>> system-specific resolution. No assumptions should be made about the
>> resolution. After reaching ULLONG_MAX the timer wraps around to 0.
>>
>> It is mandatory that get_ticks() is available before relocation.
>>
>> * Provide a set of utility functions:
>>
>> -> void wait_ticks(unsigned long ticks);
>>
>> Delay execution for "ticks" ticks.
>>
>> -> unsigned long usec2ticks(unsigned long usec);
>>
>> Convert microseconds into ticks; intended for SHORT delays only
>> (maximum depending on system clock, usually below 1 second).
>>
>> -> void __udelay(unsigned long usec);
>>
>> Delay execution for "usec" microseconds; intended for SHORT delays
>> only (maximum depending on system clock, usually below 1 second).
>> If all architectures followed the above suggestion, we could move
>> the PPC implementation to common code:
>>
>> void __udelay(unsigned long usec)
>> {
>> ulong ticks = usec2ticks(usec);
>> wait_ticks(ticks);
>> }
>>
>> __udelay() can reliably be used before relocation.
>>
>> -> void udelay(unsigned long usec)
>>
>> Similar to __udelay() with the additional functionality to trigger
>> the watchdog timer for long delays.
>>
>>
>>
>>> that will not be possible on most hardware without complicated code.
>>> We have discussed that long ago...
>>
>> I am aware of this.
>>
>>> Well, you could try to understand:
>>> tick=the "at hardware speed running" timer, if that's incrementing too fast for
>>> 32 bit "timeout" vars for reasonable timeouts (up to a minute?),
>>
>> See above. For short, high resolution timeouts you can use
>> get_ticks() and friends. For long delays you can use get_timer().
>>
>> Note that "reasonable timeouts (up to a minute?)" are only very
>> infrequently needed, and don't need the high resolution of
>> get_ticks(), so these would naturally be implemented on the base of
>> get_timer().
>>
>>
>> We have been using this implementation for more than a decade on
>> PowerPC. The only thing you need is a monotonous upward counting
>> 64 bit "time base" counter where you can read the system ticks from.
>>
>> Best regards,
>>
>> Wolfgang Denk
>
> This proposal covers what I was thinking of (oubviously I had not looked into PPC implementations) and the few differences with my proposal are not worth fighting over, so overall I am fine with the above.
>
> Let us hear from others now, and if we reach an agreement, then we'll start discussing implementation.
>
> Amicalement,

This is already implemented functionally very closely (apart from factoring and the
get_timer(void) change) to this in AT91, the only (academic) hitch is that it will
burp a few billion years after each reset :)

Check arch/arm/cpu/arm926ejs/at91/timer.c

What bothers me is the need for 64 bit mul/div in each loop iteration, for CPUs without
hardware for that this might slow down data transfer loops of the style

u32 start_time = get_timer();
do {
	if ("data_ready")
		/* transfer a byte */
	if (get_timer() - start_time > timeout)
		/* fail and exit loop */
} while (--"bytestodo" > 0);

since get_timer() will be somewhat like:

	return (tick * 1000ULL) / tickspersec;

As I stated before, tickspersec is a variable in, for example, AT91. So the
expression cannot be optimized by the compiler.

Reinhard