[U-Boot] [PATCH 1/2] common/board_f: Add back gd init

[Scott Wood]

On Wed, 2014-04-30 at 11:14 -0700, York Sun wrote:
> On 04/30/2014 10:57 AM, Scott Wood wrote:
> > On Wed, 2014-04-30 at 10:33 -0700, York Sun wrote:
> >> On 04/28/2014 03:51 PM, York Sun wrote:
> >>> For powerpc SoCs, the initial gd is in INIT_RAM, in most cases, resideing
> >>> in locked D-cache. At the time the function baord_inti_f() runs, no other
> >>> RAM is available as a stack. This technique has been used in
> >>> arch/powerpc/lib/board.c and should be added to generic board for powerpc.
> >>>
> >>> Signed-off-by: York Sun <yorksun at freescale.com>
> >>> ---
> >>>  common/board_f.c |    5 ++++-
> >>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/common/board_f.c b/common/board_f.c
> >>> index cbdf06f..3a00b92 100644
> >>> --- a/common/board_f.c
> >>> +++ b/common/board_f.c
> >>> @@ -970,7 +970,10 @@ static init_fnc_t init_sequence_f[] = {
> >>>  
> >>>  void board_init_f(ulong boot_flags)
> >>>  {
> >>> -#ifndef CONFIG_X86
> >>> +#ifdef CONFIG_PPC
> >>> +	gd = (gd_t *)(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
> >>> +	__asm__ __volatile__("" : : : "memory");
> >>> +#elif !defined(CONFIG_X86)
> >>>  	gd_t data;
> >>>  
> >>>  	gd = &data;
> >>>
> >>
> >> Scott,
> >>
> >> Please review this patch.
> > 
> > Could you respond to the comments in the RFC patch?  No point
> > duplicating them.
> > 
> >>  You mentioned in my RFC patch review that "gd is
> >> already initialized at the beginning of board_init_f()". I think that's not the
> >> case. This change is still needed to get gd correct value.
> > 
> > Could you elaborate?  You're setting it to the same value that
> > cpu_init_early_f() set it to (on mpc85xx -- not all PPC).
> > 
> 
> Before this change, we have
> 
> #ifndef CONFIG_X86
> gd_t data;
> 
> gd = &data;
> #endif
> 
> This is overriding the gd.

Yes, as I said, "If PPC needs gd before board_init_f(), then add PPC (or
some other relevant symbol if it's not all PPC) to the #ifndef X86."

> For PPC, gd is set in different places. Eg,  cpu_init_early_f() for mpc85xx,
> cpu_init_f() for for mpc512x, mpc5xxx, mpc8260, mpc83xx, mpc86xx. They are all
> in different files. Since we have been using this assignment in
> arch/powerpc/lib/board.c for all PPC, it should be safe and clear to have
> correct assignment here.

The generic board is an opportunity to clean up cruft.  Non-mpc85xx can
be dealt with when they get converted.

What 85xx currently does seems bad -- it initializes the gd, clears it,
may or may not use it, then clears it again.  Figure out if the 85xx
code is using gd before board_init_f().  If it is, skip the clear in
board_init_f(), or at least clearly document what the pre-board_init_f()
usage is and that none of those values will last (but if that's the
case, why not use the stack for such temporary values?).  If it's not
used, then get rid of the early gd setting and use gd on the stack like
the other arches do.

-Scott