[PATCH 1/5] spi: Spelling s/finised/finished/

[PATCH 1/5] spi: Spelling s/finised/finished/

[Geert Uytterhoeven]

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

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/spi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a86569e1f178..7f77f82bf95e 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -640,7 +640,7 @@ out:
  *
  * Called by SPI drivers using the core transfer_one_message()
  * implementation to notify it that the current interrupt driven
- * transfer has finised and the next one may be scheduled.
+ * transfer has finished and the next one may be scheduled.
  */
 void spi_finalize_current_transfer(struct spi_master *master)
 {
-- 
1.7.9.5

[PATCH v2 2/5] spi: Clarify transfer_one() w.r.t. spi_finalize_current_transfer()

[Geert Uytterhoeven]

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

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

 include/linux/spi/spi.h |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 21a7251d85ee..ba69940a6515 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -282,10 +282,12 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @prepare_message: set up the controller to transfer a single message,
  *                   for example doing DMA mapping.  Called from threaded
  *                   context.
- * @transfer_one: transfer a single spi_transfer. When the
- *	          driver is finished with this transfer it must call
- *	          spi_finalize_current_transfer() so the subsystem can issue
- *                the next transfer
+ * @transfer_one: transfer a single spi_transfer.
+ *                  - return 0 if the transfer is finished,
+ *                  - return 1 if the transfer is still in progress. When
+ *                    the driver is finished with this transfer it must
+ *                    call spi_finalize_current_transfer() so the subsystem
+ *                    can issue the next transfer
  * @unprepare_message: undo any work done by prepare_message().
  * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
  *	number. Any individual value may be -ENOENT for CS lines that
-- 
1.7.9.5

Re: [PATCH v2 2/5] spi: Clarify transfer_one() w.r.t. spi_finalize_current_transfer()

[Mark Brown]

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

On Tue, Jan 21, 2014 at 04:10:06PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Applied, thanks.  I suppose it should say an errno is also OK but I
guess that should be obvious enough.

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

[PATCH v2 3/5] spi: Correct set_cs() documentation

[Geert Uytterhoeven]

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

The documentation for spi_master.set_cs() says:

    assert or deassert chip select, true to assert

i.e. its "enable" parameter uses assertion-level logic.

This does not match the implementation of spi_set_cs(), which calls
spi_master.set_cs() with the wanted logic level of the chip select line,
which depends on the polarity of the chip select signal.

Correct the documentation to match the implementation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
v2: Changed documentation instead of implementation

 include/linux/spi/spi.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index ba69940a6515..a1d4ca290862 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -277,7 +277,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  *	queue so the subsystem notifies the driver that it may relax the
  *	hardware by issuing this call
- * @set_cs: assert or deassert chip select, true to assert.  May be called
+ * @set_cs: set the logic level of the chip select line.  May be called
  *          from interrupt context.
  * @prepare_message: set up the controller to transfer a single message,
  *                   for example doing DMA mapping.  Called from threaded
-- 
1.7.9.5

Re: [PATCH v2 3/5] spi: Correct set_cs() documentation

[Mark Brown]

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

On Tue, Jan 21, 2014 at 04:10:07PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> The documentation for spi_master.set_cs() says:
> 
>     assert or deassert chip select, true to assert

Applied, thanks.

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

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

[Geert Uytterhoeven]

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(+)

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;
 		}
 	}
 
-- 
1.7.9.5

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

[Mark Brown]

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

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?

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

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

[Geert Uytterhoeven]

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

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

[Gerhard Sittig]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

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

[Geert Uytterhoeven]

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

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

[Geert Uytterhoeven]

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

[PATCH 5/5] spi: rspi: Document support for Renesas QSPI in Kconfig

[Geert Uytterhoeven]

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

As of commit 5ce0ba88650f2606244a761d92e2b725f4ab3583 ("spi: rcar: add
Renesas QSPI support on RSPI") the rspi driver handles Renesas QSPI, too,
but this was not reflected in the Kconfig help text.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/spi/Kconfig |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5072b71baf5e..3aafc2cd5db0 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -376,10 +376,10 @@ config SPI_PXA2XX_PCI
 	def_tristate SPI_PXA2XX && PCI
 
 config SPI_RSPI
-	tristate "Renesas RSPI controller"
+	tristate "Renesas RSPI/QSPI controller"
 	depends on (SUPERH && SH_DMAE_BASE) || ARCH_SHMOBILE
 	help
-	  SPI driver for Renesas RSPI blocks.
+	  SPI driver for Renesas RSPI and QSPI blocks.
 
 config SPI_S3C24XX
 	tristate "Samsung S3C24XX series SPI"
-- 
1.7.9.5

Re: [PATCH 5/5] spi: rspi: Document support for Renesas QSPI in Kconfig

[Mark Brown]

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

On Tue, Jan 21, 2014 at 04:10:09PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> As of commit 5ce0ba88650f2606244a761d92e2b725f4ab3583 ("spi: rcar: add
> Renesas QSPI support on RSPI") the rspi driver handles Renesas QSPI, too,
> but this was not reflected in the Kconfig help text.

Applied, thanks.

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

Re: [PATCH 1/5] spi: Spelling s/finised/finished/

[Mark Brown]

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

On Tue, Jan 21, 2014 at 04:10:05PM +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 --]