[PATCH] spi: dw: Enable Slave Select with GPIO Chip Select.

[PATCH] spi: dw: Enable Slave Select with GPIO Chip Select.

[tthayer]

From: Thor Thayer <tthayer@opensource.altera.com>

Currently in the GPIO CS case, the transfer is stalled. The DesignWare
IP datasheet points out that the slave must be selected (SER) before the
transfer can start [1]. The SPI framework selects the chip using either
1) the internal CS mechanism or 2) the GPIO CS, but not both.

This patch adds the Slave Select locally so that the transfer will
start and complete. The GPIO CS is taken care of earlier in the SPI
framework (spi_set_cs).

Tested On:
    Altera CycloneV development kit
    Compile tested for build errors on x86_64 (allyesconfigs)

[1] DesignWare dw_apb_ssi Databook, Version 3.20a (page 39)

Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
---
 drivers/spi/spi-dw.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 27960e4..86e321d 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -296,6 +296,10 @@ static int dw_spi_transfer_one(struct spi_master *master,
 
 	spi_enable_chip(dws, 0);
 
+	/* Slave select required before transfer can begin. Force in gpio CS */
+	if (gpio_is_valid(spi->cs_gpio))
+		dw_spi_set_cs(spi, 0);
+
 	/* Handle per transfer options for bpw and speed */
 	if (transfer->speed_hz != dws->current_freq) {
 		if (transfer->speed_hz != chip->speed_hz) {
@@ -390,6 +394,10 @@ static void dw_spi_handle_err(struct spi_master *master,
 	if (dws->dma_mapped)
 		dws->dma_ops->dma_stop(dws);
 
+	/* Cleanup Slave Select in gpio CS case */
+	if (gpio_is_valid(msg->spi->cs_gpio))
+		dw_spi_set_cs(msg->spi, 1);
+
 	spi_reset_chip(dws);
 }
 
@@ -440,6 +448,10 @@ static void dw_spi_cleanup(struct spi_device *spi)
 {
 	struct chip_data *chip = spi_get_ctldata(spi);
 
+	/* Cleanup Slave Select in gpio CS case */
+	if (gpio_is_valid(spi->cs_gpio))
+		dw_spi_set_cs(spi, 1);
+
 	kfree(chip);
 	spi_set_ctldata(spi, NULL);
 }
-- 
1.7.9.5

Re: [PATCH] spi: dw: Enable Slave Select with GPIO Chip Select.

[Mark Brown]

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

On Wed, Oct 05, 2016 at 04:38:58PM -0500, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:

> This patch adds the Slave Select locally so that the transfer will
> start and complete. The GPIO CS is taken care of earlier in the SPI
> framework (spi_set_cs).

This seems like something that other devices might need - it's not the
first time I heard of a device with the data transfer start and chip
select being tied together.  Why not make this a generic feature that
the core implements and drivers can enable?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH] spi: dw: Enable Slave Select with GPIO Chip Select.

[Thor Thayer]

On 10/06/2016 04:37 AM, Mark Brown wrote:
> On Wed, Oct 05, 2016 at 04:38:58PM -0500, tthayer@opensource.altera.com wrote:
>
>> This patch adds the Slave Select locally so that the transfer will
>> start and complete. The GPIO CS is taken care of earlier in the SPI
>> framework (spi_set_cs).
>
> This seems like something that other devices might need - it's not the
> first time I heard of a device with the data transfer start and chip
> select being tied together.  Why not make this a generic feature that
> the core implements and drivers can enable?
>
OK. Thanks for the suggestion and review. I'll add a new boolean and 
device tree binding to the SPI core. Thanks.