[PATCH 0/6] pci, thunder: Add Cavium Thunder PCIe host controller

[PATCH 0/6] pci, thunder: Add Cavium Thunder PCIe host controller

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

This patches add support for the Cavium Thunder PCIe host controller.

Patches base on following branches:

 git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci pci/liviu-generic-v11
 git://linux-arm.org/linux-ld for-upstream/pci_arm64_v11

Additionally a fix is required for liviu-generic-v11 to allow the
generation of mem-mapped pci ranges:

 https://lkml.org/lkml/2014/9/22/150

It also requires Marc's gicv3-its support to work.

The first patch introduces the host controller driver. Patch 2 to 4
are DT changes including documentation. Patch 5 and 6 enable PCI
support for arm64 defconfig.

Robert Richter (2):
  arm64, defconfig: Enable PCI
  pci, thunder: Enable Cavium Thunder PCIe host controller

Sunil Goutham (3):
  pci, thunder: Add support for Thunder PCIe host controller
  pci, thunder: Add PCIe host controller devicetree bindings
  pci, thunder: Document PCIe host controller devicetree bindings

Tirumalesh Chalamarla (1):
  GICv3: Add ITS entry to THUNDER dts

 .../devicetree/bindings/pci/cavium,thunder-pci.txt |  32 +++
 arch/arm64/Kconfig                                 |  15 ++
 arch/arm64/boot/dts/thunder-88xx.dtsi              |  58 +++++
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-thunder.c                    | 246 +++++++++++++++++++++
 include/linux/pci_ids.h                            |   2 +
 7 files changed, 362 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
 create mode 100644 drivers/pci/host/pcie-thunder.c

-- 
2.1.0

[PATCH 0/6] pci, thunder: Add Cavium Thunder PCIe host controller

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

This patches add support for the Cavium Thunder PCIe host controller.

Patches base on following branches:

 git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci pci/liviu-generic-v11
 git://linux-arm.org/linux-ld for-upstream/pci_arm64_v11

Additionally a fix is required for liviu-generic-v11 to allow the
generation of mem-mapped pci ranges:

 https://lkml.org/lkml/2014/9/22/150

It also requires Marc's gicv3-its support to work.

The first patch introduces the host controller driver. Patch 2 to 4
are DT changes including documentation. Patch 5 and 6 enable PCI
support for arm64 defconfig.

Robert Richter (2):
  arm64, defconfig: Enable PCI
  pci, thunder: Enable Cavium Thunder PCIe host controller

Sunil Goutham (3):
  pci, thunder: Add support for Thunder PCIe host controller
  pci, thunder: Add PCIe host controller devicetree bindings
  pci, thunder: Document PCIe host controller devicetree bindings

Tirumalesh Chalamarla (1):
  GICv3: Add ITS entry to THUNDER dts

 .../devicetree/bindings/pci/cavium,thunder-pci.txt |  32 +++
 arch/arm64/Kconfig                                 |  15 ++
 arch/arm64/boot/dts/thunder-88xx.dtsi              |  58 +++++
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-thunder.c                    | 246 +++++++++++++++++++++
 include/linux/pci_ids.h                            |   2 +
 7 files changed, 362 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
 create mode 100644 drivers/pci/host/pcie-thunder.c

-- 
2.1.0

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds support for PCI host controller of Cavium Thunder
SoCs.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/pci/host/Kconfig        |   8 ++
 drivers/pci/host/Makefile       |   1 +
 drivers/pci/host/pcie-thunder.c | 246 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h         |   2 +
 4 files changed, 257 insertions(+)
 create mode 100644 drivers/pci/host/pcie-thunder.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 90f5ccacce4b..269c3ff786bc 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -63,4 +63,12 @@ config PCIE_SPEAR13XX
 	help
 	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
 
+config PCI_THUNDER
+	bool "Thunder PCIe host controller"
+	depends on ARM64 || COMPILE_TEST
+	depends on OF_PCI
+	depends on PCI_MSI
+	help
+	  Say Y here if you want internal PCI support on Thunder SoC.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index d0e88f114ff9..fd8041da1719 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
+obj-$(CONFIG_PCI_THUNDER) += pcie-thunder.o
diff --git a/drivers/pci/host/pcie-thunder.c b/drivers/pci/host/pcie-thunder.c
new file mode 100644
index 000000000000..947fad3b1980
--- /dev/null
+++ b/drivers/pci/host/pcie-thunder.c
@@ -0,0 +1,246 @@
+/*
+ * PCIe host controller driver for Cavium Thunder SOC
+ *
+ * Copyright (C) 2014, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/msi.h>
+
+#define PCI_DEVICE_ID_THUNDER_BRIDGE	0xa002
+
+#define THUNDER_PCIE_BUS_SHIFT		20
+#define THUNDER_PCIE_DEV_SHIFT		15
+#define THUNDER_PCIE_FUNC_SHIFT		12
+
+struct thunder_pcie {
+	struct device_node	*node;
+	struct device		*dev;
+	void __iomem		*cfg_base;
+	struct msi_chip		*msi;
+};
+
+/*
+ * This bridge is just for the sake of supporting ARI for
+ * downstream devices. No resources are attached to it.
+ * Copy upstream root bus resources to bridge which aide in
+ * resource claiming for downstream devices
+ */
+static void pci_bridge_resource_fixup(struct pci_dev *dev)
+{
+	struct pci_bus *bus;
+	int resno;
+
+	bus = dev->subordinate;
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) {
+		bus->resource[resno] = pci_bus_resource_n(bus->parent,
+			PCI_BRIDGE_RESOURCE_NUM + resno);
+	}
+
+	for (resno = PCI_BRIDGE_RESOURCES;
+		resno <= PCI_BRIDGE_RESOURCE_END; resno++) {
+		dev->resource[resno].start = dev->resource[resno].end = 0;
+		dev->resource[resno].flags = 0;
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_BRIDGE,
+			pci_bridge_resource_fixup);
+
+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+	struct resource *res;
+	int resno;
+
+	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+		dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+		res = &dev->resource[resno];
+		if (res->parent || !(res->flags & IORESOURCE_MEM))
+			continue;
+		pci_claim_resource(dev, resno);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+			pci_dev_resource_fixup);
+
+static void __iomem *thunder_pcie_cfg_base(struct thunder_pcie *pcie,
+				 unsigned int bus, unsigned int devfn)
+{
+	return  pcie->cfg_base + ((bus << THUNDER_PCIE_BUS_SHIFT)
+		| (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT)
+		| (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT));
+}
+
+static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 *val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		*val = readb(addr);
+		break;
+	case 2:
+		*val = readw(addr);
+		break;
+	case 4:
+		*val = readl(addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int thunder_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		writeb(val, addr);
+		break;
+	case 2:
+		writew(val, addr);
+		break;
+	case 4:
+		writel(val, addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops thunder_pcie_ops = {
+	.read	= thunder_pcie_read_config,
+	.write	= thunder_pcie_write_config,
+};
+
+static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
+					struct pci_bus *bus)
+{
+	struct device_node *msi_node;
+
+	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
+	if (!msi_node)
+		return -ENODEV;
+
+	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
+	if (!pcie->msi)
+		return -ENODEV;
+
+	pcie->msi->dev = pcie->dev;
+	bus->msi = pcie->msi;
+
+	return 0;
+}
+
+static int thunder_pcie_probe(struct platform_device *pdev)
+{
+	struct thunder_pcie *pcie;
+	struct resource *cfg_base;
+	struct pci_bus *bus;
+	int ret;
+	LIST_HEAD(res);
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->node = of_node_get(pdev->dev.of_node);
+	pcie->dev = &pdev->dev;
+
+	/* Get controller's configuration space range */
+	cfg_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->cfg_base = devm_ioremap_resource(&pdev->dev, cfg_base);
+	if (IS_ERR(pcie->cfg_base)) {
+		ret = PTR_ERR(pcie->cfg_base);
+		goto err_ioremap;
+	}
+
+	ret = of_pci_get_host_bridge_resources(pdev->dev.of_node,
+					       0, 255, &res, NULL);
+	if (ret)
+		goto err_get_host;
+
+	bus = pci_create_root_bus(&pdev->dev, 0, &thunder_pcie_ops, pcie, &res);
+	if (!bus) {
+		ret = -ENODEV;
+		goto err_root_bus;
+	}
+
+	/* Set reference to MSI chip */
+	ret = thunder_pcie_msi_enable(pcie, bus);
+	if (ret)
+		goto err_msi;
+
+	platform_set_drvdata(pdev, pcie);
+
+	pci_scan_child_bus(bus);
+	pci_bus_add_devices(bus);
+
+	return 0;
+err_msi:
+	pci_remove_root_bus(bus);
+err_root_bus:
+	pci_free_resource_list(&res);
+err_get_host:
+	devm_ioremap_release(pcie->dev, pcie->cfg_base);
+err_ioremap:
+	of_node_put(pcie->node);
+	kfree(pcie);
+	return ret;
+}
+
+static const struct of_device_id thunder_pcie_of_match[] = {
+	{ .compatible = "cavium,thunder-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, thunder_pcie_of_match);
+
+static struct platform_driver thunder_pcie_driver = {
+	.driver = {
+		.name = "thunder-pcie",
+		.owner = THIS_MODULE,
+		.of_match_table = thunder_pcie_of_match,
+	},
+	.probe = thunder_pcie_probe,
+};
+module_platform_driver(thunder_pcie_driver);
+
+MODULE_AUTHOR("Sunil Goutham");
+MODULE_DESCRIPTION("Cavium Thunder PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6ed0bb73a864..60f16b888c9d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2322,6 +2322,8 @@
 #define PCI_DEVICE_ID_ALTIMA_AC9100	0x03ea
 #define PCI_DEVICE_ID_ALTIMA_AC1003	0x03eb
 
+#define PCI_VENDOR_ID_CAVIUM		0x177d
+
 #define PCI_VENDOR_ID_BELKIN		0x1799
 #define PCI_DEVICE_ID_BELKIN_F5D7010V7	0x701f
 
-- 
2.1.0

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds support for PCI host controller of Cavium Thunder
SoCs.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/pci/host/Kconfig        |   8 ++
 drivers/pci/host/Makefile       |   1 +
 drivers/pci/host/pcie-thunder.c | 246 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h         |   2 +
 4 files changed, 257 insertions(+)
 create mode 100644 drivers/pci/host/pcie-thunder.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 90f5ccacce4b..269c3ff786bc 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -63,4 +63,12 @@ config PCIE_SPEAR13XX
 	help
 	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
 
+config PCI_THUNDER
+	bool "Thunder PCIe host controller"
+	depends on ARM64 || COMPILE_TEST
+	depends on OF_PCI
+	depends on PCI_MSI
+	help
+	  Say Y here if you want internal PCI support on Thunder SoC.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index d0e88f114ff9..fd8041da1719 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
+obj-$(CONFIG_PCI_THUNDER) += pcie-thunder.o
diff --git a/drivers/pci/host/pcie-thunder.c b/drivers/pci/host/pcie-thunder.c
new file mode 100644
index 000000000000..947fad3b1980
--- /dev/null
+++ b/drivers/pci/host/pcie-thunder.c
@@ -0,0 +1,246 @@
+/*
+ * PCIe host controller driver for Cavium Thunder SOC
+ *
+ * Copyright (C) 2014, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/msi.h>
+
+#define PCI_DEVICE_ID_THUNDER_BRIDGE	0xa002
+
+#define THUNDER_PCIE_BUS_SHIFT		20
+#define THUNDER_PCIE_DEV_SHIFT		15
+#define THUNDER_PCIE_FUNC_SHIFT		12
+
+struct thunder_pcie {
+	struct device_node	*node;
+	struct device		*dev;
+	void __iomem		*cfg_base;
+	struct msi_chip		*msi;
+};
+
+/*
+ * This bridge is just for the sake of supporting ARI for
+ * downstream devices. No resources are attached to it.
+ * Copy upstream root bus resources to bridge which aide in
+ * resource claiming for downstream devices
+ */
+static void pci_bridge_resource_fixup(struct pci_dev *dev)
+{
+	struct pci_bus *bus;
+	int resno;
+
+	bus = dev->subordinate;
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) {
+		bus->resource[resno] = pci_bus_resource_n(bus->parent,
+			PCI_BRIDGE_RESOURCE_NUM + resno);
+	}
+
+	for (resno = PCI_BRIDGE_RESOURCES;
+		resno <= PCI_BRIDGE_RESOURCE_END; resno++) {
+		dev->resource[resno].start = dev->resource[resno].end = 0;
+		dev->resource[resno].flags = 0;
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_BRIDGE,
+			pci_bridge_resource_fixup);
+
+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+	struct resource *res;
+	int resno;
+
+	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+		dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+		res = &dev->resource[resno];
+		if (res->parent || !(res->flags & IORESOURCE_MEM))
+			continue;
+		pci_claim_resource(dev, resno);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+			pci_dev_resource_fixup);
+
+static void __iomem *thunder_pcie_cfg_base(struct thunder_pcie *pcie,
+				 unsigned int bus, unsigned int devfn)
+{
+	return  pcie->cfg_base + ((bus << THUNDER_PCIE_BUS_SHIFT)
+		| (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT)
+		| (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT));
+}
+
+static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 *val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		*val = readb(addr);
+		break;
+	case 2:
+		*val = readw(addr);
+		break;
+	case 4:
+		*val = readl(addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int thunder_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		writeb(val, addr);
+		break;
+	case 2:
+		writew(val, addr);
+		break;
+	case 4:
+		writel(val, addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops thunder_pcie_ops = {
+	.read	= thunder_pcie_read_config,
+	.write	= thunder_pcie_write_config,
+};
+
+static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
+					struct pci_bus *bus)
+{
+	struct device_node *msi_node;
+
+	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
+	if (!msi_node)
+		return -ENODEV;
+
+	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
+	if (!pcie->msi)
+		return -ENODEV;
+
+	pcie->msi->dev = pcie->dev;
+	bus->msi = pcie->msi;
+
+	return 0;
+}
+
+static int thunder_pcie_probe(struct platform_device *pdev)
+{
+	struct thunder_pcie *pcie;
+	struct resource *cfg_base;
+	struct pci_bus *bus;
+	int ret;
+	LIST_HEAD(res);
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->node = of_node_get(pdev->dev.of_node);
+	pcie->dev = &pdev->dev;
+
+	/* Get controller's configuration space range */
+	cfg_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->cfg_base = devm_ioremap_resource(&pdev->dev, cfg_base);
+	if (IS_ERR(pcie->cfg_base)) {
+		ret = PTR_ERR(pcie->cfg_base);
+		goto err_ioremap;
+	}
+
+	ret = of_pci_get_host_bridge_resources(pdev->dev.of_node,
+					       0, 255, &res, NULL);
+	if (ret)
+		goto err_get_host;
+
+	bus = pci_create_root_bus(&pdev->dev, 0, &thunder_pcie_ops, pcie, &res);
+	if (!bus) {
+		ret = -ENODEV;
+		goto err_root_bus;
+	}
+
+	/* Set reference to MSI chip */
+	ret = thunder_pcie_msi_enable(pcie, bus);
+	if (ret)
+		goto err_msi;
+
+	platform_set_drvdata(pdev, pcie);
+
+	pci_scan_child_bus(bus);
+	pci_bus_add_devices(bus);
+
+	return 0;
+err_msi:
+	pci_remove_root_bus(bus);
+err_root_bus:
+	pci_free_resource_list(&res);
+err_get_host:
+	devm_ioremap_release(pcie->dev, pcie->cfg_base);
+err_ioremap:
+	of_node_put(pcie->node);
+	kfree(pcie);
+	return ret;
+}
+
+static const struct of_device_id thunder_pcie_of_match[] = {
+	{ .compatible = "cavium,thunder-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, thunder_pcie_of_match);
+
+static struct platform_driver thunder_pcie_driver = {
+	.driver = {
+		.name = "thunder-pcie",
+		.owner = THIS_MODULE,
+		.of_match_table = thunder_pcie_of_match,
+	},
+	.probe = thunder_pcie_probe,
+};
+module_platform_driver(thunder_pcie_driver);
+
+MODULE_AUTHOR("Sunil Goutham");
+MODULE_DESCRIPTION("Cavium Thunder PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6ed0bb73a864..60f16b888c9d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2322,6 +2322,8 @@
 #define PCI_DEVICE_ID_ALTIMA_AC9100	0x03ea
 #define PCI_DEVICE_ID_ALTIMA_AC1003	0x03eb
 
+#define PCI_VENDOR_ID_CAVIUM		0x177d
+
 #define PCI_VENDOR_ID_BELKIN		0x1799
 #define PCI_DEVICE_ID_BELKIN_F5D7010V7	0x701f
 
-- 
2.1.0

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds support for PCI host controller of Cavium Thunder
SoCs.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/pci/host/Kconfig        |   8 ++
 drivers/pci/host/Makefile       |   1 +
 drivers/pci/host/pcie-thunder.c | 246 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h         |   2 +
 4 files changed, 257 insertions(+)
 create mode 100644 drivers/pci/host/pcie-thunder.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 90f5ccacce4b..269c3ff786bc 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -63,4 +63,12 @@ config PCIE_SPEAR13XX
 	help
 	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
 
+config PCI_THUNDER
+	bool "Thunder PCIe host controller"
+	depends on ARM64 || COMPILE_TEST
+	depends on OF_PCI
+	depends on PCI_MSI
+	help
+	  Say Y here if you want internal PCI support on Thunder SoC.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index d0e88f114ff9..fd8041da1719 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
+obj-$(CONFIG_PCI_THUNDER) += pcie-thunder.o
diff --git a/drivers/pci/host/pcie-thunder.c b/drivers/pci/host/pcie-thunder.c
new file mode 100644
index 000000000000..947fad3b1980
--- /dev/null
+++ b/drivers/pci/host/pcie-thunder.c
@@ -0,0 +1,246 @@
+/*
+ * PCIe host controller driver for Cavium Thunder SOC
+ *
+ * Copyright (C) 2014, Cavium Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/of_pci.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/msi.h>
+
+#define PCI_DEVICE_ID_THUNDER_BRIDGE	0xa002
+
+#define THUNDER_PCIE_BUS_SHIFT		20
+#define THUNDER_PCIE_DEV_SHIFT		15
+#define THUNDER_PCIE_FUNC_SHIFT		12
+
+struct thunder_pcie {
+	struct device_node	*node;
+	struct device		*dev;
+	void __iomem		*cfg_base;
+	struct msi_chip		*msi;
+};
+
+/*
+ * This bridge is just for the sake of supporting ARI for
+ * downstream devices. No resources are attached to it.
+ * Copy upstream root bus resources to bridge which aide in
+ * resource claiming for downstream devices
+ */
+static void pci_bridge_resource_fixup(struct pci_dev *dev)
+{
+	struct pci_bus *bus;
+	int resno;
+
+	bus = dev->subordinate;
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCE_NUM; resno++) {
+		bus->resource[resno] = pci_bus_resource_n(bus->parent,
+			PCI_BRIDGE_RESOURCE_NUM + resno);
+	}
+
+	for (resno = PCI_BRIDGE_RESOURCES;
+		resno <= PCI_BRIDGE_RESOURCE_END; resno++) {
+		dev->resource[resno].start = dev->resource[resno].end = 0;
+		dev->resource[resno].flags = 0;
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_DEVICE_ID_THUNDER_BRIDGE,
+			pci_bridge_resource_fixup);
+
+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+	struct resource *res;
+	int resno;
+
+	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+		dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+	for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+		res = &dev->resource[resno];
+		if (res->parent || !(res->flags & IORESOURCE_MEM))
+			continue;
+		pci_claim_resource(dev, resno);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+			pci_dev_resource_fixup);
+
+static void __iomem *thunder_pcie_cfg_base(struct thunder_pcie *pcie,
+				 unsigned int bus, unsigned int devfn)
+{
+	return  pcie->cfg_base + ((bus << THUNDER_PCIE_BUS_SHIFT)
+		| (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT)
+		| (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT));
+}
+
+static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 *val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		*val = readb(addr);
+		break;
+	case 2:
+		*val = readw(addr);
+		break;
+	case 4:
+		*val = readl(addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int thunder_pcie_write_config(struct pci_bus *bus, unsigned int devfn,
+				  int reg, int size, u32 val)
+{
+	struct thunder_pcie *pcie = bus->sysdata;
+	void __iomem *addr;
+	unsigned int busnr = bus->number;
+
+	if (busnr > 255 || devfn > 255 || reg > 4095)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
+
+	switch (size) {
+	case 1:
+		writeb(val, addr);
+		break;
+	case 2:
+		writew(val, addr);
+		break;
+	case 4:
+		writel(val, addr);
+		break;
+	default:
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops thunder_pcie_ops = {
+	.read	= thunder_pcie_read_config,
+	.write	= thunder_pcie_write_config,
+};
+
+static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
+					struct pci_bus *bus)
+{
+	struct device_node *msi_node;
+
+	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
+	if (!msi_node)
+		return -ENODEV;
+
+	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
+	if (!pcie->msi)
+		return -ENODEV;
+
+	pcie->msi->dev = pcie->dev;
+	bus->msi = pcie->msi;
+
+	return 0;
+}
+
+static int thunder_pcie_probe(struct platform_device *pdev)
+{
+	struct thunder_pcie *pcie;
+	struct resource *cfg_base;
+	struct pci_bus *bus;
+	int ret;
+	LIST_HEAD(res);
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->node = of_node_get(pdev->dev.of_node);
+	pcie->dev = &pdev->dev;
+
+	/* Get controller's configuration space range */
+	cfg_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->cfg_base = devm_ioremap_resource(&pdev->dev, cfg_base);
+	if (IS_ERR(pcie->cfg_base)) {
+		ret = PTR_ERR(pcie->cfg_base);
+		goto err_ioremap;
+	}
+
+	ret = of_pci_get_host_bridge_resources(pdev->dev.of_node,
+					       0, 255, &res, NULL);
+	if (ret)
+		goto err_get_host;
+
+	bus = pci_create_root_bus(&pdev->dev, 0, &thunder_pcie_ops, pcie, &res);
+	if (!bus) {
+		ret = -ENODEV;
+		goto err_root_bus;
+	}
+
+	/* Set reference to MSI chip */
+	ret = thunder_pcie_msi_enable(pcie, bus);
+	if (ret)
+		goto err_msi;
+
+	platform_set_drvdata(pdev, pcie);
+
+	pci_scan_child_bus(bus);
+	pci_bus_add_devices(bus);
+
+	return 0;
+err_msi:
+	pci_remove_root_bus(bus);
+err_root_bus:
+	pci_free_resource_list(&res);
+err_get_host:
+	devm_ioremap_release(pcie->dev, pcie->cfg_base);
+err_ioremap:
+	of_node_put(pcie->node);
+	kfree(pcie);
+	return ret;
+}
+
+static const struct of_device_id thunder_pcie_of_match[] = {
+	{ .compatible = "cavium,thunder-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, thunder_pcie_of_match);
+
+static struct platform_driver thunder_pcie_driver = {
+	.driver = {
+		.name = "thunder-pcie",
+		.owner = THIS_MODULE,
+		.of_match_table = thunder_pcie_of_match,
+	},
+	.probe = thunder_pcie_probe,
+};
+module_platform_driver(thunder_pcie_driver);
+
+MODULE_AUTHOR("Sunil Goutham");
+MODULE_DESCRIPTION("Cavium Thunder PCIe host controller driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6ed0bb73a864..60f16b888c9d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2322,6 +2322,8 @@
 #define PCI_DEVICE_ID_ALTIMA_AC9100	0x03ea
 #define PCI_DEVICE_ID_ALTIMA_AC1003	0x03eb
 
+#define PCI_VENDOR_ID_CAVIUM		0x177d
+
 #define PCI_VENDOR_ID_BELKIN		0x1799
 #define PCI_DEVICE_ID_BELKIN_F5D7010V7	0x701f
 
-- 
2.1.0

Re: [PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
> From: Sunil Goutham <sgoutham@cavium.com>
> 
> This patch adds support for PCI host controller of Cavium Thunder
> SoCs.

I had expected this hardware to be SBSA compliant. Why do you need
a hardware specific driver, is this a workaround for buggy hardware
or just noncompliant?

> +/*
> + * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
> + * Also claim the device's valid resources to set 'res->parent' hierarchy.
> + */
> +static void pci_dev_resource_fixup(struct pci_dev *dev)
> +{
> +	struct resource *res;
> +	int resno;
> +
> +	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
> +		dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;

You have listed these as relocatable in DT, why do you have to mark
them as nonrelocatable here?

> +static void __iomem *thunder_pcie_cfg_base(struct thunder_pcie *pcie,
> +				 unsigned int bus, unsigned int devfn)
> +{
> +	return  pcie->cfg_base + ((bus << THUNDER_PCIE_BUS_SHIFT)
> +		| (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT)
> +		| (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT));
> +}
> +
> +static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
> +				  int reg, int size, u32 *val)
> +{
> +	struct thunder_pcie *pcie = bus->sysdata;
> +	void __iomem *addr;
> +	unsigned int busnr = bus->number;
> +
> +	if (busnr > 255 || devfn > 255 || reg > 4095)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
> +
> +	switch (size) {
> +	case 1:
> +		*val = readb(addr);
> +		break;
> +	case 2:
> +		*val = readw(addr);
> +		break;
> +	case 4:
> +		*val = readl(addr);
> +		break;
> +	default:
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
> +	}
> +
> +	return PCIBIOS_SUCCESSFUL;
> +}

This looks roughly ECAM compliant, are you sure you need a
private implementation?

> +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
> +					struct pci_bus *bus)
> +{
> +	struct device_node *msi_node;
> +
> +	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
> +	if (!msi_node)
> +		return -ENODEV;
> +
> +	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> +	if (!pcie->msi)
> +		return -ENODEV;
> +
> +	pcie->msi->dev = pcie->dev;
> +	bus->msi = pcie->msi;
> +
> +	return 0;
> +}

This is probably something we should add to the generic host driver as well,
so it can work with SBSA compliant implementations that come with an MSI
controller. Maybe move it into common code so it can be shared with that
driver.

	Arnd

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
> From: Sunil Goutham <sgoutham@cavium.com>
> 
> This patch adds support for PCI host controller of Cavium Thunder
> SoCs.

I had expected this hardware to be SBSA compliant. Why do you need
a hardware specific driver, is this a workaround for buggy hardware
or just noncompliant?

> +/*
> + * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
> + * Also claim the device's valid resources to set 'res->parent' hierarchy.
> + */
> +static void pci_dev_resource_fixup(struct pci_dev *dev)
> +{
> +	struct resource *res;
> +	int resno;
> +
> +	for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
> +		dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;

You have listed these as relocatable in DT, why do you have to mark
them as nonrelocatable here?

> +static void __iomem *thunder_pcie_cfg_base(struct thunder_pcie *pcie,
> +				 unsigned int bus, unsigned int devfn)
> +{
> +	return  pcie->cfg_base + ((bus << THUNDER_PCIE_BUS_SHIFT)
> +		| (PCI_SLOT(devfn) << THUNDER_PCIE_DEV_SHIFT)
> +		| (PCI_FUNC(devfn) << THUNDER_PCIE_FUNC_SHIFT));
> +}
> +
> +static int thunder_pcie_read_config(struct pci_bus *bus, unsigned int devfn,
> +				  int reg, int size, u32 *val)
> +{
> +	struct thunder_pcie *pcie = bus->sysdata;
> +	void __iomem *addr;
> +	unsigned int busnr = bus->number;
> +
> +	if (busnr > 255 || devfn > 255 || reg > 4095)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	addr = thunder_pcie_cfg_base(pcie, busnr, devfn) + reg;
> +
> +	switch (size) {
> +	case 1:
> +		*val = readb(addr);
> +		break;
> +	case 2:
> +		*val = readw(addr);
> +		break;
> +	case 4:
> +		*val = readl(addr);
> +		break;
> +	default:
> +		return PCIBIOS_BAD_REGISTER_NUMBER;
> +	}
> +
> +	return PCIBIOS_SUCCESSFUL;
> +}

This looks roughly ECAM compliant, are you sure you need a
private implementation?

> +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
> +					struct pci_bus *bus)
> +{
> +	struct device_node *msi_node;
> +
> +	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
> +	if (!msi_node)
> +		return -ENODEV;
> +
> +	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> +	if (!pcie->msi)
> +		return -ENODEV;
> +
> +	pcie->msi->dev = pcie->dev;
> +	bus->msi = pcie->msi;
> +
> +	return 0;
> +}

This is probably something we should add to the generic host driver as well,
so it can work with SBSA compliant implementations that come with an MSI
controller. Maybe move it into common code so it can be shared with that
driver.

	Arnd

Re: [PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Will Deacon]

On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
> > From: Sunil Goutham <sgoutham@cavium.com>
> > 
> > This patch adds support for PCI host controller of Cavium Thunder
> > SoCs.
> 
> I had expected this hardware to be SBSA compliant. Why do you need
> a hardware specific driver, is this a workaround for buggy hardware
> or just noncompliant?

Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
it to Liviu's new API, so do shout if it's not suitable for your needs.

> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
> > +					struct pci_bus *bus)
> > +{
> > +	struct device_node *msi_node;
> > +
> > +	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
> > +	if (!msi_node)
> > +		return -ENODEV;
> > +
> > +	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> > +	if (!pcie->msi)
> > +		return -ENODEV;
> > +
> > +	pcie->msi->dev = pcie->dev;
> > +	bus->msi = pcie->msi;
> > +
> > +	return 0;
> > +}
> 
> This is probably something we should add to the generic host driver as well,
> so it can work with SBSA compliant implementations that come with an MSI
> controller. Maybe move it into common code so it can be shared with that
> driver.

Agreed. I've been carrying something similar [1] (based on a hacked-up version
of bios32, so not bothered to post it) whilst I've been waiting for the
arm64 core PCI code to get merged.

Will

[1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Will Deacon]

On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
> > From: Sunil Goutham <sgoutham@cavium.com>
> > 
> > This patch adds support for PCI host controller of Cavium Thunder
> > SoCs.
> 
> I had expected this hardware to be SBSA compliant. Why do you need
> a hardware specific driver, is this a workaround for buggy hardware
> or just noncompliant?

Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
it to Liviu's new API, so do shout if it's not suitable for your needs.

> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
> > +					struct pci_bus *bus)
> > +{
> > +	struct device_node *msi_node;
> > +
> > +	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
> > +	if (!msi_node)
> > +		return -ENODEV;
> > +
> > +	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> > +	if (!pcie->msi)
> > +		return -ENODEV;
> > +
> > +	pcie->msi->dev = pcie->dev;
> > +	bus->msi = pcie->msi;
> > +
> > +	return 0;
> > +}
> 
> This is probably something we should add to the generic host driver as well,
> so it can work with SBSA compliant implementations that come with an MSI
> controller. Maybe move it into common code so it can be shared with that
> driver.

Agreed. I've been carrying something similar [1] (based on a hacked-up version
of bios32, so not bothered to post it) whilst I've been waiting for the
arm64 core PCI code to get merged.

Will

[1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e

Re: [PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Will Deacon]

On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
> > From: Sunil Goutham <sgoutham@cavium.com>
> > 
> > This patch adds support for PCI host controller of Cavium Thunder
> > SoCs.
> 
> I had expected this hardware to be SBSA compliant. Why do you need
> a hardware specific driver, is this a workaround for buggy hardware
> or just noncompliant?

Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
it to Liviu's new API, so do shout if it's not suitable for your needs.

> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
> > +					struct pci_bus *bus)
> > +{
> > +	struct device_node *msi_node;
> > +
> > +	msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
> > +	if (!msi_node)
> > +		return -ENODEV;
> > +
> > +	pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> > +	if (!pcie->msi)
> > +		return -ENODEV;
> > +
> > +	pcie->msi->dev = pcie->dev;
> > +	bus->msi = pcie->msi;
> > +
> > +	return 0;
> > +}
> 
> This is probably something we should add to the generic host driver as well,
> so it can work with SBSA compliant implementations that come with an MSI
> controller. Maybe move it into common code so it can be shared with that
> driver.

Agreed. I've been carrying something similar [1] (based on a hacked-up version
of bios32, so not bothered to post it) whilst I've been waiting for the
arm64 core PCI code to get merged.

Will

[1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e

Re: [PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Sunil Kovvuri]

Will/Arnd

Thanks for the comments.
There is another patch submitted for adding MSI controller to Generic driver.
https://lkml.org/lkml/2014/9/28/150

Will go through these and comeback.

On Wed, Sep 24, 2014 at 10:19 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
>> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
>> > From: Sunil Goutham <sgoutham@cavium.com>
>> >
>> > This patch adds support for PCI host controller of Cavium Thunder
>> > SoCs.
>>
>> I had expected this hardware to be SBSA compliant. Why do you need
>> a hardware specific driver, is this a workaround for buggy hardware
>> or just noncompliant?
>
> Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
> it to Liviu's new API, so do shout if it's not suitable for your needs.
>
>> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
>> > +                                   struct pci_bus *bus)
>> > +{
>> > +   struct device_node *msi_node;
>> > +
>> > +   msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
>> > +   if (!msi_node)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
>> > +   if (!pcie->msi)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi->dev = pcie->dev;
>> > +   bus->msi = pcie->msi;
>> > +
>> > +   return 0;
>> > +}
>>
>> This is probably something we should add to the generic host driver as well,
>> so it can work with SBSA compliant implementations that come with an MSI
>> controller. Maybe move it into common code so it can be shared with that
>> driver.
>
> Agreed. I've been carrying something similar [1] (based on a hacked-up version
> of bios32, so not bothered to post it) whilst I've been waiting for the
> arm64 core PCI code to get merged.
>
> Will
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Sunil Kovvuri]

Will/Arnd

Thanks for the comments.
There is another patch submitted for adding MSI controller to Generic driver.
https://lkml.org/lkml/2014/9/28/150

Will go through these and comeback.

On Wed, Sep 24, 2014 at 10:19 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
>> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
>> > From: Sunil Goutham <sgoutham@cavium.com>
>> >
>> > This patch adds support for PCI host controller of Cavium Thunder
>> > SoCs.
>>
>> I had expected this hardware to be SBSA compliant. Why do you need
>> a hardware specific driver, is this a workaround for buggy hardware
>> or just noncompliant?
>
> Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
> it to Liviu's new API, so do shout if it's not suitable for your needs.
>
>> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
>> > +                                   struct pci_bus *bus)
>> > +{
>> > +   struct device_node *msi_node;
>> > +
>> > +   msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
>> > +   if (!msi_node)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
>> > +   if (!pcie->msi)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi->dev = pcie->dev;
>> > +   bus->msi = pcie->msi;
>> > +
>> > +   return 0;
>> > +}
>>
>> This is probably something we should add to the generic host driver as well,
>> so it can work with SBSA compliant implementations that come with an MSI
>> controller. Maybe move it into common code so it can be shared with that
>> driver.
>
> Agreed. I've been carrying something similar [1] (based on a hacked-up version
> of bios32, so not bothered to post it) whilst I've been waiting for the
> arm64 core PCI code to get merged.
>
> Will
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller

[Sunil Kovvuri]

Will/Arnd

Thanks for the comments.
There is another patch submitted for adding MSI controller to Generic driver.
https://lkml.org/lkml/2014/9/28/150

Will go through these and comeback.

On Wed, Sep 24, 2014 at 10:19 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Sep 24, 2014 at 05:12:26PM +0100, Arnd Bergmann wrote:
>> On Wednesday 24 September 2014 17:37:43 Robert Richter wrote:
>> > From: Sunil Goutham <sgoutham@cavium.com>
>> >
>> > This patch adds support for PCI host controller of Cavium Thunder
>> > SoCs.
>>
>> I had expected this hardware to be SBSA compliant. Why do you need
>> a hardware specific driver, is this a workaround for buggy hardware
>> or just noncompliant?
>
> Patches welcome to pci-host-generic.c :) Lorenzo already has code to port
> it to Liviu's new API, so do shout if it's not suitable for your needs.
>
>> > +static int thunder_pcie_msi_enable(struct thunder_pcie *pcie,
>> > +                                   struct pci_bus *bus)
>> > +{
>> > +   struct device_node *msi_node;
>> > +
>> > +   msi_node = of_parse_phandle(pcie->node, "msi-parent", 0);
>> > +   if (!msi_node)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
>> > +   if (!pcie->msi)
>> > +           return -ENODEV;
>> > +
>> > +   pcie->msi->dev = pcie->dev;
>> > +   bus->msi = pcie->msi;
>> > +
>> > +   return 0;
>> > +}
>>
>> This is probably something we should add to the generic host driver as well,
>> so it can work with SBSA compliant implementations that come with an MSI
>> controller. Maybe move it into common code so it can be shared with that
>> driver.
>
> Agreed. I've been carrying something similar [1] (based on a hacked-up version
> of bios32, so not bothered to post it) whilst I've been waiting for the
> arm64 core PCI code to get merged.
>
> Will
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/drivers/pci/host/pci-host-generic.c?h=iommu/pci&id=b719acf062ceccfbd79ee7b1ae0b7904ea4da27e
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Robert Richter]

From: Tirumalesh Chalamarla <tchalamarla@cavium.com>

The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
Thunder SoCs by adding an entry to DT.

Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
index d8c0bdc51882..9cb7cf94284a 100644
--- a/arch/arm64/boot/dts/thunder-88xx.dtsi
+++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
@@ -376,10 +376,19 @@
 		gic0: interrupt-controller@8010,00000000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
 			interrupt-controller;
 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
 			interrupts = <1 9 0xf04>;
+
+			its: gic-its@8010,00020000 {
+				compatible = "arm,gic-v3-its";
+				msi-controller;
+				reg = <0x8010 0x20000 0x0 0x200000>;
+			};
 		};
 
 		uaa0: serial@87e0,24000000 {
-- 
2.1.0

[PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Robert Richter]

From: Tirumalesh Chalamarla <tchalamarla@cavium.com>

The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
Thunder SoCs by adding an entry to DT.

Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
index d8c0bdc51882..9cb7cf94284a 100644
--- a/arch/arm64/boot/dts/thunder-88xx.dtsi
+++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
@@ -376,10 +376,19 @@
 		gic0: interrupt-controller at 8010,00000000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
 			interrupt-controller;
 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
 			interrupts = <1 9 0xf04>;
+
+			its: gic-its at 8010,00020000 {
+				compatible = "arm,gic-v3-its";
+				msi-controller;
+				reg = <0x8010 0x20000 0x0 0x200000>;
+			};
 		};
 
 		uaa0: serial at 87e0,24000000 {
-- 
2.1.0

[PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Robert Richter]

From: Tirumalesh Chalamarla <tchalamarla@cavium.com>

The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
Thunder SoCs by adding an entry to DT.

Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
index d8c0bdc51882..9cb7cf94284a 100644
--- a/arch/arm64/boot/dts/thunder-88xx.dtsi
+++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
@@ -376,10 +376,19 @@
 		gic0: interrupt-controller@8010,00000000 {
 			compatible = "arm,gic-v3";
 			#interrupt-cells = <3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
 			interrupt-controller;
 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
 			interrupts = <1 9 0xf04>;
+
+			its: gic-its@8010,00020000 {
+				compatible = "arm,gic-v3-its";
+				msi-controller;
+				reg = <0x8010 0x20000 0x0 0x200000>;
+			};
 		};
 
 		uaa0: serial@87e0,24000000 {
-- 
2.1.0

Re: [PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Chalamarla, Tirumalesh]

Hi Marc,

Any feed back on this. 
Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

Thanks,
Tirumalesh. 

> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
> 
> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> 
> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
> Thunder SoCs by adding an entry to DT.
> 
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
> index d8c0bdc51882..9cb7cf94284a 100644
> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
> @@ -376,10 +376,19 @@
> 		gic0: interrupt-controller@8010,00000000 {
> 			compatible = "arm,gic-v3";
> 			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> 			interrupt-controller;
> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
> 			interrupts = <1 9 0xf04>;
> +
> +			its: gic-its@8010,00020000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				reg = <0x8010 0x20000 0x0 0x200000>;
> +			};
> 		};
> 
> 		uaa0: serial@87e0,24000000 {
> -- 
> 2.1.0
> 

[PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Chalamarla, Tirumalesh]

Hi Marc,

Any feed back on this. 
Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

Thanks,
Tirumalesh. 

> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
> 
> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> 
> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
> Thunder SoCs by adding an entry to DT.
> 
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
> index d8c0bdc51882..9cb7cf94284a 100644
> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
> @@ -376,10 +376,19 @@
> 		gic0: interrupt-controller at 8010,00000000 {
> 			compatible = "arm,gic-v3";
> 			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> 			interrupt-controller;
> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
> 			interrupts = <1 9 0xf04>;
> +
> +			its: gic-its at 8010,00020000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				reg = <0x8010 0x20000 0x0 0x200000>;
> +			};
> 		};
> 
> 		uaa0: serial at 87e0,24000000 {
> -- 
> 2.1.0
> 

Re: [PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Chalamarla, Tirumalesh]

Hi Marc,

Any feed back on this. 
Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

Thanks,
Tirumalesh. 

> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
> 
> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> 
> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
> Thunder SoCs by adding an entry to DT.
> 
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
> index d8c0bdc51882..9cb7cf94284a 100644
> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
> @@ -376,10 +376,19 @@
> 		gic0: interrupt-controller@8010,00000000 {
> 			compatible = "arm,gic-v3";
> 			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> 			interrupt-controller;
> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
> 			interrupts = <1 9 0xf04>;
> +
> +			its: gic-its@8010,00020000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				reg = <0x8010 0x20000 0x0 0x200000>;
> +			};
> 		};
> 
> 		uaa0: serial@87e0,24000000 {
> -- 
> 2.1.0
> 

Re: [PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Chalamarla, Tirumalesh]

Hi Marc,

Any feed back on this. 
Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

Thanks,
Tirumalesh. 

> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
> 
> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> 
> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
> Thunder SoCs by adding an entry to DT.
> 
> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
> index d8c0bdc51882..9cb7cf94284a 100644
> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
> @@ -376,10 +376,19 @@
> 		gic0: interrupt-controller@8010,00000000 {
> 			compatible = "arm,gic-v3";
> 			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> 			interrupt-controller;
> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
> 			interrupts = <1 9 0xf04>;
> +
> +			its: gic-its@8010,00020000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				reg = <0x8010 0x20000 0x0 0x200000>;
> +			};
> 		};
> 
> 		uaa0: serial@87e0,24000000 {
> -- 
> 2.1.0
> 

Re: [PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Marc Zyngier]

On 26/06/15 00:19, Chalamarla, Tirumalesh wrote:
> Hi Marc,
> 
> Any feed back on this. 
> Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

It looks good to me, so you can add my Acked-by on it.

I think you should repost it and get it merged through arm-soc.

Thanks,

	M.

> Thanks,
> Tirumalesh. 
> 
>> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
>>
>> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>>
>> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
>> Thunder SoCs by adding an entry to DT.
>>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>> Signed-off-by: Robert Richter <rrichter@cavium.com>
>> ---
>> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> index d8c0bdc51882..9cb7cf94284a 100644
>> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
>> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> @@ -376,10 +376,19 @@
>> 		gic0: interrupt-controller@8010,00000000 {
>> 			compatible = "arm,gic-v3";
>> 			#interrupt-cells = <3>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> 			interrupt-controller;
>> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
>> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
>> 			interrupts = <1 9 0xf04>;
>> +
>> +			its: gic-its@8010,00020000 {
>> +				compatible = "arm,gic-v3-its";
>> +				msi-controller;
>> +				reg = <0x8010 0x20000 0x0 0x200000>;
>> +			};
>> 		};
>>
>> 		uaa0: serial@87e0,24000000 {
>> -- 
>> 2.1.0
>>
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Marc Zyngier]

On 26/06/15 00:19, Chalamarla, Tirumalesh wrote:
> Hi Marc,
> 
> Any feed back on this. 
> Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

It looks good to me, so you can add my Acked-by on it.

I think you should repost it and get it merged through arm-soc.

Thanks,

	M.

> Thanks,
> Tirumalesh. 
> 
>> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
>>
>> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>>
>> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
>> Thunder SoCs by adding an entry to DT.
>>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>> Signed-off-by: Robert Richter <rrichter@cavium.com>
>> ---
>> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> index d8c0bdc51882..9cb7cf94284a 100644
>> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
>> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> @@ -376,10 +376,19 @@
>> 		gic0: interrupt-controller at 8010,00000000 {
>> 			compatible = "arm,gic-v3";
>> 			#interrupt-cells = <3>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> 			interrupt-controller;
>> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
>> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
>> 			interrupts = <1 9 0xf04>;
>> +
>> +			its: gic-its at 8010,00020000 {
>> +				compatible = "arm,gic-v3-its";
>> +				msi-controller;
>> +				reg = <0x8010 0x20000 0x0 0x200000>;
>> +			};
>> 		};
>>
>> 		uaa0: serial at 87e0,24000000 {
>> -- 
>> 2.1.0
>>
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/6] GICv3: Add ITS entry to THUNDER dts

[Marc Zyngier]

On 26/06/15 00:19, Chalamarla, Tirumalesh wrote:
> Hi Marc,
> 
> Any feed back on this. 
> Do you want me to submit this as a separate patch?so that it is easy for getting acceptance. 

It looks good to me, so you can add my Acked-by on it.

I think you should repost it and get it merged through arm-soc.

Thanks,

	M.

> Thanks,
> Tirumalesh. 
> 
>> On Sep 24, 2014, at 8:37 AM, Robert Richter <rric@kernel.org> wrote:
>>
>> From: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>>
>> The PCIe host controller uses MSIs provided by GICv3 ITS. Enable it on
>> Thunder SoCs by adding an entry to DT.
>>
>> Signed-off-by: Tirumalesh Chalamarla <tchalamarla@cavium.com>
>> Signed-off-by: Robert Richter <rrichter@cavium.com>
>> ---
>> arch/arm64/boot/dts/thunder-88xx.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> index d8c0bdc51882..9cb7cf94284a 100644
>> --- a/arch/arm64/boot/dts/thunder-88xx.dtsi
>> +++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
>> @@ -376,10 +376,19 @@
>> 		gic0: interrupt-controller@8010,00000000 {
>> 			compatible = "arm,gic-v3";
>> 			#interrupt-cells = <3>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> 			interrupt-controller;
>> 			reg = <0x8010 0x00000000 0x0 0x010000>, /* GICD */
>> 			      <0x8010 0x80000000 0x0 0x600000>; /* GICR */
>> 			interrupts = <1 9 0xf04>;
>> +
>> +			its: gic-its@8010,00020000 {
>> +				compatible = "arm,gic-v3-its";
>> +				msi-controller;
>> +				reg = <0x8010 0x20000 0x0 0x200000>;
>> +			};
>> 		};
>>
>> 		uaa0: serial@87e0,24000000 {
>> -- 
>> 2.1.0
>>
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds the PCIe host controller entry for Cavium Thunder SoCs
to the devicetree. There are 4 internal PCI controllers available.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/boot/dts/thunder-88xx.dtsi | 49 +++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
index 9cb7cf94284a..0b433b0e7af4 100644
--- a/arch/arm64/boot/dts/thunder-88xx.dtsi
+++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
@@ -407,4 +407,53 @@
 			clock-names = "apb_pclk";
 		};
 	};
+
+	pcie0@0x8480,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
+			<0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
+			<0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
+        };
+
+	pcie1@0x8490,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x8490 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8310 0x00000000 0x8310 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8100 0x00000000 0x8100 0x00000000 0x80 0x00000000>;
+        };
+
+	pcie2@0x84a0,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x84a0 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8320 0x00000000 0x8320 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8430 0x00000000 0x8430 0x00000000 0x01 0x00000000>;
+        };
+
+	pcie3@0x84b0,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x84b0 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8330 0x00000000 0x8330 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>;
+        };
 };
-- 
2.1.0

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds the PCIe host controller entry for Cavium Thunder SoCs
to the devicetree. There are 4 internal PCI controllers available.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/boot/dts/thunder-88xx.dtsi | 49 +++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/thunder-88xx.dtsi b/arch/arm64/boot/dts/thunder-88xx.dtsi
index 9cb7cf94284a..0b433b0e7af4 100644
--- a/arch/arm64/boot/dts/thunder-88xx.dtsi
+++ b/arch/arm64/boot/dts/thunder-88xx.dtsi
@@ -407,4 +407,53 @@
 			clock-names = "apb_pclk";
 		};
 	};
+
+	pcie0 at 0x8480,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
+			<0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
+			<0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
+        };
+
+	pcie1 at 0x8490,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x8490 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8310 0x00000000 0x8310 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8100 0x00000000 0x8100 0x00000000 0x80 0x00000000>;
+        };
+
+	pcie2 at 0x84a0,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x84a0 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8320 0x00000000 0x8320 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8430 0x00000000 0x8430 0x00000000 0x01 0x00000000>;
+        };
+
+	pcie3 at 0x84b0,00000000 {
+	        compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		reg = <0x84b0 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8330 0x00000000 0x8330 0x00000000 0x00 0x10000000>, /* mem ranges */
+			<0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>;
+        };
 };
-- 
2.1.0

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> 
> +       pcie0@0x8480,00000000 {

The name should be pci, not pci0.

> +               compatible = "cavium,thunder-pcie";
> +               device_type = "pci";
> +               msi-parent = <&its>;
> +               bus-range = <0 255>;
> +               #size-cells = <2>;
> +               #address-cells = <3>;
> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> +        };

If you claim the entire 0-255 bus range, I think you should also
specify a domain, otherwise it's not predictable which domain you
get.

The interrupt-map and interrupt-map-mask properties are required for PCI,
otherwise you can't do LSI interrupts.

If your hardware can support it, you should also list I/O space and prefetchable
memory spaces. Can you explain why you have multiple non-prefetchable ranges?

	Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> 
> +       pcie0 at 0x8480,00000000 {

The name should be pci, not pci0.

> +               compatible = "cavium,thunder-pcie";
> +               device_type = "pci";
> +               msi-parent = <&its>;
> +               bus-range = <0 255>;
> +               #size-cells = <2>;
> +               #address-cells = <3>;
> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> +        };

If you claim the entire 0-255 bus range, I think you should also
specify a domain, otherwise it's not predictable which domain you
get.

The interrupt-map and interrupt-map-mask properties are required for PCI,
otherwise you can't do LSI interrupts.

If your hardware can support it, you should also list I/O space and prefetchable
memory spaces. Can you explain why you have multiple non-prefetchable ranges?

	Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>
>> +       pcie0@0x8480,00000000 {
>
> The name should be pci, not pci0.

Thanks, will change.

>
>> +               compatible = "cavium,thunder-pcie";
>> +               device_type = "pci";
>> +               msi-parent = <&its>;
>> +               bus-range = <0 255>;
>> +               #size-cells = <2>;
>> +               #address-cells = <3>;
>> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
>> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
>> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
>> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
>> +        };
>
> If you claim the entire 0-255 bus range, I think you should also
> specify a domain, otherwise it's not predictable which domain you
> get.
>
> The interrupt-map and interrupt-map-mask properties are required for PCI,
> otherwise you can't do LSI interrupts.

This PCI controller supports only MSIx interrupts which are edge triggered.

>
> If your hardware can support it, you should also list I/O space and prefetchable
> memory spaces. Can you explain why you have multiple non-prefetchable ranges?

Our hardware is an ECAM based host controller and doesn't support I/O
and prefetchable memory spaces.
All on-board PCI devices connected to this PCI controller have fixed resources
and doesn't have to be allocated/reassigned. Some of these devices are
SRIOV based.
Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
to be set, otherwise doesn't
enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
aid in resource claiming and setting res->parent hierarchy.

We do call "pci_claim_resource" in controller driver code.
"[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."

>
>         Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>
>> +       pcie0 at 0x8480,00000000 {
>
> The name should be pci, not pci0.

Thanks, will change.

>
>> +               compatible = "cavium,thunder-pcie";
>> +               device_type = "pci";
>> +               msi-parent = <&its>;
>> +               bus-range = <0 255>;
>> +               #size-cells = <2>;
>> +               #address-cells = <3>;
>> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
>> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
>> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
>> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
>> +        };
>
> If you claim the entire 0-255 bus range, I think you should also
> specify a domain, otherwise it's not predictable which domain you
> get.
>
> The interrupt-map and interrupt-map-mask properties are required for PCI,
> otherwise you can't do LSI interrupts.

This PCI controller supports only MSIx interrupts which are edge triggered.

>
> If your hardware can support it, you should also list I/O space and prefetchable
> memory spaces. Can you explain why you have multiple non-prefetchable ranges?

Our hardware is an ECAM based host controller and doesn't support I/O
and prefetchable memory spaces.
All on-board PCI devices connected to this PCI controller have fixed resources
and doesn't have to be allocated/reassigned. Some of these devices are
SRIOV based.
Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
to be set, otherwise doesn't
enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
aid in resource claiming and setting res->parent hierarchy.

We do call "pci_claim_resource" in controller driver code.
"[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."

>
>         Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> >> +               compatible = "cavium,thunder-pcie";
> >> +               device_type = "pci";
> >> +               msi-parent = <&its>;
> >> +               bus-range = <0 255>;
> >> +               #size-cells = <2>;
> >> +               #address-cells = <3>;
> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> >> +        };
> >
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> >
> > The interrupt-map and interrupt-map-mask properties are required for PCI,
> > otherwise you can't do LSI interrupts.
> 
> This PCI controller supports only MSIx interrupts which are edge triggered.

Interesting, so it's not PCI compliant then? I assume this will be fixed
in the production version of the silicon, right?

Having no support for interrupts mean that the majority of PCI device drivers
will fail.

> > If your hardware can support it, you should also list I/O space and prefetchable
> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
> 
> Our hardware is an ECAM based host controller and doesn't support I/O
> and prefetchable memory spaces.
> All on-board PCI devices connected to this PCI controller have fixed resources
> and doesn't have to be allocated/reassigned. Some of these devices are
> SRIOV based.

I think you need to mark the ones that are nonrelocatable with flag
0x80000000, otherwise the PCI core might decide to reassign them.

> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
> to be set, otherwise doesn't
> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
> aid in resource claiming and setting res->parent hierarchy.

I don't understand. Isn't that just a bug in the code that you are working
around with the DT. Have you tried fixing the code instead?

> We do call "pci_claim_resource" in controller driver code.
> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."

My guess is that you are using the wrong interface here. Isn't the normal
request_resource() in the host driver enough?

	Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> >> +               compatible = "cavium,thunder-pcie";
> >> +               device_type = "pci";
> >> +               msi-parent = <&its>;
> >> +               bus-range = <0 255>;
> >> +               #size-cells = <2>;
> >> +               #address-cells = <3>;
> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> >> +        };
> >
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> >
> > The interrupt-map and interrupt-map-mask properties are required for PCI,
> > otherwise you can't do LSI interrupts.
> 
> This PCI controller supports only MSIx interrupts which are edge triggered.

Interesting, so it's not PCI compliant then? I assume this will be fixed
in the production version of the silicon, right?

Having no support for interrupts mean that the majority of PCI device drivers
will fail.

> > If your hardware can support it, you should also list I/O space and prefetchable
> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
> 
> Our hardware is an ECAM based host controller and doesn't support I/O
> and prefetchable memory spaces.
> All on-board PCI devices connected to this PCI controller have fixed resources
> and doesn't have to be allocated/reassigned. Some of these devices are
> SRIOV based.

I think you need to mark the ones that are nonrelocatable with flag
0x80000000, otherwise the PCI core might decide to reassign them.

> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
> to be set, otherwise doesn't
> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
> aid in resource claiming and setting res->parent hierarchy.

I don't understand. Isn't that just a bug in the code that you are working
around with the DT. Have you tried fixing the code instead?

> We do call "pci_claim_resource" in controller driver code.
> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."

My guess is that you are using the wrong interface here. Isn't the normal
request_resource() in the host driver enough?

	Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> >> +               compatible = "cavium,thunder-pcie";
> >> +               device_type = "pci";
> >> +               msi-parent = <&its>;
> >> +               bus-range = <0 255>;
> >> +               #size-cells = <2>;
> >> +               #address-cells = <3>;
> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> >> +        };
> >
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> >
> > The interrupt-map and interrupt-map-mask properties are required for PCI,
> > otherwise you can't do LSI interrupts.
> 
> This PCI controller supports only MSIx interrupts which are edge triggered.

Interesting, so it's not PCI compliant then? I assume this will be fixed
in the production version of the silicon, right?

Having no support for interrupts mean that the majority of PCI device drivers
will fail.

> > If your hardware can support it, you should also list I/O space and prefetchable
> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
> 
> Our hardware is an ECAM based host controller and doesn't support I/O
> and prefetchable memory spaces.
> All on-board PCI devices connected to this PCI controller have fixed resources
> and doesn't have to be allocated/reassigned. Some of these devices are
> SRIOV based.

I think you need to mark the ones that are nonrelocatable with flag
0x80000000, otherwise the PCI core might decide to reassign them.

> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
> to be set, otherwise doesn't
> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
> aid in resource claiming and setting res->parent hierarchy.

I don't understand. Isn't that just a bug in the code that you are working
around with the DT. Have you tried fixing the code instead?

> We do call "pci_claim_resource" in controller driver code.
> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."

My guess is that you are using the wrong interface here. Isn't the normal
request_resource() in the host driver enough?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
>> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>> >> +               compatible = "cavium,thunder-pcie";
>> >> +               device_type = "pci";
>> >> +               msi-parent = <&its>;
>> >> +               bus-range = <0 255>;
>> >> +               #size-cells = <2>;
>> >> +               #address-cells = <3>;
>> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
>> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
>> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
>> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
>> >> +        };
>> >
>> > If you claim the entire 0-255 bus range, I think you should also
>> > specify a domain, otherwise it's not predictable which domain you
>> > get.
>> >
>> > The interrupt-map and interrupt-map-mask properties are required for PCI,
>> > otherwise you can't do LSI interrupts.
>>
>> This PCI controller supports only MSIx interrupts which are edge triggered.
>
> Interesting, so it's not PCI compliant then? I assume this will be fixed
> in the production version of the silicon, right?
>
> Having no support for interrupts mean that the majority of PCI device drivers
> will fail.

This controller is for on-board PCI devices and all of them do support
MSIx interrupts.

>
>> > If your hardware can support it, you should also list I/O space and prefetchable
>> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
>>
>> Our hardware is an ECAM based host controller and doesn't support I/O
>> and prefetchable memory spaces.
>> All on-board PCI devices connected to this PCI controller have fixed resources
>> and doesn't have to be allocated/reassigned. Some of these devices are
>> SRIOV based.
>
> I think you need to mark the ones that are nonrelocatable with flag
> 0x80000000, otherwise the PCI core might decide to reassign them.

Is this flag part of DT pci node properties ?
I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
the same series.

>
>> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
>> to be set, otherwise doesn't
>> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
>> aid in resource claiming and setting res->parent hierarchy.
>
> I don't understand. Isn't that just a bug in the code that you are working
> around with the DT. Have you tried fixing the code instead?

I tried but wasn't sure if its going to impact existing SRIOV devices.
Will have a deeper look again .

>
>> We do call "pci_claim_resource" in controller driver code.
>> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."
>
> My guess is that you are using the wrong interface here. Isn't the normal
> request_resource() in the host driver enough?

Isn't that host driver calls "request_resource" only for resources of root port.
i.e requesting from iomem_resource/ioport_resource. Here i am referring
to SRIOV devices enumerated upon scan of root port.

>
>         Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
>> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>> >> +               compatible = "cavium,thunder-pcie";
>> >> +               device_type = "pci";
>> >> +               msi-parent = <&its>;
>> >> +               bus-range = <0 255>;
>> >> +               #size-cells = <2>;
>> >> +               #address-cells = <3>;
>> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
>> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
>> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
>> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
>> >> +        };
>> >
>> > If you claim the entire 0-255 bus range, I think you should also
>> > specify a domain, otherwise it's not predictable which domain you
>> > get.
>> >
>> > The interrupt-map and interrupt-map-mask properties are required for PCI,
>> > otherwise you can't do LSI interrupts.
>>
>> This PCI controller supports only MSIx interrupts which are edge triggered.
>
> Interesting, so it's not PCI compliant then? I assume this will be fixed
> in the production version of the silicon, right?
>
> Having no support for interrupts mean that the majority of PCI device drivers
> will fail.

This controller is for on-board PCI devices and all of them do support
MSIx interrupts.

>
>> > If your hardware can support it, you should also list I/O space and prefetchable
>> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
>>
>> Our hardware is an ECAM based host controller and doesn't support I/O
>> and prefetchable memory spaces.
>> All on-board PCI devices connected to this PCI controller have fixed resources
>> and doesn't have to be allocated/reassigned. Some of these devices are
>> SRIOV based.
>
> I think you need to mark the ones that are nonrelocatable with flag
> 0x80000000, otherwise the PCI core might decide to reassign them.

Is this flag part of DT pci node properties ?
I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
the same series.

>
>> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
>> to be set, otherwise doesn't
>> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
>> aid in resource claiming and setting res->parent hierarchy.
>
> I don't understand. Isn't that just a bug in the code that you are working
> around with the DT. Have you tried fixing the code instead?

I tried but wasn't sure if its going to impact existing SRIOV devices.
Will have a deeper look again .

>
>> We do call "pci_claim_resource" in controller driver code.
>> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."
>
> My guess is that you are using the wrong interface here. Isn't the normal
> request_resource() in the host driver enough?

Isn't that host driver calls "request_resource" only for resources of root port.
i.e requesting from iomem_resource/ioport_resource. Here i am referring
to SRIOV devices enumerated upon scan of root port.

>
>         Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> >> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> >> >> +               compatible = "cavium,thunder-pcie";
> >> >> +               device_type = "pci";
> >> >> +               msi-parent = <&its>;
> >> >> +               bus-range = <0 255>;
> >> >> +               #size-cells = <2>;
> >> >> +               #address-cells = <3>;
> >> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> >> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> >> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> >> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> >> >> +        };
> >> >
> >> > If you claim the entire 0-255 bus range, I think you should also
> >> > specify a domain, otherwise it's not predictable which domain you
> >> > get.
> >> >
> >> > The interrupt-map and interrupt-map-mask properties are required for PCI,
> >> > otherwise you can't do LSI interrupts.
> >>
> >> This PCI controller supports only MSIx interrupts which are edge triggered.
> >
> > Interesting, so it's not PCI compliant then? I assume this will be fixed
> > in the production version of the silicon, right?
> >
> > Having no support for interrupts mean that the majority of PCI device drivers
> > will fail.
> 
> This controller is for on-board PCI devices and all of them do support
> MSIx interrupts.

But in general, booting with "pci=nomsi" should still work, at least
for debugging purposes. It's hard to believe the chip designers forgot
to connect the one wire.

You said that the PCI host is SBSA compliant, so it must according to
section 8.4 have the legacy interrupts wired up to SPI lines in the GIC.

> >> > If your hardware can support it, you should also list I/O space and prefetchable
> >> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
> >>
> >> Our hardware is an ECAM based host controller and doesn't support I/O
> >> and prefetchable memory spaces.
> >> All on-board PCI devices connected to this PCI controller have fixed resources
> >> and doesn't have to be allocated/reassigned. Some of these devices are
> >> SRIOV based.
> >
> > I think you need to mark the ones that are nonrelocatable with flag
> > 0x80000000, otherwise the PCI core might decide to reassign them.
> 
> Is this flag part of DT pci node properties ?
> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
> the same series.

Ah, right. I checked the source code again and it seems that we don't handle
this right at the moment. I think a range that has the nonrelocatable
flag set should be used for IORESOURCE_PCI_FIXED mappings without any
host specific code, but that needs to be implemented in common code.

> >> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
> >> to be set, otherwise doesn't
> >> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
> >> aid in resource claiming and setting res->parent hierarchy.
> >
> > I don't understand. Isn't that just a bug in the code that you are working
> > around with the DT. Have you tried fixing the code instead?
> 
> I tried but wasn't sure if its going to impact existing SRIOV devices.
> Will have a deeper look again .

IIRC others have reported problems in the same area before, and that
turned out to be a resource that was not registered correctly. What
do you see in /proc/iomem without your workaround?

> >> We do call "pci_claim_resource" in controller driver code.
> >> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."
> >
> > My guess is that you are using the wrong interface here. Isn't the normal
> > request_resource() in the host driver enough?
> 
> Isn't that host driver calls "request_resource" only for resources of root port.
> i.e requesting from iomem_resource/ioport_resource. Here i am referring
> to SRIOV devices enumerated upon scan of root port.

The device resources should be nested within the bus resources, as you would 
e.g. find on my PC system:

dc000000-dcffffff : PCI Bus 0000:40
  dcc00000-dcefffff : PCI Bus 0000:41
    dcce0000-dccfffff : 0000:41:00.0
    dcd00000-dcefffff : PCI Bus 0000:42
      dcd00000-dcdfffff : PCI Bus 0000:45
        dcdef800-dcdeffff : 0000:45:00.0
          dcdef800-dcdeffff : ahci

	Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> >> On Wed, Sep 24, 2014 at 9:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
> >> >> +               compatible = "cavium,thunder-pcie";
> >> >> +               device_type = "pci";
> >> >> +               msi-parent = <&its>;
> >> >> +               bus-range = <0 255>;
> >> >> +               #size-cells = <2>;
> >> >> +               #address-cells = <3>;
> >> >> +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> >> >> +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> >> >> +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> >> >> +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> >> >> +        };
> >> >
> >> > If you claim the entire 0-255 bus range, I think you should also
> >> > specify a domain, otherwise it's not predictable which domain you
> >> > get.
> >> >
> >> > The interrupt-map and interrupt-map-mask properties are required for PCI,
> >> > otherwise you can't do LSI interrupts.
> >>
> >> This PCI controller supports only MSIx interrupts which are edge triggered.
> >
> > Interesting, so it's not PCI compliant then? I assume this will be fixed
> > in the production version of the silicon, right?
> >
> > Having no support for interrupts mean that the majority of PCI device drivers
> > will fail.
> 
> This controller is for on-board PCI devices and all of them do support
> MSIx interrupts.

But in general, booting with "pci=nomsi" should still work, at least
for debugging purposes. It's hard to believe the chip designers forgot
to connect the one wire.

You said that the PCI host is SBSA compliant, so it must according to
section 8.4 have the legacy interrupts wired up to SPI lines in the GIC.

> >> > If your hardware can support it, you should also list I/O space and prefetchable
> >> > memory spaces. Can you explain why you have multiple non-prefetchable ranges?
> >>
> >> Our hardware is an ECAM based host controller and doesn't support I/O
> >> and prefetchable memory spaces.
> >> All on-board PCI devices connected to this PCI controller have fixed resources
> >> and doesn't have to be allocated/reassigned. Some of these devices are
> >> SRIOV based.
> >
> > I think you need to mark the ones that are nonrelocatable with flag
> > 0x80000000, otherwise the PCI core might decide to reassign them.
> 
> Is this flag part of DT pci node properties ?
> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
> the same series.

Ah, right. I checked the source code again and it seems that we don't handle
this right at the moment. I think a range that has the nonrelocatable
flag set should be used for IORESOURCE_PCI_FIXED mappings without any
host specific code, but that needs to be implemented in common code.

> >> Kernel's SRIOV (pci/iov.c) is expecting 'resource->parent' hierarchy
> >> to be set, otherwise doesn't
> >> enable SRIOV device. So, here multiple non-prefetchable ranges of root bus
> >> aid in resource claiming and setting res->parent hierarchy.
> >
> > I don't understand. Isn't that just a bug in the code that you are working
> > around with the DT. Have you tried fixing the code instead?
> 
> I tried but wasn't sure if its going to impact existing SRIOV devices.
> Will have a deeper look again .

IIRC others have reported problems in the same area before, and that
turned out to be a resource that was not registered correctly. What
do you see in /proc/iomem without your workaround?

> >> We do call "pci_claim_resource" in controller driver code.
> >> "[PATCH 1/6] pci, thunder: Add support for Thunder PCIe host controller."
> >
> > My guess is that you are using the wrong interface here. Isn't the normal
> > request_resource() in the host driver enough?
> 
> Isn't that host driver calls "request_resource" only for resources of root port.
> i.e requesting from iomem_resource/ioport_resource. Here i am referring
> to SRIOV devices enumerated upon scan of root port.

The device resources should be nested within the bus resources, as you would 
e.g. find on my PC system:

dc000000-dcffffff : PCI Bus 0000:40
  dcc00000-dcefffff : PCI Bus 0000:41
    dcce0000-dccfffff : 0000:41:00.0
    dcd00000-dcefffff : PCI Bus 0000:42
      dcd00000-dcdfffff : PCI Bus 0000:45
        dcdef800-dcdeffff : 0000:45:00.0
          dcdef800-dcdeffff : ahci

	Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
>> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:

>> >> All on-board PCI devices connected to this PCI controller have fixed resources
>> >> and doesn't have to be allocated/reassigned. Some of these devices are
>> >> SRIOV based.
>> >
>> > I think you need to mark the ones that are nonrelocatable with flag
>> > 0x80000000, otherwise the PCI core might decide to reassign them.
>>
>> Is this flag part of DT pci node properties ?
>> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
>> the same series.
>
> Ah, right. I checked the source code again and it seems that we don't handle
> this right at the moment. I think a range that has the nonrelocatable
> flag set should be used for IORESOURCE_PCI_FIXED mappings without any
> host specific code, but that needs to be implemented in common code.

What connection do you envision between nonrelocatable ranges and
IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
not try to assign a different address, e.g., because the BAR is
read-only or because it's a legacy IDE/VGA/etc. range for which there
is no BAR at all.

Bjorn

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
>> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:

>> >> All on-board PCI devices connected to this PCI controller have fixed resources
>> >> and doesn't have to be allocated/reassigned. Some of these devices are
>> >> SRIOV based.
>> >
>> > I think you need to mark the ones that are nonrelocatable with flag
>> > 0x80000000, otherwise the PCI core might decide to reassign them.
>>
>> Is this flag part of DT pci node properties ?
>> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
>> the same series.
>
> Ah, right. I checked the source code again and it seems that we don't handle
> this right at the moment. I think a range that has the nonrelocatable
> flag set should be used for IORESOURCE_PCI_FIXED mappings without any
> host specific code, but that needs to be implemented in common code.

What connection do you envision between nonrelocatable ranges and
IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
not try to assign a different address, e.g., because the BAR is
read-only or because it's a legacy IDE/VGA/etc. range for which there
is no BAR at all.

Bjorn

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014, Bjorn Helgaas wrote:
> On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
> >> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> 
> >> >> All on-board PCI devices connected to this PCI controller have fixed resources
> >> >> and doesn't have to be allocated/reassigned. Some of these devices are
> >> >> SRIOV based.
> >> >
> >> > I think you need to mark the ones that are nonrelocatable with flag
> >> > 0x80000000, otherwise the PCI core might decide to reassign them.
> >>
> >> Is this flag part of DT pci node properties ?
> >> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
> >> the same series.
> >
> > Ah, right. I checked the source code again and it seems that we don't handle
> > this right at the moment. I think a range that has the nonrelocatable
> > flag set should be used for IORESOURCE_PCI_FIXED mappings without any
> > host specific code, but that needs to be implemented in common code.
> 
> What connection do you envision between nonrelocatable ranges and
> IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
> but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
> not try to assign a different address, e.g., because the BAR is
> read-only or because it's a legacy IDE/VGA/etc. range for which there
> is no BAR at all.

I think that is exactly the definition of the nonrelocatable flag
in http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf

The example given in section 11.1.2 is for a VGA device that has some
relocatable BARs and some nonrelocatable BARs.

	Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014, Bjorn Helgaas wrote:
> On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
> >> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
> 
> >> >> All on-board PCI devices connected to this PCI controller have fixed resources
> >> >> and doesn't have to be allocated/reassigned. Some of these devices are
> >> >> SRIOV based.
> >> >
> >> > I think you need to mark the ones that are nonrelocatable with flag
> >> > 0x80000000, otherwise the PCI core might decide to reassign them.
> >>
> >> Is this flag part of DT pci node properties ?
> >> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
> >> the same series.
> >
> > Ah, right. I checked the source code again and it seems that we don't handle
> > this right at the moment. I think a range that has the nonrelocatable
> > flag set should be used for IORESOURCE_PCI_FIXED mappings without any
> > host specific code, but that needs to be implemented in common code.
> 
> What connection do you envision between nonrelocatable ranges and
> IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
> but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
> not try to assign a different address, e.g., because the BAR is
> read-only or because it's a legacy IDE/VGA/etc. range for which there
> is no BAR at all.

I think that is exactly the definition of the nonrelocatable flag
in http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf

The example given in section 11.1.2 is for a VGA device that has some
relocatable BARs and some nonrelocatable BARs.

	Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 1:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014, Bjorn Helgaas wrote:
>> On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
>> >> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
>>
>> >> >> All on-board PCI devices connected to this PCI controller have fixed resources
>> >> >> and doesn't have to be allocated/reassigned. Some of these devices are
>> >> >> SRIOV based.
>> >> >
>> >> > I think you need to mark the ones that are nonrelocatable with flag
>> >> > 0x80000000, otherwise the PCI core might decide to reassign them.
>> >>
>> >> Is this flag part of DT pci node properties ?
>> >> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
>> >> the same series.
>> >
>> > Ah, right. I checked the source code again and it seems that we don't handle
>> > this right at the moment. I think a range that has the nonrelocatable
>> > flag set should be used for IORESOURCE_PCI_FIXED mappings without any
>> > host specific code, but that needs to be implemented in common code.
>>
>> What connection do you envision between nonrelocatable ranges and
>> IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
>> but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
>> not try to assign a different address, e.g., because the BAR is
>> read-only or because it's a legacy IDE/VGA/etc. range for which there
>> is no BAR at all.
>
> I think that is exactly the definition of the nonrelocatable flag
> in http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf
>
> The example given in section 11.1.2 is for a VGA device that has some
> relocatable BARs and some nonrelocatable BARs.

OK.  You said "a range that has the nonrelocatable flag set should be
used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
range was a bridge window, and somehow PCI_FIXED BARs should be put in
that window.

But maybe you meant that nonrelocatable ranges should have the
IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
mean we couldn't move the window, but we could put relocatable BARs
inside the window.

What needs to be implemented?  Just the code that would set
IORESOURCE_PCI_FIXED for nonrelocatable ranges?

Bjorn

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 1:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014, Bjorn Helgaas wrote:
>> On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
>> >> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
>>
>> >> >> All on-board PCI devices connected to this PCI controller have fixed resources
>> >> >> and doesn't have to be allocated/reassigned. Some of these devices are
>> >> >> SRIOV based.
>> >> >
>> >> > I think you need to mark the ones that are nonrelocatable with flag
>> >> > 0x80000000, otherwise the PCI core might decide to reassign them.
>> >>
>> >> Is this flag part of DT pci node properties ?
>> >> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
>> >> the same series.
>> >
>> > Ah, right. I checked the source code again and it seems that we don't handle
>> > this right at the moment. I think a range that has the nonrelocatable
>> > flag set should be used for IORESOURCE_PCI_FIXED mappings without any
>> > host specific code, but that needs to be implemented in common code.
>>
>> What connection do you envision between nonrelocatable ranges and
>> IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
>> but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
>> not try to assign a different address, e.g., because the BAR is
>> read-only or because it's a legacy IDE/VGA/etc. range for which there
>> is no BAR at all.
>
> I think that is exactly the definition of the nonrelocatable flag
> in http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf
>
> The example given in section 11.1.2 is for a VGA device that has some
> relocatable BARs and some nonrelocatable BARs.

OK.  You said "a range that has the nonrelocatable flag set should be
used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
range was a bridge window, and somehow PCI_FIXED BARs should be put in
that window.

But maybe you meant that nonrelocatable ranges should have the
IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
mean we couldn't move the window, but we could put relocatable BARs
inside the window.

What needs to be implemented?  Just the code that would set
IORESOURCE_PCI_FIXED for nonrelocatable ranges?

Bjorn

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 1:26 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 25 September 2014, Bjorn Helgaas wrote:
>> On Thu, Sep 25, 2014 at 1:31 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > On Thursday 25 September 2014 00:37:00 Sunil Kovvuri wrote:
>> >> On Thu, Sep 25, 2014 at 12:04 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> >> > On Wednesday 24 September 2014 23:34:04 Sunil Kovvuri wrote:
>>
>> >> >> All on-board PCI devices connected to this PCI controller have fixed resources
>> >> >> and doesn't have to be allocated/reassigned. Some of these devices are
>> >> >> SRIOV based.
>> >> >
>> >> > I think you need to mark the ones that are nonrelocatable with flag
>> >> > 0x80000000, otherwise the PCI core might decide to reassign them.
>> >>
>> >> Is this flag part of DT pci node properties ?
>> >> I am using IORESOURCE_PCI_FIXED flag. Its there in other patches of
>> >> the same series.
>> >
>> > Ah, right. I checked the source code again and it seems that we don't handle
>> > this right at the moment. I think a range that has the nonrelocatable
>> > flag set should be used for IORESOURCE_PCI_FIXED mappings without any
>> > host specific code, but that needs to be implemented in common code.
>>
>> What connection do you envision between nonrelocatable ranges and
>> IORESOURCE_PCI_FIXED?  I don't know what a nonrelocatable range is,
>> but for IORESOURCE_PCI_FIXED, all I intend is that the PCI core should
>> not try to assign a different address, e.g., because the BAR is
>> read-only or because it's a legacy IDE/VGA/etc. range for which there
>> is no BAR at all.
>
> I think that is exactly the definition of the nonrelocatable flag
> in http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf
>
> The example given in section 11.1.2 is for a VGA device that has some
> relocatable BARs and some nonrelocatable BARs.

OK.  You said "a range that has the nonrelocatable flag set should be
used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
range was a bridge window, and somehow PCI_FIXED BARs should be put in
that window.

But maybe you meant that nonrelocatable ranges should have the
IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
mean we couldn't move the window, but we could put relocatable BARs
inside the window.

What needs to be implemented?  Just the code that would set
IORESOURCE_PCI_FIXED for nonrelocatable ranges?

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014, Bjorn Helgaas wrote:
> OK.  You said "a range that has the nonrelocatable flag set should be
> used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
> range was a bridge window, and somehow PCI_FIXED BARs should be put in
> that window.
> 
> But maybe you meant that nonrelocatable ranges should have the
> IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
> mean we couldn't move the window, but we could put relocatable BARs
> inside the window.
> 
> What needs to be implemented?  Just the code that would set
> IORESOURCE_PCI_FIXED for nonrelocatable ranges?

It might be more complex than I thought. Let's see what the original
hack does:

+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+       struct resource *res;
+       int resno;
+
+       for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+               dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+       for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+               res = &dev->resource[resno];
+               if (res->parent || !(res->flags & IORESOURCE_MEM))
+                       continue;
+               pci_claim_resource(dev, resno);
+       }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+                       pci_dev_resource_fixup);

This actually looks harmful, because it means that any kernel that contains
the thunder host controller driver will do the above for any device made
by Cavium, whether it's connected to this bridge or not.

What I think we want instead is to mark any resource whose parent
resource is IORESOURCE_PCI_FIXED to have the same flag, and mark
the PCI host controller resources IORESOURCE_PCI_FIXED when the
nonrelocatable flag is set, and that should all be done in core
code, not in a driver fixup.

The part that still looks weird is the pci_claim_resource() that
Sunil mentioned. This is currently done for resources that do not
have a parent, but AFAICT all PCI device resources should have a
parent that connects it to the upstream bridge.

	Arnd

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014, Bjorn Helgaas wrote:
> OK.  You said "a range that has the nonrelocatable flag set should be
> used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
> range was a bridge window, and somehow PCI_FIXED BARs should be put in
> that window.
> 
> But maybe you meant that nonrelocatable ranges should have the
> IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
> mean we couldn't move the window, but we could put relocatable BARs
> inside the window.
> 
> What needs to be implemented?  Just the code that would set
> IORESOURCE_PCI_FIXED for nonrelocatable ranges?

It might be more complex than I thought. Let's see what the original
hack does:

+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+       struct resource *res;
+       int resno;
+
+       for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+               dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+       for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+               res = &dev->resource[resno];
+               if (res->parent || !(res->flags & IORESOURCE_MEM))
+                       continue;
+               pci_claim_resource(dev, resno);
+       }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+                       pci_dev_resource_fixup);

This actually looks harmful, because it means that any kernel that contains
the thunder host controller driver will do the above for any device made
by Cavium, whether it's connected to this bridge or not.

What I think we want instead is to mark any resource whose parent
resource is IORESOURCE_PCI_FIXED to have the same flag, and mark
the PCI host controller resources IORESOURCE_PCI_FIXED when the
nonrelocatable flag is set, and that should all be done in core
code, not in a driver fixup.

The part that still looks weird is the pci_claim_resource() that
Sunil mentioned. This is currently done for resources that do not
have a parent, but AFAICT all PCI device resources should have a
parent that connects it to the upstream bridge.

	Arnd

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Arnd Bergmann]

On Thursday 25 September 2014, Bjorn Helgaas wrote:
> OK.  You said "a range that has the nonrelocatable flag set should be
> used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
> range was a bridge window, and somehow PCI_FIXED BARs should be put in
> that window.
> 
> But maybe you meant that nonrelocatable ranges should have the
> IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
> mean we couldn't move the window, but we could put relocatable BARs
> inside the window.
> 
> What needs to be implemented?  Just the code that would set
> IORESOURCE_PCI_FIXED for nonrelocatable ranges?

It might be more complex than I thought. Let's see what the original
hack does:

+/*
+ * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
+ * Also claim the device's valid resources to set 'res->parent' hierarchy.
+ */
+static void pci_dev_resource_fixup(struct pci_dev *dev)
+{
+       struct resource *res;
+       int resno;
+
+       for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
+               dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
+
+       for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
+               res = &dev->resource[resno];
+               if (res->parent || !(res->flags & IORESOURCE_MEM))
+                       continue;
+               pci_claim_resource(dev, resno);
+       }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
+                       pci_dev_resource_fixup);

This actually looks harmful, because it means that any kernel that contains
the thunder host controller driver will do the above for any device made
by Cavium, whether it's connected to this bridge or not.

What I think we want instead is to mark any resource whose parent
resource is IORESOURCE_PCI_FIXED to have the same flag, and mark
the PCI host controller resources IORESOURCE_PCI_FIXED when the
nonrelocatable flag is set, and that should all be done in core
code, not in a driver fixup.

The part that still looks weird is the pci_claim_resource() that
Sunil mentioned. This is currently done for resources that do not
have a parent, but AFAICT all PCI device resources should have a
parent that connects it to the upstream bridge.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 2:22 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014, Bjorn Helgaas wrote:
>> OK.  You said "a range that has the nonrelocatable flag set should be
>> used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
>> range was a bridge window, and somehow PCI_FIXED BARs should be put in
>> that window.
>>
>> But maybe you meant that nonrelocatable ranges should have the
>> IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
>> mean we couldn't move the window, but we could put relocatable BARs
>> inside the window.
>>
>> What needs to be implemented?  Just the code that would set
>> IORESOURCE_PCI_FIXED for nonrelocatable ranges?
>
> It might be more complex than I thought. Let's see what the original
> hack does:
>
> +/*
> + * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
> + * Also claim the device's valid resources to set 'res->parent' hierarchy.
> + */
> +static void pci_dev_resource_fixup(struct pci_dev *dev)
> +{
> +       struct resource *res;
> +       int resno;
> +
> +       for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
> +               dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
> +
> +       for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
> +               res = &dev->resource[resno];
> +               if (res->parent || !(res->flags & IORESOURCE_MEM))
> +                       continue;
> +               pci_claim_resource(dev, resno);
> +       }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
> +                       pci_dev_resource_fixup);
>
> This actually looks harmful, because it means that any kernel that contains
> the thunder host controller driver will do the above for any device made
> by Cavium, whether it's connected to this bridge or not.

I agree, that looks broken.

> What I think we want instead is to mark any resource whose parent
> resource is IORESOURCE_PCI_FIXED to have the same flag, and mark
> the PCI host controller resources IORESOURCE_PCI_FIXED when the
> nonrelocatable flag is set, and that should all be done in core
> code, not in a driver fixup.

The PCI host controller resources are host bridge windows, right?  I
don't think it matters whether they have IORESOURCE_PCI_FIXED set,
because the bridge is not itself a PCI device, and PCI resource
assignment treats the host bridge windows as fixed.

That doesn't imply that the PCI device resources *inside* the windows
need to be fixed, though.  Regular BARs can be moved around inside the
window.

Sunil said "All on-board PCI devices connected to this PCI controller
have fixed resources..."  That sounds like these on-board PCI devices
are non-compliant because their BARs don't work per spec.  But that
doesn't sound right, because we wouldn't be able to size them.

> The part that still looks weird is the pci_claim_resource() that
> Sunil mentioned. This is currently done for resources that do not
> have a parent, but AFAICT all PCI device resources should have a
> parent that connects it to the upstream bridge.

Ideally the pci_claim_resource() would be in the core, not in arch
code, but it's a bit of a hodge-podge right now.

Bjorn

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Bjorn Helgaas]

On Thu, Sep 25, 2014 at 2:22 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 25 September 2014, Bjorn Helgaas wrote:
>> OK.  You said "a range that has the nonrelocatable flag set should be
>> used for IORESOURCE_PCI_FIXED mappings."  I thought you meant that the
>> range was a bridge window, and somehow PCI_FIXED BARs should be put in
>> that window.
>>
>> But maybe you meant that nonrelocatable ranges should have the
>> IORESOURCE_PCI_FIXED bit set in their struct resources.  That would
>> mean we couldn't move the window, but we could put relocatable BARs
>> inside the window.
>>
>> What needs to be implemented?  Just the code that would set
>> IORESOURCE_PCI_FIXED for nonrelocatable ranges?
>
> It might be more complex than I thought. Let's see what the original
> hack does:
>
> +/*
> + * All PCIe devices in Thunder have fixed resources, shouldn't be reassigned.
> + * Also claim the device's valid resources to set 'res->parent' hierarchy.
> + */
> +static void pci_dev_resource_fixup(struct pci_dev *dev)
> +{
> +       struct resource *res;
> +       int resno;
> +
> +       for (resno = 0; resno < PCI_NUM_RESOURCES; resno++)
> +               dev->resource[resno].flags |= IORESOURCE_PCI_FIXED;
> +
> +       for (resno = 0; resno < PCI_BRIDGE_RESOURCES; resno++) {
> +               res = &dev->resource[resno];
> +               if (res->parent || !(res->flags & IORESOURCE_MEM))
> +                       continue;
> +               pci_claim_resource(dev, resno);
> +       }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CAVIUM, PCI_ANY_ID,
> +                       pci_dev_resource_fixup);
>
> This actually looks harmful, because it means that any kernel that contains
> the thunder host controller driver will do the above for any device made
> by Cavium, whether it's connected to this bridge or not.

I agree, that looks broken.

> What I think we want instead is to mark any resource whose parent
> resource is IORESOURCE_PCI_FIXED to have the same flag, and mark
> the PCI host controller resources IORESOURCE_PCI_FIXED when the
> nonrelocatable flag is set, and that should all be done in core
> code, not in a driver fixup.

The PCI host controller resources are host bridge windows, right?  I
don't think it matters whether they have IORESOURCE_PCI_FIXED set,
because the bridge is not itself a PCI device, and PCI resource
assignment treats the host bridge windows as fixed.

That doesn't imply that the PCI device resources *inside* the windows
need to be fixed, though.  Regular BARs can be moved around inside the
window.

Sunil said "All on-board PCI devices connected to this PCI controller
have fixed resources..."  That sounds like these on-board PCI devices
are non-compliant because their BARs don't work per spec.  But that
doesn't sound right, because we wouldn't be able to size them.

> The part that still looks weird is the pci_claim_resource() that
> Sunil mentioned. This is currently done for resources that do not
> have a parent, but AFAICT all PCI device resources should have a
> parent that connects it to the upstream bridge.

Ideally the pci_claim_resource() would be in the core, not in arch
code, but it's a bit of a hodge-podge right now.

Bjorn

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Rob Herring]

On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>
>> +       pcie0@0x8480,00000000 {
>
> The name should be pci, not pci0.

And the address should be "@848000000000". There was some confusion
about when the comma should be used and it is only if you have things
like PCI bus, device, function for the addressing.


>
>> +               compatible = "cavium,thunder-pcie";

A bit too generic as many of the Cavium bindings have been.

Rob

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Rob Herring]

On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>
>> +       pcie0 at 0x8480,00000000 {
>
> The name should be pci, not pci0.

And the address should be "@848000000000". There was some confusion
about when the comma should be used and it is only if you have things
like PCI bus, device, function for the addressing.


>
>> +               compatible = "cavium,thunder-pcie";

A bit too generic as many of the Cavium bindings have been.

Rob

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Rob Herring]

On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>
>> +       pcie0@0x8480,00000000 {
>
> The name should be pci, not pci0.

And the address should be "@848000000000". There was some confusion
about when the comma should be used and it is only if you have things
like PCI bus, device, function for the addressing.


>
>> +               compatible = "cavium,thunder-pcie";

A bit too generic as many of the Cavium bindings have been.

Rob

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

Thanks for pointing correction ROB.
Will fix it.

On Fri, Sep 26, 2014 at 11:56 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>>
>>> +       pcie0@0x8480,00000000 {
>>
>> The name should be pci, not pci0.
>
> And the address should be "@848000000000". There was some confusion
> about when the comma should be used and it is only if you have things
> like PCI bus, device, function for the addressing.
>
>
>>
>>> +               compatible = "cavium,thunder-pcie";
>
> A bit too generic as many of the Cavium bindings have been.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

Thanks for pointing correction ROB.
Will fix it.

On Fri, Sep 26, 2014 at 11:56 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>>
>>> +       pcie0 at 0x8480,00000000 {
>>
>> The name should be pci, not pci0.
>
> And the address should be "@848000000000". There was some confusion
> about when the comma should be used and it is only if you have things
> like PCI bus, device, function for the addressing.
>
>
>>
>>> +               compatible = "cavium,thunder-pcie";
>
> A bit too generic as many of the Cavium bindings have been.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Sunil Kovvuri]

Thanks for pointing correction ROB.
Will fix it.

On Fri, Sep 26, 2014 at 11:56 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Sep 24, 2014 at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday 24 September 2014 17:37:45 Robert Richter wrote:
>>>
>>> +       pcie0@0x8480,00000000 {
>>
>> The name should be pci, not pci0.
>
> And the address should be "@848000000000". There was some confusion
> about when the comma should be used and it is only if you have things
> like PCI bus, device, function for the addressing.
>
>
>>
>>> +               compatible = "cavium,thunder-pcie";
>
> A bit too generic as many of the Cavium bindings have been.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > +               compatible = "cavium,thunder-pcie";
> > +               device_type = "pci";
> > +               msi-parent = <&its>;
> > +               bus-range = <0 255>;
> > +               #size-cells = <2>;
> > +               #address-cells = <3>;
> > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > +        };
> 
> If you claim the entire 0-255 bus range, I think you should also
> specify a domain, otherwise it's not predictable which domain you
> get.

Liviu's code assigns a unique id to the domain if missing, see
of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
property here.

Liviu's DT implementation that assigns a unique number differs a bit
from ACPI which states: "If _SEG [aka domain number] does not exist,
OSPM assumes that all PCI bus segments are in PCI Segment Group 0."

Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
multiple root bridges, the "pci-domain" property could be forced
instead.

-Robert

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > +               compatible = "cavium,thunder-pcie";
> > +               device_type = "pci";
> > +               msi-parent = <&its>;
> > +               bus-range = <0 255>;
> > +               #size-cells = <2>;
> > +               #address-cells = <3>;
> > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > +        };
> 
> If you claim the entire 0-255 bus range, I think you should also
> specify a domain, otherwise it's not predictable which domain you
> get.

Liviu's code assigns a unique id to the domain if missing, see
of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
property here.

Liviu's DT implementation that assigns a unique number differs a bit
from ACPI which states: "If _SEG [aka domain number] does not exist,
OSPM assumes that all PCI bus segments are in PCI Segment Group 0."

Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
multiple root bridges, the "pci-domain" property could be forced
instead.

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > +               compatible = "cavium,thunder-pcie";
> > +               device_type = "pci";
> > +               msi-parent = <&its>;
> > +               bus-range = <0 255>;
> > +               #size-cells = <2>;
> > +               #address-cells = <3>;
> > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > +        };
> 
> If you claim the entire 0-255 bus range, I think you should also
> specify a domain, otherwise it's not predictable which domain you
> get.

Liviu's code assigns a unique id to the domain if missing, see
of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
property here.

Liviu's DT implementation that assigns a unique number differs a bit
from ACPI which states: "If _SEG [aka domain number] does not exist,
OSPM assumes that all PCI bus segments are in PCI Segment Group 0."

Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
multiple root bridges, the "pci-domain" property could be forced
instead.

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > +               compatible = "cavium,thunder-pcie";
> > > +               device_type = "pci";
> > > +               msi-parent = <&its>;
> > > +               bus-range = <0 255>;
> > > +               #size-cells = <2>;
> > > +               #address-cells = <3>;
> > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > +        };
> > 
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> 
> Liviu's code assigns a unique id to the domain if missing, see
> of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> property here.

Not anymore! That function is gone in v12 onwards. What is in -next has
a new function called of_get_pci_domain_nr() (slight name change) but
that only gets the value set in the "linux,pci-domain" property of the
device node. It is the choice of the host bridge driver to use that
function or to use pci_get_new_domain_nr() which *does* generate an
unique id every time it gets called.

> 
> Liviu's DT implementation that assigns a unique number differs a bit
> from ACPI which states: "If _SEG [aka domain number] does not exist,
> OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> 
> Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> multiple root bridges, the "pci-domain" property could be forced
> instead.

Indeed. But the enforcing is left as an exercise to the host bridge
implementor for the moment.

Best regards,
Liviu

> 
> -Robert
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > +               compatible = "cavium,thunder-pcie";
> > > +               device_type = "pci";
> > > +               msi-parent = <&its>;
> > > +               bus-range = <0 255>;
> > > +               #size-cells = <2>;
> > > +               #address-cells = <3>;
> > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > +        };
> > 
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> 
> Liviu's code assigns a unique id to the domain if missing, see
> of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> property here.

Not anymore! That function is gone in v12 onwards. What is in -next has
a new function called of_get_pci_domain_nr() (slight name change) but
that only gets the value set in the "linux,pci-domain" property of the
device node. It is the choice of the host bridge driver to use that
function or to use pci_get_new_domain_nr() which *does* generate an
unique id every time it gets called.

> 
> Liviu's DT implementation that assigns a unique number differs a bit
> from ACPI which states: "If _SEG [aka domain number] does not exist,
> OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> 
> Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> multiple root bridges, the "pci-domain" property could be forced
> instead.

Indeed. But the enforcing is left as an exercise to the host bridge
implementor for the moment.

Best regards,
Liviu

> 
> -Robert
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > +               compatible = "cavium,thunder-pcie";
> > > +               device_type = "pci";
> > > +               msi-parent = <&its>;
> > > +               bus-range = <0 255>;
> > > +               #size-cells = <2>;
> > > +               #address-cells = <3>;
> > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > +        };
> > 
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> 
> Liviu's code assigns a unique id to the domain if missing, see
> of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> property here.

Not anymore! That function is gone in v12 onwards. What is in -next has
a new function called of_get_pci_domain_nr() (slight name change) but
that only gets the value set in the "linux,pci-domain" property of the
device node. It is the choice of the host bridge driver to use that
function or to use pci_get_new_domain_nr() which *does* generate an
unique id every time it gets called.

> 
> Liviu's DT implementation that assigns a unique number differs a bit
> from ACPI which states: "If _SEG [aka domain number] does not exist,
> OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> 
> Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> multiple root bridges, the "pci-domain" property could be forced
> instead.

Indeed. But the enforcing is left as an exercise to the host bridge
implementor for the moment.

Best regards,
Liviu

> 
> -Robert
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > +               compatible = "cavium,thunder-pcie";
> > > +               device_type = "pci";
> > > +               msi-parent = <&its>;
> > > +               bus-range = <0 255>;
> > > +               #size-cells = <2>;
> > > +               #address-cells = <3>;
> > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > +        };
> > 
> > If you claim the entire 0-255 bus range, I think you should also
> > specify a domain, otherwise it's not predictable which domain you
> > get.
> 
> Liviu's code assigns a unique id to the domain if missing, see
> of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> property here.

Not anymore! That function is gone in v12 onwards. What is in -next has
a new function called of_get_pci_domain_nr() (slight name change) but
that only gets the value set in the "linux,pci-domain" property of the
device node. It is the choice of the host bridge driver to use that
function or to use pci_get_new_domain_nr() which *does* generate an
unique id every time it gets called.

> 
> Liviu's DT implementation that assigns a unique number differs a bit
> from ACPI which states: "If _SEG [aka domain number] does not exist,
> OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> 
> Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> multiple root bridges, the "pci-domain" property could be forced
> instead.

Indeed. But the enforcing is left as an exercise to the host bridge
implementor for the moment.

Best regards,
Liviu

> 
> -Robert
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 07.10.14 16:01:49, Liviu Dudau wrote:
> On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > +               compatible = "cavium,thunder-pcie";
> > > > +               device_type = "pci";
> > > > +               msi-parent = <&its>;
> > > > +               bus-range = <0 255>;
> > > > +               #size-cells = <2>;
> > > > +               #address-cells = <3>;
> > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > +        };
> > > 
> > > If you claim the entire 0-255 bus range, I think you should also
> > > specify a domain, otherwise it's not predictable which domain you
> > > get.
> > 
> > Liviu's code assigns a unique id to the domain if missing, see
> > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > property here.
> 
> Not anymore! That function is gone in v12 onwards. What is in -next has
> a new function called of_get_pci_domain_nr() (slight name change) but
> that only gets the value set in the "linux,pci-domain" property of the
> device node. It is the choice of the host bridge driver to use that
> function or to use pci_get_new_domain_nr() which *does* generate an
> unique id every time it gets called.

I am quite confused a bit on which is the latest code base now. I was
looking into Bjorn's pci/host-generic and there is a different
implemetation:

----
/**
 * This function will try to obtain the host bridge domain number by
 * using of_alias_get_id() call with "pci-domain" as a stem.  If that
 * fails, a local allocator will be used.  The local allocator can
 * be requested to return a new domain_nr if the information is missing
 * from the device tree.
 *
 * @node: device tree node with the domain information
 * @allocate_if_missing: if DT lacks information about the domain nr,
 * allocate a new number.
 *
 * Returns the associated domain number from DT, or a new domain number
 * if DT information is missing and @allocate_if_missing is true.  If
 * @allocate_if_missing is false then the last allocated domain number
 * will be returned.
 */
int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
{
        int domain;

        domain = atomic_read(&of_domain_nr);
        if (domain == -1) {
                /* first run, get max defined domain nr in device tree */
                domain = of_get_max_pci_domain_nr();
                /* then set the start value for allocator to be max + 1 */
                atomic_set(&of_domain_nr, domain + 1);
        }
        domain = of_alias_get_id(node, "pci-domain");
        if (domain == -ENODEV) {
                domain = atomic_read(&of_domain_nr);
                if (allocate_if_missing)
                        atomic_inc(&of_domain_nr);
        }

        return domain;
}
EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
----

This differs also from v13. Please check.

It would be good to have a stable code base to work with, so I would
prefer incremental patches on top of Bjorn's pci/host-generic.

> > Liviu's DT implementation that assigns a unique number differs a bit
> > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > 
> > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > multiple root bridges, the "pci-domain" property could be forced
> > instead.
> 
> Indeed. But the enforcing is left as an exercise to the host bridge
> implementor for the moment.

Right, can be added later.

Thanks,

-Robert

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 07.10.14 16:01:49, Liviu Dudau wrote:
> On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > +               compatible = "cavium,thunder-pcie";
> > > > +               device_type = "pci";
> > > > +               msi-parent = <&its>;
> > > > +               bus-range = <0 255>;
> > > > +               #size-cells = <2>;
> > > > +               #address-cells = <3>;
> > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > +        };
> > > 
> > > If you claim the entire 0-255 bus range, I think you should also
> > > specify a domain, otherwise it's not predictable which domain you
> > > get.
> > 
> > Liviu's code assigns a unique id to the domain if missing, see
> > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > property here.
> 
> Not anymore! That function is gone in v12 onwards. What is in -next has
> a new function called of_get_pci_domain_nr() (slight name change) but
> that only gets the value set in the "linux,pci-domain" property of the
> device node. It is the choice of the host bridge driver to use that
> function or to use pci_get_new_domain_nr() which *does* generate an
> unique id every time it gets called.

I am quite confused a bit on which is the latest code base now. I was
looking into Bjorn's pci/host-generic and there is a different
implemetation:

----
/**
 * This function will try to obtain the host bridge domain number by
 * using of_alias_get_id() call with "pci-domain" as a stem.  If that
 * fails, a local allocator will be used.  The local allocator can
 * be requested to return a new domain_nr if the information is missing
 * from the device tree.
 *
 * @node: device tree node with the domain information
 * @allocate_if_missing: if DT lacks information about the domain nr,
 * allocate a new number.
 *
 * Returns the associated domain number from DT, or a new domain number
 * if DT information is missing and @allocate_if_missing is true.  If
 * @allocate_if_missing is false then the last allocated domain number
 * will be returned.
 */
int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
{
        int domain;

        domain = atomic_read(&of_domain_nr);
        if (domain == -1) {
                /* first run, get max defined domain nr in device tree */
                domain = of_get_max_pci_domain_nr();
                /* then set the start value for allocator to be max + 1 */
                atomic_set(&of_domain_nr, domain + 1);
        }
        domain = of_alias_get_id(node, "pci-domain");
        if (domain == -ENODEV) {
                domain = atomic_read(&of_domain_nr);
                if (allocate_if_missing)
                        atomic_inc(&of_domain_nr);
        }

        return domain;
}
EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
----

This differs also from v13. Please check.

It would be good to have a stable code base to work with, so I would
prefer incremental patches on top of Bjorn's pci/host-generic.

> > Liviu's DT implementation that assigns a unique number differs a bit
> > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > 
> > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > multiple root bridges, the "pci-domain" property could be forced
> > instead.
> 
> Indeed. But the enforcing is left as an exercise to the host bridge
> implementor for the moment.

Right, can be added later.

Thanks,

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 07.10.14 16:01:49, Liviu Dudau wrote:
> On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > +               compatible = "cavium,thunder-pcie";
> > > > +               device_type = "pci";
> > > > +               msi-parent = <&its>;
> > > > +               bus-range = <0 255>;
> > > > +               #size-cells = <2>;
> > > > +               #address-cells = <3>;
> > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > +        };
> > > 
> > > If you claim the entire 0-255 bus range, I think you should also
> > > specify a domain, otherwise it's not predictable which domain you
> > > get.
> > 
> > Liviu's code assigns a unique id to the domain if missing, see
> > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > property here.
> 
> Not anymore! That function is gone in v12 onwards. What is in -next has
> a new function called of_get_pci_domain_nr() (slight name change) but
> that only gets the value set in the "linux,pci-domain" property of the
> device node. It is the choice of the host bridge driver to use that
> function or to use pci_get_new_domain_nr() which *does* generate an
> unique id every time it gets called.

I am quite confused a bit on which is the latest code base now. I was
looking into Bjorn's pci/host-generic and there is a different
implemetation:

----
/**
 * This function will try to obtain the host bridge domain number by
 * using of_alias_get_id() call with "pci-domain" as a stem.  If that
 * fails, a local allocator will be used.  The local allocator can
 * be requested to return a new domain_nr if the information is missing
 * from the device tree.
 *
 * @node: device tree node with the domain information
 * @allocate_if_missing: if DT lacks information about the domain nr,
 * allocate a new number.
 *
 * Returns the associated domain number from DT, or a new domain number
 * if DT information is missing and @allocate_if_missing is true.  If
 * @allocate_if_missing is false then the last allocated domain number
 * will be returned.
 */
int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
{
        int domain;

        domain = atomic_read(&of_domain_nr);
        if (domain == -1) {
                /* first run, get max defined domain nr in device tree */
                domain = of_get_max_pci_domain_nr();
                /* then set the start value for allocator to be max + 1 */
                atomic_set(&of_domain_nr, domain + 1);
        }
        domain = of_alias_get_id(node, "pci-domain");
        if (domain == -ENODEV) {
                domain = atomic_read(&of_domain_nr);
                if (allocate_if_missing)
                        atomic_inc(&of_domain_nr);
        }

        return domain;
}
EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
----

This differs also from v13. Please check.

It would be good to have a stable code base to work with, so I would
prefer incremental patches on top of Bjorn's pci/host-generic.

> > Liviu's DT implementation that assigns a unique number differs a bit
> > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > 
> > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > multiple root bridges, the "pci-domain" property could be forced
> > instead.
> 
> Indeed. But the enforcing is left as an exercise to the host bridge
> implementor for the moment.

Right, can be added later.

Thanks,

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> On 07.10.14 16:01:49, Liviu Dudau wrote:
> > On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > > +               compatible = "cavium,thunder-pcie";
> > > > > +               device_type = "pci";
> > > > > +               msi-parent = <&its>;
> > > > > +               bus-range = <0 255>;
> > > > > +               #size-cells = <2>;
> > > > > +               #address-cells = <3>;
> > > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > > +        };
> > > > 
> > > > If you claim the entire 0-255 bus range, I think you should also
> > > > specify a domain, otherwise it's not predictable which domain you
> > > > get.
> > > 
> > > Liviu's code assigns a unique id to the domain if missing, see
> > > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > > property here.
> > 
> > Not anymore! That function is gone in v12 onwards. What is in -next has
> > a new function called of_get_pci_domain_nr() (slight name change) but
> > that only gets the value set in the "linux,pci-domain" property of the
> > device node. It is the choice of the host bridge driver to use that
> > function or to use pci_get_new_domain_nr() which *does* generate an
> > unique id every time it gets called.
> 
> I am quite confused a bit on which is the latest code base now. I was
> looking into Bjorn's pci/host-generic and there is a different
> implemetation:
> 
> ----
> /**
>  * This function will try to obtain the host bridge domain number by
>  * using of_alias_get_id() call with "pci-domain" as a stem.  If that
>  * fails, a local allocator will be used.  The local allocator can
>  * be requested to return a new domain_nr if the information is missing
>  * from the device tree.
>  *
>  * @node: device tree node with the domain information
>  * @allocate_if_missing: if DT lacks information about the domain nr,
>  * allocate a new number.
>  *
>  * Returns the associated domain number from DT, or a new domain number
>  * if DT information is missing and @allocate_if_missing is true.  If
>  * @allocate_if_missing is false then the last allocated domain number
>  * will be returned.
>  */
> int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
> {
>         int domain;
> 
>         domain = atomic_read(&of_domain_nr);
>         if (domain == -1) {
>                 /* first run, get max defined domain nr in device tree */
>                 domain = of_get_max_pci_domain_nr();
>                 /* then set the start value for allocator to be max + 1 */
>                 atomic_set(&of_domain_nr, domain + 1);
>         }
>         domain = of_alias_get_id(node, "pci-domain");
>         if (domain == -ENODEV) {
>                 domain = atomic_read(&of_domain_nr);
>                 if (allocate_if_missing)
>                         atomic_inc(&of_domain_nr);
>         }
> 
>         return domain;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
> ----
> 
> This differs also from v13. Please check.

I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
my v13. Not sure why you are still seeing this v11 version.


> 
> It would be good to have a stable code base to work with, so I would
> prefer incremental patches on top of Bjorn's pci/host-generic.

Agreed, but from what I can see from my side pci/host-generic and next
have the same versions, so there should not be any confusion.

Best regards,
Liviu

> 
> > > Liviu's DT implementation that assigns a unique number differs a bit
> > > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > > 
> > > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > > multiple root bridges, the "pci-domain" property could be forced
> > > instead.
> > 
> > Indeed. But the enforcing is left as an exercise to the host bridge
> > implementor for the moment.
> 
> Right, can be added later.
> 
> Thanks,
> 
> -Robert
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> On 07.10.14 16:01:49, Liviu Dudau wrote:
> > On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > > +               compatible = "cavium,thunder-pcie";
> > > > > +               device_type = "pci";
> > > > > +               msi-parent = <&its>;
> > > > > +               bus-range = <0 255>;
> > > > > +               #size-cells = <2>;
> > > > > +               #address-cells = <3>;
> > > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > > +        };
> > > > 
> > > > If you claim the entire 0-255 bus range, I think you should also
> > > > specify a domain, otherwise it's not predictable which domain you
> > > > get.
> > > 
> > > Liviu's code assigns a unique id to the domain if missing, see
> > > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > > property here.
> > 
> > Not anymore! That function is gone in v12 onwards. What is in -next has
> > a new function called of_get_pci_domain_nr() (slight name change) but
> > that only gets the value set in the "linux,pci-domain" property of the
> > device node. It is the choice of the host bridge driver to use that
> > function or to use pci_get_new_domain_nr() which *does* generate an
> > unique id every time it gets called.
> 
> I am quite confused a bit on which is the latest code base now. I was
> looking into Bjorn's pci/host-generic and there is a different
> implemetation:
> 
> ----
> /**
>  * This function will try to obtain the host bridge domain number by
>  * using of_alias_get_id() call with "pci-domain" as a stem.  If that
>  * fails, a local allocator will be used.  The local allocator can
>  * be requested to return a new domain_nr if the information is missing
>  * from the device tree.
>  *
>  * @node: device tree node with the domain information
>  * @allocate_if_missing: if DT lacks information about the domain nr,
>  * allocate a new number.
>  *
>  * Returns the associated domain number from DT, or a new domain number
>  * if DT information is missing and @allocate_if_missing is true.  If
>  * @allocate_if_missing is false then the last allocated domain number
>  * will be returned.
>  */
> int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
> {
>         int domain;
> 
>         domain = atomic_read(&of_domain_nr);
>         if (domain == -1) {
>                 /* first run, get max defined domain nr in device tree */
>                 domain = of_get_max_pci_domain_nr();
>                 /* then set the start value for allocator to be max + 1 */
>                 atomic_set(&of_domain_nr, domain + 1);
>         }
>         domain = of_alias_get_id(node, "pci-domain");
>         if (domain == -ENODEV) {
>                 domain = atomic_read(&of_domain_nr);
>                 if (allocate_if_missing)
>                         atomic_inc(&of_domain_nr);
>         }
> 
>         return domain;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
> ----
> 
> This differs also from v13. Please check.

I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
my v13. Not sure why you are still seeing this v11 version.


> 
> It would be good to have a stable code base to work with, so I would
> prefer incremental patches on top of Bjorn's pci/host-generic.

Agreed, but from what I can see from my side pci/host-generic and next
have the same versions, so there should not be any confusion.

Best regards,
Liviu

> 
> > > Liviu's DT implementation that assigns a unique number differs a bit
> > > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > > 
> > > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > > multiple root bridges, the "pci-domain" property could be forced
> > > instead.
> > 
> > Indeed. But the enforcing is left as an exercise to the host bridge
> > implementor for the moment.
> 
> Right, can be added later.
> 
> Thanks,
> 
> -Robert
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Liviu Dudau]

On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> On 07.10.14 16:01:49, Liviu Dudau wrote:
> > On Tue, Oct 07, 2014 at 03:27:44PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:06:04, Arnd Bergmann wrote:
> > > > > +               compatible = "cavium,thunder-pcie";
> > > > > +               device_type = "pci";
> > > > > +               msi-parent = <&its>;
> > > > > +               bus-range = <0 255>;
> > > > > +               #size-cells = <2>;
> > > > > +               #address-cells = <3>;
> > > > > +               reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
> > > > > +               ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem ranges */
> > > > > +                       <0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
> > > > > +                       <0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
> > > > > +        };
> > > > 
> > > > If you claim the entire 0-255 bus range, I think you should also
> > > > specify a domain, otherwise it's not predictable which domain you
> > > > get.
> > > 
> > > Liviu's code assigns a unique id to the domain if missing, see
> > > of_pci_get_domain_nr(). So I don't think we need to add a "pci-domain"
> > > property here.
> > 
> > Not anymore! That function is gone in v12 onwards. What is in -next has
> > a new function called of_get_pci_domain_nr() (slight name change) but
> > that only gets the value set in the "linux,pci-domain" property of the
> > device node. It is the choice of the host bridge driver to use that
> > function or to use pci_get_new_domain_nr() which *does* generate an
> > unique id every time it gets called.
> 
> I am quite confused a bit on which is the latest code base now. I was
> looking into Bjorn's pci/host-generic and there is a different
> implemetation:
> 
> ----
> /**
>  * This function will try to obtain the host bridge domain number by
>  * using of_alias_get_id() call with "pci-domain" as a stem.  If that
>  * fails, a local allocator will be used.  The local allocator can
>  * be requested to return a new domain_nr if the information is missing
>  * from the device tree.
>  *
>  * @node: device tree node with the domain information
>  * @allocate_if_missing: if DT lacks information about the domain nr,
>  * allocate a new number.
>  *
>  * Returns the associated domain number from DT, or a new domain number
>  * if DT information is missing and @allocate_if_missing is true.  If
>  * @allocate_if_missing is false then the last allocated domain number
>  * will be returned.
>  */
> int of_pci_get_domain_nr(struct device_node *node, bool allocate_if_missing)
> {
>         int domain;
> 
>         domain = atomic_read(&of_domain_nr);
>         if (domain == -1) {
>                 /* first run, get max defined domain nr in device tree */
>                 domain = of_get_max_pci_domain_nr();
>                 /* then set the start value for allocator to be max + 1 */
>                 atomic_set(&of_domain_nr, domain + 1);
>         }
>         domain = of_alias_get_id(node, "pci-domain");
>         if (domain == -ENODEV) {
>                 domain = atomic_read(&of_domain_nr);
>                 if (allocate_if_missing)
>                         atomic_inc(&of_domain_nr);
>         }
> 
>         return domain;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_domain_nr);
> ----
> 
> This differs also from v13. Please check.

I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
my v13. Not sure why you are still seeing this v11 version.


> 
> It would be good to have a stable code base to work with, so I would
> prefer incremental patches on top of Bjorn's pci/host-generic.

Agreed, but from what I can see from my side pci/host-generic and next
have the same versions, so there should not be any confusion.

Best regards,
Liviu

> 
> > > Liviu's DT implementation that assigns a unique number differs a bit
> > > from ACPI which states: "If _SEG [aka domain number] does not exist,
> > > OSPM assumes that all PCI bus segments are in PCI Segment Group 0."
> > > 
> > > Maybe of_pci_get_domain_nr() should be similar to ACPI. If there are
> > > multiple root bridges, the "pci-domain" property could be forced
> > > instead.
> > 
> > Indeed. But the enforcing is left as an exercise to the host bridge
> > implementor for the moment.
> 
> Right, can be added later.
> 
> Thanks,
> 
> -Robert
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 08.10.14 17:44:32, Liviu Dudau wrote:
> On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> > On 07.10.14 16:01:49, Liviu Dudau wrote:
> > I am quite confused a bit on which is the latest code base now. I was
> > looking into Bjorn's pci/host-generic and there is a different
> > implemetation:

> > This differs also from v13. Please check.

> I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
> my v13. Not sure why you are still seeing this v11 version.

Right, my bad. Sorry for the noise.

-Robert

[PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 08.10.14 17:44:32, Liviu Dudau wrote:
> On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> > On 07.10.14 16:01:49, Liviu Dudau wrote:
> > I am quite confused a bit on which is the latest code base now. I was
> > looking into Bjorn's pci/host-generic and there is a different
> > implemetation:

> > This differs also from v13. Please check.

> I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
> my v13. Not sure why you are still seeing this v11 version.

Right, my bad. Sorry for the noise.

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 08.10.14 17:44:32, Liviu Dudau wrote:
> On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> > On 07.10.14 16:01:49, Liviu Dudau wrote:
> > I am quite confused a bit on which is the latest code base now. I was
> > looking into Bjorn's pci/host-generic and there is a different
> > implemetation:

> > This differs also from v13. Please check.

> I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
> my v13. Not sure why you are still seeing this v11 version.

Right, my bad. Sorry for the noise.

-Robert

Re: [PATCH 3/6] pci, thunder: Add PCIe host controller devicetree bindings

[Robert Richter]

On 08.10.14 17:44:32, Liviu Dudau wrote:
> On Wed, Oct 08, 2014 at 09:49:27AM +0100, Robert Richter wrote:
> > On 07.10.14 16:01:49, Liviu Dudau wrote:
> > I am quite confused a bit on which is the latest code base now. I was
> > looking into Bjorn's pci/host-generic and there is a different
> > implemetation:

> > This differs also from v13. Please check.

> I'm not sure what you are looking at. Commit 41e5c0f81d3e does look like
> my v13. Not sure why you are still seeing this v11 version.

Right, my bad. Sorry for the noise.

-Robert
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] pci, thunder: Document PCIe host controller devicetree bindings

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds documentation for the devicetree bindings used by the
Thunder PCI host controller.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 .../devicetree/bindings/pci/cavium,thunder-pci.txt | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt b/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
new file mode 100644
index 000000000000..c8ff3d2e8630
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
@@ -0,0 +1,32 @@
+* Cavium Thunder PCIe interface
+
+Required properties:
+- compatible: should contain "cavium,thunder-pcie" to identify the core.
+- device_type: set to "pci"
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- #stream-id-cells: set to <1>
+- bus-range: PCI bus numbers covered
+- reg: base address and length of the pcie configuration space.
+- ranges: ranges for the PCI memory regions.
+- msi-parent: Link to the hardware entity that serves as the Message
+  Signaled Interrupt controller for this PCI controller.
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie0: pcie0@0x848000000000 {
+		compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		#stream-id-cells = <1>;
+		reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem */
+			<0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
+			<0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
+	};
+
-- 
2.1.0

[PATCH 4/6] pci, thunder: Document PCIe host controller devicetree bindings

[Robert Richter]

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds documentation for the devicetree bindings used by the
Thunder PCI host controller.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 .../devicetree/bindings/pci/cavium,thunder-pci.txt | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt b/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
new file mode 100644
index 000000000000..c8ff3d2e8630
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/cavium,thunder-pci.txt
@@ -0,0 +1,32 @@
+* Cavium Thunder PCIe interface
+
+Required properties:
+- compatible: should contain "cavium,thunder-pcie" to identify the core.
+- device_type: set to "pci"
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- #stream-id-cells: set to <1>
+- bus-range: PCI bus numbers covered
+- reg: base address and length of the pcie configuration space.
+- ranges: ranges for the PCI memory regions.
+- msi-parent: Link to the hardware entity that serves as the Message
+  Signaled Interrupt controller for this PCI controller.
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie0: pcie0 at 0x848000000000 {
+		compatible = "cavium,thunder-pcie";
+		device_type = "pci";
+		msi-parent = <&its>;
+		bus-range = <0 255>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		#stream-id-cells = <1>;
+		reg = <0x8480 0x00000000 0 0x10000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8010 0x00000000 0x8010 0x00000000 0x70 0x00000000>, /* mem */
+			<0x03000000 0x8300 0x00000000 0x8300 0x00000000 0x80 0x00000000>,
+			<0x03000000 0x87e0 0x00000000 0x87e0 0x00000000 0x01 0x00000000>;
+	};
+
-- 
2.1.0

[PATCH 5/6] arm64, defconfig: Enable PCI

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

Enable the PCI subsystem in defconfig. There will be more and more
systems with pci support, so default enable it. This also implicitly
enables the MSI feature.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ca3f7e9afd4d..97777e1ef5a7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -163,11 +163,15 @@ config ARM_AMBA
 
 config PCI
 	bool "PCI support"
+	default y
 	help
 	  This feature enables support for PCIe bus system. If you say Y
 	  here, the kernel will include drivers and infrastructure code
 	  to support PCIe bus devices.
 
+config PCI_MSI
+	def_bool PCI
+
 config PCI_DOMAINS
 	def_bool PCI
 
-- 
2.1.0

[PATCH 5/6] arm64, defconfig: Enable PCI

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

Enable the PCI subsystem in defconfig. There will be more and more
systems with pci support, so default enable it. This also implicitly
enables the MSI feature.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ca3f7e9afd4d..97777e1ef5a7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -163,11 +163,15 @@ config ARM_AMBA
 
 config PCI
 	bool "PCI support"
+	default y
 	help
 	  This feature enables support for PCIe bus system. If you say Y
 	  here, the kernel will include drivers and infrastructure code
 	  to support PCIe bus devices.
 
+config PCI_MSI
+	def_bool PCI
+
 config PCI_DOMAINS
 	def_bool PCI
 
-- 
2.1.0

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
>  config PCI
>         bool "PCI support"
> +       default y
>         help
>           This feature enables support for PCIe bus system. If you say Y
>           here, the kernel will include drivers and infrastructure code
>           to support PCIe bus devices.
>  
> +config PCI_MSI
> +       def_bool PCI
> +
>  config PCI_DOMAINS
>         def_bool PCI

There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
from the PCI symbol above rather than defining a second one.

	Arnd

[PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
>  config PCI
>         bool "PCI support"
> +       default y
>         help
>           This feature enables support for PCIe bus system. If you say Y
>           here, the kernel will include drivers and infrastructure code
>           to support PCIe bus devices.
>  
> +config PCI_MSI
> +       def_bool PCI
> +
>  config PCI_DOMAINS
>         def_bool PCI

There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
from the PCI symbol above rather than defining a second one.

	Arnd

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Robert Richter]

On 24.09.14 18:14:11, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> >  config PCI
> >         bool "PCI support"
> > +       default y
> >         help
> >           This feature enables support for PCIe bus system. If you say Y
> >           here, the kernel will include drivers and infrastructure code
> >           to support PCIe bus devices.
> >  
> > +config PCI_MSI
> > +       def_bool PCI
> > +
> >  config PCI_DOMAINS
> >         def_bool PCI
> 
> There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> from the PCI symbol above rather than defining a second one.

The intention is not to have a second definition, instead this should
enable the default value just for arm64. Thus I put it to
arch/arm64/Kconfig. Otherwise it would be enabled per default on all
archs.

We could have used select in config ARM64, but I tried to avoid using
select due to the dependency issue and instead implement this with
default-y/depends-on. Doing so it can be manually disabled too.

-Robert

[PATCH 5/6] arm64, defconfig: Enable PCI

[Robert Richter]

On 24.09.14 18:14:11, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> >  config PCI
> >         bool "PCI support"
> > +       default y
> >         help
> >           This feature enables support for PCIe bus system. If you say Y
> >           here, the kernel will include drivers and infrastructure code
> >           to support PCIe bus devices.
> >  
> > +config PCI_MSI
> > +       def_bool PCI
> > +
> >  config PCI_DOMAINS
> >         def_bool PCI
> 
> There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> from the PCI symbol above rather than defining a second one.

The intention is not to have a second definition, instead this should
enable the default value just for arm64. Thus I put it to
arch/arm64/Kconfig. Otherwise it would be enabled per default on all
archs.

We could have used select in config ARM64, but I tried to avoid using
select due to the dependency issue and instead implement this with
default-y/depends-on. Doing so it can be manually disabled too.

-Robert

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > >  config PCI
> > >         bool "PCI support"
> > > +       default y
> > >         help
> > >           This feature enables support for PCIe bus system. If you say Y
> > >           here, the kernel will include drivers and infrastructure code
> > >           to support PCIe bus devices.
> > >  
> > > +config PCI_MSI
> > > +       def_bool PCI
> > > +
> > >  config PCI_DOMAINS
> > >         def_bool PCI
> > 
> > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > from the PCI symbol above rather than defining a second one.
> 
> The intention is not to have a second definition, instead this should
> enable the default value just for arm64. Thus I put it to
> arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> archs.
> 
> We could have used select in config ARM64, but I tried to avoid using
> select due to the dependency issue and instead implement this with
> default-y/depends-on. Doing so it can be manually disabled too.

It may be better to simply set PCI and PCI_MSI as defaults in defconfig
rather than "default y" and an additional config PCI_MSI entry.

-- 
Catalin

[PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > >  config PCI
> > >         bool "PCI support"
> > > +       default y
> > >         help
> > >           This feature enables support for PCIe bus system. If you say Y
> > >           here, the kernel will include drivers and infrastructure code
> > >           to support PCIe bus devices.
> > >  
> > > +config PCI_MSI
> > > +       def_bool PCI
> > > +
> > >  config PCI_DOMAINS
> > >         def_bool PCI
> > 
> > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > from the PCI symbol above rather than defining a second one.
> 
> The intention is not to have a second definition, instead this should
> enable the default value just for arm64. Thus I put it to
> arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> archs.
> 
> We could have used select in config ARM64, but I tried to avoid using
> select due to the dependency issue and instead implement this with
> default-y/depends-on. Doing so it can be manually disabled too.

It may be better to simply set PCI and PCI_MSI as defaults in defconfig
rather than "default y" and an additional config PCI_MSI entry.

-- 
Catalin

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > >  config PCI
> > >         bool "PCI support"
> > > +       default y
> > >         help
> > >           This feature enables support for PCIe bus system. If you say Y
> > >           here, the kernel will include drivers and infrastructure code
> > >           to support PCIe bus devices.
> > >  
> > > +config PCI_MSI
> > > +       def_bool PCI
> > > +
> > >  config PCI_DOMAINS
> > >         def_bool PCI
> > 
> > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > from the PCI symbol above rather than defining a second one.
> 
> The intention is not to have a second definition, instead this should
> enable the default value just for arm64. Thus I put it to
> arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> archs.
> 
> We could have used select in config ARM64, but I tried to avoid using
> select due to the dependency issue and instead implement this with
> default-y/depends-on. Doing so it can be manually disabled too.

It may be better to simply set PCI and PCI_MSI as defaults in defconfig
rather than "default y" and an additional config PCI_MSI entry.

-- 
Catalin

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > >  config PCI
> > > >         bool "PCI support"
> > > > +       default y
> > > >         help
> > > >           This feature enables support for PCIe bus system. If you say Y
> > > >           here, the kernel will include drivers and infrastructure code
> > > >           to support PCIe bus devices.
> > > >  
> > > > +config PCI_MSI
> > > > +       def_bool PCI
> > > > +
> > > >  config PCI_DOMAINS
> > > >         def_bool PCI
> > > 
> > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > from the PCI symbol above rather than defining a second one.
> > 
> > The intention is not to have a second definition, instead this should
> > enable the default value just for arm64. Thus I put it to
> > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > archs.

I don't think other architectures actually see the PCI symbol that is
defined in the arm64 Kconfig file, but I might be wrong.

> > We could have used select in config ARM64, but I tried to avoid using
> > select due to the dependency issue and instead implement this with
> > default-y/depends-on. Doing so it can be manually disabled too.

How about adding a new symbol like

config ARM64_PCI
	def_bool PCI
	select PCI_MSI


> It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> rather than "default y" and an additional config PCI_MSI entry.

Selecting PCI_MSI unconditionally seems fine to me. We won't have any
ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

	Arnd

[PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > >  config PCI
> > > >         bool "PCI support"
> > > > +       default y
> > > >         help
> > > >           This feature enables support for PCIe bus system. If you say Y
> > > >           here, the kernel will include drivers and infrastructure code
> > > >           to support PCIe bus devices.
> > > >  
> > > > +config PCI_MSI
> > > > +       def_bool PCI
> > > > +
> > > >  config PCI_DOMAINS
> > > >         def_bool PCI
> > > 
> > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > from the PCI symbol above rather than defining a second one.
> > 
> > The intention is not to have a second definition, instead this should
> > enable the default value just for arm64. Thus I put it to
> > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > archs.

I don't think other architectures actually see the PCI symbol that is
defined in the arm64 Kconfig file, but I might be wrong.

> > We could have used select in config ARM64, but I tried to avoid using
> > select due to the dependency issue and instead implement this with
> > default-y/depends-on. Doing so it can be manually disabled too.

How about adding a new symbol like

config ARM64_PCI
	def_bool PCI
	select PCI_MSI


> It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> rather than "default y" and an additional config PCI_MSI entry.

Selecting PCI_MSI unconditionally seems fine to me. We won't have any
ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

	Arnd

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > >  config PCI
> > > >         bool "PCI support"
> > > > +       default y
> > > >         help
> > > >           This feature enables support for PCIe bus system. If you say Y
> > > >           here, the kernel will include drivers and infrastructure code
> > > >           to support PCIe bus devices.
> > > >  
> > > > +config PCI_MSI
> > > > +       def_bool PCI
> > > > +
> > > >  config PCI_DOMAINS
> > > >         def_bool PCI
> > > 
> > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > from the PCI symbol above rather than defining a second one.
> > 
> > The intention is not to have a second definition, instead this should
> > enable the default value just for arm64. Thus I put it to
> > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > archs.

I don't think other architectures actually see the PCI symbol that is
defined in the arm64 Kconfig file, but I might be wrong.

> > We could have used select in config ARM64, but I tried to avoid using
> > select due to the dependency issue and instead implement this with
> > default-y/depends-on. Doing so it can be manually disabled too.

How about adding a new symbol like

config ARM64_PCI
	def_bool PCI
	select PCI_MSI


> It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> rather than "default y" and an additional config PCI_MSI entry.

Selecting PCI_MSI unconditionally seems fine to me. We won't have any
ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

	Arnd

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > > >  config PCI
> > > > >         bool "PCI support"
> > > > > +       default y
> > > > >         help
> > > > >           This feature enables support for PCIe bus system. If you say Y
> > > > >           here, the kernel will include drivers and infrastructure code
> > > > >           to support PCIe bus devices.
> > > > >  
> > > > > +config PCI_MSI
> > > > > +       def_bool PCI
> > > > > +
> > > > >  config PCI_DOMAINS
> > > > >         def_bool PCI
> > > > 
> > > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > > from the PCI symbol above rather than defining a second one.
> > > 
> > > The intention is not to have a second definition, instead this should
> > > enable the default value just for arm64. Thus I put it to
> > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > archs.
> 
> I don't think other architectures actually see the PCI symbol that is
> defined in the arm64 Kconfig file, but I might be wrong.

Other archs wouldn't see the symbols in arch/arm64/Kconfig.

> > > We could have used select in config ARM64, but I tried to avoid using
> > > select due to the dependency issue and instead implement this with
> > > default-y/depends-on. Doing so it can be manually disabled too.
> 
> How about adding a new symbol like
> 
> config ARM64_PCI
> 	def_bool PCI
> 	select PCI_MSI

How is this different from just selecting PCI_MSI in config PCI in
arch/arm64/Kconfig? I don't see what another symbol brings.

> > It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> > rather than "default y" and an additional config PCI_MSI entry.
> 
> Selecting PCI_MSI unconditionally seems fine to me. We won't have any
> ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

OK then, we can go with a select.

-- 
Catalin

[PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > > >  config PCI
> > > > >         bool "PCI support"
> > > > > +       default y
> > > > >         help
> > > > >           This feature enables support for PCIe bus system. If you say Y
> > > > >           here, the kernel will include drivers and infrastructure code
> > > > >           to support PCIe bus devices.
> > > > >  
> > > > > +config PCI_MSI
> > > > > +       def_bool PCI
> > > > > +
> > > > >  config PCI_DOMAINS
> > > > >         def_bool PCI
> > > > 
> > > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > > from the PCI symbol above rather than defining a second one.
> > > 
> > > The intention is not to have a second definition, instead this should
> > > enable the default value just for arm64. Thus I put it to
> > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > archs.
> 
> I don't think other architectures actually see the PCI symbol that is
> defined in the arm64 Kconfig file, but I might be wrong.

Other archs wouldn't see the symbols in arch/arm64/Kconfig.

> > > We could have used select in config ARM64, but I tried to avoid using
> > > select due to the dependency issue and instead implement this with
> > > default-y/depends-on. Doing so it can be manually disabled too.
> 
> How about adding a new symbol like
> 
> config ARM64_PCI
> 	def_bool PCI
> 	select PCI_MSI

How is this different from just selecting PCI_MSI in config PCI in
arch/arm64/Kconfig? I don't see what another symbol brings.

> > It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> > rather than "default y" and an additional config PCI_MSI entry.
> 
> Selecting PCI_MSI unconditionally seems fine to me. We won't have any
> ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

OK then, we can go with a select.

-- 
Catalin

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Catalin Marinas]

On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > On Wed, Sep 24, 2014 at 05:26:31PM +0100, Robert Richter wrote:
> > > On 24.09.14 18:14:11, Arnd Bergmann wrote:
> > > > On Wednesday 24 September 2014 17:37:47 Robert Richter wrote:
> > > > >  config PCI
> > > > >         bool "PCI support"
> > > > > +       default y
> > > > >         help
> > > > >           This feature enables support for PCIe bus system. If you say Y
> > > > >           here, the kernel will include drivers and infrastructure code
> > > > >           to support PCIe bus devices.
> > > > >  
> > > > > +config PCI_MSI
> > > > > +       def_bool PCI
> > > > > +
> > > > >  config PCI_DOMAINS
> > > > >         def_bool PCI
> > > > 
> > > > There is already a PCI_MSI symbol in drivers/pci/Kconfig. Just select that
> > > > from the PCI symbol above rather than defining a second one.
> > > 
> > > The intention is not to have a second definition, instead this should
> > > enable the default value just for arm64. Thus I put it to
> > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > archs.
> 
> I don't think other architectures actually see the PCI symbol that is
> defined in the arm64 Kconfig file, but I might be wrong.

Other archs wouldn't see the symbols in arch/arm64/Kconfig.

> > > We could have used select in config ARM64, but I tried to avoid using
> > > select due to the dependency issue and instead implement this with
> > > default-y/depends-on. Doing so it can be manually disabled too.
> 
> How about adding a new symbol like
> 
> config ARM64_PCI
> 	def_bool PCI
> 	select PCI_MSI

How is this different from just selecting PCI_MSI in config PCI in
arch/arm64/Kconfig? I don't see what another symbol brings.

> > It may be better to simply set PCI and PCI_MSI as defaults in defconfig
> > rather than "default y" and an additional config PCI_MSI entry.
> 
> Selecting PCI_MSI unconditionally seems fine to me. We won't have any
> ARM64 systems that only do classic PCI, and MSI is mandatory for PCIe.

OK then, we can go with a select.

-- 
Catalin

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Thursday 25 September 2014 10:35:45 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > > > The intention is not to have a second definition, instead this should
> > > > enable the default value just for arm64. Thus I put it to
> > > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > > archs.
> > 
> > I don't think other architectures actually see the PCI symbol that is
> > defined in the arm64 Kconfig file, but I might be wrong.
> 
> Other archs wouldn't see the symbols in arch/arm64/Kconfig.

Ok.

> > > > We could have used select in config ARM64, but I tried to avoid using
> > > > select due to the dependency issue and instead implement this with
> > > > default-y/depends-on. Doing so it can be manually disabled too.
> > 
> > How about adding a new symbol like
> > 
> > config ARM64_PCI
> >       def_bool PCI
> >       select PCI_MSI
> 
> How is this different from just selecting PCI_MSI in config PCI in
> arch/arm64/Kconfig? I don't see what another symbol brings.

That suggestion was just in case I was wrong above. If the symbol
is indeed hidden elsewhere, we don't need it.

	Arnd

[PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Thursday 25 September 2014 10:35:45 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > > > The intention is not to have a second definition, instead this should
> > > > enable the default value just for arm64. Thus I put it to
> > > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > > archs.
> > 
> > I don't think other architectures actually see the PCI symbol that is
> > defined in the arm64 Kconfig file, but I might be wrong.
> 
> Other archs wouldn't see the symbols in arch/arm64/Kconfig.

Ok.

> > > > We could have used select in config ARM64, but I tried to avoid using
> > > > select due to the dependency issue and instead implement this with
> > > > default-y/depends-on. Doing so it can be manually disabled too.
> > 
> > How about adding a new symbol like
> > 
> > config ARM64_PCI
> >       def_bool PCI
> >       select PCI_MSI
> 
> How is this different from just selecting PCI_MSI in config PCI in
> arch/arm64/Kconfig? I don't see what another symbol brings.

That suggestion was just in case I was wrong above. If the symbol
is indeed hidden elsewhere, we don't need it.

	Arnd

Re: [PATCH 5/6] arm64, defconfig: Enable PCI

[Arnd Bergmann]

On Thursday 25 September 2014 10:35:45 Catalin Marinas wrote:
> On Wed, Sep 24, 2014 at 07:40:18PM +0100, Arnd Bergmann wrote:
> > On Wednesday 24 September 2014 18:10:48 Catalin Marinas wrote:
> > > > The intention is not to have a second definition, instead this should
> > > > enable the default value just for arm64. Thus I put it to
> > > > arch/arm64/Kconfig. Otherwise it would be enabled per default on all
> > > > archs.
> > 
> > I don't think other architectures actually see the PCI symbol that is
> > defined in the arm64 Kconfig file, but I might be wrong.
> 
> Other archs wouldn't see the symbols in arch/arm64/Kconfig.

Ok.

> > > > We could have used select in config ARM64, but I tried to avoid using
> > > > select due to the dependency issue and instead implement this with
> > > > default-y/depends-on. Doing so it can be manually disabled too.
> > 
> > How about adding a new symbol like
> > 
> > config ARM64_PCI
> >       def_bool PCI
> >       select PCI_MSI
> 
> How is this different from just selecting PCI_MSI in config PCI in
> arch/arm64/Kconfig? I don't see what another symbol brings.

That suggestion was just in case I was wrong above. If the symbol
is indeed hidden elsewhere, we don't need it.

	Arnd

[PATCH 6/6] pci, thunder: Enable Cavium Thunder PCIe host controller

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

Enable Cavium Thunder PCIe host controller driver.

IOV is enabled per default if drivers with IOV support are
enabled. IOV can be disabled.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 97777e1ef5a7..bd9724e5c8a1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -172,6 +172,14 @@ config PCI
 config PCI_MSI
 	def_bool PCI
 
+config NO_PCI_IOV
+	def_bool PCI
+	depends on !PCI_THUNDER
+
+config PCI_IOV
+	def_bool PCI
+	depends on !NO_PCI_IOV
+
 config PCI_DOMAINS
 	def_bool PCI
 
@@ -185,6 +193,9 @@ source "drivers/pci/Kconfig"
 source "drivers/pci/pcie/Kconfig"
 source "drivers/pci/hotplug/Kconfig"
 
+config PCI_THUNDER
+	def_bool PCI
+
 endmenu
 
 menu "Kernel Features"
-- 
2.1.0

[PATCH 6/6] pci, thunder: Enable Cavium Thunder PCIe host controller

[Robert Richter]

From: Robert Richter <rrichter@cavium.com>

Enable Cavium Thunder PCIe host controller driver.

IOV is enabled per default if drivers with IOV support are
enabled. IOV can be disabled.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 97777e1ef5a7..bd9724e5c8a1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -172,6 +172,14 @@ config PCI
 config PCI_MSI
 	def_bool PCI
 
+config NO_PCI_IOV
+	def_bool PCI
+	depends on !PCI_THUNDER
+
+config PCI_IOV
+	def_bool PCI
+	depends on !NO_PCI_IOV
+
 config PCI_DOMAINS
 	def_bool PCI
 
@@ -185,6 +193,9 @@ source "drivers/pci/Kconfig"
 source "drivers/pci/pcie/Kconfig"
 source "drivers/pci/hotplug/Kconfig"
 
+config PCI_THUNDER
+	def_bool PCI
+
 endmenu
 
 menu "Kernel Features"
-- 
2.1.0

Re: [PATCH 6/6] pci, thunder: Enable Cavium Thunder PCIe host controller

[Catalin Marinas]

On Wed, Sep 24, 2014 at 04:37:48PM +0100, Robert Richter wrote:
> From: Robert Richter <rrichter@cavium.com>
> 
> Enable Cavium Thunder PCIe host controller driver.
> 
> IOV is enabled per default if drivers with IOV support are
> enabled. IOV can be disabled.
> 
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  arch/arm64/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 97777e1ef5a7..bd9724e5c8a1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -172,6 +172,14 @@ config PCI
>  config PCI_MSI
>  	def_bool PCI
>  
> +config NO_PCI_IOV
> +	def_bool PCI
> +	depends on !PCI_THUNDER
> +
> +config PCI_IOV
> +	def_bool PCI
> +	depends on !NO_PCI_IOV

Does this work with single image where drivers may have different
PCI_IOV needs?

-- 
Catalin

[PATCH 6/6] pci, thunder: Enable Cavium Thunder PCIe host controller

[Catalin Marinas]

On Wed, Sep 24, 2014 at 04:37:48PM +0100, Robert Richter wrote:
> From: Robert Richter <rrichter@cavium.com>
> 
> Enable Cavium Thunder PCIe host controller driver.
> 
> IOV is enabled per default if drivers with IOV support are
> enabled. IOV can be disabled.
> 
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  arch/arm64/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 97777e1ef5a7..bd9724e5c8a1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -172,6 +172,14 @@ config PCI
>  config PCI_MSI
>  	def_bool PCI
>  
> +config NO_PCI_IOV
> +	def_bool PCI
> +	depends on !PCI_THUNDER
> +
> +config PCI_IOV
> +	def_bool PCI
> +	depends on !NO_PCI_IOV

Does this work with single image where drivers may have different
PCI_IOV needs?

-- 
Catalin

Re: [PATCH 6/6] pci, thunder: Enable Cavium Thunder PCIe host controller

[Catalin Marinas]

On Wed, Sep 24, 2014 at 04:37:48PM +0100, Robert Richter wrote:
> From: Robert Richter <rrichter@cavium.com>
> 
> Enable Cavium Thunder PCIe host controller driver.
> 
> IOV is enabled per default if drivers with IOV support are
> enabled. IOV can be disabled.
> 
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  arch/arm64/Kconfig | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 97777e1ef5a7..bd9724e5c8a1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -172,6 +172,14 @@ config PCI
>  config PCI_MSI
>  	def_bool PCI
>  
> +config NO_PCI_IOV
> +	def_bool PCI
> +	depends on !PCI_THUNDER
> +
> +config PCI_IOV
> +	def_bool PCI
> +	depends on !NO_PCI_IOV

Does this work with single image where drivers may have different
PCI_IOV needs?

-- 
Catalin