[PATCH v1 1/3] iio:adc:ltc2471: add match table for existing devices

[PATCH v1 1/3] iio:adc:ltc2471: add match table for existing devices

[Darius Berghe]

OF style match table is the proper way of matching device tree nodes
with drivers and such table was missing, this commit adds it.

Signed-off-by: Darius Berghe <darius.berghe@analog.com>
---
 drivers/iio/adc/ltc2471.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
index 55fab612843a..0e5cdb43a943 100644
--- a/drivers/iio/adc/ltc2471.c
+++ b/drivers/iio/adc/ltc2471.c
@@ -143,9 +143,17 @@ static const struct i2c_device_id ltc2471_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
 
+static const struct of_device_id ltc2471_of_match[] = {
+	{ .compatible = "adi,ltc2471" },
+	{ .compatible = "adi,ltc2473" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ltc2471_of_match);
+
 static struct i2c_driver ltc2471_i2c_driver = {
 	.driver = {
 		.name = "ltc2471",
+		.of_match_table = of_match_ptr(ltc2471_of_match)
 	},
 	.probe    = ltc2471_i2c_probe,
 	.id_table = ltc2471_i2c_id,
-- 
2.26.2

[PATCH v1 2/3] iio:adc:ltc2471: add ltc2461/ltc2463 compatible strings

[Darius Berghe]

Add compatible strings for these devices in the existing ltc2471
driver.

The only difference between 2461/2463 and 2471/2473 is the output
rate of fixed 60sps and selectible 208/833sps respectively.

The driver does not implement changing the sampling rate so the
new devices are fully code comptible with this driver.

Signed-off-by: Darius Berghe <darius.berghe@analog.com>
---
 drivers/iio/adc/ltc2471.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
index 0e5cdb43a943..098765882bba 100644
--- a/drivers/iio/adc/ltc2471.c
+++ b/drivers/iio/adc/ltc2471.c
@@ -1,7 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Driver for Linear Technology LTC2471 and LTC2473 voltage monitors
+ * Driver for Linear Technology LTC2471, LTC2473, LTC2461 and LTC2463 voltage
+ * monitors.
  * The LTC2473 is identical to the 2471, but reports a differential signal.
+ * The LTC2463 is identical to the 2461, but reports a differential signal.
  *
  * Copyright (C) 2017 Topic Embedded Products
  * Author: Mike Looijmans <mike.looijmans@topic.nl>
@@ -17,6 +19,8 @@
 enum ltc2471_chips {
 	ltc2471,
 	ltc2473,
+	ltc2461,
+	ltc2463
 };
 
 struct ltc2471_data {
@@ -120,7 +124,7 @@ static int ltc2471_i2c_probe(struct i2c_client *client,
 	indio_dev->name = id->name;
 	indio_dev->info = &ltc2471_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	if (id->driver_data == ltc2473)
+	if (id->driver_data == ltc2473 || id->driver_data == ltc2463)
 		indio_dev->channels = ltc2473_channel;
 	else
 		indio_dev->channels = ltc2471_channel;
@@ -139,6 +143,8 @@ static int ltc2471_i2c_probe(struct i2c_client *client,
 static const struct i2c_device_id ltc2471_i2c_id[] = {
 	{ "ltc2471", ltc2471 },
 	{ "ltc2473", ltc2473 },
+	{ "ltc2461", ltc2461 },
+	{ "ltc2463", ltc2463 },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
@@ -146,6 +152,8 @@ MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
 static const struct of_device_id ltc2471_of_match[] = {
 	{ .compatible = "adi,ltc2471" },
 	{ .compatible = "adi,ltc2473" },
+	{ .compatible = "adi,ltc2461" },
+	{ .compatible = "adi,ltc2463" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ltc2471_of_match);
@@ -161,6 +169,6 @@ static struct i2c_driver ltc2471_i2c_driver = {
 
 module_i2c_driver(ltc2471_i2c_driver);
 
-MODULE_DESCRIPTION("LTC2471/LTC2473 ADC driver");
+MODULE_DESCRIPTION("LTC2471/LTC2473/LTC2461/LTC2463 ADC driver");
 MODULE_AUTHOR("Topic Embedded Products");
 MODULE_LICENSE("GPL v2");
-- 
2.26.2

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

[Darius Berghe]

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

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

Re: [PATCH v1 1/3] iio:adc:ltc2471: add match table for existing devices

[Jonathan Cameron]

On Wed, 17 Jun 2020 16:35:21 +0300
Darius Berghe <darius.berghe@analog.com> wrote:

> OF style match table is the proper way of matching device tree nodes
> with drivers and such table was missing, this commit adds it.
> 
> Signed-off-by: Darius Berghe <darius.berghe@analog.com>

Hi Darius

A few minor things inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ltc2471.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/ltc2471.c b/drivers/iio/adc/ltc2471.c
> index 55fab612843a..0e5cdb43a943 100644
> --- a/drivers/iio/adc/ltc2471.c
> +++ b/drivers/iio/adc/ltc2471.c
> @@ -143,9 +143,17 @@ static const struct i2c_device_id ltc2471_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, ltc2471_i2c_id);
>  
> +static const struct of_device_id ltc2471_of_match[] = {

Should include mod_device_table.h as we are using of_device_id
which is defined in that header.

> +	{ .compatible = "adi,ltc2471" },
> +	{ .compatible = "adi,ltc2473" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ltc2471_of_match);
> +
>  static struct i2c_driver ltc2471_i2c_driver = {
>  	.driver = {
>  		.name = "ltc2471",
> +		.of_match_table = of_match_ptr(ltc2471_of_match)

Drop the of_match_ptr protection. That prevents the use of
this binding with ACPI (via the magic of PRP0001 which allows
you to use DT bindings in an ACPI DSDT). 

We are slowly working our way through all the drivers removing the
use of this macro (and converting to generic binding handling
where relevant). It'll take a while yet though before there
are no instances of this to copy!


>  	},
>  	.probe    = ltc2471_i2c_probe,
>  	.id_table = ltc2471_i2c_id,

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

[Jonathan Cameron]

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

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

[Berghe, Darius]

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

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

[Jonathan Cameron]

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