Re: [PATCH 1/2] spl: riscv: Disable SPL_BINMAN_UBOOT_SYMBOLS by default

[Yao Zi <ziyao@disroot.org>]

On Mon, Apr 07, 2025 at 10:50:07PM +1200, Simon Glass wrote:
> Hi Yao,
> 
> On Mon, 7 Apr 2025 at 15:38, Yao Zi <ziyao@disroot.org> wrote:
> >
> > The default binman configuration of RISC-V wraps proper U-Boot into a
> > FIT image instead of shipping a plain image, thus there's no
> > "u_boot_any" entry by default. Let's disable the option to prevent
> > binman from looking for a plain proper U-Boot image, failing the build
> > with message like
> >
> >   Section '/binman/spl-img': Symbol '_binman_u_boot_any_prop_size'
> >      in entry '/binman/spl-img/mkimage/u-boot-spl/u-boot-spl-nodtb':
> >        Entry 'u-boot-any' not found in list (u-boot-spl-nodtb,
> >        u-boot-spl-dtb,u-boot-spl,mkimage,spl-img)
> 
> Do you know where the symbol is being referenced? This sounds like a bug to me.
> 
> It is likely to be spl_get_image_pos(), perhaps called from
> spl_set_header_raw_uboot().

The symbol is referenced in common/spl/spl.c:54,

	#if CONFIG_IS_ENABLED(BINMAN_UBOOT_SYMBOLS)
	/* See spl.h for information about this */
	#if defined(CONFIG_SPL_BUILD)
	binman_sym_declare(ulong, u_boot_any, image_pos);
	binman_sym_declare(ulong, u_boot_any, size);
	#endif

> If you are using FIT that function should not be called.

The place where binman_sym() is used isn't where an entry in .binman_sym
section is introduced. Instead, binman_sym_declare() actually defines
the entry.

So whether spl_get_image_pos() is invoked doesn't affect the failure,
since in both cases binman_sym_declare() puts an entry into .binman_sym,
confusing binman.

imho SPL_BINMAN_UBOOT_SYMBOLS should depend on !SPL_LOAD_FIT in an ideal
world, but I'm not sure the impact on current codebase thus limited the
change to RISC-V only.

Best regards,
Yao Zi

> >
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  common/spl/Kconfig | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index 7d6780936d1..356ddab38de 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -214,6 +214,8 @@ config SPL_BINMAN_UBOOT_SYMBOLS
> >         bool "Declare binman symbols for U-Boot phases in SPL"
> >         depends on SPL_BINMAN_SYMBOLS
> >         default n if ARCH_IMX8M || ARCH_IMX8ULP || ARCH_IMX9
> > +       # A FIT image is created with default binman configuration of RISC-V
> > +       default n if RISCV
> >         default y
> >         help
> >           This enables use of symbols in SPL which refer to U-Boot phases,
> > --
> > 2.49.0
> >
> 
> Regards,
> Simon