2014-12-17 10:36:56

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 0/5] PCI: st: provide support for dw pcie

This patch-set introduces a STMicroelectronics PCIe controler.
It's based on designware PCIe driver.

Gabriel Fernandez (5):
ARM: STi: Kconfig update for PCIe support
PCI: st: Add Device Tree bindings for sti pcie
PCI: st: Provide support for the sti PCIe controller
PCI: designware: Add setup bus-related to pcie_host_ops
PCI: st: disable IO support

Documentation/devicetree/bindings/pci/st-pcie.txt | 53 ++
arch/arm/mach-sti/Kconfig | 2 +
drivers/pci/host/Kconfig | 5 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-st.c | 736 ++++++++++++++++++++++
drivers/pci/host/pcie-designware.c | 3 +
drivers/pci/host/pcie-designware.h | 1 +
7 files changed, 801 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
create mode 100644 drivers/pci/host/pci-st.c

--
1.9.1


2014-12-17 10:36:25

by Gabriel FERNANDEZ

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

sti pcie is built around a Synopsis Designware PCIe IP.

Signed-off-by: Fabrice Gasnier <[email protected]>
Signed-off-by: Gabriel Fernandez <[email protected]>
---
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 <[email protected]>
+ *
+ * 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>
+
+#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;
+ }
+
+ 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;
+ }
+
+ /* 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 <[email protected]>");
+MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
+MODULE_LICENSE("GPLv2");
--
1.9.1

2014-12-17 10:36:23

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 5/5] PCI: st: disable IO support

sti SoCs PCIe IPs are built around DesignWare IP Core.
But in these SoCs, PCIe IP doesn't support IO.
By default, when no IO space is provided, a default one is assigned.

Add an empty IO resource to the bus, and disable IO by default.

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/pci/host/pci-st.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
index bd3d32d..c0d3895 100644
--- a/drivers/pci/host/pci-st.c
+++ b/drivers/pci/host/pci-st.c
@@ -357,9 +357,15 @@ static void st_pcie_board_reset(struct pcie_port *pp)
static void st_pcie_hw_setup(struct pcie_port *pp)
{
struct st_pcie *pcie = to_st_pcie(pp);
+ u32 val;

dw_pcie_setup_rc(pp);

+ /* Disable IO support */
+ val = readl_relaxed(pp->dbi_base + PCI_COMMAND);
+ val &= ~PCI_COMMAND_IO;
+ writel_relaxed(val, pp->dbi_base + PCI_COMMAND);
+
/* 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);
@@ -445,11 +451,28 @@ static void st_pcie_host_init(struct pcie_port *pp)
st_msi_init_one(pp);
}

+static void st_pcie_setup_bus(struct pcie_port *pp, struct pci_sys_data *sys)
+{
+ struct resource *res;
+
+ /* This PCIe controller does not support IO, set an empty one. */
+ res = devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return;
+
+ res->start = 0;
+ res->end = 0;
+ res->name = "PCIe empty IO space";
+ res->flags = IORESOURCE_IO;
+ pci_add_resource(&sys->resources, res);
+}
+
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,
+ .setup_bus = st_pcie_setup_bus,
};

static int st_pcie_init(struct pcie_port *pp)
--
1.9.1

2014-12-17 10:36:57

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 4/5] PCI: designware: Add setup bus-related to pcie_host_ops

ST sti SoCs PCIe IPs are built around DesignWare IP Core.
But in these SoCs PCIe IP doesn't support IO.

To support this, add setup_bus() to pcie_host_ops.

Signed-off-by: Fabrice Gasnier <[email protected]>
Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/pci/host/pcie-designware.c | 3 +++
drivers/pci/host/pcie-designware.h | 1 +
2 files changed, 4 insertions(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index df781cd..98e19bc 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -719,6 +719,9 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
pci_add_resource(&sys->resources, &pp->busn);

+ if (pp->ops->setup_bus)
+ pp->ops->setup_bus(pp, sys);
+
return 1;
}

diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd27..5c13de7 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -73,6 +73,7 @@ struct pcie_host_ops {
u32 (*get_msi_addr)(struct pcie_port *pp);
u32 (*get_msi_data)(struct pcie_port *pp, int pos);
void (*scan_bus)(struct pcie_port *pp);
+ void (*setup_bus)(struct pcie_port *pp, struct pci_sys_data *sys);
int (*msi_host_init)(struct pcie_port *pp, struct msi_controller *chip);
};

--
1.9.1

2014-12-17 10:36:54

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 2/5] PCI: st: Add Device Tree bindings for sti pcie

sti pcie is built around a Synopsis Designware PCIe IP.

Signed-off-by: Fabrice Gasnier <[email protected]>
Signed-off-by: Gabriel Fernandez <[email protected]>
---
Documentation/devicetree/bindings/pci/st-pcie.txt | 53 +++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
new file mode 100644
index 0000000..bd3488f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
@@ -0,0 +1,53 @@
+STMicroelectronics STi PCIe controller
+
+This PCIe host controller is based on the Synopsis Designware PCIe IP
+and thus inherits all the common properties defined in designware-pcie.txt.
+
+Required properties:
+ - compatible: "st,stih407-pcie"
+ - reg: base address and length of the pcie controller, mem-window address
+ and length available to the controller.
+ - interrupts: A list of interrupt outputs of the controller.
+ - interrupt-names: Must include the following entries:
+ "msi": STi interrupt that is asserted when an MSI is received
+ - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
+ offset for IP configuration.
+ - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
+ Associated names must be "powerdown" and "softreset".
+ - phys, phy-names: the phandle for the PHY device.
+ Associated name must be "pcie_phy"
+
+Optional properties:
+ - reset-gpio: a GPIO spec to define which pin is connected to the bus reset.
+
+Example:
+
+pcie0: pcie@9b00000 {
+ compatible = "st,stih407-pcie", "snps,dw-pcie";
+ device_type = "pci";
+ reg = <0x09b00000 0x4000>, /* dbi cntrl registers */
+ <0x2fff0000 0x00010000>, /* configuration space */
+ <0x40000000 0x80000000>; /* lmi mem window */
+ reg-names = "dbi", "config", "mem-window";
+ st,syscfg = <&syscfg_core 0xd8 0xe0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ ranges = <0x00000800 0 0x2fff0000 0x2fff0000 0 0x00010000 /* configuration space */
+ 0x82000000 0 0x20000000 0x20000000 0 0x0FFF0000>; /* non-prefetchable memory */
+ num-lanes = <1>;
+ interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &intc GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, /* INT A */
+ <0 0 0 2 &intc GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>, /* INT B */
+ <0 0 0 3 &intc GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>, /* INT C */
+ <0 0 0 4 &intc GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>; /* INT D */
+
+ resets = <&powerdown STIH407_PCIE0_POWERDOWN>,
+ <&softreset STIH407_PCIE0_SOFTRESET>;
+ reset-names = "powerdown",
+ "softreset";
+ phys = <&phy_port0 PHY_TYPE_PCIE>;
+ phy-names = "pcie_phy";
+};
--
1.9.1

2014-12-17 10:37:57

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 1/5] ARM: STi: Kconfig update for PCIe support

Update Kconfig:
- MIGHT_HAVE_PCI
- PCI_DOMAINS

Signed-off-by: Fabrice Gasnier <[email protected]>
---
arch/arm/mach-sti/Kconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-sti/Kconfig b/arch/arm/mach-sti/Kconfig
index 8825bc9..d1e563c 100644
--- a/arch/arm/mach-sti/Kconfig
+++ b/arch/arm/mach-sti/Kconfig
@@ -8,6 +8,8 @@ menuconfig ARCH_STI
select ARCH_HAS_RESET_CONTROLLER
select HAVE_ARM_SCU if SMP
select ARCH_REQUIRE_GPIOLIB
+ select PCI_DOMAINS if PCI
+ select MIGHT_HAVE_PCI
select ARM_ERRATA_754322
select ARM_ERRATA_764369 if SMP
select ARM_ERRATA_775420
--
1.9.1

2014-12-17 14:03:50

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 5/5] PCI: st: disable IO support

On Wed, 17 Dec 2014 11:34:46 +0100
Gabriel FERNANDEZ <[email protected]> wrote:

> sti SoCs PCIe IPs are built around DesignWare IP Core.
> But in these SoCs, PCIe IP doesn't support IO.
> By default, when no IO space is provided, a default one is assigned.
>
> Add an empty IO resource to the bus, and disable IO by default.

As a point of PCI pedantry I don't think this is quite sufficient. PCI
has a weird corner case where I/O resources are implied rather than
allocated.

For IDE/SATA you may need to something like

if (class == PCI_CLASS_STORAGE_IDE) {
u8 progif;
pci_read_config_byte(dev, PCI_CLASS_PROG, &progif);
progif |= 5;
pci_write_config_byte(dev, PCI_CLASS_PROG, &progif);
}

so that any adapter is kicked out of legacy mode and doesn't get implied
I/O resources and interrupts. I don't know if that case matters for your
usage.

2014-12-17 22:03:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: st: Add Device Tree bindings for sti pcie

On Wednesday 17 December 2014 11:34:43 Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/st-pcie.txt | 53 +++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
> new file mode 100644
> index 0000000..bd3488f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
> @@ -0,0 +1,53 @@
> +STMicroelectronics STi PCIe controller
> +
> +This PCIe host controller is based on the Synopsis Designware PCIe IP
> +and thus inherits all the common properties defined in designware-pcie.txt.
> +
> +Required properties:
> + - compatible: "st,stih407-pcie"
> + - reg: base address and length of the pcie controller, mem-window address
> + and length available to the controller.
> + - interrupts: A list of interrupt outputs of the controller.
> + - interrupt-names: Must include the following entries:
> + "msi": STi interrupt that is asserted when an MSI is received

You should specify that only one interrupt may be present.
If you extend the binding, you will need to describe what the
second one is as well.

> + - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
> + offset for IP configuration.
> + - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
> + Associated names must be "powerdown" and "softreset".
> + - phys, phy-names: the phandle for the PHY device.
> + Associated name must be "pcie_phy"

Names should not have underscores in them in general. "pcie-phy" would
be fine, but just "pcie" seems good enough, unless there are existing
drivers that have established a specific naming.


Arnd

2014-12-17 22:15:41

by Arnd Bergmann

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

On Wednesday 17 December 2014 11:34:44 Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> 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

I'd use 'depends on ARCH_STI || (ARM && COMPILE_TEST)' to enable
building this on other platforms for test purposes.

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

It would be better not to invent another level of indirection. Try
turning this around so you have a driver that binds to the specific
SoC compatible string for the PCIe port while calling into a common
library module for things that are shared.

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

You should check that it's actually PCI that caused the abort. Don't
just ignore a hard error condition.

Usually there are registers in the PCI core that let you identify what
happened.

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

A busy-loop is extremely nasty. If this is only during the initial bus
scan, could you use an msleep instead?

Also, it sounds like the error you get is actually the fault that you
are catching above. If this is correct, then use the fault handler to
communicate this to the probe function.

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

This seems hardly performance critical, just use msleep(2) and
msleep(100) instead of the usleep_range().

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

Why do you call virt_to_phys() here? Isn't msi_data a physical address?

> +static int __init st_pcie_probe(struct platform_device *pdev)

I'd suggest removing the __init here, as discussed in the review for
the qualcomm driver.

Arnd

2014-12-17 22:17:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] PCI: designware: Add setup bus-related to pcie_host_ops

On Wednesday 17 December 2014 11:34:45 Gabriel FERNANDEZ wrote:
> ST sti SoCs PCIe IPs are built around DesignWare IP Core.
> But in these SoCs PCIe IP doesn't support IO.
>
> To support this, add setup_bus() to pcie_host_ops.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>

The dw-pcie driver should be able to tell whether the device has
an I/O space or not, and do the right thing based on that. Don't
add an implementation specific callback for that.

Arnd

2014-12-18 04:58:33

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH 4/5] PCI: designware: Add setup bus-related to pcie_host_ops

On Thursday, December 18, 2014 7:16 AM, Arnd Bergmann wrote:
> On Wednesday 17 December 2014 11:34:45 Gabriel FERNANDEZ wrote:
> > ST sti SoCs PCIe IPs are built around DesignWare IP Core.
> > But in these SoCs PCIe IP doesn't support IO.

Hi Gabriel,

I cannot understand how ST sti SoCs PCIe IP does not support I/O.
As far as I know, it cannot be selected by the 'parameter'.
Then, H/W engineers dropped out the I/O control logic?

> >
> > To support this, add setup_bus() to pcie_host_ops.


> >
> > Signed-off-by: Fabrice Gasnier <[email protected]>
> > Signed-off-by: Gabriel Fernandez <[email protected]>
>
> The dw-pcie driver should be able to tell whether the device has
> an I/O space or not, and do the right thing based on that. Don't
> add an implementation specific callback for that.

I agree with Arnd's opinion.

In addition, I have one more question.
Then, if a device that requires I/O region is connected to
PCIe slot of ST sti SoCs PCIe, what will happen?
It just prints error messages?

Best regards,
Jingoo Han

>
> Arnd

2014-12-18 06:03:41

by Jingoo Han

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

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 <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> 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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +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

2014-12-22 04:47:33

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH 2/5] PCI: st: Add Device Tree bindings for sti pcie



On Wednesday 17 December 2014 04:04 PM, Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/st-pcie.txt | 53 +++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
> new file mode 100644
> index 0000000..bd3488f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
> @@ -0,0 +1,53 @@
> +STMicroelectronics STi PCIe controller
> +
> +This PCIe host controller is based on the Synopsis Designware PCIe IP
> +and thus inherits all the common properties defined in designware-pcie.txt.
> +
> +Required properties:
> + - compatible: "st,stih407-pcie"
> + - reg: base address and length of the pcie controller, mem-window address
> + and length available to the controller.
> + - interrupts: A list of interrupt outputs of the controller.
> + - interrupt-names: Must include the following entries:
> + "msi": STi interrupt that is asserted when an MSI is received
> + - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
> + offset for IP configuration.
> + - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
> + Associated names must be "powerdown" and "softreset".
> + - phys, phy-names: the phandle for the PHY device.
> + Associated name must be "pcie_phy"
> +
> +Optional properties:
> + - reset-gpio: a GPIO spec to define which pin is connected to the bus reset.
> +
> +Example:
> +
> +pcie0: pcie@9b00000 {
> + compatible = "st,stih407-pcie", "snps,dw-pcie";
> + device_type = "pci";
> + reg = <0x09b00000 0x4000>, /* dbi cntrl registers */
> + <0x2fff0000 0x00010000>, /* configuration space */
> + <0x40000000 0x80000000>; /* lmi mem window */
> + reg-names = "dbi", "config", "mem-window";
> + st,syscfg = <&syscfg_core 0xd8 0xe0>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x00000800 0 0x2fff0000 0x2fff0000 0 0x00010000 /* configuration space */

Pass it through reg property.
See: arch/arm/boot/dts/spear1310.dtsi or any other dw pcie's dtsi.


@Jingoo, Mohit: I would suggest following changes so that no upcoming
platform can add it through ranges.

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 4fc03b7f1cee..f21570847d08 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -143,8 +143,7 @@
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
- ranges = <0x00000800 0 0x01f00000 0x01f00000 0
0x00080000 /* configuration space */
- 0x81000000 0 0 0x01f80000 0
0x00010000 /* downstream I/O */
+ ranges = 0x81000000 0 0 0x01f80000 0
0x00010000 /* downstream I/O */
0x82000000 0 0x01000000 0x01000000 0
0x00f00000>; /* non-prefetchable memory */
num-lanes = <1>;
interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 7a24fee1e7ae..72593a77455e 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -1197,14 +1197,14 @@

pcie: pcie@0x08000000 {
compatible = "fsl,imx6sx-pcie", "snps,dw-pcie";
- reg = <0x08ffc000 0x4000>; /* DBI */
+ reg = <0x08ffc000 0x4000>, /* DBI */
+ <0x08f00000 0x80000>;
+ reg-names = "dbi", "config";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
- /* configuration space */
- ranges = <0x00000800 0 0x08f00000 0x08f00000 0
0x00080000
/* downstream I/O */
- 0x81000000 0 0 0x08f80000 0
0x00010000
+ ranges = 0x81000000 0 0 0x08f80000 0
0x00010000
/* non-prefetchable memory */
0x82000000 0 0x08000000 0x08000000 0
0x00f00000>;
num-lanes = <1>;
diff --git a/drivers/pci/host/pcie-designware.c
b/drivers/pci/host/pcie-designware.c
index df781cdf13c1..0b22b42e1ff9 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -409,19 +409,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
pp->mem_mod_base = of_read_number(parser.range -
parser.np +
na, ns);
}
- if (restype == 0) {
- of_pci_range_to_resource(&range, np, &pp->cfg);
- pp->cfg0_size = resource_size(&pp->cfg)/2;
- pp->cfg1_size = resource_size(&pp->cfg)/2;
- pp->cfg0_base = pp->cfg.start;
- pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
-
- /* Find the untranslated configuration space
address */
- pp->cfg0_mod_base = of_read_number(parser.range -
- parser.np +
na, ns);
- pp->cfg1_mod_base = pp->cfg0_mod_base +
- pp->cfg0_size;
- }
}

ret = of_pci_parse_bus_range(np, &pp->busn);


Regards
Pratyush

> + 0x82000000 0 0x20000000 0x20000000 0 0x0FFF0000>; /* non-prefetchable memory */
> + num-lanes = <1>;
> + interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0 0 0 1 &intc GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, /* INT A */
> + <0 0 0 2 &intc GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>, /* INT B */
> + <0 0 0 3 &intc GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>, /* INT C */
> + <0 0 0 4 &intc GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>; /* INT D */
> +
> + resets = <&powerdown STIH407_PCIE0_POWERDOWN>,
> + <&softreset STIH407_PCIE0_SOFTRESET>;
> + reset-names = "powerdown",
> + "softreset";
> + phys = <&phy_port0 PHY_TYPE_PCIE>;
> + phy-names = "pcie_phy";
> +};
>

2014-12-22 05:14:04

by Pratyush Anand

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



On Wednesday 17 December 2014 04:04 PM, Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> 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
>

[...]

> +static int st_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> + struct pt_regs *regs)
> +{
> + return 0;
> +}
> +

You should have modification here to populate registers having cfg read
values by 0xFFFFFFFF.

I would suggest to have a look here:
drivers/pci/host/pci-keystone.c:keystone_pcie_fault

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

Not sure if retry has to be part of software. This might already be done
by hardware.

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

Have you found a situation with any of the card when your retry_count >
0 and < 1000 at this point. If not then, I think modifying abort
handler will solve your issue.

> +
> + *val = data;
> + return ret;
> +}
> +

~Pratyush