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
- 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
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.
The ms_ part allows for:
u32 time_us_delta_min(u32 from, u32 to) u32 time_us_delta_max(u32 from, u32 to) u32 time_us_delta_raw(u32 from, u32 to)
I intend to let the time_us_delta* functions drop to ms resolution of the underlying tick counter is not sub-millisecond. Where the tick counter is microsecond (or better), then arch-specific udelay becomes a trivial implementation of a loop using time_us_now() and time_us_delta_min() - Actually, we can make this a weak default and have arches with a non microsecond tick counter override it.
So our timeout from case 1) above would now be written like this:
u32 start,now; u32 timeout = 5 * CONFIG_SYS_HZ; /* timeout after 5 seconds */
start = get_timer(0);
I now have: start = time_ms_now();
while (test_for_event() == 0) { now = get_timer(0);
if (delta_timer(start, now) > timeout) handle_timeout();
udelay(100); }
and would be guaranteed never to terminate early.
Comments?
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.
Regards, Simon
Thoughts?
Regards,
Graeme