Greg Kroah-Hartman
2025-Nov-04 14:35 UTC
[PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER
On Tue, Nov 04, 2025 at 11:04:49PM +0900, Alexandre Courbot wrote:> I have noticed that build will fail when doing the following: > > - Start with the x86 defconfig, > - Using nconfig, enable `CONFIG_RUST` and `CONFIG_DRM_NOVA`, > - Start building. > > The problem is that `CONFIG_RUST_FW_LOADER_ABSTRACTIONS` remains > unselected, despite it being a dependency of `CONFIG_NOVA_CORE`. This > seems to happen because `CONFIG_DRM_NOVA` selects `CONFIG_NOVA_CORE`. > > Fix this by making `CONFIG_RUST_FW_LOADER_ABSTRACTIONS` select > `CONFIG_FW_LOADER`, and by transition make all users of > `CONFIG_RUST_FW_LOADER_ABSTRACTIONS` (so far, nova-core and net/phy) > select it as well. > > `CONFIG_FW_LOADER` is more often selected than depended on, so this > seems to make sense generally speaking. > > Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> > --- > I am not 100% percent confident that this is the proper fix, but the > problem is undeniable. :) I guess the alternative would be to make nova-drm > depend on nova-core instead of selecting it, but I suspect that the > `select` behavior is correct in this case - after all, firmware loading > does not make sense without any user. > --- > drivers/base/firmware_loader/Kconfig | 2 +- > drivers/gpu/nova-core/Kconfig | 2 +- > drivers/net/phy/Kconfig | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig > index 752b9a9bea03..15eff8a4b505 100644 > --- a/drivers/base/firmware_loader/Kconfig > +++ b/drivers/base/firmware_loader/Kconfig > @@ -38,7 +38,7 @@ config FW_LOADER_DEBUG > config RUST_FW_LOADER_ABSTRACTIONS > bool "Rust Firmware Loader abstractions" > depends on RUST > - depends on FW_LOADER=y > + select FW_LOADERPlease no, select should almost never be used, it causes hard-to-debug issues. As something is failing, perhaps another "depends" needs to be added somewhere instead? thanks, greg k-h
Danilo Krummrich
2025-Nov-04 14:48 UTC
[PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER
On Tue Nov 4, 2025 at 3:35 PM CET, Greg Kroah-Hartman wrote:> On Tue, Nov 04, 2025 at 11:04:49PM +0900, Alexandre Courbot wrote: >> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig >> index 752b9a9bea03..15eff8a4b505 100644 >> --- a/drivers/base/firmware_loader/Kconfig >> +++ b/drivers/base/firmware_loader/Kconfig >> @@ -38,7 +38,7 @@ config FW_LOADER_DEBUG >> config RUST_FW_LOADER_ABSTRACTIONS >> bool "Rust Firmware Loader abstractions" >> depends on RUST >> - depends on FW_LOADER=y >> + select FW_LOADER > > Please no, select should almost never be used, it causes hard-to-debug > issues.I agree that select can be very annoying at times, but in this case it seems to be the correct thing to do? For instance for something like: config MY_DRIVER depends on PCI depends on DRM select AUXILIARY_BUS select FW_LOADER In this case MY_DRIVER is only available if PCI and DRM is enabled, which makes sense, there is no reason to show users PCI and DRM drivers if both are disabled. However, for things like AUXILIARY_BUS and FW_LOADER, I'd argue that they are implementation details of the driver and should be selected if the driver is selected. Otherwise, wouldn't we expect users to know implementation details of drivers before being able to select them?