| Message ID | 20200805090053.11805-1-pragnesh.patel@sifive.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | common/board_f: make sure to call fix_fdt() before reserve_fdt() | expand |
On Wed, Aug 5, 2020 at 5:01 PM Pragnesh Patel <pragnesh.patel@sifive.com> wrote: > > There may be a chance that board specific fix_fdt() will change the > size of FDT blob so it's safe to call reserve_fdt() after fix_fdt() > otherwise global data (gd) will overwrite with FDT blob values. > > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > --- > common/board_f.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Hi Pragnesh > From: Pragnesh Patel [mailto:pragnesh.patel@sifive.com] > Sent: Wednesday, August 05, 2020 5:01 PM > To: atish.patra@wdc.com; bmeng.cn@gmail.com; u-boot@lists.denx.de; anup.patel@wdc.com; sagar.kadam@sifive.com; Rick Jian-Zhi Chen(陳建志) > Cc: paul.walmsley@sifive.com; Pragnesh Patel; Simon Glass; Ovidiu Panait; Stephen Warren; Patrick Delaunay; Vikas Manocha; Masahiro Yamada; Ye Li > Subject: [PATCH] common/board_f: make sure to call fix_fdt() before reserve_fdt() > > There may be a chance that board specific fix_fdt() will change the size of FDT blob so it's safe to call reserve_fdt() after fix_fdt() otherwise global data (gd) will overwrite with FDT blob values. > > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > --- > common/board_f.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Maybe you can add the fix tag if it is caused by this. Fixes: a8492e25ac71 ("riscv: Expand the DT size before copy reserved memory node") Reviewed-by: Rick Chen <rick@andestech.com> > diff --git a/common/board_f.c b/common/board_f.c index 88ff0424a7..7ae01e9fff 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { > * - board info struct > */ > setup_dest_addr, > +#ifdef CONFIG_OF_BOARD_FIXUP > + fix_fdt, > +#endif > #ifdef CONFIG_PRAM > reserve_pram, > #endif > @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { > setup_board_part2, > #endif > display_new_sp, > -#ifdef CONFIG_OF_BOARD_FIXUP > - fix_fdt, > -#endif > INIT_FUNC_WATCHDOG_RESET > reloc_fdt, > reloc_bootstage, > -- > 2.17.1
Hi Rick, >-----Original Message----- >From: Rick Chen <rickchen36@gmail.com> >Sent: 06 August 2020 08:22 >To: Pragnesh Patel <pragnesh.patel@sifive.com> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra ><atish.patra@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Anup Patel ><anup.patel@wdc.com>; Sagar Kadam <sagar.kadam@sifive.com>; Paul >Walmsley ( Sifive) <paul.walmsley@sifive.com>; Simon Glass ><sjg@chromium.org>; ovpanait@gmail.com; swarren@nvidia.com; >patrick.delaunay@st.com; vikas.manocha@st.com; masahiroy@kernel.org; >ye.li@nxp.com; rick <rick@andestech.com>; Alan Kao ><alankao@andestech.com> >Subject: Re: [PATCH] common/board_f: make sure to call fix_fdt() before >reserve_fdt() > >[External Email] Do not click links or attachments unless you recognize the >sender and know the content is safe > >Hi Pragnesh > >> From: Pragnesh Patel [mailto:pragnesh.patel@sifive.com] >> Sent: Wednesday, August 05, 2020 5:01 PM >> To: atish.patra@wdc.com; bmeng.cn@gmail.com; u-boot@lists.denx.de; >> anup.patel@wdc.com; sagar.kadam@sifive.com; Rick Jian-Zhi Chen(陳建志) >> Cc: paul.walmsley@sifive.com; Pragnesh Patel; Simon Glass; Ovidiu >> Panait; Stephen Warren; Patrick Delaunay; Vikas Manocha; Masahiro >> Yamada; Ye Li >> Subject: [PATCH] common/board_f: make sure to call fix_fdt() before >> reserve_fdt() >> >> There may be a chance that board specific fix_fdt() will change the size of FDT >blob so it's safe to call reserve_fdt() after fix_fdt() otherwise global data (gd) will >overwrite with FDT blob values. >> >> Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> >> --- >> common/board_f.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> > >Maybe you can add the fix tag if it is caused by this. >Fixes: a8492e25ac71 ("riscv: Expand the DT size before copy reserved memory >node") > >Reviewed-by: Rick Chen <rick@andestech.com> Good suggestion, will update in v2. Thanks for the review. > >> diff --git a/common/board_f.c b/common/board_f.c index >> 88ff0424a7..7ae01e9fff 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { >> * - board info struct >> */ >> setup_dest_addr, >> +#ifdef CONFIG_OF_BOARD_FIXUP >> + fix_fdt, >> +#endif >> #ifdef CONFIG_PRAM >> reserve_pram, >> #endif >> @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { >> setup_board_part2, >> #endif >> display_new_sp, >> -#ifdef CONFIG_OF_BOARD_FIXUP >> - fix_fdt, >> -#endif >> INIT_FUNC_WATCHDOG_RESET >> reloc_fdt, >> reloc_bootstage, >> -- >> 2.17.1
On Thu, Aug 6, 2020 at 12:44 PM Pragnesh Patel <pragnesh.patel@sifive.com> wrote: > > Hi Rick, > > >-----Original Message----- > >From: Rick Chen <rickchen36@gmail.com> > >Sent: 06 August 2020 08:22 > >To: Pragnesh Patel <pragnesh.patel@sifive.com> > >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra > ><atish.patra@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Anup Patel > ><anup.patel@wdc.com>; Sagar Kadam <sagar.kadam@sifive.com>; Paul > >Walmsley ( Sifive) <paul.walmsley@sifive.com>; Simon Glass > ><sjg@chromium.org>; ovpanait@gmail.com; swarren@nvidia.com; > >patrick.delaunay@st.com; vikas.manocha@st.com; masahiroy@kernel.org; > >ye.li@nxp.com; rick <rick@andestech.com>; Alan Kao > ><alankao@andestech.com> > >Subject: Re: [PATCH] common/board_f: make sure to call fix_fdt() before > >reserve_fdt() > > > >[External Email] Do not click links or attachments unless you recognize the > >sender and know the content is safe > > > >Hi Pragnesh > > > >> From: Pragnesh Patel [mailto:pragnesh.patel@sifive.com] > >> Sent: Wednesday, August 05, 2020 5:01 PM > >> To: atish.patra@wdc.com; bmeng.cn@gmail.com; u-boot@lists.denx.de; > >> anup.patel@wdc.com; sagar.kadam@sifive.com; Rick Jian-Zhi Chen(陳建志) > >> Cc: paul.walmsley@sifive.com; Pragnesh Patel; Simon Glass; Ovidiu > >> Panait; Stephen Warren; Patrick Delaunay; Vikas Manocha; Masahiro > >> Yamada; Ye Li > >> Subject: [PATCH] common/board_f: make sure to call fix_fdt() before > >> reserve_fdt() > >> > >> There may be a chance that board specific fix_fdt() will change the size of FDT > >blob so it's safe to call reserve_fdt() after fix_fdt() otherwise global data (gd) will > >overwrite with FDT blob values. > >> > >> Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > >> --- > >> common/board_f.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > > > >Maybe you can add the fix tag if it is caused by this. > >Fixes: a8492e25ac71 ("riscv: Expand the DT size before copy reserved memory > >node") > > > >Reviewed-by: Rick Chen <rick@andestech.com> > > Good suggestion, will update in v2. Thanks for the review. I tend to disagree. The ordering issue is there for a long time and not introduced by a8492e25ac71 so "Fixes" tag is not accurate. It's just a8492e25ac71 triggered the bug, not introduced the bug. > > > > >> diff --git a/common/board_f.c b/common/board_f.c index > >> 88ff0424a7..7ae01e9fff 100644 > >> --- a/common/board_f.c > >> +++ b/common/board_f.c > >> @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { > >> * - board info struct > >> */ > >> setup_dest_addr, > >> +#ifdef CONFIG_OF_BOARD_FIXUP > >> + fix_fdt, > >> +#endif > >> #ifdef CONFIG_PRAM > >> reserve_pram, > >> #endif > >> @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { > >> setup_board_part2, > >> #endif > >> display_new_sp, > >> -#ifdef CONFIG_OF_BOARD_FIXUP > >> - fix_fdt, > >> -#endif > >> INIT_FUNC_WATCHDOG_RESET > >> reloc_fdt, > >> reloc_bootstage, Regards, Bin
Hi Bin, >-----Original Message----- >From: Bin Meng <bmeng.cn@gmail.com> >Sent: 06 August 2020 13:43 >To: Pragnesh Patel <pragnesh.patel@sifive.com> >Cc: Rick Chen <rickchen36@gmail.com>; U-Boot Mailing List <u- >boot@lists.denx.de>; Atish Patra <atish.patra@wdc.com>; Anup Patel ><anup.patel@wdc.com>; Sagar Kadam <sagar.kadam@sifive.com>; Paul >Walmsley ( Sifive) <paul.walmsley@sifive.com>; Simon Glass ><sjg@chromium.org>; ovpanait@gmail.com; swarren@nvidia.com; >patrick.delaunay@st.com; vikas.manocha@st.com; masahiroy@kernel.org; >ye.li@nxp.com; rick <rick@andestech.com>; Alan Kao ><alankao@andestech.com> >Subject: Re: [PATCH] common/board_f: make sure to call fix_fdt() before >reserve_fdt() > >[External Email] Do not click links or attachments unless you recognize the >sender and know the content is safe > >On Thu, Aug 6, 2020 at 12:44 PM Pragnesh Patel <pragnesh.patel@sifive.com> >wrote: >> >> Hi Rick, >> >> >-----Original Message----- >> >From: Rick Chen <rickchen36@gmail.com> >> >Sent: 06 August 2020 08:22 >> >To: Pragnesh Patel <pragnesh.patel@sifive.com> >> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra >> ><atish.patra@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; Anup Patel >> ><anup.patel@wdc.com>; Sagar Kadam <sagar.kadam@sifive.com>; Paul >> >Walmsley ( Sifive) <paul.walmsley@sifive.com>; Simon Glass >> ><sjg@chromium.org>; ovpanait@gmail.com; swarren@nvidia.com; >> >patrick.delaunay@st.com; vikas.manocha@st.com; masahiroy@kernel.org; >> >ye.li@nxp.com; rick <rick@andestech.com>; Alan Kao >> ><alankao@andestech.com> >> >Subject: Re: [PATCH] common/board_f: make sure to call fix_fdt() >> >before >> >reserve_fdt() >> > >> >[External Email] Do not click links or attachments unless you >> >recognize the sender and know the content is safe >> > >> >Hi Pragnesh >> > >> >> From: Pragnesh Patel [mailto:pragnesh.patel@sifive.com] >> >> Sent: Wednesday, August 05, 2020 5:01 PM >> >> To: atish.patra@wdc.com; bmeng.cn@gmail.com; u-boot@lists.denx.de; >> >> anup.patel@wdc.com; sagar.kadam@sifive.com; Rick Jian-Zhi Chen(陳建志) >> >> Cc: paul.walmsley@sifive.com; Pragnesh Patel; Simon Glass; Ovidiu >> >> Panait; Stephen Warren; Patrick Delaunay; Vikas Manocha; Masahiro >> >> Yamada; Ye Li >> >> Subject: [PATCH] common/board_f: make sure to call fix_fdt() before >> >> reserve_fdt() >> >> >> >> There may be a chance that board specific fix_fdt() will change the >> >> size of FDT >> >blob so it's safe to call reserve_fdt() after fix_fdt() otherwise >> >global data (gd) will overwrite with FDT blob values. >> >> >> >> Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> >> >> --- >> >> common/board_f.c | 6 +++--- >> >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> >> > >> >Maybe you can add the fix tag if it is caused by this. >> >Fixes: a8492e25ac71 ("riscv: Expand the DT size before copy reserved >> >memory >> >node") >> > >> >Reviewed-by: Rick Chen <rick@andestech.com> >> >> Good suggestion, will update in v2. Thanks for the review. > >I tend to disagree. The ordering issue is there for a long time and not introduced >by a8492e25ac71 so "Fixes" tag is not accurate. > >It's just a8492e25ac71 triggered the bug, not introduced the bug. I agreed with you that "a8492e25ac71 triggered the bug, not introduced the bug" but this patch obviously add a fix for " a8492e25ac71 " so IMHO there is nothing wrong to add Fixes tag in this patch. If I miss any other Fixes tag for this patch just let me know. > >> >> > >> >> diff --git a/common/board_f.c b/common/board_f.c index >> >> 88ff0424a7..7ae01e9fff 100644 >> >> --- a/common/board_f.c >> >> +++ b/common/board_f.c >> >> @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { >> >> * - board info struct >> >> */ >> >> setup_dest_addr, >> >> +#ifdef CONFIG_OF_BOARD_FIXUP >> >> + fix_fdt, >> >> +#endif >> >> #ifdef CONFIG_PRAM >> >> reserve_pram, >> >> #endif >> >> @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { >> >> setup_board_part2, >> >> #endif >> >> display_new_sp, >> >> -#ifdef CONFIG_OF_BOARD_FIXUP >> >> - fix_fdt, >> >> -#endif >> >> INIT_FUNC_WATCHDOG_RESET >> >> reloc_fdt, >> >> reloc_bootstage, > >Regards, >Bin
On Wed, Aug 5, 2020 at 2:01 AM Pragnesh Patel <pragnesh.patel@sifive.com> wrote: > > There may be a chance that board specific fix_fdt() will change the > size of FDT blob so it's safe to call reserve_fdt() after fix_fdt() > otherwise global data (gd) will overwrite with FDT blob values. > > Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> > --- > common/board_f.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/board_f.c b/common/board_f.c > index 88ff0424a7..7ae01e9fff 100644 > --- a/common/board_f.c > +++ b/common/board_f.c > @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { > * - board info struct > */ > setup_dest_addr, > +#ifdef CONFIG_OF_BOARD_FIXUP > + fix_fdt, > +#endif > #ifdef CONFIG_PRAM > reserve_pram, > #endif > @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { > setup_board_part2, > #endif > display_new_sp, > -#ifdef CONFIG_OF_BOARD_FIXUP > - fix_fdt, > -#endif > INIT_FUNC_WATCHDOG_RESET > reloc_fdt, > reloc_bootstage, > -- > 2.17.1 > Reviewed-by: Atish Patra <atish.patra@wdc.com>
Hi Atish, >-----Original Message----- >From: Atish Patra <atishp@atishpatra.org> >Sent: 10 August 2020 01:51 >To: Pragnesh Patel <pragnesh.patel@sifive.com> >Cc: Atish Patra <atish.patra@wdc.com>; Bin Meng <bmeng.cn@gmail.com>; U- >Boot Mailing List <u-boot@lists.denx.de>; Anup Patel <anup.patel@wdc.com>; >Sagar Kadam <sagar.kadam@sifive.com>; Rick Chen <rick@andestech.com>; >Paul Walmsley ( Sifive) <paul.walmsley@sifive.com>; Simon Glass ><sjg@chromium.org>; Ovidiu Panait <ovpanait@gmail.com>; Stephen Warren ><swarren@nvidia.com>; Patrick Delaunay <patrick.delaunay@st.com>; Vikas >Manocha <vikas.manocha@st.com>; Masahiro Yamada ><masahiroy@kernel.org>; Ye Li <ye.li@nxp.com> >Subject: Re: [PATCH] common/board_f: make sure to call fix_fdt() before >reserve_fdt() > >[External Email] Do not click links or attachments unless you recognize the >sender and know the content is safe > >On Wed, Aug 5, 2020 at 2:01 AM Pragnesh Patel <pragnesh.patel@sifive.com> >wrote: >> >> There may be a chance that board specific fix_fdt() will change the >> size of FDT blob so it's safe to call reserve_fdt() after fix_fdt() >> otherwise global data (gd) will overwrite with FDT blob values. >> >> Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> >> --- >> common/board_f.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/common/board_f.c b/common/board_f.c index >> 88ff0424a7..7ae01e9fff 100644 >> --- a/common/board_f.c >> +++ b/common/board_f.c >> @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { >> * - board info struct >> */ >> setup_dest_addr, >> +#ifdef CONFIG_OF_BOARD_FIXUP >> + fix_fdt, >> +#endif >> #ifdef CONFIG_PRAM >> reserve_pram, >> #endif >> @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { >> setup_board_part2, >> #endif >> display_new_sp, >> -#ifdef CONFIG_OF_BOARD_FIXUP >> - fix_fdt, >> -#endif >> INIT_FUNC_WATCHDOG_RESET >> reloc_fdt, >> reloc_bootstage, >> -- >> 2.17.1 >> > > >Reviewed-by: Atish Patra <atish.patra@wdc.com> Thanks for the review, can you please send your Reviewed-by tag in the v2 version of this patch. >-- >Regards, >Atish
diff --git a/common/board_f.c b/common/board_f.c index 88ff0424a7..7ae01e9fff 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -956,6 +956,9 @@ static const init_fnc_t init_sequence_f[] = { * - board info struct */ setup_dest_addr, +#ifdef CONFIG_OF_BOARD_FIXUP + fix_fdt, +#endif #ifdef CONFIG_PRAM reserve_pram, #endif @@ -984,9 +987,6 @@ static const init_fnc_t init_sequence_f[] = { setup_board_part2, #endif display_new_sp, -#ifdef CONFIG_OF_BOARD_FIXUP - fix_fdt, -#endif INIT_FUNC_WATCHDOG_RESET reloc_fdt, reloc_bootstage,
There may be a chance that board specific fix_fdt() will change the size of FDT blob so it's safe to call reserve_fdt() after fix_fdt() otherwise global data (gd) will overwrite with FDT blob values. Signed-off-by: Pragnesh Patel <pragnesh.patel@sifive.com> --- common/board_f.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)