Hi Bharat, On 06/10/15 16:44, Bharat Kumar Gogada wrote: > Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP. > > Signed-off-by: Bharat Kumar Gogada <bharatku@xxxxxxxxxx> > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xxxxxxxxxx> > --- > Added interrupt-map, interrupt-map-mask properties > --- > .../devicetree/bindings/pci/xilinx-nwl-pcie.txt | 56 ++ > drivers/pci/host/Kconfig | 9 + > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-xilinx-nwl.c | 1029 ++++++++++++++++++++ > 4 files changed, 1095 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt > create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c > > diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt > new file mode 100644 > index 0000000..81006ab > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt > @@ -0,0 +1,56 @@ > +* Xilinx NWL PCIe Root Port Bridge DT description > + > +Required properties: > +- compatible: Should contain "xlnx,nwl-pcie-2.11" > +- #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. > +- reg: Should contain Bridge, PCIe Controller registers location, > + configuration sapce, and length > +- reg-names: Must include the following entries: > + "breg": bridge registers > + "pcireg": PCIe controller registers > + "cfg": configuration space region > +- device_type: must be "pci" > +- interrupts: Should contain NWL PCIe interrupt > +- interrupt-names: Must include the following entries: > + "misc": interrupt asserted when miscellaneous is recieved > + "intx": interrupt that is asserted when an legacy interrupt is received > + "msi_1, msi_0": interrupt asserted when msi is recieved > +- interrupt-map-mask and 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 > + > +Optional properties: > +- xlnx,msi-fifo: To enable MSI FIFO mode > + - This feature can be used to queue multiple MSI interrupts > + > +Example: > +++++++++ > +nwl_pcie: pcie@fd0e0000 { > + compatible = "xlnx,nwl-pcie-2.11"; > + #address-cells = <3>; > + #size-cells = <2>; > + #interrupt-cells = <1>; > + device_type = "pci"; > + interrupt-parent = <&gic>; > + interrupts = < 0 118 4 > + 0 116 4 > + 0 115 4 // MSI_1 [63...32] > + 0 114 4 >; // MSI_0 [31...0] > + interrupt-names = "misc", "intx", "msi_1", "msi_0"; > + interrupt-map-mask = <0x0 0x0 0x0 0x7>; > + interrupt-map = <0x0 0x0 0x0 0x1 &gic 0x0 116 0x4 > + 0x0 0x0 0x0 0x2 &gic 0x0 116 0x4 > + 0x0 0x0 0x0 0x3 &gic 0x0 116 0x4 > + 0x0 0x0 0x0 0x4 &gic 0x0 116 0x4>; > + reg = <0x0 0xfd0e0000 0x1000 > + 0x0 0xfd480000 0x1000 > + 0x0 0xE0000000 0x1000000>; > + reg-names = "breg", "pcireg", "cfg"; > + ranges = <0x02000000 0x00000000 0xE1000000 0x00000000 0xE1000000 0 0x0F000000>; > +}; Please move this to a separate patch. This is big enough, and hard enough to review. > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index c132bdd..7f5f35e 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -15,6 +15,15 @@ config PCI_MVEBU > depends on ARCH_MVEBU || ARCH_DOVE > depends on OF > > +config PCIE_XILINX_NWL > + bool "NWL PCIe Core && PCI_MSI" > + depends on ARCH_ZYNQMP > + help > + Say 'Y' here if you want kernel to support for Xilinx > + NWL PCIe controller.The controller can act as Root Port > + or End Point.The current option selection will only Add a space after the dot. > + support root port enabling. > + > config PCIE_DW > bool > > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index 140d66f..6a56df7 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -10,6 +10,7 @@ obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o > obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o > obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o > obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o > +obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o > obj-$(CONFIG_PCI_XGENE) += pci-xgene.o > obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o > obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o > diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c > new file mode 100644 > index 0000000..ff25ec1 > --- /dev/null > +++ b/drivers/pci/host/pcie-xilinx-nwl.c > @@ -0,0 +1,1029 @@ > +/* > + * PCIe host controller driver for NWL PCIe Bridge > + * Based on pci-xilinx.c, pci-tegra.c > + * > + * (C) Copyright 2014 - 2015, Xilinx, Inc. > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/delay.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/msi.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> > + > +/* Bridge core config registers */ > +#define BRCFG_PCIE_RX0 0x00000000 > +#define BRCFG_PCIE_RX1 0x00000004 > +#define BRCFG_AXI_MASTER 0x00000008 > +#define BRCFG_PCIE_TX 0x0000000C > +#define BRCFG_INTERRUPT 0x00000010 > +#define BRCFG_RAM_DISABLE0 0x00000014 > +#define BRCFG_RAM_DISABLE1 0x00000018 > +#define BRCFG_PCIE_RELAXED_ORDER 0x0000001C > +#define BRCFG_PCIE_RX_MSG_FILTER 0x00000020 > + > +/* Attribute registers */ > +#define NWL_ATTRIB_100 0x00000190 > + > +/* Egress - Bridge translation registers */ > +#define E_BREG_CAPABILITIES 0x00000200 > +#define E_BREG_STATUS 0x00000204 > +#define E_BREG_CONTROL 0x00000208 > +#define E_BREG_BASE_LO 0x00000210 > +#define E_BREG_BASE_HI 0x00000214 > +#define E_ECAM_CAPABILITIES 0x00000220 > +#define E_ECAM_STATUS 0x00000224 > +#define E_ECAM_CONTROL 0x00000228 > +#define E_ECAM_BASE_LO 0x00000230 > +#define E_ECAM_BASE_HI 0x00000234 > + > +/* Ingress - address translations */ > +#define I_MSII_CAPABILITIES 0x00000300 > +#define I_MSII_CONTROL 0x00000308 > +#define I_MSII_BASE_LO 0x00000310 > +#define I_MSII_BASE_HI 0x00000314 > + > +#define I_ISUB_CONTROL 0x000003E8 > +#define SET_ISUB_CONTROL BIT(0) > +/* Rxed msg fifo - Interrupt status registers */ > +#define MSGF_MISC_STATUS 0x00000400 > +#define MSGF_MISC_MASK 0x00000404 > +#define MSGF_LEG_STATUS 0x00000420 > +#define MSGF_LEG_MASK 0x00000424 > +#define MSGF_MSI_STATUS_LO 0x00000440 > +#define MSGF_MSI_STATUS_HI 0x00000444 > +#define MSGF_MSI_MASK_LO 0x00000448 > +#define MSGF_MSI_MASK_HI 0x0000044C > +#define MSGF_RX_FIFO_POP 0x00000484 > +#define MSGF_RX_FIFO_TYPE 0x00000488 > +#define MSGF_RX_FIFO_ADDRLO 0x00000490 > +#define MSGF_RX_FIFO_ADDRHI 0x00000494 > +#define MSGF_RX_FIFO_DATA 0x00000498 > + > +/* Msg filter mask bits */ > +#define CFG_ENABLE_PM_MSG_FWD BIT(1) > +#define CFG_ENABLE_INT_MSG_FWD BIT(2) > +#define CFG_ENABLE_ERR_MSG_FWD BIT(3) > +#define CFG_ENABLE_SLT_MSG_FWD BIT(5) > +#define CFG_ENABLE_VEN_MSG_FWD BIT(7) > +#define CFG_ENABLE_OTH_MSG_FWD BIT(13) > +#define CFG_ENABLE_VEN_MSG_EN BIT(14) > +#define CFG_ENABLE_VEN_MSG_VEN_INV BIT(15) > +#define CFG_ENABLE_VEN_MSG_VEN_ID GENMASK(31, 16) > +#define CFG_ENABLE_MSG_FILTER_MASK (CFG_ENABLE_PM_MSG_FWD | \ > + CFG_ENABLE_INT_MSG_FWD | \ > + CFG_ENABLE_ERR_MSG_FWD | \ > + CFG_ENABLE_SLT_MSG_FWD | \ > + CFG_ENABLE_VEN_MSG_FWD | \ > + CFG_ENABLE_OTH_MSG_FWD | \ > + CFG_ENABLE_VEN_MSG_EN | \ > + CFG_ENABLE_VEN_MSG_VEN_INV | \ > + CFG_ENABLE_VEN_MSG_VEN_ID) > + > +/* Misc interrupt status mask bits */ > +#define MSGF_MISC_SR_RXMSG_AVAIL BIT(0) > +#define MSGF_MISC_SR_RXMSG_OVER BIT(1) > +#define MSGF_MISC_SR_SLAVE_ERR BIT(4) > +#define MSGF_MISC_SR_MASTER_ERR BIT(5) > +#define MSGF_MISC_SR_I_ADDR_ERR BIT(6) > +#define MSGF_MISC_SR_E_ADDR_ERR BIT(7) > + > +#define MSGF_MISC_SR_PCIE_CORE GENMASK(18, 16) > +#define MSGF_MISC_SR_PCIE_CORE_ERR GENMASK(31, 20) > + > +#define MSGF_MISC_SR_MASKALL (MSGF_MISC_SR_RXMSG_AVAIL | \ > + MSGF_MISC_SR_RXMSG_OVER | \ > + MSGF_MISC_SR_SLAVE_ERR | \ > + MSGF_MISC_SR_MASTER_ERR | \ > + MSGF_MISC_SR_I_ADDR_ERR | \ > + MSGF_MISC_SR_E_ADDR_ERR | \ > + MSGF_MISC_SR_PCIE_CORE | \ > + MSGF_MISC_SR_PCIE_CORE_ERR) > + > +/* Message rx fifo type mask bits */ > +#define MSGF_RX_FIFO_TYPE_MSI (1) > +#define MSGF_RX_FIFO_TYPE_TYPE GENMASK(1, 0) > + > +/* Legacy interrupt status mask bits */ > +#define MSGF_LEG_SR_INTA BIT(0) > +#define MSGF_LEG_SR_INTB BIT(1) > +#define MSGF_LEG_SR_INTC BIT(2) > +#define MSGF_LEG_SR_INTD BIT(3) > +#define MSGF_LEG_SR_MASKALL (MSGF_LEG_SR_INTA | MSGF_LEG_SR_INTB | \ > + MSGF_LEG_SR_INTC | MSGF_LEG_SR_INTD) > + > +/* MSI interrupt status mask bits */ > +#define MSGF_MSI_SR_LO_MASK BIT(0) > +#define MSGF_MSI_SR_HI_MASK BIT(0) > + > +#define MSII_PRESENT BIT(0) > +#define MSII_ENABLE BIT(0) > +#define MSII_STATUS_ENABLE BIT(15) > + > +/* Bridge config interrupt mask */ > +#define BRCFG_INTERRUPT_MASK BIT(0) > +#define BREG_PRESENT BIT(0) > +#define BREG_ENABLE BIT(0) > +#define BREG_ENABLE_FORCE BIT(1) > + > +/* E_ECAM status mask bits */ > +#define E_ECAM_PRESENT BIT(0) > +#define E_ECAM_SR_WR_PEND BIT(16) > +#define E_ECAM_SR_RD_PEND BIT(0) > +#define E_ECAM_SR_MASKALL (E_ECAM_SR_WR_PEND | E_ECAM_SR_RD_PEND) > +#define E_ECAM_CR_ENABLE BIT(0) > +#define E_ECAM_SIZE_LOC GENMASK(20, 16) > +#define E_ECAM_SIZE_SHIFT 16 > +#define ECAM_BUS_LOC_SHIFT 20 > +#define ECAM_DEV_LOC_SHIFT 12 > +#define NWL_ECAM_VALUE_DEFAULT 12 > +#define NWL_ECAM_SIZE_MIN 4096 > + > +#define ATTR_UPSTREAM_FACING BIT(6) > +#define CFG_DMA_REG_BAR GENMASK(2, 0) > + > +/* msgf_rx_fifo_pop bits */ > +#define MSGF_RX_FIFO_POP_POP BIT(0) > + > +#define INT_PCI_MSI_NR (2 * 32) > + > +/* Readin the PS_LINKUP */ > +#define PS_LINKUP_OFFSET 0x00000238 > +#define PCIE_PHY_LINKUP_BIT BIT(0) > +#define PHY_RDY_LINKUP_BIT BIT(1) > +#define PCIE_USER_LINKUP 0 > +#define PHY_RDY_LINKUP 1 > +#define LINKUP_ITER_CHECK 5 > + > +/* PCIE Message Request */ > +#define TX_PCIE_MSG 0x00000620 > +#define TX_PCIE_MSG_CNTL 0x00000004 > +#define TX_PCIE_MSG_SPEC_LO 0x00000008 > +#define TX_PCIE_MSG_SPEC_HI 0x0000000C > +#define TX_PCIE_MSG_DATA 0x00000010 > + > +#define MSG_BUSY_BIT BIT(8) > +#define MSG_EXECUTE_BIT BIT(0) > +#define MSG_DONE_BIT BIT(16) > +#define MSG_DONE_STATUS_BIT (BIT(25) | BIT(24)) > +#define RANDOM_DIGIT 0x11223344 > +#define PATTRN_SSLP_TLP 0x01005074 > + > +#define EXP_CAP_BASE 0x60 > + > +/* SSPL ERROR */ > +#define SLVERR 0x02 > +#define DECERR 0x03 > + > +struct nwl_msi { /* struct nwl_msi - MSI information */ > + struct msi_controller chip; /* chip: MSI controller */ We're moving away from msi_controller altogether, as the kernel now has all the necessary infrastructure to do this properly. Please convert this driver to msi domains (see drivers/pci/host/pci-xgene-msi.c or the gic-v2m driver as examples of how this is being done). > + DECLARE_BITMAP(used, INT_PCI_MSI_NR); /* used: Declare Bitmap > + for MSI */ > + struct irq_domain *domain; /* domain: IRQ domain pointer */ > + unsigned long pages; /* pages: MSI pages */ > + struct mutex lock; /* lock: mutex lock */ > + int irq_msi0; /* irq_msi0: msi0 interrupt number */ > + int irq_msi1; /* irq_msi1: msi1 interrupt number */ > +}; [...] > +static irqreturn_t nwl_pcie_misc_handler(int irq, void *data) > +{ > + struct nwl_pcie *pcie = (struct nwl_pcie *)data; > + u32 misc_stat; > + > + /* Checking for misc interrupts */ > + misc_stat = nwl_bridge_readl(pcie, MSGF_MISC_STATUS) & > + MSGF_MISC_SR_MASKALL; > + if (!misc_stat) > + return IRQ_NONE; > + > + if (misc_stat & MSGF_MISC_SR_RXMSG_OVER) > + dev_err(pcie->dev, "Received Message FIFO Overflow\n"); > + > + if (misc_stat & MSGF_MISC_SR_SLAVE_ERR) > + dev_err(pcie->dev, "Slave error\n"); > + > + if (misc_stat & MSGF_MISC_SR_MASTER_ERR) > + dev_err(pcie->dev, "Master error\n"); > + > + if (misc_stat & MSGF_MISC_SR_I_ADDR_ERR) > + dev_err(pcie->dev, > + "In Misc Ingress address translation error\n"); > + > + if (misc_stat & MSGF_MISC_SR_E_ADDR_ERR) > + dev_err(pcie->dev, > + "In Misc Egress address translation error\n"); > + > + if (misc_stat & MSGF_MISC_SR_PCIE_CORE_ERR) > + dev_err(pcie->dev, "PCIe Core error\n"); > + > + if (pcie->enable_msi_fifo) { > + if (misc_stat & MSGF_MISC_SR_RXMSG_AVAIL) { > + u32 msg_type = nwl_bridge_readl(pcie, > + MSGF_RX_FIFO_TYPE) & > + MSGF_RX_FIFO_TYPE_TYPE; > + > + if (msg_type == MSGF_RX_FIFO_TYPE_MSI) { > + u32 irq_msi; > + struct nwl_msi *msi = &pcie->msi; > + u32 msi_data = nwl_bridge_readl(pcie, > + MSGF_RX_FIFO_DATA); > + /* Let all ready be completed before write */ > + rmb(); > + /* POP the FIFO */ > + nwl_bridge_writel(pcie, MSGF_RX_FIFO_POP_POP, > + MSGF_RX_FIFO_POP); > + > + /* Handle the msi virtual interrupt */ > + irq_msi = irq_find_mapping(msi->domain, > + msi_data); > + > + if (irq_msi) { > + if (test_bit(msi_data, msi->used)) > + generic_handle_irq(irq_msi); > + else > + dev_info(pcie->dev, > + "unhandled MSI %d\n", > + irq_msi); No way. This implements a chained irqchip, not a normal IRQ handler. You can't just reenter the core code like this. Please convert this to a full chained irqchip. > + } else { > + dev_info(pcie->dev, "unexpected MSI\n"); > + } > + } > + } > + } > + /* Clear misc interrupt status */ > + nwl_bridge_writel(pcie, misc_stat, MSGF_MISC_STATUS); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t nwl_pcie_leg_handler(int irq, void *data) > +{ > + struct nwl_pcie *pcie = (struct nwl_pcie *)data; > + u32 leg_stat; > + > + /* Checking for legacy interrupts */ > + leg_stat = nwl_bridge_readl(pcie, MSGF_LEG_STATUS) & > + MSGF_LEG_SR_MASKALL; > + if (!leg_stat) > + return IRQ_NONE; > + > + if (leg_stat & MSGF_LEG_SR_INTA) > + dev_dbg(pcie->dev, "legacy interruptA\n"); > + > + if (leg_stat & MSGF_LEG_SR_INTB) > + dev_dbg(pcie->dev, "legacy interruptB\n"); > + > + if (leg_stat & MSGF_LEG_SR_INTC) > + dev_dbg(pcie->dev, "legacy interruptC\n"); > + > + if (leg_stat & MSGF_LEG_SR_INTD) > + dev_dbg(pcie->dev, "legacy interruptD\n"); > + > + return IRQ_HANDLED; And that's it? How useful is that? How do you make sure the line has been lowered (if I remember well, legacy interrupts are LEVEL). > +} > + > +static void __nwl_pcie_msi_handler(struct nwl_pcie *pcie, > + unsigned long reg, u32 val) > +{ > + struct nwl_msi *msi = &pcie->msi; > + unsigned int offset, index; > + int irq_msi; > + > + offset = find_first_bit(®, 32); > + index = offset; > + > + /* Clear the interrupt */ > + nwl_bridge_writel(pcie, 1 << offset, val); > + > + /* Handle the msi virtual interrupt */ > + irq_msi = irq_find_mapping(msi->domain, index); > + if (irq_msi) { > + if (test_bit(index, msi->used)) > + generic_handle_irq(irq_msi); > + else > + dev_info(pcie->dev, "unhandled MSI\n"); > + } else { > + dev_info(pcie->dev, "unexpected MSI\n"); > + } > +} I already commented on that, and on the need to using msi domains. > + > +static irqreturn_t nwl_pcie_msi_handler(int irq, void *data) > +{ > + struct nwl_pcie *pcie = data; > + unsigned long reg; > + int processed = 0; > + > + reg = nwl_bridge_readl(pcie, MSGF_MSI_STATUS_LO); > + if (reg) { > + __nwl_pcie_msi_handler(pcie, reg, MSGF_MSI_STATUS_LO); > + processed++; > + } > + > + reg = nwl_bridge_readl(pcie, MSGF_MSI_STATUS_HI); > + if (reg) { > + __nwl_pcie_msi_handler(pcie, reg, MSGF_MSI_STATUS_HI); > + processed++; > + } > + > + return processed > 0 ? IRQ_HANDLED : IRQ_NONE; > +} I think you really need to document what all these handlers are... > + > +static int nwl_msi_alloc(struct nwl_msi *chip) > +{ > + int msi; > + > + mutex_lock(&chip->lock); > + > + msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR); > + if (msi < INT_PCI_MSI_NR) > + set_bit(msi, chip->used); > + else > + msi = -ENOSPC; > + > + mutex_unlock(&chip->lock); > + > + return msi; > +} > + > +static void nwl_msi_free(struct nwl_msi *chip, unsigned long irq) > +{ > + struct device *dev = chip->chip.dev; > + > + mutex_lock(&chip->lock); > + > + if (!test_bit(irq, chip->used)) > + dev_err(dev, "trying to free unused MSI#%lu\n", irq); > + else > + clear_bit(irq, chip->used); > + > + mutex_unlock(&chip->lock); > +} > + > +static int nwl_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev, > + struct msi_desc *desc) > +{ > + struct nwl_msi *msi = to_nwl_msi(chip); > + struct msi_msg msg; > + unsigned int irq; > + int hwirq; > + > + /* currently we are not supporting MSIx */ > + if (desc->msi_attrib.is_msix) > + return -ENOSPC; > + > + hwirq = nwl_msi_alloc(msi); > + if (hwirq < 0) > + return hwirq; > + > + irq = irq_create_mapping(msi->domain, hwirq); > + if (!irq) > + return -EINVAL; > + > + irq_set_msi_desc(irq, desc); > + > + msg.address_lo = virt_to_phys((void *)msi->pages); > + /* 32 bit address only */ > + msg.address_hi = 0; > + msg.data = hwirq; > + > + write_msi_msg(irq, &msg); > + > + return 0; > +} > + > +static void nwl_msi_teardown_irq(struct msi_controller *chip, unsigned int irq) > +{ > + struct nwl_msi *msi = to_nwl_msi(chip); > + struct irq_data *d = irq_get_irq_data(irq); > + > + nwl_msi_free(msi, d->hwirq); > +} > + > +static struct irq_chip nwl_msi_irq_chip = { > + .name = "nwl_pcie:msi", > + .irq_enable = unmask_msi_irq, > + .irq_disable = mask_msi_irq, > + .irq_mask = mask_msi_irq, > + .irq_unmask = unmask_msi_irq, > +}; All that needs to go. > + > +static int nwl_msi_map(struct irq_domain *domain, unsigned int irq, > + irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &nwl_msi_irq_chip, handle_simple_irq); > + irq_set_chip_data(irq, domain->host_data); > + set_irq_flags(irq, IRQF_VALID); > + > + return 0; > +} > + > +static const struct irq_domain_ops msi_domain_ops = { > + .map = nwl_msi_map, > +}; > + > +static int nwl_pcie_enable_msi(struct nwl_pcie *pcie, struct pci_bus *bus) > +{ > + struct platform_device *pdev = to_platform_device(pcie->dev); > + struct nwl_msi *msi = &pcie->msi; > + unsigned long base; > + int ret; > + > + mutex_init(&msi->lock); > + > + /* Assign msi chip hooks */ > + msi->chip.dev = pcie->dev; > + msi->chip.setup_irq = nwl_msi_setup_irq; > + msi->chip.teardown_irq = nwl_msi_teardown_irq; > + > + bus->msi = &msi->chip; > + msi->domain = irq_domain_add_linear(pcie->dev->of_node, INT_PCI_MSI_NR, > + &msi_domain_ops, &msi->chip); > + if (!msi->domain) { > + dev_err(&pdev->dev, "failed to create IRQ domain\n"); > + return -ENOMEM; > + } > + > + /* Check for msii_present bit */ > + ret = nwl_bridge_readl(pcie, I_MSII_CAPABILITIES) & MSII_PRESENT; > + if (!ret) { > + dev_err(pcie->dev, "MSI not present\n"); > + ret = -EIO; > + goto err; > + } > + > + /* Enable MSII */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, I_MSII_CONTROL) | > + MSII_ENABLE, I_MSII_CONTROL); > + if (!pcie->enable_msi_fifo) > + /* Enable MSII status */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, I_MSII_CONTROL) | > + MSII_STATUS_ENABLE, I_MSII_CONTROL); > + > + /* setup AFI/FPCI range */ > + msi->pages = __get_free_pages(GFP_KERNEL, 0); > + base = virt_to_phys((void *)msi->pages); > + nwl_bridge_writel(pcie, base, I_MSII_BASE_LO); > + nwl_bridge_writel(pcie, 0x0, I_MSII_BASE_HI); > + > + /* Disable high range msi interrupts */ > + nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_HI_MASK, MSGF_MSI_MASK_HI); > + > + /* Clear pending high range msi interrupts */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, MSGF_MSI_STATUS_HI) & > + MSGF_MSI_SR_HI_MASK, MSGF_MSI_STATUS_HI); > + /* Get msi_1 IRQ number */ > + msi->irq_msi1 = platform_get_irq_byname(pdev, "msi_1"); > + if (msi->irq_msi1 < 0) { > + dev_err(&pdev->dev, "failed to get IRQ#%d\n", msi->irq_msi1); > + goto err; > + } > + /* Register msi handler */ > + ret = devm_request_irq(pcie->dev, msi->irq_msi1, nwl_pcie_msi_handler, > + 0, nwl_msi_irq_chip.name, pcie); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request IRQ#%d\n", > + msi->irq_msi1); > + goto err; > + } > + > + /* Enable all high range msi interrupts */ > + nwl_bridge_writel(pcie, MSGF_MSI_SR_HI_MASK, MSGF_MSI_MASK_HI); > + > + /* Disable low range msi interrupts */ > + nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_LO_MASK, MSGF_MSI_MASK_LO); > + > + /* Clear pending low range msi interrupts */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, MSGF_MSI_STATUS_LO) & > + MSGF_MSI_SR_LO_MASK, MSGF_MSI_STATUS_LO); > + /* Get msi_0 IRQ number */ > + msi->irq_msi0 = platform_get_irq_byname(pdev, "msi_0"); > + if (msi->irq_msi0 < 0) { > + dev_err(&pdev->dev, "failed to get IRQ#%d\n", msi->irq_msi0); > + goto err; > + } > + /* Register msi handler */ > + ret = devm_request_irq(pcie->dev, msi->irq_msi0, nwl_pcie_msi_handler, > + 0, nwl_msi_irq_chip.name, pcie); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to request IRQ#%d\n", > + msi->irq_msi0); > + goto err; > + } > + /* Enable all low range msi interrupts */ > + nwl_bridge_writel(pcie, MSGF_MSI_SR_LO_MASK, MSGF_MSI_MASK_LO); > + > + return 0; > +err: > + irq_domain_remove(msi->domain); > + return ret; > +} > + > +static int nwl_pcie_bridge_init(struct nwl_pcie *pcie) > +{ > + struct platform_device *pdev = to_platform_device(pcie->dev); > + u32 breg_val, ecam_val, first_busno = 0; > + int err; > + int check_link_up = 0; > + > + /* Check for BREG present bit */ > + breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT; > + if (!breg_val) { > + dev_err(pcie->dev, "BREG is not present\n"); > + return breg_val; > + } > + /* Write bridge_off to breg base */ > + nwl_bridge_writel(pcie, (u32)(pcie->phys_breg_base), > + E_BREG_BASE_LO); > + > + /* Enable BREG */ > + nwl_bridge_writel(pcie, ~BREG_ENABLE_FORCE & BREG_ENABLE, > + E_BREG_CONTROL); > + > + /* Disable DMA channel registers */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, BRCFG_PCIE_RX0) | > + CFG_DMA_REG_BAR, BRCFG_PCIE_RX0); > + > + /* Enable the bridge config interrupt */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, BRCFG_INTERRUPT) | > + BRCFG_INTERRUPT_MASK, BRCFG_INTERRUPT); > + /* Enable Ingress subtractive decode translation */ > + nwl_bridge_writel(pcie, SET_ISUB_CONTROL, I_ISUB_CONTROL); > + > + /* Enable msg filtering details */ > + nwl_bridge_writel(pcie, CFG_ENABLE_MSG_FILTER_MASK, > + BRCFG_PCIE_RX_MSG_FILTER); > + do { > + err = nwl_pcie_link_up(pcie, PHY_RDY_LINKUP); > + if (err != 1) { > + check_link_up++; > + if (check_link_up > LINKUP_ITER_CHECK) > + return -ENODEV; > + mdelay(1000); > + } > + } while (!err); > + > + /* Check for ECAM present bit */ > + ecam_val = nwl_bridge_readl(pcie, E_ECAM_CAPABILITIES) & E_ECAM_PRESENT; > + if (!ecam_val) { > + dev_err(pcie->dev, "ECAM is not present\n"); > + return ecam_val; > + } > + > + /* Enable ECAM */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) | > + E_ECAM_CR_ENABLE, E_ECAM_CONTROL); > + /* Write ecam_value on ecam_control */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) | > + (pcie->ecam_value << E_ECAM_SIZE_SHIFT), > + E_ECAM_CONTROL); > + /* Write phy_reg_base to ecam base */ > + nwl_bridge_writel(pcie, (u32)pcie->phys_ecam_base, E_ECAM_BASE_LO); > + > + /* Get bus range */ > + ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL); > + pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT; > + /* Write primary, secondary and subordinate bus numbers */ > + ecam_val = first_busno; > + ecam_val |= (first_busno + 1) << 8; > + ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT); > + writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS)); > + > + /* Check if PCIe link is up? */ > + pcie->link_up = nwl_pcie_link_up(pcie, PCIE_USER_LINKUP); > + if (!pcie->link_up) > + dev_info(pcie->dev, "Link is DOWN\n"); > + else > + dev_info(pcie->dev, "Link is UP\n"); > + > + /* Disable all misc interrupts */ > + nwl_bridge_writel(pcie, (u32)~MSGF_MISC_SR_MASKALL, MSGF_MISC_MASK); > + > + /* Clear pending misc interrupts */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, MSGF_MISC_STATUS) & > + MSGF_MISC_SR_MASKALL, MSGF_MISC_STATUS); > + > + /* Get misc IRQ number */ > + pcie->irq_misc = platform_get_irq_byname(pdev, "misc"); > + if (pcie->irq_misc < 0) { > + dev_err(&pdev->dev, "failed to get misc IRQ#%d\n", > + pcie->irq_misc); > + return pcie->irq_misc; > + } > + /* Register misc handler */ > + err = devm_request_irq(pcie->dev, pcie->irq_misc, > + nwl_pcie_misc_handler, IRQF_SHARED, > + "nwl_pcie:misc", pcie); > + if (err) { > + dev_err(pcie->dev, "fail to register misc IRQ#%d\n", > + pcie->irq_misc); > + return err; > + } > + /* Enable all misc interrupts */ > + nwl_bridge_writel(pcie, MSGF_MISC_SR_MASKALL, MSGF_MISC_MASK); > + > + /* Disable all legacy interrupts */ > + nwl_bridge_writel(pcie, (u32)~MSGF_LEG_SR_MASKALL, MSGF_LEG_MASK); > + > + /* Clear pending legacy interrupts */ > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, MSGF_LEG_STATUS) & > + MSGF_LEG_SR_MASKALL, MSGF_LEG_STATUS); > + /* Get intx IRQ number */ > + pcie->irq_intx = platform_get_irq_byname(pdev, "intx"); > + if (pcie->irq_intx < 0) { > + dev_err(&pdev->dev, "failed to get intx IRQ#%d\n", > + pcie->irq_intx); > + return pcie->irq_intx; > + } > + > + /* Register intx handler */ > + err = devm_request_irq(pcie->dev, pcie->irq_intx, > + nwl_pcie_leg_handler, IRQF_SHARED, > + "nwl_pcie:intx", pcie); > + if (err) { > + dev_err(pcie->dev, "fail to register intx IRQ#%d\n", > + pcie->irq_intx); > + return err; > + } > + /* Enable all legacy interrupts */ > + nwl_bridge_writel(pcie, MSGF_LEG_SR_MASKALL, MSGF_LEG_MASK); > + > + return 0; > +} > + > +static int nwl_pcie_parse_dt(struct nwl_pcie *pcie, > + struct platform_device *pdev) > +{ > + struct device_node *node = pcie->dev->of_node; > + struct resource *res; > + const char *type; > + > + /* Check for device type */ > + type = of_get_property(node, "device_type", NULL); > + if (!type || strcmp(type, "pci")) { > + dev_err(pcie->dev, "invalid \"device_type\" %s\n", type); > + return -EINVAL; > + } How can this fail? > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg"); > + pcie->breg_base = devm_ioremap_resource(pcie->dev, res); > + if (IS_ERR(pcie->breg_base)) > + return PTR_ERR(pcie->breg_base); > + pcie->phys_breg_base = res->start; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcireg"); > + pcie->pcireg_base = devm_ioremap_resource(pcie->dev, res); > + if (IS_ERR(pcie->pcireg_base)) > + return PTR_ERR(pcie->pcireg_base); > + pcie->phys_pcie_reg_base = res->start; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg"); > + pcie->ecam_base = devm_ioremap_resource(pcie->dev, res); > + if (IS_ERR(pcie->ecam_base)) > + return PTR_ERR(pcie->ecam_base); > + pcie->phys_ecam_base = res->start; > + > + pcie->enable_msi_fifo = of_property_read_bool(node, "xlnx,msi-fifo"); > + > + return 0; > +} > + > +static const struct of_device_id nwl_pcie_of_match[] = { > + { .compatible = "xlnx,nwl-pcie-2.11", }, > + {} > +}; > + > +static int nwl_pcie_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct nwl_pcie *pcie; > + struct pci_bus *bus; > + int err; > + > + resource_size_t iobase = 0; > + LIST_HEAD(res); > + > + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT; > + > + pcie->dev = &pdev->dev; > + > + err = nwl_pcie_parse_dt(pcie, pdev); > + if (err) { > + dev_err(pcie->dev, "Parsing DT failed\n"); > + return err; > + } > + /* Bridge initialization */ > + err = nwl_pcie_bridge_init(pcie); > + if (err) { > + dev_err(pcie->dev, "HW Initalization failed\n"); > + return err; > + } > + > + err = of_pci_get_host_bridge_resources(node, 0, 0xff, &res, &iobase); > + if (err) { > + pr_err("Getting bridge resources failed\n"); > + return err; > + } > + bus = pci_create_root_bus(&pdev->dev, 0, > + &nwl_pcie_ops, pcie, &res); > + if (!bus) > + return -ENOMEM; > + > + /* Enable MSI */ > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > + err = nwl_pcie_enable_msi(pcie, bus); > + if (err < 0) { > + dev_err(&pdev->dev, > + "failed to enable MSI support: %d\n", > + err); > + return err; > + } > + } > + pci_scan_child_bus(bus); > + pci_assign_unassigned_bus_resources(bus); > + pci_bus_add_devices(bus); > + platform_set_drvdata(pdev, pcie); > + > + return 0; > +} > + > +static int nwl_pcie_remove(struct platform_device *pdev) > +{ > + platform_set_drvdata(pdev, NULL); > + > + return 0; > +} > + > +static struct platform_driver nwl_pcie_driver = { > + .driver = { > + .name = "nwl-pcie", > + .of_match_table = nwl_pcie_of_match, > + }, > + .probe = nwl_pcie_probe, > + .remove = nwl_pcie_remove, > +}; > +module_platform_driver(nwl_pcie_driver); > + > +MODULE_AUTHOR("Xilinx, Inc"); > +MODULE_DESCRIPTION("NWL PCIe driver"); > +MODULE_LICENSE("GPL"); > -- > 1.7.1 Overall, I think it would make sense to split the PCIe controller driver and the MSI controller (in separate files as well). Please keep me posted on the updates. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html