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

Previous thread: [PATCH 0/2] Change some config options to be automatically selected by Éric Piel on Friday, April 16, 2010 - 5:23 pm. (2 messages)

Next thread: [PATCH 2/2] Make BACKLIGHT_CLASS_DEVICE be selected automatically when needed by Éric Piel on Friday, April 16, 2010 - 5:24 pm. (2 messages)
From: Éric Piel
Date: Friday, April 16, 2010 - 5:23 pm

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 ...
From: Randy Dunlap
Date: Friday, April 16, 2010 - 5:31 pm

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.



---
~Randy
--

From: Éric Piel
Date: Friday, April 16, 2010 - 5:58 pm

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 ...
From: Matthew Garrett
Date: Friday, April 23, 2010 - 12:20 pm

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
--

From: Éric Piel
Date: Friday, April 23, 2010 - 12:51 pm

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
--

From: Matthew Garrett
Date: Friday, April 23, 2010 - 1:04 pm

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
--

Previous thread: [PATCH 0/2] Change some config options to be automatically selected by Éric Piel on Friday, April 16, 2010 - 5:23 pm. (2 messages)

Next thread: [PATCH 2/2] Make BACKLIGHT_CLASS_DEVICE be selected automatically when needed by Éric Piel on Friday, April 16, 2010 - 5:24 pm. (2 messages)