[U-Boot] [PATCH 0/8 v2] Add support to tpl boot and p1021mds board
This is the second version of patchset to add support of TPL(Tertiary Program Loader) and P1021MDS board. Compared with the previous version, patch #3 is splitted into two patches and incorporates the comments from Wolfgang and Mike. Patch#4 has changes based on 2010.12-rc2 release. The other patches remain unchanged.
[PATCH 1/8] powerpc/85xx: do not reloc l2srbar if CONFIG_FLASH_BASE is not defined [PATCH 2/8] 8xxx/ddr: add support to only compute the ddr sdram size [PATCH 3/8 v2] Introduce the Tertiary Program loader [PATCH 4/8 v2] powerpc/85xx: add TPL_BOOT support [PATCH 5/8 v2] P1021MDS: add P1021MDS board support [PATCH 6/8] powerpc/p1021: add more P1021 defines. [PATCH 7/8] powerpc/85xx: do not initialize QE if QE's firmware is in nand flash [PATCH 8/8] p1021mds: add QE and UEC support
From: Haiying Wang Haiying.Wang@freescale.com
This fixes the compiling error for the board which doesn't have NOR flash (so CONFIG_FLASH_BASE is not defined)
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com --- arch/powerpc/cpu/mpc85xx/cpu_init.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c index 27236a0..4b8faa5 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c @@ -327,7 +327,7 @@ int cpu_init_r(void) if (l2cache->l2ctl & MPC85xx_L2CTL_L2E) { puts("already enabled"); l2srbar = l2cache->l2srbar0; -#ifdef CONFIG_SYS_INIT_L2_ADDR +#if defined(CONFIG_SYS_INIT_L2_ADDR) && defined(CONFIG_SYS_FLASH_BASE) if (l2cache->l2ctl & MPC85xx_L2CTL_L2SRAM_ENTIRE && l2srbar >= CONFIG_SYS_FLASH_BASE) { l2srbar = CONFIG_SYS_INIT_L2_ADDR;
On Dec 1, 2010, at 9:35 AM, Haiying.Wang@freescale.com Haiying.Wang@freescale.com wrote:
From: Haiying Wang Haiying.Wang@freescale.com
This fixes the compiling error for the board which doesn't have NOR flash (so CONFIG_FLASH_BASE is not defined)
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com
arch/powerpc/cpu/mpc85xx/cpu_init.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
applied to 85xx
- k
From: Haiying Wang Haiying.Wang@freescale.com
This patch adds fsl_ddr_sdram_size to only calculate the ddr sdram size, in case that the DDR SDRAM is initialized in the 2nd stage uboot and should not be intialized again in the final stage uboot.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com --- arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c | 10 ++++++++- arch/powerpc/cpu/mpc8xxx/ddr/ddr.h | 8 ++++-- arch/powerpc/cpu/mpc8xxx/ddr/main.c | 31 +++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c b/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c index 3fec100..8fdafdb 100644 --- a/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c +++ b/arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c @@ -1176,7 +1176,8 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, fsl_ddr_cfg_regs_t *ddr, const common_timing_params_t *common_dimm, const dimm_params_t *dimm_params, - unsigned int dbw_cap_adj) + unsigned int dbw_cap_adj, + unsigned int size_only) { unsigned int i; unsigned int cas_latency; @@ -1394,6 +1395,13 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, printf("CS%d is disabled.\n", i); }
+ /* + * In the case we only need to compute the ddr sdram size, we only need + * to set csn registers, so return from here. + */ + if (size_only) + return 0; + set_ddr_eor(ddr, popts);
#if !defined(CONFIG_FSL_DDR1) diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/ddr.h b/arch/powerpc/cpu/mpc8xxx/ddr/ddr.h index 98acb8d..8c24131 100644 --- a/arch/powerpc/cpu/mpc8xxx/ddr/ddr.h +++ b/arch/powerpc/cpu/mpc8xxx/ddr/ddr.h @@ -1,5 +1,5 @@ /* - * Copyright 2008 Freescale Semiconductor, Inc. + * Copyright 2008-2010 Freescale Semiconductor, Inc. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -55,7 +55,8 @@ typedef struct { #define STEP_ALL 0xFFF
extern unsigned long long -fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step); +fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step, + unsigned int size_only);
extern const char * step_to_string(unsigned int step);
@@ -64,7 +65,8 @@ compute_fsl_memctl_config_regs(const memctl_options_t *popts, fsl_ddr_cfg_regs_t *ddr, const common_timing_params_t *common_dimm, const dimm_params_t *dimm_parameters, - unsigned int dbw_capacity_adjust); + unsigned int dbw_capacity_adjust, + unsigned int size_only); extern unsigned int compute_lowest_common_dimm_parameters(const dimm_params_t *dimm_params, common_timing_params_t *outpdimm, diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/main.c b/arch/powerpc/cpu/mpc8xxx/ddr/main.c index 6d582e9..b89b471 100644 --- a/arch/powerpc/cpu/mpc8xxx/ddr/main.c +++ b/arch/powerpc/cpu/mpc8xxx/ddr/main.c @@ -1,5 +1,5 @@ /* - * Copyright 2008 Freescale Semiconductor, Inc. + * Copyright 2008-2010 Freescale Semiconductor, Inc. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -233,7 +233,8 @@ int step_assign_addresses(fsl_ddr_info_t *pinfo, }
unsigned long long -fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step) +fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step, + unsigned int size_only) { unsigned int i, j; unsigned int all_controllers_memctl_interleaving = 0; @@ -338,7 +339,8 @@ fsl_ddr_compute(fsl_ddr_info_t *pinfo, unsigned int start_step) &pinfo->memctl_opts[i], &ddr_reg[i], &timing_params[i], pinfo->dimm_params[i], - dbw_capacity_adjust[i]); + dbw_capacity_adjust[i], + size_only); }
default: @@ -405,7 +407,7 @@ phys_size_t fsl_ddr_sdram(void) memset(&info, 0, sizeof(fsl_ddr_info_t));
/* Compute it once normally. */ - total_memory = fsl_ddr_compute(&info, STEP_GET_SPD); + total_memory = fsl_ddr_compute(&info, STEP_GET_SPD, 0);
/* Check for memory controller interleaving. */ memctl_interleaved = 0; @@ -430,7 +432,8 @@ phys_size_t fsl_ddr_sdram(void) info.memctl_opts[i].memctl_interleaving = 0; debug("Recomputing with memctl_interleaving off.\n"); total_memory = fsl_ddr_compute(&info, - STEP_ASSIGN_ADDRESSES); + STEP_ASSIGN_ADDRESSES, + 0); } }
@@ -477,3 +480,21 @@ phys_size_t fsl_ddr_sdram(void)
return total_memory; } + +/* + * fsl_ddr_sdram_size() - This function only returns the size of the total + * memory without setting ddr control registers. + */ +phys_size_t +fsl_ddr_sdram_size(void) +{ + fsl_ddr_info_t info; + unsigned long long total_memory = 0; + + memset(&info, 0 , sizeof(fsl_ddr_info_t)); + + /* Compute it once normally. */ + total_memory = fsl_ddr_compute(&info, STEP_GET_SPD, 1); + + return total_memory; +}
On Dec 1, 2010, at 9:35 AM, Haiying.Wang@freescale.com Haiying.Wang@freescale.com wrote:
From: Haiying Wang Haiying.Wang@freescale.com
This patch adds fsl_ddr_sdram_size to only calculate the ddr sdram size, in case that the DDR SDRAM is initialized in the 2nd stage uboot and should not be intialized again in the final stage uboot.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com
arch/powerpc/cpu/mpc8xxx/ddr/ctrl_regs.c | 10 ++++++++- arch/powerpc/cpu/mpc8xxx/ddr/ddr.h | 8 ++++-- arch/powerpc/cpu/mpc8xxx/ddr/main.c | 31 +++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-)
applied
- k
From: Haiying Wang Haiying.Wang@freescale.com
TPL is introduced to enable a loader stub that boots out of some type of RAM, after being loaded by an SPL or similar platform-specific mechanism.
One example of using this tpl loader is to initialize the ddr through spd code in case the L2 SRAM size is not big enough to hold the final uboot image and the nand spl code needs to be limitated to 4K byte, then tpl code will load the final uboot image after ddr is initialized.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com --- Incorporate Mike's comment to use new variable NAND_SPL_OBJS-y Makefile | 17 +++++++++++++++-- README | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 87a383d..94af465 100644 --- a/Makefile +++ b/Makefile @@ -290,6 +290,10 @@ LDPPFLAGS += \ $(shell $(LD) --version | \ sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p')
+ifeq ($(CONFIG_TPL_U_BOOT),y) +TPL_BOOT = tpl +endif + ifeq ($(CONFIG_NAND_U_BOOT),y) NAND_SPL = nand_spl U_BOOT_NAND = $(obj)u-boot-nand.bin @@ -407,8 +411,15 @@ $(obj)u-boot.lds: $(LDSCRIPT) $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin - cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin +$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend + $(MAKE) -C tpl/board/$(BOARDDIR) all + +NAND_SPL_OBJS-y += $(obj)nand_spl/u-boot-spl-16k.bin +NAND_SPL_OBJS-$(CONFIG_TPL_U_BOOT) += $(obj)tpl/u-boot-tpl.bin +NAND_SPL_OBJS-y += $(obj)u-boot.bin + +$(U_BOOT_NAND): $(NAND_SPL) $(TPL_BOOT) $(obj)u-boot.bin + cat $(NAND_SPL_OBJS-y) > u-boot-nand.bin
$(ONENAND_IPL): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk $(MAKE) -C onenand_ipl/board/$(BOARDDIR) all @@ -1226,6 +1237,7 @@ clean: @rm -f $(obj)lib/asm-offsets.s @rm -f $(obj)nand_spl/{u-boot.lds,u-boot-spl,u-boot-spl.map,System.map} @rm -f $(obj)onenand_ipl/onenand-{ipl,ipl.bin,ipl.map} + @rm -f $(obj)tpl/{u-boot-tpl,u-boot-tpl.map} @rm -f $(ONENAND_BIN) @rm -f $(obj)onenand_ipl/u-boot.lds @rm -f $(TIMESTAMP_FILE) $(VERSION_FILE) @@ -1250,6 +1262,7 @@ clobber: clean @rm -fr $(obj)include/generated @[ ! -d $(obj)nand_spl ] || find $(obj)nand_spl -name "*" -type l -print | xargs rm -f @[ ! -d $(obj)onenand_ipl ] || find $(obj)onenand_ipl -name "*" -type l -print | xargs rm -f + @[ ! -d $(obj)tpl ] || find $(obj)tpl -name "*" -type l -print | xargs rm -f
ifeq ($(OBJTREE),$(SRCTREE)) mrproper \ diff --git a/README b/README index 68f5fb0..9c7d40d 100644 --- a/README +++ b/README @@ -2108,6 +2108,33 @@ FIT uImage format: Adds the MTD partitioning infrastructure from the Linux kernel. Needed for UBI support.
+- NAND Boot Support + CONFIG_NAND_U_BOOT + + Builds a U-Boot image that boots from NAND, prefixed by a small + loader stub (secondary program loader -- SPL) that loads the + rest of U-Boot into RAM. This symbol will be set in all build + phases. + + CONFIG_NAND_SPL + + This is set by the build system when compiling code to go into + the SPL. It is not set when building the code that the SPL + loads. + +- TPL Boot Support + CONFIG_TPL_U_BOOT + + Builds a U-Boot image that contains a loader stub (tertiary + program loader -- TPL) that boots out of some type of RAM, + after being loaded by an SPL or similar platform-specific + mechanism. This symbol will be set in all build phases. + + CONFIG_TPL_BOOT + + This is set by the build system when compiling code to go into + the TPL. It is not set when building the code that the TPL + loads, or when building the SPL.
Modem Support: --------------
On Dec 1, 2010, at 9:35 AM, Haiying.Wang@freescale.com Haiying.Wang@freescale.com wrote:
From: Haiying Wang Haiying.Wang@freescale.com
TPL is introduced to enable a loader stub that boots out of some type of RAM, after being loaded by an SPL or similar platform-specific mechanism.
One example of using this tpl loader is to initialize the ddr through spd code in case the L2 SRAM size is not big enough to hold the final uboot image and the nand spl code needs to be limitated to 4K byte, then tpl code will load the final uboot image after ddr is initialized.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com
Incorporate Mike's comment to use new variable NAND_SPL_OBJS-y Makefile | 17 +++++++++++++++-- README | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-)
Wolfgang,
Did you plan on review this patch?
- k
Dear Kumar Gala,
In message BE116222-9427-4216-9CAE-00E3A4B14B5F@kernel.crashing.org you wrote:
Did you plan on review this patch?
Just done - I wonder if this code has ever been tested at all?
Best regards,
Wolfgang Denk
Dear Haiying.Wang@freescale.com,
In message 1291217737-3870-4-git-send-email-Haiying.Wang@freescale.com you wrote:
From: Haiying Wang Haiying.Wang@freescale.com
TPL is introduced to enable a loader stub that boots out of some type of RAM, after being loaded by an SPL or similar platform-specific mechanism.
One example of using this tpl loader is to initialize the ddr through spd code in case the L2 SRAM size is not big enough to hold the final uboot image and the nand spl code needs to be limitated to 4K byte, then tpl code will load the final uboot image after ddr is initialized.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com
Incorporate Mike's comment to use new variable NAND_SPL_OBJS-y Makefile | 17 +++++++++++++++-- README | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 87a383d..94af465 100644 --- a/Makefile +++ b/Makefile @@ -290,6 +290,10 @@ LDPPFLAGS += \ $(shell $(LD) --version | \ sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p')
+ifeq ($(CONFIG_TPL_U_BOOT),y) +TPL_BOOT = tpl +endif
I don't understand what the "TPL_BOOT" is good for, or how it's supposed to work.
ifeq ($(CONFIG_NAND_U_BOOT),y) NAND_SPL = nand_spl U_BOOT_NAND = $(obj)u-boot-nand.bin @@ -407,8 +411,15 @@ $(obj)u-boot.lds: $(LDSCRIPT) $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin
cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) all
Assume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
Has this code ever been tested?
And if we use a variable for the "tlp" string, should this not be
... $(MAKE) -C $(TPL_BOOT)/board/$(BOARDDIR) all
??
+NAND_SPL_OBJS-y += $(obj)nand_spl/u-boot-spl-16k.bin +NAND_SPL_OBJS-$(CONFIG_TPL_U_BOOT) += $(obj)tpl/u-boot-tpl.bin +NAND_SPL_OBJS-y += $(obj)u-boot.bin
Ditto here and in the following - but how is NAND_SPL related to TPL building? These should be completely independent?
+- TPL Boot Support
CONFIG_TPL_U_BOOTBuilds a U-Boot image that contains a loader stub (tertiaryprogram loader -- TPL) that boots out of some type of RAM,after being loaded by an SPL or similar platform-specificmechanism. This symbol will be set in all build phases.CONFIG_TPL_BOOTThis is set by the build system when compiling code to go intothe TPL. It is not set when building the code that the TPLloads, or when building the SPL.
Can we not do with a single variable definition?
Best regards,
Wolfgang Denk
On Sat, 2011-01-22 at 23:04 +0100, Wolfgang Denk wrote:
diff --git a/Makefile b/Makefile index 87a383d..94af465 100644 --- a/Makefile +++ b/Makefile @@ -290,6 +290,10 @@ LDPPFLAGS += \ $(shell $(LD) --version | \ sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p')
+ifeq ($(CONFIG_TPL_U_BOOT),y) +TPL_BOOT = tpl +endif
I don't understand what the "TPL_BOOT" is good for, or how it's supposed to work.
TPL_BOOT works like NAND_SPL but after NAND_SPL is executed. It is a middle stage boot loader to balance the 4K nand spl limitation which can not include ddr spd code and the 256K L2 SRAM size which can not accommodate the final uboot image on some Freescale Qoriq P1 platforms. The main purpose of tpl is to initialize the ddr with spd code in l2 sram then load the final uboot image to ddr. The reason to call tpl is because it runs after spl, the Second Program Loader. My original patch used CONFIG_MIDDLE_STAGE_SRAM_BOOT but you recommended to use CONFIG_SYS_2ND_STAGE_BOOT(http://lists.denx.de/pipermail/u-boot/2010-August/075653.html). However, 2ND STAGE is not correct here since it runs after SPL.
ifeq ($(CONFIG_NAND_U_BOOT),y) NAND_SPL = nand_spl U_BOOT_NAND = $(obj)u-boot-nand.bin @@ -407,8 +411,15 @@ $(obj)u-boot.lds: $(LDSCRIPT) $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin
cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) allAssume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
No, I don't think there is a build error.
Has this code ever been tested?
Yes, I tested it on P1021MDS board, and also built for other 85xx NAND_config without error.
... $(MAKE) -C $(TPL_BOOT)/board/$(BOARDDIR) all
??
+NAND_SPL_OBJS-y += $(obj)nand_spl/u-boot-spl-16k.bin +NAND_SPL_OBJS-$(CONFIG_TPL_U_BOOT) += $(obj)tpl/u-boot-tpl.bin +NAND_SPL_OBJS-y += $(obj)u-boot.bin
Ditto here and in the following - but how is NAND_SPL related to TPL building? These should be completely independent?
It came up with Mike Frysinger's comments http://lists.denx.de/pipermail/u-boot/2010-November/082373.html . It is applied in the case that the TPL is used with nand spl build. In fact, our another usage of TPL_BOOT is for SPI/SD boot. Patch will come is this set is accepted.
CONFIG_TPL_U_BOOTBuilds a U-Boot image that contains a loader stub (tertiaryprogram loader -- TPL) that boots out of some type of RAM,after being loaded by an SPL or similar platform-specificmechanism. This symbol will be set in all build phases.CONFIG_TPL_BOOTThis is set by the build system when compiling code to go intothe TPL. It is not set when building the code that the TPLloads, or when building the SPL.Can we not do with a single variable definition?
I did not get it. Could you please give a recommendation?
Thanks.
Haiying
Dear Haiying Wang,
In message 1295842861.2196.38.camel@haiying-laptop you wrote:
On Sat, 2011-01-22 at 23:04 +0100, Wolfgang Denk wrote:
diff --git a/Makefile b/Makefile index 87a383d..94af465 100644 --- a/Makefile +++ b/Makefile @@ -290,6 +290,10 @@ LDPPFLAGS += \ $(shell $(LD) --version | \ sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p')
+ifeq ($(CONFIG_TPL_U_BOOT),y) +TPL_BOOT = tpl +endif
I don't understand what the "TPL_BOOT" is good for, or how it's supposed to work.
TPL_BOOT works like NAND_SPL but after NAND_SPL is executed. It is a middle stage boot loader to balance the 4K nand spl limitation which can not include ddr spd code and the 256K L2 SRAM size which can not accommodate the final uboot image on some Freescale Qoriq P1 platforms.
Yes, I understand what you are atempting to do.
What I do not understand is what the TPL_BOOT variable in the Makefile is good for. I cannot understand the current use.
The main purpose of tpl is to initialize the ddr with spd code in l2 sram then load the final uboot image to ddr. The reason to call tpl is because it runs after spl, the Second Program Loader. My original patch used CONFIG_MIDDLE_STAGE_SRAM_BOOT but you recommended to use CONFIG_SYS_2ND_STAGE_BOOT(http://lists.denx.de/pipermail/u-boot/2010-August/075653.html). However, 2ND STAGE is not correct here since it runs after SPL.
ifeq ($(CONFIG_NAND_U_BOOT),y) NAND_SPL = nand_spl U_BOOT_NAND = $(obj)u-boot-nand.bin @@ -407,8 +411,15 @@ $(obj)u-boot.lds: $(LDSCRIPT) $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin
cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) allAssume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
No, I don't think there is a build error.
WEell, if CONFIG_TPL_U_BOOT is not 'y', then TPL_BOOT is not defined, which results in this make rule:
: $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
i. e. there would be no target name befoe the semicolon.
Has this code ever been tested?
Yes, I tested it on P1021MDS board, and also built for other 85xx NAND_config without error.
Did you run a "MAKEALL ppc", i. e. build for all PPC board, not only NAND booting versions?
CONFIG_TPL_U_BOOTBuilds a U-Boot image that contains a loader stub (tertiaryprogram loader -- TPL) that boots out of some type of RAM,after being loaded by an SPL or similar platform-specificmechanism. This symbol will be set in all build phases.CONFIG_TPL_BOOTThis is set by the build system when compiling code to go intothe TPL. It is not set when building the code that the TPLloads, or when building the SPL.Can we not do with a single variable definition?
I did not get it. Could you please give a recommendation?
Well, I see a pollution with such CONFIG_ settings. I don;t have a solution ready to recommend, but if you can find a way not to define so many different settings for a single purpose that wouldbe great.
Best regards,
Wolfgang Denk
On Mon, 24 Jan 2011 13:49:19 +0100 Wolfgang Denk wd@denx.de wrote:
CONFIG_TPL_U_BOOTBuilds a U-Boot image that contains a loader stub (tertiaryprogram loader -- TPL) that boots out of some type of RAM,after being loaded by an SPL or similar platform-specificmechanism. This symbol will be set in all build phases.CONFIG_TPL_BOOTThis is set by the build system when compiling code to go intothe TPL. It is not set when building the code that the TPLloads, or when building the SPL.Can we not do with a single variable definition?
I did not get it. Could you please give a recommendation?
Well, I see a pollution with such CONFIG_ settings. I don;t have a solution ready to recommend, but if you can find a way not to define so many different settings for a single purpose that wouldbe great.
They mean different things. We can't "do with a single variable definition" in the current NAND SPL. Why would TPL be any different?
Now, the naming could be clearer. Changing CONFIG_TPL_BOOT into CONFIG_TPL would make it look more like the existing SPL names. Or we could do something like:
CONFIG_HAS_SPL /* set in all of U-Boot when an SPL is used */ CONFIG_IN_SPL /* set only when building the SPL */ CONFIG_HAS_TPL /* set in all of U-Boot when a TPL is used */ CONFIG_IN_TPL /* set only when building the TPL */
-Scott
Dear Scott Wood,
In message 20110124133835.5b26bbf3@udp111988uds.am.freescale.net you wrote:
Now, the naming could be clearer. Changing CONFIG_TPL_BOOT into CONFIG_TPL would make it look more like the existing SPL names. Or we could do something like:
CONFIG_HAS_SPL /* set in all of U-Boot when an SPL is used */ CONFIG_IN_SPL /* set only when building the SPL */ CONFIG_HAS_TPL /* set in all of U-Boot when a TPL is used */ CONFIG_IN_TPL /* set only when building the TPL */
I like that much better.
Thanks.
Best regards,
Wolfgang Denk
On Mon, 2011-01-24 at 13:49 +0100, Wolfgang Denk wrote:
+ifeq ($(CONFIG_TPL_U_BOOT),y) +TPL_BOOT = tpl +endif
I don't understand what the "TPL_BOOT" is good for, or how it's supposed to work.
TPL_BOOT works like NAND_SPL but after NAND_SPL is executed. It is a middle stage boot loader to balance the 4K nand spl limitation which can not include ddr spd code and the 256K L2 SRAM size which can not accommodate the final uboot image on some Freescale Qoriq P1 platforms.
Yes, I understand what you are atempting to do.
What I do not understand is what the TPL_BOOT variable in the Makefile is good for. I cannot understand the current use.
Well, it was used to generate the tpl image under tpl/ directory. Maybe TPL_BOOT is a bad name here, I just thought it was too simple to use TPL.
The main purpose of tpl is to initialize the ddr with spd code in l2 sram then load the final uboot image to ddr. The reason to call tpl is because it runs after spl, the Second Program Loader. My original patch used CONFIG_MIDDLE_STAGE_SRAM_BOOT but you recommended to use CONFIG_SYS_2ND_STAGE_BOOT(http://lists.denx.de/pipermail/u-boot/2010-August/075653.html). However, 2ND STAGE is not correct here since it runs after SPL.
ifeq ($(CONFIG_NAND_U_BOOT),y) NAND_SPL = nand_spl U_BOOT_NAND = $(obj)u-boot-nand.bin @@ -407,8 +411,15 @@ $(obj)u-boot.lds: $(LDSCRIPT) $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin
cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) allAssume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
No, I don't think there is a build error.
WEell, if CONFIG_TPL_U_BOOT is not 'y', then TPL_BOOT is not defined, which results in this make rule:
: $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
i. e. there would be no target name befoe the semicolon.
If TPL_BOOT here is not defined, the reset(after semicolon) will not be executed, just like NAND_SPL and ONENAND_IPL etc.
Has this code ever been tested?
Yes, I tested it on P1021MDS board, and also built for other 85xx NAND_config without error.
Did you run a "MAKEALL ppc", i. e. build for all PPC board, not only NAND booting versions?
No, I didn't. I will do it and let you know. But I did pass the build for other 85xx non-nand booting version.
CONFIG_TPL_U_BOOTBuilds a U-Boot image that contains a loader stub (tertiaryprogram loader -- TPL) that boots out of some type of RAM,after being loaded by an SPL or similar platform-specificmechanism. This symbol will be set in all build phases.CONFIG_TPL_BOOTThis is set by the build system when compiling code to go intothe TPL. It is not set when building the code that the TPLloads, or when building the SPL.Can we not do with a single variable definition?
I did not get it. Could you please give a recommendation?
Well, I see a pollution with such CONFIG_ settings. I don;t have a solution ready to recommend, but if you can find a way not to define so many different settings for a single purpose that wouldbe great.
Will apply Scott's recommendation.
Thanks.
Haiying
Dear Haiying Wang,
In message 1295906076.2051.127.camel@haiying-laptop you wrote:
What I do not understand is what the TPL_BOOT variable in the Makefile is good for. I cannot understand the current use.
Well, it was used to generate the tpl image under tpl/ directory. Maybe TPL_BOOT is a bad name here, I just thought it was too simple to use TPL.
It's not the name. But you use it ina few places here, buth then hard encode "tpl" in a number of other paces there. Which means that you cannot change TPL_BOOT to any other value, or building would break. So why do we need this variable?
+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) allAssume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
No, I don't think there is a build error.
WEell, if CONFIG_TPL_U_BOOT is not 'y', then TPL_BOOT is not defined, which results in this make rule:
: $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
i. e. there would be no target name befoe the semicolon.
If TPL_BOOT here is not defined, the reset(after semicolon) will not be executed, just like NAND_SPL and ONENAND_IPL etc.
Sorry, I cannot follow - which reset? which semicolon?
Best regards,
Wolfgang Denk
On Mon, 2011-01-24 at 23:09 +0100, Wolfgang Denk wrote:
Dear Haiying Wang,
In message 1295906076.2051.127.camel@haiying-laptop you wrote:
What I do not understand is what the TPL_BOOT variable in the Makefile is good for. I cannot understand the current use.
Well, it was used to generate the tpl image under tpl/ directory. Maybe TPL_BOOT is a bad name here, I just thought it was too simple to use TPL.
It's not the name. But you use it ina few places here, buth then hard encode "tpl" in a number of other paces there. Which means that you cannot change TPL_BOOT to any other value, or building would break. So why do we need this variable?
It follows the same usage of NAND_SPL.
+$(TPL_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) depend
$(MAKE) -C tpl/board/$(BOARDDIR) allAssume CONFIG_TPL_U_BOOT is not defined, then TPL_BOOT is not defined, and this rule will probably cause a build error, doesn't it?
No, I don't think there is a build error.
WEell, if CONFIG_TPL_U_BOOT is not 'y', then TPL_BOOT is not defined, which results in this make rule:
: $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
i. e. there would be no target name befoe the semicolon.
If TPL_BOOT here is not defined, the reset(after semicolon) will not be executed, just like NAND_SPL and ONENAND_IPL etc.
Sorry, I cannot follow - which reset? which semicolon?
Sorry, :%s/reset/rest/. The semicolon is the same one in your previous comments. I meant the part: : $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
If it is as you thought, then how the platforms will be built without NAND_SPL or ONENAND_IPL is not defined?
Haiying
Dear Haiying Wang,
In message 1295907459.2051.158.camel@haiying-laptop you wrote:
It's not the name. But you use it ina few places here, buth then hard encode "tpl" in a number of other paces there. Which means that you cannot change TPL_BOOT to any other value, or building would break. So why do we need this variable?
It follows the same usage of NAND_SPL.
Ah. I see. Well, so NAND_SPL needs fixing as well :-(
Sorry, I cannot follow - which reset? which semicolon?
Sorry, :%s/reset/rest/. The semicolon is the same one in your previous comments. I meant the part: : $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
Ah, I see what you mean. But that's a colon (':') - a semicolon if a ';'.
If it is as you thought, then how the platforms will be built without NAND_SPL or ONENAND_IPL is not defined?
I have to admit that I wasnot aware that NAND_SPL was usign all teh same constructs. We should clean all this up.
Best regards,
Wolfgang Denk
On Mon, 24 Jan 2011 23:29:45 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Haiying Wang,
In message 1295907459.2051.158.camel@haiying-laptop you wrote:
It's not the name. But you use it ina few places here, buth then hard encode "tpl" in a number of other paces there. Which means that you cannot change TPL_BOOT to any other value, or building would break. So why do we need this variable?
It follows the same usage of NAND_SPL.
Ah. I see. Well, so NAND_SPL needs fixing as well :-(
Agreed, it seems like just hardcoding the target name would work fine, be clearer, and avoid relying on unportable make behavior.
Sorry, I cannot follow - which reset? which semicolon?
Sorry, :%s/reset/rest/. The semicolon is the same one in your previous comments. I meant the part: : $(TIMESTAMP_FILE) $(VERSION_FILE) depend $(MAKE) -C tpl/board/$(BOARDDIR) all
Ah, I see what you mean. But that's a colon (':') - a semicolon if a ';'.
You called it a semicolon first. :-)
-Scott
Dear Wolfgang,
On Mon, 2011-01-24 at 23:29 +0100, Wolfgang Denk wrote:
Dear Haiying Wang,
In message 1295907459.2051.158.camel@haiying-laptop you wrote:
It's not the name. But you use it ina few places here, buth then hard encode "tpl" in a number of other paces there. Which means that you cannot change TPL_BOOT to any other value, or building would break. So why do we need this variable?
It follows the same usage of NAND_SPL.
Ah. I see. Well, so NAND_SPL needs fixing as well :-(
I fixed the NAND_SPL in this way. If it is OK for you, I will generate the patches accordingly for TPL as well. Thanks. BTW, the "./MAKEALL powerpc " worked for my previous patch.
Haiying
From: Haiying Wang Haiying.Wang@freescale.com Date: Wed, 26 Jan 2011 16:03:39 -0500 Subject: [PATCH] Fix NAND_SPL in Makefile
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com --- Makefile | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile index 5f93646..f2c6d65 100644 --- a/Makefile +++ b/Makefile @@ -287,11 +287,6 @@ LDPPFLAGS += \ $(shell $(LD) --version | \ sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p')
-ifeq ($(CONFIG_NAND_U_BOOT),y) -NAND_SPL = nand_spl -U_BOOT_NAND = $(obj)u-boot-nand.bin -endif - ifeq ($(CONFIG_ONENAND_U_BOOT),y) ONENAND_IPL = onenand_ipl U_BOOT_ONENAND = $(obj)u-boot-onenand.bin @@ -320,7 +315,11 @@ BOARD_SIZE_CHECK = endif
# Always append ALL so that arch config.mk's can add custom ones -ALL += $(obj)u-boot.srec $(obj)u-boot.bin $(obj)System.map $(U_BOOT_NAND) $(U_BOOT_ONENAND) +ALL += $(obj)u-boot.srec $(obj)u-boot.bin $(obj)System.map $(U_BOOT_ONENAND) + +ifeq ($(CONFIG_NAND_U_BOOT),y) +ALL += $(obj)u-boot-nand.bin +endif
all: $(ALL)
@@ -401,10 +400,11 @@ $(LDSCRIPT): depend $(obj)u-boot.lds: $(LDSCRIPT) $(CPP) $(CPPFLAGS) $(LDPPFLAGS) -ansi -D__ASSEMBLY__ -P - <$^ >$@
-$(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) depend +$(obj)nand_spl: $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk $(MAKE) -C nand_spl/board/$(BOARDDIR) all
-$(U_BOOT_NAND): $(NAND_SPL) $(obj)u-boot.bin + +$(obj)u-boot-nand.bin: $(obj)nand_spl $(obj)u-boot.bin cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin > $(obj)u-boot-nand.bin
$(ONENAND_IPL): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
Dear Haiying Wang,
In message 1296076056.1985.47.camel@haiying-laptop you wrote:
I fixed the NAND_SPL in this way. If it is OK for you, I will generate the patches accordingly for TPL as well. Thanks. BTW, the "./MAKEALL powerpc " worked for my previous patch.
This looks good to me. Thanks a lot.
Best regards,
Wolfgang Denk
participants (5)
-
Haiying Wang -
Haiying.Wang＠freescale.com
-
Kumar Gala -
Scott Wood -
Wolfgang Denk