[U-Boot] [PATCH v1 (WIP) 00/16] [Timer]API Rewrite

[Graeme Russ]

Hi Wolfgang,

On 12/07/11 23:10, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4E1C23B8.6020101 at gmail.com> you wrote:
>>
>> So how do we deal with Nios2? It is what caused such a deep investigation
>> into the timer API. We have three choices I can think of off the top of my
>> head:
>>
>>  1. Move the whole timer API up to the architecture level and replicate
>> code everywhere
>>  2. Make the whole timer API weak
>>  3. Fundamentally allow the timer API to handle arbitrary timer resolutions
>>
>> 1. Way ugly. We already have this, and that is why we are here today
>> 2. Well, you know what will happen - Someone will be unhappy with the
>> generic API and rewrite a completely different one (timer_masked anyone!)
>> 3. Why is this so evil?
> 
> The big disadvantage of 3) is that you cannot make any reasonable
> assumptions any more in the code.  If I place a "udelay(10)" in some
> device driver, I am probably aware that we don't have exact times, and
> that the actual delay may be 10 or 12 or eventually even 20 micro-
> seconds.  We should make sure that the delay never takes less than 10 us,
> but we also have to guarantee that it does not take - for example -
> 10 milliseconds.

OK, I will assume you agree with me that #1 and #2 are unacceptable

In the case of Nios2 we cannot use the Timer API for udelay since any
udelay will blow out to 20ms - Very Bad Indeed. Maybe udelay might need to
be defined weak, but I fear this will be the thin end of the wedge...

[snip pondering on NIOS]

Also remember that if we are looking to inherit nanosecond time base from
Linux, it will be extremely unlikely that every board will support a native
1ns clock source. Typical examples would be 33MHz (~30ns) or 66MHz (~15ns).
In any case, there will be a lot of variance between boards, so we will
still need to cater for arbitrary resolutions

>> 1) Get the current time
> 
> Agreed. That's time().
> 
>> 2) Report the minimum time elapsed since an arbitrary epoch
>> 3) Report the maximum time elapsed since an arbitrary epoch
> 
> I don't understand why we would need this.

Profiling - Lets say your hardware counter is ms accurate, if an operation
takes 0.5ms then using #2 would give a 50/50 chance of reporting that the
elapsed time was 0ms. Using method #3, the 50/50 split would be 1ms/2ms.
Now as you low-level hardware clock becomes more accurate, those numbers
move down to the real 0.5ms, but will never report that the operation took 0ms.

I guess going to nanosecond time base, the need for #3 lessens as the
hardware clock gets faster.

Also, given that the architecture will need to provide a 'nanoseconds per
counter increment', #3 can be easily derived if someone really wants it,
but as soon as two people want it, you may as well move it into lib/ as it
is a trivial function which is 100% architecture independent.

>> 4) Delay for an arbitrary period of time
>>
>> 4 is a derivative of 2 - Just loop until at least the required time has
>> elapsed.
> 
> Right.  Both delays and timeouts work like that, the difference being
> that delays are "blocking", i. e. there is no other code running
> inbetween, and you can just sit in a tight spinning loop.
> 
> I have not seen any requirement yet for 3.

See above

>> And you then suggest bringing in no less than 6 functions from Linux
> 
> It's just macros.  And we don't need to use them all.  Actually
> time_after() is all that's needed to satisfy our current usage.

Oh please, macro, inline function, function - They are all API 'functions'
- As you stated before, the bigger the API, the more people will abuse is
by using the wrong functions. You stated that you want to keep the public
profile of the API as small and tight as possible by rejecting the
additional functions previously proposed.

>> Done - I will reject the current series and rebase/repost the patches you
>> have already ack'd and assign them to you in patchwork - I'll leave it up
>> to you to pull them in
> 
> Don't reject them - just mark them as RFC.

OK

>> Provided you have access to an incrementing value which increments at a
>> fixed rate and you know the rate, the rest is architecture independent
> 
> We also have to deal with decrementing counters, but this is just aan
> unimportant detail.  And it appears that we actually can have this,
> even on NIOS.

Trivial in the proposed architecture with a #define in the config
CONFIG_SYS_HW_COUNTER_DECREMENTS (or similar)

>>> We could also say this is all we need. If we have a working high
>>> precision TOD timestamp, we can derive all other things we need from
>>> that.
>>
>> So you want to look at bringing in the Linux TOD API as well? That means we
> 
> No, I don't.
> 
>>> See my previous comments. And the longer I think about it, the more I
>>> think we should just use
>>>
>>> 	u64 time(void)
>>>
>>> as core of this new code.
>>
>> Agreed - As said before (for the time being) the return value of any
>> arbitrary call to time() means nothing. It _may_ mean the number of
>> nanoseconds since power-up, but this is by no means guaranteed.

Actually, I do agree will Bill - time() is probably not the best name -
get_current_ns() or similar may be more meaningful (I still have not had a
chance to look at the Linux code)

> True.  But it is also garanteet to be usable in constructs as the
> aforementioned
> 
> 	u64 then = time() + number_of_nanoseconds;
> 	...
> 	if (time_after(time(), then))
> 		...
> 
>> But what about delays (no-brainer - ndelay, udelay, mdelay) and 'elapsed time'
> 
> Well, like this:
> 
> void udelay(u32 usec)
> {
> 	u64 then = time() + (u64)usec * (u64)1000;
> 
> 	while (!time_after(time(), then))
> 		... do something, like trigger watchdogs etc ...
> }

Yes, that is how I always imagined udelay()

> For 'elapsed time' it should be sufficient to store the start value of
> time() as early as possible in the (architecture specific) code.

I did mean 'elapsed between two code locations' such as profiling the init
functions - That was what API function #3 was all about.

>>> Well, here I think we should have a look at Linux again here. In
>>> include/linux/jiffies.h they provide a nice set of inlines, which in
>>> our case would reuse directly:
>>>
>>> 	time_after()
>>> 	time_after_eq()
>>> 	time_before()
>>> 	time_before_eq()
>>> 	time_in_range()
>>> 	time_in_range_open()
>>> 	
>>
>> When would we use time_in_range and time_in_range_open? I don't think we
> 
> I don;t suggest that we need to use all of these.  As mentioned,
> currently I only see use for time_after() (and/or time_after_eq(), it
> if should make a difference).
> 
>>> The classic timeout code then becomes:
>>>
>>> 	u64 then = time() + TIMEOUT;
>>> 	...
>>> 	if (time_after(time(), then)) {
>>> 		/* handle timeout */
>>> 	}
>>
>> Arrrgh - Sorry, but we have been around and around and around this. There
>> are sooooo many way to do this - The two big contenders have been:
> 
> Yes, and both of them do the compare in local code. This is what we
> should get rid of.

OK, this is a new philosophy for the mix. I think it is a good one for the
record. But since we will always use time_after(time(), then) why don't we
simplify it to:

	u64 start = time();
	...
	if (time_elapsed_since(start, TIMEOUT)) {
		/* handle timeout */
	}

Again, I will look at the Linux code

>> And now we throw a third into the mix.
> 
> Please read the comments for the implementation of the time_after()
> macro.  It makes sense to do it this way.
> 
>> time_since and future_time are designed to take into account the underlying
>> resolution of the hardware timer/counter. The big thing to remember is that
>> we _must_ handle the case where the underlying timer is too coarse
> 
> Do we?  What exactly is the needed resolution of the underlying
> hardware timer?  So far, it appears sufficient to have it ticking with
> 1000 Hz or more.  Are there really systems that cannot provide that?
> The only architecture I remember that seemed prolematic was NIOS - but
> then the NIOS documentation suggests that this might actually be
> solvable.

Yes, but as stated before, there is a FPGA gate count trade-off which may
not always be possible. Plus, you break existing boards

Regards,

Graeme