linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Adding support for versal CPM as Root Port driver.
@ 2019-12-20 11:41 Bharat Kumar Gogada
  2019-12-20 11:41 ` [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller Bharat Kumar Gogada
  2019-12-20 11:41 ` [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver Bharat Kumar Gogada
  0 siblings, 2 replies; 9+ messages in thread
From: Bharat Kumar Gogada @ 2019-12-20 11:41 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: bhelgaas, 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 MISC interrupt line.

Bharat Kumar Gogada (2):
  PCI: Versal CPM: Add device tree binding forversal CPM host controller
  PCI: Versal CPM: Add support for Versal CPM Root Port driver

 .../devicetree/bindings/pci/xilinx-versal-cpm.txt  |  66 +++
 drivers/pci/controller/Kconfig                     |   8 +
 drivers/pci/controller/Makefile                    |   1 +
 drivers/pci/controller/pcie-xilinx-cpm.c           | 511 +++++++++++++++++++++
 4 files changed, 586 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
 create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller
  2019-12-20 11:41 [PATCH 0/2] Adding support for versal CPM as Root Port driver Bharat Kumar Gogada
@ 2019-12-20 11:41 ` Bharat Kumar Gogada
  2020-02-25 10:58   ` Lorenzo Pieralisi
  2019-12-20 11:41 ` [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver Bharat Kumar Gogada
  1 sibling, 1 reply; 9+ messages in thread
From: Bharat Kumar Gogada @ 2019-12-20 11:41 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: bhelgaas, rgummal, Bharat Kumar Gogada

Adding device tree binding documentation for versal CPM Root Port driver.

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

diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
new file mode 100644
index 0000000..35f8556
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
@@ -0,0 +1,66 @@
+* Xilinx Versal CPM DMA Root Port Bridge DT description
+
+Required properties:
+- #address-cells: Address representation for root ports, set to <3>
+- #size-cells: Size representation for root ports, set to <2>
+- #interrupt-cells: specifies the number of cells needed to encode an
+	interrupt source. The value must be 1.
+- compatible: Should contain "xlnx,versal-cpm-host-1.00"
+- reg: Should contain configuration space (includes bridge registers) and
+	CPM system level control and status registers, and length
+- reg-names: Must include the following entries:
+	"cfg": configuration space region and bridge registers
+	"cpm_slcr": CPM system level control and status registers
+- interrupts: Should contain AXI PCIe interrupt
+- interrupt-map-mask,
+  interrupt-map: standard PCI properties to define the mapping of the
+	PCI interface to interrupt numbers.
+- ranges: ranges for the PCI memory regions (I/O space region is not
+	supported by hardware)
+	Please refer to the standard PCI bus binding document for a more
+	detailed explanation
+- msi-map: Maps a Requester ID to an MSI controller and associated MSI
+	sideband data
+- interrupt-names: Must include the following entries:
+	"misc": interrupt asserted when legacy or error interrupt is received
+
+Interrupt controller child node
++++++++++++++++++++++++++++++++
+Required properties:
+- interrupt-controller: identifies the node as an interrupt controller
+- #address-cells: specifies the number of cells needed to encode an
+	address. The value must be 0.
+- #interrupt-cells: specifies the number of cells needed to encode an
+	interrupt source. The value must be 1.
+
+
+Refer to the following binding document for more detailed description on
+the use of 'msi-map':
+	 Documentation/devicetree/bindings/pci/pci-msi.txt
+
+Example:
+	pci@fca10000 {
+		#address-cells = <3>;
+		#interrupt-cells = <1>;
+		#size-cells = <2>;
+		compatible = "xlnx,versal-cpm-host-1.00";
+		interrupt-map = <0 0 0 1 &pcie_intc_0 1>,
+				<0 0 0 2 &pcie_intc_0 2>,
+				<0 0 0 3 &pcie_intc_0 3>,
+				<0 0 0 4 &pcie_intc_0 4>;
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-parent = <&gic>;
+		interrupt-names = "misc";
+		interrupts = <0 72 4>;
+		ranges = <0x02000000 0x00000000 0xE0000000 0x0 0xE0000000 0x00000000 0x10000000>,
+			 <0x43000000 0x00000080 0x00000000 0x00000080 0x00000000 0x00000000 0x80000000>;
+		msi-map = <0x0 &its_gic 0x0 0x10000>;
+		reg = <0x6 0x00000000 0x0 0x1000000>,
+		      <0x0 0xFCA10000 0x0 0x1000>;
+		reg-names = "cfg", "cpm_slcr";
+		pcie_intc_0: pci-interrupt-controller {
+			#address-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-controller ;
+		};
+	};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver
  2019-12-20 11:41 [PATCH 0/2] Adding support for versal CPM as Root Port driver Bharat Kumar Gogada
  2019-12-20 11:41 ` [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller Bharat Kumar Gogada
@ 2019-12-20 11:41 ` Bharat Kumar Gogada
  2019-12-20 14:58   ` Bjorn Helgaas
  2020-01-05 18:49   ` Dan Carpenter
  1 sibling, 2 replies; 9+ messages in thread
From: Bharat Kumar Gogada @ 2019-12-20 11:41 UTC (permalink / raw)
  To: linux-pci, linux-kernel; +Cc: bhelgaas, 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 MISC interrupt line.

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

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index c77069c..eca496d 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -81,6 +81,14 @@ config PCIE_XILINX
 	  Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
 	  Host Bridge driver.
 
+config PCIE_XILINX_CPM
+	bool "Xilinx Versal CPM host bridge support"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	help
+	  Say 'Y' here if you want kernel to enable support the
+	  Xilinx Versal CPM host Bridge driver.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 3d4f597..6c936e9 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..9c0852a
--- /dev/null
+++ b/drivers/pci/controller/pcie-xilinx-cpm.c
@@ -0,0 +1,511 @@
+// 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 "../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)
+
+/* ECAM definitions */
+#define ECAM_BUS_NUM_SHIFT		20
+#define ECAM_DEV_NUM_SHIFT		12
+
+#define INTX_NUM                        4
+
+/**
+ * struct xilinx_cpm_pcie_port - PCIe port information
+ * @reg_base: Bridge Register Base
+ * @cpm_base: CPM SLCR Register Base
+ * @irq: Interrupt number
+ * @root_busno: Root Bus number
+ * @dev: Device pointer
+ * @leg_domain: Legacy IRQ domain pointer
+ * @irq_misc: Legacy and error interrupt number
+ */
+struct xilinx_cpm_pcie_port {
+	void __iomem *reg_base;
+	void __iomem *cpm_base;
+	u32 irq;
+	u8 root_busno;
+	struct device *dev;
+	struct irq_domain *leg_domain;
+	int irq_misc;
+};
+
+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_is_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);
+	}
+}
+
+/**
+ * xilinx_cpm_pcie_valid_device - Check if a valid device is present on bus
+ * @bus: PCI Bus structure
+ * @devfn: device/function
+ *
+ * Return: 'true' on success and 'false' if invalid device is found
+ */
+static bool xilinx_cpm_pcie_valid_device(struct pci_bus *bus,
+					 unsigned int devfn)
+{
+	struct xilinx_cpm_pcie_port *port = bus->sysdata;
+
+	/* Check if link is up when trying to access downstream ports */
+	if (bus->number != port->root_busno)
+		if (!cpm_pcie_link_is_up(port))
+			return false;
+
+	/* Only one device down on each root port */
+	if (bus->number == port->root_busno && devfn > 0)
+		return false;
+
+	return true;
+}
+
+/**
+ * xilinx_cpm_pcie_map_bus - Get configuration base
+ * @bus: PCI Bus structure
+ * @devfn: Device/function
+ * @where: Offset from base
+ *
+ * Return: Base address of the configuration space needed to be
+ *	   accessed.
+ */
+static void __iomem *xilinx_cpm_pcie_map_bus(struct pci_bus *bus,
+					     unsigned int devfn, int where)
+{
+	struct xilinx_cpm_pcie_port *port = bus->sysdata;
+	int relbus;
+
+	if (!xilinx_cpm_pcie_valid_device(bus, devfn))
+		return NULL;
+
+	relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
+		 (devfn << ECAM_DEV_NUM_SHIFT);
+
+	return port->reg_base + relbus + where;
+}
+
+/* PCIe operations */
+static struct pci_ops xilinx_cpm_pcie_ops = {
+	.map_bus = xilinx_cpm_pcie_map_bus,
+	.read	= pci_generic_config_read,
+	.write	= pci_generic_config_write,
+};
+
+/**
+ * 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, &dummy_irq_chip, handle_simple_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,
+	.xlate = pci_irqd_intx_xlate,
+};
+
+/**
+ * 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 =
+				(struct xilinx_cpm_pcie_port *)data;
+	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(port->dev, "Link Down\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
+		dev_info(port->dev, "Hot reset\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_CFG_TIMEOUT)
+		dev_warn(port->dev, "ECAM access timeout\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_CORRECTABLE) {
+		dev_warn(port->dev, "Correctable error message\n");
+		cpm_pcie_clear_err_interrupts(port);
+	}
+
+	if (status & XILINX_CPM_PCIE_INTR_NONFATAL) {
+		dev_warn(port->dev, "Non fatal error message\n");
+		cpm_pcie_clear_err_interrupts(port);
+	}
+
+	if (status & XILINX_CPM_PCIE_INTR_FATAL) {
+		dev_warn(port->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, INTX_NUM)
+			generic_handle_irq(irq_find_mapping(port->leg_domain,
+							    bit));
+	}
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_UNSUPP)
+		dev_warn(port->dev, "Slave unsupported request\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_UNEXP)
+		dev_warn(port->dev, "Slave unexpected completion\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_COMPL)
+		dev_warn(port->dev, "Slave completion timeout\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_ERRP)
+		dev_warn(port->dev, "Slave Error Poison\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_CMPABT)
+		dev_warn(port->dev, "Slave Completer Abort\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_ILLBUR)
+		dev_warn(port->dev, "Slave Illegal Burst\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_MST_DECERR)
+		dev_warn(port->dev, "Master decode error\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_MST_SLVERR)
+		dev_warn(port->dev, "Master slave error\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT)
+		dev_warn(port->dev, "PCIe ECAM access timeout\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_CFG_ERR_POISON)
+		dev_warn(port->dev, "ECAM poisoned completion received\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD)
+		dev_warn(port->dev, "PME_TO_ACK message received\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_PM_PME_RCVD)
+		dev_warn(port->dev, "PM_PME message received\n");
+
+	if (status & XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT)
+		dev_warn(port->dev, "PCIe completion timeout received\n");
+
+	/* Clear the Interrupt Decode register */
+	pcie_write(port, status, XILINX_CPM_PCIE_REG_IDR);
+	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 PTR_ERR(pcie_intc_node);
+	}
+
+	port->leg_domain = irq_domain_add_linear(pcie_intc_node, INTX_NUM,
+						 &intx_domain_ops,
+						 port);
+	if (!port->leg_domain) {
+		dev_err(dev, "Failed to get a INTx IRQ domain\n");
+		return PTR_ERR(port->leg_domain);
+	}
+
+	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_is_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);
+
+	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_byname(pdev, "misc");
+	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
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port)
+{
+	struct device *dev = port->dev;
+	struct resource *res;
+	int err;
+	struct platform_device *pdev = to_platform_device(dev);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+	port->reg_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(port->reg_base))
+		return PTR_ERR(port->reg_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+					   "cpm_slcr");
+	port->cpm_base = devm_ioremap_resource(dev, res);
+	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_bus *bus;
+	struct pci_bus *child;
+	struct pci_host_bridge *bridge;
+	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 = xilinx_cpm_pcie_parse_dt(port);
+	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;
+	}
+
+	err = pci_parse_request_of_pci_ranges(dev, &bridge->windows,
+					      &bridge->dma_ranges, NULL);
+	if (err) {
+		dev_err(dev, "Getting bridge resources failed\n");
+		return err;
+	}
+
+	bridge->dev.parent = dev;
+	bridge->sysdata = port;
+	bridge->busnr = port->root_busno;
+	bridge->ops = &xilinx_cpm_pcie_ops;
+	bridge->map_irq = of_irq_parse_and_map_pci;
+	bridge->swizzle_irq = pci_common_swizzle;
+
+	err = pci_scan_root_bus_bridge(bridge);
+	if (err)
+		return err;
+
+	bus = bridge->bus;
+
+	pci_assign_unassigned_bus_resources(bus);
+	list_for_each_entry(child, &bus->children, node)
+		pcie_bus_configure_settings(child);
+	pci_bus_add_devices(bus);
+	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] 9+ messages in thread

* Re: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver
  2019-12-20 11:41 ` [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver Bharat Kumar Gogada
@ 2019-12-20 14:58   ` Bjorn Helgaas
  2019-12-27 11:55     ` Bharat Kumar Gogada
  2020-01-05 18:49   ` Dan Carpenter
  1 sibling, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2019-12-20 14:58 UTC (permalink / raw)
  To: Bharat Kumar Gogada
  Cc: linux-pci, linux-kernel, rgummal, Michal Simek, Ley Foon Tan,
	rfi, Jingoo Han, Gustavo Pimentel

[+cc other drivers that use the broken "is link up" test in config
access]

On Fri, Dec 20, 2019 at 05:11:12PM +0530, Bharat Kumar Gogada wrote:
> - Adding support for versal CPM as Root Port.
> - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
>   block for CPM along with the integrated bridge can function
>   as PCIe Root Port.
> - Versal CPM uses GICv3 ITS feature for assigning MSI/MSI-X
>   vectors and handling MSI/MSI-X interrupts.
> - Bridge error and legacy interrupts in versal CPM are handled using
>   versal CPM specific MISC interrupt line.

"Versal" appears to be a brand name and should be capitalized
consistently.

> +#define INTX_NUM                        4

Don't we have a generic PCI core definition for this?

> +static inline bool cpm_pcie_link_is_up(struct xilinx_cpm_pcie_port *port)

Please follow the example of other drivers and name this
"cpm_pcie_link_up()".

> +{
> +	return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) &
> +		XILINX_CPM_PCIE_REG_PSCR_LNKUP) ? 1 : 0;
> +}

> +/**
> + * xilinx_cpm_pcie_valid_device - Check if a valid device is present on bus
> + * @bus: PCI Bus structure
> + * @devfn: device/function
> + *
> + * Return: 'true' on success and 'false' if invalid device is found
> + */
> +static bool xilinx_cpm_pcie_valid_device(struct pci_bus *bus,
> +					 unsigned int devfn)
> +{
> +	struct xilinx_cpm_pcie_port *port = bus->sysdata;
> +
> +	/* Check if link is up when trying to access downstream ports */
> +	if (bus->number != port->root_busno)
> +		if (!cpm_pcie_link_is_up(port))
> +			return false;

This check for the link being up is racy and should be removed.  The
link may go down after the check and before the config access.

A config access to a device where the link is down is an error, but
it's an error we expect to happen because of enumeration, surprise
unplug, or electrical issue.  It's impossible to avoid so we must be
able to handle and recover from it.

I know there are other drivers that do this (dwc, altera, xilinix-nwl,
xilinx), and I've pointed this out many times in the past.  This needs
to be fixed.

> +
> +	/* Only one device down on each root port */
> +	if (bus->number == port->root_busno && devfn > 0)
> +		return false;
> +
> +	return true;
> +}

> +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
> +{
> +	struct xilinx_cpm_pcie_port *port =
> +				(struct xilinx_cpm_pcie_port *)data;

  struct device *dev = port->dev;

to reduce repetition of "port->dev" below.

> +	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(port->dev, "Link Down\n");
> +
> +	if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> +		dev_info(port->dev, "Hot reset\n");
> ...


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver
  2019-12-20 14:58   ` Bjorn Helgaas
@ 2019-12-27 11:55     ` Bharat Kumar Gogada
  0 siblings, 0 replies; 9+ messages in thread
From: Bharat Kumar Gogada @ 2019-12-27 11:55 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Ravikiran Gummaluri, Michal Simek,
	Ley Foon Tan, rfi, Jingoo Han, Gustavo Pimentel

> Subject: Re: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port
> driver
> 
> [+cc other drivers that use the broken "is link up" test in config access]
> 
> On Fri, Dec 20, 2019 at 05:11:12PM +0530, Bharat Kumar Gogada wrote:
> > - Adding support for versal CPM as Root Port.
> > - The Versal ACAP devices include CCIX-PCIe Module (CPM). The integrated
> >   block for CPM along with the integrated bridge can function
> >   as PCIe Root Port.
> > - Versal CPM uses GICv3 ITS feature for assigning MSI/MSI-X
> >   vectors and handling MSI/MSI-X interrupts.
> > - Bridge error and legacy interrupts in versal CPM are handled using
> >   versal CPM specific MISC interrupt line.
> 
> "Versal" appears to be a brand name and should be capitalized consistently.
> 
> > +#define INTX_NUM                        4
> 
> Don't we have a generic PCI core definition for this?
Thanks Bjorn, will fix this with core definition.
> 
> > +static inline bool cpm_pcie_link_is_up(struct xilinx_cpm_pcie_port
> > +*port)
> 
> Please follow the example of other drivers and name this "cpm_pcie_link_up()".
Agreed, will change the name.
> 
> > +{
> > +	return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) &
> > +		XILINX_CPM_PCIE_REG_PSCR_LNKUP) ? 1 : 0; }
> 
> > +/**
> > + * xilinx_cpm_pcie_valid_device - Check if a valid device is present
> > +on bus
> > + * @bus: PCI Bus structure
> > + * @devfn: device/function
> > + *
> > + * Return: 'true' on success and 'false' if invalid device is found
> > +*/ static bool xilinx_cpm_pcie_valid_device(struct pci_bus *bus,
> > +					 unsigned int devfn)
> > +{
> > +	struct xilinx_cpm_pcie_port *port = bus->sysdata;
> > +
> > +	/* Check if link is up when trying to access downstream ports */
> > +	if (bus->number != port->root_busno)
> > +		if (!cpm_pcie_link_is_up(port))
> > +			return false;
> 
> This check for the link being up is racy and should be removed.  The link may go
> down after the check and before the config access.
> 
> A config access to a device where the link is down is an error, but it's an error we
> expect to happen because of enumeration, surprise unplug, or electrical issue.
> It's impossible to avoid so we must be able to handle and recover from it.
> 
> I know there are other drivers that do this (dwc, altera, xilinix-nwl, xilinx), and
> I've pointed this out many times in the past.  This needs to be fixed.
Agreed, will fix this check.
> 
> > +
> > +	/* Only one device down on each root port */
> > +	if (bus->number == port->root_busno && devfn > 0)
> > +		return false;
> > +
> > +	return true;
> > +}
> 
> > +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *data)
> > +{
> > +	struct xilinx_cpm_pcie_port *port =
> > +				(struct xilinx_cpm_pcie_port *)data;
> 
>   struct device *dev = port->dev;
> 
> to reduce repetition of "port->dev" below.
Agreed, will fix this.

> > +	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(port->dev, "Link Down\n");
> > +
> > +	if (status & XILINX_CPM_PCIE_INTR_HOT_RESET)
> > +		dev_info(port->dev, "Hot reset\n");
> > ...


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver
  2019-12-20 11:41 ` [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver Bharat Kumar Gogada
  2019-12-20 14:58   ` Bjorn Helgaas
@ 2020-01-05 18:49   ` Dan Carpenter
  2020-01-08 10:16     ` Bharat Kumar Gogada
  1 sibling, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2020-01-05 18:49 UTC (permalink / raw)
  To: kbuild, Bharat Kumar Gogada
  Cc: kbuild-all, linux-pci, linux-kernel, bhelgaas, rgummal,
	Bharat Kumar Gogada

Hi Bharat,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Bharat-Kumar-Gogada/Adding-support-for-versal-CPM-as-Root-Port-driver/20191223-193219
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next

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

New smatch warnings:
drivers/pci/controller/pcie-xilinx-cpm.c:330 xilinx_cpm_pcie_init_irq_domain() warn: passing zero to 'PTR_ERR'

Old smatch warnings:
drivers/pci/controller/pcie-xilinx-cpm.c:338 xilinx_cpm_pcie_init_irq_domain() warn: passing zero to 'PTR_ERR'

# https://github.com/0day-ci/linux/commit/f107713acb796e598f16a23b33af74fa382921b2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f107713acb796e598f16a23b33af74fa382921b2
vim +/PTR_ERR +330 drivers/pci/controller/pcie-xilinx-cpm.c

f107713acb796e Bharat Kumar Gogada 2019-12-20  320  static int xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port *port)
f107713acb796e Bharat Kumar Gogada 2019-12-20  321  {
f107713acb796e Bharat Kumar Gogada 2019-12-20  322  	struct device *dev = port->dev;
f107713acb796e Bharat Kumar Gogada 2019-12-20  323  	struct device_node *node = dev->of_node;
f107713acb796e Bharat Kumar Gogada 2019-12-20  324  	struct device_node *pcie_intc_node;
f107713acb796e Bharat Kumar Gogada 2019-12-20  325  
f107713acb796e Bharat Kumar Gogada 2019-12-20  326  	/* Setup INTx */
f107713acb796e Bharat Kumar Gogada 2019-12-20  327  	pcie_intc_node = of_get_next_child(node, NULL);
f107713acb796e Bharat Kumar Gogada 2019-12-20  328  	if (!pcie_intc_node) {
                                                            ^^^^^^^^^^^^^^^

f107713acb796e Bharat Kumar Gogada 2019-12-20  329  		dev_err(dev, "No PCIe Intc node found\n");
f107713acb796e Bharat Kumar Gogada 2019-12-20 @330  		return PTR_ERR(pcie_intc_node);
                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
(This means success).

f107713acb796e Bharat Kumar Gogada 2019-12-20  331  	}
f107713acb796e Bharat Kumar Gogada 2019-12-20  332  
f107713acb796e Bharat Kumar Gogada 2019-12-20  333  	port->leg_domain = irq_domain_add_linear(pcie_intc_node, INTX_NUM,
f107713acb796e Bharat Kumar Gogada 2019-12-20  334  						 &intx_domain_ops,
f107713acb796e Bharat Kumar Gogada 2019-12-20  335  						 port);
f107713acb796e Bharat Kumar Gogada 2019-12-20  336  	if (!port->leg_domain) {
f107713acb796e Bharat Kumar Gogada 2019-12-20  337  		dev_err(dev, "Failed to get a INTx IRQ domain\n");
f107713acb796e Bharat Kumar Gogada 2019-12-20  338  		return PTR_ERR(port->leg_domain);
f107713acb796e Bharat Kumar Gogada 2019-12-20  339  	}
f107713acb796e Bharat Kumar Gogada 2019-12-20  340  
f107713acb796e Bharat Kumar Gogada 2019-12-20  341  	return 0;
f107713acb796e Bharat Kumar Gogada 2019-12-20  342  }

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver
  2020-01-05 18:49   ` Dan Carpenter
@ 2020-01-08 10:16     ` Bharat Kumar Gogada
  0 siblings, 0 replies; 9+ messages in thread
From: Bharat Kumar Gogada @ 2020-01-08 10:16 UTC (permalink / raw)
  To: Dan Carpenter, kbuild
  Cc: kbuild-all, linux-pci, linux-kernel, bhelgaas, Ravikiran Gummaluri

Thanks Dan, will fix it and resend.

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Monday, January 6, 2020 12:20 AM
> To: kbuild@lists.01.org; Bharat Kumar Gogada <bharatku@xilinx.com>
> Cc: kbuild-all@lists.01.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; bhelgaas@google.com; Ravikiran Gummaluri
> <rgummal@xilinx.com>; Bharat Kumar Gogada <bharatku@xilinx.com>
> Subject: Re: [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port
> driver
> 
> Hi Bharat,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> url:    https://github.com/0day-ci/linux/commits/Bharat-Kumar-Gogada/Adding-
> support-for-versal-CPM-as-Root-Port-driver/20191223-193219
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
> 
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> New smatch warnings:
> drivers/pci/controller/pcie-xilinx-cpm.c:330 xilinx_cpm_pcie_init_irq_domain()
> warn: passing zero to 'PTR_ERR'
> 
> Old smatch warnings:
> drivers/pci/controller/pcie-xilinx-cpm.c:338 xilinx_cpm_pcie_init_irq_domain()
> warn: passing zero to 'PTR_ERR'
> 
> # https://github.com/0day-
> ci/linux/commit/f107713acb796e598f16a23b33af74fa382921b2
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout f107713acb796e598f16a23b33af74fa382921b2
> vim +/PTR_ERR +330 drivers/pci/controller/pcie-xilinx-cpm.c
> 
> f107713acb796e Bharat Kumar Gogada 2019-12-20  320  static int
> xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port *port)
> f107713acb796e Bharat Kumar Gogada 2019-12-20  321  {
> f107713acb796e Bharat Kumar Gogada 2019-12-20  322  	struct device *dev =
> port->dev;
> f107713acb796e Bharat Kumar Gogada 2019-12-20  323  	struct device_node
> *node = dev->of_node;
> f107713acb796e Bharat Kumar Gogada 2019-12-20  324  	struct device_node
> *pcie_intc_node;
> f107713acb796e Bharat Kumar Gogada 2019-12-20  325
> f107713acb796e Bharat Kumar Gogada 2019-12-20  326  	/* Setup INTx */
> f107713acb796e Bharat Kumar Gogada 2019-12-20  327  	pcie_intc_node =
> of_get_next_child(node, NULL);
> f107713acb796e Bharat Kumar Gogada 2019-12-20  328  	if (!pcie_intc_node) {
>                                                             ^^^^^^^^^^^^^^^
> 
> f107713acb796e Bharat Kumar Gogada 2019-12-20  329  		dev_err(dev,
> "No PCIe Intc node found\n");
> f107713acb796e Bharat Kumar Gogada 2019-12-20 @330  		return
> PTR_ERR(pcie_intc_node);
>                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> (This means success).
> 
> f107713acb796e Bharat Kumar Gogada 2019-12-20  331  	}
> f107713acb796e Bharat Kumar Gogada 2019-12-20  332
> f107713acb796e Bharat Kumar Gogada 2019-12-20  333  	port->leg_domain =
> irq_domain_add_linear(pcie_intc_node, INTX_NUM,
> f107713acb796e Bharat Kumar Gogada 2019-12-20  334
> 			 &intx_domain_ops,
> f107713acb796e Bharat Kumar Gogada 2019-12-20  335
> 			 port);
> f107713acb796e Bharat Kumar Gogada 2019-12-20  336  	if (!port->leg_domain)
> {
> f107713acb796e Bharat Kumar Gogada 2019-12-20  337  		dev_err(dev,
> "Failed to get a INTx IRQ domain\n");
> f107713acb796e Bharat Kumar Gogada 2019-12-20  338  		return
> PTR_ERR(port->leg_domain);
> f107713acb796e Bharat Kumar Gogada 2019-12-20  339  	}
> f107713acb796e Bharat Kumar Gogada 2019-12-20  340
> f107713acb796e Bharat Kumar Gogada 2019-12-20  341  	return 0;
> f107713acb796e Bharat Kumar Gogada 2019-12-20  342  }
> 
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller
  2019-12-20 11:41 ` [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller Bharat Kumar Gogada
@ 2020-02-25 10:58   ` Lorenzo Pieralisi
  2020-02-25 13:53     ` Bharat Kumar Gogada
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Pieralisi @ 2020-02-25 10:58 UTC (permalink / raw)
  To: Bharat Kumar Gogada; +Cc: linux-pci, linux-kernel, bhelgaas, rgummal

On Fri, Dec 20, 2019 at 05:11:11PM +0530, Bharat Kumar Gogada wrote:
> Adding device tree binding documentation for versal CPM Root Port driver.
> 
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
>  .../devicetree/bindings/pci/xilinx-versal-cpm.txt  | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> new file mode 100644
> index 0000000..35f8556
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> @@ -0,0 +1,66 @@
> +* Xilinx Versal CPM DMA Root Port Bridge DT description
> +
> +Required properties:
> +- #address-cells: Address representation for root ports, set to <3>
> +- #size-cells: Size representation for root ports, set to <2>
> +- #interrupt-cells: specifies the number of cells needed to encode an
> +	interrupt source. The value must be 1.
> +- compatible: Should contain "xlnx,versal-cpm-host-1.00"
> +- reg: Should contain configuration space (includes bridge registers) and
> +	CPM system level control and status registers, and length
> +- reg-names: Must include the following entries:
> +	"cfg": configuration space region and bridge registers
> +	"cpm_slcr": CPM system level control and status registers
> +- interrupts: Should contain AXI PCIe interrupt
> +- interrupt-map-mask,
> +  interrupt-map: standard PCI properties to define the mapping of the
> +	PCI interface to interrupt numbers.
> +- ranges: ranges for the PCI memory regions (I/O space region is not
> +	supported by hardware)
> +	Please refer to the standard PCI bus binding document for a more
> +	detailed explanation
> +- msi-map: Maps a Requester ID to an MSI controller and associated MSI
> +	sideband data
> +- interrupt-names: Must include the following entries:
> +	"misc": interrupt asserted when legacy or error interrupt is received
> +
> +Interrupt controller child node
> ++++++++++++++++++++++++++++++++
> +Required properties:
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #address-cells: specifies the number of cells needed to encode an
> +	address. The value must be 0.
> +- #interrupt-cells: specifies the number of cells needed to encode an
> +	interrupt source. The value must be 1.
> +
> +
> +Refer to the following binding document for more detailed description on
> +the use of 'msi-map':
> +	 Documentation/devicetree/bindings/pci/pci-msi.txt
> +
> +Example:
> +	pci@fca10000 {
> +		#address-cells = <3>;
> +		#interrupt-cells = <1>;
> +		#size-cells = <2>;
> +		compatible = "xlnx,versal-cpm-host-1.00";
> +		interrupt-map = <0 0 0 1 &pcie_intc_0 1>,
> +				<0 0 0 2 &pcie_intc_0 2>,
> +				<0 0 0 3 &pcie_intc_0 3>,
> +				<0 0 0 4 &pcie_intc_0 4>;

This is wrong, interrupts map to pin 0,1,2,3 of pcie_intc.

That's what's forcing you to use the pci_irqd_intx_xlate()
function and that's completely wrong.

I should find a way to write a common binding for all the
host bridges interrupt-map since this comes up over and over
again.

Please have a look at:

Documentation/devicetree/bindings/pci/faraday,ftpci100.txt


> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-parent = <&gic>;
> +		interrupt-names = "misc";
> +		interrupts = <0 72 4>;
> +		ranges = <0x02000000 0x00000000 0xE0000000 0x0 0xE0000000 0x00000000 0x10000000>,
> +			 <0x43000000 0x00000080 0x00000000 0x00000080 0x00000000 0x00000000 0x80000000>;
> +		msi-map = <0x0 &its_gic 0x0 0x10000>;
> +		reg = <0x6 0x00000000 0x0 0x1000000>,
> +		      <0x0 0xFCA10000 0x0 0x1000>;
> +		reg-names = "cfg", "cpm_slcr";
> +		pcie_intc_0: pci-interrupt-controller {
> +			#address-cells = <0>;
> +			#interrupt-cells = <1>;
> +			interrupt-controller ;
> +		};
> +	};
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller
  2020-02-25 10:58   ` Lorenzo Pieralisi
@ 2020-02-25 13:53     ` Bharat Kumar Gogada
  0 siblings, 0 replies; 9+ messages in thread
From: Bharat Kumar Gogada @ 2020-02-25 13:53 UTC (permalink / raw)
  To: lorenzo.pieralisi; +Cc: linux-pci, linux-kernel, bhelgaas, Ravikiran Gummaluri

> 
> On Fri, Dec 20, 2019 at 05:11:11PM +0530, Bharat Kumar Gogada wrote:
> > Adding device tree binding documentation for versal CPM Root Port driver.
> >
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > ---
> >  .../devicetree/bindings/pci/xilinx-versal-cpm.txt  | 66
> > ++++++++++++++++++++++
> >  1 file changed, 66 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> > b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> > new file mode 100644
> > index 0000000..35f8556
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.txt
> > @@ -0,0 +1,66 @@
> > +* Xilinx Versal CPM DMA Root Port Bridge DT description
> > +
> > +Required properties:
> > +- #address-cells: Address representation for root ports, set to <3>
> > +- #size-cells: Size representation for root ports, set to <2>
> > +- #interrupt-cells: specifies the number of cells needed to encode an
> > +	interrupt source. The value must be 1.
> > +- compatible: Should contain "xlnx,versal-cpm-host-1.00"
> > +- reg: Should contain configuration space (includes bridge registers) and
> > +	CPM system level control and status registers, and length
> > +- reg-names: Must include the following entries:
> > +	"cfg": configuration space region and bridge registers
> > +	"cpm_slcr": CPM system level control and status registers
> > +- interrupts: Should contain AXI PCIe interrupt
> > +- interrupt-map-mask,
> > +  interrupt-map: standard PCI properties to define the mapping of the
> > +	PCI interface to interrupt numbers.
> > +- ranges: ranges for the PCI memory regions (I/O space region is not
> > +	supported by hardware)
> > +	Please refer to the standard PCI bus binding document for a more
> > +	detailed explanation
> > +- msi-map: Maps a Requester ID to an MSI controller and associated MSI
> > +	sideband data
> > +- interrupt-names: Must include the following entries:
> > +	"misc": interrupt asserted when legacy or error interrupt is
> > +received
> > +
> > +Interrupt controller child node
> > ++++++++++++++++++++++++++++++++
> > +Required properties:
> > +- interrupt-controller: identifies the node as an interrupt
> > +controller
> > +- #address-cells: specifies the number of cells needed to encode an
> > +	address. The value must be 0.
> > +- #interrupt-cells: specifies the number of cells needed to encode an
> > +	interrupt source. The value must be 1.
> > +
> > +
> > +Refer to the following binding document for more detailed description
> > +on the use of 'msi-map':
> > +	 Documentation/devicetree/bindings/pci/pci-msi.txt
> > +
> > +Example:
> > +	pci@fca10000 {
> > +		#address-cells = <3>;
> > +		#interrupt-cells = <1>;
> > +		#size-cells = <2>;
> > +		compatible = "xlnx,versal-cpm-host-1.00";
> > +		interrupt-map = <0 0 0 1 &pcie_intc_0 1>,
> > +				<0 0 0 2 &pcie_intc_0 2>,
> > +				<0 0 0 3 &pcie_intc_0 3>,
> > +				<0 0 0 4 &pcie_intc_0 4>;
> 
> This is wrong, interrupts map to pin 0,1,2,3 of pcie_intc.
> 
> That's what's forcing you to use the pci_irqd_intx_xlate() function and that's
> completely wrong.
> 
> I should find a way to write a common binding for all the host bridges interrupt-
> map since this comes up over and over again.
> 
> Please have a look at:
> 
> Documentation/devicetree/bindings/pci/faraday,ftpci100.txt
Thanks Lorenzo, will fix this in next patch.
> 
> 
> > +		interrupt-map-mask = <0 0 0 7>;
> > +		interrupt-parent = <&gic>;
> > +		interrupt-names = "misc";
> > +		interrupts = <0 72 4>;
> > +		ranges = <0x02000000 0x00000000 0xE0000000 0x0
> 0xE0000000 0x00000000 0x10000000>,
> > +			 <0x43000000 0x00000080 0x00000000 0x00000080
> 0x00000000 0x00000000 0x80000000>;
> > +		msi-map = <0x0 &its_gic 0x0 0x10000>;
> > +		reg = <0x6 0x00000000 0x0 0x1000000>,
> > +		      <0x0 0xFCA10000 0x0 0x1000>;
> > +		reg-names = "cfg", "cpm_slcr";
> > +		pcie_intc_0: pci-interrupt-controller {
> > +			#address-cells = <0>;
> > +			#interrupt-cells = <1>;
> > +			interrupt-controller ;
> > +		};
> > +	};
> > --
> > 2.7.4
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-25 13:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 11:41 [PATCH 0/2] Adding support for versal CPM as Root Port driver Bharat Kumar Gogada
2019-12-20 11:41 ` [PATCH 1/2] PCI: Versal CPM: Add device tree binding forversal CPM host controller Bharat Kumar Gogada
2020-02-25 10:58   ` Lorenzo Pieralisi
2020-02-25 13:53     ` Bharat Kumar Gogada
2019-12-20 11:41 ` [PATCH 2/2] PCI: Versal CPM: Add support for Versal CPM Root Port driver Bharat Kumar Gogada
2019-12-20 14:58   ` Bjorn Helgaas
2019-12-27 11:55     ` Bharat Kumar Gogada
2020-01-05 18:49   ` Dan Carpenter
2020-01-08 10:16     ` 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).