2014-06-25 17:57:17

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 0/3] dra7: Add PCIe support

[1] is split into separate series in order for individual subsystem
Maintainers to pick up the patches. This series handles the PCIe
support for DRA7.

Rebased to 3.16-rc2.

[1] -> https://lkml.org/lkml/2014/5/29/258

Kishon Vijay Abraham I (3):
PCI: designware: Configuration space should be specified in 'reg'
PCI: designware: use untranslated address while programming ATU
PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller

.../devicetree/bindings/pci/designware-pcie.txt | 4 +
Documentation/devicetree/bindings/pci/ti-pci.txt | 59 +++
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++
drivers/pci/host/pcie-designware.c | 66 ++-
drivers/pci/host/pcie-designware.h | 4 +
7 files changed, 589 insertions(+), 13 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
create mode 100644 drivers/pci/host/pci-dra7xx.c

--
1.7.9.5


2014-06-25 17:57:36

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
address. So whenever the cpu issues a read/write request, the 4 most
significant bits are used by L3 to determine the target controller.
For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
the outbound translation window the *base* should be programmed as 0x000_0000.
Whenever we try to write to say 0x2000_0000, it will be translated to whatever
we have programmed in the translation window with base as 0x000_0000.

This is needed when the dt node is modelled something like below
axi {
compatible = "simple-bus";
#size-cells = <1>;
#address-cells = <1>;
ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
0x51000000 0x51000000 0x3000>;
pcie@51000000 {
reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
reg-names = "config", "ti_conf", "rc_dbics";
#address-cells = <3>;
#size-cells = <2>;
ranges = <0x81000000 0 0 0x03000 0 0x00010000
0x82000000 0 0x20013000 0x13000 0 0xffed000>;
};
};

Here the CPU address for configuration space is 0x20013000 and the controller
address for configuration space is 0x13000. The controller address should be
used while programming the ATU (in order for translation to happen properly in
DRA7xx).

Cc: Jason Gunthorpe <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Mohit Kumar <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++--------
drivers/pci/host/pcie-designware.h | 4 +++
2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 0b7b455..11aba6d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
struct of_pci_range range;
struct of_pci_range_parser parser;
struct resource *cfg_res;
- u32 val;
- int i;
+ u32 val, na, ns;
+ const __be32 *addrp;
+ int i, index;
+
+ /* Find the address cell size and the number of cells in order to get
+ * the untranslated address.
+ */
+ of_property_read_u32(np, "#address-cells", &na);
+ ns = of_n_size_cells(np);

cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
if (cfg_res) {
@@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
pp->config.cfg1_size = resource_size(cfg_res)/2;
pp->cfg0_base = cfg_res->start;
pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
+
+ /* Find the untranslated configuration space address */
+ index = of_property_match_string(np, "reg-names", "config");
+ addrp = of_get_address(np, index, false, false);
+ pp->cfg0_mod_addr = of_read_number(addrp, ns);
+ pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size;
} else {
dev_err(pp->dev, "missing *config* reg space\n");
}
@@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
pp->config.io_size = resource_size(&pp->io);
pp->config.io_bus_addr = range.pci_addr;
pp->io_base = range.cpu_addr;
+
+ /* Find the untranslated IO space address */
+ pp->io_mod_addr = of_read_number(parser.range -
+ parser.np + na, ns);
}
if (restype == IORESOURCE_MEM) {
of_pci_range_to_resource(&range, np, &pp->mem);
pp->mem.name = "MEM";
pp->config.mem_size = resource_size(&pp->mem);
pp->config.mem_bus_addr = range.pci_addr;
+
+ /* Find the untranslated MEM space address */
+ pp->mem_mod_addr = of_read_number(parser.range -
+ parser.np + na, ns);
}
if (restype == 0) {
of_pci_range_to_resource(&range, np, &pp->cfg);
@@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
pp->config.cfg1_size = resource_size(&pp->cfg)/2;
pp->cfg0_base = pp->cfg.start;
pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
+
+ /* Find the untranslated configuration space address */
+ pp->cfg0_mod_addr = of_read_number(parser.range -
+ parser.np + na, ns);
+ pp->cfg1_mod_addr = pp->cfg0_mod_addr +
+ pp->config.cfg0_size;
}
}

@@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
/* Program viewport 0 : OUTBOUND : CFG0 */
dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
PCIE_ATU_VIEWPORT);
- dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
+ dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1,
PCIE_ATU_LIMIT);
dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
@@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1,
PCIE_ATU_VIEWPORT);
dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1);
- dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
+ dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1,
PCIE_ATU_LIMIT);
dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
@@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
PCIE_ATU_VIEWPORT);
dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1);
- dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
+ dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1,
PCIE_ATU_LIMIT);
dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET);
dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 77f592f..13828f8 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -36,11 +36,15 @@ struct pcie_port {
u8 root_bus_nr;
void __iomem *dbi_base;
u64 cfg0_base;
+ u64 cfg0_mod_addr;
void __iomem *va_cfg0_base;
u64 cfg1_base;
+ u64 cfg1_mod_addr;
void __iomem *va_cfg1_base;
u64 io_base;
+ u64 io_mod_addr;
u64 mem_base;
+ u64 mem_mod_addr;
struct resource cfg;
struct resource io;
struct resource mem;
--
1.7.9.5

2014-06-25 17:57:42

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 3/3] PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller

Added support for pcie controller in dra7xx. This driver re-uses
the designware core code that is already present in kernel.

Cc: Jason Gunthorpe <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Mohit Kumar <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
Documentation/devicetree/bindings/pci/ti-pci.txt | 59 +++
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++++
4 files changed, 528 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
create mode 100644 drivers/pci/host/pci-dra7xx.c

diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
new file mode 100644
index 0000000..3d21791
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
@@ -0,0 +1,59 @@
+TI PCI Controllers
+
+PCIe Designware Controller
+ - compatible: Should be "ti,dra7-pcie""
+ - reg : Two register ranges as listed in the reg-names property
+ - reg-names : The first entry must be "ti-conf" for the TI specific registers
+ The second entry must be "rc-dbics" for the designware pcie
+ registers
+ The third entry must be "config" for the PCIe configuration space
+ - phys : list of PHY specifiers (used by generic PHY framework)
+ - phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
+ number of PHYs as specified in *phys* property.
+ - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
+ where <X> is the instance number of the pcie from the HW spec.
+ - interrupts : Two interrupt entries must be specified. The first one is for
+ main interrupt line and the second for MSI interrupt line.
+ - #address-cells,
+ #size-cells,
+ #interrupt-cells,
+ device_type,
+ ranges,
+ num-lanes,
+ interrupt-map-mask,
+ interrupt-map : as specified in ../designware-pcie.txt
+
+Example:
+axi {
+ compatible = "simple-bus";
+ #size-cells = <1>;
+ #address-cells = <1>;
+ ranges = <0x51000000 0x51000000 0x3000
+ 0x0 0x20000000 0x10000000>;
+ pcie@51000000 {
+ compatible = "ti,dra7-pcie";
+ reg = <0x51000000 0x2000>, <0x51002000 0x14c>, <0x1000 0x2000>;
+ reg-names = "rc_dbics", "ti_conf", "config";
+ interrupts = <0 232 0x4>, <0 233 0x4>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ ranges = <0x81000000 0 0 0x03000 0 0x00010000
+ 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
+ #interrupt-cells = <1>;
+ num-lanes = <1>;
+ ti,hwmods = "pcie1";
+ phys = <&pcie1_phy>;
+ phy-names = "pcie-phy0";
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc 1>,
+ <0 0 0 2 &pcie_intc 2>,
+ <0 0 0 3 &pcie_intc 3>,
+ <0 0 0 4 &pcie_intc 4>;
+ pcie_intc: interrupt-controller {
+ interrupt-controller;
+ #address-cells = <0>;
+ #interrupt-cells = <1>;
+ };
+ };
+};
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 21df477..22117b0 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -1,6 +1,16 @@
menu "PCI host controller drivers"
depends on PCI

+config PCI_DRA7XX
+ bool "TI DRA7xx PCIe controller"
+ select PCIE_DW
+ depends on OF && HAS_IOMEM && TI_PIPE3
+ help
+ Enables support for the PCIE controller present in DRA7xx SoC. There
+ are two instances of PCIE controller in DRA7xx. This controller can
+ act both as EP and RC. This reuses the same Designware core as used
+ by other SoCs.
+
config PCI_MVEBU
bool "Marvell EBU PCIe controller"
depends on ARCH_MVEBU || ARCH_DOVE || ARCH_KIRKWOOD
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 611ba4b..c42844d 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_PCIE_DW) += pcie-designware.o
+obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
new file mode 100644
index 0000000..52b34fe
--- /dev/null
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -0,0 +1,458 @@
+/*
+ * pcie-dra7xx - PCIe controller driver for TI DRA7xx SoCs
+ *
+ * Copyright (C) 2013-2014 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Authors: Kishon Vijay Abraham I <[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/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/resource.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+/* PCIe controller wrapper DRA7XX configuration registers */
+
+#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN 0x0024
+#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN 0x0028
+#define ERR_SYS BIT(0)
+#define ERR_FATAL BIT(1)
+#define ERR_NONFATAL BIT(2)
+#define ERR_COR BIT(3)
+#define ERR_AXI BIT(4)
+#define ERR_ECRC BIT(5)
+#define PME_TURN_OFF BIT(8)
+#define PME_TO_ACK BIT(9)
+#define PM_PME BIT(10)
+#define LINK_REQ_RST BIT(11)
+#define LINK_UP_EVT BIT(12)
+#define CFG_BME_EVT BIT(13)
+#define CFG_MSE_EVT BIT(14)
+#define INTERRUPTS (ERR_SYS | ERR_FATAL | ERR_NONFATAL | ERR_COR | ERR_AXI | \
+ ERR_ECRC | PME_TURN_OFF | PME_TO_ACK | PM_PME | \
+ LINK_REQ_RST | LINK_UP_EVT | CFG_BME_EVT | CFG_MSE_EVT)
+
+#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI 0x0034
+#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI 0x0038
+#define INTA BIT(0)
+#define INTB BIT(1)
+#define INTC BIT(2)
+#define INTD BIT(3)
+#define MSI BIT(4)
+#define LEG_EP_INTERRUPTS (INTA | INTB | INTC | INTD)
+
+#define PCIECTRL_DRA7XX_CONF_DEVICE_CMD 0x0104
+#define LTSSM_EN 0x1
+
+#define PCIECTRL_DRA7XX_CONF_PHY_CS 0x010C
+#define LINK_UP BIT(16)
+
+struct dra7xx_pcie {
+ void __iomem *base;
+ struct phy **phy;
+ int phy_count;
+ struct device *dev;
+ struct pcie_port pp;
+};
+
+#define to_dra7xx_pcie(x) container_of((x), struct dra7xx_pcie, pp)
+
+static inline u32 dra7xx_pcie_readl(struct dra7xx_pcie *pcie, u32 offset)
+{
+ return readl(pcie->base + offset);
+}
+
+static inline void dra7xx_pcie_writel(struct dra7xx_pcie *pcie, u32 offset,
+ u32 value)
+{
+ writel(value, pcie->base + offset);
+}
+
+static int dra7xx_pcie_link_up(struct pcie_port *pp)
+{
+ struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
+ u32 reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
+
+ return !!(reg & LINK_UP);
+}
+
+static int dra7xx_pcie_establish_link(struct pcie_port *pp)
+{
+ u32 reg;
+ unsigned int retries = 1000;
+ struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
+
+ if (dw_pcie_link_up(pp)) {
+ dev_err(pp->dev, "link is already up\n");
+ return 0;
+ }
+
+ reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
+ reg |= LTSSM_EN;
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
+
+ while (retries--) {
+ reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
+ if (reg & LINK_UP)
+ break;
+ usleep_range(10, 20);
+ }
+
+ if (retries == 0) {
+ dev_err(pp->dev, "link is not up\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
+{
+ struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
+
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
+ ~INTERRUPTS);
+ dra7xx_pcie_writel(dra7xx,
+ PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN, INTERRUPTS);
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
+ ~LEG_EP_INTERRUPTS & ~MSI);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dra7xx_pcie_writel(dra7xx,
+ PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI, MSI);
+ else
+ dra7xx_pcie_writel(dra7xx,
+ PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI,
+ LEG_EP_INTERRUPTS);
+}
+
+static void dra7xx_pcie_host_init(struct pcie_port *pp)
+{
+ dw_pcie_setup_rc(pp);
+ dra7xx_pcie_establish_link(pp);
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_init(pp);
+ dra7xx_pcie_enable_interrupts(pp);
+}
+
+static struct pcie_host_ops dra7xx_pcie_host_ops = {
+ .link_up = dra7xx_pcie_link_up,
+ .host_init = dra7xx_pcie_host_init,
+};
+
+static int dra7xx_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+ .map = dra7xx_pcie_intx_map,
+};
+
+static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
+{
+ struct device *dev = pp->dev;
+ struct device_node *node = dev->of_node;
+ struct device_node *pcie_intc_node = of_get_next_child(node, NULL);
+
+ if (!pcie_intc_node) {
+ dev_err(dev, "No PCIe Intc node found\n");
+ return PTR_ERR(pcie_intc_node);
+ }
+
+ pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
+ &intx_domain_ops, pp);
+ if (!pp->irq_domain) {
+ dev_err(dev, "Failed to get a INTx IRQ domain\n");
+ return PTR_ERR(pp->irq_domain);
+ }
+
+ return 0;
+}
+
+static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
+{
+ struct pcie_port *pp = arg;
+ struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
+ u32 reg;
+
+ reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
+
+ switch (reg) {
+ case MSI:
+ dw_handle_msi_irq(pp);
+ break;
+ case INTA:
+ case INTB:
+ case INTC:
+ case INTD:
+ generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg)));
+ break;
+ }
+
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
+
+ return IRQ_HANDLED;
+}
+
+
+static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
+{
+ struct dra7xx_pcie *dra7xx = arg;
+ u32 reg;
+
+ reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN);
+
+ if (reg & ERR_SYS)
+ dev_dbg(dra7xx->dev, "System Error\n");
+
+ if (reg & ERR_FATAL)
+ dev_dbg(dra7xx->dev, "Fatal Error\n");
+
+ if (reg & ERR_NONFATAL)
+ dev_dbg(dra7xx->dev, "Non Fatal Error\n");
+
+ if (reg & ERR_COR)
+ dev_dbg(dra7xx->dev, "Correctable Error\n");
+
+ if (reg & ERR_AXI)
+ dev_dbg(dra7xx->dev, "AXI tag lookup fatal Error\n");
+
+ if (reg & ERR_ECRC)
+ dev_dbg(dra7xx->dev, "ECRC Error\n");
+
+ if (reg & PME_TURN_OFF)
+ dev_dbg(dra7xx->dev,
+ "Power Management Event Turn-Off message received\n");
+
+ if (reg & PME_TO_ACK)
+ dev_dbg(dra7xx->dev,
+ "Power Management Turn-Off Ack message received\n");
+
+ if (reg & PM_PME)
+ dev_dbg(dra7xx->dev,
+ "PM Power Management Event message received\n");
+
+ if (reg & LINK_REQ_RST)
+ dev_dbg(dra7xx->dev, "Link Request Reset\n");
+
+ if (reg & LINK_UP_EVT)
+ dev_dbg(dra7xx->dev, "Link-up state change\n");
+
+ if (reg & CFG_BME_EVT)
+ dev_dbg(dra7xx->dev, "CFG 'Bus Master Enable' change\n");
+
+ if (reg & CFG_MSE_EVT)
+ dev_dbg(dra7xx->dev, "CFG 'Memory Space Enable' change\n");
+
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN, reg);
+
+ return IRQ_HANDLED;
+}
+
+static int add_pcie_port(struct dra7xx_pcie *dra7xx,
+ struct platform_device *pdev)
+{
+ int ret;
+ struct pcie_port *pp;
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+
+ pp = &dra7xx->pp;
+ pp->dev = dev;
+ pp->ops = &dra7xx_pcie_host_ops;
+
+ pp->irq = platform_get_irq(pdev, 1);
+ if (pp->irq < 0) {
+ dev_err(dev, "missing IRQ resource\n");
+ return -EINVAL;
+ }
+
+ ret = devm_request_irq(&pdev->dev, pp->irq,
+ dra7xx_pcie_msi_irq_handler, IRQF_SHARED,
+ "dra7-pcie-msi", pp);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request irq\n");
+ return ret;
+ }
+
+ if (!IS_ENABLED(CONFIG_PCI_MSI)) {
+ ret = dra7xx_pcie_init_irq_domain(pp);
+ if (ret < 0)
+ return ret;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
+ pp->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!pp->dbi_base)
+ return -ENOMEM;
+
+ ret = dw_pcie_host_init(pp);
+ if (ret) {
+ dev_err(dra7xx->dev, "failed to initialize host\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int __init dra7xx_pcie_probe(struct platform_device *pdev)
+{
+ u32 reg;
+ int ret;
+ int irq;
+ int i;
+ int phy_count;
+ struct phy **phy;
+ void __iomem *base;
+ struct resource *res;
+ struct dra7xx_pcie *dra7xx;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ char name[10];
+
+ dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL);
+ if (!dra7xx)
+ return -ENOMEM;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(dev, "missing IRQ resource\n");
+ return -EINVAL;
+ }
+
+ ret = devm_request_irq(dev, irq, dra7xx_pcie_irq_handler,
+ IRQF_SHARED, "dra7xx-pcie-main", dra7xx);
+ if (ret) {
+ dev_err(dev, "failed to request irq\n");
+ return ret;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
+ base = devm_ioremap_nocache(dev, res->start, resource_size(res));
+ if (!base)
+ return -ENOMEM;
+
+ phy_count = of_property_count_strings(np, "phy-names");
+ if (phy_count < 0) {
+ dev_err(dev, "unable to find the strings\n");
+ return phy_count;
+ }
+
+ phy = devm_kzalloc(dev, sizeof(*phy) * phy_count, GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ for (i = 0; i < phy_count; i++) {
+ snprintf(name, sizeof(name), "pcie-phy%d", i);
+ phy[i] = devm_phy_get(dev, name);
+ if (IS_ERR(phy[i]))
+ return PTR_ERR(phy[i]);
+
+ ret = phy_init(phy[i]);
+ if (ret < 0)
+ goto err_phy;
+
+ ret = phy_power_on(phy[i]);
+ if (ret < 0) {
+ phy_exit(phy[i]);
+ goto err_phy;
+ }
+ }
+
+ dra7xx->base = base;
+ dra7xx->phy = phy;
+ dra7xx->dev = dev;
+ dra7xx->phy_count = phy_count;
+
+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "pm_runtime_get_sync failed\n");
+ goto err_phy;
+ }
+
+ reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
+ reg &= ~LTSSM_EN;
+ dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
+
+ platform_set_drvdata(pdev, dra7xx);
+
+ ret = add_pcie_port(dra7xx, pdev);
+ if (ret < 0)
+ goto err_add_port;
+
+ return 0;
+
+err_add_port:
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+
+err_phy:
+ while (--i >= 0) {
+ phy_power_off(phy[i]);
+ phy_exit(phy[i]);
+ }
+
+ return ret;
+}
+
+static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
+{
+ struct dra7xx_pcie *dra7xx = platform_get_drvdata(pdev);
+ struct pcie_port *pp = &dra7xx->pp;
+ struct device *dev = &pdev->dev;
+ int count = dra7xx->phy_count;
+
+ if (pp->irq_domain)
+ irq_domain_remove(pp->irq_domain);
+ pm_runtime_put(dev);
+ pm_runtime_disable(dev);
+ while (count--) {
+ phy_power_off(dra7xx->phy[count]);
+ phy_exit(dra7xx->phy[count]);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id of_dra7xx_pcie_match[] = {
+ { .compatible = "ti,dra7-pcie", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_dra7xx_pcie_match);
+
+static struct platform_driver dra7xx_pcie_driver = {
+ .remove = __exit_p(dra7xx_pcie_remove),
+ .driver = {
+ .name = "dra7-pcie",
+ .owner = THIS_MODULE,
+ .of_match_table = of_dra7xx_pcie_match,
+ },
+};
+
+module_platform_driver_probe(dra7xx_pcie_driver, dra7xx_pcie_probe);
+
+MODULE_AUTHOR("Kishon Vijay Abraham I <[email protected]>");
+MODULE_DESCRIPTION("TI PCIe controller driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5

2014-06-25 17:57:35

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: [PATCH 1/3] PCI: designware: Configuration space should be specified in 'reg'

The configuration address space has so far been specified in *ranges*,
however it should be specified in *reg* making it a platform MEM resource.
Hence used 'platform_get_resource_*' API to get configuration address
space in the designware driver.

Cc: Jason Gunthorpe <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Mohit Kumar <[email protected]>
Cc: Jingoo Han <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
.../devicetree/bindings/pci/designware-pcie.txt | 4 ++++
drivers/pci/host/pcie-designware.c | 17 +++++++++++++++--
2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d0d15ee..ed0d9b9 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -2,6 +2,10 @@

Required properties:
- compatible: should contain "snps,dw-pcie" to identify the core.
+- reg: Should contain the configuration address space.
+- reg-names: Must be "config" for the PCIe configuration space.
+ (The old way of getting the configuration address space from "ranges"
+ is deprecated and should be avoided.)
- #address-cells: set to <3>
- #size-cells: set to <2>
- device_type: set to "pci"
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 1eaf4df..0b7b455 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -20,6 +20,7 @@
#include <linux/of_pci.h>
#include <linux/pci.h>
#include <linux/pci_regs.h>
+#include <linux/platform_device.h>
#include <linux/types.h>

#include "pcie-designware.h"
@@ -396,11 +397,23 @@ static const struct irq_domain_ops msi_domain_ops = {
int __init dw_pcie_host_init(struct pcie_port *pp)
{
struct device_node *np = pp->dev->of_node;
+ struct platform_device *pdev = to_platform_device(pp->dev);
struct of_pci_range range;
struct of_pci_range_parser parser;
+ struct resource *cfg_res;
u32 val;
int i;

+ cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
+ if (cfg_res) {
+ pp->config.cfg0_size = resource_size(cfg_res)/2;
+ pp->config.cfg1_size = resource_size(cfg_res)/2;
+ pp->cfg0_base = cfg_res->start;
+ pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
+ } else {
+ dev_err(pp->dev, "missing *config* reg space\n");
+ }
+
if (of_pci_range_parser_init(&parser, np)) {
dev_err(pp->dev, "missing ranges property\n");
return -EINVAL;
@@ -433,6 +446,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
of_pci_range_to_resource(&range, np, &pp->cfg);
pp->config.cfg0_size = resource_size(&pp->cfg)/2;
pp->config.cfg1_size = resource_size(&pp->cfg)/2;
+ pp->cfg0_base = pp->cfg.start;
+ pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
}
}

@@ -445,8 +460,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
}
}

- pp->cfg0_base = pp->cfg.start;
- pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
pp->mem_base = pp->mem.start;

pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
--
1.7.9.5

2014-06-26 05:37:41

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Kishon,

Few things, if you can help me to understand:

On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <[email protected]> wrote:
> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
> address. So whenever the cpu issues a read/write request, the 4 most
> significant bits are used by L3 to determine the target controller.
> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
> the outbound translation window the *base* should be programmed as 0x000_0000.
> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
> we have programmed in the translation window with base as 0x000_0000.
>
> This is needed when the dt node is modelled something like below
> axi {
> compatible = "simple-bus";
> #size-cells = <1>;
> #address-cells = <1>;
> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
> 0x51000000 0x51000000 0x3000>;
> pcie@51000000 {
> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
> reg-names = "config", "ti_conf", "rc_dbics";

So for DRA7, config base which will be coming from reg property should
be 0x1000 and size 0x2000, no?

> #address-cells = <3>;
> #size-cells = <2>;
> ranges = <0x81000000 0 0 0x03000 0 0x00010000

range type 0x81000000 tells that it is IO

> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;

range type 0x81000000 tells that it is mem


> };
> };
>
> Here the CPU address for configuration space is 0x20013000 and the controller
> address for configuration space is 0x13000. The controller address should
be

If above understanding is correct then:

Aren't these addresses(0x20013000 and 0x13000) from mem space
instead of configuration space.

If yes, then how can you get two addresses (CPU and Controller address)
from reg property for configuration space?

~Pratyush

2014-06-26 05:56:10

by Mohit KUMAR DCG

[permalink] [raw]
Subject: RE: [PATCH 1/3] PCI: designware: Configuration space should be specified in 'reg'

Hello Kishon,

> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:[email protected]]
> Sent: Wednesday, June 25, 2014 11:27 PM
> To: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; Mohit
> KUMAR DCG; [email protected]
> Cc: [email protected]; [email protected]; Jason Gunthorpe; Marek Vasut;
> Arnd Bergmann
> Subject: [PATCH 1/3] PCI: designware: Configuration space should be
> specified in 'reg'
>
> The configuration address space has so far been specified in *ranges*,
> however it should be specified in *reg* making it a platform MEM resource.
> Hence used 'platform_get_resource_*' API to get configuration address
> space in the designware driver.
>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Mohit Kumar <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Marek Vasut <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> .../devicetree/bindings/pci/designware-pcie.txt | 4 ++++
> drivers/pci/host/pcie-designware.c | 17 +++++++++++++++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index d0d15ee..ed0d9b9 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -2,6 +2,10 @@
>
> Required properties:
> - compatible: should contain "snps,dw-pcie" to identify the core.
> +- reg: Should contain the configuration address space.
> +- reg-names: Must be "config" for the PCIe configuration space.
> + (The old way of getting the configuration address space from "ranges"
> + is deprecated and should be avoided.)
> - #address-cells: set to <3>
> - #size-cells: set to <2>
> - device_type: set to "pci"
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> index 1eaf4df..0b7b455 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -20,6 +20,7 @@
> #include <linux/of_pci.h>
> #include <linux/pci.h>
> #include <linux/pci_regs.h>
> +#include <linux/platform_device.h>
> #include <linux/types.h>
>
> #include "pcie-designware.h"
> @@ -396,11 +397,23 @@ static const struct irq_domain_ops
> msi_domain_ops = { int __init dw_pcie_host_init(struct pcie_port *pp) {
> struct device_node *np = pp->dev->of_node;
> + struct platform_device *pdev = to_platform_device(pp->dev);
> struct of_pci_range range;
> struct of_pci_range_parser parser;
> + struct resource *cfg_res;
> u32 val;
> int i;
>
> + cfg_res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, "config");
> + if (cfg_res) {
> + pp->config.cfg0_size = resource_size(cfg_res)/2;
> + pp->config.cfg1_size = resource_size(cfg_res)/2;
> + pp->cfg0_base = cfg_res->start;
> + pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> + } else {
> + dev_err(pp->dev, "missing *config* reg space\n");

- so this message will remind other platform to comply and specify configuration space
in *reg* property.

> + }
> +
> if (of_pci_range_parser_init(&parser, np)) {
> dev_err(pp->dev, "missing ranges property\n");
> return -EINVAL;
> @@ -433,6 +446,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> of_pci_range_to_resource(&range, np, &pp->cfg);
> pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> + pp->cfg0_base = pp->cfg.start;
> + pp->cfg1_base = pp->cfg.start + pp-
> >config.cfg0_size;
> }
> }
>
> @@ -445,8 +460,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> }
> }
>
> - pp->cfg0_base = pp->cfg.start;
> - pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> pp->mem_base = pp->mem.start;
>
> pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,

Reviewed and Acked-by: Mohit Kumar <[email protected]>

Regards
Mohit

> --
> 1.7.9.5

2014-06-26 06:10:47

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Pratyush,

On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote:
> Hi Kishon,
>
> Few things, if you can help me to understand:
>
> On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
>> address. So whenever the cpu issues a read/write request, the 4 most
>> significant bits are used by L3 to determine the target controller.
>> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
>> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
>> the outbound translation window the *base* should be programmed as 0x000_0000.
>> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
>> we have programmed in the translation window with base as 0x000_0000.
>>
>> This is needed when the dt node is modelled something like below
>> axi {
>> compatible = "simple-bus";
>> #size-cells = <1>;
>> #address-cells = <1>;
>> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
>> 0x51000000 0x51000000 0x3000>;
>> pcie@51000000 {
>> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
>> reg-names = "config", "ti_conf", "rc_dbics";
>
> So for DRA7, config base which will be coming from reg property should
> be 0x1000 and size 0x2000, no?

right. The first element in 'reg' and 'reg-names' specify exactly that.
>
>> #address-cells = <3>;
>> #size-cells = <2>;
>> ranges = <0x81000000 0 0 0x03000 0 0x00010000
>
> range type 0x81000000 tells that it is IO
>
>> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
>
> range type 0x81000000 tells that it is mem
>
>
>> };
>> };
>>
>> Here the CPU address for configuration space is 0x20013000 and the controller
>> address for configuration space is 0x13000. The controller address should
> be
>
> If above understanding is correct then:
>
> Aren't these addresses(0x20013000 and 0x13000) from mem space
> instead of configuration space.

Sorry. I didn't get you. Configuration space is different from mem space and IO
space. We specify only the configuration space in "reg", the IO space and
memory space should be specified in ranges.

In my case
configuration space range: 0x20001000 - 0x20002fff
IO space range: 0x20003000 - 0x20012fff
Mem space range: 0x20013000 - 0x2fffffff

Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM'
space is obtained from ranges.
>
> If yes, then how can you get two addresses (CPU and Controller address)
> from reg property for configuration space?

I used platform_get_resource_byname() to get CPU address and of_get_address()
to get the untranslated controller address.

Thanks
Kishon

2014-06-26 06:34:10

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Kishon,

On Thu, Jun 26, 2014 at 02:10:02PM +0800, Kishon Vijay Abraham I wrote:
> Hi Pratyush,
>
> On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote:
> > Hi Kishon,
> >
> > Few things, if you can help me to understand:
> >
> > On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <[email protected]> wrote:
> >> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
> >> address. So whenever the cpu issues a read/write request, the 4 most
> >> significant bits are used by L3 to determine the target controller.
> >> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
> >> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
> >> the outbound translation window the *base* should be programmed as 0x000_0000.
> >> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
> >> we have programmed in the translation window with base as 0x000_0000.
> >>
> >> This is needed when the dt node is modelled something like below
> >> axi {
> >> compatible = "simple-bus";
> >> #size-cells = <1>;
> >> #address-cells = <1>;
> >> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
> >> 0x51000000 0x51000000 0x3000>;
> >> pcie@51000000 {
> >> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
> >> reg-names = "config", "ti_conf", "rc_dbics";
> >
> > So for DRA7, config base which will be coming from reg property should
> > be 0x1000 and size 0x2000, no?
>
> right. The first element in 'reg' and 'reg-names' specify exactly that.
> >
> >> #address-cells = <3>;
> >> #size-cells = <2>;
> >> ranges = <0x81000000 0 0 0x03000 0 0x00010000
> >
> > range type 0x81000000 tells that it is IO
> >
> >> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
> >
> > range type 0x81000000 tells that it is mem
> >
> >
> >> };
> >> };
> >>
> >> Here the CPU address for configuration space is 0x20013000 and the controller
> >> address for configuration space is 0x13000. The controller address should
> > be
> >
> > If above understanding is correct then:
> >
> > Aren't these addresses(0x20013000 and 0x13000) from mem space
> > instead of configuration space.
>
> Sorry. I didn't get you. Configuration space is different from mem space and IO
> space. We specify only the configuration space in "reg", the IO space and
> memory space should be specified in ranges.
>
> In my case
> configuration space range: 0x20001000 - 0x20002fff
> IO space range: 0x20003000 - 0x20012fff
> Mem space range: 0x20013000 - 0x2fffffff
>
> Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM'
> space is obtained from ranges.
> >
> > If yes, then how can you get two addresses (CPU and Controller address)
> > from reg property for configuration space?
>
> I used platform_get_resource_byname() to get CPU address and of_get_address()
> to get the untranslated controller address.

This is what I am not able to understand that how does
platform_get_resource_byname gives correct CPU address from reg =
<0x1000 0x2000>?

cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");

how cfg_res->start is now 0x20001000? Shouldn't cfg_res->start be
0x1000?

What am I missing?

Thanks for explaining.

~Pratyush



> Thanks
> Kishon

2014-06-26 08:25:09

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Pratyush,

On Thursday 26 June 2014 12:03 PM, Pratyush Anand wrote:
> Hi Kishon,
>
> On Thu, Jun 26, 2014 at 02:10:02PM +0800, Kishon Vijay Abraham I wrote:
>> Hi Pratyush,
>>
>> On Thursday 26 June 2014 11:07 AM, Pratyush Anand wrote:
>>> Hi Kishon,
>>>
>>> Few things, if you can help me to understand:
>>>
>>> On Wed, Jun 25, 2014 at 11:26 PM, Kishon Vijay Abraham I <[email protected]> wrote:
>>>> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
>>>> address. So whenever the cpu issues a read/write request, the 4 most
>>>> significant bits are used by L3 to determine the target controller.
>>>> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
>>>> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
>>>> the outbound translation window the *base* should be programmed as 0x000_0000.
>>>> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
>>>> we have programmed in the translation window with base as 0x000_0000.
>>>>
>>>> This is needed when the dt node is modelled something like below
>>>> axi {
>>>> compatible = "simple-bus";
>>>> #size-cells = <1>;
>>>> #address-cells = <1>;
>>>> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
>>>> 0x51000000 0x51000000 0x3000>;
>>>> pcie@51000000 {
>>>> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
>>>> reg-names = "config", "ti_conf", "rc_dbics";
>>>
>>> So for DRA7, config base which will be coming from reg property should
>>> be 0x1000 and size 0x2000, no?
>>
>> right. The first element in 'reg' and 'reg-names' specify exactly that.
>>>
>>>> #address-cells = <3>;
>>>> #size-cells = <2>;
>>>> ranges = <0x81000000 0 0 0x03000 0 0x00010000
>>>
>>> range type 0x81000000 tells that it is IO
>>>
>>>> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
>>>
>>> range type 0x81000000 tells that it is mem
>>>
>>>
>>>> };
>>>> };
>>>>
>>>> Here the CPU address for configuration space is 0x20013000 and the controller
>>>> address for configuration space is 0x13000. The controller address should
>>> be
>>>
>>> If above understanding is correct then:
>>>
>>> Aren't these addresses(0x20013000 and 0x13000) from mem space
>>> instead of configuration space.
>>
>> Sorry. I didn't get you. Configuration space is different from mem space and IO
>> space. We specify only the configuration space in "reg", the IO space and
>> memory space should be specified in ranges.
>>
>> In my case
>> configuration space range: 0x20001000 - 0x20002fff
>> IO space range: 0x20003000 - 0x20012fff
>> Mem space range: 0x20013000 - 0x2fffffff
>>
>> Here only the configuration space is obtained from 'reg' and 'IO' and 'MEM'
>> space is obtained from ranges.
>>>
>>> If yes, then how can you get two addresses (CPU and Controller address)
>>> from reg property for configuration space?
>>
>> I used platform_get_resource_byname() to get CPU address and of_get_address()
>> to get the untranslated controller address.
>
> This is what I am not able to understand that how does
> platform_get_resource_byname gives correct CPU address from reg =
> <0x1000 0x2000>?
>
> cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>
> how cfg_res->start is now 0x20001000? Shouldn't cfg_res->start be
> 0x1000?
>
> What am I missing?

IIUC, converting to the cpu address happens long back when the platform device
is created from dt node.

See if the following makes sense to you.

starting from drivers/of/platform.c
of_device_alloc() -> of_address_to_resource() -> __of_address_to_resource() ->
of_translate_address() -> __of_translate_address() -> of_translate_one() ->
of_bus_default_translate()

So the 'resource' in 'struct platform_device' already holds the translated
address. platform_get_resource_byname() just returns the resource from this
platform device.

Cheers
Kishon

2014-07-05 17:32:09

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/3] dra7: Add PCIe support

On Wed, Jun 25, 2014 at 11:26:44PM +0530, Kishon Vijay Abraham I wrote:
> [1] is split into separate series in order for individual subsystem
> Maintainers to pick up the patches. This series handles the PCIe
> support for DRA7.
>
> Rebased to 3.16-rc2.
>
> [1] -> https://lkml.org/lkml/2014/5/29/258
>
> Kishon Vijay Abraham I (3):
> PCI: designware: Configuration space should be specified in 'reg'
> PCI: designware: use untranslated address while programming ATU
> PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller

Mohit, I see your ack for [1/3], but not for [2/3]; are you OK with that,
too?

Pratyush, you had some questions about [2/3]; are you happy with that one?

[3/3] adds the devicetree binding; I'd like somebody to check that out.

Also, there's no MAINTAINERS update for pci-dra7xx.c. Kishon, can you
include an update for that?

Kishon, can you collect the acks and post a v2 series with those and the
MAINTAINERS update?

Bjorn

>
> .../devicetree/bindings/pci/designware-pcie.txt | 4 +
> Documentation/devicetree/bindings/pci/ti-pci.txt | 59 +++
> drivers/pci/host/Kconfig | 10 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++
> drivers/pci/host/pcie-designware.c | 66 ++-
> drivers/pci/host/pcie-designware.h | 4 +
> 7 files changed, 589 insertions(+), 13 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
> create mode 100644 drivers/pci/host/pci-dra7xx.c
>
> --
> 1.7.9.5
>

2014-07-08 03:47:18

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH 0/3] dra7: Add PCIe support

On Sat, Jul 5, 2014 at 11:02 PM, Bjorn Helgaas <[email protected]> wrote:
> On Wed, Jun 25, 2014 at 11:26:44PM +0530, Kishon Vijay Abraham I wrote:
>> [1] is split into separate series in order for individual subsystem
>> Maintainers to pick up the patches. This series handles the PCIe
>> support for DRA7.
>>
>> Rebased to 3.16-rc2.
>>
>> [1] -> https://lkml.org/lkml/2014/5/29/258
>>
>> Kishon Vijay Abraham I (3):
>> PCI: designware: Configuration space should be specified in 'reg'
>> PCI: designware: use untranslated address while programming ATU
>> PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller
>
> Mohit, I see your ack for [1/3], but not for [2/3]; are you OK with that,
> too?
>
> Pratyush, you had some questions about [2/3]; are you happy with that one?

OK for me.

~Pratyush

2014-07-08 12:32:04

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Mohit, Jingoo,

On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
> address. So whenever the cpu issues a read/write request, the 4 most
> significant bits are used by L3 to determine the target controller.
> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
> the outbound translation window the *base* should be programmed as 0x000_0000.
> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
> we have programmed in the translation window with base as 0x000_0000.
>
> This is needed when the dt node is modelled something like below
> axi {
> compatible = "simple-bus";
> #size-cells = <1>;
> #address-cells = <1>;
> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
> 0x51000000 0x51000000 0x3000>;
> pcie@51000000 {
> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
> reg-names = "config", "ti_conf", "rc_dbics";
> #address-cells = <3>;
> #size-cells = <2>;
> ranges = <0x81000000 0 0 0x03000 0 0x00010000
> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
> };
> };
>
> Here the CPU address for configuration space is 0x20013000 and the controller
> address for configuration space is 0x13000. The controller address should be
> used while programming the ATU (in order for translation to happen properly in
> DRA7xx).

Are you okay with this patch? Can you give your Reviewed-by/Acked-by?

Cheers
Kishon
>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Mohit Kumar <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Marek Vasut <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++--------
> drivers/pci/host/pcie-designware.h | 4 +++
> 2 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 0b7b455..11aba6d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> struct of_pci_range range;
> struct of_pci_range_parser parser;
> struct resource *cfg_res;
> - u32 val;
> - int i;
> + u32 val, na, ns;
> + const __be32 *addrp;
> + int i, index;
> +
> + /* Find the address cell size and the number of cells in order to get
> + * the untranslated address.
> + */
> + of_property_read_u32(np, "#address-cells", &na);
> + ns = of_n_size_cells(np);
>
> cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> if (cfg_res) {
> @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> pp->config.cfg1_size = resource_size(cfg_res)/2;
> pp->cfg0_base = cfg_res->start;
> pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> +
> + /* Find the untranslated configuration space address */
> + index = of_property_match_string(np, "reg-names", "config");
> + addrp = of_get_address(np, index, false, false);
> + pp->cfg0_mod_addr = of_read_number(addrp, ns);
> + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size;
> } else {
> dev_err(pp->dev, "missing *config* reg space\n");
> }
> @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> pp->config.io_size = resource_size(&pp->io);
> pp->config.io_bus_addr = range.pci_addr;
> pp->io_base = range.cpu_addr;
> +
> + /* Find the untranslated IO space address */
> + pp->io_mod_addr = of_read_number(parser.range -
> + parser.np + na, ns);
> }
> if (restype == IORESOURCE_MEM) {
> of_pci_range_to_resource(&range, np, &pp->mem);
> pp->mem.name = "MEM";
> pp->config.mem_size = resource_size(&pp->mem);
> pp->config.mem_bus_addr = range.pci_addr;
> +
> + /* Find the untranslated MEM space address */
> + pp->mem_mod_addr = of_read_number(parser.range -
> + parser.np + na, ns);
> }
> if (restype == 0) {
> of_pci_range_to_resource(&range, np, &pp->cfg);
> @@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> pp->cfg0_base = pp->cfg.start;
> pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> +
> + /* Find the untranslated configuration space address */
> + pp->cfg0_mod_addr = of_read_number(parser.range -
> + parser.np + na, ns);
> + pp->cfg1_mod_addr = pp->cfg0_mod_addr +
> + pp->config.cfg0_size;
> }
> }
>
> @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
> /* Program viewport 0 : OUTBOUND : CFG0 */
> dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
> PCIE_ATU_VIEWPORT);
> - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE);
> - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE);
> - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
> + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE);
> + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1,
> PCIE_ATU_LIMIT);
> dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
> @@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
> dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1,
> PCIE_ATU_VIEWPORT);
> dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1);
> - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE);
> - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE);
> - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
> + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE);
> + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1,
> PCIE_ATU_LIMIT);
> dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
> @@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
> dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
> PCIE_ATU_VIEWPORT);
> dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1);
> - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE);
> - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE);
> - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
> + dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE);
> + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1,
> PCIE_ATU_LIMIT);
> dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET);
> dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 77f592f..13828f8 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -36,11 +36,15 @@ struct pcie_port {
> u8 root_bus_nr;
> void __iomem *dbi_base;
> u64 cfg0_base;
> + u64 cfg0_mod_addr;
> void __iomem *va_cfg0_base;
> u64 cfg1_base;
> + u64 cfg1_mod_addr;
> void __iomem *va_cfg1_base;
> u64 io_base;
> + u64 io_mod_addr;
> u64 mem_base;
> + u64 mem_mod_addr;
> struct resource cfg;
> struct resource io;
> struct resource mem;
>

2014-07-08 12:35:20

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 3/3] PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller

Hi Arnd, Tony, other dt guys,

On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
> Added support for pcie controller in dra7xx. This driver re-uses
> the designware core code that is already present in kernel.
>

Are you okay with this patch? an you give your Acked-by?

Thanks
Kishon
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Mohit Kumar <[email protected]>
> Cc: Jingoo Han <[email protected]>
> Cc: Marek Vasut <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/ti-pci.txt | 59 +++
> drivers/pci/host/Kconfig | 10 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++++
> 4 files changed, 528 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
> create mode 100644 drivers/pci/host/pci-dra7xx.c
>
> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> new file mode 100644
> index 0000000..3d21791
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> @@ -0,0 +1,59 @@
> +TI PCI Controllers
> +
> +PCIe Designware Controller
> + - compatible: Should be "ti,dra7-pcie""
> + - reg : Two register ranges as listed in the reg-names property
> + - reg-names : The first entry must be "ti-conf" for the TI specific registers
> + The second entry must be "rc-dbics" for the designware pcie
> + registers
> + The third entry must be "config" for the PCIe configuration space
> + - phys : list of PHY specifiers (used by generic PHY framework)
> + - phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
> + number of PHYs as specified in *phys* property.
> + - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
> + where <X> is the instance number of the pcie from the HW spec.
> + - interrupts : Two interrupt entries must be specified. The first one is for
> + main interrupt line and the second for MSI interrupt line.
> + - #address-cells,
> + #size-cells,
> + #interrupt-cells,
> + device_type,
> + ranges,
> + num-lanes,
> + interrupt-map-mask,
> + interrupt-map : as specified in ../designware-pcie.txt
> +
> +Example:
> +axi {
> + compatible = "simple-bus";
> + #size-cells = <1>;
> + #address-cells = <1>;
> + ranges = <0x51000000 0x51000000 0x3000
> + 0x0 0x20000000 0x10000000>;
> + pcie@51000000 {
> + compatible = "ti,dra7-pcie";
> + reg = <0x51000000 0x2000>, <0x51002000 0x14c>, <0x1000 0x2000>;
> + reg-names = "rc_dbics", "ti_conf", "config";
> + interrupts = <0 232 0x4>, <0 233 0x4>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + ranges = <0x81000000 0 0 0x03000 0 0x00010000
> + 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
> + #interrupt-cells = <1>;
> + num-lanes = <1>;
> + ti,hwmods = "pcie1";
> + phys = <&pcie1_phy>;
> + phy-names = "pcie-phy0";
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
> + <0 0 0 2 &pcie_intc 2>,
> + <0 0 0 3 &pcie_intc 3>,
> + <0 0 0 4 &pcie_intc 4>;
> + pcie_intc: interrupt-controller {
> + interrupt-controller;
> + #address-cells = <0>;
> + #interrupt-cells = <1>;
> + };
> + };
> +};
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 21df477..22117b0 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -1,6 +1,16 @@
> menu "PCI host controller drivers"
> depends on PCI
>
> +config PCI_DRA7XX
> + bool "TI DRA7xx PCIe controller"
> + select PCIE_DW
> + depends on OF && HAS_IOMEM && TI_PIPE3
> + help
> + Enables support for the PCIE controller present in DRA7xx SoC. There
> + are two instances of PCIE controller in DRA7xx. This controller can
> + act both as EP and RC. This reuses the same Designware core as used
> + by other SoCs.
> +
> config PCI_MVEBU
> bool "Marvell EBU PCIe controller"
> depends on ARCH_MVEBU || ARCH_DOVE || ARCH_KIRKWOOD
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 611ba4b..c42844d 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> +obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> new file mode 100644
> index 0000000..52b34fe
> --- /dev/null
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -0,0 +1,458 @@
> +/*
> + * pcie-dra7xx - PCIe controller driver for TI DRA7xx SoCs
> + *
> + * Copyright (C) 2013-2014 Texas Instruments Incorporated - http://www.ti.com
> + *
> + * Authors: Kishon Vijay Abraham I <[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/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +/* PCIe controller wrapper DRA7XX configuration registers */
> +
> +#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN 0x0024
> +#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN 0x0028
> +#define ERR_SYS BIT(0)
> +#define ERR_FATAL BIT(1)
> +#define ERR_NONFATAL BIT(2)
> +#define ERR_COR BIT(3)
> +#define ERR_AXI BIT(4)
> +#define ERR_ECRC BIT(5)
> +#define PME_TURN_OFF BIT(8)
> +#define PME_TO_ACK BIT(9)
> +#define PM_PME BIT(10)
> +#define LINK_REQ_RST BIT(11)
> +#define LINK_UP_EVT BIT(12)
> +#define CFG_BME_EVT BIT(13)
> +#define CFG_MSE_EVT BIT(14)
> +#define INTERRUPTS (ERR_SYS | ERR_FATAL | ERR_NONFATAL | ERR_COR | ERR_AXI | \
> + ERR_ECRC | PME_TURN_OFF | PME_TO_ACK | PM_PME | \
> + LINK_REQ_RST | LINK_UP_EVT | CFG_BME_EVT | CFG_MSE_EVT)
> +
> +#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI 0x0034
> +#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI 0x0038
> +#define INTA BIT(0)
> +#define INTB BIT(1)
> +#define INTC BIT(2)
> +#define INTD BIT(3)
> +#define MSI BIT(4)
> +#define LEG_EP_INTERRUPTS (INTA | INTB | INTC | INTD)
> +
> +#define PCIECTRL_DRA7XX_CONF_DEVICE_CMD 0x0104
> +#define LTSSM_EN 0x1
> +
> +#define PCIECTRL_DRA7XX_CONF_PHY_CS 0x010C
> +#define LINK_UP BIT(16)
> +
> +struct dra7xx_pcie {
> + void __iomem *base;
> + struct phy **phy;
> + int phy_count;
> + struct device *dev;
> + struct pcie_port pp;
> +};
> +
> +#define to_dra7xx_pcie(x) container_of((x), struct dra7xx_pcie, pp)
> +
> +static inline u32 dra7xx_pcie_readl(struct dra7xx_pcie *pcie, u32 offset)
> +{
> + return readl(pcie->base + offset);
> +}
> +
> +static inline void dra7xx_pcie_writel(struct dra7xx_pcie *pcie, u32 offset,
> + u32 value)
> +{
> + writel(value, pcie->base + offset);
> +}
> +
> +static int dra7xx_pcie_link_up(struct pcie_port *pp)
> +{
> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> + u32 reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
> +
> + return !!(reg & LINK_UP);
> +}
> +
> +static int dra7xx_pcie_establish_link(struct pcie_port *pp)
> +{
> + u32 reg;
> + unsigned int retries = 1000;
> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> +
> + if (dw_pcie_link_up(pp)) {
> + dev_err(pp->dev, "link is already up\n");
> + return 0;
> + }
> +
> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
> + reg |= LTSSM_EN;
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> +
> + while (retries--) {
> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
> + if (reg & LINK_UP)
> + break;
> + usleep_range(10, 20);
> + }
> +
> + if (retries == 0) {
> + dev_err(pp->dev, "link is not up\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
> +{
> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> +
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
> + ~INTERRUPTS);
> + dra7xx_pcie_writel(dra7xx,
> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN, INTERRUPTS);
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
> + ~LEG_EP_INTERRUPTS & ~MSI);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + dra7xx_pcie_writel(dra7xx,
> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI, MSI);
> + else
> + dra7xx_pcie_writel(dra7xx,
> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI,
> + LEG_EP_INTERRUPTS);
> +}
> +
> +static void dra7xx_pcie_host_init(struct pcie_port *pp)
> +{
> + dw_pcie_setup_rc(pp);
> + dra7xx_pcie_establish_link(pp);
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + dw_pcie_msi_init(pp);
> + dra7xx_pcie_enable_interrupts(pp);
> +}
> +
> +static struct pcie_host_ops dra7xx_pcie_host_ops = {
> + .link_up = dra7xx_pcie_link_up,
> + .host_init = dra7xx_pcie_host_init,
> +};
> +
> +static int dra7xx_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> + irq_set_chip_data(irq, domain->host_data);
> + set_irq_flags(irq, IRQF_VALID);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops intx_domain_ops = {
> + .map = dra7xx_pcie_intx_map,
> +};
> +
> +static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
> +{
> + struct device *dev = pp->dev;
> + struct device_node *node = dev->of_node;
> + struct device_node *pcie_intc_node = of_get_next_child(node, NULL);
> +
> + if (!pcie_intc_node) {
> + dev_err(dev, "No PCIe Intc node found\n");
> + return PTR_ERR(pcie_intc_node);
> + }
> +
> + pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
> + &intx_domain_ops, pp);
> + if (!pp->irq_domain) {
> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> + return PTR_ERR(pp->irq_domain);
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
> +{
> + struct pcie_port *pp = arg;
> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
> + u32 reg;
> +
> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
> +
> + switch (reg) {
> + case MSI:
> + dw_handle_msi_irq(pp);
> + break;
> + case INTA:
> + case INTB:
> + case INTC:
> + case INTD:
> + generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg)));
> + break;
> + }
> +
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
> +
> + return IRQ_HANDLED;
> +}
> +
> +
> +static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
> +{
> + struct dra7xx_pcie *dra7xx = arg;
> + u32 reg;
> +
> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN);
> +
> + if (reg & ERR_SYS)
> + dev_dbg(dra7xx->dev, "System Error\n");
> +
> + if (reg & ERR_FATAL)
> + dev_dbg(dra7xx->dev, "Fatal Error\n");
> +
> + if (reg & ERR_NONFATAL)
> + dev_dbg(dra7xx->dev, "Non Fatal Error\n");
> +
> + if (reg & ERR_COR)
> + dev_dbg(dra7xx->dev, "Correctable Error\n");
> +
> + if (reg & ERR_AXI)
> + dev_dbg(dra7xx->dev, "AXI tag lookup fatal Error\n");
> +
> + if (reg & ERR_ECRC)
> + dev_dbg(dra7xx->dev, "ECRC Error\n");
> +
> + if (reg & PME_TURN_OFF)
> + dev_dbg(dra7xx->dev,
> + "Power Management Event Turn-Off message received\n");
> +
> + if (reg & PME_TO_ACK)
> + dev_dbg(dra7xx->dev,
> + "Power Management Turn-Off Ack message received\n");
> +
> + if (reg & PM_PME)
> + dev_dbg(dra7xx->dev,
> + "PM Power Management Event message received\n");
> +
> + if (reg & LINK_REQ_RST)
> + dev_dbg(dra7xx->dev, "Link Request Reset\n");
> +
> + if (reg & LINK_UP_EVT)
> + dev_dbg(dra7xx->dev, "Link-up state change\n");
> +
> + if (reg & CFG_BME_EVT)
> + dev_dbg(dra7xx->dev, "CFG 'Bus Master Enable' change\n");
> +
> + if (reg & CFG_MSE_EVT)
> + dev_dbg(dra7xx->dev, "CFG 'Memory Space Enable' change\n");
> +
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN, reg);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int add_pcie_port(struct dra7xx_pcie *dra7xx,
> + struct platform_device *pdev)
> +{
> + int ret;
> + struct pcie_port *pp;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> +
> + pp = &dra7xx->pp;
> + pp->dev = dev;
> + pp->ops = &dra7xx_pcie_host_ops;
> +
> + pp->irq = platform_get_irq(pdev, 1);
> + if (pp->irq < 0) {
> + dev_err(dev, "missing IRQ resource\n");
> + return -EINVAL;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, pp->irq,
> + dra7xx_pcie_msi_irq_handler, IRQF_SHARED,
> + "dra7-pcie-msi", pp);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request irq\n");
> + return ret;
> + }
> +
> + if (!IS_ENABLED(CONFIG_PCI_MSI)) {
> + ret = dra7xx_pcie_init_irq_domain(pp);
> + if (ret < 0)
> + return ret;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
> + pp->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
> + if (!pp->dbi_base)
> + return -ENOMEM;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(dra7xx->dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int __init dra7xx_pcie_probe(struct platform_device *pdev)
> +{
> + u32 reg;
> + int ret;
> + int irq;
> + int i;
> + int phy_count;
> + struct phy **phy;
> + void __iomem *base;
> + struct resource *res;
> + struct dra7xx_pcie *dra7xx;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + char name[10];
> +
> + dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL);
> + if (!dra7xx)
> + return -ENOMEM;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "missing IRQ resource\n");
> + return -EINVAL;
> + }
> +
> + ret = devm_request_irq(dev, irq, dra7xx_pcie_irq_handler,
> + IRQF_SHARED, "dra7xx-pcie-main", dra7xx);
> + if (ret) {
> + dev_err(dev, "failed to request irq\n");
> + return ret;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
> + base = devm_ioremap_nocache(dev, res->start, resource_size(res));
> + if (!base)
> + return -ENOMEM;
> +
> + phy_count = of_property_count_strings(np, "phy-names");
> + if (phy_count < 0) {
> + dev_err(dev, "unable to find the strings\n");
> + return phy_count;
> + }
> +
> + phy = devm_kzalloc(dev, sizeof(*phy) * phy_count, GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + for (i = 0; i < phy_count; i++) {
> + snprintf(name, sizeof(name), "pcie-phy%d", i);
> + phy[i] = devm_phy_get(dev, name);
> + if (IS_ERR(phy[i]))
> + return PTR_ERR(phy[i]);
> +
> + ret = phy_init(phy[i]);
> + if (ret < 0)
> + goto err_phy;
> +
> + ret = phy_power_on(phy[i]);
> + if (ret < 0) {
> + phy_exit(phy[i]);
> + goto err_phy;
> + }
> + }
> +
> + dra7xx->base = base;
> + dra7xx->phy = phy;
> + dra7xx->dev = dev;
> + dra7xx->phy_count = phy_count;
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (IS_ERR_VALUE(ret)) {
> + dev_err(dev, "pm_runtime_get_sync failed\n");
> + goto err_phy;
> + }
> +
> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
> + reg &= ~LTSSM_EN;
> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> +
> + platform_set_drvdata(pdev, dra7xx);
> +
> + ret = add_pcie_port(dra7xx, pdev);
> + if (ret < 0)
> + goto err_add_port;
> +
> + return 0;
> +
> +err_add_port:
> + pm_runtime_put(dev);
> + pm_runtime_disable(dev);
> +
> +err_phy:
> + while (--i >= 0) {
> + phy_power_off(phy[i]);
> + phy_exit(phy[i]);
> + }
> +
> + return ret;
> +}
> +
> +static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
> +{
> + struct dra7xx_pcie *dra7xx = platform_get_drvdata(pdev);
> + struct pcie_port *pp = &dra7xx->pp;
> + struct device *dev = &pdev->dev;
> + int count = dra7xx->phy_count;
> +
> + if (pp->irq_domain)
> + irq_domain_remove(pp->irq_domain);
> + pm_runtime_put(dev);
> + pm_runtime_disable(dev);
> + while (count--) {
> + phy_power_off(dra7xx->phy[count]);
> + phy_exit(dra7xx->phy[count]);
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_dra7xx_pcie_match[] = {
> + { .compatible = "ti,dra7-pcie", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_dra7xx_pcie_match);
> +
> +static struct platform_driver dra7xx_pcie_driver = {
> + .remove = __exit_p(dra7xx_pcie_remove),
> + .driver = {
> + .name = "dra7-pcie",
> + .owner = THIS_MODULE,
> + .of_match_table = of_dra7xx_pcie_match,
> + },
> +};
> +
> +module_platform_driver_probe(dra7xx_pcie_driver, dra7xx_pcie_probe);
> +
> +MODULE_AUTHOR("Kishon Vijay Abraham I <[email protected]>");
> +MODULE_DESCRIPTION("TI PCIe controller driver");
> +MODULE_LICENSE("GPL v2");
>

2014-07-08 12:50:57

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH 1/3] PCI: designware: Configuration space should be specified in 'reg'

On Thursday, June 26, 2014 2:56 PM, Mohit KUMAR DCG wrote:
> On Wednesday, June 25, 2014 11:27 PM, Kishon Vijay Abraham I wrote:
> >
> > The configuration address space has so far been specified in *ranges*,
> > however it should be specified in *reg* making it a platform MEM resource.
> > Hence used 'platform_get_resource_*' API to get configuration address
> > space in the designware driver.
> >
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Mohit Kumar <[email protected]>
> > Cc: Jingoo Han <[email protected]>
> > Cc: Marek Vasut <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> > ---
> > .../devicetree/bindings/pci/designware-pcie.txt | 4 ++++
> > drivers/pci/host/pcie-designware.c | 17 +++++++++++++++--
> > 2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > index d0d15ee..ed0d9b9 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > @@ -2,6 +2,10 @@
> >
> > Required properties:
> > - compatible: should contain "snps,dw-pcie" to identify the core.
> > +- reg: Should contain the configuration address space.
> > +- reg-names: Must be "config" for the PCIe configuration space.
> > + (The old way of getting the configuration address space from "ranges"
> > + is deprecated and should be avoided.)
> > - #address-cells: set to <3>
> > - #size-cells: set to <2>
> > - device_type: set to "pci"
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> > designware.c
> > index 1eaf4df..0b7b455 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -20,6 +20,7 @@
> > #include <linux/of_pci.h>
> > #include <linux/pci.h>
> > #include <linux/pci_regs.h>
> > +#include <linux/platform_device.h>
> > #include <linux/types.h>
> >
> > #include "pcie-designware.h"
> > @@ -396,11 +397,23 @@ static const struct irq_domain_ops
> > msi_domain_ops = { int __init dw_pcie_host_init(struct pcie_port *pp) {
> > struct device_node *np = pp->dev->of_node;
> > + struct platform_device *pdev = to_platform_device(pp->dev);
> > struct of_pci_range range;
> > struct of_pci_range_parser parser;
> > + struct resource *cfg_res;
> > u32 val;
> > int i;
> >
> > + cfg_res = platform_get_resource_byname(pdev,
> > IORESOURCE_MEM, "config");
> > + if (cfg_res) {
> > + pp->config.cfg0_size = resource_size(cfg_res)/2;
> > + pp->config.cfg1_size = resource_size(cfg_res)/2;
> > + pp->cfg0_base = cfg_res->start;
> > + pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> > + } else {
> > + dev_err(pp->dev, "missing *config* reg space\n");
>
> - so this message will remind other platform to comply and specify configuration space
> in *reg* property.
>
> > + }
> > +
> > if (of_pci_range_parser_init(&parser, np)) {
> > dev_err(pp->dev, "missing ranges property\n");
> > return -EINVAL;
> > @@ -433,6 +446,8 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > of_pci_range_to_resource(&range, np, &pp->cfg);
> > pp->config.cfg0_size = resource_size(&pp->cfg)/2;
> > pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > + pp->cfg0_base = pp->cfg.start;
> > + pp->cfg1_base = pp->cfg.start + pp-
> > >config.cfg0_size;
> > }
> > }
> >
> > @@ -445,8 +460,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > }
> > }
> >
> > - pp->cfg0_base = pp->cfg.start;
> > - pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> > pp->mem_base = pp->mem.start;
> >
> > pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>
> Reviewed and Acked-by: Mohit Kumar <[email protected]>

Acked-by: Jingoo Han <[email protected]>

Best regards,
Jingoo Han

>
> Regards
> Mohit
>
> > --
> > 1.7.9.5

2014-07-08 13:17:42

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

On Tuesday, July 08, 2014 9:31 PM, Kishon Vijay Abraham I wrote:
>
> Hi Mohit, Jingoo,
>
> On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
> > In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
> > address. So whenever the cpu issues a read/write request, the 4 most
> > significant bits are used by L3 to determine the target controller.
> > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
> > the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
> > the outbound translation window the *base* should be programmed as 0x000_0000.
> > Whenever we try to write to say 0x2000_0000, it will be translated to whatever
> > we have programmed in the translation window with base as 0x000_0000.
> >
> > This is needed when the dt node is modelled something like below
> > axi {
> > compatible = "simple-bus";
> > #size-cells = <1>;
> > #address-cells = <1>;
> > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
> > 0x51000000 0x51000000 0x3000>;
> > pcie@51000000 {
> > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
> > reg-names = "config", "ti_conf", "rc_dbics";
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges = <0x81000000 0 0 0x03000 0 0x00010000
> > 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
> > };
> > };
> >
> > Here the CPU address for configuration space is 0x20013000 and the controller
> > address for configuration space is 0x13000. The controller address should be
> > used while programming the ATU (in order for translation to happen properly in
> > DRA7xx).
>
> Are you okay with this patch? Can you give your Reviewed-by/Acked-by?

Sorry, I cannot understand fully what this patch does. :-(
However, I have some questions as below.

1. io_mod_addr
The 'io_mod_addr' variable is not used. Then, how about removing
'io_mod_addr' from this patch?

2. ranges
In the case of Exynos PCIe, there is no limitation such as 28bit for
access. Then, when there is no 'ranges' property in DT file, is there
no side effect? I mean the compatibility with other SoC's PCIe drivers.

3. _addr post fix

How about renaming 'cfg0_mod_addr', 'mem_mod_addr' as below?

u64 cfg0_base;
+ u64 cfg0_mod_base;
void __iomem *va_cfg0_base;
u64 cfg1_base;
+ u64 cfg1_mod_base;
void __iomem *va_cfg1_base;
u64 io_base;
+ u64 io_mod_base;
u64 mem_base;
+ u64 mem_mod_base;


Best regards,
Jingoo Han

>
> Cheers
> Kishon
> >
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Mohit Kumar <[email protected]>
> > Cc: Jingoo Han <[email protected]>
> > Cc: Marek Vasut <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> > ---
> > drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++--------
> > drivers/pci/host/pcie-designware.h | 4 +++
> > 2 files changed, 42 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 0b7b455..11aba6d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > struct of_pci_range range;
> > struct of_pci_range_parser parser;
> > struct resource *cfg_res;
> > - u32 val;
> > - int i;
> > + u32 val, na, ns;
> > + const __be32 *addrp;
> > + int i, index;
> > +
> > + /* Find the address cell size and the number of cells in order to get
> > + * the untranslated address.
> > + */
> > + of_property_read_u32(np, "#address-cells", &na);
> > + ns = of_n_size_cells(np);
> >
> > cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> > if (cfg_res) {
> > @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > pp->config.cfg1_size = resource_size(cfg_res)/2;
> > pp->cfg0_base = cfg_res->start;
> > pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> > +
> > + /* Find the untranslated configuration space address */
> > + index = of_property_match_string(np, "reg-names", "config");
> > + addrp = of_get_address(np, index, false, false);
> > + pp->cfg0_mod_addr = of_read_number(addrp, ns);
> > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size;
> > } else {
> > dev_err(pp->dev, "missing *config* reg space\n");
> > }
> > @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > pp->config.io_size = resource_size(&pp->io);
> > pp->config.io_bus_addr = range.pci_addr;
> > pp->io_base = range.cpu_addr;
> > +
> > + /* Find the untranslated IO space address */
> > + pp->io_mod_addr = of_read_number(parser.range -
> > + parser.np + na, ns);
> > }
> > if (restype == IORESOURCE_MEM) {
> > of_pci_range_to_resource(&range, np, &pp->mem);
> > pp->mem.name = "MEM";
> > pp->config.mem_size = resource_size(&pp->mem);
> > pp->config.mem_bus_addr = range.pci_addr;
> > +
> > + /* Find the untranslated MEM space address */
> > + pp->mem_mod_addr = of_read_number(parser.range -
> > + parser.np + na, ns);
> > }
> > if (restype == 0) {
> > of_pci_range_to_resource(&range, np, &pp->cfg);
> > @@ -448,6 +469,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > pp->cfg0_base = pp->cfg.start;
> > pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> > +
> > + /* Find the untranslated configuration space address */
> > + pp->cfg0_mod_addr = of_read_number(parser.range -
> > + parser.np + na, ns);
> > + pp->cfg1_mod_addr = pp->cfg0_mod_addr +
> > + pp->config.cfg0_size;
> > }
> > }
> >
> > @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
> > /* Program viewport 0 : OUTBOUND : CFG0 */
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
> > PCIE_ATU_VIEWPORT);
> > - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32), PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
> > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr, PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size - 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
> > @@ -538,9 +565,9 @@ static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1,
> > PCIE_ATU_VIEWPORT);
> > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1);
> > - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32), PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
> > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr, PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size - 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET);
> > @@ -553,9 +580,9 @@ static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX0,
> > PCIE_ATU_VIEWPORT);
> > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1);
> > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32), PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
> > + dw_pcie_writel_rc(pp, pp->mem_mod_addr, PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32), PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size - 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, pp->config.mem_bus_addr, PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
> > diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> > index 77f592f..13828f8 100644
> > --- a/drivers/pci/host/pcie-designware.h
> > +++ b/drivers/pci/host/pcie-designware.h
> > @@ -36,11 +36,15 @@ struct pcie_port {
> > u8 root_bus_nr;
> > void __iomem *dbi_base;
> > u64 cfg0_base;
> > + u64 cfg0_mod_addr;
> > void __iomem *va_cfg0_base;
> > u64 cfg1_base;
> > + u64 cfg1_mod_addr;
> > void __iomem *va_cfg1_base;
> > u64 io_base;
> > + u64 io_mod_addr;
> > u64 mem_base;
> > + u64 mem_mod_addr;
> > struct resource cfg;
> > struct resource io;
> > struct resource mem;
> >

2014-07-09 03:41:20

by Mohit KUMAR DCG

[permalink] [raw]
Subject: RE: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hello Kishon,

> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:[email protected]]
> Sent: Tuesday, July 08, 2014 6:01 PM
> To: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; Mohit
> KUMAR DCG; [email protected]
> Cc: [email protected]; Jason Gunthorpe; Marek Vasut; Arnd Bergmann
> Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while
> programming ATU
>
> Hi Mohit, Jingoo,
>
> On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
> > In DRA7, the cpu sees 32bit address, but the pcie controller can see
> > only 28bit address. So whenever the cpu issues a read/write request,
> > the 4 most significant bits are used by L3 to determine the target controller.
> > For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe
> > controller but the PCIe controller will see only (0x000_0000 -
> > 0xFFF_FFF). So for programming the outbound translation window the
> *base* should be programmed as 0x000_0000.
> > Whenever we try to write to say 0x2000_0000, it will be translated to
> > whatever we have programmed in the translation window with base as
> 0x000_0000.
> >
> > This is needed when the dt node is modelled something like below axi {
> > compatible = "simple-bus";
> > #size-cells = <1>;
> > #address-cells = <1>;
> > ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
> > 0x51000000 0x51000000 0x3000>;
> > pcie@51000000 {
> > reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000
> 0x2000>;
> > reg-names = "config", "ti_conf", "rc_dbics";
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges = <0x81000000 0 0 0x03000 0 0x00010000
> > 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
> > };
> > };
> >
> > Here the CPU address for configuration space is 0x20013000 and the
> > controller address for configuration space is 0x13000. The controller
> > address should be used while programming the ATU (in order for
> > translation to happen properly in DRA7xx).
>
> Are you okay with this patch? Can you give your Reviewed-by/Acked-by?
>

- looks fine to me.
Reviewed-by: Mohit Kumar<[email protected]>

Thanks
Mohit
> Cheers
> Kishon
> >
> > Cc: Jason Gunthorpe <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Mohit Kumar <[email protected]>
> > Cc: Jingoo Han <[email protected]>
> > Cc: Marek Vasut <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> > ---
> > drivers/pci/host/pcie-designware.c | 49
> ++++++++++++++++++++++++++++--------
> > drivers/pci/host/pcie-designware.h | 4 +++
> > 2 files changed, 42 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c
> > b/drivers/pci/host/pcie-designware.c
> > index 0b7b455..11aba6d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -401,8 +401,15 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > struct of_pci_range range;
> > struct of_pci_range_parser parser;
> > struct resource *cfg_res;
> > - u32 val;
> > - int i;
> > + u32 val, na, ns;
> > + const __be32 *addrp;
> > + int i, index;
> > +
> > + /* Find the address cell size and the number of cells in order to get
> > + * the untranslated address.
> > + */
> > + of_property_read_u32(np, "#address-cells", &na);
> > + ns = of_n_size_cells(np);
> >
> > cfg_res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM, "config");
> > if (cfg_res) {
> > @@ -410,6 +417,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > pp->config.cfg1_size = resource_size(cfg_res)/2;
> > pp->cfg0_base = cfg_res->start;
> > pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> > +
> > + /* Find the untranslated configuration space address */
> > + index = of_property_match_string(np, "reg-names",
> "config");
> > + addrp = of_get_address(np, index, false, false);
> > + pp->cfg0_mod_addr = of_read_number(addrp, ns);
> > + pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp-
> >config.cfg0_size;
> > } else {
> > dev_err(pp->dev, "missing *config* reg space\n");
> > }
> > @@ -435,12 +448,20 @@ int __init dw_pcie_host_init(struct pcie_port
> *pp)
> > pp->config.io_size = resource_size(&pp->io);
> > pp->config.io_bus_addr = range.pci_addr;
> > pp->io_base = range.cpu_addr;
> > +
> > + /* Find the untranslated IO space address */
> > + pp->io_mod_addr = of_read_number(parser.range -
> > + parser.np + na, ns);
> > }
> > if (restype == IORESOURCE_MEM) {
> > of_pci_range_to_resource(&range, np, &pp->mem);
> > pp->mem.name = "MEM";
> > pp->config.mem_size = resource_size(&pp->mem);
> > pp->config.mem_bus_addr = range.pci_addr;
> > +
> > + /* Find the untranslated MEM space address */
> > + pp->mem_mod_addr =
> of_read_number(parser.range -
> > + parser.np + na, ns);
> > }
> > if (restype == 0) {
> > of_pci_range_to_resource(&range, np, &pp->cfg);
> @@ -448,6 +469,12
> > @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > pp->cfg0_base = pp->cfg.start;
> > pp->cfg1_base = pp->cfg.start + pp-
> >config.cfg0_size;
> > +
> > + /* Find the untranslated configuration space address
> */
> > + pp->cfg0_mod_addr =
> of_read_number(parser.range -
> > + parser.np + na, ns);
> > + pp->cfg1_mod_addr = pp->cfg0_mod_addr +
> > + pp->config.cfg0_size;
> > }
> > }
> >
> > @@ -522,9 +549,9 @@ static void dw_pcie_prog_viewport_cfg0(struct
> pcie_port *pp, u32 busdev)
> > /* Program viewport 0 : OUTBOUND : CFG0 */
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND |
> PCIE_ATU_REGION_INDEX0,
> > PCIE_ATU_VIEWPORT);
> > - dw_pcie_writel_rc(pp, pp->cfg0_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->cfg0_base >> 32),
> PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->cfg0_base + pp->config.cfg0_size - 1,
> > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr,
> PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->cfg0_mod_addr >> 32),
> PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->cfg0_mod_addr + pp->config.cfg0_size -
> 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -538,9
> +565,9 @@
> > static void dw_pcie_prog_viewport_cfg1(struct pcie_port *pp, u32 busdev)
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND |
> PCIE_ATU_REGION_INDEX1,
> > PCIE_ATU_VIEWPORT);
> > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_CFG1, PCIE_ATU_CR1);
> > - dw_pcie_writel_rc(pp, pp->cfg1_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->cfg1_base >> 32),
> PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->cfg1_base + pp->config.cfg1_size - 1,
> > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr,
> PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->cfg1_mod_addr >> 32),
> PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->cfg1_mod_addr + pp->config.cfg1_size -
> 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, busdev, PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, 0, PCIE_ATU_UPPER_TARGET); @@ -553,9
> +580,9 @@
> > static void dw_pcie_prog_viewport_mem_outbound(struct pcie_port *pp)
> > dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND |
> PCIE_ATU_REGION_INDEX0,
> > PCIE_ATU_VIEWPORT);
> > dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, PCIE_ATU_CR1);
> > - dw_pcie_writel_rc(pp, pp->mem_base, PCIE_ATU_LOWER_BASE);
> > - dw_pcie_writel_rc(pp, (pp->mem_base >> 32),
> PCIE_ATU_UPPER_BASE);
> > - dw_pcie_writel_rc(pp, pp->mem_base + pp->config.mem_size - 1,
> > + dw_pcie_writel_rc(pp, pp->mem_mod_addr,
> PCIE_ATU_LOWER_BASE);
> > + dw_pcie_writel_rc(pp, (pp->mem_mod_addr >> 32),
> PCIE_ATU_UPPER_BASE);
> > + dw_pcie_writel_rc(pp, pp->mem_mod_addr + pp->config.mem_size
> - 1,
> > PCIE_ATU_LIMIT);
> > dw_pcie_writel_rc(pp, pp->config.mem_bus_addr,
> PCIE_ATU_LOWER_TARGET);
> > dw_pcie_writel_rc(pp, upper_32_bits(pp->config.mem_bus_addr),
> > diff --git a/drivers/pci/host/pcie-designware.h
> > b/drivers/pci/host/pcie-designware.h
> > index 77f592f..13828f8 100644
> > --- a/drivers/pci/host/pcie-designware.h
> > +++ b/drivers/pci/host/pcie-designware.h
> > @@ -36,11 +36,15 @@ struct pcie_port {
> > u8 root_bus_nr;
> > void __iomem *dbi_base;
> > u64 cfg0_base;
> > + u64 cfg0_mod_addr;
> > void __iomem *va_cfg0_base;
> > u64 cfg1_base;
> > + u64 cfg1_mod_addr;
> > void __iomem *va_cfg1_base;
> > u64 io_base;
> > + u64 io_mod_addr;
> > u64 mem_base;
> > + u64 mem_mod_addr;
> > struct resource cfg;
> > struct resource io;
> > struct resource mem;
> >

2014-07-09 09:39:08

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 2/3] PCI: designware: use untranslated address while programming ATU

Hi Jingoo,

On Tuesday 08 July 2014 06:47 PM, Jingoo Han wrote:
> On Tuesday, July 08, 2014 9:31 PM, Kishon Vijay Abraham I wrote:
>>
>> Hi Mohit, Jingoo,
>>
>> On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
>>> In DRA7, the cpu sees 32bit address, but the pcie controller can see only 28bit
>>> address. So whenever the cpu issues a read/write request, the 4 most
>>> significant bits are used by L3 to determine the target controller.
>>> For example, the cpu reserves 0x2000_0000 - 0x2FFF_FFFF for PCIe controller but
>>> the PCIe controller will see only (0x000_0000 - 0xFFF_FFF). So for programming
>>> the outbound translation window the *base* should be programmed as 0x000_0000.
>>> Whenever we try to write to say 0x2000_0000, it will be translated to whatever
>>> we have programmed in the translation window with base as 0x000_0000.
>>>
>>> This is needed when the dt node is modelled something like below
>>> axi {
>>> compatible = "simple-bus";
>>> #size-cells = <1>;
>>> #address-cells = <1>;
>>> ranges = <0x0 0x20000000 0x10000000 // 28-bit bus
>>> 0x51000000 0x51000000 0x3000>;
>>> pcie@51000000 {
>>> reg = <0x1000 0x2000>, <0x51002000 0x14c>, <0x51000000 0x2000>;
>>> reg-names = "config", "ti_conf", "rc_dbics";
>>> #address-cells = <3>;
>>> #size-cells = <2>;
>>> ranges = <0x81000000 0 0 0x03000 0 0x00010000
>>> 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
>>> };
>>> };
>>>
>>> Here the CPU address for configuration space is 0x20013000 and the controller
>>> address for configuration space is 0x13000. The controller address should be
>>> used while programming the ATU (in order for translation to happen properly in
>>> DRA7xx).
>>
>> Are you okay with this patch? Can you give your Reviewed-by/Acked-by?
>
> Sorry, I cannot understand fully what this patch does. :-(
> However, I have some questions as below.
>
> 1. io_mod_addr
> The 'io_mod_addr' variable is not used. Then, how about removing
> 'io_mod_addr' from this patch?

Actually I missed using it in io_outbound(). Will fix it.
>
> 2. ranges
> In the case of Exynos PCIe, there is no limitation such as 28bit for
> access. Then, when there is no 'ranges' property in DT file, is there
> no side effect? I mean the compatibility with other SoC's PCIe drivers.

It shouldn't break anything. The ranges is used to resolve cpu address while
adding resources to the device. So in other SoCs it'll use use the parent
'ranges' which is usually ocp/simple-bus to resolve cpu address. In my case it
has to use the axi ranges. So other SoCs shouldn't be impacted.
>
> 3. _addr post fix
>
> How about renaming 'cfg0_mod_addr', 'mem_mod_addr' as below?

sure.

Thanks
Kishon

2014-07-09 10:29:21

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 3/3] PCI: host: pcie-dra7xx: add support for pcie-dra7xx controller

+Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala

On Tuesday 08 July 2014 06:04 PM, Kishon Vijay Abraham I wrote:
> Hi Arnd, Tony, other dt guys,
>
> On Wednesday 25 June 2014 11:26 PM, Kishon Vijay Abraham I wrote:
>> Added support for pcie controller in dra7xx. This driver re-uses
>> the designware core code that is already present in kernel.
>>
>
> Are you okay with this patch? an you give your Acked-by?
>
> Thanks
> Kishon
>> Cc: Jason Gunthorpe <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Mohit Kumar <[email protected]>
>> Cc: Jingoo Han <[email protected]>
>> Cc: Marek Vasut <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> ---
>> Documentation/devicetree/bindings/pci/ti-pci.txt | 59 +++
>> drivers/pci/host/Kconfig | 10 +
>> drivers/pci/host/Makefile | 1 +
>> drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++++
>> 4 files changed, 528 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
>> create mode 100644 drivers/pci/host/pci-dra7xx.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> new file mode 100644
>> index 0000000..3d21791
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> @@ -0,0 +1,59 @@
>> +TI PCI Controllers
>> +
>> +PCIe Designware Controller
>> + - compatible: Should be "ti,dra7-pcie""
>> + - reg : Two register ranges as listed in the reg-names property
>> + - reg-names : The first entry must be "ti-conf" for the TI specific registers
>> + The second entry must be "rc-dbics" for the designware pcie
>> + registers
>> + The third entry must be "config" for the PCIe configuration space
>> + - phys : list of PHY specifiers (used by generic PHY framework)
>> + - phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
>> + number of PHYs as specified in *phys* property.
>> + - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
>> + where <X> is the instance number of the pcie from the HW spec.
>> + - interrupts : Two interrupt entries must be specified. The first one is for
>> + main interrupt line and the second for MSI interrupt line.
>> + - #address-cells,
>> + #size-cells,
>> + #interrupt-cells,
>> + device_type,
>> + ranges,
>> + num-lanes,
>> + interrupt-map-mask,
>> + interrupt-map : as specified in ../designware-pcie.txt
>> +
>> +Example:
>> +axi {
>> + compatible = "simple-bus";
>> + #size-cells = <1>;
>> + #address-cells = <1>;
>> + ranges = <0x51000000 0x51000000 0x3000
>> + 0x0 0x20000000 0x10000000>;
>> + pcie@51000000 {
>> + compatible = "ti,dra7-pcie";
>> + reg = <0x51000000 0x2000>, <0x51002000 0x14c>, <0x1000 0x2000>;
>> + reg-names = "rc_dbics", "ti_conf", "config";
>> + interrupts = <0 232 0x4>, <0 233 0x4>;
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + device_type = "pci";
>> + ranges = <0x81000000 0 0 0x03000 0 0x00010000
>> + 0x82000000 0 0x20013000 0x13000 0 0xffed000>;
>> + #interrupt-cells = <1>;
>> + num-lanes = <1>;
>> + ti,hwmods = "pcie1";
>> + phys = <&pcie1_phy>;
>> + phy-names = "pcie-phy0";
>> + interrupt-map-mask = <0 0 0 7>;
>> + interrupt-map = <0 0 0 1 &pcie_intc 1>,
>> + <0 0 0 2 &pcie_intc 2>,
>> + <0 0 0 3 &pcie_intc 3>,
>> + <0 0 0 4 &pcie_intc 4>;
>> + pcie_intc: interrupt-controller {
>> + interrupt-controller;
>> + #address-cells = <0>;
>> + #interrupt-cells = <1>;
>> + };
>> + };
>> +};
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 21df477..22117b0 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -1,6 +1,16 @@
>> menu "PCI host controller drivers"
>> depends on PCI
>>
>> +config PCI_DRA7XX
>> + bool "TI DRA7xx PCIe controller"
>> + select PCIE_DW
>> + depends on OF && HAS_IOMEM && TI_PIPE3
>> + help
>> + Enables support for the PCIE controller present in DRA7xx SoC. There
>> + are two instances of PCIE controller in DRA7xx. This controller can
>> + act both as EP and RC. This reuses the same Designware core as used
>> + by other SoCs.
>> +
>> config PCI_MVEBU
>> bool "Marvell EBU PCIe controller"
>> depends on ARCH_MVEBU || ARCH_DOVE || ARCH_KIRKWOOD
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 611ba4b..c42844d 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -1,4 +1,5 @@
>> obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>> +obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
>> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
>> new file mode 100644
>> index 0000000..52b34fe
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-dra7xx.c
>> @@ -0,0 +1,458 @@
>> +/*
>> + * pcie-dra7xx - PCIe controller driver for TI DRA7xx SoCs
>> + *
>> + * Copyright (C) 2013-2014 Texas Instruments Incorporated - http://www.ti.com
>> + *
>> + * Authors: Kishon Vijay Abraham I <[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/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/resource.h>
>> +#include <linux/types.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +/* PCIe controller wrapper DRA7XX configuration registers */
>> +
>> +#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN 0x0024
>> +#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN 0x0028
>> +#define ERR_SYS BIT(0)
>> +#define ERR_FATAL BIT(1)
>> +#define ERR_NONFATAL BIT(2)
>> +#define ERR_COR BIT(3)
>> +#define ERR_AXI BIT(4)
>> +#define ERR_ECRC BIT(5)
>> +#define PME_TURN_OFF BIT(8)
>> +#define PME_TO_ACK BIT(9)
>> +#define PM_PME BIT(10)
>> +#define LINK_REQ_RST BIT(11)
>> +#define LINK_UP_EVT BIT(12)
>> +#define CFG_BME_EVT BIT(13)
>> +#define CFG_MSE_EVT BIT(14)
>> +#define INTERRUPTS (ERR_SYS | ERR_FATAL | ERR_NONFATAL | ERR_COR | ERR_AXI | \
>> + ERR_ECRC | PME_TURN_OFF | PME_TO_ACK | PM_PME | \
>> + LINK_REQ_RST | LINK_UP_EVT | CFG_BME_EVT | CFG_MSE_EVT)
>> +
>> +#define PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI 0x0034
>> +#define PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI 0x0038
>> +#define INTA BIT(0)
>> +#define INTB BIT(1)
>> +#define INTC BIT(2)
>> +#define INTD BIT(3)
>> +#define MSI BIT(4)
>> +#define LEG_EP_INTERRUPTS (INTA | INTB | INTC | INTD)
>> +
>> +#define PCIECTRL_DRA7XX_CONF_DEVICE_CMD 0x0104
>> +#define LTSSM_EN 0x1
>> +
>> +#define PCIECTRL_DRA7XX_CONF_PHY_CS 0x010C
>> +#define LINK_UP BIT(16)
>> +
>> +struct dra7xx_pcie {
>> + void __iomem *base;
>> + struct phy **phy;
>> + int phy_count;
>> + struct device *dev;
>> + struct pcie_port pp;
>> +};
>> +
>> +#define to_dra7xx_pcie(x) container_of((x), struct dra7xx_pcie, pp)
>> +
>> +static inline u32 dra7xx_pcie_readl(struct dra7xx_pcie *pcie, u32 offset)
>> +{
>> + return readl(pcie->base + offset);
>> +}
>> +
>> +static inline void dra7xx_pcie_writel(struct dra7xx_pcie *pcie, u32 offset,
>> + u32 value)
>> +{
>> + writel(value, pcie->base + offset);
>> +}
>> +
>> +static int dra7xx_pcie_link_up(struct pcie_port *pp)
>> +{
>> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
>> + u32 reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
>> +
>> + return !!(reg & LINK_UP);
>> +}
>> +
>> +static int dra7xx_pcie_establish_link(struct pcie_port *pp)
>> +{
>> + u32 reg;
>> + unsigned int retries = 1000;
>> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
>> +
>> + if (dw_pcie_link_up(pp)) {
>> + dev_err(pp->dev, "link is already up\n");
>> + return 0;
>> + }
>> +
>> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
>> + reg |= LTSSM_EN;
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
>> +
>> + while (retries--) {
>> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_PHY_CS);
>> + if (reg & LINK_UP)
>> + break;
>> + usleep_range(10, 20);
>> + }
>> +
>> + if (retries == 0) {
>> + dev_err(pp->dev, "link is not up\n");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
>> +{
>> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
>> +
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN,
>> + ~INTERRUPTS);
>> + dra7xx_pcie_writel(dra7xx,
>> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MAIN, INTERRUPTS);
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
>> + ~LEG_EP_INTERRUPTS & ~MSI);
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI))
>> + dra7xx_pcie_writel(dra7xx,
>> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI, MSI);
>> + else
>> + dra7xx_pcie_writel(dra7xx,
>> + PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI,
>> + LEG_EP_INTERRUPTS);
>> +}
>> +
>> +static void dra7xx_pcie_host_init(struct pcie_port *pp)
>> +{
>> + dw_pcie_setup_rc(pp);
>> + dra7xx_pcie_establish_link(pp);
>> + if (IS_ENABLED(CONFIG_PCI_MSI))
>> + dw_pcie_msi_init(pp);
>> + dra7xx_pcie_enable_interrupts(pp);
>> +}
>> +
>> +static struct pcie_host_ops dra7xx_pcie_host_ops = {
>> + .link_up = dra7xx_pcie_link_up,
>> + .host_init = dra7xx_pcie_host_init,
>> +};
>> +
>> +static int dra7xx_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
>> + irq_hw_number_t hwirq)
>> +{
>> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
>> + irq_set_chip_data(irq, domain->host_data);
>> + set_irq_flags(irq, IRQF_VALID);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct irq_domain_ops intx_domain_ops = {
>> + .map = dra7xx_pcie_intx_map,
>> +};
>> +
>> +static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
>> +{
>> + struct device *dev = pp->dev;
>> + struct device_node *node = dev->of_node;
>> + struct device_node *pcie_intc_node = of_get_next_child(node, NULL);
>> +
>> + if (!pcie_intc_node) {
>> + dev_err(dev, "No PCIe Intc node found\n");
>> + return PTR_ERR(pcie_intc_node);
>> + }
>> +
>> + pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
>> + &intx_domain_ops, pp);
>> + if (!pp->irq_domain) {
>> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
>> + return PTR_ERR(pp->irq_domain);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
>> +{
>> + struct pcie_port *pp = arg;
>> + struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pp);
>> + u32 reg;
>> +
>> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI);
>> +
>> + switch (reg) {
>> + case MSI:
>> + dw_handle_msi_irq(pp);
>> + break;
>> + case INTA:
>> + case INTB:
>> + case INTC:
>> + case INTD:
>> + generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg)));
>> + break;
>> + }
>> +
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI, reg);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +
>> +static irqreturn_t dra7xx_pcie_irq_handler(int irq, void *arg)
>> +{
>> + struct dra7xx_pcie *dra7xx = arg;
>> + u32 reg;
>> +
>> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN);
>> +
>> + if (reg & ERR_SYS)
>> + dev_dbg(dra7xx->dev, "System Error\n");
>> +
>> + if (reg & ERR_FATAL)
>> + dev_dbg(dra7xx->dev, "Fatal Error\n");
>> +
>> + if (reg & ERR_NONFATAL)
>> + dev_dbg(dra7xx->dev, "Non Fatal Error\n");
>> +
>> + if (reg & ERR_COR)
>> + dev_dbg(dra7xx->dev, "Correctable Error\n");
>> +
>> + if (reg & ERR_AXI)
>> + dev_dbg(dra7xx->dev, "AXI tag lookup fatal Error\n");
>> +
>> + if (reg & ERR_ECRC)
>> + dev_dbg(dra7xx->dev, "ECRC Error\n");
>> +
>> + if (reg & PME_TURN_OFF)
>> + dev_dbg(dra7xx->dev,
>> + "Power Management Event Turn-Off message received\n");
>> +
>> + if (reg & PME_TO_ACK)
>> + dev_dbg(dra7xx->dev,
>> + "Power Management Turn-Off Ack message received\n");
>> +
>> + if (reg & PM_PME)
>> + dev_dbg(dra7xx->dev,
>> + "PM Power Management Event message received\n");
>> +
>> + if (reg & LINK_REQ_RST)
>> + dev_dbg(dra7xx->dev, "Link Request Reset\n");
>> +
>> + if (reg & LINK_UP_EVT)
>> + dev_dbg(dra7xx->dev, "Link-up state change\n");
>> +
>> + if (reg & CFG_BME_EVT)
>> + dev_dbg(dra7xx->dev, "CFG 'Bus Master Enable' change\n");
>> +
>> + if (reg & CFG_MSE_EVT)
>> + dev_dbg(dra7xx->dev, "CFG 'Memory Space Enable' change\n");
>> +
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MAIN, reg);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int add_pcie_port(struct dra7xx_pcie *dra7xx,
>> + struct platform_device *pdev)
>> +{
>> + int ret;
>> + struct pcie_port *pp;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> +
>> + pp = &dra7xx->pp;
>> + pp->dev = dev;
>> + pp->ops = &dra7xx_pcie_host_ops;
>> +
>> + pp->irq = platform_get_irq(pdev, 1);
>> + if (pp->irq < 0) {
>> + dev_err(dev, "missing IRQ resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = devm_request_irq(&pdev->dev, pp->irq,
>> + dra7xx_pcie_msi_irq_handler, IRQF_SHARED,
>> + "dra7-pcie-msi", pp);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to request irq\n");
>> + return ret;
>> + }
>> +
>> + if (!IS_ENABLED(CONFIG_PCI_MSI)) {
>> + ret = dra7xx_pcie_init_irq_domain(pp);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
>> + pp->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
>> + if (!pp->dbi_base)
>> + return -ENOMEM;
>> +
>> + ret = dw_pcie_host_init(pp);
>> + if (ret) {
>> + dev_err(dra7xx->dev, "failed to initialize host\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>> +{
>> + u32 reg;
>> + int ret;
>> + int irq;
>> + int i;
>> + int phy_count;
>> + struct phy **phy;
>> + void __iomem *base;
>> + struct resource *res;
>> + struct dra7xx_pcie *dra7xx;
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + char name[10];
>> +
>> + dra7xx = devm_kzalloc(dev, sizeof(*dra7xx), GFP_KERNEL);
>> + if (!dra7xx)
>> + return -ENOMEM;
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(dev, "missing IRQ resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = devm_request_irq(dev, irq, dra7xx_pcie_irq_handler,
>> + IRQF_SHARED, "dra7xx-pcie-main", dra7xx);
>> + if (ret) {
>> + dev_err(dev, "failed to request irq\n");
>> + return ret;
>> + }
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
>> + base = devm_ioremap_nocache(dev, res->start, resource_size(res));
>> + if (!base)
>> + return -ENOMEM;
>> +
>> + phy_count = of_property_count_strings(np, "phy-names");
>> + if (phy_count < 0) {
>> + dev_err(dev, "unable to find the strings\n");
>> + return phy_count;
>> + }
>> +
>> + phy = devm_kzalloc(dev, sizeof(*phy) * phy_count, GFP_KERNEL);
>> + if (!phy)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < phy_count; i++) {
>> + snprintf(name, sizeof(name), "pcie-phy%d", i);
>> + phy[i] = devm_phy_get(dev, name);
>> + if (IS_ERR(phy[i]))
>> + return PTR_ERR(phy[i]);
>> +
>> + ret = phy_init(phy[i]);
>> + if (ret < 0)
>> + goto err_phy;
>> +
>> + ret = phy_power_on(phy[i]);
>> + if (ret < 0) {
>> + phy_exit(phy[i]);
>> + goto err_phy;
>> + }
>> + }
>> +
>> + dra7xx->base = base;
>> + dra7xx->phy = phy;
>> + dra7xx->dev = dev;
>> + dra7xx->phy_count = phy_count;
>> +
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (IS_ERR_VALUE(ret)) {
>> + dev_err(dev, "pm_runtime_get_sync failed\n");
>> + goto err_phy;
>> + }
>> +
>> + reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
>> + reg &= ~LTSSM_EN;
>> + dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
>> +
>> + platform_set_drvdata(pdev, dra7xx);
>> +
>> + ret = add_pcie_port(dra7xx, pdev);
>> + if (ret < 0)
>> + goto err_add_port;
>> +
>> + return 0;
>> +
>> +err_add_port:
>> + pm_runtime_put(dev);
>> + pm_runtime_disable(dev);
>> +
>> +err_phy:
>> + while (--i >= 0) {
>> + phy_power_off(phy[i]);
>> + phy_exit(phy[i]);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int __exit dra7xx_pcie_remove(struct platform_device *pdev)
>> +{
>> + struct dra7xx_pcie *dra7xx = platform_get_drvdata(pdev);
>> + struct pcie_port *pp = &dra7xx->pp;
>> + struct device *dev = &pdev->dev;
>> + int count = dra7xx->phy_count;
>> +
>> + if (pp->irq_domain)
>> + irq_domain_remove(pp->irq_domain);
>> + pm_runtime_put(dev);
>> + pm_runtime_disable(dev);
>> + while (count--) {
>> + phy_power_off(dra7xx->phy[count]);
>> + phy_exit(dra7xx->phy[count]);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id of_dra7xx_pcie_match[] = {
>> + { .compatible = "ti,dra7-pcie", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, of_dra7xx_pcie_match);
>> +
>> +static struct platform_driver dra7xx_pcie_driver = {
>> + .remove = __exit_p(dra7xx_pcie_remove),
>> + .driver = {
>> + .name = "dra7-pcie",
>> + .owner = THIS_MODULE,
>> + .of_match_table = of_dra7xx_pcie_match,
>> + },
>> +};
>> +
>> +module_platform_driver_probe(dra7xx_pcie_driver, dra7xx_pcie_probe);
>> +
>> +MODULE_AUTHOR("Kishon Vijay Abraham I <[email protected]>");
>> +MODULE_DESCRIPTION("TI PCIe controller driver");
>> +MODULE_LICENSE("GPL v2");
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>