[PATCH v4 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

[PATCH v4 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

[Cristian Pop]

This adds device tree bindings for the AD5766 DAC.

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
 Changelog v4:
	- Add range selection
	- Reset is GPIO_ACTIVE_LOW
	
 .../bindings/iio/dac/adi,ad5766.yaml          | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
new file mode 100644
index 000000000000..846b5ee50761
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5766 DAC device driver
+
+maintainers:
+  - Cristian Pop <cristian.pop@analog.com>
+
+description: |
+  Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
+  found here:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad5766
+      - adi,ad5767
+
+  output-range:
+    description: Select converter output range.
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 1000000
+
+  spi-cpol: true
+
+  reset-gpios:
+    description: GPIO spec for the RESET pin. If specified, it will be asserted
+      during driver probe. As the line is active low, it should be marked
+      GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+required:
+  - compatible
+  - output-range
+  - reg
+  - spi-max-frequency
+  - spi-cpol
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+          #address-cells = <1>;
+          #size-cells = <0>;
+          
+          ad5766@0 {
+              compatible = "adi,ad5766";
+              output-range = <(-5) 5>;
+              reg = <0>;
+              spi-cpol;
+              spi-max-frequency = <1000000>;
+              reset-gpios = <&gpio 22 0>;
+            };
+      };
-- 
2.17.1

[PATCH v4 2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

[Cristian Pop]

New interface is proposed for dither functionality. This future allows
composing  an external signals to the selected output channel.
The dither signal can be turned on/off, scaled, inverted, or it can be
selected from different sources.

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
Changelog v4:
	- Change to "in_voltageY_dither_enable"
	- Change scale to numbers + 100% for no scaling
	- Possible dither source values: 0 - N0, 1 - N1 
 .../ABI/testing/sysfs-bus-iio-dac-ad5766      | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
new file mode 100644
index 000000000000..72100edb88bb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
@@ -0,0 +1,36 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Dither enable. Write 1 to enable dither or 0 to disable it.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Inverts the dither applied to the selected DAC channel. Dither is not
+		inverted by default. Write "1" to invert dither.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Returns possible scalings available for the current channel:
+		"100 75 50 25" scaling.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Scales the dither before it is applied to the selected channel:
+		100 - No scaling
+		75 - 75% scaling
+		50 - 50% scaling
+		25 - 25% scaling
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
+KernelVersion:
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Selects dither source applied to the selected channel. Write "0" to
+		select N0 source, write "1" to select N1 source.
-- 
2.17.1

Re: [PATCH v4 2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

[Jonathan Cameron]

On Fri, 18 Dec 2020 19:12:30 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> New interface is proposed for dither functionality. This future allows
> composing  an external signals to the selected output channel.
> The dither signal can be turned on/off, scaled, inverted, or it can be
> selected from different sources.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
Coming together nicely though a few more thoughts came to mind when reading
this. Only significant one is whether we are better with decimal scaling
than percentage scaling for consistency with channel scales etc?

Jonathan

> ---
> Changelog v4:
> 	- Change to "in_voltageY_dither_enable"
> 	- Change scale to numbers + 100% for no scaling
> 	- Possible dither source values: 0 - N0, 1 - N1 
>  .../ABI/testing/sysfs-bus-iio-dac-ad5766      | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> new file mode 100644
> index 000000000000..72100edb88bb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> @@ -0,0 +1,36 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr

in_voltageY_dither_en would be more consistent with existing ABI naming.

> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Dither enable. Write 1 to enable dither or 0 to disable it.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Inverts the dither applied to the selected DAC channel. Dither is not
> +		inverted by default. Write "1" to invert dither.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Returns possible scalings available for the current channel:

Given this one is self defining and may well get generalized to other devices I'd
got with a description that doesn't provide values.

Returns possible scalings as percentages.

However, thinking more on this it's a bit inconsistent with other scale values we have
which are given in decimal.  Would "1 0.75 0.5 0.25" work as well here?

> +		"100 75 50 25" scaling.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
> +KernelVersion:
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Scales the dither before it is applied to the selected channel:
> +		100 - No scaling
> +		75 - 75% scaling
> +		50 - 50% scaling
> +		25 - 25% scaling

As above, I'm don't really see an advantage in providing the values.  Any userspace
ought to cope with any (finding out what is there via _available).
"Scales the dither before it is applied to the selected channel.  1 corresponds
to no scaling".

> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
> +KernelVersion:

Please fill in the expected kernel version.  I'll fix it if this happens to merge near
the change over in a cycle, but it's easier to fix something that is there
than to add it from scratch :)

> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Selects dither source applied to the selected channel. Write "0" to
> +		select N0 source, write "1" to select N1 source.

Re: [PATCH v4 2/3] Documentation/ABI/testing: Add documentation for AD5766 new ABI

[Jonathan Cameron]

On Tue, 29 Dec 2020 16:27:58 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 18 Dec 2020 19:12:30 +0200
> Cristian Pop <cristian.pop@analog.com> wrote:
> 
> > New interface is proposed for dither functionality. This future allows
> > composing  an external signals to the selected output channel.
> > The dither signal can be turned on/off, scaled, inverted, or it can be
> > selected from different sources.
> > 
> > Signed-off-by: Cristian Pop <cristian.pop@analog.com>  
> Coming together nicely though a few more thoughts came to mind when reading
> this. Only significant one is whether we are better with decimal scaling
> than percentage scaling for consistency with channel scales etc?
> 
> Jonathan
> 
> > ---
> > Changelog v4:
> > 	- Change to "in_voltageY_dither_enable"
> > 	- Change scale to numbers + 100% for no scaling
> > 	- Possible dither source values: 0 - N0, 1 - N1 
> >  .../ABI/testing/sysfs-bus-iio-dac-ad5766      | 36 +++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766 b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> > new file mode 100644
> > index 000000000000..72100edb88bb
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-ad5766
> > @@ -0,0 +1,36 @@
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_pwr  
> 
> in_voltageY_dither_en would be more consistent with existing ABI naming.
Seems to be different in the code (dither_enable I think) so please sanity
check this file carefully against the driver.

Thanks,

Jonathan

> 
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Dither enable. Write 1 to enable dither or 0 to disable it.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_invert
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Inverts the dither applied to the selected DAC channel. Dither is not
> > +		inverted by default. Write "1" to invert dither.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale_available
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Returns possible scalings available for the current channel:  
> 
> Given this one is self defining and may well get generalized to other devices I'd
> got with a description that doesn't provide values.
> 
> Returns possible scalings as percentages.
> 
> However, thinking more on this it's a bit inconsistent with other scale values we have
> which are given in decimal.  Would "1 0.75 0.5 0.25" work as well here?
> 
> > +		"100 75 50 25" scaling.
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_scale
> > +KernelVersion:
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Scales the dither before it is applied to the selected channel:
> > +		100 - No scaling
> > +		75 - 75% scaling
> > +		50 - 50% scaling
> > +		25 - 25% scaling  
> 
> As above, I'm don't really see an advantage in providing the values.  Any userspace
> ought to cope with any (finding out what is there via _available).
> "Scales the dither before it is applied to the selected channel.  1 corresponds
> to no scaling".
> 
> > +
> > +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_dither_source
> > +KernelVersion:  
> 
> Please fill in the expected kernel version.  I'll fix it if this happens to merge near
> the change over in a cycle, but it's easier to fix something that is there
> than to add it from scratch :)
> 
> > +Contact:	linux-iio@vger.kernel.org
> > +Description:
> > +		Selects dither source applied to the selected channel. Write "0" to
> > +		select N0 source, write "1" to select N1 source.  
> 

[PATCH v4 3/3] iio: dac: ad5766: add driver support for AD5766

[Cristian Pop]

The AD5766/AD5767 are 16-channel, 16-bit/12-bit, voltage output dense DACs
Digital-to-Analog converters.

This change adds support for these DACs.

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
---
Changelog v4:
	- Select output range
	- Set possible dither source to: 0 - N0 and 1 for N1.
	- Better macros usage
	- Invert reset logic
	- Use GENMASK
	- Add/remove some commas
 drivers/iio/dac/Kconfig  |  10 +
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/ad5766.c | 668 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 679 insertions(+)
 create mode 100644 drivers/iio/dac/ad5766.c

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 6f6074a5d3db..cea07b4cced1 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -189,6 +189,16 @@ config AD5764
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5764.
 
+config AD5766
+	tristate "Analog Devices AD5766/AD5767 DAC driver"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for Analog Devices AD5766, AD5767
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5766.
+
 config AD5770R
 	tristate "Analog Devices AD5770R IDAC driver"
 	depends on SPI_MASTER
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 2fc481167724..33e16f14902a 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_AD5755) += ad5755.o
 obj-$(CONFIG_AD5755) += ad5758.o
 obj-$(CONFIG_AD5761) += ad5761.o
 obj-$(CONFIG_AD5764) += ad5764.o
+obj-$(CONFIG_AD5766) += ad5766.o
 obj-$(CONFIG_AD5770R) += ad5770r.o
 obj-$(CONFIG_AD5791) += ad5791.o
 obj-$(CONFIG_AD5686) += ad5686.o
diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
new file mode 100644
index 000000000000..b7405185f3d8
--- /dev/null
+++ b/drivers/iio/dac/ad5766.c
@@ -0,0 +1,668 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices AD5766, AD5767
+ * Digital to Analog Converters driver
+ * Copyright 2019-2020 Analog Devices Inc.
+ */
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned/be_byteshift.h>
+
+#define AD5766_UPPER_WORD_SPI_MASK		GENMASK(31, 16)
+#define AD5766_LOWER_WORD_SPI_MASK		GENMASK(15, 0)
+#define AD5766_DITHER_SOURCE_MASK(ch)		GENMASK(((2 * ch) + 1), (2 * ch))
+#define AD5766_DITHER_SOURCE(ch, source)	BIT((ch * 2) + source)
+#define AD5766_DITHER_SCALE_MASK(x)		AD5766_DITHER_SOURCE_MASK(x)
+#define AD5766_DITHER_SCALE(ch, scale)		(scale << (ch * 2))
+#define AD5766_DITHER_ENABLE_MASK(ch)		BIT(ch)
+#define AD5766_DITHER_ENABLE(ch, state)		((!state) << ch)
+#define AD5766_DITHER_INVERT_MASK(ch)		BIT(ch)
+#define AD5766_DITHER_INVERT(ch, state)		(state << ch)
+
+#define AD5766_CMD_NOP_MUX_OUT			0x00
+#define AD5766_CMD_SDO_CNTRL			0x01
+#define AD5766_CMD_WR_IN_REG(x)			(0x10 | ((x) & GENMASK(3, 0)))
+#define AD5766_CMD_WR_DAC_REG(x)		(0x20 | ((x) & GENMASK(3, 0)))
+#define AD5766_CMD_SW_LDAC			0x30
+#define AD5766_CMD_SPAN_REG			0x40
+#define AD5766_CMD_WR_PWR_DITHER		0x51
+#define AD5766_CMD_WR_DAC_REG_ALL		0x60
+#define AD5766_CMD_SW_FULL_RESET		0x70
+#define AD5766_CMD_READBACK_REG(x)		(0x80 | ((x) & GENMASK(3, 0)))
+#define AD5766_CMD_DITHER_SIG_1			0x90
+#define AD5766_CMD_DITHER_SIG_2			0xA0
+#define AD5766_CMD_INV_DITHER			0xB0
+#define AD5766_CMD_DITHER_SCALE_1		0xC0
+#define AD5766_CMD_DITHER_SCALE_2		0xD0
+
+#define AD5766_FULL_RESET_CODE			0x1234
+
+enum ad5766_type {
+	ID_AD5766,
+	ID_AD5767,
+};
+
+enum ad5766_voltage_range {
+	AD5766_VOLTAGE_RANGE_M20V_0V,
+	AD5766_VOLTAGE_RANGE_M16V_to_0V,
+	AD5766_VOLTAGE_RANGE_M10V_to_0V,
+	AD5766_VOLTAGE_RANGE_M12V_to_14V,
+	AD5766_VOLTAGE_RANGE_M16V_to_10V,
+	AD5766_VOLTAGE_RANGE_M10V_to_6V,
+	AD5766_VOLTAGE_RANGE_M5V_to_5V,
+	AD5766_VOLTAGE_RANGE_M10V_to_10V,
+};
+
+/**
+ * struct ad5766_chip_info - chip specific information
+ * @num_channels:	number of channels
+ * @channel:	        channel specification
+ */
+struct ad5766_chip_info {
+	unsigned int			num_channels;
+	const struct iio_chan_spec	*channels;
+};
+
+enum {
+	AD5766_DITHER_ENABLE,
+	AD5766_DITHER_INVERT,
+	AD5766_DITHER_SOURCE,
+};
+
+/*
+ * Dither signal can also be scaled.
+ * Available dither scale strings corresponding to "dither_scale" field in
+ * "struct ad5766_state".
+ */
+static const char * const ad5766_dither_scales[] = {
+	"100",
+	"75",
+	"50",
+	"25",
+};
+
+/**
+ * struct ad5766_state - driver instance specific data
+ * @spi:		SPI device
+ * @lock:		Lock used to restrict concurent access to SPI device
+ * @chip_info:		Chip model specific constants
+ * @gpio_reset:		Reset GPIO, used to reset the device
+ * @crt_range:		Current selected output range
+ * @dither_enable:	Power enable bit for each channel dither block (for
+ *			example, D15 = DAC 15,D8 = DAC 8, and D0 = DAC 0)
+ *			0 - Normal operation, 1 - Power down
+ * @dither_invert:	Inverts the dither signal applied to the selected DAC
+ *			outputs
+ * @dither_source:	Selects between 2 possible sources:
+ *			1: N0, 2: N1
+ *			Two bits are used for each channel
+ * @dither_scale:	Two bits are used for each of the 16 channels:
+ *			0: 100% SCALING, 1: 75% SCALING, 2: 50% SCALING,
+ *			3: 25% SCALING.
+ * @data:		SPI transfer buffers
+ */
+struct ad5766_state {
+	struct spi_device		*spi;
+	struct mutex			lock;
+	const struct ad5766_chip_info	*chip_info;
+	struct gpio_desc		*gpio_reset;
+	enum ad5766_voltage_range	crt_range;
+	u16		dither_enable;
+	u16		dither_invert;
+	u32		dither_source;
+	u32		dither_scale;
+	union {
+		u32	d32;
+		u16	w16[2];
+		u8	b8[4];
+	} data[3] ____cacheline_aligned;
+};
+
+struct ad5766_span_tbl {
+	int		min;
+	int		max;
+};
+
+static const struct ad5766_span_tbl ad5766_span_tbl[] = {
+	[AD5766_VOLTAGE_RANGE_M20V_0V] = {
+		.min = -20,
+		.max = 0,
+	},
+	[AD5766_VOLTAGE_RANGE_M16V_to_0V] = {
+		.min = -16,
+		.max = 0,
+	},
+	[AD5766_VOLTAGE_RANGE_M10V_to_0V] = {
+		.min = -10,
+		.max = 0,
+	},
+	[AD5766_VOLTAGE_RANGE_M12V_to_14V] = {
+		.min = -12,
+		.max = 14,
+	},
+	[AD5766_VOLTAGE_RANGE_M16V_to_10V] = {
+		.min = -16,
+		.max = 10,
+	},
+	[AD5766_VOLTAGE_RANGE_M10V_to_6V] = {
+		.min = -10,
+		.max = 6,
+	},
+	[AD5766_VOLTAGE_RANGE_M5V_to_5V] = {
+		.min = -5,
+		.max = 5,
+	},
+	[AD5766_VOLTAGE_RANGE_M10V_to_10V] = {
+		.min = -10,
+		.max = 10,
+	},
+};
+
+static int __ad5766_spi_read(struct ad5766_state *st, u8 dac, int *val)
+{
+	int ret;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &st->data[0].d32,
+			.bits_per_word = 8,
+			.len = 3,
+			.cs_change = 1,
+		}, {
+			.tx_buf = &st->data[1].d32,
+			.rx_buf = &st->data[2].d32,
+			.bits_per_word = 8,
+			.len = 3,
+		},
+	};
+
+	st->data[0].d32 = AD5766_CMD_READBACK_REG(dac);
+	st->data[1].d32 = AD5766_CMD_NOP_MUX_OUT;
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret)
+		return ret;
+
+	*val = st->data[2].w16[1];
+
+	return ret;
+}
+
+static int __ad5766_spi_write(struct ad5766_state *st, u8 command, u16 data)
+{
+	st->data[0].b8[0] = command;
+	put_unaligned_be16(data, &st->data[0].b8[1]);
+
+	return spi_write(st->spi, &st->data[0].b8[0], 3);
+}
+
+static int ad5766_read(struct iio_dev *indio_dev, u8 dac, int *val)
+{
+	struct ad5766_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __ad5766_spi_read(st, dac, val);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int ad5766_write(struct iio_dev *indio_dev, u8 dac, u16 data)
+{
+	struct ad5766_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&st->lock);
+	ret = __ad5766_spi_write(st, AD5766_CMD_WR_DAC_REG(dac), data);
+	mutex_unlock(&st->lock);
+
+	return ret;
+}
+
+static int ad5766_reset(struct ad5766_state *st)
+{
+	int ret;
+
+	if (st->gpio_reset) {
+		gpiod_set_value_cansleep(st->gpio_reset, 1);
+		ndelay(100); /* t_reset >= 100ns */
+		gpiod_set_value_cansleep(st->gpio_reset, 0);
+	} else {
+		ret = __ad5766_spi_write(st, AD5766_CMD_SW_FULL_RESET,
+					AD5766_FULL_RESET_CODE);
+		if (ret < 0)
+			return ret;
+	}
+
+	/*
+	 * Minimum time between a reset and the subsequent successful write is
+	 * typically 25 ns
+	 */
+	ndelay(25);
+
+	return 0;
+}
+
+static int ad5766_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long m)
+{
+	struct ad5766_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ad5766_read(indio_dev, chan->address, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = ad5766_span_tbl[st->crt_range].min;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = ad5766_span_tbl[st->crt_range].max -
+		       ad5766_span_tbl[st->crt_range].min;
+		*val2 = st->chip_info->channels[0].scan_type.realbits;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5766_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val,
+			    int val2,
+			    long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+	{
+		const int max_val = GENMASK(chan->scan_type.realbits - 1, 0);
+
+		if (val > max_val || val < 0)
+			return -EINVAL;
+		val <<= chan->scan_type.shift;
+		return ad5766_write(indio_dev, chan->address, val);
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ad5766_info = {
+	.read_raw = ad5766_read_raw,
+	.write_raw = ad5766_write_raw,
+};
+
+static int ad5766_get_dither_source(struct iio_dev *dev,
+				    const struct iio_chan_spec *chan)
+{
+	struct ad5766_state *st = iio_priv(dev);
+	u32 source;
+
+	source = st->dither_source & AD5766_DITHER_SOURCE_MASK(chan->channel);
+	source = source >> (chan->channel * 2);
+	source -= 1;
+
+	return source;
+}
+
+static int ad5766_set_dither_source(struct iio_dev *dev,
+			  const struct iio_chan_spec *chan,
+			  unsigned int source)
+{
+	struct ad5766_state *st = iio_priv(dev);
+	uint16_t val;
+	int ret;
+
+	st->dither_source &= ~AD5766_DITHER_SOURCE_MASK(chan->channel);
+	st->dither_source |= AD5766_DITHER_SOURCE(chan->channel, source);
+
+	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_source);
+	ret = ad5766_write(dev, AD5766_CMD_DITHER_SIG_1, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_source);
+
+	return ad5766_write(dev, AD5766_CMD_DITHER_SIG_2, val);
+}
+
+static int ad5766_get_dither_scale(struct iio_dev *dev,
+				   const struct iio_chan_spec *chan)
+{
+	struct ad5766_state *st = iio_priv(dev);
+	u32 scale;
+
+	scale = st->dither_scale & AD5766_DITHER_SCALE_MASK(chan->channel);
+
+	return (scale >> (chan->channel * 2));
+}
+
+static int ad5766_set_dither_scale(struct iio_dev *dev,
+			  const struct iio_chan_spec *chan,
+			  unsigned int scale)
+{
+	int ret;
+	struct ad5766_state *st = iio_priv(dev);
+	uint16_t val;
+
+	st->dither_scale &= ~AD5766_DITHER_SCALE_MASK(chan->channel);
+	st->dither_scale |= AD5766_DITHER_SCALE(chan->channel, scale);
+
+	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_scale);
+	ret = ad5766_write(dev, AD5766_CMD_DITHER_SCALE_1, val);
+	if (ret)
+		return ret;
+	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_scale);
+
+	return ad5766_write(dev, AD5766_CMD_DITHER_SCALE_2, val);
+}
+
+static const struct iio_enum ad5766_dither_scale_enum = {
+	.items = ad5766_dither_scales,
+	.num_items = ARRAY_SIZE(ad5766_dither_scales),
+	.set = ad5766_set_dither_scale,
+	.get = ad5766_get_dither_scale,
+};
+
+static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
+			       uintptr_t private,
+			       const struct iio_chan_spec *chan,
+			       char *buf)
+{
+	struct ad5766_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (private) {
+	case AD5766_DITHER_ENABLE:
+		return sprintf(buf, "%u\n",
+			       !(st->dither_enable & BIT(chan->channel)));
+		break;
+	case AD5766_DITHER_INVERT:
+		return sprintf(buf, "%u\n",
+			       !!(st->dither_invert & BIT(chan->channel)));
+		break;
+	case AD5766_DITHER_SOURCE:
+		return sprintf(buf, "%u\n",
+			       ad5766_get_dither_source(indio_dev, chan));
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
+				 uintptr_t private,
+				 const struct iio_chan_spec *chan,
+				 const char *buf, size_t len)
+{
+	struct ad5766_state *st = iio_priv(indio_dev);
+	bool readin;
+	int ret;
+
+	ret = kstrtobool(buf, &readin);
+	if (ret)
+		return ret;
+
+	switch (private) {
+	case AD5766_DITHER_ENABLE:
+		st->dither_enable &= ~AD5766_DITHER_ENABLE_MASK(chan->channel);
+		st->dither_enable |= AD5766_DITHER_ENABLE(chan->channel,
+							  readin);
+		ret = ad5766_write(indio_dev, AD5766_CMD_WR_PWR_DITHER,
+				   st->dither_enable);
+		break;
+	case AD5766_DITHER_INVERT:
+		st->dither_invert &= ~AD5766_DITHER_INVERT_MASK(chan->channel);
+		st->dither_invert |= AD5766_DITHER_INVERT(chan->channel,
+							  readin);
+		ret = ad5766_write(indio_dev, AD5766_CMD_INV_DITHER,
+				   st->dither_invert);
+		break;
+	case AD5766_DITHER_SOURCE:
+		ret = ad5766_set_dither_source(indio_dev, chan, readin);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret ? ret : len;
+}
+
+#define _AD5766_CHAN_EXT_INFO(_name, _what, _shared) { \
+	.name = _name, \
+	.read = ad5766_read_ext, \
+	.write = ad5766_write_ext, \
+	.private = _what, \
+	.shared = _shared, \
+}
+
+#define IIO_ENUM_AVAILABLE_SHARED(_name, _shared, _e) \
+{ \
+	.name = (_name "_available"), \
+	.shared = _shared, \
+	.read = iio_enum_available_read, \
+	.private = (uintptr_t)(_e), \
+}
+
+static const struct iio_chan_spec_ext_info ad5766_ext_info[] = {
+
+	_AD5766_CHAN_EXT_INFO("dither_enable", AD5766_DITHER_ENABLE,
+			      IIO_SEPARATE),
+	_AD5766_CHAN_EXT_INFO("dither_invert", AD5766_DITHER_INVERT,
+			      IIO_SEPARATE),
+	_AD5766_CHAN_EXT_INFO("dither_source", AD5766_DITHER_SOURCE,
+			      IIO_SEPARATE),
+	IIO_ENUM("dither_scale", IIO_SEPARATE, &ad5766_dither_scale_enum),
+	IIO_ENUM_AVAILABLE_SHARED("dither_scale",
+				  IIO_SEPARATE,
+				  &ad5766_dither_scale_enum),
+	{}
+};
+
+#define AD576x_CHANNEL(_chan, _bits) {					\
+	.type = IIO_VOLTAGE,						\
+	.indexed = 1,							\
+	.output = 1,							\
+	.channel = (_chan),						\
+	.address = (_chan),						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |		\
+		BIT(IIO_CHAN_INFO_SCALE),				\
+	.scan_type = {							\
+		.sign = 'u',						\
+		.realbits = (_bits),					\
+		.storagebits = 16,					\
+		.shift = 16 - (_bits),					\
+	},								\
+	.ext_info = ad5766_ext_info,					\
+}
+
+#define DECLARE_AD576x_CHANNELS(_name, _bits)			\
+const struct iio_chan_spec _name[] = {				\
+	AD576x_CHANNEL(0, (_bits)),				\
+	AD576x_CHANNEL(1, (_bits)),				\
+	AD576x_CHANNEL(2, (_bits)),				\
+	AD576x_CHANNEL(3, (_bits)),				\
+	AD576x_CHANNEL(4, (_bits)),				\
+	AD576x_CHANNEL(5, (_bits)),				\
+	AD576x_CHANNEL(6, (_bits)),				\
+	AD576x_CHANNEL(7, (_bits)),				\
+	AD576x_CHANNEL(8, (_bits)),				\
+	AD576x_CHANNEL(9, (_bits)),				\
+	AD576x_CHANNEL(10, (_bits)),				\
+	AD576x_CHANNEL(11, (_bits)),				\
+	AD576x_CHANNEL(12, (_bits)),				\
+	AD576x_CHANNEL(13, (_bits)),				\
+	AD576x_CHANNEL(14, (_bits)),				\
+	AD576x_CHANNEL(15, (_bits)),				\
+}
+
+static DECLARE_AD576x_CHANNELS(ad5766_channels, 16);
+static DECLARE_AD576x_CHANNELS(ad5767_channels, 12);
+
+static const struct ad5766_chip_info ad5766_chip_infos[] = {
+	[ID_AD5766] = {
+		.num_channels = ARRAY_SIZE(ad5766_channels),
+		.channels = ad5766_channels,
+	},
+	[ID_AD5767] = {
+		.num_channels = ARRAY_SIZE(ad5767_channels),
+		.channels = ad5767_channels,
+	},
+};
+
+static int ad5766_get_output_range(struct ad5766_state *st)
+{
+	int i, ret, min, max, tmp[2];
+
+	ret = device_property_read_u32_array(&st->spi->dev,
+					     "output-range",
+					     tmp, 2);
+	if (ret)
+		return ret;
+
+	min = tmp[0];
+	max = tmp[1];
+	for (i = 0; i < ARRAY_SIZE(ad5766_span_tbl); i++) {
+		if (ad5766_span_tbl[i].min != min ||
+		    ad5766_span_tbl[i].max != max)
+			continue;
+
+		st->crt_range = i;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int ad5766_default_setup(struct ad5766_state *st)
+{
+	uint16_t val;
+	int ret, i;
+
+	/* Always issue a reset before writing to the span register. */
+	ret = ad5766_reset(st);
+	if (ret)
+		return ret;
+
+	ret = ad5766_get_output_range(st);
+	if (ret)
+		return ret;
+
+	/* Dither power down */
+	st->dither_enable = GENMASK(15, 0);
+	ret = __ad5766_spi_write(st, AD5766_CMD_WR_PWR_DITHER,
+			     st->dither_enable);
+	if (ret)
+		return ret;
+
+	st->dither_source = 0;
+	for (i = 0; i < ARRAY_SIZE(ad5766_channels); i++)
+		st->dither_source |= AD5766_DITHER_SOURCE(i, 0);
+	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_source);
+	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_1, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_source);
+	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_2, val);
+	if (ret)
+		return ret;
+
+	st->dither_scale = 0;
+	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_scale);
+	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_1, val);
+	if (ret)
+		return ret;
+
+	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_scale);
+	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_2, val);
+	if (ret)
+		return ret;
+
+	st->dither_invert = 0;
+	ret = __ad5766_spi_write(st, AD5766_CMD_INV_DITHER, st->dither_invert);
+	if (ret)
+		return ret;
+
+	return  __ad5766_spi_write(st, AD5766_CMD_SPAN_REG, st->crt_range);
+}
+
+static int ad5766_probe(struct spi_device *spi)
+{
+	enum ad5766_type type;
+	struct iio_dev *indio_dev;
+	struct ad5766_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+
+	st->spi = spi;
+	type = spi_get_device_id(spi)->driver_data;
+	st->chip_info = &ad5766_chip_infos[type];
+
+	indio_dev->channels = st->chip_info->channels;
+	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->info = &ad5766_info;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
+						GPIOD_OUT_LOW);
+
+	ret = ad5766_default_setup(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad5766_dt_match[] = {
+	{ .compatible = "adi,ad5766" },
+	{ .compatible = "adi,ad5767" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ad5766_dt_match);
+
+static const struct spi_device_id ad5766_spi_ids[] = {
+	{ "ad5766", ID_AD5766 },
+	{ "ad5767", ID_AD5767 },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, ad5766_spi_ids);
+
+static struct spi_driver ad5766_driver = {
+	.driver = {
+		.name = "ad5766",
+		.of_match_table = ad5766_dt_match,
+	},
+	.probe = ad5766_probe,
+	.id_table = ad5766_spi_ids,
+};
+module_spi_driver(ad5766_driver);
+
+MODULE_AUTHOR("Denis-Gabriel Gheorghescu <denis.gheorghescu@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5766/AD5767 DACs");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v4 3/3] iio: dac: ad5766: add driver support for AD5766

[Jonathan Cameron]

On Fri, 18 Dec 2020 19:12:31 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> The AD5766/AD5767 are 16-channel, 16-bit/12-bit, voltage output dense DACs
> Digital-to-Analog converters.
> 
> This change adds support for these DACs.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
Hi Cristian,

The addressing in this device 'interesting'. I'm not sure it can be handled
much better than you have done though.

Otherwise, a few minor bits and pieces inline.

Thanks,

Jonathan


> ---
> Changelog v4:
> 	- Select output range
> 	- Set possible dither source to: 0 - N0 and 1 for N1.
> 	- Better macros usage
> 	- Invert reset logic
> 	- Use GENMASK
> 	- Add/remove some commas
>  drivers/iio/dac/Kconfig  |  10 +
>  drivers/iio/dac/Makefile |   1 +
>  drivers/iio/dac/ad5766.c | 668 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 679 insertions(+)
>  create mode 100644 drivers/iio/dac/ad5766.c
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 6f6074a5d3db..cea07b4cced1 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -189,6 +189,16 @@ config AD5764
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad5764.
>  
> +config AD5766
> +	tristate "Analog Devices AD5766/AD5767 DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for Analog Devices AD5766, AD5767
> +	  Digital to Analog Converter.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5766.
> +
>  config AD5770R
>  	tristate "Analog Devices AD5770R IDAC driver"
>  	depends on SPI_MASTER
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 2fc481167724..33e16f14902a 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_AD5755) += ad5755.o
>  obj-$(CONFIG_AD5755) += ad5758.o
>  obj-$(CONFIG_AD5761) += ad5761.o
>  obj-$(CONFIG_AD5764) += ad5764.o
> +obj-$(CONFIG_AD5766) += ad5766.o
>  obj-$(CONFIG_AD5770R) += ad5770r.o
>  obj-$(CONFIG_AD5791) += ad5791.o
>  obj-$(CONFIG_AD5686) += ad5686.o
> diff --git a/drivers/iio/dac/ad5766.c b/drivers/iio/dac/ad5766.c
> new file mode 100644
> index 000000000000..b7405185f3d8
> --- /dev/null
> +++ b/drivers/iio/dac/ad5766.c
> @@ -0,0 +1,668 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices AD5766, AD5767
> + * Digital to Analog Converters driver
> + * Copyright 2019-2020 Analog Devices Inc.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned/be_byteshift.h>
> +
> +#define AD5766_UPPER_WORD_SPI_MASK		GENMASK(31, 16)
> +#define AD5766_LOWER_WORD_SPI_MASK		GENMASK(15, 0)
> +#define AD5766_DITHER_SOURCE_MASK(ch)		GENMASK(((2 * ch) + 1), (2 * ch))
> +#define AD5766_DITHER_SOURCE(ch, source)	BIT((ch * 2) + source)
> +#define AD5766_DITHER_SCALE_MASK(x)		AD5766_DITHER_SOURCE_MASK(x)
> +#define AD5766_DITHER_SCALE(ch, scale)		(scale << (ch * 2))
> +#define AD5766_DITHER_ENABLE_MASK(ch)		BIT(ch)
> +#define AD5766_DITHER_ENABLE(ch, state)		((!state) << ch)
> +#define AD5766_DITHER_INVERT_MASK(ch)		BIT(ch)
> +#define AD5766_DITHER_INVERT(ch, state)		(state << ch)
> +
> +#define AD5766_CMD_NOP_MUX_OUT			0x00
> +#define AD5766_CMD_SDO_CNTRL			0x01
> +#define AD5766_CMD_WR_IN_REG(x)			(0x10 | ((x) & GENMASK(3, 0)))
> +#define AD5766_CMD_WR_DAC_REG(x)		(0x20 | ((x) & GENMASK(3, 0)))
> +#define AD5766_CMD_SW_LDAC			0x30
> +#define AD5766_CMD_SPAN_REG			0x40
> +#define AD5766_CMD_WR_PWR_DITHER		0x51
> +#define AD5766_CMD_WR_DAC_REG_ALL		0x60
> +#define AD5766_CMD_SW_FULL_RESET		0x70
> +#define AD5766_CMD_READBACK_REG(x)		(0x80 | ((x) & GENMASK(3, 0)))
> +#define AD5766_CMD_DITHER_SIG_1			0x90
> +#define AD5766_CMD_DITHER_SIG_2			0xA0
> +#define AD5766_CMD_INV_DITHER			0xB0
> +#define AD5766_CMD_DITHER_SCALE_1		0xC0
> +#define AD5766_CMD_DITHER_SCALE_2		0xD0

These addresses struck me as 'unusual' so I went and had a look at the datasheet.
I would separate the definition of C3-C0 and A3-A0 as then it will look a lot
more obvious as we will just have registers 0x0 to 0xD, some of which are banked
with the address bits acting as selector.

However. There are at least 2 special cases where the address bits aren't acting
to specify a banked per output channel register *sigh* so perhaps what you have
is the best we can do.

> +
> +#define AD5766_FULL_RESET_CODE			0x1234
> +
> +enum ad5766_type {
> +	ID_AD5766,
> +	ID_AD5767,
> +};
> +
> +enum ad5766_voltage_range {
> +	AD5766_VOLTAGE_RANGE_M20V_0V,
> +	AD5766_VOLTAGE_RANGE_M16V_to_0V,
> +	AD5766_VOLTAGE_RANGE_M10V_to_0V,
> +	AD5766_VOLTAGE_RANGE_M12V_to_14V,
> +	AD5766_VOLTAGE_RANGE_M16V_to_10V,
> +	AD5766_VOLTAGE_RANGE_M10V_to_6V,
> +	AD5766_VOLTAGE_RANGE_M5V_to_5V,
> +	AD5766_VOLTAGE_RANGE_M10V_to_10V,
> +};
> +
> +/**
> + * struct ad5766_chip_info - chip specific information
> + * @num_channels:	number of channels
> + * @channel:	        channel specification
> + */
> +struct ad5766_chip_info {
> +	unsigned int			num_channels;
> +	const struct iio_chan_spec	*channels;
> +};
> +
> +enum {
> +	AD5766_DITHER_ENABLE,
> +	AD5766_DITHER_INVERT,
> +	AD5766_DITHER_SOURCE,
> +};
> +
> +/*
> + * Dither signal can also be scaled.
> + * Available dither scale strings corresponding to "dither_scale" field in
> + * "struct ad5766_state".
> + */
> +static const char * const ad5766_dither_scales[] = {
> +	"100",
> +	"75",
> +	"50",
> +	"25",
> +};
> +
> +/**
> + * struct ad5766_state - driver instance specific data
> + * @spi:		SPI device
> + * @lock:		Lock used to restrict concurent access to SPI device
> + * @chip_info:		Chip model specific constants
> + * @gpio_reset:		Reset GPIO, used to reset the device
> + * @crt_range:		Current selected output range
> + * @dither_enable:	Power enable bit for each channel dither block (for
> + *			example, D15 = DAC 15,D8 = DAC 8, and D0 = DAC 0)
> + *			0 - Normal operation, 1 - Power down
> + * @dither_invert:	Inverts the dither signal applied to the selected DAC
> + *			outputs
> + * @dither_source:	Selects between 2 possible sources:
> + *			1: N0, 2: N1
> + *			Two bits are used for each channel
> + * @dither_scale:	Two bits are used for each of the 16 channels:
> + *			0: 100% SCALING, 1: 75% SCALING, 2: 50% SCALING,
> + *			3: 25% SCALING.
> + * @data:		SPI transfer buffers
> + */
> +struct ad5766_state {
> +	struct spi_device		*spi;
> +	struct mutex			lock;
> +	const struct ad5766_chip_info	*chip_info;
> +	struct gpio_desc		*gpio_reset;
> +	enum ad5766_voltage_range	crt_range;
> +	u16		dither_enable;
> +	u16		dither_invert;
> +	u32		dither_source;
> +	u32		dither_scale;
> +	union {
> +		u32	d32;
> +		u16	w16[2];
> +		u8	b8[4];
> +	} data[3] ____cacheline_aligned;
> +};
> +
> +struct ad5766_span_tbl {
> +	int		min;
> +	int		max;
> +};
> +
> +static const struct ad5766_span_tbl ad5766_span_tbl[] = {
> +	[AD5766_VOLTAGE_RANGE_M20V_0V] = {
Giving naming you 'could' just assume the min, max element was clear enough
and go with
        [AD5766_VOLTAGE_RANGE_M20V_0V] = {-20, 0}, etc
I don't think there would be any significant loss in readability.

> +		.min = -20,
> +		.max = 0,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M16V_to_0V] = {
> +		.min = -16,
> +		.max = 0,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M10V_to_0V] = {
> +		.min = -10,
> +		.max = 0,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M12V_to_14V] = {
> +		.min = -12,
> +		.max = 14,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M16V_to_10V] = {
> +		.min = -16,
> +		.max = 10,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M10V_to_6V] = {
> +		.min = -10,
> +		.max = 6,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M5V_to_5V] = {
> +		.min = -5,
> +		.max = 5,
> +	},
> +	[AD5766_VOLTAGE_RANGE_M10V_to_10V] = {
> +		.min = -10,
> +		.max = 10,
> +	},
> +};
> +
> +static int __ad5766_spi_read(struct ad5766_state *st, u8 dac, int *val)
> +{
> +	int ret;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = &st->data[0].d32,
> +			.bits_per_word = 8,
> +			.len = 3,
> +			.cs_change = 1,
> +		}, {
> +			.tx_buf = &st->data[1].d32,
> +			.rx_buf = &st->data[2].d32,
> +			.bits_per_word = 8,
> +			.len = 3,
> +		},
> +	};
> +
> +	st->data[0].d32 = AD5766_CMD_READBACK_REG(dac);
> +	st->data[1].d32 = AD5766_CMD_NOP_MUX_OUT;
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +	if (ret)
> +		return ret;
> +
> +	*val = st->data[2].w16[1];
> +
> +	return ret;
> +}
> +
> +static int __ad5766_spi_write(struct ad5766_state *st, u8 command, u16 data)
> +{
> +	st->data[0].b8[0] = command;
> +	put_unaligned_be16(data, &st->data[0].b8[1]);
> +
> +	return spi_write(st->spi, &st->data[0].b8[0], 3);
> +}
> +
> +static int ad5766_read(struct iio_dev *indio_dev, u8 dac, int *val)
> +{
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = __ad5766_spi_read(st, dac, val);
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
> +static int ad5766_write(struct iio_dev *indio_dev, u8 dac, u16 data)
> +{
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&st->lock);
> +	ret = __ad5766_spi_write(st, AD5766_CMD_WR_DAC_REG(dac), data);
> +	mutex_unlock(&st->lock);
> +
> +	return ret;
> +}
> +
> +static int ad5766_reset(struct ad5766_state *st)
> +{
> +	int ret;
> +
> +	if (st->gpio_reset) {
> +		gpiod_set_value_cansleep(st->gpio_reset, 1);
> +		ndelay(100); /* t_reset >= 100ns */
> +		gpiod_set_value_cansleep(st->gpio_reset, 0);
> +	} else {
> +		ret = __ad5766_spi_write(st, AD5766_CMD_SW_FULL_RESET,
> +					AD5766_FULL_RESET_CODE);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	/*
> +	 * Minimum time between a reset and the subsequent successful write is
> +	 * typically 25 ns
> +	 */
> +	ndelay(25);
> +
> +	return 0;
> +}
> +
> +static int ad5766_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val,
> +			   int *val2,
> +			   long m)
> +{
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (m) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = ad5766_read(indio_dev, chan->address, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = ad5766_span_tbl[st->crt_range].min;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = ad5766_span_tbl[st->crt_range].max -
> +		       ad5766_span_tbl[st->crt_range].min;
> +		*val2 = st->chip_info->channels[0].scan_type.realbits;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5766_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val,
> +			    int val2,
> +			    long info)
> +{
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +	{
> +		const int max_val = GENMASK(chan->scan_type.realbits - 1, 0);
> +
> +		if (val > max_val || val < 0)
> +			return -EINVAL;
> +		val <<= chan->scan_type.shift;
> +		return ad5766_write(indio_dev, chan->address, val);
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info ad5766_info = {
> +	.read_raw = ad5766_read_raw,
> +	.write_raw = ad5766_write_raw,
> +};
> +
> +static int ad5766_get_dither_source(struct iio_dev *dev,
> +				    const struct iio_chan_spec *chan)
> +{
> +	struct ad5766_state *st = iio_priv(dev);
> +	u32 source;
> +
> +	source = st->dither_source & AD5766_DITHER_SOURCE_MASK(chan->channel);
> +	source = source >> (chan->channel * 2);
> +	source -= 1;
> +
> +	return source;
> +}
> +
> +static int ad5766_set_dither_source(struct iio_dev *dev,
> +			  const struct iio_chan_spec *chan,
> +			  unsigned int source)
> +{
> +	struct ad5766_state *st = iio_priv(dev);
> +	uint16_t val;
> +	int ret;
> +
> +	st->dither_source &= ~AD5766_DITHER_SOURCE_MASK(chan->channel);
> +	st->dither_source |= AD5766_DITHER_SOURCE(chan->channel, source);
> +
> +	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_source);
> +	ret = ad5766_write(dev, AD5766_CMD_DITHER_SIG_1, val);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_source);
> +
> +	return ad5766_write(dev, AD5766_CMD_DITHER_SIG_2, val);
> +}
> +
> +static int ad5766_get_dither_scale(struct iio_dev *dev,
> +				   const struct iio_chan_spec *chan)
> +{
> +	struct ad5766_state *st = iio_priv(dev);
> +	u32 scale;
> +
> +	scale = st->dither_scale & AD5766_DITHER_SCALE_MASK(chan->channel);
> +
> +	return (scale >> (chan->channel * 2));
> +}
> +
> +static int ad5766_set_dither_scale(struct iio_dev *dev,
> +			  const struct iio_chan_spec *chan,
> +			  unsigned int scale)
> +{
> +	int ret;
> +	struct ad5766_state *st = iio_priv(dev);
> +	uint16_t val;
> +
> +	st->dither_scale &= ~AD5766_DITHER_SCALE_MASK(chan->channel);
> +	st->dither_scale |= AD5766_DITHER_SCALE(chan->channel, scale);
> +
> +	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_scale);
> +	ret = ad5766_write(dev, AD5766_CMD_DITHER_SCALE_1, val);
> +	if (ret)
> +		return ret;
> +	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_scale);
> +
> +	return ad5766_write(dev, AD5766_CMD_DITHER_SCALE_2, val);
> +}
> +
> +static const struct iio_enum ad5766_dither_scale_enum = {
> +	.items = ad5766_dither_scales,
> +	.num_items = ARRAY_SIZE(ad5766_dither_scales),
> +	.set = ad5766_set_dither_scale,
> +	.get = ad5766_get_dither_scale,
> +};
> +
> +static ssize_t ad5766_read_ext(struct iio_dev *indio_dev,
> +			       uintptr_t private,
> +			       const struct iio_chan_spec *chan,
> +			       char *buf)
> +{
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (private) {
> +	case AD5766_DITHER_ENABLE:
> +		return sprintf(buf, "%u\n",
> +			       !(st->dither_enable & BIT(chan->channel)));
> +		break;
> +	case AD5766_DITHER_INVERT:
> +		return sprintf(buf, "%u\n",
> +			       !!(st->dither_invert & BIT(chan->channel)));
> +		break;
> +	case AD5766_DITHER_SOURCE:
> +		return sprintf(buf, "%u\n",
> +			       ad5766_get_dither_source(indio_dev, chan));
> +	default:
> +		return -EINVAL;
> +	}
> +

Can't get here

> +	return ret;
> +}
> +
> +static ssize_t ad5766_write_ext(struct iio_dev *indio_dev,
> +				 uintptr_t private,
> +				 const struct iio_chan_spec *chan,
> +				 const char *buf, size_t len)
> +{
> +	struct ad5766_state *st = iio_priv(indio_dev);
> +	bool readin;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &readin);
> +	if (ret)
> +		return ret;
> +
> +	switch (private) {
> +	case AD5766_DITHER_ENABLE:
> +		st->dither_enable &= ~AD5766_DITHER_ENABLE_MASK(chan->channel);
> +		st->dither_enable |= AD5766_DITHER_ENABLE(chan->channel,
> +							  readin);
> +		ret = ad5766_write(indio_dev, AD5766_CMD_WR_PWR_DITHER,
> +				   st->dither_enable);
> +		break;
> +	case AD5766_DITHER_INVERT:
> +		st->dither_invert &= ~AD5766_DITHER_INVERT_MASK(chan->channel);
> +		st->dither_invert |= AD5766_DITHER_INVERT(chan->channel,
> +							  readin);
> +		ret = ad5766_write(indio_dev, AD5766_CMD_INV_DITHER,
> +				   st->dither_invert);
> +		break;
> +	case AD5766_DITHER_SOURCE:
> +		ret = ad5766_set_dither_source(indio_dev, chan, readin);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret ? ret : len;
> +}
> +
> +#define _AD5766_CHAN_EXT_INFO(_name, _what, _shared) { \
> +	.name = _name, \
> +	.read = ad5766_read_ext, \
> +	.write = ad5766_write_ext, \
> +	.private = _what, \
> +	.shared = _shared, \
> +}
> +
> +#define IIO_ENUM_AVAILABLE_SHARED(_name, _shared, _e) \
> +{ \
> +	.name = (_name "_available"), \
> +	.shared = _shared, \
> +	.read = iio_enum_available_read, \
> +	.private = (uintptr_t)(_e), \
> +}
> +
> +static const struct iio_chan_spec_ext_info ad5766_ext_info[] = {
> +
> +	_AD5766_CHAN_EXT_INFO("dither_enable", AD5766_DITHER_ENABLE,
> +			      IIO_SEPARATE),
> +	_AD5766_CHAN_EXT_INFO("dither_invert", AD5766_DITHER_INVERT,
> +			      IIO_SEPARATE),
> +	_AD5766_CHAN_EXT_INFO("dither_source", AD5766_DITHER_SOURCE,
> +			      IIO_SEPARATE),
> +	IIO_ENUM("dither_scale", IIO_SEPARATE, &ad5766_dither_scale_enum),
> +	IIO_ENUM_AVAILABLE_SHARED("dither_scale",
> +				  IIO_SEPARATE,
> +				  &ad5766_dither_scale_enum),
> +	{}
> +};
> +
> +#define AD576x_CHANNEL(_chan, _bits) {					\
> +	.type = IIO_VOLTAGE,						\
> +	.indexed = 1,							\
> +	.output = 1,							\
> +	.channel = (_chan),						\
> +	.address = (_chan),						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |		\
> +		BIT(IIO_CHAN_INFO_SCALE),				\
> +	.scan_type = {							\
> +		.sign = 'u',						\
> +		.realbits = (_bits),					\
> +		.storagebits = 16,					\
> +		.shift = 16 - (_bits),					\
> +	},								\
> +	.ext_info = ad5766_ext_info,					\
> +}
> +
> +#define DECLARE_AD576x_CHANNELS(_name, _bits)			\
> +const struct iio_chan_spec _name[] = {				\
> +	AD576x_CHANNEL(0, (_bits)),				\
> +	AD576x_CHANNEL(1, (_bits)),				\
> +	AD576x_CHANNEL(2, (_bits)),				\
> +	AD576x_CHANNEL(3, (_bits)),				\
> +	AD576x_CHANNEL(4, (_bits)),				\
> +	AD576x_CHANNEL(5, (_bits)),				\
> +	AD576x_CHANNEL(6, (_bits)),				\
> +	AD576x_CHANNEL(7, (_bits)),				\
> +	AD576x_CHANNEL(8, (_bits)),				\
> +	AD576x_CHANNEL(9, (_bits)),				\
> +	AD576x_CHANNEL(10, (_bits)),				\
> +	AD576x_CHANNEL(11, (_bits)),				\
> +	AD576x_CHANNEL(12, (_bits)),				\
> +	AD576x_CHANNEL(13, (_bits)),				\
> +	AD576x_CHANNEL(14, (_bits)),				\
> +	AD576x_CHANNEL(15, (_bits)),				\
> +}
> +
> +static DECLARE_AD576x_CHANNELS(ad5766_channels, 16);
> +static DECLARE_AD576x_CHANNELS(ad5767_channels, 12);
> +
> +static const struct ad5766_chip_info ad5766_chip_infos[] = {
> +	[ID_AD5766] = {
> +		.num_channels = ARRAY_SIZE(ad5766_channels),
> +		.channels = ad5766_channels,
> +	},
> +	[ID_AD5767] = {
> +		.num_channels = ARRAY_SIZE(ad5767_channels),
> +		.channels = ad5767_channels,
> +	},
> +};
> +
> +static int ad5766_get_output_range(struct ad5766_state *st)
> +{
> +	int i, ret, min, max, tmp[2];
> +
> +	ret = device_property_read_u32_array(&st->spi->dev,
> +					     "output-range",
> +					     tmp, 2);
> +	if (ret)
> +		return ret;
> +
> +	min = tmp[0];
> +	max = tmp[1];
> +	for (i = 0; i < ARRAY_SIZE(ad5766_span_tbl); i++) {
> +		if (ad5766_span_tbl[i].min != min ||
> +		    ad5766_span_tbl[i].max != max)
> +			continue;
> +
> +		st->crt_range = i;
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad5766_default_setup(struct ad5766_state *st)
> +{
> +	uint16_t val;
> +	int ret, i;
> +
> +	/* Always issue a reset before writing to the span register. */
> +	ret = ad5766_reset(st);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5766_get_output_range(st);
> +	if (ret)
> +		return ret;
> +
> +	/* Dither power down */
> +	st->dither_enable = GENMASK(15, 0);
> +	ret = __ad5766_spi_write(st, AD5766_CMD_WR_PWR_DITHER,
> +			     st->dither_enable);
> +	if (ret)
> +		return ret;
> +
> +	st->dither_source = 0;
> +	for (i = 0; i < ARRAY_SIZE(ad5766_channels); i++)
> +		st->dither_source |= AD5766_DITHER_SOURCE(i, 0);
> +	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_source);
> +	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_1, val);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_source);
> +	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SIG_2, val);
> +	if (ret)
> +		return ret;
> +
> +	st->dither_scale = 0;
> +	val = FIELD_GET(AD5766_LOWER_WORD_SPI_MASK, st->dither_scale);
> +	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_1, val);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_GET(AD5766_UPPER_WORD_SPI_MASK, st->dither_scale);

This is a rather unusual use of FIELD_GET to split a value that we then
write to two 16 bit registers. I guess it works, but does feel a bit odd.
If you are happy with it, I guess I can cope with it confusing me.

> +	ret = __ad5766_spi_write(st, AD5766_CMD_DITHER_SCALE_2, val);
> +	if (ret)
> +		return ret;
> +
> +	st->dither_invert = 0;
> +	ret = __ad5766_spi_write(st, AD5766_CMD_INV_DITHER, st->dither_invert);
> +	if (ret)
> +		return ret;
> +
> +	return  __ad5766_spi_write(st, AD5766_CMD_SPAN_REG, st->crt_range);
> +}
> +
> +static int ad5766_probe(struct spi_device *spi)
> +{
> +	enum ad5766_type type;
> +	struct iio_dev *indio_dev;
> +	struct ad5766_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);
> +
> +	st->spi = spi;
> +	type = spi_get_device_id(spi)->driver_data;
> +	st->chip_info = &ad5766_chip_infos[type];
> +
> +	indio_dev->channels = st->chip_info->channels;
> +	indio_dev->num_channels = st->chip_info->num_channels;
> +	indio_dev->info = &ad5766_info;
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	st->gpio_reset = devm_gpiod_get_optional(&st->spi->dev, "reset",
> +						GPIOD_OUT_LOW);

Minor point, but in theory this might return an error code rather than
NULL (which it will return if no gpio was specified). Should probably
handle that.

> +
> +	ret = ad5766_default_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
...

Re: [PATCH v4 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

[Rob Herring]

On Fri, Dec 18, 2020 at 07:12:29PM +0200, Cristian Pop wrote:
> This adds device tree bindings for the AD5766 DAC.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> ---
>  Changelog v4:
> 	- Add range selection
> 	- Reset is GPIO_ACTIVE_LOW
> 	
>  .../bindings/iio/dac/adi,ad5766.yaml          | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> new file mode 100644
> index 000000000000..846b5ee50761
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5766 DAC device driver
> +
> +maintainers:
> +  - Cristian Pop <cristian.pop@analog.com>
> +
> +description: |
> +  Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
> +  found here:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad5766
> +      - adi,ad5767
> +
> +  output-range:
> +    description: Select converter output range.

Something standard for DACs? If not needs a vendor prefix and type. 

> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 1000000
> +
> +  spi-cpol: true
> +
> +  reset-gpios:
> +    description: GPIO spec for the RESET pin. If specified, it will be asserted
> +      during driver probe. As the line is active low, it should be marked
> +      GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - output-range
> +  - reg
> +  - spi-max-frequency
> +  - spi-cpol
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          
> +          ad5766@0 {
> +              compatible = "adi,ad5766";
> +              output-range = <(-5) 5>;
> +              reg = <0>;
> +              spi-cpol;
> +              spi-max-frequency = <1000000>;
> +              reset-gpios = <&gpio 22 0>;
> +            };
> +      };
> -- 
> 2.17.1
> 

Re: [PATCH v4 1/3] dt-bindings: iio: dac: AD5766 yaml documentation

[Jonathan Cameron]

On Mon, 21 Dec 2020 11:44:34 -0700
Rob Herring <robh@kernel.org> wrote:

> On Fri, Dec 18, 2020 at 07:12:29PM +0200, Cristian Pop wrote:
> > This adds device tree bindings for the AD5766 DAC.
> > 
> > Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> > ---
> >  Changelog v4:
> > 	- Add range selection
> > 	- Reset is GPIO_ACTIVE_LOW
> > 	
> >  .../bindings/iio/dac/adi,ad5766.yaml          | 64 +++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> > new file mode 100644
> > index 000000000000..846b5ee50761
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5766.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2020 Analog Devices Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,ad5766.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD5766 DAC device driver
> > +
> > +maintainers:
> > +  - Cristian Pop <cristian.pop@analog.com>
> > +
> > +description: |
> > +  Bindings for the Analog Devices AD5766 current DAC device. Datasheet can be
> > +  found here:
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad5766-5767.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad5766
> > +      - adi,ad5767
> > +
> > +  output-range:
> > +    description: Select converter output range.  
> 
> Something standard for DACs? If not needs a vendor prefix and type. 

Hmm. It's a common enough thing that we could think about standardizing
it but it would need units in the naming.

output-range-volts

and promoting to a higher level in the DT bindings docs than the individual
driver.

We almost certainly have a bunch of preexisting bindings using manufacturer
specific equivalents.  I've not be completely consistent on this so have
encouraged separate max / min attributes but with hind sight I two value
version like this probably makes more sense in a DT binding (not so much
if userspace is controlling it).


> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 1000000
> > +
> > +  spi-cpol: true
> > +
> > +  reset-gpios:
> > +    description: GPIO spec for the RESET pin. If specified, it will be asserted
> > +      during driver probe. As the line is active low, it should be marked
> > +      GPIO_ACTIVE_LOW.
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - output-range
> > +  - reg
> > +  - spi-max-frequency
> > +  - spi-cpol
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +          
> > +          ad5766@0 {
> > +              compatible = "adi,ad5766";
> > +              output-range = <(-5) 5>;
> > +              reg = <0>;
> > +              spi-cpol;
> > +              spi-max-frequency = <1000000>;
> > +              reset-gpios = <&gpio 22 0>;
> > +            };
> > +      };
> > -- 
> > 2.17.1
> >