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

[PATCH v7 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 v7 series is the v6 series rebased
on top of v6.14-rc1 without other modifications.

Best regards,
Hervé Codina

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.47.1

[PATCH v7 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>
---
 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.47.1

[PATCH v7 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>
---
 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.47.1

[PATCH v7 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>
---
 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.47.1

[PATCH v7 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>
---
 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.47.1

[PATCH v7 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.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 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.47.1

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

[Herve Codina]

Hi Greg, Bjorn,


I took into account feedback received on v5 and sent the v6 on 01/10/2025.
I didn't receive any feedback on the v6. In the meantime, the kernel
v6.14-rc1 has been released. I rebased the v6 and sent the v7 on 12/09/2024.

Is there something blocking in this series from having feedback or having
the series applied?

Best regards,
Hervé

On Tue,  4 Feb 2025 08:34:55 +0100
Herve Codina <herve.codina@bootlin.com> 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 v7 series is the v6 series rebased
> on top of v6.14-rc1 without other modifications.
> 
> Best regards,
> Hervé Codina
> 
> 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(-)
> 

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

[Greg Kroah-Hartman]

On Tue, Feb 04, 2025 at 08:34:56AM +0100, Herve Codina wrote:
> 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>
> ---
>  drivers/base/core.c    | 61 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 63 insertions(+)
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

[Greg Kroah-Hartman]

On Wed, Feb 19, 2025 at 09:24:48AM +0100, Herve Codina wrote:
> Hi Greg, Bjorn,
> 
> 
> I took into account feedback received on v5 and sent the v6 on 01/10/2025.
> I didn't receive any feedback on the v6. In the meantime, the kernel
> v6.14-rc1 has been released. I rebased the v6 and sent the v7 on 12/09/2024.
> 
> Is there something blocking in this series from having feedback or having
> the series applied?

Sorry for the delay, the driver core changes look fine to me, now acked.

I'll defer the pci stuff to Bjorn.

thanks,

greg k-h

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

[Jonathan Cameron]

On Tue,  4 Feb 2025 08:34:56 +0100
Herve Codina <herve.codina@bootlin.com> wrote:

> 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>
A few passing comments. Not suggestions to actually change anything
at this stage though. Maybe a potential follow up if you think it's
a good idea.

> ---
>  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;
Maybe use
	struct device *d __free(put_device) = get_device(dev);

	if (!d->of_node);
		return;

Not a reason to respin though!


> +
> +	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);

Likewise could use __free() magic here as well for slight simpliciations.

> +	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)

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

[Bjorn Helgaas]

[cc->to: Rob; I'd like Rob's ack for the PCI parts]

On Tue, Feb 04, 2025 at 08:34:55AM +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 v7 series is the v6 series rebased
> on top of v6.14-rc1 without other modifications.
> 
> Best regards,
> Hervé Codina
> 
> 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.47.1
> 

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

[Bjorn Helgaas]

On Tue, Feb 04, 2025 at 08:35:00AM +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.

This is a detailed low-level description of what this patch does.  Can
we include a high level outline of what the benefit is and why we want
this patch?

Based on 185686beb464 ("misc: Add support for LAN966x PCI device"), I
assume the purpose is to deal with some kind of non-standard PCI
topology, e.g., a single B/D/F function contains several different
pieces of functionality to be driven by several different drivers, and
we build a device tree description of those pieces and then bind those
drivers to the functionality using platform_device interfaces?

Bjorn

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

[Rob Herring]

On Tue, Feb 04, 2025 at 08:34:55AM +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 v7 series is the v6 series rebased
> on top of v6.14-rc1 without other modifications.
> 
> Best regards,
> Hervé Codina
> 
> 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(-)

For the series,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

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

[Herve Codina]

Hi Jonathan,

On Wed, 19 Feb 2025 15:59:01 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

...

> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> A few passing comments. Not suggestions to actually change anything
> at this stage though. Maybe a potential follow up if you think it's
> a good idea.
> 
...

> > +void device_remove_of_node(struct device *dev)
> > +{
> > +	dev = get_device(dev);
> > +	if (!dev)
> > +		return;  
> Maybe use
> 	struct device *d __free(put_device) = get_device(dev);
> 
> 	if (!d->of_node);
> 		return;
> 
> Not a reason to respin though!
> 
> 
...

> > +int device_add_of_node(struct device *dev, struct device_node *of_node)
> > +{
> > +	int ret;
> > +
> > +	if (!of_node)
> > +		return -EINVAL;
> > +
> > +	dev = get_device(dev);  
> 
> Likewise could use __free() magic here as well for slight simpliciations.
> 

I see. Indeed, the __free(put_device) can be an improvement in core.c

I think that this has to be done out of this series in a more globally way
because put_device() is used in several place in this file and having a mix
between __free(put_device) and put_device() calls in a goto label is not the
best solution.

For this reason, as you proposed except if someone else pushes in the
__free(put_device) direction in functions introduced in this patch, I
prefer to keep this patch as it is.

Thanks for your feedback,
Hervé

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

[Herve Codina]

Hi Bjorn,

On Wed, 19 Feb 2025 11:39:12 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Tue, Feb 04, 2025 at 08:35:00AM +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.  
> 
> This is a detailed low-level description of what this patch does.  Can
> we include a high level outline of what the benefit is and why we want
> this patch?
> 
> Based on 185686beb464 ("misc: Add support for LAN966x PCI device"), I
> assume the purpose is to deal with some kind of non-standard PCI
> topology, e.g., a single B/D/F function contains several different
> pieces of functionality to be driven by several different drivers, and
> we build a device tree description of those pieces and then bind those
> drivers to the functionality using platform_device interfaces?
> 

What do you think if I add the following at the end of the commit log?

   With this done, hardware available in complex PCI device can be
   described by a device-tree overlay loaded by the PCI device driver
   on non device-tree based systems. For instance, the LAN966x PCI device
   introduced by commit 185686beb464 ("misc: Add support for LAN966x
   PCI device") can be available on x86 systems.


Best regards,
Hervé

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

[Bjorn Helgaas]

On Thu, Feb 20, 2025 at 09:25:14AM +0100, Herve Codina wrote:
> On Wed, 19 Feb 2025 11:39:12 -0600
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Feb 04, 2025 at 08:35:00AM +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.  
> > 
> > This is a detailed low-level description of what this patch does.  Can
> > we include a high level outline of what the benefit is and why we want
> > this patch?
> > 
> > Based on 185686beb464 ("misc: Add support for LAN966x PCI device"), I
> > assume the purpose is to deal with some kind of non-standard PCI
> > topology, e.g., a single B/D/F function contains several different
> > pieces of functionality to be driven by several different drivers, and
> > we build a device tree description of those pieces and then bind those
> > drivers to the functionality using platform_device interfaces?
> 
> What do you think if I add the following at the end of the commit log?
> 
>    With this done, hardware available in complex PCI device can be
>    described by a device-tree overlay loaded by the PCI device driver
>    on non device-tree based systems. For instance, the LAN966x PCI device
>    introduced by commit 185686beb464 ("misc: Add support for LAN966x
>    PCI device") can be available on x86 systems.

This isn't just about complexity of the device.  There are NICs that
are much more complex.

IIUC this is really about devices that don't follow the standard
"one PCI function <--> one driver" model, so I think it's important to
include something about the case of a single function that includes
several unrelated bits of functionality that require different
drivers.

"LAN966x" might mean something to people who know that this thing has
a half dozen separate things inside it, but the name by itself doesn't
suggest that, so I don't think it's really helpful to the general
audience.

Bjorn

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

[Herve Codina]

Hi Bjorn,

On Thu, 20 Feb 2025 18:07:53 -0600
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Thu, Feb 20, 2025 at 09:25:14AM +0100, Herve Codina wrote:
> > On Wed, 19 Feb 2025 11:39:12 -0600
> > Bjorn Helgaas <helgaas@kernel.org> wrote:  
> > > On Tue, Feb 04, 2025 at 08:35:00AM +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.    
> > > 
> > > This is a detailed low-level description of what this patch does.  Can
> > > we include a high level outline of what the benefit is and why we want
> > > this patch?
> > > 
> > > Based on 185686beb464 ("misc: Add support for LAN966x PCI device"), I
> > > assume the purpose is to deal with some kind of non-standard PCI
> > > topology, e.g., a single B/D/F function contains several different
> > > pieces of functionality to be driven by several different drivers, and
> > > we build a device tree description of those pieces and then bind those
> > > drivers to the functionality using platform_device interfaces?  
> > 
> > What do you think if I add the following at the end of the commit log?
> > 
> >    With this done, hardware available in complex PCI device can be
> >    described by a device-tree overlay loaded by the PCI device driver
> >    on non device-tree based systems. For instance, the LAN966x PCI device
> >    introduced by commit 185686beb464 ("misc: Add support for LAN966x
> >    PCI device") can be available on x86 systems.  
> 
> This isn't just about complexity of the device.  There are NICs that
> are much more complex.
> 
> IIUC this is really about devices that don't follow the standard
> "one PCI function <--> one driver" model, so I think it's important to
> include something about the case of a single function that includes
> several unrelated bits of functionality that require different
> drivers.

Yes.

> 
> "LAN966x" might mean something to people who know that this thing has
> a half dozen separate things inside it, but the name by itself doesn't
> suggest that, so I don't think it's really helpful to the general
> audience.
> 

Does this one at the end of the commit log sound better?

    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.

Best regards,
Hervé

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

[Bjorn Helgaas]

On Fri, Feb 21, 2025 at 09:34:27AM +0100, Herve Codina wrote:
> Hi Bjorn,
> 
> On Thu, 20 Feb 2025 18:07:53 -0600
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> > On Thu, Feb 20, 2025 at 09:25:14AM +0100, Herve Codina wrote:
> > > On Wed, 19 Feb 2025 11:39:12 -0600
> > > Bjorn Helgaas <helgaas@kernel.org> wrote:  
> > > > On Tue, Feb 04, 2025 at 08:35:00AM +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.    
> > > > 
> > > > This is a detailed low-level description of what this patch does.  Can
> > > > we include a high level outline of what the benefit is and why we want
> > > > this patch?
> > > > 
> > > > Based on 185686beb464 ("misc: Add support for LAN966x PCI device"), I
> > > > assume the purpose is to deal with some kind of non-standard PCI
> > > > topology, e.g., a single B/D/F function contains several different
> > > > pieces of functionality to be driven by several different drivers, and
> > > > we build a device tree description of those pieces and then bind those
> > > > drivers to the functionality using platform_device interfaces?  
> > > 
> > > What do you think if I add the following at the end of the commit log?
> > > 
> > >    With this done, hardware available in complex PCI device can be
> > >    described by a device-tree overlay loaded by the PCI device driver
> > >    on non device-tree based systems. For instance, the LAN966x PCI device
> > >    introduced by commit 185686beb464 ("misc: Add support for LAN966x
> > >    PCI device") can be available on x86 systems.  
> > 
> > This isn't just about complexity of the device.  There are NICs that
> > are much more complex.
> > 
> > IIUC this is really about devices that don't follow the standard
> > "one PCI function <--> one driver" model, so I think it's important to
> > include something about the case of a single function that includes
> > several unrelated bits of functionality that require different
> > drivers.
> 
> Yes.
> 
> > 
> > "LAN966x" might mean something to people who know that this thing has
> > a half dozen separate things inside it, but the name by itself doesn't
> > suggest that, so I don't think it's really helpful to the general
> > audience.
> > 
> 
> Does this one at the end of the commit log sound better?
> 
>     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.

Yep, thanks.