[PATCH v4 0/3] Add driver for AD3530R and AD3531R DACs

[PATCH v4 0/3] Add driver for AD3530R and AD3531R DACs

[Kim Seer Paller]

The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
low-power, 16-bit, buffered voltage output 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.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Changes in v4:
Bindings:
- Add ad3531/ad3531r datasheet link.
- Add reviewer's tag.

ad3530r:
- Add commit description for unimplemented MUXOUT ADC monitoring feature.
- Use a DMA-safe buffer for bulk register read/write.
- Inline the AD3530R_CHAN_EXT_INFO macro for the "powerdown" attribute
  since it is only used once.
- Use enum ad3530r_mode for powerdown_mode.
- Refactor ad3530r_set_dac_powerdown() use chan->address for bitmask
  calculations and add helper variables.
- Simplify single-bit configuration with regmap_set_bits().
- Add .max_register for regmap_config.
- Rework regulator handling and move ad3530r_setup() after enabling the
  regulators.

- Link to v3: https://lore.kernel.org/r/20250403-togreg-v3-0-d4b06a4af5a9@analog.com

Changes in v3:
- Drop ABI docs.

Bindings:
- Drop reviewer's tag.
- Update commit message.
- Add non-r variants to compatible list.
- Add io-channels property to enable ADC channel support for MUXOUT
  readings.
- Switch to unevaluatedProperties: false.

ad3530r:
- Update commit message.
- Drop spi field from ad3530r_state and use regmap to retrieve the device
  pointer.
- Update mutex lock comment and use devm_mutex_init().
- Fix LDAC gpio pulse logic.
- Replace usleep_range() with fsleep().
- Use sizeof(reg_val) instead of hardcoded value in regmap_bulk_read.
- Drop reporting of zero offset.
- Add internal_ref_support chip_info parameter and modify reference
  handling.

- Link to v2: https://lore.kernel.org/r/20250324-togreg-v2-0-f211d781923e@analog.com

Changes in v2:
Bindings:
- Updated commit message.
- Changed adi,double-output-range to adi,range-double property.

ad3530r:
- Changed data type to __be16 to resolve sparse warnings related to
  type mismatches.

- Link to v1: https://lore.kernel.org/r/20250319-togreg-v1-0-d8244a502f2c@analog.com

---
Kim Seer Paller (3):
      iio: ABI: add new DAC powerdown mode
      dt-bindings: iio: dac: Add adi,ad3530r.yaml
      iio: dac: ad3530r: Add driver for AD3530R and AD3531R

 Documentation/ABI/testing/sysfs-bus-iio            |   2 +
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 100 ++++
 MAINTAINERS                                        |   8 +
 drivers/iio/dac/Kconfig                            |  11 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ad3530r.c                          | 506 +++++++++++++++++++++
 6 files changed, 628 insertions(+)
---
base-commit: 8dbeb413806f9f810d97d25284f585b201aa3bdc
change-id: 20250319-togreg-fc6a0af961ed

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

[PATCH v4 1/3] iio: ABI: add new DAC powerdown mode

[Kim Seer Paller]

Add a new powerdown mode for DACs with 7.7kohm and 32kohm resistor
to GND.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 722aa989baac43f694076074b307d134867b4533..85790f943fd858021c75d67375abbd8b2976bb8b 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -740,7 +740,9 @@ Description:
 		1kohm_to_gnd: connected to ground via an 1kOhm resistor,
 		2.5kohm_to_gnd: connected to ground via a 2.5kOhm resistor,
 		6kohm_to_gnd: connected to ground via a 6kOhm resistor,
+		7.7kohm_to_gnd: connected to ground via a 7.7kOhm resistor,
 		20kohm_to_gnd: connected to ground via a 20kOhm resistor,
+		32kohm_to_gnd: connected to ground via a 32kOhm resistor,
 		42kohm_to_gnd: connected to ground via a 42kOhm resistor,
 		90kohm_to_gnd: connected to ground via a 90kOhm resistor,
 		100kohm_to_gnd: connected to ground via an 100kOhm resistor,

-- 
2.34.1

[PATCH v4 2/3] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Kim Seer Paller]

Document the AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel)
low-power, 16-bit, buffered voltage output DACs with software-
programmable gain controls. They provide full-scale output spans of 2.5V
or 5V for reference voltages of 2.5V. These devices operate on a single
2.7V to 5.5V supply and are guaranteed to be monotonic by design.
The "R" variants include a 2.5V, 5ppm/°C internal reference, which is
disabled by default.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 100 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 107 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..a355d52a9d641e488fe291b97bc95ed115e96afd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad3530r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD3530R and Similar DACs
+
+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.
+  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
+
+properties:
+  compatible:
+    enum:
+      - adi,ad3530
+      - adi,ad3530r
+      - adi,ad3531
+      - adi,ad3531r
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  vdd-supply:
+    description: Power Supply Input.
+
+  iovdd-supply:
+    description: Digital Power Supply Input.
+
+  io-channels:
+    description:
+      ADC channel used to monitor internal die temperature, output voltages, and
+      current of a selected channel via the MUXOUT pin.
+    maxItems: 1
+
+  ref-supply:
+    description:
+      Reference Input/Output. The voltage at the REF pin sets the full-scale
+      range of all channels. If not provided the internal reference is used and
+      also provided on the VREF pin.
+
+  reset-gpios:
+    description:
+      Active low signal that is falling edge sensitive. When it is deasserted,
+      the digital core initialization is performed and all DAC registers except
+      the Interface Configuration A register are reset to their default values.
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      LDAC pin to be used as a hardware trigger to update the DAC channels. If
+      not present, the DAC channels are updated by Software LDAC.
+    maxItems: 1
+
+  adi,range-double:
+    description:
+      Configure the output range for all channels. If the property is present,
+      the output will range from 0V to 2Vref. If the property is not present,
+      the output will range from 0V to Vref.
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - iovdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        dac@0 {
+            compatible = "adi,ad3530r";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+
+            vdd-supply = <&vdd>;
+            iovdd-supply = <&iovdd>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index ffdb3f21fc4fb35b349449afbb30fecd4fe72978..9deaf2561ade5b1319cef3cb31b997a4297c0cff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1289,6 +1289,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
 F:	drivers/net/amt.c
 
+ANALOG DEVICES INC AD3530R DRIVER
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+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
+
 ANALOG DEVICES INC AD3552R DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.34.1

[PATCH v4 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[Kim Seer Paller]

The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
low-power, 16-bit, buffered voltage output 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.

Support for monitoring internal die temperature, output voltages, and
current of a selected channel via the MUXOUT pin using an external ADC
is currently not implemented.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/dac/Kconfig   |  11 +
 drivers/iio/dac/Makefile  |   1 +
 drivers/iio/dac/ad3530r.c | 506 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 519 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9deaf2561ade5b1319cef3cb31b997a4297c0cff..6e64525fadd4ab5fea20279ce6b5cd80ff4c749c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1295,6 +1295,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:	drivers/iio/dac/ad3530r.c
 
 ANALOG DEVICES INC AD3552R DRIVER
 M:	Nuno Sá <nuno.sa@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 4811ea973125a0dea1f8a9cdee1e0c045bc21981..e0996dc014a3d538ab6b4e0d50ff54ede50f1527 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -6,6 +6,17 @@
 
 menu "Digital to analog converters"
 
+config AD3530R
+	tristate "Analog Devices AD3530R and Similar DACs driver"
+	depends on SPI
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for Analog Devices AD3530R, AD3531R
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad3530r.
+
 config AD3552R_HS
 	tristate "Analog Devices AD3552R DAC High Speed driver"
 	select AD3552R_LIB
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 8dd6cce81ed1152be4cf0af9ef877b5482ceb347..3684cd52b7fa9bc0ad9f855323dcbb2e4965c404 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -4,6 +4,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_AD3530R) += ad3530r.o
 obj-$(CONFIG_AD3552R_HS) += ad3552r-hs.o
 obj-$(CONFIG_AD3552R_LIB) += ad3552r-common.o
 obj-$(CONFIG_AD3552R) += ad3552r.o
diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
new file mode 100644
index 0000000000000000000000000000000000000000..ffa04f678b86d8da6f5e47c35c265b6648121843
--- /dev/null
+++ b/drivers/iio/dac/ad3530r.c
@@ -0,0 +1,506 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
+ * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#define AD3530R_INTERFACE_CONFIG_A		0x00
+#define AD3530R_OUTPUT_OPERATING_MODE_0		0x20
+#define AD3530R_OUTPUT_OPERATING_MODE_1		0x21
+#define AD3530R_OUTPUT_CONTROL_0		0x2A
+#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 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_SW_RESET			(BIT(7) | BIT(0))
+#define AD3530R_MAX_CHANNELS			8
+#define AD3531R_MAX_CHANNELS			4
+#define AD3530R_INTERNAL_VREF_MV		2500
+#define AD3530R_LDAC_PULSE_US			100
+
+enum ad3530r_mode {
+	AD3530R_NORMAL_OPERATION,
+	AD3530R_POWERDOWN_1K,
+	AD3530R_POWERDOWN_7K7,
+	AD3530R_POWERDOWN_32K,
+};
+
+struct ad3530r_chan {
+	enum ad3530r_mode powerdown_mode;
+	bool powerdown;
+};
+
+struct ad3530r_chip_info {
+	const char *name;
+	const struct iio_chan_spec *channels;
+	int (*input_ch_reg)(unsigned int c);
+	unsigned int num_channels;
+	unsigned int sw_ldac_trig_reg;
+	bool internal_ref_support;
+};
+
+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];
+	const struct ad3530r_chip_info *chip_info;
+	struct gpio_desc *ldac_gpio;
+	int vref_mv;
+	/*
+	 * DMA (thus cache coherency maintenance) may require the transfer
+	 * buffers to live in their own cache lines.
+	 */
+	__be16 buf __aligned(IIO_DMA_MINALIGN);
+};
+
+static int ad3530r_input_ch_reg(unsigned int c)
+{
+	return 2 * c + AD3530R_INPUT_CH;
+}
+
+static int ad3531r_input_ch_reg(unsigned int c)
+{
+	return 2 * c + AD3531R_INPUT_CH;
+}
+
+static const char * const ad3530r_powerdown_modes[] = {
+	"1kohm_to_gnd",
+	"7.7kohm_to_gnd",
+	"32kohm_to_gnd",
+};
+
+static int ad3530r_get_powerdown_mode(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	return st->chan[chan->channel].powerdown_mode - 1;
+}
+
+static int ad3530r_set_powerdown_mode(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      unsigned int mode)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	st->chan[chan->channel].powerdown_mode = mode + 1;
+
+	return 0;
+}
+
+static const struct iio_enum ad3530r_powerdown_mode_enum = {
+	.items = ad3530r_powerdown_modes,
+	.num_items = ARRAY_SIZE(ad3530r_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,
+					 char *buf)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	guard(mutex)(&st->lock);
+	return sysfs_emit(buf, "%d\n", st->chan[chan->channel].powerdown);
+}
+
+static ssize_t ad3530r_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);
+	int ret;
+	unsigned int mask, val, reg;
+	bool powerdown;
+
+	ret = kstrtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+	mask = GENMASK(chan->address + 1, chan->address);
+	reg = chan->channel < AD3531R_MAX_CHANNELS ?
+	      AD3530R_OUTPUT_OPERATING_MODE_0 :
+	      AD3530R_OUTPUT_OPERATING_MODE_1;
+	val = (powerdown ? st->chan[chan->channel].powerdown_mode : 0)
+	       << chan->address;
+
+	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_hw_ldac(struct gpio_desc *ldac_gpio)
+{
+	gpiod_set_value_cansleep(ldac_gpio, 1);
+	fsleep(AD3530R_LDAC_PULSE_US);
+	gpiod_set_value_cansleep(ldac_gpio, 0);
+
+	return 0;
+}
+
+static int ad3530r_dac_write(struct ad3530r_state *st, unsigned int chan,
+			     unsigned int val)
+{
+	int ret;
+
+	guard(mutex)(&st->lock);
+	st->buf = cpu_to_be16(val);
+
+	ret = regmap_bulk_write(st->regmap, st->chip_info->input_ch_reg(chan),
+				&st->buf, sizeof(st->buf));
+	if (ret)
+		return ret;
+
+	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,
+			       AD3530R_SLD_TRIG_A);
+}
+
+static int ad3530r_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long info)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	guard(mutex)(&st->lock);
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_bulk_read(st->regmap,
+				       st->chip_info->input_ch_reg(chan->channel),
+				       &st->buf, sizeof(st->buf));
+		if (ret)
+			return ret;
+
+		*val = FIELD_GET(AD3530R_REG_VAL_MASK, be16_to_cpu(st->buf));
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_mv;
+		*val2 = 16;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad3530r_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long info)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		if (val < 0 || val > U16_MAX)
+			return -EINVAL;
+
+		return ad3530r_dac_write(st, chan->channel, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad3530r_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			      unsigned int writeval, unsigned int *readval)
+{
+	struct ad3530r_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static const struct iio_chan_spec_ext_info ad3530r_ext_info[] = {
+	{
+		.name = "powerdown",
+		.shared = IIO_SEPARATE,
+		.read = ad3530r_get_dac_powerdown,
+		.write = ad3530r_set_dac_powerdown,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SEPARATE, &ad3530r_powerdown_mode_enum),
+	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE,
+			   &ad3530r_powerdown_mode_enum),
+	{ }
+};
+
+#define AD3530R_CHAN(_chan, _pos) {					\
+	.type = IIO_VOLTAGE,						\
+	.indexed = 1,							\
+	.channel = _chan,						\
+	.address = _pos,						\
+	.output = 1,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+			      BIT(IIO_CHAN_INFO_SCALE),			\
+	.ext_info = ad3530r_ext_info,					\
+}
+
+static const struct iio_chan_spec ad3530r_channels[] = {
+	AD3530R_CHAN(0, 0),
+	AD3530R_CHAN(1, 2),
+	AD3530R_CHAN(2, 4),
+	AD3530R_CHAN(3, 6),
+	AD3530R_CHAN(4, 0),
+	AD3530R_CHAN(5, 2),
+	AD3530R_CHAN(6, 4),
+	AD3530R_CHAN(7, 6),
+};
+
+static const struct iio_chan_spec ad3531r_channels[] = {
+	AD3530R_CHAN(0, 0),
+	AD3530R_CHAN(1, 2),
+	AD3530R_CHAN(2, 4),
+	AD3530R_CHAN(3, 6),
+};
+
+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,
+	.input_ch_reg = ad3530r_input_ch_reg,
+	.internal_ref_support = false,
+};
+
+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,
+	.input_ch_reg = ad3530r_input_ch_reg,
+	.internal_ref_support = true,
+};
+
+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,
+	.input_ch_reg = ad3531r_input_ch_reg,
+	.internal_ref_support = false,
+};
+
+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,
+	.input_ch_reg = ad3531r_input_ch_reg,
+	.internal_ref_support = true,
+};
+
+static int ad3530r_setup(struct ad3530r_state *st, int vref,
+			 bool has_external_vref)
+{
+	struct device *dev = regmap_get_device(st->regmap);
+	struct gpio_desc *reset_gpio;
+	int i, ret;
+	bool has_range_multiplier;
+
+	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(reset_gpio),
+				     "Failed to get reset GPIO\n");
+
+	if (reset_gpio) {
+		/* Perform hardware reset */
+		fsleep(1000);
+		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);
+		if (ret)
+			return ret;
+	}
+
+	fsleep(10000);
+
+	has_range_multiplier = false;
+	if (device_property_present(dev, "adi,range-double")) {
+		ret = regmap_set_bits(st->regmap, AD3530R_OUTPUT_CONTROL_0,
+				      AD3530R_OUTPUT_CONTROL_RANGE);
+		if (ret)
+			return ret;
+
+		has_range_multiplier = true;
+	}
+
+	if (!has_external_vref && st->chip_info->internal_ref_support) {
+		ret = regmap_set_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
+				      AD3530R_REFERENCE_CONTROL_SEL);
+		if (ret)
+			return ret;
+
+		st->vref_mv = has_range_multiplier ?
+			      2 * AD3530R_INTERNAL_VREF_MV :
+			      AD3530R_INTERNAL_VREF_MV;
+	}
+
+	if (has_external_vref)
+		st->vref_mv = has_range_multiplier ? 2 * vref / 1000 : vref / 1000;
+
+	/* Set operating mode to normal operation. */
+	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0,
+			   AD3530R_NORMAL_OPERATION);
+	if (ret)
+		return ret;
+
+	if (st->chip_info->num_channels > AD3531R_MAX_CHANNELS) {
+		ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_1,
+				   AD3530R_NORMAL_OPERATION);
+		if (ret)
+			return ret;
+	}
+
+	for (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_HIGH);
+	if (IS_ERR(st->ldac_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
+				     "Failed to get ldac GPIO\n");
+
+	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,
+	.debugfs_reg_access = ad3530r_reg_access,
+};
+
+static int ad3530r_probe(struct spi_device *spi)
+{
+	static const char * const regulators[] = { "vdd", "iovdd" };
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ad3530r_state *st;
+	bool has_external_vref;
+	int ret, vref;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	st->regmap = devm_regmap_init_spi(spi, &ad3530r_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to init regmap");
+
+	ret = devm_mutex_init(dev, &st->lock);
+	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)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
+
+	vref = devm_regulator_get_enable_read_voltage(dev, "ref");
+	if (vref < 0 && vref != -ENODEV)
+		return vref;
+
+	has_external_vref = vref != -ENODEV;
+
+	if (!st->chip_info->internal_ref_support && !has_external_vref)
+		return vref;
+
+	ret = ad3530r_setup(st, vref, has_external_vref);
+	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);
+}
+
+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 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad3530r_id);
+
+static const struct of_device_id ad3530r_of_match[] = {
+	{ .compatible = "adi,ad3530", .data = &ad3530_chip },
+	{ .compatible = "adi,ad3530r", .data = &ad3530r_chip },
+	{ .compatible = "adi,ad3531", .data = &ad3531_chip },
+	{ .compatible = "adi,ad3531r", .data = &ad3531r_chip },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad3530r_of_match);
+
+static struct spi_driver ad3530r_driver = {
+	.driver = {
+		.name = "ad3530r",
+		.of_match_table = ad3530r_of_match,
+	},
+	.probe = ad3530r_probe,
+	.id_table = ad3530r_id,
+};
+module_spi_driver(ad3530r_driver);
+
+MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD3530R and Similar DACs Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v4 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[Jonathan Cameron]

On Sat, 12 Apr 2025 13:57:32 +0800
Kim Seer Paller <kimseer.paller@analog.com> wrote:

> The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
> low-power, 16-bit, buffered voltage output 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.
> 
> Support for monitoring internal die temperature, output voltages, and
> current of a selected channel via the MUXOUT pin using an external ADC
> is currently not implemented.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Hi,

One really small comment from me.  Otherwise this just needs
to sit on list for a little while to give other reviewers time.
If nothing else comes up I may just tweak the thing below (or leave
it alone!)

Thanks,

Jonathan

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ffa04f678b86d8da6f5e47c35c265b6648121843
> --- /dev/null
> +++ b/drivers/iio/dac/ad3530r.c
> @@ -0,0 +1,506 @@

...

> +static int ad3530r_probe(struct spi_device *spi)
> +{

...

> +	vref = devm_regulator_get_enable_read_voltage(dev, "ref");
> +	if (vref < 0 && vref != -ENODEV)
> +		return vref;
> +
> +	has_external_vref = vref != -ENODEV;
> +
> +	if (!st->chip_info->internal_ref_support && !has_external_vref)
> +		return vref;
If doing a v5 I'd go with
		return -ENODEV;
rather than having people scratch their heads to figure out what is in vref.

> +

Re: [PATCH v4 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[David Lechner]

On 4/12/25 12:57 AM, Kim Seer Paller wrote:
> The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
> low-power, 16-bit, buffered voltage output 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.
> 
> Support for monitoring internal die temperature, output voltages, and
> current of a selected channel via the MUXOUT pin using an external ADC
> is currently not implemented.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---

Looks very good now. :-)

A made a few comments but maybe nothing serious enough to require a v5.

> diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..ffa04f678b86d8da6f5e47c35c265b6648121843
> --- /dev/null
> +++ b/drivers/iio/dac/ad3530r.c
> @@ -0,0 +1,506 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
> + * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>

Usually, we try to avoid including kernel.h - it includes too much.

> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
...

> +
> +static int ad3530r_setup(struct ad3530r_state *st, int vref,
> +			 bool has_external_vref)
> +{
> +	struct device *dev = regmap_get_device(st->regmap);
> +	struct gpio_desc *reset_gpio;
> +	int i, ret;
> +	bool has_range_multiplier;
> +
> +	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(reset_gpio),
> +				     "Failed to get reset GPIO\n");
> +
> +	if (reset_gpio) {
> +		/* Perform hardware reset */
> +		fsleep(1000);
> +		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);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	fsleep(10000);
> +
> +	has_range_multiplier = false;
> +	if (device_property_present(dev, "adi,range-double")) {

Since this is a flag, I think device_property_read_bool() is preferred.

> +		ret = regmap_set_bits(st->regmap, AD3530R_OUTPUT_CONTROL_0,
> +				      AD3530R_OUTPUT_CONTROL_RANGE);
> +		if (ret)
> +			return ret;
> +
> +		has_range_multiplier = true;
> +	}
> +
> +	if (!has_external_vref && st->chip_info->internal_ref_support) {
> +		ret = regmap_set_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
> +				      AD3530R_REFERENCE_CONTROL_SEL);
> +		if (ret)
> +			return ret;
> +
> +		st->vref_mv = has_range_multiplier ?
> +			      2 * AD3530R_INTERNAL_VREF_MV :
> +			      AD3530R_INTERNAL_VREF_MV;
> +	}
> +
> +	if (has_external_vref)
> +		st->vref_mv = has_range_multiplier ? 2 * vref / 1000 : vref / 1000;
> +

I think this would be simpler as:

	st->vref_mv = range_multiplier * vref / 1000;

where range_multiplier is 1 or 2.

> +	/* Set operating mode to normal operation. */
> +	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0,
> +			   AD3530R_NORMAL_OPERATION);
> +	if (ret)
> +		return ret;
> +
> +	if (st->chip_info->num_channels > AD3531R_MAX_CHANNELS) {
> +		ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_1,
> +				   AD3530R_NORMAL_OPERATION);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (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_HIGH);

I guess it doesn't matter which state this starts in but GPIOD_OUT_LOW seems
more natural since we toggle it high the low later.

> +	if (IS_ERR(st->ldac_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
> +				     "Failed to get ldac GPIO\n");
> +
> +	return 0;
> +}
> +

Re: [PATCH v4 0/3] Add driver for AD3530R and AD3531R DACs

[David Lechner]

On 4/12/25 12:57 AM, Kim Seer Paller wrote:
> The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
> low-power, 16-bit, buffered voltage output 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.
> 
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> ---
I made a few comments on the driver, but still good enough for:

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v4 3/3] iio: dac: ad3530r: Add driver for AD3530R and AD3531R

[Jonathan Cameron]

On Wed, 16 Apr 2025 14:23:11 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 4/12/25 12:57 AM, Kim Seer Paller wrote:
> > The AD3530/AD3530R (8-channel) and AD3531/AD3531R (4-channel) are
> > low-power, 16-bit, buffered voltage output 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.
> > 
> > Support for monitoring internal die temperature, output voltages, and
> > current of a selected channel via the MUXOUT pin using an external ADC
> > is currently not implemented.
> > 
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > ---  
> 
> Looks very good now. :-)
> 
> A made a few comments but maybe nothing serious enough to require a v5.

We have plenty of time in the cycle, so I think I would prefer a v5
just tidying up these last few bits.

Thanks,

Jonathan

> 
> > diff --git a/drivers/iio/dac/ad3530r.c b/drivers/iio/dac/ad3530r.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..ffa04f678b86d8da6f5e47c35c265b6648121843
> > --- /dev/null
> > +++ b/drivers/iio/dac/ad3530r.c
> > @@ -0,0 +1,506 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * AD3530R/AD3530 8-channel, 16-bit Voltage Output DAC Driver
> > + * AD3531R/AD3531 4-channel, 16-bit Voltage Output DAC Driver
> > + *
> > + * Copyright 2025 Analog Devices Inc.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/cleanup.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/kernel.h>  
> 
> Usually, we try to avoid including kernel.h - it includes too much.
> 
> > +#include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/mutex.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/spi/spi.h>
> > +  
> ...
> 
> > +
> > +static int ad3530r_setup(struct ad3530r_state *st, int vref,
> > +			 bool has_external_vref)
> > +{
> > +	struct device *dev = regmap_get_device(st->regmap);
> > +	struct gpio_desc *reset_gpio;
> > +	int i, ret;
> > +	bool has_range_multiplier;
> > +
> > +	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(reset_gpio))
> > +		return dev_err_probe(dev, PTR_ERR(reset_gpio),
> > +				     "Failed to get reset GPIO\n");
> > +
> > +	if (reset_gpio) {
> > +		/* Perform hardware reset */
> > +		fsleep(1000);
> > +		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);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	fsleep(10000);
> > +
> > +	has_range_multiplier = false;
> > +	if (device_property_present(dev, "adi,range-double")) {  
> 
> Since this is a flag, I think device_property_read_bool() is preferred.
> 
> > +		ret = regmap_set_bits(st->regmap, AD3530R_OUTPUT_CONTROL_0,
> > +				      AD3530R_OUTPUT_CONTROL_RANGE);
> > +		if (ret)
> > +			return ret;
> > +
> > +		has_range_multiplier = true;
> > +	}
> > +
> > +	if (!has_external_vref && st->chip_info->internal_ref_support) {
> > +		ret = regmap_set_bits(st->regmap, AD3530R_REFERENCE_CONTROL_0,
> > +				      AD3530R_REFERENCE_CONTROL_SEL);
> > +		if (ret)
> > +			return ret;
> > +
> > +		st->vref_mv = has_range_multiplier ?
> > +			      2 * AD3530R_INTERNAL_VREF_MV :
> > +			      AD3530R_INTERNAL_VREF_MV;
> > +	}
> > +
> > +	if (has_external_vref)
> > +		st->vref_mv = has_range_multiplier ? 2 * vref / 1000 : vref / 1000;
> > +  
> 
> I think this would be simpler as:
> 
> 	st->vref_mv = range_multiplier * vref / 1000;
> 
> where range_multiplier is 1 or 2.
> 
> > +	/* Set operating mode to normal operation. */
> > +	ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0,
> > +			   AD3530R_NORMAL_OPERATION);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (st->chip_info->num_channels > AD3531R_MAX_CHANNELS) {
> > +		ret = regmap_write(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_1,
> > +				   AD3530R_NORMAL_OPERATION);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	for (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_HIGH);  
> 
> I guess it doesn't matter which state this starts in but GPIOD_OUT_LOW seems
> more natural since we toggle it high the low later.
> 
> > +	if (IS_ERR(st->ldac_gpio))
> > +		return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
> > +				     "Failed to get ldac GPIO\n");
> > +
> > +	return 0;
> > +}
> > +