[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Fabio Estevam]

Hi,

Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.

arch_cpu_init() does CPU level initialization, so why do we need to
include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
which are board related files ?

For example:

Let's say boards X, Y and Z are based on SoC S:

1. If processor S has a arch_cpu_init() defined, then it means that
X,Y,Z need the code from arch_cpu_init() and then we need to define
CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
based on this processor would need CONFIG_ARCH_CPU_INIT)

2. If not all boards need the code inside arch_cpu_init() for
processor S, then it means that this code is not really CPU specific
and then it should be moved to board code.

I was thinking in doing the following:

--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -224,10 +224,15 @@ void __dram_init_banksize(void)
 void dram_init_banksize(void)
        __attribute__((weak, alias("__dram_init_banksize")));

+int __arch_cpu_init(void)
+{
+       return 0;
+}
+int arch_cpu_init(void)
+       __attribute__((weak, alias("__arch_cpu_init")));
+
 init_fnc_t *init_sequence[] = {
-#if defined(CONFIG_ARCH_CPU_INIT)
        arch_cpu_init,          /* basic arch cpu dependent setup */
-#endif
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
        board_early_init_f,
 #endif

,so that CONFIG_ARCH_CPU_INIT is not needed anymore.

Before I go further in this route to remove CONFIG_ARCH_CPU_INIT from
other places, I would like to know if this makes sense.

Thanks,

Fabio Estevam

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Marek Vasut]

> Hi,
> 
> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
> 
> arch_cpu_init() does CPU level initialization, so why do we need to
> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
> which are board related files ?
> 
> For example:
> 
> Let's say boards X, Y and Z are based on SoC S:
> 
> 1. If processor S has a arch_cpu_init() defined, then it means that
> X,Y,Z need the code from arch_cpu_init() and then we need to define
> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
> based on this processor would need CONFIG_ARCH_CPU_INIT)
> 
> 2. If not all boards need the code inside arch_cpu_init() for
> processor S, then it means that this code is not really CPU specific
> and then it should be moved to board code.
> 
> I was thinking in doing the following:
> 
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -224,10 +224,15 @@ void __dram_init_banksize(void)
>  void dram_init_banksize(void)
>         __attribute__((weak, alias("__dram_init_banksize")));
> 
> +int __arch_cpu_init(void)
> +{
> +       return 0;
> +}
> +int arch_cpu_init(void)
> +       __attribute__((weak, alias("__arch_cpu_init")));
> +
>  init_fnc_t *init_sequence[] = {
> -#if defined(CONFIG_ARCH_CPU_INIT)
>         arch_cpu_init,          /* basic arch cpu dependent setup */
> -#endif
>  #if defined(CONFIG_BOARD_EARLY_INIT_F)
>         board_early_init_f,
>  #endif
> 
> ,so that CONFIG_ARCH_CPU_INIT is not needed anymore.
> 
> Before I go further in this route to remove CONFIG_ARCH_CPU_INIT from
> other places, I would like to know if this makes sense.

I'm all for this.

M

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Stefan Roese]

On Thursday 01 March 2012 14:48:12 Marek Vasut wrote:
> > ,so that CONFIG_ARCH_CPU_INIT is not needed anymore.
> > 
> > Before I go further in this route to remove CONFIG_ARCH_CPU_INIT from
> > other places, I would like to know if this makes sense.
> 
> I'm all for this.

Me too.

Thanks,
Stefan

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Albert ARIBAUD]

Le 01/03/2012 14:23, Fabio Estevam a ?crit :
> Hi,
>
> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
>
> arch_cpu_init() does CPU level initialization, so why do we need to
> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
> which are board related files ?
>
> For example:
>
> Let's say boards X, Y and Z are based on SoC S:
>
> 1. If processor S has a arch_cpu_init() defined, then it means that
> X,Y,Z need the code from arch_cpu_init() and then we need to define
> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
> based on this processor would need CONFIG_ARCH_CPU_INIT)
>
> 2. If not all boards need the code inside arch_cpu_init() for
> processor S, then it means that this code is not really CPU specific
> and then it should be moved to board code.

... or some of these boards have a kind of preloader that does CPU level 
inits before U-Boot is loaded, but other have not.

Plus, if a minority of boards for a given SoC can do without the 
arch_cpu_init(), then we'll duplicate code that may well be quite 
identical, won't we?

Amicalement,
-- 
Albert.

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Christian Riesch]

Hi,

On Thu, Mar 1, 2012 at 9:48 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 01/03/2012 14:23, Fabio Estevam a ?crit :
>
>> Hi,
>>
>> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
>>
>> arch_cpu_init() does CPU level initialization, so why do we need to
>> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
>> which are board related files ?
>>
>> For example:
>>
>> Let's say boards X, Y and Z are based on SoC S:
>>
>> 1. If processor S has a arch_cpu_init() defined, then it means that
>> X,Y,Z need the code from arch_cpu_init() and then we need to define
>> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
>> based on this processor would need CONFIG_ARCH_CPU_INIT)
>>
>> 2. If not all boards need the code inside arch_cpu_init() for
>> processor S, then it means that this code is not really CPU specific
>> and then it should be moved to board code.
>
>
> ... or some of these boards have a kind of preloader that does CPU level
> inits before U-Boot is loaded, but other have not.

Exactly. da850 based systems have several options to to lowlevel
configuration, either in the AIS (a script interpreted by the ROM
bootloader of the SoC), or by UBL (a user bootloader from Texas
Instruments that does low level init and then loads u-boot) or by
u-boot itself in arch_cpu_init(). Which way is used is board specific,
but if the initialization is done by u-boot, the code is the same for
all boards, it does not make sense to duplicate it.

Boards that use the low level initialization in u-boot define
CONFIG_ARCH_CPU_INIT, boards that rely on AIS or UBL do not define it.

So if CONFIG_ARCH_CPU_INIT is removed, what should be done with da850
based boards?

Christian

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Christian Riesch]

Hi again,

On Fri, Mar 2, 2012 at 9:46 AM, Christian Riesch
<christian.riesch@omicron.at> wrote:
> Hi,
>
> On Thu, Mar 1, 2012 at 9:48 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Le 01/03/2012 14:23, Fabio Estevam a ?crit :
>>
>>> Hi,
>>>
>>> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
>>>
>>> arch_cpu_init() does CPU level initialization, so why do we need to
>>> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
>>> which are board related files ?
>>>
>>> For example:
>>>
>>> Let's say boards X, Y and Z are based on SoC S:
>>>
>>> 1. If processor S has a arch_cpu_init() defined, then it means that
>>> X,Y,Z need the code from arch_cpu_init() and then we need to define
>>> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
>>> based on this processor would need CONFIG_ARCH_CPU_INIT)
>>>
>>> 2. If not all boards need the code inside arch_cpu_init() for
>>> processor S, then it means that this code is not really CPU specific
>>> and then it should be moved to board code.
>>
>>
>> ... or some of these boards have a kind of preloader that does CPU level
>> inits before U-Boot is loaded, but other have not.
>
> Exactly. da850 based systems have several options to to lowlevel
> configuration, either in the AIS (a script interpreted by the ROM
> bootloader of the SoC), or by UBL (a user bootloader from Texas
> Instruments that does low level init and then loads u-boot) or by
> u-boot itself in arch_cpu_init(). Which way is used is board specific,
> but if the initialization is done by u-boot, the code is the same for
> all boards, it does not make sense to duplicate it.
>
> Boards that use the low level initialization in u-boot define
> CONFIG_ARCH_CPU_INIT, boards that rely on AIS or UBL do not define it.
>
> So if CONFIG_ARCH_CPU_INIT is removed, what should be done with da850
> based boards?
>
> Christian

Sorry, please forget what I wrote above. We already have a
CONFIG_DA850_LOWLEVEL option that already solves the problem. So
removing CONFIG_ARCH_CPU_INIT is fine for me.
Christian

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Simon Glass]

+Graeme

Hi,

On Thu, Mar 1, 2012 at 5:23 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi,
>
> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
>
> arch_cpu_init() does CPU level initialization, so why do we need to
> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
> which are board related files ?
>
> For example:
>
> Let's say boards X, Y and Z are based on SoC S:
>
> 1. If processor S has a arch_cpu_init() defined, then it means that
> X,Y,Z need the code from arch_cpu_init() and then we need to define
> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
> based on this processor would need CONFIG_ARCH_CPU_INIT)
>
> 2. If not all boards need the code inside arch_cpu_init() for
> processor S, then it means that this code is not really CPU specific
> and then it should be moved to board code.
>
> I was thinking in doing the following:
>
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -224,10 +224,15 @@ void __dram_init_banksize(void)
> ?void dram_init_banksize(void)
> ? ? ? ?__attribute__((weak, alias("__dram_init_banksize")));
>
> +int __arch_cpu_init(void)
> +{
> + ? ? ? return 0;
> +}
> +int arch_cpu_init(void)
> + ? ? ? __attribute__((weak, alias("__arch_cpu_init")));
> +
> ?init_fnc_t *init_sequence[] = {
> -#if defined(CONFIG_ARCH_CPU_INIT)
> ? ? ? ?arch_cpu_init, ? ? ? ? ?/* basic arch cpu dependent setup */
> -#endif
> ?#if defined(CONFIG_BOARD_EARLY_INIT_F)
> ? ? ? ?board_early_init_f,
> ?#endif
>
> ,so that CONFIG_ARCH_CPU_INIT is not needed anymore.
>
> Before I go further in this route to remove CONFIG_ARCH_CPU_INIT from
> other places, I would like to know if this makes sense.

I consider arch_cpu_init() to be an architecture function, which
should be defined in arch/arm/cpu/ somewhere. For tegra, the function
is in arch/arm/cpu/armv7/tegra2/board.c

If it is a board function then it should be renamed to
board_cpu_init() or similar.

So regarding your proposed change, I feel that the code size impact is
small and it seems reasonable.

However, Graeme's forthcoming initcall mechanism may anyway may your
change obsolete, since then architectures that need it can insert the
call into the initcall sequence, and other archs need not.

Finally, if we accept this change, then my generic board init series
could/should be changed in the same way.

Regards,
Simon

>
> Thanks,
>
> Fabio Estevam
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Do we really need CONFIG_ARCH_CPU_INIT ?

[Graeme Russ]

Hi Simon, Fabio, Stefano, Marek, Albert

On Fri, Mar 2, 2012 at 8:28 AM, Simon Glass <sjg@chromium.org> wrote:
> +Graeme
>
> Hi,
>
> On Thu, Mar 1, 2012 at 5:23 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi,
>>
>> Currently CONFIG_ARCH_CPU_INIT is used to select arch_cpu_init() function.
>>
>> arch_cpu_init() does CPU level initialization, so why do we need to
>> include CONFIG_ARCH_CPU_INIT in the include/configs/boardXYZ files,
>> which are board related files ?
>>
>> For example:
>>
>> Let's say boards X, Y and Z are based on SoC S:
>>
>> 1. If processor S has a arch_cpu_init() defined, then it means that
>> X,Y,Z need the code from arch_cpu_init() and then we need to define
>> CONFIG_ARCH_CPU_INIT for each of these boards (actually all the boards
>> based on this processor would need CONFIG_ARCH_CPU_INIT)
>>
>> 2. If not all boards need the code inside arch_cpu_init() for
>> processor S, then it means that this code is not really CPU specific
>> and then it should be moved to board code.
>>
>> I was thinking in doing the following:
>>
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -224,10 +224,15 @@ void __dram_init_banksize(void)
>> ?void dram_init_banksize(void)
>> ? ? ? ?__attribute__((weak, alias("__dram_init_banksize")));
>>
>> +int __arch_cpu_init(void)
>> +{
>> + ? ? ? return 0;
>> +}
>> +int arch_cpu_init(void)
>> + ? ? ? __attribute__((weak, alias("__arch_cpu_init")));
>> +
>> ?init_fnc_t *init_sequence[] = {
>> -#if defined(CONFIG_ARCH_CPU_INIT)
>> ? ? ? ?arch_cpu_init, ? ? ? ? ?/* basic arch cpu dependent setup */
>> -#endif
>> ?#if defined(CONFIG_BOARD_EARLY_INIT_F)
>> ? ? ? ?board_early_init_f,
>> ?#endif
>>
>> ,so that CONFIG_ARCH_CPU_INIT is not needed anymore.
>>
>> Before I go further in this route to remove CONFIG_ARCH_CPU_INIT from
>> other places, I would like to know if this makes sense.
>
> I consider arch_cpu_init() to be an architecture function, which
> should be defined in arch/arm/cpu/ somewhere. For tegra, the function
> is in arch/arm/cpu/armv7/tegra2/board.c
>
> If it is a board function then it should be renamed to
> board_cpu_init() or similar.
>
> So regarding your proposed change, I feel that the code size impact is
> small and it seems reasonable.
>
> However, Graeme's forthcoming initcall mechanism may anyway may your
> change obsolete, since then architectures that need it can insert the
> call into the initcall sequence, and other archs need not.

I have been following this thread but have not had time to respond...

Simon's assesment is correct - My new INIT_CALL architecture (which I am
still working on ... slowly ...) will move the visibility of CPU, SoC and
arch specific initialisation away from the board configuration

It will also get rid of that #if defined(CONFIG_BOARD_EARLY_INIT_F) as
well (and every other #ifdef in the init sequence arrays)

> Finally, if we accept this change, then my generic board init series
> could/should be changed in the same way.

so it comes down to how to proceed - Live with the current implementation
until INIT_CALL gets implemented (assuming it gets approved) or accept this
proposed patch as a temporary 'solution' until INIT_CALL gets implemented

I think I should just post what I have of the INIT_CALL patches even
though they are very rough and totally incomplete, at least you can all
see how it is designed to work...

Regards,

Graeme