linux-kernel - Re: [PATCH] leds: make LEDS_GPIO_REGISTER depend on NEW

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 28 May 2011 19:47:33 +0200
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Axel Lin <axel.lin@...il.com>
Cc:	linux-kernel@...r.kernel.org, Richard Purdie <rpurdie@...ys.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	kernel@...gutronix.de
Subject: Re: [PATCH] leds: make LEDS_GPIO_REGISTER depend on NEW_LEDS

Hello Axel,

On Sat, May 28, 2011 at 05:52:56PM +0800, Axel Lin wrote:
> 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.
 
> 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.
 
> 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 <u.kleine-koenig@...gutronix.de>

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.

Reported-by: Axel Lin <axel.lin@...il.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
---
 drivers/leds/Kconfig |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 23f0d5e..12ca135 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -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"
-- 
1.7.2.5



-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/