[PATCH 0/3] [SPI] loopback mode support, [POWERPC] loopback mode for spi_mpc83xx

[PATCH 0/3] [SPI] loopback mode support, [POWERPC] loopback mode for spi_mpc83xx

[Anton Vorontsov]

Hi all,

Here are three patches:
1. syncing spidev driver with new modes of spi subsystem;
2. spi subsystem support for loopback mode;
3. loopback mode support for spi_mpc83xx.

Because patch 3 depends on 1 and 2, I'm Cc'ing both lists
for all three patches.

They're based on Linus' tree as of today.

Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

[PATCH 1/3] [SPI] Sync spidev.{h,c} with spi.h

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/spi/spidev.c       |    4 ++--
 include/linux/spi/spidev.h |    3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 38b60ad..4c51a61 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -56,8 +56,8 @@ static unsigned long	minors[N_SPI_MINORS / BITS_PER_LONG];
 
 
 /* Bit masks for spi_device.mode management */
-#define SPI_MODE_MASK			(SPI_CPHA | SPI_CPOL)
-
+#define SPI_MODE_MASK			(SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | \
+					 SPI_LSB_FIRST | SPI_3WIRE)
 
 struct spidev_data {
 	struct device		dev;
diff --git a/include/linux/spi/spidev.h b/include/linux/spi/spidev.h
index 7d700be..369d42e 100644
--- a/include/linux/spi/spidev.h
+++ b/include/linux/spi/spidev.h
@@ -35,6 +35,9 @@
 #define SPI_MODE_2		(SPI_CPOL|0)
 #define SPI_MODE_3		(SPI_CPOL|SPI_CPHA)
 
+#define SPI_CS_HIGH		0x04
+#define SPI_LSB_FIRST		0x08
+#define SPI_3WIRE		0x10
 
 /*---------------------------------------------------------------------------*/
 
-- 
1.5.0.6

Re: [spi-devel-general] [PATCH 1/3] [SPI] Sync spidev.{h, c} with spi.h

[David Brownell]

On Thursday 26 July 2007, Anton Vorontsov wrote:
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -56,8 +56,8 @@ static unsigned long	minors[N_SPI_MINORS / BITS_PER_LONG];
>  
>  
>  /* Bit masks for spi_device.mode management */
> -#define SPI_MODE_MASK			(SPI_CPHA | SPI_CPOL)
> -
> +#define SPI_MODE_MASK			(SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | \
> +					 SPI_LSB_FIRST | SPI_3WIRE)

Hmm, I seem to recall explicitly leaving out CS_HIGH on the grounds
that getting it wrong could corrupt the whole bus; Not Good (tm).
And the same argument can apply to 3WIRE mode ...

On the other hand, that's not necessarily a good reason, since there
are other ways to trash hardware too.

Comments anyone?

- Dave

[PATCH 2/3] [SPI] Add new mode: SPI_LOOP

[Anton Vorontsov]

Loopback mode is supported by various controllers, this mode
is useful for testing, especially in conjunction with spidev
driver.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/spi/spidev.c       |    2 +-
 include/linux/spi/spi.h    |    1 +
 include/linux/spi/spidev.h |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 4c51a61..24c8d4d 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -57,7 +57,7 @@ static unsigned long	minors[N_SPI_MINORS / BITS_PER_LONG];
 
 /* Bit masks for spi_device.mode management */
 #define SPI_MODE_MASK			(SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | \
-					 SPI_LSB_FIRST | SPI_3WIRE)
+					 SPI_LSB_FIRST | SPI_3WIRE | SPI_LOOP)
 
 struct spidev_data {
 	struct device		dev;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 302b81d..e180615 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -77,6 +77,7 @@ struct spi_device {
 #define	SPI_CS_HIGH	0x04			/* chipselect active high? */
 #define	SPI_LSB_FIRST	0x08			/* per-word bits-on-wire */
 #define	SPI_3WIRE	0x10			/* SI/SO signals shared */
+#define	SPI_LOOP	0x20			/* loopback mode */
 	u8			bits_per_word;
 	int			irq;
 	void			*controller_state;
diff --git a/include/linux/spi/spidev.h b/include/linux/spi/spidev.h
index 369d42e..c93ef9d 100644
--- a/include/linux/spi/spidev.h
+++ b/include/linux/spi/spidev.h
@@ -38,6 +38,7 @@
 #define SPI_CS_HIGH		0x04
 #define SPI_LSB_FIRST		0x08
 #define SPI_3WIRE		0x10
+#define SPI_LOOP		0x20
 
 /*---------------------------------------------------------------------------*/
 
-- 
1.5.0.6

Re: [spi-devel-general] [PATCH 2/3] [SPI] Add new mode: SPI_LOOP

[David Brownell]

On Thursday 26 July 2007, Anton Vorontsov wrote:
> Loopback mode is supported by various controllers, this mode
> is useful for testing, especially in conjunction with spidev
> driver.

ISTR that Stephen Street provided a loopback mode for debug
in his pxa2xx_spi code.  And I know you're fight that this
mode shows up in a lot of hardware.

Comments, anyone?  This seems like a fair way to expose this
mechanism.  And I tend to agree that it'd mostly be useful in
conjunction with "spidev".

Anton -- assuming this goes in, it'd be nice if you could
contribute a simple test program using this, which we could
keep in Documentation/spi somewhere.  (Maybe with other testing
notes, if anyone comes up with such.)

- Dave


>	...
>
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -77,6 +77,7 @@ struct spi_device {
>  #define	SPI_CS_HIGH	0x04			/* chipselect active high? */
>  #define	SPI_LSB_FIRST	0x08			/* per-word bits-on-wire */
>  #define	SPI_3WIRE	0x10			/* SI/SO signals shared */
> +#define	SPI_LOOP	0x20			/* loopback mode */
>  	u8			bits_per_word;
>  	int			irq;
>  	void			*controller_state;

Re: [spi-devel-general] [PATCH 2/3] [SPI] Add new mode: SPI_LOOP

[Ned Forrester]

David Brownell wrote:
> On Thursday 26 July 2007, Anton Vorontsov wrote:
>> Loopback mode is supported by various controllers, this mode
>> is useful for testing, especially in conjunction with spidev
>> driver.
> 
> ISTR that Stephen Street provided a loopback mode for debug
> in his pxa2xx_spi code.  And I know you're fight that this
> mode shows up in a lot of hardware.
> 
> Comments, anyone?  This seems like a fair way to expose this
> mechanism.  And I tend to agree that it'd mostly be useful in
> conjunction with "spidev".

Yes, it is in pxa2xx_spi.  It is passed in a structure that is attached 
to spi_board_info.controller_data.  I have used it to great effect to 
test data modes in the driver for which I have no external hardware 
support: various bits/word, clock frequencies, fifo thresholds, dma 
burst sizes, etc.

It works for me if this is moved to struct spi_device.

I don't think I have any code that could be readily turned into a 
generic test program.

-- 
Ned Forrester                                       nforrester@whoi.edu
Oceanographic Systems Lab                                  508-289-2226
Applied Ocean Physics and Engineering Dept.
Woods Hole Oceanographic Institution          Woods Hole, MA 02543, USA
http://www.whoi.edu/sbl/liteSite.do?litesiteid=7212
http://www.whoi.edu/hpb/Site.do?id=1532
http://www.whoi.edu/page.do?pid=10079

Re: [spi-devel-general] [PATCH 2/3] [SPI] Add new mode: SPI_LOOP

[David Brownell]

On Thursday 26 July 2007, Ned Forrester wrote:
> David Brownell wrote:
> > On Thursday 26 July 2007, Anton Vorontsov wrote:
> >> Loopback mode is supported by various controllers, this mode
> >> is useful for testing, especially in conjunction with spidev
> >> driver.
> > 
> > ISTR that Stephen Street provided a loopback mode for debug
> > in his pxa2xx_spi code.  And I know you're fight that this
> > mode shows up in a lot of hardware.
> > 
> > Comments, anyone?  This seems like a fair way to expose this
> > mechanism.  And I tend to agree that it'd mostly be useful in
> > conjunction with "spidev".
> 
> Yes, it is in pxa2xx_spi.  It is passed in a structure that is attached 
> to spi_board_info.controller_data.  I have used it to great effect to 
> test data modes in the driver for which I have no external hardware 
> support: various bits/word, clock frequencies, fifo thresholds, dma 
> burst sizes, etc.

Well, the SPI_LOOP stuff is now in kernel.org GIT and 2.6.23-rc2,
so if someone wants to update pxa2xx_spi to support this through
spi->mode instead of controller data, now is a good time to start
working on that.

- Dave

Re: [spi-devel-general] [PATCH 2/3] [SPI] Add new mode: SPI_LOOP

[Anton Vorontsov]

On Thu, Jul 26, 2007 at 08:07:07PM -0700, David Brownell wrote:
> On Thursday 26 July 2007, Anton Vorontsov wrote:
> > Loopback mode is supported by various controllers, this mode
> > is useful for testing, especially in conjunction with spidev
> > driver.
> 
> ISTR that Stephen Street provided a loopback mode for debug
> in his pxa2xx_spi code.  And I know you're fight that this
> mode shows up in a lot of hardware.
> 
> Comments, anyone?  This seems like a fair way to expose this
> mechanism.  And I tend to agree that it'd mostly be useful in
> conjunction with "spidev".
> 
> Anton -- assuming this goes in, it'd be nice if you could
> contribute a simple test program using this, which we could
> keep in Documentation/spi somewhere.  (Maybe with other testing
> notes, if anyone comes up with such.)

Sure. Test utility is below.

I'm aware it's not perfect, it sends just one transfer message,
which not covers all testing requirements. So implementing
--datafile switch to pass .ppm-alike files (as in
drivers/video/logo/) would be great. But I'm leaving this feature
implementation to those who will really need it. ;-)

Thanks!

- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: [SPI] spidev_test utility

This is simple utility used to test SPI functionality.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 Documentation/spi/spidev_test.c |  202 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 202 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/spi/spidev_test.c

diff --git a/Documentation/spi/spidev_test.c b/Documentation/spi/spidev_test.c
new file mode 100644
index 0000000..d76a72b
--- /dev/null
+++ b/Documentation/spi/spidev_test.c
@@ -0,0 +1,202 @@
+/*
+ * SPI testing utility (using spidev driver)
+ *
+ * Copyright (c) 2007  MontaVista Software, Inc.
+ * Copyright (c) 2007  Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * Cross-compile with cross-gcc -I/path/to/cross-kernel/include
+ */
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <getopt.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+#include <linux/spi/spidev.h>
+
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
+
+static void pabort(const char *s)
+{
+	perror(s);
+	abort();
+}
+
+static char *device = "/dev/spidev1.1";
+static uint8_t mode;
+static uint8_t bits = 8;
+static uint32_t speed = 500000;
+static uint16_t delay;
+
+static void transfer(int fd)
+{
+	int ret;
+	uint8_t tx[] = {
+		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+		0x40, 0x00, 0x00, 0x00, 0x00, 0x95,
+		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+		0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+		0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD,
+		0xF0, 0x0D,
+	};
+	uint8_t rx[ARRAY_SIZE(tx)] = {0, };
+	struct spi_ioc_transfer tr = {
+		.tx_buf = (unsigned long)tx,
+		.rx_buf = (unsigned long)rx,
+		.len = ARRAY_SIZE(tx),
+		.delay_usecs = delay,
+		.speed_hz = speed,
+		.bits_per_word = bits,
+	};
+
+	ret = ioctl(fd, SPI_IOC_MESSAGE(1), &tr);
+	if (ret == 1)
+		pabort("can't send spi message");
+
+	for (ret = 0; ret < ARRAY_SIZE(tx); ret++) {
+		if (!(ret % 6))
+			puts("");
+		printf("%.2X ", rx[ret]);
+	}
+	puts("");
+}
+
+void print_usage(char *prog)
+{
+	printf("Usage: %s [-DsbdlHOLC3]\n", prog);
+	puts("  -D --device   device to use (default /dev/spidev1.1)\n"
+	     "  -s --speed    speed (Hz)\n"
+	     "  -d --delay    delay (usec)\n"
+	     "  -b --bpw      bits per word \n"
+	     "  -l --loop     loopback\n"
+	     "  -H --cpha     clock phase\n"
+	     "  -O --cpol     clock polarity\n"
+	     "  -L --lsb      least significant bit first\n"
+	     "  -C --cs-high  chip select active high\n"
+	     "  -3 --3wire    SI/SO signals shared\n");
+	exit(1);
+}
+
+void parse_opts(int argc, char *argv[])
+{
+	while (1) {
+		static struct option lopts[] = {
+			{ "device",  1, 0, 'D' },
+			{ "speed",   1, 0, 's' },
+			{ "delay",   1, 0, 'd' },
+			{ "bpw",     1, 0, 'b' },
+			{ "loop",    0, 0, 'l' },
+			{ "cpha",    0, 0, 'H' },
+			{ "cpol",    0, 0, 'O' },
+			{ "lsb",     0, 0, 'L' },
+			{ "cs-high", 0, 0, 'C' },
+			{ "3wire",   0, 0, '3' },
+			{ NULL, 0, 0, 0 },
+		};
+		int c;
+
+		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3", lopts, NULL);
+
+		if (c == -1)
+			break;
+
+		switch (c) {
+		case 'D':
+			device = optarg;
+			break;
+		case 's':
+			speed = atoi(optarg);
+			break;
+		case 'd':
+			delay = atoi(optarg);
+			break;
+		case 'b':
+			bits = atoi(optarg);
+			break;
+		case 'l':
+			mode |= SPI_LOOP;
+			break;
+		case 'H':
+			mode |= SPI_CPHA;
+			break;
+		case 'O':
+			mode |= SPI_CPOL;
+			break;
+		case 'L':
+			mode |= SPI_LSB_FIRST;
+			break;
+		case 'C':
+			mode |= SPI_CS_HIGH;
+			break;
+		case '3':
+			mode |= SPI_3WIRE;
+			break;
+		default:
+			print_usage(argv[0]);
+			break;
+		}
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	int ret = 0;
+	int fd;
+
+	parse_opts(argc, argv);
+
+	fd = open(device, O_RDWR);
+	if (fd < 0)
+		pabort("can't open device");
+
+	/*
+	 * spi mode
+	 */
+	ret = ioctl(fd, SPI_IOC_WR_MODE, &mode);
+	if (ret == -1)
+		pabort("can't set spi mode");
+
+	ret = ioctl(fd, SPI_IOC_RD_MODE, &mode);
+	if (ret == -1)
+		pabort("can't get spi mode");
+
+	/*
+	 * bits per word
+	 */
+	ret = ioctl(fd, SPI_IOC_WR_BITS_PER_WORD, &bits);
+	if (ret == -1)
+		pabort("can't set bits per word");
+
+	ret = ioctl(fd, SPI_IOC_RD_BITS_PER_WORD, &bits);
+	if (ret == -1)
+		pabort("can't get bits per word");
+
+	/*
+	 * max speed hz
+	 */
+	ret = ioctl(fd, SPI_IOC_WR_MAX_SPEED_HZ, &speed);
+	if (ret == -1)
+		pabort("can't set max speed hz");
+
+	ret = ioctl(fd, SPI_IOC_RD_MAX_SPEED_HZ, &speed);
+	if (ret == -1)
+		pabort("can't get max speed hz");
+
+	printf("spi mode: %d\n", mode);
+	printf("bits per word: %d\n", bits);
+	printf("max speed: Hz (kHz): %d (%d)\n", speed, speed/1000);
+
+	transfer(fd);
+
+	close(fd);
+
+	return ret;
+}
-- 
1.5.0.6

[PATCH 3/3] [POWERPC][SPI] spi_mpc83xx: add support for loopback mode

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/spi/spi_mpc83xx.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index 458075a..e9582d3 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -39,6 +39,7 @@ struct mpc83xx_spi_reg {
 };
 
 /* SPI Controller mode register definitions */
+#define	SPMODE_LOOP		(1 << 30)
 #define	SPMODE_CI_INACTIVEHIGH	(1 << 29)
 #define	SPMODE_CP_BEGIN_EDGECLK	(1 << 28)
 #define	SPMODE_DIV16		(1 << 27)
@@ -155,7 +156,7 @@ static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
 		/* mask out bits we are going to set */
 		regval &= ~(SPMODE_CP_BEGIN_EDGECLK | SPMODE_CI_INACTIVEHIGH |
 			    SPMODE_LEN(0xF) | SPMODE_DIV16 | SPMODE_PM(0xF) | \
-			    SPMODE_REV);
+			    SPMODE_REV | SPMODE_LOOP);
 
 		if (spi->mode & SPI_CPHA)
 			regval |= SPMODE_CP_BEGIN_EDGECLK;
@@ -163,6 +164,8 @@ static void mpc83xx_spi_chipselect(struct spi_device *spi, int value)
 			regval |= SPMODE_CI_INACTIVEHIGH;
 		if (!(spi->mode & SPI_LSB_FIRST))
 			regval |= SPMODE_REV;
+		if (spi->mode & SPI_LOOP)
+			regval |= SPMODE_LOOP;
 
 		regval |= SPMODE_LEN(len);
 
@@ -273,7 +276,8 @@ int mpc83xx_spi_setup_transfer(struct spi_device *spi, struct spi_transfer *t)
 }
 
 /* the spi->mode bits understood by this driver: */
-#define MODEBITS (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST)
+#define MODEBITS (SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST | \
+		  SPI_LOOP)
 
 static int mpc83xx_spi_setup(struct spi_device *spi)
 {
-- 
1.5.0.6