diff mbox

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

Message ID 1393324819-3810-1-git-send-email-geert@linux-m68k.org (mailing list archive)
State Accepted
Commit e6456186cae76f80446ba911f77eb2f85d3d927e
Headers show

Commit Message

Geert Uytterhoeven Feb. 25, 2014, 10:40 a.m. UTC 
From: Geert Uytterhoeven <geert+renesas@linux-m68k.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@linux-m68k.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(-)

Comments

Mark Brown Feb. 27, 2014, 4:50 a.m. UTC | #1 
On Tue, Feb 25, 2014 at 11:40:16AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.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?
Geert Uytterhoeven Feb. 27, 2014, 8:59 a.m. UTC | #2 
Hi Mark,

On Thu, Feb 27, 2014 at 5:50 AM, Mark Brown <broonie@kernel.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@linux-m68k.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;