On Sat, 2020-06-20 at 16:31 +0100, Jonathan Cameron wrote: > [External] > > On Wed, 17 Jun 2020 16:35:23 +0300 > Darius Berghe <darius.berghe@xxxxxxxxxx> wrote: > > > Add dt binding documentation for ltc2471 driver. This covers all supported > > devices. > > > > Signed-off-by: Darius Berghe <darius.berghe@xxxxxxxxxx> > A few things inline but basically fine. > > We should however also think about documenting power supplies. > Even though the driver doesn't currently control the binding should > be as complete as possible. > > Jonathan > Hi Jonathan, And thanks for the review ! This chips have a fixed internal vref of 1.25V that is output on the REFOUT pin, there is no place for configuration here. Or perhaps did you mean the VCC (2.7V-5.5V) ? I'm not sure what the added value would be to add vref-supply and vcc-supply to yaml if they are not implemented. I find it confusing. > > --- > > .../bindings/iio/adc/adi,ltc2471.yaml | 52 +++++++++++++++++++ > > 1 file changed, 52 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2471.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ltc2471.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ltc2471.yaml > > new file mode 100644 > > index 000000000000..0b84e14ec984 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ltc2471.yaml > > @@ -0,0 +1,52 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +# Copyright 2020 Analog Devices Inc. > > +%YAML 1.2 > > +--- > > +$id: https://urldefense.com/v3/__http://devicetree.org/schemas/bindings/iio/adc/adi,ltc2471.yaml*__;Iw!!A3Ni8CS0y2Y!vUpDwSslcaNrc3db6AQ6x3gzYHbR_WxOtQyPinkeZCjgpiQ4elEbjMzDs1OGEYZou4E$ > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!vUpDwSslcaNrc3db6AQ6x3gzYHbR_WxOtQyPinkeZCjgpiQ4elEbjMzDs1OG4cmRuW4$ > > + > > +title: Analog Devices LTC2471 16-bit I2C Sigma-Delta ADC > > + > > +maintainers: > > + - Mike Looijmans <mike.looijmans@xxxxxxxx> > > + > > +description: | > > + Analog Devices LTC2471 (single-ended) and LTC2473 (differential) 16-bit > > + I2C Sigma-Delta ADC with selectable 208/833sps output rate. > > + https://www.analog.com/media/en/technical-documentation/data-sheets/24713fb.pdf > > + > > + Analog Devices LTC2461 (single-ended) and LTC2463 (differential) 16-bit > > + I2C Sigma-Delta ADC with 60sps output rate. > > + https://www.analog.com/media/en/technical-documentation/data-sheets/24613fa.pdf > > Put these two blocks in numeric order. If we end up adding a bunch more > devices it will be much more consistent if they are order. > Ack, will do. > > + > > +properties: > > + compatible: > > + enum: > > + - adi,ltc2471 > > + - adi,ltc2473 > > + - adi,ltc2461 > > + - adi,ltc2463 > > Put them in numeric order. > Ack, will do. > > + > > + reg: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +examples: > > + - | > > + i2c0 { > > + ltc2461@14 { > > Should use a generic name > adc@14 > Ack, will do. > > + compatible = "ltc2461"; > > + reg = <0x14>; > > + }; > > + }; > > + - | > > + i2c0 { > > Not a lot of point in two examples given how similar they are. > I'd just keep the one. > Ack, will do. I only chose to give two examples because the chip has 2 possible I2C slave addresses 0x14 and 0x54 depending on the AO pin value being low or high. But you're right, they're too simple and similar. Best regards, Darius > > + ltc2473@54 { > > + compatible = "ltc2473"; > > + reg = <0x54>; > > + }; > > + }; > > +