[PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[Mario Limonciello]

This driver can be built on a kernel without rfkill, but events
won't work which causes an unexpected experience.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7d385c3b2239..22b6e7e3da13 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -419,6 +419,7 @@ config WIRELESS_HOTKEY
 	tristate "Wireless hotkey button"
 	depends on ACPI
 	depends on INPUT
+	depends on RFKILL
 	help
 	 This driver provides supports for the wireless buttons found on some AMD,
 	 HP, & Xioami laptops.
-- 
2.25.1

Re: [PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[Hans de Goede]

Hi,

On 7/20/21 4:53 AM, Mario Limonciello wrote:
> This driver can be built on a kernel without rfkill, but events
> won't work which causes an unexpected experience.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Mario I can understand where you are coming from here, but Kconfig
dependencies are meant to express true in kernel dependencies.

The wireless-hotkey driver is purely an ACPI based input driver
from the kernel pov and as such it works fine without RFKILL
being enabled.

So adding a RFKILL dependency is the wrong thing to do here IMHO.

Regards,

Hans



> ---
>  drivers/platform/x86/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 7d385c3b2239..22b6e7e3da13 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -419,6 +419,7 @@ config WIRELESS_HOTKEY
>  	tristate "Wireless hotkey button"
>  	depends on ACPI
>  	depends on INPUT
> +	depends on RFKILL
>  	help
>  	 This driver provides supports for the wireless buttons found on some AMD,
>  	 HP, & Xioami laptops.
> 

Re: [PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[Limonciello, Mario]

On 7/28/2021 07:31, Hans de Goede wrote:
> Hi,
> 
> On 7/20/21 4:53 AM, Mario Limonciello wrote:
>> This driver can be built on a kernel without rfkill, but events
>> won't work which causes an unexpected experience.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Mario I can understand where you are coming from here, but Kconfig
> dependencies are meant to express true in kernel dependencies.
> 
> The wireless-hotkey driver is purely an ACPI based input driver
> from the kernel pov and as such it works fine without RFKILL
> being enabled.
> 
> So adding a RFKILL dependency is the wrong thing to do here IMHO.
> 

Thanks, I get your point.  Is there another type of relationship that 
can be expressed for this?  Is SELECT a better idea perhaps?

> Regards,
> 
> Hans
> 
> 
> 
>> ---
>>   drivers/platform/x86/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 7d385c3b2239..22b6e7e3da13 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -419,6 +419,7 @@ config WIRELESS_HOTKEY
>>   	tristate "Wireless hotkey button"
>>   	depends on ACPI
>>   	depends on INPUT
>> +	depends on RFKILL
>>   	help
>>   	 This driver provides supports for the wireless buttons found on some AMD,
>>   	 HP, & Xioami laptops.
>>
> 

Re: [PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[Hans de Goede]

Hi,

On 7/28/21 4:55 PM, Limonciello, Mario wrote:
> On 7/28/2021 07:31, Hans de Goede wrote:
>> Hi,
>>
>> On 7/20/21 4:53 AM, Mario Limonciello wrote:
>>> This driver can be built on a kernel without rfkill, but events
>>> won't work which causes an unexpected experience.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>
>> Mario I can understand where you are coming from here, but Kconfig
>> dependencies are meant to express true in kernel dependencies.
>>
>> The wireless-hotkey driver is purely an ACPI based input driver
>> from the kernel pov and as such it works fine without RFKILL
>> being enabled.
>>
>> So adding a RFKILL dependency is the wrong thing to do here IMHO.
>>
> 
> Thanks, I get your point.  Is there another type of relationship that can be expressed for this?

No I'm afraid not.

> Is SELECT a better idea perhaps?

No select should only be used when enabling blocks of kernel code
which in essence function as a library used by other code. In general
either all users of a Kconfig symbol should either use depends on,
or select. If different consumers of the functionality mix depends on
and select then the Kconfig tools will typically exit with some error
due to e.g. circular dependencies.

Regards,

Hans




>>> ---
>>>   drivers/platform/x86/Kconfig | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>> index 7d385c3b2239..22b6e7e3da13 100644
>>> --- a/drivers/platform/x86/Kconfig
>>> +++ b/drivers/platform/x86/Kconfig
>>> @@ -419,6 +419,7 @@ config WIRELESS_HOTKEY
>>>       tristate "Wireless hotkey button"
>>>       depends on ACPI
>>>       depends on INPUT
>>> +    depends on RFKILL
>>>       help
>>>        This driver provides supports for the wireless buttons found on some AMD,
>>>        HP, & Xioami laptops.
>>>
>>
> 

Re: [PATCH] platform/x86: Kconfig: Make wireless-hotkey depend on RFKILL

[Hans de Goede]

Hi,

On 7/28/21 5:00 PM, Hans de Goede wrote:
> Hi,
> 
> On 7/28/21 4:55 PM, Limonciello, Mario wrote:
>> On 7/28/2021 07:31, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 7/20/21 4:53 AM, Mario Limonciello wrote:
>>>> This driver can be built on a kernel without rfkill, but events
>>>> won't work which causes an unexpected experience.
>>>>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>
>>> Mario I can understand where you are coming from here, but Kconfig
>>> dependencies are meant to express true in kernel dependencies.
>>>
>>> The wireless-hotkey driver is purely an ACPI based input driver
>>> from the kernel pov and as such it works fine without RFKILL
>>> being enabled.
>>>
>>> So adding a RFKILL dependency is the wrong thing to do here IMHO.
>>>
>>
>> Thanks, I get your point.  Is there another type of relationship that can be expressed for this?
> 
> No I'm afraid not.
> 
>> Is SELECT a better idea perhaps?
> 
> No select should only be used when enabling blocks of kernel code
> which in essence function as a library used by other code. In general
> either all users of a Kconfig symbol should either use depends on,
> or select. If different consumers of the functionality mix depends on
> and select then the Kconfig tools will typically exit with some error
> due to e.g. circular dependencies.

p.s.

More in general all the config options the kernel has simply offer so
much flexiblity that it is impossible to protect users against configurations
which may not work for them.

There simply is not much we can do about this. In general Kconfig changes
like the one which you propose should only be made to avoid having a
possible .config which will not build (typically due to linker errors)

Users should really just stick with def_config-s or distro configs.

Regards,

Hans





> 
> Regards,
> 
> Hans
> 
> 
> 
> 
>>>> ---
>>>>   drivers/platform/x86/Kconfig | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>> index 7d385c3b2239..22b6e7e3da13 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -419,6 +419,7 @@ config WIRELESS_HOTKEY
>>>>       tristate "Wireless hotkey button"
>>>>       depends on ACPI
>>>>       depends on INPUT
>>>> +    depends on RFKILL
>>>>       help
>>>>        This driver provides supports for the wireless buttons found on some AMD,
>>>>        HP, & Xioami laptops.
>>>>
>>>
>>