diff mbox series

[1/2] dt-bindings: iio: accel: add binding documentation for ADIS16240

Message ID 20190902005938.7734-1-rodrigorsdc@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] dt-bindings: iio: accel: add binding documentation for ADIS16240 | expand

Commit Message

Rodrigo Carvalho Sept. 2, 2019, 12:59 a.m. UTC 
This patch add device tree binding documentation for ADIS16240.

Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@gmail.com>
---
I have doubt about what maintainer I may to put in that documentation. I
put Alexandru as maintainer because he reviewed my last patch on this
driver, so I think that he is a good candidate.
 .../bindings/iio/accel/adi,adis16240.yaml     | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml

Comments

Alexandru Ardelean Sept. 2, 2019, 1:05 p.m. UTC | #1 
On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote:
> This patch add device tree binding documentation for ADIS16240.
> 
> Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@gmail.com>
> ---
> I have doubt about what maintainer I may to put in that documentation. I
> put Alexandru as maintainer because he reviewed my last patch on this
> driver, so I think that he is a good candidate.

Fine to list me as maintainer for this.

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

>  .../bindings/iio/accel/adi,adis16240.yaml     | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> new file mode 100644
> index 000000000000..08019b51611c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADIS16240 Programmable Impact Sensor and Recorder driver
> +
> +maintainers:
> +  - Alexandru Ardelean <alexandru.ardelean@analog.com>
> +
> +description: |
> +  ADIS16240 Programmable Impact Sensor and Recorder driver that supports
> +  SPI interface.
> +    https://www.analog.com/en/products/adis16240.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adis16240
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        /* Example for a SPI device node */
> +        accelerometer@0 {
> +            compatible = "adi,adis16240";
> +            reg = <0>;
> +            spi-max-frequency = <2500000>;
> +            spi-cpol;
> +            spi-cpha;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +    };
Marcelo Schmitt Sept. 2, 2019, 5:14 p.m. UTC | #2 
Hi Rodrigo,

This dt doc looks overal fine IMHO.
I would just add some inline comments about the cpha and cpol
properties.

On 09/01, Rodrigo Carvalho wrote:
> This patch add device tree binding documentation for ADIS16240.
> 
> Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@gmail.com>
> ---
> I have doubt about what maintainer I may to put in that documentation. I
> put Alexandru as maintainer because he reviewed my last patch on this
> driver, so I think that he is a good candidate.
>  .../bindings/iio/accel/adi,adis16240.yaml     | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> new file mode 100644
> index 000000000000..08019b51611c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADIS16240 Programmable Impact Sensor and Recorder driver
> +
> +maintainers:
> +  - Alexandru Ardelean <alexandru.ardelean@analog.com>
> +
> +description: |
> +  ADIS16240 Programmable Impact Sensor and Recorder driver that supports
> +  SPI interface.
> +    https://www.analog.com/en/products/adis16240.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adis16240
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
Boolean properties don't require to be explicitly set. It would also be
nice to add a description pointing to the spi-bus documentation. Like
this:

  spi-cpha:
    description: |
      See Documentation/devicetree/bindings/spi/spi-bus.txt
    maxItems: 1

  spi-cpol:
    description: |
      See Documentation/devicetree/bindings/spi/spi-bus.txt
    maxItems: 1

As far as I know, spi-cpol and spi-cpha stand for SPI chip polarity and
SPI chip phase respectively. By default, it is assumed that SPI
input/output data is available at uprising clock edges, however, some
chips may work with different configuration (taking input data and/or
push it out in falling edges). I'm not 100% sure but, from what I've
seen on IIO, cpol is set to invert the input/output logic (making IO be
taken on falling edges) while cpha is usually set when MISO valid out
data is available on SCLK falling edge. If anyone has more comments
about this please, add them here, I'm curious about it. :)

> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        /* Example for a SPI device node */
> +        accelerometer@0 {
> +            compatible = "adi,adis16240";
> +            reg = <0>;
> +            spi-max-frequency = <2500000>;
> +            spi-cpol;
> +            spi-cpha;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +    };
> -- 
> 2.23.0.rc1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20190902005938.7734-1-rodrigorsdc%40gmail.com.
Jonathan Cameron Sept. 8, 2019, 10:52 a.m. UTC | #3 
On Mon, 2 Sep 2019 14:14:18 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Hi Rodrigo,
> 
> This dt doc looks overal fine IMHO.
> I would just add some inline comments about the cpha and cpol
> properties.
> 
> On 09/01, Rodrigo Carvalho wrote:
> > This patch add device tree binding documentation for ADIS16240.
> > 
> > Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@gmail.com>
> > ---
> > I have doubt about what maintainer I may to put in that documentation. I
> > put Alexandru as maintainer because he reviewed my last patch on this
> > driver, so I think that he is a good candidate.
> >  .../bindings/iio/accel/adi,adis16240.yaml     | 55 +++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > new file mode 100644
> > index 000000000000..08019b51611c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ADIS16240 Programmable Impact Sensor and Recorder driver
> > +
> > +maintainers:
> > +  - Alexandru Ardelean <alexandru.ardelean@analog.com>
> > +
> > +description: |
> > +  ADIS16240 Programmable Impact Sensor and Recorder driver that supports
> > +  SPI interface.
> > +    https://www.analog.com/en/products/adis16240.html
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,adis16240
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-cpha: true
> > +
> > +  spi-cpol: true  
> Boolean properties don't require to be explicitly set. It would also be
> nice to add a description pointing to the spi-bus documentation. Like
> this:
> 
>   spi-cpha:
>     description: |
>       See Documentation/devicetree/bindings/spi/spi-bus.txt
>     maxItems: 1
> 
>   spi-cpol:
>     description: |
>       See Documentation/devicetree/bindings/spi/spi-bus.txt
>     maxItems: 1
> 
> As far as I know, spi-cpol and spi-cpha stand for SPI chip polarity and
> SPI chip phase respectively. By default, it is assumed that SPI
> input/output data is available at uprising clock edges, however, some
> chips may work with different configuration (taking input data and/or
> push it out in falling edges). I'm not 100% sure but, from what I've
> seen on IIO, cpol is set to invert the input/output logic (making IO be
> taken on falling edges) while cpha is usually set when MISO valid out
> data is available on SCLK falling edge. If anyone has more comments
> about this please, add them here, I'm curious about it. :)
> 

They may well be constant for a given device (some will cope with
several combinations).  So the binding should reflect if they 'must'
be set.

Adding the cross reference is indeed nice.

Jonathan

> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    spi0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        /* Example for a SPI device node */
> > +        accelerometer@0 {
> > +            compatible = "adi,adis16240";
> > +            reg = <0>;
> > +            spi-max-frequency = <2500000>;
> > +            spi-cpol;
> > +            spi-cpha;
> > +            interrupt-parent = <&gpio0>;
> > +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +        };
> > +    };
> > -- 
> > 2.23.0.rc1
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20190902005938.7734-1-rodrigorsdc%40gmail.com.
Rodrigo Carvalho Sept. 11, 2019, 4:21 p.m. UTC | #4 
Hi,

Thanks for comments. I will send a v2.

Em dom, 8 de set de 2019 às 07:52, Jonathan Cameron <jic23@kernel.org> escreveu:
>
> On Mon, 2 Sep 2019 14:14:18 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
>
> > Hi Rodrigo,
> >
> > This dt doc looks overal fine IMHO.
> > I would just add some inline comments about the cpha and cpol
> > properties.
> >
> > On 09/01, Rodrigo Carvalho wrote:
> > > This patch add device tree binding documentation for ADIS16240.
> > >
> > > Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@gmail.com>
> > > ---
> > > I have doubt about what maintainer I may to put in that documentation. I
> > > put Alexandru as maintainer because he reviewed my last patch on this
> > > driver, so I think that he is a good candidate.
> > >  .../bindings/iio/accel/adi,adis16240.yaml     | 55 +++++++++++++++++++
> > >  1 file changed, 55 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > > new file mode 100644
> > > index 000000000000..08019b51611c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
> > > @@ -0,0 +1,55 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ADIS16240 Programmable Impact Sensor and Recorder driver
> > > +
> > > +maintainers:
> > > +  - Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > +
> > > +description: |
> > > +  ADIS16240 Programmable Impact Sensor and Recorder driver that supports
> > > +  SPI interface.
> > > +    https://www.analog.com/en/products/adis16240.html
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,adis16240
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  spi-cpha: true
> > > +
> > > +  spi-cpol: true
> > Boolean properties don't require to be explicitly set. It would also be
> > nice to add a description pointing to the spi-bus documentation. Like
> > this:
> >
> >   spi-cpha:
> >     description: |
> >       See Documentation/devicetree/bindings/spi/spi-bus.txt
> >     maxItems: 1
> >
> >   spi-cpol:
> >     description: |
> >       See Documentation/devicetree/bindings/spi/spi-bus.txt
> >     maxItems: 1
> >
> > As far as I know, spi-cpol and spi-cpha stand for SPI chip polarity and
> > SPI chip phase respectively. By default, it is assumed that SPI
> > input/output data is available at uprising clock edges, however, some
> > chips may work with different configuration (taking input data and/or
> > push it out in falling edges). I'm not 100% sure but, from what I've
> > seen on IIO, cpol is set to invert the input/output logic (making IO be
> > taken on falling edges) while cpha is usually set when MISO valid out
> > data is available on SCLK falling edge. If anyone has more comments
> > about this please, add them here, I'm curious about it. :)
> >
>
> They may well be constant for a given device (some will cope with
> several combinations).  So the binding should reflect if they 'must'
> be set.
>
> Adding the cross reference is indeed nice.
>
> Jonathan
>
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +    spi0 {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        /* Example for a SPI device node */
> > > +        accelerometer@0 {
> > > +            compatible = "adi,adis16240";
> > > +            reg = <0>;
> > > +            spi-max-frequency = <2500000>;
> > > +            spi-cpol;
> > > +            spi-cpha;
> > > +            interrupt-parent = <&gpio0>;
> > > +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > > +        };
> > > +    };
> > > --
> > > 2.23.0.rc1
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "Kernel USP" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20190902005938.7734-1-rodrigorsdc%40gmail.com.
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
new file mode 100644
index 000000000000..08019b51611c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adis16240.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADIS16240 Programmable Impact Sensor and Recorder driver
+
+maintainers:
+  - Alexandru Ardelean <alexandru.ardelean@analog.com>
+
+description: |
+  ADIS16240 Programmable Impact Sensor and Recorder driver that supports
+  SPI interface.
+    https://www.analog.com/en/products/adis16240.html
+
+properties:
+  compatible:
+    enum:
+      - adi,adis16240
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* Example for a SPI device node */
+        accelerometer@0 {
+            compatible = "adi,adis16240";
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            spi-cpol;
+            spi-cpha;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+        };
+    };