All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Add host controller drivers for Cavium ThunderX PCI
@ 2016-02-05 23:41 ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree
  Cc: linux-kernel, David Daney

From: David Daney <david.daney@cavium.com>

Some Cavium ThunderX processors require quirky access methods for the
config space of the PCIe bridge.

There are now three patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.  This splits the driver for CAM and ECAM
   access methods to a separate file from the common host driver code.

2) Add the ThunderX PCIe driver to external PCIe buses, which
   leverages the code in pci-host-generic

3) Add ThunderX PCI driver for internel SoC buses used on early
   ThunderX chip revisions.

Changes from v4: Added patch 3/3.  Stylistic changes to 2/3 suggested
by Bjorn Helgaas.  When expanding config write width to 32-bits, mask
out unintened writes to W1C bits, also suggested by Bjorn Helgaas.

Changes from v3: Add some Acked-by, rebased to v4.5.0-rc1

Changes from v2: Improve device tree binding example as noted by Rob
Herring.  Rename pcie-thunder-pem.* to pci-thunder-pem.* for better
consistency.  Update MAINTAINERS to reflect the changes.

Changes from v1: Split CAM and ECAM code from common driver code as
suggested by Arnd Bergmann.  Fix spelling errors in
pcie-thunder-pem.txt

David Daney (3):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
  pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices

 .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 +++
 MAINTAINERS                                        |   9 +
 drivers/pci/host/Kconfig                           |  18 ++
 drivers/pci/host/Makefile                          |   3 +
 drivers/pci/host/pci-host-common.c                 | 194 +++++++++++
 drivers/pci/host/pci-host-common.h                 |  47 +++
 drivers/pci/host/pci-host-generic.c                | 181 +----------
 drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
 drivers/pci/host/pci-thunder-pem.c                 | 332 +++++++++++++++++++
 10 files changed, 1038 insertions(+), 177 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-common.c
 create mode 100644 drivers/pci/host/pci-host-common.h
 create mode 100644 drivers/pci/host/pci-thunder-ecam.c
 create mode 100644 drivers/pci/host/pci-thunder-pem.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 0/3] Add host controller drivers for Cavium ThunderX PCI
@ 2016-02-05 23:41 ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Daney

From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Some Cavium ThunderX processors require quirky access methods for the
config space of the PCIe bridge.

There are now three patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.  This splits the driver for CAM and ECAM
   access methods to a separate file from the common host driver code.

2) Add the ThunderX PCIe driver to external PCIe buses, which
   leverages the code in pci-host-generic

3) Add ThunderX PCI driver for internel SoC buses used on early
   ThunderX chip revisions.

Changes from v4: Added patch 3/3.  Stylistic changes to 2/3 suggested
by Bjorn Helgaas.  When expanding config write width to 32-bits, mask
out unintened writes to W1C bits, also suggested by Bjorn Helgaas.

Changes from v3: Add some Acked-by, rebased to v4.5.0-rc1

Changes from v2: Improve device tree binding example as noted by Rob
Herring.  Rename pcie-thunder-pem.* to pci-thunder-pem.* for better
consistency.  Update MAINTAINERS to reflect the changes.

Changes from v1: Split CAM and ECAM code from common driver code as
suggested by Arnd Bergmann.  Fix spelling errors in
pcie-thunder-pem.txt

David Daney (3):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
  pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices

 .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 +++
 MAINTAINERS                                        |   9 +
 drivers/pci/host/Kconfig                           |  18 ++
 drivers/pci/host/Makefile                          |   3 +
 drivers/pci/host/pci-host-common.c                 | 194 +++++++++++
 drivers/pci/host/pci-host-common.h                 |  47 +++
 drivers/pci/host/pci-host-generic.c                | 181 +----------
 drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
 drivers/pci/host/pci-thunder-pem.c                 | 332 +++++++++++++++++++
 10 files changed, 1038 insertions(+), 177 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-common.c
 create mode 100644 drivers/pci/host/pci-host-common.h
 create mode 100644 drivers/pci/host/pci-thunder-ecam.c
 create mode 100644 drivers/pci/host/pci-thunder-pem.c

-- 
1.8.3.1

--
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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 0/3] Add host controller drivers for Cavium ThunderX PCI
@ 2016-02-05 23:41 ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Daney <david.daney@cavium.com>

Some Cavium ThunderX processors require quirky access methods for the
config space of the PCIe bridge.

There are now three patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.  This splits the driver for CAM and ECAM
   access methods to a separate file from the common host driver code.

2) Add the ThunderX PCIe driver to external PCIe buses, which
   leverages the code in pci-host-generic

3) Add ThunderX PCI driver for internel SoC buses used on early
   ThunderX chip revisions.

Changes from v4: Added patch 3/3.  Stylistic changes to 2/3 suggested
by Bjorn Helgaas.  When expanding config write width to 32-bits, mask
out unintened writes to W1C bits, also suggested by Bjorn Helgaas.

Changes from v3: Add some Acked-by, rebased to v4.5.0-rc1

Changes from v2: Improve device tree binding example as noted by Rob
Herring.  Rename pcie-thunder-pem.* to pci-thunder-pem.* for better
consistency.  Update MAINTAINERS to reflect the changes.

Changes from v1: Split CAM and ECAM code from common driver code as
suggested by Arnd Bergmann.  Fix spelling errors in
pcie-thunder-pem.txt

David Daney (3):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
  pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices

 .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 +++
 MAINTAINERS                                        |   9 +
 drivers/pci/host/Kconfig                           |  18 ++
 drivers/pci/host/Makefile                          |   3 +
 drivers/pci/host/pci-host-common.c                 | 194 +++++++++++
 drivers/pci/host/pci-host-common.h                 |  47 +++
 drivers/pci/host/pci-host-generic.c                | 181 +----------
 drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
 drivers/pci/host/pci-thunder-pem.c                 | 332 +++++++++++++++++++
 10 files changed, 1038 insertions(+), 177 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-common.c
 create mode 100644 drivers/pci/host/pci-host-common.h
 create mode 100644 drivers/pci/host/pci-thunder-ecam.c
 create mode 100644 drivers/pci/host/pci-thunder-pem.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 1/3] PCI: generic: Refactor code to enable reuse by other drivers.
  2016-02-05 23:41 ` David Daney
@ 2016-02-05 23:41   ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree
  Cc: linux-kernel, David Daney

From: David Daney <david.daney@cavium.com>

No change in functionality.

Move structure definitions into a separate header file.  Move common
code to new file with Kconfig machinery to build it.  Split probe
function in to two parts:

   - a small driver specific probe function (gen_pci_probe)

   - a common probe that can be used by other drivers
     (pci_host_common_probe)

Signed-off-by: David Daney <david.daney@cavium.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 MAINTAINERS                         |   1 +
 drivers/pci/host/Kconfig            |   4 +
 drivers/pci/host/Makefile           |   1 +
 drivers/pci/host/pci-host-common.c  | 194 ++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pci-host-common.h  |  47 +++++++++
 drivers/pci/host/pci-host-generic.c | 181 +--------------------------------
 6 files changed, 251 insertions(+), 177 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-common.c
 create mode 100644 drivers/pci/host/pci-host-common.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a9010d9..9287929 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8367,6 +8367,7 @@ L:	linux-pci@vger.kernel.org
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/host-generic-pci.txt
+F:	drivers/pci/host/pci-host-common.c
 F:	drivers/pci/host/pci-host-generic.c
 
 PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 75a6054..65709b4 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -53,9 +53,13 @@ config PCI_RCAR_GEN2_PCIE
 	help
 	  Say Y here if you want PCIe controller support on R-Car Gen2 SoCs.
 
+config PCI_HOST_COMMON
+	bool
+
 config PCI_HOST_GENERIC
 	bool "Generic PCI host controller"
 	depends on (ARM || ARM64) && OF
+	select PCI_HOST_COMMON
 	help
 	  Say Y here if you want to support a simple generic PCI host
 	  controller, such as the one emulated by kvmtool.
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 7b2f20c..3b24af8 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
+obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
new file mode 100644
index 0000000..e9f850f
--- /dev/null
+++ b/drivers/pci/host/pci-host-common.c
@@ -0,0 +1,194 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
+{
+	pci_free_resource_list(&pci->resources);
+}
+
+static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
+{
+	int err, res_valid = 0;
+	struct device *dev = pci->host.dev.parent;
+	struct device_node *np = dev->of_node;
+	resource_size_t iobase;
+	struct resource_entry *win;
+
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
+					       &iobase);
+	if (err)
+		return err;
+
+	resource_list_for_each_entry(win, &pci->resources) {
+		struct resource *parent, *res = win->res;
+
+		switch (resource_type(res)) {
+		case IORESOURCE_IO:
+			parent = &ioport_resource;
+			err = pci_remap_iospace(res, iobase);
+			if (err) {
+				dev_warn(dev, "error %d: failed to map resource %pR\n",
+					 err, res);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			parent = &iomem_resource;
+			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+			break;
+		case IORESOURCE_BUS:
+			pci->cfg.bus_range = res;
+		default:
+			continue;
+		}
+
+		err = devm_request_resource(dev, parent, res);
+		if (err)
+			goto out_release_res;
+	}
+
+	if (!res_valid) {
+		dev_err(dev, "non-prefetchable memory resource required\n");
+		err = -EINVAL;
+		goto out_release_res;
+	}
+
+	return 0;
+
+out_release_res:
+	gen_pci_release_of_pci_ranges(pci);
+	return err;
+}
+
+static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
+{
+	int err;
+	u8 bus_max;
+	resource_size_t busn;
+	struct resource *bus_range;
+	struct device *dev = pci->host.dev.parent;
+	struct device_node *np = dev->of_node;
+	u32 sz = 1 << pci->cfg.ops->bus_shift;
+
+	err = of_address_to_resource(np, 0, &pci->cfg.res);
+	if (err) {
+		dev_err(dev, "missing \"reg\" property\n");
+		return err;
+	}
+
+	/* Limit the bus-range to fit within reg */
+	bus_max = pci->cfg.bus_range->start +
+		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
+	pci->cfg.bus_range->end = min_t(resource_size_t,
+					pci->cfg.bus_range->end, bus_max);
+
+	pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
+				    sizeof(*pci->cfg.win), GFP_KERNEL);
+	if (!pci->cfg.win)
+		return -ENOMEM;
+
+	/* Map our Configuration Space windows */
+	if (!devm_request_mem_region(dev, pci->cfg.res.start,
+				     resource_size(&pci->cfg.res),
+				     "Configuration Space"))
+		return -ENOMEM;
+
+	bus_range = pci->cfg.bus_range;
+	for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
+		u32 idx = busn - bus_range->start;
+
+		pci->cfg.win[idx] = devm_ioremap(dev,
+						 pci->cfg.res.start + idx * sz,
+						 sz);
+		if (!pci->cfg.win[idx])
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+int pci_host_common_probe(struct platform_device *pdev,
+			  struct gen_pci *pci)
+{
+	int err;
+	const char *type;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct pci_bus *bus, *child;
+
+	type = of_get_property(np, "device_type", NULL);
+	if (!type || strcmp(type, "pci")) {
+		dev_err(dev, "invalid \"device_type\" %s\n", type);
+		return -EINVAL;
+	}
+
+	of_pci_check_probe_only();
+
+	pci->host.dev.parent = dev;
+	INIT_LIST_HEAD(&pci->host.windows);
+	INIT_LIST_HEAD(&pci->resources);
+
+	/* Parse our PCI ranges and request their resources */
+	err = gen_pci_parse_request_of_pci_ranges(pci);
+	if (err)
+		return err;
+
+	/* Parse and map our Configuration Space windows */
+	err = gen_pci_parse_map_cfg_windows(pci);
+	if (err) {
+		gen_pci_release_of_pci_ranges(pci);
+		return err;
+	}
+
+	/* Do not reassign resources if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
+
+
+	bus = pci_scan_root_bus(dev, pci->cfg.bus_range->start,
+				&pci->cfg.ops->ops, pci, &pci->resources);
+	if (!bus) {
+		dev_err(dev, "Scanning rootbus failed");
+		return -ENODEV;
+	}
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
+	}
+
+	pci_bus_add_devices(bus);
+	return 0;
+}
+
+MODULE_DESCRIPTION("Generic PCI host driver common code");
+MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/host/pci-host-common.h b/drivers/pci/host/pci-host-common.h
new file mode 100644
index 0000000..09f3fa0
--- /dev/null
+++ b/drivers/pci/host/pci-host-common.h
@@ -0,0 +1,47 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#ifndef _PCI_HOST_COMMON_H
+#define _PCI_HOST_COMMON_H
+
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+
+struct gen_pci_cfg_bus_ops {
+	u32 bus_shift;
+	struct pci_ops ops;
+};
+
+struct gen_pci_cfg_windows {
+	struct resource				res;
+	struct resource				*bus_range;
+	void __iomem				**win;
+
+	struct gen_pci_cfg_bus_ops		*ops;
+};
+
+struct gen_pci {
+	struct pci_host_bridge			host;
+	struct gen_pci_cfg_windows		cfg;
+	struct list_head			resources;
+};
+
+int pci_host_common_probe(struct platform_device *pdev,
+			  struct gen_pci *pci);
+
+#endif /* _PCI_HOST_COMMON_H */
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 1652bc7..e8aa78f 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -25,24 +25,7 @@
 #include <linux/of_pci.h>
 #include <linux/platform_device.h>
 
-struct gen_pci_cfg_bus_ops {
-	u32 bus_shift;
-	struct pci_ops ops;
-};
-
-struct gen_pci_cfg_windows {
-	struct resource				res;
-	struct resource				*bus_range;
-	void __iomem				**win;
-
-	struct gen_pci_cfg_bus_ops		*ops;
-};
-
-struct gen_pci {
-	struct pci_host_bridge			host;
-	struct gen_pci_cfg_windows		cfg;
-	struct list_head			resources;
-};
+#include "pci-host-common.h"
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
@@ -93,175 +76,19 @@ static const struct of_device_id gen_pci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, gen_pci_of_match);
 
-static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
-{
-	pci_free_resource_list(&pci->resources);
-}
-
-static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
-{
-	int err, res_valid = 0;
-	struct device *dev = pci->host.dev.parent;
-	struct device_node *np = dev->of_node;
-	resource_size_t iobase;
-	struct resource_entry *win;
-
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
-					       &iobase);
-	if (err)
-		return err;
-
-	resource_list_for_each_entry(win, &pci->resources) {
-		struct resource *parent, *res = win->res;
-
-		switch (resource_type(res)) {
-		case IORESOURCE_IO:
-			parent = &ioport_resource;
-			err = pci_remap_iospace(res, iobase);
-			if (err) {
-				dev_warn(dev, "error %d: failed to map resource %pR\n",
-					 err, res);
-				continue;
-			}
-			break;
-		case IORESOURCE_MEM:
-			parent = &iomem_resource;
-			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
-			break;
-		case IORESOURCE_BUS:
-			pci->cfg.bus_range = res;
-		default:
-			continue;
-		}
-
-		err = devm_request_resource(dev, parent, res);
-		if (err)
-			goto out_release_res;
-	}
-
-	if (!res_valid) {
-		dev_err(dev, "non-prefetchable memory resource required\n");
-		err = -EINVAL;
-		goto out_release_res;
-	}
-
-	return 0;
-
-out_release_res:
-	gen_pci_release_of_pci_ranges(pci);
-	return err;
-}
-
-static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
-{
-	int err;
-	u8 bus_max;
-	resource_size_t busn;
-	struct resource *bus_range;
-	struct device *dev = pci->host.dev.parent;
-	struct device_node *np = dev->of_node;
-	u32 sz = 1 << pci->cfg.ops->bus_shift;
-
-	err = of_address_to_resource(np, 0, &pci->cfg.res);
-	if (err) {
-		dev_err(dev, "missing \"reg\" property\n");
-		return err;
-	}
-
-	/* Limit the bus-range to fit within reg */
-	bus_max = pci->cfg.bus_range->start +
-		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
-	pci->cfg.bus_range->end = min_t(resource_size_t,
-					pci->cfg.bus_range->end, bus_max);
-
-	pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
-				    sizeof(*pci->cfg.win), GFP_KERNEL);
-	if (!pci->cfg.win)
-		return -ENOMEM;
-
-	/* Map our Configuration Space windows */
-	if (!devm_request_mem_region(dev, pci->cfg.res.start,
-				     resource_size(&pci->cfg.res),
-				     "Configuration Space"))
-		return -ENOMEM;
-
-	bus_range = pci->cfg.bus_range;
-	for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
-		u32 idx = busn - bus_range->start;
-
-		pci->cfg.win[idx] = devm_ioremap(dev,
-						 pci->cfg.res.start + idx * sz,
-						 sz);
-		if (!pci->cfg.win[idx])
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
-	int err;
-	const char *type;
-	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
+	const struct of_device_id *of_id;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct pci_bus *bus, *child;
 
 	if (!pci)
 		return -ENOMEM;
 
-	type = of_get_property(np, "device_type", NULL);
-	if (!type || strcmp(type, "pci")) {
-		dev_err(dev, "invalid \"device_type\" %s\n", type);
-		return -EINVAL;
-	}
-
-	of_pci_check_probe_only();
-
-	of_id = of_match_node(gen_pci_of_match, np);
+	of_id = of_match_node(gen_pci_of_match, dev->of_node);
 	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
-	pci->host.dev.parent = dev;
-	INIT_LIST_HEAD(&pci->host.windows);
-	INIT_LIST_HEAD(&pci->resources);
-
-	/* Parse our PCI ranges and request their resources */
-	err = gen_pci_parse_request_of_pci_ranges(pci);
-	if (err)
-		return err;
-
-	/* Parse and map our Configuration Space windows */
-	err = gen_pci_parse_map_cfg_windows(pci);
-	if (err) {
-		gen_pci_release_of_pci_ranges(pci);
-		return err;
-	}
-
-	/* Do not reassign resources if probe only */
-	if (!pci_has_flag(PCI_PROBE_ONLY))
-		pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
-
-
-	bus = pci_scan_root_bus(dev, pci->cfg.bus_range->start,
-				&pci->cfg.ops->ops, pci, &pci->resources);
-	if (!bus) {
-		dev_err(dev, "Scanning rootbus failed");
-		return -ENODEV;
-	}
-
-	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		pci_bus_size_bridges(bus);
-		pci_bus_assign_resources(bus);
-
-		list_for_each_entry(child, &bus->children, node)
-			pcie_bus_configure_settings(child);
-	}
 
-	pci_bus_add_devices(bus);
-	return 0;
+	return pci_host_common_probe(pdev, pci);
 }
 
 static struct platform_driver gen_pci_driver = {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v5 1/3] PCI: generic: Refactor code to enable reuse by other drivers.
@ 2016-02-05 23:41   ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Daney <david.daney@cavium.com>

No change in functionality.

Move structure definitions into a separate header file.  Move common
code to new file with Kconfig machinery to build it.  Split probe
function in to two parts:

   - a small driver specific probe function (gen_pci_probe)

   - a common probe that can be used by other drivers
     (pci_host_common_probe)

Signed-off-by: David Daney <david.daney@cavium.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 MAINTAINERS                         |   1 +
 drivers/pci/host/Kconfig            |   4 +
 drivers/pci/host/Makefile           |   1 +
 drivers/pci/host/pci-host-common.c  | 194 ++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pci-host-common.h  |  47 +++++++++
 drivers/pci/host/pci-host-generic.c | 181 +--------------------------------
 6 files changed, 251 insertions(+), 177 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-common.c
 create mode 100644 drivers/pci/host/pci-host-common.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a9010d9..9287929 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8367,6 +8367,7 @@ L:	linux-pci at vger.kernel.org
 L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/host-generic-pci.txt
+F:	drivers/pci/host/pci-host-common.c
 F:	drivers/pci/host/pci-host-generic.c
 
 PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 75a6054..65709b4 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -53,9 +53,13 @@ config PCI_RCAR_GEN2_PCIE
 	help
 	  Say Y here if you want PCIe controller support on R-Car Gen2 SoCs.
 
+config PCI_HOST_COMMON
+	bool
+
 config PCI_HOST_GENERIC
 	bool "Generic PCI host controller"
 	depends on (ARM || ARM64) && OF
+	select PCI_HOST_COMMON
 	help
 	  Say Y here if you want to support a simple generic PCI host
 	  controller, such as the one emulated by kvmtool.
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 7b2f20c..3b24af8 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCI_RCAR_GEN2_PCIE) += pcie-rcar.o
+obj-$(CONFIG_PCI_HOST_COMMON) += pci-host-common.o
 obj-$(CONFIG_PCI_HOST_GENERIC) += pci-host-generic.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
new file mode 100644
index 0000000..e9f850f
--- /dev/null
+++ b/drivers/pci/host/pci-host-common.c
@@ -0,0 +1,194 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
+{
+	pci_free_resource_list(&pci->resources);
+}
+
+static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
+{
+	int err, res_valid = 0;
+	struct device *dev = pci->host.dev.parent;
+	struct device_node *np = dev->of_node;
+	resource_size_t iobase;
+	struct resource_entry *win;
+
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
+					       &iobase);
+	if (err)
+		return err;
+
+	resource_list_for_each_entry(win, &pci->resources) {
+		struct resource *parent, *res = win->res;
+
+		switch (resource_type(res)) {
+		case IORESOURCE_IO:
+			parent = &ioport_resource;
+			err = pci_remap_iospace(res, iobase);
+			if (err) {
+				dev_warn(dev, "error %d: failed to map resource %pR\n",
+					 err, res);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			parent = &iomem_resource;
+			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+			break;
+		case IORESOURCE_BUS:
+			pci->cfg.bus_range = res;
+		default:
+			continue;
+		}
+
+		err = devm_request_resource(dev, parent, res);
+		if (err)
+			goto out_release_res;
+	}
+
+	if (!res_valid) {
+		dev_err(dev, "non-prefetchable memory resource required\n");
+		err = -EINVAL;
+		goto out_release_res;
+	}
+
+	return 0;
+
+out_release_res:
+	gen_pci_release_of_pci_ranges(pci);
+	return err;
+}
+
+static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
+{
+	int err;
+	u8 bus_max;
+	resource_size_t busn;
+	struct resource *bus_range;
+	struct device *dev = pci->host.dev.parent;
+	struct device_node *np = dev->of_node;
+	u32 sz = 1 << pci->cfg.ops->bus_shift;
+
+	err = of_address_to_resource(np, 0, &pci->cfg.res);
+	if (err) {
+		dev_err(dev, "missing \"reg\" property\n");
+		return err;
+	}
+
+	/* Limit the bus-range to fit within reg */
+	bus_max = pci->cfg.bus_range->start +
+		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
+	pci->cfg.bus_range->end = min_t(resource_size_t,
+					pci->cfg.bus_range->end, bus_max);
+
+	pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
+				    sizeof(*pci->cfg.win), GFP_KERNEL);
+	if (!pci->cfg.win)
+		return -ENOMEM;
+
+	/* Map our Configuration Space windows */
+	if (!devm_request_mem_region(dev, pci->cfg.res.start,
+				     resource_size(&pci->cfg.res),
+				     "Configuration Space"))
+		return -ENOMEM;
+
+	bus_range = pci->cfg.bus_range;
+	for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
+		u32 idx = busn - bus_range->start;
+
+		pci->cfg.win[idx] = devm_ioremap(dev,
+						 pci->cfg.res.start + idx * sz,
+						 sz);
+		if (!pci->cfg.win[idx])
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+int pci_host_common_probe(struct platform_device *pdev,
+			  struct gen_pci *pci)
+{
+	int err;
+	const char *type;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct pci_bus *bus, *child;
+
+	type = of_get_property(np, "device_type", NULL);
+	if (!type || strcmp(type, "pci")) {
+		dev_err(dev, "invalid \"device_type\" %s\n", type);
+		return -EINVAL;
+	}
+
+	of_pci_check_probe_only();
+
+	pci->host.dev.parent = dev;
+	INIT_LIST_HEAD(&pci->host.windows);
+	INIT_LIST_HEAD(&pci->resources);
+
+	/* Parse our PCI ranges and request their resources */
+	err = gen_pci_parse_request_of_pci_ranges(pci);
+	if (err)
+		return err;
+
+	/* Parse and map our Configuration Space windows */
+	err = gen_pci_parse_map_cfg_windows(pci);
+	if (err) {
+		gen_pci_release_of_pci_ranges(pci);
+		return err;
+	}
+
+	/* Do not reassign resources if probe only */
+	if (!pci_has_flag(PCI_PROBE_ONLY))
+		pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
+
+
+	bus = pci_scan_root_bus(dev, pci->cfg.bus_range->start,
+				&pci->cfg.ops->ops, pci, &pci->resources);
+	if (!bus) {
+		dev_err(dev, "Scanning rootbus failed");
+		return -ENODEV;
+	}
+
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
+	if (!pci_has_flag(PCI_PROBE_ONLY)) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+
+		list_for_each_entry(child, &bus->children, node)
+			pcie_bus_configure_settings(child);
+	}
+
+	pci_bus_add_devices(bus);
+	return 0;
+}
+
+MODULE_DESCRIPTION("Generic PCI host driver common code");
+MODULE_AUTHOR("Will Deacon <will.deacon@arm.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/host/pci-host-common.h b/drivers/pci/host/pci-host-common.h
new file mode 100644
index 0000000..09f3fa0
--- /dev/null
+++ b/drivers/pci/host/pci-host-common.h
@@ -0,0 +1,47 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2014 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#ifndef _PCI_HOST_COMMON_H
+#define _PCI_HOST_COMMON_H
+
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+
+struct gen_pci_cfg_bus_ops {
+	u32 bus_shift;
+	struct pci_ops ops;
+};
+
+struct gen_pci_cfg_windows {
+	struct resource				res;
+	struct resource				*bus_range;
+	void __iomem				**win;
+
+	struct gen_pci_cfg_bus_ops		*ops;
+};
+
+struct gen_pci {
+	struct pci_host_bridge			host;
+	struct gen_pci_cfg_windows		cfg;
+	struct list_head			resources;
+};
+
+int pci_host_common_probe(struct platform_device *pdev,
+			  struct gen_pci *pci);
+
+#endif /* _PCI_HOST_COMMON_H */
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 1652bc7..e8aa78f 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -25,24 +25,7 @@
 #include <linux/of_pci.h>
 #include <linux/platform_device.h>
 
-struct gen_pci_cfg_bus_ops {
-	u32 bus_shift;
-	struct pci_ops ops;
-};
-
-struct gen_pci_cfg_windows {
-	struct resource				res;
-	struct resource				*bus_range;
-	void __iomem				**win;
-
-	struct gen_pci_cfg_bus_ops		*ops;
-};
-
-struct gen_pci {
-	struct pci_host_bridge			host;
-	struct gen_pci_cfg_windows		cfg;
-	struct list_head			resources;
-};
+#include "pci-host-common.h"
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
@@ -93,175 +76,19 @@ static const struct of_device_id gen_pci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, gen_pci_of_match);
 
-static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
-{
-	pci_free_resource_list(&pci->resources);
-}
-
-static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
-{
-	int err, res_valid = 0;
-	struct device *dev = pci->host.dev.parent;
-	struct device_node *np = dev->of_node;
-	resource_size_t iobase;
-	struct resource_entry *win;
-
-	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources,
-					       &iobase);
-	if (err)
-		return err;
-
-	resource_list_for_each_entry(win, &pci->resources) {
-		struct resource *parent, *res = win->res;
-
-		switch (resource_type(res)) {
-		case IORESOURCE_IO:
-			parent = &ioport_resource;
-			err = pci_remap_iospace(res, iobase);
-			if (err) {
-				dev_warn(dev, "error %d: failed to map resource %pR\n",
-					 err, res);
-				continue;
-			}
-			break;
-		case IORESOURCE_MEM:
-			parent = &iomem_resource;
-			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
-			break;
-		case IORESOURCE_BUS:
-			pci->cfg.bus_range = res;
-		default:
-			continue;
-		}
-
-		err = devm_request_resource(dev, parent, res);
-		if (err)
-			goto out_release_res;
-	}
-
-	if (!res_valid) {
-		dev_err(dev, "non-prefetchable memory resource required\n");
-		err = -EINVAL;
-		goto out_release_res;
-	}
-
-	return 0;
-
-out_release_res:
-	gen_pci_release_of_pci_ranges(pci);
-	return err;
-}
-
-static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
-{
-	int err;
-	u8 bus_max;
-	resource_size_t busn;
-	struct resource *bus_range;
-	struct device *dev = pci->host.dev.parent;
-	struct device_node *np = dev->of_node;
-	u32 sz = 1 << pci->cfg.ops->bus_shift;
-
-	err = of_address_to_resource(np, 0, &pci->cfg.res);
-	if (err) {
-		dev_err(dev, "missing \"reg\" property\n");
-		return err;
-	}
-
-	/* Limit the bus-range to fit within reg */
-	bus_max = pci->cfg.bus_range->start +
-		  (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1;
-	pci->cfg.bus_range->end = min_t(resource_size_t,
-					pci->cfg.bus_range->end, bus_max);
-
-	pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range),
-				    sizeof(*pci->cfg.win), GFP_KERNEL);
-	if (!pci->cfg.win)
-		return -ENOMEM;
-
-	/* Map our Configuration Space windows */
-	if (!devm_request_mem_region(dev, pci->cfg.res.start,
-				     resource_size(&pci->cfg.res),
-				     "Configuration Space"))
-		return -ENOMEM;
-
-	bus_range = pci->cfg.bus_range;
-	for (busn = bus_range->start; busn <= bus_range->end; ++busn) {
-		u32 idx = busn - bus_range->start;
-
-		pci->cfg.win[idx] = devm_ioremap(dev,
-						 pci->cfg.res.start + idx * sz,
-						 sz);
-		if (!pci->cfg.win[idx])
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
 static int gen_pci_probe(struct platform_device *pdev)
 {
-	int err;
-	const char *type;
-	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
+	const struct of_device_id *of_id;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct pci_bus *bus, *child;
 
 	if (!pci)
 		return -ENOMEM;
 
-	type = of_get_property(np, "device_type", NULL);
-	if (!type || strcmp(type, "pci")) {
-		dev_err(dev, "invalid \"device_type\" %s\n", type);
-		return -EINVAL;
-	}
-
-	of_pci_check_probe_only();
-
-	of_id = of_match_node(gen_pci_of_match, np);
+	of_id = of_match_node(gen_pci_of_match, dev->of_node);
 	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
-	pci->host.dev.parent = dev;
-	INIT_LIST_HEAD(&pci->host.windows);
-	INIT_LIST_HEAD(&pci->resources);
-
-	/* Parse our PCI ranges and request their resources */
-	err = gen_pci_parse_request_of_pci_ranges(pci);
-	if (err)
-		return err;
-
-	/* Parse and map our Configuration Space windows */
-	err = gen_pci_parse_map_cfg_windows(pci);
-	if (err) {
-		gen_pci_release_of_pci_ranges(pci);
-		return err;
-	}
-
-	/* Do not reassign resources if probe only */
-	if (!pci_has_flag(PCI_PROBE_ONLY))
-		pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
-
-
-	bus = pci_scan_root_bus(dev, pci->cfg.bus_range->start,
-				&pci->cfg.ops->ops, pci, &pci->resources);
-	if (!bus) {
-		dev_err(dev, "Scanning rootbus failed");
-		return -ENODEV;
-	}
-
-	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
-	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		pci_bus_size_bridges(bus);
-		pci_bus_assign_resources(bus);
-
-		list_for_each_entry(child, &bus->children, node)
-			pcie_bus_configure_settings(child);
-	}
 
-	pci_bus_add_devices(bus);
-	return 0;
+	return pci_host_common_probe(pdev, pci);
 }
 
 static struct platform_driver gen_pci_driver = {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
  2016-02-05 23:41 ` David Daney
@ 2016-02-05 23:41   ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree
  Cc: linux-kernel, David Daney

From: David Daney <david.daney@cavium.com>

The root complexes used to access off-chip PCIe devices (called PEM
units in the hardware manuals) on some Cavium ThunderX processors
require quirky access methods for the config space of the PCIe bridge.
Add a driver to provide these config space accessor functions.  The
pci-host-common code is used to configure the PCI machinery.

Signed-off-by: David Daney <david.daney@cavium.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 +++
 MAINTAINERS                                        |   8 +
 drivers/pci/host/Kconfig                           |   7 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-thunder-pem.c                 | 332 +++++++++++++++++++++
 5 files changed, 391 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-thunder-pem.c

diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
new file mode 100644
index 0000000..f131fae
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
@@ -0,0 +1,43 @@
+* ThunderX PEM PCIe host controller
+
+Firmware-initialized PCI host controller found on some Cavium
+ThunderX processors.
+
+The properties and their meanings are identical to those described in
+host-generic-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-generic-pci.txt:
+
+- compatible     : Must be "cavium,pci-host-thunder-pem"
+
+- reg            : Two entries: First the configuration space for down
+                   stream devices base address and size, as accessed
+                   from the parent bus. Second, the register bank of
+                   the PEM device PCIe bridge.
+
+Example:
+
+    pci@87e0,c2000000 {
+	compatible = "cavium,pci-host-thunder-pem";
+	device_type = "pci";
+	msi-parent = <&its>;
+	msi-map = <0 &its 0x10000 0x10000>;
+	bus-range = <0x8f 0xc7>;
+	#size-cells = <2>;
+	#address-cells = <3>;
+
+	reg = <0x8880 0x8f000000 0x0 0x39000000>,  /* Configuration space */
+	      <0x87e0 0xc2000000 0x0 0x00010000>; /* PEM space */
+	ranges = <0x01000000 0x00 0x00020000 0x88b0 0x00020000 0x00 0x00010000>, /* I/O */
+		 <0x03000000 0x00 0x10000000 0x8890 0x10000000 0x0f 0xf0000000>, /* mem64 */
+		 <0x43000000 0x10 0x00000000 0x88a0 0x00000000 0x10 0x00000000>, /* mem64-pref */
+		 <0x03000000 0x87e0 0xc2f00000 0x87e0 0xc2000000 0x00 0x00100000>; /* mem64 PEM BAR4 */
+
+	#interrupt-cells = <1>;
+	interrupt-map-mask = <0 0 0 7>;
+	interrupt-map = <0 0 0 1 &gic0 0 0 0 24 4>, /* INTA */
+			<0 0 0 2 &gic0 0 0 0 25 4>, /* INTB */
+			<0 0 0 3 &gic0 0 0 0 26 4>, /* INTC */
+			<0 0 0 4 &gic0 0 0 0 27 4>; /* INTD */
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9287929..ff5c367 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8413,6 +8413,14 @@ L:     linux-arm-msm@vger.kernel.org
 S:     Maintained
 F:     drivers/pci/host/*qcom*
 
+PCIE DRIVER FOR CAVIUM THUNDERX
+M:	David Daney <david.daney@cavium.com>
+L:	linux-pci@vger.kernel.org
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Supported
+F:	Documentation/devicetree/bindings/pci/pci-thunder-*
+F:	drivers/pci/host/pci-thunder-*
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia@lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 65709b4..184df22 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -195,4 +195,11 @@ config PCIE_QCOM
 	  PCIe controller uses the Designware core plus Qualcomm-specific
 	  hardware wrappers.
 
+config PCI_HOST_THUNDER_PEM
+	bool "Cavium Thunder PCIe controller to off-chip devices"
+	depends on OF && ARM64
+	select PCI_HOST_COMMON
+	help
+	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 3b24af8..8903172 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
+obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
new file mode 100644
index 0000000..e3a6554
--- /dev/null
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -0,0 +1,332 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2015 - 2016 Cavium, Inc.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+#define PEM_CFG_WR 0x28
+#define PEM_CFG_RD 0x30
+
+struct thunder_pem_pci {
+	struct gen_pci	gen_pci;
+	u32		ea_entry[3];
+	void __iomem	*pem_reg_base;
+};
+
+static void __iomem *thunder_pem_map_bus(struct pci_bus *bus,
+					 unsigned int devfn, int where)
+{
+	struct gen_pci *pci = bus->sysdata;
+	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
+
+	return pci->cfg.win[idx] + ((devfn << 16) | where);
+}
+
+static int thunder_pem_config_read(struct pci_bus *bus, unsigned int devfn,
+				   int where, int size, u32 *val)
+{
+	int r;
+	struct thunder_pem_pci *pem_pci;
+	struct gen_pci *pci = bus->sysdata;
+
+	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
+
+	/*
+	 * The first device on the bus in the PEM PCIe bridge.
+	 * Special case its config access.
+	 */
+	if (bus->number == pci->cfg.bus_range->start) {
+		u64 read_val;
+
+		if (devfn != 0 || where >= 2048) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+
+		/*
+		 * 32-bit accesses only.  Write the address to the low
+		 * order bits of PEM_CFG_RD, then trigger the read by
+		 * reading back.  The config data lands in the upper
+		 * 32-bits of PEM_CFG_RD.
+		 */
+		read_val = where & ~3ull;
+		writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+		read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+		read_val >>= 32;
+
+		/*
+		 * The config space contains some garbage, fix it up.
+		 * Also synthesize an EA capability for the BAR used
+		 * by MSI-X.
+		 */
+		switch (where & ~3u) {
+		case 0x40:
+			read_val &= 0xffff00ff;
+			read_val |= 0x00007000; /* Skip MSI CAP */
+			break;
+		case 0x70: /* Express Cap */
+			/* PME interrupt on vector 2*/
+			read_val |= (2u << 25);
+			break;
+		case 0xb0: /* MSI-X Cap */
+			/* TableSize=4, Next Cap is EA */
+			read_val &= 0xc00000ff;
+			read_val |= 0x0003bc00;
+			break;
+		case 0xb4:
+			/* Table offset=0, BIR=0 */
+			read_val = 0x00000000;
+			break;
+		case 0xb8:
+			/* BPA offset=0xf0000, BIR=0 */
+			read_val = 0x000f0000;
+			break;
+		case 0xbc:
+			/* EA, 1 entry, no next Cap */
+			read_val = 0x00010014;
+			break;
+		case 0xc0:
+			/* DW2 for type-1 */
+			read_val = 0x00000000;
+			break;
+		case 0xc4:
+			/* Entry BEI=0, PP=0x00, SP=0xff, ES=3 */
+			read_val = 0x80ff0003;
+			break;
+		case 0xc8:
+			read_val = pem_pci->ea_entry[0];
+			break;
+		case 0xcc:
+			read_val = pem_pci->ea_entry[1];
+			break;
+		case 0xd0:
+			read_val = pem_pci->ea_entry[2];
+			break;
+		default:
+			break;
+		}
+		read_val >>= (8 * (where & 3));
+		switch (size) {
+		case 1:
+			read_val &= 0xff;
+			break;
+		case 2:
+			read_val &= 0xffff;
+			break;
+		default:
+			break;
+		}
+		*val = read_val;
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (bus->number < pci->cfg.bus_range->start ||
+	    bus->number > pci->cfg.bus_range->end)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	r = pci_generic_config_read(bus, devfn, where, size, val);
+	return r;
+}
+
+static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 val)
+{
+	struct gen_pci *pci = bus->sysdata;
+	struct thunder_pem_pci *pem_pci;
+
+	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
+
+	/*
+	 * The first device on the bus in the PEM PCIe bridge.
+	 * Special case its config access.
+	 */
+	if (bus->number == pci->cfg.bus_range->start) {
+		u64 write_val, read_val;
+		u32 mask = 0;
+
+		if (devfn != 0 || where >= 2048)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+
+		/*
+		 * 32-bit accesses only.  If the write is for a size
+		 * smaller than 32-bits, we must first read the 32-bit
+		 * value and merge in the desired bits and then write
+		 * the whole 32-bits back out.
+		 */
+		switch (size) {
+		case 1:
+			read_val = where & ~3ull;
+			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val >>= 32;
+			mask = ~(0xff << (8 * (where & 3)));
+			read_val &= mask;
+			val = (val & 0xff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		case 2:
+			read_val = where & ~3ull;
+			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val >>= 32;
+			mask = ~(0xffff << (8 * (where & 3)));
+			read_val &= mask;
+			val = (val & 0xffff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		default:
+			break;
+
+		}
+
+		if (mask) {
+			/*
+			 * By expanding the write width to 32 bits, we
+			 * may inadvertently hit some W1C bits that
+			 * were not intended to be written.  Mask
+			 * these out.
+			 *
+			 * Some of the w1c_bits below also include
+			 * read-only or non-writable reserved bits,
+			 * this makes the code simpler and is OK as
+			 * the bits are not affected by writing zeros
+			 * to them.
+			 */
+			u32 w1c_bits = 0;
+
+			switch (where & 3) {
+			case 0x04: /* Command/Status */
+			case 0x1c: /* Base and I/O Limit/Secondary Status */
+				w1c_bits = 0xff000000;
+				break;
+			case 0x44: /* Power Management Control and Status */
+				w1c_bits = 0xfffffe00;
+				break;
+			case 0x78: /* Device Control/Device Status */
+			case 0x80: /* Link Control/Link Status */
+			case 0x88: /* Slot Control/Slot Status */
+			case 0x90: /* Root Status */
+			case 0xa0: /* Link Control 2 Registers/Link Status 2 */
+				w1c_bits = 0xffff0000;
+				break;
+			case 0x104: /* Uncorrectable Error Status */
+			case 0x110: /* Correctable Error Status */
+			case 0x130: /* Error Status */
+			case 0x160: /* Link Control 4 */
+				w1c_bits = 0xffffffff;
+				break;
+			default:
+				break;
+
+			}
+			if (w1c_bits) {
+				mask &= w1c_bits;
+				val &= ~mask;
+			}
+		}
+
+		/*
+		 * Low order bits are the config address, the high
+		 * order 32 bits are the data to be written.
+		 */
+		write_val = where & ~3ull;
+		write_val |= (((u64)val) << 32);
+		writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (bus->number < pci->cfg.bus_range->start ||
+	    bus->number > pci->cfg.bus_range->end)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static struct gen_pci_cfg_bus_ops thunder_pem_bus_ops = {
+	.bus_shift	= 24,
+	.ops		= {
+		.map_bus	= thunder_pem_map_bus,
+		.read		= thunder_pem_config_read,
+		.write		= thunder_pem_config_write,
+	}
+};
+
+static const struct of_device_id thunder_pem_of_match[] = {
+	{ .compatible = "cavium,pci-host-thunder-pem",
+	  .data = &thunder_pem_bus_ops },
+
+	{ },
+};
+MODULE_DEVICE_TABLE(of, thunder_pem_of_match);
+
+static int thunder_pem_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	resource_size_t bar4_start;
+	struct resource *res_pem;
+	struct thunder_pem_pci *pem_pci;
+
+	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
+	if (!pem_pci)
+		return -ENOMEM;
+
+	of_id = of_match_node(thunder_pem_of_match, dev->of_node);
+	pem_pci->gen_pci.cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	/*
+	 * The second register range is the PEM bridge to the PCIe
+	 * bus.  It has a different config access method than those
+	 * devices behind the bridge.
+	 */
+	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res_pem) {
+		dev_err(dev, "missing \"reg[1]\"property\n");
+		return -EINVAL;
+	}
+
+	pem_pci->pem_reg_base = devm_ioremap(dev, res_pem->start, 0x10000);
+	if (!pem_pci->pem_reg_base)
+		return -ENOMEM;
+
+	/*
+	 * The MSI-X BAR for the PEM and AER interrupts is located at
+	 * a fixed offset from the PEM register base.  Generate a
+	 * fragment of the synthesized Enhanced Allocation capability
+	 * structure here for the BAR.
+	 */
+	bar4_start = res_pem->start + 0xf00000;
+	pem_pci->ea_entry[0] = (u32)bar4_start | 2;
+	pem_pci->ea_entry[1] = (u32)(res_pem->end - bar4_start) & ~3u;
+	pem_pci->ea_entry[2] = (u32)(bar4_start >> 32);
+
+	return pci_host_common_probe(pdev, &pem_pci->gen_pci);
+}
+
+static struct platform_driver thunder_pem_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = thunder_pem_of_match,
+	},
+	.probe = thunder_pem_probe,
+};
+module_platform_driver(thunder_pem_driver);
+
+MODULE_DESCRIPTION("Thunder PEM PCIe host driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
@ 2016-02-05 23:41   ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Daney <david.daney@cavium.com>

The root complexes used to access off-chip PCIe devices (called PEM
units in the hardware manuals) on some Cavium ThunderX processors
require quirky access methods for the config space of the PCIe bridge.
Add a driver to provide these config space accessor functions.  The
pci-host-common code is used to configure the PCI machinery.

Signed-off-by: David Daney <david.daney@cavium.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 .../devicetree/bindings/pci/pci-thunder-pem.txt    |  43 +++
 MAINTAINERS                                        |   8 +
 drivers/pci/host/Kconfig                           |   7 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-thunder-pem.c                 | 332 +++++++++++++++++++++
 5 files changed, 391 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-thunder-pem.c

diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
new file mode 100644
index 0000000..f131fae
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-thunder-pem.txt
@@ -0,0 +1,43 @@
+* ThunderX PEM PCIe host controller
+
+Firmware-initialized PCI host controller found on some Cavium
+ThunderX processors.
+
+The properties and their meanings are identical to those described in
+host-generic-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-generic-pci.txt:
+
+- compatible     : Must be "cavium,pci-host-thunder-pem"
+
+- reg            : Two entries: First the configuration space for down
+                   stream devices base address and size, as accessed
+                   from the parent bus. Second, the register bank of
+                   the PEM device PCIe bridge.
+
+Example:
+
+    pci at 87e0,c2000000 {
+	compatible = "cavium,pci-host-thunder-pem";
+	device_type = "pci";
+	msi-parent = <&its>;
+	msi-map = <0 &its 0x10000 0x10000>;
+	bus-range = <0x8f 0xc7>;
+	#size-cells = <2>;
+	#address-cells = <3>;
+
+	reg = <0x8880 0x8f000000 0x0 0x39000000>,  /* Configuration space */
+	      <0x87e0 0xc2000000 0x0 0x00010000>; /* PEM space */
+	ranges = <0x01000000 0x00 0x00020000 0x88b0 0x00020000 0x00 0x00010000>, /* I/O */
+		 <0x03000000 0x00 0x10000000 0x8890 0x10000000 0x0f 0xf0000000>, /* mem64 */
+		 <0x43000000 0x10 0x00000000 0x88a0 0x00000000 0x10 0x00000000>, /* mem64-pref */
+		 <0x03000000 0x87e0 0xc2f00000 0x87e0 0xc2000000 0x00 0x00100000>; /* mem64 PEM BAR4 */
+
+	#interrupt-cells = <1>;
+	interrupt-map-mask = <0 0 0 7>;
+	interrupt-map = <0 0 0 1 &gic0 0 0 0 24 4>, /* INTA */
+			<0 0 0 2 &gic0 0 0 0 25 4>, /* INTB */
+			<0 0 0 3 &gic0 0 0 0 26 4>, /* INTC */
+			<0 0 0 4 &gic0 0 0 0 27 4>; /* INTD */
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9287929..ff5c367 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8413,6 +8413,14 @@ L:     linux-arm-msm at vger.kernel.org
 S:     Maintained
 F:     drivers/pci/host/*qcom*
 
+PCIE DRIVER FOR CAVIUM THUNDERX
+M:	David Daney <david.daney@cavium.com>
+L:	linux-pci at vger.kernel.org
+L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
+S:	Supported
+F:	Documentation/devicetree/bindings/pci/pci-thunder-*
+F:	drivers/pci/host/pci-thunder-*
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia at lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 65709b4..184df22 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -195,4 +195,11 @@ config PCIE_QCOM
 	  PCIe controller uses the Designware core plus Qualcomm-specific
 	  hardware wrappers.
 
+config PCI_HOST_THUNDER_PEM
+	bool "Cavium Thunder PCIe controller to off-chip devices"
+	depends on OF && ARM64
+	select PCI_HOST_COMMON
+	help
+	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 3b24af8..8903172 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
+obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
new file mode 100644
index 0000000..e3a6554
--- /dev/null
+++ b/drivers/pci/host/pci-thunder-pem.c
@@ -0,0 +1,332 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright (C) 2015 - 2016 Cavium, Inc.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+#define PEM_CFG_WR 0x28
+#define PEM_CFG_RD 0x30
+
+struct thunder_pem_pci {
+	struct gen_pci	gen_pci;
+	u32		ea_entry[3];
+	void __iomem	*pem_reg_base;
+};
+
+static void __iomem *thunder_pem_map_bus(struct pci_bus *bus,
+					 unsigned int devfn, int where)
+{
+	struct gen_pci *pci = bus->sysdata;
+	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
+
+	return pci->cfg.win[idx] + ((devfn << 16) | where);
+}
+
+static int thunder_pem_config_read(struct pci_bus *bus, unsigned int devfn,
+				   int where, int size, u32 *val)
+{
+	int r;
+	struct thunder_pem_pci *pem_pci;
+	struct gen_pci *pci = bus->sysdata;
+
+	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
+
+	/*
+	 * The first device on the bus in the PEM PCIe bridge.
+	 * Special case its config access.
+	 */
+	if (bus->number == pci->cfg.bus_range->start) {
+		u64 read_val;
+
+		if (devfn != 0 || where >= 2048) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+
+		/*
+		 * 32-bit accesses only.  Write the address to the low
+		 * order bits of PEM_CFG_RD, then trigger the read by
+		 * reading back.  The config data lands in the upper
+		 * 32-bits of PEM_CFG_RD.
+		 */
+		read_val = where & ~3ull;
+		writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+		read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+		read_val >>= 32;
+
+		/*
+		 * The config space contains some garbage, fix it up.
+		 * Also synthesize an EA capability for the BAR used
+		 * by MSI-X.
+		 */
+		switch (where & ~3u) {
+		case 0x40:
+			read_val &= 0xffff00ff;
+			read_val |= 0x00007000; /* Skip MSI CAP */
+			break;
+		case 0x70: /* Express Cap */
+			/* PME interrupt on vector 2*/
+			read_val |= (2u << 25);
+			break;
+		case 0xb0: /* MSI-X Cap */
+			/* TableSize=4, Next Cap is EA */
+			read_val &= 0xc00000ff;
+			read_val |= 0x0003bc00;
+			break;
+		case 0xb4:
+			/* Table offset=0, BIR=0 */
+			read_val = 0x00000000;
+			break;
+		case 0xb8:
+			/* BPA offset=0xf0000, BIR=0 */
+			read_val = 0x000f0000;
+			break;
+		case 0xbc:
+			/* EA, 1 entry, no next Cap */
+			read_val = 0x00010014;
+			break;
+		case 0xc0:
+			/* DW2 for type-1 */
+			read_val = 0x00000000;
+			break;
+		case 0xc4:
+			/* Entry BEI=0, PP=0x00, SP=0xff, ES=3 */
+			read_val = 0x80ff0003;
+			break;
+		case 0xc8:
+			read_val = pem_pci->ea_entry[0];
+			break;
+		case 0xcc:
+			read_val = pem_pci->ea_entry[1];
+			break;
+		case 0xd0:
+			read_val = pem_pci->ea_entry[2];
+			break;
+		default:
+			break;
+		}
+		read_val >>= (8 * (where & 3));
+		switch (size) {
+		case 1:
+			read_val &= 0xff;
+			break;
+		case 2:
+			read_val &= 0xffff;
+			break;
+		default:
+			break;
+		}
+		*val = read_val;
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (bus->number < pci->cfg.bus_range->start ||
+	    bus->number > pci->cfg.bus_range->end)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	r = pci_generic_config_read(bus, devfn, where, size, val);
+	return r;
+}
+
+static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 val)
+{
+	struct gen_pci *pci = bus->sysdata;
+	struct thunder_pem_pci *pem_pci;
+
+	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
+
+	/*
+	 * The first device on the bus in the PEM PCIe bridge.
+	 * Special case its config access.
+	 */
+	if (bus->number == pci->cfg.bus_range->start) {
+		u64 write_val, read_val;
+		u32 mask = 0;
+
+		if (devfn != 0 || where >= 2048)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+
+		/*
+		 * 32-bit accesses only.  If the write is for a size
+		 * smaller than 32-bits, we must first read the 32-bit
+		 * value and merge in the desired bits and then write
+		 * the whole 32-bits back out.
+		 */
+		switch (size) {
+		case 1:
+			read_val = where & ~3ull;
+			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val >>= 32;
+			mask = ~(0xff << (8 * (where & 3)));
+			read_val &= mask;
+			val = (val & 0xff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		case 2:
+			read_val = where & ~3ull;
+			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
+			read_val >>= 32;
+			mask = ~(0xffff << (8 * (where & 3)));
+			read_val &= mask;
+			val = (val & 0xffff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		default:
+			break;
+
+		}
+
+		if (mask) {
+			/*
+			 * By expanding the write width to 32 bits, we
+			 * may inadvertently hit some W1C bits that
+			 * were not intended to be written.  Mask
+			 * these out.
+			 *
+			 * Some of the w1c_bits below also include
+			 * read-only or non-writable reserved bits,
+			 * this makes the code simpler and is OK as
+			 * the bits are not affected by writing zeros
+			 * to them.
+			 */
+			u32 w1c_bits = 0;
+
+			switch (where & 3) {
+			case 0x04: /* Command/Status */
+			case 0x1c: /* Base and I/O Limit/Secondary Status */
+				w1c_bits = 0xff000000;
+				break;
+			case 0x44: /* Power Management Control and Status */
+				w1c_bits = 0xfffffe00;
+				break;
+			case 0x78: /* Device Control/Device Status */
+			case 0x80: /* Link Control/Link Status */
+			case 0x88: /* Slot Control/Slot Status */
+			case 0x90: /* Root Status */
+			case 0xa0: /* Link Control 2 Registers/Link Status 2 */
+				w1c_bits = 0xffff0000;
+				break;
+			case 0x104: /* Uncorrectable Error Status */
+			case 0x110: /* Correctable Error Status */
+			case 0x130: /* Error Status */
+			case 0x160: /* Link Control 4 */
+				w1c_bits = 0xffffffff;
+				break;
+			default:
+				break;
+
+			}
+			if (w1c_bits) {
+				mask &= w1c_bits;
+				val &= ~mask;
+			}
+		}
+
+		/*
+		 * Low order bits are the config address, the high
+		 * order 32 bits are the data to be written.
+		 */
+		write_val = where & ~3ull;
+		write_val |= (((u64)val) << 32);
+		writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (bus->number < pci->cfg.bus_range->start ||
+	    bus->number > pci->cfg.bus_range->end)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static struct gen_pci_cfg_bus_ops thunder_pem_bus_ops = {
+	.bus_shift	= 24,
+	.ops		= {
+		.map_bus	= thunder_pem_map_bus,
+		.read		= thunder_pem_config_read,
+		.write		= thunder_pem_config_write,
+	}
+};
+
+static const struct of_device_id thunder_pem_of_match[] = {
+	{ .compatible = "cavium,pci-host-thunder-pem",
+	  .data = &thunder_pem_bus_ops },
+
+	{ },
+};
+MODULE_DEVICE_TABLE(of, thunder_pem_of_match);
+
+static int thunder_pem_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	resource_size_t bar4_start;
+	struct resource *res_pem;
+	struct thunder_pem_pci *pem_pci;
+
+	pem_pci = devm_kzalloc(dev, sizeof(*pem_pci), GFP_KERNEL);
+	if (!pem_pci)
+		return -ENOMEM;
+
+	of_id = of_match_node(thunder_pem_of_match, dev->of_node);
+	pem_pci->gen_pci.cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	/*
+	 * The second register range is the PEM bridge to the PCIe
+	 * bus.  It has a different config access method than those
+	 * devices behind the bridge.
+	 */
+	res_pem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res_pem) {
+		dev_err(dev, "missing \"reg[1]\"property\n");
+		return -EINVAL;
+	}
+
+	pem_pci->pem_reg_base = devm_ioremap(dev, res_pem->start, 0x10000);
+	if (!pem_pci->pem_reg_base)
+		return -ENOMEM;
+
+	/*
+	 * The MSI-X BAR for the PEM and AER interrupts is located at
+	 * a fixed offset from the PEM register base.  Generate a
+	 * fragment of the synthesized Enhanced Allocation capability
+	 * structure here for the BAR.
+	 */
+	bar4_start = res_pem->start + 0xf00000;
+	pem_pci->ea_entry[0] = (u32)bar4_start | 2;
+	pem_pci->ea_entry[1] = (u32)(res_pem->end - bar4_start) & ~3u;
+	pem_pci->ea_entry[2] = (u32)(bar4_start >> 32);
+
+	return pci_host_common_probe(pdev, &pem_pci->gen_pci);
+}
+
+static struct platform_driver thunder_pem_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = thunder_pem_of_match,
+	},
+	.probe = thunder_pem_probe,
+};
+module_platform_driver(thunder_pem_driver);
+
+MODULE_DESCRIPTION("Thunder PEM PCIe host driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-05 23:41 ` David Daney
@ 2016-02-05 23:41   ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree
  Cc: linux-kernel, David Daney

From: David Daney <david.daney@cavium.com>

The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
complexes.  These root complexes (loosely referred to as ECAM units in
the hardware manuals) are used to access the Thunder on-chips devices.
They are special in that all the BARs on devices behind these root
complexes are at fixed addresses.  To handle this in a manner
compatible with the core PCI code, we have the config access functions
synthesize Enhanced Allocation (EA) capability entries for each BAR.

Since this EA synthesis is needed for exactly one chip model, we can
hard code some assumptions about the device topology and the
properties of specific DEVFNs in the driver.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
 drivers/pci/host/Kconfig                           |   7 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
 4 files changed, 396 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
 create mode 100644 drivers/pci/host/pci-thunder-ecam.c

diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
new file mode 100644
index 0000000..34658f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
@@ -0,0 +1,30 @@
+* ThunderX PCI host controller for pass-1.x silicon
+
+Firmware-initialized PCI host controller to on-chip devices found on
+some Cavium ThunderX processors.  These devices have ECAM based config
+access, but the BARs are all at fixed addresses.  We handle the fixed
+addresses by synthesizing Enhanced Allocation (EA) capabilities for
+these devices.
+
+The properties and their meanings are identical to those described in
+host-generic-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-generic-pci.txt:
+
+- compatible     : Must be "cavium,pci-host-thunder-ecam"
+
+Example:
+
+	pci@84b0,00000000 {
+		compatible = "cavium,pci-host-thunder-ecam";
+		device_type = "pci";
+		msi-parent = <&its>;
+		msi-map = <0 &its 0x30000 0x10000>;
+		bus-range = <0 31>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		#stream-id-cells = <1>;
+		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
+	};
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 184df22..f8912c6 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -202,4 +202,11 @@ config PCI_HOST_THUNDER_PEM
 	help
 	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
 
+config PCI_HOST_THUNDER_ECAM
+	bool "Cavium Thunder ECAMe controller to on-chip devices on pass-1.x silicon"
+	depends on OF && ARM64
+	select PCI_HOST_COMMON
+	help
+	  Say Y here if you want ECAM support for CN88XX-Pass-1.x Cavium Thunder SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8903172..d6af3ba 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -23,4 +23,5 @@ obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
+obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
diff --git a/drivers/pci/host/pci-thunder-ecam.c b/drivers/pci/host/pci-thunder-ecam.c
new file mode 100644
index 0000000..83ee590
--- /dev/null
+++ b/drivers/pci/host/pci-thunder-ecam.c
@@ -0,0 +1,358 @@
+/*
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2015 Cavium, Inc.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/of_pci.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+/* Mapping is standard ECAM */
+static void __iomem *thunder_ecam_map_bus(struct pci_bus *bus,
+					  unsigned int devfn,
+					  int where)
+{
+	struct gen_pci *pci = bus->sysdata;
+	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
+
+	return pci->cfg.win[idx] + ((devfn << 12) | where);
+}
+
+static void set_val(u32 v, int where, int size, u32 *val)
+{
+	int shift = (where & 3) * 8;
+
+	pr_debug("set_val %04x: %08x\n", (unsigned)(where & ~3), v);
+	v >>= shift;
+	if (size == 1)
+		v &= 0xff;
+	else if (size == 2)
+		v &= 0xffff;
+	*val = v;
+}
+
+static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus,
+			 unsigned int devfn, int where, int size, u32 *val)
+{
+	void __iomem *addr;
+	u32 v;
+	/*
+	 * Each entry is 16-byte aligned bits[2,3] select which word
+	 * in the entry
+	 */
+	int where_a = where & 0xc;
+
+	if (where_a == 0) {
+		set_val(e0, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0x4) {
+		addr = bus->ops->map_bus(bus, devfn, bar); /* BAR 0 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		v = readl(addr);
+		v &= ~0xf;
+		v |= 2; /* EA entry-1. Base-L */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0x8) {
+		u32 barl_orig;
+		u32 barl_rb;
+
+		addr = bus->ops->map_bus(bus, devfn, bar); /* BAR 0 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		barl_orig = readl(addr + 0);
+		writel(0xffffffff, addr + 0);
+		barl_rb = readl(addr + 0);
+		writel(barl_orig, addr + 0);
+		/* zeros in unsettable bits. */
+		v = ~barl_rb & ~3;
+		v |= 0xc; /* EA entry-2. Offset-L */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0xc) {
+		addr = bus->ops->map_bus(bus, devfn, bar + 4); /* BAR 1 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		v = readl(addr); /* EA entry-3. Base-H */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	return PCIBIOS_DEVICE_NOT_FOUND;
+}
+
+static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 *val)
+{
+	u32 v;
+	u32 vendor_device;
+	void __iomem *addr;
+	int cfg_type;
+	int where_a = where & ~3;
+
+
+	addr = bus->ops->map_bus(bus, devfn, 0xc);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	v = readl(addr);
+
+	/* Check for non type-00 header. */
+	cfg_type = (v >> 16) & 0x7f;
+
+	/*
+	 * All BARs have fixed addresses specified by the EA
+	 * capability, they must return zero on read.
+	 */
+	if (cfg_type == 0 &&
+	    ((where >= 0x10 && where < 0x2c) ||
+	     (where >= 0x1a4 && where < 0x1bc))) {
+		/* BAR or SRIOV BAR */
+		*val = 0;
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	addr = bus->ops->map_bus(bus, devfn, 0);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	vendor_device = readl(addr);
+	if (vendor_device == 0xffffffff)
+		goto no_emulation;
+
+	addr = bus->ops->map_bus(bus, devfn, 8);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	v = readl(addr);
+	if (v == 0xffffffff)
+		goto no_emulation;
+
+	pr_debug("%04x:%04x - Fix pass#: %08x, where: %03x, devfn: %03x\n",
+		 vendor_device & 0xffff, vendor_device >> 16, v,
+		 (unsigned) where, devfn);
+
+	/* Check for non type-00 header. */
+	if (cfg_type == 0) {
+		bool has_msix;
+		bool is_nic = (vendor_device == 0xa01e177d);
+		bool is_tns = (vendor_device == 0xa01f177d);
+
+		addr = bus->ops->map_bus(bus, devfn, 0x70);
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		/* E_CAP */
+		v = readl(addr);
+		has_msix = (v & 0xff00) != 0;
+
+		if (!has_msix && where_a == 0x70) {
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xb0) {
+			addr = bus->ops->map_bus(bus, devfn, where_a);
+			if (!addr) {
+				*val = ~0;
+				return PCIBIOS_DEVICE_NOT_FOUND;
+			}
+			v = readl(addr);
+			if (v & 0xff00)
+				pr_err("Bad MSIX cap header: %08x\n", v);
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xbc) {
+			if (is_nic)
+				v = 0x40014; /* EA last in chain, 4 entries. */
+			else if (is_tns)
+				v = 0x30014; /* EA last in chain, 3 entries. */
+			else if (has_msix)
+				v = 0x20014; /* EA last in chain, 2 entries. */
+			else
+				v = 0x10014; /* EA last in chain, 1 entry. */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a >= 0xc0 && where_a < 0xd0)
+			/* EA entry-0. PP=0, BAR0 Size:3 */
+			return handle_ea_bar(0x80ff0003,
+					     0x10, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xd0 && where_a < 0xe0 && has_msix)
+			 /* EA entry-1. PP=0, BAR4 Size:3 */
+			return handle_ea_bar(0x80ff0043,
+					     0x20, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xe0 && where_a < 0xf0 && is_tns)
+			/* EA entry-2. PP=0, BAR2, Size:3 */
+			return handle_ea_bar(0x80ff0023,
+					     0x18, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xe0 && where_a < 0xf0 && is_nic)
+			/* EA entry-2. PP=4, VF_BAR0 (9), Size:3 */
+			return handle_ea_bar(0x80ff0493,
+					     0x1a4, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xf0 && where_a < 0x100 && is_nic)
+			/* EA entry-3. PP=4, VF_BAR4 (d), Size:3 */
+			return handle_ea_bar(0x80ff04d3,
+					     0x1b4, bus, devfn, where,
+					     size, val);
+	} else if (cfg_type == 1) {
+		bool is_rsl_bridge = devfn == 0x08;
+		bool is_rad_bridge = devfn == 0xa0;
+		bool is_zip_bridge = devfn == 0xa8;
+		bool is_dfa_bridge = devfn == 0xb0;
+		bool is_nic_bridge = devfn == 0x10;
+
+		if (where_a == 0x70) {
+			addr = bus->ops->map_bus(bus, devfn, where_a);
+			if (!addr) {
+				*val = ~0;
+				return PCIBIOS_DEVICE_NOT_FOUND;
+			}
+			v = readl(addr);
+			if (v & 0xff00)
+				pr_err("Bad PCIe cap header: %08x\n", v);
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xbc) {
+			if (is_nic_bridge)
+				v = 0x10014; /* EA last in chain, 1 entry */
+			else
+				v = 0x00014; /* EA last in chain, no entries */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc0) {
+			if (is_rsl_bridge || is_nic_bridge)
+				v = 0x0101; /* subordinate:secondary = 1:1 */
+			else if (is_rad_bridge)
+				v = 0x0202; /* subordinate:secondary = 2:2 */
+			else if (is_zip_bridge)
+				v = 0x0303; /* subordinate:secondary = 3:3 */
+			else if (is_dfa_bridge)
+				v = 0x0404; /* subordinate:secondary = 4:4 */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc4 && is_nic_bridge) {
+			/* Enabled, not-Write, SP=ff, PP=05, BEI=6, ES=4 */
+			v = 0x80ff0564;
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc8 && is_nic_bridge) {
+			v = 0x00000002; /* Base-L 64-bit */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xcc && is_nic_bridge) {
+			v = 0xfffffffe; /* MaxOffset-L 64-bit */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xd0 && is_nic_bridge) {
+			v = 0x00008430; /* NIC Base-H */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xd4 && is_nic_bridge) {
+			v = 0x0000000f; /* MaxOffset-H */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+	}
+no_emulation:
+	return pci_generic_config_read(bus, devfn, where, size, val);
+}
+
+static int thunder_ecam_config_write(struct pci_bus *bus, unsigned int devfn,
+				     int where, int size, u32 val)
+{
+	/*
+	 * All BARs have fixed addresses, ignore BAR writes so they
+	 * don't get corrupted.
+	 */
+	if ((where >= 0x10 && where < 0x2c) ||
+	    (where >= 0x1a4 && where < 0x1bc))
+		/* BAR or SRIOV BAR */
+		return PCIBIOS_SUCCESSFUL;
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static struct gen_pci_cfg_bus_ops thunder_ecam_bus_ops = {
+	.bus_shift	= 20,
+	.ops		= {
+		.map_bus        = thunder_ecam_map_bus,
+		.read           = thunder_ecam_config_read,
+		.write          = thunder_ecam_config_write,
+	}
+};
+
+static const struct of_device_id thunder_ecam_of_match[] = {
+	{ .compatible = "cavium,pci-host-thunder-ecam",
+	  .data = &thunder_ecam_bus_ops },
+
+	{ },
+};
+MODULE_DEVICE_TABLE(of, thunder_ecam_of_match);
+
+static int thunder_ecam_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+
+	if (!pci)
+		return -ENOMEM;
+
+	of_id = of_match_node(thunder_ecam_of_match, dev->of_node);
+	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	return pci_host_common_probe(pdev, pci);
+}
+
+static struct platform_driver thunder_ecam_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = thunder_ecam_of_match,
+	},
+	.probe = thunder_ecam_probe,
+};
+module_platform_driver(thunder_ecam_driver);
+
+MODULE_DESCRIPTION("Thunder ECAM PCI host driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-05 23:41   ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-05 23:41 UTC (permalink / raw)
  To: linux-arm-kernel

From: David Daney <david.daney@cavium.com>

The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
complexes.  These root complexes (loosely referred to as ECAM units in
the hardware manuals) are used to access the Thunder on-chips devices.
They are special in that all the BARs on devices behind these root
complexes are at fixed addresses.  To handle this in a manner
compatible with the core PCI code, we have the config access functions
synthesize Enhanced Allocation (EA) capability entries for each BAR.

Since this EA synthesis is needed for exactly one chip model, we can
hard code some assumptions about the device topology and the
properties of specific DEVFNs in the driver.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
 drivers/pci/host/Kconfig                           |   7 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
 4 files changed, 396 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
 create mode 100644 drivers/pci/host/pci-thunder-ecam.c

diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
new file mode 100644
index 0000000..34658f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
@@ -0,0 +1,30 @@
+* ThunderX PCI host controller for pass-1.x silicon
+
+Firmware-initialized PCI host controller to on-chip devices found on
+some Cavium ThunderX processors.  These devices have ECAM based config
+access, but the BARs are all at fixed addresses.  We handle the fixed
+addresses by synthesizing Enhanced Allocation (EA) capabilities for
+these devices.
+
+The properties and their meanings are identical to those described in
+host-generic-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-generic-pci.txt:
+
+- compatible     : Must be "cavium,pci-host-thunder-ecam"
+
+Example:
+
+	pci at 84b0,00000000 {
+		compatible = "cavium,pci-host-thunder-ecam";
+		device_type = "pci";
+		msi-parent = <&its>;
+		msi-map = <0 &its 0x30000 0x10000>;
+		bus-range = <0 31>;
+		#size-cells = <2>;
+		#address-cells = <3>;
+		#stream-id-cells = <1>;
+		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
+		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
+	};
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 184df22..f8912c6 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -202,4 +202,11 @@ config PCI_HOST_THUNDER_PEM
 	help
 	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
 
+config PCI_HOST_THUNDER_ECAM
+	bool "Cavium Thunder ECAMe controller to on-chip devices on pass-1.x silicon"
+	depends on OF && ARM64
+	select PCI_HOST_COMMON
+	help
+	  Say Y here if you want ECAM support for CN88XX-Pass-1.x Cavium Thunder SoCs.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 8903172..d6af3ba 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -23,4 +23,5 @@ obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
+obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
 obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
diff --git a/drivers/pci/host/pci-thunder-ecam.c b/drivers/pci/host/pci-thunder-ecam.c
new file mode 100644
index 0000000..83ee590
--- /dev/null
+++ b/drivers/pci/host/pci-thunder-ecam.c
@@ -0,0 +1,358 @@
+/*
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2015 Cavium, Inc.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/of_pci.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "pci-host-common.h"
+
+/* Mapping is standard ECAM */
+static void __iomem *thunder_ecam_map_bus(struct pci_bus *bus,
+					  unsigned int devfn,
+					  int where)
+{
+	struct gen_pci *pci = bus->sysdata;
+	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
+
+	return pci->cfg.win[idx] + ((devfn << 12) | where);
+}
+
+static void set_val(u32 v, int where, int size, u32 *val)
+{
+	int shift = (where & 3) * 8;
+
+	pr_debug("set_val %04x: %08x\n", (unsigned)(where & ~3), v);
+	v >>= shift;
+	if (size == 1)
+		v &= 0xff;
+	else if (size == 2)
+		v &= 0xffff;
+	*val = v;
+}
+
+static int handle_ea_bar(u32 e0, int bar, struct pci_bus *bus,
+			 unsigned int devfn, int where, int size, u32 *val)
+{
+	void __iomem *addr;
+	u32 v;
+	/*
+	 * Each entry is 16-byte aligned bits[2,3] select which word
+	 * in the entry
+	 */
+	int where_a = where & 0xc;
+
+	if (where_a == 0) {
+		set_val(e0, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0x4) {
+		addr = bus->ops->map_bus(bus, devfn, bar); /* BAR 0 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		v = readl(addr);
+		v &= ~0xf;
+		v |= 2; /* EA entry-1. Base-L */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0x8) {
+		u32 barl_orig;
+		u32 barl_rb;
+
+		addr = bus->ops->map_bus(bus, devfn, bar); /* BAR 0 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		barl_orig = readl(addr + 0);
+		writel(0xffffffff, addr + 0);
+		barl_rb = readl(addr + 0);
+		writel(barl_orig, addr + 0);
+		/* zeros in unsettable bits. */
+		v = ~barl_rb & ~3;
+		v |= 0xc; /* EA entry-2. Offset-L */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	if (where_a == 0xc) {
+		addr = bus->ops->map_bus(bus, devfn, bar + 4); /* BAR 1 */
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		v = readl(addr); /* EA entry-3. Base-H */
+		set_val(v, where, size, val);
+		return PCIBIOS_SUCCESSFUL;
+	}
+	return PCIBIOS_DEVICE_NOT_FOUND;
+}
+
+static int thunder_ecam_config_read(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 *val)
+{
+	u32 v;
+	u32 vendor_device;
+	void __iomem *addr;
+	int cfg_type;
+	int where_a = where & ~3;
+
+
+	addr = bus->ops->map_bus(bus, devfn, 0xc);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	v = readl(addr);
+
+	/* Check for non type-00 header. */
+	cfg_type = (v >> 16) & 0x7f;
+
+	/*
+	 * All BARs have fixed addresses specified by the EA
+	 * capability, they must return zero on read.
+	 */
+	if (cfg_type == 0 &&
+	    ((where >= 0x10 && where < 0x2c) ||
+	     (where >= 0x1a4 && where < 0x1bc))) {
+		/* BAR or SRIOV BAR */
+		*val = 0;
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	addr = bus->ops->map_bus(bus, devfn, 0);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	vendor_device = readl(addr);
+	if (vendor_device == 0xffffffff)
+		goto no_emulation;
+
+	addr = bus->ops->map_bus(bus, devfn, 8);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	v = readl(addr);
+	if (v == 0xffffffff)
+		goto no_emulation;
+
+	pr_debug("%04x:%04x - Fix pass#: %08x, where: %03x, devfn: %03x\n",
+		 vendor_device & 0xffff, vendor_device >> 16, v,
+		 (unsigned) where, devfn);
+
+	/* Check for non type-00 header. */
+	if (cfg_type == 0) {
+		bool has_msix;
+		bool is_nic = (vendor_device == 0xa01e177d);
+		bool is_tns = (vendor_device == 0xa01f177d);
+
+		addr = bus->ops->map_bus(bus, devfn, 0x70);
+		if (!addr) {
+			*val = ~0;
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		}
+		/* E_CAP */
+		v = readl(addr);
+		has_msix = (v & 0xff00) != 0;
+
+		if (!has_msix && where_a == 0x70) {
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xb0) {
+			addr = bus->ops->map_bus(bus, devfn, where_a);
+			if (!addr) {
+				*val = ~0;
+				return PCIBIOS_DEVICE_NOT_FOUND;
+			}
+			v = readl(addr);
+			if (v & 0xff00)
+				pr_err("Bad MSIX cap header: %08x\n", v);
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xbc) {
+			if (is_nic)
+				v = 0x40014; /* EA last in chain, 4 entries. */
+			else if (is_tns)
+				v = 0x30014; /* EA last in chain, 3 entries. */
+			else if (has_msix)
+				v = 0x20014; /* EA last in chain, 2 entries. */
+			else
+				v = 0x10014; /* EA last in chain, 1 entry. */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a >= 0xc0 && where_a < 0xd0)
+			/* EA entry-0. PP=0, BAR0 Size:3 */
+			return handle_ea_bar(0x80ff0003,
+					     0x10, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xd0 && where_a < 0xe0 && has_msix)
+			 /* EA entry-1. PP=0, BAR4 Size:3 */
+			return handle_ea_bar(0x80ff0043,
+					     0x20, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xe0 && where_a < 0xf0 && is_tns)
+			/* EA entry-2. PP=0, BAR2, Size:3 */
+			return handle_ea_bar(0x80ff0023,
+					     0x18, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xe0 && where_a < 0xf0 && is_nic)
+			/* EA entry-2. PP=4, VF_BAR0 (9), Size:3 */
+			return handle_ea_bar(0x80ff0493,
+					     0x1a4, bus, devfn, where,
+					     size, val);
+		if (where_a >= 0xf0 && where_a < 0x100 && is_nic)
+			/* EA entry-3. PP=4, VF_BAR4 (d), Size:3 */
+			return handle_ea_bar(0x80ff04d3,
+					     0x1b4, bus, devfn, where,
+					     size, val);
+	} else if (cfg_type == 1) {
+		bool is_rsl_bridge = devfn == 0x08;
+		bool is_rad_bridge = devfn == 0xa0;
+		bool is_zip_bridge = devfn == 0xa8;
+		bool is_dfa_bridge = devfn == 0xb0;
+		bool is_nic_bridge = devfn == 0x10;
+
+		if (where_a == 0x70) {
+			addr = bus->ops->map_bus(bus, devfn, where_a);
+			if (!addr) {
+				*val = ~0;
+				return PCIBIOS_DEVICE_NOT_FOUND;
+			}
+			v = readl(addr);
+			if (v & 0xff00)
+				pr_err("Bad PCIe cap header: %08x\n", v);
+			v |= 0xbc00; /* next capability is EA at 0xbc */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xbc) {
+			if (is_nic_bridge)
+				v = 0x10014; /* EA last in chain, 1 entry */
+			else
+				v = 0x00014; /* EA last in chain, no entries */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc0) {
+			if (is_rsl_bridge || is_nic_bridge)
+				v = 0x0101; /* subordinate:secondary = 1:1 */
+			else if (is_rad_bridge)
+				v = 0x0202; /* subordinate:secondary = 2:2 */
+			else if (is_zip_bridge)
+				v = 0x0303; /* subordinate:secondary = 3:3 */
+			else if (is_dfa_bridge)
+				v = 0x0404; /* subordinate:secondary = 4:4 */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc4 && is_nic_bridge) {
+			/* Enabled, not-Write, SP=ff, PP=05, BEI=6, ES=4 */
+			v = 0x80ff0564;
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xc8 && is_nic_bridge) {
+			v = 0x00000002; /* Base-L 64-bit */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xcc && is_nic_bridge) {
+			v = 0xfffffffe; /* MaxOffset-L 64-bit */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xd0 && is_nic_bridge) {
+			v = 0x00008430; /* NIC Base-H */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+		if (where_a == 0xd4 && is_nic_bridge) {
+			v = 0x0000000f; /* MaxOffset-H */
+			set_val(v, where, size, val);
+			return PCIBIOS_SUCCESSFUL;
+		}
+	}
+no_emulation:
+	return pci_generic_config_read(bus, devfn, where, size, val);
+}
+
+static int thunder_ecam_config_write(struct pci_bus *bus, unsigned int devfn,
+				     int where, int size, u32 val)
+{
+	/*
+	 * All BARs have fixed addresses, ignore BAR writes so they
+	 * don't get corrupted.
+	 */
+	if ((where >= 0x10 && where < 0x2c) ||
+	    (where >= 0x1a4 && where < 0x1bc))
+		/* BAR or SRIOV BAR */
+		return PCIBIOS_SUCCESSFUL;
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static struct gen_pci_cfg_bus_ops thunder_ecam_bus_ops = {
+	.bus_shift	= 20,
+	.ops		= {
+		.map_bus        = thunder_ecam_map_bus,
+		.read           = thunder_ecam_config_read,
+		.write          = thunder_ecam_config_write,
+	}
+};
+
+static const struct of_device_id thunder_ecam_of_match[] = {
+	{ .compatible = "cavium,pci-host-thunder-ecam",
+	  .data = &thunder_ecam_bus_ops },
+
+	{ },
+};
+MODULE_DEVICE_TABLE(of, thunder_ecam_of_match);
+
+static int thunder_ecam_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+
+	if (!pci)
+		return -ENOMEM;
+
+	of_id = of_match_node(thunder_ecam_of_match, dev->of_node);
+	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	return pci_host_common_probe(pdev, pci);
+}
+
+static struct platform_driver thunder_ecam_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = thunder_ecam_of_match,
+	},
+	.probe = thunder_ecam_probe,
+};
+module_platform_driver(thunder_ecam_driver);
+
+MODULE_DESCRIPTION("Thunder ECAM PCI host driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
@ 2016-02-06 16:01     ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-06 16:01 UTC (permalink / raw)
  To: David Daney
  Cc: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-kernel, David Daney

Hi David,

I don't have time today to look at this thoroughly, but I think I did
see one actual bug and a possible way to make this more readable.  I
think reading difficulty increases as at least the square of the
maximum indent level :)

Bjorn

On Fri, Feb 05, 2016 at 03:41:14PM -0800, David Daney wrote:
> ...
> +static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
> +				    int where, int size, u32 val)
> +{
> +	struct gen_pci *pci = bus->sysdata;
> +	struct thunder_pem_pci *pem_pci;
> +
> +	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
> +
> +	/*
> +	 * The first device on the bus in the PEM PCIe bridge.
> +	 * Special case its config access.
> +	 */
> +	if (bus->number == pci->cfg.bus_range->start) {
> +		u64 write_val, read_val;
> +		u32 mask = 0;
> +
> +		if (devfn != 0 || where >= 2048)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +		/*
> +		 * 32-bit accesses only.  If the write is for a size
> +		 * smaller than 32-bits, we must first read the 32-bit
> +		 * value and merge in the desired bits and then write
> +		 * the whole 32-bits back out.
> +		 */
> +		switch (size) {
> +		case 1:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		case 2:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xffff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xffff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		default:
> +			break;
> +
> +		}

I think it would make this easier to read if the root bus case were
split to a different function, e.g.,

  static u32 thunder_pem_root_w1c_bits(...)
  {
    ...
  }

  static int thunder_pem_root_config_write(...)
  {
    u32 mask[2] = {0xff, 0xffff};

    if (devfn != 0 || where >= 2048)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (size != 1 && size != 2 && size != 4)
      return PCIBIOS_BAD_REGISTER_NUMBER;

    /* 32-bit accesses supported natively */
    if (size == 4) {
      write_val = where & ~3ull;
      write_val |= (((u64)val) << 32);
      writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
      return PCIBIOS_SUCCESSFUL;
    }

    /* smaller accesses require read/modify/write */
    read_val = where & ~3ull;
    writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val >>= 32;

    /* mask W1C bits so we don't clear them inadvertently */
    write_val = read_val & ~thunder_pem_root_w1c_bits(...);

    /* deposit new write data (which may intentionally write W1C bits) */
    val &= mask[size];
    shift = 8 * (where & 3);

    write_val &= ~(mask[size] << shift);
    write_val |= val << shift;

    write_val = where & ~3ull;
    write_val |= (((u64)val) << 32);
    writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
    return PCIBIOS_SUCCESSFUL;
  }

  static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
				    int where, int size, u32 val)
  {
    ...
    if (bus->number < pci->cfg.bus_range->start ||
        bus->number > pci->cfg.bus_range->end)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (bus->number == pci->cfg.bus_range->start)
      return thunder_pem_root_config_write(...);

    return pci_generic_config_write(bus, devfn, where, size, val);
  }

> +
> +		if (mask) {
> +			/*
> +			 * By expanding the write width to 32 bits, we
> +			 * may inadvertently hit some W1C bits that
> +			 * were not intended to be written.  Mask
> +			 * these out.
> +			 *
> +			 * Some of the w1c_bits below also include
> +			 * read-only or non-writable reserved bits,
> +			 * this makes the code simpler and is OK as
> +			 * the bits are not affected by writing zeros
> +			 * to them.
> +			 */
> +			u32 w1c_bits = 0;
> +
> +			switch (where & 3) {
> +			case 0x04: /* Command/Status */
> +			case 0x1c: /* Base and I/O Limit/Secondary Status */
> +				w1c_bits = 0xff000000;
> +				break;
> +			case 0x44: /* Power Management Control and Status */
> +				w1c_bits = 0xfffffe00;
> +				break;
> +			case 0x78: /* Device Control/Device Status */
> +			case 0x80: /* Link Control/Link Status */
> +			case 0x88: /* Slot Control/Slot Status */
> +			case 0x90: /* Root Status */
> +			case 0xa0: /* Link Control 2 Registers/Link Status 2 */
> +				w1c_bits = 0xffff0000;
> +				break;
> +			case 0x104: /* Uncorrectable Error Status */
> +			case 0x110: /* Correctable Error Status */
> +			case 0x130: /* Error Status */
> +			case 0x160: /* Link Control 4 */

"where & 3" can never match any of these cases.  I suppose you meant
"where & ~3"?

> +				w1c_bits = 0xffffffff;
> +				break;
> +			default:
> +				break;
> +
> +			}
> +			if (w1c_bits) {
> +				mask &= w1c_bits;
> +				val &= ~mask;
> +			}
> +		}
> +
> +		/*
> +		 * Low order bits are the config address, the high
> +		 * order 32 bits are the data to be written.
> +		 */
> +		write_val = where & ~3ull;
> +		write_val |= (((u64)val) << 32);
> +		writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
> +		return PCIBIOS_SUCCESSFUL;
> +	}
> +	if (bus->number < pci->cfg.bus_range->start ||
> +	    bus->number > pci->cfg.bus_range->end)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return pci_generic_config_write(bus, devfn, where, size, val);
> +}

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
@ 2016-02-06 16:01     ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-06 16:01 UTC (permalink / raw)
  To: David Daney
  Cc: Bjorn Helgaas, linux-pci-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Daney

Hi David,

I don't have time today to look at this thoroughly, but I think I did
see one actual bug and a possible way to make this more readable.  I
think reading difficulty increases as at least the square of the
maximum indent level :)

Bjorn

On Fri, Feb 05, 2016 at 03:41:14PM -0800, David Daney wrote:
> ...
> +static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
> +				    int where, int size, u32 val)
> +{
> +	struct gen_pci *pci = bus->sysdata;
> +	struct thunder_pem_pci *pem_pci;
> +
> +	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
> +
> +	/*
> +	 * The first device on the bus in the PEM PCIe bridge.
> +	 * Special case its config access.
> +	 */
> +	if (bus->number == pci->cfg.bus_range->start) {
> +		u64 write_val, read_val;
> +		u32 mask = 0;
> +
> +		if (devfn != 0 || where >= 2048)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +		/*
> +		 * 32-bit accesses only.  If the write is for a size
> +		 * smaller than 32-bits, we must first read the 32-bit
> +		 * value and merge in the desired bits and then write
> +		 * the whole 32-bits back out.
> +		 */
> +		switch (size) {
> +		case 1:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		case 2:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xffff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xffff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		default:
> +			break;
> +
> +		}

I think it would make this easier to read if the root bus case were
split to a different function, e.g.,

  static u32 thunder_pem_root_w1c_bits(...)
  {
    ...
  }

  static int thunder_pem_root_config_write(...)
  {
    u32 mask[2] = {0xff, 0xffff};

    if (devfn != 0 || where >= 2048)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (size != 1 && size != 2 && size != 4)
      return PCIBIOS_BAD_REGISTER_NUMBER;

    /* 32-bit accesses supported natively */
    if (size == 4) {
      write_val = where & ~3ull;
      write_val |= (((u64)val) << 32);
      writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
      return PCIBIOS_SUCCESSFUL;
    }

    /* smaller accesses require read/modify/write */
    read_val = where & ~3ull;
    writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val >>= 32;

    /* mask W1C bits so we don't clear them inadvertently */
    write_val = read_val & ~thunder_pem_root_w1c_bits(...);

    /* deposit new write data (which may intentionally write W1C bits) */
    val &= mask[size];
    shift = 8 * (where & 3);

    write_val &= ~(mask[size] << shift);
    write_val |= val << shift;

    write_val = where & ~3ull;
    write_val |= (((u64)val) << 32);
    writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
    return PCIBIOS_SUCCESSFUL;
  }

  static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
				    int where, int size, u32 val)
  {
    ...
    if (bus->number < pci->cfg.bus_range->start ||
        bus->number > pci->cfg.bus_range->end)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (bus->number == pci->cfg.bus_range->start)
      return thunder_pem_root_config_write(...);

    return pci_generic_config_write(bus, devfn, where, size, val);
  }

> +
> +		if (mask) {
> +			/*
> +			 * By expanding the write width to 32 bits, we
> +			 * may inadvertently hit some W1C bits that
> +			 * were not intended to be written.  Mask
> +			 * these out.
> +			 *
> +			 * Some of the w1c_bits below also include
> +			 * read-only or non-writable reserved bits,
> +			 * this makes the code simpler and is OK as
> +			 * the bits are not affected by writing zeros
> +			 * to them.
> +			 */
> +			u32 w1c_bits = 0;
> +
> +			switch (where & 3) {
> +			case 0x04: /* Command/Status */
> +			case 0x1c: /* Base and I/O Limit/Secondary Status */
> +				w1c_bits = 0xff000000;
> +				break;
> +			case 0x44: /* Power Management Control and Status */
> +				w1c_bits = 0xfffffe00;
> +				break;
> +			case 0x78: /* Device Control/Device Status */
> +			case 0x80: /* Link Control/Link Status */
> +			case 0x88: /* Slot Control/Slot Status */
> +			case 0x90: /* Root Status */
> +			case 0xa0: /* Link Control 2 Registers/Link Status 2 */
> +				w1c_bits = 0xffff0000;
> +				break;
> +			case 0x104: /* Uncorrectable Error Status */
> +			case 0x110: /* Correctable Error Status */
> +			case 0x130: /* Error Status */
> +			case 0x160: /* Link Control 4 */

"where & 3" can never match any of these cases.  I suppose you meant
"where & ~3"?

> +				w1c_bits = 0xffffffff;
> +				break;
> +			default:
> +				break;
> +
> +			}
> +			if (w1c_bits) {
> +				mask &= w1c_bits;
> +				val &= ~mask;
> +			}
> +		}
> +
> +		/*
> +		 * Low order bits are the config address, the high
> +		 * order 32 bits are the data to be written.
> +		 */
> +		write_val = where & ~3ull;
> +		write_val |= (((u64)val) << 32);
> +		writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
> +		return PCIBIOS_SUCCESSFUL;
> +	}
> +	if (bus->number < pci->cfg.bus_range->start ||
> +	    bus->number > pci->cfg.bus_range->end)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return pci_generic_config_write(bus, devfn, where, size, val);
> +}
--
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

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors.
@ 2016-02-06 16:01     ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-06 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi David,

I don't have time today to look at this thoroughly, but I think I did
see one actual bug and a possible way to make this more readable.  I
think reading difficulty increases as at least the square of the
maximum indent level :)

Bjorn

On Fri, Feb 05, 2016 at 03:41:14PM -0800, David Daney wrote:
> ...
> +static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
> +				    int where, int size, u32 val)
> +{
> +	struct gen_pci *pci = bus->sysdata;
> +	struct thunder_pem_pci *pem_pci;
> +
> +	pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci);
> +
> +	/*
> +	 * The first device on the bus in the PEM PCIe bridge.
> +	 * Special case its config access.
> +	 */
> +	if (bus->number == pci->cfg.bus_range->start) {
> +		u64 write_val, read_val;
> +		u32 mask = 0;
> +
> +		if (devfn != 0 || where >= 2048)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +		/*
> +		 * 32-bit accesses only.  If the write is for a size
> +		 * smaller than 32-bits, we must first read the 32-bit
> +		 * value and merge in the desired bits and then write
> +		 * the whole 32-bits back out.
> +		 */
> +		switch (size) {
> +		case 1:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		case 2:
> +			read_val = where & ~3ull;
> +			writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
> +			read_val >>= 32;
> +			mask = ~(0xffff << (8 * (where & 3)));
> +			read_val &= mask;
> +			val = (val & 0xffff) << (8 * (where & 3));
> +			val |= (u32)read_val;
> +			break;
> +		default:
> +			break;
> +
> +		}

I think it would make this easier to read if the root bus case were
split to a different function, e.g.,

  static u32 thunder_pem_root_w1c_bits(...)
  {
    ...
  }

  static int thunder_pem_root_config_write(...)
  {
    u32 mask[2] = {0xff, 0xffff};

    if (devfn != 0 || where >= 2048)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (size != 1 && size != 2 && size != 4)
      return PCIBIOS_BAD_REGISTER_NUMBER;

    /* 32-bit accesses supported natively */
    if (size == 4) {
      write_val = where & ~3ull;
      write_val |= (((u64)val) << 32);
      writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
      return PCIBIOS_SUCCESSFUL;
    }

    /* smaller accesses require read/modify/write */
    read_val = where & ~3ull;
    writeq(read_val, pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val = readq(pem_pci->pem_reg_base + PEM_CFG_RD);
    read_val >>= 32;

    /* mask W1C bits so we don't clear them inadvertently */
    write_val = read_val & ~thunder_pem_root_w1c_bits(...);

    /* deposit new write data (which may intentionally write W1C bits) */
    val &= mask[size];
    shift = 8 * (where & 3);

    write_val &= ~(mask[size] << shift);
    write_val |= val << shift;

    write_val = where & ~3ull;
    write_val |= (((u64)val) << 32);
    writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
    return PCIBIOS_SUCCESSFUL;
  }

  static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn,
				    int where, int size, u32 val)
  {
    ...
    if (bus->number < pci->cfg.bus_range->start ||
        bus->number > pci->cfg.bus_range->end)
      return PCIBIOS_DEVICE_NOT_FOUND;

    if (bus->number == pci->cfg.bus_range->start)
      return thunder_pem_root_config_write(...);

    return pci_generic_config_write(bus, devfn, where, size, val);
  }

> +
> +		if (mask) {
> +			/*
> +			 * By expanding the write width to 32 bits, we
> +			 * may inadvertently hit some W1C bits that
> +			 * were not intended to be written.  Mask
> +			 * these out.
> +			 *
> +			 * Some of the w1c_bits below also include
> +			 * read-only or non-writable reserved bits,
> +			 * this makes the code simpler and is OK as
> +			 * the bits are not affected by writing zeros
> +			 * to them.
> +			 */
> +			u32 w1c_bits = 0;
> +
> +			switch (where & 3) {
> +			case 0x04: /* Command/Status */
> +			case 0x1c: /* Base and I/O Limit/Secondary Status */
> +				w1c_bits = 0xff000000;
> +				break;
> +			case 0x44: /* Power Management Control and Status */
> +				w1c_bits = 0xfffffe00;
> +				break;
> +			case 0x78: /* Device Control/Device Status */
> +			case 0x80: /* Link Control/Link Status */
> +			case 0x88: /* Slot Control/Slot Status */
> +			case 0x90: /* Root Status */
> +			case 0xa0: /* Link Control 2 Registers/Link Status 2 */
> +				w1c_bits = 0xffff0000;
> +				break;
> +			case 0x104: /* Uncorrectable Error Status */
> +			case 0x110: /* Correctable Error Status */
> +			case 0x130: /* Error Status */
> +			case 0x160: /* Link Control 4 */

"where & 3" can never match any of these cases.  I suppose you meant
"where & ~3"?

> +				w1c_bits = 0xffffffff;
> +				break;
> +			default:
> +				break;
> +
> +			}
> +			if (w1c_bits) {
> +				mask &= w1c_bits;
> +				val &= ~mask;
> +			}
> +		}
> +
> +		/*
> +		 * Low order bits are the config address, the high
> +		 * order 32 bits are the data to be written.
> +		 */
> +		write_val = where & ~3ull;
> +		write_val |= (((u64)val) << 32);
> +		writeq(write_val, pem_pci->pem_reg_base + PEM_CFG_WR);
> +		return PCIBIOS_SUCCESSFUL;
> +	}
> +	if (bus->number < pci->cfg.bus_range->start ||
> +	    bus->number > pci->cfg.bus_range->end)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	return pci_generic_config_write(bus, devfn, where, size, val);
> +}

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-05 23:41   ` David Daney
@ 2016-02-08 19:56     ` Rob Herring
  -1 siblings, 0 replies; 45+ messages in thread
From: Rob Herring @ 2016-02-08 19:56 UTC (permalink / raw)
  To: David Daney
  Cc: Bjorn Helgaas, linux-pci, Will Deacon, linux-arm-kernel,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, devicetree,
	linux-kernel, David Daney

On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
> complexes.  These root complexes (loosely referred to as ECAM units in
> the hardware manuals) are used to access the Thunder on-chips devices.
> They are special in that all the BARs on devices behind these root
> complexes are at fixed addresses.  To handle this in a manner
> compatible with the core PCI code, we have the config access functions
> synthesize Enhanced Allocation (EA) capability entries for each BAR.
> 
> Since this EA synthesis is needed for exactly one chip model, we can
> hard code some assumptions about the device topology and the
> properties of specific DEVFNs in the driver.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
>  drivers/pci/host/Kconfig                           |   7 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
>  4 files changed, 396 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>  create mode 100644 drivers/pci/host/pci-thunder-ecam.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
> new file mode 100644
> index 0000000..34658f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
> @@ -0,0 +1,30 @@
> +* ThunderX PCI host controller for pass-1.x silicon
> +
> +Firmware-initialized PCI host controller to on-chip devices found on
> +some Cavium ThunderX processors.  These devices have ECAM based config
> +access, but the BARs are all at fixed addresses.  We handle the fixed
> +addresses by synthesizing Enhanced Allocation (EA) capabilities for
> +these devices.
> +
> +The properties and their meanings are identical to those described in
> +host-generic-pci.txt except as listed below.
> +
> +Properties of the host controller node that differ from
> +host-generic-pci.txt:
> +
> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
> +
> +Example:
> +
> +	pci@84b0,00000000 {

Drop the comma, and the node name should be "pcie".

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

> +		compatible = "cavium,pci-host-thunder-ecam";
> +		device_type = "pci";
> +		msi-parent = <&its>;
> +		msi-map = <0 &its 0x30000 0x10000>;
> +		bus-range = <0 31>;
> +		#size-cells = <2>;
> +		#address-cells = <3>;
> +		#stream-id-cells = <1>;
> +		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
> +		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
> +	};

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 19:56     ` Rob Herring
  0 siblings, 0 replies; 45+ messages in thread
From: Rob Herring @ 2016-02-08 19:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
> complexes.  These root complexes (loosely referred to as ECAM units in
> the hardware manuals) are used to access the Thunder on-chips devices.
> They are special in that all the BARs on devices behind these root
> complexes are at fixed addresses.  To handle this in a manner
> compatible with the core PCI code, we have the config access functions
> synthesize Enhanced Allocation (EA) capability entries for each BAR.
> 
> Since this EA synthesis is needed for exactly one chip model, we can
> hard code some assumptions about the device topology and the
> properties of specific DEVFNs in the driver.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
>  drivers/pci/host/Kconfig                           |   7 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
>  4 files changed, 396 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>  create mode 100644 drivers/pci/host/pci-thunder-ecam.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
> new file mode 100644
> index 0000000..34658f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
> @@ -0,0 +1,30 @@
> +* ThunderX PCI host controller for pass-1.x silicon
> +
> +Firmware-initialized PCI host controller to on-chip devices found on
> +some Cavium ThunderX processors.  These devices have ECAM based config
> +access, but the BARs are all at fixed addresses.  We handle the fixed
> +addresses by synthesizing Enhanced Allocation (EA) capabilities for
> +these devices.
> +
> +The properties and their meanings are identical to those described in
> +host-generic-pci.txt except as listed below.
> +
> +Properties of the host controller node that differ from
> +host-generic-pci.txt:
> +
> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
> +
> +Example:
> +
> +	pci at 84b0,00000000 {

Drop the comma, and the node name should be "pcie".

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

> +		compatible = "cavium,pci-host-thunder-ecam";
> +		device_type = "pci";
> +		msi-parent = <&its>;
> +		msi-map = <0 &its 0x30000 0x10000>;
> +		bus-range = <0 31>;
> +		#size-cells = <2>;
> +		#address-cells = <3>;
> +		#stream-id-cells = <1>;
> +		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
> +		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
> +	};

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 19:56     ` Rob Herring
  (?)
@ 2016-02-08 20:47       ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 20:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 11:56 AM, Rob Herring wrote:
> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
>> complexes.  These root complexes (loosely referred to as ECAM units in
>> the hardware manuals) are used to access the Thunder on-chips devices.
>> They are special in that all the BARs on devices behind these root
>> complexes are at fixed addresses.  To handle this in a manner
>> compatible with the core PCI code, we have the config access functions
>> synthesize Enhanced Allocation (EA) capability entries for each BAR.
>>
>> Since this EA synthesis is needed for exactly one chip model, we can
>> hard code some assumptions about the device topology and the
>> properties of specific DEVFNs in the driver.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
>>   drivers/pci/host/Kconfig                           |   7 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
>>   4 files changed, 396 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>>   create mode 100644 drivers/pci/host/pci-thunder-ecam.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> new file mode 100644
>> index 0000000..34658f2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> @@ -0,0 +1,30 @@
>> +* ThunderX PCI host controller for pass-1.x silicon
>> +
>> +Firmware-initialized PCI host controller to on-chip devices found on
>> +some Cavium ThunderX processors.  These devices have ECAM based config
>> +access, but the BARs are all at fixed addresses.  We handle the fixed
>> +addresses by synthesizing Enhanced Allocation (EA) capabilities for
>> +these devices.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-generic-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-generic-pci.txt:
>> +
>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>> +
>> +Example:
>> +
>> +	pci@84b0,00000000 {
>
> Drop the comma,

OK...

> and the node name should be "pcie".
>

Why pcie?

There are no PCIe devices or buses reachable from this type of root 
complex.  There are however many PCI devices.

> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>> +		compatible = "cavium,pci-host-thunder-ecam";
>> +		device_type = "pci";
>> +		msi-parent = <&its>;
>> +		msi-map = <0 &its 0x30000 0x10000>;
>> +		bus-range = <0 31>;
>> +		#size-cells = <2>;
>> +		#address-cells = <3>;
>> +		#stream-id-cells = <1>;
>> +		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
>> +		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
>> +	};

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 20:47       ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 20:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 11:56 AM, Rob Herring wrote:
> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
>> complexes.  These root complexes (loosely referred to as ECAM units in
>> the hardware manuals) are used to access the Thunder on-chips devices.
>> They are special in that all the BARs on devices behind these root
>> complexes are at fixed addresses.  To handle this in a manner
>> compatible with the core PCI code, we have the config access functions
>> synthesize Enhanced Allocation (EA) capability entries for each BAR.
>>
>> Since this EA synthesis is needed for exactly one chip model, we can
>> hard code some assumptions about the device topology and the
>> properties of specific DEVFNs in the driver.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
>>   drivers/pci/host/Kconfig                           |   7 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
>>   4 files changed, 396 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>>   create mode 100644 drivers/pci/host/pci-thunder-ecam.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> new file mode 100644
>> index 0000000..34658f2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> @@ -0,0 +1,30 @@
>> +* ThunderX PCI host controller for pass-1.x silicon
>> +
>> +Firmware-initialized PCI host controller to on-chip devices found on
>> +some Cavium ThunderX processors.  These devices have ECAM based config
>> +access, but the BARs are all at fixed addresses.  We handle the fixed
>> +addresses by synthesizing Enhanced Allocation (EA) capabilities for
>> +these devices.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-generic-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-generic-pci.txt:
>> +
>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>> +
>> +Example:
>> +
>> +	pci@84b0,00000000 {
>
> Drop the comma,

OK...

> and the node name should be "pcie".
>

Why pcie?

There are no PCIe devices or buses reachable from this type of root 
complex.  There are however many PCI devices.

> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>> +		compatible = "cavium,pci-host-thunder-ecam";
>> +		device_type = "pci";
>> +		msi-parent = <&its>;
>> +		msi-map = <0 &its 0x30000 0x10000>;
>> +		bus-range = <0 31>;
>> +		#size-cells = <2>;
>> +		#address-cells = <3>;
>> +		#stream-id-cells = <1>;
>> +		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
>> +		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
>> +	};

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 20:47       ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 20:47 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2016 11:56 AM, Rob Herring wrote:
> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> The cavium,pci-thunder-ecam devices are exactly ECAM based PCI root
>> complexes.  These root complexes (loosely referred to as ECAM units in
>> the hardware manuals) are used to access the Thunder on-chips devices.
>> They are special in that all the BARs on devices behind these root
>> complexes are at fixed addresses.  To handle this in a manner
>> compatible with the core PCI code, we have the config access functions
>> synthesize Enhanced Allocation (EA) capability entries for each BAR.
>>
>> Since this EA synthesis is needed for exactly one chip model, we can
>> hard code some assumptions about the device topology and the
>> properties of specific DEVFNs in the driver.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pci-thunder-ecam.txt   |  30 ++
>>   drivers/pci/host/Kconfig                           |   7 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pci-thunder-ecam.c                | 358 +++++++++++++++++++++
>>   4 files changed, 396 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>>   create mode 100644 drivers/pci/host/pci-thunder-ecam.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> new file mode 100644
>> index 0000000..34658f2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pci-thunder-ecam.txt
>> @@ -0,0 +1,30 @@
>> +* ThunderX PCI host controller for pass-1.x silicon
>> +
>> +Firmware-initialized PCI host controller to on-chip devices found on
>> +some Cavium ThunderX processors.  These devices have ECAM based config
>> +access, but the BARs are all at fixed addresses.  We handle the fixed
>> +addresses by synthesizing Enhanced Allocation (EA) capabilities for
>> +these devices.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-generic-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-generic-pci.txt:
>> +
>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>> +
>> +Example:
>> +
>> +	pci at 84b0,00000000 {
>
> Drop the comma,

OK...

> and the node name should be "pcie".
>

Why pcie?

There are no PCIe devices or buses reachable from this type of root 
complex.  There are however many PCI devices.

> Otherwise,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
>> +		compatible = "cavium,pci-host-thunder-ecam";
>> +		device_type = "pci";
>> +		msi-parent = <&its>;
>> +		msi-map = <0 &its 0x30000 0x10000>;
>> +		bus-range = <0 31>;
>> +		#size-cells = <2>;
>> +		#address-cells = <3>;
>> +		#stream-id-cells = <1>;
>> +		reg = <0x84b0 0x00000000 0 0x02000000>;  /* Configuration space */
>> +		ranges = <0x03000000 0x8180 0x00000000 0x8180 0x00000000 0x80 0x00000000>; /* mem ranges */
>> +	};

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 20:47       ` David Daney
  (?)
@ 2016-02-08 21:12         ` Rob Herring
  -1 siblings, 0 replies; 45+ messages in thread
From: Rob Herring @ 2016-02-08 21:12 UTC (permalink / raw)
  To: David Daney
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>
>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>
>>> From: David Daney <david.daney@cavium.com>

[...]

>>> +Properties of the host controller node that differ from
>>> +host-generic-pci.txt:
>>> +
>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>> +
>>> +Example:
>>> +
>>> +       pci@84b0,00000000 {
>>
>>
>> Drop the comma,
>
>
> OK...
>
>> and the node name should be "pcie".
>>
>
> Why pcie?
>
> There are no PCIe devices or buses reachable from this type of root complex.
> There are however many PCI devices.

I thought ECAM is a PCIe thing. If not, then nevermind.

Rob

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 21:12         ` Rob Herring
  0 siblings, 0 replies; 45+ messages in thread
From: Rob Herring @ 2016-02-08 21:12 UTC (permalink / raw)
  To: David Daney
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>
>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>
>>> From: David Daney <david.daney@cavium.com>

[...]

>>> +Properties of the host controller node that differ from
>>> +host-generic-pci.txt:
>>> +
>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>> +
>>> +Example:
>>> +
>>> +       pci@84b0,00000000 {
>>
>>
>> Drop the comma,
>
>
> OK...
>
>> and the node name should be "pcie".
>>
>
> Why pcie?
>
> There are no PCIe devices or buses reachable from this type of root complex.
> There are however many PCI devices.

I thought ECAM is a PCIe thing. If not, then nevermind.

Rob

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 21:12         ` Rob Herring
  0 siblings, 0 replies; 45+ messages in thread
From: Rob Herring @ 2016-02-08 21:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>
>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>
>>> From: David Daney <david.daney@cavium.com>

[...]

>>> +Properties of the host controller node that differ from
>>> +host-generic-pci.txt:
>>> +
>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>> +
>>> +Example:
>>> +
>>> +       pci at 84b0,00000000 {
>>
>>
>> Drop the comma,
>
>
> OK...
>
>> and the node name should be "pcie".
>>
>
> Why pcie?
>
> There are no PCIe devices or buses reachable from this type of root complex.
> There are however many PCI devices.

I thought ECAM is a PCIe thing. If not, then nevermind.

Rob

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 21:12         ` Rob Herring
  (?)
@ 2016-02-08 21:39           ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 21:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 01:12 PM, Rob Herring wrote:
> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>
>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>
>>>> From: David Daney <david.daney@cavium.com>
>
> [...]
>
>>>> +Properties of the host controller node that differ from
>>>> +host-generic-pci.txt:
>>>> +
>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>> +
>>>> +Example:
>>>> +
>>>> +       pci@84b0,00000000 {
>>>
>>>
>>> Drop the comma,
>>
>>
>> OK...
>>
>>> and the node name should be "pcie".
>>>
>>
>> Why pcie?
>>
>> There are no PCIe devices or buses reachable from this type of root complex.
>> There are however many PCI devices.
>
> I thought ECAM is a PCIe thing. If not, then nevermind.


Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the the 
PCI Express(R) base Specification, but it just defines a standard layout 
of address bits to memory map config space operations.  Since the PCI 
config space is a sub set of the PCIe config space, ECAM can also be 
used in PCI systems.

Really, it is a bit of a gray area here as we don't have any bridges to 
PCIe buses and there are multiple devices residing on each bus, so from 
that point of view it cannot be PCIe.  There are, however, devices that 
implement the PCI Express Capability structure, so does that make it 
PCIe?  It is not clear what the specifications demand here.

I choose to call it "pci" as it lacks much of what it means to be PCIe, 
and also you cannot plug things into it.

David Daney



>
> Rob
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 21:39           ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 21:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 01:12 PM, Rob Herring wrote:
> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>
>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>
>>>> From: David Daney <david.daney@cavium.com>
>
> [...]
>
>>>> +Properties of the host controller node that differ from
>>>> +host-generic-pci.txt:
>>>> +
>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>> +
>>>> +Example:
>>>> +
>>>> +       pci@84b0,00000000 {
>>>
>>>
>>> Drop the comma,
>>
>>
>> OK...
>>
>>> and the node name should be "pcie".
>>>
>>
>> Why pcie?
>>
>> There are no PCIe devices or buses reachable from this type of root complex.
>> There are however many PCI devices.
>
> I thought ECAM is a PCIe thing. If not, then nevermind.


Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the the 
PCI Express(R) base Specification, but it just defines a standard layout 
of address bits to memory map config space operations.  Since the PCI 
config space is a sub set of the PCIe config space, ECAM can also be 
used in PCI systems.

Really, it is a bit of a gray area here as we don't have any bridges to 
PCIe buses and there are multiple devices residing on each bus, so from 
that point of view it cannot be PCIe.  There are, however, devices that 
implement the PCI Express Capability structure, so does that make it 
PCIe?  It is not clear what the specifications demand here.

I choose to call it "pci" as it lacks much of what it means to be PCIe, 
and also you cannot plug things into it.

David Daney



>
> Rob
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 21:39           ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 21:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2016 01:12 PM, Rob Herring wrote:
> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>
>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>
>>>> From: David Daney <david.daney@cavium.com>
>
> [...]
>
>>>> +Properties of the host controller node that differ from
>>>> +host-generic-pci.txt:
>>>> +
>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>> +
>>>> +Example:
>>>> +
>>>> +       pci at 84b0,00000000 {
>>>
>>>
>>> Drop the comma,
>>
>>
>> OK...
>>
>>> and the node name should be "pcie".
>>>
>>
>> Why pcie?
>>
>> There are no PCIe devices or buses reachable from this type of root complex.
>> There are however many PCI devices.
>
> I thought ECAM is a PCIe thing. If not, then nevermind.


Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the the 
PCI Express(R) base Specification, but it just defines a standard layout 
of address bits to memory map config space operations.  Since the PCI 
config space is a sub set of the PCIe config space, ECAM can also be 
used in PCI systems.

Really, it is a bit of a gray area here as we don't have any bridges to 
PCIe buses and there are multiple devices residing on each bus, so from 
that point of view it cannot be PCIe.  There are, however, devices that 
implement the PCI Express Capability structure, so does that make it 
PCIe?  It is not clear what the specifications demand here.

I choose to call it "pci" as it lacks much of what it means to be PCIe, 
and also you cannot plug things into it.

David Daney



>
> Rob
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 21:39           ` David Daney
  (?)
@ 2016-02-08 22:12             ` Bjorn Helgaas
  -1 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 22:12 UTC (permalink / raw)
  To: David Daney
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> On 02/08/2016 01:12 PM, Rob Herring wrote:
> >On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>From: David Daney <david.daney@cavium.com>
> >
> >[...]
> >
> >>>>+Properties of the host controller node that differ from
> >>>>+host-generic-pci.txt:
> >>>>+
> >>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>+
> >>>>+Example:
> >>>>+
> >>>>+       pci@84b0,00000000 {
> >>>
> >>>
> >>>Drop the comma,
> >>
> >>
> >>OK...
> >>
> >>>and the node name should be "pcie".
> >>>
> >>
> >>Why pcie?
> >>
> >>There are no PCIe devices or buses reachable from this type of root complex.
> >>There are however many PCI devices.
> >
> >I thought ECAM is a PCIe thing. If not, then nevermind.

The "ECAM" confusion bites again :)

> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
> the PCI Express(R) base Specification, but it just defines a
> standard layout of address bits to memory map config space
> operations.  Since the PCI config space is a sub set of the PCIe
> config space, ECAM can also be used in PCI systems.
> 
> Really, it is a bit of a gray area here as we don't have any bridges
> to PCIe buses and there are multiple devices residing on each bus,
> so from that point of view it cannot be PCIe.  There are, however,
> devices that implement the PCI Express Capability structure, so does
> that make it PCIe?  It is not clear what the specifications demand
> here.

The PCI core doesn't care about the node name in the device tree.  But
it *does* care about some details of PCI/PCIe topology.  We consider
anything with a PCIe capability to be PCIe.  For example,

  - pci_cfg_space_size() thinks PCIe devices have 4K of config space

  - only_one_child() thinks a PCIe bus, i.e., a link, only has a
    single device on it

  - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
    upstream from it (we did remove some of these restrictions with
    b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but 
    it's possible we didn't get them all)

I assume your system conforms to expectations like these; I'm just
pointing them out because you mentioned buses with multiple devices on
them, which is definitely something one doesn't expect in PCIe.

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 22:12             ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 22:12 UTC (permalink / raw)
  To: David Daney
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> On 02/08/2016 01:12 PM, Rob Herring wrote:
> >On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>From: David Daney <david.daney@cavium.com>
> >
> >[...]
> >
> >>>>+Properties of the host controller node that differ from
> >>>>+host-generic-pci.txt:
> >>>>+
> >>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>+
> >>>>+Example:
> >>>>+
> >>>>+       pci@84b0,00000000 {
> >>>
> >>>
> >>>Drop the comma,
> >>
> >>
> >>OK...
> >>
> >>>and the node name should be "pcie".
> >>>
> >>
> >>Why pcie?
> >>
> >>There are no PCIe devices or buses reachable from this type of root complex.
> >>There are however many PCI devices.
> >
> >I thought ECAM is a PCIe thing. If not, then nevermind.

The "ECAM" confusion bites again :)

> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
> the PCI Express(R) base Specification, but it just defines a
> standard layout of address bits to memory map config space
> operations.  Since the PCI config space is a sub set of the PCIe
> config space, ECAM can also be used in PCI systems.
> 
> Really, it is a bit of a gray area here as we don't have any bridges
> to PCIe buses and there are multiple devices residing on each bus,
> so from that point of view it cannot be PCIe.  There are, however,
> devices that implement the PCI Express Capability structure, so does
> that make it PCIe?  It is not clear what the specifications demand
> here.

The PCI core doesn't care about the node name in the device tree.  But
it *does* care about some details of PCI/PCIe topology.  We consider
anything with a PCIe capability to be PCIe.  For example,

  - pci_cfg_space_size() thinks PCIe devices have 4K of config space

  - only_one_child() thinks a PCIe bus, i.e., a link, only has a
    single device on it

  - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
    upstream from it (we did remove some of these restrictions with
    b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but 
    it's possible we didn't get them all)

I assume your system conforms to expectations like these; I'm just
pointing them out because you mentioned buses with multiple devices on
them, which is definitely something one doesn't expect in PCIe.

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 22:12             ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 22:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> On 02/08/2016 01:12 PM, Rob Herring wrote:
> >On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>From: David Daney <david.daney@cavium.com>
> >
> >[...]
> >
> >>>>+Properties of the host controller node that differ from
> >>>>+host-generic-pci.txt:
> >>>>+
> >>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>+
> >>>>+Example:
> >>>>+
> >>>>+       pci at 84b0,00000000 {
> >>>
> >>>
> >>>Drop the comma,
> >>
> >>
> >>OK...
> >>
> >>>and the node name should be "pcie".
> >>>
> >>
> >>Why pcie?
> >>
> >>There are no PCIe devices or buses reachable from this type of root complex.
> >>There are however many PCI devices.
> >
> >I thought ECAM is a PCIe thing. If not, then nevermind.

The "ECAM" confusion bites again :)

> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
> the PCI Express(R) base Specification, but it just defines a
> standard layout of address bits to memory map config space
> operations.  Since the PCI config space is a sub set of the PCIe
> config space, ECAM can also be used in PCI systems.
> 
> Really, it is a bit of a gray area here as we don't have any bridges
> to PCIe buses and there are multiple devices residing on each bus,
> so from that point of view it cannot be PCIe.  There are, however,
> devices that implement the PCI Express Capability structure, so does
> that make it PCIe?  It is not clear what the specifications demand
> here.

The PCI core doesn't care about the node name in the device tree.  But
it *does* care about some details of PCI/PCIe topology.  We consider
anything with a PCIe capability to be PCIe.  For example,

  - pci_cfg_space_size() thinks PCIe devices have 4K of config space

  - only_one_child() thinks a PCIe bus, i.e., a link, only has a
    single device on it

  - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
    upstream from it (we did remove some of these restrictions with
    b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but 
    it's possible we didn't get them all)

I assume your system conforms to expectations like these; I'm just
pointing them out because you mentioned buses with multiple devices on
them, which is definitely something one doesn't expect in PCIe.

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 22:12             ` Bjorn Helgaas
  (?)
  (?)
@ 2016-02-08 22:41               ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 22:41 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>> From: David Daney <david.daney@cavium.com>
>>>
>>> [...]
>>>
>>>>>> +Properties of the host controller node that differ from
>>>>>> +host-generic-pci.txt:
>>>>>> +
>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +       pci@84b0,00000000 {
>>>>>
>>>>>
>>>>> Drop the comma,
>>>>
>>>>
>>>> OK...
>>>>
>>>>> and the node name should be "pcie".
>>>>>
>>>>
>>>> Why pcie?
>>>>
>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>> There are however many PCI devices.
>>>
>>> I thought ECAM is a PCIe thing. If not, then nevermind.
>
> The "ECAM" confusion bites again :)
>
>> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
>> the PCI Express(R) base Specification, but it just defines a
>> standard layout of address bits to memory map config space
>> operations.  Since the PCI config space is a sub set of the PCIe
>> config space, ECAM can also be used in PCI systems.
>>
>> Really, it is a bit of a gray area here as we don't have any bridges
>> to PCIe buses and there are multiple devices residing on each bus,
>> so from that point of view it cannot be PCIe.  There are, however,
>> devices that implement the PCI Express Capability structure, so does
>> that make it PCIe?  It is not clear what the specifications demand
>> here.
>
> The PCI core doesn't care about the node name in the device tree.  But
> it *does* care about some details of PCI/PCIe topology.  We consider
> anything with a PCIe capability to be PCIe.  For example,
>
>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>
>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>      single device on it
>
>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>      upstream from it (we did remove some of these restrictions with
>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>      it's possible we didn't get them all)
>
> I assume your system conforms to expectations like these; I'm just
> pointing them out because you mentioned buses with multiple devices on
> them, which is definitely something one doesn't expect in PCIe.
>

The topology we have is currently working with the kernel's core PCI 
code.  I don't really want to get into discussing what the definition of 
PCIe is.  We have multiple devices (more than 32) on a single bus, and 
they have PCI Express and ARI Capabilities.  Is that PCIe?  I don't know.

For the purpose of naming the device tree node, I would like to stick 
with the name "pci@..." as it is somewhat accurate, a value contemplated 
by the device tree specifications, ignored by the kernel code, and 
already implemented.

David Daney


> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 22:41               ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 22:41 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Mark Rutland, Pawel Moll, Ian Campbell, linux-pci, David Daney,
	Will Deacon, linux-kernel, devicetree, David Daney, Kumar Gala,
	Bjorn Helgaas, linux-arm-kernel

On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>> From: David Daney <david.daney@cavium.com>
>>>
>>> [...]
>>>
>>>>>> +Properties of the host controller node that differ from
>>>>>> +host-generic-pci.txt:
>>>>>> +
>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +       pci@84b0,00000000 {
>>>>>
>>>>>
>>>>> Drop the comma,
>>>>
>>>>
>>>> OK...
>>>>
>>>>> and the node name should be "pcie".
>>>>>
>>>>
>>>> Why pcie?
>>>>
>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>> There are however many PCI devices.
>>>
>>> I thought ECAM is a PCIe thing. If not, then nevermind.
>
> The "ECAM" confusion bites again :)
>
>> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
>> the PCI Express(R) base Specification, but it just defines a
>> standard layout of address bits to memory map config space
>> operations.  Since the PCI config space is a sub set of the PCIe
>> config space, ECAM can also be used in PCI systems.
>>
>> Really, it is a bit of a gray area here as we don't have any bridges
>> to PCIe buses and there are multiple devices residing on each bus,
>> so from that point of view it cannot be PCIe.  There are, however,
>> devices that implement the PCI Express Capability structure, so does
>> that make it PCIe?  It is not clear what the specifications demand
>> here.
>
> The PCI core doesn't care about the node name in the device tree.  But
> it *does* care about some details of PCI/PCIe topology.  We consider
> anything with a PCIe capability to be PCIe.  For example,
>
>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>
>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>      single device on it
>
>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>      upstream from it (we did remove some of these restrictions with
>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>      it's possible we didn't get them all)
>
> I assume your system conforms to expectations like these; I'm just
> pointing them out because you mentioned buses with multiple devices on
> them, which is definitely something one doesn't expect in PCIe.
>

The topology we have is currently working with the kernel's core PCI 
code.  I don't really want to get into discussing what the definition of 
PCIe is.  We have multiple devices (more than 32) on a single bus, and 
they have PCI Express and ARI Capabilities.  Is that PCIe?  I don't know.

For the purpose of naming the device tree node, I would like to stick 
with the name "pci@..." as it is somewhat accurate, a value contemplated 
by the device tree specifications, ignored by the kernel code, and 
already implemented.

David Daney


> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 22:41               ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 22:41 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>> From: David Daney <david.daney@cavium.com>
>>>
>>> [...]
>>>
>>>>>> +Properties of the host controller node that differ from
>>>>>> +host-generic-pci.txt:
>>>>>> +
>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +       pci@84b0,00000000 {
>>>>>
>>>>>
>>>>> Drop the comma,
>>>>
>>>>
>>>> OK...
>>>>
>>>>> and the node name should be "pcie".
>>>>>
>>>>
>>>> Why pcie?
>>>>
>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>> There are however many PCI devices.
>>>
>>> I thought ECAM is a PCIe thing. If not, then nevermind.
>
> The "ECAM" confusion bites again :)
>
>> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
>> the PCI Express(R) base Specification, but it just defines a
>> standard layout of address bits to memory map config space
>> operations.  Since the PCI config space is a sub set of the PCIe
>> config space, ECAM can also be used in PCI systems.
>>
>> Really, it is a bit of a gray area here as we don't have any bridges
>> to PCIe buses and there are multiple devices residing on each bus,
>> so from that point of view it cannot be PCIe.  There are, however,
>> devices that implement the PCI Express Capability structure, so does
>> that make it PCIe?  It is not clear what the specifications demand
>> here.
>
> The PCI core doesn't care about the node name in the device tree.  But
> it *does* care about some details of PCI/PCIe topology.  We consider
> anything with a PCIe capability to be PCIe.  For example,
>
>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>
>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>      single device on it
>
>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>      upstream from it (we did remove some of these restrictions with
>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>      it's possible we didn't get them all)
>
> I assume your system conforms to expectations like these; I'm just
> pointing them out because you mentioned buses with multiple devices on
> them, which is definitely something one doesn't expect in PCIe.
>

The topology we have is currently working with the kernel's core PCI 
code.  I don't really want to get into discussing what the definition of 
PCIe is.  We have multiple devices (more than 32) on a single bus, and 
they have PCI Express and ARI Capabilities.  Is that PCIe?  I don't know.

For the purpose of naming the device tree node, I would like to stick 
with the name "pci@..." as it is somewhat accurate, a value contemplated 
by the device tree specifications, ignored by the kernel code, and 
already implemented.

David Daney


> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 22:41               ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 22:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>> From: David Daney <david.daney@cavium.com>
>>>
>>> [...]
>>>
>>>>>> +Properties of the host controller node that differ from
>>>>>> +host-generic-pci.txt:
>>>>>> +
>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>> +
>>>>>> +Example:
>>>>>> +
>>>>>> +       pci at 84b0,00000000 {
>>>>>
>>>>>
>>>>> Drop the comma,
>>>>
>>>>
>>>> OK...
>>>>
>>>>> and the node name should be "pcie".
>>>>>
>>>>
>>>> Why pcie?
>>>>
>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>> There are however many PCI devices.
>>>
>>> I thought ECAM is a PCIe thing. If not, then nevermind.
>
> The "ECAM" confusion bites again :)
>
>> Well, Enhanced Configuration Access Mechanism (ECAM) is defined  the
>> the PCI Express(R) base Specification, but it just defines a
>> standard layout of address bits to memory map config space
>> operations.  Since the PCI config space is a sub set of the PCIe
>> config space, ECAM can also be used in PCI systems.
>>
>> Really, it is a bit of a gray area here as we don't have any bridges
>> to PCIe buses and there are multiple devices residing on each bus,
>> so from that point of view it cannot be PCIe.  There are, however,
>> devices that implement the PCI Express Capability structure, so does
>> that make it PCIe?  It is not clear what the specifications demand
>> here.
>
> The PCI core doesn't care about the node name in the device tree.  But
> it *does* care about some details of PCI/PCIe topology.  We consider
> anything with a PCIe capability to be PCIe.  For example,
>
>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>
>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>      single device on it
>
>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>      upstream from it (we did remove some of these restrictions with
>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>      it's possible we didn't get them all)
>
> I assume your system conforms to expectations like these; I'm just
> pointing them out because you mentioned buses with multiple devices on
> them, which is definitely something one doesn't expect in PCIe.
>

The topology we have is currently working with the kernel's core PCI 
code.  I don't really want to get into discussing what the definition of 
PCIe is.  We have multiple devices (more than 32) on a single bus, and 
they have PCI Express and ARI Capabilities.  Is that PCIe?  I don't know.

For the purpose of naming the device tree node, I would like to stick 
with the name "pci at ..." as it is somewhat accurate, a value contemplated 
by the device tree specifications, ignored by the kernel code, and 
already implemented.

David Daney


> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 22:41               ` David Daney
  (?)
@ 2016-02-08 23:24                 ` Bjorn Helgaas
  -1 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 23:24 UTC (permalink / raw)
  To: David Daney
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> >On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> >>On 02/08/2016 01:12 PM, Rob Herring wrote:
> >>>On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>>>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>>>From: David Daney <david.daney@cavium.com>
> >>>>>>+Properties of the host controller node that differ from
> >>>>>>+host-generic-pci.txt:
> >>>>>>+
> >>>>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>>>+
> >>>>>>+Example:
> >>>>>>+
> >>>>>>+       pci@84b0,00000000 {
> ...
> >>>>>and the node name should be "pcie".
> >>>>
> >>>>Why pcie?
> >>>>
> >>>>There are no PCIe devices or buses reachable from this type of root complex.
> >>>>There are however many PCI devices.
> ...

> >>Really, it is a bit of a gray area here as we don't have any bridges
> >>to PCIe buses and there are multiple devices residing on each bus,
> >>so from that point of view it cannot be PCIe.  There are, however,
> >>devices that implement the PCI Express Capability structure, so does
> >>that make it PCIe?  It is not clear what the specifications demand
> >>here.
> >
> >The PCI core doesn't care about the node name in the device tree.  But
> >it *does* care about some details of PCI/PCIe topology.  We consider
> >anything with a PCIe capability to be PCIe.  For example,
> >
> >   - pci_cfg_space_size() thinks PCIe devices have 4K of config space
> >
> >   - only_one_child() thinks a PCIe bus, i.e., a link, only has a
> >     single device on it
> >
> >   - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
> >     upstream from it (we did remove some of these restrictions with
> >     b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
> >     it's possible we didn't get them all)
> >
> >I assume your system conforms to expectations like these; I'm just
> >pointing them out because you mentioned buses with multiple devices on
> >them, which is definitely something one doesn't expect in PCIe.
> 
> The topology we have is currently working with the kernel's core PCI
> code.  I don't really want to get into discussing what the
> definition of PCIe is.  We have multiple devices (more than 32) on a
> single bus, and they have PCI Express and ARI Capabilities.  Is that
> PCIe?  I don't know.

I don't need to know the details of your topology.  As long as it
conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
to the spec, but things currently seem to work, that's less fine,
because a future Linux change is liable to break something for you.

I was a little concerned about your statement that "there are multiple
devices residing on each bus, so from that point of view it cannot be
PCIe."  That made it sound like you're doing something outside the
spec.  If you're just using regular multi-function devices or ARI,
then I don't see any issue (or any reason to say it can't be PCIe).

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 23:24                 ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 23:24 UTC (permalink / raw)
  To: David Daney
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> >On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> >>On 02/08/2016 01:12 PM, Rob Herring wrote:
> >>>On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>>>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>>>From: David Daney <david.daney@cavium.com>
> >>>>>>+Properties of the host controller node that differ from
> >>>>>>+host-generic-pci.txt:
> >>>>>>+
> >>>>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>>>+
> >>>>>>+Example:
> >>>>>>+
> >>>>>>+       pci@84b0,00000000 {
> ...
> >>>>>and the node name should be "pcie".
> >>>>
> >>>>Why pcie?
> >>>>
> >>>>There are no PCIe devices or buses reachable from this type of root complex.
> >>>>There are however many PCI devices.
> ...

> >>Really, it is a bit of a gray area here as we don't have any bridges
> >>to PCIe buses and there are multiple devices residing on each bus,
> >>so from that point of view it cannot be PCIe.  There are, however,
> >>devices that implement the PCI Express Capability structure, so does
> >>that make it PCIe?  It is not clear what the specifications demand
> >>here.
> >
> >The PCI core doesn't care about the node name in the device tree.  But
> >it *does* care about some details of PCI/PCIe topology.  We consider
> >anything with a PCIe capability to be PCIe.  For example,
> >
> >   - pci_cfg_space_size() thinks PCIe devices have 4K of config space
> >
> >   - only_one_child() thinks a PCIe bus, i.e., a link, only has a
> >     single device on it
> >
> >   - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
> >     upstream from it (we did remove some of these restrictions with
> >     b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
> >     it's possible we didn't get them all)
> >
> >I assume your system conforms to expectations like these; I'm just
> >pointing them out because you mentioned buses with multiple devices on
> >them, which is definitely something one doesn't expect in PCIe.
> 
> The topology we have is currently working with the kernel's core PCI
> code.  I don't really want to get into discussing what the
> definition of PCIe is.  We have multiple devices (more than 32) on a
> single bus, and they have PCI Express and ARI Capabilities.  Is that
> PCIe?  I don't know.

I don't need to know the details of your topology.  As long as it
conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
to the spec, but things currently seem to work, that's less fine,
because a future Linux change is liable to break something for you.

I was a little concerned about your statement that "there are multiple
devices residing on each bus, so from that point of view it cannot be
PCIe."  That made it sound like you're doing something outside the
spec.  If you're just using regular multi-function devices or ARI,
then I don't see any issue (or any reason to say it can't be PCIe).

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 23:24                 ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-08 23:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
> >On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
> >>On 02/08/2016 01:12 PM, Rob Herring wrote:
> >>>On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> >>>>On 02/08/2016 11:56 AM, Rob Herring wrote:
> >>>>>On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
> >>>>>>From: David Daney <david.daney@cavium.com>
> >>>>>>+Properties of the host controller node that differ from
> >>>>>>+host-generic-pci.txt:
> >>>>>>+
> >>>>>>+- compatible     : Must be "cavium,pci-host-thunder-ecam"
> >>>>>>+
> >>>>>>+Example:
> >>>>>>+
> >>>>>>+       pci at 84b0,00000000 {
> ...
> >>>>>and the node name should be "pcie".
> >>>>
> >>>>Why pcie?
> >>>>
> >>>>There are no PCIe devices or buses reachable from this type of root complex.
> >>>>There are however many PCI devices.
> ...

> >>Really, it is a bit of a gray area here as we don't have any bridges
> >>to PCIe buses and there are multiple devices residing on each bus,
> >>so from that point of view it cannot be PCIe.  There are, however,
> >>devices that implement the PCI Express Capability structure, so does
> >>that make it PCIe?  It is not clear what the specifications demand
> >>here.
> >
> >The PCI core doesn't care about the node name in the device tree.  But
> >it *does* care about some details of PCI/PCIe topology.  We consider
> >anything with a PCIe capability to be PCIe.  For example,
> >
> >   - pci_cfg_space_size() thinks PCIe devices have 4K of config space
> >
> >   - only_one_child() thinks a PCIe bus, i.e., a link, only has a
> >     single device on it
> >
> >   - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
> >     upstream from it (we did remove some of these restrictions with
> >     b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
> >     it's possible we didn't get them all)
> >
> >I assume your system conforms to expectations like these; I'm just
> >pointing them out because you mentioned buses with multiple devices on
> >them, which is definitely something one doesn't expect in PCIe.
> 
> The topology we have is currently working with the kernel's core PCI
> code.  I don't really want to get into discussing what the
> definition of PCIe is.  We have multiple devices (more than 32) on a
> single bus, and they have PCI Express and ARI Capabilities.  Is that
> PCIe?  I don't know.

I don't need to know the details of your topology.  As long as it
conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
to the spec, but things currently seem to work, that's less fine,
because a future Linux change is liable to break something for you.

I was a little concerned about your statement that "there are multiple
devices residing on each bus, so from that point of view it cannot be
PCIe."  That made it sound like you're doing something outside the
spec.  If you're just using regular multi-function devices or ARI,
then I don't see any issue (or any reason to say it can't be PCIe).

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 23:24                 ` Bjorn Helgaas
  (?)
@ 2016-02-08 23:31                   ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 23:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 03:24 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
>> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
>>> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>>>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>>>> From: David Daney <david.daney@cavium.com>
>>>>>>>> +Properties of the host controller node that differ from
>>>>>>>> +host-generic-pci.txt:
>>>>>>>> +
>>>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>>>> +
>>>>>>>> +Example:
>>>>>>>> +
>>>>>>>> +       pci@84b0,00000000 {
>> ...
>>>>>>> and the node name should be "pcie".
>>>>>>
>>>>>> Why pcie?
>>>>>>
>>>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>>>> There are however many PCI devices.
>> ...
>
>>>> Really, it is a bit of a gray area here as we don't have any bridges
>>>> to PCIe buses and there are multiple devices residing on each bus,
>>>> so from that point of view it cannot be PCIe.  There are, however,
>>>> devices that implement the PCI Express Capability structure, so does
>>>> that make it PCIe?  It is not clear what the specifications demand
>>>> here.
>>>
>>> The PCI core doesn't care about the node name in the device tree.  But
>>> it *does* care about some details of PCI/PCIe topology.  We consider
>>> anything with a PCIe capability to be PCIe.  For example,
>>>
>>>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>>>
>>>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>>>      single device on it
>>>
>>>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>>>      upstream from it (we did remove some of these restrictions with
>>>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>>>      it's possible we didn't get them all)
>>>
>>> I assume your system conforms to expectations like these; I'm just
>>> pointing them out because you mentioned buses with multiple devices on
>>> them, which is definitely something one doesn't expect in PCIe.
>>
>> The topology we have is currently working with the kernel's core PCI
>> code.  I don't really want to get into discussing what the
>> definition of PCIe is.  We have multiple devices (more than 32) on a
>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>> PCIe?  I don't know.
>
> I don't need to know the details of your topology.  As long as it
> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> to the spec, but things currently seem to work, that's less fine,
> because a future Linux change is liable to break something for you.
>
> I was a little concerned about your statement that "there are multiple
> devices residing on each bus, so from that point of view it cannot be
> PCIe."  That made it sound like you're doing something outside the
> spec.  If you're just using regular multi-function devices or ARI,
> then I don't see any issue (or any reason to say it can't be PCIe).

OK, I will make it "pcie@...."

Really, ARI is the only reason.  But since ARI is defined in the PCI 
Express specification, pcie it is.

I will send revised patches today.


David Daney

>
> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 23:31                   ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 23:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rob Herring, David Daney, Bjorn Helgaas, linux-pci, Will Deacon,
	linux-arm-kernel, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, David Daney

On 02/08/2016 03:24 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
>> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
>>> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>>>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>>>> From: David Daney <david.daney@cavium.com>
>>>>>>>> +Properties of the host controller node that differ from
>>>>>>>> +host-generic-pci.txt:
>>>>>>>> +
>>>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>>>> +
>>>>>>>> +Example:
>>>>>>>> +
>>>>>>>> +       pci@84b0,00000000 {
>> ...
>>>>>>> and the node name should be "pcie".
>>>>>>
>>>>>> Why pcie?
>>>>>>
>>>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>>>> There are however many PCI devices.
>> ...
>
>>>> Really, it is a bit of a gray area here as we don't have any bridges
>>>> to PCIe buses and there are multiple devices residing on each bus,
>>>> so from that point of view it cannot be PCIe.  There are, however,
>>>> devices that implement the PCI Express Capability structure, so does
>>>> that make it PCIe?  It is not clear what the specifications demand
>>>> here.
>>>
>>> The PCI core doesn't care about the node name in the device tree.  But
>>> it *does* care about some details of PCI/PCIe topology.  We consider
>>> anything with a PCIe capability to be PCIe.  For example,
>>>
>>>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>>>
>>>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>>>      single device on it
>>>
>>>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>>>      upstream from it (we did remove some of these restrictions with
>>>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>>>      it's possible we didn't get them all)
>>>
>>> I assume your system conforms to expectations like these; I'm just
>>> pointing them out because you mentioned buses with multiple devices on
>>> them, which is definitely something one doesn't expect in PCIe.
>>
>> The topology we have is currently working with the kernel's core PCI
>> code.  I don't really want to get into discussing what the
>> definition of PCIe is.  We have multiple devices (more than 32) on a
>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>> PCIe?  I don't know.
>
> I don't need to know the details of your topology.  As long as it
> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> to the spec, but things currently seem to work, that's less fine,
> because a future Linux change is liable to break something for you.
>
> I was a little concerned about your statement that "there are multiple
> devices residing on each bus, so from that point of view it cannot be
> PCIe."  That made it sound like you're doing something outside the
> spec.  If you're just using regular multi-function devices or ARI,
> then I don't see any issue (or any reason to say it can't be PCIe).

OK, I will make it "pcie@...."

Really, ARI is the only reason.  But since ARI is defined in the PCI 
Express specification, pcie it is.

I will send revised patches today.


David Daney

>
> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-08 23:31                   ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-08 23:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2016 03:24 PM, Bjorn Helgaas wrote:
> On Mon, Feb 08, 2016 at 02:41:41PM -0800, David Daney wrote:
>> On 02/08/2016 02:12 PM, Bjorn Helgaas wrote:
>>> On Mon, Feb 08, 2016 at 01:39:21PM -0800, David Daney wrote:
>>>> On 02/08/2016 01:12 PM, Rob Herring wrote:
>>>>> On Mon, Feb 8, 2016 at 2:47 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>>>>> On 02/08/2016 11:56 AM, Rob Herring wrote:
>>>>>>> On Fri, Feb 05, 2016 at 03:41:15PM -0800, David Daney wrote:
>>>>>>>> From: David Daney <david.daney@cavium.com>
>>>>>>>> +Properties of the host controller node that differ from
>>>>>>>> +host-generic-pci.txt:
>>>>>>>> +
>>>>>>>> +- compatible     : Must be "cavium,pci-host-thunder-ecam"
>>>>>>>> +
>>>>>>>> +Example:
>>>>>>>> +
>>>>>>>> +       pci at 84b0,00000000 {
>> ...
>>>>>>> and the node name should be "pcie".
>>>>>>
>>>>>> Why pcie?
>>>>>>
>>>>>> There are no PCIe devices or buses reachable from this type of root complex.
>>>>>> There are however many PCI devices.
>> ...
>
>>>> Really, it is a bit of a gray area here as we don't have any bridges
>>>> to PCIe buses and there are multiple devices residing on each bus,
>>>> so from that point of view it cannot be PCIe.  There are, however,
>>>> devices that implement the PCI Express Capability structure, so does
>>>> that make it PCIe?  It is not clear what the specifications demand
>>>> here.
>>>
>>> The PCI core doesn't care about the node name in the device tree.  But
>>> it *does* care about some details of PCI/PCIe topology.  We consider
>>> anything with a PCIe capability to be PCIe.  For example,
>>>
>>>    - pci_cfg_space_size() thinks PCIe devices have 4K of config space
>>>
>>>    - only_one_child() thinks a PCIe bus, i.e., a link, only has a
>>>      single device on it
>>>
>>>    - a PCIe device should have a PCIe Root Port or PCIe Downstream Port
>>>      upstream from it (we did remove some of these restrictions with
>>>      b35b1df5e6c2 ("PCI: Tolerate hierarchies with no Root Port"), but
>>>      it's possible we didn't get them all)
>>>
>>> I assume your system conforms to expectations like these; I'm just
>>> pointing them out because you mentioned buses with multiple devices on
>>> them, which is definitely something one doesn't expect in PCIe.
>>
>> The topology we have is currently working with the kernel's core PCI
>> code.  I don't really want to get into discussing what the
>> definition of PCIe is.  We have multiple devices (more than 32) on a
>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>> PCIe?  I don't know.
>
> I don't need to know the details of your topology.  As long as it
> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> to the spec, but things currently seem to work, that's less fine,
> because a future Linux change is liable to break something for you.
>
> I was a little concerned about your statement that "there are multiple
> devices residing on each bus, so from that point of view it cannot be
> PCIe."  That made it sound like you're doing something outside the
> spec.  If you're just using regular multi-function devices or ARI,
> then I don't see any issue (or any reason to say it can't be PCIe).

OK, I will make it "pcie at ...."

Really, ARI is the only reason.  But since ARI is defined in the PCI 
Express specification, pcie it is.

I will send revised patches today.


David Daney

>
> Bjorn
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-08 23:24                 ` Bjorn Helgaas
@ 2016-02-09  9:25                   ` Arnd Bergmann
  -1 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2016-02-09  9:25 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Bjorn Helgaas, David Daney, Mark Rutland, Pawel Moll,
	Ian Campbell, linux-pci, David Daney, Will Deacon, linux-kernel,
	devicetree, David Daney, Kumar Gala, Bjorn Helgaas

On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > >
> > >I assume your system conforms to expectations like these; I'm just
> > >pointing them out because you mentioned buses with multiple devices on
> > >them, which is definitely something one doesn't expect in PCIe.
> > 
> > The topology we have is currently working with the kernel's core PCI
> > code.  I don't really want to get into discussing what the
> > definition of PCIe is.  We have multiple devices (more than 32) on a
> > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > PCIe?  I don't know.
> 
> I don't need to know the details of your topology.  As long as it
> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> to the spec, but things currently seem to work, that's less fine,
> because a future Linux change is liable to break something for you.
> 
> I was a little concerned about your statement that "there are multiple
> devices residing on each bus, so from that point of view it cannot be
> PCIe."  That made it sound like you're doing something outside the
> spec.  If you're just using regular multi-function devices or ARI,
> then I don't see any issue (or any reason to say it can't be PCIe).

It doesn't conform to the PCIe port spec, because there are no external
ports but just integrated devices in the host bridge. For this special
case, I don't think it matters at all from the point of view of the DT
binding whether we call the node name "pci" or "pcie".

IIRC, even on real Open Firmware, the three companies that shipped
PCIe (or Hypertransport, which doesn't even have a formal binding)
based machines (Sun, IBM, Apple) were using slightly different
bindings in practice, so I wouldn't read to much into it. Any OS
that wants to run on real OF already has to support it either way.

	Arnd

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-09  9:25                   ` Arnd Bergmann
  0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2016-02-09  9:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > >
> > >I assume your system conforms to expectations like these; I'm just
> > >pointing them out because you mentioned buses with multiple devices on
> > >them, which is definitely something one doesn't expect in PCIe.
> > 
> > The topology we have is currently working with the kernel's core PCI
> > code.  I don't really want to get into discussing what the
> > definition of PCIe is.  We have multiple devices (more than 32) on a
> > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > PCIe?  I don't know.
> 
> I don't need to know the details of your topology.  As long as it
> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> to the spec, but things currently seem to work, that's less fine,
> because a future Linux change is liable to break something for you.
> 
> I was a little concerned about your statement that "there are multiple
> devices residing on each bus, so from that point of view it cannot be
> PCIe."  That made it sound like you're doing something outside the
> spec.  If you're just using regular multi-function devices or ARI,
> then I don't see any issue (or any reason to say it can't be PCIe).

It doesn't conform to the PCIe port spec, because there are no external
ports but just integrated devices in the host bridge. For this special
case, I don't think it matters at all from the point of view of the DT
binding whether we call the node name "pci" or "pcie".

IIRC, even on real Open Firmware, the three companies that shipped
PCIe (or Hypertransport, which doesn't even have a formal binding)
based machines (Sun, IBM, Apple) were using slightly different
bindings in practice, so I wouldn't read to much into it. Any OS
that wants to run on real OF already has to support it either way.

	Arnd

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-09  9:25                   ` Arnd Bergmann
@ 2016-02-09 16:26                     ` Bjorn Helgaas
  -1 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-09 16:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, David Daney, Mark Rutland, Pawel Moll,
	Ian Campbell, linux-pci, David Daney, Will Deacon, linux-kernel,
	devicetree, David Daney, Kumar Gala, Bjorn Helgaas

On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
> On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > > >
> > > >I assume your system conforms to expectations like these; I'm just
> > > >pointing them out because you mentioned buses with multiple devices on
> > > >them, which is definitely something one doesn't expect in PCIe.
> > > 
> > > The topology we have is currently working with the kernel's core PCI
> > > code.  I don't really want to get into discussing what the
> > > definition of PCIe is.  We have multiple devices (more than 32) on a
> > > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > > PCIe?  I don't know.
> > 
> > I don't need to know the details of your topology.  As long as it
> > conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> > to the spec, but things currently seem to work, that's less fine,
> > because a future Linux change is liable to break something for you.
> > 
> > I was a little concerned about your statement that "there are multiple
> > devices residing on each bus, so from that point of view it cannot be
> > PCIe."  That made it sound like you're doing something outside the
> > spec.  If you're just using regular multi-function devices or ARI,
> > then I don't see any issue (or any reason to say it can't be PCIe).
> 
> It doesn't conform to the PCIe port spec, because there are no external
> ports but just integrated devices in the host bridge. 

Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
think there's a requirement to have PCI Express Ports (is that what
you mean by "external ports"?)

Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
and they would not be behind a Root Port.  If you're using those, I
hope they're correctly identified via the PCIe capability Device/Port
Type (sec 7.8.2) because we rely on that type to figure out whether
the link-related registers are implemented.

The spec does include rules related to peer-to-peer transactions, MPS,
ASPM, error reporting, etc., and Linux relies on those, so I think it
would be important to get those right.

> For this special
> case, I don't think it matters at all from the point of view of the DT
> binding whether we call the node name "pci" or "pcie".

And the PCI core doesn't even know the node name, it doesn't matter
there either.

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-09 16:26                     ` Bjorn Helgaas
  0 siblings, 0 replies; 45+ messages in thread
From: Bjorn Helgaas @ 2016-02-09 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
> On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > > >
> > > >I assume your system conforms to expectations like these; I'm just
> > > >pointing them out because you mentioned buses with multiple devices on
> > > >them, which is definitely something one doesn't expect in PCIe.
> > > 
> > > The topology we have is currently working with the kernel's core PCI
> > > code.  I don't really want to get into discussing what the
> > > definition of PCIe is.  We have multiple devices (more than 32) on a
> > > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > > PCIe?  I don't know.
> > 
> > I don't need to know the details of your topology.  As long as it
> > conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> > to the spec, but things currently seem to work, that's less fine,
> > because a future Linux change is liable to break something for you.
> > 
> > I was a little concerned about your statement that "there are multiple
> > devices residing on each bus, so from that point of view it cannot be
> > PCIe."  That made it sound like you're doing something outside the
> > spec.  If you're just using regular multi-function devices or ARI,
> > then I don't see any issue (or any reason to say it can't be PCIe).
> 
> It doesn't conform to the PCIe port spec, because there are no external
> ports but just integrated devices in the host bridge. 

Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
think there's a requirement to have PCI Express Ports (is that what
you mean by "external ports"?)

Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
and they would not be behind a Root Port.  If you're using those, I
hope they're correctly identified via the PCIe capability Device/Port
Type (sec 7.8.2) because we rely on that type to figure out whether
the link-related registers are implemented.

The spec does include rules related to peer-to-peer transactions, MPS,
ASPM, error reporting, etc., and Linux relies on those, so I think it
would be important to get those right.

> For this special
> case, I don't think it matters at all from the point of view of the DT
> binding whether we call the node name "pci" or "pcie".

And the PCI core doesn't even know the node name, it doesn't matter
there either.

Bjorn

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-09 16:26                     ` Bjorn Helgaas
@ 2016-02-09 16:31                       ` Arnd Bergmann
  -1 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-arm-kernel, David Daney, Mark Rutland, Pawel Moll,
	Ian Campbell, linux-pci, David Daney, Will Deacon, linux-kernel,
	devicetree, David Daney, Kumar Gala, Bjorn Helgaas

On Tuesday 09 February 2016 10:26:28 Bjorn Helgaas wrote:
> On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
> > On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > > > >
> > > > >I assume your system conforms to expectations like these; I'm just
> > > > >pointing them out because you mentioned buses with multiple devices on
> > > > >them, which is definitely something one doesn't expect in PCIe.
> > > > 
> > > > The topology we have is currently working with the kernel's core PCI
> > > > code.  I don't really want to get into discussing what the
> > > > definition of PCIe is.  We have multiple devices (more than 32) on a
> > > > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > > > PCIe?  I don't know.
> > > 
> > > I don't need to know the details of your topology.  As long as it
> > > conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> > > to the spec, but things currently seem to work, that's less fine,
> > > because a future Linux change is liable to break something for you.
> > > 
> > > I was a little concerned about your statement that "there are multiple
> > > devices residing on each bus, so from that point of view it cannot be
> > > PCIe."  That made it sound like you're doing something outside the
> > > spec.  If you're just using regular multi-function devices or ARI,
> > > then I don't see any issue (or any reason to say it can't be PCIe).
> > 
> > It doesn't conform to the PCIe port spec, because there are no external
> > ports but just integrated devices in the host bridge. 
> 
> Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
> think there's a requirement to have PCI Express Ports (is that what
> you mean by "external ports"?)

No, I was just assuming that ports are specified in their own document,
which would not be followed here if there are none. There is nothing in
here that leads me to believe that the hardware is actually noncompliant
with any relevant standard.

> Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
> and they would not be behind a Root Port.  If you're using those, I
> hope they're correctly identified via the PCIe capability Device/Port
> Type (sec 7.8.2) because we rely on that type to figure out whether
> the link-related registers are implemented.
> 
> The spec does include rules related to peer-to-peer transactions, MPS,
> ASPM, error reporting, etc., and Linux relies on those, so I think it
> would be important to get those right.

David can probably explain more if the registers are compliant with
those parts of the spec.

	Arnd

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-09 16:31                       ` Arnd Bergmann
  0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 09 February 2016 10:26:28 Bjorn Helgaas wrote:
> On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
> > On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
> > > > >
> > > > >I assume your system conforms to expectations like these; I'm just
> > > > >pointing them out because you mentioned buses with multiple devices on
> > > > >them, which is definitely something one doesn't expect in PCIe.
> > > > 
> > > > The topology we have is currently working with the kernel's core PCI
> > > > code.  I don't really want to get into discussing what the
> > > > definition of PCIe is.  We have multiple devices (more than 32) on a
> > > > single bus, and they have PCI Express and ARI Capabilities.  Is that
> > > > PCIe?  I don't know.
> > > 
> > > I don't need to know the details of your topology.  As long as it
> > > conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
> > > to the spec, but things currently seem to work, that's less fine,
> > > because a future Linux change is liable to break something for you.
> > > 
> > > I was a little concerned about your statement that "there are multiple
> > > devices residing on each bus, so from that point of view it cannot be
> > > PCIe."  That made it sound like you're doing something outside the
> > > spec.  If you're just using regular multi-function devices or ARI,
> > > then I don't see any issue (or any reason to say it can't be PCIe).
> > 
> > It doesn't conform to the PCIe port spec, because there are no external
> > ports but just integrated devices in the host bridge. 
> 
> Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
> think there's a requirement to have PCI Express Ports (is that what
> you mean by "external ports"?)

No, I was just assuming that ports are specified in their own document,
which would not be followed here if there are none. There is nothing in
here that leads me to believe that the hardware is actually noncompliant
with any relevant standard.

> Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
> and they would not be behind a Root Port.  If you're using those, I
> hope they're correctly identified via the PCIe capability Device/Port
> Type (sec 7.8.2) because we rely on that type to figure out whether
> the link-related registers are implemented.
> 
> The spec does include rules related to peer-to-peer transactions, MPS,
> ASPM, error reporting, etc., and Linux relies on those, so I think it
> would be important to get those right.

David can probably explain more if the registers are compliant with
those parts of the spec.

	Arnd

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
  2016-02-09 16:31                       ` Arnd Bergmann
  (?)
@ 2016-02-09 16:58                         ` David Daney
  -1 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-09 16:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Bjorn Helgaas, linux-arm-kernel, Mark Rutland, Pawel Moll,
	Ian Campbell, linux-pci, David Daney, Will Deacon, linux-kernel,
	devicetree, David Daney, Kumar Gala, Bjorn Helgaas

On 02/09/2016 08:31 AM, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 10:26:28 Bjorn Helgaas wrote:
>> On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
>>> On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
>>>>>>
>>>>>> I assume your system conforms to expectations like these; I'm just
>>>>>> pointing them out because you mentioned buses with multiple devices on
>>>>>> them, which is definitely something one doesn't expect in PCIe.
>>>>>
>>>>> The topology we have is currently working with the kernel's core PCI
>>>>> code.  I don't really want to get into discussing what the
>>>>> definition of PCIe is.  We have multiple devices (more than 32) on a
>>>>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>>>>> PCIe?  I don't know.
>>>>
>>>> I don't need to know the details of your topology.  As long as it
>>>> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
>>>> to the spec, but things currently seem to work, that's less fine,
>>>> because a future Linux change is liable to break something for you.
>>>>
>>>> I was a little concerned about your statement that "there are multiple
>>>> devices residing on each bus, so from that point of view it cannot be
>>>> PCIe."  That made it sound like you're doing something outside the
>>>> spec.  If you're just using regular multi-function devices or ARI,
>>>> then I don't see any issue (or any reason to say it can't be PCIe).
>>>
>>> It doesn't conform to the PCIe port spec, because there are no external
>>> ports but just integrated devices in the host bridge.
>>
>> Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
>> think there's a requirement to have PCI Express Ports (is that what
>> you mean by "external ports"?)
>
> No, I was just assuming that ports are specified in their own document,
> which would not be followed here if there are none. There is nothing in
> here that leads me to believe that the hardware is actually noncompliant
> with any relevant standard.
>
>> Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
>> and they would not be behind a Root Port.  If you're using those, I
>> hope they're correctly identified via the PCIe capability Device/Port
>> Type (sec 7.8.2) because we rely on that type to figure out whether
>> the link-related registers are implemented.
>>
>> The spec does include rules related to peer-to-peer transactions, MPS,
>> ASPM, error reporting, etc., and Linux relies on those, so I think it
>> would be important to get those right.
>
> David can probably explain more if the registers are compliant with
> those parts of the spec.

It is somewhat moot, but in the interest of keeping this thread alive:

None of the "on-chip" devices behind these root complexes implements 
ASPM, AER, etc.  The capability structures for all the features you 
mention are not present.

All that is there beyond standard PCI capabilities are:

   - PCI Express capability, to indicate presence of PCI Express 
Extended Capabilities.

   - ARI capability so we can fit more than 16 devices on a bus.

   - SRIOV capability on devices that are virtualizable.

   - That's it!

The reality is that they are not really PCI/PCIe devices at all.  All 
the device registers are at fixed addresses and are connected to 
proprietary internal buses with various weird properties.  Witness the 
need for Enhanced Allocation capabilities to describe the fixed addressing.

The PCI config space is a veneer laid on top of it all to aid in device 
discovery and interrupt routing.

So is it PCI or PCIe?  It is not really important to say.  All we want, 
is to be able to get the pci-host-generic root complex driver to bind to 
our ECAM/ECAM-like configuration space accessors.

David Daney



>
> 	Arnd
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-09 16:58                         ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-09 16:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Bjorn Helgaas, linux-arm-kernel, Mark Rutland, Pawel Moll,
	Ian Campbell, linux-pci, David Daney, Will Deacon, linux-kernel,
	devicetree, David Daney, Kumar Gala, Bjorn Helgaas

On 02/09/2016 08:31 AM, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 10:26:28 Bjorn Helgaas wrote:
>> On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
>>> On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
>>>>>>
>>>>>> I assume your system conforms to expectations like these; I'm just
>>>>>> pointing them out because you mentioned buses with multiple devices on
>>>>>> them, which is definitely something one doesn't expect in PCIe.
>>>>>
>>>>> The topology we have is currently working with the kernel's core PCI
>>>>> code.  I don't really want to get into discussing what the
>>>>> definition of PCIe is.  We have multiple devices (more than 32) on a
>>>>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>>>>> PCIe?  I don't know.
>>>>
>>>> I don't need to know the details of your topology.  As long as it
>>>> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
>>>> to the spec, but things currently seem to work, that's less fine,
>>>> because a future Linux change is liable to break something for you.
>>>>
>>>> I was a little concerned about your statement that "there are multiple
>>>> devices residing on each bus, so from that point of view it cannot be
>>>> PCIe."  That made it sound like you're doing something outside the
>>>> spec.  If you're just using regular multi-function devices or ARI,
>>>> then I don't see any issue (or any reason to say it can't be PCIe).
>>>
>>> It doesn't conform to the PCIe port spec, because there are no external
>>> ports but just integrated devices in the host bridge.
>>
>> Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
>> think there's a requirement to have PCI Express Ports (is that what
>> you mean by "external ports"?)
>
> No, I was just assuming that ports are specified in their own document,
> which would not be followed here if there are none. There is nothing in
> here that leads me to believe that the hardware is actually noncompliant
> with any relevant standard.
>
>> Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
>> and they would not be behind a Root Port.  If you're using those, I
>> hope they're correctly identified via the PCIe capability Device/Port
>> Type (sec 7.8.2) because we rely on that type to figure out whether
>> the link-related registers are implemented.
>>
>> The spec does include rules related to peer-to-peer transactions, MPS,
>> ASPM, error reporting, etc., and Linux relies on those, so I think it
>> would be important to get those right.
>
> David can probably explain more if the registers are compliant with
> those parts of the spec.

It is somewhat moot, but in the interest of keeping this thread alive:

None of the "on-chip" devices behind these root complexes implements 
ASPM, AER, etc.  The capability structures for all the features you 
mention are not present.

All that is there beyond standard PCI capabilities are:

   - PCI Express capability, to indicate presence of PCI Express 
Extended Capabilities.

   - ARI capability so we can fit more than 16 devices on a bus.

   - SRIOV capability on devices that are virtualizable.

   - That's it!

The reality is that they are not really PCI/PCIe devices at all.  All 
the device registers are at fixed addresses and are connected to 
proprietary internal buses with various weird properties.  Witness the 
need for Enhanced Allocation capabilities to describe the fixed addressing.

The PCI config space is a veneer laid on top of it all to aid in device 
discovery and interrupt routing.

So is it PCI or PCIe?  It is not really important to say.  All we want, 
is to be able to get the pci-host-generic root complex driver to bind to 
our ECAM/ECAM-like configuration space accessors.

David Daney



>
> 	Arnd
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices
@ 2016-02-09 16:58                         ` David Daney
  0 siblings, 0 replies; 45+ messages in thread
From: David Daney @ 2016-02-09 16:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/09/2016 08:31 AM, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 10:26:28 Bjorn Helgaas wrote:
>> On Tue, Feb 09, 2016 at 10:25:33AM +0100, Arnd Bergmann wrote:
>>> On Monday 08 February 2016 17:24:30 Bjorn Helgaas wrote:
>>>>>>
>>>>>> I assume your system conforms to expectations like these; I'm just
>>>>>> pointing them out because you mentioned buses with multiple devices on
>>>>>> them, which is definitely something one doesn't expect in PCIe.
>>>>>
>>>>> The topology we have is currently working with the kernel's core PCI
>>>>> code.  I don't really want to get into discussing what the
>>>>> definition of PCIe is.  We have multiple devices (more than 32) on a
>>>>> single bus, and they have PCI Express and ARI Capabilities.  Is that
>>>>> PCIe?  I don't know.
>>>>
>>>> I don't need to know the details of your topology.  As long as it
>>>> conforms to the PCIe spec, it should be fine.  If it *doesn't* conform
>>>> to the spec, but things currently seem to work, that's less fine,
>>>> because a future Linux change is liable to break something for you.
>>>>
>>>> I was a little concerned about your statement that "there are multiple
>>>> devices residing on each bus, so from that point of view it cannot be
>>>> PCIe."  That made it sound like you're doing something outside the
>>>> spec.  If you're just using regular multi-function devices or ARI,
>>>> then I don't see any issue (or any reason to say it can't be PCIe).
>>>
>>> It doesn't conform to the PCIe port spec, because there are no external
>>> ports but just integrated devices in the host bridge.
>>
>> Is there a spec section you have in mind?  Based on sec 1.3.1, I don't
>> think there's a requirement to have PCI Express Ports (is that what
>> you mean by "external ports"?)
>
> No, I was just assuming that ports are specified in their own document,
> which would not be followed here if there are none. There is nothing in
> here that leads me to believe that the hardware is actually noncompliant
> with any relevant standard.
>
>> Root Complex Integrated Endpoints (sec 1.3.2.3) are clearly supported
>> and they would not be behind a Root Port.  If you're using those, I
>> hope they're correctly identified via the PCIe capability Device/Port
>> Type (sec 7.8.2) because we rely on that type to figure out whether
>> the link-related registers are implemented.
>>
>> The spec does include rules related to peer-to-peer transactions, MPS,
>> ASPM, error reporting, etc., and Linux relies on those, so I think it
>> would be important to get those right.
>
> David can probably explain more if the registers are compliant with
> those parts of the spec.

It is somewhat moot, but in the interest of keeping this thread alive:

None of the "on-chip" devices behind these root complexes implements 
ASPM, AER, etc.  The capability structures for all the features you 
mention are not present.

All that is there beyond standard PCI capabilities are:

   - PCI Express capability, to indicate presence of PCI Express 
Extended Capabilities.

   - ARI capability so we can fit more than 16 devices on a bus.

   - SRIOV capability on devices that are virtualizable.

   - That's it!

The reality is that they are not really PCI/PCIe devices at all.  All 
the device registers are at fixed addresses and are connected to 
proprietary internal buses with various weird properties.  Witness the 
need for Enhanced Allocation capabilities to describe the fixed addressing.

The PCI config space is a veneer laid on top of it all to aid in device 
discovery and interrupt routing.

So is it PCI or PCIe?  It is not really important to say.  All we want, 
is to be able to get the pci-host-generic root complex driver to bind to 
our ECAM/ECAM-like configuration space accessors.

David Daney



>
> 	Arnd
>

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2016-02-09 16:59 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-05 23:41 [PATCH v5 0/3] Add host controller drivers for Cavium ThunderX PCI David Daney
2016-02-05 23:41 ` David Daney
2016-02-05 23:41 ` David Daney
2016-02-05 23:41 ` [PATCH v5 1/3] PCI: generic: Refactor code to enable reuse by other drivers David Daney
2016-02-05 23:41   ` David Daney
2016-02-05 23:41 ` [PATCH v5 2/3] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors David Daney
2016-02-05 23:41   ` David Daney
2016-02-06 16:01   ` Bjorn Helgaas
2016-02-06 16:01     ` Bjorn Helgaas
2016-02-06 16:01     ` Bjorn Helgaas
2016-02-05 23:41 ` [PATCH v5 3/3] pci, pci-thunder-ecam: Add driver for ThunderX-pass1 on-chip devices David Daney
2016-02-05 23:41   ` David Daney
2016-02-08 19:56   ` Rob Herring
2016-02-08 19:56     ` Rob Herring
2016-02-08 20:47     ` David Daney
2016-02-08 20:47       ` David Daney
2016-02-08 20:47       ` David Daney
2016-02-08 21:12       ` Rob Herring
2016-02-08 21:12         ` Rob Herring
2016-02-08 21:12         ` Rob Herring
2016-02-08 21:39         ` David Daney
2016-02-08 21:39           ` David Daney
2016-02-08 21:39           ` David Daney
2016-02-08 22:12           ` Bjorn Helgaas
2016-02-08 22:12             ` Bjorn Helgaas
2016-02-08 22:12             ` Bjorn Helgaas
2016-02-08 22:41             ` David Daney
2016-02-08 22:41               ` David Daney
2016-02-08 22:41               ` David Daney
2016-02-08 22:41               ` David Daney
2016-02-08 23:24               ` Bjorn Helgaas
2016-02-08 23:24                 ` Bjorn Helgaas
2016-02-08 23:24                 ` Bjorn Helgaas
2016-02-08 23:31                 ` David Daney
2016-02-08 23:31                   ` David Daney
2016-02-08 23:31                   ` David Daney
2016-02-09  9:25                 ` Arnd Bergmann
2016-02-09  9:25                   ` Arnd Bergmann
2016-02-09 16:26                   ` Bjorn Helgaas
2016-02-09 16:26                     ` Bjorn Helgaas
2016-02-09 16:31                     ` Arnd Bergmann
2016-02-09 16:31                       ` Arnd Bergmann
2016-02-09 16:58                       ` David Daney
2016-02-09 16:58                         ` David Daney
2016-02-09 16:58                         ` David Daney

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.