> > Hi, Pekon > > 2013/8/26 Gupta, Pekon <pekon@xxxxxx>: > >> Hi, Pekon > >> > >> 2013/8/26 Gupta, Pekon <pekon@xxxxxx>: > >> >> > >> >> Signed-off-by: wangyuhang <wangyuhang2014@xxxxxxxxx> > >> >> --- > >> >> 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. > > > > OK, I got it. Logically it is better. But there are some difficulties > in Implementation. > 1. The property (spi-quad spi-dual spi-single) should be optional, > user can don't set any of these three properties. Then spi-3wire can > not be specified. Or if you want to set these properties as required, > all the spi slave (at least DT) should be changed. [Pekon]: Nope you don't need to change anything. Even today, there is an explicitly DT binding called 'spi-3wire' to enable this mode. Instead of it I'm proposing to use 'spi-tx-max-nbit = 0' or 'spi-rx-max-nbit = 0', and deprecate 'spi-3wire'. So only those DTS need to be updated which use 'spi-3wire' And, I havn't found any DTS with spi-3wire :-) grep -i 3wire arch/*/boot/dts/* -r Refer: Documentation/devicetree/bindings/spi/spi-bus.txt - spi-3wire - (optional) Empty property indicating device requires 3-wire mode. > 2. Add property like (spi-quad spi-dual spi-single) will make the > check in spi framework a little complicated. > example: > 1)if one of these three is duplex, the other can not be set. > 2)there can not be the same value in either of the three. > 3)all three can not be set at the same time. > ..... > If in the future, more wires transfer is supported, you have to add > properties, then the check will be more complex. > So I still need some good aproaches to specify spi-3wire. > [Pekon]: Anyways this proposal is invalid, bcoz I mis-understood ur patch earlier. So no issues here.. > > > >> >> 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 ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f