[U-Boot] [PATCH v2] mmc: omap: timeout counter fix

[J. William Campbell]

On 10/25/2010 11:01 PM, Reinhard Meyer wrote:
> Dear Wolfgang Denk,
>> Dear Reinhard Meyer,
>>
>> In message<4CC66A67.4000608 at emk-elektronik.de>   you wrote:
>>>> It fails in case the timer wraps around.
>>>>
>>>> Assume 32 bit counters, start time = 0xFFFFFFF0, delay = 0x20. It
>>>> will compute end = 0x10, the while codition is immediately false, and
>>>> you don't have any delay at all, which most probably generates a
>>>> false error condition.
>>> I used and assumed a 64 bit counter, that will not wrap around while
>>> our civilization still exists...
>>
>> The code is still wrong, and as a simple correct implementation exists
>> there is no excuse for using such incorrect code.
>>
>> Please fix that!
> Agreed here. People are invited to dig through u-boot and find all
> those places.
>
>>> If get_ticks() is only 32 bits worth, both methods will misbehave
>>> at a 32 bit wrap over.
>> No.
>>
>>>> 	start = time();
>>>> 	while ((time() - start)<    delay)
>>>> 		...
>>>>
>>>> This works much better (assuming unsigned arithmetics).
>>> True, provided the underlying timer is really 64 bits, otherwise
>>> this fails, too...
>   >  You are wrong. Try for example this:
>   >
>   >  --------------------- snip -------------------
>   >  #include<stdio.h>
>   >
>   >  int main(void)
>   >  {
>   >  	unsigned int time = 0xFFFFFFF0;
>   >  	unsigned int delay = 0x20;
>   >  	unsigned int start;
>
> You are wrong here, because you take it out of context.
> My "demo" is using the (declared as) 64 bit function get_ticks().
> I mentioned above that this function MUST be truly returning 64
> bits worth of (incrementing) value to make any version work.
> If get_ticks() just returns a 32 bit counter value neither method will work
> reliably. Just check all implementations that this function is implemented
> correctly.
Hi All,
       I have wondered for quite some time about the rush to make 
get_ticks() return a 64 bit value. For any "reasonable" purpose, like 
waiting a few seconds for something to complete, a 32 bit timebase is 
plenty adequate. If the number of ticks per second is 1000000000, i.e. a 
1 GHz clock rate, the clock wraps in a 32 bit word about every five 
seconds.
The trick is that time always moves forward, so a current get_ticks() - 
a previous get_ticks() is ALWAYS a positive number. It is necessary to 
check the clock more often than (0X100000000 - your_timeout) times per 
second, but unless your timeout is very near the maximum time that fits 
into 32 bits, this won't be a problem. Most CPUs have a counter that 
count at a "reasonable" rate. Some CPUs also have a "cycle counter" that 
runs at the CPU clock rate. These counters are useful to determine 
exactly how many machine cycles a certain process took, and therefore 
they have high resolution. Timers for simple delays neither need nor 
want such resolution. If the only counter available on you CPU runs at 
several GHz, and is 64 bits long,  just shift it right a few bits to 
reduce the resolution to a "reasonable"  resolution and return a 32 bit 
value. There is no need for a bunch of long long variables and extra 
code running around to process simple timeouts. It may be that we need a 
different routine for precision timing measurements with high 
resolution, but it needn't, and probably shouldn't IMHO be get_ticks().

Best Regards,
Bill Campbell
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>