2016-10-05 21:33:19

by Thor Thayer

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

From: Thor Thayer <[email protected]>

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 <[email protected]>
---
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


2016-10-06 10:16:58

by Mark Brown

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

On Wed, Oct 05, 2016 at 04:38:58PM -0500, [email protected] 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?


Attachments:
(No filename) (500.00 B)
signature.asc (455.00 B)
Download all attachments

2016-10-06 14:09:18

by Thor Thayer

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



On 10/06/2016 04:37 AM, Mark Brown wrote:
> On Wed, Oct 05, 2016 at 04:38:58PM -0500, [email protected] 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.