Re: [PATCH] common/board_f: make sure to call fix_fdt() before reserve_fdt()
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
participants (3)
-
Bin Meng -
Pragnesh Patel -
Rick Chen