Dear Ladis, Ladislav Michl wrote: > 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 <la...@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.
Ok. Is this 150MHz defined in one of the configs you modify with this patch or do you use a custom config? Just curious ;) > Perhaps I should modify those > boards wich uses 12MHz clock to use smaller divisor, Yes, this should be easily doable by changing CONFIG_SYS_PTV. > but let's wait for more > comments first. I hope there will be some ;) >>> -/* 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. Ok, understood. Thanks for the hint! >> (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; >>> + else >>> + tmo -= 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. I hope I got it right with the updated patch sent some minutes ago. Best regards Dirk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot