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

[PATCH v3 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
V2->V3: only change in patch 3 as per review of Andy Shevchenko
	* kernel doc
	* use of %*ph for dumping hex
	* fixing | to ||

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 |  217 +++++++++++++++++++++------------------
 drivers/spi/spi.c               |  171 ++++++++++++++++++++++++++++++
 include/linux/spi/spi.h         |   24 +++++
 3 files changed, 311 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 v3 1/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 v3 2/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 v3 3/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       |  171 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/spi/spi.h |   24 +++++++
 2 files changed, 195 insertions(+)

Changelog:
V1->V2: changes recommended by Andy Shevchenko
V2->V3: changes recommended by Andy Shevchenko
	specifically:
	* formating of hex dump using %*ph
	* added some kerneldoc


diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9964835..63f31f8 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,176 @@ 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)
+{
+	for (; start < len; start += spi_dump_bytes_per_line) {
+		/* print data including address and offset */
+		dev_info(&spi->dev, "%soff=0x%.5zx ptr=0x%pK: %*ph\n",
+			 prefix, start, ptr + start,
+			 min_t(size_t, len - start, spi_dump_bytes_per_line),
+			 ptr + start);
+	}
+}
+
+/**
+ * spi_transfer_dump_buffer - dump head/tail portions of a buffer
+ * @spi:      the spi device to use for printing using dev_info
+ * @prefix:   the prefix to put in front of each line for alignment
+ * @ptr:      the buffer to dump
+ * @len:      the total length of the buffer
+ * @head_len: dump this many bytes from the start of the buffer
+ *            (rounded up to a multiple of 16)
+ * @tail_len: dump this many bytes from the end of the buffer
+ *            (rounded up so that a the start offset is a multiple of 16)
+ */
+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 v3 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 v3 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 v3 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..312754f 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 v3 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".

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 312754f..001f463 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 v3 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 |   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 001f463..b49065a 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 v3 3/8] spi: core: add spi_message_dump and spi_transfer_dump

[Mark Brown]

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

On Tue, Dec 22, 2015 at 06:03:23PM +0000, 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.

It can definitely be useful but as Andy indicated memory coherency does
mean it can get fun actually using it reliably - you'd need to make sure
that both buffers are OK for the CPU to access otherwise it can give
misleading results...  Hrm.  Comments might be all that's needed here,
or flags to say which buffer contents to dump.  The flags might be most
sensible since for bidrectional transfers you'll quite often want to
dump only one direction at once when debugging.

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

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

[Mark Brown]

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

On Tue, Dec 22, 2015 at 06:03:20PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> 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 looks good apart from the memory coherency thing on the patch
moving things into the core - I'll apply what applies without a
dependency on that patch.

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