[U-Boot-Users] [PATCH] Bug fix for MIPS timer
Hello,
When building MIPS targets the warning 'integer overflow in expression' occurs in nfs.c and net.c because these targets still define their clock speed in CFG_HZ instead of the number of miliseconds per second (1000). This will cause incorrect timer behavior. The patch attached fixes this and makes sure MIPS targets now use a CFG_HZ of 1000.
With kind regards,
Robert.
(See attached file: mips_timer_fix)
Hello,
When building MIPS target Purple the warning 'pointer targets in assignment differ in signedness' occurs a couple of times in board/purple/flash.c. The patch attached fixes this.
With kind regards,
Robert. (See attached file: mips_purple_warning_fix)
On 6/22/07, Robert Delien robert.delien@nxp.com wrote:
When building MIPS targets the warning 'integer overflow in expression' occurs in nfs.c and net.c because these targets still define their clock speed in CFG_HZ instead of the number of miliseconds per second (1000). This will cause incorrect timer behavior. The patch attached fixes this and makes sure MIPS targets now use a CFG_HZ of 1000.
A couple of comments/questions:
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. Is this something NXP specific?
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;
I would calculate the max. count you can handle in the code and run the timer polling loop multiple times if need be.
Other nits:
long lines might be a good idea to throw in a compile time warning message if CFG_HZ != 1000
Hi:
I'm working with ATMEL at91rm9200-ek reference board and I have recently noticed that if characters on serial console are typed quickly, U-boot serial port's driver looses them. It's easy to see if try copy and past some string > ~20 characters long. I took the string:
"command line: root=/dev/nfs rw nfsroot=192.168.0.137:/tftpboot/leonid/"
After copy/past it looks as following:
"command line:root=/dev/nf rw nfsroot=92.168.0.13:/tftpboot/lonid/"
- many characters are missing.
I realize that it can be terminal related issue as DULG manual points out so I have performed some investigations:
A. Speed is 115200 - It happens with minicom and teraterm on U-boot. - It doesn't happen while Linux is already running - so, Linux serial driver is quite reliable (I'm running 2.6.16 now).
B. Speed is 57600 or less The issue disappears (or at least doesn't show up with the same string).
So, we must conclude it must be specific U-boot serial driver problem, showing up on high speeds.
Does anybody know about any problems with U-boot serial driver for this ATMEL chip?
Thanks,
Leonid.
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.
participants (3)
-
Andrew Dyer -
Leonid -
Robert Delien