GPIO chip select support in McSPI

GPIO chip select support in McSPI

[Ben Gamari]

I've included the OMAP list so that I can hopefully get some feedback
from folks more familiar with this code.

Background:

I've been working on adding support for GPIO chip select lines to the
McSPI driver. Grant has been working with me to try getting the
in-kernel interface right and we have finally converged on a solution
whereby a table of GPIO lines will be passed to the McSPI driver through
platform_device.device.platform_data. Unfortunately, as explained below,
there is no clear path to support this in the current McSPI
initialization code.


On Thu, 03 Mar 2011 14:42:07 -0700, Grant Likely <grant.likely@secretlab.ca> wrote:
> The spi_master platform device is registered somewhere in the
> inappropriate support code. You need to arrange to make sure the
> correct pdata is attached to it when it gets registered. There
> /should/ be a mechanism for doing so.
> 
> All spi devices already specify a cs number anyway. What you can do is
> use the cs number as the lookup index into the gpio table, and
> remember to reserve a cs# for the 'native' cs line.
> 
I've done as you suggested and added support to the McSPI driver by
passing a GPIO table through platform_data. A patch will be coming
shortly.

Unfortunately, I'm really not sure how to restructure the OMAP code to
pass this information along. Currently the McSPI devices are registered
in arch/arm/mach-omap2/devices.c (omap_init_mcspi). In order to make the
GPIO CS support configurable by board code, we need some way to change
the (currently static) platform_devices prior to registration. Does
anyone have any suggestions on how this code could be refactored to
allow for this with minimal code duplication? Obviously we could just
move the platform_devices into the board files but this seems like a lot
of code duplication to support functionality that few boards will use.

Thoughts?

- Ben

[PATCH] mcspi: Add support for GPIO chip select lines

[Ben Gamari]

Many applications require more chip select lines than the board or
processor allow. Introduce a mechanism to allow use of GPIO pins as
chip select lines with the McSPI controller. To use this functionality,
one simply provides a table mapping CS line numbers to GPIO numbers in
omap2_mcspi_platform_config.cs_gpios.

Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
---
 arch/arm/plat-omap/include/plat/mcspi.h |    3 ++-
 drivers/spi/omap2_mcspi.c               |   15 ++++++++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcspi.h b/arch/arm/plat-omap/include/plat/mcspi.h
index 1254e49..cac1d84 100644
--- a/arch/arm/plat-omap/include/plat/mcspi.h
+++ b/arch/arm/plat-omap/include/plat/mcspi.h
@@ -2,7 +2,8 @@
 #define _OMAP2_MCSPI_H
 
 struct omap2_mcspi_platform_config {
-	unsigned short	num_cs;
+	unsigned short num_cs;
+	int *cs_gpios;
 };
 
 struct omap2_mcspi_device_config {
diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index abb1ffb..59cbed4 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -35,6 +35,7 @@
 #include <linux/slab.h>
 
 #include <linux/spi/spi.h>
+#include <linux/gpio.h>
 
 #include <plat/dma.h>
 #include <plat/clock.h>
@@ -235,11 +236,15 @@ static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
 
 static void omap2_mcspi_force_cs(struct spi_device *spi, int cs_active)
 {
-	u32 l;
-
-	l = mcspi_cached_chconf0(spi);
-	MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
-	mcspi_write_chconf0(spi, l);
+	struct omap2_mcspi_platform_config *pconfig = spi->master->dev.platform_data;
+	if (pconfig->cs_gpios) {
+		int gpio = pconfig->cs_gpios[spi->chip_select];
+		gpio_set_value(gpio, cs_active);
+	} else {
+		u32 l = mcspi_cached_chconf0(spi);
+		MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
+		mcspi_write_chconf0(spi, l);
+	}
 }
 
 static void omap2_mcspi_set_master_mode(struct spi_master *master)
-- 
1.7.0.4

Re: GPIO chip select support in McSPI

[Grant Likely]

On Sun, Mar 13, 2011 at 03:04:11PM -0400, Ben Gamari wrote:
> I've included the OMAP list so that I can hopefully get some feedback
> from folks more familiar with this code.
> 
> Background:
> 
> I've been working on adding support for GPIO chip select lines to the
> McSPI driver. Grant has been working with me to try getting the
> in-kernel interface right and we have finally converged on a solution
> whereby a table of GPIO lines will be passed to the McSPI driver through
> platform_device.device.platform_data. Unfortunately, as explained below,
> there is no clear path to support this in the current McSPI
> initialization code.
> 
> 
> On Thu, 03 Mar 2011 14:42:07 -0700, Grant Likely <grant.likely@secretlab.ca> wrote:
> > The spi_master platform device is registered somewhere in the
> > inappropriate support code. You need to arrange to make sure the
> > correct pdata is attached to it when it gets registered. There
> > /should/ be a mechanism for doing so.
> > 
> > All spi devices already specify a cs number anyway. What you can do is
> > use the cs number as the lookup index into the gpio table, and
> > remember to reserve a cs# for the 'native' cs line.
> > 
> I've done as you suggested and added support to the McSPI driver by
> passing a GPIO table through platform_data. A patch will be coming
> shortly.
> 
> Unfortunately, I'm really not sure how to restructure the OMAP code to
> pass this information along. Currently the McSPI devices are registered
> in arch/arm/mach-omap2/devices.c (omap_init_mcspi). In order to make the
> GPIO CS support configurable by board code, we need some way to change
> the (currently static) platform_devices prior to registration. Does
> anyone have any suggestions on how this code could be refactored to
> allow for this with minimal code duplication? Obviously we could just
> move the platform_devices into the board files but this seems like a lot
> of code duplication to support functionality that few boards will use.

I see two solutions:
1) platform code registers a bus notifier so that it gets called back
before the device gets bound to a driver.  Then it can augment the
platform data.
2) add an api to arch/arm/mach-omap2/devices.c for providing a cs
table before the device gets registered (must be called before
arch_initcall() time).

g.

Re: [PATCH] mcspi: Add support for GPIO chip select lines

[Grant Likely]

On Sun, Mar 13, 2011 at 03:05:19PM -0400, Ben Gamari wrote:
> Many applications require more chip select lines than the board or
> processor allow. Introduce a mechanism to allow use of GPIO pins as
> chip select lines with the McSPI controller. To use this functionality,
> one simply provides a table mapping CS line numbers to GPIO numbers in
> omap2_mcspi_platform_config.cs_gpios.
> 
> Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
> ---
>  arch/arm/plat-omap/include/plat/mcspi.h |    3 ++-
>  drivers/spi/omap2_mcspi.c               |   15 ++++++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcspi.h b/arch/arm/plat-omap/include/plat/mcspi.h
> index 1254e49..cac1d84 100644
> --- a/arch/arm/plat-omap/include/plat/mcspi.h
> +++ b/arch/arm/plat-omap/include/plat/mcspi.h
> @@ -2,7 +2,8 @@
>  #define _OMAP2_MCSPI_H
>  
>  struct omap2_mcspi_platform_config {
> -	unsigned short	num_cs;
> +	unsigned short num_cs;
> +	int *cs_gpios;
>  };
>  
>  struct omap2_mcspi_device_config {
> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> index abb1ffb..59cbed4 100644
> --- a/drivers/spi/omap2_mcspi.c
> +++ b/drivers/spi/omap2_mcspi.c
> @@ -35,6 +35,7 @@
>  #include <linux/slab.h>
>  
>  #include <linux/spi/spi.h>
> +#include <linux/gpio.h>
>  
>  #include <plat/dma.h>
>  #include <plat/clock.h>
> @@ -235,11 +236,15 @@ static void omap2_mcspi_set_enable(const struct spi_device *spi, int enable)
>  
>  static void omap2_mcspi_force_cs(struct spi_device *spi, int cs_active)
>  {
> -	u32 l;
> -
> -	l = mcspi_cached_chconf0(spi);
> -	MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
> -	mcspi_write_chconf0(spi, l);
> +	struct omap2_mcspi_platform_config *pconfig = spi->master->dev.platform_data;
> +	if (pconfig->cs_gpios) {
> +		int gpio = pconfig->cs_gpios[spi->chip_select];
> +		gpio_set_value(gpio, cs_active);
> +	} else {
> +		u32 l = mcspi_cached_chconf0(spi);
> +		MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
> +		mcspi_write_chconf0(spi, l);
> +	}

What if the board wanted to use both the native SPI ss line as well as
one or more GPIOs?  You probably want to reserve cs0 for the native
gpio line.

Otherwise this patch looks good to me.

g.

Re: [PATCH] mcspi: Add support for GPIO chip select lines

[Ben Gamari]

On Mon, 14 Mar 2011 13:27:18 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> What if the board wanted to use both the native SPI ss line as well as
> one or more GPIOs?  You probably want to reserve cs0 for the native
> gpio line.
> 
Hmm, I had thought about this and assumed it would be easiest to punt on
this, requiring the user to use the native line as a GPIO. This of
course assumes that all of the CS lines also have pinmux configurations
as GPIO pins. Is this not a good assumption? 

> Otherwise this patch looks good to me.
> 
Thanks!

- Ben

Re: [PATCH] mcspi: Add support for GPIO chip select lines

[Grant Likely]

On Mon, Mar 14, 2011 at 10:06:40PM -0400, Ben Gamari wrote:
> On Mon, 14 Mar 2011 13:27:18 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> > What if the board wanted to use both the native SPI ss line as well as
> > one or more GPIOs?  You probably want to reserve cs0 for the native
> > gpio line.
> > 
> Hmm, I had thought about this and assumed it would be easiest to punt on
> this, requiring the user to use the native line as a GPIO. This of
> course assumes that all of the CS lines also have pinmux configurations
> as GPIO pins. Is this not a good assumption? 

As a general principle I would say no since it would mean adding a 2nd
GPIO device will potentially break the first if the infrastructure
isn't in place to handle the first line as a gpio.

g.

Re: GPIO chip select support in McSPI

[Ben Gamari]

On Mon, 14 Mar 2011 13:25:36 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> I see two solutions:
> 1) platform code registers a bus notifier so that it gets called back
> before the device gets bound to a driver.  Then it can augment the
> platform data.
> 
This sounds like it might be a bit involved and I'm not terribly
familiar with these facets of the driver model. This would probably be
the more flexible approach but I think I'll stick with the simpler
option.

> 2) add an api to arch/arm/mach-omap2/devices.c for providing a cs
> table before the device gets registered (must be called before
> arch_initcall() time).
> 
This is along the lines of what I was thinking. Any thoughts on how to
get function called before arch_initcall()?

- Ben

Re: GPIO chip select support in McSPI

[Grant Likely]

On Mon, Mar 14, 2011 at 10:22:31PM -0400, Ben Gamari wrote:
> On Mon, 14 Mar 2011 13:25:36 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> > I see two solutions:
> > 1) platform code registers a bus notifier so that it gets called back
> > before the device gets bound to a driver.  Then it can augment the
> > platform data.
> > 
> This sounds like it might be a bit involved and I'm not terribly
> familiar with these facets of the driver model. This would probably be
> the more flexible approach but I think I'll stick with the simpler
> option.
> 
> > 2) add an api to arch/arm/mach-omap2/devices.c for providing a cs
> > table before the device gets registered (must be called before
> > arch_initcall() time).
> > 
> This is along the lines of what I was thinking. Any thoughts on how to
> get function called before arch_initcall()?

You can call it from .init_machine()

g.

> 
> - Ben