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

Greg Kroah-Hartman
2024-06-14T14:13:35Z

On Fri, Jun 14, 2024 at 04:08:48PM +0200, Lukas Wunner wrote:
> 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.

If they do that, then we have worse problems as many of us "know" that
the order here matters a LOT.

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

For a while, until they change :)

> 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 */

That would be good to have.

thanks,

greg k-h