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

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

[Phil Edworthy]

This is version 4 of a PCIe Host driver for the R-Car Gen2 devices,
i.e. R-Car H2 (r8a7790) and R-Car M2 (r8a7791). This version includes
DT support, and includes a patch to add MSI.

v4:
 - Patch 0001 changed to correct use of runtime PM

Thanks
Phil

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 |  40 +
 arch/arm/boot/dts/r8a7790.dtsi                     |  26 +-
 arch/arm/boot/dts/r8a7791-koelsch.dts              |   4 +
 arch/arm/boot/dts/r8a7791.dtsi                     |  26 +-
 arch/arm/configs/koelsch_defconfig                 |   4 +
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-rcar.c                       | 831 +++++++++++++++++++++
 drivers/pci/host/pcie-rcar.h                       |  87 +++
 include/dt-bindings/clock/r8a7790-clock.h          |   1 +
 include/dt-bindings/clock/r8a7791-clock.h          |   1 +
 11 files changed, 1022 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 v4 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>
---
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 | 601 +++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pcie-rcar.h |  82 ++++++
 4 files changed, 690 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..16670e5
--- /dev/null
+++ b/drivers/pci/host/pcie-rcar.c
@@ -0,0 +1,601 @@
+/*
+ * 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"
+
+enum chip_id {
+	RCAR_GENERIC,
+	RCAR_H1,
+};
+
+#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
+
+static const struct of_device_id rcar_pcie_of_match[] = {
+	{ .compatible = "renesas,r8a7779-pcie", .data = (void *)RCAR_H1 },
+	{ .compatible = "renesas,r8a7790-pcie", .data = (void *)RCAR_GENERIC },
+	{ .compatible = "renesas,r8a7791-pcie", .data = (void *)RCAR_GENERIC },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rcar_pcie_of_match);
+
+/* Structure representing the PCIe interface */
+struct rcar_pcie {
+	struct device		*dev;
+	void __iomem		*base;
+	struct resource		res[PCI_MAX_RESOURCES];
+	int			haslink;
+	enum chip_id		chip;
+	u8			root_bus_nr;
+	struct clk		*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, ret;
+
+	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;
+
+		if (res->flags & IORESOURCE_IO) {
+			/* Setup IO mapped memory accesses */
+			ret = pci_ioremap_io(0, res->start);
+			if (ret)
+				return 1;
+
+			/* Correct addresses for remapped IO */
+			res->end = res->end - res->start;
+			res->start = 0;
+		}
+
+		rcar_pcie_setup_window(i, res, pcie);
+		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;
+
+	memset(&hw, 0, sizeof(hw));
+
+	hw.nr_controllers = 1;
+	hw.private_data   = (void **)&pcie;
+	hw.setup          = rcar_pcie_setup,
+	hw.map_irq        = of_irq_parse_and_map_pci,
+	hw.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_phy_init_rcar_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 0;
+
+		msleep(5);
+	}
+
+	return -ETIMEDOUT;
+}
+
+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 void __init rcar_pcie_hw_init(struct rcar_pcie *pcie)
+{
+	/* Initialise R-Car H1 PHY & wait for it to be ready */
+	if (pcie->chip == RCAR_H1)
+		if (rcar_pcie_phy_init_rcar_h1(pcie))
+			return;
+
+	/* Begin initialization */
+	pci_write_reg(pcie, 0, PCIETCTLR);
+
+	/* Set mode */
+	pci_write_reg(pcie, 1, PCIEMSR);
+
+	/*
+	 * For target transfers, setup a single 64-bit 4GB mapping at address
+	 * 0. The hardware can only map memory that starts on a power of two
+	 * boundary, and size is power of 2, so best to ignore it.
+	 */
+	pci_write_reg(pcie, 0, PCIEPRAR(0));
+	pci_write_reg(pcie, 0, PCIELAR(0));
+	pci_write_reg(pcie, 0xfffffff0UL | LAM_PREFETCH |
+		LAM_64BIT | LAR_ENABLE, PCIELAMR(0));
+	pci_write_reg(pcie, 0, PCIELAR(1));
+	pci_write_reg(pcie, 0, PCIEPRAR(1));
+	pci_write_reg(pcie, 0, PCIELAMR(1));
+
+	/*
+	 * 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. */
+	pcie->haslink = !rcar_pcie_wait_for_dl(pcie);
+
+	/* 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();
+}
+
+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 platfom clock\n");
+		return PTR_ERR(pcie->clk);
+	}
+
+	clk_prepare_enable(pcie->clk);
+
+	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->clk);
+
+	return err;
+}
+
+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;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->dev = &pdev->dev;
+	platform_set_drvdata(pdev, pcie);
+
+	of_id = of_match_device(rcar_pcie_of_match, pcie->dev);
+	if (of_id)
+		pcie->chip = (enum chip_id)of_id->data;
+
+	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;
+	}
+
+	rcar_pcie_hw_init(pcie);
+
+	if (!pcie->haslink) {
+		dev_info(&pdev->dev, "PCI: 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,
+	},
+};
+
+static int __init pcie_init(void)
+{
+	return platform_driver_probe(&rcar_pcie_driver, rcar_pcie_probe);
+}
+subsys_initcall(pcie_init);
+
+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 v4 1/9] PCI: host: rcar: Add Renesas R-Car PCIe driver

[Arnd Bergmann]

On Friday 21 March 2014 10:32:40 Phil Edworthy wrote:
> +static const struct of_device_id rcar_pcie_of_match[] = {
> +	{ .compatible = "renesas,r8a7779-pcie", .data = (void *)RCAR_H1 },
> +	{ .compatible = "renesas,r8a7790-pcie", .data = (void *)RCAR_GENERIC },
> +	{ .compatible = "renesas,r8a7791-pcie", .data = (void *)RCAR_GENERIC },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rcar_pcie_of_match);

The only difference between RCAR_H1 and RCAR_GENERIC seems to be the
use of the rcar_pcie_phy_init_rcar_h1 vs rcar_pcie_hw_init
functions. I think this would be expressed in a nicer way doing

static const struct of_device_id rcar_pcie_of_match[] = {
	{ .compatible = "renesas,r8a7779-pcie", .data =  rcar_pcie_phy_init_rcar_h1},
	{ .compatible = "renesas,r8a7790-pcie", .data =  rcar_pcie_hw_init},
	{ .compatible = "renesas,r8a7791-pcie", .data =  rcar_pcie_hw_init},
	{},
};

If you need other differences in the future, you can extend this
to use a pointer to a struct containing the init function pointer.

> +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;
> +}

Would it be possible to have this done in the boot loader so the kernel
doesn't need to be bothered with it?

> +
> +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, ret;
> +
> +	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;
> +
> +		if (res->flags & IORESOURCE_IO) {
> +			/* Setup IO mapped memory accesses */
> +			ret = pci_ioremap_io(0, res->start);
> +			if (ret)
> +				return 1;
> +
> +			/* Correct addresses for remapped IO */
> +			res->end = res->end - res->start;
> +			res->start = 0;
> +		}
> +
> +		rcar_pcie_setup_window(i, res, pcie);
> +		pci_add_resource(&sys->resources, res);
> +	}

This assumes that the start of the I/O window is always at
bus address 0, and that you have only one PCI host in the system.
Are both guaranteed through the hardware design?

> +static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
> +{
> +	struct platform_device *pdev = to_platform_device(pcie->dev);
> +	struct hw_pci hw;
> +
> +	memset(&hw, 0, sizeof(hw));
> +
> +	hw.nr_controllers = 1;
> +	hw.private_data   = (void **)&pcie;
> +	hw.setup          = rcar_pcie_setup,
> +	hw.map_irq        = of_irq_parse_and_map_pci,
> +	hw.ops		  = &rcar_pcie_ops,

You can write this slightly nicer doing

	struct hw_pci hw = {
		.nr_controllers = 1,
		.private_data = (void **)&pcie,
		...
	};
> +static int __init rcar_pcie_phy_init_rcar_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);

Have you considered moving this into a separate PHY driver?
There are probably good reasons either way, so I'm not asking you
to do it, just to think about what would be best for this driver.

It seems that you already have two different implementations
that only vary in how they access the PHY, so moving that
into a separate driver would keep the knowledge out of the
host driver. OTOH, the PHY registers look like they are part of
the pci host itself.

> +	/*
> +	 * For target transfers, setup a single 64-bit 4GB mapping at address
> +	 * 0. The hardware can only map memory that starts on a power of two
> +	 * boundary, and size is power of 2, so best to ignore it.
> +	 */
> +	pci_write_reg(pcie, 0, PCIEPRAR(0));
> +	pci_write_reg(pcie, 0, PCIELAR(0));
> +	pci_write_reg(pcie, 0xfffffff0UL | LAM_PREFETCH |
> +		LAM_64BIT | LAR_ENABLE, PCIELAMR(0));
> +	pci_write_reg(pcie, 0, PCIELAR(1));
> +	pci_write_reg(pcie, 0, PCIEPRAR(1));
> +	pci_write_reg(pcie, 0, PCIELAMR(1));

Is this the only reasonable configuration? If not, you might want to
do the same thing as the x-gene PCI driver that is not yet merged,
and parse the 'dma-ranges' property to determine what the mapping
on a particular machine should be.

Most importantly, why don't you use a mapping that includes RAM beyond
the first 4GB?

> +static int __init pcie_init(void)
> +{
> +	return platform_driver_probe(&rcar_pcie_driver, rcar_pcie_probe);
> +}
> +subsys_initcall(pcie_init);

Why so early?

Overall, this driver looks pretty good.

	Arnd

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

[Phil.Edworthy at renesas.com]

Hi Arnd,

Thanks for the review.

> From: Arnd Bergmann <arnd@arndb.de>
> To: linux-arm-kernel at lists.infradead.org, 
> Cc: Phil Edworthy <phil.edworthy@renesas.com>, 
linux-pci at vger.kernel.org, 
> linux-sh at vger.kernel.org, Magnus Damm <magnus.damm@gmail.com>, Valentine 

> Barshak <valentine.barshak@cogentembedded.com>, Simon Horman 
> <horms@verge.net.au>, Bjorn Helgaas <bhelgaas@google.com>, Ben Dooks 
> <ben.dooks@codethink.co.uk>
> Date: 21/03/2014 11:04
> Subject: Re: [PATCH v4 1/9] PCI: host: rcar: Add Renesas R-Car PCIe 
driver
> 
> On Friday 21 March 2014 10:32:40 Phil Edworthy wrote:
> > +static const struct of_device_id rcar_pcie_of_match[] = {
> > +   { .compatible = "renesas,r8a7779-pcie", .data = (void *)RCAR_H1 },
> > +   { .compatible = "renesas,r8a7790-pcie", .data = (void 
*)RCAR_GENERIC },
> > +   { .compatible = "renesas,r8a7791-pcie", .data = (void 
*)RCAR_GENERIC },
> > +   {},
> > +};
> > +MODULE_DEVICE_TABLE(of, rcar_pcie_of_match);
> 
> The only difference between RCAR_H1 and RCAR_GENERIC seems to be the
> use of the rcar_pcie_phy_init_rcar_h1 vs rcar_pcie_hw_init
> functions. I think this would be expressed in a nicer way doing
> 
> static const struct of_device_id rcar_pcie_of_match[] = {
>    { .compatible = "renesas,r8a7779-pcie", .data = 
rcar_pcie_phy_init_rcar_h1},
>    { .compatible = "renesas,r8a7790-pcie", .data =  rcar_pcie_hw_init},
>    { .compatible = "renesas,r8a7791-pcie", .data =  rcar_pcie_hw_init},
>    {},
> };
> 
> If you need other differences in the future, you can extend this
> to use a pointer to a struct containing the init function pointer.
Ok, I understand what you mean, though the rcar_pcie_phy_init_rcar_h1 
function is an extra call for r8a7779 devices to setup the PHY, not an 
alternative to rcar_pcie_hw_init.


> > +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;
> > +}
> 
> Would it be possible to have this done in the boot loader so the kernel
> doesn't need to be bothered with it?
Hmm, I don't like the idea of depending on settings made by a bootloader, 
when it may be that a different bootloader could be used, or no bootloader 
at all.


> > +
> > +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, ret;
> > +
> > +   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;
> > +
> > +      if (res->flags & IORESOURCE_IO) {
> > +         /* Setup IO mapped memory accesses */
> > +         ret = pci_ioremap_io(0, res->start);
> > +         if (ret)
> > +            return 1;
> > +
> > +         /* Correct addresses for remapped IO */
> > +         res->end = res->end - res->start;
> > +         res->start = 0;
> > +      }
> > +
> > +      rcar_pcie_setup_window(i, res, pcie);
> > +      pci_add_resource(&sys->resources, res);
> > +   }
> 
> This assumes that the start of the I/O window is always at
> bus address 0, and that you have only one PCI host in the system.
> Are both guaranteed through the hardware design?
The I/O window can be anywhere I believe, so I guess I should pull the 
actual bus address from the DT.
All existing devices I have seen, have a single PCIe host.


> > +static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
> > +{
> > +   struct platform_device *pdev = to_platform_device(pcie->dev);
> > +   struct hw_pci hw;
> > +
> > +   memset(&hw, 0, sizeof(hw));
> > +
> > +   hw.nr_controllers = 1;
> > +   hw.private_data   = (void **)&pcie;
> > +   hw.setup          = rcar_pcie_setup,
> > +   hw.map_irq        = of_irq_parse_and_map_pci,
> > +   hw.ops        = &rcar_pcie_ops,
> 
> You can write this slightly nicer doing
> 
>    struct hw_pci hw = {
>       .nr_controllers = 1,
>       .private_data = (void **)&pcie,
>       ...
>    };
Ok, will do.


> > +static int __init rcar_pcie_phy_init_rcar_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);
> 
> Have you considered moving this into a separate PHY driver?
> There are probably good reasons either way, so I'm not asking you
> to do it, just to think about what would be best for this driver.
> 
> It seems that you already have two different implementations
> that only vary in how they access the PHY, so moving that
> into a separate driver would keep the knowledge out of the
> host driver. OTOH, the PHY registers look like they are part of
> the pci host itself.
The PHYs are separate entities, but access to them is via the PCIe 
controller's registers. In the general case, there shouldn't be any PHY 
setup to do, it's just the R-Car H1 devices that needs a little tweaking.


> > +   /*
> > +    * For target transfers, setup a single 64-bit 4GB mapping at 
address
> > +    * 0. The hardware can only map memory that starts on a power of 
two
> > +    * boundary, and size is power of 2, so best to ignore it.
> > +    */
> > +   pci_write_reg(pcie, 0, PCIEPRAR(0));
> > +   pci_write_reg(pcie, 0, PCIELAR(0));
> > +   pci_write_reg(pcie, 0xfffffff0UL | LAM_PREFETCH |
> > +      LAM_64BIT | LAR_ENABLE, PCIELAMR(0));
> > +   pci_write_reg(pcie, 0, PCIELAR(1));
> > +   pci_write_reg(pcie, 0, PCIEPRAR(1));
> > +   pci_write_reg(pcie, 0, PCIELAMR(1));
> 
> Is this the only reasonable configuration? If not, you might want to
> do the same thing as the x-gene PCI driver that is not yet merged,
> and parse the 'dma-ranges' property to determine what the mapping
> on a particular machine should be.
Based on the restrictions of these registers, it seemed like a sensible 
approach to open up the whole 32-bit address space. I'll have a look at 
the x-gene patches to see if that would be better.

> Most importantly, why don't you use a mapping that includes RAM beyond
> the first 4GB?
I had intended to send a patch later on to open this up to the first 12GB. 
Limitations of this block of hardware mean that we can do at most three 
4GB regions. The first 12GB would be ok for LPAE on existing devices as 
all DDR is mapped within that range, but obviously not ideal.


> > +static int __init pcie_init(void)
> > +{
> > +   return platform_driver_probe(&rcar_pcie_driver, rcar_pcie_probe);
> > +}
> > +subsys_initcall(pcie_init);
> 
> Why so early?
Good catch, copied from another PCIe driver!

> Overall, this driver looks pretty good.
> 
>    Arnd

Thanks
Phil

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

[Ben Dooks]

On 21/03/14 14:59, Phil.Edworthy at renesas.com wrote:
> Hi Arnd,
>
> Thanks for the review.
>
>
>>> +static int __init pcie_init(void)
>>> +{
>>> +   return platform_driver_probe(&rcar_pcie_driver, rcar_pcie_probe);
>>> +}
>>> +subsys_initcall(pcie_init);
>>
>> Why so early?
> Good catch, copied from another PCIe driver!

I think you can change it to use module_driver() instead of the
init_call.


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

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

[Arnd Bergmann]

On Friday 21 March 2014 13:59:35 Phil.Edworthy at renesas.com wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > static const struct of_device_id rcar_pcie_of_match[] = {
> >    { .compatible = "renesas,r8a7779-pcie", .data = 
> rcar_pcie_phy_init_rcar_h1},
> >    { .compatible = "renesas,r8a7790-pcie", .data =  rcar_pcie_hw_init},
> >    { .compatible = "renesas,r8a7791-pcie", .data =  rcar_pcie_hw_init},
> >    {},
> > };
> > 
> > If you need other differences in the future, you can extend this
> > to use a pointer to a struct containing the init function pointer.
>
> Ok, I understand what you mean, though the rcar_pcie_phy_init_rcar_h1 
> function is an extra call for r8a7779 devices to setup the PHY, not an 
> alternative to rcar_pcie_hw_init.

I assumed that would be trivial to change, by calling rcar_pcie_hw_init
at the end of rcar_pcie_phy_init_rcar_h1 rather than the other way round.

> > > +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;
> > > +}
> > 
> > Would it be possible to have this done in the boot loader so the kernel
> > doesn't need to be bothered with it?
>
> Hmm, I don't like the idea of depending on settings made by a bootloader, 
> when it may be that a different bootloader could be used, or no bootloader 
> at all.

If you need to support the case without a boot loader, that would be
a good reason to leave the code in.

Otherwise, I think it's better to mandate early that the boot loader
sets up a lot of the basic stuff. As long as nobody has a kernel that
does the same setup, you also won't see any boot loaders that get it
wrong, because they never work. It's also easy enough to add back
if you later have to support a broken boot loader.

> > > +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, ret;
> > > +
> > > +   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;
> > > +
> > > +      if (res->flags & IORESOURCE_IO) {
> > > +         /* Setup IO mapped memory accesses */
> > > +         ret = pci_ioremap_io(0, res->start);
> > > +         if (ret)
> > > +            return 1;
> > > +
> > > +         /* Correct addresses for remapped IO */
> > > +         res->end = res->end - res->start;
> > > +         res->start = 0;
> > > +      }
> > > +
> > > +      rcar_pcie_setup_window(i, res, pcie);
> > > +      pci_add_resource(&sys->resources, res);
> > > +   }
> > 
> > This assumes that the start of the I/O window is always at
> > bus address 0, and that you have only one PCI host in the system.
> > Are both guaranteed through the hardware design?
>
> The I/O window can be anywhere I believe, so I guess I should pull the 
> actual bus address from the DT.

Ok. There is actually work on the way to simplify this a lot
so you don't have to do it in the driver, but it will probably
take a little more time, especially since Will Deacon is on an
extended vacation at the moment and he was driving things.

> All existing devices I have seen, have a single PCIe host.

The seems less obvious to rely on though, it's quite common
for newer hardware to have multiple blocks of whatever the first
generation has only one of ;-)

> > > +static int __init rcar_pcie_phy_init_rcar_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);
> > 
> > Have you considered moving this into a separate PHY driver?
> > There are probably good reasons either way, so I'm not asking you
> > to do it, just to think about what would be best for this driver.
> > 
> > It seems that you already have two different implementations
> > that only vary in how they access the PHY, so moving that
> > into a separate driver would keep the knowledge out of the
> > host driver. OTOH, the PHY registers look like they are part of
> > the pci host itself.
>
> The PHYs are separate entities, but access to them is via the PCIe 
> controller's registers. In the general case, there shouldn't be any PHY 
> setup to do, it's just the R-Car H1 devices that needs a little tweaking.

Ok.


> > Most importantly, why don't you use a mapping that includes RAM beyond
> > the first 4GB?
> I had intended to send a patch later on to open this up to the first 12GB. 
> Limitations of this block of hardware mean that we can do at most three 
> 4GB regions. The first 12GB would be ok for LPAE on existing devices as 
> all DDR is mapped within that range, but obviously not ideal.

Ok, in this case it sounds like you should definitely use the dma-ranges
property, since it's possibly that board designers put their ram in funny
places. You probably want to have a dma-ranges property with three 4GB
entries then and document the size limitation in the pci host binding,
rather than a single entry spanning all of RAM.

	Arnd

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

[Geert Uytterhoeven]

On Fri, Mar 21, 2014 at 4:07 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 21 March 2014 13:59:35 Phil.Edworthy at renesas.com wrote:
>> > From: Arnd Bergmann <arnd@arndb.de>
>> > static const struct of_device_id rcar_pcie_of_match[] = {
>> >    { .compatible = "renesas,r8a7779-pcie", .data =
>> rcar_pcie_phy_init_rcar_h1},
>> >    { .compatible = "renesas,r8a7790-pcie", .data =  rcar_pcie_hw_init},
>> >    { .compatible = "renesas,r8a7791-pcie", .data =  rcar_pcie_hw_init},
>> >    {},
>> > };
>> >
>> > If you need other differences in the future, you can extend this
>> > to use a pointer to a struct containing the init function pointer.
>>
>> Ok, I understand what you mean, though the rcar_pcie_phy_init_rcar_h1
>> function is an extra call for r8a7779 devices to setup the PHY, not an
>> alternative to rcar_pcie_hw_init.
>
> I assumed that would be trivial to change, by calling rcar_pcie_hw_init
> at the end of rcar_pcie_phy_init_rcar_h1 rather than the other way round.

And this would allow to propagate the error code from
rcar_pcie_phy_init_rcar_h1(), as currently rcar_pcie_hw_init() ignores it,
and returns void.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/pci/host/pcie-rcar.c | 232 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/host/pcie-rcar.h |   5 +
 2 files changed, 236 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 16670e5..cbbcd77 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>
@@ -33,6 +36,8 @@ enum chip_id {
 	RCAR_H1,
 };
 
+#define INT_PCI_MSI_NR	32
+
 #define RCONF(x)	(PCICONF(0)+(x))
 #define RPMCAP(x)	(PMCAP(0)+(x))
 #define REXPCAP(x)	(EXPCAP(0)+(x))
@@ -52,6 +57,20 @@ static const struct of_device_id rcar_pcie_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rcar_pcie_of_match);
 
+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 irq;
+};
+
+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;
@@ -61,6 +80,7 @@ struct rcar_pcie {
 	enum chip_id		chip;
 	u8			root_bus_nr;
 	struct clk		*clk;
+	struct			rcar_msi msi;
 };
 
 static inline struct rcar_pcie *sys_to_pcie(struct pci_sys_data *sys)
@@ -312,6 +332,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);
@@ -324,6 +353,7 @@ static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
 	hw.setup          = rcar_pcie_setup,
 	hw.map_irq        = of_irq_parse_and_map_pci,
 	hw.ops		  = &rcar_pcie_ops,
+	hw.add_bus	  = rcar_pcie_add_bus;
 
 	pci_common_init_dev(&pdev->dev, &hw);
 }
@@ -478,6 +508,10 @@ static void __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);
 
@@ -495,11 +529,190 @@ static void __init rcar_pcie_hw_init(struct rcar_pcie *pcie)
 	wmb();
 }
 
+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);
+
+	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->irq, 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->irq+1, 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)
@@ -511,6 +724,13 @@ static int __init rcar_pcie_get_resources(struct platform_device *pdev,
 		return PTR_ERR(pcie->clk);
 	}
 
+	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");
+		return -ENOENT;
+	}
+	pcie->msi.irq = i;
+
 	clk_prepare_enable(pcie->clk);
 
 	pcie->base = devm_ioremap_resource(&pdev->dev, &res);
@@ -566,6 +786,16 @@ static int __init rcar_pcie_probe(struct platform_device *pdev)
 			break;
 	}
 
+	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;
+		}
+	}
+
 	rcar_pcie_hw_init(pcie);
 
 	if (!pcie->haslink) {
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

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

[Lucas Stach]

Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  drivers/pci/host/pcie-rcar.c | 232 ++++++++++++++++++++++++++++++++++++++++++-
>  drivers/pci/host/pcie-rcar.h |   5 +
>  2 files changed, 236 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 16670e5..cbbcd77 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
[...]
> +
> +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);
> +
> +	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;
> +}
> +
>From your DT binding it seems you have only one interrupt from the PCIe
core, shared between the MSI irqs and the PCI legacy interrupts.
This means this handler may get called without an MSI irq pending, so
this function really should have a path where it's returning IRQ_NONE.

Regards,
Lucas

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Phil.Edworthy at renesas.com]

Hi Lucas,

Thanks for the review.

On 21/03/2014 11:18, Lucas wrote:
> Subject: Re: [PATCH v4 2/9] PCI: host: rcar: Add MSI support
> 
> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >  drivers/pci/host/pcie-rcar.c | 232 
++++++++++++++++++++++++++++++++++++++++++-
> >  drivers/pci/host/pcie-rcar.h |   5 +
> >  2 files changed, 236 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pcie-rcar.c 
b/drivers/pci/host/pcie-rcar.c
> > index 16670e5..cbbcd77 100644
> > --- a/drivers/pci/host/pcie-rcar.c
> > +++ b/drivers/pci/host/pcie-rcar.c
> [...]
> > +
> > +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);
> > +
> > +   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;
> > +}
> > +
> From your DT binding it seems you have only one interrupt from the PCIe
> core, shared between the MSI irqs and the PCI legacy interrupts.
> This means this handler may get called without an MSI irq pending, so
> this function really should have a path where it's returning IRQ_NONE.
Ah, yes you are right... though actually there are two interrupts, one has 
some MSI and INTx, the other has the rest of the MSIs.

> Regards,
> Lucas
> 
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil

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

[Lucas Stach]

Am Freitag, den 21.03.2014, 14:15 +0000 schrieb
Phil.Edworthy at renesas.com:
> Hi Lucas,
> 
> Thanks for the review.
> 
> On 21/03/2014 11:18, Lucas wrote:
> > Subject: Re: [PATCH v4 2/9] PCI: host: rcar: Add MSI support
> > 
> > Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > ---
> > >  drivers/pci/host/pcie-rcar.c | 232 
> ++++++++++++++++++++++++++++++++++++++++++-
> > >  drivers/pci/host/pcie-rcar.h |   5 +
> > >  2 files changed, 236 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/host/pcie-rcar.c 
> b/drivers/pci/host/pcie-rcar.c
> > > index 16670e5..cbbcd77 100644
> > > --- a/drivers/pci/host/pcie-rcar.c
> > > +++ b/drivers/pci/host/pcie-rcar.c
> > [...]
> > > +
> > > +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);
> > > +
> > > +   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;
> > > +}
> > > +
> > From your DT binding it seems you have only one interrupt from the PCIe
> > core, shared between the MSI irqs and the PCI legacy interrupts.
> > This means this handler may get called without an MSI irq pending, so
> > this function really should have a path where it's returning IRQ_NONE.
> Ah, yes you are right... though actually there are two interrupts, one has 
> some MSI and INTx, the other has the rest of the MSIs.
> 
This isn't reflected in the DT binding. If you already know that there
may be more than a single interrupt for MSI, please push this into the
binding by using named interrupts. Even if your driver doesn't support
it yet, additional functionality can always be added later, but the DT
should be a stable ABI describing your hardware.

So if you already know your hardware has more than one interrupt, please
put it in the binding, to avoid churn later on.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Phil.Edworthy at renesas.com]

Hi Lucas,

On 21/03/2014 14:27, Lucas wrote:
> Subject: Re: [PATCH v4 2/9] PCI: host: rcar: Add MSI support
> 
> Am Freitag, den 21.03.2014, 14:15 +0000 schrieb
> Phil.Edworthy at renesas.com:
> > Hi Lucas,
> > 
> > Thanks for the review.
> > 
> > On 21/03/2014 11:18, Lucas wrote:
> > > Subject: Re: [PATCH v4 2/9] PCI: host: rcar: Add MSI support
> > > 
> > > Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > ---
> > > >  drivers/pci/host/pcie-rcar.c | 232 
> > ++++++++++++++++++++++++++++++++++++++++++-
> > > >  drivers/pci/host/pcie-rcar.h |   5 +
> > > >  2 files changed, 236 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/host/pcie-rcar.c 
> > b/drivers/pci/host/pcie-rcar.c
> > > > index 16670e5..cbbcd77 100644
> > > > --- a/drivers/pci/host/pcie-rcar.c
> > > > +++ b/drivers/pci/host/pcie-rcar.c
> > > [...]
> > > > +
> > > > +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);
> > > > +
> > > > +   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;
> > > > +}
> > > > +
> > > From your DT binding it seems you have only one interrupt from the 
PCIe
> > > core, shared between the MSI irqs and the PCI legacy interrupts.
> > > This means this handler may get called without an MSI irq pending, 
so
> > > this function really should have a path where it's returning 
IRQ_NONE.
> > Ah, yes you are right... though actually there are two interrupts, one 
has 
> > some MSI and INTx, the other has the rest of the MSIs.
> > 
> This isn't reflected in the DT binding. If you already know that there
> may be more than a single interrupt for MSI, please push this into the
> binding by using named interrupts. Even if your driver doesn't support
> it yet, additional functionality can always be added later, but the DT
> should be a stable ABI describing your hardware.
> 
> So if you already know your hardware has more than one interrupt, please
> put it in the binding, to avoid churn later on.
At the moment we support both interrupts but the code implicitly gets the 
'next' interrupt for MSI. I'll change that so that all the interrupts are 
specified in the binding.


> Regards,
> Lucas
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil

[PATCH v4 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 at 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 v4 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 at 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 v4 5/9] dt-bindings: pci: rcar pcie device tree bindings

[Phil Edworthy]

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

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 ++++++++++++++++++++++
 1 file changed, 40 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..0e219b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
@@ -0,0 +1,40 @@
+* Renesas RCar PCIe interface
+
+Required properties:
+- compatible: should contain one of the following
+	"renesas,r8a7779-pcie", "renesas,r8a7790-pcie", "renesas,r8a7791-pcie"
+- 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
+- interrupts: interrupt values for MSI interrupt
+- #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"
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie: pcie at 0x01000000 {
+		compatible = "renesas,r8a7790-pcie";
+		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>;
+		interrupts = <0 116 4>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic 0 116 4>;
+		clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
+		clock-names = "pcie";
+		status = "disabled";
+	};
-- 
1.9.0

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

[Lucas Stach]

Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> This patch adds the bindings for the rcar PCIE driver. The driver
> resides under drivers/pci/host/pcie-rcar.c
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

I have one question below. If you can answer this with yes after
thinking again about it, this is:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 ++++++++++++++++++++++
>  1 file changed, 40 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..0e219b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> @@ -0,0 +1,40 @@
> +* Renesas RCar PCIe interface
> +
> +Required properties:
> +- compatible: should contain one of the following
> +	"renesas,r8a7779-pcie", "renesas,r8a7790-pcie", "renesas,r8a7791-pcie"
> +- 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
> +- interrupts: interrupt values for MSI interrupt
> +- #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"

You really only have one PCIe clock? So the controller is generating the
PCIe bus clock from it's register clock? No need to enable any other
clock for PCIe to work?

> +
> +Example:
> +
> +SoC specific DT Entry:
> +
> +	pcie: pcie at 0x01000000 {
> +		compatible = "renesas,r8a7790-pcie";
> +		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>;
> +		interrupts = <0 116 4>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0 0 0 0>;
> +		interrupt-map = <0 0 0 0 &gic 0 116 4>;
> +		clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
> +		clock-names = "pcie";
> +		status = "disabled";
> +	};

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Phil.Edworthy at renesas.com]

Hi Lucas,

On 21/03/2014 11:24, Lucas wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > This patch adds the bindings for the rcar PCIE driver. The driver
> > resides under drivers/pci/host/pcie-rcar.c
> > 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> I have one question below. If you can answer this with yes after
> thinking again about it, this is:
> 
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 
++++++++++++++++++++++
> >  1 file changed, 40 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..0e219b0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > @@ -0,0 +1,40 @@
> > +* Renesas RCar PCIe interface
> > +
> > +Required properties:
> > +- compatible: should contain one of the following
> > +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie", 
"renesas,r8a7791-pcie"
> > +- 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
> > +- interrupts: interrupt values for MSI interrupt
> > +- #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"
> 
> You really only have one PCIe clock? So the controller is generating the
> PCIe bus clock from it's register clock? No need to enable any other
> clock for PCIe to work?
Yes, just the one clock. The PCIe bus clock is an external clock dedicated 
to PCIe and SATA, and we have no control over it.


> > +
> > +Example:
> > +
> > +SoC specific DT Entry:
> > +
> > +   pcie: pcie at 0x01000000 {
> > +      compatible = "renesas,r8a7790-pcie";
> > +      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>;
> > +      interrupts = <0 116 4>;
> > +      #interrupt-cells = <1>;
> > +      interrupt-map-mask = <0 0 0 0>;
> > +      interrupt-map = <0 0 0 0 &gic 0 116 4>;
> > +      clocks = <&mstp3_clks R8A7790_CLK_PCIE>;
> > +      clock-names = "pcie";
> > +      status = "disabled";
> > +   };
> 
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil

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

[Lucas Stach]

Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
Phil.Edworthy at renesas.com:
> Hi Lucas,
> 
> On 21/03/2014 11:24, Lucas wrote:
> > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
> bindings
> > 
> > Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > This patch adds the bindings for the rcar PCIE driver. The driver
> > > resides under drivers/pci/host/pcie-rcar.c
> > > 
> > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > 
> > I have one question below. If you can answer this with yes after
> > thinking again about it, this is:
> > 
> > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > >  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 
> ++++++++++++++++++++++
> > >  1 file changed, 40 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..0e219b0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > > @@ -0,0 +1,40 @@
> > > +* Renesas RCar PCIe interface
> > > +
> > > +Required properties:
> > > +- compatible: should contain one of the following
> > > +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie", 
> "renesas,r8a7791-pcie"
> > > +- 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
> > > +- interrupts: interrupt values for MSI interrupt
> > > +- #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"
> > 
> > You really only have one PCIe clock? So the controller is generating the
> > PCIe bus clock from it's register clock? No need to enable any other
> > clock for PCIe to work?
> Yes, just the one clock. The PCIe bus clock is an external clock dedicated 
> to PCIe and SATA, and we have no control over it.
> 
Is this some kind of always-on clock? If not you probably want a
reference to it from the PCIe driver even if it's shared between SATA
and PCIe. After all you don't want to end up unconditionally enabling
this clock from the board or SoC level just to have PCIe working, right?

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

[Phil.Edworthy at renesas.com]

Hi Lucas,

On: 21/03/2014 14:31, Lucas wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> Phil.Edworthy at renesas.com:
> > Hi Lucas,
> > 
> > On 21/03/2014 11:24, Lucas wrote:
> > > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
> > bindings
> > > 
> > > Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > > This patch adds the bindings for the rcar PCIE driver. The driver
> > > > resides under drivers/pci/host/pcie-rcar.c
> > > > 
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > 
> > > I have one question below. If you can answer this with yes after
> > > thinking again about it, this is:
> > > 
> > > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 
> > ++++++++++++++++++++++
> > > >  1 file changed, 40 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..0e219b0
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > > > @@ -0,0 +1,40 @@
> > > > +* Renesas RCar PCIe interface
> > > > +
> > > > +Required properties:
> > > > +- compatible: should contain one of the following
> > > > +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie", 
> > "renesas,r8a7791-pcie"
> > > > +- 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
> > > > +- interrupts: interrupt values for MSI interrupt
> > > > +- #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"
> > > 
> > > You really only have one PCIe clock? So the controller is generating 
the
> > > PCIe bus clock from it's register clock? No need to enable any other
> > > clock for PCIe to work?
> > Yes, just the one clock. The PCIe bus clock is an external clock 
dedicated 
> > to PCIe and SATA, and we have no control over it.
> > 
> Is this some kind of always-on clock? If not you probably want a
> reference to it from the PCIe driver even if it's shared between SATA
> and PCIe. After all you don't want to end up unconditionally enabling
> this clock from the board or SoC level just to have PCIe working, right?
Ok, I see, I'll need a reference to a board clock, even if it's always on 
for these boards. 

> Regards,
> Lucas
> -- 
> Pengutronix e.K.                           | Lucas Stach |
> Industrial Linux Solutions                 | http://www.pengutronix.de/ 
|
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 
|
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 
|
> 

Thanks
Phil

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

[Phil.Edworthy at renesas.com]

Hi Lucas,

On: 21/03/2014 15:02, Phil wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> Hi Lucas,
> 
> On: 21/03/2014 14:31, Lucas wrote:
> > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> > 
> > Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> > Phil.Edworthy at renesas.com:
> > > Hi Lucas,
> > > 
> > > On 21/03/2014 11:24, Lucas wrote:
> > > > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device 
tree 
> > > bindings
> > > > 
> > > > Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > > > This patch adds the bindings for the rcar PCIE driver. The 
driver
> > > > > resides under drivers/pci/host/pcie-rcar.c
> > > > > 
> > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > 
> > > > I have one question below. If you can answer this with yes after
> > > > thinking again about it, this is:
> > > > 
> > > > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/pci/rcar-pci.txt | 40 
> > > ++++++++++++++++++++++
> > > > >  1 file changed, 40 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..0e219b0
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > > > > @@ -0,0 +1,40 @@
> > > > > +* Renesas RCar PCIe interface
> > > > > +
> > > > > +Required properties:
> > > > > +- compatible: should contain one of the following
> > > > > +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie", 
> > > "renesas,r8a7791-pcie"
> > > > > +- 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
> > > > > +- interrupts: interrupt values for MSI interrupt
> > > > > +- #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"
> > > > 
> > > > You really only have one PCIe clock? So the controller is 
generating the
> > > > PCIe bus clock from it's register clock? No need to enable any 
other
> > > > clock for PCIe to work?
> > > Yes, just the one clock. The PCIe bus clock is an external clock 
dedicated 
> > > to PCIe and SATA, and we have no control over it.
> > > 
> > Is this some kind of always-on clock? If not you probably want a
> > reference to it from the PCIe driver even if it's shared between SATA
> > and PCIe. After all you don't want to end up unconditionally enabling
> > this clock from the board or SoC level just to have PCIe working, 
right?
> Ok, I see, I'll need a reference to a board clock, even if it's always 
on for 
> these boards. 
One small question...

The patches added the PCIe host controller to two devices, r8a7790 and 
r8a7791. The r8a7790-based Lager board doesn't have the necessary hardware 
to use PCIe, so should I add a dummy pcie_bus clock to the Lager board 
dts, even though PCIe is not used?

I'm just wondering what the dts file looks like for a board that doesn't 
support many interfaces. Does it become littered with dummy clocks and 
regulators?

Thanks
Phil

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

[Ben Dooks]

On 24/03/14 12:04, Phil.Edworthy at renesas.com wrote:
> Hi Lucas,
>
> On: 21/03/2014 15:02, Phil wrote:
>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> bindings
>>
>> Hi Lucas,
>>
>> On: 21/03/2014 14:31, Lucas wrote:
>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> bindings
>>>
>>> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
>>> Phil.Edworthy at renesas.com:
>>>> Hi Lucas,
>>>>
>>>> On 21/03/2014 11:24, Lucas wrote:
>>>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device
> tree
>>>> bindings
>>>>>
>>>>> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
>>>>>> This patch adds the bindings for the rcar PCIE driver. The
> driver
>>>>>> resides under drivers/pci/host/pcie-rcar.c
>>>>>>
>>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>>>>>
>>>>> I have one question below. If you can answer this with yes after
>>>>> thinking again about it, this is:
>>>>>
>>>>> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
>>>>>> ---
>>>>>>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 40
>>>> ++++++++++++++++++++++
>>>>>>   1 file changed, 40 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..0e219b0
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
>>>>>> @@ -0,0 +1,40 @@
>>>>>> +* Renesas RCar PCIe interface
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible: should contain one of the following
>>>>>> +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie",
>>>> "renesas,r8a7791-pcie"
>>>>>> +- 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
>>>>>> +- interrupts: interrupt values for MSI interrupt
>>>>>> +- #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"
>>>>>
>>>>> You really only have one PCIe clock? So the controller is
> generating the
>>>>> PCIe bus clock from it's register clock? No need to enable any
> other
>>>>> clock for PCIe to work?
>>>> Yes, just the one clock. The PCIe bus clock is an external clock
> dedicated
>>>> to PCIe and SATA, and we have no control over it.
>>>>
>>> Is this some kind of always-on clock? If not you probably want a
>>> reference to it from the PCIe driver even if it's shared between SATA
>>> and PCIe. After all you don't want to end up unconditionally enabling
>>> this clock from the board or SoC level just to have PCIe working,
> right?
>> Ok, I see, I'll need a reference to a board clock, even if it's always
> on for
>> these boards.
> One small question...
>
> The patches added the PCIe host controller to two devices, r8a7790 and
> r8a7791. The r8a7790-based Lager board doesn't have the necessary hardware
> to use PCIe, so should I add a dummy pcie_bus clock to the Lager board
> dts, even though PCIe is not used?
>
> I'm just wondering what the dts file looks like for a board that doesn't
> support many interfaces. Does it become littered with dummy clocks and
> regulators?

The bridge node should be set to disabled by default so it never gets
probed. Any boards that want to use it can over-ride the status to be
enabled.


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

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

[Phil.Edworthy at renesas.com]

Hi Ben,

On: 24/03/2014 12:16, Ben wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> On 24/03/14 12:04, Phil.Edworthy at renesas.com wrote:
> > Hi Lucas,
> >
> > On: 21/03/2014 15:02, Phil wrote:
> >> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > bindings
> >>
> >> Hi Lucas,
> >>
> >> On: 21/03/2014 14:31, Lucas wrote:
> >>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > bindings
> >>>
> >>> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> >>> Phil.Edworthy at renesas.com:
> >>>> Hi Lucas,
> >>>>
> >>>> On 21/03/2014 11:24, Lucas wrote:
> >>>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device
> > tree
> >>>> bindings
> >>>>>
> >>>>> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> >>>>>> This patch adds the bindings for the rcar PCIE driver. The
> > driver
> >>>>>> resides under drivers/pci/host/pcie-rcar.c
> >>>>>>
> >>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >>>>>
> >>>>> I have one question below. If you can answer this with yes after
> >>>>> thinking again about it, this is:
> >>>>>
> >>>>> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> >>>>>> ---
> >>>>>>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 40
> >>>> ++++++++++++++++++++++
> >>>>>>   1 file changed, 40 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..0e219b0
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> >>>>>> @@ -0,0 +1,40 @@
> >>>>>> +* Renesas RCar PCIe interface
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible: should contain one of the following
> >>>>>> +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie",
> >>>> "renesas,r8a7791-pcie"
> >>>>>> +- 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
> >>>>>> +- interrupts: interrupt values for MSI interrupt
> >>>>>> +- #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"
> >>>>>
> >>>>> You really only have one PCIe clock? So the controller is
> > generating the
> >>>>> PCIe bus clock from it's register clock? No need to enable any
> > other
> >>>>> clock for PCIe to work?
> >>>> Yes, just the one clock. The PCIe bus clock is an external clock
> > dedicated
> >>>> to PCIe and SATA, and we have no control over it.
> >>>>
> >>> Is this some kind of always-on clock? If not you probably want a
> >>> reference to it from the PCIe driver even if it's shared between 
SATA
> >>> and PCIe. After all you don't want to end up unconditionally 
enabling
> >>> this clock from the board or SoC level just to have PCIe working,
> > right?
> >> Ok, I see, I'll need a reference to a board clock, even if it's 
always
> > on for
> >> these boards.
> > One small question...
> >
> > The patches added the PCIe host controller to two devices, r8a7790 and
> > r8a7791. The r8a7790-based Lager board doesn't have the necessary 
hardware
> > to use PCIe, so should I add a dummy pcie_bus clock to the Lager board
> > dts, even though PCIe is not used?
> >
> > I'm just wondering what the dts file looks like for a board that 
doesn't
> > support many interfaces. Does it become littered with dummy clocks and
> > regulators?
> 
> The bridge node should be set to disabled by default so it never gets
> probed. Any boards that want to use it can over-ride the status to be
> enabled.
Ok, thanks.

Phil

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

[Phil.Edworthy at renesas.com]

Hi Ben,

On: 24/03/2014 12:25, Phil wrote:
> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> 
> Hi Ben,
> 
> On: 24/03/2014 12:16, Ben wrote:
> > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
bindings
> > 
> > On 24/03/14 12:04, Phil.Edworthy at renesas.com wrote:
> > > Hi Lucas,
> > >
> > > On: 21/03/2014 15:02, Phil wrote:
> > >> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > > bindings
> > >>
> > >> Hi Lucas,
> > >>
> > >> On: 21/03/2014 14:31, Lucas wrote:
> > >>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device 
tree
> > > bindings
> > >>>
> > >>> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> > >>> Phil.Edworthy at renesas.com:
> > >>>> Hi Lucas,
> > >>>>
> > >>>> On 21/03/2014 11:24, Lucas wrote:
> > >>>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device
> > > tree
> > >>>> bindings
> > >>>>>
> > >>>>> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > >>>>>> This patch adds the bindings for the rcar PCIE driver. The
> > > driver
> > >>>>>> resides under drivers/pci/host/pcie-rcar.c
> > >>>>>>
> > >>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > >>>>>
> > >>>>> I have one question below. If you can answer this with yes after
> > >>>>> thinking again about it, this is:
> > >>>>>
> > >>>>> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > >>>>>> ---
> > >>>>>>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 40
> > >>>> ++++++++++++++++++++++
> > >>>>>>   1 file changed, 40 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..0e219b0
> > >>>>>> --- /dev/null
> > >>>>>> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > >>>>>> @@ -0,0 +1,40 @@
> > >>>>>> +* Renesas RCar PCIe interface
> > >>>>>> +
> > >>>>>> +Required properties:
> > >>>>>> +- compatible: should contain one of the following
> > >>>>>> +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie",
> > >>>> "renesas,r8a7791-pcie"
> > >>>>>> +- 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
> > >>>>>> +- interrupts: interrupt values for MSI interrupt
> > >>>>>> +- #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"
> > >>>>>
> > >>>>> You really only have one PCIe clock? So the controller is
> > > generating the
> > >>>>> PCIe bus clock from it's register clock? No need to enable any
> > > other
> > >>>>> clock for PCIe to work?
> > >>>> Yes, just the one clock. The PCIe bus clock is an external clock
> > > dedicated
> > >>>> to PCIe and SATA, and we have no control over it.
> > >>>>
> > >>> Is this some kind of always-on clock? If not you probably want a
> > >>> reference to it from the PCIe driver even if it's shared between 
SATA
> > >>> and PCIe. After all you don't want to end up unconditionally 
enabling
> > >>> this clock from the board or SoC level just to have PCIe working,
> > > right?
> > >> Ok, I see, I'll need a reference to a board clock, even if it's 
always
> > > on for
> > >> these boards.
> > > One small question...
> > >
> > > The patches added the PCIe host controller to two devices, r8a7790 
and
> > > r8a7791. The r8a7790-based Lager board doesn't have the necessary 
hardware
> > > to use PCIe, so should I add a dummy pcie_bus clock to the Lager 
board
> > > dts, even though PCIe is not used?
> > >
> > > I'm just wondering what the dts file looks like for a board that 
doesn't
> > > support many interfaces. Does it become littered with dummy clocks 
and
> > > regulators?
> > 
> > The bridge node should be set to disabled by default so it never gets
> > probed. Any boards that want to use it can over-ride the status to be
> > enabled.
> Ok, thanks.
Hmm, yes it doesn't get probed, but you still get a build problem as the 
DT entry references an clock that doesn't exist. Maybe I'm missing 
something...

Phil

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

[Lucas Stach]

Am Montag, den 24.03.2014, 12:46 +0000 schrieb
Phil.Edworthy at renesas.com:
> Hi Ben,
> 
> On: 24/03/2014 12:25, Phil wrote:
> > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
> bindings
> > 
> > Hi Ben,
> > 
> > On: 24/03/2014 12:16, Ben wrote:
> > > Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree 
> bindings
> > > 
> > > On 24/03/14 12:04, Phil.Edworthy at renesas.com wrote:
> > > > Hi Lucas,
> > > >
> > > > On: 21/03/2014 15:02, Phil wrote:
> > > >> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device tree
> > > > bindings
> > > >>
> > > >> Hi Lucas,
> > > >>
> > > >> On: 21/03/2014 14:31, Lucas wrote:
> > > >>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device 
> tree
> > > > bindings
> > > >>>
> > > >>> Am Freitag, den 21.03.2014, 14:18 +0000 schrieb
> > > >>> Phil.Edworthy at renesas.com:
> > > >>>> Hi Lucas,
> > > >>>>
> > > >>>> On 21/03/2014 11:24, Lucas wrote:
> > > >>>>> Subject: Re: [PATCH v4 5/9] dt-bindings: pci: rcar pcie device
> > > > tree
> > > >>>> bindings
> > > >>>>>
> > > >>>>> Am Freitag, den 21.03.2014, 10:32 +0000 schrieb Phil Edworthy:
> > > >>>>>> This patch adds the bindings for the rcar PCIE driver. The
> > > > driver
> > > >>>>>> resides under drivers/pci/host/pcie-rcar.c
> > > >>>>>>
> > > >>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > >>>>>
> > > >>>>> I have one question below. If you can answer this with yes after
> > > >>>>> thinking again about it, this is:
> > > >>>>>
> > > >>>>> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > > >>>>>> ---
> > > >>>>>>   Documentation/devicetree/bindings/pci/rcar-pci.txt | 40
> > > >>>> ++++++++++++++++++++++
> > > >>>>>>   1 file changed, 40 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..0e219b0
> > > >>>>>> --- /dev/null
> > > >>>>>> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> > > >>>>>> @@ -0,0 +1,40 @@
> > > >>>>>> +* Renesas RCar PCIe interface
> > > >>>>>> +
> > > >>>>>> +Required properties:
> > > >>>>>> +- compatible: should contain one of the following
> > > >>>>>> +   "renesas,r8a7779-pcie", "renesas,r8a7790-pcie",
> > > >>>> "renesas,r8a7791-pcie"
> > > >>>>>> +- 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
> > > >>>>>> +- interrupts: interrupt values for MSI interrupt
> > > >>>>>> +- #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"
> > > >>>>>
> > > >>>>> You really only have one PCIe clock? So the controller is
> > > > generating the
> > > >>>>> PCIe bus clock from it's register clock? No need to enable any
> > > > other
> > > >>>>> clock for PCIe to work?
> > > >>>> Yes, just the one clock. The PCIe bus clock is an external clock
> > > > dedicated
> > > >>>> to PCIe and SATA, and we have no control over it.
> > > >>>>
> > > >>> Is this some kind of always-on clock? If not you probably want a
> > > >>> reference to it from the PCIe driver even if it's shared between 
> SATA
> > > >>> and PCIe. After all you don't want to end up unconditionally 
> enabling
> > > >>> this clock from the board or SoC level just to have PCIe working,
> > > > right?
> > > >> Ok, I see, I'll need a reference to a board clock, even if it's 
> always
> > > > on for
> > > >> these boards.
> > > > One small question...
> > > >
> > > > The patches added the PCIe host controller to two devices, r8a7790 
> and
> > > > r8a7791. The r8a7790-based Lager board doesn't have the necessary 
> hardware
> > > > to use PCIe, so should I add a dummy pcie_bus clock to the Lager 
> board
> > > > dts, even though PCIe is not used?
> > > >
> > > > I'm just wondering what the dts file looks like for a board that 
> doesn't
> > > > support many interfaces. Does it become littered with dummy clocks 
> and
> > > > regulators?
> > > 
> > > The bridge node should be set to disabled by default so it never gets
> > > probed. Any boards that want to use it can over-ride the status to be
> > > enabled.
> > Ok, thanks.
> Hmm, yes it doesn't get probed, but you still get a build problem as the 
> DT entry references an clock that doesn't exist. Maybe I'm missing 
> something...
> 
If the clock is a board supplied one then you have to supply to clocks
property from the board dts, to get rid of the compile failure. You can
not have phandles in the PCIe node referencing a non-existing clock.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v4 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>
---
 arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index df9ec61..e7d498a 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -821,4 +821,23 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	pcie: pcie at fe000000 {
+		compatible = "renesas,r8a7790-pcie";
+		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>;
+		interrupts = <0 116 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>;
+		clock-names = "pcie";
+		status = "disabled";
+	};
 };
-- 
1.9.0

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

[Sergei Shtylyov]

Hello.

On 03/21/2014 01:32 PM, Phil Edworthy wrote:

> This patch adds the device tree nodes for R8A7790

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>   arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)

> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index df9ec61..e7d498a 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -821,4 +821,23 @@
>   		#size-cells = <0>;
>   		status = "disabled";
>   	};
> +
> +	pcie: pcie at fe000000 {
> +		compatible = "renesas,r8a7790-pcie";

    The convention adopted on this list is to put the "pcie" first, and SoC 
name the last.

> +		reg = <0 0xfe000000 0 0x80000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";

    This legacy property is only meaningful for the true OF firmware and thus 
have been phased out (it's only actively used for the memory nodes).

WBR, Sergei

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

[Phil.Edworthy at renesas.com]

Hi Sergei,

On: 21/03/2014 14:30, Sergei wrote:
> Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> Hello.
> 
> On 03/21/2014 01:32 PM, Phil Edworthy wrote:
> 
> > This patch adds the device tree nodes for R8A7790
> 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >   arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++
> >   1 file changed, 19 insertions(+)
> 
> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi 
b/arch/arm/boot/dts/r8a7790.dtsi
> > index df9ec61..e7d498a 100644
> > --- a/arch/arm/boot/dts/r8a7790.dtsi
> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
> > @@ -821,4 +821,23 @@
> >         #size-cells = <0>;
> >         status = "disabled";
> >      };
> > +
> > +   pcie: pcie at fe000000 {
> > +      compatible = "renesas,r8a7790-pcie";
> 
>     The convention adopted on this list is to put the "pcie" first, and 
SoC 
> name the last.
Ok, shame that all the other PCI hosts use vendor,device-pcie


> > +      reg = <0 0xfe000000 0 0x80000>;
> > +      #address-cells = <3>;
> > +      #size-cells = <2>;
> > +      device_type = "pci";
> 
>     This legacy property is only meaningful for the true OF firmware and 
thus 
> have been phased out (it's only actively used for the memory nodes).
Ok, thanks for pointing this out.

> WBR, Sergei
> 

Thanks
Phil

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

[Arnd Bergmann]

On Friday 21 March 2014 14:53:48 Phil.Edworthy at renesas.com wrote:
> 
> > > +      reg = <0 0xfe000000 0 0x80000>;
> > > +      #address-cells = >;
> > > +      #size-cells = <2>;
> > > +      device_type = "pci";
> > 
> >     This legacy property is only meaningful for the true OF firmware and 
> thus 
> > have been phased out (it's only actively used for the memory nodes).
> Ok, thanks for pointing this out.
> 

I think the code still relies on it for PCI though.

	Arnd

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

[Jason Gunthorpe]

On Fri, Mar 21, 2014 at 03:57:25PM +0100, Arnd Bergmann wrote:
> On Friday 21 March 2014 14:53:48 Phil.Edworthy at renesas.com wrote:
> > 
> > > > +      reg = <0 0xfe000000 0 0x80000>;
> > > > +      #address-cells = >;
> > > > +      #size-cells = <2>;
> > > > +      device_type = "pci";
> > > 
> > >     This legacy property is only meaningful for the true OF firmware and 
> > thus 
> > > have been phased out (it's only actively used for the memory nodes).
> > Ok, thanks for pointing this out.
> > 
> 
> I think the code still relies on it for PCI though.

Right, it must be present for PCI nodes, it triggers special behavior
in the address parsing routines.

What Phil has looks correct to me.

Regards,
Jason

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

[Sergei Shtylyov]

Hello.

On 03/21/2014 05:57 PM, Arnd Bergmann wrote:

>>>> +      reg = <0 0xfe000000 0 0x80000>;
>>>> +      #address-cells = >;
>>>> +      #size-cells = <2>;
>>>> +      device_type = "pci";

>>>      This legacy property is only meaningful for the true OF firmware and thus
>>> have been phased out (it's only actively used for the memory nodes).

>> Ok, thanks for pointing this out.

> I think the code still relies on it for PCI though.

    OK, I have found where it does that but the recommended value seems to be 
"pciex" for PCI-Express case.

> 	Arnd

WBR, Sergei

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

[Phil.Edworthy at renesas.com]

Hi Sergei,

On: 21/03/2014 15:42, Sergei wrote:
> Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> Hello.
> 
> On 03/21/2014 05:57 PM, Arnd Bergmann wrote:
> 
> >>>> +      reg = <0 0xfe000000 0 0x80000>;
> >>>> +      #address-cells = >;
> >>>> +      #size-cells = <2>;
> >>>> +      device_type = "pci";
> 
> >>>      This legacy property is only meaningful for the true OF 
firmware and thus
> >>> have been phased out (it's only actively used for the memory nodes).
> 
> >> Ok, thanks for pointing this out.
> 
> > I think the code still relies on it for PCI though.
> 
>     OK, I have found where it does that but the recommended value seems 
to be 
> "pciex" for PCI-Express case.

Ok, but why then do all the other PCIe hosts use "pci"?

Thanks
Phil

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

[Jason Gunthorpe]

On Fri, Mar 21, 2014 at 04:31:24PM +0000, Phil.Edworthy at renesas.com wrote:
> >     OK, I have found where it does that but the recommended value
> > seems to be "pciex" for PCI-Express case.
> 
> Ok, but why then do all the other PCIe hosts use "pci"?

Because that is what the PCI bus binding standard says to do, and
new drivers have been pushed toward conforming to that document.

I'm not sure if "pciex" was ever standardized, I haven't see a spec
that reflects that at least.

Commit 14e2abb732e485ee57d9d5b2cb8884652238e5c1 introduced support for
the "pciex" word, but it looks driven by proprietary choices by IBM..

In any event there isn't a single 'device_type = "pciex";' pre-exsting
in the kernel bindings today, so please don't add one :)

Regards,
Jason

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

[Geert Uytterhoeven]

On Fri, Mar 21, 2014 at 4:29 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>> +
>> +       pcie: pcie at fe000000 {
>> +               compatible = "renesas,r8a7790-pcie";
>
>    The convention adopted on this list is to put the "pcie" first, and SoC
> name the last.

Except for clock bindings ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[Sergei Shtylyov]

Hello.

On 03/21/2014 01:32 PM, Phil Edworthy wrote:

> This patch adds the device tree nodes for R8A7790

> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>   arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)

    Why are you not enabling PCIe for Lager, only on Koelsch?

WBR, Sergei

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

[Phil.Edworthy at renesas.com]

Hi Sergei,

On: 21/03/2014 14:32, Sergei wrote:
> Subject: Re: [PATCH v4 6/9] ARM: shmobile: Add PCIe device tree nodes 
for R8A7790
> 
> Hello.
> 
> On 03/21/2014 01:32 PM, Phil Edworthy wrote:
> 
> > This patch adds the device tree nodes for R8A7790
> 
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >   arch/arm/boot/dts/r8a7790.dtsi | 19 +++++++++++++++++++
> >   1 file changed, 19 insertions(+)
> 
>     Why are you not enabling PCIe for Lager, only on Koelsch?
The Lager board doesn't have PCIe hardware. Note that it's possible to 
modify the Lager board and convert the SATA signals to PCIe, and we've 
done that to check that it works on R-Car H2.

> WBR, Sergei
> 

Thanks
Phil

[PATCH v4 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>
---
 arch/arm/boot/dts/r8a7791-koelsch.dts |  4 ++++
 arch/arm/boot/dts/r8a7791.dtsi        | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index e24fed9..17c953f 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -352,3 +352,7 @@
 		spi-cpha;
 	};
 };
+
+&pcie {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index ccfba57..915a5a6 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -836,4 +836,23 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	pcie: pcie at fe000000 {
+		compatible = "renesas,r8a7791-pcie";
+		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>;
+		interrupts = <0 116 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>;
+		clock-names = "pcie";
+		status = "disabled";
+	};
 };
-- 
1.9.0

[PATCH v4 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

[PATCH v4 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.
---
 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