[U-Boot] [PATCH 6/7 v6] nand: tpl : introduce the TPL based on the SPL

[Scott Wood <scottwood@freescale.com>]

On 06/26/2013 02:18:31 AM, ying.zhang at freescale.com wrote:
> From: Ying Zhang <b40530@freescale.com>
> 
> Due to the nand SPL on some board(e.g. P1022DS)has a size limit, it  
> can
> not be more than 4K. So, the SPL cannot initialize the DDR with the  
> SPD
> code. This patch introduces TPL to enable a loader stub that runs in  
> the
> L2 SRAM, after being loaded by the code from the SPL. It initializes  
> the
> DDR with the SPD or other operations.
> 
> The TPL's size is sizeable, the maximum size must not exceed the size  
> of L2
> SRAM.

Using L2 SRAM for this is an implementation detail, and not appropriate  
for a description of the generic TPL infrastructure.  For example, on  
newer QorIQ chips you'd be using CPC (L3) SRAM.

> It initializes the DDR through SPD code, and copys final uboot image
> to DDR. So there are three stage uboot images:
> 	* spl_boot, * tpl_boot, * final uboot image,
> 
> This patch is on top of the patch:
> SPL : spi flash : support to start from eSPI with SPL

You don't need to mention a dependency on an earlier patch in the same  
patchset (and when you do need to mention a dependency, it should be  
below the --- line so it doesn't go into the git history).  OTOH, you  
do need to mention the dependency on the patch to have a separate  
autoconf.mk.

> Signed-off-by: Ying Zhang <b40530@freescale.com>
> ---
> Compared with the original version, Changed as below:
> 1. Split from "powerpc/p1022ds: nand: introduce the TPL based on the  
> SPL".
> 2. No change.
> 3. No change.
> 4. No change.
> 5. Use ifdef to define "nand_load_image" to non-static for non-SPL
> 
>  Makefile                                           |   21 +++-
>  README                                             |   57 +++++++-
>  arch/powerpc/config.mk                             |    2 +
>  .../cpu/mpc8xxx/ddr/lc_common_dimm_params.c        |    4 +-
>  arch/powerpc/lib/Makefile                          |    2 +
>  common/Makefile                                    |    9 +
>  common/cmd_nvedit.c                                |    8 +-
>  config.mk                                          |   32 ++++
>  doc/README.TPL                                     |   69 +++++++++
>  drivers/mtd/nand/Makefile                          |    7 +
>  drivers/mtd/nand/fsl_elbc_spl.c                    |    5 +-
>  drivers/serial/serial.c                            |    2 +-
>  include/bootstage.h                                |    3 +-
>  tpl/Makefile                                       |  161  
> ++++++++++++++++++++
>  14 files changed, 368 insertions(+), 14 deletions(-)
>  create mode 100644 doc/README.TPL
>  create mode 100644 tpl/Makefile
> 
> diff --git a/Makefile b/Makefile
> index fdaddb9..0513e4d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -118,10 +118,11 @@ endif # ifneq ($(BUILD_DIR),)
> 
>  OBJTREE		:= $(if $(BUILD_DIR),$(BUILD_DIR),$(CURDIR))
>  SPLTREE		:= $(OBJTREE)/spl
> +TPLTREE		:= $(OBJTREE)/tpl
>  SRCTREE		:= $(CURDIR)
>  TOPDIR		:= $(SRCTREE)
>  LNDIR		:= $(OBJTREE)
> -export	TOPDIR SRCTREE OBJTREE SPLTREE
> +export	TOPDIR SRCTREE OBJTREE SPLTREE TPLTREE
> 
>  MKCONFIG	:= $(SRCTREE)/mkconfig
>  export MKCONFIG
> @@ -412,6 +413,7 @@ ALL-y += $(obj)u-boot.srec $(obj)u-boot.bin  
> $(obj)System.map
>  ALL-$(CONFIG_NAND_U_BOOT) += $(obj)u-boot-nand.bin
>  ALL-$(CONFIG_ONENAND_U_BOOT) += $(obj)u-boot-onenand.bin
>  ALL-$(CONFIG_SPL) += $(obj)spl/u-boot-spl.bin
> +ALL-$(CONFIG_TPL) += $(obj)tpl/u-boot-tpl.bin
>  ALL-$(CONFIG_OF_SEPARATE) += $(obj)u-boot.dtb $(obj)u-boot-dtb.bin
>  ifneq ($(CONFIG_SPL_TARGET),)
>  ALL-$(CONFIG_SPL) += $(obj)$(subst ",,$(CONFIG_SPL_TARGET))
> @@ -498,6 +500,18 @@ $(obj)u-boot-with-spl.bin:  
> $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
>  		cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin > $@
>  		rm $(obj)spl/u-boot-spl-pad.bin
> 
> +$(obj)u-boot-with-tpl.bin: $(obj)spl/u-boot-spl.bin  
> $(obj)tpl/u-boot-tpl.bin \
> +		$(obj)u-boot.bin
> +		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SPL_PAD_TO) \
> +			-I binary -O binary \
> +			$(obj)spl/u-boot-spl.bin  
> $(obj)spl/u-boot-spl-pad.bin
> +		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_PAD_TO) \
> +			-I binary -O binary \
> +			$(obj)tpl/u-boot-tpl.bin  
> $(obj)tpl/u-boot-tpl-pad.bin
> +		cat $(obj)spl/u-boot-spl-pad.bin  
> $(obj)tpl/u-boot-tpl-pad.bin \
> +			$(obj)u-boot.bin > $@
> +		rm $(obj)spl/u-boot-spl-pad.bin  
> $(obj)tpl/u-boot-tpl-pad.bin

Can we do this with less duplication?  Ideally u-boot-with-tpl.bin  
would be just TPL and the main U-Boot, and then we'd feed that into the  
u-boot-with-spl.bin rule.  Or even call the same code twice (pattern  
rule, make function, etc).

> @@ -877,6 +894,8 @@ clobber:	tidy
>  	@rm -f  
> $(obj)nand_spl/{u-boot-nand_spl.lds,u-boot-spl,u-boot-spl.map}
>  	@rm -f $(obj)spl/{u-boot-spl,u-boot-spl.bin,u-boot-spl.map}
>  	@rm -f $(obj)spl/u-boot-spl.lds
> +	@rm -f $(obj)tpl/{u-boot-tpl,u-boot-tpl.bin,u-boot-tpl.map}
> +	@rm -f $(obj)tpl/u-boot-tpl.lds
>  	@rm -f $(obj)MLO MLO.byteswap
>  	@rm -f $(obj)SPL
>  	@rm -f $(obj)tools/xway-swap-bytes
> diff --git a/README b/README
> index b941a56..0c20253 100644
> --- a/README
> +++ b/README
> @@ -2999,9 +2999,10 @@ FIT uImage format:
>  		Set for the SPL on PPC mpc8xxx targets, support for
>  		arch/powerpc/cpu/mpc8xxx/ddr/libddr.o in SPL binary.
> 
> -		CONFIG_SPL_COMMON_INIT_DDR
> +		CONFIG_COMMON_INIT_DDR
>  		Set for common ddr init with serial presence detect in
> -		SPL binary.
> +		SPL binary or TPL binary.

Please keep SPL in the name of these.  Consider TPL as a type of SPL.   
Changing the name just causes a lot of churn, and it is confusing  
because you end up with a name that looks like it applies to the main  
U-Boot, when it actually applies just to SPL/TPL.

> +- CONFIG_SYS_MPC8XXX_INIT_DDR_SUPPORT
> +		Set for the SPL on PPC mpc8xxx targets, support for
> +		arch/powerpc/cpu/mpc8xxx/ddr/libddr.o.
> +
> +- CONFIG_SYS_ENV_SUPPORT
> +		Support for the environment operating.
> +
> +- CONFIG_SYS_SERIAL_SUPPORT
> +		Support for drivers/serial/libserial.o.
> +
> +- CONFIG_SYS_LIBGENERIC_SUPPORT
> +		Support for lib/libgeneric.o.
> +
> +- CONFIG_SYS_LIBCOMMON_SUPPORT
> +		Support for common/libcommon.o.
> +
> +- CONFIG_SYS_I2C_SUPPORT
> +		Support for drivers/i2c/libi2c.o.
> +
> +- CONFIG_SYS_NAND_SUPPORT
> +		Support for drivers/mtd/nand/libnand.o.

Likewise, except here you seem to still be duplicating the SPL defines  
rather than renaming them.  Also, CONFIG_SYS_ is generally used to  
describe the hardware or similar, not to indicate which parts of the  
source code to enable.

> diff --git a/tpl/Makefile b/tpl/Makefile
> new file mode 100644
> index 0000000..bb947e5
> --- /dev/null
> +++ b/tpl/Makefile
> @@ -0,0 +1,161 @@
> +#
> +# Copyright 2013 Freescale Semiconductor, Inc.
> +#
> +# This program is free software; you can redistribute it and/or  
> modify it
> +# under the terms of the GNU General Public License as published by  
> the Free
> +# Software Foundation; either version 2 of the License, or (at your  
> option)
> +# any later version.
> +#
> +
> +CONFIG_TPL_BUILD := y
> +export CONFIG_TPL_BUILD

Please find a way to reuse spl/Makefile by invoking it twice.

-Scott