[PATCH 1/3] Documentation: ad5064: Added devicetree bindings documentation

[PATCH 1/3] Documentation: ad5064: Added devicetree bindings documentation

[Paul Cercueil]

Signed-off-by: Paul Cercueil <paul.cercueil-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/iio/dac/ad5064.txt         | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5064.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/ad5064.txt b/Documentation/devicetree/bindings/iio/dac/ad5064.txt
new file mode 100644
index 0000000..fa2d328
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ad5064.txt
@@ -0,0 +1,48 @@
+Analog Devices AD5064 DAC device driver
+
+Required properties:
+	- compatible: Must be one of:
+		* "adi,ad5024"
+		* "adi,ad5025"
+		* "adi,ad5044"
+		* "adi,ad5045"
+		* "adi,ad5064"
+		* "adi,ad5064-1"
+		* "adi,ad5065"
+		* "adi,ad5628-1"
+		* "adi,ad5628-2"
+		* "adi,ad5648-1"
+		* "adi,ad5648-2"
+		* "adi,ad5666-1"
+		* "adi,ad5666-2"
+		* "adi,ad5668-1"
+		* "adi,ad5668-2"
+		* "adi,ad5668-3"
+	- reg: SPI chip select number for the device
+	- spi-max-frequency: Max SPI frequency to use (< 30000000)
+	- vrefA-supply, vrefB-supply: phandles to external reference voltage
+	  supplies for channels 0 and 1 respectively.
+	  This property must be present for ad5024, ad5025, ad5044, ad5045,
+	  ad5064, ad5065.
+	- vrefC-supply, vrefD-supply: phandles to external reference voltage
+	  supplies for channels 2 and 3 respectively.
+	  This property must be present for ad5024, ad5044, ad5064.
+
+Optional properties:
+	- vref-supply: phandle to the external reference voltage supply.
+	  This should only be set if there is an external reference voltage
+	  connected to the vref or vref[A-D] pins.
+	  If the property is not set, the internal reference voltage supply
+	  is used if present.
+	  This property can be used with ad5064-1, ad5628-1, ad5628-2, ad5648-1,
+	  ad5648-2, ad5666-1, ad5666-2, ad5668-1, ad5668-2, ad5668-3.
+
+Example:
+
+		ad5668-2@4 {
+			compatible = "adi,ad5668-2";
+			reg = <4>;
+			spi-max-frequency = <10000000>;
+			adi,use-external-reference;
+			vref-supply = <&vref_supply>;
+		};
-- 
2.5.3

[PATCH 2/3] iio: ad5064: Explicitly configure whether to use external supply

[Paul Cercueil]

Previously the driver would revert to internal supply if the
external supply couldn't be found. This had multiple problems:
- it caused silently ignored errors when a regulator was intended
  to be supplied, but was not specified correctly.
- if CONFIG_REGULATOR is disabled, regulator_get() will always
  return a dummy regulator, which caused a device to always use
  the external vref mode, even though there is none.

This patch addresses the issue by adding a platform data structure,
containing a boolean field use_external_ref. If the platform data
structure is present and if that boolean is set, the external vref
is used; otherwise the internal vref is used.

In the case where devicetree is used, and if regulators are listed
as external references in the device tree, we assume that the
external reference should be used.

In the case where an external vref is wanted but regulator_get()
fails, the driver no longer reverts to using the internal vref,
but returns an error instead.

Signed-off-by: Paul Cercueil <paul.cercueil-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
---
 drivers/iio/dac/ad5064.c             | 36 +++++++++++++++++++++++++++---------
 include/linux/platform_data/ad5064.h | 21 +++++++++++++++++++++
 2 files changed, 48 insertions(+), 9 deletions(-)
 create mode 100644 include/linux/platform_data/ad5064.h

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index c067e68..7146f42 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -2,7 +2,7 @@
  * AD5024, AD5025, AD5044, AD5045, AD5064, AD5064-1, AD5065, AD5628, AD5629R,
  * AD5648, AD5666, AD5668, AD5669R Digital to analog converters driver
  *
- * Copyright 2011 Analog Devices Inc.
+ * Copyright 2011, 2015 Analog Devices Inc.
  *
  * Licensed under the GPL-2.
  */
@@ -21,6 +21,8 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
+#include <linux/platform_data/ad5064.h>
+
 #define AD5064_MAX_DAC_CHANNELS			8
 #define AD5064_MAX_VREFS			4
 
@@ -446,6 +448,7 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
 	unsigned int midscale;
 	unsigned int i;
 	int ret;
+	bool ext_vref;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
@@ -461,11 +464,30 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
 	for (i = 0; i < ad5064_num_vref(st); ++i)
 		st->vref_reg[i].supply = ad5064_vref_name(st, i);
 
-	ret = devm_regulator_bulk_get(dev, ad5064_num_vref(st),
-		st->vref_reg);
-	if (ret) {
-		if (!st->chip_info->internal_vref)
+	if (dev->of_node) {
+		for (i = 0; ext_vref && i < ad5064_num_vref(st); ++i)
+			ext_vref = of_property_read_bool(dev->of_node,
+					ad5064_vref_name(st, i));
+	} else {
+		struct ad5064_platform_data *pdata = dev->platform_data;
+
+		ext_vref = pdata && pdata->use_external_ref;
+	}
+
+	if (ext_vref) {
+		ret = devm_regulator_bulk_get(dev, ad5064_num_vref(st),
+					st->vref_reg);
+		if (ret)
 			return ret;
+		ret = regulator_bulk_enable(ad5064_num_vref(st), st->vref_reg);
+		if (ret)
+			return ret;
+	} else {
+		if (!st->chip_info->internal_vref) {
+			dev_err(dev, "No vref available\n");
+			return -ENXIO;
+		}
+
 		st->use_internal_vref = true;
 		ret = ad5064_write(st, AD5064_CMD_CONFIG, 0,
 			AD5064_CONFIG_INT_VREF_ENABLE, 0);
@@ -474,10 +496,6 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
 				ret);
 			return ret;
 		}
-	} else {
-		ret = regulator_bulk_enable(ad5064_num_vref(st), st->vref_reg);
-		if (ret)
-			return ret;
 	}
 
 	indio_dev->dev.parent = dev;
diff --git a/include/linux/platform_data/ad5064.h b/include/linux/platform_data/ad5064.h
new file mode 100644
index 0000000..69bb5fe
--- /dev/null
+++ b/include/linux/platform_data/ad5064.h
@@ -0,0 +1,21 @@
+/*
+ * Analog Devices AD5064 DAC driver
+ *
+ * Copyright 2015 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#ifndef __IIO_ADC_AD5064_H__
+#define __IIO_ADC_AD5064_H__
+
+/**
+ * struct ad5064_platform_data - AD5064 platform data
+ * @use_external_ref: If set to true use an external voltage reference connected
+ * to the VREF pin, otherwise use the internal reference derived from Vdd.
+ */
+struct ad5064_platform_data {
+	bool use_external_ref;
+};
+
+#endif
-- 
2.5.3

[PATCH 3/3] iio: ad5064: Always use external vref if there is no internal vref

[Paul Cercueil]

If the device does not have an internal reference, there is no
other choice but to use the external reference. In that case,
it does not make much sense to have to specify it.

This patch ensures that the external reference is used if the
device does not feature an internal reference.

Signed-off-by: Paul Cercueil <paul.cercueil-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
---
 drivers/iio/dac/ad5064.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index 7146f42..daee39e 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -464,7 +464,9 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
 	for (i = 0; i < ad5064_num_vref(st); ++i)
 		st->vref_reg[i].supply = ad5064_vref_name(st, i);
 
-	if (dev->of_node) {
+	if (!st->chip_info->internal_vref) {
+		ext_vref = true;
+	} else if (dev->of_node) {
 		for (i = 0; ext_vref && i < ad5064_num_vref(st); ++i)
 			ext_vref = of_property_read_bool(dev->of_node,
 					ad5064_vref_name(st, i));
@@ -483,11 +485,6 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
 		if (ret)
 			return ret;
 	} else {
-		if (!st->chip_info->internal_vref) {
-			dev_err(dev, "No vref available\n");
-			return -ENXIO;
-		}
-
 		st->use_internal_vref = true;
 		ret = ad5064_write(st, AD5064_CMD_CONFIG, 0,
 			AD5064_CONFIG_INT_VREF_ENABLE, 0);
-- 
2.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: ad5064: Explicitly configure whether to use external supply

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

Hi Paul,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: cris-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=cris 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/iio/dac/ad5064.c: In function 'ad5064_probe':
>> drivers/iio/dac/ad5064.c:468:3: warning: 'ext_vref' may be used uninitialized in this function [-Wuninitialized]

vim +/ext_vref +468 drivers/iio/dac/ad5064.c

   452	
   453		indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
   454		if (indio_dev == NULL)
   455			return  -ENOMEM;
   456	
   457		st = iio_priv(indio_dev);
   458		dev_set_drvdata(dev, indio_dev);
   459	
   460		st->chip_info = &ad5064_chip_info_tbl[type];
   461		st->dev = dev;
   462		st->write = write;
   463	
   464		for (i = 0; i < ad5064_num_vref(st); ++i)
   465			st->vref_reg[i].supply = ad5064_vref_name(st, i);
   466	
   467		if (dev->of_node) {
 > 468			for (i = 0; ext_vref && i < ad5064_num_vref(st); ++i)
   469				ext_vref = of_property_read_bool(dev->of_node,
   470						ad5064_vref_name(st, i));
   471		} else {
   472			struct ad5064_platform_data *pdata = dev->platform_data;
   473	
   474			ext_vref = pdata && pdata->use_external_ref;
   475		}
   476	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 36364 bytes --]

Re: [PATCH 3/3] iio: ad5064: Always use external vref if there is no internal vref

[Jonathan Cameron]

On 02/10/15 13:41, Paul Cercueil wrote:
> If the device does not have an internal reference, there is no
> other choice but to use the external reference. In that case,
> it does not make much sense to have to specify it.
> 
> This patch ensures that the external reference is used if the
> device does not feature an internal reference.
> 
> Signed-off-by: Paul Cercueil <paul.cercueil-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Other than the issue the autobuilder found I'm happy enough
with the series.

Given it is Lars' driver I'd like him to take a look as well
before I apply it.

Thanks,

Jonathan
> ---
>  drivers/iio/dac/ad5064.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
> index 7146f42..daee39e 100644
> --- a/drivers/iio/dac/ad5064.c
> +++ b/drivers/iio/dac/ad5064.c
> @@ -464,7 +464,9 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
>  	for (i = 0; i < ad5064_num_vref(st); ++i)
>  		st->vref_reg[i].supply = ad5064_vref_name(st, i);
>  
> -	if (dev->of_node) {
> +	if (!st->chip_info->internal_vref) {
> +		ext_vref = true;
> +	} else if (dev->of_node) {
>  		for (i = 0; ext_vref && i < ad5064_num_vref(st); ++i)
>  			ext_vref = of_property_read_bool(dev->of_node,
>  					ad5064_vref_name(st, i));
> @@ -483,11 +485,6 @@ static int ad5064_probe(struct device *dev, enum ad5064_type type,
>  		if (ret)
>  			return ret;
>  	} else {
> -		if (!st->chip_info->internal_vref) {
> -			dev_err(dev, "No vref available\n");
> -			return -ENXIO;
> -		}
> -
>  		st->use_internal_vref = true;
>  		ret = ad5064_write(st, AD5064_CMD_CONFIG, 0,
>  			AD5064_CONFIG_INT_VREF_ENABLE, 0);
>