| Message ID | 20210126140052.3451769-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [v1] platform/x86: Kconfig: Surround WMI drivers by 'if ACPI_WMI' | expand |
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 >
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
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.
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
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.
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
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?
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
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(-)