[+Lorenzo] On 20/05/16 11:29, Shawn Lin wrote: > RK3399 has a PCIe controller which can be used as Root Complex. > This driver supports a PCIe controller as Root Complex mode. > > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> > --- > > drivers/pci/host/Kconfig | 12 + > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-rockchip.c | 1181 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 1194 insertions(+) > create mode 100644 drivers/pci/host/pcie-rockchip.c > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 8e4f038..76447a8 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -245,4 +245,16 @@ config PCIE_ARMADA_8K > Designware hardware and therefore the driver re-uses the > Designware core functions to implement the driver. > > +config PCIE_ROCKCHIP > + bool "Rockchip PCIe controller" > + depends on ARM64 && ARCH_ROCKCHIP > + depends on OF > + select MFD_SYSCON > + select PCI_MSI > + select PCI_MSI_IRQ_DOMAIN > + help > + Say Y here if you want internal PCI support on Rockchip SoC. > + There are 1 internal PCIe port available to support GEN2 with > + 4 slots. > + > endmenu > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index a6f85e3..fb3871e 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o > obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > +obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o > diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c > new file mode 100644 > index 0000000..4063fd3 > --- /dev/null > +++ b/drivers/pci/host/pcie-rockchip.c > @@ -0,0 +1,1181 @@ > +/* > + * Rockchip AXI PCIe host controller driver > + * > + * Copyright (c) 2016 Rockchip, Inc. > + * > + * Author: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> > + * Wenrui Li <wenrui.li@xxxxxxxxxxxxxx> > + * Bits taken from Synopsys Designware Host controller driver and > + * ARM PCI Host generic driver. > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 2 of the License, or > + * (at your option) any later version. > + */ > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/interrupt.h> > +#include <linux/irq.h> > +#include <linux/irqdomain.h> > +#include <linux/kernel.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/msi.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/of_pci.h> > +#include <linux/of_platform.h> > +#include <linux/of_irq.h> > +#include <linux/pci.h> > +#include <linux/platform_device.h> > +#include <linux/reset.h> > +#include <linux/regmap.h> > + > +#define REF_CLK_100MHZ (100 * 1000 * 1000) > +#define PCIE_CLIENT_BASE 0x0 > +#define PCIE_RC_CONFIG_BASE 0xa00000 > +#define PCIE_CORE_CTRL_MGMT_BASE 0x900000 > +#define PCIE_CORE_AXI_CONF_BASE 0xc00000 > +#define PCIE_CORE_AXI_INBOUND_BASE 0xc00800 > + > +#define PCIE_CLIENT_BASIC_STATUS0 0x44 > +#define PCIE_CLIENT_BASIC_STATUS1 0x48 > +#define PCIE_CLIENT_INT_MASK 0x4c > +#define PCIE_CLIENT_INT_STATUS 0x50 > +#define PCIE_CORE_INT_MASK 0x900210 > +#define PCIE_CORE_INT_STATUS 0x90020c > + > +/** Size of one AXI Region (not Region 0) */ > +#define AXI_REGION_SIZE (0x1 << 20) > +/** Overall size of AXI area */ > +#define AXI_OVERALL_SIZE (64 * (0x1 << 20)) > +/** Size of Region 0, equal to sum of sizes of other regions */ > +#define AXI_REGION_0_SIZE (32 * (0x1 << 20)) > +#define OB_REG_SIZE_SHIFT 5 > +#define IB_ROOT_PORT_REG_SIZE_SHIFT 3 > + > +#define AXI_WRAPPER_IO_WRITE 0x6 > +#define AXI_WRAPPER_MEM_WRITE 0x2 > +#define MAX_AXI_IB_ROOTPORT_REGION_NUM 3 > +#define MIN_AXI_ADDR_BITS_PASSED 8 > + > +#define ROCKCHIP_PCIE_RPIFR1_INTR_MASK GENMASK(8, 5) > +#define ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT 5 > +#define CLIENT_INTERRUPTS \ > + (LOC_INT | INTA | INTB | INTC | INTD |\ > + CORR_ERR | NFATAL_ERR | FATAL_ERR | DPA_INT | \ > + HOT_RESET | MSG_DONE | LEGACY_DONE) > +#define CORE_INTERRUPTS \ > + (PRFPE | CRFPE | RRPE | CRFO | RT | RTR | \ > + PE | MTR | UCR | FCE | CT | UTC | MMVC) > +#define PWR_STCG BIT(0) > +#define HOT_PLUG BIT(1) > +#define PHY_INT BIT(2) > +#define UDMA_INT BIT(3) > +#define LOC_INT BIT(4) > +#define INTA BIT(5) > +#define INTB BIT(6) > +#define INTC BIT(7) > +#define INTD BIT(8) > +#define CORR_ERR BIT(9) > +#define NFATAL_ERR BIT(10) > +#define FATAL_ERR BIT(11) > +#define DPA_INT BIT(12) > +#define HOT_RESET BIT(13) > +#define MSG_DONE BIT(14) > +#define LEGACY_DONE BIT(15) > +#define PRFPE BIT(0) > +#define CRFPE BIT(1) > +#define RRPE BIT(2) > +#define PRFO BIT(3) > +#define CRFO BIT(4) > +#define RT BIT(5) > +#define RTR BIT(6) > +#define PE BIT(7) > +#define MTR BIT(8) > +#define UCR BIT(9) > +#define FCE BIT(10) > +#define CT BIT(11) > +#define UTC BIT(18) > +#define MMVC BIT(19) > + > +#define PCIE_ECAM_BUS(x) (((x) & 0xFF) << 20) > +#define PCIE_ECAM_DEV(x) (((x) & 0x1F) << 15) > +#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12) > +#define PCIE_ECAM_REG(x) (((x) & 0xFFF) << 0) > +#define PCIE_ECAM_ADDR(bus, dev, func, reg) \ > + (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \ > + PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg)) > + > +#define RC_REGION_0_ADDR_TRANS_H 0x00000000 > +#define RC_REGION_0_ADDR_TRANS_L 0x00000000 > +#define RC_REGION_0_PASS_BITS (25 - 1) > +#define RC_REGION_1_ADDR_TRANS_H 0x00000000 > +#define RC_REGION_1_ADDR_TRANS_L 0x00400000 > +#define RC_REGION_1_PASS_BITS (20 - 1) > +#define MAX_AXI_WRAPPER_REGION_NUM 33 > +#define PCIE_CLIENT_CONF_ENABLE BIT(0) > +#define PCIE_CLIENT_CONF_LANE_NUM(x) ((x / 2) << 4) > +#define PCIE_CLIENT_MODE_RC BIT(6) > +#define PCIE_CLIENT_GEN_SEL_2 BIT(7) > +#define PCIE_CLIENT_GEN_SEL_1 0x0 > + > +struct rockchip_pcie_port { > + void __iomem *reg_base; > + void __iomem *apb_base; > + struct regmap *grf; > + unsigned int pcie_conf; > + unsigned int pcie_status; > + unsigned int pcie_laneoff; > + struct reset_control *phy_rst; > + struct reset_control *core_rst; > + struct reset_control *mgmt_rst; > + struct reset_control *mgmt_sticky_rst; > + struct reset_control *pipe_rst; > + struct clk *aclk_pcie; > + struct clk *aclk_perf_pcie; > + struct clk *hclk_pcie; > + struct clk *clk_pciephy_ref; > + struct gpio_desc *ep_gpio; > + u32 lanes; > + resource_size_t io_base; > + struct resource *cfg; > + struct resource *io; > + struct resource *mem; > + struct resource *busn; > + phys_addr_t io_bus_addr; > + u32 io_size; > + phys_addr_t mem_bus_addr; > + u32 mem_size; > + u8 root_bus_nr; > + int irq; > + struct msi_controller *msi; Don't. struct msi_controller shouldn't be used in new code, and certainly not for an arm64 system. But even worse: your SoC seems to use a GICv3 ITS. Great. Which means you do not need any of that at all. > + > + struct device *dev; > + struct irq_domain *irq_domain; > +}; [...] > +static void rockchip_pcie_msi_enable(struct rockchip_pcie_port *pp) > +{ > + struct device_node *msi_node; > + > + msi_node = of_parse_phandle(pp->dev->of_node, > + "msi-parent", 0); > + if (!msi_node) > + return; > + > + pp->msi = of_pci_find_msi_chip_by_node(msi_node); How funny. The ITS is never registered there (actually, nobody seems to register anything there anymore), so this will always return NULL. This whole function is thus completely useless. > + of_node_put(msi_node); > + > + if (pp->msi) > + pp->msi->dev = pp->dev; > +} > + > +static void rockchip_pcie_enable_interrupts(struct rockchip_pcie_port *pp) > +{ > + pcie_write(pp, (CLIENT_INTERRUPTS << 16) & > + (~CLIENT_INTERRUPTS), PCIE_CLIENT_INT_MASK); > + pcie_write(pp, CORE_INTERRUPTS, PCIE_CORE_INT_MASK); > +} > + > +static int rockchip_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; > +} > + > +static const struct irq_domain_ops intx_domain_ops = { > + .map = rockchip_pcie_intx_map, > +}; > + > +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *pp) > +{ > + struct device *dev = pp->dev; > + struct device_node *node = dev->of_node; > + struct device_node *pcie_intc_node = of_get_next_child(node, NULL); That's really ugly, as it depends on the layout of your DT. > + > + if (!pcie_intc_node) { > + dev_err(dev, "No PCIe Intc node found\n"); > + return PTR_ERR(pcie_intc_node); > + } > + pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4, > + &intx_domain_ops, pp); Why can't you just register your host controller as the interrupt controller? You don't need an intermediate node for that. > + if (!pp->irq_domain) { > + dev_err(dev, "Failed to get a INTx IRQ domain\n"); > + return PTR_ERR(pp->irq_domain); > + } > + > + return 0; > +} > + > +static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg) > +{ > + struct rockchip_pcie_port *pp = arg; > + u32 reg; > + u32 sub_reg; > + > + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS); > + if (reg & LOC_INT) { > + dev_dbg(pp->dev, "local interrupt recived\n"); > + sub_reg = pcie_read(pp, PCIE_CORE_INT_STATUS); > + if (sub_reg & PRFPE) > + dev_dbg(pp->dev, "Parity error detected while reading from the PNP Receive FIFO RAM\n"); > + > + if (sub_reg & CRFPE) > + dev_dbg(pp->dev, "Parity error detected while reading from the Completion Receive FIFO RAM\n"); > + > + if (sub_reg & RRPE) > + dev_dbg(pp->dev, "Parity error detected while reading from Replay Buffer RAM\n"); > + > + if (sub_reg & PRFO) > + dev_dbg(pp->dev, "Overflow occurred in the PNP Receive FIFO\n"); > + > + if (sub_reg & CRFO) > + dev_dbg(pp->dev, "Overflow occurred in the Completion Receive FIFO\n"); > + > + if (sub_reg & RT) > + dev_dbg(pp->dev, "Replay timer timed out\n"); > + > + if (sub_reg & RTR) > + dev_dbg(pp->dev, "Replay timer rolled over after 4 transmissions of the same TLP\n"); > + > + if (sub_reg & PE) > + dev_dbg(pp->dev, "Phy error detected on receive side\n"); > + > + if (sub_reg & MTR) > + dev_dbg(pp->dev, "Malformed TLP received from the link\n"); > + > + if (sub_reg & UCR) > + dev_dbg(pp->dev, "Malformed TLP received from the link\n"); > + > + if (sub_reg & FCE) > + dev_dbg(pp->dev, "An error was observed in the flow control advertisements from the other side\n"); > + > + if (sub_reg & CT) > + dev_dbg(pp->dev, "A request timed out waiting for completion\n"); > + > + if (sub_reg & UTC) > + dev_dbg(pp->dev, "Unmapped TC error\n"); > + > + if (sub_reg & MMVC) > + dev_dbg(pp->dev, "MSI mask register changes\n"); > + > + pcie_write(pp, sub_reg, PCIE_CORE_INT_STATUS); > + } > + > + pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg) > +{ > + struct rockchip_pcie_port *pp = arg; > + u32 reg; > + > + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS); > + if (reg & LEGACY_DONE) > + dev_dbg(pp->dev, "legacy done interrupt recived\n"); > + > + if (reg & MSG_DONE) > + dev_dbg(pp->dev, "message done interrupt recived\n"); > + > + if (reg & HOT_RESET) > + dev_dbg(pp->dev, "hot reset interrupt recived\n"); > + > + if (reg & DPA_INT) > + dev_dbg(pp->dev, "dpa interrupt recived\n"); > + > + if (reg & FATAL_ERR) > + dev_dbg(pp->dev, "fatal error interrupt recived\n"); > + > + if (reg & DPA_INT) > + dev_dbg(pp->dev, "no fatal error interrupt recived\n"); > + > + if (reg & CORR_ERR) > + dev_dbg(pp->dev, "correctable error interrupt recived\n"); > + > + pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t rockchip_pcie_legacy_int_handler(int irq, void *arg) > +{ > + struct rockchip_pcie_port *pp = arg; > + u32 reg; > + > + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS); > + reg = (reg & ROCKCHIP_PCIE_RPIFR1_INTR_MASK) >> > + ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT; > + generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg))); NAK. What you have here is a chained interrupt controller, please implement it as such. > + > + pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS); > + return IRQ_HANDLED; > +} > + > +static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *pp, > + int region_no, > + int type, u8 num_pass_bits, > + u32 lower_addr, u32 upper_addr) > +{ > + u32 ob_addr_0 = 0; > + u32 ob_addr_1 = 0; > + u32 ob_desc_0 = 0; > + u32 ob_desc_1 = 0; > + void __iomem *aw_base; > + > + if (!pp) > + return -EINVAL; > + if (region_no >= MAX_AXI_WRAPPER_REGION_NUM) > + return -EINVAL; > + if ((num_pass_bits + 1) < 8) > + return -EINVAL; > + if (num_pass_bits > 63) > + return -EINVAL; > + if (region_no == 0) { > + if (AXI_REGION_0_SIZE < (2ULL << num_pass_bits)) > + return -EINVAL; > + } > + if (region_no != 0) { > + if (AXI_REGION_SIZE < (2ULL << num_pass_bits)) > + return -EINVAL; > + } > + aw_base = pp->apb_base + PCIE_CORE_AXI_CONF_BASE; > + aw_base += (region_no << OB_REG_SIZE_SHIFT); > + > + ob_addr_0 = (ob_addr_0 & > + ~0x0000003fU) | (num_pass_bits & > + 0x0000003fU); > + ob_addr_0 = (ob_addr_0 & > + ~0xffffff00U) | (lower_addr & 0xffffff00U); > + ob_addr_1 = upper_addr; > + ob_desc_0 = (1 << 23 | type); > + > + writel(ob_addr_0, aw_base); > + writel(ob_addr_1, aw_base + 0x4); > + writel(ob_desc_0, aw_base + 0x8); > + writel(ob_desc_1, aw_base + 0xc); > + > + return 0; > +} > + > +static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *pp, > + int region_no, > + u8 num_pass_bits, > + u32 lower_addr, > + u32 upper_addr) > +{ > + u32 ib_addr_0 = 0; > + u32 ib_addr_1 = 0; > + void __iomem *aw_base; > + > + if (!pp) > + return -EINVAL; > + if (region_no > MAX_AXI_IB_ROOTPORT_REGION_NUM) > + return -EINVAL; > + if ((num_pass_bits + 1) < MIN_AXI_ADDR_BITS_PASSED) > + return -EINVAL; > + if (num_pass_bits > 63) > + return -EINVAL; > + aw_base = pp->apb_base + PCIE_CORE_AXI_INBOUND_BASE; > + aw_base += (region_no << IB_ROOT_PORT_REG_SIZE_SHIFT); > + ib_addr_0 = (ib_addr_0 & > + ~0x0000003fU) | (num_pass_bits & > + 0x0000003fU); > + > + ib_addr_0 = (ib_addr_0 & ~0xffffff00U) | > + ((lower_addr << 8) & 0xffffff00U); > + ib_addr_1 = upper_addr; > + writel(ib_addr_0, aw_base); > + writel(ib_addr_1, aw_base + 0x4); > + > + return 0; > +} > + > +static int rockchip_pcie_probe(struct platform_device *pdev) > +{ > + struct rockchip_pcie_port *port; > + struct device *dev = &pdev->dev; > + struct pci_bus *bus, *child; > + struct resource_entry *win; > + int reg_no; > + int err = 0; > + int irq; > + LIST_HEAD(res); > + > + if (!dev->of_node) > + return -ENODEV; > + > + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL); > + if (!port) > + return -ENOMEM; > + > + irq = platform_get_irq_byname(pdev, "pcie-sys"); > + if (irq < 0) { > + dev_err(dev, "missing pcie_sys IRQ resource\n"); > + return -EINVAL; > + } > + err = devm_request_irq(dev, irq, rockchip_pcie_subsys_irq_handler, > + IRQF_SHARED, "pcie-sys", port); > + if (err) { > + dev_err(dev, "failed to request pcie subsystem irq\n"); > + return err; > + } > + > + port->irq = platform_get_irq_byname(pdev, "pcie-legacy"); > + if (port->irq < 0) { > + dev_err(dev, "missing pcie_legacy IRQ resource\n"); > + return -EINVAL; > + } > + err = devm_request_irq(dev, port->irq, > + rockchip_pcie_legacy_int_handler, > + IRQF_SHARED, > + "pcie-legacy", > + port); > + if (err) { > + dev_err(&pdev->dev, "failed to request pcie-legacy irq\n"); > + return err; > + } > + > + irq = platform_get_irq_byname(pdev, "pcie-client"); > + if (irq < 0) { > + dev_err(dev, "missing pcie-client IRQ resource\n"); > + return -EINVAL; > + } > + err = devm_request_irq(dev, irq, rockchip_pcie_client_irq_handler, > + IRQF_SHARED, "pcie-client", port); > + if (err) { > + dev_err(dev, "failed to request pcie client irq\n"); > + return err; > + } > + > + port->dev = dev; > + err = rockchip_pcie_parse_dt(port); > + if (err) { > + dev_err(dev, "Parsing DT failed\n"); > + return err; > + } > + > + err = rockchip_pcie_init_port(port); > + if (err) > + return err; > + > + platform_set_drvdata(pdev, port); > + > + rockchip_pcie_enable_interrupts(port); > + if (!IS_ENABLED(CONFIG_PCI_MSI)) { > + err = rockchip_pcie_init_irq_domain(port); > + if (err < 0) > + return err; > + } > + > + err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff, > + &res, &port->io_base); > + if (err) > + return err; > + /* Get the I/O and memory ranges from DT */ > + resource_list_for_each_entry(win, &res) { > + switch (resource_type(win->res)) { > + case IORESOURCE_IO: > + port->io = win->res; > + port->io->name = "I/O"; > + port->io_size = resource_size(port->io); > + port->io_bus_addr = port->io->start - win->offset; > + err = pci_remap_iospace(port->io, port->io_base); > + if (err) { > + dev_warn(port->dev, "error %d: failed to map resource %pR\n", > + err, port->io); > + continue; > + } > + break; > + case IORESOURCE_MEM: > + port->mem = win->res; > + port->mem->name = "MEM"; > + port->mem_size = resource_size(port->mem); > + port->mem_bus_addr = port->mem->start - win->offset; > + break; > + case 0: > + port->cfg = win->res; > + break; > + case IORESOURCE_BUS: > + port->busn = win->res; > + break; > + default: > + continue; > + } > + } > + > + pcie_write(port, 0x6040000, PCIE_RC_CONFIG_BASE + 0x8); > + pcie_write(port, 0x0, PCIE_CORE_CTRL_MGMT_BASE + 0x300); > + > + pcie_write(port, (RC_REGION_0_ADDR_TRANS_L + RC_REGION_0_PASS_BITS), > + PCIE_CORE_AXI_CONF_BASE); > + pcie_write(port, RC_REGION_0_ADDR_TRANS_H, > + PCIE_CORE_AXI_CONF_BASE + 0x4); > + pcie_write(port, 0x0080000a, PCIE_CORE_AXI_CONF_BASE + 0x8); > + pcie_write(port, 0x00000000, PCIE_CORE_AXI_CONF_BASE + 0xc); > + > + for (reg_no = 0; reg_no < (port->mem_size >> 20); reg_no++) { > + err = rockchip_pcie_prog_ob_atu(port, reg_no + 1, > + AXI_WRAPPER_MEM_WRITE, > + 20 - 1, > + port->mem_bus_addr + > + (reg_no << 20), > + 0); > + if (err) { > + dev_err(dev, "Program RC outbound atu failed\n"); > + return err; > + } > + } > + > + err = rockchip_pcie_prog_ib_atu(port, 2, 32 - 1, 0x0, 0); > + if (err) { > + dev_err(dev, "Program RC inbound atu failed\n"); > + return err; > + } > + > + rockchip_pcie_msi_enable(port); > + > + port->root_bus_nr = port->busn->start; > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > + bus = pci_scan_root_bus_msi(port->dev, port->root_bus_nr, > + &rockchip_pcie_ops, port, &res, > + port->msi); You don't need this. The infrastructure will do the right thing for you. > + } else { > + bus = pci_scan_root_bus(&pdev->dev, 0, > + &rockchip_pcie_ops, port, &res); > + } > + if (!bus) > + return -ENOMEM; > + > + if (!pci_has_flag(PCI_PROBE_ONLY)) { Why do you have catter for the PCI_PROBE_ONLY case? Nobody should ever use that for properly implemented HW. > + pci_bus_size_bridges(bus); > + pci_bus_assign_resources(bus); > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + } > + > + pci_bus_add_devices(bus); > + > + return err; > +} > + > +static int rockchip_pcie_remove(struct platform_device *pdev) > +{ > + struct rockchip_pcie_port *port = platform_get_drvdata(pdev); > + > + clk_disable_unprepare(port->hclk_pcie); > + clk_disable_unprepare(port->aclk_perf_pcie); > + clk_disable_unprepare(port->aclk_pcie); > + clk_disable_unprepare(port->clk_pciephy_ref); > + > + return 0; > +} > + > +static const struct of_device_id rockchip_pcie_of_match[] = { > + { .compatible = "rockchip,rk3399-pcie", }, > + {} > +}; > + > +static struct platform_driver rockchip_pcie_driver = { > + .driver = { > + .name = "rockchip-pcie", > + .of_match_table = rockchip_pcie_of_match, > + .suppress_bind_attrs = true, > + }, > + .probe = rockchip_pcie_probe, > + .remove = rockchip_pcie_remove, > +}; > +module_platform_driver(rockchip_pcie_driver); > + > +MODULE_AUTHOR("Rockchip Inc"); > +MODULE_DESCRIPTION("Rockchip AXI PCIe driver"); > +MODULE_LICENSE("GPL v2"); > This definitely requires some rework for both the interrupt and MSI parts. I'll leave Lorenzo to comment on the PCI side of things. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html