From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Fri, 12 Jul 2013 16:43:13 -0500
Subject: [U-Boot] [PATCH 6/7 v7] NAND: TPL : introduce the TPL based on
	the SPL
In-Reply-To: <1373363450-16932-6-git-send-email-ying.zhang@freescale.com>
	(from ying.zhang@freescale.com on Tue Jul  9 04:50:49 2013)
Message-ID: <1373665393.8183.288@snotra>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 07/09/2013 04:50:49 AM, ying.zhang at freescale.com wrote:
> +During the TPL build a variable named CONFIG_TPL_BUILD is exported
> +in the make environment and also appended to CPPFLAGS with  
> -DCONFIG_TPL_BUILD.
> +Source files can therefore be compiled for TPL with different  
> settings.
> +
> +For example:
> +
> +ifeq ($(CONFIG_TPL_BUILD),y)
> +COBJS-y += board_tpl.o
> +else
> +COBJS-y += board.o
> +endif
> +
> +COBJS-$(CONFIG_TPL_BUILD) += foo.o

This is not how it should normally be done.  Normally the board config  
file should determine which SPL options to choose, based on whether  
CONFIG_TPL_BUILD is set.

> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> index bb81e84..6507fcf 100644
> --- a/drivers/mtd/nand/Makefile
> +++ b/drivers/mtd/nand/Makefile
> @@ -29,6 +29,8 @@ ifdef CONFIG_CMD_NAND
> 
>  ifdef CONFIG_SPL_BUILD
> 
> +ifndef CONFIG_TPL_BUILD

Why?  Let the board file define what it wants for the TPL case, just as  
in SPL.

>  ifdef CONFIG_SPL_NAND_DRIVERS
>  NORMAL_DRIVERS=y
>  endif
> @@ -40,6 +42,10 @@ 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
> 
> +else
> +COBJS-y += nand.o
> +endif

What do you need nand.c for?  Why do you think all other TPLs will have  
the same need?

>  else # not spl
> 
>  NORMAL_DRIVERS=y
> @@ -82,9 +88,11 @@ COBJS-$(CONFIG_NAND_DOCG4) += docg4.o
> 
>  else  # minimal SPL drivers
> 
> +ifdef CONFIG_SPL_BUILD
>  COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
>  COBJS-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
>  COBJS-$(CONFIG_NAND_MXC) += mxc_nand_spl.o
> +endif

How are you going to get here in the first place without  
CONFIG_SPL_BUILD?

> @@ -162,6 +173,17 @@ $(OBJTREE)/SPL : $(obj)u-boot-spl.bin depend
>  		$(MAKE) -C $(SRCTREE)/arch/arm/imx-common $@
> 
>  ALL-y	+= $(obj)u-boot-spl.bin
> +else
> +$(OBJTREE)/MLO: $(obj)u-boot-tpl.bin
> +	$(OBJTREE)/tools/mkimage -T omapimage \
> +		-a $(CONFIG_SPL_TEXT_BASE) -d $< $@
> +
> +$(OBJTREE)/MLO.byteswap: $(obj)u-boot-tpl.bin
> +	$(OBJTREE)/tools/mkimage -T omapimage -n byteswap \
> +		-a $(CONFIG_SPL_TEXT_BASE) -d $< $@
> +
> +ALL-y   += $(obj)u-boot-tpl.bin
> +endif

Why?  There's not even a user of MLO with TPL.

In any case, is there no way we can just set filename variables based  
on SPL/TPL here, rather than duplicating code?

> +ifndef CONFIG_TPL_BUILD
>  $(obj)u-boot-spl.bin:	$(obj)u-boot-spl
>  	$(OBJCOPY) $(OBJCFLAGS) -O binary $< $@
> 
> @@ -185,6 +208,18 @@ GEN_UBOOT = \
> 
>  $(obj)u-boot-spl:	depend $(START) $(LIBS) $(obj)u-boot-spl.lds
>  	$(GEN_UBOOT)
> +else
> +$(obj)u-boot-tpl.bin:   $(obj)u-boot-tpl
> +	$(OBJCOPY) $(OBJCFLAGS) -O binary $< $@
> +
> +GEN_UBOOT = \
> +	cd $(obj) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $(__START) \
> +		--start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \
> +		-Map u-boot-tpl.map -o u-boot-tpl
> +
> +$(obj)u-boot-tpl:	depend $(START) $(LIBS) $(obj)u-boot-spl.lds
> +	$(GEN_UBOOT)
> +endif

Likewise.

-Scott