[PATCH 1/5] [v2][POWERPC] refactor dcr code

[PATCH 1/5] [v2][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Previously, dcr support was configured at compile time to either using
MMIO or native dcr instructions.  Although this works for most
platforms, it fails on FPGA platforms:

1) Systems may include more than one dcr bus.
2) Systems may be native dcr capable and still use memory mapped dcr interface.

This patch provides runtime support based on the device trees for the
case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
selected.  Previously, this was a poorly defined configuration, which
happened to provide NATIVE support.  The runtime selection is made
based on the dcr slave device having a 'dcr-access-method' attribute
in the device tree.  If only one of the above options is selected,
then the code uses #defines to select only the used code in order to
avoid interoducing overhead in existing usage.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/dcr.c         |   91 ++++++++++++++++++++++++++++++++-----
 include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
 include/asm-powerpc/dcr-mmio.h    |   20 +++++---
 include/asm-powerpc/dcr-native.h  |   16 ++++---
 include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
 5 files changed, 186 insertions(+), 26 deletions(-)
 create mode 100644 include/asm-powerpc/dcr-generic.h

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index 437e48d..d3de0ff 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -23,6 +23,68 @@
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+bool dcr_map_ok_generic(dcr_host_t host)
+{
+	if (host.type == INVALID)
+		return 0;
+	else if (host.type == NATIVE)
+		return dcr_map_ok_native(host.host.native);
+	else
+		return dcr_map_ok_mmio(host.host.mmio);
+}
+EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
+
+dcr_host_t dcr_map_generic(struct device_node *dev,
+			   unsigned int dcr_n,
+			   unsigned int dcr_c)
+{
+	dcr_host_t host;
+	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
+
+	if (!strcmp(prop, "native")) {
+		host.type = NATIVE;
+		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
+	} else if (!strcmp(prop, "mmio")) {
+		host.type = MMIO;
+		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
+	} else
+		host.type = INVALID;
+
+	return host;
+}
+EXPORT_SYMBOL_GPL(dcr_map_generic);
+
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
+}
+EXPORT_SYMBOL_GPL(dcr_unmap_generic);
+
+u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
+{
+	if (host.type == NATIVE)
+		return dcr_read_native(host.host.native, dcr_n);
+	else
+		return dcr_read_mmio(host.host.mmio, dcr_n);
+}
+EXPORT_SYMBOL_GPL(dcr_read_generic);
+
+void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
+{
+	if (host.type == NATIVE)
+		dcr_write_native(host.host.native, dcr_n, value);
+	else
+		dcr_write_mmio(host.host.mmio, dcr_n, value);
+}
+EXPORT_SYMBOL_GPL(dcr_write_generic);
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
 	unsigned int ds;
@@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
-#ifndef CONFIG_PPC_DCR_NATIVE
+#ifdef CONFIG_PPC_DCR_MMIO
 
 static struct device_node * find_dcr_parent(struct device_node * node)
 {
@@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	return ret;
 }
 
-dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-		   unsigned int dcr_c)
+dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+			     unsigned int dcr_n,
+			     unsigned int dcr_c)
 {
-	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
+	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
 	u64 addr;
 
 	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
 		 dev->full_name, dcr_n, dcr_c);
 
 	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
-	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
-		 addr, ret.stride);
+	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
+		 (unsigned long long) addr, ret.stride);
 	if (addr == OF_BAD_ADDR)
 		return ret;
 	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
@@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 	ret.token -= dcr_n * ret.stride;
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dcr_map);
+EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
+void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
-	dcr_host_t h = host;
+	dcr_host_mmio_t h = host;
 
 	if (h.token == NULL)
 		return;
@@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 	iounmap(h.token);
 	h.token = NULL;
 }
-EXPORT_SYMBOL_GPL(dcr_unmap);
-#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
+EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
+
+#endif /* defined(CONFIG_PPC_DCR_MMIO) */
+
+#ifdef CONFIG_PPC_DCR_NATIVE
 DEFINE_SPINLOCK(dcr_ind_lock);
-#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
+#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
+
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
new file mode 100644
index 0000000..0ee74fb
--- /dev/null
+++ b/include/asm-powerpc/dcr-generic.h
@@ -0,0 +1,49 @@
+/*
+ * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
+ *                    <benh@kernel.crashing.org>
+ *
+ *   This program is free software;  you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   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, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_DCR_GENERIC_H
+#define _ASM_POWERPC_DCR_GENERIC_H
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+enum host_type_t {MMIO, NATIVE, INVALID};
+
+typedef struct {
+	enum host_type_t type;
+	union {
+		dcr_host_mmio_t mmio;
+		dcr_host_native_t native;
+	} host;
+} dcr_host_t;
+
+extern bool dcr_map_ok_generic(dcr_host_t host);
+
+extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
+			  unsigned int dcr_c);
+extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
+
+extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
+
+extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_DCR_GENERIC_H */
+
+
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 08532ff..acd491d 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -27,20 +27,26 @@ typedef struct {
 	void __iomem *token;
 	unsigned int stride;
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_mmio_t;
 
-#define DCR_MAP_OK(host)	((host).token != NULL)
+static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
+{
+	return host.token != NULL;
+}
 
-extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-			  unsigned int dcr_c);
-extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
+extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+				    unsigned int dcr_n,
+				    unsigned int dcr_c);
+extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
-static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
+static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
 {
 	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
 }
 
-static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
+static inline void dcr_write_mmio(dcr_host_mmio_t host,
+				  unsigned int dcr_n,
+				  u32 value)
 {
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index be6c879..67832e5 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -26,14 +26,18 @@
 
 typedef struct {
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_native_t;
 
-#define DCR_MAP_OK(host)	(1)
+static inline bool dcr_map_ok_native(dcr_host_native_t host)
+{
+	return 1;
+}
 
-#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_c)		do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
+#define dcr_map_native(dev, dcr_n, dcr_c) \
+	((dcr_host_native_t){ .base = (dcr_n) })
+#define dcr_unmap_native(host, dcr_c)		do {} while (0)
+#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 9338d50..6b86322 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -20,14 +20,47 @@
 #ifndef _ASM_POWERPC_DCR_H
 #define _ASM_POWERPC_DCR_H
 #ifdef __KERNEL__
+#ifndef __ASSEMBLY__
 #ifdef CONFIG_PPC_DCR
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 #include <asm/dcr-native.h>
-#else
+#endif
+
+#ifdef CONFIG_PPC_DCR_MMIO
 #include <asm/dcr-mmio.h>
 #endif
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+#include <asm/dcr-generic.h>
+
+#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
+
+#else
+
+#ifdef CONFIG_PPC_DCR_NATIVE
+typedef dcr_host_native_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
+#else
+typedef dcr_host_mmio_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
+#endif
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 /*
  * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
  * base from the device-tree
@@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
 #endif /* CONFIG_PPC_MERGE */
 
 #endif /* CONFIG_PPC_DCR */
+#endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_H */
-- 
1.5.3.4-dirty

[PATCH 2/5] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Stephen Neuendorffer]

FPGA designs may have need of both MMIO-based and NATIVE-based dcr
interfaces.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index a9260e2..b8e06df 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -123,6 +123,8 @@ config 405GPR
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_MMIO
+	select PPC_DCR_NATIVE
 
 config XILINX_VIRTEX_II_PRO
 	bool
-- 
1.5.3.4-dirty

[PATCH 3/5] [POWERPC] explicit dcr support

[Stephen Neuendorffer]

Added literal mapping support if no device-tree support.  Added
CONFIG_OF to guard device-tree parts, since literal support works for
arch=ppc.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/dcr.c         |   82 ++++++++++++++++++++++++++++---------
 include/asm-powerpc/dcr-generic.h |    2 +
 include/asm-powerpc/dcr-mmio.h    |   11 +++++
 include/asm-powerpc/dcr-native.h  |    8 ++++
 include/asm-powerpc/dcr.h         |   24 +++++++++++
 5 files changed, 108 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index d3de0ff..2ccae80 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -20,9 +20,34 @@
 #undef DEBUG
 
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+#ifdef CONFIG_OF
+static struct device_node *find_dcr_parent(struct device_node *node)
+{
+	struct device_node *par, *tmp;
+	const u32 *p;
+
+	for (par = of_node_get(node); par;) {
+		if (of_get_property(par, "dcr-controller", NULL))
+			break;
+		p = of_get_property(par, "dcr-parent", NULL);
+		tmp = par;
+		if (p == NULL)
+			par = of_get_parent(par);
+		else
+			par = of_find_node_by_phandle(*p);
+		of_node_put(tmp);
+	}
+	return par;
+}
+#endif /* CONFIG_OF */
+
+
+/* Indirection layer for providing both NATIVE and MMIO support. */
+
 #if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
 
 bool dcr_map_ok_generic(dcr_host_t host)
@@ -36,12 +61,24 @@ bool dcr_map_ok_generic(dcr_host_t host)
 }
 EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
 
+#ifdef CONFIG_OF
 dcr_host_t dcr_map_generic(struct device_node *dev,
 			   unsigned int dcr_n,
 			   unsigned int dcr_c)
 {
 	dcr_host_t host;
-	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
+	struct device_node *dp;
+
+	dp = find_dcr_parent(dev);
+	if (dp == NULL) {
+		host.type = INVALID;
+		return host;
+	}
+
+	const char *prop = of_get_property(dp, "dcr-access-method", NULL);
+
+	pr_debug("dcr_map_generic(dcr-access-method = %s)\n",
+		 prop);
 
 	if (!strcmp(prop, "native")) {
 		host.type = NATIVE;
@@ -56,6 +93,8 @@ dcr_host_t dcr_map_generic(struct device_node *dev,
 }
 EXPORT_SYMBOL_GPL(dcr_map_generic);
 
+#endif /* CONFIG_OF */
+
 void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
 {
 	if (host.type == NATIVE)
@@ -85,9 +124,10 @@ EXPORT_SYMBOL_GPL(dcr_write_generic);
 
 #endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
 
+#ifdef CONFIG_OF
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
-	unsigned int ds;
+	int ds;
 	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
 
 	if (dr == NULL || ds & 1 || index >= (ds / 8))
@@ -99,7 +139,7 @@ EXPORT_SYMBOL_GPL(dcr_resource_start);
 
 unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 {
-	unsigned int ds;
+	int ds;
 	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
 
 	if (dr == NULL || ds & 1 || index >= (ds / 8))
@@ -109,26 +149,28 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
+#endif /* CONFIG_OF */
+
+
+
+/* Support for MMIO */
 #ifdef CONFIG_PPC_DCR_MMIO
 
-static struct device_node * find_dcr_parent(struct device_node * node)
+dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
+					  unsigned int stride,
+					  unsigned int dcr_n,
+					  unsigned int dcr_c)
 {
-	struct device_node *par, *tmp;
-	const u32 *p;
-
-	for (par = of_node_get(node); par;) {
-		if (of_get_property(par, "dcr-controller", NULL))
-			break;
-		p = of_get_property(par, "dcr-parent", NULL);
-		tmp = par;
-		if (p == NULL)
-			par = of_get_parent(par);
-		else
-			par = of_find_node_by_phandle(*p);
-		of_node_put(tmp);
-	}
-	return par;
+	dcr_host_mmio_t host;
+	host.stride = stride;
+	host.token = ioremap(mmio_start, dcr_c * stride);
+	host.token -= dcr_n * stride;
+	host.base = dcr_n;
+	return host;
 }
+EXPORT_SYMBOL_GPL(dcr_map_mmio_literal_mmio);
+
+#ifdef CONFIG_OF
 
 u64 of_translate_dcr_address(struct device_node *dev,
 			     unsigned int dcr_n,
@@ -189,6 +231,8 @@ dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
 }
 EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
+#endif /* CONFIG_OF */
+
 void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
 	dcr_host_mmio_t h = host;
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
index 0ee74fb..8032795 100644
--- a/include/asm-powerpc/dcr-generic.h
+++ b/include/asm-powerpc/dcr-generic.h
@@ -34,8 +34,10 @@ typedef struct {
 
 extern bool dcr_map_ok_generic(dcr_host_t host);
 
+#ifdef CONFIG_OF
 extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
 			  unsigned int dcr_c);
+#endif
 extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
 
 extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index acd491d..b12d291 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -34,9 +34,18 @@ static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
 	return host.token != NULL;
 }
 
+extern dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
+						 unsigned int stride,
+						 unsigned int dcr_n,
+						 unsigned int dcr_c);
+
+
+#ifdef CONFIG_OF
 extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
 				    unsigned int dcr_n,
 				    unsigned int dcr_c);
+#endif
+
 extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
 static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
@@ -51,9 +60,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
 
+#ifdef CONFIG_OF
 extern u64 of_translate_dcr_address(struct device_node *dev,
 				    unsigned int dcr_n,
 				    unsigned int *stride);
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_MMIO_H */
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index 67832e5..9b3e255 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -33,6 +33,14 @@ static inline bool dcr_map_ok_native(dcr_host_native_t host)
 	return 1;
 }
 
+static inline
+dcr_host_native_t dcr_map_native_literal_native(unsigned int dcr_n)
+{
+	dcr_host_native_t host;
+	host.base = dcr_n;
+	return host;
+}
+
 #define dcr_map_native(dev, dcr_n, dcr_c) \
 	((dcr_host_native_t){ .base = (dcr_n) })
 #define dcr_unmap_native(host, dcr_c)		do {} while (0)
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 6b86322..dfd1c24 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -35,6 +35,27 @@
 
 #include <asm/dcr-generic.h>
 
+static inline
+dcr_host_t dcr_map_mmio_literal(resource_size_t mmio_start,
+				unsigned int stride,
+				unsigned int dcr_n,
+				unsigned int dcr_c)
+{
+	dcr_host_t host;
+	host.type = MMIO;
+	host.host.mmio =
+		dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c);
+	return host;
+}
+static inline
+dcr_host_t dcr_map_native_literal(unsigned int dcr_n)
+{
+	dcr_host_t host;
+	host.type = NATIVE;
+	host.host.native = dcr_map_native_literal_native(dcr_n);
+	return host;
+}
+
 #define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
@@ -45,6 +66,7 @@
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 typedef dcr_host_native_t dcr_host_t;
+#define dcr_map_native_literal(dcr_n) dcr_map_native_literal_native(dcr_n)
 #define DCR_MAP_OK(host)	dcr_map_ok_native(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
@@ -52,6 +74,8 @@ typedef dcr_host_native_t dcr_host_t;
 #define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
 #else
 typedef dcr_host_mmio_t dcr_host_t;
+#define dcr_map_mmio_literal(mmio_start, stride, dcr_n, dcr_c) \
+	dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c)
 #define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
-- 
1.5.3.4-dirty

[PATCH 4/5] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[Stephen Neuendorffer]

This device contains a dcr interface.  Previously, the dcr interface
was assumed to be used in mmio mode, and the register space of the dcr
interface was precomputed and stuffed in the device tree.  This patch
makes use of the new dcr infrastructure to represent the dcr interface
as any other dcr interface in the device tree.  This enables the dcr
interface to be connected directly to a native dcr interface in a
clean way.

In particular, the device tree expected looks like:

			dcr_v29_0: dcr@0 {
				#address-cells = <1>;
				#size-cells = <1>;
				compatible = "xlnx,dcr-v29-1.00.a";
				VGA_FrameBuffer: tft@80 {
					compatible = "xlnx,plb-tft-cntlr-ref-1.00.a";
					dcr-parent = <&opb2dcr_bridge_0>;
					dcr-reg = < 80 2 >;
					xlnx,default-tft-base-addr = <7f>;
					xlnx,dps-init = <1>;
					xlnx,on-init = <1>;
					xlnx,pixclk-is-busclk-divby4 = <1>;
				} ;
			} ;

			opb2dcr_bridge_0: opb2dcr-bridge@40700000 {
				compatible = "xlnx,opb2dcr-bridge-1.00.b";
				dcr-access-method = "mmio";
				dcr-controller ;
				dcr-mmio-range = < 40700000 1000 >;
				dcr-mmio-stride = <4>;
				reg = < 40700000 1000 >;
				xlnx,family = "virtex2p";
			} ;

Note that this patch now requires PPC_DCR_MMIO to be set in order to
advertise the framebuffer as a platform device (i.e. ARCH=ppc).

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 drivers/video/xilinxfb.c |   86 ++++++++++++++++++++++++++--------------------
 1 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 7b3a842..171cc09 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -38,6 +38,7 @@
 #endif
 #include <asm/io.h>
 #include <linux/xilinxfb.h>
+#include <asm/dcr.h>
 
 #define DRIVER_NAME		"xilinxfb"
 #define DRIVER_DESCRIPTION	"Xilinx TFT LCD frame buffer driver"
@@ -112,8 +113,9 @@ struct xilinxfb_drvdata {
 
 	struct fb_info	info;		/* FB driver info record */
 
-	u32		regs_phys;	/* phys. address of the control registers */
-	u32 __iomem	*regs;		/* virt. address of the control registers */
+	dcr_host_t      dcr_host;
+	unsigned int    dcr_start;
+	unsigned int    dcr_len;
 
 	void		*fb_virt;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
@@ -136,7 +138,7 @@ struct xilinxfb_drvdata {
  * when it's needed.
  */
 #define xilinx_fb_out_be32(driverdata, offset, val) \
-	out_be32(driverdata->regs + offset, val)
+	dcr_write(driverdata->dcr_host, offset, val)
 
 static int
 xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
@@ -204,7 +206,8 @@ static struct fb_ops xilinxfb_ops =
  * Bus independent setup/teardown
  */
 
-static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
+static int xilinxfb_assign(struct device *dev, dcr_host_t dcr_host,
+			   unsigned int dcr_start, unsigned int dcr_len,
 			   struct xilinxfb_platform_data *pdata)
 {
 	struct xilinxfb_drvdata *drvdata;
@@ -219,21 +222,9 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 	dev_set_drvdata(dev, drvdata);
 
-	/* Map the control registers in */
-	if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_region;
-	}
-	drvdata->regs_phys = physaddr;
-	drvdata->regs = ioremap(physaddr, 8);
-	if (!drvdata->regs) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_map;
-	}
+	drvdata->dcr_start = dcr_start;
+	drvdata->dcr_len = dcr_len;
+	drvdata->dcr_host = dcr_host;
 
 	/* Allocate the framebuffer memory */
 	if (pdata->fb_phys) {
@@ -248,7 +239,7 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
 		rc = -ENOMEM;
-		goto err_fbmem;
+		goto err_region;
 	}
 
 	/* Clear (turn to black) the framebuffer */
@@ -298,7 +289,6 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 
 	/* Put a banner in the log (for DEBUG) */
-	dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr, drvdata->regs);
 	dev_dbg(dev, "fb: phys=%p, virt=%p, size=%x\n",
 		(void*)drvdata->fb_phys, drvdata->fb_virt, fbsize);
 
@@ -314,12 +304,6 @@ err_cmap:
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
 
-err_fbmem:
-	iounmap(drvdata->regs);
-
-err_map:
-	release_mem_region(physaddr, 8);
-
 err_region:
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -345,9 +329,8 @@ static int xilinxfb_release(struct device *dev)
 
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
-	iounmap(drvdata->regs);
 
-	release_mem_region(drvdata->regs_phys, 8);
+	dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -355,6 +338,7 @@ static int xilinxfb_release(struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PPC_DCR_MMIO
 /* ---------------------------------------------------------------------
  * Platform bus binding
  */
@@ -364,6 +348,9 @@ xilinxfb_platform_probe(struct platform_device *pdev)
 {
 	struct xilinxfb_platform_data *pdata;
 	struct resource *res;
+	dcr_host_t dcr_host;
+	int dcr_start = 0;
+	int dcr_len = 2;
 
 	/* Find the registers address */
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
@@ -386,7 +373,12 @@ xilinxfb_platform_probe(struct platform_device *pdev)
 			pdata->yvirt = xilinx_fb_default_pdata.yvirt;
 	}
 
-	return xilinxfb_assign(&pdev->dev, res->start, pdata);
+	dcr_host = dcr_map_mmio_literal(res->start, 4, dcr_start, dcr_len);
+	if (!DCR_MAP_OK(dcr_host)) {
+		dev_err(&pdev->dev, "invalid address\n");
+		return -ENODEV;
+	}
+	return xilinxfb_assign(&pdev->dev, dcr_host, dcr_start, dcr_len, pdata);
 }
 
 static int
@@ -405,6 +397,23 @@ static struct platform_driver xilinxfb_platform_driver = {
 	},
 };
 
+/* Registration helpers to keep the number of #ifdefs to a minimum */
+static inline int __init xilinxfb_platform_register(void)
+{
+	pr_debug("xilinxfb: calling of_register_platform_driver()\n");
+	return platform_driver_register(&xilinxfb_platform_driver);
+}
+
+static inline void __exit xilinxfb_platform_unregister(void)
+{
+	platform_driver_unregister(&xilinxfb_platform_driver);
+}
+#else /* CONFIG_PPC_DCR_MMIO */
+/* CONFIG_OF not enabled; do nothing helpers */
+static inline int __init xilinxfb_platform_register(void) { return 0; }
+static inline void __exit xilinxfb_platform_unregister(void) { }
+#endif /* CONFIG_PPC_DCR_MMIO */
+
 /* ---------------------------------------------------------------------
  * OF bus binding
  */
@@ -413,20 +422,23 @@ static struct platform_driver xilinxfb_platform_driver = {
 static int __devinit
 xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 {
-	struct resource res;
 	const u32 *prop;
 	struct xilinxfb_platform_data pdata;
 	int size, rc;
+	int start, len;
+	dcr_host_t dcr_host;
 
 	/* Copy with the default pdata (not a ptr reference!) */
 	pdata = xilinx_fb_default_pdata;
 
 	dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
 
-	rc = of_address_to_resource(op->node, 0, &res);
-	if (rc) {
+	start = dcr_resource_start(op->node, 0);
+	len = dcr_resource_len(op->node, 0);
+	dcr_host = dcr_map(op->node, start, len);
+	if (!DCR_MAP_OK(dcr_host)) {
 		dev_err(&op->dev, "invalid address\n");
-		return rc;
+		return -ENODEV;
 	}
 
 	prop = of_get_property(op->node, "phys-size", &size);
@@ -450,7 +462,7 @@ xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 	if (of_find_property(op->node, "rotate-display", NULL))
 		pdata.rotate_screen = 1;
 
-	return xilinxfb_assign(&op->dev, res.start, &pdata);
+	return xilinxfb_assign(&op->dev, dcr_host, start, len, &pdata);
 }
 
 static int __devexit xilinxfb_of_remove(struct of_device *op)
@@ -505,7 +517,7 @@ xilinxfb_init(void)
 	if (rc)
 		return rc;
 
-	rc = platform_driver_register(&xilinxfb_platform_driver);
+	rc = xilinxfb_platform_register();
 	if (rc)
 		xilinxfb_of_unregister();
 
@@ -515,7 +527,7 @@ xilinxfb_init(void)
 static void __exit
 xilinxfb_cleanup(void)
 {
-	platform_driver_unregister(&xilinxfb_platform_driver);
+	xilinxfb_platform_unregister();
 	xilinxfb_of_unregister();
 }
 
-- 
1.5.3.4-dirty

[PATCH 5/5] [RFC][PPC] Use DCR for arch ppc, and enable MMIO and NATIVE for virtex.

[Stephen Neuendorffer]

Generally speaking, I'm not sure about the validity of this patch.  In
particular:

1) I'm not sure what architecture code relies on including
dcr support through ibm4xx.h.

2) I'm not sure how temporary the ifdef in
arch/powerpc/sysdev/Makefile really is.  Is it still needed or not?

However, this appears to work at least for the Virtex systems I've
tried.  In any event, I don't expect this to actually be sent to
mainline, since it's ARCH=ppc related, at least not in this form.

Unfortunately, the framebuffer driver won't work under ARCH=ppc
without this patch.
---
 arch/powerpc/sysdev/Makefile   |    2 --
 arch/ppc/Kconfig               |    6 +++++-
 arch/ppc/platforms/4xx/Kconfig |    2 ++
 include/asm-ppc/ibm4xx.h       |    1 -
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 15f3e85..c85525b 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -35,10 +35,8 @@ endif
 endif
 
 # Temporary hack until we have migrated to asm-powerpc
-ifeq ($(ARCH),powerpc)
 obj-$(CONFIG_CPM)		+= cpm_common.o
 obj-$(CONFIG_CPM2)		+= cpm2.o cpm2_pic.o
 obj-$(CONFIG_PPC_DCR)		+= dcr.o
 obj-$(CONFIG_8xx)		+= mpc8xx_pic.o cpm1.o
 obj-$(CONFIG_UCODE_PATCH)	+= micropatch.o
-endif
diff --git a/arch/ppc/Kconfig b/arch/ppc/Kconfig
index abc877f..81d6748 100644
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -116,9 +116,13 @@ config PPC_DCR_NATIVE
 	bool
 	default n
 
+config PPC_DCR_MMIO
+	bool
+	default n
+
 config PPC_DCR
 	bool
-	depends on PPC_DCR_NATIVE
+	depends on PPC_DCR_NATIVE || PPC_DCR_MMIO
 	default y
 
 config PTE_64BIT
diff --git a/arch/ppc/platforms/4xx/Kconfig b/arch/ppc/platforms/4xx/Kconfig
index 76551b6..0430fc2 100644
--- a/arch/ppc/platforms/4xx/Kconfig
+++ b/arch/ppc/platforms/4xx/Kconfig
@@ -228,6 +228,8 @@ config XILINX_VIRTEX_4_FX
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_NATIVE
+	select PPC_DCR_MMIO
 
 config STB03xxx
 	bool
diff --git a/include/asm-ppc/ibm4xx.h b/include/asm-ppc/ibm4xx.h
index ed6891a..053a86b 100644
--- a/include/asm-ppc/ibm4xx.h
+++ b/include/asm-ppc/ibm4xx.h
@@ -15,7 +15,6 @@
 #define __ASM_IBM4XX_H__
 
 #include <asm/types.h>
-#include <asm/dcr.h>
 
 #ifdef CONFIG_40x
 
-- 
1.5.3.4-dirty

[PATCH 1/5] [v2][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Previously, dcr support was configured at compile time to either using
MMIO or native dcr instructions.  Although this works for most
platforms, it fails on FPGA platforms:

1) Systems may include more than one dcr bus.
2) Systems may be native dcr capable and still use memory mapped dcr interface.

This patch provides runtime support based on the device trees for the
case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
selected.  Previously, this was a poorly defined configuration, which
happened to provide NATIVE support.  The runtime selection is made
based on the dcr slave device having a 'dcr-access-method' attribute
in the device tree.  If only one of the above options is selected,
then the code uses #defines to select only the used code in order to
avoid interoducing overhead in existing usage.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/dcr.c         |   91 ++++++++++++++++++++++++++++++++-----
 include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
 include/asm-powerpc/dcr-mmio.h    |   20 +++++---
 include/asm-powerpc/dcr-native.h  |   16 ++++---
 include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
 5 files changed, 186 insertions(+), 26 deletions(-)
 create mode 100644 include/asm-powerpc/dcr-generic.h

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index 437e48d..d3de0ff 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -23,6 +23,68 @@
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+bool dcr_map_ok_generic(dcr_host_t host)
+{
+	if (host.type == INVALID)
+		return 0;
+	else if (host.type == NATIVE)
+		return dcr_map_ok_native(host.host.native);
+	else
+		return dcr_map_ok_mmio(host.host.mmio);
+}
+EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
+
+dcr_host_t dcr_map_generic(struct device_node *dev,
+			   unsigned int dcr_n,
+			   unsigned int dcr_c)
+{
+	dcr_host_t host;
+	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
+
+	if (!strcmp(prop, "native")) {
+		host.type = NATIVE;
+		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
+	} else if (!strcmp(prop, "mmio")) {
+		host.type = MMIO;
+		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
+	} else
+		host.type = INVALID;
+
+	return host;
+}
+EXPORT_SYMBOL_GPL(dcr_map_generic);
+
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
+}
+EXPORT_SYMBOL_GPL(dcr_unmap_generic);
+
+u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
+{
+	if (host.type == NATIVE)
+		return dcr_read_native(host.host.native, dcr_n);
+	else
+		return dcr_read_mmio(host.host.mmio, dcr_n);
+}
+EXPORT_SYMBOL_GPL(dcr_read_generic);
+
+void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
+{
+	if (host.type == NATIVE)
+		dcr_write_native(host.host.native, dcr_n, value);
+	else
+		dcr_write_mmio(host.host.mmio, dcr_n, value);
+}
+EXPORT_SYMBOL_GPL(dcr_write_generic);
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
 	unsigned int ds;
@@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
-#ifndef CONFIG_PPC_DCR_NATIVE
+#ifdef CONFIG_PPC_DCR_MMIO
 
 static struct device_node * find_dcr_parent(struct device_node * node)
 {
@@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	return ret;
 }
 
-dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-		   unsigned int dcr_c)
+dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+			     unsigned int dcr_n,
+			     unsigned int dcr_c)
 {
-	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
+	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
 	u64 addr;
 
 	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
 		 dev->full_name, dcr_n, dcr_c);
 
 	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
-	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
-		 addr, ret.stride);
+	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
+		 (unsigned long long) addr, ret.stride);
 	if (addr == OF_BAD_ADDR)
 		return ret;
 	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
@@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 	ret.token -= dcr_n * ret.stride;
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dcr_map);
+EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
+void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
-	dcr_host_t h = host;
+	dcr_host_mmio_t h = host;
 
 	if (h.token == NULL)
 		return;
@@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 	iounmap(h.token);
 	h.token = NULL;
 }
-EXPORT_SYMBOL_GPL(dcr_unmap);
-#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
+EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
+
+#endif /* defined(CONFIG_PPC_DCR_MMIO) */
+
+#ifdef CONFIG_PPC_DCR_NATIVE
 DEFINE_SPINLOCK(dcr_ind_lock);
-#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
+#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
+
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
new file mode 100644
index 0000000..0ee74fb
--- /dev/null
+++ b/include/asm-powerpc/dcr-generic.h
@@ -0,0 +1,49 @@
+/*
+ * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
+ *                    <benh@kernel.crashing.org>
+ *
+ *   This program is free software;  you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   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, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_DCR_GENERIC_H
+#define _ASM_POWERPC_DCR_GENERIC_H
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+enum host_type_t {MMIO, NATIVE, INVALID};
+
+typedef struct {
+	enum host_type_t type;
+	union {
+		dcr_host_mmio_t mmio;
+		dcr_host_native_t native;
+	} host;
+} dcr_host_t;
+
+extern bool dcr_map_ok_generic(dcr_host_t host);
+
+extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
+			  unsigned int dcr_c);
+extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
+
+extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
+
+extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_DCR_GENERIC_H */
+
+
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 08532ff..acd491d 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -27,20 +27,26 @@ typedef struct {
 	void __iomem *token;
 	unsigned int stride;
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_mmio_t;
 
-#define DCR_MAP_OK(host)	((host).token != NULL)
+static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
+{
+	return host.token != NULL;
+}
 
-extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-			  unsigned int dcr_c);
-extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
+extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+				    unsigned int dcr_n,
+				    unsigned int dcr_c);
+extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
-static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
+static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
 {
 	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
 }
 
-static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
+static inline void dcr_write_mmio(dcr_host_mmio_t host,
+				  unsigned int dcr_n,
+				  u32 value)
 {
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index be6c879..67832e5 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -26,14 +26,18 @@
 
 typedef struct {
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_native_t;
 
-#define DCR_MAP_OK(host)	(1)
+static inline bool dcr_map_ok_native(dcr_host_native_t host)
+{
+	return 1;
+}
 
-#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_c)		do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
+#define dcr_map_native(dev, dcr_n, dcr_c) \
+	((dcr_host_native_t){ .base = (dcr_n) })
+#define dcr_unmap_native(host, dcr_c)		do {} while (0)
+#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 9338d50..6b86322 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -20,14 +20,47 @@
 #ifndef _ASM_POWERPC_DCR_H
 #define _ASM_POWERPC_DCR_H
 #ifdef __KERNEL__
+#ifndef __ASSEMBLY__
 #ifdef CONFIG_PPC_DCR
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 #include <asm/dcr-native.h>
-#else
+#endif
+
+#ifdef CONFIG_PPC_DCR_MMIO
 #include <asm/dcr-mmio.h>
 #endif
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+#include <asm/dcr-generic.h>
+
+#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
+
+#else
+
+#ifdef CONFIG_PPC_DCR_NATIVE
+typedef dcr_host_native_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
+#else
+typedef dcr_host_mmio_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
+#endif
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 /*
  * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
  * base from the device-tree
@@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
 #endif /* CONFIG_PPC_MERGE */
 
 #endif /* CONFIG_PPC_DCR */
+#endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_H */
-- 
1.5.3.4-dirty

[PATCH 2/5] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Stephen Neuendorffer]

FPGA designs may have need of both MMIO-based and NATIVE-based dcr
interfaces.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index a9260e2..b8e06df 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -123,6 +123,8 @@ config 405GPR
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_MMIO
+	select PPC_DCR_NATIVE
 
 config XILINX_VIRTEX_II_PRO
 	bool
-- 
1.5.3.4-dirty

[PATCH 3/5] [POWERPC] explicit dcr support

[Stephen Neuendorffer]

Added literal mapping support if no device-tree support.  Added
CONFIG_OF to guard device-tree parts, since literal support works for
arch=ppc.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/dcr.c         |   82 ++++++++++++++++++++++++++++---------
 include/asm-powerpc/dcr-generic.h |    2 +
 include/asm-powerpc/dcr-mmio.h    |   11 +++++
 include/asm-powerpc/dcr-native.h  |    8 ++++
 include/asm-powerpc/dcr.h         |   24 +++++++++++
 5 files changed, 108 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index d3de0ff..2ccae80 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -20,9 +20,34 @@
 #undef DEBUG
 
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+#ifdef CONFIG_OF
+static struct device_node *find_dcr_parent(struct device_node *node)
+{
+	struct device_node *par, *tmp;
+	const u32 *p;
+
+	for (par = of_node_get(node); par;) {
+		if (of_get_property(par, "dcr-controller", NULL))
+			break;
+		p = of_get_property(par, "dcr-parent", NULL);
+		tmp = par;
+		if (p == NULL)
+			par = of_get_parent(par);
+		else
+			par = of_find_node_by_phandle(*p);
+		of_node_put(tmp);
+	}
+	return par;
+}
+#endif /* CONFIG_OF */
+
+
+/* Indirection layer for providing both NATIVE and MMIO support. */
+
 #if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
 
 bool dcr_map_ok_generic(dcr_host_t host)
@@ -36,12 +61,24 @@ bool dcr_map_ok_generic(dcr_host_t host)
 }
 EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
 
+#ifdef CONFIG_OF
 dcr_host_t dcr_map_generic(struct device_node *dev,
 			   unsigned int dcr_n,
 			   unsigned int dcr_c)
 {
 	dcr_host_t host;
-	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
+	struct device_node *dp;
+
+	dp = find_dcr_parent(dev);
+	if (dp == NULL) {
+		host.type = INVALID;
+		return host;
+	}
+
+	const char *prop = of_get_property(dp, "dcr-access-method", NULL);
+
+	pr_debug("dcr_map_generic(dcr-access-method = %s)\n",
+		 prop);
 
 	if (!strcmp(prop, "native")) {
 		host.type = NATIVE;
@@ -56,6 +93,8 @@ dcr_host_t dcr_map_generic(struct device_node *dev,
 }
 EXPORT_SYMBOL_GPL(dcr_map_generic);
 
+#endif /* CONFIG_OF */
+
 void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
 {
 	if (host.type == NATIVE)
@@ -85,9 +124,10 @@ EXPORT_SYMBOL_GPL(dcr_write_generic);
 
 #endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
 
+#ifdef CONFIG_OF
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
-	unsigned int ds;
+	int ds;
 	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
 
 	if (dr == NULL || ds & 1 || index >= (ds / 8))
@@ -99,7 +139,7 @@ EXPORT_SYMBOL_GPL(dcr_resource_start);
 
 unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 {
-	unsigned int ds;
+	int ds;
 	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
 
 	if (dr == NULL || ds & 1 || index >= (ds / 8))
@@ -109,26 +149,28 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
+#endif /* CONFIG_OF */
+
+
+
+/* Support for MMIO */
 #ifdef CONFIG_PPC_DCR_MMIO
 
-static struct device_node * find_dcr_parent(struct device_node * node)
+dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
+					  unsigned int stride,
+					  unsigned int dcr_n,
+					  unsigned int dcr_c)
 {
-	struct device_node *par, *tmp;
-	const u32 *p;
-
-	for (par = of_node_get(node); par;) {
-		if (of_get_property(par, "dcr-controller", NULL))
-			break;
-		p = of_get_property(par, "dcr-parent", NULL);
-		tmp = par;
-		if (p == NULL)
-			par = of_get_parent(par);
-		else
-			par = of_find_node_by_phandle(*p);
-		of_node_put(tmp);
-	}
-	return par;
+	dcr_host_mmio_t host;
+	host.stride = stride;
+	host.token = ioremap(mmio_start, dcr_c * stride);
+	host.token -= dcr_n * stride;
+	host.base = dcr_n;
+	return host;
 }
+EXPORT_SYMBOL_GPL(dcr_map_mmio_literal_mmio);
+
+#ifdef CONFIG_OF
 
 u64 of_translate_dcr_address(struct device_node *dev,
 			     unsigned int dcr_n,
@@ -189,6 +231,8 @@ dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
 }
 EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
+#endif /* CONFIG_OF */
+
 void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
 	dcr_host_mmio_t h = host;
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
index 0ee74fb..8032795 100644
--- a/include/asm-powerpc/dcr-generic.h
+++ b/include/asm-powerpc/dcr-generic.h
@@ -34,8 +34,10 @@ typedef struct {
 
 extern bool dcr_map_ok_generic(dcr_host_t host);
 
+#ifdef CONFIG_OF
 extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
 			  unsigned int dcr_c);
+#endif
 extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
 
 extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index acd491d..b12d291 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -34,9 +34,18 @@ static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
 	return host.token != NULL;
 }
 
+extern dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
+						 unsigned int stride,
+						 unsigned int dcr_n,
+						 unsigned int dcr_c);
+
+
+#ifdef CONFIG_OF
 extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
 				    unsigned int dcr_n,
 				    unsigned int dcr_c);
+#endif
+
 extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
 static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
@@ -51,9 +60,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
 
+#ifdef CONFIG_OF
 extern u64 of_translate_dcr_address(struct device_node *dev,
 				    unsigned int dcr_n,
 				    unsigned int *stride);
+#endif
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_MMIO_H */
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index 67832e5..9b3e255 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -33,6 +33,14 @@ static inline bool dcr_map_ok_native(dcr_host_native_t host)
 	return 1;
 }
 
+static inline
+dcr_host_native_t dcr_map_native_literal_native(unsigned int dcr_n)
+{
+	dcr_host_native_t host;
+	host.base = dcr_n;
+	return host;
+}
+
 #define dcr_map_native(dev, dcr_n, dcr_c) \
 	((dcr_host_native_t){ .base = (dcr_n) })
 #define dcr_unmap_native(host, dcr_c)		do {} while (0)
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 6b86322..dfd1c24 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -35,6 +35,27 @@
 
 #include <asm/dcr-generic.h>
 
+static inline
+dcr_host_t dcr_map_mmio_literal(resource_size_t mmio_start,
+				unsigned int stride,
+				unsigned int dcr_n,
+				unsigned int dcr_c)
+{
+	dcr_host_t host;
+	host.type = MMIO;
+	host.host.mmio =
+		dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c);
+	return host;
+}
+static inline
+dcr_host_t dcr_map_native_literal(unsigned int dcr_n)
+{
+	dcr_host_t host;
+	host.type = NATIVE;
+	host.host.native = dcr_map_native_literal_native(dcr_n);
+	return host;
+}
+
 #define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
@@ -45,6 +66,7 @@
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 typedef dcr_host_native_t dcr_host_t;
+#define dcr_map_native_literal(dcr_n) dcr_map_native_literal_native(dcr_n)
 #define DCR_MAP_OK(host)	dcr_map_ok_native(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
@@ -52,6 +74,8 @@ typedef dcr_host_native_t dcr_host_t;
 #define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
 #else
 typedef dcr_host_mmio_t dcr_host_t;
+#define dcr_map_mmio_literal(mmio_start, stride, dcr_n, dcr_c) \
+	dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c)
 #define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
 #define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
 #define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
-- 
1.5.3.4-dirty

[PATCH 4/5] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[Stephen Neuendorffer]

This device contains a dcr interface.  Previously, the dcr interface
was assumed to be used in mmio mode, and the register space of the dcr
interface was precomputed and stuffed in the device tree.  This patch
makes use of the new dcr infrastructure to represent the dcr interface
as any other dcr interface in the device tree.  This enables the dcr
interface to be connected directly to a native dcr interface in a
clean way.

In particular, the device tree expected looks like:

			dcr_v29_0: dcr@0 {
				#address-cells = <1>;
				#size-cells = <1>;
				compatible = "xlnx,dcr-v29-1.00.a";
				VGA_FrameBuffer: tft@80 {
					compatible = "xlnx,plb-tft-cntlr-ref-1.00.a";
					dcr-parent = <&opb2dcr_bridge_0>;
					dcr-reg = < 80 2 >;
					xlnx,default-tft-base-addr = <7f>;
					xlnx,dps-init = <1>;
					xlnx,on-init = <1>;
					xlnx,pixclk-is-busclk-divby4 = <1>;
				} ;
			} ;

			opb2dcr_bridge_0: opb2dcr-bridge@40700000 {
				compatible = "xlnx,opb2dcr-bridge-1.00.b";
				dcr-access-method = "mmio";
				dcr-controller ;
				dcr-mmio-range = < 40700000 1000 >;
				dcr-mmio-stride = <4>;
				reg = < 40700000 1000 >;
				xlnx,family = "virtex2p";
			} ;

Note that this patch now requires PPC_DCR_MMIO to be set in order to
advertise the framebuffer as a platform device (i.e. ARCH=ppc).

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 drivers/video/xilinxfb.c |   86 ++++++++++++++++++++++++++--------------------
 1 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 7b3a842..171cc09 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -38,6 +38,7 @@
 #endif
 #include <asm/io.h>
 #include <linux/xilinxfb.h>
+#include <asm/dcr.h>
 
 #define DRIVER_NAME		"xilinxfb"
 #define DRIVER_DESCRIPTION	"Xilinx TFT LCD frame buffer driver"
@@ -112,8 +113,9 @@ struct xilinxfb_drvdata {
 
 	struct fb_info	info;		/* FB driver info record */
 
-	u32		regs_phys;	/* phys. address of the control registers */
-	u32 __iomem	*regs;		/* virt. address of the control registers */
+	dcr_host_t      dcr_host;
+	unsigned int    dcr_start;
+	unsigned int    dcr_len;
 
 	void		*fb_virt;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
@@ -136,7 +138,7 @@ struct xilinxfb_drvdata {
  * when it's needed.
  */
 #define xilinx_fb_out_be32(driverdata, offset, val) \
-	out_be32(driverdata->regs + offset, val)
+	dcr_write(driverdata->dcr_host, offset, val)
 
 static int
 xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
@@ -204,7 +206,8 @@ static struct fb_ops xilinxfb_ops =
  * Bus independent setup/teardown
  */
 
-static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
+static int xilinxfb_assign(struct device *dev, dcr_host_t dcr_host,
+			   unsigned int dcr_start, unsigned int dcr_len,
 			   struct xilinxfb_platform_data *pdata)
 {
 	struct xilinxfb_drvdata *drvdata;
@@ -219,21 +222,9 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 	dev_set_drvdata(dev, drvdata);
 
-	/* Map the control registers in */
-	if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_region;
-	}
-	drvdata->regs_phys = physaddr;
-	drvdata->regs = ioremap(physaddr, 8);
-	if (!drvdata->regs) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_map;
-	}
+	drvdata->dcr_start = dcr_start;
+	drvdata->dcr_len = dcr_len;
+	drvdata->dcr_host = dcr_host;
 
 	/* Allocate the framebuffer memory */
 	if (pdata->fb_phys) {
@@ -248,7 +239,7 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
 		rc = -ENOMEM;
-		goto err_fbmem;
+		goto err_region;
 	}
 
 	/* Clear (turn to black) the framebuffer */
@@ -298,7 +289,6 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 
 	/* Put a banner in the log (for DEBUG) */
-	dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr, drvdata->regs);
 	dev_dbg(dev, "fb: phys=%p, virt=%p, size=%x\n",
 		(void*)drvdata->fb_phys, drvdata->fb_virt, fbsize);
 
@@ -314,12 +304,6 @@ err_cmap:
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
 
-err_fbmem:
-	iounmap(drvdata->regs);
-
-err_map:
-	release_mem_region(physaddr, 8);
-
 err_region:
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -345,9 +329,8 @@ static int xilinxfb_release(struct device *dev)
 
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
-	iounmap(drvdata->regs);
 
-	release_mem_region(drvdata->regs_phys, 8);
+	dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -355,6 +338,7 @@ static int xilinxfb_release(struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_PPC_DCR_MMIO
 /* ---------------------------------------------------------------------
  * Platform bus binding
  */
@@ -364,6 +348,9 @@ xilinxfb_platform_probe(struct platform_device *pdev)
 {
 	struct xilinxfb_platform_data *pdata;
 	struct resource *res;
+	dcr_host_t dcr_host;
+	int dcr_start = 0;
+	int dcr_len = 2;
 
 	/* Find the registers address */
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
@@ -386,7 +373,12 @@ xilinxfb_platform_probe(struct platform_device *pdev)
 			pdata->yvirt = xilinx_fb_default_pdata.yvirt;
 	}
 
-	return xilinxfb_assign(&pdev->dev, res->start, pdata);
+	dcr_host = dcr_map_mmio_literal(res->start, 4, dcr_start, dcr_len);
+	if (!DCR_MAP_OK(dcr_host)) {
+		dev_err(&pdev->dev, "invalid address\n");
+		return -ENODEV;
+	}
+	return xilinxfb_assign(&pdev->dev, dcr_host, dcr_start, dcr_len, pdata);
 }
 
 static int
@@ -405,6 +397,23 @@ static struct platform_driver xilinxfb_platform_driver = {
 	},
 };
 
+/* Registration helpers to keep the number of #ifdefs to a minimum */
+static inline int __init xilinxfb_platform_register(void)
+{
+	pr_debug("xilinxfb: calling of_register_platform_driver()\n");
+	return platform_driver_register(&xilinxfb_platform_driver);
+}
+
+static inline void __exit xilinxfb_platform_unregister(void)
+{
+	platform_driver_unregister(&xilinxfb_platform_driver);
+}
+#else /* CONFIG_PPC_DCR_MMIO */
+/* CONFIG_OF not enabled; do nothing helpers */
+static inline int __init xilinxfb_platform_register(void) { return 0; }
+static inline void __exit xilinxfb_platform_unregister(void) { }
+#endif /* CONFIG_PPC_DCR_MMIO */
+
 /* ---------------------------------------------------------------------
  * OF bus binding
  */
@@ -413,20 +422,23 @@ static struct platform_driver xilinxfb_platform_driver = {
 static int __devinit
 xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 {
-	struct resource res;
 	const u32 *prop;
 	struct xilinxfb_platform_data pdata;
 	int size, rc;
+	int start, len;
+	dcr_host_t dcr_host;
 
 	/* Copy with the default pdata (not a ptr reference!) */
 	pdata = xilinx_fb_default_pdata;
 
 	dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
 
-	rc = of_address_to_resource(op->node, 0, &res);
-	if (rc) {
+	start = dcr_resource_start(op->node, 0);
+	len = dcr_resource_len(op->node, 0);
+	dcr_host = dcr_map(op->node, start, len);
+	if (!DCR_MAP_OK(dcr_host)) {
 		dev_err(&op->dev, "invalid address\n");
-		return rc;
+		return -ENODEV;
 	}
 
 	prop = of_get_property(op->node, "phys-size", &size);
@@ -450,7 +462,7 @@ xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 	if (of_find_property(op->node, "rotate-display", NULL))
 		pdata.rotate_screen = 1;
 
-	return xilinxfb_assign(&op->dev, res.start, &pdata);
+	return xilinxfb_assign(&op->dev, dcr_host, start, len, &pdata);
 }
 
 static int __devexit xilinxfb_of_remove(struct of_device *op)
@@ -505,7 +517,7 @@ xilinxfb_init(void)
 	if (rc)
 		return rc;
 
-	rc = platform_driver_register(&xilinxfb_platform_driver);
+	rc = xilinxfb_platform_register();
 	if (rc)
 		xilinxfb_of_unregister();
 
@@ -515,7 +527,7 @@ xilinxfb_init(void)
 static void __exit
 xilinxfb_cleanup(void)
 {
-	platform_driver_unregister(&xilinxfb_platform_driver);
+	xilinxfb_platform_unregister();
 	xilinxfb_of_unregister();
 }
 
-- 
1.5.3.4-dirty

[PATCH 5/5] [RFC][PPC] Use DCR for arch ppc, and enable MMIO and NATIVE for virtex.

[Stephen Neuendorffer]

Generally speaking, I'm not sure about the validity of this patch.  In
particular:

1) I'm not sure what architecture code relies on including
dcr support through ibm4xx.h.

2) I'm not sure how temporary the ifdef in
arch/powerpc/sysdev/Makefile really is.  Is it still needed or not?

However, this appears to work at least for the Virtex systems I've
tried.  In any event, I don't expect this to actually be sent to
mainline, since it's ARCH=ppc related, at least not in this form.

Unfortunately, the framebuffer driver won't work under ARCH=ppc
without this patch.
---
 arch/powerpc/sysdev/Makefile   |    2 --
 arch/ppc/Kconfig               |    6 +++++-
 arch/ppc/platforms/4xx/Kconfig |    2 ++
 include/asm-ppc/ibm4xx.h       |    1 -
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 15f3e85..c85525b 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -35,10 +35,8 @@ endif
 endif
 
 # Temporary hack until we have migrated to asm-powerpc
-ifeq ($(ARCH),powerpc)
 obj-$(CONFIG_CPM)		+= cpm_common.o
 obj-$(CONFIG_CPM2)		+= cpm2.o cpm2_pic.o
 obj-$(CONFIG_PPC_DCR)		+= dcr.o
 obj-$(CONFIG_8xx)		+= mpc8xx_pic.o cpm1.o
 obj-$(CONFIG_UCODE_PATCH)	+= micropatch.o
-endif
diff --git a/arch/ppc/Kconfig b/arch/ppc/Kconfig
index abc877f..81d6748 100644
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -116,9 +116,13 @@ config PPC_DCR_NATIVE
 	bool
 	default n
 
+config PPC_DCR_MMIO
+	bool
+	default n
+
 config PPC_DCR
 	bool
-	depends on PPC_DCR_NATIVE
+	depends on PPC_DCR_NATIVE || PPC_DCR_MMIO
 	default y
 
 config PTE_64BIT
diff --git a/arch/ppc/platforms/4xx/Kconfig b/arch/ppc/platforms/4xx/Kconfig
index 76551b6..0430fc2 100644
--- a/arch/ppc/platforms/4xx/Kconfig
+++ b/arch/ppc/platforms/4xx/Kconfig
@@ -228,6 +228,8 @@ config XILINX_VIRTEX_4_FX
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_NATIVE
+	select PPC_DCR_MMIO
 
 config STB03xxx
 	bool
diff --git a/include/asm-ppc/ibm4xx.h b/include/asm-ppc/ibm4xx.h
index ed6891a..053a86b 100644
--- a/include/asm-ppc/ibm4xx.h
+++ b/include/asm-ppc/ibm4xx.h
@@ -15,7 +15,6 @@
 #define __ASM_IBM4XX_H__
 
 #include <asm/types.h>
-#include <asm/dcr.h>
 
 #ifdef CONFIG_40x
 
-- 
1.5.3.4-dirty

Re: [PATCH 1/5] [v2][POWERPC] refactor dcr code

[Benjamin Herrenschmidt]

On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer wrote:
> Previously, dcr support was configured at compile time to either using
> MMIO or native dcr instructions.  Although this works for most
> platforms, it fails on FPGA platforms:
> 
> 1) Systems may include more than one dcr bus.
> 2) Systems may be native dcr capable and still use memory mapped dcr interface.
> 
> This patch provides runtime support based on the device trees for the
> case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> selected.  Previously, this was a poorly defined configuration, which
> happened to provide NATIVE support.  The runtime selection is made
> based on the dcr slave device having a 'dcr-access-method' attribute
> in the device tree.  If only one of the above options is selected,
> then the code uses #defines to select only the used code in order to
> avoid interoducing overhead in existing usage.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Looks good. Haven't had a chance to test it yet, but tentatively

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Initially, I thought about using function pointers but the if/else will
probably end up being more efficient. The only thing maybe to ponder
is whether we could avoid the dcr-generic.h file completely, and have
the generic wrappers be inline, though considering how slow DCRs are,
it may not be that useful in practice and less clean...

Cheers,
Ben. 


> ---
>  arch/powerpc/sysdev/dcr.c         |   91 ++++++++++++++++++++++++++++++++-----
>  include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
>  include/asm-powerpc/dcr-mmio.h    |   20 +++++---
>  include/asm-powerpc/dcr-native.h  |   16 ++++---
>  include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
>  5 files changed, 186 insertions(+), 26 deletions(-)
>  create mode 100644 include/asm-powerpc/dcr-generic.h
> 
> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index 437e48d..d3de0ff 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -23,6 +23,68 @@
>  #include <asm/prom.h>
>  #include <asm/dcr.h>
>  
> +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> +
> +bool dcr_map_ok_generic(dcr_host_t host)
> +{
> +	if (host.type == INVALID)
> +		return 0;
> +	else if (host.type == NATIVE)
> +		return dcr_map_ok_native(host.host.native);
> +	else
> +		return dcr_map_ok_mmio(host.host.mmio);
> +}
> +EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
> +
> +dcr_host_t dcr_map_generic(struct device_node *dev,
> +			   unsigned int dcr_n,
> +			   unsigned int dcr_c)
> +{
> +	dcr_host_t host;
> +	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
> +
> +	if (!strcmp(prop, "native")) {
> +		host.type = NATIVE;
> +		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
> +	} else if (!strcmp(prop, "mmio")) {
> +		host.type = MMIO;
> +		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
> +	} else
> +		host.type = INVALID;
> +
> +	return host;
> +}
> +EXPORT_SYMBOL_GPL(dcr_map_generic);
> +
> +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> +{
> +	if (host.type == NATIVE)
> +		dcr_unmap_native(host.host.native, dcr_c);
> +	else
> +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> +}
> +EXPORT_SYMBOL_GPL(dcr_unmap_generic);
> +
> +u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
> +{
> +	if (host.type == NATIVE)
> +		return dcr_read_native(host.host.native, dcr_n);
> +	else
> +		return dcr_read_mmio(host.host.mmio, dcr_n);
> +}
> +EXPORT_SYMBOL_GPL(dcr_read_generic);
> +
> +void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
> +{
> +	if (host.type == NATIVE)
> +		dcr_write_native(host.host.native, dcr_n, value);
> +	else
> +		dcr_write_mmio(host.host.mmio, dcr_n, value);
> +}
> +EXPORT_SYMBOL_GPL(dcr_write_generic);
> +
> +#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
> +
>  unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
>  {
>  	unsigned int ds;
> @@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
>  }
>  EXPORT_SYMBOL_GPL(dcr_resource_len);
>  
> -#ifndef CONFIG_PPC_DCR_NATIVE
> +#ifdef CONFIG_PPC_DCR_MMIO
>  
>  static struct device_node * find_dcr_parent(struct device_node * node)
>  {
> @@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct device_node *dev,
>  	return ret;
>  }
>  
> -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> -		   unsigned int dcr_c)
> +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> +			     unsigned int dcr_n,
> +			     unsigned int dcr_c)
>  {
> -	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
> +	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
>  	u64 addr;
>  
>  	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
>  		 dev->full_name, dcr_n, dcr_c);
>  
>  	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
> -	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> -		 addr, ret.stride);
> +	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
> +		 (unsigned long long) addr, ret.stride);
>  	if (addr == OF_BAD_ADDR)
>  		return ret;
>  	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
> @@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
>  	ret.token -= dcr_n * ret.stride;
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(dcr_map);
> +EXPORT_SYMBOL_GPL(dcr_map_mmio);
>  
> -void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
> +void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
>  {
> -	dcr_host_t h = host;
> +	dcr_host_mmio_t h = host;
>  
>  	if (h.token == NULL)
>  		return;
> @@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
>  	iounmap(h.token);
>  	h.token = NULL;
>  }
> -EXPORT_SYMBOL_GPL(dcr_unmap);
> -#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
> +EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
> +
> +#endif /* defined(CONFIG_PPC_DCR_MMIO) */
> +
> +#ifdef CONFIG_PPC_DCR_NATIVE
>  DEFINE_SPINLOCK(dcr_ind_lock);
> -#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
> +#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
> +
> diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
> new file mode 100644
> index 0000000..0ee74fb
> --- /dev/null
> +++ b/include/asm-powerpc/dcr-generic.h
> @@ -0,0 +1,49 @@
> +/*
> + * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
> + *                    <benh@kernel.crashing.org>
> + *
> + *   This program is free software;  you can redistribute it and/or modify
> + *   it under the terms of the GNU General Public License as published by
> + *   the Free Software Foundation; either version 2 of the License, or
> + *   (at your option) any later version.
> + *
> + *   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, write to the Free Software
> + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#ifndef _ASM_POWERPC_DCR_GENERIC_H
> +#define _ASM_POWERPC_DCR_GENERIC_H
> +#ifdef __KERNEL__
> +#ifndef __ASSEMBLY__
> +
> +enum host_type_t {MMIO, NATIVE, INVALID};
> +
> +typedef struct {
> +	enum host_type_t type;
> +	union {
> +		dcr_host_mmio_t mmio;
> +		dcr_host_native_t native;
> +	} host;
> +} dcr_host_t;
> +
> +extern bool dcr_map_ok_generic(dcr_host_t host);
> +
> +extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
> +			  unsigned int dcr_c);
> +extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
> +
> +extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> +
> +extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
> +
> +#endif /* __ASSEMBLY__ */
> +#endif /* __KERNEL__ */
> +#endif /* _ASM_POWERPC_DCR_GENERIC_H */
> +
> +
> diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
> index 08532ff..acd491d 100644
> --- a/include/asm-powerpc/dcr-mmio.h
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -27,20 +27,26 @@ typedef struct {
>  	void __iomem *token;
>  	unsigned int stride;
>  	unsigned int base;
> -} dcr_host_t;
> +} dcr_host_mmio_t;
>  
> -#define DCR_MAP_OK(host)	((host).token != NULL)
> +static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
> +{
> +	return host.token != NULL;
> +}
>  
> -extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> -			  unsigned int dcr_c);
> -extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
> +extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> +				    unsigned int dcr_n,
> +				    unsigned int dcr_c);
> +extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
>  
> -static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> +static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
>  {
>  	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
>  }
>  
> -static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
> +static inline void dcr_write_mmio(dcr_host_mmio_t host,
> +				  unsigned int dcr_n,
> +				  u32 value)
>  {
>  	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
>  }
> diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
> index be6c879..67832e5 100644
> --- a/include/asm-powerpc/dcr-native.h
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -26,14 +26,18 @@
>  
>  typedef struct {
>  	unsigned int base;
> -} dcr_host_t;
> +} dcr_host_native_t;
>  
> -#define DCR_MAP_OK(host)	(1)
> +static inline bool dcr_map_ok_native(dcr_host_native_t host)
> +{
> +	return 1;
> +}
>  
> -#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
> -#define dcr_unmap(host, dcr_c)		do {} while (0)
> -#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
> -#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
> +#define dcr_map_native(dev, dcr_n, dcr_c) \
> +	((dcr_host_native_t){ .base = (dcr_n) })
> +#define dcr_unmap_native(host, dcr_c)		do {} while (0)
> +#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
> +#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
>  
>  /* Device Control Registers */
>  void __mtdcr(int reg, unsigned int val);
> diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> index 9338d50..6b86322 100644
> --- a/include/asm-powerpc/dcr.h
> +++ b/include/asm-powerpc/dcr.h
> @@ -20,14 +20,47 @@
>  #ifndef _ASM_POWERPC_DCR_H
>  #define _ASM_POWERPC_DCR_H
>  #ifdef __KERNEL__
> +#ifndef __ASSEMBLY__
>  #ifdef CONFIG_PPC_DCR
>  
>  #ifdef CONFIG_PPC_DCR_NATIVE
>  #include <asm/dcr-native.h>
> -#else
> +#endif
> +
> +#ifdef CONFIG_PPC_DCR_MMIO
>  #include <asm/dcr-mmio.h>
>  #endif
>  
> +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> +
> +#include <asm/dcr-generic.h>
> +
> +#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
> +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
> +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
> +#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
> +
> +#else
> +
> +#ifdef CONFIG_PPC_DCR_NATIVE
> +typedef dcr_host_native_t dcr_host_t;
> +#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
> +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
> +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
> +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
> +#else
> +typedef dcr_host_mmio_t dcr_host_t;
> +#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
> +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
> +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
> +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
> +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
> +#endif
> +
> +#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
> +
>  /*
>   * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
>   * base from the device-tree
> @@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
>  #endif /* CONFIG_PPC_MERGE */
>  
>  #endif /* CONFIG_PPC_DCR */
> +#endif /* __ASSEMBLY__ */
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_DCR_H */

Re: [PATCH 3/5] [POWERPC] explicit dcr support

[Benjamin Herrenschmidt]

On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer wrote:
> Added literal mapping support if no device-tree support.  Added
> CONFIG_OF to guard device-tree parts, since literal support works for
> arch=ppc.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

I'll let others decide whether they want to do that addition
to arch/ppc (which is really what it is) or not ... I'd rather
not myself considering that arch/ppc is scheduled to go away
soon now.

Cheers,
Ben.

> ---
>  arch/powerpc/sysdev/dcr.c         |   82 ++++++++++++++++++++++++++++---------
>  include/asm-powerpc/dcr-generic.h |    2 +
>  include/asm-powerpc/dcr-mmio.h    |   11 +++++
>  include/asm-powerpc/dcr-native.h  |    8 ++++
>  include/asm-powerpc/dcr.h         |   24 +++++++++++
>  5 files changed, 108 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index d3de0ff..2ccae80 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -20,9 +20,34 @@
>  #undef DEBUG
>  
>  #include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <asm/prom.h>
>  #include <asm/dcr.h>
>  
> +#ifdef CONFIG_OF
> +static struct device_node *find_dcr_parent(struct device_node *node)
> +{
> +	struct device_node *par, *tmp;
> +	const u32 *p;
> +
> +	for (par = of_node_get(node); par;) {
> +		if (of_get_property(par, "dcr-controller", NULL))
> +			break;
> +		p = of_get_property(par, "dcr-parent", NULL);
> +		tmp = par;
> +		if (p == NULL)
> +			par = of_get_parent(par);
> +		else
> +			par = of_find_node_by_phandle(*p);
> +		of_node_put(tmp);
> +	}
> +	return par;
> +}
> +#endif /* CONFIG_OF */
> +
> +
> +/* Indirection layer for providing both NATIVE and MMIO support. */
> +
>  #if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
>  
>  bool dcr_map_ok_generic(dcr_host_t host)
> @@ -36,12 +61,24 @@ bool dcr_map_ok_generic(dcr_host_t host)
>  }
>  EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
>  
> +#ifdef CONFIG_OF
>  dcr_host_t dcr_map_generic(struct device_node *dev,
>  			   unsigned int dcr_n,
>  			   unsigned int dcr_c)
>  {
>  	dcr_host_t host;
> -	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
> +	struct device_node *dp;
> +
> +	dp = find_dcr_parent(dev);
> +	if (dp == NULL) {
> +		host.type = INVALID;
> +		return host;
> +	}
> +
> +	const char *prop = of_get_property(dp, "dcr-access-method", NULL);
> +
> +	pr_debug("dcr_map_generic(dcr-access-method = %s)\n",
> +		 prop);
>  
>  	if (!strcmp(prop, "native")) {
>  		host.type = NATIVE;
> @@ -56,6 +93,8 @@ dcr_host_t dcr_map_generic(struct device_node *dev,
>  }
>  EXPORT_SYMBOL_GPL(dcr_map_generic);
>  
> +#endif /* CONFIG_OF */
> +
>  void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
>  {
>  	if (host.type == NATIVE)
> @@ -85,9 +124,10 @@ EXPORT_SYMBOL_GPL(dcr_write_generic);
>  
>  #endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
>  
> +#ifdef CONFIG_OF
>  unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
>  {
> -	unsigned int ds;
> +	int ds;
>  	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
>  
>  	if (dr == NULL || ds & 1 || index >= (ds / 8))
> @@ -99,7 +139,7 @@ EXPORT_SYMBOL_GPL(dcr_resource_start);
>  
>  unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
>  {
> -	unsigned int ds;
> +	int ds;
>  	const u32 *dr = of_get_property(np, "dcr-reg", &ds);
>  
>  	if (dr == NULL || ds & 1 || index >= (ds / 8))
> @@ -109,26 +149,28 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
>  }
>  EXPORT_SYMBOL_GPL(dcr_resource_len);
>  
> +#endif /* CONFIG_OF */
> +
> +
> +
> +/* Support for MMIO */
>  #ifdef CONFIG_PPC_DCR_MMIO
>  
> -static struct device_node * find_dcr_parent(struct device_node * node)
> +dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
> +					  unsigned int stride,
> +					  unsigned int dcr_n,
> +					  unsigned int dcr_c)
>  {
> -	struct device_node *par, *tmp;
> -	const u32 *p;
> -
> -	for (par = of_node_get(node); par;) {
> -		if (of_get_property(par, "dcr-controller", NULL))
> -			break;
> -		p = of_get_property(par, "dcr-parent", NULL);
> -		tmp = par;
> -		if (p == NULL)
> -			par = of_get_parent(par);
> -		else
> -			par = of_find_node_by_phandle(*p);
> -		of_node_put(tmp);
> -	}
> -	return par;
> +	dcr_host_mmio_t host;
> +	host.stride = stride;
> +	host.token = ioremap(mmio_start, dcr_c * stride);
> +	host.token -= dcr_n * stride;
> +	host.base = dcr_n;
> +	return host;
>  }
> +EXPORT_SYMBOL_GPL(dcr_map_mmio_literal_mmio);
> +
> +#ifdef CONFIG_OF
>  
>  u64 of_translate_dcr_address(struct device_node *dev,
>  			     unsigned int dcr_n,
> @@ -189,6 +231,8 @@ dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
>  }
>  EXPORT_SYMBOL_GPL(dcr_map_mmio);
>  
> +#endif /* CONFIG_OF */
> +
>  void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
>  {
>  	dcr_host_mmio_t h = host;
> diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
> index 0ee74fb..8032795 100644
> --- a/include/asm-powerpc/dcr-generic.h
> +++ b/include/asm-powerpc/dcr-generic.h
> @@ -34,8 +34,10 @@ typedef struct {
>  
>  extern bool dcr_map_ok_generic(dcr_host_t host);
>  
> +#ifdef CONFIG_OF
>  extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
>  			  unsigned int dcr_c);
> +#endif
>  extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
>  
>  extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
> index acd491d..b12d291 100644
> --- a/include/asm-powerpc/dcr-mmio.h
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -34,9 +34,18 @@ static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
>  	return host.token != NULL;
>  }
>  
> +extern dcr_host_mmio_t dcr_map_mmio_literal_mmio(resource_size_t mmio_start,
> +						 unsigned int stride,
> +						 unsigned int dcr_n,
> +						 unsigned int dcr_c);
> +
> +
> +#ifdef CONFIG_OF
>  extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
>  				    unsigned int dcr_n,
>  				    unsigned int dcr_c);
> +#endif
> +
>  extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
>  
>  static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
> @@ -51,9 +60,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
>  	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
>  }
>  
> +#ifdef CONFIG_OF
>  extern u64 of_translate_dcr_address(struct device_node *dev,
>  				    unsigned int dcr_n,
>  				    unsigned int *stride);
> +#endif
>  
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_DCR_MMIO_H */
> diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
> index 67832e5..9b3e255 100644
> --- a/include/asm-powerpc/dcr-native.h
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -33,6 +33,14 @@ static inline bool dcr_map_ok_native(dcr_host_native_t host)
>  	return 1;
>  }
>  
> +static inline
> +dcr_host_native_t dcr_map_native_literal_native(unsigned int dcr_n)
> +{
> +	dcr_host_native_t host;
> +	host.base = dcr_n;
> +	return host;
> +}
> +
>  #define dcr_map_native(dev, dcr_n, dcr_c) \
>  	((dcr_host_native_t){ .base = (dcr_n) })
>  #define dcr_unmap_native(host, dcr_c)		do {} while (0)
> diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> index 6b86322..dfd1c24 100644
> --- a/include/asm-powerpc/dcr.h
> +++ b/include/asm-powerpc/dcr.h
> @@ -35,6 +35,27 @@
>  
>  #include <asm/dcr-generic.h>
>  
> +static inline
> +dcr_host_t dcr_map_mmio_literal(resource_size_t mmio_start,
> +				unsigned int stride,
> +				unsigned int dcr_n,
> +				unsigned int dcr_c)
> +{
> +	dcr_host_t host;
> +	host.type = MMIO;
> +	host.host.mmio =
> +		dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c);
> +	return host;
> +}
> +static inline
> +dcr_host_t dcr_map_native_literal(unsigned int dcr_n)
> +{
> +	dcr_host_t host;
> +	host.type = NATIVE;
> +	host.host.native = dcr_map_native_literal_native(dcr_n);
> +	return host;
> +}
> +
>  #define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
>  #define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
>  #define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> @@ -45,6 +66,7 @@
>  
>  #ifdef CONFIG_PPC_DCR_NATIVE
>  typedef dcr_host_native_t dcr_host_t;
> +#define dcr_map_native_literal(dcr_n) dcr_map_native_literal_native(dcr_n)
>  #define DCR_MAP_OK(host)	dcr_map_ok_native(host)
>  #define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
>  #define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> @@ -52,6 +74,8 @@ typedef dcr_host_native_t dcr_host_t;
>  #define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
>  #else
>  typedef dcr_host_mmio_t dcr_host_t;
> +#define dcr_map_mmio_literal(mmio_start, stride, dcr_n, dcr_c) \
> +	dcr_map_mmio_literal_mmio(mmio_start, stride, dcr_n, dcr_c)
>  #define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
>  #define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
>  #define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)

RE: [PATCH 1/5] [v2][POWERPC] refactor dcr code

[Stephen Neuendorffer]

I considered function pointers as well, but I think something like
dcr-generic.h is still required in order to get the specialization if
only one is selected.

Also note that I neglected to change the patch description: The
dcr-access-method attribute is applied to the dcr-controller node, not
the dcr slave.

I'd particularly appreciate it if someone with access to PPC64 would
test this...

Steve

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Sunday, March 30, 2008 3:03 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/5] [v2][POWERPC] refactor dcr code
>=20
>=20
> On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer wrote:
> > Previously, dcr support was configured at compile time to either
using
> > MMIO or native dcr instructions.  Although this works for most
> > platforms, it fails on FPGA platforms:
> >
> > 1) Systems may include more than one dcr bus.
> > 2) Systems may be native dcr capable and still use memory mapped dcr
interface.
> >
> > This patch provides runtime support based on the device trees for
the
> > case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> > selected.  Previously, this was a poorly defined configuration,
which
> > happened to provide NATIVE support.  The runtime selection is made
> > based on the dcr slave device having a 'dcr-access-method' attribute
> > in the device tree.  If only one of the above options is selected,
> > then the code uses #defines to select only the used code in order to
> > avoid interoducing overhead in existing usage.
> >
> > Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com>
>=20
> Looks good. Haven't had a chance to test it yet, but tentatively
>=20
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>=20
> Initially, I thought about using function pointers but the if/else
will
> probably end up being more efficient. The only thing maybe to ponder
> is whether we could avoid the dcr-generic.h file completely, and have
> the generic wrappers be inline, though considering how slow DCRs are,
> it may not be that useful in practice and less clean...
>=20
> Cheers,
> Ben.
>=20
>=20
> > ---
> >  arch/powerpc/sysdev/dcr.c         |   91
++++++++++++++++++++++++++++++++-----
> >  include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
> >  include/asm-powerpc/dcr-mmio.h    |   20 +++++---
> >  include/asm-powerpc/dcr-native.h  |   16 ++++---
> >  include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
> >  5 files changed, 186 insertions(+), 26 deletions(-)
> >  create mode 100644 include/asm-powerpc/dcr-generic.h
> >
> > diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> > index 437e48d..d3de0ff 100644
> > --- a/arch/powerpc/sysdev/dcr.c
> > +++ b/arch/powerpc/sysdev/dcr.c
> > @@ -23,6 +23,68 @@
> >  #include <asm/prom.h>
> >  #include <asm/dcr.h>
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +bool dcr_map_ok_generic(dcr_host_t host)
> > +{
> > +	if (host.type =3D=3D INVALID)
> > +		return 0;
> > +	else if (host.type =3D=3D NATIVE)
> > +		return dcr_map_ok_native(host.host.native);
> > +	else
> > +		return dcr_map_ok_mmio(host.host.mmio);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
> > +
> > +dcr_host_t dcr_map_generic(struct device_node *dev,
> > +			   unsigned int dcr_n,
> > +			   unsigned int dcr_c)
> > +{
> > +	dcr_host_t host;
> > +	const char *prop =3D of_get_property(dev, "dcr-access-method",
NULL);
> > +
> > +	if (!strcmp(prop, "native")) {
> > +		host.type =3D NATIVE;
> > +		host.host.native =3D dcr_map_native(dev, dcr_n, dcr_c);
> > +	} else if (!strcmp(prop, "mmio")) {
> > +		host.type =3D MMIO;
> > +		host.host.mmio =3D dcr_map_mmio(dev, dcr_n, dcr_c);
> > +	} else
> > +		host.type =3D INVALID;
> > +
> > +	return host;
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_generic);
> > +
> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_unmap_native(host.host.native, dcr_c);
> > +	else
> > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_unmap_generic);
> > +
> > +u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		return dcr_read_native(host.host.native, dcr_n);
> > +	else
> > +		return dcr_read_mmio(host.host.mmio, dcr_n);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_read_generic);
> > +
> > +void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32
value)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_write_native(host.host.native, dcr_n, value);
> > +	else
> > +		dcr_write_mmio(host.host.mmio, dcr_n, value);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_write_generic);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  unsigned int dcr_resource_start(struct device_node *np, unsigned
int index)
> >  {
> >  	unsigned int ds;
> > @@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node
*np, unsigned int index)
> >  }
> >  EXPORT_SYMBOL_GPL(dcr_resource_len);
> >
> > -#ifndef CONFIG_PPC_DCR_NATIVE
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >
> >  static struct device_node * find_dcr_parent(struct device_node *
node)
> >  {
> > @@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct
device_node *dev,
> >  	return ret;
> >  }
> >
> > -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> > -		   unsigned int dcr_c)
> > +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +			     unsigned int dcr_n,
> > +			     unsigned int dcr_c)
> >  {
> > -	dcr_host_t ret =3D { .token =3D NULL, .stride =3D 0, .base =3D =
dcr_n };
> > +	dcr_host_mmio_t ret =3D { .token =3D NULL, .stride =3D 0, .base =
=3D
dcr_n };
> >  	u64 addr;
> >
> >  	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> >  		 dev->full_name, dcr_n, dcr_c);
> >
> >  	addr =3D of_translate_dcr_address(dev, dcr_n, &ret.stride);
> > -	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> > -		 addr, ret.stride);
> > +	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
> > +		 (unsigned long long) addr, ret.stride);
> >  	if (addr =3D=3D OF_BAD_ADDR)
> >  		return ret;
> >  	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
> > @@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev,
unsigned int dcr_n,
> >  	ret.token -=3D dcr_n * ret.stride;
> >  	return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_map);
> > +EXPORT_SYMBOL_GPL(dcr_map_mmio);
> >
> > -void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
> > +void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
> >  {
> > -	dcr_host_t h =3D host;
> > +	dcr_host_mmio_t h =3D host;
> >
> >  	if (h.token =3D=3D NULL)
> >  		return;
> > @@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int
dcr_c)
> >  	iounmap(h.token);
> >  	h.token =3D NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_unmap);
> > -#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> >  DEFINE_SPINLOCK(dcr_ind_lock);
> > -#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
> > +#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +
> > diff --git a/include/asm-powerpc/dcr-generic.h
b/include/asm-powerpc/dcr-generic.h
> > new file mode 100644
> > index 0000000..0ee74fb
> > --- /dev/null
> > +++ b/include/asm-powerpc/dcr-generic.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
> > + *                    <benh@kernel.crashing.org>
> > + *
> > + *   This program is free software;  you can redistribute it and/or
modify
> > + *   it under the terms of the GNU General Public License as
published by
> > + *   the Free Software Foundation; either version 2 of the License,
or
> > + *   (at your option) any later version.
> > + *
> > + *   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, write to the Free Software
> > + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
> > + */
> > +
> > +#ifndef _ASM_POWERPC_DCR_GENERIC_H
> > +#define _ASM_POWERPC_DCR_GENERIC_H
> > +#ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> > +
> > +enum host_type_t {MMIO, NATIVE, INVALID};
> > +
> > +typedef struct {
> > +	enum host_type_t type;
> > +	union {
> > +		dcr_host_mmio_t mmio;
> > +		dcr_host_native_t native;
> > +	} host;
> > +} dcr_host_t;
> > +
> > +extern bool dcr_map_ok_generic(dcr_host_t host);
> > +
> > +extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned
int dcr_n,
> > +			  unsigned int dcr_c);
> > +extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
> > +
> > +extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> > +
> > +extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n,
u32 value);
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __KERNEL__ */
> > +#endif /* _ASM_POWERPC_DCR_GENERIC_H */
> > +
> > +
> > diff --git a/include/asm-powerpc/dcr-mmio.h
b/include/asm-powerpc/dcr-mmio.h
> > index 08532ff..acd491d 100644
> > --- a/include/asm-powerpc/dcr-mmio.h
> > +++ b/include/asm-powerpc/dcr-mmio.h
> > @@ -27,20 +27,26 @@ typedef struct {
> >  	void __iomem *token;
> >  	unsigned int stride;
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_mmio_t;
> >
> > -#define DCR_MAP_OK(host)	((host).token !=3D NULL)
> > +static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
> > +{
> > +	return host.token !=3D NULL;
> > +}
> >
> > -extern dcr_host_t dcr_map(struct device_node *dev, unsigned int
dcr_n,
> > -			  unsigned int dcr_c);
> > -extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
> > +extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +				    unsigned int dcr_n,
> > +				    unsigned int dcr_c);
> > +extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int
dcr_c);
> >
> > -static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> > +static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int
dcr_n)
> >  {
> >  	return in_be32(host.token + ((host.base + dcr_n) *
host.stride));
> >  }
> >
> > -static inline void dcr_write(dcr_host_t host, unsigned int dcr_n,
u32 value)
> > +static inline void dcr_write_mmio(dcr_host_mmio_t host,
> > +				  unsigned int dcr_n,
> > +				  u32 value)
> >  {
> >  	out_be32(host.token + ((host.base + dcr_n) * host.stride),
value);
> >  }
> > diff --git a/include/asm-powerpc/dcr-native.h
b/include/asm-powerpc/dcr-native.h
> > index be6c879..67832e5 100644
> > --- a/include/asm-powerpc/dcr-native.h
> > +++ b/include/asm-powerpc/dcr-native.h
> > @@ -26,14 +26,18 @@
> >
> >  typedef struct {
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_native_t;
> >
> > -#define DCR_MAP_OK(host)	(1)
> > +static inline bool dcr_map_ok_native(dcr_host_native_t host)
> > +{
> > +	return 1;
> > +}
> >
> > -#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base =3D (dcr_n)
})
> > -#define dcr_unmap(host, dcr_c)		do {} while (0)
> > -#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
> > -#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base,
value)
> > +#define dcr_map_native(dev, dcr_n, dcr_c) \
> > +	((dcr_host_native_t){ .base =3D (dcr_n) })
> > +#define dcr_unmap_native(host, dcr_c)		do {} while (0)
> > +#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n +
host.base)
> > +#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n +
host.base, value)
> >
> >  /* Device Control Registers */
> >  void __mtdcr(int reg, unsigned int val);
> > diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> > index 9338d50..6b86322 100644
> > --- a/include/asm-powerpc/dcr.h
> > +++ b/include/asm-powerpc/dcr.h
> > @@ -20,14 +20,47 @@
> >  #ifndef _ASM_POWERPC_DCR_H
> >  #define _ASM_POWERPC_DCR_H
> >  #ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> >  #ifdef CONFIG_PPC_DCR
> >
> >  #ifdef CONFIG_PPC_DCR_NATIVE
> >  #include <asm/dcr-native.h>
> > -#else
> > +#endif
> > +
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >  #include <asm/dcr-mmio.h>
> >  #endif
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +#include <asm/dcr-generic.h>
> > +
> > +#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_generic(host,
dcr_n, value)
> > +
> > +#else
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> > +typedef dcr_host_native_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n,
value)
> > +#else
> > +typedef dcr_host_mmio_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n,
value)
> > +#endif
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  /*
> >   * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
> >   * base from the device-tree
> > @@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct
device_node *np,
> >  #endif /* CONFIG_PPC_MERGE */
> >
> >  #endif /* CONFIG_PPC_DCR */
> > +#endif /* __ASSEMBLY__ */
> >  #endif /* __KERNEL__ */
> >  #endif /* _ASM_POWERPC_DCR_H */
>=20

RE: [PATCH 1/5] [v2][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Ben,

Any thoughts about device tree bindings for indirect DCR?

Steve

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Sunday, March 30, 2008 3:03 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/5] [v2][POWERPC] refactor dcr code
>=20
>=20
> On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer wrote:
> > Previously, dcr support was configured at compile time to either
using
> > MMIO or native dcr instructions.  Although this works for most
> > platforms, it fails on FPGA platforms:
> >
> > 1) Systems may include more than one dcr bus.
> > 2) Systems may be native dcr capable and still use memory mapped dcr
interface.
> >
> > This patch provides runtime support based on the device trees for
the
> > case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> > selected.  Previously, this was a poorly defined configuration,
which
> > happened to provide NATIVE support.  The runtime selection is made
> > based on the dcr slave device having a 'dcr-access-method' attribute
> > in the device tree.  If only one of the above options is selected,
> > then the code uses #defines to select only the used code in order to
> > avoid interoducing overhead in existing usage.
> >
> > Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com>
>=20
> Looks good. Haven't had a chance to test it yet, but tentatively
>=20
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>=20
> Initially, I thought about using function pointers but the if/else
will
> probably end up being more efficient. The only thing maybe to ponder
> is whether we could avoid the dcr-generic.h file completely, and have
> the generic wrappers be inline, though considering how slow DCRs are,
> it may not be that useful in practice and less clean...
>=20
> Cheers,
> Ben.
>=20
>=20
> > ---
> >  arch/powerpc/sysdev/dcr.c         |   91
++++++++++++++++++++++++++++++++-----
> >  include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
> >  include/asm-powerpc/dcr-mmio.h    |   20 +++++---
> >  include/asm-powerpc/dcr-native.h  |   16 ++++---
> >  include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
> >  5 files changed, 186 insertions(+), 26 deletions(-)
> >  create mode 100644 include/asm-powerpc/dcr-generic.h
> >
> > diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> > index 437e48d..d3de0ff 100644
> > --- a/arch/powerpc/sysdev/dcr.c
> > +++ b/arch/powerpc/sysdev/dcr.c
> > @@ -23,6 +23,68 @@
> >  #include <asm/prom.h>
> >  #include <asm/dcr.h>
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +bool dcr_map_ok_generic(dcr_host_t host)
> > +{
> > +	if (host.type =3D=3D INVALID)
> > +		return 0;
> > +	else if (host.type =3D=3D NATIVE)
> > +		return dcr_map_ok_native(host.host.native);
> > +	else
> > +		return dcr_map_ok_mmio(host.host.mmio);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
> > +
> > +dcr_host_t dcr_map_generic(struct device_node *dev,
> > +			   unsigned int dcr_n,
> > +			   unsigned int dcr_c)
> > +{
> > +	dcr_host_t host;
> > +	const char *prop =3D of_get_property(dev, "dcr-access-method",
NULL);
> > +
> > +	if (!strcmp(prop, "native")) {
> > +		host.type =3D NATIVE;
> > +		host.host.native =3D dcr_map_native(dev, dcr_n, dcr_c);
> > +	} else if (!strcmp(prop, "mmio")) {
> > +		host.type =3D MMIO;
> > +		host.host.mmio =3D dcr_map_mmio(dev, dcr_n, dcr_c);
> > +	} else
> > +		host.type =3D INVALID;
> > +
> > +	return host;
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_generic);
> > +
> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_unmap_native(host.host.native, dcr_c);
> > +	else
> > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_unmap_generic);
> > +
> > +u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		return dcr_read_native(host.host.native, dcr_n);
> > +	else
> > +		return dcr_read_mmio(host.host.mmio, dcr_n);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_read_generic);
> > +
> > +void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32
value)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_write_native(host.host.native, dcr_n, value);
> > +	else
> > +		dcr_write_mmio(host.host.mmio, dcr_n, value);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_write_generic);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  unsigned int dcr_resource_start(struct device_node *np, unsigned
int index)
> >  {
> >  	unsigned int ds;
> > @@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node
*np, unsigned int index)
> >  }
> >  EXPORT_SYMBOL_GPL(dcr_resource_len);
> >
> > -#ifndef CONFIG_PPC_DCR_NATIVE
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >
> >  static struct device_node * find_dcr_parent(struct device_node *
node)
> >  {
> > @@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct
device_node *dev,
> >  	return ret;
> >  }
> >
> > -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> > -		   unsigned int dcr_c)
> > +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +			     unsigned int dcr_n,
> > +			     unsigned int dcr_c)
> >  {
> > -	dcr_host_t ret =3D { .token =3D NULL, .stride =3D 0, .base =3D =
dcr_n };
> > +	dcr_host_mmio_t ret =3D { .token =3D NULL, .stride =3D 0, .base =
=3D
dcr_n };
> >  	u64 addr;
> >
> >  	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> >  		 dev->full_name, dcr_n, dcr_c);
> >
> >  	addr =3D of_translate_dcr_address(dev, dcr_n, &ret.stride);
> > -	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> > -		 addr, ret.stride);
> > +	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
> > +		 (unsigned long long) addr, ret.stride);
> >  	if (addr =3D=3D OF_BAD_ADDR)
> >  		return ret;
> >  	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
> > @@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev,
unsigned int dcr_n,
> >  	ret.token -=3D dcr_n * ret.stride;
> >  	return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_map);
> > +EXPORT_SYMBOL_GPL(dcr_map_mmio);
> >
> > -void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
> > +void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
> >  {
> > -	dcr_host_t h =3D host;
> > +	dcr_host_mmio_t h =3D host;
> >
> >  	if (h.token =3D=3D NULL)
> >  		return;
> > @@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int
dcr_c)
> >  	iounmap(h.token);
> >  	h.token =3D NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_unmap);
> > -#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> >  DEFINE_SPINLOCK(dcr_ind_lock);
> > -#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
> > +#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +
> > diff --git a/include/asm-powerpc/dcr-generic.h
b/include/asm-powerpc/dcr-generic.h
> > new file mode 100644
> > index 0000000..0ee74fb
> > --- /dev/null
> > +++ b/include/asm-powerpc/dcr-generic.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
> > + *                    <benh@kernel.crashing.org>
> > + *
> > + *   This program is free software;  you can redistribute it and/or
modify
> > + *   it under the terms of the GNU General Public License as
published by
> > + *   the Free Software Foundation; either version 2 of the License,
or
> > + *   (at your option) any later version.
> > + *
> > + *   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, write to the Free Software
> > + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
> > + */
> > +
> > +#ifndef _ASM_POWERPC_DCR_GENERIC_H
> > +#define _ASM_POWERPC_DCR_GENERIC_H
> > +#ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> > +
> > +enum host_type_t {MMIO, NATIVE, INVALID};
> > +
> > +typedef struct {
> > +	enum host_type_t type;
> > +	union {
> > +		dcr_host_mmio_t mmio;
> > +		dcr_host_native_t native;
> > +	} host;
> > +} dcr_host_t;
> > +
> > +extern bool dcr_map_ok_generic(dcr_host_t host);
> > +
> > +extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned
int dcr_n,
> > +			  unsigned int dcr_c);
> > +extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
> > +
> > +extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> > +
> > +extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n,
u32 value);
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __KERNEL__ */
> > +#endif /* _ASM_POWERPC_DCR_GENERIC_H */
> > +
> > +
> > diff --git a/include/asm-powerpc/dcr-mmio.h
b/include/asm-powerpc/dcr-mmio.h
> > index 08532ff..acd491d 100644
> > --- a/include/asm-powerpc/dcr-mmio.h
> > +++ b/include/asm-powerpc/dcr-mmio.h
> > @@ -27,20 +27,26 @@ typedef struct {
> >  	void __iomem *token;
> >  	unsigned int stride;
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_mmio_t;
> >
> > -#define DCR_MAP_OK(host)	((host).token !=3D NULL)
> > +static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
> > +{
> > +	return host.token !=3D NULL;
> > +}
> >
> > -extern dcr_host_t dcr_map(struct device_node *dev, unsigned int
dcr_n,
> > -			  unsigned int dcr_c);
> > -extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
> > +extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +				    unsigned int dcr_n,
> > +				    unsigned int dcr_c);
> > +extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int
dcr_c);
> >
> > -static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> > +static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int
dcr_n)
> >  {
> >  	return in_be32(host.token + ((host.base + dcr_n) *
host.stride));
> >  }
> >
> > -static inline void dcr_write(dcr_host_t host, unsigned int dcr_n,
u32 value)
> > +static inline void dcr_write_mmio(dcr_host_mmio_t host,
> > +				  unsigned int dcr_n,
> > +				  u32 value)
> >  {
> >  	out_be32(host.token + ((host.base + dcr_n) * host.stride),
value);
> >  }
> > diff --git a/include/asm-powerpc/dcr-native.h
b/include/asm-powerpc/dcr-native.h
> > index be6c879..67832e5 100644
> > --- a/include/asm-powerpc/dcr-native.h
> > +++ b/include/asm-powerpc/dcr-native.h
> > @@ -26,14 +26,18 @@
> >
> >  typedef struct {
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_native_t;
> >
> > -#define DCR_MAP_OK(host)	(1)
> > +static inline bool dcr_map_ok_native(dcr_host_native_t host)
> > +{
> > +	return 1;
> > +}
> >
> > -#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base =3D (dcr_n)
})
> > -#define dcr_unmap(host, dcr_c)		do {} while (0)
> > -#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
> > -#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base,
value)
> > +#define dcr_map_native(dev, dcr_n, dcr_c) \
> > +	((dcr_host_native_t){ .base =3D (dcr_n) })
> > +#define dcr_unmap_native(host, dcr_c)		do {} while (0)
> > +#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n +
host.base)
> > +#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n +
host.base, value)
> >
> >  /* Device Control Registers */
> >  void __mtdcr(int reg, unsigned int val);
> > diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> > index 9338d50..6b86322 100644
> > --- a/include/asm-powerpc/dcr.h
> > +++ b/include/asm-powerpc/dcr.h
> > @@ -20,14 +20,47 @@
> >  #ifndef _ASM_POWERPC_DCR_H
> >  #define _ASM_POWERPC_DCR_H
> >  #ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> >  #ifdef CONFIG_PPC_DCR
> >
> >  #ifdef CONFIG_PPC_DCR_NATIVE
> >  #include <asm/dcr-native.h>
> > -#else
> > +#endif
> > +
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >  #include <asm/dcr-mmio.h>
> >  #endif
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +#include <asm/dcr-generic.h>
> > +
> > +#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_generic(host,
dcr_n, value)
> > +
> > +#else
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> > +typedef dcr_host_native_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n,
value)
> > +#else
> > +typedef dcr_host_mmio_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n,
value)
> > +#endif
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  /*
> >   * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
> >   * base from the device-tree
> > @@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct
device_node *np,
> >  #endif /* CONFIG_PPC_MERGE */
> >
> >  #endif /* CONFIG_PPC_DCR */
> > +#endif /* __ASSEMBLY__ */
> >  #endif /* __KERNEL__ */
> >  #endif /* _ASM_POWERPC_DCR_H */
>=20

RE: [PATCH 1/5] [v2][POWERPC] refactor dcr code

[Benjamin Herrenschmidt]

On Fri, 2008-04-04 at 15:02 -0700, Stephen Neuendorffer wrote:
> Ben,
> 
> Any thoughts about device tree bindings for indirect DCR?

No, not really. For 44x PCI-E, I've just used something like an
"sdr-base" property to provide the base SDR for the port, simple but not
necessarily the nicest way to do so.

Ben.

RE: [PATCH 1/5] [v2][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Ben,

I'd like to get the first two patches tested and queued up for mainline
(although it's probably too late for this merge window, I suppose).  I'm
going to punt on the ppc patches and put off fixing the tft driver until
ARCH=3Dppc goes away.

I've fixed a typo in patch description: The dcr-access-method attribute
is on the dcr-controller, not the dcr slave.

Steve

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Sunday, March 30, 2008 3:03 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/5] [v2][POWERPC] refactor dcr code
>=20
>=20
> On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer wrote:
> > Previously, dcr support was configured at compile time to either
using
> > MMIO or native dcr instructions.  Although this works for most
> > platforms, it fails on FPGA platforms:
> >
> > 1) Systems may include more than one dcr bus.
> > 2) Systems may be native dcr capable and still use memory mapped dcr
interface.
> >
> > This patch provides runtime support based on the device trees for
the
> > case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> > selected.  Previously, this was a poorly defined configuration,
which
> > happened to provide NATIVE support.  The runtime selection is made
> > based on the dcr slave device having a 'dcr-access-method' attribute
> > in the device tree.  If only one of the above options is selected,
> > then the code uses #defines to select only the used code in order to
> > avoid interoducing overhead in existing usage.
> >
> > Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com>
>=20
> Looks good. Haven't had a chance to test it yet, but tentatively
>=20
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>=20
> Initially, I thought about using function pointers but the if/else
will
> probably end up being more efficient. The only thing maybe to ponder
> is whether we could avoid the dcr-generic.h file completely, and have
> the generic wrappers be inline, though considering how slow DCRs are,
> it may not be that useful in practice and less clean...
>=20
> Cheers,
> Ben.
>=20
>=20
> > ---
> >  arch/powerpc/sysdev/dcr.c         |   91
++++++++++++++++++++++++++++++++-----
> >  include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
> >  include/asm-powerpc/dcr-mmio.h    |   20 +++++---
> >  include/asm-powerpc/dcr-native.h  |   16 ++++---
> >  include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
> >  5 files changed, 186 insertions(+), 26 deletions(-)
> >  create mode 100644 include/asm-powerpc/dcr-generic.h
> >
> > diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> > index 437e48d..d3de0ff 100644
> > --- a/arch/powerpc/sysdev/dcr.c
> > +++ b/arch/powerpc/sysdev/dcr.c
> > @@ -23,6 +23,68 @@
> >  #include <asm/prom.h>
> >  #include <asm/dcr.h>
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +bool dcr_map_ok_generic(dcr_host_t host)
> > +{
> > +	if (host.type =3D=3D INVALID)
> > +		return 0;
> > +	else if (host.type =3D=3D NATIVE)
> > +		return dcr_map_ok_native(host.host.native);
> > +	else
> > +		return dcr_map_ok_mmio(host.host.mmio);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
> > +
> > +dcr_host_t dcr_map_generic(struct device_node *dev,
> > +			   unsigned int dcr_n,
> > +			   unsigned int dcr_c)
> > +{
> > +	dcr_host_t host;
> > +	const char *prop =3D of_get_property(dev, "dcr-access-method",
NULL);
> > +
> > +	if (!strcmp(prop, "native")) {
> > +		host.type =3D NATIVE;
> > +		host.host.native =3D dcr_map_native(dev, dcr_n, dcr_c);
> > +	} else if (!strcmp(prop, "mmio")) {
> > +		host.type =3D MMIO;
> > +		host.host.mmio =3D dcr_map_mmio(dev, dcr_n, dcr_c);
> > +	} else
> > +		host.type =3D INVALID;
> > +
> > +	return host;
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_generic);
> > +
> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_unmap_native(host.host.native, dcr_c);
> > +	else
> > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_unmap_generic);
> > +
> > +u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		return dcr_read_native(host.host.native, dcr_n);
> > +	else
> > +		return dcr_read_mmio(host.host.mmio, dcr_n);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_read_generic);
> > +
> > +void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32
value)
> > +{
> > +	if (host.type =3D=3D NATIVE)
> > +		dcr_write_native(host.host.native, dcr_n, value);
> > +	else
> > +		dcr_write_mmio(host.host.mmio, dcr_n, value);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_write_generic);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  unsigned int dcr_resource_start(struct device_node *np, unsigned
int index)
> >  {
> >  	unsigned int ds;
> > @@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node
*np, unsigned int index)
> >  }
> >  EXPORT_SYMBOL_GPL(dcr_resource_len);
> >
> > -#ifndef CONFIG_PPC_DCR_NATIVE
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >
> >  static struct device_node * find_dcr_parent(struct device_node *
node)
> >  {
> > @@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct
device_node *dev,
> >  	return ret;
> >  }
> >
> > -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> > -		   unsigned int dcr_c)
> > +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +			     unsigned int dcr_n,
> > +			     unsigned int dcr_c)
> >  {
> > -	dcr_host_t ret =3D { .token =3D NULL, .stride =3D 0, .base =3D =
dcr_n };
> > +	dcr_host_mmio_t ret =3D { .token =3D NULL, .stride =3D 0, .base =
=3D
dcr_n };
> >  	u64 addr;
> >
> >  	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> >  		 dev->full_name, dcr_n, dcr_c);
> >
> >  	addr =3D of_translate_dcr_address(dev, dcr_n, &ret.stride);
> > -	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> > -		 addr, ret.stride);
> > +	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
> > +		 (unsigned long long) addr, ret.stride);
> >  	if (addr =3D=3D OF_BAD_ADDR)
> >  		return ret;
> >  	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
> > @@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev,
unsigned int dcr_n,
> >  	ret.token -=3D dcr_n * ret.stride;
> >  	return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_map);
> > +EXPORT_SYMBOL_GPL(dcr_map_mmio);
> >
> > -void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
> > +void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
> >  {
> > -	dcr_host_t h =3D host;
> > +	dcr_host_mmio_t h =3D host;
> >
> >  	if (h.token =3D=3D NULL)
> >  		return;
> > @@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int
dcr_c)
> >  	iounmap(h.token);
> >  	h.token =3D NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(dcr_unmap);
> > -#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> >  DEFINE_SPINLOCK(dcr_ind_lock);
> > -#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
> > +#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
> > +
> > diff --git a/include/asm-powerpc/dcr-generic.h
b/include/asm-powerpc/dcr-generic.h
> > new file mode 100644
> > index 0000000..0ee74fb
> > --- /dev/null
> > +++ b/include/asm-powerpc/dcr-generic.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
> > + *                    <benh@kernel.crashing.org>
> > + *
> > + *   This program is free software;  you can redistribute it and/or
modify
> > + *   it under the terms of the GNU General Public License as
published by
> > + *   the Free Software Foundation; either version 2 of the License,
or
> > + *   (at your option) any later version.
> > + *
> > + *   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, write to the Free Software
> > + *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
> > + */
> > +
> > +#ifndef _ASM_POWERPC_DCR_GENERIC_H
> > +#define _ASM_POWERPC_DCR_GENERIC_H
> > +#ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> > +
> > +enum host_type_t {MMIO, NATIVE, INVALID};
> > +
> > +typedef struct {
> > +	enum host_type_t type;
> > +	union {
> > +		dcr_host_mmio_t mmio;
> > +		dcr_host_native_t native;
> > +	} host;
> > +} dcr_host_t;
> > +
> > +extern bool dcr_map_ok_generic(dcr_host_t host);
> > +
> > +extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned
int dcr_n,
> > +			  unsigned int dcr_c);
> > +extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
> > +
> > +extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> > +
> > +extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n,
u32 value);
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __KERNEL__ */
> > +#endif /* _ASM_POWERPC_DCR_GENERIC_H */
> > +
> > +
> > diff --git a/include/asm-powerpc/dcr-mmio.h
b/include/asm-powerpc/dcr-mmio.h
> > index 08532ff..acd491d 100644
> > --- a/include/asm-powerpc/dcr-mmio.h
> > +++ b/include/asm-powerpc/dcr-mmio.h
> > @@ -27,20 +27,26 @@ typedef struct {
> >  	void __iomem *token;
> >  	unsigned int stride;
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_mmio_t;
> >
> > -#define DCR_MAP_OK(host)	((host).token !=3D NULL)
> > +static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
> > +{
> > +	return host.token !=3D NULL;
> > +}
> >
> > -extern dcr_host_t dcr_map(struct device_node *dev, unsigned int
dcr_n,
> > -			  unsigned int dcr_c);
> > -extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
> > +extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > +				    unsigned int dcr_n,
> > +				    unsigned int dcr_c);
> > +extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int
dcr_c);
> >
> > -static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> > +static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int
dcr_n)
> >  {
> >  	return in_be32(host.token + ((host.base + dcr_n) *
host.stride));
> >  }
> >
> > -static inline void dcr_write(dcr_host_t host, unsigned int dcr_n,
u32 value)
> > +static inline void dcr_write_mmio(dcr_host_mmio_t host,
> > +				  unsigned int dcr_n,
> > +				  u32 value)
> >  {
> >  	out_be32(host.token + ((host.base + dcr_n) * host.stride),
value);
> >  }
> > diff --git a/include/asm-powerpc/dcr-native.h
b/include/asm-powerpc/dcr-native.h
> > index be6c879..67832e5 100644
> > --- a/include/asm-powerpc/dcr-native.h
> > +++ b/include/asm-powerpc/dcr-native.h
> > @@ -26,14 +26,18 @@
> >
> >  typedef struct {
> >  	unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_native_t;
> >
> > -#define DCR_MAP_OK(host)	(1)
> > +static inline bool dcr_map_ok_native(dcr_host_native_t host)
> > +{
> > +	return 1;
> > +}
> >
> > -#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base =3D (dcr_n)
})
> > -#define dcr_unmap(host, dcr_c)		do {} while (0)
> > -#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
> > -#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base,
value)
> > +#define dcr_map_native(dev, dcr_n, dcr_c) \
> > +	((dcr_host_native_t){ .base =3D (dcr_n) })
> > +#define dcr_unmap_native(host, dcr_c)		do {} while (0)
> > +#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n +
host.base)
> > +#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n +
host.base, value)
> >
> >  /* Device Control Registers */
> >  void __mtdcr(int reg, unsigned int val);
> > diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> > index 9338d50..6b86322 100644
> > --- a/include/asm-powerpc/dcr.h
> > +++ b/include/asm-powerpc/dcr.h
> > @@ -20,14 +20,47 @@
> >  #ifndef _ASM_POWERPC_DCR_H
> >  #define _ASM_POWERPC_DCR_H
> >  #ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> >  #ifdef CONFIG_PPC_DCR
> >
> >  #ifdef CONFIG_PPC_DCR_NATIVE
> >  #include <asm/dcr-native.h>
> > -#else
> > +#endif
> > +
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >  #include <asm/dcr-mmio.h>
> >  #endif
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +#include <asm/dcr-generic.h>
> > +
> > +#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_generic(host,
dcr_n, value)
> > +
> > +#else
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> > +typedef dcr_host_native_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n,
value)
> > +#else
> > +typedef dcr_host_mmio_t dcr_host_t;
> > +#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n,
value)
> > +#endif
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> >  /*
> >   * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
> >   * base from the device-tree
> > @@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct
device_node *np,
> >  #endif /* CONFIG_PPC_MERGE */
> >
> >  #endif /* CONFIG_PPC_DCR */
> > +#endif /* __ASSEMBLY__ */
> >  #endif /* __KERNEL__ */
> >  #endif /* _ASM_POWERPC_DCR_H */
>=20

[PATCH 1/2] [v3][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Previously, dcr support was configured at compile time to either using
MMIO or native dcr instructions.  Although this works for most
platforms, it fails on FPGA platforms:

1) Systems may include more than one dcr bus.
2) Systems may be native dcr capable and still use memory mapped dcr interface.

This patch provides runtime support based on the device trees for the
case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
selected.  Previously, this was a poorly defined configuration, which
happened to provide NATIVE support.  The runtime selection is made
based on the dcr controller having a 'dcr-access-method' attribute
in the device tree.  If only one of the above options is selected,
then the code uses #defines to select only the used code in order to
avoid introducing overhead in existing usage.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/dcr.c         |   91 ++++++++++++++++++++++++++++++++-----
 include/asm-powerpc/dcr-generic.h |   49 ++++++++++++++++++++
 include/asm-powerpc/dcr-mmio.h    |   20 +++++---
 include/asm-powerpc/dcr-native.h  |   16 ++++---
 include/asm-powerpc/dcr.h         |   36 ++++++++++++++-
 5 files changed, 186 insertions(+), 26 deletions(-)
 create mode 100644 include/asm-powerpc/dcr-generic.h

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index 437e48d..d3de0ff 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -23,6 +23,68 @@
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+bool dcr_map_ok_generic(dcr_host_t host)
+{
+	if (host.type == INVALID)
+		return 0;
+	else if (host.type == NATIVE)
+		return dcr_map_ok_native(host.host.native);
+	else
+		return dcr_map_ok_mmio(host.host.mmio);
+}
+EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
+
+dcr_host_t dcr_map_generic(struct device_node *dev,
+			   unsigned int dcr_n,
+			   unsigned int dcr_c)
+{
+	dcr_host_t host;
+	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
+
+	if (!strcmp(prop, "native")) {
+		host.type = NATIVE;
+		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
+	} else if (!strcmp(prop, "mmio")) {
+		host.type = MMIO;
+		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
+	} else
+		host.type = INVALID;
+
+	return host;
+}
+EXPORT_SYMBOL_GPL(dcr_map_generic);
+
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
+}
+EXPORT_SYMBOL_GPL(dcr_unmap_generic);
+
+u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
+{
+	if (host.type == NATIVE)
+		return dcr_read_native(host.host.native, dcr_n);
+	else
+		return dcr_read_mmio(host.host.mmio, dcr_n);
+}
+EXPORT_SYMBOL_GPL(dcr_read_generic);
+
+void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
+{
+	if (host.type == NATIVE)
+		dcr_write_native(host.host.native, dcr_n, value);
+	else
+		dcr_write_mmio(host.host.mmio, dcr_n, value);
+}
+EXPORT_SYMBOL_GPL(dcr_write_generic);
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
 	unsigned int ds;
@@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
-#ifndef CONFIG_PPC_DCR_NATIVE
+#ifdef CONFIG_PPC_DCR_MMIO
 
 static struct device_node * find_dcr_parent(struct device_node * node)
 {
@@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	return ret;
 }
 
-dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-		   unsigned int dcr_c)
+dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+			     unsigned int dcr_n,
+			     unsigned int dcr_c)
 {
-	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
+	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
 	u64 addr;
 
 	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
 		 dev->full_name, dcr_n, dcr_c);
 
 	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
-	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
-		 addr, ret.stride);
+	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
+		 (unsigned long long) addr, ret.stride);
 	if (addr == OF_BAD_ADDR)
 		return ret;
 	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
@@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 	ret.token -= dcr_n * ret.stride;
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dcr_map);
+EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
+void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
-	dcr_host_t h = host;
+	dcr_host_mmio_t h = host;
 
 	if (h.token == NULL)
 		return;
@@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 	iounmap(h.token);
 	h.token = NULL;
 }
-EXPORT_SYMBOL_GPL(dcr_unmap);
-#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
+EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
+
+#endif /* defined(CONFIG_PPC_DCR_MMIO) */
+
+#ifdef CONFIG_PPC_DCR_NATIVE
 DEFINE_SPINLOCK(dcr_ind_lock);
-#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
+#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
+
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
new file mode 100644
index 0000000..0ee74fb
--- /dev/null
+++ b/include/asm-powerpc/dcr-generic.h
@@ -0,0 +1,49 @@
+/*
+ * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
+ *                    <benh@kernel.crashing.org>
+ *
+ *   This program is free software;  you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   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, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_DCR_GENERIC_H
+#define _ASM_POWERPC_DCR_GENERIC_H
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+enum host_type_t {MMIO, NATIVE, INVALID};
+
+typedef struct {
+	enum host_type_t type;
+	union {
+		dcr_host_mmio_t mmio;
+		dcr_host_native_t native;
+	} host;
+} dcr_host_t;
+
+extern bool dcr_map_ok_generic(dcr_host_t host);
+
+extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
+			  unsigned int dcr_c);
+extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
+
+extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
+
+extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_DCR_GENERIC_H */
+
+
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 08532ff..acd491d 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -27,20 +27,26 @@ typedef struct {
 	void __iomem *token;
 	unsigned int stride;
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_mmio_t;
 
-#define DCR_MAP_OK(host)	((host).token != NULL)
+static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
+{
+	return host.token != NULL;
+}
 
-extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-			  unsigned int dcr_c);
-extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
+extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+				    unsigned int dcr_n,
+				    unsigned int dcr_c);
+extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
-static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
+static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
 {
 	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
 }
 
-static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
+static inline void dcr_write_mmio(dcr_host_mmio_t host,
+				  unsigned int dcr_n,
+				  u32 value)
 {
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index f8398ce..72d2b72 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -26,14 +26,18 @@
 
 typedef struct {
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_native_t;
 
-#define DCR_MAP_OK(host)	(1)
+static inline bool dcr_map_ok_native(dcr_host_native_t host)
+{
+	return 1;
+}
 
-#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_c)		do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
+#define dcr_map_native(dev, dcr_n, dcr_c) \
+	((dcr_host_native_t){ .base = (dcr_n) })
+#define dcr_unmap_native(host, dcr_c)		do {} while (0)
+#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 9338d50..6b86322 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -20,14 +20,47 @@
 #ifndef _ASM_POWERPC_DCR_H
 #define _ASM_POWERPC_DCR_H
 #ifdef __KERNEL__
+#ifndef __ASSEMBLY__
 #ifdef CONFIG_PPC_DCR
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 #include <asm/dcr-native.h>
-#else
+#endif
+
+#ifdef CONFIG_PPC_DCR_MMIO
 #include <asm/dcr-mmio.h>
 #endif
 
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+#include <asm/dcr-generic.h>
+
+#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
+
+#else
+
+#ifdef CONFIG_PPC_DCR_NATIVE
+typedef dcr_host_native_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
+#else
+typedef dcr_host_mmio_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
+#endif
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 /*
  * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
  * base from the device-tree
@@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
 #endif /* CONFIG_PPC_MERGE */
 
 #endif /* CONFIG_PPC_DCR */
+#endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_H */
-- 
1.5.3.4-dirty

[PATCH 2/2] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Stephen Neuendorffer]

FPGA designs may have need of both MMIO-based and NATIVE-based dcr
interfaces.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index a9260e2..b8e06df 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -123,6 +123,8 @@ config 405GPR
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_MMIO
+	select PPC_DCR_NATIVE
 
 config XILINX_VIRTEX_II_PRO
 	bool
-- 
1.5.3.4-dirty

Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code

[Grant Likely]

On Fri, Apr 18, 2008 at 3:55 PM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
> Previously, dcr support was configured at compile time to either using
>  MMIO or native dcr instructions.  Although this works for most
>  platforms, it fails on FPGA platforms:
>
>  1) Systems may include more than one dcr bus.
>  2) Systems may be native dcr capable and still use memory mapped dcr interface.
>
>  This patch provides runtime support based on the device trees for the
>  case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
>  selected.  Previously, this was a poorly defined configuration, which
>  happened to provide NATIVE support.  The runtime selection is made
>  based on the dcr controller having a 'dcr-access-method' attribute
>
> in the device tree.  If only one of the above options is selected,
>  then the code uses #defines to select only the used code in order to
>  avoid introducing overhead in existing usage.

I'm not really qualified to ack this patch, but it looks right to me;
except for one comment below (sorry I didn't get to looking at this
earlier).


>  diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
>  index 9338d50..6b86322 100644
>  --- a/include/asm-powerpc/dcr.h
>  +++ b/include/asm-powerpc/dcr.h
>  @@ -20,14 +20,47 @@
>   #ifndef _ASM_POWERPC_DCR_H
>   #define _ASM_POWERPC_DCR_H
>   #ifdef __KERNEL__
>  +#ifndef __ASSEMBLY__
>   #ifdef CONFIG_PPC_DCR
>
>   #ifdef CONFIG_PPC_DCR_NATIVE
>   #include <asm/dcr-native.h>
>  -#else
>  +#endif
>  +
>  +#ifdef CONFIG_PPC_DCR_MMIO
>   #include <asm/dcr-mmio.h>
>   #endif
>
>  +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
>  +
>  +#include <asm/dcr-generic.h>
>  +
>  +#define DCR_MAP_OK(host)       dcr_map_ok_generic(host)
>  +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
>  +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
>  +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
>  +#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
>  +
>  +#else
>  +
>  +#ifdef CONFIG_PPC_DCR_NATIVE
>  +typedef dcr_host_native_t dcr_host_t;
>  +#define DCR_MAP_OK(host)       dcr_map_ok_native(host)
>  +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
>  +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
>  +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
>  +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
>  +#else
>  +typedef dcr_host_mmio_t dcr_host_t;
>  +#define DCR_MAP_OK(host)       dcr_map_ok_mmio(host)
>  +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
>  +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
>  +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
>  +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
>  +#endif
>  +
>  +#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
>  +

By current conventions; these should probably be static functions (but
don't make them inline).  The compiler will do the right thing with
them.  Functions are easier to validate by the compiler and sparse
than #defines.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 2/2] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Grant Likely]

On Fri, Apr 18, 2008 at 3:55 PM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
> FPGA designs may have need of both MMIO-based and NATIVE-based dcr
>  interfaces.
>
>  Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
>  ---
>   arch/powerpc/platforms/40x/Kconfig |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
>  diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
>  index a9260e2..b8e06df 100644
>  --- a/arch/powerpc/platforms/40x/Kconfig
>  +++ b/arch/powerpc/platforms/40x/Kconfig
>  @@ -123,6 +123,8 @@ config 405GPR
>
>   config XILINX_VIRTEX
>         bool
>  +       select PPC_DCR_MMIO
>  +       select PPC_DCR_NATIVE

Yeah; we'll probably want to be more sophisticated about this in the
future; but for now...

Acked-by: Grant Likely <grant.likely@secretlab.ca>

(Paulus; but this does depend on patch 1 of this series)

>
>   config XILINX_VIRTEX_II_PRO
>         bool
>  --
>  1.5.3.4-dirty
>
>
>
>  _______________________________________________
>  Linuxppc-dev mailing list
>  Linuxppc-dev@ozlabs.org
>  https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code

[Benjamin Herrenschmidt]

> By current conventions; these should probably be static functions (but
> don't make them inline).  The compiler will do the right thing with
> them.  Functions are easier to validate by the compiler and sparse
> than #defines.

Not necessarily... yes we tend to prefer functions, but in that case
which is 100% a renaming trick, macros are fine and somewhat simpler.

There is no type difference and the macro will not prevent type checking
here.

Cheers,
Ben.

Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code

[Josh Boyer]

On Fri, 18 Apr 2008 14:55:03 -0700
Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:

> Previously, dcr support was configured at compile time to either using
> MMIO or native dcr instructions.  Although this works for most
> platforms, it fails on FPGA platforms:
> 
> 1) Systems may include more than one dcr bus.
> 2) Systems may be native dcr capable and still use memory mapped dcr interface.
> 
> This patch provides runtime support based on the device trees for the
> case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> selected.  Previously, this was a poorly defined configuration, which
> happened to provide NATIVE support.  The runtime selection is made
> based on the dcr controller having a 'dcr-access-method' attribute
> in the device tree.  If only one of the above options is selected,
> then the code uses #defines to select only the used code in order to
> avoid introducing overhead in existing usage.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Hi Stephen,

Sorry for the late review.  See some comments below.  Mostly minor
stuff and I think the general direction here is good.

> diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
> index 437e48d..d3de0ff 100644
> --- a/arch/powerpc/sysdev/dcr.c
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -23,6 +23,68 @@
>  #include <asm/prom.h>
>  #include <asm/dcr.h>
> 
> +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> +
> +bool dcr_map_ok_generic(dcr_host_t host)
> +{
> +	if (host.type == INVALID)
> +		return 0;
> +	else if (host.type == NATIVE)
> +		return dcr_map_ok_native(host.host.native);
> +	else
> +		return dcr_map_ok_mmio(host.host.mmio);
> +}
> +EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
> +
> +dcr_host_t dcr_map_generic(struct device_node *dev,
> +			   unsigned int dcr_n,
> +			   unsigned int dcr_c)
> +{
> +	dcr_host_t host;
> +	const char *prop = of_get_property(dev, "dcr-access-method", NULL);
> +
> +	if (!strcmp(prop, "native")) {
> +		host.type = NATIVE;
> +		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
> +	} else if (!strcmp(prop, "mmio")) {
> +		host.type = MMIO;
> +		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
> +	} else
> +		host.type = INVALID;
> +
> +	return host;
> +}
> +EXPORT_SYMBOL_GPL(dcr_map_generic);
> +
> +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> +{
> +	if (host.type == NATIVE)
> +		dcr_unmap_native(host.host.native, dcr_c);
> +	else
> +		dcr_unmap_mmio(host.host.mmio, dcr_c);

What happens if host.type == INVALID?  Same question for the other
accessors in dcr_*_generic.

<snip>

> diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
> new file mode 100644
> index 0000000..0ee74fb
> --- /dev/null
> +++ b/include/asm-powerpc/dcr-generic.h
> @@ -0,0 +1,49 @@

<snip> 

> +enum host_type_t {MMIO, NATIVE, INVALID};

Should these be DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID?

I worry about the generic nature of the names.

josh

RE: [PATCH 1/2] [v3][POWERPC] refactor dcr code

[Stephen Neuendorffer]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]



> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > +	if (host.type == NATIVE)
> > +		dcr_unmap_native(host.host.native, dcr_c);
> > +	else
> > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
>
> What happens if host.type == INVALID?  Same question for the other
> accessors in dcr_*_generic.

I guess looking back on it, I assumed that MAP_OK would return 0, meaning that behavior was undefined,
but I agree it's probably safer to have some error reporting there...  There starts to become a speed tradeoff
at some point, which would make function pointers more attractive.  If the ioremap does fail, or the
dcr-access-method can't be determined, then dcr_unmap_mmio would probably SEGV anyway, although that's
not something I'd really want to rely on.  I'll put an error case in there.

> > +enum host_type_t {MMIO, NATIVE, INVALID};
>
> Should these be DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID?
>
> I worry about the generic nature of the names.

Also seems reasonable, 

Steve

[-- Attachment #2: Type: text/html, Size: 1748 bytes --]

Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code

[Josh Boyer]

On Sun, 20 Apr 2008 20:56:20 -0700
"Stephen Neuendorffer" <stephen.neuendorffer@xilinx.com> wrote:

> 
> 
> > > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > > +{
> > > +	if (host.type == NATIVE)
> > > +		dcr_unmap_native(host.host.native, dcr_c);
> > > +	else
> > > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> >
> > What happens if host.type == INVALID?  Same question for the other
> > accessors in dcr_*_generic.
> 
> I guess looking back on it, I assumed that MAP_OK would return 0, meaning that behavior was undefined,
> but I agree it's probably safer to have some error reporting there...  There starts to become a speed tradeoff
> at some point, which would make function pointers more attractive.  If the ioremap does fail, or the
> dcr-access-method can't be determined, then dcr_unmap_mmio would probably SEGV anyway, although that's
> not something I'd really want to rely on.  I'll put an error case in there.

Well, MAP_OK would return 0, and the caller of the original dcr_map
should probably return an error or something.  But there's nothing to
enforce that.  Which is true for the current code today as well, so
it's not something you've introduced.

I just thought that if you were going to go to the trouble of defining
an invalid host type, that you'd check for it in the accessor
functions.  There is the concern of the speed tradeoffs.  I suppose you
could just omit INVALID altogether and have it match the existing code
in behavior if it was too big of a deal.

Ben, any thoughts?

josh

[PATCH 0/4] [v4][POWERPC] refactor dcr code

[Stephen Neuendorffer]

I've modified these patches to include Josh's comments, and also
modified the tft patches to not assume that patches to ARCH=ppc are
made.  This breaks platform_device support and hence ARCH=ppc support.
I'd appreciate it if these patches could get queued up for 2.6.27 (and
hence get built a bit before then), when ARCH=ppc is going away
anyway.

Steve

[PATCH 1/4] [v4][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Previously, dcr support was configured at compile time to either using
MMIO or native dcr instructions.  Although this works for most
platforms, it fails on FPGA platforms:

1) Systems may include more than one dcr bus.
2) Systems may be native dcr capable and still use memory mapped dcr interface.

This patch provides runtime support based on the device trees for the
case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
selected.  Previously, this was a poorly defined configuration, which
happened to provide NATIVE support.  The runtime selection is made
based on the dcr controller having a 'dcr-access-method' attribute
in the device tree.  If only one of the above options is selected,
then the code uses #defines to select only the used code in order to
avoid introducing overhead in existing usage.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

--

[v4]
Converted flags to be DCR_HOST_*
Added WARN_ON in case someone forgets to call MAP_OK()
Fixed dealing with the dcr-access-method correctly, which was still
	off in the wrong patch.
---
 arch/powerpc/sysdev/dcr.c         |  146 +++++++++++++++++++++++++++++--------
 include/asm-powerpc/dcr-generic.h |   49 ++++++++++++
 include/asm-powerpc/dcr-mmio.h    |   20 +++--
 include/asm-powerpc/dcr-native.h  |   16 +++--
 include/asm-powerpc/dcr.h         |   39 ++++++++++-
 5 files changed, 225 insertions(+), 45 deletions(-)
 create mode 100644 include/asm-powerpc/dcr-generic.h

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index 437e48d..6984424 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -23,6 +23,104 @@
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+static struct device_node *find_dcr_parent(struct device_node *node)
+{
+	struct device_node *par, *tmp;
+	const u32 *p;
+
+	for (par = of_node_get(node); par;) {
+		if (of_get_property(par, "dcr-controller", NULL))
+			break;
+		p = of_get_property(par, "dcr-parent", NULL);
+		tmp = par;
+		if (p == NULL)
+			par = of_get_parent(par);
+		else
+			par = of_find_node_by_phandle(*p);
+		of_node_put(tmp);
+	}
+	return par;
+}
+
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+bool dcr_map_ok_generic(dcr_host_t host)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		return dcr_map_ok_native(host.host.native);
+	else if (host.type == DCR_HOST_MMIO)
+		return dcr_map_ok_mmio(host.host.mmio);
+	else
+		return 0;
+}
+EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
+
+dcr_host_t dcr_map_generic(struct device_node *dev,
+			   unsigned int dcr_n,
+			   unsigned int dcr_c)
+{
+	dcr_host_t host;
+	struct device_node *dp;
+	const char *prop;
+
+	host.type = DCR_HOST_INVALID;
+
+	dp = find_dcr_parent(dev);
+	if (dp == NULL)
+		return host;
+
+	prop = of_get_property(dp, "dcr-access-method", NULL);
+
+	pr_debug("dcr_map_generic(dcr-access-method = %s)\n", prop);
+
+	if (!strcmp(prop, "native")) {
+		host.type = DCR_HOST_NATIVE;
+		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
+	} else if (!strcmp(prop, "mmio")) {
+		host.type = DCR_HOST_MMIO;
+		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
+	}
+
+	return host;
+}
+EXPORT_SYMBOL_GPL(dcr_map_generic);
+
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else if (host.type == DCR_HOST_MMIO)
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+}
+EXPORT_SYMBOL_GPL(dcr_unmap_generic);
+
+u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		return dcr_read_native(host.host.native, dcr_n);
+	else if (host.type == DCR_HOST_MMIO)
+		return dcr_read_mmio(host.host.mmio, dcr_n);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dcr_read_generic);
+
+void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		dcr_write_native(host.host.native, dcr_n, value);
+	else if (host.type == DCR_HOST_MMIO)
+		dcr_write_mmio(host.host.mmio, dcr_n, value);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+}
+EXPORT_SYMBOL_GPL(dcr_write_generic);
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
 	unsigned int ds;
@@ -47,26 +145,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
-#ifndef CONFIG_PPC_DCR_NATIVE
-
-static struct device_node * find_dcr_parent(struct device_node * node)
-{
-	struct device_node *par, *tmp;
-	const u32 *p;
-
-	for (par = of_node_get(node); par;) {
-		if (of_get_property(par, "dcr-controller", NULL))
-			break;
-		p = of_get_property(par, "dcr-parent", NULL);
-		tmp = par;
-		if (p == NULL)
-			par = of_get_parent(par);
-		else
-			par = of_find_node_by_phandle(*p);
-		of_node_put(tmp);
-	}
-	return par;
-}
+#ifdef CONFIG_PPC_DCR_MMIO
 
 u64 of_translate_dcr_address(struct device_node *dev,
 			     unsigned int dcr_n,
@@ -101,18 +180,19 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	return ret;
 }
 
-dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-		   unsigned int dcr_c)
+dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+			     unsigned int dcr_n,
+			     unsigned int dcr_c)
 {
-	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
+	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
 	u64 addr;
 
 	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
 		 dev->full_name, dcr_n, dcr_c);
 
 	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
-	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
-		 addr, ret.stride);
+	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
+		 (unsigned long long) addr, ret.stride);
 	if (addr == OF_BAD_ADDR)
 		return ret;
 	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
@@ -124,11 +204,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 	ret.token -= dcr_n * ret.stride;
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dcr_map);
+EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
+void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
-	dcr_host_t h = host;
+	dcr_host_mmio_t h = host;
 
 	if (h.token == NULL)
 		return;
@@ -136,7 +216,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 	iounmap(h.token);
 	h.token = NULL;
 }
-EXPORT_SYMBOL_GPL(dcr_unmap);
-#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
+EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
+
+#endif /* defined(CONFIG_PPC_DCR_MMIO) */
+
+#ifdef CONFIG_PPC_DCR_NATIVE
 DEFINE_SPINLOCK(dcr_ind_lock);
-#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
+#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
+
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
new file mode 100644
index 0000000..35b7159
--- /dev/null
+++ b/include/asm-powerpc/dcr-generic.h
@@ -0,0 +1,49 @@
+/*
+ * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
+ *                    <benh@kernel.crashing.org>
+ *
+ *   This program is free software;  you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   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, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_DCR_GENERIC_H
+#define _ASM_POWERPC_DCR_GENERIC_H
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+enum host_type_t {DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID};
+
+typedef struct {
+	enum host_type_t type;
+	union {
+		dcr_host_mmio_t mmio;
+		dcr_host_native_t native;
+	} host;
+} dcr_host_t;
+
+extern bool dcr_map_ok_generic(dcr_host_t host);
+
+extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
+			  unsigned int dcr_c);
+extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
+
+extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
+
+extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_DCR_GENERIC_H */
+
+
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 08532ff..acd491d 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -27,20 +27,26 @@ typedef struct {
 	void __iomem *token;
 	unsigned int stride;
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_mmio_t;
 
-#define DCR_MAP_OK(host)	((host).token != NULL)
+static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
+{
+	return host.token != NULL;
+}
 
-extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-			  unsigned int dcr_c);
-extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
+extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+				    unsigned int dcr_n,
+				    unsigned int dcr_c);
+extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
-static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
+static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
 {
 	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
 }
 
-static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
+static inline void dcr_write_mmio(dcr_host_mmio_t host,
+				  unsigned int dcr_n,
+				  u32 value)
 {
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index f8398ce..72d2b72 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -26,14 +26,18 @@
 
 typedef struct {
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_native_t;
 
-#define DCR_MAP_OK(host)	(1)
+static inline bool dcr_map_ok_native(dcr_host_native_t host)
+{
+	return 1;
+}
 
-#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_c)		do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
+#define dcr_map_native(dev, dcr_n, dcr_c) \
+	((dcr_host_native_t){ .base = (dcr_n) })
+#define dcr_unmap_native(host, dcr_c)		do {} while (0)
+#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 9338d50..53b2830 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -20,14 +20,50 @@
 #ifndef _ASM_POWERPC_DCR_H
 #define _ASM_POWERPC_DCR_H
 #ifdef __KERNEL__
+#ifndef __ASSEMBLY__
 #ifdef CONFIG_PPC_DCR
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 #include <asm/dcr-native.h>
-#else
+#endif
+
+#ifdef CONFIG_PPC_DCR_MMIO
 #include <asm/dcr-mmio.h>
 #endif
 
+
+/* Indirection layer for providing both NATIVE and MMIO support. */
+
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+#include <asm/dcr-generic.h>
+
+#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
+
+#else
+
+#ifdef CONFIG_PPC_DCR_NATIVE
+typedef dcr_host_native_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
+#else
+typedef dcr_host_mmio_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
+#endif
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 /*
  * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
  * base from the device-tree
@@ -41,5 +77,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
 #endif /* CONFIG_PPC_MERGE */
 
 #endif /* CONFIG_PPC_DCR */
+#endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_H */
-- 
1.5.3.4-dirty

[PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Stephen Neuendorffer]

FPGA designs may have need of both MMIO-based and NATIVE-based dcr
interfaces.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index a9260e2..b8e06df 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -123,6 +123,8 @@ config 405GPR
 
 config XILINX_VIRTEX
 	bool
+	select PPC_DCR_MMIO
+	select PPC_DCR_NATIVE
 
 config XILINX_VIRTEX_II_PRO
 	bool
-- 
1.5.3.4-dirty

[PATCH 3/4] [POWERPC] Xilinx: Framebuffer: remove platform device support.

[Stephen Neuendorffer]

With ARCH=ppc going away, we don't really need platform device support
anymore.  In fact it is hard, if we want to use the generic dcr
infrastructure.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 drivers/video/xilinxfb.c |   63 +---------------------------------------------
 1 files changed, 1 insertions(+), 62 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 7b3a842..4d64402 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -31,7 +31,6 @@
 #include <linux/fb.h>
 #include <linux/init.h>
 #include <linux/dma-mapping.h>
-#include <linux/platform_device.h>
 #if defined(CONFIG_OF)
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
@@ -356,56 +355,6 @@ static int xilinxfb_release(struct device *dev)
 }
 
 /* ---------------------------------------------------------------------
- * Platform bus binding
- */
-
-static int
-xilinxfb_platform_probe(struct platform_device *pdev)
-{
-	struct xilinxfb_platform_data *pdata;
-	struct resource *res;
-
-	/* Find the registers address */
-	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "Couldn't get registers resource\n");
-		return -ENODEV;
-	}
-
-	/* If a pdata structure is provided, then extract the parameters */
-	pdata = &xilinx_fb_default_pdata;
-	if (pdev->dev.platform_data) {
-		pdata = pdev->dev.platform_data;
-		if (!pdata->xres)
-			pdata->xres = xilinx_fb_default_pdata.xres;
-		if (!pdata->yres)
-			pdata->yres = xilinx_fb_default_pdata.yres;
-		if (!pdata->xvirt)
-			pdata->xvirt = xilinx_fb_default_pdata.xvirt;
-		if (!pdata->yvirt)
-			pdata->yvirt = xilinx_fb_default_pdata.yvirt;
-	}
-
-	return xilinxfb_assign(&pdev->dev, res->start, pdata);
-}
-
-static int
-xilinxfb_platform_remove(struct platform_device *pdev)
-{
-	return xilinxfb_release(&pdev->dev);
-}
-
-
-static struct platform_driver xilinxfb_platform_driver = {
-	.probe		= xilinxfb_platform_probe,
-	.remove		= xilinxfb_platform_remove,
-	.driver = {
-		.owner = THIS_MODULE,
-		.name = DRIVER_NAME,
-	},
-};
-
-/* ---------------------------------------------------------------------
  * OF bus binding
  */
 
@@ -500,22 +449,12 @@ static inline void __exit xilinxfb_of_unregister(void) { }
 static int __init
 xilinxfb_init(void)
 {
-	int rc;
-	rc = xilinxfb_of_register();
-	if (rc)
-		return rc;
-
-	rc = platform_driver_register(&xilinxfb_platform_driver);
-	if (rc)
-		xilinxfb_of_unregister();
-
-	return rc;
+	return xilinxfb_of_register();
 }
 
 static void __exit
 xilinxfb_cleanup(void)
 {
-	platform_driver_unregister(&xilinxfb_platform_driver);
 	xilinxfb_of_unregister();
 }
 
-- 
1.5.3.4-dirty

[PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[Stephen Neuendorffer]

This device contains a dcr interface.  Previously, the dcr interface
was assumed to be used in mmio mode, and the register space of the dcr
interface was precomputed and stuffed in the device tree.  This patch
makes use of the new dcr infrastructure to represent the dcr interface
as any other dcr interface in the device tree.  This enables the dcr
interface to be connected directly to a native dcr interface in a
clean way.

In particular, the device tree expected looks like:

			dcr_v29_0: dcr@0 {
				#address-cells = <1>;
				#size-cells = <1>;
				compatible = "xlnx,dcr-v29-1.00.a";
				VGA_FrameBuffer: tft@80 {
					compatible = "xlnx,plb-tft-cntlr-ref-1.00.a";
					dcr-parent = <&opb2dcr_bridge_0>;
					dcr-reg = < 80 2 >;
					xlnx,default-tft-base-addr = <7f>;
					xlnx,dps-init = <1>;
					xlnx,on-init = <1>;
					xlnx,pixclk-is-busclk-divby4 = <1>;
				} ;
			} ;

			opb2dcr_bridge_0: opb2dcr-bridge@40700000 {
				compatible = "xlnx,opb2dcr-bridge-1.00.b";
				dcr-access-method = "mmio";
				dcr-controller ;
				dcr-mmio-range = < 40700000 1000 >;
				dcr-mmio-stride = <4>;
				reg = < 40700000 1000 >;
				xlnx,family = "virtex2p";
			} ;

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 drivers/video/xilinxfb.c |   54 +++++++++++++++++-----------------------------
 1 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 4d64402..848752e 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -37,6 +37,7 @@
 #endif
 #include <asm/io.h>
 #include <linux/xilinxfb.h>
+#include <asm/dcr.h>
 
 #define DRIVER_NAME		"xilinxfb"
 #define DRIVER_DESCRIPTION	"Xilinx TFT LCD frame buffer driver"
@@ -111,8 +112,9 @@ struct xilinxfb_drvdata {
 
 	struct fb_info	info;		/* FB driver info record */
 
-	u32		regs_phys;	/* phys. address of the control registers */
-	u32 __iomem	*regs;		/* virt. address of the control registers */
+	dcr_host_t      dcr_host;
+	unsigned int    dcr_start;
+	unsigned int    dcr_len;
 
 	void		*fb_virt;	/* virt. address of the frame buffer */
 	dma_addr_t	fb_phys;	/* phys. address of the frame buffer */
@@ -135,7 +137,7 @@ struct xilinxfb_drvdata {
  * when it's needed.
  */
 #define xilinx_fb_out_be32(driverdata, offset, val) \
-	out_be32(driverdata->regs + offset, val)
+	dcr_write(driverdata->dcr_host, offset, val)
 
 static int
 xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
@@ -203,7 +205,8 @@ static struct fb_ops xilinxfb_ops =
  * Bus independent setup/teardown
  */
 
-static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
+static int xilinxfb_assign(struct device *dev, dcr_host_t dcr_host,
+			   unsigned int dcr_start, unsigned int dcr_len,
 			   struct xilinxfb_platform_data *pdata)
 {
 	struct xilinxfb_drvdata *drvdata;
@@ -218,21 +221,9 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 	dev_set_drvdata(dev, drvdata);
 
-	/* Map the control registers in */
-	if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_region;
-	}
-	drvdata->regs_phys = physaddr;
-	drvdata->regs = ioremap(physaddr, 8);
-	if (!drvdata->regs) {
-		dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
-			physaddr);
-		rc = -ENODEV;
-		goto err_map;
-	}
+	drvdata->dcr_start = dcr_start;
+	drvdata->dcr_len = dcr_len;
+	drvdata->dcr_host = dcr_host;
 
 	/* Allocate the framebuffer memory */
 	if (pdata->fb_phys) {
@@ -247,7 +238,7 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	if (!drvdata->fb_virt) {
 		dev_err(dev, "Could not allocate frame buffer memory\n");
 		rc = -ENOMEM;
-		goto err_fbmem;
+		goto err_region;
 	}
 
 	/* Clear (turn to black) the framebuffer */
@@ -297,7 +288,6 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
 	}
 
 	/* Put a banner in the log (for DEBUG) */
-	dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr, drvdata->regs);
 	dev_dbg(dev, "fb: phys=%p, virt=%p, size=%x\n",
 		(void*)drvdata->fb_phys, drvdata->fb_virt, fbsize);
 
@@ -313,12 +303,6 @@ err_cmap:
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
 
-err_fbmem:
-	iounmap(drvdata->regs);
-
-err_map:
-	release_mem_region(physaddr, 8);
-
 err_region:
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -344,9 +328,8 @@ static int xilinxfb_release(struct device *dev)
 
 	/* Turn off the display */
 	xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
-	iounmap(drvdata->regs);
 
-	release_mem_region(drvdata->regs_phys, 8);
+	dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
 
 	kfree(drvdata);
 	dev_set_drvdata(dev, NULL);
@@ -362,20 +345,23 @@ static int xilinxfb_release(struct device *dev)
 static int __devinit
 xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 {
-	struct resource res;
 	const u32 *prop;
 	struct xilinxfb_platform_data pdata;
 	int size, rc;
+	int start, len;
+	dcr_host_t dcr_host;
 
 	/* Copy with the default pdata (not a ptr reference!) */
 	pdata = xilinx_fb_default_pdata;
 
 	dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
 
-	rc = of_address_to_resource(op->node, 0, &res);
-	if (rc) {
+	start = dcr_resource_start(op->node, 0);
+	len = dcr_resource_len(op->node, 0);
+	dcr_host = dcr_map(op->node, start, len);
+	if (!DCR_MAP_OK(dcr_host)) {
 		dev_err(&op->dev, "invalid address\n");
-		return rc;
+		return -ENODEV;
 	}
 
 	prop = of_get_property(op->node, "phys-size", &size);
@@ -399,7 +385,7 @@ xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
 	if (of_find_property(op->node, "rotate-display", NULL))
 		pdata.rotate_screen = 1;
 
-	return xilinxfb_assign(&op->dev, res.start, &pdata);
+	return xilinxfb_assign(&op->dev, dcr_host, start, len, &pdata);
 }
 
 static int __devexit xilinxfb_of_remove(struct of_device *op)
-- 
1.5.3.4-dirty

Re: [PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE

[Benjamin Herrenschmidt]

On Mon, 2008-05-05 at 10:56 -0700, Stephen Neuendorffer wrote:
> FPGA designs may have need of both MMIO-based and NATIVE-based dcr
> interfaces.

You say _may_ ... wouldn't it be better if it was thus left to a given
virtex based platform to enable DCR_MMIO if it uses it and leave only
NATIVE by default ?

> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> ---
>  arch/powerpc/platforms/40x/Kconfig |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> index a9260e2..b8e06df 100644
> --- a/arch/powerpc/platforms/40x/Kconfig
> +++ b/arch/powerpc/platforms/40x/Kconfig
> @@ -123,6 +123,8 @@ config 405GPR
>  
>  config XILINX_VIRTEX
>  	bool
> +	select PPC_DCR_MMIO
> +	select PPC_DCR_NATIVE
>  
>  config XILINX_VIRTEX_II_PRO
>  	bool

Re: [PATCH 1/4] [v4][POWERPC] refactor dcr code

[Benjamin Herrenschmidt]

On Mon, 2008-05-05 at 10:56 -0700, Stephen Neuendorffer wrote:
> Previously, dcr support was configured at compile time to either using
> MMIO or native dcr instructions.  Although this works for most
> platforms, it fails on FPGA platforms:
> 
> 1) Systems may include more than one dcr bus.
> 2) Systems may be native dcr capable and still use memory mapped dcr interface.
> 
> This patch provides runtime support based on the device trees for the
> case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
> selected.  Previously, this was a poorly defined configuration, which
> happened to provide NATIVE support.  The runtime selection is made
> based on the dcr controller having a 'dcr-access-method' attribute
> in the device tree.  If only one of the above options is selected,
> then the code uses #defines to select only the used code in order to
> avoid introducing overhead in existing usage.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Quick look seems ok, but that was really only a quick look :-)

I'll rely on Josh to review further.

Cheers,
Ben.

Re: [PATCH 1/4] [v4][POWERPC] refactor dcr code

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]

Hi Stephen,

On Mon,  5 May 2008 10:56:38 -0700 Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
> +static struct device_node *find_dcr_parent(struct device_node *node)
> +{
> +	struct device_node *par, *tmp;
> +	const u32 *p;
> +
> +	for (par = of_node_get(node); par;) {
> +		if (of_get_property(par, "dcr-controller", NULL))
> +			break;
> +		p = of_get_property(par, "dcr-parent", NULL);
> +		tmp = par;
> +		if (p == NULL)
> +			par = of_get_parent(par);
> +		else
> +			par = of_find_node_by_phandle(*p);
> +		of_node_put(tmp);
> +	}
> +	return par;
> +}

	.
	.

> +dcr_host_t dcr_map_generic(struct device_node *dev,
> +			   unsigned int dcr_n,
> +			   unsigned int dcr_c)
> +{
> +	dcr_host_t host;
> +	struct device_node *dp;
> +	const char *prop;
> +
> +	host.type = DCR_HOST_INVALID;
> +
> +	dp = find_dcr_parent(dev);
> +	if (dp == NULL)
> +		return host;
> +
> +	prop = of_get_property(dp, "dcr-access-method", NULL);
> +
> +	pr_debug("dcr_map_generic(dcr-access-method = %s)\n", prop);
> +
> +	if (!strcmp(prop, "native")) {
> +		host.type = DCR_HOST_NATIVE;
> +		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
> +	} else if (!strcmp(prop, "mmio")) {
> +		host.type = DCR_HOST_MMIO;
> +		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
> +	}
> +
> +	return host;

Since find_dcr_parent has done a of_node_get on its return value, you
leak a reference to dp here i.e. you need an of_node_put(dp) before you
return.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/4] [v4][POWERPC] refactor dcr code

[Benjamin Herrenschmidt]

On Tue, 2008-05-06 at 13:40 +1000, Stephen Rothwell wrote:
> Since find_dcr_parent has done a of_node_get on its return value, you
> leak a reference to dp here i.e. you need an of_node_put(dp) before
> you return.

He inherited that bug from other dcr.c functions I wrote, my fault.

Stephen (N. not R.), would you mind fixing them while at it or do you
want me to cook up a patch ?

Cheers,
Ben.

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[Grant Likely]

On Mon, May 5, 2008 at 11:56 AM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
> This device contains a dcr interface.  Previously, the dcr interface
>  was assumed to be used in mmio mode, and the register space of the dcr
>  interface was precomputed and stuffed in the device tree.  This patch
>  makes use of the new dcr infrastructure to represent the dcr interface
>  as any other dcr interface in the device tree.  This enables the dcr
>  interface to be connected directly to a native dcr interface in a
>  clean way.
>
>  In particular, the device tree expected looks like:
>
>                         dcr_v29_0: dcr@0 {
>                                 #address-cells = <1>;
>                                 #size-cells = <1>;
>                                 compatible = "xlnx,dcr-v29-1.00.a";
>                                 VGA_FrameBuffer: tft@80 {
>                                         compatible = "xlnx,plb-tft-cntlr-ref-1.00.a";
>                                         dcr-parent = <&opb2dcr_bridge_0>;
>                                         dcr-reg = < 80 2 >;
>                                         xlnx,default-tft-base-addr = <7f>;
>                                         xlnx,dps-init = <1>;
>                                         xlnx,on-init = <1>;
>                                         xlnx,pixclk-is-busclk-divby4 = <1>;
>                                 } ;
>                         } ;
>
>                         opb2dcr_bridge_0: opb2dcr-bridge@40700000 {
>                                 compatible = "xlnx,opb2dcr-bridge-1.00.b";
>                                 dcr-access-method = "mmio";
>                                 dcr-controller ;
>                                 dcr-mmio-range = < 40700000 1000 >;
>                                 dcr-mmio-stride = <4>;
>                                 reg = < 40700000 1000 >;
>                                 xlnx,family = "virtex2p";
>                         } ;

Hmmm, something doesn't quite feel right about this.  The node
describing the tft device is a child of the dcr@0 node which is the
dcr bus.  However, dcr bindings use dcr-bus and dcr-reg instead of
parent-child relationship to specify how to access the dcr registers.
So, in this example; if the device is described by tft@80, and the dcr
bus is described by opb2dcr-bridge@40700000, then what does dcr@0
describe?  (I do understand what they really describe in EDK terms;
but I'm looking at it through device tree glasses).

I don't think the presence of a dcr@0 node is a problem, but in this
case #size/address-cells doesn't have any meaning (the child doesn't
have a reg property) and it looks like it should be a child of the
opb2dcr-bridge node (otherwise, what is it attached to?).

Cheers,
g.

>
>  Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
>  ---
>   drivers/video/xilinxfb.c |   54 +++++++++++++++++-----------------------------
>   1 files changed, 20 insertions(+), 34 deletions(-)
>
>  diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
>  index 4d64402..848752e 100644
>  --- a/drivers/video/xilinxfb.c
>  +++ b/drivers/video/xilinxfb.c
>  @@ -37,6 +37,7 @@
>   #endif
>   #include <asm/io.h>
>   #include <linux/xilinxfb.h>
>  +#include <asm/dcr.h>
>
>   #define DRIVER_NAME            "xilinxfb"
>   #define DRIVER_DESCRIPTION     "Xilinx TFT LCD frame buffer driver"
>  @@ -111,8 +112,9 @@ struct xilinxfb_drvdata {
>
>         struct fb_info  info;           /* FB driver info record */
>
>  -       u32             regs_phys;      /* phys. address of the control registers */
>  -       u32 __iomem     *regs;          /* virt. address of the control registers */
>  +       dcr_host_t      dcr_host;
>  +       unsigned int    dcr_start;
>  +       unsigned int    dcr_len;
>
>         void            *fb_virt;       /* virt. address of the frame buffer */
>         dma_addr_t      fb_phys;        /* phys. address of the frame buffer */
>  @@ -135,7 +137,7 @@ struct xilinxfb_drvdata {
>   * when it's needed.
>   */
>   #define xilinx_fb_out_be32(driverdata, offset, val) \
>  -       out_be32(driverdata->regs + offset, val)
>  +       dcr_write(driverdata->dcr_host, offset, val)
>
>   static int
>   xilinx_fb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
>  @@ -203,7 +205,8 @@ static struct fb_ops xilinxfb_ops =
>   * Bus independent setup/teardown
>   */
>
>  -static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
>  +static int xilinxfb_assign(struct device *dev, dcr_host_t dcr_host,
>  +                          unsigned int dcr_start, unsigned int dcr_len,
>                            struct xilinxfb_platform_data *pdata)
>   {
>         struct xilinxfb_drvdata *drvdata;
>  @@ -218,21 +221,9 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
>         }
>         dev_set_drvdata(dev, drvdata);
>
>  -       /* Map the control registers in */
>  -       if (!request_mem_region(physaddr, 8, DRIVER_NAME)) {
>  -               dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
>  -                       physaddr);
>  -               rc = -ENODEV;
>  -               goto err_region;
>  -       }
>  -       drvdata->regs_phys = physaddr;
>  -       drvdata->regs = ioremap(physaddr, 8);
>  -       if (!drvdata->regs) {
>  -               dev_err(dev, "Couldn't lock memory region at 0x%08lX\n",
>  -                       physaddr);
>  -               rc = -ENODEV;
>  -               goto err_map;
>  -       }
>  +       drvdata->dcr_start = dcr_start;
>  +       drvdata->dcr_len = dcr_len;
>  +       drvdata->dcr_host = dcr_host;
>
>         /* Allocate the framebuffer memory */
>         if (pdata->fb_phys) {
>  @@ -247,7 +238,7 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
>         if (!drvdata->fb_virt) {
>                 dev_err(dev, "Could not allocate frame buffer memory\n");
>                 rc = -ENOMEM;
>  -               goto err_fbmem;
>  +               goto err_region;
>         }
>
>         /* Clear (turn to black) the framebuffer */
>  @@ -297,7 +288,6 @@ static int xilinxfb_assign(struct device *dev, unsigned long physaddr,
>         }
>
>         /* Put a banner in the log (for DEBUG) */
>  -       dev_dbg(dev, "regs: phys=%lx, virt=%p\n", physaddr, drvdata->regs);
>         dev_dbg(dev, "fb: phys=%p, virt=%p, size=%x\n",
>                 (void*)drvdata->fb_phys, drvdata->fb_virt, fbsize);
>
>  @@ -313,12 +303,6 @@ err_cmap:
>         /* Turn off the display */
>         xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
>
>  -err_fbmem:
>  -       iounmap(drvdata->regs);
>  -
>  -err_map:
>  -       release_mem_region(physaddr, 8);
>  -
>   err_region:
>         kfree(drvdata);
>         dev_set_drvdata(dev, NULL);
>  @@ -344,9 +328,8 @@ static int xilinxfb_release(struct device *dev)
>
>         /* Turn off the display */
>         xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
>  -       iounmap(drvdata->regs);
>
>  -       release_mem_region(drvdata->regs_phys, 8);
>  +       dcr_unmap(drvdata->dcr_host, drvdata->dcr_len);
>
>         kfree(drvdata);
>         dev_set_drvdata(dev, NULL);
>  @@ -362,20 +345,23 @@ static int xilinxfb_release(struct device *dev)
>   static int __devinit
>   xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
>   {
>  -       struct resource res;
>         const u32 *prop;
>         struct xilinxfb_platform_data pdata;
>         int size, rc;
>  +       int start, len;
>  +       dcr_host_t dcr_host;
>
>         /* Copy with the default pdata (not a ptr reference!) */
>         pdata = xilinx_fb_default_pdata;
>
>         dev_dbg(&op->dev, "xilinxfb_of_probe(%p, %p)\n", op, match);
>
>  -       rc = of_address_to_resource(op->node, 0, &res);
>  -       if (rc) {
>  +       start = dcr_resource_start(op->node, 0);
>  +       len = dcr_resource_len(op->node, 0);
>  +       dcr_host = dcr_map(op->node, start, len);
>  +       if (!DCR_MAP_OK(dcr_host)) {
>                 dev_err(&op->dev, "invalid address\n");
>  -               return rc;
>  +               return -ENODEV;
>         }
>
>         prop = of_get_property(op->node, "phys-size", &size);
>  @@ -399,7 +385,7 @@ xilinxfb_of_probe(struct of_device *op, const struct of_device_id *match)
>         if (of_find_property(op->node, "rotate-display", NULL))
>                 pdata.rotate_screen = 1;
>
>  -       return xilinxfb_assign(&op->dev, res.start, &pdata);
>  +       return xilinxfb_assign(&op->dev, dcr_host, start, len, &pdata);
>   }
>
>   static int __devexit xilinxfb_of_remove(struct of_device *op)
>  --
>  1.5.3.4-dirty
>
>
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[David Gibson]

On Mon, May 05, 2008 at 10:55:53PM -0600, Grant Likely wrote:
> On Mon, May 5, 2008 at 11:56 AM, Stephen Neuendorffer
> <stephen.neuendorffer@xilinx.com> wrote:
> > This device contains a dcr interface.  Previously, the dcr interface
> >  was assumed to be used in mmio mode, and the register space of the dcr
> >  interface was precomputed and stuffed in the device tree.  This patch
> >  makes use of the new dcr infrastructure to represent the dcr interface
> >  as any other dcr interface in the device tree.  This enables the dcr
> >  interface to be connected directly to a native dcr interface in a
> >  clean way.
> >
> >  In particular, the device tree expected looks like:
> >
> >                         dcr_v29_0: dcr@0 {
> >                                 #address-cells = <1>;
> >                                 #size-cells = <1>;
> >                                 compatible = "xlnx,dcr-v29-1.00.a";
> >                                 VGA_FrameBuffer: tft@80 {
> >                                         compatible = "xlnx,plb-tft-cntlr-ref-1.00.a";
> >                                         dcr-parent = <&opb2dcr_bridge_0>;
> >                                         dcr-reg = < 80 2 >;
> >                                         xlnx,default-tft-base-addr = <7f>;
> >                                         xlnx,dps-init = <1>;
> >                                         xlnx,on-init = <1>;
> >                                         xlnx,pixclk-is-busclk-divby4 = <1>;
> >                                 } ;
> >                         } ;
> >
> >                         opb2dcr_bridge_0: opb2dcr-bridge@40700000 {
> >                                 compatible = "xlnx,opb2dcr-bridge-1.00.b";
> >                                 dcr-access-method = "mmio";
> >                                 dcr-controller ;
> >                                 dcr-mmio-range = < 40700000 1000 >;
> >                                 dcr-mmio-stride = <4>;
> >                                 reg = < 40700000 1000 >;
> >                                 xlnx,family = "virtex2p";
> >                         } ;
> 
> Hmmm, something doesn't quite feel right about this.  The node
> describing the tft device is a child of the dcr@0 node which is the
> dcr bus.  However, dcr bindings use dcr-bus and dcr-reg instead of
> parent-child relationship to specify how to access the dcr registers.
> So, in this example; if the device is described by tft@80, and the dcr
> bus is described by opb2dcr-bridge@40700000, then what does dcr@0
> describe?  (I do understand what they really describe in EDK terms;
> but I'm looking at it through device tree glasses).
> 
> I don't think the presence of a dcr@0 node is a problem, but in this
> case #size/address-cells doesn't have any meaning (the child doesn't
> have a reg property) and it looks like it should be a child of the
> opb2dcr-bridge node (otherwise, what is it attached to?).

Yes, indeed.  If dcr@0 is representing the DCR bus / interface it
should really have the dcr-access-method property and have all the
dcr-parent handles point at it.

Current standard practice is not to represent the DCR bus as node with
subnodes for the DCR-controlled devices.  That's because the DCR bus
tends to run in addition to other on-chip busses, and some things have
to go on another on-chip bus to make sense, but still have DCR control
registers (for example the internal bus bridges on 4xx).

Arguably for DCR-only devices we should instead have a node
representing the DCR bus and just put the devices under it with the
DCR number encoded in reg in the normal way.  But then its
inconsistent with the devices that need the other DCR representation.

Segher and I did toss around some ideas for generalizing the DCR
representation to a way of representing that any node has some
presence on a bus other than its "primary" parent (e.g. other-bus-reg
= <&dcr-bus 0x0d0 0x010 &strange-i2c-control-bus 0xabc>).  Then
DCR-only devices would use normal "reg", devices that sit on another
bus would sit on that bus and use this representation to show their
DCR control registers.  Maybe one day.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[Segher Boessenkool]

> Current standard practice is not to represent the DCR bus as node with
> subnodes for the DCR-controlled devices.  That's because the DCR bus
> tends to run in addition to other on-chip busses, and some things have
> to go on another on-chip bus to make sense, but still have DCR control
> registers (for example the internal bus bridges on 4xx).

Yeah.

> Arguably for DCR-only devices we should instead have a node
> representing the DCR bus and just put the devices under it with the
> DCR number encoded in reg in the normal way.

Right.

> But then its
> inconsistent with the devices that need the other DCR representation.

OTOH, it _is_ consistent with all other (non-DCR) devices that way.

What you could do right now is to give such DCR-only devices both
normal "reg" etc., and the "dcr-reg" etc. properties, containing
both the same info.  If you do that, your device tree will be
correct (because you got all the standard stuff right), and the
kernel will like it as well (because it looks for the "dcr-reg"
stuff).

Then maybe later, if/when the kernel supports the standard addressing
for DCR as well, you could drop the "dcr-reg" things from your DTS.
Or you could just keep it.

David, will this work, do you think?

> Segher and I did toss around some ideas for generalizing the DCR
> representation to a way of representing that any node has some
> presence on a bus other than its "primary" parent (e.g. other-bus-reg
> = <&dcr-bus 0x0d0 0x010 &strange-i2c-control-bus 0xabc>).  Then
> DCR-only devices would use normal "reg", devices that sit on another
> bus would sit on that bus and use this representation to show their
> DCR control registers.  Maybe one day.

One day perhaps, yes :-)

It sounds cleaner to split such a prop into separate props per bus.
Maybe I said the opposite before, heh :-)


Segher

RE: [PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO andNATIVE

[Stephen Neuendorffer]

Yes, but this is a symptom of a separate problem, which is that it is
sometimes valuable to generate an optimized kernel configuration for a
particular FPGA system.  First order, I'm more concerned about getting a
kernel which is generic and can work with (more or less) any device
tree.

Steve

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Monday, May 05, 2008 5:12 PM
> To: Stephen Neuendorffer
> Cc: jwboyer@linux.vnet.ibm.com; grant.likely@secretlab.ca;
linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO
andNATIVE
>=20
>=20
> On Mon, 2008-05-05 at 10:56 -0700, Stephen Neuendorffer wrote:
> > FPGA designs may have need of both MMIO-based and NATIVE-based dcr
> > interfaces.
>=20
> You say _may_ ... wouldn't it be better if it was thus left to a given
> virtex based platform to enable DCR_MMIO if it uses it and leave only
> NATIVE by default ?
>=20
> > Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com>
> > ---
> >  arch/powerpc/platforms/40x/Kconfig |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/40x/Kconfig
b/arch/powerpc/platforms/40x/Kconfig
> > index a9260e2..b8e06df 100644
> > --- a/arch/powerpc/platforms/40x/Kconfig
> > +++ b/arch/powerpc/platforms/40x/Kconfig
> > @@ -123,6 +123,8 @@ config 405GPR
> >
> >  config XILINX_VIRTEX
> >  	bool
> > +	select PPC_DCR_MMIO
> > +	select PPC_DCR_NATIVE
> >
> >  config XILINX_VIRTEX_II_PRO
> >  	bool
>=20

RE: [PATCH 1/4] [v4][POWERPC] refactor dcr code

[Stephen Neuendorffer]

I'll fix it.

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Monday, May 05, 2008 9:02 PM
> To: Stephen Rothwell
> Cc: Stephen Neuendorffer; jwboyer@linux.vnet.ibm.com;
grant.likely@secretlab.ca; linuxppc-
> dev@ozlabs.org
> Subject: Re: [PATCH 1/4] [v4][POWERPC] refactor dcr code
>=20
>=20
> On Tue, 2008-05-06 at 13:40 +1000, Stephen Rothwell wrote:
> > Since find_dcr_parent has done a of_node_get on its return value,
you
> > leak a reference to dp here i.e. you need an of_node_put(dp) before
> > you return.
>=20
> He inherited that bug from other dcr.c functions I wrote, my fault.
>=20
> Stephen (N. not R.), would you mind fixing them while at it or do you
> want me to cook up a patch ?
>=20
> Cheers,
> Ben.
>=20
>=20

Re: [PATCH 1/4] [v4][POWERPC] refactor dcr code

[Josh Boyer]

On Tue, 6 May 2008 10:34:36 -0700
"Stephen Neuendorffer" <stephen.neuendorffer@xilinx.com> wrote:

> 
> I'll fix it.

Great.  Otherwise the patch looks pretty good in my review.  I'll queue
it up for 2.6.27.

josh

RE: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcrinfrastructure.

[Stephen Neuendorffer]

> -----Original Message-----
> From: David Gibson [mailto:david@gibson.dropbear.id.au]
> Sent: Monday, May 05, 2008 11:15 PM
> To: Grant Likely
> Cc: Stephen Neuendorffer; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use
dcrinfrastructure.
>=20
> On Mon, May 05, 2008 at 10:55:53PM -0600, Grant Likely wrote:
> > On Mon, May 5, 2008 at 11:56 AM, Stephen Neuendorffer
> > <stephen.neuendorffer@xilinx.com> wrote:
> > > This device contains a dcr interface.  Previously, the dcr
interface
> > >  was assumed to be used in mmio mode, and the register space of
the dcr
> > >  interface was precomputed and stuffed in the device tree.  This
patch
> > >  makes use of the new dcr infrastructure to represent the dcr
interface
> > >  as any other dcr interface in the device tree.  This enables the
dcr
> > >  interface to be connected directly to a native dcr interface in a
> > >  clean way.
> > >
> > >  In particular, the device tree expected looks like:
> > >
> > >                         dcr_v29_0: dcr@0 {
> > >                                 #address-cells =3D <1>;
> > >                                 #size-cells =3D <1>;
> > >                                 compatible =3D
"xlnx,dcr-v29-1.00.a";
> > >                                 VGA_FrameBuffer: tft@80 {
> > >                                         compatible =3D
"xlnx,plb-tft-cntlr-ref-1.00.a";
> > >                                         dcr-parent =3D
<&opb2dcr_bridge_0>;
> > >                                         dcr-reg =3D < 80 2 >;
> > >                                         xlnx,default-tft-base-addr
=3D <7f>;
> > >                                         xlnx,dps-init =3D <1>;
> > >                                         xlnx,on-init =3D <1>;
> > >
xlnx,pixclk-is-busclk-divby4 =3D <1>;
> > >                                 } ;
> > >                         } ;
> > >
> > >                         opb2dcr_bridge_0: opb2dcr-bridge@40700000
{
> > >                                 compatible =3D
"xlnx,opb2dcr-bridge-1.00.b";
> > >                                 dcr-access-method =3D "mmio";
> > >                                 dcr-controller ;
> > >                                 dcr-mmio-range =3D < 40700000 1000
>;
> > >                                 dcr-mmio-stride =3D <4>;
> > >                                 reg =3D < 40700000 1000 >;
> > >                                 xlnx,family =3D "virtex2p";
> > >                         } ;
> >
> > Hmmm, something doesn't quite feel right about this.  The node
> > describing the tft device is a child of the dcr@0 node which is the
> > dcr bus.  However, dcr bindings use dcr-bus and dcr-reg instead of
> > parent-child relationship to specify how to access the dcr
registers.
> > So, in this example; if the device is described by tft@80, and the
dcr
> > bus is described by opb2dcr-bridge@40700000, then what does dcr@0
> > describe?  (I do understand what they really describe in EDK terms;
> > but I'm looking at it through device tree glasses).
> >
> > I don't think the presence of a dcr@0 node is a problem, but in this
> > case #size/address-cells doesn't have any meaning (the child doesn't
> > have a reg property) and it looks like it should be a child of the
> > opb2dcr-bridge node (otherwise, what is it attached to?).
>=20
> Yes, indeed.  If dcr@0 is representing the DCR bus / interface it
> should really have the dcr-access-method property and have all the
> dcr-parent handles point at it.

Hmm, I tend to agree.  Certainly the address-cells and size-cells can
go.  Part of the nastiness is that I'm trying to maintain a modicum of
backward compatibility at the moment in the device tree generator.  This
structure allow the dcr@0 node to have ranges; and the tft node to have
a properly translated reg =3D <> property for the existing driver which
only understands mmio.  I don't think it really works for the opb2dcr
bridge to be a bridge and a dcr-controller at the same time. :)  This
structure is also very similar to what is generated if the
dcr-controller is native from the processor (there's just no bridge).

> Current standard practice is not to represent the DCR bus as node with
> subnodes for the DCR-controlled devices.  That's because the DCR bus
> tends to run in addition to other on-chip busses, and some things have
> to go on another on-chip bus to make sense, but still have DCR control
> registers (for example the internal bus bridges on 4xx).
>=20
> Arguably for DCR-only devices we should instead have a node
> representing the DCR bus and just put the devices under it with the
> DCR number encoded in reg in the normal way.  But then its
> inconsistent with the devices that need the other DCR representation.

Yup, it's exactly this problem I'm trying to fix in the case of the tft
driver.

> Segher and I did toss around some ideas for generalizing the DCR
> representation to a way of representing that any node has some
> presence on a bus other than its "primary" parent (e.g. other-bus-reg
> =3D <&dcr-bus 0x0d0 0x010 &strange-i2c-control-bus 0xabc>).  Then
> DCR-only devices would use normal "reg", devices that sit on another
> bus would sit on that bus and use this representation to show their
> DCR control registers.  Maybe one day.

Don't know if I like this, since it obscures the types of the
interfaces.

Steve

[PATCH 1/4] [v5][POWERPC] refactor dcr code

[Stephen Neuendorffer]

Previously, dcr support was configured at compile time to either using
MMIO or native dcr instructions.  Although this works for most
platforms, it fails on FPGA platforms:

1) Systems may include more than one dcr bus.
2) Systems may be native dcr capable and still use memory mapped dcr interface.

This patch provides runtime support based on the device trees for the
case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both
selected.  Previously, this was a poorly defined configuration, which
happened to provide NATIVE support.  The runtime selection is made
based on the dcr controller having a 'dcr-access-method' attribute
in the device tree.  If only one of the above options is selected,
then the code uses #defines to select only the used code in order to
avoid introducing overhead in existing usage.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

--

[v4]
Converted flags to be DCR_HOST_*
Added WARN_ON in case someone forgets to call MAP_OK()
Fixed dealing with the dcr-access-method correctly, which was still
	off in the wrong patch.
[v5]
Fixed leaked reference to the device tree.
---
 arch/powerpc/sysdev/dcr.c         |  154 +++++++++++++++++++++++++++++--------
 include/asm-powerpc/dcr-generic.h |   49 ++++++++++++
 include/asm-powerpc/dcr-mmio.h    |   20 +++--
 include/asm-powerpc/dcr-native.h  |   16 +++--
 include/asm-powerpc/dcr.h         |   39 +++++++++-
 5 files changed, 231 insertions(+), 47 deletions(-)
 create mode 100644 include/asm-powerpc/dcr-generic.h

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index 437e48d..5f39a79 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -23,6 +23,105 @@
 #include <asm/prom.h>
 #include <asm/dcr.h>
 
+static struct device_node *find_dcr_parent(struct device_node *node)
+{
+	struct device_node *par, *tmp;
+	const u32 *p;
+
+	for (par = of_node_get(node); par;) {
+		if (of_get_property(par, "dcr-controller", NULL))
+			break;
+		p = of_get_property(par, "dcr-parent", NULL);
+		tmp = par;
+		if (p == NULL)
+			par = of_get_parent(par);
+		else
+			par = of_find_node_by_phandle(*p);
+		of_node_put(tmp);
+	}
+	return par;
+}
+
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+bool dcr_map_ok_generic(dcr_host_t host)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		return dcr_map_ok_native(host.host.native);
+	else if (host.type == DCR_HOST_MMIO)
+		return dcr_map_ok_mmio(host.host.mmio);
+	else
+		return 0;
+}
+EXPORT_SYMBOL_GPL(dcr_map_ok_generic);
+
+dcr_host_t dcr_map_generic(struct device_node *dev,
+			   unsigned int dcr_n,
+			   unsigned int dcr_c)
+{
+	dcr_host_t host;
+	struct device_node *dp;
+	const char *prop;
+
+	host.type = DCR_HOST_INVALID;
+
+	dp = find_dcr_parent(dev);
+	if (dp == NULL)
+		return host;
+
+	prop = of_get_property(dp, "dcr-access-method", NULL);
+
+	pr_debug("dcr_map_generic(dcr-access-method = %s)\n", prop);
+
+	if (!strcmp(prop, "native")) {
+		host.type = DCR_HOST_NATIVE;
+		host.host.native = dcr_map_native(dev, dcr_n, dcr_c);
+	} else if (!strcmp(prop, "mmio")) {
+		host.type = DCR_HOST_MMIO;
+		host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c);
+	}
+
+	of_node_put(dp);
+	return host;
+}
+EXPORT_SYMBOL_GPL(dcr_map_generic);
+
+void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		dcr_unmap_native(host.host.native, dcr_c);
+	else if (host.type == DCR_HOST_MMIO)
+		dcr_unmap_mmio(host.host.mmio, dcr_c);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+}
+EXPORT_SYMBOL_GPL(dcr_unmap_generic);
+
+u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		return dcr_read_native(host.host.native, dcr_n);
+	else if (host.type == DCR_HOST_MMIO)
+		return dcr_read_mmio(host.host.mmio, dcr_n);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dcr_read_generic);
+
+void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value)
+{
+	if (host.type == DCR_HOST_NATIVE)
+		dcr_write_native(host.host.native, dcr_n, value);
+	else if (host.type == DCR_HOST_MMIO)
+		dcr_write_mmio(host.host.mmio, dcr_n, value);
+	else /* host.type == DCR_HOST_INVALID */
+		WARN_ON(true);
+}
+EXPORT_SYMBOL_GPL(dcr_write_generic);
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 unsigned int dcr_resource_start(struct device_node *np, unsigned int index)
 {
 	unsigned int ds;
@@ -47,26 +146,7 @@ unsigned int dcr_resource_len(struct device_node *np, unsigned int index)
 }
 EXPORT_SYMBOL_GPL(dcr_resource_len);
 
-#ifndef CONFIG_PPC_DCR_NATIVE
-
-static struct device_node * find_dcr_parent(struct device_node * node)
-{
-	struct device_node *par, *tmp;
-	const u32 *p;
-
-	for (par = of_node_get(node); par;) {
-		if (of_get_property(par, "dcr-controller", NULL))
-			break;
-		p = of_get_property(par, "dcr-parent", NULL);
-		tmp = par;
-		if (p == NULL)
-			par = of_get_parent(par);
-		else
-			par = of_find_node_by_phandle(*p);
-		of_node_put(tmp);
-	}
-	return par;
-}
+#ifdef CONFIG_PPC_DCR_MMIO
 
 u64 of_translate_dcr_address(struct device_node *dev,
 			     unsigned int dcr_n,
@@ -75,7 +155,7 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	struct device_node *dp;
 	const u32 *p;
 	unsigned int stride;
-	u64 ret;
+	u64 ret = OF_BAD_ADDR;
 
 	dp = find_dcr_parent(dev);
 	if (dp == NULL)
@@ -90,7 +170,7 @@ u64 of_translate_dcr_address(struct device_node *dev,
 	if (p == NULL)
 		p = of_get_property(dp, "dcr-mmio-space", NULL);
 	if (p == NULL)
-		return OF_BAD_ADDR;
+		goto done;
 
 	/* Maybe could do some better range checking here */
 	ret = of_translate_address(dp, p);
@@ -98,21 +178,25 @@ u64 of_translate_dcr_address(struct device_node *dev,
 		ret += (u64)(stride) * (u64)dcr_n;
 	if (out_stride)
 		*out_stride = stride;
+
+ done:
+	of_node_put(dp);
 	return ret;
 }
 
-dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-		   unsigned int dcr_c)
+dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+			     unsigned int dcr_n,
+			     unsigned int dcr_c)
 {
-	dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
+	dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
 	u64 addr;
 
 	pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
 		 dev->full_name, dcr_n, dcr_c);
 
 	addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
-	pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
-		 addr, ret.stride);
+	pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
+		 (unsigned long long) addr, ret.stride);
 	if (addr == OF_BAD_ADDR)
 		return ret;
 	pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
@@ -124,11 +208,11 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 	ret.token -= dcr_n * ret.stride;
 	return ret;
 }
-EXPORT_SYMBOL_GPL(dcr_map);
+EXPORT_SYMBOL_GPL(dcr_map_mmio);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
+void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
 {
-	dcr_host_t h = host;
+	dcr_host_mmio_t h = host;
 
 	if (h.token == NULL)
 		return;
@@ -136,7 +220,11 @@ void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 	iounmap(h.token);
 	h.token = NULL;
 }
-EXPORT_SYMBOL_GPL(dcr_unmap);
-#else	/* defined(CONFIG_PPC_DCR_NATIVE) */
+EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
+
+#endif /* defined(CONFIG_PPC_DCR_MMIO) */
+
+#ifdef CONFIG_PPC_DCR_NATIVE
 DEFINE_SPINLOCK(dcr_ind_lock);
-#endif	/* !defined(CONFIG_PPC_DCR_NATIVE) */
+#endif	/* defined(CONFIG_PPC_DCR_NATIVE) */
+
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h
new file mode 100644
index 0000000..35b7159
--- /dev/null
+++ b/include/asm-powerpc/dcr-generic.h
@@ -0,0 +1,49 @@
+/*
+ * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
+ *                    <benh@kernel.crashing.org>
+ *
+ *   This program is free software;  you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   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, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_DCR_GENERIC_H
+#define _ASM_POWERPC_DCR_GENERIC_H
+#ifdef __KERNEL__
+#ifndef __ASSEMBLY__
+
+enum host_type_t {DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID};
+
+typedef struct {
+	enum host_type_t type;
+	union {
+		dcr_host_mmio_t mmio;
+		dcr_host_native_t native;
+	} host;
+} dcr_host_t;
+
+extern bool dcr_map_ok_generic(dcr_host_t host);
+
+extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned int dcr_n,
+			  unsigned int dcr_c);
+extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
+
+extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
+
+extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32 value);
+
+#endif /* __ASSEMBLY__ */
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_DCR_GENERIC_H */
+
+
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 08532ff..acd491d 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -27,20 +27,26 @@ typedef struct {
 	void __iomem *token;
 	unsigned int stride;
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_mmio_t;
 
-#define DCR_MAP_OK(host)	((host).token != NULL)
+static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
+{
+	return host.token != NULL;
+}
 
-extern dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
-			  unsigned int dcr_c);
-extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
+extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
+				    unsigned int dcr_n,
+				    unsigned int dcr_c);
+extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c);
 
-static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
+static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int dcr_n)
 {
 	return in_be32(host.token + ((host.base + dcr_n) * host.stride));
 }
 
-static inline void dcr_write(dcr_host_t host, unsigned int dcr_n, u32 value)
+static inline void dcr_write_mmio(dcr_host_mmio_t host,
+				  unsigned int dcr_n,
+				  u32 value)
 {
 	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index f8398ce..72d2b72 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -26,14 +26,18 @@
 
 typedef struct {
 	unsigned int base;
-} dcr_host_t;
+} dcr_host_native_t;
 
-#define DCR_MAP_OK(host)	(1)
+static inline bool dcr_map_ok_native(dcr_host_native_t host)
+{
+	return 1;
+}
 
-#define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_c)		do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
+#define dcr_map_native(dev, dcr_n, dcr_c) \
+	((dcr_host_native_t){ .base = (dcr_n) })
+#define dcr_unmap_native(host, dcr_c)		do {} while (0)
+#define dcr_read_native(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write_native(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
index 9338d50..53b2830 100644
--- a/include/asm-powerpc/dcr.h
+++ b/include/asm-powerpc/dcr.h
@@ -20,14 +20,50 @@
 #ifndef _ASM_POWERPC_DCR_H
 #define _ASM_POWERPC_DCR_H
 #ifdef __KERNEL__
+#ifndef __ASSEMBLY__
 #ifdef CONFIG_PPC_DCR
 
 #ifdef CONFIG_PPC_DCR_NATIVE
 #include <asm/dcr-native.h>
-#else
+#endif
+
+#ifdef CONFIG_PPC_DCR_MMIO
 #include <asm/dcr-mmio.h>
 #endif
 
+
+/* Indirection layer for providing both NATIVE and MMIO support. */
+
+#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
+
+#include <asm/dcr-generic.h>
+
+#define DCR_MAP_OK(host)	dcr_map_ok_generic(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_generic(host, dcr_n, value)
+
+#else
+
+#ifdef CONFIG_PPC_DCR_NATIVE
+typedef dcr_host_native_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_native(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n, value)
+#else
+typedef dcr_host_mmio_t dcr_host_t;
+#define DCR_MAP_OK(host)	dcr_map_ok_mmio(host)
+#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
+#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
+#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
+#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n, value)
+#endif
+
+#endif /* defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) */
+
 /*
  * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
  * base from the device-tree
@@ -41,5 +77,6 @@ extern unsigned int dcr_resource_len(struct device_node *np,
 #endif /* CONFIG_PPC_MERGE */
 
 #endif /* CONFIG_PPC_DCR */
+#endif /* __ASSEMBLY__ */
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DCR_H */
-- 
1.5.3.4

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure.

[David Gibson]

On Tue, May 06, 2008 at 12:56:44PM +0200, Segher Boessenkool wrote:
>> Current standard practice is not to represent the DCR bus as node with
>> subnodes for the DCR-controlled devices.  That's because the DCR bus
>> tends to run in addition to other on-chip busses, and some things have
>> to go on another on-chip bus to make sense, but still have DCR control
>> registers (for example the internal bus bridges on 4xx).
>
> Yeah.
>
>> Arguably for DCR-only devices we should instead have a node
>> representing the DCR bus and just put the devices under it with the
>> DCR number encoded in reg in the normal way.
>
> Right.
>
>> But then its
>> inconsistent with the devices that need the other DCR representation.
>
> OTOH, it _is_ consistent with all other (non-DCR) devices that way.
>
> What you could do right now is to give such DCR-only devices both
> normal "reg" etc., and the "dcr-reg" etc. properties, containing
> both the same info.  If you do that, your device tree will be
> correct (because you got all the standard stuff right), and the
> kernel will like it as well (because it looks for the "dcr-reg"
> stuff).

> Then maybe later, if/when the kernel supports the standard addressing
> for DCR as well, you could drop the "dcr-reg" things from your DTS.
> Or you could just keep it.
>
> David, will this work, do you think?

Um.. yeah, I guess that should work.  Don't forget to slap on
"simple-bus" as appropriate.

>> Segher and I did toss around some ideas for generalizing the DCR
>> representation to a way of representing that any node has some
>> presence on a bus other than its "primary" parent (e.g. other-bus-reg
>> = <&dcr-bus 0x0d0 0x010 &strange-i2c-control-bus 0xabc>).  Then
>> DCR-only devices would use normal "reg", devices that sit on another
>> bus would sit on that bus and use this representation to show their
>> DCR control registers.  Maybe one day.
>
> One day perhaps, yes :-)
>
> It sounds cleaner to split such a prop into separate props per bus.
> Maybe I said the opposite before, heh :-)

No I think you said that before, too, but I'm not sure I agree.
Seperate props per bus (well, per bus type, really) is probably more
human-readable, but a single prop has the advantage that we can have a
common parser that will interpret future variations on this theme
correctly without having to add things to some list of properties to
look for.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcrinfrastructure.

[David Gibson]

On Tue, May 06, 2008 at 10:43:50AM -0700, Stephen Neuendorffer wrote:
[snip]
> > > Hmmm, something doesn't quite feel right about this.  The node
> > > describing the tft device is a child of the dcr@0 node which is the
> > > dcr bus.  However, dcr bindings use dcr-bus and dcr-reg instead of
> > > parent-child relationship to specify how to access the dcr
> registers.
> > > So, in this example; if the device is described by tft@80, and the
> dcr
> > > bus is described by opb2dcr-bridge@40700000, then what does dcr@0
> > > describe?  (I do understand what they really describe in EDK terms;
> > > but I'm looking at it through device tree glasses).
> > >
> > > I don't think the presence of a dcr@0 node is a problem, but in this
> > > case #size/address-cells doesn't have any meaning (the child doesn't
> > > have a reg property) and it looks like it should be a child of the
> > > opb2dcr-bridge node (otherwise, what is it attached to?).
> > 
> > Yes, indeed.  If dcr@0 is representing the DCR bus / interface it
> > should really have the dcr-access-method property and have all the
> > dcr-parent handles point at it.
> 
> Hmm, I tend to agree.  Certainly the address-cells and size-cells can
> go.  Part of the nastiness is that I'm trying to maintain a modicum of
> backward compatibility at the moment in the device tree generator.  This
> structure allow the dcr@0 node to have ranges; and the tft node to have
> a properly translated reg = <> property for the existing driver which
> only understands mmio.  I don't think it really works for the opb2dcr
> bridge to be a bridge and a dcr-controller at the same time. :)  This
> structure is also very similar to what is generated if the
> dcr-controller is native from the processor (there's just no bridge).

I don't really understand what you're getting at here, sorry.  Perhaps
you could describe what you're doing with the tft device in more
detail?

> > Current standard practice is not to represent the DCR bus as node with
> > subnodes for the DCR-controlled devices.  That's because the DCR bus
> > tends to run in addition to other on-chip busses, and some things have
> > to go on another on-chip bus to make sense, but still have DCR control
> > registers (for example the internal bus bridges on 4xx).
> > 
> > Arguably for DCR-only devices we should instead have a node
> > representing the DCR bus and just put the devices under it with the
> > DCR number encoded in reg in the normal way.  But then its
> > inconsistent with the devices that need the other DCR representation.
> 
> Yup, it's exactly this problem I'm trying to fix in the case of the tft
> driver.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

RE: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Usedcrinfrastructure.

[Stephen Neuendorffer]

[-- Attachment #1: Type: text/plain, Size: 3882 bytes --]


The problem is that the tft driver currently in mainline assumes that the dcr control registers
are accessed through mmio, and probes for a reg=<> property.  Since the device is actually a dcr device,
it can be connected via mmio through a bride, or to the native dcr bus of the processor.  For the time being,
I'd like to generate device trees compatible with either mechanism.

so, the problem is that the tree-parents of the tft node all have ranges, and if the dcr-parent of the node is a bridge, then it has a reg=<> property,
But if the dcr-parent is also a tree-parent, then it has to have ranges; and a reg=<> property, which seems very strange, and not something I think is
a good thing to do.

Steve

-----Original Message-----
From: David Gibson [mailto:david@gibson.dropbear.id.au]
Sent: Wed 5/7/2008 6:59 PM
To: Stephen Neuendorffer
Cc: Grant Likely; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Usedcrinfrastructure.
 
On Tue, May 06, 2008 at 10:43:50AM -0700, Stephen Neuendorffer wrote:
[snip]
> > > Hmmm, something doesn't quite feel right about this.  The node
> > > describing the tft device is a child of the dcr@0 node which is the
> > > dcr bus.  However, dcr bindings use dcr-bus and dcr-reg instead of
> > > parent-child relationship to specify how to access the dcr
> registers.
> > > So, in this example; if the device is described by tft@80, and the
> dcr
> > > bus is described by opb2dcr-bridge@40700000, then what does dcr@0
> > > describe?  (I do understand what they really describe in EDK terms;
> > > but I'm looking at it through device tree glasses).
> > >
> > > I don't think the presence of a dcr@0 node is a problem, but in this
> > > case #size/address-cells doesn't have any meaning (the child doesn't
> > > have a reg property) and it looks like it should be a child of the
> > > opb2dcr-bridge node (otherwise, what is it attached to?).
> > 
> > Yes, indeed.  If dcr@0 is representing the DCR bus / interface it
> > should really have the dcr-access-method property and have all the
> > dcr-parent handles point at it.
> 
> Hmm, I tend to agree.  Certainly the address-cells and size-cells can
> go.  Part of the nastiness is that I'm trying to maintain a modicum of
> backward compatibility at the moment in the device tree generator.  This
> structure allow the dcr@0 node to have ranges; and the tft node to have
> a properly translated reg = <> property for the existing driver which
> only understands mmio.  I don't think it really works for the opb2dcr
> bridge to be a bridge and a dcr-controller at the same time. :)  This
> structure is also very similar to what is generated if the
> dcr-controller is native from the processor (there's just no bridge).

I don't really understand what you're getting at here, sorry.  Perhaps
you could describe what you're doing with the tft device in more
detail?

> > Current standard practice is not to represent the DCR bus as node with
> > subnodes for the DCR-controlled devices.  That's because the DCR bus
> > tends to run in addition to other on-chip busses, and some things have
> > to go on another on-chip bus to make sense, but still have DCR control
> > registers (for example the internal bus bridges on 4xx).
> > 
> > Arguably for DCR-only devices we should instead have a node
> > representing the DCR bus and just put the devices under it with the
> > DCR number encoded in reg in the normal way.  But then its
> > inconsistent with the devices that need the other DCR representation.
> 
> Yup, it's exactly this problem I'm trying to fix in the case of the tft
> driver.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson



[-- Attachment #2: Type: text/html, Size: 5306 bytes --]

Re: [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Usedcrinfrastructure.

[David Gibson]

On Wed, May 07, 2008 at 09:46:51PM -0700, Stephen Neuendorffer wrote:
> 
> The problem is that the tft driver currently in mainline assumes
> that the dcr control registers are accessed through mmio, and probes
> for a reg=<> property.  Since the device is actually a dcr device,
> it can be connected via mmio through a bride, or to the native dcr
> bus of the processor.  For the time being, I'd like to generate
> device trees compatible with either mechanism.

Ah, I think I see.  The current driver expects the device to be
represented as if it were an mmio device, not using the standard dcr
notation.  It can get away with this because the DCRs are
memory-mapped, in a way representable by 'ranges'.

> so, the problem is that the tree-parents of the tft node all have
> ranges, and if the dcr-parent of the node is a bridge, then it has a
> reg=<> property, But if the dcr-parent is also a tree-parent, then
> it has to have ranges; and a reg=<> property, which seems very
> strange, and not something I think is a good thing to do.

This I don't follow.

But I don't think it matters.  The representation suggested by Segher,
which has the DCR addressing redundantly described in 'reg' and
'dcr-reg' properties should work for you.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson