Message ID | 1671125446-57584-3-git-send-email-lizhi.hou@amd.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Bjorn Helgaas |
Headers | show |
Series | Generate device tree node for pci devices | expand |
Le Thu, 15 Dec 2022 09:30:45 -0800, Lizhi Hou <lizhi.hou@amd.com> a écrit : > +}; > + > +static int of_pci_prop_device_type(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); > +} > + > +static int of_pci_prop_address_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#address_cells", > + OF_PCI_ADDRESS_CELLS); > +} > + > +static int of_pci_prop_size_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#size_cells", > + OF_PCI_SIZE_CELLS); > +} Hi Lizhi, For all these functions, the "pdev" parameter is actually unused. [snip] > + > +static int of_pci_prop_compatible(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + const char *compat_strs[PROP_COMPAT_NUM] = { 0 }; > + int i, ret; > + > + compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] = > + kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device); Maybe it should be better to use "pci%04x,%04x" to keep the existing naming.
Le Thu, 15 Dec 2022 09:30:45 -0800, Lizhi Hou <lizhi.hou@amd.com> a écrit : > The PCI endpoint device such as Xilinx Alveo PCI card maps the register > spaces from multiple hardware peripherals to its PCI BAR. Normally, > the PCI core discovers devices and BARs using the PCI enumeration process. > There is no infrastructure to discover the hardware peripherals that are > present in a PCI device, and which can be accessed through the PCI BARs. > > For Alveo PCI card, the card firmware provides a flattened device tree to > describe the hardware peripherals on its BARs. The Alveo card driver can > load this flattened device tree and leverage device tree framework to > generate platform devices for the hardware peripherals eventually. > > Apparently, the device tree framework requires a device tree node for the > PCI device. Thus, it can generate the device tree nodes for hardware > peripherals underneath. Because PCI is self discoverable bus, there might > not be a device tree node created for PCI devices. This patch is to add > support to generate device tree node for PCI devices. > > Added a kernel option. When the option is turned on, the kernel will > generate device tree nodes for PCI bridges unconditionally. > > Initially, the basic properties are added for the dynamically generated > device tree nodes. > > Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> > Signed-off-by: Sonal Santan <sonal.santan@amd.com> > Signed-off-by: Max Zhen <max.zhen@amd.com> > Reviewed-by: Brian Xu <brian.xu@amd.com> > --- > drivers/pci/Kconfig | 12 ++ > drivers/pci/Makefile | 1 + > drivers/pci/bus.c | 2 + > drivers/pci/msi/irqdomain.c | 6 +- > drivers/pci/of.c | 71 ++++++++++++ > drivers/pci/of_property.c | 222 ++++++++++++++++++++++++++++++++++++ > drivers/pci/pci-driver.c | 3 +- > drivers/pci/pci.h | 19 +++ > drivers/pci/remove.c | 1 + > 9 files changed, 334 insertions(+), 3 deletions(-) > create mode 100644 drivers/pci/of_property.c > > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig > index 55c028af4bd9..1b2347aa1e5b 100644 > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -198,6 +198,18 @@ config PCI_HYPERV > The PCI device frontend driver allows the kernel to import arbitrary > PCI devices from a PCI backend to support PCI driver domains. > > +config PCI_DYNAMIC_OF_NODES > + bool "Create Devicetree nodes for PCI devices" > + depends on OF > + select OF_DYNAMIC > + help > + This option enables support for generating device tree nodes for some > + PCI devices. Thus, the driver of this kind can load and overlay > + flattened device tree for its downstream devices. > + > + Once this option is selected, the device tree nodes will be generated > + for all PCI bridges. > + > choice > prompt "PCI Express hierarchy optimization setting" > default PCIE_BUS_DEFAULT > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile > index 2680e4c92f0a..cc8b4e01e29d 100644 > --- a/drivers/pci/Makefile > +++ b/drivers/pci/Makefile > @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o > obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o > obj-$(CONFIG_VGA_ARB) += vgaarb.o > obj-$(CONFIG_PCI_DOE) += doe.o > +obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o > > # Endpoint library must be initialized before its users > obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 3cef835b375f..8507cc32b61d 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -316,6 +316,8 @@ void pci_bus_add_device(struct pci_dev *dev) > */ > pcibios_bus_add_device(dev); > pci_fixup_device(pci_fixup_final, dev); > + if (pci_is_bridge(dev)) > + of_pci_make_dev_node(dev); > pci_create_sysfs_dev_files(dev); > pci_proc_attach_device(dev); > pci_bridge_d3_update(dev); > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c > index e9cf318e6670..eeaf44169bfd 100644 > --- a/drivers/pci/msi/irqdomain.c > +++ b/drivers/pci/msi/irqdomain.c > @@ -230,8 +230,10 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev) > pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid); > > of_node = irq_domain_get_of_node(domain); > - rid = of_node ? of_msi_map_id(&pdev->dev, of_node, rid) : > - iort_msi_map_id(&pdev->dev, rid); > + if (of_node && !of_node_check_flag(of_node, OF_DYNAMIC)) > + rid = of_msi_map_id(&pdev->dev, of_node, rid); > + else > + rid = iort_msi_map_id(&pdev->dev, rid); > > return rid; > } > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index 196834ed44fe..cb34a73ac8a3 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -469,6 +469,8 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args * > } else { > /* We found a P2P bridge, check if it has a node */ > ppnode = pci_device_to_OF_node(ppdev); > + if (ppnode && of_node_check_flag(ppnode, OF_DYNAMIC)) > + ppnode = NULL; > } > > /* > @@ -599,6 +601,75 @@ int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge) > return pci_parse_request_of_pci_ranges(dev, bridge); > } > > +#if IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES) > + > +void of_pci_remove_node(struct pci_dev *pdev) > +{ > + struct device_node *np; > + > + np = pci_device_to_OF_node(pdev); > + if (!np || !of_node_check_flag(np, OF_DYNAMIC)) > + return; > + pdev->dev.of_node = NULL; > + > + of_destroy_node(np); > +} > + > +void of_pci_make_dev_node(struct pci_dev *pdev) > +{ > + struct device_node *ppnode, *np = NULL; > + const char *pci_type = "dev"; > + struct of_changeset *cset; > + const char *name; > + int ret; > + > + /* > + * If there is already a device tree node linked to this device, > + * return immediately. > + */ > + if (pci_device_to_OF_node(pdev)) > + return; > + > + /* Check if there is device tree node for parent device */ > + if (!pdev->bus->self) > + ppnode = pdev->bus->dev.of_node; > + else > + ppnode = pdev->bus->self->dev.of_node; > + if (!ppnode) > + return; > + > + if (pci_is_bridge(pdev)) > + pci_type = "pci"; > + > + name = kasprintf(GFP_KERNEL, "%s@%x,%x", pci_type, > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); > + if (!name) > + goto failed; > + > + np = of_create_node(ppnode, name, &cset); > + if (!np) > + goto failed; > + > + ret = of_pci_add_properties(pdev, cset, np); > + if (ret) > + goto failed; > + > + ret = of_changeset_apply(cset); > + if (ret) > + goto failed; > + > + pdev->dev.of_node = np; > + kfree(name); > + > + return; > + > +failed: > + if (np) > + of_destroy_node(np); > + kfree(name); > +} > +#endif > + > #endif /* CONFIG_PCI */ > > /** > diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c > new file mode 100644 > index 000000000000..05c8ca05a71b > --- /dev/null > +++ b/drivers/pci/of_property.c > @@ -0,0 +1,222 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2022, Advanced Micro Devices, Inc. > + */ > + > +#include <linux/pci.h> > +#include <linux/of.h> > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <asm/unaligned.h> > +#include "pci.h" > + > +#define OF_PCI_ADDRESS_CELLS 3 > +#define OF_PCI_SIZE_CELLS 2 > + > +struct of_pci_addr_pair { > + u32 phys_addr[OF_PCI_ADDRESS_CELLS]; > + u32 size[OF_PCI_SIZE_CELLS]; > +}; > + > +struct of_pci_range { > + u32 child_addr[OF_PCI_ADDRESS_CELLS]; > + u32 parent_addr[OF_PCI_ADDRESS_CELLS]; > + u32 size[OF_PCI_SIZE_CELLS]; > +}; > + > +#define OF_PCI_ADDR_SPACE_CONFIG 0x0 > +#define OF_PCI_ADDR_SPACE_IO 0x1 > +#define OF_PCI_ADDR_SPACE_MEM32 0x2 > +#define OF_PCI_ADDR_SPACE_MEM64 0x3 > + > +#define OF_PCI_ADDR_FIELD_NONRELOC BIT(31) > +#define OF_PCI_ADDR_FIELD_SS GENMASK(25, 24) > +#define OF_PCI_ADDR_FIELD_PREFETCH BIT(30) > +#define OF_PCI_ADDR_FIELD_BUS GENMASK(23, 16) > +#define OF_PCI_ADDR_FIELD_DEV GENMASK(15, 11) > +#define OF_PCI_ADDR_FIELD_FUNC GENMASK(10, 8) > +#define OF_PCI_ADDR_FIELD_REG GENMASK(7, 0) > + > +#define OF_PCI_ADDR_HI GENMASK_ULL(63, 32) > +#define OF_PCI_ADDR_LO GENMASK_ULL(31, 0) > +#define OF_PCI_SIZE_HI GENMASK_ULL(63, 32) > +#define OF_PCI_SIZE_LO GENMASK_ULL(31, 0) > + > +enum of_pci_prop_compatible { > + PROP_COMPAT_PCI_VVVV_DDDD, > + PROP_COMPAT_PCICLASS_CCSSPP, > + PROP_COMPAT_PCICLASS_CCSS, > + PROP_COMPAT_NUM, > +}; > + > +static int of_pci_prop_device_type(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); > +} > + > +static int of_pci_prop_address_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#address_cells", > + OF_PCI_ADDRESS_CELLS); > +} > + > +static int of_pci_prop_size_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#size_cells", > + OF_PCI_SIZE_CELLS); > +} > + > +static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) > +{ > + prop[0] = flags; > + put_unaligned(addr, &prop[1]); > +} Here, the put_unaligned() call will assume you want to store a 32bits value since prop is pointing to a u32 array. This can't work. Moreover, when storing a 64bits address in device-tree properties, they are stored starting with their 32 MSB in the first cell and the 32 LSB in the next cells. This should probably be something like this: static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) { prop[0] = flags; prop[1] = addr >> 32; prop[2] = addr; } [snip] > + > +static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs, > + struct device_node *np) > +{ > + struct of_pci_range rp[PCI_BRIDGE_RESOURCE_NUM]; > + struct resource *res; > + int i = 0, j, ret; > + u64 val64; > + u32 flags; > + > + res = &pdev->resource[PCI_BRIDGE_RESOURCES]; > + for (j = 0; j < PCI_BRIDGE_RESOURCE_NUM; j++) { > + if (!resource_size(&res[j])) > + continue; > + > + flags = OF_PCI_ADDR_FIELD_NONRELOC; > + if (of_pci_get_addr_flags(&res[j], &flags)) > + continue; > + > + val64 = res[j].start; > + of_pci_set_address(rp[i].parent_addr, val64, flags); > + of_pci_set_address(rp[i].child_addr, val64, flags); > + > + val64 = resource_size(&res[j]); > + put_unaligned(val64, rp[i].size); Same problem here, the size is meant to be a 64 bits values but will assume you want to store it using a 32bits pointer. > + > + i++; > + } > + > + ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", (u32 *)rp, > + i * sizeof(*rp) / sizeof(u32)); > + > + return ret; > +} > + > +static int of_pci_prop_reg(struct pci_dev *pdev, struct of_changeset *ocs, > + struct device_node *np) > +{ > + struct of_pci_addr_pair *reg; > + int i = 1, resno, ret = 0; > + u32 reg_val, base_addr; > + resource_size_t sz; > + > + reg = kzalloc(sizeof(*reg) * (PCI_STD_NUM_BARS + 1), GFP_KERNEL); > + if (!reg) > + return -ENOMEM; > + > + reg_val = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, OF_PCI_ADDR_SPACE_CONFIG) | > + 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)); > + of_pci_set_address(reg[0].phys_addr, 0, reg_val); > + > + base_addr = PCI_BASE_ADDRESS_0; > + for (resno = PCI_STD_RESOURCES; resno <= PCI_STD_RESOURCE_END; > + resno++, base_addr += 4) { > + sz = pci_resource_len(pdev, resno); > + if (!sz) > + continue; > + > + ret = of_pci_get_addr_flags(&pdev->resource[resno], ®_val); > + if (!ret) > + continue; > + > + reg_val &= ~OF_PCI_ADDR_FIELD_REG; > + reg_val |= FIELD_PREP(OF_PCI_ADDR_FIELD_REG, base_addr); > + of_pci_set_address(reg[i].phys_addr, 0, reg_val); > + put_unaligned((u64)sz, reg[i].size); And same here. Something like this should probably be used (untested): static void of_pci_set_size(u32 *prop, u64 size) { prop[0] = size >> 32; prop[1] = size; }
Le Thu, 15 Dec 2022 09:30:45 -0800, Lizhi Hou <lizhi.hou@amd.com> a écrit : > + > +static int of_pci_prop_address_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#address_cells", > + OF_PCI_ADDRESS_CELLS); > +} Hi Lizhi, This should be #address-cells and not #address_cells. > + > +static int of_pci_prop_size_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#size_cells", > + OF_PCI_SIZE_CELLS); > +} Ditto here, this should be #size-cells and not #size_cells. Was this really tested ? Thanks,
Le Thu, 15 Dec 2022 09:30:45 -0800, Lizhi Hou <lizhi.hou@amd.com> a écrit : Further comments > diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c > new file mode 100644 > index 000000000000..05c8ca05a71b > --- /dev/null > +++ b/drivers/pci/of_property.c > @@ -0,0 +1,222 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2022, Advanced Micro Devices, Inc. > + */ > + > +#include <linux/pci.h> > +#include <linux/of.h> > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <asm/unaligned.h> > +#include "pci.h" > + > +#define OF_PCI_ADDRESS_CELLS 3 > +#define OF_PCI_SIZE_CELLS 2 > + > +struct of_pci_addr_pair { > + u32 phys_addr[OF_PCI_ADDRESS_CELLS]; > + u32 size[OF_PCI_SIZE_CELLS]; > +}; > + > +struct of_pci_range { > + u32 child_addr[OF_PCI_ADDRESS_CELLS]; > + u32 parent_addr[OF_PCI_ADDRESS_CELLS]; > + u32 size[OF_PCI_SIZE_CELLS]; > +}; > + > +#define OF_PCI_ADDR_SPACE_CONFIG 0x0 > +#define OF_PCI_ADDR_SPACE_IO 0x1 > +#define OF_PCI_ADDR_SPACE_MEM32 0x2 > +#define OF_PCI_ADDR_SPACE_MEM64 0x3 > + > +#define OF_PCI_ADDR_FIELD_NONRELOC BIT(31) > +#define OF_PCI_ADDR_FIELD_SS GENMASK(25, 24) > +#define OF_PCI_ADDR_FIELD_PREFETCH BIT(30) > +#define OF_PCI_ADDR_FIELD_BUS GENMASK(23, 16) > +#define OF_PCI_ADDR_FIELD_DEV GENMASK(15, 11) > +#define OF_PCI_ADDR_FIELD_FUNC GENMASK(10, 8) > +#define OF_PCI_ADDR_FIELD_REG GENMASK(7, 0) > + > +#define OF_PCI_ADDR_HI GENMASK_ULL(63, 32) > +#define OF_PCI_ADDR_LO GENMASK_ULL(31, 0) > +#define OF_PCI_SIZE_HI GENMASK_ULL(63, 32) > +#define OF_PCI_SIZE_LO GENMASK_ULL(31, 0) This is unused. > + > +enum of_pci_prop_compatible { > + PROP_COMPAT_PCI_VVVV_DDDD, > + PROP_COMPAT_PCICLASS_CCSSPP, > + PROP_COMPAT_PCICLASS_CCSS, > + PROP_COMPAT_NUM, > +}; > + > +static int of_pci_prop_device_type(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); > +} > + > +static int of_pci_prop_address_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#address_cells", > + OF_PCI_ADDRESS_CELLS); > +} > + > +static int of_pci_prop_size_cells(struct pci_dev *pdev, > + struct of_changeset *ocs, > + struct device_node *np) > +{ > + return of_changeset_add_prop_u32(ocs, np, "#size_cells", > + OF_PCI_SIZE_CELLS); > +} > + > +static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) > +{ > + prop[0] = flags; > + put_unaligned(addr, &prop[1]); > +} > + > +static int of_pci_get_addr_flags(struct resource *res, u32 *flags) > +{ > + u32 ss; > + > + if (res->flags & IORESOURCE_IO) > + ss = OF_PCI_ADDR_SPACE_IO; > + else if (res->flags & IORESOURCE_MEM_64) > + ss = OF_PCI_ADDR_SPACE_MEM64; > + else if (res->flags & IORESOURCE_MEM) > + ss = OF_PCI_ADDR_SPACE_MEM32; > + else > + return -EINVAL; > + > + *flags &= ~(OF_PCI_ADDR_FIELD_SS | OF_PCI_ADDR_FIELD_PREFETCH); > + if (res->flags & IORESOURCE_PREFETCH) > + *flags |= OF_PCI_ADDR_FIELD_PREFETCH; > + > + *flags |= ss; This seems wrong, should be: *flags |= FIELD_PREP(OF_PCI_ADDR_FIELD_SS, ss);
On 1/2/23 05:56, Clément Léger wrote: > Le Thu, 15 Dec 2022 09:30:45 -0800, > Lizhi Hou <lizhi.hou@amd.com> a écrit : > >> +}; >> + >> +static int of_pci_prop_device_type(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); >> +} >> + >> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >> + OF_PCI_ADDRESS_CELLS); >> +} >> + >> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >> + OF_PCI_SIZE_CELLS); >> +} > Hi Lizhi, > > For all these functions, the "pdev" parameter is actually unused. > > [snip] Ok. I will remove unused pdev. > >> + >> +static int of_pci_prop_compatible(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + const char *compat_strs[PROP_COMPAT_NUM] = { 0 }; >> + int i, ret; >> + >> + compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] = >> + kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device); > Maybe it should be better to use "pci%04x,%04x" to keep the existing > naming. Based on https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, "pci%x,%x" should be used? "name" Based on the PCI Class Code register, pick a name from Table 1. If none apply, generate a name of the form pciVVVV,DDDD as described below under "compatible". VVVV, DDDD, SSSS, ssss and RR are lower-case ASCII hexadecimal numbers without leading zeroes. Thanks, Lizhi >
Le 2023-01-03 19:44, Lizhi Hou a écrit : > On 1/2/23 05:56, Clément Léger wrote: >> Le Thu, 15 Dec 2022 09:30:45 -0800, >> Lizhi Hou <lizhi.hou@amd.com> a écrit : >> >>> +}; >>> + >>> +static int of_pci_prop_device_type(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); >>> +} >>> + >>> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >>> + OF_PCI_ADDRESS_CELLS); >>> +} >>> + >>> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >>> + OF_PCI_SIZE_CELLS); >>> +} >> Hi Lizhi, >> >> For all these functions, the "pdev" parameter is actually unused. >> >> [snip] > Ok. I will remove unused pdev. >> >>> + >>> +static int of_pci_prop_compatible(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + const char *compat_strs[PROP_COMPAT_NUM] = { 0 }; >>> + int i, ret; >>> + >>> + compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] = >>> + kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device); >> Maybe it should be better to use "pci%04x,%04x" to keep the existing >> naming. > > Based on > https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf, > "pci%x,%x" should be used? > > "name" Based on the PCI Class Code register, pick a name from Table 1. > If none apply, generate a name of the > form pciVVVV,DDDD as described below under "compatible". > > VVVV, DDDD, SSSS, ssss and RR are lower-case ASCII hexadecimal numbers > without leading zeroes. You might be right then ! I just looked at the already existing device-tree which adds leading zeroes. Someone like Rob or Frank might be able to answer on that. Clément > > > Thanks, > > Lizhi > >>
On 1/2/23 09:16, Clément Léger wrote: > Le Thu, 15 Dec 2022 09:30:45 -0800, > Lizhi Hou <lizhi.hou@amd.com> a écrit : > >> The PCI endpoint device such as Xilinx Alveo PCI card maps the register >> spaces from multiple hardware peripherals to its PCI BAR. Normally, >> the PCI core discovers devices and BARs using the PCI enumeration process. >> There is no infrastructure to discover the hardware peripherals that are >> present in a PCI device, and which can be accessed through the PCI BARs. >> >> For Alveo PCI card, the card firmware provides a flattened device tree to >> describe the hardware peripherals on its BARs. The Alveo card driver can >> load this flattened device tree and leverage device tree framework to >> generate platform devices for the hardware peripherals eventually. >> >> Apparently, the device tree framework requires a device tree node for the >> PCI device. Thus, it can generate the device tree nodes for hardware >> peripherals underneath. Because PCI is self discoverable bus, there might >> not be a device tree node created for PCI devices. This patch is to add >> support to generate device tree node for PCI devices. >> >> Added a kernel option. When the option is turned on, the kernel will >> generate device tree nodes for PCI bridges unconditionally. >> >> Initially, the basic properties are added for the dynamically generated >> device tree nodes. >> >> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> >> Signed-off-by: Sonal Santan <sonal.santan@amd.com> >> Signed-off-by: Max Zhen <max.zhen@amd.com> >> Reviewed-by: Brian Xu <brian.xu@amd.com> >> --- >> drivers/pci/Kconfig | 12 ++ >> drivers/pci/Makefile | 1 + >> drivers/pci/bus.c | 2 + >> drivers/pci/msi/irqdomain.c | 6 +- >> drivers/pci/of.c | 71 ++++++++++++ >> drivers/pci/of_property.c | 222 ++++++++++++++++++++++++++++++++++++ >> drivers/pci/pci-driver.c | 3 +- >> drivers/pci/pci.h | 19 +++ >> drivers/pci/remove.c | 1 + >> 9 files changed, 334 insertions(+), 3 deletions(-) >> create mode 100644 drivers/pci/of_property.c >> >> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig >> index 55c028af4bd9..1b2347aa1e5b 100644 >> --- a/drivers/pci/Kconfig >> +++ b/drivers/pci/Kconfig >> @@ -198,6 +198,18 @@ config PCI_HYPERV >> The PCI device frontend driver allows the kernel to import arbitrary >> PCI devices from a PCI backend to support PCI driver domains. >> >> +config PCI_DYNAMIC_OF_NODES >> + bool "Create Devicetree nodes for PCI devices" >> + depends on OF >> + select OF_DYNAMIC >> + help >> + This option enables support for generating device tree nodes for some >> + PCI devices. Thus, the driver of this kind can load and overlay >> + flattened device tree for its downstream devices. >> + >> + Once this option is selected, the device tree nodes will be generated >> + for all PCI bridges. >> + >> choice >> prompt "PCI Express hierarchy optimization setting" >> default PCIE_BUS_DEFAULT >> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile >> index 2680e4c92f0a..cc8b4e01e29d 100644 >> --- a/drivers/pci/Makefile >> +++ b/drivers/pci/Makefile >> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o >> obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o >> obj-$(CONFIG_VGA_ARB) += vgaarb.o >> obj-$(CONFIG_PCI_DOE) += doe.o >> +obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o >> >> # Endpoint library must be initialized before its users >> obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ >> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c >> index 3cef835b375f..8507cc32b61d 100644 >> --- a/drivers/pci/bus.c >> +++ b/drivers/pci/bus.c >> @@ -316,6 +316,8 @@ void pci_bus_add_device(struct pci_dev *dev) >> */ >> pcibios_bus_add_device(dev); >> pci_fixup_device(pci_fixup_final, dev); >> + if (pci_is_bridge(dev)) >> + of_pci_make_dev_node(dev); >> pci_create_sysfs_dev_files(dev); >> pci_proc_attach_device(dev); >> pci_bridge_d3_update(dev); >> diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c >> index e9cf318e6670..eeaf44169bfd 100644 >> --- a/drivers/pci/msi/irqdomain.c >> +++ b/drivers/pci/msi/irqdomain.c >> @@ -230,8 +230,10 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev) >> pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid); >> >> of_node = irq_domain_get_of_node(domain); >> - rid = of_node ? of_msi_map_id(&pdev->dev, of_node, rid) : >> - iort_msi_map_id(&pdev->dev, rid); >> + if (of_node && !of_node_check_flag(of_node, OF_DYNAMIC)) >> + rid = of_msi_map_id(&pdev->dev, of_node, rid); >> + else >> + rid = iort_msi_map_id(&pdev->dev, rid); >> >> return rid; >> } >> diff --git a/drivers/pci/of.c b/drivers/pci/of.c >> index 196834ed44fe..cb34a73ac8a3 100644 >> --- a/drivers/pci/of.c >> +++ b/drivers/pci/of.c >> @@ -469,6 +469,8 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args * >> } else { >> /* We found a P2P bridge, check if it has a node */ >> ppnode = pci_device_to_OF_node(ppdev); >> + if (ppnode && of_node_check_flag(ppnode, OF_DYNAMIC)) >> + ppnode = NULL; >> } >> >> /* >> @@ -599,6 +601,75 @@ int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge) >> return pci_parse_request_of_pci_ranges(dev, bridge); >> } >> >> +#if IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES) >> + >> +void of_pci_remove_node(struct pci_dev *pdev) >> +{ >> + struct device_node *np; >> + >> + np = pci_device_to_OF_node(pdev); >> + if (!np || !of_node_check_flag(np, OF_DYNAMIC)) >> + return; >> + pdev->dev.of_node = NULL; >> + >> + of_destroy_node(np); >> +} >> + >> +void of_pci_make_dev_node(struct pci_dev *pdev) >> +{ >> + struct device_node *ppnode, *np = NULL; >> + const char *pci_type = "dev"; >> + struct of_changeset *cset; >> + const char *name; >> + int ret; >> + >> + /* >> + * If there is already a device tree node linked to this device, >> + * return immediately. >> + */ >> + if (pci_device_to_OF_node(pdev)) >> + return; >> + >> + /* Check if there is device tree node for parent device */ >> + if (!pdev->bus->self) >> + ppnode = pdev->bus->dev.of_node; >> + else >> + ppnode = pdev->bus->self->dev.of_node; >> + if (!ppnode) >> + return; >> + >> + if (pci_is_bridge(pdev)) >> + pci_type = "pci"; >> + >> + name = kasprintf(GFP_KERNEL, "%s@%x,%x", pci_type, >> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); >> + if (!name) >> + goto failed; >> + >> + np = of_create_node(ppnode, name, &cset); >> + if (!np) >> + goto failed; >> + >> + ret = of_pci_add_properties(pdev, cset, np); >> + if (ret) >> + goto failed; >> + >> + ret = of_changeset_apply(cset); >> + if (ret) >> + goto failed; >> + >> + pdev->dev.of_node = np; >> + kfree(name); >> + >> + return; >> + >> +failed: >> + if (np) >> + of_destroy_node(np); >> + kfree(name); >> +} >> +#endif >> + >> #endif /* CONFIG_PCI */ >> >> /** >> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c >> new file mode 100644 >> index 000000000000..05c8ca05a71b >> --- /dev/null >> +++ b/drivers/pci/of_property.c >> @@ -0,0 +1,222 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Copyright (C) 2022, Advanced Micro Devices, Inc. >> + */ >> + >> +#include <linux/pci.h> >> +#include <linux/of.h> >> +#include <linux/bitfield.h> >> +#include <linux/bits.h> >> +#include <asm/unaligned.h> >> +#include "pci.h" >> + >> +#define OF_PCI_ADDRESS_CELLS 3 >> +#define OF_PCI_SIZE_CELLS 2 >> + >> +struct of_pci_addr_pair { >> + u32 phys_addr[OF_PCI_ADDRESS_CELLS]; >> + u32 size[OF_PCI_SIZE_CELLS]; >> +}; >> + >> +struct of_pci_range { >> + u32 child_addr[OF_PCI_ADDRESS_CELLS]; >> + u32 parent_addr[OF_PCI_ADDRESS_CELLS]; >> + u32 size[OF_PCI_SIZE_CELLS]; >> +}; >> + >> +#define OF_PCI_ADDR_SPACE_CONFIG 0x0 >> +#define OF_PCI_ADDR_SPACE_IO 0x1 >> +#define OF_PCI_ADDR_SPACE_MEM32 0x2 >> +#define OF_PCI_ADDR_SPACE_MEM64 0x3 >> + >> +#define OF_PCI_ADDR_FIELD_NONRELOC BIT(31) >> +#define OF_PCI_ADDR_FIELD_SS GENMASK(25, 24) >> +#define OF_PCI_ADDR_FIELD_PREFETCH BIT(30) >> +#define OF_PCI_ADDR_FIELD_BUS GENMASK(23, 16) >> +#define OF_PCI_ADDR_FIELD_DEV GENMASK(15, 11) >> +#define OF_PCI_ADDR_FIELD_FUNC GENMASK(10, 8) >> +#define OF_PCI_ADDR_FIELD_REG GENMASK(7, 0) >> + >> +#define OF_PCI_ADDR_HI GENMASK_ULL(63, 32) >> +#define OF_PCI_ADDR_LO GENMASK_ULL(31, 0) >> +#define OF_PCI_SIZE_HI GENMASK_ULL(63, 32) >> +#define OF_PCI_SIZE_LO GENMASK_ULL(31, 0) >> + >> +enum of_pci_prop_compatible { >> + PROP_COMPAT_PCI_VVVV_DDDD, >> + PROP_COMPAT_PCICLASS_CCSSPP, >> + PROP_COMPAT_PCICLASS_CCSS, >> + PROP_COMPAT_NUM, >> +}; >> + >> +static int of_pci_prop_device_type(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); >> +} >> + >> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >> + OF_PCI_ADDRESS_CELLS); >> +} >> + >> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >> + OF_PCI_SIZE_CELLS); >> +} >> + >> +static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) >> +{ >> + prop[0] = flags; >> + put_unaligned(addr, &prop[1]); >> +} > Here, the put_unaligned() call will assume you want to store a 32bits > value since prop is pointing to a u32 array. This can't work. > > Moreover, when storing a 64bits address in device-tree properties, they > are stored starting with their 32 MSB in the first cell and the > 32 LSB in the next cells. > > This should probably be something like this: > > static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) > { > prop[0] = flags; > prop[1] = addr >> 32; > prop[2] = addr; > } > > [snip] Thanks for pointing this out. And I should add (__be64 *) as below. put_unaligned(addr, (__be64 *)&prop[1]); > >> + >> +static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + struct of_pci_range rp[PCI_BRIDGE_RESOURCE_NUM]; >> + struct resource *res; >> + int i = 0, j, ret; >> + u64 val64; >> + u32 flags; >> + >> + res = &pdev->resource[PCI_BRIDGE_RESOURCES]; >> + for (j = 0; j < PCI_BRIDGE_RESOURCE_NUM; j++) { >> + if (!resource_size(&res[j])) >> + continue; >> + >> + flags = OF_PCI_ADDR_FIELD_NONRELOC; >> + if (of_pci_get_addr_flags(&res[j], &flags)) >> + continue; >> + >> + val64 = res[j].start; >> + of_pci_set_address(rp[i].parent_addr, val64, flags); >> + of_pci_set_address(rp[i].child_addr, val64, flags); >> + >> + val64 = resource_size(&res[j]); >> + put_unaligned(val64, rp[i].size); > Same problem here, the size is meant to be a 64 bits values but will > assume you want to store it using a 32bits pointer. Will change to: put_unaligned(val64, (__be64 *)rp[i].size); > >> + >> + i++; >> + } >> + >> + ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", (u32 *)rp, >> + i * sizeof(*rp) / sizeof(u32)); >> + >> + return ret; >> +} >> + >> +static int of_pci_prop_reg(struct pci_dev *pdev, struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + struct of_pci_addr_pair *reg; >> + int i = 1, resno, ret = 0; >> + u32 reg_val, base_addr; >> + resource_size_t sz; >> + >> + reg = kzalloc(sizeof(*reg) * (PCI_STD_NUM_BARS + 1), GFP_KERNEL); >> + if (!reg) >> + return -ENOMEM; >> + >> + reg_val = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, OF_PCI_ADDR_SPACE_CONFIG) | >> + 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)); >> + of_pci_set_address(reg[0].phys_addr, 0, reg_val); >> + >> + base_addr = PCI_BASE_ADDRESS_0; >> + for (resno = PCI_STD_RESOURCES; resno <= PCI_STD_RESOURCE_END; >> + resno++, base_addr += 4) { >> + sz = pci_resource_len(pdev, resno); >> + if (!sz) >> + continue; >> + >> + ret = of_pci_get_addr_flags(&pdev->resource[resno], ®_val); >> + if (!ret) >> + continue; >> + >> + reg_val &= ~OF_PCI_ADDR_FIELD_REG; >> + reg_val |= FIELD_PREP(OF_PCI_ADDR_FIELD_REG, base_addr); >> + of_pci_set_address(reg[i].phys_addr, 0, reg_val); >> + put_unaligned((u64)sz, reg[i].size); > And same here. Something like this should probably be used (untested): will change to: put_unaligned((u64)sz, (__be64 *)reg[i].size); Thanks, Lizhi > > static void of_pci_set_size(u32 *prop, u64 size) > { > prop[0] = size >> 32; > prop[1] = size; > } >
On 1/3/23 11:45, Lizhi Hou wrote: > > On 1/2/23 09:16, Clément Léger wrote: >> Le Thu, 15 Dec 2022 09:30:45 -0800, >> Lizhi Hou <lizhi.hou@amd.com> a écrit : >> >>> The PCI endpoint device such as Xilinx Alveo PCI card maps the register >>> spaces from multiple hardware peripherals to its PCI BAR. Normally, >>> the PCI core discovers devices and BARs using the PCI enumeration >>> process. >>> There is no infrastructure to discover the hardware peripherals that >>> are >>> present in a PCI device, and which can be accessed through the PCI >>> BARs. >>> >>> For Alveo PCI card, the card firmware provides a flattened device >>> tree to >>> describe the hardware peripherals on its BARs. The Alveo card driver >>> can >>> load this flattened device tree and leverage device tree framework to >>> generate platform devices for the hardware peripherals eventually. >>> >>> Apparently, the device tree framework requires a device tree node >>> for the >>> PCI device. Thus, it can generate the device tree nodes for hardware >>> peripherals underneath. Because PCI is self discoverable bus, there >>> might >>> not be a device tree node created for PCI devices. This patch is to add >>> support to generate device tree node for PCI devices. >>> >>> Added a kernel option. When the option is turned on, the kernel will >>> generate device tree nodes for PCI bridges unconditionally. >>> >>> Initially, the basic properties are added for the dynamically generated >>> device tree nodes. >>> >>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com> >>> Signed-off-by: Sonal Santan <sonal.santan@amd.com> >>> Signed-off-by: Max Zhen <max.zhen@amd.com> >>> Reviewed-by: Brian Xu <brian.xu@amd.com> >>> --- >>> drivers/pci/Kconfig | 12 ++ >>> drivers/pci/Makefile | 1 + >>> drivers/pci/bus.c | 2 + >>> drivers/pci/msi/irqdomain.c | 6 +- >>> drivers/pci/of.c | 71 ++++++++++++ >>> drivers/pci/of_property.c | 222 >>> ++++++++++++++++++++++++++++++++++++ >>> drivers/pci/pci-driver.c | 3 +- >>> drivers/pci/pci.h | 19 +++ >>> drivers/pci/remove.c | 1 + >>> 9 files changed, 334 insertions(+), 3 deletions(-) >>> create mode 100644 drivers/pci/of_property.c >>> >>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig >>> index 55c028af4bd9..1b2347aa1e5b 100644 >>> --- a/drivers/pci/Kconfig >>> +++ b/drivers/pci/Kconfig >>> @@ -198,6 +198,18 @@ config PCI_HYPERV >>> The PCI device frontend driver allows the kernel to import >>> arbitrary >>> PCI devices from a PCI backend to support PCI driver domains. >>> +config PCI_DYNAMIC_OF_NODES >>> + bool "Create Devicetree nodes for PCI devices" >>> + depends on OF >>> + select OF_DYNAMIC >>> + help >>> + This option enables support for generating device tree nodes >>> for some >>> + PCI devices. Thus, the driver of this kind can load and overlay >>> + flattened device tree for its downstream devices. >>> + >>> + Once this option is selected, the device tree nodes will be >>> generated >>> + for all PCI bridges. >>> + >>> choice >>> prompt "PCI Express hierarchy optimization setting" >>> default PCIE_BUS_DEFAULT >>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile >>> index 2680e4c92f0a..cc8b4e01e29d 100644 >>> --- a/drivers/pci/Makefile >>> +++ b/drivers/pci/Makefile >>> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o >>> obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o >>> obj-$(CONFIG_VGA_ARB) += vgaarb.o >>> obj-$(CONFIG_PCI_DOE) += doe.o >>> +obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o >>> # Endpoint library must be initialized before its users >>> obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ >>> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c >>> index 3cef835b375f..8507cc32b61d 100644 >>> --- a/drivers/pci/bus.c >>> +++ b/drivers/pci/bus.c >>> @@ -316,6 +316,8 @@ void pci_bus_add_device(struct pci_dev *dev) >>> */ >>> pcibios_bus_add_device(dev); >>> pci_fixup_device(pci_fixup_final, dev); >>> + if (pci_is_bridge(dev)) >>> + of_pci_make_dev_node(dev); >>> pci_create_sysfs_dev_files(dev); >>> pci_proc_attach_device(dev); >>> pci_bridge_d3_update(dev); >>> diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c >>> index e9cf318e6670..eeaf44169bfd 100644 >>> --- a/drivers/pci/msi/irqdomain.c >>> +++ b/drivers/pci/msi/irqdomain.c >>> @@ -230,8 +230,10 @@ u32 pci_msi_domain_get_msi_rid(struct >>> irq_domain *domain, struct pci_dev *pdev) >>> pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid); >>> of_node = irq_domain_get_of_node(domain); >>> - rid = of_node ? of_msi_map_id(&pdev->dev, of_node, rid) : >>> - iort_msi_map_id(&pdev->dev, rid); >>> + if (of_node && !of_node_check_flag(of_node, OF_DYNAMIC)) >>> + rid = of_msi_map_id(&pdev->dev, of_node, rid); >>> + else >>> + rid = iort_msi_map_id(&pdev->dev, rid); >>> return rid; >>> } >>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c >>> index 196834ed44fe..cb34a73ac8a3 100644 >>> --- a/drivers/pci/of.c >>> +++ b/drivers/pci/of.c >>> @@ -469,6 +469,8 @@ static int of_irq_parse_pci(const struct pci_dev >>> *pdev, struct of_phandle_args * >>> } else { >>> /* We found a P2P bridge, check if it has a node */ >>> ppnode = pci_device_to_OF_node(ppdev); >>> + if (ppnode && of_node_check_flag(ppnode, OF_DYNAMIC)) >>> + ppnode = NULL; >>> } >>> /* >>> @@ -599,6 +601,75 @@ int devm_of_pci_bridge_init(struct device *dev, >>> struct pci_host_bridge *bridge) >>> return pci_parse_request_of_pci_ranges(dev, bridge); >>> } >>> +#if IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES) >>> + >>> +void of_pci_remove_node(struct pci_dev *pdev) >>> +{ >>> + struct device_node *np; >>> + >>> + np = pci_device_to_OF_node(pdev); >>> + if (!np || !of_node_check_flag(np, OF_DYNAMIC)) >>> + return; >>> + pdev->dev.of_node = NULL; >>> + >>> + of_destroy_node(np); >>> +} >>> + >>> +void of_pci_make_dev_node(struct pci_dev *pdev) >>> +{ >>> + struct device_node *ppnode, *np = NULL; >>> + const char *pci_type = "dev"; >>> + struct of_changeset *cset; >>> + const char *name; >>> + int ret; >>> + >>> + /* >>> + * If there is already a device tree node linked to this device, >>> + * return immediately. >>> + */ >>> + if (pci_device_to_OF_node(pdev)) >>> + return; >>> + >>> + /* Check if there is device tree node for parent device */ >>> + if (!pdev->bus->self) >>> + ppnode = pdev->bus->dev.of_node; >>> + else >>> + ppnode = pdev->bus->self->dev.of_node; >>> + if (!ppnode) >>> + return; >>> + >>> + if (pci_is_bridge(pdev)) >>> + pci_type = "pci"; >>> + >>> + name = kasprintf(GFP_KERNEL, "%s@%x,%x", pci_type, >>> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); >>> + if (!name) >>> + goto failed; >>> + >>> + np = of_create_node(ppnode, name, &cset); >>> + if (!np) >>> + goto failed; >>> + >>> + ret = of_pci_add_properties(pdev, cset, np); >>> + if (ret) >>> + goto failed; >>> + >>> + ret = of_changeset_apply(cset); >>> + if (ret) >>> + goto failed; >>> + >>> + pdev->dev.of_node = np; >>> + kfree(name); >>> + >>> + return; >>> + >>> +failed: >>> + if (np) >>> + of_destroy_node(np); >>> + kfree(name); >>> +} >>> +#endif >>> + >>> #endif /* CONFIG_PCI */ >>> /** >>> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c >>> new file mode 100644 >>> index 000000000000..05c8ca05a71b >>> --- /dev/null >>> +++ b/drivers/pci/of_property.c >>> @@ -0,0 +1,222 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * Copyright (C) 2022, Advanced Micro Devices, Inc. >>> + */ >>> + >>> +#include <linux/pci.h> >>> +#include <linux/of.h> >>> +#include <linux/bitfield.h> >>> +#include <linux/bits.h> >>> +#include <asm/unaligned.h> >>> +#include "pci.h" >>> + >>> +#define OF_PCI_ADDRESS_CELLS 3 >>> +#define OF_PCI_SIZE_CELLS 2 >>> + >>> +struct of_pci_addr_pair { >>> + u32 phys_addr[OF_PCI_ADDRESS_CELLS]; >>> + u32 size[OF_PCI_SIZE_CELLS]; >>> +}; >>> + >>> +struct of_pci_range { >>> + u32 child_addr[OF_PCI_ADDRESS_CELLS]; >>> + u32 parent_addr[OF_PCI_ADDRESS_CELLS]; >>> + u32 size[OF_PCI_SIZE_CELLS]; >>> +}; >>> + >>> +#define OF_PCI_ADDR_SPACE_CONFIG 0x0 >>> +#define OF_PCI_ADDR_SPACE_IO 0x1 >>> +#define OF_PCI_ADDR_SPACE_MEM32 0x2 >>> +#define OF_PCI_ADDR_SPACE_MEM64 0x3 >>> + >>> +#define OF_PCI_ADDR_FIELD_NONRELOC BIT(31) >>> +#define OF_PCI_ADDR_FIELD_SS GENMASK(25, 24) >>> +#define OF_PCI_ADDR_FIELD_PREFETCH BIT(30) >>> +#define OF_PCI_ADDR_FIELD_BUS GENMASK(23, 16) >>> +#define OF_PCI_ADDR_FIELD_DEV GENMASK(15, 11) >>> +#define OF_PCI_ADDR_FIELD_FUNC GENMASK(10, 8) >>> +#define OF_PCI_ADDR_FIELD_REG GENMASK(7, 0) >>> + >>> +#define OF_PCI_ADDR_HI GENMASK_ULL(63, 32) >>> +#define OF_PCI_ADDR_LO GENMASK_ULL(31, 0) >>> +#define OF_PCI_SIZE_HI GENMASK_ULL(63, 32) >>> +#define OF_PCI_SIZE_LO GENMASK_ULL(31, 0) >>> + >>> +enum of_pci_prop_compatible { >>> + PROP_COMPAT_PCI_VVVV_DDDD, >>> + PROP_COMPAT_PCICLASS_CCSSPP, >>> + PROP_COMPAT_PCICLASS_CCSS, >>> + PROP_COMPAT_NUM, >>> +}; >>> + >>> +static int of_pci_prop_device_type(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_string(ocs, np, "device_type", >>> "pci"); >>> +} >>> + >>> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >>> + OF_PCI_ADDRESS_CELLS); >>> +} >>> + >>> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >>> + OF_PCI_SIZE_CELLS); >>> +} >>> + >>> +static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) >>> +{ >>> + prop[0] = flags; >>> + put_unaligned(addr, &prop[1]); >>> +} >> Here, the put_unaligned() call will assume you want to store a 32bits >> value since prop is pointing to a u32 array. This can't work. >> >> Moreover, when storing a 64bits address in device-tree properties, they >> are stored starting with their 32 MSB in the first cell and the >> 32 LSB in the next cells. >> >> This should probably be something like this: >> >> static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) >> { >> prop[0] = flags; >> prop[1] = addr >> 32; >> prop[2] = addr; >> } >> >> [snip] > > Thanks for pointing this out. And I should add (__be64 *) as below. > > put_unaligned(addr, (__be64 *)&prop[1]); I looked into it a bit more. You are correct. put_unaligned() does not work here. I re-write it as below: prop[1] = FIELD_GET(OF_PCI_ADDR_HI, addr); prop[2] = FIELD_GET(OF_PCI_ADDR_LO, addr); Similar changes will be made for "size" Thanks, Lizhi > >> >>> + >>> +static int of_pci_prop_ranges(struct pci_dev *pdev, struct >>> of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + struct of_pci_range rp[PCI_BRIDGE_RESOURCE_NUM]; >>> + struct resource *res; >>> + int i = 0, j, ret; >>> + u64 val64; >>> + u32 flags; >>> + >>> + res = &pdev->resource[PCI_BRIDGE_RESOURCES]; >>> + for (j = 0; j < PCI_BRIDGE_RESOURCE_NUM; j++) { >>> + if (!resource_size(&res[j])) >>> + continue; >>> + >>> + flags = OF_PCI_ADDR_FIELD_NONRELOC; >>> + if (of_pci_get_addr_flags(&res[j], &flags)) >>> + continue; >>> + >>> + val64 = res[j].start; >>> + of_pci_set_address(rp[i].parent_addr, val64, flags); >>> + of_pci_set_address(rp[i].child_addr, val64, flags); >>> + >>> + val64 = resource_size(&res[j]); >>> + put_unaligned(val64, rp[i].size); >> Same problem here, the size is meant to be a 64 bits values but will >> assume you want to store it using a 32bits pointer. > Will change to: put_unaligned(val64, (__be64 *)rp[i].size); >> >>> + >>> + i++; >>> + } >>> + >>> + ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", (u32 >>> *)rp, >>> + i * sizeof(*rp) / sizeof(u32)); >>> + >>> + return ret; >>> +} >>> + >>> +static int of_pci_prop_reg(struct pci_dev *pdev, struct >>> of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + struct of_pci_addr_pair *reg; >>> + int i = 1, resno, ret = 0; >>> + u32 reg_val, base_addr; >>> + resource_size_t sz; >>> + >>> + reg = kzalloc(sizeof(*reg) * (PCI_STD_NUM_BARS + 1), GFP_KERNEL); >>> + if (!reg) >>> + return -ENOMEM; >>> + >>> + reg_val = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, >>> OF_PCI_ADDR_SPACE_CONFIG) | >>> + 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)); >>> + of_pci_set_address(reg[0].phys_addr, 0, reg_val); >>> + >>> + base_addr = PCI_BASE_ADDRESS_0; >>> + for (resno = PCI_STD_RESOURCES; resno <= PCI_STD_RESOURCE_END; >>> + resno++, base_addr += 4) { >>> + sz = pci_resource_len(pdev, resno); >>> + if (!sz) >>> + continue; >>> + >>> + ret = of_pci_get_addr_flags(&pdev->resource[resno], ®_val); >>> + if (!ret) >>> + continue; >>> + >>> + reg_val &= ~OF_PCI_ADDR_FIELD_REG; >>> + reg_val |= FIELD_PREP(OF_PCI_ADDR_FIELD_REG, base_addr); >>> + of_pci_set_address(reg[i].phys_addr, 0, reg_val); >>> + put_unaligned((u64)sz, reg[i].size); >> And same here. Something like this should probably be used (untested): > > will change to: put_unaligned((u64)sz, (__be64 *)reg[i].size); > > > Thanks, > > Lizhi > >> >> static void of_pci_set_size(u32 *prop, u64 size) >> { >> prop[0] = size >> 32; >> prop[1] = size; >> } > >>
On 1/3/23 06:48, Clément Léger wrote: > Le Thu, 15 Dec 2022 09:30:45 -0800, > Lizhi Hou <lizhi.hou@amd.com> a écrit : > >> + >> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >> + OF_PCI_ADDRESS_CELLS); >> +} > Hi Lizhi, > > This should be #address-cells and not #address_cells. > >> + >> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >> + struct of_changeset *ocs, >> + struct device_node *np) >> +{ >> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >> + OF_PCI_SIZE_CELLS); >> +} > Ditto here, this should be #size-cells and not #size_cells. Was this > really tested ? I will fix this. I verified the properties are generated and did not notice the '_' error. Thanks, Lizhi > > Thanks, >
Le 2023-01-04 01:31, Lizhi Hou a écrit : > On 1/3/23 06:48, Clément Léger wrote: >> Le Thu, 15 Dec 2022 09:30:45 -0800, >> Lizhi Hou <lizhi.hou@amd.com> a écrit : >> >>> + >>> +static int of_pci_prop_address_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#address_cells", >>> + OF_PCI_ADDRESS_CELLS); >>> +} >> Hi Lizhi, >> >> This should be #address-cells and not #address_cells. >> >>> + >>> +static int of_pci_prop_size_cells(struct pci_dev *pdev, >>> + struct of_changeset *ocs, >>> + struct device_node *np) >>> +{ >>> + return of_changeset_add_prop_u32(ocs, np, "#size_cells", >>> + OF_PCI_SIZE_CELLS); >>> +} >> Ditto here, this should be #size-cells and not #size_cells. Was this >> really tested ? > > I will fix this. I verified the properties are generated and did not > notice the '_' error. Hi Lizhi, The most important thing to test is if a driver can correctly retrieve resource of the device using a node added below the created PCI device node. This is resolved using the whole hierarchy ranges, address-cells and size-cells properties. If not correctly set, this will fail (and it fails). Clément > > > Thanks, > > Lizhi > >> >> Thanks, >>
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 55c028af4bd9..1b2347aa1e5b 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -198,6 +198,18 @@ config PCI_HYPERV The PCI device frontend driver allows the kernel to import arbitrary PCI devices from a PCI backend to support PCI driver domains. +config PCI_DYNAMIC_OF_NODES + bool "Create Devicetree nodes for PCI devices" + depends on OF + select OF_DYNAMIC + help + This option enables support for generating device tree nodes for some + PCI devices. Thus, the driver of this kind can load and overlay + flattened device tree for its downstream devices. + + Once this option is selected, the device tree nodes will be generated + for all PCI bridges. + choice prompt "PCI Express hierarchy optimization setting" default PCIE_BUS_DEFAULT diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 2680e4c92f0a..cc8b4e01e29d 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o obj-$(CONFIG_VGA_ARB) += vgaarb.o obj-$(CONFIG_PCI_DOE) += doe.o +obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o # Endpoint library must be initialized before its users obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 3cef835b375f..8507cc32b61d 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -316,6 +316,8 @@ void pci_bus_add_device(struct pci_dev *dev) */ pcibios_bus_add_device(dev); pci_fixup_device(pci_fixup_final, dev); + if (pci_is_bridge(dev)) + of_pci_make_dev_node(dev); pci_create_sysfs_dev_files(dev); pci_proc_attach_device(dev); pci_bridge_d3_update(dev); diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c index e9cf318e6670..eeaf44169bfd 100644 --- a/drivers/pci/msi/irqdomain.c +++ b/drivers/pci/msi/irqdomain.c @@ -230,8 +230,10 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev) pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid); of_node = irq_domain_get_of_node(domain); - rid = of_node ? of_msi_map_id(&pdev->dev, of_node, rid) : - iort_msi_map_id(&pdev->dev, rid); + if (of_node && !of_node_check_flag(of_node, OF_DYNAMIC)) + rid = of_msi_map_id(&pdev->dev, of_node, rid); + else + rid = iort_msi_map_id(&pdev->dev, rid); return rid; } diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 196834ed44fe..cb34a73ac8a3 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -469,6 +469,8 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args * } else { /* We found a P2P bridge, check if it has a node */ ppnode = pci_device_to_OF_node(ppdev); + if (ppnode && of_node_check_flag(ppnode, OF_DYNAMIC)) + ppnode = NULL; } /* @@ -599,6 +601,75 @@ int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge) return pci_parse_request_of_pci_ranges(dev, bridge); } +#if IS_ENABLED(CONFIG_PCI_DYNAMIC_OF_NODES) + +void of_pci_remove_node(struct pci_dev *pdev) +{ + struct device_node *np; + + np = pci_device_to_OF_node(pdev); + if (!np || !of_node_check_flag(np, OF_DYNAMIC)) + return; + pdev->dev.of_node = NULL; + + of_destroy_node(np); +} + +void of_pci_make_dev_node(struct pci_dev *pdev) +{ + struct device_node *ppnode, *np = NULL; + const char *pci_type = "dev"; + struct of_changeset *cset; + const char *name; + int ret; + + /* + * If there is already a device tree node linked to this device, + * return immediately. + */ + if (pci_device_to_OF_node(pdev)) + return; + + /* Check if there is device tree node for parent device */ + if (!pdev->bus->self) + ppnode = pdev->bus->dev.of_node; + else + ppnode = pdev->bus->self->dev.of_node; + if (!ppnode) + return; + + if (pci_is_bridge(pdev)) + pci_type = "pci"; + + name = kasprintf(GFP_KERNEL, "%s@%x,%x", pci_type, + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); + if (!name) + goto failed; + + np = of_create_node(ppnode, name, &cset); + if (!np) + goto failed; + + ret = of_pci_add_properties(pdev, cset, np); + if (ret) + goto failed; + + ret = of_changeset_apply(cset); + if (ret) + goto failed; + + pdev->dev.of_node = np; + kfree(name); + + return; + +failed: + if (np) + of_destroy_node(np); + kfree(name); +} +#endif + #endif /* CONFIG_PCI */ /** diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c new file mode 100644 index 000000000000..05c8ca05a71b --- /dev/null +++ b/drivers/pci/of_property.c @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2022, Advanced Micro Devices, Inc. + */ + +#include <linux/pci.h> +#include <linux/of.h> +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <asm/unaligned.h> +#include "pci.h" + +#define OF_PCI_ADDRESS_CELLS 3 +#define OF_PCI_SIZE_CELLS 2 + +struct of_pci_addr_pair { + u32 phys_addr[OF_PCI_ADDRESS_CELLS]; + u32 size[OF_PCI_SIZE_CELLS]; +}; + +struct of_pci_range { + u32 child_addr[OF_PCI_ADDRESS_CELLS]; + u32 parent_addr[OF_PCI_ADDRESS_CELLS]; + u32 size[OF_PCI_SIZE_CELLS]; +}; + +#define OF_PCI_ADDR_SPACE_CONFIG 0x0 +#define OF_PCI_ADDR_SPACE_IO 0x1 +#define OF_PCI_ADDR_SPACE_MEM32 0x2 +#define OF_PCI_ADDR_SPACE_MEM64 0x3 + +#define OF_PCI_ADDR_FIELD_NONRELOC BIT(31) +#define OF_PCI_ADDR_FIELD_SS GENMASK(25, 24) +#define OF_PCI_ADDR_FIELD_PREFETCH BIT(30) +#define OF_PCI_ADDR_FIELD_BUS GENMASK(23, 16) +#define OF_PCI_ADDR_FIELD_DEV GENMASK(15, 11) +#define OF_PCI_ADDR_FIELD_FUNC GENMASK(10, 8) +#define OF_PCI_ADDR_FIELD_REG GENMASK(7, 0) + +#define OF_PCI_ADDR_HI GENMASK_ULL(63, 32) +#define OF_PCI_ADDR_LO GENMASK_ULL(31, 0) +#define OF_PCI_SIZE_HI GENMASK_ULL(63, 32) +#define OF_PCI_SIZE_LO GENMASK_ULL(31, 0) + +enum of_pci_prop_compatible { + PROP_COMPAT_PCI_VVVV_DDDD, + PROP_COMPAT_PCICLASS_CCSSPP, + PROP_COMPAT_PCICLASS_CCSS, + PROP_COMPAT_NUM, +}; + +static int of_pci_prop_device_type(struct pci_dev *pdev, + struct of_changeset *ocs, + struct device_node *np) +{ + return of_changeset_add_prop_string(ocs, np, "device_type", "pci"); +} + +static int of_pci_prop_address_cells(struct pci_dev *pdev, + struct of_changeset *ocs, + struct device_node *np) +{ + return of_changeset_add_prop_u32(ocs, np, "#address_cells", + OF_PCI_ADDRESS_CELLS); +} + +static int of_pci_prop_size_cells(struct pci_dev *pdev, + struct of_changeset *ocs, + struct device_node *np) +{ + return of_changeset_add_prop_u32(ocs, np, "#size_cells", + OF_PCI_SIZE_CELLS); +} + +static void of_pci_set_address(u32 *prop, u64 addr, u32 flags) +{ + prop[0] = flags; + put_unaligned(addr, &prop[1]); +} + +static int of_pci_get_addr_flags(struct resource *res, u32 *flags) +{ + u32 ss; + + if (res->flags & IORESOURCE_IO) + ss = OF_PCI_ADDR_SPACE_IO; + else if (res->flags & IORESOURCE_MEM_64) + ss = OF_PCI_ADDR_SPACE_MEM64; + else if (res->flags & IORESOURCE_MEM) + ss = OF_PCI_ADDR_SPACE_MEM32; + else + return -EINVAL; + + *flags &= ~(OF_PCI_ADDR_FIELD_SS | OF_PCI_ADDR_FIELD_PREFETCH); + if (res->flags & IORESOURCE_PREFETCH) + *flags |= OF_PCI_ADDR_FIELD_PREFETCH; + + *flags |= ss; + + return 0; +} + +static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs, + struct device_node *np) +{ + struct of_pci_range rp[PCI_BRIDGE_RESOURCE_NUM]; + struct resource *res; + int i = 0, j, ret; + u64 val64; + u32 flags; + + res = &pdev->resource[PCI_BRIDGE_RESOURCES]; + for (j = 0; j < PCI_BRIDGE_RESOURCE_NUM; j++) { + if (!resource_size(&res[j])) + continue; + + flags = OF_PCI_ADDR_FIELD_NONRELOC; + if (of_pci_get_addr_flags(&res[j], &flags)) + continue; + + val64 = res[j].start; + of_pci_set_address(rp[i].parent_addr, val64, flags); + of_pci_set_address(rp[i].child_addr, val64, flags); + + val64 = resource_size(&res[j]); + put_unaligned(val64, rp[i].size); + + i++; + } + + ret = of_changeset_add_prop_u32_array(ocs, np, "ranges", (u32 *)rp, + i * sizeof(*rp) / sizeof(u32)); + + return ret; +} + +static int of_pci_prop_reg(struct pci_dev *pdev, struct of_changeset *ocs, + struct device_node *np) +{ + struct of_pci_addr_pair *reg; + int i = 1, resno, ret = 0; + u32 reg_val, base_addr; + resource_size_t sz; + + reg = kzalloc(sizeof(*reg) * (PCI_STD_NUM_BARS + 1), GFP_KERNEL); + if (!reg) + return -ENOMEM; + + reg_val = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, OF_PCI_ADDR_SPACE_CONFIG) | + 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)); + of_pci_set_address(reg[0].phys_addr, 0, reg_val); + + base_addr = PCI_BASE_ADDRESS_0; + for (resno = PCI_STD_RESOURCES; resno <= PCI_STD_RESOURCE_END; + resno++, base_addr += 4) { + sz = pci_resource_len(pdev, resno); + if (!sz) + continue; + + ret = of_pci_get_addr_flags(&pdev->resource[resno], ®_val); + if (!ret) + continue; + + reg_val &= ~OF_PCI_ADDR_FIELD_REG; + reg_val |= FIELD_PREP(OF_PCI_ADDR_FIELD_REG, base_addr); + of_pci_set_address(reg[i].phys_addr, 0, reg_val); + put_unaligned((u64)sz, reg[i].size); + i++; + } + + ret = of_changeset_add_prop_u32_array(ocs, np, "reg", (u32 *)reg, + i * sizeof(*reg) / sizeof(u32)); + kfree(reg); + + return ret; +} + +static int of_pci_prop_compatible(struct pci_dev *pdev, + struct of_changeset *ocs, + struct device_node *np) +{ + const char *compat_strs[PROP_COMPAT_NUM] = { 0 }; + int i, ret; + + compat_strs[PROP_COMPAT_PCI_VVVV_DDDD] = + kasprintf(GFP_KERNEL, "pci%x,%x", pdev->vendor, pdev->device); + compat_strs[PROP_COMPAT_PCICLASS_CCSSPP] = + kasprintf(GFP_KERNEL, "pciclass,%06x", pdev->class); + compat_strs[PROP_COMPAT_PCICLASS_CCSS] = + kasprintf(GFP_KERNEL, "pciclass,%04x", pdev->class >> 8); + + ret = of_changeset_add_prop_string_array(ocs, np, "compatible", + compat_strs, PROP_COMPAT_NUM); + for (i = 0; i < PROP_COMPAT_NUM; i++) + kfree(compat_strs[i]); + + return ret; +} + +int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs, + struct device_node *np) +{ + int ret = 0; + + if (pci_is_bridge(pdev)) { + ret |= of_pci_prop_device_type(pdev, ocs, np); + ret |= of_pci_prop_address_cells(pdev, ocs, np); + ret |= of_pci_prop_size_cells(pdev, ocs, np); + ret |= of_pci_prop_ranges(pdev, ocs, np); + } + + ret |= of_pci_prop_reg(pdev, ocs, np); + ret |= of_pci_prop_compatible(pdev, ocs, np); + + /* + * The added properties will be released when the + * changeset is destroyed. + */ + return ret; +} diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 49238ddd39ee..1540c4c9a770 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -1628,7 +1628,8 @@ static int pci_dma_configure(struct device *dev) bridge = pci_get_host_bridge_device(to_pci_dev(dev)); if (IS_ENABLED(CONFIG_OF) && bridge->parent && - bridge->parent->of_node) { + bridge->parent->of_node && + !of_node_check_flag(bridge->parent->of_node, OF_DYNAMIC)) { ret = of_dma_configure(dev, bridge->parent->of_node, true); } else if (has_acpi_companion(bridge)) { struct acpi_device *adev = to_acpi_device_node(bridge->fwnode); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 785f31086313..bd81dc4ca04f 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -678,6 +678,25 @@ static inline int devm_of_pci_bridge_init(struct device *dev, struct pci_host_br #endif /* CONFIG_OF */ +struct of_changeset; + +#ifdef CONFIG_PCI_DYNAMIC_OF_NODES +void of_pci_make_dev_node(struct pci_dev *pdev); +void of_pci_remove_node(struct pci_dev *pdev); +int of_pci_add_properties(struct pci_dev *pdev, 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) +{ +} +#endif /* CONFIG_PCI_DYNAMIC_OF_NODES */ + #ifdef CONFIG_PCIEAER void pci_no_aer(void); void pci_aer_init(struct pci_dev *dev); diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c index 4c54c75050dc..0eaa9d9a3609 100644 --- a/drivers/pci/remove.c +++ b/drivers/pci/remove.c @@ -23,6 +23,7 @@ static void pci_stop_dev(struct pci_dev *dev) device_release_driver(&dev->dev); pci_proc_detach_device(dev); pci_remove_sysfs_dev_files(dev); + of_pci_remove_node(dev); pci_dev_assign_added(dev, false); }