Hi Bin There's a similar discussion in a cleanup series I've sent [1] On Sat, Sep 11, 2021 at 10:31:23PM +0800, Bin Meng wrote: > Commit 47d73ba4f4a4 ("board: sifive: overwrite board_fdt_blob_setup in u-boot > proper") > added a board-specific implementation of board_fdt_blob_setup() which > takes a pointer as the return value, but it does not return anything > if CONFIG_OF_SEPARATE is not enabled. This will cause a build warning > seen when testing booting S-mode U-Boot directly from QEMU, per the > instructions in [1]:
It's not only a build warning, you don't even have a DTB to begin with. > > board/sifive/unleashed/unleashed.c: In function ‘board_fdt_blob_setup’: > board/sifive/unleashed/unleashed.c:125:1: warning: control reaches end of > non-void function [-Wreturn-type] > > Return &_end as the default case. > > [1] > https://qemu.readthedocs.io/en/latest/system/riscv/sifive_u.html#running-u-boot > > Signed-off-by: Bin Meng <bmeng...@gmail.com> > > --- > > Changes in v2: > - Correct the commit title > > board/sifive/unleashed/unleashed.c | 4 ++-- > board/sifive/unmatched/unmatched.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/board/sifive/unleashed/unleashed.c > b/board/sifive/unleashed/unleashed.c > index 8cd514df30..33baeda986 100644 > --- a/board/sifive/unleashed/unleashed.c > +++ b/board/sifive/unleashed/unleashed.c > @@ -119,9 +119,9 @@ void *board_fdt_blob_setup(void) > if (IS_ENABLED(CONFIG_OF_SEPARATE)) { > if (gd->arch.firmware_fdt_addr) > return (ulong *)gd->arch.firmware_fdt_addr; > - else > - return (ulong *)&_end; > } > + > + return (ulong *)&_end; > } > > int board_init(void) > diff --git a/board/sifive/unmatched/unmatched.c > b/board/sifive/unmatched/unmatched.c > index d90b252bae..8773b660fa 100644 > --- a/board/sifive/unmatched/unmatched.c > +++ b/board/sifive/unmatched/unmatched.c > @@ -16,9 +16,9 @@ void *board_fdt_blob_setup(void) > if (IS_ENABLED(CONFIG_OF_SEPARATE)) { > if (gd->arch.firmware_fdt_addr) > return (ulong *)gd->arch.firmware_fdt_addr; > - else > - return (ulong *)&_end; > } > + > + return (ulong *)&_end; is _end correct here? Doesn't the placement depend on CONFIG_SPL_BUILD and CONFIG_SPL_SEPARATE_BSS? To sum up the other thread I think the overall approach here is not ideal. OF_SEPARATE is used in conjunction with SPL in these boards. What happens (assuming I didn't miss anything), is that SPL passes the DTB to OpenSBI, which in it turn copies to a1 before invoking U-Boot. But we are better of with stricter rules for DTB. - OF_SEPARATE means: I'll read the DTB from the U-Boot binary. - OF_BOARD: The board will somehow provide it. So II think the patch should look something along the lines of: if (IS_ENABLED(CONFIG_OF_SEPARATE)) { #ifdef CONFIG_SPL_BUILD /* FDT is at end of BSS unless it is in a different memory region */ if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS)) fdt_blob = (void *)&_image_binary_end; else fdt_blob = (void *)&__bss_end; #else /* FDT is at end of image */ fdt_blob = (void *)&_end; } if (IS_ENABLED(CONFIG_OF_BOARD)) { fdt_blob = (void *)gd->arch.firmware_fdt_addr; } > } > > int board_init(void) > -- > 2.25.1 > [1] https://lore.kernel.org/u-boot/yvrivfp+sygzh...@apalos.home/ Cheers /Ilias