[PATCH 0/3] Add support for AD5706R DAC

[PATCH 0/3] 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>
---
Alexis Czezar Torreno (3):
      dt-bindings: iio: dac: Add binding for AD5706R
      iio: dac: ad5706r: Add support for AD5706R DAC
      MAINTAINERS: Add entry for AD5706R DAC driver

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

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

[PATCH 1/3] dt-bindings: iio: dac: Add binding for 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>
---
 .../devicetree/bindings/iio/dac/adi,ad5706r.yaml   | 96 ++++++++++++++++++++++
 1 file changed, 96 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..dabaf2195a07d2c66d44f69ca60e32e6bc37cf55
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
@@ -0,0 +1,96 @@
+# 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 16-bit, 4-channel current output digital-to-analog
+  converter with SPI interface.
+
+  The driver supports dynamic SPI clock rate management via an external
+  clock generator (e.g., AXI CLKGEN) referenced by the 'clocks' property.
+  This allows separate read and write SPI speeds to be configured at
+  runtime.
+
+  Datasheet:
+    https://www.analog.com/en/products/ad5706r.html
+
+properties:
+  compatible:
+    enum:
+      - adi,ad5706r
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 100000000
+
+  clocks:
+    maxItems: 1
+    description:
+      Reference clock for SPI clock rate management.
+
+  clock-names:
+    items:
+      - const: spi_clk
+
+  pwms:
+    maxItems: 1
+
+  pwm-names:
+    items:
+      - const: ad5706r_ldacb
+
+  dac-resetb-gpios:
+    maxItems: 1
+    description: GPIO connected to the active-low reset pin.
+
+  dac-shdn-gpios:
+    maxItems: 1
+    description: GPIO connected to the active-high shutdown pin.
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - clocks
+  - clock-names
+  - pwms
+  - pwm-names
+  - dac-resetb-gpios
+  - dac-shdn-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad5706r";
+            reg = <0>;
+            spi-max-frequency = <100000000>;
+
+            clocks = <&spi_clk>;
+            clock-names = "spi_clk";
+
+            dac-resetb-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
+            dac-shdn-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
+
+            pwms = <&axi_pwm_gen 0 0>;
+            pwm-names = "ad5706r_ldacb";
+        };
+    };
+...

-- 
2.34.1

[PATCH 2/3] 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>
---
 drivers/iio/dac/Kconfig   |   11 +
 drivers/iio/dac/Makefile  |    1 +
 drivers/iio/dac/ad5706r.c | 2290 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 2302 insertions(+)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index db9f5c711b3df90641f017652fbbef594cc1627d..20be74a2933049250bab779d12ecd2b9b1f5a2a7 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -178,6 +178,17 @@ 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
+	select IIO_BUFFER
+	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..2d718cf7300bcd1f599fe715aacb3170f72541af
--- /dev/null
+++ b/drivers/iio/dac/ad5706r.c
@@ -0,0 +1,2290 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5706R 16-bit Current Output Digital to Analog Converter
+ *
+ * Copyright 2026 Analog Devices Inc.
+ *
+ * This driver is designed for use with the AXI SPI Engine and AXI CLKGEN
+ * on Xilinx Zynq platforms. The 'clocks' device tree property references
+ * the AXI CLKGEN output clock, which is used to dynamically control the
+ * SPI clock rate for read and write operations independently.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/module.h>
+#include <linux/pwm.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+/* SPI Defines */
+#define AD5706R_RD_MASK		BIT(15)
+#define AD5706R_ADDR_PIN_MASK	GENMASK(14, 12)
+#define AD5706R_ADDR_MASK	GENMASK(11, 0)
+#define AD5706R_VAL_MASK	GENMASK(7, 0)
+
+/* Registers and Masks */
+#define AD5706R_MASK_RESET			(BIT(7) | BIT(0))
+#define AD5706R_MASK_DEV_ADDR(x)		((x) & GENMASK(2, 0))
+#define AD5706R_REG_INTERFACE_CONFIG_A		0x00
+#define AD5706R_MASK_INTERFACE_CONFIG_A(x)	((x) & GENMASK(7, 0))
+#define AD5706R_MASK_ADDR_ASCENSION		BIT(5)
+#define AD5706R_REG_INTERFACE_CONFIG_B		0x01
+#define AD5706R_MASK_INTERFACE_CONFIG_B(x)	((x) & GENMASK(7, 0))
+#define AD5706R_MASK_SINGLE_INSTR		BIT(7)
+#define AD5706R_REG_MULTI_DAC_SEL_CH		0x14
+#define AD5706R_MASK_MULTI_DAC_SEL_CH(x)	((x) & GENMASK(3, 0))
+#define AD5706R_REG_LDAC_SYNC_ASYNC		0x16
+#define AD5706R_MASK_LDAC_SYNC_ASYNC(x)		((x) & GENMASK(3, 0))
+#define AD5706R_REG_LDAC_HW_SW			0x18
+#define AD5706R_MASK_LDAC_HW_SW(x)		((x) & GENMASK(3, 0))
+#define AD5706R_REG_LDAC_EDGE_SEL_CH(x)		(0x1A + ((x) * 2))
+#define AD5706R_MASK_LDAC_EDGE_SEL_CH(x)	((x) & GENMASK(1, 0))
+#define AD5706R_REG_OUT_OPERATING_MODE		0x22
+#define AD5706R_MASK_OUT_OPERATING_MODE(x)      ((x) & GENMASK(3, 0))
+#define AD5706R_REG_OUT_SWITCH_EN		0x24
+#define AD5706R_MASK_OUT_SWITCH_EN(x)		((x) & GENMASK(3, 0))
+#define AD5706R_REG_SHDN_EN			0x26
+#define AD5706R_MASK_SHDN_EN(x)			((x) & GENMASK(3, 0))
+#define AD5706R_REG_OUT_RANGE_CH(x)		(0x28 + ((x) * 2))
+#define AD5706R_MASK_OUT_RANGE_CH(x)		((x) & GENMASK(1, 0))
+#define AD5706R_REG_FUNC_EN			0x30
+#define AD5706R_MASK_FUNC_EN(x)			((x) & GENMASK(3, 0))
+#define AD5706R_REG_FUNC_MODE_SEL_CH(x)		(0x32 + ((x) * 2))
+#define AD5706R_MASK_FUNC_MODE_SEL_CH(x)	((x) & BIT(0))
+#define AD5706R_REG_FUNC_DAC_INPUT_B_CH(x)	(0x3A + ((x) * 2))
+#define AD5706R_MASK_FUNC_DAC_INPUT_B_CH(x)	((x) & GENMASK(15, 0))
+#define AD5706R_REG_MUX_OUT_SEL			0x54
+#define AD5706R_MASK_MUX_OUT_SEL(x)		((x) & (BIT(7) | GENMASK(4, 0)))
+#define AD5706R_REG_MUX_OUT_CONTROL		0x56
+#define AD5706R_MASK_MUX_OUT_CONTROL(x)		((x) & BIT(0))
+#define AD5706R_REG_MULTI_DAC_SW_LDAC		0x5A
+#define AD5706R_MASK_MULTI_DAC_SW_LDAC		BIT(0)
+#define AD5706R_REG_MULTI_DAC_INPUT_A		0x5C
+#define AD5706R_MASK_MULTI_DAC_INPUT_A(x)	((x) & GENMASK(15, 0))
+#define AD5706R_REG_DAC_SW_LDAC			0x5E
+#define AD5706R_MASK_DAC_SW_LDAC(x)		((x) & GENMASK(3, 0))
+#define AD5706R_REG_DAC_INPUT_A_CH(x)		(0x60 + ((x) * 2))
+#define AD5706R_MASK_DAC_INPUT_A_CH(x)		((x) & GENMASK(15, 0))
+#define AD5706R_REG_DAC_DATA_READBACK_CH(x)	(0x68 + ((x) * 2))
+#define AD5706R_MASK_DAC_DATA_READBACK_CH(x)	((x) & GENMASK(15, 0))
+#define AD5706R_REG_BANDGAP_CONTROL		0x73
+#define AD5706R_MASK_BANDGAP_CONTROL		BIT(0)
+
+#define NUM_CHANNELS			4
+#define SPI_MAX_SPEED_HZ		(100 * HZ_PER_MHZ)	/* 100 MHz */
+#define SPI_MIN_SPEED_HZ		(3 * HZ_PER_MHZ)	/* 3 MHz */
+#define SAMPLING_FREQUENCY_MIN_HZ	1
+#define SAMPLING_FREQUENCY_MAX_HZ	10000000
+#define AD5706R_DAC_RESOLUTION		16
+#define AD5706R_DAC_MAX_CODE		BIT(AD5706R_DAC_RESOLUTION)  /* 65536 */
+#define AD5706R_MULTIBYTE_REG_START	0x14
+#define AD5706R_MULTIBYTE_REG_END	0x71
+#define AD5706R_SINGLE_BYTE_LEN		1
+#define AD5706R_DOUBLE_BYTE_LEN		2
+
+enum set_clk_mode_values {
+	CLK_MODE_UNKNOWN = 0,
+	CLK_MODE_CLKGEN = 1,
+	CLK_MODE_SPI_ENGINE = 2,
+};
+
+/*
+ * Order of attributes in code:
+ *
+ * Device Attributes:
+ *   - dev_addr
+ *   - addr_ascension
+ *   - single_instr
+ *   - hw_ldac_tg_state
+ *   - sampling_frequency
+ *   - hw_ldac_tg_pwm
+ *   - mux_out_sel
+ *   - multi_dac_input_a
+ *   - multi_dac_sw_ldac_trigger
+ *   - reference_volts
+ *   - ref_select
+ *   - hw_shutdown_state
+ *
+ * Channel Attributes:
+ *   - raw
+ *   - scale
+ *   - offset
+ *   - input_register_a
+ *   - input_register_b
+ *   - hw_active_edge
+ *   - range_sel
+ *   - output_state
+ *   - ldac_trigger_chn
+ *   - toggle_trigger_chn
+ *   - dither_trigger_chn
+ *   - multi_dac_sel_ch
+ */
+
+struct ad5706r_state {
+	struct spi_device *spi;
+	/* Mutex lock for clock transitions and device access */
+	struct mutex lock;
+
+	__be32 tx_buf __aligned(ARCH_DMA_MINALIGN);
+	__be16 rx_buf;
+
+	struct clk *reference_clk;
+	struct pwm_device *ldacb_pwm;
+	struct gpio_desc *resetb_gpio;
+	struct gpio_desc *shdn_gpio;
+
+	/* Debugfs Attributes */
+	u64 debug_streaming_len;
+	u64 debug_streaming_data;
+	u16 debug_streaming_addr;
+	u32 debug_spi_speed_hz_write;
+	u32 debug_spi_speed_hz_read;
+
+	/*
+	 * Sets SPI Frequency via Clock Generator or SPI Engine
+	 * 0 = SPI frequency changed, recompute clock mode
+	 * 1 = Frequency set via clock generator
+	 * 2 = SPI Engine controls speed
+	 */
+	u8 set_clk_mode;
+	u32 spi_max_speed_hz;
+
+	/* Device Attributes */
+	unsigned int dev_addr;
+	unsigned int addr_ascension;
+	unsigned int single_instr;
+	unsigned int shift_val;
+	unsigned int addr_desc;
+	unsigned int hw_ldac_tg_state;
+	unsigned int sampling_frequency;
+	unsigned int hw_ldac_tg_pwm;
+	unsigned int mux_out_sel;
+	unsigned int multi_dac_input_a;
+	bool multi_dac_sw_ldac_trigger;
+	unsigned int reference_volts;
+	unsigned int ref_select;
+	unsigned int hw_shutdown_state;
+
+	/* Channel Attributes */
+	unsigned int hw_active_edge[NUM_CHANNELS];
+	unsigned int range_sel[NUM_CHANNELS];
+	unsigned int output_state[NUM_CHANNELS];
+	unsigned int ldac_trigger_chn[NUM_CHANNELS];
+	unsigned int toggle_trigger_chn[NUM_CHANNELS];
+	unsigned int dither_trigger_chn[NUM_CHANNELS];
+	unsigned int multi_dac_sel_ch[NUM_CHANNELS];
+};
+
+/* ENUM Lists */
+enum addr_ascension_iio_dev_attr {
+	ADDR_ASCENSION_DECREMENT = 0,
+	ADDR_ASCENSION_INCREMENT,
+};
+
+static const char * const addr_ascension_iio_dev_attr_vals[] = {
+	[ADDR_ASCENSION_DECREMENT] = "decrement",
+	[ADDR_ASCENSION_INCREMENT] = "increment",
+};
+
+enum single_instr_iio_dev_attr {
+	SINGLE_INSTR_STREAMING = 0,
+	SINGLE_INSTR_SINGLE_INSTRUCTION,
+};
+
+static const char * const single_instr_iio_dev_attr_vals[] = {
+	[SINGLE_INSTR_STREAMING] = "streaming",
+	[SINGLE_INSTR_SINGLE_INSTRUCTION] = "single_instruction",
+};
+
+enum hw_ldac_tg_state_iio_dev_attr {
+	HW_LDAC_TG_STATE_LOW = 0,
+	HW_LDAC_TG_STATE_HIGH
+};
+
+static const char * const hw_ldac_tg_state_iio_dev_attr_vals[] = {
+	[HW_LDAC_TG_STATE_LOW] = "low",
+	[HW_LDAC_TG_STATE_HIGH] = "high",
+};
+
+enum hw_ldac_tg_pwm_iio_dev_attr {
+	HW_LDAC_TG_PWM_DISABLED,
+	HW_LDAC_TG_PWM_ENABLED,
+};
+
+static const char * const hw_ldac_tg_pwm_iio_dev_attr_vals[] = {
+	[HW_LDAC_TG_PWM_DISABLED] = "disable",
+	[HW_LDAC_TG_PWM_ENABLED] = "enable",
+};
+
+enum mux_out_sel_iio_dev_attr {
+	MUX_OUT_SEL_DISABLED = 0,	/* Index 0 */
+	MUX_OUT_SEL_AGND,		/* Index 1 */
+	MUX_OUT_SEL_AVDD,		/* Index 2 */
+	MUX_OUT_SEL_VREF,		/* Index 3 */
+	MUX_OUT_SEL_IOUT0_VMON,		/* Index 4 */
+	MUX_OUT_SEL_IOUT1_VMON,		/* Index 5 */
+	MUX_OUT_SEL_IOUT2_VMON,		/* Index 6 */
+	MUX_OUT_SEL_IOUT3_VMON,		/* Index 7 */
+	MUX_OUT_SEL_IOUT0_IMON,		/* Index 8 */
+	MUX_OUT_SEL_IOUT1_IMON,		/* Index 9 */
+	MUX_OUT_SEL_IOUT2_IMON,		/* Index 10 */
+	MUX_OUT_SEL_IOUT3_IMON,		/* Index 11 */
+	MUX_OUT_SEL_PVDD0,		/* Index 12 */
+	MUX_OUT_SEL_PVDD1,		/* Index 13 */
+	MUX_OUT_SEL_PVDD2,		/* Index 14 */
+	MUX_OUT_SEL_PVDD3,		/* Index 15 */
+	MUX_OUT_SEL_TEMP_SENSOR0,	/* Index 16 */
+	MUX_OUT_SEL_TEMP_SENSOR1,	/* Index 17 */
+	MUX_OUT_SEL_TEMP_SENSOR2,	/* Index 18 */
+	MUX_OUT_SEL_TEMP_SENSOR3,	/* Index 19 */
+	MUX_OUT_SEL_MUX_IN0,		/* Index 20 */
+	MUX_OUT_SEL_MUX_IN1,		/* Index 21 */
+	MUX_OUT_SEL_MUX_IN2,		/* Index 22 */
+	MUX_OUT_SEL_MUX_IN3,		/* Index 23 */
+};
+
+static const char * const mux_out_sel_iio_dev_attr_vals[] = {
+	[MUX_OUT_SEL_DISABLED]		= "disabled",	/* Index 0 */
+	[MUX_OUT_SEL_AGND]		= "agnd",	/* Index 1 */
+	[MUX_OUT_SEL_AVDD]		= "avdd",	/* Index 2 */
+	[MUX_OUT_SEL_VREF]		= "vref",	/* Index 3 */
+	[MUX_OUT_SEL_IOUT0_VMON]	= "iout0_vmon",	/* Index 4 */
+	[MUX_OUT_SEL_IOUT1_VMON]	= "iout1_vmon",	/* Index 5 */
+	[MUX_OUT_SEL_IOUT2_VMON]	= "iout2_vmon",	/* Index 6 */
+	[MUX_OUT_SEL_IOUT3_VMON]	= "iout3_vmon",	/* Index 7 */
+	[MUX_OUT_SEL_IOUT0_IMON]	= "iout0_imon",	/* Index 8 */
+	[MUX_OUT_SEL_IOUT1_IMON]	= "iout1_imon",	/* Index 9 */
+	[MUX_OUT_SEL_IOUT2_IMON]	= "iout2_imon",	/* Index 10 */
+	[MUX_OUT_SEL_IOUT3_IMON]	= "iout3_imon",	/* Index 11 */
+	[MUX_OUT_SEL_PVDD0]		= "pvdd0",	/* Index 12 */
+	[MUX_OUT_SEL_PVDD1]		= "pvdd1",	/* Index 13 */
+	[MUX_OUT_SEL_PVDD2]		= "pvdd2",	/* Index 14 */
+	[MUX_OUT_SEL_PVDD3]		= "pvdd3",	/* Index 15 */
+	[MUX_OUT_SEL_TEMP_SENSOR0]	= "tdiode_ch0",	/* Index 16 */
+	[MUX_OUT_SEL_TEMP_SENSOR1]	= "tdiode_ch1",	/* Index 17 */
+	[MUX_OUT_SEL_TEMP_SENSOR2]	= "tdiode_ch2",	/* Index 18 */
+	[MUX_OUT_SEL_TEMP_SENSOR3]	= "tdiode_ch3",	/* Index 19 */
+	[MUX_OUT_SEL_MUX_IN0]		= "mux_in0",	/* Index 20 */
+	[MUX_OUT_SEL_MUX_IN1]		= "mux_in1",	/* Index 21 */
+	[MUX_OUT_SEL_MUX_IN2]		= "mux_in2",	/* Index 22 */
+	[MUX_OUT_SEL_MUX_IN3]		= "mux_in3",	/* Index 23 */
+};
+
+static const u8 mux_out_sel_reg_values[] = {
+	[MUX_OUT_SEL_DISABLED]		= 0x00,
+	[MUX_OUT_SEL_AGND]		= 0x80,
+	[MUX_OUT_SEL_AVDD]		= 0x81,
+	[MUX_OUT_SEL_VREF]		= 0x82,
+	[MUX_OUT_SEL_IOUT0_VMON]	= 0x84,
+	[MUX_OUT_SEL_IOUT1_VMON]	= 0x85,
+	[MUX_OUT_SEL_IOUT2_VMON]	= 0x86,
+	[MUX_OUT_SEL_IOUT3_VMON]	= 0x87,
+	[MUX_OUT_SEL_IOUT0_IMON]	= 0x88,
+	[MUX_OUT_SEL_IOUT1_IMON]	= 0x89,
+	[MUX_OUT_SEL_IOUT2_IMON]	= 0x8A,
+	[MUX_OUT_SEL_IOUT3_IMON]	= 0x8B,
+	[MUX_OUT_SEL_PVDD0]		= 0x8C,
+	[MUX_OUT_SEL_PVDD1]		= 0x8D,
+	[MUX_OUT_SEL_PVDD2]		= 0x8E,
+	[MUX_OUT_SEL_PVDD3]		= 0x8F,
+	[MUX_OUT_SEL_TEMP_SENSOR0]	= 0x90,
+	[MUX_OUT_SEL_TEMP_SENSOR1]	= 0x91,
+	[MUX_OUT_SEL_TEMP_SENSOR2]	= 0x92,
+	[MUX_OUT_SEL_TEMP_SENSOR3]	= 0x93,
+	[MUX_OUT_SEL_MUX_IN0]		= 0x94,
+	[MUX_OUT_SEL_MUX_IN1]		= 0x95,
+	[MUX_OUT_SEL_MUX_IN2]		= 0x96,
+	[MUX_OUT_SEL_MUX_IN3]		= 0x97,
+};
+
+enum multi_dac_sw_ldac_trigger_iio_dev_attr {
+	MULTI_DAC_SW_LDAC_TRIGGER_LOW = 0,
+	MULTI_DAC_SW_LDAC_TRIGGER_TRIGGER,
+};
+
+static const char * const multi_dac_sw_ldac_trigger_iio_dev_attr_vals[] = {
+	[MULTI_DAC_SW_LDAC_TRIGGER_LOW] = "low",
+	[MULTI_DAC_SW_LDAC_TRIGGER_TRIGGER] = "trigger",
+};
+
+enum ref_select_iio_dev_attr {
+	REF_SELECT_EXTERNAL,
+	REF_SELECT_INTERNAL,
+};
+
+static const char * const ref_select_iio_dev_attr_vals[] = {
+	[REF_SELECT_EXTERNAL] = "external",
+	[REF_SELECT_INTERNAL] = "internal",
+};
+
+enum hw_shutdown_state_iio_dev_attr {
+	HW_SHUTDOWN_STATE_LOW,
+	HW_SHUTDOWN_STATE_HIGH,
+};
+
+static const char * const hw_shutdown_state_iio_dev_attr_vals[] = {
+	[HW_SHUTDOWN_STATE_LOW] = "low",
+	[HW_SHUTDOWN_STATE_HIGH] = "high",
+};
+
+enum hw_active_edge_iio_dev_attr {
+	HW_ACTIVE_EDGE_RISING_EDGE = 0,
+	HW_ACTIVE_EDGE_FALLING_EDGE,
+	HW_ACTIVE_EDGE_ANY_EDGE,
+};
+
+static const char * const hw_active_edge_iio_dev_attr_vals[] = {
+	[HW_ACTIVE_EDGE_RISING_EDGE] = "rising_edge",
+	[HW_ACTIVE_EDGE_FALLING_EDGE] = "falling_edge",
+	[HW_ACTIVE_EDGE_ANY_EDGE] = "any_edge",
+};
+
+enum range_sel_iio_dev_attr {
+	RANGE_SEL_50 = 0,
+	RANGE_SEL_150 = 1,
+	RANGE_SEL_200 = 2,
+	RANGE_SEL_300 = 3,
+};
+
+static const char * const range_sel_iio_dev_attr_vals[] = {
+	[RANGE_SEL_50] = "50mA",
+	[RANGE_SEL_150] = "150mA",
+	[RANGE_SEL_200] = "200mA",
+	[RANGE_SEL_300] = "300mA",
+};
+
+enum output_state_iio_dev_attr {
+	OUTPUT_STATE_NORMAL_SW = 0,
+	OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_SW,
+	OUTPUT_STATE_SHUTDOWN_TO_GND_SW,
+	OUTPUT_STATE_NORMAL_HW,
+	OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_HW,
+	OUTPUT_STATE_SHUTDOWN_TO_GND_HW,
+};
+
+static const char * const output_state_iio_dev_attr_vals[] = {
+	[OUTPUT_STATE_NORMAL_SW] = "normal_sw",
+	[OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_SW] = "shutdown_to_tristate_sw",
+	[OUTPUT_STATE_SHUTDOWN_TO_GND_SW] = "shutdown_to_gnd_sw",
+	[OUTPUT_STATE_NORMAL_HW] = "normal_hw",
+	[OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_HW] = "shutdown_to_tristate_hw",
+	[OUTPUT_STATE_SHUTDOWN_TO_GND_HW] = "shutdown_to_gnd_hw",
+};
+
+enum ldac_trigger_chn_iio_dev_attr {
+	LDAC_TRIGGER_CHN_NONE = 0,
+	LDAC_TRIGGER_CHN_HW_TRIGGER,
+	LDAC_TRIGGER_CHN_SW_TRIGGER,
+};
+
+static const char * const ldac_trigger_chn_iio_dev_attr_vals[] = {
+	[LDAC_TRIGGER_CHN_NONE] = "None",
+	[LDAC_TRIGGER_CHN_HW_TRIGGER] = "hw_ldac",
+	[LDAC_TRIGGER_CHN_SW_TRIGGER] = "sw_ldac",
+};
+
+enum toggle_trigger_chn_iio_dev_attr {
+	TOGGLE_TRIGGER_CHN_NONE = 0,
+	TOGGLE_TRIGGER_CHN_HW_TRIGGER,
+	TOGGLE_TRIGGER_CHN_SW_TRIGGER,
+};
+
+static const char * const toggle_trigger_chn_iio_dev_attr_vals[] = {
+	[TOGGLE_TRIGGER_CHN_NONE] = "None",
+	[TOGGLE_TRIGGER_CHN_HW_TRIGGER] = "hw_toggle",
+	[TOGGLE_TRIGGER_CHN_SW_TRIGGER] = "sw_toggle",
+};
+
+enum dither_trigger_chn_iio_dev_attr {
+	DITHER_TRIGGER_CHN_NONE = 0,
+	DITHER_TRIGGER_CHN_HW_TRIGGER,
+	DITHER_TRIGGER_CHN_SW_TRIGGER,
+};
+
+static const char * const dither_trigger_chn_iio_dev_attr_vals[] = {
+	[DITHER_TRIGGER_CHN_NONE] = "None",
+	[DITHER_TRIGGER_CHN_HW_TRIGGER] = "hw_dither",
+	[DITHER_TRIGGER_CHN_SW_TRIGGER] = "sw_dither",
+};
+
+enum multi_dac_sel_ch_iio_chan_attr {
+	MULTI_DAC_SEL_CH_EXCLUDE = 0,
+	MULTI_DAC_SEL_CH_INCLUDE = 1,
+};
+
+static const char * const multi_dac_sel_ch_iio_chan_attr_vals[] = {
+	[MULTI_DAC_SEL_CH_EXCLUDE] = "exclude",
+	[MULTI_DAC_SEL_CH_INCLUDE] = "include",
+};
+
+static int _ad5706r_set_clk_rate(struct ad5706r_state *st, u32 rate, u8 wr)
+{
+	int ret, current_rate;
+
+	rate = clamp(rate, (u32)SPI_MIN_SPEED_HZ, (u32)SPI_MAX_SPEED_HZ);
+	/* If frequency is already set, do nothing */
+	current_rate = DIV_ROUND_CLOSEST(clk_get_rate(st->reference_clk), 2);
+	if (wr && current_rate == rate) {
+		st->debug_spi_speed_hz_write = current_rate;
+		return 0;
+	}
+	if (!wr && current_rate == rate) {
+		st->debug_spi_speed_hz_read = current_rate;
+		return 0;
+	}
+
+	/* Disable the clock before setting the rate */
+	clk_disable_unprepare(st->reference_clk);
+
+	/* spi engine spi clock runs at half the SPI reference clock */
+	ret = clk_set_rate(st->reference_clk, rate * 2);
+	if (ret)
+		return ret;
+
+	/* Re-enable the clock after setting the rate */
+	ret = clk_prepare_enable(st->reference_clk);
+	if (ret)
+		return ret;
+
+	/* If frequency is not a good number, save the closest possible */
+	current_rate = DIV_ROUND_CLOSEST(clk_get_rate(st->reference_clk), 2);
+	if (wr)
+		st->debug_spi_speed_hz_write = current_rate;
+	else
+		st->debug_spi_speed_hz_read = current_rate;
+
+	/* Wait for clock to stabilize */
+	usleep_range(3000, 3100);
+
+	return 0;
+}
+
+/*
+ * Check if a frequency can be achieved using SPI Engine integer divider.
+ * SPI Engine hardware requires:
+ * - Even dividers only (hardware limitation)
+ * - Divider > 1 (divider of 1 means running at max frequency)
+ * - Exact frequency match after division (no rounding errors)
+ */
+static bool _can_use_spi_engine_divider(u32 max_freq, u32 target_freq)
+{
+	u32 divider = DIV_ROUND_CLOSEST(max_freq, target_freq);
+	u32 achieved_freq = DIV_ROUND_CLOSEST(max_freq, divider);
+
+	if (achieved_freq != target_freq)
+		return false;  /* Rounding error, need exact frequency */
+
+	if (divider <= 1)
+		return false;  /* Already at max frequency */
+
+	if (divider & 1)
+		return false;  /* SPI Engine requires even dividers */
+
+	return true;
+}
+
+/*
+ * CLKGEN mode contains a 3ms delay after changing the clock frequency, this
+ * delay is avoided if frequencies can be set by SPI Engine.
+ */
+static int _ad5706r_compute_spi_clk(struct ad5706r_state *st, int *write_hz,
+				    int *read_hz)
+{
+	int ret;
+
+	/* Check frequencies if SPI Engine can generate them */
+	if (_can_use_spi_engine_divider(st->spi_max_speed_hz, st->debug_spi_speed_hz_write) &&
+	    _can_use_spi_engine_divider(st->spi_max_speed_hz, st->debug_spi_speed_hz_read)) {
+		/* Disable the clock before setting the rate */
+		clk_disable_unprepare(st->reference_clk);
+
+		/* spi engine spi clock runs at half the SPI reference clock */
+		ret = clk_set_rate(st->reference_clk, st->spi_max_speed_hz * 2);
+		if (ret)
+			return ret;
+
+		/* Re-enable the clock after setting the rate */
+		ret = clk_prepare_enable(st->reference_clk);
+		if (ret)
+			return ret;
+
+		*write_hz = st->debug_spi_speed_hz_write;
+		*read_hz = st->debug_spi_speed_hz_read;
+		st->set_clk_mode = CLK_MODE_SPI_ENGINE;
+
+		return 0;
+	}
+
+	/* Otherwise, set to Clock Generator Mode */
+	*write_hz = 0;
+	*read_hz = 0;
+	st->set_clk_mode = CLK_MODE_CLKGEN;
+
+	return 0;
+}
+
+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_spi_write(struct ad5706r_state *st, u16 reg, u16 val)
+{
+	unsigned int num_bytes;
+	int write_hz, read_hz;
+	int ret;
+
+	num_bytes = ad5706r_reg_len(reg);
+
+	if (reg >= AD5706R_MULTIBYTE_REG_START && reg <= AD5706R_MULTIBYTE_REG_END)
+		reg = reg + st->addr_desc;
+
+	if (st->set_clk_mode == CLK_MODE_UNKNOWN) {
+		ret = _ad5706r_compute_spi_clk(st, &write_hz, &read_hz);
+		if (ret)
+			return ret;
+	}
+
+	if (st->set_clk_mode == CLK_MODE_CLKGEN) {
+		write_hz = 0;
+		ret = _ad5706r_set_clk_rate(st, st->debug_spi_speed_hz_write, 1);
+		if (ret)
+			return ret;
+	} else {
+		write_hz = st->debug_spi_speed_hz_write;
+	}
+
+	struct spi_transfer xfer = {
+		.tx_buf = &st->tx_buf,
+		.len = num_bytes + 2,
+		.speed_hz = write_hz,
+	};
+
+	st->tx_buf = cpu_to_be32(((((st->dev_addr << 12) & AD5706R_ADDR_PIN_MASK) |
+				   reg) << 16) | (val << (16 - (num_bytes * 8))));
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_spi_read(struct ad5706r_state *st, u16 reg, u16 *val)
+{
+	unsigned int num_bytes;
+	u16 cmd;
+	int write_hz, read_hz;
+	int ret;
+
+	num_bytes = ad5706r_reg_len(reg);
+
+	if (reg >= AD5706R_MULTIBYTE_REG_START && reg <= AD5706R_MULTIBYTE_REG_END)
+		reg = reg + st->addr_desc;
+
+	if (st->set_clk_mode == CLK_MODE_UNKNOWN) {
+		ret = _ad5706r_compute_spi_clk(st, &write_hz, &read_hz);
+		if (ret)
+			return ret;
+	}
+
+	if (st->set_clk_mode == CLK_MODE_CLKGEN) {
+		read_hz = 0;
+		ret = _ad5706r_set_clk_rate(st, st->debug_spi_speed_hz_read, 0);
+		if (ret)
+			return ret;
+	} else {
+		read_hz = st->debug_spi_speed_hz_read;
+	}
+
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &st->tx_buf,
+			.rx_buf = NULL,
+			.len    = 2,
+			.speed_hz = read_hz,
+		},
+		{
+			.tx_buf = NULL,
+			.rx_buf = &st->rx_buf,
+			.len    = num_bytes,
+			.speed_hz = read_hz,
+		},
+	};
+
+	cmd = AD5706R_RD_MASK |
+	      ((st->dev_addr << 12) & AD5706R_ADDR_PIN_MASK) |
+	      (reg & AD5706R_ADDR_MASK);
+
+	/* Convert to 32-bit big-endian (shift to upper 16 bits) */
+	st->tx_buf = cpu_to_be32((u32)cmd << 16);
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret)
+		return ret;
+
+	*val = be16_to_cpu(st->rx_buf) >> (16 - (num_bytes * 8));
+
+	return 0;
+}
+
+/* debugfs Register Access */
+
+static int ad5706r_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			      unsigned int writeval, unsigned int *readval)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	if (reg >= AD5706R_MULTIBYTE_REG_START && reg <= AD5706R_MULTIBYTE_REG_END)
+		reg = reg - st->addr_desc;
+
+	if (readval)
+		return ad5706r_spi_read(st, reg, (u16 *)readval);
+
+	return ad5706r_spi_write(st, reg, writeval);
+}
+
+static int ad5706r_set_streaming_addr(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	st->debug_streaming_addr = buf;
+
+	return 0;
+}
+
+static int ad5706r_show_streaming_addr(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	*val = st->debug_streaming_addr;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_streaming_addr_fops, ad5706r_show_streaming_addr,
+			 ad5706r_set_streaming_addr, "%llu\n");
+
+static int ad5706r_set_streaming_len(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	st->debug_streaming_len = buf;
+
+	return 0;
+}
+
+static int ad5706r_show_streaming_len(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	*val = st->debug_streaming_len;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_streaming_len_fops, ad5706r_show_streaming_len,
+			 ad5706r_set_streaming_len, "%llu\n");
+
+static int ad5706r_set_streaming_data(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	st->debug_streaming_data = buf;
+
+	return 0;
+}
+
+static int ad5706r_show_streaming_data(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	*val = st->debug_streaming_data;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_streaming_data_fops, ad5706r_show_streaming_data,
+			 ad5706r_set_streaming_data, "%llu\n");
+
+static int ad5706r_set_streaming_reg_access(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 word;
+	__be16 write_val[5];
+	int write_hz, read_hz;
+	int ret;
+	int i;
+
+	write_val[0] = cpu_to_be16(((st->dev_addr << 12) & AD5706R_ADDR_PIN_MASK) |
+				 st->debug_streaming_addr);
+
+	for (i = 0; i < 4; i++) {
+		/* Extract as native u16 */
+		word = (st->debug_streaming_data >> (i * 16)) & 0xFFFF;
+
+		/* Byte swap in native endian */
+		word = (word & 0x00FF) << 8 | (word & 0xFF00) >> 8;
+
+		/* Convert to big-endian for transmission */
+		write_val[i + 1] = cpu_to_be16(word);
+		}
+
+	if (st->set_clk_mode == CLK_MODE_UNKNOWN) {
+		ret = _ad5706r_compute_spi_clk(st, &write_hz, &read_hz);
+		if (ret)
+			return ret;
+	}
+
+	if (st->set_clk_mode == CLK_MODE_CLKGEN) {
+		write_hz = 0;
+		ret = _ad5706r_set_clk_rate(st, st->debug_spi_speed_hz_write, 1);
+		if (ret)
+			return ret;
+	} else {
+		write_hz = st->debug_spi_speed_hz_write;
+	}
+
+	struct spi_transfer xfer = {
+		.tx_buf = write_val,
+		.len = st->debug_streaming_len + 2,
+		.speed_hz = write_hz,
+	};
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_show_streaming_reg_access(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u64 read_val;
+	u16 cmd;
+	int write_hz, read_hz;
+	int ret;
+
+	if (st->set_clk_mode == CLK_MODE_UNKNOWN) {
+		ret = _ad5706r_compute_spi_clk(st, &write_hz, &read_hz);
+		if (ret)
+			return ret;
+	}
+
+	if (st->set_clk_mode == CLK_MODE_CLKGEN) {
+		read_hz = 0;
+		ret = _ad5706r_set_clk_rate(st, st->debug_spi_speed_hz_read, 0);
+		if (ret)
+			return ret;
+	} else {
+		read_hz = st->debug_spi_speed_hz_read;
+	}
+
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &st->tx_buf,
+			.rx_buf = NULL,
+			.len    = 2,
+			.speed_hz = read_hz,
+		},
+		{
+			.tx_buf = NULL,
+			.rx_buf = &read_val,
+			.len    = st->debug_streaming_len,
+			.speed_hz = read_hz,
+		},
+	};
+
+	cmd = AD5706R_RD_MASK |
+	      ((st->dev_addr << 12) & AD5706R_ADDR_PIN_MASK) |
+	      (st->debug_streaming_addr & AD5706R_ADDR_MASK);
+
+	/* Convert to 32-bit big-endian (shift to upper 16 bits) */
+	st->tx_buf = cpu_to_be32((u32)cmd << 16);
+
+	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret)
+		return ret;
+
+	*val = read_val;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_streaming_reg_access_fops, ad5706r_show_streaming_reg_access,
+			 ad5706r_set_streaming_reg_access, "%llu\n");
+
+static int ad5706r_set_spi_speed_write(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	st->set_clk_mode = CLK_MODE_UNKNOWN;
+	ret = _ad5706r_set_clk_rate(st, buf, 1);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_show_spi_speed_write(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	*val = st->debug_spi_speed_hz_write;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_spi_speed_write_fops, ad5706r_show_spi_speed_write,
+			 ad5706r_set_spi_speed_write, "%llu\n");
+
+static int ad5706r_set_spi_speed_read(void *arg, u64 buf)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	st->set_clk_mode = CLK_MODE_UNKNOWN;
+	ret = _ad5706r_set_clk_rate(st, buf, 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_show_spi_speed_read(void *arg, u64 *val)
+{
+	struct iio_dev *indio_dev = arg;
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	*val = st->debug_spi_speed_hz_read;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(ad5706r_spi_speed_read_fops, ad5706r_show_spi_speed_read,
+			 ad5706r_set_spi_speed_read, "%llu\n");
+
+static void ad5706r_debugs_init(struct iio_dev *indio_dev)
+{
+	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
+
+	debugfs_create_file_unsafe("streaming_addr", 0600, d,
+				   indio_dev, &ad5706r_streaming_addr_fops);
+	debugfs_create_file_unsafe("streaming_len", 0600, d,
+				   indio_dev, &ad5706r_streaming_len_fops);
+	debugfs_create_file_unsafe("streaming_data", 0600, d,
+				   indio_dev, &ad5706r_streaming_data_fops);
+	debugfs_create_file_unsafe("streaming_reg_access", 0600, d,
+				   indio_dev, &ad5706r_streaming_reg_access_fops);
+	debugfs_create_file_unsafe("spi_speed_hz_write", 0600, d,
+				   indio_dev, &ad5706r_spi_speed_write_fops);
+	debugfs_create_file_unsafe("spi_speed_hz_read", 0600, d,
+				   indio_dev, &ad5706r_spi_speed_read_fops);
+}
+
+/* Attributes */
+
+static int _set_reg_channel_mode(struct ad5706r_state *st, int chan_n,
+				 bool bool_func_en, bool bool_func_mode,
+				 bool bool_sync_async, bool bool_hw_sw)
+{
+	u16 reg_val;
+	u16 reg_val2;
+	u16 reg_val3;
+	u16 mask_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_FUNC_EN, &reg_val);
+	if (ret)
+		return ret;
+	ret = ad5706r_spi_read(st, AD5706R_REG_LDAC_SYNC_ASYNC, &reg_val2);
+	if (ret)
+		return ret;
+	ret = ad5706r_spi_read(st, AD5706R_REG_LDAC_HW_SW, &reg_val3);
+	if (ret)
+		return ret;
+
+	mask_val = BIT(chan_n + st->shift_val);
+	reg_val = ~mask_val & reg_val;
+	reg_val2 = ~mask_val & reg_val2;
+	reg_val3 = ~mask_val & reg_val3;
+
+	usleep_range(1, 2); /* Delay for device stability after mode change. */
+
+	/* Write 0 FUNC_EN - device unlock */
+	ret = ad5706r_spi_write(st, AD5706R_REG_FUNC_EN, reg_val);
+	if (ret)
+		return ret;
+
+	/* Write 1/0 to AD5706R_REG_FUNC_MODE_SEL_CH() */
+	if (bool_func_mode)
+		ret = ad5706r_spi_write(st, AD5706R_REG_FUNC_MODE_SEL_CH(chan_n),
+					BIT(st->shift_val));
+	else
+		ret = ad5706r_spi_write(st, AD5706R_REG_FUNC_MODE_SEL_CH(chan_n),
+					0 << st->shift_val);
+	if (ret)
+		return ret;
+
+	/* Write 1 to FUNC_EN */
+	if (bool_func_en)
+		ret = ad5706r_spi_write(st, AD5706R_REG_FUNC_EN,
+					reg_val | mask_val);
+	if (ret)
+		return ret;
+
+	/* Write 1/0 to LDAC_SYNC_ASYNC */
+	if (bool_sync_async)
+		ret = ad5706r_spi_write(st, AD5706R_REG_LDAC_SYNC_ASYNC,
+					reg_val2 | mask_val);
+	else
+		ret = ad5706r_spi_write(st, AD5706R_REG_LDAC_SYNC_ASYNC,
+					reg_val2);
+	if (ret)
+		return ret;
+
+	/* Write 1/0 to LDAC_HW_SW for HW */
+	if (bool_hw_sw)
+		ret = ad5706r_spi_write(st, AD5706R_REG_LDAC_HW_SW,
+					reg_val3 | mask_val);
+	else
+		ret = ad5706r_spi_write(st, AD5706R_REG_LDAC_HW_SW,
+					reg_val3);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int _set_pwm_duty_cycle(struct ad5706r_state *st, int duty_cycle)
+{
+	struct pwm_state ldacb_pwm_state;
+	int ret;
+
+	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
+
+	ldacb_pwm_state.duty_cycle = duty_cycle == 0 ? 0 :
+		DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency * 100 / duty_cycle);
+
+	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Device Attributes */
+static ssize_t ad5706r_dev_addr_write(struct iio_dev *indio_dev,
+				      uintptr_t private, const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &reg_val);
+	if (ret)
+		return ret;
+
+	st->dev_addr = AD5706R_MASK_DEV_ADDR(reg_val);
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad5706r_dev_addr_read(struct iio_dev *indio_dev,
+				     uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return sysfs_emit(buf, "%u\n", st->dev_addr);
+}
+
+static int ad5706r_addr_ascension_write(struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan, unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_INTERFACE_CONFIG_A, &reg_val);
+	if (ret)
+		return ret;
+
+	reg_val = (~AD5706R_MASK_ADDR_ASCENSION) & reg_val;
+	reg_val = AD5706R_MASK_INTERFACE_CONFIG_A(reg_val);
+
+	if (item == ADDR_ASCENSION_DECREMENT) {
+		ret = ad5706r_spi_write(st, AD5706R_REG_INTERFACE_CONFIG_A,
+					reg_val);
+		st->shift_val = 0;
+		st->addr_desc = 1;
+	} else {
+		ret = ad5706r_spi_write(st, AD5706R_REG_INTERFACE_CONFIG_A,
+					reg_val | AD5706R_MASK_ADDR_ASCENSION);
+		st->shift_val = 8;
+		st->addr_desc = 0;
+	}
+	if (ret)
+		return ret;
+
+	st->addr_ascension = item;
+
+	return 0;
+}
+
+static int ad5706r_addr_ascension_read(struct iio_dev *indio_dev,
+				       const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_INTERFACE_CONFIG_A, &reg_val);
+	if (ret)
+		return ret;
+
+	reg_val = (reg_val & AD5706R_MASK_ADDR_ASCENSION) >> 5;
+	st->addr_ascension = reg_val;
+
+	if (st->addr_ascension)
+		st->shift_val = 8;
+	else
+		st->shift_val = 0;
+
+	return st->addr_ascension;
+}
+
+static const struct iio_enum ad5706r_addr_ascension_enum = {
+	.items = addr_ascension_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(addr_ascension_iio_dev_attr_vals),
+	.set = ad5706r_addr_ascension_write,
+	.get = ad5706r_addr_ascension_read,
+};
+
+static int ad5706r_single_instr_write(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan, unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_INTERFACE_CONFIG_B, &reg_val);
+	if (ret)
+		return ret;
+
+	reg_val = (~AD5706R_MASK_SINGLE_INSTR) & reg_val;
+	reg_val = AD5706R_MASK_INTERFACE_CONFIG_B(reg_val);
+
+	if (item == SINGLE_INSTR_STREAMING)
+		ret = ad5706r_spi_write(st, AD5706R_REG_INTERFACE_CONFIG_B,
+					reg_val);
+	else
+		ret = ad5706r_spi_write(st, AD5706R_REG_INTERFACE_CONFIG_B,
+					reg_val | AD5706R_MASK_SINGLE_INSTR);
+
+	if (ret)
+		return ret;
+
+	st->single_instr = item;
+
+	return 0;
+}
+
+static int ad5706r_single_instr_read(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_INTERFACE_CONFIG_B, &reg_val);
+	if (ret)
+		return ret;
+
+	reg_val = (reg_val & AD5706R_MASK_SINGLE_INSTR) >> 7;
+	st->single_instr = reg_val;
+
+	return st->single_instr;
+}
+
+static const struct iio_enum ad5706r_single_instr_enum = {
+	.items = single_instr_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(single_instr_iio_dev_attr_vals),
+	.set = ad5706r_single_instr_write,
+	.get = ad5706r_single_instr_read,
+};
+
+static int ad5706r_hw_ldac_tg_state_write(struct iio_dev *indio_dev,
+					  const struct iio_chan_spec *chan,
+					  unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (item == HW_LDAC_TG_STATE_HIGH)
+		ret = _set_pwm_duty_cycle(st, 100);
+	else
+		ret = _set_pwm_duty_cycle(st, 0);
+
+	if (!ret)
+		st->hw_ldac_tg_state = item;
+
+	return ret;
+}
+
+static int ad5706r_hw_ldac_tg_state_read(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return st->hw_ldac_tg_state;
+}
+
+static const struct iio_enum ad5706r_hw_ldac_tg_state_enum = {
+	.items = hw_ldac_tg_state_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(hw_ldac_tg_state_iio_dev_attr_vals),
+	.set = ad5706r_hw_ldac_tg_state_write,
+	.get = ad5706r_hw_ldac_tg_state_read,
+};
+
+static int ad5706r_hw_ldac_tg_pwm_write(struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan,
+					unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (item == HW_LDAC_TG_PWM_DISABLED)
+		ret = _set_pwm_duty_cycle(st, 0);
+	else
+		ret = _set_pwm_duty_cycle(st, 50);
+
+	if (!ret)
+		st->hw_ldac_tg_pwm = item;
+
+	return ret;
+}
+
+static int ad5706r_hw_ldac_tg_pwm_read(struct iio_dev *indio_dev,
+				       const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	struct pwm_state ldacb_pwm_state;
+
+	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
+	if (ldacb_pwm_state.duty_cycle == 0 ||
+	    ldacb_pwm_state.duty_cycle == DIV_ROUND_CLOSEST_ULL(NANO,
+					  st->sampling_frequency))
+		st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_DISABLED;
+	else
+		st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_ENABLED;
+
+	return st->hw_ldac_tg_pwm;
+}
+
+static const struct iio_enum ad5706r_hw_ldac_tg_pwm_enum = {
+	.items = hw_ldac_tg_pwm_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(hw_ldac_tg_pwm_iio_dev_attr_vals),
+	.set = ad5706r_hw_ldac_tg_pwm_write,
+	.get = ad5706r_hw_ldac_tg_pwm_read,
+};
+
+static int ad5706r_mux_out_sel_write(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_value;
+	int ret;
+
+	/* Validate index */
+	if (item >= ARRAY_SIZE(mux_out_sel_reg_values))
+		return -EINVAL;
+
+	/* Convert index to register value */
+	reg_value = mux_out_sel_reg_values[item];
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_MUX_OUT_SEL,
+				reg_value << st->shift_val);
+	if (ret)
+		return ret;
+
+	st->mux_out_sel = item;
+
+	return 0;
+}
+
+static int ad5706r_mux_out_sel_read(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	u8 reg_byte;
+	int ret;
+	int i;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_MUX_OUT_SEL, &reg_val);
+	if (ret)
+		return ret;
+
+	/* Extract the 8-bit value */
+	reg_byte = (reg_val >> st->shift_val) & 0xFF;
+
+	/* Find which index has this register value */
+	for (i = 0; i < ARRAY_SIZE(mux_out_sel_reg_values); i++) {
+		if (mux_out_sel_reg_values[i] == reg_byte) {
+			st->mux_out_sel = i;
+			return i;  /* Return index, not register value */
+		}
+	}
+
+	/* Unknown value - default to disabled */
+	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
+	return MUX_OUT_SEL_DISABLED;
+}
+
+static const struct iio_enum ad5706r_mux_out_sel_enum = {
+	.items = mux_out_sel_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(mux_out_sel_iio_dev_attr_vals),
+	.set = ad5706r_mux_out_sel_write,
+	.get = ad5706r_mux_out_sel_read,
+};
+
+static ssize_t ad5706r_multi_dac_input_a_write(struct iio_dev *indio_dev,
+					       uintptr_t private, const struct iio_chan_spec *chan,
+					       const char *buf, size_t len)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	ret = kstrtou32(buf, 16, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_INPUT_A,
+				AD5706R_MASK_MULTI_DAC_INPUT_A(reg_val));
+	if (ret)
+		return ret;
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad5706r_multi_dac_input_a_read(struct iio_dev *indio_dev,
+					      uintptr_t private, const struct iio_chan_spec *chan,
+					      char *buf)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_INPUT_A, &reg_val);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "0x%lx\n", AD5706R_MASK_MULTI_DAC_INPUT_A(reg_val));
+}
+
+static int ad5706r_multi_dac_sw_ldac_trigger_write(struct iio_dev *indio_dev,
+						   const struct iio_chan_spec *chan,
+						   unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SW_LDAC, item << st->shift_val);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_multi_dac_sw_ldac_trigger_read(struct iio_dev *indio_dev,
+						  const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SW_LDAC, &reg_val);
+	if (ret)
+		return ret;
+
+	return reg_val;
+}
+
+static const struct iio_enum ad5706r_multi_dac_sw_ldac_trigger_enum = {
+	.items = multi_dac_sw_ldac_trigger_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(multi_dac_sw_ldac_trigger_iio_dev_attr_vals),
+	.set = ad5706r_multi_dac_sw_ldac_trigger_write,
+	.get = ad5706r_multi_dac_sw_ldac_trigger_read,
+};
+
+static ssize_t ad5706r_reference_volts_write(struct iio_dev *indio_dev,
+					     uintptr_t private, const struct iio_chan_spec *chan,
+					     const char *buf, size_t len)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	ret = kstrtou32(buf, 10, &reg_val);
+	if (ret)
+		return ret;
+
+	st->reference_volts = reg_val;
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad5706r_reference_volts_read(struct iio_dev *indio_dev,
+					    uintptr_t private, const struct iio_chan_spec *chan,
+					    char *buf)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return sysfs_emit(buf, "%u\n", st->reference_volts);
+}
+
+static int ad5706r_ref_select_write(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_BANDGAP_CONTROL, item);
+	if (ret)
+		return ret;
+
+	st->ref_select = item;
+
+	return 0;
+}
+
+static int ad5706r_ref_select_read(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_BANDGAP_CONTROL, &reg_val);
+	if (ret)
+		return ret;
+
+	if (reg_val)
+		st->ref_select = REF_SELECT_INTERNAL;
+	else
+		st->ref_select = REF_SELECT_EXTERNAL;
+
+	return st->ref_select;
+}
+
+static const struct iio_enum ad5706r_ref_select_enum = {
+	.items = ref_select_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(ref_select_iio_dev_attr_vals),
+	.set = ad5706r_ref_select_write,
+	.get = ad5706r_ref_select_read,
+};
+
+static int ad5706r_hw_shutdown_state_write(struct iio_dev *indio_dev,
+					   const struct iio_chan_spec *chan,
+					   unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	if (item == HW_SHUTDOWN_STATE_LOW)
+		gpiod_set_value_cansleep(st->shdn_gpio, 0);
+	else
+		gpiod_set_value_cansleep(st->shdn_gpio, 1);
+
+	st->hw_shutdown_state = item;
+
+	return 0;
+}
+
+static int ad5706r_hw_shutdown_state_read(struct iio_dev *indio_dev,
+					  const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return st->hw_shutdown_state;
+}
+
+static const struct iio_enum ad5706r_hw_shutdown_state_enum = {
+	.items = hw_shutdown_state_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(hw_shutdown_state_iio_dev_attr_vals),
+	.set = ad5706r_hw_shutdown_state_write,
+	.get = ad5706r_hw_shutdown_state_read,
+};
+
+/* Channel Attributes */
+static ssize_t ad5706r_input_register_a_write(struct iio_dev *indio_dev,
+					      uintptr_t private, const struct iio_chan_spec *chan,
+					      const char *buf, size_t len)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	ret = kstrtou32(buf, 16, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_DAC_INPUT_A_CH(chan->channel),
+				AD5706R_MASK_DAC_INPUT_A_CH(reg_val));
+	if (ret)
+		return ret;
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad5706r_input_register_a_read(struct iio_dev *indio_dev,
+					     uintptr_t private, const struct iio_chan_spec *chan,
+					     char *buf)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_DAC_INPUT_A_CH(chan->channel), &reg_val);
+
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "0x%lx\n", AD5706R_MASK_DAC_INPUT_A_CH(reg_val));
+}
+
+static ssize_t ad5706r_input_register_b_write(struct iio_dev *indio_dev,
+					      uintptr_t private, const struct iio_chan_spec *chan,
+					      const char *buf, size_t len)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	ret = kstrtou32(buf, 16, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_FUNC_DAC_INPUT_B_CH(chan->channel),
+				AD5706R_MASK_FUNC_DAC_INPUT_B_CH(reg_val));
+
+	if (ret)
+		return ret;
+
+	return ret ? ret : len;
+}
+
+static ssize_t ad5706r_input_register_b_read(struct iio_dev *indio_dev,
+					     uintptr_t private, const struct iio_chan_spec *chan,
+					     char *buf)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_FUNC_DAC_INPUT_B_CH(chan->channel),
+			       &reg_val);
+
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "0x%lx\n", AD5706R_MASK_FUNC_DAC_INPUT_B_CH(reg_val));
+}
+
+static int ad5706r_hw_active_edge_write(struct iio_dev *indio_dev,
+					const struct iio_chan_spec *chan, unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_LDAC_EDGE_SEL_CH(chan->channel),
+				AD5706R_MASK_LDAC_EDGE_SEL_CH(item) << st->shift_val);
+	if (ret)
+		return ret;
+
+	st->hw_active_edge[chan->channel] = item;
+
+	return 0;
+}
+
+static int ad5706r_hw_active_edge_read(struct iio_dev *indio_dev,
+				       const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_LDAC_EDGE_SEL_CH(chan->channel),
+			       &reg_val);
+	if (ret)
+		return ret;
+
+	st->hw_active_edge[chan->channel] = AD5706R_MASK_LDAC_EDGE_SEL_CH(reg_val >> st->shift_val);
+
+	return st->hw_active_edge[chan->channel];
+}
+
+static const struct iio_enum ad5706r_hw_active_edge_enum = {
+	.items = hw_active_edge_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(hw_active_edge_iio_dev_attr_vals),
+	.set = ad5706r_hw_active_edge_write,
+	.get = ad5706r_hw_active_edge_read,
+};
+
+static int ad5706r_range_sel_write(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan, unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = ad5706r_spi_write(st, AD5706R_REG_OUT_RANGE_CH(chan->channel),
+				AD5706R_MASK_OUT_RANGE_CH(item) << st->shift_val);
+
+	if (ret)
+		return ret;
+
+	st->range_sel[chan->channel] = item;
+
+	return 0;
+}
+
+static int ad5706r_range_sel_read(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_RANGE_CH(chan->channel), &reg_val);
+	if (ret)
+		return ret;
+
+	st->range_sel[chan->channel] = AD5706R_MASK_OUT_RANGE_CH(reg_val >> st->shift_val);
+
+	return st->range_sel[chan->channel];
+}
+
+static const struct iio_enum ad5706r_range_sel_enum = {
+	.items = range_sel_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(range_sel_iio_dev_attr_vals),
+	.set = ad5706r_range_sel_write,
+	.get = ad5706r_range_sel_read,
+};
+
+static int ad5706r_output_state_write(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	u16 reg_val2;
+	u16 reg_val3;
+	u16 mask_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_OPERATING_MODE, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_SWITCH_EN, &reg_val2);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_SHDN_EN, &reg_val3);
+	if (ret)
+		return ret;
+
+	mask_val = 1 << (chan->channel + st->shift_val);
+	reg_val = ~mask_val & reg_val;
+	reg_val2 = ~mask_val & reg_val2;
+	reg_val3 = ~mask_val & reg_val3;
+
+	switch (item) {
+	case OUTPUT_STATE_NORMAL_SW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3);
+		if (ret)
+			return ret;
+		break;
+	case OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_SW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_SWITCH_EN,
+					reg_val2);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3);
+		if (ret)
+			return ret;
+		break;
+	case OUTPUT_STATE_SHUTDOWN_TO_GND_SW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_SWITCH_EN,
+					reg_val2 | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3);
+		if (ret)
+			return ret;
+		break;
+	case OUTPUT_STATE_NORMAL_HW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3 | mask_val);
+		if (ret)
+			return ret;
+		gpiod_set_value_cansleep(st->shdn_gpio, 1);
+		break;
+	case OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_HW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_SWITCH_EN,
+					reg_val2);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3 | mask_val);
+		if (ret)
+			return ret;
+		gpiod_set_value_cansleep(st->shdn_gpio, 0);
+		break;
+	case OUTPUT_STATE_SHUTDOWN_TO_GND_HW:
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_OPERATING_MODE,
+					reg_val | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_OUT_SWITCH_EN,
+					reg_val2 | mask_val);
+		if (ret)
+			return ret;
+		ret = ad5706r_spi_write(st, AD5706R_REG_SHDN_EN, reg_val3 | mask_val);
+		if (ret)
+			return ret;
+		gpiod_set_value_cansleep(st->shdn_gpio, 0);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ad5706r_output_state_read(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	u16 reg_val2;
+	u16 reg_val3;
+	u16 gpio_val;
+	u16 mask_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_OPERATING_MODE, &reg_val);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_SWITCH_EN, &reg_val2);
+	if (ret)
+		return ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_SHDN_EN, &reg_val3);
+	if (ret)
+		return ret;
+
+	mask_val = 1 << (chan->channel + st->shift_val);
+	reg_val = mask_val & reg_val;
+	reg_val2 = mask_val & reg_val2;
+	reg_val3 = mask_val & reg_val3;
+	gpio_val = gpiod_get_value_cansleep(st->shdn_gpio);
+
+	if (reg_val && !reg_val3)
+		st->output_state[chan->channel] = OUTPUT_STATE_NORMAL_SW;
+	else if (!reg_val && !reg_val2)
+		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_SW;
+	else if (!reg_val && reg_val2)
+		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_GND_SW;
+	else if (reg_val && reg_val3 && gpio_val)
+		st->output_state[chan->channel] = OUTPUT_STATE_NORMAL_HW;
+	else if (reg_val && !reg_val2 && reg_val3 && !gpio_val)
+		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_HW;
+	else if (reg_val && reg_val2 && reg_val3 && !gpio_val)
+		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_GND_HW;
+
+	return st->output_state[chan->channel];
+}
+
+static const struct iio_enum ad5706r_output_state_enum = {
+	.items = output_state_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(output_state_iio_dev_attr_vals),
+	.set = ad5706r_output_state_write,
+	.get = ad5706r_output_state_read,
+};
+
+static int ad5706r_ldac_trigger_chn_write(struct iio_dev *indio_dev,
+					  const struct iio_chan_spec *chan,
+					  unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	bool val_func_en = 0;
+	bool val_func_mode = 0;
+	bool val_sync_async = 0;
+	bool val_hw_sw = 0;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
+	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
+	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
+
+	if (item != LDAC_TRIGGER_CHN_NONE)
+		val_sync_async = 1;	/* Write 1 LDAC_SYNC_ASYNC */
+
+	if (item == LDAC_TRIGGER_CHN_SW_TRIGGER)
+		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
+
+	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
+				    val_sync_async, val_hw_sw);
+	if (ret)
+		return ret;
+
+	st->ldac_trigger_chn[chan->channel] = item;
+
+	return 0;
+}
+
+static int ad5706r_ldac_trigger_chn_read(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return st->ldac_trigger_chn[chan->channel];
+}
+
+static const struct iio_enum ad5706r_ldac_trigger_chn_enum = {
+	.items = ldac_trigger_chn_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(ldac_trigger_chn_iio_dev_attr_vals),
+	.set = ad5706r_ldac_trigger_chn_write,
+	.get = ad5706r_ldac_trigger_chn_read,
+};
+
+static int ad5706r_toggle_trigger_chn_write(struct iio_dev *indio_dev,
+					    const struct iio_chan_spec *chan,
+					    unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	bool val_func_en = 0;
+	bool val_func_mode = 0;
+	bool val_sync_async = 0;
+	bool val_hw_sw = 0;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
+	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
+	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
+
+	if (item != TOGGLE_TRIGGER_CHN_NONE)
+		val_func_en = 1;	/* Write 1 FUNC_EN */
+	if (item == TOGGLE_TRIGGER_CHN_SW_TRIGGER)
+		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
+
+	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
+				    val_sync_async, val_hw_sw);
+	if (ret)
+		return ret;
+
+	st->toggle_trigger_chn[chan->channel] = item;
+
+	return 0;
+}
+
+static int ad5706r_toggle_trigger_chn_read(struct iio_dev *indio_dev,
+					   const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return st->toggle_trigger_chn[chan->channel];
+}
+
+static const struct iio_enum ad5706r_toggle_trigger_chn_enum = {
+	.items = toggle_trigger_chn_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(toggle_trigger_chn_iio_dev_attr_vals),
+	.set = ad5706r_toggle_trigger_chn_write,
+	.get = ad5706r_toggle_trigger_chn_read,
+};
+
+static int ad5706r_dither_trigger_chn_write(struct iio_dev *indio_dev,
+					    const struct iio_chan_spec *chan,
+					    unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	bool val_func_en = 0;
+	bool val_func_mode = 1;
+	bool val_sync_async = 0;
+	bool val_hw_sw = 0;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
+	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
+	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
+
+	if (item != DITHER_TRIGGER_CHN_NONE)
+		val_func_en = 1;	/* Write 1 FUNC_EN */
+	if (item == DITHER_TRIGGER_CHN_SW_TRIGGER)
+		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
+
+	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
+				    val_sync_async, val_hw_sw);
+	if (ret)
+		return ret;
+
+	st->dither_trigger_chn[chan->channel] = item;
+
+	return 0;
+}
+
+static int ad5706r_dither_trigger_chn_read(struct iio_dev *indio_dev,
+					   const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+
+	return st->dither_trigger_chn[chan->channel];
+}
+
+static const struct iio_enum ad5706r_dither_trigger_chn_enum = {
+	.items = dither_trigger_chn_iio_dev_attr_vals,
+	.num_items = ARRAY_SIZE(dither_trigger_chn_iio_dev_attr_vals),
+	.set = ad5706r_dither_trigger_chn_write,
+	.get = ad5706r_dither_trigger_chn_read,
+};
+
+static int ad5706r_multi_dac_sel_ch_write(struct iio_dev *indio_dev,
+					  const struct iio_chan_spec *chan,
+					  unsigned int item)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	u16 mask_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SEL_CH, &reg_val);
+	if (ret)
+		return ret;
+
+	mask_val = BIT(chan->channel + st->shift_val);
+	reg_val = ~mask_val & reg_val;
+
+	if (item == MULTI_DAC_SEL_CH_EXCLUDE)
+		ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SEL_CH,
+					reg_val);
+	else
+		ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SEL_CH,
+					reg_val | mask_val);
+
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad5706r_multi_dac_sel_ch_read(struct iio_dev *indio_dev,
+					 const struct iio_chan_spec *chan)
+{
+	struct ad5706r_state *st = iio_priv(indio_dev);
+	u16 reg_val;
+	u16 mask_val;
+	int ret;
+
+	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SEL_CH, &reg_val);
+	if (ret)
+		return ret;
+
+	mask_val = BIT(chan->channel + st->shift_val);
+	reg_val = mask_val & reg_val;
+
+	if (reg_val)
+		st->multi_dac_sel_ch[chan->channel] = MULTI_DAC_SEL_CH_INCLUDE;
+	else
+		st->multi_dac_sel_ch[chan->channel] = MULTI_DAC_SEL_CH_EXCLUDE;
+
+	return st->multi_dac_sel_ch[chan->channel];
+}
+
+static const struct iio_enum ad5706r_multi_dac_sel_ch_enum = {
+	.items = multi_dac_sel_ch_iio_chan_attr_vals,
+	.num_items = ARRAY_SIZE(multi_dac_sel_ch_iio_chan_attr_vals),
+	.set = ad5706r_multi_dac_sel_ch_write,
+	.get = ad5706r_multi_dac_sel_ch_read,
+};
+
+#define AD5706R_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) {	\
+	.name = _name,							\
+	.read = (_read),						\
+	.write = (_write),						\
+	.private = (_what),						\
+	.shared = (_shared),						\
+}
+
+static struct iio_chan_spec_ext_info ad5706r_ext_info[] = {
+	/* device_attribute */
+	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
+			      ad5706r_dev_addr_read, ad5706r_dev_addr_write),
+
+	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
+	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
+
+	IIO_ENUM("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
+	IIO_ENUM_AVAILABLE("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
+
+	IIO_ENUM("hw_ldac_tg_state", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_state_enum),
+	IIO_ENUM_AVAILABLE("hw_ldac_tg_state", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_state_enum),
+
+	/* Sampling Frequency part of read/write RAW */
+
+	IIO_ENUM("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
+	IIO_ENUM_AVAILABLE("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
+
+	IIO_ENUM("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
+	IIO_ENUM_AVAILABLE("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
+
+	AD5706R_CHAN_EXT_INFO("multi_dac_input_a", 0, IIO_SHARED_BY_ALL,
+			      ad5706r_multi_dac_input_a_read, ad5706r_multi_dac_input_a_write),
+
+	IIO_ENUM("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
+		 &ad5706r_multi_dac_sw_ldac_trigger_enum),
+	IIO_ENUM_AVAILABLE("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
+			   &ad5706r_multi_dac_sw_ldac_trigger_enum),
+
+	AD5706R_CHAN_EXT_INFO("reference_volts", 0, IIO_SHARED_BY_ALL,
+			      ad5706r_reference_volts_read, ad5706r_reference_volts_write),
+
+	IIO_ENUM("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
+	IIO_ENUM_AVAILABLE("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
+
+	IIO_ENUM("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),
+	IIO_ENUM_AVAILABLE("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),
+
+	/* Channel Attributes */
+	AD5706R_CHAN_EXT_INFO("input_register_a", 0, IIO_SEPARATE,
+			      ad5706r_input_register_a_read, ad5706r_input_register_a_write),
+
+	AD5706R_CHAN_EXT_INFO("input_register_b", 0, IIO_SEPARATE,
+			      ad5706r_input_register_b_read, ad5706r_input_register_b_write),
+
+	IIO_ENUM("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
+	IIO_ENUM_AVAILABLE("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
+
+	IIO_ENUM("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
+	IIO_ENUM_AVAILABLE("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
+
+	IIO_ENUM("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
+	IIO_ENUM_AVAILABLE("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
+
+	IIO_ENUM("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
+	IIO_ENUM_AVAILABLE("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
+
+	IIO_ENUM("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
+	IIO_ENUM_AVAILABLE("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
+
+	IIO_ENUM("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
+	IIO_ENUM_AVAILABLE("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
+
+	IIO_ENUM("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),
+	IIO_ENUM_AVAILABLE("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),
+
+	{},
+};
+
+/* Channel */
+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);
+	u16 reg_val;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		scoped_guard(mutex, &st->lock) {
+			ret = ad5706r_spi_read(st, AD5706R_REG_DAC_DATA_READBACK_CH(chan->channel),
+					       &reg_val);
+
+			if (ret)
+				return ret;
+
+			*val = reg_val;
+		}
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		switch (st->range_sel[chan->channel]) {
+		case RANGE_SEL_50:
+			*val = 50 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
+			break;
+		case RANGE_SEL_150:
+			*val = 150 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
+			break;
+		case RANGE_SEL_200:
+			*val = 200 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
+			break;
+		case RANGE_SEL_300:
+		default:
+			*val = 300 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
+			break;
+		}
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 0;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = st->sampling_frequency;
+		return IIO_VAL_INT;
+	}
+
+	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);
+	struct pwm_state ldacb_pwm_state;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		/* Sets minimum and maximum frequency */
+		val = clamp(val, SAMPLING_FREQUENCY_MIN_HZ, SAMPLING_FREQUENCY_MAX_HZ);
+
+		scoped_guard(mutex, &st->lock) {
+			pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
+			ldacb_pwm_state.duty_cycle = DIV_ROUND_CLOSEST_ULL(NANO, 2 * val);
+			ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, val);
+			ldacb_pwm_state.enabled = true;
+
+			ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
+			if (ret)
+				return ret;
+
+			st->sampling_frequency = val;
+		}
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info ad5706r_info = {
+	.read_raw = &ad5706r_read_raw,
+	.write_raw = &ad5706r_write_raw,
+	.debugfs_reg_access = &ad5706r_reg_access,
+};
+
+#define AD5706R_CHAN(_channel) {				\
+	.type = IIO_CURRENT,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE) |	\
+			      BIT(IIO_CHAN_INFO_OFFSET),	\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+	.output = 1,						\
+	.indexed = 1,						\
+	.channel = _channel,					\
+	.ext_info = ad5706r_ext_info,				\
+}
+
+static const struct iio_chan_spec ad5706r_channels[] = {
+	AD5706R_CHAN(0),
+	AD5706R_CHAN(1),
+	AD5706R_CHAN(2),
+	AD5706R_CHAN(3),
+};
+
+static int _ad5706r_setup(struct ad5706r_state *st)
+{
+	struct pwm_state ldacb_pwm_state;
+	struct device *dev = &st->spi->dev;
+	int ret;
+	int i;
+
+	guard(mutex)(&st->lock);
+
+	st->debug_streaming_len = 0;
+	st->debug_streaming_data = 0;
+	st->debug_streaming_addr = 0;
+	st->debug_spi_speed_hz_write = 10000000;
+	st->debug_spi_speed_hz_read = 10000000;
+
+	st->dev_addr = 0x00;
+	st->addr_ascension = ADDR_ASCENSION_DECREMENT;
+	st->single_instr = SINGLE_INSTR_STREAMING;
+	st->shift_val = 0;
+	st->addr_desc = 1;
+	st->hw_ldac_tg_state = HW_LDAC_TG_STATE_LOW;
+	st->sampling_frequency = 1000000;
+	st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_DISABLED;
+	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
+	st->multi_dac_input_a = 0;
+	st->reference_volts = 2500;
+	st->ref_select = REF_SELECT_EXTERNAL;
+	st->hw_shutdown_state = HW_SHUTDOWN_STATE_LOW;
+
+	for (i = 0; i < 4; i++) {
+		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
+		st->range_sel[i] = RANGE_SEL_50;
+		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
+		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
+		st->toggle_trigger_chn[i] = TOGGLE_TRIGGER_CHN_HW_TRIGGER;
+		st->dither_trigger_chn[i] = DITHER_TRIGGER_CHN_HW_TRIGGER;
+		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;
+	}
+
+	/* get spi_clk axi_clkgen, no enable as spi_engine driver enables it */
+	st->reference_clk = devm_clk_get(dev, "spi_clk");
+	if (IS_ERR(st->reference_clk))
+		return dev_err_probe(dev, PTR_ERR(st->reference_clk),
+				     "Failed to get AXI CLKGEN clock\n");
+
+	st->ldacb_pwm = devm_pwm_get(dev, "ad5706r_ldacb");
+	if (IS_ERR(st->ldacb_pwm))
+		return dev_err_probe(dev, PTR_ERR(st->ldacb_pwm),
+				     "Failed to get LDACB PWM\n");
+	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
+	ldacb_pwm_state.duty_cycle = 0;
+	ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency);
+	ldacb_pwm_state.enabled = true;
+	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to apply PWM state\n");
+
+	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb", GPIOD_OUT_LOW);
+	if (IS_ERR(st->resetb_gpio)) {
+		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
+				     "Failed to get RESET_B GPIO\n");
+	}
+
+	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->shdn_gpio)) {
+		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
+				     "Failed to get SHDN GPIO\n");
+	}
+
+	/*
+	 * Get SPI max speed from device tree. Allows up to 100MHz.
+	 * If value is taken from spi->max_speed_hz, it is capped at 25MHz.
+	 */
+	ret = device_property_read_u32(dev, "spi-max-frequency", &st->spi_max_speed_hz);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set SPI Max Speed\n");
+
+	st->spi_max_speed_hz = clamp(st->spi_max_speed_hz, SPI_MIN_SPEED_HZ, SPI_MAX_SPEED_HZ);
+
+	return 0;
+}
+
+static int ad5706r_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ad5706r_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	mutex_init(&st->lock);
+	st->spi = spi;
+
+	ret = _ad5706r_setup(st);
+	if (ret)
+		return ret;
+
+	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);
+
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	if (ret)
+		return ret;
+
+	ad5706r_debugs_init(indio_dev);
+
+	return 0;
+}
+
+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", 0 },
+	{ }
+};
+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 Driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

[PATCH 3/3] MAINTAINERS: Add entry for AD5706R DAC driver

[Alexis Czezar Torreno]

Add maintainer entry for the Analog Devices AD5706R DAC driver
and device tree binding.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1251965d70bdfa990c66966cd77f7ab52ae3385f..3d7bd98b4d1b55836e40687a9a3ac9f4935a8acb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1496,6 +1496,14 @@ 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
+F:	drivers/iio/dac/ad5706r.c
+
 ANALOG DEVICES INC AD7091R DRIVER
 M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.34.1

Re: [PATCH 3/3] MAINTAINERS: Add entry for AD5706R DAC driver

[Nuno Sá]

On Fri, 2026-02-20 at 16:02 +0800, Alexis Czezar Torreno wrote:
> Add maintainer entry for the Analog Devices AD5706R DAC driver
> and device tree binding.
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---

Hi Alexis,

Fairly sure checkpatch will complain in other patches as files are being added. Need to
speak with Jorge as I just saw this was not triggered in internal CI.

- Nuno Sá

>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1251965d70bdfa990c66966cd77f7ab52ae3385f..3d7bd98b4d1b55836e40687a9a3ac9f4935a8acb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1496,6 +1496,14 @@ 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
> +F:	drivers/iio/dac/ad5706r.c
> +
>  ANALOG DEVICES INC AD7091R DRIVER
>  M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
>  L:	linux-iio@vger.kernel.org

Re: [PATCH 0/3] Add support for AD5706R DAC

[Andy Shevchenko]

On Fri, Feb 20, 2026 at 04:02:55PM +0800, Alexis Czezar Torreno wrote:
> 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

It's ~2300 LoC file, please split by features, so the first patch brings basics
and a few followups that add the rest.

> 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

> 

No blank lines in the tag block.
(Not sure if `b4` can spread it over the series either way.)

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


-- 
With Best Regards,
Andy Shevchenko

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

[Nuno Sá]

Hi Alexis,

Some initial feedback. I guess some discussion will be needed on the ABI you have.
From a quick look it seems that at least some of it can fit in standard one and 
some might be DT.

On Fri, 2026-02-20 at 16:02 +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
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  drivers/iio/dac/Kconfig   |   11 +
>  drivers/iio/dac/Makefile  |    1 +
>  drivers/iio/dac/ad5706r.c | 2290 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 2302 insertions(+)
> 

...

> 
> +
> +static void ad5706r_debugs_init(struct iio_dev *indio_dev)
> +{
> +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> +

It should have:

if (!IS_ENABLED(CONFIG_DEBUGFS))
	return

> +	debugfs_create_file_unsafe("streaming_addr", 0600, d,
> +				   indio_dev, &ad5706r_streaming_addr_fops);
> +	debugfs_create_file_unsafe("streaming_len", 0600, d,
> +				   indio_dev, &ad5706r_streaming_len_fops);
> +	debugfs_create_file_unsafe("streaming_data", 0600, d,
> +				   indio_dev, &ad5706r_streaming_data_fops);
> +	debugfs_create_file_unsafe("streaming_reg_access", 0600, d,
> +				   indio_dev, &ad5706r_streaming_reg_access_fops);
> +	debugfs_create_file_unsafe("spi_speed_hz_write", 0600, d,
> +				   indio_dev, &ad5706r_spi_speed_write_fops);
> +	debugfs_create_file_unsafe("spi_speed_hz_read", 0600, d,
> +				   indio_dev, &ad5706r_spi_speed_read_fops);
> +}

...

> 
> +
> +static int ad5706r_mux_out_sel_write(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_value;
> +	int ret;
> +
> +	/* Validate index */
> +	if (item >= ARRAY_SIZE(mux_out_sel_reg_values))
> +		return -EINVAL;
> +
> +	/* Convert index to register value */
> +	reg_value = mux_out_sel_reg_values[item];
> +
> +	ret = ad5706r_spi_write(st, AD5706R_REG_MUX_OUT_SEL,
> +				reg_value << st->shift_val);
> +	if (ret)
> +		return ret;
> +
> +	st->mux_out_sel = item;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_mux_out_sel_read(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	u8 reg_byte;
> +	int ret;
> +	int i;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_MUX_OUT_SEL, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	/* Extract the 8-bit value */
> +	reg_byte = (reg_val >> st->shift_val) & 0xFF;
> +
> +	/* Find which index has this register value */
> +	for (i = 0; i < ARRAY_SIZE(mux_out_sel_reg_values); i++) {
> +		if (mux_out_sel_reg_values[i] == reg_byte) {
> +			st->mux_out_sel = i;
> +			return i;  /* Return index, not register value */
> +		}
> +	}
> +
> +	/* Unknown value - default to disabled */
> +	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
> +	return MUX_OUT_SEL_DISABLED;
> +}

We already have other parts with a similar setting (in frequency with this). There it
supported in DT:

https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/devicetree/bindings/iio/frequency/adi,adf4350.yaml#L82


Any benefit for this to be a runtime toggle?

> +
> +static const struct iio_enum ad5706r_mux_out_sel_enum = {
> +	.items = mux_out_sel_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(mux_out_sel_iio_dev_attr_vals),
> +	.set = ad5706r_mux_out_sel_write,
> +	.get = ad5706r_mux_out_sel_read,
> +};
> +
> +static ssize_t ad5706r_multi_dac_input_a_write(struct iio_dev *indio_dev,
> +					       uintptr_t private, const struct iio_chan_spec
> *chan,
> +					       const char *buf, size_t len)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	ret = kstrtou32(buf, 16, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_INPUT_A,
> +				AD5706R_MASK_MULTI_DAC_INPUT_A(reg_val));
> +	if (ret)
> +		return ret;
> +
> +	return ret ? ret : len;
> +}
> +
> +static ssize_t ad5706r_multi_dac_input_a_read(struct iio_dev *indio_dev,
> +					      uintptr_t private, const struct iio_chan_spec
> *chan,
> +					      char *buf)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_INPUT_A, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "0x%lx\n", AD5706R_MASK_MULTI_DAC_INPUT_A(reg_val));
> +}
> +
> +static int ad5706r_multi_dac_sw_ldac_trigger_write(struct iio_dev *indio_dev,
> +						   const struct iio_chan_spec *chan,
> +						   unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SW_LDAC, item << st->shift_val);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_multi_dac_sw_ldac_trigger_read(struct iio_dev *indio_dev,
> +						  const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SW_LDAC, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	return reg_val;
> +}
> +
> +static const struct iio_enum ad5706r_multi_dac_sw_ldac_trigger_enum = {
> +	.items = multi_dac_sw_ldac_trigger_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(multi_dac_sw_ldac_trigger_iio_dev_attr_vals),
> +	.set = ad5706r_multi_dac_sw_ldac_trigger_write,
> +	.get = ad5706r_multi_dac_sw_ldac_trigger_read,
> +};
> +
> +static ssize_t ad5706r_reference_volts_write(struct iio_dev *indio_dev,
> +					     uintptr_t private, const struct iio_chan_spec *chan,
> +					     const char *buf, size_t len)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	ret = kstrtou32(buf, 10, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	st->reference_volts = reg_val;

Really, can we get anything the user gives us :)?

> +
> +	return ret ? ret : len;
> +}
> +
> +static ssize_t ad5706r_reference_volts_read(struct iio_dev *indio_dev,
> +					    uintptr_t private, const struct iio_chan_spec *chan,
> +					    char *buf)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +
> +	return sysfs_emit(buf, "%u\n", st->reference_volts);
> +}

Ditto for the above. For DACs, we typically don't mess with the reference level at runtime
at the device can be in control of other HW.

> +
> +static int ad5706r_ref_select_write(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ad5706r_spi_write(st, AD5706R_REG_BANDGAP_CONTROL, item);
> +	if (ret)
> +		return ret;
> +
> +	st->ref_select = item;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_ref_select_read(struct iio_dev *indio_dev,
> +				   const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_BANDGAP_CONTROL, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	if (reg_val)
> +		st->ref_select = REF_SELECT_INTERNAL;
> +	else
> +		st->ref_select = REF_SELECT_EXTERNAL;
> +
> +	return st->ref_select;
> +}
> +
> +static const struct iio_enum ad5706r_ref_select_enum = {
> +	.items = ref_select_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(ref_select_iio_dev_attr_vals),
> +	.set = ad5706r_ref_select_write,
> +	.get = ad5706r_ref_select_read,
> +};

The above does not look like a good idea IIUC. It seems this needs to be supported using
regulators.

...

> 
> +
> +static int ad5706r_output_state_read(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	u16 reg_val2;
> +	u16 reg_val3;
> +	u16 gpio_val;
> +	u16 mask_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_OPERATING_MODE, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_OUT_SWITCH_EN, &reg_val2);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_SHDN_EN, &reg_val3);
> +	if (ret)
> +		return ret;
> +
> +	mask_val = 1 << (chan->channel + st->shift_val);
> +	reg_val = mask_val & reg_val;
> +	reg_val2 = mask_val & reg_val2;
> +	reg_val3 = mask_val & reg_val3;
> +	gpio_val = gpiod_get_value_cansleep(st->shdn_gpio);
> +
> +	if (reg_val && !reg_val3)
> +		st->output_state[chan->channel] = OUTPUT_STATE_NORMAL_SW;
> +	else if (!reg_val && !reg_val2)
> +		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_SW;
> +	else if (!reg_val && reg_val2)
> +		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_GND_SW;
> +	else if (reg_val && reg_val3 && gpio_val)
> +		st->output_state[chan->channel] = OUTPUT_STATE_NORMAL_HW;
> +	else if (reg_val && !reg_val2 && reg_val3 && !gpio_val)
> +		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_TRISTATE_HW;
> +	else if (reg_val && reg_val2 && reg_val3 && !gpio_val)
> +		st->output_state[chan->channel] = OUTPUT_STATE_SHUTDOWN_TO_GND_HW;
> +
> +	return st->output_state[chan->channel];
> +}

Also think we already have some standard ABI that seems to fit the above:

https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/ABI/testing/sysfs-bus-iio#L758

> +
> +static const struct iio_enum ad5706r_output_state_enum = {
> +	.items = output_state_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(output_state_iio_dev_attr_vals),
> +	.set = ad5706r_output_state_write,
> +	.get = ad5706r_output_state_read,
> +};
> +
> +static int ad5706r_ldac_trigger_chn_write(struct iio_dev *indio_dev,
> +					  const struct iio_chan_spec *chan,
> +					  unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	bool val_func_en = 0;
> +	bool val_func_mode = 0;
> +	bool val_sync_async = 0;
> +	bool val_hw_sw = 0;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
> +	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
> +	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
> +
> +	if (item != LDAC_TRIGGER_CHN_NONE)
> +		val_sync_async = 1;	/* Write 1 LDAC_SYNC_ASYNC */
> +
> +	if (item == LDAC_TRIGGER_CHN_SW_TRIGGER)
> +		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
> +
> +	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
> +				    val_sync_async, val_hw_sw);
> +	if (ret)
> +		return ret;
> +
> +	st->ldac_trigger_chn[chan->channel] = item;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_ldac_trigger_chn_read(struct iio_dev *indio_dev,
> +					 const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +
> +	return st->ldac_trigger_chn[chan->channel];
> +}
> +
> +static const struct iio_enum ad5706r_ldac_trigger_chn_enum = {
> +	.items = ldac_trigger_chn_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(ldac_trigger_chn_iio_dev_attr_vals),
> +	.set = ad5706r_ldac_trigger_chn_write,
> +	.get = ad5706r_ldac_trigger_chn_read,
> +};
> +
> +static int ad5706r_toggle_trigger_chn_write(struct iio_dev *indio_dev,
> +					    const struct iio_chan_spec *chan,
> +					    unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	bool val_func_en = 0;
> +	bool val_func_mode = 0;
> +	bool val_sync_async = 0;
> +	bool val_hw_sw = 0;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
> +	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
> +	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
> +
> +	if (item != TOGGLE_TRIGGER_CHN_NONE)
> +		val_func_en = 1;	/* Write 1 FUNC_EN */
> +	if (item == TOGGLE_TRIGGER_CHN_SW_TRIGGER)
> +		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
> +
> +	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
> +				    val_sync_async, val_hw_sw);
> +	if (ret)
> +		return ret;
> +
> +	st->toggle_trigger_chn[chan->channel] = item;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_toggle_trigger_chn_read(struct iio_dev *indio_dev,
> +					   const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +
> +	return st->toggle_trigger_chn[chan->channel];
> +}
> +
> +static const struct iio_enum ad5706r_toggle_trigger_chn_enum = {
> +	.items = toggle_trigger_chn_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(toggle_trigger_chn_iio_dev_attr_vals),
> +	.set = ad5706r_toggle_trigger_chn_write,
> +	.get = ad5706r_toggle_trigger_chn_read,
> +};
> +
> +static int ad5706r_dither_trigger_chn_write(struct iio_dev *indio_dev,
> +					    const struct iio_chan_spec *chan,
> +					    unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	bool val_func_en = 0;
> +	bool val_func_mode = 1;
> +	bool val_sync_async = 0;
> +	bool val_hw_sw = 0;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	st->ldac_trigger_chn[chan->channel] = LDAC_TRIGGER_CHN_NONE;
> +	st->toggle_trigger_chn[chan->channel] = TOGGLE_TRIGGER_CHN_NONE;
> +	st->dither_trigger_chn[chan->channel] = DITHER_TRIGGER_CHN_NONE;
> +
> +	if (item != DITHER_TRIGGER_CHN_NONE)
> +		val_func_en = 1;	/* Write 1 FUNC_EN */
> +	if (item == DITHER_TRIGGER_CHN_SW_TRIGGER)
> +		val_hw_sw = 1;		/* Write 1 LDAC_HW_SW for SW */
> +
> +	ret = _set_reg_channel_mode(st, chan->channel, val_func_en, val_func_mode,
> +				    val_sync_async, val_hw_sw);
> +	if (ret)
> +		return ret;
> +
> +	st->dither_trigger_chn[chan->channel] = item;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_dither_trigger_chn_read(struct iio_dev *indio_dev,
> +					   const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +
> +	return st->dither_trigger_chn[chan->channel];
> +}
> +
> +static const struct iio_enum ad5706r_dither_trigger_chn_enum = {
> +	.items = dither_trigger_chn_iio_dev_attr_vals,
> +	.num_items = ARRAY_SIZE(dither_trigger_chn_iio_dev_attr_vals),
> +	.set = ad5706r_dither_trigger_chn_write,
> +	.get = ad5706r_dither_trigger_chn_read,
> +};


There are already some defined ABI for DACs which have toggle and dither modes:

https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/ABI/testing/sysfs-bus-iio-dac

Make sure to see what can be reused and what needs to be added.

> +
> +static int ad5706r_multi_dac_sel_ch_write(struct iio_dev *indio_dev,
> +					  const struct iio_chan_spec *chan,
> +					  unsigned int item)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	u16 mask_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SEL_CH, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	mask_val = BIT(chan->channel + st->shift_val);
> +	reg_val = ~mask_val & reg_val;
> +
> +	if (item == MULTI_DAC_SEL_CH_EXCLUDE)
> +		ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SEL_CH,
> +					reg_val);
> +	else
> +		ret = ad5706r_spi_write(st, AD5706R_REG_MULTI_DAC_SEL_CH,
> +					reg_val | mask_val);
> +
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_multi_dac_sel_ch_read(struct iio_dev *indio_dev,
> +					 const struct iio_chan_spec *chan)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	u16 mask_val;
> +	int ret;
> +
> +	ret = ad5706r_spi_read(st, AD5706R_REG_MULTI_DAC_SEL_CH, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	mask_val = BIT(chan->channel + st->shift_val);
> +	reg_val = mask_val & reg_val;
> +
> +	if (reg_val)
> +		st->multi_dac_sel_ch[chan->channel] = MULTI_DAC_SEL_CH_INCLUDE;
> +	else
> +		st->multi_dac_sel_ch[chan->channel] = MULTI_DAC_SEL_CH_EXCLUDE;
> +
> +	return st->multi_dac_sel_ch[chan->channel];
> +}
> +
> +static const struct iio_enum ad5706r_multi_dac_sel_ch_enum = {
> +	.items = multi_dac_sel_ch_iio_chan_attr_vals,
> +	.num_items = ARRAY_SIZE(multi_dac_sel_ch_iio_chan_attr_vals),
> +	.set = ad5706r_multi_dac_sel_ch_write,
> +	.get = ad5706r_multi_dac_sel_ch_read,
> +};
> +
> +#define AD5706R_CHAN_EXT_INFO(_name, _what, _shared, _read, _write) {	\
> +	.name = _name,							\
> +	.read = (_read),						\
> +	.write = (_write),						\
> +	.private = (_what),						\
> +	.shared = (_shared),						\
> +}
> +
> +static struct iio_chan_spec_ext_info ad5706r_ext_info[] = {
> +	/* device_attribute */
> +	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_dev_addr_read, ad5706r_dev_addr_write),
> +
> +	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
> +	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
> +
> +	IIO_ENUM("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
> +	IIO_ENUM_AVAILABLE("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
> +
> +	IIO_ENUM("hw_ldac_tg_state", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_state_enum),
> +	IIO_ENUM_AVAILABLE("hw_ldac_tg_state", IIO_SHARED_BY_ALL,
> &ad5706r_hw_ldac_tg_state_enum),
> +
> +	/* Sampling Frequency part of read/write RAW */
> +
> +	IIO_ENUM("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
> +	IIO_ENUM_AVAILABLE("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
> +
> +	IIO_ENUM("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
> +	IIO_ENUM_AVAILABLE("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
> +
> +	AD5706R_CHAN_EXT_INFO("multi_dac_input_a", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_multi_dac_input_a_read, ad5706r_multi_dac_input_a_write),
> +
> +	IIO_ENUM("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
> +		 &ad5706r_multi_dac_sw_ldac_trigger_enum),
> +	IIO_ENUM_AVAILABLE("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
> +			   &ad5706r_multi_dac_sw_ldac_trigger_enum),
> +
> +	AD5706R_CHAN_EXT_INFO("reference_volts", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_reference_volts_read, ad5706r_reference_volts_write),
> +
> +	IIO_ENUM("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
> +	IIO_ENUM_AVAILABLE("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
> +
> +	IIO_ENUM("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),
> +	IIO_ENUM_AVAILABLE("hw_shutdown_state", IIO_SHARED_BY_ALL,
> &ad5706r_hw_shutdown_state_enum),
> +
> +	/* Channel Attributes */
> +	AD5706R_CHAN_EXT_INFO("input_register_a", 0, IIO_SEPARATE,
> +			      ad5706r_input_register_a_read, ad5706r_input_register_a_write),
> +
> +	AD5706R_CHAN_EXT_INFO("input_register_b", 0, IIO_SEPARATE,
> +			      ad5706r_input_register_b_read, ad5706r_input_register_b_write),
> +
> +	IIO_ENUM("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
> +	IIO_ENUM_AVAILABLE("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
> +
> +	IIO_ENUM("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
> +	IIO_ENUM_AVAILABLE("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
> +
> +	IIO_ENUM("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
> +	IIO_ENUM_AVAILABLE("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
> +
> +	IIO_ENUM("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
> +
> +	IIO_ENUM("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
> +
> +	IIO_ENUM("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
> +
> +	IIO_ENUM("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),
> +	IIO_ENUM_AVAILABLE("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),

Oh boy, not going through all of the above now but that's a lot of custom ABI. It definitely needs
and ABI doc explaining what's being done.

> +
> +	{},
> +};
> +
> +/* Channel */
> +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);
> +	u16 reg_val;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		scoped_guard(mutex, &st->lock) {
> +			ret = ad5706r_spi_read(st, AD5706R_REG_DAC_DATA_READBACK_CH(chan-
> >channel),
> +					       &reg_val);
> +
> +			if (ret)
> +				return ret;
> +
> +			*val = reg_val;
> +		}
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (st->range_sel[chan->channel]) {
> +		case RANGE_SEL_50:
> +			*val = 50 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_150:
> +			*val = 150 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_200:
> +			*val = 200 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_300:

Same comment about the prefix.

> +		default:
> +			*val = 300 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		}
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 0;
> +		return IIO_VAL_INT;

If offset is 0 we should not need it.

> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->sampling_frequency;
> +		return IIO_VAL_INT;
> +	}
> +
> +	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);
> +	struct pwm_state ldacb_pwm_state;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		/* Sets minimum and maximum frequency */
> +		val = clamp(val, SAMPLING_FREQUENCY_MIN_HZ, SAMPLING_FREQUENCY_MAX_HZ);


Typically the macros should have the device name as prefix.

> +
> +		scoped_guard(mutex, &st->lock) {
> +			pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> +			ldacb_pwm_state.duty_cycle = DIV_ROUND_CLOSEST_ULL(NANO, 2 * val);
> +			ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, val);
> +			ldacb_pwm_state.enabled = true;
> +
> +			ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> +			if (ret)
> +				return ret;
> +
> +			st->sampling_frequency = val;
> +		}
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info ad5706r_info = {
> +	.read_raw = &ad5706r_read_raw,
> +	.write_raw = &ad5706r_write_raw,
> +	.debugfs_reg_access = &ad5706r_reg_access,
> +};
> +
> +#define AD5706R_CHAN(_channel) {				\
> +	.type = IIO_CURRENT,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE) |	\
> +			      BIT(IIO_CHAN_INFO_OFFSET),	\
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
> +	.output = 1,						\
> +	.indexed = 1,						\
> +	.channel = _channel,					\
> +	.ext_info = ad5706r_ext_info,				\
> +}
> +
> +static const struct iio_chan_spec ad5706r_channels[] = {
> +	AD5706R_CHAN(0),
> +	AD5706R_CHAN(1),
> +	AD5706R_CHAN(2),
> +	AD5706R_CHAN(3),
> +};
> +
> +static int _ad5706r_setup(struct ad5706r_state *st)
> +{
> +	struct pwm_state ldacb_pwm_state;
> +	struct device *dev = &st->spi->dev;
> +	int ret;
> +	int i;
> +
> +	guard(mutex)(&st->lock);

Hmm, why the above?

> +
> +	st->debug_streaming_len = 0;
> +	st->debug_streaming_data = 0;
> +	st->debug_streaming_addr = 0;

The above should not be needed.

> +	st->debug_spi_speed_hz_write = 10000000;
> +	st->debug_spi_speed_hz_read = 10000000;
> +
> +	st->dev_addr = 0x00;
> +	st->addr_ascension = ADDR_ASCENSION_DECREMENT;
> +	st->single_instr = SINGLE_INSTR_STREAMING;
> +	st->shift_val = 0;
> +	st->addr_desc = 1;
> +	st->hw_ldac_tg_state = HW_LDAC_TG_STATE_LOW;
> +	st->sampling_frequency = 1000000;
> +	st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_DISABLED;
> +	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
> +	st->multi_dac_input_a = 0;
> +	st->reference_volts = 2500;
> +	st->ref_select = REF_SELECT_EXTERNAL;
> +	st->hw_shutdown_state = HW_SHUTDOWN_STATE_LOW;
> +
> +	for (i = 0; i < 4; i++) {
> +		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
> +		st->range_sel[i] = RANGE_SEL_50;
> +		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
> +		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
> +		st->toggle_trigger_chn[i] = TOGGLE_TRIGGER_CHN_HW_TRIGGER;
> +		st->dither_trigger_chn[i] = DITHER_TRIGGER_CHN_HW_TRIGGER;
> +		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;
> +	}
> +
> +	/* get spi_clk axi_clkgen, no enable as spi_engine driver enables it */
> +	st->reference_clk = devm_clk_get(dev, "spi_clk");
> +	if (IS_ERR(st->reference_clk))
> +		return dev_err_probe(dev, PTR_ERR(st->reference_clk),
> +				     "Failed to get AXI CLKGEN clock\n");
> +
> +	st->ldacb_pwm = devm_pwm_get(dev, "ad5706r_ldacb");
> +	if (IS_ERR(st->ldacb_pwm))
> +		return dev_err_probe(dev, PTR_ERR(st->ldacb_pwm),
> +				     "Failed to get LDACB PWM\n");

nit: new line

> +	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> +	ldacb_pwm_state.duty_cycle = 0;
> +	ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency);
> +	ldacb_pwm_state.enabled = true;
> +	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to apply PWM state\n");
> +
> +	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->resetb_gpio)) {
> +		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
> +				     "Failed to get RESET_B GPIO\n");
> +	}


reset gpios can now be handled using the reset subsystem.

> +
> +	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn", GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->shdn_gpio)) {
> +		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
> +				     "Failed to get SHDN GPIO\n");
> +	}
> +
> +	/*
> +	 * Get SPI max speed from device tree. Allows up to 100MHz.
> +	 * If value is taken from spi->max_speed_hz, it is capped at 25MHz.
> +	 */
> +	ret = device_property_read_u32(dev, "spi-max-frequency", &st->spi_max_speed_hz);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to set SPI Max Speed\n");
> +
> +	st->spi_max_speed_hz = clamp(st->spi_max_speed_hz, SPI_MIN_SPEED_HZ, SPI_MAX_SPEED_HZ);
> +

Hmm, why do we need the above? The spi core should handle it. Why is it capped? Maybe because
the controller you tested this on can't handle it?

> +	return 0;
> +}
> +
> +static int ad5706r_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ad5706r_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);

These days, devm_mutex_init()

> +	st->spi = spi;
> +
> +	ret = _ad5706r_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	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);
> +
> +	ret = devm_iio_device_register(&spi->dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ad5706r_debugs_init(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ad5706r_of_match[] = {
> +	{ .compatible = "adi,ad5706r" },
> +	{ },
> +};

No need for comma.

> +MODULE_DEVICE_TABLE(of, ad5706r_of_match);
> +
> +static const struct spi_device_id ad5706r_id[] = {
> +	{ "ad5706r", 0 },
> +	{ }
> +};


Drop the 0.

- Nuno Sá
> ;

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

[Uwe Kleine-König]

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

Hello,

On Fri, Feb 20, 2026 at 04:02:57PM +0800, Alexis Czezar Torreno wrote:
> +static int _set_pwm_duty_cycle(struct ad5706r_state *st, int duty_cycle)
> +{
> +	struct pwm_state ldacb_pwm_state;
> +	int ret;
> +
> +	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> +
> +	ldacb_pwm_state.duty_cycle = duty_cycle == 0 ? 0 :
> +		DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency * 100 / duty_cycle);

This being integer math it would benefit when simplifying that to

	10000000 * duty_cycle / st->sampling_frequency

. Consider st->sampling_frequency = 667 and duty_cycle = 99:

	NANO / (st->sampling_frequency * 100 / duty_cycle) = 1484257.8710644676
	DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency * 100 / duty_cycle) = 1485884
	DIV_ROUND_CLOSEST_ULL(10000000 * duty_cycle, st->sampling_frequency) = 1484258

With duty_cycle <= 100 this doesn't even overflow.

> +	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Device Attributes */
> +static ssize_t ad5706r_dev_addr_write(struct iio_dev *indio_dev,
> +				      uintptr_t private, const struct iio_chan_spec *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	ret = kstrtou32(buf, 10, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	st->dev_addr = AD5706R_MASK_DEV_ADDR(reg_val);
> +
> +	return ret ? ret : len;

This can be written as:

	return ret ?: len;

It's a matter of taste which one you like better.

> +}
> +

Best regards
Uwe

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

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

[Andy Shevchenko]

On Fri, Feb 20, 2026 at 10:48:59AM +0000, Nuno Sá wrote:
> On Fri, 2026-02-20 at 16:02 +0800, Alexis Czezar Torreno wrote:

...

> > +static void ad5706r_debugs_init(struct iio_dev *indio_dev)
> > +{
> > +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> 
> It should have:
> 
> if (!IS_ENABLED(CONFIG_DEBUGFS))
> 	return

But why? The debugfs is a stub when disabled, nobody should do that
in the cases when the main purpose is not the debugfs code.

> > +	debugfs_create_file_unsafe("streaming_addr", 0600, d,
> > +				   indio_dev, &ad5706r_streaming_addr_fops);
> > +	debugfs_create_file_unsafe("streaming_len", 0600, d,
> > +				   indio_dev, &ad5706r_streaming_len_fops);
> > +	debugfs_create_file_unsafe("streaming_data", 0600, d,
> > +				   indio_dev, &ad5706r_streaming_data_fops);
> > +	debugfs_create_file_unsafe("streaming_reg_access", 0600, d,
> > +				   indio_dev, &ad5706r_streaming_reg_access_fops);
> > +	debugfs_create_file_unsafe("spi_speed_hz_write", 0600, d,
> > +				   indio_dev, &ad5706r_spi_speed_write_fops);
> > +	debugfs_create_file_unsafe("spi_speed_hz_read", 0600, d,
> > +				   indio_dev, &ad5706r_spi_speed_read_fops);
> > +}

...

> > +	/* Find which index has this register value */
> > +	for (i = 0; i < ARRAY_SIZE(mux_out_sel_reg_values); i++) {

	for (size_t i...)

> > +		if (mux_out_sel_reg_values[i] == reg_byte) {
> > +			st->mux_out_sel = i;
> > +			return i;  /* Return index, not register value */
> > +		}
> > +	}

...

> > +	return ret ? ret : len;

Use Elvis operator

	return ret ?: len;

...


> > +	{},

IIO has a style for terminator entry, along with confusing trailing comma.
If it's a sentinel, it must be one even at a compile time.

> > +};

...

> > +	st->debug_spi_speed_hz_write = 10000000;
> > +	st->debug_spi_speed_hz_read = 10000000;

units.h and other headers for your help

10 * HZ_PER_MHZ

...

> > +	st->sampling_frequency = 1000000;

In the similar way.

...

> > +	st->reference_volts = 2500;

2.5kV?! I think you mistakenly put volts where should be _mV

...

> > +	for (i = 0; i < 4; i++) {

Magic 4.

> > +		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
> > +		st->range_sel[i] = RANGE_SEL_50;
> > +		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
> > +		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
> > +		st->toggle_trigger_chn[i] = TOGGLE_TRIGGER_CHN_HW_TRIGGER;
> > +		st->dither_trigger_chn[i] = DITHER_TRIGGER_CHN_HW_TRIGGER;
> > +		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;

Hmm... Perhaps memsetXX()? But original loop with the defined iterator will be
okay:

	for (unsigned int i = 0; i < $MAGIC_CONST; i++) {

> > +	}

...

> > +	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb", GPIOD_OUT_LOW);
> > +	if (IS_ERR(st->resetb_gpio)) {
> > +		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
> > +				     "Failed to get RESET_B GPIO\n");
> > +	}

> > +	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn", GPIOD_OUT_HIGH);
> > +	if (IS_ERR(st->shdn_gpio)) {
> > +		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
> > +				     "Failed to get SHDN GPIO\n");
> > +	}

The {} are not needed when the body is a single call.

...

> > +static const struct of_device_id ad5706r_of_match[] = {
> > +	{ .compatible = "adi,ad5706r" },
> > +	{ },

See above about terminator entry style.

> > +};

-- 
With Best Regards,
Andy Shevchenko

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

[Nuno Sá]

On Fri, 2026-02-20 at 13:00 +0200, Andy Shevchenko wrote:
> On Fri, Feb 20, 2026 at 10:48:59AM +0000, Nuno Sá wrote:
> > On Fri, 2026-02-20 at 16:02 +0800, Alexis Czezar Torreno wrote:
> 
> ...
> 
> > > +static void ad5706r_debugs_init(struct iio_dev *indio_dev)
> > > +{
> > > +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> > 
> > It should have:
> > 
> > if (!IS_ENABLED(CONFIG_DEBUGFS))
> > 	return
> 
> But why? The debugfs is a stub when disabled, nobody should do that
> in the cases when the main purpose is not the debugfs code.

Because the compiler can then optimize away all of the above code...

- Nuno Sá

> 
> > > +	debugfs_create_file_unsafe("streaming_addr", 0600, d,
> > > +				   indio_dev, &ad5706r_streaming_addr_fops);
> > > +	debugfs_create_file_unsafe("streaming_len", 0600, d,
> > > +				   indio_dev, &ad5706r_streaming_len_fops);
> > > +	debugfs_create_file_unsafe("streaming_data", 0600, d,
> > > +				   indio_dev, &ad5706r_streaming_data_fops);
> > > +	debugfs_create_file_unsafe("streaming_reg_access", 0600, d,
> > > +				   indio_dev, &ad5706r_streaming_reg_access_fops);
> > > +	debugfs_create_file_unsafe("spi_speed_hz_write", 0600, d,
> > > +				   indio_dev, &ad5706r_spi_speed_write_fops);
> > > +	debugfs_create_file_unsafe("spi_speed_hz_read", 0600, d,
> > > +				   indio_dev, &ad5706r_spi_speed_read_fops);
> > > +}
> 
> ...
> 
> > > +	/* Find which index has this register value */
> > > +	for (i = 0; i < ARRAY_SIZE(mux_out_sel_reg_values); i++) {
> 
> 	for (size_t i...)
> 
> > > +		if (mux_out_sel_reg_values[i] == reg_byte) {
> > > +			st->mux_out_sel = i;
> > > +			return i;  /* Return index, not register value */
> > > +		}
> > > +	}
> 
> ...
> 
> > > +	return ret ? ret : len;
> 
> Use Elvis operator
> 
> 	return ret ?: len;
> 
> ...
> 
> 
> > > +	{},
> 
> IIO has a style for terminator entry, along with confusing trailing comma.
> If it's a sentinel, it must be one even at a compile time.
> 
> > > +};
> 
> ...
> 
> > > +	st->debug_spi_speed_hz_write = 10000000;
> > > +	st->debug_spi_speed_hz_read = 10000000;
> 
> units.h and other headers for your help
> 
> 10 * HZ_PER_MHZ
> 
> ...
> 
> > > +	st->sampling_frequency = 1000000;
> 
> In the similar way.
> 
> ...
> 
> > > +	st->reference_volts = 2500;
> 
> 2.5kV?! I think you mistakenly put volts where should be _mV
> 
> ...
> 
> > > +	for (i = 0; i < 4; i++) {
> 
> Magic 4.
> 
> > > +		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
> > > +		st->range_sel[i] = RANGE_SEL_50;
> > > +		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
> > > +		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
> > > +		st->toggle_trigger_chn[i] = TOGGLE_TRIGGER_CHN_HW_TRIGGER;
> > > +		st->dither_trigger_chn[i] = DITHER_TRIGGER_CHN_HW_TRIGGER;
> > > +		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;
> 
> Hmm... Perhaps memsetXX()? But original loop with the defined iterator will be
> okay:
> 
> 	for (unsigned int i = 0; i < $MAGIC_CONST; i++) {
> 
> > > +	}
> 
> ...
> 
> > > +	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(st->resetb_gpio)) {
> > > +		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
> > > +				     "Failed to get RESET_B GPIO\n");
> > > +	}
> 
> > > +	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn", GPIOD_OUT_HIGH);
> > > +	if (IS_ERR(st->shdn_gpio)) {
> > > +		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
> > > +				     "Failed to get SHDN GPIO\n");
> > > +	}
> 
> The {} are not needed when the body is a single call.
> 
> ...
> 
> > > +static const struct of_device_id ad5706r_of_match[] = {
> > > +	{ .compatible = "adi,ad5706r" },
> > > +	{ },
> 
> See above about terminator entry style.
> 
> > > +};

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

[Andy Shevchenko]

On Fri, Feb 20, 2026 at 03:02:37PM +0000, Nuno Sá wrote:
> On Fri, 2026-02-20 at 13:00 +0200, Andy Shevchenko wrote:
> > On Fri, Feb 20, 2026 at 10:48:59AM +0000, Nuno Sá wrote:
> > > On Fri, 2026-02-20 at 16:02 +0800, Alexis Czezar Torreno wrote:

...

> > > > +static void ad5706r_debugs_init(struct iio_dev *indio_dev)
> > > > +{
> > > > +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> > > 
> > > It should have:
> > > 
> > > if (!IS_ENABLED(CONFIG_DEBUGFS))
> > > 	return
> > 
> > But why? The debugfs is a stub when disabled, nobody should do that
> > in the cases when the main purpose is not the debugfs code.
> 
> Because the compiler can then optimize away all of the above code...

How is it different to the code elimination part that is inside in each of
the below calls?

> > > > +	debugfs_create_file_unsafe("streaming_addr", 0600, d,
> > > > +				   indio_dev, &ad5706r_streaming_addr_fops);
> > > > +	debugfs_create_file_unsafe("streaming_len", 0600, d,
> > > > +				   indio_dev, &ad5706r_streaming_len_fops);
> > > > +	debugfs_create_file_unsafe("streaming_data", 0600, d,
> > > > +				   indio_dev, &ad5706r_streaming_data_fops);
> > > > +	debugfs_create_file_unsafe("streaming_reg_access", 0600, d,
> > > > +				   indio_dev, &ad5706r_streaming_reg_access_fops);
> > > > +	debugfs_create_file_unsafe("spi_speed_hz_write", 0600, d,
> > > > +				   indio_dev, &ad5706r_spi_speed_write_fops);
> > > > +	debugfs_create_file_unsafe("spi_speed_hz_read", 0600, d,
> > > > +				   indio_dev, &ad5706r_spi_speed_read_fops);
> > > > +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] dt-bindings: iio: dac: Add binding for AD5706R

[Krzysztof Kozlowski]

On Fri, Feb 20, 2026 at 04:02:56PM +0800, Alexis Czezar Torreno wrote:
> Add device tree binding documentation for the Analog Devices
> AD5706R 4-channel 16-bit current output digital-to-analog converter.

A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5706r.yaml   | 96 ++++++++++++++++++++++
>  1 file changed, 96 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..dabaf2195a07d2c66d44f69ca60e32e6bc37cf55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
> @@ -0,0 +1,96 @@
> +# 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 16-bit, 4-channel current output digital-to-analog
> +  converter with SPI interface.
> +
> +  The driver supports dynamic SPI clock rate management via an external

So this is DAC or driver? If driver, what does it drive?

> +  clock generator (e.g., AXI CLKGEN) referenced by the 'clocks' property.
> +  This allows separate read and write SPI speeds to be configured at
> +  runtime.

How binding has anything to do with runtime? What are you describing
here?

> +
> +  Datasheet:
> +    https://www.analog.com/en/products/ad5706r.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad5706r
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:

So that's SPI then you miss $ref and unevaluatedProps. Look at other
bindings.

> +    maximum: 100000000
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Reference clock for SPI clock rate management.
> +
> +  clock-names:
> +    items:
> +      - const: spi_clk

Drop clock names, pretty redundant.

> +
> +  pwms:
> +    maxItems: 1
> +
> +  pwm-names:
> +    items:
> +      - const: ad5706r_ldacb

No point to add device name in the input name.

> +
> +  dac-resetb-gpios:

So reset-gpios?

> +    maxItems: 1
> +    description: GPIO connected to the active-low reset pin.
> +
> +  dac-shdn-gpios:

Please read gpio-consumer-common.yaml

> +    maxItems: 1
> +    description: GPIO connected to the active-high shutdown pin.
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - clocks
> +  - clock-names
> +  - pwms
> +  - pwm-names
> +  - dac-resetb-gpios
> +  - dac-shdn-gpios

No supplies? How does it get power or reference voltage?

> +

Here, missing allOf with ref


> +additionalProperties: false

And that is unevaluated.


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: iio: dac: Add binding for AD5706R

[David Lechner]

On 2/20/26 2:02 AM, Alexis Czezar Torreno wrote:
> 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>
> ---
>  .../devicetree/bindings/iio/dac/adi,ad5706r.yaml   | 96 ++++++++++++++++++++++
>  1 file changed, 96 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..dabaf2195a07d2c66d44f69ca60e32e6bc37cf55
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
> @@ -0,0 +1,96 @@
> +# 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 16-bit, 4-channel current output digital-to-analog
> +  converter with SPI interface.
> +
> +  The driver supports dynamic SPI clock rate management via an external
> +  clock generator (e.g., AXI CLKGEN) referenced by the 'clocks' property.
> +  This allows separate read and write SPI speeds to be configured at
> +  runtime.

Devicetree is for describing how it is wired up, not how the driver uses
it, so leave this part out.

> +
> +  Datasheet:
> +    https://www.analog.com/en/products/ad5706r.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad5706r
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 100000000
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Reference clock for SPI clock rate management.

Based on the driver, it sounds like this clock is not connected
to this chip at all, but rather the SPI controller clock. So it
does not belong here.

> +
> +  clock-names:
> +    items:
> +      - const: spi_clk
> +
> +  pwms:
> +    maxItems: 1
> +
> +  pwm-names:
> +    items:
> +      - const: ad5706r_ldacb

Instead of giving it a name, we can just write it as:

pwms:
  items:
    - description: PWM connected to the LDAC/TGP/DCK pin.

The items: also limits it to exactly one item, so we don't need
maxItems:.

> +
> +  dac-resetb-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the active-low reset pin.

The datasheet just has a /RESET pin. Not sure where the "b" comes
from.

> +
> +  dac-shdn-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the active-high shutdown pin.

I didn't see a "shutdown" pin in the datasheet. Do you mean the
OUT_EN pin?

> +

We could also add an io-channel for reading back the MUX_OUT pin
from the ADC it is connected to.

> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency

Voltage supplies should be required.


> +  - clocks
> +  - clock-names
> +  - pwms
> +  - pwm-names
> +  - dac-resetb-gpios
> +  - dac-shdn-gpios

I would not expect any of these to be required. Reset could be hardwired if
there aren't enough GPIOs. And something other than a PWM could be connected
to the LDAC/TGP/DCK pin. 

The point of the bindings is to describe how the chip is wired up, and that
includes any way it could possibly be wired up, not just what is implemented
in the driver.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad5706r";
> +            reg = <0>;
> +            spi-max-frequency = <100000000>;
> +
> +            clocks = <&spi_clk>;
> +            clock-names = "spi_clk";
> +
> +            dac-resetb-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
> +            dac-shdn-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
> +
> +            pwms = <&axi_pwm_gen 0 0>;
> +            pwm-names = "ad5706r_ldacb";
> +        };
> +    };
> +...
> 

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

[David Lechner]

On 2/20/26 2:02 AM, 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
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> ---
>  drivers/iio/dac/Kconfig   |   11 +
>  drivers/iio/dac/Makefile  |    1 +
>  drivers/iio/dac/ad5706r.c | 2290 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 2302 insertions(+)

This is way too much to try to review in one go. Aim for 500 lines per
patch. If it gets close to 1000, it's time to start thinking about splitting
it up.

And if you can split it up into sepaerate series, you will get even better
reviews. We only have so much time, so the fewer lines sent at a time, the
more time we can spend on each line.

It looks like this has a bunch of unusal features, so I would suggest to
drop all of that for now and just start with a basic driver. Once that
gets merged, we can start looking at the unusual stuff.

> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index db9f5c711b3df90641f017652fbbef594cc1627d..20be74a2933049250bab779d12ecd2b9b1f5a2a7 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -178,6 +178,17 @@ 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
> +	select IIO_BUFFER
> +	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..2d718cf7300bcd1f599fe715aacb3170f72541af
> --- /dev/null
> +++ b/drivers/iio/dac/ad5706r.c
> @@ -0,0 +1,2290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AD5706R 16-bit Current Output Digital to Analog Converter
> + *
> + * Copyright 2026 Analog Devices Inc.
> + *
> + * This driver is designed for use with the AXI SPI Engine and AXI CLKGEN
> + * on Xilinx Zynq platforms. The 'clocks' device tree property references
> + * the AXI CLKGEN output clock, which is used to dynamically control the
> + * SPI clock rate for read and write operations independently.

Normally, we would just use .speed_hz in struct spi_transfer to specify
the speed if we need different speeds for different operations. Any
reason we can't do that here?

If there is a reason, it needs more explanation. In any case, I don't
think that changing the clock rate of the SPI controller like this without
going through the SPI subsystem is going to be acceptble.

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

[Jonathan Cameron]

On Fri, 20 Feb 2026 16:02:57 +0800
Alexis Czezar Torreno <alexisczezar.torreno@analog.com> 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
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

Hi Alexis

Welcome to IIO.  A few quick comments inline, but as others have observed
the patch is too large and needs breaking up.  Also there is a lot of custom
ABI.  Given we are reluctant to merge that at all, it definitely needs
documentation and careful consideration of whether it can be done with existing
ABI or is more appropriate in DT.

Thanks,

Jonathan

> diff --git a/drivers/iio/dac/ad5706r.c b/drivers/iio/dac/ad5706r.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2d718cf7300bcd1f599fe715aacb3170f72541af
> --- /dev/null
> +++ b/drivers/iio/dac/ad5706r.c
> @@ -0,0 +1,2290 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AD5706R 16-bit Current Output Digital to Analog Converter
> + *
> + * Copyright 2026 Analog Devices Inc.
> + *
> + * This driver is designed for use with the AXI SPI Engine and AXI CLKGEN
> + * on Xilinx Zynq platforms. The 'clocks' device tree property references
> + * the AXI CLKGEN output clock, which is used to dynamically control the
> + * SPI clock rate for read and write operations independently.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/module.h>
Follow approximate include what you use principles for kernel drivers.
That means mostly including everything that is used directly with exception
of a few headers that we know will always be included by others (usually
because they are documented as doing so).

e.g. mutex.h should always be in a driver that uses a mutex.
If you are using guards, then cleanup.h needs to be there etc.

> +#include <linux/pwm.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +/* SPI Defines */
> +#define AD5706R_RD_MASK		BIT(15)
> +#define AD5706R_ADDR_PIN_MASK	GENMASK(14, 12)
> +#define AD5706R_ADDR_MASK	GENMASK(11, 0)
> +#define AD5706R_VAL_MASK	GENMASK(7, 0)
> +
> +/* Registers and Masks */
> +#define AD5706R_MASK_RESET			(BIT(7) | BIT(0))
> +#define AD5706R_MASK_DEV_ADDR(x)		((x) & GENMASK(2, 0))

Define the mask only then use FIELD_GET() / FIELD_PREP() in the code.
That tends to end up more readable than a macro doing the same thing
without the compile time checks those bring on values fitting etc.

> +
> +#define NUM_CHANNELS			4
> +#define SPI_MAX_SPEED_HZ		(100 * HZ_PER_MHZ)	/* 100 MHz */
> +#define SPI_MIN_SPEED_HZ		(3 * HZ_PER_MHZ)	/* 3 MHz */

Very generic names.  These are going to clash with something in another header
at somepoint.  Either push the values into a helper, inline or prefix the
defines to avoid that naming clash.


> +/*
> + * Order of attributes in code:

I'm not sure how this comment helps us.

> + *
> + * Device Attributes:
> + *   - dev_addr
> + *   - addr_ascension
> + *   - single_instr
> + *   - hw_ldac_tg_state
> + *   - sampling_frequency
> + *   - hw_ldac_tg_pwm
> + *   - mux_out_sel
> + *   - multi_dac_input_a
> + *   - multi_dac_sw_ldac_trigger
> + *   - reference_volts
> + *   - ref_select
> + *   - hw_shutdown_state
> + *
> + * Channel Attributes:
> + *   - raw
> + *   - scale
> + *   - offset
> + *   - input_register_a
> + *   - input_register_b
> + *   - hw_active_edge
> + *   - range_sel
> + *   - output_state
> + *   - ldac_trigger_chn
> + *   - toggle_trigger_chn
> + *   - dither_trigger_chn
> + *   - multi_dac_sel_ch
> + */

> +
> +/* ENUM Lists */
Don't give generic comments like this on how you've laid the code
out. They just tend to end up wrong after new features are added
and don't bring much value in the first place.
> +enum addr_ascension_iio_dev_attr {


> +static struct iio_chan_spec_ext_info ad5706r_ext_info[] = {
> +	/* device_attribute */
> +	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_dev_addr_read, ad5706r_dev_addr_write),
> +
> +	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
> +	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL, &ad5706r_addr_ascension_enum),
> +
> +	IIO_ENUM("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
> +	IIO_ENUM_AVAILABLE("single_instr", IIO_SHARED_BY_ALL, &ad5706r_single_instr_enum),
> +
> +	IIO_ENUM("hw_ldac_tg_state", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_state_enum),
> +	IIO_ENUM_AVAILABLE("hw_ldac_tg_state", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_state_enum),
> +
> +	/* Sampling Frequency part of read/write RAW */
> +
> +	IIO_ENUM("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
> +	IIO_ENUM_AVAILABLE("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL, &ad5706r_hw_ldac_tg_pwm_enum),
> +
> +	IIO_ENUM("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
> +	IIO_ENUM_AVAILABLE("mux_out_sel", IIO_SHARED_BY_ALL, &ad5706r_mux_out_sel_enum),
> +
> +	AD5706R_CHAN_EXT_INFO("multi_dac_input_a", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_multi_dac_input_a_read, ad5706r_multi_dac_input_a_write),
> +
> +	IIO_ENUM("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
> +		 &ad5706r_multi_dac_sw_ldac_trigger_enum),
> +	IIO_ENUM_AVAILABLE("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
> +			   &ad5706r_multi_dac_sw_ldac_trigger_enum),
> +
> +	AD5706R_CHAN_EXT_INFO("reference_volts", 0, IIO_SHARED_BY_ALL,
> +			      ad5706r_reference_volts_read, ad5706r_reference_volts_write),
> +
> +	IIO_ENUM("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
> +	IIO_ENUM_AVAILABLE("ref_select", IIO_SHARED_BY_ALL, &ad5706r_ref_select_enum),
> +
> +	IIO_ENUM("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),
> +	IIO_ENUM_AVAILABLE("hw_shutdown_state", IIO_SHARED_BY_ALL, &ad5706r_hw_shutdown_state_enum),
> +
> +	/* Channel Attributes */
> +	AD5706R_CHAN_EXT_INFO("input_register_a", 0, IIO_SEPARATE,
> +			      ad5706r_input_register_a_read, ad5706r_input_register_a_write),
> +
> +	AD5706R_CHAN_EXT_INFO("input_register_b", 0, IIO_SEPARATE,
> +			      ad5706r_input_register_b_read, ad5706r_input_register_b_write),
> +
> +	IIO_ENUM("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
> +	IIO_ENUM_AVAILABLE("hw_active_edge", IIO_SEPARATE, &ad5706r_hw_active_edge_enum),
> +
> +	IIO_ENUM("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
> +	IIO_ENUM_AVAILABLE("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
> +
> +	IIO_ENUM("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
> +	IIO_ENUM_AVAILABLE("output_state", IIO_SEPARATE, &ad5706r_output_state_enum),
> +
> +	IIO_ENUM("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("ldac_trigger_chn", IIO_SEPARATE, &ad5706r_ldac_trigger_chn_enum),
> +
> +	IIO_ENUM("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("toggle_trigger_chn", IIO_SEPARATE, &ad5706r_toggle_trigger_chn_enum),
> +
> +	IIO_ENUM("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
> +	IIO_ENUM_AVAILABLE("dither_trigger_chn", IIO_SEPARATE, &ad5706r_dither_trigger_chn_enum),
> +
> +	IIO_ENUM("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),
> +	IIO_ENUM_AVAILABLE("multi_dac_sel_ch", IIO_SEPARATE, &ad5706r_multi_dac_sel_ch_enum),
> +

Others have pointed out that custom ABI has various basic problems.

1) Generic userspace has no way to know how to use it. As such, it gets little used
and must not be required for basic functionality.  So don't bring it in as part of
the initial patch, but instead do it as a follow up patch (can be in same series
if the series remains reasonably sized)
2) It needs full documentation for us to review it.  In
Documentation/ABI/testing/sysfs-bus-iio-driver-name

> +	{},
> +};
> +
> +/* Channel */
I'm not sure what that comment means. I'd drop it.
> +static int ad5706r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
Put multiple parameters on one line as long as we stay under 80 chars
(or a bit over if it helps readability is fine as well)

> +			    long mask)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		scoped_guard(mutex, &st->lock) {

As below. Add scope with { } and use guard()
That will reduce the very long lines here.

> +			ret = ad5706r_spi_read(st, AD5706R_REG_DAC_DATA_READBACK_CH(chan->channel),
> +					       &reg_val);
> +
> +			if (ret)
> +				return ret;
> +
> +			*val = reg_val;
> +		}
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (st->range_sel[chan->channel]) {
> +		case RANGE_SEL_50:
> +			*val = 50 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_150:
> +			*val = 150 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_200:
> +			*val = 200 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		case RANGE_SEL_300:
> +		default:

A default often doesn't make much sense. How did you get a different value
from the ones being explicitly set.

> +			*val = 300 * HZ_PER_MHZ / AD5706R_DAC_MAX_CODE;
> +			break;
> +		}
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 0;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = st->sampling_frequency;
> +		return IIO_VAL_INT;
> +	}
> +
> +	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);
> +	struct pwm_state ldacb_pwm_state;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		/* Sets minimum and maximum frequency */
> +		val = clamp(val, SAMPLING_FREQUENCY_MIN_HZ, SAMPLING_FREQUENCY_MAX_HZ);
> +
> +		scoped_guard(mutex, &st->lock) {
I would add scope the whole case statement 
	case IIO_CHAN_INFO_SAMP_FREQ: {

and then use guard(mutex)(&st->lock) here instead of scoped_guard()

That will reduce the indent of the rest and generally give slightly
more readable code.

> +			pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> +			ldacb_pwm_state.duty_cycle = DIV_ROUND_CLOSEST_ULL(NANO, 2 * val);
> +			ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, val);
> +			ldacb_pwm_state.enabled = true;
> +
> +			ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> +			if (ret)
> +				return ret;
> +
> +			st->sampling_frequency = val;
> +		}
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}

> +static int _ad5706r_setup(struct ad5706r_state *st)
> +{
> +	struct pwm_state ldacb_pwm_state;
> +	struct device *dev = &st->spi->dev;
> +	int ret;
> +	int i;
> +
> +	guard(mutex)(&st->lock);

Why is the lock needed?  This is all (I think) early probe stuff.
At that point we are serialized anyway.

> +
> +	st->debug_streaming_len = 0;
> +	st->debug_streaming_data = 0;
> +	st->debug_streaming_addr = 0;

Where they are the 'natural' defaults (which it seems these probably are)
we can just rely on the whole structure having been kzalloc'd and not set
them to 0 one by one.

> +	st->debug_spi_speed_hz_write = 10000000;
> +	st->debug_spi_speed_hz_read = 10000000;
> +
> +	st->dev_addr = 0x00;
> +	st->addr_ascension = ADDR_ASCENSION_DECREMENT;
> +	st->single_instr = SINGLE_INSTR_STREAMING;
> +	st->shift_val = 0;
> +	st->addr_desc = 1;
> +	st->hw_ldac_tg_state = HW_LDAC_TG_STATE_LOW;
> +	st->sampling_frequency = 1000000;
> +	st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_DISABLED;
> +	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
> +	st->multi_dac_input_a = 0;
> +	st->reference_volts = 2500;
> +	st->ref_select = REF_SELECT_EXTERNAL;
> +	st->hw_shutdown_state = HW_SHUTDOWN_STATE_LOW;
> +
> +	for (i = 0; i < 4; i++) {

4 is a magic number.  Use one of the size macros or define one for
the number of channels and use it consistently throughout the code.

> +		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
> +		st->range_sel[i] = RANGE_SEL_50;
> +		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
> +		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
> +		st->toggle_trigger_chn[i] = TOGGLE_TRIGGER_CHN_HW_TRIGGER;
> +		st->dither_trigger_chn[i] = DITHER_TRIGGER_CHN_HW_TRIGGER;
> +		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;
> +	}
> +
> +	/* get spi_clk axi_clkgen, no enable as spi_engine driver enables it */

don't enable it as the spi_enging driver enables it

This seems odd division of labour though.

> +	st->reference_clk = devm_clk_get(dev, "spi_clk");
> +	if (IS_ERR(st->reference_clk))
> +		return dev_err_probe(dev, PTR_ERR(st->reference_clk),
> +				     "Failed to get AXI CLKGEN clock\n");
> +
> +	st->ldacb_pwm = devm_pwm_get(dev, "ad5706r_ldacb");
> +	if (IS_ERR(st->ldacb_pwm))
> +		return dev_err_probe(dev, PTR_ERR(st->ldacb_pwm),
> +				     "Failed to get LDACB PWM\n");
> +	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> +	ldacb_pwm_state.duty_cycle = 0;
> +	ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, st->sampling_frequency);
> +	ldacb_pwm_state.enabled = true;
> +	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to apply PWM state\n");
> +
> +	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb", GPIOD_OUT_LOW);
> +	if (IS_ERR(st->resetb_gpio)) {

It's a bit of a corner case on whether the following justifies the brackets or not.
I'd not have them but others do prefer the other way around when there is a line
break in the single call like this.

> +		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
> +				     "Failed to get RESET_B GPIO\n");
> +	}
> +
> +	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn", GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->shdn_gpio)) {
> +		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
> +				     "Failed to get SHDN GPIO\n");
> +	}
> +
> +	/*
> +	 * Get SPI max speed from device tree. Allows up to 100MHz.
> +	 * If value is taken from spi->max_speed_hz, it is capped at 25MHz.
> +	 */
> +	ret = device_property_read_u32(dev, "spi-max-frequency", &st->spi_max_speed_hz);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to set SPI Max Speed\n");
> +
> +	st->spi_max_speed_hz = clamp(st->spi_max_speed_hz, SPI_MIN_SPEED_HZ, SPI_MAX_SPEED_HZ);
> +
> +	return 0;
> +}
> +
> +static int ad5706r_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ad5706r_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
Probably worth a local
	struct device *dev = &spi->dev; just to shorten the few places
it is repeated in here.

> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	mutex_init(&st->lock);
someone else pointed out
	ret = devm_mutex_init() I think.

> +	st->spi = spi;
> +
> +	ret = _ad5706r_setup(st);
> +	if (ret)
> +		return ret;
> +
> +	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);
> +
> +	ret = devm_iio_device_register(&spi->dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	ad5706r_debugs_init(indio_dev);
I'd bring all the debugfs stuff in via a later patch. It probably adds complexity
that will take more review time than people will expend all in one go.

Thanks,

Jonathan

> +
> +	return 0;
> +}

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

[Torreno, Alexis Czezar]

> > 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>
> 
> Hi Alexis
> 
> Welcome to IIO.  A few quick comments inline, but as others have observed
> the patch is too large and needs breaking up.  Also there is a lot of custom ABI.
> Given we are reluctant to merge that at all, it definitely needs documentation
> and careful consideration of whether it can be done with existing ABI or is
> more appropriate in DT.
> 
> Thanks,
> 
> Jonathan

Using this reply to acknowledge all the emails/feedback.

Hi All,

Thanks for the time you spent on this large patch.

It seems I will have to split this up first. I think it would be the basic driver,
the ABI + docu, then the debugfs. Merge 1 before I submit the others.
Will apply each feedback as I go.

Some naming schemes need to be updated, to properly reflect the datasheet
terms rather than the one internally agreed upon.

The most unusual that a lot pointed out is related to the clock/.speed_hz/
spi_engine. I was asked to make it work a bit beyond 50MHz, using a cora
fpga. The cora processor spi is limited to 50MHz due to HW, hence the need
to use an spi-engine. However, this is limited to the resolution on how small
the frequencies can be changed, the spi engine only does integer division, thus
leading me to change the clock driver of the spi_engine itself.
I'll try to see if the implementation can be changed to something simpler and
will make it easier for us.

Thanks!
Alexis

> 
> > diff --git a/drivers/iio/dac/ad5706r.c b/drivers/iio/dac/ad5706r.c new
> > file mode 100644 index
> >
> 0000000000000000000000000000000000000000..2d718cf7300bcd1f599
> fe715aacb
> > 3170f72541af
> > --- /dev/null
> > +++ b/drivers/iio/dac/ad5706r.c
> > @@ -0,0 +1,2290 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * AD5706R 16-bit Current Output Digital to Analog Converter
> > + *
> > + * Copyright 2026 Analog Devices Inc.
> > + *
> > + * This driver is designed for use with the AXI SPI Engine and AXI
> > +CLKGEN
> > + * on Xilinx Zynq platforms. The 'clocks' device tree property
> > +references
> > + * the AXI CLKGEN output clock, which is used to dynamically control
> > +the
> > + * SPI clock rate for read and write operations independently.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/module.h>
> Follow approximate include what you use principles for kernel drivers.
> That means mostly including everything that is used directly with exception of
> a few headers that we know will always be included by others (usually because
> they are documented as doing so).
> 
> e.g. mutex.h should always be in a driver that uses a mutex.
> If you are using guards, then cleanup.h needs to be there etc.
> 
> > +#include <linux/pwm.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/unaligned.h>
> > +#include <linux/units.h>
> > +
> > +/* SPI Defines */
> > +#define AD5706R_RD_MASK		BIT(15)
> > +#define AD5706R_ADDR_PIN_MASK	GENMASK(14, 12)
> > +#define AD5706R_ADDR_MASK	GENMASK(11, 0)
> > +#define AD5706R_VAL_MASK	GENMASK(7, 0)
> > +
> > +/* Registers and Masks */
> > +#define AD5706R_MASK_RESET			(BIT(7) | BIT(0))
> > +#define AD5706R_MASK_DEV_ADDR(x)		((x) & GENMASK(2,
> 0))
> 
> Define the mask only then use FIELD_GET() / FIELD_PREP() in the code.
> That tends to end up more readable than a macro doing the same thing
> without the compile time checks those bring on values fitting etc.
> 
> > +
> > +#define NUM_CHANNELS			4
> > +#define SPI_MAX_SPEED_HZ		(100 * HZ_PER_MHZ)	/* 100 MHz
> */
> > +#define SPI_MIN_SPEED_HZ		(3 * HZ_PER_MHZ)	/* 3 MHz */
> 
> Very generic names.  These are going to clash with something in another
> header at somepoint.  Either push the values into a helper, inline or prefix the
> defines to avoid that naming clash.
> 
> 
> > +/*
> > + * Order of attributes in code:
> 
> I'm not sure how this comment helps us.
> 
> > + *
> > + * Device Attributes:
> > + *   - dev_addr
> > + *   - addr_ascension
> > + *   - single_instr
> > + *   - hw_ldac_tg_state
> > + *   - sampling_frequency
> > + *   - hw_ldac_tg_pwm
> > + *   - mux_out_sel
> > + *   - multi_dac_input_a
> > + *   - multi_dac_sw_ldac_trigger
> > + *   - reference_volts
> > + *   - ref_select
> > + *   - hw_shutdown_state
> > + *
> > + * Channel Attributes:
> > + *   - raw
> > + *   - scale
> > + *   - offset
> > + *   - input_register_a
> > + *   - input_register_b
> > + *   - hw_active_edge
> > + *   - range_sel
> > + *   - output_state
> > + *   - ldac_trigger_chn
> > + *   - toggle_trigger_chn
> > + *   - dither_trigger_chn
> > + *   - multi_dac_sel_ch
> > + */
> 
> > +
> > +/* ENUM Lists */
> Don't give generic comments like this on how you've laid the code out. They
> just tend to end up wrong after new features are added and don't bring much
> value in the first place.
> > +enum addr_ascension_iio_dev_attr {
> 
> 
> > +static struct iio_chan_spec_ext_info ad5706r_ext_info[] = {
> > +	/* device_attribute */
> > +	AD5706R_CHAN_EXT_INFO("dev_addr", 0, IIO_SHARED_BY_ALL,
> > +			      ad5706r_dev_addr_read,
> ad5706r_dev_addr_write),
> > +
> > +	IIO_ENUM("addr_ascension", IIO_SHARED_BY_ALL,
> &ad5706r_addr_ascension_enum),
> > +	IIO_ENUM_AVAILABLE("addr_ascension", IIO_SHARED_BY_ALL,
> > +&ad5706r_addr_ascension_enum),
> > +
> > +	IIO_ENUM("single_instr", IIO_SHARED_BY_ALL,
> &ad5706r_single_instr_enum),
> > +	IIO_ENUM_AVAILABLE("single_instr", IIO_SHARED_BY_ALL,
> > +&ad5706r_single_instr_enum),
> > +
> > +	IIO_ENUM("hw_ldac_tg_state", IIO_SHARED_BY_ALL,
> &ad5706r_hw_ldac_tg_state_enum),
> > +	IIO_ENUM_AVAILABLE("hw_ldac_tg_state", IIO_SHARED_BY_ALL,
> > +&ad5706r_hw_ldac_tg_state_enum),
> > +
> > +	/* Sampling Frequency part of read/write RAW */
> > +
> > +	IIO_ENUM("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL,
> &ad5706r_hw_ldac_tg_pwm_enum),
> > +	IIO_ENUM_AVAILABLE("hw_ldac_tg_pwm", IIO_SHARED_BY_ALL,
> > +&ad5706r_hw_ldac_tg_pwm_enum),
> > +
> > +	IIO_ENUM("mux_out_sel", IIO_SHARED_BY_ALL,
> &ad5706r_mux_out_sel_enum),
> > +	IIO_ENUM_AVAILABLE("mux_out_sel", IIO_SHARED_BY_ALL,
> > +&ad5706r_mux_out_sel_enum),
> > +
> > +	AD5706R_CHAN_EXT_INFO("multi_dac_input_a", 0,
> IIO_SHARED_BY_ALL,
> > +			      ad5706r_multi_dac_input_a_read,
> > +ad5706r_multi_dac_input_a_write),
> > +
> > +	IIO_ENUM("multi_dac_sw_ldac_trigger", IIO_SHARED_BY_ALL,
> > +		 &ad5706r_multi_dac_sw_ldac_trigger_enum),
> > +	IIO_ENUM_AVAILABLE("multi_dac_sw_ldac_trigger",
> IIO_SHARED_BY_ALL,
> > +			   &ad5706r_multi_dac_sw_ldac_trigger_enum),
> > +
> > +	AD5706R_CHAN_EXT_INFO("reference_volts", 0,
> IIO_SHARED_BY_ALL,
> > +			      ad5706r_reference_volts_read,
> > +ad5706r_reference_volts_write),
> > +
> > +	IIO_ENUM("ref_select", IIO_SHARED_BY_ALL,
> &ad5706r_ref_select_enum),
> > +	IIO_ENUM_AVAILABLE("ref_select", IIO_SHARED_BY_ALL,
> > +&ad5706r_ref_select_enum),
> > +
> > +	IIO_ENUM("hw_shutdown_state", IIO_SHARED_BY_ALL,
> &ad5706r_hw_shutdown_state_enum),
> > +	IIO_ENUM_AVAILABLE("hw_shutdown_state", IIO_SHARED_BY_ALL,
> > +&ad5706r_hw_shutdown_state_enum),
> > +
> > +	/* Channel Attributes */
> > +	AD5706R_CHAN_EXT_INFO("input_register_a", 0, IIO_SEPARATE,
> > +			      ad5706r_input_register_a_read,
> > +ad5706r_input_register_a_write),
> > +
> > +	AD5706R_CHAN_EXT_INFO("input_register_b", 0, IIO_SEPARATE,
> > +			      ad5706r_input_register_b_read,
> > +ad5706r_input_register_b_write),
> > +
> > +	IIO_ENUM("hw_active_edge", IIO_SEPARATE,
> &ad5706r_hw_active_edge_enum),
> > +	IIO_ENUM_AVAILABLE("hw_active_edge", IIO_SEPARATE,
> > +&ad5706r_hw_active_edge_enum),
> > +
> > +	IIO_ENUM("range_sel", IIO_SEPARATE, &ad5706r_range_sel_enum),
> > +	IIO_ENUM_AVAILABLE("range_sel", IIO_SEPARATE,
> > +&ad5706r_range_sel_enum),
> > +
> > +	IIO_ENUM("output_state", IIO_SEPARATE,
> &ad5706r_output_state_enum),
> > +	IIO_ENUM_AVAILABLE("output_state", IIO_SEPARATE,
> > +&ad5706r_output_state_enum),
> > +
> > +	IIO_ENUM("ldac_trigger_chn", IIO_SEPARATE,
> &ad5706r_ldac_trigger_chn_enum),
> > +	IIO_ENUM_AVAILABLE("ldac_trigger_chn", IIO_SEPARATE,
> > +&ad5706r_ldac_trigger_chn_enum),
> > +
> > +	IIO_ENUM("toggle_trigger_chn", IIO_SEPARATE,
> &ad5706r_toggle_trigger_chn_enum),
> > +	IIO_ENUM_AVAILABLE("toggle_trigger_chn", IIO_SEPARATE,
> > +&ad5706r_toggle_trigger_chn_enum),
> > +
> > +	IIO_ENUM("dither_trigger_chn", IIO_SEPARATE,
> &ad5706r_dither_trigger_chn_enum),
> > +	IIO_ENUM_AVAILABLE("dither_trigger_chn", IIO_SEPARATE,
> > +&ad5706r_dither_trigger_chn_enum),
> > +
> > +	IIO_ENUM("multi_dac_sel_ch", IIO_SEPARATE,
> &ad5706r_multi_dac_sel_ch_enum),
> > +	IIO_ENUM_AVAILABLE("multi_dac_sel_ch", IIO_SEPARATE,
> > +&ad5706r_multi_dac_sel_ch_enum),
> > +
> 
> Others have pointed out that custom ABI has various basic problems.
> 
> 1) Generic userspace has no way to know how to use it. As such, it gets little
> used and must not be required for basic functionality.  So don't bring it in as
> part of the initial patch, but instead do it as a follow up patch (can be in same
> series if the series remains reasonably sized)
> 2) It needs full documentation for us to review it.  In
> Documentation/ABI/testing/sysfs-bus-iio-driver-name
> 
> > +	{},
> > +};
> > +
> > +/* Channel */
> I'm not sure what that comment means. I'd drop it.
> > +static int ad5706r_read_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan,
> > +			    int *val,
> > +			    int *val2,
> Put multiple parameters on one line as long as we stay under 80 chars (or a bit
> over if it helps readability is fine as well)
> 
> > +			    long mask)
> > +{
> > +	struct ad5706r_state *st = iio_priv(indio_dev);
> > +	u16 reg_val;
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		scoped_guard(mutex, &st->lock) {
> 
> As below. Add scope with { } and use guard() That will reduce the very long
> lines here.
> 
> > +			ret = ad5706r_spi_read(st,
> AD5706R_REG_DAC_DATA_READBACK_CH(chan->channel),
> > +					       &reg_val);
> > +
> > +			if (ret)
> > +				return ret;
> > +
> > +			*val = reg_val;
> > +		}
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		switch (st->range_sel[chan->channel]) {
> > +		case RANGE_SEL_50:
> > +			*val = 50 * HZ_PER_MHZ /
> AD5706R_DAC_MAX_CODE;
> > +			break;
> > +		case RANGE_SEL_150:
> > +			*val = 150 * HZ_PER_MHZ /
> AD5706R_DAC_MAX_CODE;
> > +			break;
> > +		case RANGE_SEL_200:
> > +			*val = 200 * HZ_PER_MHZ /
> AD5706R_DAC_MAX_CODE;
> > +			break;
> > +		case RANGE_SEL_300:
> > +		default:
> 
> A default often doesn't make much sense. How did you get a different value
> from the ones being explicitly set.
> 
> > +			*val = 300 * HZ_PER_MHZ /
> AD5706R_DAC_MAX_CODE;
> > +			break;
> > +		}
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		*val = 0;
> > +		return IIO_VAL_INT;
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		*val = st->sampling_frequency;
> > +		return IIO_VAL_INT;
> > +	}
> > +
> > +	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);
> > +	struct pwm_state ldacb_pwm_state;
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		/* Sets minimum and maximum frequency */
> > +		val = clamp(val, SAMPLING_FREQUENCY_MIN_HZ,
> > +SAMPLING_FREQUENCY_MAX_HZ);
> > +
> > +		scoped_guard(mutex, &st->lock) {
> I would add scope the whole case statement
> 	case IIO_CHAN_INFO_SAMP_FREQ: {
> 
> and then use guard(mutex)(&st->lock) here instead of scoped_guard()
> 
> That will reduce the indent of the rest and generally give slightly more readable
> code.
> 
> > +			pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> > +			ldacb_pwm_state.duty_cycle =
> DIV_ROUND_CLOSEST_ULL(NANO, 2 * val);
> > +			ldacb_pwm_state.period =
> DIV_ROUND_CLOSEST_ULL(NANO, val);
> > +			ldacb_pwm_state.enabled = true;
> > +
> > +			ret = pwm_apply_might_sleep(st->ldacb_pwm,
> &ldacb_pwm_state);
> > +			if (ret)
> > +				return ret;
> > +
> > +			st->sampling_frequency = val;
> > +		}
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> > +static int _ad5706r_setup(struct ad5706r_state *st) {
> > +	struct pwm_state ldacb_pwm_state;
> > +	struct device *dev = &st->spi->dev;
> > +	int ret;
> > +	int i;
> > +
> > +	guard(mutex)(&st->lock);
> 
> Why is the lock needed?  This is all (I think) early probe stuff.
> At that point we are serialized anyway.
> 
> > +
> > +	st->debug_streaming_len = 0;
> > +	st->debug_streaming_data = 0;
> > +	st->debug_streaming_addr = 0;
> 
> Where they are the 'natural' defaults (which it seems these probably are) we
> can just rely on the whole structure having been kzalloc'd and not set them to
> 0 one by one.
> 
> > +	st->debug_spi_speed_hz_write = 10000000;
> > +	st->debug_spi_speed_hz_read = 10000000;
> > +
> > +	st->dev_addr = 0x00;
> > +	st->addr_ascension = ADDR_ASCENSION_DECREMENT;
> > +	st->single_instr = SINGLE_INSTR_STREAMING;
> > +	st->shift_val = 0;
> > +	st->addr_desc = 1;
> > +	st->hw_ldac_tg_state = HW_LDAC_TG_STATE_LOW;
> > +	st->sampling_frequency = 1000000;
> > +	st->hw_ldac_tg_pwm = HW_LDAC_TG_PWM_DISABLED;
> > +	st->mux_out_sel = MUX_OUT_SEL_DISABLED;
> > +	st->multi_dac_input_a = 0;
> > +	st->reference_volts = 2500;
> > +	st->ref_select = REF_SELECT_EXTERNAL;
> > +	st->hw_shutdown_state = HW_SHUTDOWN_STATE_LOW;
> > +
> > +	for (i = 0; i < 4; i++) {
> 
> 4 is a magic number.  Use one of the size macros or define one for the number
> of channels and use it consistently throughout the code.
> 
> > +		st->hw_active_edge[i] = HW_ACTIVE_EDGE_RISING_EDGE;
> > +		st->range_sel[i] = RANGE_SEL_50;
> > +		st->output_state[i] = OUTPUT_STATE_NORMAL_SW;
> > +		st->ldac_trigger_chn[i] = LDAC_TRIGGER_CHN_HW_TRIGGER;
> > +		st->toggle_trigger_chn[i] =
> TOGGLE_TRIGGER_CHN_HW_TRIGGER;
> > +		st->dither_trigger_chn[i] =
> DITHER_TRIGGER_CHN_HW_TRIGGER;
> > +		st->multi_dac_sel_ch[i] = MULTI_DAC_SEL_CH_EXCLUDE;
> > +	}
> > +
> > +	/* get spi_clk axi_clkgen, no enable as spi_engine driver enables it
> > +*/
> 
> don't enable it as the spi_enging driver enables it
> 
> This seems odd division of labour though.
> 
> > +	st->reference_clk = devm_clk_get(dev, "spi_clk");
> > +	if (IS_ERR(st->reference_clk))
> > +		return dev_err_probe(dev, PTR_ERR(st->reference_clk),
> > +				     "Failed to get AXI CLKGEN clock\n");
> > +
> > +	st->ldacb_pwm = devm_pwm_get(dev, "ad5706r_ldacb");
> > +	if (IS_ERR(st->ldacb_pwm))
> > +		return dev_err_probe(dev, PTR_ERR(st->ldacb_pwm),
> > +				     "Failed to get LDACB PWM\n");
> > +	pwm_get_state(st->ldacb_pwm, &ldacb_pwm_state);
> > +	ldacb_pwm_state.duty_cycle = 0;
> > +	ldacb_pwm_state.period = DIV_ROUND_CLOSEST_ULL(NANO, st-
> >sampling_frequency);
> > +	ldacb_pwm_state.enabled = true;
> > +	ret = pwm_apply_might_sleep(st->ldacb_pwm, &ldacb_pwm_state);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to apply PWM
> state\n");
> > +
> > +	st->resetb_gpio = devm_gpiod_get_optional(dev, "dac-resetb",
> GPIOD_OUT_LOW);
> > +	if (IS_ERR(st->resetb_gpio)) {
> 
> It's a bit of a corner case on whether the following justifies the brackets or not.
> I'd not have them but others do prefer the other way around when there is a
> line break in the single call like this.
> 
> > +		return dev_err_probe(dev, PTR_ERR(st->resetb_gpio),
> > +				     "Failed to get RESET_B GPIO\n");
> > +	}
> > +
> > +	st->shdn_gpio = devm_gpiod_get_optional(dev, "dac-shdn",
> GPIOD_OUT_HIGH);
> > +	if (IS_ERR(st->shdn_gpio)) {
> > +		return dev_err_probe(dev, PTR_ERR(st->shdn_gpio),
> > +				     "Failed to get SHDN GPIO\n");
> > +	}
> > +
> > +	/*
> > +	 * Get SPI max speed from device tree. Allows up to 100MHz.
> > +	 * If value is taken from spi->max_speed_hz, it is capped at 25MHz.
> > +	 */
> > +	ret = device_property_read_u32(dev, "spi-max-frequency", &st-
> >spi_max_speed_hz);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to set SPI Max
> Speed\n");
> > +
> > +	st->spi_max_speed_hz = clamp(st->spi_max_speed_hz,
> SPI_MIN_SPEED_HZ,
> > +SPI_MAX_SPEED_HZ);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ad5706r_probe(struct spi_device *spi) {
> > +	struct iio_dev *indio_dev;
> > +	struct ad5706r_state *st;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> Probably worth a local
> 	struct device *dev = &spi->dev; just to shorten the few places it is
> repeated in here.
> 
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	st = iio_priv(indio_dev);
> > +	mutex_init(&st->lock);
> someone else pointed out
> 	ret = devm_mutex_init() I think.
> 
> > +	st->spi = spi;
> > +
> > +	ret = _ad5706r_setup(st);
> > +	if (ret)
> > +		return ret;
> > +
> > +	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);
> > +
> > +	ret = devm_iio_device_register(&spi->dev, indio_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ad5706r_debugs_init(indio_dev);
> I'd bring all the debugfs stuff in via a later patch. It probably adds complexity
> that will take more review time than people will expend all in one go.
> 
> Thanks,
> 
> Jonathan
> 
> > +
> > +	return 0;
> > +}

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

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 04:49:38AM +0000, Torreno, Alexis Czezar 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
> > >
> > > Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> > 
> > Hi Alexis
> > 
> > Welcome to IIO.  A few quick comments inline, but as others have observed
> > the patch is too large and needs breaking up.  Also there is a lot of custom ABI.
> > Given we are reluctant to merge that at all, it definitely needs documentation
> > and careful consideration of whether it can be done with existing ABI or is
> > more appropriate in DT.
> > 
> > Thanks,
> > 
> > Jonathan
> 
> Using this reply to acknowledge all the emails/feedback.

> Thanks for the time you spent on this large patch.
> 
> It seems I will have to split this up first. I think it would be the basic driver,
> the ABI + docu, then the debugfs. Merge 1 before I submit the others.
> Will apply each feedback as I go.
> 
> Some naming schemes need to be updated, to properly reflect the datasheet
> terms rather than the one internally agreed upon.
> 
> The most unusual that a lot pointed out is related to the clock/.speed_hz/
> spi_engine. I was asked to make it work a bit beyond 50MHz, using a cora
> fpga. The cora processor spi is limited to 50MHz due to HW, hence the need
> to use an spi-engine. However, this is limited to the resolution on how small
> the frequencies can be changed, the spi engine only does integer division, thus
> leading me to change the clock driver of the spi_engine itself.
> I'll try to see if the implementation can be changed to something simpler and
> will make it easier for us.

Please, do remove the context you are not commenting on!
I have listed ~500 lines to understand if there is anything below.
Waste of time for each reviewer / follower...

-- 
With Best Regards,
Andy Shevchenko

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

[Nuno Sá]

On Fri, 2026-02-20 at 18:56 +0200, Andy Shevchenko wrote:
> On Fri, Feb 20, 2026 at 03:02:37PM +0000, Nuno Sá wrote:
> > On Fri, 2026-02-20 at 13:00 +0200, Andy Shevchenko wrote:
> > > On Fri, Feb 20, 2026 at 10:48:59AM +0000, Nuno Sá wrote:
> > > > On Fri, 2026-02-20 at 16:02 +0800, Alexis Czezar Torreno wrote:
> 
> ...
> 
> > > > > +static void ad5706r_debugs_init(struct iio_dev *indio_dev)
> > > > > +{
> > > > > +	struct dentry *d = iio_get_debugfs_dentry(indio_dev);
> > > > 
> > > > It should have:
> > > > 
> > > > if (!IS_ENABLED(CONFIG_DEBUGFS))
> > > > 	return
> > > 
> > > But why? The debugfs is a stub when disabled, nobody should do that
> > > in the cases when the main purpose is not the debugfs code.
> > 
> > Because the compiler can then optimize away all of the above code...
> 
> How is it different to the code elimination part that is inside in each of
> the below calls?

Clearly none :). For some reason I thought it would matter. Maybe I was mistaken by
some old code that had #ifdef guards on the debug code.

Any ways, I might send some patches cleaning some places where I added the above so
people do not copy it around.

- Nuno Sá
>