[PATCH v2 0/8] spi: loopback-test: improvments and sharing dump code

[PATCH v2 0/8] spi: loopback-test: improvments and sharing dump code

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>


Move spi_message dump method into SPI-core and make use of
this extended version of the dumping code in loopback-test.

This requires also separation of some content verification
from the dump code itself.

Also it now handles module-parameters differently
(some renaming to make it clearer what they are for)
and adds SPI_LOOP support.

Changelog:
V1->V2: * changes recommended in review by Andy Shevchenko
        * better separation to smaller patches
	* reintroduction of check for RX guard-pattern
	  lost in V1
	* rename module parameters, variables and methods

Martin Sperl (8):
  spi: loopback-test: write rx pattern also when running without tx_buf
  spi: loopback-test: rename method spi_test_fill_tx to
    spi_test_fill_pattern
  spi: core: add spi_message_dump and spi_transfer_dump
  spi: loopback-test: move to use spi_message_dump_data
  spi: loopback-test: spi_check_rx_ranges can get always done
  spi: loopback-test: improve granularity of dump_messages module
    parameter
  spi: loopback-test: change module parameter name to
    have_external_loopback
  spi: loopback-test: added support for HW-loopback mode

 drivers/spi/spi-loopback-test.c |  216 +++++++++++++++++++++------------------
 drivers/spi/spi.c               |  169 ++++++++++++++++++++++++++++++
 include/linux/spi/spi.h         |   24 +++++
 3 files changed, 308 insertions(+), 101 deletions(-)

--
1.7.10.4

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

[PATCH v2 2/8] spi: loopback-test: write rx pattern also when running without tx_buf

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Currently the rx_buf does not get set with the
SPI_TEST_PATTERN_UNWRITTEN when tx_buf == NULL in the transfer.

Reorder code so that it gets done also under this specific condition.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 7f497ac..8af2e40 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -591,6 +591,10 @@ static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
 
 	/* fill all transfers with the pattern requested */
 	for (i = 0; i < test->transfer_count; i++) {
+		/* fill rx_buf with SPI_TEST_PATTERN_UNWRITTEN */
+		if (xfers[i].rx_buf)
+			memset(xfers[i].rx_buf, SPI_TEST_PATTERN_UNWRITTEN,
+			       xfers[i].len);
 		/* if tx_buf is NULL then skip */
 		tx_buf = (u8 *)xfers[i].tx_buf;
 		if (!tx_buf)
@@ -648,10 +652,6 @@ static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
 				return -EINVAL;
 			}
 		}
-		/* fill rx_buf with SPI_TEST_PATTERN_UNWRITTEN */
-		if (xfers[i].rx_buf)
-			memset(xfers[i].rx_buf, SPI_TEST_PATTERN_UNWRITTEN,
-			       xfers[i].len);
 	}
 
 	return 0;
-- 
1.7.10.4

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

[PATCH v2 3/8] spi: loopback-test: rename method spi_test_fill_tx to spi_test_fill_pattern

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Rename method spi_test_fill_tx to spi_test_fill_pattern
to better describe what it does.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 8af2e40..81fa906 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -574,7 +574,8 @@ static int spi_test_translate(struct spi_device *spi,
 	return -EINVAL;
 }
 
-static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
+static int spi_test_fill_pattern(struct spi_device *spi,
+				 struct spi_test *test)
 {
 	struct spi_transfer *xfers = test->transfers;
 	u8 *tx_buf;
@@ -691,8 +692,8 @@ static int _spi_test_run_iter(struct spi_device *spi,
 		spi_message_add_tail(x, msg);
 	}
 
-	/* fill in the transfer data */
-	ret = spi_test_fill_tx(spi, test);
+	/* fill in the transfer buffers with pattern */
+	ret = spi_test_fill_pattern(spi, test);
 	if (ret)
 		return ret;
 
-- 
1.7.10.4

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

[PATCH v2 1/8] spi: core: add spi_message_dump and spi_transfer_dump

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

This implements a means to dump a spi_message or spi_transfer.

spi_loop_back_test requires a means to report on failed transfers
(including payload data), so it makes use of this.

Such a functionality can also be helpful during development of
other drivers, so it has been exposed as a general facility.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi.c       |  169 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h |   24 +++++++
 2 files changed, 193 insertions(+)

Changelog:
V1->V2: * changes recommended by Andy Shevchenko
        * slight reorganisation of code - specifically remove
	  hex_dump_to_buffer and use %*ph instead
	* removal of one helper function and renaming another
	* change in method parameters to allow changing the amount
	  of lines to dump at the head independently from tail

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9964835..bf623db 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -31,6 +31,7 @@
 #include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_domain.h>
+#include <linux/printk.h>
 #include <linux/export.h>
 #include <linux/sched/rt.h>
 #include <linux/delay.h>
@@ -718,6 +719,174 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n)
 	return 0;
 }

+const unsigned spi_dump_bytes_per_line = 16;
+static void __spi_transfer_dump_chunk(struct spi_device *spi, char *prefix,
+				      const void *ptr, size_t start,
+				      size_t len)
+{
+	const size_t spi_dump_bytes_per_line = 16;
+	char hexdata[spi_dump_bytes_per_line * 3];
+	size_t i, l;
+
+	for (i = start; i < len; i += spi_dump_bytes_per_line) {
+		/* format the next 16 bytes */
+		snprintf(hexdata, sizeof(hexdata), "%16ph", ptr + i);
+
+		/* truncate the unnecessarily printed bytes */
+		l = min_t(size_t, len - i, spi_dump_bytes_per_line) * 3 - 1;
+		hexdata[l] = 0;
+
+		/* and print data including address and offset */
+		dev_info(&spi->dev, "%soff=0x%.5zx ptr=0x%pK: %s\n",
+			 prefix, i, ptr + i, hexdata);
+	}
+}
+
+static void spi_transfer_dump_buffer(struct spi_device *spi, char *prefix,
+				     const void *ptr, size_t len,
+				     size_t head_len, size_t tail_len)
+{
+	size_t tail_start;
+
+	/* dump head if requested */
+	if (head_len) {
+		/* calculate effective head_len - aligning if necessary */
+		head_len = (len <= head_len) ?
+			   len :
+			   roundup(head_len, spi_dump_bytes_per_line);
+
+		/* dump the head */
+		__spi_transfer_dump_chunk(spi, prefix, ptr, 0, head_len);
+
+		/* if we dumped everything return immediately */
+		if (len == head_len)
+			return;
+	}
+
+	/* return if no tail_len is requested */
+	if (!tail_len)
+		return;
+
+	/* calculate real tail start offset aligning it */
+	tail_start = (tail_len >= len) ?
+		     head_len :
+		     rounddown(len - tail_len, spi_dump_bytes_per_line);
+
+	/* special handling needed if we have been dumping head */
+	if (head_len) {
+		if (tail_start > head_len)
+			/* we are not overlapping */
+			dev_info(&spi->dev,
+				 "%struncated - continuing at offset %04x\n",
+				 prefix, tail_start);
+		else
+			/* we are overlapping, so continue at head_len */
+			tail_start = head_len;
+	}
+
+	/* dump the tail */
+	__spi_transfer_dump_chunk(spi, prefix, ptr, tail_start, len);
+}
+
+/**
+ * spi_transfer_dump - dump all the essential information
+ *                     of a @spi_transfer, when dump_size is set,
+ *                     then hex-dump that many bytes of data
+ * @spi:       @spi_device for which to dump this (dev_info)
+ * @msg:       @spi_message to which xfer belongs
+ * @xfer:      @spi_transfer to dump
+ * @head_len: bytes to dump from the start of the buffers
+ *            (rounded up to multiple of 16)
+ * @tail_len: bytes to dump from the end of the buffers
+ *            (rounded up so that tail dump starts 16 byte aligned)
+ */
+void spi_transfer_dump(struct spi_device *spi,
+		       struct spi_message *msg,
+		       struct spi_transfer *xfer,
+		       size_t head_len, size_t tail_len)
+{
+	struct device *dev = &spi->dev;
+
+	dev_info(dev, "  spi_transfer@%pK\n", xfer);
+	dev_info(dev, "    speed_hz:    %u\n", xfer->speed_hz);
+	dev_info(dev, "    len:         %u\n", xfer->len);
+	dev_info(dev, "    tx_nbits:    %u\n", xfer->tx_nbits);
+	dev_info(dev, "    rx_nbits:    %u\n", xfer->rx_nbits);
+	dev_info(dev, "    bits/word:   %u\n", xfer->bits_per_word);
+	if (xfer->delay_usecs)
+		dev_info(dev, "    delay_usecs: %u\n",
+			 xfer->delay_usecs);
+	if (xfer->cs_change)
+		dev_info(dev, "    cs_change\n");
+	if (xfer->tx_buf) {
+		dev_info(dev, "    tx_buf:      %pK\n", xfer->tx_buf);
+		if (xfer->tx_dma)
+			dev_info(dev, "    tx_dma:      %pad\n",
+				 &xfer->tx_dma);
+		if (head_len | tail_len)
+			spi_transfer_dump_buffer(spi, "\t",
+						 xfer->tx_buf, xfer->len,
+						 head_len, tail_len);
+	}
+	if (xfer->rx_buf) {
+		dev_info(dev, "    rx_buf:      %pK\n", xfer->rx_buf);
+		if (xfer->rx_dma)
+			dev_info(dev, "    rx_dma:      %pad\n",
+				 &xfer->rx_dma);
+		if (head_len | tail_len)
+			spi_transfer_dump_buffer(spi, "\t",
+						 xfer->rx_buf, xfer->len,
+						 head_len, tail_len);
+	}
+}
+EXPORT_SYMBOL_GPL(spi_transfer_dump);
+
+/**
+ * spi_message_dump_custom - dump a spi message with ability to have
+ *                           a custom dump method per transfer
+ * @spi:       @spi_device for which to dump this (dev_info)
+ * @msg:       @spi_message to dump
+ * @head_len: bytes to dump from the start of the buffers
+ *            (rounded up to multiple of 16)
+ * @tail_len: bytes to dump from the end of the buffers
+ *            (rounded up so that tail dump starts 16 byte aligned)
+ * @custom:    custom dump code to execute per transfer
+ * @context:   context to pass to the custom dump code
+ *
+ * Uses dev_info() to dump the lines.
+ */
+void spi_message_dump_custom(struct spi_device *spi,
+			     struct spi_message *msg,
+			     size_t head_len, size_t tail_len,
+			     spi_transfer_dump_custom_t custom,
+			     void *context)
+{
+	struct device *dev = &spi->dev;
+	struct spi_transfer *xfer;
+
+	/* dump the message */
+	dev_info(dev, "spi_msg@%pK\n", msg);
+	if (msg->status)
+		dev_info(dev, "  status:         %d\n", msg->status);
+	dev_info(dev, "  frame_length:   %zu\n", msg->frame_length);
+	dev_info(dev, "  actual_length:  %zu\n", msg->actual_length);
+	if (msg->complete)
+		dev_info(dev, "  complete:       %pF\n", msg->complete);
+	if (msg->context)
+		dev_info(dev, "  context:        %pF\n", msg->context);
+	if (msg->is_dma_mapped)
+		dev_info(dev, "  is_dma_mapped\n");
+	dev_info(dev, "  transfers-head: %pK\n", &msg->transfers);
+
+	/* dump transfers themselves */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		spi_transfer_dump(spi, msg, xfer, head_len, tail_len);
+		if (custom)
+			custom(spi, msg, xfer, context);
+	}
+}
+EXPORT_SYMBOL_GPL(spi_message_dump_custom);
+
 /*-------------------------------------------------------------------------*/

 static void spi_set_cs(struct spi_device *spi, bool enable)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f055a47..13f8f06 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -897,6 +897,30 @@ extern int spi_setup(struct spi_device *spi);
 extern int spi_async(struct spi_device *spi, struct spi_message *message);
 extern int spi_async_locked(struct spi_device *spi,
 			    struct spi_message *message);
+/*---------------------------------------------------------------------------*/
+
+extern void spi_transfer_dump(struct spi_device *spi,
+			      struct spi_message *msg,
+			      struct spi_transfer *xfer,
+			      size_t dump_head, size_t dump_tail);
+
+typedef void (*spi_transfer_dump_custom_t)(struct spi_device *spi,
+					   struct spi_message *msg,
+					   struct spi_transfer *xfer,
+					   void *context);
+
+extern void spi_message_dump_custom(struct spi_device *spi,
+				    struct spi_message *msg,
+				    size_t dump_head, size_t dump_tail,
+				    spi_transfer_dump_custom_t custom,
+				    void *context);
+
+static inline void spi_message_dump(struct spi_device *spi,
+				    struct spi_message *msg,
+				    size_t dump_head, size_t dump_tail)
+{
+	spi_message_dump_custom(spi, msg, dump_head, dump_tail, NULL, NULL);
+}

 /*---------------------------------------------------------------------------*/

--
1.7.10.4

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

[PATCH v2 4/8] spi: loopback-test: move to use spi_message_dump_data

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Move loopback-test to use the core spi_message_dump_data method.

A piece of functionality (testing for UNWRITTEN guard pattern)
that was inside spi_test_dump_transfer had to get moved out
to keep the functionality working.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |  116 +++++++++++++--------------------------
 1 file changed, 39 insertions(+), 77 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 81fa906..75cea07 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -331,74 +331,6 @@ MODULE_LICENSE("GPL");
 /* we allocate one page more, to allow for offsets */
 #define SPI_TEST_MAX_SIZE_PLUS (SPI_TEST_MAX_SIZE + PAGE_SIZE)
 
-static void spi_test_print_hex_dump(char *pre, const void *ptr, size_t len)
-{
-	/* limit the hex_dump */
-	if (len < 1024) {
-		print_hex_dump(KERN_INFO, pre,
-			       DUMP_PREFIX_OFFSET, 16, 1,
-			       ptr, len, 0);
-		return;
-	}
-	/* print head */
-	print_hex_dump(KERN_INFO, pre,
-		       DUMP_PREFIX_OFFSET, 16, 1,
-		       ptr, 512, 0);
-	/* print tail */
-	pr_info("%s truncated - continuing at offset %04zx\n",
-		pre, len - 512);
-	print_hex_dump(KERN_INFO, pre,
-		       DUMP_PREFIX_OFFSET, 16, 1,
-		       ptr + (len - 512), 512, 0);
-}
-
-static void spi_test_dump_message(struct spi_device *spi,
-				  struct spi_message *msg,
-				  bool dump_data)
-{
-	struct spi_transfer *xfer;
-	int i;
-	u8 b;
-
-	dev_info(&spi->dev, "  spi_msg@%pK\n", msg);
-	if (msg->status)
-		dev_info(&spi->dev, "    status:        %i\n",
-			 msg->status);
-	dev_info(&spi->dev, "    frame_length:  %i\n",
-		 msg->frame_length);
-	dev_info(&spi->dev, "    actual_length: %i\n",
-		 msg->actual_length);
-
-	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
-		dev_info(&spi->dev, "    spi_transfer@%pK\n", xfer);
-		dev_info(&spi->dev, "      len:    %i\n", xfer->len);
-		dev_info(&spi->dev, "      tx_buf: %pK\n", xfer->tx_buf);
-		if (dump_data && xfer->tx_buf)
-			spi_test_print_hex_dump("          TX: ",
-						xfer->tx_buf,
-						xfer->len);
-
-		dev_info(&spi->dev, "      rx_buf: %pK\n", xfer->rx_buf);
-		if (dump_data && xfer->rx_buf)
-			spi_test_print_hex_dump("          RX: ",
-						xfer->rx_buf,
-						xfer->len);
-		/* check for unwritten test pattern on rx_buf */
-		if (xfer->rx_buf) {
-			for (i = 0 ; i < xfer->len ; i++) {
-				b = ((u8 *)xfer->rx_buf)[xfer->len - 1 - i];
-				if (b != SPI_TEST_PATTERN_UNWRITTEN)
-					break;
-			}
-			if (i)
-				dev_info(&spi->dev,
-					 "      rx_buf filled with %02x starts at offset: %i\n",
-					 SPI_TEST_PATTERN_UNWRITTEN,
-					 xfer->len - i);
-		}
-	}
-}
-
 struct rx_ranges {
 	struct list_head list;
 	u8 *start;
@@ -423,17 +355,37 @@ static int spi_check_rx_ranges(struct spi_device *spi,
 {
 	struct spi_transfer *xfer;
 	struct rx_ranges ranges[SPI_TEST_MAX_TRANSFERS], *r;
-	int i = 0;
+	size_t i = 0;
 	LIST_HEAD(ranges_list);
-	u8 *addr;
+	u8 *addr, b;
 	int ret = 0;
 
-	/* loop over all transfers to fill in the rx_ranges */
+	/* if there is no rx, then no check is needed */
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
-		/* if there is no rx, then no check is needed */
 		if (!xfer->rx_buf)
 			continue;
-		/* fill in the rx_range */
+		/* check the unwritten pattern inside the transfer*/
+		for (i = 0; i < xfer->len ; i++) {
+			b = ((u8 *)xfer->rx_buf)[xfer->len - 1 - i];
+			if (b != SPI_TEST_PATTERN_UNWRITTEN)
+				break;
+		}
+		/* if there is a match then return with an error
+		 * note that the fill pattern makes sure that the last
+		 * TX byte per transfer is never the UNWRITTEN test pattern
+		 */
+		if (i) {
+			dev_err(&spi->dev,
+				"      rx_buf filled with %02x starts at offset: %i\n",
+				SPI_TEST_PATTERN_UNWRITTEN,
+				xfer->len - i);
+			return -EINVAL;
+		}
+	}
+
+	/* loop over all transfers to fill in the rx_ranges */
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		/* fill in the rx_range for the check below*/
 		if (RANGE_CHECK(xfer->rx_buf, xfer->len,
 				rx, SPI_TEST_MAX_SIZE_PLUS)) {
 			ranges[i].start = xfer->rx_buf;
@@ -653,6 +605,12 @@ static int spi_test_fill_pattern(struct spi_device *spi,
 				return -EINVAL;
 			}
 		}
+		/* make sure that the last byte in TX is
+		 * not the UNWRITTEN pattern
+		 */
+		tx_buf--;
+		if (*tx_buf == SPI_TEST_PATTERN_UNWRITTEN)
+			*tx_buf = 0;
 	}
 
 	return 0;
@@ -812,6 +770,7 @@ static int spi_test_run_iter(struct spi_device *spi,
 int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 			 void *tx, void *rx)
 {
+	const size_t dump_size = 512;
 	struct spi_message *msg = &test->msg;
 	int ret = 0;
 	int i;
@@ -820,7 +779,7 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 	if (!simulate_only) {
 		/* dump the complete message before and after the transfer */
 		if (dump_messages == 3)
-			spi_test_dump_message(spi, msg, true);
+			spi_message_dump(spi, msg, dump_size, dump_size);
 
 		/* run spi message */
 		ret = spi_sync(spi, msg);
@@ -855,9 +814,12 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 
 	/* if requested or on error dump message (including data) */
 exit:
-	if (dump_messages || ret)
-		spi_test_dump_message(spi, msg,
-				      (dump_messages >= 2) || (ret));
+	if (dump_messages || ret) {
+		if ((dump_messages >= 2) || (ret))
+			spi_message_dump(spi, msg, dump_size, dump_size);
+		else
+			spi_message_dump(spi, msg, 0, 0);
+	}
 
 	return ret;
 }
-- 
1.7.10.4

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

[PATCH v2 5/8] spi: loopback-test: spi_check_rx_ranges can get always done

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

The spi_check_rx_ranges can always get executed independent of
if we have a real loopback situation.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 75cea07..c26ffa1 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -441,7 +441,18 @@ static int spi_test_check_loopback_result(struct spi_device *spi,
 	struct spi_transfer *xfer;
 	u8 rxb, txb;
 	size_t i;
+	int ret;
+
+	/* checks rx_buffer pattern are valid with loopback or without */
+	ret = spi_check_rx_ranges(spi, msg, rx);
+	if (ret)
+		return ret;
 
+	/* if we run without loopback, then return now */
+	if (!loopback)
+		return 0;
+
+	/* if applicable to transfer check that rx_buf is equal to tx_buf */
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		/* if there is no rx, then no check is needed */
 		if (!xfer->rx_buf)
@@ -473,7 +484,7 @@ static int spi_test_check_loopback_result(struct spi_device *spi,
 		}
 	}
 
-	return spi_check_rx_ranges(spi, msg, rx);
+	return 0;
 
 mismatch_error:
 	dev_err(&spi->dev,
@@ -806,10 +817,8 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 			goto exit;
 		}
 
-		/* run rx-tests when in loopback mode */
-		if (loopback)
-			ret = spi_test_check_loopback_result(spi, msg,
-							     tx, rx);
+		/* run rx-buffer tests */
+		ret = spi_test_check_loopback_result(spi, msg, tx, rx);
 	}
 
 	/* if requested or on error dump message (including data) */
-- 
1.7.10.4

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

[PATCH v2 6/8] spi: loopback-test: improve granularity of dump_messages module parameter

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Make dump_messages module parameter a bitmask to allow for better
granularity when dumping messages.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |   39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index c26ffa1..930e7c2 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -36,11 +36,17 @@ MODULE_PARM_DESC(simulate_only, "if not 0 do not execute the spi message");
 
 /* dump spi messages */
 int dump_messages;
+#define SPI_DUMP_MESSAGE_AFTER		BIT(0)
+#define SPI_DUMP_MESSAGE_BEFORE		BIT(1)
+#define SPI_DUMP_MESSAGE_DATA_AFTER	BIT(2)
+#define SPI_DUMP_MESSAGE_DATA_BEFORE	BIT(3)
 module_param(dump_messages, int, 0);
-MODULE_PARM_DESC(dump_message,
-		 "=1 dump the basic spi_message_structure, " \
-		 "=2 dump the spi_message_structure including data, " \
-		 "=3 dump the spi_message structure before and after execution");
+MODULE_PARM_DESC(dump_messages,
+		 "BIT(0) - dump the basic spi_message_structure after processing, "
+		 "BIT(1) - dump the basic spi_message_structure before processing, "
+		 "BIT(2) - also dump the spi_message data after processing, "
+		 "BIT(3) - also dump the spi_message data before processing");
+
 /* the device is jumpered for loopback - enabling some rx_buf tests */
 int loopback;
 module_param(loopback, int, 0);
@@ -789,8 +795,14 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 	/* only if we do not simulate */
 	if (!simulate_only) {
 		/* dump the complete message before and after the transfer */
-		if (dump_messages == 3)
-			spi_message_dump(spi, msg, dump_size, dump_size);
+		if (dump_messages & SPI_DUMP_MESSAGE_BEFORE) {
+			if (dump_messages & SPI_DUMP_MESSAGE_DATA_BEFORE)
+				spi_message_dump(spi, msg,
+						 dump_size, dump_size);
+			else
+				spi_message_dump(spi, msg, 0, 0);
+		}
+
 
 		/* run spi message */
 		ret = spi_sync(spi, msg);
@@ -823,11 +835,16 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 
 	/* if requested or on error dump message (including data) */
 exit:
-	if (dump_messages || ret) {
-		if ((dump_messages >= 2) || (ret))
-			spi_message_dump(spi, msg, dump_size, dump_size);
-		else
-			spi_message_dump(spi, msg, 0, 0);
+	if (ret) {
+		spi_message_dump(spi, msg, dump_size, dump_size);
+	} else {
+		if (dump_messages & SPI_DUMP_MESSAGE_AFTER) {
+			if (dump_messages & SPI_DUMP_MESSAGE_DATA_AFTER)
+				spi_message_dump(spi, msg,
+						 dump_size, dump_size);
+			else
+				spi_message_dump(spi, msg, 0, 0);
+		}
 	}
 
 	return ret;
-- 
1.7.10.4

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

[PATCH v2 7/8] spi: loopback-test: change module parameter name to have_external_loopback

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

Change the "loopback" module parameter to "have_external_loopback".

Also change variable name "loopback" to "check_rx_buf", which better
describes the meaning of the variable - have_external_loopback maps
to that, as when we have an external_loopback situation, then we can
check the rx_buf if it contains valid data.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 930e7c2..b42d1a8 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -47,12 +47,12 @@ MODULE_PARM_DESC(dump_messages,
 		 "BIT(2) - also dump the spi_message data after processing, "
 		 "BIT(3) - also dump the spi_message data before processing");

-/* the device is jumpered for loopback - enabling some rx_buf tests */
-int loopback;
-module_param(loopback, int, 0);
-MODULE_PARM_DESC(loopback,
-		 "if set enable loopback mode, where the rx_buf "	\
-		 "is checked to match tx_buf after the spi_message "	\
+/* the device is jumpered for external loopback - enabling some rx_buf tests */
+int check_rx_buf;
+module_param_named(have_external_loopback, check_rx_buf, int, 0);
+MODULE_PARM_DESC(have_external_loopback,
+		 "if set enable external loopback mode, where the rx_buf "
+		 "is checked to match tx_buf after the spi_message "
 		 "is executed");

 /* run only a specific test */
@@ -455,7 +455,7 @@ static int spi_test_check_loopback_result(struct spi_device *spi,
 		return ret;

 	/* if we run without loopback, then return now */
-	if (!loopback)
+	if (!check_rx_buf)
 		return 0;

 	/* if applicable to transfer check that rx_buf is equal to tx_buf */
--
1.7.10.4

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

[PATCH v2 8/8] spi: loopback-test: added support for HW-loopback mode

[kernel-TqfNSX0MhmxHKSADF0wUEw]

From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

If the module parameter "loopback" for testing RX to match TX
is not set and the spi_master supports SPI_LOOP then
SPI_LOOP as well as RX-data testing will get enabled.

When the "loopback" module parameter is set
then the SPI_LOOP support in spi_master will not get enabled,
which means that MOSI needs to get connected to MISO by some
other means (possibly via a simple jumper connection
or a SN74AHCT125 connecting MOSI/MISO)

Suggested-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index b42d1a8..4e4f368 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -288,8 +288,34 @@ static struct spi_test spi_tests[] = {
 static int spi_loopback_test_probe(struct spi_device *spi)
 {
 	int ret;
+	char *msg;
 
-	dev_info(&spi->dev, "Executing spi-loopback-tests\n");
+	/*
+	 * Enable HW-loopback automatically if the master supports it
+	 * and we have the have_rx_buf unset (set means that we have
+	 * got an external loopback enabled, so no need to use SPI_LOOP)
+	 */
+	if (!check_rx_buf && (spi->master->mode_bits & SPI_LOOP)) {
+		spi->mode |= SPI_LOOP;
+		ret = spi_setup(spi);
+		if (ret) {
+			dev_err(&spi->dev,
+				"spi_setup failed to enable loopback: %i\n",
+				ret);
+			return ret;
+		}
+
+		/* force checking rx_buf for valid (loopback) data */
+		check_rx_buf = 1;
+
+		msg = "with SPI_LOOP support";
+	} else {
+		msg = (check_rx_buf) ?
+		      "with external loopback" :
+		      "without any loopback - rx_buf content is not checked";
+	}
+
+	dev_info(&spi->dev, "Executing spi-loopback-tests %s\n", msg);
 
 	ret = spi_test_run_tests(spi, spi_tests);
 
-- 
1.7.10.4
--
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 v2 1/8] spi: core: add spi_message_dump and spi_transfer_dump

[Andy Shevchenko]

On Mon, Dec 21, 2015 at 1:52 PM,  <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org> wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> This implements a means to dump a spi_message or spi_transfer.
>
> spi_loop_back_test requires a means to report on failed transfers
> (including payload data), so it makes use of this.
>
> Such a functionality can also be helpful during development of
> other drivers, so it has been exposed as a general facility.
>
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
>  drivers/spi/spi.c       |  169 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/spi/spi.h |   24 +++++++
>  2 files changed, 193 insertions(+)
>
> Changelog:
> V1->V2: * changes recommended by Andy Shevchenko
>         * slight reorganisation of code - specifically remove
>           hex_dump_to_buffer and use %*ph instead
>         * removal of one helper function and renaming another
>         * change in method parameters to allow changing the amount
>           of lines to dump at the head independently from tail
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 9964835..bf623db 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -31,6 +31,7 @@
>  #include <linux/of_gpio.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pm_domain.h>
> +#include <linux/printk.h>
>  #include <linux/export.h>
>  #include <linux/sched/rt.h>
>  #include <linux/delay.h>
> @@ -718,6 +719,174 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n)
>         return 0;
>  }
>
> +const unsigned spi_dump_bytes_per_line = 16;
> +static void __spi_transfer_dump_chunk(struct spi_device *spi, char *prefix,
> +                                     const void *ptr, size_t start,
> +                                     size_t len)
> +{
> +       const size_t spi_dump_bytes_per_line = 16;
> +       char hexdata[spi_dump_bytes_per_line * 3];
> +       size_t i, l;
> +
> +       for (i = start; i < len; i += spi_dump_bytes_per_line) {
> +               /* format the next 16 bytes */
> +               snprintf(hexdata, sizeof(hexdata), "%16ph", ptr + i);
> +
> +               /* truncate the unnecessarily printed bytes */

Again, you don't need the intermediate buffer along with this all
stuff. Just use %*ph properly (hint: star is put here and elsewhere on
purpose).

drivers/hid/i2c-hid/i2c-hid.c:187:      i2c_hid_dbg(ihid, "%s:
cmd=%*ph\n", __func__, length, cmd->data);

> +               l = min_t(size_t, len - i, spi_dump_bytes_per_line) * 3 - 1;
> +               hexdata[l] = 0;
> +
> +               /* and print data including address and offset */
> +               dev_info(&spi->dev, "%soff=0x%.5zx ptr=0x%pK: %s\n",
> +                        prefix, i, ptr + i, hexdata);
> +       }
> +}
> +
> +static void spi_transfer_dump_buffer(struct spi_device *spi, char *prefix,
> +                                    const void *ptr, size_t len,
> +                                    size_t head_len, size_t tail_len)
> +{

The function body looks too complicated. Can you describe what
function does in different cases?

> +       size_t tail_start;
> +
> +       /* dump head if requested */
> +       if (head_len) {
> +               /* calculate effective head_len - aligning if necessary */
> +               head_len = (len <= head_len) ?
> +                          len :
> +                          roundup(head_len, spi_dump_bytes_per_line);
> +
> +               /* dump the head */
> +               __spi_transfer_dump_chunk(spi, prefix, ptr, 0, head_len);
> +
> +               /* if we dumped everything return immediately */
> +               if (len == head_len)
> +                       return;
> +       }
> +
> +       /* return if no tail_len is requested */
> +       if (!tail_len)
> +               return;
> +
> +       /* calculate real tail start offset aligning it */
> +       tail_start = (tail_len >= len) ?
> +                    head_len :
> +                    rounddown(len - tail_len, spi_dump_bytes_per_line);
> +
> +       /* special handling needed if we have been dumping head */
> +       if (head_len) {
> +               if (tail_start > head_len)
> +                       /* we are not overlapping */
> +                       dev_info(&spi->dev,
> +                                "%struncated - continuing at offset %04x\n",
> +                                prefix, tail_start);
> +               else
> +                       /* we are overlapping, so continue at head_len */
> +                       tail_start = head_len;
> +       }
> +
> +       /* dump the tail */
> +       __spi_transfer_dump_chunk(spi, prefix, ptr, tail_start, len);
> +}
> +
> +/**
> + * spi_transfer_dump - dump all the essential information
> + *                     of a @spi_transfer, when dump_size is set,
> + *                     then hex-dump that many bytes of data
> + * @spi:       @spi_device for which to dump this (dev_info)
> + * @msg:       @spi_message to which xfer belongs
> + * @xfer:      @spi_transfer to dump
> + * @head_len: bytes to dump from the start of the buffers
> + *            (rounded up to multiple of 16)
> + * @tail_len: bytes to dump from the end of the buffers
> + *            (rounded up so that tail dump starts 16 byte aligned)
> + */
> +void spi_transfer_dump(struct spi_device *spi,
> +                      struct spi_message *msg,
> +                      struct spi_transfer *xfer,
> +                      size_t head_len, size_t tail_len)
> +{
> +       struct device *dev = &spi->dev;
> +
> +       dev_info(dev, "  spi_transfer@%pK\n", xfer);
> +       dev_info(dev, "    speed_hz:    %u\n", xfer->speed_hz);
> +       dev_info(dev, "    len:         %u\n", xfer->len);
> +       dev_info(dev, "    tx_nbits:    %u\n", xfer->tx_nbits);
> +       dev_info(dev, "    rx_nbits:    %u\n", xfer->rx_nbits);
> +       dev_info(dev, "    bits/word:   %u\n", xfer->bits_per_word);
> +       if (xfer->delay_usecs)
> +               dev_info(dev, "    delay_usecs: %u\n",
> +                        xfer->delay_usecs);
> +       if (xfer->cs_change)
> +               dev_info(dev, "    cs_change\n");
> +       if (xfer->tx_buf) {
> +               dev_info(dev, "    tx_buf:      %pK\n", xfer->tx_buf);
> +               if (xfer->tx_dma)
> +                       dev_info(dev, "    tx_dma:      %pad\n",
> +                                &xfer->tx_dma);
> +               if (head_len | tail_len)

I think || looks a bit more logical here and below.

> +                       spi_transfer_dump_buffer(spi, "\t",
> +                                                xfer->tx_buf, xfer->len,
> +                                                head_len, tail_len);
> +       }
> +       if (xfer->rx_buf) {
> +               dev_info(dev, "    rx_buf:      %pK\n", xfer->rx_buf);
> +               if (xfer->rx_dma)
> +                       dev_info(dev, "    rx_dma:      %pad\n",
> +                                &xfer->rx_dma);
> +               if (head_len | tail_len)

> +                       spi_transfer_dump_buffer(spi, "\t",
> +                                                xfer->rx_buf, xfer->len,
> +                                                head_len, tail_len);

Is the DMA buffer is in coherent memory? Otherwise you have to call
dma_sync for it before reading on CPU side.

> +       }
> +}
> +EXPORT_SYMBOL_GPL(spi_transfer_dump);
> +
> +/**
> + * spi_message_dump_custom - dump a spi message with ability to have
> + *                           a custom dump method per transfer
> + * @spi:       @spi_device for which to dump this (dev_info)
> + * @msg:       @spi_message to dump
> + * @head_len: bytes to dump from the start of the buffers
> + *            (rounded up to multiple of 16)
> + * @tail_len: bytes to dump from the end of the buffers
> + *            (rounded up so that tail dump starts 16 byte aligned)
> + * @custom:    custom dump code to execute per transfer
> + * @context:   context to pass to the custom dump code
> + *
> + * Uses dev_info() to dump the lines.
> + */
> +void spi_message_dump_custom(struct spi_device *spi,
> +                            struct spi_message *msg,
> +                            size_t head_len, size_t tail_len,
> +                            spi_transfer_dump_custom_t custom,
> +                            void *context)
> +{
> +       struct device *dev = &spi->dev;
> +       struct spi_transfer *xfer;
> +
> +       /* dump the message */
> +       dev_info(dev, "spi_msg@%pK\n", msg);
> +       if (msg->status)
> +               dev_info(dev, "  status:         %d\n", msg->status);
> +       dev_info(dev, "  frame_length:   %zu\n", msg->frame_length);
> +       dev_info(dev, "  actual_length:  %zu\n", msg->actual_length);
> +       if (msg->complete)
> +               dev_info(dev, "  complete:       %pF\n", msg->complete);
> +       if (msg->context)
> +               dev_info(dev, "  context:        %pF\n", msg->context);
> +       if (msg->is_dma_mapped)
> +               dev_info(dev, "  is_dma_mapped\n");
> +       dev_info(dev, "  transfers-head: %pK\n", &msg->transfers);
> +
> +       /* dump transfers themselves */
> +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> +               spi_transfer_dump(spi, msg, xfer, head_len, tail_len);
> +               if (custom)
> +                       custom(spi, msg, xfer, context);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(spi_message_dump_custom);
> +
>  /*-------------------------------------------------------------------------*/
>
>  static void spi_set_cs(struct spi_device *spi, bool enable)
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index f055a47..13f8f06 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -897,6 +897,30 @@ extern int spi_setup(struct spi_device *spi);
>  extern int spi_async(struct spi_device *spi, struct spi_message *message);
>  extern int spi_async_locked(struct spi_device *spi,
>                             struct spi_message *message);
> +/*---------------------------------------------------------------------------*/
> +
> +extern void spi_transfer_dump(struct spi_device *spi,
> +                             struct spi_message *msg,
> +                             struct spi_transfer *xfer,
> +                             size_t dump_head, size_t dump_tail);
> +
> +typedef void (*spi_transfer_dump_custom_t)(struct spi_device *spi,
> +                                          struct spi_message *msg,
> +                                          struct spi_transfer *xfer,
> +                                          void *context);
> +
> +extern void spi_message_dump_custom(struct spi_device *spi,
> +                                   struct spi_message *msg,
> +                                   size_t dump_head, size_t dump_tail,
> +                                   spi_transfer_dump_custom_t custom,
> +                                   void *context);
> +
> +static inline void spi_message_dump(struct spi_device *spi,
> +                                   struct spi_message *msg,
> +                                   size_t dump_head, size_t dump_tail)
> +{
> +       spi_message_dump_custom(spi, msg, dump_head, dump_tail, NULL, NULL);
> +}
>
>  /*---------------------------------------------------------------------------*/
>
> --
> 1.7.10.4
>
> --
> 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



-- 
With Best Regards,
Andy Shevchenko
--
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 v2 1/8] spi: core: add spi_message_dump and spi_transfer_dump

[Martin Sperl]

> On 21.12.2015, at 13:42, Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> +               /* format the next 16 bytes */
>> +               snprintf(hexdata, sizeof(hexdata), "%16ph", ptr + i);
>> +
>> +               /* truncate the unnecessarily printed bytes */
> 
> Again, you don't need the intermediate buffer along with this all
> stuff. Just use %*ph properly (hint: star is put here and elsewhere on
> purpose).
> 
> drivers/hid/i2c-hid/i2c-hid.c:187:      i2c_hid_dbg(ihid, "%s:
> cmd=%*ph\n", __func__, length, cmd->data);
> 
That is what was missing - did not find any sensible example that showed
that this is possible and also Documentation/printk-formats.txt does
not mention this explicitly either.


>> +               l = min_t(size_t, len - i, spi_dump_bytes_per_line) * 3 - 1;
>> +               hexdata[l] = 0;
>> +
>> +               /* and print data including address and offset */
>> +               dev_info(&spi->dev, "%soff=0x%.5zx ptr=0x%pK: %s\n",
>> +                        prefix, i, ptr + i, hexdata);
>> +       }
>> +}
>> +
>> +static void spi_transfer_dump_buffer(struct spi_device *spi, char *prefix,
>> +                                    const void *ptr, size_t len,
>> +                                    size_t head_len, size_t tail_len)
>> +{
> 
> The function body looks too complicated. Can you describe what
> function does in different cases?
> 
Can add kernel do here

>> +               if (head_len | tail_len)
> 
> I think || looks a bit more logical here and below.
true...

> 
>> +                       spi_transfer_dump_buffer(spi, "\t",
>> +                                                xfer->tx_buf, xfer->len,
>> +                                                head_len, tail_len);
>> +       }
>> +       if (xfer->rx_buf) {
>> +               dev_info(dev, "    rx_buf:      %pK\n", xfer->rx_buf);
>> +               if (xfer->rx_dma)
>> +                       dev_info(dev, "    rx_dma:      %pad\n",
>> +                                &xfer->rx_dma);
>> +               if (head_len | tail_len)
true...
> 
>> +                       spi_transfer_dump_buffer(spi, "\t",
>> +                                                xfer->rx_buf, xfer->len,
>> +                                                head_len, tail_len);
> 
> Is the DMA buffer is in coherent memory? Otherwise you have to call
> dma_sync for it before reading on CPU side.

It has to be because it is not necessary that DMA is used at all.
Also the “feature” is depreciated anyway and Mark wants to see it going away.

Thanks again for the review…

Martin--
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

Applied "spi: loopback-test: rename method spi_test_fill_tx to spi_test_fill_pattern" to the spi tree

[Mark Brown]

The patch

   spi: loopback-test: rename method spi_test_fill_tx to spi_test_fill_pattern

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

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

>From 339ec3ce54e5c7137287dc807555ae69934b2d2a Mon Sep 17 00:00:00 2001
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Date: Tue, 22 Dec 2015 18:03:22 +0000
Subject: [PATCH] spi: loopback-test: rename method spi_test_fill_tx to
 spi_test_fill_pattern

Rename method spi_test_fill_tx to spi_test_fill_pattern
to better describe what it does.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 8af2e4070153..81fa9068df43 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -574,7 +574,8 @@ static int spi_test_translate(struct spi_device *spi,
 	return -EINVAL;
 }
 
-static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
+static int spi_test_fill_pattern(struct spi_device *spi,
+				 struct spi_test *test)
 {
 	struct spi_transfer *xfers = test->transfers;
 	u8 *tx_buf;
@@ -691,8 +692,8 @@ static int _spi_test_run_iter(struct spi_device *spi,
 		spi_message_add_tail(x, msg);
 	}
 
-	/* fill in the transfer data */
-	ret = spi_test_fill_tx(spi, test);
+	/* fill in the transfer buffers with pattern */
+	ret = spi_test_fill_pattern(spi, test);
 	if (ret)
 		return ret;
 
-- 
2.7.0.rc3

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

Applied "spi: loopback-test: write rx pattern also when running without tx_buf" to the spi tree

[Mark Brown]

The patch

   spi: loopback-test: write rx pattern also when running without tx_buf

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

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

>From e6520a3c8877d02c471135afb371e79b04409ab8 Mon Sep 17 00:00:00 2001
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Date: Tue, 22 Dec 2015 18:03:21 +0000
Subject: [PATCH] spi: loopback-test: write rx pattern also when running
 without tx_buf

Currently the rx_buf does not get set with the
SPI_TEST_PATTERN_UNWRITTEN when tx_buf == NULL in the transfer.

Reorder code so that it gets done also under this specific condition.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 7f497acc906c..8af2e4070153 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -591,6 +591,10 @@ static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
 
 	/* fill all transfers with the pattern requested */
 	for (i = 0; i < test->transfer_count; i++) {
+		/* fill rx_buf with SPI_TEST_PATTERN_UNWRITTEN */
+		if (xfers[i].rx_buf)
+			memset(xfers[i].rx_buf, SPI_TEST_PATTERN_UNWRITTEN,
+			       xfers[i].len);
 		/* if tx_buf is NULL then skip */
 		tx_buf = (u8 *)xfers[i].tx_buf;
 		if (!tx_buf)
@@ -648,10 +652,6 @@ static int spi_test_fill_tx(struct spi_device *spi, struct spi_test *test)
 				return -EINVAL;
 			}
 		}
-		/* fill rx_buf with SPI_TEST_PATTERN_UNWRITTEN */
-		if (xfers[i].rx_buf)
-			memset(xfers[i].rx_buf, SPI_TEST_PATTERN_UNWRITTEN,
-			       xfers[i].len);
 	}
 
 	return 0;
-- 
2.7.0.rc3

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

Applied "spi: loopback-test: added support for HW-loopback mode" to the spi tree

[Mark Brown]

The patch

   spi: loopback-test: added support for HW-loopback mode

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

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

>From b0632bfe5ec416a30d2e72bb1cc5dde5c6864e41 Mon Sep 17 00:00:00 2001
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Date: Tue, 22 Dec 2015 18:03:28 +0000
Subject: [PATCH] spi: loopback-test: added support for HW-loopback mode

If the module parameter "loopback" for testing RX to match TX
is not set and the spi_master supports SPI_LOOP then
SPI_LOOP as well as RX-data testing will get enabled.

When the "loopback" module parameter is set
then the SPI_LOOP support in spi_master will not get enabled,
which means that MOSI needs to get connected to MISO by some
other means (possibly via a simple jumper connection
or a SN74AHCT125 connecting MOSI/MISO)

Suggested-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 7f79a77c4b68..a194d4bab7fa 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -282,8 +282,34 @@ static struct spi_test spi_tests[] = {
 static int spi_loopback_test_probe(struct spi_device *spi)
 {
 	int ret;
+	char *msg;
 
-	dev_info(&spi->dev, "Executing spi-loopback-tests\n");
+	/*
+	 * Enable HW-loopback automatically if the master supports it
+	 * and we have the have_rx_buf unset (set means that we have
+	 * got an external loopback enabled, so no need to use SPI_LOOP)
+	 */
+	if (!check_rx_buf && (spi->master->mode_bits & SPI_LOOP)) {
+		spi->mode |= SPI_LOOP;
+		ret = spi_setup(spi);
+		if (ret) {
+			dev_err(&spi->dev,
+				"spi_setup failed to enable loopback: %i\n",
+				ret);
+			return ret;
+		}
+
+		/* force checking rx_buf for valid (loopback) data */
+		check_rx_buf = 1;
+
+		msg = "with SPI_LOOP support";
+	} else {
+		msg = (check_rx_buf) ?
+		      "with external loopback" :
+		      "without any loopback - rx_buf content is not checked";
+	}
+
+	dev_info(&spi->dev, "Executing spi-loopback-tests %s\n", msg);
 
 	ret = spi_test_run_tests(spi, spi_tests);
 
-- 
2.7.0.rc3

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

Applied "spi: loopback-test: spi_check_rx_ranges can get always done" to the spi tree

[Mark Brown]

The patch

   spi: loopback-test: spi_check_rx_ranges can get always done

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

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

>From 1e8db97f0e5205c0f6fd20c9a4f38cd871bc467f Mon Sep 17 00:00:00 2001
From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Date: Tue, 22 Dec 2015 18:03:25 +0000
Subject: [PATCH] spi: loopback-test: spi_check_rx_ranges can get always done

The spi_check_rx_ranges can always get executed independent of
if we have a real loopback situation.

Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-loopback-test.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c
index 81fa9068df43..7f79a77c4b68 100644
--- a/drivers/spi/spi-loopback-test.c
+++ b/drivers/spi/spi-loopback-test.c
@@ -489,7 +489,18 @@ static int spi_test_check_loopback_result(struct spi_device *spi,
 	struct spi_transfer *xfer;
 	u8 rxb, txb;
 	size_t i;
+	int ret;
+
+	/* checks rx_buffer pattern are valid with loopback or without */
+	ret = spi_check_rx_ranges(spi, msg, rx);
+	if (ret)
+		return ret;
 
+	/* if we run without loopback, then return now */
+	if (!loopback)
+		return 0;
+
+	/* if applicable to transfer check that rx_buf is equal to tx_buf */
 	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
 		/* if there is no rx, then no check is needed */
 		if (!xfer->rx_buf)
@@ -521,7 +532,7 @@ static int spi_test_check_loopback_result(struct spi_device *spi,
 		}
 	}
 
-	return spi_check_rx_ranges(spi, msg, rx);
+	return 0;
 
 mismatch_error:
 	dev_err(&spi->dev,
@@ -847,10 +858,8 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test,
 			goto exit;
 		}
 
-		/* run rx-tests when in loopback mode */
-		if (loopback)
-			ret = spi_test_check_loopback_result(spi, msg,
-							     tx, rx);
+		/* run rx-buffer tests */
+		ret = spi_test_check_loopback_result(spi, msg, tx, rx);
 	}
 
 	/* if requested or on error dump message (including data) */
-- 
2.7.0.rc3

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