[PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Andy Shevchenko]

Surround WMI drivers by 'if ACPI_WMI' instead of depending
each of them separately.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/Kconfig | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 4a5798a0ce0c..288f8f82d796 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -37,9 +37,10 @@ config ACPI_WMI
 	  It is safe to enable this driver even if your DSDT doesn't define
 	  any ACPI-WMI devices.
 
+if ACPI_WMI
+
 config WMI_BMOF
 	tristate "WMI embedded Binary MOF driver"
-	depends on ACPI_WMI
 	default ACPI_WMI
 	help
 	  Say Y here if you want to be able to read a firmware-embedded
@@ -54,7 +55,6 @@ config ALIENWARE_WMI
 	depends on ACPI
 	depends on LEDS_CLASS
 	depends on NEW_LEDS
-	depends on ACPI_WMI
 	help
 	 This is a driver for controlling Alienware BIOS driven
 	 features.  It exposes an interface for controlling the AlienFX
@@ -64,7 +64,6 @@ config ALIENWARE_WMI
 config HUAWEI_WMI
 	tristate "Huawei WMI laptop extras driver"
 	depends on ACPI_BATTERY
-	depends on ACPI_WMI
 	depends on INPUT
 	select INPUT_SPARSEKMAP
 	select LEDS_CLASS
@@ -91,7 +90,6 @@ config UV_SYSFS
 
 config INTEL_WMI_SBL_FW_UPDATE
 	tristate "Intel WMI Slim Bootloader firmware update signaling driver"
-	depends on ACPI_WMI
 	help
 	  Say Y here if you want to be able to use the WMI interface to signal
 	  Slim Bootloader to trigger update on next reboot.
@@ -101,7 +99,6 @@ config INTEL_WMI_SBL_FW_UPDATE
 
 config INTEL_WMI_THUNDERBOLT
 	tristate "Intel WMI thunderbolt force power driver"
-	depends on ACPI_WMI
 	help
 	  Say Y here if you want to be able to use the WMI interface on select
 	  systems to force the power control of Intel Thunderbolt controllers.
@@ -112,22 +109,19 @@ config INTEL_WMI_THUNDERBOLT
 	  be called intel-wmi-thunderbolt.
 
 config MXM_WMI
-       tristate "WMI support for MXM Laptop Graphics"
-       depends on ACPI_WMI
+	tristate "WMI support for MXM Laptop Graphics"
 	help
           MXM is a standard for laptop graphics cards, the WMI interface
 	  is required for switchable nvidia graphics machines
 
 config PEAQ_WMI
 	tristate "PEAQ 2-in-1 WMI hotkey driver"
-	depends on ACPI_WMI
 	depends on INPUT
 	help
 	 Say Y here if you want to support WMI-based hotkeys on PEAQ 2-in-1s.
 
 config XIAOMI_WMI
 	tristate "Xiaomi WMI key driver"
-	depends on ACPI_WMI
 	depends on INPUT
 	help
 	  Say Y here if you want to support WMI-based keys on Xiaomi notebooks.
@@ -135,6 +129,8 @@ config XIAOMI_WMI
 	  To compile this driver as a module, choose M here: the module will
 	  be called xiaomi-wmi.
 
+endif # ACPI_WMI
+
 config ACERHDF
 	tristate "Acer Aspire One temperature and fan driver"
 	depends on ACPI && THERMAL
-- 
2.30.0

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Hans de Goede]

Hi,

On 1/26/21 3:00 PM, Andy Shevchenko wrote:
> Surround WMI drivers by 'if ACPI_WMI' instead of depending
> each of them separately.

This does not cover all drivers which depend on ACPI_WMI; and in
for-next there is a new UV_SYSFS Kconfig symbol in the middle of
the block which you are surrounding with if ACPI_WMI .. endif
and that new Kconfig symbol does not depend on ACPI_WMI.

Admittedly I should have payed more attention when the UV_SYSFS
symbol was merged, but atm there is no real ordering in the
Kconfig symbols.

Personally I think having e.g. the non WMI and WMI based drivers
for say Asus laptops together makes more sense then grouping
the WMI drivers together.

I'm tempted to add a commit to for-next which just sorts everything
alphabetically. Doing so might cause conflicts, so doing it
close (but not too close) to the merge window seems like a good
time to do this.

What do you think (about just sorting all the symbols alphabetically) ?

Regards,

Hans



> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/platform/x86/Kconfig | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 4a5798a0ce0c..288f8f82d796 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -37,9 +37,10 @@ config ACPI_WMI
>  	  It is safe to enable this driver even if your DSDT doesn't define
>  	  any ACPI-WMI devices.
>  
> +if ACPI_WMI
> +
>  config WMI_BMOF
>  	tristate "WMI embedded Binary MOF driver"
> -	depends on ACPI_WMI
>  	default ACPI_WMI
>  	help
>  	  Say Y here if you want to be able to read a firmware-embedded
> @@ -54,7 +55,6 @@ config ALIENWARE_WMI
>  	depends on ACPI
>  	depends on LEDS_CLASS
>  	depends on NEW_LEDS
> -	depends on ACPI_WMI
>  	help
>  	 This is a driver for controlling Alienware BIOS driven
>  	 features.  It exposes an interface for controlling the AlienFX
> @@ -64,7 +64,6 @@ config ALIENWARE_WMI
>  config HUAWEI_WMI
>  	tristate "Huawei WMI laptop extras driver"
>  	depends on ACPI_BATTERY
> -	depends on ACPI_WMI
>  	depends on INPUT
>  	select INPUT_SPARSEKMAP
>  	select LEDS_CLASS
> @@ -91,7 +90,6 @@ config UV_SYSFS
>  
>  config INTEL_WMI_SBL_FW_UPDATE
>  	tristate "Intel WMI Slim Bootloader firmware update signaling driver"
> -	depends on ACPI_WMI
>  	help
>  	  Say Y here if you want to be able to use the WMI interface to signal
>  	  Slim Bootloader to trigger update on next reboot.
> @@ -101,7 +99,6 @@ config INTEL_WMI_SBL_FW_UPDATE
>  
>  config INTEL_WMI_THUNDERBOLT
>  	tristate "Intel WMI thunderbolt force power driver"
> -	depends on ACPI_WMI
>  	help
>  	  Say Y here if you want to be able to use the WMI interface on select
>  	  systems to force the power control of Intel Thunderbolt controllers.
> @@ -112,22 +109,19 @@ config INTEL_WMI_THUNDERBOLT
>  	  be called intel-wmi-thunderbolt.
>  
>  config MXM_WMI
> -       tristate "WMI support for MXM Laptop Graphics"
> -       depends on ACPI_WMI
> +	tristate "WMI support for MXM Laptop Graphics"
>  	help
>            MXM is a standard for laptop graphics cards, the WMI interface
>  	  is required for switchable nvidia graphics machines
>  
>  config PEAQ_WMI
>  	tristate "PEAQ 2-in-1 WMI hotkey driver"
> -	depends on ACPI_WMI
>  	depends on INPUT
>  	help
>  	 Say Y here if you want to support WMI-based hotkeys on PEAQ 2-in-1s.
>  
>  config XIAOMI_WMI
>  	tristate "Xiaomi WMI key driver"
> -	depends on ACPI_WMI
>  	depends on INPUT
>  	help
>  	  Say Y here if you want to support WMI-based keys on Xiaomi notebooks.
> @@ -135,6 +129,8 @@ config XIAOMI_WMI
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called xiaomi-wmi.
>  
> +endif # ACPI_WMI
> +
>  config ACERHDF
>  	tristate "Acer Aspire One temperature and fan driver"
>  	depends on ACPI && THERMAL
> 

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Hans de Goede]

Hi,

On 2/3/21 11:48 AM, Hans de Goede wrote:
> Hi,
> 
> On 1/26/21 3:00 PM, Andy Shevchenko wrote:
>> Surround WMI drivers by 'if ACPI_WMI' instead of depending
>> each of them separately.
> 
> This does not cover all drivers which depend on ACPI_WMI; and in
> for-next there is a new UV_SYSFS Kconfig symbol in the middle of
> the block which you are surrounding with if ACPI_WMI .. endif
> and that new Kconfig symbol does not depend on ACPI_WMI.
> 
> Admittedly I should have payed more attention when the UV_SYSFS
> symbol was merged, but atm there is no real ordering in the
> Kconfig symbols.

Ok, so I just checked the Makefile and noticed that there is
an ordering there. But most people start with looking at Kconfig
when adding a new driver and the ordering is very much not clear
there. Also splitting the Intel bits in three groups does not
necessariyl help IMHO. E.g  CONFIG_INTEL_CHT_INT33FE is in the
generic Intel block but it is definitely PMIC related.

And the WMI drivers are grouped, except that some of them
(Asus, Dell, EEEPC at least) are not in the group...

So I still think just sorting the entire bups alphabetically
is better. We can then also add a comment at the top to please
keep things sorted alphabetically.

Regards,

Hans

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Andy Shevchenko]

On Wed, Feb 03, 2021 at 11:55:40AM +0100, Hans de Goede wrote:
> On 2/3/21 11:48 AM, Hans de Goede wrote:
> > On 1/26/21 3:00 PM, Andy Shevchenko wrote:
> >> Surround WMI drivers by 'if ACPI_WMI' instead of depending
> >> each of them separately.
> > 
> > This does not cover all drivers which depend on ACPI_WMI; and in
> > for-next there is a new UV_SYSFS Kconfig symbol in the middle of
> > the block which you are surrounding with if ACPI_WMI .. endif
> > and that new Kconfig symbol does not depend on ACPI_WMI.
> > 
> > Admittedly I should have payed more attention when the UV_SYSFS
> > symbol was merged, but atm there is no real ordering in the
> > Kconfig symbols.
> 
> Ok, so I just checked the Makefile and noticed that there is
> an ordering there. But most people start with looking at Kconfig
> when adding a new driver and the ordering is very much not clear
> there. Also splitting the Intel bits in three groups does not
> necessariyl help IMHO. E.g  CONFIG_INTEL_CHT_INT33FE is in the
> generic Intel block but it is definitely PMIC related.
> 
> And the WMI drivers are grouped, except that some of them
> (Asus, Dell, EEEPC at least) are not in the group...
> 
> So I still think just sorting the entire bups alphabetically
> is better. We can then also add a comment at the top to please
> keep things sorted alphabetically.

In long term I prefer split the entire folder to subfolders where you create
a new Kconfig with a rules like alphabetical order or so.

Shuffling now Kconfig and Makefile w/o above doesn't bring a value in my
opinion.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Hans de Goede]

Hi,

On 2/3/21 12:33 PM, Andy Shevchenko wrote:
> On Wed, Feb 03, 2021 at 11:55:40AM +0100, Hans de Goede wrote:
>> On 2/3/21 11:48 AM, Hans de Goede wrote:
>>> On 1/26/21 3:00 PM, Andy Shevchenko wrote:
>>>> Surround WMI drivers by 'if ACPI_WMI' instead of depending
>>>> each of them separately.
>>>
>>> This does not cover all drivers which depend on ACPI_WMI; and in
>>> for-next there is a new UV_SYSFS Kconfig symbol in the middle of
>>> the block which you are surrounding with if ACPI_WMI .. endif
>>> and that new Kconfig symbol does not depend on ACPI_WMI.
>>>
>>> Admittedly I should have payed more attention when the UV_SYSFS
>>> symbol was merged, but atm there is no real ordering in the
>>> Kconfig symbols.
>>
>> Ok, so I just checked the Makefile and noticed that there is
>> an ordering there. But most people start with looking at Kconfig
>> when adding a new driver and the ordering is very much not clear
>> there. Also splitting the Intel bits in three groups does not
>> necessariyl help IMHO. E.g  CONFIG_INTEL_CHT_INT33FE is in the
>> generic Intel block but it is definitely PMIC related.
>>
>> And the WMI drivers are grouped, except that some of them
>> (Asus, Dell, EEEPC at least) are not in the group...
>>
>> So I still think just sorting the entire bups alphabetically
>> is better. We can then also add a comment at the top to please
>> keep things sorted alphabetically.
> 
> In long term I prefer split the entire folder to subfolders where you create
> a new Kconfig with a rules like alphabetical order or so.
> 
> Shuffling now Kconfig and Makefile w/o above doesn't bring a value in my
> opinion.

Ok, lets keep things as is for now then and hopefully in the future
someone will have some time to clean this up a bit.

Regards,

Hans

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Andy Shevchenko]

On Wed, Feb 3, 2021 at 2:13 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 2/3/21 12:33 PM, Andy Shevchenko wrote:
> > On Wed, Feb 03, 2021 at 11:55:40AM +0100, Hans de Goede wrote:
> >> On 2/3/21 11:48 AM, Hans de Goede wrote:

...

> >> So I still think just sorting the entire bups alphabetically
> >> is better. We can then also add a comment at the top to please
> >> keep things sorted alphabetically.
> >
> > In long term I prefer split the entire folder to subfolders where you create
> > a new Kconfig with a rules like alphabetical order or so.
> >
> > Shuffling now Kconfig and Makefile w/o above doesn't bring a value in my
> > opinion.
>
> Ok, lets keep things as is for now then and hopefully in the future
> someone will have some time to clean this up a bit.

Or ask contributors from corresponding companies (Dell, Lenovo, etc)
to sort this out at least for their devices. Good we have no surface
issue with this, since it's in a separate (sibling) folder.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Hans de Goede]

Hi,

On 2/3/21 1:16 PM, Andy Shevchenko wrote:
> On Wed, Feb 3, 2021 at 2:13 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 2/3/21 12:33 PM, Andy Shevchenko wrote:
>>> On Wed, Feb 03, 2021 at 11:55:40AM +0100, Hans de Goede wrote:
>>>> On 2/3/21 11:48 AM, Hans de Goede wrote:
> 
> ...
> 
>>>> So I still think just sorting the entire bups alphabetically
>>>> is better. We can then also add a comment at the top to please
>>>> keep things sorted alphabetically.
>>>
>>> In long term I prefer split the entire folder to subfolders where you create
>>> a new Kconfig with a rules like alphabetical order or so.
>>>
>>> Shuffling now Kconfig and Makefile w/o above doesn't bring a value in my
>>> opinion.
>>
>> Ok, lets keep things as is for now then and hopefully in the future
>> someone will have some time to clean this up a bit.
> 
> Or ask contributors from corresponding companies (Dell, Lenovo, etc)
> to sort this out at least for their devices. Good we have no surface
> issue with this, since it's in a separate (sibling) folder.

The biggest group of drivers seem to be Intel drivers, so perhaps
Intel can set a good example here by moving their drivers to one
or more sub-folders ?

Regards,

Hans

Re: [PATCH v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI'

[Andy Shevchenko]

On Wed, Feb 03, 2021 at 01:57:10PM +0100, Hans de Goede wrote:
> On 2/3/21 1:16 PM, Andy Shevchenko wrote:
> > On Wed, Feb 3, 2021 at 2:13 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 2/3/21 12:33 PM, Andy Shevchenko wrote:
> >>> On Wed, Feb 03, 2021 at 11:55:40AM +0100, Hans de Goede wrote:
> >>>> On 2/3/21 11:48 AM, Hans de Goede wrote:
> > 
> > ...
> > 
> >>>> So I still think just sorting the entire bups alphabetically
> >>>> is better. We can then also add a comment at the top to please
> >>>> keep things sorted alphabetically.
> >>>
> >>> In long term I prefer split the entire folder to subfolders where you create
> >>> a new Kconfig with a rules like alphabetical order or so.
> >>>
> >>> Shuffling now Kconfig and Makefile w/o above doesn't bring a value in my
> >>> opinion.
> >>
> >> Ok, lets keep things as is for now then and hopefully in the future
> >> someone will have some time to clean this up a bit.
> > 
> > Or ask contributors from corresponding companies (Dell, Lenovo, etc)
> > to sort this out at least for their devices. Good we have no surface
> > issue with this, since it's in a separate (sibling) folder.
> 
> The biggest group of drivers seem to be Intel drivers, so perhaps
> Intel can set a good example here by moving their drivers to one
> or more sub-folders ?

Why not?

-- 
With Best Regards,
Andy Shevchenko