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

[J. William Campbell]

On 5/23/2011 6:19 AM, Wolfgang Denk wrote:
> Dear Graeme Russ,
>
> In message<4DDA5334.4060308 at 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?
Hi all,
      FWIW,  "cooked" ticks would be 1 ms ticks, although we never 
really use them as such.
>> 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.
Correct. Any free running counter whose resolution is better than one 
millisecond and which is "long enough" that it will not overflow between 
calls to get_timer is sufficient.
>> 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.
I have looked at it as a reference. However, there is one disadvantage 
in using the PPC code as a reference. It has a 64 bit timestamp. Many 
systems do not have a 64 bit timestamp, but rather a 32 bit timestamp.  
It is possible to extend the 32 bit timestamp to a 64 bit timestamp if 
get_timer is called often enough that there will only be a single 
rollover of the bottom 32 bits  between uses. However, if that condition 
is met, there is no need to extend the timer to 64 bits. Instead, just 
convert the elapsed time since the last call (which you know) to ms and 
be done with it.  As Wolfgang said above, any counter that has better 
than 1 ms resolution  is adequate to the task. The additional 
requirement that we have stated is that the counter be long enough that 
it does not overflow between calls to get_timer. If the counter is 64 
bits long, it pretty much for sure meets this requirement  (although 
bits below 0.5 ms resolution really don't help any). If the timer is 32 
bits long, it will meet any requirements using get_timer to time out 
hardware intervals. My original implementation used a 32 bit divide and 
does exactly that. This is the shortest and simplest approach, and we 
can get that working in all cases quite easily I think. We can avoid the 
"no divide" optimization until everybody is satisfied with what we have.
>>>>   - 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.
That is fine with me. The reason this was being done was to attempt to 
emulate, as much as possible, the power PC, where the 64 bit timestamp 
counter allows calls to get_timer separated by many minutes and several 
console commands to work properly. These get timer commands will NOT 
work properly on systems that have a 32 bit counter that overflows every 
200 seconds or so. The call in the idle loop was an attempt to make the 
32 bit systems work more like the 64 bit systems. One may then either

   1.   Define calls to get_timer to measure an elapsed interval
      separated by any returns to the command processor as broken.
   2. Require the use of interrupts to extend the 32 bit timestamp.
      (This may not be possible on all systems as the timer used for
      performance monitoring does not interrupt, etc.)
   3. Allow the call in the idle loop under the assumption that we are
      talking about timing in the minutes range, not a few seconds.

If you go with number 1, all problems are solved.
>> 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 get_timer is used to timeout failing hardware operations, the CPU is 
usually in a reasonably tight loop and get_timer will certainly be 
called at least once a minute. In this case, all will be well. If 
get_timer is used to obtain a timestamp during one u-boot command to be 
compared with a timestamp taken by a later command (say inside a 
script), it will not function properly if the total delay (or the delay 
in any one command) exceeds the hardware timer rollover interval. The 
elapsed time returned by the second call to get_timer will be wring 
modulo the actual number of rollovers that occurred. With an added call 
to get_timer inside the command loop, the condition becomes the delay in 
any one command, not the total delay. This is still broken, just not as 
bad. If 1 was selected above, no problem.
>>>>   - 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.
When you say a "common base", consider that all systems do not have 64 
bit timers, and extending the 32 bit timer that they do have to 64 bits 
in software is not possible if the number of rollovers between calls to 
the timer update routine is unknown. OTOH, if the number of rollovers is 
assumed to be at most 1, there is no benefit to extending the counter to 
64 bits, as the elapsed time is already known exactly and can be 
converted to ms exactly. We would extending the timer to 64 bits 
(incrementing the top 32 bits)to then do another subtraction to get the 
number we already have. If 32 bits is too short, and that is all you 
have, you are doomed anyway.

Best Regards,
Bill Campbell

>>> 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.
>
> Best regards,
>
> Wolfgang Denk
>