[PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[Alexandre Courbot]

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@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_LOADER
 	help
 	  This enables the Rust abstractions for the firmware loader API.
 
diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
index 20d3e6d0d796..527920f9c4d3 100644
--- a/drivers/gpu/nova-core/Kconfig
+++ b/drivers/gpu/nova-core/Kconfig
@@ -3,7 +3,7 @@ config NOVA_CORE
 	depends on 64BIT
 	depends on PCI
 	depends on RUST
-	depends on RUST_FW_LOADER_ABSTRACTIONS
+	select RUST_FW_LOADER_ABSTRACTIONS
 	select AUXILIARY_BUS
 	default n
 	help
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 98700d069191..d4987fc6b26c 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -132,7 +132,7 @@ config ADIN1100_PHY
 config AMCC_QT2025_PHY
 	tristate "AMCC QT2025 PHY"
 	depends on RUST_PHYLIB_ABSTRACTIONS
-	depends on RUST_FW_LOADER_ABSTRACTIONS
+	select RUST_FW_LOADER_ABSTRACTIONS
 	help
 	  Adds support for the Applied Micro Circuits Corporation QT2025 PHY.
 

---
base-commit: 6553a8f168fb7941ae73d39eccac64f3a2b9b399
change-id: 20251104-b4-select-rust-fw-aeb1e46bcee9

Best regards,
-- 
Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[Danilo Krummrich]

On Tue Nov 4, 2025 at 3:04 PM CET, 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@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.

This patch is the correct approach.

However, I think this should be three separate patches, so they can go through
different trees.

Also, please add a Fixes: tag.

Re: [PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[Greg Kroah-Hartman]

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@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_LOADER

Please 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

Re: [PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[Danilo Krummrich]

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?

Re: [PATCH] firmware_loader: make RUST_FW_LOADER_ABSTRACTIONS select FW_LOADER

[Greg Kroah-Hartman]

On Tue, Nov 04, 2025 at 03:48:10PM +0100, Danilo Krummrich wrote:
> 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?

Ah, good point, I guess this is something like a "feature" that a driver
needs to work properly.  Ok, no objection from me (other than agreeing
that it needs to be split up as you already said.)

thanks,

greg k-h