[U-Boot] core ticks/timer code
the Blackfin core tick/timer code has been around since the start of the original port, but i'm not sure it's entirely correct. some common code that uses timers seems to be misbehaving in that the timeout is pretty much immediate. makes me think that we've spent time on making udelay() work, but then just glossed over the rest.
unfortunately, there doesnt seem to be any docs on what exactly these functions do so it's hard for me to verify/change any of it.
my understanding is that: - get_ticks - return some notion of "cpu ticks" - get_tbclk - return number of "cpu ticks" that elapse in one second - timer_init - setup a core timer - get_timer(x) - not really sure what this is supposed to represent, or how "x" is used - reset_timer - reset core timer to 0 - CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as in the Linux world, this is the core timer (scheduler) frequency (how many times to execute per second) -mike
Mike Frysinger wrote:
the Blackfin core tick/timer code has been around since the start of the original port, but i'm not sure it's entirely correct. some common code that uses timers seems to be misbehaving in that the timeout is pretty much immediate. makes me think that we've spent time on making udelay() work, but then just glossed over the rest.
unfortunately, there doesnt seem to be any docs on what exactly these functions do so it's hard for me to verify/change any of it.
Yes, you are right. Some days ago I found that no proper documentation for the ticks/timer related code is there, too.
From looking at the existing code for some ARM boards (OSK, DaVinci, OMAP3) my idea of this code is:
my understanding is that:
- get_ticks - return some notion of "cpu ticks"
Yes. Returns the number of cpu ticks since power up. I.e. with 1000 ticks per second (CONFIG_SYS_HZ) the number of elapsed ms since power up.
- get_tbclk - return number of "cpu ticks" that elapse in one second
Yes. Some implementations just return CONFIG_SYS_HZ which is supposed to be 1000.
- timer_init - setup a core timer
Some implementations do this in interrupt_init(), too.
- get_timer(x) - not really sure what this is supposed to represent, or how
"x" is used
This is similar to get_ticks(), i.e. returns the number of ticks since power up. Parameter x is used as a base which is subtracted from ticks. With x == 0 this function is the same as get_ticks(). If x != 0 the value returned is
number of ticks since power up - x
- reset_timer - reset core timer to 0
Some implementations don't reset the timer (hw device) itself. They just reset the timestamp counter (sw variable) which counts the ticks since power up (i.e. that is returned by get_ticks()).
- CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as in the
Linux world, this is the core timer (scheduler) frequency (how many times to execute per second)
CONFIG_SYS_HZ is supposed to be 1000
Does this make sense?
Best regards
Dirk
so if we were to document things, it should read like this: - CONFIG_SYS_HZ is required to be 1000 - get_ticks() / get_tbclk() should be deprecated -- use get_timer() instead - get_timer(X) returns the number of milliseconds since the last call to reset_timer(), minus X (which is in milliseconds too)
is there any reason to keep ticks/tbclk around ? looking at consumers of these functions, they seem to re-implement the get_timer() logic anyways. -mike
so if we were to document things, it should read like this:
- CONFIG_SYS_HZ is required to be 1000
Can't this just be eliminated? It's stupid to have a configurable option that is neither ;-)
And it doesn't do much for my confidence when things stop working (like my tftp downloads now that the tftp timeout has been, ahem, "fixed") only because a "configurable" option wasn't set to the "correct" value.
If we can't change it with breaking something, then let's stop kidding ourselves and get rid of it.
--Scott
Dear Scott McNutt,
In message 49D2014F.4070802@psyent.com you wrote:
so if we were to document things, it should read like this:
- CONFIG_SYS_HZ is required to be 1000
Can't this just be eliminated? It's stupid to have a configurable option that is neither ;-)
And it doesn't do much for my confidence when things stop working (like my tftp downloads now that the tftp timeout has been, ahem, "fixed") only because a "configurable" option wasn't set to the "correct" value.
If we can't change it with breaking something, then let's stop kidding ourselves and get rid of it.
Agreed.
We still need some hint that the timer works in millisecond resolution, though - no matter what you call it.
Best regards,
Wolfgang Denk
On Tue, Mar 31, 2009 at 05:48:01PM +0200, Wolfgang Denk wrote:
We still need some hint that the timer works in millisecond resolution, though - no matter what you call it.
How about just calling it HZ (or something more verbose like TIMER_HZ, but the former makes it easier to share code with Linux) and declaring it in a non-board header?
-Scott
Dear Scott Wood,
In message 20090331212514.GA19920@ld0162-tx32.am.freescale.net you wrote:
On Tue, Mar 31, 2009 at 05:48:01PM +0200, Wolfgang Denk wrote:
We still need some hint that the timer works in millisecond resolution, though - no matter what you call it.
How about just calling it HZ (or something more verbose like TIMER_HZ, but the former makes it easier to share code with Linux) and declaring it in a non-board header?
It's OK with me. Other opinions?
Best regards,
Wolfgang Denk
Hi,
Dear Scott Wood,
In message 20090331212514.GA19920@ld0162-tx32.am.freescale.net you wrote:
On Tue, Mar 31, 2009 at 05:48:01PM +0200, Wolfgang Denk wrote:
We still need some hint that the timer works in millisecond resolution, though - no matter what you call it.
How about just calling it HZ (or something more verbose like TIMER_HZ, but the former makes it easier to share code with Linux) and declaring it in a non-board header?
It's OK with me. Other opinions?
Obviously only like-minded people around.
Who starts? ;)
Cheers Detlev
In message 20090331212514.GA19920@ld0162-tx32.am.freescale.net you wrote:
On Tue, Mar 31, 2009 at 05:48:01PM +0200, Wolfgang Denk wrote:
We still need some hint that the timer works in millisecond resolution, though - no matter what you call it.
How about just calling it HZ (or something more verbose like TIMER_HZ, but the former makes it easier to share code with Linux) and declaring it in a non-board header?
It's OK with me. Other opinions?
Obviously only like-minded people around.
I'm not particular ... and please don't mistake my sometimes dry sense of humor as complaining ... that's rarely my intent .... but please don't use a name that implies it is configurable. I learned the hard way that it is not. ;-)
If the value truly represents clocks per tick, or milliseconds per tick, and the value is always fixed, something along those lines would certainly be appropriate.
Best Regards, --Scott
Dear Dirk Behme,
In message 49CDCCC1.5050503@googlemail.com you wrote:
- get_ticks - return some notion of "cpu ticks"
Yes. Returns the number of cpu ticks since power up. I.e. with 1000 ticks per second (CONFIG_SYS_HZ) the number of elapsed ms since power up.
Hey. What a chance. We have a wiki. Add that information.
Yes. Some implementations just return CONFIG_SYS_HZ which is supposed to be 1000.
get_ticks() makes no sense to me.
- timer_init - setup a core timer
Some implementations do this in interrupt_init(), too.
Let them do it, if they need it, and if they document why they need it.
- get_timer(x) - not really sure what this is supposed to represent, or how
"x" is used
See ppc code.
- CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as in the
Linux world, this is the core timer (scheduler) frequency (how many times to execute per second)
CONFIG_SYS_HZ is supposed to be 1000
Does this make sense?
Yes, it makes sense that CONFIG_SYS_HZ is supposed to be 1000.
Best regards,
Wolfgang Denk
On 21:30 Fri 27 Mar , Mike Frysinger wrote:
the Blackfin core tick/timer code has been around since the start of the original port, but i'm not sure it's entirely correct. some common code that uses timers seems to be misbehaving in that the timeout is pretty much immediate. makes me think that we've spent time on making udelay() work, but then just glossed over the rest.
unfortunately, there doesnt seem to be any docs on what exactly these functions do so it's hard for me to verify/change any of it.
my understanding is that:
- get_ticks - return some notion of "cpu ticks"
- get_tbclk - return number of "cpu ticks" that elapse in one second
- timer_init - setup a core timer
- get_timer(x) - not really sure what this is supposed to represent, or how
"x" is used
- reset_timer - reset core timer to 0
- CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as in the
Linux world, this is the core timer (scheduler) frequency (how many times to execute per second)
I've take a look on the current timer API and I've a lot of limitation on it as example on bitbanging we can not generate a clock over 200Khz or near as you said the API is dark
I've in mind to partially import the clocksource linux API or create a new U-Boot api devired from it's design
I'll propose a new design with the following Requierement
Generic delay function implementation - ndelay() - udelay() - mdelay()
Generic helper - khz2cycles() - hz2cycles() - cs2ns()
Timer API - timer_init() - setup the timer - timer_reset() - reset the timer (use in case of overflow) - get_ticks() - return the current ticks - get_cycles() - return the ticks frequency in ns
Best Regards, J.
On Tuesday 31 March 2009 04:17:06 Jean-Christophe PLAGNIOL-VILLARD wrote:
On 21:30 Fri 27 Mar , Mike Frysinger wrote:
the Blackfin core tick/timer code has been around since the start of the original port, but i'm not sure it's entirely correct. some common code that uses timers seems to be misbehaving in that the timeout is pretty much immediate. makes me think that we've spent time on making udelay() work, but then just glossed over the rest.
unfortunately, there doesnt seem to be any docs on what exactly these functions do so it's hard for me to verify/change any of it.
my understanding is that:
- get_ticks - return some notion of "cpu ticks"
- get_tbclk - return number of "cpu ticks" that elapse in one second
- timer_init - setup a core timer
- get_timer(x) - not really sure what this is supposed to represent, or
how "x" is used
- reset_timer - reset core timer to 0
- CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as
in the Linux world, this is the core timer (scheduler) frequency (how many times to execute per second)
I've take a look on the current timer API and I've a lot of limitation on it as example on bitbanging we can not generate a clock over 200Khz or near as you said the API is dark
I've in mind to partially import the clocksource linux API or create a new U-Boot api devired from it's design
the clocksource framework in linux sounds like extreme overkill for u-boot. where do you see realistic usage of more than one timer ? u-boot is pretty much a single threaded app that polls.
I'll propose a new design with the following Requierement
Generic delay function implementation
- ndelay()
- udelay()
- mdelay()
Generic helper
- khz2cycles()
- hz2cycles()
- cs2ns()
Timer API
- timer_init() - setup the timer
- timer_reset() - reset the timer (use in case of overflow)
- get_ticks() - return the current ticks
- get_cycles() - return the ticks frequency in ns
do you have real use cases here ? i'd actually propose the opposite: kill off the notion of "ticks", "cycles", and "hz". i dont think ndelay() is really necessary, and mdelay() is a simple macro on top of udelay(). that leaves us with really only the three functions we have today: timer_init(), get_timer(), and reset_timer(). we clarify that the function operates in terms of milliseconds and blam, it's all so simple now. -mike
Dear Mike Frysinger,
In message 200903310513.09082.vapier@gentoo.org you wrote:
...
I've in mind to partially import the clocksource linux API or create a new U-Boot api devired from it's design
the clocksource framework in linux sounds like extreme overkill for u-boot. where do you see realistic usage of more than one timer ? u-boot is pretty much a single threaded app that polls.
Correct. We definitely do not need the full capabilities of the Linux framework. Any new implementation will be checked against the current code, and memory foot print is something we will check carefully.
OTOH, I think Sascha's u-boot-v2 uses code that was derived from the Linux code, but in a pretty lean way. It may be interesting to check there...
I'll propose a new design with the following Requierement
Generic delay function implementation
- ndelay()
- udelay()
- mdelay()
Generic helper
- khz2cycles()
- hz2cycles()
- cs2ns()
Timer API
- timer_init() - setup the timer
- timer_reset() - reset the timer (use in case of overflow)
- get_ticks() - return the current ticks
- get_cycles() - return the ticks frequency in ns
do you have real use cases here ? i'd actually propose the opposite: kill off the notion of "ticks", "cycles", and "hz". i dont think ndelay() is really necessary, and mdelay() is a simple macro on top of udelay(). that leaves us with really only the three functions we have today: timer_init(), get_timer(), and reset_timer(). we clarify that the function operates in terms of milliseconds and blam, it's all so simple now.
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
Best regards,
Wolfgang Denk
On Tuesday 31 March 2009 06:28:23 Wolfgang Denk wrote:
In message Mike Frysinger wrote:
I'll propose a new design with the following Requierement
Generic delay function implementation
- ndelay()
- udelay()
- mdelay()
Generic helper
- khz2cycles()
- hz2cycles()
- cs2ns()
Timer API
- timer_init() - setup the timer
- timer_reset() - reset the timer (use in case of overflow)
- get_ticks() - return the current ticks
- get_cycles() - return the ticks frequency in ns
do you have real use cases here ? i'd actually propose the opposite: kill off the notion of "ticks", "cycles", and "hz". i dont think ndelay() is really necessary, and mdelay() is a simple macro on top of udelay(). that leaves us with really only the three functions we have today: timer_init(), get_timer(), and reset_timer(). we clarify that the function operates in terms of milliseconds and blam, it's all so simple now.
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
hrm, i can see that. but you agree that most use of ticks in common code can be converted to get_timer() ? on Blackfin systems (and it isnt alone going by a grep), the ticks interface simply returns get_timer(0), so clearly we should be able to convert common code to all use get_timer(0). that'd leave the ticks interface as optional ... for the systems that need early time sources, they can implement/use it as they need without bringing down everyone else.
i wouldnt mind starting a patch series for post 2009.05 to clean this up ... -mike
On 07:25 Tue 31 Mar , Mike Frysinger wrote:
On Tuesday 31 March 2009 06:28:23 Wolfgang Denk wrote:
In message Mike Frysinger wrote:
I'll propose a new design with the following Requierement
Generic delay function implementation
- ndelay()
- udelay()
- mdelay()
Generic helper
- khz2cycles()
- hz2cycles()
- cs2ns()
Timer API
- timer_init() - setup the timer
- timer_reset() - reset the timer (use in case of overflow)
- get_ticks() - return the current ticks
- get_cycles() - return the ticks frequency in ns
do you have real use cases here ? i'd actually propose the opposite: kill off the notion of "ticks", "cycles", and "hz". i dont think ndelay() is really necessary, and mdelay() is a simple macro on top of udelay(). that leaves us with really only the three functions we have today: timer_init(), get_timer(), and reset_timer(). we clarify that the function operates in terms of milliseconds and blam, it's all so simple now.
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
hrm, i can see that. but you agree that most use of ticks in common code can be converted to get_timer() ? on Blackfin systems (and it isnt alone going by a grep), the ticks interface simply returns get_timer(0), so clearly we should be able to convert common code to all use get_timer(0). that'd leave the ticks interface as optional ... for the systems that need early time sources, they can implement/use it as they need without bringing down everyone else.
i wouldnt mind starting a patch series for post 2009.05 to clean this up ...
I've in mind too maybe I'll send a first version within few days for a arm soc and a blackfin based on u-boot-v2
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
i wouldnt mind starting a patch series for post 2009.05 to clean this up ...
I've in mind too maybe I'll send a first version within few days for a arm soc and a blackfin based on u-boot-v2
In contrast to Jean-Christophe's plan
http://lists.denx.de/pipermail/u-boot/2009-March/049861.html
(last sentence) I'd like that we split general ticks/timer code clean up and bug fixes. I'd like that we implement the changes proposed by Mike and Jean-Christophe independent and don't mix them with other fixes for broken timer code like
http://lists.denx.de/pipermail/u-boot/2009-March/049146.html
Having proper/fixed (old style) timer code will make it easier to convert to new style afterwards.
From my point of view this is similar to rule "don't mix coding style with functional changes".
Best regards
Dirk
Dirk,
On Tue, Mar 31, 2009 at 05:55:13PM +0200, Dirk Behme wrote:
... I'd like that we split general ticks/timer code clean up and bug fixes. I'd like that we implement the changes proposed by Mike and Jean-Christophe independent and don't mix them with other fixes for broken timer code like
http://lists.denx.de/pipermail/u-boot/2009-March/049146.html
Having proper/fixed (old style) timer code will make it easier to convert to new style afterwards.
From my point of view this is similar to rule "don't mix coding style with functional changes".
Well the very same bug is present in OMAP1 and OMAP2 timer code, so you'll end fixing copied&paste code for no good reason. At least cpu/arm925t/interrupts.c and cpu/arm926ejs/omap/timer.c deserves the same fix [*] you sent for OMAP3 code. So what about merging that timer code first and then fixing it?
ladis
[*] I have local fix for cpu/arm925t/interrupts.c, but since those copy&pasted files started diverging just because each of us cares about different piece of hardware, I do not want to make situation even worse.
Wolfgang,
Can this be put in the ToDo list so anyone who looks to clean up their CPU and/or architecture knows what it is exactly that we are trying to achieve
Thanks,
Graeme
Dear Graeme Russ,
In message d66caabb0904021418o483a709bg6b67b87d8327c308@mail.gmail.com you wrote:
Can this be put in the ToDo list so anyone who looks to clean up their CPU and/or architecture knows what it is exactly that we are trying to achieve
Maybe you can summarize and submit a patch, please?
Best regards,
Wolfgang Denk
Dear Mike Frysinger,
In message 200903310725.20652.vapier@gentoo.org you wrote:
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
hrm, i can see that. but you agree that most use of ticks in common code can be converted to get_timer() ? on Blackfin systems (and it isnt alone going by
Yes, of course I do.
a grep), the ticks interface simply returns get_timer(0), so clearly we should be able to convert common code to all use get_timer(0). that'd leave the ticks interface as optional ... for the systems that need early time sources, they can implement/use it as they need without bringing down everyone else.
i wouldnt mind starting a patch series for post 2009.05 to clean this up ...
Excellent. Thanks in advance.
For BF or for all architectures?
Best regards,
Wolfgang Denk
On 12:28 Tue 31 Mar , Wolfgang Denk wrote:
Dear Mike Frysinger,
In message 200903310513.09082.vapier@gentoo.org you wrote:
...
I've in mind to partially import the clocksource linux API or create a new U-Boot api devired from it's design
the clocksource framework in linux sounds like extreme overkill for u-boot. where do you see realistic usage of more than one timer ? u-boot is pretty much a single threaded app that polls.
That's why I've tell to partially import it as have done Sascha in u-boot V2
Correct. We definitely do not need the full capabilities of the Linux framework. Any new implementation will be checked against the current code, and memory foot print is something we will check carefully.
OTOH, I think Sascha's u-boot-v2 uses code that was derived from the Linux code, but in a pretty lean way. It may be interesting to check there...
I've take a look on it and it's the idea I've in mind to do also
I'll propose a new design with the following Requierement
Generic delay function implementation
- ndelay()
- udelay()
- mdelay()
Generic helper
- khz2cycles()
- hz2cycles()
- cs2ns()
Timer API
- timer_init() - setup the timer
- timer_reset() - reset the timer (use in case of overflow)
- get_ticks() - return the current ticks
- get_cycles() - return the ticks frequency in ns
do you have real use cases here ? i'd actually propose the opposite: kill off the notion of "ticks", "cycles", and "hz". i dont think ndelay() is really necessary, and mdelay() is a simple macro on top of udelay(). that leaves us with really only the three functions we have today: timer_init(), get_timer(), and reset_timer(). we clarify that the function operates in terms of milliseconds and blam, it's all so simple now.
Yes I've real case where I do need ndelay to generate high speed clock with a gpio and udelay is too slow.
we need get_cycles and get_ticks to calculate the udelay, mdelay and ndelay otherwise how can you known the ticks frequency?
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
not necessarely you can activate the timer just after the cpu_init so normally you do not need any get_ticks.
Best Regards, J.
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090331113956.GD28787@game.jcrosoft.org you wrote:
Agreed (except that we probably cannot completely throw away the tick; IIRC there are cases in early startup when nothing else is available yet).
not necessarely you can activate the timer just after the cpu_init so normally you do not need any get_ticks.
Please check for example the PowerPC code, then.
Ther ethe timer implementation relies heavily on interrupts and is not available in early stages, for example in NAND booting systems with tight memory restrictions.
Best regards,
Wolfgang Denk
On Tue, Mar 31, 2009 at 05:49:52PM +0200, Wolfgang Denk wrote:
Please check for example the PowerPC code, then.
Ther ethe timer implementation relies heavily on interrupts and is not available in early stages, for example in NAND booting systems with tight memory restrictions.
Is there any particular reason not to change the powerpc get_timer implementation to use the timebase (scaled down to ms)?
-Scott
Dear Scott Wood,
In message 20090331213202.GB19920@ld0162-tx32.am.freescale.net you wrote:
Is there any particular reason not to change the powerpc get_timer implementation to use the timebase (scaled down to ms)?
I think so. There are some boards where we actually measure the system clock frequency against a known reference clock. IIRC this requires the TB to be independent, or something like that. [It's a long time since that code was written.]
Best regards,
Wolfgang Denk
On Sat, Apr 04, 2009 at 11:14:09PM +0200, Wolfgang Denk wrote:
Dear Scott Wood,
In message 20090331213202.GB19920@ld0162-tx32.am.freescale.net you wrote:
Is there any particular reason not to change the powerpc get_timer implementation to use the timebase (scaled down to ms)?
I think so. There are some boards where we actually measure the system clock frequency against a known reference clock. IIRC this requires the TB to be independent, or something like that. [It's a long time since that code was written.]
The existing get_timer uses the decrementer, which is not independent of the timebase.
-Scott
Dear Scott,
In message 20090406191242.GA4439@ld0162-tx32.am.freescale.net you wrote:
Is there any particular reason not to change the powerpc get_timer implementation to use the timebase (scaled down to ms)?
Side note: we already use the timebase for the implementation of udelay() and friends which are available as soon as we have a (limited) C runtime environment, i. e. long before relocation.
I think so. There are some boards where we actually measure the system clock frequency against a known reference clock. IIRC this requires the TB to be independent, or something like that. [It's a long time since that code was written.]
The existing get_timer uses the decrementer, which is not independent of the timebase.
Yes, you are right. I should have looked up the actual implementation. Sorry, volatile memory with too few/short refresh cycles ;-)
Best regards,
Wolfgang Denk
Dear Mike Frysinger,
In message 200903272130.26825.vapier@gentoo.org you wrote:
unfortunately, there doesnt seem to be any docs on what exactly these functions do so it's hard for me to verify/change any of it.
If in doubt, look it up in the PowerPC implementation. That's the reference for what a half-way sane world could look like.
my understanding is that:
- get_ticks - return some notion of "cpu ticks"
- get_tbclk - return number of "cpu ticks" that elapse in one second
- timer_init - setup a core timer
- get_timer(x) - not really sure what this is supposed to represent, or ho> w
"x" is used
- reset_timer - reset core timer to 0
- CONFIG_SYS_HZ - no idea how this relates to ticks/timer in U-Boot as in the
Linux world, this is the core timer (scheduler) frequency (how many times to execute per second)
It's the number of ticks per second, i. e. 1000.
Best regards,
Wolfgang Denk
participants (9)
-
Detlev Zundel -
Dirk Behme -
Graeme Russ -
Jean-Christophe PLAGNIOL-VILLARD -
Ladislav Michl -
Mike Frysinger -
Scott McNutt -
Scott Wood -
Wolfgang Denk