* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S @ 2015-01-21 20:03 Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function Hans de Goede ` (3 more replies) 0 siblings, 4 replies; 30+ messages in thread From: Hans de Goede @ 2015-01-21 20:03 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function 2015-01-21 20:03 [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Hans de Goede @ 2015-01-21 20:03 ` Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit Hans de Goede ` (2 subsequent siblings) 3 siblings, 0 replies; 30+ messages in thread From: Hans de Goede @ 2015-01-21 20:03 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit 2015-01-21 20:03 [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function Hans de Goede @ 2015-01-21 20:03 ` Hans de Goede 2015-02-08 5:42 ` Ian Campbell 2015-01-21 21:59 ` [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Bill Pringlemeir 2015-01-22 16:20 ` Tom Rini 3 siblings, 1 reply; 30+ messages in thread From: Hans de Goede @ 2015-01-21 20:03 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit 2015-01-21 20:03 ` [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit Hans de Goede @ 2015-02-08 5:42 ` Ian Campbell 0 siblings, 0 replies; 30+ messages in thread From: Ian Campbell @ 2015-02-08 5:42 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-21 20:03 [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit Hans de Goede @ 2015-01-21 21:59 ` Bill Pringlemeir 2015-01-22 13:29 ` Hans de Goede 2015-01-22 16:20 ` Tom Rini 3 siblings, 1 reply; 30+ messages in thread From: Bill Pringlemeir @ 2015-01-21 21:59 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-21 21:59 ` [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Bill Pringlemeir @ 2015-01-22 13:29 ` Hans de Goede 2015-01-22 15:48 ` Bill Pringlemeir 0 siblings, 1 reply; 30+ messages in thread From: Hans de Goede @ 2015-01-22 13:29 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-22 13:29 ` Hans de Goede @ 2015-01-22 15:48 ` Bill Pringlemeir 0 siblings, 0 replies; 30+ messages in thread From: Bill Pringlemeir @ 2015-01-22 15:48 UTC (permalink / raw) To: u-boot >> 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? ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-21 20:03 [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Hans de Goede ` (2 preceding siblings ...) 2015-01-21 21:59 ` [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Bill Pringlemeir @ 2015-01-22 16:20 ` Tom Rini 2015-01-22 19:10 ` Hans de Goede 2015-01-26 8:09 ` Albert ARIBAUD 3 siblings, 2 replies; 30+ messages in thread From: Tom Rini @ 2015-01-22 16:20 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-22 16:20 ` Tom Rini @ 2015-01-22 19:10 ` Hans de Goede 2015-01-22 21:03 ` Tom Rini 2015-01-26 8:09 ` Albert ARIBAUD 1 sibling, 1 reply; 30+ messages in thread From: Hans de Goede @ 2015-01-22 19:10 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-22 19:10 ` Hans de Goede @ 2015-01-22 21:03 ` Tom Rini 2015-01-23 8:54 ` Hans de Goede 0 siblings, 1 reply; 30+ messages in thread From: Tom Rini @ 2015-01-22 21:03 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-22 21:03 ` Tom Rini @ 2015-01-23 8:54 ` Hans de Goede 2015-01-26 15:18 ` Tom Rini 0 siblings, 1 reply; 30+ messages in thread From: Hans de Goede @ 2015-01-23 8:54 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-23 8:54 ` Hans de Goede @ 2015-01-26 15:18 ` Tom Rini 2015-01-26 19:32 ` Hans de Goede 0 siblings, 1 reply; 30+ messages in thread From: Tom Rini @ 2015-01-26 15:18 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-26 15:18 ` Tom Rini @ 2015-01-26 19:32 ` Hans de Goede 2015-01-27 14:23 ` Tom Rini 0 siblings, 1 reply; 30+ messages in thread From: Hans de Goede @ 2015-01-26 19:32 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-26 19:32 ` Hans de Goede @ 2015-01-27 14:23 ` Tom Rini 2015-01-31 21:25 ` Albert ARIBAUD 0 siblings, 1 reply; 30+ messages in thread From: Tom Rini @ 2015-01-27 14:23 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-27 14:23 ` Tom Rini @ 2015-01-31 21:25 ` Albert ARIBAUD 2015-01-31 21:49 ` Tom Rini 0 siblings, 1 reply; 30+ messages in thread From: Albert ARIBAUD @ 2015-01-31 21:25 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-31 21:25 ` Albert ARIBAUD @ 2015-01-31 21:49 ` Tom Rini 2015-01-31 22:14 ` Simon Glass 0 siblings, 1 reply; 30+ messages in thread From: Tom Rini @ 2015-01-31 21:49 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-31 21:49 ` Tom Rini @ 2015-01-31 22:14 ` Simon Glass 2015-02-02 18:56 ` Tom Rini 0 siblings, 1 reply; 30+ messages in thread From: Simon Glass @ 2015-01-31 22:14 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-31 22:14 ` Simon Glass @ 2015-02-02 18:56 ` Tom Rini 2015-02-02 19:26 ` Simon Glass 2015-02-04 8:48 ` Albert ARIBAUD 0 siblings, 2 replies; 30+ messages in thread From: Tom Rini @ 2015-02-02 18:56 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-02 18:56 ` Tom Rini @ 2015-02-02 19:26 ` Simon Glass 2015-02-04 8:48 ` Albert ARIBAUD 1 sibling, 0 replies; 30+ messages in thread From: Simon Glass @ 2015-02-02 19:26 UTC (permalink / raw) To: u-boot +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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-02 18:56 ` Tom Rini 2015-02-02 19:26 ` Simon Glass @ 2015-02-04 8:48 ` Albert ARIBAUD 2015-02-05 3:00 ` Simon Glass 2015-02-10 22:07 ` Tom Rini 1 sibling, 2 replies; 30+ messages in thread From: Albert ARIBAUD @ 2015-02-04 8:48 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-04 8:48 ` Albert ARIBAUD @ 2015-02-05 3:00 ` Simon Glass 2015-02-05 8:27 ` Hans de Goede 2015-02-05 15:14 ` Albert ARIBAUD 2015-02-10 22:07 ` Tom Rini 1 sibling, 2 replies; 30+ messages in thread From: Simon Glass @ 2015-02-05 3:00 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-05 3:00 ` Simon Glass @ 2015-02-05 8:27 ` Hans de Goede 2015-02-05 15:14 ` Albert ARIBAUD 1 sibling, 0 replies; 30+ messages in thread From: Hans de Goede @ 2015-02-05 8:27 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-05 3:00 ` Simon Glass 2015-02-05 8:27 ` Hans de Goede @ 2015-02-05 15:14 ` Albert ARIBAUD 2015-02-05 15:34 ` Simon Glass 1 sibling, 1 reply; 30+ messages in thread From: Albert ARIBAUD @ 2015-02-05 15:14 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-05 15:14 ` Albert ARIBAUD @ 2015-02-05 15:34 ` Simon Glass 2015-02-05 18:02 ` Albert ARIBAUD 0 siblings, 1 reply; 30+ messages in thread From: Simon Glass @ 2015-02-05 15:34 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-05 15:34 ` Simon Glass @ 2015-02-05 18:02 ` Albert ARIBAUD 2015-02-05 19:13 ` Simon Glass 0 siblings, 1 reply; 30+ messages in thread From: Albert ARIBAUD @ 2015-02-05 18:02 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-05 18:02 ` Albert ARIBAUD @ 2015-02-05 19:13 ` Simon Glass 0 siblings, 0 replies; 30+ messages in thread From: Simon Glass @ 2015-02-05 19:13 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-04 8:48 ` Albert ARIBAUD 2015-02-05 3:00 ` Simon Glass @ 2015-02-10 22:07 ` Tom Rini 2015-02-10 23:27 ` Simon Glass 1 sibling, 1 reply; 30+ messages in thread From: Tom Rini @ 2015-02-10 22:07 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-02-10 22:07 ` Tom Rini @ 2015-02-10 23:27 ` Simon Glass 0 siblings, 0 replies; 30+ messages in thread From: Simon Glass @ 2015-02-10 23:27 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-22 16:20 ` Tom Rini 2015-01-22 19:10 ` Hans de Goede @ 2015-01-26 8:09 ` Albert ARIBAUD 2015-01-26 10:50 ` Hans de Goede 1 sibling, 1 reply; 30+ messages in thread From: Albert ARIBAUD @ 2015-01-26 8:09 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S 2015-01-26 8:09 ` Albert ARIBAUD @ 2015-01-26 10:50 ` Hans de Goede 0 siblings, 0 replies; 30+ messages in thread From: Hans de Goede @ 2015-01-26 10:50 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2015-02-10 23:27 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-21 20:03 [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 2/3] ARMv7: Add a cpu_init_cortex_a7 helper function Hans de Goede 2015-01-21 20:03 ` [U-Boot] [PATCH 3/3] sunxi: Switch to using soc_init hook for setting the ACTLR.SMP bit Hans de Goede 2015-02-08 5:42 ` Ian Campbell 2015-01-21 21:59 ` [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S Bill Pringlemeir 2015-01-22 13:29 ` Hans de Goede 2015-01-22 15:48 ` Bill Pringlemeir 2015-01-22 16:20 ` Tom Rini 2015-01-22 19:10 ` Hans de Goede 2015-01-22 21:03 ` Tom Rini 2015-01-23 8:54 ` Hans de Goede 2015-01-26 15:18 ` Tom Rini 2015-01-26 19:32 ` Hans de Goede 2015-01-27 14:23 ` Tom Rini 2015-01-31 21:25 ` Albert ARIBAUD 2015-01-31 21:49 ` Tom Rini 2015-01-31 22:14 ` Simon Glass 2015-02-02 18:56 ` Tom Rini 2015-02-02 19:26 ` Simon Glass 2015-02-04 8:48 ` Albert ARIBAUD 2015-02-05 3:00 ` Simon Glass 2015-02-05 8:27 ` Hans de Goede 2015-02-05 15:14 ` Albert ARIBAUD 2015-02-05 15:34 ` Simon Glass 2015-02-05 18:02 ` Albert ARIBAUD 2015-02-05 19:13 ` Simon Glass 2015-02-10 22:07 ` Tom Rini 2015-02-10 23:27 ` Simon Glass 2015-01-26 8:09 ` Albert ARIBAUD 2015-01-26 10:50 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.