Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1343791 > unrolled thread

[PATCH RESEND 0/2] spi: sun4i: add DMA support

Back to article view | Back to linux.kernel

Contents

  [PATCH RESEND 0/2] spi: sun4i: add DMA support Priit Laes <plaes@plaes.org> - 2016-02-26 07:00 +0100
    [PATCH 1/2] spi: sun4i: add DMA support Priit Laes <plaes@plaes.org> - 2016-02-26 07:00 +0100
      Re: [PATCH 1/2] spi: sun4i: add DMA support Mark Brown <broonie@kernel.org> - 2016-02-26 13:30 +0100
        Re: [PATCH 1/2] spi: sun4i: add DMA support Michal Suchanek <hramrach@gmail.com> - 2016-02-26 14:00 +0100
    [PATCH 2/2] ARM: sunxi: spi: add notice about SPI FIFO limit. Priit Laes <plaes@plaes.org> - 2016-02-26 07:00 +0100
    Re: [PATCH RESEND 0/2] spi: sun4i: add DMA support Mark Brown <broonie@kernel.org> - 2016-02-26 13:20 +0100

#1343791 — [PATCH RESEND 0/2] spi: sun4i: add DMA support

While trying to get SPI TFT screen working with Cubietruck, I noticed that
at current state driver didn't actually support SPI data bursts bigger than
n bytes (n is probably ~64 bytes).

Patches below were sent to mailinglists a while ago (May 2015) but apparently
were not addressed to correct maintainers and therefore were never applied.

With those patches, it's now possible to use TFT screens over SPI (fbtft
driver in staging), although there seems to be a possible issue where bigger
data bursts cause the driver to temporarily fall back to PIO mode:

spi_master spi32766: Using DMA mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using DMA mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer
spi_master spi32766: Using PIO mode for transfer


Emilio López (1):
  spi: sun4i: add DMA support

Michal Suchanek (1):
  ARM: sunxi: spi: add notice about SPI FIFO limit.

 drivers/spi/spi-sun4i.c | 142 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 130 insertions(+), 12 deletions(-)

-- 
2.7.2

[toc] | [next] | [standalone]

#1343792 — [PATCH 1/2] spi: sun4i: add DMA support

From: Emilio López <emilio@elopez.com.ar>

This patch adds support for 64 byte or bigger transfers on the
sun4i SPI controller. Said transfers will be performed via DMA.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Tested-by: Michal Suchanek <hramrach@gmail.com>
Tested-by: Priit Laes <plaes@plaes.org>
---
 drivers/spi/spi-sun4i.c | 140 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 128 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 1ddd9e2..78141a6 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -14,6 +14,8 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -34,6 +36,7 @@
 #define SUN4I_CTL_CPHA				BIT(2)
 #define SUN4I_CTL_CPOL				BIT(3)
 #define SUN4I_CTL_CS_ACTIVE_LOW			BIT(4)
+#define SUN4I_CTL_DMAMC_DEDICATED		BIT(5)
 #define SUN4I_CTL_LMTF				BIT(6)
 #define SUN4I_CTL_TF_RST			BIT(8)
 #define SUN4I_CTL_RF_RST			BIT(9)
@@ -51,6 +54,8 @@
 #define SUN4I_INT_STA_REG		0x10
 
 #define SUN4I_DMA_CTL_REG		0x14
+#define SUN4I_DMA_CTL_RF_READY			BIT(0)
+#define SUN4I_DMA_CTL_TF_NOT_FULL		BIT(10)
 
 #define SUN4I_WAIT_REG			0x18
 
@@ -130,6 +135,13 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)
 	}
 }
 
+static bool sun4i_spi_can_dma(struct spi_master *master,
+			      struct spi_device *spi,
+			      struct spi_transfer *tfr)
+{
+	return tfr->len >= SUN4I_FIFO_DEPTH;
+}
+
 static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(spi->master);
@@ -172,14 +184,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 				  struct spi_transfer *tfr)
 {
 	struct sun4i_spi *sspi = spi_master_get_devdata(master);
+	struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL;
 	unsigned int mclk_rate, div, timeout;
 	unsigned int tx_len = 0;
+	u32 reg, trigger = 0;
 	int ret = 0;
-	u32 reg;
-
-	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN4I_FIFO_DEPTH)
-		return -EINVAL;
 
 	reinit_completion(&sspi->done);
 	sspi->tx_buf = tfr->tx_buf;
@@ -189,7 +198,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	/* Clear pending interrupts */
 	sun4i_spi_write(sspi, SUN4I_INT_STA_REG, ~0);
 
-
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 
 	/* Reset FIFOs */
@@ -269,12 +277,65 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 	sun4i_spi_write(sspi, SUN4I_BURST_CNT_REG, SUN4I_BURST_CNT(tfr->len));
 	sun4i_spi_write(sspi, SUN4I_XMIT_CNT_REG, SUN4I_XMIT_CNT(tx_len));
 
-	/* Fill the TX FIFO */
-	sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
-
 	/* Enable the interrupts */
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC);
 
+	if (sun4i_spi_can_dma(master, spi, tfr)) {
+		dev_dbg(&sspi->master->dev, "Using DMA mode for transfer\n");
+
+		if (sspi->tx_buf) {
+			desc_tx = dmaengine_prep_slave_sg(master->dma_tx,
+					tfr->tx_sg.sgl, tfr->tx_sg.nents,
+					DMA_TO_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_tx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				return -EIO;
+			}
+
+			trigger |= SUN4I_DMA_CTL_TF_NOT_FULL;
+
+			dmaengine_submit(desc_tx);
+			dma_async_issue_pending(master->dma_tx);
+
+		}
+
+		if (sspi->rx_buf) {
+			desc_rx = dmaengine_prep_slave_sg(master->dma_rx,
+					tfr->rx_sg.sgl, tfr->rx_sg.nents,
+					DMA_FROM_DEVICE,
+					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+			if (!desc_rx) {
+				dev_err(&sspi->master->dev,
+					"Couldn't prepare dma slave\n");
+				return -EIO;
+			}
+
+			trigger |= SUN4I_DMA_CTL_RF_READY;
+
+			dmaengine_submit(desc_rx);
+			dma_async_issue_pending(master->dma_rx);
+		}
+
+		/* Enable Dedicated DMA requests */
+		reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+		reg |= SUN4I_CTL_DMAMC_DEDICATED;
+		sun4i_spi_write(sspi, SUN4I_CTL_REG, reg);
+		sun4i_spi_write(sspi, SUN4I_DMA_CTL_REG, trigger);
+	} else {
+		dev_dbg(&sspi->master->dev, "Using PIO mode for transfer\n");
+
+		/* Disable DMA requests */
+		reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
+		sun4i_spi_write(sspi, SUN4I_CTL_REG,
+				reg & ~SUN4I_CTL_DMAMC_DEDICATED);
+		sun4i_spi_write(sspi, SUN4I_DMA_CTL_REG, 0);
+
+		/* Fill the TX FIFO */
+		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
+	}
+
 	/* Start the transfer */
 	reg = sun4i_spi_read(sspi, SUN4I_CTL_REG);
 	sun4i_spi_write(sspi, SUN4I_CTL_REG, reg | SUN4I_CTL_XCH);
@@ -286,7 +347,12 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+	if (sun4i_spi_can_dma(master, spi, tfr) && desc_rx) {
+		/* The receive transfer should be the last one to finish */
+		dma_wait_for_async_tx(desc_rx);
+	} else {
+		sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH);
+	}
 
 out:
 	sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0);
@@ -351,6 +417,7 @@ static int sun4i_spi_runtime_suspend(struct device *dev)
 
 static int sun4i_spi_probe(struct platform_device *pdev)
 {
+	struct dma_slave_config dma_sconfig;
 	struct spi_master *master;
 	struct sun4i_spi *sspi;
 	struct resource	*res;
@@ -386,7 +453,9 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
+	init_completion(&sspi->done);
 	sspi->master = master;
+	master->can_dma = sun4i_spi_can_dma;
 	master->set_cs = sun4i_spi_set_cs;
 	master->transfer_one = sun4i_spi_transfer_one;
 	master->num_chipselect = 4;
@@ -409,7 +478,45 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 		goto err_free_master;
 	}
 
-	init_completion(&sspi->done);
+	master->dma_tx = dma_request_slave_channel_reason(&pdev->dev, "tx");
+	if (IS_ERR(master->dma_tx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel TX\n");
+		ret = PTR_ERR(master->dma_tx);
+		goto err_free_master;
+	}
+
+	dma_sconfig.direction = DMA_MEM_TO_DEV;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr = res->start + SUN4I_TXDATA_REG;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
+
+	ret = dmaengine_slave_config(master->dma_tx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure TX DMA slave\n");
+		goto err_tx_dma_release;
+	}
+
+	master->dma_rx = dma_request_slave_channel_reason(&pdev->dev, "rx");
+	if (IS_ERR(master->dma_rx)) {
+		dev_err(&pdev->dev, "Unable to acquire DMA channel RX\n");
+		ret = PTR_ERR(master->dma_rx);
+		goto err_tx_dma_release;
+	}
+
+	dma_sconfig.direction = DMA_DEV_TO_MEM;
+	dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	dma_sconfig.src_addr = res->start + SUN4I_RXDATA_REG;
+	dma_sconfig.src_maxburst = 1;
+	dma_sconfig.dst_maxburst = 1;
+
+	ret = dmaengine_slave_config(master->dma_rx, &dma_sconfig);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to configure RX DMA slave\n");
+		goto err_rx_dma_release;
+	}
 
 	/*
 	 * This wake-up/shutdown pattern is to be able to have the
@@ -418,7 +525,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 	ret = sun4i_spi_runtime_resume(&pdev->dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Couldn't resume the device\n");
-		goto err_free_master;
+		goto err_rx_dma_release;
 	}
 
 	pm_runtime_set_active(&pdev->dev);
@@ -436,6 +543,10 @@ static int sun4i_spi_probe(struct platform_device *pdev)
 err_pm_disable:
 	pm_runtime_disable(&pdev->dev);
 	sun4i_spi_runtime_suspend(&pdev->dev);
+err_rx_dma_release:
+	dma_release_channel(master->dma_rx);
+err_tx_dma_release:
+	dma_release_channel(master->dma_tx);
 err_free_master:
 	spi_master_put(master);
 	return ret;
@@ -443,8 +554,13 @@ err_free_master:
 
 static int sun4i_spi_remove(struct platform_device *pdev)
 {
+	struct spi_master *master = platform_get_drvdata(pdev);
+
 	pm_runtime_disable(&pdev->dev);
 
+	dma_release_channel(master->dma_rx);
+	dma_release_channel(master->dma_tx);
+
 	return 0;
 }
 
-- 
2.7.2

[toc] | [prev] | [next] | [standalone]

#1344217 — Re: [PATCH 1/2] spi: sun4i: add DMA support

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 26, 2016 at 07:56:56AM +0200, Priit Laes wrote:
> From: Emilio López <emilio@elopez.com.ar>
> 
> This patch adds support for 64 byte or bigger transfers on the
> sun4i SPI controller. Said transfers will be performed via DMA.
> 
> Signed-off-by: Emilio López <emilio@elopez.com.ar>
> Tested-by: Michal Suchanek <hramrach@gmail.com>
> Tested-by: Priit Laes <plaes@plaes.org>
> ---

You *must* sign off any patches you are sending, please see
SubmittingPatches.

[toc] | [prev] | [next] | [standalone]

#1344233 — Re: [PATCH 1/2] spi: sun4i: add DMA support

Hello,

On 26 February 2016 at 13:25, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Feb 26, 2016 at 07:56:56AM +0200, Priit Laes wrote:
>> From: Emilio López <emilio@elopez.com.ar>
>>
>> This patch adds support for 64 byte or bigger transfers on the
>> sun4i SPI controller. Said transfers will be performed via DMA.
>>
>> Signed-off-by: Emilio López <emilio@elopez.com.ar>
>> Tested-by: Michal Suchanek <hramrach@gmail.com>
>> Tested-by: Priit Laes <plaes@plaes.org>
>> ---
>
> You *must* sign off any patches you are sending, please see
> SubmittingPatches.

I have sent these patches in the past. Besides this non-technical
objection there were multiple technical objections.

IIRC one was that the driver does not handle the case when the DMA
channels are not available. As I understand it the channels are
exclusively reserved for a particular peripherial on sunxi platform so
this ShoulNotHappen(tm). So it's probably fine for the driver to fail
probe  when you have broken DT or no DMA engine support for sunxi
platform.

The driver could also work fully without DMA and there is a patch for
that also but since nobody will be testing that codepath it's probably
better to not include it.

I have not addressed the other objections so far.

Thanks

Michal

[toc] | [prev] | [next] | [standalone]

#1343793 — [PATCH 2/2] ARM: sunxi: spi: add notice about SPI FIFO limit.

From: Michal Suchanek <hramrach@gmail.com>

When testing SPI without DMA I noticed that filling the FIFO on the
spi controller causes timeout. This should never happen with DMA support
so just adding a comment.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
---
 drivers/spi/spi-sun4i.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
index 78141a6..b750664 100644
--- a/drivers/spi/spi-sun4i.c
+++ b/drivers/spi/spi-sun4i.c
@@ -333,7 +333,9 @@ static int sun4i_spi_transfer_one(struct spi_master *master,
 		sun4i_spi_write(sspi, SUN4I_DMA_CTL_REG, 0);
 
 		/* Fill the TX FIFO */
-		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH);
+		/* Filling the fifo fully causes timeout for some reason - at least on spi2 on a10s */
+		/* The can_dma check is txlen >= SUN4I_FIFO_DEPTH so with DMA this should never happen anyway. */
+		sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1);
 	}
 
 	/* Start the transfer */
-- 
2.7.2

[toc] | [prev] | [next] | [standalone]

#1344206

[Multipart message — attachments visible in raw view] — view raw

On Fri, Feb 26, 2016 at 07:56:55AM +0200, Priit Laes wrote:
> While trying to get SPI TFT screen working with Cubietruck, I noticed that
> at current state driver didn't actually support SPI data bursts bigger than
> n bytes (n is probably ~64 bytes).

If you're going to add noise like "RESEND" to your subject lines (which
makes the visible space for the actual subject smaller so makes handling
incoming mail harder) please at least make sure it's true, you've not
sent these before.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web