[U-Boot] TIMER cleanup RFC, was: [PATCH 4/4] arm920t/at91/timer: replace bss variables by gd

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

On 11/30/2010 1:14 AM, Reinhard Meyer wrote:
> Dear Wolfgang Denk,
>
> what we really need is only a 32 bit monotonous free running tick that increments
> at a rate of at least 1 MHz. As someone pointed out a while ago, even at 1GHz that would
> last for four seconds before it rolls over. But a 1HGz counter could be 64 bit internally
> and always be returned as 32 bits when it is shifted right to bring it into the "few" MHz
> range.
>
> Any architecture should be able to provide such a 32 bit value. On powerpc that would
> simply be tbu|tbl shifted right a few bits.
>
> An architecture and SoC specific timer should export 3 functions:
>
> int timer_init(void);
> u32 get_tick(void); /* return the current value of the internal free running counter */
> u32 get_tbclk(void); /* return the rate at which that counter increments (per second) */
>
> A generic timer function common to *most* architectures and SoCs would use those two
> functions to provice udelay() and reset_timer() and get_timer().
> Any other timer functions should not be required in u-boot anymore.
>
> However get_timer() and reset_timer() are a bit of a functional problem:
>
> currently reset_timer() does either actually RESET the free running timer (BAD!) or
> remember its current value in another (gd-)static variable which later is subtracted
> when get_timer() is called. That precludes the use of several timers concurrently.
>
> Also, since the 1000Hz base for that timer is usually derived from get_tick() by
> dividing it by some value, the resulting 1000Hz base is not exhausting the 32 bits
> before it wraps to zero.
>
> Therefore I propose two new functions that are to replace reset_timer() and get_timer():
>
> u32 init_timeout(u32 timeout_in_ms); /* return the 32 bit tick value when the timeout will be */
> bool is_timeout(u32 reference); /* return true if reference is in the past */
>
> A timeout loop would therefore be like:
>
> u32 t_ref = timeout_init(3000);	/* init a 3 second timeout */
>
> do ... loop ... while (!is_timeout(t_ref));
>
> coarse sketches of those functions:
>
> u32 init_timeout(u32 ms)
> {
> 	return get_ticks() + ((u64)get_tbclk() * (u64)ms) / (u64)1000;
> }
>
> bool is_timeout(u32 reference)
> {
> 	return ((int)get_ticks() - (int)reference)>  0;
> }
>
> Unfortunately this requires to "fix" all uses of get_timer() and friends, but I see no other
> long term solution to the current incoherencies.
>
> Comments welcome (and I would provide patches)...
Hi All,
       The idea of changing the get_timer interface to the 
init_timeout/is_timeout pair has the advantage that it is only necessary 
to change the delay time in ms to an internal timebase once, and after 
that, only a 32-bit subtraction is required. I do not however like the 
idea of using 64 bit math to do so, as on many systems this is quite 
expensive. However, this is a feature that can be optimized for 
particular CPUs. I also REALLY don't like the idea of having a get_ticks 
function, because for sure people will use this instead of the desired 
interface to the timer because it is "better". Then we get back into a 
mess. Since in most cases get_ticks is one or two instructions, please, 
let us hide them in init_timeout/is_timeout.
        An alternate approach, which has the merit of being more like 
the originally intended interface, simply disallows reset_timer since it 
is  totally unnecessary. The only dis-advantage of the original approach 
using just get_timer is that the conversion to ms must be considered at 
each call to get_timer, and will require at a minimum one 32 bit integer 
to remember the hardware timer value the last time get_timer was called 
(unless the hardware time can be trivially converted to a 32 bit value 
in ms, which is quite uncommon). This is not a high price to pay, and 
matches the current usage. This is probably for Mr. Denk to decide. If 
we were just starting now, the init_timeout/is_timeout is simpler, but 
since we are not, perhaps keeping the current approach has value.
        I would really like to help by providing some patches, but I am 
just way too busy at present.

Best Regards,
Bill Campbell
> Reinhard
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>