From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Sat, 25 Jun 2011 14:10:50 +0200
Subject: [U-Boot] SPL framework re-design
In-Reply-To: <4E00799A.5040302@ti.com>
References: <4DF9B9E0.8020206@ti.com>
	<20110616104716.762DD19E5AC3@gemini.denx.de>
	<4DFB8579.1020309@ti.com> <4E00799A.5040302@ti.com>
Message-ID: <20110625121050.6CAAB20155D@gemini.denx.de>
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

Dear Aneesh V,

In message <4E00799A.5040302@ti.com> you wrote:
>
> > Here is a crude implementation of the top-down approach you had been
> > suggesting (or my interpretation of it). This is not complete yet and
> > serves only as a material for further discussions on this topic.
> 
> Here is an updated version of my prototype implementation with fixes
> for some issues pointed out by Scott. Please let me know your views
> about this.
> 
> ---
>   Makefile                        |    5 ++
>   include/configs/omap4_sdp4430.h |    1 +
>   spl/Makefile                    |   94 
> +++++++++++++++++++++++++++++++++++++++
>   spl/mmc/Makefile                |   55 +++++++++++++++++++++++
>   4 files changed, 155 insertions(+), 0 deletions(-)
>   create mode 100644 spl/Makefile
>   create mode 100644 spl/mmc/Makefile
> 
> diff --git a/Makefile b/Makefile
> index 8540e39..0321634 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -316,6 +316,7 @@ BOARD_SIZE_CHECK =
>   endif
> 
>   # Always append ALL so that arch config.mk's can add custom ones
> +ALL += spl
>   ALL += $(obj)u-boot.srec $(obj)u-boot.bin $(obj)System.map

This patch seems strangely white-space corrupted.

>   ifeq ($(CONFIG_NAND_U_BOOT),y)
> @@ -428,6 +429,9 @@ $(obj)u-boot-onenand.bin:	onenand_ipl $(obj)u-boot.bin
>   mmc_spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
>   		$(MAKE) -C mmc_spl/board/$(BOARDDIR) all
> 
> +spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
> +	$(MAKE) -C spl/ all
> +
>   $(obj)mmc_spl/u-boot-mmc-spl.bin:	mmc_spl

The mmc_spl/ is suppoed to be moved into spl/, isn't it?

>   $(VERSION_FILE):
> @@ -1142,6 +1146,7 @@ clean:
>   	@rm -f $(obj)spl/{u-boot-spl-generated.lds,u-boot-spl,u-boot-spl.map}
>   	@rm -f $(obj)onenand_ipl/onenand-{ipl,ipl.bin,ipl.map}

Dito here for onenand_ipl/ ?

>   	@rm -f 
> $(obj)mmc_spl/{u-boot.lds,u-boot-spl,u-boot-spl.map,u-boot-spl.bin,u-boot-mmc-spl.bin}

...also line-wrapped.

> +	@rm -f 
> $(obj)spl/{u-boot.lds,u-boot-spl,u-boot-spl.map,u-boot-spl.bin,u-boot-mmc-spl.bin}

Make this:

	@rm -f $(obj)spl/u-boot{.lds,-spl,-spl.map,-spl.bin,-mmc-spl.bin}

instead.

>   	@rm -f $(ONENAND_BIN)
>   	@rm -f $(obj)onenand_ipl/u-boot.lds

Goes away?

> --- /dev/null
> +++ b/spl/Makefile
> @@ -0,0 +1,94 @@
> +#
> +# (C) Copyright 2011 Daniel Schwierzeck, daniel.schwierzeck at googlemail.com.

Really???

> +# This file is released under the terms of GPL v2 and any later version.
> +# See the file COPYING in the root directory of the source tree for 
> details.
> +#
> +
> +include $(TOPDIR)/config.mk
> +LIBS-$(CONFIG_SYS_SPL_MMC_SUPPORT) = mmc/libmmc.o
> +LIBS-$(CONFIG_SYS_SPL_FAT_SUPPORT) += fat/libfat.o
> +LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += nand/libnand.o
> +LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += onenand/libonenand.o

As Mike mentioned, we can eventually directly include the OBJSs here
and omit the building of libraries?

> +LIBS-y += $(shell if [ -f $(ARCH)/Makefile ]; then echo \
> +	"$(ARCH)/lib$(ARCH).o"; fi)
> +LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/Makefile ]; then echo \
> +	"$(ARCH)/$(CPU)/lib$(CPU).o"; fi)
> +LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/$(SOC)/Makefile ]; then echo \
> +	"$(ARCH)/$(CPU)/$(SOC)/lib$(SOC).o"; fi)
> +LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/$(SOC)/$(BOARD)/Makefile ]; 
> then echo \
> +	"$(ARCH)/$(CPU)/$(SOC)/$(BOARD)/lib$(BOARD).o"; fi)

We should probably use /$(BOARDDIR)? here instead of /$(BOARD)/ to
allow for vendor directories (where "BOARDDIR = $(VENDOR)/$(BOARD)").

> +ALL	= $(obj)u-boot-spl.bin
> +
> +all:	$(ALL)

Do we need ALL then at all?

...
> diff --git a/spl/mmc/Makefile b/spl/mmc/Makefile
> new file mode 100644
> index 0000000..b4f7efd
> --- /dev/null
> +++ b/spl/mmc/Makefile
> @@ -0,0 +1,55 @@
> +#
> +# (C) Copyright 2000-2003
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.

Really???

> +$(obj)mmc.c:
> +	@rm -f $@
> +	@ln -s $(TOPDIR)/drivers/mmc/mmc.c $@
> +
> +$(obj)omap_hsmmc.c:
> +	@rm -f $@
> +	@ln -s $(TOPDIR)/drivers/mmc/omap_hsmmc.c $@

Hm... can we try to do without the symlinks?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The whole problem with the world is  that  fools  and  fanatics  are
always so certain of themselves, but wiser people so full of doubts."
- Bertrand Russell