[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Kever Yang]

The board_early_init_r() suppose to be called before board_init(),
then the board callback functions in board_r will be:
- board_early_init_r()
- board_init()
- board_late_init()

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 common/board_r.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index abc31b17b8..c5e33c4654 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -681,6 +681,9 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_DM
 	initr_dm,
 #endif
+#if defined(CONFIG_BOARD_EARLY_INIT_R)
+	board_early_init_r,
+#endif
 #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
 	defined(CONFIG_SANDBOX)
 	board_init,	/* Setup chipselects */
@@ -712,9 +715,6 @@ static init_fnc_t init_sequence_r[] = {
 #endif
 #ifdef CONFIG_ADDR_MAP
 	initr_addr_map,
-#endif
-#if defined(CONFIG_BOARD_EARLY_INIT_R)
-	board_early_init_r,
 #endif
 	INIT_FUNC_WATCHDOG_RESET
 #ifdef CONFIG_POST
-- 
2.17.1

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Simon Goldschmidt]

On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> The board_early_init_r() suppose to be called before board_init(),
> then the board callback functions in board_r will be:
> - board_early_init_r()
> - board_init()
> - board_late_init()

Searching through the code, elixir.bootlin.com gives me 52 definitions
of board_early_init_r(). Does this patch break any of those boards
when it changes the order of those calls?

Regards,
Simon

>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
>  common/board_r.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index abc31b17b8..c5e33c4654 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -681,6 +681,9 @@ static init_fnc_t init_sequence_r[] = {
>  #ifdef CONFIG_DM
>         initr_dm,
>  #endif
> +#if defined(CONFIG_BOARD_EARLY_INIT_R)
> +       board_early_init_r,
> +#endif
>  #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
>         defined(CONFIG_SANDBOX)
>         board_init,     /* Setup chipselects */
> @@ -712,9 +715,6 @@ static init_fnc_t init_sequence_r[] = {
>  #endif
>  #ifdef CONFIG_ADDR_MAP
>         initr_addr_map,
> -#endif
> -#if defined(CONFIG_BOARD_EARLY_INIT_R)
> -       board_early_init_r,
>  #endif
>         INIT_FUNC_WATCHDOG_RESET
>  #ifdef CONFIG_POST
> --
> 2.17.1
>

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Kever Yang]

On 2019/7/24 下午6:22, Simon Goldschmidt wrote:
> On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>> The board_early_init_r() suppose to be called before board_init(),
>> then the board callback functions in board_r will be:
>> - board_early_init_r()
>> - board_init()
>> - board_late_init()
> Searching through the code, elixir.bootlin.com gives me 52 definitions
> of board_early_init_r(). Does this patch break any of those boards
> when it changes the order of those calls?

I do have check some of the implement and most of them should be OK, but 
to be honest,

I'm don't have any of those boards, and not sure if this break any of 
them, and I'm not sure

if people using this interface have notice it's after the board_init().

When I try to use this board_early_init_r(), I thought this is before 
board_init(), but it actually

after the board_init(), that make people confusing.

I think the _early_ one should be at the first, isn't it?

Thanks,
- Kever
>
> Regards,
> Simon
>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>>   common/board_r.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/board_r.c b/common/board_r.c
>> index abc31b17b8..c5e33c4654 100644
>> --- a/common/board_r.c
>> +++ b/common/board_r.c
>> @@ -681,6 +681,9 @@ static init_fnc_t init_sequence_r[] = {
>>   #ifdef CONFIG_DM
>>          initr_dm,
>>   #endif
>> +#if defined(CONFIG_BOARD_EARLY_INIT_R)
>> +       board_early_init_r,
>> +#endif
>>   #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
>>          defined(CONFIG_SANDBOX)
>>          board_init,     /* Setup chipselects */
>> @@ -712,9 +715,6 @@ static init_fnc_t init_sequence_r[] = {
>>   #endif
>>   #ifdef CONFIG_ADDR_MAP
>>          initr_addr_map,
>> -#endif
>> -#if defined(CONFIG_BOARD_EARLY_INIT_R)
>> -       board_early_init_r,
>>   #endif
>>          INIT_FUNC_WATCHDOG_RESET
>>   #ifdef CONFIG_POST
>> --
>> 2.17.1
>>

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Matthias Brugger]

On 24/07/2019 14:22, Kever Yang wrote:
> 
> On 2019/7/24 下午6:22, Simon Goldschmidt wrote:
>> On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>>> The board_early_init_r() suppose to be called before board_init(),
>>> then the board callback functions in board_r will be:
>>> - board_early_init_r()
>>> - board_init()
>>> - board_late_init()
>> Searching through the code, elixir.bootlin.com gives me 52 definitions
>> of board_early_init_r(). Does this patch break any of those boards
>> when it changes the order of those calls?
> 
> I do have check some of the implement and most of them should be OK, but to be
> honest,
> 
> I'm don't have any of those boards, and not sure if this break any of them, and
> I'm not sure
> 
> if people using this interface have notice it's after the board_init().
> 
> When I try to use this board_early_init_r(), I thought this is before
> board_init(), but it actually
> 
> after the board_init(), that make people confusing.
> 
> I think the _early_ one should be at the first, isn't it?

I agree. Maybe we should rename it to board_post_init?

Regards,
Matthias

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Kever Yang]

On 2019/7/31 下午3:23, Matthias Brugger wrote:
>
> On 24/07/2019 14:22, Kever Yang wrote:
>> On 2019/7/24 下午6:22, Simon Goldschmidt wrote:
>>> On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>>>> The board_early_init_r() suppose to be called before board_init(),
>>>> then the board callback functions in board_r will be:
>>>> - board_early_init_r()
>>>> - board_init()
>>>> - board_late_init()
>>> Searching through the code, elixir.bootlin.com gives me 52 definitions
>>> of board_early_init_r(). Does this patch break any of those boards
>>> when it changes the order of those calls?
>> I do have check some of the implement and most of them should be OK, but to be
>> honest,
>>
>> I'm don't have any of those boards, and not sure if this break any of them, and
>> I'm not sure
>>
>> if people using this interface have notice it's after the board_init().
>>
>> When I try to use this board_early_init_r(), I thought this is before
>> board_init(), but it actually
>>
>> after the board_init(), that make people confusing.
>>
>> I think the _early_ one should be at the first, isn't it?
> I agree. Maybe we should rename it to board_post_init?
Sorry , do you mean add/rename a board_post_init() for what's done by
board_early_init_r() now and then add/move ad board api before board_init()?
There is a board_late_init(), which is after env init, a new 
board_post_init() seems
not a good idea.


Here is the Kconfig help for BOARD_EARLY_INIT_R, which also means we it 
should
be called before board_init().

config BOARD_EARLY_INIT_R
         bool "Call board-specific init after relocation"
help
           Some boards need to perform initialisation as directly after
           relocation. With this option, U-Boot calls board_early_init_r()
           in the post-relocation init sequence.


Thanks,

- Kever

>
> Regards,
> Matthias
>

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Simon Glass]

Hi Kever,

On Wed, 24 Jul 2019 at 04:01, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> The board_early_init_r() suppose to be called before board_init(),
> then the board callback functions in board_r will be:
> - board_early_init_r()
> - board_init()
> - board_late_init()

board_early_init_r() was introduced for PowerPC as part of creating
the generic board-init code (board_f.c and board_r.c).

I wonder whether any board is actually using both board_init() and
board_early_init_r(). To me they serve the same function.

So I think we should remove board_early_init_r() and change all uses
to board_init() instead. I expect they will mostly be PowerPC.

>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
>  common/board_r.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index abc31b17b8..c5e33c4654 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -681,6 +681,9 @@ static init_fnc_t init_sequence_r[] = {
>  #ifdef CONFIG_DM
>         initr_dm,
>  #endif
> +#if defined(CONFIG_BOARD_EARLY_INIT_R)
> +       board_early_init_r,
> +#endif
>  #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
>         defined(CONFIG_SANDBOX)
>         board_init,     /* Setup chipselects */
> @@ -712,9 +715,6 @@ static init_fnc_t init_sequence_r[] = {
>  #endif
>  #ifdef CONFIG_ADDR_MAP
>         initr_addr_map,
> -#endif
> -#if defined(CONFIG_BOARD_EARLY_INIT_R)
> -       board_early_init_r,
>  #endif
>         INIT_FUNC_WATCHDOG_RESET
>  #ifdef CONFIG_POST
> --
> 2.17.1
>

Regards,
Simon

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Matthias Brugger]

On 31/07/2019 10:58, Kever Yang wrote:
> 
> On 2019/7/31 下午3:23, Matthias Brugger wrote:
>>
>> On 24/07/2019 14:22, Kever Yang wrote:
>>> On 2019/7/24 下午6:22, Simon Goldschmidt wrote:
>>>> On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>> The board_early_init_r() suppose to be called before board_init(),
>>>>> then the board callback functions in board_r will be:
>>>>> - board_early_init_r()
>>>>> - board_init()
>>>>> - board_late_init()
>>>> Searching through the code, elixir.bootlin.com gives me 52 definitions
>>>> of board_early_init_r(). Does this patch break any of those boards
>>>> when it changes the order of those calls?
>>> I do have check some of the implement and most of them should be OK, but to be
>>> honest,
>>>
>>> I'm don't have any of those boards, and not sure if this break any of them, and
>>> I'm not sure
>>>
>>> if people using this interface have notice it's after the board_init().
>>>
>>> When I try to use this board_early_init_r(), I thought this is before
>>> board_init(), but it actually
>>>
>>> after the board_init(), that make people confusing.
>>>
>>> I think the _early_ one should be at the first, isn't it?
>> I agree. Maybe we should rename it to board_post_init?
> Sorry , do you mean add/rename a board_post_init() for what's done by
> board_early_init_r() now and then add/move ad board api before board_init()?
> There is a board_late_init(), which is after env init, a new board_post_init()
> seems
> not a good idea.
> 

My idea was to rename board_early_init_r to board_post_init as it is done after
board_init but before board_late_init. It's not perfect but at least less
confusing then the naming right now.

Regards,
Matthias

> 
> Here is the Kconfig help for BOARD_EARLY_INIT_R, which also means we it should
> be called before board_init().
> 
> config BOARD_EARLY_INIT_R
>         bool "Call board-specific init after relocation"
> help
>           Some boards need to perform initialisation as directly after
>           relocation. With this option, U-Boot calls board_early_init_r()
>           in the post-relocation init sequence.
> 
> 
> Thanks,
> 
> - Kever
> 
>>
>> Regards,
>> Matthias
>>
> 
> 

[U-Boot] [PATCH] board_r: re-order the board_early_init_r()

[Simon Glass]

Hi Matthias,

On Thu, 1 Aug 2019 at 10:40, Matthias Brugger <matthias.bgg@gmail.com> wrote:
>
>
>
> On 31/07/2019 10:58, Kever Yang wrote:
> >
> > On 2019/7/31 下午3:23, Matthias Brugger wrote:
> >>
> >> On 24/07/2019 14:22, Kever Yang wrote:
> >>> On 2019/7/24 下午6:22, Simon Goldschmidt wrote:
> >>>> On Wed, Jul 24, 2019 at 12:01 PM Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>>> The board_early_init_r() suppose to be called before board_init(),
> >>>>> then the board callback functions in board_r will be:
> >>>>> - board_early_init_r()
> >>>>> - board_init()
> >>>>> - board_late_init()
> >>>> Searching through the code, elixir.bootlin.com gives me 52 definitions
> >>>> of board_early_init_r(). Does this patch break any of those boards
> >>>> when it changes the order of those calls?
> >>> I do have check some of the implement and most of them should be OK, but to be
> >>> honest,
> >>>
> >>> I'm don't have any of those boards, and not sure if this break any of them, and
> >>> I'm not sure
> >>>
> >>> if people using this interface have notice it's after the board_init().
> >>>
> >>> When I try to use this board_early_init_r(), I thought this is before
> >>> board_init(), but it actually
> >>>
> >>> after the board_init(), that make people confusing.
> >>>
> >>> I think the _early_ one should be at the first, isn't it?
> >> I agree. Maybe we should rename it to board_post_init?
> > Sorry , do you mean add/rename a board_post_init() for what's done by
> > board_early_init_r() now and then add/move ad board api before board_init()?
> > There is a board_late_init(), which is after env init, a new board_post_init()
> > seems
> > not a good idea.
> >
>
> My idea was to rename board_early_init_r to board_post_init as it is done after
> board_init but before board_late_init. It's not perfect but at least less
> confusing then the naming right now.

No I really think we should merge them. If we really cannot, then
let's rename board_early_init_r() to board_init_powerpc() for now.

Regards,
Simon

>
> Regards,
> Matthias
>
> >
> > Here is the Kconfig help for BOARD_EARLY_INIT_R, which also means we it should
> > be called before board_init().
> >
> > config BOARD_EARLY_INIT_R
> >         bool "Call board-specific init after relocation"
> > help
> >           Some boards need to perform initialisation as directly after
> >           relocation. With this option, U-Boot calls board_early_init_r()
> >           in the post-relocation init sequence.
> >
> >
> > Thanks,
> >
> > - Kever
> >
> >>
> >> Regards,
> >> Matthias
> >>
> >
> >