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

[Reinhard Meyer <u-boot@emk-elektronik.de>]

Dear Albert ARIBAUD,
> Hi All,
>
> I am starting this thread to revive and, hopefully, come to a general
> agreement on how timers should be implemented and used in the ARM
> architecture, and get rid of current quick fixes. Let us start with
> Reinhard's proposal:
>
>> There were several suggestions about that in the past (including from
>> me) that involve rework everywhere HZ related timeouts are used. I
>> still prefer a method as follows (because it does not need repeated
>> mul/div calculations nor necessarily 64 bit arithmetic):
>
> Agreed for unnecessary mult-div, but 64-bit we would not avoid, and
> should not IMO, when the HW has it.
>
>> u32 timeout = timeout_init(100); /* 100ms timeout */
>>
>> do {...} while (!timed_out(timeout));
>>
>> Internally it would be like:
>>
>> timeout_init(x):
>> return fast_tick + (x * fast_tick_rate) / CONFIG_SYS_HZ;
>> /* this might need 64 bit precision in some implementations */
>>
>> time_out(x):
>> return ((i32)(x - fast_tick))<  0;
>>
>> If the tick were really high speed (and then 64 bits), fast_tick
>> could be derived by shifting the tick some bits to the right.
>
> The only thing I slightly dislike about the overall idea is the signed
> rather than unsigned comparison in the timeout function (I prefer using
> the full 32-bit range, even if only as an academic point) and the fact
> that the value of the timeout is encoded in advance in the loop control
> variable 'timeout'.

1. you always need signed compares there, unless you never anticipate a
rollover of your timer value to zero.
2. whats the problem with initializing the timeout value at the beginning?

>
> I'd rather have a single 'time(x)' (or 'ticks_elapsed(x)', names are
> negotiable) macro which subtract its argument from the current ticks,
> e.g. 'then = time(0)' would set 'then' to the number of ticks elapsed
> from boot, while 'now = time(then)' would set 'now' the ticks elapsed
> from 'then'; and a 'ms_to_ticks(x)' (again, or 'milliseconds(x)') :
>
> 	#define time(x) (ticks - x)
> 	#define ms_to_ticks(m) ( (m * fast_tick_rate) / CONFIG_SYS_HZ)

We have exactly an equivalent of this in use at AT91.
It works only as long as the ticks value does not roll back to zero -
which in the current 64 bit implementation I did is after the end of
the universe...

Note also that "fast_tick_rate" would not be a constant in AT91, it is
dynamically calculated from main xtal frequency measured against the 32kHz
xtal and PLL settings.

>
> Note that time(x) assumes unsigned arguments and amounts to an unsigned
> compare, because we're always computing an difference time, i.e. even
> with x = 2 and ticks = 1, the result is correct -- that's assuming ticks
> is monotonous 32-bits (or 64-bits for the platforms that can support it
> as an atomic value)

Assume: Monotonous AND never wrapping back to zero!

>
> Your example would then become
>
> 	then = time(0);
> 	do {...} while ( time(then)<  ms_to_ticks(100) );

That looks ugly to me. We don't want to see the high speed(64 bit) values
in the drivers - I think.

>
> ... moving the actual timeout value impact from the time sample before
> the 'while' to the 'while' condition at then end.

Which does a multiply and a divide in 64 bit each loop iteration...
(fast_tick_rate being a variable)

>
> For expressiveness, added macros such as:
>
> 	#define now() time(0)
>    	#define ms_elapsed(then,ms) ( time(then)<  ms_to_ticks(ms) )
>
> ... would allow writing the same example as:
>
> 	then = now();
> 	do {...} while ( !ms_elapsed(then,100) );
>

Why make everything so complicated???

>> But, as long as we cannot agree on something, there will be no
>> time spent to make patches...
>
> Makes sense, hence this specific thread. :)

The how-many-th thread about timers is this ? :)

Best Regards,
Reinhard