[PATCH v4 0/2] Add support for AD5706R DAC

[PATCH v4 0/2] Add support for AD5706R DAC

[Alexis Czezar Torreno]

This series adds support for the Analog Devices AD5706R, a 4-channel
16-bit current output digital-to-analog converter with SPI interface.

The AD5706R features:
  - 4 independent current output DAC channels
  - Configurable output ranges (50mA, 150mA, 200mA, 300mA)
  - Hardware and software LDAC trigger with configurable edge selection
  - Toggle and dither modes per channel
  - Internal or external voltage reference selection
  - PWM-controlled LDAC
  - Dynamic change SPI speed

The driver exposes standard IIO raw/scale/offset channel attributes for
DAC output control, sampling frequency for PWM-based LDAC timing, and
extended attributes for device configuration including output range
selection, trigger mode, and multiplexer output.

This driver is developed and tested on the Cora Z7S platform using
the AXI SPI Engine and AXI CLKGEN IP cores. The 'clocks' property
enables dynamic SPI clock rate management via the CLKGEN.

Datasheet: https://www.analog.com/en/products/ad5706r.html

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
Changes in v4:
- dt-bindings:
  - Reverted pwm and gpio entries.
  - Added missing power supply properties
  - Clocks not added back as they were driver specific, not device
    properties
- driver:
  - Added missing includes
  - Converted to use regmap with custom SPI bus implementation.
    spi_write_then_read not applied as suggested, prevents future
    need to change SPI speed
  - removed driver speciifc mutex/guards in favor of regmap internal
    locking
  - Minor style cleanups
- Link to v3: https://lore.kernel.org/r/20260318-dev_ad5706r-v3-0-5d078f41e988@analog.com

Changes in v3:
- Added MAINTAINERS entry, files added on each patch
- dt-bindings:
  - Added allOf and ref to spi-peripheral-props.yaml
  - Changed additionalProperties to unevaluatedProperties
  - Added avdd-supply property and added it to required
- driver:
  - Removed redundant includes, added respective includes of APIs used
  - Simplified bit manipulation in SPI read/write, used feedback from v2
  - Fixed inconsistent trailing commas in device ID tables
  - Removed zero initialization in spi_device_id
- Link to v2: https://lore.kernel.org/r/20260311-dev_ad5706r-v2-0-f367063dbd1b@analog.com

Changes in v2:
- Stripped driver down to basic DAC functionality (read/write raw,
  read-only scale) as suggested.
- Removed PWM (LDAC), GPIO (reset/shutdown), clock generator,
  SPI engine frequency switching, debugfs streaming, and all
  custom ext_info sysfs attributes
- Removed regmap, IIO_BUFFER, and iio/sysfs.h dependencies
- Simplified SPI read/write to use standard spi_sync_transfer
  without clock mode logic
- Scale reports default 50mA range as read-only using
  IIO_VAL_FRACTIONAL_LOG2; writable range selection deferred
  to future follow-up series
- Simplified DT binding to only require compatible, reg, and
  spi-max-frequency
- Link to v1: https://lore.kernel.org/r/20260220-dev_ad5706r-v1-0-7253bbd74889@analog.com

---
Alexis Czezar Torreno (2):
      dt-bindings: iio: dac: Add ADI AD5706R
      iio: dac: ad5706r: Add support for AD5706R DAC

 .../devicetree/bindings/iio/dac/adi,ad5706r.yaml   | 105 +++++++++
 MAINTAINERS                                        |   8 +
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ad5706r.c                          | 244 +++++++++++++++++++++
 5 files changed, 368 insertions(+)
---
base-commit: 3674f3ca92730d9a07b42b311f1337d83c4d5605
change-id: 20260220-dev_ad5706r-2105e1dd29ab

Best regards,
-- 
Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

[PATCH v4 1/2] dt-bindings: iio: dac: Add ADI AD5706R

[Alexis Czezar Torreno]

Add device tree binding documentation for the Analog Devices
AD5706R 4-channel 16-bit current output digital-to-analog converter.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

---
Changes since v1:
  - Removed clocks, clock-names, pwms, pwm-names, gpio properties
  - Simplified example to use plain SPI bus
---
---
 .../devicetree/bindings/iio/dac/adi,ad5706r.yaml   | 105 +++++++++++++++++++++
 MAINTAINERS                                        |   7 ++
 2 files changed, 112 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..589da8371e98d3377c9ef05015e5299edf98573f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5706r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5706R 4-Channel Current Output DAC
+
+maintainers:
+  - Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
+
+description: |
+  The AD5706R is a 4-channel, 16-bit resolution, current output
+  digital-to-analog converter (DAC) with programmable output current
+  ranges (50mA, 150mA, 200mA, 300mA), an integrated 2.5V voltage
+  reference, and load DAC, A/B toggle, and dither functions.
+
+  Datasheet:
+    https://www.analog.com/en/products/ad5706r.html
+
+properties:
+  compatible:
+    enum:
+      - adi,ad5706r
+
+  reg:
+    maxItems: 1
+
+  avdd-supply:
+    description: Analog power supply (2.9V to 3.6V).
+
+  iovdd-supply:
+    description: Logic power supply (1.14V to 1.89V).
+
+  pvdd0-supply:
+    description: Power supply for IDAC0 channel (1.65V to AVDD).
+
+  pvdd1-supply:
+    description: Power supply for IDAC1 channel (1.65V to AVDD).
+
+  pvdd2-supply:
+    description: Power supply for IDAC2 channel (1.65V to AVDD).
+
+  pvdd3-supply:
+    description: Power supply for IDAC3 channel (1.65V to AVDD).
+
+  vref-supply:
+    description:
+      Optional external 2.5V voltage reference. If not provided, the
+      internal 2.5V reference is used.
+
+  pwms:
+    maxItems: 1
+    description:
+      Optional PWM connected to the LDAC/TGP/DCK pin for hardware
+      triggered DAC updates, toggle, or dither clock generation.
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the active low RESET pin. If not provided,
+      software reset is used.
+
+  out-en-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the active low OUT_EN pin. Controls whether
+      the current outputs are enabled or in high-Z/ground state.
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - iovdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad5706r";
+            reg = <0>;
+            avdd-supply = <&avdd>;
+            iovdd-supply = <&iovdd>;
+            pvdd0-supply = <&pvdd>;
+            pvdd1-supply = <&pvdd>;
+            pvdd2-supply = <&pvdd>;
+            pvdd3-supply = <&pvdd>;
+            vref-supply = <&vref>;
+            spi-max-frequency = <50000000>;
+            pwms = <&pwm0 0 1000000 0>;
+            reset-gpios = <&gpio0 10 GPIO_ACTIVE_LOW>;
+            out-en-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 1251965d70bdfa990c66966cd77f7ab52ae3385f..17a3d2d45fccb9cd3c93fd35666fb85d17d53cde 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1496,6 +1496,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4851.yaml
 F:	drivers/iio/adc/ad4851.c
 
+ANALOG DEVICES INC AD5706R DRIVER
+M:	Alexis Czezar Torreno <alexisczezar.torreno@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,ad5706r.yaml
+
 ANALOG DEVICES INC AD7091R DRIVER
 M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.34.1

Re: [PATCH v4 1/2] dt-bindings: iio: dac: Add ADI AD5706R

[Krzysztof Kozlowski]

On Wed, Apr 01, 2026 at 06:20:03PM +0800, Alexis Czezar Torreno wrote:
> +  vref-supply:
> +    description:
> +      Optional external 2.5V voltage reference. If not provided, the
> +      internal 2.5V reference is used.
> +
> +  pwms:
> +    maxItems: 1
> +    description:
> +      Optional PWM connected to the LDAC/TGP/DCK pin for hardware
> +      triggered DAC updates, toggle, or dither clock generation.
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO connected to the active low RESET pin. If not provided,
> +      software reset is used.
> +
> +  out-en-gpios:

Isn't this just enable-gpios from gpio-consumer-common? Does the device
have more enable-like GPIOs?

Best regards,
Krzysztof

RE: [PATCH v4 1/2] dt-bindings: iio: dac: Add ADI AD5706R

[Torreno, Alexis Czezar]

> > +  pwms:
> > +    maxItems: 1
> > +    description:
> > +      Optional PWM connected to the LDAC/TGP/DCK pin for hardware
> > +      triggered DAC updates, toggle, or dither clock generation.
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO connected to the active low RESET pin. If not provided,
> > +      software reset is used.
> > +
> > +  out-en-gpios:
> 
> Isn't this just enable-gpios from gpio-consumer-common? Does the device
> have more enable-like GPIOs?
> 

Ah, there's only 1 enable-like GPIO. Yeah, need to rename this to 'enable-gpios'

Regards,
Alexis

[PATCH v4 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Alexis Czezar Torreno]

Add support for the Analog Devices AD5706R, a 4-channel 16-bit
current output digital-to-analog converter with SPI interface.

Features:
  - 4 independent DAC channels
  - Hardware and software LDAC trigger
  - Configurable output range
  - PWM-based LDAC control
  - Dither and toggle modes
  - Dynamically configurable SPI speed

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

---
Changes since v1:
  - Removed PWM, GPIO, clock generator, debugfs, regmap, IIO_BUFFER
  - Removed all custom ext_info sysfs attributes
  - Simplified to basic raw read/write and read-only scale
  - SPI read/write can handle multibyte registers
---
---
 MAINTAINERS               |   1 +
 drivers/iio/dac/Kconfig   |  10 ++
 drivers/iio/dac/Makefile  |   1 +
 drivers/iio/dac/ad5706r.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 256 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 17a3d2d45fccb9cd3c93fd35666fb85d17d53cde..3d7bd98b4d1b55836e40687a9a3ac9f4935a8acb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1502,6 +1502,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
+F:	drivers/iio/dac/ad5706r.c
 
 ANALOG DEVICES INC AD7091R DRIVER
 M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index db9f5c711b3df90641f017652fbbef594cc1627d..8ccbdf6dfbca8640a47bf05b4afc6b4bf90a7e26 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -178,6 +178,16 @@ config AD5624R_SPI
 	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
 	  AD5664R converters (DAC). This driver uses the common SPI interface.
 
+config AD5706R
+	tristate "Analog Devices AD5706R DAC driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices AD5706R 4-channel,
+	  16-bit current output DAC.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5706r.
+
 config AD9739A
 	tristate "Analog Devices AD9739A RF DAC spi driver"
 	depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 2a80bbf4e80ad557da79ed916027cedff286984b..0034317984985035f7987a744899924bfd4612e3 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_AD5449) += ad5449.o
 obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
 obj-$(CONFIG_AD5592R) += ad5592r.o
 obj-$(CONFIG_AD5593R) += ad5593r.o
+obj-$(CONFIG_AD5706R) += ad5706r.o
 obj-$(CONFIG_AD5755) += ad5755.o
 obj-$(CONFIG_AD5758) += ad5758.o
 obj-$(CONFIG_AD5761) += ad5761.o
diff --git a/drivers/iio/dac/ad5706r.c b/drivers/iio/dac/ad5706r.c
new file mode 100644
index 0000000000000000000000000000000000000000..c8c7b7b966dcfef90ecbcbe91170bf84ca609179
--- /dev/null
+++ b/drivers/iio/dac/ad5706r.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5706R 16-bit Current Output Digital to Analog Converter
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+/* SPI frame layout */
+#define AD5706R_RD_MASK			BIT(15)
+#define AD5706R_ADDR_MASK		GENMASK(11, 0)
+
+/* Registers */
+#define AD5706R_REG_DAC_INPUT_A_CH(x)		(0x60 + ((x) * 2))
+#define AD5706R_REG_DAC_DATA_READBACK_CH(x)	(0x68 + ((x) * 2))
+
+#define AD5706R_DAC_RESOLUTION		16
+#define AD5706R_DAC_MAX_CODE		BIT(16)
+#define AD5706R_MULTIBYTE_REG_START	0x14
+#define AD5706R_MULTIBYTE_REG_END	0x71
+#define AD5706R_MAX_REG			0x77
+#define AD5706R_SINGLE_BYTE_LEN		1
+#define AD5706R_DOUBLE_BYTE_LEN		2
+
+struct ad5706r_state {
+	struct spi_device *spi;
+	struct regmap *regmap;
+
+	u8 tx_buf[4] __aligned(ARCH_DMA_MINALIGN);
+	u8 rx_buf[4];
+};
+
+static int ad5706r_reg_len(unsigned int reg)
+{
+	if (reg >= AD5706R_MULTIBYTE_REG_START && reg <= AD5706R_MULTIBYTE_REG_END)
+		return AD5706R_DOUBLE_BYTE_LEN;
+
+	return AD5706R_SINGLE_BYTE_LEN;
+}
+
+static int ad5706r_regmap_write(void *context, const void *data, size_t count)
+{
+	struct ad5706r_state *st = context;
+	unsigned int num_bytes;
+	u16 reg;
+
+	reg = get_unaligned_be16(data);
+	num_bytes = ad5706r_reg_len(reg);
+
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx_buf,
+		.len = num_bytes + 2,
+	};
+
+	memcpy(st->tx_buf, data, count);
+
+	/* For single byte, copy the data to the correct position */
+	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
+		st->tx_buf[2] = st->tx_buf[3];
+
+	return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
+static int ad5706r_regmap_read(void *context, const void *reg_buf,
+			       size_t reg_size, void *val_buf, size_t val_size)
+{
+	struct ad5706r_state *st = context;
+	unsigned int num_bytes;
+	u16 reg, cmd;
+	int ret;
+
+	reg = get_unaligned_be16(reg_buf);
+	num_bytes = ad5706r_reg_len(reg);
+
+	/* Full duplex, device responds immediately after command */
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx_buf,
+		.rx_buf = st->rx_buf,
+		.len = 2 + num_bytes,
+	};
+
+	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
+	put_unaligned_be16(cmd, st->tx_buf);
+	memset(st->tx_buf + 2, 0, num_bytes);
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	/* Ignore the first two bytes (echo during command) */
+	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
+		put_unaligned_be16(st->rx_buf[2], val_buf);
+	else
+		memcpy(val_buf, st->rx_buf + 2, num_bytes);
+
+	return 0;
+}
+
+static int ad5706r_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg, reg_val;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		reg = AD5706R_REG_DAC_DATA_READBACK_CH(chan->channel);
+		ret = regmap_read(st->regmap, reg, &reg_val);
+		if (ret)
+			return ret;
+
+		*val = reg_val;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 50;
+		*val2 = AD5706R_DAC_RESOLUTION;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5706r_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int val,
+			     int val2, long mask)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (!in_range(val, 0, AD5706R_DAC_MAX_CODE))
+			return -EINVAL;
+
+		reg = AD5706R_REG_DAC_INPUT_A_CH(chan->channel);
+		return regmap_write(st->regmap, reg, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct regmap_bus ad5706r_regmap_bus = {
+	.write = ad5706r_regmap_write,
+	.read = ad5706r_regmap_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static const struct regmap_config ad5706r_regmap_config = {
+	.reg_bits = 16,
+	.val_bits = 16,
+	.max_register = AD5706R_MAX_REG,
+};
+
+static const struct iio_info ad5706r_info = {
+	.read_raw = ad5706r_read_raw,
+	.write_raw = ad5706r_write_raw,
+};
+
+#define AD5706R_CHAN(_channel) {				\
+	.type = IIO_CURRENT,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),	\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+}
+
+static const struct iio_chan_spec ad5706r_channels[] = {
+	AD5706R_CHAN(0),
+	AD5706R_CHAN(1),
+	AD5706R_CHAN(2),
+	AD5706R_CHAN(3),
+};
+
+static int ad5706r_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ad5706r_state *st;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	st->regmap = devm_regmap_init(&spi->dev, &ad5706r_regmap_bus,
+				      st, &ad5706r_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
+				     "Failed to init regmap");
+
+	indio_dev->name = "ad5706r";
+	indio_dev->info = &ad5706r_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad5706r_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ad5706r_channels);
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ad5706r_of_match[] = {
+	{ .compatible = "adi,ad5706r" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5706r_of_match);
+
+static const struct spi_device_id ad5706r_id[] = {
+	{ "ad5706r" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad5706r_id);
+
+static struct spi_driver ad5706r_driver = {
+	.driver = {
+		.name = "ad5706r",
+		.of_match_table = ad5706r_of_match,
+	},
+	.probe = ad5706r_probe,
+	.id_table = ad5706r_id,
+};
+module_spi_driver(ad5706r_driver);
+
+MODULE_AUTHOR("Alexis Czezar Torreno <alexisczezar.torreno@analog.com>");
+MODULE_DESCRIPTION("AD5706R 16-bit Current Output DAC driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v4 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Andy Shevchenko]

On Wed, Apr 01, 2026 at 06:20:04PM +0800, Alexis Czezar Torreno wrote:
> Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> current output digital-to-analog converter with SPI interface.
> 
> Features:
>   - 4 independent DAC channels
>   - Hardware and software LDAC trigger
>   - Configurable output range
>   - PWM-based LDAC control
>   - Dither and toggle modes
>   - Dynamically configurable SPI speed

...

> ---
> Changes since v1:
>   - Removed PWM, GPIO, clock generator, debugfs, regmap, IIO_BUFFER
>   - Removed all custom ext_info sysfs attributes
>   - Simplified to basic raw read/write and read-only scale
>   - SPI read/write can handle multibyte registers
> ---

A bit confusing to have this changelog w/o having v3..v4 ones.

...

> +config AD5706R
> +	tristate "Analog Devices AD5706R DAC driver"
> +	depends on SPI

Shouldn't you select REGMAP?

> +	help
> +	  Say yes here to build support for Analog Devices AD5706R 4-channel,
> +	  16-bit current output DAC.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5706r.

...

> +#include <linux/array_size.h>
> +#include <linux/bits.h>

> +#include <linux/device.h>

Not used, but dev_printk.h is missing.

> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>

> +#include <linux/errno.h>

No need (in most cases) when err.h is included.

> +#include <linux/iio/iio.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>

Based on the above comments, please revisit the header block.

...

> +struct ad5706r_state {
> +	struct spi_device *spi;
> +	struct regmap *regmap;
> +
> +	u8 tx_buf[4] __aligned(ARCH_DMA_MINALIGN);

Don't we have specific IIO macro for that?

> +	u8 rx_buf[4];
> +};

...

> +static int ad5706r_regmap_write(void *context, const void *data, size_t count)
> +{
> +	struct ad5706r_state *st = context;
> +	unsigned int num_bytes;

Currently only 1 and 2 bytes are supported, right? Any updates are planned on
this in the future?

> +	u16 reg;
> +
> +	reg = get_unaligned_be16(data);
> +	num_bytes = ad5706r_reg_len(reg);
> +
> +	struct spi_transfer xfer = {
> +		.tx_buf = st->tx_buf,
> +		.len = num_bytes + 2,
> +	};
> +
> +	memcpy(st->tx_buf, data, count);
> +
> +	/* For single byte, copy the data to the correct position */
> +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> +		st->tx_buf[2] = st->tx_buf[3];
> +
> +	return spi_sync_transfer(st->spi, &xfer, 1);
> +}
> +
> +static int ad5706r_regmap_read(void *context, const void *reg_buf,
> +			       size_t reg_size, void *val_buf, size_t val_size)
> +{
> +	struct ad5706r_state *st = context;
> +	unsigned int num_bytes;
> +	u16 reg, cmd;
> +	int ret;
> +
> +	reg = get_unaligned_be16(reg_buf);
> +	num_bytes = ad5706r_reg_len(reg);
> +
> +	/* Full duplex, device responds immediately after command */
> +	struct spi_transfer xfer = {
> +		.tx_buf = st->tx_buf,
> +		.rx_buf = st->rx_buf,
> +		.len = 2 + num_bytes,
> +	};
> +
> +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> +	put_unaligned_be16(cmd, st->tx_buf);

> +	memset(st->tx_buf + 2, 0, num_bytes);

I would use &st->tx_buf[2] here and below for the sake of consistency with
put_unaligned_*().

> +	ret = spi_sync_transfer(st->spi, &xfer, 1);
> +	if (ret)
> +		return ret;
> +
> +	/* Ignore the first two bytes (echo during command) */
> +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> +		put_unaligned_be16(st->rx_buf[2], val_buf);

The comment wants to explain why it's required to put 2 bytes anyway.

> +	else
> +		memcpy(val_buf, st->rx_buf + 2, num_bytes);

However with the above question in mind, if it's all about 1 or 2 bytes, can't
we simply use the same approach everywhere, like put_unaligned_*()?

> +	return 0;
> +}

...

> +static int ad5706r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)

Better to use logical split (here and elsewhere where appropriate)

static int ad5706r_read_raw(struct iio_dev *indio_dev,
			    struct iio_chan_spec const *chan,
			    int *val, int *val2, long mask)

...

> +	st->regmap = devm_regmap_init(&spi->dev, &ad5706r_regmap_bus,
> +				      st, &ad5706r_regmap_config);

Use

	struct device *dev = &spi->dev;

at the top of the function to make this look better.

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

Missing \n.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v4 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Torreno, Alexis Czezar]

> On Wed, Apr 01, 2026 at 06:20:04PM +0800, Alexis Czezar Torreno wrote:
> > Add support for the Analog Devices AD5706R, a 4-channel 16-bit current
> > output digital-to-analog converter with SPI interface.
> >
> > Features:
> >   - 4 independent DAC channels
> >   - Hardware and software LDAC trigger
> >   - Configurable output range
> >   - PWM-based LDAC control
> >   - Dither and toggle modes
> >   - Dynamically configurable SPI speed
> 
> ...
> 
> > ---
> > Changes since v1:
> >   - Removed PWM, GPIO, clock generator, debugfs, regmap, IIO_BUFFER
> >   - Removed all custom ext_info sysfs attributes
> >   - Simplified to basic raw read/write and read-only scale
> >   - SPI read/write can handle multibyte registers
> > ---
> 
> A bit confusing to have this changelog w/o having v3..v4 ones.

Will add the others

> 
> ...
> 
> > +config AD5706R
> > +	tristate "Analog Devices AD5706R DAC driver"
> > +	depends on SPI
> 
> Shouldn't you select REGMAP?

Oh yeah, I should've. Will add

> 
> > +	help
> > +	  Say yes here to build support for Analog Devices AD5706R 4-channel,
> > +	  16-bit current output DAC.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called ad5706r.
> 
> ...
> 
> > +#include <linux/array_size.h>
> > +#include <linux/bits.h>
> 
> > +#include <linux/device.h>
> 
> Not used, but dev_printk.h is missing.
> 
> > +#include <linux/dma-mapping.h>
> > +#include <linux/err.h>
> 
> > +#include <linux/errno.h>
> 
> No need (in most cases) when err.h is included.
> 
> > +#include <linux/iio/iio.h>
> > +#include <linux/minmax.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/unaligned.h>
> 
> Based on the above comments, please revisit the header block.

Will do, thanks for the suggestions above.

> 
> ...
> 
> > +struct ad5706r_state {
> > +	struct spi_device *spi;
> > +	struct regmap *regmap;
> > +
> > +	u8 tx_buf[4] __aligned(ARCH_DMA_MINALIGN);
> 
> Don't we have specific IIO macro for that?

Ah yeah I think it was IIO_DMA_MINALIGN, will change, will also remove the
header where ARCH_DMA_MINALIGN comes from

> 
> > +static int ad5706r_regmap_write(void *context, const void *data,
> > +size_t count) {
> > +	struct ad5706r_state *st = context;
> > +	unsigned int num_bytes;
> 
> Currently only 1 and 2 bytes are supported, right? Any updates are planned on
> this in the future?

Yes only 1 and 2 bytes, no future extension. Should I make num_bytes a 'u8'?

> 
> > +	u16 reg;
> > +

...

> > +
> > +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> > +	put_unaligned_be16(cmd, st->tx_buf);
> 
> > +	memset(st->tx_buf + 2, 0, num_bytes);
> 
> I would use &st->tx_buf[2] here and below for the sake of consistency with
> put_unaligned_*().

Will edit for consistnency.

> 
> > +	ret = spi_sync_transfer(st->spi, &xfer, 1);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Ignore the first two bytes (echo during command) */
> > +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> > +		put_unaligned_be16(st->rx_buf[2], val_buf);
> 
> The comment wants to explain why it's required to put 2 bytes anyway.

Will add clearer comments for this

> 
> > +	else
> > +		memcpy(val_buf, st->rx_buf + 2, num_bytes);
> 
> However with the above question in mind, if it's all about 1 or 2 bytes, can't we
> simply use the same approach everywhere, like put_unaligned_*()?

For consistency, put_unaligned_*() can work. 

Although since rx_buf is u8, this line:

     memcpy(val_buf, &st->rx_buf[2], num_bytes);

Will look like this for 2 bytes:

    x = get_unaligned_be16( &st->rx_buf[2] )
    put_unaligned_be16( x, val_buf )

I suppose the mem* commands looks cleaner

> 
> ...
> 
> > +static int ad5706r_read_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan, int *val,
> > +			    int *val2, long mask)
> 
> Better to use logical split (here and elsewhere where appropriate)
> 
> static int ad5706r_read_raw(struct iio_dev *indio_dev,
> 			    struct iio_chan_spec const *chan,
> 			    int *val, int *val2, long mask)
> 
> ...
> 
> > +	st->regmap = devm_regmap_init(&spi->dev, &ad5706r_regmap_bus,
> > +				      st, &ad5706r_regmap_config);
> 
> Use
> 
> 	struct device *dev = &spi->dev;
> 
> at the top of the function to make this look better.
> 
> > +	if (IS_ERR(st->regmap))
> > +		return dev_err_probe(&spi->dev, PTR_ERR(st->regmap),
> > +				     "Failed to init regmap");
> 
> Missing \n.
> 

Will apply the three minor edits above

Re: [PATCH v4 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Andy Shevchenko]

On Mon, Apr 06, 2026 at 07:04:44AM +0000, Torreno, Alexis Czezar wrote:
> > On Wed, Apr 01, 2026 at 06:20:04PM +0800, Alexis Czezar Torreno wrote:

...

> > > +static int ad5706r_regmap_write(void *context, const void *data,
> > > +size_t count) {
> > > +	struct ad5706r_state *st = context;
> > > +	unsigned int num_bytes;
> > 
> > Currently only 1 and 2 bytes are supported, right? Any updates are planned on
> > this in the future?
> 
> Yes only 1 and 2 bytes, no future extension. Should I make num_bytes a 'u8'?
> 
> > > +	u16 reg;

> > > +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> > > +	put_unaligned_be16(cmd, st->tx_buf);
> > 
> > > +	memset(st->tx_buf + 2, 0, num_bytes);
> > 
> > I would use &st->tx_buf[2] here and below for the sake of consistency with
> > put_unaligned_*().
> 
> Will edit for consistnency.
> 
> > > +	ret = spi_sync_transfer(st->spi, &xfer, 1);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Ignore the first two bytes (echo during command) */
> > > +	if (num_bytes == AD5706R_SINGLE_BYTE_LEN)
> > > +		put_unaligned_be16(st->rx_buf[2], val_buf);
> > 
> > The comment wants to explain why it's required to put 2 bytes anyway.
> 
> Will add clearer comments for this
> 
> > > +	else
> > > +		memcpy(val_buf, st->rx_buf + 2, num_bytes);
> > 
> > However with the above question in mind, if it's all about 1 or 2 bytes, can't we
> > simply use the same approach everywhere, like put_unaligned_*()?
> 
> For consistency, put_unaligned_*() can work. 
> 
> Although since rx_buf is u8, this line:
> 
>      memcpy(val_buf, &st->rx_buf[2], num_bytes);
> 
> Will look like this for 2 bytes:
> 
>     x = get_unaligned_be16( &st->rx_buf[2] )
>     put_unaligned_be16( x, val_buf )
> 
> I suppose the mem* commands looks cleaner

We optimise it a bit, so what you are going to have is something like

	if (num_bytes == 1)
		x = &st->rx_buf[2];
	else if (num_bytes == 2)
		x = get_unaligned...(...);
	else
		return -EINVAL;

	put_unaligned...(...);

-- 
With Best Regards,
Andy Shevchenko