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

2011-08-30 Thread 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
tionality,
one simply provides a table mapping CS line numbers to GPIO numbers

omap2_mcspi_platform_config.cs_gpios.

Signed-off-by: Ben Gamari 
---
 arch/arm/plat-omap/include/plat/mcspi.h |1 +
 drivers/spi/omap2_mcspi.c   |   19 +--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcspi.h 
b/arch/arm/plat-omap/include/plat/mcspi.h
index 3d51b18..c5670f2 100644
--- a/arch/arm/plat-omap/include/plat/mcspi.h
+++ b/arch/arm/plat-omap/include/plat/mcspi.h
@@ -10,6 +10,7 @@
 struct omap2_mcspi_platform_config {
unsigned short  num_cs;
unsigned int regs_offset;
+   int *cs_gpios;
 };
 
 struct omap2_mcspi_dev_attr {
diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index f41c906..20dec5d 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -36,6 +36,7 @@
 #include 
 
 #include 
+#include 
 
 #include 
 #include 
@@ -121,6 +122,7 @@ struct omap2_mcspi {
/* SPI1 has 4 channels, while SPI2 has 2 */
struct omap2_mcspi_dma  *dma_channels;
struct  device  *dev;
+int *cs_gpios;
 };
 
 struct omap2_mcspi_cs {
@@ -227,7 +229,11 @@ 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;
+   struct omap2_mcspi* mcspi = spi_master_get_devdata(spi->master);
+   if (mcspi->cs_gpios)
+   gpio_set_value(mcspi->cs_gpios[spi->chip_select], cs_active);
 
+   // TXS times out unless we force the CHCONF reg as well
l = mcspi_cached_chconf0(spi);
MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
mcspi_write_chconf0(spi, l);
@@ -1088,6 +1094,8 @@ static int __init omap2_mcspi_probe(struct 
platform_device *pdev)
struct omap2_mcspi  *mcspi;
struct resource *r;
int status = 0, i;
+   struct omap2_mcspi_platform_config* pconfig = pdev->dev.platform_data;
+   int num_dma;
 
master = spi_alloc_master(&pdev->dev, sizeof *mcspi);
if (master == NULL) {
@@ -1110,6 +1118,13 @@ static int __init omap2_mcspi_probe(struct 
platform_device *pdev)
 
mcspi = spi_master_get_devdata(master);
mcspi->master = master;
+   if (pconfig && pconfig->cs_gpios) {
+   mcspi->cs_gpios = pconfig->cs_gpios;
+   num_dma = 1;
+   } else {
+   mcspi->cs_gpios = NULL;
+   num_dma = master->num_chipselect;
+   }
 
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (r == NULL) {
@@ -1139,14 +1154,14 @@ static int __init omap2_mcspi_probe(struct 
platform_device *pdev)
INIT_LIST_HEAD(&mcspi->msg_queue);
INIT_LIST_HEAD(&omap2_mcspi_ctx[master->bus_num - 1].cs);
 
-   mcspi->dma_channels = kcalloc(master->num_chipselect,
+   mcspi->dma_channels = kcalloc(num_dma,
sizeof(struct omap2_mcspi_dma),
GFP_KERNEL);
 
if (mcspi->dma_channels == NULL)
goto err2;
 
-   for (i = 0; i < master->num_chipselect; i++) {
+   for (i = 0; i < num_dma; i++) {
char dma_ch_name[14];
struct resource *dma_res;
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-03-13 Thread 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 
---
 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 
 
 #include 
+#include 
 
 #include 
 #include 
@@ -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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-21 Thread Ben Gamari
This mechanism is in large part stolen from the s3c64xx-spi module. To
use this functionality, one simply must define a set_level function to
set the CS state and a omap2_mcspi_csinfo struct for each chip select in
the board file.

Each spi_board_info.controller_data should then be set
to point to the appropriate csinfo struct. This will cause the driver to
call the csinfo->set_level function instead of toggling the McSPI chip
select lines.

Signed-off-by: Ben Gamari 
---
 arch/arm/plat-omap/include/plat/mcspi.h |   14 ++
 drivers/spi/omap2_mcspi.c   |   14 +-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcspi.h 
b/arch/arm/plat-omap/include/plat/mcspi.h
index 1254e49..ab84b8d 100644
--- a/arch/arm/plat-omap/include/plat/mcspi.h
+++ b/arch/arm/plat-omap/include/plat/mcspi.h
@@ -1,6 +1,20 @@
 #ifndef _OMAP2_MCSPI_H
 #define _OMAP2_MCSPI_H
 
+/**
+ * struct omap2_mcspi_csinfo - Chip Select description
+ * @line: Custom 'identity' of the CS line
+ * @set_level: Function to set the state of a given CS line
+ *
+ * This is to allow use of GPIO lines as CS lines. Allocate and initialize one
+ * in the machine init code and make spi_board_info.controller_data point to
+ * it.
+ */
+struct omap2_mcspi_csinfo {
+unsigned line;
+void (*set_level)(unsigned line_id, int lvl);
+};
+
 struct omap2_mcspi_platform_config {
unsigned short  num_cs;
 };
diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
index 2a651e6..92ccbd6 100644
--- a/drivers/spi/omap2_mcspi.c
+++ b/drivers/spi/omap2_mcspi.c
@@ -35,6 +35,7 @@
 #include 
 
 #include 
+#include 
 
 #include 
 #include 
@@ -235,11 +236,14 @@ 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);
+   if (spi->controller_data) {
+   struct omap2_mcspi_csinfo *csinfo = spi->controller_data;
+   (*csinfo->set_level)(csinfo->line, 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.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-03-14 Thread 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 
> ---
>  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 
>  
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -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.


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-03-14 Thread Ben Gamari
On Mon, 14 Mar 2011 13:27:18 -0600, Grant Likely  
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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-03-14 Thread 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  
> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Tony Lindgren
* Ben Gamari  [101221 09:56]:
> This mechanism is in large part stolen from the s3c64xx-spi module. To
> use this functionality, one simply must define a set_level function to
> set the CS state and a omap2_mcspi_csinfo struct for each chip select in
> the board file.
> 
> Each spi_board_info.controller_data should then be set
> to point to the appropriate csinfo struct. This will cause the driver to
> call the csinfo->set_level function instead of toggling the McSPI chip
> select lines.
> 
> Signed-off-by: Ben Gamari 

This one should go via the SPI list:

Acked-by: Tony Lindgren 

> ---
>  arch/arm/plat-omap/include/plat/mcspi.h |   14 ++
>  drivers/spi/omap2_mcspi.c   |   14 +-
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcspi.h 
> b/arch/arm/plat-omap/include/plat/mcspi.h
> index 1254e49..ab84b8d 100644
> --- a/arch/arm/plat-omap/include/plat/mcspi.h
> +++ b/arch/arm/plat-omap/include/plat/mcspi.h
> @@ -1,6 +1,20 @@
>  #ifndef _OMAP2_MCSPI_H
>  #define _OMAP2_MCSPI_H
>  
> +/**
> + * struct omap2_mcspi_csinfo - Chip Select description
> + * @line: Custom 'identity' of the CS line
> + * @set_level: Function to set the state of a given CS line
> + *
> + * This is to allow use of GPIO lines as CS lines. Allocate and initialize 
> one
> + * in the machine init code and make spi_board_info.controller_data point to
> + * it.
> + */
> +struct omap2_mcspi_csinfo {
> +unsigned line;
> +void (*set_level)(unsigned line_id, int lvl);
> +};
> +
>  struct omap2_mcspi_platform_config {
>   unsigned short  num_cs;
>  };
> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> index 2a651e6..92ccbd6 100644
> --- a/drivers/spi/omap2_mcspi.c
> +++ b/drivers/spi/omap2_mcspi.c
> @@ -35,6 +35,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -235,11 +236,14 @@ 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);
> + if (spi->controller_data) {
> + struct omap2_mcspi_csinfo *csinfo = spi->controller_data;
> + (*csinfo->set_level)(csinfo->line, 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.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Grant Likely
On Tue, Dec 21, 2010 at 10:56 AM, Ben Gamari  wrote:
> This mechanism is in large part stolen from the s3c64xx-spi module. To
> use this functionality, one simply must define a set_level function to
> set the CS state and a omap2_mcspi_csinfo struct for each chip select in
> the board file.
>
> Each spi_board_info.controller_data should then be set
> to point to the appropriate csinfo struct. This will cause the driver to
> call the csinfo->set_level function instead of toggling the McSPI chip
> select lines.
>
> Signed-off-by: Ben Gamari 

I'd rather see the spi driver modified to use the gpio api directly.
The drivers are already tending in that direction and it doesn't
require machine specific set_level functions to be defined.

g.

> ---
>  arch/arm/plat-omap/include/plat/mcspi.h |   14 ++
>  drivers/spi/omap2_mcspi.c               |   14 +-
>  2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mcspi.h 
> b/arch/arm/plat-omap/include/plat/mcspi.h
> index 1254e49..ab84b8d 100644
> --- a/arch/arm/plat-omap/include/plat/mcspi.h
> +++ b/arch/arm/plat-omap/include/plat/mcspi.h
> @@ -1,6 +1,20 @@
>  #ifndef _OMAP2_MCSPI_H
>  #define _OMAP2_MCSPI_H
>
> +/**
> + * struct omap2_mcspi_csinfo - Chip Select description
> + * @line: Custom 'identity' of the CS line
> + * @set_level: Function to set the state of a given CS line
> + *
> + * This is to allow use of GPIO lines as CS lines. Allocate and initialize 
> one
> + * in the machine init code and make spi_board_info.controller_data point to
> + * it.
> + */
> +struct omap2_mcspi_csinfo {
> +        unsigned line;
> +        void (*set_level)(unsigned line_id, int lvl);
> +};
> +
>  struct omap2_mcspi_platform_config {
>        unsigned short  num_cs;
>  };
> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> index 2a651e6..92ccbd6 100644
> --- a/drivers/spi/omap2_mcspi.c
> +++ b/drivers/spi/omap2_mcspi.c
> @@ -35,6 +35,7 @@
>  #include 
>
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -235,11 +236,14 @@ 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);
> +       if (spi->controller_data) {
> +               struct omap2_mcspi_csinfo *csinfo = spi->controller_data;
> +               (*csinfo->set_level)(csinfo->line, 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.1
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Ben Gamari
On Thu, 23 Dec 2010 14:38:57 -0700, Grant Likely  
wrote:
> On Tue, Dec 21, 2010 at 10:56 AM, Ben Gamari  wrote:
> > This mechanism is in large part stolen from the s3c64xx-spi module. To
> > use this functionality, one simply must define a set_level function to
> > set the CS state and a omap2_mcspi_csinfo struct for each chip select in
> > the board file.
> >
> > Each spi_board_info.controller_data should then be set
> > to point to the appropriate csinfo struct. This will cause the driver to
> > call the csinfo->set_level function instead of toggling the McSPI chip
> > select lines.
> >
> > Signed-off-by: Ben Gamari 
> 
> I'd rather see the spi driver modified to use the gpio api directly.
> The drivers are already tending in that direction and it doesn't
> require machine specific set_level functions to be defined.
> 
The reason I left this up to the board is it's easy to foresee cases
where you want a non-trivial mapping between logical CS numbers and CS
pin states. In my case, I using a 2-to-4 multiplexer as the source of
chip select.

- Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Grant Likely
On Thu, Dec 23, 2010 at 4:09 PM, Ben Gamari  wrote:
> On Thu, 23 Dec 2010 14:38:57 -0700, Grant Likely  
> wrote:
>> On Tue, Dec 21, 2010 at 10:56 AM, Ben Gamari  wrote:
>> > This mechanism is in large part stolen from the s3c64xx-spi module. To
>> > use this functionality, one simply must define a set_level function to
>> > set the CS state and a omap2_mcspi_csinfo struct for each chip select in
>> > the board file.
>> >
>> > Each spi_board_info.controller_data should then be set
>> > to point to the appropriate csinfo struct. This will cause the driver to
>> > call the csinfo->set_level function instead of toggling the McSPI chip
>> > select lines.
>> >
>> > Signed-off-by: Ben Gamari 
>>
>> I'd rather see the spi driver modified to use the gpio api directly.
>> The drivers are already tending in that direction and it doesn't
>> require machine specific set_level functions to be defined.
>>
> The reason I left this up to the board is it's easy to foresee cases
> where you want a non-trivial mapping between logical CS numbers and CS
> pin states. In my case, I using a 2-to-4 multiplexer as the source of
> chip select.

Hi Ben,

I understand and appreciate the motivation.  However in practice, the
gpio api is sufficient for pretty much any use case, even when the
backing gpio controller driver ends up driving some oddball device
with different constraints.  The big downside to using a callback is
that it forces all users to do the extra work of implementing the
callback.  With the gpio api, only the oddball cases have to do extra
code (to adapt the custom device to the gpio api).

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Ben Gamari
On Thu, 23 Dec 2010 17:37:12 -0700, Grant Likely  
wrote:
> On Thu, Dec 23, 2010 at 4:09 PM, Ben Gamari  wrote:
> > The reason I left this up to the board is it's easy to foresee cases
> > where you want a non-trivial mapping between logical CS numbers and CS
> > pin states. In my case, I using a 2-to-4 multiplexer as the source of
> > chip select.
> 
> Hi Ben,
> 
> I understand and appreciate the motivation.  However in practice, the
> gpio api is sufficient for pretty much any use case, even when the
> backing gpio controller driver ends up driving some oddball device
> with different constraints.  The big downside to using a callback is
> that it forces all users to do the extra work of implementing the
> callback.  With the gpio api, only the oddball cases have to do extra
> code (to adapt the custom device to the gpio api).

I understand your concerns, but I'm not sure how to satisfy them without
crippling the design's ability to accomodate my use-case. I can't pass a
GPIO line per spi_board_info since in my case of a multiplexed CS
configuration a single pin's state does not uniquely determine the
desired CS. The only other option I can think of is that we somehow
provide a list of GPIOs for each bus and map the CS numbers to
permutations of GPIO states. Unfortunately, I don't know of any suitable
structure to put this GPIO list in. Perhaps I'm missing something obvious?

- Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Grant Likely
On Thu, Dec 23, 2010 at 09:27:20PM -0500, Ben Gamari wrote:
> On Thu, 23 Dec 2010 17:37:12 -0700, Grant Likely  
> wrote:
> > On Thu, Dec 23, 2010 at 4:09 PM, Ben Gamari  wrote:
> > > The reason I left this up to the board is it's easy to foresee cases
> > > where you want a non-trivial mapping between logical CS numbers and CS
> > > pin states. In my case, I using a 2-to-4 multiplexer as the source of
> > > chip select.
> > 
> > Hi Ben,
> > 
> > I understand and appreciate the motivation.  However in practice, the
> > gpio api is sufficient for pretty much any use case, even when the
> > backing gpio controller driver ends up driving some oddball device
> > with different constraints.  The big downside to using a callback is
> > that it forces all users to do the extra work of implementing the
> > callback.  With the gpio api, only the oddball cases have to do extra
> > code (to adapt the custom device to the gpio api).
> 
> I understand your concerns, but I'm not sure how to satisfy them without
> crippling the design's ability to accomodate my use-case. I can't pass a
> GPIO line per spi_board_info since in my case of a multiplexed CS
> configuration a single pin's state does not uniquely determine the
> desired CS. The only other option I can think of is that we somehow
> provide a list of GPIOs for each bus and map the CS numbers to
> permutations of GPIO states. Unfortunately, I don't know of any suitable
> structure to put this GPIO list in. Perhaps I'm missing something obvious?

Close, but not quite.  Assign one gpio number to each cs state, and
write a gpio controller driver that maps the linux-gpio number to the
physical gpio state permutation.  The mapping from gpio# to ss# is
1:1, but the driver behind the gpio# can do whatever you need it to
do.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-23 Thread Ben Gamari
On Thu, 23 Dec 2010 20:28:19 -0700, Grant Likely  
wrote:
> On Thu, Dec 23, 2010 at 09:27:20PM -0500, Ben Gamari wrote:
> > I understand your concerns, but I'm not sure how to satisfy them without
> > crippling the design's ability to accomodate my use-case. I can't pass a
> > GPIO line per spi_board_info since in my case of a multiplexed CS
> > configuration a single pin's state does not uniquely determine the
> > desired CS. The only other option I can think of is that we somehow
> > provide a list of GPIOs for each bus and map the CS numbers to
> > permutations of GPIO states. Unfortunately, I don't know of any suitable
> > structure to put this GPIO list in. Perhaps I'm missing something obvious?
> 
> Close, but not quite.  Assign one gpio number to each cs state, and
> write a gpio controller driver that maps the linux-gpio number to the
> physical gpio state permutation.  The mapping from gpio# to ss# is
> 1:1, but the driver behind the gpio# can do whatever you need it to
> do.
> 
I see. I'm still not convinced that this is the route to take,
however. It seems like this virtual gpio interface is not only pretty
clunky (simple board file glue turns into an entire gpio chip driver),
it seems like this is a very inaccurate and not very useful way to
expose a multiplexed CS configuration; e.g. what is this chip driver to
do if the user tries to set two CS pins at once? Is there precedent for
using the GPIO subsystem in this way?

Cheers,

- Ben

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-02-12 Thread Grant Likely
On Fri, Dec 24, 2010 at 01:05:56AM -0500, Ben Gamari wrote:
> On Thu, 23 Dec 2010 20:28:19 -0700, Grant Likely  
> wrote:
> > On Thu, Dec 23, 2010 at 09:27:20PM -0500, Ben Gamari wrote:
> > > I understand your concerns, but I'm not sure how to satisfy them without
> > > crippling the design's ability to accomodate my use-case. I can't pass a
> > > GPIO line per spi_board_info since in my case of a multiplexed CS
> > > configuration a single pin's state does not uniquely determine the
> > > desired CS. The only other option I can think of is that we somehow
> > > provide a list of GPIOs for each bus and map the CS numbers to
> > > permutations of GPIO states. Unfortunately, I don't know of any suitable
> > > structure to put this GPIO list in. Perhaps I'm missing something obvious?
> > 
> > Close, but not quite.  Assign one gpio number to each cs state, and
> > write a gpio controller driver that maps the linux-gpio number to the
> > physical gpio state permutation.  The mapping from gpio# to ss# is
> > 1:1, but the driver behind the gpio# can do whatever you need it to
> > do.
> > 
> I see. I'm still not convinced that this is the route to take,
> however. It seems like this virtual gpio interface is not only pretty
> clunky (simple board file glue turns into an entire gpio chip driver),
> it seems like this is a very inaccurate and not very useful way to
> expose a multiplexed CS configuration; e.g. what is this chip driver to
> do if the user tries to set two CS pins at once?

(Sorry for the really laggy reply)

I don't see it being that big a deal.  gpio drivers are pretty
lightweight and it is fine to have domain-specific limitations on how
gpios for a particular "gpio controller" behave.  In your example, if
the hardware doesn't support enabling 2 CS pins at once, then you make
a choice, either setting one when the other is already set simply does
not work; or it could reset the other state.  Either choice would be
fine.  The SPI driver must do the right thing regardless and deselect
the previous CS line before enabling a new one.

The other alternative would be to implement a new SPI chipselect
common infrastructure, but IMO, that would just end up looking like a
duplication of the gpio infrastructure.

Regardless, my point still stands, platform callbacks for what amounts
to gpio manipulations doesn't make a whole lot of sense.

> Is there precedent for
> using the GPIO subsystem in this way?

Yes, in drivers/spi look at mpc52xx_spi.c, davinci_spi.c, ath79_spi.c,
atmel_spi.c, and many more.  Grep for gpio in drivers/spi.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2011-02-13 Thread Ben Gamari
On Sat, 12 Feb 2011 01:33:24 -0700, Grant Likely  
wrote:
> (Sorry for the really laggy reply)
> 
No worries, I was just starting to think about this again myself.

> I don't see it being that big a deal.  gpio drivers are pretty
> lightweight and it is fine to have domain-specific limitations on how
> gpios for a particular "gpio controller" behave.  In your example, if
> the hardware doesn't support enabling 2 CS pins at once, then you make
> a choice, either setting one when the other is already set simply does
> not work; or it could reset the other state.  Either choice would be
> fine.  The SPI driver must do the right thing regardless and deselect
> the previous CS line before enabling a new one.
> 
Having now finally having had a chance to implement a gpio_chip driver,
I agree. It's quite simple and seems to fill the need pretty well

> The other alternative would be to implement a new SPI chipselect
> common infrastructure, but IMO, that would just end up looking like a
> duplication of the gpio infrastructure.
> 
I think you are probably right. GPIO seems to be a good fit.

Patch coming shortly.  I use spi_board_info.controller_data to pass the
gpio number to the controller which doesn't seem like the best way
forward, but I wasn't sure how else to proceed. Also, since the gpio
numbers are dynamically determined spi_board_info.controller_data must
be filled in at runtime, which is also a bit of a bummer (although not
easily solved as far as I can tell). Let me know what you think.

 - Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html