[PATCH v9 0/2] Adding support for Versal CPM as Root Port driver

[PATCH v9 0/2] Adding support for Versal CPM as Root Port driver

[Bharat Kumar Gogada]

- Adding support for Versal CPM as Root port.
- The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
  block for CPM along with the integrated bridge can function
  as PCIe Root Port.
- Versal CPM uses GICv3 ITS feature for assigning MSI/MSI-X
  vectors and handling MSI/MSI-X interrupts.
- Bridge error and legacy interrupts in Versal CPM are handled using
  Versal CPM specific interrupt line.

Changes for v9:
- Removed interrupt enablement outside irqchip flow as suggested
  by Marc.
- Removed using WARN_ON in if statement.

Bharat Kumar Gogada (2):
  PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
  PCI: xilinx-cpm: Add Versal CPM Root Port driver

 .../devicetree/bindings/pci/xilinx-versal-cpm.yaml |  99 ++++
 drivers/pci/controller/Kconfig                     |   8 +
 drivers/pci/controller/Makefile                    |   1 +
 drivers/pci/controller/pcie-xilinx-cpm.c           | 615 +++++++++++++++++++++
 4 files changed, 723 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
 create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c

-- 
2.7.4

[PATCH v9 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port

[Bharat Kumar Gogada]

Add YAML schemas documentation for Versal CPM Root Port driver.

Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
---
 .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 99 ++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml

diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
new file mode 100644
index 0000000..a2bbc0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/xilinx-versal-cpm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CPM Host Controller device tree for Xilinx Versal SoCs
+
+maintainers:
+  - Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    const: xlnx,versal-cpm-host-1.00
+
+  reg:
+    items:
+      - description: Configuration space region and bridge registers.
+      - description: CPM system level control and status registers.
+
+  reg-names:
+    items:
+      - const: cfg
+      - const: cpm_slcr
+
+  interrupts:
+    maxItems: 1
+
+  msi-map:
+    description:
+      Maps a Requester ID to an MSI controller and associated MSI sideband data.
+
+  ranges:
+    maxItems: 2
+
+  "#interrupt-cells":
+    const: 1
+
+  interrupt-controller:
+    description: Interrupt controller node for handling legacy PCI interrupts.
+    type: object
+    properties:
+      "#address-cells":
+        const: 0
+      "#interrupt-cells":
+        const: 1
+      "interrupt-controller": true
+    additionalProperties: false
+
+required:
+  - reg
+  - reg-names
+  - "#interrupt-cells"
+  - interrupts
+  - interrupt-parent
+  - interrupt-map
+  - interrupt-map-mask
+  - bus-range
+  - msi-map
+  - interrupt-controller
+
+unevaluatedProperties: false
+
+examples:
+  - |
+
+    versal {
+               #address-cells = <2>;
+               #size-cells = <2>;
+               cpm_pcie: pcie@fca10000 {
+                       compatible = "xlnx,versal-cpm-host-1.00";
+                       device_type = "pci";
+                       #address-cells = <3>;
+                       #interrupt-cells = <1>;
+                       #size-cells = <2>;
+                       interrupts = <0 72 4>;
+                       interrupt-parent = <&gic>;
+                       interrupt-map-mask = <0 0 0 7>;
+                       interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
+                                       <0 0 0 2 &pcie_intc_0 1>,
+                                       <0 0 0 3 &pcie_intc_0 2>,
+                                       <0 0 0 4 &pcie_intc_0 3>;
+                       bus-range = <0x00 0xff>;
+                       ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000>,
+                                <0x43000000 0x80 0x00000000 0x80 0x00000000 0x0 0x80000000>;
+                       msi-map = <0x0 &its_gic 0x0 0x10000>;
+                       reg = <0x6 0x00000000 0x0 0x10000000>,
+                             <0x0 0xfca10000 0x0 0x1000>;
+                       reg-names = "cfg", "cpm_slcr";
+                       pcie_intc_0: interrupt-controller {
+                               #address-cells = <0>;
+                               #interrupt-cells = <1>;
+                               interrupt-controller;
+                       };
+               };
+    };
-- 
2.7.4

Re: [PATCH v9 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port

[Rob Herring]

On Tue, Jun 16, 2020 at 6:57 AM Bharat Kumar Gogada
<bharat.kumar.gogada@xilinx.com> wrote:
>
> Add YAML schemas documentation for Versal CPM Root Port driver.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 99 ++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml

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

Though it was not sent to the DT list, so no automatic checking happened.

[PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bharat Kumar Gogada]

- Add support for Versal CPM as Root Port.
- The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
  block for CPM along with the integrated bridge can function
  as PCIe Root Port.
- Bridge error and legacy interrupts in Versal CPM are handled using
  Versal CPM specific interrupt line.

Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
---
 drivers/pci/controller/Kconfig           |   8 +
 drivers/pci/controller/Makefile          |   1 +
 drivers/pci/controller/pcie-xilinx-cpm.c | 617 +++++++++++++++++++++++++++++++
 3 files changed, 626 insertions(+)
 create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 20bf00f..d9e393a 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -81,6 +81,14 @@ config PCIE_XILINX
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
 
+config PCIE_XILINX_CPM
+	bool "Xilinx Versal CPM host bridge support"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	select PCI_HOST_COMMON
+	help
+	  Say 'Y' here if you want kernel support for the
+	  Xilinx Versal CPM host bridge.
+
 config PCI_XGENE
 	bool "X-Gene PCIe controller"
 	depends on ARM64 || COMPILE_TEST
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index 01b2502..78dabda 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
 obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
+obj-$(CONFIG_PCIE_XILINX_CPM) += pcie-xilinx-cpm.o
 obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o
 obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
 obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
new file mode 100644
index 0000000..2592dbb4
--- /dev/null
+++ b/drivers/pci/controller/pcie-xilinx-cpm.c
@@ -0,0 +1,617 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
+ *
+ * (C) Copyright 2019 - 2020, Xilinx, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/pci-ecam.h>
+
+#include "../pci.h"
+
+/* Register definitions */
+#define XILINX_CPM_PCIE_REG_IDR		0x00000E10
+#define XILINX_CPM_PCIE_REG_IMR		0x00000E14
+#define XILINX_CPM_PCIE_REG_PSCR	0x00000E1C
+#define XILINX_CPM_PCIE_REG_RPSC	0x00000E20
+#define XILINX_CPM_PCIE_REG_RPEFR	0x00000E2C
+#define XILINX_CPM_PCIE_REG_IDRN	0x00000E38
+#define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
+#define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
+#define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348
+#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
+
+/* Interrupt registers definitions */
+#define XILINX_CPM_PCIE_INTR_LINK_DOWN		0
+#define XILINX_CPM_PCIE_INTR_HOT_RESET		3
+#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT	4
+#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT	8
+#define XILINX_CPM_PCIE_INTR_CORRECTABLE	9
+#define XILINX_CPM_PCIE_INTR_NONFATAL		10
+#define XILINX_CPM_PCIE_INTR_FATAL		11
+#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON	12
+#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD	15
+#define XILINX_CPM_PCIE_INTR_INTX		16
+#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD	17
+#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP		20
+#define XILINX_CPM_PCIE_INTR_SLV_UNEXP		21
+#define XILINX_CPM_PCIE_INTR_SLV_COMPL		22
+#define XILINX_CPM_PCIE_INTR_SLV_ERRP		23
+#define XILINX_CPM_PCIE_INTR_SLV_CMPABT		24
+#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR		25
+#define XILINX_CPM_PCIE_INTR_MST_DECERR		26
+#define XILINX_CPM_PCIE_INTR_MST_SLVERR		27
+#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT	28
+
+#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x)
+
+#define XILINX_CPM_PCIE_IMR_ALL_MASK			\
+	(						\
+		IMR(LINK_DOWN)		|		\
+		IMR(HOT_RESET)		|		\
+		IMR(CFG_PCIE_TIMEOUT)	|		\
+		IMR(CFG_TIMEOUT)	|		\
+		IMR(CORRECTABLE)	|		\
+		IMR(NONFATAL)		|		\
+		IMR(FATAL)		|		\
+		IMR(CFG_ERR_POISON)	|		\
+		IMR(PME_TO_ACK_RCVD)	|		\
+		IMR(INTX)		|		\
+		IMR(PM_PME_RCVD)	|		\
+		IMR(SLV_UNSUPP)		|		\
+		IMR(SLV_UNEXP)		|		\
+		IMR(SLV_COMPL)		|		\
+		IMR(SLV_ERRP)		|		\
+		IMR(SLV_CMPABT)		|		\
+		IMR(SLV_ILLBUR)		|		\
+		IMR(MST_DECERR)		|		\
+		IMR(MST_SLVERR)		|		\
+		IMR(SLV_PCIE_TIMEOUT)			\
+	)
+
+#define XILINX_CPM_PCIE_IDR_ALL_MASK		0xFFFFFFFF
+#define XILINX_CPM_PCIE_IDRN_MASK		GENMASK(19, 16)
+#define XILINX_CPM_PCIE_IDRN_SHIFT		16
+
+/* Root Port Error FIFO Read Register definitions */
+#define XILINX_CPM_PCIE_RPEFR_ERR_VALID		BIT(18)
+#define XILINX_CPM_PCIE_RPEFR_REQ_ID		GENMASK(15, 0)
+#define XILINX_CPM_PCIE_RPEFR_ALL_MASK		0xFFFFFFFF
+
+/* Root Port Status/control Register definitions */
+#define XILINX_CPM_PCIE_REG_RPSC_BEN		BIT(0)
+
+/* Phy Status/Control Register definitions */
+#define XILINX_CPM_PCIE_REG_PSCR_LNKUP		BIT(11)
+
+/**
+ * struct xilinx_cpm_pcie_port - PCIe port information
+ * @reg_base: Bridge Register Base
+ * @cpm_base: CPM System Level Control and Status Register(SLCR) Base
+ * @dev: Device pointer
+ * @intx_domain: Legacy IRQ domain pointer
+ * @cfg: Holds mappings of config space window
+ * @intx_irq: legacy interrupt number
+ * @irq: Error interrupt number
+ * @lock: lock protecting shared register access
+ */
+struct xilinx_cpm_pcie_port {
+	void __iomem			*reg_base;
+	void __iomem			*cpm_base;
+	struct device			*dev;
+	struct irq_domain		*intx_domain;
+	struct irq_domain		*cpm_domain;
+	struct pci_config_window	*cfg;
+	int				intx_irq;
+	int				irq;
+	raw_spinlock_t			lock;
+};
+
+static u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg)
+{
+	return readl_relaxed(port->reg_base + reg);
+}
+
+static void pcie_write(struct xilinx_cpm_pcie_port *port,
+		       u32 val, u32 reg)
+{
+	writel_relaxed(val, port->reg_base + reg);
+}
+
+static bool cpm_pcie_link_up(struct xilinx_cpm_pcie_port *port)
+{
+	return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) &
+		XILINX_CPM_PCIE_REG_PSCR_LNKUP);
+}
+
+/**
+ * xilinx_cpm_pcie_clear_err_interrupts - Clear Error Interrupts
+ * @port: PCIe port information
+ */
+static void cpm_pcie_clear_err_interrupts(struct xilinx_cpm_pcie_port *port)
+{
+	unsigned long val = pcie_read(port, XILINX_CPM_PCIE_REG_RPEFR);
+
+	if (val & XILINX_CPM_PCIE_RPEFR_ERR_VALID) {
+		dev_dbg(port->dev, "Requester ID %lu\n",
+			val & XILINX_CPM_PCIE_RPEFR_REQ_ID);
+		pcie_write(port, XILINX_CPM_PCIE_RPEFR_ALL_MASK,
+			   XILINX_CPM_PCIE_REG_RPEFR);
+	}
+}
+
+static void xilinx_cpm_mask_leg_irq(struct irq_data *data)
+{
+	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data);
+	unsigned long flags;
+	u32 mask;
+	u32 val;
+
+	mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT);
+	raw_spin_lock_irqsave(&port->lock, flags);
+	val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK);
+	pcie_write(port, (val & (~mask)), XILINX_CPM_PCIE_REG_IDRN_MASK);
+	raw_spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
+{
+	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data);
+	unsigned long flags;
+	u32 mask;
+	u32 val;
+
+	mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT);
+	raw_spin_lock_irqsave(&port->lock, flags);
+	val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK);
+	pcie_write(port, (val | mask), XILINX_CPM_PCIE_REG_IDRN_MASK);
+	raw_spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static struct irq_chip xilinx_cpm_leg_irq_chip = {
+	.name		= "INTx",
+	.irq_mask	= xilinx_cpm_mask_leg_irq,
+	.irq_unmask	= xilinx_cpm_unmask_leg_irq,
+};
+
+/**
+ * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark IRQ as valid
+ * @domain: IRQ domain
+ * @irq: Virtual IRQ number
+ * @hwirq: HW interrupt number
+ *
+ * Return: Always returns 0.
+ */
+static int xilinx_cpm_pcie_intx_map(struct irq_domain *domain,
+				    unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &xilinx_cpm_leg_irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_status_flags(irq, IRQ_LEVEL);
+
+	return 0;
+}
+
+/* INTx IRQ Domain operations */
+static const struct irq_domain_ops intx_domain_ops = {
+	.map = xilinx_cpm_pcie_intx_map,
+};
+
+static void xilinx_cpm_pcie_intx_flow(struct irq_desc *desc)
+{
+	struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long val;
+	int i;
+
+	chained_irq_enter(chip, desc);
+
+	val = FIELD_GET(XILINX_CPM_PCIE_IDRN_MASK,
+			pcie_read(port, XILINX_CPM_PCIE_REG_IDRN));
+
+	for_each_set_bit(i, &val, PCI_NUM_INTX)
+		generic_handle_irq(irq_find_mapping(port->intx_domain, i));
+
+	chained_irq_exit(chip, desc);
+}
+
+static void xilinx_cpm_mask_event_irq(struct irq_data *d)
+{
+	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
+	u32 val;
+
+	raw_spin_lock(&port->lock);
+	val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+	val &= ~BIT(d->hwirq);
+	pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
+	raw_spin_unlock(&port->lock);
+}
+
+static void xilinx_cpm_unmask_event_irq(struct irq_data *d)
+{
+	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
+	u32 val;
+
+	raw_spin_lock(&port->lock);
+	val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+	val |= BIT(d->hwirq);
+	pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
+	raw_spin_unlock(&port->lock);
+}
+
+static struct irq_chip xilinx_cpm_event_irq_chip = {
+	.name		= "RC-Event",
+	.irq_mask	= xilinx_cpm_mask_event_irq,
+	.irq_unmask	= xilinx_cpm_unmask_event_irq,
+};
+
+static int xilinx_cpm_pcie_event_map(struct irq_domain *domain,
+				     unsigned int irq, irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &xilinx_cpm_event_irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	irq_set_status_flags(irq, IRQ_LEVEL);
+	return 0;
+}
+
+static const struct irq_domain_ops event_domain_ops = {
+	.map = xilinx_cpm_pcie_event_map,
+};
+
+static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc)
+{
+	struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long val;
+	int i;
+
+	chained_irq_enter(chip, desc);
+	val =  pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
+	val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+	for_each_set_bit(i, &val, 32)
+		generic_handle_irq(irq_find_mapping(port->cpm_domain, i));
+	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
+
+	/*
+	 * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
+	 * CPM SLCR block.
+	 */
+	val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+	if (val)
+		writel_relaxed(val,
+			       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+
+	chained_irq_exit(chip, desc);
+}
+
+#define _IC(x, s)                              \
+	[XILINX_CPM_PCIE_INTR_ ## x] = { __stringify(x), s }
+
+static const struct {
+	const char      *sym;
+	const char      *str;
+} intr_cause[32] = {
+	_IC(LINK_DOWN,		"Link Down"),
+	_IC(HOT_RESET,		"Hot reset"),
+	_IC(CFG_TIMEOUT,	"ECAM access timeout"),
+	_IC(CORRECTABLE,	"Correctable error message"),
+	_IC(NONFATAL,		"Non fatal error message"),
+	_IC(FATAL,		"Fatal error message"),
+	_IC(SLV_UNSUPP,		"Slave unsupported request"),
+	_IC(SLV_UNEXP,		"Slave unexpected completion"),
+	_IC(SLV_COMPL,		"Slave completion timeout"),
+	_IC(SLV_ERRP,		"Slave Error Poison"),
+	_IC(SLV_CMPABT,		"Slave Completer Abort"),
+	_IC(SLV_ILLBUR,		"Slave Illegal Burst"),
+	_IC(MST_DECERR,		"Master decode error"),
+	_IC(MST_SLVERR,		"Master slave error"),
+	_IC(CFG_PCIE_TIMEOUT,	"PCIe ECAM access timeout"),
+	_IC(CFG_ERR_POISON,	"ECAM poisoned completion received"),
+	_IC(PME_TO_ACK_RCVD,	"PME_TO_ACK message received"),
+	_IC(PM_PME_RCVD,	"PM_PME message received"),
+	_IC(SLV_PCIE_TIMEOUT,	"PCIe completion timeout received"),
+};
+
+static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *dev_id)
+{
+	struct xilinx_cpm_pcie_port *port = dev_id;
+	struct device *dev = port->dev;
+	struct irq_data *d;
+
+	d = irq_domain_get_irq_data(port->cpm_domain, irq);
+
+	switch (d->hwirq) {
+	case XILINX_CPM_PCIE_INTR_CORRECTABLE:
+	case XILINX_CPM_PCIE_INTR_NONFATAL:
+	case XILINX_CPM_PCIE_INTR_FATAL:
+		cpm_pcie_clear_err_interrupts(port);
+		fallthrough;
+
+	default:
+		if (intr_cause[d->hwirq].str)
+			dev_warn(dev, "%s\n", intr_cause[d->hwirq].str);
+		else
+			dev_warn(dev, "Unknown IRQ %ld\n", d->hwirq);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void xilinx_cpm_free_irq_domains(struct xilinx_cpm_pcie_port *port)
+{
+	if (port->intx_domain) {
+		irq_domain_remove(port->intx_domain);
+		port->intx_domain = NULL;
+	}
+
+	if (port->cpm_domain) {
+		irq_domain_remove(port->cpm_domain);
+		port->cpm_domain = NULL;
+	}
+}
+
+/**
+ * xilinx_cpm_pcie_init_irq_domain - Initialize IRQ domain
+ * @port: PCIe port information
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port *port)
+{
+	struct device *dev = port->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *pcie_intc_node;
+
+	/* Setup INTx */
+	pcie_intc_node = of_get_next_child(node, NULL);
+	if (!pcie_intc_node) {
+		dev_err(dev, "No PCIe Intc node found\n");
+		return -EINVAL;
+	}
+
+	port->cpm_domain = irq_domain_add_linear(pcie_intc_node, 32,
+						 &event_domain_ops,
+						 port);
+	if (!port->cpm_domain)
+		goto out;
+
+	irq_domain_update_bus_token(port->cpm_domain, DOMAIN_BUS_NEXUS);
+
+	port->intx_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
+						  &intx_domain_ops,
+						  port);
+	if (!port->intx_domain)
+		goto out;
+
+	irq_domain_update_bus_token(port->intx_domain, DOMAIN_BUS_WIRED);
+
+	of_node_put(pcie_intc_node);
+	raw_spin_lock_init(&port->lock);
+
+	return 0;
+out:
+	xilinx_cpm_free_irq_domains(port);
+	dev_err(dev, "Failed to allocate IRQ domains\n");
+
+	return -ENOMEM;
+}
+
+static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port)
+{
+	struct device *dev = port->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	int i, irq;
+
+	port->irq = platform_get_irq(pdev, 0);
+	if (port->irq < 0)
+		return port->irq;
+
+	for (i = 0; i < ARRAY_SIZE(intr_cause); i++) {
+		int err;
+
+		if (!intr_cause[i].str)
+			continue;
+
+		irq = irq_create_mapping(port->cpm_domain, i);
+		if (!irq) {
+			dev_err(dev, "Failed to map interrupt\n");
+			return -ENXIO;
+		}
+
+		err = devm_request_irq(dev, irq, xilinx_cpm_pcie_intr_handler,
+				       0, intr_cause[i].sym, port);
+		if (err) {
+			dev_err(dev, "Failed to request IRQ %d\n", irq);
+			return err;
+		}
+	}
+
+	port->intx_irq = irq_create_mapping(port->cpm_domain,
+					    XILINX_CPM_PCIE_INTR_INTX);
+	if (!port->intx_irq) {
+		dev_err(dev, "Failed to map INTx interrupt\n");
+		return -ENXIO;
+	}
+
+	/* Plug the INTx chained handler */
+	irq_set_chained_handler_and_data(port->intx_irq,
+					 xilinx_cpm_pcie_intx_flow, port);
+
+	/* Plug the main event chained handler */
+	irq_set_chained_handler_and_data(port->irq,
+					 xilinx_cpm_pcie_event_flow, port);
+
+	return 0;
+}
+
+/**
+ * xilinx_cpm_pcie_init_port - Initialize hardware
+ * @port: PCIe port information
+ */
+static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
+{
+	if (cpm_pcie_link_up(port))
+		dev_info(port->dev, "PCIe Link is UP\n");
+	else
+		dev_info(port->dev, "PCIe Link is DOWN\n");
+
+	/* Disable all interrupts */
+	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
+		   XILINX_CPM_PCIE_REG_IMR);
+
+	/* Clear pending interrupts */
+	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
+		   XILINX_CPM_PCIE_IMR_ALL_MASK,
+		   XILINX_CPM_PCIE_REG_IDR);
+
+	/*
+	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
+	 * CPM SLCR block.
+	 */
+	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
+	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
+	/* Enable the Bridge enable bit */
+	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
+		   XILINX_CPM_PCIE_REG_RPSC_BEN,
+		   XILINX_CPM_PCIE_REG_RPSC);
+}
+
+/**
+ * xilinx_cpm_pcie_parse_dt - Parse Device tree
+ * @port: PCIe port information
+ * @bus_range: Bus resource
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
+				    struct resource *bus_range)
+{
+	struct device *dev = port->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct resource *res;
+
+	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
+							       "cpm_slcr");
+	if (IS_ERR(port->cpm_base))
+		return PTR_ERR(port->cpm_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+	if (!res)
+		return -ENXIO;
+
+	port->cfg = pci_ecam_create(dev, res, bus_range,
+				    &pci_generic_ecam_ops);
+	if (IS_ERR(port->cfg))
+		return PTR_ERR(port->cfg);
+
+	port->reg_base = port->cfg->win;
+
+	return 0;
+}
+
+void xilinx_cpm_free_interrupts(struct xilinx_cpm_pcie_port *port)
+{
+	irq_set_chained_handler_and_data(port->intx_irq, NULL, NULL);
+	irq_set_chained_handler_and_data(port->irq, NULL, NULL);
+}
+
+/**
+ * xilinx_cpm_pcie_probe - Probe function
+ * @pdev: Platform device pointer
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
+{
+	struct xilinx_cpm_pcie_port *port;
+	struct device *dev = &pdev->dev;
+	struct pci_host_bridge *bridge;
+	struct resource *bus_range;
+	int err;
+
+	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
+	if (!bridge)
+		return -ENODEV;
+
+	port = pci_host_bridge_priv(bridge);
+
+	port->dev = dev;
+
+	err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
+					      &bridge->dma_ranges, &bus_range);
+	if (err) {
+		dev_err(dev, "Getting bridge resources failed\n");
+		return err;
+	}
+
+	err = xilinx_cpm_pcie_init_irq_domain(port);
+	if (err)
+		return err;
+
+	err = xilinx_cpm_pcie_parse_dt(port, bus_range);
+	if (err) {
+		dev_err(dev, "Parsing DT failed\n");
+		goto err_parse_dt;
+	}
+
+	xilinx_cpm_pcie_init_port(port);
+
+	err = xilinx_cpm_setup_irq(port);
+	if (err) {
+		dev_err(dev, "Failed to set up interrupts\n");
+		goto err_setup_irq;
+	}
+
+	bridge->dev.parent = dev;
+	bridge->sysdata = port->cfg;
+	bridge->busnr = port->cfg->busr.start;
+	bridge->ops = &pci_generic_ecam_ops.pci_ops;
+	bridge->map_irq = of_irq_parse_and_map_pci;
+	bridge->swizzle_irq = pci_common_swizzle;
+
+	err = pci_host_probe(bridge);
+	if (err < 0)
+		goto err_host_bridge;
+
+	return 0;
+
+err_host_bridge:
+	xilinx_cpm_free_interrupts(port);
+err_setup_irq:
+	pci_ecam_free(port->cfg);
+err_parse_dt:
+	xilinx_cpm_free_irq_domains(port);
+	return err;
+}
+
+static const struct of_device_id xilinx_cpm_pcie_of_match[] = {
+	{ .compatible = "xlnx,versal-cpm-host-1.00", },
+	{}
+};
+
+static struct platform_driver xilinx_cpm_pcie_driver = {
+	.driver = {
+		.name = "xilinx-cpm-pcie",
+		.of_match_table = xilinx_cpm_pcie_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = xilinx_cpm_pcie_probe,
+};
+
+builtin_platform_driver(xilinx_cpm_pcie_driver);
-- 
2.7.4

RE: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bharat Kumar Gogada]

Hi All,

Gentle ping.

Regards,
Bharat

> -----Original Message-----
> From: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> Sent: Tuesday, June 16, 2020 6:27 PM
> To: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: bhelgaas@google.com; lorenzo.pieralisi@arm.com; robh@kernel.org;
> maz@kernel.org; Bharat Kumar Gogada <bharatku@xilinx.com>
> Subject: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
> 
> - Add support for Versal CPM as Root Port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
>   block for CPM along with the integrated bridge can function
>   as PCIe Root Port.
> - Bridge error and legacy interrupts in Versal CPM are handled using
>   Versal CPM specific interrupt line.
> 
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  drivers/pci/controller/Kconfig           |   8 +
>  drivers/pci/controller/Makefile          |   1 +
>  drivers/pci/controller/pcie-xilinx-cpm.c | 617
> +++++++++++++++++++++++++++++++
>  3 files changed, 626 insertions(+)
>  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 20bf00f..d9e393a 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -81,6 +81,14 @@ config PCIE_XILINX
>  	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
>  	  Host Bridge driver.
> 
> +config PCIE_XILINX_CPM
> +	bool "Xilinx Versal CPM host bridge support"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	select PCI_HOST_COMMON
> +	help
> +	  Say 'Y' here if you want kernel support for the
> +	  Xilinx Versal CPM host bridge.
> +
>  config PCI_XGENE
>  	bool "X-Gene PCIe controller"
>  	depends on ARM64 || COMPILE_TEST
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index 01b2502..78dabda 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-
> common.o
>  obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
>  obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>  obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
> +obj-$(CONFIG_PCIE_XILINX_CPM) += pcie-xilinx-cpm.o
>  obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o
>  obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>  obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o diff --git
> a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-
> cpm.c
> new file mode 100644
> index 0000000..2592dbb4
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -0,0 +1,617 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
> + *
> + * (C) Copyright 2019 - 2020, Xilinx, Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/pci-ecam.h>
> +
> +#include "../pci.h"
> +
> +/* Register definitions */
> +#define XILINX_CPM_PCIE_REG_IDR		0x00000E10
> +#define XILINX_CPM_PCIE_REG_IMR		0x00000E14
> +#define XILINX_CPM_PCIE_REG_PSCR	0x00000E1C
> +#define XILINX_CPM_PCIE_REG_RPSC	0x00000E20
> +#define XILINX_CPM_PCIE_REG_RPEFR	0x00000E2C
> +#define XILINX_CPM_PCIE_REG_IDRN	0x00000E38
> +#define XILINX_CPM_PCIE_REG_IDRN_MASK	0x00000E3C
> +#define XILINX_CPM_PCIE_MISC_IR_STATUS	0x00000340
> +#define XILINX_CPM_PCIE_MISC_IR_ENABLE	0x00000348
> +#define XILINX_CPM_PCIE_MISC_IR_LOCAL	BIT(1)
> +
> +/* Interrupt registers definitions */
> +#define XILINX_CPM_PCIE_INTR_LINK_DOWN		0
> +#define XILINX_CPM_PCIE_INTR_HOT_RESET		3
> +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT	4
> +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT	8
> +#define XILINX_CPM_PCIE_INTR_CORRECTABLE	9
> +#define XILINX_CPM_PCIE_INTR_NONFATAL		10
> +#define XILINX_CPM_PCIE_INTR_FATAL		11
> +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON	12
> +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD	15
> +#define XILINX_CPM_PCIE_INTR_INTX		16
> +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD	17
> +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP		20
> +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP		21
> +#define XILINX_CPM_PCIE_INTR_SLV_COMPL		22
> +#define XILINX_CPM_PCIE_INTR_SLV_ERRP		23
> +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT		24
> +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR		25
> +#define XILINX_CPM_PCIE_INTR_MST_DECERR		26
> +#define XILINX_CPM_PCIE_INTR_MST_SLVERR		27
> +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT	28
> +
> +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x)
> +
> +#define XILINX_CPM_PCIE_IMR_ALL_MASK			\
> +	(						\
> +		IMR(LINK_DOWN)		|		\
> +		IMR(HOT_RESET)		|		\
> +		IMR(CFG_PCIE_TIMEOUT)	|		\
> +		IMR(CFG_TIMEOUT)	|		\
> +		IMR(CORRECTABLE)	|		\
> +		IMR(NONFATAL)		|		\
> +		IMR(FATAL)		|		\
> +		IMR(CFG_ERR_POISON)	|		\
> +		IMR(PME_TO_ACK_RCVD)	|		\
> +		IMR(INTX)		|		\
> +		IMR(PM_PME_RCVD)	|		\
> +		IMR(SLV_UNSUPP)		|		\
> +		IMR(SLV_UNEXP)		|		\
> +		IMR(SLV_COMPL)		|		\
> +		IMR(SLV_ERRP)		|		\
> +		IMR(SLV_CMPABT)		|		\
> +		IMR(SLV_ILLBUR)		|		\
> +		IMR(MST_DECERR)		|		\
> +		IMR(MST_SLVERR)		|		\
> +		IMR(SLV_PCIE_TIMEOUT)			\
> +	)
> +
> +#define XILINX_CPM_PCIE_IDR_ALL_MASK		0xFFFFFFFF
> +#define XILINX_CPM_PCIE_IDRN_MASK		GENMASK(19, 16)
> +#define XILINX_CPM_PCIE_IDRN_SHIFT		16
> +
> +/* Root Port Error FIFO Read Register definitions */
> +#define XILINX_CPM_PCIE_RPEFR_ERR_VALID		BIT(18)
> +#define XILINX_CPM_PCIE_RPEFR_REQ_ID		GENMASK(15, 0)
> +#define XILINX_CPM_PCIE_RPEFR_ALL_MASK		0xFFFFFFFF
> +
> +/* Root Port Status/control Register definitions */
> +#define XILINX_CPM_PCIE_REG_RPSC_BEN		BIT(0)
> +
> +/* Phy Status/Control Register definitions */
> +#define XILINX_CPM_PCIE_REG_PSCR_LNKUP		BIT(11)
> +
> +/**
> + * struct xilinx_cpm_pcie_port - PCIe port information
> + * @reg_base: Bridge Register Base
> + * @cpm_base: CPM System Level Control and Status Register(SLCR) Base
> + * @dev: Device pointer
> + * @intx_domain: Legacy IRQ domain pointer
> + * @cfg: Holds mappings of config space window
> + * @intx_irq: legacy interrupt number
> + * @irq: Error interrupt number
> + * @lock: lock protecting shared register access  */ struct
> +xilinx_cpm_pcie_port {
> +	void __iomem			*reg_base;
> +	void __iomem			*cpm_base;
> +	struct device			*dev;
> +	struct irq_domain		*intx_domain;
> +	struct irq_domain		*cpm_domain;
> +	struct pci_config_window	*cfg;
> +	int				intx_irq;
> +	int				irq;
> +	raw_spinlock_t			lock;
> +};
> +
> +static u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg) {
> +	return readl_relaxed(port->reg_base + reg); }
> +
> +static void pcie_write(struct xilinx_cpm_pcie_port *port,
> +		       u32 val, u32 reg)
> +{
> +	writel_relaxed(val, port->reg_base + reg); }
> +
> +static bool cpm_pcie_link_up(struct xilinx_cpm_pcie_port *port) {
> +	return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) &
> +		XILINX_CPM_PCIE_REG_PSCR_LNKUP);
> +}
> +
> +/**
> + * xilinx_cpm_pcie_clear_err_interrupts - Clear Error Interrupts
> + * @port: PCIe port information
> + */
> +static void cpm_pcie_clear_err_interrupts(struct xilinx_cpm_pcie_port
> +*port) {
> +	unsigned long val = pcie_read(port, XILINX_CPM_PCIE_REG_RPEFR);
> +
> +	if (val & XILINX_CPM_PCIE_RPEFR_ERR_VALID) {
> +		dev_dbg(port->dev, "Requester ID %lu\n",
> +			val & XILINX_CPM_PCIE_RPEFR_REQ_ID);
> +		pcie_write(port, XILINX_CPM_PCIE_RPEFR_ALL_MASK,
> +			   XILINX_CPM_PCIE_REG_RPEFR);
> +	}
> +}
> +
> +static void xilinx_cpm_mask_leg_irq(struct irq_data *data) {
> +	struct xilinx_cpm_pcie_port *port =
> irq_data_get_irq_chip_data(data);
> +	unsigned long flags;
> +	u32 mask;
> +	u32 val;
> +
> +	mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT);
> +	raw_spin_lock_irqsave(&port->lock, flags);
> +	val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK);
> +	pcie_write(port, (val & (~mask)),
> XILINX_CPM_PCIE_REG_IDRN_MASK);
> +	raw_spin_unlock_irqrestore(&port->lock, flags); }
> +
> +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data) {
> +	struct xilinx_cpm_pcie_port *port =
> irq_data_get_irq_chip_data(data);
> +	unsigned long flags;
> +	u32 mask;
> +	u32 val;
> +
> +	mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT);
> +	raw_spin_lock_irqsave(&port->lock, flags);
> +	val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK);
> +	pcie_write(port, (val | mask), XILINX_CPM_PCIE_REG_IDRN_MASK);
> +	raw_spin_unlock_irqrestore(&port->lock, flags); }
> +
> +static struct irq_chip xilinx_cpm_leg_irq_chip = {
> +	.name		= "INTx",
> +	.irq_mask	= xilinx_cpm_mask_leg_irq,
> +	.irq_unmask	= xilinx_cpm_unmask_leg_irq,
> +};
> +
> +/**
> + * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark IRQ
> +as valid
> + * @domain: IRQ domain
> + * @irq: Virtual IRQ number
> + * @hwirq: HW interrupt number
> + *
> + * Return: Always returns 0.
> + */
> +static int xilinx_cpm_pcie_intx_map(struct irq_domain *domain,
> +				    unsigned int irq, irq_hw_number_t hwirq)
> {
> +	irq_set_chip_and_handler(irq, &xilinx_cpm_leg_irq_chip,
> +				 handle_level_irq);
> +	irq_set_chip_data(irq, domain->host_data);
> +	irq_set_status_flags(irq, IRQ_LEVEL);
> +
> +	return 0;
> +}
> +
> +/* INTx IRQ Domain operations */
> +static const struct irq_domain_ops intx_domain_ops = {
> +	.map = xilinx_cpm_pcie_intx_map,
> +};
> +
> +static void xilinx_cpm_pcie_intx_flow(struct irq_desc *desc) {
> +	struct xilinx_cpm_pcie_port *port =
> irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned long val;
> +	int i;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	val = FIELD_GET(XILINX_CPM_PCIE_IDRN_MASK,
> +			pcie_read(port, XILINX_CPM_PCIE_REG_IDRN));
> +
> +	for_each_set_bit(i, &val, PCI_NUM_INTX)
> +		generic_handle_irq(irq_find_mapping(port->intx_domain, i));
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void xilinx_cpm_mask_event_irq(struct irq_data *d) {
> +	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
> +	u32 val;
> +
> +	raw_spin_lock(&port->lock);
> +	val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> +	val &= ~BIT(d->hwirq);
> +	pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
> +	raw_spin_unlock(&port->lock);
> +}
> +
> +static void xilinx_cpm_unmask_event_irq(struct irq_data *d) {
> +	struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
> +	u32 val;
> +
> +	raw_spin_lock(&port->lock);
> +	val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> +	val |= BIT(d->hwirq);
> +	pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
> +	raw_spin_unlock(&port->lock);
> +}
> +
> +static struct irq_chip xilinx_cpm_event_irq_chip = {
> +	.name		= "RC-Event",
> +	.irq_mask	= xilinx_cpm_mask_event_irq,
> +	.irq_unmask	= xilinx_cpm_unmask_event_irq,
> +};
> +
> +static int xilinx_cpm_pcie_event_map(struct irq_domain *domain,
> +				     unsigned int irq, irq_hw_number_t hwirq)
> {
> +	irq_set_chip_and_handler(irq, &xilinx_cpm_event_irq_chip,
> +				 handle_level_irq);
> +	irq_set_chip_data(irq, domain->host_data);
> +	irq_set_status_flags(irq, IRQ_LEVEL);
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops event_domain_ops = {
> +	.map = xilinx_cpm_pcie_event_map,
> +};
> +
> +static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) {
> +	struct xilinx_cpm_pcie_port *port =
> irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned long val;
> +	int i;
> +
> +	chained_irq_enter(chip, desc);
> +	val =  pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> +	val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> +	for_each_set_bit(i, &val, 32)
> +		generic_handle_irq(irq_find_mapping(port->cpm_domain,
> i));
> +	pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
> +
> +	/*
> +	 * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
> +	 * CPM SLCR block.
> +	 */
> +	val = readl_relaxed(port->cpm_base +
> XILINX_CPM_PCIE_MISC_IR_STATUS);
> +	if (val)
> +		writel_relaxed(val,
> +			       port->cpm_base +
> XILINX_CPM_PCIE_MISC_IR_STATUS);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +#define _IC(x, s)                              \
> +	[XILINX_CPM_PCIE_INTR_ ## x] = { __stringify(x), s }
> +
> +static const struct {
> +	const char      *sym;
> +	const char      *str;
> +} intr_cause[32] = {
> +	_IC(LINK_DOWN,		"Link Down"),
> +	_IC(HOT_RESET,		"Hot reset"),
> +	_IC(CFG_TIMEOUT,	"ECAM access timeout"),
> +	_IC(CORRECTABLE,	"Correctable error message"),
> +	_IC(NONFATAL,		"Non fatal error message"),
> +	_IC(FATAL,		"Fatal error message"),
> +	_IC(SLV_UNSUPP,		"Slave unsupported request"),
> +	_IC(SLV_UNEXP,		"Slave unexpected completion"),
> +	_IC(SLV_COMPL,		"Slave completion timeout"),
> +	_IC(SLV_ERRP,		"Slave Error Poison"),
> +	_IC(SLV_CMPABT,		"Slave Completer Abort"),
> +	_IC(SLV_ILLBUR,		"Slave Illegal Burst"),
> +	_IC(MST_DECERR,		"Master decode error"),
> +	_IC(MST_SLVERR,		"Master slave error"),
> +	_IC(CFG_PCIE_TIMEOUT,	"PCIe ECAM access timeout"),
> +	_IC(CFG_ERR_POISON,	"ECAM poisoned completion received"),
> +	_IC(PME_TO_ACK_RCVD,	"PME_TO_ACK message received"),
> +	_IC(PM_PME_RCVD,	"PM_PME message received"),
> +	_IC(SLV_PCIE_TIMEOUT,	"PCIe completion timeout received"),
> +};
> +
> +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *dev_id)
> +{
> +	struct xilinx_cpm_pcie_port *port = dev_id;
> +	struct device *dev = port->dev;
> +	struct irq_data *d;
> +
> +	d = irq_domain_get_irq_data(port->cpm_domain, irq);
> +
> +	switch (d->hwirq) {
> +	case XILINX_CPM_PCIE_INTR_CORRECTABLE:
> +	case XILINX_CPM_PCIE_INTR_NONFATAL:
> +	case XILINX_CPM_PCIE_INTR_FATAL:
> +		cpm_pcie_clear_err_interrupts(port);
> +		fallthrough;
> +
> +	default:
> +		if (intr_cause[d->hwirq].str)
> +			dev_warn(dev, "%s\n", intr_cause[d->hwirq].str);
> +		else
> +			dev_warn(dev, "Unknown IRQ %ld\n", d->hwirq);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void xilinx_cpm_free_irq_domains(struct xilinx_cpm_pcie_port
> +*port) {
> +	if (port->intx_domain) {
> +		irq_domain_remove(port->intx_domain);
> +		port->intx_domain = NULL;
> +	}
> +
> +	if (port->cpm_domain) {
> +		irq_domain_remove(port->cpm_domain);
> +		port->cpm_domain = NULL;
> +	}
> +}
> +
> +/**
> + * xilinx_cpm_pcie_init_irq_domain - Initialize IRQ domain
> + * @port: PCIe port information
> + *
> + * Return: '0' on success and error value on failure  */ static int
> +xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port *port) {
> +	struct device *dev = port->dev;
> +	struct device_node *node = dev->of_node;
> +	struct device_node *pcie_intc_node;
> +
> +	/* Setup INTx */
> +	pcie_intc_node = of_get_next_child(node, NULL);
> +	if (!pcie_intc_node) {
> +		dev_err(dev, "No PCIe Intc node found\n");
> +		return -EINVAL;
> +	}
> +
> +	port->cpm_domain = irq_domain_add_linear(pcie_intc_node, 32,
> +						 &event_domain_ops,
> +						 port);
> +	if (!port->cpm_domain)
> +		goto out;
> +
> +	irq_domain_update_bus_token(port->cpm_domain,
> DOMAIN_BUS_NEXUS);
> +
> +	port->intx_domain = irq_domain_add_linear(pcie_intc_node,
> PCI_NUM_INTX,
> +						  &intx_domain_ops,
> +						  port);
> +	if (!port->intx_domain)
> +		goto out;
> +
> +	irq_domain_update_bus_token(port->intx_domain,
> DOMAIN_BUS_WIRED);
> +
> +	of_node_put(pcie_intc_node);
> +	raw_spin_lock_init(&port->lock);
> +
> +	return 0;
> +out:
> +	xilinx_cpm_free_irq_domains(port);
> +	dev_err(dev, "Failed to allocate IRQ domains\n");
> +
> +	return -ENOMEM;
> +}
> +
> +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) {
> +	struct device *dev = port->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	int i, irq;
> +
> +	port->irq = platform_get_irq(pdev, 0);
> +	if (port->irq < 0)
> +		return port->irq;
> +
> +	for (i = 0; i < ARRAY_SIZE(intr_cause); i++) {
> +		int err;
> +
> +		if (!intr_cause[i].str)
> +			continue;
> +
> +		irq = irq_create_mapping(port->cpm_domain, i);
> +		if (!irq) {
> +			dev_err(dev, "Failed to map interrupt\n");
> +			return -ENXIO;
> +		}
> +
> +		err = devm_request_irq(dev, irq,
> xilinx_cpm_pcie_intr_handler,
> +				       0, intr_cause[i].sym, port);
> +		if (err) {
> +			dev_err(dev, "Failed to request IRQ %d\n", irq);
> +			return err;
> +		}
> +	}
> +
> +	port->intx_irq = irq_create_mapping(port->cpm_domain,
> +					    XILINX_CPM_PCIE_INTR_INTX);
> +	if (!port->intx_irq) {
> +		dev_err(dev, "Failed to map INTx interrupt\n");
> +		return -ENXIO;
> +	}
> +
> +	/* Plug the INTx chained handler */
> +	irq_set_chained_handler_and_data(port->intx_irq,
> +					 xilinx_cpm_pcie_intx_flow, port);
> +
> +	/* Plug the main event chained handler */
> +	irq_set_chained_handler_and_data(port->irq,
> +					 xilinx_cpm_pcie_event_flow, port);
> +
> +	return 0;
> +}
> +
> +/**
> + * xilinx_cpm_pcie_init_port - Initialize hardware
> + * @port: PCIe port information
> + */
> +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port
> +*port) {
> +	if (cpm_pcie_link_up(port))
> +		dev_info(port->dev, "PCIe Link is UP\n");
> +	else
> +		dev_info(port->dev, "PCIe Link is DOWN\n");
> +
> +	/* Disable all interrupts */
> +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> +		   XILINX_CPM_PCIE_REG_IMR);
> +
> +	/* Clear pending interrupts */
> +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> +		   XILINX_CPM_PCIE_REG_IDR);
> +
> +	/*
> +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> +	 * CPM SLCR block.
> +	 */
> +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> +	/* Enable the Bridge enable bit */
> +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> +		   XILINX_CPM_PCIE_REG_RPSC);
> +}
> +
> +/**
> + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> + * @port: PCIe port information
> + * @bus_range: Bus resource
> + *
> + * Return: '0' on success and error value on failure  */ static int
> +xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> +				    struct resource *bus_range)
> +{
> +	struct device *dev = port->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct resource *res;
> +
> +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> +							       "cpm_slcr");
> +	if (IS_ERR(port->cpm_base))
> +		return PTR_ERR(port->cpm_base);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "cfg");
> +	if (!res)
> +		return -ENXIO;
> +
> +	port->cfg = pci_ecam_create(dev, res, bus_range,
> +				    &pci_generic_ecam_ops);
> +	if (IS_ERR(port->cfg))
> +		return PTR_ERR(port->cfg);
> +
> +	port->reg_base = port->cfg->win;
> +
> +	return 0;
> +}
> +
> +void xilinx_cpm_free_interrupts(struct xilinx_cpm_pcie_port *port) {
> +	irq_set_chained_handler_and_data(port->intx_irq, NULL, NULL);
> +	irq_set_chained_handler_and_data(port->irq, NULL, NULL); }
> +
> +/**
> + * xilinx_cpm_pcie_probe - Probe function
> + * @pdev: Platform device pointer
> + *
> + * Return: '0' on success and error value on failure  */ static int
> +xilinx_cpm_pcie_probe(struct platform_device *pdev) {
> +	struct xilinx_cpm_pcie_port *port;
> +	struct device *dev = &pdev->dev;
> +	struct pci_host_bridge *bridge;
> +	struct resource *bus_range;
> +	int err;
> +
> +	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> +	if (!bridge)
> +		return -ENODEV;
> +
> +	port = pci_host_bridge_priv(bridge);
> +
> +	port->dev = dev;
> +
> +	err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> +					      &bridge->dma_ranges,
> &bus_range);
> +	if (err) {
> +		dev_err(dev, "Getting bridge resources failed\n");
> +		return err;
> +	}
> +
> +	err = xilinx_cpm_pcie_init_irq_domain(port);
> +	if (err)
> +		return err;
> +
> +	err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> +	if (err) {
> +		dev_err(dev, "Parsing DT failed\n");
> +		goto err_parse_dt;
> +	}
> +
> +	xilinx_cpm_pcie_init_port(port);
> +
> +	err = xilinx_cpm_setup_irq(port);
> +	if (err) {
> +		dev_err(dev, "Failed to set up interrupts\n");
> +		goto err_setup_irq;
> +	}
> +
> +	bridge->dev.parent = dev;
> +	bridge->sysdata = port->cfg;
> +	bridge->busnr = port->cfg->busr.start;
> +	bridge->ops = &pci_generic_ecam_ops.pci_ops;
> +	bridge->map_irq = of_irq_parse_and_map_pci;
> +	bridge->swizzle_irq = pci_common_swizzle;
> +
> +	err = pci_host_probe(bridge);
> +	if (err < 0)
> +		goto err_host_bridge;
> +
> +	return 0;
> +
> +err_host_bridge:
> +	xilinx_cpm_free_interrupts(port);
> +err_setup_irq:
> +	pci_ecam_free(port->cfg);
> +err_parse_dt:
> +	xilinx_cpm_free_irq_domains(port);
> +	return err;
> +}
> +
> +static const struct of_device_id xilinx_cpm_pcie_of_match[] = {
> +	{ .compatible = "xlnx,versal-cpm-host-1.00", },
> +	{}
> +};
> +
> +static struct platform_driver xilinx_cpm_pcie_driver = {
> +	.driver = {
> +		.name = "xilinx-cpm-pcie",
> +		.of_match_table = xilinx_cpm_pcie_of_match,
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe = xilinx_cpm_pcie_probe,
> +};
> +
> +builtin_platform_driver(xilinx_cpm_pcie_driver);
> --
> 2.7.4

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Rob Herring]

On Tue, Jun 16, 2020 at 6:57 AM Bharat Kumar Gogada
<bharat.kumar.gogada@xilinx.com> wrote:
>
> - Add support for Versal CPM as Root Port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
>   block for CPM along with the integrated bridge can function
>   as PCIe Root Port.
> - Bridge error and legacy interrupts in Versal CPM are handled using
>   Versal CPM specific interrupt line.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  drivers/pci/controller/Kconfig           |   8 +
>  drivers/pci/controller/Makefile          |   1 +
>  drivers/pci/controller/pcie-xilinx-cpm.c | 617 +++++++++++++++++++++++++++++++
>  3 files changed, 626 insertions(+)
>  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c

[...]

> +static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
> +{
> +       struct xilinx_cpm_pcie_port *port;
> +       struct device *dev = &pdev->dev;
> +       struct pci_host_bridge *bridge;
> +       struct resource *bus_range;
> +       int err;
> +
> +       bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> +       if (!bridge)
> +               return -ENODEV;
> +
> +       port = pci_host_bridge_priv(bridge);
> +
> +       port->dev = dev;
> +
> +       err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> +                                             &bridge->dma_ranges, &bus_range);
> +       if (err) {
> +               dev_err(dev, "Getting bridge resources failed\n");
> +               return err;
> +       }
> +
> +       err = xilinx_cpm_pcie_init_irq_domain(port);
> +       if (err)
> +               return err;
> +
> +       err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> +       if (err) {
> +               dev_err(dev, "Parsing DT failed\n");
> +               goto err_parse_dt;
> +       }
> +
> +       xilinx_cpm_pcie_init_port(port);
> +
> +       err = xilinx_cpm_setup_irq(port);
> +       if (err) {
> +               dev_err(dev, "Failed to set up interrupts\n");
> +               goto err_setup_irq;
> +       }

All the h/w init here can be moved to an .init() function in ecam ops
and then use pci_host_common_probe. Given this is v9, that can be a
follow-up I guess.

Otherwise,

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

> +
> +       bridge->dev.parent = dev;
> +       bridge->sysdata = port->cfg;
> +       bridge->busnr = port->cfg->busr.start;
> +       bridge->ops = &pci_generic_ecam_ops.pci_ops;
> +       bridge->map_irq = of_irq_parse_and_map_pci;
> +       bridge->swizzle_irq = pci_common_swizzle;
> +
> +       err = pci_host_probe(bridge);
> +       if (err < 0)
> +               goto err_host_bridge;
> +
> +       return 0;
> +
> +err_host_bridge:
> +       xilinx_cpm_free_interrupts(port);
> +err_setup_irq:
> +       pci_ecam_free(port->cfg);
> +err_parse_dt:
> +       xilinx_cpm_free_irq_domains(port);
> +       return err;
> +}

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Lorenzo Pieralisi]

On Fri, Jul 10, 2020 at 09:16:57AM -0600, Rob Herring wrote:
> On Tue, Jun 16, 2020 at 6:57 AM Bharat Kumar Gogada
> <bharat.kumar.gogada@xilinx.com> wrote:
> >
> > - Add support for Versal CPM as Root Port.
> > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> >   block for CPM along with the integrated bridge can function
> >   as PCIe Root Port.
> > - Bridge error and legacy interrupts in Versal CPM are handled using
> >   Versal CPM specific interrupt line.
> >
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > ---
> >  drivers/pci/controller/Kconfig           |   8 +
> >  drivers/pci/controller/Makefile          |   1 +
> >  drivers/pci/controller/pcie-xilinx-cpm.c | 617 +++++++++++++++++++++++++++++++
> >  3 files changed, 626 insertions(+)
> >  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c
> 
> [...]
> 
> > +static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
> > +{
> > +       struct xilinx_cpm_pcie_port *port;
> > +       struct device *dev = &pdev->dev;
> > +       struct pci_host_bridge *bridge;
> > +       struct resource *bus_range;
> > +       int err;
> > +
> > +       bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> > +       if (!bridge)
> > +               return -ENODEV;
> > +
> > +       port = pci_host_bridge_priv(bridge);
> > +
> > +       port->dev = dev;
> > +
> > +       err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> > +                                             &bridge->dma_ranges, &bus_range);
> > +       if (err) {
> > +               dev_err(dev, "Getting bridge resources failed\n");
> > +               return err;
> > +       }
> > +
> > +       err = xilinx_cpm_pcie_init_irq_domain(port);
> > +       if (err)
> > +               return err;
> > +
> > +       err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> > +       if (err) {
> > +               dev_err(dev, "Parsing DT failed\n");
> > +               goto err_parse_dt;
> > +       }
> > +
> > +       xilinx_cpm_pcie_init_port(port);
> > +
> > +       err = xilinx_cpm_setup_irq(port);
> > +       if (err) {
> > +               dev_err(dev, "Failed to set up interrupts\n");
> > +               goto err_setup_irq;
> > +       }
> 
> All the h/w init here can be moved to an .init() function in ecam ops
> and then use pci_host_common_probe. Given this is v9, that can be a
> follow-up I guess.

I think there is time to get it done, Bharat please let me know if
you can repost it shortly with Rob's requested change implemented.

Thanks,
Lorenzo

> Otherwise,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> > +
> > +       bridge->dev.parent = dev;
> > +       bridge->sysdata = port->cfg;
> > +       bridge->busnr = port->cfg->busr.start;
> > +       bridge->ops = &pci_generic_ecam_ops.pci_ops;
> > +       bridge->map_irq = of_irq_parse_and_map_pci;
> > +       bridge->swizzle_irq = pci_common_swizzle;
> > +
> > +       err = pci_host_probe(bridge);
> > +       if (err < 0)
> > +               goto err_host_bridge;
> > +
> > +       return 0;
> > +
> > +err_host_bridge:
> > +       xilinx_cpm_free_interrupts(port);
> > +err_setup_irq:
> > +       pci_ecam_free(port->cfg);
> > +err_parse_dt:
> > +       xilinx_cpm_free_irq_domains(port);
> > +       return err;
> > +}

RE: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bharat Kumar Gogada]

> Subject: Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
> 
> On Fri, Jul 10, 2020 at 09:16:57AM -0600, Rob Herring wrote:
> > On Tue, Jun 16, 2020 at 6:57 AM Bharat Kumar Gogada
> > <bharat.kumar.gogada@xilinx.com> wrote:
> > >
> > > - Add support for Versal CPM as Root Port.
> > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The
> integrated
> > >   block for CPM along with the integrated bridge can function
> > >   as PCIe Root Port.
> > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > >   Versal CPM specific interrupt line.
> > >
> > > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > > ---
> > >  drivers/pci/controller/Kconfig           |   8 +
> > >  drivers/pci/controller/Makefile          |   1 +
> > >  drivers/pci/controller/pcie-xilinx-cpm.c | 617
> > > +++++++++++++++++++++++++++++++
> > >  3 files changed, 626 insertions(+)
> > >  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c
> >
> > [...]
> >
> > > +static int xilinx_cpm_pcie_probe(struct platform_device *pdev) {
> > > +       struct xilinx_cpm_pcie_port *port;
> > > +       struct device *dev = &pdev->dev;
> > > +       struct pci_host_bridge *bridge;
> > > +       struct resource *bus_range;
> > > +       int err;
> > > +
> > > +       bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> > > +       if (!bridge)
> > > +               return -ENODEV;
> > > +
> > > +       port = pci_host_bridge_priv(bridge);
> > > +
> > > +       port->dev = dev;
> > > +
> > > +       err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> > > +                                             &bridge->dma_ranges, &bus_range);
> > > +       if (err) {
> > > +               dev_err(dev, "Getting bridge resources failed\n");
> > > +               return err;
> > > +       }
> > > +
> > > +       err = xilinx_cpm_pcie_init_irq_domain(port);
> > > +       if (err)
> > > +               return err;
> > > +
> > > +       err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> > > +       if (err) {
> > > +               dev_err(dev, "Parsing DT failed\n");
> > > +               goto err_parse_dt;
> > > +       }
> > > +
> > > +       xilinx_cpm_pcie_init_port(port);
> > > +
> > > +       err = xilinx_cpm_setup_irq(port);
> > > +       if (err) {
> > > +               dev_err(dev, "Failed to set up interrupts\n");
> > > +               goto err_setup_irq;
> > > +       }
> >
> > All the h/w init here can be moved to an .init() function in ecam ops
> > and then use pci_host_common_probe. Given this is v9, that can be a
> > follow-up I guess.
> 
> I think there is time to get it done, Bharat please let me know if you can
> repost it shortly with Rob's requested change implemented.
>
Thanks Rob for your time.
Thanks Lorenzo, the reason I cannot use pci_host_common_probe is, 
I need pci_config_window locally as the we use same ecam space for local bridge register access.
In xilinx_cpm_pcie_parse_dt funciton
port->reg_base = port->cfg->win;

If we move to pci_host_common_probe, I will not be able to access controller registers.
So can we please proceed with existing flow. 

Regards,
Bharat

> >
> > > +
> > > +       bridge->dev.parent = dev;
> > > +       bridge->sysdata = port->cfg;
> > > +       bridge->busnr = port->cfg->busr.start;
> > > +       bridge->ops = &pci_generic_ecam_ops.pci_ops;
> > > +       bridge->map_irq = of_irq_parse_and_map_pci;
> > > +       bridge->swizzle_irq = pci_common_swizzle;
> > > +
> > > +       err = pci_host_probe(bridge);
> > > +       if (err < 0)
> > > +               goto err_host_bridge;
> > > +
> > > +       return 0;
> > > +
> > > +err_host_bridge:
> > > +       xilinx_cpm_free_interrupts(port);
> > > +err_setup_irq:
> > > +       pci_ecam_free(port->cfg);
> > > +err_parse_dt:
> > > +       xilinx_cpm_free_irq_domains(port);
> > > +       return err;
> > > +}

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Rob Herring]

On Mon, Jul 13, 2020 at 6:24 AM Bharat Kumar Gogada <bharatku@xilinx.com> wrote:
>
> > Subject: Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
> >
> > On Fri, Jul 10, 2020 at 09:16:57AM -0600, Rob Herring wrote:
> > > On Tue, Jun 16, 2020 at 6:57 AM Bharat Kumar Gogada
> > > <bharat.kumar.gogada@xilinx.com> wrote:
> > > >
> > > > - Add support for Versal CPM as Root Port.
> > > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The
> > integrated
> > > >   block for CPM along with the integrated bridge can function
> > > >   as PCIe Root Port.
> > > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > > >   Versal CPM specific interrupt line.
> > > >
> > > > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > > > ---
> > > >  drivers/pci/controller/Kconfig           |   8 +
> > > >  drivers/pci/controller/Makefile          |   1 +
> > > >  drivers/pci/controller/pcie-xilinx-cpm.c | 617
> > > > +++++++++++++++++++++++++++++++
> > > >  3 files changed, 626 insertions(+)
> > > >  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c
> > >
> > > [...]
> > >
> > > > +static int xilinx_cpm_pcie_probe(struct platform_device *pdev) {
> > > > +       struct xilinx_cpm_pcie_port *port;
> > > > +       struct device *dev = &pdev->dev;
> > > > +       struct pci_host_bridge *bridge;
> > > > +       struct resource *bus_range;
> > > > +       int err;
> > > > +
> > > > +       bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> > > > +       if (!bridge)
> > > > +               return -ENODEV;
> > > > +
> > > > +       port = pci_host_bridge_priv(bridge);
> > > > +
> > > > +       port->dev = dev;
> > > > +
> > > > +       err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
> > > > +                                             &bridge->dma_ranges, &bus_range);
> > > > +       if (err) {
> > > > +               dev_err(dev, "Getting bridge resources failed\n");
> > > > +               return err;
> > > > +       }
> > > > +
> > > > +       err = xilinx_cpm_pcie_init_irq_domain(port);
> > > > +       if (err)
> > > > +               return err;
> > > > +
> > > > +       err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> > > > +       if (err) {
> > > > +               dev_err(dev, "Parsing DT failed\n");
> > > > +               goto err_parse_dt;
> > > > +       }
> > > > +
> > > > +       xilinx_cpm_pcie_init_port(port);
> > > > +
> > > > +       err = xilinx_cpm_setup_irq(port);
> > > > +       if (err) {
> > > > +               dev_err(dev, "Failed to set up interrupts\n");
> > > > +               goto err_setup_irq;
> > > > +       }
> > >
> > > All the h/w init here can be moved to an .init() function in ecam ops
> > > and then use pci_host_common_probe. Given this is v9, that can be a
> > > follow-up I guess.
> >
> > I think there is time to get it done, Bharat please let me know if you can
> > repost it shortly with Rob's requested change implemented.
> >
> Thanks Rob for your time.
> Thanks Lorenzo, the reason I cannot use pci_host_common_probe is,
> I need pci_config_window locally as the we use same ecam space for local bridge register access.
> In xilinx_cpm_pcie_parse_dt funciton
> port->reg_base = port->cfg->win;

The .init() function is passed cfg, so what's the issue? You'll need
to alloc struct xilinx_cpm_pcie_port and then set it to cfg->priv. I'd
expect some fields to also be removed as there's no reason to store
things twice.

Rob

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bjorn Helgaas]

On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> - Add support for Versal CPM as Root Port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
>   block for CPM along with the integrated bridge can function
>   as PCIe Root Port.
> - Bridge error and legacy interrupts in Versal CPM are handled using
>   Versal CPM specific interrupt line.

> +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> +{
> +	if (cpm_pcie_link_up(port))
> +		dev_info(port->dev, "PCIe Link is UP\n");
> +	else
> +		dev_info(port->dev, "PCIe Link is DOWN\n");
> +
> +	/* Disable all interrupts */
> +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> +		   XILINX_CPM_PCIE_REG_IMR);
> +
> +	/* Clear pending interrupts */
> +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> +		   XILINX_CPM_PCIE_REG_IDR);
> +
> +	/*
> +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> +	 * CPM SLCR block.
> +	 */
> +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> +	/* Enable the Bridge enable bit */
> +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> +		   XILINX_CPM_PCIE_REG_RPSC);
> +}
> +
> +/**
> + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> + * @port: PCIe port information
> + * @bus_range: Bus resource
> + *
> + * Return: '0' on success and error value on failure
> + */
> +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> +				    struct resource *bus_range)
> +{
> +	struct device *dev = port->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct resource *res;
> +
> +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> +							       "cpm_slcr");
> +	if (IS_ERR(port->cpm_base))
> +		return PTR_ERR(port->cpm_base);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> +	if (!res)
> +		return -ENXIO;
> +
> +	port->cfg = pci_ecam_create(dev, res, bus_range,
> +				    &pci_generic_ecam_ops);

Aren't we passing an uninitialized pointer (bus_range) here?  This
looks broken to me.

The kernelci build warns about it:
https://kernelci.org/build/next/branch/master/kernel/next-20200805/

  /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]

I'm dropping this for now.  I can't believe this actually works.

Bjorn

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Lorenzo Pieralisi]

On Wed, Aug 05, 2020 at 03:43:58PM -0500, Bjorn Helgaas wrote:
> On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> > - Add support for Versal CPM as Root Port.
> > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> >   block for CPM along with the integrated bridge can function
> >   as PCIe Root Port.
> > - Bridge error and legacy interrupts in Versal CPM are handled using
> >   Versal CPM specific interrupt line.
> 
> > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> > +{
> > +	if (cpm_pcie_link_up(port))
> > +		dev_info(port->dev, "PCIe Link is UP\n");
> > +	else
> > +		dev_info(port->dev, "PCIe Link is DOWN\n");
> > +
> > +	/* Disable all interrupts */
> > +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> > +		   XILINX_CPM_PCIE_REG_IMR);
> > +
> > +	/* Clear pending interrupts */
> > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> > +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> > +		   XILINX_CPM_PCIE_REG_IDR);
> > +
> > +	/*
> > +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > +	 * CPM SLCR block.
> > +	 */
> > +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > +	/* Enable the Bridge enable bit */
> > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> > +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> > +		   XILINX_CPM_PCIE_REG_RPSC);
> > +}
> > +
> > +/**
> > + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> > + * @port: PCIe port information
> > + * @bus_range: Bus resource
> > + *
> > + * Return: '0' on success and error value on failure
> > + */
> > +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> > +				    struct resource *bus_range)
> > +{
> > +	struct device *dev = port->dev;
> > +	struct platform_device *pdev = to_platform_device(dev);
> > +	struct resource *res;
> > +
> > +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > +							       "cpm_slcr");
> > +	if (IS_ERR(port->cpm_base))
> > +		return PTR_ERR(port->cpm_base);
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> > +	if (!res)
> > +		return -ENXIO;
> > +
> > +	port->cfg = pci_ecam_create(dev, res, bus_range,
> > +				    &pci_generic_ecam_ops);
> 
> Aren't we passing an uninitialized pointer (bus_range) here?  This
> looks broken to me.
> 
> The kernelci build warns about it:
> https://kernelci.org/build/next/branch/master/kernel/next-20200805/
> 
>   /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]
> 
> I'm dropping this for now.  I can't believe this actually works.

It is caused by my rebase to fix -next after the rework in pci/misc
(I had to drop the call to pci_parse_request_of_pci_ranges()).

I will look into this tomorrow if Rob does not beat me to it.

Apologies, it is a new driver that was based on an interface
that is being reworked, for good reasons, in pci/misc.

Lorenzo

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bjorn Helgaas]

On Wed, Aug 05, 2020 at 10:39:28PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 05, 2020 at 03:43:58PM -0500, Bjorn Helgaas wrote:
> > On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> > > - Add support for Versal CPM as Root Port.
> > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> > >   block for CPM along with the integrated bridge can function
> > >   as PCIe Root Port.
> > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > >   Versal CPM specific interrupt line.
> > 
> > > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> > > +{
> > > +	if (cpm_pcie_link_up(port))
> > > +		dev_info(port->dev, "PCIe Link is UP\n");
> > > +	else
> > > +		dev_info(port->dev, "PCIe Link is DOWN\n");
> > > +
> > > +	/* Disable all interrupts */
> > > +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> > > +		   XILINX_CPM_PCIE_REG_IMR);
> > > +
> > > +	/* Clear pending interrupts */
> > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> > > +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> > > +		   XILINX_CPM_PCIE_REG_IDR);
> > > +
> > > +	/*
> > > +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > > +	 * CPM SLCR block.
> > > +	 */
> > > +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > > +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > > +	/* Enable the Bridge enable bit */
> > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> > > +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> > > +		   XILINX_CPM_PCIE_REG_RPSC);
> > > +}
> > > +
> > > +/**
> > > + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> > > + * @port: PCIe port information
> > > + * @bus_range: Bus resource
> > > + *
> > > + * Return: '0' on success and error value on failure
> > > + */
> > > +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> > > +				    struct resource *bus_range)
> > > +{
> > > +	struct device *dev = port->dev;
> > > +	struct platform_device *pdev = to_platform_device(dev);
> > > +	struct resource *res;
> > > +
> > > +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > > +							       "cpm_slcr");
> > > +	if (IS_ERR(port->cpm_base))
> > > +		return PTR_ERR(port->cpm_base);
> > > +
> > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> > > +	if (!res)
> > > +		return -ENXIO;
> > > +
> > > +	port->cfg = pci_ecam_create(dev, res, bus_range,
> > > +				    &pci_generic_ecam_ops);
> > 
> > Aren't we passing an uninitialized pointer (bus_range) here?  This
> > looks broken to me.
> > 
> > The kernelci build warns about it:
> > https://kernelci.org/build/next/branch/master/kernel/next-20200805/
> > 
> >   /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]
> > 
> > I'm dropping this for now.  I can't believe this actually works.
> 
> It is caused by my rebase to fix -next after the rework in pci/misc
> (I had to drop the call to pci_parse_request_of_pci_ranges()).
> 
> I will look into this tomorrow if Rob does not beat me to it.
> 
> Apologies, it is a new driver that was based on an interface
> that is being reworked, for good reasons, in pci/misc.

Oh, yep, I think I see what happened.  I'll try to fix this in hopes
of making linux-next tonight.

Bjorn

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bjorn Helgaas]

On Wed, Aug 05, 2020 at 05:03:26PM -0500, Bjorn Helgaas wrote:
> On Wed, Aug 05, 2020 at 10:39:28PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Aug 05, 2020 at 03:43:58PM -0500, Bjorn Helgaas wrote:
> > > On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> > > > - Add support for Versal CPM as Root Port.
> > > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> > > >   block for CPM along with the integrated bridge can function
> > > >   as PCIe Root Port.
> > > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > > >   Versal CPM specific interrupt line.
> > > 
> > > > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> > > > +{
> > > > +	if (cpm_pcie_link_up(port))
> > > > +		dev_info(port->dev, "PCIe Link is UP\n");
> > > > +	else
> > > > +		dev_info(port->dev, "PCIe Link is DOWN\n");
> > > > +
> > > > +	/* Disable all interrupts */
> > > > +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> > > > +		   XILINX_CPM_PCIE_REG_IMR);
> > > > +
> > > > +	/* Clear pending interrupts */
> > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> > > > +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> > > > +		   XILINX_CPM_PCIE_REG_IDR);
> > > > +
> > > > +	/*
> > > > +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > > > +	 * CPM SLCR block.
> > > > +	 */
> > > > +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > > > +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > > > +	/* Enable the Bridge enable bit */
> > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> > > > +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> > > > +		   XILINX_CPM_PCIE_REG_RPSC);
> > > > +}
> > > > +
> > > > +/**
> > > > + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> > > > + * @port: PCIe port information
> > > > + * @bus_range: Bus resource
> > > > + *
> > > > + * Return: '0' on success and error value on failure
> > > > + */
> > > > +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> > > > +				    struct resource *bus_range)
> > > > +{
> > > > +	struct device *dev = port->dev;
> > > > +	struct platform_device *pdev = to_platform_device(dev);
> > > > +	struct resource *res;
> > > > +
> > > > +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > > > +							       "cpm_slcr");
> > > > +	if (IS_ERR(port->cpm_base))
> > > > +		return PTR_ERR(port->cpm_base);
> > > > +
> > > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> > > > +	if (!res)
> > > > +		return -ENXIO;
> > > > +
> > > > +	port->cfg = pci_ecam_create(dev, res, bus_range,
> > > > +				    &pci_generic_ecam_ops);
> > > 
> > > Aren't we passing an uninitialized pointer (bus_range) here?  This
> > > looks broken to me.
> > > 
> > > The kernelci build warns about it:
> > > https://kernelci.org/build/next/branch/master/kernel/next-20200805/
> > > 
> > >   /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]
> > > 
> > > I'm dropping this for now.  I can't believe this actually works.
> > 
> > It is caused by my rebase to fix -next after the rework in pci/misc
> > (I had to drop the call to pci_parse_request_of_pci_ranges()).
> > 
> > I will look into this tomorrow if Rob does not beat me to it.
> > 
> > Apologies, it is a new driver that was based on an interface
> > that is being reworked, for good reasons, in pci/misc.
> 
> Oh, yep, I think I see what happened.  I'll try to fix this in hopes
> of making linux-next tonight.

OK, I think I fixed it.  Man, that was a lot of work for a git novice
like me ;)  Current head: 6f119ec8d9c8 ("Merge branch 'pci/irq-error'")

Diff from yesterday's "next" branch (a231039775c4 ("Merge branch
'pci/irq-error'")):

diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
index 5f9b9fc12500..f3082de44e8a 100644
--- a/drivers/pci/controller/pcie-xilinx-cpm.c
+++ b/drivers/pci/controller/pcie-xilinx-cpm.c
@@ -539,7 +539,7 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
 	struct xilinx_cpm_pcie_port *port;
 	struct device *dev = &pdev->dev;
 	struct pci_host_bridge *bridge;
-	struct resource *bus_range;
+	struct resource_entry *bus;
 	int err;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
@@ -554,7 +554,11 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	err = xilinx_cpm_pcie_parse_dt(port, bus_range);
+	bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
+	if (!bus)
+		return -ENODEV;
+
+	err = xilinx_cpm_pcie_parse_dt(port, bus->res);
 	if (err) {
 		dev_err(dev, "Parsing DT failed\n");
 		goto err_parse_dt;

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Lorenzo Pieralisi]

On Wed, Aug 05, 2020 at 06:30:50PM -0500, Bjorn Helgaas wrote:
> On Wed, Aug 05, 2020 at 05:03:26PM -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 05, 2020 at 10:39:28PM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Aug 05, 2020 at 03:43:58PM -0500, Bjorn Helgaas wrote:
> > > > On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> > > > > - Add support for Versal CPM as Root Port.
> > > > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> > > > >   block for CPM along with the integrated bridge can function
> > > > >   as PCIe Root Port.
> > > > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > > > >   Versal CPM specific interrupt line.
> > > > 
> > > > > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> > > > > +{
> > > > > +	if (cpm_pcie_link_up(port))
> > > > > +		dev_info(port->dev, "PCIe Link is UP\n");
> > > > > +	else
> > > > > +		dev_info(port->dev, "PCIe Link is DOWN\n");
> > > > > +
> > > > > +	/* Disable all interrupts */
> > > > > +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> > > > > +		   XILINX_CPM_PCIE_REG_IMR);
> > > > > +
> > > > > +	/* Clear pending interrupts */
> > > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> > > > > +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> > > > > +		   XILINX_CPM_PCIE_REG_IDR);
> > > > > +
> > > > > +	/*
> > > > > +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > > > > +	 * CPM SLCR block.
> > > > > +	 */
> > > > > +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > > > > +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > > > > +	/* Enable the Bridge enable bit */
> > > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> > > > > +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> > > > > +		   XILINX_CPM_PCIE_REG_RPSC);
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> > > > > + * @port: PCIe port information
> > > > > + * @bus_range: Bus resource
> > > > > + *
> > > > > + * Return: '0' on success and error value on failure
> > > > > + */
> > > > > +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> > > > > +				    struct resource *bus_range)
> > > > > +{
> > > > > +	struct device *dev = port->dev;
> > > > > +	struct platform_device *pdev = to_platform_device(dev);
> > > > > +	struct resource *res;
> > > > > +
> > > > > +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > > > > +							       "cpm_slcr");
> > > > > +	if (IS_ERR(port->cpm_base))
> > > > > +		return PTR_ERR(port->cpm_base);
> > > > > +
> > > > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> > > > > +	if (!res)
> > > > > +		return -ENXIO;
> > > > > +
> > > > > +	port->cfg = pci_ecam_create(dev, res, bus_range,
> > > > > +				    &pci_generic_ecam_ops);
> > > > 
> > > > Aren't we passing an uninitialized pointer (bus_range) here?  This
> > > > looks broken to me.
> > > > 
> > > > The kernelci build warns about it:
> > > > https://kernelci.org/build/next/branch/master/kernel/next-20200805/
> > > > 
> > > >   /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]
> > > > 
> > > > I'm dropping this for now.  I can't believe this actually works.
> > > 
> > > It is caused by my rebase to fix -next after the rework in pci/misc
> > > (I had to drop the call to pci_parse_request_of_pci_ranges()).
> > > 
> > > I will look into this tomorrow if Rob does not beat me to it.
> > > 
> > > Apologies, it is a new driver that was based on an interface
> > > that is being reworked, for good reasons, in pci/misc.
> > 
> > Oh, yep, I think I see what happened.  I'll try to fix this in hopes
> > of making linux-next tonight.
> 
> OK, I think I fixed it.  Man, that was a lot of work for a git novice
> like me ;)  Current head: 6f119ec8d9c8 ("Merge branch 'pci/irq-error'")

Sorry about that.

> Diff from yesterday's "next" branch (a231039775c4 ("Merge branch
> 'pci/irq-error'")):
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c
> index 5f9b9fc12500..f3082de44e8a 100644
> --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -539,7 +539,7 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
>  	struct xilinx_cpm_pcie_port *port;
>  	struct device *dev = &pdev->dev;
>  	struct pci_host_bridge *bridge;
> -	struct resource *bus_range;
> +	struct resource_entry *bus;
>  	int err;
>  
>  	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
> @@ -554,7 +554,11 @@ static int xilinx_cpm_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> -	err = xilinx_cpm_pcie_parse_dt(port, bus_range);
> +	bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
> +	if (!bus)
> +		return -ENODEV;
> +
> +	err = xilinx_cpm_pcie_parse_dt(port, bus->res);
>  	if (err) {
>  		dev_err(dev, "Parsing DT failed\n");
>  		goto err_parse_dt;

Thanks - that seems all right to me. Side note: I think this is
even more robust than the original code since you are *actually*
checking that an IORESOURCE_BUS is present (and it is not a possible
dangling pointer).

Re: [PATCH v9 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bjorn Helgaas]

On Thu, Aug 06, 2020 at 10:54:45AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 05, 2020 at 06:30:50PM -0500, Bjorn Helgaas wrote:
> > On Wed, Aug 05, 2020 at 05:03:26PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Aug 05, 2020 at 10:39:28PM +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Aug 05, 2020 at 03:43:58PM -0500, Bjorn Helgaas wrote:
> > > > > On Tue, Jun 16, 2020 at 06:26:54PM +0530, Bharat Kumar Gogada wrote:
> > > > > > - Add support for Versal CPM as Root Port.
> > > > > > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> > > > > >   block for CPM along with the integrated bridge can function
> > > > > >   as PCIe Root Port.
> > > > > > - Bridge error and legacy interrupts in Versal CPM are handled using
> > > > > >   Versal CPM specific interrupt line.
> > > > > 
> > > > > > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port)
> > > > > > +{
> > > > > > +	if (cpm_pcie_link_up(port))
> > > > > > +		dev_info(port->dev, "PCIe Link is UP\n");
> > > > > > +	else
> > > > > > +		dev_info(port->dev, "PCIe Link is DOWN\n");
> > > > > > +
> > > > > > +	/* Disable all interrupts */
> > > > > > +	pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK,
> > > > > > +		   XILINX_CPM_PCIE_REG_IMR);
> > > > > > +
> > > > > > +	/* Clear pending interrupts */
> > > > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) &
> > > > > > +		   XILINX_CPM_PCIE_IMR_ALL_MASK,
> > > > > > +		   XILINX_CPM_PCIE_REG_IDR);
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> > > > > > +	 * CPM SLCR block.
> > > > > > +	 */
> > > > > > +	writel(XILINX_CPM_PCIE_MISC_IR_LOCAL,
> > > > > > +	       port->cpm_base + XILINX_CPM_PCIE_MISC_IR_ENABLE);
> > > > > > +	/* Enable the Bridge enable bit */
> > > > > > +	pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_RPSC) |
> > > > > > +		   XILINX_CPM_PCIE_REG_RPSC_BEN,
> > > > > > +		   XILINX_CPM_PCIE_REG_RPSC);
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * xilinx_cpm_pcie_parse_dt - Parse Device tree
> > > > > > + * @port: PCIe port information
> > > > > > + * @bus_range: Bus resource
> > > > > > + *
> > > > > > + * Return: '0' on success and error value on failure
> > > > > > + */
> > > > > > +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port,
> > > > > > +				    struct resource *bus_range)
> > > > > > +{
> > > > > > +	struct device *dev = port->dev;
> > > > > > +	struct platform_device *pdev = to_platform_device(dev);
> > > > > > +	struct resource *res;
> > > > > > +
> > > > > > +	port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > > > > > +							       "cpm_slcr");
> > > > > > +	if (IS_ERR(port->cpm_base))
> > > > > > +		return PTR_ERR(port->cpm_base);
> > > > > > +
> > > > > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> > > > > > +	if (!res)
> > > > > > +		return -ENXIO;
> > > > > > +
> > > > > > +	port->cfg = pci_ecam_create(dev, res, bus_range,
> > > > > > +				    &pci_generic_ecam_ops);
> > > > > 
> > > > > Aren't we passing an uninitialized pointer (bus_range) here?  This
> > > > > looks broken to me.
> > > > > 
> > > > > The kernelci build warns about it:
> > > > > https://kernelci.org/build/next/branch/master/kernel/next-20200805/
> > > > > 
> > > > >   /scratch/linux/drivers/pci/controller/pcie-xilinx-cpm.c:557:39: warning: variable 'bus_range' is uninitialized when used here [-Wuninitialized]
> > > > > 
> > > > > I'm dropping this for now.  I can't believe this actually works.
> > > > 
> > > > It is caused by my rebase to fix -next after the rework in pci/misc
> > > > (I had to drop the call to pci_parse_request_of_pci_ranges()).
> > > > 
> > > > I will look into this tomorrow if Rob does not beat me to it.
> > > > 
> > > > Apologies, it is a new driver that was based on an interface
> > > > that is being reworked, for good reasons, in pci/misc.
> > > 
> > > Oh, yep, I think I see what happened.  I'll try to fix this in hopes
> > > of making linux-next tonight.
> > 
> > OK, I think I fixed it.  Man, that was a lot of work for a git novice
> > like me ;)  Current head: 6f119ec8d9c8 ("Merge branch 'pci/irq-error'")
> 
> Sorry about that.

No problem, if I were less OCD and more smart about git, it would have
been trivial.  But it did make it into the Aug 6 linux-next, so that's
good!

Re: [PATCH v9 0/2] Adding support for Versal CPM as Root Port driver

[Lorenzo Pieralisi]

On Tue, Jun 16, 2020 at 06:26:52PM +0530, Bharat Kumar Gogada wrote:
> - Adding support for Versal CPM as Root port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
>   block for CPM along with the integrated bridge can function
>   as PCIe Root Port.
> - Versal CPM uses GICv3 ITS feature for assigning MSI/MSI-X
>   vectors and handling MSI/MSI-X interrupts.
> - Bridge error and legacy interrupts in Versal CPM are handled using
>   Versal CPM specific interrupt line.
> 
> Changes for v9:
> - Removed interrupt enablement outside irqchip flow as suggested
>   by Marc.
> - Removed using WARN_ON in if statement.
> 
> Bharat Kumar Gogada (2):
>   PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
>   PCI: xilinx-cpm: Add Versal CPM Root Port driver
> 
>  .../devicetree/bindings/pci/xilinx-versal-cpm.yaml |  99 ++++
>  drivers/pci/controller/Kconfig                     |   8 +
>  drivers/pci/controller/Makefile                    |   1 +
>  drivers/pci/controller/pcie-xilinx-cpm.c           | 615 +++++++++++++++++++++
>  4 files changed, 723 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
>  create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c

Applied to pci/xilinx (I removed a function kerneldoc comment
(xilinx_cpm_pcie_clear_err_interrupts()) since it was not matching
the function name and honestly it is not really needed) for v5.9.

Thanks,
Lorenzo