All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/9] Remove use of gdata for global_data
@ 2014-12-23 19:04 Simon Glass
  2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
                   ` (10 more replies)
  0 siblings, 11 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

Some ARM boards use global_data in SPL before it set up by crt0.S. To
achieve this they use a separate global_data variable called gdata which
resides in the data section. The one set up by crt0.S is generally ignored.

This prevents crt0.S from setting up things like the early malloc() pool.
It therefore prevents driver model from being used in SPL.

However gdata really isn't needed. In fact lowlevel_init() is called just
before board_init_f() so, for SPL at least, there is no point in doing
anything before board_init_f(). The one slightly messy area is that SPL
may want to move the stack from SRAM to SDRAM at some point. But this should
be done at the end of board_init_f() (or before board_init_r() is called)
and is not a reason to init DRAM before board_init_f().

It isn't that difficult to get rid of gdata. This series builds on Tom Rini's
recent series for omap3, and extends it to the other offenders: imx, sunxi
and zynq.

I have tested so far only on sunxi. This series is available at u-boot-dm
branch 'gd-working'.


Simon Glass (9):
  arm: Add warnings about using gdata
  sunxi: Move SPL s_init() code to board_init_f()
  sunxi: Drop use of lowlevel_init()
  arm: Reduce the scope of lowlevel_init()
  zynq: Remove reference to gdata
  imx: cm_fx6: Remove reference to gdata
  imx: woodburn: Remove reference to gdata
  imx: ls102xa: Remove reference to gdata
  arm: Drop gdata global_data variable in SPL

 arch/arm/cpu/armv7/lowlevel_init.S      | 23 +++++++-----
 arch/arm/cpu/armv7/sunxi/board.c        | 66 ++++++++++++++++-----------------
 arch/arm/cpu/armv7/zynq/spl.c           |  3 --
 arch/arm/include/asm/spl.h              |  2 -
 arch/arm/lib/spl.c                      | 11 +++---
 board/compulab/cm_fx6/spl.c             |  1 -
 board/freescale/ls1021aqds/ls1021aqds.c |  3 --
 board/freescale/ls1021atwr/ls1021atwr.c |  3 --
 board/woodburn/woodburn.c               |  3 --
 include/configs/sunxi-common.h          |  1 +
 10 files changed, 53 insertions(+), 63 deletions(-)

-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-24  6:53   ` Igor Grinberg
  2015-01-20 21:40   ` [U-Boot] [U-Boot,1/9] " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f() Simon Glass
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

We need to get rid of this SPL-specific setting of the global_data pointer.
It is already set up in start.S immediately before board_init_f() is called,
and there may be information there that is needed (e.g. pre-reloc malloc
info).

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

 arch/arm/lib/spl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index dfcc596..c41850a 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -15,6 +15,11 @@
 
 /* Pointer to as well as the global data structure for SPL */
 DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * WARNING: This is going away very soon. Don't use it and don't submit
+ * pafches that rely on it. The global_data area is set up in crt0.S.
+ */
 gd_t gdata __attribute__ ((section(".data")));
 
 /*
@@ -28,7 +33,7 @@ void __weak board_init_f(ulong dummy)
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
-	/* Set global data pointer. */
+	/* TODO: Remove settings of the global data pointer here */
 	gd = &gdata;
 
 	board_init_r(NULL, 0);
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
  2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-28  9:19   ` Ian Campbell
  2015-01-20 21:40   ` [U-Boot] [U-Boot, " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init() Simon Glass
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

The current sunxi implementation uses gdata, which is going away. It also
sets up DRAM before board_init_f() in SPL.

There is really no reason to do much in s_init() since board_init_f() is
called immediately afterwards. The only change is that we need our own
implementation of board_init_f() which sets up DRAM before the BSS (which
is in DRAM) is cleared.

The s_init() code runs once for SPL and again for U-Boot proper. We
shouldn't need to init the clock/timer/gpio/i2c init twice, so just have it
in SPL.

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

 arch/arm/cpu/armv7/sunxi/board.c | 67 +++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
index 9b3e80c..7831ac9 100644
--- a/arch/arm/cpu/armv7/sunxi/board.c
+++ b/arch/arm/cpu/armv7/sunxi/board.c
@@ -46,9 +46,8 @@ u32 spl_boot_mode(void)
 {
 	return MMCSD_MODE_RAW;
 }
-#endif
 
-int gpio_init(void)
+static int gpio_init(void)
 {
 #if CONFIG_CONS_INDEX == 1 && defined(CONFIG_UART0_PORT_F)
 #if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN7I)
@@ -86,6 +85,41 @@ int gpio_init(void)
 	return 0;
 }
 
+void board_init_f(ulong dummy)
+{
+#ifdef CONFIG_MACH_SUN6I
+	/* Magic (undocmented) value taken from boot0, without this DRAM
+	 * access gets messed up (seems cache related) */
+	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
+#endif
+#if !defined CONFIG_SPL_BUILD && (defined CONFIG_MACH_SUN7I || \
+		defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I)
+	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
+	asm volatile(
+		"mrc p15, 0, r0, c1, c0, 1\n"
+		"orr r0, r0, #1 << 6\n"
+		"mcr p15, 0, r0, c1, c0, 1\n");
+#endif
+	clock_init();
+	timer_init();
+	gpio_init();
+	i2c_init_board();
+
+	preloader_console_init();
+
+#ifdef CONFIG_SPL_I2C_SUPPORT
+	/* Needed early by sunxi_board_init if PMU is enabled */
+	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
+#endif
+	sunxi_board_init();
+
+	/* Clear the BSS. */
+	memset(__bss_start, 0, __bss_end - __bss_start);
+
+	board_init_r(NULL, 0);
+}
+#endif
+
 void reset_cpu(ulong addr)
 {
 #if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUN7I)
@@ -114,35 +148,6 @@ void reset_cpu(ulong addr)
 /* do some early init */
 void s_init(void)
 {
-#if defined CONFIG_SPL_BUILD && defined CONFIG_MACH_SUN6I
-	/* Magic (undocmented) value taken from boot0, without this DRAM
-	 * access gets messed up (seems cache related) */
-	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
-#endif
-#if !defined CONFIG_SPL_BUILD && (defined CONFIG_MACH_SUN7I || \
-		defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I)
-	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
-	asm volatile(
-		"mrc p15, 0, r0, c1, c0, 1\n"
-		"orr r0, r0, #1 << 6\n"
-		"mcr p15, 0, r0, c1, c0, 1\n");
-#endif
-
-	clock_init();
-	timer_init();
-	gpio_init();
-	i2c_init_board();
-
-#ifdef CONFIG_SPL_BUILD
-	gd = &gdata;
-	preloader_console_init();
-
-#ifdef CONFIG_SPL_I2C_SUPPORT
-	/* Needed early by sunxi_board_init if PMU is enabled */
-	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
-#endif
-	sunxi_board_init();
-#endif
 }
 
 #ifndef CONFIG_SYS_DCACHE_OFF
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init()
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
  2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
  2014-12-23 19:04 ` [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f() Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-28  9:10   ` Ian Campbell
  2015-01-20 21:40   ` [U-Boot] [U-Boot,3/9] " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 4/9] arm: Reduce the scope " Simon Glass
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

This does nothing now, so drop it. We have SPL anyway to do our low-level
init.

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

 arch/arm/cpu/armv7/sunxi/board.c | 5 -----
 include/configs/sunxi-common.h   | 1 +
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
index 7831ac9..73c40ab 100644
--- a/arch/arm/cpu/armv7/sunxi/board.c
+++ b/arch/arm/cpu/armv7/sunxi/board.c
@@ -145,11 +145,6 @@ void reset_cpu(ulong addr)
 #endif
 }
 
-/* do some early init */
-void s_init(void)
-{
-}
-
 #ifndef CONFIG_SYS_DCACHE_OFF
 void enable_caches(void)
 {
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 3f890b2..a4daeca 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -109,6 +109,7 @@
 #define CONFIG_SYS_PBSIZE	1024	/* Print Buffer Size */
 #define CONFIG_SYS_MAXARGS	16	/* max number of command args */
 #define CONFIG_SYS_GENERIC_BOARD
+#define CONFIG_SKIP_LOWLEVEL_INIT
 
 /* Boot Argument Buffer Size */
 #define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 4/9] arm: Reduce the scope of lowlevel_init()
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (2 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init() Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-23 19:04 ` [U-Boot] [PATCH 5/9] zynq: Remove reference to gdata Simon Glass
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

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

 arch/arm/cpu/armv7/lowlevel_init.S | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/arm/cpu/armv7/lowlevel_init.S b/arch/arm/cpu/armv7/lowlevel_init.S
index f1aea05..4803150 100644
--- a/arch/arm/cpu/armv7/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/lowlevel_init.S
@@ -17,24 +17,27 @@
 
 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_SPL_BUILD
-	ldr	r9, =gdata
-#else
-	sub	sp, sp, #GD_SIZE
-	bic	sp, sp, #7
-	mov	r9, sp
-#endif
+	mov	r9, #0
 	/*
 	 * 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

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

* [U-Boot] [PATCH 5/9] zynq: Remove reference to gdata
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (3 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 4/9] arm: Reduce the scope " Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2015-01-20 21:40   ` [U-Boot] [U-Boot,5/9] " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

The global_data pointer (gd) has already been set before board_init_f()
is called. We should not assign it again. We should also not use gdata since
it is going away.

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

 arch/arm/cpu/armv7/zynq/spl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/cpu/armv7/zynq/spl.c b/arch/arm/cpu/armv7/zynq/spl.c
index 31627f9..0936bdd 100644
--- a/arch/arm/cpu/armv7/zynq/spl.c
+++ b/arch/arm/cpu/armv7/zynq/spl.c
@@ -20,9 +20,6 @@ void board_init_f(ulong dummy)
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
-	/* Set global data pointer. */
-	gd = &gdata;
-
 	preloader_console_init();
 	arch_cpu_init();
 	board_init_r(NULL, 0);
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 6/9] imx: cm_fx6: Remove reference to gdata
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (4 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 5/9] zynq: Remove reference to gdata Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-24  6:50   ` Igor Grinberg
                     ` (2 more replies)
  2014-12-23 19:04 ` [U-Boot] [PATCH 7/9] imx: woodburn: " Simon Glass
                   ` (4 subsequent siblings)
  10 siblings, 3 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

The global_data pointer (gd) has already been set before board_init_f()
is called. We should not assign it again. We should also not use gdata since
it is going away.

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

 board/compulab/cm_fx6/spl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/board/compulab/cm_fx6/spl.c b/board/compulab/cm_fx6/spl.c
index 6fe937b..5b4b76f 100644
--- a/board/compulab/cm_fx6/spl.c
+++ b/board/compulab/cm_fx6/spl.c
@@ -313,7 +313,6 @@ void board_init_f(ulong dummy)
 {
 	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
 
-	gd = &gdata;
 	/*
 	 * We don't use DMA in SPL, but we do need it in U-Boot. U-Boot
 	 * initializes DMA very early (before all board code), so the only
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 7/9] imx: woodburn: Remove reference to gdata
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (5 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-30 12:10   ` Stefano Babic
  2015-01-20 21:40   ` [U-Boot] [U-Boot,7/9] " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 8/9] imx: ls102xa: " Simon Glass
                   ` (3 subsequent siblings)
  10 siblings, 2 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

The global_data pointer (gd) has already been set before board_init_f()
is called. We should not assign it again. We should also not use gdata since
it is going away.

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

 board/woodburn/woodburn.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/board/woodburn/woodburn.c b/board/woodburn/woodburn.c
index 2744514..3da61a4 100644
--- a/board/woodburn/woodburn.c
+++ b/board/woodburn/woodburn.c
@@ -137,9 +137,6 @@ void board_init_f(ulong dummy)
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
-	/* Set global data pointer. */
-	gd = &gdata;
-
 	preloader_console_init();
 	timer_init();
 
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 8/9] imx: ls102xa: Remove reference to gdata
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (6 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 7/9] imx: woodburn: " Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2015-01-20 21:40   ` [U-Boot] [U-Boot,8/9] " Tom Rini
  2014-12-23 19:04 ` [U-Boot] [PATCH 9/9] arm: Drop gdata global_data variable in SPL Simon Glass
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

The global_data pointer (gd) has already been set before board_init_f()
is called. We should not assign it again. We should also not use gdata since
it is going away.

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

 board/freescale/ls1021aqds/ls1021aqds.c | 3 ---
 board/freescale/ls1021atwr/ls1021atwr.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/board/freescale/ls1021aqds/ls1021aqds.c b/board/freescale/ls1021aqds/ls1021aqds.c
index f08e54f..152da2d 100644
--- a/board/freescale/ls1021aqds/ls1021aqds.c
+++ b/board/freescale/ls1021aqds/ls1021aqds.c
@@ -219,9 +219,6 @@ void board_init_f(ulong dummy)
 		 pinctl);
 #endif
 
-	/* Set global data pointer */
-	gd = &gdata;
-
 	/* Clear the BSS */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
diff --git a/board/freescale/ls1021atwr/ls1021atwr.c b/board/freescale/ls1021atwr/ls1021atwr.c
index 8ab229d..027b67e 100644
--- a/board/freescale/ls1021atwr/ls1021atwr.c
+++ b/board/freescale/ls1021atwr/ls1021atwr.c
@@ -287,9 +287,6 @@ int board_early_init_f(void)
 #ifdef CONFIG_SPL_BUILD
 void board_init_f(ulong dummy)
 {
-	/* Set global data pointer */
-	gd = &gdata;
-
 	/* Clear the BSS */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 9/9] arm: Drop gdata global_data variable in SPL
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (7 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 8/9] imx: ls102xa: " Simon Glass
@ 2014-12-23 19:04 ` Simon Glass
  2014-12-28  9:09 ` [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Ian Campbell
  2015-01-19  6:48 ` Albert ARIBAUD
  10 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-23 19:04 UTC (permalink / raw)
  To: u-boot

In SPL as in U-Boot proper, global_data should only be set up in one place
which is crt0.S. We should not use on global_data before board_init_f().
The pointer does not exist.

In fact there is no need for this anyway and it just contorts the early init
code for SPL. Drop it.

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

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

diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h
index 8acd7cd..e5dbab0 100644
--- a/arch/arm/include/asm/spl.h
+++ b/arch/arm/include/asm/spl.h
@@ -33,6 +33,4 @@ enum {
 /* Linker symbols. */
 extern char __bss_start[], __bss_end[];
 
-extern gd_t gdata;
-
 #endif
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index c41850a..f130fa3 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -17,25 +17,21 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 /*
- * WARNING: This is going away very soon. Don't use it and don't submit
- * pafches that rely on it. The global_data area is set up in crt0.S.
- */
-gd_t gdata __attribute__ ((section(".data")));
-
-/*
  * In the context of SPL, board_init_f must ensure that any clocks/etc for
  * DDR are enabled, ensure that the stack pointer is valid, clear the BSS
- * and call board_init_f.  We provide this version by default but mark it
+ * and call board_init_r.  We provide this version by default but mark it
  * as __weak to allow for platforms to do this in their own way if needed.
+ *
+ * Note that DRAM will not yet be set up, so this version will not normally
+ * be useful if BSS is located in DRAM.
  */
 void __weak board_init_f(ulong dummy)
 {
+	/* If overriding this function, set up DRAM here */
+
 	/* Clear the BSS. */
 	memset(__bss_start, 0, __bss_end - __bss_start);
 
-	/* TODO: Remove settings of the global data pointer here */
-	gd = &gdata;
-
 	board_init_r(NULL, 0);
 }
 
-- 
2.2.0.rc0.207.ga3a616c

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

* [U-Boot] [PATCH 6/9] imx: cm_fx6: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
@ 2014-12-24  6:50   ` Igor Grinberg
  2014-12-31 12:25   ` Nikita Kiryanov
  2015-01-20 21:40   ` [U-Boot] [U-Boot,6/9] " Tom Rini
  2 siblings, 0 replies; 41+ messages in thread
From: Igor Grinberg @ 2014-12-24  6:50 UTC (permalink / raw)
  To: u-boot

On 12/23/14 21:04, Simon Glass wrote:
> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

> ---
> 
>  board/compulab/cm_fx6/spl.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/board/compulab/cm_fx6/spl.c b/board/compulab/cm_fx6/spl.c
> index 6fe937b..5b4b76f 100644
> --- a/board/compulab/cm_fx6/spl.c
> +++ b/board/compulab/cm_fx6/spl.c
> @@ -313,7 +313,6 @@ void board_init_f(ulong dummy)
>  {
>  	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
>  
> -	gd = &gdata;
>  	/*
>  	 * We don't use DMA in SPL, but we do need it in U-Boot. U-Boot
>  	 * initializes DMA very early (before all board code), so the only
> 

-- 
Regards,
Igor.

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

* [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
@ 2014-12-24  6:53   ` Igor Grinberg
  2014-12-29 16:24     ` Simon Glass
  2015-01-20 21:40   ` [U-Boot] [U-Boot,1/9] " Tom Rini
  1 sibling, 1 reply; 41+ messages in thread
From: Igor Grinberg @ 2014-12-24  6:53 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 12/23/14 21:04, Simon Glass wrote:
> We need to get rid of this SPL-specific setting of the global_data pointer.
> It is already set up in start.S immediately before board_init_f() is called,
> and there may be information there that is needed (e.g. pre-reloc malloc
> info).
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/arm/lib/spl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
> index dfcc596..c41850a 100644
> --- a/arch/arm/lib/spl.c
> +++ b/arch/arm/lib/spl.c
> @@ -15,6 +15,11 @@
>  
>  /* Pointer to as well as the global data structure for SPL */
>  DECLARE_GLOBAL_DATA_PTR;
> +
> +/*
> + * WARNING: This is going away very soon. Don't use it and don't submit
> + * pafches that rely on it. The global_data area is set up in crt0.S.
> + */
>  gd_t gdata __attribute__ ((section(".data")));
>  
>  /*
> @@ -28,7 +33,7 @@ void __weak board_init_f(ulong dummy)
>  	/* Clear the BSS. */
>  	memset(__bss_start, 0, __bss_end - __bss_start);
>  
> -	/* Set global data pointer. */
> +	/* TODO: Remove settings of the global data pointer here */

Why do you need this patch at all if you remove this stuff in 9/9?

>  	gd = &gdata;
>  
>  	board_init_r(NULL, 0);
> 

-- 
Regards,
Igor.

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

* [U-Boot] [PATCH 0/9] Remove use of gdata for global_data
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (8 preceding siblings ...)
  2014-12-23 19:04 ` [U-Boot] [PATCH 9/9] arm: Drop gdata global_data variable in SPL Simon Glass
@ 2014-12-28  9:09 ` Ian Campbell
  2015-01-19  6:48 ` Albert ARIBAUD
  10 siblings, 0 replies; 41+ messages in thread
From: Ian Campbell @ 2014-12-28  9:09 UTC (permalink / raw)
  To: u-boot

On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
> Some ARM boards use global_data in SPL before it set up by crt0.S. To
> achieve this they use a separate global_data variable called gdata which
> resides in the data section. The one set up by crt0.S is generally ignored.
> 
> This prevents crt0.S from setting up things like the early malloc() pool.
> It therefore prevents driver model from being used in SPL.
> 
> However gdata really isn't needed. In fact lowlevel_init() is called just
> before board_init_f() so, for SPL at least, there is no point in doing
> anything before board_init_f(). The one slightly messy area is that SPL
> may want to move the stack from SRAM to SDRAM at some point. But this should
> be done at the end of board_init_f() (or before board_init_r() is called)
> and is not a reason to init DRAM before board_init_f().
> 
> It isn't that difficult to get rid of gdata. This series builds on Tom Rini's
> recent series for omap3, and extends it to the other offenders: imx, sunxi
> and zynq.
> 
> I have tested so far only on sunxi.

Did you test both FEL and regular mode? (IOW, to what extent do I need
to refresh my memory on the differences between the two...)


>  This series is available at u-boot-dm
> branch 'gd-working'.
> 
> 
> Simon Glass (9):
>   arm: Add warnings about using gdata
>   sunxi: Move SPL s_init() code to board_init_f()
>   sunxi: Drop use of lowlevel_init()
>   arm: Reduce the scope of lowlevel_init()
>   zynq: Remove reference to gdata
>   imx: cm_fx6: Remove reference to gdata
>   imx: woodburn: Remove reference to gdata
>   imx: ls102xa: Remove reference to gdata
>   arm: Drop gdata global_data variable in SPL
> 
>  arch/arm/cpu/armv7/lowlevel_init.S      | 23 +++++++-----
>  arch/arm/cpu/armv7/sunxi/board.c        | 66 ++++++++++++++++-----------------
>  arch/arm/cpu/armv7/zynq/spl.c           |  3 --
>  arch/arm/include/asm/spl.h              |  2 -
>  arch/arm/lib/spl.c                      | 11 +++---
>  board/compulab/cm_fx6/spl.c             |  1 -
>  board/freescale/ls1021aqds/ls1021aqds.c |  3 --
>  board/freescale/ls1021atwr/ls1021atwr.c |  3 --
>  board/woodburn/woodburn.c               |  3 --
>  include/configs/sunxi-common.h          |  1 +
>  10 files changed, 53 insertions(+), 63 deletions(-)
> 

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

* [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init()
  2014-12-23 19:04 ` [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init() Simon Glass
@ 2014-12-28  9:10   ` Ian Campbell
  2015-01-20 21:40   ` [U-Boot] [U-Boot,3/9] " Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Ian Campbell @ 2014-12-28  9:10 UTC (permalink / raw)
  To: u-boot

On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
> This does nothing now, so drop it. We have SPL anyway to do our low-level
> init.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Ian Campbell <ijc@hellion.org.uk>

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2014-12-23 19:04 ` [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f() Simon Glass
@ 2014-12-28  9:19   ` Ian Campbell
  2014-12-29 16:15     ` Simon Glass
  2015-01-20 21:40   ` [U-Boot] [U-Boot, " Tom Rini
  1 sibling, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-12-28  9:19 UTC (permalink / raw)
  To: u-boot

On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:

> +void board_init_f(ulong dummy)
> +{
[...]
> +	/* Clear the BSS. */
> +	memset(__bss_start, 0, __bss_end - __bss_start);
> +
> +	board_init_r(NULL, 0);

The previous (__weak) version of board_init_f also sets gd, which you've
also removed from s_init here and not added back anywhere (indeed, this
is the point...). But where is gd initialised now?

The patch generally looks good, two quick questions: has it been tested
in both FEL and regular mode, and has it been tested with a "legacy" as
well as a driver model system? (I might be able to find time in a day or
two to answer these myself, but for now I'll just ask).

Ian.

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2014-12-28  9:19   ` Ian Campbell
@ 2014-12-29 16:15     ` Simon Glass
  2015-01-30 17:53       ` Siarhei Siamashka
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2014-12-29 16:15 UTC (permalink / raw)
  To: u-boot

Hi Ian,

On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
>
>> +void board_init_f(ulong dummy)
>> +{
> [...]
>> +     /* Clear the BSS. */
>> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> +
>> +     board_init_r(NULL, 0);
>
> The previous (__weak) version of board_init_f also sets gd, which you've
> also removed from s_init here and not added back anywhere (indeed, this
> is the point...). But where is gd initialised now?

It's still in start.S, I've just removed this duplicate.

>
> The patch generally looks good, two quick questions: has it been tested
> in both FEL and regular mode, and has it been tested with a "legacy" as
> well as a driver model system? (I might be able to find time in a day or
> two to answer these myself, but for now I'll just ask).

I haven't tried FEL, I only just heard of it in your email. I'll see
if I can figure out how to test that.

I don't think this patch affects any drivers, but I can test that too.

Regards,
Simon

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

* [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata
  2014-12-24  6:53   ` Igor Grinberg
@ 2014-12-29 16:24     ` Simon Glass
  2014-12-30  7:39       ` Igor Grinberg
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2014-12-29 16:24 UTC (permalink / raw)
  To: u-boot

Hi Igor,

On 23 December 2014 at 23:53, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 12/23/14 21:04, Simon Glass wrote:
>> We need to get rid of this SPL-specific setting of the global_data pointer.
>> It is already set up in start.S immediately before board_init_f() is called,
>> and there may be information there that is needed (e.g. pre-reloc malloc
>> info).
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  arch/arm/lib/spl.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
>> index dfcc596..c41850a 100644
>> --- a/arch/arm/lib/spl.c
>> +++ b/arch/arm/lib/spl.c
>> @@ -15,6 +15,11 @@
>>
>>  /* Pointer to as well as the global data structure for SPL */
>>  DECLARE_GLOBAL_DATA_PTR;
>> +
>> +/*
>> + * WARNING: This is going away very soon. Don't use it and don't submit
>> + * pafches that rely on it. The global_data area is set up in crt0.S.
>> + */
>>  gd_t gdata __attribute__ ((section(".data")));
>>
>>  /*
>> @@ -28,7 +33,7 @@ void __weak board_init_f(ulong dummy)
>>       /* Clear the BSS. */
>>       memset(__bss_start, 0, __bss_end - __bss_start);
>>
>> -     /* Set global data pointer. */
>> +     /* TODO: Remove settings of the global data pointer here */
>
> Why do you need this patch at all if you remove this stuff in 9/9?

I imagine that 9/9 might take some time to be applied, since it needs
testing, so I've put that in as a clean-up patch.
>
>>       gd = &gdata;
>>
>>       board_init_r(NULL, 0);

Regards,
Simon

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

* [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata
  2014-12-29 16:24     ` Simon Glass
@ 2014-12-30  7:39       ` Igor Grinberg
  2014-12-30 22:12         ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Igor Grinberg @ 2014-12-30  7:39 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 12/29/14 18:24, Simon Glass wrote:
> Hi Igor,
> 
> On 23 December 2014 at 23:53, Igor Grinberg <grinberg@compulab.co.il> wrote:
>> Hi Simon,
>>
>> On 12/23/14 21:04, Simon Glass wrote:
>>> We need to get rid of this SPL-specific setting of the global_data pointer.
>>> It is already set up in start.S immediately before board_init_f() is called,
>>> and there may be information there that is needed (e.g. pre-reloc malloc
>>> info).
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>  arch/arm/lib/spl.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
>>> index dfcc596..c41850a 100644
>>> --- a/arch/arm/lib/spl.c
>>> +++ b/arch/arm/lib/spl.c
>>> @@ -15,6 +15,11 @@
>>>
>>>  /* Pointer to as well as the global data structure for SPL */
>>>  DECLARE_GLOBAL_DATA_PTR;
>>> +
>>> +/*
>>> + * WARNING: This is going away very soon. Don't use it and don't submit
>>> + * pafches that rely on it. The global_data area is set up in crt0.S.
>>> + */
>>>  gd_t gdata __attribute__ ((section(".data")));
>>>
>>>  /*
>>> @@ -28,7 +33,7 @@ void __weak board_init_f(ulong dummy)
>>>       /* Clear the BSS. */
>>>       memset(__bss_start, 0, __bss_end - __bss_start);
>>>
>>> -     /* Set global data pointer. */
>>> +     /* TODO: Remove settings of the global data pointer here */
>>
>> Why do you need this patch at all if you remove this stuff in 9/9?
> 
> I imagine that 9/9 might take some time to be applied, since it needs
> testing, so I've put that in as a clean-up patch.

I personally, like this patch set and think we should move forward with it.
We'll give it a try (hopefully this week), but I don't think it should be
merged before the next merge window.
Is this (1/9) patch intended to go in during the rc?

>>
>>>       gd = &gdata;
>>>
>>>       board_init_r(NULL, 0);
> 
> Regards,
> Simon
> 

-- 
Regards,
Igor.

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

* [U-Boot] [PATCH 7/9] imx: woodburn: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 7/9] imx: woodburn: " Simon Glass
@ 2014-12-30 12:10   ` Stefano Babic
  2015-01-20 21:40   ` [U-Boot] [U-Boot,7/9] " Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Stefano Babic @ 2014-12-30 12:10 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 23/12/2014 20:04, Simon Glass wrote:
> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  board/woodburn/woodburn.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/board/woodburn/woodburn.c b/board/woodburn/woodburn.c
> index 2744514..3da61a4 100644
> --- a/board/woodburn/woodburn.c
> +++ b/board/woodburn/woodburn.c
> @@ -137,9 +137,6 @@ void board_init_f(ulong dummy)
>  	/* Clear the BSS. */
>  	memset(__bss_start, 0, __bss_end - __bss_start);
>  
> -	/* Set global data pointer. */
> -	gd = &gdata;
> -
>  	preloader_console_init();
>  	timer_init();
>  

Agree on this concept.

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic



-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

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

* [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata
  2014-12-30  7:39       ` Igor Grinberg
@ 2014-12-30 22:12         ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2014-12-30 22:12 UTC (permalink / raw)
  To: u-boot

Hi Igor,

On 30 December 2014 at 00:39, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 12/29/14 18:24, Simon Glass wrote:
>> Hi Igor,
>>
>> On 23 December 2014 at 23:53, Igor Grinberg <grinberg@compulab.co.il> wrote:
>>> Hi Simon,
>>>
>>> On 12/23/14 21:04, Simon Glass wrote:
>>>> We need to get rid of this SPL-specific setting of the global_data pointer.
>>>> It is already set up in start.S immediately before board_init_f() is called,
>>>> and there may be information there that is needed (e.g. pre-reloc malloc
>>>> info).
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>>  arch/arm/lib/spl.c | 7 ++++++-
>>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
>>>> index dfcc596..c41850a 100644
>>>> --- a/arch/arm/lib/spl.c
>>>> +++ b/arch/arm/lib/spl.c
>>>> @@ -15,6 +15,11 @@
>>>>
>>>>  /* Pointer to as well as the global data structure for SPL */
>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +/*
>>>> + * WARNING: This is going away very soon. Don't use it and don't submit
>>>> + * pafches that rely on it. The global_data area is set up in crt0.S.
>>>> + */
>>>>  gd_t gdata __attribute__ ((section(".data")));
>>>>
>>>>  /*
>>>> @@ -28,7 +33,7 @@ void __weak board_init_f(ulong dummy)
>>>>       /* Clear the BSS. */
>>>>       memset(__bss_start, 0, __bss_end - __bss_start);
>>>>
>>>> -     /* Set global data pointer. */
>>>> +     /* TODO: Remove settings of the global data pointer here */
>>>
>>> Why do you need this patch at all if you remove this stuff in 9/9?
>>
>> I imagine that 9/9 might take some time to be applied, since it needs
>> testing, so I've put that in as a clean-up patch.
>
> I personally, like this patch set and think we should move forward with it.
> We'll give it a try (hopefully this week), but I don't think it should be
> merged before the next merge window.
> Is this (1/9) patch intended to go in during the rc?

There's not enough time to test/fix this, so it would be better to
apply it when the merge window opens. Then any breakage can be dealt
with. At least this time we have the patches (this series plus Tom's
patches) so the plan should work.

Regards,
Simon

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

* [U-Boot] [PATCH 6/9] imx: cm_fx6: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
  2014-12-24  6:50   ` Igor Grinberg
@ 2014-12-31 12:25   ` Nikita Kiryanov
  2015-01-20 21:40   ` [U-Boot] [U-Boot,6/9] " Tom Rini
  2 siblings, 0 replies; 41+ messages in thread
From: Nikita Kiryanov @ 2014-12-31 12:25 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 12/23/2014 09:04 PM, Simon Glass wrote:
> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Tested-by: Nikita Kiryanov <nikita@compulab.co.il>
Acked-by: Nikita Kiryanov <nikita@compulab.co.il>

-- 
Regards,
Nikita Kiryanov

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

* [U-Boot] [PATCH 0/9] Remove use of gdata for global_data
  2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
                   ` (9 preceding siblings ...)
  2014-12-28  9:09 ` [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Ian Campbell
@ 2015-01-19  6:48 ` Albert ARIBAUD
  10 siblings, 0 replies; 41+ messages in thread
From: Albert ARIBAUD @ 2015-01-19  6:48 UTC (permalink / raw)
  To: u-boot

Hello Simon,

On Tue, 23 Dec 2014 12:04:50 -0700, Simon Glass <sjg@chromium.org>
wrote:
> Some ARM boards use global_data in SPL before it set up by crt0.S. To
> achieve this they use a separate global_data variable called gdata which
> resides in the data section. The one set up by crt0.S is generally ignored.
> 
> This prevents crt0.S from setting up things like the early malloc() pool.
> It therefore prevents driver model from being used in SPL.
> 
> However gdata really isn't needed. In fact lowlevel_init() is called just
> before board_init_f() so, for SPL at least, there is no point in doing
> anything before board_init_f(). The one slightly messy area is that SPL
> may want to move the stack from SRAM to SDRAM at some point. But this should
> be done at the end of board_init_f() (or before board_init_r() is called)
> and is not a reason to init DRAM before board_init_f().
> 
> It isn't that difficult to get rid of gdata. This series builds on Tom Rini's
> recent series for omap3, and extends it to the other offenders: imx, sunxi
> and zynq.
> 
> I have tested so far only on sunxi. This series is available at u-boot-dm
> branch 'gd-working'.

Overall series:

Acked-by: Albert ARIBAUD <albert.u.boot@aribaud.net>

Amicalement,
-- 
Albert.

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

* [U-Boot] [U-Boot,1/9] arm: Add warnings about using gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
  2014-12-24  6:53   ` Igor Grinberg
@ 2015-01-20 21:40   ` Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:51PM -0700, Simon Glass wrote:

> We need to get rid of this SPL-specific setting of the global_data pointer.
> It is already set up in start.S immediately before board_init_f() is called,
> and there may be information there that is needed (e.g. pre-reloc malloc
> info).
> 
> 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/20150120/4573794d/attachment.pgp>

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

* [U-Boot] [U-Boot, 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2014-12-23 19:04 ` [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f() Simon Glass
  2014-12-28  9:19   ` Ian Campbell
@ 2015-01-20 21:40   ` Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:52PM -0700, Simon Glass wrote:

> The current sunxi implementation uses gdata, which is going away. It also
> sets up DRAM before board_init_f() in SPL.
> 
> There is really no reason to do much in s_init() since board_init_f() is
> called immediately afterwards. The only change is that we need our own
> implementation of board_init_f() which sets up DRAM before the BSS (which
> is in DRAM) is cleared.
> 
> The s_init() code runs once for SPL and again for U-Boot proper. We
> shouldn't need to init the clock/timer/gpio/i2c init twice, so just have it
> in SPL.
> 
> 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/20150120/95a683d5/attachment.pgp>

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

* [U-Boot] [U-Boot,3/9] sunxi: Drop use of lowlevel_init()
  2014-12-23 19:04 ` [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init() Simon Glass
  2014-12-28  9:10   ` Ian Campbell
@ 2015-01-20 21:40   ` Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:53PM -0700, Simon Glass wrote:

> This does nothing now, so drop it. We have SPL anyway to do our low-level
> init.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Ian Campbell <ijc@hellion.org.uk>

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/20150120/f8e65d85/attachment.pgp>

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

* [U-Boot] [U-Boot,5/9] zynq: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 5/9] zynq: Remove reference to gdata Simon Glass
@ 2015-01-20 21:40   ` Tom Rini
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:55PM -0700, Simon Glass wrote:

> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> 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/20150120/d4f6adc8/attachment.pgp>

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

* [U-Boot] [U-Boot,6/9] imx: cm_fx6: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
  2014-12-24  6:50   ` Igor Grinberg
  2014-12-31 12:25   ` Nikita Kiryanov
@ 2015-01-20 21:40   ` Tom Rini
  2 siblings, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:56PM -0700, Simon Glass wrote:

> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> Tested-by: Nikita Kiryanov <nikita@compulab.co.il>
> Acked-by: Nikita Kiryanov <nikita@compulab.co.il>

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/20150120/79bf1c57/attachment.pgp>

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

* [U-Boot] [U-Boot,7/9] imx: woodburn: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 7/9] imx: woodburn: " Simon Glass
  2014-12-30 12:10   ` Stefano Babic
@ 2015-01-20 21:40   ` Tom Rini
  1 sibling, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:57PM -0700, Simon Glass wrote:

> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Acked-by: Stefano Babic <sbabic@denx.de>

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/20150120/ade2160f/attachment.pgp>

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

* [U-Boot] [U-Boot,8/9] imx: ls102xa: Remove reference to gdata
  2014-12-23 19:04 ` [U-Boot] [PATCH 8/9] imx: ls102xa: " Simon Glass
@ 2015-01-20 21:40   ` Tom Rini
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-01-20 21:40 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 23, 2014 at 12:04:58PM -0700, Simon Glass wrote:

> The global_data pointer (gd) has already been set before board_init_f()
> is called. We should not assign it again. We should also not use gdata since
> it is going away.
> 
> 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/20150120/47dbcc44/attachment.pgp>

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2014-12-29 16:15     ` Simon Glass
@ 2015-01-30 17:53       ` Siarhei Siamashka
  2015-02-01 16:29         ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Siarhei Siamashka @ 2015-01-30 17:53 UTC (permalink / raw)
  To: u-boot

On Mon, 29 Dec 2014 09:15:36 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi Ian,
> 
> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
> >
> >> +void board_init_f(ulong dummy)
> >> +{
> > [...]
> >> +     /* Clear the BSS. */
> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> >> +
> >> +     board_init_r(NULL, 0);
> >
> > The previous (__weak) version of board_init_f also sets gd, which you've
> > also removed from s_init here and not added back anywhere (indeed, this
> > is the point...). But where is gd initialised now?
> 
> It's still in start.S, I've just removed this duplicate.
> 
> >
> > The patch generally looks good, two quick questions: has it been tested
> > in both FEL and regular mode, and has it been tested with a "legacy" as
> > well as a driver model system? (I might be able to find time in a day or
> > two to answer these myself, but for now I'll just ask).
> 
> I haven't tried FEL, I only just heard of it in your email. I'll see
> if I can figure out how to test that.

Just like Ian suspected, this patch has messed up the FEL boot mode
support.

In a nutshell, FEL is a special USB protocol (accessible on a USB OTG
connector), which is implemented by the boot ROM and activated by
holding a special hardware button pressed and rebooting the device.
FEL supports commands to read/write device RAM and execute code on
the device. It is designed for device unbricking and firmware recovery.

In particular, the FEL boot mode support is very useful for debugging
u-boot and kernel problems on tablets (the SD card slot can be used
for the UART console, while the system is booted over a micro-USB cable
with the help of FEL):
    http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg

In u-boot it is used in the following way:
 1. The SPL code is uploaded from the linux PC to the device SRAM via
    a FEL command (using the 'fel' program from sunxi-tools).
 2. The SPL code is executed via a FEL command and expected to
    initialize the DRAM controller. The code is executed as a
    normal C or assembly function, which needs to return control
    back to the BROM code when it is done. Right now this
    function is s_init().
 3. As the DRAM is initialized and available now, the main u-boot
    binary is now uploaded to DRAM via FEL. Together with boot.scr,
    the kernel, the dtb file and optionally initramfs as needed.
 4. The main u-boot binary is executed via a FEL command, but never
    returns back to BROM anymore.

More details are available in the linux-sunxi wiki:
    http://linux-sunxi.org/FEL
    http://linux-sunxi.org/FEL/Protocol
    http://linux-sunxi.org/FEL/USBBoot

I have submitted a patch to fix this regression:
    https://patchwork.ozlabs.org/patch/434826/

If you encounter problems and/or need help when testing FEL, please
let me know.

-- 
Best regards,
Siarhei Siamashka

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-01-30 17:53       ` Siarhei Siamashka
@ 2015-02-01 16:29         ` Simon Glass
  2015-02-01 16:45           ` Siarhei Siamashka
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-01 16:29 UTC (permalink / raw)
  To: u-boot

Hi,

On 30 January 2015 at 10:53, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Mon, 29 Dec 2014 09:15:36 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi Ian,
>>
>> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
>> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
>> >
>> >> +void board_init_f(ulong dummy)
>> >> +{
>> > [...]
>> >> +     /* Clear the BSS. */
>> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> >> +
>> >> +     board_init_r(NULL, 0);
>> >
>> > The previous (__weak) version of board_init_f also sets gd, which you've
>> > also removed from s_init here and not added back anywhere (indeed, this
>> > is the point...). But where is gd initialised now?
>>
>> It's still in start.S, I've just removed this duplicate.
>>
>> >
>> > The patch generally looks good, two quick questions: has it been tested
>> > in both FEL and regular mode, and has it been tested with a "legacy" as
>> > well as a driver model system? (I might be able to find time in a day or
>> > two to answer these myself, but for now I'll just ask).
>>
>> I haven't tried FEL, I only just heard of it in your email. I'll see
>> if I can figure out how to test that.
>
> Just like Ian suspected, this patch has messed up the FEL boot mode
> support.
>
> In a nutshell, FEL is a special USB protocol (accessible on a USB OTG
> connector), which is implemented by the boot ROM and activated by
> holding a special hardware button pressed and rebooting the device.
> FEL supports commands to read/write device RAM and execute code on
> the device. It is designed for device unbricking and firmware recovery.

If I understand it correctly, this is the same function that is
available on Exynos5, Tegra, MX6 and probably others.

>
> In particular, the FEL boot mode support is very useful for debugging
> u-boot and kernel problems on tablets (the SD card slot can be used
> for the UART console, while the system is booted over a micro-USB cable
> with the help of FEL):
>     http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg
>
> In u-boot it is used in the following way:
>  1. The SPL code is uploaded from the linux PC to the device SRAM via
>     a FEL command (using the 'fel' program from sunxi-tools).
>  2. The SPL code is executed via a FEL command and expected to
>     initialize the DRAM controller. The code is executed as a
>     normal C or assembly function, which needs to return control
>     back to the BROM code when it is done. Right now this
>     function is s_init().

Isn't this just another way of loading SPL then? What is so special /
different about FEL?

>  3. As the DRAM is initialized and available now, the main u-boot
>     binary is now uploaded to DRAM via FEL. Together with boot.scr,
>     the kernel, the dtb file and optionally initramfs as needed.
>  4. The main u-boot binary is executed via a FEL command, but never
>     returns back to BROM anymore.
>
> More details are available in the linux-sunxi wiki:
>     http://linux-sunxi.org/FEL
>     http://linux-sunxi.org/FEL/Protocol
>     http://linux-sunxi.org/FEL/USBBoot
>
> I have submitted a patch to fix this regression:
>     https://patchwork.ozlabs.org/patch/434826/
>
> If you encounter problems and/or need help when testing FEL, please
> let me know.

OK I commented on that patch.

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-01 16:29         ` Simon Glass
@ 2015-02-01 16:45           ` Siarhei Siamashka
  2015-02-01 17:00             ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Siarhei Siamashka @ 2015-02-01 16:45 UTC (permalink / raw)
  To: u-boot

On Sun, 1 Feb 2015 09:29:53 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi,
> 
> On 30 January 2015 at 10:53, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
> > On Mon, 29 Dec 2014 09:15:36 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >
> >> Hi Ian,
> >>
> >> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
> >> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
> >> >
> >> >> +void board_init_f(ulong dummy)
> >> >> +{
> >> > [...]
> >> >> +     /* Clear the BSS. */
> >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> >> >> +
> >> >> +     board_init_r(NULL, 0);
> >> >
> >> > The previous (__weak) version of board_init_f also sets gd, which you've
> >> > also removed from s_init here and not added back anywhere (indeed, this
> >> > is the point...). But where is gd initialised now?
> >>
> >> It's still in start.S, I've just removed this duplicate.
> >>
> >> >
> >> > The patch generally looks good, two quick questions: has it been tested
> >> > in both FEL and regular mode, and has it been tested with a "legacy" as
> >> > well as a driver model system? (I might be able to find time in a day or
> >> > two to answer these myself, but for now I'll just ask).
> >>
> >> I haven't tried FEL, I only just heard of it in your email. I'll see
> >> if I can figure out how to test that.
> >
> > Just like Ian suspected, this patch has messed up the FEL boot mode
> > support.
> >
> > In a nutshell, FEL is a special USB protocol (accessible on a USB OTG
> > connector), which is implemented by the boot ROM and activated by
> > holding a special hardware button pressed and rebooting the device.
> > FEL supports commands to read/write device RAM and execute code on
> > the device. It is designed for device unbricking and firmware recovery.
> 
> If I understand it correctly, this is the same function that is
> available on Exynos5, Tegra, MX6 and probably others.
> 
> >
> > In particular, the FEL boot mode support is very useful for debugging
> > u-boot and kernel problems on tablets (the SD card slot can be used
> > for the UART console, while the system is booted over a micro-USB cable
> > with the help of FEL):
> >     http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg
> >
> > In u-boot it is used in the following way:
> >  1. The SPL code is uploaded from the linux PC to the device SRAM via
> >     a FEL command (using the 'fel' program from sunxi-tools).
> >  2. The SPL code is executed via a FEL command and expected to
> >     initialize the DRAM controller. The code is executed as a
> >     normal C or assembly function, which needs to return control
> >     back to the BROM code when it is done. Right now this
> >     function is s_init().
> 
> Isn't this just another way of loading SPL then? What is so special /
> different about FEL?

I'm not very familiar with the other platforms, but the special part
here is "needs to return control back to the BROM code when it is done".

We are relying on the FEL USB protocol implementation, which is
hardcoded in BROM. FEL is used to upload data over USB to the device
memory. And FEL needs to be used twice. Once for uploading data to SRAM
and running the SPL. And one more time for uploading the rest of the
data to DRAM.

As we naturally can't modify the Allwinner code in BROM, here u-boot
needs to play according to the FEL rules and not the other way around.

As I see it, an alternative solution would be to implement USB OTG
support and the FEL protocol (or some other protocol) in the u-boot
SPL and stop relying on BROM code for uploading data to DRAM. Until
this is done, the sunxi FEL variant of SPL needs to return control to
the BROM code nicely after it has initialized DRAM and abstain from
doing anything else.

I hope that this clarifies the situation.

> >  3. As the DRAM is initialized and available now, the main u-boot
> >     binary is now uploaded to DRAM via FEL. Together with boot.scr,
> >     the kernel, the dtb file and optionally initramfs as needed.
> >  4. The main u-boot binary is executed via a FEL command, but never
> >     returns back to BROM anymore.
> >
> > More details are available in the linux-sunxi wiki:
> >     http://linux-sunxi.org/FEL
> >     http://linux-sunxi.org/FEL/Protocol
> >     http://linux-sunxi.org/FEL/USBBoot
> >
> > I have submitted a patch to fix this regression:
> >     https://patchwork.ozlabs.org/patch/434826/
> >
> > If you encounter problems and/or need help when testing FEL, please
> > let me know.
> 
> OK I commented on that patch.

Thanks.

-- 
Best regards,
Siarhei Siamashka

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-01 16:45           ` Siarhei Siamashka
@ 2015-02-01 17:00             ` Simon Glass
  2015-02-01 18:37               ` Siarhei Siamashka
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-01 17:00 UTC (permalink / raw)
  To: u-boot

Hi Siarhei,

On 1 February 2015 at 09:45, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Sun, 1 Feb 2015 09:29:53 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi,
>>
>> On 30 January 2015 at 10:53, Siarhei Siamashka
>> <siarhei.siamashka@gmail.com> wrote:
>> > On Mon, 29 Dec 2014 09:15:36 -0700
>> > Simon Glass <sjg@chromium.org> wrote:
>> >
>> >> Hi Ian,
>> >>
>> >> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
>> >> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
>> >> >
>> >> >> +void board_init_f(ulong dummy)
>> >> >> +{
>> >> > [...]
>> >> >> +     /* Clear the BSS. */
>> >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
>> >> >> +
>> >> >> +     board_init_r(NULL, 0);
>> >> >
>> >> > The previous (__weak) version of board_init_f also sets gd, which you've
>> >> > also removed from s_init here and not added back anywhere (indeed, this
>> >> > is the point...). But where is gd initialised now?
>> >>
>> >> It's still in start.S, I've just removed this duplicate.
>> >>
>> >> >
>> >> > The patch generally looks good, two quick questions: has it been tested
>> >> > in both FEL and regular mode, and has it been tested with a "legacy" as
>> >> > well as a driver model system? (I might be able to find time in a day or
>> >> > two to answer these myself, but for now I'll just ask).
>> >>
>> >> I haven't tried FEL, I only just heard of it in your email. I'll see
>> >> if I can figure out how to test that.
>> >
>> > Just like Ian suspected, this patch has messed up the FEL boot mode
>> > support.
>> >
>> > In a nutshell, FEL is a special USB protocol (accessible on a USB OTG
>> > connector), which is implemented by the boot ROM and activated by
>> > holding a special hardware button pressed and rebooting the device.
>> > FEL supports commands to read/write device RAM and execute code on
>> > the device. It is designed for device unbricking and firmware recovery.
>>
>> If I understand it correctly, this is the same function that is
>> available on Exynos5, Tegra, MX6 and probably others.
>>
>> >
>> > In particular, the FEL boot mode support is very useful for debugging
>> > u-boot and kernel problems on tablets (the SD card slot can be used
>> > for the UART console, while the system is booted over a micro-USB cable
>> > with the help of FEL):
>> >     http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg
>> >
>> > In u-boot it is used in the following way:
>> >  1. The SPL code is uploaded from the linux PC to the device SRAM via
>> >     a FEL command (using the 'fel' program from sunxi-tools).
>> >  2. The SPL code is executed via a FEL command and expected to
>> >     initialize the DRAM controller. The code is executed as a
>> >     normal C or assembly function, which needs to return control
>> >     back to the BROM code when it is done. Right now this
>> >     function is s_init().
>>
>> Isn't this just another way of loading SPL then? What is so special /
>> different about FEL?
>
> I'm not very familiar with the other platforms, but the special part
> here is "needs to return control back to the BROM code when it is done".
>
> We are relying on the FEL USB protocol implementation, which is
> hardcoded in BROM. FEL is used to upload data over USB to the device
> memory. And FEL needs to be used twice. Once for uploading data to SRAM
> and running the SPL. And one more time for uploading the rest of the
> data to DRAM.
>
> As we naturally can't modify the Allwinner code in BROM, here u-boot
> needs to play according to the FEL rules and not the other way around.
>
> As I see it, an alternative solution would be to implement USB OTG
> support and the FEL protocol (or some other protocol) in the u-boot
> SPL and stop relying on BROM code for uploading data to DRAM. Until
> this is done, the sunxi FEL variant of SPL needs to return control to
> the BROM code nicely after it has initialized DRAM and abstain from
> doing anything else.
>
> I hope that this clarifies the situation.

Exynos calls back into its IROM also from SPL, when it sees that it is
in USB A-A mode. See arch/arm/cpu/armv7/exynos/spl_boot.c where you
will find a table of IROM entry points.

I think it is fine to use the BROM and FEL, I am just concerned that
we need to stop using gdata and make FEL fit within the existing SPL
sequence. We need to avoid an SOC-specific hack, and if such a hack is
needed, it should be localised to sunxi and not rely on gdata, etc.

When you say 'return back to BROM', what exactly is a 'return'? Does
it mean that you need to jump to a return address in one of the
registers, or something else? Can you point me to the code that does
this and instructions on how to use it? Then I could try it out and
try to understand it better.

>
>> >  3. As the DRAM is initialized and available now, the main u-boot
>> >     binary is now uploaded to DRAM via FEL. Together with boot.scr,
>> >     the kernel, the dtb file and optionally initramfs as needed.
>> >  4. The main u-boot binary is executed via a FEL command, but never
>> >     returns back to BROM anymore.
>> >
>> > More details are available in the linux-sunxi wiki:
>> >     http://linux-sunxi.org/FEL
>> >     http://linux-sunxi.org/FEL/Protocol
>> >     http://linux-sunxi.org/FEL/USBBoot
>> >
>> > I have submitted a patch to fix this regression:
>> >     https://patchwork.ozlabs.org/patch/434826/
>> >
>> > If you encounter problems and/or need help when testing FEL, please
>> > let me know.
>>
>> OK I commented on that patch.
>
> Thanks.

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-01 17:00             ` Simon Glass
@ 2015-02-01 18:37               ` Siarhei Siamashka
  2015-02-01 20:59                 ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Siarhei Siamashka @ 2015-02-01 18:37 UTC (permalink / raw)
  To: u-boot

On Sun, 1 Feb 2015 10:00:43 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi Siarhei,
> 
> On 1 February 2015 at 09:45, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
> > On Sun, 1 Feb 2015 09:29:53 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >
> >> Hi,
> >>
> >> On 30 January 2015 at 10:53, Siarhei Siamashka
> >> <siarhei.siamashka@gmail.com> wrote:
> >> > On Mon, 29 Dec 2014 09:15:36 -0700
> >> > Simon Glass <sjg@chromium.org> wrote:
> >> >
> >> >> Hi Ian,
> >> >>
> >> >> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk> wrote:
> >> >> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
> >> >> >
> >> >> >> +void board_init_f(ulong dummy)
> >> >> >> +{
> >> >> > [...]
> >> >> >> +     /* Clear the BSS. */
> >> >> >> +     memset(__bss_start, 0, __bss_end - __bss_start);
> >> >> >> +
> >> >> >> +     board_init_r(NULL, 0);
> >> >> >
> >> >> > The previous (__weak) version of board_init_f also sets gd, which you've
> >> >> > also removed from s_init here and not added back anywhere (indeed, this
> >> >> > is the point...). But where is gd initialised now?
> >> >>
> >> >> It's still in start.S, I've just removed this duplicate.
> >> >>
> >> >> >
> >> >> > The patch generally looks good, two quick questions: has it been tested
> >> >> > in both FEL and regular mode, and has it been tested with a "legacy" as
> >> >> > well as a driver model system? (I might be able to find time in a day or
> >> >> > two to answer these myself, but for now I'll just ask).
> >> >>
> >> >> I haven't tried FEL, I only just heard of it in your email. I'll see
> >> >> if I can figure out how to test that.
> >> >
> >> > Just like Ian suspected, this patch has messed up the FEL boot mode
> >> > support.
> >> >
> >> > In a nutshell, FEL is a special USB protocol (accessible on a USB OTG
> >> > connector), which is implemented by the boot ROM and activated by
> >> > holding a special hardware button pressed and rebooting the device.
> >> > FEL supports commands to read/write device RAM and execute code on
> >> > the device. It is designed for device unbricking and firmware recovery.
> >>
> >> If I understand it correctly, this is the same function that is
> >> available on Exynos5, Tegra, MX6 and probably others.
> >>
> >> >
> >> > In particular, the FEL boot mode support is very useful for debugging
> >> > u-boot and kernel problems on tablets (the SD card slot can be used
> >> > for the UART console, while the system is booted over a micro-USB cable
> >> > with the help of FEL):
> >> >     http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg
> >> >
> >> > In u-boot it is used in the following way:
> >> >  1. The SPL code is uploaded from the linux PC to the device SRAM via
> >> >     a FEL command (using the 'fel' program from sunxi-tools).
> >> >  2. The SPL code is executed via a FEL command and expected to
> >> >     initialize the DRAM controller. The code is executed as a
> >> >     normal C or assembly function, which needs to return control
> >> >     back to the BROM code when it is done. Right now this
> >> >     function is s_init().
> >>
> >> Isn't this just another way of loading SPL then? What is so special /
> >> different about FEL?
> >
> > I'm not very familiar with the other platforms, but the special part
> > here is "needs to return control back to the BROM code when it is done".
> >
> > We are relying on the FEL USB protocol implementation, which is
> > hardcoded in BROM. FEL is used to upload data over USB to the device
> > memory. And FEL needs to be used twice. Once for uploading data to SRAM
> > and running the SPL. And one more time for uploading the rest of the
> > data to DRAM.
> >
> > As we naturally can't modify the Allwinner code in BROM, here u-boot
> > needs to play according to the FEL rules and not the other way around.
> >
> > As I see it, an alternative solution would be to implement USB OTG
> > support and the FEL protocol (or some other protocol) in the u-boot
> > SPL and stop relying on BROM code for uploading data to DRAM. Until
> > this is done, the sunxi FEL variant of SPL needs to return control to
> > the BROM code nicely after it has initialized DRAM and abstain from
> > doing anything else.
> >
> > I hope that this clarifies the situation.
> 
> Exynos calls back into its IROM also from SPL, when it sees that it is
> in USB A-A mode. See arch/arm/cpu/armv7/exynos/spl_boot.c where you
> will find a table of IROM entry points.

I think that the difference is a matter of who calls whom. In the
case of Exynos, the SPL can call iROM functions. But in the case of
Allwinner, the whole SPL works as a kind of plugin, which is called
from the BROM code.

I'm not sure if the Allwinner BROM can be used in the same way as
the Exynos iROM, because the Allwinner BROM is not documented at all
as far as I know. Exynos probably has a better documentation.

> I think it is fine to use the BROM and FEL, I am just concerned that
> we need to stop using gdata and make FEL fit within the existing SPL
> sequence. We need to avoid an SOC-specific hack, and if such a hack is
> needed, it should be localised to sunxi and not rely on gdata, etc.

Yes, I agree. At least in the long term.

But in the short term it would be very nice to get the FEL boot
mode working again in the v2015.04 release.

> When you say 'return back to BROM', what exactly is a 'return'? Does
> it mean that you need to jump to a return address in one of the
> registers, or something else?

The return address to the BROM code is in the 'lr' register. Just like
in the case of calling any normal function.

> Can you point me to the code that does this and instructions on
> how to use it? Then I could try it out and try to understand it better.

A comprehensive FEL usage guide is supposed to be in the linux-sunxi
wiki. But it might be not very up to date with the mainline u-boot
usage.

Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
repository and build it. This will provide you with the x86 binary of
the 'fel' tool, which is used on your desktop PC to talk with the
device.

Then connect the device to your PC using a "USB A to USB mini/micro B"
cable. And reset the board while keeping the "FEL" button pressed. The
button might be labelled as "UPGRADE" on your pcduino3 board:
    http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode

Then just run "fel ver" command:
    http://linux-sunxi.org/FEL#Running_the_fel_tool

If everything is fine, it should respond with something like this:
AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
00000000 00000000

Now you can use "write" commands to upload data to SRAM. And "exe"
command to execute functions on the device (works as some kind of RPC):
    http://linux-sunxi.org/FEL/USBBoot#Manual_loading

As a simple test, you can upload just a single "bx lr" instruction
(compiled in ARM mode) to the address 0x2000 and try to execute it.
In the case if your code screws up something and does not return
control to the BROM correctly, then the "fel" tool can't communicate
over USB anymore and just timeouts. Regarding the address space, you
can use SRAM addresses starting from 0x2000 up to something like 0x5E00
(that's where the stack pointer is set). This was a description of a
"bare metal" FEL usage.

Now as for the u-boot support. You can compile and run u-boot in the FEL
mode configuration on pcduino3 in the following way:

    make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
    make CROSS_COMPILE=arm-none-gnueabi- -j2

    fel write 0x2000 spl/u-boot-spl.bin
    fel exe   0x2000

    sleep 1 # Wait for DRAM initialization to complete

    fel write 0x4a000000 u-boot.bin
    fel exe   0x4a000000

Please note the use of '*_felconfig' instead of '*_defconfig'.
And also right now the FEL mode support is broken. That's why the
    http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
patchset tried to address this problem.

> >> >  3. As the DRAM is initialized and available now, the main u-boot
> >> >     binary is now uploaded to DRAM via FEL. Together with boot.scr,
> >> >     the kernel, the dtb file and optionally initramfs as needed.
> >> >  4. The main u-boot binary is executed via a FEL command, but never
> >> >     returns back to BROM anymore.
> >> >
> >> > More details are available in the linux-sunxi wiki:
> >> >     http://linux-sunxi.org/FEL
> >> >     http://linux-sunxi.org/FEL/Protocol
> >> >     http://linux-sunxi.org/FEL/USBBoot
> >> >
> >> > I have submitted a patch to fix this regression:
> >> >     https://patchwork.ozlabs.org/patch/434826/
> >> >
> >> > If you encounter problems and/or need help when testing FEL, please
> >> > let me know.
> >>
> >> OK I commented on that patch.
> >
> > Thanks.
> 
> Regards,
> Simon

-- 
Best regards,
Siarhei Siamashka

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-01 18:37               ` Siarhei Siamashka
@ 2015-02-01 20:59                 ` Simon Glass
  2015-02-02  8:07                   ` Hans de Goede
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-01 20:59 UTC (permalink / raw)
  To: u-boot

Hi Siarhei,

On 1 February 2015 at 11:37, Siarhei Siamashka <siarhei.siamashka@gmail.com>
wrote:
> On Sun, 1 Feb 2015 10:00:43 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi Siarhei,
>>
>> On 1 February 2015 at 09:45, Siarhei Siamashka
>> <siarhei.siamashka@gmail.com> wrote:
>> > On Sun, 1 Feb 2015 09:29:53 -0700
>> > Simon Glass <sjg@chromium.org> wrote:
>> >
>> >> Hi,
>> >>
>> >> On 30 January 2015 at 10:53, Siarhei Siamashka
>> >> <siarhei.siamashka@gmail.com> wrote:
>> >> > On Mon, 29 Dec 2014 09:15:36 -0700
>> >> > Simon Glass <sjg@chromium.org> wrote:
>> >> >
>> >> >> Hi Ian,
>> >> >>
>> >> >> On 28 December 2014 at 02:19, Ian Campbell <ijc@hellion.org.uk>
wrote:
>> >> >> > On Tue, 2014-12-23 at 12:04 -0700, Simon Glass wrote:
>> >> >> >
>> >> >> >> +void board_init_f(ulong dummy)
>> >> >> >> +{
>> >> >> > [...]
>> >> >> >> + /* Clear the BSS. */
>> >> >> >> + memset(__bss_start, 0, __bss_end - __bss_start);
>> >> >> >> +
>> >> >> >> + board_init_r(NULL, 0);
>> >> >> >
>> >> >> > The previous (__weak) version of board_init_f also sets gd,
which you've
>> >> >> > also removed from s_init here and not added back anywhere
(indeed, this
>> >> >> > is the point...). But where is gd initialised now?
>> >> >>
>> >> >> It's still in start.S, I've just removed this duplicate.
>> >> >>
>> >> >> >
>> >> >> > The patch generally looks good, two quick questions: has it been
tested
>> >> >> > in both FEL and regular mode, and has it been tested with a
"legacy" as
>> >> >> > well as a driver model system? (I might be able to find time in
a day or
>> >> >> > two to answer these myself, but for now I'll just ask).
>> >> >>
>> >> >> I haven't tried FEL, I only just heard of it in your email. I'll
see
>> >> >> if I can figure out how to test that.
>> >> >
>> >> > Just like Ian suspected, this patch has messed up the FEL boot mode
>> >> > support.
>> >> >
>> >> > In a nutshell, FEL is a special USB protocol (accessible on a USB
OTG
>> >> > connector), which is implemented by the boot ROM and activated by
>> >> > holding a special hardware button pressed and rebooting the device.
>> >> > FEL supports commands to read/write device RAM and execute code on
>> >> > the device. It is designed for device unbricking and firmware
recovery.
>> >>
>> >> If I understand it correctly, this is the same function that is
>> >> available on Exynos5, Tegra, MX6 and probably others.
>> >>
>> >> >
>> >> > In particular, the FEL boot mode support is very useful for
debugging
>> >> > u-boot and kernel problems on tablets (the SD card slot can be used
>> >> > for the UART console, while the system is booted over a micro-USB
cable
>> >> > with the help of FEL):
>> >> > http://linux-sunxi.org/File:MSI_Primo81_and_MicroSD_breakout.jpg
>> >> >
>> >> > In u-boot it is used in the following way:
>> >> > 1. The SPL code is uploaded from the linux PC to the device SRAM via
>> >> > a FEL command (using the 'fel' program from sunxi-tools).
>> >> > 2. The SPL code is executed via a FEL command and expected to
>> >> > initialize the DRAM controller. The code is executed as a
>> >> > normal C or assembly function, which needs to return control
>> >> > back to the BROM code when it is done. Right now this
>> >> > function is s_init().
>> >>
>> >> Isn't this just another way of loading SPL then? What is so special /
>> >> different about FEL?
>> >
>> > I'm not very familiar with the other platforms, but the special part
>> > here is "needs to return control back to the BROM code when it is
done".
>> >
>> > We are relying on the FEL USB protocol implementation, which is
>> > hardcoded in BROM. FEL is used to upload data over USB to the device
>> > memory. And FEL needs to be used twice. Once for uploading data to SRAM
>> > and running the SPL. And one more time for uploading the rest of the
>> > data to DRAM.
>> >
>> > As we naturally can't modify the Allwinner code in BROM, here u-boot
>> > needs to play according to the FEL rules and not the other way around.
>> >
>> > As I see it, an alternative solution would be to implement USB OTG
>> > support and the FEL protocol (or some other protocol) in the u-boot
>> > SPL and stop relying on BROM code for uploading data to DRAM. Until
>> > this is done, the sunxi FEL variant of SPL needs to return control to
>> > the BROM code nicely after it has initialized DRAM and abstain from
>> > doing anything else.
>> >
>> > I hope that this clarifies the situation.
>>
>> Exynos calls back into its IROM also from SPL, when it sees that it is
>> in USB A-A mode. See arch/arm/cpu/armv7/exynos/spl_boot.c where you
>> will find a table of IROM entry points.
>
> I think that the difference is a matter of who calls whom. In the
> case of Exynos, the SPL can call iROM functions. But in the case of
> Allwinner, the whole SPL works as a kind of plugin, which is called
> from the BROM code.
>
> I'm not sure if the Allwinner BROM can be used in the same way as
> the Exynos iROM, because the Allwinner BROM is not documented at all
> as far as I know. Exynos probably has a better documentation.
>
>> I think it is fine to use the BROM and FEL, I am just concerned that
>> we need to stop using gdata and make FEL fit within the existing SPL
>> sequence. We need to avoid an SOC-specific hack, and if such a hack is
>> needed, it should be localised to sunxi and not rely on gdata, etc.
>
> Yes, I agree. At least in the long term.
>
> But in the short term it would be very nice to get the FEL boot
> mode working again in the v2015.04 release.

I agree on that last point - but I think we should get this working
properly (without gdata) in the short term. We missed the last merge window
with our SPL fixes and I'm pretty determined that we don't miss this one.

>
>> When you say 'return back to BROM', what exactly is a 'return'? Does
>> it mean that you need to jump to a return address in one of the
>> registers, or something else?
>
> The return address to the BROM code is in the 'lr' register. Just like
> in the case of calling any normal function.

OK I see. It does not seem radically different from how Exynos works, in
that all exynos does after the USB download completes is jump to U-Boot.

>
>> Can you point me to the code that does this and instructions on
>> how to use it? Then I could try it out and try to understand it better.
>
> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
> wiki. But it might be not very up to date with the mainline u-boot
> usage.
>
> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
> repository and build it. This will provide you with the x86 binary of
> the 'fel' tool, which is used on your desktop PC to talk with the
> device.
>
> Then connect the device to your PC using a "USB A to USB mini/micro B"
> cable. And reset the board while keeping the "FEL" button pressed. The
> button might be labelled as "UPGRADE" on your pcduino3 board:
> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>
> Then just run "fel ver" command:
> http://linux-sunxi.org/FEL#Running_the_fel_tool
>
> If everything is fine, it should respond with something like this:
> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
> 00000000 00000000
>
> Now you can use "write" commands to upload data to SRAM. And "exe"
> command to execute functions on the device (works as some kind of RPC):
> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>
> As a simple test, you can upload just a single "bx lr" instruction
> (compiled in ARM mode) to the address 0x2000 and try to execute it.
> In the case if your code screws up something and does not return
> control to the BROM correctly, then the "fel" tool can't communicate
> over USB anymore and just timeouts. Regarding the address space, you
> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
> (that's where the stack pointer is set). This was a description of a
> "bare metal" FEL usage.
>
> Now as for the u-boot support. You can compile and run u-boot in the FEL
> mode configuration on pcduino3 in the following way:
>
> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
> make CROSS_COMPILE=arm-none-gnueabi- -j2
>
> fel write 0x2000 spl/u-boot-spl.bin
> fel exe 0x2000
>
> sleep 1 # Wait for DRAM initialization to complete

i.e. wait for U-Boot SPL to return to the BROM?

>
> fel write 0x4a000000 u-boot.bin
> fel exe 0x4a000000
>
> Please note the use of '*_felconfig' instead of '*_defconfig'.
> And also right now the FEL mode support is broken. That's why the
> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
> patchset tried to address this problem.
>

OK that looks like an excellent guide, thank you. I will make some time to
take a more detailed look at this as soon as I can, but definitely in the
next few days.

>> >> > 3. As the DRAM is initialized and available now, the main u-boot
>> >> > binary is now uploaded to DRAM via FEL. Together with boot.scr,
>> >> > the kernel, the dtb file and optionally initramfs as needed.
>> >> > 4. The main u-boot binary is executed via a FEL command, but never
>> >> > returns back to BROM anymore.
>> >> >
>> >> > More details are available in the linux-sunxi wiki:
>> >> > http://linux-sunxi.org/FEL
>> >> > http://linux-sunxi.org/FEL/Protocol
>> >> > http://linux-sunxi.org/FEL/USBBoot
>> >> >
>> >> > I have submitted a patch to fix this regression:
>> >> > https://patchwork.ozlabs.org/patch/434826/
>> >> >
>> >> > If you encounter problems and/or need help when testing FEL, please
>> >> > let me know.
>> >>
>> >> OK I commented on that patch.
>> >
>> > Thanks.

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-01 20:59                 ` Simon Glass
@ 2015-02-02  8:07                   ` Hans de Goede
  2015-02-03  5:29                     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Hans de Goede @ 2015-02-02  8:07 UTC (permalink / raw)
  To: u-boot

Hi Simon, Siarhei,

On 01-02-15 21:59, Simon Glass wrote:
> Hi Siarhei,
>
> On 1 February 2015 at 11:37, Siarhei Siamashka <siarhei.siamashka@gmail.com>

<snip>
>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
>> wiki. But it might be not very up to date with the mainline u-boot
>> usage.
>>
>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
>> repository and build it. This will provide you with the x86 binary of
>> the 'fel' tool, which is used on your desktop PC to talk with the
>> device.
>>
>> Then connect the device to your PC using a "USB A to USB mini/micro B"
>> cable. And reset the board while keeping the "FEL" button pressed. The
>> button might be labelled as "UPGRADE" on your pcduino3 board:
>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>>
>> Then just run "fel ver" command:
>> http://linux-sunxi.org/FEL#Running_the_fel_tool
>>
>> If everything is fine, it should respond with something like this:
>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
>> 00000000 00000000
>>
>> Now you can use "write" commands to upload data to SRAM. And "exe"
>> command to execute functions on the device (works as some kind of RPC):
>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>>
>> As a simple test, you can upload just a single "bx lr" instruction
>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
>> In the case if your code screws up something and does not return
>> control to the BROM correctly, then the "fel" tool can't communicate
>> over USB anymore and just timeouts. Regarding the address space, you
>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
>> (that's where the stack pointer is set). This was a description of a
>> "bare metal" FEL usage.
>>
>> Now as for the u-boot support. You can compile and run u-boot in the FEL
>> mode configuration on pcduino3 in the following way:
>>
>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
>> make CROSS_COMPILE=arm-none-gnueabi- -j2
>>
>> fel write 0x2000 spl/u-boot-spl.bin
>> fel exe 0x2000
>>
>> sleep 1 # Wait for DRAM initialization to complete
>
> i.e. wait for U-Boot SPL to return to the BROM?
>
>>
>> fel write 0x4a000000 u-boot.bin
>> fel exe 0x4a000000
>>
>> Please note the use of '*_felconfig' instead of '*_defconfig'.
>> And also right now the FEL mode support is broken. That's why the
>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
>> patchset tried to address this problem.
>>
>
> OK that looks like an excellent guide, thank you. I will make some time to
> take a more detailed look at this as soon as I can, but definitely in the
> next few days.

I've been reading all threads on this, thank you both for looking into this,
since you're on it I'm going to leave this be (and not review / apply
Siarhei's patches for this), and we can revisit this later, with hopefully a
better fix.

Thanks & Regards,

Hans

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-02  8:07                   ` Hans de Goede
@ 2015-02-03  5:29                     ` Simon Glass
  2015-02-04  0:55                       ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-03  5:29 UTC (permalink / raw)
  To: u-boot

Hi,

On 2 February 2015 at 01:07, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi Simon, Siarhei,
>
> On 01-02-15 21:59, Simon Glass wrote:
>>
>> Hi Siarhei,
>>
>> On 1 February 2015 at 11:37, Siarhei Siamashka
>> <siarhei.siamashka@gmail.com>
>
>
> <snip>
>
>>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
>>> wiki. But it might be not very up to date with the mainline u-boot
>>> usage.
>>>
>>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
>>> repository and build it. This will provide you with the x86 binary of
>>> the 'fel' tool, which is used on your desktop PC to talk with the
>>> device.
>>>
>>> Then connect the device to your PC using a "USB A to USB mini/micro B"
>>> cable. And reset the board while keeping the "FEL" button pressed. The
>>> button might be labelled as "UPGRADE" on your pcduino3 board:
>>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>>>
>>> Then just run "fel ver" command:
>>> http://linux-sunxi.org/FEL#Running_the_fel_tool
>>>
>>> If everything is fine, it should respond with something like this:
>>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
>>> 00000000 00000000
>>>
>>> Now you can use "write" commands to upload data to SRAM. And "exe"
>>> command to execute functions on the device (works as some kind of RPC):
>>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>>>
>>> As a simple test, you can upload just a single "bx lr" instruction
>>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
>>> In the case if your code screws up something and does not return
>>> control to the BROM correctly, then the "fel" tool can't communicate
>>> over USB anymore and just timeouts. Regarding the address space, you
>>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
>>> (that's where the stack pointer is set). This was a description of a
>>> "bare metal" FEL usage.
>>>
>>> Now as for the u-boot support. You can compile and run u-boot in the FEL
>>> mode configuration on pcduino3 in the following way:
>>>
>>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
>>> make CROSS_COMPILE=arm-none-gnueabi- -j2
>>>
>>> fel write 0x2000 spl/u-boot-spl.bin
>>> fel exe 0x2000
>>>
>>> sleep 1 # Wait for DRAM initialization to complete
>>
>>
>> i.e. wait for U-Boot SPL to return to the BROM?
>>
>>>
>>> fel write 0x4a000000 u-boot.bin
>>> fel exe 0x4a000000
>>>
>>> Please note the use of '*_felconfig' instead of '*_defconfig'.
>>> And also right now the FEL mode support is broken. That's why the
>>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
>>> patchset tried to address this problem.
>>>
>>
>> OK that looks like an excellent guide, thank you. I will make some time to
>> take a more detailed look at this as soon as I can, but definitely in the
>> next few days.
>
>
> I've been reading all threads on this, thank you both for looking into this,
> since you're on it I'm going to leave this be (and not review / apply
> Siarhei's patches for this), and we can revisit this later, with hopefully a
> better fix.

Thanks to Siarhei's instructions I have a working setup with pcduino3.
One less board where I have to swap SD cards. Can I suggest that it
would be really nice to have a README.sunxi that covers all of this?

Anyway I will fiddle around and see what I can come up with.

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-03  5:29                     ` Simon Glass
@ 2015-02-04  0:55                       ` Simon Glass
  2015-02-04  1:58                         ` Siarhei Siamashka
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-04  0:55 UTC (permalink / raw)
  To: u-boot

Hi,

On 2 February 2015 at 22:29, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 2 February 2015 at 01:07, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi Simon, Siarhei,
>>
>> On 01-02-15 21:59, Simon Glass wrote:
>>>
>>> Hi Siarhei,
>>>
>>> On 1 February 2015 at 11:37, Siarhei Siamashka
>>> <siarhei.siamashka@gmail.com>
>>
>>
>> <snip>
>>
>>>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
>>>> wiki. But it might be not very up to date with the mainline u-boot
>>>> usage.
>>>>
>>>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
>>>> repository and build it. This will provide you with the x86 binary of
>>>> the 'fel' tool, which is used on your desktop PC to talk with the
>>>> device.
>>>>
>>>> Then connect the device to your PC using a "USB A to USB mini/micro B"
>>>> cable. And reset the board while keeping the "FEL" button pressed. The
>>>> button might be labelled as "UPGRADE" on your pcduino3 board:
>>>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>>>>
>>>> Then just run "fel ver" command:
>>>> http://linux-sunxi.org/FEL#Running_the_fel_tool
>>>>
>>>> If everything is fine, it should respond with something like this:
>>>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
>>>> 00000000 00000000
>>>>
>>>> Now you can use "write" commands to upload data to SRAM. And "exe"
>>>> command to execute functions on the device (works as some kind of RPC):
>>>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>>>>
>>>> As a simple test, you can upload just a single "bx lr" instruction
>>>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
>>>> In the case if your code screws up something and does not return
>>>> control to the BROM correctly, then the "fel" tool can't communicate
>>>> over USB anymore and just timeouts. Regarding the address space, you
>>>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
>>>> (that's where the stack pointer is set). This was a description of a
>>>> "bare metal" FEL usage.
>>>>
>>>> Now as for the u-boot support. You can compile and run u-boot in the FEL
>>>> mode configuration on pcduino3 in the following way:
>>>>
>>>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
>>>> make CROSS_COMPILE=arm-none-gnueabi- -j2
>>>>
>>>> fel write 0x2000 spl/u-boot-spl.bin
>>>> fel exe 0x2000
>>>>
>>>> sleep 1 # Wait for DRAM initialization to complete
>>>
>>>
>>> i.e. wait for U-Boot SPL to return to the BROM?
>>>
>>>>
>>>> fel write 0x4a000000 u-boot.bin
>>>> fel exe 0x4a000000
>>>>
>>>> Please note the use of '*_felconfig' instead of '*_defconfig'.
>>>> And also right now the FEL mode support is broken. That's why the
>>>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
>>>> patchset tried to address this problem.
>>>>
>>>
>>> OK that looks like an excellent guide, thank you. I will make some time to
>>> take a more detailed look at this as soon as I can, but definitely in the
>>> next few days.
>>
>>
>> I've been reading all threads on this, thank you both for looking into this,
>> since you're on it I'm going to leave this be (and not review / apply
>> Siarhei's patches for this), and we can revisit this later, with hopefully a
>> better fix.
>
> Thanks to Siarhei's instructions I have a working setup with pcduino3.
> One less board where I have to swap SD cards. Can I suggest that it
> would be really nice to have a README.sunxi that covers all of this?
>
> Anyway I will fiddle around and see what I can come up with.

The problem seems to be the start.S code 'Setup vector' and
cpu_init_cp15. If I remove these then FEL boots OK without all the
gdata/r9 stuff and with the rest of the boot flow normal.

I would quite like FEL to be an option you enable, but we can detect
whether it is actually being used (as with Tegra and Exynos). Perhaps
checking the value of lr will allow this to work.

I'm not sure of the best way to skip the vector/cp15 setup. For now I
can do an #ifdef but I'm wondering if we should allow
save_boot_params() to return some flags indicating what should be
skipped?

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-04  0:55                       ` Simon Glass
@ 2015-02-04  1:58                         ` Siarhei Siamashka
  2015-02-04  3:23                           ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Siarhei Siamashka @ 2015-02-04  1:58 UTC (permalink / raw)
  To: u-boot

On Tue, 3 Feb 2015 17:55:52 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi,
> 
> On 2 February 2015 at 22:29, Simon Glass <sjg@chromium.org> wrote:
> > Hi,
> >
> > On 2 February 2015 at 01:07, Hans de Goede <hdegoede@redhat.com> wrote:
> >> Hi Simon, Siarhei,
> >>
> >> On 01-02-15 21:59, Simon Glass wrote:
> >>>
> >>> Hi Siarhei,
> >>>
> >>> On 1 February 2015 at 11:37, Siarhei Siamashka
> >>> <siarhei.siamashka@gmail.com>
> >>
> >>
> >> <snip>
> >>
> >>>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
> >>>> wiki. But it might be not very up to date with the mainline u-boot
> >>>> usage.
> >>>>
> >>>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
> >>>> repository and build it. This will provide you with the x86 binary of
> >>>> the 'fel' tool, which is used on your desktop PC to talk with the
> >>>> device.
> >>>>
> >>>> Then connect the device to your PC using a "USB A to USB mini/micro B"
> >>>> cable. And reset the board while keeping the "FEL" button pressed. The
> >>>> button might be labelled as "UPGRADE" on your pcduino3 board:
> >>>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
> >>>>
> >>>> Then just run "fel ver" command:
> >>>> http://linux-sunxi.org/FEL#Running_the_fel_tool
> >>>>
> >>>> If everything is fine, it should respond with something like this:
> >>>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
> >>>> 00000000 00000000
> >>>>
> >>>> Now you can use "write" commands to upload data to SRAM. And "exe"
> >>>> command to execute functions on the device (works as some kind of RPC):
> >>>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
> >>>>
> >>>> As a simple test, you can upload just a single "bx lr" instruction
> >>>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
> >>>> In the case if your code screws up something and does not return
> >>>> control to the BROM correctly, then the "fel" tool can't communicate
> >>>> over USB anymore and just timeouts. Regarding the address space, you
> >>>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
> >>>> (that's where the stack pointer is set). This was a description of a
> >>>> "bare metal" FEL usage.
> >>>>
> >>>> Now as for the u-boot support. You can compile and run u-boot in the FEL
> >>>> mode configuration on pcduino3 in the following way:
> >>>>
> >>>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
> >>>> make CROSS_COMPILE=arm-none-gnueabi- -j2
> >>>>
> >>>> fel write 0x2000 spl/u-boot-spl.bin
> >>>> fel exe 0x2000
> >>>>
> >>>> sleep 1 # Wait for DRAM initialization to complete
> >>>
> >>>
> >>> i.e. wait for U-Boot SPL to return to the BROM?
> >>>
> >>>>
> >>>> fel write 0x4a000000 u-boot.bin
> >>>> fel exe 0x4a000000
> >>>>
> >>>> Please note the use of '*_felconfig' instead of '*_defconfig'.
> >>>> And also right now the FEL mode support is broken. That's why the
> >>>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
> >>>> patchset tried to address this problem.
> >>>>
> >>>
> >>> OK that looks like an excellent guide, thank you. I will make some time to
> >>> take a more detailed look at this as soon as I can, but definitely in the
> >>> next few days.
> >>
> >>
> >> I've been reading all threads on this, thank you both for looking into this,
> >> since you're on it I'm going to leave this be (and not review / apply
> >> Siarhei's patches for this), and we can revisit this later, with hopefully a
> >> better fix.
> >
> > Thanks to Siarhei's instructions I have a working setup with pcduino3.
> > One less board where I have to swap SD cards. Can I suggest that it
> > would be really nice to have a README.sunxi that covers all of this?
> >
> > Anyway I will fiddle around and see what I can come up with.
> 
> The problem seems to be the start.S code 'Setup vector' and
> cpu_init_cp15. If I remove these then FEL boots OK without all the
> gdata/r9 stuff and with the rest of the boot flow normal.

I don't quite follow this brief explanation. Could you perhaps
attach/paste a patch or provide a link to some test branch with
the code?

> I would quite like FEL to be an option you enable, but we can detect
> whether it is actually being used (as with Tegra and Exynos).

FEL is already a Kconfig option, which can be enabled/disabled
in menuconfig. Currently it also disables certain parts of code
(MMC support) in order to reduce the SPL size. If these parts are
all included (but deactivated via a runtime check), then the SPL
binary becomes too large.

> Perhaps checking the value of lr will allow this to work.

Or by peeking into some of the MUSB hardware registers after the
start of SPL. MUSB must be already initialized for FEL to work.

However I'm still curious about how the SPL size problem is going
to be addressed.

> I'm not sure of the best way to skip the vector/cp15 setup. For now I
> can do an #ifdef but I'm wondering if we should allow
> save_boot_params() to return some flags indicating what should be
> skipped?

-- 
Best regards,
Siarhei Siamashka

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-04  1:58                         ` Siarhei Siamashka
@ 2015-02-04  3:23                           ` Simon Glass
  2015-02-04  4:14                             ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-02-04  3:23 UTC (permalink / raw)
  To: u-boot

Hi Siarhei,

On 3 February 2015 at 18:58, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Tue, 3 Feb 2015 17:55:52 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi,
>>
>> On 2 February 2015 at 22:29, Simon Glass <sjg@chromium.org> wrote:
>> > Hi,
>> >
>> > On 2 February 2015 at 01:07, Hans de Goede <hdegoede@redhat.com> wrote:
>> >> Hi Simon, Siarhei,
>> >>
>> >> On 01-02-15 21:59, Simon Glass wrote:
>> >>>
>> >>> Hi Siarhei,
>> >>>
>> >>> On 1 February 2015 at 11:37, Siarhei Siamashka
>> >>> <siarhei.siamashka@gmail.com>
>> >>
>> >>
>> >> <snip>
>> >>
>> >>>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
>> >>>> wiki. But it might be not very up to date with the mainline u-boot
>> >>>> usage.
>> >>>>
>> >>>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
>> >>>> repository and build it. This will provide you with the x86 binary of
>> >>>> the 'fel' tool, which is used on your desktop PC to talk with the
>> >>>> device.
>> >>>>
>> >>>> Then connect the device to your PC using a "USB A to USB mini/micro B"
>> >>>> cable. And reset the board while keeping the "FEL" button pressed. The
>> >>>> button might be labelled as "UPGRADE" on your pcduino3 board:
>> >>>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>> >>>>
>> >>>> Then just run "fel ver" command:
>> >>>> http://linux-sunxi.org/FEL#Running_the_fel_tool
>> >>>>
>> >>>> If everything is fine, it should respond with something like this:
>> >>>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
>> >>>> 00000000 00000000
>> >>>>
>> >>>> Now you can use "write" commands to upload data to SRAM. And "exe"
>> >>>> command to execute functions on the device (works as some kind of RPC):
>> >>>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>> >>>>
>> >>>> As a simple test, you can upload just a single "bx lr" instruction
>> >>>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
>> >>>> In the case if your code screws up something and does not return
>> >>>> control to the BROM correctly, then the "fel" tool can't communicate
>> >>>> over USB anymore and just timeouts. Regarding the address space, you
>> >>>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
>> >>>> (that's where the stack pointer is set). This was a description of a
>> >>>> "bare metal" FEL usage.
>> >>>>
>> >>>> Now as for the u-boot support. You can compile and run u-boot in the FEL
>> >>>> mode configuration on pcduino3 in the following way:
>> >>>>
>> >>>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
>> >>>> make CROSS_COMPILE=arm-none-gnueabi- -j2
>> >>>>
>> >>>> fel write 0x2000 spl/u-boot-spl.bin
>> >>>> fel exe 0x2000
>> >>>>
>> >>>> sleep 1 # Wait for DRAM initialization to complete
>> >>>
>> >>>
>> >>> i.e. wait for U-Boot SPL to return to the BROM?
>> >>>
>> >>>>
>> >>>> fel write 0x4a000000 u-boot.bin
>> >>>> fel exe 0x4a000000
>> >>>>
>> >>>> Please note the use of '*_felconfig' instead of '*_defconfig'.
>> >>>> And also right now the FEL mode support is broken. That's why the
>> >>>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
>> >>>> patchset tried to address this problem.
>> >>>>
>> >>>
>> >>> OK that looks like an excellent guide, thank you. I will make some time to
>> >>> take a more detailed look at this as soon as I can, but definitely in the
>> >>> next few days.
>> >>
>> >>
>> >> I've been reading all threads on this, thank you both for looking into this,
>> >> since you're on it I'm going to leave this be (and not review / apply
>> >> Siarhei's patches for this), and we can revisit this later, with hopefully a
>> >> better fix.
>> >
>> > Thanks to Siarhei's instructions I have a working setup with pcduino3.
>> > One less board where I have to swap SD cards. Can I suggest that it
>> > would be really nice to have a README.sunxi that covers all of this?
>> >
>> > Anyway I will fiddle around and see what I can come up with.
>>
>> The problem seems to be the start.S code 'Setup vector' and
>> cpu_init_cp15. If I remove these then FEL boots OK without all the
>> gdata/r9 stuff and with the rest of the boot flow normal.
>
> I don't quite follow this brief explanation. Could you perhaps
> attach/paste a patch or provide a link to some test branch with
> the code?

If you look at code in start.S it fiddles with cp15 and this seems to
stuff up FEL. I'm not exactly sure what is breaking it, but we seem to
need to skip that code.

I will push what I have to a branch - I am just trying to tidy it up a bit.

>
>> I would quite like FEL to be an option you enable, but we can detect
>> whether it is actually being used (as with Tegra and Exynos).
>
> FEL is already a Kconfig option, which can be enabled/disabled
> in menuconfig. Currently it also disables certain parts of code
> (MMC support) in order to reduce the SPL size. If these parts are
> all included (but deactivated via a runtime check), then the SPL
> binary becomes too large.

OK. My point is that FEL support should not be such a pervasive
option, changing everything about how it boots. We should just change
the minimum to make FEL work. The only thing that really is a problem
is the size, so with FEL we must disable MMC. That is fine.

>
>> Perhaps checking the value of lr will allow this to work.
>
> Or by peeking into some of the MUSB hardware registers after the
> start of SPL. MUSB must be already initialized for FEL to work.

That sounds good, we should use whatever is a cast-iron way of knowing
the boot mode. On Exynos there is a register.

>
> However I'm still curious about how the SPL size problem is going
> to be addressed.

Well it probably cannot be addressed. Does the boot ROM have an MMC
stack? Surely it must if it supports booting from MMC?

>
>> I'm not sure of the best way to skip the vector/cp15 setup. For now I
>> can do an #ifdef but I'm wondering if we should allow
>> save_boot_params() to return some flags indicating what should be
>> skipped?

Regards,
Simon

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

* [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f()
  2015-02-04  3:23                           ` Simon Glass
@ 2015-02-04  4:14                             ` Simon Glass
  0 siblings, 0 replies; 41+ messages in thread
From: Simon Glass @ 2015-02-04  4:14 UTC (permalink / raw)
  To: u-boot

Hi Siarhei,

On 3 February 2015 at 20:23, Simon Glass <sjg@chromium.org> wrote:
> Hi Siarhei,
>
> On 3 February 2015 at 18:58, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
>> On Tue, 3 Feb 2015 17:55:52 -0700
>> Simon Glass <sjg@chromium.org> wrote:
>>
>>> Hi,
>>>
>>> On 2 February 2015 at 22:29, Simon Glass <sjg@chromium.org> wrote:
>>> > Hi,
>>> >
>>> > On 2 February 2015 at 01:07, Hans de Goede <hdegoede@redhat.com> wrote:
>>> >> Hi Simon, Siarhei,
>>> >>
>>> >> On 01-02-15 21:59, Simon Glass wrote:
>>> >>>
>>> >>> Hi Siarhei,
>>> >>>
>>> >>> On 1 February 2015 at 11:37, Siarhei Siamashka
>>> >>> <siarhei.siamashka@gmail.com>
>>> >>
>>> >>
>>> >> <snip>
>>> >>
>>> >>>> A comprehensive FEL usage guide is supposed to be in the linux-sunxi
>>> >>>> wiki. But it might be not very up to date with the mainline u-boot
>>> >>>> usage.
>>> >>>>
>>> >>>> Anyway, first just clone the https://github.com/linux-sunxi/sunxi-tools
>>> >>>> repository and build it. This will provide you with the x86 binary of
>>> >>>> the 'fel' tool, which is used on your desktop PC to talk with the
>>> >>>> device.
>>> >>>>
>>> >>>> Then connect the device to your PC using a "USB A to USB mini/micro B"
>>> >>>> cable. And reset the board while keeping the "FEL" button pressed. The
>>> >>>> button might be labelled as "UPGRADE" on your pcduino3 board:
>>> >>>> http://linux-sunxi.org/LinkSprite_pcDuino_V3#FEL_mode
>>> >>>>
>>> >>>> Then just run "fel ver" command:
>>> >>>> http://linux-sunxi.org/FEL#Running_the_fel_tool
>>> >>>>
>>> >>>> If everything is fine, it should respond with something like this:
>>> >>>> AWUSBFEX soc=00162500(A13) 00000001 ver=0001 44 08 scratchpad=00007e00
>>> >>>> 00000000 00000000
>>> >>>>
>>> >>>> Now you can use "write" commands to upload data to SRAM. And "exe"
>>> >>>> command to execute functions on the device (works as some kind of RPC):
>>> >>>> http://linux-sunxi.org/FEL/USBBoot#Manual_loading
>>> >>>>
>>> >>>> As a simple test, you can upload just a single "bx lr" instruction
>>> >>>> (compiled in ARM mode) to the address 0x2000 and try to execute it.
>>> >>>> In the case if your code screws up something and does not return
>>> >>>> control to the BROM correctly, then the "fel" tool can't communicate
>>> >>>> over USB anymore and just timeouts. Regarding the address space, you
>>> >>>> can use SRAM addresses starting from 0x2000 up to something like 0x5E00
>>> >>>> (that's where the stack pointer is set). This was a description of a
>>> >>>> "bare metal" FEL usage.
>>> >>>>
>>> >>>> Now as for the u-boot support. You can compile and run u-boot in the FEL
>>> >>>> mode configuration on pcduino3 in the following way:
>>> >>>>
>>> >>>> make CROSS_COMPILE=arm-none-gnueabi- Linksprite_pcDuino3_felconfig
>>> >>>> make CROSS_COMPILE=arm-none-gnueabi- -j2
>>> >>>>
>>> >>>> fel write 0x2000 spl/u-boot-spl.bin
>>> >>>> fel exe 0x2000
>>> >>>>
>>> >>>> sleep 1 # Wait for DRAM initialization to complete
>>> >>>
>>> >>>
>>> >>> i.e. wait for U-Boot SPL to return to the BROM?
>>> >>>
>>> >>>>
>>> >>>> fel write 0x4a000000 u-boot.bin
>>> >>>> fel exe 0x4a000000
>>> >>>>
>>> >>>> Please note the use of '*_felconfig' instead of '*_defconfig'.
>>> >>>> And also right now the FEL mode support is broken. That's why the
>>> >>>> http://lists.denx.de/pipermail/u-boot/2015-January/203282.html
>>> >>>> patchset tried to address this problem.
>>> >>>>
>>> >>>
>>> >>> OK that looks like an excellent guide, thank you. I will make some time to
>>> >>> take a more detailed look at this as soon as I can, but definitely in the
>>> >>> next few days.
>>> >>
>>> >>
>>> >> I've been reading all threads on this, thank you both for looking into this,
>>> >> since you're on it I'm going to leave this be (and not review / apply
>>> >> Siarhei's patches for this), and we can revisit this later, with hopefully a
>>> >> better fix.
>>> >
>>> > Thanks to Siarhei's instructions I have a working setup with pcduino3.
>>> > One less board where I have to swap SD cards. Can I suggest that it
>>> > would be really nice to have a README.sunxi that covers all of this?
>>> >
>>> > Anyway I will fiddle around and see what I can come up with.
>>>
>>> The problem seems to be the start.S code 'Setup vector' and
>>> cpu_init_cp15. If I remove these then FEL boots OK without all the
>>> gdata/r9 stuff and with the rest of the boot flow normal.
>>
>> I don't quite follow this brief explanation. Could you perhaps
>> attach/paste a patch or provide a link to some test branch with
>> the code?
>
> If you look at code in start.S it fiddles with cp15 and this seems to
> stuff up FEL. I'm not exactly sure what is breaking it, but we seem to
> need to skip that code.
>
> I will push what I have to a branch - I am just trying to tidy it up a bit.
>
>>
>>> I would quite like FEL to be an option you enable, but we can detect
>>> whether it is actually being used (as with Tegra and Exynos).
>>
>> FEL is already a Kconfig option, which can be enabled/disabled
>> in menuconfig. Currently it also disables certain parts of code
>> (MMC support) in order to reduce the SPL size. If these parts are
>> all included (but deactivated via a runtime check), then the SPL
>> binary becomes too large.
>
> OK. My point is that FEL support should not be such a pervasive
> option, changing everything about how it boots. We should just change
> the minimum to make FEL work. The only thing that really is a problem
> is the size, so with FEL we must disable MMC. That is fine.
>
>>
>>> Perhaps checking the value of lr will allow this to work.
>>
>> Or by peeking into some of the MUSB hardware registers after the
>> start of SPL. MUSB must be already initialized for FEL to work.
>
> That sounds good, we should use whatever is a cast-iron way of knowing
> the boot mode. On Exynos there is a register.
>
>>
>> However I'm still curious about how the SPL size problem is going
>> to be addressed.
>
> Well it probably cannot be addressed. Does the boot ROM have an MMC
> stack? Surely it must if it supports booting from MMC?
>
>>
>>> I'm not sure of the best way to skip the vector/cp15 setup. For now I
>>> can do an #ifdef but I'm wondering if we should allow
>>> save_boot_params() to return some flags indicating what should be
>>> skipped?

I will send a series separately for you to try. It needs work but I
hope it illustrates the idea.

Regards,
Simon

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

end of thread, other threads:[~2015-02-04  4:14 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-23 19:04 [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Simon Glass
2014-12-23 19:04 ` [U-Boot] [PATCH 1/9] arm: Add warnings about using gdata Simon Glass
2014-12-24  6:53   ` Igor Grinberg
2014-12-29 16:24     ` Simon Glass
2014-12-30  7:39       ` Igor Grinberg
2014-12-30 22:12         ` Simon Glass
2015-01-20 21:40   ` [U-Boot] [U-Boot,1/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 2/9] sunxi: Move SPL s_init() code to board_init_f() Simon Glass
2014-12-28  9:19   ` Ian Campbell
2014-12-29 16:15     ` Simon Glass
2015-01-30 17:53       ` Siarhei Siamashka
2015-02-01 16:29         ` Simon Glass
2015-02-01 16:45           ` Siarhei Siamashka
2015-02-01 17:00             ` Simon Glass
2015-02-01 18:37               ` Siarhei Siamashka
2015-02-01 20:59                 ` Simon Glass
2015-02-02  8:07                   ` Hans de Goede
2015-02-03  5:29                     ` Simon Glass
2015-02-04  0:55                       ` Simon Glass
2015-02-04  1:58                         ` Siarhei Siamashka
2015-02-04  3:23                           ` Simon Glass
2015-02-04  4:14                             ` Simon Glass
2015-01-20 21:40   ` [U-Boot] [U-Boot, " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 3/9] sunxi: Drop use of lowlevel_init() Simon Glass
2014-12-28  9:10   ` Ian Campbell
2015-01-20 21:40   ` [U-Boot] [U-Boot,3/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 4/9] arm: Reduce the scope " Simon Glass
2014-12-23 19:04 ` [U-Boot] [PATCH 5/9] zynq: Remove reference to gdata Simon Glass
2015-01-20 21:40   ` [U-Boot] [U-Boot,5/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 6/9] imx: cm_fx6: " Simon Glass
2014-12-24  6:50   ` Igor Grinberg
2014-12-31 12:25   ` Nikita Kiryanov
2015-01-20 21:40   ` [U-Boot] [U-Boot,6/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 7/9] imx: woodburn: " Simon Glass
2014-12-30 12:10   ` Stefano Babic
2015-01-20 21:40   ` [U-Boot] [U-Boot,7/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 8/9] imx: ls102xa: " Simon Glass
2015-01-20 21:40   ` [U-Boot] [U-Boot,8/9] " Tom Rini
2014-12-23 19:04 ` [U-Boot] [PATCH 9/9] arm: Drop gdata global_data variable in SPL Simon Glass
2014-12-28  9:09 ` [U-Boot] [PATCH 0/9] Remove use of gdata for global_data Ian Campbell
2015-01-19  6:48 ` Albert ARIBAUD

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