| Message ID | 1513181317-19914-1-git-send-email-l.subrahmanya@mobiveil.co.in (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Wed, Dec 13, 2017 at 11:08:37AM -0500, Subrahmanya Lingappa wrote: > Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP - > GPEX 4.0, a PCIe gen4 IP. This IP has upto 8 > outbound and inbound windows for the address translation. > > Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Cc: linux-pci@vger.kernel.org > Cc: devicetree@vger.kernel.org > --- > drivers/pci/host/Kconfig | 8 + > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-mobiveil.c | 653 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 662 insertions(+) > create mode 100644 drivers/pci/host/pcie-mobiveil.c > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 38d1298..09bf1d9 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -27,6 +27,14 @@ config PCIE_XILINX_NWL > or End Point. The current option selection will only > support root port enabling. > > +config PCIE_MOBIVEIL > + bool "Mobiveil AXI PCIe host bridge support" > + depends on ARCH_ZYNQMP || COMPILE_TEST > + depends on PCI_MSI_IRQ_DOMAIN There is no PCI_MSI_IRQ_DOMAIN dependency in this patch. > + help > + Say 'Y' here if you want kernel to support the Mobiveil AXI PCIe > + Host Bridge driver. > + > config PCI_FTPCI100 > bool "Faraday Technology FTPCI100 PCI controller" > depends on OF > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index 34ec1d8..d745d68 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o > obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o > obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o > obj-$(CONFIG_VMD) += vmd.o > +obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o > > # The following drivers are for devices that use the generic ACPI > # pci_root.c driver but don't support standard ECAM config access. > diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c > new file mode 100644 > index 0000000..8611aaa > --- /dev/null > +++ b/drivers/pci/host/pcie-mobiveil.c > @@ -0,0 +1,653 @@ > +/* > + * PCIe host controller driver for Mobiveil PCIe Host controller > + * > + * SPDX-License-Identifier: GPL-2.0 > + * Copyright (c) 2017 Mobiveil Inc. > + * Author: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> > + */ > + > +#include <linux/delay.h> > +#include <linux/interrupt.h> > +#include <linux/init.h> > +#include <linux/irq.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_irq.h> > +#include <linux/of_platform.h> > +#include <linux/of_pci.h> > +#include <linux/pci.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +/* register offsets and bit positions */ > + > +/* > + * translation tables are grouped into windows, each window registers are > + * grouped into blocks of 4 or 16 registers each > + */ > +#define PAB_REG_BLOCK_SIZE_4 4 > +#define PAB_REG_BLOCK_SIZE_16 16 What I wanted to say is that you can tag the block with a name instead of using a number. I can say that block size 4 is only for PAB_EXT_*, you can tag it as such. eg PAB_EXT_REG_BLOCK_SIZE > +#define PAB_REG_ADDR_4(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_4)) > +#define PAB_REG_ADDR_16(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_16)) > + > +#define LTSSM_STATUS 0x0404 > +#define LTSSM_STATUS_L0_MASK 0x3f > +#define LTSSM_STATUS_L0 0x2d > + > +#define PAB_CTRL 0x0808 > +#define AMBA_PIO_ENABLE_SHIFT 0 > +#define PEX_PIO_ENABLE_SHIFT 1 > +#define PAGE_SEL_SHIFT 13 > +#define PAGE_SEL_MASK 0x3f > +#define PAGE_LO_MASK 0x3ff > +#define PAGE_SEL_EN 0xc00 > +#define PAGE_SEL_OFFSET_SHIFT 10 > + > +#define PAB_AXI_PIO_CTRL 0x0840 > +#define APIO_EN_MASK 0xf > + > +#define PAB_PEX_PIO_CTRL 0x08c0 > +#define PIO_ENABLE_SHIFT 0 > + > +#define PAB_INTP_AMBA_MISC_ENB 0x0b0c > +#define PAB_INTP_AMBA_MISC_STAT 0x0b1c > +#define PAB_INTP_INTX_MASK 0x1e0 > + > +#define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win) > +#define WIN_ENABLE_SHIFT 0 > +#define WIN_TYPE_SHIFT 1 > +#define WIN_SIZE_SHIFT 10 > + > +#define PAB_EXT_AXI_AMAP_SIZE(win) PAB_REG_ADDR_4(0xbaf0, win) > + > +#define PAB_AXI_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x0ba4, win) > +#define AXI_WINDOW_BASE_SHIFT 2 > + > +#define PAB_AXI_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x0ba8, win) > +#define PAB_BUS_SHIFT 24 > +#define PAB_DEVICE_SHIFT 19 > +#define PAB_FUNCTION_SHIFT 16 > + > +#define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x0bac, win) > +#define PAB_INTP_AXI_PIO_CLASS 0x474 This one is a bit odd here, move it up in the file. > +#define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR_16(0x4ba0, win) > +#define AMAP_CTRL_EN_SHIFT 0 > +#define AMAP_CTRL_TYPE_SHIFT 1 > + > +#define PAB_EXT_PEX_AMAP_SIZEN(win) PAB_REG_ADDR_4(0xbef0, win) > +#define PAB_PEX_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x4ba4, win) > +#define PAB_PEX_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x4ba8, win) > +#define PAB_PEX_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x4bac, win) Here you have three base offsets: 0xb00 0x4000 0xb000 you can create a macro for each of them, according to what they represent and then add an offset into each. > + > +/* supported number of interrupts */ > +#define PCI_NUM_INTX 4 It is already defined in linux/pci.h > +#define PAB_INTA_POS 5 > + > +/* outbound and inbound window definitions */ > +#define WIN_NUM_0 0 > +#define WIN_NUM_1 1 > +#define CFG_WINDOW_TYPE 0 > +#define IO_WINDOW_TYPE 1 > +#define MEM_WINDOW_TYPE 2 > +#define IB_WIN_SIZE (256 * 1024 * 1024) > +#define MAX_PIO_WINDOWS 8 > + > +/* Parameters for the waiting for link up routine */ > +#define LINK_WAIT_MAX_RETRIES 10 > +#define LINK_WAIT_MIN 90000 > +#define LINK_WAIT_MAX 100000 > + > +#ifndef UINT64_MAX > +#define UINT64_MAX (u64)(~((u64)0)) > +#endif > + > +struct mobiveil_pcie { > + struct platform_device *pdev; > + struct list_head resources; > + void __iomem *config_axi_slave_base; /* endpoint config base */ > + void __iomem *csr_axi_slave_base; /* root port config base */ > + struct irq_domain *intx_domain; > + int irq; > + int apio_wins; > + int ppio_wins; > + int ob_wins_configured; /* configured outbound windows */ > + int ib_wins_configured; /* configured inbound windows */ > + struct resource *ob_io_res; > + char root_bus_nr; > +}; > + > +static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value, > + const u32 reg) > +{ > + writel_relaxed(value, pcie->csr_axi_slave_base + reg); > +} > + > +static inline u32 csr_readl(struct mobiveil_pcie *pcie, const u32 reg) > +{ > + return readl_relaxed(pcie->csr_axi_slave_base + reg); > +} > + > +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie) > +{ > + return (csr_readl(pcie, LTSSM_STATUS) & > + LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0; > +} https://marc.info/?l=linux-pci&m=151329230814614&w=2 Bjorn would like to remove it. > +static bool mobiveil_pcie_valid_device(struct pci_bus *bus, unsigned int devfn) > +{ > + struct mobiveil_pcie *pcie = bus->sysdata; > + > + /* Check if link is up when trying to access downstream ports */ > + if (bus->number != pcie->root_bus_nr) > + if (!mobiveil_pcie_link_up(pcie)) > + return false; > + > + /* Only one device down on each root port */ > + if ((bus->number == pcie->root_bus_nr) && (devfn > 0)) > + return false; > + > + /* > + * Do not read more than one device on the bus directly > + * attached to RC > + */ > + if ((bus->primary == pcie->root_bus_nr) && (devfn > 0)) > + return false; > + > + return true; > +} > + > +/* > + * mobiveil_pcie_map_bus - routine to get the configuration base of either > + * root port or endpoint > + */ > +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus, > + unsigned int devfn, int where) > +{ > + struct mobiveil_pcie *pcie = bus->sysdata; > + > + if (!mobiveil_pcie_valid_device(bus, devfn)) > + return NULL; > + > + if (bus->number == pcie->root_bus_nr) { > + /* RC config access */ > + return pcie->csr_axi_slave_base + where; > + } else { > + /* > + * EP config access (in Config/APIO space) > + * Program PEX Address base (31..16 bits) with appropriate value > + * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register. > + * Relies on pci_lock serialization > + */ > + csr_writel(pcie, > + bus->number << PAB_BUS_SHIFT | > + PCI_SLOT(devfn) << PAB_DEVICE_SHIFT | > + PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT, > + PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0)); > + return pcie->config_axi_slave_base + where; > + } > +} > + > +static struct pci_ops mobiveil_pcie_ops = { > + .map_bus = mobiveil_pcie_map_bus, > + .read = pci_generic_config_read, > + .write = pci_generic_config_write, > +}; > + > +static void mobiveil_pcie_isr(struct irq_desc *desc) > +{ > + struct irq_chip *chip = irq_desc_get_chip(desc); > + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc); > + struct device *dev = &pcie->pdev->dev; > + u32 intr_status; > + unsigned long shifted_status; Why not u32 ? > + u32 bit, virq; > + u32 val, mask; > + > + chained_irq_enter(chip, desc); > + > + /* read INTx status */ > + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB); > + intr_status = val & mask; > + > + /* Handle INTx */ > + if (intr_status & PAB_INTP_INTX_MASK) { > + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); Why can't you reuse val to read the status and create shifted_status from it ? eg shifted_status = val << PAB_INTA_POS; csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT); Just a hint to make the loop more readable. BTW, I do not think that writing shifted_status into that register is OK, see below. > + do { > + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) { > + > + /* clear interrupts */ > + csr_writel(pcie, shifted_status << PAB_INTA_POS, > + PAB_INTP_AMBA_MISC_STAT); Should not you clear just the interrupt you are about to handle ? > + virq = irq_find_mapping(pcie->intx_domain, > + bit + 1); > + if (virq) > + generic_handle_irq(virq); > + else > + dev_err_ratelimited(dev, > + "unexpected IRQ, INT%d\n", > + bit); > + > + } > + > + shifted_status = csr_readl(pcie, > + PAB_INTP_AMBA_MISC_STAT); > + > + } while ((shifted_status >> PAB_INTA_POS) != 0); I do not understand this. Can you explain to me how the register at: PAB_INTP_AMBA_MISC_STAT works ? A patch for mediatek has been posted: https://patchwork.ozlabs.org/patch/851181/ It looks like this loop may suffer from the same issue, so please do have a look. On top of that, most of the operations you are carrying out here can be done automatically by making them part of the struct irq_chip methods (ie acking IRQs, masking IRQs, etc, see below). > + } > + > + csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT); > + chained_irq_exit(chip, desc); > +} > + > +static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie) > +{ > + struct device *dev = &pcie->pdev->dev; > + struct platform_device *pdev = pcie->pdev; > + struct device_node *node = dev->of_node; > + struct resource *res; > + const char *type; > + > + type = of_get_property(node, "device_type", NULL); > + if (!type || strcmp(type, "pci")) { > + dev_err(dev, "invalid \"device_type\" %s\n", type); > + return -EINVAL; > + } > + > + /* map config resource */ > + res = platform_get_resource_byname(pdev, > + IORESOURCE_MEM, "config_axi_slave"); > + pcie->config_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); > + if (IS_ERR(pcie->config_axi_slave_base)) > + return PTR_ERR(pcie->config_axi_slave_base); > + pcie->ob_io_res = res; > + > + /* map csr resource */ > + res = platform_get_resource_byname(pdev, > + IORESOURCE_MEM, "csr_axi_slave"); > + pcie->csr_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); > + if (IS_ERR(pcie->csr_axi_slave_base)) > + return PTR_ERR(pcie->csr_axi_slave_base); > + > + /* read the number of windows requested */ > + if (!pcie->apio_wins && > + of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) { > + pcie->apio_wins = MAX_PIO_WINDOWS; > + } > + > + if (!pcie->ppio_wins && What I wanted to say in my previous review is that apio_wins and ppio_wins are zero here why would they be any different and why do you need to check that. > + of_property_read_u32(node, "ppio-wins", &pcie->ppio_wins)) { > + pcie->ppio_wins = MAX_PIO_WINDOWS; > + } > + > + pcie->irq = platform_get_irq(pdev, 0); > + if (pcie->irq <= 0) { > + dev_err(dev, "failed to map IRQ: %d\n", pcie->irq); > + return -ENODEV; > + } > + > + irq_set_chained_handler_and_data(pcie->irq, mobiveil_pcie_isr, pcie); > + > + return 0; > +} > + > +/* > + * select_paged_register - routine to access paged register of root complex > + * > + * registers of RC are paged, for this scheme to work > + * extracted higher 6 bits of the offset will be written to pg_sel > + * field of PAB_CTRL register and rest of the lower 10 bits enabled with > + * PAGE_SEL_EN are used as offset of the register. > + * > + */ > +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset) > +{ > + int pab_ctrl_dw, pg_sel; > + > + /* clear pg_sel field */ > + pab_ctrl_dw = csr_readl(pcie, PAB_CTRL); > + pab_ctrl_dw = (pab_ctrl_dw & ~(PAGE_SEL_MASK << PAGE_SEL_SHIFT)); > + > + /* set pg_sel field */ > + pg_sel = (offset >> PAGE_SEL_OFFSET_SHIFT) & PAGE_SEL_MASK; > + pab_ctrl_dw |= ((pg_sel << PAGE_SEL_SHIFT)); > + csr_writel(pcie, pab_ctrl_dw, PAB_CTRL); > +} > + > +static void write_paged_register(struct mobiveil_pcie *pcie, > + u32 val, u32 offset) > +{ > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; > + > + select_paged_register(pcie, offset); > + csr_writel(pcie, val, off); > +} > + > +static u32 read_paged_register(struct mobiveil_pcie *pcie, u32 offset) > +{ > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; > + > + select_paged_register(pcie, offset); > + return csr_readl(pcie, off); > +} > + > +static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num, > + int pci_addr, int type, int size_kb) > +{ > + int pio_ctrl_val; > + int amap_ctrl_dw; > + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb))); > + > + if ((pcie->ib_wins_configured + 1) > pcie->ppio_wins) { > + dev_err(&pcie->pdev->dev, > + "ERROR: max inbound windows reached !\n"); > + return; > + } > + > + pio_ctrl_val = csr_readl(pcie, PAB_PEX_PIO_CTRL); > + csr_writel(pcie, > + pio_ctrl_val | (1 << PIO_ENABLE_SHIFT), PAB_PEX_PIO_CTRL); > + amap_ctrl_dw = read_paged_register(pcie, PAB_PEX_AMAP_CTRL(win_num)); > + amap_ctrl_dw = (amap_ctrl_dw | (type << AMAP_CTRL_TYPE_SHIFT)); > + amap_ctrl_dw = (amap_ctrl_dw | (1 << AMAP_CTRL_EN_SHIFT)); > + > + write_paged_register(pcie, amap_ctrl_dw | lower_32_bits(size64), > + PAB_PEX_AMAP_CTRL(win_num)); > + > + write_paged_register(pcie, upper_32_bits(size64), > + PAB_EXT_PEX_AMAP_SIZEN(win_num)); > + > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_AXI_WIN(win_num)); > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_PEX_WIN_L(win_num)); > + write_paged_register(pcie, 0, PAB_PEX_AMAP_PEX_WIN_H(win_num)); > +} > + > +static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num, > + u64 cpu_addr, u64 pci_addr, int config_io_bit, > + int size_kb) > +{ > + > + u32 value, type; > + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb))); Can you explain to me the sizing mechanism please ? It is just to make sure I got it right. > + if ((pcie->ob_wins_configured + 1) > pcie->apio_wins) { > + dev_err(&pcie->pdev->dev, > + "ERROR: max outbound windows reached !\n"); > + return; > + } > + > + /* > + * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit > + * to 4 KB in PAB_AXI_AMAP_CTRL register > + */ > + type = config_io_bit; > + value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num)); > + csr_writel(pcie, > + 1 << WIN_ENABLE_SHIFT | > + type << WIN_TYPE_SHIFT | > + lower_32_bits(size64), > + PAB_AXI_AMAP_CTRL(win_num)); > + > + write_paged_register(pcie, upper_32_bits(size64), > + PAB_EXT_AXI_AMAP_SIZE(win_num)); > + > + /* > + * program AXI window base with appropriate value in > + * PAB_AXI_AMAP_AXI_WIN0 register > + */ > + value = csr_readl(pcie, PAB_AXI_AMAP_AXI_WIN(win_num)); > + csr_writel(pcie, > + cpu_addr >> AXI_WINDOW_BASE_SHIFT << > + AXI_WINDOW_BASE_SHIFT, PAB_AXI_AMAP_AXI_WIN(win_num)); > + > + value = csr_readl(pcie, PAB_AXI_AMAP_PEX_WIN_H(win_num)); What's "value" used for in this function ? > + csr_writel(pcie, lower_32_bits(pci_addr), > + PAB_AXI_AMAP_PEX_WIN_L(win_num)); > + csr_writel(pcie, upper_32_bits(pci_addr), > + PAB_AXI_AMAP_PEX_WIN_H(win_num)); > + > + pcie->ob_wins_configured++; > +} > + > +static int mobiveil_bringup_link(struct mobiveil_pcie *pcie) > +{ > + int retries; > + > + /* check if the link is up or not */ > + for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) { > + if (mobiveil_pcie_link_up(pcie)) > + return 0; > + > + usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX); > + } > + dev_err(&pcie->pdev->dev, "link never came up\n"); > + return -ETIMEDOUT; > +} > + > +static int mobiveil_host_init(struct mobiveil_pcie *pcie) > +{ > + u32 value; > + int type = 0; You reinitialize it later and it should not be an int but an unsigned type. > + u32 pab_ctrl; > + int err; > + struct resource_entry *win, *tmp; > + > + /* setup the PCIe slot link power*/ Comment is useless, remove it. > + err = mobiveil_bringup_link(pcie); > + if (err) { > + dev_info(&pcie->pdev->dev, "link bring-up failed\n"); > + return err; > + } > + > + /* > + * program Bus Master Enable Bit in Command Register in PAB Config > + * Space > + */ > + value = csr_readl(pcie, PCI_COMMAND); > + csr_writel(pcie, > + value | Why is this on a separate line ? > + PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER, > + PCI_COMMAND); > + > + /* > + * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL > + * register > + */ > + pab_ctrl = csr_readl(pcie, PAB_CTRL); > + csr_writel(pcie, pab_ctrl | (1 << AMBA_PIO_ENABLE_SHIFT) | > + (1 << PEX_PIO_ENABLE_SHIFT), > + PAB_CTRL); > + > + csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB); > + > + /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in > + * PAB_AXI_PIO_CTRL Register > + */ > + value = csr_readl(pcie, PAB_AXI_PIO_CTRL); > + csr_writel(pcie, value | APIO_EN_MASK, PAB_AXI_PIO_CTRL); > + > + /* > + * we'll program one outbound window for config reads and > + * another default inbound window for all the upstream traffic > + * rest of the outbound windows will be configured according to > + * the "ranges" field defined in device tree > + */ > + > + /* config outbound translation window */ > + program_ob_windows(pcie, pcie->ob_wins_configured, > + pcie->ob_io_res->start, 0, CFG_WINDOW_TYPE, > + resource_size(pcie->ob_io_res)/1024); > + > + /* memory inbound translation window */ > + program_ib_windows(pcie, WIN_NUM_1, 0, MEM_WINDOW_TYPE, IB_WIN_SIZE); > + > + /* Get the I/O and memory ranges from DT */ > + resource_list_for_each_entry_safe(win, tmp, &pcie->resources) { > + type = 0; > + if (resource_type(win->res) == IORESOURCE_MEM) > + type = MEM_WINDOW_TYPE; > + if (resource_type(win->res) == IORESOURCE_IO) > + type = IO_WINDOW_TYPE; > + if (type) { > + /* configure outbound translation window */ > + program_ob_windows(pcie, pcie->ob_wins_configured, > + win->res->start, 0, type, > + resource_size(win->res)/1024); > + } > + } > + > + return err; > +} > + > +/* routine to setup the INTx related data */ > +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq, > + irq_hw_number_t hwirq) > +{ > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); IIUC, the host bridge acts as an INTX/MSI controller/multiplexer. So, instead of relying on dummy_irq_chip, you should create your own irq_chip with the methods that you require (eg irq_ack(), irq_mask(), irq_compose_msi_msg()) and use that as bottom irq_chip for both INTX and MSI domains. That's why I asked you to have a look at pcie-tango.c (except that there INTX aren't supported but the basic idea does not change) and implement IRQ domains handling like that same driver. > + irq_set_chip_data(irq, domain->host_data); > + return 0; > +} > + > +/* INTx domain opeartions structure */ s/opeartions/operations > +static const struct irq_domain_ops intx_domain_ops = { > + .map = mobiveil_pcie_intx_map, > +}; > + > +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie) > +{ > + struct device *dev = &pcie->pdev->dev; > + struct device_node *node = dev->of_node; > + int ret; ret is unused > + /* setup INTx */ > + pcie->intx_domain = irq_domain_add_linear(node, > + PCI_NUM_INTX + 1, &intx_domain_ops, pcie); You should use PCI_NUM_INTX and add pci_irqd_intx_xlate() as the domain ops .xlate. > + if (!pcie->intx_domain) { > + dev_err(dev, "Failed to get a INTx IRQ domain\n"); > + return -ENODEV; > + } > + > + return 0; > +} > + > +static int mobiveil_pcie_probe(struct platform_device *pdev) > +{ > + struct mobiveil_pcie *pcie; > + struct pci_bus *bus; > + struct pci_bus *child; > + struct pci_host_bridge *bridge; > + struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > + resource_size_t iobase; > + int ret; > + > + /* allocate the PCIe port */ > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > + if (!bridge) > + return -ENODEV; > + > + pcie = pci_host_bridge_priv(bridge); > + if (!pcie) > + return -ENOMEM; > + > + pcie->pdev = pdev; > + > + /* parse the device tree */ Remove this comment, it does not add anything. > + ret = mobiveil_pcie_parse_dt(pcie); > + if (ret) { > + dev_err(dev, "Parsing DT failed, ret: %x\n", ret); > + return ret; > + } > + > + INIT_LIST_HEAD(&pcie->resources); > + > + /* parse the host bridge base addresses from the device tree file */ > + ret = of_pci_get_host_bridge_resources(node, > + 0, 0xff, &pcie->resources, &iobase); > + if (ret) { > + dev_err(dev, "Getting bridge resources failed\n"); > + return -ENOMEM; > + } > + > + /* > + * configure all inbound and outbound windows and prepare the RC for > + * config access > + */ > + ret = mobiveil_host_init(pcie); > + if (ret) { > + dev_err(dev, "Failed to initialize host\n"); > + goto error; > + } > + > + /* fixup for PCIe class register */ > + csr_writel(pcie, 0x060402ab, PAB_INTP_AXI_PIO_CLASS); > + > + /* initialize the IRQ domains */ > + ret = mobiveil_pcie_init_irq_domain(pcie); > + if (ret) { > + dev_err(dev, "Failed creating IRQ Domain\n"); > + goto error; > + } > + > + Two empty lines, should be one. > + ret = devm_request_pci_bus_resources(dev, &pcie->resources); > + if (ret) > + goto error; > + > + /* Initialize bridge */ > + list_splice_init(&pcie->resources, &bridge->windows); > + bridge->dev.parent = dev; > + bridge->sysdata = pcie; > + bridge->busnr = pcie->root_bus_nr; > + bridge->ops = &mobiveil_pcie_ops; > + bridge->map_irq = of_irq_parse_and_map_pci; > + bridge->swizzle_irq = pci_common_swizzle; > + > + /* setup the kernel resources for the newly added PCIe root bus */ > + ret = pci_scan_root_bus_bridge(bridge); > + if (ret) > + goto error; > + > + bus = bridge->bus; > + > + pci_assign_unassigned_bus_resources(bus); > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + pci_bus_add_devices(bus); > + > + platform_set_drvdata(pdev, pcie); Is this really needed ? Thanks, Lorenzo > + > + return 0; > +error: > + pci_free_resource_list(&pcie->resources); > + return ret; > +} > + > +static const struct of_device_id mobiveil_pcie_of_match[] = { > + {.compatible = "mbvl,gpex40-pcie",}, > +}; > + > +MODULE_DEVICE_TABLE(of, mobiveil_pcie_of_match); > + > +static struct platform_driver mobiveil_pcie_driver = { > + .probe = mobiveil_pcie_probe, > + .driver = { > + .name = "mobiveil-pcie", > + .of_match_table = mobiveil_pcie_of_match, > + .suppress_bind_attrs = true, > + }, > +}; > + > +builtin_platform_driver(mobiveil_pcie_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Mobiveil PCIe host controller driver"); > +MODULE_AUTHOR("Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>"); > -- > 1.8.3.1 >
Lorenzo, On Wed, Dec 20, 2017 at 10:33 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Wed, Dec 13, 2017 at 11:08:37AM -0500, Subrahmanya Lingappa wrote: > > Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP - > > GPEX 4.0, a PCIe gen4 IP. This IP has upto 8 > > outbound and inbound windows for the address translation. > > > > Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Cc: Marc Zyngier <marc.zyngier@arm.com> > > Cc: linux-pci@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > --- > > drivers/pci/host/Kconfig | 8 + > > drivers/pci/host/Makefile | 1 + > > drivers/pci/host/pcie-mobiveil.c | 653 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 662 insertions(+) > > create mode 100644 drivers/pci/host/pcie-mobiveil.c > > > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > > index 38d1298..09bf1d9 100644 > > --- a/drivers/pci/host/Kconfig > > +++ b/drivers/pci/host/Kconfig > > @@ -27,6 +27,14 @@ config PCIE_XILINX_NWL > > or End Point. The current option selection will only > > support root port enabling. > > > > +config PCIE_MOBIVEIL > > + bool "Mobiveil AXI PCIe host bridge support" > > + depends on ARCH_ZYNQMP || COMPILE_TEST > > + depends on PCI_MSI_IRQ_DOMAIN > > There is no PCI_MSI_IRQ_DOMAIN dependency in this patch. > > > + help > > + Say 'Y' here if you want kernel to support the Mobiveil AXI PCIe > > + Host Bridge driver. > > + > > config PCI_FTPCI100 > > bool "Faraday Technology FTPCI100 PCI controller" > > depends on OF > > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > > index 34ec1d8..d745d68 100644 > > --- a/drivers/pci/host/Makefile > > +++ b/drivers/pci/host/Makefile > > @@ -23,6 +23,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o > > obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o > > obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o > > obj-$(CONFIG_VMD) += vmd.o > > +obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o > > > > # The following drivers are for devices that use the generic ACPI > > # pci_root.c driver but don't support standard ECAM config access. > > diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c > > new file mode 100644 > > index 0000000..8611aaa > > --- /dev/null > > +++ b/drivers/pci/host/pcie-mobiveil.c > > @@ -0,0 +1,653 @@ > > +/* > > + * PCIe host controller driver for Mobiveil PCIe Host controller > > + * > > + * SPDX-License-Identifier: GPL-2.0 > > + * Copyright (c) 2017 Mobiveil Inc. > > + * Author: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> > > + */ > > + > > +#include <linux/delay.h> > > +#include <linux/interrupt.h> > > +#include <linux/init.h> > > +#include <linux/irq.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_irq.h> > > +#include <linux/of_platform.h> > > +#include <linux/of_pci.h> > > +#include <linux/pci.h> > > +#include <linux/platform_device.h> > > +#include <linux/slab.h> > > + > > +/* register offsets and bit positions */ > > + > > +/* > > + * translation tables are grouped into windows, each window registers are > > + * grouped into blocks of 4 or 16 registers each > > + */ > > +#define PAB_REG_BLOCK_SIZE_4 4 > > +#define PAB_REG_BLOCK_SIZE_16 16 > > What I wanted to say is that you can tag the block with a name > instead of using a number. > > I can say that block size 4 is only for PAB_EXT_*, you can tag > it as such. > > eg PAB_EXT_REG_BLOCK_SIZE Ok, I'll rename them as below, and use them accordingly. #define PAB_REG_BLOCK_SIZE 16 #define PAB_EXT_REG_BLOCK_SIZE 4 > > > > +#define PAB_REG_ADDR_4(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_4)) > > +#define PAB_REG_ADDR_16(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_16)) > > + > > +#define LTSSM_STATUS 0x0404 > > +#define LTSSM_STATUS_L0_MASK 0x3f > > +#define LTSSM_STATUS_L0 0x2d > > + > > +#define PAB_CTRL 0x0808 > > +#define AMBA_PIO_ENABLE_SHIFT 0 > > +#define PEX_PIO_ENABLE_SHIFT 1 > > +#define PAGE_SEL_SHIFT 13 > > +#define PAGE_SEL_MASK 0x3f > > +#define PAGE_LO_MASK 0x3ff > > +#define PAGE_SEL_EN 0xc00 > > +#define PAGE_SEL_OFFSET_SHIFT 10 > > + > > +#define PAB_AXI_PIO_CTRL 0x0840 > > +#define APIO_EN_MASK 0xf > > + > > +#define PAB_PEX_PIO_CTRL 0x08c0 > > +#define PIO_ENABLE_SHIFT 0 > > + > > +#define PAB_INTP_AMBA_MISC_ENB 0x0b0c > > +#define PAB_INTP_AMBA_MISC_STAT 0x0b1c > > +#define PAB_INTP_INTX_MASK 0x1e0 > > + > > +#define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win) > > +#define WIN_ENABLE_SHIFT 0 > > +#define WIN_TYPE_SHIFT 1 > > +#define WIN_SIZE_SHIFT 10 > > + > > +#define PAB_EXT_AXI_AMAP_SIZE(win) PAB_REG_ADDR_4(0xbaf0, win) > > + > > +#define PAB_AXI_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x0ba4, win) > > +#define AXI_WINDOW_BASE_SHIFT 2 > > + > > +#define PAB_AXI_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x0ba8, win) > > +#define PAB_BUS_SHIFT 24 > > +#define PAB_DEVICE_SHIFT 19 > > +#define PAB_FUNCTION_SHIFT 16 > > + > > +#define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x0bac, win) > > +#define PAB_INTP_AXI_PIO_CLASS 0x474 > > This one is a bit odd here, move it up in the file. > > > +#define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR_16(0x4ba0, win) > > +#define AMAP_CTRL_EN_SHIFT 0 > > +#define AMAP_CTRL_TYPE_SHIFT 1 > > + > > +#define PAB_EXT_PEX_AMAP_SIZEN(win) PAB_REG_ADDR_4(0xbef0, win) > > +#define PAB_PEX_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x4ba4, win) > > +#define PAB_PEX_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x4ba8, win) > > +#define PAB_PEX_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x4bac, win) > > Here you have three base offsets: > > 0xb00 > 0x4000 > 0xb000 > > you can create a macro for each of them, according to what they > represent and then add an offset into each. > > > + > > +/* supported number of interrupts */ > > +#define PCI_NUM_INTX 4 > > It is already defined in linux/pci.h > > > +#define PAB_INTA_POS 5 > > + > > +/* outbound and inbound window definitions */ > > +#define WIN_NUM_0 0 > > +#define WIN_NUM_1 1 > > +#define CFG_WINDOW_TYPE 0 > > +#define IO_WINDOW_TYPE 1 > > +#define MEM_WINDOW_TYPE 2 > > +#define IB_WIN_SIZE (256 * 1024 * 1024) > > +#define MAX_PIO_WINDOWS 8 > > + > > +/* Parameters for the waiting for link up routine */ > > +#define LINK_WAIT_MAX_RETRIES 10 > > +#define LINK_WAIT_MIN 90000 > > +#define LINK_WAIT_MAX 100000 > > + > > +#ifndef UINT64_MAX > > +#define UINT64_MAX (u64)(~((u64)0)) > > +#endif > > + > > +struct mobiveil_pcie { > > + struct platform_device *pdev; > > + struct list_head resources; > > + void __iomem *config_axi_slave_base; /* endpoint config base */ > > + void __iomem *csr_axi_slave_base; /* root port config base */ > > + struct irq_domain *intx_domain; > > + int irq; > > + int apio_wins; > > + int ppio_wins; > > + int ob_wins_configured; /* configured outbound windows */ > > + int ib_wins_configured; /* configured inbound windows */ > > + struct resource *ob_io_res; > > + char root_bus_nr; > > +}; > > + > > +static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value, > > + const u32 reg) > > +{ > > + writel_relaxed(value, pcie->csr_axi_slave_base + reg); > > +} > > + > > +static inline u32 csr_readl(struct mobiveil_pcie *pcie, const u32 reg) > > +{ > > + return readl_relaxed(pcie->csr_axi_slave_base + reg); > > +} > > + > > +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie) > > +{ > > + return (csr_readl(pcie, LTSSM_STATUS) & > > + LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0; > > +} > > https://marc.info/?l=linux-pci&m=151329230814614&w=2 > > Bjorn would like to remove it. I do not have a solid explanation for this, as I followed the earlier driver framework. I'll remove it for now. > > > > +static bool mobiveil_pcie_valid_device(struct pci_bus *bus, unsigned int devfn) > > +{ > > + struct mobiveil_pcie *pcie = bus->sysdata; > > + > > + /* Check if link is up when trying to access downstream ports */ > > + if (bus->number != pcie->root_bus_nr) > > + if (!mobiveil_pcie_link_up(pcie)) > > + return false; > > + > > + /* Only one device down on each root port */ > > + if ((bus->number == pcie->root_bus_nr) && (devfn > 0)) > > + return false; > > + > > + /* > > + * Do not read more than one device on the bus directly > > + * attached to RC > > + */ > > + if ((bus->primary == pcie->root_bus_nr) && (devfn > 0)) > > + return false; > > + > > + return true; > > +} > > + > > +/* > > + * mobiveil_pcie_map_bus - routine to get the configuration base of either > > + * root port or endpoint > > + */ > > +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus, > > + unsigned int devfn, int where) > > +{ > > + struct mobiveil_pcie *pcie = bus->sysdata; > > + > > + if (!mobiveil_pcie_valid_device(bus, devfn)) > > + return NULL; > > + > > + if (bus->number == pcie->root_bus_nr) { > > + /* RC config access */ > > + return pcie->csr_axi_slave_base + where; > > + } else { > > + /* > > + * EP config access (in Config/APIO space) > > + * Program PEX Address base (31..16 bits) with appropriate value > > + * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register. > > + * Relies on pci_lock serialization > > + */ > > + csr_writel(pcie, > > + bus->number << PAB_BUS_SHIFT | > > + PCI_SLOT(devfn) << PAB_DEVICE_SHIFT | > > + PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT, > > + PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0)); > > + return pcie->config_axi_slave_base + where; > > + } > > +} > > + > > +static struct pci_ops mobiveil_pcie_ops = { > > + .map_bus = mobiveil_pcie_map_bus, > > + .read = pci_generic_config_read, > > + .write = pci_generic_config_write, > > +}; > > + > > +static void mobiveil_pcie_isr(struct irq_desc *desc) > > +{ > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc); > > + struct device *dev = &pcie->pdev->dev; > > + u32 intr_status; > > + unsigned long shifted_status; > > Why not u32 ? > for_each_set_bit() api warns about the variable not being unsigned long. So used the same to take out the warning. > > + u32 bit, virq; > > + u32 val, mask; > > + > > + chained_irq_enter(chip, desc); > > + > > + /* read INTx status */ > > + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB); > > + intr_status = val & mask; > > + > > + /* > Handle INTx */ > > + if (intr_status & PAB_INTP_INTX_MASK) { > > + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > Why can't you reuse val to read the status and create shifted_status > from it ? > > eg > > shifted_status = val << PAB_INTA_POS; > csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT); > > Just a hint to make the loop more readable. BTW, I do not think that > writing shifted_status into that register is OK, see below. > > > + do { > > + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) { > > + > > + /* clear interrupts */ > > + csr_writel(pcie, shifted_status << > PAB_INTA_POS, > > + > PAB_INTP_AMBA_MISC_STAT); > > Should not you clear just the interrupt you are about to handle ? PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status bits at positions 5,6,7 and 8, they are RW1C bits, so writing 1 to this bit clears the status. So here the status read was written back to it to clear. > > > > + virq = irq_find_mapping(pcie->intx_domain, > > + bit + 1); > > + if (virq) > > + > generic_handle_irq(virq); > > + else > > + dev_err_ratelimited(dev, > > + "unexpected IRQ, INT%d\n", > > + bit); > > + > > + } > > + > > + shifted_status = csr_readl(pcie, > > + PAB_INTP_AMBA_MISC_STAT); > > + > > + } while ((shifted_status >> PAB_INTA_POS) != 0); > > I do not understand this. Can you explain to me how the > register at: > > PAB_INTP_AMBA_MISC_STAT > > works ? > just repeating what explained before, to ease the readablility. PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status bits at positions 5,6,7 and 8, they are RW1C bits, so writing 1 to this bit clears the status. > > A patch for mediatek has been posted: > > https://patchwork.ozlabs.org/patch/851181/ > > It looks like this loop may suffer from the same issue, so please do > have a look. > I will clear the the interrupt after its hadled by generic_handle_irq() . right ? > > On top of that, most of the operations you are carrying out here > can be done automatically by making them part of the struct irq_chip > methods (ie acking IRQs, masking IRQs, etc, see below). > Any side effects of keeping this code as is ? > > > + } > > + > > + csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT); > > > + chained_irq_exit(chip, desc); > > +} > > + > > +static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie) > > +{ > > + struct device *dev = &pcie->pdev->dev; > > + struct platform_device *pdev = pcie->pdev; > > + struct device_node *node = dev->of_node; > > + struct resource *res; > > + const char *type; > > + > > + type = of_get_property(node, "device_type", NULL); > > + if (!type || strcmp(type, "pci")) { > > + dev_err(dev, "invalid \"device_type\" %s\n", type); > > + return -EINVAL; > > + } > > + > > + /* map config resource */ > > + res = platform_get_resource_byname(pdev, > > + IORESOURCE_MEM, "config_axi_slave"); > > + pcie->config_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); > > + if (IS_ERR(pcie->config_axi_slave_base)) > > + return PTR_ERR(pcie->config_axi_slave_base); > > + pcie->ob_io_res = res; > > + > > + /* map csr resource */ > > + res = platform_get_resource_byname(pdev, > > + IORESOURCE_MEM, "csr_axi_slave"); > > + pcie->csr_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); > > + if (IS_ERR(pcie->csr_axi_slave_base)) > > + return PTR_ERR(pcie->csr_axi_slave_base); > > + > > + /* read the number of windows requested */ > > + if (!pcie->apio_wins && > > + of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) { > > + pcie->apio_wins = MAX_PIO_WINDOWS; > > + } > > + > > + if (!pcie->ppio_wins && > > What I wanted to say in my previous review is that apio_wins and > ppio_wins are zero here why would they be any different and why > do you need to check that. Understood. I will remove this redundant check. > > > > + of_property_read_u32(node, "ppio-wins", &pcie->ppio_wins)) { > > + pcie->ppio_wins = MAX_PIO_WINDOWS; > > + } > > + > > + pcie->irq = platform_get_irq(pdev, 0); > > + if (pcie->irq <= 0) { > > + dev_err(dev, "failed to map IRQ: %d\n", pcie->irq); > > + return -ENODEV; > > + } > > + > > + irq_set_chained_handler_and_data(pcie->irq, mobiveil_pcie_isr, pcie); > > + > > + return 0; > > +} > > + > > +/* > > + * select_paged_register - routine to access paged register of root complex > > + * > > + * registers of RC are paged, for this scheme to work > > + * extracted higher 6 bits of the offset will be written to pg_sel > > + * field of PAB_CTRL register and rest of the lower 10 bits enabled with > > + * PAGE_SEL_EN are used as offset of the register. > > + * > > + */ > > +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset) > > +{ > > + int pab_ctrl_dw, pg_sel; > > + > > + /* clear pg_sel field */ > > + pab_ctrl_dw = csr_readl(pcie, PAB_CTRL); > > + pab_ctrl_dw = (pab_ctrl_dw & ~(PAGE_SEL_MASK << PAGE_SEL_SHIFT)); > > + > > + /* set pg_sel field */ > > + pg_sel = (offset >> PAGE_SEL_OFFSET_SHIFT) & PAGE_SEL_MASK; > > + pab_ctrl_dw |= ((pg_sel << PAGE_SEL_SHIFT)); > > + csr_writel(pcie, pab_ctrl_dw, PAB_CTRL); > > +} > > + > > +static void write_paged_register(struct mobiveil_pcie *pcie, > > + u32 val, u32 offset) > > +{ > > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; > > + > > + select_paged_register(pcie, offset); > > + csr_writel(pcie, val, off); > > +} > > + > > +static u32 read_paged_register(struct mobiveil_pcie *pcie, u32 offset) > > +{ > > + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; > > + > > + select_paged_register(pcie, offset); > > + return csr_readl(pcie, off); > > +} > > + > > +static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num, > > + int pci_addr, int type, int size_kb) > > +{ > > + int pio_ctrl_val; > > + int amap_ctrl_dw; > > + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb))); > > + > > + if ((pcie->ib_wins_configured + 1) > pcie->ppio_wins) { > > + dev_err(&pcie->pdev->dev, > > + "ERROR: max inbound windows reached !\n"); > > + return; > > + } > > + > > + pio_ctrl_val = csr_readl(pcie, PAB_PEX_PIO_CTRL); > > + csr_writel(pcie, > > + pio_ctrl_val | (1 << PIO_ENABLE_SHIFT), PAB_PEX_PIO_CTRL); > > + amap_ctrl_dw = read_paged_register(pcie, PAB_PEX_AMAP_CTRL(win_num)); > > + amap_ctrl_dw = (amap_ctrl_dw | (type << AMAP_CTRL_TYPE_SHIFT)); > > + amap_ctrl_dw = (amap_ctrl_dw | (1 << AMAP_CTRL_EN_SHIFT)); > > + > > + write_paged_register(pcie, amap_ctrl_dw | lower_32_bits(size64), > > + PAB_PEX_AMAP_CTRL(win_num)); > > + > > + write_paged_register(pcie, upper_32_bits(size64), > > + PAB_EXT_PEX_AMAP_SIZEN(win_num)); > > + > > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_AXI_WIN(win_num)); > > + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_PEX_WIN_L(win_num)); > > + write_paged_register(pcie, 0, PAB_PEX_AMAP_PEX_WIN_H(win_num)); > > +} > > + > > +static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num, > > + u64 cpu_addr, u64 pci_addr, int config_io_bit, > > + int size_kb) > > +{ > > + > > + u32 value, type; > > + u64 > size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + > ilog2(size_kb))); > > Can you explain to me the sizing mechanism please ? It is just to make > sure I got it right. this register follows the PCI BAR sizing convention. size64 is actually ~(size-1), Looks like we can just do that instead of jugglery of bit shifting and ilog2(). > > > > + if ((pcie->ob_wins_configured + 1) > pcie->apio_wins) { > > + dev_err(&pcie->pdev->dev, > > + "ERROR: max outbound windows reached !\n"); > > + return; > > + } > > + > > + /* > > + * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit > > + * to 4 KB in PAB_AXI_AMAP_CTRL register > > + */ > > + type = config_io_bit; > > + value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num)); > > + csr_writel(pcie, > > + 1 << WIN_ENABLE_SHIFT | > > + type << WIN_TYPE_SHIFT | > > + lower_32_bits(size64), > > + PAB_AXI_AMAP_CTRL(win_num)); > > + > > + write_paged_register(pcie, upper_32_bits(size64), > > + PAB_EXT_AXI_AMAP_SIZE(win_num)); > > + > > + /* > > + * > program > AXI window base with appropriate value in > > + * PAB_AXI_AMAP_AXI_WIN0 register > > + */ > > + value = csr_readl(pcie, PAB_AXI_AMAP_AXI_WIN(win_num)); > > + csr_writel(pcie, > > + cpu_addr >> AXI_WINDOW_BASE_SHIFT << > > + AXI_WINDOW_BASE_SHIFT, PAB_AXI_AMAP_AXI_WIN(win_num)); > > + > > + value = csr_readl(pcie, PAB_AXI_AMAP_PEX_WIN_H(win_num)); > > What's "value" used for in this function ? My bad, it was used in the previous versions and not cleaned up. I will now. > > > > > + csr_writel(pcie, lower_32_bits(pci_addr), > > + PAB_AXI_AMAP_PEX_WIN_L(win_num)); > > + csr_writel(pcie, upper_32_bits(pci_addr), > > + PAB_AXI_AMAP_PEX_WIN_H(win_num)); > > + > > + pcie->ob_wins_configured++; > > +} > > + > > +static int mobiveil_bringup_link(struct mobiveil_pcie *pcie) > > +{ > > + int retries; > > + > > + /* check if the link is up or not */ > > + for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) { > > + if (mobiveil_pcie_link_up(pcie)) > > + return 0; > > + > > + usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX); > > + } > > + dev_err(&pcie->pdev->dev, "link never came up\n"); > > + return -ETIMEDOUT; > > +} > > + > > +static int > mobiveil_host_init(struct mobiveil_pcie *pcie) > > +{ > > + u32 value; > > + int type = 0; > > You reinitialize it later and it should not be an int but an unsigned > type. > > > + u32 pab_ctrl; > > + int err; > > + struct resource_entry *win, *tmp; > > + > > + /* setup the PCIe slot link power*/ > > Comment is useless, remove it. > > > + err = mobiveil_bringup_link(pcie); > > + if (err) { > > + dev_info(&pcie->pdev->dev, "link bring-up failed\n"); > > + return err; > > + } > > + > > + /* > > + * program Bus Master Enable Bit in Command Register in PAB Config > > + * Space > > + */ > > + value = csr_readl(pcie, PCI_COMMAND); > > + csr_writel(pcie, > > + value | > > Why is this on a separate line ? > > > + PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER, > > + PCI_COMMAND); > > + > > + /* > > + * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL > > + * register > > + */ > > + pab_ctrl = csr_readl(pcie, PAB_CTRL); > > + csr_writel(pcie, pab_ctrl | (1 << AMBA_PIO_ENABLE_SHIFT) | > > + (1 << PEX_PIO_ENABLE_SHIFT), > > + PAB_CTRL); > > + > > + csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB); > > + > > + /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in > > + * PAB_AXI_PIO_CTRL Register > > + */ > > + value = csr_readl(pcie, PAB_AXI_PIO_CTRL); > > + csr_writel(pcie, value | APIO_EN_MASK, PAB_AXI_PIO_CTRL); > > + > > + /* > > + * we'll program one outbound window for config reads and > > + * another default inbound window for all the upstream traffic > > + * rest of the outbound windows will be configured according to > > + * the "ranges" field defined in device tree > > + */ > > + > > + /* config outbound translation window */ > > + program_ob_windows(pcie, pcie->ob_wins_configured, > > + pcie->ob_io_res->start, 0, CFG_WINDOW_TYPE, > > + resource_size(pcie->ob_io_res)/1024); > > + > > + /* memory inbound translation window */ > > + program_ib_windows(pcie, WIN_NUM_1, 0, MEM_WINDOW_TYPE, IB_WIN_SIZE); > > + > > + /* Get the I/O and memory ranges from DT */ > > + resource_list_for_each_entry_safe(win, tmp, &pcie->resources) { > > + type = 0; > > + if (resource_type(win->res) == IORESOURCE_MEM) > > + type = MEM_WINDOW_TYPE; > > + if (resource_type(win->res) == IORESOURCE_IO) > > + type = IO_WINDOW_TYPE; > > + if (type) { > > + /* configure outbound translation window */ > > + program_ob_windows(pcie, pcie->ob_wins_configured, > > + win->res->start, 0, type, > > + resource_size(win->res)/1024); > > + } > > + } > > + > > + return err; > > +} > > + > > +/* routine to setup the INTx related data */ > > +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq, > > + irq_hw_number_t hwirq) > > +{ > > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); > > IIUC, the host bridge acts as an INTX/MSI controller/multiplexer. > > So, instead of relying on > dummy_irq_chip, you should create your own > irq_chip with the methods that you require (eg irq_ack(), irq_mask(), > irq_compose_msi_msg()) and use that as bottom irq_chip for both > INTX and MSI domains. > > That's why I asked you to have a look at pcie-tango.c (except that there > INTX aren't supported but the basic idea does not change) and implement > IRQ domains handling like that same driver. > are there any disadvantages of keeping dummy_irq_chip, as I see quite a few host bridge drivers including otehr two major soft IP drivers from altera and xilinx with similar MSI implementaion. > > > + irq_set_chip_data(irq, domain->host_data); > > + return 0; > > +} > > + > > +/* INTx domain opeartions structure */ > > s/opeartions/operations > > > +static const struct irq_domain_ops intx_domain_ops = { > > + .map = mobiveil_pcie_intx_map, > > +}; > > + > > +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie) > > +{ > > + struct device *dev = &pcie->pdev->dev; > > + struct device_node *node = dev->of_node; > > + int ret; > > ret is unused > > > + /* setup INTx */ > > + pcie->intx_domain = irq_domain_add_linear(node, > > + PCI_NUM_INTX + 1, &intx_domain_ops, pcie); > > You should use PCI_NUM_INTX and add pci_irqd_intx_xlate() as the > domain ops .xlate. > > > + if (!pcie->intx_domain) { > > + dev_err(dev, "Failed to get a INTx IRQ domain\n"); > > + return -ENODEV; > > + } > > + > > + return 0; > > +} > > + > > +static int mobiveil_pcie_probe(struct platform_device *pdev) > > +{ > > + struct mobiveil_pcie *pcie; > > + struct pci_bus *bus; > > + struct pci_bus *child; > > + struct pci_host_bridge *bridge; > > + struct device *dev = &pdev->dev; > > + struct device_node *node = dev->of_node; > > + resource_size_t iobase; > > + int ret; > > + > > + /* allocate the PCIe port */ > > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); > > + if (!bridge) > > + return -ENODEV; > > + > > + pcie = pci_host_bridge_priv(bridge); > > + if (!pcie) > > + return -ENOMEM; > > + > > + pcie->pdev = pdev; > > + > > + /* parse the device tree */ > > Remove this comment, it does not add anything. > > > + ret = mobiveil_pcie_parse_dt(pcie); > > + if (ret) { > > + dev_err(dev, "Parsing DT failed, ret: %x\n", ret); > > + return ret; > > + } > > + > > + INIT_LIST_HEAD(&pcie->resources); > > + > > + /* parse the host bridge base addresses from the device tree file */ > > + ret = of_pci_get_host_bridge_resources(node, > > + 0, 0xff, &pcie->resources, &iobase); > > + if (ret) { > > + dev_err(dev, "Getting bridge resources failed\n"); > > + return -ENOMEM; > > + } > > + > > + /* > > + * configure all inbound and outbound windows and prepare the RC for > > + * config access > > + */ > > + ret = mobiveil_host_init(pcie); > > + if (ret) { > > + dev_err(dev, "Failed to initialize host\n"); > > + goto error; > > + } > > + > > + /* fixup for PCIe class register */ > > + csr_writel(pcie, 0x060402ab, PAB_INTP_AXI_PIO_CLASS); > > + > > + /* initialize the IRQ domains */ > > + ret = mobiveil_pcie_init_irq_domain(pcie); > > + if (ret) { > > + dev_err(dev, "Failed creating IRQ Domain\n"); > > + goto error; > > + } > > + > > + > > Two empty lines, should be one. > > > + ret = devm_request_pci_bus_resources(dev, &pcie->resources); > > + if (ret) > > + goto error; > > + > > + /* Initialize bridge */ > > + list_splice_init(&pcie->resources, &bridge->windows); > > + bridge->dev.parent = dev; > > + bridge->sysdata = pcie; > > + bridge->busnr = pcie->root_bus_nr; > > + bridge->ops = &mobiveil_pcie_ops; > > + bridge->map_irq = of_irq_parse_and_map_pci; > > + bridge->swizzle_irq = pci_common_swizzle; > > + > > + /* setup the kernel resources for the newly added PCIe root bus */ > > + ret = pci_scan_root_bus_bridge(bridge); > > + if (ret) > > + goto error; > > + > > + bus = bridge->bus; > > + > > + pci_assign_unassigned_bus_resources(bus); > > + list_for_each_entry(child, &bus->children, node) > > + pcie_bus_configure_settings(child); > > + pci_bus_add_devices(bus); > > + > > + platform_set_drvdata(pdev, pcie); > > Is this really needed ? > > Thanks, > Lorenzo > > > + > > + return 0; > > +error: > > + pci_free_resource_list(&pcie->resources); > > + return ret; > > +} > > + > > +static const struct of_device_id mobiveil_pcie_of_match[] = { > > + {.compatible = "mbvl,gpex40-pcie",}, > > +}; > > + > > +MODULE_DEVICE_TABLE(of, mobiveil_pcie_of_match); > > + > > +static struct platform_driver mobiveil_pcie_driver = { > > + .probe = mobiveil_pcie_probe, > > + .driver = { > > + .name = "mobiveil-pcie", > > + .of_match_table = mobiveil_pcie_of_match, > > + .suppress_bind_attrs = true, > > + }, > > +}; > > + > > +builtin_platform_driver(mobiveil_pcie_driver); > > + > > +MODULE_LICENSE("GPL v2"); > > +MODULE_DESCRIPTION("Mobiveil PCIe host controller driver"); > > +MODULE_AUTHOR("Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>"); > > -- > > 1.8.3.1 > > Thanks, Subrahmanya
On Fri, Dec 22, 2017 at 01:29:00PM +0530, Subrahmanya Lingappa wrote: [...] > > > +static void mobiveil_pcie_isr(struct irq_desc *desc) > > > +{ > > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > > + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc); > > > + struct device *dev = &pcie->pdev->dev; > > > + u32 intr_status; > > > + unsigned long shifted_status; > > > > Why not u32 ? > > > for_each_set_bit() api warns about the variable not being unsigned > long. So used the same to take out the warning. > > > > > + u32 bit, virq; > > > + u32 val, mask; > > > + > > > + chained_irq_enter(chip, desc); > > > + > > > + /* read INTx status */ > > > + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > > + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB); > > > + intr_status = val & mask; > > > + > > > + /* > > Handle INTx */ > > > + if (intr_status & PAB_INTP_INTX_MASK) { > > > + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > > > Why can't you reuse val to read the status and create shifted_status > > from it ? > > > > eg > > > > shifted_status = val << PAB_INTA_POS; > > csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT); > > > > Just a hint to make the loop more readable. BTW, I do not think that > > writing shifted_status into that register is OK, see below. > > > > > + do { > > > + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) { > > > + > > > + /* clear interrupts */ > > > + csr_writel(pcie, shifted_status << > > PAB_INTA_POS, > > > + > > PAB_INTP_AMBA_MISC_STAT); > > > > Should not you clear just the interrupt you are about to handle ? > > PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status > bits at positions 5,6,7 and 8, > they are RW1C bits, so writing 1 to this bit clears the status. > So here the status read was written back to it to clear. Understood - the point is that IIUC you should clear just the IRQ (bit) that you are handling at each iteration not all at once. What do PAB_INTP_AMBA_MISC_STAT[4:0] represent then ? > > > + virq = irq_find_mapping(pcie->intx_domain, > > > + bit + 1); > > > + if (virq) > > > + > > generic_handle_irq(virq); > > > + else > > > + dev_err_ratelimited(dev, > > > + "unexpected IRQ, INT%d\n", > > > + bit); > > > + > > > + } > > > + > > > + shifted_status = csr_readl(pcie, > > > + PAB_INTP_AMBA_MISC_STAT); > > > + > > > + } while ((shifted_status >> PAB_INTA_POS) != 0); > > > > I do not understand this. Can you explain to me how the > > register at: > > > > PAB_INTP_AMBA_MISC_STAT > > > > works ? > > > just repeating what explained before, to ease the readablility. > PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status > bits at positions 5,6,7 and 8, > they are RW1C bits, so writing 1 to this bit clears the status. > > > > > A patch for mediatek has been posted: > > > > https://patchwork.ozlabs.org/patch/851181/ > > > > It looks like this loop may suffer from the same issue, so please do > > have a look. > > > I will clear the the interrupt after > > its hadled by generic_handle_irq() > . > right ? Where ? Can you explain please what every bit in PAB_INTP_AMBA_MISC_STAT represent ? > > On top of that, most of the operations you are carrying out here > > can be done automatically by making them part of the struct irq_chip > > methods (ie acking IRQs, masking IRQs, etc, see below). > > > Any side effects of keeping this code as is ? Yes, that it will take us more effort to convert it to the usage I describe above - which is how it is expected to be written. [...] > > > +/* routine to setup the INTx related data */ > > > +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq, > > > + irq_hw_number_t hwirq) > > > +{ > > > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); > > > > IIUC, the host bridge acts as an INTX/MSI controller/multiplexer. > > > > So, instead of relying on > > dummy_irq_chip, you should create your own > > irq_chip with the methods that you require (eg irq_ack(), irq_mask(), > > irq_compose_msi_msg()) and use that as bottom irq_chip for both > > INTX and MSI domains. > > > > That's why I asked you to have a look at pcie-tango.c (except that there > > INTX aren't supported but the basic idea does not change) and implement > > IRQ domains handling like that same driver. > > > are there any disadvantages of keeping dummy_irq_chip, as I see quite > a few host bridge > > drivers including > otehr two major soft IP drivers from altera and xilinx with similar > MSI implementaion. See above, this is a new driver, the existing IP drivers will have to be converted eventually, new code should set an example not add more burden to code refactoring. Lorenzo
Lorenzo, On Tue, Jan 2, 2018 at 7:43 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Fri, Dec 22, 2017 at 01:29:00PM +0530, Subrahmanya Lingappa wrote: > > [...] > > > > > +static void mobiveil_pcie_isr(struct irq_desc *desc) > > > > +{ > > > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > > > + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc); > > > > + struct device *dev = &pcie->pdev->dev; > > > > + u32 intr_status; > > > > + unsigned long shifted_status; > > > > > > Why not u32 ? > > > > > for_each_set_bit() api warns about the variable not being unsigned > > long. So used the same to take out the warning. > > > > > > > > + u32 bit, virq; > > > > + u32 val, mask; > > > > + > > > > + chained_irq_enter(chip, desc); > > > > + > > > > + /* read INTx status */ > > > > + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > > > + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB); > > > > + intr_status = val & mask; > > > > + > > > > + /* > > > Handle INTx */ > > > > + if (intr_status & PAB_INTP_INTX_MASK) { > > > > + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); > > > > > > Why can't you reuse val to read the status and create shifted_status > > > from it ? > > > > > > eg > > > > > > shifted_status = val << PAB_INTA_POS; > > > csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT); > > > > > > Just a hint to make the loop more readable. BTW, I do not think that > > > writing shifted_status into that register is OK, see below. > > > > > > > + do { > > > > + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) { > > > > + > > > > + /* clear interrupts */ > > > > + csr_writel(pcie, shifted_status << > > > PAB_INTA_POS, > > > > + > > > PAB_INTP_AMBA_MISC_STAT); > > > > > > Should not you clear just the interrupt you are about to handle ? > > > > PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status > > bits at positions 5,6,7 and 8, > > they are RW1C bits, so writing 1 to this bit clears the status. > > So here the status read was written back to it to clear. > > Understood - the point is that IIUC you should clear just the IRQ (bit) > that you are handling at each iteration not all at once. > Understood > > What do PAB_INTP_AMBA_MISC_STAT[4:0] represent then ? 0 : PCIE to AXI Mailbox Ready This bit is set by hardware when a PCIE-to-AXI mailbox is ready with data to be transferred to PCI-Express host. 1 : Reserved 2 : Reserved 3 : Reserved 4 : Vendor Defined Message Received This bit is set by hardware when an vendor defined message is received on the PEX link. 5 : INTA Received 6 : INTB Received 7 : INTC Received 8 : INTD Received 9-31: Other root port error status bits > > > > > > + virq = irq_find_mapping(pcie->intx_domain, > > > > + bit + 1); > > > > + if (virq) > > > > + > > > generic_handle_irq(virq); > > > > + else > > > > + dev_err_ratelimited(dev, > > > > + "unexpected IRQ, INT%d\n", > > > > + bit); > > > > + > > > > + } > > > > + > > > > + shifted_status = csr_readl(pcie, > > > > + PAB_INTP_AMBA_MISC_STAT); > > > > + > > > > + } while ((shifted_status >> PAB_INTA_POS) != 0); > > > > > > I do not understand this. Can you explain to me how the > > > register at: > > > > > > PAB_INTP_AMBA_MISC_STAT > > > > > > works ? > > > > > just repeating what explained before, to ease the readablility. > > PAB_INTP_AMBA_MISC_STAT register has INTA, INTB, INTC, INTD status > > bits at positions 5,6,7 and 8, > > they are RW1C bits, so writing 1 to this bit clears the status. > > > > > > > > A patch for mediatek has been posted: > > > > > > https://patchwork.ozlabs.org/patch/851181/ > > > > > > It looks like this loop may suffer from the same issue, so please do > > > have a look. > > > > > I will clear the the interrupt after > > > > its hadled by generic_handle_irq() > > . > > right ? > > Where ? Can you explain please what every bit in > > PAB_INTP_AMBA_MISC_STAT > > represent ? > Please see the list above > > > On top of that, most of the operations you are carrying out here > > > can be done automatically by making them part of the struct irq_chip > > > methods (ie acking IRQs, masking IRQs, etc, see below). > > > > > Any side effects of keeping this code as is ? > > Yes, that it will take us more effort to convert it to the usage > I describe above - which is how it is expected to be written. > Ok, I'll convert this by making these operations as part of irq_chip methods > > [...] > > > > > +/* routine to setup the INTx related data */ > > > > +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq, > > > > + irq_hw_number_t hwirq) > > > > +{ > > > > + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); > > > > > > IIUC, the host bridge acts as an INTX/MSI controller/multiplexer. > > > > > > So, instead of relying on > > > dummy_irq_chip, you should create your own > > > irq_chip with the methods that you require (eg irq_ack(), irq_mask(), > > > irq_compose_msi_msg()) and use that as bottom irq_chip for both > > > INTX and MSI domains. > > > > > > That's why I asked you to have a look at pcie-tango.c (except that there > > > INTX aren't supported but the basic idea does not change) and implement > > > IRQ domains handling like that same driver. > > > > > are there any disadvantages of keeping dummy_irq_chip, as I see quite > > a few host bridge > > > > drivers including > > otehr two major soft IP drivers from altera and xilinx with similar > > MSI implementaion. > > See above, this is a new driver, the existing IP drivers will have to > be converted eventually, new code should set an example not add more > burden to code refactoring. > Agreed, I will follow the tango driver approach to remove dummy_irq_chip. > > Lorenzo Thanks, Subrahmanya
On Wed, Dec 20, 2017 at 05:03:07PM +0000, Lorenzo Pieralisi wrote: [...] > > +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie) > > +{ > > + struct device *dev = &pcie->pdev->dev; > > + struct device_node *node = dev->of_node; > > + int ret; > > ret is unused > > > + /* setup INTx */ > > + pcie->intx_domain = irq_domain_add_linear(node, > > + PCI_NUM_INTX + 1, &intx_domain_ops, pcie); > > You should use PCI_NUM_INTX and add pci_irqd_intx_xlate() as the > domain ops .xlate. Actually that's not quite correct - so scrap this. The point here is, the PCI host controller interrupt domain has to have 4 hwirqs (this is a pseudo interrupt controller - a multiplexer) and it is through DT interrupt-map that INTx pins (1-4) can be mapped to the "PCI interrupt controller hwirq inputs" that we consider numbered from 0 to 3. It is not that clean-cut but IMO it is the DT interrupt mapping that should carry out the translation. See for instance: arch/arm64/boot/dts/marvell/armada-37xx.dtsi This requires DT/irqchip maintainers acknowledgement before proceeding. Lorenzo
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index 38d1298..09bf1d9 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -27,6 +27,14 @@ config PCIE_XILINX_NWL or End Point. The current option selection will only support root port enabling. +config PCIE_MOBIVEIL + bool "Mobiveil AXI PCIe host bridge support" + depends on ARCH_ZYNQMP || COMPILE_TEST + depends on PCI_MSI_IRQ_DOMAIN + help + Say 'Y' here if you want kernel to support the Mobiveil AXI PCIe + Host Bridge driver. + config PCI_FTPCI100 bool "Faraday Technology FTPCI100 PCI controller" depends on OF diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index 34ec1d8..d745d68 100644 --- a/drivers/pci/host/Makefile +++ b/drivers/pci/host/Makefile @@ -23,6 +23,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o obj-$(CONFIG_VMD) += vmd.o +obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o # The following drivers are for devices that use the generic ACPI # pci_root.c driver but don't support standard ECAM config access. diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c new file mode 100644 index 0000000..8611aaa --- /dev/null +++ b/drivers/pci/host/pcie-mobiveil.c @@ -0,0 +1,653 @@ +/* + * PCIe host controller driver for Mobiveil PCIe Host controller + * + * SPDX-License-Identifier: GPL-2.0 + * Copyright (c) 2017 Mobiveil Inc. + * Author: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> + */ + +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/init.h> +#include <linux/irq.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_irq.h> +#include <linux/of_platform.h> +#include <linux/of_pci.h> +#include <linux/pci.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +/* register offsets and bit positions */ + +/* + * translation tables are grouped into windows, each window registers are + * grouped into blocks of 4 or 16 registers each + */ +#define PAB_REG_BLOCK_SIZE_4 4 +#define PAB_REG_BLOCK_SIZE_16 16 +#define PAB_REG_ADDR_4(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_4)) +#define PAB_REG_ADDR_16(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_16)) + +#define LTSSM_STATUS 0x0404 +#define LTSSM_STATUS_L0_MASK 0x3f +#define LTSSM_STATUS_L0 0x2d + +#define PAB_CTRL 0x0808 +#define AMBA_PIO_ENABLE_SHIFT 0 +#define PEX_PIO_ENABLE_SHIFT 1 +#define PAGE_SEL_SHIFT 13 +#define PAGE_SEL_MASK 0x3f +#define PAGE_LO_MASK 0x3ff +#define PAGE_SEL_EN 0xc00 +#define PAGE_SEL_OFFSET_SHIFT 10 + +#define PAB_AXI_PIO_CTRL 0x0840 +#define APIO_EN_MASK 0xf + +#define PAB_PEX_PIO_CTRL 0x08c0 +#define PIO_ENABLE_SHIFT 0 + +#define PAB_INTP_AMBA_MISC_ENB 0x0b0c +#define PAB_INTP_AMBA_MISC_STAT 0x0b1c +#define PAB_INTP_INTX_MASK 0x1e0 + +#define PAB_AXI_AMAP_CTRL(win) PAB_REG_ADDR_16(0x0ba0, win) +#define WIN_ENABLE_SHIFT 0 +#define WIN_TYPE_SHIFT 1 +#define WIN_SIZE_SHIFT 10 + +#define PAB_EXT_AXI_AMAP_SIZE(win) PAB_REG_ADDR_4(0xbaf0, win) + +#define PAB_AXI_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x0ba4, win) +#define AXI_WINDOW_BASE_SHIFT 2 + +#define PAB_AXI_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x0ba8, win) +#define PAB_BUS_SHIFT 24 +#define PAB_DEVICE_SHIFT 19 +#define PAB_FUNCTION_SHIFT 16 + +#define PAB_AXI_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x0bac, win) +#define PAB_INTP_AXI_PIO_CLASS 0x474 + +#define PAB_PEX_AMAP_CTRL(win) PAB_REG_ADDR_16(0x4ba0, win) +#define AMAP_CTRL_EN_SHIFT 0 +#define AMAP_CTRL_TYPE_SHIFT 1 + +#define PAB_EXT_PEX_AMAP_SIZEN(win) PAB_REG_ADDR_4(0xbef0, win) +#define PAB_PEX_AMAP_AXI_WIN(win) PAB_REG_ADDR_16(0x4ba4, win) +#define PAB_PEX_AMAP_PEX_WIN_L(win) PAB_REG_ADDR_16(0x4ba8, win) +#define PAB_PEX_AMAP_PEX_WIN_H(win) PAB_REG_ADDR_16(0x4bac, win) + +/* supported number of interrupts */ +#define PCI_NUM_INTX 4 +#define PAB_INTA_POS 5 + +/* outbound and inbound window definitions */ +#define WIN_NUM_0 0 +#define WIN_NUM_1 1 +#define CFG_WINDOW_TYPE 0 +#define IO_WINDOW_TYPE 1 +#define MEM_WINDOW_TYPE 2 +#define IB_WIN_SIZE (256 * 1024 * 1024) +#define MAX_PIO_WINDOWS 8 + +/* Parameters for the waiting for link up routine */ +#define LINK_WAIT_MAX_RETRIES 10 +#define LINK_WAIT_MIN 90000 +#define LINK_WAIT_MAX 100000 + +#ifndef UINT64_MAX +#define UINT64_MAX (u64)(~((u64)0)) +#endif + +struct mobiveil_pcie { + struct platform_device *pdev; + struct list_head resources; + void __iomem *config_axi_slave_base; /* endpoint config base */ + void __iomem *csr_axi_slave_base; /* root port config base */ + struct irq_domain *intx_domain; + int irq; + int apio_wins; + int ppio_wins; + int ob_wins_configured; /* configured outbound windows */ + int ib_wins_configured; /* configured inbound windows */ + struct resource *ob_io_res; + char root_bus_nr; +}; + +static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value, + const u32 reg) +{ + writel_relaxed(value, pcie->csr_axi_slave_base + reg); +} + +static inline u32 csr_readl(struct mobiveil_pcie *pcie, const u32 reg) +{ + return readl_relaxed(pcie->csr_axi_slave_base + reg); +} + +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie) +{ + return (csr_readl(pcie, LTSSM_STATUS) & + LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0; +} + +static bool mobiveil_pcie_valid_device(struct pci_bus *bus, unsigned int devfn) +{ + struct mobiveil_pcie *pcie = bus->sysdata; + + /* Check if link is up when trying to access downstream ports */ + if (bus->number != pcie->root_bus_nr) + if (!mobiveil_pcie_link_up(pcie)) + return false; + + /* Only one device down on each root port */ + if ((bus->number == pcie->root_bus_nr) && (devfn > 0)) + return false; + + /* + * Do not read more than one device on the bus directly + * attached to RC + */ + if ((bus->primary == pcie->root_bus_nr) && (devfn > 0)) + return false; + + return true; +} + +/* + * mobiveil_pcie_map_bus - routine to get the configuration base of either + * root port or endpoint + */ +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus, + unsigned int devfn, int where) +{ + struct mobiveil_pcie *pcie = bus->sysdata; + + if (!mobiveil_pcie_valid_device(bus, devfn)) + return NULL; + + if (bus->number == pcie->root_bus_nr) { + /* RC config access */ + return pcie->csr_axi_slave_base + where; + } else { + /* + * EP config access (in Config/APIO space) + * Program PEX Address base (31..16 bits) with appropriate value + * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register. + * Relies on pci_lock serialization + */ + csr_writel(pcie, + bus->number << PAB_BUS_SHIFT | + PCI_SLOT(devfn) << PAB_DEVICE_SHIFT | + PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT, + PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0)); + return pcie->config_axi_slave_base + where; + } +} + +static struct pci_ops mobiveil_pcie_ops = { + .map_bus = mobiveil_pcie_map_bus, + .read = pci_generic_config_read, + .write = pci_generic_config_write, +}; + +static void mobiveil_pcie_isr(struct irq_desc *desc) +{ + struct irq_chip *chip = irq_desc_get_chip(desc); + struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc); + struct device *dev = &pcie->pdev->dev; + u32 intr_status; + unsigned long shifted_status; + u32 bit, virq; + u32 val, mask; + + chained_irq_enter(chip, desc); + + /* read INTx status */ + val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); + mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB); + intr_status = val & mask; + + /* Handle INTx */ + if (intr_status & PAB_INTP_INTX_MASK) { + shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT); + do { + for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) { + + /* clear interrupts */ + csr_writel(pcie, shifted_status << PAB_INTA_POS, + PAB_INTP_AMBA_MISC_STAT); + + virq = irq_find_mapping(pcie->intx_domain, + bit + 1); + if (virq) + generic_handle_irq(virq); + else + dev_err_ratelimited(dev, + "unexpected IRQ, INT%d\n", + bit); + + } + + shifted_status = csr_readl(pcie, + PAB_INTP_AMBA_MISC_STAT); + + } while ((shifted_status >> PAB_INTA_POS) != 0); + } + + csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT); + chained_irq_exit(chip, desc); +} + +static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie) +{ + struct device *dev = &pcie->pdev->dev; + struct platform_device *pdev = pcie->pdev; + struct device_node *node = dev->of_node; + struct resource *res; + const char *type; + + type = of_get_property(node, "device_type", NULL); + if (!type || strcmp(type, "pci")) { + dev_err(dev, "invalid \"device_type\" %s\n", type); + return -EINVAL; + } + + /* map config resource */ + res = platform_get_resource_byname(pdev, + IORESOURCE_MEM, "config_axi_slave"); + pcie->config_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); + if (IS_ERR(pcie->config_axi_slave_base)) + return PTR_ERR(pcie->config_axi_slave_base); + pcie->ob_io_res = res; + + /* map csr resource */ + res = platform_get_resource_byname(pdev, + IORESOURCE_MEM, "csr_axi_slave"); + pcie->csr_axi_slave_base = devm_pci_remap_cfg_resource(dev, res); + if (IS_ERR(pcie->csr_axi_slave_base)) + return PTR_ERR(pcie->csr_axi_slave_base); + + /* read the number of windows requested */ + if (!pcie->apio_wins && + of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) { + pcie->apio_wins = MAX_PIO_WINDOWS; + } + + if (!pcie->ppio_wins && + of_property_read_u32(node, "ppio-wins", &pcie->ppio_wins)) { + pcie->ppio_wins = MAX_PIO_WINDOWS; + } + + pcie->irq = platform_get_irq(pdev, 0); + if (pcie->irq <= 0) { + dev_err(dev, "failed to map IRQ: %d\n", pcie->irq); + return -ENODEV; + } + + irq_set_chained_handler_and_data(pcie->irq, mobiveil_pcie_isr, pcie); + + return 0; +} + +/* + * select_paged_register - routine to access paged register of root complex + * + * registers of RC are paged, for this scheme to work + * extracted higher 6 bits of the offset will be written to pg_sel + * field of PAB_CTRL register and rest of the lower 10 bits enabled with + * PAGE_SEL_EN are used as offset of the register. + * + */ +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset) +{ + int pab_ctrl_dw, pg_sel; + + /* clear pg_sel field */ + pab_ctrl_dw = csr_readl(pcie, PAB_CTRL); + pab_ctrl_dw = (pab_ctrl_dw & ~(PAGE_SEL_MASK << PAGE_SEL_SHIFT)); + + /* set pg_sel field */ + pg_sel = (offset >> PAGE_SEL_OFFSET_SHIFT) & PAGE_SEL_MASK; + pab_ctrl_dw |= ((pg_sel << PAGE_SEL_SHIFT)); + csr_writel(pcie, pab_ctrl_dw, PAB_CTRL); +} + +static void write_paged_register(struct mobiveil_pcie *pcie, + u32 val, u32 offset) +{ + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; + + select_paged_register(pcie, offset); + csr_writel(pcie, val, off); +} + +static u32 read_paged_register(struct mobiveil_pcie *pcie, u32 offset) +{ + u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN; + + select_paged_register(pcie, offset); + return csr_readl(pcie, off); +} + +static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num, + int pci_addr, int type, int size_kb) +{ + int pio_ctrl_val; + int amap_ctrl_dw; + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb))); + + if ((pcie->ib_wins_configured + 1) > pcie->ppio_wins) { + dev_err(&pcie->pdev->dev, + "ERROR: max inbound windows reached !\n"); + return; + } + + pio_ctrl_val = csr_readl(pcie, PAB_PEX_PIO_CTRL); + csr_writel(pcie, + pio_ctrl_val | (1 << PIO_ENABLE_SHIFT), PAB_PEX_PIO_CTRL); + amap_ctrl_dw = read_paged_register(pcie, PAB_PEX_AMAP_CTRL(win_num)); + amap_ctrl_dw = (amap_ctrl_dw | (type << AMAP_CTRL_TYPE_SHIFT)); + amap_ctrl_dw = (amap_ctrl_dw | (1 << AMAP_CTRL_EN_SHIFT)); + + write_paged_register(pcie, amap_ctrl_dw | lower_32_bits(size64), + PAB_PEX_AMAP_CTRL(win_num)); + + write_paged_register(pcie, upper_32_bits(size64), + PAB_EXT_PEX_AMAP_SIZEN(win_num)); + + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_AXI_WIN(win_num)); + write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_PEX_WIN_L(win_num)); + write_paged_register(pcie, 0, PAB_PEX_AMAP_PEX_WIN_H(win_num)); +} + +static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num, + u64 cpu_addr, u64 pci_addr, int config_io_bit, + int size_kb) +{ + + u32 value, type; + u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb))); + + if ((pcie->ob_wins_configured + 1) > pcie->apio_wins) { + dev_err(&pcie->pdev->dev, + "ERROR: max outbound windows reached !\n"); + return; + } + + /* + * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit + * to 4 KB in PAB_AXI_AMAP_CTRL register + */ + type = config_io_bit; + value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num)); + csr_writel(pcie, + 1 << WIN_ENABLE_SHIFT | + type << WIN_TYPE_SHIFT | + lower_32_bits(size64), + PAB_AXI_AMAP_CTRL(win_num)); + + write_paged_register(pcie, upper_32_bits(size64), + PAB_EXT_AXI_AMAP_SIZE(win_num)); + + /* + * program AXI window base with appropriate value in + * PAB_AXI_AMAP_AXI_WIN0 register + */ + value = csr_readl(pcie, PAB_AXI_AMAP_AXI_WIN(win_num)); + csr_writel(pcie, + cpu_addr >> AXI_WINDOW_BASE_SHIFT << + AXI_WINDOW_BASE_SHIFT, PAB_AXI_AMAP_AXI_WIN(win_num)); + + value = csr_readl(pcie, PAB_AXI_AMAP_PEX_WIN_H(win_num)); + + csr_writel(pcie, lower_32_bits(pci_addr), + PAB_AXI_AMAP_PEX_WIN_L(win_num)); + csr_writel(pcie, upper_32_bits(pci_addr), + PAB_AXI_AMAP_PEX_WIN_H(win_num)); + + pcie->ob_wins_configured++; +} + +static int mobiveil_bringup_link(struct mobiveil_pcie *pcie) +{ + int retries; + + /* check if the link is up or not */ + for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) { + if (mobiveil_pcie_link_up(pcie)) + return 0; + + usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX); + } + dev_err(&pcie->pdev->dev, "link never came up\n"); + return -ETIMEDOUT; +} + +static int mobiveil_host_init(struct mobiveil_pcie *pcie) +{ + u32 value; + int type = 0; + u32 pab_ctrl; + int err; + struct resource_entry *win, *tmp; + + /* setup the PCIe slot link power*/ + + err = mobiveil_bringup_link(pcie); + if (err) { + dev_info(&pcie->pdev->dev, "link bring-up failed\n"); + return err; + } + + /* + * program Bus Master Enable Bit in Command Register in PAB Config + * Space + */ + value = csr_readl(pcie, PCI_COMMAND); + csr_writel(pcie, + value | + PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER, + PCI_COMMAND); + + /* + * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL + * register + */ + pab_ctrl = csr_readl(pcie, PAB_CTRL); + csr_writel(pcie, pab_ctrl | (1 << AMBA_PIO_ENABLE_SHIFT) | + (1 << PEX_PIO_ENABLE_SHIFT), + PAB_CTRL); + + csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB); + + /* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in + * PAB_AXI_PIO_CTRL Register + */ + value = csr_readl(pcie, PAB_AXI_PIO_CTRL); + csr_writel(pcie, value | APIO_EN_MASK, PAB_AXI_PIO_CTRL); + + /* + * we'll program one outbound window for config reads and + * another default inbound window for all the upstream traffic + * rest of the outbound windows will be configured according to + * the "ranges" field defined in device tree + */ + + /* config outbound translation window */ + program_ob_windows(pcie, pcie->ob_wins_configured, + pcie->ob_io_res->start, 0, CFG_WINDOW_TYPE, + resource_size(pcie->ob_io_res)/1024); + + /* memory inbound translation window */ + program_ib_windows(pcie, WIN_NUM_1, 0, MEM_WINDOW_TYPE, IB_WIN_SIZE); + + /* Get the I/O and memory ranges from DT */ + resource_list_for_each_entry_safe(win, tmp, &pcie->resources) { + type = 0; + if (resource_type(win->res) == IORESOURCE_MEM) + type = MEM_WINDOW_TYPE; + if (resource_type(win->res) == IORESOURCE_IO) + type = IO_WINDOW_TYPE; + if (type) { + /* configure outbound translation window */ + program_ob_windows(pcie, pcie->ob_wins_configured, + win->res->start, 0, type, + resource_size(win->res)/1024); + } + } + + return err; +} + +/* routine to setup the INTx related data */ +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq, + irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); + irq_set_chip_data(irq, domain->host_data); + return 0; +} + +/* INTx domain opeartions structure */ +static const struct irq_domain_ops intx_domain_ops = { + .map = mobiveil_pcie_intx_map, +}; + +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie) +{ + struct device *dev = &pcie->pdev->dev; + struct device_node *node = dev->of_node; + int ret; + + /* setup INTx */ + pcie->intx_domain = irq_domain_add_linear(node, + PCI_NUM_INTX + 1, &intx_domain_ops, pcie); + + if (!pcie->intx_domain) { + dev_err(dev, "Failed to get a INTx IRQ domain\n"); + return -ENODEV; + } + + return 0; +} + +static int mobiveil_pcie_probe(struct platform_device *pdev) +{ + struct mobiveil_pcie *pcie; + struct pci_bus *bus; + struct pci_bus *child; + struct pci_host_bridge *bridge; + struct device *dev = &pdev->dev; + struct device_node *node = dev->of_node; + resource_size_t iobase; + int ret; + + /* allocate the PCIe port */ + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie)); + if (!bridge) + return -ENODEV; + + pcie = pci_host_bridge_priv(bridge); + if (!pcie) + return -ENOMEM; + + pcie->pdev = pdev; + + /* parse the device tree */ + ret = mobiveil_pcie_parse_dt(pcie); + if (ret) { + dev_err(dev, "Parsing DT failed, ret: %x\n", ret); + return ret; + } + + INIT_LIST_HEAD(&pcie->resources); + + /* parse the host bridge base addresses from the device tree file */ + ret = of_pci_get_host_bridge_resources(node, + 0, 0xff, &pcie->resources, &iobase); + if (ret) { + dev_err(dev, "Getting bridge resources failed\n"); + return -ENOMEM; + } + + /* + * configure all inbound and outbound windows and prepare the RC for + * config access + */ + ret = mobiveil_host_init(pcie); + if (ret) { + dev_err(dev, "Failed to initialize host\n"); + goto error; + } + + /* fixup for PCIe class register */ + csr_writel(pcie, 0x060402ab, PAB_INTP_AXI_PIO_CLASS); + + /* initialize the IRQ domains */ + ret = mobiveil_pcie_init_irq_domain(pcie); + if (ret) { + dev_err(dev, "Failed creating IRQ Domain\n"); + goto error; + } + + + ret = devm_request_pci_bus_resources(dev, &pcie->resources); + if (ret) + goto error; + + /* Initialize bridge */ + list_splice_init(&pcie->resources, &bridge->windows); + bridge->dev.parent = dev; + bridge->sysdata = pcie; + bridge->busnr = pcie->root_bus_nr; + bridge->ops = &mobiveil_pcie_ops; + bridge->map_irq = of_irq_parse_and_map_pci; + bridge->swizzle_irq = pci_common_swizzle; + + /* setup the kernel resources for the newly added PCIe root bus */ + ret = pci_scan_root_bus_bridge(bridge); + if (ret) + goto error; + + bus = bridge->bus; + + pci_assign_unassigned_bus_resources(bus); + list_for_each_entry(child, &bus->children, node) + pcie_bus_configure_settings(child); + pci_bus_add_devices(bus); + + platform_set_drvdata(pdev, pcie); + + return 0; +error: + pci_free_resource_list(&pcie->resources); + return ret; +} + +static const struct of_device_id mobiveil_pcie_of_match[] = { + {.compatible = "mbvl,gpex40-pcie",}, +}; + +MODULE_DEVICE_TABLE(of, mobiveil_pcie_of_match); + +static struct platform_driver mobiveil_pcie_driver = { + .probe = mobiveil_pcie_probe, + .driver = { + .name = "mobiveil-pcie", + .of_match_table = mobiveil_pcie_of_match, + .suppress_bind_attrs = true, + }, +}; + +builtin_platform_driver(mobiveil_pcie_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Mobiveil PCIe host controller driver"); +MODULE_AUTHOR("Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in>");
Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP - GPEX 4.0, a PCIe gen4 IP. This IP has upto 8 outbound and inbound windows for the address translation. Signed-off-by: Subrahmanya Lingappa <l.subrahmanya@mobiveil.co.in> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: linux-pci@vger.kernel.org Cc: devicetree@vger.kernel.org --- drivers/pci/host/Kconfig | 8 + drivers/pci/host/Makefile | 1 + drivers/pci/host/pcie-mobiveil.c | 653 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 662 insertions(+) create mode 100644 drivers/pci/host/pcie-mobiveil.c