| Message ID | 1455053911-7530-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Accepted |
| Commit | cc8d698fabf072f49ef4dfb2cfdbe0f57d54af81 |
| Delegated to: | Tom Rini |
| Headers | show |
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; > - else > - cnt = 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; >> - else >> - cnt = 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!
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); }
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(-)