input: CONFIG_INPUT_APANEL build fix

x86.git testing found this build failure in upstream -git:

drivers/built-in.o: In function `apanel_detach_client':
apanel.c:(.text+0x3027af): undefined reference to led_classdev_unregister' drivers/built-in.o: In function apanel_probe':
apanel.c:(.text+0x302ac9): undefined reference to `led_classdev_register'

with this config:

http://redhat.com/~mingo/misc/config-Wed_Apr_30_20_22_20_CEST_2008.bad

this driver has a missing CONFIG_NEW_LEDS dependency.

Signed-off-by Ingo Molnar <[email protected]>

---
 drivers/input/misc/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/input/misc/Kconfig
===================================================================

drivers/input/misc/Kconfig +1 -1

@@ -43,7 +43,7 @@ config INPUT_M68K_BEEP
 
 config INPUT_APANEL
 	tristate "Fujitsu Lifebook Application Panel buttons"
-	depends on X86 && I2C && LEDS_CLASS
+	depends on X86 && I2C && LEDS_CLASS && NEW_LEDS
 	select INPUT_POLLDEV
 	select CHECK_SIGNATURE
 	help

Quoted 10 lines · Ingo Molnar

x86.git testing found this build failure in upstream -git:

drivers/built-in.o: In function `apanel_detach_client':
apanel.c:(.text+0x3027af): undefined reference to led_classdev_unregister' drivers/built-in.o: In function apanel_probe':
apanel.c:(.text+0x302ac9): undefined reference to `led_classdev_register'

with this config:

Quoted 3 lines

http://redhat.com/~mingo/misc/config-Wed_Apr_30_20_22_20_CEST_2008.bad

this driver has a missing CONFIG_NEW_LEDS dependency.

Thank you Ingo. Isn't this a Kconfig issue though? I know that Roman is not willing to fix SELECT to evaluate the whole dependency chain, but in this case it is "depens on" that does not evaluate properly...

Quoted 3 lines

Thank you Ingo. Isn't this a Kconfig issue though? I know that Roman is not willing to fix SELECT to evaluate the whole dependency chain, but in this case it is "depens on" that does not evaluate properly...

yes and no. It can be fixed in Kconfig but i'm not going to flame yet another person about long-existing unfixed infrastructure bugs ;-)

Hi,

Quoted 4 lines · Ingo Molnar

Quoted 3 lines

Thank you Ingo. Isn't this a Kconfig issue though? I know that Roman is not willing to fix SELECT to evaluate the whole dependency chain, but in this case it is "depens on" that does not evaluate properly...

Quoted 2 lines

yes and no. It can be fixed in Kconfig but i'm not going to flame yet another person about long-existing unfixed infrastructure bugs ;-)

You don't have to, because this simply is no infrastructure bug... Don't blame Kconfig for abusing select for something it was never intendend for. There is no bug!

In this case there is now a mixture of depends and selects on leds. First decide whether led support should be automatically included or the driver is only available when led support is enabled. In the first case it's probably better to create LEDS_SUPPORT, which then again selects everything necessary.

Quoted 11 lines · Dmitry Torokhov

Quoted 10 lines · Ingo Molnar

x86.git testing found this build failure in upstream -git:

drivers/built-in.o: In function `apanel_detach_client':
apanel.c:(.text+0x3027af): undefined reference to led_classdev_unregister' drivers/built-in.o: In function apanel_probe':
apanel.c:(.text+0x302ac9): undefined reference to `led_classdev_register'

with this config:

Quoted 7 lines

Quoted 3 lines

http://redhat.com/~mingo/misc/config-Wed_Apr_30_20_22_20_CEST_2008.bad

this driver has a missing CONFIG_NEW_LEDS dependency.

Thank you Ingo. Isn't this a Kconfig issue though? I know that Roman is not willing to fix SELECT to evaluate the whole dependency chain, but in this case it is "depens on" that does not evaluate properly...

yes and no. It can be fixed in Kconfig but i'm not going to flame yet another person about long-existing unfixed infrastructure bugs ;-)

The current (many years old) rule is to fix things up as much as we can and never expose build errors, because 1) randconfig testing is way too useful and because 2) users can stumble across that .config combination as well and get rightfully grumpy at us about breaking their build. Most of the thousands of drivers are fine - it's those with more complex Kconfig dependencies that have trouble.

I thought the many years old rule for Linux kernel is to fix the issues properly and not turn kernel into patchwork of crappy code.

If we want to fix the randconfi issue I'd rather revert commit 66242f7ec531953fbc2f4040c5ffe1f1ffe6c5c9 for now (they only thing it did was beautify menuconfig look) and wait for Kconfig to be fixed. It is not driver's task to track through all dependencies.

Quoted 2 lines · Dmitry Torokhov

I thought the many years old rule for Linux kernel is to fix the issues properly and not turn kernel into patchwork of crappy code.

correct, except if that would mean breakage for users.

Quoted 4 lines

If we want to fix the randconfi issue I'd rather revert commit 66242f7ec531953fbc2f4040c5ffe1f1ffe6c5c9 for now (they only thing it did was beautify menuconfig look) and wait for Kconfig to be fixed. It is not driver's task to track through all dependencies.

yep ... just dont export our internal crappiness to testers and users.

i'm not suggesting this is your fault in any way - but nevertheless many other subsystems have to deal with the same Kconfig issues and they manage to limp along.

Quoted 3 lines

i'm not suggesting this is your fault in any way - but nevertheless many other subsystems have to deal with the same Kconfig issues and they manage to limp along.

I believe I see a steady stream of breakage for leds dependencies from all subsystems.

Quoted 4 lines · Dmitry Torokhov

Quoted 3 lines

i'm not suggesting this is your fault in any way - but nevertheless many other subsystems have to deal with the same Kconfig issues and they manage to limp along.

Quoted 2 lines

I believe I see a steady stream of breakage for leds dependencies from all subsystems.

they limp along by adding "depends on NEW_LEDS". 99% of the users will use some pre-cooked distro kernel where all these options are turned on, so the flattening and coupling of the dependencies is not a real issue in practice.

You still did not answer to the main question - do you think we should revert the commit that actually introduced breakage in the sense that anything depending on LEDS_CLASS should also add NEW_LEDS dependancy? That will take care of the problem (as far as LEDs are concerned) for _all_ subsystems and drivers at once.

Hi,

Quoted 5 lines · Dmitry Torokhov

You still did not answer to the main question - do you think we should revert the commit that actually introduced breakage in the sense that anything depending on LEDS_CLASS should also add NEW_LEDS dependancy? That will take care of the problem (as far as LEDs are concerned) for _all_ subsystems and drivers at once.

Well, that would mean reverting the complete led subsystem, since the menuconfig patch you pointed at, isn't really the problem (did you actually try to revert that patch?).

Anyway, the actual issue are all the drivers that select LEDS_CLASS without selecting NEW_LEDS.

The patch below fixes the dependency problem without adding a lot of new selects by using LEDS_CORE, which is selected by LEDS_CLASS/LEDS_TRIGGERS as needed.
I removed the comments as they were a little redundant and I removed the selects of NEW_LEDS as these are not needed anymore.

Signed-off-by Roman Zippel <[email protected]>

BTW, does this mean that all 'if'-guarded symbols should be converted to this style?

Quoted 7 lines · Ingo Molnar

Quoted 4 lines · Dmitry Torokhov

Quoted 3 lines

i'm not suggesting this is your fault in any way - but nevertheless many other subsystems have to deal with the same Kconfig issues and they manage to limp along.

Quoted 7 lines

Quoted 2 lines

I believe I see a steady stream of breakage for leds dependencies from all subsystems.

they limp along by adding "depends on NEW_LEDS". 99% of the users will use some pre-cooked distro kernel where all these options are turned on, so the flattening and coupling of the dependencies is not a real issue in practice.

You still did not answer to the main question - do you think we should revert the commit that actually introduced breakage in the sense that anything depending on LEDS_CLASS should also add NEW_LEDS dependancy? That will take care of the problem (as far as LEDs are concerned) for _all_ subsystems and drivers at once.