[U-Boot] [PATCH v6 0/6] Clean out SPL and enable driver model

[U-Boot] [PATCH v6 0/6] Clean out SPL and enable driver model

[Simon Glass]

This series includes a few more patches aimed at getting rid of gdata, the
parallel global_data structure introduced on some ARM boards.

It also collects together the other patches which have been sent previously,
so that everything is in one place.

I would like get this agreed and applied to u-boot-dm this week if possible
so that we will at last have driver model available in SPL.

This series is available at u-boot-dm/spl-working.

Changes in v6:
- Make changes only when CONFIG_DM is defined, just in case
- Add new patch for TI boards to avoid gdata

Changes in v5:
- Rebase to master

Changes in v4:
- Squash the gdata revert patch into this one
- Allow gdata to survive unless CONFIG_DM is defined, for now
- Rebase on top of dm/master
- Adjust README to mention that lowlevel_init() should have no stack
- Add new patch to make the export interface support CONFIG_SYS_MALLOC_SIMPLE

Changes in v3:
- Drop extra blank line

Changes in v2:
- Remove gdata definition from ARM spl header file also
- Rebase on top of the SPI series
- Move docs to top-level README file and expand them to cover U-Boot proper
- Add Kconfig settings

Simon Glass (6):
  arm: Reduce the scope of lowlevel_init()
  arm: spl: Avoid setting up a duplicate global data structure
  dm: tegra: Enable driver model in SPL and adjust the GPIO driver
  arm: spl: Allow board_init_r() to run with a larger stack
  Make export interface support CONFIG_SYS_MALLOC_SIMPLE
  ti: armv7: am33xx: Move SPL SDRAM into the correct place

 Kconfig                            | 18 ++++++++++
 README                             | 69 ++++++++++++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/am33xx/board.c  | 13 ++++---
 arch/arm/cpu/armv7/lowlevel_init.S | 22 ++++++++++--
 arch/arm/include/asm/spl.h         |  2 ++
 arch/arm/lib/crt0.S                | 13 +++++--
 arch/arm/lib/spl.c                 |  4 +++
 arch/arm/mach-tegra/Kconfig        |  3 ++
 board/nvidia/seaboard/seaboard.c   |  4 +--
 common/spl/spl.c                   | 35 +++++++++++++++++++
 drivers/gpio/Makefile              |  4 +++
 drivers/gpio/tegra_gpio.c          | 27 ++++++---------
 include/_exports.h                 |  2 ++
 include/configs/tegra-common.h     |  6 ----
 include/configs/ti_armv7_common.h  |  3 +-
 include/exports.h                  |  2 ++
 16 files changed, 192 insertions(+), 35 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 1/6] arm: Reduce the scope of lowlevel_init()

[Simon Glass]

This function has grown into something of a monster. Some boards are setting
up a console and DRAM here in SPL. This requires global_data which should be
set up in one place (crt0.S).

There is no need for SPL to use s_init() for anything since board_init_f()
is called immediately afterwards.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6:
- Make changes only when CONFIG_DM is defined, just in case

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/cpu/armv7/lowlevel_init.S | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/lowlevel_init.S b/arch/arm/cpu/armv7/lowlevel_init.S
index f1aea05..427b0b1 100644
--- a/arch/arm/cpu/armv7/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/lowlevel_init.S
@@ -17,10 +17,17 @@
 
 ENTRY(lowlevel_init)
 	/*
-	 * Setup a temporary stack
+	 * Setup a temporary stack. Global data is not available yet.
 	 */
 	ldr	sp, =CONFIG_SYS_INIT_SP_ADDR
 	bic	sp, sp, #7 /* 8-byte alignment for ABI compliance */
+#ifdef CONFIG_DM
+	mov	r9, #0
+#else
+	/*
+	 * Set up global data for boards that still need it. This will be
+	 * removed soon.
+	 */
 #ifdef CONFIG_SPL_BUILD
 	ldr	r9, =gdata
 #else
@@ -28,13 +35,24 @@ ENTRY(lowlevel_init)
 	bic	sp, sp, #7
 	mov	r9, sp
 #endif
+#endif
 	/*
 	 * Save the old lr(passed in ip) and the current lr to stack
 	 */
 	push	{ip, lr}
 
 	/*
-	 * go setup pll, mux, memory
+	 * Call the very early init function. This should do only the
+	 * absolute bare minimum to get started. It should not:
+	 *
+	 * - set up DRAM
+	 * - use global_data
+	 * - clear BSS
+	 * - try to start a console
+	 *
+	 * For boards with SPL this should be empty since SPL can do all of
+	 * this init in the SPL board_init_f() function which is called
+	 * immediately after this.
 	 */
 	bl	s_init
 	pop	{ip, pc}
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 2/6] arm: spl: Avoid setting up a duplicate global data structure

[Simon Glass]

This is already set up in crt0.S. We don't need a new structure and don't
really want one in the 'data' section of the image, since it will be empty
and crt0.S's changes will be ignored.

As an interim measure, remove it only if CONFIG_DM is not defined. This
allows us to press ahead with driver model in SPL and allow the stragglers
to catch up.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- Squash the gdata revert patch into this one
- Allow gdata to survive unless CONFIG_DM is defined, for now

Changes in v3: None
Changes in v2:
- Remove gdata definition from ARM spl header file also

 arch/arm/include/asm/spl.h | 2 ++
 arch/arm/lib/spl.c         | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h
index 17b6f54..6db405d 100644
--- a/arch/arm/include/asm/spl.h
+++ b/arch/arm/include/asm/spl.h
@@ -37,6 +37,8 @@ void spl_board_load_image(void);
 /* Linker symbols. */
 extern char __bss_start[], __bss_end[];
 
+#ifndef CONFIG_DM
 extern gd_t gdata;
+#endif
 
 #endif
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index c41850a..bd8c7d2 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -13,6 +13,7 @@
 #include <image.h>
 #include <linux/compiler.h>
 
+#ifndef CONFIG_DM
 /* Pointer to as well as the global data structure for SPL */
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -21,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR;
  * pafches that rely on it. The global_data area is set up in crt0.S.
  */
 gd_t gdata __attribute__ ((section(".data")));
+#endif
 
 /*
  * In the context of SPL, board_init_f must ensure that any clocks/etc for
@@ -33,8 +35,10 @@ void __weak board_init_f(ulong dummy)
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
+#ifndef CONFIG_DM
 	/* TODO: Remove settings of the global data pointer here */
 	gd = &gdata;
+#endif
 
 	board_init_r(NULL, 0);
 }
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Simon Glass]

Use the full driver model GPIO and serial drivers in SPL now that these are
supported. Since device tree is not available they will use platform data.

Remove the special SPL GPIO function as it is no longer needed.

This is all in one commit to maintain bisectability.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- Rebase on top of dm/master

Changes in v3:
- Drop extra blank line

Changes in v2:
- Rebase on top of the SPI series

 arch/arm/mach-tegra/Kconfig      |  3 +++
 board/nvidia/seaboard/seaboard.c |  4 +---
 drivers/gpio/Makefile            |  4 ++++
 drivers/gpio/tegra_gpio.c        | 27 +++++++++++----------------
 include/configs/tegra-common.h   |  6 ------
 5 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 8615248..fccfd79 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -29,6 +29,9 @@ config USE_PRIVATE_LIBGCC
 config DM
 	default y
 
+config SPL_DM
+	default y
+
 config DM_SERIAL
 	default y
 
diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
index 11472eb..25480e4 100644
--- a/board/nvidia/seaboard/seaboard.c
+++ b/board/nvidia/seaboard/seaboard.c
@@ -20,10 +20,8 @@
 void gpio_early_init_uart(void)
 {
 	/* Enable UART via GPIO_PI3 (port 8, bit 3) so serial console works */
-#ifndef CONFIG_SPL_BUILD
 	gpio_request(GPIO_PI3, NULL);
-#endif
-	tegra_spl_gpio_direction_output(GPIO_PI3, 0);
+	gpio_direction_output(GPIO_PI3, 0);
 }
 #endif
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index aa11f15..fe9a3b2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -8,6 +8,10 @@
 ifndef CONFIG_SPL_BUILD
 obj-$(CONFIG_DM_GPIO)		+= gpio-uclass.o
 endif
+/* TODO(sjg at chromium.org): Only tegra supports driver model in SPL */
+ifdef CONFIG_TEGRA_GPIO
+obj-$(CONFIG_DM_GPIO)		+= gpio-uclass.o
+endif
 
 obj-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 obj-$(CONFIG_INTEL_ICH6_GPIO)	+= intel_ich6_gpio.o
diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
index 43928b8..f870cdb 100644
--- a/drivers/gpio/tegra_gpio.c
+++ b/drivers/gpio/tegra_gpio.c
@@ -132,21 +132,6 @@ static void set_level(unsigned gpio, int high)
 	writel(u, &bank->gpio_out[GPIO_PORT(gpio)]);
 }
 
-/* set GPIO pin 'gpio' as an output, with polarity 'value' */
-int tegra_spl_gpio_direction_output(int gpio, int value)
-{
-	/* Configure as a GPIO */
-	set_config(gpio, 1);
-
-	/* Configure GPIO output value. */
-	set_level(gpio, value);
-
-	/* Configure GPIO direction as output. */
-	set_direction(gpio, 1);
-
-	return 0;
-}
-
 /*
  * Generic_GPIO primitives.
  */
@@ -338,12 +323,19 @@ static int gpio_tegra_bind(struct udevice *parent)
 	int bank_count;
 	int bank;
 	int ret;
-	int len;
 
 	/* If this is a child device, there is nothing to do here */
 	if (plat)
 		return 0;
 
+	/* TODO(sjg at chromium.org): Remove once SPL supports device tree */
+#ifdef CONFIG_SPL_BUILD
+	ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	bank_count = TEGRA_GPIO_BANKS;
+#else
+	{
+	int len;
+
 	/*
 	 * This driver does not make use of interrupts, other than to figure
 	 * out the number of GPIO banks
@@ -353,6 +345,8 @@ static int gpio_tegra_bind(struct udevice *parent)
 	bank_count = len / 3 / sizeof(u32);
 	ctlr = (struct gpio_ctlr *)fdtdec_get_addr(gd->fdt_blob,
 						   parent->of_offset, "reg");
+	}
+#endif
 	for (bank = 0; bank < bank_count; bank++) {
 		int port;
 
@@ -388,4 +382,5 @@ U_BOOT_DRIVER(gpio_tegra) = {
 	.probe = gpio_tegra_probe,
 	.priv_auto_alloc_size = sizeof(struct tegra_port_info),
 	.ops	= &gpio_tegra_ops,
+	.flags	= DM_FLAG_PRE_RELOC,
 };
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 005fc6a..fa6ccc1 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -43,13 +43,7 @@
 /*
  * NS16550 Configuration
  */
-#ifdef CONFIG_SPL_BUILD
-#define CONFIG_SYS_NS16550_SERIAL
-#define CONFIG_SYS_NS16550_REG_SIZE	(-4)
-#define CONFIG_SYS_NS16550_CLK		V_NS16550_CLK
-#else
 #define CONFIG_TEGRA_SERIAL
-#endif
 #define CONFIG_SYS_NS16550
 
 /*
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Simon Glass]

At present SPL uses a single stack, either CONFIG_SPL_STACK or
CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
point into SDRAM. They then set up SDRAM very early, before board_init_f(),
so that the larger stack can be used.

This is an abuse of lowlevel_init(). That function should only be used for
essential start-up code which cannot be delayed. An example of a valid use is
when only part of the SPL code is visible/executable, and the SoC must be set
up so that board_init_f() can be reached. It should not be used for SDRAM
init, console init, etc.

Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
address before board_init_r() is called in SPL.

The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.

Signed-off-by: Simon Glass <sjg@chromium.org>
For version 1:
Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Bo Shen <voice.shen@atmel.com>
Acked-by: Bo Shen <voice.shen@atmel.com>
Acked-by: Heiko Schocher <hs@denx.de>
Tested-by: Heiko Schocher <hs@denx.de>
---

Changes in v6: None
Changes in v5:
- Rebase to master

Changes in v4:
- Adjust README to mention that lowlevel_init() should have no stack

Changes in v3: None
Changes in v2:
- Move docs to top-level README file and expand them to cover U-Boot proper
- Add Kconfig settings

 Kconfig             | 18 ++++++++++++++
 README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/lib/crt0.S | 13 +++++++---
 common/spl/spl.c    | 35 +++++++++++++++++++++++++++
 4 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/Kconfig b/Kconfig
index 91a0618..8de731d 100644
--- a/Kconfig
+++ b/Kconfig
@@ -96,6 +96,24 @@ config SPL
 	help
 	  If you want to build SPL as well as the normal image, say Y.
 
+config CONFIG_SPL_STACK_R
+	depends on SPL
+	bool "Enable SDRAM location for SPL stack"
+	help
+	  SPL starts off execution in SRAM and thus typically has only a small
+	  stack available. Since SPL sets up DRAM while in its board_init_f()
+	  function, it is possible for the stack to move there before
+	  board_init_r() is reached. This option enables a special SDRAM
+	  location for the SPL stack. U-Boot SPL switches to this after
+	  board_init_f() completes, and before board_init_r() starts.
+
+config CONFIG_SPL_STACK_R_ADDR
+	depends on CONFIG_SPL_STACK_R
+	hex "SDRAM location for SPL stack"
+	help
+	  Specify the address in SDRAM for the SPL stack. This will be set up
+	  before board_init_r() is called.
+
 config TPL
 	bool
 	depends on SPL && SUPPORT_TPL
diff --git a/README b/README
index febefb5..3547ead 100644
--- a/README
+++ b/README
@@ -273,6 +273,75 @@ run some of U-Boot's tests.
 See board/sandbox/README.sandbox for more details.
 
 
+Board Initialisation Flow:
+--------------------------
+
+This is the intended start-up flow for boards. This should apply for both
+SPL and U-Boot proper (i.e. they both follow the same rules). At present SPL
+mostly uses a separate code path, but the funtion names and roles of each
+function are the same. Some boards or architectures may not conform to this.
+At least most ARM boards which use CONFIG_SPL_FRAMEWORK conform to this.
+
+Execution starts with start.S with three functions called during init after
+that. The purpose and limitations of each is described below.
+
+lowlevel_init():
+	- purpose: essential init to permit execution to reach board_init_f()
+	- no global_data or BSS
+	- there is no stack (ARMv7 may have one but it will soon be removed)
+	- must not set up SDRAM or use console
+	- must only do the bare minimum to allow execution to continue to
+		board_init_f()
+	- this is almost never needed
+	- return normally from this function
+
+board_init_f():
+	- purpose: set up the machine ready for running board_init_r():
+		i.e. SDRAM and serial UART
+	- global_data is available
+	- stack is in SRAM
+	- BSS is not available, so you cannot use global/static variables,
+		only stack variables and global_data
+
+	Non-SPL-specific notes:
+	- dram_init() is called to set up DRAM. If already done in SPL this
+		can do nothing
+
+	SPL-specific notes:
+	- you can override the entire board_init_f() function with your own
+		version as needed.
+	- preloader_console_init() can be called here in extremis
+	- should set up SDRAM, and anything needed to make the UART work
+	- these is no need to clear BSS, it will be done by crt0.S
+	- must return normally from this function (don't call board_init_r()
+		directly)
+
+Here the BSS is cleared. For SPL, if CONFIG_SPL_STACK_R is defined, then at
+this point the stack and global_data are relocated to below
+CONFIG_SPL_STACK_R_ADDR. For non-SPL, U-Boot is relocated to run at the top of
+memory.
+
+board_init_r():
+	- purpose: main execution, common code
+	- global_data is available
+	- SDRAM is available
+	- BSS is available, all static/global variables can be used
+	- execution eventually continues to main_loop()
+
+	Non-SPL-specific notes:
+	- U-Boot is relocated to the top of memory and is now running from
+		there.
+
+	SPL-specific notes:
+	- stack is optionally in SDRAM, if CONFIG_SPL_STACK_R is defined and
+		CONFIG_SPL_STACK_R_ADDR points into SDRAM
+	- preloader_console_init() can be called here - typically this is
+		done by defining CONFIG_SPL_BOARD_INIT and then supplying a
+		spl_board_init() function containing this call
+	- loads U-Boot or (in falcon mode) Linux
+
+
+
 Configuration Options:
 ----------------------
 
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 22df3e5..7939ced 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -113,7 +113,14 @@ here:
 /* Set up final (full) environment */
 
 	bl	c_runtime_cpu_setup	/* we still call old routine here */
-
+#endif
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
+# ifdef CONFIG_SPL_BUILD
+	/* Use a DRAM stack for the rest of SPL, if requested */
+	bl	spl_relocate_stack_gd
+	cmp	r0, #0
+	movne	sp, r0
+# endif
 	ldr	r0, =__bss_start	/* this is auto-relocated! */
 	ldr	r1, =__bss_end		/* this is auto-relocated! */
 
@@ -124,9 +131,10 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
 	addlo	r0, r0, #4		/* move to next */
 	blo	clbss_l
 
+#if ! defined(CONFIG_SPL_BUILD)
 	bl coloured_LED_init
 	bl red_led_on
-
+#endif
 	/* call board_init_r(gd_t *id, ulong dest_addr) */
 	mov     r0, r9                  /* gd_t */
 	ldr	r1, [r9, #GD_RELOCADDR]	/* dest_addr */
@@ -134,7 +142,6 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
 	ldr	pc, =board_init_r	/* this is auto-relocated! */
 
 	/* we should not return here. */
-
 #endif
 
 ENDPROC(_main)
diff --git a/common/spl/spl.c b/common/spl/spl.c
index ded0f30..cd75bbc 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -281,3 +281,38 @@ void preloader_console_init(void)
 	spl_display_print();
 #endif
 }
+
+/**
+ * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
+ *
+ * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
+ * for the main board_init_r() execution. This is typically because we need
+ * more stack space for things like the MMC sub-system.
+ *
+ * This function calculates the stack position, copies the global_data into
+ * place and returns the new stack position. The caller is responsible for
+ * setting up the sp register.
+ *
+ * @return new stack location, or 0 to use the same stack
+ */
+ulong spl_relocate_stack_gd(void)
+{
+#ifdef CONFIG_SPL_STACK_R
+	gd_t *new_gd;
+	ulong ptr;
+
+	/* Get stack position: use 8-byte alignment for ABI compliance */
+	ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
+	ptr &= ~7;
+	new_gd = (gd_t *)ptr;
+	memcpy(new_gd, (void *)gd, sizeof(gd_t));
+	gd = new_gd;
+
+	/* Clear the BSS. */
+	memset(__bss_start, 0, __bss_end - __bss_start);
+
+	return ptr;
+#else
+	return 0;
+#endif
+}
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 5/6] Make export interface support CONFIG_SYS_MALLOC_SIMPLE

[Simon Glass]

When CONFIG_SYS_MALLOC_SIMPLE is defined, free() is a static inline. Make
sure that the export interface still builds in this case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6: None
Changes in v5: None
Changes in v4:
- Add new patch to make the export interface support CONFIG_SYS_MALLOC_SIMPLE

Changes in v3: None
Changes in v2: None

 include/_exports.h | 2 ++
 include/exports.h  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/_exports.h b/include/_exports.h
index 5944703..279017e 100644
--- a/include/_exports.h
+++ b/include/_exports.h
@@ -23,7 +23,9 @@
 	EXPORT_FUNC(dummy, void, free_hdlr, void)
 #endif
 	EXPORT_FUNC(malloc, void *, malloc, size_t)
+#ifndef CONFIG_SYS_MALLOC_SIMPLE
 	EXPORT_FUNC(free, void, free, void *)
+#endif
 	EXPORT_FUNC(udelay, void, udelay, unsigned long)
 	EXPORT_FUNC(get_timer, unsigned long, get_timer, unsigned long)
 	EXPORT_FUNC(vprintf, int, vprintf, const char *, va_list)
diff --git a/include/exports.h b/include/exports.h
index 205affe..1a01e43 100644
--- a/include/exports.h
+++ b/include/exports.h
@@ -15,7 +15,9 @@ int printf(const char* fmt, ...);
 void install_hdlr(int, interrupt_handler_t, void*);
 void free_hdlr(int);
 void *malloc(size_t);
+#ifndef CONFIG_SYS_MALLOC_SIMPLE
 void free(void*);
+#endif
 void __udelay(unsigned long);
 unsigned long get_timer(unsigned long);
 int vprintf(const char *, va_list);
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 6/6] ti: armv7: am33xx: Move SPL SDRAM into the correct place

[Simon Glass]

Currently SDRAM is set up before global_data is available. This will soon
not work. Adjust the ordering to cope, using the new CONFIG_SPL_STACK_R
option to ensure that the stack is still in SDRAM once baord_init_r()
starts in SPL.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6:
- Add new patch for TI boards to avoid gdata

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/cpu/armv7/am33xx/board.c | 13 +++++++++----
 include/configs/ti_armv7_common.h |  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
index 81477aa..67bef23 100644
--- a/arch/arm/cpu/armv7/am33xx/board.c
+++ b/arch/arm/cpu/armv7/am33xx/board.c
@@ -275,6 +275,14 @@ static void watchdog_disable(void)
 		;
 }
 
+#ifdef CONFIG_SPL_BUILD
+void board_init_f(ulong dummy)
+{
+	board_early_init_f();
+	sdram_init();
+}
+#endif
+
 void s_init(void)
 {
 	/*
@@ -290,6 +298,7 @@ void s_init(void)
 	setup_clocks_for_console();
 	uart_soft_reset();
 #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
+	/* TODO: This does not work, gd is not available yet */
 	gd->baudrate = CONFIG_BAUDRATE;
 	serial_init();
 	gd->have_console = 1;
@@ -298,9 +307,5 @@ void s_init(void)
 	/* Enable RTC32K clock */
 	rtc32k_enable();
 #endif
-#ifdef CONFIG_SPL_BUILD
-	board_early_init_f();
-	sdram_init();
-#endif
 }
 #endif
diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
index 2bd1164..9e7d94a 100644
--- a/include/configs/ti_armv7_common.h
+++ b/include/configs/ti_armv7_common.h
@@ -216,7 +216,8 @@
  * end of the BSS area.  We place our stack at 32MiB after the start of
  * DRAM to allow room for all of the above.
  */
-#define CONFIG_SPL_STACK		(CONFIG_SYS_SDRAM_BASE + (32 << 20))
+#define CONFIG_SPL_STACK		CONFIG_SYS_INIT_SP_ADDR
+#define CONFIG_SPL_STACK_R		(CONFIG_SYS_SDRAM_BASE + (32 << 20))
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE		0x80800000
 #endif
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH v6 6/6] ti: armv7: am33xx: Move SPL SDRAM into the correct place

[Tom Rini]

On Tue, Mar 03, 2015 at 08:03:02AM -0700, Simon Glass wrote:

> Currently SDRAM is set up before global_data is available. This will soon
> not work. Adjust the ordering to cope, using the new CONFIG_SPL_STACK_R
> option to ensure that the stack is still in SDRAM once baord_init_r()
> starts in SPL.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

It's good that we're on the same path :)  I think we should stick this
into the config files as part of getting people used to that.  What I've
got locally will also remove a bunch of CONFIG_SPL_STACK settings that
don't do anything now (since they pointed back up to
CONFIG_SYS_INIT_SP_ADDR).  I'm going to test the hard case (falcon mode
poking at env for settings).

But I'm going to grab your patch as the base since keeping
board_early_init_f() around will keep the am335x+NOR case closer to
working.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150303/07ec6093/attachment.sig>

[U-Boot] [PATCH v6 6/6] ti: armv7: am33xx: Move SPL SDRAM into the correct place

[Simon Glass]

Hi Tom,

On 3 March 2015 at 08:15, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Mar 03, 2015 at 08:03:02AM -0700, Simon Glass wrote:
>
>> Currently SDRAM is set up before global_data is available. This will soon
>> not work. Adjust the ordering to cope, using the new CONFIG_SPL_STACK_R
>> option to ensure that the stack is still in SDRAM once baord_init_r()
>> starts in SPL.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> It's good that we're on the same path :)  I think we should stick this
> into the config files as part of getting people used to that.  What I've
> got locally will also remove a bunch of CONFIG_SPL_STACK settings that
> don't do anything now (since they pointed back up to
> CONFIG_SYS_INIT_SP_ADDR).  I'm going to test the hard case (falcon mode
> poking at env for settings).

Agreed, I was nervous about doing that for things not in Kconfig.

>
> But I'm going to grab your patch as the base since keeping
> board_early_init_f() around will keep the am335x+NOR case closer to
> working.

Please do whatever suits and thanks for sorting this out.

Regards,
Simon

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:

> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> so that the larger stack can be used.
> 
> This is an abuse of lowlevel_init(). That function should only be used for
> essential start-up code which cannot be delayed. An example of a valid use is
> when only part of the SPL code is visible/executable, and the SoC must be set
> up so that board_init_f() can be reached. It should not be used for SDRAM
> init, console init, etc.
> 
> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> address before board_init_r() is called in SPL.
> 
> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> For version 1:
> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Tested-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Heiko Schocher <hs@denx.de>
> Tested-by: Heiko Schocher <hs@denx.de>
> ---
> 
> Changes in v6: None
> Changes in v5:
> - Rebase to master
> 
> Changes in v4:
> - Adjust README to mention that lowlevel_init() should have no stack
> 
> Changes in v3: None
> Changes in v2:
> - Move docs to top-level README file and expand them to cover U-Boot proper
> - Add Kconfig settings
> 
>  Kconfig             | 18 ++++++++++++++
>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/lib/crt0.S | 13 +++++++---
>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
>  4 files changed, 132 insertions(+), 3 deletions(-)
[snip]
> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> index 22df3e5..7939ced 100644
> --- a/arch/arm/lib/crt0.S
> +++ b/arch/arm/lib/crt0.S
> @@ -113,7 +113,14 @@ here:
>  /* Set up final (full) environment */
>  
>  	bl	c_runtime_cpu_setup	/* we still call old routine here */
> -
> +#endif
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
> +# ifdef CONFIG_SPL_BUILD
> +	/* Use a DRAM stack for the rest of SPL, if requested */
> +	bl	spl_relocate_stack_gd
> +	cmp	r0, #0
> +	movne	sp, r0
> +# endif
>  	ldr	r0, =__bss_start	/* this is auto-relocated! */
>  	ldr	r1, =__bss_end		/* this is auto-relocated! */
>  
> @@ -124,9 +131,10 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
>  	addlo	r0, r0, #4		/* move to next */
>  	blo	clbss_l
>  
> +#if ! defined(CONFIG_SPL_BUILD)
>  	bl coloured_LED_init
>  	bl red_led_on
> -
> +#endif
>  	/* call board_init_r(gd_t *id, ulong dest_addr) */
>  	mov     r0, r9                  /* gd_t */
>  	ldr	r1, [r9, #GD_RELOCADDR]	/* dest_addr */
> @@ -134,7 +142,6 @@ clbss_l:cmp	r0, r1			/* while not at end of BSS */
>  	ldr	pc, =board_init_r	/* this is auto-relocated! */
>  
>  	/* we should not return here. */
> -
>  #endif
>  
>  ENDPROC(_main)
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index ded0f30..cd75bbc 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -281,3 +281,38 @@ void preloader_console_init(void)
>  	spl_display_print();
>  #endif
>  }
> +
> +/**
> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
> + *
> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
> + * for the main board_init_r() execution. This is typically because we need
> + * more stack space for things like the MMC sub-system.
> + *
> + * This function calculates the stack position, copies the global_data into
> + * place and returns the new stack position. The caller is responsible for
> + * setting up the sp register.
> + *
> + * @return new stack location, or 0 to use the same stack
> + */
> +ulong spl_relocate_stack_gd(void)
> +{
> +#ifdef CONFIG_SPL_STACK_R
> +	gd_t *new_gd;
> +	ulong ptr;
> +
> +	/* Get stack position: use 8-byte alignment for ABI compliance */
> +	ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
> +	ptr &= ~7;
> +	new_gd = (gd_t *)ptr;
> +	memcpy(new_gd, (void *)gd, sizeof(gd_t));
> +	gd = new_gd;
> +
> +	/* Clear the BSS. */
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +
> +	return ptr;
> +#else
> +	return 0;
> +#endif
> +}

All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
the big use case (am335x_boneblack for example where
CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
the area for stack.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150303/38d2dd60/attachment.sig>

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Simon Glass]

Hi Tom,

On 3 March 2015 at 10:49, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
>
>> At present SPL uses a single stack, either CONFIG_SPL_STACK or
>> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
>> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
>> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
>> so that the larger stack can be used.
>>
>> This is an abuse of lowlevel_init(). That function should only be used for
>> essential start-up code which cannot be delayed. An example of a valid use is
>> when only part of the SPL code is visible/executable, and the SoC must be set
>> up so that board_init_f() can be reached. It should not be used for SDRAM
>> init, console init, etc.
>>
>> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
>> address before board_init_r() is called in SPL.
>>
>> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> For version 1:
>> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>> Tested-by: Bo Shen <voice.shen@atmel.com>
>> Acked-by: Bo Shen <voice.shen@atmel.com>
>> Acked-by: Heiko Schocher <hs@denx.de>
>> Tested-by: Heiko Schocher <hs@denx.de>
>> ---
>>
>> Changes in v6: None
>> Changes in v5:
>> - Rebase to master
>>
>> Changes in v4:
>> - Adjust README to mention that lowlevel_init() should have no stack
>>
>> Changes in v3: None
>> Changes in v2:
>> - Move docs to top-level README file and expand them to cover U-Boot proper
>> - Add Kconfig settings
>>
>>  Kconfig             | 18 ++++++++++++++
>>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  arch/arm/lib/crt0.S | 13 +++++++---
>>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
>>  4 files changed, 132 insertions(+), 3 deletions(-)
> [snip]
>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
>> index 22df3e5..7939ced 100644
>> --- a/arch/arm/lib/crt0.S
>> +++ b/arch/arm/lib/crt0.S
>> @@ -113,7 +113,14 @@ here:
>>  /* Set up final (full) environment */
>>
>>       bl      c_runtime_cpu_setup     /* we still call old routine here */
>> -
>> +#endif
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
>> +# ifdef CONFIG_SPL_BUILD
>> +     /* Use a DRAM stack for the rest of SPL, if requested */
>> +     bl      spl_relocate_stack_gd
>> +     cmp     r0, #0
>> +     movne   sp, r0
>> +# endif
>>       ldr     r0, =__bss_start        /* this is auto-relocated! */
>>       ldr     r1, =__bss_end          /* this is auto-relocated! */
>>
>> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
>>       addlo   r0, r0, #4              /* move to next */
>>       blo     clbss_l
>>
>> +#if ! defined(CONFIG_SPL_BUILD)
>>       bl coloured_LED_init
>>       bl red_led_on
>> -
>> +#endif
>>       /* call board_init_r(gd_t *id, ulong dest_addr) */
>>       mov     r0, r9                  /* gd_t */
>>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
>> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
>>       ldr     pc, =board_init_r       /* this is auto-relocated! */
>>
>>       /* we should not return here. */
>> -
>>  #endif
>>
>>  ENDPROC(_main)
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index ded0f30..cd75bbc 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -281,3 +281,38 @@ void preloader_console_init(void)
>>       spl_display_print();
>>  #endif
>>  }
>> +
>> +/**
>> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
>> + *
>> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
>> + * for the main board_init_r() execution. This is typically because we need
>> + * more stack space for things like the MMC sub-system.
>> + *
>> + * This function calculates the stack position, copies the global_data into
>> + * place and returns the new stack position. The caller is responsible for
>> + * setting up the sp register.
>> + *
>> + * @return new stack location, or 0 to use the same stack
>> + */
>> +ulong spl_relocate_stack_gd(void)
>> +{
>> +#ifdef CONFIG_SPL_STACK_R
>> +     gd_t *new_gd;
>> +     ulong ptr;
>> +
>> +     /* Get stack position: use 8-byte alignment for ABI compliance */
>> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
>> +     ptr &= ~7;
>> +     new_gd = (gd_t *)ptr;
>> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
>> +     gd = new_gd;
>> +
>> +     /* Clear the BSS. */
>> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> +
>> +     return ptr;
>> +#else
>> +     return 0;
>> +#endif
>> +}
>
> All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
> does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
> the big use case (am335x_boneblack for example where
> CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
> the area for stack.

OK I'll have to test more. What sort of problem should i see?

The return value from spl_relocate_stack_gd() should be shoved into
sp. Does that not happen?

Regards,
Simon

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Tue, Mar 03, 2015 at 12:04:16PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On 3 March 2015 at 10:49, Tom Rini <trini@konsulko.com> wrote:
> > On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
> >
> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> >> so that the larger stack can be used.
> >>
> >> This is an abuse of lowlevel_init(). That function should only be used for
> >> essential start-up code which cannot be delayed. An example of a valid use is
> >> when only part of the SPL code is visible/executable, and the SoC must be set
> >> up so that board_init_f() can be reached. It should not be used for SDRAM
> >> init, console init, etc.
> >>
> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> >> address before board_init_r() is called in SPL.
> >>
> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> For version 1:
> >> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >> Reviewed-by: Stefan Roese <sr@denx.de>
> >> Tested-by: Bo Shen <voice.shen@atmel.com>
> >> Acked-by: Bo Shen <voice.shen@atmel.com>
> >> Acked-by: Heiko Schocher <hs@denx.de>
> >> Tested-by: Heiko Schocher <hs@denx.de>
> >> ---
> >>
> >> Changes in v6: None
> >> Changes in v5:
> >> - Rebase to master
> >>
> >> Changes in v4:
> >> - Adjust README to mention that lowlevel_init() should have no stack
> >>
> >> Changes in v3: None
> >> Changes in v2:
> >> - Move docs to top-level README file and expand them to cover U-Boot proper
> >> - Add Kconfig settings
> >>
> >>  Kconfig             | 18 ++++++++++++++
> >>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  arch/arm/lib/crt0.S | 13 +++++++---
> >>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
> >>  4 files changed, 132 insertions(+), 3 deletions(-)
> > [snip]
> >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> >> index 22df3e5..7939ced 100644
> >> --- a/arch/arm/lib/crt0.S
> >> +++ b/arch/arm/lib/crt0.S
> >> @@ -113,7 +113,14 @@ here:
> >>  /* Set up final (full) environment */
> >>
> >>       bl      c_runtime_cpu_setup     /* we still call old routine here */
> >> -
> >> +#endif
> >> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
> >> +# ifdef CONFIG_SPL_BUILD
> >> +     /* Use a DRAM stack for the rest of SPL, if requested */
> >> +     bl      spl_relocate_stack_gd
> >> +     cmp     r0, #0
> >> +     movne   sp, r0
> >> +# endif
> >>       ldr     r0, =__bss_start        /* this is auto-relocated! */
> >>       ldr     r1, =__bss_end          /* this is auto-relocated! */
> >>
> >> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
> >>       addlo   r0, r0, #4              /* move to next */
> >>       blo     clbss_l
> >>
> >> +#if ! defined(CONFIG_SPL_BUILD)
> >>       bl coloured_LED_init
> >>       bl red_led_on
> >> -
> >> +#endif
> >>       /* call board_init_r(gd_t *id, ulong dest_addr) */
> >>       mov     r0, r9                  /* gd_t */
> >>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
> >> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
> >>       ldr     pc, =board_init_r       /* this is auto-relocated! */
> >>
> >>       /* we should not return here. */
> >> -
> >>  #endif
> >>
> >>  ENDPROC(_main)
> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
> >> index ded0f30..cd75bbc 100644
> >> --- a/common/spl/spl.c
> >> +++ b/common/spl/spl.c
> >> @@ -281,3 +281,38 @@ void preloader_console_init(void)
> >>       spl_display_print();
> >>  #endif
> >>  }
> >> +
> >> +/**
> >> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
> >> + *
> >> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
> >> + * for the main board_init_r() execution. This is typically because we need
> >> + * more stack space for things like the MMC sub-system.
> >> + *
> >> + * This function calculates the stack position, copies the global_data into
> >> + * place and returns the new stack position. The caller is responsible for
> >> + * setting up the sp register.
> >> + *
> >> + * @return new stack location, or 0 to use the same stack
> >> + */
> >> +ulong spl_relocate_stack_gd(void)
> >> +{
> >> +#ifdef CONFIG_SPL_STACK_R
> >> +     gd_t *new_gd;
> >> +     ulong ptr;
> >> +
> >> +     /* Get stack position: use 8-byte alignment for ABI compliance */
> >> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
> >> +     ptr &= ~7;
> >> +     new_gd = (gd_t *)ptr;
> >> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
> >> +     gd = new_gd;
> >> +
> >> +     /* Clear the BSS. */
> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> >> +
> >> +     return ptr;
> >> +#else
> >> +     return 0;
> >> +#endif
> >> +}
> >
> > All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
> > does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
> > the big use case (am335x_boneblack for example where
> > CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
> > the area for stack.
> 
> OK I'll have to test more. What sort of problem should i see?

We hang on entering env_reloc_spec() which is what puts some big sized
variables on the stack.

> The return value from spl_relocate_stack_gd() should be shoved into
> sp. Does that not happen?

It doesn't appear to be.  I hadn't broken out the BBB with JTAG tho.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150303/d24e864e/attachment.sig>

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Simon Glass]

Hi Tom,

On 3 March 2015 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Mar 03, 2015 at 12:04:16PM -0700, Simon Glass wrote:
>> Hi Tom,
>>
>> On 3 March 2015 at 10:49, Tom Rini <trini@konsulko.com> wrote:
>> > On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
>> >
>> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
>> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
>> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
>> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
>> >> so that the larger stack can be used.
>> >>
>> >> This is an abuse of lowlevel_init(). That function should only be used for
>> >> essential start-up code which cannot be delayed. An example of a valid use is
>> >> when only part of the SPL code is visible/executable, and the SoC must be set
>> >> up so that board_init_f() can be reached. It should not be used for SDRAM
>> >> init, console init, etc.
>> >>
>> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
>> >> address before board_init_r() is called in SPL.
>> >>
>> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
>> >>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> For version 1:
>> >> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
>> >> Reviewed-by: Stefan Roese <sr@denx.de>
>> >> Tested-by: Bo Shen <voice.shen@atmel.com>
>> >> Acked-by: Bo Shen <voice.shen@atmel.com>
>> >> Acked-by: Heiko Schocher <hs@denx.de>
>> >> Tested-by: Heiko Schocher <hs@denx.de>
>> >> ---
>> >>
>> >> Changes in v6: None
>> >> Changes in v5:
>> >> - Rebase to master
>> >>
>> >> Changes in v4:
>> >> - Adjust README to mention that lowlevel_init() should have no stack
>> >>
>> >> Changes in v3: None
>> >> Changes in v2:
>> >> - Move docs to top-level README file and expand them to cover U-Boot proper
>> >> - Add Kconfig settings
>> >>
>> >>  Kconfig             | 18 ++++++++++++++
>> >>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  arch/arm/lib/crt0.S | 13 +++++++---
>> >>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
>> >>  4 files changed, 132 insertions(+), 3 deletions(-)
>> > [snip]
>> >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
>> >> index 22df3e5..7939ced 100644
>> >> --- a/arch/arm/lib/crt0.S
>> >> +++ b/arch/arm/lib/crt0.S
>> >> @@ -113,7 +113,14 @@ here:
>> >>  /* Set up final (full) environment */
>> >>
>> >>       bl      c_runtime_cpu_setup     /* we still call old routine here */
>> >> -
>> >> +#endif
>> >> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
>> >> +# ifdef CONFIG_SPL_BUILD
>> >> +     /* Use a DRAM stack for the rest of SPL, if requested */
>> >> +     bl      spl_relocate_stack_gd
>> >> +     cmp     r0, #0
>> >> +     movne   sp, r0
>> >> +# endif
>> >>       ldr     r0, =__bss_start        /* this is auto-relocated! */
>> >>       ldr     r1, =__bss_end          /* this is auto-relocated! */
>> >>
>> >> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
>> >>       addlo   r0, r0, #4              /* move to next */
>> >>       blo     clbss_l
>> >>
>> >> +#if ! defined(CONFIG_SPL_BUILD)
>> >>       bl coloured_LED_init
>> >>       bl red_led_on
>> >> -
>> >> +#endif
>> >>       /* call board_init_r(gd_t *id, ulong dest_addr) */
>> >>       mov     r0, r9                  /* gd_t */
>> >>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
>> >> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
>> >>       ldr     pc, =board_init_r       /* this is auto-relocated! */
>> >>
>> >>       /* we should not return here. */
>> >> -
>> >>  #endif
>> >>
>> >>  ENDPROC(_main)
>> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> >> index ded0f30..cd75bbc 100644
>> >> --- a/common/spl/spl.c
>> >> +++ b/common/spl/spl.c
>> >> @@ -281,3 +281,38 @@ void preloader_console_init(void)
>> >>       spl_display_print();
>> >>  #endif
>> >>  }
>> >> +
>> >> +/**
>> >> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
>> >> + *
>> >> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
>> >> + * for the main board_init_r() execution. This is typically because we need
>> >> + * more stack space for things like the MMC sub-system.
>> >> + *
>> >> + * This function calculates the stack position, copies the global_data into
>> >> + * place and returns the new stack position. The caller is responsible for
>> >> + * setting up the sp register.
>> >> + *
>> >> + * @return new stack location, or 0 to use the same stack
>> >> + */
>> >> +ulong spl_relocate_stack_gd(void)
>> >> +{
>> >> +#ifdef CONFIG_SPL_STACK_R
>> >> +     gd_t *new_gd;
>> >> +     ulong ptr;
>> >> +
>> >> +     /* Get stack position: use 8-byte alignment for ABI compliance */
>> >> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
>> >> +     ptr &= ~7;
>> >> +     new_gd = (gd_t *)ptr;
>> >> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
>> >> +     gd = new_gd;
>> >> +
>> >> +     /* Clear the BSS. */
>> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> >> +
>> >> +     return ptr;
>> >> +#else
>> >> +     return 0;
>> >> +#endif
>> >> +}
>> >
>> > All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
>> > does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
>> > the big use case (am335x_boneblack for example where
>> > CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
>> > the area for stack.
>>
>> OK I'll have to test more. What sort of problem should i see?
>
> We hang on entering env_reloc_spec() which is what puts some big sized
> variables on the stack.
>
>> The return value from spl_relocate_stack_gd() should be shoved into
>> sp. Does that not happen?
>
> It doesn't appear to be.  I hadn't broken out the BBB with JTAG tho.

I printed out the stack in board_init_r() and it looks right: 32MB
above the base of SDRAM.

Also I don't seem to get a crash in SPL. How do I make that happen? If
I define CONFIG_SPL_USBETH_SUPPORT I get an error about the .data
section not fitin in .sram for u-boot-spl.

Regards,
Simon

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Tue, Mar 03, 2015 at 08:23:23PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On 3 March 2015 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> > On Tue, Mar 03, 2015 at 12:04:16PM -0700, Simon Glass wrote:
> >> Hi Tom,
> >>
> >> On 3 March 2015 at 10:49, Tom Rini <trini@konsulko.com> wrote:
> >> > On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
> >> >
> >> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> >> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> >> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> >> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> >> >> so that the larger stack can be used.
> >> >>
> >> >> This is an abuse of lowlevel_init(). That function should only be used for
> >> >> essential start-up code which cannot be delayed. An example of a valid use is
> >> >> when only part of the SPL code is visible/executable, and the SoC must be set
> >> >> up so that board_init_f() can be reached. It should not be used for SDRAM
> >> >> init, console init, etc.
> >> >>
> >> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> >> >> address before board_init_r() is called in SPL.
> >> >>
> >> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> >> >>
> >> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> >> For version 1:
> >> >> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >> >> Reviewed-by: Stefan Roese <sr@denx.de>
> >> >> Tested-by: Bo Shen <voice.shen@atmel.com>
> >> >> Acked-by: Bo Shen <voice.shen@atmel.com>
> >> >> Acked-by: Heiko Schocher <hs@denx.de>
> >> >> Tested-by: Heiko Schocher <hs@denx.de>
> >> >> ---
> >> >>
> >> >> Changes in v6: None
> >> >> Changes in v5:
> >> >> - Rebase to master
> >> >>
> >> >> Changes in v4:
> >> >> - Adjust README to mention that lowlevel_init() should have no stack
> >> >>
> >> >> Changes in v3: None
> >> >> Changes in v2:
> >> >> - Move docs to top-level README file and expand them to cover U-Boot proper
> >> >> - Add Kconfig settings
> >> >>
> >> >>  Kconfig             | 18 ++++++++++++++
> >> >>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >>  arch/arm/lib/crt0.S | 13 +++++++---
> >> >>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
> >> >>  4 files changed, 132 insertions(+), 3 deletions(-)
> >> > [snip]
> >> >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> >> >> index 22df3e5..7939ced 100644
> >> >> --- a/arch/arm/lib/crt0.S
> >> >> +++ b/arch/arm/lib/crt0.S
> >> >> @@ -113,7 +113,14 @@ here:
> >> >>  /* Set up final (full) environment */
> >> >>
> >> >>       bl      c_runtime_cpu_setup     /* we still call old routine here */
> >> >> -
> >> >> +#endif
> >> >> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
> >> >> +# ifdef CONFIG_SPL_BUILD
> >> >> +     /* Use a DRAM stack for the rest of SPL, if requested */
> >> >> +     bl      spl_relocate_stack_gd
> >> >> +     cmp     r0, #0
> >> >> +     movne   sp, r0
> >> >> +# endif
> >> >>       ldr     r0, =__bss_start        /* this is auto-relocated! */
> >> >>       ldr     r1, =__bss_end          /* this is auto-relocated! */
> >> >>
> >> >> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
> >> >>       addlo   r0, r0, #4              /* move to next */
> >> >>       blo     clbss_l
> >> >>
> >> >> +#if ! defined(CONFIG_SPL_BUILD)
> >> >>       bl coloured_LED_init
> >> >>       bl red_led_on
> >> >> -
> >> >> +#endif
> >> >>       /* call board_init_r(gd_t *id, ulong dest_addr) */
> >> >>       mov     r0, r9                  /* gd_t */
> >> >>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
> >> >> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
> >> >>       ldr     pc, =board_init_r       /* this is auto-relocated! */
> >> >>
> >> >>       /* we should not return here. */
> >> >> -
> >> >>  #endif
> >> >>
> >> >>  ENDPROC(_main)
> >> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
> >> >> index ded0f30..cd75bbc 100644
> >> >> --- a/common/spl/spl.c
> >> >> +++ b/common/spl/spl.c
> >> >> @@ -281,3 +281,38 @@ void preloader_console_init(void)
> >> >>       spl_display_print();
> >> >>  #endif
> >> >>  }
> >> >> +
> >> >> +/**
> >> >> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
> >> >> + *
> >> >> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
> >> >> + * for the main board_init_r() execution. This is typically because we need
> >> >> + * more stack space for things like the MMC sub-system.
> >> >> + *
> >> >> + * This function calculates the stack position, copies the global_data into
> >> >> + * place and returns the new stack position. The caller is responsible for
> >> >> + * setting up the sp register.
> >> >> + *
> >> >> + * @return new stack location, or 0 to use the same stack
> >> >> + */
> >> >> +ulong spl_relocate_stack_gd(void)
> >> >> +{
> >> >> +#ifdef CONFIG_SPL_STACK_R
> >> >> +     gd_t *new_gd;
> >> >> +     ulong ptr;
> >> >> +
> >> >> +     /* Get stack position: use 8-byte alignment for ABI compliance */
> >> >> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
> >> >> +     ptr &= ~7;
> >> >> +     new_gd = (gd_t *)ptr;
> >> >> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
> >> >> +     gd = new_gd;
> >> >> +
> >> >> +     /* Clear the BSS. */
> >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> >> >> +
> >> >> +     return ptr;
> >> >> +#else
> >> >> +     return 0;
> >> >> +#endif
> >> >> +}
> >> >
> >> > All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
> >> > does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
> >> > the big use case (am335x_boneblack for example where
> >> > CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
> >> > the area for stack.
> >>
> >> OK I'll have to test more. What sort of problem should i see?
> >
> > We hang on entering env_reloc_spec() which is what puts some big sized
> > variables on the stack.
> >
> >> The return value from spl_relocate_stack_gd() should be shoved into
> >> sp. Does that not happen?
> >
> > It doesn't appear to be.  I hadn't broken out the BBB with JTAG tho.
> 
> I printed out the stack in board_init_r() and it looks right: 32MB
> above the base of SDRAM.
> 
> Also I don't seem to get a crash in SPL. How do I make that happen? If
> I define CONFIG_SPL_USBETH_SUPPORT I get an error about the .data
> section not fitin in .sram for u-boot-spl.

With v5 at least, am335x_boneblack_config running on a BBB does it.
I'll try v6 this morning and see what's going on..

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/56f20808/attachment.sig>

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Wed, Mar 04, 2015 at 08:38:33AM -0500, Tom Rini wrote:
> On Tue, Mar 03, 2015 at 08:23:23PM -0700, Simon Glass wrote:
> > Hi Tom,
> > 
> > On 3 March 2015 at 13:14, Tom Rini <trini@konsulko.com> wrote:
> > > On Tue, Mar 03, 2015 at 12:04:16PM -0700, Simon Glass wrote:
> > >> Hi Tom,
> > >>
> > >> On 3 March 2015 at 10:49, Tom Rini <trini@konsulko.com> wrote:
> > >> > On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
> > >> >
> > >> >> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> > >> >> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> > >> >> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> > >> >> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> > >> >> so that the larger stack can be used.
> > >> >>
> > >> >> This is an abuse of lowlevel_init(). That function should only be used for
> > >> >> essential start-up code which cannot be delayed. An example of a valid use is
> > >> >> when only part of the SPL code is visible/executable, and the SoC must be set
> > >> >> up so that board_init_f() can be reached. It should not be used for SDRAM
> > >> >> init, console init, etc.
> > >> >>
> > >> >> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> > >> >> address before board_init_r() is called in SPL.
> > >> >>
> > >> >> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> > >> >>
> > >> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> > >> >> For version 1:
> > >> >> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > >> >> Reviewed-by: Stefan Roese <sr@denx.de>
> > >> >> Tested-by: Bo Shen <voice.shen@atmel.com>
> > >> >> Acked-by: Bo Shen <voice.shen@atmel.com>
> > >> >> Acked-by: Heiko Schocher <hs@denx.de>
> > >> >> Tested-by: Heiko Schocher <hs@denx.de>
> > >> >> ---
> > >> >>
> > >> >> Changes in v6: None
> > >> >> Changes in v5:
> > >> >> - Rebase to master
> > >> >>
> > >> >> Changes in v4:
> > >> >> - Adjust README to mention that lowlevel_init() should have no stack
> > >> >>
> > >> >> Changes in v3: None
> > >> >> Changes in v2:
> > >> >> - Move docs to top-level README file and expand them to cover U-Boot proper
> > >> >> - Add Kconfig settings
> > >> >>
> > >> >>  Kconfig             | 18 ++++++++++++++
> > >> >>  README              | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >> >>  arch/arm/lib/crt0.S | 13 +++++++---
> > >> >>  common/spl/spl.c    | 35 +++++++++++++++++++++++++++
> > >> >>  4 files changed, 132 insertions(+), 3 deletions(-)
> > >> > [snip]
> > >> >> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> > >> >> index 22df3e5..7939ced 100644
> > >> >> --- a/arch/arm/lib/crt0.S
> > >> >> +++ b/arch/arm/lib/crt0.S
> > >> >> @@ -113,7 +113,14 @@ here:
> > >> >>  /* Set up final (full) environment */
> > >> >>
> > >> >>       bl      c_runtime_cpu_setup     /* we still call old routine here */
> > >> >> -
> > >> >> +#endif
> > >> >> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
> > >> >> +# ifdef CONFIG_SPL_BUILD
> > >> >> +     /* Use a DRAM stack for the rest of SPL, if requested */
> > >> >> +     bl      spl_relocate_stack_gd
> > >> >> +     cmp     r0, #0
> > >> >> +     movne   sp, r0
> > >> >> +# endif
> > >> >>       ldr     r0, =__bss_start        /* this is auto-relocated! */
> > >> >>       ldr     r1, =__bss_end          /* this is auto-relocated! */
> > >> >>
> > >> >> @@ -124,9 +131,10 @@ clbss_l:cmp      r0, r1                  /* while not at end of BSS */
> > >> >>       addlo   r0, r0, #4              /* move to next */
> > >> >>       blo     clbss_l
> > >> >>
> > >> >> +#if ! defined(CONFIG_SPL_BUILD)
> > >> >>       bl coloured_LED_init
> > >> >>       bl red_led_on
> > >> >> -
> > >> >> +#endif
> > >> >>       /* call board_init_r(gd_t *id, ulong dest_addr) */
> > >> >>       mov     r0, r9                  /* gd_t */
> > >> >>       ldr     r1, [r9, #GD_RELOCADDR] /* dest_addr */
> > >> >> @@ -134,7 +142,6 @@ clbss_l:cmp       r0, r1                  /* while not at end of BSS */
> > >> >>       ldr     pc, =board_init_r       /* this is auto-relocated! */
> > >> >>
> > >> >>       /* we should not return here. */
> > >> >> -
> > >> >>  #endif
> > >> >>
> > >> >>  ENDPROC(_main)
> > >> >> diff --git a/common/spl/spl.c b/common/spl/spl.c
> > >> >> index ded0f30..cd75bbc 100644
> > >> >> --- a/common/spl/spl.c
> > >> >> +++ b/common/spl/spl.c
> > >> >> @@ -281,3 +281,38 @@ void preloader_console_init(void)
> > >> >>       spl_display_print();
> > >> >>  #endif
> > >> >>  }
> > >> >> +
> > >> >> +/**
> > >> >> + * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution
> > >> >> + *
> > >> >> + * Sometimes board_init_f() runs with a stack in SRAM but we want to use SDRAM
> > >> >> + * for the main board_init_r() execution. This is typically because we need
> > >> >> + * more stack space for things like the MMC sub-system.
> > >> >> + *
> > >> >> + * This function calculates the stack position, copies the global_data into
> > >> >> + * place and returns the new stack position. The caller is responsible for
> > >> >> + * setting up the sp register.
> > >> >> + *
> > >> >> + * @return new stack location, or 0 to use the same stack
> > >> >> + */
> > >> >> +ulong spl_relocate_stack_gd(void)
> > >> >> +{
> > >> >> +#ifdef CONFIG_SPL_STACK_R
> > >> >> +     gd_t *new_gd;
> > >> >> +     ulong ptr;
> > >> >> +
> > >> >> +     /* Get stack position: use 8-byte alignment for ABI compliance */
> > >> >> +     ptr = CONFIG_SPL_STACK_R - sizeof(gd_t);
> > >> >> +     ptr &= ~7;
> > >> >> +     new_gd = (gd_t *)ptr;
> > >> >> +     memcpy(new_gd, (void *)gd, sizeof(gd_t));
> > >> >> +     gd = new_gd;
> > >> >> +
> > >> >> +     /* Clear the BSS. */
> > >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> > >> >> +
> > >> >> +     return ptr;
> > >> >> +#else
> > >> >> +     return 0;
> > >> >> +#endif
> > >> >> +}
> > >> >
> > >> > All of this _does_ move gd into where CONFIG_SPL_STACK_R points.  It
> > >> > does _not_ move the stack itself into where CONFIG_SPL_STACK_R points so
> > >> > the big use case (am335x_boneblack for example where
> > >> > CONFIG_SPL_ENV_SUPPORT is set) doesn't work and blows up as we overflow
> > >> > the area for stack.
> > >>
> > >> OK I'll have to test more. What sort of problem should i see?
> > >
> > > We hang on entering env_reloc_spec() which is what puts some big sized
> > > variables on the stack.
> > >
> > >> The return value from spl_relocate_stack_gd() should be shoved into
> > >> sp. Does that not happen?
> > >
> > > It doesn't appear to be.  I hadn't broken out the BBB with JTAG tho.
> > 
> > I printed out the stack in board_init_r() and it looks right: 32MB
> > above the base of SDRAM.
> > 
> > Also I don't seem to get a crash in SPL. How do I make that happen? If
> > I define CONFIG_SPL_USBETH_SUPPORT I get an error about the .data
> > section not fitin in .sram for u-boot-spl.
> 
> With v5 at least, am335x_boneblack_config running on a BBB does it.
> I'll try v6 this morning and see what's going on..

With your v6 as-is, things are working.  So I'll go track down what's
hopefully some problem with what I was doing from here, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/569875bf/attachment.sig>

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:

> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> so that the larger stack can be used.
> 
> This is an abuse of lowlevel_init(). That function should only be used for
> essential start-up code which cannot be delayed. An example of a valid use is
> when only part of the SPL code is visible/executable, and the SoC must be set
> up so that board_init_f() can be reached. It should not be used for SDRAM
> init, console init, etc.
> 
> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> address before board_init_r() is called in SPL.
> 
> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
[snip]
> diff --git a/Kconfig b/Kconfig
> index 91a0618..8de731d 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -96,6 +96,24 @@ config SPL
>  	help
>  	  If you want to build SPL as well as the normal image, say Y.
>  
> +config CONFIG_SPL_STACK_R
> +	depends on SPL
> +	bool "Enable SDRAM location for SPL stack"
> +	help
> +	  SPL starts off execution in SRAM and thus typically has only a small
> +	  stack available. Since SPL sets up DRAM while in its board_init_f()
> +	  function, it is possible for the stack to move there before
> +	  board_init_r() is reached. This option enables a special SDRAM
> +	  location for the SPL stack. U-Boot SPL switches to this after
> +	  board_init_f() completes, and before board_init_r() starts.
> +
> +config CONFIG_SPL_STACK_R_ADDR
> +	depends on CONFIG_SPL_STACK_R

I found my problem!  Incorrect Kconfig syntax, no CONFIG_ in 'config' or
'depends' lines only help lines.  So I was right in that my stack wasn't
moving because I was doing it via the defconfig and that was broken :)

I'm just going to pick up 1-5 (with this fixed) and send out my own 6
after I test it on omap3 and omap4/5 which also need a similar
conversion.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/50169024/attachment.sig>

[U-Boot] [PATCH v7 6/6] ti: armv7: Move SPL SDRAM init to the right place, drop unused CONFIG_SPL_STACK

[Tom Rini]

From: Simon Glass <sjg@chromium.org>

Currently in some cases SDRAM init requires global_data to be available
and soon this will not be available prior to board_init_f().  Adjust the
code paths in these cases to be correct.  In some cases we had the SPL
stack be in DDR as we might have large stacks (due to Falcon Mode +
Environment).  In these cases switch to CONFIG_SPL_STACK_R.  In other
cases we had simply been setting CONFIG_SPL_STACK into SRAM.  In these
cases we no longer need to (CONFIG_SYS_INIT_SP_ADDR is used and is also
in SRAM) so drop those lines.

Signed-off-by: Simon Glass <sjg@chromium.org>
Tested on Beagleboard, Beagleboard xM
Tested-by: Matt Porter <mporter@konsulko.com>
Tested on Beaglebone Black, AM43xx GP EVM, OMAP5 uEVM, OMAP4 Pandaboard
Tested-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 arch/arm/cpu/armv7/am33xx/board.c              |   13 +++++++++----
 arch/arm/cpu/armv7/omap-common/hwinit-common.c |    6 +++++-
 arch/arm/cpu/armv7/omap3/board.c               |   10 ++++++----
 configs/am335x_boneblack_defconfig             |    2 ++
 configs/am335x_boneblack_vboot_defconfig       |    2 ++
 configs/am335x_evm_defconfig                   |    2 ++
 configs/am335x_evm_nor_defconfig               |    2 ++
 configs/am335x_evm_spiboot_defconfig           |    2 ++
 configs/am335x_evm_usbspl_defconfig            |    2 ++
 configs/am335x_igep0033_defconfig              |    2 ++
 include/configs/am3517_crane.h                 |    1 -
 include/configs/am3517_evm.h                   |    1 -
 include/configs/bur_am335x_common.h            |    1 -
 include/configs/cm_t35.h                       |    1 -
 include/configs/devkit8000.h                   |    2 --
 include/configs/omap3_evm_common.h             |    1 -
 include/configs/siemens-am33x-common.h         |    1 -
 include/configs/tam3517-common.h               |    1 -
 include/configs/tao3530.h                      |    1 -
 include/configs/ti814x_evm.h                   |    1 -
 include/configs/ti816x_evm.h                   |    1 -
 include/configs/ti_armv7_common.h              |    5 ++---
 include/configs/tricorder.h                    |    1 -
 23 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
index 81477aa..67bef23 100644
--- a/arch/arm/cpu/armv7/am33xx/board.c
+++ b/arch/arm/cpu/armv7/am33xx/board.c
@@ -275,6 +275,14 @@ static void watchdog_disable(void)
 		;
 }
 
+#ifdef CONFIG_SPL_BUILD
+void board_init_f(ulong dummy)
+{
+	board_early_init_f();
+	sdram_init();
+}
+#endif
+
 void s_init(void)
 {
 	/*
@@ -290,6 +298,7 @@ void s_init(void)
 	setup_clocks_for_console();
 	uart_soft_reset();
 #if defined(CONFIG_NOR_BOOT) || defined(CONFIG_QSPI_BOOT)
+	/* TODO: This does not work, gd is not available yet */
 	gd->baudrate = CONFIG_BAUDRATE;
 	serial_init();
 	gd->have_console = 1;
@@ -298,9 +307,5 @@ void s_init(void)
 	/* Enable RTC32K clock */
 	rtc32k_enable();
 #endif
-#ifdef CONFIG_SPL_BUILD
-	board_early_init_f();
-	sdram_init();
-#endif
 }
 #endif
diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c
index cb35c19..6c8f3bc 100644
--- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c
+++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c
@@ -128,14 +128,18 @@ void s_init(void)
 	do_io_settings();
 #endif
 	prcm_init();
+}
+
 #ifdef CONFIG_SPL_BUILD
+void board_init_f(ulong dummy)
+{
 #ifdef CONFIG_BOARD_EARLY_INIT_F
 	board_early_init_f();
 #endif
 	/* For regular u-boot sdram_init() is called from dram_init() */
 	sdram_init();
-#endif
 }
+#endif
 
 /*
  * Routine: wait_for_command_complete
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 90d6ae7..347947c 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -240,8 +240,6 @@ void try_unlock_memory(void)
  *****************************************************************************/
 void s_init(void)
 {
-	int in_sdram = is_running_in_sdram();
-
 	watchdog_init();
 
 	try_unlock_memory();
@@ -264,10 +262,14 @@ void s_init(void)
 #ifdef CONFIG_USB_EHCI_OMAP
 	ehci_clocks_enable();
 #endif
+}
 
-	if (!in_sdram)
-		mem_init();
+#ifdef CONFIG_SPL_BUILD
+void board_init_f(ulong dummy)
+{
+	mem_init();
 }
+#endif
 
 /*
  * Routine: misc_init_r
diff --git a/configs/am335x_boneblack_defconfig b/configs/am335x_boneblack_defconfig
index b631c41..f5b807b 100644
--- a/configs/am335x_boneblack_defconfig
+++ b/configs/am335x_boneblack_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="EMMC_BOOT"
 +S:CONFIG_ARM=y
 +S:CONFIG_TARGET_AM335X_EVM=y
diff --git a/configs/am335x_boneblack_vboot_defconfig b/configs/am335x_boneblack_vboot_defconfig
index 51bf370..db27c3e 100644
--- a/configs/am335x_boneblack_vboot_defconfig
+++ b/configs/am335x_boneblack_vboot_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="EMMC_BOOT,ENABLE_VBOOT"
 +S:CONFIG_ARM=y
 +S:CONFIG_TARGET_AM335X_EVM=y
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
index 2e5aeaa..b2f332e 100644
--- a/configs/am335x_evm_defconfig
+++ b/configs/am335x_evm_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="NAND"
 CONFIG_CONS_INDEX=1
 +S:CONFIG_ARM=y
diff --git a/configs/am335x_evm_nor_defconfig b/configs/am335x_evm_nor_defconfig
index be90163..ed72af6 100644
--- a/configs/am335x_evm_nor_defconfig
+++ b/configs/am335x_evm_nor_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="NAND"
 CONFIG_CONS_INDEX=1
 +S:CONFIG_ARM=y
diff --git a/configs/am335x_evm_spiboot_defconfig b/configs/am335x_evm_spiboot_defconfig
index a6188ea..097dd47 100644
--- a/configs/am335x_evm_spiboot_defconfig
+++ b/configs/am335x_evm_spiboot_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="SPI_BOOT"
 CONFIG_CONS_INDEX=1
 +S:CONFIG_ARM=y
diff --git a/configs/am335x_evm_usbspl_defconfig b/configs/am335x_evm_usbspl_defconfig
index 352c1fb..773042a 100644
--- a/configs/am335x_evm_usbspl_defconfig
+++ b/configs/am335x_evm_usbspl_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 CONFIG_SYS_EXTRA_OPTIONS="NAND,SPL_USBETH_SUPPORT"
 CONFIG_CONS_INDEX=1
 +S:CONFIG_ARM=y
diff --git a/configs/am335x_igep0033_defconfig b/configs/am335x_igep0033_defconfig
index f3544b5..7634d03 100644
--- a/configs/am335x_igep0033_defconfig
+++ b/configs/am335x_igep0033_defconfig
@@ -1,4 +1,6 @@
 CONFIG_SPL=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
 +S:CONFIG_ARM=y
 +S:CONFIG_TARGET_AM335X_IGEP0033=y
 CONFIG_SYS_MALLOC_F=y
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 09ee10c..290a6a3 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -297,7 +297,6 @@
 #define CONFIG_SPL_NAND_SIMPLE
 #define CONFIG_SPL_TEXT_BASE		0x40200800
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index 190ef0e..3de5079 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -306,7 +306,6 @@
 #define CONFIG_SPL_NAND_SIMPLE
 #define CONFIG_SPL_TEXT_BASE		0x40200800
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
diff --git a/include/configs/bur_am335x_common.h b/include/configs/bur_am335x_common.h
index e9d5d01..49afe46 100644
--- a/include/configs/bur_am335x_common.h
+++ b/include/configs/bur_am335x_common.h
@@ -175,7 +175,6 @@
  *
  * ----------------------------------------------------------------------------
  */
-#define CONFIG_SPL_STACK		CONFIG_SYS_INIT_SP_ADDR
 #undef  CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE		0x80800000
 #define CONFIG_SPL_BSS_START_ADDR	0x80A00000
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index b2a9f35..9feca1b 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -365,7 +365,6 @@
 
 #define CONFIG_SPL_TEXT_BASE		0x40200800
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 /*
  * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index 1c69551..84b047e 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -213,8 +213,6 @@
 
 #undef CONFIG_SPL_TEXT_BASE
 #define CONFIG_SPL_TEXT_BASE		0x40200000 /*CONFIG_SYS_SRAM_START*/
-#undef CONFIG_SPL_STACK
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 /* NAND boot config */
 #define CONFIG_SYS_NAND_BUSWIDTH_16BIT	16
diff --git a/include/configs/omap3_evm_common.h b/include/configs/omap3_evm_common.h
index 8885e17..e7df154 100644
--- a/include/configs/omap3_evm_common.h
+++ b/include/configs/omap3_evm_common.h
@@ -259,7 +259,6 @@
 #define CONFIG_SPL_FRAMEWORK
 #define CONFIG_SPL_TEXT_BASE		0x40200800
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
diff --git a/include/configs/siemens-am33x-common.h b/include/configs/siemens-am33x-common.h
index 21e13e5..c7affd6 100644
--- a/include/configs/siemens-am33x-common.h
+++ b/include/configs/siemens-am33x-common.h
@@ -142,7 +142,6 @@
 #define CONFIG_SPL_FRAMEWORK
 #define CONFIG_SPL_TEXT_BASE		0x402F0400
 #define CONFIG_SPL_MAX_SIZE		(101 * 1024)
-#define CONFIG_SPL_STACK		CONFIG_SYS_INIT_SP_ADDR
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
diff --git a/include/configs/tam3517-common.h b/include/configs/tam3517-common.h
index 9fbe68a..38288f6 100644
--- a/include/configs/tam3517-common.h
+++ b/include/configs/tam3517-common.h
@@ -237,7 +237,6 @@
 
 #define CONFIG_SPL_TEXT_BASE		0x40200000 /*CONFIG_SYS_SRAM_START*/
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 #define CONFIG_SYS_SPL_MALLOC_START	0x8f000000
 #define CONFIG_SYS_SPL_MALLOC_SIZE	0x80000
diff --git a/include/configs/tao3530.h b/include/configs/tao3530.h
index 7d2c0d2..dd69d4e 100644
--- a/include/configs/tao3530.h
+++ b/include/configs/tao3530.h
@@ -346,7 +346,6 @@
 
 #define CONFIG_SPL_TEXT_BASE		0x40200800
 #define CONFIG_SPL_MAX_SIZE		(54 * 1024)	/* 8 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 /*
  * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the
diff --git a/include/configs/ti814x_evm.h b/include/configs/ti814x_evm.h
index deb6bb2..dcc2bdc 100644
--- a/include/configs/ti814x_evm.h
+++ b/include/configs/ti814x_evm.h
@@ -171,7 +171,6 @@
 #define CONFIG_SPL_FRAMEWORK
 #define CONFIG_SPL_TEXT_BASE		0x40300000
 #define CONFIG_SPL_MAX_SIZE		((128 - 18) * 1024)
-#define CONFIG_SPL_STACK		CONFIG_SYS_INIT_SP_ADDR
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
diff --git a/include/configs/ti816x_evm.h b/include/configs/ti816x_evm.h
index f69a559..27a3dd1 100644
--- a/include/configs/ti816x_evm.h
+++ b/include/configs/ti816x_evm.h
@@ -136,7 +136,6 @@
 #define CONFIG_SPL_FRAMEWORK
 #define CONFIG_SPL_TEXT_BASE    0x40400000
 #define CONFIG_SPL_MAX_SIZE     ((128 - 18) * 1024)
-#define CONFIG_SPL_STACK        CONFIG_SYS_INIT_SP_ADDR
 
 #define CONFIG_SPL_BSS_START_ADDR   0x80000000
 #define CONFIG_SPL_BSS_MAX_SIZE     0x80000     /* 512 KB */
diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
index 2bd1164..c0c1060 100644
--- a/include/configs/ti_armv7_common.h
+++ b/include/configs/ti_armv7_common.h
@@ -213,10 +213,9 @@
  * SPLs).  We have our BSS be placed 2MiB after this, to allow for the
  * default Linux kernel address of 0x80008000 to work with most sized
  * kernels, in the Falcon Mode case.  We have the SPL malloc pool at the
- * end of the BSS area.  We place our stack at 32MiB after the start of
- * DRAM to allow room for all of the above.
+ * end of the BSS area.  We suggest that the stack be placed at 32MiB after
+ * the start of DRAM to allow room for all of the above (handled in Kconfig).
  */
-#define CONFIG_SPL_STACK		(CONFIG_SYS_SDRAM_BASE + (32 << 20))
 #ifndef CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_TEXT_BASE		0x80800000
 #endif
diff --git a/include/configs/tricorder.h b/include/configs/tricorder.h
index 36621a5..10ac4a4 100644
--- a/include/configs/tricorder.h
+++ b/include/configs/tricorder.h
@@ -354,7 +354,6 @@
 
 #define CONFIG_SPL_TEXT_BASE		0x40200000 /*CONFIG_SYS_SRAM_START*/
 #define CONFIG_SPL_MAX_SIZE		(57 * 1024)	/* 7 KB for stack */
-#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
 
 #define CONFIG_SPL_BSS_START_ADDR	0x80000000 /*CONFIG_SYS_SDRAM_BASE*/
 #define CONFIG_SPL_BSS_MAX_SIZE		0x80000
-- 
1.7.9.5

[U-Boot] [PATCH v7 6/6] ti: armv7: Move SPL SDRAM init to the right place, drop unused CONFIG_SPL_STACK

[Simon Glass]

Hi Tom,

On 4 March 2015 at 09:30, Tom Rini <trini@konsulko.com> wrote:
> From: Simon Glass <sjg@chromium.org>
>
> Currently in some cases SDRAM init requires global_data to be available
> and soon this will not be available prior to board_init_f().  Adjust the
> code paths in these cases to be correct.  In some cases we had the SPL
> stack be in DDR as we might have large stacks (due to Falcon Mode +
> Environment).  In these cases switch to CONFIG_SPL_STACK_R.  In other
> cases we had simply been setting CONFIG_SPL_STACK into SRAM.  In these
> cases we no longer need to (CONFIG_SYS_INIT_SP_ADDR is used and is also
> in SRAM) so drop those lines.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Tested on Beagleboard, Beagleboard xM
> Tested-by: Matt Porter <mporter@konsulko.com>
> Tested on Beaglebone Black, AM43xx GP EVM, OMAP5 uEVM, OMAP4 Pandaboard
> Tested-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>

Thanks for cleaning this all up.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
SImon

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Simon Glass]

Hi Tom,

On 4 March 2015 at 08:42, Tom Rini <trini@konsulko.com> wrote:
> On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:
>
>> At present SPL uses a single stack, either CONFIG_SPL_STACK or
>> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
>> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
>> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
>> so that the larger stack can be used.
>>
>> This is an abuse of lowlevel_init(). That function should only be used for
>> essential start-up code which cannot be delayed. An example of a valid use is
>> when only part of the SPL code is visible/executable, and the SoC must be set
>> up so that board_init_f() can be reached. It should not be used for SDRAM
>> init, console init, etc.
>>
>> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
>> address before board_init_r() is called in SPL.
>>
>> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> [snip]
>> diff --git a/Kconfig b/Kconfig
>> index 91a0618..8de731d 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -96,6 +96,24 @@ config SPL
>>       help
>>         If you want to build SPL as well as the normal image, say Y.
>>
>> +config CONFIG_SPL_STACK_R
>> +     depends on SPL
>> +     bool "Enable SDRAM location for SPL stack"
>> +     help
>> +       SPL starts off execution in SRAM and thus typically has only a small
>> +       stack available. Since SPL sets up DRAM while in its board_init_f()
>> +       function, it is possible for the stack to move there before
>> +       board_init_r() is reached. This option enables a special SDRAM
>> +       location for the SPL stack. U-Boot SPL switches to this after
>> +       board_init_f() completes, and before board_init_r() starts.
>> +
>> +config CONFIG_SPL_STACK_R_ADDR
>> +     depends on CONFIG_SPL_STACK_R
>
> I found my problem!  Incorrect Kconfig syntax, no CONFIG_ in 'config' or
> 'depends' lines only help lines.  So I was right in that my stack wasn't
> moving because I was doing it via the defconfig and that was broken :)
>
> I'm just going to pick up 1-5 (with this fixed) and send out my own 6
> after I test it on omap3 and omap4/5 which also need a similar
> conversion.

Great, thanks for doing this!

Regards,
Simon

[U-Boot] [PATCH v6 1/6] arm: Reduce the scope of lowlevel_init()

[Tom Rini]

On Tue, Mar 03, 2015 at 08:02:57AM -0700, Simon Glass wrote:

> This function has grown into something of a monster. Some boards are setting
> up a console and DRAM here in SPL. This requires global_data which should be
> set up in one place (crt0.S).
> 
> There is no need for SPL to use s_init() for anything since board_init_f()
> is called immediately afterwards.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/a2bfcbe6/attachment.sig>

[U-Boot] [PATCH v6 2/6] arm: spl: Avoid setting up a duplicate global data structure

[Tom Rini]

On Tue, Mar 03, 2015 at 08:02:58AM -0700, Simon Glass wrote:

> This is already set up in crt0.S. We don't need a new structure and don't
> really want one in the 'data' section of the image, since it will be empty
> and crt0.S's changes will be ignored.
> 
> As an interim measure, remove it only if CONFIG_DM is not defined. This
> allows us to press ahead with driver model in SPL and allow the stragglers
> to catch up.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/f5bb6a06/attachment.sig>

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Tom Rini]

On Tue, Mar 03, 2015 at 08:02:59AM -0700, Simon Glass wrote:

> Use the full driver model GPIO and serial drivers in SPL now that these are
> supported. Since device tree is not available they will use platform data.
> 
> Remove the special SPL GPIO function as it is no longer needed.
> 
> This is all in one commit to maintain bisectability.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/b25d4127/attachment.sig>

[U-Boot] [PATCH v6 4/6] arm: spl: Allow board_init_r() to run with a larger stack

[Tom Rini]

On Tue, Mar 03, 2015 at 08:03:00AM -0700, Simon Glass wrote:

> At present SPL uses a single stack, either CONFIG_SPL_STACK or
> CONFIG_SYS_INIT_SP_ADDR. Since some SPL features (such as MMC and
> environment) require a lot of stack, some boards set CONFIG_SPL_STACK to
> point into SDRAM. They then set up SDRAM very early, before board_init_f(),
> so that the larger stack can be used.
> 
> This is an abuse of lowlevel_init(). That function should only be used for
> essential start-up code which cannot be delayed. An example of a valid use is
> when only part of the SPL code is visible/executable, and the SoC must be set
> up so that board_init_f() can be reached. It should not be used for SDRAM
> init, console init, etc.
> 
> Add a CONFIG_SPL_STACK_R option, which allows the stack to be moved to a new
> address before board_init_r() is called in SPL.
> 
> The expected SPL flow (for CONFIG_SPL_FRAMEWORK) is documented in the README.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> For version 1:
> Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Tested-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Bo Shen <voice.shen@atmel.com>
> Acked-by: Heiko Schocher <hs@denx.de>
> Tested-by: Heiko Schocher <hs@denx.de>

After fixing the Kconfig part to not have 'CONFIG_' in the 'config' and
'depends' lines, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/ab1e354a/attachment.sig>

[U-Boot] [PATCH v6 5/6] Make export interface support CONFIG_SYS_MALLOC_SIMPLE

[Tom Rini]

On Tue, Mar 03, 2015 at 08:03:01AM -0700, Simon Glass wrote:

> When CONFIG_SYS_MALLOC_SIMPLE is defined, free() is a static inline. Make
> sure that the export interface still builds in this case.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/2f0b7702/attachment.sig>

[U-Boot] [PATCH v7 6/6] ti: armv7: Move SPL SDRAM init to the right place, drop unused CONFIG_SPL_STACK

[Tom Rini]

On Wed, Mar 04, 2015 at 11:30:28AM -0500, Tom Rini wrote:

> From: Simon Glass <sjg@chromium.org>
> 
> Currently in some cases SDRAM init requires global_data to be available
> and soon this will not be available prior to board_init_f().  Adjust the
> code paths in these cases to be correct.  In some cases we had the SPL
> stack be in DDR as we might have large stacks (due to Falcon Mode +
> Environment).  In these cases switch to CONFIG_SPL_STACK_R.  In other
> cases we had simply been setting CONFIG_SPL_STACK into SRAM.  In these
> cases we no longer need to (CONFIG_SYS_INIT_SP_ADDR is used and is also
> in SRAM) so drop those lines.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Tested on Beagleboard, Beagleboard xM
> Tested-by: Matt Porter <mporter@konsulko.com>
> Tested on Beaglebone Black, AM43xx GP EVM, OMAP5 uEVM, OMAP4 Pandaboard
> Tested-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150304/fac824e1/attachment.sig>

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Stephen Warren]

On 03/04/2015 03:16 PM, Tom Rini wrote:
> On Tue, Mar 03, 2015 at 08:02:59AM -0700, Simon Glass wrote:
> 
>> Use the full driver model GPIO and serial drivers in SPL now that
>> these are supported. Since device tree is not available they will
>> use platform data.
>> 
>> Remove the special SPL GPIO function as it is no longer needed.
>> 
>> This is all in one commit to maintain bisectability.
>> 
>> Signed-off-by: Simon Glass <sjg@chromium.org>
> 
> Applied to u-boot/master, thanks!

This patch breaks the serial console on at least NVIDIA Tegra
Seaboard. LCD/usbkbd still work though. I can't test any other Tegra
boards right now since I'm not at work. I wonder if the UART/SPI GPIO
is at issue? That would be specific to this board.

(I just happened to test with that board while investigating why
usbkbd was not working right on the RPi, since Seaboard has usbkbd
support... Yay nested bugs!)

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Simon Glass]

Hi Stephen,

On 8 March 2015 at 15:00, Stephen Warren <swarren@nvidia.com> wrote:
>
> On 03/04/2015 03:16 PM, Tom Rini wrote:
> > On Tue, Mar 03, 2015 at 08:02:59AM -0700, Simon Glass wrote:
> >
> >> Use the full driver model GPIO and serial drivers in SPL now that
> >> these are supported. Since device tree is not available they will
> >> use platform data.
> >>
> >> Remove the special SPL GPIO function as it is no longer needed.
> >>
> >> This is all in one commit to maintain bisectability.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > Applied to u-boot/master, thanks!
>
> This patch breaks the serial console on at least NVIDIA Tegra
> Seaboard. LCD/usbkbd still work though. I can't test any other Tegra
> boards right now since I'm not at work. I wonder if the UART/SPI GPIO
> is at issue? That would be specific to this board.
>

Could be - what do you have it set to? I should be able to give it
another test sometime today and nut it out.

> (I just happened to test with that board while investigating why
> usbkbd was not working right on the RPi, since Seaboard has usbkbd
> support... Yay nested bugs!)

The worst type.

Regards,
Simon

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Stephen Warren]

On 03/09/2015 11:04 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 8 March 2015 at 15:00, Stephen Warren <swarren@nvidia.com> wrote:
>>
>> On 03/04/2015 03:16 PM, Tom Rini wrote:
>>> On Tue, Mar 03, 2015 at 08:02:59AM -0700, Simon Glass wrote:
>>>
>>>> Use the full driver model GPIO and serial drivers in SPL now that
>>>> these are supported. Since device tree is not available they will
>>>> use platform data.
>>>>
>>>> Remove the special SPL GPIO function as it is no longer needed.
>>>>
>>>> This is all in one commit to maintain bisectability.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>
>>> Applied to u-boot/master, thanks!
>>
>> This patch breaks the serial console on at least NVIDIA Tegra
>> Seaboard. LCD/usbkbd still work though. I can't test any other Tegra
>> boards right now since I'm not at work. I wonder if the UART/SPI GPIO
>> is at issue? That would be specific to this board.
>
> Could be - what do you have it set to? I should be able to give it
> another test sometime today and nut it out.

I assume it must be set to "GPIO control" (especially since a 
GPIO-related change broke it). IIRC it came this way, and since the 
board boots from NAND and hence SPI isn't useful, I was going to change 
it to "always UART". However, when I attempted to modify my other 
Seaboard (actually, Springbank) that way, I broke one of the pads on the 
PCB. Luckily that left the board in the "always UART" state that I 
wanted, even without any pull-ups/downs present. Anyway, I decided not 
to touch my other board for fear of breaking it in a way that didn't 
work at all!

[U-Boot] [PATCH v6 3/6] dm: tegra: Enable driver model in SPL and adjust the GPIO driver

[Simon Glass]

Hi Stephen,

On 9 March 2015 at 11:23, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/09/2015 11:04 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 8 March 2015 at 15:00, Stephen Warren <swarren@nvidia.com> wrote:
>>>
>>>
>>> On 03/04/2015 03:16 PM, Tom Rini wrote:
>>>>
>>>> On Tue, Mar 03, 2015 at 08:02:59AM -0700, Simon Glass wrote:
>>>>
>>>>> Use the full driver model GPIO and serial drivers in SPL now that
>>>>> these are supported. Since device tree is not available they will
>>>>> use platform data.
>>>>>
>>>>> Remove the special SPL GPIO function as it is no longer needed.
>>>>>
>>>>> This is all in one commit to maintain bisectability.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>
>>>>
>>>> Applied to u-boot/master, thanks!
>>>
>>>
>>> This patch breaks the serial console on at least NVIDIA Tegra
>>> Seaboard. LCD/usbkbd still work though. I can't test any other Tegra
>>> boards right now since I'm not at work. I wonder if the UART/SPI GPIO
>>> is at issue? That would be specific to this board.
>>
>>
>> Could be - what do you have it set to? I should be able to give it
>> another test sometime today and nut it out.
>
>
> I assume it must be set to "GPIO control" (especially since a GPIO-related
> change broke it). IIRC it came this way, and since the board boots from NAND
> and hence SPI isn't useful, I was going to change it to "always UART".
> However, when I attempted to modify my other Seaboard (actually, Springbank)
> that way, I broke one of the pads on the PCB. Luckily that left the board in
> the "always UART" state that I wanted, even without any pull-ups/downs
> present. Anyway, I decided not to touch my other board for fear of breaking
> it in a way that didn't work at all!

Yes that's it. I must have tested it with the switch in the other
position. I sent a patch to fix it - the problem is that
gpio_request() needs a GPIO name now.

Regards,
Simon