| Message ID | 1411060140-2801-3-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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).
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?
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.
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.
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
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(-)