Fwd: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

Fwd: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

[Eric Miao]

[-- Attachment #1: Type: text/plain, Size: 8379 bytes --]

David,

I have to ping you again on this patch due to the coming merge window.


---------- Forwarded message ----------
From: Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue, Nov 25, 2008 at 3:17 PM
Subject: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to
simplify the common cases
To: LAK <linux-arm-kernel-xIg/pKzrS19vn6HldHNs0ANdhmdF6hFW@public.gmane.org>
Cc: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>, Richard Purdie <rpurdie-Fm38FmjxZ/leoWH0uzbU5w@public.gmane.org>


Most SPI peripherals use GPIOs as their chip selects, introduce .gpio_cs
for this.

Signed-off-by: Eric Miao <eric.miao-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
---

Now rebased and posted again.

 arch/arm/mach-pxa/include/mach/pxa2xx_spi.h |    1 +
 drivers/spi/pxa2xx_spi.c                    |   96 ++++++++++++++++++++++-----
 2 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
b/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
index 2206cb6..b87cecd 100644
--- a/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
+++ b/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
@@ -38,6 +38,7 @@ struct pxa2xx_spi_chip {
       u8 dma_burst_size;
       u32 timeout;
       u8 enable_loopback;
+       int gpio_cs;
       void (*cs_control)(u32 command);
 };

diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
index cf12f2d..f3587b4 100644
--- a/drivers/spi/pxa2xx_spi.c
+++ b/drivers/spi/pxa2xx_spi.c
@@ -28,6 +28,7 @@
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/clk.h>
+#include <linux/gpio.h>

 #include <asm/io.h>
 #include <asm/irq.h>
@@ -168,6 +169,8 @@ struct chip_data {
       u8 enable_dma;
       u8 bits_per_word;
       u32 speed_hz;
+       int gpio_cs;
+       int gpio_cs_inverted;
       int (*write)(struct driver_data *drv_data);
       int (*read)(struct driver_data *drv_data);
       void (*cs_control)(u32 command);
@@ -175,6 +178,32 @@ struct chip_data {

 static void pump_messages(struct work_struct *work);

+static void cs_assert(struct driver_data *drv_data)
+{
+       struct chip_data *chip = drv_data->cur_chip;
+
+       if (chip->cs_control) {
+               chip->cs_control(PXA2XX_CS_ASSERT);
+               return;
+       }
+
+       if (gpio_is_valid(chip->gpio_cs))
+               gpio_set_value(chip->gpio_cs, chip->gpio_cs_inverted);
+}
+
+static void cs_deassert(struct driver_data *drv_data)
+{
+       struct chip_data *chip = drv_data->cur_chip;
+
+       if (chip->cs_control) {
+               chip->cs_control(PXA2XX_CS_ASSERT);
+               return;
+       }
+
+       if (gpio_is_valid(chip->gpio_cs))
+               gpio_set_value(chip->gpio_cs, !chip->gpio_cs_inverted);
+}
+
 static int flush(struct driver_data *drv_data)
 {
       unsigned long limit = loops_per_jiffy << 1;
@@ -191,10 +220,6 @@ static int flush(struct driver_data *drv_data)
       return limit;
 }

-static void null_cs_control(u32 command)
-{
-}
-
 static int null_writer(struct driver_data *drv_data)
 {
       void __iomem *reg = drv_data->ioaddr;
@@ -402,7 +427,6 @@ static void giveback(struct driver_data *drv_data)
       msg = drv_data->cur_msg;
       drv_data->cur_msg = NULL;
       drv_data->cur_transfer = NULL;
-       drv_data->cur_chip = NULL;
       queue_work(drv_data->workqueue, &drv_data->pump_messages);
       spin_unlock_irqrestore(&drv_data->lock, flags);

@@ -418,7 +442,7 @@ static void giveback(struct driver_data *drv_data)
        * a message with an error, or next message is for another chip
        */
       if (!last_transfer->cs_change)
-               drv_data->cs_control(PXA2XX_CS_DEASSERT);
+               cs_deassert(drv_data);
       else {
               struct spi_message *next_msg;

@@ -447,12 +471,14 @@ static void giveback(struct driver_data *drv_data)
               if (next_msg && next_msg->spi != msg->spi)
                       next_msg = NULL;
               if (!next_msg || msg->state == ERROR_STATE)
-                       drv_data->cs_control(PXA2XX_CS_DEASSERT);
+                       cs_deassert(drv_data);
       }

       msg->state = NULL;
       if (msg->complete)
               msg->complete(msg->context);
+
+       drv_data->cur_chip = NULL;
 }

 static int wait_ssp_rx_stall(void const __iomem *ioaddr)
@@ -889,7 +915,7 @@ static void pump_transfers(unsigned long data)

               /* Drop chip select only if cs_change is requested */
               if (previous->cs_change)
-                       drv_data->cs_control(PXA2XX_CS_DEASSERT);
+                       cs_deassert(drv_data);
       }

       /* Check for transfers that need multiple DMA segments */
@@ -924,7 +950,6 @@ static void pump_transfers(unsigned long data)
       }
       drv_data->n_bytes = chip->n_bytes;
       drv_data->dma_width = chip->dma_width;
-       drv_data->cs_control = chip->cs_control;
       drv_data->tx = (void *)transfer->tx_buf;
       drv_data->tx_end = drv_data->tx + transfer->len;
       drv_data->rx = transfer->rx_buf;
@@ -1086,11 +1111,7 @@ static void pump_transfers(unsigned long data)
                       write_SSTO(chip->timeout, reg);
       }

-       /* FIXME, need to handle cs polarity,
-        * this driver uses struct pxa2xx_spi_chip.cs_control to
-        * specify a CS handling function, and it ignores most
-        * struct spi_device.mode[s], including SPI_CS_HIGH */
-       drv_data->cs_control(PXA2XX_CS_ASSERT);
+       cs_assert(drv_data);

       /* after chip select, release the data by enabling service
        * requests and interrupts, without changing any mode bits */
@@ -1168,6 +1189,44 @@ static int transfer(struct spi_device *spi,
struct spi_message *msg)
 /* the spi->mode bits understood by this driver: */
 #define MODEBITS (SPI_CPOL | SPI_CPHA)

+static int setup_cs(struct spi_device *spi, struct chip_data *chip,
+                   struct pxa2xx_spi_chip *chip_info)
+{
+       int err = 0;
+
+       if (chip == NULL || chip_info == NULL)
+               return 0;
+
+       /* NOTE: setup() can be called multiple times, possibly with
+        * different chip_info, release previously requested GPIO
+        */
+       if (gpio_is_valid(chip->gpio_cs))
+               gpio_free(chip->gpio_cs);
+
+       /* If (*cs_control) is provided, ignore GPIO chip select */
+       if (chip_info->cs_control) {
+               chip->cs_control = chip_info->cs_control;
+               return 0;
+       }
+
+       if (gpio_is_valid(chip_info->gpio_cs)) {
+               err = gpio_request(chip_info->gpio_cs, "SPI_CS");
+               if (err) {
+                       dev_err(&spi->dev, "failed to request chip select "
+                                       "GPIO%d\n", chip_info->gpio_cs);
+                       return err;
+               }
+
+               chip->gpio_cs = chip_info->gpio_cs;
+               chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
+
+               err = gpio_direction_output(chip->gpio_cs,
+                                       !chip->gpio_cs_inverted);
+       }
+
+       return err;
+}
+
 static int setup(struct spi_device *spi)
 {
       struct pxa2xx_spi_chip *chip_info = NULL;
@@ -1213,7 +1272,7 @@ static int setup(struct spi_device *spi)
                       return -ENOMEM;
               }

-               chip->cs_control = null_cs_control;
+               chip->gpio_cs = -1;
               chip->enable_dma = 0;
               chip->timeout = TIMOUT_DFLT;
               chip->dma_burst_size = drv_data->master_info->enable_dma ?
@@ -1227,8 +1286,6 @@ static int setup(struct spi_device *spi)
       /* chip_info isn't always needed */
       chip->cr1 = 0;
       if (chip_info) {
-               if (chip_info->cs_control)
-                       chip->cs_control = chip_info->cs_control;
               if (chip_info->timeout)
                       chip->timeout = chip_info->timeout;
               if (chip_info->tx_threshold)
@@ -1310,13 +1367,16 @@ static int setup(struct spi_device *spi)

       spi_set_ctldata(spi, chip);

-       return 0;
+       return setup_cs(spi, chip, chip_info);
 }

 static void cleanup(struct spi_device *spi)
 {
       struct chip_data *chip = spi_get_ctldata(spi);

+       if (gpio_is_valid(chip->gpio_cs))
+               gpio_free(chip->gpio_cs);
+
       kfree(chip);
 }

--
1.6.0.2



-- 
Cheers
- eric

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-spi-pxa2xx_spi-introduces-chipselect-GPIO-to-simpli.patch --]
[-- Type: text/x-diff; name=0001-spi-pxa2xx_spi-introduces-chipselect-GPIO-to-simpli.patch, Size: 6878 bytes --]

From 165dfa9bfdb9c167be526d51b2df2cfeb1a5a56a Mon Sep 17 00:00:00 2001
From: Eric Miao <eric.miao@marvell.com>
Date: Tue, 25 Nov 2008 15:06:35 +0800
Subject: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

Most SPI peripherals use GPIOs as their chip selects, introduce .gpio_cs
for this.

Signed-off-by: Eric Miao <eric.miao@marvell.com>
---
 arch/arm/mach-pxa/include/mach/pxa2xx_spi.h |    1 +
 drivers/spi/pxa2xx_spi.c                    |   96 ++++++++++++++++++++++-----
 2 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h b/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
index 2206cb6..b87cecd 100644
--- a/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
+++ b/arch/arm/mach-pxa/include/mach/pxa2xx_spi.h
@@ -38,6 +38,7 @@ struct pxa2xx_spi_chip {
 	u8 dma_burst_size;
 	u32 timeout;
 	u8 enable_loopback;
+	int gpio_cs;
 	void (*cs_control)(u32 command);
 };
 
diff --git a/drivers/spi/pxa2xx_spi.c b/drivers/spi/pxa2xx_spi.c
index cf12f2d..f3587b4 100644
--- a/drivers/spi/pxa2xx_spi.c
+++ b/drivers/spi/pxa2xx_spi.c
@@ -28,6 +28,7 @@
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/clk.h>
+#include <linux/gpio.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
@@ -168,6 +169,8 @@ struct chip_data {
 	u8 enable_dma;
 	u8 bits_per_word;
 	u32 speed_hz;
+	int gpio_cs;
+	int gpio_cs_inverted;
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
 	void (*cs_control)(u32 command);
@@ -175,6 +178,32 @@ struct chip_data {
 
 static void pump_messages(struct work_struct *work);
 
+static void cs_assert(struct driver_data *drv_data)
+{
+	struct chip_data *chip = drv_data->cur_chip;
+
+	if (chip->cs_control) {
+		chip->cs_control(PXA2XX_CS_ASSERT);
+		return;
+	}
+
+	if (gpio_is_valid(chip->gpio_cs))
+		gpio_set_value(chip->gpio_cs, chip->gpio_cs_inverted);
+}
+
+static void cs_deassert(struct driver_data *drv_data)
+{
+	struct chip_data *chip = drv_data->cur_chip;
+
+	if (chip->cs_control) {
+		chip->cs_control(PXA2XX_CS_ASSERT);
+		return;
+	}
+
+	if (gpio_is_valid(chip->gpio_cs))
+		gpio_set_value(chip->gpio_cs, !chip->gpio_cs_inverted);
+}
+
 static int flush(struct driver_data *drv_data)
 {
 	unsigned long limit = loops_per_jiffy << 1;
@@ -191,10 +220,6 @@ static int flush(struct driver_data *drv_data)
 	return limit;
 }
 
-static void null_cs_control(u32 command)
-{
-}
-
 static int null_writer(struct driver_data *drv_data)
 {
 	void __iomem *reg = drv_data->ioaddr;
@@ -402,7 +427,6 @@ static void giveback(struct driver_data *drv_data)
 	msg = drv_data->cur_msg;
 	drv_data->cur_msg = NULL;
 	drv_data->cur_transfer = NULL;
-	drv_data->cur_chip = NULL;
 	queue_work(drv_data->workqueue, &drv_data->pump_messages);
 	spin_unlock_irqrestore(&drv_data->lock, flags);
 
@@ -418,7 +442,7 @@ static void giveback(struct driver_data *drv_data)
 	 * a message with an error, or next message is for another chip
 	 */
 	if (!last_transfer->cs_change)
-		drv_data->cs_control(PXA2XX_CS_DEASSERT);
+		cs_deassert(drv_data);
 	else {
 		struct spi_message *next_msg;
 
@@ -447,12 +471,14 @@ static void giveback(struct driver_data *drv_data)
 		if (next_msg && next_msg->spi != msg->spi)
 			next_msg = NULL;
 		if (!next_msg || msg->state == ERROR_STATE)
-			drv_data->cs_control(PXA2XX_CS_DEASSERT);
+			cs_deassert(drv_data);
 	}
 
 	msg->state = NULL;
 	if (msg->complete)
 		msg->complete(msg->context);
+
+	drv_data->cur_chip = NULL;
 }
 
 static int wait_ssp_rx_stall(void const __iomem *ioaddr)
@@ -889,7 +915,7 @@ static void pump_transfers(unsigned long data)
 
 		/* Drop chip select only if cs_change is requested */
 		if (previous->cs_change)
-			drv_data->cs_control(PXA2XX_CS_DEASSERT);
+			cs_deassert(drv_data);
 	}
 
 	/* Check for transfers that need multiple DMA segments */
@@ -924,7 +950,6 @@ static void pump_transfers(unsigned long data)
 	}
 	drv_data->n_bytes = chip->n_bytes;
 	drv_data->dma_width = chip->dma_width;
-	drv_data->cs_control = chip->cs_control;
 	drv_data->tx = (void *)transfer->tx_buf;
 	drv_data->tx_end = drv_data->tx + transfer->len;
 	drv_data->rx = transfer->rx_buf;
@@ -1086,11 +1111,7 @@ static void pump_transfers(unsigned long data)
 			write_SSTO(chip->timeout, reg);
 	}
 
-	/* FIXME, need to handle cs polarity,
-	 * this driver uses struct pxa2xx_spi_chip.cs_control to
-	 * specify a CS handling function, and it ignores most
-	 * struct spi_device.mode[s], including SPI_CS_HIGH */
-	drv_data->cs_control(PXA2XX_CS_ASSERT);
+	cs_assert(drv_data);
 
 	/* after chip select, release the data by enabling service
 	 * requests and interrupts, without changing any mode bits */
@@ -1168,6 +1189,44 @@ static int transfer(struct spi_device *spi, struct spi_message *msg)
 /* the spi->mode bits understood by this driver: */
 #define MODEBITS (SPI_CPOL | SPI_CPHA)
 
+static int setup_cs(struct spi_device *spi, struct chip_data *chip,
+		    struct pxa2xx_spi_chip *chip_info)
+{
+	int err = 0;
+
+	if (chip == NULL || chip_info == NULL)
+		return 0;
+
+	/* NOTE: setup() can be called multiple times, possibly with
+	 * different chip_info, release previously requested GPIO
+	 */
+	if (gpio_is_valid(chip->gpio_cs))
+		gpio_free(chip->gpio_cs);
+
+	/* If (*cs_control) is provided, ignore GPIO chip select */
+	if (chip_info->cs_control) {
+		chip->cs_control = chip_info->cs_control;
+		return 0;
+	}
+
+	if (gpio_is_valid(chip_info->gpio_cs)) {
+		err = gpio_request(chip_info->gpio_cs, "SPI_CS");
+		if (err) {
+			dev_err(&spi->dev, "failed to request chip select "
+					"GPIO%d\n", chip_info->gpio_cs);
+			return err;
+		}
+
+		chip->gpio_cs = chip_info->gpio_cs;
+		chip->gpio_cs_inverted = spi->mode & SPI_CS_HIGH;
+
+		err = gpio_direction_output(chip->gpio_cs,
+					!chip->gpio_cs_inverted);
+	}
+
+	return err;
+}
+
 static int setup(struct spi_device *spi)
 {
 	struct pxa2xx_spi_chip *chip_info = NULL;
@@ -1213,7 +1272,7 @@ static int setup(struct spi_device *spi)
 			return -ENOMEM;
 		}
 
-		chip->cs_control = null_cs_control;
+		chip->gpio_cs = -1;
 		chip->enable_dma = 0;
 		chip->timeout = TIMOUT_DFLT;
 		chip->dma_burst_size = drv_data->master_info->enable_dma ?
@@ -1227,8 +1286,6 @@ static int setup(struct spi_device *spi)
 	/* chip_info isn't always needed */
 	chip->cr1 = 0;
 	if (chip_info) {
-		if (chip_info->cs_control)
-			chip->cs_control = chip_info->cs_control;
 		if (chip_info->timeout)
 			chip->timeout = chip_info->timeout;
 		if (chip_info->tx_threshold)
@@ -1310,13 +1367,16 @@ static int setup(struct spi_device *spi)
 
 	spi_set_ctldata(spi, chip);
 
-	return 0;
+	return setup_cs(spi, chip, chip_info);
 }
 
 static void cleanup(struct spi_device *spi)
 {
 	struct chip_data *chip = spi_get_ctldata(spi);
 
+	if (gpio_is_valid(chip->gpio_cs))
+		gpio_free(chip->gpio_cs);
+
 	kfree(chip);
 }
 
-- 
1.6.0.2


[-- Attachment #3: Type: text/plain, Size: 366 bytes --]

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

[-- Attachment #4: Type: text/plain, Size: 210 bytes --]

_______________________________________________
spi-devel-general mailing list
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Re: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

[Eric Miao]

On Fri, Dec 5, 2008 at 10:28 AM, Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> David,
>
> I have to ping you again on this patch due to the coming merge window.
>

Well, one week passed and still no response, I'll push this
one through my tree anyway.

Thanks

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

[David Brownell]

On Thursday 11 December 2008, Eric Miao wrote:
> On Fri, Dec 5, 2008 at 10:28 AM, Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > David,
> >
> > I have to ping you again on this patch due to the coming merge window.
> >
> 
> Well, one week passed and still no response, I'll push this
> one through my tree anyway.

I'm still catching up on stuff ... what's the rush?

It's probably OK -- it was almost OK before -- but I'd
still rather send it with my next batch of SPI updates.

- Dave

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

[Eric Miao]

On Thu, Dec 11, 2008 at 6:36 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> On Thursday 11 December 2008, Eric Miao wrote:
>> On Fri, Dec 5, 2008 at 10:28 AM, Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > David,
>> >
>> > I have to ping you again on this patch due to the coming merge window.
>> >
>>
>> Well, one week passed and still no response, I'll push this
>> one through my tree anyway.
>
> I'm still catching up on stuff ... what's the rush?
>

Rush for your quick confirmation, I have several other
platform-specific patches depending on this one.

> It's probably OK -- it was almost OK before -- but I'd
> still rather send it with my next batch of SPI updates.
>

Good.

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

Re: [PATCH] spi: pxa2xx_spi introduces chipselect GPIO to simplify the common cases

[Eric Miao]

On Thu, Dec 11, 2008 at 6:43 PM, Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Dec 11, 2008 at 6:36 PM, David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
>> On Thursday 11 December 2008, Eric Miao wrote:
>>> On Fri, Dec 5, 2008 at 10:28 AM, Eric Miao <eric.y.miao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> > David,
>>> >
>>> > I have to ping you again on this patch due to the coming merge window.
>>> >
>>>
>>> Well, one week passed and still no response, I'll push this
>>> one through my tree anyway.
>>
>> I'm still catching up on stuff ... what's the rush?
>>
>
> Rush for your quick confirmation, I have several other
> platform-specific patches depending on this one.
>
>> It's probably OK -- it was almost OK before -- but I'd
>> still rather send it with my next batch of SPI updates.
>>

I'm not seeing the patches in Linus' tree, could you please help check?
And please also propose constructional suggestions to my
platform_device_id patch, ok?


-- 
Cheers
- eric

------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword