On 5/22/2011 11:29 PM, 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.
I am glad you are looking at our discussion. I am sure we are going to need all the help/oversight/questions that we can get, as this is a change that will affect all architectures.
Le 23/05/2011 07:25, Graeme Russ a écrit :

On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
<jwilliamcampb...@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
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).
I think you are correct, it was the rate function desired here. I think the best way to go is use a "get_raw_tick_rate_in_mhz" function, because it is probably the easiest one to implement, and in many cases something like it already exists.
   - 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
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.
Exactly so. Note that this same function probably needs to be called in udelay for the same reason. More precisely, the get_timer function will call get_raw_ms, which will call get_raw_ticks. I think it may be better to move get_timer "down" a level in the hierarchy, so we don't need a get_raw_ms. get_timer would then be part of the HAL. One would use a get_timer(0) in order to do what get_raw_ms alone would have done. If the user had a good reason, he would then override get_timer with his own version. What do you think Graeme? It reduces the nesting depth by one level. As for the name change to get_timer_in_ms, I would support it. Naturally, such a change would be up to Mr. Denk. Since by definition that is what the function does, it seems to be a good change from my point of view.
   - 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 was referenced as a library function a few lines above. Right now, I think the functionality we require from the HAL is

  1. get_raw_tick_rate_in_mhz
  2. get_raw_ms
  3. get_raw_ticks
  4. (optional)get_raw_us

There is also  APIs for these functions called get_timer.

I think we need to add a call to another function called "initialize_timer_system" or similar that will initialize the data structures in gd by calling get_raw_tick_rate_in_mhz. Additionally, I think we need to provide a udelay function, simply because it can interact with calling get_raw_ms often enough. We are somewhat caught between two fires here, in that on the one hand we want to provide a very "generic" approach to the timing system that will work on "any" CPU while on the other hand we want to allow easy customization/simplification of the timer system where appropriate. There is more than one way to approach this. Another approach is to define the HAL as:

  1. get_timer (_in_ms if approved)
  2. udelay
  3. (optional)get_raw_us
  4. get_raw_tick_rate_in_mhz
  5.   initialize_timer_system

We would provide a "reference" implementation of get_timer and udelay with comments regarding the implementation of the get_raw_ticks functionality (static inline comes to mind). This would provide a functional but possibly not optimal timer system. Later, once everything is working, the user could "hack up" the get_timer routines and the initialize_timer_system routines to remove any functionality that was not relevant to his system, such as using a fixed clock rate etc. The benefit of this approach is that in simple systems where space is real tight one can still utilize a minimal, optimal timer system without the extra code required to make it generic. There are no "extra levels". The disadvantage of this approach is that it allows the user to wander off into an incompatible non-standard implementation. Thoughts on this subject are welcome. The design we have will work, but it will require somewhat more text than the "present" system. How much more, and does it matter, I don't know.
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
I share your concern. We may have to iterate a bit.

Best Regards,
Bill Campbell

Regards,

Graeme
Amicalement,

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to