[PATCH v2 1/4] iio: ad5755: add support for dt bindings

[PATCH v2 1/4] iio: ad5755: add support for dt bindings

[Sean Nyekjaer]

Still missing a way to provide default modes and settings
for the driver from dt, but I see this as a start.

Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
---
Changes since v1:
- none

 drivers/iio/dac/ad5755.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index bfb350a..e1b6e78 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -607,6 +607,16 @@ static const struct spi_device_id ad5755_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, ad5755_id);
 
+static const struct of_device_id ad5755_of_match[] = {
+	{ .compatible = "adi,ad5755" },
+	{ .compatible = "adi,ad5755-1" },
+	{ .compatible = "adi,ad5757" },
+	{ .compatible = "adi,ad5735" },
+	{ .compatible = "adi,ad5737" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5755_of_match);
+
 static struct spi_driver ad5755_driver = {
 	.driver = {
 		.name = "ad5755",
-- 
2.7.0

[PATCH v2 1/4] iio: ad5755: add support for dt bindings

[Sean Nyekjaer]

Still missing a way to provide default modes and settings
for the driver from dt, but I see this as a start.

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
Changes since v1:
- none

 drivers/iio/dac/ad5755.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index bfb350a..e1b6e78 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -607,6 +607,16 @@ static const struct spi_device_id ad5755_id[] = {
 };
 MODULE_DEVICE_TABLE(spi, ad5755_id);
 
+static const struct of_device_id ad5755_of_match[] = {
+	{ .compatible = "adi,ad5755" },
+	{ .compatible = "adi,ad5755-1" },
+	{ .compatible = "adi,ad5757" },
+	{ .compatible = "adi,ad5735" },
+	{ .compatible = "adi,ad5737" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5755_of_match);
+
 static struct spi_driver ad5755_driver = {
 	.driver = {
 		.name = "ad5755",
-- 
2.7.0

[PATCH v2 2/4] iio: ad5755: Add DT binding documentation

[Sean Nyekjaer]

Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
---
Changes since v1:
- none

 .../devicetree/bindings/iio/dac/ad5755.txt         | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
new file mode 100644
index 0000000..4c91583
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
@@ -0,0 +1,24 @@
+* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver
+
+Required properties:
+ - compatible: Has to contain one of the following:
+	adi,ad5755
+	adi,ad5755-1
+	adi,ad5757
+	adi,ad5735
+	adi,ad5737
+
+ - reg: spi chip select number for the device
+ - spi-cpha or spi-cpol: is the only modes that is supported
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+		Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Example:
+dac@0 {
+	compatible = "adi,ad5755";
+	reg = <0>;
+	spi-max-frequency = <1000000>;
+	spi-cpha;
+};
-- 
2.7.0

[PATCH v2 2/4] iio: ad5755: Add DT binding documentation

[Sean Nyekjaer]

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
Changes since v1:
- none

 .../devicetree/bindings/iio/dac/ad5755.txt         | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
new file mode 100644
index 0000000..4c91583
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
@@ -0,0 +1,24 @@
+* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver
+
+Required properties:
+ - compatible: Has to contain one of the following:
+	adi,ad5755
+	adi,ad5755-1
+	adi,ad5757
+	adi,ad5735
+	adi,ad5737
+
+ - reg: spi chip select number for the device
+ - spi-cpha or spi-cpol: is the only modes that is supported
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+		Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Example:
+dac@0 {
+	compatible = "adi,ad5755";
+	reg = <0>;
+	spi-max-frequency = <1000000>;
+	spi-cpha;
+};
-- 
2.7.0

[PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjaer]

Devicetree can provide platform data

Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
---
Changes since v1:
- this patch added

I'm adding dt bindings header file with the same info as, the platform data as
the devicetree can't understand enums.
Please give an idea to work around this :-)

 drivers/iio/dac/ad5755.c             | 87 +++++++++++++++++++++++++++++++++++-
 include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/iio/adi,ad5755.h

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index e1b6e78..c1e2546 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -14,9 +14,11 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
+#include <linux/of.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/platform_data/ad5755.h>
+#include <dt-bindings/iio/adi,ad5755.h>
 
 #define AD5755_NUM_CHANNELS 4
 
@@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = {
 	},
 };
 
+#ifdef CONFIG_OF
+static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *pp;
+	struct ad5755_platform_data *pdata;
+	unsigned int tmp;
+	unsigned int tmparray[3];
+	int devnr;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->ext_dc_dc_compenstation_resistor =
+	       of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor");
+
+	if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
+		pdata->dc_dc_phase = tmp;
+	else
+		pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
+
+	if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
+		pdata->dc_dc_freq = tmp;
+	else
+		pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
+
+	if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
+		pdata->dc_dc_maxv = tmp;
+	else
+		pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
+
+	devnr = 0;
+	for_each_child_of_node(np, pp) {
+		if (devnr > AD5755_NUM_CHANNELS) {
+			dev_err(dev, "There is to many channels defined in DT\n");
+			goto error_out;
+		}
+
+		if (!of_property_read_u32(pp, "adi,mode", &tmp))
+			pdata->dac[devnr].mode = tmp;
+		else
+			pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
+
+		pdata->dac[devnr].ext_current_sense_resistor =
+		       of_property_read_bool(pp, "adi,ext_current_sense_resistor");
+
+		pdata->dac[devnr].enable_voltage_overrange =
+			of_property_read_bool(pp, "adi,enable_voltage_overrange");
+		if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
+			pdata->dac[devnr].slew.enable = tmparray[0];
+			pdata->dac[devnr].slew.rate = tmparray[1];
+			pdata->dac[devnr].slew.step_size = tmparray[2];
+		} else {
+			pdata->dac[devnr].slew.enable = false;
+			pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
+			pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
+		}
+
+		devnr++;
+	}
+
+	return pdata;
+
+error_out:
+	devm_kfree(dev, pdata);
+	return NULL;
+}
+#else
+static
+struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int ad5755_probe(struct spi_device *spi)
 {
 	enum ad5755_type type = spi_get_device_id(spi)->driver_data;
@@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->num_channels = AD5755_NUM_CHANNELS;
 
-	if (!pdata)
+	if (spi->dev.of_node)
+		pdata = ad5755_parse_dt(&spi->dev);
+	else
+		pdata = spi->dev.platform_data;
+
+	if (!pdata) {
+		dev_warn(&spi->dev, "no platform data? using default\n");
 		pdata = &ad5755_default_pdata;
+	}
 
 	ret = ad5755_init_channels(indio_dev, pdata);
 	if (ret)
diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h
new file mode 100644
index 0000000..117466b
--- /dev/null
+++ b/include/dt-bindings/iio/adi,ad5755.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_AD5755_H
+#define _DT_BINDINGS_AD5755_H
+
+#define	AD5755_MODE_VOLTAGE_0V_5V			0
+#define AD5755_MODE_VOLTAGE_0V_10V			1
+#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V		2
+#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V		3
+#define AD5755_MODE_CURRENT_4mA_20mA			4
+#define AD5755_MODE_CURRENT_0mA_20mA			5
+#define AD5755_MODE_CURRENT_0mA_24mA			6
+
+#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE		0
+#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE	1
+#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE	2
+#define AD5755_DC_DC_PHASE_90_DEGREE			3
+
+#define AD5755_DC_DC_FREQ_250kHZ			0
+#define AD5755_DC_DC_FREQ_410kHZ			1
+#define AD5755_DC_DC_FREQ_650kHZ			2
+
+#define AD5755_DC_DC_MAXV_23V				0
+#define AD5755_DC_DC_MAXV_24V5				1
+#define AD5755_DC_DC_MAXV_27V				2
+#define AD5755_DC_DC_MAXV_29V5				3
+
+#define AD5755_SLEW_RATE_64k				0
+#define AD5755_SLEW_RATE_32k				1
+#define AD5755_SLEW_RATE_16k				2
+#define AD5755_SLEW_RATE_8k				3
+#define AD5755_SLEW_RATE_4k				4
+#define AD5755_SLEW_RATE_2k				5
+#define AD5755_SLEW_RATE_1k				6
+#define AD5755_SLEW_RATE_500				7
+#define AD5755_SLEW_RATE_250				8
+#define AD5755_SLEW_RATE_125				9
+#define AD5755_SLEW_RATE_64				10
+#define AD5755_SLEW_RATE_32				11
+#define AD5755_SLEW_RATE_16				12
+#define AD5755_SLEW_RATE_8				13
+#define AD5755_SLEW_RATE_4				14
+#define AD5755_SLEW_RATE_0_5				15
+
+#define AD5755_SLEW_STEP_SIZE_1				0
+#define AD5755_SLEW_STEP_SIZE_2				1
+#define AD5755_SLEW_STEP_SIZE_4				2
+#define AD5755_SLEW_STEP_SIZE_8				3
+#define AD5755_SLEW_STEP_SIZE_16 			4
+#define AD5755_SLEW_STEP_SIZE_32			5
+#define AD5755_SLEW_STEP_SIZE_64			6
+#define AD5755_SLEW_STEP_SIZE_128			7
+#define AD5755_SLEW_STEP_SIZE_256			8
+
+#endif				/* _DT_BINDINGS_AD5755_H */
-- 
2.7.0

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

[PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjaer]

Devicetree can provide platform data

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
Changes since v1:
- this patch added

I'm adding dt bindings header file with the same info as, the platform data as
the devicetree can't understand enums.
Please give an idea to work around this :-)

 drivers/iio/dac/ad5755.c             | 87 +++++++++++++++++++++++++++++++++++-
 include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/iio/adi,ad5755.h

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index e1b6e78..c1e2546 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -14,9 +14,11 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/delay.h>
+#include <linux/of.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/platform_data/ad5755.h>
+#include <dt-bindings/iio/adi,ad5755.h>
 
 #define AD5755_NUM_CHANNELS 4
 
@@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = {
 	},
 };
 
+#ifdef CONFIG_OF
+static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *pp;
+	struct ad5755_platform_data *pdata;
+	unsigned int tmp;
+	unsigned int tmparray[3];
+	int devnr;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->ext_dc_dc_compenstation_resistor =
+	       of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor");
+
+	if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
+		pdata->dc_dc_phase = tmp;
+	else
+		pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
+
+	if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
+		pdata->dc_dc_freq = tmp;
+	else
+		pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
+
+	if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
+		pdata->dc_dc_maxv = tmp;
+	else
+		pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
+
+	devnr = 0;
+	for_each_child_of_node(np, pp) {
+		if (devnr > AD5755_NUM_CHANNELS) {
+			dev_err(dev, "There is to many channels defined in DT\n");
+			goto error_out;
+		}
+
+		if (!of_property_read_u32(pp, "adi,mode", &tmp))
+			pdata->dac[devnr].mode = tmp;
+		else
+			pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
+
+		pdata->dac[devnr].ext_current_sense_resistor =
+		       of_property_read_bool(pp, "adi,ext_current_sense_resistor");
+
+		pdata->dac[devnr].enable_voltage_overrange =
+			of_property_read_bool(pp, "adi,enable_voltage_overrange");
+		if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
+			pdata->dac[devnr].slew.enable = tmparray[0];
+			pdata->dac[devnr].slew.rate = tmparray[1];
+			pdata->dac[devnr].slew.step_size = tmparray[2];
+		} else {
+			pdata->dac[devnr].slew.enable = false;
+			pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
+			pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
+		}
+
+		devnr++;
+	}
+
+	return pdata;
+
+error_out:
+	devm_kfree(dev, pdata);
+	return NULL;
+}
+#else
+static
+struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int ad5755_probe(struct spi_device *spi)
 {
 	enum ad5755_type type = spi_get_device_id(spi)->driver_data;
@@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->num_channels = AD5755_NUM_CHANNELS;
 
-	if (!pdata)
+	if (spi->dev.of_node)
+		pdata = ad5755_parse_dt(&spi->dev);
+	else
+		pdata = spi->dev.platform_data;
+
+	if (!pdata) {
+		dev_warn(&spi->dev, "no platform data? using default\n");
 		pdata = &ad5755_default_pdata;
+	}
 
 	ret = ad5755_init_channels(indio_dev, pdata);
 	if (ret)
diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h
new file mode 100644
index 0000000..117466b
--- /dev/null
+++ b/include/dt-bindings/iio/adi,ad5755.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_AD5755_H
+#define _DT_BINDINGS_AD5755_H
+
+#define	AD5755_MODE_VOLTAGE_0V_5V			0
+#define AD5755_MODE_VOLTAGE_0V_10V			1
+#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V		2
+#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V		3
+#define AD5755_MODE_CURRENT_4mA_20mA			4
+#define AD5755_MODE_CURRENT_0mA_20mA			5
+#define AD5755_MODE_CURRENT_0mA_24mA			6
+
+#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE		0
+#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE	1
+#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE	2
+#define AD5755_DC_DC_PHASE_90_DEGREE			3
+
+#define AD5755_DC_DC_FREQ_250kHZ			0
+#define AD5755_DC_DC_FREQ_410kHZ			1
+#define AD5755_DC_DC_FREQ_650kHZ			2
+
+#define AD5755_DC_DC_MAXV_23V				0
+#define AD5755_DC_DC_MAXV_24V5				1
+#define AD5755_DC_DC_MAXV_27V				2
+#define AD5755_DC_DC_MAXV_29V5				3
+
+#define AD5755_SLEW_RATE_64k				0
+#define AD5755_SLEW_RATE_32k				1
+#define AD5755_SLEW_RATE_16k				2
+#define AD5755_SLEW_RATE_8k				3
+#define AD5755_SLEW_RATE_4k				4
+#define AD5755_SLEW_RATE_2k				5
+#define AD5755_SLEW_RATE_1k				6
+#define AD5755_SLEW_RATE_500				7
+#define AD5755_SLEW_RATE_250				8
+#define AD5755_SLEW_RATE_125				9
+#define AD5755_SLEW_RATE_64				10
+#define AD5755_SLEW_RATE_32				11
+#define AD5755_SLEW_RATE_16				12
+#define AD5755_SLEW_RATE_8				13
+#define AD5755_SLEW_RATE_4				14
+#define AD5755_SLEW_RATE_0_5				15
+
+#define AD5755_SLEW_STEP_SIZE_1				0
+#define AD5755_SLEW_STEP_SIZE_2				1
+#define AD5755_SLEW_STEP_SIZE_4				2
+#define AD5755_SLEW_STEP_SIZE_8				3
+#define AD5755_SLEW_STEP_SIZE_16 			4
+#define AD5755_SLEW_STEP_SIZE_32			5
+#define AD5755_SLEW_STEP_SIZE_64			6
+#define AD5755_SLEW_STEP_SIZE_128			7
+#define AD5755_SLEW_STEP_SIZE_256			8
+
+#endif				/* _DT_BINDINGS_AD5755_H */
-- 
2.7.0

[PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Sean Nyekjaer]

Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
---
Changes since v1:
- this patch added

 .../devicetree/bindings/iio/dac/ad5755.txt         | 83 ++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
index 4c91583..3aac20d 100644
--- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt
+++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
@@ -15,10 +15,93 @@ Recommended properties:
  - spi-max-frequency: Definition as per
 		Documentation/devicetree/bindings/spi/spi-bus.txt
 
+Optional properties:
+See include/dt-bindings/iio/ad5755.h
+ - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor
+ - adi,dc_dc_phase: can be set to
+	AD5755_DC_DC_PHASE_ALL_SAME_EDGE
+	AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE
+	AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE
+	AD5755_DC_DC_PHASE_90_DEGREE
+ - adi,dc_dc_freq:
+	AD5755_DC_DC_FREQ_250kHZ
+	AD5755_DC_DC_FREQ_410kHZ
+	AD5755_DC_DC_FREQ_650kHZ
+ - adi,dc_dc_maxv:
+	AD5755_DC_DC_MAXV_23V
+	AD5755_DC_DC_MAXV_24V5
+	AD5755_DC_DC_MAXV_27V
+	AD5755_DC_DC_MAXV_29V5
+
+Optional for every channel:
+	 - adi,mode:
+		AD5755_MODE_VOLTAGE_0V_5V
+		AD5755_MODE_VOLTAGE_0V_10V
+		AD5755_MODE_VOLTAGE_PLUSMINUS_5V
+		AD5755_MODE_VOLTAGE_PLUSMINUS_10V
+		AD5755_MODE_CURRENT_4mA_20mA
+		AD5755_MODE_CURRENT_0mA_20mA
+		AD5755_MODE_CURRENT_0mA_24mA
+	 - adi,ext_current_sense_resistor: boolean set if using external
+					   current sense resistor
+	 - adi,enable_voltage_overrange: boolean enable voltage overrange
+	 - adi,slew: Array of slewrate settings should contain 3 fields:
+		1: Should be either 0 or 1 in order to enable or disable slew
+		2: Slew rate settings:
+			AD5755_SLEW_RATE_64k
+			AD5755_SLEW_RATE_32k
+			AD5755_SLEW_RATE_16k
+			AD5755_SLEW_RATE_8k
+			AD5755_SLEW_RATE_4k
+			AD5755_SLEW_RATE_2k
+			AD5755_SLEW_RATE_1k
+			AD5755_SLEW_RATE_500
+			AD5755_SLEW_RATE_250
+			AD5755_SLEW_RATE_125
+			AD5755_SLEW_RATE_64
+			AD5755_SLEW_RATE_32
+			AD5755_SLEW_RATE_16
+			AD5755_SLEW_RATE_8
+			AD5755_SLEW_RATE_4
+			AD5755_SLEW_RATE_0_5
+		3: Slew step size:
+			AD5755_SLEW_STEP_SIZE_1
+			AD5755_SLEW_STEP_SIZE_2
+			AD5755_SLEW_STEP_SIZE_4
+			AD5755_SLEW_STEP_SIZE_8
+			AD5755_SLEW_STEP_SIZE_16
+			AD5755_SLEW_STEP_SIZE_32
+			AD5755_SLEW_STEP_SIZE_64
+			AD5755_SLEW_STEP_SIZE_128
+			AD5755_SLEW_STEP_SIZE_256
+
 Example:
 dac@0 {
 	compatible = "adi,ad5755";
 	reg = <0>;
 	spi-max-frequency = <1000000>;
 	spi-cpha;
+	adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>;
+	adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>;
+	adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>;
+	channel@0 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@1 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@2 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@3 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
 };
-- 
2.7.0

[PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Sean Nyekjaer]

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
Changes since v1:
- this patch added

 .../devicetree/bindings/iio/dac/ad5755.txt         | 83 ++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
index 4c91583..3aac20d 100644
--- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt
+++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
@@ -15,10 +15,93 @@ Recommended properties:
  - spi-max-frequency: Definition as per
 		Documentation/devicetree/bindings/spi/spi-bus.txt
 
+Optional properties:
+See include/dt-bindings/iio/ad5755.h
+ - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor
+ - adi,dc_dc_phase: can be set to
+	AD5755_DC_DC_PHASE_ALL_SAME_EDGE
+	AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE
+	AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE
+	AD5755_DC_DC_PHASE_90_DEGREE
+ - adi,dc_dc_freq:
+	AD5755_DC_DC_FREQ_250kHZ
+	AD5755_DC_DC_FREQ_410kHZ
+	AD5755_DC_DC_FREQ_650kHZ
+ - adi,dc_dc_maxv:
+	AD5755_DC_DC_MAXV_23V
+	AD5755_DC_DC_MAXV_24V5
+	AD5755_DC_DC_MAXV_27V
+	AD5755_DC_DC_MAXV_29V5
+
+Optional for every channel:
+	 - adi,mode:
+		AD5755_MODE_VOLTAGE_0V_5V
+		AD5755_MODE_VOLTAGE_0V_10V
+		AD5755_MODE_VOLTAGE_PLUSMINUS_5V
+		AD5755_MODE_VOLTAGE_PLUSMINUS_10V
+		AD5755_MODE_CURRENT_4mA_20mA
+		AD5755_MODE_CURRENT_0mA_20mA
+		AD5755_MODE_CURRENT_0mA_24mA
+	 - adi,ext_current_sense_resistor: boolean set if using external
+					   current sense resistor
+	 - adi,enable_voltage_overrange: boolean enable voltage overrange
+	 - adi,slew: Array of slewrate settings should contain 3 fields:
+		1: Should be either 0 or 1 in order to enable or disable slew
+		2: Slew rate settings:
+			AD5755_SLEW_RATE_64k
+			AD5755_SLEW_RATE_32k
+			AD5755_SLEW_RATE_16k
+			AD5755_SLEW_RATE_8k
+			AD5755_SLEW_RATE_4k
+			AD5755_SLEW_RATE_2k
+			AD5755_SLEW_RATE_1k
+			AD5755_SLEW_RATE_500
+			AD5755_SLEW_RATE_250
+			AD5755_SLEW_RATE_125
+			AD5755_SLEW_RATE_64
+			AD5755_SLEW_RATE_32
+			AD5755_SLEW_RATE_16
+			AD5755_SLEW_RATE_8
+			AD5755_SLEW_RATE_4
+			AD5755_SLEW_RATE_0_5
+		3: Slew step size:
+			AD5755_SLEW_STEP_SIZE_1
+			AD5755_SLEW_STEP_SIZE_2
+			AD5755_SLEW_STEP_SIZE_4
+			AD5755_SLEW_STEP_SIZE_8
+			AD5755_SLEW_STEP_SIZE_16
+			AD5755_SLEW_STEP_SIZE_32
+			AD5755_SLEW_STEP_SIZE_64
+			AD5755_SLEW_STEP_SIZE_128
+			AD5755_SLEW_STEP_SIZE_256
+
 Example:
 dac@0 {
 	compatible = "adi,ad5755";
 	reg = <0>;
 	spi-max-frequency = <1000000>;
 	spi-cpha;
+	adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>;
+	adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>;
+	adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>;
+	channel@0 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@1 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@2 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
+	channel@3 {
+		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
+		adi,ext_current_sense_resistor;
+		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
+	};
 };
-- 
2.7.0

Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Jonathan Cameron]

On 03/02/16 14:31, Sean Nyekjaer wrote:
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
Hmm. Complex beast.  Going to want Lars to look at this in particular.
Also a device tree review would be good on this one.

My personal gut feeling is not to wrap up things that are really numeric
with names, but rather to provide docs on the valid values for them.
Not sure what is the preferred by the device tree guys??

Jonathan
> ---
> Changes since v1:
> - this patch added
> 
>  .../devicetree/bindings/iio/dac/ad5755.txt         | 83 ++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> index 4c91583..3aac20d 100644
> --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> @@ -15,10 +15,93 @@ Recommended properties:
>   - spi-max-frequency: Definition as per
>  		Documentation/devicetree/bindings/spi/spi-bus.txt
>  
> +Optional properties:
> +See include/dt-bindings/iio/ad5755.h
> + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor
> + - adi,dc_dc_phase: can be set to
> +	AD5755_DC_DC_PHASE_ALL_SAME_EDGE
> +	AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE
> +	AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE
> +	AD5755_DC_DC_PHASE_90_DEGREE
> + - adi,dc_dc_freq:
> +	AD5755_DC_DC_FREQ_250kHZ
> +	AD5755_DC_DC_FREQ_410kHZ
> +	AD5755_DC_DC_FREQ_650kHZ
Can we not just make this numeric with a set of valid values specified in this doc?
> + - adi,dc_dc_maxv:
> +	AD5755_DC_DC_MAXV_23V
> +	AD5755_DC_DC_MAXV_24V5
> +	AD5755_DC_DC_MAXV_27V
> +	AD5755_DC_DC_MAXV_29V5
Same on this.
> +
> +Optional for every channel:
> +	 - adi,mode:
> +		AD5755_MODE_VOLTAGE_0V_5V
> +		AD5755_MODE_VOLTAGE_0V_10V
> +		AD5755_MODE_VOLTAGE_PLUSMINUS_5V
> +		AD5755_MODE_VOLTAGE_PLUSMINUS_10V
> +		AD5755_MODE_CURRENT_4mA_20mA
> +		AD5755_MODE_CURRENT_0mA_20mA
> +		AD5755_MODE_CURRENT_0mA_24mA
Again, could break this into two numeric values and have options specified in the
doc + reject any invalid ones. 
> +	 - adi,ext_current_sense_resistor: boolean set if using external
> +					   current sense resistor
> +	 - adi,enable_voltage_overrange: boolean enable voltage overrange
> +	 - adi,slew: Array of slewrate settings should contain 3 fields:
> +		1: Should be either 0 or 1 in order to enable or disable slew
> +		2: Slew rate settings:
> +			AD5755_SLEW_RATE_64k
> +			AD5755_SLEW_RATE_32k
> +			AD5755_SLEW_RATE_16k
> +			AD5755_SLEW_RATE_8k
> +			AD5755_SLEW_RATE_4k
> +			AD5755_SLEW_RATE_2k
> +			AD5755_SLEW_RATE_1k
> +			AD5755_SLEW_RATE_500
> +			AD5755_SLEW_RATE_250
> +			AD5755_SLEW_RATE_125
> +			AD5755_SLEW_RATE_64
> +			AD5755_SLEW_RATE_32
> +			AD5755_SLEW_RATE_16
> +			AD5755_SLEW_RATE_8
> +			AD5755_SLEW_RATE_4
> +			AD5755_SLEW_RATE_0_5
> +		3: Slew step size:
> +			AD5755_SLEW_STEP_SIZE_1
> +			AD5755_SLEW_STEP_SIZE_2
> +			AD5755_SLEW_STEP_SIZE_4
> +			AD5755_SLEW_STEP_SIZE_8
> +			AD5755_SLEW_STEP_SIZE_16
> +			AD5755_SLEW_STEP_SIZE_32
> +			AD5755_SLEW_STEP_SIZE_64
> +			AD5755_SLEW_STEP_SIZE_128
> +			AD5755_SLEW_STEP_SIZE_256
Same with these.
> +
>  Example:
>  dac@0 {
>  	compatible = "adi,ad5755";
>  	reg = <0>;
>  	spi-max-frequency = <1000000>;
>  	spi-cpha;
> +	adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>;
> +	adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>;
> +	adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>;
> +	channel@0 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@1 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@2 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@3 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
>  };
> 

Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Jonathan Cameron]

On 03/02/16 14:31, Sean Nyekjaer wrote:
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Hmm. Complex beast.  Going to want Lars to look at this in particular.
Also a device tree review would be good on this one.

My personal gut feeling is not to wrap up things that are really numeric
with names, but rather to provide docs on the valid values for them.
Not sure what is the preferred by the device tree guys??

Jonathan
> ---
> Changes since v1:
> - this patch added
> 
>  .../devicetree/bindings/iio/dac/ad5755.txt         | 83 ++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> index 4c91583..3aac20d 100644
> --- a/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> @@ -15,10 +15,93 @@ Recommended properties:
>   - spi-max-frequency: Definition as per
>  		Documentation/devicetree/bindings/spi/spi-bus.txt
>  
> +Optional properties:
> +See include/dt-bindings/iio/ad5755.h
> + - adi,ext_dc_dc_compenstation_resistor: boolean set if using compensation resistor
> + - adi,dc_dc_phase: can be set to
> +	AD5755_DC_DC_PHASE_ALL_SAME_EDGE
> +	AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE
> +	AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE
> +	AD5755_DC_DC_PHASE_90_DEGREE
> + - adi,dc_dc_freq:
> +	AD5755_DC_DC_FREQ_250kHZ
> +	AD5755_DC_DC_FREQ_410kHZ
> +	AD5755_DC_DC_FREQ_650kHZ
Can we not just make this numeric with a set of valid values specified in this doc?
> + - adi,dc_dc_maxv:
> +	AD5755_DC_DC_MAXV_23V
> +	AD5755_DC_DC_MAXV_24V5
> +	AD5755_DC_DC_MAXV_27V
> +	AD5755_DC_DC_MAXV_29V5
Same on this.
> +
> +Optional for every channel:
> +	 - adi,mode:
> +		AD5755_MODE_VOLTAGE_0V_5V
> +		AD5755_MODE_VOLTAGE_0V_10V
> +		AD5755_MODE_VOLTAGE_PLUSMINUS_5V
> +		AD5755_MODE_VOLTAGE_PLUSMINUS_10V
> +		AD5755_MODE_CURRENT_4mA_20mA
> +		AD5755_MODE_CURRENT_0mA_20mA
> +		AD5755_MODE_CURRENT_0mA_24mA
Again, could break this into two numeric values and have options specified in the
doc + reject any invalid ones. 
> +	 - adi,ext_current_sense_resistor: boolean set if using external
> +					   current sense resistor
> +	 - adi,enable_voltage_overrange: boolean enable voltage overrange
> +	 - adi,slew: Array of slewrate settings should contain 3 fields:
> +		1: Should be either 0 or 1 in order to enable or disable slew
> +		2: Slew rate settings:
> +			AD5755_SLEW_RATE_64k
> +			AD5755_SLEW_RATE_32k
> +			AD5755_SLEW_RATE_16k
> +			AD5755_SLEW_RATE_8k
> +			AD5755_SLEW_RATE_4k
> +			AD5755_SLEW_RATE_2k
> +			AD5755_SLEW_RATE_1k
> +			AD5755_SLEW_RATE_500
> +			AD5755_SLEW_RATE_250
> +			AD5755_SLEW_RATE_125
> +			AD5755_SLEW_RATE_64
> +			AD5755_SLEW_RATE_32
> +			AD5755_SLEW_RATE_16
> +			AD5755_SLEW_RATE_8
> +			AD5755_SLEW_RATE_4
> +			AD5755_SLEW_RATE_0_5
> +		3: Slew step size:
> +			AD5755_SLEW_STEP_SIZE_1
> +			AD5755_SLEW_STEP_SIZE_2
> +			AD5755_SLEW_STEP_SIZE_4
> +			AD5755_SLEW_STEP_SIZE_8
> +			AD5755_SLEW_STEP_SIZE_16
> +			AD5755_SLEW_STEP_SIZE_32
> +			AD5755_SLEW_STEP_SIZE_64
> +			AD5755_SLEW_STEP_SIZE_128
> +			AD5755_SLEW_STEP_SIZE_256
Same with these.
> +
>  Example:
>  dac@0 {
>  	compatible = "adi,ad5755";
>  	reg = <0>;
>  	spi-max-frequency = <1000000>;
>  	spi-cpha;
> +	adi,dc_dc_phase = <AD5755_DC_DC_PHASE_ALL_SAME_EDGE>;
> +	adi,dc_dc_freq = <AD5755_DC_DC_FREQ_410kHZ>;
> +	adi,dc_dc_maxv = <AD5755_DC_DC_MAXV_23V>;
> +	channel@0 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@1 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@2 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
> +	channel@3 {
> +		adi,mode = <AD5755_MODE_CURRENT_4mA_20mA>;
> +		adi,ext_current_sense_resistor;
> +		adi,slew = <0 AD5755_SLEW_RATE_64k AD5755_SLEW_STEP_SIZE_1>;
> +	};
>  };
> 

Re: [PATCH v2 2/4] iio: ad5755: Add DT binding documentation

[Jonathan Cameron]

On 03/02/16 14:31, Sean Nyekjaer wrote:
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
Please pick up Rob's Ack in any future versions of the series
(I'm sending this as mostly as a reminder to myself if this one goes
in as it is!)
> ---
> Changes since v1:
> - none
> 
>  .../devicetree/bindings/iio/dac/ad5755.txt         | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> new file mode 100644
> index 0000000..4c91583
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> @@ -0,0 +1,24 @@
> +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver
> +
> +Required properties:
> + - compatible: Has to contain one of the following:
> +	adi,ad5755
> +	adi,ad5755-1
> +	adi,ad5757
> +	adi,ad5735
> +	adi,ad5737
> +
> + - reg: spi chip select number for the device
> + - spi-cpha or spi-cpol: is the only modes that is supported
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> +		Documentation/devicetree/bindings/spi/spi-bus.txt
> +
> +Example:
> +dac@0 {
> +	compatible = "adi,ad5755";
> +	reg = <0>;
> +	spi-max-frequency = <1000000>;
> +	spi-cpha;
> +};
> 

Re: [PATCH v2 2/4] iio: ad5755: Add DT binding documentation

[Jonathan Cameron]

On 03/02/16 14:31, Sean Nyekjaer wrote:
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Please pick up Rob's Ack in any future versions of the series
(I'm sending this as mostly as a reminder to myself if this one goes
in as it is!)
> ---
> Changes since v1:
> - none
> 
>  .../devicetree/bindings/iio/dac/ad5755.txt         | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ad5755.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ad5755.txt b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> new file mode 100644
> index 0000000..4c91583
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ad5755.txt
> @@ -0,0 +1,24 @@
> +* Analog Device AD5755 IIO Multi-Channel DAC Linux Driver
> +
> +Required properties:
> + - compatible: Has to contain one of the following:
> +	adi,ad5755
> +	adi,ad5755-1
> +	adi,ad5757
> +	adi,ad5735
> +	adi,ad5737
> +
> + - reg: spi chip select number for the device
> + - spi-cpha or spi-cpol: is the only modes that is supported
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> +		Documentation/devicetree/bindings/spi/spi-bus.txt
> +
> +Example:
> +dac@0 {
> +	compatible = "adi,ad5755";
> +	reg = <0>;
> +	spi-max-frequency = <1000000>;
> +	spi-cpha;
> +};
> 

Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Rob Herring]

On Sat, Feb 06, 2016 at 06:30:04PM +0000, Jonathan Cameron wrote:
> On 03/02/16 14:31, Sean Nyekjaer wrote:
> > Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
> Hmm. Complex beast.  Going to want Lars to look at this in particular.
> Also a device tree review would be good on this one.
> 
> My personal gut feeling is not to wrap up things that are really numeric
> with names, but rather to provide docs on the valid values for them.
> Not sure what is the preferred by the device tree guys??

Agreed. I'm not such a fan of defines in DT.

Also, if there is some possibility that bindings could be generic, I 
prefer to see them use the native units rather than the enumerated 
register values. The downside is adding the translation into the driver. 
These look like they could possibly be generic DAC properties.

Rob

Re: [PATCH v2 4/4] iio: ad5755: Add full DT binding documentation

[Rob Herring]

On Sat, Feb 06, 2016 at 06:30:04PM +0000, Jonathan Cameron wrote:
> On 03/02/16 14:31, Sean Nyekjaer wrote:
> > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> Hmm. Complex beast.  Going to want Lars to look at this in particular.
> Also a device tree review would be good on this one.
> 
> My personal gut feeling is not to wrap up things that are really numeric
> with names, but rather to provide docs on the valid values for them.
> Not sure what is the preferred by the device tree guys??

Agreed. I'm not such a fan of defines in DT.

Also, if there is some possibility that bindings could be generic, I 
prefer to see them use the native units rather than the enumerated 
register values. The downside is adding the translation into the driver. 
These look like they could possibly be generic DAC properties.

Rob

Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjær]

On 2016-02-03 15:31, Sean Nyekjaer wrote:
> Devicetree can provide platform data
>
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
Lars do you have time to look at this before i'm fixing the comments 
from Jonathan and Rob :-)

Sean
> ---
> Changes since v1:
> - this patch added
>
> I'm adding dt bindings header file with the same info as, the platform data as
> the devicetree can't understand enums.
> Please give an idea to work around this :-)
>
>   drivers/iio/dac/ad5755.c             | 87 +++++++++++++++++++++++++++++++++++-
>   include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++
>   2 files changed, 152 insertions(+), 1 deletion(-)
>   create mode 100644 include/dt-bindings/iio/adi,ad5755.h
>
> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> index e1b6e78..c1e2546 100644
> --- a/drivers/iio/dac/ad5755.c
> +++ b/drivers/iio/dac/ad5755.c
> @@ -14,9 +14,11 @@
>   #include <linux/slab.h>
>   #include <linux/sysfs.h>
>   #include <linux/delay.h>
> +#include <linux/of.h>
>   #include <linux/iio/iio.h>
>   #include <linux/iio/sysfs.h>
>   #include <linux/platform_data/ad5755.h>
> +#include <dt-bindings/iio/adi,ad5755.h>
>   
>   #define AD5755_NUM_CHANNELS 4
>   
> @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = {
>   	},
>   };
>   
> +#ifdef CONFIG_OF
> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *pp;
> +	struct ad5755_platform_data *pdata;
> +	unsigned int tmp;
> +	unsigned int tmparray[3];
> +	int devnr;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	pdata->ext_dc_dc_compenstation_resistor =
> +	       of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor");
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
> +		pdata->dc_dc_phase = tmp;
> +	else
> +		pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
> +		pdata->dc_dc_freq = tmp;
> +	else
> +		pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
> +		pdata->dc_dc_maxv = tmp;
> +	else
> +		pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
> +
> +	devnr = 0;
> +	for_each_child_of_node(np, pp) {
> +		if (devnr > AD5755_NUM_CHANNELS) {
> +			dev_err(dev, "There is to many channels defined in DT\n");
> +			goto error_out;
> +		}
> +
> +		if (!of_property_read_u32(pp, "adi,mode", &tmp))
> +			pdata->dac[devnr].mode = tmp;
> +		else
> +			pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
> +
> +		pdata->dac[devnr].ext_current_sense_resistor =
> +		       of_property_read_bool(pp, "adi,ext_current_sense_resistor");
> +
> +		pdata->dac[devnr].enable_voltage_overrange =
> +			of_property_read_bool(pp, "adi,enable_voltage_overrange");
> +		if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
> +			pdata->dac[devnr].slew.enable = tmparray[0];
> +			pdata->dac[devnr].slew.rate = tmparray[1];
> +			pdata->dac[devnr].slew.step_size = tmparray[2];
> +		} else {
> +			pdata->dac[devnr].slew.enable = false;
> +			pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
> +			pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
> +		}
> +
> +		devnr++;
> +	}
> +
> +	return pdata;
> +
> +error_out:
> +	devm_kfree(dev, pdata);
> +	return NULL;
> +}
> +#else
> +static
> +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>   static int ad5755_probe(struct spi_device *spi)
>   {
>   	enum ad5755_type type = spi_get_device_id(spi)->driver_data;
> @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi)
>   	indio_dev->modes = INDIO_DIRECT_MODE;
>   	indio_dev->num_channels = AD5755_NUM_CHANNELS;
>   
> -	if (!pdata)
> +	if (spi->dev.of_node)
> +		pdata = ad5755_parse_dt(&spi->dev);
> +	else
> +		pdata = spi->dev.platform_data;
> +
> +	if (!pdata) {
> +		dev_warn(&spi->dev, "no platform data? using default\n");
>   		pdata = &ad5755_default_pdata;
> +	}
>   
>   	ret = ad5755_init_channels(indio_dev, pdata);
>   	if (ret)
> diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h
> new file mode 100644
> index 0000000..117466b
> --- /dev/null
> +++ b/include/dt-bindings/iio/adi,ad5755.h
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef _DT_BINDINGS_AD5755_H
> +#define _DT_BINDINGS_AD5755_H
> +
> +#define	AD5755_MODE_VOLTAGE_0V_5V			0
> +#define AD5755_MODE_VOLTAGE_0V_10V			1
> +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V		2
> +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V		3
> +#define AD5755_MODE_CURRENT_4mA_20mA			4
> +#define AD5755_MODE_CURRENT_0mA_20mA			5
> +#define AD5755_MODE_CURRENT_0mA_24mA			6
> +
> +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE		0
> +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE	1
> +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE	2
> +#define AD5755_DC_DC_PHASE_90_DEGREE			3
> +
> +#define AD5755_DC_DC_FREQ_250kHZ			0
> +#define AD5755_DC_DC_FREQ_410kHZ			1
> +#define AD5755_DC_DC_FREQ_650kHZ			2
> +
> +#define AD5755_DC_DC_MAXV_23V				0
> +#define AD5755_DC_DC_MAXV_24V5				1
> +#define AD5755_DC_DC_MAXV_27V				2
> +#define AD5755_DC_DC_MAXV_29V5				3
> +
> +#define AD5755_SLEW_RATE_64k				0
> +#define AD5755_SLEW_RATE_32k				1
> +#define AD5755_SLEW_RATE_16k				2
> +#define AD5755_SLEW_RATE_8k				3
> +#define AD5755_SLEW_RATE_4k				4
> +#define AD5755_SLEW_RATE_2k				5
> +#define AD5755_SLEW_RATE_1k				6
> +#define AD5755_SLEW_RATE_500				7
> +#define AD5755_SLEW_RATE_250				8
> +#define AD5755_SLEW_RATE_125				9
> +#define AD5755_SLEW_RATE_64				10
> +#define AD5755_SLEW_RATE_32				11
> +#define AD5755_SLEW_RATE_16				12
> +#define AD5755_SLEW_RATE_8				13
> +#define AD5755_SLEW_RATE_4				14
> +#define AD5755_SLEW_RATE_0_5				15
> +
> +#define AD5755_SLEW_STEP_SIZE_1				0
> +#define AD5755_SLEW_STEP_SIZE_2				1
> +#define AD5755_SLEW_STEP_SIZE_4				2
> +#define AD5755_SLEW_STEP_SIZE_8				3
> +#define AD5755_SLEW_STEP_SIZE_16 			4
> +#define AD5755_SLEW_STEP_SIZE_32			5
> +#define AD5755_SLEW_STEP_SIZE_64			6
> +#define AD5755_SLEW_STEP_SIZE_128			7
> +#define AD5755_SLEW_STEP_SIZE_256			8
> +
> +#endif				/* _DT_BINDINGS_AD5755_H */

Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjær]

On 2016-02-03 15:31, Sean Nyekjaer wrote:
> Devicetree can provide platform data
>
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Lars do you have time to look at this before i'm fixing the comments 
from Jonathan and Rob :-)

Sean
> ---
> Changes since v1:
> - this patch added
>
> I'm adding dt bindings header file with the same info as, the platform data as
> the devicetree can't understand enums.
> Please give an idea to work around this :-)
>
>   drivers/iio/dac/ad5755.c             | 87 +++++++++++++++++++++++++++++++++++-
>   include/dt-bindings/iio/adi,ad5755.h | 66 +++++++++++++++++++++++++++
>   2 files changed, 152 insertions(+), 1 deletion(-)
>   create mode 100644 include/dt-bindings/iio/adi,ad5755.h
>
> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> index e1b6e78..c1e2546 100644
> --- a/drivers/iio/dac/ad5755.c
> +++ b/drivers/iio/dac/ad5755.c
> @@ -14,9 +14,11 @@
>   #include <linux/slab.h>
>   #include <linux/sysfs.h>
>   #include <linux/delay.h>
> +#include <linux/of.h>
>   #include <linux/iio/iio.h>
>   #include <linux/iio/sysfs.h>
>   #include <linux/platform_data/ad5755.h>
> +#include <dt-bindings/iio/adi,ad5755.h>
>   
>   #define AD5755_NUM_CHANNELS 4
>   
> @@ -556,6 +558,82 @@ static const struct ad5755_platform_data ad5755_default_pdata = {
>   	},
>   };
>   
> +#ifdef CONFIG_OF
> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *pp;
> +	struct ad5755_platform_data *pdata;
> +	unsigned int tmp;
> +	unsigned int tmparray[3];
> +	int devnr;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	pdata->ext_dc_dc_compenstation_resistor =
> +	       of_property_read_bool(np, "adi,ext_dc_dc_compenstation_resistor");
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
> +		pdata->dc_dc_phase = tmp;
> +	else
> +		pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
> +		pdata->dc_dc_freq = tmp;
> +	else
> +		pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
> +
> +	if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
> +		pdata->dc_dc_maxv = tmp;
> +	else
> +		pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
> +
> +	devnr = 0;
> +	for_each_child_of_node(np, pp) {
> +		if (devnr > AD5755_NUM_CHANNELS) {
> +			dev_err(dev, "There is to many channels defined in DT\n");
> +			goto error_out;
> +		}
> +
> +		if (!of_property_read_u32(pp, "adi,mode", &tmp))
> +			pdata->dac[devnr].mode = tmp;
> +		else
> +			pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
> +
> +		pdata->dac[devnr].ext_current_sense_resistor =
> +		       of_property_read_bool(pp, "adi,ext_current_sense_resistor");
> +
> +		pdata->dac[devnr].enable_voltage_overrange =
> +			of_property_read_bool(pp, "adi,enable_voltage_overrange");
> +		if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
> +			pdata->dac[devnr].slew.enable = tmparray[0];
> +			pdata->dac[devnr].slew.rate = tmparray[1];
> +			pdata->dac[devnr].slew.step_size = tmparray[2];
> +		} else {
> +			pdata->dac[devnr].slew.enable = false;
> +			pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
> +			pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
> +		}
> +
> +		devnr++;
> +	}
> +
> +	return pdata;
> +
> +error_out:
> +	devm_kfree(dev, pdata);
> +	return NULL;
> +}
> +#else
> +static
> +struct adf4350_platform_data *adf4350_parse_dt(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
>   static int ad5755_probe(struct spi_device *spi)
>   {
>   	enum ad5755_type type = spi_get_device_id(spi)->driver_data;
> @@ -583,8 +661,15 @@ static int ad5755_probe(struct spi_device *spi)
>   	indio_dev->modes = INDIO_DIRECT_MODE;
>   	indio_dev->num_channels = AD5755_NUM_CHANNELS;
>   
> -	if (!pdata)
> +	if (spi->dev.of_node)
> +		pdata = ad5755_parse_dt(&spi->dev);
> +	else
> +		pdata = spi->dev.platform_data;
> +
> +	if (!pdata) {
> +		dev_warn(&spi->dev, "no platform data? using default\n");
>   		pdata = &ad5755_default_pdata;
> +	}
>   
>   	ret = ad5755_init_channels(indio_dev, pdata);
>   	if (ret)
> diff --git a/include/dt-bindings/iio/adi,ad5755.h b/include/dt-bindings/iio/adi,ad5755.h
> new file mode 100644
> index 0000000..117466b
> --- /dev/null
> +++ b/include/dt-bindings/iio/adi,ad5755.h
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef _DT_BINDINGS_AD5755_H
> +#define _DT_BINDINGS_AD5755_H
> +
> +#define	AD5755_MODE_VOLTAGE_0V_5V			0
> +#define AD5755_MODE_VOLTAGE_0V_10V			1
> +#define AD5755_MODE_VOLTAGE_PLUSMINUS_5V		2
> +#define AD5755_MODE_VOLTAGE_PLUSMINUS_10V		3
> +#define AD5755_MODE_CURRENT_4mA_20mA			4
> +#define AD5755_MODE_CURRENT_0mA_20mA			5
> +#define AD5755_MODE_CURRENT_0mA_24mA			6
> +
> +#define AD5755_DC_DC_PHASE_ALL_SAME_EDGE		0
> +#define AD5755_DC_DC_PHASE_A_B_SAME_EDGE_C_D_OPP_EDGE	1
> +#define AD5755_DC_DC_PHASE_A_C_SAME_EDGE_B_D_OPP_EDGE	2
> +#define AD5755_DC_DC_PHASE_90_DEGREE			3
> +
> +#define AD5755_DC_DC_FREQ_250kHZ			0
> +#define AD5755_DC_DC_FREQ_410kHZ			1
> +#define AD5755_DC_DC_FREQ_650kHZ			2
> +
> +#define AD5755_DC_DC_MAXV_23V				0
> +#define AD5755_DC_DC_MAXV_24V5				1
> +#define AD5755_DC_DC_MAXV_27V				2
> +#define AD5755_DC_DC_MAXV_29V5				3
> +
> +#define AD5755_SLEW_RATE_64k				0
> +#define AD5755_SLEW_RATE_32k				1
> +#define AD5755_SLEW_RATE_16k				2
> +#define AD5755_SLEW_RATE_8k				3
> +#define AD5755_SLEW_RATE_4k				4
> +#define AD5755_SLEW_RATE_2k				5
> +#define AD5755_SLEW_RATE_1k				6
> +#define AD5755_SLEW_RATE_500				7
> +#define AD5755_SLEW_RATE_250				8
> +#define AD5755_SLEW_RATE_125				9
> +#define AD5755_SLEW_RATE_64				10
> +#define AD5755_SLEW_RATE_32				11
> +#define AD5755_SLEW_RATE_16				12
> +#define AD5755_SLEW_RATE_8				13
> +#define AD5755_SLEW_RATE_4				14
> +#define AD5755_SLEW_RATE_0_5				15
> +
> +#define AD5755_SLEW_STEP_SIZE_1				0
> +#define AD5755_SLEW_STEP_SIZE_2				1
> +#define AD5755_SLEW_STEP_SIZE_4				2
> +#define AD5755_SLEW_STEP_SIZE_8				3
> +#define AD5755_SLEW_STEP_SIZE_16 			4
> +#define AD5755_SLEW_STEP_SIZE_32			5
> +#define AD5755_SLEW_STEP_SIZE_64			6
> +#define AD5755_SLEW_STEP_SIZE_128			7
> +#define AD5755_SLEW_STEP_SIZE_256			8
> +
> +#endif				/* _DT_BINDINGS_AD5755_H */

Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Lars-Peter Clausen]

On 02/09/2016 09:22 AM, Sean Nyekjær wrote:
> 
> On 2016-02-03 15:31, Sean Nyekjaer wrote:
>> Devicetree can provide platform data
>>
>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
> Lars do you have time to look at this before i'm fixing the comments from
> Jonathan and Rob :-)

The obvious thing is to use '-' instead of '_' since that is the DT convention.

Other than that this non-trivial. This is the first binding for such a
device and it is always hard to come up with a good ABI without knowing what
other similar devices might need to be supported by the same bindings. And
it is also not always clear what is hardware description and what is driver
policy.

The external resistor configuration should be clear, that's a description of
the hardware whether the resistor exists or not.

For the others it's not so clear. E.g. the DC-to-DC converter configuration,
this is most certainly not a description of the hardware. But we could get
away with it by saying this is a system level design decision and is made by
the system designer based on the operating parameters of the hardware and
hence is policy that belongs in the DT. It might make sense to follow what
the regulator bindings do here, even though the regulator is not exposed
externally.

The configuration of the output pin (voltage level, current level, etc) is
certainly influenced by the restrictions of the hardware itself. You don't
want to allow a configuration that can damage the hardware. But on the other
hand it is still a valid usecase for some applications to switch between two
setups at runtime, which is something you said you need to be able to do.
Here we might want to use something similar to pinctrl where the devicetree
can offer a selection of valid configurations and the driver can switch
between those configurations.

- Lars

[...]
>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
>> +{
>> +    struct device_node *np = dev->of_node;
>> +    struct device_node *pp;
>> +    struct ad5755_platform_data *pdata;
>> +    unsigned int tmp;
>> +    unsigned int tmparray[3];
>> +    int devnr;
>> +
>> +    pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +    if (!pdata)
>> +        return NULL;
>> +
>> +    pdata->ext_dc_dc_compenstation_resistor =
>> +           of_property_read_bool(np,
>> "adi,ext_dc_dc_compenstation_resistor");
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
>> +        pdata->dc_dc_phase = tmp;
>> +    else
>> +        pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
>> +        pdata->dc_dc_freq = tmp;
>> +    else
>> +        pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
>> +        pdata->dc_dc_maxv = tmp;
>> +    else
>> +        pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
>> +
>> +    devnr = 0;
>> +    for_each_child_of_node(np, pp) {
>> +        if (devnr > AD5755_NUM_CHANNELS) {
>> +            dev_err(dev, "There is to many channels defined in DT\n");
>> +            goto error_out;
>> +        }
>> +
>> +        if (!of_property_read_u32(pp, "adi,mode", &tmp))
>> +            pdata->dac[devnr].mode = tmp;
>> +        else
>> +            pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
>> +
>> +        pdata->dac[devnr].ext_current_sense_resistor =
>> +               of_property_read_bool(pp, "adi,ext_current_sense_resistor");
>> +
>> +        pdata->dac[devnr].enable_voltage_overrange =
>> +            of_property_read_bool(pp, "adi,enable_voltage_overrange");
>> +        if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
>> +            pdata->dac[devnr].slew.enable = tmparray[0];
>> +            pdata->dac[devnr].slew.rate = tmparray[1];
>> +            pdata->dac[devnr].slew.step_size = tmparray[2];
>> +        } else {
>> +            pdata->dac[devnr].slew.enable = false;
>> +            pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
>> +            pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
>> +        }
>> +
>> +        devnr++;
>> +    }
>> +
>> +    return pdata;

[...]

Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Lars-Peter Clausen]

On 02/09/2016 09:22 AM, Sean Nyekjær wrote:
> 
> On 2016-02-03 15:31, Sean Nyekjaer wrote:
>> Devicetree can provide platform data
>>
>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> Lars do you have time to look at this before i'm fixing the comments from
> Jonathan and Rob :-)

The obvious thing is to use '-' instead of '_' since that is the DT convention.

Other than that this non-trivial. This is the first binding for such a
device and it is always hard to come up with a good ABI without knowing what
other similar devices might need to be supported by the same bindings. And
it is also not always clear what is hardware description and what is driver
policy.

The external resistor configuration should be clear, that's a description of
the hardware whether the resistor exists or not.

For the others it's not so clear. E.g. the DC-to-DC converter configuration,
this is most certainly not a description of the hardware. But we could get
away with it by saying this is a system level design decision and is made by
the system designer based on the operating parameters of the hardware and
hence is policy that belongs in the DT. It might make sense to follow what
the regulator bindings do here, even though the regulator is not exposed
externally.

The configuration of the output pin (voltage level, current level, etc) is
certainly influenced by the restrictions of the hardware itself. You don't
want to allow a configuration that can damage the hardware. But on the other
hand it is still a valid usecase for some applications to switch between two
setups at runtime, which is something you said you need to be able to do.
Here we might want to use something similar to pinctrl where the devicetree
can offer a selection of valid configurations and the driver can switch
between those configurations.

- Lars

[...]
>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
>> +{
>> +    struct device_node *np = dev->of_node;
>> +    struct device_node *pp;
>> +    struct ad5755_platform_data *pdata;
>> +    unsigned int tmp;
>> +    unsigned int tmparray[3];
>> +    int devnr;
>> +
>> +    pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +    if (!pdata)
>> +        return NULL;
>> +
>> +    pdata->ext_dc_dc_compenstation_resistor =
>> +           of_property_read_bool(np,
>> "adi,ext_dc_dc_compenstation_resistor");
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
>> +        pdata->dc_dc_phase = tmp;
>> +    else
>> +        pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
>> +        pdata->dc_dc_freq = tmp;
>> +    else
>> +        pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
>> +
>> +    if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
>> +        pdata->dc_dc_maxv = tmp;
>> +    else
>> +        pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
>> +
>> +    devnr = 0;
>> +    for_each_child_of_node(np, pp) {
>> +        if (devnr > AD5755_NUM_CHANNELS) {
>> +            dev_err(dev, "There is to many channels defined in DT\n");
>> +            goto error_out;
>> +        }
>> +
>> +        if (!of_property_read_u32(pp, "adi,mode", &tmp))
>> +            pdata->dac[devnr].mode = tmp;
>> +        else
>> +            pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
>> +
>> +        pdata->dac[devnr].ext_current_sense_resistor =
>> +               of_property_read_bool(pp, "adi,ext_current_sense_resistor");
>> +
>> +        pdata->dac[devnr].enable_voltage_overrange =
>> +            of_property_read_bool(pp, "adi,enable_voltage_overrange");
>> +        if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
>> +            pdata->dac[devnr].slew.enable = tmparray[0];
>> +            pdata->dac[devnr].slew.rate = tmparray[1];
>> +            pdata->dac[devnr].slew.step_size = tmparray[2];
>> +        } else {
>> +            pdata->dac[devnr].slew.enable = false;
>> +            pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
>> +            pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
>> +        }
>> +
>> +        devnr++;
>> +    }
>> +
>> +    return pdata;

[...]

Re: [PATCH v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjær]

On 2016-02-09 21:30, Lars-Peter Clausen wrote:
> On 02/09/2016 09:22 AM, Sean Nyekjær wrote:
>> On 2016-02-03 15:31, Sean Nyekjaer wrote:
>>> Devicetree can provide platform data
>>>
>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer-rjjw5hvvQKZaa/9Udqfwiw@public.gmane.org>
>> Lars do you have time to look at this before i'm fixing the comments from
>> Jonathan and Rob :-)
> The obvious thing is to use '-' instead of '_' since that is the DT convention.
I'm removing the defines in from the DT doc.
>
> Other than that this non-trivial. This is the first binding for such a
> device and it is always hard to come up with a good ABI without knowing what
> other similar devices might need to be supported by the same bindings. And
> it is also not always clear what is hardware description and what is driver
> policy.
I was hoping we could cooperate on enabling this feature. It's not the
majority of linux devices that are using board files any more.
>
> The external resistor configuration should be clear, that's a description of
> the hardware whether the resistor exists or not.
>
> For the others it's not so clear. E.g. the DC-to-DC converter configuration,
> this is most certainly not a description of the hardware. But we could get
> away with it by saying this is a system level design decision and is made by
> the system designer based on the operating parameters of the hardware and
> hence is policy that belongs in the DT. It might make sense to follow what
> the regulator bindings do here, even though the regulator is not exposed
> externally.
>
> The configuration of the output pin (voltage level, current level, etc) is
> certainly influenced by the restrictions of the hardware itself. You don't
> want to allow a configuration that can damage the hardware. But on the other
> hand it is still a valid usecase for some applications to switch between two
> setups at runtime, which is something you said you need to be able to do.
I can't why enabling configuration of the DAC from the DT is posing a 
greater
risk of damaging hardware than the previous way of editing the board file.
> Here we might want to use something similar to pinctrl where the devicetree
> can offer a selection of valid configurations and the driver can switch
> between those configurations.
>
> - Lars
>
> [...]
I'll submit an PATCH v3 where i have removed the defines from the DT doc 
and introduce checks for the values passed from DT.

/Sean
>>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
>>> +{
>>> +    struct device_node *np = dev->of_node;
>>> +    struct device_node *pp;
>>> +    struct ad5755_platform_data *pdata;
>>> +    unsigned int tmp;
>>> +    unsigned int tmparray[3];
>>> +    int devnr;
>>> +
>>> +    pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> +    if (!pdata)
>>> +        return NULL;
>>> +
>>> +    pdata->ext_dc_dc_compenstation_resistor =
>>> +           of_property_read_bool(np,
>>> "adi,ext_dc_dc_compenstation_resistor");
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
>>> +        pdata->dc_dc_phase = tmp;
>>> +    else
>>> +        pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
>>> +        pdata->dc_dc_freq = tmp;
>>> +    else
>>> +        pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
>>> +        pdata->dc_dc_maxv = tmp;
>>> +    else
>>> +        pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
>>> +
>>> +    devnr = 0;
>>> +    for_each_child_of_node(np, pp) {
>>> +        if (devnr > AD5755_NUM_CHANNELS) {
>>> +            dev_err(dev, "There is to many channels defined in DT\n");
>>> +            goto error_out;
>>> +        }
>>> +
>>> +        if (!of_property_read_u32(pp, "adi,mode", &tmp))
>>> +            pdata->dac[devnr].mode = tmp;
>>> +        else
>>> +            pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
>>> +
>>> +        pdata->dac[devnr].ext_current_sense_resistor =
>>> +               of_property_read_bool(pp, "adi,ext_current_sense_resistor");
>>> +
>>> +        pdata->dac[devnr].enable_voltage_overrange =
>>> +            of_property_read_bool(pp, "adi,enable_voltage_overrange");
>>> +        if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
>>> +            pdata->dac[devnr].slew.enable = tmparray[0];
>>> +            pdata->dac[devnr].slew.rate = tmparray[1];
>>> +            pdata->dac[devnr].slew.step_size = tmparray[2];
>>> +        } else {
>>> +            pdata->dac[devnr].slew.enable = false;
>>> +            pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
>>> +            pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
>>> +        }
>>> +
>>> +        devnr++;
>>> +    }
>>> +
>>> +    return pdata;
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" 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 v2 3/4] iio: ad5755: added full support for devicetree

[Sean Nyekjær]

On 2016-02-09 21:30, Lars-Peter Clausen wrote:
> On 02/09/2016 09:22 AM, Sean Nyekjær wrote:
>> On 2016-02-03 15:31, Sean Nyekjaer wrote:
>>> Devicetree can provide platform data
>>>
>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>> Lars do you have time to look at this before i'm fixing the comments from
>> Jonathan and Rob :-)
> The obvious thing is to use '-' instead of '_' since that is the DT convention.
I'm removing the defines in from the DT doc.
>
> Other than that this non-trivial. This is the first binding for such a
> device and it is always hard to come up with a good ABI without knowing what
> other similar devices might need to be supported by the same bindings. And
> it is also not always clear what is hardware description and what is driver
> policy.
I was hoping we could cooperate on enabling this feature. It's not the
majority of linux devices that are using board files any more.
>
> The external resistor configuration should be clear, that's a description of
> the hardware whether the resistor exists or not.
>
> For the others it's not so clear. E.g. the DC-to-DC converter configuration,
> this is most certainly not a description of the hardware. But we could get
> away with it by saying this is a system level design decision and is made by
> the system designer based on the operating parameters of the hardware and
> hence is policy that belongs in the DT. It might make sense to follow what
> the regulator bindings do here, even though the regulator is not exposed
> externally.
>
> The configuration of the output pin (voltage level, current level, etc) is
> certainly influenced by the restrictions of the hardware itself. You don't
> want to allow a configuration that can damage the hardware. But on the other
> hand it is still a valid usecase for some applications to switch between two
> setups at runtime, which is something you said you need to be able to do.
I can't why enabling configuration of the DAC from the DT is posing a 
greater
risk of damaging hardware than the previous way of editing the board file.
> Here we might want to use something similar to pinctrl where the devicetree
> can offer a selection of valid configurations and the driver can switch
> between those configurations.
>
> - Lars
>
> [...]
I'll submit an PATCH v3 where i have removed the defines from the DT doc 
and introduce checks for the values passed from DT.

/Sean
>>> +static struct ad5755_platform_data *ad5755_parse_dt(struct device *dev)
>>> +{
>>> +    struct device_node *np = dev->of_node;
>>> +    struct device_node *pp;
>>> +    struct ad5755_platform_data *pdata;
>>> +    unsigned int tmp;
>>> +    unsigned int tmparray[3];
>>> +    int devnr;
>>> +
>>> +    pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>>> +    if (!pdata)
>>> +        return NULL;
>>> +
>>> +    pdata->ext_dc_dc_compenstation_resistor =
>>> +           of_property_read_bool(np,
>>> "adi,ext_dc_dc_compenstation_resistor");
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_phase", &tmp))
>>> +        pdata->dc_dc_phase = tmp;
>>> +    else
>>> +        pdata->dc_dc_phase = AD5755_DC_DC_PHASE_ALL_SAME_EDGE;
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_freq", &tmp))
>>> +        pdata->dc_dc_freq = tmp;
>>> +    else
>>> +        pdata->dc_dc_freq = AD5755_DC_DC_FREQ_410kHZ;
>>> +
>>> +    if (!of_property_read_u32(np, "adi,dc_dc_maxv", &tmp))
>>> +        pdata->dc_dc_maxv = tmp;
>>> +    else
>>> +        pdata->dc_dc_maxv = AD5755_DC_DC_MAXV_23V;
>>> +
>>> +    devnr = 0;
>>> +    for_each_child_of_node(np, pp) {
>>> +        if (devnr > AD5755_NUM_CHANNELS) {
>>> +            dev_err(dev, "There is to many channels defined in DT\n");
>>> +            goto error_out;
>>> +        }
>>> +
>>> +        if (!of_property_read_u32(pp, "adi,mode", &tmp))
>>> +            pdata->dac[devnr].mode = tmp;
>>> +        else
>>> +            pdata->dac[devnr].mode = AD5755_MODE_CURRENT_4mA_20mA;
>>> +
>>> +        pdata->dac[devnr].ext_current_sense_resistor =
>>> +               of_property_read_bool(pp, "adi,ext_current_sense_resistor");
>>> +
>>> +        pdata->dac[devnr].enable_voltage_overrange =
>>> +            of_property_read_bool(pp, "adi,enable_voltage_overrange");
>>> +        if (!of_property_read_u32_array(pp, "adi,slew", tmparray, 3)) {
>>> +            pdata->dac[devnr].slew.enable = tmparray[0];
>>> +            pdata->dac[devnr].slew.rate = tmparray[1];
>>> +            pdata->dac[devnr].slew.step_size = tmparray[2];
>>> +        } else {
>>> +            pdata->dac[devnr].slew.enable = false;
>>> +            pdata->dac[devnr].slew.rate = AD5755_SLEW_RATE_64k;
>>> +            pdata->dac[devnr].slew.step_size = AD5755_SLEW_STEP_SIZE_1;
>>> +        }
>>> +
>>> +        devnr++;
>>> +    }
>>> +
>>> +    return pdata;
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html