[PATCH] fs: Fix SPL build if SPL_FS_LOADER is enabled and FS_LOADER is disabled

[Tom Rini]

On Thu, Sep 05, 2024 at 01:44:37AM +0200, Francesco Dolcini wrote:
> Hello Tom,
> 
> On Wed, Sep 04, 2024 at 03:27:21PM -0600, Tom Rini wrote:
> > On Sat, Aug 31, 2024 at 03:49:11PM +0200, Francesco Dolcini wrote:
> > > On Fri, Aug 30, 2024 at 02:51:19PM -0600, Tom Rini wrote:
> > > > On Mon, Aug 26, 2024 at 03:19:33PM -0300, Hiago De Franco wrote:
> > > > 
> > > > > From: Hiago De Franco <hiago.franco at toradex.com>
> > > > > 
> > > > > When SPL_FS_LOADER is set to y and FS_LOADER is not enabled, the SPL build
> > > > > fails with the following errors:
> > > > > 
> > > > >   AR      spl/boot/built-in.o
> > > > >   LD      spl/u-boot-spl
> > > > > arm-none-linux-gnueabihf-ld.bfd: drivers/misc/fs_loader.o: in function
> > > > > `fw_get_filesystem_firmware':
> > > > > /u-boot/drivers/misc/fs_loader.c:162: undefined reference to
> > > > > `fs_set_blk_dev'
> > > > > arm-none-linux-gnueabihf-ld.bfd: /home/frh/tdx/src/u-boot/drivers/misc/
> > > > > fs_loader.c:185: undefined reference to `fs_read'
> > > > > arm-none-linux-gnueabihf-ld.bfd: drivers/misc/fs_loader.o: in function
> > > > > `select_fs_dev':
> > > > > /u-boot/drivers/misc/fs_loader.c:89: undefined reference to
> > > > > `fs_set_blk_dev_with_part'
> > > > > make[1]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
> > > > > make: *** [Makefile:2055: spl/u-boot-spl] Error 2
> > > > > 
> > > > > Fix it by replacing the FS_LOADER with SPL_FS_LOADER in the Makefile, so
> > > > > the fs.c with the necessary function definitions are compiled.
> > > > > 
> > > > > Fixes: b071a07743d4 ("drivers: misc: Makefile: Enable fs_loader compilation at SPL Level")
> > > > > Suggested-by: Francesco Dolcini <francesco.dolcini at toradex.com>
> > > > > Signed-off-by: Hiago De Franco <hiago.franco at toradex.com>
> > > > > Reviewed-by: Tom Rini <trini at konsulko.com>
> > > > 
> > > > This leads to failure to build on sandbox_noinst_defconfig.
> > > 
> > > --- a/drivers/block/Kconfig
> > > +++ b/drivers/block/Kconfig
> > > @@ -110,6 +110,7 @@ config EFI_MEDIA
> > >  config SPL_BLK_FS
> > >         bool "Load images from filesystems on block devices"
> > >         depends on SPL_BLK
> > > +       select SPL_FS_LOADER
> > >         help
> > >           Use generic support to load images from fat/ext filesystems on
> > >           different types of block devices such as NVMe.
> > > 
> > > 
> > > ?
> > 
> > I think "depends on" and sandbox_noinst_defconfig should enable
> > CONFIG_SPL_FS_LOADER too.
> 
> What do you mean?
> 
> The change I proposed fixes the issue you reported with sandbox_noinst_defconfig and I
> think is the right solution. Are you ok with that or are you proposing to just
> change sandbox_noinst_defconfig enabling CONFIG_SPL_FS_LOADER explicitly?

I'm saying, based on the other options which use SPL_FS_LOADER it should
be a depends not select here, and that sandbox_noinst_defconfig should
be enabling SPL_FS_LOADER. There's also an argument to be made that
SPL_FS_LOADER (and FS_LOADER) should be unprompted options (ie "bool"
without the text to explain the option) and selected by all of its
users. I'm not immediately sure which is better now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240905/05e5f273/attachment.sig>