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

[Wolfgang Denk]

Dear Graeme,

I'm trying to summarize your last 3 postings here.

In message <4E1B7E0C.8000900 at gmail.com> you wrote:
> 
> > First, I would very much like to get rid of this "_ms" thing.  We
> > should rather make very clear in the documentation which unit the time
> > services are based on, and use this consequently.  Only functions
> > using a different unit should make this clean in their names.
> 
> Ideally, I think this should be microseconds (a lot happens in a
> microsecond on modern platforms, and a millisecond can be an eternity)

Then we might as well follow the example of the Linux generic TOD
framework and use nanoseconds instead - we will need u64 data types 
anyway when going for microseconds, so we can just follow their
example.

> > Third: all this time_*_max(), ..._min() and ...raw() stuff.  Aren't we
> > over-engineering here?  We have been successfully writing U-Boot code
> > for 11 years now, and never needed these before.  Neither does Linux
> 
> Part of the problem came from the realisation that some architectures have
> really bad timer resolution (Nios2 in particular). In an ideal world, I
> think a uniform microsecond timer (millisecond is to coarse to cater for
> all needs such as udelay and profiling) would be sufficient to not need
> these functions but while ever we have platforms with 'poor' timers, I
> think you may find these functions have a place.

No, I disagree here.  We should not clutter up generic code with
things that are only needed for a single (unusually restricted)
architecture.  Instead, we should try to come up with a design that
hides such details in architecture specific code.


> As stated - This started from a suggestion made by yourself (albeit one you
> did not think was apparently necessary). The idea of the separate functions
> (rather than a parameter to a single function) was that if the functions
> were not used, they would be optimised out.

You know how things go: if you offer the users a set of 20 functions
so they can chose the one or two that fit their purposes best, they
will end up with using all 20 of them, especially when diffferent
parts of the code get maintained by different people with different
preferences.  Approaches like that always get out of hand quickly.

We don't have any such stuff now, and U-Boot is working fine.  Let's
keep it with that, or make it even more simple.  But not the other
way.


> What would you like me to do with the clean-up patches that you have
> already ack'd which do not relate directly to the new API? Should I mark
> the current series as Rejected in patchwork and create a brand new smaller
> series which only has those specific patches?

I suggest you create a new patch series from the independent clean-up
patches and submit this as normal patches (i. e. without the WIP / RFC
part).  This would already be a great improvement, I think.  Thanks a
lot for the efforts!




In message <4E1B88EE.9040104 at gmail.com> you wrote:

> - Looking at the low-level framework described in ols2006-hrtimers.pdf
> (Linux API), we are looking at implementing the same thing - An
> architecture specific free running, high speed, high resolution, high
> accuracy hardware counter/timer and a low speed interrupt which translates
> the hardware counter/timer to a common API. In this respect, we are not
> re-inventing that wheel at all

Indeed.  We should also avoid re-inventing the algorithms, and
eventually re-implementing the code.  In any case, I think it would be
great to use a compatible API so we don;t have to change too many
things when adapting code from Linux etc.

> - The rest of the Linux API is like hitting a thumb-tack with a
> sledgehammer - Timer Queues, NTP, Callbacks, Scheduling etc, etc, etc. We
> only want to do two things:

Right.  Linux is a full-blown OS with a lot of needs we don't have in
U-Boot.

>   1. Monitor timeouts
>   2. Calculate code execution time (and even then, not everyone all the
>      time)

Actrually we need just timestamps.

All the rest (like delays, timeouts, executin times etc.) can be
derived from that.

> - The Linux API is nanosecond resolution. We are hard pressed to get the
> average hardware to support microsecond resolution

On the other hand, it makes little difference to the code wether we
use microseconds or nanoseconds.  It's just slightly different values
in the u64 variables.

> - The Linux API includes usage of 'clock events' - These are timer
> interrupts driven by programmable hardware and relieve the stress on the
> timer scheduler keeping track of every single timer - Overkill for a boot
> loader

Agreed. Well, partially.  We should still follow the example of
keeping the generic code clean of architecture / timer specific code.
This should be implemented in the respective arechitecture specific
code.

> - The Linux API includes 'Time of Day' (Wall Time) - Again, I don't think
> we really need this in a boot loader (we have an RTC API we can use if we
> want to get the current Date and Time).

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, all we need is a fixed resolution free running timer we can use to do
> simple (one at a time) timing operations. No callbacks, no scheduling, to
> 'Wall Time', no 'Clock Events', no NTP etc. The Linux API is 'Too Big'

I never proposed to adapt all of it.

My suggestion was to pick the parts that fit, and use them.
especially, use the same API (like going for nanoseconds as unit of
time), and use their algorithms (and ideally also their code) to solve
the problems we have to deal with.

> I don't think we are re-inventing the wheel with our underlying concept -

Not with the concept, but with the code.



In message <4E1BAED7.3070009 at gmail.com> you wrote:
>
> I think I can safely say that:
> 
> a) We do not want to inherit the code from Linux. It's good code clean
> code, but it is just way too much for what we need. It handles registering
> and unregistering clock sources, change clock source 'rating' (what ever
> that is) and relies heavily on quite heavy (for U-Boot) struct's (which may
> need to be carted around in Global Data)

See above.  You are right, simply copying code unreflected makes no
sense.  But we should use it as example for API, algorithms, and code.

> b) Barebox doesn't really inherit that much from Linux anyway

Right, they did what I proposed, and copied only a part of it, i. e.
the generic TOD framework.

Even if we assume that they made a good choice for this selection for
the purposes of barebox, this still does not mean that the same
selection should be made for U-Boot.

I'm the last to complein if we come up with a leaner implementation
that still solves the problems we're trying to solve.

> I think we have the basics right (and that is the same as Linux). We just
> need to settle on a few finer points such as:
> 
>  - Raw time interval. Linux uses nanoseconds. A 64-bit nanosecond clock
>    source goes for >580 years so even if the highest resolution clock
>    available to us right now is microsecond, it will not hurt to go with a
>    nanosecond time-base as that will provide us with the greatest
>    flexibility going forward. However, this will lead to a lot of integer
>    divides and/or multiplies to scale to millisecond and microsecond
>    intervals

You argumented above against milliseconds (which still works
reasonably well with 32 bit data types) and suggested to use
microseconds.  With a 32 bit data type and using microseconds as unit,
you can get some 4,300 seconds for unsigned and some 2,100 seconds for
signed data types - in other words, in the order of a little over one
hour resp. half an hour.  This is too tight - so we would have to use
64 bit data types here, too.

>  - Function naming

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.

>  - Performing time comparisons for timeouts - Barebox for example has a
>    is_timeout function which takes a start time and a delay (in
>    nanoseconds) an implements ndealy, udealy, and mdelay using is_timeout

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()
	
The classic timeout code then becomes:

	u64 then = time() + TIMEOUT;
	...
	if (time_after(time(), then)) {
		/* handle timeout */
	}

> I think what has been proposed here recently and documented (slightly
> out-of-date) on the wiki is the way to go. Taking the Linux or Barebox
> source will be more effort than it's worth for the complexity it brings in.

I don't want to take all of it.  But there is alarge amount of good
things, and we should pick up bits and pieces we can use to save us
efforts and pain.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
News is what a chap who doesn't care much  about  anything  wants  to
read. And it's only news until he's read it. After that it's dead.
                           - Evelyn Waugh _Scoop_ (1938) bk. 1, ch. 5