[PATCH v5 0/9] R-Car Gen2 PCIe host driver

[PATCH v5 0/9] R-Car Gen2 PCIe host driver

[Phil Edworthy]

This is version 5 of a PCIe Host driver for the R-Car Gen2 devices,
i.e. R-Car H2 (r8a7790) and R-Car M2 (r8a7791).

v5:
 - Use module_platform_driver instead of subsys_initcall
 - Use the of_device_id data field for HW init function
 - Init hw_pci struct in declaration
 - Renesas SoC compatible string has peripheral before device name
 - Add PCIe bus clock reference
 - Use dma-ranges property to specify inbound memory regions
 - Support multiple IO windows and correct resources
 - Return IRQ_NONE from MSI isr when there is no pending MSI
 - Add additional interrupt bindings

v4:
 - Use runtime PM properly

Phil Edworthy (9):
  PCI: host: rcar: Add Renesas R-Car PCIe driver
  PCI: host: rcar: Add MSI support
  ARM: shmobile: r8a7790: Add PCIe clock device tree nodes
  ARM: shmobile: r8a7791: Add PCIe clock device tree nodes
  dt-bindings: pci: rcar pcie device tree bindings
  ARM: shmobile: Add PCIe device tree nodes for R8A7790
  ARM: shmobile: Add PCIe device tree nodes for R8A7791 Koelsch board
  ARM: koelsch: Add PCIe to defconfig
  ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

 Documentation/devicetree/bindings/pci/rcar-pci.txt |  44 +
 arch/arm/boot/dts/r8a7790-lager.dts                |  10 +
 arch/arm/boot/dts/r8a7790.dtsi                     |  31 +-
 arch/arm/boot/dts/r8a7791-koelsch.dts              |  14 +
 arch/arm/boot/dts/r8a7791.dtsi                     |  31 +-
 arch/arm/configs/koelsch_defconfig                 |   4 +
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-rcar.c                       | 937 +++++++++++++++++++++
 drivers/pci/host/pcie-rcar.h                       |  87 ++
 include/dt-bindings/clock/r8a7790-clock.h          |   1 +
 include/dt-bindings/clock/r8a7791-clock.h          |   1 +
 12 files changed, 1162 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci.txt
 create mode 100644 drivers/pci/host/pcie-rcar.c
 create mode 100644 drivers/pci/host/pcie-rcar.h

-- 
1.9.0

[PATCH v5 1/9] PCI: host: rcar: Add Renesas R-Car PCIe driver

[Phil Edworthy]

This PCIe Host driver currently does not support MSI, so cards
fall back to INTx interrupts.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Use module_platform_driver instead of subsys_initcall
 - Use the of_device_id data field for HW init function
 - Init hw_pci struct in declaration
 - Renesas SoC compatible string has peripheral before device name
 - Add PCIe bus clock reference
 - Use dma-ranges property to specify inbound memory regions
 - Support multiple IO windows and correct resources

v4:
 - Use runtime PM properly

v3:
 - Add DT support
 - Use 'of_irq_parse_and_map_pci' for '.map_irq'
 - Use pm ops to enable clocks
 - Fix checkpatch errors
 - Use subsys_initcall to overcome issues with port bus driver
 - Adjust Kconfig to match other R-Car drivers

v2:
 - Use msleep instead of udelay when waiting for the link
 - Use pm_runtime
 - Removed unused definition
 - Also replaced call to devm_request_and_ioremap with devm_ioremap_resource
   and fixed a bug with this when reporting errors.
---
 drivers/pci/host/Kconfig     |   6 +
 drivers/pci/host/Makefile    |   1 +
 drivers/pci/host/pcie-rcar.c | 693 +++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pcie-rcar.h |  82 +++++
 4 files changed, 782 insertions(+)
 create mode 100644 drivers/pci/host/pcie-rcar.c
 create mode 100644 drivers/pci/host/pcie-rcar.h

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 47d46c6..dc627e5 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -33,4 +33,10 @@ config PCI_RCAR_GEN2
 	  There are 3 internal PCI controllers available with a single
 	  built-in EHCI/OHCI host controller present on each one.
 
+config PCI_RCAR_GEN2_PCIE
+	bool "Renesas R-Car PCIe controller"
+	depends on ARCH_SHMOBILE || (ARM && COMPILE_TEST)
+	help
+	  Say Y here if you want PCIe controller support on R-Car Gen2 SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 13fb333..19946f9 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
+obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
new file mode 100644
index 0000000..c22c896
--- /dev/null
+++ b/drivers/pci/host/pcie-rcar.c
@@ -0,0 +1,693 @@
+/*
+ * PCIe driver for Renesas R-Car SoCs
+ *  Copyright (C) 2014 Renesas Electronics Europe Ltd
+ *
+ * Based on:
+ *  arch/sh/drivers/pci/pcie-sh7786.c
+ *  arch/sh/drivers/pci/ops-sh7786.c
+ *  Copyright (C) 2009 - 2011  Paul Mundt
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include "pcie-rcar.h"
+
+#define DRV_NAME "rcar-pcie"
+
+#define RCONF(x)	(PCICONF(0)+(x))
+#define RPMCAP(x)	(PMCAP(0)+(x))
+#define REXPCAP(x)	(EXPCAP(0)+(x))
+#define RVCCAP(x)	(VCCAP(0)+(x))
+
+#define  PCIE_CONF_BUS(b)	(((b) & 0xff) << 24)
+#define  PCIE_CONF_DEV(d)	(((d) & 0x1f) << 19)
+#define  PCIE_CONF_FUNC(f)	(((f) & 0x7) << 16)
+
+#define PCI_MAX_RESOURCES 4
+#define MAX_NR_INBOUND_MAPS 6
+
+/* Structure representing the PCIe interface */
+struct rcar_pcie {
+	struct device		*dev;
+	void __iomem		*base;
+	struct resource		res[PCI_MAX_RESOURCES];
+	u8			root_bus_nr;
+	struct clk		*clk;
+	struct clk		*bus_clk;
+};
+
+static inline struct rcar_pcie *sys_to_pcie(struct pci_sys_data *sys)
+{
+	return sys->private_data;
+}
+
+static void
+pci_write_reg(struct rcar_pcie *pcie, unsigned long val, unsigned long reg)
+{
+	writel(val, pcie->base + reg);
+}
+
+static unsigned long
+pci_read_reg(struct rcar_pcie *pcie, unsigned long reg)
+{
+	return readl(pcie->base + reg);
+}
+
+enum {
+	PCI_ACCESS_READ,
+	PCI_ACCESS_WRITE,
+};
+
+static void rcar_rmw32(struct rcar_pcie *pcie,
+			    int where, u32 mask, u32 data)
+{
+	int shift = 8 * (where & 3);
+	u32 val = pci_read_reg(pcie, where & ~3);
+	val &= ~(mask << shift);
+	val |= data << shift;
+	pci_write_reg(pcie, val, where & ~3);
+}
+
+static u32 rcar_read_conf(struct rcar_pcie *pcie, int where)
+{
+	int shift = 8 * (where & 3);
+	u32 val = pci_read_reg(pcie, where & ~3);
+	return val >> shift;
+}
+
+static int rcar_pcie_config_access(struct rcar_pcie *pcie,
+		unsigned char access_type, struct pci_bus *bus,
+		unsigned int devfn, int where, u32 *data)
+{
+	int dev, func, reg, index;
+
+	dev = PCI_SLOT(devfn);
+	func = PCI_FUNC(devfn);
+	reg = where & ~3;
+	index = reg / 4;
+
+	if (bus->number > 255 || dev > 31 || func > 7)
+		return PCIBIOS_FUNC_NOT_SUPPORTED;
+
+	/*
+	 * While each channel has its own memory-mapped extended config
+	 * space, it's generally only accessible when in endpoint mode.
+	 * When in root complex mode, the controller is unable to target
+	 * itself with either type 0 or type 1 accesses, and indeed, any
+	 * controller initiated target transfer to its own config space
+	 * result in a completer abort.
+	 *
+	 * Each channel effectively only supports a single device, but as
+	 * the same channel <-> device access works for any PCI_SLOT()
+	 * value, we cheat a bit here and bind the controller's config
+	 * space to devfn 0 in order to enable self-enumeration. In this
+	 * case the regular ECAR/ECDR path is sidelined and the mangled
+	 * config access itself is initiated as an internal bus  transaction.
+	 */
+	if (pci_is_root_bus(bus)) {
+		if (dev != 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+
+		if (access_type == PCI_ACCESS_READ)
+			*data = pci_read_reg(pcie, PCICONF(index));
+		else
+			pci_write_reg(pcie, *data, PCICONF(index));
+
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	/* Clear errors */
+	pci_write_reg(pcie, pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
+
+	/* Set the PIO address */
+	pci_write_reg(pcie, PCIE_CONF_BUS(bus->number) | PCIE_CONF_DEV(dev) |
+				PCIE_CONF_FUNC(func) | reg, PCIECAR);
+
+	/* Enable the configuration access */
+	if (bus->parent->number == pcie->root_bus_nr)
+		pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE0, PCIECCTLR);
+	else
+		pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE1, PCIECCTLR);
+
+	/* Check for errors */
+	if (pci_read_reg(pcie, PCIEERRFR) & UNSUPPORTED_REQUEST)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	/* Check for master and target aborts */
+	if (rcar_read_conf(pcie, RCONF(PCI_STATUS)) &
+		(PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT))
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (access_type == PCI_ACCESS_READ)
+		*data = pci_read_reg(pcie, PCIECDR);
+	else
+		pci_write_reg(pcie, *data, PCIECDR);
+
+	/* Disable the configuration access */
+	pci_write_reg(pcie, 0, PCIECCTLR);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int rcar_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
+				int where, int size, u32 *val)
+{
+	struct rcar_pcie *pcie = sys_to_pcie(bus->sysdata);
+	int ret;
+
+	if ((size == 2) && (where & 1))
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	else if ((size == 4) && (where & 3))
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+
+	ret = rcar_pcie_config_access(pcie, PCI_ACCESS_READ,
+				      bus, devfn, where, val);
+	if (ret != PCIBIOS_SUCCESSFUL) {
+		*val = 0xffffffff;
+		return ret;
+	}
+
+	if (size == 1)
+		*val = (*val >> (8 * (where & 3))) & 0xff;
+	else if (size == 2)
+		*val = (*val >> (8 * (where & 2))) & 0xffff;
+
+	dev_dbg(&bus->dev, "pcie-config-read: bus=%3d devfn=0x%04x "
+		"where=0x%04x size=%d val=0x%08lx\n", bus->number,
+		devfn, where, size, (unsigned long)*val);
+
+	return ret;
+}
+
+static int rcar_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
+				 int where, int size, u32 val)
+{
+	struct rcar_pcie *pcie = sys_to_pcie(bus->sysdata);
+	int shift, ret;
+	u32 data;
+
+	if ((size == 2) && (where & 1))
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	else if ((size == 4) && (where & 3))
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+
+	ret = rcar_pcie_config_access(pcie, PCI_ACCESS_READ,
+				      bus, devfn, where, &data);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		return ret;
+
+	dev_dbg(&bus->dev, "pcie-config-write: bus=%3d devfn=0x%04x "
+		"where=0x%04x size=%d val=0x%08lx\n", bus->number,
+		devfn, where, size, (unsigned long)val);
+
+	if (size == 1) {
+		shift = 8 * (where & 3);
+		data &= ~(0xff << shift);
+		data |= ((val & 0xff) << shift);
+	} else if (size == 2) {
+		shift = 8 * (where & 2);
+		data &= ~(0xffff << shift);
+		data |= ((val & 0xffff) << shift);
+	} else
+		data = val;
+
+	ret = rcar_pcie_config_access(pcie, PCI_ACCESS_WRITE,
+				      bus, devfn, where, &data);
+
+	return ret;
+}
+
+static struct pci_ops rcar_pcie_ops = {
+	.read	= rcar_pcie_read_conf,
+	.write	= rcar_pcie_write_conf,
+};
+
+static int rcar_pcie_setup_window(int win, struct resource *res,
+					struct rcar_pcie *pcie)
+{
+	/* Setup PCIe address space mappings for each resource */
+	resource_size_t size;
+	u32 mask;
+
+	pci_write_reg(pcie, 0x00000000, PCIEPTCTLR(win));
+
+	/*
+	 * The PAMR mask is calculated in units of 128Bytes, which
+	 * keeps things pretty simple.
+	 */
+	size = resource_size(res);
+	mask = (roundup_pow_of_two(size) / SZ_128) - 1;
+	pci_write_reg(pcie, mask << 7, PCIEPAMR(win));
+
+	pci_write_reg(pcie, upper_32_bits(res->start), PCIEPARH(win));
+	pci_write_reg(pcie, lower_32_bits(res->start), PCIEPARL(win));
+
+	/* First resource is for IO */
+	mask = PAR_ENABLE;
+	if (res->flags & IORESOURCE_IO)
+		mask |= IO_SPACE;
+
+	pci_write_reg(pcie, mask, PCIEPTCTLR(win));
+
+	return 0;
+}
+
+static int rcar_pcie_setup(int nr, struct pci_sys_data *sys)
+{
+	struct rcar_pcie *pcie = sys_to_pcie(sys);
+	struct resource *res;
+	int i;
+
+	pcie->root_bus_nr = sys->busnr;
+
+	/* Setup PCI resources */
+	for (i = 0; i < PCI_MAX_RESOURCES; i++) {
+
+		res = &pcie->res[i];
+		if (!res->flags)
+			continue;
+
+		rcar_pcie_setup_window(i, res, pcie);
+
+		if (res->flags & IORESOURCE_IO)
+			pci_ioremap_io(nr * SZ_64K, res->start);
+		else
+			pci_add_resource(&sys->resources, res);
+	}
+
+	return 1;
+}
+
+static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
+{
+	struct platform_device *pdev = to_platform_device(pcie->dev);
+	struct hw_pci hw = {
+		.nr_controllers = 1,
+		.private_data   = (void **)&pcie,
+		.setup          = rcar_pcie_setup,
+		.map_irq        = of_irq_parse_and_map_pci,
+		.ops            = &rcar_pcie_ops,
+	};
+
+	pci_common_init_dev(&pdev->dev, &hw);
+}
+
+static int __init phy_wait_for_ack(struct rcar_pcie *pcie)
+{
+	unsigned int timeout = 100;
+
+	while (timeout--) {
+		if (pci_read_reg(pcie, H1_PCIEPHYADRR) & PHY_ACK)
+			return 0;
+
+		udelay(100);
+	}
+
+	dev_err(pcie->dev, "Access to PCIe phy timed out\n");
+
+	return -ETIMEDOUT;
+}
+
+static void __init phy_write_reg(struct rcar_pcie *pcie,
+				 unsigned int rate, unsigned int addr,
+				 unsigned int lane, unsigned int data)
+{
+	unsigned long phyaddr;
+
+	phyaddr = WRITE_CMD |
+		((rate & 1) << RATE_POS) |
+		((lane & 0xf) << LANE_POS) |
+		((addr & 0xff) << ADR_POS);
+
+	/* Set write data */
+	pci_write_reg(pcie, data, H1_PCIEPHYDOUTR);
+	pci_write_reg(pcie, phyaddr, H1_PCIEPHYADRR);
+
+	/* Ignore errors as they will be dealt with if the data link is down */
+	phy_wait_for_ack(pcie);
+
+	/* Clear command */
+	pci_write_reg(pcie, 0, H1_PCIEPHYDOUTR);
+	pci_write_reg(pcie, 0, H1_PCIEPHYADRR);
+
+	/* Ignore errors as they will be dealt with if the data link is down */
+	phy_wait_for_ack(pcie);
+}
+
+static int __init rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
+{
+	unsigned int timeout = 10;
+
+	while (timeout--) {
+		if ((pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
+			return 0;
+
+		msleep(5);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int __init rcar_pcie_hw_init(struct rcar_pcie *pcie)
+{
+	int err;
+
+	/* Begin initialization */
+	pci_write_reg(pcie, 0, PCIETCTLR);
+
+	/* Set mode */
+	pci_write_reg(pcie, 1, PCIEMSR);
+
+	/*
+	 * Initial header for port config space is type 1, set the device
+	 * class to match. Hardware takes care of propagating the IDSETR
+	 * settings, so there is no need to bother with a quirk.
+	 */
+	pci_write_reg(pcie, PCI_CLASS_BRIDGE_PCI << 16, IDSETR1);
+
+	/*
+	 * Setup Secondary Bus Number & Subordinate Bus Number, even though
+	 * they aren't used, to avoid bridge being detected as broken.
+	 */
+	rcar_rmw32(pcie, RCONF(PCI_SECONDARY_BUS), 0xff, 1);
+	rcar_rmw32(pcie, RCONF(PCI_SUBORDINATE_BUS), 0xff, 1);
+
+	/* Initialize default capabilities. */
+	rcar_rmw32(pcie, REXPCAP(0), 0, PCI_CAP_ID_EXP);
+	rcar_rmw32(pcie, REXPCAP(PCI_EXP_FLAGS),
+		PCI_EXP_FLAGS_TYPE, PCI_EXP_TYPE_ROOT_PORT << 4);
+	rcar_rmw32(pcie, RCONF(PCI_HEADER_TYPE), 0x7f,
+		PCI_HEADER_TYPE_BRIDGE);
+
+	/* Enable data link layer active state reporting */
+	rcar_rmw32(pcie, REXPCAP(PCI_EXP_LNKCAP), 0, PCI_EXP_LNKCAP_DLLLARC);
+
+	/* Write out the physical slot number = 0 */
+	rcar_rmw32(pcie, REXPCAP(PCI_EXP_SLTCAP), PCI_EXP_SLTCAP_PSN, 0);
+
+	/* Set the completion timer timeout to the maximum 50ms. */
+	rcar_rmw32(pcie, TLCTLR+1, 0x3f, 50);
+
+	/* Terminate list of capabilities (Next Capability Offset=0) */
+	rcar_rmw32(pcie, RVCCAP(0), 0xfff0, 0);
+
+	/* Enable MAC data scrambling. */
+	rcar_rmw32(pcie, MACCTLR, SCRAMBLE_DISABLE, 0);
+
+	/* Finish initialization - establish a PCI Express link */
+	pci_write_reg(pcie, CFINIT, PCIETCTLR);
+
+	/* This will timeout if we don't have a link. */
+	err = rcar_pcie_wait_for_dl(pcie);
+	if (err)
+		return err;
+
+	/* Enable INTx interrupts */
+	rcar_rmw32(pcie, PCIEINTXR, 0, 0xF << 8);
+
+	/* Enable slave Bus Mastering */
+	rcar_rmw32(pcie, RCONF(PCI_STATUS), PCI_STATUS_DEVSEL_MASK,
+		PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
+		PCI_STATUS_CAP_LIST | PCI_STATUS_DEVSEL_FAST);
+
+	wmb();
+
+	return 0;
+}
+
+static int __init rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
+{
+	unsigned int timeout = 10;
+
+	/* Initialize the phy */
+	phy_write_reg(pcie, 0, 0x42, 0x1, 0x0EC34191);
+	phy_write_reg(pcie, 1, 0x42, 0x1, 0x0EC34180);
+	phy_write_reg(pcie, 0, 0x43, 0x1, 0x00210188);
+	phy_write_reg(pcie, 1, 0x43, 0x1, 0x00210188);
+	phy_write_reg(pcie, 0, 0x44, 0x1, 0x015C0014);
+	phy_write_reg(pcie, 1, 0x44, 0x1, 0x015C0014);
+	phy_write_reg(pcie, 1, 0x4C, 0x1, 0x786174A0);
+	phy_write_reg(pcie, 1, 0x4D, 0x1, 0x048000BB);
+	phy_write_reg(pcie, 0, 0x51, 0x1, 0x079EC062);
+	phy_write_reg(pcie, 0, 0x52, 0x1, 0x20000000);
+	phy_write_reg(pcie, 1, 0x52, 0x1, 0x20000000);
+	phy_write_reg(pcie, 1, 0x56, 0x1, 0x00003806);
+
+	phy_write_reg(pcie, 0, 0x60, 0x1, 0x004B03A5);
+	phy_write_reg(pcie, 0, 0x64, 0x1, 0x3F0F1F0F);
+	phy_write_reg(pcie, 0, 0x66, 0x1, 0x00008000);
+
+	while (timeout--) {
+		if (pci_read_reg(pcie, H1_PCIEPHYSR))
+			return rcar_pcie_hw_init(pcie);
+
+		msleep(5);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int __init rcar_pcie_get_resources(struct platform_device *pdev,
+	struct rcar_pcie *pcie)
+{
+	struct resource res;
+	int err;
+
+	err = of_address_to_resource(pdev->dev.of_node, 0, &res);
+	if (err)
+		return err;
+
+	pcie->clk = devm_clk_get(&pdev->dev, "pcie");
+	if (IS_ERR(pcie->clk)) {
+		dev_err(pcie->dev, "cannot get platform clock\n");
+		return PTR_ERR(pcie->clk);
+	}
+	err = clk_prepare_enable(pcie->clk);
+	if (err)
+		goto fail_clk;
+
+	pcie->bus_clk = devm_clk_get(&pdev->dev, "pcie_bus");
+	if (IS_ERR(pcie->bus_clk)) {
+		dev_err(pcie->dev, "cannot get pcie bus clock\n");
+		err = PTR_ERR(pcie->bus_clk);
+		goto fail_clk;
+	}
+	err = clk_prepare_enable(pcie->bus_clk);
+	if (err)
+		goto err_map_reg;
+
+	pcie->base = devm_ioremap_resource(&pdev->dev, &res);
+	if (IS_ERR(pcie->base)) {
+		err = PTR_ERR(pcie->base);
+		goto err_map_reg;
+	}
+
+	return 0;
+
+err_map_reg:
+	clk_disable_unprepare(pcie->bus_clk);
+fail_clk:
+	clk_disable_unprepare(pcie->clk);
+
+	return err;
+}
+
+static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
+				    struct of_pci_range *range,
+				    int *index)
+{
+	u64 restype = range->flags;
+	u64 cpu_addr = range->cpu_addr;
+	u64 cpu_end = range->cpu_addr + range->size;
+	u64 pci_addr = range->pci_addr;
+	u32 flags = LAM_64BIT | LAR_ENABLE;
+	u64 mask;
+	u64 size;
+	int idx = *index;
+
+	if (restype & IORESOURCE_PREFETCH)
+		flags |= LAM_PREFETCH;
+
+	/*
+	 * If the size of the range is larger than the alignment of the start
+	 * address, we have to use multiple entries to perform the mapping.
+	 */
+	if (cpu_addr > 0) {
+		unsigned long nr_zeros = __ffs64(cpu_addr);
+		u64 alignment = 1ULL << nr_zeros;
+		size = min(range->size, alignment);
+	} else {
+		size = range->size;
+	}
+	/* Hardware supports max 4GiB inbound region */
+	size = min(size, 1ULL << 32);
+
+	mask = roundup_pow_of_two(size) - 1;
+	mask &= ~0xf;
+
+	while (cpu_addr < cpu_end) {
+		/*
+		 * Set up 64-bit inbound regions as the range parser doesn't
+		 * distinguish between 32 and 64-bit types.
+		 */
+		pci_write_reg(pcie, lower_32_bits(pci_addr), PCIEPRAR(idx));
+		pci_write_reg(pcie, lower_32_bits(cpu_addr), PCIELAR(idx));
+		pci_write_reg(pcie, lower_32_bits(mask) | flags, PCIELAMR(idx));
+
+		pci_write_reg(pcie, upper_32_bits(pci_addr), PCIEPRAR(idx+1));
+		pci_write_reg(pcie, upper_32_bits(cpu_addr), PCIELAR(idx+1));
+		pci_write_reg(pcie, 0, PCIELAMR(idx+1));
+
+		pci_addr += size;
+		cpu_addr += size;
+		idx += 2;
+
+		if (idx > MAX_NR_INBOUND_MAPS) {
+			dev_err(pcie->dev, "Failed to map inbound regions!\n");
+			return -EINVAL;
+		}
+	}
+	*index = idx;
+
+	return 0;
+}
+
+static int pci_dma_range_parser_init(struct of_pci_range_parser *parser,
+				     struct device_node *node)
+{
+	const int na = 3, ns = 2;
+	int rlen;
+
+	parser->node = node;
+	parser->pna = of_n_addr_cells(node);
+	parser->np = parser->pna + na + ns;
+
+	parser->range = of_get_property(node, "dma-ranges", &rlen);
+	if (!parser->range)
+		return -ENOENT;
+
+	parser->end = parser->range + rlen / sizeof(__be32);
+	return 0;
+}
+
+static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
+					  struct device_node *np)
+{
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	int index = 0;
+	int err;
+
+	if (pci_dma_range_parser_init(&parser, np))
+		return -EINVAL;
+
+	/* Get the dma-ranges from DT */
+	for_each_of_pci_range(&parser, &range) {
+		u64 end = range.cpu_addr + range.size - 1;
+		dev_dbg(pcie->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
+			range.flags, range.cpu_addr, end, range.pci_addr);
+
+		err = rcar_pcie_inbound_ranges(pcie, &range, &index);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id rcar_pcie_of_match[] = {
+	{ .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
+	{ .compatible = "renesas,pcie-r8a7790", .data = rcar_pcie_hw_init },
+	{ .compatible = "renesas,pcie-r8a7791", .data = rcar_pcie_hw_init },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rcar_pcie_of_match);
+
+static int __init rcar_pcie_probe(struct platform_device *pdev)
+{
+	struct rcar_pcie *pcie;
+	unsigned int data;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	const struct of_device_id *of_id;
+	int err, win = 0;
+	int (*hw_init_fn)(struct rcar_pcie *);
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->dev = &pdev->dev;
+	platform_set_drvdata(pdev, pcie);
+
+	if (of_pci_range_parser_init(&parser, pdev->dev.of_node)) {
+		dev_err(&pdev->dev, "missing ranges property\n");
+		return -EINVAL;
+	}
+
+	err = rcar_pcie_get_resources(pdev, pcie);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to request resources: %d\n", err);
+		return err;
+	}
+
+	for_each_of_pci_range(&parser, &range) {
+		of_pci_range_to_resource(&range, pdev->dev.of_node,
+						&pcie->res[win++]);
+
+		if (win > PCI_MAX_RESOURCES)
+			break;
+	}
+
+	 err = rcar_pcie_parse_map_dma_ranges(pcie, pdev->dev.of_node);
+	 if (err)
+		return err;
+
+	of_id = of_match_device(rcar_pcie_of_match, pcie->dev);
+	if (!of_id || !of_id->data)
+		return -EINVAL;
+	hw_init_fn = of_id->data;
+
+	/* Failure to get a link might just be that no cards are inserted */
+	err = hw_init_fn(pcie);
+	if (err) {
+		dev_info(&pdev->dev, "PCIe link down\n");
+		return 0;
+	}
+
+	data = pci_read_reg(pcie, MACSR);
+	dev_info(&pdev->dev, "PCIe x%d: link up\n", (data >> 20) & 0x3f);
+
+	rcar_pcie_enable(pcie);
+
+	return 0;
+}
+
+static struct platform_driver rcar_pcie_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = rcar_pcie_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = rcar_pcie_probe,
+};
+module_platform_driver(rcar_pcie_driver);
+
+MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
+MODULE_DESCRIPTION("Renesas R-Car PCIe driver");
+MODULE_LICENSE("GPLv2");
diff --git a/drivers/pci/host/pcie-rcar.h b/drivers/pci/host/pcie-rcar.h
new file mode 100644
index 0000000..3dc026b
--- /dev/null
+++ b/drivers/pci/host/pcie-rcar.h
@@ -0,0 +1,82 @@
+/*
+ * PCI Express definitions for Renesas R-Car SoCs
+ */
+#ifndef __PCI_RCAR_H
+#define __PCI_RCAR_H
+
+#define PCIECAR			0x000010
+#define PCIECCTLR		0x000018
+#define  CONFIG_SEND_ENABLE	(1 << 31)
+#define  TYPE0			(0 << 8)
+#define  TYPE1			(1 << 8)
+#define PCIECDR			0x000020
+#define PCIEMSR			0x000028
+#define PCIEINTXR		0x000400
+#define PCIEPHYSR		0x0007f0
+
+/* Transfer control */
+#define PCIETCTLR		0x02000
+#define  CFINIT			1
+#define PCIETSTR		0x02004
+#define  DATA_LINK_ACTIVE	1
+#define PCIEINTR		0x02008
+#define PCIEINTER		0x0200c
+#define PCIEERRFR		0x02020
+#define  UNSUPPORTED_REQUEST	(1 << 4)
+#define PCIEERRFER		0x02024
+#define PCIEERRFR2		0x02028
+#define PCIEPMSR		0x02034
+#define PCIEPMSCIER		0x02038
+#define PCIEMSIFR		0x02044
+
+/* root port address */
+#define PCIEPRAR(x)		(0x02080 + ((x) * 0x4))
+
+/* local address reg & mask */
+#define PCIELAR(x)		(0x02200 + ((x) * 0x20))
+#define PCIELAMR(x)		(0x02208 + ((x) * 0x20))
+#define  LAM_PMIOLAMnB3		(1 << 3)
+#define  LAM_PMIOLAMnB2		(1 << 2)
+#define  LAM_PREFETCH		(1 << 3)
+#define  LAM_64BIT		(1 << 2)
+#define  LAR_ENABLE		(1 << 1)
+
+/* PCIe address reg & mask */
+#define PCIEPARL(x)		(0x03400 + ((x) * 0x20))
+#define PCIEPARH(x)		(0x03404 + ((x) * 0x20))
+#define PCIEPAMR(x)		(0x03408 + ((x) * 0x20))
+#define PCIEPTCTLR(x)		(0x0340c + ((x) * 0x20))
+#define  PAR_ENABLE		(1 << 31)
+#define  IO_SPACE		(1 << 8)
+
+/* Configuration */
+#define PCICONF(x)		(0x010000 + ((x) * 0x4))
+#define PMCAP(x)		(0x010040 + ((x) * 0x4))
+#define MSICAP(x)		(0x010050 + ((x) * 0x4))
+#define EXPCAP(x)		(0x010070 + ((x) * 0x4))
+#define VCCAP(x)		(0x010100 + ((x) * 0x4))
+#define SERNUMCAP(x)		(0x0101b0 + ((x) * 0x4))
+
+/* link layer */
+#define IDSETR0			0x011000
+#define IDSETR1			0x011004
+#define SUBIDSETR		0x011024
+#define DSERSETR0		0x01102c
+#define DSERSETR1		0x011030
+#define TLCTLR			0x011048
+#define MACSR			0x011054
+#define MACCTLR			0x011058
+#define  SCRAMBLE_DISABLE	(1 << 27)
+
+/* R-Car H1 PHY */
+#define H1_PCIEPHYCTLR		0x040008
+#define H1_PCIEPHYADRR		0x04000c
+#define  WRITE_CMD		(1 << 16)
+#define  PHY_ACK		(1 << 24)
+#define  RATE_POS		12
+#define  LANE_POS		8
+#define  ADR_POS		0
+#define H1_PCIEPHYDOUTR		0x040014
+#define H1_PCIEPHYSR		0x040018
+
+#endif /* __PCI_RCAR_H */
-- 
1.9.0

[PATCH v5 2/9] PCI: host: rcar: Add MSI support

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Return IRQ_NONE from MSI isr when there is no pending MSI
 - Add additional interrupt bindings
---
 drivers/pci/host/pcie-rcar.c | 246 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/host/pcie-rcar.h |   5 +
 2 files changed, 250 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index c22c896..280f370 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -15,8 +15,11 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/msi.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_pci.h>
@@ -28,6 +31,8 @@
 
 #define DRV_NAME "rcar-pcie"
 
+#define INT_PCI_MSI_NR	32
+
 #define RCONF(x)	(PCICONF(0)+(x))
 #define RPMCAP(x)	(PMCAP(0)+(x))
 #define REXPCAP(x)	(EXPCAP(0)+(x))
@@ -40,6 +45,21 @@
 #define PCI_MAX_RESOURCES 4
 #define MAX_NR_INBOUND_MAPS 6
 
+struct rcar_msi {
+	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
+	struct irq_domain *domain;
+	struct msi_chip chip;
+	unsigned long pages;
+	struct mutex lock;
+	int irq1;
+	int irq2;
+};
+
+static inline struct rcar_msi *to_rcar_msi(struct msi_chip *chip)
+{
+	return container_of(chip, struct rcar_msi, chip);
+}
+
 /* Structure representing the PCIe interface */
 struct rcar_pcie {
 	struct device		*dev;
@@ -48,6 +68,7 @@ struct rcar_pcie {
 	u8			root_bus_nr;
 	struct clk		*clk;
 	struct clk		*bus_clk;
+	struct			rcar_msi msi;
 };
 
 static inline struct rcar_pcie *sys_to_pcie(struct pci_sys_data *sys)
@@ -292,6 +313,15 @@ static int rcar_pcie_setup(int nr, struct pci_sys_data *sys)
 	return 1;
 }
 
+static void rcar_pcie_add_bus(struct pci_bus *bus)
+{
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		struct rcar_pcie *pcie = sys_to_pcie(bus->sysdata);
+
+		bus->msi = &pcie->msi.chip;
+	}
+}
+
 static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
 {
 	struct platform_device *pdev = to_platform_device(pcie->dev);
@@ -301,6 +331,7 @@ static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
 		.setup          = rcar_pcie_setup,
 		.map_irq        = of_irq_parse_and_map_pci,
 		.ops            = &rcar_pcie_ops,
+		.add_bus        = rcar_pcie_add_bus,
 	};
 
 	pci_common_init_dev(&pdev->dev, &hw);
@@ -408,6 +439,10 @@ static int __init rcar_pcie_hw_init(struct rcar_pcie *pcie)
 	/* Enable MAC data scrambling. */
 	rcar_rmw32(pcie, MACCTLR, SCRAMBLE_DISABLE, 0);
 
+	/* Enable MSI */
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		pci_write_reg(pcie, 0x101f0000, PCIEMSITXR);
+
 	/* Finish initialization - establish a PCI Express link */
 	pci_write_reg(pcie, CFINIT, PCIETCTLR);
 
@@ -461,11 +496,194 @@ static int __init rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
 	return -ETIMEDOUT;
 }
 
+static int rcar_msi_alloc(struct rcar_msi *chip)
+{
+	int msi;
+
+	mutex_lock(&chip->lock);
+
+	msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR);
+	if (msi < INT_PCI_MSI_NR)
+		set_bit(msi, chip->used);
+	else
+		msi = -ENOSPC;
+
+	mutex_unlock(&chip->lock);
+
+	return msi;
+}
+
+static void rcar_msi_free(struct rcar_msi *chip, unsigned long irq)
+{
+	struct device *dev = chip->chip.dev;
+
+	mutex_lock(&chip->lock);
+
+	if (!test_bit(irq, chip->used))
+		dev_err(dev, "trying to free unused MSI#%lu\n", irq);
+	else
+		clear_bit(irq, chip->used);
+
+	mutex_unlock(&chip->lock);
+}
+
+static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
+{
+	struct rcar_pcie *pcie = data;
+	struct rcar_msi *msi = &pcie->msi;
+	unsigned long reg;
+
+	reg = pci_read_reg(pcie, PCIEMSIFR);
+
+	/* MSI & INTx share an interrupt - we only handle MSI here */
+	if (!reg)
+		return IRQ_NONE;
+
+	while (reg) {
+		unsigned int index = find_first_bit(&reg, 32);
+		unsigned int irq;
+
+		/* clear the interrupt */
+		pci_write_reg(pcie, 1 << index, PCIEMSIFR);
+
+		irq = irq_find_mapping(msi->domain, index);
+		if (irq) {
+			if (test_bit(index, msi->used))
+				generic_handle_irq(irq);
+			else
+				dev_info(pcie->dev, "unhandled MSI\n");
+		} else {
+			/*
+			 * that's weird who triggered this?
+			 * just clear it
+			 */
+			dev_info(pcie->dev, "unexpected MSI\n");
+		}
+
+		/* see if there's any more pending in this vector */
+		reg = pci_read_reg(pcie, PCIEMSIFR);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int rcar_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
+			       struct msi_desc *desc)
+{
+	struct rcar_msi *msi = to_rcar_msi(chip);
+	struct rcar_pcie *pcie = container_of(chip, struct rcar_pcie, msi.chip);
+	struct msi_msg msg;
+	unsigned int irq;
+	int hwirq;
+
+	hwirq = rcar_msi_alloc(msi);
+	if (hwirq < 0)
+		return hwirq;
+
+	irq = irq_create_mapping(msi->domain, hwirq);
+	if (!irq) {
+		rcar_msi_free(msi, hwirq);
+		return -EINVAL;
+	}
+
+	irq_set_msi_desc(irq, desc);
+
+	msg.address_lo = pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
+	msg.address_hi = pci_read_reg(pcie, PCIEMSIAUR);
+	msg.data = hwirq;
+
+	write_msi_msg(irq, &msg);
+
+	return 0;
+}
+
+static void rcar_msi_teardown_irq(struct msi_chip *chip, unsigned int irq)
+{
+	struct rcar_msi *msi = to_rcar_msi(chip);
+	struct irq_data *d = irq_get_irq_data(irq);
+
+	rcar_msi_free(msi, d->hwirq);
+}
+
+static struct irq_chip rcar_msi_irq_chip = {
+	.name = "R-Car PCIe MSI",
+	.irq_enable = unmask_msi_irq,
+	.irq_disable = mask_msi_irq,
+	.irq_mask = mask_msi_irq,
+	.irq_unmask = unmask_msi_irq,
+};
+
+static int rcar_msi_map(struct irq_domain *domain, unsigned int irq,
+			 irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(irq, &rcar_msi_irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, domain->host_data);
+	set_irq_flags(irq, IRQF_VALID);
+
+	return 0;
+}
+
+static const struct irq_domain_ops msi_domain_ops = {
+	.map = rcar_msi_map,
+};
+
+static int rcar_pcie_enable_msi(struct rcar_pcie *pcie)
+{
+	struct platform_device *pdev = to_platform_device(pcie->dev);
+	struct rcar_msi *msi = &pcie->msi;
+	unsigned long base;
+	int err;
+
+	mutex_init(&msi->lock);
+
+	msi->chip.dev = pcie->dev;
+	msi->chip.setup_irq = rcar_msi_setup_irq;
+	msi->chip.teardown_irq = rcar_msi_teardown_irq;
+
+	msi->domain = irq_domain_add_linear(pcie->dev->of_node, INT_PCI_MSI_NR,
+					    &msi_domain_ops, &msi->chip);
+	if (!msi->domain) {
+		dev_err(&pdev->dev, "failed to create IRQ domain\n");
+		return -ENOMEM;
+	}
+
+	/* Two irqs are for MSI, but they are also used for non-MSI irqs */
+	err = devm_request_irq(&pdev->dev, msi->irq1, rcar_pcie_msi_irq,
+			       IRQF_SHARED, rcar_msi_irq_chip.name, pcie);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to request IRQ: %d\n", err);
+		goto err;
+	}
+
+	err = devm_request_irq(&pdev->dev, msi->irq2, rcar_pcie_msi_irq,
+			       IRQF_SHARED, rcar_msi_irq_chip.name, pcie);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to request IRQ: %d\n", err);
+		goto err;
+	}
+
+	/* setup MSI data target */
+	msi->pages = __get_free_pages(GFP_KERNEL, 0);
+	base = virt_to_phys((void *)msi->pages);
+
+	pci_write_reg(pcie, base | MSIFE, PCIEMSIALR);
+	pci_write_reg(pcie, 0, PCIEMSIAUR);
+
+	/* enable all MSI interrupts */
+	pci_write_reg(pcie, 0xffffffff, PCIEMSIIER);
+
+	return 0;
+
+err:
+	irq_domain_remove(msi->domain);
+	return err;
+}
+
 static int __init rcar_pcie_get_resources(struct platform_device *pdev,
 	struct rcar_pcie *pcie)
 {
 	struct resource res;
-	int err;
+	int err, i;
 
 	err = of_address_to_resource(pdev->dev.of_node, 0, &res);
 	if (err)
@@ -490,6 +708,22 @@ static int __init rcar_pcie_get_resources(struct platform_device *pdev,
 	if (err)
 		goto err_map_reg;
 
+	i = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (i < 0) {
+		dev_err(pcie->dev, "cannot get platform resources for msi interrupt\n");
+		err = -ENOENT;
+		goto err_map_reg;
+	}
+	pcie->msi.irq1 = i;
+
+	i = irq_of_parse_and_map(pdev->dev.of_node, 1);
+	if (i < 0) {
+		dev_err(pcie->dev, "cannot get platform resources for msi interrupt\n");
+		err = -ENOENT;
+		goto err_map_reg;
+	}
+	pcie->msi.irq2 = i;
+
 	pcie->base = devm_ioremap_resource(&pdev->dev, &res);
 	if (IS_ERR(pcie->base)) {
 		err = PTR_ERR(pcie->base);
@@ -657,6 +891,16 @@ static int __init rcar_pcie_probe(struct platform_device *pdev)
 	 if (err)
 		return err;
 
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		err = rcar_pcie_enable_msi(pcie);
+		if (err < 0) {
+			dev_err(&pdev->dev,
+				"failed to enable MSI support: %d\n",
+				err);
+			return err;
+		}
+	}
+
 	of_id = of_match_device(rcar_pcie_of_match, pcie->dev);
 	if (!of_id || !of_id->data)
 		return -EINVAL;
diff --git a/drivers/pci/host/pcie-rcar.h b/drivers/pci/host/pcie-rcar.h
index 3dc026b..4f0c678 100644
--- a/drivers/pci/host/pcie-rcar.h
+++ b/drivers/pci/host/pcie-rcar.h
@@ -13,6 +13,7 @@
 #define PCIEMSR			0x000028
 #define PCIEINTXR		0x000400
 #define PCIEPHYSR		0x0007f0
+#define PCIEMSITXR		0x000840
 
 /* Transfer control */
 #define PCIETCTLR		0x02000
@@ -28,6 +29,10 @@
 #define PCIEPMSR		0x02034
 #define PCIEPMSCIER		0x02038
 #define PCIEMSIFR		0x02044
+#define PCIEMSIALR		0x02048
+#define  MSIFE			1
+#define PCIEMSIAUR		0x0204c
+#define PCIEMSIIER		0x02050
 
 /* root port address */
 #define PCIEPRAR(x)		(0x02080 + ((x) * 0x4))
-- 
1.9.0

Re: [PATCH v5 2/9] PCI: host: rcar: Add MSI support

[Ben Dooks]

On 25/03/14 16:56, Phil Edworthy wrote:
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v5:
>   - Return IRQ_NONE from MSI isr when there is no pending MSI
>   - Add additional interrupt bindings
> +
> +static void rcar_msi_free(struct rcar_msi *chip, unsigned long irq)
> +{
> +	struct device *dev = chip->chip.dev;
> +
> +	mutex_lock(&chip->lock);
> +
> +	if (!test_bit(irq, chip->used))
> +		dev_err(dev, "trying to free unused MSI#%lu\n", irq);

Does the upper level not check for this?

> +	else
> +		clear_bit(irq, chip->used);
> +
> +	mutex_unlock(&chip->lock);


> +}
> +
> +static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
> +{
> +	struct rcar_pcie *pcie = data;
> +	struct rcar_msi *msi = &pcie->msi;
> +	unsigned long reg;
> +
> +	reg = pci_read_reg(pcie, PCIEMSIFR);
> +
> +	/* MSI & INTx share an interrupt - we only handle MSI here */
> +	if (!reg)
> +		return IRQ_NONE;
> +
> +	while (reg) {
> +		unsigned int index = find_first_bit(&reg, 32);
> +		unsigned int irq;
> +
> +		/* clear the interrupt */
> +		pci_write_reg(pcie, 1 << index, PCIEMSIFR);
> +
> +		irq = irq_find_mapping(msi->domain, index);
> +		if (irq) {
> +			if (test_bit(index, msi->used))
> +				generic_handle_irq(irq);
> +			else
> +				dev_info(pcie->dev, "unhandled MSI\n");
> +		} else {
> +			/*
> +			 * that's weird who triggered this?
> +			 * just clear it
> +			 */
> +			dev_info(pcie->dev, "unexpected MSI\n");
> +		}

You may want to change this to something that is either rate-limited
or is say debug level so it does not spam the console if something
does go wrong.

Also the comment could easily be made one line
	/* Weird, unknown MSI IRQ, just clear it */


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH v5 2/9] PCI: host: rcar: Add MSI support

[Phil.Edworthy]

Hi Ben,

On: 25/03/2014 17:05, Ben wrote:
> Subject: Re: [PATCH v5 2/9] PCI: host: rcar: Add MSI support
> 
> On 25/03/14 16:56, Phil Edworthy wrote:
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> > v5:
> >   - Return IRQ_NONE from MSI isr when there is no pending MSI
> >   - Add additional interrupt bindings
> > +
> > +static void rcar_msi_free(struct rcar_msi *chip, unsigned long irq)
> > +{
> > +   struct device *dev = chip->chip.dev;
> > +
> > +   mutex_lock(&chip->lock);
> > +
> > +   if (!test_bit(irq, chip->used))
> > +      dev_err(dev, "trying to free unused MSI#%lu\n", irq);
> 
> Does the upper level not check for this?
Yes, the default_teardown_msi_irqs function checks for unused MSIs, so we 
don't really need to check here.


> > +   else
> > +      clear_bit(irq, chip->used);
> > +
> > +   mutex_unlock(&chip->lock);
> 
> 
> > +}
> > +
> > +static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
> > +{
> > +   struct rcar_pcie *pcie = data;
> > +   struct rcar_msi *msi = &pcie->msi;
> > +   unsigned long reg;
> > +
> > +   reg = pci_read_reg(pcie, PCIEMSIFR);
> > +
> > +   /* MSI & INTx share an interrupt - we only handle MSI here */
> > +   if (!reg)
> > +      return IRQ_NONE;
> > +
> > +   while (reg) {
> > +      unsigned int index = find_first_bit(&reg, 32);
> > +      unsigned int irq;
> > +
> > +      /* clear the interrupt */
> > +      pci_write_reg(pcie, 1 << index, PCIEMSIFR);
> > +
> > +      irq = irq_find_mapping(msi->domain, index);
> > +      if (irq) {
> > +         if (test_bit(index, msi->used))
> > +            generic_handle_irq(irq);
> > +         else
> > +            dev_info(pcie->dev, "unhandled MSI\n");
> > +      } else {
> > +         /*
> > +          * that's weird who triggered this?
> > +          * just clear it
> > +          */
> > +         dev_info(pcie->dev, "unexpected MSI\n");
> > +      }
> 
> You may want to change this to something that is either rate-limited
> or is say debug level so it does not spam the console if something
> does go wrong.
I'll change that to debug level.

> Also the comment could easily be made one line
>    /* Weird, unknown MSI IRQ, just clear it */
Sure!

Thanks
Phil

[PATCH v5 3/9] ARM: shmobile: r8a7790: Add PCIe clock device tree nodes

[Phil Edworthy]

This patch adds the device tree clock nodes for PCIe

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 arch/arm/boot/dts/r8a7790.dtsi            | 7 ++++---
 include/dt-bindings/clock/r8a7790-clock.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index da69afc..df9ec61 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -704,16 +704,17 @@
 			reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
 			clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
 				 <&cpg_clocks R8A7790_CLK_SD1>, <&cpg_clocks R8A7790_CLK_SD0>,
-				 <&mmc0_clk>, <&rclk_clk>;
+				 <&mmc0_clk>, <&rclk_clk>, <&mp_clk>;
 			#clock-cells = <1>;
 			renesas,clock-indices = <
 				R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
 				R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
-				R8A7790_CLK_MMCIF0 R8A7790_CLK_CMT1
+				R8A7790_CLK_MMCIF0 R8A7790_CLK_CMT1 R8A7790_CLK_PCIE
 			>;
 			clock-output-names =
 				"tpu0", "mmcif1", "sdhi3", "sdhi2",
-				"sdhi1", "sdhi0", "mmcif0", "cmt1";
+				"sdhi1", "sdhi0", "mmcif0", "cmt1",
+				"pcie";
 		};
 		mstp5_clks: mstp5_clks@e6150144 {
 			compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
diff --git a/include/dt-bindings/clock/r8a7790-clock.h b/include/dt-bindings/clock/r8a7790-clock.h
index 6548a5f..840dbc8 100644
--- a/include/dt-bindings/clock/r8a7790-clock.h
+++ b/include/dt-bindings/clock/r8a7790-clock.h
@@ -57,6 +57,7 @@
 #define R8A7790_CLK_SDHI1		13
 #define R8A7790_CLK_SDHI0		14
 #define R8A7790_CLK_MMCIF0		15
+#define R8A7790_CLK_PCIE		19
 #define R8A7790_CLK_SSUSB		28
 #define R8A7790_CLK_CMT1		29
 #define R8A7790_CLK_USBDMAC0		30
-- 
1.9.0

[PATCH v5 4/9] ARM: shmobile: r8a7791: Add PCIe clock device tree nodes

[Phil Edworthy]

This patch adds the device tree clock nodes for PCIe

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 arch/arm/boot/dts/r8a7791.dtsi            | 7 +++++--
 include/dt-bindings/clock/r8a7791-clock.h | 1 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 71bac2c..ccfba57 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -717,14 +717,17 @@
 			compatible = "renesas,r8a7791-mstp-clocks", "renesas,cpg-mstp-clocks";
 			reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
 			clocks = <&cp_clk>, <&sd2_clk>, <&sd1_clk>,
-				<&cpg_clocks R8A7791_CLK_SD0>, <&mmc0_clk>, <&rclk_clk>;
+				<&cpg_clocks R8A7791_CLK_SD0>, <&mmc0_clk>, <&rclk_clk>,
+				<&mp_clk>;
 			#clock-cells = <1>;
 			renesas,clock-indices = <
 				R8A7791_CLK_TPU0 R8A7791_CLK_SDHI2 R8A7791_CLK_SDHI1
 				R8A7791_CLK_SDHI0 R8A7791_CLK_MMCIF0 R8A7791_CLK_CMT1
+				R8A7791_CLK_PCIE
 			>;
 			clock-output-names =
-				"tpu0", "sdhi2", "sdhi1", "sdhi0", "mmcif0", "cmt1";
+				"tpu0", "sdhi2", "sdhi1", "sdhi0", "mmcif0", "cmt1",
+				"pcie";
 		};
 		mstp5_clks: mstp5_clks@e6150144 {
 			compatible = "renesas,r8a7791-mstp-clocks", "renesas,cpg-mstp-clocks";
diff --git a/include/dt-bindings/clock/r8a7791-clock.h b/include/dt-bindings/clock/r8a7791-clock.h
index 30f82f2..d3e2cf5 100644
--- a/include/dt-bindings/clock/r8a7791-clock.h
+++ b/include/dt-bindings/clock/r8a7791-clock.h
@@ -51,6 +51,7 @@
 #define R8A7791_CLK_SDHI1		12
 #define R8A7791_CLK_SDHI0		14
 #define R8A7791_CLK_MMCIF0		15
+#define R8A7791_CLK_PCIE		19
 #define R8A7791_CLK_SSUSB		28
 #define R8A7791_CLK_CMT1		29
 #define R8A7791_CLK_USBDMAC0		30
-- 
1.9.0

[PATCH v5 5/9] dt-bindings: pci: rcar pcie device tree bindings

[Phil Edworthy]

This patch adds the bindings for the R-Car PCIe driver. The driver
resides under drivers/pci/host/pcie-rcar.c

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Add PCIe bus clock reference
 - Add additional interrupt bindings
 - Use dma-ranges property to specify inbound memory regions
---
 Documentation/devicetree/bindings/pci/rcar-pci.txt | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt b/Documentation/devicetree/bindings/pci/rcar-pci.txt
new file mode 100644
index 0000000..61bf5ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
@@ -0,0 +1,44 @@
+* Renesas RCar PCIe interface
+
+Required properties:
+- compatible: should contain one of the following
+	"renesas,pcie-r8a7779", "renesas,pcie-r8a7790", "renesas,pcie-r8a7791"
+- reg: base addresses and lengths of the pcie controller.
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- device_type: set to "pci"
+- ranges: ranges for the PCI memory and I/O regions.
+- dma-ranges: ranges for the inbound memory regions.
+- interrupts: two interrupt sources for MSI interrupts, followed by interrupt
+	source for hardware related interrupts (e.g. link speed change).
+- #interrupt-cells: set to <1>
+- interrupt-map-mask and interrupt-map: standard PCI properties
+	to define the mapping of the PCIe interface to interrupt
+	numbers.
+- clocks: from common clock binding: handle to pci clock.
+- clock-names: from common clock binding: should be "pcie" and "pcie_bus".
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie: pcie@fe000000 {
+		compatible = "renesas,pcie-r8a7791";
+		reg = <0 0xfe000000 0 0x80000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
+			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
+			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
+			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
+		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000
+			      0x42000000 2 0x00000000 2 0x00000000 0 0x40000000>;
+		interrupts = <0 116 4 0 117 4 0 118 4>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 4>;
+		clocks = <&mstp3_clks R8A7791_CLK_PCIE>, <&pcie_bus_clk>;
+		clock-names = "pcie", "pcie_bus";
+		status = "disabled";
+	};
-- 
1.9.0

Re: [PATCH v5 5/9] dt-bindings: pci: rcar pcie device tree bindings

[Sergei Shtylyov]

Hello.

On 03/25/2014 07:56 PM, Phil Edworthy wrote:

> This patch adds the bindings for the R-Car PCIe driver. The driver
> resides under drivers/pci/host/pcie-rcar.c

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v5:
>   - Add PCIe bus clock reference
>   - Add additional interrupt bindings
>   - Use dma-ranges property to specify inbound memory regions
> ---
>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 44 ++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/pci/rcar-pci.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> new file mode 100644
> index 0000000..61bf5ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> @@ -0,0 +1,44 @@
> +* Renesas RCar PCIe interface
> +
> +Required properties:
> +- compatible: should contain one of the following
> +	"renesas,pcie-r8a7779", "renesas,pcie-r8a7790", "renesas,pcie-r8a7791"
> +- reg: base addresses and lengths of the pcie controller.

    Hmm, need "registers" at end. And why plural?

> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions.
> +- dma-ranges: ranges for the inbound memory regions.
> +- interrupts: two interrupt sources for MSI interrupts, followed by interrupt
> +	source for hardware related interrupts (e.g. link speed change).
> +- #interrupt-cells: set to <1>
> +- interrupt-map-mask and interrupt-map: standard PCI properties
> +	to define the mapping of the PCIe interface to interrupt
> +	numbers.
> +- clocks: from common clock binding: handle to pci clock.

    s/handle/phandle/, s/pci/PCI/.
    Actually, it's a clock specifier consisting not only of phandle but also 
of clock #. Looking at your example, it's even a pair of clock specifiers.

> +- clock-names: from common clock binding: should be "pcie" and "pcie_bus".
> +
> +Example:
> +
> +SoC specific DT Entry:
> +
> +	pcie: pcie@fe000000 {
> +		compatible = "renesas,pcie-r8a7791";
> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> +			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> +			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> +			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> +		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000
> +			      0x42000000 2 0x00000000 2 0x00000000 0 0x40000000>;

    Hmm, this prop looks board-dependent...

> +		interrupts = <0 116 4 0 117 4 0 118 4>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 4>;
> +		clocks = <&mstp3_clks R8A7791_CLK_PCIE>, <&pcie_bus_clk>;
> +		clock-names = "pcie", "pcie_bus";
> +		status = "disabled";
> +	};

WBR, Sergei

Re: [PATCH v5 5/9] dt-bindings: pci: rcar pcie device tree bindings

[Phil.Edworthy]

Hi Sergei,

On: 25/03/2014 19:22, Sergei wrote:
> Subject: Re: [PATCH v5 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> Hello.
> 
> On 03/25/2014 07:56 PM, Phil Edworthy wrote:
> 
> > This patch adds the bindings for the R-Car PCIe driver. The driver
> > resides under drivers/pci/host/pcie-rcar.c
> 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> > v5:
> >   - Add PCIe bus clock reference
> >   - Add additional interrupt bindings
> >   - Use dma-ranges property to specify inbound memory regions
> > ---
> >   Documentation/devicetree/bindings/pci/rcar-pci.txt | 44 
++++++++++++++++++++++
> >   1 file changed, 44 insertions(+)
> >   create mode 100644 
Documentation/devicetree/bindings/pci/rcar-pci.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt b/
> Documentation/devicetree/bindings/pci/rcar-pci.txt
> > new file mode 100644
> > index 0000000..61bf5ef
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > @@ -0,0 +1,44 @@
> > +* Renesas RCar PCIe interface
> > +
> > +Required properties:
> > +- compatible: should contain one of the following
> > +   "renesas,pcie-r8a7779", "renesas,pcie-r8a7790", 
"renesas,pcie-r8a7791"
> > +- reg: base addresses and lengths of the pcie controller.
> 
>     Hmm, need "registers" at end. And why plural?
Ok, & yes it should be singular.


> > +- #address-cells: set to <3>
> > +- #size-cells: set to <2>
> > +- device_type: set to "pci"
> > +- ranges: ranges for the PCI memory and I/O regions.
> > +- dma-ranges: ranges for the inbound memory regions.
> > +- interrupts: two interrupt sources for MSI interrupts, followed by 
interrupt
> > +   source for hardware related interrupts (e.g. link speed change).
> > +- #interrupt-cells: set to <1>
> > +- interrupt-map-mask and interrupt-map: standard PCI properties
> > +   to define the mapping of the PCIe interface to interrupt
> > +   numbers.
> > +- clocks: from common clock binding: handle to pci clock.
> 
>     s/handle/phandle/, s/pci/PCI/.
>     Actually, it's a clock specifier consisting not only of phandle but 
also 
> of clock #. Looking at your example, it's even a pair of clock 
specifiers.
Right, and yes it's two clocks - I missed that when adding the PCI bus 
clock.

 
> > +- clock-names: from common clock binding: should be "pcie" and 
"pcie_bus".
> > +
> > +Example:
> > +
> > +SoC specific DT Entry:
> > +
> > +   pcie: pcie@fe000000 {
> > +      compatible = "renesas,pcie-r8a7791";
> > +      reg = <0 0xfe000000 0 0x80000>;
> > +      #address-cells = <3>;
> > +      #size-cells = <2>;
> > +      device_type = "pci";
> > +      ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> > +           0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> > +           0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> > +           0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> > +      dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000
> > +               0x42000000 2 0x00000000 2 0x00000000 0 0x40000000>;
> 
>     Hmm, this prop looks board-dependent...
Initially I thought that it would be board dependent and map the actual 
DDR available. However, I then considered that it is probably ok to map 
all possible DDR ranges that the device is capable of using. I'm ok with 
either approach though...


> > +      interrupts = <0 116 4 0 117 4 0 118 4>;
> > +      #interrupt-cells = <1>;
> > +      interrupt-map-mask = <0 0 0 0>;
> > +      interrupt-map = <0 0 0 0 &gic 0 116 4>;
> > +      clocks = <&mstp3_clks R8A7791_CLK_PCIE>, <&pcie_bus_clk>;
> > +      clock-names = "pcie", "pcie_bus";
> > +      status = "disabled";
> > +   };
> 
> WBR, Sergei
> 

Thanks
Phil

[PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil Edworthy]

This patch adds the device tree nodes for R8A7790

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Renesas SoCs compatible string has peripheral before device name
 - Add PCIe bus clock reference
 - Add additional interrupt bindings
 - Use dma-ranges property to specify inbound memory regions
---
 arch/arm/boot/dts/r8a7790-lager.dts | 10 ++++++++++
 arch/arm/boot/dts/r8a7790.dtsi      | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index a55c5f8..bbbcb63 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -139,6 +139,16 @@
 		states = <3300000 1
 			  1800000 0>;
 	};
+
+	clocks {
+		/* External PCIe bus clock - not used */
+		pcie_bus_clk: pcie_bus_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <0>;
+			clock-output-names = "pcie_bus";
+		};
+	};
 };
 
 &extal_clk {
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index df9ec61..dbcea57 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -821,4 +821,28 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	pcie: pcie@fe000000 {
+		compatible = "renesas,pcie-r8a7790";
+		reg = <0 0xfe000000 0 0x80000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
+			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
+			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
+			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
+		/* Map all possible DDR as inbound ranges */
+		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
+			      0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;
+		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
+			      0 117 IRQ_TYPE_LEVEL_HIGH
+			      0 118 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7790_CLK_PCIE>, <&pcie_bus_clk>;
+		clock-names = "pcie", "pcie_bus";
+		status = "disabled";
+	};
 };
-- 
1.9.0

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Arnd Bergmann]

On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> +               /* Map all possible DDR as inbound ranges */
> +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
> +                             0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;

Typo: 0x43000000 should be 0x42000000 I guess.

Since you control the mapping, I wonder if you could also do this as

               dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 0x80000000
                             0x43000000 0 0x80000000 1 0x80000000 0 0x80000000>;

i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
This would be really nice from the perspective that now all PCI
devices could access all of RAM without using an IOMMU or the
relying on 64-bit master capability.

The downside of this is that you'd probably need a custom dma_map_ops
wrapper.

	Arnd

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil.Edworthy]

Hi Arnd,

On: 25/03/2014 18:42, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > +               /* Map all possible DDR as inbound ranges */
> > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 
0x80000000
> > +                             0x43000000 1 0x80000000 1 0x80000000 0 
0x80000000>;
> 
> Typo: 0x43000000 should be 0x42000000 I guess.
I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
currently treats both 32 and 64-bit types the same way, but I thought it 
would be good to set this in case we ever need to use it.
 
> Since you control the mapping, I wonder if you could also do this as
> 
>                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
0x80000000
>                              0x43000000 0 0x80000000 1 0x80000000 0 
0x80000000>;
> 
> i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
> This would be really nice from the perspective that now all PCI
> devices could access all of RAM without using an IOMMU or the
> relying on 64-bit master capability.
> 
> The downside of this is that you'd probably need a custom dma_map_ops
> wrapper.
Since the OF PCi range code treats both 32 and 64-bit types the same way, 
my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
controller has to use a 64-bit mapping for anything over 2GiB. Based on 
this, I think it's sensible to leave the mappings as 1-to-1.

Thanks
Phil

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Arnd Bergmann]

On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> Hi Arnd,
> 
> On: 25/03/2014 18:42, Arnd wrote:
> > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
> for R8A7790
> > 
> > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > +               /* Map all possible DDR as inbound ranges */
> > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 
> 0x80000000
> > > +                             0x43000000 1 0x80000000 1 0x80000000 0 
> 0x80000000>;
> > 
> > Typo: 0x43000000 should be 0x42000000 I guess.
> I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> currently treats both 32 and 64-bit types the same way, but I thought it 
> would be good to set this in case we ever need to use it.

Ah, I forgot about the space identifier. It looks correct then, but
it seems  a little strange to use a 32-bit identifier in one case
and a 64-bit one in the other.

> > Since you control the mapping, I wonder if you could also do this as
> > 
> >                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
> 0x80000000
> >                              0x43000000 0 0x80000000 1 0x80000000 0 
> 0x80000000>;
> > 
> > i.e. map all the RAM into PCI bus addresses below the 32-bit boundary.
> > This would be really nice from the perspective that now all PCI
> > devices could access all of RAM without using an IOMMU or the
> > relying on 64-bit master capability.
> > 
> > The downside of this is that you'd probably need a custom dma_map_ops
> > wrapper.
> Since the OF PCi range code treats both 32 and 64-bit types the same way, 
> my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> controller has to use a 64-bit mapping for anything over 2GiB. Based on 
> this, I think it's sensible to leave the mappings as 1-to-1.

I'm not following, sorry. What is the hardware requirement in the
controller?

	Arnd

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil.Edworthy]

Hi Arnd,

On: 26/03/2014 10:34, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > Hi Arnd,
> > 
> > On: 25/03/2014 18:42, Arnd wrote:
> > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
nodes 
> > for R8A7790
> > > 
> > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > +               /* Map all possible DDR as inbound ranges */
> > > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 
0 
> > 0x80000000
> > > > +                             0x43000000 1 0x80000000 1 0x80000000 
0 
> > 0x80000000>;
> > > 
> > > Typo: 0x43000000 should be 0x42000000 I guess.
> > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > currently treats both 32 and 64-bit types the same way, but I thought 
it 
> > would be good to set this in case we ever need to use it.
> 
> Ah, I forgot about the space identifier. It looks correct then, but
> it seems  a little strange to use a 32-bit identifier in one case
> and a 64-bit one in the other.

If the OF PCI range code allowed the PCIe host driver to determine if it's 
a 32-bit mapping, we could use that and get a small performance 
improvement with PCIe throughput.

> > > Since you control the mapping, I wonder if you could also do this as
> > > 
> > >                dma-ranges = <0x42000000 0 0x00000000 0 0x40000000 0 
> > 0x80000000
> > >                              0x43000000 0 0x80000000 1 0x80000000 0 
> > 0x80000000>;
> > > 
> > > i.e. map all the RAM into PCI bus addresses below the 32-bit 
boundary.
> > > This would be really nice from the perspective that now all PCI
> > > devices could access all of RAM without using an IOMMU or the
> > > relying on 64-bit master capability.
> > > 
> > > The downside of this is that you'd probably need a custom 
dma_map_ops
> > > wrapper.
> > Since the OF PCi range code treats both 32 and 64-bit types the same 
way, 
> > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> > controller has to use a 64-bit mapping for anything over 2GiB. Based 
on 
> > this, I think it's sensible to leave the mappings as 1-to-1.
> 
> I'm not following, sorry. What is the hardware requirement in the
> controller?
With this controller, you can only specify maps whose size are a power of 
two, and the size must be less than or equal to the cpu address alignment. 
Further, when the size is 4GiB, you have to use a 64-bit mapping. Thinking 
about it, the 4GiB case is not relevant to our discussion about 32-bit vs 
64-bit mappings.

Still, my comment about the OF PCI range code treating both 32 and 64-bit 
types the same way means that PCIe host driver has to assume it's a 64-bit 
mapping.

Thanks
Phil

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Arnd Bergmann]

On Wednesday 26 March 2014 11:01:46 Phil.Edworthy@renesas.com wrote:
> On: 26/03/2014 10:34, Arnd wrote:
> > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
> for R8A7790
> > 
> > On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > > Hi Arnd,
> > > 
> > > On: 25/03/2014 18:42, Arnd wrote:
> > > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
> nodes 
> > > for R8A7790
> > > > 
> > > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > > +               /* Map all possible DDR as inbound ranges */
> > > > > +               dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 
> 0 
> > > 0x80000000
> > > > > +                             0x43000000 1 0x80000000 1 0x80000000 
> 0 
> > > 0x80000000>;
> > > > 
> > > > Typo: 0x43000000 should be 0x42000000 I guess.
> > > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > > currently treats both 32 and 64-bit types the same way, but I thought 
> it 
> > > would be good to set this in case we ever need to use it.
> > 
> > Ah, I forgot about the space identifier. It looks correct then, but
> > it seems  a little strange to use a 32-bit identifier in one case
> > and a 64-bit one in the other.
> 
> If the OF PCI range code allowed the PCIe host driver to determine if it's 
> a 32-bit mapping, we could use that and get a small performance 
> improvement with PCIe throughput.

I don't think it's supposed to care. Soem of the upper bits of the ranges
only really make sense of PCI device registers, not for the top-level
ranges property. The driver can however still look at the address itself
to get that information.

> > > Since the OF PCi range code treats both 32 and 64-bit types the same 
> way, 
> > > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 
> > > controller has to use a 64-bit mapping for anything over 2GiB. Based 
> on 
> > > this, I think it's sensible to leave the mappings as 1-to-1.
> > 
> > I'm not following, sorry. What is the hardware requirement in the
> > controller?
> With this controller, you can only specify maps whose size are a power of 
> two, and the size must be less than or equal to the cpu address alignment. 
> Further, when the size is 4GiB, you have to use a 64-bit mapping. Thinking 
> about it, the 4GiB case is not relevant to our discussion about 32-bit vs 
> 64-bit mappings.

But the ranges you specified in the property don't actually fit in those
constraints: you have a range with size 0x8000000 and start 0x40000000,
which you say can't be programmed into the hardware.

> Still, my comment about the OF PCI range code treating both 32 and 64-bit 
> types the same way means that PCIe host driver has to assume it's a 64-bit 
> mapping.

I was thinking more of PCI devices than the host itself. If the host
driver can verify that all mappings are in the first 4GB and cover all of
RAM, we won't have to use an swiotlb for devices that don't support 64-bit
DMA, which is a very significant performance difference.

	Arnd

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil.Edworthy]

Hi Arnd,

On: 26/03/2014 11:14, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 11:01:46 Phil.Edworthy@renesas.com wrote:
> > On: 26/03/2014 10:34, Arnd wrote:
> > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
nodes 
> > for R8A7790
> > > 
> > > On Wednesday 26 March 2014 09:55:04 Phil.Edworthy@renesas.com wrote:
> > > > Hi Arnd,
> > > > 
> > > > On: 25/03/2014 18:42, Arnd wrote:
> > > > > Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree 
> > nodes 
> > > > for R8A7790
> > > > > 
> > > > > On Tuesday 25 March 2014 16:56:41 Phil Edworthy wrote:
> > > > > > +               /* Map all possible DDR as inbound ranges */
> > > > > > +               dma-ranges = <0x42000000 0 0x40000000 0 
0x40000000 
> > 0 
> > > > 0x80000000
> > > > > > +                             0x43000000 1 0x80000000 1 
0x80000000 
> > 0 
> > > > 0x80000000>;
> > > > > 
> > > > > Typo: 0x43000000 should be 0x42000000 I guess.
> > > > I used 0x43000000 as this is a 64-bit type. The OF PCI range code 
> > > > currently treats both 32 and 64-bit types the same way, but I 
thought 
> > it 
> > > > would be good to set this in case we ever need to use it.
> > > 
> > > Ah, I forgot about the space identifier. It looks correct then, but
> > > it seems  a little strange to use a 32-bit identifier in one case
> > > and a 64-bit one in the other.
> > 
> > If the OF PCI range code allowed the PCIe host driver to determine if 
it's 
> > a 32-bit mapping, we could use that and get a small performance 
> > improvement with PCIe throughput.
> 
> I don't think it's supposed to care. Soem of the upper bits of the 
ranges
> only really make sense of PCI device registers, not for the top-level
> ranges property. The driver can however still look at the address itself
> to get that information.

Ah, yes that is a possibility.


> > > > Since the OF PCi range code treats both 32 and 64-bit types the 
same 
> > way, 
> > > > my PCIe driver only creates 64-bit mappings. In addition, the PCIe 

> > > > controller has to use a 64-bit mapping for anything over 2GiB. 
Based 
> > on 
> > > > this, I think it's sensible to leave the mappings as 1-to-1.
> > > 
> > > I'm not following, sorry. What is the hardware requirement in the
> > > controller?
> > With this controller, you can only specify maps whose size are a power 
of 
> > two, and the size must be less than or equal to the cpu address 
alignment. 
> > Further, when the size is 4GiB, you have to use a 64-bit mapping. 
Thinking 
> > about it, the 4GiB case is not relevant to our discussion about 32-bit 
vs 
> > 64-bit mappings.
> 
> But the ranges you specified in the property don't actually fit in those
> constraints: you have a range with size 0x8000000 and start 0x40000000,
> which you say can't be programmed into the hardware.

Actually, the driver checks the dma-ranges against these constraints, and 
if necessary will create multiple mappings to fulfil the requested 
dma-ranges.


> > Still, my comment about the OF PCI range code treating both 32 and 
64-bit 
> > types the same way means that PCIe host driver has to assume it's a 
64-bit 
> > mapping.
> 
> I was thinking more of PCI devices than the host itself. If the host
> driver can verify that all mappings are in the first 4GB and cover all 
of
> RAM, we won't have to use an swiotlb for devices that don't support 
64-bit
> DMA, which is a very significant performance difference.
Ok, I think I understand. However, all the other PCI host drivers just do 
1-to-1 mapping between PCI and CPU addresses, right? Whilst it might be 
nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
under 4GiB, can that be something to consider later on?

Thanks
Phil

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Arnd Bergmann]

On Wednesday 26 March 2014 11:34:41 Phil.Edworthy@renesas.com wrote:
> > But the ranges you specified in the property don't actually fit in those
> > constraints: you have a range with size 0x8000000 and start 0x40000000,
> > which you say can't be programmed into the hardware.
> 
> Actually, the driver checks the dma-ranges against these constraints, and 
> if necessary will create multiple mappings to fulfil the requested 
> dma-ranges.

Ok, I didn't notice. My initial suggestion was to not put that logic
into the driver but instead specify in the host bridge binding that each
entry in the dma-ranges property has to meet the hardware constraints.

As long as you don't have too much complexity to detect this case, I'm
fine with it either way.
 
> > > Still, my comment about the OF PCI range code treating both 32 and 
> 64-bit 
> > > types the same way means that PCIe host driver has to assume it's a 
> 64-bit 
> > > mapping.
> > 
> > I was thinking more of PCI devices than the host itself. If the host
> > driver can verify that all mappings are in the first 4GB and cover all 
> of
> > RAM, we won't have to use an swiotlb for devices that don't support 
> 64-bit
> > DMA, which is a very significant performance difference.
> Ok, I think I understand. However, all the other PCI host drivers just do 
> 1-to-1 mapping between PCI and CPU addresses, right? Whilst it might be 
> nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
> under 4GiB, can that be something to consider later on?

Yes, fair enough. The current version is much simpler, so that's ok.
Just keep it in mind if you run into performance problems. Also, note
that we don't actually support swiotlb on arm32 yet, so your current
code is broken for an PCI DMA master that is not 64-bit capable.
We need swiotlb on arm32 anyway, and that will fix this problem, but
adding the hack I described would also fix it.

	Arnd

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil.Edworthy]

Hi Arnd,

On: 26/03/2014 11:53, Arnd wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Wednesday 26 March 2014 11:34:41 Phil.Edworthy@renesas.com wrote:
> > > But the ranges you specified in the property don't actually fit in 
those
> > > constraints: you have a range with size 0x8000000 and start 
0x40000000,
> > > which you say can't be programmed into the hardware.
> > 
> > Actually, the driver checks the dma-ranges against these constraints, 
and 
> > if necessary will create multiple mappings to fulfil the requested 
> > dma-ranges.
> 
> Ok, I didn't notice. My initial suggestion was to not put that logic
> into the driver but instead specify in the host bridge binding that each
> entry in the dma-ranges property has to meet the hardware constraints.
> 
> As long as you don't have too much complexity to detect this case, I'm
> fine with it either way.
Ok, good!

> > > > Still, my comment about the OF PCI range code treating both 32 and 

> > 64-bit 
> > > > types the same way means that PCIe host driver has to assume it's 
a 
> > 64-bit 
> > > > mapping.
> > > 
> > > I was thinking more of PCI devices than the host itself. If the host
> > > driver can verify that all mappings are in the first 4GB and cover 
all 
> > of
> > > RAM, we won't have to use an swiotlb for devices that don't support 
> > 64-bit
> > > DMA, which is a very significant performance difference.
> > Ok, I think I understand. However, all the other PCI host drivers just 
do 
> > 1-to-1 mapping between PCI and CPU addresses, right? Whilst it might 
be 
> > nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
> > under 4GiB, can that be something to consider later on?
> 
> Yes, fair enough. The current version is much simpler, so that's ok.
> Just keep it in mind if you run into performance problems. Also, note
> that we don't actually support swiotlb on arm32 yet, so your current
> code is broken for an PCI DMA master that is not 64-bit capable.
> We need swiotlb on arm32 anyway, and that will fix this problem, but
> adding the hack I described would also fix it.
Ok, thanks for all your comments!

Phil

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Simon Horman]

On Tue, Mar 25, 2014 at 04:56:41PM +0000, Phil Edworthy wrote:
> This patch adds the device tree nodes for R8A7790
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Please split this patch into two patches.

1. A patch that updates arch/arm/boot/dts/r8a7790.dtsi

   I suggest calling that patch
   "ARM: shmobile: r8a7791: Add PCI device nodes"

2. A patch that updates arch/arm/boot/dts/r8a7790-lager.dts

   I suggest calling that patch
   "ARM: shmobile: lager: Add PCI device nodes"

> ---
> v5:
>  - Renesas SoCs compatible string has peripheral before device name
>  - Add PCIe bus clock reference
>  - Add additional interrupt bindings
>  - Use dma-ranges property to specify inbound memory regions
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts | 10 ++++++++++
>  arch/arm/boot/dts/r8a7790.dtsi      | 24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index a55c5f8..bbbcb63 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -139,6 +139,16 @@
>  		states = <3300000 1
>  			  1800000 0>;
>  	};
> +
> +	clocks {
> +		/* External PCIe bus clock - not used */
> +		pcie_bus_clk: pcie_bus_clk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <0>;
> +			clock-output-names = "pcie_bus";
> +		};
> +	};
>  };
>  
>  &extal_clk {
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index df9ec61..dbcea57 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -821,4 +821,28 @@
>  		#size-cells = <0>;
>  		status = "disabled";
>  	};
> +
> +	pcie: pcie@fe000000 {
> +		compatible = "renesas,pcie-r8a7790";
> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> +			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> +			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> +			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> +		/* Map all possible DDR as inbound ranges */
> +		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
> +			      0x43000000 1 0x80000000 1 0x80000000 0 0x80000000>;
> +		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
> +			      0 117 IRQ_TYPE_LEVEL_HIGH
> +			      0 118 IRQ_TYPE_LEVEL_HIGH>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7790_CLK_PCIE>, <&pcie_bus_clk>;
> +		clock-names = "pcie", "pcie_bus";
> +		status = "disabled";
> +	};
>  };
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes for R8A7790

[Phil.Edworthy]

Hi Simon,

On: 25/03/2014 21:03, Simon wrote:
> Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> On Tue, Mar 25, 2014 at 04:56:41PM +0000, Phil Edworthy wrote:
> > This patch adds the device tree nodes for R8A7790
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Please split this patch into two patches.
> 
> 1. A patch that updates arch/arm/boot/dts/r8a7790.dtsi
> 
>    I suggest calling that patch
>    "ARM: shmobile: r8a7791: Add PCI device nodes"
> 
> 2. A patch that updates arch/arm/boot/dts/r8a7790-lager.dts
> 
>    I suggest calling that patch
>    "ARM: shmobile: lager: Add PCI device nodes"
> 

For this, and your other comments, will do!

Thanks
Phil

[PATCH v5 7/9] ARM: shmobile: Add PCIe device tree nodes for R8A7791 Koelsch board

[Phil Edworthy]

This patch adds the device tree nodes for the R8A7791 Koelsch board

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v5:
 - Renesas SoCs compatible string has peripheral before device name
 - Add PCIe bus clock reference
 - Add additional interrupt bindings
 - Use dma-ranges property to specify inbound memory regions
---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 14 ++++++++++++++
 arch/arm/boot/dts/r8a7791.dtsi        | 24 ++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index ee23b7b..3130a0c 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -204,6 +204,16 @@
 		states = <3300000 1
 			  1800000 0>;
 	};
+
+	clocks {
+		/* External PCIe bus clock */
+		pcie_bus_clk: pcie_bus_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <100000000>;
+			clock-output-names = "pcie_bus";
+		};
+	};
 };
 
 &extal_clk {
@@ -382,3 +392,7 @@
 		spi-cpha;
 	};
 };
+
+&pcie {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index ccfba57..7a4be4c 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -836,4 +836,28 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	pcie: pcie@fe000000 {
+		compatible = "renesas,pcie-r8a7791";
+		reg = <0 0xfe000000 0 0x80000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
+			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
+			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
+			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
+		/* Map all possible DDR as inbound ranges */
+		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
+			      0x43000000 2 0x00000000 2 0x00000000 1 0x00000000>;
+		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
+			      0 117 IRQ_TYPE_LEVEL_HIGH
+			      0 118 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7791_CLK_PCIE>, <&pcie_bus_clk>;
+		clock-names = "pcie", "pcie_bus";
+		status = "disabled";
+	};
 };
-- 
1.9.0

Re: [PATCH v5 7/9] ARM: shmobile: Add PCIe device tree nodes for R8A7791 Koelsch board

[Simon Horman]

On Tue, Mar 25, 2014 at 04:56:42PM +0000, Phil Edworthy wrote:
> This patch adds the device tree nodes for the R8A7791 Koelsch board

Please split this patch into two patches.

1. A patch that updates arch/arm/boot/dts/r8a7791.dtsi

   I suggest calling that patch
   "ARM: shmobile: r8a7791: Add PCI device nodes"

2. A patch that updates arch/arm/boot/dts/r8a7791-koelsch.dts

   I suggest calling that patch
   "ARM: shmobile: koelsch: Add PCI device nodes"

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v5:
>  - Renesas SoCs compatible string has peripheral before device name
>  - Add PCIe bus clock reference
>  - Add additional interrupt bindings
>  - Use dma-ranges property to specify inbound memory regions
> ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 14 ++++++++++++++
>  arch/arm/boot/dts/r8a7791.dtsi        | 24 ++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index ee23b7b..3130a0c 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -204,6 +204,16 @@
>  		states = <3300000 1
>  			  1800000 0>;
>  	};
> +
> +	clocks {
> +		/* External PCIe bus clock */
> +		pcie_bus_clk: pcie_bus_clk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <100000000>;
> +			clock-output-names = "pcie_bus";
> +		};
> +	};
>  };
>  
>  &extal_clk {
> @@ -382,3 +392,7 @@
>  		spi-cpha;
>  	};
>  };
> +
> +&pcie {
> +	status = "okay";
> +};
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index ccfba57..7a4be4c 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -836,4 +836,28 @@
>  		#size-cells = <0>;
>  		status = "disabled";
>  	};
> +
> +	pcie: pcie@fe000000 {
> +		compatible = "renesas,pcie-r8a7791";
> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x01000000 0 0x00000000 0 0xfe100000 0 0x00100000
> +			  0x02000000 0 0xfe200000 0 0xfe200000 0 0x00200000
> +			  0x02000000 0 0x30000000 0 0x30000000 0 0x08000000
> +			  0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>;
> +		/* Map all possible DDR as inbound ranges */
> +		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000
> +			      0x43000000 2 0x00000000 2 0x00000000 1 0x00000000>;
> +		interrupts = <0 116 IRQ_TYPE_LEVEL_HIGH
> +			      0 117 IRQ_TYPE_LEVEL_HIGH
> +			      0 118 IRQ_TYPE_LEVEL_HIGH>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7791_CLK_PCIE>, <&pcie_bus_clk>;
> +		clock-names = "pcie", "pcie_bus";
> +		status = "disabled";
> +	};
>  };
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v5 8/9] ARM: koelsch: Add PCIe to defconfig

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 arch/arm/configs/koelsch_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/koelsch_defconfig b/arch/arm/configs/koelsch_defconfig
index 86faab5..ec14547 100644
--- a/arch/arm/configs/koelsch_defconfig
+++ b/arch/arm/configs/koelsch_defconfig
@@ -15,6 +15,9 @@ CONFIG_MACH_KOELSCH=y
 CONFIG_CPU_BPREDICT_DISABLE=y
 CONFIG_PL310_ERRATA_588369=y
 CONFIG_ARM_ERRATA_754322=y
+CONFIG_PCI=y
+CONFIG_PCI_MSI=y
+CONFIG_PCI_RCAR_GEN2_PCIE=y
 CONFIG_SMP=y
 CONFIG_SCHED_MC=y
 CONFIG_NR_CPUS=8
-- 
1.9.0

Re: [PATCH v5 8/9] ARM: koelsch: Add PCIe to defconfig

[Simon Horman]

Please update the subject of this patch to:

ARM: shmobile: koelsch: Add PCIe to defconfig

On Tue, Mar 25, 2014 at 04:56:43PM +0000, Phil Edworthy wrote:
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  arch/arm/configs/koelsch_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/configs/koelsch_defconfig b/arch/arm/configs/koelsch_defconfig
> index 86faab5..ec14547 100644
> --- a/arch/arm/configs/koelsch_defconfig
> +++ b/arch/arm/configs/koelsch_defconfig
> @@ -15,6 +15,9 @@ CONFIG_MACH_KOELSCH=y
>  CONFIG_CPU_BPREDICT_DISABLE=y
>  CONFIG_PL310_ERRATA_588369=y
>  CONFIG_ARM_ERRATA_754322=y
> +CONFIG_PCI=y
> +CONFIG_PCI_MSI=y
> +CONFIG_PCI_RCAR_GEN2_PCIE=y
>  CONFIG_SMP=y
>  CONFIG_SCHED_MC=y
>  CONFIG_NR_CPUS=8
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v5 9/9] ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

[Phil Edworthy]

This is fixes a boot problem when the R-Car PCIe driver is enabled.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 arch/arm/configs/koelsch_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/koelsch_defconfig b/arch/arm/configs/koelsch_defconfig
index ec14547..67666b6 100644
--- a/arch/arm/configs/koelsch_defconfig
+++ b/arch/arm/configs/koelsch_defconfig
@@ -15,6 +15,7 @@ CONFIG_MACH_KOELSCH=y
 CONFIG_CPU_BPREDICT_DISABLE=y
 CONFIG_PL310_ERRATA_588369=y
 CONFIG_ARM_ERRATA_754322=y
+CONFIG_HAVE_ARM_ARCH_TIMER=y
 CONFIG_PCI=y
 CONFIG_PCI_MSI=y
 CONFIG_PCI_RCAR_GEN2_PCIE=y
-- 
1.9.0

Re: [PATCH v5 9/9] ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

[Simon Horman]

Please update the subject of this patch to:

ARM: shmobile: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

On Tue, Mar 25, 2014 at 04:56:44PM +0000, Phil Edworthy wrote:
> This is fixes a boot problem when the R-Car PCIe driver is enabled.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  arch/arm/configs/koelsch_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/koelsch_defconfig b/arch/arm/configs/koelsch_defconfig
> index ec14547..67666b6 100644
> --- a/arch/arm/configs/koelsch_defconfig
> +++ b/arch/arm/configs/koelsch_defconfig
> @@ -15,6 +15,7 @@ CONFIG_MACH_KOELSCH=y
>  CONFIG_CPU_BPREDICT_DISABLE=y
>  CONFIG_PL310_ERRATA_588369=y
>  CONFIG_ARM_ERRATA_754322=y
> +CONFIG_HAVE_ARM_ARCH_TIMER=y
>  CONFIG_PCI=y
>  CONFIG_PCI_MSI=y
>  CONFIG_PCI_RCAR_GEN2_PCIE=y
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v5 9/9] ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

[Magnus Damm]

On Wed, Mar 26, 2014 at 1:56 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> This is fixes a boot problem when the R-Car PCIe driver is enabled.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  arch/arm/configs/koelsch_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/configs/koelsch_defconfig b/arch/arm/configs/koelsch_defconfig
> index ec14547..67666b6 100644
> --- a/arch/arm/configs/koelsch_defconfig
> +++ b/arch/arm/configs/koelsch_defconfig
> @@ -15,6 +15,7 @@ CONFIG_MACH_KOELSCH=y
>  CONFIG_CPU_BPREDICT_DISABLE=y
>  CONFIG_PL310_ERRATA_588369=y
>  CONFIG_ARM_ERRATA_754322=y
> +CONFIG_HAVE_ARM_ARCH_TIMER=y

Hi Phil,

Thanks for your patches! I appreciate that you send out a whole series
with broken out patches, that makes it easier to see what is going on.

However, regarding this particular patch I'm not so sure if your fix
is correct. The issue is most likely there, no doubt about that, but
this "select" does not match well with how we currently handle timers
for the rest of the SoCs in mach-shmobile. We for instance want to be
able to boot with CMT only.

So here's a gentle "NAK" on this particular patch. I think this shows
that either deferred probing or a different init ordering is needed
somehow.

Thanks,

/ magnus

Re: [PATCH v5 9/9] ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to defconfig

[Phil.Edworthy]

Hi Magnus,

On: 26/03/2014 05:39, Magnus wrote:
> Subject: Re: [PATCH v5 9/9] ARM: koelsch: Add HAVE_ARM_ARCH_TIMER to 
defconfig
> 
> On Wed, Mar 26, 2014 at 1:56 AM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > This is fixes a boot problem when the R-Car PCIe driver is enabled.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >  arch/arm/configs/koelsch_defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/configs/koelsch_defconfig 
b/arch/arm/configs/koelsch_defconfig
> > index ec14547..67666b6 100644
> > --- a/arch/arm/configs/koelsch_defconfig
> > +++ b/arch/arm/configs/koelsch_defconfig
> > @@ -15,6 +15,7 @@ CONFIG_MACH_KOELSCH=y
> >  CONFIG_CPU_BPREDICT_DISABLE=y
> >  CONFIG_PL310_ERRATA_588369=y
> >  CONFIG_ARM_ERRATA_754322=y
> > +CONFIG_HAVE_ARM_ARCH_TIMER=y
> 
> Hi Phil,
> 
> Thanks for your patches! I appreciate that you send out a whole series
> with broken out patches, that makes it easier to see what is going on.
> 
> However, regarding this particular patch I'm not so sure if your fix
> is correct. The issue is most likely there, no doubt about that, but
> this "select" does not match well with how we currently handle timers
> for the rest of the SoCs in mach-shmobile. We for instance want to be
> able to boot with CMT only.
> 
> So here's a gentle "NAK" on this particular patch. I think this shows
> that either deferred probing or a different init ordering is needed
> somehow.

When reading your comments, I realised that since I changed the driver to 
use module_platform_driver instead of subsys_initcall, we no longer need 
this patch.

Thanks
Phil