* [PATCH v7 0/2] Adding support for Versal CPM as Root Port driver
@ 2020-05-07 11:58 Bharat Kumar Gogada
2020-05-07 11:58 ` [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
0 siblings, 2 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-07 11:58 UTC (permalink / raw)
To: linux-pci, linux-kernel
Cc: lorenzo.pieralisi, bhelgaas, robh, rgummal, 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 v7:
- Adding device tree documentation as schema.
- Using pci_host_probe for registering bridge.
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 | 105 +++++
drivers/pci/controller/Kconfig | 9 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-xilinx-cpm.c | 506 +++++++++++++++++++++
4 files changed, 621 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
2020-05-07 11:58 [PATCH v7 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada
@ 2020-05-07 11:58 ` Bharat Kumar Gogada
2020-05-15 5:38 ` Bharat Kumar Gogada
2020-05-20 22:20 ` Rob Herring
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
1 sibling, 2 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-07 11:58 UTC (permalink / raw)
To: linux-pci, linux-kernel
Cc: lorenzo.pieralisi, bhelgaas, robh, rgummal, 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 | 105 +++++++++++++++++++++
1 file changed, 105 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..5fc5c3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
@@ -0,0 +1,105 @@
+# 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>
+
+properties:
+ compatible:
+ const: xlnx,versal-cpm-host-1.00
+
+ "#address-cells":
+ const: 3
+
+ "#size-cells":
+ const: 2
+
+ 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-map-mask:
+ description: Standard PCI IRQ mapping properties.
+
+ interrupt-map:
+ description: Standard PCI IRQ mapping properties.
+
+ interrupt_controller:
+ description: Interrupt controller child node.
+
+ bus-range:
+ description: Range of bus numbers associated with this controller.
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+ - reg
+ - reg-names
+ - "#interrupt-cells"
+ - interrupts
+ - interrupt-parent
+ - interrupt-map
+ - interrupt-map-mask
+ - ranges
+ - bus-range
+ - msi-map
+
+additionalProperties: false
+
+examples:
+ - |
+
+ versal {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ cpm_pcie: pci@fca10000 {
+ compatible = "xlnx,versal-cpm-host-1.00";
+ #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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-07 11:58 [PATCH v7 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada
2020-05-07 11:58 ` [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada
@ 2020-05-07 11:58 ` Bharat Kumar Gogada
2020-05-07 19:49 ` Bjorn Helgaas
` (2 more replies)
1 sibling, 3 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-07 11:58 UTC (permalink / raw)
To: linux-pci, linux-kernel
Cc: lorenzo.pieralisi, bhelgaas, robh, rgummal, 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 | 9 +
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-xilinx-cpm.c | 506 +++++++++++++++++++++++++++++++
3 files changed, 516 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..ca0ae24 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -81,6 +81,15 @@ 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. The driver supports
+ MSI/MSI-X interrupts using GICv3 ITS feature.
+
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..e8c0aa7
--- /dev/null
+++ b/drivers/pci/controller/pcie-xilinx-cpm.c
@@ -0,0 +1,506 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
+ *
+ * (C) Copyright 2019 - 2020, Xilinx, Inc.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/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 BIT(0)
+#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
+#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
+#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
+#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
+#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
+#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
+#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
+#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
+#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
+#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
+#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
+#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
+#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
+#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
+#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
+#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
+#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF
+#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16)
+#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT BIT(4)
+#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
+#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
+#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
+#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
+#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
+ * @leg_domain: Legacy IRQ domain pointer
+ * @cfg: Holds mappings of config space window
+ * @irq_misc: Legacy and error interrupt number
+ * @leg_mask_lock: lock for legacy interrupts
+ */
+struct xilinx_cpm_pcie_port {
+ void __iomem *reg_base;
+ void __iomem *cpm_base;
+ struct device *dev;
+ struct irq_domain *leg_domain;
+ struct pci_config_window *cfg;
+ int irq_misc;
+ raw_spinlock_t leg_mask_lock;
+};
+
+static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg)
+{
+ return readl(port->reg_base + reg);
+}
+
+static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
+ u32 val, u32 reg)
+{
+ writel(val, port->reg_base + reg);
+}
+
+static inline 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) ? 1 : 0;
+}
+
+/**
+ * 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 irq_desc *desc = irq_to_desc(data->irq);
+ struct xilinx_cpm_pcie_port *port;
+ unsigned long flags;
+ u32 mask;
+ u32 val;
+
+ port = irq_desc_get_chip_data(desc);
+ mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
+ raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
+}
+
+static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
+{
+ struct irq_desc *desc = irq_to_desc(data->irq);
+ struct xilinx_cpm_pcie_port *port;
+ unsigned long flags;
+ u32 mask;
+ u32 val;
+
+ port = irq_desc_get_chip_data(desc);
+ mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
+ raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
+}
+
+static struct irq_chip xilinx_cpm_leg_irq_chip = {
+ .name = "xilinx_cpm_pcie:legacy",
+ .irq_enable = xilinx_cpm_unmask_leg_irq,
+ .irq_disable = xilinx_cpm_mask_leg_irq,
+ .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,
+};
+
+/**
+ * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
+ * @irq: IRQ number
+ * @data: PCIe port information
+ *
+ * Return: IRQ_HANDLED on success and IRQ_NONE on failure
+ */
+static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
+{
+ struct xilinx_cpm_pcie_port *port = data;
+ struct device *dev = port->dev;
+ u32 val, mask, status, bit;
+ unsigned long intr_val;
+
+ /* Read interrupt decode and mask registers */
+ val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
+ mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+
+ status = val & mask;
+ if (!status)
+ return IRQ_NONE;
+
+ if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
+ dev_warn(dev, "Link Down\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
+ dev_info(dev, "Hot reset\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
+ dev_warn(dev, "ECAM access timeout\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
+ dev_warn(dev, "Correctable error message\n");
+ cpm_pcie_clear_err_interrupts(port);
+ }
+
+ if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
+ dev_warn(dev, "Non fatal error message\n");
+ cpm_pcie_clear_err_interrupts(port);
+ }
+
+ if (status & XILINX_CPM_PCIE_INTR_FATAL) {
+ dev_warn(dev, "Fatal error message\n");
+ cpm_pcie_clear_err_interrupts(port);
+ }
+
+ if (status & XILINX_CPM_PCIE_INTR_INTX) {
+ /* Handle INTx Interrupt */
+ intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
+ intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
+
+ for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
+ generic_handle_irq(irq_find_mapping(port->leg_domain,
+ bit));
+ }
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
+ dev_warn(dev, "Slave unsupported request\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
+ dev_warn(dev, "Slave unexpected completion\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
+ dev_warn(dev, "Slave completion timeout\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
+ dev_warn(dev, "Slave Error Poison\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
+ dev_warn(dev, "Slave Completer Abort\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
+ dev_warn(dev, "Slave Illegal Burst\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
+ dev_warn(dev, "Master decode error\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
+ dev_warn(dev, "Master slave error\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
+ dev_warn(dev, "PCIe ECAM access timeout\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
+ dev_warn(dev, "ECAM poisoned completion received\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
+ dev_warn(dev, "PME_TO_ACK message received\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
+ dev_warn(dev, "PM_PME message received\n");
+
+ if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
+ dev_warn(dev, "PCIe completion timeout received\n");
+
+ /* Clear the Interrupt Decode register */
+ pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
+
+ /*
+ * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
+ * CPM SLCR block.
+ */
+ val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+ if (val)
+ writel(val, port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * 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->leg_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
+ &intx_domain_ops,
+ port);
+ of_node_put(pcie_intc_node);
+ if (!port->leg_domain) {
+ dev_err(dev, "Failed to get a INTx IRQ domain\n");
+ return -ENOMEM;
+ }
+
+ raw_spin_lock_init(&port->leg_mask_lock);
+ 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);
+
+ /* Enable all interrupts */
+ pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
+ XILINX_CPM_PCIE_REG_IMR);
+ pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
+ XILINX_CPM_PCIE_REG_IDRN_MASK);
+
+ /*
+ * 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);
+}
+
+static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port *port)
+{
+ struct device *dev = port->dev;
+ struct platform_device *pdev = to_platform_device(dev);
+ int err;
+
+ port->irq_misc = platform_get_irq(pdev, 0);
+ if (port->irq_misc <= 0) {
+ dev_err(dev, "Unable to find misc IRQ line\n");
+ return port->irq_misc;
+ }
+
+ err = devm_request_irq(dev, port->irq_misc,
+ xilinx_cpm_pcie_intr_handler,
+ IRQF_SHARED | IRQF_NO_THREAD,
+ "xilinx-pcie", port);
+ if (err) {
+ dev_err(dev, "unable to request misc IRQ line %d\n",
+ port->irq_misc);
+ return err;
+ }
+
+ return 0;
+}
+
+/**
+ * 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;
+ int err;
+
+ 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;
+
+ port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
+ "cpm_slcr");
+ if (IS_ERR(port->cpm_base))
+ return PTR_ERR(port->cpm_base);
+
+ err = xilinx_cpm_request_misc_irq(port);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+/**
+ * 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_parse_dt(port, bus_range);
+ if (err) {
+ dev_err(dev, "Parsing DT failed\n");
+ return err;
+ }
+
+ xilinx_cpm_pcie_init_port(port);
+
+ err = xilinx_cpm_pcie_init_irq_domain(port);
+ if (err) {
+ dev_err(dev, "Failed creating IRQ Domain\n");
+ return err;
+ }
+
+ 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) {
+ irq_domain_remove(port->leg_domain);
+ devm_free_irq(dev, port->irq_misc, port);
+ return err;
+ }
+
+ return 0;
+}
+
+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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
@ 2020-05-07 19:49 ` Bjorn Helgaas
2020-05-11 10:47 ` Bharat Kumar Gogada
2020-05-20 22:36 ` Rob Herring
2020-05-22 15:51 ` Marc Zyngier
2 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2020-05-07 19:49 UTC (permalink / raw)
To: Bharat Kumar Gogada
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, robh, rgummal
On Thu, May 07, 2020 at 05:28:36PM +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 inline 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) ? 1 : 0;
Almost all of the *_link_up() functions return "int". I don't know if
there's really any benefit to using "bool", but if you do, you should
probably return "true" or "false" instead of 1/0.
> + port->irq_misc = platform_get_irq(pdev, 0);
> + if (port->irq_misc <= 0) {
Use:
if (port->irq_misc < 0) {
See https://lore.kernel.org/r/20200501224042.141366-3-helgaas@kernel.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-07 19:49 ` Bjorn Helgaas
@ 2020-05-11 10:47 ` Bharat Kumar Gogada
0 siblings, 0 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-11 10:47 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, robh,
Ravikiran Gummaluri
> Subject: Re: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
>
> On Thu, May 07, 2020 at 05:28:36PM +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 inline 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) ? 1 : 0;
>
> Almost all of the *_link_up() functions return "int". I don't know if there's really
> any benefit to using "bool", but if you do, you should probably return "true" or
> "false" instead of 1/0.
Hi Bjorn, thanks will fix this.
>
> > + port->irq_misc = platform_get_irq(pdev, 0);
> > + if (port->irq_misc <= 0) {
>
> Use:
>
> if (port->irq_misc < 0) {
>
> See https://lore.kernel.org/r/20200501224042.141366-3-helgaas@kernel.org
Agreed, will fix this.
Regards,
Bharat
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
2020-05-07 11:58 ` [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada
@ 2020-05-15 5:38 ` Bharat Kumar Gogada
2020-05-20 22:20 ` Rob Herring
1 sibling, 0 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-15 5:38 UTC (permalink / raw)
To: Bharat Kumar Gogada, linux-pci, linux-kernel
Cc: lorenzo.pieralisi, bhelgaas, robh, Ravikiran Gummaluri
Hi Rob,
Can you please let us know if you have any inputs on this.
Regards,
Bharat
> -----Original Message-----
> From: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> Sent: Thursday, May 7, 2020 5:29 PM
> To: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: lorenzo.pieralisi@arm.com; bhelgaas@google.com; robh@kernel.org;
> Ravikiran Gummaluri <rgummal@xilinx.com>; Bharat Kumar Gogada
> <bharatku@xilinx.com>
> Subject: [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM
> Root Port
>
> 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 | 105
> +++++++++++++++++++++
> 1 file changed, 105 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..5fc5c3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -0,0 +1,105 @@
> +# 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>
> +
> +properties:
> + compatible:
> + const: xlnx,versal-cpm-host-1.00
> +
> + "#address-cells":
> + const: 3
> +
> + "#size-cells":
> + const: 2
> +
> + 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-map-mask:
> + description: Standard PCI IRQ mapping properties.
> +
> + interrupt-map:
> + description: Standard PCI IRQ mapping properties.
> +
> + interrupt_controller:
> + description: Interrupt controller child node.
> +
> + bus-range:
> + description: Range of bus numbers associated with this controller.
> +
> +required:
> + - compatible
> + - "#address-cells"
> + - "#size-cells"
> + - reg
> + - reg-names
> + - "#interrupt-cells"
> + - interrupts
> + - interrupt-parent
> + - interrupt-map
> + - interrupt-map-mask
> + - ranges
> + - bus-range
> + - msi-map
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> + versal {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + cpm_pcie: pci@fca10000 {
> + compatible = "xlnx,versal-cpm-host-1.00";
> + #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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
2020-05-07 11:58 ` [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada
2020-05-15 5:38 ` Bharat Kumar Gogada
@ 2020-05-20 22:20 ` Rob Herring
2020-05-26 12:50 ` Bharat Kumar Gogada
1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-05-20 22:20 UTC (permalink / raw)
To: Bharat Kumar Gogada
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, rgummal
On Thu, May 07, 2020 at 05:28:35PM +0530, Bharat Kumar Gogada 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 | 105 +++++++++++++++++++++
> 1 file changed, 105 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..5fc5c3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> @@ -0,0 +1,105 @@
> +# 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
> +
> + "#address-cells":
> + const: 3
> +
> + "#size-cells":
> + const: 2
Can drop.
> +
> + 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-map-mask:
> + description: Standard PCI IRQ mapping properties.
> +
> + interrupt-map:
> + description: Standard PCI IRQ mapping properties.
Can drop these 2.
> +
> + interrupt_controller:
s/_/-/
> + description: Interrupt controller child node.
type: object
And then need to describe all the properties under it too.
> +
> + bus-range:
> + description: Range of bus numbers associated with this controller.
Can drop.
> +
> +required:
> + - compatible
> + - "#address-cells"
> + - "#size-cells"
> + - reg
> + - reg-names
> + - "#interrupt-cells"
> + - interrupts
> + - interrupt-parent
> + - interrupt-map
> + - interrupt-map-mask
> + - ranges
> + - bus-range
> + - msi-map
interrupt-controller node not required?
You can drop all the standard properties required in pci-bus.yaml (it's
in dtschema repo).
> +
> +additionalProperties: false
This will need to be 'unevaluatedProperties: false'
> +
> +examples:
> + - |
> +
> + versal {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + cpm_pcie: pci@fca10000 {
pcie@...
> + compatible = "xlnx,versal-cpm-host-1.00";
> + #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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
2020-05-07 19:49 ` Bjorn Helgaas
@ 2020-05-20 22:36 ` Rob Herring
2020-05-22 15:51 ` Marc Zyngier
2 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2020-05-20 22:36 UTC (permalink / raw)
To: Bharat Kumar Gogada
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, rgummal
On Thu, May 07, 2020 at 05:28:36PM +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.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
> drivers/pci/controller/Kconfig | 9 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-xilinx-cpm.c | 506 +++++++++++++++++++++++++++++++
> 3 files changed, 516 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..ca0ae24 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -81,6 +81,15 @@ 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. The driver supports
> + MSI/MSI-X interrupts using GICv3 ITS feature.
> +
> 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..e8c0aa7
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -0,0 +1,506 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
> + *
> + * (C) Copyright 2019 - 2020, Xilinx, Inc.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/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 BIT(0)
> +#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
> +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
> +#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
> +#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
> +#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
> +#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
> +#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
> +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
> +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
> +#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
> +#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
> +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
> +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
> +#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
> +#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
> +#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
> +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF
> +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16)
> +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT BIT(4)
> +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
> +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
> +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
> +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
> +#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
> + * @leg_domain: Legacy IRQ domain pointer
> + * @cfg: Holds mappings of config space window
> + * @irq_misc: Legacy and error interrupt number
> + * @leg_mask_lock: lock for legacy interrupts
> + */
> +struct xilinx_cpm_pcie_port {
> + void __iomem *reg_base;
> + void __iomem *cpm_base;
> + struct device *dev;
> + struct irq_domain *leg_domain;
> + struct pci_config_window *cfg;
> + int irq_misc;
> + raw_spinlock_t leg_mask_lock;
> +};
> +
> +static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg)
> +{
> + return readl(port->reg_base + reg);
> +}
> +
> +static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
> + u32 val, u32 reg)
> +{
> + writel(val, port->reg_base + reg);
> +}
> +
> +static inline 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) ? 1 : 0;
> +}
> +
> +/**
> + * 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 irq_desc *desc = irq_to_desc(data->irq);
> + struct xilinx_cpm_pcie_port *port;
> + unsigned long flags;
> + u32 mask;
> + u32 val;
> +
> + port = irq_desc_get_chip_data(desc);
> + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
> +}
> +
> +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
> +{
> + struct irq_desc *desc = irq_to_desc(data->irq);
> + struct xilinx_cpm_pcie_port *port;
> + unsigned long flags;
> + u32 mask;
> + u32 val;
> +
> + port = irq_desc_get_chip_data(desc);
> + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
> +}
> +
> +static struct irq_chip xilinx_cpm_leg_irq_chip = {
> + .name = "xilinx_cpm_pcie:legacy",
> + .irq_enable = xilinx_cpm_unmask_leg_irq,
> + .irq_disable = xilinx_cpm_mask_leg_irq,
> + .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,
> +};
> +
> +/**
> + * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
> + * @irq: IRQ number
> + * @data: PCIe port information
> + *
> + * Return: IRQ_HANDLED on success and IRQ_NONE on failure
> + */
> +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
> +{
> + struct xilinx_cpm_pcie_port *port = data;
> + struct device *dev = port->dev;
> + u32 val, mask, status, bit;
> + unsigned long intr_val;
> +
> + /* Read interrupt decode and mask registers */
> + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> + mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> +
> + status = val & mask;
> + if (!status)
> + return IRQ_NONE;
> +
> + if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
> + dev_warn(dev, "Link Down\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> + dev_info(dev, "Hot reset\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
> + dev_warn(dev, "ECAM access timeout\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
> + dev_warn(dev, "Correctable error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
> + dev_warn(dev, "Non fatal error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_FATAL) {
> + dev_warn(dev, "Fatal error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_INTX) {
> + /* Handle INTx Interrupt */
> + intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
> + intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
> +
> + for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
> + generic_handle_irq(irq_find_mapping(port->leg_domain,
> + bit));
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
> + dev_warn(dev, "Slave unsupported request\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
> + dev_warn(dev, "Slave unexpected completion\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
> + dev_warn(dev, "Slave completion timeout\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
> + dev_warn(dev, "Slave Error Poison\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
> + dev_warn(dev, "Slave Completer Abort\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
> + dev_warn(dev, "Slave Illegal Burst\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
> + dev_warn(dev, "Master decode error\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
> + dev_warn(dev, "Master slave error\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
> + dev_warn(dev, "PCIe ECAM access timeout\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
> + dev_warn(dev, "ECAM poisoned completion received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
> + dev_warn(dev, "PME_TO_ACK message received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
> + dev_warn(dev, "PM_PME message received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
> + dev_warn(dev, "PCIe completion timeout received\n");
> +
> + /* Clear the Interrupt Decode register */
> + pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
> +
> + /*
> + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
> + * CPM SLCR block.
> + */
> + val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> + if (val)
> + writel(val, port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * 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->leg_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
> + &intx_domain_ops,
> + port);
> + of_node_put(pcie_intc_node);
> + if (!port->leg_domain) {
> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + raw_spin_lock_init(&port->leg_mask_lock);
> + 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);
> +
> + /* Enable all interrupts */
> + pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
> + XILINX_CPM_PCIE_REG_IMR);
> + pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
> + XILINX_CPM_PCIE_REG_IDRN_MASK);
> +
> + /*
> + * 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);
> +}
> +
> +static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port *port)
> +{
> + struct device *dev = port->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + int err;
> +
> + port->irq_misc = platform_get_irq(pdev, 0);
> + if (port->irq_misc <= 0) {
> + dev_err(dev, "Unable to find misc IRQ line\n");
> + return port->irq_misc;
> + }
> +
> + err = devm_request_irq(dev, port->irq_misc,
> + xilinx_cpm_pcie_intr_handler,
> + IRQF_SHARED | IRQF_NO_THREAD,
> + "xilinx-pcie", port);
> + if (err) {
> + dev_err(dev, "unable to request misc IRQ line %d\n",
> + port->irq_misc);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * 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;
> + int err;
> +
> + 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);
Any errors after this point need to call pci_ecam_free(). Maybe this can
be done later?
I think you can rework this to use pci_host_common_probe() instead.
You'll need an .init() hook.
> +
> + port->reg_base = port->cfg->win;
> +
> + port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> + "cpm_slcr");
> + if (IS_ERR(port->cpm_base))
> + return PTR_ERR(port->cpm_base);
> +
> + err = xilinx_cpm_request_misc_irq(port);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +/**
> + * 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_parse_dt(port, bus_range);
> + if (err) {
> + dev_err(dev, "Parsing DT failed\n");
> + return err;
> + }
> +
> + xilinx_cpm_pcie_init_port(port);
> +
> + err = xilinx_cpm_pcie_init_irq_domain(port);
> + if (err) {
> + dev_err(dev, "Failed creating IRQ Domain\n");
> + return err;
> + }
> +
> + 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) {
> + irq_domain_remove(port->leg_domain);
> + devm_free_irq(dev, port->irq_misc, port);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
2020-05-07 19:49 ` Bjorn Helgaas
2020-05-20 22:36 ` Rob Herring
@ 2020-05-22 15:51 ` Marc Zyngier
2020-05-27 17:08 ` Bharat Kumar Gogada
2 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-05-22 15:51 UTC (permalink / raw)
To: Bharat Kumar Gogada
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, robh, rgummal
Bharat,
On 2020-05-07 12:58, 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.
>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
> drivers/pci/controller/Kconfig | 9 +
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-xilinx-cpm.c | 506
> +++++++++++++++++++++++++++++++
> 3 files changed, 516 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..ca0ae24 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -81,6 +81,15 @@ 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. The driver supports
> + MSI/MSI-X interrupts using GICv3 ITS feature.
I don't think this last sentense makes any sense here. We usually don't
mention things that the driver *doesn't* implement.
> +
> 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..e8c0aa7
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -0,0 +1,506 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
> + *
> + * (C) Copyright 2019 - 2020, Xilinx, Inc.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/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 BIT(0)
> +#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
> +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
> +#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
> +#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
> +#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
> +#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
> +#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
> +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
> +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
> +#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
> +#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
> +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
> +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
> +#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
> +#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
> +#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
I assume that this is the logical OR of all the XILINX_CPM_PCIE_INTR_*
bits. Please express it as such.
> +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF
> +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16)
> +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT BIT(4)
> +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
> +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
> +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
So we have two definitions for bit 17... Given that nothing uses the MSI
version, I assume it is useless...
> +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
Please group all the XILINX_CPM_PCIE_INTR_* together.
> +#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
> + * @leg_domain: Legacy IRQ domain pointer
leg, arm, thumb, limb... Given that oyu use the 'intx' description all
over the driver, please be consistent and name this intx_domain.
> + * @cfg: Holds mappings of config space window
> + * @irq_misc: Legacy and error interrupt number
Let's face it, this is the *only* interrupt this thing as, so the 'misc'
is supperfluous.
> + * @leg_mask_lock: lock for legacy interrupts
> + */
> +struct xilinx_cpm_pcie_port {
> + void __iomem *reg_base;
> + void __iomem *cpm_base;
> + struct device *dev;
> + struct irq_domain *leg_domain;
> + struct pci_config_window *cfg;
> + int irq_misc;
> + raw_spinlock_t leg_mask_lock;
> +};
> +
> +static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32
> reg)
> +{
> + return readl(port->reg_base + reg);
There is no need for enforced ordering with non-device writes, so you
can turn this into readl_relaxed. You can also drop the inline, as the
compiler will do the right thing for you.
> +}
> +
> +static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
> + u32 val, u32 reg)
> +{
> + writel(val, port->reg_base + reg);
Same thing.
> +}
> +
> +static inline 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) ? 1 : 0;
If you are making the return value a bool, you might as well return
true/false. But that's a cumbersome way to write:
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 irq_desc *desc = irq_to_desc(data->irq);
> + struct xilinx_cpm_pcie_port *port;
> + unsigned long flags;
> + u32 mask;
> + u32 val;
> +
> + port = irq_desc_get_chip_data(desc);
port = irq_data_get_irq_chip_data(data);
You should never have to get the irq_desc (and using irq_to_desc() is a
prettyodd way to get it when you already have the irq_data).
> + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
Also known as BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT).
> + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
> +}
> +
> +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
> +{
> + struct irq_desc *desc = irq_to_desc(data->irq);
> + struct xilinx_cpm_pcie_port *port;
> + unsigned long flags;
> + u32 mask;
> + u32 val;
> +
> + port = irq_desc_get_chip_data(desc);
> + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags);
> +}
> +
> +static struct irq_chip xilinx_cpm_leg_irq_chip = {
> + .name = "xilinx_cpm_pcie:legacy",
"INTx" is a good enough description.
> + .irq_enable = xilinx_cpm_unmask_leg_irq,
> + .irq_disable = xilinx_cpm_mask_leg_irq,
> + .irq_mask = xilinx_cpm_mask_leg_irq,
> + .irq_unmask = xilinx_cpm_unmask_leg_irq,
This makes no sense. If enable/disable have the same implementation as
unmask/mask, then enable/disable is pretty useless. Please drop them.
> +};
> +
> +/**
> + * 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,
> +};
> +
> +/**
> + * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
> + * @irq: IRQ number
> + * @data: PCIe port information
> + *
> + * Return: IRQ_HANDLED on success and IRQ_NONE on failure
> + */
> +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
> +{
> + struct xilinx_cpm_pcie_port *port = data;
> + struct device *dev = port->dev;
> + u32 val, mask, status, bit;
> + unsigned long intr_val;
> +
> + /* Read interrupt decode and mask registers */
> + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> + mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> +
> + status = val & mask;
> + if (!status)
> + return IRQ_NONE;
> +
> + if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
> + dev_warn(dev, "Link Down\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> + dev_info(dev, "Hot reset\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
> + dev_warn(dev, "ECAM access timeout\n");
There is a certain sense of repetition here. It really begs the question
of *why* you want to take these interrupts if all you do is spam the
console with warnings, not taking any action to potentially remedy the
problem.
> +
> + if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
> + dev_warn(dev, "Correctable error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
> + dev_warn(dev, "Non fatal error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_FATAL) {
> + dev_warn(dev, "Fatal error message\n");
> + cpm_pcie_clear_err_interrupts(port);
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_INTX) {
> + /* Handle INTx Interrupt */
> + intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
> + intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
> +
> + for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
> + generic_handle_irq(irq_find_mapping(port->leg_domain,
> + bit));
That's a firm no. We don't demux chained handlers in an interrupt
handler. It may work for now, but it will eventually break. And to be
clear, this whole function needs to die. By the look of it, this PCie
controller implements *TWO* interrupt multiplexers:
- one that muxes all the ILINX_CPM_PCIE_INTR_* events onto a single
top-level IRQ
- another one that muxes all INTX lines onto XILINX_CPM_PCIE_INTR_INTX
Please implement the whole thing as described above.
> + }
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
> + dev_warn(dev, "Slave unsupported request\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
> + dev_warn(dev, "Slave unexpected completion\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
> + dev_warn(dev, "Slave completion timeout\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
> + dev_warn(dev, "Slave Error Poison\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
> + dev_warn(dev, "Slave Completer Abort\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
> + dev_warn(dev, "Slave Illegal Burst\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
> + dev_warn(dev, "Master decode error\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
> + dev_warn(dev, "Master slave error\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
> + dev_warn(dev, "PCIe ECAM access timeout\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
> + dev_warn(dev, "ECAM poisoned completion received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
> + dev_warn(dev, "PME_TO_ACK message received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
> + dev_warn(dev, "PM_PME message received\n");
> +
> + if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
> + dev_warn(dev, "PCIe completion timeout received\n");
I'm pretty sure there is a slightly better way to write this...
> +
> + /* Clear the Interrupt Decode register */
> + pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
> +
> + /*
> + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
> + * CPM SLCR block.
> + */
> + val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> + if (val)
> + writel(val, port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * 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->leg_domain = irq_domain_add_linear(pcie_intc_node,
> PCI_NUM_INTX,
> + &intx_domain_ops,
> + port);
> + of_node_put(pcie_intc_node);
> + if (!port->leg_domain) {
> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + raw_spin_lock_init(&port->leg_mask_lock);
> + 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);
> +
> + /* Enable all interrupts */
> + pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
> + XILINX_CPM_PCIE_REG_IMR);
> + pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
> + XILINX_CPM_PCIE_REG_IDRN_MASK);
No. We don't enable interrupts randomly. They need a handler registered
with the core IRq subsystem *first*, which is why this needs to be
hooked in as a proper irqchip, and each event handled individualy.
> +
> + /*
> + * 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);
> +}
> +
> +static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port
> *port)
> +{
> + struct device *dev = port->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + int err;
> +
> + port->irq_misc = platform_get_irq(pdev, 0);
> + if (port->irq_misc <= 0) {
If 0 is an error, how do you distinguish it from the non-error case?
> + dev_err(dev, "Unable to find misc IRQ line\n");
> + return port->irq_misc;
> + }
> +
> + err = devm_request_irq(dev, port->irq_misc,
> + xilinx_cpm_pcie_intr_handler,
> + IRQF_SHARED | IRQF_NO_THREAD,
> + "xilinx-pcie", port);
> + if (err) {
> + dev_err(dev, "unable to request misc IRQ line %d\n",
> + port->irq_misc);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * 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;
> + int err;
> +
> + 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;
> +
> + port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> + "cpm_slcr");
> + if (IS_ERR(port->cpm_base))
> + return PTR_ERR(port->cpm_base);
> +
> + err = xilinx_cpm_request_misc_irq(port);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +/**
> + * 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_parse_dt(port, bus_range);
> + if (err) {
> + dev_err(dev, "Parsing DT failed\n");
> + return err;
> + }
> +
> + xilinx_cpm_pcie_init_port(port);
> +
> + err = xilinx_cpm_pcie_init_irq_domain(port);
> + if (err) {
> + dev_err(dev, "Failed creating IRQ Domain\n");
> + return err;
> + }
> +
> + 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) {
> + irq_domain_remove(port->leg_domain);
> + devm_free_irq(dev, port->irq_misc, port);
Why calling dem_free_irq()? The whole point of using devm* is not to
have to manage the error handling.
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +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);
I don't think this driver is in a state where it can be merged. Given
that we're already at v7 and to save everyone a bit of time, I've
written the patch that implements the above comments. It compiles, but
is probably broken. Hopefully you can test it and figure things out.
M.
From 8b5d158374e59edf26e61c512eeb00ca7c9d891d Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Fri, 22 May 2020 16:33:32 +0100
Subject: [PATCH] PCI: xilinx-cpm: Revamp irq handling
The xilinx-cpm driver missuses the IRQ layer in creative ways.
It implements a chained interrupt demultiplexer inside a normal
handler, enables interrupts randomly, and could overall do with
a good cleanup.
Instead, implement the IRQ support as two nested chained irqchips,
the outer one dealing with all the PCIe events, the inner one
namaging the INTx signals.
Additionally, the whole driver is cleanup-up so that it can make
a bit more sense to me. YMMV.
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/pci/controller/Kconfig | 3 +-
drivers/pci/controller/pcie-xilinx-cpm.c | 427 ++++++++++++++---------
2 files changed, 263 insertions(+), 167 deletions(-)
diff --git a/drivers/pci/controller/Kconfig
b/drivers/pci/controller/Kconfig
index 1e0f63865775..d0abd671a7b6 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -87,8 +87,7 @@ config PCIE_XILINX_CPM
select PCI_HOST_COMMON
help
Say 'Y' here if you want kernel support for the
- Xilinx Versal CPM host bridge. The driver supports
- MSI/MSI-X interrupts using GICv3 ITS feature.
+ Xilinx Versal CPM host bridge.
config PCI_XGENE
bool "X-Gene PCIe controller"
diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c
b/drivers/pci/controller/pcie-xilinx-cpm.c
index e8c0aa757f72..32ed9e7e2676 100644
--- a/drivers/pci/controller/pcie-xilinx-cpm.c
+++ b/drivers/pci/controller/pcie-xilinx-cpm.c
@@ -5,8 +5,11 @@
* (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>
@@ -33,30 +36,55 @@
#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1)
/* Interrupt registers definitions */
-#define XILINX_CPM_PCIE_INTR_LINK_DOWN BIT(0)
-#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
-#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
-#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
-#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
-#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
-#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
-#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
-#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
-#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
-#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
-#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
-#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
-#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
-#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
-#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
-#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
+#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_INTR_CFG_PCIE_TIMEOUT BIT(4)
-#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
-#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
-#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
-#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
#define XILINX_CPM_PCIE_IDRN_SHIFT 16
/* Root Port Error FIFO Read Register definitions */
@@ -75,36 +103,37 @@
* @reg_base: Bridge Register Base
* @cpm_base: CPM System Level Control and Status Register(SLCR) Base
* @dev: Device pointer
- * @leg_domain: Legacy IRQ domain pointer
+ * @intx_domain: Legacy IRQ domain pointer
* @cfg: Holds mappings of config space window
- * @irq_misc: Legacy and error interrupt number
- * @leg_mask_lock: lock for legacy interrupts
+ * @irq: INTx and 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 *leg_domain;
- struct pci_config_window *cfg;
- int irq_misc;
- raw_spinlock_t leg_mask_lock;
+ 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 irq;
+ raw_spinlock_t lock;
};
static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg)
{
- return readl(port->reg_base + reg);
+ return readl_relaxed(port->reg_base + reg);
}
static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
u32 val, u32 reg)
{
- writel(val, port->reg_base + reg);
+ writel_relaxed(val, port->reg_base + reg);
}
static inline 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) ? 1 : 0;
+ XILINX_CPM_PCIE_REG_PSCR_LNKUP);
}
/**
@@ -125,44 +154,56 @@ static void cpm_pcie_clear_err_interrupts(struct
xilinx_cpm_pcie_port *port)
static void xilinx_cpm_mask_leg_irq(struct irq_data *data)
{
- struct irq_desc *desc = irq_to_desc(data->irq);
- struct xilinx_cpm_pcie_port *port;
+ struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data);
unsigned long flags;
u32 mask;
u32 val;
- port = irq_desc_get_chip_data(desc);
- mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
- raw_spin_lock_irqsave(&port->leg_mask_lock, flags);
+ 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->leg_mask_lock, flags);
+ raw_spin_unlock_irqrestore(&port->lock, flags);
}
static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
{
- struct irq_desc *desc = irq_to_desc(data->irq);
- struct xilinx_cpm_pcie_port *port;
+ struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data);
unsigned long flags;
u32 mask;
u32 val;
- port = irq_desc_get_chip_data(desc);
- mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
- raw_spin_lock_irqsave(&port->leg_mask_lock, flags);
+ 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->leg_mask_lock, flags);
+ raw_spin_unlock_irqrestore(&port->lock, flags);
}
static struct irq_chip xilinx_cpm_leg_irq_chip = {
- .name = "xilinx_cpm_pcie:legacy",
- .irq_enable = xilinx_cpm_unmask_leg_irq,
- .irq_disable = xilinx_cpm_mask_leg_irq,
- .irq_mask = xilinx_cpm_mask_leg_irq,
- .irq_unmask = xilinx_cpm_unmask_leg_irq,
+ .name = "INTx",
+ .irq_mask = xilinx_cpm_mask_leg_irq,
+ .irq_unmask = xilinx_cpm_unmask_leg_irq,
};
+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);
+}
+
/**
* xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark IRQ
as valid
* @domain: IRQ domain
@@ -187,111 +228,130 @@ static const struct irq_domain_ops
intx_domain_ops = {
.map = xilinx_cpm_pcie_intx_map,
};
-/**
- * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
- * @irq: IRQ number
- * @data: PCIe port information
- *
- * Return: IRQ_HANDLED on success and IRQ_NONE on failure
- */
-static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
+static void xilinx_cpm_mask_event_irq(struct irq_data *d)
{
- struct xilinx_cpm_pcie_port *port = data;
- struct device *dev = port->dev;
- u32 val, mask, status, bit;
- unsigned long intr_val;
-
- /* Read interrupt decode and mask registers */
- val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
- mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
-
- status = val & mask;
- if (!status)
- return IRQ_NONE;
-
- if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
- dev_warn(dev, "Link Down\n");
-
- if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
- dev_info(dev, "Hot reset\n");
-
- if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
- dev_warn(dev, "ECAM access timeout\n");
-
- if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
- dev_warn(dev, "Correctable error message\n");
- cpm_pcie_clear_err_interrupts(port);
- }
-
- if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
- dev_warn(dev, "Non fatal error message\n");
- cpm_pcie_clear_err_interrupts(port);
- }
-
- if (status & XILINX_CPM_PCIE_INTR_FATAL) {
- dev_warn(dev, "Fatal error message\n");
- cpm_pcie_clear_err_interrupts(port);
- }
-
- if (status & XILINX_CPM_PCIE_INTR_INTX) {
- /* Handle INTx Interrupt */
- intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
- intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
-
- for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
- generic_handle_irq(irq_find_mapping(port->leg_domain,
- bit));
- }
-
- if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
- dev_warn(dev, "Slave unsupported request\n");
- if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
- dev_warn(dev, "Slave unexpected completion\n");
+ struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
+ u32 val;
- if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
- dev_warn(dev, "Slave completion timeout\n");
+ raw_spin_lock(&port->lock);
+ val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+ val &= ~d->hwirq;
+ pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
+ raw_spin_unlock(&port->lock);
+}
- if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
- dev_warn(dev, "Slave Error Poison\n");
+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;
- if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
- dev_warn(dev, "Slave Completer Abort\n");
+ raw_spin_lock(&port->lock);
+ val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
+ val |= d->hwirq;
+ pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
+ raw_spin_unlock(&port->lock);
+}
- if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
- dev_warn(dev, "Slave Illegal Burst\n");
+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,
+};
- if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
- dev_warn(dev, "Master decode error\n");
+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);
- if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
- dev_warn(dev, "Master slave error\n");
+ return 0;
+}
- if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
- dev_warn(dev, "PCIe ECAM access timeout\n");
+static const struct irq_domain_ops event_domain_ops = {
+ .map = xilinx_cpm_pcie_event_map,
+};
- if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
- dev_warn(dev, "ECAM poisoned completion received\n");
+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;
- if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
- dev_warn(dev, "PME_TO_ACK message received\n");
+ chained_irq_enter(chip, desc);
- if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
- dev_warn(dev, "PM_PME message received\n");
+ val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
+ val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
- if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
- dev_warn(dev, "PCIe completion timeout received\n");
+ for_each_set_bit(i, &val, 32)
+ generic_handle_irq(irq_find_mapping(port->cpm_domain, i));
/* Clear the Interrupt Decode register */
- pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
+ pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
/*
* XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
* CPM SLCR block.
*/
- val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+ val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
if (val)
- writel(val, port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
+ 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 interrupt\n");
+ }
return IRQ_HANDLED;
}
@@ -315,17 +375,41 @@ static int xilinx_cpm_pcie_init_irq_domain(struct
xilinx_cpm_pcie_port *port)
return -EINVAL;
}
- port->leg_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
+ 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);
- if (!port->leg_domain) {
- dev_err(dev, "Failed to get a INTx IRQ domain\n");
- return -ENOMEM;
- }
+ raw_spin_lock_init(&port->lock);
- raw_spin_lock_init(&port->leg_mask_lock);
return 0;
+
+out:
+ of_node_put(pcie_intc_node);
+ 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;
+ }
+
+ dev_err(dev, "Failed to allocate IRQ domains\n");
+ return -ENOMEM;
}
/**
@@ -348,12 +432,6 @@ static void xilinx_cpm_pcie_init_port(struct
xilinx_cpm_pcie_port *port)
XILINX_CPM_PCIE_IMR_ALL_MASK,
XILINX_CPM_PCIE_REG_IDR);
- /* Enable all interrupts */
- pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
- XILINX_CPM_PCIE_REG_IMR);
- pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
- XILINX_CPM_PCIE_REG_IDRN_MASK);
-
/*
* XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
* CPM SLCR block.
@@ -366,28 +444,45 @@ static void xilinx_cpm_pcie_init_port(struct
xilinx_cpm_pcie_port *port)
XILINX_CPM_PCIE_REG_RPSC);
}
-static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port
*port)
+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 err;
+ int i, irq;
- port->irq_misc = platform_get_irq(pdev, 0);
- if (port->irq_misc <= 0) {
- dev_err(dev, "Unable to find misc IRQ line\n");
- return port->irq_misc;
+ port->irq = platform_get_irq(pdev, 0);
+ if (port->irq < 0) {
+ dev_err(dev, "Unable to find IRQ line\n");
+ return port->irq;
}
- err = devm_request_irq(dev, port->irq_misc,
- xilinx_cpm_pcie_intr_handler,
- IRQF_SHARED | IRQF_NO_THREAD,
- "xilinx-pcie", port);
- if (err) {
- dev_err(dev, "unable to request misc IRQ line %d\n",
- port->irq_misc);
- return err;
+ 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 (WARN_ON(irq <= 0))
+ return -ENXIO;
+
+ err = devm_request_irq(dev, irq, xilinx_cpm_pcie_intr_handler,
+ 0, intr_cause[i].sym, port);
+ if (WARN_ON(err))
+ return err;
}
+ irq = irq_create_mapping(port->cpm_domain, XILINX_CPM_PCIE_INTR_INTX);
+ if (WARN_ON(irq <= 0))
+ return -ENXIO;
+
+ /* Plug the INTx chained handler */
+ irq_set_chained_handler_and_data(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;
}
@@ -422,7 +517,7 @@ static int xilinx_cpm_pcie_parse_dt(struct
xilinx_cpm_pcie_port *port,
if (IS_ERR(port->cpm_base))
return PTR_ERR(port->cpm_base);
- err = xilinx_cpm_request_misc_irq(port);
+ err = xilinx_cpm_setup_irq(port);
if (err)
return err;
@@ -481,8 +576,10 @@ static int xilinx_cpm_pcie_probe(struct
platform_device *pdev)
err = pci_host_probe(bridge);
if (err < 0) {
- irq_domain_remove(port->leg_domain);
- devm_free_irq(dev, port->irq_misc, port);
+ if (port->intx_domain)
+ irq_domain_remove(port->intx_domain);
+ if (port->cpm_domain)
+ irq_domain_remove(port->cpm_domain);
return err;
}
--
2.26.2
--
Jazz is not dead. It just smells funny...
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port
2020-05-20 22:20 ` Rob Herring
@ 2020-05-26 12:50 ` Bharat Kumar Gogada
0 siblings, 0 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-26 12:50 UTC (permalink / raw)
To: Rob Herring
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas,
Ravikiran Gummaluri
> Subject: Re: [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal
> CPM Root Port
>
> On Thu, May 07, 2020 at 05:28:35PM +0530, Bharat Kumar Gogada 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 | 105
> > +++++++++++++++++++++
> > 1 file changed, 105 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..5fc5c3f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml
> > @@ -0,0 +1,105 @@
> > +# 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
> > +
>
> > + "#address-cells":
> > + const: 3
> > +
> > + "#size-cells":
> > + const: 2
>
> Can drop.
>
> > +
> > + 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-map-mask:
> > + description: Standard PCI IRQ mapping properties.
> > +
> > + interrupt-map:
> > + description: Standard PCI IRQ mapping properties.
>
> Can drop these 2.
>
> > +
> > + interrupt_controller:
>
> s/_/-/
>
> > + description: Interrupt controller child node.
>
> type: object
>
> And then need to describe all the properties under it too.
Agreed, will describe properties under this child node.
>
> > +
> > + bus-range:
> > + description: Range of bus numbers associated with this controller.
>
> Can drop.
>
> > +
> > +required:
> > + - compatible
> > + - "#address-cells"
> > + - "#size-cells"
> > + - reg
> > + - reg-names
> > + - "#interrupt-cells"
> > + - interrupts
> > + - interrupt-parent
> > + - interrupt-map
> > + - interrupt-map-mask
> > + - ranges
> > + - bus-range
> > + - msi-map
>
> interrupt-controller node not required?
Yes required, will add.
>
> You can drop all the standard properties required in pci-bus.yaml (it's in
> dtschema repo).
Agreed, will drop.
>
> > +
> > +additionalProperties: false
>
> This will need to be 'unevaluatedProperties: false'
Thanks Rob, will fix these in next patch.
Regards,
Bharat
>
> > +
> > +examples:
> > + - |
> > +
> > + versal {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + cpm_pcie: pci@fca10000 {
>
> pcie@...
>
> > + compatible = "xlnx,versal-cpm-host-1.00";
> > + #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
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
2020-05-22 15:51 ` Marc Zyngier
@ 2020-05-27 17:08 ` Bharat Kumar Gogada
0 siblings, 0 replies; 11+ messages in thread
From: Bharat Kumar Gogada @ 2020-05-27 17:08 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-pci, linux-kernel, lorenzo.pieralisi, bhelgaas, robh,
Ravikiran Gummaluri
> Bharat,
>
> On 2020-05-07 12:58, 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.
> >
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > ---
> > drivers/pci/controller/Kconfig | 9 +
> > drivers/pci/controller/Makefile | 1 +
> > drivers/pci/controller/pcie-xilinx-cpm.c | 506
> > +++++++++++++++++++++++++++++++
> > 3 files changed, 516 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..ca0ae24 100644
> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -81,6 +81,15 @@ 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. The driver supports
> > + MSI/MSI-X interrupts using GICv3 ITS feature.
>
> I don't think this last sentense makes any sense here. We usually don't
> mention things that the driver *doesn't* implement.
Agreed, will remove these details.
>
> > +
> > 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..e8c0aa7
> > --- /dev/null
> > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> > @@ -0,0 +1,506 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge
> > + *
> > + * (C) Copyright 2019 - 2020, Xilinx, Inc.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/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 BIT(0)
> > +#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
> > +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
> > +#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
> > +#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
> > +#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
> > +#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
> > +#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
> > +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
> > +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
> > +#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
> > +#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
> > +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
> > +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
> > +#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
> > +#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
> > +#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
>
> I assume that this is the logical OR of all the XILINX_CPM_PCIE_INTR_* bits.
> Please express it as such.
Agreed, will change this.
>
> > +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF
> > +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16)
> > +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT BIT(4)
> > +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
> > +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
> > +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
>
> So we have two definitions for bit 17... Given that nothing uses the MSI
> version, I assume it is useless...
>
> > +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
>
> Please group all the XILINX_CPM_PCIE_INTR_* together.
>
Agreed, will change this.
> > +#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
> > + * @leg_domain: Legacy IRQ domain pointer
>
> leg, arm, thumb, limb... Given that oyu use the 'intx' description all over the
> driver, please be consistent and name this intx_domain.
>
> > + * @cfg: Holds mappings of config space window
> > + * @irq_misc: Legacy and error interrupt number
>
> Let's face it, this is the *only* interrupt this thing as, so the 'misc'
> is supperfluous.
>
> > + * @leg_mask_lock: lock for legacy interrupts */ struct
> > +xilinx_cpm_pcie_port {
> > + void __iomem *reg_base;
> > + void __iomem *cpm_base;
> > + struct device *dev;
> > + struct irq_domain *leg_domain;
> > + struct pci_config_window *cfg;
> > + int irq_misc;
> > + raw_spinlock_t leg_mask_lock;
> > +};
> > +
> > +static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32
> > reg)
> > +{
> > + return readl(port->reg_base + reg);
>
> There is no need for enforced ordering with non-device writes, so you can
> turn this into readl_relaxed. You can also drop the inline, as the compiler
> will do the right thing for you.
>
Agreed, will change to readl_relaxed.
> > +}
> > +
> > +static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
> > + u32 val, u32 reg)
> > +{
> > + writel(val, port->reg_base + reg);
>
> Same thing.
>
> > +}
> > +
> > +static inline 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) ? 1 : 0;
>
> If you are making the return value a bool, you might as well return
> true/false. But that's a cumbersome way to write:
>
> 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 irq_desc *desc = irq_to_desc(data->irq);
> > + struct xilinx_cpm_pcie_port *port;
> > + unsigned long flags;
> > + u32 mask;
> > + u32 val;
> > +
> > + port = irq_desc_get_chip_data(desc);
>
> port = irq_data_get_irq_chip_data(data);
>
> You should never have to get the irq_desc (and using irq_to_desc() is a
> prettyodd way to get it when you already have the irq_data).
>
> > + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
>
> Also known as BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT).
>
> > + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags); }
> > +
> > +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data) {
> > + struct irq_desc *desc = irq_to_desc(data->irq);
> > + struct xilinx_cpm_pcie_port *port;
> > + unsigned long flags;
> > + u32 mask;
> > + u32 val;
> > +
> > + port = irq_desc_get_chip_data(desc);
> > + mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> > + raw_spin_lock_irqsave(&port->leg_mask_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->leg_mask_lock, flags); }
> > +
> > +static struct irq_chip xilinx_cpm_leg_irq_chip = {
> > + .name = "xilinx_cpm_pcie:legacy",
>
> "INTx" is a good enough description.
>
> > + .irq_enable = xilinx_cpm_unmask_leg_irq,
> > + .irq_disable = xilinx_cpm_mask_leg_irq,
> > + .irq_mask = xilinx_cpm_mask_leg_irq,
> > + .irq_unmask = xilinx_cpm_unmask_leg_irq,
>
> This makes no sense. If enable/disable have the same implementation as
> unmask/mask, then enable/disable is pretty useless. Please drop them.
>
Agreed, will drop these methods.
> > +};
> > +
> > +/**
> > + * 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,
> > +};
> > +
> > +/**
> > + * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
> > + * @irq: IRQ number
> > + * @data: PCIe port information
> > + *
> > + * Return: IRQ_HANDLED on success and IRQ_NONE on failure */ static
> > +irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data) {
> > + struct xilinx_cpm_pcie_port *port = data;
> > + struct device *dev = port->dev;
> > + u32 val, mask, status, bit;
> > + unsigned long intr_val;
> > +
> > + /* Read interrupt decode and mask registers */
> > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> > + mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> > +
> > + status = val & mask;
> > + if (!status)
> > + return IRQ_NONE;
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
> > + dev_warn(dev, "Link Down\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> > + dev_info(dev, "Hot reset\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
> > + dev_warn(dev, "ECAM access timeout\n");
>
> There is a certain sense of repetition here. It really begs the question of
> *why* you want to take these interrupts if all you do is spam the console
> with warnings, not taking any action to potentially remedy the problem.
>
We use this information if user faces an issue to track down possible cause.
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
> > + dev_warn(dev, "Correctable error message\n");
> > + cpm_pcie_clear_err_interrupts(port);
> > + }
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
> > + dev_warn(dev, "Non fatal error message\n");
> > + cpm_pcie_clear_err_interrupts(port);
> > + }
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_FATAL) {
> > + dev_warn(dev, "Fatal error message\n");
> > + cpm_pcie_clear_err_interrupts(port);
> > + }
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_INTX) {
> > + /* Handle INTx Interrupt */
> > + intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
> > + intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
> > +
> > + for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
> > + generic_handle_irq(irq_find_mapping(port-
> >leg_domain,
> > + bit));
>
> That's a firm no. We don't demux chained handlers in an interrupt handler.
> It may work for now, but it will eventually break. And to be clear, this whole
> function needs to die. By the look of it, this PCie controller implements
> *TWO* interrupt multiplexers:
>
> - one that muxes all the ILINX_CPM_PCIE_INTR_* events onto a single top-
> level IRQ
> - another one that muxes all INTX lines onto XILINX_CPM_PCIE_INTR_INTX
>
> Please implement the whole thing as described above.
Agreed, thanks for resolving this.
>
> > + }
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
> > + dev_warn(dev, "Slave unsupported request\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
> > + dev_warn(dev, "Slave unexpected completion\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
> > + dev_warn(dev, "Slave completion timeout\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
> > + dev_warn(dev, "Slave Error Poison\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
> > + dev_warn(dev, "Slave Completer Abort\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
> > + dev_warn(dev, "Slave Illegal Burst\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
> > + dev_warn(dev, "Master decode error\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
> > + dev_warn(dev, "Master slave error\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
> > + dev_warn(dev, "PCIe ECAM access timeout\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
> > + dev_warn(dev, "ECAM poisoned completion received\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
> > + dev_warn(dev, "PME_TO_ACK message received\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
> > + dev_warn(dev, "PM_PME message received\n");
> > +
> > + if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
> > + dev_warn(dev, "PCIe completion timeout received\n");
>
> I'm pretty sure there is a slightly better way to write this...
>
> > +
> > + /* Clear the Interrupt Decode register */
> > + pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
> > +
> > + /*
> > + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
> > + * CPM SLCR block.
> > + */
> > + val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> > + if (val)
> > + writel(val, port->cpm_base +
> XILINX_CPM_PCIE_MISC_IR_STATUS);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/**
> > + * 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->leg_domain = irq_domain_add_linear(pcie_intc_node,
> > PCI_NUM_INTX,
> > + &intx_domain_ops,
> > + port);
> > + of_node_put(pcie_intc_node);
> > + if (!port->leg_domain) {
> > + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> > + return -ENOMEM;
> > + }
> > +
> > + raw_spin_lock_init(&port->leg_mask_lock);
> > + 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);
> > +
> > + /* Enable all interrupts */
> > + pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
> > + XILINX_CPM_PCIE_REG_IMR);
> > + pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
> > + XILINX_CPM_PCIE_REG_IDRN_MASK);
>
> No. We don't enable interrupts randomly. They need a handler registered
> with the core IRq subsystem *first*, which is why this needs to be hooked in
> as a proper irqchip, and each event handled individualy.
>
> > +
> > + /*
> > + * 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);
> > +}
> > +
> > +static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port
> > *port)
> > +{
> > + struct device *dev = port->dev;
> > + struct platform_device *pdev = to_platform_device(dev);
> > + int err;
> > +
> > + port->irq_misc = platform_get_irq(pdev, 0);
> > + if (port->irq_misc <= 0) {
>
> If 0 is an error, how do you distinguish it from the non-error case?
>
> > + dev_err(dev, "Unable to find misc IRQ line\n");
> > + return port->irq_misc;
> > + }
> > +
> > + err = devm_request_irq(dev, port->irq_misc,
> > + xilinx_cpm_pcie_intr_handler,
> > + IRQF_SHARED | IRQF_NO_THREAD,
> > + "xilinx-pcie", port);
> > + if (err) {
> > + dev_err(dev, "unable to request misc IRQ line %d\n",
> > + port->irq_misc);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * 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;
> > + int err;
> > +
> > + 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;
> > +
> > + port->cpm_base = devm_platform_ioremap_resource_byname(pdev,
> > + "cpm_slcr");
> > + if (IS_ERR(port->cpm_base))
> > + return PTR_ERR(port->cpm_base);
> > +
> > + err = xilinx_cpm_request_misc_irq(port);
> > + if (err)
> > + return err;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * 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_parse_dt(port, bus_range);
> > + if (err) {
> > + dev_err(dev, "Parsing DT failed\n");
> > + return err;
> > + }
> > +
> > + xilinx_cpm_pcie_init_port(port);
> > +
> > + err = xilinx_cpm_pcie_init_irq_domain(port);
> > + if (err) {
> > + dev_err(dev, "Failed creating IRQ Domain\n");
> > + return err;
> > + }
> > +
> > + 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) {
> > + irq_domain_remove(port->leg_domain);
> > + devm_free_irq(dev, port->irq_misc, port);
>
> Why calling dem_free_irq()? The whole point of using devm* is not to have
> to manage the error handling.
>
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +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);
>
> I don't think this driver is in a state where it can be merged. Given that we're
> already at v7 and to save everyone a bit of time, I've written the patch that
> implements the above comments. It compiles, but is probably broken.
> Hopefully you can test it and figure things out.
Hi Marc,
Thanks for your time and inputs.
Regards,
Bharat
>
> M.
>
> From 8b5d158374e59edf26e61c512eeb00ca7c9d891d Mon Sep 17 00:00:00
> 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Fri, 22 May 2020 16:33:32 +0100
> Subject: [PATCH] PCI: xilinx-cpm: Revamp irq handling
>
> The xilinx-cpm driver missuses the IRQ layer in creative ways.
> It implements a chained interrupt demultiplexer inside a normal handler,
> enables interrupts randomly, and could overall do with a good cleanup.
>
> Instead, implement the IRQ support as two nested chained irqchips, the
> outer one dealing with all the PCIe events, the inner one namaging the INTx
> signals.
>
> Additionally, the whole driver is cleanup-up so that it can make a bit more
> sense to me. YMMV.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/pci/controller/Kconfig | 3 +-
> drivers/pci/controller/pcie-xilinx-cpm.c | 427 ++++++++++++++---------
> 2 files changed, 263 insertions(+), 167 deletions(-)
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 1e0f63865775..d0abd671a7b6 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -87,8 +87,7 @@ config PCIE_XILINX_CPM
> select PCI_HOST_COMMON
> help
> Say 'Y' here if you want kernel support for the
> - Xilinx Versal CPM host bridge. The driver supports
> - MSI/MSI-X interrupts using GICv3 ITS feature.
> + Xilinx Versal CPM host bridge.
>
> config PCI_XGENE
> bool "X-Gene PCIe controller"
> diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c
> b/drivers/pci/controller/pcie-xilinx-cpm.c
> index e8c0aa757f72..32ed9e7e2676 100644
> --- a/drivers/pci/controller/pcie-xilinx-cpm.c
> +++ b/drivers/pci/controller/pcie-xilinx-cpm.c
> @@ -5,8 +5,11 @@
> * (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>
> @@ -33,30 +36,55 @@
> #define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1)
>
> /* Interrupt registers definitions */
> -#define XILINX_CPM_PCIE_INTR_LINK_DOWN BIT(0)
> -#define XILINX_CPM_PCIE_INTR_HOT_RESET BIT(3)
> -#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT BIT(8)
> -#define XILINX_CPM_PCIE_INTR_CORRECTABLE BIT(9)
> -#define XILINX_CPM_PCIE_INTR_NONFATAL BIT(10)
> -#define XILINX_CPM_PCIE_INTR_FATAL BIT(11)
> -#define XILINX_CPM_PCIE_INTR_INTX BIT(16)
> -#define XILINX_CPM_PCIE_INTR_MSI BIT(17)
> -#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP BIT(20)
> -#define XILINX_CPM_PCIE_INTR_SLV_UNEXP BIT(21)
> -#define XILINX_CPM_PCIE_INTR_SLV_COMPL BIT(22)
> -#define XILINX_CPM_PCIE_INTR_SLV_ERRP BIT(23)
> -#define XILINX_CPM_PCIE_INTR_SLV_CMPABT BIT(24)
> -#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR BIT(25)
> -#define XILINX_CPM_PCIE_INTR_MST_DECERR BIT(26)
> -#define XILINX_CPM_PCIE_INTR_MST_SLVERR BIT(27)
> -#define XILINX_CPM_PCIE_IMR_ALL_MASK 0x1FF39FF9
> +#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_INTR_CFG_PCIE_TIMEOUT BIT(4)
> -#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON BIT(12)
> -#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD BIT(15)
> -#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD BIT(17)
> -#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT BIT(28)
> #define XILINX_CPM_PCIE_IDRN_SHIFT 16
>
> /* Root Port Error FIFO Read Register definitions */ @@ -75,36 +103,37 @@
> * @reg_base: Bridge Register Base
> * @cpm_base: CPM System Level Control and Status Register(SLCR) Base
> * @dev: Device pointer
> - * @leg_domain: Legacy IRQ domain pointer
> + * @intx_domain: Legacy IRQ domain pointer
> * @cfg: Holds mappings of config space window
> - * @irq_misc: Legacy and error interrupt number
> - * @leg_mask_lock: lock for legacy interrupts
> + * @irq: INTx and 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 *leg_domain;
> - struct pci_config_window *cfg;
> - int irq_misc;
> - raw_spinlock_t leg_mask_lock;
> + 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 irq;
> + raw_spinlock_t lock;
> };
>
> static inline u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg)
> {
> - return readl(port->reg_base + reg);
> + return readl_relaxed(port->reg_base + reg);
> }
>
> static inline void pcie_write(struct xilinx_cpm_pcie_port *port,
> u32 val, u32 reg)
> {
> - writel(val, port->reg_base + reg);
> + writel_relaxed(val, port->reg_base + reg);
> }
>
> static inline 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) ? 1 : 0;
> + XILINX_CPM_PCIE_REG_PSCR_LNKUP);
> }
>
> /**
> @@ -125,44 +154,56 @@ static void cpm_pcie_clear_err_interrupts(struct
> xilinx_cpm_pcie_port *port)
>
> static void xilinx_cpm_mask_leg_irq(struct irq_data *data)
> {
> - struct irq_desc *desc = irq_to_desc(data->irq);
> - struct xilinx_cpm_pcie_port *port;
> + struct xilinx_cpm_pcie_port *port =
> irq_data_get_irq_chip_data(data);
> unsigned long flags;
> u32 mask;
> u32 val;
>
> - port = irq_desc_get_chip_data(desc);
> - mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> - raw_spin_lock_irqsave(&port->leg_mask_lock, flags);
> + 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->leg_mask_lock, flags);
> + raw_spin_unlock_irqrestore(&port->lock, flags);
> }
>
> static void xilinx_cpm_unmask_leg_irq(struct irq_data *data)
> {
> - struct irq_desc *desc = irq_to_desc(data->irq);
> - struct xilinx_cpm_pcie_port *port;
> + struct xilinx_cpm_pcie_port *port =
> irq_data_get_irq_chip_data(data);
> unsigned long flags;
> u32 mask;
> u32 val;
>
> - port = irq_desc_get_chip_data(desc);
> - mask = (1 << data->hwirq) << XILINX_CPM_PCIE_IDRN_SHIFT;
> - raw_spin_lock_irqsave(&port->leg_mask_lock, flags);
> + 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->leg_mask_lock, flags);
> + raw_spin_unlock_irqrestore(&port->lock, flags);
> }
>
> static struct irq_chip xilinx_cpm_leg_irq_chip = {
> - .name = "xilinx_cpm_pcie:legacy",
> - .irq_enable = xilinx_cpm_unmask_leg_irq,
> - .irq_disable = xilinx_cpm_mask_leg_irq,
> - .irq_mask = xilinx_cpm_mask_leg_irq,
> - .irq_unmask = xilinx_cpm_unmask_leg_irq,
> + .name = "INTx",
> + .irq_mask = xilinx_cpm_mask_leg_irq,
> + .irq_unmask = xilinx_cpm_unmask_leg_irq,
> };
>
> +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);
> +}
> +
> /**
> * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark IRQ as
> valid
> * @domain: IRQ domain
> @@ -187,111 +228,130 @@ static const struct irq_domain_ops
> intx_domain_ops = {
> .map = xilinx_cpm_pcie_intx_map,
> };
>
> -/**
> - * xilinx_cpm_pcie_intr_handler - Interrupt Service Handler
> - * @irq: IRQ number
> - * @data: PCIe port information
> - *
> - * Return: IRQ_HANDLED on success and IRQ_NONE on failure
> - */
> -static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
> +static void xilinx_cpm_mask_event_irq(struct irq_data *d)
> {
> - struct xilinx_cpm_pcie_port *port = data;
> - struct device *dev = port->dev;
> - u32 val, mask, status, bit;
> - unsigned long intr_val;
> -
> - /* Read interrupt decode and mask registers */
> - val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> - mask = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> -
> - status = val & mask;
> - if (!status)
> - return IRQ_NONE;
> -
> - if (status & XILINX_CPM_PCIE_INTR_LINK_DOWN)
> - dev_warn(dev, "Link Down\n");
> -
> - if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> - dev_info(dev, "Hot reset\n");
> -
> - if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
> - dev_warn(dev, "ECAM access timeout\n");
> -
> - if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
> - dev_warn(dev, "Correctable error message\n");
> - cpm_pcie_clear_err_interrupts(port);
> - }
> -
> - if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
> - dev_warn(dev, "Non fatal error message\n");
> - cpm_pcie_clear_err_interrupts(port);
> - }
> -
> - if (status & XILINX_CPM_PCIE_INTR_FATAL) {
> - dev_warn(dev, "Fatal error message\n");
> - cpm_pcie_clear_err_interrupts(port);
> - }
> -
> - if (status & XILINX_CPM_PCIE_INTR_INTX) {
> - /* Handle INTx Interrupt */
> - intr_val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN);
> - intr_val = intr_val >> XILINX_CPM_PCIE_IDRN_SHIFT;
> -
> - for_each_set_bit(bit, &intr_val, PCI_NUM_INTX)
> - generic_handle_irq(irq_find_mapping(port-
> >leg_domain,
> - bit));
> - }
> -
> - if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
> - dev_warn(dev, "Slave unsupported request\n");
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
> - dev_warn(dev, "Slave unexpected completion\n");
> + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d);
> + u32 val;
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
> - dev_warn(dev, "Slave completion timeout\n");
> + raw_spin_lock(&port->lock);
> + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> + val &= ~d->hwirq;
> + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
> + raw_spin_unlock(&port->lock);
> +}
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
> - dev_warn(dev, "Slave Error Poison\n");
> +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;
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
> - dev_warn(dev, "Slave Completer Abort\n");
> + raw_spin_lock(&port->lock);
> + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
> + val |= d->hwirq;
> + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR);
> + raw_spin_unlock(&port->lock);
> +}
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
> - dev_warn(dev, "Slave Illegal Burst\n");
> +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,
> +};
>
> - if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
> - dev_warn(dev, "Master decode error\n");
> +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);
>
> - if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
> - dev_warn(dev, "Master slave error\n");
> + return 0;
> +}
>
> - if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
> - dev_warn(dev, "PCIe ECAM access timeout\n");
> +static const struct irq_domain_ops event_domain_ops = {
> + .map = xilinx_cpm_pcie_event_map,
> +};
>
> - if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
> - dev_warn(dev, "ECAM poisoned completion received\n");
> +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;
>
> - if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
> - dev_warn(dev, "PME_TO_ACK message received\n");
> + chained_irq_enter(chip, desc);
>
> - if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
> - dev_warn(dev, "PM_PME message received\n");
> + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR);
> + val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR);
>
> - if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
> - dev_warn(dev, "PCIe completion timeout received\n");
> + for_each_set_bit(i, &val, 32)
> + generic_handle_irq(irq_find_mapping(port->cpm_domain,
> i));
>
> /* Clear the Interrupt Decode register */
> - pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
> + pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR);
>
> /*
> * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to
> * CPM SLCR block.
> */
> - val = readl(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS);
> + val = readl_relaxed(port->cpm_base +
> XILINX_CPM_PCIE_MISC_IR_STATUS);
> if (val)
> - writel(val, port->cpm_base +
> XILINX_CPM_PCIE_MISC_IR_STATUS);
> + 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 interrupt\n");
> + }
>
> return IRQ_HANDLED;
> }
> @@ -315,17 +375,41 @@ static int xilinx_cpm_pcie_init_irq_domain(struct
> xilinx_cpm_pcie_port *port)
> return -EINVAL;
> }
>
> - port->leg_domain = irq_domain_add_linear(pcie_intc_node,
> PCI_NUM_INTX,
> + 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);
> - if (!port->leg_domain) {
> - dev_err(dev, "Failed to get a INTx IRQ domain\n");
> - return -ENOMEM;
> - }
> + raw_spin_lock_init(&port->lock);
>
> - raw_spin_lock_init(&port->leg_mask_lock);
> return 0;
> +
> +out:
> + of_node_put(pcie_intc_node);
> + 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;
> + }
> +
> + dev_err(dev, "Failed to allocate IRQ domains\n");
> + return -ENOMEM;
> }
>
> /**
> @@ -348,12 +432,6 @@ static void xilinx_cpm_pcie_init_port(struct
> xilinx_cpm_pcie_port *port)
> XILINX_CPM_PCIE_IMR_ALL_MASK,
> XILINX_CPM_PCIE_REG_IDR);
>
> - /* Enable all interrupts */
> - pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK,
> - XILINX_CPM_PCIE_REG_IMR);
> - pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK,
> - XILINX_CPM_PCIE_REG_IDRN_MASK);
> -
> /*
> * XILINX_CPM_PCIE_MISC_IR_ENABLE register is mapped to
> * CPM SLCR block.
> @@ -366,28 +444,45 @@ static void xilinx_cpm_pcie_init_port(struct
> xilinx_cpm_pcie_port *port)
> XILINX_CPM_PCIE_REG_RPSC);
> }
>
> -static int xilinx_cpm_request_misc_irq(struct xilinx_cpm_pcie_port
> *port)
> +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 err;
> + int i, irq;
>
> - port->irq_misc = platform_get_irq(pdev, 0);
> - if (port->irq_misc <= 0) {
> - dev_err(dev, "Unable to find misc IRQ line\n");
> - return port->irq_misc;
> + port->irq = platform_get_irq(pdev, 0);
> + if (port->irq < 0) {
> + dev_err(dev, "Unable to find IRQ line\n");
> + return port->irq;
> }
>
> - err = devm_request_irq(dev, port->irq_misc,
> - xilinx_cpm_pcie_intr_handler,
> - IRQF_SHARED | IRQF_NO_THREAD,
> - "xilinx-pcie", port);
> - if (err) {
> - dev_err(dev, "unable to request misc IRQ line %d\n",
> - port->irq_misc);
> - return err;
> + 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 (WARN_ON(irq <= 0))
> + return -ENXIO;
> +
> + err = devm_request_irq(dev, irq,
> xilinx_cpm_pcie_intr_handler,
> + 0, intr_cause[i].sym, port);
> + if (WARN_ON(err))
> + return err;
> }
>
> + irq = irq_create_mapping(port->cpm_domain,
> XILINX_CPM_PCIE_INTR_INTX);
> + if (WARN_ON(irq <= 0))
> + return -ENXIO;
> +
> + /* Plug the INTx chained handler */
> + irq_set_chained_handler_and_data(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;
> }
>
> @@ -422,7 +517,7 @@ static int xilinx_cpm_pcie_parse_dt(struct
> xilinx_cpm_pcie_port *port,
> if (IS_ERR(port->cpm_base))
> return PTR_ERR(port->cpm_base);
>
> - err = xilinx_cpm_request_misc_irq(port);
> + err = xilinx_cpm_setup_irq(port);
> if (err)
> return err;
>
> @@ -481,8 +576,10 @@ static int xilinx_cpm_pcie_probe(struct
> platform_device *pdev)
>
> err = pci_host_probe(bridge);
> if (err < 0) {
> - irq_domain_remove(port->leg_domain);
> - devm_free_irq(dev, port->irq_misc, port);
> + if (port->intx_domain)
> + irq_domain_remove(port->intx_domain);
> + if (port->cpm_domain)
> + irq_domain_remove(port->cpm_domain);
> return err;
> }
>
> --
> 2.26.2
>
>
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-27 17:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 11:58 [PATCH v7 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada
2020-05-07 11:58 ` [PATCH v7 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada
2020-05-15 5:38 ` Bharat Kumar Gogada
2020-05-20 22:20 ` Rob Herring
2020-05-26 12:50 ` Bharat Kumar Gogada
2020-05-07 11:58 ` [PATCH v7 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada
2020-05-07 19:49 ` Bjorn Helgaas
2020-05-11 10:47 ` Bharat Kumar Gogada
2020-05-20 22:36 ` Rob Herring
2020-05-22 15:51 ` Marc Zyngier
2020-05-27 17:08 ` Bharat Kumar Gogada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).