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

[PATCH 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.

Murali Karicheri (2):
  spi: davinci: fix to support more than 2 chip selects
  spi: davinci: add support to configure GPIO CS through DT

 .../devicetree/bindings/spi/spi-davinci.txt        |    9 ++-
 drivers/spi/spi-davinci.c                          |   71 +++++++++++++++++---
 2 files changed, 68 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] spi: davinci: fix to support more than 2 chip selects

[Grygorii Strashko]

From: Murali Karicheri <m-karicheri2@ti.com>

Currently, the driver defines SPI_MAX_CHIPSELECT as 2 and
use per device array bytes_per_word based on this. This breaks
if num_chipselect per device is greater than 2. This patch
fix this and allocate memory for this array based on
num_chipselect.

It's preparation patch to enable GPIO CS feature for
Davinci SPI.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/spi/spi-davinci.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 50f7509..2477af4 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -38,8 +38,6 @@
 
 #define SPI_NO_RESOURCE		((resource_size_t)-1)
 
-#define SPI_MAX_CHIPSELECT	2
-
 #define CS_DEFAULT	0xFF
 
 #define SPIFMT_PHASE_MASK	BIT(16)
@@ -142,7 +140,7 @@ struct davinci_spi {
 	void			(*get_rx)(u32 rx_data, struct davinci_spi *);
 	u32			(*get_tx)(struct davinci_spi *);
 
-	u8			bytes_per_word[SPI_MAX_CHIPSELECT];
+	u8			*bytes_per_word;
 };
 
 static struct davinci_spi_config davinci_spi_default_cfg;
@@ -876,6 +874,14 @@ static int davinci_spi_probe(struct platform_device *pdev)
 	/* pdata in dspi is now updated and point pdata to that */
 	pdata = &dspi->pdata;
 
+	dspi->bytes_per_word = devm_kzalloc(&pdev->dev,
+					    sizeof(*dspi->bytes_per_word) *
+					    pdata->num_chipselect, GFP_KERNEL);
+	if (dspi->bytes_per_word == NULL) {
+		ret = -ENOMEM;
+		goto free_master;
+	}
+
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (r == NULL) {
 		ret = -ENOENT;
-- 
1.7.9.5

[PATCH 1/2] spi: davinci: fix to support more than 2 chip selects

[Mark Brown]

On Thu, Jul 31, 2014 at 08:33:14PM +0300, Grygorii Strashko wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
> 
> Currently, the driver defines SPI_MAX_CHIPSELECT as 2 and
> use per device array bytes_per_word based on this. This breaks
> if num_chipselect per device is greater than 2. This patch
> fix this and allocate memory for this array based on
> num_chipselect.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140731/cfb4f923/attachment.sig>

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

[Grygorii Strashko]

From: Murali Karicheri <m-karicheri2@ti.com>

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@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/spi/spi-davinci.txt        |    9 ++-
 drivers/spi/spi-davinci.c                          |   59 +++++++++++++++++---
 2 files changed, 59 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..6aa04d1 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,36 @@ 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)) {
+			retval =
+				gpio_request(spi->cs_gpio, dev_name(&spi->dev));
+			if (retval) {
+				dev_err(&spi->dev,
+					"GPIO %d request failed\n",
+					spi->cs_gpio);
+				return -ENODEV;
+			}
+			gpio_direction_output(spi->cs_gpio,
+					      !(spi->mode & SPI_CS_HIGH));
+			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 +443,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 +850,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 +963,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

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

[Mark Brown]

On Thu, Jul 31, 2014 at 08:33:15PM +0300, Grygorii Strashko wrote:

> +	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];
> +	}

This would all be a lot simpler and more direct if you were to arrange
to use cs_gpio in the struct spi_device, and move you closer to
converting to use more of the core functionality.

> +		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
> +			retval =
> +				gpio_request(spi->cs_gpio, dev_name(&spi->dev));
> +			if (retval) {
> +				dev_err(&spi->dev,
> +					"GPIO %d request failed\n",
> +					spi->cs_gpio);
> +				return -ENODEV;
> +			}
> +			gpio_direction_output(spi->cs_gpio,
> +					      !(spi->mode & SPI_CS_HIGH));
> +			internal_cs = false;

It's much better to use gpio_request_one() rather than using
gpio_request(), it's one function apart from anything else.  The above
is also discarding the return code of gpio_request() meaning it's broken
for deferred probe.  The error code should also appear in the error
message.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140731/9580c14e/attachment.sig>

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

[Grygorii Strashko]

Hi Mark,
On 07/31/2014 10:35 PM, Mark Brown wrote:
> On Thu, Jul 31, 2014 at 08:33:15PM +0300, Grygorii Strashko wrote:
> 
>> +	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];
>> +	}
> 
> This would all be a lot simpler and more direct if you were to arrange
> to use cs_gpio in the struct spi_device, and move you closer to
> converting to use more of the core functionality.

Unfortunately, I'm not sure this is good idea, because this part of
code is used by Davinci arch which is non-DT platform.
As result, this code was kept mostly unchanged.

I can try to rework it, but I'd like to do it as standalone patch.
Is it ok for you?

> 
>> +		if (np && (master->cs_gpios != NULL) && (spi->cs_gpio >= 0)) {
>> +			retval =
>> +				gpio_request(spi->cs_gpio, dev_name(&spi->dev));
>> +			if (retval) {
>> +				dev_err(&spi->dev,
>> +					"GPIO %d request failed\n",
>> +					spi->cs_gpio);
>> +				return -ENODEV;
>> +			}
>> +			gpio_direction_output(spi->cs_gpio,
>> +					      !(spi->mode & SPI_CS_HIGH));
>> +			internal_cs = false;
> 
> It's much better to use gpio_request_one() rather than using
> gpio_request(), it's one function apart from anything else.  The above
> is also discarding the return code of gpio_request() meaning it's broken
> for deferred probe.  The error code should also appear in the error
> message.
> 
Thanks, will update.

Regards,
-grygorii

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

[Grygorii Strashko]

Hi Mark,

On 08/01/2014 06:28 PM, Grygorii Strashko wrote:
> Hi Mark,
> On 07/31/2014 10:35 PM, Mark Brown wrote:
>> On Thu, Jul 31, 2014 at 08:33:15PM +0300, Grygorii Strashko wrote:
>>
>>> +	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];
>>> +	}
>>
>> This would all be a lot simpler and more direct if you were to arrange
>> to use cs_gpio in the struct spi_device, and move you closer to
>> converting to use more of the core functionality.
> 
> Unfortunately, I'm not sure this is good idea, because this part of
> code is used by Davinci arch which is non-DT platform.
> As result, this code was kept mostly unchanged.
> 
> I can try to rework it, but I'd like to do it as standalone patch.
> Is it ok for you?

I've just sent updated version with additional patch where
I reworked driver for using cs_gpio for  both DT and non-DT cases.
http://www.spinics.net/lists/arm-kernel/msg352382.html

Regards,
-grygorii