On 09/01/2016 01:18, Bjorn Helgaas wrote:
> Hi John,
>
> On Thu, Jan 07, 2016 at 10:06:13PM +0100, John Crispin wrote:
>> Add PCIE controller support on MediaTek MT2701/MT7623. The driver supports
>
> Please capitalize as "PCIe" consistently. You have some of each
> ("PCIE" and "PCIe").
>
>> a single Root complex (RC) with 3 Root Ports. The SoCs supports a Gen2
>> 1-lan Link on each port.
>
> What does "1-lan" mean? I expected "x1" or something similar.
>
Hi Bjorn,
thanks for the quick review, I'll look into those things over the next
couple of days. need to find out about the 8/16 bit i/o. the code is
based on the SDK driver which never had those functions.
the driver works and has been properly tested however when it loads we
see a few weird error messages.
mediatek-pcie 1a140000.pcie: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
pci_bus 0000:00: root bus resource [io 0x1a160000-0x1a16ffff]
pci_bus 0000:00: root bus resource [bus 00-ff]
PCI: bus0: Fast back to back transfers disabled
pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
PCI: bus1: Fast back to back transfers disabled
pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
pci 0000:00:00.0: BAR 1: assigned [mem 0x60200000-0x6020ffff]
pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x600fffff 64bit]
pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
pci 0000:00:00.0: PCI bridge to [bus 01]
pci 0000:00:00.0: bridge window [mem 0x60000000-0x600fffff]
pci 0000:00:00.0: bridge window [mem 0x60100000-0x601fffff pref]
i guess the "no space" boils down to "#define MEMORY_BASE 0x80000000"
not sure about the others, ideas are welcome.
John
>> Signed-off-by: John Crispin <blogic@xxxxxxxxxxx>
>> ---
>> arch/arm/mach-mediatek/Kconfig | 1 +
>> drivers/pci/host/Kconfig | 11 +
>> drivers/pci/host/Makefile | 1 +
>> drivers/pci/host/pcie-mediatek.c | 604 ++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 617 insertions(+)
>> create mode 100644 drivers/pci/host/pcie-mediatek.c
>>
>> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
>> index 7fb605e..a7fef77 100644
>> --- a/arch/arm/mach-mediatek/Kconfig
>> +++ b/arch/arm/mach-mediatek/Kconfig
>> @@ -24,6 +24,7 @@ config MACH_MT6592
>> config MACH_MT7623
>> bool "MediaTek MT7623 SoCs support"
>> default ARCH_MEDIATEK
>> + select MIGHT_HAVE_PCI
>>
>> config MACH_MT8127
>> bool "MediaTek MT8127 SoCs support"
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index f131ba9..912f0e1 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -172,4 +172,15 @@ config PCI_HISI
>> help
>> Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>>
>> +config PCIE_MTK
>> + bool "Mediatek PCIe Controller"
>> + depends on MACH_MT2701 || MACH_MT7623
>> + depends on OF
>> + depends on PCI
>> + help
>> + Say Y here if you want to enable PCI controller support on Mediatek MT7623.
>> + MT7623 PCIe supports single Root complex (RC) with 3 Root Ports.
>> + Each port supports a Gen2 1-lan Link.
>> + PCIe include one Host/PCI bridge and 3 PCIe MAC.
>
> What does "3 PCIe MAC" mean?
>
>> +
>> endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 9d4d3c6..3b53374 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>> obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
>> obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
>> obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
>> +obj-$(CONFIG_PCIE_MTK) += pcie-mediatek.o
>> diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
>> new file mode 100644
>> index 0000000..6bd283a
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-mediatek.c
>> @@ -0,0 +1,604 @@
>> +/*
>> + * Mediatek MT2701/MT7623 SoC PCIE support
>> + *
>> + * Copyright (C) 2015 Mediatek
>> + * Copyright (C) 2015 Ziv Huang <ziv.huang@xxxxxxxxxxxx>
>> + * Copyright (C) 2015 John Crispin <blogic@xxxxxxxxxxx>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> + * by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/pci.h>
>> +#include <linux/ioport.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/init.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/delay.h>
>> +#include <asm/irq.h>
>> +#include <asm/mach/pci.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/reset.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/clk.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#define MEMORY_BASE 0x80000000
>
> What's this? It looks like it might be a hard-coded memory address,
> which should be avoided or maybe learned via DT.
>
>> +/* PCIE Registers */
>> +#define PCICFG 0x00
>> +#define PCIINT 0x08
>> +#define PCIENA 0x0c
>> +#define CFGADDR 0x20
>> +#define CFGDATA 0x24
>> +#define MEMBASE 0x28
>> +#define IOBASE 0x2c
>> +
>> +/* per Port Registers */
>> +#define BAR0SETUP 0x10
>> +#define IMBASEBAR0 0x18
>> +#define PCIE_CLASS 0x34
>> +#define PCIE_SISTAT 0x50
>> +
>> +#define MTK_PCIE_HIGH_PERF BIT(14)
>> +#define PCIEP0_BASE 0x2000
>> +#define PCIEP1_BASE 0x3000
>> +#define PCIEP2_BASE 0x4000
>> +
>> +#define PHY_P0_CTL 0x9000
>> +#define PHY_P1_CTL 0xa000
>> +#define PHY_P2_CTL 0x4000
>> +
>> +#define RSTCTL_PCIE0_RST BIT(24)
>> +#define RSTCTL_PCIE1_RST BIT(25)
>> +#define RSTCTL_PCIE2_RST BIT(26)
>> +
>> +#define HIFSYS_SYSCFG1 0x14
>> +#define HIFSYS_SYSCFG1_PHY2_MASK (0x3 << 20)
>> +
>> +static struct mtk_pcie_port {
>> + int id;
>> + int enable;
>> + u32 base;
>> + u32 perst_n;
>> + u32 interrupt_en;
>> + int irq;
>> + u32 link;
>> + void __iomem *phy_base;
>> + struct reset_control *rstc;
>> +} mtk_pcie_port[] = {
>> + { 0, 0, PCIEP0_BASE, BIT(1), BIT(20) },
>> + { 1, 0, PCIEP1_BASE, BIT(2), BIT(21) },
>> + { 2, 0, PCIEP2_BASE, BIT(3), BIT(22) },
>> +};
>> +#define MAX_PORT_NUM ARRAY_SIZE(mtk_pcie_port)
>> +#define mtk_foreach_port(p) \
>> + for ((p) = mtk_pcie_port; \
>> + (p) != &mtk_pcie_port[MAX_PORT_NUM]; (p)++)
>
> If I understand this correctly, you dynamically allocate a struct
> mtk_pcie for each Root Complex, which is as it should be, but you
> statically allocate mtk_pcie_ports for three Root Ports. That's wrong
> because if you had two Root Complexes, you would have *six* Root
> Ports.
>
> Maybe you want an array of three mtk_pcie_ports inside each mtk_pcie?
>
>> +
>> +struct mtk_pcie {
>> + struct device *dev;
>> + void __iomem *pcie_base;
>> + struct regmap *hifsys;
>> +
>> + struct resource io;
>> + struct resource pio;
>> + struct resource mem;
>> + struct resource prefetch;
>> + struct resource busn;
>> +
>> + u32 io_bus_addr;
>> + u32 mem_bus_addr;
>> +
>> + struct clk *clk;
>> + int pcie_card_link;
>> +};
>> +
>> +static const struct mtk_phy_init {
>> + uint32_t reg;
>> + uint32_t mask;
>> + uint32_t val;
>> +} mtk_phy_init[] = {
>> + { 0xc00, 0x33000, 0x22000 },
>> + { 0xb04, 0xe0000000, 0x40000000 },
>> + { 0xb00, 0xe, 0x4 },
>> + { 0xc3C, 0xffff0000, 0x3c0000 },
>> + { 0xc48, 0xffff, 0x36 },
>> + { 0xc0c, 0x30000000, 0x10000000 },
>> + { 0xc08, 0x3800c0, 0xc0 },
>> + { 0xc10, 0xf0000, 0x20000 },
>> + { 0xc0c, 0xf000, 0x1000 },
>> + { 0xc14, 0xf0000, 0xa0000 },
>> +};
>
> These magic tables of initialization values are sort of frowned upon.
> It's basically a tiny embedded binary blob. Can you replace this with
> descriptive #defines for the registers so we have some clue about
> what's going on?
>
>> +
>> +static struct mtk_pcie *sys_to_pcie(struct pci_sys_data *sys)
>> +{
>> + return sys->private_data;
>> +}
>> +
>> +static void pcie_w32(struct mtk_pcie *pcie, u32 val, unsigned reg)
>> +{
>> + iowrite32(val, pcie->pcie_base + reg);
>> +}
>> +
>> +static u32 pcie_r32(struct mtk_pcie *pcie, unsigned reg)
>> +{
>> + return ioread32(pcie->pcie_base + reg);
>> +}
>> +
>> +static void pcie_m32(struct mtk_pcie *pcie, u32 mask, u32 val, unsigned reg)
>> +{
>> + u32 v = pcie_r32(pcie, reg);
>> +
>> + v &= mask;
>> + v |= val;
>> + pcie_w32(pcie, v, reg);
>> +}
>> +
>> +static int pcie_config_read(struct pci_bus *bus, unsigned int devfn, int where,
>> + int size, u32 *val)
>> +{
>> + struct mtk_pcie *pcie = sys_to_pcie(bus->sysdata);
>> + unsigned int slot = PCI_SLOT(devfn);
>> + u8 func = PCI_FUNC(devfn);
>> + u32 address;
>> + u32 data;
>> + u32 num = 0;
>> +
>> + if (bus)
>> + num = bus->number;
>> +
>> + address = (((where & 0xf00) >> 8) << 24) |
>> + (num << 16) |
>> + (slot << 11) |
>> + (func << 8) |
>> + (where & 0xfc);
>> +
>> + pcie_m32(pcie, 0xf0000000, address, CFGADDR);
>
> 0xf0000000 should be a #define. Apparently it defines a field in the
> CFGADDR register.
>
>> + data = pcie_r32(pcie, CFGDATA);
>> +
>> + switch (size) {
>> + case 1:
>> + *val = (data >> ((where & 3) << 3)) & 0xff;
>> + break;
>> + case 2:
>> + *val = (data >> ((where & 3) << 3)) & 0xffff;
>> + break;
>> + case 4:
>> + *val = data;
>> + break;
>> + }
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int pcie_config_write(struct pci_bus *bus, unsigned int devfn, int where,
>> + int size, u32 val)
>> +{
>> + struct mtk_pcie *pcie = sys_to_pcie(bus->sysdata);
>> + unsigned int slot = PCI_SLOT(devfn);
>> + u8 func = PCI_FUNC(devfn);
>> + u32 address;
>> + u32 data;
>> + u32 num = 0;
>> +
>> + if (bus)
>> + num = bus->number;
>> +
>> + address = (((where & 0xf00) >> 8) << 24) |
>> + (num << 16) | (slot << 11) | (func << 8) | (where & 0xfc);
>> + pcie_m32(pcie, 0xf0000000, address, CFGADDR);
>> + data = pcie_r32(pcie, CFGDATA);
>> +
>> + switch (size) {
>> + case 1:
>> + data = (data & ~(0xff << ((where & 3) << 3))) |
>> + (val << ((where & 3) << 3));
>> + break;
>> + case 2:
>> + data = (data & ~(0xffff << ((where & 3) << 3))) |
>> + (val << ((where & 3) << 3));
>> + break;
>> + case 4:
>> + data = val;
>> + break;
>> + }
>> + pcie_w32(pcie, data, CFGDATA);
>
> Oops, is this another device that can't do sub-32 bit config writes?
> If so, that's a hardware defect because the read/modify/write will
> corrupt RW1C bits in registers.
>
> If the hardware is capable of 8- and 16-bit writes, please change the
> code to use them. If not, please print a warning at probe-time so we
> have a debugging clue for when the corruption bites us. There other
> drivers with the same problem, so please follow their format.
>
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static struct pci_ops mtk_pcie_ops = {
>> + .read = pcie_config_read,
>> + .write = pcie_config_write,
>> +};
>> +
>> +static int __init mtk_pcie_setup(int nr, struct pci_sys_data *sys)
>> +{
>> + struct mtk_pcie *pcie = sys_to_pcie(sys);
>> +
>> + if (!pcie->pcie_card_link)
>> + return -1;
>
> Please restructure the code so these guards aren't necessary.
>
>> +
>> + request_resource(&ioport_resource, &pcie->pio);
>> + request_resource(&iomem_resource, &pcie->mem);
>> +
>> + pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
>> + pci_add_resource_offset(&sys->resources, &pcie->pio, sys->io_offset);
>> + pci_add_resource(&sys->resources, &pcie->busn);
>> +
>> + return 1;
>> +}
>> +
>> +static struct pci_bus * __init mtk_pcie_scan_bus(int nr,
>> + struct pci_sys_data *sys)
>> +{
>> + struct mtk_pcie *pcie = sys_to_pcie(sys);
>> + struct pci_bus *bus;
>> +
>> + if (!pcie->pcie_card_link)
>> + return NULL;
>> +
>> + bus = pci_create_root_bus(pcie->dev, sys->busnr, &mtk_pcie_ops, sys,
>> + &sys->resources);
>> + if (!bus)
>> + return NULL;
>> +
>> + pci_scan_child_bus(bus);
>> +
>> + return bus;
>> +}
>> +
>> +static int __init mtk_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
>> +{
>> + struct mtk_pcie *pcie = sys_to_pcie(dev->bus->sysdata);
>> + struct mtk_pcie_port *port;
>> + int irq = -1;
>> +
>> + if (!pcie->pcie_card_link)
>> + return irq;
>> +
>> + mtk_foreach_port(port)
>> + if (port->id == slot)
>> + irq = port->irq;
>> +
>> + return irq;
>> +}
>> +
>> +static void mtk_pcie_configure_phy(struct mtk_pcie *pcie,
>> + struct mtk_pcie_port *port)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(mtk_phy_init); i++) {
>> + void __iomem *phy_addr = port->phy_base + mtk_phy_init[i].reg;
>> + u32 val = ioread32(phy_addr);
>> +
>> + val &= ~mtk_phy_init[i].mask;
>> + val |= mtk_phy_init[i].val;
>> + iowrite32(val, phy_addr);
>> + }
>> + usleep_range(5000, 6000);
>> +}
>> +
>> +static void mtk_pcie_configure_rc(struct mtk_pcie *pcie,
>> + struct mtk_pcie_port *port,
>> + struct pci_bus *bus)
>> +{
>> + u32 val = 0;
>> +
>> + pcie_config_write(bus,
>> + port->id << 3,
>> + PCI_BASE_ADDRESS_0, 4, MEMORY_BASE);
>> +
>> + pcie_config_read(bus,
>> + port->id << 3, PCI_BASE_ADDRESS_0, 4, &val);
>> +
>> + /* Configure RC Credit */
>> + pcie_config_read(bus, port->id << 3, 0x73c, 4, &val);
>> + val &= ~(0x9fff) << 16;
>> + val |= 0x806c << 16;
>> + pcie_config_write(bus, port->id << 3, 0x73c, 4, val);
>> +
>> + /* Configure RC FTS number */
>> + pcie_config_read(bus, port->id << 3, 0x70c, 4, &val);
>> + val &= ~(0xff3) << 8;
>> + val |= 0x50 << 8;
>> + pcie_config_write(bus, port->id << 3, 0x70c, 4, val);
>> +}
>> +
>> +static void mtk_pcie_preinit(struct mtk_pcie *pcie)
>> +{
>> + struct mtk_pcie_port *port;
>> + u32 val = 0;
>> + struct pci_bus bus;
>> + struct pci_sys_data sys;
>> +
>> + memset(&bus, 0, sizeof(bus));
>> + memset(&sys, 0, sizeof(sys));
>> + bus.sysdata = (void *)&sys;
>> + sys.private_data = (void *)pcie;
>> +
>> + pcibios_min_io = 0;
>> + pcibios_min_mem = 0;
>
> These are ARM-specific variables that shouldn't be used by a host
> bridge driver.
>
>> +
>> + /* The PHY on Port 2 is shared with USB */
>> + if (mtk_pcie_port[2].enable)
>> + regmap_update_bits(pcie->hifsys, HIFSYS_SYSCFG1,
>> + HIFSYS_SYSCFG1_PHY2_MASK, 0x0);
>> +
>> + /* PCIe RC Reset */
>> + mtk_foreach_port(port)
>> + if (port->enable)
>> + reset_control_assert(port->rstc);
>> + usleep_range(1000, 2000);
>> + mtk_foreach_port(port)
>> + if (port->enable)
>> + reset_control_deassert(port->rstc);
>> + usleep_range(1000, 2000);
>> +
>> + /* Configure PCIe PHY */
>> + mtk_foreach_port(port)
>> + if (port->enable)
>> + mtk_pcie_configure_phy(pcie, port);
>> +
>> + /* PCIe EP reset */
>> + val = 0;
>> + mtk_foreach_port(port)
>> + if (port->enable)
>> + val |= port->perst_n;
>> + pcie_w32(pcie, pcie_r32(pcie, PCICFG) | val, PCICFG);
>> + usleep_range(1000, 2000);
>> + pcie_w32(pcie, pcie_r32(pcie, PCICFG) & ~val, PCICFG);
>> + usleep_range(1000, 2000);
>> + msleep(100);
>> +
>> + /* check the link status */
>> + val = 0;
>> + mtk_foreach_port(port) {
>> + if (port->enable) {
>> + if ((pcie_r32(pcie, port->base + PCIE_SISTAT) & 0x1))
>> + port->link = 1;
>> + else
>> + reset_control_assert(port->rstc);
>> + }
>> + }
>> +
>> + mtk_foreach_port(port)
>> + if (port->link)
>> + pcie->pcie_card_link++;
>> +
>> + if (!pcie->pcie_card_link)
>> + return;
>> +
>> + pcie_w32(pcie, pcie->mem_bus_addr, MEMBASE);
>> + pcie_w32(pcie, pcie->io_bus_addr, IOBASE);
>> +
>> + mtk_foreach_port(port) {
>> + if (port->link) {
>> + pcie_m32(pcie, 0, port->interrupt_en, PCIENA);
>> + pcie_w32(pcie, 0x7fff0001, port->base + BAR0SETUP);
>> + pcie_w32(pcie, MEMORY_BASE, port->base + IMBASEBAR0);
>> + pcie_w32(pcie, 0x06040001, port->base + PCIE_CLASS);
>> + }
>> + }
>> +
>> + mtk_foreach_port(port)
>> + if (port->link)
>> + mtk_pcie_configure_rc(pcie, port, &bus);
>> +}
>> +
>> +static int mtk_pcie_parse_dt(struct mtk_pcie *pcie)
>> +{
>> + struct device_node *np = pcie->dev->of_node, *port;
>> + struct of_pci_range_parser parser;
>> + struct of_pci_range range;
>> + struct resource res;
>> + int err;
>> +
>> + pcie->hifsys = syscon_regmap_lookup_by_phandle(np, "mediatek,hifsys");
>> + if (IS_ERR(pcie->hifsys)) {
>> + dev_err(pcie->dev, "missing \"mediatek,hifsys\" phandle\n");
>> + return PTR_ERR(pcie->hifsys);
>> + }
>> +
>> + if (of_pci_range_parser_init(&parser, np)) {
>> + dev_err(pcie->dev, "missing \"ranges\" property\n");
>> + return -EINVAL;
>> + }
>> +
>> + for_each_of_pci_range(&parser, &range) {
>> + err = of_pci_range_to_resource(&range, np, &res);
>> + if (err < 0) {
>> + dev_err(pcie->dev, "failed to read resource range\n");
>> + return err;
>> + }
>> +
>> + switch (res.flags & IORESOURCE_TYPE_BITS) {
>> + case IORESOURCE_IO:
>> + memcpy(&pcie->pio, &res, sizeof(res));
>> + pcie->pio.start = (resource_size_t)range.pci_addr;
>> + pcie->pio.end = (resource_size_t)
>> + (range.pci_addr + range.size - 1);
>> + pcie->io_bus_addr = (resource_size_t)range.cpu_addr;
>> + break;
>> +
>> + case IORESOURCE_MEM:
>> + if (res.flags & IORESOURCE_PREFETCH) {
>> + memcpy(&pcie->prefetch, &res, sizeof(res));
>> + pcie->prefetch.name = "prefetchable";
>> + pcie->prefetch.start =
>> + (resource_size_t)range.pci_addr;
>> + pcie->prefetch.end = (resource_size_t)
>> + (range.pci_addr + range.size - 1);
>> + } else {
>> + memcpy(&pcie->mem, &res, sizeof(res));
>> + pcie->mem.name = "non-prefetchable";
>> + pcie->mem.start = (resource_size_t)
>> + range.pci_addr;
>> + pcie->prefetch.end = (resource_size_t)
>> + (range.pci_addr + range.size - 1);
>> + pcie->mem_bus_addr = (resource_size_t)
>> + range.cpu_addr;
>> + }
>> + break;
>> + }
>> + }
>> +
>> + err = of_pci_parse_bus_range(np, &pcie->busn);
>> + if (err < 0) {
>> + dev_err(pcie->dev, "failed to parse ranges property: %d\n",
>> + err);
>> + pcie->busn.name = np->name;
>> + pcie->busn.start = 0;
>> + pcie->busn.end = 0xff;
>> + pcie->busn.flags = IORESOURCE_BUS;
>> + }
>> +
>> + /* parse root ports */
>> + for_each_child_of_node(np, port) {
>> + unsigned int index;
>> + char rst[] = "pcie0";
>> +
>> + err = of_pci_get_devfn(port);
>> + if (err < 0) {
>> + dev_err(pcie->dev, "failed to parse address: %d\n",
>> + err);
>> + return err;
>> + }
>> +
>> + index = PCI_SLOT(err);
>> +
>> + if (index > MAX_PORT_NUM) {
>> + dev_err(pcie->dev, "invalid port number: %d\n", index);
>> + continue;
>> + }
>> + index--;
>> +
>> + if (!of_device_is_available(port))
>> + continue;
>> +
>> + rst[4] += index;
>> + mtk_pcie_port[index].rstc = devm_reset_control_get(pcie->dev,
>> + rst);
>> + if (!IS_ERR(mtk_pcie_port[index].rstc))
>> + mtk_pcie_port[index].enable = 1;
>> + }
>> + return 0;
>> +}
>> +
>> +static int mtk_pcie_get_resources(struct mtk_pcie *pcie)
>> +{
>> + struct platform_device *pdev = to_platform_device(pcie->dev);
>> + struct mtk_pcie_port *port;
>> + struct resource *res;
>> +
>> + pcie->clk = devm_clk_get(&pdev->dev, "pcie");
>> + if (IS_ERR(pcie->clk)) {
>> + dev_err(&pdev->dev, "Failed to get pcie clk\n");
>> + return PTR_ERR(pcie->clk);
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + pcie->pcie_base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(pcie->pcie_base)) {
>> + dev_err(&pdev->dev, "Failed to get pcie range\n");
>> + return PTR_ERR(pcie->pcie_base);
>> + }
>> +
>> + mtk_foreach_port(port) {
>> + if (!port->enable)
>> + continue;
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, port->id + 1);
>> + port->phy_base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(port->phy_base)) {
>> + dev_err(&pdev->dev, "Failed to get pcie phy%d range\n",
>> + port->id);
>> + return PTR_ERR(port->phy_base);
>> + }
>> + port->irq = platform_get_irq(pdev, port->id);
>> + }
>> +
>> + return clk_prepare_enable(pcie->clk);
>> +}
>> +
>> +static int mtk_pcie_probe(struct platform_device *pdev)
>> +{
>> + struct mtk_pcie *pcie;
>> + struct hw_pci hw;
>> + int ret;
>> +
>> + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
>> + if (!pcie)
>> + return -ENOMEM;
>> +
>> + pcie->dev = &pdev->dev;
>> + ret = mtk_pcie_parse_dt(pcie);
>> + if (ret < 0)
>> + return ret;
>> +
>> + pm_runtime_enable(&pdev->dev);
>> + pm_runtime_get_sync(&pdev->dev);
>> +
>> + ret = mtk_pcie_get_resources(pcie);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "failed to request resources: %d\n", ret);
>> + goto err_out;
>> + }
>> +
>> + mtk_pcie_preinit(pcie);
>> +
>> + memset(&hw, 0, sizeof(hw));
>> + hw.nr_controllers = 1;
>> + hw.private_data = (void **)&pcie;
>> + hw.setup = mtk_pcie_setup;
>> + hw.map_irq = mtk_pcie_map_irq;
>> + hw.scan = mtk_pcie_scan_bus;
>> +
>> + pci_common_init_dev(pcie->dev, &hw);
>
> Calling pci_common_init_dev() makes this driver unnecessarily
> ARM-specific. Please rework this so it doesn't use these ARM-specific
> entry points.
>
>> + platform_set_drvdata(pdev, pcie);
>> +
>> + return 0;
>> +
>> +err_out:
>> + clk_disable_unprepare(pcie->clk);
>> + pm_runtime_put_sync(&pdev->dev);
>> + pm_runtime_disable(&pdev->dev);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct of_device_id mtk_pcie_ids[] = {
>> + { .compatible = "mediatek,mt2701-pcie" },
>> + { .compatible = "mediatek,mt7623-pcie" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_pcie_ids);
>> +
>> +static struct platform_driver mtk_pcie_driver = {
>> + .probe = mtk_pcie_probe,
>> + .driver = {
>> + .name = "mediatek-pcie",
>> + .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(mtk_pcie_ids),
>> + },
>> +};
>> +
>> +static int __init mtk_pcie_init(void)
>> +{
>> + return platform_driver_register(&mtk_pcie_driver);
>> +}
>> +
>> +module_init(mtk_pcie_init);
>> --
>> 1.7.10.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
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
--
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
