[PATCH 0/2] Introduce spl_arch_init() for architecture specific initialization

[PATCH 0/2] Introduce spl_arch_init() for architecture specific initialization

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>


Currently some architectures use spl_board_init() for their architecture
specific initialization. This prohibits board developers from adding
board init code using said function. This series introduces a new
function in order to separate arch init code from board init code.



Lukas Funke (2):
  spl: Introduce architecture specific init function
  arm64: zynq(mp): Rename spl_board_init() to spl_arch_init()

 arch/arm/Kconfig           | 4 ++--
 arch/arm/mach-zynq/spl.c   | 4 ++--
 arch/arm/mach-zynqmp/spl.c | 4 ++--
 common/spl/Kconfig         | 7 +++++++
 common/spl/spl.c           | 3 +++
 include/spl.h              | 8 ++++++++
 6 files changed, 24 insertions(+), 6 deletions(-)

-- 
2.30.2

[PATCH 1/2] spl: Introduce architecture specific init function

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Some architectures use spl_board_init() in their architecture specific
implementation. Board developers should be able to add board specific
implementation via spl_board_init(). Hence, introduce a spl_arch_init()
method which is called right before spl_board_init() for architecture
specific implementation.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

 common/spl/Kconfig | 7 +++++++
 common/spl/spl.c   | 3 +++
 include/spl.h      | 8 ++++++++
 3 files changed, 18 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 6405374bcc..1a987037bb 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -272,6 +272,13 @@ config SPL_TEXT_BASE
 	help
 	  The address in memory that SPL will be running from.
 
+config SPL_ARCH_INIT
+	bool "Call arch-specific initialization in SPL"
+	help
+	  If this option is enabled, U-Boot will call the function
+	  spl_arch_init() from board_init_r(). This function should be
+	  provided by the architecture.
+
 config SPL_BOARD_INIT
 	bool "Call board-specific initialization in SPL"
 	help
diff --git a/common/spl/spl.c b/common/spl/spl.c
index b65c439e7a..2f2deae14f 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -711,6 +711,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 		}
 	}
 
+	if (CONFIG_IS_ENABLED(ARCH_INIT))
+		spl_arch_init();
+
 	if (CONFIG_IS_ENABLED(BOARD_INIT))
 		spl_board_init();
 
diff --git a/include/spl.h b/include/spl.h
index 043875f10f..2d23c8c0de 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -816,6 +816,14 @@ int spl_early_init(void);
  */
 int spl_init(void);
 
+/*
+ * spl_arch_init() - Do architecture-specific init in SPL
+ *
+ * If SPL_ARCH_INIT is enabled, this is called from board_init_r() before
+ * jumping to the next phase.
+ */
+void spl_arch_init(void);
+
 /*
  * spl_board_init() - Do board-specific init in SPL
  *
-- 
2.30.2

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Wed, Mar 20, 2024 at 02:19:26PM +0100, lukas.funke-oss@weidmueller.com wrote:

> From: Lukas Funke <lukas.funke@weidmueller.com>
> 
> Some architectures use spl_board_init() in their architecture specific
> implementation. Board developers should be able to add board specific
> implementation via spl_board_init(). Hence, introduce a spl_arch_init()
> method which is called right before spl_board_init() for architecture
> specific implementation.
> 
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>

I think this could allow for other SoCs to clean up their existing
ad-hoc mechanisms for SoC/architecture init and then board init, so this
looks like a good idea to me.

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Devarsh Thakkar]

Hi Tom, Lukas,

Thanks for the patch Lukas.

On 20/03/24 20:00, Tom Rini wrote:
> On Wed, Mar 20, 2024 at 02:19:26PM +0100, lukas.funke-oss@weidmueller.com wrote:
> 
>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>
>> Some architectures use spl_board_init() in their architecture specific
>> implementation. Board developers should be able to add board specific
>> implementation via spl_board_init(). Hence, introduce a spl_arch_init()
>> method which is called right before spl_board_init() for architecture
>> specific implementation.
>>
>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> 
> I think this could allow for other SoCs to clean up their existing

Does it make more sense to make this SoC specific then instead of arch
specific to allow broader range of code?

For e.g., if we have something like spl_soc_init then it would help
accommodate soc specific functionality too along with calling arch specific code ?

Also I see some vendors already having implemented spl_soc_init in their arch
specific files [1], so I think it would be good if it is moved it to framework
level.

[1] :
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/riscv/cpu/fu540/spl.c?ref_type=heads#L10
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/riscv/include/asm/arch-fu740/spl.h?ref_type=heads#L12

Regards
Devarsh

> ad-hoc mechanisms for SoC/architecture init and then board init, so this
> looks like a good idea to me.
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

On Wed, Mar 20, 2024 at 08:52:30PM +0530, Devarsh Thakkar wrote:
> Hi Tom, Lukas,
> 
> Thanks for the patch Lukas.
> 
> On 20/03/24 20:00, Tom Rini wrote:
> > On Wed, Mar 20, 2024 at 02:19:26PM +0100, lukas.funke-oss@weidmueller.com wrote:
> > 
> >> From: Lukas Funke <lukas.funke@weidmueller.com>
> >>
> >> Some architectures use spl_board_init() in their architecture specific
> >> implementation. Board developers should be able to add board specific
> >> implementation via spl_board_init(). Hence, introduce a spl_arch_init()
> >> method which is called right before spl_board_init() for architecture
> >> specific implementation.
> >>
> >> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> > 
> > I think this could allow for other SoCs to clean up their existing
> 
> Does it make more sense to make this SoC specific then instead of arch
> specific to allow broader range of code?

"soc" and "arch" are somewhat interchangeable at times, so I think we
can go one step at a time here and bring in this abstraction and see
where everyone else is able to clean their code up to.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Andre Przywara]

On Wed, 20 Mar 2024 11:33:16 -0400
Tom Rini <trini@konsulko.com> wrote:

Hi,

> On Wed, Mar 20, 2024 at 08:52:30PM +0530, Devarsh Thakkar wrote:
> > Hi Tom, Lukas,
> > 
> > Thanks for the patch Lukas.
> > 
> > On 20/03/24 20:00, Tom Rini wrote:  
> > > On Wed, Mar 20, 2024 at 02:19:26PM +0100, lukas.funke-oss@weidmueller.com wrote:
> > >   
> > >> From: Lukas Funke <lukas.funke@weidmueller.com>
> > >>
> > >> Some architectures use spl_board_init() in their architecture specific
> > >> implementation. Board developers should be able to add board specific
> > >> implementation via spl_board_init(). Hence, introduce a spl_arch_init()
> > >> method which is called right before spl_board_init() for architecture
> > >> specific implementation.
> > >>
> > >> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>  
> > > 
> > > I think this could allow for other SoCs to clean up their existing  
> > 
> > Does it make more sense to make this SoC specific then instead of arch
> > specific to allow broader range of code?  
> 
> "soc" and "arch" are somewhat interchangeable at times, so I think we

Isn't "arch" ambiguous anyway? I connect that with CPU architecture, as in
x86, ARM, RISC-V. And we have that in the top level directories: arch/arm,
etc.
But here it's one level below, isn't it? Where "platform" (or "plat")
would be a more suiting term to describe a SoC family, like xilinx or
sunxi?
So the hierarchy would be really: arch -> plat -> soc -> board?

Or am I confused here?

Cheers,
Andre


> can go one step at a time here and bring in this abstraction and see
> where everyone else is able to clean their code up to.
> 

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Lukas Funke]

On 20.03.2024 16:49, Andre Przywara wrote:
> On Wed, 20 Mar 2024 11:33:16 -0400
> Tom Rini <trini@konsulko.com> wrote:
> 
> Hi,
> 
>> On Wed, Mar 20, 2024 at 08:52:30PM +0530, Devarsh Thakkar wrote:
>>> Hi Tom, Lukas,
>>>
>>> Thanks for the patch Lukas.
>>>
>>> On 20/03/24 20:00, Tom Rini wrote:
>>>> On Wed, Mar 20, 2024 at 02:19:26PM +0100, lukas.funke-oss@weidmueller.com wrote:
>>>>    
>>>>> From: Lukas Funke <lukas.funke@weidmueller.com>
>>>>>
>>>>> Some architectures use spl_board_init() in their architecture specific
>>>>> implementation. Board developers should be able to add board specific
>>>>> implementation via spl_board_init(). Hence, introduce a spl_arch_init()
>>>>> method which is called right before spl_board_init() for architecture
>>>>> specific implementation.
>>>>>
>>>>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
>>>>
>>>> I think this could allow for other SoCs to clean up their existing
>>>
>>> Does it make more sense to make this SoC specific then instead of arch
>>> specific to allow broader range of code?
>>
>> "soc" and "arch" are somewhat interchangeable at times, so I think we
> 
> Isn't "arch" ambiguous anyway? I connect that with CPU architecture, as in
> x86, ARM, RISC-V. And we have that in the top level directories: arch/arm,
> etc.
> But here it's one level below, isn't it? Where "platform" (or "plat")
> would be a more suiting term to describe a SoC family, like xilinx or
> sunxi?
> So the hierarchy would be really: arch -> plat -> soc -> board?
> 
> Or am I confused here?

No. But in some cases the 'platform' level is missing: for xilinx it's 
"arch->arm->mach-zynq-><impl>", for rockchip it's 
"arch->arm->plat->soc", i.e. "arch->arm->mach-rockchip->rk3399-><impl>". 
If we follow your proposed rule it should be: 
arch->arm->mach-xilinx->zynq-><impl>. Maybe this is an idea for the next 
cleanup round?

Regarding the patch:

I'd agree that the init code is not (cpu)architecture dependent. Some 
vendors init the console, some init the ddr, some init the leds and so 
on. Thus, we should go one level upwards and change it to 
"spl_soc_init()" if everyone is okay with it?

Best regards,
Lukas

> 
> Cheers,
> Andre
> 
> 
>> can go one step at a time here and bring in this abstraction and see
>> where everyone else is able to clean their code up to.
>>
> 

Re: [PATCH 1/2] spl: Introduce architecture specific init function

[Michal Simek]

čt 21. 3. 2024 v 8:55 odesílatel Lukas Funke <
lukas.funke-oss@weidmueller.com> napsal:

> On 20.03.2024 16:49, Andre Przywara wrote:
> > On Wed, 20 Mar 2024 11:33:16 -0400
> > Tom Rini <trini@konsulko.com> wrote:
> >
> > Hi,
> >
> >> On Wed, Mar 20, 2024 at 08:52:30PM +0530, Devarsh Thakkar wrote:
> >>> Hi Tom, Lukas,
> >>>
> >>> Thanks for the patch Lukas.
> >>>
> >>> On 20/03/24 20:00, Tom Rini wrote:
> >>>> On Wed, Mar 20, 2024 at 02:19:26PM +0100,
> lukas.funke-oss@weidmueller.com wrote:
> >>>>
> >>>>> From: Lukas Funke <lukas.funke@weidmueller.com>
> >>>>>
> >>>>> Some architectures use spl_board_init() in their architecture
> specific
> >>>>> implementation. Board developers should be able to add board specific
> >>>>> implementation via spl_board_init(). Hence, introduce a
> spl_arch_init()
> >>>>> method which is called right before spl_board_init() for architecture
> >>>>> specific implementation.
> >>>>>
> >>>>> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> >>>>
> >>>> I think this could allow for other SoCs to clean up their existing
> >>>
> >>> Does it make more sense to make this SoC specific then instead of arch
> >>> specific to allow broader range of code?
> >>
> >> "soc" and "arch" are somewhat interchangeable at times, so I think we
> >
> > Isn't "arch" ambiguous anyway? I connect that with CPU architecture, as
> in
> > x86, ARM, RISC-V. And we have that in the top level directories:
> arch/arm,
> > etc.
> > But here it's one level below, isn't it? Where "platform" (or "plat")
> > would be a more suiting term to describe a SoC family, like xilinx or
> > sunxi?
> > So the hierarchy would be really: arch -> plat -> soc -> board?
> >
> > Or am I confused here?
>
> No. But in some cases the 'platform' level is missing: for xilinx it's
> "arch->arm->mach-zynq-><impl>", for rockchip it's
> "arch->arm->plat->soc", i.e. "arch->arm->mach-rockchip->rk3399-><impl>".
> If we follow your proposed rule it should be:
> arch->arm->mach-xilinx->zynq-><impl>. Maybe this is an idea for the next
> cleanup round?
>
> Regarding the patch:
>
> I'd agree that the init code is not (cpu)architecture dependent. Some
> vendors init the console, some init the ddr, some init the leds and so
> on. Thus, we should go one level upwards and change it to
> "spl_soc_init()" if everyone is okay with it?
>
>
In our case soc and plat are the same.
Arch is clear that's armv7/v8,
socs/platform are zynq/zynqmp/versal/versal-net
And boards are generic/kria or antminer, syzygy, topic, etc.

And agree that we should do some cleanup in this.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

[PATCH 2/2] arm64: zynq(mp): Rename spl_board_init() to spl_arch_init()

[lukas.funke-oss]

From: Lukas Funke <lukas.funke@weidmueller.com>

Rename spl_board_init() to spl_arch_init(). Architecture specific
implementation should be separated from board specific implementation
in order to be extended by board developers.

Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
---

 arch/arm/Kconfig           | 4 ++--
 arch/arm/mach-zynq/spl.c   | 4 ++--
 arch/arm/mach-zynqmp/spl.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01d6556c42..4ea889d09b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1275,7 +1275,7 @@ config ARCH_ZYNQ
 	select OF_CONTROL
 	select MTD
 	select SPI
-	select SPL_BOARD_INIT if SPL
+	select SPL_ARCH_INIT if SPL
 	select SPL_CLK if SPL
 	select SPL_DM if SPL
 	select SPL_DM_SPI if SPL
@@ -1318,7 +1318,7 @@ config ARCH_ZYNQMP
 	imply FIRMWARE
 	select GICV2
 	select OF_CONTROL
-	select SPL_BOARD_INIT if SPL
+	select SPL_ARCH_INIT if SPL
 	select SPL_CLK if SPL
 	select SPL_DM if SPL
 	select SPL_DM_SPI if SPI && SPL_DM
diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c
index fea1c9b12a..0cae792e21 100644
--- a/arch/arm/mach-zynq/spl.c
+++ b/arch/arm/mach-zynq/spl.c
@@ -32,8 +32,8 @@ void board_init_f(ulong dummy)
 	arch_cpu_init();
 }
 
-#ifdef CONFIG_SPL_BOARD_INIT
-void spl_board_init(void)
+#ifdef CONFIG_SPL_ARCH_INIT
+void spl_arch_init(void)
 {
 	preloader_console_init();
 #if defined(CONFIG_ARCH_EARLY_INIT_R) && defined(CONFIG_SPL_FPGA)
diff --git a/arch/arm/mach-zynqmp/spl.c b/arch/arm/mach-zynqmp/spl.c
index a0f35f36fa..03a45173e6 100644
--- a/arch/arm/mach-zynqmp/spl.c
+++ b/arch/arm/mach-zynqmp/spl.c
@@ -56,8 +56,8 @@ static void ps_mode_reset(ulong mode)
 # define MODE_RESET	PS_MODE1
 #endif
 
-#ifdef CONFIG_SPL_BOARD_INIT
-void spl_board_init(void)
+#ifdef CONFIG_SPL_ARCH_INIT
+void spl_arch_init(void)
 {
 	preloader_console_init();
 	ps_mode_reset(MODE_RESET);
-- 
2.30.2