<html><body>
<p><tt>Hello Andrew,<br>
<br>
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...</tt><br>
<br>
<tt>> Why did you change the while loop and add the rollover code? The old code:<br>
> - while ((ulong)((mips_count_get() - start)) < tmo)<br>
> - /*NOP*/;<br>
> seems perfectly sane to me and should work whether or not you use<br>
> read_32bit_cp0_register(CP0_COUNT) or mips_count_get() assuming tmo<br>
> gets set correctly.</tt><br>
<br>
<tt>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 ;-)</tt><br>
<br>
<tt>> Is this something NXP specific?<br>
</tt><br>
<tt>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.</tt><br>
<tt><br>
> In udelay() I don't like this part that silently truncates delays.<br>
> This could lead to some ugly hard to find timing bugs.<br>
> <br>
> + /* Safeguard for too-long delays */<br>
> + if (delayTicks >= 0x80000000)<br>
> + delayTicks = 0x7FFFFFFF;<br>
</tt><br>
<tt>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.</tt><br>
<tt><br>
> I would calculate the max. count you can handle in the code and run<br>
> the timer polling loop multiple times if need be.<br>
</tt><br>
<tt>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.</tt><br>
<tt><br>
> long lines<br>
</tt><br>
<tt>> might be a good idea to throw in a compile time warning message if<br>
> CFG_HZ != 1000<br>
</tt><br>
<tt>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 ;-)</tt><br>
<br>
<tt>With kind regards,</tt><br>
<br>
<tt> Robert.</tt></body></html>