Re: [PATCH v2 1/3] leds: Init leds class earlier

[Lukas Wunner <lukas@wunner.de>]

On Tue, May 28, 2024 at 09:22:22PM -0700, Dan Williams wrote:
> Mariusz Tkaczyk wrote:
> > PCI subsystem is registered on subsys_initcall() same as leds class.
> > NPEM driver will require leds class, there is race possibility.
> > Register led class on arch_initcall() to avoid it.
> 
> Another way to solve this without changing initcall levels is to just
> make sure that the leds subsys initcall happens first, i.e.:
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index fe9ceb0d2288..d47b4186108a 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_GENERIC_PHY)     += phy/
>  obj-$(CONFIG_PINCTRL)          += pinctrl/
>  obj-$(CONFIG_GPIOLIB)          += gpio/
>  obj-y                          += pwm/
> +obj-y                          += leds/
>  
>  obj-y                          += pci/
>  
> @@ -130,7 +131,6 @@ obj-$(CONFIG_CPU_IDLE)              += cpuidle/
>  obj-y                          += mmc/
>  obj-y                          += ufs/
>  obj-$(CONFIG_MEMSTICK)         += memstick/
> -obj-y                          += leds/
>  obj-$(CONFIG_INFINIBAND)       += infiniband/
>  obj-y                          += firmware/
>  obj-$(CONFIG_CRYPTO)           += crypto/

To me, this seems more fragile than changing the initcall level.

The risk I see is that someone comes along and rearranges the Makefile in,
say, alphabetic order because "why not?" and unwittingly breaks NPEM.

Changing initcall levels at least explicitly sets the order in stone.

However, perhaps a code comment is helpful to tell the casual
code reader why this particular initcall level is needed.

Something like...

/* LEDs may already be registered in subsys initcall level */

... may already be sufficient.

Thanks,

Lukas