2010-04-17 00:24:07

by Éric Piel

[permalink] [raw]
Subject: [PATCH 1/2] Make WMI be selected automatically when needed

Many different modules depend on WMI. In Kconfig, some used to "depend"
on it, while others "selected" it. As WMI by itself is useless and more
like a library, it's easier for the user to have it automatically
selected whenever needed. It's especially true for options in completely
different places (like LEDS_DELL_NETBOOKS). So we consistently "select"
it.

Signed-off-by: Éric Piel <[email protected]>
---
drivers/leds/Kconfig | 3 ++-
drivers/platform/x86/Kconfig | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 505eb64..71e8a51 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -280,7 +280,8 @@ config LEDS_ADP5520

config LEDS_DELL_NETBOOKS
tristate "External LED on Dell Business Netbooks"
- depends on X86 && ACPI_WMI
+ depends on X86
+ select ACPI_WMI
help
This adds support for the Latitude 2100 and similar
notebooks that have an external LED.
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7bec458..9808ef3 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -89,8 +89,8 @@ config DELL_LAPTOP

config DELL_WMI
tristate "Dell WMI extras"
- depends on ACPI_WMI
depends on INPUT
+ select ACPI_WMI
---help---
Say Y here if you want to support WMI-based hotkeys on Dell laptops.

@@ -136,9 +136,9 @@ config TC1100_WMI

config HP_WMI
tristate "HP WMI extras"
- depends on ACPI_WMI
depends on INPUT
depends on RFKILL || RFKILL = n
+ select ACPI_WMI
help
Say Y here if you want to support WMI-based hotkeys on HP laptops and
to read data from WMI such as docking or ambient light sensor state.
@@ -387,9 +387,9 @@ config EEEPC_LAPTOP

config EEEPC_WMI
tristate "Eee PC WMI Hotkey Driver (EXPERIMENTAL)"
- depends on ACPI_WMI
depends on INPUT
depends on EXPERIMENTAL
+ select ACPI_WMI
---help---
Say Y here if you want to support WMI-based hotkeys on Eee PC laptops.

@@ -419,9 +419,9 @@ config ACPI_WMI

config MSI_WMI
tristate "MSI WMI extras"
- depends on ACPI_WMI
depends on INPUT
depends on BACKLIGHT_CLASS_DEVICE
+ select ACPI_WMI
select INPUT_SPARSEKMAP
help
Say Y here if you want to support WMI-based hotkeys on MSI laptops.
--
1.7.0.5


2010-04-17 00:33:14

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/2] Make WMI be selected automatically when needed

On Sat, 17 Apr 2010 02:23:57 +0200 ?ric Piel wrote:

> Many different modules depend on WMI. In Kconfig, some used to "depend"
> on it, while others "selected" it. As WMI by itself is useless and more
> like a library, it's easier for the user to have it automatically
> selected whenever needed. It's especially true for options in completely
> different places (like LEDS_DELL_NETBOOKS). So we consistently "select"
> it.
>
> Signed-off-by: ?ric Piel <[email protected]>

config ACPI_WMI
tristate "WMI"
depends on ACPI

so what happens when one of these configs selects ACPI_WMI
but ACPI is not enabled? Hint: that will not enable ACPI.

Did you test that kconfig combination?


> ---
> drivers/leds/Kconfig | 3 ++-
> drivers/platform/x86/Kconfig | 8 ++++----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 505eb64..71e8a51 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -280,7 +280,8 @@ config LEDS_ADP5520
>
> config LEDS_DELL_NETBOOKS
> tristate "External LED on Dell Business Netbooks"
> - depends on X86 && ACPI_WMI
> + depends on X86
> + select ACPI_WMI
> help
> This adds support for the Latitude 2100 and similar
> notebooks that have an external LED.
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 7bec458..9808ef3 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -89,8 +89,8 @@ config DELL_LAPTOP
>
> config DELL_WMI
> tristate "Dell WMI extras"
> - depends on ACPI_WMI
> depends on INPUT
> + select ACPI_WMI
> ---help---
> Say Y here if you want to support WMI-based hotkeys on Dell laptops.
>
> @@ -136,9 +136,9 @@ config TC1100_WMI
>
> config HP_WMI
> tristate "HP WMI extras"
> - depends on ACPI_WMI
> depends on INPUT
> depends on RFKILL || RFKILL = n
> + select ACPI_WMI
> help
> Say Y here if you want to support WMI-based hotkeys on HP laptops and
> to read data from WMI such as docking or ambient light sensor state.
> @@ -387,9 +387,9 @@ config EEEPC_LAPTOP
>
> config EEEPC_WMI
> tristate "Eee PC WMI Hotkey Driver (EXPERIMENTAL)"
> - depends on ACPI_WMI
> depends on INPUT
> depends on EXPERIMENTAL
> + select ACPI_WMI
> ---help---
> Say Y here if you want to support WMI-based hotkeys on Eee PC laptops.
>
> @@ -419,9 +419,9 @@ config ACPI_WMI
>
> config MSI_WMI
> tristate "MSI WMI extras"
> - depends on ACPI_WMI
> depends on INPUT
> depends on BACKLIGHT_CLASS_DEVICE
> + select ACPI_WMI
> select INPUT_SPARSEKMAP
> help
> Say Y here if you want to support WMI-based hotkeys on MSI laptops.
> --


---
~Randy

2010-04-17 00:58:34

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 1/2] Make WMI be selected automatically when needed

Op 17-04-10 02:31, Randy Dunlap schreef:
> On Sat, 17 Apr 2010 02:23:57 +0200 ?ric Piel wrote:
>
>> Many different modules depend on WMI. In Kconfig, some used to "depend"
>> on it, while others "selected" it. As WMI by itself is useless and more
>> like a library, it's easier for the user to have it automatically
>> selected whenever needed. It's especially true for options in completely
>> different places (like LEDS_DELL_NETBOOKS). So we consistently "select"
>> it.
>>
>> Signed-off-by: ?ric Piel <[email protected]>
>
> config ACPI_WMI
> tristate "WMI"
> depends on ACPI
>
> so what happens when one of these configs selects ACPI_WMI
> but ACPI is not enabled? Hint: that will not enable ACPI.
>
> Did you test that kconfig combination?
Indeed, I hadn't really thought of no ACPI support at all... I guess the best
is to just do like the other config which select ACPI_WMI, make them depend
on ACPI. Sounds good to you? (new version appended)

Eric

8<------------------------------
Many different modules depend on WMI. In Kconfig, some used to "depend"
on it, while others "selected" it. As WMI by itself is useless and more
like a library, it's easier for the user to have it automatically
selected whenever needed. It's especially true for options in completely
different places (like LEDS_DELL_NETBOOKS). So we consistently "select"
it.

v2: depend on ACPI to avoid selecting ACPI_WMI without ACPI.

Signed-off-by: ?ric Piel <[email protected]>
---
drivers/leds/Kconfig | 4 +++-
drivers/platform/x86/Kconfig | 12 ++++++++----
2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 505eb64..f937f88 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -280,7 +280,9 @@ config LEDS_ADP5520

config LEDS_DELL_NETBOOKS
tristate "External LED on Dell Business Netbooks"
- depends on X86 && ACPI_WMI
+ depends on X86
+ depends on ACPI
+ select ACPI_WMI
help
This adds support for the Latitude 2100 and similar
notebooks that have an external LED.
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7bec458..28c33e8 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -89,8 +89,9 @@ config DELL_LAPTOP

config DELL_WMI
tristate "Dell WMI extras"
- depends on ACPI_WMI
depends on INPUT
+ depends on ACPI
+ select ACPI_WMI
---help---
Say Y here if you want to support WMI-based hotkeys on Dell laptops.

@@ -136,9 +137,10 @@ config TC1100_WMI

config HP_WMI
tristate "HP WMI extras"
- depends on ACPI_WMI
depends on INPUT
depends on RFKILL || RFKILL = n
+ depends on ACPI
+ select ACPI_WMI
help
Say Y here if you want to support WMI-based hotkeys on HP laptops and
to read data from WMI such as docking or ambient light sensor state.
@@ -387,9 +389,10 @@ config EEEPC_LAPTOP

config EEEPC_WMI
tristate "Eee PC WMI Hotkey Driver (EXPERIMENTAL)"
- depends on ACPI_WMI
depends on INPUT
depends on EXPERIMENTAL
+ depends on ACPI
+ select ACPI_WMI
---help---
Say Y here if you want to support WMI-based hotkeys on Eee PC laptops.

@@ -419,9 +422,10 @@ config ACPI_WMI

config MSI_WMI
tristate "MSI WMI extras"
- depends on ACPI_WMI
depends on INPUT
depends on BACKLIGHT_CLASS_DEVICE
+ depends on ACPI
+ select ACPI_WMI
select INPUT_SPARSEKMAP
help
Say Y here if you want to support WMI-based hotkeys on MSI laptops.
--
1.7.0.5


2010-04-23 19:21:11

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/2] Make WMI be selected automatically when needed

On Sat, Apr 17, 2010 at 02:58:29AM +0200, ?ric Piel wrote:

> config LEDS_DELL_NETBOOKS
> tristate "External LED on Dell Business Netbooks"
> - depends on X86 && ACPI_WMI
> + depends on X86
> + depends on ACPI
> + select ACPI_WMI

I dislike this. The originally stated dependencies are correct and more
accurately represent what the code requires, and changing that for the
sake of making it easier for people to find the driver sounds like it's
trying to cover up shortcomings in our configuration system more than
anything else.

--
Matthew Garrett | [email protected]

2010-04-23 19:52:05

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH 1/2] Make WMI be selected automatically when needed

Op 23-04-10 21:20, Matthew Garrett schreef:
> On Sat, Apr 17, 2010 at 02:58:29AM +0200, ?ric Piel wrote:
>
>> config LEDS_DELL_NETBOOKS
>> tristate "External LED on Dell Business Netbooks"
>> - depends on X86 && ACPI_WMI
>> + depends on X86
>> + depends on ACPI
>> + select ACPI_WMI
>
> I dislike this. The originally stated dependencies are correct and more
> accurately represent what the code requires, and changing that for the
> sake of making it easier for people to find the driver sounds like it's
> trying to cover up shortcomings in our configuration system more than
> anything else.
>
Currently there are 2 modules which select ACPI_WMI and 4 which depend
on it. There are 7 modules which select BACKLIGHT_CLASS_DEVICE and as
many which depend on it. Making usage of the select functionality all
the time would avoid these inconsistencies.

Eric

2010-04-23 20:04:35

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/2] Make WMI be selected automatically when needed

On Fri, Apr 23, 2010 at 09:51:54PM +0200, ?ric Piel wrote:

> Currently there are 2 modules which select ACPI_WMI and 4 which depend
> on it. There are 7 modules which select BACKLIGHT_CLASS_DEVICE and as
> many which depend on it. Making usage of the select functionality all
> the time would avoid these inconsistencies.

Using select for ACPI_WMI is pretty clearly wrong since it fails in
cases where ACPI isn't enabled. But I don't think the appropriate way to
handle that is to encode the dependency between ACPI_WMI and ACPI
multiple times - they should just be switched to depend on it instead.

--
Matthew Garrett | [email protected]