[U-Boot] SPL framework re-design

[Aneesh V]

Dear Wolfgang,

On Saturday 25 June 2011 05:40 PM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<4E00799A.5040302 at 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.

Maybe, because I copy-pasted in Thunderbird instead of git-send-mail.

>
>>    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?

This patch was intended only as a prototype for the new directory
structure. I didn't bother to touch the existing stuff.

>
>>    $(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.

ok.

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

I am yet to think about the migration of existing code. If it's mere
movement of code I should be able to do it for all. But if it is about
re-factoring the code of other SoCs, I will not be able to do it. Hope
respective maintainers will take care of that.

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

I copied Daniel's Makefile and started from there.

>
>> +# 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?

I can't seem to find a mail from Mike on this thread. Did I miss any
mail?

Do you mean re-using equivalent libraries from the normal U-Boot
without re-compiling them?

>
>> +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)").

I didn't want to make the directory structure any longer than required.
But I can add this if required.

>
>> +ALL	= $(obj)u-boot-spl.bin
>> +
>> +all:	$(ALL)
>
> Do we need ALL then at all?

ok. I will remove it.
>
> ...
>> 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???

Again I copied the Makefile from arch/arm/cpu/armv7/Makefile as a
template.

>
>> +$(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?

Well. I think it's difficult. Most of my hardware initialization such
as clock init, SDRAM init etc need to know under what context it
is getting executed. The context can be:
1. SPL
2. Regular U-Boot executing from NOR flash
3. Regular U-Boot executing from SDRAM
etc.

Based on the context, the level of initialization will differ, but most
of the code could still be shared between these different contexts.

While the identification between a NOR boot and SDRAM boot is
determined run-time the distinction between SPL and normal U-Boot is
made based on CONFIG_PRELOADER.

Also, as mentioned by Daniel, start.S needs some customization for SPL.

Also, I agree with Scott's opinion that re-compiling some files while
re-using the binary of some other files won't be a good idea. In this
case, CONFIG_PRELOADER will be honored in some files but not in other
files. That will be a source of confusion for developers.

If you want to do away with symlinks, I would propose going with
Daniel's approach. This uses /spl as a remote building directory, but
do not create any symlinks.

Daniel,
You mentioned that /spl can not be used for source files. Isn't there a
way to workaround this problem?

best regards,
Aneesh