All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
@ 2010-08-16  8:04 Haiying Wang
  2010-08-16 10:23 ` Wolfgang Denk
  0 siblings, 1 reply; 7+ messages in thread
From: Haiying Wang @ 2010-08-16  8:04 UTC (permalink / raw)
  To: u-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 <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 +++++++++++
 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

diff --git a/Makefile b/Makefile
index 4f1cb1b..b1d92b7 100644
--- a/Makefile
+++ b/Makefile
@@ -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 \
diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c b/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c
index 8fb27ab..ff09bea 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c
@@ -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 */
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.o
+
+ifdef CONFIG_RAMBOOT_NAND
+COBJS += nand_boot_fsl_elbc.o
+endif
+
+LIBS = $(TOPDIR)/arch/powerpc/cpu/mpc8xxx/ddr/libddr.a
+LIBS += $(TOPDIR)/drivers/i2c/libi2c.a
+
+
+SRCS	:= $(addprefix $(obj),$(SOBJS:.o=.S) $(COBJS:.o=.c))
+OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
+__OBJS	:= $(SOBJS) $(COBJS)
+LNDIR	:= $(OBJTREE)/$(CPUDIR)/sram_boot
+
+sramobj	:= $(OBJTREE)/$(CPUDIR)/
+
+ALL	= $(sramobj)u-boot-sram $(sramobj)u-boot-sram.bin
+
+all:	$(obj).depend $(ALL)
+
+$(sramobj)u-boot-sram.bin: $(sramobj)u-boot-sram
+	$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary $< $@
+
+$(sramobj)u-boot-sram:	$(OBJS) $(LIBS)
+	cd $(LNDIR) && $(LD) $(LDFLAGS) $(__OBJS) $(LIBS) $(PLATFORM_LIBS) \
+		-Map $(sramobj)u-boot-sram.map \
+		-o $(sramobj)u-boot-sram
+
+# create symbolic links for common files
+
+$(obj)cache.c:
+	@rm -f $(obj)cache.c
+	ln -sf $(SRCTREE)/arch/powerpc/lib/cache.c $(obj)cache.c
+
+$(obj)cpu_init_early.c:
+	@rm -f $(obj)cpu_init_early.c
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/cpu_init_early.c $(obj)cpu_init_early.c
+
+$(obj)cpu_init_nand.c:
+	@rm -f $(obj)cpu_init_nand.c
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/cpu_init_nand.c $(obj)cpu_init_nand.c
+
+$(obj)speed.c:
+	@rm -f $(obj)speed.c
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/speed.c $(obj)speed.c
+
+$(obj)fsl_law.c:
+	@rm -f $(obj)fsl_law.c
+	ln -sf $(SRCTREE)/drivers/misc/fsl_law.c $(obj)fsl_law.c
+
+$(obj)law.c:
+	@rm -f $(obj)law.c
+	ln -sf $(SRCTREE)/board/$(BOARDDIR)/law.c $(obj)law.c
+
+$(obj)nand_boot_fsl_elbc.c:
+	@rm -f $(obj)nand_boot_fsl_elbc.c
+	ln -sf $(SRCTREE)/nand_spl/nand_boot_fsl_elbc.c \
+	       $(obj)nand_boot_fsl_elbc.c
+
+$(obj)ns16550.c:
+	@rm -f $(obj)ns16550.c
+	ln -sf $(SRCTREE)/drivers/serial/ns16550.c $(obj)ns16550.c
+
+$(obj)fixed_ivor.S:
+	@rm -f $(obj)fixed_ivor.S
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/fixed_ivor.S $(obj)fixed_ivor.S
+
+$(obj)start.S: $(obj)fixed_ivor.S
+	@rm -f $(obj)start.S
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/start.S $(obj)start.S
+
+$(obj)ticks.S:
+	@rm -f $(obj)ticks.S
+	ln -sf $(SRCTREE)/arch/powerpc/lib/ticks.S $(obj)ticks.S
+
+$(obj)tlb.c:
+	@rm -f $(obj)tlb.c
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/tlb.c $(obj)tlb.c
+
+$(obj)tlb_table.c:
+	@rm -f $(obj)tlb_table.c
+	ln -sf $(SRCTREE)/board/$(BOARDDIR)/tlb.c $(obj)tlb_table.c
+
+$(obj)ddr.c:
+	@rm -f $(obj)ddr.c
+	ln -sf $(SRCTREE)/board/$(BOARDDIR)/ddr.c $(obj)ddr.c
+
+$(obj)time.c:
+	@rm -f $(obj)time.o
+	ln -sf $(SRCTREE)/arch/powerpc/lib/time.c $(obj)time.c
+
+$(obj)time_lib.c:
+	@rm -f $(obj)time_lib.o
+	ln -sf $(SRCTREE)/lib/time.c $(obj)time_lib.c
+
+$(obj)ppcstring.S:
+	@rm -f $(obj)ppcstring.S
+	ln -sf $(SRCTREE)/arch/powerpc/lib/ppcstring.S $(obj)ppcstring.S
+
+$(obj)ddr-gen3.c:
+	@rm -f $(obj)ddr-gen3.c
+	ln -sf $(SRCTREE)/arch/powerpc/cpu/mpc85xx/ddr-gen3.c $(obj)ddr-gen3.c
+
+$(obj)ddr_spd.c:
+	@rm -f $(obj)ddr_spd.c
+	ln -sf $(SRCTREE)/common/ddr_spd.c $(obj)ddr_spd.c
+
+$(obj)ctype.c:
+	@rm -f $(obj)ctype.c
+	ln -sf $(SRCTREE)/lib/ctype.c $(obj)ctype.c
+
+$(obj)div64.c:
+	@rm -f $(obj)div64.c
+	ln -sf $(SRCTREE)/lib/div64.c $(obj)div64.c
+
+$(obj)env_common.c:
+	@rm -f $(obj)env_common.c
+	ln -sf $(SRCTREE)/common/env_common.c $(obj)env_common.c
+
+$(obj)env_nand.c:
+	@rm -f $(obj)env_nand.c
+	ln -sf $(SRCTREE)/common/env_nand.c $(obj)env_nand.c
+
+$(obj)cmd_nvedit.c:
+	@rm -f $(obj)cmd_nvedit.c
+	ln -sf $(SRCTREE)/common/cmd_nvedit.c $(obj)cmd_nvedit.c
+
+$(obj)console.c:
+	@rm -f $(obj)console.c
+	ln -sf $(SRCTREE)/common/console.c $(obj)console.c
+
+$(obj)hwconfig.c:
+	@rm -f $(obj)hwconfig.c
+	ln -sf $(SRCTREE)/common/hwconfig.c $(obj)hwconfig.c
+
+$(obj)string.c:
+	@rm -f $(obj)string.c
+	ln -sf $(SRCTREE)/lib/string.c $(obj)string.c
+
+$(obj)vsprintf.c:
+	@rm -f $(obj)vsprintf.c
+	ln -sf $(SRCTREE)/lib/vsprintf.c $(obj)vsprintf.c
+
+$(obj)display_options.c:
+	@rm -f $(obj)display_options.c
+	ln -sf $(SRCTREE)/lib/display_options.c $(obj)display_options.c
+
+ifneq ($(OBJTREE), $(SRCTREE))
+$(obj)sram_boot.c:
+	@rm -f $(obj)sram_boot.c
+	ln -s $(SRCTREE)/board/freescale/common/sram_boot/sram_boot.c $(obj)sram_boot.c
+endif
+
+#########################################################################
+
+$(obj)%.o:	$(obj)%.S
+	$(CC) $(AFLAGS) -c -o $@ $<
+
+$(obj)%.o:	$(obj)%.c
+	$(CC) $(CFLAGS) -c -o $@ $<
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
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
@@ -0,0 +1,76 @@
+/*
+ * 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.
+ *
+ */
+#include <common.h>
+#include <mpc85xx.h>
+#include <asm/io.h>
+#include <ns16550.h>
+#include <nand.h>
+#include <asm/mmu.h>
+#include <asm/immap_85xx.h>
+#include <asm/fsl_ddr_sdram.h>
+#include <asm/fsl_law.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+const char *board_hwconfig = "foo:bar=baz";
+const char *cpu_hwconfig = "foo:bar=baz";
+
+void board_init_f(ulong bootflag)
+{
+	uint plat_ratio, bus_clk, sys_clk = 0;
+	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
+
+	sys_clk = CONFIG_SYS_CLK_FREQ;
+
+	plat_ratio = in_be32(&gur->porpllsr) & MPC85xx_PORPLLSR_PLAT_RATIO;
+	plat_ratio >>= 1;
+	bus_clk = plat_ratio * sys_clk;
+	get_clocks();
+
+	NS16550_init((NS16550_t)CONFIG_SYS_NS16550_COM1,
+			bus_clk / 16 / CONFIG_BAUDRATE);
+
+	/* load environment */
+#ifdef CONFIG_NAND_U_BOOT
+	nand_load(CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
+				(uchar *)CONFIG_ENV_ADDR);
+#endif
+
+	gd->env_addr  = (ulong)(CONFIG_ENV_ADDR);
+	gd->env_valid = 1;
+
+	/* board specific DDR initialization */
+	gd->ram_size = init_ddr_dram();
+	puts("DRAM:");
+	print_size(gd->ram_size, "");
+
+	puts("\nSRAM boot (middle stage uboot in sram)... ");
+
+	/*
+	 * Load final image to DDR and let it run from there.
+	 */
+#ifdef CONFIG_NAND_U_BOOT
+	nand_boot();
+#endif
+}
+
+void putc(char c)
+{
+	if (c == '\n')
+		NS16550_putc((NS16550_t)CONFIG_SYS_NS16550_COM1, '\r');
+
+	NS16550_putc((NS16550_t)CONFIG_SYS_NS16550_COM1, c);
+}
+
+void puts(const char *str)
+{
+	while (*str)
+		putc(*str++);
+}
diff --git a/arch/powerpc/cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds b/arch/powerpc/cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds
new file mode 100644
index 0000000..ddccfef
--- /dev/null
+++ b/arch/powerpc/cpu/mpc85xx/sram_boot/u-boot-sram-boot.lds
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2009 Freescale Semiconductor, Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+OUTPUT_ARCH(powerpc)
+/* Do we need any of these for elf?
+   __DYNAMIC = 0;    */
+PHDRS
+{
+  text PT_LOAD;
+  bss PT_LOAD;
+}
+
+SECTIONS
+{
+  /* Read-only sections, merged into text segment: */
+  . = + SIZEOF_HEADERS;
+  .interp : { *(.interp) }
+  .hash          : { *(.hash)		}
+  .dynsym        : { *(.dynsym)		}
+  .dynstr        : { *(.dynstr)		}
+  .init          : { *(.init)	}
+  .plt : { *(.plt) }
+  .text      :
+  {
+    *(.text)
+    *(.got1)
+   } :text
+    _etext = .;
+    PROVIDE (etext = .);
+    .rodata    :
+   {
+    *(.eh_frame)
+    *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*)))
+  } :text
+  .fini      : { *(.fini)    } =0
+  .ctors     : { *(.ctors)   }
+  .dtors     : { *(.dtors)   }
+
+  /* Read-write section, merged into data segment: */
+  . = (. + 0x00FF) & 0xFFFFFF00;
+  _erotext = .;
+  PROVIDE (erotext = .);
+
+  .data    :
+  {
+    *(.data)
+    *(.data1)
+    *(.sdata)
+    *(.sdata2)
+    *(.dynamic)
+    CONSTRUCTORS
+  }
+  _edata  =  .;
+  PROVIDE (edata = .);
+
+  . = ALIGN(256);
+  __init_begin = .;
+  .text.init : { *(.text.init) }
+  .data.init : { *(.data.init) }
+  . = ALIGN(256);
+  __init_end = .;
+
+  .bootpg ADDR(.text) - 0x1000 :
+  {
+    start.o	(.bootpg)
+  } :text = 0xffff
+
+  . = ADDR(.text) + 0x80000;
+
+  __bss_start = .;
+  .bss (NOLOAD)       :
+  {
+   *(.sbss) *(.scommon)
+   *(.dynbss)
+   *(.bss)
+   *(COMMON)
+  } :bss
+
+  . = ALIGN(4);
+  _end = . ;
+  PROVIDE (end = .);
+}
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index 3278b10..0b59b58 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -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 */
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
@@ -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 */
diff --git a/common/console.c b/common/console.c
index 7e01886..3dfc8e8 100644
--- a/common/console.c
+++ b/common/console.c
@@ -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 */
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
@@ -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 */
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
@@ -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 */
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
@@ -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 */
diff --git a/lib/string.c b/lib/string.c
index b375b81..255496a 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -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 */
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index aa214dd..f28ee5b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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 */
-- 
1.7.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-16  8:04 [U-Boot] [PATCH 3/7] Add support for SRAM Boot Haiying Wang
@ 2010-08-16 10:23 ` Wolfgang Denk
  2010-08-16 21:03   ` Scott Wood
  2010-08-17  5:46   ` Haiying Wang
  0 siblings, 2 replies; 7+ messages in thread
From: Wolfgang Denk @ 2010-08-16 10:23 UTC (permalink / raw)
  To: u-boot

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

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

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

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-16 10:23 ` Wolfgang Denk
@ 2010-08-16 21:03   ` Scott Wood
  2010-08-17  5:46   ` Haiying Wang
  1 sibling, 0 replies; 7+ messages in thread
From: Scott Wood @ 2010-08-16 21:03 UTC (permalink / raw)
  To: u-boot

On Mon, Aug 16, 2010 at 12:23:56PM +0200, Wolfgang Denk wrote:
> > 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.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-16 10:23 ` Wolfgang Denk
  2010-08-16 21:03   ` Scott Wood
@ 2010-08-17  5:46   ` Haiying Wang
  2010-08-17  9:20     ` Wolfgang Denk
  1 sibling, 1 reply; 7+ messages in thread
From: Haiying Wang @ 2010-08-17  5:46 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-17  5:46   ` Haiying Wang
@ 2010-08-17  9:20     ` Wolfgang Denk
  2010-08-17 18:19       ` Scott Wood
  0 siblings, 1 reply; 7+ messages in thread
From: Wolfgang Denk @ 2010-08-17  9:20 UTC (permalink / raw)
  To: u-boot

Dear Haiying Wang,

In message <1282024011.2814.61.camel@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: 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_

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-17  9:20     ` Wolfgang Denk
@ 2010-08-17 18:19       ` Scott Wood
  2010-08-17 18:54         ` Wolfgang Denk
  0 siblings, 1 reply; 7+ messages in thread
From: Scott Wood @ 2010-08-17 18:19 UTC (permalink / raw)
  To: u-boot

On Tue, 17 Aug 2010 11:20:00 +0200
Wolfgang Denk <wd@denx.de> wrote:

> In message <1282024011.2814.61.camel@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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH 3/7] Add support for SRAM Boot
  2010-08-17 18:19       ` Scott Wood
@ 2010-08-17 18:54         ` Wolfgang Denk
  0 siblings, 0 replies; 7+ messages in thread
From: Wolfgang Denk @ 2010-08-17 18:54 UTC (permalink / raw)
  To: u-boot

Dear Scott Wood,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-08-17 18:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-16  8:04 [U-Boot] [PATCH 3/7] Add support for SRAM Boot Haiying Wang
2010-08-16 10:23 ` Wolfgang Denk
2010-08-16 21:03   ` Scott Wood
2010-08-17  5:46   ` Haiying Wang
2010-08-17  9:20     ` Wolfgang Denk
2010-08-17 18:19       ` Scott Wood
2010-08-17 18:54         ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.