ARmv7: Add a soc_init hook to start.S

Replace our current DIY solution for setting the Cortex A7 ACTLR.SMP bit with using the new soc_init hook and cpu_init_cortex_a7 helper function.

Signed-off-by Hans de Goede <[email protected]>

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

Quoted 1 line

Quoted 1 line

-64,7 +64,7 @@ reset:

	/* the mask ROM code should have PLL and others stable */

#ifndef CONFIG_SKIP_LOWLEVEL_INIT

arch/arm/cpu/armv7/start.S +1 -1

-	bl	cpu_init_cp15
+	bl	soc_init

bl cpu_init_crit
#endif

Quoted 1 line

Quoted 1 line

-102,6 +102,22 @@ ENDPROC(save_boot_params)

/*************************************************************************
*
+ * void soc_init(void)
+ * __attribute__((weak));
+ *
+ * Stack pointer is not yet initialized at this moment
+ * Don't save anything to stack even if compiled with -O0
+ *
+ *************************************************************************/

arch/arm/cpu/armv7/start.S +3 -0

+
+/*************************************************************************
+ *
 * cpu_init_cp15
 *
 * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless

Quoted 36 lines · hdegoede at redhat.com

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

Quoted 1 line

Quoted 1 line

-64,7 +64,7 @@ reset:

	/* the mask ROM code should have PLL and others stable */

#ifndef CONFIG_SKIP_LOWLEVEL_INIT

arch/arm/cpu/armv7/start.S +1 -1

-	bl	cpu_init_cp15
+	bl	soc_init

bl cpu_init_crit
#endif

Quoted 1 line

Quoted 1 line

-102,6 +102,22 @@ ENDPROC(save_boot_params)

/*************************************************************************
*
+ * void soc_init(void)
+ * __attribute__((weak));
+ *
+ * Stack pointer is not yet initialized at this moment
+ * Don't save anything to stack even if compiled with -O0
+ *
+ *************************************************************************/

arch/arm/cpu/armv7/start.S ENDPROC(save_boot_params) · line 117

Quoted 6 lines

arch/arm/cpu/armv7/start.S +6 -0

+ENTRY(soc_init)
+	mov	r9, lr
+	bl	cpu_init_cp15
+	mov	pc, r9			@ back to my caller
+ENDPROC(soc_init)
+	.weak	soc_init

You could just use a 'tail call' and make this,

+ENTRY(soc_init)
+ b cpu_init_cp15
+ENDPROC(soc_init)

Or even as the code follows just add a duplicate label, so 'soc_init' is a weak version of 'cpu_init_cp15'? This gives no additional code in a final binary. I guess it depends on how the generic 'soc_init' might be modified in the future?

If we put code after the 'cpu_init_cp15' in the generic version, then we should keep saving the 'lr' into 'r9'; also 'cpu_init_cp15' should never use 'r9'. This maybe good to document (or someone may break your sunxi code).

arch/arm/cpu/armv7/start.S ENDPROC(save_boot_params) · line 123

Quoted 7 lines

arch/arm/cpu/armv7/start.S +3 -0

+
+/*************************************************************************
+ *
  * cpu_init_cp15
  *
  * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless

Quoted 41 lines · hdegoede at redhat.com

Quoted 39 lines

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-

1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

Quoted 1 line

Quoted 1 line

-64,7 +64,7 @@ reset:

	/* the mask ROM code should have PLL and others stable */

#ifndef CONFIG_SKIP_LOWLEVEL_INIT

arch/arm/cpu/armv7/start.S +1 -1

-	bl	cpu_init_cp15
+	bl	soc_init

bl cpu_init_crit
#endif

Quoted 1 line

Quoted 1 line

-102,6 +102,22 @@ ENDPROC(save_boot_params)

/*************************************************************************
* + * void soc_init(void) + * __attribute__((weak)); + * + * Stack pointer is not yet initialized at this moment + * Don't save anything to stack even if compiled with -O0 + * + *************************************************************************/

arch/arm/cpu/armv7/start.S +6 -0

+ENTRY(soc_init)
+ mov r9, lr 
+ bl cpu_init_cp15 
+ mov pc, r9 @ back to my caller 
+ENDPROC(soc_init)
+ .weak soc_init

Quoted 6 lines · Bill Pringlemeir

You could just use a 'tail call' and make this,

arch/arm/cpu/armv7/start.S +3 -0

+ENTRY(soc_init)
+	b	cpu_init_cp15
+ENDPROC(soc_init)

True, although I think that actually saving lr to r9 is useful as an example for boards who want to override this, and that we can spare the 2 extra instructions :)

Quoted 4 lines

Or even as the code follows just add a duplicate label, so 'soc_init' is a weak version of 'cpu_init_cp15'? This gives no additional code in a final binary. I guess it depends on how the generic 'soc_init' might be modified in the future?

I think that having a separate entry for it is much more clear, otherwise the entire symbol will be hard to find / grok for people trying to get into the code.

Albert what do you want to do here?

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
  	/* the mask ROM code should have PLL and others stable */
  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
  	bl	cpu_init_crit
  #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
  	/* the mask ROM code should have PLL and others stable */
  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
  	bl	cpu_init_crit
  #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
	bl	cpu_init_cp15

bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
  	/* the mask ROM code should have PLL and others stable */
  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
  	bl	cpu_init_crit
  #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
	bl	cpu_init_cp15
    	bl	cpu_init_crit

#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Hi,

Quoted 105 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
	bl	cpu_init_cp15

bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

Quoted 9 lines

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

Quoted 118 lines · Hans de Goede

Hi,

Quoted 114 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
	bl	cpu_init_cp15

bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Quoted 4 lines

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

I suppose that's what happens when you have a detailed plan, will do ;)

Signature · 1 line

Tom

Hello Tom,

Quoted 124 lines · Tom Rini

Quoted 122 lines · Hans de Goede

Hi,

Quoted 105 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
	bl	cpu_init_cp15

bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

Quoted 9 lines

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

Quoted 1 line

I suppose that's what happens when you have a detailed plan, will do ;)

I'll comment if the patch is out (going through my mail one at a time) but I just want to chime in on C contexts: I'd rather have this done only once and from crt0.S. If this means crt0.S must do the call to lowlevel_init, then so be it IMO.

Quoted 127 lines · Albert ARIBAUD

Hello Tom,

Quoted 124 lines · Tom Rini

Quoted 122 lines · Hans de Goede

Hi,

Quoted 105 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
   /* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-   bl      cpu_init_cp15
+   bl      soc_init
   bl      cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl cpu_init_cp15
bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

Quoted 9 lines

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

Quoted 6 lines

Quoted 1 line

I suppose that's what happens when you have a detailed plan, will do ;)

I'll comment if the patch is out (going through my mail one at a time) but I just want to chime in on C contexts: I'd rather have this done only once and from crt0.S. If this means crt0.S must do the call to lowlevel_init, then so be it IMO.

This might be workable, yeah. What I've decided after doing some of the work is that the rest of arm has cpu_init_crit and some callouts and I'm thinking that rather than make armv7 much different we should re-adjust things to fit back into the regular mold which I think is possible.

Simon, do you think we could move arch_cpu_init up in the call sequence in common/board_f.c ?

Hi Tom,

Quoted 141 lines · Tom Rini

Quoted 127 lines · Albert ARIBAUD

Hello Tom,

Quoted 124 lines · Tom Rini

Quoted 122 lines · Hans de Goede

Hi,

Quoted 105 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
   /* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-   bl      cpu_init_cp15
+   bl      soc_init
   bl      cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl cpu_init_cp15
bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

Quoted 9 lines

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

Quoted 6 lines

Quoted 1 line

I suppose that's what happens when you have a detailed plan, will do ;)

I'll comment if the patch is out (going through my mail one at a time) but I just want to chime in on C contexts: I'd rather have this done only once and from crt0.S. If this means crt0.S must do the call to lowlevel_init, then so be it IMO.

This might be workable, yeah. What I've decided after doing some of the work is that the rest of arm has cpu_init_crit and some callouts and I'm thinking that rather than make armv7 much different we should re-adjust things to fit back into the regular mold which I think is possible.

Quoted 2 lines

Simon, do you think we could move arch_cpu_init up in the call sequence in common/board_f.c ?

On x86 we set up early PCI then, since without that we cannot init the CPU.

I suppose we could split it into two calls, but why do you want to move it earlier?

Quoted 149 lines · Simon Glass

Hi Tom,

Quoted 136 lines · Tom Rini

Quoted 127 lines · Albert ARIBAUD

Hello Tom,

Quoted 124 lines · Tom Rini

Quoted 122 lines · Hans de Goede

Hi,

Quoted 105 lines · Tom Rini

Quoted 71 lines · Hans de Goede

Hi,

Quoted 48 lines · Tom Rini

Quoted 41 lines · Hans de Goede

Hi,

Quoted 32 lines · Tom Rini

Quoted 17 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 7 lines

arch/arm/cpu/armv7/start.S +1 -1

@@ -64,7 +64,7 @@ reset:
 
   /* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-   bl      cpu_init_cp15
+   bl      soc_init
   bl      cpu_init_crit
 #endif

I like the direction here. And I want to make sure I get the sunxi direction right here too (as I agree with the need / desire for boot0 + U-Boot to be a valid combination). I think we can take this a step farther. cpu_init_crit (on armv7) is basically a call to s_init().

Quoted 5 lines

For am33xx (and I bet but need to do and test omap3+) we can, with Simon's patch to let us move stack to DDR a tiny bit later, in the SPL case make s_init empty, which just leaves us with (with your patch) soc_init. Is there some way we can put all of this together in a function?

Quoted 5 lines

You mean essentially call s_init here and have s_init call cpu_init_cp15 I guess we could do that, but it would require auditing all existing armv7 users of s_init. This may require me to rethink how / when I do timer & gpio init etc. for u-boot.bin on sunxi, but that should not be a (big) problem.

Quoted 4 lines

Basically. From my first pass audit of s_init, it's either empty (Kona), sunxi, or omap/etc so I get to deal with it. And the default soc_init would just be the call to cpu_init_cp15 as you have it and we drop the lowlevel_init hurdles.

Ok, so what you're suggesting is a patch which:

1) Changes:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl cpu_init_cp15
bl cpu_init_crit
#endif

Into:

#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl lowlevel_init
#endif

Quoted 6 lines

Which will setup the stack and then call the s_init C function

2) Adds a weak default s_init which calls cpu_init_cp15

3) Patch all existing s_init functions to call cpu_init_cp15 before doing anything else.

Pretty close. Simon's SPL DM series and related clean-ups got me thinking that yes, seemingly too much got shoved into "s_init" that really could have been done using an existing hook done slightly later.

Quoted 8 lines

And then in follow up patches we can:

4) Drop cpu_init_crit

5) Cleanup some s_init functions (this will be left to the individual SoC maintainers)

I think that is a good idea, Albert what do you think about this ?

So I'd like to see 5 done "soon" afterwards as it's me (omap*) and sunxi. I think we can simplfy the call sequence too, to roughly:
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
... Set up stack for C, it's just a few instrs
bl lowlevel_init
#endif
bl _main

__weak asm
lowlevel_init:
bl cpu_init_cp15
return to caller

Quoted 9 lines

And comment that anything called via lowlevel_init must be C-callable. I hope that once #5 is done no one actually has a lowlevel_init that's done in C but we've kept the door open should it be needed down the road (as I _think_ we can shuffle both the omap* and sunxi stuff to do their inits as needed in both SPL and full U-Boot from an early hook in board_init_r, top of my head is board_init calls some_other_func() in full U-Boot to ensure GPIOs, etc, on sunxi and spl_board_init() calls same func in SPL, and we can consolidate again further down the road as we get SPL and full U-Boot more in sync on the call chain).

Sounds good to me, and I'm fine with working the sunxi side of things.

Since you seem to have this all in your head can you do a patch for this replacing my patchset ?

Quoted 6 lines

Quoted 1 line

I suppose that's what happens when you have a detailed plan, will do ;)

I'll comment if the patch is out (going through my mail one at a time) but I just want to chime in on C contexts: I'd rather have this done only once and from crt0.S. If this means crt0.S must do the call to lowlevel_init, then so be it IMO.

Quoted 7 lines

This might be workable, yeah. What I've decided after doing some of the work is that the rest of arm has cpu_init_crit and some callouts and I'm thinking that rather than make armv7 much different we should re-adjust things to fit back into the regular mold which I think is possible.

Simon, do you think we could move arch_cpu_init up in the call sequence in common/board_f.c ?

On x86 we set up early PCI then, since without that we cannot init the CPU.

Quoted 2 lines

I suppose we could split it into two calls, but why do you want to move it earlier?

Wait, since we need it early, why would moving it up earlier in board_init_r() be a problem for x86? And (and this is being split into different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

Signature · 1 line

Tom

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation

right. We have a C environment of sorts, with a stack but without writable data and without BSS; only GD is writable. That's because the current *running* address may be in Flash, hence the "_f". Code called from board_init_f() may set up some more PLLs/clocks, devices, peripherals, etc. as long as it's needed for relocation (e.g. querying a display's characteristics in order to compute how much memory it will reserve from top).

"Erring early": after relocation. That's board_init_r. We have a full C environment and we're running entirely from RAM ("_r"). There, U-Boot does whatever initializations are still needed to get the payload running.

The actual setting up of environments between stages is supposed to happen in some architecture code -- for ARM it would all be in arch/arm/lib/crt0.S.

(if more official names were needed -- and my point sort of *is* that they /are/ needed, to replace all these "early something" names we've been using so far -- I'd suggest "reset", "init", "layout" and "run" respectively.)

So... for me, Tom, your "very early but written in C" maps to my "effing early" / "layout"; something that has to run first in that stage should just be first in init_sequence_f[]. OTOH, it /cannot/ be something needed to reset or initialize the target.

Now, /some/ SoCs might be able to set up a C environment of sorts in place between the "reset" and "init" phases - SRAM, locked cache... This could be invoked before calling the "init" stage. Generic init code would not assume any C env, but SoC init code would be able to use it.

Hi Albert,

Quoted 20 lines · Albert ARIBAUD

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

Quoted 3 lines

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

This is the current lowlelvel_init() I think. I don't believe it should set up DDR.

Quoted 1 line

Quoted 2 lines

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation

At present board_init_f() ses up DDR and I'd prefer we keep that, e.g. with console output before DDR is ready.

Hi Albert,

Quoted 20 lines · Albert ARIBAUD

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

Quoted 3 lines

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

This is the current lowlelvel_init() I think. I don't believe it should set up DDR.

Quoted 2 lines

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation

At present board_init_f() ses up DDR and I'd prefer we keep that, e.g. with console output before DDR is ready.

Quoted 13 lines

right. We have a C environment of sorts, with a stack but without writable data and without BSS; only GD is writable. That's because the current *running* address may be in Flash, hence the "_f". Code called from board_init_f() may set up some more PLLs/clocks, devices, peripherals, etc. as long as it's needed for relocation (e.g. querying a display's characteristics in order to compute how much memory it will reserve from top).

"Erring early": after relocation. That's board_init_r. We have a full C environment and we're running entirely from RAM ("_r"). There, U-Boot does whatever initializations are still needed to get the payload running.

Quoted 8 lines

The actual setting up of environments between stages is supposed to happen in some architecture code -- for ARM it would all be in arch/arm/lib/crt0.S.

(if more official names were needed -- and my point sort of *is* that they /are/ needed, to replace all these "early something" names we've been using so far -- I'd suggest "reset", "init", "layout" and "run" respectively.)

For 'layout' could we have a word that starts with 'f'? Flash? Frame? Is it better to use relocated rather than run? Still, run is shorter.

Quoted 11 lines

So... for me, Tom, your "very early but written in C" maps to my "effing early" / "layout"; something that has to run first in that stage should just be first in init_sequence_f[]. OTOH, it /cannot/ be something needed to reset or initialize the target.

Now, /some/ SoCs might be able to set up a C environment of sorts in place between the "reset" and "init" phases - SRAM, locked cache... This could be invoked before calling the "init" stage. Generic init code would not assume any C env, but SoC init code would be able to use it.

Some sort of RAM has to exist before board_init_f() is called. Your suggestion was to not allow a stack in the 'init' phase I think. So perhaps we should lock these down more tightly:

reset - no stack, no RAM ( (except SPL can presumably access data section)

init - no stack, no RAM (except SPL can presumably access data section)

layout - stack, RAM for global_data, but no DRAM

run - stack, DRAM, global data in DRAM

Regards,
Simon

Hello Simon,

Quoted 23 lines · Simon Glass

Hi Albert,

Quoted 20 lines · Albert ARIBAUD

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

Quoted 6 lines

Quoted 3 lines

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

This is the current lowlelvel_init() I think. I don't believe it should set up DDR.

See below (*)

Quoted 5 lines

Quoted 2 lines

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation

At present board_init_f() ses up DDR and I'd prefer we keep that, e.g. with console output before DDR is ready.

Understood. But even console is not needed by board_init_f -- actually, board_init_f *does* initialize the console. So the only thing that is needed after resetting the core to a known state and before entering board_init_f() is... To make sure board_init_f() can run, which is not
much: for instance, locking cache to be used as SRAM for stack and GD -- basicaly what armv7 does.

Quoted 38 lines

Quoted 14 lines

right. We have a C environment of sorts, with a stack but without writable data and without BSS; only GD is writable. That's because the current *running* address may be in Flash, hence the "_f". Code called from board_init_f() may set up some more PLLs/clocks, devices, peripherals, etc. as long as it's needed for relocation (e.g. querying a display's characteristics in order to compute how much memory it will reserve from top).

"Erring early": after relocation. That's board_init_r. We have a full C environment and we're running entirely from RAM ("_r"). There, U-Boot does whatever initializations are still needed to get the payload running.

Quoted 8 lines

The actual setting up of environments between stages is supposed to happen in some architecture code -- for ARM it would all be in arch/arm/lib/crt0.S.

(if more official names were needed -- and my point sort of *is* that they /are/ needed, to replace all these "early something" names we've been using so far -- I'd suggest "reset", "init", "layout" and "run" respectively.)

For 'layout' could we have a word that starts with 'f'? Flash? Frame? Is it better to use relocated rather than run? Still, run is shorter.

Quoted 11 lines

So... for me, Tom, your "very early but written in C" maps to my "effing early" / "layout"; something that has to run first in that stage should just be first in init_sequence_f[]. OTOH, it /cannot/ be something needed to reset or initialize the target.

Now, /some/ SoCs might be able to set up a C environment of sorts in place between the "reset" and "init" phases - SRAM, locked cache... This could be invoked before calling the "init" stage. Generic init code would not assume any C env, but SoC init code would be able to use it.

Quoted 3 lines

Some sort of RAM has to exist before board_init_f() is called. Your suggestion was to not allow a stack in the 'init' phase I think. So perhaps we should lock these down more tightly:

[I think we should separate the hardware items (do we have DDR? do we have PLLs? etc) from the program concepts (do we have a stack? Do we have data write access? Do we have BSS?. My comments below reflect that]

Quoted 1 line

reset - no stack, no RAM ( (except SPL can presumably access data section)

More precisely: from a hardware viewpoint there would be no DDR, but there could be SRAM, and from a program viewpoint, no stack and no writable data: text, rodata and initialized data sections could be read from, but should never be written to.

Plus, this stage should be strongly restricted to the role of saving what is vital and urgent to save. If there is SRAM, the reset stage could use it. If there is no SRAM but cache, the reset code could lock the cache and use it as SRAM. But in most cases, there should be only a few registers to save, and these could be saved inside the core (e.g. in the FIQ banked registers), so the reset stage should really only be a few assembly language instructions long.

Quoted 1 line

init - no stack, no RAM (except SPL can presumably access data section)

Same as reset. The difference is that we now have full use of the core registers without fear of trashing something important.

This stage should limit itself to resetting the target to a known valid state.

Quoted 1 line

layout - stack, RAM for global_data, but no DRAM

Not exactly. From a hardware standpoint, still no DDR, just like previous stages; from a program perspective, we gain a stack and that's
all: we wtill cannot write to data and BSS, we still can only write to GD.

Quoted 1 line

run - stack, DRAM, global data in DRAM

AKA "Full C environment" (whether in DRAM or in generous SRAM does not matter).

"layout" does not entirely cover what board_init_f() does, though, and I don't like "init" when what it does is actually resetting the board to a known state -- yes, I know we often use "reset" for "restart". How about this?

        "start"
        "init"
        "forerun"
        "run"

You've got the "f" that you wanted :) and the "run" in the third and last stage are nice reminders that we do have a run-time C environment there, except the third stage is not quite a running environment yet.

Hi Albert,

Quoted 6 lines

Quoted 1 line

reset - no stack, no RAM ( (except SPL can presumably access data section)

More precisely: from a hardware viewpoint there would be no DDR, but there could be SRAM, and from a program viewpoint, no stack and no writable data: text, rodata and initialized data sections could be read from, but should never be written to.

I can see in some rare cases we would want to store data here (as you say in the next paragraph) and in that case, initialised data is a good option - we cannot use BSS and it is ugly to write to memory directly. My sunxi patches store lr and sp, for example.

Quoted 10 lines

"layout" does not entirely cover what board_init_f() does, though, and I don't like "init" when what it does is actually resetting the board to a known state -- yes, I know we often use "reset" for "restart". How about this?

        "start"
        "init"
        "forerun"
        "run"

Quoted 3 lines

You've got the "f" that you wanted :) and the "run" in the third and last stage are nice reminders that we do have a run-time C environment there, except the third stage is not quite a running environment yet.

All of the above sounds good to me. Yes forerun has an f. I think it is better than the other things I can think of (find, furnish, formulate, fabricate, former)

Regards,
Simon

Hello Simon,

Quoted 9 lines · Simon Glass

Hi Albert,

Quoted 6 lines

Quoted 1 line

reset - no stack, no RAM ( (except SPL can presumably access data section)

More precisely: from a hardware viewpoint there would be no DDR, but there could be SRAM, and from a program viewpoint, no stack and no writable data: text, rodata and initialized data sections could be read from, but should never be written to.

Quoted 4 lines

I can see in some rare cases we would want to store data here (as you say in the next paragraph) and in that case, initialised data is a good option - we cannot use BSS and it is ugly to write to memory directly. My sunxi patches store lr and sp, for example.

Not sure I'm getting your "initialized data is a good option", as initialized data, just like BSS, cannot be written at reset stage.

Quoted 15 lines

Quoted 14 lines

"layout" does not entirely cover what board_init_f() does, though, and I don't like "init" when what it does is actually resetting the board to a known state -- yes, I know we often use "reset" for "restart". How about this?

        "start"
        "init"
        "forerun"
        "run"

You've got the "f" that you wanted :) and the "run" in the third and last stage are nice reminders that we do have a run-time C environment there, except the third stage is not quite a running environment yet.

Quoted 3 lines

All of the above sounds good to me. Yes forerun has an f. I think it is better than the other things I can think of (find, furnish, formulate, fabricate, former)

"Anything else we does beginnin' with F, boys?" -- Otis McCoy, head of the "feudin', fightin', feedin', funnin', flatin' McCoys" gang, /in/ the Judge Dredd (the comics) story "Alabama Blimps".

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation right. We have a C environment of sorts, with a stack but without writable data and without BSS; only GD is writable. That's because the current *running* address may be in Flash, hence the "_f". Code called from board_init_f() may set up some more PLLs/clocks, devices, peripherals, etc. as long as it's needed for relocation (e.g. querying a display's characteristics in order to compute how much memory it will reserve from top).

"Erring early": after relocation. That's board_init_r. We have a full C environment and we're running entirely from RAM ("_r"). There, U-Boot does whatever initializations are still needed to get the payload running.

The actual setting up of environments between stages is supposed to happen in some architecture code -- for ARM it would all be in arch/arm/lib/crt0.S.

(if more official names were needed -- and my point sort of *is* that they /are/ needed, to replace all these "early something" names we've been using so far -- I'd suggest "reset", "init", "layout" and "run" respectively.)

So... for me, Tom, your "very early but written in C" maps to my "effing early" / "layout"; something that has to run first in that stage should just be first in init_sequence_f[]. OTOH, it /cannot/ be something needed to reset or initialize the target.

Now, /some/ SoCs might be able to set up a C environment of sorts in place between the "reset" and "init" phases - SRAM, locked cache... This could be invoked before calling the "init" stage. Generic init code would not assume any C env, but SoC init code would be able to use it.

Comments?

Quoted 59 lines · Albert ARIBAUD

Hello Tom,

Quoted 4 lines · Tom Rini

And (and this is being split into
different email threads, sigh), it would be good, possibly, if we have something that means "very early init things, but we can be written in C".

"Very early" -- and "early" too, BTW -- is way too vague for me to be sure we're talking about the same thing, so let's find out what various earlinesses mean to us. My own view:

"Starting early": the 'start', or 'reset', entry point, can't get earlier than that. This is where U-Boot resets the target to a known state (cache disable and invalidate, for instance). For some SoCs, at this point core registers have to be saved somewhere because they contain informative values that we want to keep, so we need to be able to hook this stage. There is no C environment.

"Flipping early": after the entry point but before the DDR is usable. That's where PLLs/clocks are set up minimaly to get going and have access to the needed resources including DDR. Still no C environment.

"Effing early": after the DDR was made usable but before relocation. That's when board_init_f() starts. It's there to get the relocation right. We have a C environment of sorts, with a stack but without writable data and without BSS; only GD is writable. That's because the current *running* address may be in Flash, hence the "_f". Code called from board_init_f() may set up some more PLLs/clocks, devices, peripherals, etc. as long as it's needed for relocation (e.g. querying a display's characteristics in order to compute how much memory it will reserve from top).

"Erring early": after relocation. That's board_init_r. We have a full C environment and we're running entirely from RAM ("_r"). There, U-Boot does whatever initializations are still needed to get the payload running.

The actual setting up of environments between stages is supposed to happen in some architecture code -- for ARM it would all be in arch/arm/lib/crt0.S.

(if more official names were needed -- and my point sort of *is* that they /are/ needed, to replace all these "early something" names we've been using so far -- I'd suggest "reset", "init", "layout" and "run" respectively.)

So... for me, Tom, your "very early but written in C" maps to my "effing early" / "layout"; something that has to run first in that stage should just be first in init_sequence_f[]. OTOH, it /cannot/ be something needed to reset or initialize the target.

Now, /some/ SoCs might be able to set up a C environment of sorts in place between the "reset" and "init" phases - SRAM, locked cache... This could be invoked before calling the "init" stage. Generic init code would not assume any C env, but SoC init code would be able to use it.

Quoted 1 line

Comments?

I think this sounds about right. I need to post what I have that is a good clean up / re-org of some of the code now on a few platforms now and we'll see how Hans' fixup for sunxi fits in again?

Quoted 18 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644
@@ -64,7 +64,7 @@ reset:
 

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 6 lines

arch/arm/cpu/armv7/start.S +1 -1

 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
 	bl	cpu_init_crit
 #endif

I like the direction here.

Hello Tom,

Quoted 20 lines · Tom Rini

Quoted 18 lines · Hans de Goede

On some SoCs / ARMv7 CPU cores we need to do some setup before enabling the icache, etc. Add a soc_init hook with a weak default which just calls cpu_init_cp15.

This way different implementations can be provided to do some extra work before or after cpu_init_cp15, or completely replacing cpu_init_cp15.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
  1 file changed, 17 insertions(+), 1 deletion(-)

arch/arm/cpu/armv7/start.S +0 -0

index fdc05b9..9882b20 100644
@@ -64,7 +64,7 @@ reset:
 

Quoted 8 lines

arch/arm/cpu/armv7/start.S reset: · line 69

Quoted 6 lines

arch/arm/cpu/armv7/start.S +1 -1

  	/* the mask ROM code should have PLL and others stable */
  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
-	bl	cpu_init_cp15
+	bl	soc_init
  	bl	cpu_init_crit
  #endif

I like the direction here.

I don't like all of it, specifically the double call, to cpu_init_cp15 then soc_init, for two reasons:

0) soc_init might touch cp15, but it's hidden in the name whereas it is shown in cpu_init_cp15. Either both names should explicitly say they are touching cp15, or both should not.

1) we cannot tell if cpu_init_cp15 should always happen before soc_init. Having a single call to soc_init, with the understanding that it will handle the cpu_init_cp15 call itself, makes it possible for soc_init yo decide whether it wants to call cpu_init_cp15 first, last or at all.