[U-Boot] [PATCH 6/7 v9] NAND: TPL : introduce the TPL based on the SPL

Scott Wood scottwood at freescale.com
Wed Jul 24 02:21:20 CEST 2013 

On 07/23/2013 04:17:03 AM, ying.zhang at freescale.com wrote:
> From: Ying Zhang <b40530 at 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 is 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 is decided by the  
> memory's
> size that TPL runs. 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: Makefile: Build a separate autoconf.mk for SPL
> 
> Signed-off-by: Ying Zhang <b40530 at freescale.com>
> ---
> Change from v8:
> - Modify the doc/README.TPL.
> - Modify the Makefile.
> - Modify the drivers/mtd/nand/fsl_elbc_spl.c.
> - Modify the spl/Makefile.
> Change from v7:
> - Modify the doc/README.TPL
> - Modify the spl/Makefile.
> Change from v6:
> - Modify the description of the patch.
> - Add the separate the autoconf.mk for TPL.
> - Delete the file tpl/Makefile and the directory tpl.
> - Reuse the spl/Makefie in TPL.
> Change from v5:
> - Use ifdef to define "nand_load_image" to non-static for non-SPL.
> Change from v4:
> - No change.
> Change from v3:
> - No change.
> Change from v2:
> - No change.
> Change from v1:
> - Split from "powerpc/p1022ds: nand: introduce the TPL based on the  
> SPL".
> 
>  Makefile                        |   53  
> +++++++++++++++++++++++++++++++++------
>  README                          |   16 +++++++++++
>  config.mk                       |   30 ++++++++++++++++++++-
>  doc/README.TPL                  |   45  
> +++++++++++++++++++++++++++++++++
>  drivers/mtd/nand/Makefile       |    1 +
>  drivers/mtd/nand/fsl_elbc_spl.c |   20 ++++++++++++++
>  spl/Makefile                    |   29 +++++++++++++++------
>  7 files changed, 176 insertions(+), 18 deletions(-)
>  create mode 100644 doc/README.TPL
> 
> diff --git a/Makefile b/Makefile
> index 64e0ea1..b95ec5b 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
> @@ -413,6 +414,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))
> @@ -491,13 +493,27 @@ $(obj)u-boot.sha1:	$(obj)u-boot.bin
>  $(obj)u-boot.dis:	$(obj)u-boot
>  		$(OBJDUMP) -d $< > $@
> 
> +# $@ is output, $(1) and $(2) are inputs, $(3) is padded  
> intermediate,
> +# $(4) is pad-to
> +SPL_PAD_APPEND = \
> +		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(4) -I binary -O  
> binary \
> +		$(1) $(obj)$(3); \
> +		cat $(obj)$(3) $(obj)$(2) > $@; \
> +		rm $(obj)$(3)
> 
> +ifdef CONFIG_TPL
> +PAD_BIN := $(obj)tpl/u-boot-with-tpl.bin
> +else
> +PAD_BIN := $(obj)u-boot.bin
> +endif

I'd call this SPL_PAYLOAD.  PAD_BIN sounds like what gets passed in  
$(3), not $(2).

> -$(obj)u-boot-with-spl.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> -		$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SPL_PAD_TO) \
> -			-I binary -O binary $<  
> $(obj)spl/u-boot-spl-pad.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-spl.bin: $(obj)spl/u-boot-spl.bin $(PAD_BIN)
> +		$(call SPL_PAD_APPEND, \
> +		 
> $<,$(PAD_BIN),spl/u-boot-spl-pad.bin,$(CONFIG_SPL_PAD_TO))
> +
> +$(obj)tpl/u-boot-with-tpl.bin: $(obj)tpl/u-boot-tpl.bin  
> $(obj)u-boot.bin
> +		$(call SPL_PAD_APPEND, \
> +		$<,u-boot.bin,  
> tpl/u-boot-tpl-pad.bin,$(CONFIG_TPL_PAD_TO))

No space before "tpl/u-boot-tpl-pad.bin", and no line break.  Did you  
test this with an out-of-tree build?

>  $(obj)u-boot-with-spl.imx: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
>  		$(MAKE) -C $(SRCTREE)/arch/arm/imx-common \
> @@ -623,6 +639,10 @@ $(obj)u-boot-nand.bin:	nand_spl  
> $(obj)u-boot.bin
>  $(obj)spl/u-boot-spl.bin:	$(SUBDIR_TOOLS) depend
>  		$(MAKE) -C spl all
> 
> +$(obj)tpl/u-boot-tpl.bin:	$(SUBDIR_TOOLS) depend
> +		$(shell [ -d ${TPLTREE} ] || mkdir -p ${TPLTREE})
> +		$(MAKE) -C spl all CONFIG_TPL_BUILD=y

Let config.mk create the directory.

Also, there's no reason to test whether the directory already exists  
when using "mkdir -p".  And you're already in a shell at that point. :-P

> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index bb81e84..5270b3c 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -39,6 +39,7 @@ COBJS-$(CONFIG_SPL_NAND_SIMPLE) += nand_spl_simple.o
>  COBJS-$(CONFIG_SPL_NAND_LOAD) += nand_spl_load.o
>  COBJS-$(CONFIG_SPL_NAND_ECC) += nand_ecc.o
>  COBJS-$(CONFIG_SPL_NAND_BASE) += nand_base.o
> +COBJS-$(CONFIG_SPL_ENV_IN_NAND) += nand.o

This belongs in the next patch.  It's about the mpc85xx NAND TPL, not  
about TPL in general.

Likewise with the fsl_elbc_spl.c changes.

> diff --git a/drivers/mtd/nand/fsl_elbc_spl.c  
> b/drivers/mtd/nand/fsl_elbc_spl.c
> index 50ff4fe..9b959d1 100644
> --- a/drivers/mtd/nand/fsl_elbc_spl.c
> +++ b/drivers/mtd/nand/fsl_elbc_spl.c
> @@ -47,7 +47,11 @@ static void nand_wait(void)
>  	}
>  }
> 
> +#ifdef CONFIG_TPL_BUILD
> +int nand_spl_load_image(uint32_t offs, unsigned int uboot_size, void  
> *vdst)
> +#else
>  static int nand_load_image(uint32_t offs, unsigned int uboot_size,  
> void *vdst)
> +#endif
>  {
>  	fsl_lbc_t *regs = LBC_BASE_ADDR;
>  	uchar *buf = (uchar *)CONFIG_SYS_NAND_BASE;
> @@ -137,19 +141,35 @@ void nand_boot(void)
>  	/*
>  	 * Load U-Boot image from NAND into RAM
>  	 */
> +#ifdef CONFIG_TPL_BUILD
> +	nand_spl_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
> +				CONFIG_SYS_NAND_U_BOOT_SIZE,
> +				(void *)CONFIG_SYS_NAND_U_BOOT_DST);
> +#else
>  	nand_load_image(CONFIG_SYS_NAND_U_BOOT_OFFS,
>  			CONFIG_SYS_NAND_U_BOOT_SIZE,
>  			(void *)CONFIG_SYS_NAND_U_BOOT_DST);
> +#endif

Please use a #define to control the name of the function so the callers  
can stay the same -- and add a comment explaining why we don't just  
always use nand_spl_load_image (non-static makes the code too large for  
certain SPLs).

>  #ifdef CONFIG_NAND_ENV_DST
> +#ifdef CONFIG_TPL_BUILD
> +        nand_spl_load_image(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
> +			(void *)CONFIG_NAND_ENV_DST);
> +#else

Whitespace

>  	nand_load_image(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
>  			(void *)CONFIG_NAND_ENV_DST);
> +#endif
> 
>  #ifdef CONFIG_ENV_OFFSET_REDUND
> +#ifdef CONFIG_TPL_BUILD
> +        nand_spl_load_image(CONFIG_ENV_OFFSET_REDUND,  
> CONFIG_ENV_SIZE,
> +			(void *)CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE);
> +#else

Whitespace

> diff --git a/spl/Makefile b/spl/Makefile
> index eef8c87..37b01e3 100644
> --- a/spl/Makefile
> +++ b/spl/Makefile
> @@ -18,10 +18,23 @@
>  CONFIG_SPL_BUILD := y
>  export CONFIG_SPL_BUILD
> 
> +ifeq ($(CONFIG_TPL_BUILD),y)
> +export CONFIG_TPL_BUILD
> +SPL_BIN := u-boot-tpl
> +else
> +SPL_BIN := u-boot-spl
> +endif
> +
>  include $(TOPDIR)/config.mk
> 
>  # We want the final binaries in this directory
> +ifeq ($(CONFIG_TPL_BUILD),y)
> +obj := $(OBJTREE)/tpl/
> +SPL_TREE := $(TPLTREE)
> +else
>  obj := $(OBJTREE)/spl/
> +SPL_TREE := $(SPLTREE)
> +endif
> 
>  HAVE_VENDOR_COMMON_LIB = $(if $(wildcard  
> $(SRCTREE)/board/$(VENDOR)/common/Makefile),y,n)
> 
> @@ -111,12 +124,12 @@ endif
> 
>  # Add GCC lib
>  ifeq ("$(USE_PRIVATE_LIBGCC)", "yes")
> -PLATFORM_LIBGCC = $(SPLTREE)/arch/$(ARCH)/lib/libgcc.o
> +PLATFORM_LIBGCC = $(SPL_TREE)/arch/$(ARCH)/lib/libgcc.o
>  PLATFORM_LIBS := $(filter-out %/libgcc.o, $(filter-out -lgcc,  
> $(PLATFORM_LIBS))) $(PLATFORM_LIBGCC)
>  endif

Please do not have one variable named SPLTREE and another named  
SPL_TREE.

-Scott

More information about the U-Boot mailing list