Messages in this thread | | | Date | Tue, 9 Jan 2024 11:10:08 +0100 | Subject | Re: [PATCH 1/2] leds: remove led_init_default_state_get() and devm_led_classdev_register_ext() stubs | From | Heiner Kallweit <> |
| |
On 09.01.2024 10:06, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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@gmail.com> > Link: https://lore.kernel.org/linux-leds/0f6f432b-c650-4bb8-a1b5-fe3372804d52@gmail.com/T/#u > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > ---
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.
| |