Message ID | 20240904104848.23480-2-mariusz.tkaczyk@linux.intel.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 78efa53e715e21a97c722dba20f8437a0860521e |
Headers | show |
Series | PCIe Enclosure LED Management | expand |
On Wed, 04 Sep 2024, Mariusz Tkaczyk wrote: > NPEM driver will require leds class, there is an init-order conflict. > Make sure that LEDs initialization happens first and add comment. > > Suggested-by: Dan Williams <dan.j.williams@intel.com> Do you have a link to this discussion? > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> > --- > drivers/Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/Makefile b/drivers/Makefile > index fe9ceb0d2288..45d1c3e630f7 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -17,6 +17,9 @@ obj-$(CONFIG_PINCTRL) += pinctrl/ > obj-$(CONFIG_GPIOLIB) += gpio/ > obj-y += pwm/ > > +# LEDs must come before PCI, it is needed by NPEM driver This seems very fragile. Isn't there a better way to describe the dependency in Kconfig etc? > +obj-y += leds/ > + > obj-y += pci/ > > obj-$(CONFIG_PARISC) += parisc/ > @@ -130,7 +133,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/ > -- > 2.35.3 >
On Mon, 9 Sep 2024 10:03:40 +0100 Lee Jones <lee@kernel.org> wrote: > On Wed, 04 Sep 2024, Mariusz Tkaczyk wrote: > > > NPEM driver will require leds class, there is an init-order conflict. > > Make sure that LEDs initialization happens first and add comment. > > > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > > Do you have a link to this discussion? https://lore.kernel.org/linux-pci/2024061429-handiness-waggle-d87a@gregkh/ I discussed this with Lukas and Dan offline and as a result I updated Makefile instead initcall change. > > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> > > --- > > drivers/Makefile | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/Makefile b/drivers/Makefile > > index fe9ceb0d2288..45d1c3e630f7 100644 > > --- a/drivers/Makefile > > +++ b/drivers/Makefile > > @@ -17,6 +17,9 @@ obj-$(CONFIG_PINCTRL) += pinctrl/ > > obj-$(CONFIG_GPIOLIB) += gpio/ > > obj-y += pwm/ > > > > +# LEDs must come before PCI, it is needed by NPEM driver > > This seems very fragile. > > Isn't there a better way to describe the dependency in Kconfig etc? I don't know if it is better. What I know is (according to Greg) that maintainers are aware of possible ordering issues and not described dependencies so they are not likely to allow changing it. Mariusz
On Mon, Sep 09, 2024 at 10:03:40AM +0100, Lee Jones wrote: > On Wed, 04 Sep 2024, Mariusz Tkaczyk wrote: > > NPEM driver will require leds class, there is an init-order conflict. > > Make sure that LEDs initialization happens first and add comment. [...] > > --- a/drivers/Makefile > > +++ b/drivers/Makefile > > @@ -17,6 +17,9 @@ obj-$(CONFIG_PINCTRL) += pinctrl/ > > obj-$(CONFIG_GPIOLIB) += gpio/ > > obj-y += pwm/ > > > > +# LEDs must come before PCI, it is needed by NPEM driver > > +obj-y += leds/ > > + > > obj-y += pci/ > > > > obj-$(CONFIG_PARISC) += parisc/ > > @@ -130,7 +133,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/ > > This seems very fragile. > > Isn't there a better way to describe the dependency in Kconfig etc? In v2 of this series, the correct init order was ensured by running leds_init() at "arch" initcall level, instead of "subsys": https://lore.kernel.org/linux-pci/20240528131940.16924-2-mariusz.tkaczyk@linux.intel.com/ If you prefer this alternative approach, then Bjorn or Krzysztof can probably replace the commit on the pci.git/npem branch. AFAIK, all topic branches in pci.git are to be considered drafts that can be amended until the pull request is sent. Thanks, Lukas
diff --git a/drivers/Makefile b/drivers/Makefile index fe9ceb0d2288..45d1c3e630f7 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -17,6 +17,9 @@ obj-$(CONFIG_PINCTRL) += pinctrl/ obj-$(CONFIG_GPIOLIB) += gpio/ obj-y += pwm/ +# LEDs must come before PCI, it is needed by NPEM driver +obj-y += leds/ + obj-y += pci/ obj-$(CONFIG_PARISC) += parisc/ @@ -130,7 +133,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/