dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Mark Brown <broonie@kernel.org>, Vinod Koul <vkoul@kernel.org>,
	Stefan Wahren <wahrenst@gmx.net>,
	linux-spi@vger.kernel.org, dmaengine@vger.kernel.org,
	linux-rpi-kernel@lists.infradead.org,
	bcm-kernel-feedback-list@broadcom.com
Cc: Eric Anholt <eric@anholt.net>, Nuno Sa <nuno.sa@analog.com>,
	Martin Sperl <kernel@martin.sperl.org>,
	Noralf Tronnes <noralf@tronnes.org>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	Florian Kauer <florian.kauer@koalo.de>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>
Subject: [PATCH 05/10] spi: bcm2835: Work around DONE bit erratum
Date: Sat, 3 Aug 2019 12:10:00 +0200	[thread overview]
Message-ID: <edb004dff4af6106f6bfcb89e1a96391e96eb857.1564825752.git.lukas@wunner.de> (raw)
In-Reply-To: <cover.1564825752.git.lukas@wunner.de>

Commit 3bd7f6589f67 ("spi: bcm2835: Overcome sglist entry length
limitation") amended the BCM2835 SPI driver with support for DMA
transfers whose buffers are not aligned to 4 bytes and require more than
one sglist entry.

When testing this feature with upcoming commits to speed up TX-only and
RX-only transfers, I noticed that SPI transmission sometimes breaks.
A function introduced by the commit, bcm2835_spi_transfer_prologue(),
performs one or two PIO transmissions as a prologue to the actual DMA
transmission.  It turns out that the breakage goes away if the DONE bit
in the CS register is set when ending such a PIO transmission.

The DONE bit signifies emptiness of the TX FIFO.  According to the spec,
the bit is of type RO, so writing it should never have any effect.
Perhaps the spec is wrong and the bit is actually of type RW1C.
E.g. the I2C controller on the BCM2835 does have an RW1C DONE bit which
needs to be cleared by the driver.  Another, possibly more likely
explanation is that it's a hardware erratum since the issue does not
occur consistently.

Either way, amend bcm2835_spi_transfer_prologue() to always write the
DONE bit.

Usually a transmission is ended by bcm2835_spi_reset_hw().  If the
transmission was successful, the TX FIFO is empty and thus the DONE bit
is set when bcm2835_spi_reset_hw() reads the CS register.  The bit is
then written back to the register, so we happen to do the right thing.

However if DONE is not set, e.g. because transmission is aborted with
a non-empty TX FIFO, the bit won't be written by bcm2835_spi_reset_hw()
and it seems possible that transmission might subsequently break.  To be
on the safe side, likewise amend bcm2835_spi_reset_hw() to always write
the bit.

Tested-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/spi/spi-bcm2835.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 2bf725e909fd..f6391c302363 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -317,6 +317,13 @@ static void bcm2835_spi_reset_hw(struct spi_controller *ctlr)
 		BCM2835_SPI_CS_INTD |
 		BCM2835_SPI_CS_DMAEN |
 		BCM2835_SPI_CS_TA);
+	/*
+	 * Transmission sometimes breaks unless the DONE bit is written at the
+	 * end of every transfer.  The spec says it's a RO bit.  Either the
+	 * spec is wrong and the bit is actually of type RW1C, or it's a
+	 * hardware erratum.
+	 */
+	cs |= BCM2835_SPI_CS_DONE;
 	/* and reset RX/TX FIFOS */
 	cs |= BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX;
 
@@ -475,7 +482,9 @@ static void bcm2835_spi_transfer_prologue(struct spi_controller *ctlr,
 		bcm2835_wr_fifo_count(bs, bs->rx_prologue);
 		bcm2835_wait_tx_fifo_empty(bs);
 		bcm2835_rd_fifo_count(bs, bs->rx_prologue);
-		bcm2835_spi_reset_hw(ctlr);
+		bcm2835_wr(bs, BCM2835_SPI_CS, cs | BCM2835_SPI_CS_CLEAR_RX
+						  | BCM2835_SPI_CS_CLEAR_TX
+						  | BCM2835_SPI_CS_DONE);
 
 		dma_sync_single_for_device(ctlr->dma_rx->device->dev,
 					   sg_dma_address(&tfr->rx_sg.sgl[0]),
@@ -496,7 +505,8 @@ static void bcm2835_spi_transfer_prologue(struct spi_controller *ctlr,
 						  | BCM2835_SPI_CS_DMAEN);
 		bcm2835_wr_fifo_count(bs, tx_remaining);
 		bcm2835_wait_tx_fifo_empty(bs);
-		bcm2835_wr(bs, BCM2835_SPI_CS, cs | BCM2835_SPI_CS_CLEAR_TX);
+		bcm2835_wr(bs, BCM2835_SPI_CS, cs | BCM2835_SPI_CS_CLEAR_TX
+						  | BCM2835_SPI_CS_DONE);
 	}
 
 	if (likely(!bs->tx_spillover)) {
-- 
2.20.1


  parent reply	other threads:[~2019-08-03 10:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-03 10:10 [PATCH 00/10] Raspberry Pi SPI speedups Lukas Wunner
2019-08-03 10:10 ` [PATCH 01/10] dmaengine: bcm2835: Allow reusable descriptors Lukas Wunner
2019-08-08 12:30   ` Vinod Koul
2019-08-03 10:10 ` [PATCH 04/10] spi: bcm2835: Drop dma_pending flag Lukas Wunner
2019-08-03 10:10 ` Lukas Wunner [this message]
2019-08-11 19:45   ` [PATCH 05/10] spi: bcm2835: Work around DONE bit erratum Stefan Wahren
2019-08-11 19:57     ` Lukas Wunner
2019-08-11 20:29       ` Eric Anholt
2019-08-19 19:20       ` Stefan Wahren
2019-08-03 10:10 ` [PATCH 09/10] dmaengine: bcm2835: Avoid accessing memory when copying zeroes Lukas Wunner
2019-08-08 12:31   ` Vinod Koul
2019-08-03 10:10 ` [PATCH 07/10] spi: bcm2835: Speed up TX-only DMA transfers by clearing RX FIFO Lukas Wunner
2019-08-03 10:10 ` [PATCH 06/10] spi: bcm2835: Cache CS register value for ->prepare_message() Lukas Wunner
2019-08-03 10:10 ` [PATCH 02/10] dmaengine: bcm2835: Allow cyclic transactions without interrupt Lukas Wunner
2019-08-08 12:30   ` Vinod Koul
2019-08-03 10:10 ` [PATCH 08/10] dmaengine: bcm2835: Document struct bcm2835_dmadev Lukas Wunner
2019-08-08 12:31   ` Vinod Koul
2019-08-03 10:10 ` [PATCH 10/10] spi: bcm2835: Speed up RX-only DMA transfers by zero-filling TX FIFO Lukas Wunner
2019-08-03 10:10 ` [PATCH 03/10] spi: Guarantee cacheline alignment of driver-private data Lukas Wunner
2019-09-10 11:29   ` Mark Brown
2019-08-03 16:01 ` [PATCH 00/10] Raspberry Pi SPI speedups Noralf Trønnes
2019-08-11 19:50 ` Stefan Wahren
2019-08-11 19:52   ` Lukas Wunner
2019-08-19 19:22 ` Stefan Wahren
2019-08-21 15:21 ` kernel
2019-08-24 10:33 ` Lukas Wunner
2019-09-07  9:06 ` Lukas Wunner
2019-09-09 16:56   ` Mark Brown
2019-09-10 11:21   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edb004dff4af6106f6bfcb89e1a96391e96eb857.1564825752.git.lukas@wunner.de \
    --to=lukas@wunner.de \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=broonie@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=eric@anholt.net \
    --cc=f.fainelli@gmail.com \
    --cc=florian.kauer@koalo.de \
    --cc=kernel@martin.sperl.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=noralf@tronnes.org \
    --cc=nuno.sa@analog.com \
    --cc=rjui@broadcom.com \
    --cc=robert.jarzmik@free.fr \
    --cc=sbranden@broadcom.com \
    --cc=vkoul@kernel.org \
    --cc=wahrenst@gmx.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).