[PATCH v2 0/4] Add support for AD5710R/AD5711R DAC

[PATCH v2 0/4] Add support for AD5710R/AD5711R DAC

[Kim Seer Paller]

The AD5710R (16-bit) and AD5711R (12-bit) are 8-channel, low-power,
configurable current/voltage output DACs with an on-chip 2.5V reference.
Each channel can be independently configured as a voltage output
(0V to VREF or 0V to 2xVREF) or a current output (0mA to 50mA).

This series adds support for these parts:
 - ABI documentation for the high_z powerdown mode and the current-output
   powerdown attributes
 - device tree bindings for the AD5710R/AD5711R
 - basic IIO DAC driver, supporting per-channel voltage/current output
   selection, per-channel powerdown, software or hardware (LDAC) DAC
   updates, optional output range doubling, internal or external
   reference, and hardware reset (with a software-reset fallback).

Datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/ad5711r-ad5710r.pdf

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Changes in v2:
- Drop the standalone ad5710r driver and fold AD5710R/AD5711R support
  into the existing ad3530r driver.
- Add a preparatory patch parameterizing the DAC resolution in ad3530r
  so the 12-/16-bit AD5711R/AD5710R can reuse the raw/scale paths.
- ABI: drop the "high_z" powerdown mode, which is a synonym of the
  existing "three_state" and use "three_state" instead.
- Link to v1: https://patch.msgid.link/20260716-iio-ad5710r-upstream-v1-0-e848a4bb701f@analog.com

To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Michael Hennerich <Michael.Hennerich@analog.com>
To: Kim Seer Paller <kimseer.paller@analog.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: linux-iio@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux@analog.com
Cc: devicetree@vger.kernel.org

---
Kim Seer Paller (4):
      iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode
      dt-bindings: iio: dac: add adi,ad5710r.yaml
      iio: dac: ad3530r: parameterize DAC resolution
      iio: dac: ad3530r: add support for AD5710R/AD5711R

 Documentation/ABI/testing/sysfs-bus-iio            |   5 +
 .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 ++++++++++
 MAINTAINERS                                        |   1 +
 drivers/iio/dac/Kconfig                            |   1 +
 drivers/iio/dac/ad3530r.c                          | 295 +++++++++++++++++++--
 5 files changed, 425 insertions(+), 20 deletions(-)
---
base-commit: 36c12dbda81c284d72f3c64689461647497b643b
change-id: 20260716-iio-ad5710r-upstream-e41c298614c6

Best regards,
--  
Kim Seer Paller <kimseer.paller@analog.com>

[PATCH v2 1/4] iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode

[Kim Seer Paller]

Document the out_currentY_powerdown and out_currentY_powerdown_mode
attributes for current-output DAC channels, and add the 15kohm_to_gnd
powerdown mode.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 5cc2e82c4997..ed3dd189c108 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -759,6 +759,8 @@ What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_powerdown_mode
 What:		/sys/bus/iio/devices/iio:deviceX/out_voltage_powerdown_mode
 What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_powerdown_mode
 What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage_powerdown_mode
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_powerdown_mode
+What:		/sys/bus/iio/devices/iio:deviceX/out_current_powerdown_mode
 KernelVersion:	2.6.38
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -771,6 +773,7 @@ Description:
 		6kohm_to_gnd: connected to ground via a 6kOhm resistor,
 		7.7kohm_to_gnd: connected to ground via a 7.7kOhm resistor,
 		10kohm_to_gnd: connected to ground via a 10kOhm resistor,
+		15kohm_to_gnd: connected to ground via a 15kOhm resistor,
 		16kohm_to_gnd: connected to ground via a 16kOhm resistor,
 		20kohm_to_gnd: connected to ground via a 20kOhm resistor,
 		32kohm_to_gnd: connected to ground via a 32kOhm resistor,
@@ -799,6 +802,8 @@ What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_powerdown
 What:		/sys/bus/iio/devices/iio:deviceX/out_voltage_powerdown
 What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltageY_powerdown
 What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage_powerdown
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_powerdown
+What:		/sys/bus/iio/devices/iio:deviceX/out_current_powerdown
 KernelVersion:	2.6.38
 Contact:	linux-iio@vger.kernel.org
 Description:

-- 
2.34.1

[PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Kim Seer Paller]

Add device tree bindings for the Analog Devices AD5710R/AD5711R
8-channel 12-/16-bit Configurable IDAC/VDAC.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 144 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
new file mode 100644
index 000000000000..17c1bf009242
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
@@ -0,0 +1,143 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5710r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5710R/AD5711R 8-channel 12-/16-bit Configurable IDAC/VDAC
+
+maintainers:
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description: |
+  The AD5710R (16-bit) and AD5711R (12-bit) are 8-channel, low-power,
+  configurable current/voltage output digital-to-analog converters (DACs) with
+  an on-chip 2.5V, 3ppm/°C reference. Each channel can be independently
+  configured as a voltage output (0V to VREF or 0V to 2xVREF) or a current
+  output (0mA to 50mA). These devices operate from a single 2.7V to 5.5V
+  supply and are guaranteed monotonic by design.
+  Datasheet can be found here:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad5711r-ad5710r.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad5710r
+      - adi,ad5711r
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 20000000
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  vdd-supply:
+    description: Power Supply Input.
+
+  iovdd-supply:
+    description: Digital Power Supply Input.
+
+  io-channels:
+    description:
+      ADC channel used to monitor internal die temperature, output voltages, and
+      current of a selected channel via the MUXOUT pin.
+    maxItems: 1
+
+  ref-supply:
+    description:
+      Reference Input/Output. The voltage at the REF pin sets the full-scale
+      range of all channels. If not provided the internal reference is used and
+      also provided on the VREF pin.
+
+  reset-gpios:
+    description:
+      Active low signal that is falling edge sensitive. When it is deasserted,
+      the digital core initialization is performed and all DAC registers except
+      the Interface Configuration A register are reset to their default values.
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      LDAC pin to be used as a hardware trigger to update the DAC channels. If
+      not present, the DAC channels are updated by Software LDAC.
+    maxItems: 1
+
+  adi,range-double:
+    description:
+      Configure the output range for all channels. If the property is present,
+      the output will range from 0V to 2Vref. If the property is not present,
+      the output will range from 0V to Vref.
+    type: boolean
+
+patternProperties:
+  "^channel@[0-7]$":
+    $ref: /schemas/iio/dac/dac.yaml#
+    type: object
+    description:
+      Represents the external channels which are connected to the DAC.
+
+    properties:
+      reg:
+        description: Channel number
+        items:
+          minimum: 0
+          maximum: 7
+
+      adi,ch-func:
+        description:
+          Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
+          output or CH_FUNC_CURRENT_OUTPUT for current output.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [1, 2]
+
+    required:
+      - reg
+      - adi,ch-func
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - iovdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/iio/addac/adi,ad74413r.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        dac@0 {
+            compatible = "adi,ad5710r";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            vdd-supply = <&vdd>;
+            iovdd-supply = <&iovdd>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 {
+                reg = <0>;
+                adi,ch-func = <CH_FUNC_VOLTAGE_OUTPUT>;
+            };
+
+            channel@1 {
+                reg = <1>;
+                adi,ch-func = <CH_FUNC_CURRENT_OUTPUT>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7077ed7c9efe..f207125383f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1444,6 +1444,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
+F:	Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
 F:	drivers/iio/dac/ad3530r.c
 
 ANALOG DEVICES INC AD3552R DRIVER

-- 
2.34.1

[PATCH v2 3/4] iio: dac: ad3530r: parameterize DAC resolution

[Kim Seer Paller]

Add a per-chip resolution field and use it in the raw read/write and
scale paths instead of assuming 16 bits. Drop AD3530R_REG_VAL_MASK and
AD3530R_DAC_MAX_VAL, which hardcoded the 16-bit width and are now
unused.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 drivers/iio/dac/ad3530r.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
index 06b7f280f61b..97483534f8c3 100644
--- a/drivers/iio/dac/ad3530r.c
+++ b/drivers/iio/dac/ad3530r.c
@@ -61,14 +61,12 @@
 #define AD3530R_SLD_TRIG_A			BIT(7)
 #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
 #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
-#define AD3530R_REG_VAL_MASK			GENMASK(15, 0)
 #define AD3530R_OP_MODE_CHAN_MSK(chan)		(GENMASK(1, 0) << 2 * (chan))
 
 #define AD3530R_SW_RESET			(BIT(7) | BIT(0))
 #define AD3530R_INTERNAL_VREF_mV		2500
 #define AD3530R_LDAC_PULSE_US			100
 
-#define AD3530R_DAC_MAX_VAL			GENMASK(15, 0)
 #define AD3530R_CH_PER_REG			4
 #define AD3530R_CH_PER_BANK			8
 #define AD3531R_MAX_CHANNELS			4
@@ -99,6 +97,7 @@ struct ad3530r_chip_info {
 	unsigned int num_channels;
 	unsigned int num_banks;
 	unsigned int num_op_mode_regs;
+	unsigned int resolution;
 	bool internal_ref_support;
 };
 
@@ -310,7 +309,7 @@ static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
 	int ret;
 
 	guard(mutex)(&st->lock);
-	st->buf = cpu_to_be16(val);
+	st->buf = cpu_to_be16(val << (16 - st->chip_info->resolution));
 
 	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
 				&st->buf, sizeof(st->buf));
@@ -340,12 +339,12 @@ static int ad3530r_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(st->buf));
+		*val = be16_to_cpu(st->buf) >> (16 - st->chip_info->resolution);
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->vref_mV;
-		*val2 = 16;
+		*val2 = st->chip_info->resolution;
 
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
@@ -361,7 +360,7 @@ static int ad3530r_write_raw(struct iio_dev *indio_dev,
 
 	switch (info) {
 	case IIO_CHAN_INFO_RAW:
-		if (val < 0 || val > AD3530R_DAC_MAX_VAL)
+		if (val < 0 || val > (1 << st->chip_info->resolution) - 1)
 			return -EINVAL;
 
 		return ad3530r_dac_write(st, chan->channel, val);
@@ -525,6 +524,7 @@ static const struct regmap_config ad3532r_regmap_config = {
 
 static const struct ad3530r_chip_info ad3530_chip = {
 	.name = "ad3530",
+	.resolution = 16,
 	.channels = ad3530r_channels,
 	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
@@ -541,6 +541,7 @@ static const struct ad3530r_chip_info ad3530_chip = {
 
 static const struct ad3530r_chip_info ad3530r_chip = {
 	.name = "ad3530r",
+	.resolution = 16,
 	.channels = ad3530r_channels,
 	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
@@ -557,6 +558,7 @@ static const struct ad3530r_chip_info ad3530r_chip = {
 
 static const struct ad3530r_chip_info ad3531_chip = {
 	.name = "ad3531",
+	.resolution = 16,
 	.channels = ad3531r_channels,
 	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
@@ -573,6 +575,7 @@ static const struct ad3530r_chip_info ad3531_chip = {
 
 static const struct ad3530r_chip_info ad3531r_chip = {
 	.name = "ad3531r",
+	.resolution = 16,
 	.channels = ad3531r_channels,
 	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
@@ -589,6 +592,7 @@ static const struct ad3530r_chip_info ad3531r_chip = {
 
 static const struct ad3530r_chip_info ad3532_chip = {
 	.name = "ad3532",
+	.resolution = 16,
 	.channels = ad3532r_channels,
 	.regmap_config = &ad3532r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3532r_channels),
@@ -605,6 +609,7 @@ static const struct ad3530r_chip_info ad3532_chip = {
 
 static const struct ad3530r_chip_info ad3532r_chip = {
 	.name = "ad3532r",
+	.resolution = 16,
 	.channels = ad3532r_channels,
 	.regmap_config = &ad3532r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3532r_channels),

-- 
2.34.1

[PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

[Kim Seer Paller]

Add support for the AD5710R/AD5711R, 8-channel 16-/12-bit configurable
IDAC/VDAC parts. They share the AD3530R register map and access model,
so fold them into this driver.

Each channel is configured as voltage or current output from its DT
channel@N node via adi,ch-func, building the iio_chan_spec dynamically.
Voltage channels enable VMODE_EN and report the reference-derived scale,
current channels report the 50 mA internal Iref scale. The powerdown
mode is read-only and derived from the channel's configured type.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 drivers/iio/dac/Kconfig   |   1 +
 drivers/iio/dac/ad3530r.c | 278 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 265 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index d6d560c09e25..e9f388f8fac3 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -16,6 +16,7 @@ config AD3530R
 	  - AD3530/AD3530R (8-channel)
 	  - AD3531/AD3531R (4-channel)
 	  - AD3532/AD3532R (16-channel)
+	  - AD5710R/AD5711R (8-channel configurable IDAC/VDAC)
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad3530r.
diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
index 97483534f8c3..935b3a832d8e 100644
--- a/drivers/iio/dac/ad3530r.c
+++ b/drivers/iio/dac/ad3530r.c
@@ -3,6 +3,7 @@
  * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
  * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
  * AD3532R/AD3532 16-channel, 16-bit Voltage Output DAC Driver
+ * AD5710R/AD5711R 8-channel, 16-/12-bit Configurable IDAC/VDAC Driver
  *
  * Copyright 2025 Analog Devices Inc.
  */
@@ -27,6 +28,8 @@
 #include <linux/types.h>
 #include <linux/units.h>
 
+#include <dt-bindings/iio/addac/adi,ad74413r.h>
+
 #define AD3530R_INTERFACE_CONFIG_A		0x00
 #define AD3530R_OUTPUT_OPERATING_MODE_0		0x20
 #define AD3530R_OUTPUT_OPERATING_MODE_1		0x21
@@ -39,6 +42,8 @@
 #define AD3531R_SW_LDAC_TRIG_A			0xDD
 #define AD3531R_INPUT_CH			0xE3
 
+#define AD5710R_CHN_VMODE_EN			0xFF
+
 /* AD3532R/AD3532 bank 0 registers (channels 0-7) */
 #define AD3532R_INTERFACE_CONFIG_A_0		0x1000
 #define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
@@ -62,15 +67,18 @@
 #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
 #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
 #define AD3530R_OP_MODE_CHAN_MSK(chan)		(GENMASK(1, 0) << 2 * (chan))
+#define AD5710R_CHN_VMODE_EN_BIT(chan)		BIT(chan)
 
 #define AD3530R_SW_RESET			(BIT(7) | BIT(0))
 #define AD3530R_INTERNAL_VREF_mV		2500
+#define AD5710R_INTERNAL_IREF_mA		50
 #define AD3530R_LDAC_PULSE_US			100
 
 #define AD3530R_CH_PER_REG			4
 #define AD3530R_CH_PER_BANK			8
 #define AD3531R_MAX_CHANNELS			4
 #define AD3532R_MAX_CHANNELS			16
+#define AD5710R_NUM_CHANNELS			8
 
 enum ad3530r_mode {
 	AD3530R_NORMAL_OP,
@@ -99,6 +107,7 @@ struct ad3530r_chip_info {
 	unsigned int num_op_mode_regs;
 	unsigned int resolution;
 	bool internal_ref_support;
+	bool channel_configurable;
 };
 
 struct ad3530r_state {
@@ -107,6 +116,8 @@ struct ad3530r_state {
 	struct mutex lock;
 	struct ad3530r_chan chan[AD3532R_MAX_CHANNELS];
 	const struct ad3530r_chip_info *chip_info;
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
 	struct gpio_desc *ldac_gpio;
 	int vref_mV;
 	/*
@@ -152,6 +163,11 @@ static const char * const ad3532r_powerdown_modes[] = {
 	"three_state",
 };
 
+static const char * const ad5710r_powerdown_modes[] = {
+	"15kohm_to_gnd",
+	"three_state",
+};
+
 static int ad3530r_get_powerdown_mode(struct iio_dev *indio_dev,
 				      const struct iio_chan_spec *chan)
 {
@@ -194,6 +210,26 @@ static const struct iio_enum ad3532r_powerdown_mode_enum = {
 	.set = ad3530r_set_powerdown_mode,
 };
 
+static int ad5710r_get_powerdown_mode(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, AD5710R_CHN_VMODE_EN, &val);
+	if (ret)
+		return ret;
+
+	return !(val & AD5710R_CHN_VMODE_EN_BIT(chan->channel));
+}
+
+static const struct iio_enum ad5710r_powerdown_mode_enum = {
+	.items = ad5710r_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad5710r_powerdown_modes),
+	.get = ad5710r_get_powerdown_mode,
+};
+
 static ssize_t ad3530r_get_dac_powerdown(struct iio_dev *indio_dev,
 					 uintptr_t private,
 					 const struct iio_chan_spec *chan,
@@ -277,6 +313,54 @@ static ssize_t ad3532r_set_dac_powerdown(struct iio_dev *indio_dev,
 	return len;
 }
 
+static ssize_t ad5710r_get_dac_powerdown(struct iio_dev *indio_dev,
+					 uintptr_t private,
+					 const struct iio_chan_spec *chan,
+					 char *buf)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	unsigned int reg_offset, ch_in_reg, reg, mode, mask;
+	int ret;
+
+	reg_offset = chan->channel / AD3530R_CH_PER_REG;
+	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
+	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
+	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
+
+	ret = regmap_read(st->regmap, reg, &mode);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%d\n", !!(mode & mask));
+}
+
+static ssize_t ad5710r_set_dac_powerdown(struct iio_dev *indio_dev,
+					 uintptr_t private,
+					 const struct iio_chan_spec *chan,
+					 const char *buf, size_t len)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	unsigned int reg_offset, ch_in_reg, reg, mask, val;
+	bool powerdown;
+	int ret;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	reg_offset = chan->channel / AD3530R_CH_PER_REG;
+	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
+	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
+	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
+	val = field_prep(mask, powerdown);
+
+	ret = regmap_update_bits(st->regmap, reg, mask, val);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static int ad3530r_trigger_sw_ldac_reg(unsigned int channel)
 {
 	return AD3530R_SW_LDAC_TRIG_A;
@@ -343,7 +427,10 @@ static int ad3530r_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = st->vref_mV;
+		if (chan->type == IIO_CURRENT)
+			*val = AD5710R_INTERNAL_IREF_mA;
+		else
+			*val = st->vref_mV;
 		*val2 = st->chip_info->resolution;
 
 		return IIO_VAL_FRACTIONAL_LOG2;
@@ -419,6 +506,17 @@ static const struct iio_chan_spec_ext_info ad3532r_ext_info[] = {
 	{ }
 };
 
+static const struct iio_chan_spec_ext_info ad5710r_ext_info[] = {
+	{
+		.name = "powerdown",
+		.shared = IIO_SEPARATE,
+		.read = ad5710r_get_dac_powerdown,
+		.write = ad5710r_set_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad5710r_powerdown_mode_enum),
+	{ }
+};
+
 #define AD3530R_CHAN(_chan, _ext_info)				\
 {								\
 	.type = IIO_VOLTAGE,					\
@@ -467,6 +565,17 @@ static const struct iio_chan_spec ad3532r_channels[] = {
 	AD3530R_CHAN(15, ad3532r_ext_info),
 };
 
+static const struct iio_chan_spec ad5710r_channels[] = {
+	AD3530R_CHAN(0, ad5710r_ext_info),
+	AD3530R_CHAN(1, ad5710r_ext_info),
+	AD3530R_CHAN(2, ad5710r_ext_info),
+	AD3530R_CHAN(3, ad5710r_ext_info),
+	AD3530R_CHAN(4, ad5710r_ext_info),
+	AD3530R_CHAN(5, ad5710r_ext_info),
+	AD3530R_CHAN(6, ad5710r_ext_info),
+	AD3530R_CHAN(7, ad5710r_ext_info),
+};
+
 static const unsigned int ad3530r_if_config[] = {
 	AD3530R_INTERFACE_CONFIG_A,
 };
@@ -522,6 +631,12 @@ static const struct regmap_config ad3532r_regmap_config = {
 	.max_register = AD3532R_MAX_REG_ADDR,
 };
 
+static const struct regmap_config ad5710r_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = AD5710R_CHN_VMODE_EN,
+};
+
 static const struct ad3530r_chip_info ad3530_chip = {
 	.name = "ad3530",
 	.resolution = 16,
@@ -537,6 +652,7 @@ static const struct ad3530r_chip_info ad3530_chip = {
 	.num_banks = ARRAY_SIZE(ad3530r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3530r_op_mode),
 	.internal_ref_support = false,
+	.channel_configurable = false,
 };
 
 static const struct ad3530r_chip_info ad3530r_chip = {
@@ -554,6 +670,7 @@ static const struct ad3530r_chip_info ad3530r_chip = {
 	.num_banks = ARRAY_SIZE(ad3530r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3530r_op_mode),
 	.internal_ref_support = true,
+	.channel_configurable = false,
 };
 
 static const struct ad3530r_chip_info ad3531_chip = {
@@ -571,6 +688,7 @@ static const struct ad3530r_chip_info ad3531_chip = {
 	.num_banks = ARRAY_SIZE(ad3530r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3531r_op_mode),
 	.internal_ref_support = false,
+	.channel_configurable = false,
 };
 
 static const struct ad3530r_chip_info ad3531r_chip = {
@@ -588,6 +706,7 @@ static const struct ad3530r_chip_info ad3531r_chip = {
 	.num_banks = ARRAY_SIZE(ad3530r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3531r_op_mode),
 	.internal_ref_support = true,
+	.channel_configurable = false,
 };
 
 static const struct ad3530r_chip_info ad3532_chip = {
@@ -605,6 +724,7 @@ static const struct ad3530r_chip_info ad3532_chip = {
 	.num_banks = ARRAY_SIZE(ad3532r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3532r_op_mode),
 	.internal_ref_support = false,
+	.channel_configurable = false,
 };
 
 static const struct ad3530r_chip_info ad3532r_chip = {
@@ -622,8 +742,124 @@ static const struct ad3530r_chip_info ad3532r_chip = {
 	.num_banks = ARRAY_SIZE(ad3532r_if_config),
 	.num_op_mode_regs = ARRAY_SIZE(ad3532r_op_mode),
 	.internal_ref_support = true,
+	.channel_configurable = false,
+};
+
+static const struct ad3530r_chip_info ad5710r_chip = {
+	.name = "ad5710r",
+	.resolution = 16,
+	.regmap_config = &ad5710r_regmap_config,
+	.num_channels = ARRAY_SIZE(ad5710r_channels),
+	.sw_ldac_trig_reg = ad3530r_trigger_sw_ldac_reg,
+	.input_ch_reg = ad3530r_input_ch_reg,
+	.interface_config_a = ad3530r_if_config,
+	.output_control = ad3530r_out_ctrl,
+	.reference_control = ad3530r_ref_ctrl,
+	.op_mode = ad3530r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3530r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3530r_op_mode),
+	.internal_ref_support = true,
+	.channel_configurable = true,
+};
+
+static const struct ad3530r_chip_info ad5711r_chip = {
+	.name = "ad5711r",
+	.resolution = 12,
+	.regmap_config = &ad5710r_regmap_config,
+	.num_channels = ARRAY_SIZE(ad5710r_channels),
+	.sw_ldac_trig_reg = ad3530r_trigger_sw_ldac_reg,
+	.input_ch_reg = ad3530r_input_ch_reg,
+	.interface_config_a = ad3530r_if_config,
+	.output_control = ad3530r_out_ctrl,
+	.reference_control = ad3530r_ref_ctrl,
+	.op_mode = ad3530r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3530r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3530r_op_mode),
+	.internal_ref_support = true,
+	.channel_configurable = true,
 };
 
+static int ad3530r_parse_channel_cfg(struct ad3530r_state *st)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	struct iio_chan_spec *channels;
+	int ret, num_chan;
+	int i = 0;
+	u32 reg;
+
+	num_chan = device_get_child_node_count(dev);
+	if (!num_chan)
+		return dev_err_probe(dev, -ENODEV, "No channels configured\n");
+
+	channels = devm_kcalloc(dev, num_chan, sizeof(*channels), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
+
+	device_for_each_child_node_scoped(dev, child) {
+		unsigned int reg_offset, ch_in_reg, mode_reg, mode_mask, ch_func;
+		enum iio_chan_type chan_type;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to read reg property of %pfwP\n",
+					     child);
+
+		if (reg >= AD5710R_NUM_CHANNELS)
+			return dev_err_probe(dev, -EINVAL,
+					     "reg out of range in %pfwP\n",
+					     child);
+
+		ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Missing adi,ch-func property for %pfwP\n",
+					     child);
+
+		switch (ch_func) {
+		case CH_FUNC_VOLTAGE_OUTPUT:
+			ret = regmap_set_bits(st->regmap, AD5710R_CHN_VMODE_EN,
+					      AD5710R_CHN_VMODE_EN_BIT(reg));
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to set voltage output for %pfwP\n",
+						     child);
+
+			chan_type = IIO_VOLTAGE;
+			break;
+		case CH_FUNC_CURRENT_OUTPUT:
+			chan_type = IIO_CURRENT;
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid adi,ch-func %u for %pfwP\n",
+					     ch_func, child);
+		}
+
+		channels[i] = ad5710r_channels[reg];
+		channels[i].type = chan_type;
+		i++;
+
+		reg_offset = reg / AD3530R_CH_PER_REG;
+		ch_in_reg = reg % AD3530R_CH_PER_REG;
+		mode_reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
+		mode_mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
+
+		/* Enable the channel in normal operation mode */
+		ret = regmap_update_bits(st->regmap, mode_reg, mode_mask,
+					 field_prep(mode_mask, AD3530R_NORMAL_OP));
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to set normal operating mode for %pfwP\n",
+					     child);
+	}
+
+	st->channels = channels;
+	st->num_channels = num_chan;
+
+	return 0;
+}
+
 static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
 				     const unsigned int *regs,
 				     unsigned int num_regs,
@@ -707,19 +943,29 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 		st->vref_mV = range_multiplier * AD3530R_INTERNAL_VREF_mV;
 	}
 
-	/* Set normal operating mode for all channels */
-	val = FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(0), AD3530R_NORMAL_OP) |
-	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(1), AD3530R_NORMAL_OP) |
-	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(2), AD3530R_NORMAL_OP) |
-	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(3), AD3530R_NORMAL_OP);
+	if (chip_info->channel_configurable) {
+		/* Channels and their operating mode are configured from DT */
+		ret = ad3530r_parse_channel_cfg(st);
+		if (ret)
+			return ret;
+	} else {
+		/* Set normal operating mode for all channels */
+		val = FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(0), AD3530R_NORMAL_OP) |
+		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(1), AD3530R_NORMAL_OP) |
+		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(2), AD3530R_NORMAL_OP) |
+		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(3), AD3530R_NORMAL_OP);
+
+		ret = ad3530r_write_reg_banks(st, chip_info->op_mode,
+					      chip_info->num_op_mode_regs, val);
+		if (ret)
+			return ret;
 
-	ret = ad3530r_write_reg_banks(st, st->chip_info->op_mode,
-				      st->chip_info->num_op_mode_regs, val);
-	if (ret)
-		return ret;
+		for (unsigned int i = 0; i < chip_info->num_channels; i++)
+			st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
 
-	for (unsigned int i = 0; i < st->chip_info->num_channels; i++)
-		st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
+		st->channels = chip_info->channels;
+		st->num_channels = chip_info->num_channels;
+	}
 
 	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_LOW);
 	if (IS_ERR(st->ldac_gpio))
@@ -784,8 +1030,8 @@ static int ad3530r_probe(struct spi_device *spi)
 	indio_dev->name = st->chip_info->name;
 	indio_dev->info = &ad3530r_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = st->chip_info->channels;
-	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->channels = st->channels;
+	indio_dev->num_channels = st->num_channels;
 
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
@@ -797,6 +1043,8 @@ static const struct spi_device_id ad3530r_id[] = {
 	{ .name = "ad3531r", .driver_data = (kernel_ulong_t)&ad3531r_chip },
 	{ .name = "ad3532", .driver_data = (kernel_ulong_t)&ad3532_chip },
 	{ .name = "ad3532r", .driver_data = (kernel_ulong_t)&ad3532r_chip },
+	{ .name = "ad5710r", .driver_data = (kernel_ulong_t)&ad5710r_chip },
+	{ .name = "ad5711r", .driver_data = (kernel_ulong_t)&ad5711r_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad3530r_id);
@@ -808,6 +1056,8 @@ static const struct of_device_id ad3530r_of_match[] = {
 	{ .compatible = "adi,ad3531r", .data = &ad3531r_chip },
 	{ .compatible = "adi,ad3532", .data = &ad3532_chip },
 	{ .compatible = "adi,ad3532r", .data = &ad3532r_chip },
+	{ .compatible = "adi,ad5710r", .data = &ad5710r_chip },
+	{ .compatible = "adi,ad5711r", .data = &ad5711r_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad3530r_of_match);

-- 
2.34.1

Re: [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `powerdown_mode` sysfs attribute is exposed with write permissions despite being read-only, causing write attempts to fail unconditionally with `-EINVAL`.
--

--- Patch [4]: [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R ---
commit 3c7f6e84bbb3d2178089f97875a1f7dc3599d417
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: add support for AD5710R/AD5711R

This patch adds support for the AD5710R/AD5711R 8-channel configurable 
IDAC/VDAC parts. The new devices are folded into the ad3530r driver as 
they share the same register map and access model.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 97483534f8c33..935b3a832d8e9 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c

[ ... ]

> @@ -194,6 +210,26 @@ static const struct iio_enum ad3532r_powerdown_mode_enum = {
>  	.set = ad3530r_set_powerdown_mode,
>  };
>  
> +static int ad5710r_get_powerdown_mode(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, AD5710R_CHN_VMODE_EN, &val);
> +	if (ret)
> +		return ret;
> +
> +	return !(val & AD5710R_CHN_VMODE_EN_BIT(chan->channel));
> +}
> +
> +static const struct iio_enum ad5710r_powerdown_mode_enum = {
> +	.items = ad5710r_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ad5710r_powerdown_modes),
> +	.get = ad5710r_get_powerdown_mode,
> +};
> +

[ ... ]

> @@ -419,6 +506,17 @@ static const struct iio_chan_spec_ext_info ad3532r_ext_info[] = {
>  	{ }
>  };
>  
> +static const struct iio_chan_spec_ext_info ad5710r_ext_info[] = {
> +	{
> +		.name = "powerdown",
> +		.shared = IIO_SEPARATE,
> +		.read = ad5710r_get_dac_powerdown,
> +		.write = ad5710r_set_dac_powerdown,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad5710r_powerdown_mode_enum),

[Severity: Low]
Does using the IIO_ENUM macro here incorrectly expose the attribute as writable?

Since ad5710r_powerdown_mode_enum lacks a .set callback, any writes to the
powerdown_mode sysfs attribute will fail unconditionally with -EINVAL. 

However, the IIO_ENUM macro automatically assigns iio_enum_write to the .write 
callback, which grants write permissions in sysfs. Should this attribute be 
manually defined in the ext_info array without a .write callback to properly 
enforce read-only semantics at the VFS layer?

> +	{ }
> +};
> +
>  #define AD3530R_CHAN(_chan, _ext_info)				\
>  {								\
>  	.type = IIO_VOLTAGE,					\

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-iio-ad5710r-upstream-v2-0-324949dc72da@analog.com?part=4

Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `adi,range-double` property is defined as a global device property, artificially limiting the hardware's per-channel capabilities and conflicting with current output modes.
- [Medium] The `reg` property inside `patternProperties` incorrectly uses `items` to apply numeric constraints, rendering channel bounds checking completely ineffective.
--

--- Patch [2]: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml ---
commit fe69da2e77f6f1de3acc9e529bec6856901e4e98
Author: Kim Seer Paller <kimseer.paller@analog.com>

dt-bindings: iio: dac: add adi,ad5710r.yaml

Add device tree bindings for the Analog Devices AD5710R/AD5711R
8-channel 12-/16-bit Configurable IDAC/VDAC.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
> new file mode 100644
> index 0000000000000..17c1bf0092421
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml

[ ... ]

> +  adi,range-double:
> +    description:
> +      Configure the output range for all channels. If the property is present,
> +      the output will range from 0V to 2Vref. If the property is not present,
> +      the output will range from 0V to Vref.
> +    type: boolean

[Severity: Medium]
Since the description notes that each channel can be independently configured
as either a voltage or current output, should this property be placed inside
the channel patternProperties block instead? 

Defining it globally seems to restrict configuring the output range on a
per-channel basis, and might also semantically conflict with channels
configured for current output.

> +
> +patternProperties:
> +  "^channel@[0-7]$":
> +    $ref: /schemas/iio/dac/dac.yaml#
> +    type: object
> +    description:
> +      Represents the external channels which are connected to the DAC.
> +
> +    properties:
> +      reg:
> +        description: Channel number
> +        items:
> +          minimum: 0
> +          maximum: 7

[Severity: Medium]
Does placing minimum and maximum inside an items dictionary correctly apply
the constraints? 

In dt-schema, the reg property is mapped to a matrix. Wrapping the constraints
in items applies them to the tuple array instead of the scalar integers,
causing schema validation to ignore them and silently accept out-of-bounds
channel numbers. 

Could these bounds be placed directly under the reg property?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-iio-ad5710r-upstream-v2-0-324949dc72da@analog.com?part=2

Re: [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

[Andy Shevchenko]

On Tue, Jul 21, 2026 at 04:47:13PM +0800, Kim Seer Paller wrote:
> Add support for the AD5710R/AD5711R, 8-channel 16-/12-bit configurable
> IDAC/VDAC parts. They share the AD3530R register map and access model,
> so fold them into this driver.
> 
> Each channel is configured as voltage or current output from its DT
> channel@N node via adi,ch-func, building the iio_chan_spec dynamically.
> Voltage channels enable VMODE_EN and report the reference-derived scale,
> current channels report the 50 mA internal Iref scale. The powerdown
> mode is read-only and derived from the channel's configured type.

...

>   * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
>   * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
>   * AD3532R/AD3532 16-channel, 16-bit Voltage Output DAC Driver
> + * AD5710R/AD5711R 8-channel, 16-/12-bit Configurable IDAC/VDAC Driver

In the above only a single data width is mentioned, maybe split this one?

...

>  #define AD3531R_MAX_CHANNELS			4
>  #define AD3532R_MAX_CHANNELS			16
> +#define AD5710R_NUM_CHANNELS			8

Why NUM and not MAX?

...

> +static int ad5710r_get_powerdown_mode(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, AD5710R_CHN_VMODE_EN, &val);
> +	if (ret)
> +		return ret;
> +
> +	return !(val & AD5710R_CHN_VMODE_EN_BIT(chan->channel));

regmap_test_bits()

> +}

...

> +static ssize_t ad5710r_get_dac_powerdown(struct iio_dev *indio_dev,
> +					 uintptr_t private,
> +					 const struct iio_chan_spec *chan,
> +					 char *buf)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_offset, ch_in_reg, reg, mode, mask;
> +	int ret;
> +
> +	reg_offset = chan->channel / AD3530R_CH_PER_REG;
> +	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
> +	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> +	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> +
> +	ret = regmap_read(st->regmap, reg, &mode);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%d\n", !!(mode & mask));

Ditto.

> +}

...

> +static const struct regmap_config ad5710r_regmap_config = {
> +	.reg_bits = 16,
> +	.val_bits = 8,
> +	.max_register = AD5710R_CHN_VMODE_EN,
> +};

No cache?

...

> +static int ad3530r_parse_channel_cfg(struct ad3530r_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct iio_chan_spec *channels;
> +	int ret, num_chan;

Why is 'num_chan' signed?


> +	int i = 0;

Signed? Also, split assignment and move it closer to its first user.

> +	u32 reg;
> +
> +	num_chan = device_get_child_node_count(dev);
> +	if (!num_chan)
> +		return dev_err_probe(dev, -ENODEV, "No channels configured\n");
> +
> +	channels = devm_kcalloc(dev, num_chan, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;


	i = 0;

> +	device_for_each_child_node_scoped(dev, child) {
> +		unsigned int reg_offset, ch_in_reg, mode_reg, mode_mask, ch_func;
> +		enum iio_chan_type chan_type;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to read reg property of %pfwP\n",
> +					     child);
> +
> +		if (reg >= AD5710R_NUM_CHANNELS)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "reg out of range in %pfwP\n",
> +					     child);
> +
> +		ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Missing adi,ch-func property for %pfwP\n",
> +					     child);
> +
> +		switch (ch_func) {
> +		case CH_FUNC_VOLTAGE_OUTPUT:
> +			ret = regmap_set_bits(st->regmap, AD5710R_CHN_VMODE_EN,
> +					      AD5710R_CHN_VMODE_EN_BIT(reg));
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Failed to set voltage output for %pfwP\n",
> +						     child);
> +
> +			chan_type = IIO_VOLTAGE;
> +			break;
> +		case CH_FUNC_CURRENT_OUTPUT:
> +			chan_type = IIO_CURRENT;
> +			break;
> +		default:
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid adi,ch-func %u for %pfwP\n",
> +					     ch_func, child);
> +		}
> +
> +		channels[i] = ad5710r_channels[reg];
> +		channels[i].type = chan_type;
> +		i++;
> +
> +		reg_offset = reg / AD3530R_CH_PER_REG;
> +		ch_in_reg = reg % AD3530R_CH_PER_REG;
> +		mode_reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> +		mode_mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> +
> +		/* Enable the channel in normal operation mode */
> +		ret = regmap_update_bits(st->regmap, mode_reg, mode_mask,
> +					 field_prep(mode_mask, AD3530R_NORMAL_OP));
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to set normal operating mode for %pfwP\n",
> +					     child);
> +	}
> +
> +	st->channels = channels;
> +	st->num_channels = num_chan;
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Conor Dooley]

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

On Tue, Jul 21, 2026 at 04:47:11PM +0800, Kim Seer Paller wrote:
> Add device tree bindings for the Analog Devices AD5710R/AD5711R
> 8-channel 12-/16-bit Configurable IDAC/VDAC.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++

> +patternProperties:
> +  "^channel@[0-7]$":
> +    $ref: /schemas/iio/dac/dac.yaml#
> +    type: object
> +    description:
> +      Represents the external channels which are connected to the DAC.
> +
> +    properties:
> +      reg:
> +        description: Channel number
> +        items:
> +          minimum: 0
> +          maximum: 7
> +
> +      adi,ch-func:
> +        description:
> +          Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
> +          output or CH_FUNC_CURRENT_OUTPUT for current output.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [1, 2]

Please make this property a string.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

[Kim Seer Paller]

On Tue Jul 21, 2026 at 6:38 PM PST, Andy Shevchenko wrote:
> On Tue, Jul 21, 2026 at 04:47:13PM +0800, Kim Seer Paller wrote:
> > Add support for the AD5710R/AD5711R, 8-channel 16-/12-bit configurable
> > IDAC/VDAC parts. They share the AD3530R register map and access model,
> > so fold them into this driver.
> > 
> > Each channel is configured as voltage or current output from its DT
> > channel@N node via adi,ch-func, building the iio_chan_spec dynamically.
> > Voltage channels enable VMODE_EN and report the reference-derived scale,
> > current channels report the 50 mA internal Iref scale. The powerdown
> > mode is read-only and derived from the channel's configured type.
>
> ...
>
> >   * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
> >   * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
> >   * AD3532R/AD3532 16-channel, 16-bit Voltage Output DAC Driver
> > + * AD5710R/AD5711R 8-channel, 16-/12-bit Configurable IDAC/VDAC Driver
>
> In the above only a single data width is mentioned, maybe split this one?

I agree, will split this into 2 lines.

>
> ...
>
> >  #define AD3531R_MAX_CHANNELS			4
> >  #define AD3532R_MAX_CHANNELS			16
> > +#define AD5710R_NUM_CHANNELS			8
>
> Why NUM and not MAX?

I think this can be drop since it was only used once in ad3530r_parse_channel_cfg()
and that check can used chip_info->num_channels instead.

>
> ...
>
> > +static int ad5710r_get_powerdown_mode(struct iio_dev *indio_dev,
> > +				      const struct iio_chan_spec *chan)
> > +{
> > +	struct ad3530r_state *st = iio_priv(indio_dev);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	ret = regmap_read(st->regmap, AD5710R_CHN_VMODE_EN, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return !(val & AD5710R_CHN_VMODE_EN_BIT(chan->channel));
>
> regmap_test_bits()
>
> > +}
>
> ...
>
> > +static ssize_t ad5710r_get_dac_powerdown(struct iio_dev *indio_dev,
> > +					 uintptr_t private,
> > +					 const struct iio_chan_spec *chan,
> > +					 char *buf)
> > +{
> > +	struct ad3530r_state *st = iio_priv(indio_dev);
> > +	unsigned int reg_offset, ch_in_reg, reg, mode, mask;
> > +	int ret;
> > +
> > +	reg_offset = chan->channel / AD3530R_CH_PER_REG;
> > +	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
> > +	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> > +	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> > +
> > +	ret = regmap_read(st->regmap, reg, &mode);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return sysfs_emit(buf, "%d\n", !!(mode & mask));
>
> Ditto.
>
> > +}
>
> ...
>
> > +static const struct regmap_config ad5710r_regmap_config = {
> > +	.reg_bits = 16,
> > +	.val_bits = 8,
> > +	.max_register = AD5710R_CHN_VMODE_EN,
> > +};
>
> No cache?

The existing ad3530r/ad3532r regmap configs don't use a cache either,
so kept this one consistent. Can add it as a separate cleanup across
all 3 configs if preferred.

>
> ...
>
> > +static int ad3530r_parse_channel_cfg(struct ad3530r_state *st)
> > +{
> > +	struct device *dev = regmap_get_device(st->regmap);
> > +	struct iio_chan_spec *channels;
> > +	int ret, num_chan;
>
> Why is 'num_chan' signed?
>
>
> > +	int i = 0;
>
> Signed? Also, split assignment and move it closer to its first user.
>
> > +	u32 reg;
> > +
> > +	num_chan = device_get_child_node_count(dev);
> > +	if (!num_chan)
> > +		return dev_err_probe(dev, -ENODEV, "No channels configured\n");
> > +
> > +	channels = devm_kcalloc(dev, num_chan, sizeof(*channels), GFP_KERNEL);
> > +	if (!channels)
> > +		return -ENOMEM;
>
>
> 	i = 0;
>
> > +	device_for_each_child_node_scoped(dev, child) {
> > +		unsigned int reg_offset, ch_in_reg, mode_reg, mode_mask, ch_func;
> > +		enum iio_chan_type chan_type;
> > +
> > +		ret = fwnode_property_read_u32(child, "reg", &reg);
> > +		if (ret)
> > +			return dev_err_probe(dev, ret,
> > +					     "Failed to read reg property of %pfwP\n",
> > +					     child);
> > +
> > +		if (reg >= AD5710R_NUM_CHANNELS)
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "reg out of range in %pfwP\n",
> > +					     child);
> > +
> > +		ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
> > +		if (ret)
> > +			return dev_err_probe(dev, ret,
> > +					     "Missing adi,ch-func property for %pfwP\n",
> > +					     child);
> > +
> > +		switch (ch_func) {
> > +		case CH_FUNC_VOLTAGE_OUTPUT:
> > +			ret = regmap_set_bits(st->regmap, AD5710R_CHN_VMODE_EN,
> > +					      AD5710R_CHN_VMODE_EN_BIT(reg));
> > +			if (ret)
> > +				return dev_err_probe(dev, ret,
> > +						     "Failed to set voltage output for %pfwP\n",
> > +						     child);
> > +
> > +			chan_type = IIO_VOLTAGE;
> > +			break;
> > +		case CH_FUNC_CURRENT_OUTPUT:
> > +			chan_type = IIO_CURRENT;
> > +			break;
> > +		default:
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "Invalid adi,ch-func %u for %pfwP\n",
> > +					     ch_func, child);
> > +		}
> > +
> > +		channels[i] = ad5710r_channels[reg];
> > +		channels[i].type = chan_type;
> > +		i++;
> > +
> > +		reg_offset = reg / AD3530R_CH_PER_REG;
> > +		ch_in_reg = reg % AD3530R_CH_PER_REG;
> > +		mode_reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> > +		mode_mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> > +
> > +		/* Enable the channel in normal operation mode */
> > +		ret = regmap_update_bits(st->regmap, mode_reg, mode_mask,
> > +					 field_prep(mode_mask, AD3530R_NORMAL_OP));
> > +		if (ret)
> > +			return dev_err_probe(dev, ret,
> > +					     "Failed to set normal operating mode for %pfwP\n",
> > +					     child);
> > +	}
> > +
> > +	st->channels = channels;
> > +	st->num_channels = num_chan;
> > +
> > +	return 0;
> > +}
>
> -- 
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Jonathan Cameron]

On Tue, 21 Jul 2026 16:52:10 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Jul 21, 2026 at 04:47:11PM +0800, Kim Seer Paller wrote:
> > Add device tree bindings for the Analog Devices AD5710R/AD5711R
> > 8-channel 12-/16-bit Configurable IDAC/VDAC.
> > 
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---
> >  .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++  
> 
> > +patternProperties:
> > +  "^channel@[0-7]$":
> > +    $ref: /schemas/iio/dac/dac.yaml#
> > +    type: object
> > +    description:
> > +      Represents the external channels which are connected to the DAC.
> > +
> > +    properties:
> > +      reg:
> > +        description: Channel number
> > +        items:
> > +          minimum: 0
> > +          maximum: 7
> > +
> > +      adi,ch-func:
> > +        description:
> > +          Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
> > +          output or CH_FUNC_CURRENT_OUTPUT for current output.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [1, 2]  
> 
> Please make this property a string.

It's an existing binding.  I guess it is generic enough that perhaps we
should make it a generic channel property and as part of that we can
flip over to a string.

We may need to create a top level channels.yaml to cover both ADC and DAC
channels though and reference that from both files.

I guess we don't need to do that yet though given this is a DAC only device.
The original place we added adi,ch-func is a combined ADC / DAC and 
has a lot more possible functions.

Jonathan

Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Jonathan Cameron]

On Tue, 21 Jul 2026 16:47:11 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> Add device tree bindings for the Analog Devices AD5710R/AD5711R
> 8-channel 12-/16-bit Configurable IDAC/VDAC.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 144 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
> new file mode 100644
> index 000000000000..17c1bf009242
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5710r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5710R/AD5711R 8-channel 12-/16-bit Configurable IDAC/VDAC
> +
> +maintainers:
> +  - Kim Seer Paller <kimseer.paller@analog.com>
> +
> +description: |
> +  The AD5710R (16-bit) and AD5711R (12-bit) are 8-channel, low-power,
> +  configurable current/voltage output digital-to-analog converters (DACs) with
> +  an on-chip 2.5V, 3ppm/°C reference. Each channel can be independently
> +  configured as a voltage output (0V to VREF or 0V to 2xVREF) or a current

I'd put spaces around the x

> +  output (0mA to 50mA). These devices operate from a single 2.7V to 5.5V
> +  supply and are guaranteed monotonic by design.
> +  Datasheet can be found here:
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/ad5711r-ad5710r.pdf
...

> +  adi,range-double:
> +    description:
> +      Configure the output range for all channels. If the property is present,
> +      the output will range from 0V to 2Vref. If the property is not present,

Use the 2 x Vref form for consistency.

> +      the output will range from 0V to Vref.
> +    type: boolean
> +

Re: [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R

[Jonathan Cameron]

On Tue, 21 Jul 2026 16:47:13 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> Add support for the AD5710R/AD5711R, 8-channel 16-/12-bit configurable
> IDAC/VDAC parts. They share the AD3530R register map and access model,
> so fold them into this driver.
> 
> Each channel is configured as voltage or current output from its DT
> channel@N node via adi,ch-func, building the iio_chan_spec dynamically.
> Voltage channels enable VMODE_EN and report the reference-derived scale,
> current channels report the 50 mA internal Iref scale. The powerdown
> mode is read-only and derived from the channel's configured type.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Hi,

A few minor things inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 97483534f8c3..935b3a832d8e 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c


> @@ -277,6 +313,54 @@ static ssize_t ad3532r_set_dac_powerdown(struct iio_dev *indio_dev,
>  	return len;
>  }
>  
> +static ssize_t ad5710r_get_dac_powerdown(struct iio_dev *indio_dev,
> +					 uintptr_t private,
> +					 const struct iio_chan_spec *chan,
> +					 char *buf)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_offset, ch_in_reg, reg, mode, mask;
> +	int ret;
> +
> +	reg_offset = chan->channel / AD3530R_CH_PER_REG;
> +	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
> +	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> +	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> +
> +	ret = regmap_read(st->regmap, reg, &mode);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%d\n", !!(mode & mask));

For consistency with the prep below, perhaps field_get() is appropriate here
rather than !! trickery.

> +}
> +
> +static ssize_t ad5710r_set_dac_powerdown(struct iio_dev *indio_dev,
> +					 uintptr_t private,
> +					 const struct iio_chan_spec *chan,
> +					 const char *buf, size_t len)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_offset, ch_in_reg, reg, mask, val;
> +	bool powerdown;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	reg_offset = chan->channel / AD3530R_CH_PER_REG;
> +	ch_in_reg = chan->channel % AD3530R_CH_PER_REG;
> +	reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> +	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> +	val = field_prep(mask, powerdown);
> +
> +	ret = regmap_update_bits(st->regmap, reg, mask, val);
> +	if (ret)
> +		return ret;
See below. This code is duplicated - factor it out.

> +
> +	return len;

> +static int ad3530r_parse_channel_cfg(struct ad3530r_state *st)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct iio_chan_spec *channels;
> +	int ret, num_chan;
> +	int i = 0;
> +	u32 reg;
> +
> +	num_chan = device_get_child_node_count(dev);
> +	if (!num_chan)
> +		return dev_err_probe(dev, -ENODEV, "No channels configured\n");
> +
> +	channels = devm_kcalloc(dev, num_chan, sizeof(*channels), GFP_KERNEL);
> +	if (!channels)
> +		return -ENOMEM;
> +
> +	device_for_each_child_node_scoped(dev, child) {
> +		unsigned int reg_offset, ch_in_reg, mode_reg, mode_mask, ch_func;
> +		enum iio_chan_type chan_type;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to read reg property of %pfwP\n",
> +					     child);
> +
> +		if (reg >= AD5710R_NUM_CHANNELS)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "reg out of range in %pfwP\n",
> +					     child);
> +
> +		ret = fwnode_property_read_u32(child, "adi,ch-func", &ch_func);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Missing adi,ch-func property for %pfwP\n",
> +					     child);
> +
> +		switch (ch_func) {
> +		case CH_FUNC_VOLTAGE_OUTPUT:
> +			ret = regmap_set_bits(st->regmap, AD5710R_CHN_VMODE_EN,
> +					      AD5710R_CHN_VMODE_EN_BIT(reg));
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Failed to set voltage output for %pfwP\n",
> +						     child);
> +
> +			chan_type = IIO_VOLTAGE;
> +			break;
> +		case CH_FUNC_CURRENT_OUTPUT:
> +			chan_type = IIO_CURRENT;
> +			break;
> +		default:
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid adi,ch-func %u for %pfwP\n",
> +					     ch_func, child);
> +		}
> +
> +		channels[i] = ad5710r_channels[reg];
> +		channels[i].type = chan_type;
> +		i++;
> +
> +		reg_offset = reg / AD3530R_CH_PER_REG;

This block of code is duplicating the code in the set_dac_powerdown() above.
Factor out a little helper to use in both places.

> +		ch_in_reg = reg % AD3530R_CH_PER_REG;
> +		mode_reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_offset;
> +		mode_mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
> +
> +		/* Enable the channel in normal operation mode */
> +		ret = regmap_update_bits(st->regmap, mode_reg, mode_mask,
> +					 field_prep(mode_mask, AD3530R_NORMAL_OP));
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to set normal operating mode for %pfwP\n",
> +					     child);
> +	}
> +
> +	st->channels = channels;
> +	st->num_channels = num_chan;

I don't think anything is stopping you passing in iio_dev and directly setting
iio_dev->channels and num_channels here.  That saves on having another copy of
pointer and count.

> +
> +	return 0;
> +}
> +
>  static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
>  				     const unsigned int *regs,
>  				     unsigned int num_regs,
> @@ -707,19 +943,29 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)

To assign the iio_dev channels / num_channels directly you'd need to pass it
in here, but that seems reasonable to me.

>  		st->vref_mV = range_multiplier * AD3530R_INTERNAL_VREF_mV;
>  	}
>  
> -	/* Set normal operating mode for all channels */
> -	val = FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(0), AD3530R_NORMAL_OP) |
> -	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(1), AD3530R_NORMAL_OP) |
> -	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(2), AD3530R_NORMAL_OP) |
> -	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(3), AD3530R_NORMAL_OP);
> +	if (chip_info->channel_configurable) {
> +		/* Channels and their operating mode are configured from DT */
> +		ret = ad3530r_parse_channel_cfg(st);
> +		if (ret)
> +			return ret;
> +	} else {
> +		/* Set normal operating mode for all channels */
> +		val = FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(0), AD3530R_NORMAL_OP) |
> +		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(1), AD3530R_NORMAL_OP) |
> +		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(2), AD3530R_NORMAL_OP) |
> +		      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(3), AD3530R_NORMAL_OP);
> +
> +		ret = ad3530r_write_reg_banks(st, chip_info->op_mode,
> +					      chip_info->num_op_mode_regs, val);
> +		if (ret)
> +			return ret;
>  
> -	ret = ad3530r_write_reg_banks(st, st->chip_info->op_mode,
> -				      st->chip_info->num_op_mode_regs, val);
> -	if (ret)
> -		return ret;
> +		for (unsigned int i = 0; i < chip_info->num_channels; i++)
> +			st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
>  
> -	for (unsigned int i = 0; i < st->chip_info->num_channels; i++)
> -		st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
> +		st->channels = chip_info->channels;
> +		st->num_channels = chip_info->num_channels;

As above. I would assign the iio_dev elements directly here.

> +	}
>  
>  	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_LOW);
>  	if (IS_ERR(st->ldac_gpio))
> @@ -784,8 +1030,8 @@ static int ad3530r_probe(struct spi_device *spi)
>  	indio_dev->name = st->chip_info->name;
>  	indio_dev->info = &ad3530r_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = st->chip_info->channels;
> -	indio_dev->num_channels = st->chip_info->num_channels;
> +	indio_dev->channels = st->channels;
> +	indio_dev->num_channels = st->num_channels;

Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Conor Dooley]

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

On Fri, Jul 24, 2026 at 10:57:38PM +0100, Jonathan Cameron wrote:
> On Tue, 21 Jul 2026 16:52:10 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Tue, Jul 21, 2026 at 04:47:11PM +0800, Kim Seer Paller wrote:
> > > Add device tree bindings for the Analog Devices AD5710R/AD5711R
> > > 8-channel 12-/16-bit Configurable IDAC/VDAC.
> > > 
> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > ---
> > >  .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++  
> > 
> > > +patternProperties:
> > > +  "^channel@[0-7]$":
> > > +    $ref: /schemas/iio/dac/dac.yaml#
> > > +    type: object
> > > +    description:
> > > +      Represents the external channels which are connected to the DAC.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: Channel number
> > > +        items:
> > > +          minimum: 0
> > > +          maximum: 7
> > > +
> > > +      adi,ch-func:
> > > +        description:
> > > +          Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
> > > +          output or CH_FUNC_CURRENT_OUTPUT for current output.
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        enum: [1, 2]  
> > 
> > Please make this property a string.
> 
> It's an existing binding.  I guess it is generic enough that perhaps we
> should make it a generic channel property and as part of that we can
> flip over to a string.

Ah, if it is something already established then it can stay as-is.

> We may need to create a top level channels.yaml to cover both ADC and DAC
> channels though and reference that from both files.

I mean yeah, that was one thought I had originally about the
spi-device-address property. I'm surprised there's already not more
commonality to begin with.

Just how common is some sort of "channel function" property and how
many devices are bastardising other properties like diff-channel with a
virtual channel to achieve the same result?

> 
> I guess we don't need to do that yet though given this is a DAC only device.
> The original place we added adi,ch-func is a combined ADC / DAC and 
> has a lot more possible functions.
> 
> Jonathan
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/4] iio: dac: ad3530r: parameterize DAC resolution

[Nuno Sá]

On Tue, Jul 21, 2026 at 04:47:12PM +0800, Kim Seer Paller wrote:
> Add a per-chip resolution field and use it in the raw read/write and
> scale paths instead of assuming 16 bits. Drop AD3530R_REG_VAL_MASK and
> AD3530R_DAC_MAX_VAL, which hardcoded the 16-bit width and are now
> unused.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---

You could have stated in the commit that this in preparation of a
follow up commit. Anyways:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/dac/ad3530r.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 06b7f280f61b..97483534f8c3 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c
> @@ -61,14 +61,12 @@
>  #define AD3530R_SLD_TRIG_A			BIT(7)
>  #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
>  #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
> -#define AD3530R_REG_VAL_MASK			GENMASK(15, 0)
>  #define AD3530R_OP_MODE_CHAN_MSK(chan)		(GENMASK(1, 0) << 2 * (chan))
>  
>  #define AD3530R_SW_RESET			(BIT(7) | BIT(0))
>  #define AD3530R_INTERNAL_VREF_mV		2500
>  #define AD3530R_LDAC_PULSE_US			100
>  
> -#define AD3530R_DAC_MAX_VAL			GENMASK(15, 0)
>  #define AD3530R_CH_PER_REG			4
>  #define AD3530R_CH_PER_BANK			8
>  #define AD3531R_MAX_CHANNELS			4
> @@ -99,6 +97,7 @@ struct ad3530r_chip_info {
>  	unsigned int num_channels;
>  	unsigned int num_banks;
>  	unsigned int num_op_mode_regs;
> +	unsigned int resolution;
>  	bool internal_ref_support;
>  };
>  
> @@ -310,7 +309,7 @@ static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
>  	int ret;
>  
>  	guard(mutex)(&st->lock);
> -	st->buf = cpu_to_be16(val);
> +	st->buf = cpu_to_be16(val << (16 - st->chip_info->resolution));
>  
>  	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
>  				&st->buf, sizeof(st->buf));
> @@ -340,12 +339,12 @@ static int ad3530r_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  
> -		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(st->buf));
> +		*val = be16_to_cpu(st->buf) >> (16 - st->chip_info->resolution);
>  
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = st->vref_mV;
> -		*val2 = 16;
> +		*val2 = st->chip_info->resolution;
>  
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
> @@ -361,7 +360,7 @@ static int ad3530r_write_raw(struct iio_dev *indio_dev,
>  
>  	switch (info) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val < 0 || val > AD3530R_DAC_MAX_VAL)
> +		if (val < 0 || val > (1 << st->chip_info->resolution) - 1)
>  			return -EINVAL;
>  
>  		return ad3530r_dac_write(st, chan->channel, val);
> @@ -525,6 +524,7 @@ static const struct regmap_config ad3532r_regmap_config = {
>  
>  static const struct ad3530r_chip_info ad3530_chip = {
>  	.name = "ad3530",
> +	.resolution = 16,
>  	.channels = ad3530r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3530r_channels),
> @@ -541,6 +541,7 @@ static const struct ad3530r_chip_info ad3530_chip = {
>  
>  static const struct ad3530r_chip_info ad3530r_chip = {
>  	.name = "ad3530r",
> +	.resolution = 16,
>  	.channels = ad3530r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3530r_channels),
> @@ -557,6 +558,7 @@ static const struct ad3530r_chip_info ad3530r_chip = {
>  
>  static const struct ad3530r_chip_info ad3531_chip = {
>  	.name = "ad3531",
> +	.resolution = 16,
>  	.channels = ad3531r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3531r_channels),
> @@ -573,6 +575,7 @@ static const struct ad3530r_chip_info ad3531_chip = {
>  
>  static const struct ad3530r_chip_info ad3531r_chip = {
>  	.name = "ad3531r",
> +	.resolution = 16,
>  	.channels = ad3531r_channels,
>  	.regmap_config = &ad3530r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3531r_channels),
> @@ -589,6 +592,7 @@ static const struct ad3530r_chip_info ad3531r_chip = {
>  
>  static const struct ad3530r_chip_info ad3532_chip = {
>  	.name = "ad3532",
> +	.resolution = 16,
>  	.channels = ad3532r_channels,
>  	.regmap_config = &ad3532r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3532r_channels),
> @@ -605,6 +609,7 @@ static const struct ad3530r_chip_info ad3532_chip = {
>  
>  static const struct ad3530r_chip_info ad3532r_chip = {
>  	.name = "ad3532r",
> +	.resolution = 16,
>  	.channels = ad3532r_channels,
>  	.regmap_config = &ad3532r_regmap_config,
>  	.num_channels = ARRAY_SIZE(ad3532r_channels),
> 
> -- 
> 2.34.1
>