[PATCH 1/2] spi: dual and quad support(device tree)

[Gupta, Pekon]

> Hi, Pekon
> 
> 2013/8/26 Gupta, Pekon <pekon at ti.com>:
> >>
> >> Signed-off-by: wangyuhang <wangyuhang2014 at gmail.com>
> >> ---
> >>  Documentation/devicetree/bindings/spi/spi-bus.txt |   14
> ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt
> >> b/Documentation/devicetree/bindings/spi/spi-bus.txt
> >> index 296015e..145ba96 100644
> >> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> >> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> >> @@ -55,6 +55,20 @@ contain the following properties.
> >>               chip select active high
> >>  - spi-3wire       - (optional) Empty property indicating device requires
> >>                   3-wire mode.
> >> +- spi-tx-nbits    - (optional) Number of bits used for MOSI(writting)
> >> +- spi-rx-nbits    - (optional) Number of bits used for MISO(reading)
> >> +
> >> +So if for example the slave has 4 wires for writting and 2 wires for
> reading,
> >> +and the spi-tx/rx-nbits property should be set as follows:
> >> +
> >> +spi-tx-nbits = <4>;
> >> +spi-rx-nbits = <2>;
> >
> > [Pekon]: there is a problem here...
> > spi-tx-nbit = <4> suggests that SPI device support QUAD writes, but it does
> > not indicate whether DUAL writes are supported by device or not.
> > So, In my view having either of the following implementation could help
> > in specifying capabilities independently and clearly.
> > *Implementation-1 Boolean*
> > spi-tx-quad = <true | false>
> > spi-tx-dual = <true | false>
> > spi-tx-single = <true | false>
> > Same way for Rx..
> > spi-rx-quad = <true | false>
> > spi-rx-dual = <true | false>
> > spi-rx-single = <true | false>
> >
> > *Implementation-2 Multi-option*
> > spi-quad = <tx-only | rx-only | duplex>
> > spi-dual = <tx-only | rx-only | duplex>
> > spi-single = <tx-only | rx-only | full-duplex | half-duplex>
> >
> Not exactly,  spi-tx-nbit = <4> suggests that SPI device will use QUAD
> writes, not support QUAD writes. There is no need to set what mode
> slave supports, user just set the certain mode slave will work in.
> 
[Pekon]: Wait.. I think there is some mis-match in our understandings.. 
(a) struct spi_device->mode determines capabilities spi_device can support
(b) And, when user (here m25p80 like flash driver) puts a spi_transfer, then
it specifies whether to use spi_transfer->tx_nbits = 4 or not.. 
And you are using DT binding to specify (a) correct ?

Now, how would you specify following configuration, using DT ?
	spi_device->mode = SPI_TX_SINGLE | SPI_TX_QUAD. (but not dual)
so that driver
- driver give error,
	when m25p80 driver uses spi_transfer->tx-nbits = 2
- But allow, 
	when m25p80 driver uses spi_transfer->tx-nbits = 1 or
	when m25p80 driver uses spi_transfer->tx-nbits = 4

> >> +
> >> +Now the value that spi-tx-nbits and spi-rx-nbits can receive is only
> >> +1(single), 2(dual) and 4(quad). If you don't set spi-tx-nbits or spi-rx-nbits,
> >> +spi_device mode will be set in single(1 wire) as default. Another point, if
> >> +property:spi-3wire is set, spi-tx/rx-nbits is forbidden to set to <2 or 4>,
> >> +otherwise, an errro will return.
> >>
> > [Pekon]: Also, instead of having separate binding for 'spi-3wire', it can be
> > moved under as spi-single = <half-duplex>.
> > Full-duplex = Tx and Rx operate on independent channels and
> concurrently.
> > Half-duplex = Tx and Rx use same bi-directional channel for transmission
> >         one by one
> >
> Actually, spi-3wire can be regarded as a part of spi-single, but
> corrected as what you said, there will be some inconvenient.
> 1,the driver that has already used spi-3wire need a big change.
> 2,there have to be a complexed check in spi framework if set like:
>  spi-quad = <tx-only | rx-only | duplex>
>  spi-dual = <tx-only | rx-only | duplex>
>  spi-single = <tx-only | rx-only | full-duplex | half-duplex>
> 
[Pekon]: No, I'm not asking you to update logic in all drivers,
just the DT bindings. Something like this..
@@ -872,46 +872,42 @@ static void of_register_spi_devices(struct spi_master *master)
 		/* Device DUAL/QUAD mode */
 		prop = of_get_property(nc, "spi-single", &len);
		if (prop == HALF_DUPLEX)
			spi->mode |= SPI_3WIRE;

So, you set the same spi->mode[SPI_3WIRE] bit, thus other drivers are not
impacted. And you can deprecate the older "spi-3wire" binding.

with regards, pekon

> >>  If a gpio chipselect is used for the SPI slave the gpio number will be
> passed
> >>  via the cs_gpio
> >> --
> >> 1.7.9.5
> >
> > with regards, pekon