[v2,2/2] dt-bindings: Add documentation for SPI daisy chain driver.
diff mbox series

Message ID 20200706092247.20740-2-adrian.fiergolski@fastree3d.com
State New
Headers show
Series
  • [v2,1/2] spi: Add the SPI daisy chain support.
Related show

Commit Message

Adrian Fiergolski July 6, 2020, 9:22 a.m. UTC
Add documentation for SPI daisy chain driver.

Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
---
 .../bindings/spi/spi-daisy_chain.txt          | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-daisy_chain.txt

Comments

Geert Uytterhoeven July 6, 2020, 3:10 p.m. UTC | #1
Hi Adrian,

On Mon, Jul 6, 2020 at 11:23 AM Adrian Fiergolski
<adrian.fiergolski@fastree3d.com> wrote:
> Add documentation for SPI daisy chain driver.
>
> Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt
> @@ -0,0 +1,56 @@
> +spi-daisy_chain : The driver handling SPI daisy chains.
> +-----------------------------------------------------------
> +
> +Required properties:
> +- compatible           : Should be "spi,daisy_chain"
> +- reg                  : Chip select assigned to the chain
> +
> +  For the SPI devices on a common SPI chain - nodes of daisy_chain):
> +- spi-daisy-chain-len  : Length (in bytes) of the SPI transfer,
> +                        when the SPI device is part of a device chain.
> +- spi-daisy-chain-noop : Byte string of no-operation command which should
> +                        be send when device is not addressed during the
> +                        given SPI transfer

The above two properties are device-specific, and the same for all
devices of the same type, thus leading to duplication.
Hence I think this should not be specified in DT, but instead handled
by the driver.  I.e. for Linux, you would retrieve this from struct
spi_device, as filled in by the slave driver.

> +
> +Optional properties:
> +  (for the SPI devices on a common SPI chain (nodes of daisy_chain):
> +- spi-daisy-chain-bits_per_word : no-operation transfers involve
> +                                  one or more words; word sizes like
> +                                 eight or 12 bits are common.
> +                                 In-memory wordsizes are powers of two
> +                                 bytes (e.g. 20 bit samples use 32 bits).
> +                                 If not defined, it is assumed to be 8.

Same here.

> +Example:
> +
> +       daisy_chain0: daisy_chain@0 {
> +               compatible = "spi,daisy_chain";
> +               spi-max-frequency = <10000000>;
> +               reg = <0>;
> +
> +               dac0: ltc2632@0 {
> +                     compatible = "lltc,ltc2634-l12";
> +                     spi-daisy-chain-len = <4>;
> +                     spi-daisy-chain-noop = [00 F0 00 00];
> +               };
> +               dac1: ltc2632@1 {
> +                     compatible = "lltc,ltc2634-l12";
> +                     spi-daisy-chain-len = <4>;
> +                     spi-daisy-chain-noop = [00 F0 00 00];
> +               };
> +               dac2: ltc2632@2 {
> +                     compatible = "lltc,ltc2634-l12";
> +                     spi-daisy-chain-len = <4>;
> +                     spi-daisy-chain-noop = [00 F0 00 00];
> +               };
> +       };

Gr{oetje,eeting}s,

                        Geert
Adrian Fiergolski July 6, 2020, 3:19 p.m. UTC | #2
Hi Geert,

Thank you for your reply.

On 06.07.2020 17:10, Geert Uytterhoeven wrote:
> Hi Adrian,
>
> On Mon, Jul 6, 2020 at 11:23 AM Adrian Fiergolski
> <adrian.fiergolski@fastree3d.com> wrote:
>> Add documentation for SPI daisy chain driver.
>>
>> Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
> Thanks for your patch!
>
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt
>> @@ -0,0 +1,56 @@
>> +spi-daisy_chain : The driver handling SPI daisy chains.
>> +-----------------------------------------------------------
>> +
>> +Required properties:
>> +- compatible           : Should be "spi,daisy_chain"
>> +- reg                  : Chip select assigned to the chain
>> +
>> +  For the SPI devices on a common SPI chain - nodes of daisy_chain):
>> +- spi-daisy-chain-len  : Length (in bytes) of the SPI transfer,
>> +                        when the SPI device is part of a device chain.
>> +- spi-daisy-chain-noop : Byte string of no-operation command which should
>> +                        be send when device is not addressed during the
>> +                        given SPI transfer
> The above two properties are device-specific, and the same for all
> devices of the same type, thus leading to duplication.
> Hence I think this should not be specified in DT, but instead handled
> by the driver.  I.e. for Linux, you would retrieve this from struct
> spi_device, as filled in by the slave driver.
The problem is that then this patch would not be compatible out of the
box with all SPI devices (as it's now) and would require rewrite (adding
this extra information in the spi_driver struct) of all SPI drivers
which support daisy chain.

Regards,

Adrian
Geert Uytterhoeven July 6, 2020, 3:32 p.m. UTC | #3
Hi Adrian,

On Mon, Jul 6, 2020 at 5:19 PM Adrian Fiergolski
<adrian.fiergolski@fastree3d.com> wrote:
> On 06.07.2020 17:10, Geert Uytterhoeven wrote:
> > On Mon, Jul 6, 2020 at 11:23 AM Adrian Fiergolski
> > <adrian.fiergolski@fastree3d.com> wrote:
> >> Add documentation for SPI daisy chain driver.
> >>
> >> Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
> > Thanks for your patch!
> >
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt
> >> @@ -0,0 +1,56 @@
> >> +spi-daisy_chain : The driver handling SPI daisy chains.
> >> +-----------------------------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible           : Should be "spi,daisy_chain"
> >> +- reg                  : Chip select assigned to the chain
> >> +
> >> +  For the SPI devices on a common SPI chain - nodes of daisy_chain):
> >> +- spi-daisy-chain-len  : Length (in bytes) of the SPI transfer,
> >> +                        when the SPI device is part of a device chain.
> >> +- spi-daisy-chain-noop : Byte string of no-operation command which should
> >> +                        be send when device is not addressed during the
> >> +                        given SPI transfer
> > The above two properties are device-specific, and the same for all
> > devices of the same type, thus leading to duplication.
> > Hence I think this should not be specified in DT, but instead handled
> > by the driver.  I.e. for Linux, you would retrieve this from struct
> > spi_device, as filled in by the slave driver.
> The problem is that then this patch would not be compatible out of the
> box with all SPI devices (as it's now) and would require rewrite (adding
> this extra information in the spi_driver struct) of all SPI drivers
> which support daisy chain.

That's correct.
However, that information would need to be added to each driver only once.
With your proposal, it has to be added to all affected nodes of all DTSes
of all users.

Gr{oetje,eeting}s,

                        Geert
Mark Brown July 6, 2020, 4:22 p.m. UTC | #4
On Mon, Jul 06, 2020 at 05:32:51PM +0200, Geert Uytterhoeven wrote:

> However, that information would need to be added to each driver only once.
> With your proposal, it has to be added to all affected nodes of all DTSes
> of all users.

Right, these are fixed properties of the silicon which we know simply
from knowing which device we have - there is no need to put them in DT
at all.
Adrian Fiergolski July 7, 2020, 9:55 a.m. UTC | #5
Hi Geert and Mark,

Thank you for your comments. I will try to address them in the next replies.

On 06.07.2020 18:22, Mark Brown wrote:
> On Mon, Jul 06, 2020 at 05:32:51PM +0200, Geert Uytterhoeven wrote:
>
>> However, that information would need to be added to each driver only once.
>> With your proposal, it has to be added to all affected nodes of all DTSes
>> of all users.
> Right, these are fixed properties of the silicon which we know simply
> from knowing which device we have - there is no need to put them in DT
> at all.

I see. I agree with you. My concern was just the lack of compatibility
with the existing drivers. I will try to add daisy_chain information to
spi_driver struct in version v3 of the patch.

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt b/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt
new file mode 100644
index 000000000000..1e5b046dda83
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-daisy_chain.txt
@@ -0,0 +1,56 @@ 
+spi-daisy_chain : The driver handling SPI daisy chains.
+-----------------------------------------------------------
+
+Required properties:
+- compatible		: Should be "spi,daisy_chain"
+- reg			: Chip select assigned to the chain
+
+  For the SPI devices on a common SPI chain - nodes of daisy_chain):
+- spi-daisy-chain-len  : Length (in bytes) of the SPI transfer,
+		         when the SPI device is part of a device chain.
+- spi-daisy-chain-noop : Byte string of no-operation command which should
+		         be send when device is not addressed during the
+			 given SPI transfer
+
+Optional properties:
+  (for the SPI devices on a common SPI chain (nodes of daisy_chain):
+- spi-daisy-chain-bits_per_word : no-operation transfers involve
+                                  one or more words; word sizes like
+				  eight or 12 bits are common.
+				  In-memory wordsizes are powers of two
+				  bytes (e.g. 20 bit samples use 32 bits).
+				  If not defined, it is assumed to be 8.
+
+The daisy chain is a virtual device represented as a regular SPI device. Its
+nodes define physical devices available on the chain. The order of the nodes
+defines the order of the physical devices on the chain: MOSI pin of a device
+represented by the first node is the last one on the MOSI daisy chain. The
+daisy-chain functionality is transparent to the drivers of the physical devices
+on the chain. All nodes share SPI mode, chip select and a max speed of the
+virtual daisy chain device. Once one of the physical devices is being accessed,
+the spi-daisy_chain driver combines this data with no-operation commands of all
+other devices on the chain.
+
+Example:
+
+	daisy_chain0: daisy_chain@0 {
+	        compatible = "spi,daisy_chain";
+		spi-max-frequency = <10000000>;
+		reg = <0>;
+
+		dac0: ltc2632@0 {
+	              compatible = "lltc,ltc2634-l12";
+		      spi-daisy-chain-len = <4>;
+		      spi-daisy-chain-noop = [00 F0 00 00];
+		};
+		dac1: ltc2632@1 {
+	              compatible = "lltc,ltc2634-l12";
+		      spi-daisy-chain-len = <4>;
+		      spi-daisy-chain-noop = [00 F0 00 00];
+		};
+		dac2: ltc2632@2 {
+	              compatible = "lltc,ltc2634-l12";
+		      spi-daisy-chain-len = <4>;
+		      spi-daisy-chain-noop = [00 F0 00 00];
+		};
+	};