| Message ID | 20200706092247.20740-2-adrian.fiergolski@fastree3d.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] spi: Add the SPI daisy chain support. | expand |
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
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
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
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.
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.
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]; + }; + };
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