[PATCH 0/2] pci: Add host controller driver for Cavium ThunderX PCIe

[PATCH 0/2] pci: Add host controller driver for Cavium ThunderX PCIe

[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 two patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.

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

David Daney (2):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

 .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-host-generic.c                |  53 ++--
 drivers/pci/host/pci-host-generic.h                |  56 ++++
 drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
 6 files changed, 407 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-generic.h
 create mode 100644 drivers/pci/host/pcie-thunder-pem.c

-- 
1.8.3.1

[PATCH 0/2] pci: Add host controller driver for Cavium ThunderX PCIe

[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 two patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.

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

David Daney (2):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

 .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-host-generic.c                |  53 ++--
 drivers/pci/host/pci-host-generic.h                |  56 ++++
 drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
 6 files changed, 407 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-generic.h
 create mode 100644 drivers/pci/host/pcie-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

[PATCH 0/2] pci: Add host controller driver for Cavium ThunderX PCIe

[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 two patches:

1) Refactor code in pci-host-generic so that it can more easily be
   used by other drivers.

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

David Daney (2):
  PCI: generic: Refactor code to enable reuse by other drivers.
  pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

 .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-host-generic.c                |  53 ++--
 drivers/pci/host/pci-host-generic.h                |  56 ++++
 drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
 6 files changed, 407 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
 create mode 100644 drivers/pci/host/pci-host-generic.h
 create mode 100644 drivers/pci/host/pcie-thunder-pem.c

-- 
1.8.3.1

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

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

No change in functionality.

Move structure definitions into a separate header file.  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
     (gen_pci_common_probe)

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
 drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 35 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-generic.h

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 5434c90..e83cec7 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -25,33 +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;
-};
-
-/*
- * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
- * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
- * that when we use a gen_pci pointer as sysdata, it is also a pointer to
- * a struct pci_sys_data.
- */
-struct gen_pci {
-#ifdef CONFIG_ARM
-	struct pci_sys_data			sys;
-#endif
-	struct pci_host_bridge			host;
-	struct gen_pci_cfg_windows		cfg;
-	struct list_head			resources;
-};
+#include "pci-host-generic.h"
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
@@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_probe(struct platform_device *pdev)
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci)
 {
 	int err;
 	const char *type;
-	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	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);
@@ -229,8 +199,6 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	of_pci_check_probe_only();
 
-	of_id = of_match_node(gen_pci_of_match, np);
-	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);
@@ -273,6 +241,21 @@ static int gen_pci_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int gen_pci_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(gen_pci_of_match, dev->of_node);
+	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	return gen_pci_common_probe(pdev, pci);
+}
+
 static struct platform_driver gen_pci_driver = {
 	.driver = {
 		.name = "pci-host-generic",
diff --git a/drivers/pci/host/pci-host-generic.h b/drivers/pci/host/pci-host-generic.h
new file mode 100644
index 0000000..089fecb
--- /dev/null
+++ b/drivers/pci/host/pci-host-generic.h
@@ -0,0 +1,56 @@
+/*
+ * 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_GENERIC_H
+#define _PCI_HOST_GENERIC_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;
+};
+
+/*
+ * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
+ * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
+ * that when we use a gen_pci pointer as sysdata, it is also a pointer to
+ * a struct pci_sys_data.
+ */
+struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
+	struct pci_host_bridge			host;
+	struct gen_pci_cfg_windows		cfg;
+	struct list_head			resources;
+};
+
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci);
+
+#endif /* _PCI_HOST_GENERIC_H */
-- 
1.8.3.1

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

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

No change in functionality.

Move structure definitions into a separate header file.  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
     (gen_pci_common_probe)

Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
 drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 35 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-generic.h

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 5434c90..e83cec7 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -25,33 +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;
-};
-
-/*
- * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
- * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
- * that when we use a gen_pci pointer as sysdata, it is also a pointer to
- * a struct pci_sys_data.
- */
-struct gen_pci {
-#ifdef CONFIG_ARM
-	struct pci_sys_data			sys;
-#endif
-	struct pci_host_bridge			host;
-	struct gen_pci_cfg_windows		cfg;
-	struct list_head			resources;
-};
+#include "pci-host-generic.h"
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
@@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_probe(struct platform_device *pdev)
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci)
 {
 	int err;
 	const char *type;
-	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	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);
@@ -229,8 +199,6 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	of_pci_check_probe_only();
 
-	of_id = of_match_node(gen_pci_of_match, np);
-	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);
@@ -273,6 +241,21 @@ static int gen_pci_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int gen_pci_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(gen_pci_of_match, dev->of_node);
+	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	return gen_pci_common_probe(pdev, pci);
+}
+
 static struct platform_driver gen_pci_driver = {
 	.driver = {
 		.name = "pci-host-generic",
diff --git a/drivers/pci/host/pci-host-generic.h b/drivers/pci/host/pci-host-generic.h
new file mode 100644
index 0000000..089fecb
--- /dev/null
+++ b/drivers/pci/host/pci-host-generic.h
@@ -0,0 +1,56 @@
+/*
+ * 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-5wv7dgnIgG8@public.gmane.org>
+ */
+
+#ifndef _PCI_HOST_GENERIC_H
+#define _PCI_HOST_GENERIC_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;
+};
+
+/*
+ * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
+ * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
+ * that when we use a gen_pci pointer as sysdata, it is also a pointer to
+ * a struct pci_sys_data.
+ */
+struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
+	struct pci_host_bridge			host;
+	struct gen_pci_cfg_windows		cfg;
+	struct list_head			resources;
+};
+
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci);
+
+#endif /* _PCI_HOST_GENERIC_H */
-- 
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

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

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

No change in functionality.

Move structure definitions into a separate header file.  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
     (gen_pci_common_probe)

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
 drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 35 deletions(-)
 create mode 100644 drivers/pci/host/pci-host-generic.h

diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 5434c90..e83cec7 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -25,33 +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;
-};
-
-/*
- * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
- * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
- * that when we use a gen_pci pointer as sysdata, it is also a pointer to
- * a struct pci_sys_data.
- */
-struct gen_pci {
-#ifdef CONFIG_ARM
-	struct pci_sys_data			sys;
-#endif
-	struct pci_host_bridge			host;
-	struct gen_pci_cfg_windows		cfg;
-	struct list_head			resources;
-};
+#include "pci-host-generic.h"
 
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
@@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 	return 0;
 }
 
-static int gen_pci_probe(struct platform_device *pdev)
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci)
 {
 	int err;
 	const char *type;
-	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
-	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);
@@ -229,8 +199,6 @@ static int gen_pci_probe(struct platform_device *pdev)
 
 	of_pci_check_probe_only();
 
-	of_id = of_match_node(gen_pci_of_match, np);
-	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);
@@ -273,6 +241,21 @@ static int gen_pci_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int gen_pci_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(gen_pci_of_match, dev->of_node);
+	pci->cfg.ops = (struct gen_pci_cfg_bus_ops *)of_id->data;
+
+	return gen_pci_common_probe(pdev, pci);
+}
+
 static struct platform_driver gen_pci_driver = {
 	.driver = {
 		.name = "pci-host-generic",
diff --git a/drivers/pci/host/pci-host-generic.h b/drivers/pci/host/pci-host-generic.h
new file mode 100644
index 0000000..089fecb
--- /dev/null
+++ b/drivers/pci/host/pci-host-generic.h
@@ -0,0 +1,56 @@
+/*
+ * 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_GENERIC_H
+#define _PCI_HOST_GENERIC_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;
+};
+
+/*
+ * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
+ * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
+ * that when we use a gen_pci pointer as sysdata, it is also a pointer to
+ * a struct pci_sys_data.
+ */
+struct gen_pci {
+#ifdef CONFIG_ARM
+	struct pci_sys_data			sys;
+#endif
+	struct pci_host_bridge			host;
+	struct gen_pci_cfg_windows		cfg;
+	struct list_head			resources;
+};
+
+int gen_pci_common_probe(struct platform_device *pdev,
+			 struct gen_pci *pci);
+
+#endif /* _PCI_HOST_GENERIC_H */
-- 
1.8.3.1

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[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.  Add a driver to provide these config
space accessor functions.  The pci-host-generic driver code is used to
configure the PCI machinery.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
 4 files changed, 333 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
 create mode 100644 drivers/pci/host/pcie-thunder-pem.c

diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
new file mode 100644
index 0000000..66824d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
@@ -0,0 +1,43 @@
+* ThunderX PEM PCIe host controller
+
+Firmware-initialized PCIe host controller found on some Cavium
+ThunderX processors.
+
+The properties and their meanings are identical to those described in
+host-heneric-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-heneric-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:
+
+    pem2 {
+	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..16ed9c3 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,10 @@ config PCI_HISI
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
 
+config PCIE_HOST_THUNDER_PEM
+	bool "Cavium Thunder PCIe controller to off-chip devices"
+	depends on PCI_HOST_GENERIC && ARM64
+	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 9d4d3c6..70bfc37 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 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_HOST_THUNDER_PEM) += pcie-thunder-pem.o
diff --git a/drivers/pci/host/pcie-thunder-pem.c b/drivers/pci/host/pcie-thunder-pem.c
new file mode 100644
index 0000000..3b2fa52
--- /dev/null
+++ b/drivers/pci/host/pcie-thunder-pem.c
@@ -0,0 +1,283 @@
+/*
+ * 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 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-generic.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 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;
+
+		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;
+			read_val &= ~(0xff << (8 * (where & 3)));
+			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;
+			read_val &= ~(0xffff << (8 * (where & 3)));
+			val = (val & 0xffff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		default:
+			break;
+
+		}
+		/*
+		 * 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 void __iomem *map_cfg_bus_thunder_pem(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 struct gen_pci_cfg_bus_ops thunder_pem_bus_ops = {
+	.bus_shift	= 24,
+	.ops		= {
+		.map_bus	= map_cfg_bus_thunder_pem,
+		.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 gen_pci_common_probe(pdev, &pem_pci->gen_pci);
+}
+
+static struct platform_driver thunder_pem_driver = {
+	.driver = {
+		.name = "pcie-thunder-pem",
+		.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

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[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.  Add a driver to provide these config
space accessor functions.  The pci-host-generic driver code is used to
configure the PCI machinery.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
 drivers/pci/host/Kconfig                           |   6 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
 4 files changed, 333 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
 create mode 100644 drivers/pci/host/pcie-thunder-pem.c

diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
new file mode 100644
index 0000000..66824d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
@@ -0,0 +1,43 @@
+* ThunderX PEM PCIe host controller
+
+Firmware-initialized PCIe host controller found on some Cavium
+ThunderX processors.
+
+The properties and their meanings are identical to those described in
+host-heneric-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-heneric-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:
+
+    pem2 {
+	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..16ed9c3 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,10 @@ config PCI_HISI
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
 
+config PCIE_HOST_THUNDER_PEM
+	bool "Cavium Thunder PCIe controller to off-chip devices"
+	depends on PCI_HOST_GENERIC && ARM64
+	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 9d4d3c6..70bfc37 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 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_HOST_THUNDER_PEM) += pcie-thunder-pem.o
diff --git a/drivers/pci/host/pcie-thunder-pem.c b/drivers/pci/host/pcie-thunder-pem.c
new file mode 100644
index 0000000..3b2fa52
--- /dev/null
+++ b/drivers/pci/host/pcie-thunder-pem.c
@@ -0,0 +1,283 @@
+/*
+ * 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 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-generic.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 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;
+
+		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;
+			read_val &= ~(0xff << (8 * (where & 3)));
+			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;
+			read_val &= ~(0xffff << (8 * (where & 3)));
+			val = (val & 0xffff) << (8 * (where & 3));
+			val |= (u32)read_val;
+			break;
+		default:
+			break;
+
+		}
+		/*
+		 * 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 void __iomem *map_cfg_bus_thunder_pem(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 struct gen_pci_cfg_bus_ops thunder_pem_bus_ops = {
+	.bus_shift	= 24,
+	.ops		= {
+		.map_bus	= map_cfg_bus_thunder_pem,
+		.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 gen_pci_common_probe(pdev, &pem_pci->gen_pci);
+}
+
+static struct platform_driver thunder_pem_driver = {
+	.driver = {
+		.name = "pcie-thunder-pem",
+		.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

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[Will Deacon]

On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> 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-generic driver code is used to
> configure the PCI machinery.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>  drivers/pci/host/Kconfig                           |   6 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>  create mode 100644 drivers/pci/host/pcie-thunder-pem.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> new file mode 100644
> index 0000000..66824d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> @@ -0,0 +1,43 @@
> +* ThunderX PEM PCIe host controller
> +
> +Firmware-initialized PCIe host controller found on some Cavium
> +ThunderX processors.
> +
> +The properties and their meanings are identical to those described in
> +host-heneric-pci.txt except as listed below.
> +
> +Properties of the host controller node that differ from
> +host-heneric-pci.txt:

Consistently odd typo (s/heneric/generic/)!

> +
> +- 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:
> +
> +    pem2 {
> +	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f131ba9..16ed9c3 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -172,4 +172,10 @@ config PCI_HISI
>  	help
>  	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>  
> +config PCIE_HOST_THUNDER_PEM
> +	bool "Cavium Thunder PCIe controller to off-chip devices"
> +	depends on PCI_HOST_GENERIC && ARM64

|| COMPILE_TEST ?

(or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)

Will

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[Will Deacon]

On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> 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-generic driver code is used to
> configure the PCI machinery.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>  drivers/pci/host/Kconfig                           |   6 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>  create mode 100644 drivers/pci/host/pcie-thunder-pem.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> new file mode 100644
> index 0000000..66824d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> @@ -0,0 +1,43 @@
> +* ThunderX PEM PCIe host controller
> +
> +Firmware-initialized PCIe host controller found on some Cavium
> +ThunderX processors.
> +
> +The properties and their meanings are identical to those described in
> +host-heneric-pci.txt except as listed below.
> +
> +Properties of the host controller node that differ from
> +host-heneric-pci.txt:

Consistently odd typo (s/heneric/generic/)!

> +
> +- 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:
> +
> +    pem2 {
> +	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f131ba9..16ed9c3 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -172,4 +172,10 @@ config PCI_HISI
>  	help
>  	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>  
> +config PCIE_HOST_THUNDER_PEM
> +	bool "Cavium Thunder PCIe controller to off-chip devices"
> +	depends on PCI_HOST_GENERIC && ARM64

|| COMPILE_TEST ?

(or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)

Will

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Will Deacon]

On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> No change in functionality.
> 
> Move structure definitions into a separate header file.  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
>      (gen_pci_common_probe)
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>  drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 35 deletions(-)
>  create mode 100644 drivers/pci/host/pci-host-generic.h
> 
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 5434c90..e83cec7 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -25,33 +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;
> -};
> -
> -/*
> - * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
> - * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
> - * that when we use a gen_pci pointer as sysdata, it is also a pointer to
> - * a struct pci_sys_data.
> - */
> -struct gen_pci {
> -#ifdef CONFIG_ARM
> -	struct pci_sys_data			sys;
> -#endif
> -	struct pci_host_bridge			host;
> -	struct gen_pci_cfg_windows		cfg;
> -	struct list_head			resources;
> -};
> +#include "pci-host-generic.h"
>  
>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>  					     unsigned int devfn,
> @@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	return 0;
>  }
>  
> -static int gen_pci_probe(struct platform_device *pdev)
> +int gen_pci_common_probe(struct platform_device *pdev,
> +			 struct gen_pci *pci)

Whilst I'm fine with this patch, I don't know how Bjorn will feel about
exposing this function outside of the generic host driver. We could avoid
it by turning things upside-down and having the generic driver probe
the other drivers by matching a compatible string with a probe function
pointer, but I'd be interested to see what others think.

Will

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Will Deacon]

On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> No change in functionality.
> 
> Move structure definitions into a separate header file.  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
>      (gen_pci_common_probe)
> 
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>  drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 35 deletions(-)
>  create mode 100644 drivers/pci/host/pci-host-generic.h
> 
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 5434c90..e83cec7 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -25,33 +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;
> -};
> -
> -/*
> - * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
> - * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
> - * that when we use a gen_pci pointer as sysdata, it is also a pointer to
> - * a struct pci_sys_data.
> - */
> -struct gen_pci {
> -#ifdef CONFIG_ARM
> -	struct pci_sys_data			sys;
> -#endif
> -	struct pci_host_bridge			host;
> -	struct gen_pci_cfg_windows		cfg;
> -	struct list_head			resources;
> -};
> +#include "pci-host-generic.h"
>  
>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>  					     unsigned int devfn,
> @@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	return 0;
>  }
>  
> -static int gen_pci_probe(struct platform_device *pdev)
> +int gen_pci_common_probe(struct platform_device *pdev,
> +			 struct gen_pci *pci)

Whilst I'm fine with this patch, I don't know how Bjorn will feel about
exposing this function outside of the generic host driver. We could avoid
it by turning things upside-down and having the generic driver probe
the other drivers by matching a compatible string with a probe function
pointer, but I'd be interested to see what others think.

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

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Will Deacon]

On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> No change in functionality.
> 
> Move structure definitions into a separate header file.  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
>      (gen_pci_common_probe)
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>  drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+), 35 deletions(-)
>  create mode 100644 drivers/pci/host/pci-host-generic.h
> 
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 5434c90..e83cec7 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -25,33 +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;
> -};
> -
> -/*
> - * ARM pcibios functions expect the ARM struct pci_sys_data as the PCI
> - * sysdata.  Add pci_sys_data as the first element in struct gen_pci so
> - * that when we use a gen_pci pointer as sysdata, it is also a pointer to
> - * a struct pci_sys_data.
> - */
> -struct gen_pci {
> -#ifdef CONFIG_ARM
> -	struct pci_sys_data			sys;
> -#endif
> -	struct pci_host_bridge			host;
> -	struct gen_pci_cfg_windows		cfg;
> -	struct list_head			resources;
> -};
> +#include "pci-host-generic.h"
>  
>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>  					     unsigned int devfn,
> @@ -208,19 +182,15 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	return 0;
>  }
>  
> -static int gen_pci_probe(struct platform_device *pdev)
> +int gen_pci_common_probe(struct platform_device *pdev,
> +			 struct gen_pci *pci)

Whilst I'm fine with this patch, I don't know how Bjorn will feel about
exposing this function outside of the generic host driver. We could avoid
it by turning things upside-down and having the generic driver probe
the other drivers by matching a compatible string with a probe function
pointer, but I'd be interested to see what others think.

Will

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

On 12/22/2015 02:07 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> No change in functionality.
>>
>> Move structure definitions into a separate header file.  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
>>       (gen_pci_common_probe)
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>>   drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 74 insertions(+), 35 deletions(-)
>>   create mode 100644 drivers/pci/host/pci-host-generic.h
>>
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index 5434c90..e83cec7 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
[...]
>> -static int gen_pci_probe(struct platform_device *pdev)
>> +int gen_pci_common_probe(struct platform_device *pdev,
>> +			 struct gen_pci *pci)
>
> Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> exposing this function outside of the generic host driver. We could avoid
> it by turning things upside-down and having the generic driver probe
> the other drivers by matching a compatible string with a probe function
> pointer, but I'd be interested to see what others think.
>

Note: I know that pci-host-generic is not built as a loadable module, but...

struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
registering of platform drivers is fairly well standardized in the 
kernel, and module loading userpace tools.

The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
same module as the driver for the device.   We are creating a separate 
driver precisely because we don't want to mix all this ThunderX specific 
code into the pci-host-generic driver when it is used by arm-32bit and 
others.  This means that, at a minimum, we would have to export the 
pci-host-generic probe function so that it could be referenced by struct 
platform_driver in other modules.

This brings up the next problem.  How to attach driver specific data to 
the generic driver structures?  At first I tried augmenting  struct 
gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
generic driver, but this would also require adding an an element to 
struct gen_pci to point to a driver specific data object.  It felt a 
little convoluted and complex.

This led me to the current design where struct gen_pci is embedded in 
the driver specific structure, and the allocation of this is done in the 
driver specific probe function.  No more callbacks, no additions to the 
pci-host-generic structures.  I think it is a little cleaner this way.

If there are suggestions as to how it can be made cleaner yet, I would 
be happy to implement and test them.

David Daney

> Will
>

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

On 12/22/2015 02:07 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> No change in functionality.
>>
>> Move structure definitions into a separate header file.  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
>>       (gen_pci_common_probe)
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>>   drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 74 insertions(+), 35 deletions(-)
>>   create mode 100644 drivers/pci/host/pci-host-generic.h
>>
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index 5434c90..e83cec7 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
[...]
>> -static int gen_pci_probe(struct platform_device *pdev)
>> +int gen_pci_common_probe(struct platform_device *pdev,
>> +			 struct gen_pci *pci)
>
> Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> exposing this function outside of the generic host driver. We could avoid
> it by turning things upside-down and having the generic driver probe
> the other drivers by matching a compatible string with a probe function
> pointer, but I'd be interested to see what others think.
>

Note: I know that pci-host-generic is not built as a loadable module, but...

struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
registering of platform drivers is fairly well standardized in the 
kernel, and module loading userpace tools.

The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
same module as the driver for the device.   We are creating a separate 
driver precisely because we don't want to mix all this ThunderX specific 
code into the pci-host-generic driver when it is used by arm-32bit and 
others.  This means that, at a minimum, we would have to export the 
pci-host-generic probe function so that it could be referenced by struct 
platform_driver in other modules.

This brings up the next problem.  How to attach driver specific data to 
the generic driver structures?  At first I tried augmenting  struct 
gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
generic driver, but this would also require adding an an element to 
struct gen_pci to point to a driver specific data object.  It felt a 
little convoluted and complex.

This led me to the current design where struct gen_pci is embedded in 
the driver specific structure, and the allocation of this is done in the 
driver specific probe function.  No more callbacks, no additions to the 
pci-host-generic structures.  I think it is a little cleaner this way.

If there are suggestions as to how it can be made cleaner yet, I would 
be happy to implement and test them.

David Daney

> Will
>

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[David Daney]

On 12/22/2015 02:07 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> No change in functionality.
>>
>> Move structure definitions into a separate header file.  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
>>       (gen_pci_common_probe)
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   drivers/pci/host/pci-host-generic.c | 53 ++++++++++++-----------------------
>>   drivers/pci/host/pci-host-generic.h | 56 +++++++++++++++++++++++++++++++++++++
>>   2 files changed, 74 insertions(+), 35 deletions(-)
>>   create mode 100644 drivers/pci/host/pci-host-generic.h
>>
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index 5434c90..e83cec7 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
[...]
>> -static int gen_pci_probe(struct platform_device *pdev)
>> +int gen_pci_common_probe(struct platform_device *pdev,
>> +			 struct gen_pci *pci)
>
> Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> exposing this function outside of the generic host driver. We could avoid
> it by turning things upside-down and having the generic driver probe
> the other drivers by matching a compatible string with a probe function
> pointer, but I'd be interested to see what others think.
>

Note: I know that pci-host-generic is not built as a loadable module, but...

struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
registering of platform drivers is fairly well standardized in the 
kernel, and module loading userpace tools.

The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
same module as the driver for the device.   We are creating a separate 
driver precisely because we don't want to mix all this ThunderX specific 
code into the pci-host-generic driver when it is used by arm-32bit and 
others.  This means that, at a minimum, we would have to export the 
pci-host-generic probe function so that it could be referenced by struct 
platform_driver in other modules.

This brings up the next problem.  How to attach driver specific data to 
the generic driver structures?  At first I tried augmenting  struct 
gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
generic driver, but this would also require adding an an element to 
struct gen_pci to point to a driver specific data object.  It felt a 
little convoluted and complex.

This led me to the current design where struct gen_pci is embedded in 
the driver specific structure, and the allocation of this is done in the 
driver specific probe function.  No more callbacks, no additions to the 
pci-host-generic structures.  I think it is a little cleaner this way.

If there are suggestions as to how it can be made cleaner yet, I would 
be happy to implement and test them.

David Daney

> Will
>

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 02:03 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> 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-generic driver code is used to
>> configure the PCI machinery.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>>   drivers/pci/host/Kconfig                           |   6 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>>   4 files changed, 333 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>>   create mode 100644 drivers/pci/host/pcie-thunder-pem.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> new file mode 100644
>> index 0000000..66824d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> @@ -0,0 +1,43 @@
>> +* ThunderX PEM PCIe host controller
>> +
>> +Firmware-initialized PCIe host controller found on some Cavium
>> +ThunderX processors.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-heneric-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-heneric-pci.txt:
>
> Consistently odd typo (s/heneric/generic/)!
>

I'm not sure how I managed to do that.  I will fix these and resend.

>> +
>> +- 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:
>> +
>> +    pem2 {
>> +	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index f131ba9..16ed9c3 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -172,4 +172,10 @@ config PCI_HISI
>>   	help
>>   	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>>
>> +config PCIE_HOST_THUNDER_PEM
>> +	bool "Cavium Thunder PCIe controller to off-chip devices"
>> +	depends on PCI_HOST_GENERIC && ARM64
>
> || COMPILE_TEST ?
>
> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)

Yes, we must use writeq in the driver, I will change it to 
||(COMPILE_TEST && 64BIT)

Thanks,
David Daney


>
> Will
>

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 02:03 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> 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-generic driver code is used to
>> configure the PCI machinery.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>>   drivers/pci/host/Kconfig                           |   6 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>>   4 files changed, 333 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>>   create mode 100644 drivers/pci/host/pcie-thunder-pem.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> new file mode 100644
>> index 0000000..66824d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> @@ -0,0 +1,43 @@
>> +* ThunderX PEM PCIe host controller
>> +
>> +Firmware-initialized PCIe host controller found on some Cavium
>> +ThunderX processors.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-heneric-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-heneric-pci.txt:
>
> Consistently odd typo (s/heneric/generic/)!
>

I'm not sure how I managed to do that.  I will fix these and resend.

>> +
>> +- 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:
>> +
>> +    pem2 {
>> +	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index f131ba9..16ed9c3 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -172,4 +172,10 @@ config PCI_HISI
>>   	help
>>   	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>>
>> +config PCIE_HOST_THUNDER_PEM
>> +	bool "Cavium Thunder PCIe controller to off-chip devices"
>> +	depends on PCI_HOST_GENERIC && ARM64
>
> || COMPILE_TEST ?
>
> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)

Yes, we must use writeq in the driver, I will change it to 
||(COMPILE_TEST && 64BIT)

Thanks,
David Daney


>
> Will
>

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 02:03 AM, Will Deacon wrote:
> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> 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-generic driver code is used to
>> configure the PCI machinery.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>>   drivers/pci/host/Kconfig                           |   6 +
>>   drivers/pci/host/Makefile                          |   1 +
>>   drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>>   4 files changed, 333 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>>   create mode 100644 drivers/pci/host/pcie-thunder-pem.c
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> new file mode 100644
>> index 0000000..66824d5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>> @@ -0,0 +1,43 @@
>> +* ThunderX PEM PCIe host controller
>> +
>> +Firmware-initialized PCIe host controller found on some Cavium
>> +ThunderX processors.
>> +
>> +The properties and their meanings are identical to those described in
>> +host-heneric-pci.txt except as listed below.
>> +
>> +Properties of the host controller node that differ from
>> +host-heneric-pci.txt:
>
> Consistently odd typo (s/heneric/generic/)!
>

I'm not sure how I managed to do that.  I will fix these and resend.

>> +
>> +- 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:
>> +
>> +    pem2 {
>> +	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/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index f131ba9..16ed9c3 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -172,4 +172,10 @@ config PCI_HISI
>>   	help
>>   	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>>
>> +config PCIE_HOST_THUNDER_PEM
>> +	bool "Cavium Thunder PCIe controller to off-chip devices"
>> +	depends on PCI_HOST_GENERIC && ARM64
>
> || COMPILE_TEST ?
>
> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)

Yes, we must use writeq in the driver, I will change it to 
||(COMPILE_TEST && 64BIT)

Thanks,
David Daney


>
> Will
>

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Arnd Bergmann]

On Tuesday 22 December 2015, David Daney wrote:
> On 12/22/2015 02:07 AM, Will Deacon wrote:
> > On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> >> From: David Daney <david.daney@cavium.com>
> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> >> index 5434c90..e83cec7 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> [...]
> >> -static int gen_pci_probe(struct platform_device *pdev)
> >> +int gen_pci_common_probe(struct platform_device *pdev,
> >> +			 struct gen_pci *pci)
> >
> > Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> > exposing this function outside of the generic host driver. We could avoid
> > it by turning things upside-down and having the generic driver probe
> > the other drivers by matching a compatible string with a probe function
> > pointer, but I'd be interested to see what others think.
> >
> 
> Note: I know that pci-host-generic is not built as a loadable module, but...
> 
> struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
> registering of platform drivers is fairly well standardized in the 
> kernel, and module loading userpace tools.

Agreed, this is the correct way to do the abstraction if we want one, the way
that Will describes is generally not a good idea, and we've converted a
number of drivers that did it like that to the way you do it here.

> The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
> same module as the driver for the device.   We are creating a separate 
> driver precisely because we don't want to mix all this ThunderX specific 
> code into the pci-host-generic driver when it is used by arm-32bit and 
> others.  This means that, at a minimum, we would have to export the 
> pci-host-generic probe function so that it could be referenced by struct 
> platform_driver in other modules.

Right.

> This brings up the next problem.  How to attach driver specific data to 
> the generic driver structures?  At first I tried augmenting  struct 
> gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
> generic driver, but this would also require adding an an element to 
> struct gen_pci to point to a driver specific data object.  It felt a 
> little convoluted and complex.
> 
> This led me to the current design where struct gen_pci is embedded in 
> the driver specific structure, and the allocation of this is done in the 
> driver specific probe function.  No more callbacks, no additions to the 
> pci-host-generic structures.  I think it is a little cleaner this way.
> 
> If there are suggestions as to how it can be made cleaner yet, I would 
> be happy to implement and test them.

My idea of the long-term direction for the pci-host-generic driver
would be to move more parts into the PCI core code as library functions
that can be used by other drivers as well. This would also address my
other concern that I'd like to see the generic host driver remain the
simplest example that we have, and only require any additional code in
other drivers to add functionality or workarounds.

Adding an abstraction layer within the driver to some degree goes in the
opposite direction of that.

One approach that might work would be to split the existing driver into
three files: one for CAM, one for ECAM and one for the common parts,
with an interface similar to what you have here. Then you can add your
driver as a third front-end, and we can keep working on integrating the
common parts further into the PCI core.

	Arnd

Re: [PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Arnd Bergmann]

On Tuesday 22 December 2015, David Daney wrote:
> On 12/22/2015 02:07 AM, Will Deacon wrote:
> > On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> >> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> >> index 5434c90..e83cec7 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> [...]
> >> -static int gen_pci_probe(struct platform_device *pdev)
> >> +int gen_pci_common_probe(struct platform_device *pdev,
> >> +			 struct gen_pci *pci)
> >
> > Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> > exposing this function outside of the generic host driver. We could avoid
> > it by turning things upside-down and having the generic driver probe
> > the other drivers by matching a compatible string with a probe function
> > pointer, but I'd be interested to see what others think.
> >
> 
> Note: I know that pci-host-generic is not built as a loadable module, but...
> 
> struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
> registering of platform drivers is fairly well standardized in the 
> kernel, and module loading userpace tools.

Agreed, this is the correct way to do the abstraction if we want one, the way
that Will describes is generally not a good idea, and we've converted a
number of drivers that did it like that to the way you do it here.

> The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
> same module as the driver for the device.   We are creating a separate 
> driver precisely because we don't want to mix all this ThunderX specific 
> code into the pci-host-generic driver when it is used by arm-32bit and 
> others.  This means that, at a minimum, we would have to export the 
> pci-host-generic probe function so that it could be referenced by struct 
> platform_driver in other modules.

Right.

> This brings up the next problem.  How to attach driver specific data to 
> the generic driver structures?  At first I tried augmenting  struct 
> gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
> generic driver, but this would also require adding an an element to 
> struct gen_pci to point to a driver specific data object.  It felt a 
> little convoluted and complex.
> 
> This led me to the current design where struct gen_pci is embedded in 
> the driver specific structure, and the allocation of this is done in the 
> driver specific probe function.  No more callbacks, no additions to the 
> pci-host-generic structures.  I think it is a little cleaner this way.
> 
> If there are suggestions as to how it can be made cleaner yet, I would 
> be happy to implement and test them.

My idea of the long-term direction for the pci-host-generic driver
would be to move more parts into the PCI core code as library functions
that can be used by other drivers as well. This would also address my
other concern that I'd like to see the generic host driver remain the
simplest example that we have, and only require any additional code in
other drivers to add functionality or workarounds.

Adding an abstraction layer within the driver to some degree goes in the
opposite direction of that.

One approach that might work would be to split the existing driver into
three files: one for CAM, one for ECAM and one for the common parts,
with an interface similar to what you have here. Then you can add your
driver as a third front-end, and we can keep working on integrating the
common parts further into the PCI core.

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

[PATCH 1/2] PCI: generic: Refactor code to enable reuse by other drivers.

[Arnd Bergmann]

On Tuesday 22 December 2015, David Daney wrote:
> On 12/22/2015 02:07 AM, Will Deacon wrote:
> > On Mon, Dec 21, 2015 at 05:53:41PM -0800, David Daney wrote:
> >> From: David Daney <david.daney@cavium.com>
> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> >> index 5434c90..e83cec7 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> [...]
> >> -static int gen_pci_probe(struct platform_device *pdev)
> >> +int gen_pci_common_probe(struct platform_device *pdev,
> >> +			 struct gen_pci *pci)
> >
> > Whilst I'm fine with this patch, I don't know how Bjorn will feel about
> > exposing this function outside of the generic host driver. We could avoid
> > it by turning things upside-down and having the generic driver probe
> > the other drivers by matching a compatible string with a probe function
> > pointer, but I'd be interested to see what others think.
> >
> 
> Note: I know that pci-host-generic is not built as a loadable module, but...
> 
> struct of_device_id, MODULE_DEVICE_TABLE, struct platform_driver and the 
> registering of platform drivers is fairly well standardized in the 
> kernel, and module loading userpace tools.

Agreed, this is the correct way to do the abstraction if we want one, the way
that Will describes is generally not a good idea, and we've converted a
number of drivers that did it like that to the way you do it here.

> The struct of_device_id, MODULE_DEVICE_TABLE must really reside in the 
> same module as the driver for the device.   We are creating a separate 
> driver precisely because we don't want to mix all this ThunderX specific 
> code into the pci-host-generic driver when it is used by arm-32bit and 
> others.  This means that, at a minimum, we would have to export the 
> pci-host-generic probe function so that it could be referenced by struct 
> platform_driver in other modules.

Right.

> This brings up the next problem.  How to attach driver specific data to 
> the generic driver structures?  At first I tried augmenting  struct 
> gen_pci_cfg_bus_ops with a callback .init() function to be called by the 
> generic driver, but this would also require adding an an element to 
> struct gen_pci to point to a driver specific data object.  It felt a 
> little convoluted and complex.
> 
> This led me to the current design where struct gen_pci is embedded in 
> the driver specific structure, and the allocation of this is done in the 
> driver specific probe function.  No more callbacks, no additions to the 
> pci-host-generic structures.  I think it is a little cleaner this way.
> 
> If there are suggestions as to how it can be made cleaner yet, I would 
> be happy to implement and test them.

My idea of the long-term direction for the pci-host-generic driver
would be to move more parts into the PCI core code as library functions
that can be used by other drivers as well. This would also address my
other concern that I'd like to see the generic host driver remain the
simplest example that we have, and only require any additional code in
other drivers to add functionality or workarounds.

Adding an abstraction layer within the driver to some degree goes in the
opposite direction of that.

One approach that might work would be to split the existing driver into
three files: one for CAM, one for ECAM and one for the common parts,
with an interface similar to what you have here. Then you can add your
driver as a third front-end, and we can keep working on integrating the
common parts further into the PCI core.

	Arnd

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 11:18 AM, David Daney wrote:
> On 12/22/2015 02:03 AM, Will Deacon wrote:
>> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
[...]
>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>> index f131ba9..16ed9c3 100644
>>> --- a/drivers/pci/host/Kconfig
>>> +++ b/drivers/pci/host/Kconfig
>>> @@ -172,4 +172,10 @@ config PCI_HISI
>>>       help
>>>         Say Y here if you want PCIe controller support on HiSilicon
>>> HIP05 SoC
>>>
>>> +config PCIE_HOST_THUNDER_PEM
>>> +    bool "Cavium Thunder PCIe controller to off-chip devices"
>>> +    depends on PCI_HOST_GENERIC && ARM64
>>
>> || COMPILE_TEST ?
>>
>> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)
>
> Yes, we must use writeq in the driver, I will change it to
> ||(COMPILE_TEST && 64BIT)
>

Actually, it turns out that this is not easily done.

For x86, it appears difficult to include asm-generic/pci-bridge.h which 
includes the needed definition of PCI_PROBE_ONLY.  So I think it is not 
worth supporting COMPILE_TEST

David Daney

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 11:18 AM, David Daney wrote:
> On 12/22/2015 02:03 AM, Will Deacon wrote:
>> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
[...]
>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>> index f131ba9..16ed9c3 100644
>>> --- a/drivers/pci/host/Kconfig
>>> +++ b/drivers/pci/host/Kconfig
>>> @@ -172,4 +172,10 @@ config PCI_HISI
>>>       help
>>>         Say Y here if you want PCIe controller support on HiSilicon
>>> HIP05 SoC
>>>
>>> +config PCIE_HOST_THUNDER_PEM
>>> +    bool "Cavium Thunder PCIe controller to off-chip devices"
>>> +    depends on PCI_HOST_GENERIC && ARM64
>>
>> || COMPILE_TEST ?
>>
>> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)
>
> Yes, we must use writeq in the driver, I will change it to
> ||(COMPILE_TEST && 64BIT)
>

Actually, it turns out that this is not easily done.

For x86, it appears difficult to include asm-generic/pci-bridge.h which 
includes the needed definition of PCI_PROBE_ONLY.  So I think it is not 
worth supporting COMPILE_TEST

David Daney

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[David Daney]

On 12/22/2015 11:18 AM, David Daney wrote:
> On 12/22/2015 02:03 AM, Will Deacon wrote:
>> On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
>>> From: David Daney <david.daney@cavium.com>
[...]
>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>> index f131ba9..16ed9c3 100644
>>> --- a/drivers/pci/host/Kconfig
>>> +++ b/drivers/pci/host/Kconfig
>>> @@ -172,4 +172,10 @@ config PCI_HISI
>>>       help
>>>         Say Y here if you want PCIe controller support on HiSilicon
>>> HIP05 SoC
>>>
>>> +config PCIE_HOST_THUNDER_PEM
>>> +    bool "Cavium Thunder PCIe controller to off-chip devices"
>>> +    depends on PCI_HOST_GENERIC && ARM64
>>
>> || COMPILE_TEST ?
>>
>> (or does the use of writeq get you? If so, maybe COMPILE_TEST && 64BIT)
>
> Yes, we must use writeq in the driver, I will change it to
> ||(COMPILE_TEST && 64BIT)
>

Actually, it turns out that this is not easily done.

For x86, it appears difficult to include asm-generic/pci-bridge.h which 
includes the needed definition of PCI_PROBE_ONLY.  So I think it is not 
worth supporting COMPILE_TEST

David Daney

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[Rob Herring]

On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> 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-generic driver code is used to
> configure the PCI machinery.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>  drivers/pci/host/Kconfig                           |   6 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>  create mode 100644 drivers/pci/host/pcie-thunder-pem.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> new file mode 100644
> index 0000000..66824d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> @@ -0,0 +1,43 @@
> +* ThunderX PEM PCIe host controller
> +
> +Firmware-initialized PCIe host controller found on some Cavium
> +ThunderX processors.
> +
> +The properties and their meanings are identical to those described in
> +host-heneric-pci.txt except as listed below.

s/heneric/generic/

> +
> +Properties of the host controller node that differ from
> +host-heneric-pci.txt:

ditto

> +
> +- compatible     : Must be "cavium,pci-host-thunder-pem"

pcie rather than pci?


> +
> +- 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:
> +
> +    pem2 {

pcie-controller@...

> +	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 */
> +    };

Re: [PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[Rob Herring]

On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
> 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.  Add a driver to provide these config
> space accessor functions.  The pci-host-generic driver code is used to
> configure the PCI machinery.
> 
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>  drivers/pci/host/Kconfig                           |   6 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>  create mode 100644 drivers/pci/host/pcie-thunder-pem.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> new file mode 100644
> index 0000000..66824d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> @@ -0,0 +1,43 @@
> +* ThunderX PEM PCIe host controller
> +
> +Firmware-initialized PCIe host controller found on some Cavium
> +ThunderX processors.
> +
> +The properties and their meanings are identical to those described in
> +host-heneric-pci.txt except as listed below.

s/heneric/generic/

> +
> +Properties of the host controller node that differ from
> +host-heneric-pci.txt:

ditto

> +
> +- compatible     : Must be "cavium,pci-host-thunder-pem"

pcie rather than pci?


> +
> +- 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:
> +
> +    pem2 {

pcie-controller@...

> +	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 */
> +    };

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

[PATCH 2/2] pci, pcie-thunder-pem: Add PCIe host driver for ThunderX processors.

[Rob Herring]

On Mon, Dec 21, 2015 at 05:53:42PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> 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-generic driver code is used to
> configure the PCI machinery.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/pci/pcie-thunder-pem.txt   |  43 ++++
>  drivers/pci/host/Kconfig                           |   6 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-thunder-pem.c                | 283 +++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
>  create mode 100644 drivers/pci/host/pcie-thunder-pem.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> new file mode 100644
> index 0000000..66824d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-thunder-pem.txt
> @@ -0,0 +1,43 @@
> +* ThunderX PEM PCIe host controller
> +
> +Firmware-initialized PCIe host controller found on some Cavium
> +ThunderX processors.
> +
> +The properties and their meanings are identical to those described in
> +host-heneric-pci.txt except as listed below.

s/heneric/generic/

> +
> +Properties of the host controller node that differ from
> +host-heneric-pci.txt:

ditto

> +
> +- compatible     : Must be "cavium,pci-host-thunder-pem"

pcie rather than pci?


> +
> +- 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:
> +
> +    pem2 {

pcie-controller at ...

> +	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 */
> +    };