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

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

On 23/05/11 23:19, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4DDA5334.4060308@gmail.com> you wrote:
>>
>>>>  - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>>    and get_tick_rate() to correctly maintain the ms counter used by
>>>>    get_timer() - This function can be weak (so next point)
>>>
>>> Ditto.  What would that do?  If it gets milliseconds as the name
>>> suggest, that's already the function needed for get_timer()?
>>
>> OK, there appears to be a consensus that not all hardware actually supports
>> a free-running timer with 1ms resolution. To overcome this, the idea is to
> 
> Indeed.  I guess most of them do not.
> 
>> create a common library function which maintains the free running counter.
>> The library function acts as a pre-scaler using a 'raw tick counter' and a
>> 'raw tick rate' supplied by the low level architecture. We define this weak
> 
> What are "raw" ticks?  And what are "cooked" ticks, then?
> 

We never talked about cooked tick - I agree 'raw' is superfluous

>> so that if the architecture can provide a free running 1ms counter, there
>> is no (code size) penalty
> 
> Why do we need a free running 1ms counter at all?  Any free running
> counter of at least millisecoind resolution should be good enough.
> 

Because it get_timer() is 1ms - Yes, get_timer() can calculate it from a
higher resolution source, but that creates multiple implementations of the
pre-scaler of multiple arches - The goal is to reduce code duplication
here. get_timer() calls get_raw_ms() (ok, we could just call that get_ms)
which is a common library routine which does the pre-scaling. Arches which
can maintain this 1ms counter (via interrupts or native hardware counters
for example) don't need it.

>> This approach eliminates all the existing per-arch code which (attempts) to
>> manage the time base behind get time. So we simplify each arch down to it's
>> bare essentials - Provide a counter which increments at a natural fixed
>> rate and what the rate is - Let common library code deal with the rest.
> 
> Did you have a look at the PowerPC implementation?  I'd like to see
> this used as reference.
> 
>>>>  - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>>    depend on the raw tick rate, but I image it will never be necessary
>>>>    to call more often than once every few minutes)
>>>
>>> NAK. This concept is fundamentally broken.  I will not accept it.
>>
>> Some existing timers are fundamentally broken - The glitch at the
>> 0xffffffff to 0x00000000 rollover or rollover early - The method discussed
>> in this thread eliminates all such glitches. Provided pre-scaler in /lib/
>> (triggered by get_timer() usually) is called often enough (71 minutes for a
>> 32-bit 1MHz counter) then there is no need. Even then, it is only important
> 
> We already have this nightmare of code for triggering the watchdog on
> systems that use it.
> 
> Assuming there are places in the main loop that get executed often
> enough is a broken concept, and I will not accept any such code.
> 
>> over the time period you are measuring (i.e. two independent 5s delays 2
>> hours apart will not be a problem)
> 
> What is the practical purpose of get_timer()?  What is the longest
> interval we have to cover?  And what is the problem with a rollover?
> 
>>>>  - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>    the main loop is required
>>>
>>> We should make no such requirements.
>>
>> No such requirement of what?
> 
> Of making any calls in the main loop.
> 
>>> Can we not start simple, say by a plain free-runnign 64 bit counter,
>>> be it implemented in hardwar eor in software?  On PowerPC, we have
>>
>> That's exactly what we are suggesting - Let the hardware be free to
>> implement the counter at whatever frequency suits it. 64-bit is not needed
>> in reality
> 
> It may not be needed on some systems, but may be needed on others
> where 32 bit is too short.  Let's use the common base that is know to
> work on all systems, even if it's not strictly needed on all of them.
> 
>>> this immediately in form of the time base register (or more precisely
>>> in form of the two 32 bit registers tbu and tbl representing time base
>>> upper and time base lower).
>>>
>>> Modelling a similar interface using _any_ kind of timer service should
>>> be trivial.  And from there, we can just use the existing code again.
>>
>> Yes, have the library function manage the actual '1ms' aspect
> 
> This is what PPC is doing. And I understand that Reinhard did the same
> in software for AT91.

Yes, duplicate code - lets 'libify' it

Regards,

Graeme