This patch adds a PCIe controller driver for Keystone SoCs. This
is based on v1 of the series posted to the mailing list.
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
Changelog:
V2
- Split the designware pcie enhancement patch to multiple
patches based on functionality added
- Remove the quirk code and add a patch to fix mps/mrss
tuning for ARM. Use kernel command line parameter
pci=pcie_bus_perf to work with Keystone PCI Controller.
Following patch addressed this.
[PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
- Add documentation for device tree bindings
- Add separate interrupt controller nodes for MSI and Legacy
IRQs and use irq map for legacy IRQ
- Use compatibility to identify v3.65 version of the DW hardware
and use it to customize the designware common code.
- Use reg property for configuration space instead of range
- Other minor updates based on code inspection.
V1
- Add an interrupt controller node for Legacy irq chip and use
interrupt map/map-mask property to map legacy IRQs A/B/C/D
- Add a Phy driver to replace the original serdes driver
- Move common application register handling code to a separate
file to allow re-use across other platforms that use older
DW PCIe h/w
- PCI quirk for maximum read request size. Check and override only
if the maximum is higher than what controller can handle.
- Converted to a module platform driver.
Murali Karicheri (8):
PCI: designware: add rd[wr]_other_conf API
PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
PCI: designware: update pcie core driver to work with dw hw version
3.65
PCI: designware: add msi controller functions for v3.65 hw
PCI: designware: add PCI controller functions for v3.65 DW hw
phy: Add serdes phy driver for keystone
PCI: keystone: add pcie driver based on designware core driver
ARM: keystone: add pcie related options
.../devicetree/bindings/pci/designware-pcie.txt | 42 ++
.../devicetree/bindings/pci/pci-keystone.txt | 56 +++
.../bindings/phy/phy-keystone-serdes.txt | 25 ++
arch/arm/mach-keystone/Kconfig | 1 +
drivers/pci/host/Kconfig | 12 +
drivers/pci/host/Makefile | 2 +
drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
drivers/pci/host/pci-dw-v3_65.h | 34 ++
drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
drivers/pci/host/pcie-designware.c | 175 +++++---
drivers/pci/host/pcie-designware.h | 42 +-
drivers/phy/Kconfig | 6 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
15 files changed, 1531 insertions(+), 52 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
create mode 100644 drivers/pci/host/pci-dw-v3_65.c
create mode 100644 drivers/pci/host/pci-dw-v3_65.h
create mode 100644 drivers/pci/host/pci-keystone.c
create mode 100644 drivers/phy/phy-keystone-serdes.c
--
1.7.9.5
Add common PCI controller functions for v3.65 DW hw version. This
provides a function, dw_v3_65_pcie_host_init() to initialize the host.
It check compatibility string dw,snps-pcie-v3.65 that is expected
to be present in the device node of a v3.65 compliant hw and initialize
the controller using other APIs (existing and newly added). Added
new fields in the port struct to support the new hw version.
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
---
drivers/pci/host/Makefile | 2 +-
drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++++++++++++++++++++
drivers/pci/host/pci-dw-v3_65.h | 14 ++
drivers/pci/host/pcie-designware.h | 1 +
4 files changed, 406 insertions(+), 1 deletion(-)
create mode 100644 drivers/pci/host/pci-dw-v3_65.c
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 28af710..b44a878 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,4 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
-obj-$(CONFIG_PCI_DW_V3_65) += pci-dw-v3_65-msi.o
+obj-$(CONFIG_PCI_DW_V3_65) += pci-dw-v3_65-msi.o pci-dw-v3_65.o
diff --git a/drivers/pci/host/pci-dw-v3_65.c b/drivers/pci/host/pci-dw-v3_65.c
new file mode 100644
index 0000000..fde50a1
--- /dev/null
+++ b/drivers/pci/host/pci-dw-v3_65.c
@@ -0,0 +1,390 @@
+/*
+ * Designware(dw) v3.65 common functions
+ *
+ * Copyright (C) 2013-2014 Texas Instruments., Ltd.
+ * http://www.ti.com
+ *
+ * Author: Murali Karicheri <[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/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-designware.h"
+#include "pci-dw-v3_65.h"
+
+/* Application register defines */
+#define LTSSM_EN_VAL BIT(0)
+#define LTSSM_STATE_MASK 0x1f
+#define LTSSM_STATE_L0 0x11
+#define DIR_SPD (1 << 17)
+#define DBI_CS2_EN_VAL BIT(5)
+#define OB_XLAT_EN_VAL BIT(1)
+
+/* Application registers */
+#define CMD_STATUS 0x004
+#define CFG_SETUP 0x008
+#define OB_SIZE 0x030
+#define CFG_PCIM_WIN_SZ_IDX 3
+#define CFG_PCIM_WIN_CNT 32
+#define SPACE0_REMOTE_CFG_OFFSET 0x1000
+#define OB_OFFSET_INDEX(n) (0x200 + (8 * n))
+#define OB_OFFSET_HI(n) (0x204 + (8 * n))
+#define IRQ_EOI 0x050
+#define IRQ_STATUS 0x184
+#define IRQ_ENABLE_SET 0x188
+#define IRQ_ENABLE_CLR 0x18c
+
+/* Config space registers */
+#define DEBUG0 0x728
+#define PL_GEN2 0x80c
+
+static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
+{
+ return sys->private_data;
+}
+
+void dw_v3_65_enable_legacy_irqs(struct pcie_port *pp)
+{
+ int i;
+
+ for (i = 0; i < MAX_LEGACY_IRQS; i++)
+ writel(0x1, pp->va_app_base + IRQ_ENABLE_SET + (i << 4));
+}
+
+void dw_v3_65_handle_legacy_irq(struct pcie_port *pp, int offset)
+{
+ u32 pending;
+ int virq;
+
+ pending = readl(pp->va_app_base + IRQ_STATUS + (offset << 4));
+
+ if (BIT(0) & pending) {
+ virq = irq_linear_revmap(pp->legacy_irq_domain, offset);
+ dev_dbg(pp->dev,
+ ": irq: irq_offset %d, virq %d\n", offset, virq);
+ generic_handle_irq(virq);
+ }
+
+ /* EOI the INTx interrupt */
+ writel(offset, pp->va_app_base + IRQ_EOI);
+}
+
+static void dw_v3_65_ack_irq(struct irq_data *d)
+{
+}
+
+static void dw_v3_65_mask_irq(struct irq_data *d)
+{
+}
+
+static void dw_v3_65_unmask_irq(struct irq_data *d)
+{
+}
+
+static struct irq_chip dw_v3_65_legacy_irq_chip = {
+ .name = "PCI-DW-Legacy-v3.65-irq",
+ .irq_ack = dw_v3_65_ack_irq,
+ .irq_mask = dw_v3_65_mask_irq,
+ .irq_unmask = dw_v3_65_unmask_irq,
+};
+
+static int dw_v3_65_init_legacy_irq_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw_irq)
+{
+ irq_set_chip_and_handler(irq, &dw_v3_65_legacy_irq_chip,
+ handle_level_irq);
+ irq_set_chip_data(irq, d->host_data);
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+static const struct irq_domain_ops dw_v3_65_legacy_irq_domian_ops = {
+ .map = dw_v3_65_init_legacy_irq_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+/**
+ * dw_v3_65_set_ob_regs() - Set PHYADDR <-> BUSADDR
+ * mapping for outbound
+ */
+void dw_v3_65_setup_ob_regs(struct pcie_port *pp)
+{
+ u32 start = pp->mem.start, end = pp->mem.end;
+ int i, tr_size;
+
+ dev_dbg(pp->dev, "Setting outbound translation for %#x-%#x\n",
+ start, end);
+
+ /* Set outbound translation size per window division */
+ writel(CFG_PCIM_WIN_SZ_IDX & 0x7, pp->va_app_base + OB_SIZE);
+
+ tr_size = (1 << (CFG_PCIM_WIN_SZ_IDX & 0x7)) * SZ_1M;
+
+ /* Using Direct 1:1 mapping of RC <-> PCI memory space */
+ for (i = 0; (i < CFG_PCIM_WIN_CNT) && (start < end); i++) {
+ writel(start | 1, pp->va_app_base + OB_OFFSET_INDEX(i));
+ writel(0, pp->va_app_base + OB_OFFSET_HI(i));
+ start += tr_size;
+ }
+
+ /* Enable OB translation */
+ writel(OB_XLAT_EN_VAL | readl(pp->va_app_base + CMD_STATUS),
+ pp->va_app_base + CMD_STATUS);
+}
+
+/**
+ * dw_v3_65_set_dbi_mode() - Set DBI mode to access overlaid BAR mask registers
+ *
+ * Since modification of dbi_cs2 involves different clock domain, read the
+ * status back to ensure the transition is complete.
+ */
+static inline void dw_v3_65_set_dbi_mode(void __iomem *reg_virt)
+{
+ u32 val;
+
+ writel(DBI_CS2_EN_VAL | readl(reg_virt + CMD_STATUS),
+ reg_virt + CMD_STATUS);
+
+ do {
+ val = readl(reg_virt + CMD_STATUS);
+ } while (!(val & DBI_CS2_EN_VAL));
+}
+
+/**
+ * dw_v3_65_clear_dbi_mode() - Disable DBI mode
+ *
+ * Since modification of dbi_cs2 involves different clock domain, read the
+ * status back to ensure the transition is complete.
+ */
+static inline void dw_v3_65_clear_dbi_mode(void __iomem *reg_virt)
+{
+ u32 val;
+
+ writel(~DBI_CS2_EN_VAL & readl(reg_virt + CMD_STATUS),
+ reg_virt + CMD_STATUS);
+
+ do {
+ val = readl(reg_virt + CMD_STATUS);
+ } while (val & DBI_CS2_EN_VAL);
+}
+
+void dw_v3_65_disable_bars(struct pcie_port *pp)
+{
+ dw_v3_65_set_dbi_mode(pp->va_app_base);
+ writel(0, pp->dbi_base + PCI_BASE_ADDRESS_0);
+ writel(0, pp->dbi_base + PCI_BASE_ADDRESS_1);
+ dw_v3_65_clear_dbi_mode(pp->va_app_base);
+}
+
+/**
+ * dw_v3_65_setup_config_addr() - Set up configuration space address for a
+ * device
+ *
+ * @pp: ptr to pcie_port structure
+ * @bus: Bus number the device is residing on
+ * @device: Device number
+ * @function: Function number in device
+ *
+ * Forms and returns the address of configuration space mapped in PCIESS
+ * address space 0. Also configures CFG_SETUP for remote configuration space
+ * access.
+ *
+ * The address space has two regions to access configuration - local and remote.
+ * We access local region for bus 0 (as RC is attached on bus 0) and remote
+ * region for others with TYPE 1 access when bus > 1. As for device on bus = 1,
+ * we will do TYPE 0 access as it will be on our secondary bus (logical).
+ * CFG_SETUP is needed only for remote configuration access.
+ */
+static inline void __iomem *
+dw_v3_65_setup_config_addr(struct pcie_port *pp, u8 bus, u8 device, u8 function)
+{
+ u32 regval;
+
+ if (bus == 0)
+ return pp->dbi_base;
+
+ regval = (bus << 16) | (device << 8) | function;
+ /*
+ * Since Bus#1 will be a virtual bus, we need to have TYPE0
+ * access only.
+ * TYPE 1
+ */
+ if (bus != 1)
+ regval |= BIT(24);
+
+ writel(regval, pp->va_app_base + CFG_SETUP);
+
+ return pp->dbi_base + SPACE0_REMOTE_CFG_OFFSET;
+}
+
+int dw_v3_65_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 *val)
+{
+ u8 bus_num = bus->number;
+ void __iomem *addr;
+ int ret;
+
+ addr = dw_v3_65_setup_config_addr(pp, bus_num, PCI_SLOT(devfn),
+ PCI_FUNC(devfn));
+
+ ret = dw_pcie_cfg_read(addr + (where & ~0x3), where, size, val);
+
+ return ret;
+}
+
+int dw_v3_65_wr_other_conf(struct pcie_port *pp,
+ struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 val)
+{
+ u8 bus_num = bus->number;
+ void __iomem *addr;
+
+ addr = dw_v3_65_setup_config_addr(pp, bus_num, PCI_SLOT(devfn),
+ PCI_FUNC(devfn));
+ return dw_pcie_cfg_write(addr + (where & ~0x3), where, size, val);
+}
+
+/**
+ * dw_v3_65_set_ib_access() - Setup inbound access
+ *
+ * Configure BAR0 for inbound access. BAR0 is set up in h/w to have
+ * access to PCIESS application register space and just needs to set up
+ * inbound address (mainly used for MSI).
+ */
+static void dw_v3_65_set_ib_access(struct pcie_port *pp)
+{
+ /* Configure and set up BAR0 */
+ dw_v3_65_set_dbi_mode(pp->va_app_base);
+
+ /* Enable BAR0 */
+ writel(1, pp->dbi_base + PCI_BASE_ADDRESS_0);
+ writel(SZ_4K - 1, pp->dbi_base + PCI_BASE_ADDRESS_0);
+
+ dw_v3_65_clear_dbi_mode(pp->va_app_base);
+ /*
+ * For BAR0, just setting bus address for inbound writes (MSI) should
+ * be sufficient. Use physical address to avoid any conflicts.
+ */
+ writel(pp->app.start, pp->dbi_base + PCI_BASE_ADDRESS_0);
+}
+
+/**
+ * dw_v3_65_pcie_scan_bus() - common function to scan bus
+ *
+ * common functin to scan v3_65 dw based pci bus. This also sets inbound access
+ * after scan.
+ */
+static struct pci_bus *dw_v3_65_pcie_scan_bus(int nr, struct pci_sys_data *sys)
+{
+ struct pcie_port *pp = sys_to_pcie(sys);
+ struct pci_bus *bus;
+
+ bus = dw_pcie_scan_bus(nr, sys);
+ if (bus)
+ dw_v3_65_set_ib_access(pp);
+
+ return bus;
+}
+
+/**
+ * dw_v3_65_pcie_link_up() - Check if link up
+ *
+ * optionally enable link train using link_train option and check if link is up.
+ */
+int dw_v3_65_pcie_link_up(struct pcie_port *pp, int link_train)
+{
+ u32 val;
+
+ if (link_train) {
+ /*
+ * KeyStone devices do not support h/w autonomous
+ * link up-training to GEN2 from GEN1 in either EP/RC modes.
+ * The software needs to initiate speed change.
+ */
+ val = readl(pp->dbi_base + PL_GEN2);
+ writel(val | DIR_SPD, pp->dbi_base + PL_GEN2);
+ /*
+ * Initiate Link Training. We will delay for L0 as specified by
+ * standard, but will still proceed and return success
+ * irrespective of L0 status as this will be handled by explicit
+ * L0 state checks during enumeration.
+ */
+ val = readl(pp->va_app_base + CMD_STATUS);
+ writel(LTSSM_EN_VAL | val, pp->va_app_base + CMD_STATUS);
+
+ }
+
+ val = readl(pp->dbi_base + DEBUG0);
+
+ return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
+}
+
+static struct hw_pci dw_v3_65_pcie_hw = {
+ .nr_controllers = 1,
+ .setup = dw_pcie_setup,
+ .scan = dw_v3_65_pcie_scan_bus,
+ .add_bus = dw_pcie_add_bus,
+ .map_irq = dw_pcie_map_irq,
+};
+
+
+/**
+ * dw_v3_65_pcie_host_init() - initialize host for v3_65 dw hardware
+ *
+ * Parse the pcie resources from DT bindings and then call common
+ * dw functions to do host initialization.
+ */
+int __init dw_v3_65_pcie_host_init(struct pcie_port *pp,
+ struct device_node *legacy_intc_np,
+ struct device_node *msi_intc_np)
+{
+ int ret;
+
+ /* check if compatible with v3.65 DW h/w */
+ if (!of_device_is_compatible(pp->dev->of_node, "snps,dw-pcie-v3.65")) {
+ dev_err(pp->dev, "Host driver not compatible\n");
+ return -EINVAL;
+ }
+
+ pp->version = DW_V3_65;
+ /* parse PCI bus resources */
+ ret = dw_pcie_parse_resource(pp);
+ if (ret)
+ return ret;
+
+ pp->dbi_base = devm_ioremap_resource(pp->dev, &pp->cfg);
+ if (IS_ERR(pp->dbi_base))
+ return PTR_ERR(pp->dbi_base);
+
+ pp->va_app_base = devm_ioremap_resource(pp->dev, &pp->app);
+ if (IS_ERR(pp->va_app_base))
+ return PTR_ERR(pp->va_app_base);
+
+ /* create legacy irq domain */
+ pp->legacy_irq_domain = irq_domain_add_linear(legacy_intc_np,
+ MAX_LEGACY_IRQS,
+ &dw_v3_65_legacy_irq_domian_ops, NULL);
+
+ if (!pp->legacy_irq_domain) {
+ dev_err(pp->dev, "Failed to add irq domain for legacy irqs\n");
+ return -EINVAL;
+ }
+
+ ret = dw_pcie_msi_host_init(pp, msi_intc_np, &dw_v3_65_msi_domain_ops);
+ if (ret)
+ return ret;
+
+ return dw_pcie_common_host_init(pp, &dw_v3_65_pcie_hw);
+}
diff --git a/drivers/pci/host/pci-dw-v3_65.h b/drivers/pci/host/pci-dw-v3_65.h
index 689256a..a7584e8 100644
--- a/drivers/pci/host/pci-dw-v3_65.h
+++ b/drivers/pci/host/pci-dw-v3_65.h
@@ -18,3 +18,17 @@
extern const struct irq_domain_ops dw_v3_65_msi_domain_ops;
void dw_v3_65_handle_msi_irq(struct pcie_port *pp, int offset);
u32 dw_v3_65_get_msi_data(struct pcie_port *pp);
+
+/* v3.65 specific PCI controller APIs */
+void dw_v3_65_enable_legacy_irqs(struct pcie_port *pp);
+void dw_v3_65_handle_legacy_irq(struct pcie_port *pp, int offset);
+int dw_v3_65_pcie_host_init(struct pcie_port *pp,
+ struct device_node *legacy_intc_np,
+ struct device_node *msi_intc_np);
+int dw_v3_65_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 val);
+int dw_v3_65_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 *val);
+void dw_v3_65_disable_bars(struct pcie_port *pp);
+void dw_v3_65_setup_ob_regs(struct pcie_port *pp);
+int dw_v3_65_pcie_link_up(struct pcie_port *pp, int link_train);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 05bb590..715928e 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -54,6 +54,7 @@ struct pcie_port {
struct {
void __iomem *va_app_base;
struct resource app;
+ struct irq_domain *legacy_irq_domain;
};
};
u64 io_base;
--
1.7.9.5
v3.65 version of the designware h/w, requires application space
registers to be configured to access the remote EP config space.
To support this, add rd[wr]_other_conf API in the pcie_host_opts
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
drivers/pci/host/pcie-designware.c | 12 ++++++++++--
drivers/pci/host/pcie-designware.h | 4 ++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index c4e3732..e8f5d8d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -654,7 +654,11 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
spin_lock_irqsave(&pp->conf_lock, flags);
if (bus->number != pp->root_bus_nr)
- ret = dw_pcie_rd_other_conf(pp, bus, devfn,
+ if (pp->ops->rd_other_conf)
+ ret = pp->ops->rd_other_conf(pp, bus, devfn,
+ where, size, val);
+ else
+ ret = dw_pcie_rd_other_conf(pp, bus, devfn,
where, size, val);
else
ret = dw_pcie_rd_own_conf(pp, where, size, val);
@@ -680,7 +684,11 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
spin_lock_irqsave(&pp->conf_lock, flags);
if (bus->number != pp->root_bus_nr)
- ret = dw_pcie_wr_other_conf(pp, bus, devfn,
+ if (pp->ops->wr_other_conf)
+ ret = pp->ops->wr_other_conf(pp, bus, devfn,
+ where, size, val);
+ else
+ ret = dw_pcie_wr_other_conf(pp, bus, devfn,
where, size, val);
else
ret = dw_pcie_wr_own_conf(pp, where, size, val);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 3063b35..2d6dd66 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -62,6 +62,10 @@ struct pcie_host_ops {
u32 val, void __iomem *dbi_base);
int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
int (*wr_own_conf)(struct pcie_port *pp, int where, int size, u32 val);
+ int (*rd_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 *val);
+ int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 val);
int (*link_up)(struct pcie_port *pp);
void (*host_init)(struct pcie_port *pp);
};
--
1.7.9.5
v3.65 version of the dw hw has MSI controller implemented in the
application space. Add a version variable in the port struct to
identify v3.65 hardware for different code treatment. This variable
will have DW_V3_65 bit set when running on this hw version. The host
init code is expected to set this version based on compatibility
string dw,snps-pcie-v3.65.
Some of the MSI specific functions from current DW driver are re-used
on v3.65 hw. However on v3.65, MSI controller registers are in the
application register space and PCIE_MSI_INTR0_ENABLE is not applicable.
Modify assign_irq() to check for version and not execute the code
for PCIE_MSI_INTR0_ENABLE configuration on v3.65 hw. Additionally
MSI IRQ register in application space is written by EP to raise an MSI
IRQ. So add a get_msi_data() function in pcie_host_ops to retrieve the
register address in dw_msi_setup_irq(). v3.65 dw core driver implements
this function.
Also make some of the functions available in dw core driver global
and make their prototypes available in the header file for re-use on
v3.65.
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
drivers/pci/host/pcie-designware.c | 27 +++++++++++++++++----------
drivers/pci/host/pcie-designware.h | 16 ++++++++++++++++
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index e4bd19a..f985811 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -277,11 +277,15 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
}
set_bit(pos0 + i, pp->msi_irq_in_use);
/*Enable corresponding interrupt in MSI interrupt controller */
- res = ((pos0 + i) / 32) * 12;
- bit = (pos0 + i) % 32;
- dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
- val |= 1 << bit;
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
+ if (!(pp->version & DW_V3_65)) {
+ res = ((pos0 + i) / 32) * 12;
+ bit = (pos0 + i) % 32;
+ dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+ 4, &val);
+ val |= 1 << bit;
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+ 4, val);
+ }
}
*pos = pos0;
@@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
*/
desc->msi_attrib.multiple = msgvec;
- msg.address_lo = virt_to_phys((void *)pp->msi_data);
+ if (pp->ops->get_msi_data)
+ msg.address_lo = pp->ops->get_msi_data(pp);
+ else
+ msg.address_lo = virt_to_phys((void *)pp->msi_data);
msg.address_hi = 0x0;
msg.data = pos;
write_msi_msg(irq, &msg);
@@ -768,7 +775,7 @@ static struct pci_ops dw_pcie_ops = {
.write = dw_pcie_wr_conf,
};
-static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
+int dw_pcie_setup(int nr, struct pci_sys_data *sys)
{
struct pcie_port *pp;
@@ -791,7 +798,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
return 1;
}
-static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
+struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
{
struct pci_bus *bus;
struct pcie_port *pp = sys_to_pcie(sys);
@@ -808,7 +815,7 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
return bus;
}
-static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
{
struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
int irq;
@@ -820,7 +827,7 @@ static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
return irq;
}
-static void dw_pcie_add_bus(struct pci_bus *bus)
+void dw_pcie_add_bus(struct pci_bus *bus)
{
if (IS_ENABLED(CONFIG_PCI_MSI)) {
struct pcie_port *pp = sys_to_pcie(bus->sysdata);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 2d6dd66..3a6a6eb 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -39,6 +39,12 @@ struct pcie_port {
void __iomem *va_cfg0_base;
u64 cfg1_base;
void __iomem *va_cfg1_base;
+ /*
+ * v3.65 DW hw implements application register space for
+ * MSI and has no ATU view port
+ */
+#define DW_V3_65 BIT(0)
+ u32 version;
u64 io_base;
u64 mem_base;
spinlock_t conf_lock;
@@ -68,6 +74,7 @@ struct pcie_host_ops {
unsigned int devfn, int where, int size, u32 val);
int (*link_up)(struct pcie_port *pp);
void (*host_init)(struct pcie_port *pp);
+ u32 (*get_msi_data)(struct pcie_port *pp);
};
int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
@@ -77,5 +84,14 @@ void dw_pcie_msi_init(struct pcie_port *pp);
int dw_pcie_link_up(struct pcie_port *pp);
void dw_pcie_setup_rc(struct pcie_port *pp);
int dw_pcie_host_init(struct pcie_port *pp);
+int dw_pcie_setup(int nr, struct pci_sys_data *sys);
+struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys);
+void dw_pcie_add_bus(struct pci_bus *bus);
+int dw_pcie_parse_resource(struct pcie_port *pp);
+/* internal to DW common core driver */
+int dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw);
+int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
+ const struct irq_domain_ops *irq_ops);
+int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin);
#endif /* _PCIE_DESIGNWARE_H */
--
1.7.9.5
Current DW PCI host init code has code specific to newer hw such as
ATU port specific resource parsing and map. v3.65 DW PCI host has
MSI controller in application space and requires different controller
initialization code. So refactor the msi host controller code into a
separate function. Other common functions across both hw versions
are moved to dw_pcie_common_host_init() and called from the newer and
v3.65 hw host initialization code.
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
1 file changed, 101 insertions(+), 35 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index e8f5d8d..e4bd19a 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
.map = dw_pcie_msi_map,
};
-int __init dw_pcie_host_init(struct pcie_port *pp)
+/*
+ * dw_pcie_parse_resource() - Function to parse common resources
+ *
+ * @pp: ptr to pcie port
+ *
+ * Parse the range property for MEM, IO and cfg resources, and map
+ * the cfg register space.
+ */
+int __init dw_pcie_parse_resource(struct pcie_port *pp)
{
struct device_node *np = pp->dev->of_node;
struct of_pci_range range;
struct of_pci_range_parser parser;
- u32 val;
- int i;
if (of_pci_range_parser_init(&parser, np)) {
dev_err(pp->dev, "missing ranges property\n");
@@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
pp->config.cfg1_size = resource_size(&pp->cfg)/2;
}
}
-
- if (!pp->dbi_base) {
- pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
- resource_size(&pp->cfg));
- if (!pp->dbi_base) {
- dev_err(pp->dev, "error with ioremap\n");
- return -ENOMEM;
- }
- }
-
- 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,
- pp->config.cfg0_size);
- if (!pp->va_cfg0_base) {
- dev_err(pp->dev, "error with ioremap in function\n");
- return -ENOMEM;
- }
- pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
- pp->config.cfg1_size);
- if (!pp->va_cfg1_base) {
- dev_err(pp->dev, "error with ioremap\n");
- return -ENOMEM;
- }
+ return 0;
+}
- if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
- dev_err(pp->dev, "Failed to parse the number of lanes\n");
- return -EINVAL;
- }
+/*
+ * dw_pcie_msi_host_init() - Function to initialize msi host controller
+ *
+ * @pp: ptr to pcie port
+ * @np: device node ptr to msi irq controller
+ * @irq_msi_ops: ptr to MSI irq_domain_ops struct
+ *
+ * Function register irq domain for msi irq controller and create mappings
+ * for MSI irqs.
+ */
+int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
+ const struct irq_domain_ops *irq_msi_ops)
+{
+ int i;
if (IS_ENABLED(CONFIG_PCI_MSI)) {
pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
- MAX_MSI_IRQS, &msi_domain_ops,
+ MAX_MSI_IRQS, irq_msi_ops,
&dw_pcie_msi_chip);
if (!pp->irq_domain) {
dev_err(pp->dev, "irq domain init failed\n");
@@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
irq_create_mapping(pp->irq_domain, i);
}
+ return 0;
+}
+
+/*
+ * dw_pcie_common_host_init() - common host init function across different
+ * versions of the designware PCI controller.
+ * @pp: ptr to pcie port
+ * @hw: ptr to hw_pci structure
+ *
+ * This functions parses common DT properties, call host_init() callback
+ * of the PCI controller driver. Also initialize the common RC configurations
+ * and call common pci core function to intialize the controller driver.
+ */
+int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
+{
+ struct device_node *np = pp->dev->of_node;
+ u32 val;
+
+ if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
+ dev_err(pp->dev, "Failed to parse the number of lanes\n");
+ return -EINVAL;
+ }
+
if (pp->ops->host_init)
pp->ops->host_init(pp);
@@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
val |= PORT_LOGIC_SPEED_CHANGE;
dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
- dw_pci.nr_controllers = 1;
- dw_pci.private_data = (void **)&pp;
+ hw->nr_controllers = 1;
+ hw->private_data = (void **)&pp;
- pci_common_init_dev(pp->dev, &dw_pci);
+ pci_common_init_dev(pp->dev, hw);
pci_assign_unassigned_resources();
#ifdef CONFIG_PCI_DOMAINS
dw_pci.domain++;
@@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
return 0;
}
+/*
+ * dw_pcie_host_init() - Host init function for new designware h/w
+ *
+ * @pp: ptr to pcie port
+ *
+ * The function parse the PCI resurces for IO, Memory and map the config
+ * space addresses. Also initliaze the MSI irq controller and call
+ * dw_pcie_common_host_init() to initialize the PCI controller.
+ */
+int __init dw_pcie_host_init(struct pcie_port *pp)
+{
+ int ret;
+
+ ret = dw_pcie_parse_resource(pp);
+ if (ret)
+ return ret;
+
+ if (!pp->dbi_base) {
+ pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
+ resource_size(&pp->cfg));
+ if (!pp->dbi_base) {
+ dev_err(pp->dev, "error with ioremap\n");
+ return -ENOMEM;
+ }
+ }
+
+ pp->cfg0_base = pp->cfg.start;
+ pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
+
+ pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
+ pp->config.cfg0_size);
+ if (!pp->va_cfg0_base) {
+ dev_err(pp->dev, "error with ioremap in function\n");
+ return -ENOMEM;
+ }
+ pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
+ pp->config.cfg1_size);
+ if (!pp->va_cfg1_base) {
+ dev_err(pp->dev, "error with ioremap\n");
+ return -ENOMEM;
+ }
+
+ ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
+ if (ret < 0)
+ return ret;
+
+ return dw_pcie_common_host_init(pp, &dw_pci);
+}
+
static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
{
/* Program viewport 0 : OUTBOUND : CFG0 */
--
1.7.9.5
Add dw msi controller functions for v3.65 hw. This adds dw_v3_65_msi_chip
and dw_v3_65_msi_domain_ops so that can be used on this version of the hw.
This required since MSI irq registers reside in the application space
for v3.65 hw. The functions are used by v3.65 dw pci core functions to
support implementation of PCI controllers based on this hw version.
While at it, move the ATU hw specific variable and msi irq to a separate
struct inside a union and add another struct inside the union to hold
dw v3.65 specific variables.
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
drivers/pci/host/Kconfig | 5 ++
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++++++++++++++++++++++++++++++
drivers/pci/host/pci-dw-v3_65.h | 20 +++++
drivers/pci/host/pcie-designware.h | 21 +++--
5 files changed, 191 insertions(+), 5 deletions(-)
create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
create mode 100644 drivers/pci/host/pci-dw-v3_65.h
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index a6f67ec..2fcd9f9 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -9,6 +9,11 @@ config PCI_MVEBU
config PCIE_DW
bool
+config PCI_DW_V3_65
+ bool "Designware PCIe h/w v3.65"
+ help
+ Say Y here if the DW h/w version is 3.65
+
config PCI_EXYNOS
bool "Samsung Exynos PCIe controller"
depends on SOC_EXYNOS5440
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 13fb333..28af710 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
+obj-$(CONFIG_PCI_DW_V3_65) += pci-dw-v3_65-msi.o
diff --git a/drivers/pci/host/pci-dw-v3_65-msi.c b/drivers/pci/host/pci-dw-v3_65-msi.c
new file mode 100644
index 0000000..a26ffdd
--- /dev/null
+++ b/drivers/pci/host/pci-dw-v3_65-msi.c
@@ -0,0 +1,149 @@
+/*
+ * Designware(dw) MSI controller version 3.65
+ *
+ * Copyright (C) 2013-2014 Texas Instruments., Ltd.
+ * http://www.ti.com
+ *
+ * Author: Murali Karicheri <[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/of_irq.h>
+#include <linux/kernel.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+
+#include "pcie-designware.h"
+
+#define MSI_IRQ 0x054
+#define MSI0_IRQ_STATUS 0x104
+#define MSI0_IRQ_ENABLE_SET 0x108
+#define MSI0_IRQ_ENABLE_CLR 0x10c
+#define IRQ_STATUS 0x184
+#define IRQ_EOI 0x050
+#define MSI_IRQ_OFFSET 4
+
+static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
+{
+ return sys->private_data;
+}
+
+static inline void update_reg_offset_bit_pos(u32 offset, u32 *reg_offset,
+ u32 *bit_pos)
+{
+ *reg_offset = offset % 8;
+ *bit_pos = offset >> 3;
+}
+
+inline u32 dw_v3_65_get_msi_data(struct pcie_port *pp)
+{
+ return pp->app.start + MSI_IRQ;
+}
+
+void dw_v3_65_handle_msi_irq(struct pcie_port *pp, int offset)
+{
+ u32 pending, vector;
+ int src, virq;
+
+ pending = readl(pp->va_app_base + MSI0_IRQ_STATUS + (offset << 4));
+ /*
+ * MSI0, Status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
+ * shows 1, 9, 17, 25 and so forth
+ */
+ for (src = 0; src < 4; src++) {
+ if (BIT(src) & pending) {
+ vector = offset + (src << 3);
+ virq = irq_linear_revmap(pp->irq_domain, vector);
+ dev_dbg(pp->dev,
+ "irq: bit %d, vector %d, virq %d\n",
+ src, vector, virq);
+ generic_handle_irq(virq);
+ }
+ }
+}
+
+static void dw_v3_65_msi_irq_ack(struct irq_data *d)
+{
+ u32 offset, reg_offset, bit_pos;
+ unsigned int irq = d->irq;
+ struct msi_desc *msi;
+ struct pcie_port *pp;
+
+ msi = irq_get_msi_desc(irq);
+ pp = sys_to_pcie(msi->dev->bus->sysdata);
+ offset = irq - irq_linear_revmap(pp->irq_domain, 0);
+ update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
+
+ writel(BIT(bit_pos),
+ pp->va_app_base + MSI0_IRQ_STATUS + (reg_offset << 4));
+ writel(reg_offset + MSI_IRQ_OFFSET, pp->va_app_base + IRQ_EOI);
+}
+
+static void dw_v3_65_msi_irq_mask(struct irq_data *d)
+{
+ u32 offset, reg_offset, bit_pos;
+ unsigned int irq = d->irq;
+ struct msi_desc *msi;
+ struct pcie_port *pp;
+
+ msi = irq_get_msi_desc(irq);
+ pp = sys_to_pcie(msi->dev->bus->sysdata);
+ offset = irq - irq_linear_revmap(pp->irq_domain, 0);
+ update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
+
+ /* mask the end point if PVM implemented */
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ if (msi->msi_attrib.maskbit)
+ mask_msi_irq(d);
+ }
+
+ writel(BIT(bit_pos),
+ pp->va_app_base + MSI0_IRQ_ENABLE_CLR + (reg_offset << 4));
+}
+
+static void dw_v3_65_msi_irq_unmask(struct irq_data *d)
+{
+ u32 offset, reg_offset, bit_pos;
+ unsigned int irq = d->irq;
+ struct msi_desc *msi;
+ struct pcie_port *pp;
+
+ msi = irq_get_msi_desc(irq);
+ pp = sys_to_pcie(msi->dev->bus->sysdata);
+ offset = irq - irq_linear_revmap(pp->irq_domain, 0);
+ update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
+
+ /* mask the end point if PVM implemented */
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ if (msi->msi_attrib.maskbit)
+ unmask_msi_irq(d);
+ }
+
+ writel(BIT(bit_pos),
+ pp->va_app_base + MSI0_IRQ_ENABLE_SET + (reg_offset << 4));
+}
+
+static struct irq_chip dw_v3_65_msi_chip = {
+ .name = "PCI-DW-MSI-OLD",
+ .irq_ack = dw_v3_65_msi_irq_ack,
+ .irq_mask = dw_v3_65_msi_irq_mask,
+ .irq_unmask = dw_v3_65_msi_irq_unmask,
+};
+
+static int dw_v3_65_msi_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &dw_v3_65_msi_chip, handle_level_irq);
+ irq_set_chip_data(irq, domain->host_data);
+ set_irq_flags(irq, IRQF_VALID);
+
+ return 0;
+}
+
+const struct irq_domain_ops dw_v3_65_msi_domain_ops = {
+ .map = dw_v3_65_msi_map,
+};
diff --git a/drivers/pci/host/pci-dw-v3_65.h b/drivers/pci/host/pci-dw-v3_65.h
new file mode 100644
index 0000000..689256a
--- /dev/null
+++ b/drivers/pci/host/pci-dw-v3_65.h
@@ -0,0 +1,20 @@
+/*
+ * Designware(dw) v3.65 controller common includes
+ *
+ * Copyright (C) 2013-2014 Texas Instruments., Ltd.
+ * http://www.ti.com
+ *
+ * Author: Murali Karicheri <[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.
+ */
+
+#define MAX_LEGACY_IRQS 4
+
+/* v3.65 specific MSI controller APIs/definitions */
+extern const struct irq_domain_ops dw_v3_65_msi_domain_ops;
+void dw_v3_65_handle_msi_irq(struct pcie_port *pp, int offset);
+u32 dw_v3_65_get_msi_data(struct pcie_port *pp);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 3a6a6eb..05bb590 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -35,16 +35,27 @@ struct pcie_port {
struct device *dev;
u8 root_bus_nr;
void __iomem *dbi_base;
- u64 cfg0_base;
- void __iomem *va_cfg0_base;
- u64 cfg1_base;
- void __iomem *va_cfg1_base;
/*
* v3.65 DW hw implements application register space for
* MSI and has no ATU view port
*/
#define DW_V3_65 BIT(0)
u32 version;
+ union {
+ /* New DW hw specific */
+ struct {
+ u64 cfg0_base;
+ void __iomem *va_cfg0_base;
+ u64 cfg1_base;
+ void __iomem *va_cfg1_base;
+ int msi_irq;
+ };
+ /* v3.65 DW hw specific */
+ struct {
+ void __iomem *va_app_base;
+ struct resource app;
+ };
+ };
u64 io_base;
u64 mem_base;
spinlock_t conf_lock;
@@ -55,7 +66,7 @@ struct pcie_port {
int irq;
u32 lanes;
struct pcie_host_ops *ops;
- int msi_irq;
+ /* msi irq domain */
struct irq_domain *irq_domain;
unsigned long msi_data;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
--
1.7.9.5
keystone pcie hardware is based on designware version 3.65.
This driver make use of the functions from pci-dw-old.c and
pci-dw-old-msi.c to implement the driver.
Driver mainly handle the platform specific part of the
PCI driver and depends on DW Old driver to configure
application specific registers. Also routes the irq events
and ack the interrupt after the same is acked by the end
point device driver. This requires irqchip implementation
for legacy and MSI irq handling.
Signed-off-by: Murali Karicheri <[email protected]>
Signed-off-by: Grygorii Strashko <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
.../devicetree/bindings/pci/designware-pcie.txt | 42 ++
.../devicetree/bindings/pci/pci-keystone.txt | 56 +++
drivers/pci/host/Kconfig | 7 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
5 files changed, 524 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
create mode 100644 drivers/pci/host/pci-keystone.c
diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d6fae13..3ea00f9 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -81,3 +81,45 @@ Board specific DT Entry:
pcie@2a0000 {
reset-gpio = <&pin_ctrl 22 0>;
};
+
+* Synopsys Designware PCIe interface for v3.65 hardware.
+========================================================
+
+v3.65 hardware has application registers for msi/irq and inbound/outbound
+translation configuration. So the core designware driver has changes to
+support this difference in hardware. Following are the diffs:-
+
+Required Properties:
+- compatible: should contain additionally "snps,dw-pcie-v3.65" to identify
+ v3.65 hardware.
+- reg: index0 should be base addresses and lengths of the config space (first
+ 4K for RC and next 4K for EP). index1 is base address and length of
+ application space (4K). Other indexes are specific to an implementation.
+- interrupts: N/A
+- clock-names: from common clock binding: should be "pcie"
+
+Other Required properties are present
+
+Example
+
+ pcie@21800000 {
+ compatible = "ti,keystone-pcie", "snps,dw-pcie-v3.65", "snps,dw-pcie";
+ device_type = "pci";
+ clocks = <&clkpcie>;
+ clock-names = "pcie";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ reg = <0x21801000 0x2000>, <0x21800000 0x1000>;
+
+ ranges = <0x81000000 0 0x00000000 0x24000000 0 0x00004000 /* downstream I/O */
+ 0x82000000 0 0x50000000 0x50000000 0 0x10000000>; /* non-prefetchable memory */
+
+ num-lanes = <2>;
+
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 7>;
+ interrupt-map = <0 0 0 1 &pcie_intc 0>, // INT A
+ <0 0 0 2 &pcie_intc 1>, // INT B
+ <0 0 0 3 &pcie_intc 2>, // INT C
+ <0 0 0 4 &pcie_intc 3>; // INT D
+ };
diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
new file mode 100644
index 0000000..dd72cb2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -0,0 +1,56 @@
+DT binding documentation for Keystone PCI Controller
+====================================================
+
+Keystone PCI Controller is compliant to Designware PCI controller v3.65.
+Please refer to Documentation/devicetree/bindings/pci/designware-pci.txt
+for the details. Additional attributes are described here.
+
+Required Properties:-
+
+reg: index 2 is the base address and length of PCI mode configuration register
+ index 3 is the base address and length of PCI device ID register.
+
+pcie_msi_intc : Interrupt controller device node for MSI irq chip
+ interrupt-cells: should be set to 1
+ interrupt-parent: Parent interrupt controller phandle
+ interrupts: GIC interrupt lines connected to PCI MSI interrupt lines
+
+ Example:
+ pcie_msi_intc: msi-interrupt-controller {
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 30 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 31 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 32 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 33 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 34 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 37 IRQ_TYPE_EDGE_RISING>;
+ };
+
+pcie_intc: Interrupt controller device node for Legacy irq chip
+ interrupt-cells: should be set to 1
+ interrupt-parent: Parent interrupt controller phandle
+ interrupts: GIC interrupt lines connected to PCI Legacy interrupt lines
+
+ Example:
+ pcie_intc: legacy-interrupt-controller {
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 27 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 28 IRQ_TYPE_EDGE_RISING>,
+ <GIC_SPI 29 IRQ_TYPE_EDGE_RISING>;
+ };
+
+Optional properties:-
+ phys: phandle to Generic Keystone SerDes phy for PCI
+ phy-names: name of the Generic Keystine SerDes phy for PCI
+ - If boot loader already does PCI link establishment, then phys and
+ phy-names shouldn't be present.
+ ti,enable-linktrain - Enable Link training.
+ - If boot loader already does PCI link establishment, then this
+ shouldn't be present.
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 2fcd9f9..a019b77 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -38,4 +38,11 @@ config PCI_RCAR_GEN2
There are 3 internal PCI controllers available with a single
built-in EHCI/OHCI host controller present on each one.
+config PCI_KEYSTONE
+ bool "TI Keystone PCIe controller"
+ depends on ARCH_KEYSTONE
+ select PCIE_DW
+ select PCI_DW_V3_65
+ select PCIEPORTBUS
+ select PHY_TI_KEYSTONE_SERDES
endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index b44a878..d7c1857 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
obj-$(CONFIG_PCI_DW_V3_65) += pci-dw-v3_65-msi.o pci-dw-v3_65.o
+obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
new file mode 100644
index 0000000..d82ac7f
--- /dev/null
+++ b/drivers/pci/host/pci-keystone.c
@@ -0,0 +1,418 @@
+/*
+ * PCIe host controller driver for Texas Instruments Keystone SoCs
+ *
+ * Copyright (C) 2013-2014 Texas Instruments., Ltd.
+ * http://www.ti.com
+ *
+ * Author: Murali Karicheri <[email protected]>
+ * Implementation based on pci-exynos.c and pcie-designware.c
+ *
+ * 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/irqchip/chained_irq.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_irq.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+
+#include "pcie-designware.h"
+#include "pci-dw-v3_65.h"
+
+#define DRIVER_NAME "keystone-pcie"
+
+/* driver specific constants */
+#define MAX_MSI_HOST_IRQS 8
+#define MAX_LEGACY_HOST_IRQS 4
+
+/* RC mode settings */
+#define PCIE_RC_MODE (BIT(2))
+#define PCIE_MODE_MASK (BIT(1) | BIT(2))
+
+/* DEV_STAT_CTRL */
+#define PCIE_CAP_BASE 0x70
+
+struct keystone_pcie {
+ struct clk *clk;
+ int en_link_train;
+ struct pcie_port pp;
+ void __iomem *va_reg_pciid;
+
+ int num_legacy_host_irqs;
+ int legacy_host_irqs[MAX_LEGACY_HOST_IRQS];
+ struct device_node *legacy_intc_np;
+
+ int num_msi_host_irqs;
+ int msi_host_irqs[MAX_MSI_HOST_IRQS];
+ struct device_node *msi_intc_np;
+};
+
+#define to_keystone_pcie(x) container_of(x, struct keystone_pcie, pp)
+
+static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie)
+{
+ struct pcie_port *pp = &ks_pcie->pp;
+ int count = 200;
+
+ dw_pcie_setup_rc(pp);
+
+ /* check if the link is up or not */
+ while (!dw_pcie_link_up(pp)) {
+ usleep_range(100, 1000);
+ if (--count)
+ continue;
+ dev_err(pp->dev, "phy link never came up\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void ks_pcie_msi_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+ struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
+ u32 offset = irq - ks_pcie->msi_host_irqs[0];
+ struct pcie_port *pp = &ks_pcie->pp;
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+
+ dev_dbg(pp->dev, "ks_pcie_msi_irq_handler, irq %d\n", irq);
+
+ /*
+ * The chained irq handler installation would have replaced normal
+ * interrupt driver handler so we need to take care of mask/unmask and
+ * ack operation.
+ */
+ chained_irq_enter(chip, desc);
+ dw_v3_65_handle_msi_irq(pp, offset);
+ chained_irq_exit(chip, desc);
+}
+
+/**
+ * ks_pcie_legacy_irq_handler() - Handle legacy interrupt
+ * @irq: IRQ line for legacy interrupts
+ * @desc: Pointer to irq descriptor
+ *
+ * Traverse through pending legacy interrupts and invoke handler for each. Also
+ * takes care of interrupt controller level mask/ack operation.
+ */
+static void ks_pcie_legacy_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+ struct keystone_pcie *ks_pcie = irq_desc_get_handler_data(desc);
+ u32 irq_offset = irq - ks_pcie->legacy_host_irqs[0];
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct pcie_port *pp = &ks_pcie->pp;
+
+ dev_dbg(ks_pcie->pp.dev, ": Handling legacy irq %d\n", irq);
+
+ /*
+ * The chained irq handler installation would have replaced normal
+ * interrupt driver handler so we need to take care of mask/unmask and
+ * ack operation.
+ */
+ chained_irq_enter(chip, desc);
+ dw_v3_65_handle_legacy_irq(pp, irq_offset);
+ chained_irq_exit(chip, desc);
+}
+
+static int ks_pcie_get_irq_controller_info(struct keystone_pcie *ks_pcie,
+ char *controller, int *num_irqs)
+{
+ int temp, max_host_irqs, legacy = 1, *host_irqs, ret = -EINVAL;
+ struct device *dev = ks_pcie->pp.dev;
+ struct device_node *np_pcie = dev->of_node, **np_temp;
+
+ if (!strcmp(controller, "msi-interrupt-controller"))
+ legacy = 0;
+
+ if (legacy) {
+ np_temp = &ks_pcie->legacy_intc_np;
+ max_host_irqs = MAX_LEGACY_HOST_IRQS;
+ host_irqs = &ks_pcie->legacy_host_irqs[0];
+ } else {
+ np_temp = &ks_pcie->msi_intc_np;
+ max_host_irqs = MAX_MSI_HOST_IRQS;
+ host_irqs = &ks_pcie->msi_host_irqs[0];
+ }
+
+ /* interrupt controller is in a child node */
+ *np_temp = of_find_node_by_name(np_pcie, controller);
+ if (!(*np_temp)) {
+ dev_err(dev, "Node for %s is absent\n", controller);
+ goto out;
+ }
+ temp = of_irq_count(*np_temp);
+ if (!temp)
+ goto out;
+ if (temp > max_host_irqs)
+ dev_warn(dev, "Too many %s interrupts defined %u\n",
+ (legacy ? "legacy" : "MSI"), temp);
+
+ /*
+ * support upto max_host_irqs. In dt from index 0 to 3 (legacy) or 0 to
+ * 7 (MSI)
+ */
+ for (temp = 0; temp < max_host_irqs; temp++) {
+ host_irqs[temp] = irq_of_parse_and_map(*np_temp, temp);
+ if (host_irqs[temp] < 0)
+ break;
+ }
+ if (temp) {
+ *num_irqs = temp;
+ ret = 0;
+ }
+out:
+ return ret;
+}
+
+static void ks_pcie_enable_interrupts(struct keystone_pcie *ks_pcie)
+{
+ struct pcie_port *pp = &ks_pcie->pp;
+ int i;
+
+ /* Legacy IRQ */
+ for (i = 0; i < ks_pcie->num_legacy_host_irqs; i++) {
+ irq_set_handler_data(ks_pcie->legacy_host_irqs[i], ks_pcie);
+ irq_set_chained_handler(ks_pcie->legacy_host_irqs[i],
+ ks_pcie_legacy_irq_handler);
+ }
+ dw_v3_65_enable_legacy_irqs(pp);
+
+ /* MSI IRQ */
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ for (i = 0; i < ks_pcie->num_msi_host_irqs; i++) {
+ irq_set_chained_handler(ks_pcie->msi_host_irqs[i],
+ ks_pcie_msi_irq_handler);
+ irq_set_handler_data(ks_pcie->msi_host_irqs[i],
+ ks_pcie);
+
+ }
+ }
+
+ return;
+}
+
+/*
+ * When a PCI device does not exist during config cycles, keystone host gets a
+ * bus error instead of returning 0xffffffff. This handler always returns 0
+ * for this kind of faults.
+ */
+static int
+keystone_pcie_fault(unsigned long addr, unsigned int fsr,
+ struct pt_regs *regs)
+{
+ unsigned long instr = *(unsigned long *) instruction_pointer(regs);
+
+ if ((instr & 0x0e100090) == 0x00100090) {
+ int reg = (instr >> 12) & 15;
+
+ regs->uregs[reg] = -1;
+ regs->ARM_pc += 4;
+ }
+
+ return 0;
+}
+
+static void __init ks_pcie_host_init(struct pcie_port *pp)
+{
+ u32 vendor_device_id, val;
+ struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+
+ ks_pcie_establish_link(ks_pcie);
+ dw_v3_65_disable_bars(pp);
+ dw_v3_65_setup_ob_regs(pp);
+ ks_pcie_enable_interrupts(ks_pcie);
+ writew(PCI_IO_RANGE_TYPE_32 | (PCI_IO_RANGE_TYPE_32 << 8),
+ pp->dbi_base + PCI_IO_BASE);
+
+ /* update the Vendor ID */
+ vendor_device_id = readl(ks_pcie->va_reg_pciid);
+ writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
+
+ /* update the DEV_STAT_CTRL to publish right mrrs */
+ val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
+ val &= ~PCI_EXP_DEVCTL_READRQ;
+ /* set the mrrs to 256 bytes */
+ val |= BIT(12);
+ writel(val, pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
+
+ /*
+ * PCIe access errors that result into OCP errors are caught by ARM as
+ * "External aborts"
+ */
+ hook_fault_code(17, keystone_pcie_fault, SIGBUS, 0,
+ "Asynchronous external abort");
+}
+
+int ks_pcie_link_up(struct pcie_port *pp)
+{
+ struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+
+ return dw_v3_65_pcie_link_up(pp, ks_pcie->en_link_train);
+}
+
+static struct pcie_host_ops keystone_pcie_host_ops = {
+ .rd_other_conf = dw_v3_65_rd_other_conf,
+ .wr_other_conf = dw_v3_65_wr_other_conf,
+ .link_up = ks_pcie_link_up,
+ .host_init = ks_pcie_host_init,
+ .get_msi_data = dw_v3_65_get_msi_data,
+};
+
+static int add_pcie_port(struct keystone_pcie *ks_pcie,
+ struct platform_device *pdev)
+{
+ struct pcie_port *pp = &ks_pcie->pp;
+ int ret;
+
+ ret = ks_pcie_get_irq_controller_info(ks_pcie,
+ "legacy-interrupt-controller",
+ &ks_pcie->num_legacy_host_irqs);
+ if (ret)
+ return ret;
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ ret = ks_pcie_get_irq_controller_info(ks_pcie,
+ "msi-interrupt-controller",
+ &ks_pcie->num_msi_host_irqs);
+ if (ret)
+ return ret;
+ }
+
+ pp->root_bus_nr = -1;
+ pp->ops = &keystone_pcie_host_ops;
+ spin_lock_init(&pp->conf_lock);
+ ret = dw_v3_65_pcie_host_init(pp, ks_pcie->legacy_intc_np,
+ ks_pcie->msi_intc_np);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize host\n");
+ return ret;
+ }
+
+ return ret;
+}
+
+static const struct of_device_id ks_pcie_of_match[] = {
+ {
+ .type = "pci",
+ .compatible = "ti,keystone-pcie",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ks_pcie_of_match);
+
+static int __exit ks_pcie_remove(struct platform_device *pdev)
+{
+ struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
+
+ clk_disable_unprepare(ks_pcie->clk);
+
+ return 0;
+}
+
+static int __init ks_pcie_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct keystone_pcie *ks_pcie;
+ struct pcie_port *pp;
+ struct resource *res;
+ void __iomem *reg_p;
+ struct phy *phy;
+ int ret = 0;
+ u32 val;
+
+ ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
+ GFP_KERNEL);
+ if (!ks_pcie) {
+ dev_err(dev, "no memory for keystone pcie\n");
+ return -ENOMEM;
+ }
+
+ pp = &ks_pcie->pp;
+
+ /* If phy is initialized by a boot loader, the below can be skipped */
+ phy = devm_phy_get(dev, "pcie-phy");
+ if (phy) {
+ ret = phy_init(phy);
+ if (ret < 0)
+ return ret;
+ }
+
+ /* index 0 is the config reg. space address */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pp->cfg = *res;
+
+ /* index 1 is the application reg. space address */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ pp->app = *res;
+
+ /* index 2 is the devcfg register for RC mode settings */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+ reg_p = devm_ioremap_resource(dev, res);
+ if (IS_ERR(reg_p))
+ return PTR_ERR(reg_p);
+
+ /* enable RC mode in devcfg */
+ val = readl(reg_p);
+ val &= ~PCIE_MODE_MASK;
+ val |= PCIE_RC_MODE;
+ writel(val, reg_p);
+
+ /* index 3 is to read PCI DEVICE_ID */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+ reg_p = devm_ioremap_resource(dev, res);
+ if (IS_ERR(reg_p))
+ return PTR_ERR(reg_p);
+ ks_pcie->va_reg_pciid = reg_p;
+
+ /* check if we need to enable link training */
+ ks_pcie->en_link_train =
+ (of_get_property(np, "ti,enable-linktrain", NULL) != NULL);
+
+ pp->dev = dev;
+ platform_set_drvdata(pdev, ks_pcie);
+ ks_pcie->clk = devm_clk_get(dev, "pcie");
+ if (IS_ERR(ks_pcie->clk)) {
+ dev_err(dev, "Failed to get pcie rc clock\n");
+ return PTR_ERR(ks_pcie->clk);
+ }
+ ret = clk_prepare_enable(ks_pcie->clk);
+ if (ret)
+ return ret;
+
+ ret = add_pcie_port(ks_pcie, pdev);
+ if (ret < 0)
+ goto fail_clk;
+
+ return 0;
+fail_clk:
+ clk_disable_unprepare(ks_pcie->clk);
+
+ return ret;
+}
+
+static struct platform_driver ks_pcie_driver __refdata = {
+ .probe = ks_pcie_probe,
+ .remove = __exit_p(ks_pcie_remove),
+ .driver = {
+ .name = "keystone-pcie",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(ks_pcie_of_match),
+ },
+};
+
+module_platform_driver(ks_pcie_driver);
+
+MODULE_AUTHOR("Murali Karicheri <[email protected]>");
+MODULE_DESCRIPTION("Keystone PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
This phy driver added to support keystone PCI driver. This is
a generic phy driver and can work with multiple hardware IPs
available on Keystone SoC. The hw vendor that provides the phy
hw published only registers and their values. So this driver
uses these hard coded values to initialize the phy.
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
.../bindings/phy/phy-keystone-serdes.txt | 25 +++
drivers/phy/Kconfig | 6 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-keystone-serdes.c | 230 ++++++++++++++++++++
4 files changed, 262 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
create mode 100644 drivers/phy/phy-keystone-serdes.c
diff --git a/Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt b/Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
new file mode 100644
index 0000000..612c7b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
@@ -0,0 +1,25 @@
+TI Keystone SerDes Phy: DT documentation
+=======================================
+
+Required properties:
+ - compatible: should be "ti,keystone-serdes-phy";
+ - reg: base address and length of the SerDes register set
+ - reg-names: should be "reg_serdes", name of the reg SerDes register set
+ - #phy-cells: from the generic phy bindings, must be 0;
+
+Exampel for Keystone PCIE,
+
+ pcie0_phy: serdes_phy@2320000 {
+ #phy-cells = <0>;
+ compatible = "ti,keystone-serdes-phy";
+ reg = <0x02320000 0x4000>;
+ reg-names = "reg_serdes";
+ };
+
+
+Then the PHY can be used in PCIe controller node as
+
+pcie@21800000 {
+ phys = <&pcie0_phy>;
+ phy-names = "pcie-phy";
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 4906c27..8b652a4 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -167,4 +167,10 @@ config PHY_XGENE
help
This option enables support for APM X-Gene SoC multi-purpose PHY.
+config PHY_TI_KEYSTONE_SERDES
+ bool "TI Keystone SerDes PHY support"
+ depends on ARCH_KEYSTONE
+ select GENERIC_PHY
+ help
+ This option enables support for TI Keystone SerDes PHY.
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 7728518..c9f2dd4 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -19,3 +19,4 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o
phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o
phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
+obj-$(CONFIG_PHY_TI_KEYSTONE_SERDES) += phy-keystone-serdes.o
diff --git a/drivers/phy/phy-keystone-serdes.c b/drivers/phy/phy-keystone-serdes.c
new file mode 100644
index 0000000..6d52ab4
--- /dev/null
+++ b/drivers/phy/phy-keystone-serdes.c
@@ -0,0 +1,230 @@
+/*
+ * Keystone SerDes Phy driver code
+ *
+ * Copyright (C) 2013-2014 Texas Instruments, Inc.
+ * http://www.ti.com
+ *
+ * Author: Murali Karicheri <[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/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define reg_dump(addr, mask) \
+ pr_debug("reg %p has value %x\n", (void *)addr, \
+ (readl(addr) & ~mask))
+
+/* mask bits point to bits being modified */
+#define reg_rmw(addr, value, mask) \
+ writel(((readl(addr) & (~(mask))) | \
+ (value & (mask))), (addr))
+struct serdes_config {
+ u32 reg;
+ u32 val;
+ u32 mask;
+};
+
+struct ks_serdes_phy {
+ struct device *dev;
+ void __iomem *base;
+};
+
+static struct serdes_config ks_100mhz_5gbps_serdes[] = {
+ {0x0000, 0x00000800, 0x0000ff00},
+ {0x0060, 0x00041c5c, 0x00ffffff},
+ {0x0064, 0x0343c700, 0xffffff00},
+ {0x006c, 0x00000012, 0x000000ff},
+ {0x0068, 0x00070000, 0x00ff0000},
+ {0x0078, 0x0000c000, 0x0000ff00},
+
+ {0x0200, 0x00000000, 0x000000ff},
+ {0x0204, 0x5e000080, 0xff0000ff},
+ {0x0208, 0x00000006, 0x000000ff},
+ {0x0210, 0x00000023, 0x000000ff},
+ {0x0214, 0x2e003060, 0xff00ffff},
+ {0x0218, 0x76000000, 0xff000000},
+ {0x022c, 0x00200002, 0x00ff00ff},
+ {0x02a0, 0xffee0000, 0xffff0000},
+ {0x02a4, 0x0000000f, 0x000000ff},
+ {0x0204, 0x5e000000, 0xff000000},
+ {0x0208, 0x00000006, 0x000000ff},
+ {0x0278, 0x00002000, 0x0000ff00},
+ {0x0280, 0x00280028, 0x00ff00ff},
+ {0x0284, 0x2d0f0385, 0xffffffff},
+ {0x0250, 0xd0000000, 0xff000000},
+ {0x0284, 0x00000085, 0x000000ff},
+ {0x0294, 0x20000000, 0xff000000},
+
+ {0x0400, 0x00000000, 0x000000ff},
+ {0x0404, 0x5e000080, 0xff0000ff},
+ {0x0408, 0x00000006, 0x000000ff},
+ {0x0410, 0x00000023, 0x000000ff},
+ {0x0414, 0x2e003060, 0xff00ffff},
+ {0x0418, 0x76000000, 0xff000000},
+ {0x042c, 0x00200002, 0x00ff00ff},
+ {0x04a0, 0xffee0000, 0xffff0000},
+ {0x04a4, 0x0000000f, 0x000000ff},
+ {0x0404, 0x5e000000, 0xff000000},
+ {0x0408, 0x00000006, 0x000000ff},
+ {0x0478, 0x00002000, 0x0000ff00},
+ {0x0480, 0x00280028, 0x00ff00ff},
+ {0x0484, 0x2d0f0385, 0xffffffff},
+ {0x0450, 0xd0000000, 0xff000000},
+ {0x0494, 0x20000000, 0xff000000},
+
+ {0x0604, 0x00000080, 0x000000ff},
+ {0x0600, 0x00000000, 0x000000ff},
+ {0x0604, 0x5e000000, 0xff000000},
+ {0x0608, 0x00000006, 0x000000ff},
+ {0x0610, 0x00000023, 0x000000ff},
+ {0x0614, 0x2e003060, 0xff00ffff},
+ {0x0618, 0x76000000, 0xff000000},
+ {0x062c, 0x00200002, 0x00ff00ff},
+ {0x06a0, 0xffee0000, 0xffff0000},
+ {0x06a4, 0x0000000f, 0x000000ff},
+ {0x0604, 0x5e000000, 0xff000000},
+ {0x0608, 0x00000006, 0x000000ff},
+ {0x0678, 0x00002000, 0x0000ff00},
+ {0x0680, 0x00280028, 0x00ff00ff},
+ {0x0684, 0x2d0f0385, 0xffffffff},
+ {0x0650, 0xd0000000, 0xff000000},
+ {0x0694, 0x20000000, 0xff000000},
+
+ {0x0800, 0x00000000, 0x000000ff},
+ {0x0804, 0x5e000080, 0xff0000ff},
+ {0x0808, 0x00000006, 0x000000ff},
+ {0x0810, 0x00000023, 0x000000ff},
+ {0x0814, 0x2e003060, 0xff00ffff},
+ {0x0818, 0x76000000, 0xff000000},
+ {0x082c, 0x00200002, 0x00ff00ff},
+ {0x08a0, 0xffee0000, 0xffff0000},
+ {0x08a4, 0x0000000f, 0x000000ff},
+ {0x0804, 0x5e000000, 0xff000000},
+ {0x0808, 0x00000006, 0x000000ff},
+ {0x0878, 0x00002000, 0x0000ff00},
+ {0x0880, 0x00280028, 0x00ff00ff},
+ {0x0884, 0x2d0f0385, 0xffffffff},
+ {0x0850, 0xd0000000, 0xff000000},
+ {0x0894, 0x20000000, 0xff000000},
+
+ {0x0a00, 0x00000100, 0x0000ff00},
+ {0x0a08, 0x00e12c08, 0x00ffffff},
+ {0x0a0c, 0x00000081, 0x000000ff},
+ {0x0a18, 0x00e80000, 0x00ff0000},
+ {0x0a30, 0x002f2f00, 0x00ffff00},
+ {0x0a4c, 0xac820000, 0xffff0000},
+ {0x0a54, 0xc0000000, 0xff000000},
+ {0x0a58, 0x00001441, 0x0000ffff},
+ {0x0a84, 0x00000301, 0x0000ffff},
+
+ {0x0a8c, 0x81030000, 0xffff0000},
+ {0x0a90, 0x00006001, 0x0000ffff},
+ {0x0a94, 0x01000000, 0xff000000},
+ {0x0aa0, 0x81000000, 0xff000000},
+ {0x0abc, 0xff000000, 0xff000000},
+ {0x0ac0, 0x0000008b, 0x000000ff},
+
+ {0x0000, 0x00000003, 0x000000ff},
+ {0x0a00, 0x0000009f, 0x000000ff},
+
+ {0x0a44, 0x5f733d00, 0xffffff00},
+ {0x0a48, 0x00fdca00, 0x00ffff00},
+ {0x0a5c, 0x00000000, 0xffff0000},
+ {0x0a60, 0x00008000, 0xffffffff},
+ {0x0a64, 0x0c581220, 0xffffffff},
+ {0x0a68, 0xe13b0602, 0xffffffff},
+ {0x0a6c, 0xb8074cc1, 0xffffffff},
+ {0x0a70, 0x3f02e989, 0xffffffff},
+ {0x0a74, 0x00000001, 0x000000ff},
+ {0x0b14, 0x00370000, 0x00ff0000},
+ {0x0b10, 0x37000000, 0xff000000},
+ {0x0b14, 0x0000005d, 0x000000ff},
+};
+
+static int ks_serdes_phy_init(struct phy *phy)
+{
+ struct serdes_config *p;
+ struct ks_serdes_phy *ks_phy = phy_get_drvdata(phy);
+
+ int i;
+
+ for (i = 0, p = &ks_100mhz_5gbps_serdes[0];
+ i < ARRAY_SIZE(ks_100mhz_5gbps_serdes);
+ i++, p++) {
+ reg_rmw((ks_phy->base + p->reg), p->val, p->mask);
+ reg_dump((ks_phy->base + p->reg), p->mask);
+ }
+ msleep(20);
+
+ return 0;
+}
+
+static struct phy_ops ks_serdes_phy_ops = {
+ .init = ks_serdes_phy_init,
+ .owner = THIS_MODULE,
+};
+
+static int ks_serdes_phy_probe(struct platform_device *pdev)
+{
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ struct ks_serdes_phy *ks_phy;
+ struct phy *phy;
+ struct resource *res;
+
+ ks_phy = devm_kzalloc(dev, sizeof(*ks_phy), GFP_KERNEL);
+ if (!ks_phy)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg_serdes");
+ ks_phy->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ks_phy->base))
+ return PTR_ERR(ks_phy->base);
+
+ ks_phy->dev = dev;
+ phy = devm_phy_create(dev, &ks_serdes_phy_ops, NULL);
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
+
+ phy_set_drvdata(phy, ks_phy);
+ phy_provider = devm_of_phy_provider_register(ks_phy->dev,
+ of_phy_simple_xlate);
+
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ dev_info(dev, "Keystone SerDes phy initialized\n");
+ return 0;
+}
+
+static const struct of_device_id ks_serdes_phy_of_match[] = {
+ { .compatible = "ti,keystone-serdes-phy" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, ks_serdes_phy_of_match);
+
+static struct platform_driver ks_serdes_phy_driver = {
+ .probe = ks_serdes_phy_probe,
+ .driver = {
+ .of_match_table = ks_serdes_phy_of_match,
+ .name = "ti,keystone-serdes-phy",
+ .owner = THIS_MODULE,
+ }
+};
+module_platform_driver(ks_serdes_phy_driver);
+
+MODULE_DESCRIPTION("TI Keystone SerDes PHY driver");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Murali Karicheri <[email protected]>");
--
1.7.9.5
Add pcie related options by default for keystone architecture
Signed-off-by: Murali Karicheri <[email protected]>
CC: Santosh Shilimkar <[email protected]>
CC: Russell King <[email protected]>
CC: Grant Likely <[email protected]>
CC: Rob Herring <[email protected]>
CC: Mohit Kumar <[email protected]>
CC: Jingoo Han <[email protected]>
CC: Bjorn Helgaas <[email protected]>
CC: Pratyush Anand <[email protected]>
CC: Richard Zhu <[email protected]>
CC: Kishon Vijay Abraham I <[email protected]>
CC: Marek Vasut <[email protected]>
CC: Arnd Bergmann <[email protected]>
CC: Pawel Moll <[email protected]>
CC: Mark Rutland <[email protected]>
CC: Ian Campbell <[email protected]>
CC: Kumar Gala <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: Grant Likely <[email protected]>
---
arch/arm/mach-keystone/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-keystone/Kconfig b/arch/arm/mach-keystone/Kconfig
index f50bc93..8f06806 100644
--- a/arch/arm/mach-keystone/Kconfig
+++ b/arch/arm/mach-keystone/Kconfig
@@ -8,6 +8,7 @@ config ARCH_KEYSTONE
select COMMON_CLK_KEYSTONE
select ARCH_SUPPORTS_BIG_ENDIAN
select ZONE_DMA if ARM_LPAE
+ select MIGHT_HAVE_PCI
help
Support for boards based on the Texas Instruments Keystone family of
SoCs.
--
1.7.9.5
On Tue, Jun 10, 2014 at 02:51:19PM -0400, Murali Karicheri wrote:
> This patch adds a PCIe controller driver for Keystone SoCs. This
> is based on v1 of the series posted to the mailing list.
>
> CC: Santosh Shilimkar <[email protected]>
> CC: Russell King <[email protected]>
> CC: Grant Likely <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Mohit Kumar <[email protected]>
> CC: Jingoo Han <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Pratyush Anand <[email protected]>
> CC: Richard Zhu <[email protected]>
> CC: Kishon Vijay Abraham I <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Arnd Bergmann <[email protected]>
> CC: Pawel Moll <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: Kumar Gala <[email protected]>
> CC: Randy Dunlap <[email protected]>
> CC: Grant Likely <[email protected]>
>
>
> Changelog:
>
> V2
> - Split the designware pcie enhancement patch to multiple
> patches based on functionality added
> - Remove the quirk code and add a patch to fix mps/mrss
> tuning for ARM. Use kernel command line parameter
> pci=pcie_bus_perf to work with Keystone PCI Controller.
> Following patch addressed this.
> [PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
> - Add documentation for device tree bindings
> - Add separate interrupt controller nodes for MSI and Legacy
> IRQs and use irq map for legacy IRQ
> - Use compatibility to identify v3.65 version of the DW hardware
> and use it to customize the designware common code.
> - Use reg property for configuration space instead of range
> - Other minor updates based on code inspection.
>
> V1
> - Add an interrupt controller node for Legacy irq chip and use
> interrupt map/map-mask property to map legacy IRQs A/B/C/D
> - Add a Phy driver to replace the original serdes driver
> - Move common application register handling code to a separate
> file to allow re-use across other platforms that use older
> DW PCIe h/w
> - PCI quirk for maximum read request size. Check and override only
> if the maximum is higher than what controller can handle.
> - Converted to a module platform driver.
>
>
> Murali Karicheri (8):
> PCI: designware: add rd[wr]_other_conf API
> PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
> PCI: designware: update pcie core driver to work with dw hw version
> 3.65
> PCI: designware: add msi controller functions for v3.65 hw
> PCI: designware: add PCI controller functions for v3.65 DW hw
> phy: Add serdes phy driver for keystone
> PCI: keystone: add pcie driver based on designware core driver
> ARM: keystone: add pcie related options
>
> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> .../bindings/phy/phy-keystone-serdes.txt | 25 ++
> arch/arm/mach-keystone/Kconfig | 1 +
> drivers/pci/host/Kconfig | 12 +
> drivers/pci/host/Makefile | 2 +
> drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
> drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
> drivers/pci/host/pci-dw-v3_65.h | 34 ++
> drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
> drivers/pci/host/pcie-designware.c | 175 +++++---
> drivers/pci/host/pcie-designware.h | 42 +-
> drivers/phy/Kconfig | 6 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
> 15 files changed, 1531 insertions(+), 52 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
> create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
> create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
> create mode 100644 drivers/pci/host/pci-dw-v3_65.c
> create mode 100644 drivers/pci/host/pci-dw-v3_65.h
> create mode 100644 drivers/pci/host/pci-keystone.c
> create mode 100644 drivers/phy/phy-keystone-serdes.c
I'm not willing to merge phy-keystone-serdes.c because I don't
maintain drivers/phy and because of the binary blob of register values
it contains, but maybe somebody else will. I assume it could be
merged by itself before the rest of this.
I'm looking for acks from Mohit and/or Jingoo for the pci/host
changes, and from Arnd for the devicetree/bindings changes.
Adding these "-dw-3_64" files is sort of ugly. If that code is only
used by keystone, maybe it could just be moved to pci-keystone.c? But
I'll defer to Mohit and Jingoo on that and the way you modify
pcie-designware.c.
Bjorn
On Wednesday, June 18, 2014 9:09 AM, Bjorn Helgaas wrote:
>
> On Tue, Jun 10, 2014 at 02:51:19PM -0400, Murali Karicheri wrote:
> > This patch adds a PCIe controller driver for Keystone SoCs. This
> > is based on v1 of the series posted to the mailing list.
> >
> > CC: Santosh Shilimkar <[email protected]>
> > CC: Russell King <[email protected]>
> > CC: Grant Likely <[email protected]>
> > CC: Rob Herring <[email protected]>
> > CC: Mohit Kumar <[email protected]>
> > CC: Jingoo Han <[email protected]>
> > CC: Bjorn Helgaas <[email protected]>
> > CC: Pratyush Anand <[email protected]>
> > CC: Richard Zhu <[email protected]>
> > CC: Kishon Vijay Abraham I <[email protected]>
> > CC: Marek Vasut <[email protected]>
> > CC: Arnd Bergmann <[email protected]>
> > CC: Pawel Moll <[email protected]>
> > CC: Mark Rutland <[email protected]>
> > CC: Ian Campbell <[email protected]>
> > CC: Kumar Gala <[email protected]>
> > CC: Randy Dunlap <[email protected]>
> > CC: Grant Likely <[email protected]>
> >
> >
> > Changelog:
> >
> > V2
> > - Split the designware pcie enhancement patch to multiple
> > patches based on functionality added
> > - Remove the quirk code and add a patch to fix mps/mrss
> > tuning for ARM. Use kernel command line parameter
> > pci=pcie_bus_perf to work with Keystone PCI Controller.
> > Following patch addressed this.
> > [PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
> > - Add documentation for device tree bindings
> > - Add separate interrupt controller nodes for MSI and Legacy
> > IRQs and use irq map for legacy IRQ
> > - Use compatibility to identify v3.65 version of the DW hardware
> > and use it to customize the designware common code.
> > - Use reg property for configuration space instead of range
> > - Other minor updates based on code inspection.
> >
> > V1
> > - Add an interrupt controller node for Legacy irq chip and use
> > interrupt map/map-mask property to map legacy IRQs A/B/C/D
> > - Add a Phy driver to replace the original serdes driver
> > - Move common application register handling code to a separate
> > file to allow re-use across other platforms that use older
> > DW PCIe h/w
> > - PCI quirk for maximum read request size. Check and override only
> > if the maximum is higher than what controller can handle.
> > - Converted to a module platform driver.
> >
> >
> > Murali Karicheri (8):
> > PCI: designware: add rd[wr]_other_conf API
> > PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
> > PCI: designware: update pcie core driver to work with dw hw version
> > 3.65
> > PCI: designware: add msi controller functions for v3.65 hw
> > PCI: designware: add PCI controller functions for v3.65 DW hw
> > phy: Add serdes phy driver for keystone
> > PCI: keystone: add pcie driver based on designware core driver
> > ARM: keystone: add pcie related options
> >
> > .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> > .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> > .../bindings/phy/phy-keystone-serdes.txt | 25 ++
> > arch/arm/mach-keystone/Kconfig | 1 +
> > drivers/pci/host/Kconfig | 12 +
> > drivers/pci/host/Makefile | 2 +
> > drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
> > drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
> > drivers/pci/host/pci-dw-v3_65.h | 34 ++
> > drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
> > drivers/pci/host/pcie-designware.c | 175 +++++---
> > drivers/pci/host/pcie-designware.h | 42 +-
> > drivers/phy/Kconfig | 6 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
> > 15 files changed, 1531 insertions(+), 52 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
> > create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
> > create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
> > create mode 100644 drivers/pci/host/pci-dw-v3_65.c
> > create mode 100644 drivers/pci/host/pci-dw-v3_65.h
> > create mode 100644 drivers/pci/host/pci-keystone.c
> > create mode 100644 drivers/phy/phy-keystone-serdes.c
>
> I'm not willing to merge phy-keystone-serdes.c because I don't
> maintain drivers/phy and because of the binary blob of register values
> it contains, but maybe somebody else will. I assume it could be
> merged by itself before the rest of this.
I think so, too.
DT maintainers and arch maintainers should review the following
dt bindings.
.../devicetree/bindings/pci/designware-pcie.txt | 42 ++
.../devicetree/bindings/pci/pci-keystone.txt | 56 +++
Generic PHY maintainer (Kishon Vijay Abraham I) should review the
following phy driver.
drivers/phy/phy-keystone-serdes.c
>
> I'm looking for acks from Mohit and/or Jingoo for the pci/host
> changes, and from Arnd for the devicetree/bindings changes.
>
> Adding these "-dw-3_64" files is sort of ugly. If that code is only
> used by keystone, maybe it could just be moved to pci-keystone.c? But
> I'll defer to Mohit and Jingoo on that and the way you modify
> pcie-designware.c.
I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
look terrible! I don't have a good way to handle this; however,
moving this code to pci-keystone.c looks better.
Best regards,
Jingoo Han
>
> Bjorn
On Wed, Jun 11, 2014 at 02:51:20AM +0800, Murali Karicheri wrote:
> v3.65 version of the designware h/w, requires application space
> registers to be configured to access the remote EP config space.
> To support this, add rd[wr]_other_conf API in the pcie_host_opts
>
> Signed-off-by: Murali Karicheri <[email protected]>
>
> CC: Santosh Shilimkar <[email protected]>
> CC: Russell King <[email protected]>
> CC: Grant Likely <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Mohit Kumar <[email protected]>
> CC: Jingoo Han <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Pratyush Anand <[email protected]>
> CC: Richard Zhu <[email protected]>
> CC: Kishon Vijay Abraham I <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Arnd Bergmann <[email protected]>
> CC: Pawel Moll <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: Kumar Gala <[email protected]>
> CC: Randy Dunlap <[email protected]>
> CC: Grant Likely <[email protected]>
>
> ---
> drivers/pci/host/pcie-designware.c | 12 ++++++++++--
> drivers/pci/host/pcie-designware.h | 4 ++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index c4e3732..e8f5d8d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -654,7 +654,11 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>
> spin_lock_irqsave(&pp->conf_lock, flags);
> if (bus->number != pp->root_bus_nr)
> - ret = dw_pcie_rd_other_conf(pp, bus, devfn,
> + if (pp->ops->rd_other_conf)
> + ret = pp->ops->rd_other_conf(pp, bus, devfn,
> + where, size, val);
> + else
> + ret = dw_pcie_rd_other_conf(pp, bus, devfn,
> where, size, val);
> else
> ret = dw_pcie_rd_own_conf(pp, where, size, val);
> @@ -680,7 +684,11 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>
> spin_lock_irqsave(&pp->conf_lock, flags);
> if (bus->number != pp->root_bus_nr)
> - ret = dw_pcie_wr_other_conf(pp, bus, devfn,
> + if (pp->ops->wr_other_conf)
> + ret = pp->ops->wr_other_conf(pp, bus, devfn,
> + where, size, val);
> + else
> + ret = dw_pcie_wr_other_conf(pp, bus, devfn,
> where, size, val);
> else
> ret = dw_pcie_wr_own_conf(pp, where, size, val);
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 3063b35..2d6dd66 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -62,6 +62,10 @@ struct pcie_host_ops {
> u32 val, void __iomem *dbi_base);
> int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
> int (*wr_own_conf)(struct pcie_port *pp, int where, int size, u32 val);
> + int (*rd_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
> + unsigned int devfn, int where, int size, u32 *val);
> + int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
> + unsigned int devfn, int where, int size, u32 val);
> int (*link_up)(struct pcie_port *pp);
> void (*host_init)(struct pcie_port *pp);
> };
Reviewed-by: Pratyush Anand <[email protected]>
> --
> 1.7.9.5
Hi Murali,
On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
> Current DW PCI host init code has code specific to newer hw such as
> ATU port specific resource parsing and map. v3.65 DW PCI host has
OK, Older version did not had standard viewport implementation, so patch 1
of this series will help you to take care for that.
> MSI controller in application space and requires different controller
Since MSI controller is implemented in application space, so this may
not be same for different older version dw controller users.
Therefore, I would suggest to implement all application specific code
in your keystone driver only.
> initialization code. So refactor the msi host controller code into a
> separate function. Other common functions across both hw versions
> are moved to dw_pcie_common_host_init() and called from the newer and
> v3.65 hw host initialization code.
>
> Signed-off-by: Murali Karicheri <[email protected]>
>
> CC: Santosh Shilimkar <[email protected]>
> CC: Russell King <[email protected]>
> CC: Grant Likely <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Mohit Kumar <[email protected]>
> CC: Jingoo Han <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Pratyush Anand <[email protected]>
> CC: Richard Zhu <[email protected]>
> CC: Kishon Vijay Abraham I <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Arnd Bergmann <[email protected]>
> CC: Pawel Moll <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: Kumar Gala <[email protected]>
> CC: Randy Dunlap <[email protected]>
> CC: Grant Likely <[email protected]>
>
> ---
> drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
> 1 file changed, 101 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index e8f5d8d..e4bd19a 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
> .map = dw_pcie_msi_map,
> };
>
> -int __init dw_pcie_host_init(struct pcie_port *pp)
> +/*
> + * dw_pcie_parse_resource() - Function to parse common resources
> + *
> + * @pp: ptr to pcie port
> + *
> + * Parse the range property for MEM, IO and cfg resources, and map
> + * the cfg register space.
> + */
Why do you need this function? If you have some extra resource, you
can ioremap that before you call dw_pcie_host_init.
> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
> {
> struct device_node *np = pp->dev->of_node;
> struct of_pci_range range;
> struct of_pci_range_parser parser;
> - u32 val;
> - int i;
>
> if (of_pci_range_parser_init(&parser, np)) {
> dev_err(pp->dev, "missing ranges property\n");
> @@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> }
> }
> -
> - if (!pp->dbi_base) {
> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> - resource_size(&pp->cfg));
> - if (!pp->dbi_base) {
> - dev_err(pp->dev, "error with ioremap\n");
> - return -ENOMEM;
> - }
> - }
> -
> - 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,
> - pp->config.cfg0_size);
> - if (!pp->va_cfg0_base) {
> - dev_err(pp->dev, "error with ioremap in function\n");
> - return -ENOMEM;
> - }
> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> - pp->config.cfg1_size);
> - if (!pp->va_cfg1_base) {
> - dev_err(pp->dev, "error with ioremap\n");
> - return -ENOMEM;
> - }
> + return 0;
> +}
>
> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
> - return -EINVAL;
> - }
> +/*
> + * dw_pcie_msi_host_init() - Function to initialize msi host controller
> + *
> + * @pp: ptr to pcie port
> + * @np: device node ptr to msi irq controller
> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
> + *
> + * Function register irq domain for msi irq controller and create mappings
> + * for MSI irqs.
> + */
May be you can only do following to support your MSI chip:
Initialize pp->irq_domain into your add_pcie_port function before you
call dw_pcie_host_init.
In dw_pcie_host_init,
if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
> + const struct irq_domain_ops *irq_msi_ops)
> +{
> + int i;
>
> if (IS_ENABLED(CONFIG_PCI_MSI)) {
> pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
> - MAX_MSI_IRQS, &msi_domain_ops,
> + MAX_MSI_IRQS, irq_msi_ops,
> &dw_pcie_msi_chip);
> if (!pp->irq_domain) {
> dev_err(pp->dev, "irq domain init failed\n");
> @@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> irq_create_mapping(pp->irq_domain, i);
> }
>
> + return 0;
> +}
> +
> +/*
> + * dw_pcie_common_host_init() - common host init function across different
> + * versions of the designware PCI controller.
> + * @pp: ptr to pcie port
> + * @hw: ptr to hw_pci structure
> + *
> + * This functions parses common DT properties, call host_init() callback
> + * of the PCI controller driver. Also initialize the common RC configurations
> + * and call common pci core function to intialize the controller driver.
> + */
> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
> +{
> + struct device_node *np = pp->dev->of_node;
> + u32 val;
> +
> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
> + return -EINVAL;
> + }
> +
> if (pp->ops->host_init)
> pp->ops->host_init(pp);
>
> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> val |= PORT_LOGIC_SPEED_CHANGE;
> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>
> - dw_pci.nr_controllers = 1;
> - dw_pci.private_data = (void **)&pp;
> + hw->nr_controllers = 1;
> + hw->private_data = (void **)&pp;
>
> - pci_common_init_dev(pp->dev, &dw_pci);
> + pci_common_init_dev(pp->dev, hw);
> pci_assign_unassigned_resources();
> #ifdef CONFIG_PCI_DOMAINS
> dw_pci.domain++;
> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> return 0;
> }
>
> +/*
> + * dw_pcie_host_init() - Host init function for new designware h/w
> + *
> + * @pp: ptr to pcie port
> + *
> + * The function parse the PCI resurces for IO, Memory and map the config
> + * space addresses. Also initliaze the MSI irq controller and call
> + * dw_pcie_common_host_init() to initialize the PCI controller.
> + */
> +int __init dw_pcie_host_init(struct pcie_port *pp)
> +{
> + int ret;
> +
> + ret = dw_pcie_parse_resource(pp);
> + if (ret)
> + return ret;
> +
> + if (!pp->dbi_base) {
> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> + resource_size(&pp->cfg));
> + if (!pp->dbi_base) {
> + dev_err(pp->dev, "error with ioremap\n");
> + return -ENOMEM;
> + }
> + }
> +
> + pp->cfg0_base = pp->cfg.start;
> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> +
> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
> + pp->config.cfg0_size);
> + if (!pp->va_cfg0_base) {
> + dev_err(pp->dev, "error with ioremap in function\n");
> + return -ENOMEM;
> + }
> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> + pp->config.cfg1_size);
> + if (!pp->va_cfg1_base) {
> + dev_err(pp->dev, "error with ioremap\n");
> + return -ENOMEM;
> + }
> +
> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
> + if (ret < 0)
> + return ret;
> +
> + return dw_pcie_common_host_init(pp, &dw_pci);
> +}
> +
> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
> {
> /* Program viewport 0 : OUTBOUND : CFG0 */
Regards
Pratyush
> --
> 1.7.9.5
Hello Murali,
> -----Original Message-----
> From: Murali Karicheri [mailto:[email protected]]
> Sent: Wednesday, June 11, 2014 12:21 AM
> To: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
> Subject: [PATCH v2 3/8] PCI: designware: update pcie core driver to work
> with dw hw version 3.65
>
> v3.65 version of the dw hw has MSI controller implemented in the application
> space. Add a version variable in the port struct to identify v3.65 hardware for
> different code treatment. This variable will have DW_V3_65 bit set when
> running on this hw version. The host init code is expected to set this version
> based on compatibility string dw,snps-pcie-v3.65.
>
> Some of the MSI specific functions from current DW driver are re-used on
> v3.65 hw. However on v3.65, MSI controller registers are in the application
> register space and PCIE_MSI_INTR0_ENABLE is not applicable.
> Modify assign_irq() to check for version and not execute the code for
> PCIE_MSI_INTR0_ENABLE configuration on v3.65 hw. Additionally MSI IRQ
> register in application space is written by EP to raise an MSI IRQ. So add a
> get_msi_data() function in pcie_host_ops to retrieve the register address in
> dw_msi_setup_irq(). v3.65 dw core driver implements this function.
>
> Also make some of the functions available in dw core driver global and make
> their prototypes available in the header file for re-use on v3.65.
>
- Pls apply MSI specific changes on the top of Lucas patches:
[PATCH 0/4] proper multi MSI handling for designware host
Thanks
Mohit
> Signed-off-by: Murali Karicheri <[email protected]>
>
> CC: Santosh Shilimkar <[email protected]>
> CC: Russell King <[email protected]>
> CC: Grant Likely <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Mohit Kumar <[email protected]>
> CC: Jingoo Han <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Pratyush Anand <[email protected]>
> CC: Richard Zhu <[email protected]>
> CC: Kishon Vijay Abraham I <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Arnd Bergmann <[email protected]>
> CC: Pawel Moll <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: Kumar Gala <[email protected]>
> CC: Randy Dunlap <[email protected]>
> CC: Grant Likely <[email protected]>
>
> ---
> drivers/pci/host/pcie-designware.c | 27 +++++++++++++++++----------
> drivers/pci/host/pcie-designware.h | 16 ++++++++++++++++
> 2 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> index e4bd19a..f985811 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -277,11 +277,15 @@ static int assign_irq(int no_irqs, struct msi_desc
> *desc, int *pos)
> }
> set_bit(pos0 + i, pp->msi_irq_in_use);
> /*Enable corresponding interrupt in MSI interrupt controller
> */
> - res = ((pos0 + i) / 32) * 12;
> - bit = (pos0 + i) % 32;
> - dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
> 4, &val);
> - val |= 1 << bit;
> - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
> 4, val);
> + if (!(pp->version & DW_V3_65)) {
> + res = ((pos0 + i) / 32) * 12;
> + bit = (pos0 + i) % 32;
> + dw_pcie_rd_own_conf(pp,
> PCIE_MSI_INTR0_ENABLE + res,
> + 4, &val);
> + val |= 1 << bit;
> + dw_pcie_wr_own_conf(pp,
> PCIE_MSI_INTR0_ENABLE + res,
> + 4, val);
> + }
> }
>
> *pos = pos0;
> @@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
> struct pci_dev *pdev,
> */
> desc->msi_attrib.multiple = msgvec;
>
> - msg.address_lo = virt_to_phys((void *)pp->msi_data);
> + if (pp->ops->get_msi_data)
> + msg.address_lo = pp->ops->get_msi_data(pp);
> + else
> + msg.address_lo = virt_to_phys((void *)pp->msi_data);
> msg.address_hi = 0x0;
> msg.data = pos;
> write_msi_msg(irq, &msg);
> @@ -768,7 +775,7 @@ static struct pci_ops dw_pcie_ops = {
> .write = dw_pcie_wr_conf,
> };
>
> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> +int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> {
> struct pcie_port *pp;
>
> @@ -791,7 +798,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data
> *sys)
> return 1;
> }
>
> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> +struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> {
> struct pci_bus *bus;
> struct pcie_port *pp = sys_to_pcie(sys); @@ -808,7 +815,7 @@ static
> struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> return bus;
> }
>
> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> +int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> {
> struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
> int irq;
> @@ -820,7 +827,7 @@ static int dw_pcie_map_irq(const struct pci_dev
> *dev, u8 slot, u8 pin)
> return irq;
> }
>
> -static void dw_pcie_add_bus(struct pci_bus *bus)
> +void dw_pcie_add_bus(struct pci_bus *bus)
> {
> if (IS_ENABLED(CONFIG_PCI_MSI)) {
> struct pcie_port *pp = sys_to_pcie(bus->sysdata); diff --git
> a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 2d6dd66..3a6a6eb 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -39,6 +39,12 @@ struct pcie_port {
> void __iomem *va_cfg0_base;
> u64 cfg1_base;
> void __iomem *va_cfg1_base;
> + /*
> + * v3.65 DW hw implements application register space for
> + * MSI and has no ATU view port
> + */
> +#define DW_V3_65 BIT(0)
> + u32 version;
> u64 io_base;
> u64 mem_base;
> spinlock_t conf_lock;
> @@ -68,6 +74,7 @@ struct pcie_host_ops {
> unsigned int devfn, int where, int size, u32 val);
> int (*link_up)(struct pcie_port *pp);
> void (*host_init)(struct pcie_port *pp);
> + u32 (*get_msi_data)(struct pcie_port *pp);
> };
>
> int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
> @@ -77,5 +84,14 @@ void dw_pcie_msi_init(struct pcie_port *pp); int
> dw_pcie_link_up(struct pcie_port *pp); void dw_pcie_setup_rc(struct
> pcie_port *pp); int dw_pcie_host_init(struct pcie_port *pp);
> +int dw_pcie_setup(int nr, struct pci_sys_data *sys); struct pci_bus
> +*dw_pcie_scan_bus(int nr, struct pci_sys_data *sys); void
> +dw_pcie_add_bus(struct pci_bus *bus); int
> dw_pcie_parse_resource(struct
> +pcie_port *pp);
>
> +/* internal to DW common core driver */ int
> +dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw); int
> +dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
> + const struct irq_domain_ops *irq_ops); int
> dw_pcie_map_irq(const
> +struct pci_dev *dev, u8 slot, u8 pin);
> #endif /* _PCIE_DESIGNWARE_H */
> --
> 1.7.9.5
Hello Murali,
> -----Original Message-----
> From: Murali Karicheri [mailto:[email protected]]
> Sent: Wednesday, June 11, 2014 12:21 AM
> To: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
> Subject: [PATCH v2 4/8] PCI: designware: add msi controller functions for
> v3.65 hw
>
> Add dw msi controller functions for v3.65 hw. This adds dw_v3_65_msi_chip
> and dw_v3_65_msi_domain_ops so that can be used on this version of the
> hw.
> This required since MSI irq registers reside in the application space for v3.65
> hw. The functions are used by v3.65 dw pci core functions to support
> implementation of PCI controllers based on this hw version.
>
> While at it, move the ATU hw specific variable and msi irq to a separate struct
> inside a union and add another struct inside the union to hold dw v3.65
> specific variables.
>
- Here also, pls look into Lucas work for MSI cleanup and then apply your changes:
[PATCH 0/4] proper multi MSI handling for designware host
Thanks
Mohit
> Signed-off-by: Murali Karicheri <[email protected]>
>
> CC: Santosh Shilimkar <[email protected]>
> CC: Russell King <[email protected]>
> CC: Grant Likely <[email protected]>
> CC: Rob Herring <[email protected]>
> CC: Mohit Kumar <[email protected]>
> CC: Jingoo Han <[email protected]>
> CC: Bjorn Helgaas <[email protected]>
> CC: Pratyush Anand <[email protected]>
> CC: Richard Zhu <[email protected]>
> CC: Kishon Vijay Abraham I <[email protected]>
> CC: Marek Vasut <[email protected]>
> CC: Arnd Bergmann <[email protected]>
> CC: Pawel Moll <[email protected]>
> CC: Mark Rutland <[email protected]>
> CC: Ian Campbell <[email protected]>
> CC: Kumar Gala <[email protected]>
> CC: Randy Dunlap <[email protected]>
> CC: Grant Likely <[email protected]>
>
> ---
> drivers/pci/host/Kconfig | 5 ++
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pci-dw-v3_65-msi.c | 149
> +++++++++++++++++++++++++++++++++++
> drivers/pci/host/pci-dw-v3_65.h | 20 +++++
> drivers/pci/host/pcie-designware.h | 21 +++--
> 5 files changed, 191 insertions(+), 5 deletions(-) create mode 100644
> drivers/pci/host/pci-dw-v3_65-msi.c
> create mode 100644 drivers/pci/host/pci-dw-v3_65.h
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index
> a6f67ec..2fcd9f9 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -9,6 +9,11 @@ config PCI_MVEBU
> config PCIE_DW
> bool
>
> +config PCI_DW_V3_65
> + bool "Designware PCIe h/w v3.65"
> + help
> + Say Y here if the DW h/w version is 3.65
> +
> config PCI_EXYNOS
> bool "Samsung Exynos PCIe controller"
> depends on SOC_EXYNOS5440
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile index
> 13fb333..28af710 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
> obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
> obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
> +obj-$(CONFIG_PCI_DW_V3_65) += pci-dw-v3_65-msi.o
> diff --git a/drivers/pci/host/pci-dw-v3_65-msi.c b/drivers/pci/host/pci-dw-
> v3_65-msi.c
> new file mode 100644
> index 0000000..a26ffdd
> --- /dev/null
> +++ b/drivers/pci/host/pci-dw-v3_65-msi.c
> @@ -0,0 +1,149 @@
> +/*
> + * Designware(dw) MSI controller version 3.65
> + *
> + * Copyright (C) 2013-2014 Texas Instruments., Ltd.
> + * http://www.ti.com
> + *
> + * Author: Murali Karicheri <[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/of_irq.h>
> +#include <linux/kernel.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
> +#include "pcie-designware.h"
> +
> +#define MSI_IRQ 0x054
> +#define MSI0_IRQ_STATUS 0x104
> +#define MSI0_IRQ_ENABLE_SET 0x108
> +#define MSI0_IRQ_ENABLE_CLR 0x10c
> +#define IRQ_STATUS 0x184
> +#define IRQ_EOI 0x050
> +#define MSI_IRQ_OFFSET 4
> +
> +static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys) {
> + return sys->private_data;
> +}
> +
> +static inline void update_reg_offset_bit_pos(u32 offset, u32 *reg_offset,
> + u32 *bit_pos)
> +{
> + *reg_offset = offset % 8;
> + *bit_pos = offset >> 3;
> +}
> +
> +inline u32 dw_v3_65_get_msi_data(struct pcie_port *pp) {
> + return pp->app.start + MSI_IRQ;
> +}
> +
> +void dw_v3_65_handle_msi_irq(struct pcie_port *pp, int offset) {
> + u32 pending, vector;
> + int src, virq;
> +
> + pending = readl(pp->va_app_base + MSI0_IRQ_STATUS + (offset <<
> 4));
> + /*
> + * MSI0, Status bit 0-3 shows vectors 0, 8, 16, 24, MSI1 status bit
> + * shows 1, 9, 17, 25 and so forth
> + */
> + for (src = 0; src < 4; src++) {
> + if (BIT(src) & pending) {
> + vector = offset + (src << 3);
> + virq = irq_linear_revmap(pp->irq_domain, vector);
> + dev_dbg(pp->dev,
> + "irq: bit %d, vector %d, virq %d\n",
> + src, vector, virq);
> + generic_handle_irq(virq);
> + }
> + }
> +}
> +
> +static void dw_v3_65_msi_irq_ack(struct irq_data *d) {
> + u32 offset, reg_offset, bit_pos;
> + unsigned int irq = d->irq;
> + struct msi_desc *msi;
> + struct pcie_port *pp;
> +
> + msi = irq_get_msi_desc(irq);
> + pp = sys_to_pcie(msi->dev->bus->sysdata);
> + offset = irq - irq_linear_revmap(pp->irq_domain, 0);
> + update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
> +
> + writel(BIT(bit_pos),
> + pp->va_app_base + MSI0_IRQ_STATUS + (reg_offset << 4));
> + writel(reg_offset + MSI_IRQ_OFFSET, pp->va_app_base + IRQ_EOI);
> }
> +
> +static void dw_v3_65_msi_irq_mask(struct irq_data *d) {
> + u32 offset, reg_offset, bit_pos;
> + unsigned int irq = d->irq;
> + struct msi_desc *msi;
> + struct pcie_port *pp;
> +
> + msi = irq_get_msi_desc(irq);
> + pp = sys_to_pcie(msi->dev->bus->sysdata);
> + offset = irq - irq_linear_revmap(pp->irq_domain, 0);
> + update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
> +
> + /* mask the end point if PVM implemented */
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + if (msi->msi_attrib.maskbit)
> + mask_msi_irq(d);
> + }
> +
> + writel(BIT(bit_pos),
> + pp->va_app_base + MSI0_IRQ_ENABLE_CLR + (reg_offset <<
> 4)); }
> +
> +static void dw_v3_65_msi_irq_unmask(struct irq_data *d) {
> + u32 offset, reg_offset, bit_pos;
> + unsigned int irq = d->irq;
> + struct msi_desc *msi;
> + struct pcie_port *pp;
> +
> + msi = irq_get_msi_desc(irq);
> + pp = sys_to_pcie(msi->dev->bus->sysdata);
> + offset = irq - irq_linear_revmap(pp->irq_domain, 0);
> + update_reg_offset_bit_pos(offset, ®_offset, &bit_pos);
> +
> + /* mask the end point if PVM implemented */
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + if (msi->msi_attrib.maskbit)
> + unmask_msi_irq(d);
> + }
> +
> + writel(BIT(bit_pos),
> + pp->va_app_base + MSI0_IRQ_ENABLE_SET + (reg_offset <<
> 4)); }
> +
> +static struct irq_chip dw_v3_65_msi_chip = {
> + .name = "PCI-DW-MSI-OLD",
> + .irq_ack = dw_v3_65_msi_irq_ack,
> + .irq_mask = dw_v3_65_msi_irq_mask,
> + .irq_unmask = dw_v3_65_msi_irq_unmask, };
> +
> +static int dw_v3_65_msi_map(struct irq_domain *domain, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_and_handler(irq, &dw_v3_65_msi_chip,
> handle_level_irq);
> + irq_set_chip_data(irq, domain->host_data);
> + set_irq_flags(irq, IRQF_VALID);
> +
> + return 0;
> +}
> +
> +const struct irq_domain_ops dw_v3_65_msi_domain_ops = {
> + .map = dw_v3_65_msi_map,
> +};
> diff --git a/drivers/pci/host/pci-dw-v3_65.h b/drivers/pci/host/pci-dw-
> v3_65.h new file mode 100644 index 0000000..689256a
> --- /dev/null
> +++ b/drivers/pci/host/pci-dw-v3_65.h
> @@ -0,0 +1,20 @@
> +/*
> + * Designware(dw) v3.65 controller common includes
> + *
> + * Copyright (C) 2013-2014 Texas Instruments., Ltd.
> + * http://www.ti.com
> + *
> + * Author: Murali Karicheri <[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.
> + */
> +
> +#define MAX_LEGACY_IRQS 4
> +
> +/* v3.65 specific MSI controller APIs/definitions */ extern const
> +struct irq_domain_ops dw_v3_65_msi_domain_ops; void
> +dw_v3_65_handle_msi_irq(struct pcie_port *pp, int offset);
> +u32 dw_v3_65_get_msi_data(struct pcie_port *pp);
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-
> designware.h
> index 3a6a6eb..05bb590 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -35,16 +35,27 @@ struct pcie_port {
> struct device *dev;
> u8 root_bus_nr;
> void __iomem *dbi_base;
> - u64 cfg0_base;
> - void __iomem *va_cfg0_base;
> - u64 cfg1_base;
> - void __iomem *va_cfg1_base;
> /*
> * v3.65 DW hw implements application register space for
> * MSI and has no ATU view port
> */
> #define DW_V3_65 BIT(0)
> u32 version;
> + union {
> + /* New DW hw specific */
> + struct {
> + u64 cfg0_base;
> + void __iomem *va_cfg0_base;
> + u64 cfg1_base;
> + void __iomem *va_cfg1_base;
> + int msi_irq;
> + };
> + /* v3.65 DW hw specific */
> + struct {
> + void __iomem *va_app_base;
> + struct resource app;
> + };
> + };
> u64 io_base;
> u64 mem_base;
> spinlock_t conf_lock;
> @@ -55,7 +66,7 @@ struct pcie_port {
> int irq;
> u32 lanes;
> struct pcie_host_ops *ops;
> - int msi_irq;
> + /* msi irq domain */
> struct irq_domain *irq_domain;
> unsigned long msi_data;
> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> --
> 1.7.9.5
Hello Murali,
> -----Original Message-----
> From: Murali Karicheri [mailto:[email protected]]
> Sent: Wednesday, June 11, 2014 12:21 AM
> To: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
> Subject: [PATCH v2 0/8] Add Keystone PCIe controller driver
>
> This patch adds a PCIe controller driver for Keystone SoCs. This is based on v1
> of the series posted to the mailing list.
>
1. I think your first patch is OK which handles platform specific ATU implementation.
2. For MSI part, I think you just need to add two new callbacks with pp-ops, something similar to:
pp->ops->msi_set
pp->ops->msi_clear
With these two platform specific callbacks you should be able to manage MSI handling.
So idea is that dw_msi code uses pp->ops->msi_set/clear if platform define these,
otherwise use dw_msi_set/clear (which you need to refactor from existing code)
So other than your keystone changes we expect 3 patches:
-- 1st same as you sent 1/8: for ATU handeling
-- 2nd to refactor dw_msi_set/clear: refactor from existing code
-- 3rd to use pp->ops->msi_set/clear if defined.
Pls let us know for any issue or have different opinion.
Regards
Mohit
>
> --
> 1.7.9.5
On 6/17/2014 8:08 PM, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2014 at 02:51:19PM -0400, Murali Karicheri wrote:
>> This patch adds a PCIe controller driver for Keystone SoCs. This
>> is based on v1 of the series posted to the mailing list.
>>
>> CC: Santosh Shilimkar <[email protected]>
>> CC: Russell King <[email protected]>
>> CC: Grant Likely <[email protected]>
>> CC: Rob Herring <[email protected]>
>> CC: Mohit Kumar <[email protected]>
>> CC: Jingoo Han <[email protected]>
>> CC: Bjorn Helgaas <[email protected]>
>> CC: Pratyush Anand <[email protected]>
>> CC: Richard Zhu <[email protected]>
>> CC: Kishon Vijay Abraham I <[email protected]>
>> CC: Marek Vasut <[email protected]>
>> CC: Arnd Bergmann <[email protected]>
>> CC: Pawel Moll <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: Kumar Gala <[email protected]>
>> CC: Randy Dunlap <[email protected]>
>> CC: Grant Likely <[email protected]>
>>
>>
>> Changelog:
>>
>> V2
>> - Split the designware pcie enhancement patch to multiple
>> patches based on functionality added
>> - Remove the quirk code and add a patch to fix mps/mrss
>> tuning for ARM. Use kernel command line parameter
>> pci=pcie_bus_perf to work with Keystone PCI Controller.
>> Following patch addressed this.
>> [PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
>> - Add documentation for device tree bindings
>> - Add separate interrupt controller nodes for MSI and Legacy
>> IRQs and use irq map for legacy IRQ
>> - Use compatibility to identify v3.65 version of the DW hardware
>> and use it to customize the designware common code.
>> - Use reg property for configuration space instead of range
>> - Other minor updates based on code inspection.
>>
>> V1
>> - Add an interrupt controller node for Legacy irq chip and use
>> interrupt map/map-mask property to map legacy IRQs A/B/C/D
>> - Add a Phy driver to replace the original serdes driver
>> - Move common application register handling code to a separate
>> file to allow re-use across other platforms that use older
>> DW PCIe h/w
>> - PCI quirk for maximum read request size. Check and override only
>> if the maximum is higher than what controller can handle.
>> - Converted to a module platform driver.
>>
>>
>> Murali Karicheri (8):
>> PCI: designware: add rd[wr]_other_conf API
>> PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
>> PCI: designware: update pcie core driver to work with dw hw version
>> 3.65
>> PCI: designware: add msi controller functions for v3.65 hw
>> PCI: designware: add PCI controller functions for v3.65 DW hw
>> phy: Add serdes phy driver for keystone
>> PCI: keystone: add pcie driver based on designware core driver
>> ARM: keystone: add pcie related options
>>
>> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
>> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
>> .../bindings/phy/phy-keystone-serdes.txt | 25 ++
>> arch/arm/mach-keystone/Kconfig | 1 +
>> drivers/pci/host/Kconfig | 12 +
>> drivers/pci/host/Makefile | 2 +
>> drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
>> drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
>> drivers/pci/host/pci-dw-v3_65.h | 34 ++
>> drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
>> drivers/pci/host/pcie-designware.c | 175 +++++---
>> drivers/pci/host/pcie-designware.h | 42 +-
>> drivers/phy/Kconfig | 6 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
>> 15 files changed, 1531 insertions(+), 52 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
>> create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
>> create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
>> create mode 100644 drivers/pci/host/pci-dw-v3_65.c
>> create mode 100644 drivers/pci/host/pci-dw-v3_65.h
>> create mode 100644 drivers/pci/host/pci-keystone.c
>> create mode 100644 drivers/phy/phy-keystone-serdes.c
> I'm not willing to merge phy-keystone-serdes.c because I don't
> maintain drivers/phy and because of the binary blob of register values
> it contains, but maybe somebody else will. I assume it could be
> merged by itself before the rest of this.
>
> I'm looking for acks from Mohit and/or Jingoo for the pci/host
> changes, and from Arnd for the devicetree/bindings changes.
>
> Adding these "-dw-3_64" files is sort of ugly. If that code is only
> used by keystone, maybe it could just be moved to pci-keystone.c? But
> I'll defer to Mohit and Jingoo on that and the way you modify
> pcie-designware.c.
>
> Bjorn
Bjorn, Jingoo,
Unfortunately I lost Jingoo's email from my Inbox. So I cut-n-paste the
comment from
internet and respond.
Jingoo Han wrote:-
=============================================================================
I think so, too.
DT maintainers and arch maintainers should review the following
dt bindings.
.../devicetree/bindings/pci/designware-pcie.txt | 42 ++
.../devicetree/bindings/pci/pci-keystone.txt | 56 +++
Generic PHY maintainer (Kishon Vijay Abraham I) should review the
following phy driver.
drivers/phy/phy-keystone-serdes.c
>
> I'm looking for acks from Mohit and/or Jingoo for the pci/host
> changes, and from Arnd for the devicetree/bindings changes.
>
> Adding these "-dw-3_64" files is sort of ugly. If that code is only
> used by keystone, maybe it could just be moved to pci-keystone.c? But
> I'll defer to Mohit and Jingoo on that and the way you modify
> pcie-designware.c.
I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
look terrible! I don't have a good way to handle this; however,
moving this code to pci-keystone.c looks better.
======================================================================
The original RFC I had submitted had all of the application space register
handling code as part of the Keystone PCI driver. As per Arnd's comment
(See
my change log against v1), the code was moved to a separate file so that
the next driver that has same version of the DW hw could re-use this code.
I agree with Arnd and moved the code to v_3_65 specific files. What is
your proposal? Do you have objection to the file name? or it's content?
If objection is on the file name, please suggest alternate names. If you
are okay with the file name, and doesn't like the code, it will be helpful
to review the code and provide specific comments against the patch itself
so that I can address the same.
Murali
On Friday 20 June 2014 11:31 AM, Murali Karicheri wrote:
> On 6/17/2014 8:08 PM, Bjorn Helgaas wrote:
>> On Tue, Jun 10, 2014 at 02:51:19PM -0400, Murali Karicheri wrote:
>>> This patch adds a PCIe controller driver for Keystone SoCs. This
>>> is based on v1 of the series posted to the mailing list.
>>>
>>> CC: Santosh Shilimkar <[email protected]>
>>> CC: Russell King <[email protected]>
>>> CC: Grant Likely <[email protected]>
>>> CC: Rob Herring <[email protected]>
>>> CC: Mohit Kumar <[email protected]>
>>> CC: Jingoo Han <[email protected]>
>>> CC: Bjorn Helgaas <[email protected]>
>>> CC: Pratyush Anand <[email protected]>
>>> CC: Richard Zhu <[email protected]>
>>> CC: Kishon Vijay Abraham I <[email protected]>
>>> CC: Marek Vasut <[email protected]>
>>> CC: Arnd Bergmann <[email protected]>
>>> CC: Pawel Moll <[email protected]>
>>> CC: Mark Rutland <[email protected]>
>>> CC: Ian Campbell <[email protected]>
>>> CC: Kumar Gala <[email protected]>
>>> CC: Randy Dunlap <[email protected]>
>>> CC: Grant Likely <[email protected]>
>>>
>>>
>>> Changelog:
>>>
>>> V2
>>> - Split the designware pcie enhancement patch to multiple
>>> patches based on functionality added
>>> - Remove the quirk code and add a patch to fix mps/mrss
>>> tuning for ARM. Use kernel command line parameter
>>> pci=pcie_bus_perf to work with Keystone PCI Controller.
>>> Following patch addressed this.
>>> [PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
>>> - Add documentation for device tree bindings
>>> - Add separate interrupt controller nodes for MSI and Legacy
>>> IRQs and use irq map for legacy IRQ
>>> - Use compatibility to identify v3.65 version of the DW hardware
>>> and use it to customize the designware common code.
>>> - Use reg property for configuration space instead of range
>>> - Other minor updates based on code inspection.
>>>
>>> V1
>>> - Add an interrupt controller node for Legacy irq chip and use
>>> interrupt map/map-mask property to map legacy IRQs A/B/C/D
>>> - Add a Phy driver to replace the original serdes driver
>>> - Move common application register handling code to a separate
>>> file to allow re-use across other platforms that use older
>>> DW PCIe h/w
>>> - PCI quirk for maximum read request size. Check and override only
>>> if the maximum is higher than what controller can handle.
>>> - Converted to a module platform driver.
>>>
>>>
>>> Murali Karicheri (8):
>>> PCI: designware: add rd[wr]_other_conf API
>>> PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
>>> PCI: designware: update pcie core driver to work with dw hw version
>>> 3.65
>>> PCI: designware: add msi controller functions for v3.65 hw
>>> PCI: designware: add PCI controller functions for v3.65 DW hw
>>> phy: Add serdes phy driver for keystone
>>> PCI: keystone: add pcie driver based on designware core driver
>>> ARM: keystone: add pcie related options
>>>
>>> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
>>> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
>>> .../bindings/phy/phy-keystone-serdes.txt | 25 ++
>>> arch/arm/mach-keystone/Kconfig | 1 +
>>> drivers/pci/host/Kconfig | 12 +
>>> drivers/pci/host/Makefile | 2 +
>>> drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
>>> drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
>>> drivers/pci/host/pci-dw-v3_65.h | 34 ++
>>> drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
>>> drivers/pci/host/pcie-designware.c | 175 +++++---
>>> drivers/pci/host/pcie-designware.h | 42 +-
>>> drivers/phy/Kconfig | 6 +
>>> drivers/phy/Makefile | 1 +
>>> drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
>>> 15 files changed, 1531 insertions(+), 52 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
>>> create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
>>> create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
>>> create mode 100644 drivers/pci/host/pci-dw-v3_65.c
>>> create mode 100644 drivers/pci/host/pci-dw-v3_65.h
>>> create mode 100644 drivers/pci/host/pci-keystone.c
>>> create mode 100644 drivers/phy/phy-keystone-serdes.c
>> I'm not willing to merge phy-keystone-serdes.c because I don't
>> maintain drivers/phy and because of the binary blob of register values
>> it contains, but maybe somebody else will. I assume it could be
>> merged by itself before the rest of this.
>>
>> I'm looking for acks from Mohit and/or Jingoo for the pci/host
>> changes, and from Arnd for the devicetree/bindings changes.
>>
>> Adding these "-dw-3_64" files is sort of ugly. If that code is only
>> used by keystone, maybe it could just be moved to pci-keystone.c? But
>> I'll defer to Mohit and Jingoo on that and the way you modify
>> pcie-designware.c.
>>
>> Bjorn
> Bjorn, Jingoo,
>
> Unfortunately I lost Jingoo's email from my Inbox. So I cut-n-paste the comment from
> internet and respond.
>
> Jingoo Han wrote:-
> =============================================================================
> I think so, too.
>
> DT maintainers and arch maintainers should review the following
> dt bindings.
> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> Generic PHY maintainer (Kishon Vijay Abraham I) should review the
> following phy driver.
> drivers/phy/phy-keystone-serdes.c
>
>>
>> I'm looking for acks from Mohit and/or Jingoo for the pci/host
>> changes, and from Arnd for the devicetree/bindings changes.
>>
>> Adding these "-dw-3_64" files is sort of ugly. If that code is only
>> used by keystone, maybe it could just be moved to pci-keystone.c? But
>> I'll defer to Mohit and Jingoo on that and the way you modify
>> pcie-designware.c.
>
> I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
> look terrible! I don't have a good way to handle this; however,
> moving this code to pci-keystone.c looks better.
> ======================================================================
>
> The original RFC I had submitted had all of the application space register
> handling code as part of the Keystone PCI driver. As per Arnd's comment (See
> my change log against v1), the code was moved to a separate file so that
> the next driver that has same version of the DW hw could re-use this code.
> I agree with Arnd and moved the code to v_3_65 specific files. What is
> your proposal? Do you have objection to the file name? or it's content?
>
> If objection is on the file name, please suggest alternate names. If you
> are okay with the file name, and doesn't like the code, it will be helpful
> to review the code and provide specific comments against the patch itself
> so that I can address the same.
>
Arnd suggestion was to have the version 3.65 code in generic place since
its IP specific and just in case some other vendor using the same version
can leverage the code.
Concern here seems toe really those name of the files. I can't think of
any other appropriate name.
Regards,
Santosh
On 6/18/2014 3:13 AM, Mohit KUMAR DCG wrote:
> Hello Murali,
>
>> -----Original Message-----
>> From: Murali Karicheri [mailto:[email protected]]
>> Sent: Wednesday, June 11, 2014 12:21 AM
>> To: [email protected]; [email protected];
>> [email protected]; [email protected]; linux-
>> [email protected]
>> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
>> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
>> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
>> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
>> Subject: [PATCH v2 3/8] PCI: designware: update pcie core driver to work
>> with dw hw version 3.65
>>
>> v3.65 version of the dw hw has MSI controller implemented in the application
>> space. Add a version variable in the port struct to identify v3.65 hardware for
>> different code treatment. This variable will have DW_V3_65 bit set when
>> running on this hw version. The host init code is expected to set this version
>> based on compatibility string dw,snps-pcie-v3.65.
>>
>> Some of the MSI specific functions from current DW driver are re-used on
>> v3.65 hw. However on v3.65, MSI controller registers are in the application
>> register space and PCIE_MSI_INTR0_ENABLE is not applicable.
>> Modify assign_irq() to check for version and not execute the code for
>> PCIE_MSI_INTR0_ENABLE configuration on v3.65 hw. Additionally MSI IRQ
>> register in application space is written by EP to raise an MSI IRQ. So add a
>> get_msi_data() function in pcie_host_ops to retrieve the register address in
>> dw_msi_setup_irq(). v3.65 dw core driver implements this function.
>>
>> Also make some of the functions available in dw core driver global and make
>> their prototypes available in the header file for re-use on v3.65.
>>
> - Pls apply MSI specific changes on the top of Lucas patches:
> [PATCH 0/4] proper multi MSI handling for designware host
>
> Thanks
> Mohit
Ok. I have pulled in Lucas's patches and use it for my work.
Murali
>> Signed-off-by: Murali Karicheri <[email protected]>
>>
>> CC: Santosh Shilimkar <[email protected]>
>> CC: Russell King <[email protected]>
>> CC: Grant Likely <[email protected]>
>> CC: Rob Herring <[email protected]>
>> CC: Mohit Kumar <[email protected]>
>> CC: Jingoo Han <[email protected]>
>> CC: Bjorn Helgaas <[email protected]>
>> CC: Pratyush Anand <[email protected]>
>> CC: Richard Zhu <[email protected]>
>> CC: Kishon Vijay Abraham I <[email protected]>
>> CC: Marek Vasut <[email protected]>
>> CC: Arnd Bergmann <[email protected]>
>> CC: Pawel Moll <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: Kumar Gala <[email protected]>
>> CC: Randy Dunlap <[email protected]>
>> CC: Grant Likely <[email protected]>
>>
>> ---
>> drivers/pci/host/pcie-designware.c | 27 +++++++++++++++++----------
>> drivers/pci/host/pcie-designware.h | 16 ++++++++++++++++
>> 2 files changed, 33 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
>> designware.c
>> index e4bd19a..f985811 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -277,11 +277,15 @@ static int assign_irq(int no_irqs, struct msi_desc
>> *desc, int *pos)
>> }
>> set_bit(pos0 + i, pp->msi_irq_in_use);
>> /*Enable corresponding interrupt in MSI interrupt controller
>> */
>> - res = ((pos0 + i) / 32) * 12;
>> - bit = (pos0 + i) % 32;
>> - dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
>> 4, &val);
>> - val |= 1 << bit;
>> - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
>> 4, val);
>> + if (!(pp->version & DW_V3_65)) {
>> + res = ((pos0 + i) / 32) * 12;
>> + bit = (pos0 + i) % 32;
>> + dw_pcie_rd_own_conf(pp,
>> PCIE_MSI_INTR0_ENABLE + res,
>> + 4, &val);
>> + val |= 1 << bit;
>> + dw_pcie_wr_own_conf(pp,
>> PCIE_MSI_INTR0_ENABLE + res,
>> + 4, val);
>> + }
>> }
>>
>> *pos = pos0;
>> @@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
>> struct pci_dev *pdev,
>> */
>> desc->msi_attrib.multiple = msgvec;
>>
>> - msg.address_lo = virt_to_phys((void *)pp->msi_data);
>> + if (pp->ops->get_msi_data)
>> + msg.address_lo = pp->ops->get_msi_data(pp);
>> + else
>> + msg.address_lo = virt_to_phys((void *)pp->msi_data);
>> msg.address_hi = 0x0;
>> msg.data = pos;
>> write_msi_msg(irq, &msg);
>> @@ -768,7 +775,7 @@ static struct pci_ops dw_pcie_ops = {
>> .write = dw_pcie_wr_conf,
>> };
>>
>> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
>> +int dw_pcie_setup(int nr, struct pci_sys_data *sys)
>> {
>> struct pcie_port *pp;
>>
>> @@ -791,7 +798,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data
>> *sys)
>> return 1;
>> }
>>
>> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> +struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> {
>> struct pci_bus *bus;
>> struct pcie_port *pp = sys_to_pcie(sys); @@ -808,7 +815,7 @@ static
>> struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> return bus;
>> }
>>
>> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
>> +int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
>> {
>> struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
>> int irq;
>> @@ -820,7 +827,7 @@ static int dw_pcie_map_irq(const struct pci_dev
>> *dev, u8 slot, u8 pin)
>> return irq;
>> }
>>
>> -static void dw_pcie_add_bus(struct pci_bus *bus)
>> +void dw_pcie_add_bus(struct pci_bus *bus)
>> {
>> if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> struct pcie_port *pp = sys_to_pcie(bus->sysdata); diff --git
>> a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
>> index 2d6dd66..3a6a6eb 100644
>> --- a/drivers/pci/host/pcie-designware.h
>> +++ b/drivers/pci/host/pcie-designware.h
>> @@ -39,6 +39,12 @@ struct pcie_port {
>> void __iomem *va_cfg0_base;
>> u64 cfg1_base;
>> void __iomem *va_cfg1_base;
>> + /*
>> + * v3.65 DW hw implements application register space for
>> + * MSI and has no ATU view port
>> + */
>> +#define DW_V3_65 BIT(0)
>> + u32 version;
>> u64 io_base;
>> u64 mem_base;
>> spinlock_t conf_lock;
>> @@ -68,6 +74,7 @@ struct pcie_host_ops {
>> unsigned int devfn, int where, int size, u32 val);
>> int (*link_up)(struct pcie_port *pp);
>> void (*host_init)(struct pcie_port *pp);
>> + u32 (*get_msi_data)(struct pcie_port *pp);
>> };
>>
>> int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
>> @@ -77,5 +84,14 @@ void dw_pcie_msi_init(struct pcie_port *pp); int
>> dw_pcie_link_up(struct pcie_port *pp); void dw_pcie_setup_rc(struct
>> pcie_port *pp); int dw_pcie_host_init(struct pcie_port *pp);
>> +int dw_pcie_setup(int nr, struct pci_sys_data *sys); struct pci_bus
>> +*dw_pcie_scan_bus(int nr, struct pci_sys_data *sys); void
>> +dw_pcie_add_bus(struct pci_bus *bus); int
>> dw_pcie_parse_resource(struct
>> +pcie_port *pp);
>>
>> +/* internal to DW common core driver */ int
>> +dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw); int
>> +dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
>> + const struct irq_domain_ops *irq_ops); int
>> dw_pcie_map_irq(const
>> +struct pci_dev *dev, u8 slot, u8 pin);
>> #endif /* _PCIE_DESIGNWARE_H */
>> --
>> 1.7.9.5
On Friday 20 June 2014 01:27 PM, Murali Karicheri wrote:
> On 6/18/2014 3:13 AM, Mohit KUMAR DCG wrote:
>> Hello Murali,
>>
>>> -----Original Message-----
>>> From: Murali Karicheri [mailto:[email protected]]
>>> Sent: Wednesday, June 11, 2014 12:21 AM
>>> To: [email protected]; [email protected];
>>> [email protected]; [email protected]; linux-
>>> [email protected]
>>> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
>>> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
>>> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
>>> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
>>> Subject: [PATCH v2 3/8] PCI: designware: update pcie core driver to work
>>> with dw hw version 3.65
>>>
>>> v3.65 version of the dw hw has MSI controller implemented in the application
>>> space. Add a version variable in the port struct to identify v3.65 hardware for
>>> different code treatment. This variable will have DW_V3_65 bit set when
>>> running on this hw version. The host init code is expected to set this version
>>> based on compatibility string dw,snps-pcie-v3.65.
>>>
>>> Some of the MSI specific functions from current DW driver are re-used on
>>> v3.65 hw. However on v3.65, MSI controller registers are in the application
>>> register space and PCIE_MSI_INTR0_ENABLE is not applicable.
>>> Modify assign_irq() to check for version and not execute the code for
>>> PCIE_MSI_INTR0_ENABLE configuration on v3.65 hw. Additionally MSI IRQ
>>> register in application space is written by EP to raise an MSI IRQ. So add a
>>> get_msi_data() function in pcie_host_ops to retrieve the register address in
>>> dw_msi_setup_irq(). v3.65 dw core driver implements this function.
>>>
>>> Also make some of the functions available in dw core driver global and make
>>> their prototypes available in the header file for re-use on v3.65.
>>>
>> - Pls apply MSI specific changes on the top of Lucas patches:
>> [PATCH 0/4] proper multi MSI handling for designware host
>>
>> Thanks
>> Mohit
> Ok. I have pulled in Lucas's patches and use it for my work.
Are those patches already acked and getting into the queue. Please
don't create UN-necessary merge dependencies otherwise.
regards,
Santosh
On 6/18/2014 3:05 AM, Pratyush Anand wrote:
> Hi Murali,
>
> On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
>> Current DW PCI host init code has code specific to newer hw such as
>> ATU port specific resource parsing and map. v3.65 DW PCI host has
> OK, Older version did not had standard viewport implementation, so patch 1
> of this series will help you to take care for that.
>
>> MSI controller in application space and requires different controller
> Since MSI controller is implemented in application space, so this may
> not be same for different older version dw controller users.
>
> Therefore, I would suggest to implement all application specific code
> in your keystone driver only.
Pratyush,
Thanks for the comments.
This is IP specific code and another driver that has this version of the
IP will be able to
re-use the code. This is also being discussed in another thread from
Bjorn and Jingoo.
Please discuss this in that thread.
>> initialization code. So refactor the msi host controller code into a
>> separate function. Other common functions across both hw versions
>> are moved to dw_pcie_common_host_init() and called from the newer and
>> v3.65 hw host initialization code.
>>
>> Signed-off-by: Murali Karicheri <[email protected]>
>>
>> CC: Santosh Shilimkar <[email protected]>
>> CC: Russell King <[email protected]>
>> CC: Grant Likely <[email protected]>
>> CC: Rob Herring <[email protected]>
>> CC: Mohit Kumar <[email protected]>
>> CC: Jingoo Han <[email protected]>
>> CC: Bjorn Helgaas <[email protected]>
>> CC: Pratyush Anand <[email protected]>
>> CC: Richard Zhu <[email protected]>
>> CC: Kishon Vijay Abraham I <[email protected]>
>> CC: Marek Vasut <[email protected]>
>> CC: Arnd Bergmann <[email protected]>
>> CC: Pawel Moll <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: Kumar Gala <[email protected]>
>> CC: Randy Dunlap <[email protected]>
>> CC: Grant Likely <[email protected]>
>>
>> ---
>> drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
>> 1 file changed, 101 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index e8f5d8d..e4bd19a 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
>> .map = dw_pcie_msi_map,
>> };
>>
>> -int __init dw_pcie_host_init(struct pcie_port *pp)
>> +/*
>> + * dw_pcie_parse_resource() - Function to parse common resources
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * Parse the range property for MEM, IO and cfg resources, and map
>> + * the cfg register space.
>> + */
> Why do you need this function? If you have some extra resource, you
> can ioremap that before you call dw_pcie_host_init.
I assume you are referring to dw_pcie_parse_resource(). Keystone PCIe
driver needs
this to parse the range property for IORESOURCE_MEM and IORESOURCE_IO
resources.
So refactored this into a function and called from host_init()
code for v3.65 and dw_pcie_host_init . But for Keystone PCI driver, no
need to ioremap
cfg1 and cfg2 as it doesn't have ATU ports. So host_init() code has to
be little different.
Idea is to have IP specific host_init() and refactor and re-use the
common code on both.
>
>> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
>> {
>> struct device_node *np = pp->dev->of_node;
>> struct of_pci_range range;
>> struct of_pci_range_parser parser;
>> - u32 val;
>> - int i;
>>
>> if (of_pci_range_parser_init(&parser, np)) {
>> dev_err(pp->dev, "missing ranges property\n");
>> @@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>> }
>> }
>> -
>> - if (!pp->dbi_base) {
>> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> - resource_size(&pp->cfg));
>> - if (!pp->dbi_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> - }
>> -
>> - 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,
>> - pp->config.cfg0_size);
>> - if (!pp->va_cfg0_base) {
>> - dev_err(pp->dev, "error with ioremap in function\n");
>> - return -ENOMEM;
>> - }
>> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> - pp->config.cfg1_size);
>> - if (!pp->va_cfg1_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> + return 0;
>> +}
>>
>> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> - return -EINVAL;
>> - }
>> +/*
>> + * dw_pcie_msi_host_init() - Function to initialize msi host controller
>> + *
>> + * @pp: ptr to pcie port
>> + * @np: device node ptr to msi irq controller
>> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
>> + *
>> + * Function register irq domain for msi irq controller and create mappings
>> + * for MSI irqs.
>> + */
>
> May be you can only do following to support your MSI chip:
>
> Initialize pp->irq_domain into your add_pcie_port function before you
> call dw_pcie_host_init.
>
> In dw_pcie_host_init,
>
> if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
How do I pass the keystone specific msi irq_domain_ops? It looks clean
the way it is implemented
in this patch. Also since MSI host controller and legacy host controller
are different, it make
sense to add separate host_init for each. Let me know.
Regards,
Murali
>
>> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
>> + const struct irq_domain_ops *irq_msi_ops)
>> +{
>> + int i;
>>
>> if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
>> - MAX_MSI_IRQS, &msi_domain_ops,
>> + MAX_MSI_IRQS, irq_msi_ops,
>> &dw_pcie_msi_chip);
>> if (!pp->irq_domain) {
>> dev_err(pp->dev, "irq domain init failed\n");
>> @@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> irq_create_mapping(pp->irq_domain, i);
>> }
>>
>> + return 0;
>> +}
>> +
>> +/*
>> + * dw_pcie_common_host_init() - common host init function across different
>> + * versions of the designware PCI controller.
>> + * @pp: ptr to pcie port
>> + * @hw: ptr to hw_pci structure
>> + *
>> + * This functions parses common DT properties, call host_init() callback
>> + * of the PCI controller driver. Also initialize the common RC configurations
>> + * and call common pci core function to intialize the controller driver.
>> + */
>> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
>> +{
>> + struct device_node *np = pp->dev->of_node;
>> + u32 val;
>> +
>> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> + return -EINVAL;
>> + }
>> +
>> if (pp->ops->host_init)
>> pp->ops->host_init(pp);
>>
>> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> val |= PORT_LOGIC_SPEED_CHANGE;
>> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>>
>> - dw_pci.nr_controllers = 1;
>> - dw_pci.private_data = (void **)&pp;
>> + hw->nr_controllers = 1;
>> + hw->private_data = (void **)&pp;
>>
>> - pci_common_init_dev(pp->dev, &dw_pci);
>> + pci_common_init_dev(pp->dev, hw);
>> pci_assign_unassigned_resources();
>> #ifdef CONFIG_PCI_DOMAINS
>> dw_pci.domain++;
>> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> return 0;
>> }
>>
>> +/*
>> + * dw_pcie_host_init() - Host init function for new designware h/w
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * The function parse the PCI resurces for IO, Memory and map the config
>> + * space addresses. Also initliaze the MSI irq controller and call
>> + * dw_pcie_common_host_init() to initialize the PCI controller.
>> + */
>> +int __init dw_pcie_host_init(struct pcie_port *pp)
>> +{
>> + int ret;
>> +
>> + ret = dw_pcie_parse_resource(pp);
>> + if (ret)
>> + return ret;
>> +
>> + if (!pp->dbi_base) {
>> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> + resource_size(&pp->cfg));
>> + if (!pp->dbi_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> + }
>> +
>> + pp->cfg0_base = pp->cfg.start;
>> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
>> +
>> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>> + pp->config.cfg0_size);
>> + if (!pp->va_cfg0_base) {
>> + dev_err(pp->dev, "error with ioremap in function\n");
>> + return -ENOMEM;
>> + }
>> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> + pp->config.cfg1_size);
>> + if (!pp->va_cfg1_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> +
>> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return dw_pcie_common_host_init(pp, &dw_pci);
>> +}
>> +
>> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
>> {
>> /* Program viewport 0 : OUTBOUND : CFG0 */
> Regards
> Pratyush
>
>> --
>> 1.7.9.5
On 6/18/2014 3:05 AM, Pratyush Anand wrote:
> Hi Murali,
>
> On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
>> Current DW PCI host init code has code specific to newer hw such as
>> ATU port specific resource parsing and map. v3.65 DW PCI host has
> OK, Older version did not had standard viewport implementation, so patch 1
> of this series will help you to take care for that.
>
>> MSI controller in application space and requires different controller
> Since MSI controller is implemented in application space, so this may
> not be same for different older version dw controller users.
>
> Therefore, I would suggest to implement all application specific code
> in your keystone driver only.
Pratyush,
Thanks for the comments.
This is IP specific code and another driver that has this version of the
IP will be able to
re-use the code. This is also being discussed in another thread from
Bjorn and Jingoo.
Please discuss this in that thread.
>> initialization code. So refactor the msi host controller code into a
>> separate function. Other common functions across both hw versions
>> are moved to dw_pcie_common_host_init() and called from the newer and
>> v3.65 hw host initialization code.
>>
>> Signed-off-by: Murali Karicheri <[email protected]>
>>
>> CC: Santosh Shilimkar <[email protected]>
>> CC: Russell King <[email protected]>
>> CC: Grant Likely <[email protected]>
>> CC: Rob Herring <[email protected]>
>> CC: Mohit Kumar <[email protected]>
>> CC: Jingoo Han <[email protected]>
>> CC: Bjorn Helgaas <[email protected]>
>> CC: Pratyush Anand <[email protected]>
>> CC: Richard Zhu <[email protected]>
>> CC: Kishon Vijay Abraham I <[email protected]>
>> CC: Marek Vasut <[email protected]>
>> CC: Arnd Bergmann <[email protected]>
>> CC: Pawel Moll <[email protected]>
>> CC: Mark Rutland <[email protected]>
>> CC: Ian Campbell <[email protected]>
>> CC: Kumar Gala <[email protected]>
>> CC: Randy Dunlap <[email protected]>
>> CC: Grant Likely <[email protected]>
>>
>> ---
>> drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
>> 1 file changed, 101 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index e8f5d8d..e4bd19a 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
>> .map = dw_pcie_msi_map,
>> };
>>
>> -int __init dw_pcie_host_init(struct pcie_port *pp)
>> +/*
>> + * dw_pcie_parse_resource() - Function to parse common resources
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * Parse the range property for MEM, IO and cfg resources, and map
>> + * the cfg register space.
>> + */
> Why do you need this function? If you have some extra resource, you
> can ioremap that before you call dw_pcie_host_init.
I assume you are referring to dw_pcie_parse_resource(). Keystone PCIe
driver needs
this to parse the range property for IORESOURCE_MEM and IORESOURCE_IO
resources.
So refactored this into a function and called from host_init()
code for v3.65 and dw_pcie_host_init . But for Keystone PCI driver, no
need to ioremap
cfg1 and cfg2 as it doesn't have ATU ports. So host_init() code has to
be little different.
Idea is to have IP specific host_init() and refactor and re-use the
common code on both.
>
>> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
>> {
>> struct device_node *np = pp->dev->of_node;
>> struct of_pci_range range;
>> struct of_pci_range_parser parser;
>> - u32 val;
>> - int i;
>>
>> if (of_pci_range_parser_init(&parser, np)) {
>> dev_err(pp->dev, "missing ranges property\n");
>> @@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>> }
>> }
>> -
>> - if (!pp->dbi_base) {
>> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> - resource_size(&pp->cfg));
>> - if (!pp->dbi_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> - }
>> -
>> - 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,
>> - pp->config.cfg0_size);
>> - if (!pp->va_cfg0_base) {
>> - dev_err(pp->dev, "error with ioremap in function\n");
>> - return -ENOMEM;
>> - }
>> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> - pp->config.cfg1_size);
>> - if (!pp->va_cfg1_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> + return 0;
>> +}
>>
>> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> - return -EINVAL;
>> - }
>> +/*
>> + * dw_pcie_msi_host_init() - Function to initialize msi host controller
>> + *
>> + * @pp: ptr to pcie port
>> + * @np: device node ptr to msi irq controller
>> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
>> + *
>> + * Function register irq domain for msi irq controller and create mappings
>> + * for MSI irqs.
>> + */
>
> May be you can only do following to support your MSI chip:
>
> Initialize pp->irq_domain into your add_pcie_port function before you
> call dw_pcie_host_init.
>
> In dw_pcie_host_init,
>
> if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
How do I pass the keystone specific msi irq_domain_ops? It looks clean
the way it is implemented
in this patch. Also since MSI host controller and legacy host controller
are different, it make
sense to add separate host_init for each. Let me know.
Regards,
Murali
>
>> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
>> + const struct irq_domain_ops *irq_msi_ops)
>> +{
>> + int i;
>>
>> if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
>> - MAX_MSI_IRQS, &msi_domain_ops,
>> + MAX_MSI_IRQS, irq_msi_ops,
>> &dw_pcie_msi_chip);
>> if (!pp->irq_domain) {
>> dev_err(pp->dev, "irq domain init failed\n");
>> @@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> irq_create_mapping(pp->irq_domain, i);
>> }
>>
>> + return 0;
>> +}
>> +
>> +/*
>> + * dw_pcie_common_host_init() - common host init function across different
>> + * versions of the designware PCI controller.
>> + * @pp: ptr to pcie port
>> + * @hw: ptr to hw_pci structure
>> + *
>> + * This functions parses common DT properties, call host_init() callback
>> + * of the PCI controller driver. Also initialize the common RC configurations
>> + * and call common pci core function to intialize the controller driver.
>> + */
>> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
>> +{
>> + struct device_node *np = pp->dev->of_node;
>> + u32 val;
>> +
>> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> + return -EINVAL;
>> + }
>> +
>> if (pp->ops->host_init)
>> pp->ops->host_init(pp);
>>
>> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> val |= PORT_LOGIC_SPEED_CHANGE;
>> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>>
>> - dw_pci.nr_controllers = 1;
>> - dw_pci.private_data = (void **)&pp;
>> + hw->nr_controllers = 1;
>> + hw->private_data = (void **)&pp;
>>
>> - pci_common_init_dev(pp->dev, &dw_pci);
>> + pci_common_init_dev(pp->dev, hw);
>> pci_assign_unassigned_resources();
>> #ifdef CONFIG_PCI_DOMAINS
>> dw_pci.domain++;
>> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> return 0;
>> }
>>
>> +/*
>> + * dw_pcie_host_init() - Host init function for new designware h/w
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * The function parse the PCI resurces for IO, Memory and map the config
>> + * space addresses. Also initliaze the MSI irq controller and call
>> + * dw_pcie_common_host_init() to initialize the PCI controller.
>> + */
>> +int __init dw_pcie_host_init(struct pcie_port *pp)
>> +{
>> + int ret;
>> +
>> + ret = dw_pcie_parse_resource(pp);
>> + if (ret)
>> + return ret;
>> +
>> + if (!pp->dbi_base) {
>> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> + resource_size(&pp->cfg));
>> + if (!pp->dbi_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> + }
>> +
>> + pp->cfg0_base = pp->cfg.start;
>> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
>> +
>> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>> + pp->config.cfg0_size);
>> + if (!pp->va_cfg0_base) {
>> + dev_err(pp->dev, "error with ioremap in function\n");
>> + return -ENOMEM;
>> + }
>> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> + pp->config.cfg1_size);
>> + if (!pp->va_cfg1_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> +
>> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return dw_pcie_common_host_init(pp, &dw_pci);
>> +}
>> +
>> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
>> {
>> /* Program viewport 0 : OUTBOUND : CFG0 */
> Regards
> Pratyush
>
>> --
>> 1.7.9.5
On Friday 20 June 2014 13:11:37 Santosh Shilimkar wrote:
> >
> > Unfortunately I lost Jingoo's email from my Inbox. So I cut-n-paste the comment from
> > internet and respond.
> >
> > Jingoo Han wrote:-
> > =============================================================================
> > I think so, too.
> >
> > DT maintainers and arch maintainers should review the following
> > dt bindings.
> > .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> > .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> > Generic PHY maintainer (Kishon Vijay Abraham I) should review the
> > following phy driver.
> > drivers/phy/phy-keystone-serdes.c
> >
> >>
> >> I'm looking for acks from Mohit and/or Jingoo for the pci/host
> >> changes, and from Arnd for the devicetree/bindings changes.
> >>
> >> Adding these "-dw-3_64" files is sort of ugly. If that code is only
> >> used by keystone, maybe it could just be moved to pci-keystone.c? But
> >> I'll defer to Mohit and Jingoo on that and the way you modify
> >> pcie-designware.c.
> >
> > I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
> > look terrible! I don't have a good way to handle this; however,
> > moving this code to pci-keystone.c looks better.
> > ======================================================================
> >
> > The original RFC I had submitted had all of the application space register
> > handling code as part of the Keystone PCI driver. As per Arnd's comment (See
> > my change log against v1), the code was moved to a separate file so that
> > the next driver that has same version of the DW hw could re-use this code.
> > I agree with Arnd and moved the code to v_3_65 specific files. What is
> > your proposal? Do you have objection to the file name? or it's content?
> >
> > If objection is on the file name, please suggest alternate names. If you
> > are okay with the file name, and doesn't like the code, it will be helpful
> > to review the code and provide specific comments against the patch itself
> > so that I can address the same.
> >
> Arnd suggestion was to have the version 3.65 code in generic place since
> its IP specific and just in case some other vendor using the same version
> can leverage the code.
>
> Concern here seems toe really those name of the files. I can't think of
> any other appropriate name.
We should definitely keep the version in the DT "compatible" strings
wherever we know it. Regarding a better file name, I have no idea.
Arnd
Sorry, my previous response was in html and not sure it has made to the
list. I did
get an error as well. So resending my response.
On 6/18/2014 6:14 AM, Mohit KUMAR DCG wrote:
> Hello Murali,
>
>> -----Original Message-----
>> From: Murali Karicheri [mailto:[email protected]]
>> Sent: Wednesday, June 11, 2014 12:21 AM
>> To:[email protected];[email protected];
>> [email protected];[email protected]; linux-
>> [email protected]
>> Cc: Murali Karicheri; Santosh Shilimkar; Russell King; Grant Likely; Rob Herring;
>> Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Pratyush ANAND; Richard
>> Zhu; Kishon Vijay Abraham I; Marek Vasut; Arnd Bergmann; Pawel Moll;
>> Mark Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
>> Subject: [PATCH v2 0/8] Add Keystone PCIe controller driver
>>
>> This patch adds a PCIe controller driver for Keystone SoCs. This is based on v1
>> of the series posted to the mailing list.
>>
> 1. I think your first patch is OK which handles platform specific ATU implementation.
>
> 2. For MSI part, I think you just need to add two new callbacks with pp-ops, something similar to:
> pp->ops->msi_set
>
> pp->ops->msi_clear
>
> With these two platform specific callbacks you should be able to manage MSI handling.
> So idea is that dw_msi code uses pp->ops->msi_set/clear if platform define these,
> otherwise use dw_msi_set/clear (which you need to refactor from existing code)
>
> So other than your keystone changes we expect 3 patches:
> -- 1st same as you sent 1/8: for ATU handeling
> -- 2nd to refactor dw_msi_set/clear: refactor from existing code
Mohit,
Thanks for your comments.
Just want to be on the same page
1. In my original patch [PATCH v2 3/8] PCI: designware: update pcie core
driver to work with dw hw version 3.65.
I had following changes:-
diff --git a/drivers/pci/host/pcie-designware.c
b/drivers/pci/host/pcie-designware.c
index e4bd19a..f985811 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -277,11 +277,15 @@ static int assign_irq(int no_irqs, struct msi_desc
*desc, int *pos)
}
set_bit(pos0 + i, pp->msi_irq_in_use);
/*Enable corresponding interrupt in MSI interrupt controller */
-res = ((pos0 + i) / 32) * 12;
-bit = (pos0 + i) % 32;
-dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
-val |= 1 << bit;
-dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
+if (!(pp->version & DW_V3_65)) {
+res = ((pos0 + i) / 32) * 12;
+bit = (pos0 + i) % 32;
+dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+4, &val);
+val |= 1 << bit;
+dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res,
+4, val);
+}
}
I assume you are referring to the above code for msi_set(). I missed to
add similar code change to clear_irq().
So in assign_irq()
if (pp->ops->msi_set)
pp->ops->msi_set()
Similarly in clear_irq()
if (pp->ops->msi_clear)
pp->ops->msi_clear()
*pos = pos0;
@@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
struct pci_dev *pdev,
*/
desc->msi_attrib.multiple = msgvec;
-msg.address_lo = virt_to_phys((void *)pp->msi_data);
+if (pp->ops->get_msi_data)
+msg.address_lo = pp->ops->get_msi_data(pp);
+else
+msg.address_lo = virt_to_phys((void *)pp->msi_data);
msg.address_hi = 0x0;
msg.data = pos;
What about this code? This requires get_msi_data() as well
> -- 3rd to use pp->ops->msi_set/clear if defined.
Why not API enhancement and refactor the code in a single patch?
Murali
> Pls let us know for any issue or have different opinion.
>
> Regards
> Mohit
>
>
>
>
>> --
>> 1.7.9.5
On Saturday, June 21, 2014 12:31 AM, Murali Karicheri wrote:
> On 6/17/2014 8:08 PM, Bjorn Helgaas wrote:
> > On Tue, Jun 10, 2014 at 02:51:19PM -0400, Murali Karicheri wrote:
> >> This patch adds a PCIe controller driver for Keystone SoCs. This
> >> is based on v1 of the series posted to the mailing list.
> >>
> >> CC: Santosh Shilimkar <[email protected]>
> >> CC: Russell King <[email protected]>
> >> CC: Grant Likely <[email protected]>
> >> CC: Rob Herring <[email protected]>
> >> CC: Mohit Kumar <[email protected]>
> >> CC: Jingoo Han <[email protected]>
> >> CC: Bjorn Helgaas <[email protected]>
> >> CC: Pratyush Anand <[email protected]>
> >> CC: Richard Zhu <[email protected]>
> >> CC: Kishon Vijay Abraham I <[email protected]>
> >> CC: Marek Vasut <[email protected]>
> >> CC: Arnd Bergmann <[email protected]>
> >> CC: Pawel Moll <[email protected]>
> >> CC: Mark Rutland <[email protected]>
> >> CC: Ian Campbell <[email protected]>
> >> CC: Kumar Gala <[email protected]>
> >> CC: Randy Dunlap <[email protected]>
> >> CC: Grant Likely <[email protected]>
> >>
> >>
> >> Changelog:
> >>
> >> V2
> >> - Split the designware pcie enhancement patch to multiple
> >> patches based on functionality added
> >> - Remove the quirk code and add a patch to fix mps/mrss
> >> tuning for ARM. Use kernel command line parameter
> >> pci=pcie_bus_perf to work with Keystone PCI Controller.
> >> Following patch addressed this.
> >> [PATCH v1] ARM: pci: add call to pcie_bus_configure_settings()
> >> - Add documentation for device tree bindings
> >> - Add separate interrupt controller nodes for MSI and Legacy
> >> IRQs and use irq map for legacy IRQ
> >> - Use compatibility to identify v3.65 version of the DW hardware
> >> and use it to customize the designware common code.
> >> - Use reg property for configuration space instead of range
> >> - Other minor updates based on code inspection.
> >>
> >> V1
> >> - Add an interrupt controller node for Legacy irq chip and use
> >> interrupt map/map-mask property to map legacy IRQs A/B/C/D
> >> - Add a Phy driver to replace the original serdes driver
> >> - Move common application register handling code to a separate
> >> file to allow re-use across other platforms that use older
> >> DW PCIe h/w
> >> - PCI quirk for maximum read request size. Check and override only
> >> if the maximum is higher than what controller can handle.
> >> - Converted to a module platform driver.
> >>
> >>
> >> Murali Karicheri (8):
> >> PCI: designware: add rd[wr]_other_conf API
> >> PCI: designware: refactor host init code to re-use on v3.65 DW pci hw
> >> PCI: designware: update pcie core driver to work with dw hw version
> >> 3.65
> >> PCI: designware: add msi controller functions for v3.65 hw
> >> PCI: designware: add PCI controller functions for v3.65 DW hw
> >> phy: Add serdes phy driver for keystone
> >> PCI: keystone: add pcie driver based on designware core driver
> >> ARM: keystone: add pcie related options
> >>
> >> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> >> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> >> .../bindings/phy/phy-keystone-serdes.txt | 25 ++
> >> arch/arm/mach-keystone/Kconfig | 1 +
> >> drivers/pci/host/Kconfig | 12 +
> >> drivers/pci/host/Makefile | 2 +
> >> drivers/pci/host/pci-dw-v3_65-msi.c | 149 +++++++
> >> drivers/pci/host/pci-dw-v3_65.c | 390 ++++++++++++++++++
> >> drivers/pci/host/pci-dw-v3_65.h | 34 ++
> >> drivers/pci/host/pci-keystone.c | 418 ++++++++++++++++++++
> >> drivers/pci/host/pcie-designware.c | 175 +++++---
> >> drivers/pci/host/pcie-designware.h | 42 +-
> >> drivers/phy/Kconfig | 6 +
> >> drivers/phy/Makefile | 1 +
> >> drivers/phy/phy-keystone-serdes.c | 230 +++++++++++
> >> 15 files changed, 1531 insertions(+), 52 deletions(-)
> >> create mode 100644 Documentation/devicetree/bindings/pci/pci-keystone.txt
> >> create mode 100644 Documentation/devicetree/bindings/phy/phy-keystone-serdes.txt
> >> create mode 100644 drivers/pci/host/pci-dw-v3_65-msi.c
> >> create mode 100644 drivers/pci/host/pci-dw-v3_65.c
> >> create mode 100644 drivers/pci/host/pci-dw-v3_65.h
> >> create mode 100644 drivers/pci/host/pci-keystone.c
> >> create mode 100644 drivers/phy/phy-keystone-serdes.c
> > I'm not willing to merge phy-keystone-serdes.c because I don't
> > maintain drivers/phy and because of the binary blob of register values
> > it contains, but maybe somebody else will. I assume it could be
> > merged by itself before the rest of this.
> >
> > I'm looking for acks from Mohit and/or Jingoo for the pci/host
> > changes, and from Arnd for the devicetree/bindings changes.
> >
> > Adding these "-dw-3_64" files is sort of ugly. If that code is only
> > used by keystone, maybe it could just be moved to pci-keystone.c? But
> > I'll defer to Mohit and Jingoo on that and the way you modify
> > pcie-designware.c.
> >
> > Bjorn
> Bjorn, Jingoo,
>
> Unfortunately I lost Jingoo's email from my Inbox. So I cut-n-paste the
> comment from
> internet and respond.
>
> Jingoo Han wrote:-
> =============================================================================
> I think so, too.
>
> DT maintainers and arch maintainers should review the following
> dt bindings.
> .../devicetree/bindings/pci/designware-pcie.txt | 42 ++
> .../devicetree/bindings/pci/pci-keystone.txt | 56 +++
> Generic PHY maintainer (Kishon Vijay Abraham I) should review the
> following phy driver.
> drivers/phy/phy-keystone-serdes.c
>
> >
> > I'm looking for acks from Mohit and/or Jingoo for the pci/host
> > changes, and from Arnd for the devicetree/bindings changes.
> >
> > Adding these "-dw-3_64" files is sort of ugly. If that code is only
> > used by keystone, maybe it could just be moved to pci-keystone.c? But
> > I'll defer to Mohit and Jingoo on that and the way you modify
> > pcie-designware.c.
>
> I agree with Bjorn Helgaas's opinion. These three "-dw-3_64" files
> look terrible! I don't have a good way to handle this; however,
> moving this code to pci-keystone.c looks better.
> ======================================================================
>
> The original RFC I had submitted had all of the application space register
> handling code as part of the Keystone PCI driver. As per Arnd's comment
> (See
> my change log against v1), the code was moved to a separate file so that
> the next driver that has same version of the DW hw could re-use this code.
> I agree with Arnd and moved the code to v_3_65 specific files. What is
> your proposal? Do you have objection to the file name? or it's content?
I said utterly that "I agree with Bjorn Helgaas's opinion". Also, Bjorn
said that "Adding these "-dw-3_64" files is sort of ugly".
Thus, the point is the file name, not the content, as Santosh Shilimkar
said.
>
> If objection is on the file name, please suggest alternate names. If you
> are okay with the file name, and doesn't like the code, it will be helpful
> to review the code and provide specific comments against the patch itself
> so that I can address the same.
I say again that "I have no idea about the alternate file names".
However, if you want to add v_3_65 specific files, please don't
split the files. Please make these three files as 1 file.
drivers/pci/host/pci-dw-v3_65-msi.c
drivers/pci/host/pci-dw-v3_65.c
drivers/pci/host/pci-dw-v3_65.h
--> drivers/pci/host/pci-dw-v3-65.c
(I don't have no idea about the name)
Best regards,
Jingoo Han
>
> Murali
On Sat, Jun 21, 2014 at 02:47:27AM +0800, Murali Karicheri wrote:
> On 6/18/2014 3:05 AM, Pratyush Anand wrote:
> > Hi Murali,
> >
> > On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
[...]
> Pratyush,
>
> Thanks for the comments.
>
> This is IP specific code and another driver that has this version of the
> IP will be able to
> re-use the code. This is also being discussed in another thread from
> Bjorn and Jingoo.
Anything which is implemented in application space, will not be same
across all IP users. For example, you have MSI0_IRQ_ENABLE_SET at
offset 0x108 in application space.Other vendor may not have this
register at the same offset. Moreover, other vendors are not even
obliged to implement MSI Enable signals in same way, so internal bit
definition of the register may change.
In summary: Code is only reusable if all register offset and bit
definitions are same across vendors. In case of DW driver none of the
code which are accessed using va_app_base should go to common area.
> Please discuss this in that thread.
Sorry, I am not following pci mailing list. Doing something else these
days. So if you can help me with the thread link.
Regards
Pratyush
>
> >> initialization code. So refactor the msi host controller code into a
> >> separate function. Other common functions across both hw versions
> >> are moved to dw_pcie_common_host_init() and called from the newer and
> >> v3.65 hw host initialization code.
> >>
> >> Signed-off-by: Murali Karicheri <[email protected]>
> >>
> >> CC: Santosh Shilimkar <[email protected]>
> >> CC: Russell King <[email protected]>
> >> CC: Grant Likely <[email protected]>
> >> CC: Rob Herring <[email protected]>
> >> CC: Mohit Kumar <[email protected]>
> >> CC: Jingoo Han <[email protected]>
> >> CC: Bjorn Helgaas <[email protected]>
> >> CC: Pratyush Anand <[email protected]>
> >> CC: Richard Zhu <[email protected]>
> >> CC: Kishon Vijay Abraham I <[email protected]>
> >> CC: Marek Vasut <[email protected]>
> >> CC: Arnd Bergmann <[email protected]>
> >> CC: Pawel Moll <[email protected]>
> >> CC: Mark Rutland <[email protected]>
> >> CC: Ian Campbell <[email protected]>
> >> CC: Kumar Gala <[email protected]>
> >> CC: Randy Dunlap <[email protected]>
> >> CC: Grant Likely <[email protected]>
> >>
> >> ---
> >> drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
> >> 1 file changed, 101 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> >> index e8f5d8d..e4bd19a 100644
> >> --- a/drivers/pci/host/pcie-designware.c
> >> +++ b/drivers/pci/host/pcie-designware.c
> >> @@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
> >> .map = dw_pcie_msi_map,
> >> };
> >>
> >> -int __init dw_pcie_host_init(struct pcie_port *pp)
> >> +/*
> >> + * dw_pcie_parse_resource() - Function to parse common resources
> >> + *
> >> + * @pp: ptr to pcie port
> >> + *
> >> + * Parse the range property for MEM, IO and cfg resources, and map
> >> + * the cfg register space.
> >> + */
> > Why do you need this function? If you have some extra resource, you
> > can ioremap that before you call dw_pcie_host_init.
>
> I assume you are referring to dw_pcie_parse_resource(). Keystone PCIe
> driver needs
> this to parse the range property for IORESOURCE_MEM and IORESOURCE_IO
> resources.
> So refactored this into a function and called from host_init()
> code for v3.65 and dw_pcie_host_init . But for Keystone PCI driver, no
> need to ioremap
> cfg1 and cfg2 as it doesn't have ATU ports. So host_init() code has to
> be little different.
> Idea is to have IP specific host_init() and refactor and re-use the
> common code on both.
>
> >
> >> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
> >> {
> >> struct device_node *np = pp->dev->of_node;
> >> struct of_pci_range range;
> >> struct of_pci_range_parser parser;
> >> - u32 val;
> >> - int i;
> >>
> >> if (of_pci_range_parser_init(&parser, np)) {
> >> dev_err(pp->dev, "missing ranges property\n");
> >> @@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> >> }
> >> }
> >> -
> >> - if (!pp->dbi_base) {
> >> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >> - resource_size(&pp->cfg));
> >> - if (!pp->dbi_base) {
> >> - dev_err(pp->dev, "error with ioremap\n");
> >> - return -ENOMEM;
> >> - }
> >> - }
> >> -
> >> - 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,
> >> - pp->config.cfg0_size);
> >> - if (!pp->va_cfg0_base) {
> >> - dev_err(pp->dev, "error with ioremap in function\n");
> >> - return -ENOMEM;
> >> - }
> >> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> >> - pp->config.cfg1_size);
> >> - if (!pp->va_cfg1_base) {
> >> - dev_err(pp->dev, "error with ioremap\n");
> >> - return -ENOMEM;
> >> - }
> >> + return 0;
> >> +}
> >>
> >> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> >> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
> >> - return -EINVAL;
> >> - }
> >> +/*
> >> + * dw_pcie_msi_host_init() - Function to initialize msi host controller
> >> + *
> >> + * @pp: ptr to pcie port
> >> + * @np: device node ptr to msi irq controller
> >> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
> >> + *
> >> + * Function register irq domain for msi irq controller and create mappings
> >> + * for MSI irqs.
> >> + */
> >
> > May be you can only do following to support your MSI chip:
> >
> > Initialize pp->irq_domain into your add_pcie_port function before you
> > call dw_pcie_host_init.
> >
> > In dw_pcie_host_init,
> >
> > if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
>
> How do I pass the keystone specific msi irq_domain_ops? It looks clean
> the way it is implemented
> in this patch. Also since MSI host controller and legacy host controller
> are different, it make
> sense to add separate host_init for each. Let me know.
>
> Regards,
>
> Murali
>
> >
> >> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
> >> + const struct irq_domain_ops *irq_msi_ops)
> >> +{
> >> + int i;
> >>
> >> if (IS_ENABLED(CONFIG_PCI_MSI)) {
> >> pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
> >> - MAX_MSI_IRQS, &msi_domain_ops,
> >> + MAX_MSI_IRQS, irq_msi_ops,
> >> &dw_pcie_msi_chip);
> >> if (!pp->irq_domain) {
> >> dev_err(pp->dev, "irq domain init failed\n");
> >> @@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >> irq_create_mapping(pp->irq_domain, i);
> >> }
> >>
> >> + return 0;
> >> +}
> >> +
> >> +/*
> >> + * dw_pcie_common_host_init() - common host init function across different
> >> + * versions of the designware PCI controller.
> >> + * @pp: ptr to pcie port
> >> + * @hw: ptr to hw_pci structure
> >> + *
> >> + * This functions parses common DT properties, call host_init() callback
> >> + * of the PCI controller driver. Also initialize the common RC configurations
> >> + * and call common pci core function to intialize the controller driver.
> >> + */
> >> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
> >> +{
> >> + struct device_node *np = pp->dev->of_node;
> >> + u32 val;
> >> +
> >> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> >> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> if (pp->ops->host_init)
> >> pp->ops->host_init(pp);
> >>
> >> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >> val |= PORT_LOGIC_SPEED_CHANGE;
> >> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
> >>
> >> - dw_pci.nr_controllers = 1;
> >> - dw_pci.private_data = (void **)&pp;
> >> + hw->nr_controllers = 1;
> >> + hw->private_data = (void **)&pp;
> >>
> >> - pci_common_init_dev(pp->dev, &dw_pci);
> >> + pci_common_init_dev(pp->dev, hw);
> >> pci_assign_unassigned_resources();
> >> #ifdef CONFIG_PCI_DOMAINS
> >> dw_pci.domain++;
> >> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >> return 0;
> >> }
> >>
> >> +/*
> >> + * dw_pcie_host_init() - Host init function for new designware h/w
> >> + *
> >> + * @pp: ptr to pcie port
> >> + *
> >> + * The function parse the PCI resurces for IO, Memory and map the config
> >> + * space addresses. Also initliaze the MSI irq controller and call
> >> + * dw_pcie_common_host_init() to initialize the PCI controller.
> >> + */
> >> +int __init dw_pcie_host_init(struct pcie_port *pp)
> >> +{
> >> + int ret;
> >> +
> >> + ret = dw_pcie_parse_resource(pp);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!pp->dbi_base) {
> >> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> >> + resource_size(&pp->cfg));
> >> + if (!pp->dbi_base) {
> >> + dev_err(pp->dev, "error with ioremap\n");
> >> + return -ENOMEM;
> >> + }
> >> + }
> >> +
> >> + pp->cfg0_base = pp->cfg.start;
> >> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> >> +
> >> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
> >> + pp->config.cfg0_size);
> >> + if (!pp->va_cfg0_base) {
> >> + dev_err(pp->dev, "error with ioremap in function\n");
> >> + return -ENOMEM;
> >> + }
> >> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> >> + pp->config.cfg1_size);
> >> + if (!pp->va_cfg1_base) {
> >> + dev_err(pp->dev, "error with ioremap\n");
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return dw_pcie_common_host_init(pp, &dw_pci);
> >> +}
> >> +
> >> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
> >> {
> >> /* Program viewport 0 : OUTBOUND : CFG0 */
> > Regards
> > Pratyush
> >
> >> --
> >> 1.7.9.5
On Sat, Jun 21, 2014 at 05:17:07AM +0800, Murali Karicheri wrote:
>
> Sorry, my previous response was in html and not sure it has made to the
> list. I did
> get an error as well. So resending my response.
>
> On 6/18/2014 6:14 AM, Mohit KUMAR DCG wrote:
> > Hello Murali,
> >
>
[...]
> *pos = pos0;
>
> @@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
> struct pci_dev *pdev,
>
> */
>
> desc->msi_attrib.multiple = msgvec;
>
> -msg.address_lo = virt_to_phys((void *)pp->msi_data);
>
> +if (pp->ops->get_msi_data)
>
> +msg.address_lo = pp->ops->get_msi_data(pp);
>
> +else
>
> +msg.address_lo = virt_to_phys((void *)pp->msi_data);
>
> msg.address_hi = 0x0;
>
> msg.data = pos;
>
>
> What about this code? This requires get_msi_data() as well
pp->msi_data is set in dw_pcie_msi_init, which is a global function
called from vendor specific code. You can have your own
keystone_pcie_msi_init and then you do not need above changes.
>
> > -- 3rd to use pp->ops->msi_set/clear if defined.
> Why not API enhancement and refactor the code in a single patch?
Yes, can be. You can send changes in 2 or 3 patches as you wish, but I
believe that should be able to solve problem in best way.
Regards
Pratyush
>
> Murali
> > Pls let us know for any issue or have different opinion.
> >
> > Regards
> > Mohit
> >
> >
> >
> >
> >> --
> >> 1.7.9.5
>
Hello Murali,
> -----Original Message-----
> From: Pratyush ANAND
> Sent: Monday, June 23, 2014 10:36 AM
> To: Murali Karicheri
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Shilimkar, Santosh; Russell King; Grant Likely; Rob
> Herring; Mohit KUMAR DCG; Jingoo Han; Bjorn Helgaas; Richard Zhu;
> ABRAHAM, KISHON VIJAY; Marek Vasut; Arnd Bergmann; Pawel Moll; Mark
> Rutland; Ian Campbell; Kumar Gala; Randy Dunlap
> Subject: Re: [PATCH v2 2/8] PCI: designware: refactor host init code to re-use
> on v3.65 DW pci hw
>
> On Sat, Jun 21, 2014 at 02:47:27AM +0800, Murali Karicheri wrote:
> > On 6/18/2014 3:05 AM, Pratyush Anand wrote:
> > > Hi Murali,
> > >
> > > On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
>
> [...]
>
> > Pratyush,
> >
> > Thanks for the comments.
> >
> > This is IP specific code and another driver that has this version of
> > the IP will be able to re-use the code. This is also being discussed
> > in another thread from Bjorn and Jingoo.
>
> Anything which is implemented in application space, will not be same across
> all IP users. For example, you have MSI0_IRQ_ENABLE_SET at offset 0x108 in
> application space.Other vendor may not have this register at the same
> offset. Moreover, other vendors are not even obliged to implement MSI
> Enable signals in same way, so internal bit definition of the register may
> change.
>
> In summary: Code is only reusable if all register offset and bit definitions are
> same across vendors. In case of DW driver none of the code which are
> accessed using va_app_base should go to common area.
>
- yes, I completely agree with Pratyush.
As explained earlier also that prior to Synopsys version 3.70 MSI and ATU
code was not standardized and each platform have its specific implementation through
pcie application space logic. So let us follow the convention and not merge any application
specific code to the DW core code.
Thanks
Mohit
> > Please discuss this in that thread.
>
> Sorry, I am not following pci mailing list. Doing something else these days. So
> if you can help me with the thread link.
>
> Regards
> Pratyush
> >
> > >> initialization code. So refactor the msi host controller code into
> > >> a separate function. Other common functions across both hw versions
> > >> are moved to dw_pcie_common_host_init() and called from the newer
> > >> and
> > >> v3.65 hw host initialization code.
> > >>
> > >> Signed-off-by: Murali Karicheri <[email protected]>
> > >>
> > >> CC: Santosh Shilimkar <[email protected]>
> > >> CC: Russell King <[email protected]>
> > >> CC: Grant Likely <[email protected]>
> > >> CC: Rob Herring <[email protected]>
> > >> CC: Mohit Kumar <[email protected]>
> > >> CC: Jingoo Han <[email protected]>
> > >> CC: Bjorn Helgaas <[email protected]>
> > >> CC: Pratyush Anand <[email protected]>
> > >> CC: Richard Zhu <[email protected]>
> > >> CC: Kishon Vijay Abraham I <[email protected]>
> > >> CC: Marek Vasut <[email protected]>
> > >> CC: Arnd Bergmann <[email protected]>
> > >> CC: Pawel Moll <[email protected]>
> > >> CC: Mark Rutland <[email protected]>
> > >> CC: Ian Campbell <[email protected]>
> > >> CC: Kumar Gala <[email protected]>
> > >> CC: Randy Dunlap <[email protected]>
> > >> CC: Grant Likely <[email protected]>
> > >>
> > >> ---
> > >> drivers/pci/host/pcie-designware.c | 136
> ++++++++++++++++++++++++++----------
> > >> 1 file changed, 101 insertions(+), 35 deletions(-)
> > >>
> > >> diff --git a/drivers/pci/host/pcie-designware.c
> > >> b/drivers/pci/host/pcie-designware.c
> > >> index e8f5d8d..e4bd19a 100644
> > >> --- a/drivers/pci/host/pcie-designware.c
> > >> +++ b/drivers/pci/host/pcie-designware.c
> > >> @@ -389,13 +389,19 @@ static const struct irq_domain_ops
> msi_domain_ops = {
> > >> .map = dw_pcie_msi_map,
> > >> };
> > >>
> > >> -int __init dw_pcie_host_init(struct pcie_port *pp)
> > >> +/*
> > >> + * dw_pcie_parse_resource() - Function to parse common resources
> > >> + *
> > >> + * @pp: ptr to pcie port
> > >> + *
> > >> + * Parse the range property for MEM, IO and cfg resources, and map
> > >> + * the cfg register space.
> > >> + */
> > > Why do you need this function? If you have some extra resource, you
> > > can ioremap that before you call dw_pcie_host_init.
> >
> > I assume you are referring to dw_pcie_parse_resource(). Keystone PCIe
> > driver needs this to parse the range property for IORESOURCE_MEM and
> > IORESOURCE_IO resources.
> > So refactored this into a function and called from host_init() code
> > for v3.65 and dw_pcie_host_init . But for Keystone PCI driver, no need
> > to ioremap
> > cfg1 and cfg2 as it doesn't have ATU ports. So host_init() code has to
> > be little different.
> > Idea is to have IP specific host_init() and refactor and re-use the
> > common code on both.
> >
> > >
> > >> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
> > >> {
> > >> struct device_node *np = pp->dev->of_node;
> > >> struct of_pci_range range;
> > >> struct of_pci_range_parser parser;
> > >> - u32 val;
> > >> - int i;
> > >>
> > >> if (of_pci_range_parser_init(&parser, np)) {
> > >> dev_err(pp->dev, "missing ranges property\n"); @@ -431,41
> > >> +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > >> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
> > >> }
> > >> }
> > >> -
> > >> - if (!pp->dbi_base) {
> > >> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> > >> - resource_size(&pp->cfg));
> > >> - if (!pp->dbi_base) {
> > >> - dev_err(pp->dev, "error with ioremap\n");
> > >> - return -ENOMEM;
> > >> - }
> > >> - }
> > >> -
> > >> - 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,
> > >> - pp->config.cfg0_size);
> > >> - if (!pp->va_cfg0_base) {
> > >> - dev_err(pp->dev, "error with ioremap in function\n");
> > >> - return -ENOMEM;
> > >> - }
> > >> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> > >> - pp->config.cfg1_size);
> > >> - if (!pp->va_cfg1_base) {
> > >> - dev_err(pp->dev, "error with ioremap\n");
> > >> - return -ENOMEM;
> > >> - }
> > >> + return 0;
> > >> +}
> > >>
> > >> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> > >> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
> > >> - return -EINVAL;
> > >> - }
> > >> +/*
> > >> + * dw_pcie_msi_host_init() - Function to initialize msi host
> > >> +controller
> > >> + *
> > >> + * @pp: ptr to pcie port
> > >> + * @np: device node ptr to msi irq controller
> > >> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
> > >> + *
> > >> + * Function register irq domain for msi irq controller and create
> > >> +mappings
> > >> + * for MSI irqs.
> > >> + */
> > >
> > > May be you can only do following to support your MSI chip:
> > >
> > > Initialize pp->irq_domain into your add_pcie_port function before
> > > you call dw_pcie_host_init.
> > >
> > > In dw_pcie_host_init,
> > >
> > > if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
> >
> > How do I pass the keystone specific msi irq_domain_ops? It looks clean
> > the way it is implemented in this patch. Also since MSI host
> > controller and legacy host controller are different, it make sense to
> > add separate host_init for each. Let me know.
> >
> > Regards,
> >
> > Murali
> >
> > >
> > >> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node
> *np,
> > >> + const struct irq_domain_ops *irq_msi_ops) {
> > >> + int i;
> > >>
> > >> if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > >> pp->irq_domain = irq_domain_add_linear(pp->dev-
> >of_node,
> > >> - MAX_MSI_IRQS, &msi_domain_ops,
> > >> + MAX_MSI_IRQS, irq_msi_ops,
> > >> &dw_pcie_msi_chip);
> > >> if (!pp->irq_domain) {
> > >> dev_err(pp->dev, "irq domain init failed\n"); @@ -
> 476,6
> > >> +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> > >> irq_create_mapping(pp->irq_domain, i);
> > >> }
> > >>
> > >> + return 0;
> > >> +}
> > >> +
> > >> +/*
> > >> + * dw_pcie_common_host_init() - common host init function across
> different
> > >> + * versions of the designware PCI
> controller.
> > >> + * @pp: ptr to pcie port
> > >> + * @hw: ptr to hw_pci structure
> > >> + *
> > >> + * This functions parses common DT properties, call host_init()
> > >> +callback
> > >> + * of the PCI controller driver. Also initialize the common RC
> > >> +configurations
> > >> + * and call common pci core function to intialize the controller driver.
> > >> + */
> > >> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct
> > >> +hw_pci *hw) {
> > >> + struct device_node *np = pp->dev->of_node;
> > >> + u32 val;
> > >> +
> > >> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
> > >> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
> > >> + return -EINVAL;
> > >> + }
> > >> +
> > >> if (pp->ops->host_init)
> > >> pp->ops->host_init(pp);
> > >>
> > >> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port
> *pp)
> > >> val |= PORT_LOGIC_SPEED_CHANGE;
> > >> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4,
> val);
> > >>
> > >> - dw_pci.nr_controllers = 1;
> > >> - dw_pci.private_data = (void **)&pp;
> > >> + hw->nr_controllers = 1;
> > >> + hw->private_data = (void **)&pp;
> > >>
> > >> - pci_common_init_dev(pp->dev, &dw_pci);
> > >> + pci_common_init_dev(pp->dev, hw);
> > >> pci_assign_unassigned_resources();
> > >> #ifdef CONFIG_PCI_DOMAINS
> > >> dw_pci.domain++;
> > >> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port
> *pp)
> > >> return 0;
> > >> }
> > >>
> > >> +/*
> > >> + * dw_pcie_host_init() - Host init function for new designware h/w
> > >> + *
> > >> + * @pp: ptr to pcie port
> > >> + *
> > >> + * The function parse the PCI resurces for IO, Memory and map the
> > >> +config
> > >> + * space addresses. Also initliaze the MSI irq controller and call
> > >> + * dw_pcie_common_host_init() to initialize the PCI controller.
> > >> + */
> > >> +int __init dw_pcie_host_init(struct pcie_port *pp) {
> > >> + int ret;
> > >> +
> > >> + ret = dw_pcie_parse_resource(pp);
> > >> + if (ret)
> > >> + return ret;
> > >> +
> > >> + if (!pp->dbi_base) {
> > >> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> > >> + resource_size(&pp->cfg));
> > >> + if (!pp->dbi_base) {
> > >> + dev_err(pp->dev, "error with ioremap\n");
> > >> + return -ENOMEM;
> > >> + }
> > >> + }
> > >> +
> > >> + pp->cfg0_base = pp->cfg.start;
> > >> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
> > >> +
> > >> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
> > >> + pp->config.cfg0_size);
> > >> + if (!pp->va_cfg0_base) {
> > >> + dev_err(pp->dev, "error with ioremap in function\n");
> > >> + return -ENOMEM;
> > >> + }
> > >> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
> > >> + pp->config.cfg1_size);
> > >> + if (!pp->va_cfg1_base) {
> > >> + dev_err(pp->dev, "error with ioremap\n");
> > >> + return -ENOMEM;
> > >> + }
> > >> +
> > >> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node,
> &msi_domain_ops);
> > >> + if (ret < 0)
> > >> + return ret;
> > >> +
> > >> + return dw_pcie_common_host_init(pp, &dw_pci); }
> > >> +
> > >> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32
> busdev)
> > >> {
> > >> /* Program viewport 0 : OUTBOUND : CFG0 */
> > > Regards
> > > Pratyush
> > >
> > >> --
> > >> 1.7.9.5
On Sat, Jun 21, 2014 at 03:05:30AM +0800, Arnd Bergmann wrote:
> On Friday 20 June 2014 13:11:37 Santosh Shilimkar wrote:
> > >
> > Arnd suggestion was to have the version 3.65 code in generic place since
> > its IP specific and just in case some other vendor using the same version
> > can leverage the code.
Sorry, I do not follow PCIe mailing list these days, doing something else
now. So coming to this topic a bit delayed.
> >
> > Concern here seems toe really those name of the files. I can't think of
> > any other appropriate name.
>
> We should definitely keep the version in the DT "compatible" strings
> wherever we know it. Regarding a better file name, I have no idea.
In my opinion, we do not need any of dw-v3_65 files, as code in these
files will not be usable by other vendors.
Anything which is implemented in application space, will not be same
across all IP users. For example, MSI0_IRQ_ENABLE_SET has been defined
at offset 0x108 in keystone PCIe application space.Other vendor may
not have this register at the same offset. Moreover, other vendors are
not even obliged to implement MSI Enable signals in same way, so
internal bit definition of the register may change.
Therefore code is not reusable if all register offset and bit
definitions are not same across vendors. So, in case of DW driver none
of the code which are accessed using va_app_base should go to common
area.
Regards
Pratyush
>
> Arnd
Pratyush,
On 06/23/2014 12:50 PM, Santosh Shilimkar wrote:
>
>
>
> -------- Original Message --------
> Subject: Re: [PATCH v2 0/8] Add Keystone PCIe controller driver
> On Sat, Jun 21, 2014 at 03:05:30AM +0800, Arnd Bergmann wrote:
>> On Friday 20 June 2014 13:11:37 Santosh Shilimkar wrote:
>
>>>>
>>> Arnd suggestion was to have the version 3.65 code in generic place since
>>> its IP specific and just in case some other vendor using the same version
>>> can leverage the code.
>
> Sorry, I do not follow PCIe mailing list these days, doing something else
> now. So coming to this topic a bit delayed.
>
My Apologies for the email format as I mysteriously lost this email and
had to resort to a forwarded email to respond to this.
Let us have the discussion on this thread as I lost the original emails.
>>>
>>> Concern here seems toe really those name of the files. I can't think of
>>> any other appropriate name.
>>
>> We should definitely keep the version in the DT "compatible" strings
>> wherever we know it. Regarding a better file name, I have no idea.
>
> In my opinion, we do not need any of dw-v3_65 files, as code in these
> files will not be usable by other vendors.
>
> Anything which is implemented in application space, will not be same
> across all IP users. For example, MSI0_IRQ_ENABLE_SET has been defined
> at offset 0x108 in keystone PCIe application space.Other vendor may
> not have this register at the same offset. Moreover, other vendors are
> not even obliged to implement MSI Enable signals in same way, so
> internal bit definition of the register may change.
>
> Therefore code is not reusable if all register offset and bit
> definitions are not same across vendors. So, in case of DW driver none
> of the code which are accessed using va_app_base should go to common
> area.
>
I think based on the response far on this issue, it is best to keep
the Application specific code as part of Keystone driver and in
future if there is any driver that has similar application register
implemented. we can refactor the code and re-use.
My V3 will revert back to implementation similar to RFC. Also since this
is individual h/w specific, there is no no need for a compatibility as
well. Will use keystone specific compatibility string for this.
Arnd, hope this is fine. Please respond if you still think a
compatibility string is needed.
Murali
> Pratyush
>
>>
>> Arnd
>
>
Mohit,
On 06/23/2014 12:49 PM, Santosh Shilimkar wrote:
>
>
>
> -------- Original Message --------
> Subject: Re: [PATCH v2 0/8] Add Keystone PCIe controller driver
> Date: Mon, 23 Jun 2014 10:43:46 +0530
> From: Pratyush Anand<[email protected]>
> To: Murali Karicheri<[email protected]>
>
> On Sat, Jun 21, 2014 at 05:17:07AM +0800, Murali Karicheri wrote:
>>
>> Sorry, my previous response was in html and not sure it has made to the
>> list. I did
>> get an error as well. So resending my response.
>>
>> On 6/18/2014 6:14 AM, Mohit KUMAR DCG wrote:
>>> Hello Murali,
>>>
>>
>
> [...]
>
>> *pos = pos0;
>>
>> @@ -349,7 +353,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip,
>> struct pci_dev *pdev,
>>
>> */
>>
>> desc->msi_attrib.multiple = msgvec;
>>
>> -msg.address_lo = virt_to_phys((void *)pp->msi_data);
>>
>> +if (pp->ops->get_msi_data)
>>
>> +msg.address_lo = pp->ops->get_msi_data(pp);
>>
>> +else
>>
>> +msg.address_lo = virt_to_phys((void *)pp->msi_data);
>>
>> msg.address_hi = 0x0;
>>
>> msg.data = pos;
>>
>>
>> What about this code? This requires get_msi_data() as well
>
> pp->msi_data is set in dw_pcie_msi_init, which is a global function
> called from vendor specific code. You can have your own
> keystone_pcie_msi_init and then you do not need above changes.
>
My Apologies for the email format as I lost the original email and had
to respond using a forwarded email.
If you look at my original patch, keystone driver was not calling
dw_pcie_msi_init(). However the issue is msi_chip of DW core is
re-used as most of the code is platform independent. However
code that sends write_msi_msg() can be re-used on keystone by
adding this get_msi_data() API as the only difference IMO is the
address EP writes to raise an MSI IRQ event. I don't see any
reason why the entire code needs to be duplicated on Keystone.
BTW, currently I have not tested the MSI IRQ and tested my driver
only with Legacy IRQ. I plan to test this before sending my
v3 of the patch.
Murali
>>
>>> -- 3rd to use pp->ops->msi_set/clear if defined.
>> Why not API enhancement and refactor the code in a single patch?
>
> Yes, can be. You can send changes in 2 or 3 patches as you wish, but I
> believe that should be able to solve problem in best way.
>
> Regards
> Pratyush
>>
>> Murali
>>> Pls let us know for any issue or have different opinion.
>>>
>>> Regards
>>> Mohit
>>>
>>>
>>>
>>>
>>>> --
>>>> 1.7.9.5
>>
>
>
On 06/24/2014 12:08 PM, Murali Karicheri wrote:
> Pratyush,
>
> On 06/23/2014 12:50 PM, Santosh Shilimkar wrote:
>>
>>
>>
>> -------- Original Message --------
>> Subject: Re: [PATCH v2 0/8] Add Keystone PCIe controller driver
>> On Sat, Jun 21, 2014 at 03:05:30AM +0800, Arnd Bergmann wrote:
>>> On Friday 20 June 2014 13:11:37 Santosh Shilimkar wrote:
>>
>>>>>
>>>> Arnd suggestion was to have the version 3.65 code in generic place
>>>> since
>>>> its IP specific and just in case some other vendor using the same
>>>> version
>>>> can leverage the code.
>>
>> Sorry, I do not follow PCIe mailing list these days, doing something else
>> now. So coming to this topic a bit delayed.
>>
> My Apologies for the email format as I mysteriously lost this email and
> had to resort to a forwarded email to respond to this.
>
> Let us have the discussion on this thread as I lost the original emails.
>>>>
>>>> Concern here seems toe really those name of the files. I can't think of
>>>> any other appropriate name.
>>>
>>> We should definitely keep the version in the DT "compatible" strings
>>> wherever we know it. Regarding a better file name, I have no idea.
>>
>> In my opinion, we do not need any of dw-v3_65 files, as code in these
>> files will not be usable by other vendors.
>>
>> Anything which is implemented in application space, will not be same
>> across all IP users. For example, MSI0_IRQ_ENABLE_SET has been defined
>> at offset 0x108 in keystone PCIe application space.Other vendor may
>> not have this register at the same offset. Moreover, other vendors are
>> not even obliged to implement MSI Enable signals in same way, so
>> internal bit definition of the register may change.
>>
>> Therefore code is not reusable if all register offset and bit
>> definitions are not same across vendors. So, in case of DW driver none
>> of the code which are accessed using va_app_base should go to common
>> area.
>>
>
> I think based on the response far on this issue, it is best to keep
> the Application specific code as part of Keystone driver and in
> future if there is any driver that has similar application register
> implemented. we can refactor the code and re-use.
>
> My V3 will revert back to implementation similar to RFC. Also since this
> is individual h/w specific, there is no no need for a compatibility as
> well. Will use keystone specific compatibility string for this.
>
> Arnd, hope this is fine. Please respond if you still think a
> compatibility string is needed.
On a second thought, I think it is better to keep the compatibility
string to differentiate the h/w and do any old h/w specific initialization.
Thanks and regards,
Murali
>
> Murali
>
>> Pratyush
>>
>>>
>>> Arnd
>>
>>
>
> --
> 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