| Message ID | 20250204073501.278248-6-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Add support for the PCI host bridge device-tree node creation. | expand |
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
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é
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
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é
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); }
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(-)