[PATCH RFC 0/7] "NAND on UPM" and related patches

[PATCH RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

Hi all,

Here are patches to support NAND on UPM. That driver is generic for
all processors with FSL UPMs. And because of that, few more patches are
needed -- GPIO API and generic FSL UPM functions.

This is early RFC, all patches are in single thread, so everyone could
make up overall picture of what is going on. I'll split the thread by
topics after that RFC.

Ok, the patches and what they are for:

1,2,3,4. GPIO API:
------------------
Usually NAND chips exports RNB (Ready-Not-Busy) pin, so drivers
could read it and get a hint when chip is ready.

Often, WP (write protect) pin is also connected to GPIO. So, GPIO API
is mandatory for generic drivers.

OF device tree GPIOs bindings are similar to IRQs:

node {
	gpios = <bank pin bank pin bank pin>;
	gpio-parent = <&par_io_controller>;
};

"bank pin" scheme is controller specific, so controllers that want
to implement flat mappings or any other could do so.

So far I implemented GPIO API for QE and CPM2 chips.

QE GPIO API tested to work with FSL UPM NAND driver and MPC8360E-RDK
(STMicro NAND512W3A2BN6E).

CPM2 GPIO API tested to work with MPC8555E+Samsung HY27UF081G6 (LP).

GPIO API is described in Documentation/gpio.txt, and these
patches are tend to support most of it.

As an additional bonus, PowerPC now gets access to few pleasing
drivers:

- drivers/leds/leds-gpio.c (we could use it to play with
  on-board LEDs);
- drivers/i2c/busses/i2c-gpio.c (generic I2C bit-banging driver);
- drivers/input/keyboard/gpio_keys.c - gpio keys (requires
  gpio_to_irq, so far not implemented);
- Could be more (I named the ones I knew about).


Also, in the upcoming kernels, there will be GPIOLIB[1] addition to
the generic GPIO API, to support off-chip GPIO expanders (like MFDs
on I2C/LBC).

[1] http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc4/2.6.24-rc4-mm1/broken-out/generic-gpio-gpio_chip-support.patch

But so far we support on-chip GPIOs only.

5 - FSL UPM infrastructure:
---------------------------
UPM address register is shared among UPMs, so we have to do
proper locking. On the other hand, if we know that specific
board using only one UPM we could bypass locking, and gain some
performance win.


6. FSL UPM NAND driver:
-----------------------
It's using FSL UPM functions and GPIO API. It does not implement
device tree partitions parsing. That issue is completely other
matter, that is, I have to factor out parsing functions from
physmap_of.c. This desires separate patch on top of the whole
series. If anyone currently working on this, let me know.


7. Example of usage.
--------------------
MPC8360E-RDK as an example.


I would appreciate any comments and suggestions, thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

[PATCH RFC 1/7] [POWERPC] Implement GPIO API embryo

[Anton Vorontsov]

This patch implements GPIO API as described in Documentation/gpio.txt.
Two calls unimplemented though: irq_to_gpio and gpio_to_irq.

I'm also providing OF helpers.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/Kconfig             |    4 +++
 arch/powerpc/kernel/prom_parse.c |   50 +++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/gpio.h       |   51 ++++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/prom.h       |   23 +++++++++++++++++
 4 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-powerpc/gpio.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 232c298..596982f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -73,6 +73,10 @@ config GENERIC_FIND_NEXT_BIT
 	bool
 	default y
 
+config GENERIC_GPIO
+	bool
+	default n
+
 config ARCH_NO_VIRT_TO_BUS
 	def_bool PPC64
 
diff --git a/arch/powerpc/kernel/prom_parse.c b/arch/powerpc/kernel/prom_parse.c
index b5c96af..5228fe6 100644
--- a/arch/powerpc/kernel/prom_parse.c
+++ b/arch/powerpc/kernel/prom_parse.c
@@ -7,6 +7,7 @@
 #include <linux/ioport.h>
 #include <linux/etherdevice.h>
 #include <asm/prom.h>
+#include <asm/gpio.h>
 #include <asm/pci-bridge.h>
 
 #ifdef DEBUG
@@ -1067,3 +1068,52 @@ void __iomem *of_iomap(struct device_node *np, int index)
 	return ioremap(res.start, 1 + res.end - res.start);
 }
 EXPORT_SYMBOL(of_iomap);
+
+int __of_parse_gpio_bank_pin(struct device_node *np, int index,
+			     int bank_width, int max_bank)
+{
+	int bank;
+	int pin;
+	const u32 *gpios;
+	int len;
+
+	gpios = of_get_property(np, "gpios", &len);
+	len /= sizeof(u32);
+
+	if (len < 2 || len % 2 || index > len / 2 - 1)
+		return -EINVAL;
+
+	bank = gpios[index * 2];
+	pin = gpios[index * 2 + 1];
+
+	if (bank >= max_bank || pin >= bank_width)
+		return -EINVAL;
+
+	return bank * bank_width + pin;
+}
+EXPORT_SYMBOL_GPL(__of_parse_gpio_bank_pin);
+
+int of_get_gpio(struct device_node *np, int index)
+{
+	int ret;
+	const phandle *gc_ph;
+	struct device_node *gc;
+	struct of_gpio_chip *of_gpio_chip;
+
+	gc_ph = of_get_property(np, "gpio-parent", NULL);
+	if (!gc_ph)
+		return -EINVAL;
+
+	gc = of_find_node_by_phandle(*gc_ph);
+	if (!gc || !gc->data)
+		return -EINVAL;
+
+	of_gpio_chip = gc->data;
+
+	ret = of_gpio_chip->xlate(np, index);
+	if (ret < 0)
+		of_node_put(gc);
+
+	return ret;
+}
+EXPORT_SYMBOL(of_get_gpio);
diff --git a/include/asm-powerpc/gpio.h b/include/asm-powerpc/gpio.h
new file mode 100644
index 0000000..f7513ff
--- /dev/null
+++ b/include/asm-powerpc/gpio.h
@@ -0,0 +1,51 @@
+/*
+ * Generic GPIO API implementation for PowerPC.
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ * Copyright (c) 2007  Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#ifndef __ASM_POWERPC_GPIO_H
+#define __ASM_POWERPC_GPIO_H
+
+#ifdef CONFIG_GENERIC_GPIO
+
+extern int gpio_request(unsigned int gpio, const char *label);
+static inline void gpio_free(unsigned int gpio) {}
+
+extern int gpio_direction_input(unsigned int gpio);
+extern int gpio_direction_output(unsigned int gpio, int value);
+
+extern int gpio_get_value(unsigned int gpio);
+extern int gpio_set_value(unsigned int gpio, int value);
+
+/*
+ * Not implemented, yet.
+ */
+static inline int gpio_to_irq(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	return -ENOSYS;
+}
+
+/*
+ * OF specific gpio_chip handler.
+ */
+struct of_gpio_chip {
+	int (*xlate)(struct device_node *np, int index);
+};
+
+#endif /* CONFIG_GENERIC_GPIO */
+
+#include <asm-generic/gpio.h>
+
+#endif /* __ASM_POWERPC_GPIO_H */
diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
index 925e2d3..019af61 100644
--- a/include/asm-powerpc/prom.h
+++ b/include/asm-powerpc/prom.h
@@ -326,6 +326,29 @@ extern int of_irq_to_resource(struct device_node *dev, int index,
  */
 extern void __iomem *of_iomap(struct device_node *device, int index);
 
+/**
+ * __of_parse_gpio_bank_pin - Helper function to translate "bank pin" GPIOs
+ * @np:		device node to get GPIO from
+ * @index:	index of the GPIO
+ * @bank_width:	pins per bank
+ * @max_bank:	maximum value to represent a bank
+ *
+ * Returns GPIO number to use with Linux generic GPIO api, or
+ * one of the errno value on the error condition.
+ */
+extern int __of_parse_gpio_bank_pin(struct device_node *np, int index,
+				    int bank_width, int max_bank);
+
+/**
+ * of_get_gpio - Translate GPIO number given in the device tree
+ * @np:		device node to get GPIO from
+ * @index:	index of the GPIO
+ *
+ * Returns GPIO number to use with Linux generic GPIO API, or
+ * one of the errno value on the error condition.
+ */
+extern int of_get_gpio(struct device_node *np, int index);
+
 /*
  * NB:  This is here while we transition from using asm/prom.h
  * to linux/of.h
-- 
1.5.2.2

[PATCH RFC 2/7] [POWERPC] QE: implement GPIO API

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/Kconfig     |    1 +
 arch/powerpc/sysdev/qe_lib/qe_io.c |   93 +++++++++++++++++++++++++++++++++++-
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index ea22cad..3d9ff27 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -265,6 +265,7 @@ config TAU_AVERAGE
 config QUICC_ENGINE
 	bool
 	select PPC_LIB_RHEAP
+	select GENERIC_GPIO
 	help
 	  The QUICC Engine (QE) is a new generation of communications
 	  coprocessors on Freescale embedded CPUs (akin to CPM in older chips).
diff --git a/arch/powerpc/sysdev/qe_lib/qe_io.c b/arch/powerpc/sysdev/qe_lib/qe_io.c
index e53ea4d..3c9cf65 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_io.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_io.c
@@ -23,6 +23,7 @@
 
 #include <asm/io.h>
 #include <asm/prom.h>
+#include <asm/gpio.h>
 #include <sysdev/fsl_soc.h>
 
 #undef DEBUG
@@ -43,12 +44,23 @@ struct port_regs {
 
 static struct port_regs *par_io = NULL;
 static int num_par_io_ports = 0;
+static spinlock_t *qe_pio_locks;
+
+static int par_io_xlate(struct device_node *np, int index)
+{
+	return __of_parse_gpio_bank_pin(np, index, 32, num_par_io_ports);
+}
+
+static struct of_gpio_chip of_gpio_chip = {
+	.xlate = par_io_xlate,
+};
 
 int par_io_init(struct device_node *np)
 {
 	struct resource res;
 	int ret;
 	const u32 *num_ports;
+	int i;
 
 	/* Map Parallel I/O ports registers */
 	ret = of_address_to_resource(np, 0, &res);
@@ -60,6 +72,18 @@ int par_io_init(struct device_node *np)
 	if (num_ports)
 		num_par_io_ports = *num_ports;
 
+	qe_pio_locks = kzalloc(sizeof(*qe_pio_locks) * num_par_io_ports,
+			       GFP_KERNEL);
+	if (!qe_pio_locks) {
+		iounmap(par_io);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_par_io_ports; i++)
+		spin_lock_init(&qe_pio_locks[i]);
+
+	np->data = &of_gpio_chip;
+
 	return 0;
 }
 
@@ -67,9 +91,12 @@ int par_io_config_pin(u8 port, u8 pin, int dir, int open_drain,
 		      int assignment, int has_irq)
 {
 	u32 pin_mask1bit, pin_mask2bits, new_mask2bits, tmp_val;
+	unsigned long flags;
 
-	if (!par_io)
-		return -1;
+	if (!par_io || port > num_par_io_ports)
+		return -EINVAL;
+
+	spin_lock_irqsave(&qe_pio_locks[port], flags);
 
 	/* calculate pin location for single and 2 bits information */
 	pin_mask1bit = (u32) (1 << (NUM_OF_PINS - (pin + 1)));
@@ -126,6 +153,8 @@ int par_io_config_pin(u8 port, u8 pin, int dir, int open_drain,
 		out_be32(&par_io[port].cppar1, new_mask2bits | tmp_val);
 	}
 
+	spin_unlock_irqrestore(&qe_pio_locks[port], flags);
+
 	return 0;
 }
 EXPORT_SYMBOL(par_io_config_pin);
@@ -133,6 +162,7 @@ EXPORT_SYMBOL(par_io_config_pin);
 int par_io_data_set(u8 port, u8 pin, u8 val)
 {
 	u32 pin_mask, tmp_val;
+	unsigned long flags;
 
 	if (port >= num_par_io_ports)
 		return -EINVAL;
@@ -141,6 +171,8 @@ int par_io_data_set(u8 port, u8 pin, u8 val)
 	/* calculate pin location */
 	pin_mask = (u32) (1 << (NUM_OF_PINS - 1 - pin));
 
+	spin_lock_irqsave(&qe_pio_locks[port], flags);
+
 	tmp_val = in_be32(&par_io[port].cpdata);
 
 	if (val == 0)		/* clear */
@@ -148,10 +180,25 @@ int par_io_data_set(u8 port, u8 pin, u8 val)
 	else			/* set */
 		out_be32(&par_io[port].cpdata, pin_mask | tmp_val);
 
+	spin_unlock_irqrestore(&qe_pio_locks[port], flags);
 	return 0;
 }
 EXPORT_SYMBOL(par_io_data_set);
 
+static inline int par_io_data_get(u8 port, u8 pin)
+{
+	u32 pin_mask;
+
+	if (port >= num_par_io_ports)
+		return -EINVAL;
+	if (pin >= NUM_OF_PINS)
+		return -EINVAL;
+	/* calculate pin location */
+	pin_mask = (u32) (1 << (NUM_OF_PINS - 1 - pin));
+
+	return !!(in_be32(&par_io[port].cpdata) & pin_mask);
+}
+
 int par_io_of_config(struct device_node *np)
 {
 	struct device_node *pio;
@@ -195,6 +242,48 @@ int par_io_of_config(struct device_node *np)
 }
 EXPORT_SYMBOL(par_io_of_config);
 
+int gpio_request(unsigned int gpio, const char *label)
+{
+	if (!qe_pio_locks)
+		return -ENODEV;
+
+	if (gpio / 32 > num_par_io_ports)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpio_request);
+
+int gpio_direction_input(unsigned int gpio)
+{
+	return par_io_config_pin(gpio / 32, gpio % 32, 2, 0, 0, 0);
+}
+EXPORT_SYMBOL_GPL(gpio_direction_input);
+
+int gpio_direction_output(unsigned int gpio, int value)
+{
+	int ret;
+
+	ret = par_io_config_pin(gpio / 32, gpio % 32, 1, 0, 0, 0);
+	if (ret)
+		return ret;
+
+	return par_io_data_set(gpio / 32, gpio % 32, value);
+}
+EXPORT_SYMBOL_GPL(gpio_direction_output);
+
+int gpio_get_value(unsigned int gpio)
+{
+	return par_io_data_get(gpio / 32, gpio % 32);
+}
+EXPORT_SYMBOL_GPL(gpio_get_value);
+
+int gpio_set_value(unsigned int gpio, int value)
+{
+	return par_io_data_set(gpio / 32, gpio % 32, value);
+}
+EXPORT_SYMBOL_GPL(gpio_set_value);
+
 #ifdef DEBUG
 static void dump_par_io(void)
 {
-- 
1.5.2.2

[PATCH RFC 3/7] [POWERPC] CPM2: implement GPIO API

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/platforms/Kconfig    |    1 +
 arch/powerpc/sysdev/cpm2_common.c |  142 +++++++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 3d9ff27..cc2d54e 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -277,6 +277,7 @@ config CPM2
 	default n
 	select CPM
 	select PPC_LIB_RHEAP
+	select GENERIC_GPIO
 	help
 	  The CPM2 (Communications Processor Module) is a coprocessor on
 	  embedded CPUs made by Freescale.  Selecting this option means that
diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c
index 859362f..33b99d4 100644
--- a/arch/powerpc/sysdev/cpm2_common.c
+++ b/arch/powerpc/sysdev/cpm2_common.c
@@ -31,12 +31,14 @@
 #include <linux/param.h>
 #include <linux/string.h>
 #include <linux/mm.h>
+#include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
+#include <asm/gpio.h>
 #include <asm/mpc8260.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
@@ -61,9 +63,62 @@ cpm2_map_t __iomem *cpm2_immr;
 					   of space for CPM as it is larger
 					   than on PQ2 */
 
+static spinlock_t *cpm2_port_locks;
+static int cpm2_num_ports;
+
+static int par_io_xlate(struct device_node *np, int index)
+{
+	return __of_parse_gpio_bank_pin(np, index, 32, cpm2_num_ports);
+}
+
+static struct of_gpio_chip of_gpio_chip = {
+	.xlate = par_io_xlate,
+};
+
+int cpm2_init_par_io(void)
+{
+	int ret;
+	struct device_node *np;
+	const u32 *num_ports;
+	int i;
+
+	np = of_find_node_by_name(NULL, "par_io");
+	if (!np) {
+		ret = -ENOENT;
+		goto err0;
+	}
+
+	num_ports = of_get_property(np, "num-ports", NULL);
+	if (!num_ports) {
+		ret = -ENOENT;
+		goto err1;
+	}
+
+	cpm2_num_ports = *num_ports;
+	cpm2_port_locks = kzalloc(sizeof(*cpm2_port_locks) * cpm2_num_ports,
+				  GFP_KERNEL);
+	if (!cpm2_port_locks) {
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	for (i = 0; i < cpm2_num_ports; i++)
+		spin_lock_init(&cpm2_port_locks[i]);
+
+	np->data = &of_gpio_chip;
+
+	return 0;
+err1:
+	of_node_put(np);
+err0:
+	return ret;
+}
+
 void
 cpm2_reset(void)
 {
+	int ret;
+
 #ifdef CONFIG_PPC_85xx
 	cpm2_immr = ioremap(CPM_MAP_ADDR, CPM_MAP_SIZE);
 #else
@@ -81,6 +136,10 @@ cpm2_reset(void)
 	/* Tell everyone where the comm processor resides.
 	 */
 	cpmp = &cpm2_immr->im_cpm;
+
+	ret = cpm2_init_par_io();
+	if (ret)
+		pr_warning("CPM2 PIO not initialized!\n");
 }
 
 /* Set a baud rate generator.  This needs lots of work.  There are
@@ -444,3 +503,86 @@ void cpm2_set_pin(int port, int pin, int flags)
 	else
 		clrbits32(&iop[port].odr, pin);
 }
+
+int gpio_request(unsigned int gpio, const char *label)
+{
+	if (!cpm2_port_locks)
+		return -ENODEV;
+
+	if (gpio / 32 > cpm2_num_ports)
+		return -EINVAL;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpio_request);
+
+int gpio_direction_input(unsigned int gpio)
+{
+	unsigned long flags;
+	int port = gpio / 32;
+	int pin = gpio % 32;
+
+	spin_lock_irqsave(&cpm2_port_locks[port], flags);
+
+	cpm2_set_pin(port, pin, CPM_PIN_INPUT | CPM_PIN_GPIO);
+
+	spin_unlock_irqrestore(&cpm2_port_locks[port], flags);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpio_direction_input);
+
+int gpio_direction_output(unsigned int gpio, int value)
+{
+	struct cpm2_ioports __iomem *iop =
+		(struct cpm2_ioports __iomem *)&cpm2_immr->im_ioport;
+	int port = gpio / 32;
+	int pin = gpio % 32;
+	unsigned long flags;
+
+	pin = 1 << (31 - pin);
+
+	spin_lock_irqsave(&cpm2_port_locks[port], flags);
+
+	cpm2_set_pin(port, pin, CPM_PIN_OUTPUT | CPM_PIN_GPIO);
+	if (value)
+		setbits32(&iop[port].dat, pin);
+	else
+		clrbits32(&iop[port].dat, pin);
+
+	spin_unlock_irqrestore(&cpm2_port_locks[port], flags);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpio_direction_output);
+
+int gpio_get_value(unsigned int gpio)
+{
+	struct cpm2_ioports __iomem *iop =
+		(struct cpm2_ioports __iomem *)&cpm2_immr->im_ioport;
+	int port = gpio / 32;
+	int pin = gpio % 32;
+
+	pin = 1 << (31 - pin);
+
+	return !!(in_be32(&iop[port].dat) & pin);
+}
+EXPORT_SYMBOL_GPL(gpio_get_value);
+
+int gpio_set_value(unsigned int gpio, int value)
+{
+	struct cpm2_ioports __iomem *iop =
+		(struct cpm2_ioports __iomem *)&cpm2_immr->im_ioport;
+	int port = gpio / 32;
+	int pin = gpio % 32;
+	unsigned long flags;
+
+	pin = 1 << (31 - pin);
+
+	spin_lock_irqsave(&cpm2_port_locks[port], flags);
+	if (value)
+		setbits32(&iop[port].dat, pin);
+	else
+		clrbits32(&iop[port].dat, pin);
+	spin_unlock_irqrestore(&cpm2_port_locks[port], flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpio_set_value);
-- 
1.5.2.2

[PATCH RFC 4/7] [GPIO] Let drivers link if they support GPIO API as an addition

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---

Hello David,

I'm interested in your opinion about that patch. You're also Cc'ed
to patch that using that feature.

I know, currently that patch will conflict with GPIOLIB patches in -mm,
so this is only RFC.

 include/asm-generic/gpio.h |   47 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 2d0aab1..cf76a69 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -1,6 +1,53 @@
 #ifndef _ASM_GENERIC_GPIO_H
 #define _ASM_GENERIC_GPIO_H
 
+#ifndef CONFIG_GENERIC_GPIO
+
+/*
+ * Drivers could be gpio api aware, and at the same time, some
+ * of them can live without it, or support call-backs approach
+ * in addition. Let them link.
+ */
+static inline int gpio_request(unsigned int gpio, const char *label)
+{
+	return -ENOSYS;
+}
+
+static inline void gpio_free(unsigned int gpio)
+{
+}
+
+static inline int gpio_direction_input(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int gpio_direction_output(unsigned int gpio, int value)
+{
+	return -ENOSYS;
+}
+
+static inline int gpio_get_value(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+}
+
+static inline int gpio_to_irq(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	return -ENOSYS;
+}
+
+#endif /* CONFIG_GENERIC_GPIO */
+
 /* platforms that don't directly support access to GPIOs through I2C, SPI,
  * or other blocking infrastructure can use these wrappers.
  */
-- 
1.5.2.2

[PATCH RFC 5/7] [POWERPC] FSL UPM: routines to manage FSL UPMs

[Anton Vorontsov]

Here are few routines needed to manage FSL UPMs properly. FSL UPM
infrastructure also supports lockless variant, to use when we're
pretty sure that only one UPM is used on the board.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/Kconfig          |    7 +++
 arch/powerpc/sysdev/Makefile  |    1 +
 arch/powerpc/sysdev/fsl_upm.c |   74 +++++++++++++++++++++++++++++
 include/asm-powerpc/fsl_upm.h |  102 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 184 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/sysdev/fsl_upm.c
 create mode 100644 include/asm-powerpc/fsl_upm.h

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 596982f..1d47a35 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -464,6 +464,13 @@ config FSL_PCI
  	bool
 	select PPC_INDIRECT_PCI
 
+config FSL_UPM
+	bool
+
+config FSL_UPM_LOCKLESS
+	depends on FSL_UPM
+	bool
+
 # Yes MCA RS/6000s exist but Linux-PPC does not currently support any
 config MCA
 	bool
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 99a77d7..98dbfdd 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_U3_DART)		+= dart_iommu.o
 obj-$(CONFIG_MMIO_NVRAM)	+= mmio_nvram.o
 obj-$(CONFIG_FSL_SOC)		+= fsl_soc.o
 obj-$(CONFIG_FSL_PCI)		+= fsl_pci.o
+obj-$(CONFIG_FSL_UPM)		+= fsl_upm.o
 obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
 obj-$(CONFIG_PPC_BESTCOMM)	+= bestcomm/
diff --git a/arch/powerpc/sysdev/fsl_upm.c b/arch/powerpc/sysdev/fsl_upm.c
new file mode 100644
index 0000000..98d079d
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_upm.c
@@ -0,0 +1,74 @@
+/*
+ * Freescale UPM routines.
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ * Copyright (c) 2007  Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <asm/fsl_upm.h>
+
+int fsl_upm_get_for(struct device_node *node, const char *name,
+		    struct fsl_upm *upm)
+{
+	int ret;
+	struct device_node *lbus;
+	struct resource lbc_res;
+	ptrdiff_t mxmr_offs;
+
+	lbus = of_get_parent(node);
+	if (!lbus) {
+		pr_err("FSL UPM: can't get parent local bus node\n");
+		return -ENOENT;
+	}
+
+	ret = of_address_to_resource(lbus, 0, &lbc_res);
+	if (ret) {
+		pr_err("FSL UPM: can't get parent local bus base\n");
+		return -ENOMEM;
+	}
+
+	switch (name[0]) {
+	case 'A':
+		mxmr_offs = LBC_MAMR;
+		break;
+	case 'B':
+		mxmr_offs = LBC_MBMR;
+		break;
+	case 'C':
+		mxmr_offs = LBC_MCMR;
+		break;
+	default:
+		pr_err("FSL UPM: unknown UPM requested\n");
+		return -EINVAL;
+		break;
+	}
+
+	upm->lbc_base = ioremap_nocache(lbc_res.start,
+					lbc_res.end - lbc_res.start + 1);
+	if (!upm->lbc_base)
+		return -ENOMEM;
+
+	upm->mxmr = upm->lbc_base + mxmr_offs;
+	upm->mar = upm->lbc_base + LBC_MAR;
+
+	return 0;
+}
+
+#ifndef CONFIG_FSL_UPM_LOCKLESS
+spinlock_t upm_lock;
+unsigned long upm_lock_flags;
+
+static int __init fsl_upm_init(void)
+{
+	spin_lock_init(&upm_lock);
+	return 0;
+}
+arch_initcall(fsl_upm_init);
+#endif
diff --git a/include/asm-powerpc/fsl_upm.h b/include/asm-powerpc/fsl_upm.h
new file mode 100644
index 0000000..19f5f9d
--- /dev/null
+++ b/include/asm-powerpc/fsl_upm.h
@@ -0,0 +1,102 @@
+/*
+ * Freescale UPM routines.
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ * Copyright (c) 2007  Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#ifndef __ASM_POWERPC_FSL_UPM
+#define __ASM_POWERPC_FSL_UPM
+
+#include <linux/spinlock.h>
+#include <asm/io.h>
+
+#define LBC_MAR		0x68
+#define LBC_MAMR	0x70
+#define LBC_MBMR	0x74
+#define LBC_MCMR	0x78
+
+#define LBC_MXMR_RUNP	0x30000000
+
+struct fsl_upm {
+	void __iomem *lbc_base;
+	void __iomem *mxmr;
+	void __iomem *mar;
+};
+
+#ifndef CONFIG_FSL_UPM_LOCKLESS
+extern spinlock_t upm_lock;
+extern unsigned long upm_lock_flags;
+
+static inline void upm_do_lock(void)
+{
+	spin_lock_irqsave(&upm_lock, upm_lock_flags);
+}
+
+static inline void upm_do_unlock(void)
+{
+	spin_unlock_irqrestore(&upm_lock, upm_lock_flags);
+}
+#else /* CONFIG_FSL_UPM_LOCKLESS */
+static inline void upm_do_lock(void) {}
+static inline void upm_do_unlock(void) {}
+#endif /* CONFIG_FSL_UPM_LOCKLESS */
+
+extern int fsl_upm_get_for(struct device_node *node, const char *name,
+			   struct fsl_upm *upm);
+
+static inline void fsl_upm_free(struct fsl_upm *upm)
+{
+	iounmap(upm->lbc_base);
+}
+
+static inline int fsl_upm_got(struct fsl_upm *upm)
+{
+	return !!upm->lbc_base;
+}
+
+static inline void fsl_upm_start_pattern(struct fsl_upm *upm, u32 pat_offset)
+{
+	upm_do_lock();
+	out_be32(upm->mxmr, LBC_MXMR_RUNP | pat_offset);
+}
+
+static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
+{
+	out_be32(upm->mxmr, 0x0);
+
+	while (in_be32(upm->mxmr) != 0x0)
+		cpu_relax();
+
+	upm_do_unlock();
+}
+
+static inline int fsl_upm_run_pattern(struct fsl_upm *upm,
+				      void __iomem *io_base,
+				      int width, u32 cmd)
+{
+	out_be32(upm->mar, cmd << (32 - width));
+	switch (width) {
+	case 8:
+		out_8(io_base, 0x0);
+		break;
+	case 16:
+		out_be16(io_base, 0x0);
+		break;
+	case 32:
+		out_be32(io_base, 0x0);
+		break;
+	default:
+		return -EINVAL;
+		break;
+	}
+
+	return 0;
+}
+
+#endif /* __ASM_POWERPC_FSL_UPM */
-- 
1.5.2.2

[PATCH RFC 6/7] [POWERPC][NAND] FSL UPM NAND driver

[Anton Vorontsov]

It's using FSL UPM infrastructure. So far only 8 bit accessors
are implemented.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mtd/nand/Kconfig   |    7 +
 drivers/mtd/nand/Makefile  |    1 +
 drivers/mtd/nand/fsl_upm.c |  310 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 318 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mtd/nand/fsl_upm.c

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 246d451..910b4d5 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -306,4 +306,11 @@ config MTD_ALAUDA
 	  These two (and possibly other) Alauda-based cardreaders for
 	  SmartMedia and xD allow raw flash access.
 
+config MTD_NAND_FSL_UPM
+	tristate "MTD driver for NAND on Freescale UPM"
+	depends on MTD_NAND && FSL_UPM
+	help
+	  Enables support for NAND Flash wired to Freescale processors'
+	  localbus with pre-programmed User-Programmable Machine.
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 3ad6c01..d553ea3 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -29,5 +29,6 @@ obj-$(CONFIG_MTD_NAND_CM_X270)		+= cmx270_nand.o
 obj-$(CONFIG_MTD_NAND_BASLER_EXCITE)	+= excite_nandflash.o
 obj-$(CONFIG_MTD_NAND_PLATFORM)		+= plat_nand.o
 obj-$(CONFIG_MTD_ALAUDA)		+= alauda.o
+obj-$(CONFIG_MTD_NAND_FSL_UPM)		+= fsl_upm.o
 
 nand-objs := nand_base.o nand_bbt.o
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
new file mode 100644
index 0000000..a2bddb0
--- /dev/null
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -0,0 +1,310 @@
+/*
+ * Freescale UPM NAND driver.
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ * Copyright (c) 2007  Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mtd/nand.h>
+#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/mtd.h>
+#include <linux/of_platform.h>
+#include <linux/io.h>
+#include <asm/io.h>
+#include <asm/gpio.h>
+#include <asm/fsl_upm.h>
+
+struct upm_data {
+	struct of_device *ofdev;
+	struct mtd_info mtd;
+	struct nand_chip chip;
+	int last_ctrl;
+#ifdef CONFIG_MTD_PARTITIONS
+	struct mtd_partition *parts;
+#endif
+
+	struct fsl_upm upm;
+
+	int width;
+	int upm_addr_offset;
+	int upm_cmd_offset;
+	void __iomem *io_base;
+	int rnb_gpio;
+	const u32 *wait_pattern;
+	const u32 *wait_write;
+	int chip_delay;
+};
+
+#define to_upm_data(mtd) container_of(mtd, struct upm_data, mtd)
+
+static int upm_chip_ready(struct mtd_info *mtd)
+{
+	struct upm_data *ud = to_upm_data(mtd);
+
+	if (gpio_get_value(ud->rnb_gpio))
+		return 1;
+
+	dev_vdbg(&ud->ofdev->dev, "busy\n");
+	return 0;
+}
+
+static void upm_wait_rnb(struct upm_data *ud)
+{
+	int cnt = 1000000;
+
+	if (ud->rnb_gpio >= 0) {
+		while (--cnt && !upm_chip_ready(&ud->mtd))
+			cpu_relax();
+	}
+
+	if (!cnt)
+		dev_err(&ud->ofdev->dev, "tired waiting for RNB\n");
+}
+
+static void upm_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct upm_data *ud = to_upm_data(mtd);
+
+	if (!(ctrl & ud->last_ctrl)) {
+		fsl_upm_end_pattern(&ud->upm);
+
+		if (cmd == NAND_CMD_NONE)
+			return;
+
+		ud->last_ctrl = ctrl & (NAND_ALE | NAND_CLE);
+	}
+
+	if (ctrl & NAND_CTRL_CHANGE) {
+		if (ctrl & NAND_ALE)
+			fsl_upm_start_pattern(&ud->upm, ud->upm_addr_offset);
+		else if (ctrl & NAND_CLE)
+			fsl_upm_start_pattern(&ud->upm, ud->upm_cmd_offset);
+	}
+
+	fsl_upm_run_pattern(&ud->upm, ud->io_base, ud->width, cmd);
+
+	if (ud->wait_pattern)
+		upm_wait_rnb(ud);
+}
+
+static uint8_t upm_read_byte(struct mtd_info *mtd)
+{
+	struct upm_data *ud = to_upm_data(mtd);
+
+	return in_8(ud->chip.IO_ADDR_R);
+}
+
+static void upm_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct upm_data *ud = to_upm_data(mtd);
+	int i;
+
+	for (i = 0; i < len; i++)
+		buf[i] = in_8(ud->chip.IO_ADDR_R);
+}
+
+static void upm_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
+{
+	struct upm_data *ud = to_upm_data(mtd);
+	int i;
+
+	for (i = 0; i < len; i++) {
+		out_8(ud->chip.IO_ADDR_W, buf[i]);
+		if (ud->wait_write)
+			upm_wait_rnb(ud);
+	}
+}
+
+static int __devinit upm_chip_init(struct upm_data *ud)
+{
+	int ret;
+#ifdef CONFIG_MTD_PARTITIONS
+	static const char *part_types[] = { "cmdlinepart", NULL, };
+#endif
+
+	ud->chip.IO_ADDR_R = ud->io_base;
+	ud->chip.IO_ADDR_W = ud->io_base;
+	ud->chip.cmd_ctrl = upm_cmd_ctrl;
+	ud->chip.chip_delay = ud->chip_delay;
+	ud->chip.read_byte = upm_read_byte;
+	ud->chip.read_buf = upm_read_buf;
+	ud->chip.write_buf = upm_write_buf;
+	ud->chip.ecc.mode = NAND_ECC_SOFT;
+
+	if (ud->rnb_gpio != (unsigned int)-1)
+		ud->chip.dev_ready = upm_chip_ready;
+
+	ud->mtd.priv = &ud->chip;
+	ud->mtd.owner = THIS_MODULE;
+
+	ret = nand_scan(&ud->mtd, 1);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_MTD_PARTITIONS
+	ret = parse_mtd_partitions(&ud->mtd, part_types, &ud->parts, 0);
+	if (ret > 0)
+		return add_mtd_partitions(&ud->mtd, ud->parts, ret);
+#endif
+	return add_mtd_device(&ud->mtd);
+}
+
+static int __devinit upm_chip_probe(struct of_device *ofdev,
+				    const struct of_device_id *ofid)
+{
+	struct upm_data *ud;
+	struct resource io_res;
+	const u32 *prop;
+	int ret;
+
+	ud = kzalloc(sizeof(*ud), GFP_KERNEL);
+	if (!ud)
+		return -ENOMEM;
+
+	ret = of_address_to_resource(ofdev->node, 0, &io_res);
+	if (ret) {
+		dev_err(&ofdev->dev, "can't get IO base\n");
+		goto err;
+	}
+
+	ud->width = io_res.end - io_res.start + 1;
+	if (ud->width != 1) {
+		dev_err(&ofdev->dev, "> 8 bit accessors not implemented\n");
+		ret = -ENOSYS;
+		goto err;
+	}
+	ud->width *= 8;
+
+	prop = of_get_property(ofdev->node, "upm", NULL);
+	if (!prop) {
+		dev_err(&ofdev->dev, "can't get UPM to use\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	ret = fsl_upm_get_for(ofdev->node, (const char *)prop, &ud->upm);
+	if (ret) {
+		dev_err(&ofdev->dev, "can't get FSL UPM\n");
+		goto err;
+	}
+
+	prop = of_get_property(ofdev->node, "upm-addr-offset", NULL);
+	if (!prop) {
+		dev_err(&ofdev->dev, "can't get UPM address offset\n");
+		ret = -EINVAL;
+		goto err;
+	}
+	ud->upm_addr_offset = *prop;
+
+	prop = of_get_property(ofdev->node, "upm-cmd-offset", NULL);
+	if (!prop) {
+		dev_err(&ofdev->dev, "can't get UPM command offset\n");
+		ret = -EINVAL;
+		goto err;
+	}
+	ud->upm_cmd_offset = *prop;
+
+	ud->rnb_gpio = of_get_gpio(ofdev->node, 0);
+	if (ud->rnb_gpio >= 0) {
+		ret = gpio_request(ud->rnb_gpio, ofdev->dev.bus_id);
+		if (ret) {
+			dev_err(&ofdev->dev, "can't request RNB gpio\n");
+			goto err;
+		}
+	} else if (ud->rnb_gpio == -EINVAL) {
+		dev_err(&ofdev->dev, "specified RNB gpio is invalid\n");
+		goto err;
+	}
+
+	ud->io_base = devm_ioremap_nocache(&ofdev->dev, io_res.start,
+					  io_res.end - io_res.start + 1);
+	if (!ud->io_base) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ud->ofdev = ofdev;
+	ud->last_ctrl = NAND_CLE;
+	ud->wait_pattern = of_get_property(ofdev->node, "wait-pattern", NULL);
+	ud->wait_write = of_get_property(ofdev->node, "wait-write", NULL);
+
+	prop = of_get_property(ofdev->node, "chip-delay", NULL);
+	if (prop)
+		ud->chip_delay = *prop;
+	else
+		ud->chip_delay = 50;
+
+	ret = upm_chip_init(ud);
+	if (ret)
+		goto err;
+
+	dev_set_drvdata(&ofdev->dev, ud);
+
+	return 0;
+
+err:
+	if (fsl_upm_got(&ud->upm))
+		fsl_upm_free(&ud->upm);
+
+	if (ud->rnb_gpio >= 0)
+		gpio_free(ud->rnb_gpio);
+
+	kfree(ud);
+
+	return ret;
+}
+
+static int __devexit upm_chip_remove(struct of_device *ofdev)
+{
+	struct upm_data *ud = dev_get_drvdata(&ofdev->dev);
+
+	nand_release(&ud->mtd);
+
+	fsl_upm_free(&ud->upm);
+
+	if (ud->rnb_gpio != (unsigned int)-1)
+		gpio_free(ud->rnb_gpio);
+
+	kfree(ud);
+
+	return 0;
+}
+
+static struct of_device_id of_upm_nand_match[] = {
+	{ .compatible = "fsl,upm-nand" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_upm_nand_match);
+
+static struct of_platform_driver of_upm_chip_driver = {
+	.name		= "fsl_upm_nand",
+	.match_table	= of_upm_nand_match,
+	.probe		= upm_chip_probe,
+	.remove		= __devexit_p(upm_chip_remove),
+};
+
+static int __init upm_nand_init(void)
+{
+	return of_register_platform_driver(&of_upm_chip_driver);
+}
+
+static void __exit upm_nand_exit(void)
+{
+	of_unregister_platform_driver(&of_upm_chip_driver);
+}
+
+module_init(upm_nand_init);
+module_exit(upm_nand_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_DESCRIPTION("Driver for NAND chips working through Freescale "
+		   "LocalBus User-Programmable Machine");
-- 
1.5.2.2

[PATCH RFC 7/7] [POWERPC] MPC8360E-RDK: add support for NAND on UPM

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc836x_rdk.dts     |   24 +++++++++++++++++++++++-
 arch/powerpc/platforms/83xx/Kconfig       |    2 ++
 arch/powerpc/platforms/83xx/mpc836x_rdk.c |    1 +
 3 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc836x_rdk.dts b/arch/powerpc/boot/dts/mpc836x_rdk.dts
index a1b2da6..f57ba53 100644
--- a/arch/powerpc/boot/dts/mpc836x_rdk.dts
+++ b/arch/powerpc/boot/dts/mpc836x_rdk.dts
@@ -115,7 +115,7 @@
 			device_type = "ipic";
 		};
 
-		par_io@1400 {
+		qe_pio: par_io@1400 {
 			reg = <0x1400 0x100>;
 			num-ports = <7>;
 		};
@@ -229,4 +229,26 @@
 			interrupt-parent = <&ipic>;
 		};
 	};
+
+	localbus@e0005000 {
+		#address-cells = <2>;
+		#size-cells = <1>;
+		compatible = "fsl,mpc8360erdk-localbus",
+			     "fsl,mpc8360e-localbus",
+			     "fsl,pq2pro-localbus";
+		reg = <0xe0005000 0xd8>;
+		ranges = <1 0 0x60000000 1>;
+
+		nand-flash@1,0 {
+			compatible = "STMicro,NAND512W3A2BN6E", "fsl,upm-nand";
+			reg = <1 0 1>;
+			upm = "A";
+			upm-addr-offset = <16>;
+			upm-cmd-offset = <8>;
+			gpios = <4 18>;
+			gpio-parent = <&qe_pio>;
+			wait-pattern;
+			wait-write;
+		};
+	};
 };
diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index 98f6358..2fc60c1 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -54,6 +54,7 @@ config MPC836x_RDK
 	bool "Freescale/Logic MPC836x RDK"
 	select DEFAULT_UIMAGE
 	select QUICC_ENGINE
+	select FSL_UPM_LOCKLESS
 	help
 	  This option enables support for the MPC836x RDK Processor Board,
 	  also known as ZOOM PowerQUICC Kit.
@@ -82,4 +83,5 @@ config PPC_MPC836x
 	bool
 	select PPC_UDBG_16550
 	select PPC_INDIRECT_PCI
+	select FSL_UPM
 	default y if MPC836x_MDS || MPC836x_RDK
diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
index be9e2fd..4288e16 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
@@ -27,6 +27,7 @@ static struct of_device_id mpc836x_rdk_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
 	{ .type = "qe", },
+	{ .compatible = "fsl,pq2pro-localbus", },
 	{},
 };
 
-- 
1.5.2.2

Re: [PATCH RFC 4/7] [GPIO] Let drivers link if they support GPIO API as an addition

[David Brownell]

> I'm interested in your opinion about that patch. You're also Cc'ed
> to patch that using that feature.
>
> I know, currently that patch will conflict with GPIOLIB patches in -mm,
> so this is only RFC.

The point of CONFIG_GENERIC_GPIO is to be *the* conditional used to
tell whether that programming interface is available ... starting
from "#include <asm/gpio.h>", and including all gpio_*() calls.

So my first reaction is to not like this patch.  It changes semantics
in an incompatible way.  And AFAICT, needlessly so.

What other options did consider?  Like, why not #ifdef the GPIO parts
of that NAND driver, or have the whole driver depend on GENERIC_GPIO?

- Dave

Re: [PATCH RFC 7/7] [POWERPC] MPC8360E-RDK: add support for NAND on UPM

[David Gibson]

On Mon, Dec 10, 2007 at 11:49:51PM +0300, Anton Vorontsov wrote:
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/boot/dts/mpc836x_rdk.dts     |   24 +++++++++++++++++++++++-
>  arch/powerpc/platforms/83xx/Kconfig       |    2 ++
>  arch/powerpc/platforms/83xx/mpc836x_rdk.c |    1 +
>  3 files changed, 26 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc836x_rdk.dts b/arch/powerpc/boot/dts/mpc836x_rdk.dts
> index a1b2da6..f57ba53 100644
> --- a/arch/powerpc/boot/dts/mpc836x_rdk.dts
> +++ b/arch/powerpc/boot/dts/mpc836x_rdk.dts
> @@ -115,7 +115,7 @@
>  			device_type = "ipic";
>  		};
>  
> -		par_io@1400 {
> +		qe_pio: par_io@1400 {
>  			reg = <0x1400 0x100>;
>  			num-ports = <7>;
>  		};
> @@ -229,4 +229,26 @@
>  			interrupt-parent = <&ipic>;
>  		};
>  	};
> +
> +	localbus@e0005000 {
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		compatible = "fsl,mpc8360erdk-localbus",
> +			     "fsl,mpc8360e-localbus",
> +			     "fsl,pq2pro-localbus";
> +		reg = <0xe0005000 0xd8>;
> +		ranges = <1 0 0x60000000 1>;

The bridge translates a range one byte wide?  I don't think so...

> +
> +		nand-flash@1,0 {
> +			compatible = "STMicro,NAND512W3A2BN6E", "fsl,upm-nand";
> +			reg = <1 0 1>;

The device has a register window *one byte* wide?  That seems
improbable...

-- 
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 RFC 4/7] [GPIO] Let drivers link if they support GPIO API as an addition

[Anton Vorontsov]

On Mon, Dec 10, 2007 at 02:55:24PM -0800, David Brownell wrote:
> > I'm interested in your opinion about that patch. You're also Cc'ed
> > to patch that using that feature.
> >
> > I know, currently that patch will conflict with GPIOLIB patches in -mm,
> > so this is only RFC.
> 
> The point of CONFIG_GENERIC_GPIO is to be *the* conditional used to
> tell whether that programming interface is available ... starting
> from "#include <asm/gpio.h>", and including all gpio_*() calls.
> 
> So my first reaction is to not like this patch.  It changes semantics
> in an incompatible way.  And AFAICT, needlessly so.

Why incompatible? gpio-aware drivers will get -ENOSYS on gpio_request,
thus they will not do anything wrong. GPIO-only drivers could still
depend on GENERIC_GPIO, and their behaviour will not change.

> What other options did consider?  Like, why not #ifdef the GPIO parts
> of that NAND driver,

Hehe, it's used to avoid #ifdef hell indeed. ;-) And no-op wrappers
like that I purposed is widely used to avoid #ifdefs. They will just
optimize away.

> or have the whole driver depend on GENERIC_GPIO?

Well, this is an option, and I was considering this. Further more,
I implemented gpio api for all platforms which currently using fsl
upm nand, so in theory I can add "depends on GENERIC_GPIO".

But the thing is: some drivers don't actually *require* gpios, they
can use it, but at the same time they might live without it.

As for fsl upm nand (the user of that change), it can poll status
from the nand chip instead of looking for RNB gpio. So, strict
depending on GENERIC_GPIO looks weird in that case.


Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[David Gibson]

On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> Hi all,
> 
> Here are patches to support NAND on UPM. That driver is generic for
> all processors with FSL UPMs. And because of that, few more patches are
> needed -- GPIO API and generic FSL UPM functions.
> 
> This is early RFC, all patches are in single thread, so everyone could
> make up overall picture of what is going on. I'll split the thread by
> topics after that RFC.
> 
> Ok, the patches and what they are for:
> 
> 1,2,3,4. GPIO API:
> ------------------
> Usually NAND chips exports RNB (Ready-Not-Busy) pin, so drivers
> could read it and get a hint when chip is ready.
> 
> Often, WP (write protect) pin is also connected to GPIO. So, GPIO API
> is mandatory for generic drivers.
> 
> OF device tree GPIOs bindings are similar to IRQs:
> 
> node {
> 	gpios = <bank pin bank pin bank pin>;
> 	gpio-parent = <&par_io_controller>;
> };
> 
> "bank pin" scheme is controller specific, so controllers that want
> to implement flat mappings or any other could do so.

It might be safest to do as is done for interrupts, and not define the
internal format at all.  The gpio within the gpio controller would be
defined by a gpio-descriptor whose format is determined by the
controller.  You would need to add a #gpio-cells property in this
case, so you can at least determine the size of the descriptors
associated with a particular gpio controller.

-- 
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 RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

On Tue, Dec 11, 2007 at 10:04:53AM +1100, David Gibson wrote:
> On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> > Hi all,
> > 
> > Here are patches to support NAND on UPM. That driver is generic for
> > all processors with FSL UPMs. And because of that, few more patches are
> > needed -- GPIO API and generic FSL UPM functions.
> > 
> > This is early RFC, all patches are in single thread, so everyone could
> > make up overall picture of what is going on. I'll split the thread by
> > topics after that RFC.
> > 
> > Ok, the patches and what they are for:
> > 
> > 1,2,3,4. GPIO API:
> > ------------------
> > Usually NAND chips exports RNB (Ready-Not-Busy) pin, so drivers
> > could read it and get a hint when chip is ready.
> > 
> > Often, WP (write protect) pin is also connected to GPIO. So, GPIO API
> > is mandatory for generic drivers.
> > 
> > OF device tree GPIOs bindings are similar to IRQs:
> > 
> > node {
> > 	gpios = <bank pin bank pin bank pin>;
> > 	gpio-parent = <&par_io_controller>;
> > };
> > 
> > "bank pin" scheme is controller specific, so controllers that want
> > to implement flat mappings or any other could do so.
> 
> It might be safest to do as is done for interrupts, and not define the
> internal format at all.

This is how it is done already. Take a look into second and third patches:

+static int par_io_xlate(struct device_node *np, int index)
+{
+       return __of_parse_gpio_bank_pin(np, index, 32, num_par_io_ports);
+}
+
+static struct of_gpio_chip of_gpio_chip = {
+       .xlate = par_io_xlate,
+};

__of_parse_gpio_bank_pin() is helper function, I just factored
it out, because both QE and CPM2 using same format.

But generally, controllers are encouraged to do their own xlates.

Or am I missing the point?

> The gpio within the gpio controller would be
> defined by a gpio-descriptor whose format is determined by the
> controller.  You would need to add a #gpio-cells property in this
> case, so you can at least determine the size of the descriptors
> associated with a particular gpio controller.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 7/7] [POWERPC] MPC8360E-RDK: add support for NAND on UPM

[Anton Vorontsov]

On Tue, Dec 11, 2007 at 10:03:21AM +1100, David Gibson wrote:
> On Mon, Dec 10, 2007 at 11:49:51PM +0300, Anton Vorontsov wrote:
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> >  arch/powerpc/boot/dts/mpc836x_rdk.dts     |   24 +++++++++++++++++++++++-
> >  arch/powerpc/platforms/83xx/Kconfig       |    2 ++
> >  arch/powerpc/platforms/83xx/mpc836x_rdk.c |    1 +
> >  3 files changed, 26 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/powerpc/boot/dts/mpc836x_rdk.dts b/arch/powerpc/boot/dts/mpc836x_rdk.dts
> > index a1b2da6..f57ba53 100644
> > --- a/arch/powerpc/boot/dts/mpc836x_rdk.dts
> > +++ b/arch/powerpc/boot/dts/mpc836x_rdk.dts
> > @@ -115,7 +115,7 @@
> >  			device_type = "ipic";
> >  		};
> >  
> > -		par_io@1400 {
> > +		qe_pio: par_io@1400 {
> >  			reg = <0x1400 0x100>;
> >  			num-ports = <7>;
> >  		};
> > @@ -229,4 +229,26 @@
> >  			interrupt-parent = <&ipic>;
> >  		};
> >  	};
> > +
> > +	localbus@e0005000 {
> > +		#address-cells = <2>;
> > +		#size-cells = <1>;
> > +		compatible = "fsl,mpc8360erdk-localbus",
> > +			     "fsl,mpc8360e-localbus",
> > +			     "fsl,pq2pro-localbus";
> > +		reg = <0xe0005000 0xd8>;
> > +		ranges = <1 0 0x60000000 1>;
> 
> The bridge translates a range one byte wide?  I don't think so...

Nope, 4KB min, IIRC.

> > +
> > +		nand-flash@1,0 {
> > +			compatible = "STMicro,NAND512W3A2BN6E", "fsl,upm-nand";
> > +			reg = <1 0 1>;
> 
> The device has a register window *one byte* wide?  That seems
> improbable...

Here, actually yes. The device just 8 bits wide. Reading next 8 bits
will return the same value, obviously. ;-)

But points taken. I should not derivate chip width from the
ranges/reg. This looks unusual indeed, will implement chip-width
property.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[David Gibson]

On Tue, Dec 11, 2007 at 02:10:52AM +0300, Anton Vorontsov wrote:
> On Tue, Dec 11, 2007 at 10:04:53AM +1100, David Gibson wrote:
> > On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> > > Hi all,
> > > 
> > > Here are patches to support NAND on UPM. That driver is generic for
> > > all processors with FSL UPMs. And because of that, few more patches are
> > > needed -- GPIO API and generic FSL UPM functions.
> > > 
> > > This is early RFC, all patches are in single thread, so everyone could
> > > make up overall picture of what is going on. I'll split the thread by
> > > topics after that RFC.
> > > 
> > > Ok, the patches and what they are for:
> > > 
> > > 1,2,3,4. GPIO API:
> > > ------------------
> > > Usually NAND chips exports RNB (Ready-Not-Busy) pin, so drivers
> > > could read it and get a hint when chip is ready.
> > > 
> > > Often, WP (write protect) pin is also connected to GPIO. So, GPIO API
> > > is mandatory for generic drivers.
> > > 
> > > OF device tree GPIOs bindings are similar to IRQs:
> > > 
> > > node {
> > > 	gpios = <bank pin bank pin bank pin>;
> > > 	gpio-parent = <&par_io_controller>;
> > > };
> > > 
> > > "bank pin" scheme is controller specific, so controllers that want
> > > to implement flat mappings or any other could do so.
> > 
> > It might be safest to do as is done for interrupts, and not define the
> > internal format at all.
> 
> This is how it is done already. Take a look into second and third patches:
> 
> +static int par_io_xlate(struct device_node *np, int index)
> +{
> +       return __of_parse_gpio_bank_pin(np, index, 32, num_par_io_ports);
> +}
> +
> +static struct of_gpio_chip of_gpio_chip = {
> +       .xlate = par_io_xlate,
> +};
> 
> __of_parse_gpio_bank_pin() is helper function, I just factored
> it out, because both QE and CPM2 using same format.
> 
> But generally, controllers are encouraged to do their own xlates.
> 
> Or am I missing the point?

Right, but you are assuming a fixed size (2 cells?) for the bank/pin
information, arent' you - I didn't see any #gpio-cells or similar
looking property.  I'm wondering if some gpio controllers might want
less (only one bank) or more (bank, pin, polarity/flags perhaps).

-- 
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 RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

On Tue, Dec 11, 2007 at 11:36:47AM +1100, David Gibson wrote:
[...]
> > > > OF device tree GPIOs bindings are similar to IRQs:
> > > > 
> > > > node {
> > > > 	gpios = <bank pin bank pin bank pin>;
> > > > 	gpio-parent = <&par_io_controller>;
> > > > };
> > > > 
> > > > "bank pin" scheme is controller specific, so controllers that want
> > > > to implement flat mappings or any other could do so.
> > > 
> > > It might be safest to do as is done for interrupts, and not define the
> > > internal format at all.
> > 
> > This is how it is done already. Take a look into second and third patches:
> > 
> > +static int par_io_xlate(struct device_node *np, int index)
> > +{
> > +       return __of_parse_gpio_bank_pin(np, index, 32, num_par_io_ports);
> > +}
> > +
> > +static struct of_gpio_chip of_gpio_chip = {
> > +       .xlate = par_io_xlate,
> > +};
> > 
> > __of_parse_gpio_bank_pin() is helper function, I just factored
> > it out, because both QE and CPM2 using same format.
> > 
> > But generally, controllers are encouraged to do their own xlates.
> > 
> > Or am I missing the point?
> 
> Right, but you are assuming a fixed size (2 cells?)

Nope, of_get_gpio() doesn't assume that, but per-controller's .xlate()
is free to assume (and check) that, right.

> for the bank/pin
> information, arent' you - I didn't see any #gpio-cells or similar
> looking property.

Well, per-controller .xlate() is checking for correct cell count
(just reminding -- __of_parse_gpio_bank_pin is helper function,
controllers are free to implement their own):

int __of_parse_gpio_bank_pin(struct device_node *np, int index,
                             int bank_width, int max_bank)
{
...
        gpios = of_get_property(np, "gpios", &len);
        len /= sizeof(u32);

        if (len < 2 || len % 2 || index > len / 2 - 1)
                return -EINVAL;
...
}

On the other hand, I might indeed introduce #gpio-cells, and move
that check into generic of_get_gpio(), which will use #gpio-cells.
I think this is good idea anyway, so I'll just do it. ;-)

> I'm wondering if some gpio controllers might want
> less (only one bank) or more (bank, pin, polarity/flags perhaps).

Yup, they might. With #gpio-cells or without. The matter of where
to place sanity checks.


Much thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 3/7] [POWERPC] CPM2: implement GPIO API

[Jochen Friedrich]

Hi Anton,

> +int gpio_direction_input(unsigned int gpio)
> +{
> +	unsigned long flags;
> +	int port = gpio / 32;
> +	int pin = gpio % 32;
> +
> +	spin_lock_irqsave(&cpm2_port_locks[port], flags);
> +
> +	cpm2_set_pin(port, pin, CPM_PIN_INPUT | CPM_PIN_GPIO);

> +int gpio_direction_output(unsigned int gpio, int value)
> +{
> +	struct cpm2_ioports __iomem *iop =
> +		(struct cpm2_ioports __iomem *)&cpm2_immr->im_ioport;
> +	int port = gpio / 32;
> +	int pin = gpio % 32;
> +	unsigned long flags;
> +
> +	pin = 1 << (31 - pin);
> +
> +	spin_lock_irqsave(&cpm2_port_locks[port], flags);
> +
> +	cpm2_set_pin(port, pin, CPM_PIN_OUTPUT | CPM_PIN_GPIO);

You seem to do the pin -> bitmask conversation twice in gpio_direction_output().

cpm2_set_pin() must be executed before pin = 1 << (31 - pin);

Thanks,
Jochen

Re: [PATCH RFC 3/7] [POWERPC] CPM2: implement GPIO API

[Anton Vorontsov]

On Wed, Dec 12, 2007 at 04:49:46PM +0100, Jochen Friedrich wrote:
> Hi Anton,
> 
> > +int gpio_direction_input(unsigned int gpio)
> > +{
> > +	unsigned long flags;
> > +	int port = gpio / 32;
> > +	int pin = gpio % 32;
> > +
> > +	spin_lock_irqsave(&cpm2_port_locks[port], flags);
> > +
> > +	cpm2_set_pin(port, pin, CPM_PIN_INPUT | CPM_PIN_GPIO);
> 
> > +int gpio_direction_output(unsigned int gpio, int value)
> > +{
> > +	struct cpm2_ioports __iomem *iop =
> > +		(struct cpm2_ioports __iomem *)&cpm2_immr->im_ioport;
> > +	int port = gpio / 32;
> > +	int pin = gpio % 32;
> > +	unsigned long flags;
> > +
> > +	pin = 1 << (31 - pin);
> > +
> > +	spin_lock_irqsave(&cpm2_port_locks[port], flags);
> > +
> > +	cpm2_set_pin(port, pin, CPM_PIN_OUTPUT | CPM_PIN_GPIO);
> 
> You seem to do the pin -> bitmask conversation twice in gpio_direction_output().
> 
> cpm2_set_pin() must be executed before pin = 1 << (31 - pin);

Yup, found this just after posting. ;-) I've tried to move as much as
possible out of spinlocked section, but done it obviously wrong.
Will fix.

Anyhow, much thanks for looking into this.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Scott Wood]

On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> 6. FSL UPM NAND driver:
> -----------------------
> It's using FSL UPM functions and GPIO API. It does not implement
> device tree partitions parsing. That issue is completely other
> matter, that is, I have to factor out parsing functions from
> physmap_of.c. This desires separate patch on top of the whole
> series. If anyone currently working on this, let me know.

I've got a patch coming for that.

-Scott

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Scott Wood]

On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> 5 - FSL UPM infrastructure:
> ---------------------------
> UPM address register is shared among UPMs, so we have to do
> proper locking. On the other hand, if we know that specific
> board using only one UPM we could bypass locking, and gain some
> performance win.

Not enough to be worth the complexity compared to the overhead of NAND
access -- especially in the likely case of a non-SMP build.

-Scott

Re: [PATCH RFC 1/7] [POWERPC] Implement GPIO API embryo

[Scott Wood]

On Mon, Dec 10, 2007 at 11:48:25PM +0300, Anton Vorontsov wrote:
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 232c298..596982f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -73,6 +73,10 @@ config GENERIC_FIND_NEXT_BIT
>  	bool
>  	default y
>  
> +config GENERIC_GPIO
> +	bool
> +	default n
> +

default n is the default.  No need to explicitly state it.

> +
> +int __of_parse_gpio_bank_pin(struct device_node *np, int index,
> +			     int bank_width, int max_bank)

Why the leading underscores?

> diff --git a/include/asm-powerpc/gpio.h b/include/asm-powerpc/gpio.h
> new file mode 100644
> index 0000000..f7513ff
> --- /dev/null
> +++ b/include/asm-powerpc/gpio.h
> @@ -0,0 +1,51 @@
> +/*
> + * Generic GPIO API implementation for PowerPC.

Is there anything really powerpc specific here, or should it go under
drivers/of?

-Scott

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Scott Wood]

Anton Vorontsov wrote:
> On Wed, Dec 12, 2007 at 10:40:35AM -0600, Scott Wood wrote:
>> Not enough to be worth the complexity compared to the overhead of NAND
>> access -- especially in the likely case of a non-SMP build.
> 
> I'm allowing UPM access from the IRQ handlers (because nothing prevents
> this, so why deny?). Thus locks are needed even on non-SMP build,

No, it just needs to disable interrupts.  Which is what locks do on 
non-SMP.  The overhead of this is not worth 30 lines of code to avoid.

-Scott

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

On Wed, Dec 12, 2007 at 10:40:35AM -0600, Scott Wood wrote:
> On Mon, Dec 10, 2007 at 11:47:05PM +0300, Anton Vorontsov wrote:
> > 5 - FSL UPM infrastructure:
> > ---------------------------
> > UPM address register is shared among UPMs, so we have to do
> > proper locking. On the other hand, if we know that specific
> > board using only one UPM we could bypass locking, and gain some
> > performance win.
> 
> Not enough to be worth the complexity compared to the overhead of NAND
> access -- especially in the likely case of a non-SMP build.

I'm allowing UPM access from the IRQ handlers (because nothing prevents
this, so why deny?). Thus locks are needed even on non-SMP build, on
UP they aren't thrown away. Lockless variant occupy less than 30 lines of
code, so I'd rather keep it.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 3/7] [POWERPC] CPM2: implement GPIO API

[Scott Wood]

On Mon, Dec 10, 2007 at 11:48:45PM +0300, Anton Vorontsov wrote:
> +EXPORT_SYMBOL_GPL(gpio_request);
> +EXPORT_SYMBOL_GPL(gpio_direction_input);
> +EXPORT_SYMBOL_GPL(gpio_direction_output);
> +EXPORT_SYMBOL_GPL(gpio_get_value);
> +EXPORT_SYMBOL_GPL(gpio_set_value);

These look like APIs, not internal functions.  Why the GPL DRM?

-Scott

Re: [PATCH RFC 7/7] [POWERPC] MPC8360E-RDK: add support for NAND on UPM

[Scott Wood]

On Mon, Dec 10, 2007 at 11:49:51PM +0300, Anton Vorontsov wrote:
> +	localbus@e0005000 {
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		compatible = "fsl,mpc8360erdk-localbus",
> +			     "fsl,mpc8360e-localbus",
> +			     "fsl,pq2pro-localbus";

The board name shouldn't be in there.

> +		reg = <0xe0005000 0xd8>;
> +		ranges = <1 0 0x60000000 1>;
> +
> +		nand-flash@1,0 {
> +			compatible = "STMicro,NAND512W3A2BN6E", "fsl,upm-nand";

Technically, if the vendor part begins with a capital letter it's supposed
to be either a stock symbol or an OUI.

-Scott

Re: [PATCH RFC 1/7] [POWERPC] Implement GPIO API embryo

[Anton Vorontsov]

On Wed, Dec 12, 2007 at 10:48:47AM -0600, Scott Wood wrote:
> On Mon, Dec 10, 2007 at 11:48:25PM +0300, Anton Vorontsov wrote:
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 232c298..596982f 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -73,6 +73,10 @@ config GENERIC_FIND_NEXT_BIT
> >  	bool
> >  	default y
> >  
> > +config GENERIC_GPIO
> > +	bool
> > +	default n
> > +
> 
> default n is the default.  No need to explicitly state it.

Thanks, fill fix.

> > +
> > +int __of_parse_gpio_bank_pin(struct device_node *np, int index,
> > +			     int bank_width, int max_bank)
> 
> Why the leading underscores?

This is low-level helper/library function, used by gpio controllers to
parse "bank pin" gpio = <> scheme. To denote that this isn't some kind
of API leading underscores are used.

> > diff --git a/include/asm-powerpc/gpio.h b/include/asm-powerpc/gpio.h
> > new file mode 100644
> > index 0000000..f7513ff
> > --- /dev/null
> > +++ b/include/asm-powerpc/gpio.h
> > @@ -0,0 +1,51 @@
> > +/*
> > + * Generic GPIO API implementation for PowerPC.
> 
> Is there anything really powerpc specific here, or should it go under
> drivers/of?

Because <asm/gpio.h> should be used by all architectures to match
generic GPIO API. asm-sh/gpio.h asm-arm/gpio.h asm-avr32/gpio.h
and so on.

The only thing I can place into of.h is
- - - -
/*
 * OF specific gpio_chip handler.
 */
struct of_gpio_chip {
        int (*xlate)(struct device_node *np, int index);
};
- - - -

But frankly speaking, of_gpio_chip could be architecture-specific
too.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

On Wed, Dec 12, 2007 at 10:54:29AM -0600, Scott Wood wrote:
> Anton Vorontsov wrote:
> >On Wed, Dec 12, 2007 at 10:40:35AM -0600, Scott Wood wrote:
> >>Not enough to be worth the complexity compared to the overhead of NAND
> >>access -- especially in the likely case of a non-SMP build.
> >
> >I'm allowing UPM access from the IRQ handlers (because nothing prevents
> >this, so why deny?). Thus locks are needed even on non-SMP build,
> 
> No, it just needs to disable interrupts.
> Which is what locks do on non-SMP.
> The overhead of this is not worth 30 lines of code to avoid.

Well, speaking of overhead. There could be a lot of fsl_upm_run_pattern
calls between _start and _end. In NAND case it's maximum 3, plus
they're indirect (i.e. NAND layer calls them via pointers to cmdfunc,
and cmdfunc calls run_patterns). Each out_X is another sync, and all
that time we're holding a lock with local IRQs disabled.

fsl upm infrastructure isn't only for NAND though, so I might imagine
use cases when there might be more run_patterns between start and end.

As the compromise I might suggest this: forbid pattern_start/pattern_end
from the ISRs (by marking them as might_sleep()), and replace _irqsave
spinlock with simple spinlock.

That way on UP we don't lose anything, but on SMP we still have an
overhead in case of single used UPM. :-/

Given that, personally I'd want to lockless variant to stay.

So, you still want to get rid of it?


Much thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Scott Wood]

Anton Vorontsov wrote:
> As the compromise I might suggest this: forbid pattern_start/pattern_end
> from the ISRs (by marking them as might_sleep()), and replace _irqsave
> spinlock with simple spinlock.

No, you cannot use a bare spinlock with IRQs enabled.  You'll deadlock 
on SMP, and you'll have races with preemption enabled.

> Given that, personally I'd want to lockless variant to stay.
> 
> So, you still want to get rid of it?

Yes, in the absence of benchmarking that shows it makes a real 
difference.  Premature optimization being the root of all evil, and what 
not.

-Scott

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Scott Wood]

Scott Wood wrote:
> Anton Vorontsov wrote:
>> As the compromise I might suggest this: forbid pattern_start/pattern_end
>> from the ISRs (by marking them as might_sleep()), and replace _irqsave
>> spinlock with simple spinlock.
> 
> No, you cannot use a bare spinlock with IRQs enabled.  You'll deadlock 
> on SMP, and you'll have races with preemption enabled.

Sorry, brain fart...  obviously, with preemption on it'd disable 
preemption even with a plain spinlock, and on SMP without preemption 
it's not an issue.

Still, all you're getting rid of is the MSR twiddling, which should be 
pretty minor.

-Scott

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Chris Fester]

On Mon, 2007-12-10 at 23:47 +0300, Anton Vorontsov wrote:
> Hi all,
> 
> Here are patches to support NAND on UPM. That driver is generic for
> all processors with FSL UPMs. And because of that, few more patches are
> needed -- GPIO API and generic FSL UPM functions.
> I would appreciate any comments and suggestions, thanks!

Hi Anton,

Apologies for not getting back sooner to comment...

We've been using a home-brewed UPM driver for our NAND for a while now.
There's several issues we've hit that I figured I would share.  The
issues involve deficiencies of both the UPM and the NAND part itself.

We modeled our driver loosely after the suggestions in this white paper:
http://www.freescale.com/files/32bit/doc/white_paper/NANDFLASHWP.pdf?fpsp=1&WT_TYPE=WhitePapers&WT_VENDOR=FREESCALE&WT_FILE_FORMAT=pdf&WT_ASSET=Documentation
We also connected the NAND according to the suggestions in the above
document.  

Something that you should be made aware of right away about the UPM -
you do not have control of the logic lines all of the time.  You only
can control them during a transaction (in our case, a byte transaction).
In other words, the time that is between reading/writing bytes, or
putting the address on the bus, or asserting commands you are not able
to hold the lines to a certain state.  Most of the control lines coming
from the UPM default to a logic 1 when unused (I believe only one
defaults to a logic 0).

The first issue is timings between versions of processors.  We've got
one board using an MPC8349 running a 1Gb ST micro part (NAND01G-B).
Another board is an MPC8347 using the same 1Gb ST micro part.  After
much time analyzing logic analyzer captures we found that write pulses
with the 8349 had to be shifted to the left a half clock in order to
properly latch the data on the bus.  This was unexpected given the two
boards are very similar and the processors are in the same family.  I
suppose the fix was easy enough, we just had to make a new version of
the microcode.  What's interesting is that Freescale said that there's
nothing different between the two processors' UPMs.  Yet there is a
distinct difference between local bus behavior on the 2 cpus, our LA
captures prove it.

The second issue was with interrupts during reads.  We noticed that some
of our boards experienced ECC errors while reading the NAND.  Logic
analyzer captures and debugging revealed that when an interrupt happened
while the driver's read code was executing, the UPM went into an idle
state, and the chip select of the part went high.  This may not be a
problem for other NAND parts, but for our ST Micro parts, the spec says
that if CS is held high for more than 100us, the part will exit page
read mode.  The default state of the UPM pins are high, so when the UPM
is idle all the signals are at logic 1.  

Anyways, when the interrupt had completed, or driver wasn't aware that
an interrupt had occurred, so it continued to do it's read loop.  The ST
Micro part was now in an undefined state.  The page read appeared to
continue just fine, but we almost always had one or more bits corrupted
for the very first resuming data byte.  

The fix that we ultimately did was to move the chip select control from
the UPM to a GPIO from the processor.  This forced the chip to never
"sleep" during an interrupt, because we could keep the chip select low
for the entire page read transaction.  This brought up other minor
issues.  You must make sure that your boot loader (U-Boot in our case)
sets the GPIO data BEFORE it sets the GPIO direction.  Otherwise the
NAND will automatically enter a start up read condition that will drive
the local bus, causing anything else on the local bus (such as a NOR
flash) to be useless...

The latest version of the app note online (link above) does not mention
the problem you may experience with interrupts.  I requested from our
Freescale support contact that Freescale document this fact and make it
available to others such that they won't bump into the same problems we
did!

The THIRD issue we had (actually, are having) is very related to our
second issue.  Our customer now wants a 2Gb NAND on our board.  We
swapped the chip out.  We included one more address byte transaction
that is needed with the larger part in the driver.  And it doesn't work.
The part is considered to be the "bigger brother" to the 1Gb part, but
apparently enough of it's state machine has changed that it doesn't
tolerate the AL and CL lines going high while chip select is low.
Incidentally, when debugging our second issue we were told by ST Micro
support that the only time AL and CL were sampled by the NAND was when
W# and R# were driven low.  Had we known that the 2Gb part wouldn't have
worked, we would have done much more major reworking to the NAND
subsystem when we respun our board.

This mail isn't meant to be a rant.  Everyone knows that this kind of
low-level hardware/software work is difficult.  This mail is meant to
caution you against using the NAND with the UPM.  Freescale may say that
it works, and present you a pretty looking white paper, but it's really
kludgy and you are pretty guaranteed to have problems.  Please encourage
your board designers to hook the NAND to some sort of CPLD device rather
than the UPM.  At least with a CPLD you have some real control over the
state of the control lines.

If you for whatever reason are stuck with the UPM...
1.)  Get your hardware people to stick inverters on the AL and CL lines.
Adjust your UPM microcode accordingly.
2.)  Put your CS under control of a GPIO pin.
3.)  Buy a nice logic analyzer, because you're going to need it!!!

I hope this helps someone out there... I've been spending a lot of time
on-and-off on this.  Every time we think we have it right one little
seemingly benign change throws the whole thing out the window...

Okie, happy coding,

Chris Fester

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[Anton Vorontsov]

On Wed, Dec 12, 2007 at 03:06:54PM -0600, Scott Wood wrote:
[...]
> >Given that, personally I'd want to lockless variant to stay.
> >
> >So, you still want to get rid of it?
> 
> Yes, in the absence of benchmarking that shows it makes a real 
> difference.

Benchmarking shows no difference in throughput between lock and
lockless variants. This is expected of course, flashes are slow,
so I can't really benchmark anything with it.

> Premature optimization being the root of all evil, and what 
> not.

;-) Will remove it, thanks!

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 0/7] "NAND on UPM" and related patches

[David Gibson]

On Wed, Dec 12, 2007 at 03:47:32PM +0300, Anton Vorontsov wrote:
> On Tue, Dec 11, 2007 at 11:36:47AM +1100, David Gibson wrote:
> [...]
> > > > > OF device tree GPIOs bindings are similar to IRQs:
> > > > > 
> > > > > node {
> > > > > 	gpios = <bank pin bank pin bank pin>;
> > > > > 	gpio-parent = <&par_io_controller>;
> > > > > };
> > > > > 
> > > > > "bank pin" scheme is controller specific, so controllers that want
> > > > > to implement flat mappings or any other could do so.
> > > > 
> > > > It might be safest to do as is done for interrupts, and not define the
> > > > internal format at all.
> > > 
> > > This is how it is done already. Take a look into second and third patches:
> > > 
> > > +static int par_io_xlate(struct device_node *np, int index)
> > > +{
> > > +       return __of_parse_gpio_bank_pin(np, index, 32, num_par_io_ports);
> > > +}
> > > +
> > > +static struct of_gpio_chip of_gpio_chip = {
> > > +       .xlate = par_io_xlate,
> > > +};
> > > 
> > > __of_parse_gpio_bank_pin() is helper function, I just factored
> > > it out, because both QE and CPM2 using same format.
> > > 
> > > But generally, controllers are encouraged to do their own xlates.
> > > 
> > > Or am I missing the point?
> > 
> > Right, but you are assuming a fixed size (2 cells?)
> 
> Nope, of_get_gpio() doesn't assume that, but per-controller's .xlate()
> is free to assume (and check) that, right.

Ah, ok, I see.  I think doing this without an explicit '#gpio-cells'
is not a great idea, though.  It would be nice to be able to sanity
check the "gpio descriptors" without having to deduce the correct
format from the controller's "compatible" property.

-- 
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 RFC 4/7] [GPIO] Let drivers link if they support GPIO API as an addition

[Anton Vorontsov]

On Fri, Feb 22, 2008 at 03:42:03PM -0800, David Brownell wrote:
> On Monday 10 December 2007, Anton Vorontsov wrote:
> > On Mon, Dec 10, 2007 at 02:55:24PM -0800, David Brownell wrote:
> 
> > > The point of CONFIG_GENERIC_GPIO is to be *the* conditional used to
> > > tell whether that programming interface is available ... starting
> > > from "#include <asm/gpio.h>", and including all gpio_*() calls.
> > > 
> > > So my first reaction is to not like this patch.  It changes semantics
> > > in an incompatible way.  And AFAICT, needlessly so.
> > 
> > Why incompatible? gpio-aware drivers will get -ENOSYS on gpio_request,
> > thus they will not do anything wrong. GPIO-only drivers could still
> > depend on GENERIC_GPIO, and their behaviour will not change.
> 
> If you still want this, I think a better approach would be:
> 
>   http://marc.info/?l=linux-kernel&m=120295461410848&w=2
> 
> That is, #include <linux/gpio.h> and have *that* do the relevant
> switch, based on GENERIC_GPIO.  No semantic changes at all, if
> one discounts the implicit switch to <linux/gpio.h> (important
> for platforms that don't *have* any <asm/gpio.h> header), which
> won't affect any existing code.
> 
> So your NAND code could use that, and work equally well on
> SOC variants that have generic GPIOs and those that don't.
> 
> Comments?

I like it. :-) Thanks.

p.s. would be great to see this in 2.6.25, so we can start use
this include for the new code.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [PATCH RFC 4/7] [GPIO] Let drivers link if they support GPIO API as an addition

[David Brownell]

On Monday 10 December 2007, Anton Vorontsov wrote:
> On Mon, Dec 10, 2007 at 02:55:24PM -0800, David Brownell wrote:

> > The point of CONFIG_GENERIC_GPIO is to be *the* conditional used to
> > tell whether that programming interface is available ... starting
> > from "#include <asm/gpio.h>", and including all gpio_*() calls.
> > 
> > So my first reaction is to not like this patch.  It changes semantics
> > in an incompatible way.  And AFAICT, needlessly so.
> 
> Why incompatible? gpio-aware drivers will get -ENOSYS on gpio_request,
> thus they will not do anything wrong. GPIO-only drivers could still
> depend on GENERIC_GPIO, and their behaviour will not change.

If you still want this, I think a better approach would be:

  http://marc.info/?l=linux-kernel&m=120295461410848&w=2

That is, #include <linux/gpio.h> and have *that* do the relevant
switch, based on GENERIC_GPIO.  No semantic changes at all, if
one discounts the implicit switch to <linux/gpio.h> (important
for platforms that don't *have* any <asm/gpio.h> header), which
won't affect any existing code.

So your NAND code could use that, and work equally well on
SOC variants that have generic GPIOs and those that don't.

Comments?

- Dave