[PATCH v4 0/2] PCI: of: update endpoint ranges dynamically

[PATCH v4 0/2] PCI: of: update endpoint ranges dynamically

[Alex Elder]

This series fixes a bug in PCI devicetree handling code.

If a PCI endpoint function is represented in devicetree, and it
implements a PCI endpoint bus, its ranges property must be created
dynamically.  This is because the PCI function's BAR addresses are
assigned during PCI enumeration, and a static ranges property cannot
assume a particular static BAR address matches what was assigned.

The solution is to dynamically generate a ranges property for PCI
functions defined in devicetree that implement a PCI endpoint bus.

					-Alex

Note: this series depends on the patch "PCI: of: drop the reg_num
argument to of_pci_set_address()" found in this series:
  https://lore.kernel.org/lkml/20261105153801.435566-5-elder@riscstar.com/

The entire series (based on v7.4-rc1 and including that series as
prerequisite) is available here:
  https://github.com/riscstar/linux/tree/outgoing/dynamic_ranges-v4

Version 3 is available here:
  https://lore.kernel.org/lkml/20260924222444.1351466-1-elder@riscstar.com/

Between version 3 and version 4:
- Dramatically simplified the description in this cover letter.  See
  earlier versions for a more lengthy explanation.
- Dropped the first patch, which allowed a null pointer for the flags
  argument to of_pci_get_addr_flags().  This wasn't really necessary
  and didn't really add value.
- Indicated clearly that Andrea della Porta has tested this series on
  the Raspberry Pi 5, which uses PCI endpoint bus for its RP1 chip.
  The current statically-defined ranges property works for this system,
  and when the necessary PCI quirk is enabled, the dynamically generated
  ranges property works correctly as well.

Between version 2 and version 3:
- The ranges property for a PCI endpoint with an existing devicetree node
  is only updated if its ranges property has an empty value (i.e., it is
  "ranges;", or is not defined)

Version 2 is available here:
  https://lore.kernel.org/lkml/20260910021919.3421449-1-elder@riscstar.com/

Between version 1 and version 2:
- Included the first patch (which was previously posted in a different
  series)
- Modified the last patch so the ranges property is updated only for
  PCI endpoints having at least one "pci-ep-bus" node
- Rebased on v7.3-rc2 (and the prerequisite series)

Version 1 is available here:
  https://lore.kernel.org/lkml/20260813220717.1394644-1-elder@riscstar.com/



Alex Elder (2):
  PCI: of: introduce of_pci_build_prop_ranges()
  PCI: of: introduce of_pci_update_endpoint_node_ranges()

 drivers/pci/of.c          |  97 ++++++++++++++++++++++++++---
 drivers/pci/of_property.c | 126 +++++++++++++++++++++++++-------------
 drivers/pci/pci.h         |   1 +
 3 files changed, 173 insertions(+), 51 deletions(-)


base-commit: 8c786803613eb0d3352417509611d1db877c65af
-- 
2.53.0

[PATCH v4 1/2] PCI: of: introduce of_pci_build_prop_ranges()

[Alex Elder]

Move the bulk of what happens inside of_pci_prop_ranges() into
a helper function that builds up the value to be passed to
of_changeset_add_prop_u32_array().

This helper will be used in the next commit to build the ranges
property for a statically-defined PCI endpoint devicetree node
when it has a "pci-ep-bus" sub-node.

Tested-by: Andrea della Porta <andrea.porta@suse.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/pci/of_property.c | 126 +++++++++++++++++++++++++-------------
 1 file changed, 83 insertions(+), 43 deletions(-)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 702675672fbc5..b202f45b3660b 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -8,6 +8,10 @@
 #include <linux/of_irq.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+#include <linux/wordpart.h>
+
 #include "pci.h"
 
 #define OF_PCI_ADDRESS_CELLS		3
@@ -104,60 +108,96 @@ static int of_pci_prop_bus_range(struct pci_dev *pdev,
 					       ARRAY_SIZE(bus_range));
 }
 
-static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs,
-			      struct device_node *np)
+/*
+ * Build a "ranges" property value that defines the mapping between
+ * child and parent PCI address space for each of the given PCI
+ * device's resources.
+ *
+ * Returns a dynamically allocated array of u32 devicetree cells, or
+ * a null pointer if allocation fails.  The cell array is built as a
+ * of_pci_prop_ranges structure consisting of 8 32-bit cells in host
+ * byte order.  It is suitable for use as the value of a PCI device
+ * node "ranges" property passed to of_changeset_add_prop_u32_array().
+ * The total number of cells in the array is returned in *count.
+ *
+ * Caller is responsible for ensuring the returned pointer gets freed.
+ */
+static u32 *of_pci_build_prop_ranges(struct pci_dev *pdev, u32 *count)
 {
-	struct of_pci_range_entry *rp;
+	bool bridge_device = pci_is_bridge(pdev);
+	struct of_pci_range_entry *entries;
+	struct of_pci_range_entry *ep;
+	u32 resource_count = 0;
 	struct resource *res;
-	int i, j, ret;
 	u32 flags, num;
-	u64 val64;
+	u32 first;
+	u32 i;
 
-	if (pci_is_bridge(pdev)) {
+	if (bridge_device) {
+		first = PCI_BRIDGE_RESOURCES;
 		num = PCI_BRIDGE_RESOURCE_NUM;
-		res = &pdev->resource[PCI_BRIDGE_RESOURCES];
 	} else {
+		first = PCI_STD_RESOURCES;
 		num = PCI_STD_NUM_BARS;
-		res = &pdev->resource[PCI_STD_RESOURCES];
 	}
 
-	rp = kzalloc_objs(*rp, num);
-	if (!rp)
+	/* First count how many resources will get a range property */
+	res = pci_resource_n(pdev, first);
+	for (i = 0; i < num; i++, res++)
+		if (resource_size(res) && !of_pci_get_addr_flags(res, &flags))
+			resource_count++;
+
+	entries = kzalloc_objs(*entries, resource_count);
+	if (!entries)
+		return NULL;
+
+	ep = entries;
+	res = pci_resource_n(pdev, first);
+	for (i = 0; i < num; i++, res++) {
+		u64 size = resource_size(res);
+
+		if (!size || of_pci_get_addr_flags(res, &flags))
+			continue;
+
+		/* Record the size in the range entry */
+		ep->size[0] = upper_32_bits(size);
+		ep->size[1] = lower_32_bits(size);
+
+		/* Record the parent bus address from the resource */
+		of_pci_set_address(pdev, ep->parent_addr,
+				   pci_bus_address(pdev, first + i),
+				   flags, false);
+
+		/*
+		 * For a bridge device, the child address matches the
+		 * parent address (including its flags cell).  For an
+		 * endpoint device, the (flags) cell contains the BAR
+		 * number, and the two address cells are zero.
+		 */
+		if (bridge_device)
+			memcpy(ep->child_addr, ep->parent_addr,
+			       sizeof(ep->child_addr));
+		else
+			ep->child_addr[0] = i;
+
+		ep++;
+	}
+	*count = resource_count * sizeof(*ep) / sizeof(u32);
+
+	return (u32 *)entries;
+}
+
+static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs,
+			      struct device_node *np)
+{
+	u32 *value;
+	u32 count;
+
+	value = of_pci_build_prop_ranges(pdev, &count);
+	if (!value)
 		return -ENOMEM;
 
-	for (i = 0, j = 0; j < num; j++) {
-		if (!resource_size(&res[j]))
-			continue;
-
-		if (of_pci_get_addr_flags(&res[j], &flags))
-			continue;
-
-		val64 = pci_bus_address(pdev, &res[j] - pdev->resource);
-		of_pci_set_address(pdev, rp[i].parent_addr, val64, flags,
-				   false);
-		if (pci_is_bridge(pdev)) {
-			memcpy(rp[i].child_addr, rp[i].parent_addr,
-			       sizeof(rp[i].child_addr));
-		} else {
-			/*
-			 * For endpoint device, the lower 64-bits of child
-			 * address is always zero.
-			 */
-			rp[i].child_addr[0] = j;
-		}
-
-		val64 = resource_size(&res[j]);
-		rp[i].size[0] = upper_32_bits(val64);
-		rp[i].size[1] = lower_32_bits(val64);
-
-		i++;
-	}
-
-	ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", (u32 *)rp,
-					      i * sizeof(*rp) / sizeof(u32));
-	kfree(rp);
-
-	return ret;
+	return of_changeset_add_prop_u32_array(ocs, np, "ranges", value, count);
 }
 
 static int of_pci_prop_reg(struct pci_dev *pdev, struct of_changeset *ocs,
-- 
2.53.0

[PATCH v4 2/2] PCI: of: introduce of_pci_update_endpoint_node_ranges()

[Alex Elder]

Commit 407d1a51921e9 ("PCI: Create device tree node for bridge")
introduced the PCI_DYNAMIC_OF_NODES Kconfig option, which creates
a devicetree node for a PCI bridge as part of pci_bus_add_device().
Its successor commit ae9813db1dc5a ("PCI: Add quirks to generate
device tree node for Xilinx Alveo U50") shows how to use a PCI final
fixup quirk to also create a devicetree node for a non-bridge PCI
device.  In both cases, of_pci_make_dev_node() uses an OF changeset
to dynamically create a node populated with appropriate properties
and apply it to the live devicetree.

The dynamic devicetree node for a PCI device will include a "ranges"
property, and a new type of 3-cell address is introduced for use
within an endpoint.  The endpoint's ranges property will contain a
range entry for each of the endpoint's BARs.  The "child address"
portion of each range will use the BAR number in the "flags" (first)
cell in the address.  This allows addresses within the endpoint to
be expressed relative to whatever address gets assigned to each BAR.

Unfortunately, if a PCI endpoint device had a devicetree node set
up statically, its "ranges" property (if present) will be static,
and it cannot contain the addresses assigned to the endpoint's BARs
during enumeration.

This means that the "BAR number" based addressing scheme doesn't
work for PCI endpoints whose devicetree nodes are created statically.

To remedy this, modify of_pci_make_dev_node() to dynamically create
a "ranges" property even if the device already has a devicetree
node--just as is done when the endpoint has none.  A few conditions:
  - A PCI bridge node's ranges property is never updated
  - If a PCI endpoint node defines a ranges property with a
    non-empty value (i.e., it's not just "ranges;"), that
    ranges property is preserved

Otherwise a new ranges property is created using assigned addresses,
and it replaces (or adds) that property to the PCI endpoint node.

As a result, "BAR number" addresses work correctly even when the
endpoint's devicetree node is created statically.  This has been
verified by Andrea della Porta (with both static and dynamic ranges
properties) on the Raspberry Pi 5 RP1.

Tested-by: Andrea della Porta <andrea.porta@suse.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
 drivers/pci/of.c          | 97 +++++++++++++++++++++++++++++++++++----
 drivers/pci/of_property.c |  2 +-
 drivers/pci/pci.h         |  1 +
 3 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 5a040ed836744..fa9e9377c3eb9 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -17,6 +17,9 @@
 #include <linux/of_pci.h>
 #include <linux/platform_device.h>
 #include <linux/pm_wakeirq.h>
+
+#include <asm/byteorder.h>
+
 #include "pci.h"
 
 #ifdef CONFIG_PCI
@@ -742,20 +745,98 @@ void of_pci_remove_node(struct pci_dev *pdev)
 	of_node_put(np);
 }
 
+/* Returns true if the ranges property was added or updated successfully */
+static bool of_pci_update_endpoint_node_ranges(struct pci_dev *pdev)
+{
+	struct device_node *np = pci_device_to_OF_node(pdev);
+	struct property *prop;
+	u32 *value;
+	u32 size;
+
+	prop = kzalloc_obj(*prop);
+	if (!prop)
+		return false;
+
+	value = of_pci_build_prop_ranges(pdev, &size);
+	if (!value) {
+		kfree(prop);
+		return false;
+	}
+
+	prop->name = "ranges";
+	prop->length = size * sizeof(u32);
+	prop->value = value;
+
+	/* The property value needs to be in big-endian byte order */
+	while (size--)
+		cpu_to_be32s(value++);
+
+	/* of_update_property() consumes the allocated property */
+	of_update_property(np, prop);
+
+	return true;
+}
+
+/*
+ * Create a devicetree node for a PCI device.  If the device is a bridge
+ * and it already has a devicetree node, there's nothing further to do.
+ * If it is a bridge without an existing devicetree node, one is created
+ * dynamically.
+ *
+ * This function can also be called (via PCI quirk) for a PCI endpoint
+ * (function) that implements a PCI endpoint bus.  As with a PCI bridge,
+ * if the endpoint has no existing devicetree node, one is created
+ * dynamically.  The node will include a ranges property that maps
+ * BAR-relative addresses in the child to the PCI address ranges
+ * assigned to the PCI endpoint BARs.
+ *
+ * If an endpoint already has a devicetree node, and it includes a
+ * "pci-ep-bus" sub-node, its ranges property must still be dynamically
+ * populated so that it can take into account the BAR ranges assigned
+ * during PCI enumeration.
+ */
 void of_pci_make_dev_node(struct pci_dev *pdev)
 {
-	struct device_node *ppnode, *np = NULL;
+	struct device_node *np = pci_device_to_OF_node(pdev);
+	struct device *dev = &pdev->dev;
+	struct device_node *ppnode;
+	struct of_changeset *cset;
 	const char *pci_type;
-	struct of_changeset *cset;
 	const char *name;
 	int ret;
 
-	/*
-	 * If there is already a device tree node linked to this device,
-	 * return immediately.
-	 */
-	if (pci_device_to_OF_node(pdev))
+	/* See if the PCI device already has a devicetree node */
+	if (np) {
+		struct device_node *child;
+		unsigned int rlen = 0;
+
+		/* Nothing further needed for a bridge */
+		if (pci_is_bridge(pdev))
+			return;
+
+		/*
+		 * A ranges property is only needed if the endpoint's
+		 * devicetree node includes a "pci-ep-bus" sub-node.
+		 */
+		child = of_get_child_by_name(np, "pci-ep-bus");
+		if (!child)
+			return;
+		of_node_put(child);
+
+		/* If the ranges property is non-empty, just keep it.*/
+		if (of_get_property(np, "ranges", &rlen) && rlen)
+			return;
+
+		/*
+		 * Otherwise create a ranges property, defining an entry for
+		 * each BAR, and map BAR offsets to the PCI bus address based
+		 * on the BAR's assigned range.
+		 */
+		if (!of_pci_update_endpoint_node_ranges(pdev))
+			dev_err(dev, "failed to update ranges property\n");
+
 		return;
+	}
 
 	/* Check if there is device tree node for parent device */
 	if (!pdev->bus->self)
@@ -794,7 +875,7 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 
 	np->data = cset;
 
-	ret = device_add_of_node(&pdev->dev, np);
+	ret = device_add_of_node(dev, np);
 	if (ret)
 		goto out_revert_cset;
 
diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index b202f45b3660b..44e169f4fbf4d 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -122,7 +122,7 @@ static int of_pci_prop_bus_range(struct pci_dev *pdev,
  *
  * Caller is responsible for ensuring the returned pointer gets freed.
  */
-static u32 *of_pci_build_prop_ranges(struct pci_dev *pdev, u32 *count)
+u32 *of_pci_build_prop_ranges(struct pci_dev *pdev, u32 *count)
 {
 	bool bridge_device = pci_is_bridge(pdev);
 	struct of_pci_range_entry *entries;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e93f602b9ca48..af21a46cfdb3c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1324,6 +1324,7 @@ struct of_changeset;
 #ifdef CONFIG_PCI_DYNAMIC_OF_NODES
 void of_pci_make_dev_node(struct pci_dev *pdev);
 void of_pci_remove_node(struct pci_dev *pdev);
+u32 *of_pci_build_prop_ranges(struct pci_dev *pdev, u32 *count);
 int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
 			  struct device_node *np);
 void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge);
-- 
2.53.0