[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[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 <hdegoede@redhat.com>
---
 arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index fdc05b9..9882b20 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -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
 
@@ -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
+ *
+ *************************************************************************/
+ENTRY(soc_init)
+	mov	r9, lr
+	bl	cpu_init_cp15
+	mov	pc, r9			@ back to my caller
+ENDPROC(soc_init)
+	.weak	soc_init
+
+/*************************************************************************
+ *
  * cpu_init_cp15
  *
  * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless
-- 
2.1.0

[U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function

[Hans de Goede]

Helper function for SoCs which use Cortex A7 cpu cores, this should be called
by the SoC's soc_init function to properly setup the cpu core before calling
cpu_init_cp15.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/cpu/armv7/start.S | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 9882b20..f53e5ef 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -182,6 +182,23 @@ ENTRY(cpu_init_cp15)
 	mov	pc, lr			@ back to my caller
 ENDPROC(cpu_init_cp15)
 
+/*************************************************************************
+ *
+ * cpu_init_cortex_a7
+ *
+ * Set the ACTLR.SMP bit. "Cortex-A7 MPCore Technical Reference Manual":
+ * "You must ensure this bit is set to 1 before the caches and MMU are
+ * enabled, or any cache and TLB maintenance operations are performed."
+ * So for cortex A7 this must be called before calling cpu_init_cp15.
+ *
+ *************************************************************************/
+ENTRY(cpu_init_cortex_a7)
+	mrc	p15, 0, r0, c1, c0, 1
+	orr	r0, r0, #(1<<6)
+	mcr	p15, 0, r0, c1, c0, 1
+	mov	pc, lr			@ back to my caller
+ENDPROC(cpu_init_cortex_a7)
+
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 /*************************************************************************
  *
-- 
2.1.0

[U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit

[Hans de Goede]

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 <hdegoede@redhat.com>
---
 arch/arm/cpu/armv7/sunxi/Makefile        |  1 +
 arch/arm/cpu/armv7/sunxi/board.c         |  8 --------
 arch/arm/cpu/armv7/sunxi/lowlevel_init.S | 19 +++++++++++++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/lowlevel_init.S

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
index 1c4b763..27264f5 100644
--- a/arch/arm/cpu/armv7/sunxi/Makefile
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -7,6 +7,7 @@
 #
 # SPDX-License-Identifier:	GPL-2.0+
 #
+obj-y	+= lowlevel_init.o
 obj-y	+= timer.o
 obj-y	+= board.o
 obj-y	+= clock.o
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c
index 6e28bcd..3eed8a9 100644
--- a/arch/arm/cpu/armv7/sunxi/board.c
+++ b/arch/arm/cpu/armv7/sunxi/board.c
@@ -72,14 +72,6 @@ void s_init(void)
 	 * access gets messed up (seems cache related) */
 	setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800);
 #endif
-#if !defined CONFIG_SPL_BUILD && (defined CONFIG_MACH_SUN7I || \
-		defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I)
-	/* Enable SMP mode for CPU0, by setting bit 6 of Auxiliary Ctl reg */
-	asm volatile(
-		"mrc p15, 0, r0, c1, c0, 1\n"
-		"orr r0, r0, #1 << 6\n"
-		"mcr p15, 0, r0, c1, c0, 1\n");
-#endif
 
 	clock_init();
 	timer_init();
diff --git a/arch/arm/cpu/armv7/sunxi/lowlevel_init.S b/arch/arm/cpu/armv7/sunxi/lowlevel_init.S
new file mode 100644
index 0000000..f7182e6
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/lowlevel_init.S
@@ -0,0 +1,19 @@
+/*
+ * (C) Copyright 2015 Hans de Goede <hdegoede@redhat.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <config.h>
+#include <version.h>
+#include <linux/linkage.h>
+
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN7I || \
+    defined CONFIG_MACH_SUN8I
+ENTRY(soc_init)
+	mov	r9, lr
+	bl	cpu_init_cortex_a7
+	bl	cpu_init_cp15
+	mov	pc, r9			@ back to my caller
+ENDPROC(soc_init)
+#endif
-- 
2.1.0

[U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit

[Ian Campbell]

On Wed, 2015-01-21 at 21:03 +0100, Hans de Goede wrote:
> 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 <hdegoede@redhat.com>

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

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Bill Pringlemeir]

On 21 Jan 2015, hdegoede at redhat.com wrote:

> 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 <hdegoede@redhat.com>
> ---
> arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index fdc05b9..9882b20 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
>>> -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
>
>>> -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
>+ *
>+ *************************************************************************/
>+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).

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

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 21-01-15 22:59, Bill Pringlemeir wrote:
> On 21 Jan 2015, hdegoede at redhat.com wrote:
>
>> 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 <hdegoede@redhat.com>
>> ---
>> arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index fdc05b9..9882b20 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>>>> -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
>>
>>>> -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
>> + *
>> + *************************************************************************/
>> +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)

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

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

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

A valid point, once I've some more feedback I'll do a v2 adding a comment to
the top of cpu_init_cp15 that it must not touch r9 as it caller may have that
in use.


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

Regards,

Hans

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Bill Pringlemeir]

>> On 21 Jan 2015, hdegoede at redhat.com wrote:
>>
>>> 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 <hdegoede@redhat.com>
>>> ---
>>> arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>> index fdc05b9..9882b20 100644
>>> --- a/arch/arm/cpu/armv7/start.S
>>> +++ b/arch/arm/cpu/armv7/start.S
>>>>> -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
>>>
>>>>> -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 + * +
>>> *************************************************************************/
>>> +ENTRY(soc_init)
>>> + mov r9, lr 
>>> + bl cpu_init_cp15 
>>> + mov pc, r9 @ back to my caller 
>>> +ENDPROC(soc_init)
>>> + .weak soc_init

> On 21-01-15 22:59, Bill Pringlemeir wrote:
>>
>> You could just use a 'tail call' and make this,
>>
>> +ENTRY(soc_init)
>> +	b	cpu_init_cp15
>> +ENDPROC(soc_init)

On 22 Jan 2015, hdegoede at redhat.com wrote:

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

Yes, it is a good example if you are looking to over-ride the 'soc_init'
and I expect that it was hard to figure out that you needed to save the
lr; I can see why you want to warn others.  Two instructions isn't bad.
However, I looked a the code from a different perspective.  I just want
to see what is going on in a normal boot.  I would look at this code and
think what the heck is this about.  Especially as 'r9' is used for other
purposes.

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

Hmm.  I understood better after looking at the sunxi implementation.
The weak 'soc_init' seemed like I didn't understand something.  The
saving of 'lr' in 'r9' is only needed if you have extra code and is
nothing to do with global data or something.  So, it is not just less
code but I think it is more clear what the default case is doing.

I agree, it is definitely an opinion.  Either way works.

> Albert what do you want to do here?

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:

> 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 <hdegoede@redhat.com>
> ---
>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index fdc05b9..9882b20 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150122/4b6fede6/attachment.pgp>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 22-01-15 17:20, Tom Rini wrote:
> On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
>
>> 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 <hdegoede@redhat.com>
>> ---
>>   arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index fdc05b9..9882b20 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>> @@ -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?

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.

Regards,

Hans

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 22-01-15 17:20, Tom Rini wrote:
> >On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> >
> >>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 <hdegoede@redhat.com>
> >>---
> >>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> >>  1 file changed, 17 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >>index fdc05b9..9882b20 100644
> >>--- a/arch/arm/cpu/armv7/start.S
> >>+++ b/arch/arm/cpu/armv7/start.S
> >>@@ -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?
> 
> 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150122/d937bc42/attachment.pgp>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 22-01-15 22:03, Tom Rini wrote:
> On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 22-01-15 17:20, Tom Rini wrote:
>>> On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
>>>
>>>> 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 <hdegoede@redhat.com>
>>>> ---
>>>>   arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>>> index fdc05b9..9882b20 100644
>>>> --- a/arch/arm/cpu/armv7/start.S
>>>> +++ b/arch/arm/cpu/armv7/start.S
>>>> @@ -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?
>>
>> 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.

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 ?

Regards,

Hans

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 22-01-15 22:03, Tom Rini wrote:
> >On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 22-01-15 17:20, Tom Rini wrote:
> >>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> >>>
> >>>>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 <hdegoede@redhat.com>
> >>>>---
> >>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> >>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> >>>>
> >>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >>>>index fdc05b9..9882b20 100644
> >>>>--- a/arch/arm/cpu/armv7/start.S
> >>>>+++ b/arch/arm/cpu/armv7/start.S
> >>>>@@ -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?
> >>
> >>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.
> 
> 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.

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.

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

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150126/388351ac/attachment.pgp>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 26-01-15 16:18, Tom Rini wrote:
> On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 22-01-15 22:03, Tom Rini wrote:
>>> On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 22-01-15 17:20, Tom Rini wrote:
>>>>> On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
>>>>>
>>>>>> 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 <hdegoede@redhat.com>
>>>>>> ---
>>>>>>   arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>>>>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>>>>> index fdc05b9..9882b20 100644
>>>>>> --- a/arch/arm/cpu/armv7/start.S
>>>>>> +++ b/arch/arm/cpu/armv7/start.S
>>>>>> @@ -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?
>>>>
>>>> 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.
>>
>> 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.
>
> 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.
>
>> 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).

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 ?

Regards,

Hans

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 26-01-15 16:18, Tom Rini wrote:
> >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 22-01-15 22:03, Tom Rini wrote:
> >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> >>>>Hi,
> >>>>
> >>>>On 22-01-15 17:20, Tom Rini wrote:
> >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> >>>>>
> >>>>>>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 <hdegoede@redhat.com>
> >>>>>>---
> >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> >>>>>>index fdc05b9..9882b20 100644
> >>>>>>--- a/arch/arm/cpu/armv7/start.S
> >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> >>>>>>@@ -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?
> >>>>
> >>>>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.
> >>
> >>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.
> >
> >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.
> >
> >>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).
> 
> 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 ;)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150127/f3e2782c/attachment.pgp>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Tom,

On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini@ti.com> wrote:
> On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 26-01-15 16:18, Tom Rini wrote:
> > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > >>Hi,
> > >>
> > >>On 22-01-15 22:03, Tom Rini wrote:
> > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > >>>>Hi,
> > >>>>
> > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > >>>>>
> > >>>>>>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 <hdegoede@redhat.com>
> > >>>>>>---
> > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > >>>>>>index fdc05b9..9882b20 100644
> > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > >>>>>>@@ -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?
> > >>>>
> > >>>>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.
> > >>
> > >>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.
> > >
> > >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.
> > >
> > >>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).
> > 
> > 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 ;)

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.

> -- 
> Tom



Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Sat, Jan 31, 2015 at 10:25:50PM +0100, Albert ARIBAUD wrote:
> Hello Tom,
> 
> On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini@ti.com> wrote:
> > On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 26-01-15 16:18, Tom Rini wrote:
> > > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > > >>Hi,
> > > >>
> > > >>On 22-01-15 22:03, Tom Rini wrote:
> > > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > > >>>>Hi,
> > > >>>>
> > > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > > >>>>>
> > > >>>>>>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 <hdegoede@redhat.com>
> > > >>>>>>---
> > > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > > >>>>>>
> > > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > > >>>>>>index fdc05b9..9882b20 100644
> > > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > > >>>>>>@@ -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?
> > > >>>>
> > > >>>>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.
> > > >>
> > > >>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.
> > > >
> > > >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.
> > > >
> > > >>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).
> > > 
> > > 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 ;)
> 
> 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 ?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150131/c412ed23/attachment.sig>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

Hi Tom,

On 31 January 2015 at 14:49, Tom Rini <trini@ti.com> wrote:
>
> On Sat, Jan 31, 2015 at 10:25:50PM +0100, Albert ARIBAUD wrote:
> > Hello Tom,
> >
> > On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini@ti.com> wrote:
> > > On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > > > Hi,
> > > >
> > > > On 26-01-15 16:18, Tom Rini wrote:
> > > > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > > > >>Hi,
> > > > >>
> > > > >>On 22-01-15 22:03, Tom Rini wrote:
> > > > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > > > >>>>Hi,
> > > > >>>>
> > > > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > > > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > > > >>>>>
> > > > >>>>>>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 <hdegoede@redhat.com>
> > > > >>>>>>---
> > > > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > > > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > > > >>>>>>
> > > > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > > > >>>>>>index fdc05b9..9882b20 100644
> > > > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > > > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > > > >>>>>>@@ -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?
> > > > >>>>
> > > > >>>>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.
> > > > >>
> > > > >>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.
> > > > >
> > > > >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.
> > > > >
> > > > >>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).
> > > >
> > > > 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 ;)
> >
> > 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 ?

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?

Regards,
Simon

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Sat, Jan 31, 2015 at 03:14:35PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On 31 January 2015 at 14:49, Tom Rini <trini@ti.com> wrote:
> >
> > On Sat, Jan 31, 2015 at 10:25:50PM +0100, Albert ARIBAUD wrote:
> > > Hello Tom,
> > >
> > > On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini@ti.com> wrote:
> > > > On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > > > > Hi,
> > > > >
> > > > > On 26-01-15 16:18, Tom Rini wrote:
> > > > > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > > > > >>Hi,
> > > > > >>
> > > > > >>On 22-01-15 22:03, Tom Rini wrote:
> > > > > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > > > > >>>>Hi,
> > > > > >>>>
> > > > > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > > > > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > > > > >>>>>
> > > > > >>>>>>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 <hdegoede@redhat.com>
> > > > > >>>>>>---
> > > > > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > > > > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > > > > >>>>>>
> > > > > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > > > > >>>>>>index fdc05b9..9882b20 100644
> > > > > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > > > > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > > > > >>>>>>@@ -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?
> > > > > >>>>
> > > > > >>>>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.
> > > > > >>
> > > > > >>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.
> > > > > >
> > > > > >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.
> > > > > >
> > > > > >>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).
> > > > >
> > > > > 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 ;)
> > >
> > > 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 ?
> 
> 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?

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

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150202/2c405091/attachment.sig>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

+Bin

Hi Tom,

On 2 February 2015 at 11:56, Tom Rini <trini@ti.com> wrote:
>
> On Sat, Jan 31, 2015 at 03:14:35PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On 31 January 2015 at 14:49, Tom Rini <trini@ti.com> wrote:
> > >
> > > On Sat, Jan 31, 2015 at 10:25:50PM +0100, Albert ARIBAUD wrote:
> > > > Hello Tom,
> > > >
> > > > On Tue, 27 Jan 2015 09:23:47 -0500, Tom Rini <trini@ti.com> wrote:
> > > > > On Mon, Jan 26, 2015 at 08:32:41PM +0100, Hans de Goede wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 26-01-15 16:18, Tom Rini wrote:
> > > > > > >On Fri, Jan 23, 2015 at 09:54:12AM +0100, Hans de Goede wrote:
> > > > > > >>Hi,
> > > > > > >>
> > > > > > >>On 22-01-15 22:03, Tom Rini wrote:
> > > > > > >>>On Thu, Jan 22, 2015 at 08:10:06PM +0100, Hans de Goede wrote:
> > > > > > >>>>Hi,
> > > > > > >>>>
> > > > > > >>>>On 22-01-15 17:20, Tom Rini wrote:
> > > > > > >>>>>On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> > > > > > >>>>>
> > > > > > >>>>>>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 <hdegoede@redhat.com>
> > > > > > >>>>>>---
> > > > > > >>>>>>  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> > > > > > >>>>>>  1 file changed, 17 insertions(+), 1 deletion(-)
> > > > > > >>>>>>
> > > > > > >>>>>>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > > > > > >>>>>>index fdc05b9..9882b20 100644
> > > > > > >>>>>>--- a/arch/arm/cpu/armv7/start.S
> > > > > > >>>>>>+++ b/arch/arm/cpu/armv7/start.S
> > > > > > >>>>>>@@ -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?
> > > > > > >>>>
> > > > > > >>>>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.
> > > > > > >>
> > > > > > >>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.
> > > > > > >
> > > > > > >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.
> > > > > > >
> > > > > > >>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).
> > > > > >
> > > > > > 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 ;)
> > > >
> > > > 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 ?
> >
> > 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?
>
> 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".

Do you mean board_init_f() or board_init_r()? I was talking about
board_init_f().

PCI gets inited twice on x86 (for some boards) - once before
relocation and one after.

Regards,
Simon

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Tom,

On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:

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

> -- 
> Tom

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

Hi Albert,

On 4 February 2015 at 01:48, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hello Tom,
>
> On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
>
>> 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.

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

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

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

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.

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

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 05-02-15 04:00, Simon Glass wrote:
> Hi Albert,
>
> On 4 February 2015 at 01:48, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
>> Hello Tom,
>>
>> On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
>>
>>> 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.
>
> This is the current lowlelvel_init() I think. I don't believe it
> should set up DDR.
>
>>
>> "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.

Ack, DRAM setup is iffy, and we definitely want to have (early) console
output working at that point. I do not even want to think about having
to debug DRAM setup without console output.

Regards,

Hans

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Simon,

On Wed, 4 Feb 2015 20:00:48 -0700, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,
> 
> On 4 February 2015 at 01:48, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> > Hello Tom,
> >
> > On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
> >
> >> 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.
> 
> This is the current lowlelvel_init() I think. I don't believe it
> should set up DDR.

See below (*)

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

> > 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.)
> 
> 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.
> 
> >
> > 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:

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

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

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

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

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

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

Hi Albert,

On 5 February 2015 at 08:14, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hello Simon,
>
> On Wed, 4 Feb 2015 20:00:48 -0700, Simon Glass <sjg@chromium.org> wrote:
>> Hi Albert,
>>
>> On 4 February 2015 at 01:48, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
>> > Hello Tom,
>> >
>> > On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
>> >
>> >> 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.
>>
>> This is the current lowlelvel_init() I think. I don't believe it
>> should set up DDR.
>
> See below (*)
>
>> > "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.

That's right, I didn't mean the console is needed by board_init_f(),
just that it is set up by board_init_f(), before init_dram().

>
>> > 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.)
>>
>> 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.
>>
>> >
>> > 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:
>
> [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]
>
>> 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.

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

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

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Simon,

On Thu, 5 Feb 2015 08:34:53 -0700, Simon Glass <sjg@chromium.org> wrote:
> Hi Albert,

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

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

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

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

Hi Albert,

On 5 February 2015 at 11:02, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hello Simon,
>
> On Thu, 5 Feb 2015 08:34:53 -0700, Simon Glass <sjg@chromium.org> wrote:
>> Hi Albert,
>
>> >> 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.
>
> Not sure I'm getting your "initialized data is a good option", as
> initialized data, just like BSS, cannot be written at reset stage.

I mean in SPL - in this case we are running from SRAM. But in the
general case I'm not sure where we could store anything. In any case
this can be board-specific.

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

You've got them all.

Regards,
Simon

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Tom Rini]

On Wed, Feb 04, 2015 at 09:48:32AM +0100, Albert ARIBAUD wrote:
> Hello Tom,
> 
> On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
> 
> > 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?

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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150210/81d15e74/attachment.sig>

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Simon Glass]

Hi Tom,

On 10 February 2015 at 15:07, Tom Rini <trini@ti.com> wrote:
> On Wed, Feb 04, 2015 at 09:48:32AM +0100, Albert ARIBAUD wrote:
>> Hello Tom,
>>
>> On Mon, 2 Feb 2015 13:56:57 -0500, Tom Rini <trini@ti.com> wrote:
>>
>> > 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?
>
> 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?

Related, I sent a v4 series which collects the various gdata/early init patches.

Regards,
Simon

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Albert ARIBAUD]

Hello Tom,

On Thu, 22 Jan 2015 11:20:58 -0500, Tom Rini <trini@ti.com> wrote:
> On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
> 
> > 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 <hdegoede@redhat.com>
> > ---
> >  arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> > index fdc05b9..9882b20 100644
> > --- a/arch/arm/cpu/armv7/start.S
> > +++ b/arch/arm/cpu/armv7/start.S
> > @@ -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.

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.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

Hi,

On 26-01-15 09:09, Albert ARIBAUD wrote:
> Hello Tom,
>
> On Thu, 22 Jan 2015 11:20:58 -0500, Tom Rini <trini@ti.com> wrote:
>> On Wed, Jan 21, 2015 at 09:03:25PM +0100, Hans de Goede wrote:
>>
>>> 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 <hdegoede@redhat.com>
>>> ---
>>>   arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++-
>>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>> index fdc05b9..9882b20 100644
>>> --- a/arch/arm/cpu/armv7/start.S
>>> +++ b/arch/arm/cpu/armv7/start.S
>>> @@ -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.
>
> 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.

There is no double call, the call to cpu_init_cp15 is being replaced with
a call to soc_init, which then can call cpu_init_cp15 at any time it likes.

Also see the discussion further in the thread about replacing soc_init
with a call to lowlevel_init which then call s_init which may then call
cpu_init_cp15 at a convenient time, the advantage of this approach is that
the soc / board specific code can be written in C rather then in asm.

Regards,

Hans