[U-Boot] [PATCH 0/4] sunxi: FEL mode fixes

[U-Boot] [PATCH 0/4] sunxi: FEL mode fixes

[Siarhei Siamashka]

The recent u-boot changes broke FEL mode support on sunxi hardware.
This patch series fixes the regression and also introduces some other
cleanups.

Siarhei Siamashka (4):
  sunxi: Make FEL mode usable again
  sunxi: Use Thumb2 for the FEL mode SPL
  sunxi: Get rid of u-boot-spl-fel.lds
  sunxi: Use more realistic size limit for FEL SPL

 arch/arm/cpu/armv7/sunxi/Makefile           |  1 +
 arch/arm/cpu/armv7/sunxi/board.c            | 26 ++++-----
 arch/arm/cpu/armv7/sunxi/start_fel.S        | 16 ++++++
 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 82 -----------------------------
 arch/arm/cpu/armv7/sunxi/u-boot-spl.lds     |  4 +-
 include/configs/sunxi-common.h              | 18 ++++---
 6 files changed, 39 insertions(+), 108 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
 delete mode 100644 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds

-- 
2.0.5

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Siarhei Siamashka]

The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
'sunxi: Move SPL s_init() code to board_init_f()'
broke the FEL boot mode.

This patch moves the DRAM initialization back to s_init() and
introduces an assembly entry point for FEL in order to provide
guaranteed initialization of the gdata pointer (r9). The assembly
entry point is also needed to ensure that the SPL code starts
executing in ARM mode.

Because the sunxi board_init_f() does not contain anything that
is not already done by the default board_init_f(), it is removed
too.

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
 arch/arm/cpu/armv7/sunxi/Makefile           |  1 +
 arch/arm/cpu/armv7/sunxi/board.c            | 26 ++++++++++----------------
 arch/arm/cpu/armv7/sunxi/start_fel.S        | 16 ++++++++++++++++
 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds |  3 ++-
 4 files changed, 29 insertions(+), 17 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
index 48db744..e0d413d 100644
--- a/arch/arm/cpu/armv7/sunxi/Makefile
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I)	+= dram_sun4i.o
 obj-$(CONFIG_MACH_SUN8I)	+= dram_sun8i.o
 ifdef CONFIG_SPL_FEL
 obj-y	+= start.o
+extra-y	+= start_fel.o
 endif
 endif
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
index 6e28bcd..ea6cb60 100644
--- a/arch/arm/cpu/armv7/sunxi/board.c
+++ b/arch/arm/cpu/armv7/sunxi/board.c
@@ -85,6 +85,16 @@ void s_init(void)
 	timer_init();
 	gpio_init();
 	i2c_init_board();
+
+#ifdef CONFIG_SPL_BUILD
+	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
 }
 
 #ifdef CONFIG_SPL_BUILD
@@ -103,22 +113,6 @@ u32 spl_boot_mode(void)
 {
 	return MMCSD_MODE_RAW;
 }
-
-void board_init_f(ulong dummy)
-{
-	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)
diff --git a/arch/arm/cpu/armv7/sunxi/start_fel.S b/arch/arm/cpu/armv7/sunxi/start_fel.S
new file mode 100644
index 0000000..e1c7cd4
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/start_fel.S
@@ -0,0 +1,16 @@
+/*
+ * Entry point of the FEL mode SPL.
+ *
+ * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamashka@gmail.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <asm-offsets.h>
+#include <config.h>
+#include <linux/linkage.h>
+
+ENTRY(_start_fel)
+	ldr	r9, =gdata
+	b	s_init
+ENDPROC(_start_fel)
diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
index 928b7c1..beb8900 100644
--- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
+++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
@@ -6,7 +6,7 @@
  */
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
-ENTRY(s_init)
+ENTRY(_start_fel)
 SECTIONS
 {
 	. = 0x00002000;
@@ -14,6 +14,7 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
+		arch/arm/cpu/armv7/sunxi/start_fel.o	(.text)
 		*(.text.s_init)
 		*(.text*)
 	}
-- 
2.0.5

[U-Boot] [PATCH 2/4] sunxi: Use Thumb2 for the FEL mode SPL

[Siarhei Siamashka]

With the FEL entry point converted to assembly code (which uses ARM
mode), the rest of the SPL can be now compiled in Thumb2 mode safely.
This provides a significant code size reduction:

== before ==
   text	   data	    bss	    dec	    hex	filename
  13938	    440	     28	  14406	   3846	spl/u-boot-spl

== after ==
   text	   data	    bss	    dec	    hex	filename
  10918	    440	     28	  11386	   2c7a	spl/u-boot-spl

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
 include/configs/sunxi-common.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 6cfd7e1..f570d9c 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -18,10 +18,8 @@
  */
 #define CONFIG_SUNXI		/* sunxi family */
 #ifdef CONFIG_SPL_BUILD
-#ifndef CONFIG_SPL_FEL
 #define CONFIG_SYS_THUMB_BUILD	/* Thumbs mode to save space in SPL */
 #endif
-#endif
 
 #include <asm/arch/cpu.h>	/* get chip and board defs */
 
-- 
2.0.5

[U-Boot] [PATCH 3/4] sunxi: Get rid of u-boot-spl-fel.lds

[Siarhei Siamashka]

The existing u-boot-spl-fel.lds linker script is rather messy.
Instead of fixing it, just use u-boot-spl.lds for both normal
SPL and FEL SPL.

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 83 -----------------------------
 arch/arm/cpu/armv7/sunxi/u-boot-spl.lds     |  4 +-
 include/configs/sunxi-common.h              | 14 +++--
 3 files changed, 11 insertions(+), 90 deletions(-)
 delete mode 100644 arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds

diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
deleted file mode 100644
index beb8900..0000000
--- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * (C) Copyright 2013
- * Henrik Nordstrom <henrik@henriknordstrom.net>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
-OUTPUT_ARCH(arm)
-ENTRY(_start_fel)
-SECTIONS
-{
-	. = 0x00002000;
-
-	. = ALIGN(4);
-	.text :
-	{
-		arch/arm/cpu/armv7/sunxi/start_fel.o	(.text)
-		*(.text.s_init)
-		*(.text*)
-	}
-
-	. = ALIGN(4);
-	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
-
-	. = ALIGN(4);
-	.data : {
-		*(.data*)
-	}
-
-	. = ALIGN(4);
-	.u_boot_list : {
-		KEEP(*(SORT(.u_boot_list*)));
-	}
-
-	. = ALIGN(4);
-	. = .;
-
-	. = ALIGN(4);
-	.rel.dyn : {
-		__rel_dyn_start = .;
-		*(.rel*)
-		__rel_dyn_end = .;
-	}
-
-	.dynsym : {
-		__dynsym_start = .;
-		*(.dynsym)
-	}
-
-	. = ALIGN(4);
-	.note.gnu.build-id :
-	{
-		*(.note.gnu.build-id)
-	}
-	_end = .;
-
-	. = ALIGN(4096);
-	.mmutable : {
-		*(.mmutable)
-	}
-
-	.bss_start __rel_dyn_start (OVERLAY) : {
-		KEEP(*(.__bss_start));
-		__bss_base = .;
-	}
-
-	.bss __bss_base (OVERLAY) : {
-		*(.bss*)
-		. = ALIGN(4);
-		__bss_limit = .;
-	}
-
-	.bss_end __bss_limit (OVERLAY) : {
-		KEEP(*(.__bss_end));
-	}
-
-	/DISCARD/ : { *(.dynstr*) }
-	/DISCARD/ : { *(.dynamic*) }
-	/DISCARD/ : { *(.plt*) }
-	/DISCARD/ : { *(.interp*) }
-	/DISCARD/ : { *(.gnu*) }
-	/DISCARD/ : { *(.note*) }
-}
diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
index 53f0cbd..7806bcf 100644
--- a/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
+++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl.lds
@@ -21,14 +21,14 @@ MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
-ENTRY(_start)
+ENTRY(CONFIG_SPL_ENTRY_POINT_FUNCTION)
 SECTIONS
 {
 	.text      :
 	{
 		__start = .;
 		*(.vectors)
-		arch/arm/cpu/armv7/start.o	(.text)
+		CONFIG_SPL_ENTRY_POINT_OBJ_FILE	(.text)
 		*(.text*)
 	} > .sram
 
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index f570d9c..c644ad4 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -144,18 +144,21 @@
 #define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_LIBGENERIC_SUPPORT
 
+#define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/sunxi/u-boot-spl.lds"
+#define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
+#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KiB */
+
 #ifdef CONFIG_SPL_FEL
 
-#define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds"
+#define CONFIG_SPL_ENTRY_POINT_FUNCTION "_start_fel"
+#define CONFIG_SPL_ENTRY_POINT_OBJ_FILE "arch/arm/cpu/armv7/sunxi/start_fel.o"
+
 #define CONFIG_SPL_START_S_PATH "arch/arm/cpu/armv7/sunxi"
 #define CONFIG_SPL_TEXT_BASE		0x2000
 #define CONFIG_SPL_MAX_SIZE		0x4000		/* 16 KiB */
 
 #else /* CONFIG_SPL */
 
-#define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
-#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KiB */
-
 #define CONFIG_SPL_TEXT_BASE		0x20		/* sram start+header */
 #define CONFIG_SPL_MAX_SIZE		0x5fe0		/* 24KB on sun4i/sun7i */
 
@@ -165,7 +168,8 @@
 #define CONFIG_SPL_MMC_SUPPORT
 #endif
 
-#define CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv7/sunxi/u-boot-spl.lds"
+#define CONFIG_SPL_ENTRY_POINT_FUNCTION "_start"
+#define CONFIG_SPL_ENTRY_POINT_OBJ_FILE "arch/arm/cpu/armv7/start.o"
 
 #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	80	/* 40KiB */
 #define CONFIG_SPL_PAD_TO		32768		/* decimal for 'dd' */
-- 
2.0.5

[U-Boot] [PATCH 4/4] sunxi: Use more realistic size limit for FEL SPL

[Siarhei Siamashka]

The older 16 KiB limit was overly optimistic. The FEL SPL binary
is loaded at the address 0x2000 (which is also the entry point),
and the stack pointer is set by the FEL BROM code to 0x5E00 on
Allwinner A20 right at the start. This gives us around 15872
bytes for everything (code, data and stack). Considering that
the stack usage is somewhere between 1 KiB and 1.5 KiB (mostly
because of printf buffers), setting 14 KiB as the FEL SPL size
limit is reasonable.

Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
---
 include/configs/sunxi-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index c644ad4..85965f4 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -155,7 +155,7 @@
 
 #define CONFIG_SPL_START_S_PATH "arch/arm/cpu/armv7/sunxi"
 #define CONFIG_SPL_TEXT_BASE		0x2000
-#define CONFIG_SPL_MAX_SIZE		0x4000		/* 16 KiB */
+#define CONFIG_SPL_MAX_SIZE		0x3800		/* 14 KiB */
 
 #else /* CONFIG_SPL */
 
-- 
2.0.5

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Siarhei Siamashka]

On Fri, 30 Jan 2015 13:58:46 +0200
Siarhei Siamashka <siarhei.siamashka@gmail.com> wrote:

> The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
> 'sunxi: Move SPL s_init() code to board_init_f()'
> broke the FEL boot mode.
> 
> This patch moves the DRAM initialization back to s_init() and
> introduces an assembly entry point for FEL in order to provide
> guaranteed initialization of the gdata pointer (r9). The assembly
> entry point is also needed to ensure that the SPL code starts
> executing in ARM mode.
> 
> Because the sunxi board_init_f() does not contain anything that
> is not already done by the default board_init_f(), it is removed
> too.
> 
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> ---
>  arch/arm/cpu/armv7/sunxi/Makefile           |  1 +
>  arch/arm/cpu/armv7/sunxi/board.c            | 26 ++++++++++----------------
>  arch/arm/cpu/armv7/sunxi/start_fel.S        | 16 ++++++++++++++++
>  arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds |  3 ++-
>  4 files changed, 29 insertions(+), 17 deletions(-)
>  create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S

[...]

> +++ b/arch/arm/cpu/armv7/sunxi/start_fel.S
> @@ -0,0 +1,16 @@
> +/*
> + * Entry point of the FEL mode SPL.
> + *
> + * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamashka@gmail.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <asm-offsets.h>
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(_start_fel)
> +	ldr	r9, =gdata
> +	b	s_init
> +ENDPROC(_start_fel)

In fact, we probably need to save/restore the r9 register and do it as:

    push    {r9, lr}
    ldr     r9, =gdata
    bl      s_init
    pop     {r9, pc}

And maybe save some other registers, depending on the calling
conventions expected by the FEL code in BROM.

As a side note, corrupting r9 mimics the old u-boot sunxi behaviour.
And it used not to cause any visible problems so far, at least
when working with the BROM code in the current Allwinner SoCs.

Also as I see it, the ".bss" sections is supposed to be in DRAM,
and cleared only after the DRAM is initialized. This violates the
C standard a little bit and enforces some sort of u-boot specific 
coding tricks. Such as explicitly placing gdata in the ".data"
section instead of ".bss". This is ugly, but probably justified.

I'll submit a fixed v2 version of this patch later, but will first
wait for additional comments from the other people.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Simon Glass]

Hi,

On 30 January 2015 at 04:58, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
> 'sunxi: Move SPL s_init() code to board_init_f()'
> broke the FEL boot mode.
>
> This patch moves the DRAM initialization back to s_init() and
> introduces an assembly entry point for FEL in order to provide
> guaranteed initialization of the gdata pointer (r9). The assembly
> entry point is also needed to ensure that the SPL code starts
> executing in ARM mode.
>
> Because the sunxi board_init_f() does not contain anything that
> is not already done by the default board_init_f(), it is removed
> too.
>
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> ---
>  arch/arm/cpu/armv7/sunxi/Makefile           |  1 +
>  arch/arm/cpu/armv7/sunxi/board.c            | 26 ++++++++++----------------
>  arch/arm/cpu/armv7/sunxi/start_fel.S        | 16 ++++++++++++++++
>  arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds |  3 ++-
>  4 files changed, 29 insertions(+), 17 deletions(-)
>  create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
>
> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> index 48db744..e0d413d 100644
> --- a/arch/arm/cpu/armv7/sunxi/Makefile
> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I)      += dram_sun4i.o
>  obj-$(CONFIG_MACH_SUN8I)       += dram_sun8i.o
>  ifdef CONFIG_SPL_FEL
>  obj-y  += start.o
> +extra-y        += start_fel.o
>  endif
>  endif
> diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> index 6e28bcd..ea6cb60 100644
> --- a/arch/arm/cpu/armv7/sunxi/board.c
> +++ b/arch/arm/cpu/armv7/sunxi/board.c
> @@ -85,6 +85,16 @@ void s_init(void)
>         timer_init();
>         gpio_init();
>         i2c_init_board();
> +
> +#ifdef CONFIG_SPL_BUILD
> +       preloader_console_init();

s_init() is called before we have global_data, so you can't use a console.

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

Why do you need this code here? Can it not go in board_init_f()?

>  }
>
>  #ifdef CONFIG_SPL_BUILD
> @@ -103,22 +113,6 @@ u32 spl_boot_mode(void)
>  {
>         return MMCSD_MODE_RAW;
>  }
> -
> -void board_init_f(ulong dummy)
> -{
> -       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)
> diff --git a/arch/arm/cpu/armv7/sunxi/start_fel.S b/arch/arm/cpu/armv7/sunxi/start_fel.S
> new file mode 100644
> index 0000000..e1c7cd4
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/sunxi/start_fel.S
> @@ -0,0 +1,16 @@
> +/*
> + * Entry point of the FEL mode SPL.
> + *
> + * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamashka@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <asm-offsets.h>
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(_start_fel)
> +       ldr     r9, =gdata
> +       b       s_init

No we don't want global data here, and need to get rid of gdata so we
can use driver model, etc.

> +ENDPROC(_start_fel)
> diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> index 928b7c1..beb8900 100644
> --- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> +++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> @@ -6,7 +6,7 @@
>   */
>  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>  OUTPUT_ARCH(arm)
> -ENTRY(s_init)
> +ENTRY(_start_fel)
>  SECTIONS
>  {
>         . = 0x00002000;
> @@ -14,6 +14,7 @@ SECTIONS
>         . = ALIGN(4);
>         .text :
>         {
> +               arch/arm/cpu/armv7/sunxi/start_fel.o    (.text)
>                 *(.text.s_init)

Why does this have to jump to a special s_init? Can it not just start
SPL normally as it does on Tegra, Exynos, etc?

>                 *(.text*)
>         }

There has to be a better way of making this work. Also do you have
instructions on how I can try this out on a pcduino3 or other low-cost
board?

I understand that we need to fix this, but other archs deal with this
within the existing framework, so I'd really like to get sunxi into
the same state.

Regards,
Simon

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Siarhei Siamashka]

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

> Hi,
> 
> On 30 January 2015 at 04:58, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
> > The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
> > 'sunxi: Move SPL s_init() code to board_init_f()'
> > broke the FEL boot mode.
> >
> > This patch moves the DRAM initialization back to s_init() and
> > introduces an assembly entry point for FEL in order to provide
> > guaranteed initialization of the gdata pointer (r9). The assembly
> > entry point is also needed to ensure that the SPL code starts
> > executing in ARM mode.
> >
> > Because the sunxi board_init_f() does not contain anything that
> > is not already done by the default board_init_f(), it is removed
> > too.
> >
> > Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> > ---
> >  arch/arm/cpu/armv7/sunxi/Makefile           |  1 +
> >  arch/arm/cpu/armv7/sunxi/board.c            | 26 ++++++++++----------------
> >  arch/arm/cpu/armv7/sunxi/start_fel.S        | 16 ++++++++++++++++
> >  arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds |  3 ++-
> >  4 files changed, 29 insertions(+), 17 deletions(-)
> >  create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
> >
> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> > index 48db744..e0d413d 100644
> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> > @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I)      += dram_sun4i.o
> >  obj-$(CONFIG_MACH_SUN8I)       += dram_sun8i.o
> >  ifdef CONFIG_SPL_FEL
> >  obj-y  += start.o
> > +extra-y        += start_fel.o
> >  endif
> >  endif
> > diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
> > index 6e28bcd..ea6cb60 100644
> > --- a/arch/arm/cpu/armv7/sunxi/board.c
> > +++ b/arch/arm/cpu/armv7/sunxi/board.c
> > @@ -85,6 +85,16 @@ void s_init(void)
> >         timer_init();
> >         gpio_init();
> >         i2c_init_board();
> > +
> > +#ifdef CONFIG_SPL_BUILD
> > +       preloader_console_init();
> 
> s_init() is called before we have global_data, so you can't use a console.

Oh, but somehow it just works for me.

> > +
> > +#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
> 
> Why do you need this code here?

The i2c_init() call is needed to initialize the PMIC as the comment
says. The PMIC is needed to set correct voltages, necessary for
the DRAM controller.

And the sunxi_board_init() initializes DRAM.

> Can it not go in board_init_f()?

Then we probably would need a special stripped down FEL variant of
board_init_f(), which makes the code a bit more messy.

> >  }
> >
> >  #ifdef CONFIG_SPL_BUILD
> > @@ -103,22 +113,6 @@ u32 spl_boot_mode(void)
> >  {
> >         return MMCSD_MODE_RAW;
> >  }
> > -
> > -void board_init_f(ulong dummy)
> > -{
> > -       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)
> > diff --git a/arch/arm/cpu/armv7/sunxi/start_fel.S b/arch/arm/cpu/armv7/sunxi/start_fel.S
> > new file mode 100644
> > index 0000000..e1c7cd4
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv7/sunxi/start_fel.S
> > @@ -0,0 +1,16 @@
> > +/*
> > + * Entry point of the FEL mode SPL.
> > + *
> > + * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamashka@gmail.com>
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <asm-offsets.h>
> > +#include <config.h>
> > +#include <linux/linkage.h>
> > +
> > +ENTRY(_start_fel)
> > +       ldr     r9, =gdata
> > +       b       s_init
> 
> No we don't want global data here, and need to get rid of gdata so we
> can use driver model, etc.

Appears that I need to educate myself on the global data vs. gdata
differences.

Using driver model in the sunxi SPL is a bit challenging because
we don't have abundant amounts of SRAM there:
    http://linux-sunxi.org/SRAM_Controller
Without relying on SoC-variant specific SRAM sections, we have 32 KiB
for normal SPL, and only ~15 KiB for FEL SPL (the rest is used by the
BROM FEL code). And SRAM is very much needed for the other important
features.

So far I can see that the pointer to gdata is stored in the r9 register.
And gdata resides in the ".data" section, which means that it is
initialized to 0 automatically. And this works now, unless I'm
misunderstanding something.

I would be more than happy to fix it in a future proof way. However it
is very much desired to have a properly functioning FEL boot mode in
u-boot 2015.04 and IMHO a quick hack relying on the legacy gdata feature
might be not the worst possible option today.

> > +ENDPROC(_start_fel)
> > diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> > index 928b7c1..beb8900 100644
> > --- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> > +++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
> > @@ -6,7 +6,7 @@
> >   */
> >  OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> >  OUTPUT_ARCH(arm)
> > -ENTRY(s_init)
> > +ENTRY(_start_fel)
> >  SECTIONS
> >  {
> >         . = 0x00002000;
> > @@ -14,6 +14,7 @@ SECTIONS
> >         . = ALIGN(4);
> >         .text :
> >         {
> > +               arch/arm/cpu/armv7/sunxi/start_fel.o    (.text)
> >                 *(.text.s_init)
> 
> Why does this have to jump to a special s_init? Can it not just start
> SPL normally as it does on Tegra, Exynos, etc?
>
> >                 *(.text*)
> >         }
> 
> There has to be a better way of making this work. Also do you have
> instructions on how I can try this out on a pcduino3 or other low-cost
> board?
> 
> I understand that we need to fix this, but other archs deal with this
> within the existing framework, so I'd really like to get sunxi into
> the same state.

For these parts, see my replies in:
    http://lists.denx.de/pipermail/u-boot/2015-February/203439.html
    http://lists.denx.de/pipermail/u-boot/2015-February/203485.html

I'm afraid that we can't always fit the BROM code into the existing
frameworks. But maybe some good solution exists for this particular
case.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Simon Glass]

Hi Siarhei,

On 1 February 2015 at 11:48, Siarhei Siamashka <siarhei.siamashka@gmail.com>
wrote:
> On Sun, 1 Feb 2015 09:28:36 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi,
>>
>> On 30 January 2015 at 04:58, Siarhei Siamashka
>> <siarhei.siamashka@gmail.com> wrote:
>> > The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
>> > 'sunxi: Move SPL s_init() code to board_init_f()'
>> > broke the FEL boot mode.
>> >
>> > This patch moves the DRAM initialization back to s_init() and
>> > introduces an assembly entry point for FEL in order to provide
>> > guaranteed initialization of the gdata pointer (r9). The assembly
>> > entry point is also needed to ensure that the SPL code starts
>> > executing in ARM mode.
>> >
>> > Because the sunxi board_init_f() does not contain anything that
>> > is not already done by the default board_init_f(), it is removed
>> > too.
>> >
>> > Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
>> > ---
>> > arch/arm/cpu/armv7/sunxi/Makefile | 1 +
>> > arch/arm/cpu/armv7/sunxi/board.c | 26 ++++++++++----------------
>> > arch/arm/cpu/armv7/sunxi/start_fel.S | 16 ++++++++++++++++
>> > arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 3 ++-
>> > 4 files changed, 29 insertions(+), 17 deletions(-)
>> > create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
>> >
>> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile
b/arch/arm/cpu/armv7/sunxi/Makefile
>> > index 48db744..e0d413d 100644
>> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
>> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
>> > @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I) += dram_sun4i.o
>> > obj-$(CONFIG_MACH_SUN8I) += dram_sun8i.o
>> > ifdef CONFIG_SPL_FEL
>> > obj-y += start.o
>> > +extra-y += start_fel.o
>> > endif
>> > endif
>> > diff --git a/arch/arm/cpu/armv7/sunxi/board.c
b/arch/arm/cpu/armv7/sunxi/board.c
>> > index 6e28bcd..ea6cb60 100644
>> > --- a/arch/arm/cpu/armv7/sunxi/board.c
>> > +++ b/arch/arm/cpu/armv7/sunxi/board.c
>> > @@ -85,6 +85,16 @@ void s_init(void)
>> > timer_init();
>> > gpio_init();
>> > i2c_init_board();
>> > +
>> > +#ifdef CONFIG_SPL_BUILD
>> > + preloader_console_init();
>>
>> s_init() is called before we have global_data, so you can't use a
console.
>
> Oh, but somehow it just works for me.

I should have said that there *should* be no global_data at this point,
i.e. we need to drop the hacks that add this. In fact global_data should be
set up once in crt0.S and not before.

>
>> > +
>> > +#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
>>
>> Why do you need this code here?
>
> The i2c_init() call is needed to initialize the PMIC as the comment
> says. The PMIC is needed to set correct voltages, necessary for
> the DRAM controller.
>
> And the sunxi_board_init() initializes DRAM.
>
>> Can it not go in board_init_f()?
>
> Then we probably would need a special stripped down FEL variant of
> board_init_f(), which makes the code a bit more messy.

Yes I think it is well worth figuring out the really differences are
between an SPL that boots from board storage and one that boots from FEL.
For Exynos is it just a single switch() to select the boot source. For
Tegra it essentially nothing.

Exynos has a flag that tells SPL when it is booting from USB A-A. Does
sunxi?

>
>> > }
>> >
>> > #ifdef CONFIG_SPL_BUILD
>> > @@ -103,22 +113,6 @@ u32 spl_boot_mode(void)
>> > {
>> > return MMCSD_MODE_RAW;
>> > }
>> > -
>> > -void board_init_f(ulong dummy)
>> > -{
>> > - 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)
>> > diff --git a/arch/arm/cpu/armv7/sunxi/start_fel.S
b/arch/arm/cpu/armv7/sunxi/start_fel.S
>> > new file mode 100644
>> > index 0000000..e1c7cd4
>> > --- /dev/null
>> > +++ b/arch/arm/cpu/armv7/sunxi/start_fel.S
>> > @@ -0,0 +1,16 @@
>> > +/*
>> > + * Entry point of the FEL mode SPL.
>> > + *
>> > + * Copyright (c) 2015 Siarhei Siamashka <siarhei.siamashka@gmail.com>
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0+
>> > + */
>> > +
>> > +#include <asm-offsets.h>
>> > +#include <config.h>
>> > +#include <linux/linkage.h>
>> > +
>> > +ENTRY(_start_fel)
>> > + ldr r9, =gdata
>> > + b s_init
>>
>> No we don't want global data here, and need to get rid of gdata so we
>> can use driver model, etc.
>
> Appears that I need to educate myself on the global data vs. gdata
> differences.
>
> Using driver model in the sunxi SPL is a bit challenging because
> we don't have abundant amounts of SRAM there:
> http://linux-sunxi.org/SRAM_Controller
> Without relying on SoC-variant specific SRAM sections, we have 32 KiB
> for normal SPL, and only ~15 KiB for FEL SPL (the rest is used by the
> BROM FEL code). And SRAM is very much needed for the other important
> features.
>
> So far I can see that the pointer to gdata is stored in the r9 register.
> And gdata resides in the ".data" section, which means that it is
> initialized to 0 automatically. And this works now, unless I'm
> misunderstanding something.
>
> I would be more than happy to fix it in a future proof way. However it
> is very much desired to have a properly functioning FEL boot mode in
> u-boot 2015.04 and IMHO a quick hack relying on the legacy gdata feature
> might be not the worst possible option today.
>
>> > +ENDPROC(_start_fel)
>> > diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
>> > index 928b7c1..beb8900 100644
>> > --- a/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
>> > +++ b/arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds
>> > @@ -6,7 +6,7 @@
>> > */
>> > OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>> > OUTPUT_ARCH(arm)
>> > -ENTRY(s_init)
>> > +ENTRY(_start_fel)
>> > SECTIONS
>> > {
>> > . = 0x00002000;
>> > @@ -14,6 +14,7 @@ SECTIONS
>> > . = ALIGN(4);
>> > .text :
>> > {
>> > + arch/arm/cpu/armv7/sunxi/start_fel.o (.text)
>> > *(.text.s_init)
>>
>> Why does this have to jump to a special s_init? Can it not just start
>> SPL normally as it does on Tegra, Exynos, etc?
>>
>> > *(.text*)
>> > }
>>
>> There has to be a better way of making this work. Also do you have
>> instructions on how I can try this out on a pcduino3 or other low-cost
>> board?
>>
>> I understand that we need to fix this, but other archs deal with this
>> within the existing framework, so I'd really like to get sunxi into
>> the same state.
>
> For these parts, see my replies in:
> http://lists.denx.de/pipermail/u-boot/2015-February/203439.html
> http://lists.denx.de/pipermail/u-boot/2015-February/203485.html
>
> I'm afraid that we can't always fit the BROM code into the existing
> frameworks. But maybe some good solution exists for this particular
> case.
>
> --
> Best regards,
> Siarhei Siamashka

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Siarhei Siamashka]

On Sun, 1 Feb 2015 13:59:41 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi Siarhei,
> 
> On 1 February 2015 at 11:48, Siarhei Siamashka <siarhei.siamashka@gmail.com>
> wrote:
> > On Sun, 1 Feb 2015 09:28:36 -0700
> > Simon Glass <sjg@chromium.org> wrote:
> >
> >> Hi,
> >>
> >> On 30 January 2015 at 04:58, Siarhei Siamashka
> >> <siarhei.siamashka@gmail.com> wrote:
> >> > The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
> >> > 'sunxi: Move SPL s_init() code to board_init_f()'
> >> > broke the FEL boot mode.
> >> >
> >> > This patch moves the DRAM initialization back to s_init() and
> >> > introduces an assembly entry point for FEL in order to provide
> >> > guaranteed initialization of the gdata pointer (r9). The assembly
> >> > entry point is also needed to ensure that the SPL code starts
> >> > executing in ARM mode.
> >> >
> >> > Because the sunxi board_init_f() does not contain anything that
> >> > is not already done by the default board_init_f(), it is removed
> >> > too.
> >> >
> >> > Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> >> > ---
> >> > arch/arm/cpu/armv7/sunxi/Makefile | 1 +
> >> > arch/arm/cpu/armv7/sunxi/board.c | 26 ++++++++++----------------
> >> > arch/arm/cpu/armv7/sunxi/start_fel.S | 16 ++++++++++++++++
> >> > arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 3 ++-
> >> > 4 files changed, 29 insertions(+), 17 deletions(-)
> >> > create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
> >> >
> >> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile
> b/arch/arm/cpu/armv7/sunxi/Makefile
> >> > index 48db744..e0d413d 100644
> >> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
> >> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> >> > @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I) += dram_sun4i.o
> >> > obj-$(CONFIG_MACH_SUN8I) += dram_sun8i.o
> >> > ifdef CONFIG_SPL_FEL
> >> > obj-y += start.o
> >> > +extra-y += start_fel.o
> >> > endif
> >> > endif
> >> > diff --git a/arch/arm/cpu/armv7/sunxi/board.c
> b/arch/arm/cpu/armv7/sunxi/board.c
> >> > index 6e28bcd..ea6cb60 100644
> >> > --- a/arch/arm/cpu/armv7/sunxi/board.c
> >> > +++ b/arch/arm/cpu/armv7/sunxi/board.c
> >> > @@ -85,6 +85,16 @@ void s_init(void)
> >> > timer_init();
> >> > gpio_init();
> >> > i2c_init_board();
> >> > +
> >> > +#ifdef CONFIG_SPL_BUILD
> >> > + preloader_console_init();
> >>
> >> s_init() is called before we have global_data, so you can't use a
> console.
> >
> > Oh, but somehow it just works for me.
> 
> I should have said that there *should* be no global_data at this point,
> i.e. we need to drop the hacks that add this. In fact global_data should be
> set up once in crt0.S and not before.

OK, I understand.

However crt0.S does not seem to be particularly compatible with FEL.
It tries to override the stack pointer (in the FEL mode we need to
use the original stack pointer provided to us by the BROM). And
implies that 'board_init_f' needs to return control back to the
BROM, however doing return from multiple nested levels of function
calls is a tricky exercise, especially considering that the stack
has been already moved elsewhere.

On the other hand, I see that crt0.S just allocates global data on
stack, clears it and then sets all the same r9 register. So what
is the real difference compared to having global data defined in
the ".data" section?

I would say that right now an easy hack would be to remove gdata
globally in u-boot (that's an admirable goal), but keep it with a
different name under the CONFIG_SPL_FEL define just for sunxi.
We might just rework this patch by providing the following FEL
entry point:

    push    {r9, lr}
    ldr     r9, =sunxi_fel_gdata
    bl      s_init
    bl      board_init_f
    pop     {r9, pc}

Then hide the unneeded parts of board_init_f() under
!defined(CONFIG_SPL_FEL) check, so that it just returns
right after initializing DRAM.

I see that the rationale for gdata removal is to allow having an
early malloc pool for the driver model in SPL:
    http://lists.denx.de/pipermail/u-boot/2014-December/199528.html

I think that you can keep experimenting with the driver model
on sunxi with the regular SPL build, but just leave the FEL mode
build alone for now. It is not like u-boot is going to ever switch
to the driver model in the SPL on every platform (there are platforms
where the SRAM size is extremely small, even smaller than on sunxi),
so the sunxi FEL mode is not going to be alone doing this.

> >> > +
> >> > +#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
> >>
> >> Why do you need this code here?
> >
> > The i2c_init() call is needed to initialize the PMIC as the comment
> > says. The PMIC is needed to set correct voltages, necessary for
> > the DRAM controller.
> >
> > And the sunxi_board_init() initializes DRAM.
> >
> >> Can it not go in board_init_f()?
> >
> > Then we probably would need a special stripped down FEL variant of
> > board_init_f(), which makes the code a bit more messy.
> 
> Yes I think it is well worth figuring out the really differences are
> between an SPL that boots from board storage and one that boots from FEL.
> For Exynos is it just a single switch() to select the boot source. For
> Tegra it essentially nothing.
> 
> Exynos has a flag that tells SPL when it is booting from USB A-A. Does
> sunxi?

Currently sunxi has the CONFIG_SPL_FEL define.

Even though I'm generally in favour of runtime detection, we can't do
it in this case. This is only because the amount of available SRAM
space is much smaller in the FEL mode and the normal SPL simply does
not fit. If we could afford it, then surely having a single SPL
binary (both bootable in the USB FEL mode and from the SD card) would
be much preferable without separate '*_felconfig' and '*_defconfig'
configs.

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH 0/4] sunxi: FEL mode fixes

[Tom Rini]

On Fri, Jan 30, 2015 at 01:58:45PM +0200, Siarhei Siamashka wrote:

> The recent u-boot changes broke FEL mode support on sunxi hardware.
> This patch series fixes the regression and also introduces some other
> cleanups.

I think the community at large here could benefit from some background
and details on FEL.  I found http://linux-sunxi.org/FEL and I feel a bit
more enlightened now, but what exactly is going on?  Are we being loaded
by the ROM or do we get loaded, make use of ROM and then continue
executing?  a new payload gets executed?

-- 
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/20150202/dfd9e955/attachment.sig>

[U-Boot] [PATCH 1/4] sunxi: Make FEL mode usable again

[Simon Glass]

Hi Siarhei,

On 1 February 2015 at 16:59, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
>
> On Sun, 1 Feb 2015 13:59:41 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Siarhei,
> >
> > On 1 February 2015 at 11:48, Siarhei Siamashka <siarhei.siamashka@gmail.com>
> > wrote:
> > > On Sun, 1 Feb 2015 09:28:36 -0700
> > > Simon Glass <sjg@chromium.org> wrote:
> > >
> > >> Hi,
> > >>
> > >> On 30 January 2015 at 04:58, Siarhei Siamashka
> > >> <siarhei.siamashka@gmail.com> wrote:
> > >> > The commit f630974ccb3ce93e9607a3354e9acb266a8b7e95
> > >> > 'sunxi: Move SPL s_init() code to board_init_f()'
> > >> > broke the FEL boot mode.
> > >> >
> > >> > This patch moves the DRAM initialization back to s_init() and
> > >> > introduces an assembly entry point for FEL in order to provide
> > >> > guaranteed initialization of the gdata pointer (r9). The assembly
> > >> > entry point is also needed to ensure that the SPL code starts
> > >> > executing in ARM mode.
> > >> >
> > >> > Because the sunxi board_init_f() does not contain anything that
> > >> > is not already done by the default board_init_f(), it is removed
> > >> > too.
> > >> >
> > >> > Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> > >> > ---
> > >> > arch/arm/cpu/armv7/sunxi/Makefile | 1 +
> > >> > arch/arm/cpu/armv7/sunxi/board.c | 26 ++++++++++----------------
> > >> > arch/arm/cpu/armv7/sunxi/start_fel.S | 16 ++++++++++++++++
> > >> > arch/arm/cpu/armv7/sunxi/u-boot-spl-fel.lds | 3 ++-
> > >> > 4 files changed, 29 insertions(+), 17 deletions(-)
> > >> > create mode 100644 arch/arm/cpu/armv7/sunxi/start_fel.S
> > >> >
> > >> > diff --git a/arch/arm/cpu/armv7/sunxi/Makefile
> > b/arch/arm/cpu/armv7/sunxi/Makefile
> > >> > index 48db744..e0d413d 100644
> > >> > --- a/arch/arm/cpu/armv7/sunxi/Makefile
> > >> > +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> > >> > @@ -40,5 +40,6 @@ obj-$(CONFIG_MACH_SUN7I) += dram_sun4i.o
> > >> > obj-$(CONFIG_MACH_SUN8I) += dram_sun8i.o
> > >> > ifdef CONFIG_SPL_FEL
> > >> > obj-y += start.o
> > >> > +extra-y += start_fel.o
> > >> > endif
> > >> > endif
> > >> > diff --git a/arch/arm/cpu/armv7/sunxi/board.c
> > b/arch/arm/cpu/armv7/sunxi/board.c
> > >> > index 6e28bcd..ea6cb60 100644
> > >> > --- a/arch/arm/cpu/armv7/sunxi/board.c
> > >> > +++ b/arch/arm/cpu/armv7/sunxi/board.c
> > >> > @@ -85,6 +85,16 @@ void s_init(void)
> > >> > timer_init();
> > >> > gpio_init();
> > >> > i2c_init_board();
> > >> > +
> > >> > +#ifdef CONFIG_SPL_BUILD
> > >> > + preloader_console_init();
> > >>
> > >> s_init() is called before we have global_data, so you can't use a
> > console.
> > >
> > > Oh, but somehow it just works for me.
> >
> > I should have said that there *should* be no global_data at this point,
> > i.e. we need to drop the hacks that add this. In fact global_data should be
> > set up once in crt0.S and not before.
>
> OK, I understand.
>
> However crt0.S does not seem to be particularly compatible with FEL.
> It tries to override the stack pointer (in the FEL mode we need to
> use the original stack pointer provided to us by the BROM). And
> implies that 'board_init_f' needs to return control back to the
> BROM, however doing return from multiple nested levels of function
> calls is a tricky exercise, especially considering that the stack
> has been already moved elsewhere.

How about we save sp and lr in other registers before they get changed.

Then sunxi lowlevel_init can stash them in SRAM, or if we are clever,
board_init_f() can do it.

Then we can just restore the original stack and jump to lr when SPL is finished.

I see no problem with using our own stack in SPL. In fact I'd prefer it.

>
> On the other hand, I see that crt0.S just allocates global data on
> stack, clears it and then sets all the same r9 register. So what
> is the real difference compared to having global data defined in
> the ".data" section?
>
> I would say that right now an easy hack would be to remove gdata
> globally in u-boot (that's an admirable goal), but keep it with a
> different name under the CONFIG_SPL_FEL define just for sunxi.
> We might just rework this patch by providing the following FEL
> entry point:
>
>     push    {r9, lr}
>     ldr     r9, =sunxi_fel_gdata
>     bl      s_init
>     bl      board_init_f
>     pop     {r9, pc}

No, let's just declare a locate data section variable for sunxi. It
does not need to go in global_data. That just confuses things.

>
> Then hide the unneeded parts of board_init_f() under
> !defined(CONFIG_SPL_FEL) check, so that it just returns
> right after initializing DRAM.

Can we not return in board_init_r() like SPL normally does? Even in
FEL mode we might want to do something else.

>
> I see that the rationale for gdata removal is to allow having an
> early malloc pool for the driver model in SPL:
>     http://lists.denx.de/pipermail/u-boot/2014-December/199528.html
>
> I think that you can keep experimenting with the driver model
> on sunxi with the regular SPL build, but just leave the FEL mode
> build alone for now. It is not like u-boot is going to ever switch
> to the driver model in the SPL on every platform (there are platforms
> where the SRAM size is extremely small, even smaller than on sunxi),
> so the sunxi FEL mode is not going to be alone doing this.

I believe that even FEL has enough SRAM for driver model, but I agree
there will be cases where it is impossible (e.g. Atmel's 4KB seems
really hard).

>
> > >> > +
> > >> > +#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
> > >>
> > >> Why do you need this code here?
> > >
> > > The i2c_init() call is needed to initialize the PMIC as the comment
> > > says. The PMIC is needed to set correct voltages, necessary for
> > > the DRAM controller.
> > >
> > > And the sunxi_board_init() initializes DRAM.
> > >
> > >> Can it not go in board_init_f()?
> > >
> > > Then we probably would need a special stripped down FEL variant of
> > > board_init_f(), which makes the code a bit more messy.
> >
> > Yes I think it is well worth figuring out the really differences are
> > between an SPL that boots from board storage and one that boots from FEL.
> > For Exynos is it just a single switch() to select the boot source. For
> > Tegra it essentially nothing.
> >
> > Exynos has a flag that tells SPL when it is booting from USB A-A. Does
> > sunxi?
>
> Currently sunxi has the CONFIG_SPL_FEL define.
>
> Even though I'm generally in favour of runtime detection, we can't do
> it in this case. This is only because the amount of available SRAM
> space is much smaller in the FEL mode and the normal SPL simply does
> not fit. If we could afford it, then surely having a single SPL
> binary (both bootable in the USB FEL mode and from the SD card) would
> be much preferable without separate '*_felconfig' and '*_defconfig'
> configs.

Oh dear that is awful! Perhaps the build should create two SPLs, one
for FEL and one for normal? That avoids all the fiddling, although
presumably FEL mode is mostly for U-Boot development?

Why does FEL mode need so much SRAM? The model is wrong. It should not
be calling into SPL - maybe someone should take this up with Allwinner
for future chips.

Regards,
Simon