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

[J. William Campbell <jwilliamcampbell@comcast.net>]

On 5/25/2011 4:13 PM, Graeme Russ wrote:
> Hi Wolfgang
>
> On Thu, May 26, 2011 at 7:16 AM, Wolfgang Denk<wd@denx.de>  wrote:
>> Dear Graeme Russ,
>>
>> In message<4DDD7066.4000505@gmail.com>  you wrote:
>>>> No, not at all. And I already answered this. For example on PPC, just
>>>> reading the timebase would be perfectly sufficient, and simpler and
>>>> more reliable than the current interrupt based approach.
>>> I assume by 'timebase' you mean the 64-bit tick counter. If so, that is
>> By timebase I mean the timebase register, implemented as two 32 bit
>> registers tbu and tbl, holding the upper and the lower 32 bits of the
>> free-running 64 bit counter, respective.
> And remember, not all platforms have this implementation. The AMD sc520
> for example has a microsecond register which counts 0-999 that ticks a
> 16-bit millisecond register and resets to zero. And the millisecond
> register latches the value of the microsecond register and resets
> (the millisecond register) back to zero.
>
> The thing is, this can all be abstracted away via get_tick() which
> (provided it is called every 65 seconds or so) can maintain a software
> version of the timebase register. So, every 65 seconds, the prescaler
> needs to be kicked. Now, if all we want to use get_timer() for is to
> monitor a timeout (which I think might be every single use in U-Boot
> to date) then the while (get_timer(start)<  timeout) loop will work. If
> get_timer() is needed to measure time between two arbitrary events (which
> I 100% agree it should be able to do) then the prescaler will need to be
> kicked (typically by an interrupt)
>
>>> _exactly_ what I am suggesting we do (and what does already happen on ARM).
>> I don't think so.
Hi All,
       Just to be clear, while ARMv7 has a 64 bit performance counter, 
it is not presently used by get_time. This is a change we want to make 
correct?
> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
> udelay() both end up calling get_timer_raw() (with udelay only having
> millisecond resolution it seems).
I am not sure why you say at91 appears to use interrupts. There is a 
comment in arch/arm/cpu/arm930t/at91/timer.c that says "timer without 
interrupts" (line 73). There is the same comment in 
arch/arm/cpu/arm930t/at91rm9200/timer.c Nothing in either routine refers 
to interrupts, so I would say the timer doesn't use them. I could be 
wrong of course.
>   Some others can be configured to
> increment the timer using an interrupt. ARM is, quite frankly, a complete
> mess - It has a mass of *_timer_masked() functions which the core timer
> functions are 'wafer thin' wrapper around, udelay() silently resets
> the timebase trashing get_timer() loops etc.
I sure agree with this last part. The only arm timer I found that 
clearly thought it could use interrupts was in arch/arm/cpu/ixp, and 
that was conditional, not mandatory.
> So let's wind back and distill the approach I am suggesting:
>
>   1) A common prescaler function in /lib/ - It's purpose is to maintain
>      a 1ms resolution timer (if the platform cannot otherwise do so)[1]
>      The prescaler utilises a platform provided get_ticks()[2]
>   2) A get_ticks() function provided by the platform - This function must
>      return an unsigned counter which wraps from all 1's to all 0's - It
>      DOES NOT have to be initialised to zero at system start. get_ticks()
>      hides the low-level tick counter implementation - The sc520 example
>      above is a classic example, so is your PPC tbu/tbl example.
>   3) [Optional]An ISR which calls the prescaler[3]
>
> Now there is an optimisation if your tick counter has a 1ms resolution
> and is not small (i.e. 64-bits) - The prescaler is defined weak, so in
> the platform code, re-implement the prescaler to simply copy the tick
> counter to the timer variable.
>
> And what are the specific implementation types (in decending order of
> preference)? I think:
>   1) A 64-bit micro-second tick counter[5]
>        - No interrupts needed
>        - Can be used by udelay() and get_timer() trivially
>   2) A 64-bit sub-micro-second tick counter
>        - Interrupts most likely undeeded unless the tick frequency is
>          insanely high
>        - Can be used by udelay() and get_timer() trivially
>   3) A 64-bit milli-second tick counter
>        - No interrupts needed
>        - No prescaler needed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   4) A 32-bit milli-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is ~50 days
>        - ISR needed to kick prescaler if events longer than 50 days need
>          to be timed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   5) A 24-bit milli-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is ~4.5 hours
>        - ISR needed to kick prescaler if events longer than 4.5 hours need
>          to be timed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   6) A 32-bit micro-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is 71 minutes
>        - ISR needed to kick prescaler if events longer than 71 minutes need
>          to be timed
>        - Can be used by get_timer() trivially
I think this should be "Can be used by udelay and get_timer trivially"
Best Regards,
Bill Campbell
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>
> Any implementation which does not fit withing the above is going to
> require an ISR to kick the prescaler in order to support timing of 'long
> events' (i.e. not just simple timeout loops)
>
> [1]The prescaler would still be needed by platforms which has a 64-bit
> tick counter which ticks at a rate greater than 1ms
> [2]Exposing get_ticks() reduces code duplication
> [3]Only required if the rollover time of the tick counter (i.e. the maximum
> permissible time between any two get_ticks() calls) is 'small'[4]
> [4]'small' is at the discretion of the implementer - 1 second is always
> small, 1 hour might be, 500 years is not
> [5]A tick counter is something maintained by the underlying platform
> independent of any U-Boot code
> [6]Although wise to override the prescaler function so the timer ISR is
> consistent with all other platforms
>
> Regards,
>
> Graeme
>
>