[PATCH v2] of: property: Create devlink between PCI Host bridge and Root Port supplies

[PATCH v2] of: property: Create devlink between PCI Host bridge and Root Port supplies

[Manivannan Sadhasivam]

Recently, devicetree started to represent the PCI Host bridge supplies like
PHY in the Root Port nodes as seen in commit 38fcbfbd4207
("dt-bindings: PCI: qcom: Move PHY & reset GPIO to Root Port node"). But
the Host bridge drivers still control the Root Port supplies as a part of
their controller initialization/deinitialization sequence.

So the Host bridge drivers end up parsing the Root Port supplies in their
probe() and control them. A downside to this approach is that the devlink
dependency between the suppliers and Host bridge is completely broken. Due
to this, the driver core probes the Host bridge drivers even if the
supplies are not ready, causing probe deferrals and setup teardowns in
Host bridge probe().

These probe deferrals sometime happen over 1000 times (as reported in Qcom
Glymur platform) leading to a waste of CPU resources and increase in boot
time. So to fix these unnecessary deferrals, create devlink between the
Host bridge and Root Port supplies in of_fwnode_add_links(). This will
allow the driver core to probe the Host bridge drivers only when all Root
Port supplies are available.

Reported-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---

Changes in v2:

* Reworded the commit message slightly
* Dropped the "linux,pci-domain" property and used PCI node parent check to
  identify the Host bridge node as suggested by Rob.

 drivers/of/property.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 136946f8b746..6aa1d3fc2165 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1561,6 +1561,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 /**
  * of_link_property - Create device links to suppliers listed in a property
  * @con_np: The consumer device tree node which contains the property
+ * @parent_np: Optional parent device tree node requiring child's supplies
  * @prop_name: Name of property to be parsed
  *
  * This function checks if the property @prop_name that is present in the
@@ -1577,7 +1578,8 @@ static const struct supplier_bindings of_supplier_bindings[] = {
  * device tree nodes even when attempts to create a link to one or more
  * suppliers fail.
  */
-static int of_link_property(struct device_node *con_np, const char *prop_name)
+static int of_link_property(struct device_node *con_np, struct device_node *parent_np,
+			    const char *prop_name)
 {
 	struct device_node *phandle;
 	const struct supplier_bindings *s = of_supplier_bindings;
@@ -1598,6 +1600,10 @@ static int of_link_property(struct device_node *con_np, const char *prop_name)
 			matched = true;
 			i++;
 			of_link_to_phandle(con_dev_np, phandle, s->fwlink_flags);
+
+			/* Link the child's supplies to parent if needed */
+			if (parent_np)
+				of_link_to_phandle(parent_np, phandle, s->fwlink_flags);
 			of_node_put(phandle);
 		}
 		s++;
@@ -1656,7 +1662,26 @@ static int of_fwnode_add_links(struct fwnode_handle *fwnode)
 		return -EINVAL;
 
 	for_each_property_of_node(con_np, p)
-		of_link_property(con_np, p->name);
+		of_link_property(con_np, NULL, p->name);
+
+	/*
+	 * Since the host bridge drivers parse and control the Root Port
+	 * supplies, create a devlink between host bridge and Root Port
+	 * supplies. This will prevent the host bridge drivers from being
+	 * probed before the supplies become available.
+	 *
+	 * For checking the host bridge node, first ensure that it is a PCI node
+	 * and its parent is not a PCI node. Only host bridge nodes will have
+	 * this structure.
+	 */
+	if (of_node_is_type(con_np, "pci") && !of_node_is_type(con_np->parent, "pci")) {
+		for_each_available_child_of_node_scoped(con_np, child) {
+			if (of_node_is_type(child, "pci")) {
+				for_each_property_of_node(child, p)
+					of_link_property(child, con_np, p->name);
+			}
+		}
+	}
 
 	return 0;
 }
-- 
2.51.0

Re: [PATCH v2] of: property: Create devlink between PCI Host bridge and Root Port supplies

[Konrad Dybcio]

On 5/6/26 9:56 AM, Manivannan Sadhasivam wrote:
> Recently, devicetree started to represent the PCI Host bridge supplies like
> PHY in the Root Port nodes as seen in commit 38fcbfbd4207
> ("dt-bindings: PCI: qcom: Move PHY & reset GPIO to Root Port node"). But
> the Host bridge drivers still control the Root Port supplies as a part of
> their controller initialization/deinitialization sequence.
> 
> So the Host bridge drivers end up parsing the Root Port supplies in their
> probe() and control them. A downside to this approach is that the devlink
> dependency between the suppliers and Host bridge is completely broken. Due
> to this, the driver core probes the Host bridge drivers even if the
> supplies are not ready, causing probe deferrals and setup teardowns in
> Host bridge probe().
> 
> These probe deferrals sometime happen over 1000 times (as reported in Qcom
> Glymur platform) leading to a waste of CPU resources and increase in boot
> time. So to fix these unnecessary deferrals, create devlink between the
> Host bridge and Root Port supplies in of_fwnode_add_links(). This will
> allow the driver core to probe the Host bridge drivers only when all Root
> Port supplies are available.
> 
> Reported-by: Bjorn Andersson <andersson@kernel.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

[...]

> +	/*
> +	 * Since the host bridge drivers parse and control the Root Port
> +	 * supplies, create a devlink between host bridge and Root Port
> +	 * supplies. This will prevent the host bridge drivers from being
> +	 * probed before the supplies become available.
> +	 *
> +	 * For checking the host bridge node, first ensure that it is a PCI node
> +	 * and its parent is not a PCI node. Only host bridge nodes will have
> +	 * this structure.
> +	 */
> +	if (of_node_is_type(con_np, "pci") && !of_node_is_type(con_np->parent, "pci")) {
> +		for_each_available_child_of_node_scoped(con_np, child) {
> +			if (of_node_is_type(child, "pci")) {

Doesn't this go a level too deep now? (a change vs v1)

for example, in x1e80100.dtsi:

con_np = &pcie3_port0
con_np->parent = &pcie3

And I think the code now looks for device(such that device_type="pci")
*under* &pcie3_port0

Konrad

> +				for_each_property_of_node(child, p)
> +					of_link_property(child, con_np, p->name);
> +			}
> +		}
> +	}
>  
>  	return 0;
>  }