Patchwork [V2,2/2] spi: dual and quad support(device tree)

login
register
mail settings
Submitter 王宇航
Date Aug. 27, 2013, 12:50 a.m.
Message ID <1377564603-4030-2-git-send-email-wangyuhang2014@gmail.com>
Download mbox | patch
Permalink /patch/270003/
State New
Headers show

Comments

王宇航 - Aug. 27, 2013, 12:50 a.m.
Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
Modify the related dt document(spi-bus.txt)
 spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
 spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
[1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538

Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)
pekon gupta - Aug. 27, 2013, 4:03 a.m.
> 
> Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
> Modify the related dt document(spi-bus.txt)
>  spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
>  spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
> Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
> [1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
> Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538
> 
> Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt |   16
> ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt
> b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 296015e..211336c 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -55,6 +55,22 @@ contain the following properties.
>      		chip select active high
>  - spi-3wire       - (optional) Empty property indicating device requires
>      		    3-wire mode.
> +- spi-tmax-nbits  - (optional) Max number of bits slave will use for
> +    		    MOSI(writting)
> +- spi-rmax-nbits  - (optional) Max number of bits slave will use 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-tmax-nbits = <4>;
> +spi-rmax-nbits = <2>;
> +
> +Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is only
> +1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits or spi-rmax-
> nbits,
> +spi_device mode will be set in single(1 wire) as default. Another point, if
> +property:spi-3wire is set, spi-t/rmax-nbits is forbidden to set to <2 or 4>,
> +otherwise, an errro will return.
> 
>  If a gpio chipselect is used for the SPI slave the gpio number will be passed
>  via the cs_gpio
> --
> 1.7.9.5

Acked-by: Pekon Gupta <pekon@ti.com>

with regards, pekon
Stephen Warren - Aug. 30, 2013, 9:26 p.m.
On 08/26/2013 06:50 PM, wangyuhang wrote:
> Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
> Modify the related dt document(spi-bus.txt)
>  spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
>  spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
> Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
> [1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
> Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538

> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt

> +- spi-tmax-nbits  - (optional) Max number of bits slave will use for
> +    		    MOSI(writting)
> +- spi-rmax-nbits  - (optional) Max number of bits slave will use for
> +    		    MISO(reading)

I would suggest the property names

spi-tx-num-wires, spi-rx-num-wires

or:

spi-tx-bus-width, spi-rx-bus-width (this option from Tomasz Figa on IRC)

Those name alone self-document their purpose much more clearly to me.

I would also change the descriptions a bit, resulting in the following
overall:

==========
- spi-tx-num-wires  - (optional) The number of data wires that
                      transfer data from the SPI controller to the
                      SPI device. Defaults to 1 if not present.
- spi-rx-num-wires  - (optional) The number of data wires that
                      transfer data from the SPI device to the
                      SPI controller. Defaults to 1 if not present.
==========

> +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-tmax-nbits = <4>;
> +spi-rmax-nbits = <2>;
> +
> +Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is only
> +1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits or spi-rmax-nbits,
> +spi_device mode will be set in single(1 wire) as default. Another point, if
> +property:spi-3wire is set, spi-t/rmax-nbits is forbidden to set to <2 or 4>,
> +otherwise, an errro will return.

I think most of that explanation can be removed if you use the text I
wrote above.

I'm not sure you should ban spi-3wire if those properties are specified;
can't all those properties be present together, if the num-wires are set
to 1? Actually, isn't spi-3write the default; what does that property mean?
Tomasz Figa - Aug. 30, 2013, 9:30 p.m.
Hi,

[Ccing DT binding maintainers]

On Tuesday 27 of August 2013 08:50:03 wangyuhang wrote:
> Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
> Modify the related dt document(spi-bus.txt)
>  spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
>  spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
> Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
> [1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
> Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538
> 
> Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-bus.txt |   16
> ++++++++++++++++ 1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt
> b/Documentation/devicetree/bindings/spi/spi-bus.txt index
> 296015e..211336c 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -55,6 +55,22 @@ contain the following properties.
>      		chip select active high
>  - spi-3wire       - (optional) Empty property indicating device
> requires 3-wire mode.
> +- spi-tmax-nbits  - (optional) Max number of bits slave will use for
> +    		    MOSI(writting)
> +- spi-rmax-nbits  - (optional) Max number of bits slave will use for
> +    		    MISO(reading)

May I ask for more human-readable names, please?

If we look around, there are already devices using bus-width property, so 
for consistency we could use tx-bus-width and rx-bus-width here or even a 
single bus-width property taking two cells, first tx and second rx?

Let's hear others' opinion on this as well.

Also the properties should be telling information about hardware, so 
probably in this case the meaning would be the number of wires that are 
physically wired on the board.

> +

One or two sentences about what this dual/quad thing is about would be 
nice here, e.g.

	Some SPI controllers and devices support Dual and/or Quad SPI
	mode, which is [...]. It allows [...], etc.

> +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-tmax-nbits = <4>;
> +spi-rmax-nbits = <2>;
> +
> +Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is
> only +1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits
> or spi-rmax-nbits, +spi_device mode will be set in single(1 wire) as
> default. Another point, if +property:spi-3wire is set, spi-t/rmax-nbits
> is forbidden to set to <2 or 4>, +otherwise, an errro will return.

typo: errro

Also an error or "returning an error" is an OS specific term. This 
statement should rather be something along

	Dual/Quad mode is not allowed when 3-wire mode is used.

Best regards,
Tomasz
Mark Brown - Aug. 30, 2013, 10:23 p.m.
On Fri, Aug 30, 2013 at 03:26:44PM -0600, Stephen Warren wrote:

> I would suggest the property names

> spi-tx-num-wires, spi-rx-num-wires

> or:

> spi-tx-bus-width, spi-rx-bus-width (this option from Tomasz Figa on IRC)

> Those name alone self-document their purpose much more clearly to me.

These both seem like good suggestions.
王宇航 - Sept. 1, 2013, 8:05 a.m.
Hi, Stephen

2013/8/31 Stephen Warren <swarren@wwwdotorg.org>:
> On 08/26/2013 06:50 PM, wangyuhang wrote:
>> Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
>> Modify the related dt document(spi-bus.txt)
>>  spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
>>  spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
>> Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
>> [1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
>> Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538
>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>
>> +- spi-tmax-nbits  - (optional) Max number of bits slave will use for
>> +                 MOSI(writting)
>> +- spi-rmax-nbits  - (optional) Max number of bits slave will use for
>> +                 MISO(reading)
>
> I would suggest the property names
>
> spi-tx-num-wires, spi-rx-num-wires
>
> or:
>
> spi-tx-bus-width, spi-rx-bus-width (this option from Tomasz Figa on IRC)
>
> Those name alone self-document their purpose much more clearly to me.
>
> I would also change the descriptions a bit, resulting in the following
> overall:
>
> ==========
> - spi-tx-num-wires  - (optional) The number of data wires that
>                       transfer data from the SPI controller to the
>                       SPI device. Defaults to 1 if not present.
> - spi-rx-num-wires  - (optional) The number of data wires that
>                       transfer data from the SPI device to the
>                       SPI controller. Defaults to 1 if not present.
> ==========
>

OK, that seems better.

>> +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-tmax-nbits = <4>;
>> +spi-rmax-nbits = <2>;
>> +
>> +Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is only
>> +1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits or spi-rmax-nbits,
>> +spi_device mode will be set in single(1 wire) as default. Another point, if
>> +property:spi-3wire is set, spi-t/rmax-nbits is forbidden to set to <2 or 4>,
>> +otherwise, an errro will return.
>
> I think most of that explanation can be removed if you use the text I
> wrote above.
>
> I'm not sure you should ban spi-3wire if those properties are specified;
> can't all those properties be present together, if the num-wires are set
> to 1? Actually, isn't spi-3write the default; what does that property mean?

Property spi-3write means some controllers only have cs, clk and
sda(SI/SO shared) 3 wires. So if spi-3wire is set, then dual and quad
set should be forbidden. I didn't mean that spi-3wire can't be present
together with spi-tx/rx-num-wires, if set to <1> it's OK. Just can't
be <2 | 4>(dual and quad).
王宇航 - Sept. 1, 2013, 8:14 a.m.
Hi, Tomasz

2013/8/31 Tomasz Figa <tomasz.figa@gmail.com>:
> Hi,
>
> [Ccing DT binding maintainers]
>
> On Tuesday 27 of August 2013 08:50:03 wangyuhang wrote:
>> Add spi-tmax-nbits and spi-rmax-nbits for spi slave node.
>> Modify the related dt document(spi-bus.txt)
>>  spi-tmax-nbits:Max number of bits slave will use for MOSI(writting)
>>  spi-rmax-nbits:Max number of bits slave will use for MISO(reading)
>> Support for spi-tx/rmax-nbits in SPI framework has been picked[1].
>> [1]: http://comments.gmane.org/gmane.linux.kernel.spi.devel/14420
>> Commit Id:f477b7fb13df2b843997559ff34e87d054ba6538
>>
>> Signed-off-by: wangyuhang <wangyuhang2014@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/spi/spi-bus.txt |   16
>> ++++++++++++++++ 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt
>> b/Documentation/devicetree/bindings/spi/spi-bus.txt index
>> 296015e..211336c 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> @@ -55,6 +55,22 @@ contain the following properties.
>>               chip select active high
>>  - spi-3wire       - (optional) Empty property indicating device
>> requires 3-wire mode.
>> +- spi-tmax-nbits  - (optional) Max number of bits slave will use for
>> +                 MOSI(writting)
>> +- spi-rmax-nbits  - (optional) Max number of bits slave will use for
>> +                 MISO(reading)
>
> May I ask for more human-readable names, please?
>
> If we look around, there are already devices using bus-width property, so
> for consistency we could use tx-bus-width and rx-bus-width here or even a
> single bus-width property taking two cells, first tx and second rx?
>
Yes, you are right. bus_width is really more widely used. So I will correct it.

> Let's hear others' opinion on this as well.
>
> Also the properties should be telling information about hardware, so
> probably in this case the meaning would be the number of wires that are
> physically wired on the board.
>
>> +
>
> One or two sentences about what this dual/quad thing is about would be
> nice here, e.g.
>
>         Some SPI controllers and devices support Dual and/or Quad SPI
>         mode, which is [...]. It allows [...], etc.
>
OK, Thanks.

>> +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-tmax-nbits = <4>;
>> +spi-rmax-nbits = <2>;
>> +
>> +Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is
>> only +1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits
>> or spi-rmax-nbits, +spi_device mode will be set in single(1 wire) as
>> default. Another point, if +property:spi-3wire is set, spi-t/rmax-nbits
>> is forbidden to set to <2 or 4>, +otherwise, an errro will return.
>
> typo: errro
>
> Also an error or "returning an error" is an OS specific term. This
> statement should rather be something along
>
>         Dual/Quad mode is not allowed when 3-wire mode is used.
>
Got it, Thanks a lot.

Patch

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 296015e..211336c 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -55,6 +55,22 @@  contain the following properties.
     		chip select active high
 - spi-3wire       - (optional) Empty property indicating device requires
     		    3-wire mode.
+- spi-tmax-nbits  - (optional) Max number of bits slave will use for
+    		    MOSI(writting)
+- spi-rmax-nbits  - (optional) Max number of bits slave will use 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-tmax-nbits = <4>;
+spi-rmax-nbits = <2>;
+
+Now the value that spi-tmax-nbits and spi-rmax-nbits can receive is only
+1(single), 2(dual) and 4(quad). If you don't set spi-tmax-nbits or spi-rmax-nbits,
+spi_device mode will be set in single(1 wire) as default. Another point, if
+property:spi-3wire is set, spi-t/rmax-nbits is forbidden to set to <2 or 4>,
+otherwise, an errro will return.
 
 If a gpio chipselect is used for the SPI slave the gpio number will be passed
 via the cs_gpio