[U-Boot] Regarding revert of "Simplify RAM size detection" patch

Hi guys,
I see that "Simplify RAM size detection" patch is reverted now. It was breaking boot on DRA7XX EVM as well. But I think I found the actual bug in that patch. This line (from patch):
- for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) { + for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
is changing loop condition to "cnt >= 0". And once cnt is 0, loop continue iterating, turning into forever loop. To fix this one can add "break" line here:
for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) { addr = base + cnt; /* pointer arith! */ sync(); save[i] = *addr; sync(); if (cnt) { i++; *addr = ~cnt; } else { *addr = 0; + break; }
So if someone is going to rework it and push again -- this note can be helpful.

Pe 10 feb. 2016 8:40 p.m., "Sam Protsenko" semen.protsenko@linaro.org a scris:
Hi guys,
Hi Sam,
I see that "Simplify RAM size detection" patch is reverted now. It was breaking boot on DRA7XX EVM as well. But I think I found the actual bug in that patch. This line (from patch):
for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
is changing loop condition to "cnt >= 0". And once cnt is 0, loop continue iterating, turning into forever loop. To fix this one can add "break" line here:
Yes, I got that part, see here:
http://lists.denx.de/pipermail/u-boot/2016-February/245080.html
The problem is that the bottom part still has an issue because the size is reported incorrectly even with the addendum.
for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) { addr = base + cnt; /* pointer arith! */ sync(); save[i] = *addr; sync(); if (cnt) { i++; *addr = ~cnt; } else { *addr = 0;
break; }
So if someone is going to rework it and push again -- this note can be
helpful.
Thanks, Eddy
participants (2)
-
Eddy Petrișor
-
Sam Protsenko