On 16/06/11 02:03, Simon Glass wrote:
> Hi Graeme,
>
> On Wed, Jun 15, 2011 at 6:17 AM, Graeme Russ <graeme.r...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
