Re: [PATCH v5 03/16] spi: dw: Locally wait for the DMA transactions completion
From: Andy Shevchenko <hidden>
Date: 2020-05-29 07:55:50
Also in:
linux-mips, linux-spi, lkml
On Fri, May 29, 2020 at 7:02 AM Serge Semin [off-list ref] wrote:
Even if DMA transactions are finished it doesn't mean that the SPI transfers are also completed. It's specifically concerns the Tx-only SPI transfers, since there might be data left in the SPI Tx FIFO after the DMA engine notifies that the Tx DMA procedure is done. In order to completely fix the problem first the driver has to wait for the DMA transaction completion, then for the corresponding SPI operations to be finished. In this commit we implement the former part of the solution. Note we can't just move the SPI operations wait procedure to the DMA completion callbacks, since these callbacks might be executed in the tasklet context (and they will be in case of the DW DMA). In case of slow SPI bus it can cause significant system performance drop.
I read commit message, I read the code. What's going on here since you repeated xfer_completion (and its wait routine) from SPI core and I'm wondering what happened to it? Why we are not calling spi_finalize_current_transfer()? ...
dws->master->cur_msg->status = -EIO;
- spi_finalize_current_transfer(dws->master);
+ complete(&dws->dma_completion);
return IRQ_HANDLED;...
quoted hunk ↗ jump to hunk
+static int dw_spi_dma_wait(struct dw_spi *dws, struct spi_transfer *xfer) +{ + unsigned long long ms; + + ms = xfer->len * MSEC_PER_SEC * BITS_PER_BYTE; + do_div(ms, xfer->effective_speed_hz); + ms += ms + 200; + + if (ms > UINT_MAX) + ms = UINT_MAX; + + ms = wait_for_completion_timeout(&dws->dma_completion, + msecs_to_jiffies(ms)); + + if (ms == 0) { + dev_err(&dws->master->cur_msg->spi->dev, + "DMA transaction timed out\n"); + return -ETIMEDOUT; + } + + return 0; +} + /* * dws->dma_chan_busy is set before the dma transfer starts, callback for tx * channel will clear a corresponding bit.@@ -155,7 +184,7 @@ static void dw_spi_dma_tx_done(void *arg) return; dw_writel(dws, DW_SPI_DMACR, 0); - spi_finalize_current_transfer(dws->master); + complete(&dws->dma_completion); } static struct dma_async_tx_descriptor *dw_spi_dma_prepare_tx(struct dw_spi *dws,@@ -204,7 +233,7 @@ static void dw_spi_dma_rx_done(void *arg) return; dw_writel(dws, DW_SPI_DMACR, 0); - spi_finalize_current_transfer(dws->master); + complete(&dws->dma_completion); }
-- With Best Regards, Andy Shevchenko