[v5,3/4] spi/spi-bcm2835: Split transfers that exceed DLEN

Commit Message

Noralf Trønnes April 13, 2019, 6:24 p.m. UTC

From: Meghana Madhyastha <[e-mail]>

Split spi transfers into chunks of <=65532 to enable the driver to
perform DMA transfer on big buffers. The DLEN register specifies the
number of bytes to transfer in DMA mode. It is 16-bit wide and thus the
maximum DMA transfer is 65535 bytes. Set the maximum to 65532 (4 byte
aligned) since the FIFO in DMA mode is accessed in 4 byte chunks.

->max_dma_len is the value the spi core uses when splitting the buffer
into scatter gather entries.
The BCM2835 DMA block has 2 types of channels/engines:
- Normal: Max length: 1G
- Lite: Max length: 65535

Even when using a Lite channel we will not exceed the max length, so
let's drop setting ->max_dma_len.

Signed-off-by: Meghana Madhyastha <[e-mail]>
Signed-off-by: Noralf Trønnes <[e-mail]>
---
 drivers/spi/spi-bcm2835.c | 39 +++++++++++----------------------------
 1 file changed, 11 insertions(+), 28 deletions(-)

Comments

Lukas Wunner April 13, 2019, 8:24 p.m. UTC | #1

On Sat, Apr 13, 2019 at 08:24:14PM +0200, Noralf Trønnes wrote:
> @@ -844,6 +816,17 @@ static int bcm2835_spi_prepare_message(struct spi_master *master,
>  	struct spi_device *spi = msg->spi;
>  	struct bcm2835_spi *bs = spi_master_get_devdata(master);
>  	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
> +	int ret;
> +
> +	/*
> +	 * DMA transfers are limited to 16 bit (0 to 65535 bytes) by the SPI HW
> +	 * due to DLEN. Split up transfers (32-bit FIFO aligned) if the limit is
> +	 * exceeded.
> +	 */
> +	ret = spi_split_transfers_maxsize(master, msg, 65532,
> +					  GFP_KERNEL | GFP_DMA);
> +	if (ret)
> +		return ret;

This looks much better than the previous version because
spi_split_transfers_maxsize() is now used as a library function
by an individual driver, rather than something that is inflicted
on *all* drivers (midlayer fallacy).

Of course the performance is suboptimal compared to an approach
which transmits the sglist in portions which are < 65535 each,
but we can move to that later.

In case it helps,
Reviewed-by: Lukas Wunner <[e-mail]>

Thanks,

Lukas

Patch

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 35aebdfd3b4e..8aa22713c483 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -335,20 +335,6 @@  static int bcm2835_spi_transfer_one_irq(struct spi_master *master,
 	return 1;
 }
 
-/*
- * DMA support
- *
- * this implementation has currently a few issues in so far as it does
- * not work arrount limitations of the HW.
- *
- * the main one being that DMA transfers are limited to 16 bit
- * (so 0 to 65535 bytes) by the SPI HW due to BCM2835_SPI_DLEN
- *
- * there may be a few more border-cases we may need to address as well
- * but unfortunately this would mean splitting up the scatter-gather
- * list making it slightly unpractical...
- */
-
 /**
  * bcm2835_spi_transfer_prologue() - transfer first few bytes without DMA
  * @master: SPI master
@@ -630,19 +616,6 @@  static bool bcm2835_spi_can_dma(struct spi_master *master,
 	if (tfr->len < BCM2835_SPI_DMA_MIN_LENGTH)
 		return false;
 
-	/* BCM2835_SPI_DLEN has defined a max transfer size as
-	 * 16 bit, so max is 65535
-	 * we can revisit this by using an alternative transfer
-	 * method - ideally this would get done without any more
-	 * interaction...
-	 */
-	if (tfr->len > 65535) {
-		dev_warn_once(&spi->dev,
-			      "transfer size of %d too big for dma-transfer\n",
-			      tfr->len);
-		return false;
-	}
-
 	/* return OK */
 	return true;
 }
@@ -707,7 +680,6 @@  static void bcm2835_dma_init(struct spi_master *master, struct device *dev)
 
 	/* all went well, so set can_dma */
 	master->can_dma = bcm2835_spi_can_dma;
-	master->max_dma_len = 65535; /* limitation by BCM2835_SPI_DLEN */
 	/* need to do TX AND RX DMA, so we need dummy buffers */
 	master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
 
@@ -844,6 +816,17 @@  static int bcm2835_spi_prepare_message(struct spi_master *master,
 	struct spi_device *spi = msg->spi;
 	struct bcm2835_spi *bs = spi_master_get_devdata(master);
 	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);
+	int ret;
+
+	/*
+	 * DMA transfers are limited to 16 bit (0 to 65535 bytes) by the SPI HW
+	 * due to DLEN. Split up transfers (32-bit FIFO aligned) if the limit is
+	 * exceeded.
+	 */
+	ret = spi_split_transfers_maxsize(master, msg, 65532,
+					  GFP_KERNEL | GFP_DMA);
+	if (ret)
+		return ret;
 
 	cs &= ~(BCM2835_SPI_CS_CPOL | BCM2835_SPI_CS_CPHA);