[PATCH V1 RESEND 0/4] Infrastructure to define apertures in a PCIe device with a flattened device tree

[PATCH V1 RESEND 0/4] Infrastructure to define apertures in a PCIe device with a flattened device tree

[Lizhi Hou]

Hello,

This V1 of patch series is to provide the required pci OF interfaces for
the PCIe device which uses flattened device tree to describe apertures in
its PCIe BARs. e.g, Xilinx Alveo PCIe accelerator. This requires a base
device tree which contains nodes for PCIe devices. A PCIe device driver
can then overlay a flattened device tree on the PCIe device tree node.
There are two separate parts for this to work. First, not all system has
a base device tree created by default. Thus, a patch to create an empty
device tree root node has been submitted.
  https://lore.kernel.org/lkml/20220216050056.311496-1-lizhi.hou@xilinx.com/
Second, PCIe is self discoverable bus and there might not be a device tree
node created for PCIe device. This patch provides a new interface to create
a ‘pci-ep-bus’ node under the base device tree root node. PCIe device
driver may call this interface in its probe routine to create device tree
node, then overlays its device tree to the node.
For the overlayed device tree nodes, each node presents a hardware aperture
implemented in its PCIe BARs. The aperture register address consists of BAR
index and offset. It uses the following encoding:
  0xIooooooo 0xoooooooo
Where:
  I = BAR index
  ooooooo oooooooo = BAR offset
The ‘pci-ep-bus’ node been created is compatible with ‘simple-bus’ and
contains ‘ranges’ property for translating aperture address to CPU address.
The last patch enhances of_overlay_fdt_apply(). The ‘pci-ep-bus’ device
node is created dynamically. The flattened device tree may not specify an
fixed target overlay path in front. Instead, a relative path to the
‘pci-ep-bus’ node is specified in the flattened tree. Thus, a new
parameter is added to point the target base node which is ‘pci-ep-bus’
node in this case. Then the entire overlay target path is target base node
path plus the relative path specified in the flattened device tree.

Lizhi Hou (4):
  pci: add interface to create pci-ep device tree node
  Documentation: devicetree: bindings: add binding for PCIe endpoint bus
  fpga: xrt: management physical function driver
  of: enhance overlay applying interface to specific target base node

 .../devicetree/bindings/bus/pci-ep-bus.yaml   |  72 +++++++
 drivers/fpga/Kconfig                          |   3 +
 drivers/fpga/Makefile                         |   3 +
 drivers/fpga/xrt/Kconfig                      |  24 +++
 drivers/fpga/xrt/Makefile                     |   8 +
 drivers/fpga/xrt/mgmt/Makefile                |  13 ++
 drivers/fpga/xrt/mgmt/dt-test.dts             |  15 ++
 drivers/fpga/xrt/mgmt/dt-test.h               |  15 ++
 drivers/fpga/xrt/mgmt/xmgmt-drv.c             | 102 ++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_of.c          |   2 +-
 drivers/of/overlay.c                          |  37 ++--
 drivers/of/unittest.c                         |   2 +-
 drivers/pci/of.c                              | 180 ++++++++++++++++++
 include/linux/of.h                            |   2 +-
 include/linux/of_pci.h                        |  15 ++
 15 files changed, 479 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
 create mode 100644 drivers/fpga/xrt/Kconfig
 create mode 100644 drivers/fpga/xrt/Makefile
 create mode 100644 drivers/fpga/xrt/mgmt/Makefile
 create mode 100644 drivers/fpga/xrt/mgmt/dt-test.dts
 create mode 100644 drivers/fpga/xrt/mgmt/dt-test.h
 create mode 100644 drivers/fpga/xrt/mgmt/xmgmt-drv.c

-- 
2.27.0

[PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node

[Lizhi Hou]

This patch enables PCIe device to uses flattened device tree to describe
apertures in its PCIe BARs. The aperture address consists of PCIe BAR index
and offset.

For this kind of device, the driver probe routine calls the new added
interface to create a device tree node. This device tree node is attached
under system device tree root. Then the driver may load the flatten device
tree overlay and attach it under this node. And the node also contains
'ranges' property which is used to translate aperture address(BAR index
and offset) to CPU address.

Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
Signed-off-by: Max Zhen <max.zhen@xilinx.com>
Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
---
 drivers/pci/of.c       | 180 +++++++++++++++++++++++++++++++++++++++++
 include/linux/of_pci.h |  15 ++++
 2 files changed, 195 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index cb2e8351c2cc..198f08351070 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -605,6 +605,186 @@ 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_OF_DYNAMIC)
+
+static void devm_of_pci_destroy_bus_endpoint(struct device *dev, void *res)
+{
+	struct device_node *node = res;
+
+	of_detach_node(node);
+}
+
+static int of_ep_add_property(struct device *dev, struct property **proplist, const char *name,
+			      const int length, void *value)
+{
+	struct property *new;
+
+	new = devm_kzalloc(dev, sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	new->name = devm_kstrdup(dev, name, GFP_KERNEL);
+	if (!new->name)
+		return -ENOMEM;
+
+	new->value = devm_kmalloc(dev, length, GFP_KERNEL);
+	if (!new->value)
+		return -ENOMEM;
+
+	memcpy(new->value, value, length);
+	new->length = length;
+	new->next = *proplist;
+	*proplist = new;
+
+	return 0;
+}
+
+static struct device_node *of_ep_alloc_node(struct pci_dev *pdev, const char *name)
+{
+	struct device_node *node;
+	char *full_name;
+
+	node = devres_alloc(devm_of_pci_destroy_bus_endpoint, sizeof(*node), GFP_KERNEL);
+	if (!node)
+		return NULL;
+
+	full_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "/%s@%llx", name,
+				   (u64)pci_resource_start(pdev, 0));
+	if (!full_name)
+		return NULL;
+
+	node->parent = of_root;
+	node->full_name = full_name;
+	of_node_set_flag(node, OF_DYNAMIC);
+	of_node_init(node);
+
+	return node;
+}
+
+/**
+ * devm_of_pci_create_bus_endpoint - Create a device node for the given pci device.
+ * @pdev: PCI device pointer.
+ *
+ * For PCI device which uses flattened device tree to describe apertures in its BARs,
+ * a device node for the given pci device is required. Then the flattened device tree
+ * overlay from the device can be applied to the base tree.
+ * The device node is under root node and act like bus node. It contains a "ranges"
+ * property which is used for address translation of its children. Each child node
+ * corresponds an aperture and use BAR index and offset as its address.
+
+ * Returns 0 on success or a negative error-code on failure.
+ */
+int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev)
+{
+	struct property *proplist = NULL;
+	struct device *dev = &pdev->dev;
+	int range_ncells, addr_ncells;
+	struct device_node *node;
+	void *prop = NULL;
+	u32 *range_cell;
+	__be32 val;
+	int i, ret;
+
+	node = of_ep_alloc_node(pdev, "pci-ep-bus");
+	if (!node)
+		return -ENOMEM;
+
+	/* the endpoint node works as 'simple-bus' to translate aperture addresses. */
+	prop = "simple-bus";
+	ret = of_ep_add_property(dev, &proplist, "compatible", strlen(prop) + 1, prop);
+	if (ret)
+		goto cleanup;
+
+	/* The address and size cells of nodes underneath are 2 */
+	val = cpu_to_be32(2);
+	ret = of_ep_add_property(dev, &proplist, "#address-cells", sizeof(u32), &val);
+	if (ret)
+		goto cleanup;
+
+	ret = of_ep_add_property(dev, &proplist, "#size-cells", sizeof(u32), &val);
+	if (ret)
+		goto cleanup;
+
+	/* child address format: 0xIooooooo oooooooo, I = bar index, o = offset on bar */
+	addr_ncells = of_n_addr_cells(node);
+	if (addr_ncells > 2) {
+		/* does not support number of address cells greater than 2 */
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	/* range cells include <node addr cells> <child addr cells> <child size cells> */
+	range_ncells = addr_ncells + 4;
+	prop = kzalloc(range_ncells * sizeof(u32) * PCI_STD_NUM_BARS, GFP_KERNEL);
+	if (!prop) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	range_cell = prop;
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+		if (!pci_resource_len(pdev, i))
+			continue;
+		/* highest 4 bits of address are bar index */
+		*(__be64 *)range_cell = cpu_to_be64((u64)i << 60);
+		range_cell += 2;
+		if (addr_ncells == 2)
+			*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_start(pdev, i));
+		else
+			*(__be32 *)range_cell = cpu_to_be32((u32)pci_resource_start(pdev, i));
+
+		range_cell += addr_ncells;
+		*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_len(pdev, i));
+		range_cell += 2;
+	}
+
+	/* error out if there is not PCI BAR been found */
+	if ((void *)range_cell == prop) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	ret = of_ep_add_property(dev, &proplist, "ranges", (void *)range_cell - prop, prop);
+	kfree(prop);
+	if (ret)
+		goto cleanup;
+
+	node->properties = proplist;
+	ret = of_attach_node(node);
+	if (ret)
+		goto cleanup;
+
+	devres_add(dev, node);
+
+	return 0;
+
+cleanup:
+	kfree(prop);
+	if (node)
+		devres_free(node);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_of_pci_create_bus_endpoint);
+
+struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev)
+{
+	struct device_node *dn;
+	char *path;
+
+	path = kasprintf(GFP_KERNEL, "/pci-ep-bus@%llx",
+			 (u64)pci_resource_start(pdev, 0));
+	if (!path)
+		return NULL;
+
+	dn = of_find_node_by_path(path);
+	kfree(path);
+
+	return dn;
+}
+EXPORT_SYMBOL_GPL(of_pci_find_bus_endpoint);
+#endif /* CONFIG_OF_DYNAMIC */
+
 #endif /* CONFIG_PCI */
 
 /**
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..c1d86be321b2 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -38,4 +38,19 @@ of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_OF_DYNAMIC) && IS_ENABLED(CONFIG_PCI)
+int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev);
+struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev);
+#else
+static inline int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev)
+{
+	return -EINVAL;
+}
+
+static inline struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev)
+{
+	return NULL;
+}
+#endif
+
 #endif
-- 
2.27.0

Re: [PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node

[Dan Carpenter]

Hi Lizhi,

url:    https://github.com/0day-ci/linux/commits/Lizhi-Hou/Infrastructure-to-define-apertures-in-a-PCIe-device-with-a-flattened-device-tree/20220307-141939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-m021-20220307 (https://download.01.org/0day-ci/archive/20220310/202203100338.8jox1rCr-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/pci/of.c:762 devm_of_pci_create_bus_endpoint() error: double free of 'prop'

vim +/prop +762 drivers/pci/of.c

3a2c08c0f0ef77 Lizhi Hou 2022-03-04  677  int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  678  {
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  679  	struct property *proplist = NULL;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  680  	struct device *dev = &pdev->dev;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  681  	int range_ncells, addr_ncells;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  682  	struct device_node *node;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  683  	void *prop = NULL;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  684  	u32 *range_cell;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  685  	__be32 val;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  686  	int i, ret;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  687  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  688  	node = of_ep_alloc_node(pdev, "pci-ep-bus");
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  689  	if (!node)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  690  		return -ENOMEM;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  691  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  692  	/* the endpoint node works as 'simple-bus' to translate aperture addresses. */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  693  	prop = "simple-bus";
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  694  	ret = of_ep_add_property(dev, &proplist, "compatible", strlen(prop) + 1, prop);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  695  	if (ret)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  696  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  697  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  698  	/* The address and size cells of nodes underneath are 2 */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  699  	val = cpu_to_be32(2);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  700  	ret = of_ep_add_property(dev, &proplist, "#address-cells", sizeof(u32), &val);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  701  	if (ret)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  702  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  703  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  704  	ret = of_ep_add_property(dev, &proplist, "#size-cells", sizeof(u32), &val);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  705  	if (ret)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  706  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  707  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  708  	/* child address format: 0xIooooooo oooooooo, I = bar index, o = offset on bar */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  709  	addr_ncells = of_n_addr_cells(node);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  710  	if (addr_ncells > 2) {
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  711  		/* does not support number of address cells greater than 2 */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  712  		ret = -EINVAL;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  713  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  714  	}
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  715  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  716  	/* range cells include <node addr cells> <child addr cells> <child size cells> */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  717  	range_ncells = addr_ncells + 4;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  718  	prop = kzalloc(range_ncells * sizeof(u32) * PCI_STD_NUM_BARS, GFP_KERNEL);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  719  	if (!prop) {
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  720  		ret = -ENOMEM;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  721  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  722  	}
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  723  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  724  	range_cell = prop;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  725  	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  726  		if (!pci_resource_len(pdev, i))
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  727  			continue;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  728  		/* highest 4 bits of address are bar index */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  729  		*(__be64 *)range_cell = cpu_to_be64((u64)i << 60);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  730  		range_cell += 2;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  731  		if (addr_ncells == 2)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  732  			*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_start(pdev, i));
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  733  		else
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  734  			*(__be32 *)range_cell = cpu_to_be32((u32)pci_resource_start(pdev, i));
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  735  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  736  		range_cell += addr_ncells;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  737  		*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_len(pdev, i));
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  738  		range_cell += 2;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  739  	}
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  740  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  741  	/* error out if there is not PCI BAR been found */
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  742  	if ((void *)range_cell == prop) {
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  743  		ret = -EINVAL;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  744  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  745  	}
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  746  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  747  	ret = of_ep_add_property(dev, &proplist, "ranges", (void *)range_cell - prop, prop);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  748  	kfree(prop);
                                                ^^^^^^^^^^^^
Free

3a2c08c0f0ef77 Lizhi Hou 2022-03-04  749  	if (ret)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  750  		goto cleanup;
                                                        ^^^^^^^^^^^^^
Double free after goto.


3a2c08c0f0ef77 Lizhi Hou 2022-03-04  751  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  752  	node->properties = proplist;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  753  	ret = of_attach_node(node);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  754  	if (ret)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  755  		goto cleanup;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  756  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  757  	devres_add(dev, node);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  758  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  759  	return 0;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  760  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  761  cleanup:
3a2c08c0f0ef77 Lizhi Hou 2022-03-04 @762  	kfree(prop);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  763  	if (node)
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  764  		devres_free(node);
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  765  
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  766  	return ret;
3a2c08c0f0ef77 Lizhi Hou 2022-03-04  767  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node

[Bjorn Helgaas]

Run "git log --oneline drivers/pci/of.c" and follow the convention,
e.g., something like:

  PCI: Add DT Endpoint description interfaces

On Fri, Mar 04, 2022 at 09:23:01PM -0800, Lizhi Hou wrote:
> This patch enables PCIe device to uses flattened device tree to describe
> apertures in its PCIe BARs. The aperture address consists of PCIe BAR index
> and offset.
> 
> For this kind of device, the driver probe routine calls the new added
> interface to create a device tree node. This device tree node is attached
> under system device tree root. Then the driver may load the flatten device
> tree overlay and attach it under this node. And the node also contains
> 'ranges' property which is used to translate aperture address(BAR index
> and offset) to CPU address.

In the commit log, please say *what* this patch does and why we need
it.  The current text talks about how some interface might be used,
but doesn't specifically say what interface that is or that this patch
adds it.

It should also have a specific pointer to the relevant DT binding.

> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  drivers/pci/of.c       | 180 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_pci.h |  15 ++++
>  2 files changed, 195 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index cb2e8351c2cc..198f08351070 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -605,6 +605,186 @@ 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_OF_DYNAMIC)
> +
> +static void devm_of_pci_destroy_bus_endpoint(struct device *dev, void *res)
> +{
> +	struct device_node *node = res;
> +
> +	of_detach_node(node);
> +}
> +
> +static int of_ep_add_property(struct device *dev, struct property **proplist, const char *name,

Please rewrap code and comments to fit in 80 columns like the rest of
the file.  There's a lot more below that I snipped out.

Bjorn

Re: [PATCH V1 RESEND 1/4] pci: add interface to create pci-ep device tree node

[Manivannan Sadhasivam]

On Fri, Mar 04, 2022 at 09:23:01PM -0800, Lizhi Hou wrote:
> This patch enables PCIe device to uses flattened device tree to describe
> apertures in its PCIe BARs. The aperture address consists of PCIe BAR index
> and offset.
> 
> For this kind of device, the driver probe routine calls the new added
> interface to create a device tree node. This device tree node is attached
> under system device tree root. Then the driver may load the flatten device
> tree overlay and attach it under this node. And the node also contains
> 'ranges' property which is used to translate aperture address(BAR index
> and offset) to CPU address.
> 

This is the devicetree support for the PCI endpoint subsystem. Hence, the
code should live under drivers/pci/endpoint/.

But let's first settle on the structure of the devicetree binding first.

Thanks,
Mani

> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  drivers/pci/of.c       | 180 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_pci.h |  15 ++++
>  2 files changed, 195 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index cb2e8351c2cc..198f08351070 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -605,6 +605,186 @@ 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_OF_DYNAMIC)
> +
> +static void devm_of_pci_destroy_bus_endpoint(struct device *dev, void *res)
> +{
> +	struct device_node *node = res;
> +
> +	of_detach_node(node);
> +}
> +
> +static int of_ep_add_property(struct device *dev, struct property **proplist, const char *name,
> +			      const int length, void *value)
> +{
> +	struct property *new;
> +
> +	new = devm_kzalloc(dev, sizeof(*new), GFP_KERNEL);
> +	if (!new)
> +		return -ENOMEM;
> +
> +	new->name = devm_kstrdup(dev, name, GFP_KERNEL);
> +	if (!new->name)
> +		return -ENOMEM;
> +
> +	new->value = devm_kmalloc(dev, length, GFP_KERNEL);
> +	if (!new->value)
> +		return -ENOMEM;
> +
> +	memcpy(new->value, value, length);
> +	new->length = length;
> +	new->next = *proplist;
> +	*proplist = new;
> +
> +	return 0;
> +}
> +
> +static struct device_node *of_ep_alloc_node(struct pci_dev *pdev, const char *name)
> +{
> +	struct device_node *node;
> +	char *full_name;
> +
> +	node = devres_alloc(devm_of_pci_destroy_bus_endpoint, sizeof(*node), GFP_KERNEL);
> +	if (!node)
> +		return NULL;
> +
> +	full_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "/%s@%llx", name,
> +				   (u64)pci_resource_start(pdev, 0));
> +	if (!full_name)
> +		return NULL;
> +
> +	node->parent = of_root;
> +	node->full_name = full_name;
> +	of_node_set_flag(node, OF_DYNAMIC);
> +	of_node_init(node);
> +
> +	return node;
> +}
> +
> +/**
> + * devm_of_pci_create_bus_endpoint - Create a device node for the given pci device.
> + * @pdev: PCI device pointer.
> + *
> + * For PCI device which uses flattened device tree to describe apertures in its BARs,
> + * a device node for the given pci device is required. Then the flattened device tree
> + * overlay from the device can be applied to the base tree.
> + * The device node is under root node and act like bus node. It contains a "ranges"
> + * property which is used for address translation of its children. Each child node
> + * corresponds an aperture and use BAR index and offset as its address.
> +
> + * Returns 0 on success or a negative error-code on failure.
> + */
> +int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev)
> +{
> +	struct property *proplist = NULL;
> +	struct device *dev = &pdev->dev;
> +	int range_ncells, addr_ncells;
> +	struct device_node *node;
> +	void *prop = NULL;
> +	u32 *range_cell;
> +	__be32 val;
> +	int i, ret;
> +
> +	node = of_ep_alloc_node(pdev, "pci-ep-bus");
> +	if (!node)
> +		return -ENOMEM;
> +
> +	/* the endpoint node works as 'simple-bus' to translate aperture addresses. */
> +	prop = "simple-bus";
> +	ret = of_ep_add_property(dev, &proplist, "compatible", strlen(prop) + 1, prop);
> +	if (ret)
> +		goto cleanup;
> +
> +	/* The address and size cells of nodes underneath are 2 */
> +	val = cpu_to_be32(2);
> +	ret = of_ep_add_property(dev, &proplist, "#address-cells", sizeof(u32), &val);
> +	if (ret)
> +		goto cleanup;
> +
> +	ret = of_ep_add_property(dev, &proplist, "#size-cells", sizeof(u32), &val);
> +	if (ret)
> +		goto cleanup;
> +
> +	/* child address format: 0xIooooooo oooooooo, I = bar index, o = offset on bar */
> +	addr_ncells = of_n_addr_cells(node);
> +	if (addr_ncells > 2) {
> +		/* does not support number of address cells greater than 2 */
> +		ret = -EINVAL;
> +		goto cleanup;
> +	}
> +
> +	/* range cells include <node addr cells> <child addr cells> <child size cells> */
> +	range_ncells = addr_ncells + 4;
> +	prop = kzalloc(range_ncells * sizeof(u32) * PCI_STD_NUM_BARS, GFP_KERNEL);
> +	if (!prop) {
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +
> +	range_cell = prop;
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +		if (!pci_resource_len(pdev, i))
> +			continue;
> +		/* highest 4 bits of address are bar index */
> +		*(__be64 *)range_cell = cpu_to_be64((u64)i << 60);
> +		range_cell += 2;
> +		if (addr_ncells == 2)
> +			*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_start(pdev, i));
> +		else
> +			*(__be32 *)range_cell = cpu_to_be32((u32)pci_resource_start(pdev, i));
> +
> +		range_cell += addr_ncells;
> +		*(__be64 *)range_cell = cpu_to_be64((u64)pci_resource_len(pdev, i));
> +		range_cell += 2;
> +	}
> +
> +	/* error out if there is not PCI BAR been found */
> +	if ((void *)range_cell == prop) {
> +		ret = -EINVAL;
> +		goto cleanup;
> +	}
> +
> +	ret = of_ep_add_property(dev, &proplist, "ranges", (void *)range_cell - prop, prop);
> +	kfree(prop);
> +	if (ret)
> +		goto cleanup;
> +
> +	node->properties = proplist;
> +	ret = of_attach_node(node);
> +	if (ret)
> +		goto cleanup;
> +
> +	devres_add(dev, node);
> +
> +	return 0;
> +
> +cleanup:
> +	kfree(prop);
> +	if (node)
> +		devres_free(node);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_of_pci_create_bus_endpoint);
> +
> +struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev)
> +{
> +	struct device_node *dn;
> +	char *path;
> +
> +	path = kasprintf(GFP_KERNEL, "/pci-ep-bus@%llx",
> +			 (u64)pci_resource_start(pdev, 0));
> +	if (!path)
> +		return NULL;
> +
> +	dn = of_find_node_by_path(path);
> +	kfree(path);
> +
> +	return dn;
> +}
> +EXPORT_SYMBOL_GPL(of_pci_find_bus_endpoint);
> +#endif /* CONFIG_OF_DYNAMIC */
> +
>  #endif /* CONFIG_PCI */
>  
>  /**
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 29658c0ee71f..c1d86be321b2 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -38,4 +38,19 @@ of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_OF_DYNAMIC) && IS_ENABLED(CONFIG_PCI)
> +int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev);
> +struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev);
> +#else
> +static inline int devm_of_pci_create_bus_endpoint(struct pci_dev *pdev)
> +{
> +	return -EINVAL;
> +}
> +
> +static inline struct device_node *of_pci_find_bus_endpoint(struct pci_dev *pdev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>  #endif
> -- 
> 2.27.0
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Lizhi Hou]

Create device tree binding document for PCIe endpoint bus.

Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
Signed-off-by: Max Zhen <max.zhen@xilinx.com>
Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
---
 .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml

diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
new file mode 100644
index 000000000000..0ca96298db6f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PCIe Endpoint Bus binding
+
+description: |
+  PCIe device may use flattened device tree to describe apertures in its
+  PCIe BARs. The Bus PCIe endpoint node is created and attached under the
+  device tree root node for this kind of device. Then the flatten device
+  tree overlay for this device is attached under the endpoint node.
+
+  The aperture address which is under the endpoint node consists of BAR
+  index and offset. It uses the following encoding:
+
+    0xIooooooo 0xoooooooo
+
+  Where:
+
+    I = BAR index
+    oooooo oooooooo = BAR offset
+
+  The endpoint is compatible with 'simple-bus' and contains 'ranges'
+  property for translating aperture address to CPU address.
+
+allOf:
+  - $ref: /schemas/simple-bus.yaml#
+
+maintainers:
+  - Lizhi Hou <lizhi.hou@xilinx.com>
+
+properties:
+  compatible:
+    contains:
+      const: pci-ep-bus
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  ranges: true
+
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    description: hardware apertures belong to this device.
+    type: object
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        pci-ep-bus@e0000000 {
+            compatible = "pci-ep-bus", "simple-bus";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
+                      0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
+        };
+    };
-- 
2.27.0

Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Tom Rix]

Lizhi,

Sorry for the delay, I am fighting with checking this with 'make 
dt_binding_check'

There is a recent failure in linux-next around display/mediatek,* 
between next-20220301 and next-20220302 that I am bisecting.

There are a couple of checkpatch --strict warnings for this set, the 
obvious one is adding to the MAINTAINERS for new files.

Tom

On 3/4/22 9:23 PM, Lizhi Hou wrote:
> Create device tree binding document for PCIe endpoint bus.
>
> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>   .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 +++++++++++++++++++
>   1 file changed, 72 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>
> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> new file mode 100644
> index 000000000000..0ca96298db6f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe Endpoint Bus binding
> +
> +description: |
> +  PCIe device may use flattened device tree to describe apertures in its
> +  PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> +  device tree root node for this kind of device. Then the flatten device
> +  tree overlay for this device is attached under the endpoint node.
> +
> +  The aperture address which is under the endpoint node consists of BAR
> +  index and offset. It uses the following encoding:
> +
> +    0xIooooooo 0xoooooooo
> +
> +  Where:
> +
> +    I = BAR index
> +    oooooo oooooooo = BAR offset
> +
> +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
> +  property for translating aperture address to CPU address.
> +
> +allOf:
> +  - $ref: /schemas/simple-bus.yaml#
> +
> +maintainers:
> +  - Lizhi Hou <lizhi.hou@xilinx.com>
> +
> +properties:
> +  compatible:
> +    contains:
> +      const: pci-ep-bus
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    description: hardware apertures belong to this device.
> +    type: object
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        pci-ep-bus@e0000000 {
> +            compatible = "pci-ep-bus", "simple-bus";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
> +                      0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
> +        };
> +    };

Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Rob Herring]

On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <trix@redhat.com> wrote:
>
> Lizhi,
>
> Sorry for the delay, I am fighting with checking this with 'make
> dt_binding_check'
>
> There is a recent failure in linux-next around display/mediatek,*
> between next-20220301 and next-20220302 that I am bisecting.

There's already patches for that posted.

Just use 'make -k'.

>
> There are a couple of checkpatch --strict warnings for this set, the
> obvious one is adding to the MAINTAINERS for new files.
>
> Tom
>
> On 3/4/22 9:23 PM, Lizhi Hou wrote:
> > Create device tree binding document for PCIe endpoint bus.
> >
> > Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> > Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> > Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> > ---
> >   .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 +++++++++++++++++++
> >   1 file changed, 72 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> > new file mode 100644
> > index 000000000000..0ca96298db6f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: PCIe Endpoint Bus binding
> > +
> > +description: |
> > +  PCIe device may use flattened device tree to describe apertures in its
> > +  PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> > +  device tree root node for this kind of device. Then the flatten device
> > +  tree overlay for this device is attached under the endpoint node.
> > +
> > +  The aperture address which is under the endpoint node consists of BAR
> > +  index and offset. It uses the following encoding:
> > +
> > +    0xIooooooo 0xoooooooo
> > +
> > +  Where:
> > +
> > +    I = BAR index
> > +    oooooo oooooooo = BAR offset
> > +
> > +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
> > +  property for translating aperture address to CPU address.


This binding is completely confusing because 'PCIe endpoint' is
generally used (in context of bindings and Linux) for describing the
endpoint's system (i.e. the internal structure of a PCIe device (e.g.
add-in card) from the view of its own processor (not the host
system)). This binding seems to be describing the host system's view
of a PCIe device. We already have that! That's just the PCI bus
binding[1] which has existed for ~25 years.

For a non-DT system, what you are going to need here is some way to
create DT nodes of the PCI bus hierarchy or at least from your device
back up to the host bridge. I would suggest you solve that problem
separately from implementing the FPGA driver by making it work first
on a DT based system.

Rob

[1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf

Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Lizhi Hou]

Hi Rob,

On 3/7/22 06:07, Rob Herring wrote:
> On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <trix@redhat.com> wrote:
>> Lizhi,
>>
>> Sorry for the delay, I am fighting with checking this with 'make
>> dt_binding_check'
>>
>> There is a recent failure in linux-next around display/mediatek,*
>> between next-20220301 and next-20220302 that I am bisecting.
> There's already patches for that posted.
>
> Just use 'make -k'.
>
>> There are a couple of checkpatch --strict warnings for this set, the
>> obvious one is adding to the MAINTAINERS for new files.
>>
>> Tom
>>
>> On 3/4/22 9:23 PM, Lizhi Hou wrote:
>>> Create device tree binding document for PCIe endpoint bus.
>>>
>>> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
>>> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
>>> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
>>> ---
>>>    .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 +++++++++++++++++++
>>>    1 file changed, 72 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>> new file mode 100644
>>> index 000000000000..0ca96298db6f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>> @@ -0,0 +1,72 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: PCIe Endpoint Bus binding
>>> +
>>> +description: |
>>> +  PCIe device may use flattened device tree to describe apertures in its
>>> +  PCIe BARs. The Bus PCIe endpoint node is created and attached under the
>>> +  device tree root node for this kind of device. Then the flatten device
>>> +  tree overlay for this device is attached under the endpoint node.
>>> +
>>> +  The aperture address which is under the endpoint node consists of BAR
>>> +  index and offset. It uses the following encoding:
>>> +
>>> +    0xIooooooo 0xoooooooo
>>> +
>>> +  Where:
>>> +
>>> +    I = BAR index
>>> +    oooooo oooooooo = BAR offset
>>> +
>>> +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
>>> +  property for translating aperture address to CPU address.
>
> This binding is completely confusing because 'PCIe endpoint' is
> generally used (in context of bindings and Linux) for describing the
> endpoint's system (i.e. the internal structure of a PCIe device (e.g.
> add-in card) from the view of its own processor (not the host
> system)). This binding seems to be describing the host system's view
> of a PCIe device. We already have that! That's just the PCI bus
> binding[1] which has existed for ~25 years.
>
> For a non-DT system, what you are going to need here is some way to
> create DT nodes of the PCI bus hierarchy or at least from your device
> back up to the host bridge. I would suggest you solve that problem
> separately from implementing the FPGA driver by making it work first
> on a DT based system.
>
> Rob
>
> [1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
>
>
I investigated the implementation detail for adding device tree node for
PCIe devices. Based on my findings this is quite involved and so would
like to bounce off my approach with you before I start making changes.

We will start with DT-Base system which already has device node for PCIe
controller in base device tree. And we will focus on:

- Adding functions to generate device tree node for all PCIe devices.
- Linking dynamically generated DT nodes to the PCIe to the PCIe devices.

So the first question to resolve is when the PCIe DT node will be created
and destroyed. Here are the different options:

- Option #1: Add functions in pci_bus_add_device()/pci_stop_dev()
   - The same place for creating/destroying device sysfs node. This should
     be able to handle different cases: SR-IOV vf, hotplug, virtual device
     etc.
   - Leverage existing PCI enumeration and get the device information
     directly from pci_dev structure.

-  Option #2: Enumerate PCIe devices and create DT node without relying
    on PCI subsystem enumeration.
   - E.g. Enumerating and creating PCIe dt node in an init callback
     pure_initcall()
   - Linking dt node to PCIe device is already supported in
     pci_setup_device()
   - Eventually need to handle hotplug case separately.

Option #1 looks an easier and cleaner way to implement.

The second question is for linking the dynamic generated dt node to PCIe
device through pci_dev->dev.of_node. The biggest concern is that current
kernel and driver code may check of_node pointer and run complete different
code path if of_node is not NULL. Here are some examples.

  - of_irq_parse_pci(): 
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/of.c#L492
  - pci_msi_domain_get_msi_rid(): 
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/msi/irqdomain.c#L233
  - pci_dma_configure(): 
https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/pci-driver.c#L1610

It needs to identify all the places which use of_node and make sure using
dynamic generated of_node is equivalent with the code without using
of_node. Considering there are so many hardware and virtualization Linux
support, the risk might high for changing the code which has been existing
for long time. And how to validate the change could be another challenge.
Introducing compiling option (e.g. CONFIG_PCI_DT_NODE) may lower the risk.

There are other approaches for linking dt node to PCIe device.
- Option #a: Add a special flag or property to dynamic generated PCIe DT
   node. Then convert the code.

       if (pdev->dev.of_node)
           funcA();
       else
           funcB();

    to
       struct device_node *pci_get_of_node(pdev)
       {
           if (pdev->dev.of_node is dynamic generated)
               return NULL;
           return pdev->dev.of_node;
       }

       if (pci_get_of_node(pdev))
           funcA ();
       else
           funcB ();

- Option #b: Introduce a new data member "dyn_of_node" in struct device
   to link the dynamic generated PCIe dt node.

       struct device {
           ...
           of_node: Associated device tree node.
           fwnode: Associated device node supplied by platform firmware.
           dyn_of_node: Associated dynamic generated device tree node.
           ...

Could we implement Option #b for now because it is lower risk?

The last question is about the properties for each dynamic generated
PCIe dt node. To keep the generated device node lightweight, what will be
the desired (minimum) set of properties to generate? Besides the properties
defined in IEEE Std 1275, it looks more properties are needed. E.g.
     interrupt-map, interrupt-map-mask, ...

Thanks,
Lizhi

Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Lizhi Hou]

Hi Rob,


Do you have any comment on this? We are looking for guidance on the 
approaches suggested.


Thanks,

Lizhi

On 4/22/22 14:57, Lizhi Hou wrote:
> Hi Rob,
>
> On 3/7/22 06:07, Rob Herring wrote:
>> On Sun, Mar 6, 2022 at 9:37 AM Tom Rix <trix@redhat.com> wrote:
>>> Lizhi,
>>>
>>> Sorry for the delay, I am fighting with checking this with 'make
>>> dt_binding_check'
>>>
>>> There is a recent failure in linux-next around display/mediatek,*
>>> between next-20220301 and next-20220302 that I am bisecting.
>> There's already patches for that posted.
>>
>> Just use 'make -k'.
>>
>>> There are a couple of checkpatch --strict warnings for this set, the
>>> obvious one is adding to the MAINTAINERS for new files.
>>>
>>> Tom
>>>
>>> On 3/4/22 9:23 PM, Lizhi Hou wrote:
>>>> Create device tree binding document for PCIe endpoint bus.
>>>>
>>>> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
>>>> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
>>>> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
>>>> ---
>>>>    .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 
>>>> +++++++++++++++++++
>>>>    1 file changed, 72 insertions(+)
>>>>    create mode 100644 
>>>> Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml 
>>>> b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>> new file mode 100644
>>>> index 000000000000..0ca96298db6f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
>>>> @@ -0,0 +1,72 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: PCIe Endpoint Bus binding
>>>> +
>>>> +description: |
>>>> +  PCIe device may use flattened device tree to describe apertures 
>>>> in its
>>>> +  PCIe BARs. The Bus PCIe endpoint node is created and attached 
>>>> under the
>>>> +  device tree root node for this kind of device. Then the flatten 
>>>> device
>>>> +  tree overlay for this device is attached under the endpoint node.
>>>> +
>>>> +  The aperture address which is under the endpoint node consists 
>>>> of BAR
>>>> +  index and offset. It uses the following encoding:
>>>> +
>>>> +    0xIooooooo 0xoooooooo
>>>> +
>>>> +  Where:
>>>> +
>>>> +    I = BAR index
>>>> +    oooooo oooooooo = BAR offset
>>>> +
>>>> +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
>>>> +  property for translating aperture address to CPU address.
>>
>> This binding is completely confusing because 'PCIe endpoint' is
>> generally used (in context of bindings and Linux) for describing the
>> endpoint's system (i.e. the internal structure of a PCIe device (e.g.
>> add-in card) from the view of its own processor (not the host
>> system)). This binding seems to be describing the host system's view
>> of a PCIe device. We already have that! That's just the PCI bus
>> binding[1] which has existed for ~25 years.
>>
>> For a non-DT system, what you are going to need here is some way to
>> create DT nodes of the PCI bus hierarchy or at least from your device
>> back up to the host bridge. I would suggest you solve that problem
>> separately from implementing the FPGA driver by making it work first
>> on a DT based system.
>>
>> Rob
>>
>> [1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
>>
>>
> I investigated the implementation detail for adding device tree node for
> PCIe devices. Based on my findings this is quite involved and so would
> like to bounce off my approach with you before I start making changes.
>
> We will start with DT-Base system which already has device node for PCIe
> controller in base device tree. And we will focus on:
>
> - Adding functions to generate device tree node for all PCIe devices.
> - Linking dynamically generated DT nodes to the PCIe to the PCIe devices.
>
> So the first question to resolve is when the PCIe DT node will be created
> and destroyed. Here are the different options:
>
> - Option #1: Add functions in pci_bus_add_device()/pci_stop_dev()
>   - The same place for creating/destroying device sysfs node. This should
>     be able to handle different cases: SR-IOV vf, hotplug, virtual device
>     etc.
>   - Leverage existing PCI enumeration and get the device information
>     directly from pci_dev structure.
>
> -  Option #2: Enumerate PCIe devices and create DT node without relying
>    on PCI subsystem enumeration.
>   - E.g. Enumerating and creating PCIe dt node in an init callback
>     pure_initcall()
>   - Linking dt node to PCIe device is already supported in
>     pci_setup_device()
>   - Eventually need to handle hotplug case separately.
>
> Option #1 looks an easier and cleaner way to implement.
>
> The second question is for linking the dynamic generated dt node to PCIe
> device through pci_dev->dev.of_node. The biggest concern is that current
> kernel and driver code may check of_node pointer and run complete 
> different
> code path if of_node is not NULL. Here are some examples.
>
>  - of_irq_parse_pci(): 
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/of.c#L492
>  - pci_msi_domain_get_msi_rid(): 
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/msi/irqdomain.c#L233
>  - pci_dma_configure(): 
> https://elixir.bootlin.com/linux/v5.18-rc2/source/drivers/pci/pci-driver.c#L1610
>
> It needs to identify all the places which use of_node and make sure using
> dynamic generated of_node is equivalent with the code without using
> of_node. Considering there are so many hardware and virtualization Linux
> support, the risk might high for changing the code which has been 
> existing
> for long time. And how to validate the change could be another challenge.
> Introducing compiling option (e.g. CONFIG_PCI_DT_NODE) may lower the 
> risk.
>
> There are other approaches for linking dt node to PCIe device.
> - Option #a: Add a special flag or property to dynamic generated PCIe DT
>   node. Then convert the code.
>
>       if (pdev->dev.of_node)
>           funcA();
>       else
>           funcB();
>
>    to
>       struct device_node *pci_get_of_node(pdev)
>       {
>           if (pdev->dev.of_node is dynamic generated)
>               return NULL;
>           return pdev->dev.of_node;
>       }
>
>       if (pci_get_of_node(pdev))
>           funcA ();
>       else
>           funcB ();
>
> - Option #b: Introduce a new data member "dyn_of_node" in struct device
>   to link the dynamic generated PCIe dt node.
>
>       struct device {
>           ...
>           of_node: Associated device tree node.
>           fwnode: Associated device node supplied by platform firmware.
>           dyn_of_node: Associated dynamic generated device tree node.
>           ...
>
> Could we implement Option #b for now because it is lower risk?
>
> The last question is about the properties for each dynamic generated
> PCIe dt node. To keep the generated device node lightweight, what will be
> the desired (minimum) set of properties to generate? Besides the 
> properties
> defined in IEEE Std 1275, it looks more properties are needed. E.g.
>     interrupt-map, interrupt-map-mask, ...
>
> Thanks,
> Lizhi
>

Re: [PATCH V1 RESEND 2/4] Documentation: devicetree: bindings: add binding for PCIe endpoint bus

[Manivannan Sadhasivam]

Hi,

+ Kishon

On Fri, Mar 04, 2022 at 09:23:02PM -0800, Lizhi Hou wrote:
> Create device tree binding document for PCIe endpoint bus.
> 

I'm currently working on a PCI endpoint function driver for MHI bus [1] and
hence interested in this topic.

Comments below.

[1] https://lore.kernel.org/lkml/20220502060611.58987-9-manivannan.sadhasivam@linaro.org/

> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  .../devicetree/bindings/bus/pci-ep-bus.yaml   | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> new file mode 100644
> index 000000000000..0ca96298db6f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/pci-ep-bus.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/pci-ep-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe Endpoint Bus binding
> +
> +description: |
> +  PCIe device may use flattened device tree to describe apertures in its
> +  PCIe BARs. The Bus PCIe endpoint node is created and attached under the
> +  device tree root node for this kind of device. Then the flatten device
> +  tree overlay for this device is attached under the endpoint node.
> +
> +  The aperture address which is under the endpoint node consists of BAR
> +  index and offset. It uses the following encoding:
> +

On top of Rob's reply:

Currently, the BAR memory for the PCI endpoint device is either allocated
dynamically using pci_epf_alloc_space() or we need to pass the address in
"phys_addr" field of "struct pci_epf_bar".

In most of the PCI endpoint devices, we need to use a fixed memory region as
the BAR. Since there is no devicetree integration for PCI endpoint subsystem,
I've been using the 2nd approach of obtaining the BAR address from PCI endpoint
controller devicetree node and passing it to "phys_addr" of
"struct pci_epf_bar".

Ideally, the BAR information should come from the devicetree. But we cannot use
just "pci-ep-bus" node. I've been thinking about the below structure:

pcie_ep: pcie-ep@40000000 {
        compatible = "pcie-ep";
	....

	pci_epf_0: pci-epf@100000 {
		reg = <0x100000 0x1000>; # BAR0
	};

	pci_epf_1: pci-epf@200000 {
		reg = <0x200000 0x1000>; # BAR1
	};
};

Where, "pci-epf@" represents each of the PCI endpoint functions implemented by
this device (note that there can be more than one function per endpoint device)
and "reg" has the BAR address/size for that function.

Rob, what do you think?

Thanks,
Mani

> +    0xIooooooo 0xoooooooo
> +
> +  Where:
> +
> +    I = BAR index
> +    oooooo oooooooo = BAR offset
> +
> +  The endpoint is compatible with 'simple-bus' and contains 'ranges'
> +  property for translating aperture address to CPU address.
> +
> +allOf:
> +  - $ref: /schemas/simple-bus.yaml#
> +
> +maintainers:
> +  - Lizhi Hou <lizhi.hou@xilinx.com>
> +
> +properties:
> +  compatible:
> +    contains:
> +      const: pci-ep-bus
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^.*@[0-9a-f]+$":
> +    description: hardware apertures belong to this device.
> +    type: object
> +
> +required:
> +  - compatible
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        pci-ep-bus@e0000000 {
> +            compatible = "pci-ep-bus", "simple-bus";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges = <0x0 0x0 0x0 0xe0000000 0x0 0x2000000
> +                      0x20000000 0x0 0x0 0xe4200000 0x0 0x40000>;
> +        };
> +    };
> -- 
> 2.27.0
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V1 RESEND 3/4] fpga: xrt: management physical function driver

[Lizhi Hou]

The PCIe device driver which attaches to management function on Alveo
devices. The first version of this driver demonstrates calling PCIe
interface to create device tree node.

Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
Signed-off-by: Max Zhen <max.zhen@xilinx.com>
Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
---
 drivers/fpga/Kconfig              |  3 ++
 drivers/fpga/Makefile             |  3 ++
 drivers/fpga/xrt/Kconfig          | 24 ++++++++++++
 drivers/fpga/xrt/Makefile         |  8 ++++
 drivers/fpga/xrt/mgmt/Makefile    | 12 ++++++
 drivers/fpga/xrt/mgmt/xmgmt-drv.c | 63 +++++++++++++++++++++++++++++++
 6 files changed, 113 insertions(+)
 create mode 100644 drivers/fpga/xrt/Kconfig
 create mode 100644 drivers/fpga/xrt/Makefile
 create mode 100644 drivers/fpga/xrt/mgmt/Makefile
 create mode 100644 drivers/fpga/xrt/mgmt/xmgmt-drv.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 991b3f361ec9..93ae387c97c5 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -243,4 +243,7 @@ config FPGA_MGR_VERSAL_FPGA
 	  configure the programmable logic(PL).
 
 	  To compile this as a module, choose M here.
+
+source "drivers/fpga/xrt/Kconfig"
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 0bff783d1b61..81ea43c40c64 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -49,3 +49,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
 
 # Drivers for FPGAs which implement DFL
 obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
+
+# XRT drivers for Xilinx Alveo platforms
+obj-$(CONFIG_FPGA_XRT)		+= xrt/
diff --git a/drivers/fpga/xrt/Kconfig b/drivers/fpga/xrt/Kconfig
new file mode 100644
index 000000000000..47efc8f71cec
--- /dev/null
+++ b/drivers/fpga/xrt/Kconfig
@@ -0,0 +1,24 @@
+
+# XRT Alveo FPGA device configuration
+#
+
+config FPGA_XRT
+	tristate "XRT Alveo Drivers"
+	depends on OF
+	select OF_EMPTY_ROOT
+	select OF_OVERLAY
+	help
+	  Select this option to enable Xilinx XRT Alveo drivers. Xilinx Alveo
+	  card is PCIe device and has two PCIe functions. The first function
+	  performs board manangement and XRT management driver will be attached
+	  to it. The second function performs data movement, compute unit
+	  scheduling etc. And an XRT user driver will be attached to it.
+
+config FPGA_XRT_XMGMT
+	tristate "Xilinx Alveo Management Driver"
+	depends on FPGA_XRT
+	help
+	  Select this option to enable XRT PCIe driver for Xilinx Alveo FPGA.
+	  This driver provides interfaces for userspace application to access
+	  Alveo FPGA device, such as: downloading FPGA bitstream, query card
+	  information, hot reset card etc.
diff --git a/drivers/fpga/xrt/Makefile b/drivers/fpga/xrt/Makefile
new file mode 100644
index 000000000000..2d251b5653bb
--- /dev/null
+++ b/drivers/fpga/xrt/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2020-2022 Xilinx, Inc. All rights reserved.
+#
+# Authors: Lizhi.Hou@xilinx.com
+#
+
+obj-$(CONFIG_FPGA_XRT_XMGMT) += mgmt/
diff --git a/drivers/fpga/xrt/mgmt/Makefile b/drivers/fpga/xrt/mgmt/Makefile
new file mode 100644
index 000000000000..b893c7293d70
--- /dev/null
+++ b/drivers/fpga/xrt/mgmt/Makefile
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2020-2022 Xilinx, Inc. All rights reserved.
+#
+# Authors: Sonal.Santan@xilinx.com
+#          Lizhi.Hou@xilinx.com
+#
+
+obj-$(CONFIG_FPGA_XRT_XMGMT) += xrt-mgmt.o
+
+xrt-mgmt-objs :=		\
+	xmgmt-drv.o
diff --git a/drivers/fpga/xrt/mgmt/xmgmt-drv.c b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
new file mode 100644
index 000000000000..60742a478a43
--- /dev/null
+++ b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Alveo Management Function Driver
+ *
+ * Copyright (C) 2020-2022 Xilinx, Inc.
+ *
+ * Authors:
+ *     Cheng Zhen <maxz@xilinx.com>
+ *     Lizhi Hou <lizhih@xilinx.com>
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/aer.h>
+#include <linux/vmalloc.h>
+#include <linux/delay.h>
+#include <linux/of_pci.h>
+
+#define XMGMT_MODULE_NAME	"xrt-mgmt"
+
+/* PCI Device IDs */
+#define PCI_DEVICE_ID_U50	0x5020
+static const struct pci_device_id xmgmt_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_XILINX, PCI_DEVICE_ID_U50), }, /* Alveo U50 */
+	{ 0, }
+};
+
+static int xmgmt_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	devm_of_pci_create_bus_endpoint(pdev);
+
+	return 0;
+}
+
+static struct pci_driver xmgmt_driver = {
+	.name = XMGMT_MODULE_NAME,
+	.id_table = xmgmt_pci_ids,
+	.probe = xmgmt_probe,
+};
+
+static int __init xmgmt_init(void)
+{
+	int res;
+
+	res = pci_register_driver(&xmgmt_driver);
+	if (res)
+		return res;
+
+	return 0;
+}
+
+static __exit void xmgmt_exit(void)
+{
+	pci_unregister_driver(&xmgmt_driver);
+}
+
+module_init(xmgmt_init);
+module_exit(xmgmt_exit);
+
+MODULE_DEVICE_TABLE(pci, xmgmt_pci_ids);
+MODULE_AUTHOR("XRT Team <runtime@xilinx.com>");
+MODULE_DESCRIPTION("Xilinx Alveo management function driver");
+MODULE_LICENSE("GPL v2");
-- 
2.27.0

Re: [PATCH V1 RESEND 3/4] fpga: xrt: management physical function driver

[Manivannan Sadhasivam]

+ Kishon

On Fri, Mar 04, 2022 at 09:23:03PM -0800, Lizhi Hou wrote:
> The PCIe device driver which attaches to management function on Alveo
> devices. The first version of this driver demonstrates calling PCIe
> interface to create device tree node.
> 

I'm assuming that this driver implements the PCI endpoint functions. Then this
driver should be under drivers/pci/endpoint/functions/ making use of the
existing PCI endpoint subsystem.

Thanks,
Mani

> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  drivers/fpga/Kconfig              |  3 ++
>  drivers/fpga/Makefile             |  3 ++
>  drivers/fpga/xrt/Kconfig          | 24 ++++++++++++
>  drivers/fpga/xrt/Makefile         |  8 ++++
>  drivers/fpga/xrt/mgmt/Makefile    | 12 ++++++
>  drivers/fpga/xrt/mgmt/xmgmt-drv.c | 63 +++++++++++++++++++++++++++++++
>  6 files changed, 113 insertions(+)
>  create mode 100644 drivers/fpga/xrt/Kconfig
>  create mode 100644 drivers/fpga/xrt/Makefile
>  create mode 100644 drivers/fpga/xrt/mgmt/Makefile
>  create mode 100644 drivers/fpga/xrt/mgmt/xmgmt-drv.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 991b3f361ec9..93ae387c97c5 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -243,4 +243,7 @@ config FPGA_MGR_VERSAL_FPGA
>  	  configure the programmable logic(PL).
>  
>  	  To compile this as a module, choose M here.
> +
> +source "drivers/fpga/xrt/Kconfig"
> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 0bff783d1b61..81ea43c40c64 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -49,3 +49,6 @@ obj-$(CONFIG_FPGA_DFL_NIOS_INTEL_PAC_N3000)	+= dfl-n3000-nios.o
>  
>  # Drivers for FPGAs which implement DFL
>  obj-$(CONFIG_FPGA_DFL_PCI)		+= dfl-pci.o
> +
> +# XRT drivers for Xilinx Alveo platforms
> +obj-$(CONFIG_FPGA_XRT)		+= xrt/
> diff --git a/drivers/fpga/xrt/Kconfig b/drivers/fpga/xrt/Kconfig
> new file mode 100644
> index 000000000000..47efc8f71cec
> --- /dev/null
> +++ b/drivers/fpga/xrt/Kconfig
> @@ -0,0 +1,24 @@
> +
> +# XRT Alveo FPGA device configuration
> +#
> +
> +config FPGA_XRT
> +	tristate "XRT Alveo Drivers"
> +	depends on OF
> +	select OF_EMPTY_ROOT
> +	select OF_OVERLAY
> +	help
> +	  Select this option to enable Xilinx XRT Alveo drivers. Xilinx Alveo
> +	  card is PCIe device and has two PCIe functions. The first function
> +	  performs board manangement and XRT management driver will be attached
> +	  to it. The second function performs data movement, compute unit
> +	  scheduling etc. And an XRT user driver will be attached to it.
> +
> +config FPGA_XRT_XMGMT
> +	tristate "Xilinx Alveo Management Driver"
> +	depends on FPGA_XRT
> +	help
> +	  Select this option to enable XRT PCIe driver for Xilinx Alveo FPGA.
> +	  This driver provides interfaces for userspace application to access
> +	  Alveo FPGA device, such as: downloading FPGA bitstream, query card
> +	  information, hot reset card etc.
> diff --git a/drivers/fpga/xrt/Makefile b/drivers/fpga/xrt/Makefile
> new file mode 100644
> index 000000000000..2d251b5653bb
> --- /dev/null
> +++ b/drivers/fpga/xrt/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2020-2022 Xilinx, Inc. All rights reserved.
> +#
> +# Authors: Lizhi.Hou@xilinx.com
> +#
> +
> +obj-$(CONFIG_FPGA_XRT_XMGMT) += mgmt/
> diff --git a/drivers/fpga/xrt/mgmt/Makefile b/drivers/fpga/xrt/mgmt/Makefile
> new file mode 100644
> index 000000000000..b893c7293d70
> --- /dev/null
> +++ b/drivers/fpga/xrt/mgmt/Makefile
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2020-2022 Xilinx, Inc. All rights reserved.
> +#
> +# Authors: Sonal.Santan@xilinx.com
> +#          Lizhi.Hou@xilinx.com
> +#
> +
> +obj-$(CONFIG_FPGA_XRT_XMGMT) += xrt-mgmt.o
> +
> +xrt-mgmt-objs :=		\
> +	xmgmt-drv.o
> diff --git a/drivers/fpga/xrt/mgmt/xmgmt-drv.c b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
> new file mode 100644
> index 000000000000..60742a478a43
> --- /dev/null
> +++ b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx Alveo Management Function Driver
> + *
> + * Copyright (C) 2020-2022 Xilinx, Inc.
> + *
> + * Authors:
> + *     Cheng Zhen <maxz@xilinx.com>
> + *     Lizhi Hou <lizhih@xilinx.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/aer.h>
> +#include <linux/vmalloc.h>
> +#include <linux/delay.h>
> +#include <linux/of_pci.h>
> +
> +#define XMGMT_MODULE_NAME	"xrt-mgmt"
> +
> +/* PCI Device IDs */
> +#define PCI_DEVICE_ID_U50	0x5020
> +static const struct pci_device_id xmgmt_pci_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_XILINX, PCI_DEVICE_ID_U50), }, /* Alveo U50 */
> +	{ 0, }
> +};
> +
> +static int xmgmt_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	devm_of_pci_create_bus_endpoint(pdev);
> +
> +	return 0;
> +}
> +
> +static struct pci_driver xmgmt_driver = {
> +	.name = XMGMT_MODULE_NAME,
> +	.id_table = xmgmt_pci_ids,
> +	.probe = xmgmt_probe,
> +};
> +
> +static int __init xmgmt_init(void)
> +{
> +	int res;
> +
> +	res = pci_register_driver(&xmgmt_driver);
> +	if (res)
> +		return res;
> +
> +	return 0;
> +}
> +
> +static __exit void xmgmt_exit(void)
> +{
> +	pci_unregister_driver(&xmgmt_driver);
> +}
> +
> +module_init(xmgmt_init);
> +module_exit(xmgmt_exit);
> +
> +MODULE_DEVICE_TABLE(pci, xmgmt_pci_ids);
> +MODULE_AUTHOR("XRT Team <runtime@xilinx.com>");
> +MODULE_DESCRIPTION("Xilinx Alveo management function driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.27.0
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V1 RESEND 3/4] fpga: xrt: management physical function driver

[Bjorn Helgaas]

On Fri, Mar 04, 2022 at 09:23:03PM -0800, Lizhi Hou wrote:
> The PCIe device driver which attaches to management function on Alveo
> devices. The first version of this driver demonstrates calling PCIe
> interface to create device tree node.
> ...

> +static int __init xmgmt_init(void)
> +{
> +	int res;
> +
> +	res = pci_register_driver(&xmgmt_driver);
> +	if (res)
> +		return res;
> +
> +	return 0;

This is the same as:

  return pci_register_driver(&xmgmt_driver);

Bjorn

[PATCH V1 RESEND 4/4] of: enhance overlay applying interface to specific target base node

[Lizhi Hou]

The self discovered device, e.g. PCIe device, may carry a device tree
overlay to describe its downstream devices. In this case, the device node
is created dynamically by device driver. And in the overlay it does not
specify a fixed target path. Instead, a relative path to the device node
is specified.
Thus, a new parameter is added to overlay applying interface. This
parameter is the pointer of target base node. Then the entire overlay
target path is target base node path plus the relative path specified in
the device tree overlay.

Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
Signed-off-by: Max Zhen <max.zhen@xilinx.com>
Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
---
 drivers/fpga/xrt/mgmt/Makefile       |  1 +
 drivers/fpga/xrt/mgmt/dt-test.dts    | 15 ++++++++++
 drivers/fpga/xrt/mgmt/dt-test.h      | 15 ++++++++++
 drivers/fpga/xrt/mgmt/xmgmt-drv.c    | 41 +++++++++++++++++++++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_of.c |  2 +-
 drivers/of/overlay.c                 | 37 +++++++++++++++++--------
 drivers/of/unittest.c                |  2 +-
 include/linux/of.h                   |  2 +-
 8 files changed, 100 insertions(+), 15 deletions(-)
 create mode 100644 drivers/fpga/xrt/mgmt/dt-test.dts
 create mode 100644 drivers/fpga/xrt/mgmt/dt-test.h

diff --git a/drivers/fpga/xrt/mgmt/Makefile b/drivers/fpga/xrt/mgmt/Makefile
index b893c7293d70..a87ab1f6b403 100644
--- a/drivers/fpga/xrt/mgmt/Makefile
+++ b/drivers/fpga/xrt/mgmt/Makefile
@@ -9,4 +9,5 @@
 obj-$(CONFIG_FPGA_XRT_XMGMT) += xrt-mgmt.o
 
 xrt-mgmt-objs :=		\
+	dt-test.dtb.o		\
 	xmgmt-drv.o
diff --git a/drivers/fpga/xrt/mgmt/dt-test.dts b/drivers/fpga/xrt/mgmt/dt-test.dts
new file mode 100644
index 000000000000..3bc3d0c95180
--- /dev/null
+++ b/drivers/fpga/xrt/mgmt/dt-test.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+/ {
+	fragment@0 {
+		target-path="";
+		__overlay__ {
+			pr_isolate_ulp@0,41000 {
+				compatible = "xlnx,alveo-pr-isolation";
+				reg = <0x0 0x41000 0x0 0x1000>;
+			};
+		};
+	};
+};
+
diff --git a/drivers/fpga/xrt/mgmt/dt-test.h b/drivers/fpga/xrt/mgmt/dt-test.h
new file mode 100644
index 000000000000..dee0de570c8a
--- /dev/null
+++ b/drivers/fpga/xrt/mgmt/dt-test.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020-2022 Xilinx, Inc.
+ *
+ * Authors:
+ *     Lizhi Hou <lizhih@xilinx.com>
+ */
+
+#ifndef _DT_TEST_H_
+#define _DT_TEST_H_
+
+extern u8 __dtb_dt_test_begin[];
+extern u8 __dtb_dt_test_end[];
+
+#endif /* _DT_TEST_H_ */
diff --git a/drivers/fpga/xrt/mgmt/xmgmt-drv.c b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
index 60742a478a43..54078ffe37fe 100644
--- a/drivers/fpga/xrt/mgmt/xmgmt-drv.c
+++ b/drivers/fpga/xrt/mgmt/xmgmt-drv.c
@@ -14,8 +14,11 @@
 #include <linux/aer.h>
 #include <linux/vmalloc.h>
 #include <linux/delay.h>
+#include <linux/of.h>
 #include <linux/of_pci.h>
 
+#include "dt-test.h"
+
 #define XMGMT_MODULE_NAME	"xrt-mgmt"
 
 /* PCI Device IDs */
@@ -25,17 +28,53 @@ static const struct pci_device_id xmgmt_pci_ids[] = {
 	{ 0, }
 };
 
+struct xmgmt {
+	int ovcs_id;
+};
+
 static int xmgmt_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
-	devm_of_pci_create_bus_endpoint(pdev);
+	struct device_node *dn;
+	struct xmgmt *xm;
+	int ret;
+
+	xm = devm_kzalloc(&pdev->dev, sizeof(*xm), GFP_KERNEL);
+	if (!xm)
+		return -ENOMEM;
+	pci_set_drvdata(pdev, xm);
+
+	ret = devm_of_pci_create_bus_endpoint(pdev);
+	if (ret)
+		return ret;
+
+	dn = of_pci_find_bus_endpoint(pdev);
+	if (!dn) {
+		dev_err(&pdev->dev, "does not find bus endpoint");
+		return -EINVAL;
+	}
+
+	ret = of_overlay_fdt_apply(__dtb_dt_test_begin,
+				   (u32)(__dtb_dt_test_end - __dtb_dt_test_begin),
+				   &xm->ovcs_id, dn);
+	of_node_put(dn);
+	if (ret)
+		return ret;
 
 	return 0;
 }
 
+static void xmgmt_remove(struct pci_dev *pdev)
+{
+	struct xmgmt *xm = pci_get_drvdata(pdev);
+
+	of_overlay_remove(&xm->ovcs_id);
+}
+
 static struct pci_driver xmgmt_driver = {
 	.name = XMGMT_MODULE_NAME,
 	.id_table = xmgmt_pci_ids,
 	.probe = xmgmt_probe,
+	.remove = xmgmt_remove,
 };
 
 static int __init xmgmt_init(void)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c
index afef69669bb4..8799ed235f83 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -60,7 +60,7 @@ static int __init rcar_du_of_apply_overlay(const struct rcar_du_of_overlay *dtbs
 
 	ovcs_id = 0;
 	return of_overlay_fdt_apply(dtb->begin, dtb->end - dtb->begin,
-				    &ovcs_id);
+				    &ovcs_id, NULL);
 }
 
 static int __init rcar_du_of_add_property(struct of_changeset *ocs,
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index d80160cf34bb..4d192dff5055 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -686,9 +686,11 @@ static int build_changeset(struct overlay_changeset *ovcs)
  * 1) "target" property containing the phandle of the target
  * 2) "target-path" property containing the path of the target
  */
-static struct device_node *find_target(struct device_node *info_node)
+static struct device_node *find_target(struct device_node *info_node,
+				       struct device_node *target_base)
 {
 	struct device_node *node;
+	char *target_path;
 	const char *path;
 	u32 val;
 	int ret;
@@ -704,10 +706,23 @@ static struct device_node *find_target(struct device_node *info_node)
 
 	ret = of_property_read_string(info_node, "target-path", &path);
 	if (!ret) {
-		node =  of_find_node_by_path(path);
-		if (!node)
-			pr_err("find target, node: %pOF, path '%s' not found\n",
-			       info_node, path);
+		if (target_base) {
+			target_path = kasprintf(GFP_KERNEL, "%pOF%s", target_base, path);
+			if (!target_path)
+				return NULL;
+			node = of_find_node_by_path(target_path);
+			if (!node) {
+				pr_err("find target, node: %pOF, path '%s' not found\n",
+				       info_node, target_path);
+			}
+			kfree(target_path);
+		} else {
+			node =  of_find_node_by_path(path);
+			if (!node) {
+				pr_err("find target, node: %pOF, path '%s' not found\n",
+				       info_node, path);
+			}
+		}
 		return node;
 	}
 
@@ -730,7 +745,7 @@ static struct device_node *find_target(struct device_node *info_node)
  * detected in @tree, or -ENOSPC if idr_alloc() error.
  */
 static int init_overlay_changeset(struct overlay_changeset *ovcs,
-		const void *fdt, struct device_node *tree)
+		const void *fdt, struct device_node *tree, struct device_node *target_base)
 {
 	struct device_node *node, *overlay_node;
 	struct fragment *fragment;
@@ -792,7 +807,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
 
 		fragment = &fragments[cnt];
 		fragment->overlay = overlay_node;
-		fragment->target = find_target(node);
+		fragment->target = find_target(node, target_base);
 		if (!fragment->target) {
 			of_node_put(fragment->overlay);
 			ret = -EINVAL;
@@ -914,7 +929,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
  */
 
 static int of_overlay_apply(const void *fdt, struct device_node *tree,
-		int *ovcs_id)
+		int *ovcs_id, struct device_node *base)
 {
 	struct overlay_changeset *ovcs;
 	int ret = 0, ret_revert, ret_tmp;
@@ -947,7 +962,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
 	if (ret)
 		goto err_free_tree;
 
-	ret = init_overlay_changeset(ovcs, fdt, tree);
+	ret = init_overlay_changeset(ovcs, fdt, tree, base);
 	if (ret)
 		goto err_free_tree;
 
@@ -1015,7 +1030,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
 }
 
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
-			 int *ovcs_id)
+			 int *ovcs_id, struct device_node *base)
 {
 	void *new_fdt;
 	void *new_fdt_align;
@@ -1053,7 +1068,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 		goto out_free_new_fdt;
 	}
 
-	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
+	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id, base);
 	if (ret < 0) {
 		/*
 		 * new_fdt and overlay_root now belong to the overlay
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 70992103c07d..471bf6f2ec9a 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -3010,7 +3010,7 @@ static int __init overlay_data_apply(const char *overlay_name, int *ovcs_id)
 	if (!size)
 		pr_err("no overlay data for %s\n", overlay_name);
 
-	ret = of_overlay_fdt_apply(info->dtb_begin, size, &info->ovcs_id);
+	ret = of_overlay_fdt_apply(info->dtb_begin, size, &info->ovcs_id, NULL);
 	if (ovcs_id)
 		*ovcs_id = info->ovcs_id;
 	if (ret < 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 2dc77430a91a..7c5ecafb98c8 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1567,7 +1567,7 @@ struct of_overlay_notify_data {
 #ifdef CONFIG_OF_OVERLAY
 
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
-			 int *ovcs_id);
+			 int *ovcs_id, struct device_node *target_base);
 int of_overlay_remove(int *ovcs_id);
 int of_overlay_remove_all(void);
 
-- 
2.27.0

Re: [PATCH V1 RESEND 4/4] of: enhance overlay applying interface to specific target base node

[Rob Herring]

On Fri, Mar 04, 2022 at 09:23:04PM -0800, Lizhi Hou wrote:
> The self discovered device, e.g. PCIe device, may carry a device tree
> overlay to describe its downstream devices. In this case, the device node
> is created dynamically by device driver. And in the overlay it does not
> specify a fixed target path. Instead, a relative path to the device node
> is specified.
> Thus, a new parameter is added to overlay applying interface. This
> parameter is the pointer of target base node. Then the entire overlay
> target path is target base node path plus the relative path specified in
> the device tree overlay.
> 
> Signed-off-by: Sonal Santan <sonal.santan@xilinx.com>
> Signed-off-by: Max Zhen <max.zhen@xilinx.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@xilinx.com>
> ---
>  drivers/fpga/xrt/mgmt/Makefile       |  1 +
>  drivers/fpga/xrt/mgmt/dt-test.dts    | 15 ++++++++++
>  drivers/fpga/xrt/mgmt/dt-test.h      | 15 ++++++++++
>  drivers/fpga/xrt/mgmt/xmgmt-drv.c    | 41 +++++++++++++++++++++++++++-
>  drivers/gpu/drm/rcar-du/rcar_du_of.c |  2 +-
>  drivers/of/overlay.c                 | 37 +++++++++++++++++--------
>  drivers/of/unittest.c                |  2 +-
>  include/linux/of.h                   |  2 +-

The DT changes should be a 2nd patch.

That's the least of all the problems with this series though.

Rob

Re: [PATCH V1 RESEND 0/4] Infrastructure to define apertures in a PCIe device with a flattened device tree

[Bjorn Helgaas]

On Fri, Mar 04, 2022 at 09:23:00PM -0800, Lizhi Hou wrote:
> Hello,

Why is this a resend?  I see
https://lore.kernel.org/r/20220305051105.725838-1-lizhi.hou@xilinx.com,
which looks like it was posted just a few minutes before this.  I
assume this "RESEND" is identical?

> This V1 of patch series is to provide the required pci OF interfaces for
> the PCIe device which uses flattened device tree to describe apertures in
> its PCIe BARs. e.g, Xilinx Alveo PCIe accelerator. This requires a base
> device tree which contains nodes for PCIe devices. A PCIe device driver
> can then overlay a flattened device tree on the PCIe device tree node.
> There are two separate parts for this to work. First, not all system has
> a base device tree created by default. Thus, a patch to create an empty
> device tree root node has been submitted.
>   https://lore.kernel.org/lkml/20220216050056.311496-1-lizhi.hou@xilinx.com/
> Second, PCIe is self discoverable bus and there might not be a device tree
> node created for PCIe device. This patch provides a new interface to create
> a ‘pci-ep-bus’ node under the base device tree root node. PCIe device
> driver may call this interface in its probe routine to create device tree
> node, then overlays its device tree to the node.
> For the overlayed device tree nodes, each node presents a hardware aperture
> implemented in its PCIe BARs. The aperture register address consists of BAR
> index and offset. It uses the following encoding:
>   0xIooooooo 0xoooooooo
> Where:
>   I = BAR index
>   ooooooo oooooooo = BAR offset
> The ‘pci-ep-bus’ node been created is compatible with ‘simple-bus’ and
> contains ‘ranges’ property for translating aperture address to CPU address.
> The last patch enhances of_overlay_fdt_apply(). The ‘pci-ep-bus’ device
> node is created dynamically. The flattened device tree may not specify an
> fixed target overlay path in front. Instead, a relative path to the
> ‘pci-ep-bus’ node is specified in the flattened tree. Thus, a new
> parameter is added to point the target base node which is ‘pci-ep-bus’
> node in this case. Then the entire overlay target path is target base node
> path plus the relative path specified in the flattened device tree.

s/pci/PCI/ (capitalize acronyms above, also in other subjects, commit
logs, and code comments)

s/PCIe/PCI/ (in most cases, the above is not PCIe-specific)

Please add blank lines between paragraphs to make this easier to read.

The above tells *what* this series does, but not *why* we need it.

Apparently you want to describe PCI BARs in DT.  Normally the PCI core
discovers devices and BARs using the PCI enumeration process (read
config space looking for a Device ID, read standard BAR locations
(unimplemented BARs are hardwired to zero)).  Obviously you know all
of this.  What we need here (and in the commit log for the relevant
patch) is some explanation about why this standard process doesn't
work and you need to do something via DT.

I'm guessing this is for the case where Linux is running *on* the
endpoint, so instead of enumerating devices from the perspective of a
PCI host controller, it's on the "other" side of the device, e.g., as
described in Documentation/PCI/endpoint/pci-endpoint.rst

So the commit log should mention that and explain why we need this new
DT support.  The endpoint support has been around for a while, so this
should explain what's different about Xilinx Alveo and why it needs
this new stuff.

Bjorn