[02/10] spi: dw-mid: always use duplex transfers when DMA
diff mbox

Message ID 1411060140-2801-3-git-send-email-andriy.shevchenko@linux.intel.com
State New, archived
Headers show

Commit Message

Andy Shevchenko Sept. 18, 2014, 5:08 p.m. UTC
In case of DMA we never got things done until we enable both channels. In case
of receive only or send only mode the user has to provide a dummy buffer.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-dw-mid.c | 6 ++----
 drivers/spi/spi-dw.h     | 4 ++++
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Mark Brown Sept. 24, 2014, 8:44 a.m. UTC | #1
On Thu, Sep 18, 2014 at 08:08:52PM +0300, Andy Shevchenko wrote:
> In case of DMA we never got things done until we enable both channels. In case
> of receive only or send only mode the user has to provide a dummy buffer.

The driver must never impose such requirements on clients, they
shouldn't need to know about the quirks of the controller.  There's
support in the core providing dummy buffers for transmit and receive,
you should use that (possibly extending it so the dummies are only
provided for DMA).
Andy Shevchenko Sept. 24, 2014, 10:32 a.m. UTC | #2
On Wed, 2014-09-24 at 09:44 +0100, Mark Brown wrote:
> On Thu, Sep 18, 2014 at 08:08:52PM +0300, Andy Shevchenko wrote:
> > In case of DMA we never got things done until we enable both channels. In case
> > of receive only or send only mode the user has to provide a dummy buffer.
> 
> The driver must never impose such requirements on clients, they
> shouldn't need to know about the quirks of the controller.  There's
> support in the core providing dummy buffers for transmit and receive,
> you should use that (possibly extending it so the dummies are only
> provided for DMA).


Currently driver in case of receiving or sending only will never call
the xfer_done from DMA callback since we never will have counter equal
to 2. So, the driver already relies on duplex mode. Am I missing
something?
Mark Brown Sept. 24, 2014, 11:07 a.m. UTC | #3
On Wed, Sep 24, 2014 at 01:32:27PM +0300, Andy Shevchenko wrote:
> On Wed, 2014-09-24 at 09:44 +0100, Mark Brown wrote:
> > On Thu, Sep 18, 2014 at 08:08:52PM +0300, Andy Shevchenko wrote:

> > > In case of DMA we never got things done until we enable both channels. In case
> > > of receive only or send only mode the user has to provide a dummy buffer.

> > The driver must never impose such requirements on clients, they
> > shouldn't need to know about the quirks of the controller.  There's
> > support in the core providing dummy buffers for transmit and receive,
> > you should use that (possibly extending it so the dummies are only
> > provided for DMA).

> Currently driver in case of receiving or sending only will never call
> the xfer_done from DMA callback since we never will have counter equal
> to 2. So, the driver already relies on duplex mode. Am I missing
> something?

Please re-read what I wrote above, especially the first phrase.  You
can't require that we rewrite essentialy every SPI driver on the off
chance that it's used with this controller.
Andy Shevchenko Sept. 24, 2014, 12:02 p.m. UTC | #4
On Wed, 2014-09-24 at 12:07 +0100, Mark Brown wrote:
> On Wed, Sep 24, 2014 at 01:32:27PM +0300, Andy Shevchenko wrote:
> > On Wed, 2014-09-24 at 09:44 +0100, Mark Brown wrote:
> > > On Thu, Sep 18, 2014 at 08:08:52PM +0300, Andy Shevchenko wrote:
> 
> > > > In case of DMA we never got things done until we enable both channels. In case
> > > > of receive only or send only mode the user has to provide a dummy buffer.
> 
> > > The driver must never impose such requirements on clients, they
> > > shouldn't need to know about the quirks of the controller.  There's
> > > support in the core providing dummy buffers for transmit and receive,
> > > you should use that (possibly extending it so the dummies are only
> > > provided for DMA).
> 
> > Currently driver in case of receiving or sending only will never call
> > the xfer_done from DMA callback since we never will have counter equal
> > to 2. So, the driver already relies on duplex mode. Am I missing
> > something?
> 
> Please re-read what I wrote above, especially the first phrase.  You
> can't require that we rewrite essentialy every SPI driver on the off
> chance that it's used with this controller.

I think I know how to satisfy the user and fix the driver.
I will remake the approach.

Patch
diff mbox

diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
index f7f0ad2..97156e4 100644
--- a/drivers/spi/spi-dw-mid.c
+++ b/drivers/spi/spi-dw-mid.c
@@ -119,10 +119,8 @@  static int mid_spi_dma_transfer(struct dw_spi *dws, int cs_change)
 		spi_enable_chip(dws, 0);
 		dw_writew(dws, DW_SPI_DMARDLR, 0xf);
 		dw_writew(dws, DW_SPI_DMATDLR, 0x10);
-		if (dws->tx_dma)
-			dma_ctrl |= 0x2;
-		if (dws->rx_dma)
-			dma_ctrl |= 0x1;
+		dma_ctrl |= SPI_DMA_TDMAE;
+		dma_ctrl |= SPI_DMA_RDMAE;
 		dw_writew(dws, DW_SPI_DMACR, dma_ctrl);
 		spi_enable_chip(dws, 1);
 	}
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 089fc4b..83a103a 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -74,6 +74,10 @@ 
 #define SPI_INT_RXFI			(1 << 4)
 #define SPI_INT_MSTI			(1 << 5)
 
+/* Bit fields in DMACR */
+#define SPI_DMA_RDMAE			(1 << 0)
+#define SPI_DMA_TDMAE			(1 << 1)
+
 /* TX RX interrupt level threshold, max can be 256 */
 #define SPI_INT_THRESHOLD		32