* [U-Boot] [RFC 1/3] ARM,crt0.S: call s_init instead from ctr0.S
2013-08-24 16:32 [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
@ 2013-08-24 16:32 ` Jeroen Hofstee
2013-08-24 16:41 ` Jeroen Hofstee
2013-08-24 16:32 ` [U-Boot] [RFC 2/3] ARM,crt0.S: optional init gd to gdata for spl Jeroen Hofstee
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Jeroen Hofstee @ 2013-08-24 16:32 UTC (permalink / raw)
To: u-boot
The only reason gd is setup before _main seems to be
the call to s_init. Therefore call s_init in crt0.S
after gd has been setup. (It might become system_init
one day, but that has the disadvantage that all versions
of board_init_f must call system_init).
---
arch/arm/cpu/armv7/lowlevel_init.S | 23 +----------------------
arch/arm/cpu/armv7/omap3/lowlevel_init.S | 8 ++++----
arch/arm/cpu/armv7/rmobile/lowlevel_init.S | 6 ------
arch/arm/lib/crt0.S | 3 +++
arch/arm/lib/reset.c | 5 +++++
board/ti/omap5912osk/lowlevel_init.S | 11 -----------
6 files changed, 13 insertions(+), 43 deletions(-)
diff --git a/arch/arm/cpu/armv7/lowlevel_init.S b/arch/arm/cpu/armv7/lowlevel_init.S
index 82b2b86..1ec7481 100644
--- a/arch/arm/cpu/armv7/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/lowlevel_init.S
@@ -16,26 +16,5 @@
#include <linux/linkage.h>
ENTRY(lowlevel_init)
- /*
- * Setup a temporary stack
- */
- ldr sp, =CONFIG_SYS_INIT_SP_ADDR
- bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
-#ifdef CONFIG_SPL_BUILD
- ldr r8, =gdata
-#else
- sub sp, #GD_SIZE
- bic sp, sp, #7
- mov r8, sp
-#endif
- /*
- * Save the old lr(passed in ip) and the current lr to stack
- */
- push {ip, lr}
-
- /*
- * go setup pll, mux, memory
- */
- bl s_init
- pop {ip, pc}
+ mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
index bdf74ea..db1d4dc 100644
--- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S
@@ -197,22 +197,22 @@ pll_div_val5:
#endif
ENTRY(lowlevel_init)
+#if !defined(CONFIG_SYS_NAND_BOOT) && !defined(CONFIG_SYS_ONENAND_BOOT)
ldr sp, SRAM_STACK
str ip, [sp] /* stash ip register */
mov ip, lr /* save link reg across call */
-#if !defined(CONFIG_SYS_NAND_BOOT) && !defined(CONFIG_SYS_ONENAND_BOOT)
/*
* No need to copy/exec the clock code - DPLL adjust already done
* in NAND/oneNAND Boot.
*/
ldr r1, =SRAM_CLK_CODE
bl cpy_clk_code
-#endif /* NAND Boot */
+
mov lr, ip /* restore link reg */
ldr ip, [sp] /* restore save ip */
- /* tail-call s_init to setup pll, mux, memory */
- b s_init
+#endif /* NAND Boot */
+ mov pc, lr
ENDPROC(lowlevel_init)
/* the literal pools origin */
diff --git a/arch/arm/cpu/armv7/rmobile/lowlevel_init.S b/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
index ff18d96..70ec22d 100644
--- a/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
+++ b/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
@@ -59,14 +59,8 @@ loop0:
subs r0, r0, #1
bne loop0
- ldr sp, MERAM_STACK
- b s_init
-
.pool
.align 4
ENDPROC(lowlevel_init)
.ltorg
-
-MERAM_STACK:
- .word LOW_LEVEL_MERAM_STACK
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 960d12e..98d7881 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -70,6 +70,9 @@ ENTRY(_main)
sub sp, #GD_SIZE /* allocate one GD above SP */
bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
mov r8, sp /* GD is above SP */
+
+ bl s_init
+
mov r0, #0
bl board_init_f
diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
index 7a03580..d14eac9 100644
--- a/arch/arm/lib/reset.c
+++ b/arch/arm/lib/reset.c
@@ -35,3 +35,8 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
/*NOTREACHED*/
return 0;
}
+
+/* HACK */
+__weak void s_init(void)
+{
+}
diff --git a/board/ti/omap5912osk/lowlevel_init.S b/board/ti/omap5912osk/lowlevel_init.S
index cad0a5a..61c8605 100644
--- a/board/ti/omap5912osk/lowlevel_init.S
+++ b/board/ti/omap5912osk/lowlevel_init.S
@@ -296,17 +296,6 @@ common_tc:
ldr sp, SRAM_STACK
bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
- /*
- * Save the old lr(passed in ip) and the current lr to stack
- */
- push {ip, lr}
-
- /*
- * go setup pll, mux, memory
- */
- bl s_init
- pop {ip, pc}
-
/* back to arch calling code */
mov pc, lr
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] [RFC 1/3] ARM,crt0.S: call s_init instead from ctr0.S
2013-08-24 16:32 ` [U-Boot] [RFC 1/3] ARM,crt0.S: call s_init instead from ctr0.S Jeroen Hofstee
@ 2013-08-24 16:41 ` Jeroen Hofstee
0 siblings, 0 replies; 7+ messages in thread
From: Jeroen Hofstee @ 2013-08-24 16:41 UTC (permalink / raw)
To: u-boot
On 08/24/2013 06:32 PM, Jeroen Hofstee wrote:
> /* the literal pools origin */
> diff --git a/arch/arm/cpu/armv7/rmobile/lowlevel_init.S b/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
> index ff18d96..70ec22d 100644
> --- a/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
> +++ b/arch/arm/cpu/armv7/rmobile/lowlevel_init.S
> @@ -59,14 +59,8 @@ loop0:
> subs r0, r0, #1
> bne loop0
>
> - ldr sp, MERAM_STACK
> - b s_init
> -
right, this ^^^ will brick things nicely, how about returning to the
caller...
> .pool
> .align 4
>
> ENDPROC(lowlevel_init)
> .ltorg
> -
> -MERAM_STACK:
> - .word LOW_LEVEL_MERAM_STACK
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [RFC 2/3] ARM,crt0.S: optional init gd to gdata for spl
2013-08-24 16:32 [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
2013-08-24 16:32 ` [U-Boot] [RFC 1/3] ARM,crt0.S: call s_init instead from ctr0.S Jeroen Hofstee
@ 2013-08-24 16:32 ` Jeroen Hofstee
2013-08-24 16:32 ` [U-Boot] [RFC 3/3] ARM: do not assign gd outside of crt0.S Jeroen Hofstee
2013-09-22 18:26 ` [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
3 siblings, 0 replies; 7+ messages in thread
From: Jeroen Hofstee @ 2013-08-24 16:32 UTC (permalink / raw)
To: u-boot
The common/spl changes gd to gdata in board_init_f.
I fail to see why. The only reason I can think of to
use the gdata is the case where is won't fit into the
region where the stack lives.
Move the init to crt0.S and make it a CONFIG.
---
arch/arm/lib/crt0.S | 5 +++++
arch/arm/lib/spl.c | 5 ++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 98d7881..b8fa10e 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -67,9 +67,14 @@ ENTRY(_main)
ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
#endif
bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
+
+#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_GD_GLOBAL)
+ ldr r8, =gdata /* SPL assigns GD directly to &gdata */
+#else
sub sp, #GD_SIZE /* allocate one GD above SP */
bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
mov r8, sp /* GD is above SP */
+#endif
bl s_init
diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
index 583bdb3..52dba2f 100644
--- a/arch/arm/lib/spl.c
+++ b/arch/arm/lib/spl.c
@@ -15,7 +15,9 @@
/* Pointer to as well as the global data structure for SPL */
DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_SPL_GD_GLOBAL
gd_t gdata __attribute__ ((section(".data")));
+#endif
/*
* In the context of SPL, board_init_f must ensure that any clocks/etc for
@@ -31,9 +33,6 @@ void __weak board_init_f(ulong dummy)
/* Clear the BSS. */
memset(__bss_start, 0, __bss_end - __bss_start);
- /* Set global data pointer. */
- gd = &gdata;
-
board_init_r(NULL, 0);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] [RFC 3/3] ARM: do not assign gd outside of crt0.S
2013-08-24 16:32 [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
2013-08-24 16:32 ` [U-Boot] [RFC 1/3] ARM,crt0.S: call s_init instead from ctr0.S Jeroen Hofstee
2013-08-24 16:32 ` [U-Boot] [RFC 2/3] ARM,crt0.S: optional init gd to gdata for spl Jeroen Hofstee
@ 2013-08-24 16:32 ` Jeroen Hofstee
2013-09-22 18:26 ` [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
3 siblings, 0 replies; 7+ messages in thread
From: Jeroen Hofstee @ 2013-08-24 16:32 UTC (permalink / raw)
To: u-boot
---
arch/arm/cpu/arm926ejs/davinci/spl.c | 3 +--
arch/arm/cpu/armv7/exynos/spl_boot.c | 6 ++----
arch/arm/cpu/armv7/omap-common/hwinit-common.c | 2 --
arch/arm/cpu/armv7/omap3/board.c | 2 --
board/isee/igep0033/board.c | 1 -
board/phytec/pcm051/board.c | 2 --
board/ti/am335x/board.c | 2 --
board/ti/ti814x/evm.c | 2 --
board/woodburn/woodburn.c | 3 ---
9 files changed, 3 insertions(+), 20 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/davinci/spl.c b/arch/arm/cpu/arm926ejs/davinci/spl.c
index 59b304e..0af3646 100644
--- a/arch/arm/cpu/arm926ejs/davinci/spl.c
+++ b/arch/arm/cpu/arm926ejs/davinci/spl.c
@@ -50,8 +50,7 @@ void board_init_f(ulong dummy)
/* Third, we clear the BSS. */
memset(__bss_start, 0, __bss_end - __bss_start);
- /* Finally, setup gd and move to the next step. */
- gd = &gdata;
+ /* Finally move to the next step. */
board_init_r(NULL, 0);
}
diff --git a/arch/arm/cpu/armv7/exynos/spl_boot.c b/arch/arm/cpu/armv7/exynos/spl_boot.c
index 3651c00..c0287aa 100644
--- a/arch/arm/cpu/armv7/exynos/spl_boot.c
+++ b/arch/arm/cpu/armv7/exynos/spl_boot.c
@@ -149,9 +149,8 @@ void memzero(void *s, size_t n)
*
* @param gdp Value to give to gd
*/
-static void setup_global_data(gd_t *gdp)
+static void setup_global_data(void)
{
- gd = gdp;
memzero((void *)gd, sizeof(gd_t));
gd->flags |= GD_FLG_RELOC;
gd->baudrate = CONFIG_BAUDRATE;
@@ -160,10 +159,9 @@ static void setup_global_data(gd_t *gdp)
void board_init_f(unsigned long bootflag)
{
- __aligned(8) gd_t local_gd;
__attribute__((noreturn)) void (*uboot)(void);
- setup_global_data(&local_gd);
+ setup_global_data();
if (do_lowlevel_init())
power_exit_wakeup();
diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c
index 85d3754..69cbfd6 100644
--- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c
+++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c
@@ -143,8 +143,6 @@ void s_init(void)
srcomp_enable();
setup_clocks_for_console();
- gd = &gdata;
-
preloader_console_init();
do_io_settings();
#endif
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c
index 7d1f8d9..3c0042d 100644
--- a/arch/arm/cpu/armv7/omap3/board.c
+++ b/arch/arm/cpu/armv7/omap3/board.c
@@ -240,8 +240,6 @@ void s_init(void)
#endif
#ifdef CONFIG_SPL_BUILD
- gd = &gdata;
-
preloader_console_init();
timer_init();
diff --git a/board/isee/igep0033/board.c b/board/isee/igep0033/board.c
index c0f0c0d..8cb9e4c 100644
--- a/board/isee/igep0033/board.c
+++ b/board/isee/igep0033/board.c
@@ -102,7 +102,6 @@ void s_init(void)
enable_uart0_pin_mux();
uart_soft_reset();
- gd = &gdata;
preloader_console_init();
diff --git a/board/phytec/pcm051/board.c b/board/phytec/pcm051/board.c
index 6291d03..3bf45ac 100644
--- a/board/phytec/pcm051/board.c
+++ b/board/phytec/pcm051/board.c
@@ -113,8 +113,6 @@ void s_init(void)
enable_uart0_pin_mux();
uart_soft_reset();
- gd = &gdata;
-
preloader_console_init();
/* Initalize the board header */
diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
index 7138d73..f11eb93 100644
--- a/board/ti/am335x/board.c
+++ b/board/ti/am335x/board.c
@@ -328,8 +328,6 @@ void s_init(void)
uart_soft_reset();
- gd = &gdata;
-
preloader_console_init();
/* Initalize the board header */
diff --git a/board/ti/ti814x/evm.c b/board/ti/ti814x/evm.c
index 17fba5a..7800a4d 100644
--- a/board/ti/ti814x/evm.c
+++ b/board/ti/ti814x/evm.c
@@ -143,8 +143,6 @@ void s_init(void)
/* Enable UART */
uart_enable();
- gd = &gdata;
-
preloader_console_init();
config_dmm(&evm_lisa_map_regs);
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();
--
1.8.1.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] [RFC 0/3] ARM: cleanup gd init
2013-08-24 16:32 [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
` (2 preceding siblings ...)
2013-08-24 16:32 ` [U-Boot] [RFC 3/3] ARM: do not assign gd outside of crt0.S Jeroen Hofstee
@ 2013-09-22 18:26 ` Jeroen Hofstee
2013-09-28 20:39 ` Albert ARIBAUD
3 siblings, 1 reply; 7+ messages in thread
From: Jeroen Hofstee @ 2013-09-22 18:26 UTC (permalink / raw)
To: u-boot
Hello,
On 08/24/2013 06:32 PM, Jeroen Hofstee wrote:
> 4) Keep the s_init in crt0.S or move it to the board_init_f?
> The disadvantage of the later is that all the different
> board_init_f's need to call system_init.
Since this has been on the mailing-list for a month without a reply,
let's push this a bit.
Moving s_init to board_init_f is not smart since there are many
board_init_f already and likely there will be more in the future
since SPL uses it as well. It also provides a chance to save bootrom
registers to gd. After boad_init_f r1 is clobbered at least..
Regards,
Jeroen
^ permalink raw reply [flat|nested] 7+ messages in thread* [U-Boot] [RFC 0/3] ARM: cleanup gd init
2013-09-22 18:26 ` [U-Boot] [RFC 0/3] ARM: cleanup gd init Jeroen Hofstee
@ 2013-09-28 20:39 ` Albert ARIBAUD
0 siblings, 0 replies; 7+ messages in thread
From: Albert ARIBAUD @ 2013-09-28 20:39 UTC (permalink / raw)
To: u-boot
Hi Jeroen,
On Sun, 22 Sep 2013 20:26:07 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:
> Hello,
>
> On 08/24/2013 06:32 PM, Jeroen Hofstee wrote:
> > 4) Keep the s_init in crt0.S or move it to the board_init_f?
> > The disadvantage of the later is that all the different
> > board_init_f's need to call system_init.
> Since this has been on the mailing-list for a month without a reply,
> let's push this a bit.
>
> Moving s_init to board_init_f is not smart since there are many
> board_init_f already and likely there will be more in the future
> since SPL uses it as well. It also provides a chance to save bootrom
> registers to gd. After boad_init_f r1 is clobbered at least..
Not sure what you mean by there being "many" board_init_f()s -- I
count five occurrences, one of which is the standard board_init_f(),
one is a weak default in SPL, and three are custom versions; the last
four may or may not be merged into 'the' board_init_f() one.
Regarding saving bootrom (or other) registers, that is a feature for
start.S, more precisely from _start, which is the only place in the
whole of U-Boot that has not clobbered any register yet.
The right process is thus to split s_init(): any part that deals with
saving registers at boot should go in start.S; any part that deals
with anything else and is not absolutely required before entering
crt0.S should move in a function called from board_init_f().
> Regards,
> Jeroen
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 7+ messages in thread