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

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

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

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)
#endif INIT_FUNC_WATCHDOG_RESET #ifdef CONFIG_POSTboard_early_init_r,
-- 2.17.1

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

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

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

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

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
participants (4)
-
Kever Yang
-
Matthias Brugger
-
Simon Glass
-
Simon Goldschmidt