[U-Boot] [PATCH 0/4] sunxi: FEL mode fixes
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
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*) }
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.
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, =gdatab 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
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, =gdatab s_initNo 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.
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
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.
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
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 */
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' */
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 */
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?
participants (3)
-
Siarhei Siamashka -
Simon Glass -
Tom Rini