On Tue, Jul 01, 2025 at 09:00:34AM GMT, Lukas Wunner wrote:
> On Tue, Jul 01, 2025 at 12:17:31PM +0530, Manivannan Sadhasivam wrote:
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -2508,6 +2508,7 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
> > }
> > EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
> >
> > +#if IS_ENABLED(CONFIG_PCI_PWRCTRL)
> > static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> > {
>
> Hm, why does pci_pwrctrl_create_device() return a pointer, even though the
> sole caller doesn't make any use of it? Why not return a negative errno?
>
> Then you could just do this:
>
> if (!IS_ENABLED(CONFIG_PCI_PWRCTRL))
> return 0;
>
> ... at the top of the function and you don't need the extra LoC for the
> empty inline stub.
>
This is what I initially submitted [1] though that returned NULL, but the idea
was the same. But Bjorn didn't like that.
> Another option is to set "struct pci_dev *pdev = NULL;" and #ifdef the body
> of the function, save for the "return pdev;" at the bottom.
>
This is similar to what Bjorn submitted [2], but you were in favor of providing
a stub instead [3]. It also looked better to my eyes.
> Of course you could also do:
>
> if (!IS_ENABLED(CONFIG_PCI_PWRCTRL))
> return NULL;
>
> ... at the top of the function, but again, the caller doesn't make any
> use of the returned pointer.
>
Right. I could make it to return a errno, but that's not the scope of this
patch. Bjorn wanted to have the #ifdef to be guarded to make the compiled out
part more visible [4], so I ended up with this version.
But whatever the style is, we should make sure that the patch lands in 6.16-rcS.
It is taking more time than needed.
- Mani
[1] https://lore.kernel.org/all/20250522140326.93869-1-manivannan.sadhasivam@xxxxxxxxxx/
[2] https://lore.kernel.org/linux-pci/20250523201935.1586198-1-helgaas@xxxxxxxxxx/
[3] https://lore.kernel.org/linux-pci/aDFnWhFa9ZGqr67T@xxxxxxxxx/
[4] https://lore.kernel.org/linux-pci/20250629190219.GA1717534@bhelgaas/
> Thanks,
>
> Lukas
>
--
மணிவண்ணன் சதாசிவம்
