[PATCH v8 0/5] Add support for the PCI host bridge device-tree node creation.

[PATCH v8 0/5] Add support for the PCI host bridge device-tree node creation.

[Herve Codina]

Hi,

This series adds support for creating a device-tree node for the PCI
host bridge on non device-tree based system.

Creating device-tree nodes for PCI devices and PCI-PCI bridges already
exists upstream. It was added in commit 407d1a51921e ("PCI: Create
device tree node for bridge"). Created device-tree nodes need a parent
node to be attached to. For the first level devices, on device-tree
based system, this parent node (i.e. the PCI host bridge) is described
in the base device-tree. The PCI bus related to this bridge (PCI root
bus) inherit of the PCI host bridge device-tree node.

The LAN966x PCI device driver, available since commit 185686beb464
("misc: Add support for LAN966x PCI device"), relies on this feature.

On system where the base hardware is not described by a device-tree, the
PCI host bridge to which first level created PCI devices need to be
attach to does not exist. This is the case for instance on ACPI
described systems such as x86.

This series goal is to handle this case.

In order to have the PCI host bridge device-tree node available even
on x86, this top level node is created (if not already present) based on
information computed by the PCI core. It follows the same mechanism as
the one used for PCI devices device-tree node creation.

As for device-tree based system, the PCI root bus handled by the PCI
host bridge inherit of this created node.

In order to have this feature available, a number of changes are needed:
  - Patch 1 and 2: Introduce and use device_{add,remove}_of_node().
    This function will also be used in the root PCI bus node creation.

  - Patch 3 and 4: Improve existing functions to reuse them in the root
    PCI bus node creation.

  - Patch 5: The PCI host bridge device-tree node creation itself.

With those modifications, the LAN966x PCI device is working on x86 systems
and all device-tree kunit tests (including the of_unittest_pci_node test)
pass successfully with the following command:
  qemu-system-x86_64 -machine q35 -nographic \
    -kernel arch/x86_64/boot/bzImage --append console=ttyS0 \
    -device pcie-root-port,port=0x10,chassis=9,id=pci.9,bus=pcie.0,multifunction=on,addr=0x3 \
    -device pcie-root-port,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0x3.0x1 \
    -device x3130-upstream,id=pci.11,bus=pci.9,addr=0x0 \
    -device xio3130-downstream,port=0x0,chassis=11,id=pci.12,bus=pci.11,multifunction=on,addr=0x0 \
    -device i82801b11-bridge,id=pci.13,bus=pcie.0,addr=0x4 \
    -device pci-bridge,chassis_nr=14,id=pci.14,bus=pci.13,addr=0x0 \
    -device pci-testdev,bus=pci.12,addr=0x0

Compare to previous iteration, this v8 series mainly improves a commit
log.

Best regards,
Hervé Codina

Changes v7 -> v8
  v7: https://lore.kernel.org/lkml/20250204073501.278248-1-herve.codina@bootlin.com/

  - Patch 1
    Add 'Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>'
    Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'

  - Patch 2, 3, 4
    Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'

  - Patch 5
    Improve commit log adding a high level part outlining the benefit of
    changes.
    Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'

Changes v6 -> v7
  v6: https://lore.kernel.org/lkml/20250110082143.917590-1-herve.codina@bootlin.com/

  Rebase on top of v6.14-rc1

Changes v5 -> v6
  v5: https://lore.kernel.org/lkml/20241209130339.81354-1-herve.codina@bootlin.com/

  - Patch 1
    Add a return error code in device_add_of_node()

  - Patches 2 and 5
    Handle the device_add_of_node() error code

  - Patches 3 and 4
    No changes

Changes v4 -> v5
  v4: https://lore.kernel.org/lkml/20241202131522.142268-1-herve.codina@bootlin.com/

  - Patch 1
    Use dev_warn() instead of WARN()

  - Patches 2 to 4
    No changes

  - Patch 5 (v4 patch 6)
    Use dev_err()
    Fix a typo in commit log

  Patch removed in v5
    - Patch 5 in v4
      Already applied

Changes v3 -> v4
  v3: https://lore.kernel.org/lkml/20241114165446.611458-1-herve.codina@bootlin.com/

  Rebase on top of v6.13-rc1

  - Patches 1 to 6
    No changes

Changes v2 -> v3
  v2: https://lore.kernel.org/lkml/20241108143600.756224-1-herve.codina@bootlin.com/

  - Patch 5
    Fix commit log.
    Use 2 for #size-cells.

  - Patches 1 to 4 and 6
    No changes

Changes v1 -> v2
  v1: https://lore.kernel.org/lkml/20241104172001.165640-1-herve.codina@bootlin.com/

  - Patch 1
    Remove Cc: stable

  - Patch 2
    Remove Fixup tag and Cc: stable

  - Patches 3 and 4
    No changes

  - Patch 5
    Add #address-cells/#size-cells in the empty root DT node instead of
    updating default values for x86.
    Update commit log and commit title.

  - Patch 6
    Create device-tree node for the PCI host bridge and reuse it for
    the PCI root bus. Rename functions accordingly.
    Use "pci" instead of "pci-root" for the PCI host bridge node name.
    Use "res->start - windows->offset" for the PCI bus addresses.
    Update commit log and commit title.

Herve Codina (5):
  driver core: Introduce device_{add,remove}_of_node()
  PCI: of: Use device_{add,remove}_of_node() to attach of_node to
    existing device
  PCI: of_property: Add support for NULL pdev in of_pci_set_address()
  PCI: of_property: Constify parameter in of_pci_get_addr_flags()
  PCI: of: Create device-tree PCI host bridge node

 drivers/base/core.c       |  61 ++++++++++++++++++++
 drivers/pci/of.c          | 115 +++++++++++++++++++++++++++++++++++++-
 drivers/pci/of_property.c | 114 +++++++++++++++++++++++++++++++++++--
 drivers/pci/pci.h         |   6 ++
 drivers/pci/probe.c       |   2 +
 drivers/pci/remove.c      |   2 +
 include/linux/device.h    |   2 +
 7 files changed, 295 insertions(+), 7 deletions(-)

-- 
2.48.1

[PATCH v8 1/5] driver core: Introduce device_{add,remove}_of_node()

[Herve Codina]

An of_node can be set to a device using device_set_node().
This function cannot prevent any of_node and/or fwnode overwrites.

When adding an of_node on an already present device, the following
operations need to be done:
- Attach the of_node if no of_node were already attached
- Attach the of_node as a fwnode if no fwnode were already attached

This is the purpose of device_add_of_node().
device_remove_of_node() reverts the operations done by
device_add_of_node().

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/base/core.c    | 61 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 63 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5a1f05198114..d1b044af64de 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5170,6 +5170,67 @@ void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(set_secondary_fwnode);
 
+/**
+ * device_remove_of_node - Remove an of_node from a device
+ * @dev: device whose device-tree node is being removed
+ */
+void device_remove_of_node(struct device *dev)
+{
+	dev = get_device(dev);
+	if (!dev)
+		return;
+
+	if (!dev->of_node)
+		goto end;
+
+	if (dev->fwnode == of_fwnode_handle(dev->of_node))
+		dev->fwnode = NULL;
+
+	of_node_put(dev->of_node);
+	dev->of_node = NULL;
+
+end:
+	put_device(dev);
+}
+EXPORT_SYMBOL_GPL(device_remove_of_node);
+
+/**
+ * device_add_of_node - Add an of_node to an existing device
+ * @dev: device whose device-tree node is being added
+ * @of_node: of_node to add
+ *
+ * Return: 0 on success or error code on failure.
+ */
+int device_add_of_node(struct device *dev, struct device_node *of_node)
+{
+	int ret;
+
+	if (!of_node)
+		return -EINVAL;
+
+	dev = get_device(dev);
+	if (!dev)
+		return -EINVAL;
+
+	if (dev->of_node) {
+		dev_err(dev, "Cannot replace node %pOF with %pOF\n",
+			dev->of_node, of_node);
+		ret = -EBUSY;
+		goto end;
+	}
+
+	dev->of_node = of_node_get(of_node);
+
+	if (!dev->fwnode)
+		dev->fwnode = of_fwnode_handle(of_node);
+
+	ret = 0;
+end:
+	put_device(dev);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(device_add_of_node);
+
 /**
  * device_set_of_node_from_dev - reuse device-tree node of another device
  * @dev: device whose device-tree node is being set
diff --git a/include/linux/device.h b/include/linux/device.h
index 80a5b3268986..1244e5892292 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1191,6 +1191,8 @@ int device_online(struct device *dev);
 void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
 void set_secondary_fwnode(struct device *dev, struct fwnode_handle *fwnode);
 void device_set_node(struct device *dev, struct fwnode_handle *fwnode);
+int device_add_of_node(struct device *dev, struct device_node *of_node);
+void device_remove_of_node(struct device *dev);
 void device_set_of_node_from_dev(struct device *dev, const struct device *dev2);
 
 static inline struct device_node *dev_of_node(struct device *dev)
-- 
2.48.1

[PATCH v8 2/5] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Herve Codina]

The commit 407d1a51921e ("PCI: Create device tree node for bridge")
creates of_node for PCI devices. The newly created of_node is attached
to an existing device. This is done setting directly pdev->dev.of_node
in the code.

Even if pdev->dev.of_node cannot be previously set, this doesn't handle
the fwnode field of the struct device. Indeed, this field needs to be
set if it hasn't already been set.

device_{add,remove}_of_node() have been introduced to handle this case.

Use them instead of the direct setting.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/pci/of.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 7a806f5c0d20..fb5e6da1ead0 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -653,8 +653,8 @@ void of_pci_remove_node(struct pci_dev *pdev)
 	np = pci_device_to_OF_node(pdev);
 	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
 		return;
-	pdev->dev.of_node = NULL;
 
+	device_remove_of_node(&pdev->dev);
 	of_changeset_revert(np->data);
 	of_changeset_destroy(np->data);
 	of_node_put(np);
@@ -711,11 +711,18 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 		goto out_free_node;
 
 	np->data = cset;
-	pdev->dev.of_node = np;
+
+	ret = device_add_of_node(&pdev->dev, np);
+	if (ret)
+		goto out_revert_cset;
+
 	kfree(name);
 
 	return;
 
+out_revert_cset:
+	np->data = NULL;
+	of_changeset_revert(cset);
 out_free_node:
 	of_node_put(np);
 out_destroy_cset:
-- 
2.48.1

[PATCH v8 3/5] PCI: of_property: Add support for NULL pdev in of_pci_set_address()

[Herve Codina]

The pdev (pointer to a struct pci_dev) parameter of of_pci_set_address()
cannot be NULL.

In order to reuse of_pci_set_address() when creating the PCI root bus
node, this function needs to support a NULL pdev parameter. Indeed, in
the case of the PCI root bus node creation, no pdev are available and
of_pci_set_address() will be used with the bridge windows.

Allow to call of_pci_set_address() with a NULL pdev.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/pci/of_property.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 58fbafac7c6a..962d2e8cc095 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -54,9 +54,13 @@ enum of_pci_prop_compatible {
 static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
 			       u32 reg_num, u32 flags, bool reloc)
 {
-	prop[0] = FIELD_PREP(OF_PCI_ADDR_FIELD_BUS, pdev->bus->number) |
-		FIELD_PREP(OF_PCI_ADDR_FIELD_DEV, PCI_SLOT(pdev->devfn)) |
-		FIELD_PREP(OF_PCI_ADDR_FIELD_FUNC, PCI_FUNC(pdev->devfn));
+	if (pdev)
+		prop[0] = FIELD_PREP(OF_PCI_ADDR_FIELD_BUS, pdev->bus->number) |
+			  FIELD_PREP(OF_PCI_ADDR_FIELD_DEV, PCI_SLOT(pdev->devfn)) |
+			  FIELD_PREP(OF_PCI_ADDR_FIELD_FUNC, PCI_FUNC(pdev->devfn));
+	else
+		prop[0] = 0;
+
 	prop[0] |= flags | reg_num;
 	if (!reloc) {
 		prop[0] |= OF_PCI_ADDR_FIELD_NONRELOC;
-- 
2.48.1

[PATCH v8 4/5] PCI: of_property: Constify parameter in of_pci_get_addr_flags()

[Herve Codina]

The res parameter has no reason to be a pointer to an un-const struct
resource. Indeed, struct resource is not supposed to be modified by the
function.

Constify the res parameter.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/pci/of_property.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 962d2e8cc095..b17dcb04de18 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -69,7 +69,7 @@ static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
 	}
 }
 
-static int of_pci_get_addr_flags(struct resource *res, u32 *flags)
+static int of_pci_get_addr_flags(const struct resource *res, u32 *flags)
 {
 	u32 ss;
 
-- 
2.48.1

[PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Herve Codina]

PCI devices device-tree nodes can be already created. This was
introduced by commit 407d1a51921e ("PCI: Create device tree node for
bridge").

In order to have device-tree nodes related to PCI devices attached on
their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
root bus device-tree node is needed. This root bus node will be used as
the parent node of the first level devices scanned on the bus. On
device-tree based systems, this PCI root bus device tree node is set to
the node of the related PCI host bridge. The PCI host bridge node is
available in the device-tree used to describe the hardware passed at
boot.

On non device-tree based system (such as ACPI), a device-tree node for
the PCI host bridge or for the root bus does not exist. Indeed, the PCI
host bridge is not described in a device-tree used at boot simply
because no device-tree are passed at boot.

The device-tree PCI host bridge node creation needs to be done at
runtime. This is done in the same way as for the creation of the PCI
device nodes. I.e. node and properties are created based on computed
information done by the PCI core. Also, as is done on device-tree based
systems, this PCI host bridge node is used for the PCI root bus.

With this done, hardware available in a PCI device that doesn't follow
the PCI model consisting in one PCI function handled by one driver can
be described by a device-tree overlay loaded by the PCI device driver on
non device-tree based systems. Those PCI devices provide a single PCI
function that includes several functionalities that require different
driver. The device-tree overlay describes in that case the internal
devices and their relationships. It allows to load drivers needed by
those different devices in order to have functionalities handled.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 drivers/pci/of.c          | 104 +++++++++++++++++++++++++++++++++++++-
 drivers/pci/of_property.c | 102 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h         |   6 +++
 drivers/pci/probe.c       |   2 +
 drivers/pci/remove.c      |   2 +
 5 files changed, 215 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index fb5e6da1ead0..c59429e909c0 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -731,7 +731,109 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
 out_free_name:
 	kfree(name);
 }
-#endif
+
+void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
+{
+	struct device_node *np;
+
+	np = pci_bus_to_OF_node(bridge->bus);
+	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
+		return;
+
+	device_remove_of_node(&bridge->bus->dev);
+	device_remove_of_node(&bridge->dev);
+	of_changeset_revert(np->data);
+	of_changeset_destroy(np->data);
+	of_node_put(np);
+}
+
+void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
+{
+	struct device_node *np = NULL;
+	struct of_changeset *cset;
+	const char *name;
+	int ret;
+
+	/*
+	 * If there is already a device-tree node linked to the PCI bus handled
+	 * by this bridge (i.e. the PCI root bus), nothing to do.
+	 */
+	if (pci_bus_to_OF_node(bridge->bus))
+		return;
+
+	/* The root bus has no node. Check that the host bridge has no node too */
+	if (bridge->dev.of_node) {
+		dev_err(&bridge->dev, "PCI host bridge of_node already set");
+		return;
+	}
+
+	/* Check if there is a DT root node to attach the created node */
+	if (!of_root) {
+		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
+		return;
+	}
+
+	name = kasprintf(GFP_KERNEL, "pci@%x,%x", pci_domain_nr(bridge->bus),
+			 bridge->bus->number);
+	if (!name)
+		return;
+
+	cset = kmalloc(sizeof(*cset), GFP_KERNEL);
+	if (!cset)
+		goto out_free_name;
+	of_changeset_init(cset);
+
+	np = of_changeset_create_node(cset, of_root, name);
+	if (!np)
+		goto out_destroy_cset;
+
+	ret = of_pci_add_host_bridge_properties(bridge, cset, np);
+	if (ret)
+		goto out_free_node;
+
+	/*
+	 * This of_node will be added to an existing device. The of_node parent
+	 * is the root OF node and so this node will be handled by the platform
+	 * bus. Avoid any new device creation.
+	 */
+	of_node_set_flag(np, OF_POPULATED);
+	np->fwnode.dev = &bridge->dev;
+	fwnode_dev_initialized(&np->fwnode, true);
+
+	ret = of_changeset_apply(cset);
+	if (ret)
+		goto out_free_node;
+
+	np->data = cset;
+
+	/* Add the of_node to host bridge and the root bus */
+	ret = device_add_of_node(&bridge->dev, np);
+	if (ret)
+		goto out_revert_cset;
+
+	ret = device_add_of_node(&bridge->bus->dev, np);
+	if (ret)
+		goto out_remove_bridge_dev_of_node;
+
+	kfree(name);
+
+	return;
+
+out_remove_bridge_dev_of_node:
+	device_remove_of_node(&bridge->dev);
+out_revert_cset:
+	np->data = NULL;
+	of_changeset_revert(cset);
+out_free_node:
+	of_node_put(np);
+out_destroy_cset:
+	of_changeset_destroy(cset);
+	kfree(cset);
+out_free_name:
+	kfree(name);
+}
+
+#endif /* CONFIG_PCI_DYNAMIC_OF_NODES */
 
 /**
  * of_pci_supply_present() - Check if the power supply is present for the PCI
diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index b17dcb04de18..e0069c9eb23f 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -394,3 +394,105 @@ int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
 
 	return 0;
 }
+
+static bool of_pci_is_range_resource(const struct resource *res, u32 *flags)
+{
+	if (!(resource_type(res) & IORESOURCE_MEM) &&
+	    !(resource_type(res) & IORESOURCE_MEM_64))
+		return false;
+
+	if (of_pci_get_addr_flags(res, flags))
+		return false;
+
+	return true;
+}
+
+static int of_pci_host_bridge_prop_ranges(struct pci_host_bridge *bridge,
+					  struct of_changeset *ocs,
+					  struct device_node *np)
+{
+	struct resource_entry *window;
+	unsigned int ranges_sz = 0;
+	unsigned int n_range = 0;
+	struct resource *res;
+	int n_addr_cells;
+	u32 *ranges;
+	u64 val64;
+	u32 flags;
+	int ret;
+
+	n_addr_cells = of_n_addr_cells(np);
+	if (n_addr_cells <= 0 || n_addr_cells > 2)
+		return -EINVAL;
+
+	resource_list_for_each_entry(window, &bridge->windows) {
+		res = window->res;
+		if (!of_pci_is_range_resource(res, &flags))
+			continue;
+		n_range++;
+	}
+
+	if (!n_range)
+		return 0;
+
+	ranges = kcalloc(n_range,
+			 (OF_PCI_ADDRESS_CELLS + OF_PCI_SIZE_CELLS +
+			  n_addr_cells) * sizeof(*ranges),
+			 GFP_KERNEL);
+	if (!ranges)
+		return -ENOMEM;
+
+	resource_list_for_each_entry(window, &bridge->windows) {
+		res = window->res;
+		if (!of_pci_is_range_resource(res, &flags))
+			continue;
+
+		/* PCI bus address */
+		val64 = res->start;
+		of_pci_set_address(NULL, &ranges[ranges_sz], val64 - window->offset,
+				   0, flags, false);
+		ranges_sz += OF_PCI_ADDRESS_CELLS;
+
+		/* Host bus address */
+		if (n_addr_cells == 2)
+			ranges[ranges_sz++] = upper_32_bits(val64);
+		ranges[ranges_sz++] = lower_32_bits(val64);
+
+		/* Size */
+		val64 = resource_size(res);
+		ranges[ranges_sz] = upper_32_bits(val64);
+		ranges[ranges_sz + 1] = lower_32_bits(val64);
+		ranges_sz += OF_PCI_SIZE_CELLS;
+	}
+
+	ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", ranges, ranges_sz);
+	kfree(ranges);
+	return ret;
+}
+
+int of_pci_add_host_bridge_properties(struct pci_host_bridge *bridge,
+				      struct of_changeset *ocs,
+				      struct device_node *np)
+{
+	int ret;
+
+	ret = of_changeset_add_prop_string(ocs, np, "device_type", "pci");
+	if (ret)
+		return ret;
+
+	ret = of_changeset_add_prop_u32(ocs, np, "#address-cells",
+					OF_PCI_ADDRESS_CELLS);
+	if (ret)
+		return ret;
+
+	ret = of_changeset_add_prop_u32(ocs, np, "#size-cells",
+					OF_PCI_SIZE_CELLS);
+	if (ret)
+		return ret;
+
+	ret = of_pci_host_bridge_prop_ranges(bridge, ocs, np);
+	if (ret)
+		return ret;
+
+	return 0;
+}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 01e51db8d285..f916d0f386a2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -876,9 +876,15 @@ void of_pci_make_dev_node(struct pci_dev *pdev);
 void of_pci_remove_node(struct pci_dev *pdev);
 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);
+void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge);
+int of_pci_add_host_bridge_properties(struct pci_host_bridge *bridge, struct of_changeset *ocs,
+				      struct device_node *np);
 #else
 static inline void of_pci_make_dev_node(struct pci_dev *pdev) { }
 static inline void of_pci_remove_node(struct pci_dev *pdev) { }
+static inline void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge) { }
+static inline void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge) { }
 #endif
 
 #ifdef CONFIG_PCIEAER
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b6536ed599c3..b74a12a0193a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1094,6 +1094,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
 		dev_info(&bus->dev, "root bus resource %pR%s\n", res, addr);
 	}
 
+	of_pci_make_host_bridge_node(bridge);
+
 	down_write(&pci_bus_sem);
 	list_add_tail(&bus->node, &pci_root_buses);
 	up_write(&pci_bus_sem);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index efc37fcb73e2..9f7df2b20183 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -163,6 +163,8 @@ void pci_stop_root_bus(struct pci_bus *bus)
 					 &bus->devices, bus_list)
 		pci_stop_bus_device(child);
 
+	of_pci_remove_host_bridge_node(host_bridge);
+
 	/* stop the host bridge */
 	device_release_driver(&host_bridge->dev);
 }
-- 
2.48.1

Re: [PATCH v8 0/5] Add support for the PCI host bridge device-tree node creation.

[Bjorn Helgaas]

On Mon, Feb 24, 2025 at 03:13:50PM +0100, Herve Codina wrote:
> Hi,
> 
> This series adds support for creating a device-tree node for the PCI
> host bridge on non device-tree based system.
> 
> Creating device-tree nodes for PCI devices and PCI-PCI bridges already
> exists upstream. It was added in commit 407d1a51921e ("PCI: Create
> device tree node for bridge"). Created device-tree nodes need a parent
> node to be attached to. For the first level devices, on device-tree
> based system, this parent node (i.e. the PCI host bridge) is described
> in the base device-tree. The PCI bus related to this bridge (PCI root
> bus) inherit of the PCI host bridge device-tree node.
> 
> The LAN966x PCI device driver, available since commit 185686beb464
> ("misc: Add support for LAN966x PCI device"), relies on this feature.
> 
> On system where the base hardware is not described by a device-tree, the
> PCI host bridge to which first level created PCI devices need to be
> attach to does not exist. This is the case for instance on ACPI
> described systems such as x86.
> 
> This series goal is to handle this case.
> 
> In order to have the PCI host bridge device-tree node available even
> on x86, this top level node is created (if not already present) based on
> information computed by the PCI core. It follows the same mechanism as
> the one used for PCI devices device-tree node creation.
> 
> As for device-tree based system, the PCI root bus handled by the PCI
> host bridge inherit of this created node.
> 
> In order to have this feature available, a number of changes are needed:
>   - Patch 1 and 2: Introduce and use device_{add,remove}_of_node().
>     This function will also be used in the root PCI bus node creation.
> 
>   - Patch 3 and 4: Improve existing functions to reuse them in the root
>     PCI bus node creation.
> 
>   - Patch 5: The PCI host bridge device-tree node creation itself.
> 
> With those modifications, the LAN966x PCI device is working on x86 systems
> and all device-tree kunit tests (including the of_unittest_pci_node test)
> pass successfully with the following command:
>   qemu-system-x86_64 -machine q35 -nographic \
>     -kernel arch/x86_64/boot/bzImage --append console=ttyS0 \
>     -device pcie-root-port,port=0x10,chassis=9,id=pci.9,bus=pcie.0,multifunction=on,addr=0x3 \
>     -device pcie-root-port,port=0x11,chassis=10,id=pci.10,bus=pcie.0,addr=0x3.0x1 \
>     -device x3130-upstream,id=pci.11,bus=pci.9,addr=0x0 \
>     -device xio3130-downstream,port=0x0,chassis=11,id=pci.12,bus=pci.11,multifunction=on,addr=0x0 \
>     -device i82801b11-bridge,id=pci.13,bus=pcie.0,addr=0x4 \
>     -device pci-bridge,chassis_nr=14,id=pci.14,bus=pci.13,addr=0x0 \
>     -device pci-testdev,bus=pci.12,addr=0x0
> 
> Compare to previous iteration, this v8 series mainly improves a commit
> log.
> 
> Best regards,
> Hervé Codina
> 
> Changes v7 -> v8
>   v7: https://lore.kernel.org/lkml/20250204073501.278248-1-herve.codina@bootlin.com/
> 
>   - Patch 1
>     Add 'Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>'
>     Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'
> 
>   - Patch 2, 3, 4
>     Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'
> 
>   - Patch 5
>     Improve commit log adding a high level part outlining the benefit of
>     changes.
>     Add 'Reviewed-by: Rob Herring (Arm) <robh@kernel.org>'
> 
> Changes v6 -> v7
>   v6: https://lore.kernel.org/lkml/20250110082143.917590-1-herve.codina@bootlin.com/
> 
>   Rebase on top of v6.14-rc1
> 
> Changes v5 -> v6
>   v5: https://lore.kernel.org/lkml/20241209130339.81354-1-herve.codina@bootlin.com/
> 
>   - Patch 1
>     Add a return error code in device_add_of_node()
> 
>   - Patches 2 and 5
>     Handle the device_add_of_node() error code
> 
>   - Patches 3 and 4
>     No changes
> 
> Changes v4 -> v5
>   v4: https://lore.kernel.org/lkml/20241202131522.142268-1-herve.codina@bootlin.com/
> 
>   - Patch 1
>     Use dev_warn() instead of WARN()
> 
>   - Patches 2 to 4
>     No changes
> 
>   - Patch 5 (v4 patch 6)
>     Use dev_err()
>     Fix a typo in commit log
> 
>   Patch removed in v5
>     - Patch 5 in v4
>       Already applied
> 
> Changes v3 -> v4
>   v3: https://lore.kernel.org/lkml/20241114165446.611458-1-herve.codina@bootlin.com/
> 
>   Rebase on top of v6.13-rc1
> 
>   - Patches 1 to 6
>     No changes
> 
> Changes v2 -> v3
>   v2: https://lore.kernel.org/lkml/20241108143600.756224-1-herve.codina@bootlin.com/
> 
>   - Patch 5
>     Fix commit log.
>     Use 2 for #size-cells.
> 
>   - Patches 1 to 4 and 6
>     No changes
> 
> Changes v1 -> v2
>   v1: https://lore.kernel.org/lkml/20241104172001.165640-1-herve.codina@bootlin.com/
> 
>   - Patch 1
>     Remove Cc: stable
> 
>   - Patch 2
>     Remove Fixup tag and Cc: stable
> 
>   - Patches 3 and 4
>     No changes
> 
>   - Patch 5
>     Add #address-cells/#size-cells in the empty root DT node instead of
>     updating default values for x86.
>     Update commit log and commit title.
> 
>   - Patch 6
>     Create device-tree node for the PCI host bridge and reuse it for
>     the PCI root bus. Rename functions accordingly.
>     Use "pci" instead of "pci-root" for the PCI host bridge node name.
>     Use "res->start - windows->offset" for the PCI bus addresses.
>     Update commit log and commit title.
> 
> Herve Codina (5):
>   driver core: Introduce device_{add,remove}_of_node()
>   PCI: of: Use device_{add,remove}_of_node() to attach of_node to
>     existing device
>   PCI: of_property: Add support for NULL pdev in of_pci_set_address()
>   PCI: of_property: Constify parameter in of_pci_get_addr_flags()
>   PCI: of: Create device-tree PCI host bridge node
> 
>  drivers/base/core.c       |  61 ++++++++++++++++++++
>  drivers/pci/of.c          | 115 +++++++++++++++++++++++++++++++++++++-
>  drivers/pci/of_property.c | 114 +++++++++++++++++++++++++++++++++++--
>  drivers/pci/pci.h         |   6 ++
>  drivers/pci/probe.c       |   2 +
>  drivers/pci/remove.c      |   2 +
>  include/linux/device.h    |   2 +
>  7 files changed, 295 insertions(+), 7 deletions(-)

Applied to pci/devtree-create, planning for v6.15, thanks!

Re: [PATCH v8 2/5] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Bjorn Helgaas]

On Mon, Feb 24, 2025 at 03:13:52PM +0100, Herve Codina wrote:
> The commit 407d1a51921e ("PCI: Create device tree node for bridge")
> creates of_node for PCI devices. The newly created of_node is attached
> to an existing device. This is done setting directly pdev->dev.of_node
> in the code.
> 
> Even if pdev->dev.of_node cannot be previously set, this doesn't handle
> the fwnode field of the struct device. Indeed, this field needs to be
> set if it hasn't already been set.
> 
> device_{add,remove}_of_node() have been introduced to handle this case.

I guess another way to say this is:

  - If dev->of_node has already been set, it is an error and we want
    to do nothing.  The error is impossible in this case because
    of_pci_make_dev_node() returns early if dev->of_node has been set.

  - Otherwise, we want to set dev->of_node (just as we previously
    did), and

  - if dev->fwnode has not been set, we want to set that too.

So the whole point of this is to set dev->fwnode, which we didn't do
before.  But has np->fwnode been set to anything?  Maybe it's buried
somewhere inside of_changeset_create_node(), but I didn't see it.

I can't tell if this actually *does* anything.  And if it does, all
the above is basically C code translated into English, so it doesn't
tell us *why* this is useful, which is what I try to put in the merge
commit log.

> Use them instead of the direct setting.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  drivers/pci/of.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 7a806f5c0d20..fb5e6da1ead0 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -653,8 +653,8 @@ void of_pci_remove_node(struct pci_dev *pdev)
>  	np = pci_device_to_OF_node(pdev);
>  	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
>  		return;
> -	pdev->dev.of_node = NULL;
>  
> +	device_remove_of_node(&pdev->dev);
>  	of_changeset_revert(np->data);
>  	of_changeset_destroy(np->data);
>  	of_node_put(np);
> @@ -711,11 +711,18 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>  		goto out_free_node;
>  
>  	np->data = cset;
> -	pdev->dev.of_node = np;
> +
> +	ret = device_add_of_node(&pdev->dev, np);
> +	if (ret)
> +		goto out_revert_cset;
> +
>  	kfree(name);
>  
>  	return;
>  
> +out_revert_cset:
> +	np->data = NULL;
> +	of_changeset_revert(cset);
>  out_free_node:
>  	of_node_put(np);
>  out_destroy_cset:
> -- 
> 2.48.1
> 

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Bjorn Helgaas]

On Mon, Feb 24, 2025 at 03:13:55PM +0100, Herve Codina wrote:
> PCI devices device-tree nodes can be already created. This was
> introduced by commit 407d1a51921e ("PCI: Create device tree node for
> bridge").
> 
> In order to have device-tree nodes related to PCI devices attached on
> their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
> root bus device-tree node is needed. This root bus node will be used as
> the parent node of the first level devices scanned on the bus. On
> device-tree based systems, this PCI root bus device tree node is set to
> the node of the related PCI host bridge. The PCI host bridge node is
> available in the device-tree used to describe the hardware passed at
> boot.
> 
> On non device-tree based system (such as ACPI), a device-tree node for
> the PCI host bridge or for the root bus does not exist. Indeed, the PCI
> host bridge is not described in a device-tree used at boot simply
> because no device-tree are passed at boot.
> 
> The device-tree PCI host bridge node creation needs to be done at
> runtime. This is done in the same way as for the creation of the PCI
> device nodes. I.e. node and properties are created based on computed
> information done by the PCI core. Also, as is done on device-tree based
> systems, this PCI host bridge node is used for the PCI root bus.
> 
> With this done, hardware available in a PCI device that doesn't follow
> the PCI model consisting in one PCI function handled by one driver can
> be described by a device-tree overlay loaded by the PCI device driver on
> non device-tree based systems. Those PCI devices provide a single PCI
> function that includes several functionalities that require different
> driver. The device-tree overlay describes in that case the internal
> devices and their relationships. It allows to load drivers needed by
> those different devices in order to have functionalities handled.

Since this adds host bridge nodes, does this patch specifically enable
device tree overlays for devices on the root bus?

Were we able to load DT overlays for devices deeper in the hierarchy
already, even without this patch?

Bjorn

Re: [PATCH v8 2/5] PCI: of: Use device_{add,remove}_of_node() to attach of_node to existing device

[Herve Codina]

On Fri, 28 Feb 2025 14:58:55 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Mon, Feb 24, 2025 at 03:13:52PM +0100, Herve Codina wrote:
> > The commit 407d1a51921e ("PCI: Create device tree node for bridge")
> > creates of_node for PCI devices. The newly created of_node is attached
> > to an existing device. This is done setting directly pdev->dev.of_node
> > in the code.
> > 
> > Even if pdev->dev.of_node cannot be previously set, this doesn't handle
> > the fwnode field of the struct device. Indeed, this field needs to be
> > set if it hasn't already been set.
> > 
> > device_{add,remove}_of_node() have been introduced to handle this case.  
> 
> I guess another way to say this is:
> 
>   - If dev->of_node has already been set, it is an error and we want
>     to do nothing.  The error is impossible in this case because
>     of_pci_make_dev_node() returns early if dev->of_node has been set.
> 
>   - Otherwise, we want to set dev->of_node (just as we previously
>     did), and
> 
>   - if dev->fwnode has not been set, we want to set that too.
> 
> So the whole point of this is to set dev->fwnode, which we didn't do
> before.  But has np->fwnode been set to anything?  Maybe it's buried
> somewhere inside of_changeset_create_node(), but I didn't see it.

np->fwnode can be set by ACPI. We are at the frontier between ACPI and
device-tree.

The ofnode is created and filled from an already existing device. This
device can be created from information provided by the ACPI world.
In that case, np->fwnode is set to and ACPI fwnode.

Best regards,
Hervé

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Herve Codina]

Hi Bjorn,

On Fri, 28 Feb 2025 15:21:57 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Mon, Feb 24, 2025 at 03:13:55PM +0100, Herve Codina wrote:
> > PCI devices device-tree nodes can be already created. This was
> > introduced by commit 407d1a51921e ("PCI: Create device tree node for
> > bridge").
> > 
> > In order to have device-tree nodes related to PCI devices attached on
> > their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
> > root bus device-tree node is needed. This root bus node will be used as
> > the parent node of the first level devices scanned on the bus. On
> > device-tree based systems, this PCI root bus device tree node is set to
> > the node of the related PCI host bridge. The PCI host bridge node is
> > available in the device-tree used to describe the hardware passed at
> > boot.
> > 
> > On non device-tree based system (such as ACPI), a device-tree node for
> > the PCI host bridge or for the root bus does not exist. Indeed, the PCI
> > host bridge is not described in a device-tree used at boot simply
> > because no device-tree are passed at boot.
> > 
> > The device-tree PCI host bridge node creation needs to be done at
> > runtime. This is done in the same way as for the creation of the PCI
> > device nodes. I.e. node and properties are created based on computed
> > information done by the PCI core. Also, as is done on device-tree based
> > systems, this PCI host bridge node is used for the PCI root bus.
> > 
> > With this done, hardware available in a PCI device that doesn't follow
> > the PCI model consisting in one PCI function handled by one driver can
> > be described by a device-tree overlay loaded by the PCI device driver on
> > non device-tree based systems. Those PCI devices provide a single PCI
> > function that includes several functionalities that require different
> > driver. The device-tree overlay describes in that case the internal
> > devices and their relationships. It allows to load drivers needed by
> > those different devices in order to have functionalities handled.  
> 
> Since this adds host bridge nodes, does this patch specifically enable
> device tree overlays for devices on the root bus?
> 
> Were we able to load DT overlays for devices deeper in the hierarchy
> already, even without this patch?
> 

This patch itself doesn't need any support for overlay.

Yes, without this patch we can load overlay but only on a device-tree based
system. This is done by the driver that needs an overlay to handle the PCI
device. This patch is needed to load overlays in the same way on non
device-tree based system i.e. on ACPI systems.

Best regards,
Hervé

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Claudiu Beznea]

Hi, Herve,

On 24.02.2025 16:13, Herve Codina wrote:
> PCI devices device-tree nodes can be already created. This was
> introduced by commit 407d1a51921e ("PCI: Create device tree node for
> bridge").
> 
> In order to have device-tree nodes related to PCI devices attached on
> their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
> root bus device-tree node is needed. This root bus node will be used as
> the parent node of the first level devices scanned on the bus. On
> device-tree based systems, this PCI root bus device tree node is set to
> the node of the related PCI host bridge. The PCI host bridge node is
> available in the device-tree used to describe the hardware passed at
> boot.
> 
> On non device-tree based system (such as ACPI), a device-tree node for
> the PCI host bridge or for the root bus does not exist. Indeed, the PCI
> host bridge is not described in a device-tree used at boot simply
> because no device-tree are passed at boot.
> 
> The device-tree PCI host bridge node creation needs to be done at
> runtime. This is done in the same way as for the creation of the PCI
> device nodes. I.e. node and properties are created based on computed
> information done by the PCI core. Also, as is done on device-tree based
> systems, this PCI host bridge node is used for the PCI root bus.
> 
> With this done, hardware available in a PCI device that doesn't follow
> the PCI model consisting in one PCI function handled by one driver can
> be described by a device-tree overlay loaded by the PCI device driver on
> non device-tree based systems. Those PCI devices provide a single PCI
> function that includes several functionalities that require different
> driver. The device-tree overlay describes in that case the internal
> devices and their relationships. It allows to load drivers needed by
> those different devices in order to have functionalities handled.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  drivers/pci/of.c          | 104 +++++++++++++++++++++++++++++++++++++-
>  drivers/pci/of_property.c | 102 +++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h         |   6 +++
>  drivers/pci/probe.c       |   2 +
>  drivers/pci/remove.c      |   2 +
>  5 files changed, 215 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index fb5e6da1ead0..c59429e909c0 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -731,7 +731,109 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>  out_free_name:
>  	kfree(name);
>  }
> -#endif
> +
> +void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
> +{
> +	struct device_node *np;
> +
> +	np = pci_bus_to_OF_node(bridge->bus);
> +	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
> +		return;
> +
> +	device_remove_of_node(&bridge->bus->dev);
> +	device_remove_of_node(&bridge->dev);
> +	of_changeset_revert(np->data);
> +	of_changeset_destroy(np->data);
> +	of_node_put(np);
> +}
> +
> +void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> +{
> +	struct device_node *np = NULL;
> +	struct of_changeset *cset;
> +	const char *name;
> +	int ret;
> +
> +	/*
> +	 * If there is already a device-tree node linked to the PCI bus handled
> +	 * by this bridge (i.e. the PCI root bus), nothing to do.
> +	 */
> +	if (pci_bus_to_OF_node(bridge->bus))
> +		return;
> +
> +	/* The root bus has no node. Check that the host bridge has no node too */
> +	if (bridge->dev.of_node) {
> +		dev_err(&bridge->dev, "PCI host bridge of_node already set");
> +		return;
> +	}
> +
> +	/* Check if there is a DT root node to attach the created node */
> +	if (!of_root) {
> +		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> +		return;
> +	}
> +
> +	name = kasprintf(GFP_KERNEL, "pci@%x,%x", pci_domain_nr(bridge->bus),
> +			 bridge->bus->number);

After testing series [1] on next-20250528 I noticed the INTx are not
working anymore. Debugging it, I found it is related to the creation of a
node with this name.

On [1], the interrupt-map points to the pcie node itself. If I activate the
debug messages in of_irq_parse_raw() I'm getting this output:

[    0.466542] rzg3s-pcie-host 11e40000.pcie: PCIe link status [0x100014e]
[    0.466571] rzg3s-pcie-host 11e40000.pcie: PCIe x1: link up
[    0.571095] rzg3s-pcie-host 11e40000.pcie: PCI host bridge to bus 0000:00
[    0.571161] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.571198] pci_bus 0000:00: root bus resource [mem 0x30000000-0x37ffffff]
[    0.571289] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
Root Port
[    0.571355] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    0.571393] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
[    0.571433] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
64bit pref]
[    0.571533] pci 0000:00:00.0: PME# supported from D0 D3hot
[    0.574149] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
Endpoint
[    0.574223] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
0x4 may corrupt adjacent RW1C bits
[    0.574775] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
[    0.575722] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
with 8.0 GT/s PCIe x2 link)
[    0.576434] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
assigned
[    0.576491] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
assigned
[    0.576618] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    0.576654] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
[    0.576697] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
[    0.576730] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
[    0.576800] of_irq_parse_raw:  /soc/pcie@11e40000:00000001
[    0.576864] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
[    0.576904] OF:  -> addrsize=3
[    0.576936] OF:  -> match=1 (imaplen=32)
[    0.576962] OF:  -> intsize=1, addrsize=3
[    0.576984] OF:  -> imaplen=31
[    0.577009] OF: /soc/pcie@11e40000 interrupt-map entry to self
[    0.577044] of_irq_parse_raw:  /soc/pcie@11e40000:00000002
[    0.577089] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
[    0.577126] OF:  -> addrsize=3
[    0.577153] OF:  -> match=0 (imaplen=32)
[    0.577177] OF:  -> intsize=1, addrsize=3
[    0.577198] OF:  -> imaplen=31
[    0.577220] OF:  -> imaplen=27
[    0.577238] OF:  -> match=1 (imaplen=23)
[    0.577261] OF:  -> intsize=1, addrsize=3
[    0.577282] OF:  -> imaplen=22
[    0.577303] OF: /soc/pcie@11e40000 interrupt-map entry to self
[    0.577337] of_irq_parse_raw:  /soc/pcie@11e40000:00000003
[    0.577382] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
[    0.577419] OF:  -> addrsize=3
[    0.577445] OF:  -> match=0 (imaplen=32)
[    0.577469] OF:  -> intsize=1, addrsize=3
[    0.577490] OF:  -> imaplen=31
[    0.577511] OF:  -> imaplen=27
[    0.577530] OF:  -> match=0 (imaplen=23)
[    0.577553] OF:  -> intsize=1, addrsize=3
[    0.577573] OF:  -> imaplen=22
[    0.577595] OF:  -> imaplen=18
[    0.577613] OF:  -> match=1 (imaplen=14)
[    0.577637] OF:  -> intsize=1, addrsize=3
[    0.577657] OF:  -> imaplen=13
[    0.577678] OF: /soc/pcie@11e40000 interrupt-map entry to self
[    0.577712] of_irq_parse_raw:  /soc/pcie@11e40000:00000004
[    0.577758] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
[    0.577794] OF:  -> addrsize=3
[    0.577820] OF:  -> match=0 (imaplen=32)
[    0.577844] OF:  -> intsize=1, addrsize=3
[    0.577865] OF:  -> imaplen=31
[    0.577886] OF:  -> imaplen=27
[    0.577905] OF:  -> match=0 (imaplen=23)
[    0.577928] OF:  -> intsize=1, addrsize=3
[    0.577948] OF:  -> imaplen=22
[    0.577969] OF:  -> imaplen=18
[    0.577987] OF:  -> match=0 (imaplen=14)
[    0.578010] OF:  -> intsize=1, addrsize=3
[    0.578031] OF:  -> imaplen=13
[    0.578052] OF:  -> imaplen=9
[    0.578070] OF:  -> match=1 (imaplen=5)
[    0.578092] OF:  -> intsize=1, addrsize=3
[    0.578113] OF:  -> imaplen=4
[    0.578133] OF: /soc/pcie@11e40000 interrupt-map entry to self
[    0.578609] pci_assign_irq(): pin=0
[    0.578641] assign IRQ: got 0
[    0.578677] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
[    0.579095] pci_assign_irq(): pin=1
[    0.579124] pci_assign_irq(): swizzle_irq=806c2ad8, map_irq=806daa90
[    0.579154] pci_common_swizzle(): pin=1
[    0.579177] pci_assign_irq(): slot=0, pin=1
[    0.579209] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
[    0.579271] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
[    0.579314] OF:  -> addrsize=3
[    0.579339] OF:  -> match=1 (imaplen=32)
[    0.579365] OF:  -> intsize=1, addrsize=3
[    0.579386] OF:  -> imaplen=31
[    0.579409] OF:  -> new parent: /soc/pcie@11e40000
[    0.579452] OF:  -> match=0 (imaplen=32)
[    0.579476] OF:  -> intsize=1, addrsize=3
[    0.579497] OF:  -> imaplen=31
[    0.579520] OF:  -> imaplen=27
[    0.579538] OF:  -> match=0 (imaplen=23)
[    0.579562] OF:  -> intsize=1, addrsize=3
[    0.579583] OF:  -> imaplen=22
[    0.579604] OF:  -> imaplen=18
[    0.579622] OF:  -> match=0 (imaplen=14)
[    0.579645] OF:  -> intsize=1, addrsize=3
[    0.579666] OF:  -> imaplen=13
[    0.579688] OF:  -> imaplen=9
[    0.579706] OF:  -> match=0 (imaplen=5)
[    0.579728] OF:  -> intsize=1, addrsize=3
[    0.579749] OF:  -> imaplen=4
[    0.579769] OF:  -> imaplen=0
[    0.580473] nvme 0000:01:00.0: of_irq_parse_pci: failed with rc=-22
[    0.580952] assign IRQ: got 0
[    0.581718] nvme nvme0: pci function 0000:01:00.0
[    0.581811] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    0.585447] nvme nvme0: missing or invalid SUBNQN field.
[    0.592193] nvme nvme0: 1/0/0 default/read/poll queues
[    0.600035]  nvme0n1: p1


I currently managed to fix it by applying the following diff on top of [1]:

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
index f1d642c70436..aac917f0b143 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
@@ -54,13 +54,6 @@ pcie: pcie@11e40000 {
                                  "intb", "intc", "intd", "msi",
                                  "link_bandwidth", "pm_pme", "dma",
                                  "pcie_evt", "msg", "all";
-               #interrupt-cells = <1>;
-               interrupt-controller;
-               interrupt-map-mask = <0 0 0 7>;
-               interrupt-map = <0 0 0 1 &pcie 0 0 0 0>, /* INT A */
-                               <0 0 0 2 &pcie 0 0 0 1>, /* INT B */
-                               <0 0 0 3 &pcie 0 0 0 2>, /* INT C */
-                               <0 0 0 4 &pcie 0 0 0 3>; /* INT D */
                device_type = "pci";
                num-lanes = <1>;
                #address-cells = <3>;
@@ -70,5 +63,20 @@ pcie: pcie@11e40000 {
                device-id = <0x0033>;
                renesas,sysc = <&sysc>;
                status = "disabled";
+
+               port0: pci@0,0 {
+                       device_type = "pci";
+                       reg = <0x0 0x0 0x0 0x0 0x0>;
+                       #address-cells = <3>;
+                       #size-cells = <2>;
+                       ranges;
+                       #interrupt-cells = <1>;
+                       interrupt-controller;
+                       interrupt-map-mask = <0 0 0 7>;
+                       interrupt-map = <0 0 0 1 &port0 0 0 0 0>, /* INT A */
+                                       <0 0 0 2 &port0 0 0 0 1>, /* INT B */
+                                       <0 0 0 3 &port0 0 0 0 2>, /* INT C */
+                                       <0 0 0 4 &port0 0 0 0 3>; /* INT D */
+               };
        };
 };
diff --git a/drivers/pci/controller/pcie-rzg3s-host.c
b/drivers/pci/controller/pcie-rzg3s-host.c
index 39140183addf..affbf4f79f23 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -431,7 +431,24 @@ static int rzg3s_pcie_root_write(struct pci_bus *bus,
unsigned int devfn,
        return PCIBIOS_SUCCESSFUL;
 }

+static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
device_node *port);
+
+static int rzg3s_pcie_root_add_bus(struct pci_bus *bus)
+{
+       struct device_node *of_port;
+
+       for_each_child_of_node(bus->dev.of_node, of_port) {
+               int ret = rzg3s_pcie_intx_setup(bus->sysdata, of_port);
+
+               if (ret)
+                       return ret;
+       }
+
+       return 0;
+}
+
 static struct pci_ops rzg3s_pcie_root_ops = {
+       .add_bus        = rzg3s_pcie_root_add_bus,
        .read           = pci_generic_config_read,
        .write          = rzg3s_pcie_root_write,
        .map_bus        = rzg3s_pcie_root_map_bus,
@@ -895,7 +912,7 @@ static void rzg3s_pcie_intx_teardown(void *data)
        irq_domain_remove(host->intx_domain);
 }

-static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host)
+static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
device_node *port)
 {
        struct device *dev = host->dev;

@@ -918,7 +935,7 @@ static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host
*host)
                                                 host);
        }

-       host->intx_domain = irq_domain_add_linear(dev->of_node, PCI_NUM_INTX,
+       host->intx_domain = irq_domain_add_linear(port, PCI_NUM_INTX,
                                                  &rzg3s_pcie_intx_domain_ops,
                                                  host);
        if (!host->intx_domain)
@@ -1542,7 +1559,7 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)

        raw_spin_lock_init(&host->hw_lock);

-       ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_intx_setup,
+       ret = rzg3s_pcie_host_setup(host, NULL,
                                    rzg3s_pcie_msi_enable, true);
        if (ret)
                return ret;


With this, of_irq_parse_pci() no longer fails with -22:


[    0.564106] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
Root Port
[    0.564173] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    0.564212] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
[    0.564252] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
64bit pref]
[    0.564355] pci 0000:00:00.0: PME# supported from D0 D3hot
[    0.564999] /soc/pcie@11e40000/pci@0,0: Fixed dependency cycle(s) with
/soc/pcie@11e40000/pci@0,0
[    0.567407] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
Endpoint
[    0.567483] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
0x4 may corrupt adjacent RW1C bits
[    0.567922] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
[    0.568870] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
with 8.0 GT/s PCIe x2 link)
[    0.569599] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
assigned
[    0.569657] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
assigned
[    0.569785] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    0.569822] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
[    0.570056] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
[    0.570095] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
[    0.570311] pci_assign_irq(): pin=0
[    0.570338] assign IRQ: got 0
[    0.570373] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
[    0.570775] pci_assign_irq(): pin=1
[    0.570805] pci_assign_irq(): swizzle_irq=806c2b70, map_irq=806dab28
[    0.570834] pci_common_swizzle(): pin=1
[    0.570855] pci_assign_irq(): slot=0, pin=1
[    0.570888] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
[    0.570954] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
[    0.570997] OF:  -> addrsize=3
[    0.571028] OF:  -> match=1 (imaplen=32)
[    0.571053] OF:  -> intsize=1, addrsize=3
[    0.571075] OF:  -> imaplen=31
[    0.571095] OF: /soc/pcie@11e40000/pci@0,0 interrupt-map entry to self
[    0.571270] assign IRQ: got 46
[    0.571998] nvme nvme0: pci function 0000:01:00.0
[    0.572082] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    0.575619] nvme nvme0: missing or invalid SUBNQN field.
[    0.584554] nvme nvme0: 1/0/0 default/read/poll queues
[    0.592958]  nvme0n1: p1


Could you please let me know if this is how the PCIe controller should now
be described in DT with your patch?

Thank you,
Claudiu


[1]
https://lore.kernel.org/all/20250530111917.1495023-1-claudiu.beznea.uj@bp.renesas.com

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Herve Codina]

Hi Claudiu,

On Wed, 11 Jun 2025 11:26:37 +0300
Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:

> Hi, Herve,
> 
> On 24.02.2025 16:13, Herve Codina wrote:
> > PCI devices device-tree nodes can be already created. This was
> > introduced by commit 407d1a51921e ("PCI: Create device tree node for
> > bridge").
> > 
> > In order to have device-tree nodes related to PCI devices attached on
> > their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
> > root bus device-tree node is needed. This root bus node will be used as
> > the parent node of the first level devices scanned on the bus. On
> > device-tree based systems, this PCI root bus device tree node is set to
> > the node of the related PCI host bridge. The PCI host bridge node is
> > available in the device-tree used to describe the hardware passed at
> > boot.
> > 
> > On non device-tree based system (such as ACPI), a device-tree node for
> > the PCI host bridge or for the root bus does not exist. Indeed, the PCI
> > host bridge is not described in a device-tree used at boot simply
> > because no device-tree are passed at boot.
> > 
> > The device-tree PCI host bridge node creation needs to be done at
> > runtime. This is done in the same way as for the creation of the PCI
> > device nodes. I.e. node and properties are created based on computed
> > information done by the PCI core. Also, as is done on device-tree based
> > systems, this PCI host bridge node is used for the PCI root bus.
> > 
> > With this done, hardware available in a PCI device that doesn't follow
> > the PCI model consisting in one PCI function handled by one driver can
> > be described by a device-tree overlay loaded by the PCI device driver on
> > non device-tree based systems. Those PCI devices provide a single PCI
> > function that includes several functionalities that require different
> > driver. The device-tree overlay describes in that case the internal
> > devices and their relationships. It allows to load drivers needed by
> > those different devices in order to have functionalities handled.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> >  drivers/pci/of.c          | 104 +++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/of_property.c | 102 +++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h         |   6 +++
> >  drivers/pci/probe.c       |   2 +
> >  drivers/pci/remove.c      |   2 +
> >  5 files changed, 215 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index fb5e6da1ead0..c59429e909c0 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -731,7 +731,109 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
> >  out_free_name:
> >  	kfree(name);
> >  }
> > -#endif
> > +
> > +void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
> > +{
> > +	struct device_node *np;
> > +
> > +	np = pci_bus_to_OF_node(bridge->bus);
> > +	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
> > +		return;
> > +
> > +	device_remove_of_node(&bridge->bus->dev);
> > +	device_remove_of_node(&bridge->dev);
> > +	of_changeset_revert(np->data);
> > +	of_changeset_destroy(np->data);
> > +	of_node_put(np);
> > +}
> > +
> > +void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
> > +{
> > +	struct device_node *np = NULL;
> > +	struct of_changeset *cset;
> > +	const char *name;
> > +	int ret;
> > +
> > +	/*
> > +	 * If there is already a device-tree node linked to the PCI bus handled
> > +	 * by this bridge (i.e. the PCI root bus), nothing to do.
> > +	 */
> > +	if (pci_bus_to_OF_node(bridge->bus))
> > +		return;
> > +
> > +	/* The root bus has no node. Check that the host bridge has no node too */
> > +	if (bridge->dev.of_node) {
> > +		dev_err(&bridge->dev, "PCI host bridge of_node already set");
> > +		return;
> > +	}
> > +
> > +	/* Check if there is a DT root node to attach the created node */
> > +	if (!of_root) {
> > +		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
> > +		return;
> > +	}
> > +
> > +	name = kasprintf(GFP_KERNEL, "pci@%x,%x", pci_domain_nr(bridge->bus),
> > +			 bridge->bus->number);  
> 
> After testing series [1] on next-20250528 I noticed the INTx are not
> working anymore. Debugging it, I found it is related to the creation of a
> node with this name.
> 
> On [1], the interrupt-map points to the pcie node itself. If I activate the
> debug messages in of_irq_parse_raw() I'm getting this output:
> 
> [    0.466542] rzg3s-pcie-host 11e40000.pcie: PCIe link status [0x100014e]
> [    0.466571] rzg3s-pcie-host 11e40000.pcie: PCIe x1: link up
> [    0.571095] rzg3s-pcie-host 11e40000.pcie: PCI host bridge to bus 0000:00
> [    0.571161] pci_bus 0000:00: root bus resource [bus 00-ff]
> [    0.571198] pci_bus 0000:00: root bus resource [mem 0x30000000-0x37ffffff]
> [    0.571289] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
> Root Port
> [    0.571355] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    0.571393] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
> [    0.571433] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
> 64bit pref]
> [    0.571533] pci 0000:00:00.0: PME# supported from D0 D3hot
> [    0.574149] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
> Endpoint
> [    0.574223] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
> 0x4 may corrupt adjacent RW1C bits
> [    0.574775] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
> [    0.575722] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
> limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
> with 8.0 GT/s PCIe x2 link)
> [    0.576434] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
> assigned
> [    0.576491] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
> assigned
> [    0.576618] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    0.576654] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
> [    0.576697] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
> [    0.576730] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
> [    0.576800] of_irq_parse_raw:  /soc/pcie@11e40000:00000001
> [    0.576864] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
> [    0.576904] OF:  -> addrsize=3
> [    0.576936] OF:  -> match=1 (imaplen=32)
> [    0.576962] OF:  -> intsize=1, addrsize=3
> [    0.576984] OF:  -> imaplen=31
> [    0.577009] OF: /soc/pcie@11e40000 interrupt-map entry to self
> [    0.577044] of_irq_parse_raw:  /soc/pcie@11e40000:00000002
> [    0.577089] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
> [    0.577126] OF:  -> addrsize=3
> [    0.577153] OF:  -> match=0 (imaplen=32)
> [    0.577177] OF:  -> intsize=1, addrsize=3
> [    0.577198] OF:  -> imaplen=31
> [    0.577220] OF:  -> imaplen=27
> [    0.577238] OF:  -> match=1 (imaplen=23)
> [    0.577261] OF:  -> intsize=1, addrsize=3
> [    0.577282] OF:  -> imaplen=22
> [    0.577303] OF: /soc/pcie@11e40000 interrupt-map entry to self
> [    0.577337] of_irq_parse_raw:  /soc/pcie@11e40000:00000003
> [    0.577382] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
> [    0.577419] OF:  -> addrsize=3
> [    0.577445] OF:  -> match=0 (imaplen=32)
> [    0.577469] OF:  -> intsize=1, addrsize=3
> [    0.577490] OF:  -> imaplen=31
> [    0.577511] OF:  -> imaplen=27
> [    0.577530] OF:  -> match=0 (imaplen=23)
> [    0.577553] OF:  -> intsize=1, addrsize=3
> [    0.577573] OF:  -> imaplen=22
> [    0.577595] OF:  -> imaplen=18
> [    0.577613] OF:  -> match=1 (imaplen=14)
> [    0.577637] OF:  -> intsize=1, addrsize=3
> [    0.577657] OF:  -> imaplen=13
> [    0.577678] OF: /soc/pcie@11e40000 interrupt-map entry to self
> [    0.577712] of_irq_parse_raw:  /soc/pcie@11e40000:00000004
> [    0.577758] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
> [    0.577794] OF:  -> addrsize=3
> [    0.577820] OF:  -> match=0 (imaplen=32)
> [    0.577844] OF:  -> intsize=1, addrsize=3
> [    0.577865] OF:  -> imaplen=31
> [    0.577886] OF:  -> imaplen=27
> [    0.577905] OF:  -> match=0 (imaplen=23)
> [    0.577928] OF:  -> intsize=1, addrsize=3
> [    0.577948] OF:  -> imaplen=22
> [    0.577969] OF:  -> imaplen=18
> [    0.577987] OF:  -> match=0 (imaplen=14)
> [    0.578010] OF:  -> intsize=1, addrsize=3
> [    0.578031] OF:  -> imaplen=13
> [    0.578052] OF:  -> imaplen=9
> [    0.578070] OF:  -> match=1 (imaplen=5)
> [    0.578092] OF:  -> intsize=1, addrsize=3
> [    0.578113] OF:  -> imaplen=4
> [    0.578133] OF: /soc/pcie@11e40000 interrupt-map entry to self
> [    0.578609] pci_assign_irq(): pin=0
> [    0.578641] assign IRQ: got 0
> [    0.578677] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
> [    0.579095] pci_assign_irq(): pin=1
> [    0.579124] pci_assign_irq(): swizzle_irq=806c2ad8, map_irq=806daa90
> [    0.579154] pci_common_swizzle(): pin=1
> [    0.579177] pci_assign_irq(): slot=0, pin=1
> [    0.579209] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
> [    0.579271] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
> [    0.579314] OF:  -> addrsize=3
> [    0.579339] OF:  -> match=1 (imaplen=32)
> [    0.579365] OF:  -> intsize=1, addrsize=3
> [    0.579386] OF:  -> imaplen=31
> [    0.579409] OF:  -> new parent: /soc/pcie@11e40000
> [    0.579452] OF:  -> match=0 (imaplen=32)
> [    0.579476] OF:  -> intsize=1, addrsize=3
> [    0.579497] OF:  -> imaplen=31
> [    0.579520] OF:  -> imaplen=27
> [    0.579538] OF:  -> match=0 (imaplen=23)
> [    0.579562] OF:  -> intsize=1, addrsize=3
> [    0.579583] OF:  -> imaplen=22
> [    0.579604] OF:  -> imaplen=18
> [    0.579622] OF:  -> match=0 (imaplen=14)
> [    0.579645] OF:  -> intsize=1, addrsize=3
> [    0.579666] OF:  -> imaplen=13
> [    0.579688] OF:  -> imaplen=9
> [    0.579706] OF:  -> match=0 (imaplen=5)
> [    0.579728] OF:  -> intsize=1, addrsize=3
> [    0.579749] OF:  -> imaplen=4
> [    0.579769] OF:  -> imaplen=0
> [    0.580473] nvme 0000:01:00.0: of_irq_parse_pci: failed with rc=-22
> [    0.580952] assign IRQ: got 0
> [    0.581718] nvme nvme0: pci function 0000:01:00.0
> [    0.581811] nvme 0000:01:00.0: enabling device (0000 -> 0002)
> [    0.585447] nvme nvme0: missing or invalid SUBNQN field.
> [    0.592193] nvme nvme0: 1/0/0 default/read/poll queues
> [    0.600035]  nvme0n1: p1
> 
> 
> I currently managed to fix it by applying the following diff on top of [1]:
> 
> diff --git a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
> b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
> index f1d642c70436..aac917f0b143 100644
> --- a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
> @@ -54,13 +54,6 @@ pcie: pcie@11e40000 {
>                                   "intb", "intc", "intd", "msi",
>                                   "link_bandwidth", "pm_pme", "dma",
>                                   "pcie_evt", "msg", "all";
> -               #interrupt-cells = <1>;
> -               interrupt-controller;
> -               interrupt-map-mask = <0 0 0 7>;
> -               interrupt-map = <0 0 0 1 &pcie 0 0 0 0>, /* INT A */
> -                               <0 0 0 2 &pcie 0 0 0 1>, /* INT B */
> -                               <0 0 0 3 &pcie 0 0 0 2>, /* INT C */
> -                               <0 0 0 4 &pcie 0 0 0 3>; /* INT D */
>                 device_type = "pci";
>                 num-lanes = <1>;
>                 #address-cells = <3>;
> @@ -70,5 +63,20 @@ pcie: pcie@11e40000 {
>                 device-id = <0x0033>;
>                 renesas,sysc = <&sysc>;
>                 status = "disabled";
> +
> +               port0: pci@0,0 {
> +                       device_type = "pci";
> +                       reg = <0x0 0x0 0x0 0x0 0x0>;
> +                       #address-cells = <3>;
> +                       #size-cells = <2>;
> +                       ranges;
> +                       #interrupt-cells = <1>;
> +                       interrupt-controller;
> +                       interrupt-map-mask = <0 0 0 7>;
> +                       interrupt-map = <0 0 0 1 &port0 0 0 0 0>, /* INT A */
> +                                       <0 0 0 2 &port0 0 0 0 1>, /* INT B */
> +                                       <0 0 0 3 &port0 0 0 0 2>, /* INT C */
> +                                       <0 0 0 4 &port0 0 0 0 3>; /* INT D */
> +               };
>         };
>  };
> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c
> b/drivers/pci/controller/pcie-rzg3s-host.c
> index 39140183addf..affbf4f79f23 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -431,7 +431,24 @@ static int rzg3s_pcie_root_write(struct pci_bus *bus,
> unsigned int devfn,
>         return PCIBIOS_SUCCESSFUL;
>  }
> 
> +static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
> device_node *port);
> +
> +static int rzg3s_pcie_root_add_bus(struct pci_bus *bus)
> +{
> +       struct device_node *of_port;
> +
> +       for_each_child_of_node(bus->dev.of_node, of_port) {
> +               int ret = rzg3s_pcie_intx_setup(bus->sysdata, of_port);
> +
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
>  static struct pci_ops rzg3s_pcie_root_ops = {
> +       .add_bus        = rzg3s_pcie_root_add_bus,
>         .read           = pci_generic_config_read,
>         .write          = rzg3s_pcie_root_write,
>         .map_bus        = rzg3s_pcie_root_map_bus,
> @@ -895,7 +912,7 @@ static void rzg3s_pcie_intx_teardown(void *data)
>         irq_domain_remove(host->intx_domain);
>  }
> 
> -static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host)
> +static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
> device_node *port)
>  {
>         struct device *dev = host->dev;
> 
> @@ -918,7 +935,7 @@ static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host
> *host)
>                                                  host);
>         }
> 
> -       host->intx_domain = irq_domain_add_linear(dev->of_node, PCI_NUM_INTX,
> +       host->intx_domain = irq_domain_add_linear(port, PCI_NUM_INTX,
>                                                   &rzg3s_pcie_intx_domain_ops,
>                                                   host);
>         if (!host->intx_domain)
> @@ -1542,7 +1559,7 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
> 
>         raw_spin_lock_init(&host->hw_lock);
> 
> -       ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_intx_setup,
> +       ret = rzg3s_pcie_host_setup(host, NULL,
>                                     rzg3s_pcie_msi_enable, true);
>         if (ret)
>                 return ret;
> 
> 
> With this, of_irq_parse_pci() no longer fails with -22:
> 
> 
> [    0.564106] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
> Root Port
> [    0.564173] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    0.564212] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
> [    0.564252] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
> 64bit pref]
> [    0.564355] pci 0000:00:00.0: PME# supported from D0 D3hot
> [    0.564999] /soc/pcie@11e40000/pci@0,0: Fixed dependency cycle(s) with
> /soc/pcie@11e40000/pci@0,0
> [    0.567407] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
> Endpoint
> [    0.567483] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
> 0x4 may corrupt adjacent RW1C bits
> [    0.567922] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
> [    0.568870] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
> limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
> with 8.0 GT/s PCIe x2 link)
> [    0.569599] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
> assigned
> [    0.569657] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
> assigned
> [    0.569785] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> [    0.569822] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
> [    0.570056] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
> [    0.570095] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
> [    0.570311] pci_assign_irq(): pin=0
> [    0.570338] assign IRQ: got 0
> [    0.570373] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
> [    0.570775] pci_assign_irq(): pin=1
> [    0.570805] pci_assign_irq(): swizzle_irq=806c2b70, map_irq=806dab28
> [    0.570834] pci_common_swizzle(): pin=1
> [    0.570855] pci_assign_irq(): slot=0, pin=1
> [    0.570888] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
> [    0.570954] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
> [    0.570997] OF:  -> addrsize=3
> [    0.571028] OF:  -> match=1 (imaplen=32)
> [    0.571053] OF:  -> intsize=1, addrsize=3
> [    0.571075] OF:  -> imaplen=31
> [    0.571095] OF: /soc/pcie@11e40000/pci@0,0 interrupt-map entry to self
> [    0.571270] assign IRQ: got 46
> [    0.571998] nvme nvme0: pci function 0000:01:00.0
> [    0.572082] nvme 0000:01:00.0: enabling device (0000 -> 0002)
> [    0.575619] nvme nvme0: missing or invalid SUBNQN field.
> [    0.584554] nvme nvme0: 1/0/0 default/read/poll queues
> [    0.592958]  nvme0n1: p1
> 
> 
> Could you please let me know if this is how the PCIe controller should now
> be described in DT with your patch?

In my series, no node should be created if a node is already existing.
  https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L764
  https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L771

The idea is to create the missing node which can be used as PCI device node
parent.

On my side, I have performed tests using an Armada 3720 board and so, I used
this DT:
  https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/marvell/armada-3720-db.dts
and more precisely, this PCIe controller node description:
  https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/marvell/armada-37xx.dtsi#L495

On this system, I didn't observed any issues but of course, the PCIe drivers are
different.
Also, on my system, no node were created by of_pci_make_host_bridge_node().

To be honest, I didn't re-test recently to see if something has been broken.
I can do that on my side with my system.

On your side, maybe you can have look at the Armada PCIe driver and see if
something could explain your behavior. I am not sure that you need to add the
pci@0,0 node in your DT.

Best regards,
Hervé

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Claudiu Beznea]

Hi, Herve,

On 11.06.2025 17:56, Herve Codina wrote:
> Hi Claudiu,
> 
> On Wed, 11 Jun 2025 11:26:37 +0300
> Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
> 
>> Hi, Herve,
>>
>> On 24.02.2025 16:13, Herve Codina wrote:
>>> PCI devices device-tree nodes can be already created. This was
>>> introduced by commit 407d1a51921e ("PCI: Create device tree node for
>>> bridge").
>>>
>>> In order to have device-tree nodes related to PCI devices attached on
>>> their PCI root bus (the PCI bus handled by the PCI host bridge), a PCI
>>> root bus device-tree node is needed. This root bus node will be used as
>>> the parent node of the first level devices scanned on the bus. On
>>> device-tree based systems, this PCI root bus device tree node is set to
>>> the node of the related PCI host bridge. The PCI host bridge node is
>>> available in the device-tree used to describe the hardware passed at
>>> boot.
>>>
>>> On non device-tree based system (such as ACPI), a device-tree node for
>>> the PCI host bridge or for the root bus does not exist. Indeed, the PCI
>>> host bridge is not described in a device-tree used at boot simply
>>> because no device-tree are passed at boot.
>>>
>>> The device-tree PCI host bridge node creation needs to be done at
>>> runtime. This is done in the same way as for the creation of the PCI
>>> device nodes. I.e. node and properties are created based on computed
>>> information done by the PCI core. Also, as is done on device-tree based
>>> systems, this PCI host bridge node is used for the PCI root bus.
>>>
>>> With this done, hardware available in a PCI device that doesn't follow
>>> the PCI model consisting in one PCI function handled by one driver can
>>> be described by a device-tree overlay loaded by the PCI device driver on
>>> non device-tree based systems. Those PCI devices provide a single PCI
>>> function that includes several functionalities that require different
>>> driver. The device-tree overlay describes in that case the internal
>>> devices and their relationships. It allows to load drivers needed by
>>> those different devices in order to have functionalities handled.
>>>
>>> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>>> ---
>>>  drivers/pci/of.c          | 104 +++++++++++++++++++++++++++++++++++++-
>>>  drivers/pci/of_property.c | 102 +++++++++++++++++++++++++++++++++++++
>>>  drivers/pci/pci.h         |   6 +++
>>>  drivers/pci/probe.c       |   2 +
>>>  drivers/pci/remove.c      |   2 +
>>>  5 files changed, 215 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>>> index fb5e6da1ead0..c59429e909c0 100644
>>> --- a/drivers/pci/of.c
>>> +++ b/drivers/pci/of.c
>>> @@ -731,7 +731,109 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>>>  out_free_name:
>>>  	kfree(name);
>>>  }
>>> -#endif
>>> +
>>> +void of_pci_remove_host_bridge_node(struct pci_host_bridge *bridge)
>>> +{
>>> +	struct device_node *np;
>>> +
>>> +	np = pci_bus_to_OF_node(bridge->bus);
>>> +	if (!np || !of_node_check_flag(np, OF_DYNAMIC))
>>> +		return;
>>> +
>>> +	device_remove_of_node(&bridge->bus->dev);
>>> +	device_remove_of_node(&bridge->dev);
>>> +	of_changeset_revert(np->data);
>>> +	of_changeset_destroy(np->data);
>>> +	of_node_put(np);
>>> +}
>>> +
>>> +void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
>>> +{
>>> +	struct device_node *np = NULL;
>>> +	struct of_changeset *cset;
>>> +	const char *name;
>>> +	int ret;
>>> +
>>> +	/*
>>> +	 * If there is already a device-tree node linked to the PCI bus handled
>>> +	 * by this bridge (i.e. the PCI root bus), nothing to do.
>>> +	 */
>>> +	if (pci_bus_to_OF_node(bridge->bus))
>>> +		return;
>>> +
>>> +	/* The root bus has no node. Check that the host bridge has no node too */
>>> +	if (bridge->dev.of_node) {
>>> +		dev_err(&bridge->dev, "PCI host bridge of_node already set");
>>> +		return;
>>> +	}
>>> +
>>> +	/* Check if there is a DT root node to attach the created node */
>>> +	if (!of_root) {
>>> +		pr_err("of_root node is NULL, cannot create PCI host bridge node\n");
>>> +		return;
>>> +	}
>>> +
>>> +	name = kasprintf(GFP_KERNEL, "pci@%x,%x", pci_domain_nr(bridge->bus),
>>> +			 bridge->bus->number);  
>>
>> After testing series [1] on next-20250528 I noticed the INTx are not
>> working anymore. Debugging it, I found it is related to the creation of a
>> node with this name.

I pointed to the wrong function. It's not of_pci_make_host_bridge_node()
[1] but of_pci_make_dev_node() which creates a node with a similar naming
and makes things not working on my side.

[1] https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L694

>>
>> On [1], the interrupt-map points to the pcie node itself. If I activate the
>> debug messages in of_irq_parse_raw() I'm getting this output:
>>
>> [    0.466542] rzg3s-pcie-host 11e40000.pcie: PCIe link status [0x100014e]
>> [    0.466571] rzg3s-pcie-host 11e40000.pcie: PCIe x1: link up
>> [    0.571095] rzg3s-pcie-host 11e40000.pcie: PCI host bridge to bus 0000:00
>> [    0.571161] pci_bus 0000:00: root bus resource [bus 00-ff]
>> [    0.571198] pci_bus 0000:00: root bus resource [mem 0x30000000-0x37ffffff]
>> [    0.571289] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
>> Root Port
>> [    0.571355] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> [    0.571393] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
>> [    0.571433] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
>> 64bit pref]
>> [    0.571533] pci 0000:00:00.0: PME# supported from D0 D3hot
>> [    0.574149] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
>> Endpoint
>> [    0.574223] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
>> 0x4 may corrupt adjacent RW1C bits
>> [    0.574775] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
>> [    0.575722] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
>> limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
>> with 8.0 GT/s PCIe x2 link)
>> [    0.576434] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
>> assigned
>> [    0.576491] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
>> assigned
>> [    0.576618] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> [    0.576654] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
>> [    0.576697] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
>> [    0.576730] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
>> [    0.576800] of_irq_parse_raw:  /soc/pcie@11e40000:00000001
>> [    0.576864] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
>> [    0.576904] OF:  -> addrsize=3
>> [    0.576936] OF:  -> match=1 (imaplen=32)
>> [    0.576962] OF:  -> intsize=1, addrsize=3
>> [    0.576984] OF:  -> imaplen=31
>> [    0.577009] OF: /soc/pcie@11e40000 interrupt-map entry to self
>> [    0.577044] of_irq_parse_raw:  /soc/pcie@11e40000:00000002
>> [    0.577089] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
>> [    0.577126] OF:  -> addrsize=3
>> [    0.577153] OF:  -> match=0 (imaplen=32)
>> [    0.577177] OF:  -> intsize=1, addrsize=3
>> [    0.577198] OF:  -> imaplen=31
>> [    0.577220] OF:  -> imaplen=27
>> [    0.577238] OF:  -> match=1 (imaplen=23)
>> [    0.577261] OF:  -> intsize=1, addrsize=3
>> [    0.577282] OF:  -> imaplen=22
>> [    0.577303] OF: /soc/pcie@11e40000 interrupt-map entry to self
>> [    0.577337] of_irq_parse_raw:  /soc/pcie@11e40000:00000003
>> [    0.577382] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
>> [    0.577419] OF:  -> addrsize=3
>> [    0.577445] OF:  -> match=0 (imaplen=32)
>> [    0.577469] OF:  -> intsize=1, addrsize=3
>> [    0.577490] OF:  -> imaplen=31
>> [    0.577511] OF:  -> imaplen=27
>> [    0.577530] OF:  -> match=0 (imaplen=23)
>> [    0.577553] OF:  -> intsize=1, addrsize=3
>> [    0.577573] OF:  -> imaplen=22
>> [    0.577595] OF:  -> imaplen=18
>> [    0.577613] OF:  -> match=1 (imaplen=14)
>> [    0.577637] OF:  -> intsize=1, addrsize=3
>> [    0.577657] OF:  -> imaplen=13
>> [    0.577678] OF: /soc/pcie@11e40000 interrupt-map entry to self
>> [    0.577712] of_irq_parse_raw:  /soc/pcie@11e40000:00000004
>> [    0.577758] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000, size=1
>> [    0.577794] OF:  -> addrsize=3
>> [    0.577820] OF:  -> match=0 (imaplen=32)
>> [    0.577844] OF:  -> intsize=1, addrsize=3
>> [    0.577865] OF:  -> imaplen=31
>> [    0.577886] OF:  -> imaplen=27
>> [    0.577905] OF:  -> match=0 (imaplen=23)
>> [    0.577928] OF:  -> intsize=1, addrsize=3
>> [    0.577948] OF:  -> imaplen=22
>> [    0.577969] OF:  -> imaplen=18
>> [    0.577987] OF:  -> match=0 (imaplen=14)
>> [    0.578010] OF:  -> intsize=1, addrsize=3
>> [    0.578031] OF:  -> imaplen=13
>> [    0.578052] OF:  -> imaplen=9
>> [    0.578070] OF:  -> match=1 (imaplen=5)
>> [    0.578092] OF:  -> intsize=1, addrsize=3
>> [    0.578113] OF:  -> imaplen=4
>> [    0.578133] OF: /soc/pcie@11e40000 interrupt-map entry to self
>> [    0.578609] pci_assign_irq(): pin=0
>> [    0.578641] assign IRQ: got 0
>> [    0.578677] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
>> [    0.579095] pci_assign_irq(): pin=1
>> [    0.579124] pci_assign_irq(): swizzle_irq=806c2ad8, map_irq=806daa90
>> [    0.579154] pci_common_swizzle(): pin=1
>> [    0.579177] pci_assign_irq(): slot=0, pin=1
>> [    0.579209] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
>> [    0.579271] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
>> [    0.579314] OF:  -> addrsize=3
>> [    0.579339] OF:  -> match=1 (imaplen=32)
>> [    0.579365] OF:  -> intsize=1, addrsize=3
>> [    0.579386] OF:  -> imaplen=31
>> [    0.579409] OF:  -> new parent: /soc/pcie@11e40000
>> [    0.579452] OF:  -> match=0 (imaplen=32)
>> [    0.579476] OF:  -> intsize=1, addrsize=3
>> [    0.579497] OF:  -> imaplen=31
>> [    0.579520] OF:  -> imaplen=27
>> [    0.579538] OF:  -> match=0 (imaplen=23)
>> [    0.579562] OF:  -> intsize=1, addrsize=3
>> [    0.579583] OF:  -> imaplen=22
>> [    0.579604] OF:  -> imaplen=18
>> [    0.579622] OF:  -> match=0 (imaplen=14)
>> [    0.579645] OF:  -> intsize=1, addrsize=3
>> [    0.579666] OF:  -> imaplen=13
>> [    0.579688] OF:  -> imaplen=9
>> [    0.579706] OF:  -> match=0 (imaplen=5)
>> [    0.579728] OF:  -> intsize=1, addrsize=3
>> [    0.579749] OF:  -> imaplen=4
>> [    0.579769] OF:  -> imaplen=0
>> [    0.580473] nvme 0000:01:00.0: of_irq_parse_pci: failed with rc=-22
>> [    0.580952] assign IRQ: got 0
>> [    0.581718] nvme nvme0: pci function 0000:01:00.0
>> [    0.581811] nvme 0000:01:00.0: enabling device (0000 -> 0002)
>> [    0.585447] nvme nvme0: missing or invalid SUBNQN field.
>> [    0.592193] nvme nvme0: 1/0/0 default/read/poll queues
>> [    0.600035]  nvme0n1: p1
>>
>>
>> I currently managed to fix it by applying the following diff on top of [1]:
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
>> b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
>> index f1d642c70436..aac917f0b143 100644
>> --- a/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r9a08g045s33.dtsi
>> @@ -54,13 +54,6 @@ pcie: pcie@11e40000 {
>>                                   "intb", "intc", "intd", "msi",
>>                                   "link_bandwidth", "pm_pme", "dma",
>>                                   "pcie_evt", "msg", "all";
>> -               #interrupt-cells = <1>;
>> -               interrupt-controller;
>> -               interrupt-map-mask = <0 0 0 7>;
>> -               interrupt-map = <0 0 0 1 &pcie 0 0 0 0>, /* INT A */
>> -                               <0 0 0 2 &pcie 0 0 0 1>, /* INT B */
>> -                               <0 0 0 3 &pcie 0 0 0 2>, /* INT C */
>> -                               <0 0 0 4 &pcie 0 0 0 3>; /* INT D */
>>                 device_type = "pci";
>>                 num-lanes = <1>;
>>                 #address-cells = <3>;
>> @@ -70,5 +63,20 @@ pcie: pcie@11e40000 {
>>                 device-id = <0x0033>;
>>                 renesas,sysc = <&sysc>;
>>                 status = "disabled";
>> +
>> +               port0: pci@0,0 {
>> +                       device_type = "pci";
>> +                       reg = <0x0 0x0 0x0 0x0 0x0>;
>> +                       #address-cells = <3>;
>> +                       #size-cells = <2>;
>> +                       ranges;
>> +                       #interrupt-cells = <1>;
>> +                       interrupt-controller;
>> +                       interrupt-map-mask = <0 0 0 7>;
>> +                       interrupt-map = <0 0 0 1 &port0 0 0 0 0>, /* INT A */
>> +                                       <0 0 0 2 &port0 0 0 0 1>, /* INT B */
>> +                                       <0 0 0 3 &port0 0 0 0 2>, /* INT C */
>> +                                       <0 0 0 4 &port0 0 0 0 3>; /* INT D */
>> +               };
>>         };
>>  };
>> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c
>> b/drivers/pci/controller/pcie-rzg3s-host.c
>> index 39140183addf..affbf4f79f23 100644
>> --- a/drivers/pci/controller/pcie-rzg3s-host.c
>> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
>> @@ -431,7 +431,24 @@ static int rzg3s_pcie_root_write(struct pci_bus *bus,
>> unsigned int devfn,
>>         return PCIBIOS_SUCCESSFUL;
>>  }
>>
>> +static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
>> device_node *port);
>> +
>> +static int rzg3s_pcie_root_add_bus(struct pci_bus *bus)
>> +{
>> +       struct device_node *of_port;
>> +
>> +       for_each_child_of_node(bus->dev.of_node, of_port) {
>> +               int ret = rzg3s_pcie_intx_setup(bus->sysdata, of_port);
>> +
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static struct pci_ops rzg3s_pcie_root_ops = {
>> +       .add_bus        = rzg3s_pcie_root_add_bus,
>>         .read           = pci_generic_config_read,
>>         .write          = rzg3s_pcie_root_write,
>>         .map_bus        = rzg3s_pcie_root_map_bus,
>> @@ -895,7 +912,7 @@ static void rzg3s_pcie_intx_teardown(void *data)
>>         irq_domain_remove(host->intx_domain);
>>  }
>>
>> -static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host)
>> +static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host *host, struct
>> device_node *port)
>>  {
>>         struct device *dev = host->dev;
>>
>> @@ -918,7 +935,7 @@ static int rzg3s_pcie_intx_setup(struct rzg3s_pcie_host
>> *host)
>>                                                  host);
>>         }
>>
>> -       host->intx_domain = irq_domain_add_linear(dev->of_node, PCI_NUM_INTX,
>> +       host->intx_domain = irq_domain_add_linear(port, PCI_NUM_INTX,
>>                                                   &rzg3s_pcie_intx_domain_ops,
>>                                                   host);
>>         if (!host->intx_domain)
>> @@ -1542,7 +1559,7 @@ static int rzg3s_pcie_probe(struct platform_device *pdev)
>>
>>         raw_spin_lock_init(&host->hw_lock);
>>
>> -       ret = rzg3s_pcie_host_setup(host, rzg3s_pcie_intx_setup,
>> +       ret = rzg3s_pcie_host_setup(host, NULL,
>>                                     rzg3s_pcie_msi_enable, true);
>>         if (ret)
>>                 return ret;
>>
>>
>> With this, of_irq_parse_pci() no longer fails with -22:
>>
>>
>> [    0.564106] pci 0000:00:00.0: [1912:0033] type 01 class 0x060400 PCIe
>> Root Port
>> [    0.564173] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> [    0.564212] pci 0000:00:00.0:   bridge window [mem 0xfff00000-0xffffffff]
>> [    0.564252] pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff
>> 64bit pref]
>> [    0.564355] pci 0000:00:00.0: PME# supported from D0 D3hot
>> [    0.564999] /soc/pcie@11e40000/pci@0,0: Fixed dependency cycle(s) with
>> /soc/pcie@11e40000/pci@0,0
>> [    0.567407] pci 0000:01:00.0: [1d79:2263] type 00 class 0x010802 PCIe
>> Endpoint
>> [    0.567483] pci_bus 0000:01: 2-byte config write to 0000:01:00.0 offset
>> 0x4 may corrupt adjacent RW1C bits
>> [    0.567922] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00003fff 64bit]
>> [    0.568870] pci 0000:01:00.0: 4.000 Gb/s available PCIe bandwidth,
>> limited by 5.0 GT/s PCIe x1 link at 0000:00:00.0 (capable of 15.752 Gb/s
>> with 8.0 GT/s PCIe x2 link)
>> [    0.569599] pci 0000:00:00.0: bridge window [mem 0x30000000-0x300fffff]:
>> assigned
>> [    0.569657] pci 0000:01:00.0: BAR 0 [mem 0x30000000-0x30003fff 64bit]:
>> assigned
>> [    0.569785] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> [    0.569822] pci 0000:00:00.0:   bridge window [mem 0x30000000-0x300fffff]
>> [    0.570056] pci_bus 0000:00: resource 4 [mem 0x30000000-0x37ffffff]
>> [    0.570095] pci_bus 0000:01: resource 1 [mem 0x30000000-0x300fffff]
>> [    0.570311] pci_assign_irq(): pin=0
>> [    0.570338] assign IRQ: got 0
>> [    0.570373] pcieport 0000:00:00.0: enabling device (0000 -> 0002)
>> [    0.570775] pci_assign_irq(): pin=1
>> [    0.570805] pci_assign_irq(): swizzle_irq=806c2b70, map_irq=806dab28
>> [    0.570834] pci_common_swizzle(): pin=1
>> [    0.570855] pci_assign_irq(): slot=0, pin=1
>> [    0.570888] of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001
>> [    0.570954] OF: of_irq_parse_raw: ipar=/soc/pcie@11e40000/pci@0,0, size=1
>> [    0.570997] OF:  -> addrsize=3
>> [    0.571028] OF:  -> match=1 (imaplen=32)
>> [    0.571053] OF:  -> intsize=1, addrsize=3
>> [    0.571075] OF:  -> imaplen=31
>> [    0.571095] OF: /soc/pcie@11e40000/pci@0,0 interrupt-map entry to self
>> [    0.571270] assign IRQ: got 46
>> [    0.571998] nvme nvme0: pci function 0000:01:00.0
>> [    0.572082] nvme 0000:01:00.0: enabling device (0000 -> 0002)
>> [    0.575619] nvme nvme0: missing or invalid SUBNQN field.
>> [    0.584554] nvme nvme0: 1/0/0 default/read/poll queues
>> [    0.592958]  nvme0n1: p1
>>
>>
>> Could you please let me know if this is how the PCIe controller should now
>> be described in DT with your patch?
> 
> In my series, no node should be created if a node is already existing.
>   https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L764
>   https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L771
> 
> The idea is to create the missing node which can be used as PCI device node
> parent.
> 
> On my side, I have performed tests using an Armada 3720 board and so, I used
> this DT:
>   https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> and more precisely, this PCIe controller node description:
>   https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/marvell/armada-37xx.dtsi#L495

This driver along with its device tree uses a dedicated node to expose the
legacy interrupt controller:
https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/marvell/armada-37xx.dtsi#L524

If I'm exposing the interrupt controller the same way (as proposed here:
https://lore.kernel.org/all/20250430103236.3511989-7-claudiu.beznea.uj@bp.renesas.com/)
it works for me as well w/ and w/o PCI_DYNAMIC_OF_NODES.

> 
> On this system, I didn't observed any issues but of course, the PCIe drivers are
> different.
> Also, on my system, no node were created by of_pci_make_host_bridge_node().

Sorry for the confusion, it is of_pci_make_dev_node() on my side which
creates the node.

> 
> To be honest, I didn't re-test recently to see if something has been broken.
> I can do that on my side with my system.
> 
> On your side, maybe you can have look at the Armada PCIe driver and see if
> something could explain your behavior. I am not sure that you need to add the
> pci@0,0 node in your DT.

I can't find a driver that uses the approach I'm trying in my patches. This
approach was suggested in the review process [2] by Rob who mentioned that
now we should be able drop legacy interrupt controller nodes. There are
some Apple device trees that points the interrupt-map to the port node (the
way I tried in my workaround) [3], but I can't find more than that.

The topology in my case is:

root@smarc-rzg3s:~# lspci -t
-[0000:00]---00.0-[01]----00.0

root@smarc-rzg3s:~# lspci
00:00.0 PCI bridge: Renesas Technology Corp. Device 0033
01:00.0 Non-Volatile memory controller: Micron Technology Inc 2550 NVMe SSD
(DRAM-less) (rev 01)

When not working pci@0,0 is exported as follows in rootfs:

root@smarc-rzg3s:~# ls /sys/firmware/devicetree/base/soc/pcie@11e40000 -l
-r--r--r--    1 root     root             4 Jan 12 10:28 #address-cells
-r--r--r--    1 root     root             4 Jan 12 10:28 #interrupt-cells
-r--r--r--    1 root     root             4 Jan 12 10:28 #size-cells
-r--r--r--    1 root     root             8 Jan 12 10:28 bus-range
-r--r--r--    1 root     root            13 Jan 12 10:28 clock-names
-r--r--r--    1 root     root            24 Jan 12 10:28 clocks
-r--r--r--    1 root     root            26 Jan 12 10:28 compatible
-r--r--r--    1 root     root             4 Jan 12 10:28 device-id
-r--r--r--    1 root     root             4 Jan 12 10:28 device_type
-r--r--r--    1 root     root            28 Jan 12 10:28 dma-ranges
-r--r--r--    1 root     root             0 Jan 12 10:28 interrupt-controller
-r--r--r--    1 root     root           144 Jan 12 10:28 interrupt-map
-r--r--r--    1 root     root            16 Jan 12 10:28 interrupt-map-mask
-r--r--r--    1 root     root           164 Jan 12 10:28 interrupt-names
-r--r--r--    1 root     root             4 Jan 12 10:28 interrupt-parrent
-r--r--r--    1 root     root           192 Jan 12 10:28 interrupts
-r--r--r--    1 root     root             5 Jan 12 10:28 name
-r--r--r--    1 root     root             4 Jan 12 10:28 num-lanes
drwxr-xr-x    2 root     root             0 Jan 12 10:17 pci@0,0
-r--r--r--    1 root     root             4 Jan 12 10:28 phandle
-r--r--r--    1 root     root             4 Jan 12 10:28 pinctrl-0
-r--r--r--    1 root     root             8 Jan 12 10:28 pinctrl-names
-r--r--r--    1 root     root             4 Jan 12 10:28 power-domains
-r--r--r--    1 root     root            28 Jan 12 10:28 ranges
-r--r--r--    1 root     root            16 Jan 12 10:28 reg
-r--r--r--    1 root     root             4 Jan 12 10:28 renesas,sysc
-r--r--r--    1 root     root            63 Jan 12 10:28 reset-names
-r--r--r--    1 root     root            56 Jan 12 10:28 resets
-r--r--r--    1 root     root             5 Jan 12 10:28 status
-r--r--r--    1 root     root             4 Jan 12 10:28 vendor-id
root@smarc-rzg3s:~#
root@smarc-rzg3s:~# ls
/sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0 -l
-r--r--r--    1 root     root             4 Jan 12 10:17 #address-cells
-r--r--r--    1 root     root             4 Jan 12 10:17 #interrupt-cells
-r--r--r--    1 root     root             4 Jan 12 10:17 #size-cells
-r--r--r--    1 root     root             8 Jan 12 10:17 bus-range
-r--r--r--    1 root     root            41 Jan 12 10:17 compatible
-r--r--r--    1 root     root             4 Jan 12 10:17 device_type
-r--r--r--    1 root     root           144 Jan 12 10:17 interrupt-map
-r--r--r--    1 root     root            16 Jan 12 10:17 interrupt-map-mask
-r--r--r--    1 root     root            32 Jan 12 10:17 ranges
-r--r--r--    1 root     root            20 Jan 12 10:17 reg
root@smarc-rzg3s:~#
root@smarc-rzg3s:~#
root@smarc-rzg3s:~#
root@smarc-rzg3s:~#
root@smarc-rzg3s:~# cat
/sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0/compatible
pci1912,33pciclass,060400pciclass,0604root@smarc-rzg3s:~#
root@smarc-rzg3s:~#
root@smarc-rzg3s:~#

In case I describe a port in device tree, it works because the pci@0,0 is
not created anymore when device is enumerated and thus the interrupt
parsing is working.

Herve: do you have some hints?

Rob: do you know some device trees where the interrupt-map points to the
node itself as suggested in [2] so that I can check is something is missing
on my side?

Thank you,
Claudiu

[2] https://lore.kernel.org/all/20250509210800.GB4080349-robh@kernel.org/
[3]
https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/apple/t8112.dtsi#L951

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Herve Codina]

Hi Claudiu,

On Fri, 13 Jun 2025 16:36:16 +0300
Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:

...

> I pointed to the wrong function. It's not of_pci_make_host_bridge_node()
> [1] but of_pci_make_dev_node() which creates a node with a similar naming
> and makes things not working on my side.
> 
> [1] https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L694

Ok, so your issue is not related patches applied from the "PCI: of: Create
device-tree PCI host bridge node" series.
  https://lore.kernel.org/all/20250224141356.36325-6-herve.codina@bootlin.com/

Indeed, this series add the node creation for the host bridge with
of_pci_make_host_bridge_node() but you pointed now of_pci_make_dev_node()
which is the creation for PCI device node and this function was not modify by the
series.

of_pci_make_host_bridge_node() should not create anything. Can you confirm on your
side that it doesn't create any nodes.

If so, maybe the problem comes from of_irq_parse_raw() or similar.

...

> 
> > 
> > On this system, I didn't observed any issues but of course, the PCIe drivers are
> > different.
> > Also, on my system, no node were created by of_pci_make_host_bridge_node().  
> 
> Sorry for the confusion, it is of_pci_make_dev_node() on my side which
> creates the node.
> 
> > 
> > To be honest, I didn't re-test recently to see if something has been broken.
> > I can do that on my side with my system.

I have re-tested and I confirm that I have no issue on my system.

> > 
> > On your side, maybe you can have look at the Armada PCIe driver and see if
> > something could explain your behavior. I am not sure that you need to add the
> > pci@0,0 node in your DT.  
> 
> I can't find a driver that uses the approach I'm trying in my patches. This
> approach was suggested in the review process [2] by Rob who mentioned that
> now we should be able drop legacy interrupt controller nodes. There are
> some Apple device trees that points the interrupt-map to the port node (the
> way I tried in my workaround) [3], but I can't find more than that.
> 
> The topology in my case is:
> 
> root@smarc-rzg3s:~# lspci -t
> -[0000:00]---00.0-[01]----00.0
> 
> root@smarc-rzg3s:~# lspci
> 00:00.0 PCI bridge: Renesas Technology Corp. Device 0033
> 01:00.0 Non-Volatile memory controller: Micron Technology Inc 2550 NVMe SSD
> (DRAM-less) (rev 01)
> 
> When not working pci@0,0 is exported as follows in rootfs:
> 
> root@smarc-rzg3s:~# ls /sys/firmware/devicetree/base/soc/pcie@11e40000 -l
> -r--r--r--    1 root     root             4 Jan 12 10:28 #address-cells
> -r--r--r--    1 root     root             4 Jan 12 10:28 #interrupt-cells
> -r--r--r--    1 root     root             4 Jan 12 10:28 #size-cells
> -r--r--r--    1 root     root             8 Jan 12 10:28 bus-range
> -r--r--r--    1 root     root            13 Jan 12 10:28 clock-names
> -r--r--r--    1 root     root            24 Jan 12 10:28 clocks
> -r--r--r--    1 root     root            26 Jan 12 10:28 compatible
> -r--r--r--    1 root     root             4 Jan 12 10:28 device-id
> -r--r--r--    1 root     root             4 Jan 12 10:28 device_type
> -r--r--r--    1 root     root            28 Jan 12 10:28 dma-ranges
> -r--r--r--    1 root     root             0 Jan 12 10:28 interrupt-controller
> -r--r--r--    1 root     root           144 Jan 12 10:28 interrupt-map
> -r--r--r--    1 root     root            16 Jan 12 10:28 interrupt-map-mask
> -r--r--r--    1 root     root           164 Jan 12 10:28 interrupt-names
> -r--r--r--    1 root     root             4 Jan 12 10:28 interrupt-parrent

Why parrent instead of parent in interrupt-parrent ?

> -r--r--r--    1 root     root           192 Jan 12 10:28 interrupts
> -r--r--r--    1 root     root             5 Jan 12 10:28 name
> -r--r--r--    1 root     root             4 Jan 12 10:28 num-lanes
> drwxr-xr-x    2 root     root             0 Jan 12 10:17 pci@0,0
> -r--r--r--    1 root     root             4 Jan 12 10:28 phandle
> -r--r--r--    1 root     root             4 Jan 12 10:28 pinctrl-0
> -r--r--r--    1 root     root             8 Jan 12 10:28 pinctrl-names
> -r--r--r--    1 root     root             4 Jan 12 10:28 power-domains
> -r--r--r--    1 root     root            28 Jan 12 10:28 ranges
> -r--r--r--    1 root     root            16 Jan 12 10:28 reg
> -r--r--r--    1 root     root             4 Jan 12 10:28 renesas,sysc
> -r--r--r--    1 root     root            63 Jan 12 10:28 reset-names
> -r--r--r--    1 root     root            56 Jan 12 10:28 resets
> -r--r--r--    1 root     root             5 Jan 12 10:28 status
> -r--r--r--    1 root     root             4 Jan 12 10:28 vendor-id
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~# ls
> /sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0 -l
> -r--r--r--    1 root     root             4 Jan 12 10:17 #address-cells
> -r--r--r--    1 root     root             4 Jan 12 10:17 #interrupt-cells
> -r--r--r--    1 root     root             4 Jan 12 10:17 #size-cells
> -r--r--r--    1 root     root             8 Jan 12 10:17 bus-range
> -r--r--r--    1 root     root            41 Jan 12 10:17 compatible
> -r--r--r--    1 root     root             4 Jan 12 10:17 device_type
> -r--r--r--    1 root     root           144 Jan 12 10:17 interrupt-map
> -r--r--r--    1 root     root            16 Jan 12 10:17 interrupt-map-mask
> -r--r--r--    1 root     root            32 Jan 12 10:17 ranges
> -r--r--r--    1 root     root            20 Jan 12 10:17 reg
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~# cat
> /sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0/compatible
> pci1912,33pciclass,060400pciclass,0604root@smarc-rzg3s:~#
> root@smarc-rzg3s:~#
> root@smarc-rzg3s:~#
> 
> In case I describe a port in device tree, it works because the pci@0,0 is
> not created anymore when device is enumerated and thus the interrupt
> parsing is working.
> 
> Herve: do you have some hints?

First interrupt-parrent in your /sys/firmware/devicetree/base/soc/pcie@11e40000
files.

If it is just a typo in this email, maybe the interrupt parsing itself.

Can you provide an extract for the DT with nodes created at runtime.
I mean can you run 'dtc -I dtb -O dts /proc/device-tree' and provide the output
related to PCI nodes including the PCIe controller ?

> 
> Rob: do you know some device trees where the interrupt-map points to the
> node itself as suggested in [2] so that I can check is something is missing
> on my side?
> 
> Thank you,
> Claudiu
> 
> [2] https://lore.kernel.org/all/20250509210800.GB4080349-robh@kernel.org/
> [3]
> https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/apple/t8112.dtsi#L951
> 

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v8 5/5] PCI: of: Create device-tree PCI host bridge node

[Claudiu Beznea]

Hi, Herve, Rob, Bjorn, Lizhi,

On 17.06.2025 10:00, Herve Codina wrote:
> Hi Claudiu,
> 
> On Fri, 13 Jun 2025 16:36:16 +0300
> Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
> 
> ...
> 
>> I pointed to the wrong function. It's not of_pci_make_host_bridge_node()
>> [1] but of_pci_make_dev_node() which creates a node with a similar naming
>> and makes things not working on my side.
>>
>> [1] https://elixir.bootlin.com/linux/v6.15/source/drivers/pci/of.c#L694
> 
> Ok, so your issue is not related patches applied from the "PCI: of: Create
> device-tree PCI host bridge node" series.
>   https://lore.kernel.org/all/20250224141356.36325-6-herve.codina@bootlin.com/

That's true, I was wrongly pointing it.

> 
> Indeed, this series add the node creation for the host bridge with
> of_pci_make_host_bridge_node() but you pointed now of_pci_make_dev_node()
> which is the creation for PCI device node and this function was not modify by the
> series.
> 
> of_pci_make_host_bridge_node() should not create anything. Can you confirm on your
> side that it doesn't create any nodes.

I confirm it doesn't create any node.

> 
> If so, maybe the problem comes from of_irq_parse_raw() or similar.
> 
> ...
> 
>>
>>>
>>> On this system, I didn't observed any issues but of course, the PCIe drivers are
>>> different.
>>> Also, on my system, no node were created by of_pci_make_host_bridge_node().  
>>
>> Sorry for the confusion, it is of_pci_make_dev_node() on my side which
>> creates the node.
>>
>>>
>>> To be honest, I didn't re-test recently to see if something has been broken.
>>> I can do that on my side with my system.
> 
> I have re-tested and I confirm that I have no issue on my system.

Thank you for doing this.

> 
>>>
>>> On your side, maybe you can have look at the Armada PCIe driver and see if
>>> something could explain your behavior. I am not sure that you need to add the
>>> pci@0,0 node in your DT.  
>>
>> I can't find a driver that uses the approach I'm trying in my patches. This
>> approach was suggested in the review process [2] by Rob who mentioned that
>> now we should be able drop legacy interrupt controller nodes. There are
>> some Apple device trees that points the interrupt-map to the port node (the
>> way I tried in my workaround) [3], but I can't find more than that.
>>
>> The topology in my case is:
>>
>> root@smarc-rzg3s:~# lspci -t
>> -[0000:00]---00.0-[01]----00.0
>>
>> root@smarc-rzg3s:~# lspci
>> 00:00.0 PCI bridge: Renesas Technology Corp. Device 0033
>> 01:00.0 Non-Volatile memory controller: Micron Technology Inc 2550 NVMe SSD
>> (DRAM-less) (rev 01)
>>
>> When not working pci@0,0 is exported as follows in rootfs:
>>
>> root@smarc-rzg3s:~# ls /sys/firmware/devicetree/base/soc/pcie@11e40000 -l
>> -r--r--r--    1 root     root             4 Jan 12 10:28 #address-cells
>> -r--r--r--    1 root     root             4 Jan 12 10:28 #interrupt-cells
>> -r--r--r--    1 root     root             4 Jan 12 10:28 #size-cells
>> -r--r--r--    1 root     root             8 Jan 12 10:28 bus-range
>> -r--r--r--    1 root     root            13 Jan 12 10:28 clock-names
>> -r--r--r--    1 root     root            24 Jan 12 10:28 clocks
>> -r--r--r--    1 root     root            26 Jan 12 10:28 compatible
>> -r--r--r--    1 root     root             4 Jan 12 10:28 device-id
>> -r--r--r--    1 root     root             4 Jan 12 10:28 device_type
>> -r--r--r--    1 root     root            28 Jan 12 10:28 dma-ranges
>> -r--r--r--    1 root     root             0 Jan 12 10:28 interrupt-controller
>> -r--r--r--    1 root     root           144 Jan 12 10:28 interrupt-map
>> -r--r--r--    1 root     root            16 Jan 12 10:28 interrupt-map-mask
>> -r--r--r--    1 root     root           164 Jan 12 10:28 interrupt-names
>> -r--r--r--    1 root     root             4 Jan 12 10:28 interrupt-parrent
> 
> Why parrent instead of parent in interrupt-parrent ?

Sorry about that, I did the wrong copy-paste here. This was from my various
experiments (and yes, it was a typo in there in my device tree). The point
I was trying to emphasize here is about the presence of the pci@0,0 node
and its content, along with the pci topology (considering this might give
you some clue).

> 
>> -r--r--r--    1 root     root           192 Jan 12 10:28 interrupts
>> -r--r--r--    1 root     root             5 Jan 12 10:28 name
>> -r--r--r--    1 root     root             4 Jan 12 10:28 num-lanes
>> drwxr-xr-x    2 root     root             0 Jan 12 10:17 pci@0,0
>> -r--r--r--    1 root     root             4 Jan 12 10:28 phandle
>> -r--r--r--    1 root     root             4 Jan 12 10:28 pinctrl-0
>> -r--r--r--    1 root     root             8 Jan 12 10:28 pinctrl-names
>> -r--r--r--    1 root     root             4 Jan 12 10:28 power-domains
>> -r--r--r--    1 root     root            28 Jan 12 10:28 ranges
>> -r--r--r--    1 root     root            16 Jan 12 10:28 reg
>> -r--r--r--    1 root     root             4 Jan 12 10:28 renesas,sysc
>> -r--r--r--    1 root     root            63 Jan 12 10:28 reset-names
>> -r--r--r--    1 root     root            56 Jan 12 10:28 resets
>> -r--r--r--    1 root     root             5 Jan 12 10:28 status
>> -r--r--r--    1 root     root             4 Jan 12 10:28 vendor-id
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~# ls
>> /sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0 -l
>> -r--r--r--    1 root     root             4 Jan 12 10:17 #address-cells
>> -r--r--r--    1 root     root             4 Jan 12 10:17 #interrupt-cells
>> -r--r--r--    1 root     root             4 Jan 12 10:17 #size-cells
>> -r--r--r--    1 root     root             8 Jan 12 10:17 bus-range
>> -r--r--r--    1 root     root            41 Jan 12 10:17 compatible
>> -r--r--r--    1 root     root             4 Jan 12 10:17 device_type
>> -r--r--r--    1 root     root           144 Jan 12 10:17 interrupt-map
>> -r--r--r--    1 root     root            16 Jan 12 10:17 interrupt-map-mask
>> -r--r--r--    1 root     root            32 Jan 12 10:17 ranges
>> -r--r--r--    1 root     root            20 Jan 12 10:17 reg
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~# cat
>> /sys/firmware/devicetree/base/soc/pcie@11e40000/pci@0,0/compatible
>> pci1912,33pciclass,060400pciclass,0604root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~#
>> root@smarc-rzg3s:~#
>>
>> In case I describe a port in device tree, it works because the pci@0,0 is
>> not created anymore when device is enumerated and thus the interrupt
>> parsing is working.
>>
>> Herve: do you have some hints?
> 
> First interrupt-parrent in your /sys/firmware/devicetree/base/soc/pcie@11e40000
> files.
> 
> If it is just a typo in this email, maybe the interrupt parsing itself.
> 
> Can you provide an extract for the DT with nodes created at runtime.
> I mean can you run 'dtc -I dtb -O dts /proc/device-tree' and provide the output

That was a good hint. Thank for it.

> related to PCI nodes including the PCIe controller ?

I continued yesterday to investigate this.

To me it looks like there is an issue with interrupt-map property for the
node created with of_pci_make_dev_node(), whose content is populated with
data retrieved after the interrupt was checked to be a valid map.

With the current linux-next and the driver I'm working on [1] the pci dts
node obtained from /proc/device-tree is as follows:

pcie@11e40000 {
    power-domains = <0x02>;
    dma-ranges = <0x42000000 0x00 0x48000000 0x00 0x48000000 0x00 0x38000000>;
    pinctrl-names = "default";
    #address-cells = <0x03>;
    bus-range = <0x00 0xff>;
    pinctrl-0 = <0x21>;
    clock-names = "aclk\0pm";
    resets = <0x02 0x53 0x02 0x54 0x02 0x55 0x02 0x56 0x02 0x57 0x02 0x58
0x02 0x59>;
    interrupts = <0x00 0x18b 0x04 0x00 0x18c 0x04 0x00 0x18d 0x04 0x00
0x18e 0x04 0x00 0x18f 0x04 0x00 0x190 0x04 0x00 0x191 0x04 0x00 0x192 0x04
0x00 0x193 0x04 0x00 0x194 0x04 0x00 0x195 0x04 0x00 0x196 0x04 0x00 0x197
0x04 0x00 0x198 0x04 0x00 0x199 0x04 0x00 0x19a 0x04>;
    clocks = <0x02 0x01 0x65 0x02 0x01 0x66>;
    interrupt-map = <0x00 0x00 0x00 0x01 0x1f 0x00 0x00 0x00 0x00
                     0x00 0x00 0x00 0x02 0x1f 0x00 0x00 0x00 0x01
                     0x00 0x00 0x00 0x03 0x1f 0x00 0x00 0x00 0x02
                     0x00 0x00 0x00 0x04 0x1f 0x00 0x00 0x00 0x03>;
    #size-cells = <0x02>;
    renesas,sysc = <0x20>;
    device_type = "pci";
    interrupt-map-mask = <0x00 0x00 0x00 0x07>;
    num-lanes = <0x01>;
    compatible = "renesas,r9a08g045s33-pcie";
    ranges = <0x3000000 0x00 0x30000000 0x00 0x30000000 0x00 0x8000000>;
    #interrupt-cells = <0x01>;
    status = "okay";
    vendor-id = <0x1912>;
    interrupt-names =
"serr\0serr_cor\0serr_nonfatal\0serr_fatal\0axi_err\0inta\0intb\0intc\0intd\0msi\0link_bandwidth\0pm_pme\0dma\0pcie_evt\0msg\0all";
    reg = <0x00 0x11e40000 0x00 0x10000>;
    phandle = <0x1f>;
    reset-names =
"aresetn\0rst_b\0rst_gp_b\0rst_ps_b\0rst_rsm_b\0rst_cfg_b\0rst_load_b";
    device-id = <0x33>;
    interrupt-controller;

    pci@0,0 {
        #address-cells = <0x03>;
        bus-range = <0x01 0xff>;
        interrupt-map = <0x10000 0x00 0x00 0x01 0x1f 0x00 0x11e40000 0x00 0x00
                         0x10000 0x00 0x00 0x02 0x1f 0x00 0x11e40000 0x00 0x01
                         0x10000 0x00 0x00 0x03 0x1f 0x00 0x11e40000 0x00 0x02
                         0x10000 0x00 0x00 0x04 0x1f 0x00 0x11e40000 0x00
0x03>;
        #size-cells = <0x02>;
        device_type = "pci";
        interrupt-map-mask = <0xffff00 0x00 0x00 0x07>;
        compatible = "pci1912,33\0pciclass,060400\0pciclass,0604";
        ranges = <0x82000000 0x00 0x30000000 0x82000000 0x00 0x30000000
0x00 0x100000>;
        #interrupt-cells = <0x01>;
        reg = <0x00 0x00 0x00 0x00 0x00>;
    };
};

The pci@0,0 is created by:

pci_host_probe() ->
  pci_bus_add_devices() ->
    pci_bus_add_device() ->
      of_pci_make_dev_node() ->
        of_pci_add_properties() ->
          of_pci_prop_intr_map()

for bridge->child_ops populated in rzg3s_pcie_probe() (child->ops in
pci_bus_add_devices() is bridge->child_ops populated in rzg3s_pcie_probe()).

static int rzg3s_pcie_probe(struct platform_device *dev)
{
	// ...

        bridge->sysdata = host;

        bridge->ops = &rzg3s_pcie_root_ops;

        bridge->child_ops = &rzg3s_pcie_child_ops;

        ret = pci_host_probe(bridge);


	// ...
}

On my side, on child bus is connected a NVMe device.

When it is enumerated pci_assign_irq() is called for it, call trace as follows:

[    0.599711]  of_irq_parse_and_map_pci+0x1e4/0x284
[    0.599727]  pci_assign_irq+0x130/0x158
[    0.599743]  pci_device_probe+0x5c/0x234
[    0.599757]  really_probe+0xbc/0x2a0
[    0.599771]  __driver_probe_device+0x78/0x12c
[    0.599786]  driver_probe_device+0x40/0x160
[    0.599800]  __device_attach_driver+0xb8/0x134
[    0.599815]  bus_for_each_drv+0x80/0xdc
[    0.599835]  __device_attach+0xa8/0x1b0
[    0.599849]  device_attach+0x14/0x20
[    0.599863]  pci_bus_add_device+0xec/0x198
[    0.599882]  pci_bus_add_devices+0x38/0x84
[    0.599900]  pci_bus_add_devices+0x64/0x84
[    0.599919]  pci_host_probe+0x90/0x108
[    0.599932]  rzg3s_pcie_probe+0x3c8/0x4f0

Activating the of_print_phandle_args("of_irq_parse_raw: ", out_irq) debug
from of_irq_parse_raw() prints the following:

of_irq_parse_raw:  /soc/pcie@11e40000/pci@0,0:00000001

requesting INTA.

From this I understand the interrupt is requested from the pci@0,0 node
created by of_pci_make_dev_node() for child bus device. This matches the
1st interrupt-map of the pci@0,0 node:

interrupt-map = <0x10000 0x00 0x00 0x01 0x1f 0x00 0x11e40000 0x00 0x00>

and then the of_irq_parse_raw() goes to the node with phandle 0x1f
(pcie@11e40000) and tries to find a map for IRQ "0x00 0x11e40000 0x00 0x00"
entry but this one is not there.

Updating the interrupt-map property of pcie@11e40000 node by adding
<0x00 0x00 0x00 0x00 0x1f 0x00 0x00 0x00 0x00> entry solves the issue I'm
seeing:

    interrupt-map = <0x00 0x00 0x00 0x00 0x1f 0x00 0x00 0x00 0x00
                     0x00 0x00 0x00 0x01 0x1f 0x00 0x00 0x00 0x00
                     0x00 0x00 0x00 0x02 0x1f 0x00 0x00 0x00 0x01
                     0x00 0x00 0x00 0x03 0x1f 0x00 0x00 0x00 0x02
                     0x00 0x00 0x00 0x04 0x1f 0x00 0x00 0x00 0x03>;

The code that updates the interrupt map for the node created by
of_pci_make_dev_node() is of_pci_prop_intr_map(). This looks in the IRQ
mapping tree for an INTx interrupt match and looks it up to the parent node
than can provide this interrupt. If a match is found it returns the match
for the node that can provide the interrupt. And this information is used
to populate the interrupt-map property of the node that can is created by
of_pci_make_dev_node().

The following diff I tried solves the problem I see:

diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 506fcd507113..7d7f469a1db6 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -243,6 +243,10 @@ static int of_pci_prop_intr_map(struct pci_dev *pdev,
struct of_changeset *ocs,
                }
                of_property_read_u32(out_irq[i].np, "#address-cells",
                                     &addr_sz[i]);
+               /* Restore the arguments of the next level parent if a map
was found. */
+               out_irq[i].np = pnode;
+               out_irq[i].args_count = 1;
+               out_irq[i].args[0] = pin;
        }

        list_for_each_entry(child, &pdev->subordinate->devices, bus_list) {

With this, the live pcie device tree node is as follows:

pcie@11e40000 {
    power-domains = <0x02>;
    dma-ranges = <0x42000000 0x00 0x48000000 0x00 0x48000000 0x00 0x38000000>;
    pinctrl-names = "default";
    #address-cells = <0x03>;
    bus-range = <0x00 0xff>;
    pinctrl-0 = <0x21>;
    clock-names = "aclk\0pm";
    resets = <0x02 0x53 0x02 0x54 0x02 0x55 0x02 0x56 0x02 0x57 0x02 0x58
0x02 0x59>;
    interrupts = <0x00 0x18b 0x04 0x00 0x18c 0x04 0x00 0x18d 0x04 0x00
0x18e 0x04 0x00 0x18f 0x04 0x00 0x190 0x04 0x00 0x191 0x04 0x00 0x192 0x04
0x00 0x193 0x04 0x00 0x194 0x04 0x00 0x195 0x04 0x00 0x196 0x04 0x00 0x197
0x04 0x00 0x198 0x04 0x00 0x199 0x04 0x00 0x19a 0x04>;
    clocks = <0x02 0x01 0x65 0x02 0x01 0x66>;
    interrupt-map = <0x00 0x00 0x00 0x01 0x1f 0x00 0x00 0x00 0x00
                     0x00 0x00 0x00 0x02 0x1f 0x00 0x00 0x00 0x01
                     0x00 0x00 0x00 0x03 0x1f 0x00 0x00 0x00 0x02
                     0x00 0x00 0x00 0x04 0x1f 0x00 0x00 0x00 0x03>;
    #size-cells = <0x02>;
    renesas,sysc = <0x20>;
    device_type = "pci";
    interrupt-map-mask = <0x00 0x00 0x00 0x07>;
    num-lanes = <0x01>;
    compatible = "renesas,r9a08g045s33-pcie";
    ranges = <0x3000000 0x00 0x30000000 0x00 0x30000000 0x00 0x8000000>;
    #interrupt-cells = <0x01>;
    status = "okay";
    vendor-id = <0x1912>;
    interrupt-names =
"serr\0serr_cor\0serr_nonfatal\0serr_fatal\0axi_err\0inta\0intb\0intc\0intd\0msi\0link_bandwidth\0pm_pme\0dma\0pcie_evt\0msg\0all";
    reg = <0x00 0x11e40000 0x00 0x10000>;
    phandle = <0x1f>;
    reset-names =
"aresetn\0rst_b\0rst_gp_b\0rst_ps_b\0rst_rsm_b\0rst_cfg_b\0rst_load_b";
    device-id = <0x33>;
    interrupt-controller;

    pci@0,0 {
        #address-cells = <0x03>;
        bus-range = <0x01 0xff>;
        interrupt-map = <0x10000 0x00 0x00 0x01 0x1f 0x00 0x11e40000 0x00 0x01
                         0x10000 0x00 0x00 0x02 0x1f 0x00 0x11e40000 0x00 0x02
                         0x10000 0x00 0x00 0x03 0x1f 0x00 0x11e40000 0x00 0x03
                         0x10000 0x00 0x00 0x04 0x1f 0x00 0x11e40000 0x00
0x04>;
        #size-cells = <0x02>;
        device_type = "pci";
        interrupt-map-mask = <0xffff00 0x00 0x00 0x07>;
        compatible = "pci1912,33\0pciclass,060400\0pciclass,0604";
        ranges = <0x82000000 0x00 0x30000000 0x82000000 0x00 0x30000000
0x00 0x100000>;
        #interrupt-cells = <0x01>;
        reg = <0x00 0x00 0x00 0x00 0x00>;
    };
};

Note the interrupt-map property of pci@0,0 changes.

This started to reproduce on my side after the CONFIG_PCI_DYNAMIC_OF_NODES
was enabled in ARM64 defconfig through commits:

b8e22cf599d1 ("arm64: defconfig: Enable OF_OVERLAY option")
10c68f40b86e ("arm64: defconfig: Enable RP1 misc/clock/gpio drivers")

Rob, Bjorn, Lizhi, PCI experts,

Can you please let me know your input on this?

Do you consider there is something wrong with the driver I'm working on
(series [1])?

Thank you for your support,
Claudiu

[1]
https://lore.kernel.org/all/20250530111917.1495023-1-claudiu.beznea.uj@bp.renesas.com

>>
>> Rob: do you know some device trees where the interrupt-map points to the
>> node itself as suggested in [2] so that I can check is something is missing
>> on my side?
>>
>> Thank you,
>> Claudiu
>>
>> [2] https://lore.kernel.org/all/20250509210800.GB4080349-robh@kernel.org/
>> [3]
>> https://elixir.bootlin.com/linux/v6.15/source/arch/arm64/boot/dts/apple/t8112.dtsi#L951
>>
> 
> Best regards,
> Hervé
>