On 01.11.2010 14:47, J. William Campbell wrote:
> On 10/27/2010 11:02 PM, Reinhard Meyer wrote:
>> Dear J. William Campbell,
>>> Hi All,
>>>
>>> I am pretty sure the migration to 64 bits was caused by 1) people not
>>> understanding that the timer operating on time DIFFERENCES would work fine
>>> even if the underlying timer wrapped around (most probable problem) and
>>> possibly 2) broken timer functions causing bogus timeouts, improperly
>>> "fixed" by switching to 64 bits.
>>>
>>> I think u-boot could get along just fine with only 2 time related
>>> functions, uint_32 get_timer(uint_32 base) and udelay(uint 32 delay).
>>> udelay will only work on "small" values of delay, on the order of
>>> milliseconds. It is to be used when a "short" but "precise" delay in
>>> microsecond resolution is required. Users of get_timer must understand that
>>> it is only valid if it is called "often enough", i.e. at least once per
>>> period of the underlying timer. This is required because u-boot does not
>>> want to rely on interrupts as a timer update method. Therefore, all uses of
>>> get_timer must 1) call it once initially to get a start value, and 2) call
>>> get_timer at least once per period of the underlying hardware counter. This
>>> underlying period is guaranteed to be at least 4.29 seconds (32 bit counter
>>> at 4 GHz). Note that this does NOT mean that the total wait must be less
>>> than 4.29 seconds, only that the rate at which the elapsed time is TESTED
>>> must be adequate.
>>>
>>> In order to implement this functionality, at least one hardware timer of
>>> some kind is required. An additional software "timer" in 1 ms resolution
>>> may be useful in maintaining the software time. If the hardware timer
>>> update rate is programmable, u-boot MAY set the update rate on
>>> initialization On initialization, u-boot MAY reset the hardware timer and
>>> MAY reset any associated software timer. The hardware timer MAY be started
>>> on initialization. On each call to get_timer(), u-boot MUST start the
>>> hardware timer if it was not started already. On calls to get_timer, u-boot
>>> MUST NOT reset the hardware timer if it was already started. The software
>>> timer MAY be reset if u-boot can unambiguously determine that more than
>>> 4.29 seconds has elapsed since the last call to get_timer.
>>>
>>> The simplest case for implementing this scheme is if two programmable
>>> timers exist that can be set to 1ms and 1us. The timers are initialized at
>>> start-up, get_timer just returns the 32 bit 1 ms timer and udelay just
>>> waits for the number of ticks required on the second timer to elapse. The
>>> most common harder case is where there is only one timer available, it is
>>> running at 1 us per tick or faster, and we cannot control the rate. udelay
>>> is still easy, because we can convert the (small) delay in us to a delay in
>>> ticks by a 32 bit multiply that will not overflow 32 bits even if we have
>>> quite a few fractional bits in the tics per microsecond value. The elapsed
>>> ticks required is the (delay in us * us/per tick) >> # fractional bits in
>>> us/per tick. If that is not close enough for you, you can do it as (delay
>>> in us * (integer part of us/tick)) + ((delay in us * (fractional
>>> part)us/tick) >> # fraction bits). For "nice" numbers, like any integral
>>> number of MHz, there is no fraction
al
>>
>>> part. Only numbers like 66 MHz, or 1.666 GHz require messing with the
>>> fractional part.
>>> For get_timer, it is a bit harder. The program must keep two 32 bit global
>>> variables, the timer reading "last time" and the software timer in 1 ms
>>> resolution. Whenever get_timer is called, it must increase the software
>>> timer by the number of ms that have elapsed since the previous update and
>>> record the corresponding timer reading as the new "last time". Note that if
>>> the number of ms elapsed is not an integer (a common case), the value
>>> recorded as the "last time" must be decreased by the number of ticks not
>>> included in the 1 ms resolution software timer. There are many different
>>> ways to accomplish update, depending on what hardware math capabilities are
>>> available, and whether one thinks efficiency is important here.
>>> Conceptually, you convert the elapsed time in ticks into an equivalent
>>> number of ms, add that number to the software timer, store the current
>>> value of the hardware timer in last time, and subtract any "remainder"
>>> ticks from that value. If the elapsed time is l
es
>> s
>>> that one ms, do no update of "last hardware time" and return the current
>>> software counter. If the elapsed time is greater than 4.29 seconds, reset
>>> the software counter to 0, record the current hardware counter time and
>>> return the current software counter. In between, do the math, which will
>>> fit into 32 bits.
>>>
>>> If this idea seems like a good one, I can provide more detail on the
>>> conversions for various hardware capabilities is people want. Comments
>>> welcome.
>>
>> To get the timer mess cleaned up three things have to happen:
>>
> Hi All,
>
> I am glad somebody was still interested. I was afraid I had scared everyone
> off.
>> 1. A consensus and documentation how it MUST be handled
>>
>> 2. Fix all timer implementations to adhere to that
>>
>> 3. Fix all timer uses to adhere to that
> I agree this is required.
>>
>> To start, RFC:
>>
>> a) udelay(u32) MUST NOT interfere with other timer functions
>> b) u32 get_timer(u32 base) MUST return (current_time - base) using
>> millisecond units
>> c) get_timer() MUST exhaust the full 32 bit range before wrapping
>> d) to ensure the function of internal high frequency wrapping
>> processes, get_timer() MUST be called at least once a second while
>> a timeout loop is run (this will typically be the case anyway)
> I have no problem with this, but it might be stricter than needed. I cant
> imagine anybody has a timer running faster than 4 GHz, and that allows 4
> seconds of interval.
4 GHz / 2**32 yields 1 second. Assuming no prescaler...
just theoretical anyway. A polling loop with timeout that does poll that seldom
is illogical anyway.
>> e) reset_timer() is not needed, potentially harmful if used and
>> shall be removed. This will also allow for nested timeouts, e.g.
>> inner timeout for accessing a hardware and outer timeout for having
>> the hardware perform a function
>> f) get_ticks() and get_tbclk() SHALL NOT be used to check for timeouts
>> except for cases where the better than ms resolution is really needed
> I have an issue here. I think no such use cases really exist. For things like
> bit-banging, udelay is more what one wants. What are the counter-examples
> where sub-millisecond resolution in a loop waiting for some expected event is
> required?
Well, the "really needed" clause says it. I don't see it either.
>> g) get_ticks() MUST return true 64 bit time
> I hope get_ticks must not exist, at least as a "generic" globally visible
> u-boot function. Having it present spawns confusion. People will use it
> instead of the generic get_timer when they don't have to, creating the
> portability problems we are trying to get rid of.
I also agree it would be best to get rid of externally available get_ticks()
and friends.
>> h) all uses of get_ticks() and get_timer() shall be fixed to use
>> get_timer() as follows:
>>
>> u32 start_ms;
>>
>> start_ms = get_timer(0);
>>
>> any_type_of_loop {
>> ...
>> /* check for timeout */
>> if (get_timer(start_ms) >= TIMEOUT_IN_MS)
>> /* handle timeout */
>> ...
>> }
>> Alternative, but in contrast to current get_timer() implementation
>> is to drop the get_timer parameter and do the subtraction in the if.
> No problem here.
How much code uses get_ticks, or get_timer, or just uses a counted
loop of small udelays? I have seen all variants but never made stats
of that.
We probably should leave get_timer() as it is.
>>
>> --> get_timer(base) is a bit of a misnomer, it should be better named
>> something like get_ms_since(base).
>>
>> _Observation:_
>>
>> It seems possible to move udelay() and get_timer() and static helper
>> functions into a common, ARCH and SoC independent file, provided that
>> the ARCH/SoC/board dependant implementations of get_ticks() runs
>> at >= 1 MHz and is true 64 bits. IF above d) is met get_ticks might be
>> replaced by a u32 function to be used by the common udelay() and
>> get_timer() implementation. That would allow all timer code to use 32
>> bit arithmetic only.
> I see no down-side in requiring d) to be true. It is clearly no problem for
> udelay, as the caller gives up control anyway. I can't imagine a wait for
> event loop that takes a second to execute.
> It may be that "common" C code is not really a good idea, in that different
> CPUs may want to convert timer ticks to millisec/microsec in different ways,
> based on clock rates and available CPU capabilities (like having or not
> having a 32 bit divide, needing to deal with clock rates with repeating
> fractional parts etc.). If ticks per microsecond is an integer and a 32 bit
> divide is available, the code is trivial. If we must avoid the divide and/or
> the ticks per microsecond is not an integer, things get somewhat messier but
> not impossible. If we don't care about the divide taking a bit of time,
> generic code is pretty easy to write even allowing for fractional ticks per
> nicrosecond. Usually, the high-rate timer can be read without requiring a
> call to another routine. This might be a good idea in that people will not
> then "grow" another timer structure on top of the read timer routine!
> It is for sure true that if we are worried about efficiency, there are at
> most three or four versions of the ticks to microseconds or milliseconds code
> required for all the different cases, so it is not so bad to write them out
> and let the users decide which one fits their cpu.
For efficiency it would help if CONFIG_SYS_HZ would be allowed to be in the
range
of 1000 - 2000 Hz (or 700 - 1400), so on any hardware a simple shift would do
for
get_timer(). Of course the constant CONFIG_SYS_HZ should be replaced by a
function
like get_timer_hz() to allow for runtime calculation of the best fit.
Best Regards,
Reinhard
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
