[U-Boot] [REGRESSION] commit b502611b51... "Change env_get_char from a..." breaks imx31_phycore

[Joakim Tjernlund <Joakim.Tjernlund@transmode.se>]

> -----Original Message-----
> From: Guennadi Liakhovetski [mailto:lg at denx.de]
> Sent: den 5 september 2008 21:26
> To: Joakim Tjernlund
> Cc: U-Boot at lists.denx.de; Jean-Christophe PLAGNIOL-VILLARD; Remy Bohmer
> Subject: RE: [REGRESSION] commit b502611b51... "Change env_get_char from a..." breaks imx31_phycore
> 
> On Fri, 5 Sep 2008, Joakim Tjernlund wrote:
> 
> >
> > > -----Original Message-----
> > > From: Guennadi Liakhovetski [mailto:lg at denx.de]
> > > Sent: den 5 september 2008 20:01
> > > To: U-Boot at lists.denx.de
> > > Cc: Joakim Tjernlund
> > > Subject: [REGRESSION] commit b502611b51... "Change env_get_char from a..." breaks imx31_phycore
> > >
> > > Hi,
> > >
> > > The aforementioned commit
> > >
> > > commit b502611b51f02718c2d1117d4981dabceb5af6de
> > > Author: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > > Date:   Sun Jul 6 12:30:09 2008 +0200
> > >
> > >     Change env_get_char from a global function ptr to a function
> > >
> > >     This avoids an early global data reference.
> > >
> > >     Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > >
> > > found by bisection and causes at least the imx31_phycore board to break.
> > > The boot process becomes slow, printenv is very slow too, booting does not
> > > always come to the bootdelay countdown, tftp wtops working too. Reverting
> > > this commit from the current HEAD fixes the problem.
> >
> > Your board probably don't flip the GD_FLG_RELOC flag after relocation. A few
> > ARM boards had a problem with this too.
> 
> Ok, this sounds good, but a grep over the current tree (as of commit
> 3e3c026ed746a284c6f0ef139b26d859939de7e9) reveals only one ARM board that
> does this: davinci. It is also set globally if you define
> CONFIG_SKIP_RELOCATE_UBOOT, which also is done by a couple of boards. From
> the README:
> 
> - CONFIG_SKIP_LOWLEVEL_INIT
> - CONFIG_SKIP_RELOCATE_UBOOT
> 
> 		[ARM only] If these variables are defined, then
> 		certain low level initializations (like setting up
> 		the memory controller) are omitted and/or U-Boot does
> 		not relocate itself into RAM.
> 		Normally these variables MUST NOT be defined. The
> 		only exception is when U-Boot is loaded (to RAM) by
> 		some other boot loader or by a debugger which
> 		performs these initializations itself.
> 
> So, this doesn't look like the proper way to force setting of
> GD_FLG_RELOC. OTOH, other architectures do it centrally in their
> lib_*/board.c::board_init_[fr](). I certainly do not know all ARM boards
> (maintainer added to CC), so, the question is: shall / can we do the same
> on ARM - set this flag centrally, or is there a reason not to do that? I
> see this email
> 
> http://lists.denx.de/pipermail/u-boot/2008-July/037375.html
> 
> trying to do exactly this, as a reply came this
> 
> http://lists.denx.de/pipermail/u-boot/2008-July/037389.html
> 
> promising a fix for all, and that resulted in this:
> 
> http://lists.denx.de/pipermail/u-boot/attachments/20080722/92a646d6/attachment.txt
> 
> which does indeed fix it for all boards setting
> CONFIG_SKIP_RELOCATE_UBOOT, i.e., booting directly from RAM... Please,
> correct me if I am wrong!

I think Remy and friends can best answer this.