Hello Andrew,

Thank you for reviewing. It's always good to see somebody taking the time and effort to improve the code. Though by the level of broken-ness it seems nobody is actually using U-Boot on the MIPS platform anymore, except for us. The timer was broken, the whole GTH2 board support is broken and the other boards are full of warnings...

> Why did you change the while loop and add the rollover code?  The old code:
> -   while ((ulong)((mips_count_get() - start)) < tmo)
> -      /*NOP*/;
> seems perfectly sane to me and should work whether or not you use
> read_32bit_cp0_register(CP0_COUNT) or mips_count_get() assuming tmo
> gets set correctly.

I have to dig deep for that; It's almost a year ago since I changed that. It may be an Obi Wan, but whatever the problem was, this implementation is perectly sane too ;-)

> Is this something NXP specific?

No, it's MIPS specific: There's no use for a 'mips_count_get'-function. The proper way to read the count register is to use the already available function 'read_32bit_cp0_register', because the count register is just another MIPS coprocessor 0 register.

> In udelay() I don't like this part that silently truncates delays.
> This could lead to some ugly hard to find timing bugs.
>
> +   /* Safeguard for too-long delays */
> +   if (delayTicks >= 0x80000000)
> +      delayTicks = 0x7FFFFFFF;

Well, it still beats the previous implementation that allowed a wrap-around, resulting in a much shorter delay than desired. And with no error logging in place, truncating is the only thing that could be done.

> I would calculate the max. count you can handle in the code and run
> the timer polling loop multiple times if need be.

We're talking about delays of over 35 seconds being truncated. I think it's hardly worth the extra code to support such long delays in a function called 'udelay'. People using it for delays that long have other problems.

> long lines

> might be a good idea to throw in a compile time warning message if
> CFG_HZ != 1000

No, I'm sorry; I can't put a warning everywhere people might make a mistake ;-). Besides that, this would have to be global warning, covering all targets. If such a check would be desired, a mechanism should be implemented for that, which could also check other commonly made mistakes. But I'd rather suggest people to read the manual ;-)

With kind regards,

Robert.