From mboxrd@z Thu Jan 1 00:00:00 1970 From: Graeme Russ Date: Sun, 22 May 2011 17:44:28 +1000 Subject: [U-Boot] [RFC] Review of U-Boot timer API In-Reply-To: <4DD8B993.3010704@comcast.net> References: <4DD7B245.5000008@gmail.com> <4DD8908E.7040501@emk-elektronik.de> <4DD8ABD3.2070506@gmail.com> <4DD8B993.3010704@comcast.net> Message-ID: <4DD8BEDC.2080306@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 22/05/11 17:21, J. William Campbell wrote: > On 5/21/2011 11:23 PM, Graeme Russ wrote: >> On 22/05/11 14:26, Reinhard Meyer wrote: >>> - shall not be used for delays in the seconds range and longer >>> (or any other limit we see practical) >> Any udelay up to the full range of a ulong should be handled without error >> by udelay() - If the arch dependant implementation of udelay() uses >> get_timer() to implement long delays due to hardware limitations then that >> should be fine. > I wonder about this requirement. I think it might be better to say that > udelay can only be used up to a delay of a short unsigned (65 ms), If a We would have to change the prototype for udelay() > longer delay is required, a get_timer loop should be used. Since udelay > uses a different resolution, it implies a more precise delay is required. Agree > If you are delaying more than 65 ms, I suspect precision is not what you > are after, and udelay should return an error (not be void). This makes > implementing udelay a lot easier, in that one can always convert the > desired time interval into an internal clock resolution without difficulty. Who is ever going to check if udelay() returned an error? It is a primitive that one assumes 'just works' and should work for all possible parameter values >>> Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer >>> than >>> "n * m" microseconds and therefore is the wrong approach to implement a >>> timeout. >>> get_timer() must be used for any such timeouts instead! >> ACK - as mentioned, udelay() can use get_timer() of the delay is>> 1ms if >> such an implementation provides better accuracy. If the HAL implementation >> of get_raw_ms() uses a hardware level microsecond time base, there will be >> no accuracy advantage anyway. > What if it provides worse accuracy? It is better if udelay can use the > hardware timer resolution directly. This is easy if the delay is a short > unsigned, so the desired delay can be internally expressed as a long > unsigned in hardware timer ticks. Otherwise, it gets harder, and udelay is > not oriented towards longer delays. It CAN be done, but why should we? What if the udelay parameter is calculated? You would then need an explicit check for >65ms - Seems a bit onerous if you're expecting to occasionally (if ever) to exceed this, but it not being out of the realm of possibility >>> 2. u32 get_timer(u32 base) with the following properties: >>> - must return the elapsed milliseconds since base >> ACK >> >>> - must work without wrapping problems for at least several hours >> Provided that the architecture implementation of get_raw_ms() is >> implemented as described, the only limitation will be the maximum >> measurable delay. The timer API should work correctly no matter how long >> the device has been running >> >> I think the HAL should implement: >> - get_raw_ms() - 32-bit millisecond counter > Only the get_raw_ms should be mandatory. I propose adding udelay to the > HAL, as it is best mechanized at the hardware timer tick level. > >> - get_raw_us() - 32-bit microsecond counter > Used for performance monitor. Not mandatory. True, but useful >> - get_raw_us64() - 64-bit microsecond counter > I can see no possible use for this in u-boot. What operation that we care > about could take this long and require microsecond resolution? 71 seconds *cough* minutes *cough* ;) > is certainly long enough for anything this precise. This certainly should > NOT be mandatory. 64 bits is not for free on many processors. True - I agree there is no need for it Regards, Graeme