Re: [PATCH 1/5] PCI: host: Add Renesas R-Car PCIe driver

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 26/02/14 16:08, Phil Edworthy wrote:
> 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>

> +	bool "Renesas R-Car PCIe controller"
> +	depends on ARM && (ARCH_R8A7790 || ARCH_R8A7791 || 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..d9a315f
> --- /dev/null
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -0,0 +1,614 @@
> +/*
> + * PCIe driver for Renesas R-Car SoCs
> + *  Copyright (C) 2013 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/module.h>
> +#include <linux/kernel.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 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 NR_PCI_RESOURCES 4
> +
> +/* Structure representing the PCIe interface */
> +struct rcar_pcie {
> +	struct device		*dev;
> +	void __iomem		*base;
> +	int			irq;
> +	struct clk		*clk;
> +	struct resource		*res[NR_PCI_RESOURCES];
> +	int			haslink;
> +	enum chip_id		chip;
> +	u8			root_bus_nr;
> +};
> +
> +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);
> +}

Do we really need wrappers like these?

> +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 int rcar_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> +{
> +	struct rcar_pcie *pcie = sys_to_pcie(dev->bus->sysdata);
> +	return pcie->irq;
> +}
> +
> +static void __init rcar_pcie_enable(struct rcar_pcie *pcie)
> +{
> +	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        = rcar_pcie_map_irq,
> +	hw.ops		  = &rcar_pcie_ops,
> +
> +	pci_common_init(&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;
> +}

Could this be done with some sort of sleep, instead of having
to keep delaying? How long is the average wait for pci to finish
a write?

> +
> +static void __init phy_write_reg(struct rcar_pcie *pcie,
> +				 unsigned int rate, unsigned int addr,
> +				 unsigned int lane, unsigned int data)
> +{
> +	unsigned long phyaddr;
> +
> +	phyaddr = WRITE_CMD |
> +		((rate & 1) << RATE_POS) |
> +		((lane & 0xf) << LANE_POS) |
> +		((addr & 0xff) << ADR_POS);
> +
> +	/* Set write data */
> +	pci_write_reg(pcie, data, H1_PCIEPHYDOUTR);
> +	pci_write_reg(pcie, phyaddr, H1_PCIEPHYADRR);
> +
> +	/* Ignore errors as they will be dealt with if the data link is down */
> +	phy_wait_for_ack(pcie);
> +
> +	/* Clear command */
> +	pci_write_reg(pcie, 0, H1_PCIEPHYDOUTR);
> +	pci_write_reg(pcie, 0, H1_PCIEPHYADRR);
> +
> +	/* Ignore errors as they will be dealt with if the data link is down */
> +	phy_wait_for_ack(pcie);
> +}
> +
>

> +static int __init rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
> +{
> +	unsigned int timeout = 100;
> +
> +	while (timeout--) {
> +		if ((pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
> +			return 0;
> +
> +		udelay(100);
> +	}
> +
> +	return -ETIMEDOUT;
> +}

Same comments as for previous delay loop


> +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, RCONF(PCI_INTERRUPT_LINE), 0xff, 0);
> +	rcar_rmw32(pcie, RCONF(PCI_INTERRUPT_PIN), 0xff, 1);
> +	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 rcar_pcie_clocks_get(struct rcar_pcie *pcie)
> +{
> +	int err = 0;
> +
> +	pcie->clk = clk_get(pcie->dev, NULL);
> +	if (IS_ERR(pcie->clk))
> +		err = PTR_ERR(pcie->clk);
> +	else
> +		clk_enable(pcie->clk);
> +
> +	return err;
> +}

Shouldn't you be using pm_runtime for this?

> +static void rcar_pcie_clocks_put(struct rcar_pcie *pcie)
> +{
> +	clk_put(pcie->clk);
> +}
> +
> +struct rcar_pcie_errs {
> +	int bit;
> +	const char *description;
> +};
> +
> +static int __init rcar_pcie_get_resources(struct platform_device *pdev,
> +	struct rcar_pcie *pcie)
> +{
> +	struct resource *res;
> +	int i;
> +	int err;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	i = platform_get_irq(pdev, 0);
> +	if (!res || i < 0) {
> +		dev_err(pcie->dev, "cannot get platform resources\n");
> +		return -ENOENT;
> +	}
> +	pcie->irq = i;
> +
> +	err = rcar_pcie_clocks_get(pcie);
> +	if (err) {
> +		dev_err(&pdev->dev, "failed to get clocks: %d\n", err);
> +		return err;
> +	}
> +
> +	pcie->base = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!pcie->base) {
> +		dev_err(&pdev->dev, "failed to map PCIe registers: %d\n", err);
> +		err = -ENOMEM;
> +		goto err_map_reg;
> +	}
> +
> +	return 0;
> +
> +err_map_reg:
> +	rcar_pcie_clocks_put(pcie);
> +
> +	return err;
> +}
> +
> +static struct platform_device_id rcar_pcie_id_table[] = {
> +	{ "r8a7779-pcie", RCAR_H1 },
> +	{ "r8a7790-pcie", RCAR_GENERIC },
> +	{ "r8a7791-pcie", RCAR_GENERIC },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(platform, rcar_pcie_id_table);

Really, are you still going to submit drivers without OF bindings?

NAK!

> +static int __init rcar_pcie_probe(struct platform_device *pdev)
> +{
> +	struct rcar_pcie *pcie;
> +	struct resource *res;
> +	unsigned int data;
> +	int i, err;
> +
> +	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	pcie->dev = &pdev->dev;
> +	pcie->chip = pdev->id_entry->driver_data;
> +
> +	/* Get resources */
> +	err = rcar_pcie_get_resources(pdev, pcie);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "failed to request resources: %d\n", err);
> +		return err;
> +	}
> +
> +	/* Get the I/O and memory ranges */
> +	for (i = 0; i < NR_PCI_RESOURCES; i++) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i+1);
> +		if (!res) {
> +			dev_err(&pdev->dev, "cannot get MEM%d resources\n", i);
> +			return -ENOENT;
> +		}
> +		pcie->res[i] = res;
> +	}
> +
> +	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);
> +
> +	platform_set_drvdata(pdev, pcie);
> +	return 0;
> +}
> +
> +static struct platform_driver rcar_pcie_driver = {
> +	.probe		= rcar_pcie_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +	},
> +	.id_table	= rcar_pcie_id_table,
> +};
> +
> +module_platform_driver(rcar_pcie_driver);
> +
> +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
> +MODULE_DESCRIPTION("Renesas R-Car PCIe driver");
> +MODULE_LICENSE("GPLv2");
> diff --git a/drivers/pci/host/pcie-rcar.h b/drivers/pci/host/pcie-rcar.h
> new file mode 100644
> index 0000000..a6b407f
> --- /dev/null
> +++ b/drivers/pci/host/pcie-rcar.h
> @@ -0,0 +1,84 @@
> +/*
> + * PCI Express definitions for Renesas R-Car SoCs
> + */
> +#ifndef __PCI_RCAR_H
> +#define __PCI_RCAR_H
> +
> +#define PCI_DEVICE_ID_RENESAS_RCAR	0x0018

should this go into pci ids header?

> +#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 */
>


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