[PATCH 1/2] dt-bindings: add MedieTak PCIE binding documentation

[PATCH 1/2] dt-bindings: add MedieTak PCIE binding documentation

[John Crispin]

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 .../devicetree/bindings/pci/mediatek-pcie.txt      |  140 ++++++++++++++++++++
 1 file changed, 140 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
new file mode 100644
index 0000000..1bd20b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
@@ -0,0 +1,140 @@
+Mediatek PCIe controller
+
+Required properties:
+- compatible: Should be one of:
+	- "mediatek,mt2701-pcie"
+	- "mediatek,mt7623-pcie"
+- device_type: Must be "pci"
+- reg: A list of physical base address and length for each set of controller
+  registers. A list of register ranges to use. Must contain an
+    entry for each entry in the reg-names property.
+- reg-names: Must include the following entries:
+  "pcie": PCIE registers
+  "pcie phy0": PCIE PHY0 registers
+  "pcie phy1": PCIE PHY0 registers
+  "pcie phy2": PCIE PHY0 registers
+- interrupts: A list of interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following entries:
+  "pcie0": The interrupt that is asserted for port0
+  "pcie1": The interrupt that is asserted for port1
+  "pcie2": The interrupt that is asserted for port2
+- bus-range: Range of bus numbers associated with this controller
+- #address-cells: Address representation for root ports (must be 3)
+- #size-cells: Size representation for root ports (must be 2)
+- ranges: Describes the translation of addresses for root ports and standard
+  PCI regions. The entries must be 6 cells each.
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- #interrupt-cells: Size representation for interrupts (must be 1)
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - pcie0
+  - pcie1
+  - pcie2
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - pcie0
+  - pcie1
+  - pcie2
+- mediatek,hifsys: Must contain a phandle to the HIFSYS syscon range.
+Root ports are defined as subnodes of the PCIe controller node.
+
+Required properties:
+- device_type: Must be "pci"
+- assigned-addresses: Address and size of the port configuration registers
+- reg: PCI bus address of the root port
+- #address-cells: Must be 3
+- #size-cells: Must be 2
+- ranges: Sub-ranges distributed from the PCIe controller node. An empty
+  property is sufficient.
+
+Example:
+
+SoC DTSI:
+
+	hifsys: clock-controller@1a000000 {
+		compatible = "mediatek,mt7623-hifsys",
+			     "mediatek,mt2701-hifsys",
+			     "syscon";
+		reg = <0 0x1a000000 0 0x1000>;
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+
+	pcie-controller@1a140000 {
+		compatible = "mediatek,mt7623-pcie";
+		device_type = "pci";
+		reg = <0 0x1a140000 0 0x8000>, /* PCI-Express registers */
+		      <0 0x1a149000 0 0x1000>, /* PCI-Express PHY0 */
+		      <0 0x1a14a000 0 0x1000>, /* PCI-Express PHY1 */
+		      <0 0x1a244000 0 0x1000>; /* PCI-Express PHY2 */
+		reg-names = "pcie", "pcie phy0", "pcie phy1", "pcie phy2";
+		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>,
+			     <GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "pcie0", "pcie1", "pcie2";
+		clocks = <&topckgen CLK_TOP_ETHIF_SEL>;
+		clock-names = "pcie";
+		power-domains = <&scpsys MT2701_POWER_DOMAIN_HIF>;
+		resets = <&hifsys MT2701_HIFSYS_PCIE0_RST>,
+			 <&hifsys MT2701_HIFSYS_PCIE1_RST>,
+			 <&hifsys MT2701_HIFSYS_PCIE2_RST>;
+		reset-names = "pcie0", "pice1", "pcie2";
+
+		bus-range = <0x00 0xff>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+                mediatek,hifsys = <&hifsys>;
+
+		ranges = <0x81000000 0 0x1a160000 0 0x1a160000 0 0x00010000 /* io space */
+			  0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>; /* pci memory */
+
+		status = "disabled";
+
+		pcie@1,0 {
+			device_type = "pci";
+			reg = <0x0800 0 0 0 0>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			status = "disabled";
+		};
+
+		pcie@2,0{
+			device_type = "pci";
+			reg = <0x1000 0 0 0 0>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			status = "disabled";
+		};
+
+		pcie@3,0{
+			device_type = "pci";
+			reg = <0x1800 0 0 0 0>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges;
+
+			status = "disabled";
+		};
+	};
+
+Board DTS:
+
+	pcie-controller {
+		status = "okay";
+
+		pci@1,0 {
+			status = "okay";
+		};
+	};
-- 
1.7.10.4

[PATCH 2/2] PCI: mediatek: add support for PCIE found on MT7623/MT2701

[John Crispin]

Add PCIE controller support on MediaTek MT2701/MT7623. The driver supports
a single Root complex (RC) with 3 Root Ports. The SoCs supports a Gen2
1-lan Link on each port.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 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.
+
 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@mediatek.com>
+ *  Copyright (C) 2015 John Crispin <blogic@openwrt.org>
+ *
+ *  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
+
+/* 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)++)
+
+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 },
+};
+
+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);
+	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);
+
+	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;
+
+	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;
+
+	/* 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);
+	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

Re: [PATCH 2/2] PCI: mediatek: add support for PCIE found on MT7623/MT2701

[Bjorn Helgaas]

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.

> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  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@mediatek.com>
> + *  Copyright (C) 2015 John Crispin <blogic@openwrt.org>
> + *
> + *  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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] dt-bindings: add MedieTak PCIE binding documentation

[Bjorn Helgaas]

In subject:

  add MedieTak PCIE binding documentation

has a couple misspellings ("MedieTak" and "PCIE").

Use "PCIe" consistently below.

On Thu, Jan 07, 2016 at 10:06:12PM +0100, John Crispin wrote:
> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  .../devicetree/bindings/pci/mediatek-pcie.txt      |  140 ++++++++++++++++++++
>  1 file changed, 140 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> new file mode 100644
> index 0000000..1bd20b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
> @@ -0,0 +1,140 @@
> +Mediatek PCIe controller
> +
> +Required properties:
> +- compatible: Should be one of:
> +	- "mediatek,mt2701-pcie"
> +	- "mediatek,mt7623-pcie"
> +- device_type: Must be "pci"
> +- reg: A list of physical base address and length for each set of controller
> +  registers. A list of register ranges to use. Must contain an
> +    entry for each entry in the reg-names property.
> +- reg-names: Must include the following entries:
> +  "pcie": PCIE registers
> +  "pcie phy0": PCIE PHY0 registers
> +  "pcie phy1": PCIE PHY0 registers
> +  "pcie phy2": PCIE PHY0 registers
> +- interrupts: A list of interrupt outputs of the controller. Must contain an
> +  entry for each entry in the interrupt-names property.
> +- interrupt-names: Must include the following entries:
> +  "pcie0": The interrupt that is asserted for port0
> +  "pcie1": The interrupt that is asserted for port1
> +  "pcie2": The interrupt that is asserted for port2
> +- bus-range: Range of bus numbers associated with this controller
> +- #address-cells: Address representation for root ports (must be 3)
> +- #size-cells: Size representation for root ports (must be 2)
> +- ranges: Describes the translation of addresses for root ports and standard
> +  PCI regions. The entries must be 6 cells each.
> +  Please refer to the standard PCI bus binding document for a more detailed
> +  explanation.
> +- #interrupt-cells: Size representation for interrupts (must be 1)
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - pcie0
> +  - pcie1
> +  - pcie2
> +- resets: Must contain an entry for each entry in reset-names.
> +  See ../reset/reset.txt for details.
> +- reset-names: Must include the following entries:
> +  - pcie0
> +  - pcie1
> +  - pcie2
> +- mediatek,hifsys: Must contain a phandle to the HIFSYS syscon range.
> +Root ports are defined as subnodes of the PCIe controller node.
> +
> +Required properties:
> +- device_type: Must be "pci"
> +- assigned-addresses: Address and size of the port configuration registers
> +- reg: PCI bus address of the root port
> +- #address-cells: Must be 3
> +- #size-cells: Must be 2
> +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> +  property is sufficient.
> +
> +Example:
> +
> +SoC DTSI:
> +
> +	hifsys: clock-controller@1a000000 {
> +		compatible = "mediatek,mt7623-hifsys",
> +			     "mediatek,mt2701-hifsys",
> +			     "syscon";
> +		reg = <0 0x1a000000 0 0x1000>;
> +		#clock-cells = <1>;
> +		#reset-cells = <1>;
> +	};
> +
> +	pcie-controller@1a140000 {
> +		compatible = "mediatek,mt7623-pcie";
> +		device_type = "pci";
> +		reg = <0 0x1a140000 0 0x8000>, /* PCI-Express registers */
> +		      <0 0x1a149000 0 0x1000>, /* PCI-Express PHY0 */
> +		      <0 0x1a14a000 0 0x1000>, /* PCI-Express PHY1 */
> +		      <0 0x1a244000 0 0x1000>; /* PCI-Express PHY2 */
> +		reg-names = "pcie", "pcie phy0", "pcie phy1", "pcie phy2";
> +		interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>,
> +			     <GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-names = "pcie0", "pcie1", "pcie2";
> +		clocks = <&topckgen CLK_TOP_ETHIF_SEL>;
> +		clock-names = "pcie";
> +		power-domains = <&scpsys MT2701_POWER_DOMAIN_HIF>;
> +		resets = <&hifsys MT2701_HIFSYS_PCIE0_RST>,
> +			 <&hifsys MT2701_HIFSYS_PCIE1_RST>,
> +			 <&hifsys MT2701_HIFSYS_PCIE2_RST>;
> +		reset-names = "pcie0", "pice1", "pcie2";
> +
> +		bus-range = <0x00 0xff>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +
> +                mediatek,hifsys = <&hifsys>;
> +
> +		ranges = <0x81000000 0 0x1a160000 0 0x1a160000 0 0x00010000 /* io space */
> +			  0x83000000 0 0x60000000 0 0x60000000 0 0x10000000>; /* pci memory */
> +
> +		status = "disabled";
> +
> +		pcie@1,0 {
> +			device_type = "pci";
> +			reg = <0x0800 0 0 0 0>;
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			status = "disabled";
> +		};
> +
> +		pcie@2,0{
> +			device_type = "pci";
> +			reg = <0x1000 0 0 0 0>;
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			status = "disabled";
> +		};
> +
> +		pcie@3,0{
> +			device_type = "pci";
> +			reg = <0x1800 0 0 0 0>;
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			status = "disabled";
> +		};
> +	};
> +
> +Board DTS:
> +
> +	pcie-controller {
> +		status = "okay";
> +
> +		pci@1,0 {
> +			status = "okay";
> +		};
> +	};
> -- 
> 1.7.10.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] PCI: mediatek: add support for PCIE found on MT7623/MT2701

[John Crispin]

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@openwrt.org>
>> ---
>>  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@mediatek.com>
>> + *  Copyright (C) 2015 John Crispin <blogic@openwrt.org>
>> + *
>> + *  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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>