[PATCH v2 0/2] spi: davinci: add support for gpio cs through dt

[PATCH v2 0/2] spi: davinci: add support for gpio cs through dt

[Grygorii Strashko]

This small series enables GPIO chip select feature for Davinci SPI
when DT-boot mode is used.
This is actual for Keystone 2 which supports DT-boot mode only.

Changes in v2:
- gpio_request_one() is used
- as suggested by Mark Brown, new patch has been added which updates
  Davinci SPI driver to use cs_gpio field
  of SPI device structure (spi_device) for both DT and non-DT cases.

v1:
 http://www.spinics.net/lists/linux-spi/msg01446.html

Grygorii Strashko (1):
  spi: davinci: use spi_device.cs_gpio to store gpio cs per spi device

Murali Karicheri (1):
  spi: davinci: add support to configure gpio cs through dt

 .../devicetree/bindings/spi/spi-davinci.txt        |    9 ++-
 drivers/spi/spi-davinci.c                          |   60 ++++++++++++++------
 2 files changed, 52 insertions(+), 17 deletions(-)

-- 
1.7.9.5

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

[PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Grygorii Strashko]

From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>

Currently driver supports only configuration of GPIO CS through
platform data. This patch enhances the driver to configure GPIO
CS through DT. Also update the DT binding documentation to
reflect the availability of cs-gpios.

Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/spi/spi-davinci.txt        |    9 ++-
 drivers/spi/spi-davinci.c                          |   64 +++++++++++++++++---
 2 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt b/Documentation/devicetree/bindings/spi/spi-davinci.txt
index 6d0ac8d..f80887b 100644
--- a/Documentation/devicetree/bindings/spi/spi-davinci.txt
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -8,7 +8,8 @@ Required properties:
 	- "ti,dm6441-spi" for SPI used similar to that on DM644x SoC family
 	- "ti,da830-spi" for SPI used similar to that on DA8xx SoC family
 - reg: Offset and length of SPI controller register space
-- num-cs: Number of chip selects
+- num-cs: Number of chip selects. This includes internal as well as
+	GPIO chip selects.
 - ti,davinci-spi-intr-line: interrupt line used to connect the SPI
 	IP to the interrupt controller within the SoC. Possible values
 	are 0 and 1. Manual says one of the two possible interrupt
@@ -17,6 +18,12 @@ Required properties:
 - interrupts: interrupt number mapped to CPU.
 - clocks: spi clk phandle
 
+Optional:
+- cs-gpios: gpio chip selects
+	For example to have 3 internal CS and 2 GPIO CS, user could define
+	cs-gpios = <0>, <0>, <0>, <&gpio1 30 0>, <&gpio1 31 0>;
+	where first three are internal CS and last two are GPIO CS.
+
 Example of a NOR flash slave device (n25q032) connected to DaVinci
 SPI controller device over the SPI bus.
 
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 2477af4..ac4414e0 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -30,6 +30,7 @@
 #include <linux/edma.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi_bitbang.h>
 #include <linux/slab.h>
@@ -207,17 +208,28 @@ static inline void clear_io_bits(void __iomem *addr, u32 bits)
 static void davinci_spi_chipselect(struct spi_device *spi, int value)
 {
 	struct davinci_spi *dspi;
+	struct device_node *np = spi->dev.of_node;
 	struct davinci_spi_platform_data *pdata;
+	struct spi_master *master = spi->master;
 	u8 chip_sel = spi->chip_select;
 	u16 spidat1 = CS_DEFAULT;
 	bool gpio_chipsel = false;
+	int gpio;
 
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
-	if (pdata->chip_sel && chip_sel < pdata->num_chipselect &&
-				pdata->chip_sel[chip_sel] != SPI_INTERN_CS)
+	if (np && master->cs_gpios != NULL && spi->cs_gpio >= 0) {
+		/* SPI core parse and update master->cs_gpio */
 		gpio_chipsel = true;
+		gpio = spi->cs_gpio;
+	} else if (pdata->chip_sel &&
+		   chip_sel < pdata->num_chipselect &&
+		   pdata->chip_sel[chip_sel] != SPI_INTERN_CS) {
+		/* platform data defines chip_sel */
+		gpio_chipsel = true;
+		gpio = pdata->chip_sel[chip_sel];
+	}
 
 	/*
 	 * Board specific chip select logic decides the polarity and cs
@@ -225,9 +237,9 @@ static void davinci_spi_chipselect(struct spi_device *spi, int value)
 	 */
 	if (gpio_chipsel) {
 		if (value == BITBANG_CS_ACTIVE)
-			gpio_set_value(pdata->chip_sel[chip_sel], 0);
+			gpio_set_value(gpio, 0);
 		else
-			gpio_set_value(pdata->chip_sel[chip_sel], 1);
+			gpio_set_value(gpio, 1);
 	} else {
 		if (value == BITBANG_CS_ACTIVE) {
 			spidat1 |= SPIDAT1_CSHOLD_MASK;
@@ -390,17 +402,41 @@ static int davinci_spi_setup(struct spi_device *spi)
 	int retval = 0;
 	struct davinci_spi *dspi;
 	struct davinci_spi_platform_data *pdata;
+	struct spi_master *master = spi->master;
+	struct device_node *np = spi->dev.of_node;
+	bool internal_cs = true;
 
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
 	if (!(spi->mode & SPI_NO_CS)) {
-		if ((pdata->chip_sel == NULL) ||
-		    (pdata->chip_sel[spi->chip_select] == SPI_INTERN_CS))
-			set_io_bits(dspi->base + SPIPC0, 1 << spi->chip_select);
-
+		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
+			unsigned long flags;
+
+			flags = GPIOF_DIR_OUT;
+			if (spi->mode & SPI_CS_HIGH)
+				flags |= GPIOF_INIT_LOW;
+			else
+				flags |= GPIOF_INIT_HIGH;
+			retval = gpio_request_one(spi->cs_gpio,
+						  flags, dev_name(&spi->dev));
+			if (retval) {
+				dev_err(&spi->dev,
+					"GPIO %d request failed (%d)\n",
+					spi->cs_gpio, retval);
+				return retval;
+			}
+			internal_cs = false;
+		} else if (pdata->chip_sel &&
+			   spi->chip_select < pdata->num_chipselect &&
+			   pdata->chip_sel[spi->chip_select] != SPI_INTERN_CS) {
+			internal_cs = false;
+		}
 	}
 
+	if (internal_cs)
+		set_io_bits(dspi->base + SPIPC0, 1 << spi->chip_select);
+
 	if (spi->mode & SPI_READY)
 		set_io_bits(dspi->base + SPIPC0, SPIPC0_SPIENA_MASK);
 
@@ -412,6 +448,15 @@ static int davinci_spi_setup(struct spi_device *spi)
 	return retval;
 }
 
+static void davinci_spi_cleanup(struct spi_device *spi)
+{
+	struct spi_master *master = spi->master;
+	struct device_node *np = spi->dev.of_node;
+
+	if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0))
+		gpio_free(spi->cs_gpio);
+}
+
 static int davinci_spi_check_error(struct davinci_spi *dspi, int int_status)
 {
 	struct device *sdev = dspi->bitbang.master->dev.parent;
@@ -810,6 +855,8 @@ static int spi_davinci_get_pdata(struct platform_device *pdev,
 
 	/*
 	 * default num_cs is 1 and all chipsel are internal to the chip
+	 * indicated by chip_sel being NULL or cs_gpios being NULL or
+	 * set to -ENOENT. num-cs includes internal as well as gpios.
 	 * indicated by chip_sel being NULL. GPIO based CS is not
 	 * supported yet in DT bindings.
 	 */
@@ -921,6 +968,7 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = pdata->num_chipselect;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
 	master->setup = davinci_spi_setup;
+	master->cleanup = davinci_spi_cleanup;
 
 	dspi->bitbang.chipselect = davinci_spi_chipselect;
 	dspi->bitbang.setup_transfer = davinci_spi_setup_transfer;
-- 
1.7.9.5

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

Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Mark Brown]

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

On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> 
> Currently driver supports only configuration of GPIO CS through
> platform data. This patch enhances the driver to configure GPIO
> CS through DT. Also update the DT binding documentation to
> reflect the availability of cs-gpios.

Please submit an incremental patch with whatever the changes were from
the version of this that I applied already.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Grygorii Strashko]

On 08/01/2014 08:26 PM, Mark Brown wrote:
> On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
>> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
>>
>> Currently driver supports only configuration of GPIO CS through
>> platform data. This patch enhances the driver to configure GPIO
>> CS through DT. Also update the DT binding documentation to
>> reflect the availability of cs-gpios.
>
> Please submit an incremental patch with whatever the changes were from
> the version of this that I applied already.
>

This series is based on top of:
"[PATCH 1/2] spi: davinci: fix to support more than 2 chip selects"
http://www.spinics.net/lists/linux-spi/msg01447.html

Sorry, I've forgot to mention this in cover letter.
Is that what you mean?

regards,
-grygorii

Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Mark Brown]

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

On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> 
> Currently driver supports only configuration of GPIO CS through
> platform data. This patch enhances the driver to configure GPIO
> CS through DT. Also update the DT binding documentation to
> reflect the availability of cs-gpios.

Ah, sorry - this is the unapplied patch from before.  It would have been
much better to submit this as the second patch after your rework patch
since this...

> -	if (pdata->chip_sel && chip_sel < pdata->num_chipselect &&
> -				pdata->chip_sel[chip_sel] != SPI_INTERN_CS)
> +	if (np && master->cs_gpios != NULL && spi->cs_gpio >= 0) {
> +		/* SPI core parse and update master->cs_gpio */
>  		gpio_chipsel = true;
> +		gpio = spi->cs_gpio;
> +	} else if (pdata->chip_sel &&
> +		   chip_sel < pdata->num_chipselect &&
> +		   pdata->chip_sel[chip_sel] != SPI_INTERN_CS) {
> +		/* platform data defines chip_sel */
> +		gpio_chipsel = true;
> +		gpio = pdata->chip_sel[chip_sel];
> +	}

...still looks excessively confusing - it is way more logic than I'd
expect to see on every chip select.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Grygorii Strashko]

On 08/01/2014 08:35 PM, Mark Brown wrote:
> On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
>> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
>>
>> Currently driver supports only configuration of GPIO CS through
>> platform data. This patch enhances the driver to configure GPIO
>> CS through DT. Also update the DT binding documentation to
>> reflect the availability of cs-gpios.
> 
> Ah, sorry - this is the unapplied patch from before.  It would have been
> much better to submit this as the second patch after your rework patch
> since this...
> 

I can do this, but It will wait for a week :(

>> -	if (pdata->chip_sel && chip_sel < pdata->num_chipselect &&
>> -				pdata->chip_sel[chip_sel] != SPI_INTERN_CS)
>> +	if (np && master->cs_gpios != NULL && spi->cs_gpio >= 0) {
>> +		/* SPI core parse and update master->cs_gpio */
>>   		gpio_chipsel = true;
>> +		gpio = spi->cs_gpio;
>> +	} else if (pdata->chip_sel &&
>> +		   chip_sel < pdata->num_chipselect &&
>> +		   pdata->chip_sel[chip_sel] != SPI_INTERN_CS) {
>> +		/* platform data defines chip_sel */
>> +		gpio_chipsel = true;
>> +		gpio = pdata->chip_sel[chip_sel];
>> +	}
> 
> ...still looks excessively confusing - it is way more logic than I'd
> expect to see on every chip select.
> 

Yep. Finally, patch "[PATCH v2 2/2] spi: davinci: use spi_device.cs_gpio to store gpio cs per spi device"
simplifies logic above.

Regards,
-grygorii

Re: [PATCH v2 1/2] spi: davinci: add support to configure gpio cs through dt

[Mark Brown]

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

On Fri, Aug 01, 2014 at 07:40:32PM +0300, Grygorii Strashko wrote:
> From: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> 
> Currently driver supports only configuration of GPIO CS through
> platform data. This patch enhances the driver to configure GPIO
> CS through DT. Also update the DT binding documentation to
> reflect the availability of cs-gpios.

Applied both, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 2/2] spi: davinci: use spi_device.cs_gpio to store gpio cs per spi device

[Grygorii Strashko]

Rework Davinci SPI driver to store GPIO CS number in cs_gpio field
of SPI device structure (spi_device) for both DT and non-DT cases.
This will make Davinci SPI driver code simpler and allows to reuse
more SPI core functionality.

Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
---
 drivers/spi/spi-davinci.c |   54 ++++++++++++++-------------------------------
 1 file changed, 17 insertions(+), 37 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index ac4414e0..276a388 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -208,9 +208,7 @@ static inline void clear_io_bits(void __iomem *addr, u32 bits)
 static void davinci_spi_chipselect(struct spi_device *spi, int value)
 {
 	struct davinci_spi *dspi;
-	struct device_node *np = spi->dev.of_node;
 	struct davinci_spi_platform_data *pdata;
-	struct spi_master *master = spi->master;
 	u8 chip_sel = spi->chip_select;
 	u16 spidat1 = CS_DEFAULT;
 	bool gpio_chipsel = false;
@@ -219,16 +217,10 @@ static void davinci_spi_chipselect(struct spi_device *spi, int value)
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
-	if (np && master->cs_gpios != NULL && spi->cs_gpio >= 0) {
+	if (spi->cs_gpio >= 0) {
 		/* SPI core parse and update master->cs_gpio */
 		gpio_chipsel = true;
 		gpio = spi->cs_gpio;
-	} else if (pdata->chip_sel &&
-		   chip_sel < pdata->num_chipselect &&
-		   pdata->chip_sel[chip_sel] != SPI_INTERN_CS) {
-		/* platform data defines chip_sel */
-		gpio_chipsel = true;
-		gpio = pdata->chip_sel[chip_sel];
 	}
 
 	/*
@@ -237,9 +229,9 @@ static void davinci_spi_chipselect(struct spi_device *spi, int value)
 	 */
 	if (gpio_chipsel) {
 		if (value == BITBANG_CS_ACTIVE)
-			gpio_set_value(gpio, 0);
+			gpio_set_value(gpio, spi->mode & SPI_CS_HIGH);
 		else
-			gpio_set_value(gpio, 1);
+			gpio_set_value(gpio, !(spi->mode & SPI_CS_HIGH));
 	} else {
 		if (value == BITBANG_CS_ACTIVE) {
 			spidat1 |= SPIDAT1_CSHOLD_MASK;
@@ -405,35 +397,34 @@ static int davinci_spi_setup(struct spi_device *spi)
 	struct spi_master *master = spi->master;
 	struct device_node *np = spi->dev.of_node;
 	bool internal_cs = true;
+	unsigned long flags = GPIOF_DIR_OUT;
 
 	dspi = spi_master_get_devdata(spi->master);
 	pdata = &dspi->pdata;
 
+	flags |= (spi->mode & SPI_CS_HIGH) ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH;
+
 	if (!(spi->mode & SPI_NO_CS)) {
 		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
-			unsigned long flags;
-
-			flags = GPIOF_DIR_OUT;
-			if (spi->mode & SPI_CS_HIGH)
-				flags |= GPIOF_INIT_LOW;
-			else
-				flags |= GPIOF_INIT_HIGH;
 			retval = gpio_request_one(spi->cs_gpio,
 						  flags, dev_name(&spi->dev));
-			if (retval) {
-				dev_err(&spi->dev,
-					"GPIO %d request failed (%d)\n",
-					spi->cs_gpio, retval);
-				return retval;
-			}
 			internal_cs = false;
 		} else if (pdata->chip_sel &&
 			   spi->chip_select < pdata->num_chipselect &&
 			   pdata->chip_sel[spi->chip_select] != SPI_INTERN_CS) {
+			spi->cs_gpio = pdata->chip_sel[spi->chip_select];
+			retval = gpio_request_one(spi->cs_gpio,
+						  flags, dev_name(&spi->dev));
 			internal_cs = false;
 		}
 	}
 
+	if (retval) {
+		dev_err(&spi->dev, "GPIO %d setup failed (%d)\n",
+			spi->cs_gpio, retval);
+		return retval;
+	}
+
 	if (internal_cs)
 		set_io_bits(dspi->base + SPIPC0, 1 << spi->chip_select);
 
@@ -450,10 +441,7 @@ static int davinci_spi_setup(struct spi_device *spi)
 
 static void davinci_spi_cleanup(struct spi_device *spi)
 {
-	struct spi_master *master = spi->master;
-	struct device_node *np = spi->dev.of_node;
-
-	if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0))
+	if (spi->cs_gpio >= 0)
 		gpio_free(spi->cs_gpio);
 }
 
@@ -895,7 +883,7 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	struct resource *r;
 	resource_size_t dma_rx_chan = SPI_NO_RESOURCE;
 	resource_size_t	dma_tx_chan = SPI_NO_RESOURCE;
-	int i = 0, ret = 0;
+	int ret = 0;
 	u32 spipc0;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct davinci_spi));
@@ -1016,14 +1004,6 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	spipc0 = SPIPC0_DIFUN_MASK | SPIPC0_DOFUN_MASK | SPIPC0_CLKFUN_MASK;
 	iowrite32(spipc0, dspi->base + SPIPC0);
 
-	/* initialize chip selects */
-	if (pdata->chip_sel) {
-		for (i = 0; i < pdata->num_chipselect; i++) {
-			if (pdata->chip_sel[i] != SPI_INTERN_CS)
-				gpio_direction_output(pdata->chip_sel[i], 1);
-		}
-	}
-
 	if (pdata->intr_line)
 		iowrite32(SPI_INTLVL_1, dspi->base + SPILVL);
 	else
-- 
1.7.9.5

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