[PATCH v2 1/4] spi: spidev: Restore all SPI mode flags on ioctl failure

[PATCH v2 1/4] spi: spidev: Restore all SPI mode flags on ioctl failure

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

In commit f477b7fb13df2b843997559ff34e87d054ba6538 ("spi: DUAL and QUAD
support"), spi_device.mode was enlarged from 8 to 16 bits.

However, the spidev code still only saved 8 bits of data. If a spidev
SPI_IOC_WR_MODE or SPI_IOC_WR_LSB_FIRST request failed, only the lower 8
bits of the SPI mode were restored, inadvertently clearing the upper 8
bits, possibly disabling Quad or Dual SPI transfers for the device.

Save up to 32 bits to fix this.

For SPI_IOC_WR_MODE this is probably not so important, as it doesn't allow
setting Quad or Dual mode anyway, but SPI_IOC_WR_LSB_FIRST is used to just
set or clear a single bit.

Signed-off-by: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
v2:
  - Split off from "spi: spidev: Add support for Dual/Quad SPI Transfers",
    as this is a bug fix.
    Mark, do you think this is worthwhile for stable (v3.12 and v3.13)?

 drivers/spi/spidev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index d7c6e36021e8..2abc0f5a82be 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -374,7 +374,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case SPI_IOC_WR_MODE:
 		retval = __get_user(tmp, (u8 __user *)arg);
 		if (retval == 0) {
-			u8	save = spi->mode;
+			u32	save = spi->mode;
 
 			if (tmp & ~SPI_MODE_MASK) {
 				retval = -EINVAL;
@@ -393,7 +393,7 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	case SPI_IOC_WR_LSB_FIRST:
 		retval = __get_user(tmp, (__u8 __user *)arg);
 		if (retval == 0) {
-			u8	save = spi->mode;
+			u32	save = spi->mode;
 
 			if (tmp)
 				spi->mode |= SPI_LSB_FIRST;
-- 
1.7.9.5

--
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/4] spi: spidev: Add support for Dual/Quad SPI Transfers

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Add support for Dual/Quad SPI Transfers to the spidev API.
As this uses SPI mode bits that don't fit in a single byte, two new
ioctls (SPI_IOC_RD_MODE32 and SPI_IOC_WR_MODE32) are introduced.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
v2:
  - Introduce SPI_IOC_{RD,WR}_MODE32 to avoid breaking backwards
    compatibility,
  - Split off the spidev_test changes in a separate patch
  - Update documentation.

 Documentation/spi/spidev        |    6 ++++++
 drivers/spi/spidev.c            |   19 +++++++++++++++----
 include/uapi/linux/spi/spidev.h |   14 ++++++++++++--
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/spi/spidev b/Documentation/spi/spidev
index ed2da5e5b28a..3d14035b1766 100644
--- a/Documentation/spi/spidev
+++ b/Documentation/spi/spidev
@@ -85,6 +85,12 @@ settings for data transfer parameters:
 	SPI_MODE_0..SPI_MODE_3; or if you prefer you can combine SPI_CPOL
 	(clock polarity, idle high iff this is set) or SPI_CPHA (clock phase,
 	sample on trailing edge iff this is set) flags.
+	Note that this request is limited to SPI mode flags that fit in a
+	single byte.
+
+    SPI_IOC_RD_MODE32, SPI_IOC_WR_MODE32 ... pass a pointer to a uin32_t
+	which will return (RD) or assign (WR) the full SPI transfer mode,
+	not limited to the bits that fit in one byte.
 
     SPI_IOC_RD_LSB_FIRST, SPI_IOC_WR_LSB_FIRST ... pass a pointer to a byte
 	which will return (RD) or assign (WR) the bit justification used to
diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 2abc0f5a82be..e3bc23bb5883 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -73,7 +73,8 @@ static DECLARE_BITMAP(minors, N_SPI_MINORS);
  */
 #define SPI_MODE_MASK		(SPI_CPHA | SPI_CPOL | SPI_CS_HIGH \
 				| SPI_LSB_FIRST | SPI_3WIRE | SPI_LOOP \
-				| SPI_NO_CS | SPI_READY)
+				| SPI_NO_CS | SPI_READY | SPI_TX_DUAL \
+				| SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD)
 
 struct spidev_data {
 	dev_t			devt;
@@ -265,6 +266,8 @@ static int spidev_message(struct spidev_data *spidev,
 		buf += k_tmp->len;
 
 		k_tmp->cs_change = !!u_tmp->cs_change;
+		k_tmp->tx_nbits = u_tmp->tx_nbits;
+		k_tmp->rx_nbits = u_tmp->rx_nbits;
 		k_tmp->bits_per_word = u_tmp->bits_per_word;
 		k_tmp->delay_usecs = u_tmp->delay_usecs;
 		k_tmp->speed_hz = u_tmp->speed_hz;
@@ -359,6 +362,10 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		retval = __put_user(spi->mode & SPI_MODE_MASK,
 					(__u8 __user *)arg);
 		break;
+	case SPI_IOC_RD_MODE32:
+		retval = __put_user(spi->mode & SPI_MODE_MASK,
+					(__u32 __user *)arg);
+		break;
 	case SPI_IOC_RD_LSB_FIRST:
 		retval = __put_user((spi->mode & SPI_LSB_FIRST) ?  1 : 0,
 					(__u8 __user *)arg);
@@ -372,7 +379,11 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
 	/* write requests */
 	case SPI_IOC_WR_MODE:
-		retval = __get_user(tmp, (u8 __user *)arg);
+	case SPI_IOC_WR_MODE32:
+		if (cmd == SPI_IOC_WR_MODE)
+			retval = __get_user(tmp, (u8 __user *)arg);
+		else
+			retval = __get_user(tmp, (u32 __user *)arg);
 		if (retval == 0) {
 			u32	save = spi->mode;
 
@@ -382,12 +393,12 @@ spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			}
 
 			tmp |= spi->mode & ~SPI_MODE_MASK;
-			spi->mode = (u8)tmp;
+			spi->mode = (u16)tmp;
 			retval = spi_setup(spi);
 			if (retval < 0)
 				spi->mode = save;
 			else
-				dev_dbg(&spi->dev, "spi mode %02x\n", tmp);
+				dev_dbg(&spi->dev, "spi mode %x\n", tmp);
 		}
 		break;
 	case SPI_IOC_WR_LSB_FIRST:
diff --git a/include/uapi/linux/spi/spidev.h b/include/uapi/linux/spi/spidev.h
index 52d9ed01855f..dd5f21e75805 100644
--- a/include/uapi/linux/spi/spidev.h
+++ b/include/uapi/linux/spi/spidev.h
@@ -42,6 +42,10 @@
 #define SPI_LOOP		0x20
 #define SPI_NO_CS		0x40
 #define SPI_READY		0x80
+#define SPI_TX_DUAL		0x100
+#define SPI_TX_QUAD		0x200
+#define SPI_RX_DUAL		0x400
+#define SPI_RX_QUAD		0x800
 
 /*---------------------------------------------------------------------------*/
 
@@ -92,7 +96,9 @@ struct spi_ioc_transfer {
 	__u16		delay_usecs;
 	__u8		bits_per_word;
 	__u8		cs_change;
-	__u32		pad;
+	__u8		tx_nbits;
+	__u8		rx_nbits;
+	__u16		pad;
 
 	/* If the contents of 'struct spi_ioc_transfer' ever change
 	 * incompatibly, then the ioctl number (currently 0) must change;
@@ -110,7 +116,7 @@ struct spi_ioc_transfer {
 #define SPI_IOC_MESSAGE(N) _IOW(SPI_IOC_MAGIC, 0, char[SPI_MSGSIZE(N)])
 
 
-/* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) */
+/* Read / Write of SPI mode (SPI_MODE_0..SPI_MODE_3) (limited to 8 bits) */
 #define SPI_IOC_RD_MODE			_IOR(SPI_IOC_MAGIC, 1, __u8)
 #define SPI_IOC_WR_MODE			_IOW(SPI_IOC_MAGIC, 1, __u8)
 
@@ -126,6 +132,10 @@ struct spi_ioc_transfer {
 #define SPI_IOC_RD_MAX_SPEED_HZ		_IOR(SPI_IOC_MAGIC, 4, __u32)
 #define SPI_IOC_WR_MAX_SPEED_HZ		_IOW(SPI_IOC_MAGIC, 4, __u32)
 
+/* Read / Write of the SPI mode field */
+#define SPI_IOC_RD_MODE32		_IOR(SPI_IOC_MAGIC, 5, __u32)
+#define SPI_IOC_WR_MODE32		_IOW(SPI_IOC_MAGIC, 5, __u32)
+
 
 
 #endif /* SPIDEV_H */
-- 
1.7.9.5

Re: [PATCH v2 2/4] spi: spidev: Add support for Dual/Quad SPI Transfers

[Mark Brown]

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

On Tue, Feb 25, 2014 at 11:40:17AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Add support for Dual/Quad SPI Transfers to the spidev API.
> As this uses SPI mode bits that don't fit in a single byte, two new
> ioctls (SPI_IOC_RD_MODE32 and SPI_IOC_WR_MODE32) are introduced.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v2 3/4] spi: spidev_test: Add support for Dual/Quad SPI Transfers

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
v2:
  - Introduce SPI_IOC_{RD,WR}_MODE32 to avoid breaking backwards
    compatibility,
  - Split off the spidev_test changes in a separate patch

 Documentation/spi/spidev_test.c |   43 +++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/Documentation/spi/spidev_test.c b/Documentation/spi/spidev_test.c
index efd7385e907f..3a2f9d59edab 100644
--- a/Documentation/spi/spidev_test.c
+++ b/Documentation/spi/spidev_test.c
@@ -30,7 +30,7 @@ static void pabort(const char *s)
 }
 
 static const char *device = "/dev/spidev1.1";
-static uint8_t mode;
+static uint32_t mode;
 static uint8_t bits = 8;
 static uint32_t speed = 500000;
 static uint16_t delay;
@@ -57,6 +57,21 @@ static void transfer(int fd)
 		.bits_per_word = bits,
 	};
 
+	if (mode & SPI_TX_QUAD)
+		tr.tx_nbits = 4;
+	else if (mode & SPI_TX_DUAL)
+		tr.tx_nbits = 2;
+	if (mode & SPI_RX_QUAD)
+		tr.rx_nbits = 4;
+	else if (mode & SPI_RX_DUAL)
+		tr.rx_nbits = 2;
+	if (!(mode & SPI_LOOP)) {
+		if (mode & (SPI_TX_QUAD | SPI_TX_DUAL))
+			tr.rx_buf = 0;
+		else if (mode & (SPI_RX_QUAD | SPI_RX_DUAL))
+			tr.tx_buf = 0;
+	}
+
 	ret = ioctl(fd, SPI_IOC_MESSAGE(1), &tr);
 	if (ret < 1)
 		pabort("can't send spi message");
@@ -83,7 +98,9 @@ static void print_usage(const char *prog)
 	     "  -C --cs-high  chip select active high\n"
 	     "  -3 --3wire    SI/SO signals shared\n"
 	     "  -N --no-cs    no chip select\n"
-	     "  -R --ready    slave pulls low to pause\n");
+	     "  -R --ready    slave pulls low to pause\n"
+	     "  -2 --dual     dual transfer\n"
+	     "  -4 --quad     quad transfer\n");
 	exit(1);
 }
 
@@ -103,11 +120,13 @@ static void parse_opts(int argc, char *argv[])
 			{ "3wire",   0, 0, '3' },
 			{ "no-cs",   0, 0, 'N' },
 			{ "ready",   0, 0, 'R' },
+			{ "dual",    0, 0, '2' },
+			{ "quad",    0, 0, '4' },
 			{ NULL, 0, 0, 0 },
 		};
 		int c;
 
-		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR", lopts, NULL);
+		c = getopt_long(argc, argv, "D:s:d:b:lHOLC3NR24", lopts, NULL);
 
 		if (c == -1)
 			break;
@@ -149,11 +168,23 @@ static void parse_opts(int argc, char *argv[])
 		case 'R':
 			mode |= SPI_READY;
 			break;
+		case '2':
+			mode |= SPI_TX_DUAL;
+			break;
+		case '4':
+			mode |= SPI_TX_QUAD;
+			break;
 		default:
 			print_usage(argv[0]);
 			break;
 		}
 	}
+	if (mode & SPI_LOOP) {
+		if (mode & SPI_TX_DUAL)
+			mode |= SPI_RX_DUAL;
+		if (mode & SPI_TX_QUAD)
+			mode |= SPI_RX_QUAD;
+	}
 }
 
 int main(int argc, char *argv[])
@@ -170,11 +201,11 @@ int main(int argc, char *argv[])
 	/*
 	 * spi mode
 	 */
-	ret = ioctl(fd, SPI_IOC_WR_MODE, &mode);
+	ret = ioctl(fd, SPI_IOC_WR_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't set spi mode");
 
-	ret = ioctl(fd, SPI_IOC_RD_MODE, &mode);
+	ret = ioctl(fd, SPI_IOC_RD_MODE32, &mode);
 	if (ret == -1)
 		pabort("can't get spi mode");
 
@@ -200,7 +231,7 @@ int main(int argc, char *argv[])
 	if (ret == -1)
 		pabort("can't get max speed hz");
 
-	printf("spi mode: %d\n", mode);
+	printf("spi mode: 0x%x\n", mode);
 	printf("bits per word: %d\n", bits);
 	printf("max speed: %d Hz (%d KHz)\n", speed, speed/1000);
 
-- 
1.7.9.5

Re: [PATCH v2 3/4] spi: spidev_test: Add support for Dual/Quad SPI Transfers

[Mark Brown]

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

On Tue, Feb 25, 2014 at 11:40:18AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v2 4/4] spi: spidev_fdx: Add support for Dual/Quad SPI Transfers

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Use SPI_IOC_RD_MODE32 to print the full SPI mode, now in hex.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
v2:
  - New

 Documentation/spi/spidev_fdx.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/spi/spidev_fdx.c b/Documentation/spi/spidev_fdx.c
index 36ec0774ca0b..0ea3e51292fc 100644
--- a/Documentation/spi/spidev_fdx.c
+++ b/Documentation/spi/spidev_fdx.c
@@ -78,10 +78,10 @@ static void do_msg(int fd, int len)
 
 static void dumpstat(const char *name, int fd)
 {
-	__u8	mode, lsb, bits;
-	__u32	speed;
+	__u8	lsb, bits;
+	__u32	mode, speed;
 
-	if (ioctl(fd, SPI_IOC_RD_MODE, &mode) < 0) {
+	if (ioctl(fd, SPI_IOC_RD_MODE32, &mode) < 0) {
 		perror("SPI rd_mode");
 		return;
 	}
@@ -98,7 +98,7 @@ static void dumpstat(const char *name, int fd)
 		return;
 	}
 
-	printf("%s: spi mode %d, %d bits %sper word, %d Hz max\n",
+	printf("%s: spi mode 0x%x, %d bits %sper word, %d Hz max\n",
 		name, mode, bits, lsb ? "(lsb first) " : "", speed);
 }
 
-- 
1.7.9.5

Re: [PATCH v2 4/4] spi: spidev_fdx: Add support for Dual/Quad SPI Transfers

[Mark Brown]

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

On Tue, Feb 25, 2014 at 11:40:19AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> 
> Use SPI_IOC_RD_MODE32 to print the full SPI mode, now in hex.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 1/4] spi: spidev: Restore all SPI mode flags on ioctl failure

[Mark Brown]

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

On Tue, Feb 25, 2014 at 11:40:16AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> 
> In commit f477b7fb13df2b843997559ff34e87d054ba6538 ("spi: DUAL and QUAD
> support"), spi_device.mode was enlarged from 8 to 16 bits.

Applied, thanks.

> For SPI_IOC_WR_MODE this is probably not so important, as it doesn't allow
> setting Quad or Dual mode anyway, but SPI_IOC_WR_LSB_FIRST is used to just
> set or clear a single bit.

Since there's no API for it at present I'd not expect somethng that is
using spidev to be able to have enabled any of the high mode bits.
Unless I'm missing some path for this?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 1/4] spi: spidev: Restore all SPI mode flags on ioctl failure

[Geert Uytterhoeven]

Hi Mark,

On Thu, Feb 27, 2014 at 5:50 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > In commit f477b7fb13df2b843997559ff34e87d054ba6538 ("spi: DUAL and QUAD
> > support"), spi_device.mode was enlarged from 8 to 16 bits.
>
> Applied, thanks.

Thanks a lot!

>> For SPI_IOC_WR_MODE this is probably not so important, as it doesn't allow
>> setting Quad or Dual mode anyway, but SPI_IOC_WR_LSB_FIRST is used to just
>> set or clear a single bit.
>
> Since there's no API for it at present I'd not expect somethng that is
> using spidev to be able to have enabled any of the high mode bits.
> Unless I'm missing some path for this?

No, you're right.

While Quad mode could have been enabled in board info or DT, you can't use
it without setting [rt]x_nbits, and you can't share an SPI slave between spidev
and another driver[*]. I forgot about the latter.

[*] It would be a nice feature for debugging, though. In fact I'm
doing it, as the
    RSPI driver currently ignores the chip select number, so I have 3 m25p80
    drivers (in single, dual, resp. quad mode), and an spidev driver,
all talking to
    the same SPI FLASH.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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