Re: [PATCH 1/2] leds: remove led_init_default_state_get() and devm_led_classdev_register_ext() stubs

[Heiner Kallweit]

On 09.01.2024 10:06, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
> 
> These two functions have stub implementations that are called when
> NEW_LEDS and/or LEDS_CLASS are disabled, theorerically allowing drivers
> to optionally use the LED subsystem.
> 
> However, this has never really worked because a built-in driver is
> unable to link against these functions if the LED class is in a loadable
> module. Heiner ran into this problem with a driver that newly gained
> a LEDS_CLASS dependency and suggested using an IS_REACHABLE() check.
> 
> This is the reverse approach, removing the stub entirely to acknowledge
> that it is pointless in its current form, and that not having it avoids
> misleading developers into thinking that they can rely on it.
> 
> This survived around 1000 randconfig builds to validate that any callers
> of the interface already have the correct Kconfig dependency already,
> with the exception of the one that Heiner just added.
> 
> Cc: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> Link: https://lore.kernel.org/linux-leds/0f6f432b-c650-4bb8-a1b5-fe3372804d52@xxxxxxxxx/T/#u
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---

For r8169 we have a Kconfig-based solution now, right. I had a brief look
at other drivers using LED functionality, and already the first one I looked
at seems to suffer from the same problem. input/keyboard/qt2160.c has the
following what should result in the same link error if qt2160 is built-in
and CONFIG_LEDS_CLASS=m. qt2160 has a Kconfig dependency only on I2C.

#ifdef CONFIG_LEDS_CLASS
static int qt2160_register_leds(struct qt2160_data *qt2160)
{
[...]
	error = devm_led_classdev_register(&client->dev, &led->cdev);
[...]		
}
#else

Personally I don't have a strong opinion pro/con IS_REACHABLE.
I see two options for now:
1. If outcome should be that stubs are a good thing, then the condition
   for the stub should make sure that the stub is chosen when needed.
   This would result in using IS_REACHABLE.
2. If stubs are removed (but also in the current situation, see example),
   then it seems some drivers need adding proper build dependencies.