[U-Boot] [PATCH 1/4] common: imx: Implement generic u-boot.nand target

[Benoît Thébaudeau]

Hi Tom,

On Thursday, February 28, 2013 4:24:22 PM, Benoît Thébaudeau wrote:
> Hi Tom,
> 
> On Thursday, February 28, 2013 3:03:09 PM, Tom Rini write:
> > On Wed, Feb 27, 2013 at 11:18:48PM +0100, Beno??t Th??baudeau wrote:
> > > Hi Marek,
> > > 
> > > On Monday, February 25, 2013 7:19:54 PM, Marek Vasut wrote:
> > > > Implement u-boot.nand target that can be reused with a small amount of
> > > > churn across all CPU models. The idea is to delegate the u-boot.nand
> > > > target
> > > > out of the main Makefile and into the CPU's Makefile (very similar to
> > > > what
> > > > u-boot.imx does now). The main Makefile shall only contain path to
> > > > which
> > > > the
> > > > u-boot.nand target is delegated. Hopefully this will not produce too
> > > > much
> > > > bloat in the main Makefile.
> > > > 
> > > > To demonstrate this implementation, add u-boot.nand target for i.MX53.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex at denx.de>
> > > > Cc: Beno??t Th??baudeau <benoit.thebaudeau at advansee.com>
> > > > Cc: Fabio Estevam <fabio.estevam at freescale.com>
> > > > Cc: Stefano Babic <sbabic at denx.de>
> > > > ---
> > > >  Makefile                     |   18 ++++++++++++++++++
> > > >  arch/arm/imx-common/Makefile |    6 ++++++
> > > >  2 files changed, 24 insertions(+)
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index 41054b7..8b1010a 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -470,6 +470,23 @@ $(obj)u-boot.img:	$(obj)u-boot.bin
> > > >  $(obj)u-boot.imx: $(obj)u-boot.bin depend
> > > >  		$(MAKE) -C $(SRCTREE)/arch/arm/imx-common $(obj)u-boot.imx
> > > >  
> > > > +#
> > > > +# Generic u-boot.nand target.
> > > > +#
> > > > +# Every CPU that needs u-boot.nand must add a path to an
> > > > implementation
> > > > of
> > > > +# the actual u-boot.nand generator below.
> > > > +#
> > > > +ifdef CONFIG_MX53
> > > > +CONFIG_NAND_TRG_PATH := $(SRCTREE)/arch/arm/imx-common
> > > > +endif
> > > > +
> > > > +$(obj)u-boot.nand: $(obj)u-boot.bin depend
> > > > +		if [ "X$(CONFIG_NAND_TRG_PATH)X" = "XX" ] ; then		\
> > > > +			echo "This CPU does not support u-boot.nand target!" ;	\
> > > > +			exit 1 ;						\
> > > > +		fi
> > > > +		$(MAKE) -C $(CONFIG_NAND_TRG_PATH) $(obj)u-boot.nand
> > > > +
> > > >  $(obj)u-boot.kwb:       $(obj)u-boot.bin
> > > >  		$(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \
> > > >  		-a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $@
> > > > @@ -857,6 +874,7 @@ clobber:	tidy
> > > >  	@rm -f $(obj)u-boot.kwb
> > > >  	@rm -f $(obj)u-boot.pbl
> > > >  	@rm -f $(obj)u-boot.imx
> > > > +	@rm -f $(obj)u-boot.nand
> > > >  	@rm -f $(obj)u-boot.ubl
> > > >  	@rm -f $(obj)u-boot.ais
> > > >  	@rm -f $(obj)u-boot.dtb
> > > > diff --git a/arch/arm/imx-common/Makefile
> > > > b/arch/arm/imx-common/Makefile
> > > > index 5d5c5b2..71ea36f 100644
> > > > --- a/arch/arm/imx-common/Makefile
> > > > +++ b/arch/arm/imx-common/Makefile
> > > > @@ -50,6 +50,12 @@ $(obj)u-boot.imx: $(OBJTREE)/u-boot.bin
> > > > $(OBJTREE)/$(patsubst "%",%,$(CONFIG_IMX
> > > >  	$(OBJTREE)/tools/mkimage -n $(filter-out %.bin,$^) -T imximage \
> > > >  	-e $(CONFIG_SYS_TEXT_BASE) -d $< $@
> > > >  
> > > > +$(obj)u-boot.nand: $(obj)u-boot.imx
> > > > +	(								\
> > > > +		echo -ne '\x00\x00\x00\x00\x46\x43\x42\x20\x01' ;	\
> > >                      ^
> > > It does not work in my environment (Ubuntu 12.10). -ne is interpreted as
> > > text,
> > > so the FCB is broken. This is because /bin/sh (set to dash) is invoked by
> > > default on my machine here. It would work with the /bin/bash set by the
> > > main
> > > Makefile for SHELL, but this is not passed to the sub-make.
> > > 
> > > So what do you think we should do:
> > >  1) Add "export SHELL" to the main Makefile?
> > >  2) Move the SHELL assignment from the main Makefile to the top-level
> > >  config.mk?
> > >  3) Set "SHELL=$(SHELL)" on the command line when invoking the sub-make?
> > >  4) Use printf instead of echo?
> > >  5) Something else?
> > > 
> > > I like 1). Do you see possible side effects?
> > 
> > Wait!  We do 1 already and have for years (cf7a7b99), so what's going
> > on?
> 
> Indeed! But I get anyway SHELL set to /bin/bash in /Makefile, and to /bin/sh
> in
> /arch/arm/imx-common/makefile.
> 
> I don't see SHELL being set anywhere else, and $(MAKE) SHELL=$(SHELL) works.
> 
> That's really weird. It's like SHELL export was ignored, or applied before
> the
> assignment. I will further dig into this.