[PATCH 0/3] iio: dac: ad3530r: Add support for AD3532R/AD3532

[PATCH 0/3] iio: dac: ad3530r: 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 uses a dual-bank register architecture (bank 0 at 0x1000
for channels 0-7, bank 1 at 0x3000 for channels 8-15), which required
refactoring the register access paths. A table-driven register bank
approach is introduced where per-chip register address arrays in
chip_info are iterated by helper functions, replacing the 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.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Kim Seer Paller (3):
      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                            |   4 +-
 drivers/iio/dac/ad3530r.c                          | 315 +++++++++++++++++++--
 4 files changed, 302 insertions(+), 34 deletions(-)
---
base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
change-id: 20260604-iio-ad3532r-support-759067e904e1

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

[PATCH 1/3] 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 d8d6d85235b0..67446cd37183 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

[PATCH 2/3] 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.

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 2/3] dt-bindings: iio: dac: add support for AD3532R/AD3532

[Conor Dooley]

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

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

[PATCH 3/3] 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).

Introduce a table-driven register bank approach: per-chip register
address arrays in chip_info are iterated by ad3530r_update_reg_banks()
and ad3530r_write_reg_banks() helpers. This replaces the single-register
setup calls for existing variants (AD3530R, AD3531R) and scales
naturally to the AD3532R's dual-bank layout without per-variant
conditionals in the setup path.

Convert sw_ldac_trig_reg from a static register address to a function
pointer to handle the AD3532R's per-bank LDAC trigger registers.

Add AD3532R-specific powerdown modes (1kohm_to_gnd, 10kohm_to_gnd,
three_state) matching the OUTPUT_OPERATING_MODE register encoding, and
a dedicated ad3532r_set_dac_powerdown() using arithmetic channel-to-
register mapping for the 16-channel address space.

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

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..f034f7dda703 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -11,8 +11,8 @@ 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 Analog Devices AD3530/AD3530R,
+	  AD3531/AD3531R, and AD3532/AD3532R Digital to Analog Converters.
 
 	  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 d9db3226ecd6..2bdff438b2a0 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.
  */
@@ -34,11 +35,26 @@
 #define AD3530R_REFERENCE_CONTROL_0		0x3C
 #define AD3530R_SW_LDAC_TRIG_A			0xE5
 #define AD3530R_INPUT_CH			0xEB
-#define AD3530R_MAX_REG_ADDR			0xF9
 
 #define AD3531R_SW_LDAC_TRIG_A			0xDD
 #define AD3531R_INPUT_CH			0xE3
 
+#define AD3532R_INTERFACE_CONFIG_A_0		0x1000
+#define AD3532R_INTERFACE_CONFIG_A_1		0x3000
+#define AD3532R_OUTPUT_OPERATING_MODE_0		0x1020
+#define AD3532R_OUTPUT_OPERATING_MODE_1		0x1021
+#define AD3532R_OUTPUT_OPERATING_MODE_2		0x3020
+#define AD3532R_OUTPUT_OPERATING_MODE_3		0x3021
+#define AD3532R_OUTPUT_CONTROL_0		0x102A
+#define AD3532R_OUTPUT_CONTROL_1		0x302A
+#define AD3532R_REFERENCE_CONTROL_0		0x103C
+#define AD3532R_REFERENCE_CONTROL_1		0x303C
+#define AD3532R_SW_LDAC_TRIG_0			0x10E5
+#define AD3532R_SW_LDAC_TRIG_1			0x30E5
+#define AD3532R_INPUT_CH_0			0x10EB
+#define AD3532R_INPUT_CH_1			0x30EB
+#define AD3532R_MAX_REG_ADDR			0x30FA
+
 #define AD3530R_SLD_TRIG_A			BIT(7)
 #define AD3530R_OUTPUT_CONTROL_RANGE		BIT(2)
 #define AD3530R_REFERENCE_CONTROL_SEL		BIT(0)
@@ -50,8 +66,11 @@
 #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 AD3530R_MAX_CHANNELS			8
 #define AD3531R_MAX_CHANNELS			4
+#define AD3532R_MAX_CHANNELS			16
 
 enum ad3530r_mode {
 	AD3530R_NORMAL_OP,
@@ -69,8 +88,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;
 };
 
@@ -78,7 +103,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;
@@ -99,6 +124,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",
@@ -111,6 +144,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)
 {
@@ -146,6 +185,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,
@@ -163,9 +209,9 @@ static ssize_t ad3530r_set_dac_powerdown(struct iio_dev *indio_dev,
 					 const char *buf, size_t len)
 {
 	struct ad3530r_state *st = iio_priv(indio_dev);
-	int ret;
 	unsigned int reg, pdmode, mask, val;
 	bool powerdown;
+	int ret;
 
 	ret = kstrtobool(buf, &powerdown);
 	if (ret)
@@ -190,6 +236,56 @@ 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 reg, pdmode, mask, val, local_ch;
+	bool powerdown;
+	int ret;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+	local_ch = chan->channel % AD3530R_CH_PER_BANK;
+	reg = (chan->channel < AD3530R_CH_PER_BANK ? AD3532R_OUTPUT_OPERATING_MODE_0 :
+	       AD3532R_OUTPUT_OPERATING_MODE_2) + local_ch / AD3530R_CH_PER_REG;
+	mask = AD3530R_OP_MODE_CHAN_MSK(local_ch % AD3530R_CH_PER_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;
+}
+
+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);
@@ -215,7 +311,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);
 }
 
@@ -302,6 +398,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,					\
@@ -331,12 +440,80 @@ 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,
+};
+
+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 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 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 +521,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 +536,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 +551,88 @@ 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 const struct ad3530r_chip_info ad3532_chip = {
+	.name = "ad3532",
+	.channels = ad3532r_channels,
+	.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,
+	.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_update_reg_banks(const struct ad3530r_state *st,
+				    const unsigned int *regs,
+				    unsigned int num_regs,
+				    unsigned int mask, unsigned int val)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < num_regs; i++) {
+		ret = regmap_update_bits(st->regmap, regs[i], mask, val);
+		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)
+{
+	unsigned int i;
+	int ret;
+
+	for (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 i, ret;
 
 	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(reset_gpio))
@@ -384,9 +644,10 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 		fsleep(1 * USEC_PER_MSEC);
 		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_update_reg_banks(st, chip_info->interface_config_a,
+					       chip_info->num_banks,
+					       AD3530R_SW_RESET,
+					       AD3530R_SW_RESET);
 		if (ret)
 			return ret;
 	}
@@ -395,8 +656,10 @@ 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_update_reg_banks(st, chip_info->output_control,
+					       chip_info->num_banks,
+					       AD3530R_OUTPUT_CONTROL_RANGE,
+					       AD3530R_OUTPUT_CONTROL_RANGE);
 		if (ret)
 			return ret;
 
@@ -406,8 +669,10 @@ 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_update_reg_banks(st, chip_info->reference_control,
+					       chip_info->num_banks,
+					       AD3530R_REFERENCE_CONTROL_SEL,
+					       AD3530R_REFERENCE_CONTROL_SEL);
 		if (ret)
 			return ret;
 
@@ -420,17 +685,11 @@ 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++)
 		st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
 
@@ -445,7 +704,7 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
 static const struct regmap_config ad3530r_regmap_config = {
 	.reg_bits = 16,
 	.val_bits = 8,
-	.max_register = AD3530R_MAX_REG_ADDR,
+	.max_register = AD3532R_MAX_REG_ADDR,
 };
 
 static const struct iio_info ad3530r_info = {
@@ -514,6 +773,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);
@@ -523,6 +784,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 3/3] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Jonathan Cameron]

On Thu, 4 Jun 2026 15:13:45 +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).
> 
> Introduce a table-driven register bank approach: per-chip register
> address arrays in chip_info are iterated by ad3530r_update_reg_banks()
> and ad3530r_write_reg_banks() helpers. This replaces the single-register
> setup calls for existing variants (AD3530R, AD3531R) and scales
> naturally to the AD3532R's dual-bank layout without per-variant
> conditionals in the setup path.
> 
> Convert sw_ldac_trig_reg from a static register address to a function
> pointer to handle the AD3532R's per-bank LDAC trigger registers.
> 
> Add AD3532R-specific powerdown modes (1kohm_to_gnd, 10kohm_to_gnd,
> three_state) matching the OUTPUT_OPERATING_MODE register encoding, and
> a dedicated ad3532r_set_dac_powerdown() using arithmetic channel-to-
> register mapping for the 16-channel address space.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Hi

Some minor stuff inline. Biggest thing is split the patch into:
1) Patch that refactors existing code only
2) Patch that adds new device support.

Jonathan

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> index d9db3226ecd6..2bdff438b2a0 100644
> --- a/drivers/iio/dac/ad3530r.c
> +++ b/drivers/iio/dac/ad3530r.c
> @@ -2,6 +2,7 @@

>  static ssize_t ad3530r_get_dac_powerdown(struct iio_dev *indio_dev,
>  					 uintptr_t private,
>  					 const struct iio_chan_spec *chan,
> @@ -163,9 +209,9 @@ static ssize_t ad3530r_set_dac_powerdown(struct iio_dev *indio_dev,
>  					 const char *buf, size_t len)
>  {
>  	struct ad3530r_state *st = iio_priv(indio_dev);
> -	int ret;
>  	unsigned int reg, pdmode, mask, val;
>  	bool powerdown;
> +	int ret;
>  
>  	ret = kstrtobool(buf, &powerdown);
>  	if (ret)
> @@ -190,6 +236,56 @@ 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 reg, pdmode, mask, val, local_ch;
> +	bool powerdown;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +	local_ch = chan->channel % AD3530R_CH_PER_BANK;
> +	reg = (chan->channel < AD3530R_CH_PER_BANK ? AD3532R_OUTPUT_OPERATING_MODE_0 :
> +	       AD3532R_OUTPUT_OPERATING_MODE_2) + local_ch / AD3530R_CH_PER_REG;
Wrap is rather hard to read.
	reg = (chan->channel < AD3530R_CH_PER_BANK ? AD3532R_OUTPUT_OPERATING_MODE_0 :	
	       AD3532R_OUTPUT_OPERATING_MODE_2) +
	      local_ch / AD3530R_CH_PER_REG;
helps a bit

> +	mask = AD3530R_OP_MODE_CHAN_MSK(local_ch % AD3530R_CH_PER_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 const struct ad3530r_chip_info ad3532_chip = {
> +	.name = "ad3532",
> +	.channels = ad3532r_channels,
> +	.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,
> +	.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,
> +};

As mentioned below - split patch into refactors but no new parts, then
a patch just adding the new part support.

> +
> +static int ad3530r_update_reg_banks(const struct ad3530r_state *st,
> +				    const unsigned int *regs,
> +				    unsigned int num_regs,
> +				    unsigned int mask, unsigned int val)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < num_regs; i++) {
	for (unsigned int i...
> +		ret = regmap_update_bits(st->regmap, regs[i], mask, val);
> +		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)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < num_regs; i++) {
	for (unsigned int i = 0;
> +		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 i, ret;

Not in this patch. If you want to tidy up existing code ordering separate patch.

>  
>  	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>  	if (IS_ERR(reset_gpio))
> @@ -384,9 +644,10 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
>  		fsleep(1 * USEC_PER_MSEC);
>  		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_update_reg_banks(st, chip_info->interface_config_a,
> +					       chip_info->num_banks,
> +					       AD3530R_SW_RESET,
> +					       AD3530R_SW_RESET);
>  		if (ret)
>  			return ret;
>  	}
> @@ -395,8 +656,10 @@ 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_update_reg_banks(st, chip_info->output_control,
> +					       chip_info->num_banks,
> +					       AD3530R_OUTPUT_CONTROL_RANGE,
> +					       AD3530R_OUTPUT_CONTROL_RANGE);

Maybe worth a helper for 
ad3530r_set_reg_bank_bits() or something like that.

>  		if (ret)
>  			return ret;
>  
> @@ -406,8 +669,10 @@ 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_update_reg_banks(st, chip_info->reference_control,
> +					       chip_info->num_banks,
> +					       AD3530R_REFERENCE_CONTROL_SEL,
> +					       AD3530R_REFERENCE_CONTROL_SEL);
>  		if (ret)
>  			return ret;
>  
> @@ -420,17 +685,11 @@ 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)

Given there is non trivial refactoring in here to make it easier to support the new
device, the patch should be spilt.  Refactors and no new support in first patch
so that we can just verify it does the same thing, then second patch adding new
device support.

>  		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++)
>  		st->chan[i].powerdown_mode = AD3530R_POWERDOWN_32K;
>  
> @@ -445,7 +704,7 @@ static int ad3530r_setup(struct ad3530r_state *st, int external_vref_uV)
>  static const struct regmap_config ad3530r_regmap_config = {
>  	.reg_bits = 16,
>  	.val_bits = 8,
> -	.max_register = AD3530R_MAX_REG_ADDR,
> +	.max_register = AD3532R_MAX_REG_ADDR,

What happens if we read off the end (via debugfs) for the smaller parts?

>  };
>  
>  static const struct iio_info ad3530r_info = {
> @@ -514,6 +773,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 },

Add a precursor patch to switch this to named initializers. Otherwise
this will clash with the work Uwe is doing to ensure these are all done that
way.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(spi, ad3530r_id);
> @@ -523,6 +784,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);
> 

RE: [PATCH 3/3] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Paller, Kim Seer]

> > @@ -445,7 +704,7 @@ static int ad3530r_setup(struct ad3530r_state *st,
> > int external_vref_uV)  static const struct regmap_config
> ad3530r_regmap_config = {
> >  	.reg_bits = 16,
> >  	.val_bits = 8,
> > -	.max_register = AD3530R_MAX_REG_ADDR,
> > +	.max_register = AD3532R_MAX_REG_ADDR,
> 
> What happens if we read off the end (via debugfs) for the smaller parts?

I tested reading registers at 0x1000 and above on AD3531R it just
returns 0xFF and no crash. Should I add a per-chip regmap_config to limit
the exposed register space?

> >  };
> >
> >  static const struct iio_info ad3530r_info = { @@ -514,6 +773,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 },
> 
> Add a precursor patch to switch this to named initializers. Otherwise this will
> clash with the work Uwe is doing to ensure these are all done that way.
> 
> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(spi, ad3530r_id); @@ -523,6 +784,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);
> >

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

[Nuno Sá]

On Thu, Jun 11, 2026 at 07:04:37AM +0000, Paller, Kim Seer wrote:
> > > @@ -445,7 +704,7 @@ static int ad3530r_setup(struct ad3530r_state *st,
> > > int external_vref_uV)  static const struct regmap_config
> > ad3530r_regmap_config = {
> > >  	.reg_bits = 16,
> > >  	.val_bits = 8,
> > > -	.max_register = AD3530R_MAX_REG_ADDR,
> > > +	.max_register = AD3532R_MAX_REG_ADDR,
> > 
> > What happens if we read off the end (via debugfs) for the smaller parts?
> 
> I tested reading registers at 0x1000 and above on AD3531R it just
> returns 0xFF and no crash. Should I add a per-chip regmap_config to limit
> the exposed register space?

Personally, that would make sense to me.

- Nuno Sá

> 
> > >  };
> > >
> > >  static const struct iio_info ad3530r_info = { @@ -514,6 +773,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 },
> > 
> > Add a precursor patch to switch this to named initializers. Otherwise this will
> > clash with the work Uwe is doing to ensure these are all done that way.
> > 
> > >  	{ }
> > >  };
> > >  MODULE_DEVICE_TABLE(spi, ad3530r_id); @@ -523,6 +784,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);
> > >
> 

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

[Andy Shevchenko]

On Thu, Jun 11, 2026 at 01:29:53PM +0100, Nuno Sá wrote:
> On Thu, Jun 11, 2026 at 07:04:37AM +0000, Paller, Kim Seer wrote:

...

> > > > static const struct regmap_config ad3530r_regmap_config = {
> > > >  	.reg_bits = 16,
> > > >  	.val_bits = 8,
> > > > -	.max_register = AD3530R_MAX_REG_ADDR,
> > > > +	.max_register = AD3532R_MAX_REG_ADDR,
> > > 
> > > What happens if we read off the end (via debugfs) for the smaller parts?
> > 
> > I tested reading registers at 0x1000 and above on AD3531R it just
> > returns 0xFF and no crash. Should I add a per-chip regmap_config to limit
> > the exposed register space?
> 
> Personally, that would make sense to me.

And strictly speaking it's better in case of extending the driver with some
other features.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Andy Shevchenko]

On Thu, Jun 04, 2026 at 03:13:42PM +0800, Kim Seer Paller wrote:
> This series adds support for the AD3532R/AD3532, a 16-channel, 16-bit
> voltage output DAC, to the existing ad3530r driver.
> 
> The AD3532R uses a dual-bank register architecture (bank 0 at 0x1000
> for channels 0-7, bank 1 at 0x3000 for channels 8-15), which required
> refactoring the register access paths. A table-driven register bank
> approach is introduced where per-chip register address arrays in
> chip_info are iterated by helper functions, replacing the 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.

This cover letter misses two required things:
- why do we need a brand new driver (provide an explanation)?
- where to get the datasheet (provide a link)?

Just reply to this email and if new version is required do not forget
to add it to the cover letter.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH 0/3] iio: dac: ad3530r: Add support for AD3532R/AD3532

[Paller, Kim Seer]

> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Thursday, June 4, 2026 5:37 PM
> To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> Cc: Jonathan Cameron <jic23@kernel.org>; David Lechner
> <dlechner@baylibre.com>; Sa, Nuno <Nuno.Sa@analog.com>; Andy
> Shevchenko <andy@kernel.org>; Hennerich, Michael
> <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> iio@vger.kernel.org; linux-kernel@vger.kernel.org; linux <linux@analog.com>;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 0/3] iio: dac: ad3530r: Add support for AD3532R/AD3532
> 
> On Thu, Jun 04, 2026 at 03:13:42PM +0800, Kim Seer Paller wrote:
> > This series adds support for the AD3532R/AD3532, a 16-channel, 16-bit
> > voltage output DAC, to the existing ad3530r driver.
> >
> > The AD3532R uses a dual-bank register architecture (bank 0 at 0x1000
> > for channels 0-7, bank 1 at 0x3000 for channels 8-15), which required
> > refactoring the register access paths. A table-driven register bank
> > approach is introduced where per-chip register address arrays in
> > chip_info are iterated by helper functions, replacing the
> > 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.
> 
> This cover letter misses two required things:
> - why do we need a brand new driver (provide an explanation)?
> - where to get the datasheet (provide a link)?

Hi,

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.

Here's the datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad3532r.pdf

> 
> Just reply to this email and if new version is required do not forget to add it to
> the cover letter.
> 
> --
> With Best Regards,
> Andy Shevchenko
>