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

[Guennadi Liakhovetski]

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 at 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 at 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!

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de