Matthias Kaehlcke wrote: > Hi Wolfgang, > > El Tue, Feb 23, 2010 at 11:23:42PM +0100 Wolfgang Denk ha dit: > >> In message <20100223220421.gk20...@darwin> you wrote: >>> ep93xx: Refactoring of the timer code, including the following changes >> ... >>> +#define TIMER_FREQ 508469 >>> +#define CLK_TICKS_PER_SYS_TICK (TIMER_FREQ / CONFIG_SYS_HZ) >> ... >>> + ticks *= (CLK_TICKS_PER_SYS_TICK * CONFIG_SYS_HZ); >> ... >>> + ticks = usecs * CLK_TICKS_PER_SYS_TICK * CONFIG_SYS_HZ; >> Why don't you use >> >> ticks *= TIMER_FREQ; >> resp. >> ticks = usecs * TIMER_FREQ; >> >> The combination of " / CONFIG_SYS_HZ * CONFIG_SYS_HZ" is just a bad >> NO-OP (with rounding errors). > > you certainly have a point, i'm going to change this as you proposed > >> Hm... re-reading the optimized code makes me wonder if the variable >> really should be called "ticks" - looks more as a frequency to me? > > here i disagree, the function returns the number of ticks that pass in > a certain number of microseconds, so i think 'ticks' is an appropiate > name > Perhaps a comment on the units of TIMER_FREQ ? This is minor point. Otherwise looks fine. Tom
> thanks for your review! > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
