[U-Boot] [PATCH] Revert "common/memsize.c: Simplify RAM size detection"
This commit breaks bootup on sunxi boards, the get stuck when running the main u-boot binary at:
CPU: Allwinner H3 (SUN8I) I2C: ready DRAM:
This reverts commit 8e7cba048baae68ee0916a8f52b4304277328d5e.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/memsize.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/common/memsize.c b/common/memsize.c index 5c0d279..0fb9ba5 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -33,28 +33,38 @@ long get_ram_size(long *base, long maxsize) long size; int i = 0;
- for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) { + for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) { addr = base + cnt; /* pointer arith! */ sync(); - save[i] = *addr; + save[i++] = *addr; sync(); - if (cnt) { - i++; - *addr = ~cnt; - } else { - *addr = 0; - } + *addr = ~cnt; }
+ addr = base; + sync(); + save[i] = *addr; + sync(); + *addr = 0; + sync(); - cnt = 0; - do { + if ((val = *addr) != 0) { + /* Restore the original data before leaving the function. */ + sync(); + *addr = save[i]; + for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) { + addr = base + cnt; + sync(); + *addr = save[--i]; + } + return (0); + } + + for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt; /* pointer arith! */ val = *addr; - *addr = save[i--]; - sync(); - if (((cnt == 0) && (val != 0)) || - ((cnt != 0) && (val != ~cnt))) { + *addr = save[--i]; + if (val != ~cnt) { size = cnt * sizeof(long); /* * Restore the original data @@ -64,16 +74,11 @@ long get_ram_size(long *base, long maxsize) cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt; - *addr = save[i--]; + *addr = save[--i]; } return (size); } - - if (cnt) - cnt = cnt << 1; - else - cnt = 1; - } while (cnt < maxsize / sizeof(long)); + }
return (maxsize); }
Hi Hans,
I sent a fixup patch yesterday (I managed somehow to send an outdated patch), did you try the fixup patch?
http://lists.denx.de/pipermail/u-boot/2016-February/245080.html Pe 9 feb. 2016 11:38 p.m., "Hans de Goede" hdegoede@redhat.com a scris:
This commit breaks bootup on sunxi boards, the get stuck when running the main u-boot binary at:
CPU: Allwinner H3 (SUN8I) I2C: ready DRAM:
This reverts commit 8e7cba048baae68ee0916a8f52b4304277328d5e.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/memsize.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/common/memsize.c b/common/memsize.c index 5c0d279..0fb9ba5 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -33,28 +33,38 @@ long get_ram_size(long *base, long maxsize) long size; int i = 0;
for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) { addr = base + cnt; /* pointer arith! */ sync();
save[i] = *addr;
save[i++] = *addr; sync();
if (cnt) {i++;*addr = ~cnt;} else {*addr = 0;}
*addr = ~cnt; }addr = base;sync();save[i] = *addr;sync();*addr = 0;sync();
cnt = 0;do {
if ((val = *addr) != 0) {/* Restore the original data before leaving the function.*/
sync();*addr = save[i];for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {addr = base + cnt;sync();*addr = save[--i];}return (0);}for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt; /* pointer arith! */ val = *addr;
*addr = save[i--];sync();if (((cnt == 0) && (val != 0)) ||((cnt != 0) && (val != ~cnt))) {
*addr = save[--i];if (val != ~cnt) { size = cnt * sizeof(long); /* * Restore the original data@@ -64,16 +74,11 @@ long get_ram_size(long *base, long maxsize) cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt;
*addr = save[i--];
*addr = save[--i]; } return (size); }
if (cnt)cnt = cnt << 1;elsecnt = 1;} while (cnt < maxsize / sizeof(long));
} return (maxsize);}
2.5.0
Hi,
On 10-02-16 07:11, Eddy Petrișor wrote:
Hi Hans,
I sent a fixup patch yesterday (I managed somehow to send an outdated patch),
Ah I did not notice that one.
did you try the fixup patch?
I just did try it, same results as Hannes the hang is gone, but now the RAM size is misdedetected (it detects 2 GiB on a 1 GiB board).
It seems this code was as it was for a reason and that a revert of your simplification is probably the best solution.
Regards,
Hans
http://lists.denx.de/pipermail/u-boot/2016-February/245080.html Pe 9 feb. 2016 11:38 p.m., "Hans de Goede" hdegoede@redhat.com a scris:
This commit breaks bootup on sunxi boards, the get stuck when running the main u-boot binary at:
CPU: Allwinner H3 (SUN8I) I2C: ready DRAM:
This reverts commit 8e7cba048baae68ee0916a8f52b4304277328d5e.
Signed-off-by: Hans de Goede hdegoede@redhat.com
common/memsize.c | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/common/memsize.c b/common/memsize.c index 5c0d279..0fb9ba5 100644 --- a/common/memsize.c +++ b/common/memsize.c @@ -33,28 +33,38 @@ long get_ram_size(long *base, long maxsize) long size; int i = 0;
for (cnt = (maxsize / sizeof(long)) >> 1; cnt >= 0; cnt >>= 1) {
for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) { addr = base + cnt; /* pointer arith! */ sync();
save[i] = *addr;
save[i++] = *addr; sync();
if (cnt) {i++;*addr = ~cnt;} else {*addr = 0;}
*addr = ~cnt; }addr = base;sync();save[i] = *addr;sync();*addr = 0;sync();
cnt = 0;do {
if ((val = *addr) != 0) {/* Restore the original data before leaving the function.*/
sync();*addr = save[i];for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {addr = base + cnt;sync();*addr = save[--i];}return (0);}for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt; /* pointer arith! */ val = *addr;
*addr = save[i--];sync();if (((cnt == 0) && (val != 0)) ||((cnt != 0) && (val != ~cnt))) {
*addr = save[--i];if (val != ~cnt) { size = cnt * sizeof(long); /* * Restore the original data@@ -64,16 +74,11 @@ long get_ram_size(long *base, long maxsize) cnt < maxsize / sizeof(long); cnt <<= 1) { addr = base + cnt;
*addr = save[i--];
*addr = save[--i]; } return (size); }
if (cnt)cnt = cnt << 1;elsecnt = 1;} while (cnt < maxsize / sizeof(long));
} return (maxsize);}
-- 2.5.0
2016-02-10 10:20 GMT+02:00 Hans de Goede hdegoede@redhat.com:
Hi,
On 10-02-16 07:11, Eddy Petrișor wrote:
Hi Hans,
I sent a fixup patch yesterday (I managed somehow to send an outdated patch),
Ah I did not notice that one.
did you try the fixup patch?
I just did try it, same results as Hannes the hang is gone, but now the RAM size is misdedetected (it detects 2 GiB on a 1 GiB board).
It seems this code was as it was for a reason and that a revert of your simplification is probably the best solution.
OK, agreed, that makes sense. My buggy HW (DDR unstable) doesn't help.
I will probably rework the patch and target the v2016.05 release.
Dear Hans,
In message 56BAF2B1.3040507@redhat.com you wrote:
It seems this code was as it was for a reason and that a revert of your simplification is probably the best solution.
Indeed. Over the past 15 years I have seen several attempts to improve or simplify the mesize code. Most did not make it into mainline as they worked for the poster, but caused problems on some other boards.
We had a similar "simplification" in 2012, see commit b8496cce and revert in 3ab270d5, but all these attempts caused problems of some kind, so the basic structure of the code is still exactly the same as in the original version of 2004 (commit c83bf6a2).
Probably this is just Perfect Code (TM) which cannot improved further :-)
Best regards,
Wolfgang Denk
On Tue, Feb 09, 2016 at 10:38:31PM +0100, Hans de Goede wrote:
This commit breaks bootup on sunxi boards, the get stuck when running the main u-boot binary at:
CPU: Allwinner H3 (SUN8I) I2C: ready DRAM:
This reverts commit 8e7cba048baae68ee0916a8f52b4304277328d5e.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Applied to u-boot/master, thanks!
participants (4)
-
Eddy Petrișor -
Hans de Goede -
Tom Rini -
Wolfgang Denk