[PATCH 0/2] spi: Add DMA support for ti-qspi

[PATCH 0/2] spi: Add DMA support for ti-qspi

[Vignesh R]

This series adds support for DMA during QSPI flash read via memory
mapped mode.

Tested on DRA74 EVM on linux-next with patch for mtd support[1] applied.
Also tested with series[2] from Boris refractoring spi_map_buf().
[1]http://lists.infradead.org/pipermail/linux-mtd/2016-March/066335.html
[2]http://www.spinics.net/lists/linux-mm/msg104694.html



Vignesh R (2):
  spi: Add DMA support for spi_flash_read()
  spi: spi-ti-qspi: Add DMA support for QSPI mmap read

 drivers/spi/spi-ti-qspi.c | 132 +++++++++++++++++++++++++++++++++++++++++-----
 drivers/spi/spi.c         |  15 ++++++
 include/linux/spi/spi.h   |   2 +
 3 files changed, 137 insertions(+), 12 deletions(-)

-- 
2.8.0

[PATCH 1/2] spi: Add DMA support for spi_flash_read()

[Vignesh R]

Few SPI devices provide accelerated read interfaces to read from
SPI-NOR flash devices. These hardwares also support DMA to transfer data
from flash to memory either via mem-to-mem DMA or dedicated slave DMA
channels. Hence, add support for DMA in order to improve throughput and
reduce CPU load.
Use spi_map_buf() to get sg table for the buffer and pass it to SPI
driver.

Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
---
 drivers/spi/spi.c       | 15 +++++++++++++++
 include/linux/spi/spi.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index de2f2f90d799..2fb97f5b79ab 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2722,6 +2722,7 @@ int spi_flash_read(struct spi_device *spi,
 
 {
 	struct spi_master *master = spi->master;
+	struct device *rx_dev = NULL;
 	int ret;
 
 	if ((msg->opcode_nbits == SPI_NBITS_DUAL ||
@@ -2747,8 +2748,22 @@ int spi_flash_read(struct spi_device *spi,
 			return ret;
 		}
 	}
+
 	mutex_lock(&master->bus_lock_mutex);
+	if (master->dma_rx) {
+		rx_dev = master->dma_rx->device->dev;
+		ret = spi_map_buf(master, rx_dev, &msg->rx_sg,
+				  msg->buf, msg->len,
+				  DMA_FROM_DEVICE);
+		if (ret != 0)
+			goto  err;
+	}
 	ret = master->spi_flash_read(spi, msg);
+	if (master->dma_rx)
+		spi_unmap_buf(master, rx_dev, &msg->rx_sg,
+			      DMA_FROM_DEVICE);
+
+err:
 	mutex_unlock(&master->bus_lock_mutex);
 	if (master->auto_runtime_pm)
 		pm_runtime_put(master->dev.parent);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 857a9a1d82b5..5b9c745eda92 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1141,6 +1141,7 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
  * @opcode_nbits: number of lines to send opcode
  * @addr_nbits: number of lines to send address
  * @data_nbits: number of lines for data
+ * @rx_sg: Scatterlist for receive data
  */
 struct spi_flash_read_message {
 	void *buf;
@@ -1153,6 +1154,7 @@ struct spi_flash_read_message {
 	u8 opcode_nbits;
 	u8 addr_nbits;
 	u8 data_nbits;
+	struct sg_table rx_sg;
 };
 
 /* SPI core interface for flash read support */
-- 
2.8.0

--
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/2] spi: Add DMA support for spi_flash_read()

[Mark Brown]

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

On Tue, Apr 05, 2016 at 09:19:51AM +0530, Vignesh R wrote:

>  	mutex_lock(&master->bus_lock_mutex);
> +	if (master->dma_rx) {
> +		rx_dev = master->dma_rx->device->dev;
> +		ret = spi_map_buf(master, rx_dev, &msg->rx_sg,
> +				  msg->buf, msg->len,
> +				  DMA_FROM_DEVICE);
> +		if (ret != 0)
> +			goto  err;
> +	}

This is unconditionally DMA mapping the buffer if DMA is supported.
That's going to be common but I'm not sure it'll be universal, we need
to think of something better here.  I'm not immediately seeing what
though.  Possibly a flag...

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

Re: [PATCH 1/2] spi: Add DMA support for spi_flash_read()

[Vignesh R]

On 04/12/2016 10:01 AM, Mark Brown wrote:
> On Tue, Apr 05, 2016 at 09:19:51AM +0530, Vignesh R wrote:
> 
>>  	mutex_lock(&master->bus_lock_mutex);
>> +	if (master->dma_rx) {
>> +		rx_dev = master->dma_rx->device->dev;
>> +		ret = spi_map_buf(master, rx_dev, &msg->rx_sg,
>> +				  msg->buf, msg->len,
>> +				  DMA_FROM_DEVICE);
>> +		if (ret != 0)
>> +			goto  err;
>> +	}
> 
> This is unconditionally DMA mapping the buffer if DMA is supported.
> That's going to be common but I'm not sure it'll be universal, we need
> to think of something better here.  I'm not immediately seeing what
> though.  Possibly a flag...
> 

Ok, I will introduced a flag along the lines of cur_msg_mapped currently
part of spi_message struct.

This reminds me the issue of possible kmap'd buffers(falling in
PKMAP_BASE - PAGE_OFFSET-1  region) that might be passed to
spi_map_buf() which are not currently being handled properly. Boris
attempted to fix this in generic way[1] but was rejected as it couldn't
handle all type of caches.
I was wondering whether you would accept a patch returning error when
kmap'd buffers are passed to spi_map_buf()? Or would it still make sense
to port changes from that series to handle kmap'd buffers to SPI core alone?

[1]https://lkml.org/lkml/2016/3/31/462

-- 
Regards
Vignesh

[PATCH 2/2] spi: spi-ti-qspi: Add DMA support for QSPI mmap read

[Vignesh R]

Use mem-to-mem DMA to read from flash when reading in mmap mode. This
gives improved read performance and reduces CPU load.

With this patch the raw-read throughput is ~16MB/s on DRA74 EVM. And CPU
load is <20%. UBIFS read ~13 MB/s.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/spi-ti-qspi.c | 132 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 120 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index eac3c960b2de..1f6a11a45891 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -41,6 +41,8 @@ struct ti_qspi_regs {
 };
 
 struct ti_qspi {
+	struct completion	transfer_complete;
+
 	/* list synchronization */
 	struct mutex            list_lock;
 
@@ -54,6 +56,9 @@ struct ti_qspi {
 
 	struct ti_qspi_regs     ctx_reg;
 
+	dma_addr_t		mmap_phys_base;
+	struct dma_chan		*rx_chan;
+
 	u32 spi_max_frequency;
 	u32 cmd;
 	u32 dc;
@@ -377,6 +382,78 @@ static int qspi_transfer_msg(struct ti_qspi *qspi, struct spi_transfer *t)
 	return 0;
 }
 
+static void qspi_dma_callback(void *param)
+{
+	struct ti_qspi *qspi = param;
+
+	complete(&qspi->transfer_complete);
+}
+
+static int qspi_dma_transfer(struct ti_qspi *qspi, dma_addr_t dma_dst,
+			     dma_addr_t dma_src, size_t len)
+{
+	struct dma_chan *chan = qspi->rx_chan;
+	struct dma_device *dma_dev = chan->device;
+	dma_cookie_t cookie;
+	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
+	struct dma_async_tx_descriptor *tx = NULL;
+	int ret;
+
+	tx = dma_dev->device_prep_dma_memcpy(chan, dma_dst, dma_src,
+					     len, flags);
+	if (!tx) {
+		dev_err(qspi->dev, "device_prep_dma_memcpy error\n");
+		ret = -EIO;
+		goto err;
+	}
+
+	tx->callback = qspi_dma_callback;
+	tx->callback_param = qspi;
+	cookie = tx->tx_submit(tx);
+
+	ret = dma_submit_error(cookie);
+	if (ret) {
+		dev_err(qspi->dev, "dma_submit_error %d\n", cookie);
+		goto err;
+	}
+
+	dma_async_issue_pending(chan);
+	ret = wait_for_completion_timeout(&qspi->transfer_complete,
+					  msecs_to_jiffies(len));
+	if (ret <= 0) {
+		dmaengine_terminate_all(chan);
+		dev_err(qspi->dev, "DMA wait_for_completion_timeout\n");
+		if (!ret)
+			ret = -ETIMEDOUT;
+		goto err;
+	}
+
+	ret = 0;
+
+err:
+	return ret;
+}
+
+static int qspi_dma_sg(struct ti_qspi *qspi,
+		       struct spi_flash_read_message *msg)
+{
+	struct scatterlist *sg;
+	dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
+	dma_addr_t dma_dst;
+	int i, len, ret = 0;
+
+	for_each_sg(msg->rx_sg.sgl, sg, msg->rx_sg.nents, i) {
+		dma_dst = sg_dma_address(sg);
+		len = sg_dma_len(sg);
+		ret = qspi_dma_transfer(qspi, dma_dst, dma_src, len);
+		if (ret != 0)
+			return ret;
+		dma_src += len;
+	}
+
+	return ret;
+}
+
 static void ti_qspi_enable_memory_map(struct spi_device *spi)
 {
 	struct ti_qspi  *qspi = spi_master_get_devdata(spi->master);
@@ -435,9 +512,17 @@ static int ti_qspi_spi_flash_read(struct  spi_device *spi,
 	if (!qspi->mmap_enabled)
 		ti_qspi_enable_memory_map(spi);
 	ti_qspi_setup_mmap_read(spi, msg);
-	memcpy_fromio(msg->buf, qspi->mmap_base + msg->from, msg->len);
+
+	if (qspi->rx_chan) {
+		ret = qspi_dma_sg(qspi, msg);
+		if (ret != 0)
+			goto err;
+	} else {
+		memcpy_fromio(msg->buf, qspi->mmap_base + msg->from, msg->len);
+	}
 	msg->retlen = msg->len;
 
+err:
 	mutex_unlock(&qspi->list_lock);
 
 	return ret;
@@ -525,6 +610,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	u32 max_freq;
 	int ret = 0, num_cs, irq;
+	dma_cap_mask_t mask;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(*qspi));
 	if (!master)
@@ -539,6 +625,7 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	master->dev.of_node = pdev->dev.of_node;
 	master->bits_per_word_mask = SPI_BPW_MASK(32) | SPI_BPW_MASK(16) |
 				     SPI_BPW_MASK(8);
+	master->spi_flash_read = ti_qspi_spi_flash_read;
 
 	if (!of_property_read_u32(np, "num-cs", &num_cs))
 		master->num_chipselect = num_cs;
@@ -581,17 +668,6 @@ static int ti_qspi_probe(struct platform_device *pdev)
 		goto free_master;
 	}
 
-	if (res_mmap) {
-		qspi->mmap_base = devm_ioremap_resource(&pdev->dev,
-							res_mmap);
-		master->spi_flash_read = ti_qspi_spi_flash_read;
-		if (IS_ERR(qspi->mmap_base)) {
-			dev_err(&pdev->dev,
-				"falling back to PIO mode\n");
-			master->spi_flash_read = NULL;
-		}
-	}
-	qspi->mmap_enabled = false;
 
 	if (of_property_read_bool(np, "syscon-chipselects")) {
 		qspi->ctrl_base =
@@ -626,6 +702,33 @@ static int ti_qspi_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_master;
 
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_MEMCPY, mask);
+
+	qspi->rx_chan = dma_request_channel(mask, NULL, NULL);
+	if (!qspi->rx_chan) {
+		dev_err(qspi->dev,
+			"No Rx DMA available, trying mmap mode\n");
+		ret = 0;
+		goto no_dma;
+	}
+	master->dma_rx = qspi->rx_chan;
+	init_completion(&qspi->transfer_complete);
+	if (res_mmap)
+		qspi->mmap_phys_base = (dma_addr_t)res_mmap->start;
+
+no_dma:
+	if (!qspi->rx_chan && res_mmap) {
+		qspi->mmap_base = devm_ioremap_resource(&pdev->dev, res_mmap);
+		if (IS_ERR(qspi->mmap_base)) {
+			dev_info(&pdev->dev,
+				 "mmap failed with error %ld using PIO mode\n",
+				 PTR_ERR(qspi->mmap_base));
+			qspi->mmap_base = NULL;
+			master->spi_flash_read = NULL;
+		}
+	}
+	qspi->mmap_enabled = false;
 	return 0;
 
 free_master:
@@ -635,9 +738,14 @@ free_master:
 
 static int ti_qspi_remove(struct platform_device *pdev)
 {
+	struct ti_qspi *qspi = platform_get_drvdata(pdev);
+
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (qspi->rx_chan)
+		dma_release_channel(qspi->rx_chan);
+
 	return 0;
 }
 
-- 
2.8.0