Skip to contentSkip to conversation

leds: make LEDS_GPIO_REGISTER depend on NEW_LEDS

PATCH In review Axel Lin 2 participants 4 messages May 28, 2011 – May 31, 2011 LED

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 leds: make LEDS_GPIO_REGISTER depend on NEW_LEDS

Commit 4440673a "leds: provide helper to register "leds-gpio" devices" adds "config LEDS_GPIO_REGISTER" in drivers/leds/Kconfig.
However, config LEDS_GPIO_REGISTER does not depend on NEW_LEDS, while executing make menuconfig a side-effect is observed that the "LED drivers" and "LED Triggers" are now displayed at the same level of "LED Support". ( They was in sub-menu of "LED Support" before the commit. )

This patch makes LEDS_GPIO_REGISTER depend on NEW_LEDS to correctly show "LED drivers" and "LED Triggers" in sub-menu of "LED Support".

Besides, LEDS_GPIO_REGISTER is set to be bool but no description. Thus I cannot find LEDS_GPIO_REGISTER config option while executing make menuconfig.

This patch also adds the description for LEDS_GPIO_REGISTER so the option can be correctly displayed.

---
 drivers/leds/Kconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/leds/Kconfig +2 -1
index 23f0d5e..784f74f 100644
@@ -15,7 +15,8 @@ config LEDS_CLASS
 	  need this to do anything useful with LEDs.  If unsure, say N.
 
 config LEDS_GPIO_REGISTER
-	bool
+	bool "LED GPIO Register Support"
+	depends on NEW_LEDS
 	help
 	  This option provides the function gpio_led_register_device.
 	  As this function is used by arch code it must not be compiled as a
Signature · 1 line
1.7.1
#1 lore raw

Hello Axel,

Quoted 6 lines · Axel Lin

Commit 4440673a "leds: provide helper to register "leds-gpio" devices" adds "config LEDS_GPIO_REGISTER" in drivers/leds/Kconfig.
However, config LEDS_GPIO_REGISTER does not depend on NEW_LEDS, while executing make menuconfig a side-effect is observed that the "LED drivers" and "LED Triggers" are now displayed at the same level of "LED Support". ( They was in sub-menu of "LED Support" before the commit. )

OK, this was unintended.

Quoted 2 lines

This patch makes LEDS_GPIO_REGISTER depend on NEW_LEDS to correctly show "LED drivers" and "LED Triggers" in sub-menu of "LED Support".

This is wrong, LEDS_GPIO_REGISTER has to be available even when NEW_LEDS=n.

Quoted 2 lines

Besides, LEDS_GPIO_REGISTER is set to be bool but no description. Thus I cannot find LEDS_GPIO_REGISTER config option while executing make menuconfig.

And it's not supposed to be user-selectable because it only provides a function used by arch code. And if nothing uses this function enabling doesn't make sense. OTOH if arch code uses the function disabling doesn't make sense as it would result in a build failure, so arch code needs to select LEDS_GPIO_REGISTER and it doesn't need to be visible.

Below is a patch that fixes the layout problem but keeps the functionality as before and intended.

-----8<-----
leds: move LEDS_GPIO_REGISTER out of menuconfig NEW_LEDS

From: Uwe Kleine-König <[email protected]>

Commit 4440673a "leds: provide helper to register "leds-gpio" devices" broke the display of the NEW_LEDS menu as it didn't depend on NEW_LEDS and so made "LED drivers" and "LED Triggers" appear at the same level as "LED Support" instead of below it as it was before 4440673a.

Moving LEDS_GPIO_REGISTER out of the menuconfig NEW_LEDS fixes this unintended side effect.

---
 drivers/leds/Kconfig |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
drivers/leds/Kconfig +7 -7
index 23f0d5e..12ca135 100644
@@ -1,3 +1,10 @@
+config LEDS_GPIO_REGISTER
+	bool
+	help
+	  This option provides the function gpio_led_register_device.
+	  As this function is used by arch code it must not be compiled as a
+	  module.
+
 menuconfig NEW_LEDS
 	bool "LED Support"
 	help
@@ -14,13 +21,6 @@ config LEDS_CLASS
 	  This option enables the led sysfs class in /sys/class/leds.  You'll
 	  need this to do anything useful with LEDs.  If unsure, say N.
 
-config LEDS_GPIO_REGISTER
-	bool
-	help
-	  This option provides the function gpio_led_register_device.
-	  As this function is used by arch code it must not be compiled as a
-	  module.
-
 if NEW_LEDS
 
 comment "LED drivers"
Signature · 7 lines
1.7.2.5



-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw

hi Uwe,

Quoted 13 lines

Below is a patch that fixes the layout problem but keeps the functionality as before and intended.

-----8<-----
leds: move LEDS_GPIO_REGISTER out of menuconfig NEW_LEDS

From: Uwe Kleine-König <[email protected]>

Commit 4440673a "leds: provide helper to register "leds-gpio" devices" broke the display of the NEW_LEDS menu as it didn't depend on NEW_LEDS and so made "LED drivers" and "LED Triggers" appear at the same level as "LED Support" instead of below it as it was before 4440673a.

Quoted 6 lines

Moving LEDS_GPIO_REGISTER out of the menuconfig NEW_LEDS fixes this unintended side effect.

---

Your patch looks good.

Thanks,
Axel

#3 lore raw
Quoted 14 lines · Axel Lin

hi Uwe,

Quoted 13 lines

Below is a patch that fixes the layout problem but keeps the functionality as before and intended.

-----8<-----
leds: move LEDS_GPIO_REGISTER out of menuconfig NEW_LEDS

From: Uwe Kleine-König <[email protected]>

Commit 4440673a "leds: provide helper to register "leds-gpio" devices" broke the display of the NEW_LEDS menu as it didn't depend on NEW_LEDS and so made "LED drivers" and "LED Triggers" appear at the same level as "LED Support" instead of below it as it was before 4440673a.

Quoted 7 lines
Quoted 6 lines

Moving LEDS_GPIO_REGISTER out of the menuconfig NEW_LEDS fixes this unintended side effect.

---

Your patch looks good.

Richard, thoughts? Andrew, do you take my patch provided that Richard is OK with it?

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |