diff mbox series

[v2,2/5] PCI/pwrctl: Create pwrctl devices only if at least one power supply is present

Message ID 20241025-pci-pwrctl-rework-v2-2-568756156cbe@linaro.org
State New
Headers show
Series PCI/pwrctl: Ensure that the pwrctl drivers are probed before PCI client drivers | expand

Commit Message

Manivannan Sadhasivam via B4 Relay Oct. 25, 2024, 7:54 a.m. UTC 
From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Currently, pwrctl devices are created if the corresponding PCI nodes are
defined in devicetree. But this is not correct, because not all PCI nodes
defined in devicetree require pwrctl support. Pwrctl comes into picture
only when the device requires kernel to manage its power state. This can
be determined using the power supply properties present in the devicetree
node of the device.

So add a new API, of_pci_is_supply_present() that checks the devicetree
node if at least one power supply property is present or not. If present,
then the pwrctl device will be created for that PCI node. Otherwise, it
will be skipped.

Cc: stable+noautosel@kernel.org # Depends on of_platform_device_create() rework
Fixes: 8fb18619d910 ("PCI/pwrctl: Create platform devices for child OF nodes of the port node")
Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
Tested-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/bus.c | 11 +++++++++++
 drivers/pci/of.c  | 27 +++++++++++++++++++++++++++
 drivers/pci/pci.h |  5 +++++
 3 files changed, 43 insertions(+)

Comments

Bjorn Helgaas Nov. 6, 2024, 9:28 p.m. UTC | #1 
On Fri, Oct 25, 2024 at 01:24:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Currently, pwrctl devices are created if the corresponding PCI nodes are
> defined in devicetree. But this is not correct, because not all PCI nodes
> defined in devicetree require pwrctl support. Pwrctl comes into picture
> only when the device requires kernel to manage its power state. This can
> be determined using the power supply properties present in the devicetree
> node of the device.

> +bool of_pci_is_supply_present(struct device_node *np)
> +{
> +	struct property *prop;
> +	char *supply;
> +
> +	if (!np)
> +		return false;

Why do we need to test !np here?  It should always be non-NULL.

> +	for_each_property_of_node(np, prop) {
> +		supply = strrchr(prop->name, '-');
> +		if (supply && !strcmp(supply, "-supply"))
> +			return true;
> +	}
> +
> +	return false;
> +}
Bartosz Golaszewski Nov. 7, 2024, 9:52 a.m. UTC | #2 
On Wed, Nov 6, 2024 at 10:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Fri, Oct 25, 2024 at 01:24:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > Currently, pwrctl devices are created if the corresponding PCI nodes are
> > defined in devicetree. But this is not correct, because not all PCI nodes
> > defined in devicetree require pwrctl support. Pwrctl comes into picture
> > only when the device requires kernel to manage its power state. This can
> > be determined using the power supply properties present in the devicetree
> > node of the device.
>
> > +bool of_pci_is_supply_present(struct device_node *np)
> > +{
> > +     struct property *prop;
> > +     char *supply;
> > +
> > +     if (!np)
> > +             return false;
>
> Why do we need to test !np here?  It should always be non-NULL.
>

Right, I think this can be dropped. We check for the OF node in the
function above.

Bart
Manivannan Sadhasivam Nov. 7, 2024, 11:15 a.m. UTC | #3 
On Thu, Nov 07, 2024 at 10:52:35AM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 6, 2024 at 10:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Fri, Oct 25, 2024 at 01:24:52PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > >
> > > Currently, pwrctl devices are created if the corresponding PCI nodes are
> > > defined in devicetree. But this is not correct, because not all PCI nodes
> > > defined in devicetree require pwrctl support. Pwrctl comes into picture
> > > only when the device requires kernel to manage its power state. This can
> > > be determined using the power supply properties present in the devicetree
> > > node of the device.
> >
> > > +bool of_pci_is_supply_present(struct device_node *np)
> > > +{
> > > +     struct property *prop;
> > > +     char *supply;
> > > +
> > > +     if (!np)
> > > +             return false;
> >
> > Why do we need to test !np here?  It should always be non-NULL.
> >
> 
> Right, I think this can be dropped. We check for the OF node in the
> function above.
> 

I think it was a leftover that I didn't cleanup. But I do plan to move this API
to drivers/of once 6.13-rc1 is out. So even if it didn't get dropped now, I will
do it later.

- Mani
diff mbox series

Patch

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 9d278d3a19ff..02a492aa5f17 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -354,6 +354,17 @@  void pci_bus_add_device(struct pci_dev *dev)
 
 	if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) {
 		for_each_available_child_of_node_scoped(dn, child) {
+			/*
+			 * First check whether the pwrctl device 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_is_supply_present(child)) {
+				pci_dbg(dev, "skipping OF node: %s\n", child->name);
+				continue;
+			}
+
 			pdev = of_platform_device_create(child, NULL, &dev->dev);
 			if (!pdev)
 				pci_err(dev, "failed to create OF node: %s\n", child->name);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index dacea3fc5128..1f6a15a35a82 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -728,6 +728,33 @@  void of_pci_make_dev_node(struct pci_dev *pdev)
 }
 #endif
 
+/**
+ * of_pci_is_supply_present() - Check if the power supply is present for the PCI
+ *				device
+ * @np: Device tree node
+ *
+ * Check if the power supply for the PCI device is present in the device tree
+ * node or not.
+ *
+ * Return: true if at least one power supply exists; false otherwise.
+ */
+bool of_pci_is_supply_present(struct device_node *np)
+{
+	struct property *prop;
+	char *supply;
+
+	if (!np)
+		return false;
+
+	for_each_property_of_node(np, prop) {
+		supply = strrchr(prop->name, '-');
+		if (supply && !strcmp(supply, "-supply"))
+			return true;
+	}
+
+	return false;
+}
+
 #endif /* CONFIG_PCI */
 
 /**
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 14d00ce45bfa..c8081b427267 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -746,6 +746,7 @@  void pci_set_bus_of_node(struct pci_bus *bus);
 void pci_release_bus_of_node(struct pci_bus *bus);
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge);
+bool of_pci_is_supply_present(struct device_node *np);
 
 #else
 static inline int
@@ -793,6 +794,10 @@  static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br
 	return 0;
 }
 
+static inline bool of_pci_is_supply_present(struct device_node *np);
+{
+	return false;
+}
 #endif /* CONFIG_OF */
 
 struct of_changeset;