| Message ID | CAC5umyg2fJxYKOB4J96+3igkNx+eJs5MET8S8NnafZ3hcPPEag@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, May 12, 2016 at 12:39:35AM +0900, Akinobu Mita wrote: > I realized that this undo patch has not been merged to Linus' tree yet. > I have a fix for this issue (attached patch). But the change is not > small and also v4.6 release is soon. So I think this undo patch > should go into 4.6 release. I actually just sent that a few minutes ago anyway. > The actual DMA transfer length by dmaengine can be smaller than SPI > transfer length in the specific condition. In that case, the last > word needs to be filled after DMA transfer completion. > This fixes it by detecting that case and remap the scatterlist with > correct DMA transfer length. This feels like it should be in the framework, I imagine other devices will have similar limiations. Especially if the limitation comes from the DMA engine I'd hope we can arrange to query it somehow... can you provide a bit more detail on what the restriction is please?
2016-05-12 1:20 GMT+09:00 Mark Brown <broonie@kernel.org>: > On Thu, May 12, 2016 at 12:39:35AM +0900, Akinobu Mita wrote: > >> I realized that this undo patch has not been merged to Linus' tree yet. >> I have a fix for this issue (attached patch). But the change is not >> small and also v4.6 release is soon. So I think this undo patch >> should go into 4.6 release. > > I actually just sent that a few minutes ago anyway. > >> The actual DMA transfer length by dmaengine can be smaller than SPI >> transfer length in the specific condition. In that case, the last >> word needs to be filled after DMA transfer completion. > >> This fixes it by detecting that case and remap the scatterlist with >> correct DMA transfer length. > > This feels like it should be in the framework, I imagine other devices > will have similar limiations. Especially if the limitation comes from > the DMA engine I'd hope we can arrange to query it somehow... can you > provide a bit more detail on what the restriction is please? According to the source code (I couldn't find out the reason from TRM yet), the last word (or two words if TURBO mode is used) of RX data needs to be transferred by PIO transfer instead of DMA transfer. But if FIFO buffer support[*] is used, the whole RX data can be transferred by DMA transfer. The use of FIFO buffer is determined by omap2_mcspi_set_fifo() with the total transfer length, bits per words, TX/RX only or both, and etc. [*] commit d33f473dcd8e ("spi: omap2-mcspi: Add FIFO buffer support") If we have an optional callback to get the actual DMA transfer length instead of using spi_transfer->len for each spi_transfer in the framework, we can use it to determine the correct map size used by spi_map_buf() in spi.c. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 12, 2016 at 10:49:15PM +0900, Akinobu Mita wrote: > If we have an optional callback to get the actual DMA transfer length > instead of using spi_transfer->len for each spi_transfer in the > framework, we can use it to determine the correct map size used by > spi_map_buf() in spi.c. That sounds plausible, we can also do some querying of the dmaengine at the same place we call that when we get around to that. Can you try to come up with an implementation?
2016-05-13 0:44 GMT+09:00 Mark Brown <broonie@kernel.org>: > On Thu, May 12, 2016 at 10:49:15PM +0900, Akinobu Mita wrote: > >> If we have an optional callback to get the actual DMA transfer length >> instead of using spi_transfer->len for each spi_transfer in the >> framework, we can use it to determine the correct map size used by >> spi_map_buf() in spi.c. > > That sounds plausible, we can also do some querying of the dmaengine at > the same place we call that when we get around to that. Can you try to > come up with an implementation? OK, I'll try and see how it looks like. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From 5abc589ab08e239e884fb2e8937d74f81fb0b888 Mon Sep 17 00:00:00 2001
From: Akinobu Mita <akinobu.mita@gmail.com>
Date: Mon, 21 Mar 2016 01:16:06 +0900
Subject: [PATCH] spi: omap2-mcspi: fix dma transfer
This fixes the problem introduced by the commit 3525e0aac91c ("spi:
omap2-mcspi: fix dma transfer for vmalloced buffer")
The actual DMA transfer length by dmaengine can be smaller than SPI
transfer length in the specific condition. In that case, the last
word needs to be filled after DMA transfer completion.
This fixes it by detecting that case and remap the scatterlist with
correct DMA transfer length.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-omap2-mcspi.c | 47 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 43a02e3..a7fafd0 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -439,7 +439,41 @@ static void omap2_mcspi_tx_dma(struct spi_device *spi,
}
dma_async_issue_pending(mcspi_dma->dma_tx);
omap2_mcspi_set_dma_req(spi, 0, 1);
+}
+
+static int
+omap2_mcspi_rx_dma_remap(struct spi_device *spi, struct spi_transfer *xfer,
+ unsigned int dma_count)
+{
+ int i;
+ struct scatterlist *sg;
+ struct omap2_mcspi *mcspi;
+ unsigned int nents = 0;
+ unsigned int count = 0;
+ int ret;
+
+ mcspi = spi_master_get_devdata(spi->master);
+
+ dma_unmap_sg(mcspi->dev, xfer->rx_sg.sgl, xfer->rx_sg.nents,
+ DMA_FROM_DEVICE);
+
+ for_each_sg(xfer->rx_sg.sgl, sg, xfer->rx_sg.nents, i) {
+ nents++;
+ if (count + sg->length < dma_count) {
+ count += sg->length;
+ continue;
+ }
+ sg->length = dma_count - count;
+ break;
+ }
+ ret = dma_map_sg(mcspi->dev, xfer->rx_sg.sgl, nents, DMA_FROM_DEVICE);
+ if (!ret)
+ return -ENOMEM;
+
+ xfer->rx_sg.nents = ret;
+
+ return 0;
}
static unsigned
@@ -480,6 +514,16 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
if ((l & OMAP2_MCSPI_CHCONF_TURBO) && mcspi->fifo_depth == 0)
dma_count -= es;
+ if (xfer->len != dma_count) {
+ int ret;
+
+ ret = omap2_mcspi_rx_dma_remap(spi, xfer, dma_count);
+ if (ret) {
+ dev_err(&spi->dev, "failed to map rx buf\n");
+ return 0;
+ }
+ }
+
tx = dmaengine_prep_slave_sg(mcspi_dma->dma_rx, xfer->rx_sg.sgl,
xfer->rx_sg.nents, DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -496,6 +540,9 @@ omap2_mcspi_rx_dma(struct spi_device *spi, struct spi_transfer *xfer,
omap2_mcspi_set_dma_req(spi, 1, 1);
wait_for_completion(&mcspi_dma->dma_rx_completion);
+ dma_unmap_sg(mcspi->dev, xfer->rx_sg.sgl, xfer->rx_sg.nents,
+ DMA_FROM_DEVICE);
+ sg_free_table(&xfer->rx_sg);
if (mcspi->fifo_depth > 0)
return count;
--
2.7.4