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