[PATCH 1/2] iio: adc: ad799x: Implement selecting external reference voltage input on AD7991, AD7995 and AD7999.

[PATCH 1/2] iio: adc: ad799x: Implement selecting external reference voltage input on AD7991, AD7995 and AD7999.

[Florian Boor]

Make use of the AD7991_REF_SEL bit and support using the external
reference voltage if 'vref-supply' is present. Use VCC voltage supply
as reference if no extra reference is supplied.

Signed-off-by: Florian Boor <florian.boor@kernelconcepts.de>
---
Changes in v3:
- Do not create dummy reference regulator, check for vref presence where needed.
- Use VCC as reference if no extra reference voltage is provided
- Add interrupt information to documentation


Changes in v2:
- Check if a provided external vref regulator is provided.
- Drop unused setting
- Add ad79xx documentation (second patch)

 drivers/iio/adc/ad799x.c | 60 ++++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 18bf8386d50a..af72fd980030 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -299,7 +299,11 @@ static int ad799x_read_raw(struct iio_dev *indio_dev,
 			GENMASK(chan->scan_type.realbits - 1, 0);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = regulator_get_voltage(st->vref);
+		if (st->vref)
+			ret = regulator_get_voltage(st->vref);
+		else
+			ret = regulator_get_voltage(st->reg);
+		
 		if (ret < 0)
 			return ret;
 		*val = ret / 1000;
@@ -770,6 +774,7 @@ static int ad799x_probe(struct i2c_client *client,
 				   const struct i2c_device_id *id)
 {
 	int ret;
+	int extra_config = 0;
 	struct ad799x_state *st;
 	struct iio_dev *indio_dev;
 	const struct ad799x_chip_info *chip_info =
@@ -797,14 +802,28 @@ static int ad799x_probe(struct i2c_client *client,
 	ret = regulator_enable(st->reg);
 	if (ret)
 		return ret;
-	st->vref = devm_regulator_get(&client->dev, "vref");
+		
+	/* check if an external reference is supplied */
+	st->vref = devm_regulator_get_optional(&client->dev, "vref");
+
 	if (IS_ERR(st->vref)) {
-		ret = PTR_ERR(st->vref);
-		goto error_disable_reg;
+		st->vref = NULL;
+		dev_info(&client->dev, "Using VCC reference voltage\n");
+	} else {
+		/* Use external reference voltage if supported by hardware.
+		 * This is optional if voltage / regulator present, use VCC otherwise.
+		 */
+		if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)) {
+			dev_info(&client->dev, "Using external reference voltage\n");
+			extra_config |= AD7991_REF_SEL;
+			ret = regulator_enable(st->vref);
+			if (ret)
+				goto error_disable_reg;
+		} else {
+			st->vref = NULL;
+			dev_warn(&client->dev, "Supplied reference not supported\n");
+		}
 	}
-	ret = regulator_enable(st->vref);
-	if (ret)
-		goto error_disable_reg;
 
 	st->client = client;
 
@@ -815,7 +834,7 @@ static int ad799x_probe(struct i2c_client *client,
 	indio_dev->channels = st->chip_config->channel;
 	indio_dev->num_channels = chip_info->num_channels;
 
-	ret = ad799x_update_config(st, st->chip_config->default_config);
+	ret = ad799x_update_config(st, st->chip_config->default_config | extra_config);
 	if (ret)
 		goto error_disable_vref;
 
@@ -845,7 +864,8 @@ static int ad799x_probe(struct i2c_client *client,
 error_cleanup_ring:
 	iio_triggered_buffer_cleanup(indio_dev);
 error_disable_vref:
-	regulator_disable(st->vref);
+	if (st->vref) 
+		regulator_disable(st->vref);
 error_disable_reg:
 	regulator_disable(st->reg);
 
@@ -860,7 +880,8 @@ static int ad799x_remove(struct i2c_client *client)
 	iio_device_unregister(indio_dev);
 
 	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(st->vref);
+	if (st->vref) 
+		regulator_disable(st->vref);
 	regulator_disable(st->reg);
 	kfree(st->rx_buf);
 
@@ -872,7 +893,8 @@ static int __maybe_unused ad799x_suspend(struct device *dev)
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ad799x_state *st = iio_priv(indio_dev);
 
-	regulator_disable(st->vref);
+	if (st->vref) 
+		regulator_disable(st->vref);
 	regulator_disable(st->reg);
 
 	return 0;
@@ -889,17 +911,21 @@ static int __maybe_unused ad799x_resume(struct device *dev)
 		dev_err(dev, "Unable to enable vcc regulator\n");
 		return ret;
 	}
-	ret = regulator_enable(st->vref);
-	if (ret) {
-		regulator_disable(st->reg);
-		dev_err(dev, "Unable to enable vref regulator\n");
-		return ret;
+
+	if (st->vref) {
+		ret = regulator_enable(st->vref);
+		if (ret) {
+			regulator_disable(st->reg);
+			dev_err(dev, "Unable to enable vref regulator\n");
+			return ret;
+		}
 	}
 
 	/* resync config */
 	ret = ad799x_update_config(st, st->config);
 	if (ret) {
-		regulator_disable(st->vref);
+		if (st->vref) 
+			regulator_disable(st->vref);
 		regulator_disable(st->reg);
 		return ret;
 	}
-- 
2.30.2

[PATCH 2/2] dt-binding: iio: Add documentation for AD799x driver

[Florian Boor]

New documentation for AD799x series of I²C ADC ICs.

Signed-off-by: Florian Boor <florian.boor@kernelconcepts.de>
---
 .../bindings/iio/adc/adi,ad799x.yaml          | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml
new file mode 100644
index 000000000000..66de61050deb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad799x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7991, AD7992, AD7993, AD7994, AD7995, AD7997, AD7998,
+       AD7999 and similar analog to digital converters
+
+maintainers:
+  - Michael Hennerich, Analog Devices Inc. <Michael.Hennerich@analog.com>
+
+description: |
+  Specifications on the converters can be found at:
+    AD7991, AD7995, AD7999:
+      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7991_7995_7999.pdf
+    AD7992:
+      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7992.pdf
+    AD7993, AD7994:
+      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7993_7994.pdf
+    AD7997, AD7998:
+      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7997_7998.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7991
+      - adi,ad7992
+      - adi,ad7993
+      - adi,ad7994
+      - adi,ad7995
+      - adi,ad7997
+      - adi,ad7998
+      - adi,ad7999
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vcc-supply:
+    description:
+      ADC power supply
+
+  vref-supply:
+    description:
+      ADC reference voltage supply, optional for AD7991, AD7995 and AD7999
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+        adc1: ad7991@28 {
+                reg = <0x28>;
+                compatible = "adi,ad7991";
+                interrupts = <13 2>;
+                interrupt-parent = <&gpio6>;
+
+                vcc-supply = <&vcc_3v3>;
+                vref-supply = <&adc_vref>;
+        };
+...
-- 
2.30.2

Re: [PATCH 2/2] dt-binding: iio: Add documentation for AD799x driver

[Jonathan Cameron]

On Tue, 14 Sep 2021 16:51:42 +0200
Florian Boor <florian.boor@kernelconcepts.de> wrote:

Patch title should be dt-bindings: iio: ad779x: Add binding document.

Also, all bindings need to be sent to Rob and the dt mailing list.

Otherwise looks good to me.

Whilst I've +cc'd Rob and device tree lists I don't think patchwork will
pick it up so you should send a v4 (where did version number go on this one)
with them cc'd to be sure it's on their radar.

Thanks,

Jonathan


> New documentation for AD799x series of I²C ADC ICs.
> 
> Signed-off-by: Florian Boor <florian.boor@kernelconcepts.de>
> ---
>  .../bindings/iio/adc/adi,ad799x.yaml          | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml
> new file mode 100644
> index 000000000000..66de61050deb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad799x.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad799x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7991, AD7992, AD7993, AD7994, AD7995, AD7997, AD7998,
> +       AD7999 and similar analog to digital converters
> +
> +maintainers:
> +  - Michael Hennerich, Analog Devices Inc. <Michael.Hennerich@analog.com>
> +
> +description: |
> +  Specifications on the converters can be found at:
> +    AD7991, AD7995, AD7999:
> +      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7991_7995_7999.pdf
> +    AD7992:
> +      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7992.pdf
> +    AD7993, AD7994:
> +      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7993_7994.pdf
> +    AD7997, AD7998:
> +      https://www.analog.com/media/en/technical-documentation/data-sheets/AD7997_7998.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7991
> +      - adi,ad7992
> +      - adi,ad7993
> +      - adi,ad7994
> +      - adi,ad7995
> +      - adi,ad7997
> +      - adi,ad7998
> +      - adi,ad7999
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vcc-supply:
> +    description:
> +      ADC power supply
> +
> +  vref-supply:
> +    description:
> +      ADC reference voltage supply, optional for AD7991, AD7995 and AD7999
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        adc1: ad7991@28 {
> +                reg = <0x28>;
> +                compatible = "adi,ad7991";
> +                interrupts = <13 2>;
> +                interrupt-parent = <&gpio6>;
> +
> +                vcc-supply = <&vcc_3v3>;
> +                vref-supply = <&adc_vref>;
> +        };
> +...

Re: [PATCH 1/2] iio: adc: ad799x: Implement selecting external reference voltage input on AD7991, AD7995 and AD7999.

[Jonathan Cameron]

On Tue, 14 Sep 2021 16:51:41 +0200
Florian Boor <florian.boor@kernelconcepts.de> wrote:

> Make use of the AD7991_REF_SEL bit and support using the external
> reference voltage if 'vref-supply' is present. Use VCC voltage supply
> as reference if no extra reference is supplied.
> 
> Signed-off-by: Florian Boor <florian.boor@kernelconcepts.de>
Hi Florian,

Looks fairly good, but you need to be careful about what error return
you get.  It may not indicate the case you are thinking of (no regulator
provided) and for at least some other values you want to fail the probe.

Thanks,

Jonathan

> ---
> Changes in v3:
> - Do not create dummy reference regulator, check for vref presence where needed.
> - Use VCC as reference if no extra reference voltage is provided
> - Add interrupt information to documentation
> 
> 
> Changes in v2:
> - Check if a provided external vref regulator is provided.
> - Drop unused setting
> - Add ad79xx documentation (second patch)
> 
>  drivers/iio/adc/ad799x.c | 60 ++++++++++++++++++++++++++++------------
>  1 file changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 18bf8386d50a..af72fd980030 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -299,7 +299,11 @@ static int ad799x_read_raw(struct iio_dev *indio_dev,
>  			GENMASK(chan->scan_type.realbits - 1, 0);
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = regulator_get_voltage(st->vref);
> +		if (st->vref)
> +			ret = regulator_get_voltage(st->vref);
> +		else
> +			ret = regulator_get_voltage(st->reg);
> +		
>  		if (ret < 0)
>  			return ret;
>  		*val = ret / 1000;
> @@ -770,6 +774,7 @@ static int ad799x_probe(struct i2c_client *client,
>  				   const struct i2c_device_id *id)
>  {
>  	int ret;
> +	int extra_config = 0;
>  	struct ad799x_state *st;
>  	struct iio_dev *indio_dev;
>  	const struct ad799x_chip_info *chip_info =
> @@ -797,14 +802,28 @@ static int ad799x_probe(struct i2c_client *client,
>  	ret = regulator_enable(st->reg);
>  	if (ret)
>  		return ret;
> -	st->vref = devm_regulator_get(&client->dev, "vref");
> +		
> +	/* check if an external reference is supplied */
> +	st->vref = devm_regulator_get_optional(&client->dev, "vref");
> +
>  	if (IS_ERR(st->vref)) {

Ah. We need to be careful here as not all errors indicate no regulator is
provided.  IIRC you need to check for -ENODEV (check that)

If we got a deferred probe return we should error out here as the driver
will re probe later after the regulator driver is ready.

> -		ret = PTR_ERR(st->vref);
> -		goto error_disable_reg;
> +		st->vref = NULL;
> +		dev_info(&client->dev, "Using VCC reference voltage\n");
> +	} else {
> +		/* Use external reference voltage if supported by hardware.

IIO preferred comment syntax is

		/*
		 * Use....
		 * This...
		 */

> +		 * This is optional if voltage / regulator present, use VCC otherwise.
> +		 */
> +		if ((st->id == ad7991) || (st->id == ad7995) || (st->id == ad7999)) {
> +			dev_info(&client->dev, "Using external reference voltage\n");
> +			extra_config |= AD7991_REF_SEL;
> +			ret = regulator_enable(st->vref);
> +			if (ret)
> +				goto error_disable_reg;
> +		} else {
> +			st->vref = NULL;
> +			dev_warn(&client->dev, "Supplied reference not supported\n");
> +		}
>  	}
> -	ret = regulator_enable(st->vref);
> -	if (ret)
> -		goto error_disable_reg;
>  
>  	st->client = client;
>  
> @@ -815,7 +834,7 @@ static int ad799x_probe(struct i2c_client *client,
>  	indio_dev->channels = st->chip_config->channel;
>  	indio_dev->num_channels = chip_info->num_channels;
>  
> -	ret = ad799x_update_config(st, st->chip_config->default_config);
> +	ret = ad799x_update_config(st, st->chip_config->default_config | extra_config);
>  	if (ret)
>  		goto error_disable_vref;
>  
> @@ -845,7 +864,8 @@ static int ad799x_probe(struct i2c_client *client,
>  error_cleanup_ring:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  error_disable_vref:
> -	regulator_disable(st->vref);
> +	if (st->vref) 
> +		regulator_disable(st->vref);
>  error_disable_reg:
>  	regulator_disable(st->reg);
>  
> @@ -860,7 +880,8 @@ static int ad799x_remove(struct i2c_client *client)
>  	iio_device_unregister(indio_dev);
>  
>  	iio_triggered_buffer_cleanup(indio_dev);
> -	regulator_disable(st->vref);
> +	if (st->vref) 
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  	kfree(st->rx_buf);
>  
> @@ -872,7 +893,8 @@ static int __maybe_unused ad799x_suspend(struct device *dev)
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>  	struct ad799x_state *st = iio_priv(indio_dev);
>  
> -	regulator_disable(st->vref);
> +	if (st->vref) 
> +		regulator_disable(st->vref);
>  	regulator_disable(st->reg);
>  
>  	return 0;
> @@ -889,17 +911,21 @@ static int __maybe_unused ad799x_resume(struct device *dev)
>  		dev_err(dev, "Unable to enable vcc regulator\n");
>  		return ret;
>  	}
> -	ret = regulator_enable(st->vref);
> -	if (ret) {
> -		regulator_disable(st->reg);
> -		dev_err(dev, "Unable to enable vref regulator\n");
> -		return ret;
> +
> +	if (st->vref) {
> +		ret = regulator_enable(st->vref);
> +		if (ret) {
> +			regulator_disable(st->reg);
> +			dev_err(dev, "Unable to enable vref regulator\n");
> +			return ret;
> +		}
>  	}
>  
>  	/* resync config */
>  	ret = ad799x_update_config(st, st->config);
>  	if (ret) {
> -		regulator_disable(st->vref);
> +		if (st->vref) 
> +			regulator_disable(st->vref);
>  		regulator_disable(st->reg);
>  		return ret;
>  	}