All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
@ 2015-10-12 11:50 ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-iio,
	Paul Cercueil

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

 v2: No change in this patch

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
@ 2015-10-12 11:50 ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, 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

 v2: No change in this patch

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv2 2/3] iio: ad5064: Explicitly configure whether to use external supply
@ 2015-10-12 11:50   ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-iio,
	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@analog.com>
---
 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

 v2: Fix uninitialized 'ext_vref' variable

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index c067e68..e77631d 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 = true;
 
 	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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv2 2/3] iio: ad5064: Explicitly configure whether to use external supply
@ 2015-10-12 11:50   ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, 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

 v2: Fix uninitialized 'ext_vref' variable

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index c067e68..e77631d 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 = true;
 
 	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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv2 3/3] iio: ad5064: Always use external vref if there is no internal vref
@ 2015-10-12 11:50   ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-iio,
	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@analog.com>
---
 drivers/iio/dac/ad5064.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

 v2: No change in this patch

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index e77631d..18674e5 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -464,14 +464,16 @@ 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) {
-		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 (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) {
@@ -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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv2 3/3] iio: ad5064: Always use external vref if there is no internal vref
@ 2015-10-12 11:50   ` Paul Cercueil
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Cercueil @ 2015-10-12 11:50 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Michael Hennerich, Lars-Peter Clausen, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, 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 | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

 v2: No change in this patch

diff --git a/drivers/iio/dac/ad5064.c b/drivers/iio/dac/ad5064.c
index e77631d..18674e5 100644
--- a/drivers/iio/dac/ad5064.c
+++ b/drivers/iio/dac/ad5064.c
@@ -464,14 +464,16 @@ 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) {
-		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 (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) {
@@ -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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
@ 2015-10-12 15:18   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2015-10-12 15:18 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Jonathan Cameron, Michael Hennerich, Lars-Peter Clausen,
	Hartmut Knaack, Peter Meerwald, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-iio

On Mon, Oct 12, 2015 at 01:50:32PM +0200, Paul Cercueil wrote:
> Signed-off-by: Paul Cercueil <paul.cercueil@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/ad5064.txt         | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5064.txt
> 
>  v2: No change in this patch
> 
> 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.

Nit: please don't use CamelCase property names. just have vref-a-supply
and so on.

> +
> +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.

I don't understand. Surely the latter case means you're describing the
same supply twice (as it should already be in vref*-supply)?

What vref pins does the HW actually have?

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

This is undocumented (and unused by the driver?).

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
@ 2015-10-12 15:18   ` Mark Rutland
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Rutland @ 2015-10-12 15:18 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Jonathan Cameron, Michael Hennerich, Lars-Peter Clausen,
	Hartmut Knaack, Peter Meerwald, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA

On Mon, Oct 12, 2015 at 01:50:32PM +0200, Paul Cercueil wrote:
> 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
> 
>  v2: No change in this patch
> 
> 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.

Nit: please don't use CamelCase property names. just have vref-a-supply
and so on.

> +
> +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.

I don't understand. Surely the latter case means you're describing the
same supply twice (as it should already be in vref*-supply)?

What vref pins does the HW actually have?

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

This is undocumented (and unused by the driver?).

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
  2015-10-12 15:18   ` Mark Rutland
@ 2015-10-13 11:11     ` Lars-Peter Clausen
  -1 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-10-13 11:11 UTC (permalink / raw)
  To: Mark Rutland, Paul Cercueil
  Cc: Jonathan Cameron, Michael Hennerich, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, devicetree, linux-kernel, linux-iio

>> +
>> +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.
> 
> I don't understand. Surely the latter case means you're describing the
> same supply twice (as it should already be in vref*-supply)?
> 
> What vref pins does the HW actually have?

Some off the devices supported by the binding have a shared reference for
all DAC outputs, while others have a separate reference for each output. In
addition to that some of them have an internal reference that can used, in
which case the external reference is optional, otherwise the external
reference is required. The bindings documentation should clarify on which
applies to which device.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation
@ 2015-10-13 11:11     ` Lars-Peter Clausen
  0 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-10-13 11:11 UTC (permalink / raw)
  To: Mark Rutland, Paul Cercueil
  Cc: Jonathan Cameron, Michael Hennerich, Hartmut Knaack,
	Peter Meerwald, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA

>> +
>> +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.
> 
> I don't understand. Surely the latter case means you're describing the
> same supply twice (as it should already be in vref*-supply)?
> 
> What vref pins does the HW actually have?

Some off the devices supported by the binding have a shared reference for
all DAC outputs, while others have a separate reference for each output. In
addition to that some of them have an internal reference that can used, in
which case the external reference is optional, otherwise the external
reference is required. The bindings documentation should clarify on which
applies to which device.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-10-13 11:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12 11:50 [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation Paul Cercueil
2015-10-12 11:50 ` Paul Cercueil
2015-10-12 11:50 ` [PATCHv2 2/3] iio: ad5064: Explicitly configure whether to use external supply Paul Cercueil
2015-10-12 11:50   ` Paul Cercueil
2015-10-12 11:50 ` [PATCHv2 3/3] iio: ad5064: Always use external vref if there is no internal vref Paul Cercueil
2015-10-12 11:50   ` Paul Cercueil
2015-10-12 15:18 ` [PATCHv2 1/3] Documentation: ad5064: Added devicetree bindings documentation Mark Rutland
2015-10-12 15:18   ` Mark Rutland
2015-10-13 11:11   ` Lars-Peter Clausen
2015-10-13 11:11     ` Lars-Peter Clausen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.