API Rewrite
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest0 of 16 patches 0 repliesNew
Patches in v1 No applies recorded yet
None of the 16 patches the cover letter announces are in this archive.
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [U-Boot] [PATCH v1 (WIP) 00/16] [Timer]API Rewrite
Hi Wolfgang,
Quoted 7 lines · Wolfgang Denk
Dear Graeme Russ,
Quoted 3 lines · In message <[email protected]> you wrote:
The following series is a work-in-progress revamp of the timer API. The aim is to create a new userland API consisting of the following functions (along with a few arch level support functions):
Quoted 5 lines
I have to apologize for not commenting on all of this for such a long time. There are a number of reeasons for this silence - lack of available time being one of them (and probably the most pressing one), but I also needed some time to lean back and think through all of this.
I understand - I also would rather spend more time on this and get it right now rather than going back and fixing it again
Quoted 10 lines
One thing I always wanted to do in the previous discussion was to check what other projects (like Linux, barebox, etc.) are doing in this area. I think it is worth reading the Linux
Documentation/timers/highres.txtdocument, especially the sections about John Stultz's Generic Time Of Day (GTOD) framework, please the documents linked there (i. e. the OLS slides [the link inDocumentation/timers/highres.txtis stale; use
http://www.kernel.org/pub/linux/kernel/people/tglx/hrtimers/ols2006-hrtimers.pdf
instead] and the paper "We Are Not Getting Any Younger: A New Approach to Time and Timers" by J. Stultz et al. in
http://www.linuxsymposium.org/2005/linuxsymposium_procv1.pdf p. 219ff).
Will do
Quoted 14 lines
Having this still in mind, I took a look across the fence to what barebox is doing. Guess what? From "
common/clock.c":*
clock.c- generic clocksource implementation
*
* This file contains the clocksource implementation from the Linux
* kernel originally by John StultzOK. Message received.
What I'm asking myself (and now you) is: Should we really re-invent the wheel again?
Quoted 8 lines
Regarding the function names:
Quoted 3 lines
u32
time_now_ms(void);
u32time_since_ms(u32 from, u32 to);
u32time_max_since_ms(u32 from, u32 to);I don't like these. They appear wrong to me, and they are not in sync with the wiki page either.
I know - there was some discussion about these names and they were in a little bit of flux (hence WIP). I was looking to get some community clarity on this first before updating the wiki. In hindsight, I'm glad I didn't update the wiki as it looks like there may be a better approach anyway
Quoted 4 lines
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)
Quoted 5 lines
Second, I don't feel well with "
time_now()" - what is this, what does it do? Does it set or get a time? The "get_time()" suggestion we discussed earlier feels still much better to me (as it clearly says which operation the function performs). Or we could even follow the example of the Unix system call time(2) and just use "time()".
Well time_ was the namespace for the new API and now is, well, now ;)
Quoted 5 lines
The signature of
time_since(from, to)is broken. For "time since" it seems logically to expect a single argument only:time_since(when). The wiki page still lists this function astime_delta()which seems way more logical to me [although I have to admit that I don;t understand what the "delta_type" argument might be.
It was a refinement of your suggestion that sometimes we want to know that at least a specific period of time has passed (timeouts) and sometimes we want to know what the maximum amount of time that has passed is (profiling)
Quoted 3 lines
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.
Quoted 5 lines
need any of those, nor every other project I'm aware of. Come on, let's keep the code small and efficient and do without these bells and whistles. Quote Antoine de Saint-Exupery: "Perfection is reached, not when there is no longer anything to add, but when there is no longer anything to take away."
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.
Quoted 6 lines
Quoted 3 lines
This current patch series migrates the users of the existing timer API consisting of
get_timer()andreset_timer()to the new API while still retaining the arch specific framework in the background.I feel bad that you already have spent so much work, and only now I come and call everything into question again. Sorry.
That's OK - Thanks for the feedback.
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?
Regards,
Graeme