[v2,01/11] dt-bindings: spi: allow expressing DTR capability

Allow spi devices to express DTR receive and transmit capabilities via
the properties "spi-rx-dtr" and "spi-tx-dtr".

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 .../devicetree/bindings/spi/spi-controller.yaml        | 10 ++++++++++
 1 file changed, 10 insertions(+)

On Wed, 26 Feb 2020 15:06:53 +0530
Pratyush Yadav <p.yadav@ti.com> wrote:

> Allow spi devices to express DTR receive and transmit capabilities via
> the properties "spi-rx-dtr" and "spi-tx-dtr".

Is the RX/TX granularity really useful?

> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  .../devicetree/bindings/spi/spi-controller.yaml        | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> index 1e0ca6ccf64b..7a84debed213 100644
> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -120,6 +120,11 @@ patternProperties:
>          description:
>            Delay, in microseconds, after a read transfer.
>  
> +      spi-rx-dtr:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description:
> +          Device supports receiving in DTR mode.
> +
>        spi-tx-bus-width:
>          allOf:
>            - $ref: /schemas/types.yaml#/definitions/uint32
> @@ -132,6 +137,11 @@ patternProperties:
>          description:
>            Delay, in microseconds, after a write transfer.
>  
> +      spi-tx-dtr:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description:
> +          Device supports transmitting in DTR mode.
> +
>      required:
>        - compatible
>        - reg

On Thu, Feb 27, 2020 at 05:11:47PM +0100, Boris Brezillon wrote:
> Pratyush Yadav <p.yadav@ti.com> wrote:

> > Allow spi devices to express DTR receive and transmit capabilities via
> > the properties "spi-rx-dtr" and "spi-tx-dtr".

> Is the RX/TX granularity really useful?

It's what we do for other properties, and if this is anything like the
other things adding extra wiring you can't assume that the ability to
use the feature for TX implies RX.

Hi Pratyush,

On Wed, Feb 26, 2020 at 10:37 AM Pratyush Yadav <p.yadav@ti.com> wrote:
> Allow spi devices to express DTR receive and transmit capabilities via
> the properties "spi-rx-dtr" and "spi-tx-dtr".
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> @@ -120,6 +120,11 @@ patternProperties:
>          description:
>            Delay, in microseconds, after a read transfer.
>
> +      spi-rx-dtr:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description:
> +          Device supports receiving in DTR mode.

Please explain "DTR" in the document, at least once, e.g.

    Double Transfer Rate (DTR).

Gr{oetje,eeting}s,

                        Geert

Hi Mark,

On Thu, Feb 27, 2020 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:
> On Thu, Feb 27, 2020 at 05:11:47PM +0100, Boris Brezillon wrote:
> > Pratyush Yadav <p.yadav@ti.com> wrote:
>
> > > Allow spi devices to express DTR receive and transmit capabilities via
> > > the properties "spi-rx-dtr" and "spi-tx-dtr".
>
> > Is the RX/TX granularity really useful?
>
> It's what we do for other properties, and if this is anything like the
> other things adding extra wiring you can't assume that the ability to
> use the feature for TX implies RX.

Double Transfer Rate uses the same wire.
But as you sample at both the rising and the falling edges of the clock, this
makes the cpha setting meaningless for such transfers, I think ;-)

However, as the future may bring us QDR, perhaps this should not be a
boolean flag, but an integer value?
Cfr. spi-tx-bus-width vs. the original spi-tx-dual/spi-tx-quad proposal.

What would be a good name (as we only need one)? spi-data-phases?

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

On Thu, Feb 27, 2020 at 05:40:31PM +0100, Geert Uytterhoeven wrote:
> On Thu, Feb 27, 2020 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:

> > It's what we do for other properties, and if this is anything like the
> > other things adding extra wiring you can't assume that the ability to
> > use the feature for TX implies RX.

> Double Transfer Rate uses the same wire.

But is it still on either the TX or RX signals?

> But as you sample at both the rising and the falling edges of the clock, this
> makes the cpha setting meaningless for such transfers, I think ;-)

Might affect what the first bit is possibly?

> However, as the future may bring us QDR, perhaps this should not be a
> boolean flag, but an integer value?
> Cfr. spi-tx-bus-width vs. the original spi-tx-dual/spi-tx-quad proposal.

> What would be a good name (as we only need one)? spi-data-phases?

Sounds reasonable, apart from the increasingly vague connection with
something that's recognizably SPI :P

Hi Mark,

On Thu, Feb 27, 2020 at 5:44 PM Mark Brown <broonie@kernel.org> wrote:
> On Thu, Feb 27, 2020 at 05:40:31PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 27, 2020 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:
> > > It's what we do for other properties, and if this is anything like the
> > > other things adding extra wiring you can't assume that the ability to
> > > use the feature for TX implies RX.
>
> > Double Transfer Rate uses the same wire.
>
> But is it still on either the TX or RX signals?

E.g. good old Spansion S25FL512S supports single/dual/quad DDR, but
apparently only for read, not write.
Other FLASHes may support both directions. I guess.

> > But as you sample at both the rising and the falling edges of the clock, this
> > makes the cpha setting meaningless for such transfers, I think ;-)
>
> Might affect what the first bit is possibly?

Quoting the manual for the above part:

4.1.2
Double Data Rate (DDR)
Mode 0 and Mode 3 are also supported for DDR commands. In DDR
commands, the instruction bits are
always latched on the rising edge of clock, the same as in SDR
commands. However, the address and input
data that follow the instruction are latched on both the rising and
falling edges of SCK. The first address bit is
latched on the first rising edge of SCK following the falling edge at
the end of the last instruction bit. The first
bit of output data is driven on the falling edge at the end of the
last access latency (dummy) cycle.
SCK cycles are measured (counted) in the same way as in SDR commands,
from one falling edge of SCK to
the next falling edge of SCK. In mode 0 the beginning of the first SCK
cycle in a command is measured from
the falling edge of CS# to the first falling edge of SCK because SCK
is already low at the beginning of a
command.

> > However, as the future may bring us QDR, perhaps this should not be a
> > boolean flag, but an integer value?
> > Cfr. spi-tx-bus-width vs. the original spi-tx-dual/spi-tx-quad proposal.
>
> > What would be a good name (as we only need one)? spi-data-phases?
>
> Sounds reasonable, apart from the increasingly vague connection with
> something that's recognizably SPI :P

Yeah, especially Octal and Hyper modes are far from serial ;-)

Gr{oetje,eeting}s,

                        Geert

On Thu, 27 Feb 2020 16:44:25 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 27, 2020 at 05:40:31PM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 27, 2020 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:  
> 
> > > It's what we do for other properties, and if this is anything like the
> > > other things adding extra wiring you can't assume that the ability to
> > > use the feature for TX implies RX.  
> 
> > Double Transfer Rate uses the same wire.  
> 
> But is it still on either the TX or RX signals?

There's no separate RX/TX pins when using xD-xD-xD modes (pins switch
from RX to TX) and I doubt DTR will ever be used on single SPI.

> 
> > But as you sample at both the rising and the falling edges of the clock, this
> > makes the cpha setting meaningless for such transfers, I think ;-)  
> 
> Might affect what the first bit is possibly?
> 
> > However, as the future may bring us QDR, perhaps this should not be a
> > boolean flag, but an integer value?
> > Cfr. spi-tx-bus-width vs. the original spi-tx-dual/spi-tx-quad proposal.  
> 
> > What would be a good name (as we only need one)? spi-data-phases?  
> 
> Sounds reasonable, apart from the increasingly vague connection with
> something that's recognizably SPI :P

Or maybe we should refrain from adding a new flag and wait a bit to see
if this DTR mode is actually used for regular SPI transfers (AKA not
spi-mem) :-).

Hi Geert,

On 27/02/20 05:29PM, Geert Uytterhoeven wrote:
> Hi Pratyush,
> 
> On Wed, Feb 26, 2020 at 10:37 AM Pratyush Yadav <p.yadav@ti.com> wrote:
> > Allow spi devices to express DTR receive and transmit capabilities via
> > the properties "spi-rx-dtr" and "spi-tx-dtr".
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > @@ -120,6 +120,11 @@ patternProperties:
> >          description:
> >            Delay, in microseconds, after a read transfer.
> >
> > +      spi-rx-dtr:
> > +        $ref: /schemas/types.yaml#/definitions/flag
> > +        description:
> > +          Device supports receiving in DTR mode.
> 
> Please explain "DTR" in the document, at least once, e.g.
> 
>     Double Transfer Rate (DTR).

Will do.
 
> 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

On 27/02/20 06:03PM, Geert Uytterhoeven wrote:
> Hi Mark,
> 
> On Thu, Feb 27, 2020 at 5:44 PM Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Feb 27, 2020 at 05:40:31PM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Feb 27, 2020 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:
> > > > It's what we do for other properties, and if this is anything like the
> > > > other things adding extra wiring you can't assume that the ability to
> > > > use the feature for TX implies RX.
> >
> > > Double Transfer Rate uses the same wire.
> >
> > But is it still on either the TX or RX signals?
> 
> E.g. good old Spansion S25FL512S supports single/dual/quad DDR, but
> apparently only for read, not write.
> Other FLASHes may support both directions. I guess.

The flash datasheet says under section 9.4 (Read Memory Array Commands):

  Some commands transfer address and data on both the rising edge and 
  falling edge of SCK. These are called Double Data Rate (DDR) commands.

Since the address is transferred in DDR mode, both Tx and Rx signals use 
DDR.

So, unless we have a flash that supports a mode like 1S-1S-8D, we don't 
really need two properties.