| Message ID | 4942738A.80609@harris.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
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 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
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 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; > } > } >
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.
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.
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.
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,
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);
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