Dear Reinhard > I think the timer.c is more broken that just that return value: > You are right, but the patch fixes only the single small problem. > example 1: > /* > * timer without interrupts > */ > unsigned long long get_ticks(void) > { > at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE; > > ulong now = readl(&pit->piir); > > if (now >= lastinc) /* normal mode (non roll) */ > /* move stamp forward with absolut diff ticks */ > timestamp += (now - lastinc); > else /* we have rollover of incrementer */ > timestamp += (0xFFFFFFFF - lastinc) + now; > lastinc = now; > return timestamp; > } > > observation: timestamp, lastinc, now are all ulong. > timestamp += (0xFFFFFFFF - lastinc) + now; > is the same as > timestamp += (now - lastinc) - 1; > so in case of a "rollover" timestamp is incremented just by one less. > I think the if is superfluous. ulong will handle the rollover automatically
But this is only true, if we are using ulong variables. I think the idea behind this code is use unsigned long long for variables. (especially timestamp) > > example 2: > void __udelay(unsigned long usec) > { > unsigned long long tmp; > ulong tmo; > > tmo = usec_to_tick(usec); > tmp = get_ticks() + tmo; /* get current timestamp */ > > while (get_ticks() < tmp) /* loop till event */ > /*NOP*/; > } > > observation: tmp being unsigned long long, get_ticks returning > the unsigned long timestamp, tmp being a sum of two ulongs, > the while might NEVER end. In practice that is very unlikely, > however the code should be corrected. Right, but this isn't only a problem of AT91 arch. Is should be fixed global. The code is correct, if get_ticks returns real long long values. > > I was going to rework that timer sooner or later to address all > those issues, but you are welcome to go ahead, too. Just we > should avoid double work :) I have no time enough at the moment to do that. > > Greetings, Reinhard Best regards Jens Scharsig _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot