linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] Make WMI be selected automatically when needed
@ 2010-04-17  0:23 Éric Piel
  2010-04-17  0:31 ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Éric Piel @ 2010-04-17  0:23 UTC (permalink / raw)
  To: platform-driver-x86, Andrew Morton
  Cc: Brown, Len, Matthew Garrett, Linux Kernel Mailing List

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 <eric.piel@tremplin-utc.net>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Make WMI be selected automatically when needed
  2010-04-17  0:23 [PATCH 1/2] Make WMI be selected automatically when needed Éric Piel
@ 2010-04-17  0:31 ` Randy Dunlap
  2010-04-17  0:58   ` Éric Piel
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2010-04-17  0:31 UTC (permalink / raw)
  To: Éric Piel
  Cc: platform-driver-x86, Andrew Morton, Brown, Len, Matthew Garrett,
	Linux Kernel Mailing List

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 <eric.piel@tremplin-utc.net>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Make WMI be selected automatically when needed
  2010-04-17  0:31 ` Randy Dunlap
@ 2010-04-17  0:58   ` Éric Piel
  2010-04-23 19:20     ` Matthew Garrett
  0 siblings, 1 reply; 6+ messages in thread
From: Éric Piel @ 2010-04-17  0:58 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: platform-driver-x86, Andrew Morton, Brown, Len, Matthew Garrett,
	Linux Kernel Mailing List

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 <eric.piel@tremplin-utc.net>
> 
> 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 <eric.piel@tremplin-utc.net>
---
 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


 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Make WMI be selected automatically when needed
  2010-04-17  0:58   ` Éric Piel
@ 2010-04-23 19:20     ` Matthew Garrett
  2010-04-23 19:51       ` Éric Piel
  0 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2010-04-23 19:20 UTC (permalink / raw)
  To: Éric Piel
  Cc: Randy Dunlap, platform-driver-x86, Andrew Morton, Brown, Len,
	Linux Kernel Mailing List

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 | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Make WMI be selected automatically when needed
  2010-04-23 19:20     ` Matthew Garrett
@ 2010-04-23 19:51       ` Éric Piel
  2010-04-23 20:04         ` Matthew Garrett
  0 siblings, 1 reply; 6+ messages in thread
From: Éric Piel @ 2010-04-23 19:51 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Randy Dunlap, platform-driver-x86, Andrew Morton, Brown, Len,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Make WMI be selected automatically when needed
  2010-04-23 19:51       ` Éric Piel
@ 2010-04-23 20:04         ` Matthew Garrett
  0 siblings, 0 replies; 6+ messages in thread
From: Matthew Garrett @ 2010-04-23 20:04 UTC (permalink / raw)
  To: Éric Piel
  Cc: Randy Dunlap, platform-driver-x86, Andrew Morton, Brown, Len,
	Linux Kernel Mailing List

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 | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-04-23 20:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-17  0:23 [PATCH 1/2] Make WMI be selected automatically when needed Éric Piel
2010-04-17  0:31 ` Randy Dunlap
2010-04-17  0:58   ` Éric Piel
2010-04-23 19:20     ` Matthew Garrett
2010-04-23 19:51       ` Éric Piel
2010-04-23 20:04         ` Matthew Garrett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).