Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
On Mon, 2010-16-08 at 12:23 +0200, Wolfgang Denk wrote: Dear Haiying Wang, In message 1281945897.24612.17.ca...@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: 1. 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.w...@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.bin We 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 000..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.o You 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 000..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.
Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
Dear Haiying Wang,
In message 1282024011.2814.61.ca...@localhost.localdomain you wrote:
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.
Not sram_boot, please. The fact that you are using SRAM is specific to
your situation only, but not to the problem.
Assume you want to boot from NAND, and for reliability purposes the
U-Boot image is stored in a UBI partition. The initial NAND bootloader
(1st stage) does not allow to add UBI support, so we will probably
have to make it just load the whole first (guaranteed to be error
free) block into RAM, which then contains full UBI support (but still
not the complete U-Boot image). This 2nd stage loder will then load
the real U-Boot from an UBI partition.
This is a completely different use case, but the basic operation is
the same as in your case.
Please implement your code such that we can re-use it for such other
use cases as well.
+$(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.bin
We really need bette rnames here, too.
Does SRAM_BOOT/sram_boot sound bad? :)
Yes, it does. This has actually nothing to do with SRAM.
You 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
I think this is a wrong approach. Instead, you should free the DDR
code from such calls.
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?
Yes: remove the need for these functions.
+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.
It is probably abad concept to depend on such variables that early in
the code, especially when there is SPD information?
In any way, foo:bar=baz does not make any sense.
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?
Change the approach. If you cannot afford the code size for these
funtions, then do not use them.
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: w...@denx.de
A Chairman was as necessary to a Board planet as the zero was in
mathematics, but being a zero had big disadvantages...
- Terry Pratchett, _The Dark Side of the Sun_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
On Tue, 17 Aug 2010 11:20:00 +0200 Wolfgang Denk w...@denx.de wrote: In message 1282024011.2814.61.ca...@localhost.localdomain you wrote: 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 I think this is a wrong approach. Instead, you should free the DDR code from such calls. They're there for a reason, and space isn't so tight that we need to drop console output and other features at this level. In this specific case, we probably have room to suck in the full implementations of printf and the string functions (I believe Haiying is currently creating a 48K middle stage, while SRAM on this chip is 256K), so I think we could scale back on the intrusiveness of those changes by letting the middle stage grow a bit... 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? Change the approach. If you cannot afford the code size for these funtions, then do not use them. ...but take this entire subsystem as is or go without anything vaguely resembling this code, lest it be called 'duplication' is a rather limiting pair of choices. It seems reasonable to refactor things to be more modular (possibly in a nicer way than sprinkling ifdefs), or provide an alternate trimmed-down implementation. One thing that should probably be tried first, though, is -ffunction-sections and --gc-sections, to have the linker discard any functions that aren't referenced. It seems some arches/boards already use this. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
Dear Scott Wood, In message 20100817131904.5703f...@schlenkerla.am.freescale.net you wrote: ...but take this entire subsystem as is or go without anything vaguely resembling this code, lest it be called 'duplication' is a rather limiting pair of choices. It seems reasonable to refactor things to be more modular (possibly in a nicer way than sprinkling ifdefs), or provide an alternate trimmed-down implementation. Agreed. Factoring out functions that can then be selected on Makefile level is probably OK. One thing that should probably be tried first, though, is -ffunction-sections and --gc-sections, to have the linker discard any functions that aren't referenced. It seems some arches/boards already use this. I guess that should help here a lot, too. 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: w...@denx.de A day without sunshine is like night. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
Dear Haiying Wang,
In message 1281945897.24612.17.ca...@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:
1. 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_)
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.
Signed-off-by: Haiying Wang haiying.w...@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.
...
--- 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.bin
We really need bette rnames here, too.
...
diff --git a/arch/powerpc/cpu/mpc85xx/sram_boot/Makefile
b/arch/powerpc/cpu/mpc85xx/sram_boot/Makefile
new file mode 100644
index 000..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.o
You do not want to duplicate all this stuff here.
This makes no sense. Also, it is unmaintainable.
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 000..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.
Please fix!
+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.
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
Re: [U-Boot] [PATCH 3/7] Add support for SRAM Boot
On Mon, Aug 16, 2010 at 12:23:56PM +0200, Wolfgang Denk wrote:
Signed-off-by: Haiying Wang haiying.w...@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.
With NAND there's some common code (or at least a few common alternatives,
that aren't tied to a specific cpu or board) to actually do the NAND
loading. I'm not sure if there would be anything common here -- it's just
cutting size down, plus storage-specific code to load the final image.
+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.o
You do not want to duplicate all this stuff here.
This makes no sense. Also, it is unmaintainable.
Here you say not to duplicate things (it's not actually duplicating the
code, just the list of objects, just like nand_spl does), but later you
say not to modify what already exists (which is also already done for
nand_spl in some places). We need to cut down the image size some way or
another. :-)
We're open to suggestions on how to better structure this kind of thing for
maintainability; the current approaches certainly aren't pretty. But we
need to do *something*.
+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.
These are tiny alternatives that are used in nand_spl for some boards (a
whopping 13 lines to squeeze console output in a 4K image, not the end of
the world) -- but for the SRAM phase we should have plenty of room for the
normal console implementation.
-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
