[PATCH v3 0/4] Add support for AD3532R/AD3532

[PATCH v3 0/4] Add support for AD3532R/AD3532

[Kim Seer Paller]

This series adds support for the AD3532R/AD3532, a 16-channel, 16-bit
voltage output DAC, to the existing ad3530r driver.

The AD3532R is part of the AD3530R family and shares similar
functionality (channel configuration, LDAC triggering, powerdown
control). It extends the existing ad3530r driver as the underlying
workflow remains the same. The main difference being the register
address map due to the dual-bank architecture, which is handled
by table-driven helpers introduced in this series.

The AD3532R uses a dual-bank register architecture (bank 0 at 0x1000
for channels 0-7, bank 1 at 0x3000 for channels 8-15). Per-chip
register address arrays in chip_info are iterated by bank helpers,
replacing single-register setup calls for existing variants and scaling
naturally to the AD3532R's dual-bank layout.

The series also adds AD3532R-specific powerdown modes (1kohm_to_gnd,
10kohm_to_gnd, three_state) and a new ABI entry for the 10kohm_to_gnd
powerdown mode.

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

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Changes in v3:
- Reverted the spi_device_id named initializer change from v2, to avoid
  crossing with Uwe's tree-wide SPI series.
- Reworked the Kconfig help text into an explicit per-part list.
- Removed a duplicate .input_ch_reg initialization in ad3530r_chip
  caught by Sashiko.
- Sorted AD3532R register defines by address (bank 0, then bank 1) with
  a comment about the two banks.
- Split the register/mask calculation in ad3532r_set_dac_powerdown()
  into named variables for readability.
- Link to v2: https://patch.msgid.link/20260615-iio-ad3532r-support-v2-0-84a0af8b83fa@analog.com

Changes in v2:
- Split AD3532R patch into refactor only and new device support patches.
- Add ad3530r_set_reg_bank_bits() helper for set-bits call sites.
- Use for (unsigned int i = 0; ...) in bank helpers.
- Add per-chip regmap_config to limit debugfs register space per variant.
- Switch spi_device_id to named initializers.
- Fix line wrapping in ad3532r_set_dac_powerdown().
- Link to v1: https://patch.msgid.link/20260604-iio-ad3532r-support-v1-0-c3552f9031de@analog.com

To: Nuno Sá <nuno.sa@analog.com>
To: Michael Hennerich <Michael.Hennerich@analog.com>
To: Kim Seer Paller <kimseer.paller@analog.com>
To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Andy Shevchenko <andy@kernel.org>
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@analog.com
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org

---
Kim Seer Paller (4):
      iio: dac: ad3530r: Refactor setup to table-driven register bank approach
      iio: ABI: add DAC 10kohm_to_gnd powerdown mode
      dt-bindings: iio: dac: add support for AD3532R/AD3532
      iio: dac: ad3530r: Add support for AD3532R/AD3532

 Documentation/ABI/testing/sysfs-bus-iio            |   1 +
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   |  16 +-
 drivers/iio/dac/Kconfig                            |   7 +-
 drivers/iio/dac/ad3530r.c                          | 350 ++++++++++++++++++---
 4 files changed, 331 insertions(+), 43 deletions(-)
---
base-commit: 7667a80340e99fd45357d0c90ae05813b01bbfef
change-id: 20260604-iio-ad3532r-support-759067e904e1

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

[PATCH v3 1/4] iio: dac: ad3530r: Refactor setup to table-driven register bank approach

[Kim Seer Paller]

Replace direct register calls in ad3530r_setup() with per-chip register
address arrays and bank helpers (ad3530r_set_reg_bank_bits,
ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static
register address to a function pointer for per-bank LDAC trigger
register selection. Switch spi_device_id to named initializers.

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

diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
index d9db3226ecd6..3f7c983739fc 100644
--- a/drivers/iio/dac/ad3530r.c
+++ b/drivers/iio/dac/ad3530r.c
@@ -69,8 +69,14 @@ struct ad3530r_chip_info {
 	const char *name;
 	const struct iio_chan_spec *channels;
 	int (*input_ch_reg)(unsigned int channel);
+	int (*sw_ldac_trig_reg)(unsigned int channel);
+	const unsigned int *interface_config_a;
+	const unsigned int *output_control;
+	const unsigned int *reference_control;
+	const unsigned int *op_mode;
 	unsigned int num_channels;
-	unsigned int sw_ldac_trig_reg;
+	unsigned int num_banks;
+	unsigned int num_op_mode_regs;
 	bool internal_ref_support;
 };
 
@@ -190,6 +196,16 @@ static ssize_t ad3530r_set_dac_powerdown(struct iio_dev *indio_dev,
 	return len;
 }
 
+static int ad3530r_trigger_sw_ldac_reg(unsigned int channel)
+{
+	return AD3530R_SW_LDAC_TRIG_A;
+}
+
+static int ad3531r_trigger_sw_ldac_reg(unsigned int channel)
+{
+	return AD3531R_SW_LDAC_TRIG_A;
+}
+
 static int ad3530r_trigger_hw_ldac(struct gpio_desc *ldac_gpio)
 {
 	gpiod_set_value_cansleep(ldac_gpio, 1);
@@ -215,7 +231,7 @@ static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
 	if (st->ldac_gpio)
 		return ad3530r_trigger_hw_ldac(st->ldac_gpio);
 
-	return regmap_set_bits(st->regmap, st->chip_info->sw_ldac_trig_reg,
+	return regmap_set_bits(st->regmap, st->chip_info->sw_ldac_trig_reg(chan),
 			       AD3530R_SLD_TRIG_A);
 }
 
@@ -331,12 +347,39 @@ static const struct iio_chan_spec ad3531r_channels[] = {
 	AD3530R_CHAN(3, ad3531r_ext_info),
 };
 
+static const unsigned int ad3530r_if_config[] = {
+	AD3530R_INTERFACE_CONFIG_A,
+};
+
+static const unsigned int ad3530r_out_ctrl[] = {
+	AD3530R_OUTPUT_CONTROL_0,
+};
+
+static const unsigned int ad3530r_ref_ctrl[] = {
+	AD3530R_REFERENCE_CONTROL_0,
+};
+
+static const unsigned int ad3530r_op_mode[] = {
+	AD3530R_OUTPUT_OPERATING_MODE_0,
+	AD3530R_OUTPUT_OPERATING_MODE_1,
+};
+
+static const unsigned int ad3531r_op_mode[] = {
+	AD3530R_OUTPUT_OPERATING_MODE_0,
+};
+
 static const struct ad3530r_chip_info ad3530_chip = {
 	.name = "ad3530",
 	.channels = ad3530r_channels,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
-	.sw_ldac_trig_reg = AD3530R_SW_LDAC_TRIG_A,
+	.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 = false,
 };
 
@@ -344,8 +387,14 @@ static const struct ad3530r_chip_info ad3530r_chip = {
 	.name = "ad3530r",
 	.channels = ad3530r_channels,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
-	.sw_ldac_trig_reg = AD3530R_SW_LDAC_TRIG_A,
+	.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,
 };
 
@@ -353,8 +402,14 @@ static const struct ad3530r_chip_info ad3531_chip = {
 	.name = "ad3531",
 	.channels = ad3531r_channels,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
-	.sw_ldac_trig_reg = AD3531R_SW_LDAC_TRIG_A,
+	.sw_ldac_trig_reg = ad3531r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3531r_input_ch_reg,
+	.interface_config_a = ad3530r_if_config,
+	.output_control = ad3530r_out_ctrl,
+	.reference_control = ad3530r_ref_ctrl,
+	.op_mode = ad3531r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3530r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3531r_op_mode),
 	.internal_ref_support = false,
 };
 
@@ -362,17 +417,56 @@ static const struct ad3530r_chip_info ad3531r_chip = {
 	.name = "ad3531r",
 	.channels = ad3531r_channels,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
-	.sw_ldac_trig_reg = AD3531R_SW_LDAC_TRIG_A,
+	.sw_ldac_trig_reg = ad3531r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3531r_input_ch_reg,
+	.interface_config_a = ad3530r_if_config,
+	.output_control = ad3530r_out_ctrl,
+	.reference_control = ad3530r_ref_ctrl,
+	.op_mode = ad3531r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3530r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3531r_op_mode),
 	.internal_ref_support = true,
 };
 
+static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
+				     const unsigned int *regs,
+				     unsigned int num_regs,
+				     unsigned int mask)
+{
+	int ret;
+
+	for (unsigned int i = 0; i < num_regs; i++) {
+		ret = regmap_update_bits(st->regmap, regs[i], mask, mask);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int ad3530r_write_reg_banks(const struct ad3530r_state *st,
+				   const unsigned int *regs,
+				   unsigned int num_regs,
+				   unsigned int val)
+{
+	int ret;
+
+	for (unsigned int i = 0; i < num_regs; i++) {
+		ret = regmap_write(st->regmap, regs[i], val);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 {
+	const struct ad3530r_chip_info *chip_info = st->chip_info;
 	struct device *dev = regmap_get_device(st->regmap);
 	struct gpio_desc *reset_gpio;
-	int i, ret;
 	u8 range_multiplier, val;
+	int ret;
 
 	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(reset_gpio))
@@ -385,8 +479,9 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 		gpiod_set_value_cansleep(reset_gpio, 0);
 	} else {
 		/* Perform software reset */
-		ret = regmap_update_bits(st->regmap, AD3530R_INTERFACE_CONFIG_A,
-					 AD3530R_SW_RESET, AD3530R_SW_RESET);
+		ret = ad3530r_set_reg_bank_bits(st, chip_info->interface_config_a,
+						chip_info->num_banks,
+						AD3530R_SW_RESET);
 		if (ret)
 			return ret;
 	}
@@ -395,8 +490,9 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 
 	range_multiplier = 1;
 	if (device_property_read_bool(dev, "adi,range-double")) {
-		ret = regmap_set_bits(st->regmap, AD3530R_OUTPUT_CONTROL_0,
-				      AD3530R_OUTPUT_CONTROL_RANGE);
+		ret = ad3530r_set_reg_bank_bits(st, chip_info->output_control,
+						chip_info->num_banks,
+						AD3530R_OUTPUT_CONTROL_RANGE);
 		if (ret)
 			return ret;
 
@@ -406,8 +502,9 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 	if (external_vref_uV) {
 		st->vref_mV = range_multiplier * external_vref_uV / MILLI;
 	} else {
-		ret = regmap_set_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
-				      AD3530R_REFERENCE_CONTROL_SEL);
+		ret = ad3530r_set_reg_bank_bits(st, chip_info->reference_control,
+						chip_info->num_banks,
+						AD3530R_REFERENCE_CONTROL_SEL);
 		if (ret)
 			return ret;
 
@@ -420,18 +517,12 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(2), AD3530R_NORMAL_OP) |
 	      FIELD_PREP(AD3530R_OP_MODE_CHAN_MSK(3), AD3530R_NORMAL_OP);
 
-	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0, val);
+	ret = ad3530r_write_reg_banks(st, st->chip_info->op_mode,
+				      st->chip_info->num_op_mode_regs, val);
 	if (ret)
 		return ret;
 
-	if (st->chip_info->num_channels > 4) {
-		ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_1,
-				   val);
-		if (ret)
-			return ret;
-	}
-
-	for (i = 0; i < st->chip_info->num_channels; i++)
+	for (unsigned int i = 0; i < st->chip_info->num_channels; i++)
 		st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
 
 	st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_LOW);

-- 
2.34.1

Re: [PATCH v3 1/4] iio: dac: ad3530r: Refactor setup to table-driven register bank approach

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to switch `spi_device_id` to named initializers, but this change is missing from the patch.
--

commit 54fc2d1026683255f5c87f5095c8f2ddef61fbf3
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: Refactor setup to table-driven register bank approach

This commit replaces direct register calls in ad3530r_setup() with
per-chip register address arrays and bank helpers. It also converts
sw_ldac_trig_reg to a function pointer for per-bank LDAC trigger
register selection.

> Replace direct register calls in ad3530r_setup() with per-chip register
> address arrays and bank helpers (ad3530r_set_reg_bank_bits,
> ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static
> register address to a function pointer for per-bank LDAC trigger
> register selection. Switch spi_device_id to named initializers.

[Severity: Medium]
Is this description accurate?

The commit message claims to switch spi_device_id to named initializers,
but this change is missing from the patch. Looking at ad3530r_id in
drivers/iio/dac/ad3530r.c, it still uses positional initializers:

static const struct spi_device_id ad3530r_id[] = {
	{ "ad3530", (kernel_ulong_t)&ad3530_chip },
	{ "ad3530r", (kernel_ulong_t)&ad3530r_chip },
	{ "ad3531", (kernel_ulong_t)&ad3531_chip },
	{ "ad3531r", (kernel_ulong_t)&ad3531r_chip },
	{ }
};

Was this change accidentally omitted from the patch, or should the commit
message be updated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-iio-ad3532r-support-v3-0-f6e4f4abebbe@analog.com?part=1

Re: [PATCH v3 1/4] iio: dac: ad3530r: Refactor setup to table-driven register bank approach

[Andy Shevchenko]

On Mon, Jun 29, 2026 at 04:31:04PM +0800, Kim Seer Paller wrote:
> Replace direct register calls in ad3530r_setup() with per-chip register
> address arrays and bank helpers (ad3530r_set_reg_bank_bits,
> ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static
ad3530r_set_reg_bank_bits(),
ad3530r_write_reg_banks().

> Convert sw_ldac_trig_reg from a static
> register address to a function pointer for per-bank LDAC trigger
> register selection. Switch spi_device_id to named initializers.

Split this patch to do one thing per a change.

...

> +static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
> +				     const unsigned int *regs,
> +				     unsigned int num_regs,
> +				     unsigned int mask)
> +{
> +	int ret;
> +
> +	for (unsigned int i = 0; i < num_regs; i++) {
> +		ret = regmap_update_bits(st->regmap, regs[i], mask, mask);

_set_bits()

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] iio: dac: ad3530r: Refactor setup to table-driven register bank approach

[Jonathan Cameron]

On Mon, 29 Jun 2026 16:57:26 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Jun 29, 2026 at 04:31:04PM +0800, Kim Seer Paller wrote:
> > Replace direct register calls in ad3530r_setup() with per-chip register
> > address arrays and bank helpers (ad3530r_set_reg_bank_bits,
> > ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static  
> ad3530r_set_reg_bank_bits(),
> ad3530r_write_reg_banks().
> 
> > Convert sw_ldac_trig_reg from a static
> > register address to a function pointer for per-bank LDAC trigger
> > register selection. Switch spi_device_id to named initializers.  
> 
> Split this patch to do one thing per a change.
> 
Perhaps more relevant here is remember to update the changelog.
It doesn't do that last thing as Uwe already dealt with it.

> ...
> 
> > +static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
> > +				     const unsigned int *regs,
> > +				     unsigned int num_regs,
> > +				     unsigned int mask)
> > +{
> > +	int ret;
> > +
> > +	for (unsigned int i = 0; i < num_regs; i++) {
> > +		ret = regmap_update_bits(st->regmap, regs[i], mask, mask);  
> 
> _set_bits()
> 
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}  
> 

[PATCH v3 2/4] iio: ABI: add DAC 10kohm_to_gnd powerdown mode

[Kim Seer Paller]

Add a new powerdown mode for DACs with 10kohm resistor to GND.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 4ea5598e7cd2..5cc2e82c4997 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -770,6 +770,7 @@ Description:
 		3.85kohm_to_gnd: connected to ground via a 3.85kOhm resistor,
 		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,
 		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,

-- 
2.34.1

Re: [PATCH v3 2/4] iio: ABI: add DAC 10kohm_to_gnd powerdown mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch fails to implement the spi_device_id named initializers change promised in the commit message.
--

commit 54fc2d1026683255f5c87f5095c8f2ddef61fbf3
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: Refactor setup to table-driven register bank approach

This commit replaces direct register calls in ad3530r_setup() with per-chip
register address arrays and bank helpers. It also converts sw_ldac_trig_reg
to a function pointer for per-bank LDAC trigger register selection, and states
that it switches spi_device_id to named initializers.

> Replace direct register calls in ad3530r_setup() with per-chip register
> address arrays and bank helpers (ad3530r_set_reg_bank_bits,
> ad3530r_write_reg_banks). Convert sw_ldac_trig_reg from a static
> register address to a function pointer for per-bank LDAC trigger
> register selection. Switch spi_device_id to named initializers.

[Severity: Medium]
This isn't a bug, but the commit message states it switches spi_device_id to
named initializers, yet this patch doesn't appear to change ad3530r_id[] in
drivers/iio/dac/ad3530r.c. The array still uses positional initializers:

static const struct spi_device_id ad3530r_id[] = {
	{ "ad3530", (kernel_ulong_t)&ad3530_chip },
	{ "ad3530r", (kernel_ulong_t)&ad3530r_chip },
	...
};

Was this change accidentally omitted from this patch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-iio-ad3532r-support-v3-0-f6e4f4abebbe@analog.com?part=2

[PATCH v3 3/4] dt-bindings: iio: dac: add support for AD3532R/AD3532

[Kim Seer Paller]

The AD3532R/AD3532 is a 16-channel version of the AD3530R/AD3530.
This adds compatible strings for the AD3532R/AD3532.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml         | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
index a355d52a9d64..2fe098619772 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
@@ -10,15 +10,17 @@ maintainers:
   - Kim Seer Paller <kimseer.paller@analog.com>
 
 description: |
-  The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are low-power,
-  16-bit, buffered voltage output digital-to-analog converters (DACs) with
-  software-programmable gain controls, providing full-scale output spans of 2.5V
-  or 5V for reference voltages of 2.5V. These devices operate from a single 2.7V
-  to 5.5V supply and are guaranteed monotonic by design. The "R" variants
-  include a 2.5V, 5ppm/°C internal reference, which is disabled by default.
+  The AD3530/AD3530R (8-channel), AD3531/AD3531R (4-channel), and AD3532/AD3532R
+  (16-channel) are low-power, 16-bit, buffered voltage output digital-to-analog
+  converters (DACs) with software-programmable gain controls, providing
+  full-scale output spans of 2.5V or 5V for reference voltages of 2.5V. These
+  devices operate from a single 2.7V to 5.5V supply and are guaranteed monotonic
+  by design. The "R" variants include a 2.5V, 5ppm/°C internal reference, which
+  is disabled by default.
   Datasheet can be found here:
   https://www.analog.com/media/en/technical-documentation/data-sheets/ad3530_ad530r.pdf
   https://www.analog.com/media/en/technical-documentation/data-sheets/ad3531-ad3531r.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad3532r.pdf
 
 properties:
   compatible:
@@ -27,6 +29,8 @@ properties:
       - adi,ad3530r
       - adi,ad3531
       - adi,ad3531r
+      - adi,ad3532
+      - adi,ad3532r
 
   reg:
     maxItems: 1

-- 
2.34.1

Re: [PATCH v3 3/4] dt-bindings: iio: dac: add support for AD3532R/AD3532

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message for the refactoring patch claims to 'Switch spi_device_id to named initializers', but this change was never actually implemented in the code.
--

commit 2f85a102e878e53764e40f42a40c71a43b98d329
Author: Kim Seer Paller <kimseer.paller@analog.com>

dt-bindings: iio: dac: add support for AD3532R/AD3532

This commit adds support for the AD3532 and AD3532R 16-channel DACs.
It updates the device tree binding description and adds the corresponding
compatible strings for the new variants.

> dt-bindings: iio: dac: add support for AD3532R/AD3532
>
> The AD3532R/AD3532 is a 16-channel version of the AD3530R/AD3530.
> This adds compatible strings for the AD3532R/AD3532.

[Severity: Medium]
Are we missing the change to switch spi_device_id to named initializers?

The commit message for the refactoring patch claims to 'Switch spi_device_id
to named initializers', but this change was never actually implemented in the
code.

Looking at ad3530r_id in drivers/iio/dac/ad3530r.c, the array remains
populated with positional initializers with type casts:

static const struct spi_device_id ad3530r_id[] = {
	{ "ad3530", (kernel_ulong_t)&ad3530_chip },
	{ "ad3530r", (kernel_ulong_t)&ad3530r_chip },

Is there a missing patch in this series where the named initializers are
actually applied?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-iio-ad3532r-support-v3-0-f6e4f4abebbe@analog.com?part=3

[PATCH v3 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Kim Seer Paller]

The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
dual-bank register architecture (bank 0 at 0x1000 for channels 0-7,
bank 1 at 0x3000 for channels 8-15). It shares similar functionality
with AD3530R (channel configuration, LDAC triggering, powerdown control),
the main difference being the register address map due to the dual-bank
architecture, handled by table-driven helpers.

Add AD3532R-specific register definitions, channel specs, per-bank
register arrays, a dedicated ad3532r_set_dac_powerdown(), and per-chip
regmap_config to limit debugfs-exposed register space to each variant's
actual address range.

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

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..4ec5bf5bf877 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -11,8 +11,11 @@ config AD3530R
 	depends on SPI
 	select REGMAP_SPI
 	help
-	  Say yes here to build support for Analog Devices AD3530R, AD3531R
-	  Digital to Analog Converter.
+	  Say yes here to build support for the following Analog Devices
+	  Digital to Analog Converters:
+	  - AD3530/AD3530R (8-channel)
+	  - AD3531/AD3531R (4-channel)
+	  - AD3532/AD3532R (16-channel)
 
 	  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 3f7c983739fc..8e17e738d4c8 100644
--- a/drivers/iio/dac/ad3530r.c
+++ b/drivers/iio/dac/ad3530r.c
@@ -2,6 +2,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
  *
  * Copyright 2025 Analog Devices Inc.
  */
@@ -39,6 +40,23 @@
 #define AD3531R_SW_LDAC_TRIG_A			0xDD
 #define AD3531R_INPUT_CH			0xE3
 
+/* AD3532R/AD3532 has two register banks: bank 0 at 0x10xx, bank 1 at 0x30xx */
+#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
+#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
+#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
+#define AD3532R_OUTPUT_CONTROL_0		0x102A
+#define AD3532R_REFERENCE_CONTROL_0		0x103C
+#define AD3532R_SW_LDAC_TRIG_0			0x10E5
+#define AD3532R_INPUT_CH_0			0x10EB
+#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
+#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
+#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
+#define AD3532R_OUTPUT_CONTROL_1		0x302A
+#define AD3532R_REFERENCE_CONTROL_1		0x303C
+#define AD3532R_SW_LDAC_TRIG_1			0x30E5
+#define AD3532R_INPUT_CH_1			0x30EB
+#define AD3532R_MAX_REG_ADDR			0x30F9
+
 #define AD3530R_SLD_TRIG_A			BIT(7)
 #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
 #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
@@ -50,8 +68,10 @@
 #define AD3530R_LDAC_PULSE_US			100
 
 #define AD3530R_DAC_MAX_VAL			GENMASK(15, 0)
-#define AD3530R_MAX_CHANNELS			8
+#define AD3530R_CH_PER_REG			4
+#define AD3530R_CH_PER_BANK			8
 #define AD3531R_MAX_CHANNELS			4
+#define AD3532R_MAX_CHANNELS			16
 
 enum ad3530r_mode {
 	AD3530R_NORMAL_OP,
@@ -68,6 +88,7 @@ struct ad3530r_chan {
 struct ad3530r_chip_info {
 	const char *name;
 	const struct iio_chan_spec *channels;
+	const struct regmap_config *regmap_config;
 	int (*input_ch_reg)(unsigned int channel);
 	int (*sw_ldac_trig_reg)(unsigned int channel);
 	const unsigned int *interface_config_a;
@@ -84,7 +105,7 @@ struct ad3530r_state {
 	struct regmap *regmap;
 	/* lock to protect against multiple access to the device and shared data */
 	struct mutex lock;
-	struct ad3530r_chan chan[AD3530R_MAX_CHANNELS];
+	struct ad3530r_chan chan[AD3532R_MAX_CHANNELS];
 	const struct ad3530r_chip_info *chip_info;
 	struct gpio_desc *ldac_gpio;
 	int vref_mV;
@@ -105,6 +126,14 @@ static int ad3531r_input_ch_reg(unsigned int channel)
 	return 2 * channel + AD3531R_INPUT_CH;
 }
 
+static int ad3532r_input_ch_reg(unsigned int channel)
+{
+	if (channel < 8)
+		return 2 * channel + AD3532R_INPUT_CH_0;
+
+	return 2 * (channel - 8) + AD3532R_INPUT_CH_1;
+}
+
 static const char * const ad3530r_powerdown_modes[] = {
 	"1kohm_to_gnd",
 	"7.7kohm_to_gnd",
@@ -117,6 +146,12 @@ static const char * const ad3531r_powerdown_modes[] = {
 	"16kohm_to_gnd",
 };
 
+static const char * const ad3532r_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"10kohm_to_gnd",
+	"three_state",
+};
+
 static int ad3530r_get_powerdown_mode(struct iio_dev *indio_dev,
 				      const struct iio_chan_spec *chan)
 {
@@ -152,6 +187,13 @@ static const struct iio_enum ad3531r_powerdown_mode_enum = {
 	.set = ad3530r_set_powerdown_mode,
 };
 
+static const struct iio_enum ad3532r_powerdown_mode_enum = {
+	.items = ad3532r_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad3532r_powerdown_modes),
+	.get = ad3530r_get_powerdown_mode,
+	.set = ad3530r_set_powerdown_mode,
+};
+
 static ssize_t ad3530r_get_dac_powerdown(struct iio_dev *indio_dev,
 					 uintptr_t private,
 					 const struct iio_chan_spec *chan,
@@ -196,6 +238,45 @@ static ssize_t ad3530r_set_dac_powerdown(struct iio_dev *indio_dev,
 	return len;
 }
 
+static ssize_t ad3532r_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 bank, local_ch, reg_in_bank, ch_in_reg;
+	unsigned int reg, pdmode, mask, val;
+	bool powerdown;
+	int ret;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	bank = chan->channel / AD3530R_CH_PER_BANK;
+	local_ch = chan->channel % AD3530R_CH_PER_BANK;
+	reg_in_bank = local_ch / AD3530R_CH_PER_REG;
+	ch_in_reg = local_ch % AD3530R_CH_PER_REG;
+
+	reg = bank ? AD3532R_OUTPUT_OPERATING_MODE_2 :
+		     AD3532R_OUTPUT_OPERATING_MODE_0;
+	reg += reg_in_bank;
+	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);
+
+	pdmode = powerdown ? st->chan[chan->channel].powerdown_mode : 0;
+	val = field_prep(mask, pdmode);
+
+	ret = regmap_update_bits(st->regmap, reg, mask, val);
+	if (ret)
+		return ret;
+
+	st->chan[chan->channel].powerdown = powerdown;
+
+	return len;
+}
+
 static int ad3530r_trigger_sw_ldac_reg(unsigned int channel)
 {
 	return AD3530R_SW_LDAC_TRIG_A;
@@ -206,6 +287,14 @@ static int ad3531r_trigger_sw_ldac_reg(unsigned int channel)
 	return AD3531R_SW_LDAC_TRIG_A;
 }
 
+static int ad3532r_trigger_sw_ldac_reg(unsigned int channel)
+{
+	if (channel < 8)
+		return AD3532R_SW_LDAC_TRIG_0;
+
+	return AD3532R_SW_LDAC_TRIG_1;
+}
+
 static int ad3530r_trigger_hw_ldac(struct gpio_desc *ldac_gpio)
 {
 	gpiod_set_value_cansleep(ldac_gpio, 1);
@@ -318,6 +407,19 @@ static const struct iio_chan_spec_ext_info ad3531r_ext_info[] = {
 	{ }
 };
 
+static const struct iio_chan_spec_ext_info ad3532r_ext_info[] = {
+	{
+		.name = "powerdown",
+		.shared = IIO_SEPARATE,
+		.read = ad3530r_get_dac_powerdown,
+		.write = ad3532r_set_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad3532r_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
+			   &ad3532r_powerdown_mode_enum),
+	{ }
+};
+
 #define AD3530R_CHAN(_chan, _ext_info)				\
 {								\
 	.type = IIO_VOLTAGE,					\
@@ -347,6 +449,25 @@ static const struct iio_chan_spec ad3531r_channels[] = {
 	AD3530R_CHAN(3, ad3531r_ext_info),
 };
 
+static const struct iio_chan_spec ad3532r_channels[] = {
+	AD3530R_CHAN(0, ad3532r_ext_info),
+	AD3530R_CHAN(1, ad3532r_ext_info),
+	AD3530R_CHAN(2, ad3532r_ext_info),
+	AD3530R_CHAN(3, ad3532r_ext_info),
+	AD3530R_CHAN(4, ad3532r_ext_info),
+	AD3530R_CHAN(5, ad3532r_ext_info),
+	AD3530R_CHAN(6, ad3532r_ext_info),
+	AD3530R_CHAN(7, ad3532r_ext_info),
+	AD3530R_CHAN(8, ad3532r_ext_info),
+	AD3530R_CHAN(9, ad3532r_ext_info),
+	AD3530R_CHAN(10, ad3532r_ext_info),
+	AD3530R_CHAN(11, ad3532r_ext_info),
+	AD3530R_CHAN(12, ad3532r_ext_info),
+	AD3530R_CHAN(13, ad3532r_ext_info),
+	AD3530R_CHAN(14, ad3532r_ext_info),
+	AD3530R_CHAN(15, ad3532r_ext_info),
+};
+
 static const unsigned int ad3530r_if_config[] = {
 	AD3530R_INTERFACE_CONFIG_A,
 };
@@ -368,9 +489,44 @@ static const unsigned int ad3531r_op_mode[] = {
 	AD3530R_OUTPUT_OPERATING_MODE_0,
 };
 
+static const unsigned int ad3532r_if_config[] = {
+	AD3532R_INTERFACE_CONFIG_A_0,
+	AD3532R_INTERFACE_CONFIG_A_1,
+};
+
+static const unsigned int ad3532r_out_ctrl[] = {
+	AD3532R_OUTPUT_CONTROL_0,
+	AD3532R_OUTPUT_CONTROL_1,
+};
+
+static const unsigned int ad3532r_ref_ctrl[] = {
+	AD3532R_REFERENCE_CONTROL_0,
+	AD3532R_REFERENCE_CONTROL_1,
+};
+
+static const unsigned int ad3532r_op_mode[] = {
+	AD3532R_OUTPUT_OPERATING_MODE_0,
+	AD3532R_OUTPUT_OPERATING_MODE_1,
+	AD3532R_OUTPUT_OPERATING_MODE_2,
+	AD3532R_OUTPUT_OPERATING_MODE_3,
+};
+
+static const struct regmap_config ad3530r_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = AD3530R_MAX_REG_ADDR,
+};
+
+static const struct regmap_config ad3532r_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = AD3532R_MAX_REG_ADDR,
+};
+
 static const struct ad3530r_chip_info ad3530_chip = {
 	.name = "ad3530",
 	.channels = ad3530r_channels,
+	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
 	.sw_ldac_trig_reg = ad3530r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3530r_input_ch_reg,
@@ -386,6 +542,7 @@ static const struct ad3530r_chip_info ad3530_chip = {
 static const struct ad3530r_chip_info ad3530r_chip = {
 	.name = "ad3530r",
 	.channels = ad3530r_channels,
+	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3530r_channels),
 	.sw_ldac_trig_reg = ad3530r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3530r_input_ch_reg,
@@ -401,6 +558,7 @@ static const struct ad3530r_chip_info ad3530r_chip = {
 static const struct ad3530r_chip_info ad3531_chip = {
 	.name = "ad3531",
 	.channels = ad3531r_channels,
+	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
 	.sw_ldac_trig_reg = ad3531r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3531r_input_ch_reg,
@@ -416,6 +574,7 @@ static const struct ad3530r_chip_info ad3531_chip = {
 static const struct ad3530r_chip_info ad3531r_chip = {
 	.name = "ad3531r",
 	.channels = ad3531r_channels,
+	.regmap_config = &ad3530r_regmap_config,
 	.num_channels = ARRAY_SIZE(ad3531r_channels),
 	.sw_ldac_trig_reg = ad3531r_trigger_sw_ldac_reg,
 	.input_ch_reg = ad3531r_input_ch_reg,
@@ -428,6 +587,38 @@ static const struct ad3530r_chip_info ad3531r_chip = {
 	.internal_ref_support = true,
 };
 
+static const struct ad3530r_chip_info ad3532_chip = {
+	.name = "ad3532",
+	.channels = ad3532r_channels,
+	.regmap_config = &ad3532r_regmap_config,
+	.num_channels = ARRAY_SIZE(ad3532r_channels),
+	.sw_ldac_trig_reg = ad3532r_trigger_sw_ldac_reg,
+	.input_ch_reg = ad3532r_input_ch_reg,
+	.interface_config_a = ad3532r_if_config,
+	.output_control = ad3532r_out_ctrl,
+	.reference_control = ad3532r_ref_ctrl,
+	.op_mode = ad3532r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3532r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3532r_op_mode),
+	.internal_ref_support = false,
+};
+
+static const struct ad3530r_chip_info ad3532r_chip = {
+	.name = "ad3532r",
+	.channels = ad3532r_channels,
+	.regmap_config = &ad3532r_regmap_config,
+	.num_channels = ARRAY_SIZE(ad3532r_channels),
+	.sw_ldac_trig_reg = ad3532r_trigger_sw_ldac_reg,
+	.input_ch_reg = ad3532r_input_ch_reg,
+	.interface_config_a = ad3532r_if_config,
+	.output_control = ad3532r_out_ctrl,
+	.reference_control = ad3532r_ref_ctrl,
+	.op_mode = ad3532r_op_mode,
+	.num_banks = ARRAY_SIZE(ad3532r_if_config),
+	.num_op_mode_regs = ARRAY_SIZE(ad3532r_op_mode),
+	.internal_ref_support = true,
+};
+
 static int ad3530r_set_reg_bank_bits(const struct ad3530r_state *st,
 				     const unsigned int *regs,
 				     unsigned int num_regs,
@@ -533,12 +724,6 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 	return 0;
 }
 
-static const struct regmap_config ad3530r_regmap_config = {
-	.reg_bits = 16,
-	.val_bits = 8,
-	.max_register = AD3530R_MAX_REG_ADDR,
-};
-
 static const struct iio_info ad3530r_info = {
 	.read_raw = ad3530r_read_raw,
 	.write_raw = ad3530r_write_raw,
@@ -559,7 +744,11 @@ static int ad3530r_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
-	st->regmap = devm_regmap_init_spi(spi, &ad3530r_regmap_config);
+	st->chip_info = spi_get_device_match_data(spi);
+	if (!st->chip_info)
+		return -ENODEV;
+
+	st->regmap = devm_regmap_init_spi(spi, st->chip_info->regmap_config);
 	if (IS_ERR(st->regmap))
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
 				     "Failed to init regmap");
@@ -568,10 +757,6 @@ static int ad3530r_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	st->chip_info = spi_get_device_match_data(spi);
-	if (!st->chip_info)
-		return -ENODEV;
-
 	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
 					     regulators);
 	if (ret)
@@ -605,6 +790,8 @@ static const struct spi_device_id ad3530r_id[] = {
 	{ "ad3530r", (kernel_ulong_t)&ad3530r_chip },
 	{ "ad3531", (kernel_ulong_t)&ad3531_chip },
 	{ "ad3531r", (kernel_ulong_t)&ad3531r_chip },
+	{ "ad3532", (kernel_ulong_t)&ad3532_chip },
+	{ "ad3532r", (kernel_ulong_t)&ad3532r_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad3530r_id);
@@ -614,6 +801,8 @@ static const struct of_device_id ad3530r_of_match[] = {
 	{ .compatible = "adi,ad3530r", .data = &ad3530r_chip },
 	{ .compatible = "adi,ad3531", .data = &ad3531_chip },
 	{ .compatible = "adi,ad3531r", .data = &ad3531r_chip },
+	{ .compatible = "adi,ad3532", .data = &ad3532_chip },
+	{ .compatible = "adi,ad3532r", .data = &ad3532r_chip },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad3530r_of_match);

-- 
2.34.1

Re: [PATCH v3 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `max_register` configuration for AD3532R is off-by-one, permanently breaking writes to the highest channel.
--

commit 471c601807c7303ea879a6246deaa17af7989f08
Author: Kim Seer Paller <kimseer.paller@analog.com>

iio: dac: ad3530r: Add support for AD3532R/AD3532

The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
dual-bank register architecture. This commit adds AD3532R-specific register
definitions, channel specs, per-bank arrays, and per-chip regmap_config.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index 3f7c983739fc7..8e17e738d4c8c 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c

[ ... ]

> @@ -39,6 +40,23 @@
>  #define AD3531R_SW_LDAC_TRIG_A			0xDD
>  #define AD3531R_INPUT_CH			0xE3
>  
> +/* AD3532R/AD3532 has two register banks: bank 0 at 0x10xx, bank 1 at 0x30xx */
> +#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
> +#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
> +#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
> +#define AD3532R_OUTPUT_CONTROL_0		0x102A
> +#define AD3532R_REFERENCE_CONTROL_0		0x103C
> +#define AD3532R_SW_LDAC_TRIG_0			0x10E5
> +#define AD3532R_INPUT_CH_0			0x10EB
> +#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
> +#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
> +#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
> +#define AD3532R_OUTPUT_CONTROL_1		0x302A
> +#define AD3532R_REFERENCE_CONTROL_1		0x303C
> +#define AD3532R_SW_LDAC_TRIG_1			0x30E5
> +#define AD3532R_INPUT_CH_1			0x30EB
> +#define AD3532R_MAX_REG_ADDR			0x30F9

[Severity: High]
Does this max register bound leave out the second byte for channel 15?

Looking at ad3532r_input_ch_reg(), the base register for channel 15 is
calculated as:
2 * (15 - 8) + AD3532R_INPUT_CH_1 (0x30EB) = 0x30F9.

Since this is a 16-bit DAC, writing a value to channel 15 requires writing
to both 0x30F9 and 0x30FA. Because ad3532r_regmap_config sets max_register
to 0x30F9, will regmap reject bulk writes to the final byte and return
-EINVAL, effectively making channel 15 unusable?

It looks like this off-by-one pattern might have been copied from the
pre-existing AD3530R_MAX_REG_ADDR configuration.

> +
>  #define AD3530R_SLD_TRIG_A			BIT(7)
>  #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
>  #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-iio-ad3532r-support-v3-0-f6e4f4abebbe@analog.com?part=4

Re: [PATCH v3 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Andy Shevchenko]

On Mon, Jun 29, 2026 at 04:31:07PM +0800, Kim Seer Paller wrote:
> The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
> dual-bank register architecture (bank 0 at 0x1000 for channels 0-7,
> bank 1 at 0x3000 for channels 8-15). It shares similar functionality
> with AD3530R (channel configuration, LDAC triggering, powerdown control),
> the main difference being the register address map due to the dual-bank
> architecture, handled by table-driven helpers.
> 
> Add AD3532R-specific register definitions, channel specs, per-bank
> register arrays, a dedicated ad3532r_set_dac_powerdown(), and per-chip
> regmap_config to limit debugfs-exposed register space to each variant's
> actual address range.

...

> +/* AD3532R/AD3532 has two register banks: bank 0 at 0x10xx, bank 1 at 0x30xx */

Split this to two comments, see below.

> +#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
> +#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
> +#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
> +#define AD3532R_OUTPUT_CONTROL_0		0x102A
> +#define AD3532R_REFERENCE_CONTROL_0		0x103C
> +#define AD3532R_SW_LDAC_TRIG_0			0x10E5
> +#define AD3532R_INPUT_CH_0			0x10EB

+ Blank line and a comment.

> +#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
> +#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
> +#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
> +#define AD3532R_OUTPUT_CONTROL_1		0x302A
> +#define AD3532R_REFERENCE_CONTROL_1		0x303C
> +#define AD3532R_SW_LDAC_TRIG_1			0x30E5
> +#define AD3532R_INPUT_CH_1			0x30EB
> +#define AD3532R_MAX_REG_ADDR			0x30F9

...

> +static int ad3532r_input_ch_reg(unsigned int channel)
> +{

Maybe

	unsigned int bank = channel / 8;
	unsigned int ch_in_reg = channel % 8;

> +	if (channel < 8)
> +		return 2 * channel + AD3532R_INPUT_CH_0;
> +
> +	return 2 * (channel - 8) + AD3532R_INPUT_CH_1;

	return 2 * ch_in_reg + (bank ? AD3532R_INPUT_CH_1 : AD3532R_INPUT_CH_0);

? This might need the correction in variable names. I tried to deduce them from
the _dac_powerdown() below. But if you think it makes things more complicated,
don't refactor.

> +}

...

> +static ssize_t ad3532r_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 bank, local_ch, reg_in_bank, ch_in_reg;
> +	unsigned int reg, pdmode, mask, val;
> +	bool powerdown;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);

May chan->channel be modified behind our back here?
If not, what's the point of protecting the below lines
(till IO)?

> +	bank = chan->channel / AD3530R_CH_PER_BANK;
> +	local_ch = chan->channel % AD3530R_CH_PER_BANK;
> +	reg_in_bank = local_ch / AD3530R_CH_PER_REG;
> +	ch_in_reg = local_ch % AD3530R_CH_PER_REG;
> +
> +	reg = bank ? AD3532R_OUTPUT_OPERATING_MODE_2 :
> +		     AD3532R_OUTPUT_OPERATING_MODE_0;
> +	reg += reg_in_bank;

	reg = reg_in_bank + bank ? AD3532R_OUTPUT_OPERATING_MODE_2 :
				   AD3532R_OUTPUT_OPERATING_MODE_0;

> +	mask = AD3530R_OP_MODE_CHAN_MSK(ch_in_reg);

> +	pdmode = powerdown ? st->chan[chan->channel].powerdown_mode : 0;
> +	val = field_prep(mask, pdmode);
> +
> +	ret = regmap_update_bits(st->regmap, reg, mask, val);

Okay, now it's cleaner and we may make it even clearer:

	if (powerdown) {
		val = field_prep(mask, st->chan[chan->channel].powerdown_mode);
// Here is the question, do we even need a field_prep()?
		ret = regmap_update_bits(st->regmap, reg, mask, val);
	} else {
		ret = regmap_clear_bits(st->regmap, reg, mask);
	}

so pdmode variable is not needed.

> +	if (ret)
> +		return ret;
> +
> +	st->chan[chan->channel].powerdown = powerdown;
> +
> +	return len;
> +}

...

> +static int ad3532r_trigger_sw_ldac_reg(unsigned int channel)
> +{
> +	if (channel < 8)
> +		return AD3532R_SW_LDAC_TRIG_0;
> +
> +	return AD3532R_SW_LDAC_TRIG_1;
> +}

Taking the above, not sure if we benefit from the parametrized macros like

#define AD3532R_SW_LDAC_TRIG(channel)	\
	(((channel) < 8) ? AD3532R_SW_LDAC_TRIG_0 : AD3532R_SW_LDAC_TRIG_1)

...

> +	st->chip_info = spi_get_device_match_data(spi);
> +	if (!st->chip_info)
> +		return -ENODEV;

> -	st->regmap = devm_regmap_init_spi(spi, &ad3530r_regmap_config);
> +	st->regmap = devm_regmap_init_spi(spi, st->chip_info->regmap_config);
>  	if (IS_ERR(st->regmap))
>  		return dev_err_probe(dev, PTR_ERR(st->regmap),
>  				     "Failed to init regmap");

> -	st->chip_info = spi_get_device_match_data(spi);
> -	if (!st->chip_info)
> -		return -ENODEV;

This is simply moved up, make it happen in a separate patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Jonathan Cameron]

On Mon, 29 Jun 2026 16:31:07 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> The AD3532R/AD3532 is a 16-channel, 16-bit voltage output DAC with a
> dual-bank register architecture (bank 0 at 0x1000 for channels 0-7,
> bank 1 at 0x3000 for channels 8-15). It shares similar functionality
> with AD3530R (channel configuration, LDAC triggering, powerdown control),
> the main difference being the register address map due to the dual-bank
> architecture, handled by table-driven helpers.
> 
> Add AD3532R-specific register definitions, channel specs, per-bank
> register arrays, a dedicated ad3532r_set_dac_powerdown(), and per-chip
> regmap_config to limit debugfs-exposed register space to each variant's
> actual address range.
The change to add the ability to provide different regmap configs
would ideally have been a precursor patch (a noop easy to review one).

Then this patch would have been just adding the new device support,
not a small refactor as well.

Otherwise, I didn't see anything to add to what Andy has called out

Thanks

Jonathan