diff mbox

[01/13] spi: imx: allow only WML aligned transfers to use DMA

Message ID 1455715739-25161-2-git-send-email-s.hauer@pengutronix.de (mailing list archive)
State Accepted
Commit 390f0ffe92aea878b763c7fd8afd1dff62e0d20b
Headers show

Commit Message

Sascha Hauer Feb. 17, 2016, 1:28 p.m. UTC 
From: Anton Bondarenko <anton.bondarenko.sama@gmail.com>

RX DMA tail data handling doesn't work correctly in many cases with current
implementation. It happens because SPI core was setup to generates both RX
and RX TAIL events. And RX TAIL event does not work correctly.
This can be easily verified by sending SPI transaction with size modulus
WML(32 in our case) not equal 0.

Also removing change introduced in f6ee9b582d2db652497b73c1f117591dfb6d3a90
since this change only fix usecases with transfer size from 33 to 128 bytes
and doesn't fix 129 bytes and bigger.

This is output from transaction with len 138 bytes in loopback mode at 10Mhz:
TX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2
TX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75
TX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95
TX0030: 5f 3c 35 b5 c4 8c dd 6c 11 32 3d e2 b4 b4 59 cf
TX0040: ce 23 3d 27 df a7 f9 96 fc 1e e0 66 2c 0e 7b 8c
TX0050: ca 30 42 8f bc 9f 7b ce d1 b8 b1 87 ec 8a d6 bb
TX0060: 2e 15 63 0e 3c dc a4 3a 7a 06 20 a7 93 1b 34 dd
TX0070: 4c f5 ec 88 96 68 d6 68 a0 09 6f 8e 93 47 c9 41
TX0080: db ac cf 97 89 f3 51 05 79 71

RX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2
RX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75
RX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95
RX0030: 5f 3c 35 00 00 b5 00 00 00 c4 00 00 8c 00 00 dd
RX0040: 6c 11 32 3d e2 b4 b4 59 cf ce 23 3d 27 df a7 f9
RX0050: 96 fc 1e e0 66 2c 0e 7b 8c ca 30 42 8f 1f 1f bc
RX0060: 9f 7b ce d1 b8 b1 87 ec 8a d6 bb 2e 15 63 0e ed
RX0070: ed 3c 58 58 58 dc 3d 3d a4 6a 6a 3a 52 52 7a 36
RX0080: 06 20 a7 93 1b 34 dd 4c f5 ec

Zeros at offset 33 and 34 caused by reading empty RX FIFO which not possible
if DMA RX read was triggered by RX event. This mean DMA was triggered
by RX TAIL event.

Signed-off-by: Anton Bondarenko <anton.bondarenko.sama@gmail.com>
Signed-off--by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/spi/spi-imx.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

Comments

Mark Brown Feb. 17, 2016, 2:16 p.m. UTC | #1 
On Wed, Feb 17, 2016 at 02:28:47PM +0100, Sascha Hauer wrote:

> Also removing change introduced in f6ee9b582d2db652497b73c1f117591dfb6d3a90
> since this change only fix usecases with transfer size from 33 to 128 bytes
> and doesn't fix 129 bytes and bigger.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.
Sascha Hauer Feb. 17, 2016, 3:01 p.m. UTC | #2 
On Wed, Feb 17, 2016 at 02:16:19PM +0000, Mark Brown wrote:
> On Wed, Feb 17, 2016 at 02:28:47PM +0100, Sascha Hauer wrote:
> 
> > Also removing change introduced in f6ee9b582d2db652497b73c1f117591dfb6d3a90
> > since this change only fix usecases with transfer size from 33 to 128 bytes
> > and doesn't fix 129 bytes and bigger.
> 
> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.

You seem to have a template for this, I remember having read the same
text before ;)

Will fix this and add a signed-off-by.

Sascha
Mark Brown Feb. 17, 2016, 4:05 p.m. UTC | #3 
On Wed, Feb 17, 2016 at 04:01:46PM +0100, Sascha Hauer wrote:

> You seem to have a template for this, I remember having read the same
> text before ;)

I do, but even before I had a template I'd always end up typing the same
thing by hand anyway.
diff mbox

Patch

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index d98c33c..08492d6 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -204,8 +204,8 @@  static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
 {
 	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
 
-	if (spi_imx->dma_is_inited &&
-	    transfer->len > spi_imx->wml * sizeof(u32))
+	if (spi_imx->dma_is_inited && transfer->len >= spi_imx->wml &&
+	    (transfer->len % spi_imx->wml) == 0)
 		return true;
 	return false;
 }
@@ -919,8 +919,6 @@  static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
 	struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL;
 	int ret;
 	unsigned long timeout;
-	u32 dma;
-	int left;
 	struct spi_master *master = spi_imx->bitbang.master;
 	struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg;
 
@@ -954,13 +952,6 @@  static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
 	/* Trigger the cspi module. */
 	spi_imx->dma_finished = 0;
 
-	dma = readl(spi_imx->base + MX51_ECSPI_DMA);
-	dma = dma & (~MX51_ECSPI_DMA_RXT_WML_MASK);
-	/* Change RX_DMA_LENGTH trigger dma fetch tail data */
-	left = transfer->len % spi_imx->wml;
-	if (left)
-		writel(dma | (left << MX51_ECSPI_DMA_RXT_WML_OFFSET),
-				spi_imx->base + MX51_ECSPI_DMA);
 	/*
 	 * Set these order to avoid potential RX overflow. The overflow may
 	 * happen if we enable SPI HW before starting RX DMA due to rescheduling
@@ -992,10 +983,6 @@  static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,
 			spi_imx->devtype_data->reset(spi_imx);
 			dmaengine_terminate_all(master->dma_rx);
 		}
-		dma &= ~MX51_ECSPI_DMA_RXT_WML_MASK;
-		writel(dma |
-		       spi_imx->wml << MX51_ECSPI_DMA_RXT_WML_OFFSET,
-		       spi_imx->base + MX51_ECSPI_DMA);
 	}
 
 	spi_imx->dma_finished = 1;