On 07/15/2013 04:36:29 AM, ying.zh...@freescale.com wrote:
+ifdef CONFIG_TPL
+$(obj)u-boot-with-spl.bin: $(obj)spl/u-boot-spl.bin $(obj)spl/u-boot-tpl.bin \
+               $(obj)u-boot.bin
+               $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SPL_PAD_TO) \
+                       -I binary -O binary \
+ $(obj)spl/u-boot-spl.bin $(obj)spl/u-boot-spl-pad.bin
+               $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SPL_PAD_TO) \
+                       -I binary -O binary \
+ $(obj)spl/u-boot-tpl.bin $(obj)spl/u-boot-tpl-pad.bin + cat $(obj)spl/u-boot-spl-pad.bin $(obj)spl/u-boot-tpl-pad.bin \
+                       $(obj)u-boot.bin > $@
+ rm $(obj)spl/u-boot-spl-pad.bin $(obj)spl/u-boot-tpl-pad.bin
+else
 $(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
+endif

Are you sure CONFIG_SPL_PAD_TO will always be the same for both stages?

How about something like:

# $@ 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)

$(obj)u-boot-with-spl.bin: $(obj)spl/u-boot-spl.bin $(obj)tpl/u-boot-with-tpl.bin $(call SPL_PAD_APPEND,$<,u-boot-with-tpl.bin,spl/u-boot-spl-pad.bin,$(CONFIG_SPL_PAD_TO))

$(obj)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))

@@ -621,7 +635,12 @@ $(obj)u-boot-nand.bin: nand_spl $(obj)u-boot.bin cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin

 $(obj)spl/u-boot-spl.bin:      $(SUBDIR_TOOLS) depend
-               $(MAKE) -C spl all
+               $(MAKE) -C spl clean
+               $(MAKE) -C spl all CONFIG_SPL_BUILD=y
+
+$(obj)spl/u-boot-tpl.bin:      $(SUBDIR_TOOLS) depend
+               $(MAKE) -C spl clean
+               $(MAKE) -C spl all CONFIG_TPL_BUILD=y CONFIG_SPL_BUILD=y

This will break in a parallel build, among other problems. Please use a separate tpl directory.

+ifeq ($(CONFIG_TPL_BUILD),y)
+LDFLAGS_u-boot-tpl += -T $(obj)u-boot-spl.lds $(LDFLAGS_FINAL)
+ifneq ($(CONFIG_SPL_TEXT_BASE),)
+LDFLAGS_u-boot-tpl += -Ttext $(CONFIG_SPL_TEXT_BASE)
+endif
+else
+ifeq ($(CONFIG_SPL_BUILD),y)
 LDFLAGS_u-boot-spl += -T $(obj)u-boot-spl.lds $(LDFLAGS_FINAL)
 ifneq ($(CONFIG_SPL_TEXT_BASE),)
 LDFLAGS_u-boot-spl += -Ttext $(CONFIG_SPL_TEXT_BASE)
 endif
+endif
+endif


Why do we need separate LDFLAGS for SPL and TPL? It doesn't look like you define them any differently. Can you use $(SPL_BIN) (a.k.a. $(FILENAME)) here? Making sure to ifdef CONFIG_SPL_BUILD so that we don't define $(LDFLAGS_) in other contexts.

 # Linus' kernel sanity checking tool
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
diff --git a/doc/README.TPL b/doc/README.TPL
new file mode 100644
index 0000000..3056696
--- /dev/null
+++ b/doc/README.TPL
@@ -0,0 +1,71 @@
+Generic TPL framework
+=====================
+
+Overview
+--------
+
+TPL---Third Program Loader.
+
+Due to the SPL on some boards(powerpc mpc85xx) has a size limit and cannot +be compatible with all the external device(e.g. DDR). So add a tertiary +program loader (TPL) to enable a loader stub loaded by the code from the +SPL. It loads the final uboot image into DDR, then jump to it to begin +execution. Now, only the powerpc mpc85xx has this requirement and will
+implemente it.
+
+Keep consistent with SPL, with this framework almost all source files for a +board can be reused. No code duplication or symlinking is necessary anymore.
+
+How it works
+------------
+
+There has been a directory TOPDIR/spl which contains only a Makefile. It is +shared by SPL and TPL. By the way, TPL will share something with SPL, such
+as options defined in the board config files.
+
+The object files are built separately for SPL/TPL and placed in this
+directory. The final binaries which are generated are u-boot-{spl|tpl},
+u-boot-{spl|tpl}.bin and u-boot-{spl|tpl}.map.
+
+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 be compiled for TPL with options choosed in the board
+config file, based on whether CONFIG_TPL_BUILD is set.
+
+For example:
+
+drivers/mtd/nand/Makefile:
+COBJS-$(CONFIG_SPL_NAND_INIT) += nand.o
+
+CONFIG_SPL_NAND_INIT is set in the include/configs/P1022DS.h:
+#ifdef CONFIG_TPL_BUILD
+#define CONFIG_SPL_NAND_INIT
+#endif
+
+The building of TPL images can be with:
+
+#define CONFIG_TPL
+
+Because TPL images normally have a different text base, one has to be
+configured by defining CONFIG_SPL_TEXT_BASE. The linker script has to be +defined with CONFIG_SPL_LDSCRIPT. Likewise, these symbols are all shared +with SPL, base on whether CONFIG_SPL_BUILD or CONFIG_TPL_BUILD is set.
+
+To support generic U-Boot libraries and drivers in the TPL binary one can
+optionally define CONFIG_SPL_XXX_SUPPORT. Currently following options
+are supported:
+
+CONFIG_SPL_SLIBCOMMON_SUPPORT (common/libcommon.o)
+CONFIG_SPL_LIBDISK_SUPPORT (disk/libdisk.o)
+CONFIG_SPL_I2C_SUPPORT (drivers/i2c/libi2c.o)
+CONFIG_SPL_GPIO_SUPPORT (drivers/gpio/libgpio.o)
+CONFIG_SPL_MMC_SUPPORT (drivers/mmc/libmmc.o)
+CONFIG_SPL_SERIAL_SUPPORT (drivers/serial/libserial.o)
+CONFIG_SPL_SPI_FLASH_SUPPORT (drivers/mtd/spi/libspi_flash.o)
+CONFIG_SPL_SPI_SUPPORT (drivers/spi/libspi.o)
+CONFIG_SPL_FAT_SUPPORT (fs/fat/libfat.o)
+CONFIG_SPL_LIBGENERIC_SUPPORT (lib/libgeneric.o)
+CONFIG_SPL_POWER_SUPPORT (drivers/power/libpower.o)
+CONFIG_SPL_NAND_SUPPORT (drivers/mtd/nand/libnand.o)
+CONFIG_SPL_DMA_SUPPORT (drivers/dma/libdma.o)
+CONFIG_SPL_POST_MEM_SUPPORT (post/drivers/memory.o)

Please don't duplicate all this. Only talk about what's different from normal SPL.

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index bb81e84..e1f817a 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_NAND_INIT) += nand.o

Thank you for not reorganizing the makefile, but could you explain why you need nand.o when none of the other SPLs (even non-minimal) do? Why can't platform code call board_nand_init() directly? Where is nand_init() being called from?

diff --git a/include/nand.h b/include/nand.h
index 228d871..2aa7238 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -153,6 +153,9 @@ int nand_unlock(nand_info_t *meminfo, loff_t start, size_t length,
 int nand_get_lock_status(nand_info_t *meminfo, loff_t offset);

 int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst);
+#ifdef CONFIG_TPL_BUILD
+int nand_load_image(uint32_t offs, unsigned int uboot_size, void *vdst);
+#endif
 void nand_deselect(void);


Don't ifdef prototypes. Plus, some other platforms may want this in other configurations.

+ifeq ($(CONFIG_SPL_BUILD),y)
 export CONFIG_SPL_BUILD
+endif

Why is this conditional? Remember, we set CONFIG_SPL_BUILD regardless of whether we have CONFIG_TPL_BUILD.

In any case, is there any problem exporting variables that aren't defined? Isn't that a no-op (at most, affecting the behavior if the variable is defined in the future)?

 # We want the final binaries in this directory
 obj := $(OBJTREE)/spl/

+clean:
+       @find $(obj) -type f \
+               \( -name 'core' -o -name '*.bak' -o -name '*~' \
+               -o -name '*.o'  -o -name '*.a' \) -print \
+               | xargs rm -f

What does this have to do with TPL?

+ifndef CONFIG_TPL_BUILD
 $(OBJTREE)/MLO:        $(obj)u-boot-spl.bin
        $(OBJTREE)/tools/mkimage -T omapimage \
                -a $(CONFIG_SPL_TEXT_BASE) -d $< $@
@@ -157,11 +171,12 @@ $(OBJTREE)/MLO:   $(obj)u-boot-spl.bin
 $(OBJTREE)/MLO.byteswap: $(obj)u-boot-spl.bin
        $(OBJTREE)/tools/mkimage -T omapimage -n byteswap \
                -a $(CONFIG_SPL_TEXT_BASE) -d $< $@
+endif

Is the ifndef really needed?

 $(OBJTREE)/SPL : $(obj)u-boot-spl.bin depend
                $(MAKE) -C $(SRCTREE)/arch/arm/imx-common $@

-ALL-y  += $(obj)u-boot-spl.bin
+ALL-y  += $(obj)$(FILENAME).bin

The makefile deals with many filenames... could you be more specific with something like $(SPL_NAME)?

-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to