[PATCH] common/board_f: make sure to call fix_fdt() before reserve_fdt()

[Pragnesh Patel]

Hi Bin,

>-----Original Message-----
>From: Bin Meng <bmeng.cn at gmail.com>
>Sent: 06 August 2020 13:43
>To: Pragnesh Patel <pragnesh.patel at sifive.com>
>Cc: Rick Chen <rickchen36 at gmail.com>; U-Boot Mailing List <u-
>boot at lists.denx.de>; Atish Patra <atish.patra at wdc.com>; Anup Patel
><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Paul
>Walmsley ( Sifive) <paul.walmsley at sifive.com>; Simon Glass
><sjg at chromium.org>; ovpanait at gmail.com; swarren at nvidia.com;
>patrick.delaunay at st.com; vikas.manocha at st.com; masahiroy at kernel.org;
>ye.li at nxp.com; rick <rick at andestech.com>; Alan Kao
><alankao at 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 at sifive.com>
>wrote:
>>
>> Hi Rick,
>>
>> >-----Original Message-----
>> >From: Rick Chen <rickchen36 at gmail.com>
>> >Sent: 06 August 2020 08:22
>> >To: Pragnesh Patel <pragnesh.patel at sifive.com>
>> >Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
>> ><atish.patra at wdc.com>; Bin Meng <bmeng.cn at gmail.com>; Anup Patel
>> ><anup.patel at wdc.com>; Sagar Kadam <sagar.kadam at sifive.com>; Paul
>> >Walmsley ( Sifive) <paul.walmsley at sifive.com>; Simon Glass
>> ><sjg at chromium.org>; ovpanait at gmail.com; swarren at nvidia.com;
>> >patrick.delaunay at st.com; vikas.manocha at st.com; masahiroy at kernel.org;
>> >ye.li at nxp.com; rick <rick at andestech.com>; Alan Kao
>> ><alankao at 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 at sifive.com]
>> >> Sent: Wednesday, August 05, 2020 5:01 PM
>> >> To: atish.patra at wdc.com; bmeng.cn at gmail.com; u-boot at lists.denx.de;
>> >> anup.patel at wdc.com; sagar.kadam at sifive.com; Rick Jian-Zhi Chen(陳建志)
>> >> Cc: paul.walmsley at 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 at 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 at 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