LinuxLists.cc - [PATCH v3] Add SPI over GPIO driver

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Mon, Jul 21, 2008 at 4:46 AM, Michael Buesch <[email protected]> wrote:
> This adds a driver that lets you drive an SPI bus over
> generic GPIO pins.
>
> Signed-off-by: Michael Buesch <[email protected]>

Very useful. Fits my shiny new GPIO implementation for SuperH like a glove.

Michael, as you know, I have some SuperH board code that makes use of
this driver. Are there any outstanding issues with this driver that I
can help resolving?

Andrew, is this driver still in -mm? Any chance of getting it included
in 2.6.28?

Cheers,

/ magnus

2008-10-17 08:53:39

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Friday 17 October 2008 05:14:26 Magnus Damm wrote:
> On Mon, Jul 21, 2008 at 4:46 AM, Michael Buesch <[email protected]> wrote:
> > This adds a driver that lets you drive an SPI bus over
> > generic GPIO pins.
> >
> > Signed-off-by: Michael Buesch <[email protected]>
>
> Very useful. Fits my shiny new GPIO implementation for SuperH like a glove.
>
> Michael, as you know, I have some SuperH board code that makes use of
> this driver. Are there any outstanding issues with this driver that I
> can help resolving?

bool cs_activelow should probably be removed from struct spi_gpio_platform_data.

I think cs always is activelow, so we can simply hardcode this in spi_gpio_chipselect()

If you want to do this, please feel free to do so.
I currently don't have the time for this.
The rest of the driver is fine with me.

--
Greetings Michael.

2008-10-17 09:52:44

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

[removed closed openwrt-devel list]

On Fri, Oct 17, 2008 at 5:52 PM, Michael Buesch <[email protected]> wrote:
> On Friday 17 October 2008 05:14:26 Magnus Damm wrote:
>> On Mon, Jul 21, 2008 at 4:46 AM, Michael Buesch <[email protected]> wrote:
>> > This adds a driver that lets you drive an SPI bus over
>> > generic GPIO pins.
>> >
>> > Signed-off-by: Michael Buesch <[email protected]>
>>
>> Very useful. Fits my shiny new GPIO implementation for SuperH like a glove.
>>
>> Michael, as you know, I have some SuperH board code that makes use of
>> this driver. Are there any outstanding issues with this driver that I
>> can help resolving?
>
> bool cs_activelow should probably be removed from struct spi_gpio_platform_data.
>
> I think cs always is activelow, so we can simply hardcode this in spi_gpio_chipselect()

Really? There _must_ be chips out there that want active high CS!
I'd rather keep the activelow option or maybe turn in into activehigh instead.

> If you want to do this, please feel free to do so.
> I currently don't have the time for this.
> The rest of the driver is fine with me.

Ok, thank you.

David or Andrew, any change of getting this merged as-is somehow? Thanks!

/ magnus

2008-10-17 11:19:20

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Friday 17 October 2008 11:52:32 Magnus Damm wrote:
> [removed closed openwrt-devel list]
>
> On Fri, Oct 17, 2008 at 5:52 PM, Michael Buesch <[email protected]> wrote:
> > On Friday 17 October 2008 05:14:26 Magnus Damm wrote:
> >> On Mon, Jul 21, 2008 at 4:46 AM, Michael Buesch <[email protected]> wrote:
> >> > This adds a driver that lets you drive an SPI bus over
> >> > generic GPIO pins.
> >> >
> >> > Signed-off-by: Michael Buesch <[email protected]>
> >>
> >> Very useful. Fits my shiny new GPIO implementation for SuperH like a glove.
> >>
> >> Michael, as you know, I have some SuperH board code that makes use of
> >> this driver. Are there any outstanding issues with this driver that I
> >> can help resolving?
> >
> > bool cs_activelow should probably be removed from struct spi_gpio_platform_data.
> >
> > I think cs always is activelow, so we can simply hardcode this in spi_gpio_chipselect()
>
> Really? There _must_ be chips out there that want active high CS!
> I'd rather keep the activelow option or maybe turn in into activehigh instead.

Ok, I dunno. I never saw one. All of my SPI stuff is activelow.
So well, let's keep it.

> > If you want to do this, please feel free to do so.
> > I currently don't have the time for this.
> > The rest of the driver is fine with me.
>
> Ok, thank you.
>
> David or Andrew, any change of getting this merged as-is somehow? Thanks!

Yes, please merge it.

--
Greetings Michael.

2008-10-18 04:37:33

by Andrew Morton

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Fri, 17 Oct 2008 13:18:32 +0200 Michael Buesch <[email protected]> wrote:

> On Friday 17 October 2008 11:52:32 Magnus Damm wrote:
> > [removed closed openwrt-devel list]
> >
> > On Fri, Oct 17, 2008 at 5:52 PM, Michael Buesch <[email protected]> wrote:
> > > On Friday 17 October 2008 05:14:26 Magnus Damm wrote:
> > >> On Mon, Jul 21, 2008 at 4:46 AM, Michael Buesch <[email protected]> wrote:
> > >> > This adds a driver that lets you drive an SPI bus over
> > >> > generic GPIO pins.
> > >> >
> > >> > Signed-off-by: Michael Buesch <[email protected]>
> > >>
> > >> Very useful. Fits my shiny new GPIO implementation for SuperH like a glove.
> > >>
> > >> Michael, as you know, I have some SuperH board code that makes use of
> > >> this driver. Are there any outstanding issues with this driver that I
> > >> can help resolving?
> > >
> > > bool cs_activelow should probably be removed from struct spi_gpio_platform_data.
> > >
> > > I think cs always is activelow, so we can simply hardcode this in spi_gpio_chipselect()
> >
> > Really? There _must_ be chips out there that want active high CS!
> > I'd rather keep the activelow option or maybe turn in into activehigh instead.
>
> Ok, I dunno. I never saw one. All of my SPI stuff is activelow.
> So well, let's keep it.
>
> > > If you want to do this, please feel free to do so.
> > > I currently don't have the time for this.
> > > The rest of the driver is fine with me.
> >
> > Ok, thank you.
> >
> > David or Andrew, any change of getting this merged as-is somehow? Thanks!
>
> Yes, please merge it.

This patch has some "issues" a month or so ago. Current status is that
I've pinged David to see if he's OK with a 2.6.28 merge and he hasn't
yet replied.

Let's not bust a gut to get this into 2.6.28-rc1 - let's get it right
rather than fast. If we end up deciding to merge it into, say,
2.6.28-rc2 then that should be OK under the circumstances.

From: Michael Buesch <[email protected]>

This adds a driver that lets you drive an SPI bus over generic GPIO pins.

Signed-off-by: Michael Buesch <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Pierre Ossman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

MAINTAINERS | 5
drivers/spi/Kconfig | 13 +
drivers/spi/Makefile | 1
drivers/spi/spi_gpio.c | 252 +++++++++++++++++++++++++++++++++
include/linux/spi/spi_gpio.h | 73 +++++++++
5 files changed, 344 insertions(+)

diff -puN MAINTAINERS~spi-add-spi-over-gpio-driver MAINTAINERS
--- a/MAINTAINERS~spi-add-spi-over-gpio-driver
+++ a/MAINTAINERS
@@ -3911,6 +3911,11 @@ M: [email protected]
L: [email protected]
S: Supported

+SPI GPIO MASTER DRIVER
+P: Michael Buesch
+M: [email protected]
+S: Maintained
+
SPU FILE SYSTEM
P: Jeremy Kerr
M: [email protected]
diff -puN drivers/spi/Kconfig~spi-add-spi-over-gpio-driver drivers/spi/Kconfig
--- a/drivers/spi/Kconfig~spi-add-spi-over-gpio-driver
+++ a/drivers/spi/Kconfig
@@ -100,6 +100,19 @@ config SPI_BUTTERFLY
inexpensive battery powered microcontroller evaluation board.
This same cable can be used to flash new firmware.

+config SPI_GPIO
+ tristate "GPIO API based bitbanging SPI controller"
+ depends on SPI_MASTER && GENERIC_GPIO
+ select SPI_BITBANG
+ help
+ This is a platform driver that can be used for bitbanging
+ an SPI bus over GPIO pins.
+ Select this if you have any SPI device that is connected via
+ GPIO pins.
+ The module will be called spi_gpio.
+
+ If unsure, say N.
+
config SPI_IMX
tristate "Freescale iMX SPI controller"
depends on ARCH_IMX && EXPERIMENTAL
diff -puN drivers/spi/Makefile~spi-add-spi-over-gpio-driver drivers/spi/Makefile
--- a/drivers/spi/Makefile~spi-add-spi-over-gpio-driver
+++ a/drivers/spi/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_SPI_BFIN) += spi_bfin5xx.
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
obj-$(CONFIG_SPI_AU1550) += au1550_spi.o
obj-$(CONFIG_SPI_BUTTERFLY) += spi_butterfly.o
+obj-$(CONFIG_SPI_GPIO) += spi_gpio.o
obj-$(CONFIG_SPI_IMX) += spi_imx.o
obj-$(CONFIG_SPI_LM70_LLP) += spi_lm70llp.o
obj-$(CONFIG_SPI_PXA2XX) += pxa2xx_spi.o
diff -puN /dev/null drivers/spi/spi_gpio.c
--- /dev/null
+++ a/drivers/spi/spi_gpio.c
@@ -0,0 +1,252 @@
+/*
+ * Bitbanging SPI bus driver using GPIO API
+ *
+ * Copyright (c) 2008 Piotr Skamruk
+ * Copyright (c) 2008 Michael Buesch
+ *
+ * based on spi_s3c2410_gpio.c
+ * Copyright (c) 2006 Ben Dooks
+ * Copyright (c) 2006 Simtec Electronics
+ * and on i2c-gpio.c
+ * Copyright (C) 2007 Atmel Corporation
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi_bitbang.h>
+#include <linux/spi/spi_gpio.h>
+#include <linux/gpio.h>
+#include <asm/atomic.h>
+
+
+struct spi_gpio {
+ struct spi_bitbang bitbang;
+ struct spi_gpio_platform_data *info;
+ struct platform_device *pdev;
+ struct spi_board_info bi;
+};
+
+
+static inline struct spi_gpio *spidev_to_sg(struct spi_device *dev)
+{
+ return dev->controller_data;
+}
+
+static inline void setsck(struct spi_device *dev, int val)
+{
+ struct spi_gpio *sp = spidev_to_sg(dev);
+ gpio_set_value(sp->info->pin_clk, val ? 1 : 0);
+}
+
+static inline void setmosi(struct spi_device *dev, int val)
+{
+ struct spi_gpio *sp = spidev_to_sg(dev);
+ gpio_set_value(sp->info->pin_mosi, val ? 1 : 0);
+}
+
+static inline u32 getmiso(struct spi_device *dev)
+{
+ struct spi_gpio *sp = spidev_to_sg(dev);
+ return gpio_get_value(sp->info->pin_miso) ? 1 : 0;
+}
+
+static inline void do_spidelay(struct spi_device *dev, unsigned nsecs)
+{
+ struct spi_gpio *sp = spidev_to_sg(dev);
+
+ if (!sp->info->no_spi_delay)
+ ndelay(nsecs);
+}
+
+#define spidelay(nsecs) do { \
+ /* Steal the spi_device pointer from our caller. \
+ * The bitbang-API should probably get fixed here... */ \
+ do_spidelay(spi, nsecs); \
+ } while (0)
+
+#define EXPAND_BITBANG_TXRX
+#include <linux/spi/spi_bitbang.h>
+
+static u32 spi_gpio_txrx_mode0(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
+}
+
+static u32 spi_gpio_txrx_mode1(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha1(spi, nsecs, 0, word, bits);
+}
+
+static u32 spi_gpio_txrx_mode2(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha0(spi, nsecs, 1, word, bits);
+}
+
+static u32 spi_gpio_txrx_mode3(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha1(spi, nsecs, 1, word, bits);
+}
+
+static void spi_gpio_chipselect(struct spi_device *dev, int on)
+{
+ struct spi_gpio *sp = spidev_to_sg(dev);
+
+ if (sp->info->cs_activelow)
+ on = !on;
+ gpio_set_value(sp->info->pin_cs, on ? 1 : 0);
+}
+
+static int spi_gpio_probe(struct platform_device *pdev)
+{
+ struct spi_master *master;
+ struct spi_gpio_platform_data *pdata;
+ struct spi_gpio *sp;
+ struct spi_device *spidev;
+ int err;
+
+ pdata = pdev->dev.platform_data;
+ if (!pdata)
+ return -ENXIO;
+
+ err = -ENOMEM;
+ master = spi_alloc_master(&pdev->dev, sizeof(struct spi_gpio));
+ if (!master)
+ goto err_alloc_master;
+
+ sp = spi_master_get_devdata(master);
+ platform_set_drvdata(pdev, sp);
+ sp->info = pdata;
+
+ err = gpio_request(pdata->pin_clk, "spi_clock");
+ if (err)
+ goto err_request_clk;
+ err = gpio_request(pdata->pin_mosi, "spi_mosi");
+ if (err)
+ goto err_request_mosi;
+ err = gpio_request(pdata->pin_miso, "spi_miso");
+ if (err)
+ goto err_request_miso;
+ err = gpio_request(pdata->pin_cs, "spi_cs");
+ if (err)
+ goto err_request_cs;
+
+ sp->bitbang.master = spi_master_get(master);
+ sp->bitbang.master->bus_num = -1;
+ sp->bitbang.master->num_chipselect = 1;
+ sp->bitbang.chipselect = spi_gpio_chipselect;
+ sp->bitbang.txrx_word[SPI_MODE_0] = spi_gpio_txrx_mode0;
+ sp->bitbang.txrx_word[SPI_MODE_1] = spi_gpio_txrx_mode1;
+ sp->bitbang.txrx_word[SPI_MODE_2] = spi_gpio_txrx_mode2;
+ sp->bitbang.txrx_word[SPI_MODE_3] = spi_gpio_txrx_mode3;
+
+ gpio_direction_output(pdata->pin_clk, 0);
+ gpio_direction_output(pdata->pin_mosi, 0);
+ gpio_direction_output(pdata->pin_cs,
+ pdata->cs_activelow ? 1 : 0);
+ gpio_direction_input(pdata->pin_miso);
+
+ err = spi_bitbang_start(&sp->bitbang);
+ if (err)
+ goto err_no_bitbang;
+ err = pdata->boardinfo_setup(&sp->bi, master,
+ pdata->boardinfo_setup_data);
+ if (err)
+ goto err_bi_setup;
+ sp->bi.controller_data = sp;
+ err = -ENOMEM;
+ spidev = spi_new_device(master, &sp->bi);
+ if (!spidev)
+ goto err_new_dev;
+
+ return 0;
+
+err_new_dev:
+err_bi_setup:
+ spi_bitbang_stop(&sp->bitbang);
+err_no_bitbang:
+ spi_master_put(sp->bitbang.master);
+ gpio_free(pdata->pin_cs);
+err_request_cs:
+ gpio_free(pdata->pin_miso);
+err_request_miso:
+ gpio_free(pdata->pin_mosi);
+err_request_mosi:
+ gpio_free(pdata->pin_clk);
+err_request_clk:
+ kfree(master);
+
+err_alloc_master:
+ return err;
+}
+
+static int __devexit spi_gpio_remove(struct platform_device *pdev)
+{
+ struct spi_gpio *sp;
+ struct spi_gpio_platform_data *pdata;
+
+ pdata = pdev->dev.platform_data;
+ sp = platform_get_drvdata(pdev);
+
+ gpio_free(pdata->pin_clk);
+ gpio_free(pdata->pin_mosi);
+ gpio_free(pdata->pin_miso);
+ gpio_free(pdata->pin_cs);
+ spi_bitbang_stop(&sp->bitbang);
+ spi_master_put(sp->bitbang.master);
+
+ return 0;
+}
+
+static struct platform_driver spi_gpio_driver = {
+ .driver = {
+ .name = SPI_GPIO_PLATDEV_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = spi_gpio_probe,
+ .remove = __devexit_p(spi_gpio_remove),
+};
+
+int spi_gpio_next_id(void)
+{
+ static atomic_t counter = ATOMIC_INIT(-1);
+
+ return atomic_inc_return(&counter);
+}
+EXPORT_SYMBOL(spi_gpio_next_id);
+
+static int __init spi_gpio_init(void)
+{
+ int err;
+
+ err = platform_driver_register(&spi_gpio_driver);
+ if (err)
+ printk(KERN_ERR "spi-gpio: register failed: %d\n", err);
+
+ return err;
+}
+module_init(spi_gpio_init);
+
+static void __exit spi_gpio_exit(void)
+{
+ platform_driver_unregister(&spi_gpio_driver);
+}
+module_exit(spi_gpio_exit);
+
+MODULE_AUTHOR("Piot Skamruk <piotr.skamruk at gmail.com>");
+MODULE_AUTHOR("Michael Buesch");
+MODULE_DESCRIPTION("Platform independent GPIO bitbanging SPI driver");
+MODULE_LICENSE("GPL v2");
diff -puN /dev/null include/linux/spi/spi_gpio.h
--- /dev/null
+++ a/include/linux/spi/spi_gpio.h
@@ -0,0 +1,73 @@
+/*
+ * spi_gpio interface to platform code
+ *
+ * Copyright (c) 2008 Piotr Skamruk
+ * Copyright (c) 2008 Michael Buesch
+ *
+ * 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.
+ */
+#ifndef _LINUX_SPI_SPI_GPIO
+#define _LINUX_SPI_SPI_GPIO
+
+#include <linux/types.h>
+#include <linux/spi/spi.h>
+
+
+/**
+ * struct spi_gpio_platform_data - Data definitions for a SPI-GPIO device.
+ *
+ * This structure holds information about a GPIO-based SPI device.
+ *
+ * @pin_clk: The GPIO pin number of the CLOCK pin.
+ *
+ * @pin_miso: The GPIO pin number of the MISO pin.
+ *
+ * @pin_mosi: The GPIO pin number of the MOSI pin.
+ *
+ * @pin_cs: The GPIO pin number of the CHIPSELECT pin.
+ *
+ * @cs_activelow: If true, the chip is selected when the CS line is low.
+ *
+ * @no_spi_delay: If true, no delay is done in the lowlevel bitbanging.
+ * Note that doing no delay is not standards compliant,
+ * but it might be needed to speed up transfers on some
+ * slow embedded machines.
+ *
+ * @boardinfo_setup: This callback is called after the
+ * SPI master device was registered, but before the
+ * device is registered.
+ * @boardinfo_setup_data: Data argument passed to boardinfo_setup().
+ */
+struct spi_gpio_platform_data {
+ unsigned int pin_clk;
+ unsigned int pin_miso;
+ unsigned int pin_mosi;
+ unsigned int pin_cs;
+ bool cs_activelow;
+ bool no_spi_delay;
+ int (*boardinfo_setup)(struct spi_board_info *bi,
+ struct spi_master *master,
+ void *data);
+ void *boardinfo_setup_data;
+};
+
+/**
+ * SPI_GPIO_PLATDEV_NAME - The platform device name string.
+ *
+ * The name string that has to be used for platform_device_alloc
+ * when allocating a spi-gpio device.
+ */
+#define SPI_GPIO_PLATDEV_NAME "spi-gpio"
+
+/**
+ * spi_gpio_next_id - Get another platform device ID number.
+ *
+ * This returns the next platform device ID number that has to be used
+ * for platform_device_alloc. The ID is opaque and should not be used for
+ * anything else.
+ */
+int spi_gpio_next_id(void);
+
+#endif /* _LINUX_SPI_SPI_GPIO */
_

2008-10-18 09:33:18

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Saturday 18 October 2008 06:36:31 Andrew Morton wrote:
> This patch has some "issues" a month or so ago. Current status is that

yeah, upstream maintainers don't like these patches.
Please drop this one and the "gpiommc" patch now.

--
Greetings Michael.

2008-10-18 20:41:51

by Piotr Skamruk

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

2008/10/18 Michael Buesch <[email protected]>:
> On Saturday 18 October 2008 06:36:31 Andrew Morton wrote:
>> This patch has some "issues" a month or so ago. Current status is that
>
> yeah, upstream maintainers don't like these patches.
> Please drop this one and the "gpiommc" patch now.
>
As i remember - David stands that these patches (or even
functionality) are not usable for him, so he don't want to see them in
mainstream kernel sources. It's his choice, he is maintainer of this
subsystem.

You have this on openwrt svn, I have it on my dev gittree - and as You
can see on Magnus example - anybody interested can google for it.
Maybe if there will be next few peoples - David will change his
opinion on this subject...

2008-10-18 20:54:37

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Saturday 18 October 2008, Piotr Skamruk wrote:
> 2008/10/18 Michael Buesch <[email protected]>:
> > On Saturday 18 October 2008 06:36:31 Andrew Morton wrote:
> >> This patch has some "issues" a month or so ago. Current status is that
> >
> > yeah, upstream maintainers don't like these patches.
> > Please drop this one and the "gpiommc" patch now.
> >
> As i remember - David stands that these patches (or even
> functionality) are not usable for him, so he don't want to see them in
> mainstream kernel sources. It's his choice, he is maintainer of this
> subsystem.

That patch is very limited, and wrong in several respects.

I fully support the idea of SPI over GPIO. It's just that
particular patch which is problematic.

> You have this on openwrt svn, I have it on my dev gittree - and as You
> can see on Magnus example - anybody interested can google for it.
> Maybe if there will be next few peoples - David will change his
> opinion on this subject...

I'm sanity testing a refresh of the *original* late-2006
SPI-over-GPIO driver. It's actually usable as a drop-in
replacement for a "native" driver.

Unfortunately I goofed and pulled down the current GIT tree
and found lots of unrelated breakage (non-SPI) that's keept
that sanity test from proceeding apace. And the board on
which it's most recently been used is currently broken.
I may just send that patch out soon, as an FYI...

- Dave

2008-10-20 02:44:30

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Sun, Oct 19, 2008 at 5:54 AM, David Brownell <[email protected]> wrote:
> On Saturday 18 October 2008, Piotr Skamruk wrote:
>> 2008/10/18 Michael Buesch <[email protected]>:
>> > On Saturday 18 October 2008 06:36:31 Andrew Morton wrote:
>> >> This patch has some "issues" a month or so ago. Current status is that
>> >
>> > yeah, upstream maintainers don't like these patches.
>> > Please drop this one and the "gpiommc" patch now.
>> >
>> As i remember - David stands that these patches (or even
>> functionality) are not usable for him, so he don't want to see them in
>> mainstream kernel sources. It's his choice, he is maintainer of this
>> subsystem.
>
> That patch is very limited, and wrong in several respects.
>
> I fully support the idea of SPI over GPIO. It's just that
> particular patch which is problematic.

Ok, let's fix those issues then!

>> You have this on openwrt svn, I have it on my dev gittree - and as You
>> can see on Magnus example - anybody interested can google for it.
>> Maybe if there will be next few peoples - David will change his
>> opinion on this subject...
>
> I'm sanity testing a refresh of the *original* late-2006
> SPI-over-GPIO driver. It's actually usable as a drop-in
> replacement for a "native" driver.

The Openwrt driver by Michael Buesch works just fine for me, but if
you prefer to switch driver that's of course ok too.

> Unfortunately I goofed and pulled down the current GIT tree
> and found lots of unrelated breakage (non-SPI) that's keept
> that sanity test from proceeding apace. And the board on
> which it's most recently been used is currently broken.
> I may just send that patch out soon, as an FYI...

Please CC me and I'll test it on some SuperH boards. Thanks!

/ magnus

2008-10-20 04:42:18

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Sunday 19 October 2008, Magnus Damm wrote:
> >
> > I fully support the idea of SPI over GPIO. ?It's just that
> > particular patch which is problematic.
>
> Ok, let's fix those issues then!

The issues I identified earlier still haven't been resolved...
one of several reasons IMO to just use a different driver.

> > I'm sanity testing a refresh of the *original* late-2006
> > SPI-over-GPIO driver. ?It's actually usable as a drop-in
> > replacement for a "native" driver.
>
> The Openwrt driver by Michael Buesch works just fine for me, but if
> you prefer to switch driver that's of course ok too.
>
> > Unfortunately I goofed and pulled down the current GIT tree
> > and found lots of unrelated breakage (non-SPI) that's keept
> > that sanity test from proceeding apace. ?And the board on
> > which it's most recently been used is currently broken.
> > I may just send that patch out soon, as an FYI...
>
> Please CC me and I'll test it on some SuperH boards. Thanks!

Patch follows in a separate message, with trimmed CC list.

- Dave

2008-10-20 04:42:32

[permalink] [raw]

Subject: [patch 2.6.27-git] spi_gpio driver

From: David Brownell <[email protected]>

Generalize the old at91rm9200 "bootstrap" bitbanging SPI master driver
as "spi_gpio", so it works with arbitrary GPIOs and can be configured
through platform_data. Such SPI masters support:

- any number of bus instances (bus_num is the platform_device.id)
- any number of chipselects (one GPIO per spi_device)
- all four SPI_MODE values, and SPI_CS_HIGH
- i/o word sizes from 1 to 32 bits;
- devices configured as with any other spi_master controller

When configured using platform_data, this provides relatively low clock
rates. On platforms that support inlined GPIO calls, significantly
improved transfer speeds are also possible with a semi-custom driver.
(It's still painful when accessing flash memory, but less so.)

Sanity checked by using this version to replace both native controllers
on a board with six different SPI slaves, relying on three different
SPI_MODE_* values and both SPI_CS_HIGH settings for correct operation.

Signed-off-by: David Brownell <[email protected]>
---
drivers/spi/Kconfig | 18 +-
drivers/spi/Makefile | 1
drivers/spi/spi_gpio.c | 360 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi_gpio.h | 60 ++++++
4 files changed, 438 insertions(+), 1 deletion(-)

--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -78,7 +78,7 @@ config SPI_AU1550
will be called au1550_spi.

config SPI_BITBANG
- tristate "Bitbanging SPI master"
+ tristate "Utilities for Bitbanging SPI masters"
help
With a few GPIO pins, your system can bitbang the SPI protocol.
Select this to get SPI support through I/O pins (GPIO, parallel
@@ -100,6 +100,22 @@ config SPI_BUTTERFLY
inexpensive battery powered microcontroller evaluation board.
This same cable can be used to flash new firmware.

+config SPI_GPIO
+ tristate "GPIO-based bitbanging SPI Master"
+ depends on GENERIC_GPIO
+ select SPI_BITBANG
+ help
+ This simple GPIO bitbanging SPI master uses the arch-neutral GPIO
+ interface to manage MOSI, MISO, SCK, and chipselect signals. SPI
+ slaves connected to a bus using this driver are configured as usual,
+ except that the spi_board_info.controller_data holds the GPIO number
+ for the chipselect used by this controller driver.
+
+ Note that this driver often won't achieve even 1 Mbit/sec speeds,
+ making it unusually slow for SPI. If your platform can inline
+ GPIO operations, you should be able to leverage that for better
+ speed with a custom version of this driver; see the source code.
+
config SPI_IMX
tristate "Freescale iMX SPI controller"
depends on ARCH_IMX && EXPERIMENTAL
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_SPI_BFIN) += spi_bfin5xx.
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
obj-$(CONFIG_SPI_AU1550) += au1550_spi.o
obj-$(CONFIG_SPI_BUTTERFLY) += spi_butterfly.o
+obj-$(CONFIG_SPI_GPIO) += spi_gpio.o
obj-$(CONFIG_SPI_IMX) += spi_imx.o
obj-$(CONFIG_SPI_LM70_LLP) += spi_lm70llp.o
obj-$(CONFIG_SPI_PXA2XX) += pxa2xx_spi.o
--- /dev/null
+++ b/drivers/spi/spi_gpio.c
@@ -0,0 +1,360 @@
+/*
+ * spi_gpio.c - SPI master driver using generic bitbanged GPIO
+ *
+ * Copyright (C) 2006,2008 David Brownell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+
+#include <linux/spi/spi.h>
+#include <linux/spi/spi_bitbang.h>
+#include <linux/spi/spi_gpio.h>
+
+
+/*
+ * This bitbanging SPI master driver should help make systems usable
+ * when a native hardware SPI engine is not available, perhaps because
+ * its driver isn't yet working or because the I/O pins it requires
+ * are used for other purposes.
+ *
+ * platform_device->driver_data ... points to spi_gpio
+ *
+ * spi->controller_state ... reserved for bitbang framework code
+ * spi->controller_data ... holds chipselect GPIO
+ *
+ * spi->master->dev.driver_data ... points to spi_gpio->bitbang
+ */
+
+struct spi_gpio {
+ struct spi_bitbang bitbang;
+ struct spi_gpio_platform_data pdata;
+ struct platform_device *pdev;
+};
+
+/*----------------------------------------------------------------------*/
+
+/*
+ * Because the overhead of going through four GPIO procedure calls
+ * per transferred bit can make performance a problem, this code
+ * is set up so that you can use it in either of two ways:
+ *
+ * - The slow generic way: set up platform_data to hold the GPIO
+ * numbers used for MISO/MOSI/SCK, and issue procedure calls for
+ * each of them. This driver can handle several such busses.
+ *
+ * - The quicker inlined way: only helps with platform GPIO code
+ * that inlines operations for constant GPIOs. This can give
+ * you tight (fast!) inner loops, but each such bus needs a
+ * new driver. You'll define a new C file, with Makefile and
+ * Kconfig support; the C code can be a total of six lines:
+ *
+ * #define DRIVER_NAME "myboard_spi2"
+ * #define SPI_MISO_GPIO 119
+ * #define SPI_MOSI_GPIO 120
+ * #define SPI_SCK_GPIO 121
+ * #define SPI_N_CHIPSEL 4
+ * #include "spi_gpio.c"
+ */
+
+#ifndef DRIVER_NAME
+#define DRIVER_NAME "spi_gpio"
+
+#define GENERIC_BITBANG /* vs tight inlines */
+
+/* all functions referencing these symbols must define pdata */
+#define SPI_MISO_GPIO ((pdata)->miso)
+#define SPI_MOSI_GPIO ((pdata)->mosi)
+#define SPI_SCK_GPIO ((pdata)->sck)
+
+#define SPI_N_CHIPSEL ((pdata)->num_chipselect)
+
+#endif
+
+/*----------------------------------------------------------------------*/
+
+static inline const struct spi_gpio_platform_data * __pure
+spi_to_pdata(const struct spi_device *spi)
+{
+ const struct spi_bitbang *bang;
+ const struct spi_gpio *spi_gpio;
+
+ bang = spi_master_get_devdata(spi->master);
+ spi_gpio = container_of(bang, struct spi_gpio, bitbang);
+ return &spi_gpio->pdata;
+}
+
+/* this is #defined to avoid unused-variable warnings when inlining */
+#define pdata spi_to_pdata(spi)
+
+static inline void setsck(const struct spi_device *spi, int is_on)
+{
+ gpio_set_value(SPI_SCK_GPIO, is_on);
+}
+
+static inline void setmosi(const struct spi_device *spi, int is_on)
+{
+ gpio_set_value(SPI_MOSI_GPIO, is_on);
+}
+
+static inline int getmiso(const struct spi_device *spi)
+{
+ return gpio_get_value(SPI_MISO_GPIO);
+}
+
+#undef pdata
+
+/*
+ * NOTE: this clocks "as fast as we can". It "should" be a function of the
+ * requested device clock. Software overhead means we usually have trouble
+ * reaching even one Mbit/sec (except when we can inline bitops), so for now
+ * we'll just assume we never need additional per-bit slowdowns.
+ */
+#define spidelay(nsecs) do{} while(0)
+
+#define EXPAND_BITBANG_TXRX
+#include <linux/spi/spi_bitbang.h>
+
+/*
+ * These functions can leverage inline expansion of GPIO calls to shrink
+ * costs for a txrx bit, often by factors of around ten (by instruction
+ * count). That is particularly visible for larger word sizes, but helps
+ * even with default 8-bit words.
+ *
+ * REVISIT overheads calling these functions for each word also have
+ * significant performance costs. Having txrx_bufs() calls that inline
+ * the txrx_word() logic would help performance, e.g. on larger blocks
+ * used with flash storage or MMC/SD. There should also be ways to make
+ * GCC be less stupid about reloading registers inside the I/O loops,
+ * even without inlined GPIO calls; __attribute__((hot)) on GCC 4.3?
+ */
+
+static u32 spi_gpio_txrx_word_mode0(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha0(spi, nsecs, 0, word, bits);
+}
+
+static u32 spi_gpio_txrx_word_mode1(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha1(spi, nsecs, 0, word, bits);
+}
+
+static u32 spi_gpio_txrx_word_mode2(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha0(spi, nsecs, 1, word, bits);
+}
+
+static u32 spi_gpio_txrx_word_mode3(struct spi_device *spi,
+ unsigned nsecs, u32 word, u8 bits)
+{
+ return bitbang_txrx_be_cpha1(spi, nsecs, 1, word, bits);
+}
+
+/*----------------------------------------------------------------------*/
+
+static void spi_gpio_chipselect(struct spi_device *spi, int is_active)
+{
+ unsigned long cs = (unsigned long) spi->controller_data;
+
+ /* set initial clock polarity */
+ if (is_active)
+ setsck(spi, spi->mode & SPI_CPOL);
+
+ /* SPI is normally active-low */
+ gpio_set_value(cs, (spi->mode & SPI_CS_HIGH) ? is_active : !is_active);
+}
+
+static int spi_gpio_setup(struct spi_device *spi)
+{
+ unsigned long cs = (unsigned long) spi->controller_data;
+ int status = 0;
+
+ if (spi->bits_per_word > 32)
+ return -EINVAL;
+
+ if (!spi->controller_state) {
+ status = gpio_request(cs, spi->dev.bus_id);
+ if (status)
+ return status;
+ status = gpio_direction_output(cs, spi->mode & SPI_CS_HIGH);
+ }
+ if (!status)
+ status = spi_bitbang_setup(spi);
+ if (status) {
+ if (!spi->controller_state)
+ gpio_free(cs);
+ }
+ return status;
+}
+
+static void spi_gpio_cleanup(struct spi_device *spi)
+{
+ unsigned long cs = (unsigned long) spi->controller_data;
+
+ gpio_free(cs);
+ spi_bitbang_cleanup(spi);
+}
+
+static int __init spi_gpio_alloc(unsigned pin, const char *label, bool is_in)
+{
+ int value;
+
+ value = gpio_request(pin, label);
+ if (value == 0) {
+ if (is_in)
+ value = gpio_direction_input(pin);
+ else
+ value = gpio_direction_output(pin, 0);
+ }
+ return value;
+}
+
+static int __init
+spi_gpio_request(struct spi_gpio_platform_data *pdata, const char *label)
+{
+ int value;
+
+ /* NOTE: SPI_*_GPIO symbols may reference "pdata" */
+
+ value = spi_gpio_alloc(SPI_MOSI_GPIO, label, false);
+ if (value)
+ goto done;
+
+ value = spi_gpio_alloc(SPI_MISO_GPIO, label, true);
+ if (value)
+ goto free_mosi;
+
+ value = spi_gpio_alloc(SPI_SCK_GPIO, label, false);
+ if (value)
+ goto free_miso;
+
+ goto done;
+
+free_miso:
+ gpio_free(SPI_MISO_GPIO);
+free_mosi:
+ gpio_free(SPI_MOSI_GPIO);
+done:
+ return value;
+}
+
+static int __init spi_gpio_probe(struct platform_device *pdev)
+{
+ int status;
+ struct spi_master *master;
+ struct spi_gpio *spi_gpio;
+ struct spi_gpio_platform_data *pdata;
+
+ pdata = pdev->dev.platform_data;
+#ifdef GENERIC_BITBANG
+ if (!pdata || !pdata->num_chipselect)
+ return -ENODEV;
+#endif
+
+ status = spi_gpio_request(pdata, dev_name(&pdev->dev));
+ if (status < 0)
+ return status;
+
+ master = spi_alloc_master(&pdev->dev, sizeof *spi_gpio);
+ if (!master) {
+ status = -ENOMEM;
+ goto gpio_free;
+ }
+ spi_gpio = spi_master_get_devdata(master);
+ platform_set_drvdata(pdev, spi_gpio);
+
+ spi_gpio->pdev = pdev;
+ if (pdata)
+ spi_gpio->pdata = *pdata;
+
+ master->bus_num = pdev->id;
+ master->num_chipselect = SPI_N_CHIPSEL;
+ master->setup = spi_gpio_setup;
+ master->cleanup = spi_gpio_cleanup;
+
+ spi_gpio->bitbang.master = spi_master_get(master);
+ spi_gpio->bitbang.chipselect = spi_gpio_chipselect;
+ spi_gpio->bitbang.txrx_word[SPI_MODE_0] = spi_gpio_txrx_word_mode0;
+ spi_gpio->bitbang.txrx_word[SPI_MODE_1] = spi_gpio_txrx_word_mode1;
+ spi_gpio->bitbang.txrx_word[SPI_MODE_2] = spi_gpio_txrx_word_mode2;
+ spi_gpio->bitbang.txrx_word[SPI_MODE_3] = spi_gpio_txrx_word_mode3;
+ spi_gpio->bitbang.setup_transfer = spi_bitbang_setup_transfer;
+ spi_gpio->bitbang.flags = SPI_CS_HIGH;
+
+ status = spi_bitbang_start(&spi_gpio->bitbang);
+ if (status < 0) {
+ spi_master_put(spi_gpio->bitbang.master);
+gpio_free:
+ gpio_free(SPI_MISO_GPIO);
+ gpio_free(SPI_MOSI_GPIO);
+ gpio_free(SPI_SCK_GPIO);
+ spi_master_put(master);
+ }
+
+ return status;
+}
+
+static int __exit spi_gpio_remove(struct platform_device *pdev)
+{
+ struct spi_gpio *spi_gpio;
+ struct spi_gpio_platform_data *pdata;
+ int status;
+
+ spi_gpio = platform_get_drvdata(pdev);
+ pdata = pdev->dev.platform_data;
+
+ /* stop() unregisters child devices too */
+ status = spi_bitbang_stop(&spi_gpio->bitbang);
+ (void) spi_master_put(spi_gpio->bitbang.master);
+
+ platform_set_drvdata(pdev, NULL);
+
+ gpio_free(SPI_MISO_GPIO);
+ gpio_free(SPI_MOSI_GPIO);
+ gpio_free(SPI_SCK_GPIO);
+
+ return status;
+}
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+
+static struct platform_driver spi_gpio_driver = {
+ .driver.name = DRIVER_NAME,
+ .driver.owner = THIS_MODULE,
+ .remove = __exit_p(spi_gpio_remove),
+};
+
+static int __init spi_gpio_init(void)
+{
+ return platform_driver_probe(&spi_gpio_driver, spi_gpio_probe);
+}
+module_init(spi_gpio_init);
+
+static void __exit spi_gpio_exit(void)
+{
+ platform_driver_unregister(&spi_gpio_driver);
+}
+module_exit(spi_gpio_exit);
+
+
+MODULE_DESCRIPTION("SPI master driver using generic bitbanged GPIO ");
+MODULE_AUTHOR("David Brownell");
+MODULE_LICENSE("GPL");
--- /dev/null
+++ b/include/linux/spi/spi_gpio.h
@@ -0,0 +1,60 @@
+#ifndef __LINUX_SPI_GPIO_H
+#define __LINUX_SPI_GPIO_H
+
+/*
+ * For each bitbanged SPI bus, set up a platform_device node with:
+ * - name "spi_gpio"
+ * - id the same as the SPI bus number it implements
+ * - dev.platform data pointing to a struct spi_gpio_platform_data
+ *
+ * Or, see the driver code for information about speedups that are
+ * possible on platforms that support inlined access for GPIOs (no
+ * spi_gpio_platform_data is used).
+ *
+ * Use spi_board_info with these busses in the usual way, being sure
+ * that the controller_data being the GPIO used for each device's
+ * chipselect:
+ *
+ * static struct spi_board_info ... [] = {
+ * ...
+ * // this slave uses GPIO 42 for its chipselect
+ * .controller_data = (void *) 42,
+ * ...
+ * // this one uses GPIO 86 for its chipselect
+ * .controller_data = (void *) 86,
+ * ...
+ * };
+ *
+ * If the bitbanged bus is later switched to a "native" controller,
+ * that platform_device and controller_data should be removed.
+ */
+
+/**
+ * struct spi_gpio_platform_data - parameter for bitbanged SPI master
+ * @sck: number of the GPIO used for clock output
+ * @mosi: number of the GPIO used for Master Output, Slave In (MOSI) data
+ * @miso: number of the GPIO used for Master Input, Slave Output (MISO) data
+ * @num_chipselect: how many slaves to allow
+ *
+ * All GPIO signals used with the SPI bus managed through this driver
+ * (chipselects, MOSI, MISO, SCK) must be configured as GPIOs, instead
+ * of some alternate function.
+ *
+ * It can be convenient to use this driver with pins that have alternate
+ * functions associated with a "native" SPI controller if a driver for that
+ * controller is not available, or is missing important functionality.
+ *
+ * On platforms which can do so, configure MISO with a weak pullup unless
+ * there's an external pullup on that signal. That saves power by avoiding
+ * floating signals. (A weak pulldown would save power too, but many
+ * drivers expect to see all-ones data as the no slave "response".)
+ */
+struct spi_gpio_platform_data {
+ unsigned sck;
+ unsigned mosi;
+ unsigned miso;
+
+ u16 num_chipselect;
+};
+
+#endif /* __LINUX_SPI_GPIO_H */

2008-10-20 08:11:48

by Ben Dooks

[permalink] [raw]

Subject: Re: [spi-devel-general] [patch 2.6.27-git] spi_gpio driver

On Sun, Oct 19, 2008 at 09:24:36PM -0700, David Brownell wrote:
> From: David Brownell <[email protected]>
>
> Generalize the old at91rm9200 "bootstrap" bitbanging SPI master driver
> as "spi_gpio", so it works with arbitrary GPIOs and can be configured
> through platform_data. Such SPI masters support:
>
> - any number of bus instances (bus_num is the platform_device.id)
> - any number of chipselects (one GPIO per spi_device)
> - all four SPI_MODE values, and SPI_CS_HIGH
> - i/o word sizes from 1 to 32 bits;
> - devices configured as with any other spi_master controller

Probably worth looking at removing the s3c24xx gpio specific driver and
replacing it with this.

--
Ben

Q: What's a light-year?
A: One-third less calories than a regular year.

2008-10-20 08:37:17

[permalink] [raw]

Subject: Re: [spi-devel-general] [patch 2.6.27-git] spi_gpio driver

On Monday 20 October 2008, Ben Dooks wrote:
> On Sun, Oct 19, 2008 at 09:24:36PM -0700, David Brownell wrote:
> > From: David Brownell <[email protected]>
> >
> > Generalize the old at91rm9200 "bootstrap" bitbanging SPI master driver
> > as "spi_gpio", so it works with arbitrary GPIOs and can be configured
> > through platform_data. Such SPI masters support:
> >
> > - any number of bus instances (bus_num is the platform_device.id)
> > - any number of chipselects (one GPIO per spi_device)
> > - all four SPI_MODE values, and SPI_CS_HIGH
> > - i/o word sizes from 1 to 32 bits;
> > - devices configured as with any other spi_master controller
>
> Probably worth looking at removing the s3c24xx gpio specific driver and
> replacing it with this.

Good point. Let me know if you see any issues doing that,
beyond the need to change some boards' setup code and
defconfigs first.

- Dave

2008-10-20 09:48:55

[permalink] [raw]

Subject: Re: [PATCH v3] Add SPI over GPIO driver

On Monday 20 October 2008 06:16:51 David Brownell wrote:
> On Sunday 19 October 2008, Magnus Damm wrote:
> > >
> > > I fully support the idea of SPI over GPIO. ?It's just that
> > > particular patch which is problematic.
> >
> > Ok, let's fix those issues then!
>
> The issues I identified earlier still haven't been resolved...

What the hell are you talking about?
I rewrote it several times.

--
Greetings Michael.

2008-10-21 12:40:59