Add support for SRAM Boot

PATCH New Haiying Wang 3 participants 7 messages Aug 16, 2010 – Aug 17, 2010

Subsystems (5)

PowerPC Kbuild Library code Vsprintf +1 more subsystems

lore

Share & tools

lore

Haiying Wang

author

2010-08-16 08:04 UTC #0 lore raw

PATCH 3/7 Add support for SRAM Boot

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.

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.

Signed-off-by Haiying Wang <[email protected]>

---
 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 +++++++++++
 arch/powerpc/cpu/mpc85xx/start.S                   |    8 +-
 common/cmd_nvedit.c                                |    8 +-
 common/console.c                                   |    4 +
 common/env_common.c                                |    4 +
 common/env_nand.c                                  |    3 +-
 lib/display_options.c                              |    2 +
 lib/string.c                                       |   10 +
 lib/vsprintf.c                                     |    2 +
 13 files changed, 450 insertions(+), 7 deletions(-)
 create mode 100644 arch/powerpc/cpu/mpc85xx/sram_boot/Makefile
 create mode 100644 arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c
 create mode 100644 arch/powerpc/cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds

Makefile +17 -1

index 4f1cb1b..b1d92b7 100644
@@ -280,10 +280,18 @@ 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_MIDDLE_STAGE_SRAM_BOOT),y)
+SRAM_BOOT = sram_boot
+endif
+
 ifeq ($(CONFIG_NAND_U_BOOT),y)
 NAND_SPL = nand_spl
+ifeq ($(CONFIG_MIDDLE_STAGE_SRAM_BOOT),y)
+U_BOOT_NAND_SRAM = $(obj)u-boot-nand.bin
+else
 U_BOOT_NAND = $(obj)u-boot-nand.bin
 endif
+endif
 
 ifeq ($(CONFIG_ONENAND_U_BOOT),y)
 ONENAND_IPL = onenand_ipl

@@ -298,7 +306,7 @@ __LIBS := $(subst $(obj),,$(LIBS)) $(subst $(obj),,$(LIBBOARD))
 #########################################################################
 
 # 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_NAND) $(U_BOOT_ONENAND) $(U_BOOT_NAND_SRAM)
 
 all:		$(ALL)

@@ -382,6 +390,12 @@ $(NAND_SPL):	$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
 $(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
 
+$(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
+
 $(ONENAND_IPL):	$(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
 		$(MAKE) -C onenand_ipl/board/$(BOARDDIR) all

@@ -2459,6 +2473,7 @@ clean:
 	@rm -f $(obj)include/bmp_logo.h
 	@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)$(CPUDIR)/{u-boot-sram,u-boot-sram.map}
 	@rm -f $(ONENAND_BIN)
 	@rm -f $(obj)onenand_ipl/u-boot.lds
 	@rm -f $(TIMESTAMP_FILE) $(VERSION_FILE)

@@ -2482,6 +2497,7 @@ clobber:	clean
 	@rm -f $(obj)include/asm/proc $(obj)include/asm/arch $(obj)include/asm
 	@[ ! -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)board/freescale ] || find $(obj)board/freescale -name "*" -type l -print | xargs rm -f
 
 ifeq ($(OBJTREE),$(SRCTREE))
 mrproper \

arch/powerpc/cpu/mpc85xx/cpu_init_nand.c +30 -1

index 8fb27ab..ff09bea 100644
@@ -40,7 +40,8 @@ void cpu_init_f(void)
 #error  CONFIG_NAND_BR_PRELIM, CONFIG_NAND_OR_PRELIM must be defined
 #endif
 
-#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L2_ADDR)
+#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L2_ADDR) \
+	&& !defined(CONFIG_SRAM_BOOT)
 	ccsr_l2cache_t *l2cache = (void *)CONFIG_SYS_MPC85xx_L2_ADDR;
 	char *l2srbar;
 	int i;

@@ -60,4 +61,32 @@ void cpu_init_f(void)
 	for (i = 0; i < CONFIG_SYS_L2_SIZE; i++)
 		l2srbar[i] = 0;
 #endif
+#ifdef CONFIG_SRAM_BOOT
+	init_used_tlb_cams();
+#endif
+}
+
+#ifdef CONFIG_SRAM_BOOT
+/* Because the primary cpu's info is enough in SRAM BOOT stage  we define the
+ * cpu number to 1 so as to keep code size for sram boot uboot as small as
+ * possible.
+ */
+int cpu_numcores()
+{
+	return 1;
+}
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * Get timebase clock frequency
+ */
+unsigned long get_tbclk(void)
+{
+#ifdef CONFIG_FSL_CORENET
+	return (gd->bus_clk + 8) / 16;
+#else
+	return (gd->bus_clk + 4UL)/8UL;
+#endif
 }
+#endif /* CONFIG_SRAM_BOOT */

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +190 -0

190 lines not on this page · raw message

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +76 -0

76 lines not on this page · raw message

arch/powerpc/cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds new file +101 -0

101 lines not on this page · raw message

arch/powerpc/cpu/mpc85xx/start.S +6 -2

index 3278b10..0b59b58 100644
@@ -54,6 +54,7 @@
  * Use r12 to access the GOT
  */
 	START_GOT
+#ifndef CONFIG_SRAM_BOOT
 	GOT_ENTRY(_GOT2_TABLE_)
 	GOT_ENTRY(_FIXUP_TABLE_)

@@ -63,6 +64,7 @@
 	GOT_ENTRY(_end_of_vectors)
 	GOT_ENTRY(transfer_to_handler)
 #endif
+#endif /* !CONFIG_SRAM_BOOT */
 
 	GOT_ENTRY(__init_end)
 	GOT_ENTRY(_end)

@@ -432,7 +434,7 @@ _start_cont:
 	bl	board_init_f
 	isync
 
-#ifndef CONFIG_NAND_SPL
+#if !defined(CONFIG_NAND_SPL) && !defined(CONFIG_SRAM_BOOT)
 	. = EXC_OFF_SYS_RESET
 	.globl	_start_of_vectors
 _start_of_vectors:

@@ -874,7 +876,7 @@ in32:
 in32r:
 	lwbrx	r3,r0,r3
 	blr
-#endif  /* !CONFIG_NAND_SPL */
+#endif  /* !CONFIG_NAND_SPL || !CONFIG_SRAM_BOOT*/
 
 /*------------------------------------------------------------------------------*/

@@ -900,6 +902,7 @@ write_tlb:
 	isync
 	blr
 
+#ifndef CONFIG_SRAM_BOOT
 /*
  * void relocate_code (addr_sp, gd, addr_moni)
  *

@@ -1203,3 +1206,4 @@ setup_ivors:
 #include "fixed_ivor.S"
 	blr
 #endif /* !CONFIG_NAND_SPL */
+#endif /* !CONFIG_SRAM_BOOT */

common/cmd_nvedit.c +6 -2

index fd5320d..af24491 100644
@@ -80,6 +80,7 @@ SPI_FLASH|MG_DISK|NVRAM|NOWHERE}
 static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
 #define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
 
+#ifndef CONFIG_SRAM_BOOT
 /*
  * This variable is incremented on each do_setenv (), so it can
  * be used via get_env_id() as an indication, if the environment

@@ -514,6 +515,7 @@ int do_editenv(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return setenv(argv[1], buffer);
 }
 #endif /* CONFIG_CMD_EDITENV */
+#endif /* !CONFIG_SRAM_BOOT */
 
 /************************************************************************
  * Look up variable from environment,

@@ -543,6 +545,7 @@ char *getenv (char *name)
 	return (NULL);
 }
 
+#ifndef CONFIG_SRAM_BOOT
 int getenv_f(char *name, char *buf, unsigned len)
 {
 	int i, nxt;

@@ -592,7 +595,7 @@ U_BOOT_CMD(
 );
 
 #endif
-
+#endif /* !CONFIG_SRAM_BOOT */
 
 /************************************************************************
  * Match a name / name=value pair

@@ -613,7 +616,7 @@ int envmatch (uchar *s1, int i2)
 	return(-1);
 }
 
-
+#ifndef CONFIG_SRAM_BOOT
 /**************************************************/
 
 #if defined(CONFIG_CMD_EDITENV)

@@ -668,3 +671,4 @@ U_BOOT_CMD(
 	"    - run the commands in the environment variable(s) 'var'"
 );
 #endif
+#endif /* !CONFIG_SRAM_BOOT */

common/console.c +4 -0

index 7e01886..3dfc8e8 100644
@@ -29,6 +29,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifndef CONFIG_SRAM_BOOT
 #ifdef CONFIG_SYS_CONSOLE_IS_IN_ENV
 /*
  * if overwrite_console returns 1, the stdin, stderr and stdout

@@ -364,6 +365,7 @@ void puts(const char *s)
 		serial_puts(s);
 	}
 }
+#endif /* !CONFIG_SRAM_BOOT */
 
 int printf(const char *fmt, ...)
 {

@@ -384,6 +386,7 @@ int printf(const char *fmt, ...)
 	return i;
 }
 
+#ifndef CONFIG_SRAM_BOOT
 int vprintf(const char *fmt, va_list args)
 {
 	uint i;

@@ -720,3 +723,4 @@ int console_init_r(void)
 }
 
 #endif /* CONFIG_SYS_CONSOLE_IS_IN_ENV */
+#endif /* !CONFIG_SRAM_BOOT */

common/env_common.c +4 -0

index 460309b..e04a985 100644
@@ -134,10 +134,12 @@ uchar default_environment[] = {
 	"\0"
 };
 
+#ifndef CONFIG_SRAM_BOOT
 void env_crc_update (void)
 {
 	env_ptr->crc = crc32(0, env_ptr->data, ENV_SIZE);
 }
+#endif /* !CONFIG_SRAM_BOOT */
 
 static uchar env_get_char_init (int index)
 {

@@ -185,6 +187,7 @@ uchar *env_get_addr (int index)
 	}
 }
 
+#ifndef CONFIG_SRAM_BOOT
 void set_default_env(void)
 {
 	if (sizeof(default_environment) > ENV_SIZE) {

@@ -281,3 +284,4 @@ int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf)
 	return found;
 }
 #endif
+#endif /* !CONFIG_SRAM_BOOT */

common/env_nand.c +2 -1

index a5e1038..0f7b83c 100644
@@ -82,7 +82,7 @@ uchar env_get_char_spec (int index)
 	return ( *((uchar *)(gd->env_addr + index)) );
 }
 
-
+#ifndef CONFIG_SRAM_BOOT
 /* this is called before nand_init()
  * so we can't read Nand to validate env data.
  * Mark it OK for now. env_relocate() in env_common.c

@@ -414,3 +414,4 @@ static void use_default()
 	set_default_env();
 }
 #endif
+#endif /* !CONFIG_SRAM_BOOT */

lib/display_options.c +2 -0

index 20319e6..240ad62 100644
@@ -84,6 +84,7 @@ void print_size(unsigned long long size, const char *s)
 	printf (" %ciB%s", c, s);
 }
 
+#ifndef CONFIG_SRAM_BOOT
 /*
  * Print data buffer in hex and ascii form to the terminal.
  *

@@ -149,3 +150,4 @@ int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen)
 
 	return 0;
 }
+#endif /* !CONFIG_SRAM_BOOT */

lib/string.c +10 -0

index b375b81..255496a 100644
@@ -54,6 +54,7 @@ int strnicmp(const char *s1, const char *s2, size_t len)
 }
 #endif
 
+#ifndef CONFIG_SRAM_BOOT
 char * ___strtok;
 
 #ifndef __HAVE_ARCH_STRCPY

@@ -160,6 +161,7 @@ int strcmp(const char * cs,const char * ct)
 	return __res;
 }
 #endif
+#endif /* !CONFIG_SRAM_BOOT */
 
 #ifndef __HAVE_ARCH_STRNCMP
 /**

@@ -182,6 +184,7 @@ int strncmp(const char * cs,const char * ct,size_t count)
 }
 #endif
 
+#ifndef CONFIG_SRAM_BOOT
 #ifndef __HAVE_ARCH_STRCHR
 /**
  * strchr - Find the first occurrence of a character in a string

@@ -228,6 +231,7 @@ size_t strlen(const char * s)
 	return sc - s;
 }
 #endif
+#endif /* !CONFIG_SRAM_BOOT */
 
 #ifndef __HAVE_ARCH_STRNLEN
 /**

@@ -245,6 +249,7 @@ size_t strnlen(const char * s, size_t count)
 }
 #endif
 
+#ifndef CONFIG_SRAM_BOOT
 #ifndef __HAVE_ARCH_STRDUP
 char * strdup(const char *s)
 {

@@ -286,6 +291,7 @@ size_t strspn(const char *s, const char *accept)
 	return count;
 }
 #endif
+#endif /* !CONFIG_SRAM_BOOT */
 
 #ifndef __HAVE_ARCH_STRPBRK
 /**

@@ -307,6 +313,7 @@ char * strpbrk(const char * cs,const char * ct)
 }
 #endif
 
+#ifndef CONFIG_SRAM_BOOT
 #ifndef __HAVE_ARCH_STRTOK
 /**
  * strtok - Split a string into tokens

@@ -556,6 +563,7 @@ void * memscan(void * addr, int c, size_t size)
 	return (void *) p;
 }
 #endif
+#endif /* CONFIG_SRAM_BOOT */
 
 #ifndef __HAVE_ARCH_STRSTR
 /**

@@ -581,6 +589,7 @@ char * strstr(const char * s1,const char * s2)
 }
 #endif
 
+#ifndef CONFIG_SRAM_BOOT
 #ifndef __HAVE_ARCH_MEMCHR
 /**
  * memchr - Find a character in an area of memory.

@@ -603,3 +612,4 @@ void *memchr(const void *s, int c, size_t n)
 }
 
 #endif
+#endif /* !CONFIG_SRAM_BOOT */

lib/vsprintf.c +2 -0

index aa214dd..f28ee5b 100644
@@ -665,6 +665,7 @@ int sprintf(char * buf, const char *fmt, ...)
 	return i;
 }
 
+#ifndef CONFIG_SRAM_BOOT
 void panic(const char *fmt, ...)
 {
 	va_list	args;

@@ -679,3 +680,4 @@ void panic(const char *fmt, ...)
 	do_reset (NULL, 0, 0, NULL);
 #endif
 }
+#endif /* !CONFIG_SRAM_BOOT */

Signature · 1 line

1.7.0

Wolfgang Denk

2010-08-16 10:23 UTC #1 lore raw

PATCH 3/7 Add support for SRAM Boot

Dear Haiying Wang,

Quoted 10 lines · In message <[email protected]> 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_)

Quoted 4 lines

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.

Quoted 7 lines

Signed-off-by Haiying Wang <[email protected]>

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

...

Quoted 2 lines

Makefile +0 -0

...

Makefile $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk · line 397

Quoted 5 lines

+$(SRAM_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk

Makefile +4 -0

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

...

Quoted 19 lines

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +13 -0

new file mode 100644
index 0000000..7c86095
@@ -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
+

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile line 23

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +10 -0

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

Quoted 5 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +0 -0
new file mode 100644
index 0000000..7b90eee

...

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 23

Quoted 2 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+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!

Quoted 4 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +4 -0

+void board_init_f(ulong bootflag)
+{
+	uint plat_ratio, bus_clk, sys_clk = 0;
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);

...

Quoted 2 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+void putc(char c)
+{

...

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 73

Quoted 2 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+void puts(const char *str)
+{

Argh...

Please do not reimplement everything. This will result in a terribke mess of unmaintainable code.

Quoted 4 lines

common/cmd_nvedit.c +0 -0
index fd5320d..af24491 100644

...

Quoted 4 lines

common/console.c +0 -0
index 7e01886..3dfc8e8 100644

...

Quoted 4 lines

common/env_common.c +0 -0
index 460309b..e04a985 100644

...

Quoted 4 lines

common/env_nand.c +0 -0
index a5e1038..0f7b83c 100644

...

Quoted 4 lines

lib/display_options.c +0 -0
index 20319e6..240ad62 100644

...

Quoted 5 lines

lib/display_options.c +1 -0
+#endif /* !CONFIG_SRAM_BOOT */
lib/string.c +0 -0
index b375b81..255496a 100644

...

lib/vsprintf.c

Quoted 4 lines

lib/vsprintf.c +0 -0
index aa214dd..f28ee5b 100644

Full NAK to all these modifications of common code. This is not acceptable.

Best regards,

Wolfgang Denk

Signature · 6 lines

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: wd at denx.de
If it happens once, it's a bug.
If it happens twice, it's a feature.
If it happens more than twice, it's a design philosophy.

Scott Wood

2010-08-16 21:03 UTC #2 lore raw

PATCH 3/7 Add support for SRAM Boot

Quoted 10 lines · Wolfgang Denk

Quoted 7 lines
Signed-off-by Haiying Wang <[email protected]>
---
 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.

Quoted 9 lines

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile line 23
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +5 -0
+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*.

Quoted 8 lines

Quoted 4 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +4 -0
+void board_init_f(ulong bootflag)
+{
+	uint plat_ratio, bus_clk, sys_clk = 0;
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
...
Quoted 2 lines
(quoted diff) +2 -0
+void putc(char c)
+{
...

Quoted 7 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 73
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+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

Haiying Wang

author

2010-08-17 05:46 UTC #3 lore raw

PATCH 3/7 Add support for SRAM Boot

In reply to Wolfgang Denk

Quoted 14 lines · Wolfgang Denk

Dear Haiying Wang,
Quoted 10 lines · In message <[email protected]> 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.

Quoted 7 lines

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.

Quoted 6 lines

Quoted 4 lines
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.

Quoted 10 lines

Quoted 7 lines
Signed-off-by Haiying Wang <[email protected]>
---
 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.

Quoted 11 lines

...
Quoted 2 lines
Makefile +0 -0
...
Makefile $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk · line 397
Quoted 5 lines
+$(SRAM_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
Makefile +4 -0
+		$(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? :)

Quoted 33 lines

...

Quoted 19 lines

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +13 -0

new file mode 100644
index 0000000..7c86095
@@ -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
+

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile line 23

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/sram_boot/Makefile new file +10 -0

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

Quoted 1 line

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?

Quoted 11 lines

Quoted 6 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +0 -0
new file mode 100644
index 0000000..7b90eee
...
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 23
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+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.

Quoted 1 line

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.

Quoted 8 lines

Quoted 4 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +4 -0
+void board_init_f(ulong bootflag)
+{
+	uint plat_ratio, bus_clk, sys_clk = 0;
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
...
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+void putc(char c)
+{
...

Quoted 7 lines

arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 73
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+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.

Quoted 32 lines

Quoted 5 lines
common/cmd_nvedit.c +0 -0
index fd5320d..af24491 100644
...
Quoted 4 lines
common/console.c +0 -0
index 7e01886..3dfc8e8 100644
...
Quoted 4 lines
common/env_common.c +0 -0
index 460309b..e04a985 100644
...
Quoted 4 lines
common/env_nand.c +0 -0
index a5e1038..0f7b83c 100644
...
Quoted 4 lines
lib/display_options.c +0 -0
index 20319e6..240ad62 100644
...
Quoted 5 lines
lib/display_options.c +1 -0
+#endif /* !CONFIG_SRAM_BOOT */
lib/string.c +0 -0
index b375b81..255496a 100644
...

Quoted 8 lines

lib/vsprintf.c
Quoted 4 lines
lib/vsprintf.c +0 -0
index aa214dd..f28ee5b 100644
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

Wolfgang Denk

2010-08-17 09:20 UTC #4 lore raw

PATCH 3/7 Add support for SRAM Boot

Dear Haiying Wang,

Quoted 7 lines · In message <[email protected]> you wrote:

Quoted 7 lines

Quoted 6 lines

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

Quoted 5 lines

Quoted 2 lines
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.

Quoted 8 lines

Quoted 7 lines
Makefile $(NAND_SPL): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk · line 397
Quoted 5 lines
+$(SRAM_BOOT): $(TIMESTAMP_FILE) $(VERSION_FILE) $(obj)include/autoconf.mk
Makefile +4 -0
+		$(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.

Quoted 5 lines

Quoted 3 lines
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.

Quoted 5 lines

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.

Quoted 14 lines

Quoted 4 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c line 23
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/sram_boot/sram_boot.c new file +2 -0
+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.
Quoted 1 line
Please fix!
The problem here is:
The commit 79e4e6480b359cb28129cecfa2cae0ef9cccf803:
"powerpc/8xxx: Enabled hwconfig for memory interleaving"
makes changes as:
(quoted diff) +1 -1
-       if ((p = getenv("memctl_intlv_ctl")) != NULL) {
+       if (hwconfig_sub("fsl_ddr", "ctlr_intlv")) {

Quoted 8 lines

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.

Quoted 2 lines

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

Signature · 6 lines

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: wd at 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_

Scott Wood

2010-08-17 18:19 UTC #5 lore raw

PATCH 3/7 Add support for SRAM Boot

Quoted 6 lines · On Tue, 17 Aug 2010 11:20:00 +0200 Wolfgang Denk <[email protected]> wrote:

Quoted 2 lines · In message <[email protected]> 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...

Quoted 5 lines

Quoted 2 lines
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

Wolfgang Denk

2010-08-17 18:54 UTC #6 lore raw

PATCH 3/7 Add support for SRAM Boot

Dear Scott Wood,

Quoted 6 lines · In message <[email protected]> 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.

Quoted 4 lines

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

Signature · 4 lines

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: wd at denx.de
A day without sunshine is like night.