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

Scott Wood scottwood at freescale.com
Mon Aug 16 23:03:51 CEST 2010


On Mon, Aug 16, 2010 at 12:23:56PM +0200, Wolfgang Denk wrote:
> > Signed-off-by: Haiying Wang <Haiying.Wang at freescale.com>
> > ---
> >  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.

With NAND there's some common code (or at least a few common alternatives,
that aren't tied to a specific cpu or board) to actually do the NAND
loading.  I'm not sure if there would be anything common here -- it's just
cutting size down, plus storage-specific code to load the final image.

> > +SOBJS	= start.o ticks.o ppcstring.o
> > +COBJS	= cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o speed.o \
> > +	  sram_boot.o ns16550.o tlb.o tlb_table.o string.o hwconfig.o ddr.o \
> > +	  time.o time_lib.o ddr-gen3.o ddr_spd.o ctype.o div64.o console.o \
> > +	  cmd_nvedit.o env_common.o env_nand.o vsprintf.o display_options.o
> 
> You do not want to duplicate all this stuff here.
> 
> This makes no sense.  Also, it is unmaintainable.

Here you say not to duplicate things (it's not actually duplicating the
code, just the list of objects, just like nand_spl does), but later you
say not to modify what already exists (which is also already done for
nand_spl in some places).  We need to cut down the image size some way or
another.  :-)

We're open to suggestions on how to better structure this kind of thing for
maintainability; the current approaches certainly aren't pretty.  But we
need to do *something*.

> > +void board_init_f(ulong bootflag)
> > +{
> > +	uint plat_ratio, bus_clk, sys_clk = 0;
> > +	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> ...
> > +void putc(char c)
> > +{
> ...
> > +void puts(const char *str)
> > +{
> 
> Argh... 
> 
> Please do not reimplement everything. This will result in a terribke
> mess of unmaintainable code.

These are tiny alternatives that are used in nand_spl for some boards (a
whopping 13 lines to squeeze console output in a 4K image, not the end of
the world) -- but for the SRAM phase we should have plenty of room for the
normal console implementation.

-Scott



More information about the U-Boot mailing list