linux-samsung-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx
@ 2022-09-16 11:39 Vincent Whitchurch
  2022-09-16 11:39 ` [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing Vincent Whitchurch
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-16 11:39 UTC (permalink / raw)
  To: broonie, krzysztof.kozlowski, andi
  Cc: kernel, Vincent Whitchurch, alim.akhtar, linux-spi, linux-kernel,
	linux-samsung-soc, linux-arm-kernel

This series fixes some bugs I found while running spi-loopback-test with
spi-s3c64xx.  The first problem (which I actually noticed while trying to fix
the second problem with transfers >64KiB) seems to be a generic issue which
affects several drivers so I fixed it in the core.

The series has been tested on ARTPEC-8, which has a version of the IP similar
to Exynos 7 and with 64 byte FIFOs (compatible with "tesla,fsd-spi").

Cc: alim.akhtar@samsung.com
Cc: linux-spi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org

Vincent Whitchurch (4):
  spi: spi-loopback-test: Add test to trigger DMA/PIO mixing
  spi: Save current RX and TX DMA devices
  spi: Fix cache corruption due to DMA/PIO overlap
  spi: s3c64xx: Fix large transfers with DMA

 drivers/spi/spi-loopback-test.c |  27 +++++++
 drivers/spi/spi-s3c64xx.c       |  10 +++
 drivers/spi/spi.c               | 126 +++++++++++++++++++++++---------
 include/linux/spi/spi.h         |   4 +
 4 files changed, 131 insertions(+), 36 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing
  2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
@ 2022-09-16 11:39 ` Vincent Whitchurch
  2022-09-19 14:41   ` Mark Brown
  2022-09-16 11:39 ` [PATCH 2/4] spi: Save current RX and TX DMA devices Vincent Whitchurch
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-16 11:39 UTC (permalink / raw)
  To: broonie, krzysztof.kozlowski, andi
  Cc: kernel, Vincent Whitchurch, alim.akhtar, linux-spi, linux-kernel,
	linux-samsung-soc, linux-arm-kernel

Add a test where a small and a large transfer in a message hit the same
cache line.  This test currently fails on spi-s3c64xx on in DMA mode
since it ends up mixing DMA and PIO without proper cache maintenance.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/spi/spi-loopback-test.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 4d4f77a186a9..dd7de8fa37d0 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -313,6 +313,33 @@ static struct spi_test spi_tests[] = {
 			},
 		},
 	},
+	{
+		.description	= "three tx+rx transfers with overlapping cache lines",
+		.fill_option	= FILL_COUNT_8,
+		/*
+		 * This should be large enough for the controller driver to
+		 * choose to transfer it with DMA.
+		 */
+		.iterate_len    = { 512, -1 },
+		.iterate_transfer_mask = BIT(1),
+		.transfer_count = 3,
+		.transfers		= {
+			{
+				.len = 1,
+				.tx_buf = TX(0),
+				.rx_buf = RX(0),
+			},
+			{
+				.tx_buf = TX(1),
+				.rx_buf = RX(1),
+			},
+			{
+				.len = 1,
+				.tx_buf = TX(513),
+				.rx_buf = RX(513),
+			},
+		},
+	},
 
 	{ /* end of tests sequence */ }
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] spi: Save current RX and TX DMA devices
  2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
  2022-09-16 11:39 ` [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing Vincent Whitchurch
@ 2022-09-16 11:39 ` Vincent Whitchurch
  2022-09-16 11:39 ` [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap Vincent Whitchurch
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-16 11:39 UTC (permalink / raw)
  To: broonie, krzysztof.kozlowski, andi
  Cc: kernel, Vincent Whitchurch, alim.akhtar, linux-spi, linux-kernel,
	linux-samsung-soc, linux-arm-kernel

Save the current RX and TX DMA devices to avoid having to duplicate the
logic to pick them, since we'll need access to them in some more
functions to fix a bug in the cache handling.

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/spi/spi.c       | 19 ++++---------------
 include/linux/spi/spi.h |  4 ++++
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 32c01e684af3..a9134d062ff1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1147,6 +1147,8 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 		}
 	}
 
+	ctlr->cur_rx_dma_dev = rx_dev;
+	ctlr->cur_tx_dma_dev = tx_dev;
 	ctlr->cur_msg_mapped = true;
 
 	return 0;
@@ -1154,26 +1156,13 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 
 static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
 {
+	struct device *rx_dev = ctlr->cur_rx_dma_dev;
+	struct device *tx_dev = ctlr->cur_tx_dma_dev;
 	struct spi_transfer *xfer;
-	struct device *tx_dev, *rx_dev;
 
 	if (!ctlr->cur_msg_mapped || !ctlr->can_dma)
 		return 0;
 
-	if (ctlr->dma_tx)
-		tx_dev = ctlr->dma_tx->device->dev;
-	else if (ctlr->dma_map_dev)
-		tx_dev = ctlr->dma_map_dev;
-	else
-		tx_dev = ctlr->dev.parent;
-
-	if (ctlr->dma_rx)
-		rx_dev = ctlr->dma_rx->device->dev;
-	else if (ctlr->dma_map_dev)
-		rx_dev = ctlr->dma_map_dev;
-	else
-		rx_dev = ctlr->dev.parent;
-
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		if (!ctlr->can_dma(ctlr, msg->spi, xfer))
 			continue;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f089ee1ead58..7466a171d7e5 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -378,6 +378,8 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
  * @cleanup: frees controller-specific state
  * @can_dma: determine whether this controller supports DMA
  * @dma_map_dev: device which can be used for DMA mapping
+ * @cur_rx_dma_dev: device which is currently used for RX DMA mapping
+ * @cur_tx_dma_dev: device which is currently used for TX DMA mapping
  * @queued: whether this controller is providing an internal message queue
  * @kworker: pointer to thread struct for message pump
  * @pump_messages: work struct for scheduling work to the message pump
@@ -610,6 +612,8 @@ struct spi_controller {
 					   struct spi_device *spi,
 					   struct spi_transfer *xfer);
 	struct device *dma_map_dev;
+	struct device *cur_rx_dma_dev;
+	struct device *cur_tx_dma_dev;
 
 	/*
 	 * These hooks are for drivers that want to use the generic
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap
  2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
  2022-09-16 11:39 ` [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing Vincent Whitchurch
  2022-09-16 11:39 ` [PATCH 2/4] spi: Save current RX and TX DMA devices Vincent Whitchurch
@ 2022-09-16 11:39 ` Vincent Whitchurch
  2022-09-16 16:06   ` kernel test robot
  2022-09-16 11:39 ` [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA Vincent Whitchurch
  2022-09-19 16:50 ` (subset) [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Mark Brown
  4 siblings, 1 reply; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-16 11:39 UTC (permalink / raw)
  To: broonie, krzysztof.kozlowski, andi
  Cc: kernel, Vincent Whitchurch, alim.akhtar, linux-spi, linux-kernel,
	linux-samsung-soc, linux-arm-kernel

The SPI core DMA mapping support performs cache management once for the
entire message and not between transfers, and this leads to cache
corruption if a message has two or more RX transfers with both
transfers targeting the same cache line, and the controller driver
decides to handle one using DMA and the other using PIO (for example,
because one is much larger than the other).

Fix it by syncing before/after the actual transfers.  This also means
that we can skip the sync during the map/unmap of the message.

Fixes: 99adef310f682d6343 ("spi: Provide core support for DMA mapping transfers")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/spi/spi.c | 107 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a9134d062ff1..f7cd737bbf6f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1010,9 +1010,9 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
 }
 
 #ifdef CONFIG_HAS_DMA
-int spi_map_buf(struct spi_controller *ctlr, struct device *dev,
-		struct sg_table *sgt, void *buf, size_t len,
-		enum dma_data_direction dir)
+static int spi_map_buf_attrs(struct spi_controller *ctlr, struct device *dev,
+			     struct sg_table *sgt, void *buf, size_t len,
+			     enum dma_data_direction dir, unsigned long attrs)
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	unsigned int max_seg_size = dma_get_max_seg_size(dev);
@@ -1078,28 +1078,39 @@ int spi_map_buf(struct spi_controller *ctlr, struct device *dev,
 		sg = sg_next(sg);
 	}
 
-	ret = dma_map_sg(dev, sgt->sgl, sgt->nents, dir);
-	if (!ret)
-		ret = -ENOMEM;
+	ret = dma_map_sgtable(dev, sgt, dir, attrs);
 	if (ret < 0) {
 		sg_free_table(sgt);
 		return ret;
 	}
 
-	sgt->nents = ret;
-
 	return 0;
 }
 
-void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
-		   struct sg_table *sgt, enum dma_data_direction dir)
+int spi_map_buf(struct spi_controller *ctlr, struct device *dev,
+		struct sg_table *sgt, void *buf, size_t len,
+		enum dma_data_direction dir)
+{
+	return spi_map_buf_attrs(ctlr, dev, sgt, buf, len, dir, 0);
+}
+
+static void spi_unmap_buf_attrs(struct spi_controller *ctlr,
+				struct device *dev, struct sg_table *sgt,
+				enum dma_data_direction dir,
+				unsigned long attrs)
 {
 	if (sgt->orig_nents) {
-		dma_unmap_sg(dev, sgt->sgl, sgt->orig_nents, dir);
+		dma_unmap_sgtable(dev, sgt, dir, attrs);
 		sg_free_table(sgt);
 	}
 }
 
+void spi_unmap_buf(struct spi_controller *ctlr, struct device *dev,
+		   struct sg_table *sgt, enum dma_data_direction dir)
+{
+	spi_unmap_buf_attrs(ctlr, dev, sgt, dir, 0);
+}
+
 static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 {
 	struct device *tx_dev, *rx_dev;
@@ -1124,24 +1135,30 @@ static int __spi_map_msg(struct spi_controller *ctlr, struct spi_message *msg)
 		rx_dev = ctlr->dev.parent;
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		/* The sync is done before each transfer. */
+		unsigned long attrs = DMA_ATTR_SKIP_CPU_SYNC;
+
 		if (!ctlr->can_dma(ctlr, msg->spi, xfer))
 			continue;
 
 		if (xfer->tx_buf != NULL) {
-			ret = spi_map_buf(ctlr, tx_dev, &xfer->tx_sg,
-					  (void *)xfer->tx_buf, xfer->len,
-					  DMA_TO_DEVICE);
+			ret = spi_map_buf_attrs(ctlr, tx_dev, &xfer->tx_sg,
+						(void *)xfer->tx_buf,
+						xfer->len, DMA_TO_DEVICE,
+						attrs);
 			if (ret != 0)
 				return ret;
 		}
 
 		if (xfer->rx_buf != NULL) {
-			ret = spi_map_buf(ctlr, rx_dev, &xfer->rx_sg,
-					  xfer->rx_buf, xfer->len,
-					  DMA_FROM_DEVICE);
+			ret = spi_map_buf_attrs(ctlr, rx_dev, &xfer->rx_sg,
+						xfer->rx_buf, xfer->len,
+						DMA_FROM_DEVICE, attrs);
 			if (ret != 0) {
-				spi_unmap_buf(ctlr, tx_dev, &xfer->tx_sg,
-					      DMA_TO_DEVICE);
+				spi_unmap_buf_attrs(ctlr, tx_dev,
+						&xfer->tx_sg, DMA_TO_DEVICE,
+						attrs);
+
 				return ret;
 			}
 		}
@@ -1164,17 +1181,52 @@ static int __spi_unmap_msg(struct spi_controller *ctlr, struct spi_message *msg)
 		return 0;
 
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		/* The sync has already been done after each transfer. */
+		unsigned long attrs = DMA_ATTR_SKIP_CPU_SYNC;
+
 		if (!ctlr->can_dma(ctlr, msg->spi, xfer))
 			continue;
 
-		spi_unmap_buf(ctlr, rx_dev, &xfer->rx_sg, DMA_FROM_DEVICE);
-		spi_unmap_buf(ctlr, tx_dev, &xfer->tx_sg, DMA_TO_DEVICE);
+		spi_unmap_buf_attrs(ctlr, rx_dev, &xfer->rx_sg,
+				    DMA_FROM_DEVICE, attrs);
+		spi_unmap_buf_attrs(ctlr, tx_dev, &xfer->tx_sg,
+				    DMA_TO_DEVICE, attrs);
 	}
 
 	ctlr->cur_msg_mapped = false;
 
 	return 0;
 }
+
+static void spi_dma_sync_for_device(struct spi_controller *ctlr,
+				    struct spi_transfer *xfer)
+{
+	struct device *rx_dev = ctlr->cur_rx_dma_dev;
+	struct device *tx_dev = ctlr->cur_tx_dma_dev;
+
+	if (!ctlr->cur_msg_mapped)
+		return;
+
+	if (xfer->tx_sg.orig_nents)
+		dma_sync_sgtable_for_device(tx_dev, &xfer->tx_sg, DMA_TO_DEVICE);
+	if (xfer->rx_sg.orig_nents)
+		dma_sync_sgtable_for_device(rx_dev, &xfer->rx_sg, DMA_FROM_DEVICE);
+}
+
+static void spi_dma_sync_for_cpu(struct spi_controller *ctlr,
+				 struct spi_transfer *xfer)
+{
+	struct device *rx_dev = ctlr->cur_rx_dma_dev;
+	struct device *tx_dev = ctlr->cur_tx_dma_dev;
+
+	if (!ctlr->cur_msg_mapped)
+		return;
+
+	if (xfer->rx_sg.orig_nents)
+		dma_sync_sgtable_for_cpu(rx_dev, &xfer->rx_sg, DMA_FROM_DEVICE);
+	if (xfer->tx_sg.orig_nents)
+		dma_sync_sgtable_for_cpu(tx_dev, &xfer->tx_sg, DMA_TO_DEVICE);
+}
 #else /* !CONFIG_HAS_DMA */
 static inline int __spi_map_msg(struct spi_controller *ctlr,
 				struct spi_message *msg)
@@ -1187,6 +1239,14 @@ static inline int __spi_unmap_msg(struct spi_controller *ctlr,
 {
 	return 0;
 }
+
+void spi_dma_sync_for_device(struct spi_controller *ctrl, struct spi_transfer *xfer)
+{
+}
+
+void spi_dma_sync_for_cpu(struct spi_controller *ctrl, struct spi_transfer *xfer)
+{
+}
 #endif /* !CONFIG_HAS_DMA */
 
 static inline int spi_unmap_msg(struct spi_controller *ctlr,
@@ -1444,8 +1504,11 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 			reinit_completion(&ctlr->xfer_completion);
 
 fallback_pio:
+			spi_dma_sync_for_device(ctlr, xfer);
 			ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
 			if (ret < 0) {
+				spi_dma_sync_for_cpu(ctlr, xfer);
+
 				if (ctlr->cur_msg_mapped &&
 				   (xfer->error & SPI_TRANS_FAIL_NO_START)) {
 					__spi_unmap_msg(ctlr, msg);
@@ -1468,6 +1531,8 @@ static int spi_transfer_one_message(struct spi_controller *ctlr,
 				if (ret < 0)
 					msg->status = ret;
 			}
+
+			spi_dma_sync_for_cpu(ctlr, xfer);
 		} else {
 			if (xfer->len)
 				dev_err(&msg->spi->dev,
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA
  2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
                   ` (2 preceding siblings ...)
  2022-09-16 11:39 ` [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap Vincent Whitchurch
@ 2022-09-16 11:39 ` Vincent Whitchurch
  2022-09-19  9:43   ` Krzysztof Kozlowski
  2022-09-19 14:38   ` Mark Brown
  2022-09-19 16:50 ` (subset) [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Mark Brown
  4 siblings, 2 replies; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-16 11:39 UTC (permalink / raw)
  To: broonie, krzysztof.kozlowski, andi
  Cc: kernel, Vincent Whitchurch, alim.akhtar, linux-spi, linux-kernel,
	linux-samsung-soc, linux-arm-kernel

The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
value is 65535.  Asking the driver to transfer a larger size currently
leads to the DMA transfer timing out.  Fix this by splitting the
transfer as needed.

With this, the len>64 KiB tests in spi-loopback-test pass.

(Note that len==64 KiB tests work even without this patch for some reason.
 The driver programs 0 to the COUNT_VALUE field in that case, but it's
 unclear if it's by design, since the hardware documentation doesn't say
 anything about the behaviour when COUNT_VALUE == 0, so play it safe and
 split at 65535.)

Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/spi/spi-s3c64xx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 7f346866614a..85e1d1f90109 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -701,6 +701,16 @@ static int s3c64xx_spi_prepare_message(struct spi_master *master,
 	struct spi_device *spi = msg->spi;
 	struct s3c64xx_spi_csinfo *cs = spi->controller_data;
 
+	if (master->can_dma) {
+		int ret;
+
+		/* Limited by size of PACKET_CNT.COUNT_VALUE. */
+		ret = spi_split_transfers_maxsize(master, msg, 65535,
+						  GFP_KERNEL | GFP_DMA);
+		if (ret)
+			return ret;
+	}
+
 	/* Configure feedback delay */
 	if (!cs)
 		/* No delay if not defined */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap
  2022-09-16 11:39 ` [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap Vincent Whitchurch
@ 2022-09-16 16:06   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2022-09-16 16:06 UTC (permalink / raw)
  To: Vincent Whitchurch, broonie, krzysztof.kozlowski, andi
  Cc: kbuild-all, kernel, Vincent Whitchurch, alim.akhtar, linux-spi,
	linux-kernel, linux-samsung-soc, linux-arm-kernel

Hi Vincent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-spi/for-next]
[also build test WARNING on krzk/for-next linus/master v6.0-rc5 next-20220916]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Whitchurch/spi-Fix-DMA-bugs-in-not-only-spi-s3c64xx/20220916-194234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220916/202209162337.7NbxJFXD-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/d9c81b85380dadb07e24d2f5dd6b27bacb72845a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vincent-Whitchurch/spi-Fix-DMA-bugs-in-not-only-spi-s3c64xx/20220916-194234
        git checkout d9c81b85380dadb07e24d2f5dd6b27bacb72845a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/spi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi.c:1243:6: warning: no previous prototype for 'spi_dma_sync_for_device' [-Wmissing-prototypes]
    1243 | void spi_dma_sync_for_device(struct spi_controller *ctrl, struct spi_transfer *xfer)
         |      ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/spi/spi.c:1247:6: warning: no previous prototype for 'spi_dma_sync_for_cpu' [-Wmissing-prototypes]
    1247 | void spi_dma_sync_for_cpu(struct spi_controller *ctrl, struct spi_transfer *xfer)
         |      ^~~~~~~~~~~~~~~~~~~~


vim +/spi_dma_sync_for_device +1243 drivers/spi/spi.c

  1242	
> 1243	void spi_dma_sync_for_device(struct spi_controller *ctrl, struct spi_transfer *xfer)
  1244	{
  1245	}
  1246	
> 1247	void spi_dma_sync_for_cpu(struct spi_controller *ctrl, struct spi_transfer *xfer)
  1248	{
  1249	}
  1250	#endif /* !CONFIG_HAS_DMA */
  1251	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA
  2022-09-16 11:39 ` [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA Vincent Whitchurch
@ 2022-09-19  9:43   ` Krzysztof Kozlowski
  2022-09-19 14:38   ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-19  9:43 UTC (permalink / raw)
  To: Vincent Whitchurch, broonie, andi
  Cc: kernel, alim.akhtar, linux-spi, linux-kernel, linux-samsung-soc,
	linux-arm-kernel

On 16/09/2022 13:39, Vincent Whitchurch wrote:
> The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> value is 65535.  Asking the driver to transfer a larger size currently
> leads to the DMA transfer timing out.  Fix this by splitting the
> transfer as needed.
> 
> With this, the len>64 KiB tests in spi-loopback-test pass.
> 
> (Note that len==64 KiB tests work even without this patch for some reason.
>  The driver programs 0 to the COUNT_VALUE field in that case, but it's
>  unclear if it's by design, since the hardware documentation doesn't say
>  anything about the behaviour when COUNT_VALUE == 0, so play it safe and
>  split at 65535.)
> 
> Fixes: 230d42d422e7b69 ("spi: Add s3c64xx SPI Controller driver")
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA
  2022-09-16 11:39 ` [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA Vincent Whitchurch
  2022-09-19  9:43   ` Krzysztof Kozlowski
@ 2022-09-19 14:38   ` Mark Brown
  2022-09-26 13:42     ` Vincent Whitchurch
  1 sibling, 1 reply; 12+ messages in thread
From: Mark Brown @ 2022-09-19 14:38 UTC (permalink / raw)
  To: Vincent Whitchurch
  Cc: krzysztof.kozlowski, andi, kernel, alim.akhtar, linux-spi,
	linux-kernel, linux-samsung-soc, linux-arm-kernel

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

On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote:
> The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> value is 65535.  Asking the driver to transfer a larger size currently
> leads to the DMA transfer timing out.  Fix this by splitting the
> transfer as needed.

The driver should just set max_transfer_size and let the core
handle this.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing
  2022-09-16 11:39 ` [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing Vincent Whitchurch
@ 2022-09-19 14:41   ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2022-09-19 14:41 UTC (permalink / raw)
  To: Vincent Whitchurch
  Cc: krzysztof.kozlowski, andi, kernel, alim.akhtar, linux-spi,
	linux-kernel, linux-samsung-soc, linux-arm-kernel

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

On Fri, Sep 16, 2022 at 01:39:48PM +0200, Vincent Whitchurch wrote:
> Add a test where a small and a large transfer in a message hit the same
> cache line.  This test currently fails on spi-s3c64xx on in DMA mode
> since it ends up mixing DMA and PIO without proper cache maintenance.

To make life easier with sending fixes as such fixes should come
at the start of a patch series and new features at the end.  This
avoids creating spurious dependencies.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: (subset) [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx
  2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
                   ` (3 preceding siblings ...)
  2022-09-16 11:39 ` [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA Vincent Whitchurch
@ 2022-09-19 16:50 ` Mark Brown
  4 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2022-09-19 16:50 UTC (permalink / raw)
  To: Vincent Whitchurch, andi, krzysztof.kozlowski
  Cc: linux-kernel, linux-spi, linux-arm-kernel, kernel, alim.akhtar,
	linux-samsung-soc

On Fri, 16 Sep 2022 13:39:47 +0200, Vincent Whitchurch wrote:
> This series fixes some bugs I found while running spi-loopback-test with
> spi-s3c64xx.  The first problem (which I actually noticed while trying to fix
> the second problem with transfers >64KiB) seems to be a generic issue which
> affects several drivers so I fixed it in the core.
> 
> The series has been tested on ARTPEC-8, which has a version of the IP similar
> to Exynos 7 and with 64 byte FIFOs (compatible with "tesla,fsd-spi").
> 
> [...]

Applied to

   broonie/spi.git for-next

Thanks!

[1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing
      commit: b85ad8a54e0a446b3daa7f526e4996ddb6d4373f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA
  2022-09-19 14:38   ` Mark Brown
@ 2022-09-26 13:42     ` Vincent Whitchurch
  2022-09-26 13:52       ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Vincent Whitchurch @ 2022-09-26 13:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: krzysztof.kozlowski, andi, kernel, alim.akhtar, linux-spi,
	linux-kernel, linux-samsung-soc, linux-arm-kernel

On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote:
> On Fri, Sep 16, 2022 at 01:39:51PM +0200, Vincent Whitchurch wrote:
> > The COUNT_VALUE in the PACKET_CNT register is 16-bit so the maximum
> > value is 65535.  Asking the driver to transfer a larger size currently
> > leads to the DMA transfer timing out.  Fix this by splitting the
> > transfer as needed.
> 
> The driver should just set max_transfer_size and let the core
> handle this.

Hmm, AFAICS, the core doesn't actually do anything with
max_transfer_size?  It's only used from spi-mem and a handful of other
clients via spi_max_transfer_size().

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA
  2022-09-26 13:42     ` Vincent Whitchurch
@ 2022-09-26 13:52       ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2022-09-26 13:52 UTC (permalink / raw)
  To: Vincent Whitchurch
  Cc: krzysztof.kozlowski, andi, kernel, alim.akhtar, linux-spi,
	linux-kernel, linux-samsung-soc, linux-arm-kernel

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

On Mon, Sep 26, 2022 at 03:42:30PM +0200, Vincent Whitchurch wrote:
> On Mon, Sep 19, 2022 at 03:38:29PM +0100, Mark Brown wrote:

> > The driver should just set max_transfer_size and let the core
> > handle this.

> Hmm, AFAICS, the core doesn't actually do anything with
> max_transfer_size?  It's only used from spi-mem and a handful of other
> clients via spi_max_transfer_size().

Then we should fix the core to handle it properly.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-09-26 15:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-16 11:39 [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Vincent Whitchurch
2022-09-16 11:39 ` [PATCH 1/4] spi: spi-loopback-test: Add test to trigger DMA/PIO mixing Vincent Whitchurch
2022-09-19 14:41   ` Mark Brown
2022-09-16 11:39 ` [PATCH 2/4] spi: Save current RX and TX DMA devices Vincent Whitchurch
2022-09-16 11:39 ` [PATCH 3/4] spi: Fix cache corruption due to DMA/PIO overlap Vincent Whitchurch
2022-09-16 16:06   ` kernel test robot
2022-09-16 11:39 ` [PATCH 4/4] spi: s3c64xx: Fix large transfers with DMA Vincent Whitchurch
2022-09-19  9:43   ` Krzysztof Kozlowski
2022-09-19 14:38   ` Mark Brown
2022-09-26 13:42     ` Vincent Whitchurch
2022-09-26 13:52       ` Mark Brown
2022-09-19 16:50 ` (subset) [PATCH 0/4] spi: Fix DMA bugs in (not only) spi-s3c64xx Mark Brown

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).