On Mon, 2 Sep 2019 14:14:18 -0300 Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx> 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@xxxxxxxxx> > > --- > > 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@xxxxxxxxxx> > > + > > +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@xxxxxxxxxxxxxxxx. > > To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/20190902005938.7734-1-rodrigorsdc%40gmail.com.
