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

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Sun, Feb 13, 2011 at 05:10:33PM -0500, 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 specifies the GPIO number in spi_board_info.controller_data.
> 
> Signed-off-by: Ben Gamari <bgamari.foss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/omap2_mcspi.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> index 951a160..ca4a7dc 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,13 @@ 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) {
> +                gpio_set_value((unsigned) spi->controller_data, cs_active);
> +	} else {
> +		u32 l = mcspi_cached_chconf0(spi);
> +		MOD_REG_BIT(l, OMAP2_MCSPI_CHCONF_FORCE, cs_active);
> +		mcspi_write_chconf0(spi, l);
> +	}

Yes, this is a better approach.  However, I'd rather see the gpio data
passed as an array to the spi master platform_device via the
private_data structure.  controller_data is really intended to be a
pointer that the spi_master can populate if it needs to, so it is not
a good idea to populate it in setup code instead.  Also, I see cs line
wireups more as a property of the controller than a property of the
device (there is some debate on this area, but in the end it probably
is more important to be consistent about how the data is passed than
the relative merits between master-centric or slave-centric; which
really means I get to make the decision about which approach
spi_masters should take).

g.


------------------------------------------------------------------------------
Free Software Download: Index, Search & Analyze Logs and other IT data in 
Real-Time with Splunk. Collect, index and harness all the fast moving IT data 
generated by your applications, servers and devices whether physical, virtual
or in the cloud. Deliver compliance at lower cost and gain new business 
insights. http://p.sf.net/sfu/splunk-dev2dev