diff mbox series

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

Message ID 20240826181933.252020-1-hiagofranco@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series fs: Fix SPL build if SPL_FS_LOADER is enabled and FS_LOADER is disabled | expand

Commit Message

Hiago De Franco Aug. 26, 2024, 6:19 p.m. UTC 
From: Hiago De Franco <hiago.franco@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@toradex.com>
Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
---
 fs/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Rini Aug. 26, 2024, 6:25 p.m. UTC | #1 
On Mon, Aug 26, 2024 at 03:19:33PM -0300, Hiago De Franco wrote:

> From: Hiago De Franco <hiago.franco@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@toradex.com>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>

Reviewed-by: Tom Rini <trini@konsulko.com>
Tom Rini Aug. 30, 2024, 8:51 p.m. UTC | #2 
On Mon, Aug 26, 2024 at 03:19:33PM -0300, Hiago De Franco wrote:

> From: Hiago De Franco <hiago.franco@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@toradex.com>
> Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

This leads to failure to build on sandbox_noinst_defconfig.

> ---
>  fs/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index 7b05c79e0ccf..a3ee0a361d3d 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -5,7 +5,7 @@
>  # Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
>  
>  ifdef CONFIG_SPL_BUILD
> -obj-$(CONFIG_FS_LOADER) += fs.o
> +obj-$(CONFIG_SPL_FS_LOADER) += fs.o
>  obj-$(CONFIG_SPL_FS_FAT) += fat/
>  obj-$(CONFIG_SPL_FS_EXT4) += ext4/
>  obj-$(CONFIG_SPL_FS_CBFS) += cbfs/
Francesco Dolcini Aug. 31, 2024, 1:49 p.m. UTC | #3 
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@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@toradex.com>
> > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > Reviewed-by: Tom Rini <trini@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.


?
Tom Rini Sept. 4, 2024, 9:27 p.m. UTC | #4 
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@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@toradex.com>
> > > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > > Reviewed-by: Tom Rini <trini@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.
Francesco Dolcini Sept. 4, 2024, 11:44 p.m. UTC | #5 
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@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@toradex.com>
> > > > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > > > Reviewed-by: Tom Rini <trini@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?

Francesco
Tom Rini Sept. 5, 2024, 3:07 p.m. UTC | #6 
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@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@toradex.com>
> > > > > Signed-off-by: Hiago De Franco <hiago.franco@toradex.com>
> > > > > Reviewed-by: Tom Rini <trini@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.
diff mbox series

Patch

diff --git a/fs/Makefile b/fs/Makefile
index 7b05c79e0ccf..a3ee0a361d3d 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -5,7 +5,7 @@ 
 # Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
 
 ifdef CONFIG_SPL_BUILD
-obj-$(CONFIG_FS_LOADER) += fs.o
+obj-$(CONFIG_SPL_FS_LOADER) += fs.o
 obj-$(CONFIG_SPL_FS_FAT) += fat/
 obj-$(CONFIG_SPL_FS_EXT4) += ext4/
 obj-$(CONFIG_SPL_FS_CBFS) += cbfs/