diff mbox series

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

Message ID 20200617133523.58158-3-darius.berghe@analog.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/3] iio:adc:ltc2471: add match table for existing devices | expand

Commit Message

Darius Berghe June 17, 2020, 1:35 p.m. UTC 
Add dt binding documentation for ltc2471 driver. This covers all supported
devices.

Signed-off-by: Darius Berghe <darius.berghe@analog.com>
---
 .../bindings/iio/adc/adi,ltc2471.yaml         | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ltc2471.yaml

Comments

Jonathan Cameron June 20, 2020, 3:31 p.m. UTC | #1 
On Wed, 17 Jun 2020 16:35:23 +0300
Darius Berghe <darius.berghe@analog.com> wrote:

> Add dt binding documentation for ltc2471 driver. This covers all supported
> devices.
> 
> Signed-off-by: Darius Berghe <darius.berghe@analog.com>
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

> ---
>  .../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: http://devicetree.org/schemas/bindings/iio/adc/adi,ltc2471.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices LTC2471 16-bit I2C Sigma-Delta ADC
> +
> +maintainers:
> +  - Mike Looijmans <mike.looijmans@topic.nl>
> +
> +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.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ltc2471
> +      - adi,ltc2473
> +      - adi,ltc2461
> +      - adi,ltc2463

Put them in numeric order.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    i2c0 {
> +      ltc2461@14 {

Should use a generic name
adc@14

> +        compatible = "ltc2461";
> +        reg = <0x14>;
> +      };
> +    };
> +  - |
> +    i2c0 {

Not a lot of point in two examples given how similar they are.
I'd just keep the one. 

> +      ltc2473@54 {
> +        compatible = "ltc2473";
> +        reg = <0x54>;
> +      };
> +    };
> +
Darius Berghe June 22, 2020, 8:05 a.m. UTC | #2 
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@analog.com> wrote:
> 
> > Add dt binding documentation for ltc2471 driver. This covers all supported
> > devices.
> > 
> > Signed-off-by: Darius Berghe <darius.berghe@analog.com>
> 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@topic.nl>
> > +
> > +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>;
> > +      };
> > +    };
> > +
Jonathan Cameron June 27, 2020, 2:46 p.m. UTC | #3 
On Mon, 22 Jun 2020 08:05:13 +0000
"Berghe, Darius" <Darius.Berghe@analog.com> wrote:

> 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@analog.com> wrote:
> >   
> > > Add dt binding documentation for ltc2471 driver. This covers all supported
> > > devices.
> > > 
> > > Signed-off-by: Darius Berghe <darius.berghe@analog.com>  
> > 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) ?

Yes, VCC was what I was referring to.

> 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 are intended to describe the hardware rather than what we've implemented
support for in the driver.   It's fairly likely that we will end up supporting
regulator control sooner or later (tends to happen if a driver is getting much use
as someone will care about powering down the supplies when suspending etc).

So ideally we'd add support to the driver, but even if it's not there we can consider
adding it to the binding docs. However, it's not (to my mind) vital.

> 
> > > ---
> > >  .../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@topic.nl>
> > > +
> > > +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.

Agreed

Thanks,

Jonathan

> 
> Best regards,
> Darius
> 
> > > +      ltc2473@54 {
> > > +        compatible = "ltc2473";
> > > +        reg = <0x54>;
> > > +      };
> > > +    };
> > > +
diff mbox series

Patch

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: http://devicetree.org/schemas/bindings/iio/adc/adi,ltc2471.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices LTC2471 16-bit I2C Sigma-Delta ADC
+
+maintainers:
+  - Mike Looijmans <mike.looijmans@topic.nl>
+
+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
+
+properties:
+  compatible:
+    enum:
+      - adi,ltc2471
+      - adi,ltc2473
+      - adi,ltc2461
+      - adi,ltc2463
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c0 {
+      ltc2461@14 {
+        compatible = "ltc2461";
+        reg = <0x14>;
+      };
+    };
+  - |
+    i2c0 {
+      ltc2473@54 {
+        compatible = "ltc2473";
+        reg = <0x54>;
+      };
+    };
+