Re: [PATCH v2] PCI/pwrctrl: Skip creating pwrctrl device unless CONFIG_PCI_PWRCTRL is enabled

[Manivannan Sadhasivam]

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
> 

-- 
மணிவண்ணன் சதாசிவம்