* [PATCH v8 0/2] Adding support for Versal CPM as Root Port driver @ 2020-06-08 13:18 Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada 0 siblings, 2 replies; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-08 13:18 UTC (permalink / raw) To: linux-pci, linux-kernel Cc: bhelgaas, lorenzo.pieralisi, robh, 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 v8: - Added support for handling error events and INTx interrupts separately using nested chained irqchips. - Added support to free allocated resources in error cases. Bharat Kumar Gogada (2): PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port PCI: xilinx-cpm: Add Versal CPM Root Port driver .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 99 ++++ drivers/pci/controller/Kconfig | 8 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pcie-xilinx-cpm.c | 621 +++++++++++++++++++++ 4 files changed, 729 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] 9+ messages in thread
* [PATCH v8 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port 2020-06-08 13:18 [PATCH v8 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada @ 2020-06-08 13:18 ` Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada 1 sibling, 0 replies; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-08 13:18 UTC (permalink / raw) To: linux-pci, linux-kernel Cc: bhelgaas, lorenzo.pieralisi, robh, Bharat Kumar Gogada Add YAML schemas documentation for Versal CPM Root Port driver. Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com> --- .../devicetree/bindings/pci/xilinx-versal-cpm.yaml | 99 ++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml diff --git a/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml new file mode 100644 index 0000000..a2bbc0e --- /dev/null +++ b/Documentation/devicetree/bindings/pci/xilinx-versal-cpm.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/xilinx-versal-cpm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: CPM Host Controller device tree for Xilinx Versal SoCs + +maintainers: + - Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com> + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: + const: xlnx,versal-cpm-host-1.00 + + reg: + items: + - description: Configuration space region and bridge registers. + - description: CPM system level control and status registers. + + reg-names: + items: + - const: cfg + - const: cpm_slcr + + interrupts: + maxItems: 1 + + msi-map: + description: + Maps a Requester ID to an MSI controller and associated MSI sideband data. + + ranges: + maxItems: 2 + + "#interrupt-cells": + const: 1 + + interrupt-controller: + description: Interrupt controller node for handling legacy PCI interrupts. + type: object + properties: + "#address-cells": + const: 0 + "#interrupt-cells": + const: 1 + "interrupt-controller": true + additionalProperties: false + +required: + - reg + - reg-names + - "#interrupt-cells" + - interrupts + - interrupt-parent + - interrupt-map + - interrupt-map-mask + - bus-range + - msi-map + - interrupt-controller + +unevaluatedProperties: false + +examples: + - | + + versal { + #address-cells = <2>; + #size-cells = <2>; + cpm_pcie: pcie@fca10000 { + compatible = "xlnx,versal-cpm-host-1.00"; + device_type = "pci"; + #address-cells = <3>; + #interrupt-cells = <1>; + #size-cells = <2>; + interrupts = <0 72 4>; + interrupt-parent = <&gic>; + interrupt-map-mask = <0 0 0 7>; + interrupt-map = <0 0 0 1 &pcie_intc_0 0>, + <0 0 0 2 &pcie_intc_0 1>, + <0 0 0 3 &pcie_intc_0 2>, + <0 0 0 4 &pcie_intc_0 3>; + bus-range = <0x00 0xff>; + ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000>, + <0x43000000 0x80 0x00000000 0x80 0x00000000 0x0 0x80000000>; + msi-map = <0x0 &its_gic 0x0 0x10000>; + reg = <0x6 0x00000000 0x0 0x10000000>, + <0x0 0xfca10000 0x0 0x1000>; + reg-names = "cfg", "cpm_slcr"; + pcie_intc_0: interrupt-controller { + #address-cells = <0>; + #interrupt-cells = <1>; + interrupt-controller; + }; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-08 13:18 [PATCH v8 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada @ 2020-06-08 13:18 ` Bharat Kumar Gogada 2020-06-08 17:58 ` Marc Zyngier 2020-06-11 20:56 ` Bjorn Helgaas 1 sibling, 2 replies; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-08 13:18 UTC (permalink / raw) To: linux-pci, linux-kernel Cc: bhelgaas, lorenzo.pieralisi, robh, Bharat Kumar Gogada, Marc Zyngier - 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> Signed-off-by: Marc Zyngier <maz@kernel.org> --- drivers/pci/controller/Kconfig | 8 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pcie-xilinx-cpm.c | 621 +++++++++++++++++++++++++++++++ 3 files changed, 630 insertions(+) create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index 20bf00f..d9e393a 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -81,6 +81,14 @@ config PCIE_XILINX Say 'Y' here if you want kernel to support the Xilinx AXI PCIe Host Bridge driver. +config PCIE_XILINX_CPM + bool "Xilinx Versal CPM host bridge support" + depends on ARCH_ZYNQMP || COMPILE_TEST + select PCI_HOST_COMMON + help + Say 'Y' here if you want kernel support for the + Xilinx Versal CPM host bridge. + config PCI_XGENE bool "X-Gene PCIe controller" depends on ARM64 || COMPILE_TEST diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile index 01b2502..78dabda 100644 --- a/drivers/pci/controller/Makefile +++ b/drivers/pci/controller/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o +obj-$(CONFIG_PCIE_XILINX_CPM) += pcie-xilinx-cpm.o obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c b/drivers/pci/controller/pcie-xilinx-cpm.c new file mode 100644 index 0000000..5bc481e --- /dev/null +++ b/drivers/pci/controller/pcie-xilinx-cpm.c @@ -0,0 +1,621 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge + * + * (C) Copyright 2019 - 2020, Xilinx, Inc. + */ + +#include <linux/bitfield.h> +#include <linux/interrupt.h> +#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_pci.h> +#include <linux/of_platform.h> +#include <linux/of_irq.h> +#include <linux/pci.h> +#include <linux/platform_device.h> +#include <linux/pci-ecam.h> + +#include "../pci.h" + +/* Register definitions */ +#define XILINX_CPM_PCIE_REG_IDR 0x00000E10 +#define XILINX_CPM_PCIE_REG_IMR 0x00000E14 +#define XILINX_CPM_PCIE_REG_PSCR 0x00000E1C +#define XILINX_CPM_PCIE_REG_RPSC 0x00000E20 +#define XILINX_CPM_PCIE_REG_RPEFR 0x00000E2C +#define XILINX_CPM_PCIE_REG_IDRN 0x00000E38 +#define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C +#define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 +#define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 +#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) + +/* Interrupt registers definitions */ +#define XILINX_CPM_PCIE_INTR_LINK_DOWN 0 +#define XILINX_CPM_PCIE_INTR_HOT_RESET 3 +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT 4 +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT 8 +#define XILINX_CPM_PCIE_INTR_CORRECTABLE 9 +#define XILINX_CPM_PCIE_INTR_NONFATAL 10 +#define XILINX_CPM_PCIE_INTR_FATAL 11 +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON 12 +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD 15 +#define XILINX_CPM_PCIE_INTR_INTX 16 +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD 17 +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP 20 +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP 21 +#define XILINX_CPM_PCIE_INTR_SLV_COMPL 22 +#define XILINX_CPM_PCIE_INTR_SLV_ERRP 23 +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT 24 +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR 25 +#define XILINX_CPM_PCIE_INTR_MST_DECERR 26 +#define XILINX_CPM_PCIE_INTR_MST_SLVERR 27 +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT 28 + +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) + +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) + +#define XILINX_CPM_PCIE_IMR_ALL_MASK \ + ( \ + IMR(LINK_DOWN) | \ + IMR(HOT_RESET) | \ + IMR(CFG_PCIE_TIMEOUT) | \ + IMR(CFG_TIMEOUT) | \ + IMR(CORRECTABLE) | \ + IMR(NONFATAL) | \ + IMR(FATAL) | \ + IMR(CFG_ERR_POISON) | \ + IMR(PME_TO_ACK_RCVD) | \ + IMR(INTX) | \ + IMR(PM_PME_RCVD) | \ + IMR(SLV_UNSUPP) | \ + IMR(SLV_UNEXP) | \ + IMR(SLV_COMPL) | \ + IMR(SLV_ERRP) | \ + IMR(SLV_CMPABT) | \ + IMR(SLV_ILLBUR) | \ + IMR(MST_DECERR) | \ + IMR(MST_SLVERR) | \ + IMR(SLV_PCIE_TIMEOUT) \ + ) + +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16) +#define XILINX_CPM_PCIE_IDRN_SHIFT 16 + +/* Root Port Error FIFO Read Register definitions */ +#define XILINX_CPM_PCIE_RPEFR_ERR_VALID BIT(18) +#define XILINX_CPM_PCIE_RPEFR_REQ_ID GENMASK(15, 0) +#define XILINX_CPM_PCIE_RPEFR_ALL_MASK 0xFFFFFFFF + +/* Root Port Status/control Register definitions */ +#define XILINX_CPM_PCIE_REG_RPSC_BEN BIT(0) + +/* Phy Status/Control Register definitions */ +#define XILINX_CPM_PCIE_REG_PSCR_LNKUP BIT(11) + +/** + * struct xilinx_cpm_pcie_port - PCIe port information + * @reg_base: Bridge Register Base + * @cpm_base: CPM System Level Control and Status Register(SLCR) Base + * @dev: Device pointer + * @intx_domain: Legacy IRQ domain pointer + * @cfg: Holds mappings of config space window + * @intx_irq: legacy interrupt number + * @irq: Error interrupt number + * @lock: lock protecting shared register access + */ +struct xilinx_cpm_pcie_port { + void __iomem *reg_base; + void __iomem *cpm_base; + struct device *dev; + struct irq_domain *intx_domain; + struct irq_domain *cpm_domain; + struct pci_config_window *cfg; + int intx_irq; + int irq; + raw_spinlock_t lock; +}; + +static u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg) +{ + return readl_relaxed(port->reg_base + reg); +} + +static void pcie_write(struct xilinx_cpm_pcie_port *port, + u32 val, u32 reg) +{ + writel_relaxed(val, port->reg_base + reg); +} + +static bool cpm_pcie_link_up(struct xilinx_cpm_pcie_port *port) +{ + return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) & + XILINX_CPM_PCIE_REG_PSCR_LNKUP); +} + +/** + * xilinx_cpm_pcie_clear_err_interrupts - Clear Error Interrupts + * @port: PCIe port information + */ +static void cpm_pcie_clear_err_interrupts(struct xilinx_cpm_pcie_port *port) +{ + unsigned long val = pcie_read(port, XILINX_CPM_PCIE_REG_RPEFR); + + if (val & XILINX_CPM_PCIE_RPEFR_ERR_VALID) { + dev_dbg(port->dev, "Requester ID %lu\n", + val & XILINX_CPM_PCIE_RPEFR_REQ_ID); + pcie_write(port, XILINX_CPM_PCIE_RPEFR_ALL_MASK, + XILINX_CPM_PCIE_REG_RPEFR); + } +} + +static void xilinx_cpm_mask_leg_irq(struct irq_data *data) +{ + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data); + unsigned long flags; + u32 mask; + u32 val; + + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); + raw_spin_lock_irqsave(&port->lock, flags); + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); + pcie_write(port, (val & (~mask)), XILINX_CPM_PCIE_REG_IDRN_MASK); + raw_spin_unlock_irqrestore(&port->lock, flags); +} + +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data) +{ + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data); + unsigned long flags; + u32 mask; + u32 val; + + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); + raw_spin_lock_irqsave(&port->lock, flags); + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); + pcie_write(port, (val | mask), XILINX_CPM_PCIE_REG_IDRN_MASK); + raw_spin_unlock_irqrestore(&port->lock, flags); +} + +static struct irq_chip xilinx_cpm_leg_irq_chip = { + .name = "INTx", + .irq_mask = xilinx_cpm_mask_leg_irq, + .irq_unmask = xilinx_cpm_unmask_leg_irq, +}; + +/** + * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark IRQ as valid + * @domain: IRQ domain + * @irq: Virtual IRQ number + * @hwirq: HW interrupt number + * + * Return: Always returns 0. + */ +static int xilinx_cpm_pcie_intx_map(struct irq_domain *domain, + unsigned int irq, irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &xilinx_cpm_leg_irq_chip, + handle_level_irq); + irq_set_chip_data(irq, domain->host_data); + irq_set_status_flags(irq, IRQ_LEVEL); + + return 0; +} + +/* INTx IRQ Domain operations */ +static const struct irq_domain_ops intx_domain_ops = { + .map = xilinx_cpm_pcie_intx_map, +}; + +static void xilinx_cpm_pcie_intx_flow(struct irq_desc *desc) +{ + struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long val; + int i; + + chained_irq_enter(chip, desc); + + val = FIELD_GET(XILINX_CPM_PCIE_IDRN_MASK, + pcie_read(port, XILINX_CPM_PCIE_REG_IDRN)); + + for_each_set_bit(i, &val, PCI_NUM_INTX) + generic_handle_irq(irq_find_mapping(port->intx_domain, i)); + + chained_irq_exit(chip, desc); +} + +static void xilinx_cpm_mask_event_irq(struct irq_data *d) +{ + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); + u32 val; + + raw_spin_lock(&port->lock); + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); + val &= ~d->hwirq; + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); + raw_spin_unlock(&port->lock); +} + +static void xilinx_cpm_unmask_event_irq(struct irq_data *d) +{ + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); + u32 val; + + raw_spin_lock(&port->lock); + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); + val |= d->hwirq; + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); + raw_spin_unlock(&port->lock); +} + +static struct irq_chip xilinx_cpm_event_irq_chip = { + .name = "RC-Event", + .irq_mask = xilinx_cpm_mask_event_irq, + .irq_unmask = xilinx_cpm_unmask_event_irq, +}; + +static int xilinx_cpm_pcie_event_map(struct irq_domain *domain, + unsigned int irq, irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &xilinx_cpm_event_irq_chip, + handle_level_irq); + irq_set_chip_data(irq, domain->host_data); + irq_set_status_flags(irq, IRQ_LEVEL); + return 0; +} + +static const struct irq_domain_ops event_domain_ops = { + .map = xilinx_cpm_pcie_event_map, +}; + +static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) +{ + struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long val; + int i; + + chained_irq_enter(chip, desc); + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR); + val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR); + for_each_set_bit(i, &val, 32) + generic_handle_irq(irq_find_mapping(port->cpm_domain, i)); + pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); + + /* + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to + * CPM SLCR block. + */ + val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS); + if (val) + writel_relaxed(val, + port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS); + + chained_irq_exit(chip, desc); +} + +#define _IC(x, s) \ + [XILINX_CPM_PCIE_INTR_ ## x] = { __stringify(x), s } + +static const struct { + const char *sym; + const char *str; +} intr_cause[32] = { + _IC(LINK_DOWN, "Link Down"), + _IC(HOT_RESET, "Hot reset"), + _IC(CFG_TIMEOUT, "ECAM access timeout"), + _IC(CORRECTABLE, "Correctable error message"), + _IC(NONFATAL, "Non fatal error message"), + _IC(FATAL, "Fatal error message"), + _IC(SLV_UNSUPP, "Slave unsupported request"), + _IC(SLV_UNEXP, "Slave unexpected completion"), + _IC(SLV_COMPL, "Slave completion timeout"), + _IC(SLV_ERRP, "Slave Error Poison"), + _IC(SLV_CMPABT, "Slave Completer Abort"), + _IC(SLV_ILLBUR, "Slave Illegal Burst"), + _IC(MST_DECERR, "Master decode error"), + _IC(MST_SLVERR, "Master slave error"), + _IC(CFG_PCIE_TIMEOUT, "PCIe ECAM access timeout"), + _IC(CFG_ERR_POISON, "ECAM poisoned completion received"), + _IC(PME_TO_ACK_RCVD, "PME_TO_ACK message received"), + _IC(PM_PME_RCVD, "PM_PME message received"), + _IC(SLV_PCIE_TIMEOUT, "PCIe completion timeout received"), +}; + +static irqreturn_t xilinx_cpm_pcie_intr_handler(int irq, void *dev_id) +{ + struct xilinx_cpm_pcie_port *port = dev_id; + struct device *dev = port->dev; + struct irq_data *d; + + d = irq_domain_get_irq_data(port->cpm_domain, irq); + + switch (d->hwirq) { + case XILINX_CPM_PCIE_INTR_CORRECTABLE: + case XILINX_CPM_PCIE_INTR_NONFATAL: + case XILINX_CPM_PCIE_INTR_FATAL: + cpm_pcie_clear_err_interrupts(port); + fallthrough; + + default: + if (intr_cause[d->hwirq].str) + dev_warn(dev, "%s\n", intr_cause[d->hwirq].str); + else + dev_warn(dev, "Unknown interrupt\n"); + } + + return IRQ_HANDLED; +} + +static void xilinx_cpm_free_irq_domains(struct xilinx_cpm_pcie_port *port) +{ + if (port->intx_domain) { + irq_domain_remove(port->intx_domain); + port->intx_domain = NULL; + } + + if (port->cpm_domain) { + irq_domain_remove(port->cpm_domain); + port->cpm_domain = NULL; + } +} + +/** + * xilinx_cpm_pcie_init_irq_domain - Initialize IRQ domain + * @port: PCIe port information + * + * Return: '0' on success and error value on failure + */ +static int xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port *port) +{ + struct device *dev = port->dev; + struct device_node *node = dev->of_node; + struct device_node *pcie_intc_node; + + /* Setup INTx */ + pcie_intc_node = of_get_next_child(node, NULL); + if (!pcie_intc_node) { + dev_err(dev, "No PCIe Intc node found\n"); + return -EINVAL; + } + + port->cpm_domain = irq_domain_add_linear(node, 32, + &event_domain_ops, + port); + if (!port->cpm_domain) + goto out; + + irq_domain_update_bus_token(port->cpm_domain, DOMAIN_BUS_NEXUS); + + port->intx_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX, + &intx_domain_ops, + port); + if (!port->intx_domain) + goto out; + + irq_domain_update_bus_token(port->intx_domain, DOMAIN_BUS_WIRED); + + of_node_put(pcie_intc_node); + raw_spin_lock_init(&port->lock); + + return 0; +out: + xilinx_cpm_free_irq_domains(port); + dev_err(dev, "Failed to allocate IRQ domains\n"); + + return -ENOMEM; +} + +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) +{ + struct device *dev = port->dev; + struct platform_device *pdev = to_platform_device(dev); + int i, irq; + + port->irq = platform_get_irq(pdev, 0); + if (port->irq < 0) { + dev_err(dev, "Unable to find misc IRQ line\n"); + return port->irq; + } + + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) { + int err; + + if (!intr_cause[i].str) + continue; + + irq = irq_create_mapping(port->cpm_domain, i); + if (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; + } + + port->intx_irq = irq_create_mapping(port->cpm_domain, + XILINX_CPM_PCIE_INTR_INTX); + if (WARN_ON(port->intx_irq <= 0)) + return -ENXIO; + + /* Plug the INTx chained handler */ + irq_set_chained_handler_and_data(port->intx_irq, + xilinx_cpm_pcie_intx_flow, port); + + /* Plug the main event chained handler */ + irq_set_chained_handler_and_data(port->irq, + xilinx_cpm_pcie_event_flow, port); + + return 0; +} + +/** + * xilinx_cpm_pcie_init_port - Initialize hardware + * @port: PCIe port information + */ +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port *port) +{ + if (cpm_pcie_link_up(port)) + dev_info(port->dev, "PCIe Link is UP\n"); + else + dev_info(port->dev, "PCIe Link is DOWN\n"); + + /* Disable all interrupts */ + pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK, + XILINX_CPM_PCIE_REG_IMR); + + /* Clear pending interrupts */ + pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) & + XILINX_CPM_PCIE_IMR_ALL_MASK, + XILINX_CPM_PCIE_REG_IDR); + + /* 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); +} + +/** + * xilinx_cpm_pcie_parse_dt - Parse Device tree + * @port: PCIe port information + * @bus_range: Bus resource + * + * Return: '0' on success and error value on failure + */ +static int xilinx_cpm_pcie_parse_dt(struct xilinx_cpm_pcie_port *port, + struct resource *bus_range) +{ + struct device *dev = port->dev; + struct platform_device *pdev = to_platform_device(dev); + struct resource *res; + + port->cpm_base = devm_platform_ioremap_resource_byname(pdev, + "cpm_slcr"); + if (IS_ERR(port->cpm_base)) + return PTR_ERR(port->cpm_base); + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg"); + if (!res) + return -ENXIO; + + port->cfg = pci_ecam_create(dev, res, bus_range, + &pci_generic_ecam_ops); + if (IS_ERR(port->cfg)) + return PTR_ERR(port->cfg); + + port->reg_base = port->cfg->win; + + return 0; +} + +void xilinx_cpm_free_interrupts(struct xilinx_cpm_pcie_port *port) +{ + irq_set_chained_handler_and_data(port->intx_irq, NULL, NULL); + irq_set_chained_handler_and_data(port->irq, NULL, NULL); +} + +/** + * xilinx_cpm_pcie_probe - Probe function + * @pdev: Platform device pointer + * + * Return: '0' on success and error value on failure + */ +static int xilinx_cpm_pcie_probe(struct platform_device *pdev) +{ + struct xilinx_cpm_pcie_port *port; + struct device *dev = &pdev->dev; + struct pci_host_bridge *bridge; + struct resource *bus_range; + int err; + + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port)); + if (!bridge) + return -ENODEV; + + port = pci_host_bridge_priv(bridge); + + port->dev = dev; + + err = pci_parse_request_of_pci_ranges(dev, &bridge->windows, + &bridge->dma_ranges, &bus_range); + if (err) { + dev_err(dev, "Getting bridge resources failed\n"); + return err; + } + + err = xilinx_cpm_pcie_init_irq_domain(port); + if (err) + return err; + + err = xilinx_cpm_pcie_parse_dt(port, bus_range); + if (err) { + dev_err(dev, "Parsing DT failed\n"); + goto err_parse_dt; + } + + err = xilinx_cpm_setup_irq(port); + if (err) { + dev_err(dev, "Failed to set up interrupts\n"); + goto err_setup_irq; + } + + xilinx_cpm_pcie_init_port(port); + + bridge->dev.parent = dev; + bridge->sysdata = port->cfg; + bridge->busnr = port->cfg->busr.start; + bridge->ops = &pci_generic_ecam_ops.pci_ops; + bridge->map_irq = of_irq_parse_and_map_pci; + bridge->swizzle_irq = pci_common_swizzle; + + err = pci_host_probe(bridge); + if (err < 0) + goto err_host_bridge; + + return 0; + +err_host_bridge: + xilinx_cpm_free_interrupts(port); +err_setup_irq: + pci_ecam_free(port->cfg); +err_parse_dt: + xilinx_cpm_free_irq_domains(port); + return err; +} + +static const struct of_device_id xilinx_cpm_pcie_of_match[] = { + { .compatible = "xlnx,versal-cpm-host-1.00", }, + {} +}; + +static struct platform_driver xilinx_cpm_pcie_driver = { + .driver = { + .name = "xilinx-cpm-pcie", + .of_match_table = xilinx_cpm_pcie_of_match, + .suppress_bind_attrs = true, + }, + .probe = xilinx_cpm_pcie_probe, +}; + +builtin_platform_driver(xilinx_cpm_pcie_driver); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-08 13:18 ` [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada @ 2020-06-08 17:58 ` Marc Zyngier 2020-06-11 15:51 ` Bharat Kumar Gogada 2020-06-11 20:56 ` Bjorn Helgaas 1 sibling, 1 reply; 9+ messages in thread From: Marc Zyngier @ 2020-06-08 17:58 UTC (permalink / raw) To: Bharat Kumar Gogada Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh Hi Bharat, On 2020-06-08 14:18, 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> > Signed-off-by: Marc Zyngier <maz@kernel.org> Please don't. Either you take my initial patch as is (potentially with reworks that we discuss on the list), or you add a note saying that I suggested some of the changes. But do not add my Sob without me agreeing to it, specially when there are changes that I object to (see below). > --- > drivers/pci/controller/Kconfig | 8 + > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pcie-xilinx-cpm.c | 621 > +++++++++++++++++++++++++++++++ > 3 files changed, 630 insertions(+) > create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c > > diff --git a/drivers/pci/controller/Kconfig > b/drivers/pci/controller/Kconfig > index 20bf00f..d9e393a 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -81,6 +81,14 @@ config PCIE_XILINX > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe > Host Bridge driver. > > +config PCIE_XILINX_CPM > + bool "Xilinx Versal CPM host bridge support" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + select PCI_HOST_COMMON > + help > + Say 'Y' here if you want kernel support for the > + Xilinx Versal CPM host bridge. > + > config PCI_XGENE > bool "X-Gene PCIe controller" > depends on ARM64 || COMPILE_TEST > diff --git a/drivers/pci/controller/Makefile > b/drivers/pci/controller/Makefile > index 01b2502..78dabda 100644 > --- a/drivers/pci/controller/Makefile > +++ b/drivers/pci/controller/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o > obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o > obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o > obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o > +obj-$(CONFIG_PCIE_XILINX_CPM) += pcie-xilinx-cpm.o > obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o > obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o > obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o > diff --git a/drivers/pci/controller/pcie-xilinx-cpm.c > b/drivers/pci/controller/pcie-xilinx-cpm.c > new file mode 100644 > index 0000000..5bc481e > --- /dev/null > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c > @@ -0,0 +1,621 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge > + * > + * (C) Copyright 2019 - 2020, Xilinx, Inc. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqchip.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irqdomain.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of_address.h> > +#include <linux/of_pci.h> > +#include <linux/of_platform.h> > +#include <linux/of_irq.h> > +#include <linux/pci.h> > +#include <linux/platform_device.h> > +#include <linux/pci-ecam.h> > + > +#include "../pci.h" > + > +/* Register definitions */ > +#define XILINX_CPM_PCIE_REG_IDR 0x00000E10 > +#define XILINX_CPM_PCIE_REG_IMR 0x00000E14 > +#define XILINX_CPM_PCIE_REG_PSCR 0x00000E1C > +#define XILINX_CPM_PCIE_REG_RPSC 0x00000E20 > +#define XILINX_CPM_PCIE_REG_RPEFR 0x00000E2C > +#define XILINX_CPM_PCIE_REG_IDRN 0x00000E38 > +#define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C > +#define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 > +#define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 > +#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) > + > +/* Interrupt registers definitions */ > +#define XILINX_CPM_PCIE_INTR_LINK_DOWN 0 > +#define XILINX_CPM_PCIE_INTR_HOT_RESET 3 > +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT 4 > +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT 8 > +#define XILINX_CPM_PCIE_INTR_CORRECTABLE 9 > +#define XILINX_CPM_PCIE_INTR_NONFATAL 10 > +#define XILINX_CPM_PCIE_INTR_FATAL 11 > +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON 12 > +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD 15 > +#define XILINX_CPM_PCIE_INTR_INTX 16 > +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD 17 > +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP 20 > +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP 21 > +#define XILINX_CPM_PCIE_INTR_SLV_COMPL 22 > +#define XILINX_CPM_PCIE_INTR_SLV_ERRP 23 > +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT 24 > +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR 25 > +#define XILINX_CPM_PCIE_INTR_MST_DECERR 26 > +#define XILINX_CPM_PCIE_INTR_MST_SLVERR 27 > +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT 28 > + > +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) > + > +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) Why do we have this twice? > + > +#define XILINX_CPM_PCIE_IMR_ALL_MASK \ > + ( \ > + IMR(LINK_DOWN) | \ nit: Please use tabs between | and \. > + IMR(HOT_RESET) | \ > + IMR(CFG_PCIE_TIMEOUT) | \ > + IMR(CFG_TIMEOUT) | \ > + IMR(CORRECTABLE) | \ > + IMR(NONFATAL) | \ > + IMR(FATAL) | \ > + IMR(CFG_ERR_POISON) | \ > + IMR(PME_TO_ACK_RCVD) | \ > + IMR(INTX) | \ > + IMR(PM_PME_RCVD) | \ > + IMR(SLV_UNSUPP) | \ > + IMR(SLV_UNEXP) | \ > + IMR(SLV_COMPL) | \ > + IMR(SLV_ERRP) | \ > + IMR(SLV_CMPABT) | \ > + IMR(SLV_ILLBUR) | \ > + IMR(MST_DECERR) | \ > + IMR(MST_SLVERR) | \ > + IMR(SLV_PCIE_TIMEOUT) \ > + ) > + > +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF > +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16) > +#define XILINX_CPM_PCIE_IDRN_SHIFT 16 > + > +/* Root Port Error FIFO Read Register definitions */ > +#define XILINX_CPM_PCIE_RPEFR_ERR_VALID BIT(18) > +#define XILINX_CPM_PCIE_RPEFR_REQ_ID GENMASK(15, 0) > +#define XILINX_CPM_PCIE_RPEFR_ALL_MASK 0xFFFFFFFF > + > +/* Root Port Status/control Register definitions */ > +#define XILINX_CPM_PCIE_REG_RPSC_BEN BIT(0) > + > +/* Phy Status/Control Register definitions */ > +#define XILINX_CPM_PCIE_REG_PSCR_LNKUP BIT(11) > + > +/** > + * struct xilinx_cpm_pcie_port - PCIe port information > + * @reg_base: Bridge Register Base > + * @cpm_base: CPM System Level Control and Status Register(SLCR) Base > + * @dev: Device pointer > + * @intx_domain: Legacy IRQ domain pointer > + * @cfg: Holds mappings of config space window > + * @intx_irq: legacy interrupt number > + * @irq: Error interrupt number > + * @lock: lock protecting shared register access > + */ > +struct xilinx_cpm_pcie_port { > + void __iomem *reg_base; > + void __iomem *cpm_base; > + struct device *dev; > + struct irq_domain *intx_domain; > + struct irq_domain *cpm_domain; > + struct pci_config_window *cfg; > + int intx_irq; > + int irq; > + raw_spinlock_t lock; > +}; > + > +static u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg) > +{ > + return readl_relaxed(port->reg_base + reg); > +} > + > +static void pcie_write(struct xilinx_cpm_pcie_port *port, > + u32 val, u32 reg) > +{ > + writel_relaxed(val, port->reg_base + reg); > +} > + > +static bool cpm_pcie_link_up(struct xilinx_cpm_pcie_port *port) > +{ > + return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) & > + XILINX_CPM_PCIE_REG_PSCR_LNKUP); > +} > + > +/** > + * xilinx_cpm_pcie_clear_err_interrupts - Clear Error Interrupts > + * @port: PCIe port information > + */ > +static void cpm_pcie_clear_err_interrupts(struct xilinx_cpm_pcie_port > *port) > +{ > + unsigned long val = pcie_read(port, XILINX_CPM_PCIE_REG_RPEFR); > + > + if (val & XILINX_CPM_PCIE_RPEFR_ERR_VALID) { > + dev_dbg(port->dev, "Requester ID %lu\n", > + val & XILINX_CPM_PCIE_RPEFR_REQ_ID); > + pcie_write(port, XILINX_CPM_PCIE_RPEFR_ALL_MASK, > + XILINX_CPM_PCIE_REG_RPEFR); > + } > +} > + > +static void xilinx_cpm_mask_leg_irq(struct irq_data *data) > +{ > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data); > + unsigned long flags; > + u32 mask; > + u32 val; > + > + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); > + raw_spin_lock_irqsave(&port->lock, flags); > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); > + pcie_write(port, (val & (~mask)), XILINX_CPM_PCIE_REG_IDRN_MASK); > + raw_spin_unlock_irqrestore(&port->lock, flags); > +} > + > +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data) > +{ > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(data); > + unsigned long flags; > + u32 mask; > + u32 val; > + > + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); > + raw_spin_lock_irqsave(&port->lock, flags); > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); > + pcie_write(port, (val | mask), XILINX_CPM_PCIE_REG_IDRN_MASK); > + raw_spin_unlock_irqrestore(&port->lock, flags); > +} > + > +static struct irq_chip xilinx_cpm_leg_irq_chip = { > + .name = "INTx", > + .irq_mask = xilinx_cpm_mask_leg_irq, > + .irq_unmask = xilinx_cpm_unmask_leg_irq, nit: Please align the assignments vertically: static struct irq_chip xilinx_cpm_leg_irq_chip = { .name = "INTx", .irq_mask = xilinx_cpm_mask_leg_irq, .irq_unmask = xilinx_cpm_unmask_leg_irq, }; > +}; > + > +/** > + * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark > IRQ as valid > + * @domain: IRQ domain > + * @irq: Virtual IRQ number > + * @hwirq: HW interrupt number > + * > + * Return: Always returns 0. > + */ > +static int xilinx_cpm_pcie_intx_map(struct irq_domain *domain, > + unsigned int irq, irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &xilinx_cpm_leg_irq_chip, > + handle_level_irq); > + irq_set_chip_data(irq, domain->host_data); > + irq_set_status_flags(irq, IRQ_LEVEL); > + > + return 0; > +} > + > +/* INTx IRQ Domain operations */ > +static const struct irq_domain_ops intx_domain_ops = { > + .map = xilinx_cpm_pcie_intx_map, > +}; > + > +static void xilinx_cpm_pcie_intx_flow(struct irq_desc *desc) > +{ > + struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long val; > + int i; > + > + chained_irq_enter(chip, desc); > + > + val = FIELD_GET(XILINX_CPM_PCIE_IDRN_MASK, > + pcie_read(port, XILINX_CPM_PCIE_REG_IDRN)); > + > + for_each_set_bit(i, &val, PCI_NUM_INTX) > + generic_handle_irq(irq_find_mapping(port->intx_domain, i)); > + > + chained_irq_exit(chip, desc); > +} > + > +static void xilinx_cpm_mask_event_irq(struct irq_data *d) > +{ > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); > + u32 val; > + > + raw_spin_lock(&port->lock); > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > + val &= ~d->hwirq; > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); > + raw_spin_unlock(&port->lock); > +} > + > +static void xilinx_cpm_unmask_event_irq(struct irq_data *d) > +{ > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); > + u32 val; > + > + raw_spin_lock(&port->lock); > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > + val |= d->hwirq; > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); > + raw_spin_unlock(&port->lock); > +} > + > +static struct irq_chip xilinx_cpm_event_irq_chip = { > + .name = "RC-Event", > + .irq_mask = xilinx_cpm_mask_event_irq, > + .irq_unmask = xilinx_cpm_unmask_event_irq, nit: Please use tabs between the field name and the = sign. > +}; > + > +static int xilinx_cpm_pcie_event_map(struct irq_domain *domain, > + unsigned int irq, irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &xilinx_cpm_event_irq_chip, > + handle_level_irq); > + irq_set_chip_data(irq, domain->host_data); > + irq_set_status_flags(irq, IRQ_LEVEL); > + return 0; > +} > + > +static const struct irq_domain_ops event_domain_ops = { > + .map = xilinx_cpm_pcie_event_map, > +}; > + > +static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) > +{ > + struct xilinx_cpm_pcie_port *port = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long val; > + int i; > + > + chained_irq_enter(chip, desc); > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR); > + val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > + for_each_set_bit(i, &val, 32) > + generic_handle_irq(irq_find_mapping(port->cpm_domain, i)); > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); > + > + /* > + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to > + * CPM SLCR block. > + */ > + val = readl_relaxed(port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS); > + if (val) > + writel_relaxed(val, > + port->cpm_base + XILINX_CPM_PCIE_MISC_IR_STATUS); The write_relaxed() call would be more readable on a single line. > + > + 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; > +} > + > +static void xilinx_cpm_free_irq_domains(struct xilinx_cpm_pcie_port > *port) > +{ > + if (port->intx_domain) { > + irq_domain_remove(port->intx_domain); > + port->intx_domain = NULL; > + } > + > + if (port->cpm_domain) { > + irq_domain_remove(port->cpm_domain); > + port->cpm_domain = NULL; > + } > +} > + > +/** > + * xilinx_cpm_pcie_init_irq_domain - Initialize IRQ domain > + * @port: PCIe port information > + * > + * Return: '0' on success and error value on failure > + */ > +static int xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port > *port) > +{ > + struct device *dev = port->dev; > + struct device_node *node = dev->of_node; > + struct device_node *pcie_intc_node; > + > + /* Setup INTx */ > + pcie_intc_node = of_get_next_child(node, NULL); > + if (!pcie_intc_node) { > + dev_err(dev, "No PCIe Intc node found\n"); > + return -EINVAL; > + } > + > + port->cpm_domain = irq_domain_add_linear(node, 32, No. The domain must be attached to the intc node, not to the RC. Why would you have this intc node otherwise? > + &event_domain_ops, > + port); > + if (!port->cpm_domain) > + goto out; > + > + irq_domain_update_bus_token(port->cpm_domain, DOMAIN_BUS_NEXUS); > + > + port->intx_domain = irq_domain_add_linear(pcie_intc_node, > PCI_NUM_INTX, > + &intx_domain_ops, > + port); > + if (!port->intx_domain) > + goto out; > + > + irq_domain_update_bus_token(port->intx_domain, DOMAIN_BUS_WIRED); > + > + of_node_put(pcie_intc_node); > + raw_spin_lock_init(&port->lock); > + > + return 0; > +out: > + xilinx_cpm_free_irq_domains(port); > + dev_err(dev, "Failed to allocate IRQ domains\n"); > + > + return -ENOMEM; > +} > + > +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) > +{ > + struct device *dev = port->dev; > + struct platform_device *pdev = to_platform_device(dev); > + int i, irq; > + > + port->irq = platform_get_irq(pdev, 0); > + if (port->irq < 0) { > + dev_err(dev, "Unable to find misc IRQ line\n"); > + return port->irq; > + } > + > + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) { > + int err; > + > + if (!intr_cause[i].str) > + continue; > + > + irq = irq_create_mapping(port->cpm_domain, i); > + if (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; > + } > + > + port->intx_irq = irq_create_mapping(port->cpm_domain, > + XILINX_CPM_PCIE_INTR_INTX); > + if (WARN_ON(port->intx_irq <= 0)) > + return -ENXIO; > + > + /* Plug the INTx chained handler */ > + irq_set_chained_handler_and_data(port->intx_irq, > + xilinx_cpm_pcie_intx_flow, port); > + > + /* Plug the main event chained handler */ > + irq_set_chained_handler_and_data(port->irq, > + xilinx_cpm_pcie_event_flow, port); > + > + return 0; > +} > + > +/** > + * xilinx_cpm_pcie_init_port - Initialize hardware > + * @port: PCIe port information > + */ > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port > *port) > +{ > + if (cpm_pcie_link_up(port)) > + dev_info(port->dev, "PCIe Link is UP\n"); > + else > + dev_info(port->dev, "PCIe Link is DOWN\n"); > + > + /* Disable all interrupts */ > + pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK, > + XILINX_CPM_PCIE_REG_IMR); > + > + /* Clear pending interrupts */ > + pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) & > + XILINX_CPM_PCIE_IMR_ALL_MASK, > + XILINX_CPM_PCIE_REG_IDR); > + > + /* 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. I've explained in the previous review why this was a terrible thing to do, and my patch got rid of it for a good reason. If the mask/unmask calls do not work, please explain what is wrong, and let's fix them. But we DO NOT enable interrupts outside of an irqchip callback, full stop. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-08 17:58 ` Marc Zyngier @ 2020-06-11 15:51 ` Bharat Kumar Gogada 2020-06-11 15:59 ` Marc Zyngier 0 siblings, 1 reply; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-11 15:51 UTC (permalink / raw) To: Marc Zyngier; +Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh > > Hi Bharat, > > On 2020-06-08 14:18, 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> > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > Please don't. Either you take my initial patch as is (potentially with reworks > that we discuss on the list), or you add a note saying that I suggested some > of the changes. But do not add my Sob without me agreeing to it, specially > when there are changes that I object to (see below). > Hi Marc Agreed, will add note. > > --- > > drivers/pci/controller/Kconfig | 8 + > > drivers/pci/controller/Makefile | 1 + > > drivers/pci/controller/pcie-xilinx-cpm.c | 621 > > +++++++++++++++++++++++++++++++ > > 3 files changed, 630 insertions(+) > > create mode 100644 drivers/pci/controller/pcie-xilinx-cpm.c > > > > diff --git a/drivers/pci/controller/Kconfig > > b/drivers/pci/controller/Kconfig index 20bf00f..d9e393a 100644 > > --- a/drivers/pci/controller/Kconfig > > +++ b/drivers/pci/controller/Kconfig > > @@ -81,6 +81,14 @@ config PCIE_XILINX > > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe > > Host Bridge driver. > > > > +config PCIE_XILINX_CPM > > + bool "Xilinx Versal CPM host bridge support" > > + depends on ARCH_ZYNQMP || COMPILE_TEST > > + select PCI_HOST_COMMON > > + help > > + Say 'Y' here if you want kernel support for the > > + Xilinx Versal CPM host bridge. > > + > > config PCI_XGENE > > bool "X-Gene PCIe controller" > > depends on ARM64 || COMPILE_TEST > > diff --git a/drivers/pci/controller/Makefile > > b/drivers/pci/controller/Makefile index 01b2502..78dabda 100644 > > --- a/drivers/pci/controller/Makefile > > +++ b/drivers/pci/controller/Makefile > > @@ -12,6 +12,7 @@ obj-$(CONFIG_PCI_HOST_COMMON) += pci-host- > common.o > > obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o > > obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o > > obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o > > +obj-$(CONFIG_PCIE_XILINX_CPM) += pcie-xilinx-cpm.o > > obj-$(CONFIG_PCI_V3_SEMI) += pci-v3-semi.o > > obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o > > obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o diff --git > > a/drivers/pci/controller/pcie-xilinx-cpm.c > > b/drivers/pci/controller/pcie-xilinx-cpm.c > > new file mode 100644 > > index 0000000..5bc481e > > --- /dev/null > > +++ b/drivers/pci/controller/pcie-xilinx-cpm.c > > @@ -0,0 +1,621 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * PCIe host controller driver for Xilinx Versal CPM DMA Bridge > > + * > > + * (C) Copyright 2019 - 2020, Xilinx, Inc. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/interrupt.h> > > +#include <linux/irq.h> > > +#include <linux/irqchip.h> > > +#include <linux/irqchip/chained_irq.h> #include <linux/irqdomain.h> > > +#include <linux/kernel.h> #include <linux/module.h> #include > > +<linux/of_address.h> #include <linux/of_pci.h> #include > > +<linux/of_platform.h> #include <linux/of_irq.h> #include > > +<linux/pci.h> #include <linux/platform_device.h> #include > > +<linux/pci-ecam.h> > > + > > +#include "../pci.h" > > + > > +/* Register definitions */ > > +#define XILINX_CPM_PCIE_REG_IDR 0x00000E10 > > +#define XILINX_CPM_PCIE_REG_IMR 0x00000E14 > > +#define XILINX_CPM_PCIE_REG_PSCR 0x00000E1C > > +#define XILINX_CPM_PCIE_REG_RPSC 0x00000E20 > > +#define XILINX_CPM_PCIE_REG_RPEFR 0x00000E2C > > +#define XILINX_CPM_PCIE_REG_IDRN 0x00000E38 > > +#define XILINX_CPM_PCIE_REG_IDRN_MASK 0x00000E3C > > +#define XILINX_CPM_PCIE_MISC_IR_STATUS 0x00000340 > > +#define XILINX_CPM_PCIE_MISC_IR_ENABLE 0x00000348 > > +#define XILINX_CPM_PCIE_MISC_IR_LOCAL BIT(1) > > + > > +/* Interrupt registers definitions */ > > +#define XILINX_CPM_PCIE_INTR_LINK_DOWN 0 > > +#define XILINX_CPM_PCIE_INTR_HOT_RESET 3 > > +#define XILINX_CPM_PCIE_INTR_CFG_PCIE_TIMEOUT 4 > > +#define XILINX_CPM_PCIE_INTR_CFG_TIMEOUT 8 > > +#define XILINX_CPM_PCIE_INTR_CORRECTABLE 9 > > +#define XILINX_CPM_PCIE_INTR_NONFATAL 10 > > +#define XILINX_CPM_PCIE_INTR_FATAL 11 > > +#define XILINX_CPM_PCIE_INTR_CFG_ERR_POISON 12 > > +#define XILINX_CPM_PCIE_INTR_PME_TO_ACK_RCVD 15 > > +#define XILINX_CPM_PCIE_INTR_INTX 16 > > +#define XILINX_CPM_PCIE_INTR_PM_PME_RCVD 17 > > +#define XILINX_CPM_PCIE_INTR_SLV_UNSUPP 20 > > +#define XILINX_CPM_PCIE_INTR_SLV_UNEXP 21 > > +#define XILINX_CPM_PCIE_INTR_SLV_COMPL 22 > > +#define XILINX_CPM_PCIE_INTR_SLV_ERRP 23 > > +#define XILINX_CPM_PCIE_INTR_SLV_CMPABT 24 > > +#define XILINX_CPM_PCIE_INTR_SLV_ILLBUR 25 > > +#define XILINX_CPM_PCIE_INTR_MST_DECERR 26 > > +#define XILINX_CPM_PCIE_INTR_MST_SLVERR 27 > > +#define XILINX_CPM_PCIE_INTR_SLV_PCIE_TIMEOUT 28 > > + > > +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) > > + > > +#define IMR(x) BIT(XILINX_CPM_PCIE_INTR_ ##x) > > Why do we have this twice? Will remove this. > > > + > > +#define XILINX_CPM_PCIE_IMR_ALL_MASK \ > > + ( \ > > + IMR(LINK_DOWN) | \ > > nit: Please use tabs between | and \. > Agreed, will fix it. > > + IMR(HOT_RESET) | \ > > + IMR(CFG_PCIE_TIMEOUT) | \ > > + IMR(CFG_TIMEOUT) | \ > > + IMR(CORRECTABLE) | \ > > + IMR(NONFATAL) | \ > > + IMR(FATAL) | \ > > + IMR(CFG_ERR_POISON) | \ > > + IMR(PME_TO_ACK_RCVD) | \ > > + IMR(INTX) | \ > > + IMR(PM_PME_RCVD) | \ > > + IMR(SLV_UNSUPP) | \ > > + IMR(SLV_UNEXP) | \ > > + IMR(SLV_COMPL) | \ > > + IMR(SLV_ERRP) | \ > > + IMR(SLV_CMPABT) | \ > > + IMR(SLV_ILLBUR) | \ > > + IMR(MST_DECERR) | \ > > + IMR(MST_SLVERR) | \ > > + IMR(SLV_PCIE_TIMEOUT) \ > > + ) > > + > > +#define XILINX_CPM_PCIE_IDR_ALL_MASK 0xFFFFFFFF > > +#define XILINX_CPM_PCIE_IDRN_MASK GENMASK(19, 16) > > +#define XILINX_CPM_PCIE_IDRN_SHIFT 16 > > + > > +/* Root Port Error FIFO Read Register definitions */ > > +#define XILINX_CPM_PCIE_RPEFR_ERR_VALID BIT(18) > > +#define XILINX_CPM_PCIE_RPEFR_REQ_ID GENMASK(15, 0) > > +#define XILINX_CPM_PCIE_RPEFR_ALL_MASK 0xFFFFFFFF > > + > > +/* Root Port Status/control Register definitions */ > > +#define XILINX_CPM_PCIE_REG_RPSC_BEN BIT(0) > > + > > +/* Phy Status/Control Register definitions */ > > +#define XILINX_CPM_PCIE_REG_PSCR_LNKUP BIT(11) > > + > > +/** > > + * struct xilinx_cpm_pcie_port - PCIe port information > > + * @reg_base: Bridge Register Base > > + * @cpm_base: CPM System Level Control and Status Register(SLCR) Base > > + * @dev: Device pointer > > + * @intx_domain: Legacy IRQ domain pointer > > + * @cfg: Holds mappings of config space window > > + * @intx_irq: legacy interrupt number > > + * @irq: Error interrupt number > > + * @lock: lock protecting shared register access */ struct > > +xilinx_cpm_pcie_port { > > + void __iomem *reg_base; > > + void __iomem *cpm_base; > > + struct device *dev; > > + struct irq_domain *intx_domain; > > + struct irq_domain *cpm_domain; > > + struct pci_config_window *cfg; > > + int intx_irq; > > + int irq; > > + raw_spinlock_t lock; > > +}; > > + > > +static u32 pcie_read(struct xilinx_cpm_pcie_port *port, u32 reg) { > > + return readl_relaxed(port->reg_base + reg); } > > + > > +static void pcie_write(struct xilinx_cpm_pcie_port *port, > > + u32 val, u32 reg) > > +{ > > + writel_relaxed(val, port->reg_base + reg); } > > + > > +static bool cpm_pcie_link_up(struct xilinx_cpm_pcie_port *port) { > > + return (pcie_read(port, XILINX_CPM_PCIE_REG_PSCR) & > > + XILINX_CPM_PCIE_REG_PSCR_LNKUP); > > +} > > + > > +/** > > + * xilinx_cpm_pcie_clear_err_interrupts - Clear Error Interrupts > > + * @port: PCIe port information > > + */ > > +static void cpm_pcie_clear_err_interrupts(struct xilinx_cpm_pcie_port > > *port) > > +{ > > + unsigned long val = pcie_read(port, XILINX_CPM_PCIE_REG_RPEFR); > > + > > + if (val & XILINX_CPM_PCIE_RPEFR_ERR_VALID) { > > + dev_dbg(port->dev, "Requester ID %lu\n", > > + val & XILINX_CPM_PCIE_RPEFR_REQ_ID); > > + pcie_write(port, XILINX_CPM_PCIE_RPEFR_ALL_MASK, > > + XILINX_CPM_PCIE_REG_RPEFR); > > + } > > +} > > + > > +static void xilinx_cpm_mask_leg_irq(struct irq_data *data) { > > + struct xilinx_cpm_pcie_port *port = > irq_data_get_irq_chip_data(data); > > + unsigned long flags; > > + u32 mask; > > + u32 val; > > + > > + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); > > + raw_spin_lock_irqsave(&port->lock, flags); > > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); > > + pcie_write(port, (val & (~mask)), > XILINX_CPM_PCIE_REG_IDRN_MASK); > > + raw_spin_unlock_irqrestore(&port->lock, flags); } > > + > > +static void xilinx_cpm_unmask_leg_irq(struct irq_data *data) { > > + struct xilinx_cpm_pcie_port *port = > irq_data_get_irq_chip_data(data); > > + unsigned long flags; > > + u32 mask; > > + u32 val; > > + > > + mask = BIT(data->hwirq + XILINX_CPM_PCIE_IDRN_SHIFT); > > + raw_spin_lock_irqsave(&port->lock, flags); > > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDRN_MASK); > > + pcie_write(port, (val | mask), XILINX_CPM_PCIE_REG_IDRN_MASK); > > + raw_spin_unlock_irqrestore(&port->lock, flags); } > > + > > +static struct irq_chip xilinx_cpm_leg_irq_chip = { > > + .name = "INTx", > > + .irq_mask = xilinx_cpm_mask_leg_irq, > > + .irq_unmask = xilinx_cpm_unmask_leg_irq, > > nit: Please align the assignments vertically: > Agreed, will fix it. > static struct irq_chip xilinx_cpm_leg_irq_chip = { > .name = "INTx", > .irq_mask = xilinx_cpm_mask_leg_irq, > .irq_unmask = xilinx_cpm_unmask_leg_irq, > }; > > > > +}; > > + > > +/** > > + * xilinx_cpm_pcie_intx_map - Set the handler for the INTx and mark > > IRQ as valid > > + * @domain: IRQ domain > > + * @irq: Virtual IRQ number > > + * @hwirq: HW interrupt number > > + * > > + * Return: Always returns 0. > > + */ > > +static int xilinx_cpm_pcie_intx_map(struct irq_domain *domain, > > + unsigned int irq, irq_hw_number_t hwirq) > { > > + irq_set_chip_and_handler(irq, &xilinx_cpm_leg_irq_chip, > > + handle_level_irq); > > + irq_set_chip_data(irq, domain->host_data); > > + irq_set_status_flags(irq, IRQ_LEVEL); > > + > > + return 0; > > +} > > + > > +/* INTx IRQ Domain operations */ > > +static const struct irq_domain_ops intx_domain_ops = { > > + .map = xilinx_cpm_pcie_intx_map, > > +}; > > + > > +static void xilinx_cpm_pcie_intx_flow(struct irq_desc *desc) { > > + struct xilinx_cpm_pcie_port *port = > irq_desc_get_handler_data(desc); > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + unsigned long val; > > + int i; > > + > > + chained_irq_enter(chip, desc); > > + > > + val = FIELD_GET(XILINX_CPM_PCIE_IDRN_MASK, > > + pcie_read(port, XILINX_CPM_PCIE_REG_IDRN)); > > + > > + for_each_set_bit(i, &val, PCI_NUM_INTX) > > + generic_handle_irq(irq_find_mapping(port->intx_domain, i)); > > + > > + chained_irq_exit(chip, desc); > > +} > > + > > +static void xilinx_cpm_mask_event_irq(struct irq_data *d) { > > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); > > + u32 val; > > + > > + raw_spin_lock(&port->lock); > > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > > + val &= ~d->hwirq; > > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); > > + raw_spin_unlock(&port->lock); > > +} > > + > > +static void xilinx_cpm_unmask_event_irq(struct irq_data *d) { > > + struct xilinx_cpm_pcie_port *port = irq_data_get_irq_chip_data(d); > > + u32 val; > > + > > + raw_spin_lock(&port->lock); > > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > > + val |= d->hwirq; > > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IMR); > > + raw_spin_unlock(&port->lock); > > +} > > + > > +static struct irq_chip xilinx_cpm_event_irq_chip = { > > + .name = "RC-Event", > > + .irq_mask = xilinx_cpm_mask_event_irq, > > + .irq_unmask = xilinx_cpm_unmask_event_irq, > > nit: Please use tabs between the field name and the = sign. > Agreed, will fix it. > > +}; > > + > > +static int xilinx_cpm_pcie_event_map(struct irq_domain *domain, > > + unsigned int irq, irq_hw_number_t hwirq) > { > > + irq_set_chip_and_handler(irq, &xilinx_cpm_event_irq_chip, > > + handle_level_irq); > > + irq_set_chip_data(irq, domain->host_data); > > + irq_set_status_flags(irq, IRQ_LEVEL); > > + return 0; > > +} > > + > > +static const struct irq_domain_ops event_domain_ops = { > > + .map = xilinx_cpm_pcie_event_map, > > +}; > > + > > +static void xilinx_cpm_pcie_event_flow(struct irq_desc *desc) { > > + struct xilinx_cpm_pcie_port *port = > irq_desc_get_handler_data(desc); > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + unsigned long val; > > + int i; > > + > > + chained_irq_enter(chip, desc); > > + val = pcie_read(port, XILINX_CPM_PCIE_REG_IDR); > > + val &= pcie_read(port, XILINX_CPM_PCIE_REG_IMR); > > + for_each_set_bit(i, &val, 32) > > + generic_handle_irq(irq_find_mapping(port->cpm_domain, > i)); > > + pcie_write(port, val, XILINX_CPM_PCIE_REG_IDR); > > + > > + /* > > + * XILINX_CPM_PCIE_MISC_IR_STATUS register is mapped to > > + * CPM SLCR block. > > + */ > > + val = readl_relaxed(port->cpm_base + > XILINX_CPM_PCIE_MISC_IR_STATUS); > > + if (val) > > + writel_relaxed(val, > > + port->cpm_base + > XILINX_CPM_PCIE_MISC_IR_STATUS); > > The write_relaxed() call would be more readable on a single line. > Agreed, will fix it. > > + > > + 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; > > +} > > + > > +static void xilinx_cpm_free_irq_domains(struct xilinx_cpm_pcie_port > > *port) > > +{ > > + if (port->intx_domain) { > > + irq_domain_remove(port->intx_domain); > > + port->intx_domain = NULL; > > + } > > + > > + if (port->cpm_domain) { > > + irq_domain_remove(port->cpm_domain); > > + port->cpm_domain = NULL; > > + } > > +} > > + > > +/** > > + * xilinx_cpm_pcie_init_irq_domain - Initialize IRQ domain > > + * @port: PCIe port information > > + * > > + * Return: '0' on success and error value on failure */ static int > > +xilinx_cpm_pcie_init_irq_domain(struct xilinx_cpm_pcie_port > > *port) > > +{ > > + struct device *dev = port->dev; > > + struct device_node *node = dev->of_node; > > + struct device_node *pcie_intc_node; > > + > > + /* Setup INTx */ > > + pcie_intc_node = of_get_next_child(node, NULL); > > + if (!pcie_intc_node) { > > + dev_err(dev, "No PCIe Intc node found\n"); > > + return -EINVAL; > > + } > > + > > + port->cpm_domain = irq_domain_add_linear(node, 32, > > No. The domain must be attached to the intc node, not to the RC. > Why would you have this intc node otherwise? Agreed, will fix it. > > > + &event_domain_ops, > > + port); > > + if (!port->cpm_domain) > > + goto out; > > + > > + irq_domain_update_bus_token(port->cpm_domain, > DOMAIN_BUS_NEXUS); > > + > > + port->intx_domain = irq_domain_add_linear(pcie_intc_node, > > PCI_NUM_INTX, > > + &intx_domain_ops, > > + port); > > + if (!port->intx_domain) > > + goto out; > > + > > + irq_domain_update_bus_token(port->intx_domain, > DOMAIN_BUS_WIRED); > > + > > + of_node_put(pcie_intc_node); > > + raw_spin_lock_init(&port->lock); > > + > > + return 0; > > +out: > > + xilinx_cpm_free_irq_domains(port); > > + dev_err(dev, "Failed to allocate IRQ domains\n"); > > + > > + return -ENOMEM; > > +} > > + > > +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) { > > + struct device *dev = port->dev; > > + struct platform_device *pdev = to_platform_device(dev); > > + int i, irq; > > + > > + port->irq = platform_get_irq(pdev, 0); > > + if (port->irq < 0) { > > + dev_err(dev, "Unable to find misc IRQ line\n"); > > + return port->irq; > > + } > > + > > + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) { > > + int err; > > + > > + if (!intr_cause[i].str) > > + continue; > > + > > + irq = irq_create_mapping(port->cpm_domain, i); > > + if (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; > > + } > > + > > + port->intx_irq = irq_create_mapping(port->cpm_domain, > > + XILINX_CPM_PCIE_INTR_INTX); > > + if (WARN_ON(port->intx_irq <= 0)) > > + return -ENXIO; > > + > > + /* Plug the INTx chained handler */ > > + irq_set_chained_handler_and_data(port->intx_irq, > > + xilinx_cpm_pcie_intx_flow, port); > > + > > + /* Plug the main event chained handler */ > > + irq_set_chained_handler_and_data(port->irq, > > + xilinx_cpm_pcie_event_flow, port); > > + > > + return 0; > > +} > > + > > +/** > > + * xilinx_cpm_pcie_init_port - Initialize hardware > > + * @port: PCIe port information > > + */ > > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port > > *port) > > +{ > > + if (cpm_pcie_link_up(port)) > > + dev_info(port->dev, "PCIe Link is UP\n"); > > + else > > + dev_info(port->dev, "PCIe Link is DOWN\n"); > > + > > + /* Disable all interrupts */ > > + pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK, > > + XILINX_CPM_PCIE_REG_IMR); > > + > > + /* Clear pending interrupts */ > > + pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) & > > + XILINX_CPM_PCIE_IMR_ALL_MASK, > > + XILINX_CPM_PCIE_REG_IDR); > > + > > + /* 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. I've explained in the previous review why this was a terrible thing to do, > and my patch got rid of it for a good reason. > > If the mask/unmask calls do not work, please explain what is wrong, and let's > fix them. But we DO NOT enable interrupts outside of an irqchip callback, full > stop. The issue here is, we do not have any other register to enable interrupts for above events, in order to see an interrupt assert for these events, the respective mask bits shall be set to 1. Regards, bharat ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-11 15:51 ` Bharat Kumar Gogada @ 2020-06-11 15:59 ` Marc Zyngier 2020-06-12 17:16 ` Bharat Kumar Gogada 0 siblings, 1 reply; 9+ messages in thread From: Marc Zyngier @ 2020-06-11 15:59 UTC (permalink / raw) To: Bharat Kumar Gogada Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh On 2020-06-11 16:51, Bharat Kumar Gogada wrote: [...] >> > +/** >> > + * 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. I've explained in the previous review why this was a terrible >> thing to do, >> and my patch got rid of it for a good reason. >> >> If the mask/unmask calls do not work, please explain what is wrong, >> and let's >> fix them. But we DO NOT enable interrupts outside of an irqchip >> callback, full >> stop. > The issue here is, we do not have any other register to enable > interrupts for above > events, in order to see an interrupt assert for these events, the > respective mask bits > shall be set to 1. I still disagree, because you're not explaining anything. We enable the interrupts as they are requested already (that's why we write to the these register in the respective mask/unmask callbacks). Why do you need to enable them ahead of the request? M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-11 15:59 ` Marc Zyngier @ 2020-06-12 17:16 ` Bharat Kumar Gogada 0 siblings, 0 replies; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-12 17:16 UTC (permalink / raw) To: Marc Zyngier; +Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh > Subject: Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver > > On 2020-06-11 16:51, Bharat Kumar Gogada wrote: > > [...] > > >> > +/** > >> > + * 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. I've explained in the previous review why this was a terrible > >> thing to do, and my patch got rid of it for a good reason. > >> > >> If the mask/unmask calls do not work, please explain what is wrong, > >> and let's fix them. But we DO NOT enable interrupts outside of an > >> irqchip callback, full stop. > > The issue here is, we do not have any other register to enable > > interrupts for above events, in order to see an interrupt assert for > > these events, the respective mask bits shall be set to 1. > > I still disagree, because you're not explaining anything. > > We enable the interrupts as they are requested already (that's why we write > to the these register in the respective mask/unmask callbacks). Why do you > need to enable them ahead of the request? HI Marc, Yes agreed, this is not needed. In xilinx_cpm_unmask_event_irq { ... val |= d->hwirq; //which needs to be BIT(d->hwirq) ... } Did not notice this earlier that the required bit is not being set here. Will fix it next patch. Regards, Bharat ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-08 13:18 ` [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada 2020-06-08 17:58 ` Marc Zyngier @ 2020-06-11 20:56 ` Bjorn Helgaas 2020-06-12 17:37 ` Bharat Kumar Gogada 1 sibling, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2020-06-11 20:56 UTC (permalink / raw) To: Bharat Kumar Gogada Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh, Marc Zyngier On Mon, Jun 08, 2020 at 06:48:58PM +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 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"); Maybe include d->hwirq in the "Unknown interrupt" message? I assume if we take the default case, there's no need to clear the interrupt? > + } > + > + return IRQ_HANDLED; > +} > +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) > +{ > + struct device *dev = port->dev; > + struct platform_device *pdev = to_platform_device(dev); > + int i, irq; > + > + port->irq = platform_get_irq(pdev, 0); > + if (port->irq < 0) { > + dev_err(dev, "Unable to find misc IRQ line\n"); platform_get_irq() already prints an error message; you probably don't need another. > + return port->irq; > + } > + > + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) { > + int err; > + > + if (!intr_cause[i].str) > + continue; > + > + irq = irq_create_mapping(port->cpm_domain, i); > + if (WARN_ON(irq <= 0)) I'm not a huge fan of WARN_ON() inside "if" statements, but ... OK. Why do these all need to be WARN_ON() instead of dev_warn()? > + 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; > + } > + > + port->intx_irq = irq_create_mapping(port->cpm_domain, > + XILINX_CPM_PCIE_INTR_INTX); > + if (WARN_ON(port->intx_irq <= 0)) > + return -ENXIO; > + > + /* Plug the INTx chained handler */ > + irq_set_chained_handler_and_data(port->intx_irq, > + xilinx_cpm_pcie_intx_flow, port); ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver 2020-06-11 20:56 ` Bjorn Helgaas @ 2020-06-12 17:37 ` Bharat Kumar Gogada 0 siblings, 0 replies; 9+ messages in thread From: Bharat Kumar Gogada @ 2020-06-12 17:37 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, linux-kernel, bhelgaas, lorenzo.pieralisi, robh, Marc Zyngier > Subject: Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver > > On Mon, Jun 08, 2020 at 06:48:58PM +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 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"); > > Maybe include d->hwirq in the "Unknown interrupt" message? Hi Bjorn, Yes, we can add. > > I assume if we take the default case, there's no need to clear the interrupt? It's being handled in xilinx_cpm_pcie_event_flow. > > > + } > > + > > + return IRQ_HANDLED; > > +} > > > +static int xilinx_cpm_setup_irq(struct xilinx_cpm_pcie_port *port) { > > + struct device *dev = port->dev; > > + struct platform_device *pdev = to_platform_device(dev); > > + int i, irq; > > + > > + port->irq = platform_get_irq(pdev, 0); > > + if (port->irq < 0) { > > + dev_err(dev, "Unable to find misc IRQ line\n"); > > platform_get_irq() already prints an error message; you probably don't need > another. > > > + return port->irq; > > + } > > + > > + for (i = 0; i < ARRAY_SIZE(intr_cause); i++) { > > + int err; > > + > > + if (!intr_cause[i].str) > > + continue; > > + > > + irq = irq_create_mapping(port->cpm_domain, i); > > + if (WARN_ON(irq <= 0)) > > I'm not a huge fan of WARN_ON() inside "if" statements, but ... OK. > > Why do these all need to be WARN_ON() instead of dev_warn()? Yes, it can be replaced with dev_warn(). Will fix these. Regards, Bharat > > > + 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; > > + } > > + > > + port->intx_irq = irq_create_mapping(port->cpm_domain, > > + XILINX_CPM_PCIE_INTR_INTX); > > + if (WARN_ON(port->intx_irq <= 0)) > > + return -ENXIO; > > + > > + /* Plug the INTx chained handler */ > > + irq_set_chained_handler_and_data(port->intx_irq, > > + xilinx_cpm_pcie_intx_flow, port); ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-12 17:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-08 13:18 [PATCH v8 0/2] Adding support for Versal CPM as Root Port driver Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 1/2] PCI: xilinx-cpm: Add YAML schemas for Versal CPM Root Port Bharat Kumar Gogada 2020-06-08 13:18 ` [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver Bharat Kumar Gogada 2020-06-08 17:58 ` Marc Zyngier 2020-06-11 15:51 ` Bharat Kumar Gogada 2020-06-11 15:59 ` Marc Zyngier 2020-06-12 17:16 ` Bharat Kumar Gogada 2020-06-11 20:56 ` Bjorn Helgaas 2020-06-12 17:37 ` 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).