diff mbox

[4/5] spi: Check that Quad/Dual is half duplex

Message ID 1390317009-8292-4-git-send-email-geert@linux-m68k.org (mailing list archive)
State Changes Requested
Headers show

Commit Message

Geert Uytterhoeven Jan. 21, 2014, 3:10 p.m. UTC
From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Quad and Dual SPI Transfers use all available data lines (incl. MOSI/MISO),
hence they must be half duplex. Add a check that verify that.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
I think this check is done nowhere else. Please correct me if I missed it.

 drivers/spi/spi.c |    5 +++++
 1 file changed, 5 insertions(+)

Comments

Mark Brown Jan. 21, 2014, 6:21 p.m. UTC | #1
On Tue, Jan 21, 2014 at 04:10:08PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Quad and Dual SPI Transfers use all available data lines (incl. MOSI/MISO),
> hence they must be half duplex. Add a check that verify that.

This is surprising to me - I had expected that there would be extra
signals that would be used for these modes, not that the opposite
direction data line would be one of the ones being reused.  On the other
hand if this is what all the flash chips do then it would seem
reasonable that controllers do the same.  Can you clarify please?
Geert Uytterhoeven Jan. 21, 2014, 8:06 p.m. UTC | #2
On Tue, Jan 21, 2014 at 7:21 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jan 21, 2014 at 04:10:08PM +0100, Geert Uytterhoeven wrote:
>> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> Quad and Dual SPI Transfers use all available data lines (incl. MOSI/MISO),
>> hence they must be half duplex. Add a check that verify that.
>
> This is surprising to me - I had expected that there would be extra
> signals that would be used for these modes, not that the opposite
> direction data line would be one of the ones being reused.  On the other
> hand if this is what all the flash chips do then it would seem
> reasonable that controllers do the same.  Can you clarify please?

Dual SPI works by aggregating the MOSI and MISO lines for 2-bit
unidirectional transfers.
Quad SPI aggregates MOSI, MISO, and 2 additional lines for 4-bit
unidirectional transfers.

Hence Dual SPI uses the traditional 4-wire wiring, while Quad SPI uses
6-wire.

SPI FLASH chips handle it this way, and the Renesas QSPI controller
in the r8a7790/7791 SoCs, too.

Typically the first transfer in a message is a Single Transfer (e.g. read data
command), while subsequent transfers can be Dual or Quad Transfers (e.g.
the actual data read from the 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
Geert Uytterhoeven Jan. 22, 2014, 2:27 p.m. UTC | #3
On Tue, Jan 21, 2014 at 4:10 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Quad and Dual SPI Transfers use all available data lines (incl. MOSI/MISO),
> hence they must be half duplex. Add a check that verify that.

Actually this is valid if SPI_LOOP is also set. Will send an update.

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
Gerhard Sittig Jan. 22, 2014, 7:19 p.m. UTC | #4
On Tue, Jan 21, 2014 at 21:06 +0100, Geert Uytterhoeven wrote:
> 
> On Tue, Jan 21, 2014 at 7:21 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Jan 21, 2014 at 04:10:08PM +0100, Geert Uytterhoeven wrote:
> >> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> >> Quad and Dual SPI Transfers use all available data lines
> >> (incl. MOSI/MISO), hence they must be half duplex. Add a
> >> check that verify that.
> >
> > This is surprising to me - I had expected that there would be
> > extra signals that would be used for these modes, not that
> > the opposite direction data line would be one of the ones
> > being reused.  On the other hand if this is what all the
> > flash chips do then it would seem reasonable that controllers
> > do the same.  Can you clarify please?
> 
> Dual SPI works by aggregating the MOSI and MISO lines for 2-bit
> unidirectional transfers.
> Quad SPI aggregates MOSI, MISO, and 2 additional lines for 4-bit
> unidirectional transfers.

Does it help to only use the MOSI/MISO names for single data line
transfers, and to explicitly mention the fact that multi data
line transfers change the signals' meaning to "IO0 .. IO3", and
that these "IOn" lines are used in half duplex ways?

> Hence Dual SPI uses the traditional 4-wire wiring, while Quad
> SPI uses 6-wire.

Be careful!  Just because the number of wires is identical, I
would not want to refer to it as "the traditional 4-wire wiring"
in the dual data line case.  Strictly speaking it's not that you
connect MISO+MOSI with MISO+MOSI, insted you connect IO0+IO1 with
IO0+IO1.  It just happens that the same pins are re-used, while
the signals do change their meaning.

Now add unidirectional transmitters (amplifiers and/or level
shifters) to the picture instead of mere wires, and you cannot
run all modes across these connections any longer as you may
assume at the moment.

IO2 and IO3 in quad data line mode do change the meaning of the
pins which they re-purpose, too (that's write protect and hold
usually).  Which is why the features of the SPI controller and
the flash chip alone may not be sufficient to determine which
modes are available, as the board may add constraints as well.

> SPI FLASH chips handle it this way, and the Renesas QSPI
> controller in the r8a7790/7791 SoCs, too.
> 
> Typically the first transfer in a message is a Single Transfer
> (e.g. read data command), while subsequent transfers can be
> Dual or Quad Transfers (e.g.  the actual data read from the
> FLASH).

Please note that subsequent transfers may be single data line
transfers as well. :)


BTW am I trying to be strict about the above implicit assumption
of "dual/quad" meaning the number of data lines.  To not confuse
them with dual data rate transfers, which are orthogonal to the
number of data lines in use, and are available in chips as well
(see the S25FL256S data sheet that was referenced in the Quad-SPI
discussion earlier).

Let's be clear from the beginning, to not have to cleanup or
guess afterwards.


virtually yours
Gerhard Sittig
Geert Uytterhoeven Jan. 22, 2014, 8:26 p.m. UTC | #5
On Wed, Jan 22, 2014 at 8:19 PM, Gerhard Sittig <gsi@denx.de> wrote:
> On Tue, Jan 21, 2014 at 21:06 +0100, Geert Uytterhoeven wrote:
>> On Tue, Jan 21, 2014 at 7:21 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Tue, Jan 21, 2014 at 04:10:08PM +0100, Geert Uytterhoeven wrote:
>> >> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> >> Quad and Dual SPI Transfers use all available data lines
>> >> (incl. MOSI/MISO), hence they must be half duplex. Add a
>> >> check that verify that.
>> >
>> > This is surprising to me - I had expected that there would be
>> > extra signals that would be used for these modes, not that
>> > the opposite direction data line would be one of the ones
>> > being reused.  On the other hand if this is what all the
>> > flash chips do then it would seem reasonable that controllers
>> > do the same.  Can you clarify please?
>>
>> Dual SPI works by aggregating the MOSI and MISO lines for 2-bit
>> unidirectional transfers.
>> Quad SPI aggregates MOSI, MISO, and 2 additional lines for 4-bit
>> unidirectional transfers.
>
> Does it help to only use the MOSI/MISO names for single data line
> transfers, and to explicitly mention the fact that multi data
> line transfers change the signals' meaning to "IO0 .. IO3", and
> that these "IOn" lines are used in half duplex ways?

Maybe. But both MOSI/MISO and IO0/IO1 are carried over the
same physical signal lines.

>> Hence Dual SPI uses the traditional 4-wire wiring, while Quad
>> SPI uses 6-wire.
>
> Be careful!  Just because the number of wires is identical, I
> would not want to refer to it as "the traditional 4-wire wiring"
> in the dual data line case.  Strictly speaking it's not that you
> connect MISO+MOSI with MISO+MOSI, insted you connect IO0+IO1 with
> IO0+IO1.  It just happens that the same pins are re-used, while
> the signals do change their meaning.

Sure you do, as typically the first transfer is a Single SPI write
to the slave, to tell it the next transfer will be Dual or Quad.

So you do connect both MISO+MOSI with MISO+MOSI, and
IO0+IO1 to IO0+IO1.

> Now add unidirectional transmitters (amplifiers and/or level
> shifters) to the picture instead of mere wires, and you cannot
> run all modes across these connections any longer as you may
> assume at the moment.

That should be reflected in the DT, using the spi-tx-bus-width and
spi-rx-bus-width properties of the SPI slave device, right?

> IO2 and IO3 in quad data line mode do change the meaning of the
> pins which they re-purpose, too (that's write protect and hold
> usually).  Which is why the features of the SPI controller and
> the flash chip alone may not be sufficient to determine which
> modes are available, as the board may add constraints as well.

Same here.

>> SPI FLASH chips handle it this way, and the Renesas QSPI
>> controller in the r8a7790/7791 SoCs, too.
>>
>> Typically the first transfer in a message is a Single Transfer
>> (e.g. read data command), while subsequent transfers can be
>> Dual or Quad Transfers (e.g.  the actual data read from the
>> FLASH).
>
> Please note that subsequent transfers may be single data line
> transfers as well. :)

Yes they can ;-)

> BTW am I trying to be strict about the above implicit assumption
> of "dual/quad" meaning the number of data lines.  To not confuse
> them with dual data rate transfers, which are orthogonal to the
> number of data lines in use, and are available in chips as well
> (see the S25FL256S data sheet that was referenced in the Quad-SPI
> discussion earlier).
> Let's be clear from the beginning, to not have to cleanup or
> guess afterwards.

Sure, that's why I try to always write "Dual SPI Transfers" or "Quad
SPI Transfers".

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/spi.c b/drivers/spi/spi.c
index 7f77f82bf95e..4afcdb4851fa 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1672,6 +1672,7 @@  static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 		/* check transfer tx/rx_nbits:
 		 * 1. check the value matches one of single, dual and quad
 		 * 2. check tx/rx_nbits match the mode in spi_device
+		 * 3. check dual or quad is half duplex
 		 */
 		if (xfer->tx_buf) {
 			if (xfer->tx_nbits != SPI_NBITS_SINGLE &&
@@ -1684,6 +1685,8 @@  static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 			if ((xfer->tx_nbits == SPI_NBITS_QUAD) &&
 				!(spi->mode & SPI_TX_QUAD))
 				return -EINVAL;
+			if (xfer->tx_nbits > SPI_NBITS_SINGLE && xfer->rx_buf)
+				return -EINVAL;
 		}
 		/* check transfer rx_nbits */
 		if (xfer->rx_buf) {
@@ -1697,6 +1700,8 @@  static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 			if ((xfer->rx_nbits == SPI_NBITS_QUAD) &&
 				!(spi->mode & SPI_RX_QUAD))
 				return -EINVAL;
+			if (xfer->rx_nbits > SPI_NBITS_SINGLE && xfer->tx_buf)
+				return -EINVAL;
 		}
 	}