[2/2] spi: spi-ti-qspi: Use bounce buffer if read buffer is not DMA'ble
diff mbox

Message ID 20170331114950.21489-3-vigneshr@ti.com
State New
Headers show

Commit Message

Vignesh Raghavendra March 31, 2017, 11:49 a.m. UTC
Flash filesystems like JFFS2, UBIFS and MTD block layer can provide
vmalloc'd or kmap'd buffers that cannot be mapped using dma_map_sg() and
can potentially be in memory region above 32bit addressable region(ie
buffers belonging to memory region backed by LPAE) of DMA, implement
spi_flash_can_dma() interface to inform SPI core not to map such
buffers.
When buffers are not mapped for DMA, then use a pre allocated bounce
buffer(64K = typical flash erase sector size) to read from flash and
then do a copy to actual destination buffer. This is approach is much
faster than using memcpy using CPU and also reduces CPU load.

With this patch, UBIFS read speed is ~18MB/s and CPU utilization <20% on
DRA74 Rev H EVM. Performance degradation is negligible when compared
with non bounce buffer case while using UBIFS.

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

Comments

kbuild test robot April 2, 2017, 11:33 p.m. UTC | #1
Hi Vignesh,

[auto build test WARNING on spi/for-next]
[also build test WARNING on v4.11-rc4 next-20170331]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vignesh-R/spi-ti-qspi-Handle-vmalloc-d-buffers/20170403-030332
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': unknown attribute
   drivers/spi/spi-ti-qspi.c:449:35: sparse: incompatible types in comparison expression (different type sizes)
   drivers/spi/spi-ti-qspi.c: In function 'ti_qspi_dma_bounce_buffer':
>> drivers/spi/spi-ti-qspi.c:440:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     unsigned int to = (unsigned int)msg->buf;
                       ^
   In file included from drivers/spi/spi-ti-qspi.c:16:0:
   include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:758:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
>> drivers/spi/spi-ti-qspi.c:449:21: note: in expansion of macro 'min'
      size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
                        ^~~
>> drivers/spi/spi-ti-qspi.c:455:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
             ^

sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> drivers/spi/spi-ti-qspi.c:449:35: sparse: incompatible types in comparison expression (different type sizes)
   drivers/spi/spi-ti-qspi.c: In function 'ti_qspi_dma_bounce_buffer':
   drivers/spi/spi-ti-qspi.c:440:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     unsigned int to = (unsigned int)msg->buf;
                       ^
   In file included from drivers/spi/spi-ti-qspi.c:16:0:
   include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:758:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
   drivers/spi/spi-ti-qspi.c:449:21: note: in expansion of macro 'min'
      size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
                        ^~~
   drivers/spi/spi-ti-qspi.c:455:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
             ^

vim +440 drivers/spi/spi-ti-qspi.c

   434	}
   435	
   436	static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
   437					     struct spi_flash_read_message *msg)
   438	{
   439		size_t readsize = msg->len;
 > 440		unsigned int to = (unsigned int)msg->buf;
   441		dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
   442		int ret = 0;
   443	
   444		/*
   445		 * Use bounce buffer as FS like jffs2, ubifs may pass
   446		 * buffers that does not belong to kernel lowmem region.
   447		 */
   448		while (readsize != 0) {
 > 449			size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
   450	
   451			ret = ti_qspi_dma_xfer(qspi, qspi->rx_bb_dma_addr,
   452					       dma_src, xfer_len);
   453			if (ret != 0)
   454				return ret;
 > 455			memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
   456			readsize -= xfer_len;
   457			dma_src += xfer_len;
   458			to += xfer_len;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Vignesh Raghavendra April 3, 2017, 6:22 a.m. UTC | #2
On Monday 03 April 2017 05:03 AM, kbuild test robot wrote:
> Hi Vignesh,
> 
> [auto build test WARNING on spi/for-next]
> [also build test WARNING on v4.11-rc4 next-20170331]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Vignesh-R/spi-ti-qspi-Handle-vmalloc-d-buffers/20170403-030332
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> config: x86_64-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):

Thanks for the report. I will fix below warnings in the next version.

> 
>    include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': unknown attribute
>    drivers/spi/spi-ti-qspi.c:449:35: sparse: incompatible types in comparison expression (different type sizes)
>    drivers/spi/spi-ti-qspi.c: In function 'ti_qspi_dma_bounce_buffer':
>>> drivers/spi/spi-ti-qspi.c:440:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>      unsigned int to = (unsigned int)msg->buf;
>                        ^
>    In file included from drivers/spi/spi-ti-qspi.c:16:0:
>    include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
>      (void) (&min1 == &min2);   \
>                    ^
>    include/linux/kernel.h:758:2: note: in expansion of macro '__min'
>      __min(typeof(x), typeof(y),   \
>      ^~~~~
>>> drivers/spi/spi-ti-qspi.c:449:21: note: in expansion of macro 'min'
>       size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
>                         ^~~
>>> drivers/spi/spi-ti-qspi.c:455:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>       memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
>              ^
> 
> sparse warnings: (new ones prefixed by >>)
> 
>    include/linux/compiler.h:264:8: sparse: attribute 'no_sanitize_address': unknown attribute
>>> drivers/spi/spi-ti-qspi.c:449:35: sparse: incompatible types in comparison expression (different type sizes)
>    drivers/spi/spi-ti-qspi.c: In function 'ti_qspi_dma_bounce_buffer':
>    drivers/spi/spi-ti-qspi.c:440:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>      unsigned int to = (unsigned int)msg->buf;
>                        ^
>    In file included from drivers/spi/spi-ti-qspi.c:16:0:
>    include/linux/kernel.h:755:16: warning: comparison of distinct pointer types lacks a cast
>      (void) (&min1 == &min2);   \
>                    ^
>    include/linux/kernel.h:758:2: note: in expansion of macro '__min'
>      __min(typeof(x), typeof(y),   \
>      ^~~~~
>    drivers/spi/spi-ti-qspi.c:449:21: note: in expansion of macro 'min'
>       size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
>                         ^~~
>    drivers/spi/spi-ti-qspi.c:455:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
>       memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
>              ^
> 
> vim +440 drivers/spi/spi-ti-qspi.c
> 
>    434	}
>    435	
>    436	static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
>    437					     struct spi_flash_read_message *msg)
>    438	{
>    439		size_t readsize = msg->len;
>  > 440		unsigned int to = (unsigned int)msg->buf;
>    441		dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
>    442		int ret = 0;
>    443	
>    444		/*
>    445		 * Use bounce buffer as FS like jffs2, ubifs may pass
>    446		 * buffers that does not belong to kernel lowmem region.
>    447		 */
>    448		while (readsize != 0) {
>  > 449			size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
>    450	
>    451			ret = ti_qspi_dma_xfer(qspi, qspi->rx_bb_dma_addr,
>    452					       dma_src, xfer_len);
>    453			if (ret != 0)
>    454				return ret;
>  > 455			memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
>    456			readsize -= xfer_len;
>    457			dma_src += xfer_len;
>    458			to += xfer_len;
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
kbuild test robot April 4, 2017, 10:56 a.m. UTC | #3
Hi Vignesh,

[auto build test WARNING on spi/for-next]
[also build test WARNING on v4.11-rc5 next-20170404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vignesh-R/spi-ti-qspi-Handle-vmalloc-d-buffers/20170403-030332
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: tile-allmodconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers//spi/spi-ti-qspi.c: In function 'ti_qspi_dma_bounce_buffer':
   drivers//spi/spi-ti-qspi.c:440:20: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>> drivers//spi/spi-ti-qspi.c:449:21: warning: comparison of distinct pointer types lacks a cast [enabled by default]
   drivers//spi/spi-ti-qspi.c:455:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

vim +449 drivers//spi/spi-ti-qspi.c

   434	}
   435	
   436	static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
   437					     struct spi_flash_read_message *msg)
   438	{
   439		size_t readsize = msg->len;
 > 440		unsigned int to = (unsigned int)msg->buf;
   441		dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
   442		int ret = 0;
   443	
   444		/*
   445		 * Use bounce buffer as FS like jffs2, ubifs may pass
   446		 * buffers that does not belong to kernel lowmem region.
   447		 */
   448		while (readsize != 0) {
 > 449			size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
   450	
   451			ret = ti_qspi_dma_xfer(qspi, qspi->rx_bb_dma_addr,
   452					       dma_src, xfer_len);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 7b39bc204a30..dce6ce634143 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -57,6 +57,8 @@  struct ti_qspi {
 	struct ti_qspi_regs     ctx_reg;
 
 	dma_addr_t		mmap_phys_base;
+	dma_addr_t		rx_bb_dma_addr;
+	void			*rx_bb_addr;
 	struct dma_chan		*rx_chan;
 
 	u32 spi_max_frequency;
@@ -126,6 +128,8 @@  struct ti_qspi {
 #define QSPI_SETUP_ADDR_SHIFT		8
 #define QSPI_SETUP_DUMMY_SHIFT		10
 
+#define QSPI_DMA_BUFFER_SIZE            65536U
+
 static inline unsigned long ti_qspi_read(struct ti_qspi *qspi,
 		unsigned long reg)
 {
@@ -429,6 +433,34 @@  static int ti_qspi_dma_xfer(struct ti_qspi *qspi, dma_addr_t dma_dst,
 	return 0;
 }
 
+static int ti_qspi_dma_bounce_buffer(struct ti_qspi *qspi,
+				     struct spi_flash_read_message *msg)
+{
+	size_t readsize = msg->len;
+	unsigned int to = (unsigned int)msg->buf;
+	dma_addr_t dma_src = qspi->mmap_phys_base + msg->from;
+	int ret = 0;
+
+	/*
+	 * Use bounce buffer as FS like jffs2, ubifs may pass
+	 * buffers that does not belong to kernel lowmem region.
+	 */
+	while (readsize != 0) {
+		size_t xfer_len = min(QSPI_DMA_BUFFER_SIZE, readsize);
+
+		ret = ti_qspi_dma_xfer(qspi, qspi->rx_bb_dma_addr,
+				       dma_src, xfer_len);
+		if (ret != 0)
+			return ret;
+		memcpy((void *)to, qspi->rx_bb_addr, xfer_len);
+		readsize -= xfer_len;
+		dma_src += xfer_len;
+		to += xfer_len;
+	}
+
+	return ret;
+}
+
 static int ti_qspi_dma_xfer_sg(struct ti_qspi *qspi, struct sg_table rx_sg,
 			       loff_t from)
 {
@@ -496,6 +528,12 @@  static void ti_qspi_setup_mmap_read(struct spi_device *spi,
 		      QSPI_SPI_SETUP_REG(spi->chip_select));
 }
 
+static bool ti_qspi_spi_flash_can_dma(struct spi_device *spi,
+				      struct spi_flash_read_message *msg)
+{
+	return virt_addr_valid(msg->buf);
+}
+
 static int ti_qspi_spi_flash_read(struct spi_device *spi,
 				  struct spi_flash_read_message *msg)
 {
@@ -509,15 +547,12 @@  static int ti_qspi_spi_flash_read(struct spi_device *spi,
 	ti_qspi_setup_mmap_read(spi, msg);
 
 	if (qspi->rx_chan) {
-		if (msg->cur_msg_mapped) {
+		if (msg->cur_msg_mapped)
 			ret = ti_qspi_dma_xfer_sg(qspi, msg->rx_sg, msg->from);
-			if (ret)
-				goto err_unlock;
-		} else {
-			dev_err(qspi->dev, "Invalid address for DMA\n");
-			ret = -EIO;
+		else
+			ret = ti_qspi_dma_bounce_buffer(qspi, msg);
+		if (ret)
 			goto err_unlock;
-		}
 	} else {
 		memcpy_fromio(msg->buf, qspi->mmap_base + msg->from, msg->len);
 	}
@@ -723,6 +758,17 @@  static int ti_qspi_probe(struct platform_device *pdev)
 		ret = 0;
 		goto no_dma;
 	}
+	qspi->rx_bb_addr = dma_alloc_coherent(qspi->dev,
+					      QSPI_DMA_BUFFER_SIZE,
+					      &qspi->rx_bb_dma_addr,
+					      GFP_KERNEL | GFP_DMA);
+	if (!qspi->rx_bb_addr) {
+		dev_err(qspi->dev,
+			"dma_alloc_coherent failed, using PIO mode\n");
+		dma_release_channel(qspi->rx_chan);
+		goto no_dma;
+	}
+	master->spi_flash_can_dma = ti_qspi_spi_flash_can_dma;
 	master->dma_rx = qspi->rx_chan;
 	init_completion(&qspi->transfer_complete);
 	if (res_mmap)
@@ -763,6 +809,10 @@  static int ti_qspi_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (qspi->rx_bb_addr)
+		dma_free_coherent(qspi->dev, QSPI_DMA_BUFFER_SIZE,
+				  qspi->rx_bb_addr,
+				  qspi->rx_bb_dma_addr);
 	if (qspi->rx_chan)
 		dma_release_channel(qspi->rx_chan);