Changes from v2:
* Added myself as MAINTAINER of pcie dra7xx driver
Changes from v1:
* fixed dw_pcie_prog_viewport_io_outbound() to use untranslated address
* split dra7xx patch into driver part and documentation part
[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 pci -next.
[1] -> https://lkml.org/lkml/2014/5/29/258
Kishon Vijay Abraham I (4):
PCI: designware: Configuration space should be specified in 'reg'
PCI: designware: use untranslated address while programming ATU
Documentation: pci: ti: Add dt binding documentation for PCIe in
DRA7xx
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 +++
MAINTAINERS | 8 +
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++
drivers/pci/host/pcie-designware.c | 72 ++-
drivers/pci/host/pcie-designware.h | 4 +
8 files changed, 600 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
create mode 100644 drivers/pci/host/pci-dra7xx.c
--
1.7.9.5
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: Marek Vasut <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Acked-by: Mohit Kumar <[email protected]>
Acked-by: Jingoo Han <[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
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]>
Acked-by: Jingoo Han <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
MAINTAINERS | 8 +
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-dra7xx.c | 458 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 477 insertions(+)
create mode 100644 drivers/pci/host/pci-dra7xx.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 134483f..fdd34d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6796,6 +6796,14 @@ S: Supported
F: Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
F: drivers/pci/host/pci-tegra.c
+PCI DRIVER FOR TI DRA7XX
+M: Kishon Vijay Abraham I <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Supported
+F: Documentation/devicetree/bindings/pci/ti-pci.txt
+F: drivers/pci/host/pci-dra7xx.c
+
PCI DRIVER FOR RENESAS R-CAR
M: Simon Horman <[email protected]>
L: [email protected]
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
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 | 55 +++++++++++++++++++++++++++---------
drivers/pci/host/pcie-designware.h | 4 +++
2 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 0b7b455..8aab1d6 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_base = of_read_number(addrp, ns);
+ pp->cfg1_mod_base = pp->cfg0_mod_base + 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_base = 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_base = 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_base = of_read_number(parser.range -
+ parser.np + na, ns);
+ pp->cfg1_mod_base = pp->cfg0_mod_base +
+ 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_base, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->cfg0_mod_base >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->cfg0_mod_base + 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_base, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->cfg1_mod_base >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->cfg1_mod_base + 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_base, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->mem_mod_base >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->mem_mod_base + 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),
@@ -569,9 +596,9 @@ static void dw_pcie_prog_viewport_io_outbound(struct pcie_port *pp)
dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | PCIE_ATU_REGION_INDEX1,
PCIE_ATU_VIEWPORT);
dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_IO, PCIE_ATU_CR1);
- dw_pcie_writel_rc(pp, pp->io_base, PCIE_ATU_LOWER_BASE);
- dw_pcie_writel_rc(pp, (pp->io_base >> 32), PCIE_ATU_UPPER_BASE);
- dw_pcie_writel_rc(pp, pp->io_base + pp->config.io_size - 1,
+ dw_pcie_writel_rc(pp, pp->io_mod_base, PCIE_ATU_LOWER_BASE);
+ dw_pcie_writel_rc(pp, (pp->io_mod_base >> 32), PCIE_ATU_UPPER_BASE);
+ dw_pcie_writel_rc(pp, pp->io_mod_base + pp->config.io_size - 1,
PCIE_ATU_LIMIT);
dw_pcie_writel_rc(pp, pp->config.io_bus_addr, PCIE_ATU_LOWER_TARGET);
dw_pcie_writel_rc(pp, upper_32_bits(pp->config.io_bus_addr),
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 77f592f..add6527 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_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;
struct resource cfg;
struct resource io;
struct resource mem;
--
1.7.9.5
Added dt binding documentation of PCIe in DRA7xx.
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
Documentation/devicetree/bindings/pci/ti-pci.txt | 59 ++++++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
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>;
+ };
+ };
+};
--
1.7.9.5
Hi Bjorn,
On Thursday 17 July 2014 02:30 PM, Kishon Vijay Abraham I wrote:
> Changes from v2:
> * Added myself as MAINTAINER of pcie dra7xx driver
>
> Changes from v1:
> * fixed dw_pcie_prog_viewport_io_outbound() to use untranslated address
> * split dra7xx patch into driver part and documentation part
>
> [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 pci -next.
>
> [1] -> https://lkml.org/lkml/2014/5/29/258
I tried to get Acks for the DT documentation from DT maintainers but couldn't.
I'd assume they would have reverted if they had any concerns. Moreover the dt
documentation for DRA7xx is just an extension of the dt documentation for
designware. And since Pratyush, Jingoo, Mohit seems to be okay with this patch
series, can we get this series merged?
Cheers
Kishon
On Thu, Jul 17, 2014 at 02:30:40PM +0530, 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: Marek Vasut <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Mohit Kumar <[email protected]>
> Acked-by: Jingoo Han <[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.)
I'm not a devicetree person, so please excuse my floundering here.
There doesn't seem to be consistent usage of "reg" or "reg-names".
Here's what I found in the existing Documentation/devicetree/bindings/pci
stuff:
83xx-512x-pci.txt
- reg: should contain two address length tuples
The first is for the internal pci bridge registers
The second is for the pci config space access registers
fsl,imx6q-pcie.txt
- reg: base addresse and length of the pcie controller
host-generic-pci.txt
- reg : The Configuration Space base address and size, as accessed
from the parent bus.
nvidia,tegra20-pcie.txt
- reg: A list of physical base address and length for each set of controller
registers. Must contain an entry for each entry in the reg-names property.
- reg-names: Must include the following entries:
"pads": PADS registers
"afi": AFI registers
"cs": configuration space region
pci-rcar-gen2.txt
- reg: A list of physical regions to access the device: the first is
the operational registers for the OHCI/EHCI controllers and the
second is for the bridge configuration and control registers.
ralink,rt3883-pci.txt
- reg: specifies the physical base address of the controller and
the length of the memory mapped region.
rcar-pci.txt
- reg: base address and length of the pcie controller registers.
samsung,exynos5440-pcie.txt
- reg: base addresses and lengths of the pcie controller,
the phy controller, additional register for the phy controller.
v3-v360epc-pci.txt
- reg: should contain the base address of the V3 adapter.
So my questions are:
1) Is "reg" the right place for configuration space? I assume this is
CAM or ECAM. Only 83xx-512x-pci.txt, host-generic-pci.txt, and
nvidia,tegra20-pcie.txt mention using "reg" for config space; all the
others use "reg" for register space for the PCIe controller itself.
2) Is "config" the correct value for "reg-names"? Only
nvidia,tegra20-pcie.txt has something similar, and it wants "cs".
Should these be the same?
3) Doesn't this add a revlock (kernels with this patch require a new
device tree that adds "reg" and "reg-names"? I know both Mohit and
Jingoo acked this [1,2], so I'm just double-checking that this is
expected and desired.
Bjorn
[1] http://lkml.kernel.org/r/[email protected]
[2] http://marc.info/?l=linux-pci&m=140482389231290&w=2
> - #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
>
On Tuesday 22 July 2014 14:53:26 Bjorn Helgaas wrote:
> > @@ -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.)
>
> I'm not a devicetree person, so please excuse my floundering here.
> There doesn't seem to be consistent usage of "reg" or "reg-names".
I think the above is ok, given the constraints we hae.
> Here's what I found in the existing Documentation/devicetree/bindings/pci
> stuff:
>
> 83xx-512x-pci.txt
> - reg: should contain two address length tuples
> The first is for the internal pci bridge registers
> The second is for the pci config space access registers
This is fine
> fsl,imx6q-pcie.txt
> - reg: base addresse and length of the pcie controller
This should have included an entry for the config space, it was a
mistake
> host-generic-pci.txt
> - reg : The Configuration Space base address and size, as accessed
> from the parent bus.
This is fine.
> nvidia,tegra20-pcie.txt
> - reg: A list of physical base address and length for each set of controller
> registers. Must contain an entry for each entry in the reg-names property.
> - reg-names: Must include the following entries:
> "pads": PADS registers
> "afi": AFI registers
> "cs": configuration space region
The names are a bit strange, but it's also ok. In general we should mandate
the order of the entries as well.
> pci-rcar-gen2.txt
> - reg: A list of physical regions to access the device: the first is
> the operational registers for the OHCI/EHCI controllers and the
> second is for the bridge configuration and control registers.
This one is wrong, as previously discussed. The first region should
have been listed in the 'ranges' property, because these are the registers
of the child nodes (the actual PCI devices) rather than the host controller
itself.
> ralink,rt3883-pci.txt
> - reg: specifies the physical base address of the controller and
> the length of the memory mapped region.
The wording is a bit strange but it seems otherwise correct.
> rcar-pci.txt
> - reg: base address and length of the pcie controller registers.
This is ok
> samsung,exynos5440-pcie.txt
> - reg: base addresses and lengths of the pcie controller,
> the phy controller, additional register for the phy controller.
Could be a bit more verbose, but is not wrong. In order to work
with the "snps,dw-pcie" binding, this will need a reg-names property.
> v3-v360epc-pci.txt
> - reg: should contain the base address of the V3 adapter.
This has the same mistake that the pcie-dw driver has and should
be fixed by putting the config space in there.
> So my questions are:
>
> 1) Is "reg" the right place for configuration space? I assume this is
> CAM or ECAM. Only 83xx-512x-pci.txt, host-generic-pci.txt, and
> nvidia,tegra20-pcie.txt mention using "reg" for config space; all the
> others use "reg" for register space for the PCIe controller itself.
We had long debates about this. The conclusion was that we should
have put them in 'reg' from the start and not used 'ranges' here.
> 2) Is "config" the correct value for "reg-names"? Only
> nvidia,tegra20-pcie.txt has something similar, and it wants "cs".
> Should these be the same?
It would be better if they were the same, but it's not important.
The names are private to the device driver, and the register layout
within this region is driver specific, so we can't use common code
to parse them anyway.
> 3) Doesn't this add a revlock (kernels with this patch require a new
> device tree that adds "reg" and "reg-names"? I know both Mohit and
> Jingoo acked this [1,2], so I'm just double-checking that this is
> expected and desired.
My understanding is that the patch still allows the old method as
a fallback if no reg with the name "config" is present.
Arnd
On Thu, Jul 17, 2014 at 02:30:39PM +0530, Kishon Vijay Abraham I wrote:
> Changes from v2:
> * Added myself as MAINTAINER of pcie dra7xx driver
>
> Changes from v1:
> * fixed dw_pcie_prog_viewport_io_outbound() to use untranslated address
> * split dra7xx patch into driver part and documentation part
>
> [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 pci -next.
>
> [1] -> https://lkml.org/lkml/2014/5/29/258
>
> Kishon Vijay Abraham I (4):
> PCI: designware: Configuration space should be specified in 'reg'
> PCI: designware: use untranslated address while programming ATU
> Documentation: pci: ti: Add dt binding documentation for PCIe in
> DRA7xx
> 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 +++
> MAINTAINERS | 8 +
> drivers/pci/host/Kconfig | 10 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++
> drivers/pci/host/pcie-designware.c | 72 ++-
> drivers/pci/host/pcie-designware.h | 4 +
> 8 files changed, 600 insertions(+), 16 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
> create mode 100644 drivers/pci/host/pci-dra7xx.c
I applied these to pci/host-designware for v3.17. This will not be in my
first pull request because I'm going on vacation and won't have time to get
everything through -next and tagged. But I'm planning a second pull
request that will include this.
Mohit, I applied your ack of "use untranslated address" from the v1 posting
(your mail of Jul 09) to the corresponding patch here, since Kishon didn't
carry it forward. Let me know if that's not OK.
I also squashed the last two patches into one so the DT binding
documentation goes along with the dra7xx driver addition.
Bjorn
Hello Bjorn,
> -----Original Message-----
> From: Bjorn Helgaas [mailto:[email protected]]
> Sent: Wednesday, July 23, 2014 3:03 AM
> To: Kishon Vijay Abraham I
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Mohit KUMAR DCG; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH v3 0/4] dra7: Add PCIe support
>
> On Thu, Jul 17, 2014 at 02:30:39PM +0530, Kishon Vijay Abraham I wrote:
> > Changes from v2:
> > * Added myself as MAINTAINER of pcie dra7xx driver
[...]
>
> I applied these to pci/host-designware for v3.17. This will not be in my first
> pull request because I'm going on vacation and won't have time to get
> everything through -next and tagged. But I'm planning a second pull request
> that will include this.
>
> Mohit, I applied your ack of "use untranslated address" from the v1 posting
> (your mail of Jul 09) to the corresponding patch here, since Kishon didn't
> carry it forward. Let me know if that's not OK.
- yes, OK with me.
Thanks
Mohit
>
> I also squashed the last two patches into one so the DT binding
> documentation goes along with the dra7xx driver addition.
>
> Bjorn
On Wednesday 23 July 2014 03:03 AM, Bjorn Helgaas wrote:
> On Thu, Jul 17, 2014 at 02:30:39PM +0530, Kishon Vijay Abraham I wrote:
>> Changes from v2:
>> * Added myself as MAINTAINER of pcie dra7xx driver
>>
>> Changes from v1:
>> * fixed dw_pcie_prog_viewport_io_outbound() to use untranslated address
>> * split dra7xx patch into driver part and documentation part
>>
>> [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 pci -next.
>>
>> [1] -> https://lkml.org/lkml/2014/5/29/258
>>
>> Kishon Vijay Abraham I (4):
>> PCI: designware: Configuration space should be specified in 'reg'
>> PCI: designware: use untranslated address while programming ATU
>> Documentation: pci: ti: Add dt binding documentation for PCIe in
>> DRA7xx
>> 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 +++
>> MAINTAINERS | 8 +
>> drivers/pci/host/Kconfig | 10 +
>> drivers/pci/host/Makefile | 1 +
>> drivers/pci/host/pci-dra7xx.c | 458 ++++++++++++++++++++
>> drivers/pci/host/pcie-designware.c | 72 ++-
>> drivers/pci/host/pcie-designware.h | 4 +
>> 8 files changed, 600 insertions(+), 16 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/pci/ti-pci.txt
>> create mode 100644 drivers/pci/host/pci-dra7xx.c
>
> I applied these to pci/host-designware for v3.17. This will not be in my
> first pull request because I'm going on vacation and won't have time to get
> everything through -next and tagged. But I'm planning a second pull
> request that will include this.
>
> Mohit, I applied your ack of "use untranslated address" from the v1 posting
> (your mail of Jul 09) to the corresponding patch here, since Kishon didn't
> carry it forward. Let me know if that's not OK.
>
> I also squashed the last two patches into one so the DT binding
> documentation goes along with the dra7xx driver addition.
>
Thanks Bjorn.
-Kishon
> Bjorn
>