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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190902171417.qbj7rwi43tr77mr5@smtp.gmail.com>
Date:   Mon, 2 Sep 2019 14:14:18 -0300
From:   Marcelo Schmitt <marcelo.schmitt1@...il.com>
To:     Rodrigo Carvalho <rodrigorsdc@...il.com>
Cc:     Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Stefan Popa <stefan.popa@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        alexandru.ardelean@...log.com, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-usp@...glegroups.com
Subject: Re: [PATCH 1/2] dt-bindings: iio: accel: add binding documentation
 for ADIS16240

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@...il.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@...log.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@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20190902005938.7734-1-rodrigorsdc%40gmail.com.