On Mon, Apr 20, 2009 at 08:27:34PM +0200, Dirk Behme wrote:
Dear Ladis,
ah, and some remarks on the patch itself ;)
Thanks, I'm glad someone still cares about ancient stuff.
Ladislav Michl wrote:
Let CONFIG_SYS_HZ to have value of 1000 effectively fixing all users of get_timer.
Signed-off-by: Ladislav Michl ladis@linux-mips.org
diff --git a/cpu/arm925t/interrupts.c b/cpu/arm925t/interrupts.c index e5c77f7..a22be66 100644 --- a/cpu/arm925t/interrupts.c +++ b/cpu/arm925t/interrupts.c
...
-#define TIMER_LOAD_VAL 0xffffffff +#define TIMER_LOAD_VAL 0xffffffff +#define TIMER_CLOCK (CONFIG_SYS_CLK_FREQ / (2 << CONFIG_SYS_PTV))
Just to get an idea of the math:
CONFIG_SYS_CLK_FREQ is 12000000 (12MHz)? This is divided by 256, so TIMER_CLOCK is 46875Hz? A free running 32-bit count down timer is used starting at 0xffffffff? Underflow (0) is reached after ~91626s == ~25hours with this?
Please correct if something is wrong ;)
Math is perfectly correct, except in my case CONFIG_SYS_CLK_FREQ is 150MHz, so resolution is actually 12.5 times better. Perhaps I should modify those boards wich uses 12MHz clock to use smaller divisor, but let's wait for more comments first.
-/* delay x useconds AND preserve advance timestamp value */ +/* delay usec microseconds preserving timestamp value */
Hmm, 'usec microseconds' sounds somehow confusing?
It depends. 'usec' is obviously variable name and 'microsecond' is a time unit, while 'x' is unreferenced variable and 'usec' is abbreviation. And I prefer former (or deleting that part of comment entirely).
void udelay (unsigned long usec) {
...
- int32_t tmo = usec * (TIMER_CLOCK / 1000) / 1000;
- uint32_t now, last = __raw_readl(CONFIG_SYS_TIMERBASE + READ_TIM);
The first '1000' should be CONFIG_SYS_HZ? I.e.
No. Actually it should read 'usec * (TIMER_CLOCK / (1000 * 1000))', where one '1000' is to get miliseconds and other brings you to microseconds digit place. But integer math needs former writing.
(TIMER_CLOCK / CONFIG_SYS_HZ) / 1000;
?
In my udelay patch, I use
- tmo = usec * (TIMER_CLOCK / CONFIG_SYS_HZ);
- tmo /= 1000;
From some printf debugging, for OMAP3 there was a difference doing it in one or two lines. If I remember correctly due to integer vs floating point math and rounding. What do you think?
I think all that udelay code is pointless once CONFIG_SYS_HZ always _have_ to be 1000 and can be simplyfied.
Running OMAP3 counter with 1.625MHz, max udelay resolution is ~1.62us. If you run with 46875Hz, you have max udelay resolution of ~22us?
See above, it is ~1.7us.
- while (tmo > 0) {
now = __raw_readl(CONFIG_SYS_TIMERBASE + READ_TIM);if (last < now) /* count down timer underflow */tmo -= TIMER_LOAD_VAL - now + last;elsetmo -= last - now;last = now;I will think about this, I always need some time for this clock math ;)
In contrast to OMAP3 your timer here counts down, right? So while OMAP1 has to deal with underflow, OMAP3 will need overflow handling, right?
Right, but the key point here is to unbind udelay from get_timer as now get_timer works with miliseconds resolution.
ladis