On Fri, Jun 10, 2016 at 05:54:15PM +0200, Thomas Petazzoni wrote: > This commit adds a new driver for a PCIe controller called Aardvark, > which is used on the Marvell Armada 3700 ARM64 SoC. > > This patch is based on work done by Hezi Shahmoon > <hezi.shahmoon@xxxxxxxxxxx> and Marcin Wojtas <mw@xxxxxxxxxxxx>. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> > --- > MAINTAINERS | 7 + > drivers/pci/host/Kconfig | 9 + > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pci-aardvark.c | 1023 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 1040 insertions(+) > create mode 100644 drivers/pci/host/pci-aardvark.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7304d2e..373215c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -8740,6 +8740,13 @@ L: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx (moderated for non-subscribers) > S: Maintained > F: drivers/pci/host/*mvebu* > > +PCI DRIVER FOR AARDVARK (Marvell Armada 3700) > +M: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> > +L: linux-pci@xxxxxxxxxxxxxxx > +L: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx (moderated for non-subscribers) > +S: Maintained > +F: drivers/pci/host/pci-aardvark.c > + > PCI DRIVER FOR NVIDIA TEGRA > M: Thierry Reding <thierry.reding@xxxxxxxxx> > L: linux-tegra@xxxxxxxxxxxxxxx > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 5d2374e..5f085fd 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -16,6 +16,15 @@ config PCI_MVEBU > depends on ARM > depends on OF > > +config PCI_AARDVARK > + bool "Aardvark PCIe controller" > + depends on ARCH_MVEBU && ARM64 > + depends on OF > + select PCI_MSI I'm guessing this should be "depends on PCI_MSI_IRQ_DOMAIN" per Arnd's recent patch: http://lkml.kernel.org/r/8531046.3ceRqUA835@wuerfel I have merged Arnd's patch, so it will be in v4.8. > + help > + Add support for Aardvark 64bit PCIe Host Controller. This > + controller is part of the South Bridge of the Marvel Armada > + 3700 SoC. > > config PCIE_XILINX_NWL > bool "NWL PCIe Core" > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index 9c8698e..66f45b6 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -5,6 +5,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o > obj-$(CONFIG_PCI_IMX6) += pci-imx6.o > obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o > obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o > +obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o > obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o > obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o > obj-$(CONFIG_PCIE_RCAR) += pcie-rcar.o > diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c > new file mode 100644 > index 0000000..15d66a7 > --- /dev/null > +++ b/drivers/pci/host/pci-aardvark.c > @@ -0,0 +1,1023 @@ > +/* > + * Driver for the Aardvark PCIe controller, used on Marvell Armada > + * 3700. > + * > + * Copyright (C) 2016 Marvell > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + */ > + > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/kernel.h> > +#include <linux/pci.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/of_address.h> > +#include <linux/of_pci.h> > + > +/* PCIe core registers */ > +#define PCIE_CORE_CMD_STATUS_REG 0x4 > +#define PCIE_CORE_CMD_IO_ACCESS_EN BIT(0) > +#define PCIE_CORE_CMD_MEM_ACCESS_EN BIT(1) > +#define PCIE_CORE_CMD_MEM_IO_REQ_EN BIT(2) > +#define PCIE_CORE_DEV_CTRL_STATS_REG 0xC8 Please use either upper- or lower-case in hex constants consistently. Most of the ones below are lower-case. > +#define PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE (0 << 4) > +#define PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT 5 > +#define PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE (0 << 11) > +#define PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT 12 > +#define PCIE_CORE_LINK_CTRL_STAT_REG 0xD0 > +#define PCIE_CORE_LINK_L0S_ENTRY BIT(0) > +#define PCIE_CORE_LINK_TRAINING BIT(5) > +#define PCIE_CORE_LINK_WIDTH_SHIFT 20 > +#define PCIE_CORE_ERR_CAPCTL_REG 0x118 > +#define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX BIT(5) > +#define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN BIT(6) > +#define PCIE_CORE_ERR_CAPCTL_ECRC_CHCK BIT(7) > +#define PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV BIT(8) > + > +/* PIO registers base address and register offsets */ > +#define PIO_BASE_ADDR 0x4000 > +#define PIO_CTRL (PIO_BASE_ADDR + 0x0) > +#define PIO_STAT (PIO_BASE_ADDR + 0x4) > +#define PIO_COMPLETION_STATUS_SHIFT 7 > +#define PIO_COMPLETION_STATUS_MASK GENMASK(9, 7) > +#define PIO_COMPLETION_STATUS_OK 0 > +#define PIO_COMPLETION_STATUS_UR 1 > +#define PIO_COMPLETION_STATUS_CRS 2 > +#define PIO_COMPLETION_STATUS_CA 4 > +#define PIO_NON_POSTED_REQ BIT(0) > +#define PIO_ADDR_LS (PIO_BASE_ADDR + 0x8) > +#define PIO_ADDR_MS (PIO_BASE_ADDR + 0xc) > +#define PIO_WR_DATA (PIO_BASE_ADDR + 0x10) > +#define PIO_WR_DATA_STRB (PIO_BASE_ADDR + 0x14) > +#define PIO_RD_DATA (PIO_BASE_ADDR + 0x18) > +#define PIO_START (PIO_BASE_ADDR + 0x1c) > +#define PIO_ISR (PIO_BASE_ADDR + 0x20) > +#define PIO_ISRM (PIO_BASE_ADDR + 0x24) > + > +/* Aardvark Control registers */ > +#define CONTROL_BASE_ADDR 0x4800 > +#define PCIE_CORE_CTRL0_REG (CONTROL_BASE_ADDR + 0x0) > +#define PCIE_GEN_SEL_MSK 0x3 > +#define PCIE_GEN_SEL_SHIFT 0x0 > +#define SPEED_GEN_1 0 > +#define SPEED_GEN_2 1 > +#define SPEED_GEN_3 2 > +#define IS_RC_MSK 1 > +#define IS_RC_SHIFT 2 > +#define LANE_CNT_MSK 0x18 > +#define LANE_CNT_SHIFT 0x3 > +#define LANE_COUNT_1 (0 << LANE_CNT_SHIFT) > +#define LANE_COUNT_2 (1 << LANE_CNT_SHIFT) > +#define LANE_COUNT_4 (2 << LANE_CNT_SHIFT) > +#define LANE_COUNT_8 (3 << LANE_CNT_SHIFT) > +#define LINK_TRAINING_EN BIT(6) > +#define LEGACY_INTA BIT(28) > +#define LEGACY_INTB BIT(29) > +#define LEGACY_INTC BIT(30) > +#define LEGACY_INTD BIT(31) > +#define PCIE_CORE_CTRL1_REG (CONTROL_BASE_ADDR + 0x4) > +#define HOT_RESET_GEN BIT(0) > +#define PCIE_CORE_CTRL2_REG (CONTROL_BASE_ADDR + 0x8) > +#define PCIE_CORE_CTRL2_RESERVED 0x7 > +#define PCIE_CORE_CTRL2_TD_ENABLE BIT(4) > +#define PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE BIT(5) > +#define PCIE_CORE_CTRL2_OB_WIN_ENABLE BIT(6) > +#define PCIE_CORE_CTRL2_MSI_ENABLE BIT(10) > +#define PCIE_ISR0_REG (CONTROL_BASE_ADDR + 0x40) > +#define PCIE_ISR0_MASK_REG (CONTROL_BASE_ADDR + 0x44) > +#define PCIE_ISR0_FLR_INT BIT(26) > +#define PCIE_ISR0_MSG_LTR BIT(25) > +#define PCIE_ISR0_MSI_INT_PENDING BIT(24) > +#define PCIE_ISR0_INTD_DEASSERT BIT(23) > +#define PCIE_ISR0_INTC_DEASSERT BIT(22) > +#define PCIE_ISR0_INTB_DEASSERT BIT(21) > +#define PCIE_ISR0_INTA_DEASSERT BIT(20) > +#define PCIE_ISR0_INTD_ASSERT BIT(19) > +#define PCIE_ISR0_INTC_ASSERT BIT(18) > +#define PCIE_ISR0_INTB_ASSERT BIT(17) > +#define PCIE_ISR0_INTA_ASSERT BIT(16) > +#define PCIE_ISR0_FAT_ERR BIT(13) > +#define PCIE_ISR0_NFAT_ERR BIT(12) > +#define PCIE_ISR0_CORR_ERR BIT(11) > +#define PCIE_ISR0_LMI_LOCAL_INT BIT(10) > +#define PCIE_ISR0_LEGACY_INT_SENT BIT(9) > +#define PCIE_ISR0_MSG_PM_ACTIVE_STATE_NAK BIT(8) > +#define PCIE_ISR0_MSG_PM_PME BIT(7) > +#define PCIE_ISR0_MSG_PM_TURN_OFF BIT(6) > +#define PCIE_ISR0_MSG_PME_TO_ACK BIT(5) > +#define PCIE_ISR0_INB_DP_FERR_PERR_IRQ BIT(4) > +#define PCIE_ISR0_OUTB_DP_FERR_PERR_IRQ BIT(3) > +#define PCIE_ISR0_INBOUND_MSG BIT(2) > +#define PCIE_ISR0_LINK_DOWN BIT(1) > +#define PCIE_ISR0_HOT_RESET BIT(0) > +#define PCIE_ISR0_INTX_ASSERT(val) BIT(16 + (val)) > +#define PCIE_ISR0_INTX_DEASSERT(val) BIT(20 + (val)) > +#define PCIE_ISR0_ALL_MASK GENMASK(26, 0) > +#define PCIE_ISR1_REG (CONTROL_BASE_ADDR + 0x48) > +#define PCIE_ISR1_MASK_REG (CONTROL_BASE_ADDR + 0x4C) > +#define PCIE_ISR1_POWER_STATE_CHANGE BIT(4) > +#define PCIE_ISR1_FLUSH BIT(5) > +#define PCIE_ISR1_ALL_MASK GENMASK(5, 4) > +#define PCIE_MSI_ADDR_LOW_REG (CONTROL_BASE_ADDR + 0x50) > +#define PCIE_MSI_ADDR_HIGH_REG (CONTROL_BASE_ADDR + 0x54) > +#define PCIE_MSI_STATUS_REG (CONTROL_BASE_ADDR + 0x58) > +#define PCIE_MSI_MASK_REG (CONTROL_BASE_ADDR + 0x5C) > +#define PCIE_MSI_PAYLOAD_REG (CONTROL_BASE_ADDR + 0x9C) > + > +/* PCIe window configuration */ > +#define OB_WIN_BASE_ADDR 0x4c00 > +#define OB_WIN_BLOCK_SIZE 0x20 > +#define OB_WIN_REG_ADDR(win, offset) (OB_WIN_BASE_ADDR + \ > + OB_WIN_BLOCK_SIZE * (win) + \ > + (offset)) > +#define OB_WIN_MATCH_LS(win) OB_WIN_REG_ADDR(win, 0x00) > +#define OB_WIN_MATCH_MS(win) OB_WIN_REG_ADDR(win, 0x04) > +#define OB_WIN_REMAP_LS(win) OB_WIN_REG_ADDR(win, 0x08) > +#define OB_WIN_REMAP_MS(win) OB_WIN_REG_ADDR(win, 0x0c) > +#define OB_WIN_MASK_LS(win) OB_WIN_REG_ADDR(win, 0x10) > +#define OB_WIN_MASK_MS(win) OB_WIN_REG_ADDR(win, 0x14) > +#define OB_WIN_ACTIONS(win) OB_WIN_REG_ADDR(win, 0x18) > + > +/* PCIe window types */ > +#define OB_PCIE_MEM 0x0 > +#define OB_PCIE_IO 0x4 > +#define OB_PCIE_CONFIG0 0x8 > +#define OB_PCIE_CONFIG1 0x9 > +#define OB_PCIE_MSG 0xc > +#define OB_PCIE_MSG_VENDOR 0xd Some of these definitions (ISR bits above, these window types, probably others) are not actually used. I usually omit unused ones on the theory that they add bulk and opportunities for transcription errors. But if they're useful to you, I'm OK with keeping them. > +/* LMI registers base address and register offsets */ > +#define LMI_BASE_ADDR 0x6000 > +#define CFG_REG (LMI_BASE_ADDR + 0x0) > +#define LTSSM_SHIFT 24 > +#define LTSSM_MASK 0x3f > +#define LTSSM_L0 0x10 > +#define RC_BAR_CONFIG 0x300 > + > +/* PCIe core controller registers */ > +#define CTRL_CORE_BASE_ADDR 0x18000 > +#define CTRL_CONFIG_REG (CTRL_CORE_BASE_ADDR + 0x0) > +#define CTRL_MODE_SHIFT 0x0 > +#define CTRL_MODE_MASK 0x1 > +#define PCIE_CORE_MODE_DIRECT 0x0 > +#define PCIE_CORE_MODE_COMMAND 0x1 > + > +/* PCIe Central Interrupts Registers */ > +#define CENTRAL_INT_BASE_ADDR 0x1b000 > +#define HOST_CTRL_INT_STATUS_REG (CENTRAL_INT_BASE_ADDR + 0x0) > +#define HOST_CTRL_INT_MASK_REG (CENTRAL_INT_BASE_ADDR + 0x4) > +#define PCIE_IRQ_CMDQ_INT BIT(0) > +#define PCIE_IRQ_MSI_STATUS_INT BIT(1) > +#define PCIE_IRQ_CMD_SENT_DONE BIT(3) > +#define PCIE_IRQ_DMA_INT BIT(4) > +#define PCIE_IRQ_IB_DXFERDONE BIT(5) > +#define PCIE_IRQ_OB_DXFERDONE BIT(6) > +#define PCIE_IRQ_OB_RXFERDONE BIT(7) > +#define PCIE_IRQ_COMPQ_INT BIT(12) > +#define PCIE_IRQ_DIR_RD_DDR_DET BIT(13) > +#define PCIE_IRQ_DIR_WR_DDR_DET BIT(14) > +#define PCIE_IRQ_CORE_INT BIT(16) > +#define PCIE_IRQ_CORE_INT_PIO BIT(17) > +#define PCIE_IRQ_DPMU_INT BIT(18) > +#define PCIE_IRQ_PCIE_MIS_INT BIT(19) > +#define PCIE_IRQ_MSI_INT1_DET BIT(20) > +#define PCIE_IRQ_MSI_INT2_DET BIT(21) > +#define PCIE_IRQ_RC_DBELL_DET BIT(22) > +#define PCIE_IRQ_EP_STATUS BIT(23) > +#define PCIE_IRQ_ALL_MASK 0xfff0fb > +#define PCIE_IRQ_ENABLE_INTS_MASK PCIE_IRQ_CORE_INT > + > +/* Transaction types */ > +#define PCIE_CONFIG_RD_TYPE0 0x8 > +#define PCIE_CONFIG_RD_TYPE1 0x9 > +#define PCIE_CONFIG_WR_TYPE0 0xa > +#define PCIE_CONFIG_WR_TYPE1 0xb > + > +/* PCI_BDF shifts 8bit, so we need extra 4bit shift */ > +#define PCIE_BDF(dev) (dev << 4) > +#define PCIE_CONF_BUS(bus) (((bus) & 0xff) << 20) > +#define PCIE_CONF_DEV(dev) (((dev) & 0x1f) << 15) > +#define PCIE_CONF_FUNC(fun) (((fun) & 0x7) << 12) > +#define PCIE_CONF_REG(reg) ((reg) & 0xffc) > +#define PCIE_CONF_ADDR(bus, devfn, where) \ > + (PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn)) | \ > + PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where)) > + > +#define PIO_TIMEOUT_MS (1) > +#define PCIE_LINKUP_TIMEOUT_MS (10) Bare numbers do not require parentheses. > +#define LEGACY_IRQ_NUM 4 > +#define MSI_IRQ_NUM 32 > + > +struct advk_pcie { > + struct platform_device *pdev; > + void __iomem *base; > + struct list_head resources; > + struct irq_domain *irq_domain; > + struct irq_chip irq_chip; > + struct msi_controller msi; > + struct irq_domain *msi_domain; > + struct irq_chip msi_irq_chip; > + DECLARE_BITMAP(msi_irq_in_use, MSI_IRQ_NUM); > + struct mutex msi_used_lock; > + u16 msi_msg; > +}; > + > +static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg) > +{ > + writel(val, pcie->base + reg); > +} > + > +static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg) > +{ > + return readl(pcie->base + reg); > +} > + > +static bool advk_pcie_link_up(struct advk_pcie *pcie) > +{ > + unsigned long timeout; > + u32 ltssm_state; > + u32 val; > + > + timeout = jiffies + msecs_to_jiffies(PCIE_LINKUP_TIMEOUT_MS); > + > + while (time_before(jiffies, timeout)) { > + val = advk_readl(pcie, CFG_REG); > + ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK; > + if (ltssm_state >= LTSSM_L0) > + return true; > + } To try to improve consistency with other similar drivers, please make advk_pcie_link_up() a simple function that contains one register read and no loop. Then make a second advk_wait_for_link() function with a loop and timeout. Please use the same timeout/usleep structure used in dw_pcie_wait_for_link() and nwl_wait_for_link(). > + return false; > +} > + > +/* > + * Set PCIe address window register which could be used for memory > + * mapping. > + */ > +static void advk_pcie_set_ob_win(struct advk_pcie *pcie, > + u32 win_num, u32 match_ms, > + u32 match_ls, u32 mask_ms, > + u32 mask_ls, u32 remap_ms, > + u32 remap_ls, u32 action) > +{ > + advk_writel(pcie, match_ls, OB_WIN_MATCH_LS(win_num)); > + advk_writel(pcie, match_ms, OB_WIN_MATCH_MS(win_num)); > + advk_writel(pcie, mask_ms, OB_WIN_MASK_MS(win_num)); > + advk_writel(pcie, mask_ls, OB_WIN_MASK_LS(win_num)); > + advk_writel(pcie, remap_ms, OB_WIN_REMAP_MS(win_num)); > + advk_writel(pcie, remap_ls, OB_WIN_REMAP_LS(win_num)); > + advk_writel(pcie, action, OB_WIN_ACTIONS(win_num)); > + advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num)); > +} > + > +static void advk_pcie_setup_hw(struct advk_pcie *pcie) > +{ > + u32 reg; > + int i; > + > + /* Point PCIe unit MBUS decode windows to DRAM space. */ Tidy up the comments here by making them all one-line (when they fit) and consistently omitting (or keeping) the periods. > + for (i = 0; i < 8; i++) > + advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0); > + > + /* Set to Direct mode. */ > + reg = advk_readl(pcie, CTRL_CONFIG_REG); > + reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT); > + reg |= ((PCIE_CORE_MODE_DIRECT & CTRL_MODE_MASK) << CTRL_MODE_SHIFT); > + advk_writel(pcie, reg, CTRL_CONFIG_REG); > + > + /* Set PCI global control register to RC mode */ > + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); > + reg |= (IS_RC_MSK << IS_RC_SHIFT); > + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); > + > + /* > + * Set Advanced Error Capabilities and Control PF0 register > + */ > + reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX | > + PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN | > + PCIE_CORE_ERR_CAPCTL_ECRC_CHCK | > + PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV; > + advk_writel(pcie, reg, PCIE_CORE_ERR_CAPCTL_REG); > + > + /* > + * Set PCIe Device Control and Status 1 PF0 register > + */ > + reg = PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE | > + (7 << PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT) | > + PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE | > + PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT; > + advk_writel(pcie, reg, PCIE_CORE_DEV_CTRL_STATS_REG); > + > + /* > + * Program PCIe Control 2 to disable strict ordering. > + */ > + reg = PCIE_CORE_CTRL2_RESERVED | > + PCIE_CORE_CTRL2_TD_ENABLE; > + advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG); > + > + /* Set GEN2 */ > + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); > + reg &= ~PCIE_GEN_SEL_MSK; > + reg |= SPEED_GEN_2; > + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); > + > + /* Set lane X1 */ > + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); > + reg &= ~LANE_CNT_MSK; > + reg |= LANE_COUNT_1; > + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); > + > + /* Enable link training */ > + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG); > + reg |= LINK_TRAINING_EN; > + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG); > + > + /* Enable MSI */ > + reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG); > + reg |= PCIE_CORE_CTRL2_MSI_ENABLE; > + advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG); > + > + /* Clear all interrupts. */ > + advk_writel(pcie, PCIE_ISR0_ALL_MASK, PCIE_ISR0_REG); > + advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_REG); > + advk_writel(pcie, PCIE_IRQ_ALL_MASK, HOST_CTRL_INT_STATUS_REG); > + > + /* Disable All ISR0/1 Sources */ > + reg = PCIE_ISR0_ALL_MASK; > + reg &= ~PCIE_ISR0_MSI_INT_PENDING; > + advk_writel(pcie, reg, PCIE_ISR0_MASK_REG); > + > + advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_MASK_REG); > + > + /* Unmask all MSI's */ > + advk_writel(pcie, 0, PCIE_MSI_MASK_REG); > + > + /* Enable summary interrupt for GIC SPI source */ > + reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK); > + advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG); > + > + /* Start link training */ > + reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG); > + reg |= PCIE_CORE_LINK_TRAINING; > + advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG); > + > + advk_pcie_link_up(pcie); > + > + reg = PCIE_CORE_LINK_L0S_ENTRY | > + (1 << PCIE_CORE_LINK_WIDTH_SHIFT); > + advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG); > + > + reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG); > + reg |= PCIE_CORE_CMD_MEM_ACCESS_EN | > + PCIE_CORE_CMD_IO_ACCESS_EN | > + PCIE_CORE_CMD_MEM_IO_REQ_EN; > + advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG); > +} > + > +/* > + * This routine is used to enable or disable AXI address window > + * location generation. > + * Disabled: No address window mapping. Use AXI user fields > + * provided by the AXI fabric. > + * Enabled: Enable the address window mapping. The HAL bridge > + * generates the AXI user field locally. Use the local generated AXI user > + * fields. > + * It should be disabled when accessing the PCIe device by PIO. > + * It should be enabled when accessing the PCIe device by memory > + * access directly. > + */ > +static void advk_pcie_enable_axi_addr_gen(struct advk_pcie *pcie, bool enable) > +{ > + u32 reg; > + > + reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG); > + if (enable) > + reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE; > + else > + reg &= ~PCIE_CORE_CTRL2_OB_WIN_ENABLE; > + advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG); > +} > + > +static void advk_pcie_check_pio_status(struct advk_pcie *pcie) > +{ > + u32 reg; > + unsigned int status; > + char *strcomp_status, *str_posted; > + > + reg = advk_readl(pcie, PIO_STAT); > + status = (reg & PIO_COMPLETION_STATUS_MASK) >> > + PIO_COMPLETION_STATUS_SHIFT; > + > + if (!status) > + return; > + > + switch (status) { > + case PIO_COMPLETION_STATUS_UR: > + strcomp_status = "UR"; > + break; > + case PIO_COMPLETION_STATUS_CRS: > + strcomp_status = "CRS"; > + break; > + case PIO_COMPLETION_STATUS_CA: > + strcomp_status = "CA"; > + break; > + default: > + strcomp_status = "Unknown"; > + break; > + } > + > + if (reg & PIO_NON_POSTED_REQ) > + str_posted = "Non-posted"; > + else > + str_posted = "Posted"; > + > + dev_err(&pcie->pdev->dev, > + "%s PIO Response Status: %s, %#x @ %#x\n", > + str_posted, strcomp_status, reg, > + advk_readl(pcie, PIO_ADDR_LS)); > +} > + > +static int advk_pcie_wait_pio(struct advk_pcie *pcie) > +{ > + unsigned long timeout; > + u32 reg, is_done; > + > + timeout = jiffies + msecs_to_jiffies(PIO_TIMEOUT_MS); > + > + while (time_before(jiffies, timeout)) { > + reg = advk_readl(pcie, PIO_START); > + is_done = advk_readl(pcie, PIO_ISR); > + if ((!reg) && is_done) > + return 0; > + } This busy-loop could use usleep_range() similar to what dw_pcie_wait_for_link() does. > + dev_err(&pcie->pdev->dev, "config read/write timed out\n"); > + return -ETIME; > +} > + > +static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, > + int where, int size, u32 *val) > +{ > + struct advk_pcie *pcie = bus->sysdata; > + u32 reg; > + int ret; > + > + if (PCI_SLOT(devfn) != 0) { > + *val = 0xffffffff; > + return PCIBIOS_DEVICE_NOT_FOUND; > + } > + > + advk_pcie_enable_axi_addr_gen(pcie, false); This AXI enable/disable requires a mutex to prevent another thread from performing a simulataneous config access, so please add a comment about which mutex you are relying on. I think there's a PCI global one, but I'd just like to make it explicit that we need it here, because many config accessors don't actually require the mutex. I assume the AXI enable/disable does not affect MMIO accesses by drivers to areas mapped by PCIe device BARs. If it did, that would be a pretty serious problem because it would be hard to ensure the mutual exclusion. > + /* Start PIO */ > + advk_writel(pcie, 0, PIO_START); > + advk_writel(pcie, 1, PIO_ISR); > + > + /* Program the control register */ > + if (bus->number == 0) > + advk_writel(pcie, PCIE_CONFIG_RD_TYPE0, PIO_CTRL); Your DT documentation requires the bus range, and of_pci_get_host_bridge_resources() parses it, so you probably should save that bus range from the DT in advk_pcie and use it here instead of assuming zero. > + else > + advk_writel(pcie, PCIE_CONFIG_RD_TYPE1, PIO_CTRL); > + > + /* Program the address registers */ > + reg = PCIE_BDF(devfn) | PCIE_CONF_REG(where); > + advk_writel(pcie, reg, PIO_ADDR_LS); > + advk_writel(pcie, 0, PIO_ADDR_MS); > + > + /* Program the data strobe */ > + advk_writel(pcie, 0xf, PIO_WR_DATA_STRB); > + > + /* Start the transfer */ > + advk_writel(pcie, 1, PIO_START); > + > + ret = advk_pcie_wait_pio(pcie); > + if (ret < 0) { > + advk_pcie_enable_axi_addr_gen(pcie, true); > + return PCIBIOS_SET_FAILED; > + } > + > + advk_pcie_check_pio_status(pcie); > + > + /* Get the read result */ > + *val = advk_readl(pcie, PIO_RD_DATA); > + if (size == 1) > + *val = (*val >> (8 * (where & 3))) & 0xff; > + else if (size == 2) > + *val = (*val >> (8 * (where & 3))) & 0xffff; > + > + advk_pcie_enable_axi_addr_gen(pcie, true); > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn, > + int where, int size, u32 val) > +{ > + struct advk_pcie *pcie = bus->sysdata; > + u32 reg; > + u32 data_strobe = 0x0; > + int offset; > + int ret; > + > + if (PCI_SLOT(devfn) != 0) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + advk_pcie_enable_axi_addr_gen(pcie, false); > + > + /* Start PIO */ > + advk_writel(pcie, 0, PIO_START); > + advk_writel(pcie, 1, PIO_ISR); > + > + if (bus->number == 0) > + advk_writel(pcie, PCIE_CONFIG_WR_TYPE0, PIO_CTRL); > + else > + advk_writel(pcie, PCIE_CONFIG_WR_TYPE1, PIO_CTRL); > + > + /* Program the address registers */ > + reg = PCIE_CONF_ADDR(bus->number, devfn, where); > + advk_writel(pcie, reg, PIO_ADDR_LS); > + advk_writel(pcie, 0, PIO_ADDR_MS); > + > + if (where % size) { > + advk_pcie_enable_axi_addr_gen(pcie, true); > + return PCIBIOS_SET_FAILED; This could be checked earlier, before fiddling with AXI and touching PIO_START. > + } > + > + /* Calculate the write strobe */ > + offset = where & 0x3; > + reg = val << (8 * offset); > + data_strobe = GENMASK(size - 1, 0) << offset; > + > + /* Program the data register */ > + advk_writel(pcie, reg, PIO_WR_DATA); > + > + /* Program the data strobe */ > + advk_writel(pcie, data_strobe, PIO_WR_DATA_STRB); > + > + /* Start the transfer */ > + advk_writel(pcie, 1, PIO_START); > + > + ret = advk_pcie_wait_pio(pcie); > + if (ret < 0) { > + advk_pcie_enable_axi_addr_gen(pcie, true); > + return PCIBIOS_SET_FAILED; > + } > + > + advk_pcie_check_pio_status(pcie); > + > + advk_pcie_enable_axi_addr_gen(pcie, true); > + > + return PCIBIOS_SUCCESSFUL; > +} > + > +static struct pci_ops advk_pcie_ops = { > + .read = advk_pcie_rd_conf, > + .write = advk_pcie_wr_conf, > +}; > + > +static int advk_pcie_alloc_msi(struct advk_pcie *pcie) > +{ > + int hwirq; > + > + mutex_lock(&pcie->msi_used_lock); > + hwirq = find_first_zero_bit(pcie->msi_irq_in_use, MSI_IRQ_NUM); > + if (hwirq >= MSI_IRQ_NUM) > + hwirq = -ENOSPC; > + else > + set_bit(hwirq, pcie->msi_irq_in_use); > + mutex_unlock(&pcie->msi_used_lock); > + > + return hwirq; > +} > + > +static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq) > +{ > + mutex_lock(&pcie->msi_used_lock); > + if (!test_bit(hwirq, pcie->msi_irq_in_use)) > + pr_err("trying to free unused MSI#%d\n", hwirq); Use dev_err(). > + else > + clear_bit(hwirq, pcie->msi_irq_in_use); > + mutex_unlock(&pcie->msi_used_lock); > +} > + > +static int advk_pcie_setup_msi_irq(struct msi_controller *chip, > + struct pci_dev *pdev, > + struct msi_desc *desc) > +{ > + struct advk_pcie *pcie = pdev->bus->sysdata; > + struct msi_msg msg; > + int virq, hwirq; > + phys_addr_t msi_msg_phys; > + > + /* We support MSI, but not MSI-X */ > + if (desc->msi_attrib.is_msix) > + return -EINVAL; > + > + hwirq = advk_pcie_alloc_msi(pcie); > + if (hwirq < 0) > + return hwirq; > + > + virq = irq_create_mapping(pcie->msi_domain, hwirq); > + if (!virq) { > + advk_pcie_free_msi(pcie, hwirq); > + return -EINVAL; > + } > + > + irq_set_msi_desc(virq, desc); > + > + msi_msg_phys = virt_to_phys(&pcie->msi_msg); > + > + msg.address_lo = lower_32_bits(msi_msg_phys); > + msg.address_hi = upper_32_bits(msi_msg_phys); > + msg.data = virq; > + > + pci_write_msi_msg(virq, &msg); > + > + return 0; > +} > + > +static void advk_pcie_teardown_msi_irq(struct msi_controller *chip, > + unsigned int irq) > +{ > + struct irq_data *d = irq_get_irq_data(irq); > + struct msi_desc *msi = irq_data_get_msi_desc(d); > + struct advk_pcie *pcie = msi_desc_to_pci_sysdata(msi); > + unsigned long hwirq = d->hwirq; > + > + irq_dispose_mapping(irq); > + advk_pcie_free_msi(pcie, hwirq); > +} > + > +static int advk_pcie_msi_map(struct irq_domain *domain, > + unsigned int virq, irq_hw_number_t hw) > +{ > + struct advk_pcie *pcie = domain->host_data; > + > + irq_set_chip_and_handler(virq, &pcie->msi_irq_chip, > + handle_simple_irq); > + > + return 0; > +} > + > +static const struct irq_domain_ops advk_pcie_msi_irq_ops = { > + .map = advk_pcie_msi_map, > +}; > + > +static void advk_pcie_irq_mask(struct irq_data *d) > +{ > + struct advk_pcie *pcie = d->domain->host_data; > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > + u32 mask; > + > + mask = advk_readl(pcie, PCIE_ISR0_MASK_REG); > + mask |= PCIE_ISR0_INTX_ASSERT(hwirq); > + advk_writel(pcie, mask, PCIE_ISR0_MASK_REG); > +} > + > +static void advk_pcie_irq_unmask(struct irq_data *d) > +{ > + struct advk_pcie *pcie = d->domain->host_data; > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > + u32 mask; > + > + mask = advk_readl(pcie, PCIE_ISR0_MASK_REG); > + mask &= ~PCIE_ISR0_INTX_ASSERT(hwirq); > + advk_writel(pcie, mask, PCIE_ISR0_MASK_REG); > +} > + > +static int advk_pcie_irq_map(struct irq_domain *h, > + unsigned int virq, irq_hw_number_t hwirq) > +{ > + struct advk_pcie *pcie = h->host_data; > + > + advk_pcie_irq_mask(irq_get_irq_data(virq)); > + irq_set_status_flags(virq, IRQ_LEVEL); > + irq_set_chip_and_handler(virq, &pcie->irq_chip, > + handle_level_irq); > + irq_set_chip_data(virq, pcie); > + > + return 0; > +} > + > +static const struct irq_domain_ops advk_pcie_irq_domain_ops = { > + .map = advk_pcie_irq_map, > + .xlate = irq_domain_xlate_onecell, > +}; > + > +static int advk_pcie_init_irq(struct advk_pcie *pcie) > +{ > + struct device *dev = &pcie->pdev->dev; > + struct device_node *node = dev->of_node; > + struct device_node *pcie_intc_node; > + struct irq_chip *irq_chip, *msi_irq_chip; > + struct msi_controller *msi; > + phys_addr_t msi_msg_phys; > + int ret; > + > + pcie_intc_node = of_get_next_child(node, NULL); > + if (!pcie_intc_node) { > + dev_err(dev, "No PCIe Intc node found\n"); > + return PTR_ERR(pcie_intc_node); > + } > + > + irq_chip = &pcie->irq_chip; > + > + irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq", > + dev_name(dev)); > + if (!irq_chip->name) > + return -ENOMEM; > + irq_chip->irq_mask = advk_pcie_irq_mask; > + irq_chip->irq_mask_ack = advk_pcie_irq_mask; > + irq_chip->irq_unmask = advk_pcie_irq_unmask; > + > + pcie->irq_domain = > + irq_domain_add_linear(pcie_intc_node, LEGACY_IRQ_NUM, > + &advk_pcie_irq_domain_ops, pcie); > + if (!pcie->irq_domain) { > + dev_err(dev, "Failed to get a INTx IRQ domain\n"); > + return PTR_ERR(pcie->irq_domain); > + } Can you rename this to advk_pcie_init_irq_domain() and possibly split to advk_pcie_init_msi_irq_domain() so it looks more similar to the other drivers, e.g., nwl_pcie_init_irq_domain() or xilinx_pcie_init_irq_domain()? > + msi_irq_chip = &pcie->msi_irq_chip; > + > + msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi", > + dev_name(dev)); > + if (!msi_irq_chip->name) { > + irq_domain_remove(pcie->irq_domain); > + return -ENOMEM; > + } > + > + msi_irq_chip->irq_enable = pci_msi_unmask_irq; > + msi_irq_chip->irq_disable = pci_msi_mask_irq; > + msi_irq_chip->irq_mask = pci_msi_mask_irq; > + msi_irq_chip->irq_unmask = pci_msi_unmask_irq; > + > + msi = &pcie->msi; > + > + msi->setup_irq = advk_pcie_setup_msi_irq; > + msi->teardown_irq = advk_pcie_teardown_msi_irq; > + msi->of_node = node; > + > + mutex_init(&pcie->msi_used_lock); > + > + msi_msg_phys = virt_to_phys(&pcie->msi_msg); > + > + advk_writel(pcie, lower_32_bits(msi_msg_phys), > + PCIE_MSI_ADDR_LOW_REG); > + advk_writel(pcie, upper_32_bits(msi_msg_phys), > + PCIE_MSI_ADDR_HIGH_REG); > + > + pcie->msi_domain = > + irq_domain_add_linear(NULL, MSI_IRQ_NUM, > + &advk_pcie_msi_irq_ops, pcie); > + if (!pcie->msi_domain) { > + irq_domain_remove(pcie->irq_domain); > + return -ENOMEM; > + } > + > + ret = of_pci_msi_chip_add(msi); > + if (ret < 0) { > + irq_domain_remove(pcie->msi_domain); > + irq_domain_remove(pcie->irq_domain); > + return ret; > + } > + > + return 0; > +} > + > +static void advk_pcie_handle_msi(struct advk_pcie *pcie) > +{ > + u32 msi_val, msi_mask, msi_status, msi_idx; > + u16 msi_data; > + > + msi_mask = advk_readl(pcie, PCIE_MSI_MASK_REG); > + msi_val = advk_readl(pcie, PCIE_MSI_STATUS_REG); > + msi_status = msi_val & ~msi_mask; > + > + for (msi_idx = 0; msi_idx < MSI_IRQ_NUM; msi_idx++) { > + if (!(BIT(msi_idx) & msi_status)) > + continue; > + > + advk_writel(pcie, BIT(msi_idx), PCIE_MSI_STATUS_REG); > + msi_data = advk_readl(pcie, PCIE_MSI_PAYLOAD_REG) & 0xFF; > + generic_handle_irq(msi_data); > + } > + > + advk_writel(pcie, PCIE_ISR0_MSI_INT_PENDING, > + PCIE_ISR0_REG); > +} > + > +static void advk_pcie_handle_int(struct advk_pcie *pcie) > +{ > + u32 val, mask, status; > + int i, virq; > + > + val = advk_readl(pcie, PCIE_ISR0_REG); > + mask = advk_readl(pcie, PCIE_ISR0_MASK_REG); > + status = val & ((~mask) & PCIE_ISR0_ALL_MASK); > + > + if (!status) { > + advk_writel(pcie, val, PCIE_ISR0_REG); > + return; > + } > + > + /* Process MSI interrupts */ > + if (status & PCIE_ISR0_MSI_INT_PENDING) > + advk_pcie_handle_msi(pcie); > + > + /* Process legacy interrupts */ > + for (i = 0; i < LEGACY_IRQ_NUM; i++) { > + if (!(status & PCIE_ISR0_INTX_ASSERT(i))) > + continue; > + > + advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i), > + PCIE_ISR0_REG); > + > + virq = irq_find_mapping(pcie->irq_domain, i); > + generic_handle_irq(virq); > + } > +} > + > +static irqreturn_t advk_pcie_irq_handler(int irq, void *arg) > +{ > + struct advk_pcie *pcie = arg; > + u32 status; > + > + status = advk_readl(pcie, HOST_CTRL_INT_STATUS_REG); > + if (!(status & PCIE_IRQ_CORE_INT)) > + return IRQ_NONE; > + > + advk_pcie_handle_int(pcie); > + > + /* Clear interrupt */ > + advk_writel(pcie, PCIE_IRQ_CORE_INT, HOST_CTRL_INT_STATUS_REG); > + > + return IRQ_HANDLED; > +} > + > +static void advk_pcie_release_of_pci_ranges(struct advk_pcie *pcie) > +{ > + pci_free_resource_list(&pcie->resources); You can inline this call below; I know other drivers wrap it too, but I don't know why. > +} > + > +static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie) > +{ > + int err, res_valid = 0; > + struct device *dev = &pcie->pdev->dev; > + struct device_node *np = dev->of_node; > + struct resource_entry *win; > + resource_size_t iobase; > + > + INIT_LIST_HEAD(&pcie->resources); > + > + err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources, > + &iobase); > + if (err) > + return err; > + > + resource_list_for_each_entry(win, &pcie->resources) { > + struct resource *parent, *res = win->res; > + > + switch (resource_type(res)) { > + case IORESOURCE_IO: > + parent = &ioport_resource; > + advk_pcie_set_ob_win(pcie, 1, > + upper_32_bits(res->start), > + lower_32_bits(res->start), > + 0, 0xF8000000, 0, > + lower_32_bits(res->start), > + OB_PCIE_IO); > + err = pci_remap_iospace(res, iobase); > + if (err) { > + dev_warn(dev, "error %d: failed to map resource %pR\n", > + err, res); > + continue; > + } > + break; > + case IORESOURCE_MEM: > + parent = &iomem_resource; > + advk_pcie_set_ob_win(pcie, 0, > + upper_32_bits(res->start), > + lower_32_bits(res->start), > + 0x0, 0xF8000000, 0, > + lower_32_bits(res->start), > + (2 << 20) | OB_PCIE_MEM); > + res_valid |= !(res->flags & IORESOURCE_PREFETCH); > + break; > + default: > + continue; > + } > + > + err = devm_request_resource(dev, parent, res); Thanks for including this. I'll try to remember to update this to use devm_request_pci_bus_resources() when I merge it (that's currently on my pci/host-request-windows branch, not upstream yet). > + if (err) > + goto out_release_res; > + } > + > + if (!res_valid) { > + dev_err(dev, "non-prefetchable memory resource required\n"); > + err = -EINVAL; > + goto out_release_res; > + } > + > + return 0; > + > +out_release_res: > + advk_pcie_release_of_pci_ranges(pcie); > + return err; > +} > + > +static int advk_pcie_probe(struct platform_device *pdev) > +{ > + struct advk_pcie *pcie; > + struct resource *res; > + struct pci_bus *bus, *child; > + struct msi_controller *msi; > + struct device_node *msi_node; > + int ret, irq; > + > + pcie = devm_kzalloc(&pdev->dev, sizeof(struct advk_pcie), > + GFP_KERNEL); > + if (!pcie) > + return -ENOMEM; > + > + pcie->pdev = pdev; > + platform_set_drvdata(pdev, pcie); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + pcie->base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(pcie->base)) { > + dev_err(&pdev->dev, "failed to map registers\n"); > + return PTR_ERR(pcie->base); > + } > + > + irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, irq, advk_pcie_irq_handler, > + IRQF_SHARED | IRQF_NO_THREAD, "advk-pcie", > + pcie); > + if (ret) { > + dev_err(&pdev->dev, "Failed to register interrupt\n"); > + return ret; > + } > + > + ret = advk_pcie_parse_request_of_pci_ranges(pcie); > + if (ret) { > + dev_err(&pdev->dev, "Failed to parse resources\n"); > + return ret; > + } > + > + advk_pcie_setup_hw(pcie); > + advk_pcie_enable_axi_addr_gen(pcie, true); > + > + ret = advk_pcie_init_irq(pcie); > + if (ret) { > + dev_err(&pdev->dev, "Failed to initialize irq\n"); > + return ret; > + } > + > + msi_node = of_parse_phandle(pdev->dev.of_node, "msi-parent", 0); > + if (msi_node) > + msi = of_pci_find_msi_chip_by_node(msi_node); > + else > + msi = NULL; > + > + bus = pci_scan_root_bus_msi(&pdev->dev, 0, &advk_pcie_ops, > + pcie, &pcie->resources, &pcie->msi); > + if (!bus) > + return -ENOMEM; > + > + pci_bus_assign_resources(bus); > + > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + > + pci_bus_add_devices(bus); > + > + return 0; > +} > + > +static const struct of_device_id advk_pcie_of_match_table[] = { > + { .compatible = "marvell,armada-3700-pcie", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table); Since this driver is currently bool in Kconfig, can you omit the MODULE_*() uses? It would be ideal to make this modular, of course, but if it's not modular, let's make it so it doesn't *look* like a module. > +static struct platform_driver advk_pcie_driver = { > + .driver = { > + .name = "advk-pcie", > + .of_match_table = advk_pcie_of_match_table, > + /* Driver unloading/unbinding currently not supported */ > + .suppress_bind_attrs = true, I see some other drivers (mvebu, rcar, tegra, altera, xilinx) also use suppress_bind_attrs = true, but I don't know what's special about them. Do you know? Do we really need this here? What would it take to change this driver so we could omit it? > + }, > + .probe = advk_pcie_probe, > +}; > +module_platform_driver(advk_pcie_driver); > + > +MODULE_AUTHOR("Hezi Shahmoon <hezi.shahmoon@xxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Aardvark PCIe driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 > > -- > 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 -- 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
