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

[Graeme Russ]

On 23/05/11 16:29, Albert ARIBAUD wrote:
> Hi all,
> 
> Sorry, could not follow the discussion although I find it very 
> interesting, so I will handle the task of coming in late and asking the 
> silly questions.
> 
> Le 23/05/2011 07:25, Graeme Russ a écrit :
> 
>> On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
>> <jwilliamcampbell at comcast.net>  wrote:
>>> On 5/22/2011 6:42 PM, Graeme Russ wrote:
>>>>
>>>> OK, so in summary, we can (in theory) have:
>>>>   - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>>>>   - A HAL with two functions
>>>>     - u32 get_raw_ticks()
>>>>     - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>>>>       max's out at just after 4GHz)
>>>>   - 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)
>>>>   - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>>>>     can be overridden to simply return the hardware counter. In this case,
>>>>     get_raw_ticks() would return 1
> 
> Are you sure you did not mean 'get_raw_ticks_rate' here? Besides, I'd 

Yes, I did mean get_raw_ticks_rate()

> like the name to specify the units used: 'get_raw_ticks_rate_in_khz' (or 
> conversively 'get_raw_ticks_per_ms', depending on which is simpler to 
> implement and use).
> 

indeed, get_raw_ticks_per_ms() is a better name

>>>>   - 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)
> 
> That's to keep track of get_raw_ticks() rollovers, right? And then the 

Yes

> get_timer function (which, again, I would prefer to have '... in ms' 
> expressed in its name) would call get_raw_ticks() in confidence that at 
> most one rollover may have occurred since the last time the helper 
> function was called, so a simple difference of the current vs last tick 
> value will always be correct.

Correct - And a change to get_ms_timer() could be in order if generally
supported, but this would be a big patch across a lot of code for a simple
rename

>>>>   - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>     the main loop is required
>>>>   - An optional HAL function u32 get_raw_us() which can be used for
>>>>     performance profiling (must wrap correctly)
>>>
>>> Hi All,
>>>       Graeme, I think you have stated exactly what is the "best" approach to
>>> this problem.  I will provide a version of "get_raw_ms" that is  initialized
>>> using get_raw_tick_rate that will work for all "reasonable" values of
>>> raw_tick_rate. This will be the "generic" solution. Both the initialization
>>> function and the get_raw_ms function can be overridden if there is reason to
>>> do so, like "exact" clock rates. I will do the same with get_raw_us. This
>>> will be posted sometime on Monday for people to review, and to make sure I
>>> didn't get too far off base. Thank you to both Graeme and Reinhard for
>>> looking at/working on this.. Hopefully, this solution will put this timing
>>> issue to rest for all future ports as well as the presently existing ones.
> 
> In Greame's description, I did not see a get_raw_ms, only a get_raw_us. 
> Was this last one a typo or is that a third HAL function?

get_raw_ms() is used by get_timer(). get_raw_us() is an optional function
which can be used for performance profiling


>> Great - I am in the middle of cleaning up the current usages of the timer
>> API, reducing it all down to get_timer() - I will then 'libify'
>> get_timer() and setup the hooks into the HAL get_raw_ticks() and
>> get_raw_tick_rate() API
>>
>> I think there will need to be a lot of cleanup, especially in arch/arm to
>> get this to all fit
>>
>> Regards,
>>
>> Graeme
> 
> Amicalement,

Regards,

Graeme