* [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device() definition to drivers/pci/pwrctrl/
@ 2025-06-14 11:20 Manivannan Sadhasivam
2025-06-15 7:25 ` Sathyanarayanan Kuppuswamy
0 siblings, 1 reply; 3+ messages in thread
From: Manivannan Sadhasivam @ 2025-06-14 11:20 UTC (permalink / raw)
To: bhelgaas, brgl
Cc: linux-pci, linux-kernel, lukas, Manivannan Sadhasivam,
Jim Quinlan, Bjorn Helgaas
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@broadcom.com>
Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
---
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
+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)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device() definition to drivers/pci/pwrctrl/
2025-06-14 11:20 [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device() definition to drivers/pci/pwrctrl/ Manivannan Sadhasivam
@ 2025-06-15 7:25 ` Sathyanarayanan Kuppuswamy
2025-06-16 4:47 ` Manivannan Sadhasivam
0 siblings, 1 reply; 3+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-06-15 7:25 UTC (permalink / raw)
To: Manivannan Sadhasivam, bhelgaas, brgl
Cc: linux-pci, linux-kernel, lukas, Jim Quinlan, Bjorn Helgaas
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@broadcom.com>
> Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
>
> 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?
> +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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device() definition to drivers/pci/pwrctrl/
2025-06-15 7:25 ` Sathyanarayanan Kuppuswamy
@ 2025-06-16 4:47 ` Manivannan Sadhasivam
0 siblings, 0 replies; 3+ messages in thread
From: Manivannan Sadhasivam @ 2025-06-16 4:47 UTC (permalink / raw)
To: Sathyanarayanan Kuppuswamy
Cc: bhelgaas, brgl, linux-pci, linux-kernel, lukas, Jim Quinlan,
Bjorn Helgaas
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@broadcom.com>
> > Closes: https://lore.kernel.org/r/CA+-6iNwgaByXEYD3j=-+H_PKAxXRU78svPMRHDKKci8AGXAUPg@mail.gmail.com
> > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> > Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
> > ---
> >
> > 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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-16 4:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-14 11:20 [PATCH v2] PCI/pwrctrl: Move pci_pwrctrl_create_device() definition to drivers/pci/pwrctrl/ Manivannan Sadhasivam
2025-06-15 7:25 ` Sathyanarayanan Kuppuswamy
2025-06-16 4:47 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).