From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Tue, 12 Jul 2011 10:49:54 +0200 Subject: [U-Boot] [PATCH v1 (WIP) 00/16] [Timer]API Rewrite In-Reply-To: <4E1B7E0C.8000900@gmail.com>, <4E1B88EE.9040104@gmail.com>, <4E1BAED7.3070009@gmail.com> References: <1309261269-4363-1-git-send-email-graeme.russ@gmail.com> <20110711215637.6BC6A1579E14@gemini.denx.de> <4E1B7E0C.8000900@gmail.com> <4E1B88EE.9040104@gmail.com> <4E1BAED7.3070009@gmail.com> Message-ID: <20110712084954.C928C16F7669@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Graeme, I'm trying to summarize your last 3 postings here. In message <4E1B7E0C.8000900@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@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@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