[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Torsten Fleischer]

From: Torsten Fleischer <torfl6749-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Additionally to the current DMA transfer the PDC allows to set up a next DMA
transfer. This is useful for larger SPI transfers.

The driver currently waits for ENDRX as end of the transfer. But ENDRX is set
when the current DMA transfer is done (RCR = 0), i.e. it doesn't include the
next DMA transfer.
Thus a subsequent SPI transfer could be started although there is currently a
transfer in progress. This can cause invalid accesses to the SPI slave devices
and to SPI transfer errors.

This issue has been observed on a hardware with a M25P128 SPI NOR flash.

So instead of ENDRX we should wait for RXBUFF. This flag is set if there is
no more DMA transfer in progress (RCR = RNCR = 0).

Signed-off-by: Torsten Fleischer <torfl6749-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-atmel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 9af7841..06de340 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -764,17 +764,17 @@ static void atmel_spi_pdc_next_xfer(struct spi_master *master,
 			(unsigned long long)xfer->rx_dma);
 	}
 
-	/* REVISIT: We're waiting for ENDRX before we start the next
+	/* REVISIT: We're waiting for RXBUFF before we start the next
 	 * transfer because we need to handle some difficult timing
-	 * issues otherwise. If we wait for ENDTX in one transfer and
-	 * then starts waiting for ENDRX in the next, it's difficult
-	 * to tell the difference between the ENDRX interrupt we're
-	 * actually waiting for and the ENDRX interrupt of the
+	 * issues otherwise. If we wait for TXBUFE in one transfer and
+	 * then starts waiting for RXBUFF in the next, it's difficult
+	 * to tell the difference between the RXBUFF interrupt we're
+	 * actually waiting for and the RXBUFF interrupt of the
 	 * previous transfer.
 	 *
 	 * It should be doable, though. Just not now...
 	 */
-	spi_writel(as, IER, SPI_BIT(ENDRX) | SPI_BIT(OVRES));
+	spi_writel(as, IER, SPI_BIT(RXBUFF) | SPI_BIT(OVRES));
 	spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

On Tue, Feb 24, 2015 at 04:32:57PM +0100, Torsten Fleischer wrote:
> From: Torsten Fleischer <torfl6749-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Additionally to the current DMA transfer the PDC allows to set up a next DMA
> transfer. This is useful for larger SPI transfers.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Mark Brown]

On Tue, Feb 24, 2015 at 04:32:57PM +0100, Torsten Fleischer wrote:
> From: Torsten Fleischer <torfl6749@gmail.com>
> 
> Additionally to the current DMA transfer the PDC allows to set up a next DMA
> transfer. This is useful for larger SPI transfers.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150226/6efb6cc6/attachment.sig>

Re: [PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Tue, Feb 24, 2015 at 01:26:20PM +0100, Torsten Fleischer wrote:

> spi_master *master,
>   (unsigned long long)xfer->rx_dma);
>   }
> 
> - /* REVISIT: We're waiting for ENDRX before we start the next
> + /* REVISIT: We're waiting for RXBUFF before we start the next
>   * transfer because we need to handle some difficult timing
> - * issues otherwise. If we wait for ENDTX in one transfer and
> - * then starts waiting for ENDRX in the next, it's difficult
> - * to tell the difference between the ENDRX interrupt we're
> - * actually waiting for and the ENDRX interrupt of the
> + * issues otherwise. If we wait for TXBUFE in one transfer and
> + * then starts waiting for RXBUFF in the next, it's difficult
> + * to tell the difference between the RXBUFF interrupt we're
> + * actually waiting for and the RXBUFF interrupt of the
>   * previous transfer.
>   *
>   * It should be doable, though. Just not now...
>   */
> - spi_writel(as, IER, SPI_BIT(ENDRX) | SPI_BIT(OVRES));
> + spi_writel(as, IER, SPI_BIT(RXBUFF) | SPI_BIT(OVRES));
>   spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
>  }

It looks like your mail program has corrupted the patch and turned all
tabs into spaces...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Mark Brown]

On Tue, Feb 24, 2015 at 01:26:20PM +0100, Torsten Fleischer wrote:

> spi_master *master,
>   (unsigned long long)xfer->rx_dma);
>   }
> 
> - /* REVISIT: We're waiting for ENDRX before we start the next
> + /* REVISIT: We're waiting for RXBUFF before we start the next
>   * transfer because we need to handle some difficult timing
> - * issues otherwise. If we wait for ENDTX in one transfer and
> - * then starts waiting for ENDRX in the next, it's difficult
> - * to tell the difference between the ENDRX interrupt we're
> - * actually waiting for and the ENDRX interrupt of the
> + * issues otherwise. If we wait for TXBUFE in one transfer and
> + * then starts waiting for RXBUFF in the next, it's difficult
> + * to tell the difference between the RXBUFF interrupt we're
> + * actually waiting for and the RXBUFF interrupt of the
>   * previous transfer.
>   *
>   * It should be doable, though. Just not now...
>   */
> - spi_writel(as, IER, SPI_BIT(ENDRX) | SPI_BIT(OVRES));
> + spi_writel(as, IER, SPI_BIT(RXBUFF) | SPI_BIT(OVRES));
>   spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
>  }

It looks like your mail program has corrupted the patch and turned all
tabs into spaces...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150224/a6f2fe26/attachment.sig>

[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Torsten Fleischer]

Additionally to the current DMA transfer the PDC allows to set up a next DMA
transfer. This is useful for larger SPI transfers.

The driver currently waits for ENDRX as end of the transfer. But ENDRX is set
when the current DMA transfer is done (RCR = 0), i.e. it doesn't include the
next DMA transfer.
Thus a subsequent SPI transfer could be started although there is currently a
transfer in progress. This can cause invalid accesses to the SPI slave devices
and to SPI transfer errors.

This issue has been observed on a hardware with a M25P128 SPI NOR flash.

So instead of ENDRX we should wait for RXBUFF. This flag is set if there is
no more DMA transfer in progress (RCR = RNCR = 0).

Signed-off-by: Torsten Fleischer <torfl6749-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/spi/spi-atmel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 9af7841..06de340 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -764,17 +764,17 @@ static void atmel_spi_pdc_next_xfer(struct
spi_master *master,
  (unsigned long long)xfer->rx_dma);
  }

- /* REVISIT: We're waiting for ENDRX before we start the next
+ /* REVISIT: We're waiting for RXBUFF before we start the next
  * transfer because we need to handle some difficult timing
- * issues otherwise. If we wait for ENDTX in one transfer and
- * then starts waiting for ENDRX in the next, it's difficult
- * to tell the difference between the ENDRX interrupt we're
- * actually waiting for and the ENDRX interrupt of the
+ * issues otherwise. If we wait for TXBUFE in one transfer and
+ * then starts waiting for RXBUFF in the next, it's difficult
+ * to tell the difference between the RXBUFF interrupt we're
+ * actually waiting for and the RXBUFF interrupt of the
  * previous transfer.
  *
  * It should be doable, though. Just not now...
  */
- spi_writel(as, IER, SPI_BIT(ENDRX) | SPI_BIT(OVRES));
+ spi_writel(as, IER, SPI_BIT(RXBUFF) | SPI_BIT(OVRES));
  spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
 }

-- 
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] spi: atmel: Fix interrupt setup for PDC transfers

[Torsten Fleischer]

Additionally to the current DMA transfer the PDC allows to set up a next DMA
transfer. This is useful for larger SPI transfers.

The driver currently waits for ENDRX as end of the transfer. But ENDRX is set
when the current DMA transfer is done (RCR = 0), i.e. it doesn't include the
next DMA transfer.
Thus a subsequent SPI transfer could be started although there is currently a
transfer in progress. This can cause invalid accesses to the SPI slave devices
and to SPI transfer errors.

This issue has been observed on a hardware with a M25P128 SPI NOR flash.

So instead of ENDRX we should wait for RXBUFF. This flag is set if there is
no more DMA transfer in progress (RCR = RNCR = 0).

Signed-off-by: Torsten Fleischer <torfl6749@gmail.com>
---
 drivers/spi/spi-atmel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 9af7841..06de340 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -764,17 +764,17 @@ static void atmel_spi_pdc_next_xfer(struct
spi_master *master,
  (unsigned long long)xfer->rx_dma);
  }

- /* REVISIT: We're waiting for ENDRX before we start the next
+ /* REVISIT: We're waiting for RXBUFF before we start the next
  * transfer because we need to handle some difficult timing
- * issues otherwise. If we wait for ENDTX in one transfer and
- * then starts waiting for ENDRX in the next, it's difficult
- * to tell the difference between the ENDRX interrupt we're
- * actually waiting for and the ENDRX interrupt of the
+ * issues otherwise. If we wait for TXBUFE in one transfer and
+ * then starts waiting for RXBUFF in the next, it's difficult
+ * to tell the difference between the RXBUFF interrupt we're
+ * actually waiting for and the RXBUFF interrupt of the
  * previous transfer.
  *
  * It should be doable, though. Just not now...
  */
- spi_writel(as, IER, SPI_BIT(ENDRX) | SPI_BIT(OVRES));
+ spi_writel(as, IER, SPI_BIT(RXBUFF) | SPI_BIT(OVRES));
  spi_writel(as, PTCR, SPI_BIT(TXTEN) | SPI_BIT(RXTEN));
 }

-- 
2.1.4