[U-Boot] [RFC] ARM timing code refactoring

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4D3AAF63.1030600 at free.fr> you wrote:
> 
> Agreed for unnecessary mult-div, but 64-bit we would not avoid, and 
> should not IMO, when the HW has it.

When attempting to come up with true generic code, we should probably
_always_ use a (virtual) unsigned 64 bit counter.

> > u32 timeout = timeout_init(100); /* 100ms timeout */
> >
> > do {...} while (!timed_out(timeout));
> >
> > Internally it would be like:
> >
> > timeout_init(x):
> > return fast_tick + (x * fast_tick_rate) / CONFIG_SYS_HZ;
> > /* this might need 64 bit precision in some implementations */
> >
> > time_out(x):
> > return ((i32)(x - fast_tick)) < 0;
> >
> > If the tick were really high speed (and then 64 bits), fast_tick
> > could be derived by shifting the tick some bits to the right.
> 
> The only thing I slightly dislike about the overall idea is the signed 
> rather than unsigned comparison in the timeout function (I prefer using 
> the full 32-bit range, even if only as an academic point) and the fact 
> that the value of the timeout is encoded in advance in the loop control 
> variable 'timeout'.

Please don't.  Use an unsigned counter and allow it to roll over.
Anything else causes additional (and unnecessary) restrictions and
makes the code more complicated.

> I'd rather have a single 'time(x)' (or 'ticks_elapsed(x)', names are 
> negotiable) macro which subtract its argument from the current ticks, 

I'm not sure what you have in mind with "substract".  I strongly
recommend to avoid problems with wrap-around etc. right from the
beginning, and let unsigned arithmentcs handle this for us.

> e.g. 'then = time(0)' would set 'then' to the number of ticks elapsed 
> from boot, while 'now = time(then)' would set 'now' the ticks elapsed 
> from 'then'; and a 'ms_to_ticks(x)' (again, or 'milliseconds(x)') :

Do we really need such a function?  As far as I can tell we don't
really have any time (in the sense of wallclock time) related
requirements, we only need delta times, and nobody really cares about
the starting point.

We should _always_ be able to use the standard approach of

	start = get_timer()

	while ((get_timer() - start) < timeout) {
		... wait ...
	}

> Your example would then become
> 
> 	then = time(0);
> 	do {...} while ( time(then) < ms_to_ticks(100) );

We should NOT do this.  It is bound to break as soon as the code 
in the loop (the "..." part) needs to implement a timeout, too.

> 	#define now() time(0)
>   	#define ms_elapsed(then,ms) ( time(then) < ms_to_ticks(ms) )

This is not a good isea for the same reason.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There has been an alarming increase in the number of things you  know
nothing about.