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

[J. William Campbell <jwilliamcampbell@comcast.net>]

On 5/27/2011 8:13 AM, Simon Glass wrote:
> On Fri, May 27, 2011 at 8:00 AM, J. William Campbell
> <jwilliamcampbell@comcast.net>  wrote:
> [snip]
>> Hi All,
>>      A more precise statement of the problem is that all timer delays
>> may be shortened by the timer resolution. So this means that if you have
>> a timeout of 1 ms in your get_time(0) {   } while ( ...<  1), then your
>> actual delay may be anywhere between 0 and 1 ms. The problem arises when
>> some piece of common code uses a delay of say 8 millisec, expecting the
>> actual delay to be between 7 and 8. If the resolution is 10 ms, the
>> delay will be between 0 and 10 ms, 0 being particularly bad. This can be
>> fixed in get_timer, making the 8 ms delay  become a minimum of 10 ms at
>> the expense of it becoming up to 20 ms sometimes. Since these delays are
>> used mostly for error conditions, making them longer will probably be
>> ok, and doesn't require changing any of the common code. It probably
>> will not make things slower either, because the error timeouts should
>> not be reached. The reset of the hardware timer would cause all "short"
>> delays to become 10 ms. This reset approach is bad in that it prevents
>> proper nesting of timing loops. However, in this case it isn't so bad,
>> in that the nested loops are just extended, not shortened. Note that if
>> the reset is only resetting the HARDWARE interrupt generator, not the
>> actual timestamp itself, we are just extending all existing timeouts by
>> 0 to 10 ms.. So this just lengthens all pending timeouts. The other fix
>> is in my opinion nicer, because it affects the nest loops less. If the
>> inner loop is executed 100 times, with the reset, the outer loop timeout
>> is extended by up to 1000 ms.
>>
>> Best Regards,
>> Bill Campbell
> Hi Bill,
>
> Yes I agree that this is ugly - I didn't realize that this is what
> reset_timer() does, but I think these 10ms platforms should have to
> live with the fact that timeouts will be 0-10ms longer than hoped.
> Perhaps reset_timer() should become a non-standard board thing that is
> deprecated. Really if you have a 10ms timer and are asking for a 10ms
> timeout you are being a bit hopeful.
Hi All,
     Yes, but the person writing the driver was writing "common" code. 
He probably didn't even know there was a timer whose resolution was not 
1 ms.
> But perhaps this argues for a function to check timeouts - at the
> moment get_timer() returns the time since an event and it is used at
> the start of the loop and the end. Perhaps we should have:
>
> #define TIMEOUTMS 2000
>
> stop_time = get_future_time(TIMEOUT_MS);  // Returns current time +
> TIMEOUT_MS + (resolution of timer)
> while (get_timer(stop_time)<  0)     // (I would much prefer while
> (!timed_out(stop_time))
>     wait for something
> }
>
> Regards,
> Simon
In the existing system, you can get the same result by running the while 
loop  with a condition of (get_timer(base) < TIMEOUTMS + TIMER_RESOLUTION).
We could just make TIMER_RESOLUTION a mandatory define for all u-boots. 
Then common code would be wrong if the TIMER_RESOLUTION were omitted. 
For all I know, there may be such a define already. Anybody know of one?

Best Regards,
Bill Campbell