Re: [PATCH 3/5] PCI: st: Provide support for the sti PCIe controller

From: Jingoo Han
Date: Thu Dec 18 2014 - 01:03:45 EST


On Wednesday, December 17, 2014 7:35 PM, Gabriel FERNANDEZ wrote:
>
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@xxxxxxxxxx>
> ---
> drivers/pci/host/Kconfig | 5 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pci-st.c | 713 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 719 insertions(+)
> create mode 100644 drivers/pci/host/pci-st.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index c4b6568..999d2b9 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -102,4 +102,9 @@ config PCI_LAYERSCAPE
> help
> Say Y here if you want PCIe controller support on Layerscape SoCs.
>
> +config PCI_ST
> + bool "ST STiH41x PCIe controller"
> + depends on ARCH_STI
> + select PCIE_DW
> +
> endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 44c2699..ca14829 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
> obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
> obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_ST) += pci-st.o
> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
> new file mode 100644
> index 0000000..bd3d32d
> --- /dev/null
> +++ b/drivers/pci/host/pci-st.c
> @@ -0,0 +1,713 @@
> +/*
> + * Copyright (C) 2014 STMicroelectronics
> + *
> + * STMicroelectronics PCI express Driver for sti SoCs.
> + * ST PCIe IPs are built around a Synopsys IP Core.
> + *
> + * Author: Fabrice Gasnier <fabrice.gasnier@xxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/delay.h>
> +#include <linux/reset.h>
> +#include <linux/phy/phy.h>

Please, re-order these headers alphabetically.
It enhances the readability.

> +
> +#include "pcie-designware.h"
> +
> +#define TRANSLATION_CONTROL 0x900
> +/* Controls if area is inclusive or exclusive */
> +#define RC_PASS_ADDR_RANGE BIT(1)
> +
> +/* Base of area reserved for config accesses. Fixed size of 64K. */
> +#define CFG_BASE_ADDRESS 0x92c
> +#define CFG_REGION_SIZE 65536
> +
> +/* First 4K of config space has this BDF (bus,device,function) */
> +#define FUNC0_BDF_NUM 0x930
> +
> +/* Mem regions */
> +#define IN0_MEM_ADDR_START 0x964
> +#define IN0_MEM_ADDR_LIMIT 0x968
> +#define IN1_MEM_ADDR_START 0x974
> +#define IN1_MEM_ADDR_LIMIT 0x978
> +
> +/* This actually contains the LTSSM state machine state */
> +#define PORT_LOGIC_DEBUG_REG_0 0x728
> +
> +/* LTSSM state machine values */
> +#define DEBUG_REG_0_LTSSM_MASK 0x1f
> +#define S_DETECT_QUIET 0x00
> +#define S_DETECT_ACT 0x01
> +#define S_POLL_ACTIVE 0x02
> +#define S_POLL_COMPLIANCE 0x03
> +#define S_POLL_CONFIG 0x04
> +#define S_PRE_DETECT_QUIET 0x05
> +#define S_DETECT_WAIT 0x06
> +#define S_CFG_LINKWD_START 0x07
> +#define S_CFG_LINKWD_ACEPT 0x08
> +#define S_CFG_LANENUM_WAIT 0x09
> +#define S_CFG_LANENUM_ACEPT 0x0A
> +#define S_CFG_COMPLETE 0x0B
> +#define S_CFG_IDLE 0x0C
> +#define S_RCVRY_LOCK 0x0D
> +#define S_RCVRY_SPEED 0x0E
> +#define S_RCVRY_RCVRCFG 0x0F
> +#define S_RCVRY_IDLE 0x10
> +#define S_L0 0x11
> +#define S_L0S 0x12
> +#define S_L123_SEND_EIDLE 0x13
> +#define S_L1_IDLE 0x14
> +#define S_L2_IDLE 0x15
> +#define S_L2_WAKE 0x16
> +#define S_DISABLED_ENTRY 0x17
> +#define S_DISABLED_IDLE 0x18
> +#define S_DISABLED 0x19
> +#define S_LPBK_ENTRY 0x1A
> +#define S_LPBK_ACTIVE 0x1B
> +#define S_LPBK_EXIT 0x1C
> +#define S_LPBK_EXIT_TIMEOUT 0x1D
> +#define S_HOT_RESET_ENTRY 0x1E
> +#define S_HOT_RESET 0x1F
> +
> +/* syscfg bits */
> +#define PCIE_SYS_INT BIT(5)
> +#define PCIE_APP_REQ_RETRY_EN BIT(3)
> +#define PCIE_APP_LTSSM_ENABLE BIT(2)
> +#define PCIE_APP_INIT_RST BIT(1)
> +#define PCIE_DEVICE_TYPE BIT(0)
> +#define PCIE_DEFAULT_VAL PCIE_DEVICE_TYPE
> +
> +/* Time to wait between testing the link in msecs (hardware poll interval) */
> +#define LINK_LOOP_DELAY_MS 1
> +/* Total amount of time to wait for the link to come up in msecs */
> +#define LINK_WAIT_MS 120
> +#define LINK_LOOP_COUNT (LINK_WAIT_MS / LINK_LOOP_DELAY_MS)
> +
> +/* st,syscfg offsets */
> +#define SYSCFG0_REG 1
> +#define SYSCFG1_REG 2
> +
> +#define to_st_pcie(x) container_of(x, struct st_pcie, pp)
> +
> +/**
> + * struct st_pcie_ops - SOC dependent data
> + * @init: reference to controller power & reset init routine
> + * @enable_ltssm: reference to controller link enable routine
> + * @disable_ltssm: reference to controller link disable routine
> + * @phy_auto: flag when phy automatically configured
> + */
> +struct st_pcie_ops {
> + int (*init)(struct pcie_port *pp);
> + int (*enable_ltssm)(struct pcie_port *pp);
> + int (*disable_ltssm)(struct pcie_port *pp);
> + bool phy_auto;
> +};
> +
> +/**
> + * struct st_pcie - private data of the controller
> + * @pp: designware pcie port
> + * @syscfg0: PCIe configuration 0 register, regmap offset
> + * @syscfg1: PCIe configuration 1 register, regmap offset
> + * @phy: associated pcie phy
> + * @config_area: PCIe configuration space
> + * @lmi: memory made available to the controller
> + * @data: SOC dependent data
> + * @regmap: Syscfg registers bank in which PCIe port is configured
> + * @pwr: power control
> + * @rst: reset control
> + * @reset_gpio: optional reset gpio
> + * @config_window_start: start address of 64K config space area
> + */
> +struct st_pcie {
> + struct pcie_port pp;
> + int syscfg0;
> + int syscfg1;
> + struct phy *phy;
> + void __iomem *config_area;
> + struct resource *lmi;
> + const struct st_pcie_ops *data;
> + struct regmap *regmap;
> + struct reset_control *pwr;
> + struct reset_control *rst;
> + int reset_gpio;
> + phys_addr_t config_window_start;
> +};
> +
> +/*
> + * Function to test if the link is in an operational state or not. We must
> + * ensure the link is operational before we try to do a configuration access.
> + */
> +static int st_pcie_link_up(struct pcie_port *pp)
> +{
> + u32 status;
> + int link_up;
> + int count = 0;
> +
> + /*
> + * We have to be careful here. This is used in config read/write,
> + * The higher levels switch off interrupts, so you cannot use
> + * jiffies to do a timeout, or reschedule
> + */
> + do {
> + /*
> + * What about L2? I think software intervention is
> + * required to get it out of L2, so in effect the link
> + * is down. Requires more work when/if we implement power
> + * management
> + */
> + status = readl_relaxed(pp->dbi_base + PORT_LOGIC_DEBUG_REG_0);
> + status &= DEBUG_REG_0_LTSSM_MASK;
> +
> + link_up = (status == S_L0) || (status == S_L0S) ||
> + (status == S_L1_IDLE);
> +
> + /*
> + * It can take some considerable time for the link to actually
> + * come up, caused by the PLLs. Experiments indicate it takes
> + * about 8ms to actually bring the link up, but this can vary
> + * considerably according to the specification. This code should
> + * allow sufficient time
> + */
> + if (!link_up)
> + mdelay(LINK_LOOP_DELAY_MS);
> +
> + } while (!link_up && ++count < LINK_LOOP_COUNT);
> +
> + return link_up;
> +}
> +
> +/*
> + * On ARM platforms, we actually get a bus error returned when the PCIe IP
> + * returns a UR or CRS instead of an OK.
> + */
> +static int st_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> + struct pt_regs *regs)
> +{
> + return 0;
> +}
> +
> +/*
> + * The PCI express core IP expects the following arrangement on it's address
> + * bus (slv_haddr) when driving config cycles.
> + * bus_number [31:24]
> + * dev_number [23:19]
> + * func_number [18:16]
> + * unused [15:12]
> + * ext_reg_number [11:8]
> + * reg_number [7:2]
> + *
> + * Bits [15:12] are unused.
> + *
> + * In the glue logic there is a 64K region of address space that can be
> + * written/read to generate config cycles. The base address of this is
> + * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
> + * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
> + * window into 8 regions at 4K boundaries. These control the bus,device and
> + * function number you are trying to talk to.
> + *
> + * The decision on whether to generate a type 0 or type 1 access is controlled
> + * by bits 15:12 of the address you write to. If they are zero, then a type 0
> + * is generated, if anything else it will be a type 1. Thus the bottom 4K
> + * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
> + * can only generate type 1.
> + *
> + * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
> + * by bit 12 of the address you write to. The selected register is then used
> + * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
> + * wired to zero, and bits 11:2 form the address of the register you want to
> + * read in config space.
> + *
> + * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
> + * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
> + */
> +static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
> +{
> + return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
> +}
> +
> +static inline unsigned config_addr(int where, int is_root_bus)
> +{
> + return (where & ~3) | (!is_root_bus << 12);
> +}
> +
> +static int st_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> + unsigned int devfn, int where, int size,
> + u32 *val)
> +{
> + u32 data;
> + u32 bdf;
> + struct st_pcie *pcie = to_st_pcie(pp);
> + int is_root_bus = pci_is_root_bus(bus);
> + int retry_count = 0;
> + int ret;
> + void __iomem *addr;
> +
> + /*
> + * Prerequisite
> + * PCI express devices will respond to all config type 0 cycles, since
> + * they are point to point links. Thus to avoid probing for multiple
> + * devices on the root, dw-pcie already check for us if it is on the
> + * root bus / other slots. Also, dw-pcie checks for the link being up
> + * as we will hang if we issue a config request and the link is down.
> + * A switch will reject requests for slots it knows do not exist.
> + */
> + bdf = bdf_num(bus->number, devfn, is_root_bus);
> + addr = pcie->config_area + config_addr(where,
> + bus->parent->number == pp->root_bus_nr);
> +retry:
> + /* Set the config packet devfn */
> + writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
> + readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
> +
> + ret = dw_pcie_cfg_read(addr, where, size, &data);
> +
> + /*
> + * This is intended to help with when we are probing the bus. The
> + * problem is that the wrapper logic doesn't have any way to
> + * interrogate if the configuration request failed or not.
> + * On the ARM we actually get a real bus error.
> + *
> + * Unfortunately this means it is impossible to tell the difference
> + * between when a device doesn't exist (the switch will return a UR
> + * completion) or the device does exist but isn't yet ready to accept
> + * configuration requests (the device will return a CRS completion)
> + *
> + * The result of this is that we will miss devices when probing.
> + *
> + * So if we are trying to read the dev/vendor id on devfn 0 and we
> + * appear to get zero back, then we retry the request. We know that
> + * zero can never be a valid device/vendor id. The specification says
> + * we must retry for up to a second before we decide the device is
> + * dead. If we are still dead then we assume there is nothing there and
> + * return ~0
> + *
> + * The downside of this is that we incur a delay of 1s for every pci
> + * express link that doesn't have a device connected.
> + */
> + if (((where & ~3) == 0) && devfn == 0 && (data == 0 || data == ~0)) {
> + if (retry_count++ < 1000) {
> + mdelay(1);
> + goto retry;
> + } else {
> + *val = ~0;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }
> + }
> +
> + *val = data;
> + return ret;
> +}
> +
> +static int st_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> + unsigned int devfn, int where, int size,
> + u32 val)
> +{
> + u32 bdf;
> + struct st_pcie *pcie = to_st_pcie(pp);
> + int is_root_bus = pci_is_root_bus(bus);
> + void __iomem *addr;
> +
> + bdf = bdf_num(bus->number, devfn, is_root_bus);
> + addr = pcie->config_area + config_addr(where,
> + bus->parent->number == pp->root_bus_nr);
> +
> + /* Set the config packet devfn */
> + writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
> + readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
> +
> + return dw_pcie_cfg_write(addr, where, size, val);
> +}
> +
> +static void st_pcie_board_reset(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + if (!gpio_is_valid(pcie->reset_gpio))
> + return;
> +
> + if (gpio_direction_output(pcie->reset_gpio, 0)) {
> + dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
> + pcie->reset_gpio);
> + return;
> + }
> +
> + /* From PCIe spec */
> + usleep_range(1000, 2000);
> + gpio_direction_output(pcie->reset_gpio, 1);
> +
> + /*
> + * PCIe specification states that you should not issue any config
> + * requests until 100ms after asserting reset, so we enforce that here
> + */
> + usleep_range(100000, 150000);
> +}
> +
> +static void st_pcie_hw_setup(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + dw_pcie_setup_rc(pp);
> +
> + /* Set up the config window to the top of the PCI address space */
> + writel_relaxed(pcie->config_window_start,
> + pp->dbi_base + CFG_BASE_ADDRESS);
> +
> + /*
> + * Open up memory to the PCI controller. We could do slightly
> + * better than this and exclude the kernel text segment and bss etc.
> + * They are base/limit registers so can be of arbitrary alignment
> + * presumably
> + */
> + writel_relaxed(pcie->lmi->start, pp->dbi_base + IN0_MEM_ADDR_START);
> + writel_relaxed(pcie->lmi->end, pp->dbi_base + IN0_MEM_ADDR_LIMIT);
> +
> + /* Disable the 2nd region */
> + writel_relaxed(~0, pp->dbi_base + IN1_MEM_ADDR_START);
> + writel_relaxed(0, pp->dbi_base + IN1_MEM_ADDR_LIMIT);
> +
> + writel_relaxed(RC_PASS_ADDR_RANGE, pp->dbi_base + TRANSLATION_CONTROL);
> +
> + /* Now assert the board level reset to the other PCIe device */
> + st_pcie_board_reset(pp);
> +}
> +
> +static irqreturn_t st_pcie_msi_irq_handler(int irq, void *arg)
> +{
> + struct pcie_port *pp = arg;
> +
> + return dw_handle_msi_irq(pp);
> +}
> +
> +static void st_msi_init_one(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + /*
> + * Set the magic address the hardware responds to. This has to be in
> + * the range the PCI controller can write to.
> + */
> + dw_pcie_msi_init(pp);
> +
> + if ((virt_to_phys((void *)pp->msi_data) < pcie->lmi->start) ||
> + (virt_to_phys((void *)pp->msi_data) > pcie->lmi->end))
> + dev_err(pp->dev, "MSI addr miss-configured\n");
> +}
> +
> +static void st_pcie_host_init(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> + int err;
> +
> + /*
> + * We have to initialise the PCIe cell on some hardware before we can
> + * talk to the phy
> + */
> + err = pcie->data->init(pp);
> + if (err)
> + return;
> +
> + err = pcie->data->disable_ltssm(pp);
> + if (err) {
> + dev_err(pp->dev, "disable ltssm failed, %d\n", err);
> + return;
> + }
> +
> + if (!pcie->data->phy_auto) {
> + /* Now init the associated miphy */
> + err = phy_init(pcie->phy);
> + if (err < 0) {
> + dev_err(pp->dev, "Cannot init PHY: %d\n", err);
> + return;
> + }
> + }
> +
> + /* Now do all the register poking */
> + st_pcie_hw_setup(pp);
> +
> + /* Re-enable the link */
> + err = pcie->data->enable_ltssm(pp);
> + if (err)
> + dev_err(pp->dev, "enable ltssm failed, %d\n", err);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + st_msi_init_one(pp);
> +}
> +
> +static struct pcie_host_ops st_pcie_host_ops = {
> + .rd_other_conf = st_pcie_rd_other_conf,
> + .wr_other_conf = st_pcie_wr_other_conf,
> + .link_up = st_pcie_link_up,
> + .host_init = st_pcie_host_init,
> +};
> +
> +static int st_pcie_init(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> + int ret;
> +
> + ret = reset_control_deassert(pcie->pwr);
> + if (ret) {
> + dev_err(pp->dev, "unable to bring out of powerdown\n");
> + return ret;
> + }
> +
> + ret = reset_control_deassert(pcie->rst);
> + if (ret) {
> + dev_err(pp->dev, "unable to bring out of softreset\n");
> + return ret;
> + }
> +
> + /* Set device type : Root Complex */
> + ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_DEVICE_TYPE);
> + if (ret < 0) {
> + dev_err(pp->dev, "unable to set device type\n");
> + return ret;
> + }
> +
> + usleep_range(1000, 2000);
> + return ret;
> +}
> +
> +/* STiH407 */
> +static int stih407_pcie_enable_ltssm(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + if (!pcie->syscfg1)
> + return -EINVAL;
> +
> + return regmap_update_bits(pcie->regmap, pcie->syscfg1,
> + PCIE_APP_LTSSM_ENABLE, PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int stih407_pcie_disable_ltssm(struct pcie_port *pp)
> +{
> + struct st_pcie *pcie = to_st_pcie(pp);
> +
> + if (!pcie->syscfg1)
> + return -EINVAL;
> +
> + return regmap_update_bits(pcie->regmap, pcie->syscfg1,
> + PCIE_APP_LTSSM_ENABLE, 0);
> +}
> +
> +static struct st_pcie_ops stih407_pcie_ops = {
> + .init = st_pcie_init,
> + .enable_ltssm = stih407_pcie_enable_ltssm,
> + .disable_ltssm = stih407_pcie_disable_ltssm,
> +};
> +
> +static const struct of_device_id st_pcie_of_match[] = {
> + { .compatible = "st,stih407-pcie", .data = (void *)&stih407_pcie_ops},
> + { },
> +};
> +
> +#ifdef CONFIG_PM
> +static int st_pcie_suspend(struct device *pcie_dev)
> +{
> + struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
> +
> + /* To guarantee a real phy initialization on resume */
> + if (!pcie->data->phy_auto)
> + phy_exit(pcie->phy);
> +
> + return 0;
> +}
> +
> +static int st_pcie_resume(struct device *pcie_dev)
> +{
> + struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
> +
> + st_pcie_host_init(&pcie->pp);
> +
> + return 0;
> +}
> +#endif
> +
> +const struct dev_pm_ops st_pcie_pm_ops = {
> + .suspend_noirq = st_pcie_suspend,
> + .resume_noirq = st_pcie_resume,
> +};
> +
> +static int __init st_pcie_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + struct st_pcie *pcie;
> + struct device_node *np = pdev->dev.of_node;
> + struct pcie_port *pp;
> + int ret;
> +
> + pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + memset(pcie, 0, sizeof(*pcie));
> +
> + pp = &pcie->pp;
> + pp->dev = &pdev->dev;
> +
> + /* mem regions */
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> + pp->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pp->dbi_base))
> + return PTR_ERR(pp->dbi_base);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> + if (!res)
> + return -ENXIO;
> +
> + /* Check that this has sensible values */
> + if ((resource_size(res) != CFG_REGION_SIZE) ||
> + (res->start & (CFG_REGION_SIZE - 1))) {
> + dev_err(&pdev->dev, "Invalid config space properties\n");
> + return -EINVAL;
> + }
> + pcie->config_window_start = res->start;
> +
> + pcie->config_area = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pcie->config_area))
> + return PTR_ERR(pcie->config_area);
> +
> + pcie->lmi = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "mem-window");
> + if (!pcie->lmi)
> + return -ENXIO;
> +
> + /* regmap registers for PCIe IP configuration */
> + pcie->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> + if (IS_ERR(pcie->regmap)) {
> + dev_err(&pdev->dev, "No syscfg phandle specified\n");
> + return PTR_ERR(pcie->regmap);
> + }
> +
> + ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG0_REG,
> + &pcie->syscfg0);
> + if (ret) {
> + dev_err(&pdev->dev, "can't get syscfg0 offset (%d)\n", ret);
> + return ret;
> + }
> +
> + ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG1_REG,
> + &pcie->syscfg1);
> + if (ret) {
> + dev_err(&pdev->dev, "can't get syscfg1 offset (%d)\n", ret);
> + return ret;
> + }
> +
> + /* retrieve data opts from compatible */
> + pcie->data = of_match_node(st_pcie_of_match, np)->data;
> + if (!pcie->data) {
> + dev_err(&pdev->dev, "compatible data not provided\n");
> + return -EINVAL;
> + }
> +
> + /* powerdown / resets */
> + pcie->pwr = devm_reset_control_get(&pdev->dev, "powerdown");
> + if (IS_ERR(pcie->pwr)) {
> + dev_err(&pdev->dev, "powerdown reset control not defined\n");
> + return -EINVAL;

Please return PTR_ERR(pcie->pwr) instead of '-EINVAL'.

> + }
> +
> + pcie->rst = devm_reset_control_get(&pdev->dev, "softreset");
> + if (IS_ERR(pcie->rst)) {
> + dev_err(&pdev->dev, "Soft reset control not defined\n");
> + return -EINVAL;

Please return PTR_ERR(pcie->pwr) instead of '-EINVAL'.

> + }
> +
> + /* phy */
> + if (!pcie->data->phy_auto) {
> + pcie->phy = devm_phy_get(&pdev->dev, "pcie_phy");
> + if (IS_ERR(pcie->phy)) {
> + dev_err(&pdev->dev, "no PHY configured\n");
> + return PTR_ERR(pcie->phy);
> + }
> + }
> +
> + /* Claim the GPIO for PRST# if available */
> + pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> + if (!gpio_is_valid(pcie->reset_gpio))
> + dev_dbg(&pdev->dev, "No reset-gpio configured\n");
> + else {
> + ret = devm_gpio_request(&pdev->dev,
> + pcie->reset_gpio,
> + "PCIe reset");
> + if (ret) {
> + dev_err(&pdev->dev, "Cannot request reset-gpio %d\n",
> + pcie->reset_gpio);
> + return ret;
> + }
> + }
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> + if (pp->msi_irq <= 0) {
> + dev_err(&pdev->dev, "failed to get MSI irq\n");
> + return -ENODEV;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, pp->msi_irq,
> + st_pcie_msi_irq_handler,
> + IRQF_SHARED, "st-pcie-msi", pp);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request MSI irq\n");
> + return -ENODEV;
> + }
> + }
> +
> + if (IS_ENABLED(CONFIG_ARM)) {
> + /*
> + * We have to hook the abort handler so that we can intercept
> + * bus errors when doing config read/write that return UR,
> + * which is flagged up as a bus error
> + */
> + hook_fault_code(16+6, st_pcie_abort_handler, SIGBUS, 0,
> + "imprecise external abort");
> + }
> +
> + pp->root_bus_nr = -1;
> + pp->ops = &st_pcie_host_ops;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, pcie);
> +
> + dev_info(&pdev->dev, "Initialized\n");
> + return 0;
> +}
> +
> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);
> +
> +static struct platform_driver st_pcie_driver = {
> + .driver = {
> + .name = "st-pcie",
> + .of_match_table = st_pcie_of_match,
> + .pm = &st_pcie_pm_ops,
> + },
> +};
> +
> +/* ST PCIe driver does not allow module unload */
> +static int __init pcie_init(void)
> +{
> + return platform_driver_probe(&st_pcie_driver, st_pcie_probe);
> +}
> +device_initcall(pcie_init);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@xxxxxx>");
> +MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
> +MODULE_LICENSE("GPLv2");

Please add one space between 'GPL' and 'v2'.

s/"GPLv2"/"GPL v2"

According to the license_is_gpl_compatible(), the MODULE_LICENSE() string
for GPL v2 is "GPL v2" not "GPLv2".

Best regards,
Jingoo Han

> --
> 1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/