On Mon, 2010-16-08 at 12:23 +0200, Wolfgang Denk wrote:
Dear Haiying Wang,
In message 1281945897.24612.17.camel@localhost.localdomain you wrote:
Once CONFIG_MIDDLE_STAGE_SRAM_BOOT is defined, CONFIG_SRAM_BOOT is enabled to generate u-boot-sram.bin which will run in the l2/l3 sram. This middle stage uboot will init ddr sdram with ddr spd code and load the final uboot image to ddr and start from there. It is useful for the silicons which have small l2/l3 size under the two scenarios:
- NAND boot. The 4k NAND SPL uboot can not enable the spd ddr code to
initialize the ddr because of the 4k size limitation, and the l2/l3 as SRAM also is not large enough to acoommodate the final uboot image. 2. SD/eSPI boot. we don't want to statically init ddr in SD/eSPI's configuration part, but l2/l3 as SRAM is small for final uboot.
The concept may be useful for other situations as well, so we should try and make this as generic as possible.
First, the name CONFIG_MIDDLE_STAGE_SRAM_BOOT is too long and too specific to your case. Please use a more generic name, for example CONFIG_SYS_2ND_STAGE_BOOT or similar (I don't think this is a user configurable option, hence the CONFIG_SYS_)
OK. will rename it.
This patch has nand boot support, SD/eSPI support will be submited later.
Because ddr spd code calls some functions defined the files in common/ and lib/,#ifndef CONFIG_SRAM_BOOT is used in those files to keep the sram uboot size as small as possible.
Line too long.
will fix it.
Signed-off-by: Haiying Wang Haiying.Wang@freescale.com
Makefile | 18 ++- arch/powerpc/cpu/mpc85xx/cpu_init_nand.c | 31 +++- arch/powerpc/cpu/mpc85xx/sram_boot/Makefile | 190 ++++++++++++++++++++ arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c | 76 ++++++++ .../cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds | 101 +++++++++++
The code for this should not live in some specific 85xx directory, but instead be generalized similar to what we have with nand_spl.
Should we let it structured as $(TOPDIR)/sram_boot/board/freescale....? At least current, the above code is mostly only used for 85xx. The only common part I can tell is the changes in Makefile.
...
--- a/Makefile +++ b/Makefile
...
+$(SRAM_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
$(MAKE) -C $(CPUDIR)/sram_boot all+$(U_BOOT_NAND_SRAM): $(NAND_SPL) $(SRAM_BOOT) $(obj)u-boot.bin
cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)$(CPUDIR)/u-boot-sram.bin $(obj)u-boot.bin > $(obj)u-boot-nand.binWe really need bette rnames here, too.
Does SRAM_BOOT/sram_boot sound bad? :)
...
diff --git a/arch/powerpc/cpu/mpc85xx/sram_boot/Makefile b/arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file mode 100644 index 0000000..7c86095 --- /dev/null +++ b/arch/powerpc/cpu/mpc85xx/sram_boot/Makefile @@ -0,0 +1,190 @@ +# +# Copyright (C) 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 as published by the Free +# Software Foundation; either version 2 of the License, or (at your option) +# any later version. +#
+SRAM_BOOT := y
+include $(TOPDIR)/config.mk
+LDSCRIPT= $(TOPDIR)/$(CPUDIR)/sram_boot/u-boot-sram-boot.lds +LDFLAGS = -Bstatic -T $(LDSCRIPT) -Ttext $(TEXT_BASE) $(PLATFORM_LDFLAGS) +AFLAGS += -DCONFIG_SRAM_BOOT +CFLAGS += -DCONFIG_SRAM_BOOT
+SOBJS = start.o ticks.o ppcstring.o +COBJS = cache.o cpu_init_early.o cpu_init_nand.o fsl_law.o law.o speed.o \
sram_boot.o ns16550.o tlb.o tlb_table.o string.o hwconfig.o ddr.o \time.o time_lib.o ddr-gen3.o ddr_spd.o ctype.o div64.o console.o \cmd_nvedit.o env_common.o env_nand.o vsprintf.o display_options.oYou do not want to duplicate all this stuff here.
This makes no sense. Also, it is unmaintainable.
For this case, I need to call some functions like getenv, hwconfig, printf, strcmp etc. which are needed in ddr spd code, but I don't want to link the libs for those file because if so, the 2nd stage uboot will be larger. It might also not be a good idea to copy all those functions into some new files which are really duplicate. I agree it is unmaintainable here. As Scott pointed, we need to find a better way. Any suggestion?
diff --git a/arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c b/arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file mode 100644 index 0000000..7b90eee --- /dev/null +++ b/arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c
...
+const char *board_hwconfig = "foo:bar=baz"; +const char *cpu_hwconfig = "foo:bar=baz";
This does not exactly look like useful values to me.
The only use is to make board_hwconfig and cpu_hwconfig from sbss to sdata section.
Please fix!
The problem here is: The commit 79e4e6480b359cb28129cecfa2cae0ef9cccf803: "powerpc/8xxx: Enabled hwconfig for memory interleaving" makes changes as: - if ((p = getenv("memctl_intlv_ctl")) != NULL) { + if (hwconfig_sub("fsl_ddr", "ctlr_intlv")) {
Thus the hwconfig is called before ddr initialized, and the system hangs at " if (board_hwconfig) return hwconfig_parse(board_hwconfig, strlen(board_hwconfig), opt, ";", ':', arglen); " in common/hwconfig.c. I did not get it yet, but just copied above code from mpc8641hpcn.c to make the system boot up.
+void board_init_f(ulong bootflag) +{
- uint plat_ratio, bus_clk, sys_clk = 0;
- ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
...
+void putc(char c) +{
...
+void puts(const char *str) +{
Argh...
Please do not reimplement everything. This will result in a terribke mess of unmaintainable code.
Sorry, will fix it.
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index fd5320d..af24491 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c
...
diff --git a/common/console.c b/common/console.c index 7e01886..3dfc8e8 100644 --- a/common/console.c +++ b/common/console.c
...
diff --git a/common/env_common.c b/common/env_common.c index 460309b..e04a985 100644 --- a/common/env_common.c +++ b/common/env_common.c
...
diff --git a/common/env_nand.c b/common/env_nand.c index a5e1038..0f7b83c 100644 --- a/common/env_nand.c +++ b/common/env_nand.c
...
diff --git a/lib/display_options.c b/lib/display_options.c index 20319e6..240ad62 100644 --- a/lib/display_options.c +++ b/lib/display_options.c
...
+#endif /* !CONFIG_SRAM_BOOT */ diff --git a/lib/string.c b/lib/string.c index b375b81..255496a 100644 --- a/lib/string.c +++ b/lib/string.c
...
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index aa214dd..f28ee5b 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c
Full NAK to all these modifications of common code. This is not acceptable.
Again, if those are not acceptable, do you have any suggestion on how to pick the code for the functions I need to use in sram boot?
Thanks.
Haiying