On Sun, Jun 15, 2025 at 12:25:38AM -0700, Sathyanarayanan Kuppuswamy wrote:
>
> On 6/14/25 4:20 AM, Manivannan Sadhasivam wrote:
> > pci_pwrctrl_create_device() is a PWRCTRL framework API. So it should be
> > built only when CONFIG_PWRCTRL is enabled. Currently, it is built
> > independently of CONFIG_PWRCTRL. This creates enumeration failure on
> > platforms like brcmstb using out-of-tree devicetree that describes the
> > power supplies for endpoints in the PCIe child node, but doesn't use
> > PWRCTRL framework to manage the supplies. The controller driver itself
> > manages the supplies.
> >
> > But in any case, the API should be built only when CONFIG_PWRCTRL is
> > enabled. So move its definition to drivers/pci/pwrctrl/core.c and provide
> > a stub in drivers/pci/pci.h when CONFIG_PWRCTRL is not enabled. This also
> > fixes the enumeration issues on the affected platforms.
> >
> > Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
> > Reported-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx>
> > Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@xxxxxxxxxxxxxx
> > Suggested-by: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > Signed-off-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>
> > ---
> >
> > Changes in v2:
> >
> > * Dropped the unused headers from probe.c (Lukas)
> >
> > drivers/pci/pci.h | 8 ++++++++
> > drivers/pci/probe.c | 32 --------------------------------
> > drivers/pci/pwrctrl/core.c | 36 ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 44 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 12215ee72afb..c5efd8b9c96a 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -1159,4 +1159,12 @@ static inline int pci_msix_write_tph_tag(struct pci_dev *pdev, unsigned int inde
> > (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
> > PCI_CONF1_EXT_REG(reg))
> > +#ifdef CONFIG_PCI_PWRCTRL
>
> Use IS_ENABLED?
>
Yes, thanks for pointing it out. I should've never used ifdef here.
- Mani
> > +struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus,
> > + int devfn);
> > +#else
> > +static inline struct platform_device *
> > +pci_pwrctrl_create_device(struct pci_bus *bus, int devfn) { return NULL; }
> > +#endif
> > +
> > #endif /* DRIVERS_PCI_H */
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 4b8693ec9e4c..478e217928a6 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -9,8 +9,6 @@
> > #include <linux/pci.h>
> > #include <linux/msi.h>
> > #include <linux/of_pci.h>
> > -#include <linux/of_platform.h>
> > -#include <linux/platform_device.h>
> > #include <linux/pci_hotplug.h>
> > #include <linux/slab.h>
> > #include <linux/module.h>
> > @@ -2508,36 +2506,6 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
> > }
> > EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
> > -static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> > -{
> > - struct pci_host_bridge *host = pci_find_host_bridge(bus);
> > - struct platform_device *pdev;
> > - struct device_node *np;
> > -
> > - np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> > - if (!np || of_find_device_by_node(np))
> > - return NULL;
> > -
> > - /*
> > - * First check whether the pwrctrl device really needs to be created or
> > - * not. This is decided based on at least one of the power supplies
> > - * being defined in the devicetree node of the device.
> > - */
> > - if (!of_pci_supply_present(np)) {
> > - pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
> > - return NULL;
> > - }
> > -
> > - /* Now create the pwrctrl device */
> > - pdev = of_platform_device_create(np, NULL, &host->dev);
> > - if (!pdev) {
> > - pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
> > - return NULL;
> > - }
> > -
> > - return pdev;
> > -}
> > -
> > /*
> > * Read the config data for a PCI device, sanity-check it,
> > * and fill in the dev structure.
> > diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> > index 6bdbfed584d6..20585b2c3681 100644
> > --- a/drivers/pci/pwrctrl/core.c
> > +++ b/drivers/pci/pwrctrl/core.c
> > @@ -6,11 +6,47 @@
> > #include <linux/device.h>
> > #include <linux/export.h>
> > #include <linux/kernel.h>
> > +#include <linux/of.h>
> > +#include <linux/of_pci.h>
> > +#include <linux/of_platform.h>
> > #include <linux/pci.h>
> > #include <linux/pci-pwrctrl.h>
> > +#include <linux/platform_device.h>
> > #include <linux/property.h>
> > #include <linux/slab.h>
> > +#include "../pci.h"
> > +
> > +struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
> > +{
> > + struct pci_host_bridge *host = pci_find_host_bridge(bus);
> > + struct platform_device *pdev;
> > + struct device_node *np;
> > +
> > + np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> > + if (!np || of_find_device_by_node(np))
> > + return NULL;
> > +
> > + /*
> > + * First check whether the pwrctrl device really needs to be created or
> > + * not. This is decided based on at least one of the power supplies
> > + * being defined in the devicetree node of the device.
> > + */
> > + if (!of_pci_supply_present(np)) {
> > + pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
> > + return NULL;
> > + }
> > +
> > + /* Now create the pwrctrl device */
> > + pdev = of_platform_device_create(np, NULL, &host->dev);
> > + if (!pdev) {
> > + pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
> > + return NULL;
> > + }
> > +
> > + return pdev;
> > +}
> > +
> > static int pci_pwrctrl_notify(struct notifier_block *nb, unsigned long action,
> > void *data)
> > {
>
> --
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer
>
--
மணிவண்ணன் சதாசிவம்
