[U-Boot] [RFC][Timer API] Revised Specification - Implementation details

[Graeme Russ <graeme.russ@gmail.com>]

On 16/06/11 02:03, Simon Glass wrote:
> Hi Graeme,
> 
> On Wed, Jun 15, 2011 at 6:17 AM, Graeme Russ <graeme.russ@gmail.com> wrote:
>> Hi Wolfgang,
> ...
>>>
>>>       /*
>>>        * round - used to control rounding:
>>>        * <0 : round down, return time that has passed AT LEAST
>>>        * =0 : don't round, provide raw time difference
>>>        * >0 : round up, return time that has passed AT MOST
>>>        */
>>>        u32 delta_timer(u32 from, u32 to, int round)
>>>        {
>>
>> [snip]
>>
>>>       }
>>
>> I decided to implement three separate functions:
>>
>> u32 time_ms_delta_min(u32 from, u32 to)
>> u32 time_ms_delta_max(u32 from, u32 to)
>> u32 time_ms_delta_raw(u32 from, u32 to)
>>
>> So if you only use one, the rest get stripped out of the binary
> 
> Here is m 2p worth:
> 
> - the common case is min I think, so let's get rid of the min prefix
> so everyone will use it without question or needing to read screeds of
> doc

I don't like this idea - Extrapolate this to dropping the 'ms' common case
and you end up with:

u32 time_delta(u32 from, u32 to)
u32 time_delta_max(u32 from, u32 to)
u32 time_delta_raw(u32 from, u32 to)

u32 time_us_delta(u32 from, u32 to)
u32 time_us_delta_max(u32 from, u32 to)
u32 time_us_delta_raw(u32 from, u32 to)

Not as grep'able IMHO

> - would prefer the ms and us at the end as I think it reads better.
> Getting the time is the important bit - the units are generally at the
> end

Hmm, time_since_ms or time_ms_since - I suppose either reads OK - But if I
keep min/max/raw, I find time_min_ms_since (barely) easier in the eye than
time_min_since_ms - I'm not bothered either way and will go with the
general consensus

> This code from your excellent wiki page bothers me. Can we find a way
> to shrink it?
> 
>                 now = timer_ms_now();
>                 if (time_ms_delta_min(start, now)> timeout)
> 
> How about:
> 
>                 if (time_since_ms(start) > timeout)
> 
> The idea of the time 'since' an event is more natural than the delta
> between then and now which seems more abstract.

I tend to agree - I have been thinking about 'since' functions for a while now

[snip]

>>
>> With the 'time_ms_' prefix, it's starting to get rather long, and I'm
>> pushing a lot of timeout checks beyond 80 columns - especially when
>> existing code has variables like 'start_time_tx' - I'm starting to consider
>> dropping the time_ prefix (all time functions will still have a ms_ or us_
>> prefix anyway) and rename a lot of variables
> 
> Now I see why you want units at the start. Seems a bit ugly to me -
> which lines are getting long - do you mean the time delta check line?
> If so see above, or perhaps it is shorter without _min.

An example from drivers/net/ns7520_eth.c:

	ulStartJiffies = time_ms_now();
	while (time_ms_delta_min(ulStartJiffies, time_ms_now())
			< NS7520_MII_NEG_DELAY) {

Could be reduced to:

	ulStartJiffies = time_ms_now();
	while (time_min_ms_since(ulStartJiffies) < NS7520_MII_NEG_DELAY) {

And with a rename:

	start_ms = time_ms_now();
	while (time_min_ms_since(start_ms) < NS7520_MII_NEG_DELAY) {

Regards,

Graeme