| Message ID | 20190724100119.7626-1-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [U-Boot] board_r: re-order the board_early_init_r() | expand |
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) >> - board_early_init_r, >> #endif >> INIT_FUNC_WATCHDOG_RESET >> #ifdef CONFIG_POST >> -- >> 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 >
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
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 > >> > > > >
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
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(-)