Re: [PATCH v1 3/3] iio:adc:ltc2471: add dt binding yaml

From: Berghe, Darius
Date: Mon Jun 22 2020 - 04:05:23 EST


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>;
> > + };
> > + };
> > +