[RFC] Dummy GPIO driver for use with SPI

[RFC] Dummy GPIO driver for use with SPI

[Steven A. Falco]

This patch adds a dummy GPIO driver, which is useful for SPI devices
that do not have a physical chip select.

Signed-off-by: Steven A. Falco <sfalco@harris.com>
---
The SPI subsystem requires a chip-select for each connected slave
device.  I have a custom board with an Atmel "co-processor".  This part
is reprogrammed via SPI, so it needs a chip select to satisfy the SPI
subsystem, but my hardware does not have a physical CS connected.

I could waste a "no-connect" GPIO pin, but I'd rather not.  So, I've
written a dummy GPIO driver, which behaves exactly like a real GPIO
device, but with no underlying hardware.  This could also be useful
as a template for real GPIO drivers.

I use the following dts entry:

	GPIO3: dummy@ef500000 {
		compatible = "linux,dummy-gpio";
		reg = <ef500000 1>;
		gpio-controller;
		#gpio-cells = <2>;
	};

Because the device is registered via of_mm_gpiochip_add, I have to
provide it a resource.  The driver will ioremap this resource, but will
not touch it.  So, I simply chose a reserved address in the 440EPx
address space, to satisfy that requirement.  Any unused, ioremap-able
address can be used for this purpose.

I've put this into the platforms/44x/Kconfig for this example, but it
probably ought to go into sysdev/Kconfig, if it is of general interest.

Comments invited.

	Steve


 arch/powerpc/platforms/44x/Kconfig |    8 +++
 arch/powerpc/sysdev/Makefile       |    1 +
 arch/powerpc/sysdev/dummy_gpio.c   |   98 ++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/sysdev/dummy_gpio.c

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index eec5cb4..5b52956 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -143,6 +143,14 @@ config XILINX_VIRTEX440_GENERIC_BOARD
 	  Most Virtex 5 designs should use this unless it needs to do some
 	  special configuration at board probe time.
 
+config DUMMY_GPIO
+	bool "Dummy GPIO support"
+	depends on 44x
+	select ARCH_REQUIRE_GPIOLIB
+	select GENERIC_GPIO
+	help
+	  Enable gpiolib support for dummy devices
+
 config PPC4xx_GPIO
 	bool "PPC4xx GPIO support"
 	depends on 44x
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 35d5765..0f76f2a 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -36,6 +36,7 @@ ifeq ($(CONFIG_PCI),y)
 obj-$(CONFIG_4xx)		+= ppc4xx_pci.o
 endif
 obj-$(CONFIG_PPC4xx_GPIO)	+= ppc4xx_gpio.o
+obj-$(CONFIG_DUMMY_GPIO)	+= dummy_gpio.o
 
 # Temporary hack until we have migrated to asm-powerpc
 ifeq ($(ARCH),powerpc)
diff --git a/arch/powerpc/sysdev/dummy_gpio.c b/arch/powerpc/sysdev/dummy_gpio.c
new file mode 100644
index 0000000..22f93d6
--- /dev/null
+++ b/arch/powerpc/sysdev/dummy_gpio.c
@@ -0,0 +1,98 @@
+/*
+ * Dummy gpio driver
+ *
+ * Copyright (c) 2008 Harris Corporation
+ * Copyright (c) 2008 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *
+ * Author: Steve Falco <sfalco@harris.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+
+struct dummy_gpio_chip {
+	struct of_mm_gpio_chip mm_gc;
+};
+
+static int dummy_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static void
+dummy_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+}
+
+static int dummy_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return 0;
+}
+
+static int
+dummy_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	return 0;
+}
+
+static int __init dummy_add_gpiochips(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "linux,dummy-gpio") {
+		int ret;
+		struct dummy_gpio_chip *dummy_gc;
+		struct of_mm_gpio_chip *mm_gc;
+		struct of_gpio_chip *of_gc;
+		struct gpio_chip *gc;
+
+		dummy_gc = kzalloc(sizeof(*dummy_gc), GFP_KERNEL);
+		if (!dummy_gc) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		mm_gc = &dummy_gc->mm_gc;
+		of_gc = &mm_gc->of_gc;
+		gc = &of_gc->gc;
+
+		of_gc->gpio_cells = 2;
+		gc->ngpio = 32;
+		gc->direction_input = dummy_gpio_dir_in;
+		gc->direction_output = dummy_gpio_dir_out;
+		gc->get = dummy_gpio_get;
+		gc->set = dummy_gpio_set;
+
+		ret = of_mm_gpiochip_add(np, mm_gc);
+		if (ret)
+			goto err;
+		continue;
+err:
+		pr_err("%s: registration failed with status %d\n",
+		       np->full_name, ret);
+		kfree(dummy_gc);
+		/* try others anyway */
+	}
+	return 0;
+}
+arch_initcall(dummy_add_gpiochips);
-- 
1.6.0.2

Re: [RFC] Dummy GPIO driver for use with SPI

[Anton Vorontsov]

On Fri, Dec 12, 2008 at 09:22:02AM -0500, Steven A. Falco wrote:
> This patch adds a dummy GPIO driver, which is useful for SPI devices
> that do not have a physical chip select.

Hm. Then you don't need a chip-select, and SPI driver must understand
this case. When SPI controller has no "gpios" property, it should just
ignore any chip-select toggling operations.

As an implementation example you can use this patch:

http://patchwork.ozlabs.org/patch/12499/

grep for "SPI w/o chip-select line."

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [RFC] Dummy GPIO driver for use with SPI

[Steven A. Falco]

Anton Vorontsov wrote:
> On Fri, Dec 12, 2008 at 09:22:02AM -0500, Steven A. Falco wrote:
>> This patch adds a dummy GPIO driver, which is useful for SPI devices
>> that do not have a physical chip select.
> 
> Hm. Then you don't need a chip-select, and SPI driver must understand
> this case. When SPI controller has no "gpios" property, it should just
> ignore any chip-select toggling operations.
> 
> As an implementation example you can use this patch:
> 
> http://patchwork.ozlabs.org/patch/12499/
> 
> grep for "SPI w/o chip-select line."
> 

My actual situation is a bit more complicated - serves me right for
trying to simplify it in my RFC.

We have three devices on the SPI bus.  Two have well-behaved chip
selects - they are ST flash memory devices.  The third device, the
Atmel chip does not have a chip select.  It does have a RESET pin,
which is similar to a chip select, but the Atmel protocol requires
that that pin be low during the entire programming operation, and
I cannot chain all the tx/rx operations together into one atomic 
SPI transaction, so I cannot use that pin as the SPI chip select.

Instead, I manage the RESET pin outside of the SPI driver, and hence
there is no chip select for that one device, so I use my dummy CS
driver to provide a fake chip select to satisfy the SPI driver.

This does have the limitation that I must be careful not to access
the flash parts at the same time as I access the Atmel, but that is
ok for my application.  I guess I could use something like your
patch, but I'd maybe have to extend the flags to include a "do not
use" bit, which would bypass the gpio_is_valid and gpio_request
calls.

What do you think about having a mechanism to specify that some
SPI slaves have a chip select, while others don't have to have a
chip select managed by the SPI subsystem?

	Steve

Re: [RFC] Dummy GPIO driver for use with SPI

[Anton Vorontsov]

On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
> Anton Vorontsov wrote:
> > On Fri, Dec 12, 2008 at 09:22:02AM -0500, Steven A. Falco wrote:
> >> This patch adds a dummy GPIO driver, which is useful for SPI devices
> >> that do not have a physical chip select.
> > 
> > Hm. Then you don't need a chip-select, and SPI driver must understand
> > this case. When SPI controller has no "gpios" property, it should just
> > ignore any chip-select toggling operations.
> > 
> > As an implementation example you can use this patch:
> > 
> > http://patchwork.ozlabs.org/patch/12499/
> > 
> > grep for "SPI w/o chip-select line."
> > 
> 
> My actual situation is a bit more complicated - serves me right for
> trying to simplify it in my RFC.
> 
> We have three devices on the SPI bus.  Two have well-behaved chip
> selects - they are ST flash memory devices.  The third device, the
> Atmel chip does not have a chip select.  It does have a RESET pin,
> which is similar to a chip select, but the Atmel protocol requires
> that that pin be low during the entire programming operation, and
> I cannot chain all the tx/rx operations together into one atomic 
> SPI transaction, so I cannot use that pin as the SPI chip select.
> 
> Instead, I manage the RESET pin outside of the SPI driver, and hence
> there is no chip select for that one device, so I use my dummy CS
> driver to provide a fake chip select to satisfy the SPI driver.
> 
> This does have the limitation that I must be careful not to access
> the flash parts at the same time as I access the Atmel, but that is
> ok for my application.  I guess I could use something like your
> patch, but I'd maybe have to extend the flags to include a "do not
> use" bit, which would bypass the gpio_is_valid and gpio_request
> calls.
> 
> What do you think about having a mechanism to specify that some
> SPI slaves have a chip select, while others don't have to have a
> chip select managed by the SPI subsystem?

Um.. do you know that you can pass '0' as a GPIO?

For example,

spi-controller {
	gpios = <&pio1 1 0	/* cs0 */
		 0		/* cs1, no GPIO */
		 &pio2 2 0>;	/* cs2 */

	device@0 {
		reg = <0>; /* spi device, cs 0: "&pio1 1 0" */
	}

	device@1 {
		reg = <1>; /* spi device, cs 1: no actual GPIO */
	}

	device@2 {
		reg = <2>; /* spi device, cs 2: "&pio2 2 0" */
	}
};

With this patch
http://patchwork.ozlabs.org/patch/12450/

of_get_gpio() will differentiate "end of gpios" and "no gpio" cases.
So, in the SPI driver you can do something like this:

count = of_gpio_count(np);
for (i = 0; i < count; i++) {
	int gpio;

	gpio = of_get_gpio(np, i);
	if (gpio_is_valid(gpio)) {
		normal case;
	} else if (gpio == -EEXIST) {
		the special case;
	} else {
		error;
	}
}

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [RFC] Dummy GPIO driver for use with SPI

[Steven A. Falco]

Anton Vorontsov wrote:
> On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
>> Anton Vorontsov wrote:
>>> On Fri, Dec 12, 2008 at 09:22:02AM -0500, Steven A. Falco wrote:
>>>> This patch adds a dummy GPIO driver, which is useful for SPI devices
>>>> that do not have a physical chip select.
>>> Hm. Then you don't need a chip-select, and SPI driver must understand
>>> this case. When SPI controller has no "gpios" property, it should just
>>> ignore any chip-select toggling operations.
>>>
>>> As an implementation example you can use this patch:
>>>
>>> http://patchwork.ozlabs.org/patch/12499/
>>>
>>> grep for "SPI w/o chip-select line."
>>>
>> My actual situation is a bit more complicated - serves me right for
>> trying to simplify it in my RFC.
>>
>> We have three devices on the SPI bus.  Two have well-behaved chip
>> selects - they are ST flash memory devices.  The third device, the
>> Atmel chip does not have a chip select.  It does have a RESET pin,
>> which is similar to a chip select, but the Atmel protocol requires
>> that that pin be low during the entire programming operation, and
>> I cannot chain all the tx/rx operations together into one atomic 
>> SPI transaction, so I cannot use that pin as the SPI chip select.
>>
>> Instead, I manage the RESET pin outside of the SPI driver, and hence
>> there is no chip select for that one device, so I use my dummy CS
>> driver to provide a fake chip select to satisfy the SPI driver.
>>
>> This does have the limitation that I must be careful not to access
>> the flash parts at the same time as I access the Atmel, but that is
>> ok for my application.  I guess I could use something like your
>> patch, but I'd maybe have to extend the flags to include a "do not
>> use" bit, which would bypass the gpio_is_valid and gpio_request
>> calls.
>>
>> What do you think about having a mechanism to specify that some
>> SPI slaves have a chip select, while others don't have to have a
>> chip select managed by the SPI subsystem?
> 
> Um.. do you know that you can pass '0' as a GPIO?

I did not know that. :-)  Ok, so I'll look at modifying spi_ppc4xx.c
based on your suggestions.

	Thanks!
	Steve

> 
> For example,
> 
> spi-controller {
> 	gpios = <&pio1 1 0	/* cs0 */
> 		 0		/* cs1, no GPIO */
> 		 &pio2 2 0>;	/* cs2 */
> 
> 	device@0 {
> 		reg = <0>; /* spi device, cs 0: "&pio1 1 0" */
> 	}
> 
> 	device@1 {
> 		reg = <1>; /* spi device, cs 1: no actual GPIO */
> 	}
> 
> 	device@2 {
> 		reg = <2>; /* spi device, cs 2: "&pio2 2 0" */
> 	}
> };
> 
> With this patch
> http://patchwork.ozlabs.org/patch/12450/
> 
> of_get_gpio() will differentiate "end of gpios" and "no gpio" cases.
> So, in the SPI driver you can do something like this:
> 
> count = of_gpio_count(np);
> for (i = 0; i < count; i++) {
> 	int gpio;
> 
> 	gpio = of_get_gpio(np, i);
> 	if (gpio_is_valid(gpio)) {
> 		normal case;
> 	} else if (gpio == -EEXIST) {
> 		the special case;
> 	} else {
> 		error;
> 	}
> }
> 

Re: [RFC] Dummy GPIO driver for use with SPI

[Trent Piepho]

On Fri, 12 Dec 2008, Anton Vorontsov wrote:
> On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
>> What do you think about having a mechanism to specify that some
>> SPI slaves have a chip select, while others don't have to have a
>> chip select managed by the SPI subsystem?
>
> Um.. do you know that you can pass '0' as a GPIO?
>
> For example,
>
> spi-controller {
> 	gpios = <&pio1 1 0	/* cs0 */
> 		 0		/* cs1, no GPIO */
> 		 &pio2 2 0>;	/* cs2 */

It's ok the that middle specifier is only one word instead of three?  Seems
like "0 0 0" would be better, so all the specifiers are the same size.

> 		normal case;
> 	} else if (gpio == -EEXIST) {

Isn't EEXIST (pathname already exists) backward?  Seems like ENOENT would
be the right error code.  Except that's used for reading past the end...

Maybe a reading past the end should be EINVAL or EBADF?

Or return ENODEV for the 'hole' cell?  Or ENOLINK?

EEXIST is for trying to create something that already exists.  The 'hole'
is more like trying to follow a broken link or find something that doesn't
exist.

Re: [RFC] Dummy GPIO driver for use with SPI

[David Gibson]

On Fri, Dec 12, 2008 at 01:39:45PM -0800, Trent Piepho wrote:
> On Fri, 12 Dec 2008, Anton Vorontsov wrote:
> > On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
> >> What do you think about having a mechanism to specify that some
> >> SPI slaves have a chip select, while others don't have to have a
> >> chip select managed by the SPI subsystem?
> >
> > Um.. do you know that you can pass '0' as a GPIO?
> >
> > For example,
> >
> > spi-controller {
> > 	gpios = <&pio1 1 0	/* cs0 */
> > 		 0		/* cs1, no GPIO */
> > 		 &pio2 2 0>;	/* cs2 */
> 
> It's ok the that middle specifier is only one word instead of three?  Seems
> like "0 0 0" would be better, so all the specifiers are the same
> size.

No.  The gpio specifier size is already a property of the gpio
controller, so the phandle must be understood before reading the
specifier cells (interrupt specifiers are variable size in the same
way).  It's just that nearly every (real) gpio controller has a 2 cell
specifier.  Having the "null" gpio have #gpio-cells non-zero would be
silly, though.

-- 
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: [RFC] Dummy GPIO driver for use with SPI

[David Gibson]

On Fri, Dec 12, 2008 at 09:22:02AM -0500, Steven A. Falco wrote:
> This patch adds a dummy GPIO driver, which is useful for SPI devices
> that do not have a physical chip select.
> 
> Signed-off-by: Steven A. Falco <sfalco@harris.com>
> ---
> The SPI subsystem requires a chip-select for each connected slave
> device.  I have a custom board with an Atmel "co-processor".  This part
> is reprogrammed via SPI, so it needs a chip select to satisfy the SPI
> subsystem, but my hardware does not have a physical CS connected.
> 
> I could waste a "no-connect" GPIO pin, but I'd rather not.  So, I've
> written a dummy GPIO driver, which behaves exactly like a real GPIO
> device, but with no underlying hardware.  This could also be useful
> as a template for real GPIO drivers.
> 
> I use the following dts entry:
> 
> 	GPIO3: dummy@ef500000 {
> 		compatible = "linux,dummy-gpio";
> 		reg = <ef500000 1>;
> 		gpio-controller;
> 		#gpio-cells = <2>;
> 	};

This is not sane.  I can see reasons it might be useful to have a
dummy gpio driver within the kernel, but since this doesn't represent
any real hardware, it should not appear in the device tree.

-- 
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: [RFC] Dummy GPIO driver for use with SPI

[Anton Vorontsov]

On Fri, Dec 12, 2008 at 01:39:45PM -0800, Trent Piepho wrote:
> On Fri, 12 Dec 2008, Anton Vorontsov wrote:
> > On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
> >> What do you think about having a mechanism to specify that some
> >> SPI slaves have a chip select, while others don't have to have a
> >> chip select managed by the SPI subsystem?
> >
> > Um.. do you know that you can pass '0' as a GPIO?
> >
> > For example,
> >
> > spi-controller {
> > 	gpios = <&pio1 1 0	/* cs0 */
> > 		 0		/* cs1, no GPIO */
> > 		 &pio2 2 0>;	/* cs2 */
> 
> It's ok the that middle specifier is only one word instead of three?  Seems
> like "0 0 0" would be better, so all the specifiers are the same size.
> 
> > 		normal case;
> > 	} else if (gpio == -EEXIST) {
> 
> Isn't EEXIST (pathname already exists) backward?

In my thinking it's "the GPIO is specified (exists in the list), but
it's would be an error if you try to use it". So EEXIST.

> Seems like ENOENT would
> be the right error code.  Except that's used for reading past the end...
> Maybe a reading past the end should be EINVAL or EBADF?
> 
> Or return ENODEV for the 'hole' cell?  Or ENOLINK?

I'd say it's a matter of taste, none of the errors are actually
appropriate. An appropriate errno value would be EHOLEINALIST,
but we don't have it.

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2