On Sat, 2019-11-23 at 11:41 +0000, Jonathan Cameron wrote: > On Sat, 23 Nov 2019 02:19:27 -0300 > Rodrigo Carvalho <rodrigorsdc@xxxxxxxxx> wrote: > > > This patch add device tree binding documentation for ADIS16240. > > > > Signed-off-by: Rodrigo Ribeiro Carvalho <rodrigorsdc@xxxxxxxxx> My bad for the late timing on this. I'm slightly more fresh on Mondays. But I will get overloaded with work in a few hours, so I may not have time ot respond. > No problem with this patch, but I definitely want to see an accompanying > one enforcing the SPI mode in the driver. > So, then the binding should probably also define spi-cpol & spi-cpha as mandatory. Maybe, the driver would do a check and print a warning. I'm noticing that this device uses SPI mode 3, but this DT binding defaults to SPI mode 0 > Right now the driver doesn't set it and so I'm fairly sure not putting > it in the binding will leave you with a non working device. > > The right option if only one option is supported is for the driver > to call spi_setup with the relevant options. > What if the board uses some level inverters [because of some weird reason] and that messes up with the SPI mode? It's not common, but it is possible. > Thanks, > > Jonathan > > > --- > > V4: > > - Remove spi-cpha and spi-cpol in binding example, since this driver > > supports only one timing mode. > > .../bindings/iio/accel/adi,adis16240.yaml | 49 +++++++++++++++++++ > > 1 file changed, 49 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..8e902f7c49e6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16240.yaml > > @@ -0,0 +1,49 @@ > > +# 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 > > + > > + 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>; > > + interrupt-parent = <&gpio0>; > > + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + };
