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

[PATCH v3 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 v3:
- ad3530r: factor the op-mode register/mask computation into a helper
  shared by the powerdown access and channel parsing.
- ad3530r: pass iio_dev into setup/parse and set channels/num_channels
  directly, dropping the duplicated pointers in driver state.
- ad3530r: use regmap_test_bits() for the powerdown reads and drop the
  unused AD5710R_NUM_CHANNELS macro.
- ad3530r: split the file header comment into separate AD5710R/AD5711R lines.
- dt-bindings: drop the redundant items wrapper on channel reg and tidy
  the 2 x Vref wording.
- Link to v2: https://patch.msgid.link/20260721-iio-ad5710r-upstream-v2-0-324949dc72da@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                          | 302 +++++++++++++++++++--
 5 files changed, 430 insertions(+), 22 deletions(-)
---
base-commit: fc69c7ed31f2c36d0232a6d04750acec8f9e0c31
change-id: 20260716-iio-ad5710r-upstream-e41c298614c6

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

[PATCH v3 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 a4f5595722ad..a0f9aada5c62 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -762,6 +762,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:
@@ -774,6 +776,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,
@@ -802,6 +805,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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit documents `out_current[Y]_powerdown_mode` and `out_current[Y]_powerdown` but fails to document the corresponding `out_current[Y]_powerdown_mode_available` attributes.
--

--- Patch [1]: [PATCH v3 1/4] iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode ---
commit 3e8e143ec75a90ed9c1acf741435fcedffa2116c
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode

This commit documents the out_currentY_powerdown and
out_currentY_powerdown_mode attributes for current-output DAC channels,
and adds the 15kohm_to_gnd powerdown mode.

> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index a4f5595722ade..a0f9aada5c62d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -762,6 +762,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

[Severity: Low]
Since this commit documents out_current[Y]_powerdown_mode, should the
corresponding out_current[Y]_powerdown_mode_available attributes also be
documented?

The existing file has the available powerdown modes documented for voltage
outputs nearby:

What:		/sys/.../iio:deviceX/out_voltageY_powerdown_mode_available
What:		/sys/.../iio:deviceX/out_voltage_powerdown_mode_available
What:		/sys/.../iio:deviceX/out_altvoltageY_powerdown_mode_available
What:		/sys/.../iio:deviceX/out_altvoltage_powerdown_mode_available

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-iio-ad5710r-upstream-v3-0-3a4bf7b9c78b@analog.com?part=1

[PATCH v3 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..259866ca87cf
--- /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 2 x VREF) 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 2 x Vref. 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
+        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 3c5084fec202..bed05d0cccb7 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

Re: [PATCH v3 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 device tree binding uses a global custom boolean property (`adi,range-double`) to configure the output range for all channels simultaneously, which contradicts the hardware's per-channel capability and ignores the standard IIO properties.
- [Low] The new device tree binding is incorrectly grouped under an unrelated driver section in the MAINTAINERS file.
--

--- Patch [2]: [PATCH v3 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml ---
commit b2de4e5383bdec1581e94efdee27327a62fd9078
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..259866ca87cff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml

[ ... ]

> +  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 2 x Vref. If the property is not present,
> +      the output will range from 0V to Vref.
> +    type: boolean

[Severity: Medium]
Does this global custom property restrict the per-channel hardware capability
and ignore standard IIO range properties?

The binding description earlier in this file states "Each channel can be
independently configured as a voltage output (0V to VREF or 0V to 2 x VREF)".
However, `adi,range-double` is defined here as a global setting, which prevents
configuring voltage ranges independently for different channels.

Would it be better to move this to the per-channel `patternProperties` and
utilize the standard IIO property `output-range-microvolt` instead of a custom
boolean?

[ ... ]

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3c5084fec2023..bed05d0cccb73 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

[Severity: Low]
This isn't a bug, but is the new device tree binding incorrectly grouped
under an unrelated driver section?

The `adi,ad5710r.yaml` file appears to be appended to the
"ANALOG DEVICES INC AD3530R DRIVER" section, which could be misleading
when using standard maintainer tools.

>  F:	drivers/iio/dac/ad3530r.c
>  
>  ANALOG DEVICES INC AD3552R DRIVER

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-iio-ad5710r-upstream-v3-0-3a4bf7b9c78b@analog.com?part=2

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

[Kim Seer Paller]

In preparation for adding the AD5710R/AD5711R, which support selectable
12-/16-bit resolution, 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.

Reviewed-by: Nuno Sá <nuno.sa@analog.com>
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

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

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 02:37:13PM +0800, Kim Seer Paller wrote:
> In preparation for adding the AD5710R/AD5711R, which support selectable
> 12-/16-bit resolution, 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.

...

>  	case IIO_CHAN_INFO_RAW:
> -		if (val < 0 || val > AD3530R_DAC_MAX_VAL)
> +		if (val < 0 || val > (1 << st->chip_info->resolution) - 1)

		if (val < 0 || val > (BIT(st->chip_info->resolution) - 1))

OR

		if (val < 0 || val > GENMASK(st->chip_info->resolution - 1, 0))

One (GENMASK) works for 32, another (BIT) for 0. While it's tempting to use
BIT() for readability, the 0 is unlikely is correct, but 32 might be.

>  			return -EINVAL;

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 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 | 285 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 270 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 04c90d98f86f..83a35d35c199 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..aebe4f11771b 100644
--- a/drivers/iio/dac/ad3530r.c
+++ b/drivers/iio/dac/ad3530r.c
@@ -3,6 +3,8 @@
  * 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 8-channel, 16-bit Configurable IDAC/VDAC Driver
+ * AD5711R 8-channel, 12-bit Configurable IDAC/VDAC Driver
  *
  * Copyright 2025 Analog Devices Inc.
  */
@@ -27,6 +29,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 +43,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,9 +68,11 @@
 #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
@@ -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 {
@@ -152,6 +161,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 +208,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);
+	int ret;
+
+	ret = regmap_test_bits(st->regmap, AD5710R_CHN_VMODE_EN,
+			       AD5710R_CHN_VMODE_EN_BIT(chan->channel));
+	if (ret < 0)
+		return ret;
+
+	return !ret;
+}
+
+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 +311,61 @@ static ssize_t ad3532r_set_dac_powerdown(struct iio_dev *indio_dev,
 	return len;
 }
 
+static void ad5710r_get_op_mode_reg(unsigned int channel, unsigned int *reg,
+				    unsigned int *mask)
+{
+	unsigned int local_ch, reg_in_bank, ch_in_reg;
+
+	local_ch = channel % AD3530R_CH_PER_BANK;
+	reg_in_bank = local_ch / AD3530R_CH_PER_REG;
+	ch_in_reg = local_ch % AD3530R_CH_PER_REG;
+
+	*reg = AD3530R_OUTPUT_OPERATING_MODE_0 + reg_in_bank;
+	*mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
+}
+
+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, mask;
+	int ret;
+
+	ad5710r_get_op_mode_reg(chan->channel, &reg, &mask);
+
+	ret = regmap_test_bits(st->regmap, reg, field_prep(mask, 1));
+	if (ret < 0)
+		return ret;
+
+	return sysfs_emit(buf, "%d\n", ret);
+}
+
+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, mask;
+	bool powerdown;
+	int ret;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	ad5710r_get_op_mode_reg(chan->channel, &reg, &mask);
+
+	ret = regmap_update_bits(st->regmap, reg, mask,
+				 field_prep(mask, powerdown));
+	if (ret)
+		return ret;
+
+	return len;
+}
+
 static int ad3530r_trigger_sw_ldac_reg(unsigned int channel)
 {
 	return AD3530R_SW_LDAC_TRIG_A;
@@ -343,7 +432,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 +511,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 +570,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 +636,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 +657,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 +675,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 +693,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 +711,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 +729,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 +747,123 @@ 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 iio_dev *indio_dev)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->regmap);
+	struct iio_chan_spec *channels;
+	unsigned int num_chan, i;
+	int ret;
+	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 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 >= st->chip_info->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++;
+
+		ad5710r_get_op_mode_reg(reg, &mode_reg, &mode_mask);
+
+		/* 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);
+	}
+
+	indio_dev->channels = channels;
+	indio_dev->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,
@@ -656,8 +896,9 @@ static int ad3530r_write_reg_banks(const struct ad3530r_state *st,
 	return 0;
 }
 
-static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
+static int ad3530r_setup(struct iio_dev *indio_dev, int external_vref_uV)
 {
+	struct ad3530r_state *st = iio_priv(indio_dev);
 	const struct ad3530r_chip_info *chip_info = st->chip_info;
 	struct device *dev = regmap_get_device(st->regmap);
 	struct gpio_desc *reset_gpio;
@@ -707,19 +948,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(indio_dev);
+		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;
+		indio_dev->channels = chip_info->channels;
+		indio_dev->num_channels = chip_info->num_channels;
+	}
 
 	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_LOW);
 	if (IS_ERR(st->ldac_gpio))
@@ -777,15 +1028,13 @@ static int ad3530r_probe(struct spi_device *spi)
 	if (!st->chip_info->internal_ref_support && external_vref_uV == 0)
 		return -ENODEV;
 
-	ret = ad3530r_setup(st, external_vref_uV);
+	ret = ad3530r_setup(indio_dev, external_vref_uV);
 	if (ret)
 		return ret;
 
 	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;
 
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
@@ -797,6 +1046,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 +1059,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 v3 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] Read-only `powerdown_mode` enum is exposed as writable in sysfs, returning -EINVAL on write.
--

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

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

Adds support for the AD5710R/AD5711R 8-channel configurable IDAC/VDAC parts.
Each channel is configured as voltage or current output from its DT node,
with the powerdown mode derived from the channel's configured type.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c
[ ... ]
> @@ -419,6 +511,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]
The commit message mentions that the powerdown mode is read-only. Does using
the IIO_ENUM macro here inadvertently make the sysfs attribute writable?

The IIO_ENUM macro automatically populates both the .read and .write
callbacks (iio_enum_read and iio_enum_write). Since
ad5710r_powerdown_mode_enum lacks a .set callback, any writes from userspace
will safely fail and return -EINVAL.

Should this be manually defined as an iio_chan_spec_ext_info struct without
a .write callback to ensure the sysfs attribute is correctly exposed as
read-only?

> +	{ }
> +};
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-iio-ad5710r-upstream-v3-0-3a4bf7b9c78b@analog.com?part=4

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

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 02:37:14PM +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.

...

> +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);
> +	int ret;
> +
> +	ret = regmap_test_bits(st->regmap, AD5710R_CHN_VMODE_EN,
> +			       AD5710R_CHN_VMODE_EN_BIT(chan->channel));
> +	if (ret < 0)
> +		return ret;

> +	return !ret;

I think this '!' requires for a comment.
	/* Negate the returned value as 0 represents ..., and 1 ... */

> +}

...

> +static int ad3530r_parse_channel_cfg(struct iio_dev *indio_dev)
> +{
> +	struct ad3530r_state *st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct iio_chan_spec *channels;
> +	unsigned int num_chan, i;
> +	int ret;
> +	u32 reg;
> +
> +	num_chan = device_get_child_node_count(dev);
> +	if (!num_chan)
> +		return dev_err_probe(dev, -ENODEV, "No channels configured\n");

Optionally this can be -ENOENT, as we usually return in other APIs for
count == 0. (-ENODEV semantically seems also okay, but I just shared
an additional info for making a decision.)

> +	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 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 >= st->chip_info->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++;
> +
> +		ad5710r_get_op_mode_reg(reg, &mode_reg, &mode_mask);
> +
> +		/* 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);
> +	}
> +
> +	indio_dev->channels = channels;
> +	indio_dev->num_channels = num_chan;
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] Add support for AD5710R/AD5711R DAC

[Jonathan Cameron]

On Mon, 17 Aug 2026 14:37:10 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

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

I took another look and don't have anything to add.
If Andy hadn't left you with some choices (and I don't care either way) then
might have just tweaked them whilst applying.  Still need an Ack for the
dt-binding anyway so I can't pick them up today.  I'd suggest a v4 resolving
those opens in Andy's feedback.

Also, Sashiko got a bit confused in places on this one. I took a look and
don't think we care about any of the stuff it has raised, some of which
is incorrect anyway.

Jonathan

> ---
> Changes in v3:
> - ad3530r: factor the op-mode register/mask computation into a helper
>   shared by the powerdown access and channel parsing.
> - ad3530r: pass iio_dev into setup/parse and set channels/num_channels
>   directly, dropping the duplicated pointers in driver state.
> - ad3530r: use regmap_test_bits() for the powerdown reads and drop the
>   unused AD5710R_NUM_CHANNELS macro.
> - ad3530r: split the file header comment into separate AD5710R/AD5711R lines.
> - dt-bindings: drop the redundant items wrapper on channel reg and tidy
>   the 2 x Vref wording.
> - Link to v2: https://patch.msgid.link/20260721-iio-ad5710r-upstream-v2-0-324949dc72da@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                          | 302 +++++++++++++++++++--
>  5 files changed, 430 insertions(+), 22 deletions(-)
> ---
> base-commit: fc69c7ed31f2c36d0232a6d04750acec8f9e0c31
> change-id: 20260716-iio-ad5710r-upstream-e41c298614c6
> 
> Best regards,
> --  
> Kim Seer Paller <kimseer.paller@analog.com>
>