[U-Boot] [PATCH 3/7] Add support for SRAM Boot

[Wolfgang Denk]

Dear Haiying Wang,

In message <1282024011.2814.61.camel at localhost.localdomain> you wrote:
>
> > >  Makefile                                           |   18 ++-
> > >  arch/powerpc/cpu/mpc85xx/cpu_init_nand.c           |   31 +++-
> > >  arch/powerpc/cpu/mpc85xx/sram_boot/Makefile        |  190 ++++++++++++++++++++
> > >  arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c     |   76 ++++++++
> > >  .../cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds     |  101 +++++++++++
> > 
> > The code for this should not live in some specific 85xx directory, but
> > instead be generalized similar to what we have with nand_spl.
> Should we let it structured as $(TOPDIR)/sram_boot/board/freescale....?
> At least current, the above code is mostly only used for 85xx. The only
> common part I can tell is the changes in Makefile. 

Not sram_boot, please. The fact that you are using SRAM is specific to
your situation only, but not to the problem.

Assume you want to boot from NAND, and for reliability purposes the
U-Boot image is stored in a UBI partition. The initial NAND bootloader
(1st stage) does not allow to add UBI support, so we will probably
have to make it just load the whole first (guaranteed to be error
free) block into RAM, which then contains full UBI support (but still
not the complete U-Boot image). This 2nd stage loder will then load
the real U-Boot from an UBI partition.

This is a completely different use case, but the basic operation is
the same as in your case.

Please implement your code such that we can re-use it for such other
use cases as well.

> > > +$(SRAM_BOOT):	$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
> > > +		$(MAKE) -C $(CPUDIR)/sram_boot all
> > > +
> > > +$(U_BOOT_NAND_SRAM): $(NAND_SPL) $(SRAM_BOOT) $(obj)u-boot.bin
> > > +		cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)$(CPUDIR)/u-boot-sram.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin
> > 
> > We really need bette rnames here, too.
> Does SRAM_BOOT/sram_boot sound bad? :)

Yes, it does. This has actually nothing to do with SRAM.

> > You do not want to duplicate all this stuff here.
> > 
> > This makes no sense.  Also, it is unmaintainable.
> For this case, I need to call some functions like getenv, hwconfig,
> printf, strcmp etc. which are needed in ddr spd code, but I don't want

I think this is a wrong approach. Instead, you should free the DDR
code from such calls.

> to link the libs for those file because if so, the 2nd stage uboot will
> be larger. It might also not be a good idea to copy all those functions
> into some new files which are really duplicate. I agree it is
> unmaintainable here. As Scott pointed, we need to find a better way. Any
> suggestion?

Yes: remove the need for these functions.

> > > +const char *board_hwconfig = "foo:bar=baz";
> > > +const char *cpu_hwconfig = "foo:bar=baz";
> > 
> > This does not exactly look like useful values to me.
> The only use is to make board_hwconfig and cpu_hwconfig from sbss to sdata section.
> 
> > Please fix!
> The problem here is: 
> The commit 79e4e6480b359cb28129cecfa2cae0ef9cccf803:
>      "powerpc/8xxx: Enabled hwconfig for memory interleaving"
> makes changes as:
> -       if ((p = getenv("memctl_intlv_ctl")) != NULL) {
> +       if (hwconfig_sub("fsl_ddr", "ctlr_intlv")) {
> 
> Thus the hwconfig is called before ddr initialized, and the system hangs
> at "
>          if (board_hwconfig)
>                 return hwconfig_parse(board_hwconfig, strlen(board_hwconfig),
>                                       opt, ";", ':', arglen);
> " in common/hwconfig.c.
> I did not get it yet, but just copied above code from mpc8641hpcn.c to
> make the system boot up. 

It is probably abad concept to depend on such variables that early in
the code, especially when there is SPD information?

In any way, "foo:bar=baz" does not make any sense.


> Again, if those are not acceptable, do you have any suggestion on how to
> pick the code for the functions I need to use in sram boot?

Change the approach. If you cannot afford the code size for these
funtions, then do not use them.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A Chairman was as necessary to a Board planet  as  the  zero  was  in
mathematics, but being a zero had big disadvantages...
                         - Terry Pratchett, _The Dark Side of the Sun_