[PATCH 0/2] iio: dac: add support for AD5413

[PATCH 0/2] iio: dac: add support for AD5413

[Bruce Tsao via B4 Relay]

This series adds support for the Analog Devices AD5413: a single-channel,
14-bit, serial-input DAC with software-selectable voltage (+/-10.5 V) and
current (0-24 mA) outputs. The SPI interface runs in mode 1 and carries a
CRC-8 on every frame, which the device has enabled out of reset.

Patch 1 documents the device-tree binding; patch 2 adds the IIO driver.

Tested on an EVAL-AD5413SDZ with a Raspberry Pi 5 and an NXP i.MX93 FRDM
board (probe, voltage- and current-output, scale/offset, powerdown/resume).

Signed-off-by: Bruce Tsao <bruce.tsao@analog.com>
---
Bruce Tsao (2):
      dt-bindings: iio: dac: Document Analog Devices AD5413 single-channel DAC
      iio: dac: add AD5413 driver

 .../devicetree/bindings/iio/dac/adi,ad5413.yaml    | 101 ++++
 drivers/iio/dac/Kconfig                            |  11 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ad5413.c                           | 666 +++++++++++++++++++++
 4 files changed, 779 insertions(+)
---
base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
change-id: 20261017-ad5413-upstream-f8664fff4fdb

Best regards,
-- 
Bruce Tsao <bruce.tsao@analog.com>

[PATCH 1/2] dt-bindings: iio: dac: Document Analog Devices AD5413 single-channel DAC

[Bruce Tsao via B4 Relay]

From: Bruce Tsao <bruce.tsao@analog.com>

Document the devicetree bindings for the Analog Devices AD5413, a
single-channel precision DAC controlled via SPI. The device supports
voltage or current output modes and programmable slew-rate control.

Signed-off-by: Bruce Tsao <bruce.tsao@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5413.yaml    | 101 +++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
new file mode 100644
index 000000000000..74e869304aab
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5413.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5413 DAC
+
+description:
+  Analog Devices AD5413 is a single-channel precision DAC supporting voltage
+  or current output modes with programmable slew-rate control.
+  The output mode is selected via the adi,chan-current-mode property.
+  Optional reset GPIO may be provided.
+
+maintainers:
+  - bruce.tsao@analog.com
+
+properties:
+  compatible:
+    const: adi,ad5413
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  spi-cpha: true
+
+  spi-cpol: false
+
+  reset-gpios:
+    maxItems: 1
+    description: Optional reset GPIO (active low).
+
+  avdd-supply:
+    description: Analog positive supply (AVDD)
+
+  dvdd-supply:
+    description: Digital positive supply (DVDD)
+
+  avss-supply:
+    description: Analog negative supply (AVSS)
+
+  adi,chan-current-mode:
+    type: boolean
+    description: Select current output mode. If absent, voltage mode is used.
+
+  adi,slew-time-us:
+    description: Output digital slew control time in microseconds
+    minimum: 0
+    maximum: 1000000
+    default: 0
+
+required:
+  - compatible
+  - spi-cpha
+  - avdd-supply
+  - dvdd-supply
+  - avss-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad5413";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpha;
+            reset-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
+            avdd-supply = <&avdd>;
+            dvdd-supply = <&dvdd>;
+            avss-supply = <&avss>;
+            adi,slew-time-us = <125>;
+        };
+    };
+
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad5413";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpha;
+            avdd-supply = <&avdd>;
+            dvdd-supply = <&dvdd>;
+            avss-supply = <&avss>;
+
+            adi,chan-current-mode;
+        };
+    };

-- 
2.53.0

Re: [PATCH 1/2] dt-bindings: iio: dac: Document Analog Devices AD5413 single-channel DAC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: iio: dac: adi,ad5413: missing reg property and requirement
- [Medium] dt-bindings: iio: dac: adi,ad5413: invalid maintainers and spi-cpol schema
- [Medium] dt-bindings: iio: dac: adi,ad5413: custom boolean mode property duplicates standard range bindings
--

commit b9fdb51ef057c689f916309a04134bf25c49ed72
Author: Bruce Tsao <bruce.tsao@analog.com>

dt-bindings: iio: dac: Document Analog Devices AD5413 single-channel DAC

This commit documents the devicetree bindings for the Analog Devices AD5413
single-channel SPI DAC, including power supplies, reset GPIO, output mode,
and slew-rate control.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
> new file mode 100644
> index 0000000000000..74e869304aab9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5413.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5413 DAC
> +
> +description:
> +  Analog Devices AD5413 is a single-channel precision DAC supporting voltage
> +  or current output modes with programmable slew-rate control.
> +  The output mode is selected via the adi,chan-current-mode property.
> +  Optional reset GPIO may be provided.
> +
> +maintainers:
> +  - bruce.tsao@analog.com
> +
> +properties:
> +  compatible:
> +    const: adi,ad5413
> +
> +  spi-max-frequency:
> +    maximum: 50000000
> +
> +  spi-cpha: true
> +
> +  spi-cpol: false

[Severity: Medium]
Will this schema pass make dt_binding_check?

In the maintainers block, the entry uses a bare email address rather than
the required Full Name <email@domain> format:

  maintainers:
    - Bruce Tsao <bruce.tsao@analog.com>

Also, in the top-level properties block, spi-cpol is set to false. In DT
schema, boolean false property schemas are only valid inside conditional
if/then/else blocks to disallow a property defined at the top level. Since
spi-cpol is not defined in spi-peripheral-props.yaml and this binding sets
unevaluatedProperties: false, should spi-cpol: false simply be omitted?

> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Optional reset GPIO (active low).
> +
> +  avdd-supply:
> +    description: Analog positive supply (AVDD)
> +
> +  dvdd-supply:
> +    description: Digital positive supply (DVDD)
> +
> +  avss-supply:
> +    description: Analog negative supply (AVSS)
> +
> +  adi,chan-current-mode:
> +    type: boolean
> +    description: Select current output mode. If absent, voltage mode is used.

[Severity: Medium]
Should the output mode and range be described using standard IIO DAC range
properties rather than a custom adi,chan-current-mode boolean?

In drivers/iio/dac/ad5413.c, the hardware supports a 4-bit range configuration
field (AD5413_REG_DAC_CONFIG_RANGE_MSK = GENMASK(3, 0)) with multiple voltage
and current ranges, while the driver currently hardcodes only two of them
(AD5413_RANGE_PLUSMINUS_10_5V = 0x3 and AD5413_RANGE_0MA_24MA = 0x9) based on
this boolean.

Could this use output-range-microvolt and output-range-microamp from
Documentation/devicetree/bindings/iio/dac/dac.yaml (or adi,range-microvolt and
adi,range-microamp as in Documentation/devicetree/bindings/iio/dac/adi,ad5758.yaml)
so the binding describes the hardware capabilities rather than the current
driver limitation?

> +
> +  adi,slew-time-us:
> +    description: Output digital slew control time in microseconds
> +    minimum: 0
> +    maximum: 1000000
> +    default: 0
> +
> +required:
> +  - compatible
> +  - spi-cpha
> +  - avdd-supply
> +  - dvdd-supply
> +  - avss-supply

[Severity: Medium]
Should reg be defined with maxItems: 1 under properties and listed in
required?

The binding references /schemas/spi/spi-peripheral-props.yaml# under allOf,
which defines reg with minItems: 1 and maxItems: 256 and does not mark reg
as required. Because AD5413 is a single-chip-select SPI device and both
examples specify reg = <0>, omitting reg from properties and required leaves
the chip-select count unconstrained and optional during DT validation.

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad5413";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cpha;
> +            reset-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
> +            avdd-supply = <&avdd>;
> +            dvdd-supply = <&dvdd>;
> +            avss-supply = <&avss>;
> +            adi,slew-time-us = <125>;
> +        };
> +    };
> +
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad5413";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cpha;
> +            avdd-supply = <&avdd>;
> +            dvdd-supply = <&dvdd>;
> +            avss-supply = <&avss>;
> +
> +            adi,chan-current-mode;
> +        };
> +    };

In both examples above, could the standard properties (avdd-supply,
avss-supply, dvdd-supply, reset-gpios, spi-cpha, spi-max-frequency) be placed
in canonical natural-sort order after reg per
Documentation/devicetree/bindings/dts-coding-style.rst?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-ad5413-upstream-v1-0-61f40c8fef25@analog.com?part=1

Re: [PATCH 1/2] dt-bindings: iio: dac: Document Analog Devices AD5413 single-channel DAC

[Jonathan Cameron]

On Sat, 17 Oct 2026 03:23:53 +0800
Bruce Tsao via B4 Relay <devnull+bruce.tsao.analog.com@kernel.org> wrote:

> From: Bruce Tsao <bruce.tsao@analog.com>
> 
> Document the devicetree bindings for the Analog Devices AD5413, a
> single-channel precision DAC controlled via SPI. The device supports
> voltage or current output modes and programmable slew-rate control.
> 
> Signed-off-by: Bruce Tsao <bruce.tsao@analog.com>
Hi Bruce,

Welcome to IIO.

> ---
>  .../devicetree/bindings/iio/dac/adi,ad5413.yaml    | 101 +++++++++++++++++++++
>  1 file changed, 101 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
> new file mode 100644
> index 000000000000..74e869304aab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5413.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/adi,ad5413.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD5413 DAC
> +
> +description:
> +  Analog Devices AD5413 is a single-channel precision DAC supporting voltage
> +  or current output modes with programmable slew-rate control.
> +  The output mode is selected via the adi,chan-current-mode property.
> +  Optional reset GPIO may be provided.

New line per sentence isn't really adding anything. I'd reflow this
a a paragraph.

> +
> +maintainers:
> +  - bruce.tsao@analog.com
> +
> +properties:
> +  compatible:
> +    const: adi,ad5413
> +
> +  spi-max-frequency:
> +    maximum: 50000000
> +
> +  spi-cpha: true
> +
> +  spi-cpol: false
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Optional reset GPIO (active low).
From a quick glance looks like we also have LDAC SYNC and FAULT.
+ a reference out that probably wants to be modeled as a supply.

Upshot is this looks like an incomplete binding and generally 
we try to make them as complete as possible from the start, even
if the driver doesn't yet use everything.

> +
> +  avdd-supply:
> +    description: Analog positive supply (AVDD)
> +
> +  dvdd-supply:
> +    description: Digital positive supply (DVDD)
> +
> +  avss-supply:
> +    description: Analog negative supply (AVSS)
> +
> +  adi,chan-current-mode:
> +    type: boolean
> +    description: Select current output mode. If absent, voltage mode is used.

We have some existing dt bindings for multi mode channels on adi
parts. See adi,ch-func in various other bindings.
Whilst we've had various discussions about generalizing that to
a generic dac binding so far we haven't done so. Maybe now is the time...


> +
> +  adi,slew-time-us:
> +    description: Output digital slew control time in microseconds
> +    minimum: 0
> +    maximum: 1000000
> +    default: 0
> +
> +required:
> +  - compatible
> +  - spi-cpha
> +  - avdd-supply
> +  - dvdd-supply
> +  - avss-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad5413";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cpha;
> +            reset-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
> +            avdd-supply = <&avdd>;
> +            dvdd-supply = <&dvdd>;
> +            avss-supply = <&avss>;
> +            adi,slew-time-us = <125>;
> +        };
> +    };
> +
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@0 {
> +            compatible = "adi,ad5413";
> +            reg = <0>;
> +            spi-max-frequency = <1000000>;
> +            spi-cpha;
> +            avdd-supply = <&avdd>;
> +            dvdd-supply = <&dvdd>;
> +            avss-supply = <&avss>;
> +
> +            adi,chan-current-mode;
> +        };
> +    };
> 

[PATCH 2/2] iio: dac: add AD5413 driver

[Bruce Tsao via B4 Relay]

From: Bruce Tsao <bruce.tsao@analog.com>

The AD5413 is a single-channel, 14-bit, voltage and current output
digital-to-analog converter (DAC) with programmable output ranges,
slew rate control and on-chip diagnostics.

Add an IIO driver supporting the +-10.5 V voltage range and the
0-24 mA current range, output enable, slew rate control and device
reset over the SPI interface.

The device powers up with the SPI CRC enabled, so every frame carries
a CRC-8 byte and the interface is configured for SPI mode 1. This lets
the driver bring the device up directly from a cold boot without any
pre-conditioning.

Signed-off-by: Bruce Tsao <bruce.tsao@analog.com>
---
 drivers/iio/dac/Kconfig  |  11 +
 drivers/iio/dac/Makefile |   1 +
 drivers/iio/dac/ad5413.c | 666 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 678 insertions(+)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e373885e7c7a..402370c67e87 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -91,6 +91,17 @@ config AD5380
 	  To compile this driver as module choose M here: the module will be called
 	  ad5380.
 
+config AD5413
+	tristate "Analog Devices AD5413 DAC driver"
+	depends on SPI_MASTER
+	select CRC8
+	help
+	  Say yes here to build support for Analog Devices AD5413 single channel
+	  Digital to Analog Converter.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5413.
+
 config AD5421
 	tristate "Analog Devices AD5421 DAC driver"
 	depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 2cd0fe23db9e..0157face1f67 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_AD3552R_LIB) += ad3552r-common.o
 obj-$(CONFIG_AD3552R) += ad3552r.o
 obj-$(CONFIG_AD5360) += ad5360.o
 obj-$(CONFIG_AD5380) += ad5380.o
+obj-$(CONFIG_AD5413) += ad5413.o
 obj-$(CONFIG_AD5421) += ad5421.o
 obj-$(CONFIG_AD5624R_SPI) += ad5624r_spi.o
 obj-$(CONFIG_AD5064) += ad5064.o
diff --git a/drivers/iio/dac/ad5413.c b/drivers/iio/dac/ad5413.c
new file mode 100644
index 000000000000..96cce32fc640
--- /dev/null
+++ b/drivers/iio/dac/ad5413.c
@@ -0,0 +1,666 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5413 Digital to analog converters driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+#include <linux/bitfield.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iopoll.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+
+/* AD5413 registers definition */
+#define AD5413_REG_NOP 0x00
+#define AD5413_REG_DAC_INPUT 0x01
+#define AD5413_REG_DAC_OUTPUT 0x02
+#define AD5413_REG_CLEAR_CODE 0x03
+#define AD5413_REG_USER_GAIN 0x04
+#define AD5413_REG_USER_OFFSET 0x05
+#define AD5413_REG_DAC_CONFIG 0x06
+#define AD5413_REG_DAC_CONFIG_SR_STEP_MSK GENMASK(15, 13)
+#define AD5413_REG_DAC_CONFIG_SR_CLOCK_MSK GENMASK(12, 9)
+#define AD5413_REG_DAC_CONFIG_SR_EN_MSK BIT(8)
+#define AD5413_REG_DAC_CONFIG_RSET_EXT_EN_MSK BIT(7)
+#define AD5413_REG_DAC_CONFIG_OUT_EN_MSK BIT(6)
+#define AD5413_REG_DAC_CONFIG_INT_EN_MSK BIT(5)
+#define AD5413_REG_DAC_CONFIG_OVRNG_EN_MSK BIT(4)
+#define AD5413_REG_DAC_CONFIG_RANGE_MSK GENMASK(3, 0)
+#define AD5413_REG_SW_LDAC 0x07
+#define AD5413_REG_KEY 0x08
+#define AD5413_REG_KEY_REG_ADDR_MASK GENMASK(20, 16)
+#define AD5413_REG_KEY_CODE_MASK GENMASK(15, 0)
+#define AD5413_REG_GP_CONFIG1 0x09
+#define AD5413_REG_GP_CONFIG2 0x0A
+#define AD5413_REG_DIGITAL_DIAG_CONFIG 0x10
+#define AD5413_REG_FAULT_PIN_CONFIG 0x12
+#define AD5413_REG_TWO_STAGE_READBACK_SELECT 0x13
+#define AD5413_REG_DIGITAL_DIAG_RESULTS 0x14
+#define AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK BIT(15)
+#define AD5413_REG_DIGITAL_DIAG_SLEW_BUSY_MSK BIT(14)
+#define AD5413_REG_ANALOG_DIAG_RESULTS 0x15
+#define AD5413_REG_STATUS 0x16
+#define AD5413_REG_CHIP_ID 0x17
+#define AD5413_REG_FREQ_MONITOR 0x18
+#define AD5413_REG_DEVICE_ID_3 0x1C
+
+/* Special Key Codes (bits[15:0]) */
+#define AD5413_REG_KEY_CODE_RESET_1 0x15FA
+#define AD5413_REG_KEY_CODE_RESET_2 0xAF51
+#define AD5413_REG_KEY_CODE_CALIB_MEM_REFRESH 0xFCBA
+
+/* DIGITAL_DIAG_RESULTS Register (Addr = 0x14) Busy Flags */
+
+#define AD5413_WR_FLAG_MSK(x) (0x80u | ((x) & 0x1F))
+
+/* CRC-8 polynomial x^8 + x^2 + x + 1, enabled by default on the device */
+#define AD5413_CRC8_POLY 0x07
+
+#define AD5413_FULL_SCALE_MICRO (16383ULL * MICRO)
+
+struct ad5413_range {
+	int reg;
+	int min;
+	int max;
+};
+
+struct ad5413_state {
+	struct spi_device *spi;
+	struct regulator_bulk_data supplies[3];
+	struct mutex lock; /* Protects all register access via SPI */
+	const struct ad5413_range *out_range;
+	unsigned int slew_time;
+	bool pwr_down;
+	u8 crc8_tbl[CRC8_TABLE_SIZE];
+	__be32 d32[3] __aligned(IIO_DMA_MINALIGN);
+};
+
+/*
+ * Output ranges corresponding to DAC_CONFIG[3:0] bits.
+ * Defined by the following symbolic identifiers:
+ *   AD5413_RANGE_PLUSMINUS_10_5V - ±10.5 V voltage range
+ *   AD5413_RANGE_0MA_24MA        - 0 mA to 24 mA current range
+ */
+enum ad5413_output_range {
+	AD5413_RANGE_PLUSMINUS_10_5V = 0x3,
+	AD5413_RANGE_0MA_24MA = 0x9,
+};
+
+static const struct ad5413_range ad5413_voltage_range[] = { {
+	.reg = AD5413_RANGE_PLUSMINUS_10_5V,
+	.min = -10500000,
+	.max = 10500000,
+} };
+
+static const struct ad5413_range ad5413_current_range[] = { {
+	.reg = AD5413_RANGE_0MA_24MA,
+	.min = 0,
+	.max = 24000,
+} };
+
+static const int ad5413_sr_clk[16] = {
+	240000, 200000, 150000, 128000, 64000, 32000, 16000, 8000,
+	4000,	2000,	1000,	512,	256,   128,   64,    16,
+};
+
+static const int ad5413_sr_step[8] = {
+	4, 12, 64, 120, 256, 500, 1820, 2048,
+};
+
+static int ad5413_spi_reg_read(struct ad5413_state *st, unsigned int addr,
+			       u16 *val)
+{
+	struct spi_transfer t[] = {
+		{
+			.tx_buf = &st->d32[0],
+			.len = 4,
+			.cs_change = 1,
+		},
+		{
+			.tx_buf = &st->d32[1],
+			.rx_buf = &st->d32[2],
+			.len = 4,
+		},
+	};
+	u8 sel[3], nop[3];
+	int ret;
+
+	/* Two-stage readback: select the register to read back. */
+	sel[0] = AD5413_WR_FLAG_MSK(AD5413_REG_TWO_STAGE_READBACK_SELECT);
+	sel[1] = 0;
+	sel[2] = addr;
+	st->d32[0] = cpu_to_be32((sel[0] << 24) | (sel[1] << 16) |
+				 (sel[2] << 8) |
+				 crc8(st->crc8_tbl, sel, sizeof(sel), 0));
+
+	/* NOP frame clocks the selected register contents out on SDO. */
+	nop[0] = AD5413_WR_FLAG_MSK(AD5413_REG_NOP);
+	nop[1] = 0;
+	nop[2] = 0;
+	st->d32[1] = cpu_to_be32((nop[0] << 24) | (nop[1] << 16) |
+				 (nop[2] << 8) |
+				 crc8(st->crc8_tbl, nop, sizeof(nop), 0));
+
+	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * The readback CRC covers the live FAULT pin status bit, which can
+	 * toggle between capture and clock-out, so it is not validated here.
+	 */
+	*val = be32_to_cpu(st->d32[2]) >> 8;
+
+	return 0;
+}
+
+static int ad5413_spi_reg_write(struct ad5413_state *st, unsigned int addr,
+				unsigned int val)
+{
+	u8 buf[3] = { };
+
+	buf[0] = AD5413_WR_FLAG_MSK(addr);
+	buf[1] = val >> 8;
+	buf[2] = val;
+
+	st->d32[0] = cpu_to_be32((buf[0] << 24) | (buf[1] << 16) |
+				 (buf[2] << 8) |
+				 crc8(st->crc8_tbl, buf, sizeof(buf), 0));
+
+	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
+}
+
+static int ad5413_spi_write_mask(struct ad5413_state *st, unsigned int addr,
+				 unsigned long mask, unsigned int val)
+{
+	u16 regval;
+	int ret;
+
+	ret = ad5413_spi_reg_read(st, addr, &regval);
+	if (ret)
+		return ret;
+
+	return ad5413_spi_reg_write(st, addr, (regval & ~mask) | val);
+}
+
+static int ad5413_wait_for_task_complete(struct ad5413_state *st,
+					 unsigned int reg, unsigned int mask)
+{
+	u16 val;
+	int ret, err;
+
+	err = read_poll_timeout(ad5413_spi_reg_read, ret,
+				ret || !(val & mask),
+				100, 1000, false, st, reg, &val);
+	if (ret)
+		return ret;
+
+	if (err) {
+		dev_err(&st->spi->dev,
+			"Timeout waiting for bit 0x%x to clear in reg 0x%x\n",
+			mask, reg);
+		return err;
+	}
+
+	return 0;
+}
+
+static int ad5413_calib_mem_refresh(struct ad5413_state *st)
+{
+	int ret;
+
+	ret = ad5413_spi_reg_write(st, AD5413_REG_KEY,
+				   AD5413_REG_KEY_CODE_CALIB_MEM_REFRESH);
+	if (ret < 0)
+		return dev_err_probe(&st->spi->dev, ret,
+				     "Failed to initiate a calibration memory refresh\n");
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5413_wait_for_task_complete(st,
+					     AD5413_REG_DIGITAL_DIAG_RESULTS,
+					     AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5413_slew_rate_set(struct ad5413_state *st,
+				unsigned int sr_clk_idx,
+				unsigned int sr_step_idx)
+{
+	unsigned int mode;
+	unsigned long mask;
+	int ret;
+
+	mask = AD5413_REG_DAC_CONFIG_SR_EN_MSK |
+	       AD5413_REG_DAC_CONFIG_SR_CLOCK_MSK |
+	       AD5413_REG_DAC_CONFIG_SR_STEP_MSK;
+	mode = FIELD_PREP(AD5413_REG_DAC_CONFIG_SR_EN_MSK, 1) |
+	       FIELD_PREP(AD5413_REG_DAC_CONFIG_SR_STEP_MSK, sr_step_idx) |
+	       FIELD_PREP(AD5413_REG_DAC_CONFIG_SR_CLOCK_MSK, sr_clk_idx);
+
+	ret = ad5413_spi_write_mask(st, AD5413_REG_DAC_CONFIG, mask, mode);
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5413_wait_for_task_complete(st,
+					     AD5413_REG_DIGITAL_DIAG_RESULTS,
+					     AD5413_REG_DIGITAL_DIAG_SLEW_BUSY_MSK);
+}
+
+static int ad5413_slew_rate_config(struct ad5413_state *st)
+{
+	unsigned int sr_clk_idx = 0, sr_step_idx = 0;
+	int i, res;
+	s64 diff_new, diff_old = S64_MAX;
+	u64 sr_step, calc_slew_time;
+
+	/*
+	 * The slew time can be determined by using the formula:
+	 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
+	 * where Slew time is expressed in microseconds
+	 * Given the desired slew time, the following algorithm determines the
+	 * best match for the step size and the update clock frequency.
+	 */
+	for (i = 0; i < ARRAY_SIZE(ad5413_sr_clk); i++) {
+		/*
+		 * Go through each valid update clock freq and determine a raw
+		 * value for the step size by using the formula:
+		 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
+		 */
+		sr_step = div64_u64(AD5413_FULL_SCALE_MICRO,
+				    (u64)ad5413_sr_clk[i] * st->slew_time);
+		/*
+		 * After a raw value for step size was determined, find the
+		 * closest valid match
+		 */
+		res = find_closest(sr_step, ad5413_sr_step, ARRAY_SIZE(ad5413_sr_step));
+
+		/* Calculate the slew time */
+		calc_slew_time = div_u64(AD5413_FULL_SCALE_MICRO,
+					 ad5413_sr_step[res] * ad5413_sr_clk[i]);
+		/*
+		 * Determine with how many microseconds the calculated slew time
+		 * is different from the desired slew time and store the diff
+		 * for the next iteration
+		 */
+		diff_new = abs(st->slew_time - calc_slew_time);
+		if (diff_new < diff_old) {
+			diff_old = diff_new;
+			sr_clk_idx = i;
+			sr_step_idx = res;
+		}
+	}
+
+	return ad5413_slew_rate_set(st, sr_clk_idx, sr_step_idx);
+}
+
+static int ad5413_set_out_range(struct ad5413_state *st, int range)
+{
+	int ret;
+
+	ret = ad5413_spi_write_mask(st,
+				    AD5413_REG_DAC_CONFIG,
+				    AD5413_REG_DAC_CONFIG_RANGE_MSK,
+				    FIELD_PREP(AD5413_REG_DAC_CONFIG_RANGE_MSK,
+					       range));
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5413_wait_for_task_complete(st,
+					     AD5413_REG_DIGITAL_DIAG_RESULTS,
+					     AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5413_internal_buffers_en(struct ad5413_state *st, bool enable)
+{
+	int ret;
+
+	ret = ad5413_spi_write_mask(st,
+				    AD5413_REG_DAC_CONFIG,
+				    AD5413_REG_DAC_CONFIG_INT_EN_MSK,
+				    FIELD_PREP(AD5413_REG_DAC_CONFIG_INT_EN_MSK,
+					       enable));
+	if (ret < 0)
+		return ret;
+
+	/* Wait to allow time for the internal calibrations to complete */
+	return ad5413_wait_for_task_complete(st,
+					     AD5413_REG_DIGITAL_DIAG_RESULTS,
+					     AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK);
+}
+
+static int ad5413_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			     unsigned int writeval, unsigned int *readval)
+{
+	struct ad5413_state *st = iio_priv(indio_dev);
+	u16 regval;
+	int ret;
+
+	guard(mutex)(&st->lock);
+	if (readval) {
+		ret = ad5413_spi_reg_read(st, reg, &regval);
+		if (ret)
+			return ret;
+
+		*readval = regval;
+		return 0;
+	}
+
+	return ad5413_spi_reg_write(st, reg, writeval);
+}
+
+static int ad5413_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long info)
+{
+	struct ad5413_state *st = iio_priv(indio_dev);
+	int max, min, ret;
+	u16 regval;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
+		ret = ad5413_spi_reg_read(st, AD5413_REG_DAC_INPUT, &regval);
+		if (ret)
+			return ret;
+
+		/* 14-bit DAC data is in bits [15:2] of the register. */
+		*val = regval >> 2;
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SCALE:
+		min = st->out_range->min;
+		max = st->out_range->max;
+		*val = (max - min) / 1000;
+		*val2 = 14;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	case IIO_CHAN_INFO_OFFSET:
+		min = st->out_range->min;
+		max = st->out_range->max;
+		*val = div_s64((s64)min * BIT(14), max - min);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5413_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long info)
+{
+	struct ad5413_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
+		/* 14-bit DAC data is in bits [15:2] of the register. */
+		ret = ad5413_spi_reg_write(st, AD5413_REG_DAC_INPUT, val << 2);
+		return ret;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static ssize_t ad5413_read_powerdown(struct iio_dev *indio_dev, uintptr_t priv,
+				     const struct iio_chan_spec *chan,
+				     char *buf)
+{
+	struct ad5413_state *st = iio_priv(indio_dev);
+
+	return sysfs_emit(buf, "%d\n", st->pwr_down);
+}
+
+static ssize_t ad5413_write_powerdown(struct iio_dev *indio_dev, uintptr_t priv,
+				      struct iio_chan_spec const *chan,
+				      const char *buf, size_t len)
+{
+	struct ad5413_state *st = iio_priv(indio_dev);
+	bool pwr_down;
+	unsigned int dac_config_mode, val;
+	unsigned long dac_config_msk;
+	int ret;
+
+	ret = kstrtobool(buf, &pwr_down);
+	if (ret)
+		return ret;
+
+	val = pwr_down ? 0 : 1;
+
+	dac_config_mode = FIELD_PREP(AD5413_REG_DAC_CONFIG_OUT_EN_MSK, (val)) |
+			  FIELD_PREP(AD5413_REG_DAC_CONFIG_INT_EN_MSK, (val));
+	dac_config_msk = AD5413_REG_DAC_CONFIG_OUT_EN_MSK |
+			 AD5413_REG_DAC_CONFIG_INT_EN_MSK;
+
+	guard(mutex)(&st->lock);
+	ret = ad5413_spi_write_mask(st, AD5413_REG_DAC_CONFIG, dac_config_msk,
+				    dac_config_mode);
+	if (ret < 0)
+		return ret;
+
+	st->pwr_down = pwr_down;
+
+	return len;
+}
+
+static const struct iio_info ad5413_info = {
+	.read_raw = ad5413_read_raw,
+	.write_raw = ad5413_write_raw,
+	.debugfs_reg_access = &ad5413_reg_access,
+};
+
+static const struct iio_chan_spec_ext_info ad5413_ext_info[] = {
+	{
+		.name = "powerdown",
+		.read = ad5413_read_powerdown,
+		.write = ad5413_write_powerdown,
+		.shared = IIO_SHARED_BY_TYPE,
+	},
+	{ }
+};
+
+#define AD5413_DAC_CHAN(_chan_type)                                    \
+	{                                                              \
+		.type = (_chan_type),                                  \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+					    BIT(IIO_CHAN_INFO_OFFSET), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),          \
+		.indexed = 1,                                          \
+		.output = 1,                                           \
+		.ext_info = ad5413_ext_info,                           \
+	}
+
+static const struct iio_chan_spec ad5413_voltage_ch[] = {
+	AD5413_DAC_CHAN(IIO_VOLTAGE),
+};
+
+static const struct iio_chan_spec ad5413_current_ch[] = {
+	AD5413_DAC_CHAN(IIO_CURRENT),
+};
+
+static int ad5413_parse_fw(struct ad5413_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	int ret;
+
+	if (device_property_read_bool(dev, "adi,chan-current-mode"))
+		st->out_range = &ad5413_current_range[0];
+	else
+		st->out_range = &ad5413_voltage_range[0];
+
+	st->slew_time = 0;
+	if (device_property_present(dev, "adi,slew-time-us")) {
+		ret = device_property_read_u32(dev,
+					       "adi,slew-time-us",
+					       &st->slew_time);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to read adi,slew-time-us\n");
+
+		if (st->slew_time > 1 * MICRO)
+			return dev_err_probe(dev, -EINVAL, "adi,slew-time-us out of range\n");
+	}
+
+	return 0;
+}
+
+static int ad5413_init(struct ad5413_state *st)
+{
+	struct reset_control *reset;
+	u16 regval;
+	int ret;
+
+	reset = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
+	if (IS_ERR(reset))
+		return PTR_ERR(reset);
+
+	if (reset) {
+		/* Hard reset via the reset controller */
+		ret = reset_control_assert(reset);
+		if (ret)
+			return ret;
+
+		fsleep(100);
+
+		ret = reset_control_deassert(reset);
+		if (ret)
+			return ret;
+
+		fsleep(100);
+	} else {
+		/*
+		 * CRC is enabled by default after power-on/reset. The SPI
+		 * helpers always append the CRC byte, so the device accepts
+		 * these soft-reset key frames without any bootstrap step.
+		 */
+		ret = ad5413_spi_reg_write(st, AD5413_REG_KEY,
+					   AD5413_REG_KEY_CODE_RESET_1);
+		if (ret)
+			return ret;
+
+		ret = ad5413_spi_reg_write(st, AD5413_REG_KEY,
+					   AD5413_REG_KEY_CODE_RESET_2);
+		if (ret)
+			return ret;
+
+		fsleep(100);
+	}
+
+	ret = ad5413_calib_mem_refresh(st);
+	if (ret < 0)
+		return ret;
+
+	ret = ad5413_spi_reg_read(st, AD5413_REG_DIGITAL_DIAG_RESULTS, &regval);
+	if (ret)
+		return ret;
+
+	ret = ad5413_spi_reg_write(st, AD5413_REG_DIGITAL_DIAG_RESULTS, regval);
+	if (ret < 0)
+		return ret;
+
+	ret = ad5413_set_out_range(st, st->out_range->reg);
+	if (ret < 0)
+		return ret;
+
+	if (st->slew_time) {
+		ret = ad5413_slew_rate_config(st);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = ad5413_internal_buffers_en(st, 1);
+	if (ret < 0)
+		return ret;
+
+	return ad5413_spi_write_mask(st,
+				    AD5413_REG_DAC_CONFIG,
+				    AD5413_REG_DAC_CONFIG_OUT_EN_MSK,
+				    FIELD_PREP(AD5413_REG_DAC_CONFIG_OUT_EN_MSK, 1));
+}
+
+static const char * const ad5413_supply_names[] = {
+	"avdd",
+	"dvdd",
+	"avss",
+};
+
+static int ad5413_probe(struct spi_device *spi)
+{
+	struct ad5413_state *st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	st->spi = spi;
+
+	ret = devm_mutex_init(&spi->dev, &st->lock);
+	if (ret)
+		return ret;
+
+	crc8_populate_msb(st->crc8_tbl, AD5413_CRC8_POLY);
+
+	indio_dev->name = "ad5413";
+	indio_dev->info = &ad5413_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->num_channels = 1;
+
+	ret = ad5413_parse_fw(st);
+	if (ret < 0)
+		return ret;
+
+	if (st->out_range->reg == AD5413_RANGE_PLUSMINUS_10_5V)
+		indio_dev->channels = ad5413_voltage_ch;
+	else
+		indio_dev->channels = ad5413_current_ch;
+
+	ret = devm_regulator_bulk_get_enable(&spi->dev,
+					     ARRAY_SIZE(ad5413_supply_names),
+					     ad5413_supply_names);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "failed to enable supplies\n");
+
+	ret = ad5413_init(st);
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret, "AD5413 init failed\n");
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad5413_id[] = {
+	{ .name = "ad5413" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad5413_id);
+
+static const struct of_device_id ad5413_of_match[] = {
+	{ .compatible = "adi,ad5413" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5413_of_match);
+
+static struct spi_driver ad5413_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = ad5413_of_match,
+	},
+	.probe = ad5413_probe,
+	.id_table = ad5413_id,
+};
+module_spi_driver(ad5413_driver);
+
+MODULE_AUTHOR("Bruce Tsao <bruce.tsao@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5413 DAC");
+MODULE_LICENSE("GPL");

-- 
2.53.0

Re: [PATCH 2/2] iio: dac: add AD5413 driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] iio: dac: ad5413: missing range check in ad5413_write_raw()
- [Low] iio: dac: ad5413: unused supplies array in struct ad5413_state
--

commit bcfddef4b6cda1b4c239a6094a38aa835c2860bd
Author: Bruce Tsao <bruce.tsao@analog.com>

iio: dac: add AD5413 driver

This commit adds an IIO driver for the Analog Devices AD5413 single-channel
14-bit voltage and current output DAC over SPI.

> diff --git a/drivers/iio/dac/ad5413.c b/drivers/iio/dac/ad5413.c
> new file mode 100644
> index 0000000000000..96cce32fc640e
> --- /dev/null
> +++ b/drivers/iio/dac/ad5413.c
> @@ -0,0 +1,666 @@

[ ... ]

> +struct ad5413_state {
> +	struct spi_device *spi;
> +	struct regulator_bulk_data supplies[3];

[Severity: Low]
Is supplies[3] in struct ad5413_state needed? ad5413_probe() enables the
regulators using devm_regulator_bulk_get_enable() with ad5413_supply_names, so
this array appears to be unused.

> +	struct mutex lock; /* Protects all register access via SPI */
> +	const struct ad5413_range *out_range;
> +	unsigned int slew_time;
> +	bool pwr_down;
> +	u8 crc8_tbl[CRC8_TABLE_SIZE];
> +	__be32 d32[3] __aligned(IIO_DMA_MINALIGN);
> +};

[ ... ]

> +static int ad5413_spi_reg_write(struct ad5413_state *st, unsigned int addr,
> +				unsigned int val)
> +{
> +	u8 buf[3] = { };
> +
> +	buf[0] = AD5413_WR_FLAG_MSK(addr);
> +	buf[1] = val >> 8;
> +	buf[2] = val;
> +
> +	st->d32[0] = cpu_to_be32((buf[0] << 24) | (buf[1] << 16) |
> +				 (buf[2] << 8) |
> +				 crc8(st->crc8_tbl, buf, sizeof(buf), 0));
> +
> +	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
> +}

[ ... ]

> +static int ad5413_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val, int val2,
> +			    long info)
> +{
> +	struct ad5413_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&st->lock);
> +		/* 14-bit DAC data is in bits [15:2] of the register. */
> +		ret = ad5413_spi_reg_write(st, AD5413_REG_DAC_INPUT, val << 2);
> +		return ret;
> +	}

[Severity: Medium]
Should ad5413_write_raw() validate that val is within the 14-bit range
(0 <= val < BIT(14)) before shifting and writing to AD5413_REG_DAC_INPUT?

When userspace writes a negative value or a value >= BIT(14) to the raw sysfs
attribute, evaluating val << 2 on a negative signed int or a large positive
int triggers signed left-shift undefined behavior, and ad5413_spi_reg_write()
silently truncates the shifted value into the lower 16 bits of buf[1] and
buf[2], programming an unintended DAC output instead of returning -EINVAL.

> +	default:
> +		return -EINVAL;
> +	}
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017-ad5413-upstream-v1-0-61f40c8fef25@analog.com?part=2

Re: [PATCH 2/2] iio: dac: add AD5413 driver

[Jonathan Cameron]

On Sat, 17 Oct 2026 03:23:54 +0800
Bruce Tsao via B4 Relay <devnull+bruce.tsao.analog.com@kernel.org> wrote:

> From: Bruce Tsao <bruce.tsao@analog.com>
> 
> The AD5413 is a single-channel, 14-bit, voltage and current output
> digital-to-analog converter (DAC) with programmable output ranges,
> slew rate control and on-chip diagnostics.
> 
> Add an IIO driver supporting the +-10.5 V voltage range and the
> 0-24 mA current range, output enable, slew rate control and device
> reset over the SPI interface.
> 
> The device powers up with the SPI CRC enabled, so every frame carries
> a CRC-8 byte and the interface is configured for SPI mode 1. This lets
> the driver bring the device up directly from a cold boot without any
> pre-conditioning.
> 
> Signed-off-by: Bruce Tsao <bruce.tsao@analog.com>
Hi Bruce

Good state for a v1.  Main thing in my feedback is take
another look at the endian conversions. It think those
have evolved into being way more complex than they need to be.

Jonathan

> diff --git a/drivers/iio/dac/ad5413.c b/drivers/iio/dac/ad5413.c
> new file mode 100644
> index 000000000000..96cce32fc640
> --- /dev/null
> +++ b/drivers/iio/dac/ad5413.c
> @@ -0,0 +1,666 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AD5413 Digital to analog converters driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iopoll.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/spi/spi.h>
> +#include <linux/units.h>
> +#include <linux/util_macros.h>
> +
> +/* AD5413 registers definition */
> +#define AD5413_REG_NOP 0x00
> +#define AD5413_REG_DAC_INPUT 0x01
> +#define AD5413_REG_DAC_OUTPUT 0x02
> +#define AD5413_REG_CLEAR_CODE 0x03
> +#define AD5413_REG_USER_GAIN 0x04
> +#define AD5413_REG_USER_OFFSET 0x05
> +#define AD5413_REG_DAC_CONFIG 0x06
> +#define AD5413_REG_DAC_CONFIG_SR_STEP_MSK GENMASK(15, 13)
Mixing fields and register defines can make for hard reading. I sugest
using a bit of indenting to make them visually different.
#define AD5413_REG_DAC_CONFIG 0x06
#define   AD5413_REG_DAC_CONFIG_SR_STEP_MSK GENMASK(15, 13)
#define   AD5413_REG_DAC_CONFIG_SR_CLOCK_MSK GENMASK(12, 9)
#define   AD5413_REG_DAC_CONFIG_SR_EN_MSK BIT(8)
#define   AD5413_REG_DAC_CONFIG_RSET_EXT_EN_MSK BIT(7)
#define   AD5413_REG_DAC_CONFIG_OUT_EN_MSK BIT(6)
#define   AD5413_REG_DAC_CONFIG_INT_EN_MSK BIT(5)
#define   AD5413_REG_DAC_CONFIG_OVRNG_EN_MSK BIT(4)
#define   AD5413_REG_DAC_CONFIG_RANGE_MSK GENMASK(3, 0)
define AD5413_REG_SW_LDAC 0x07

etc. If you have values of fields defined, then indent them
even further.

> +#define AD5413_REG_DAC_CONFIG_SR_CLOCK_MSK GENMASK(12, 9)
> +#define AD5413_REG_DAC_CONFIG_SR_EN_MSK BIT(8)
> +#define AD5413_REG_DAC_CONFIG_RSET_EXT_EN_MSK BIT(7)
> +#define AD5413_REG_DAC_CONFIG_OUT_EN_MSK BIT(6)
> +#define AD5413_REG_DAC_CONFIG_INT_EN_MSK BIT(5)
> +#define AD5413_REG_DAC_CONFIG_OVRNG_EN_MSK BIT(4)
> +#define AD5413_REG_DAC_CONFIG_RANGE_MSK GENMASK(3, 0)
> +#define AD5413_REG_SW_LDAC 0x07
> +#define AD5413_REG_KEY 0x08
> +#define AD5413_REG_KEY_REG_ADDR_MASK GENMASK(20, 16)
> +#define AD5413_REG_KEY_CODE_MASK GENMASK(15, 0)
> +#define AD5413_REG_GP_CONFIG1 0x09
> +#define AD5413_REG_GP_CONFIG2 0x0A
> +#define AD5413_REG_DIGITAL_DIAG_CONFIG 0x10
> +#define AD5413_REG_FAULT_PIN_CONFIG 0x12
> +#define AD5413_REG_TWO_STAGE_READBACK_SELECT 0x13
> +#define AD5413_REG_DIGITAL_DIAG_RESULTS 0x14
> +#define AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK BIT(15)
> +#define AD5413_REG_DIGITAL_DIAG_SLEW_BUSY_MSK BIT(14)
> +#define AD5413_REG_ANALOG_DIAG_RESULTS 0x15
> +#define AD5413_REG_STATUS 0x16
> +#define AD5413_REG_CHIP_ID 0x17
> +#define AD5413_REG_FREQ_MONITOR 0x18
> +#define AD5413_REG_DEVICE_ID_3 0x1C
> +
> +/* Special Key Codes (bits[15:0]) */
> +#define AD5413_REG_KEY_CODE_RESET_1 0x15FA
> +#define AD5413_REG_KEY_CODE_RESET_2 0xAF51
> +#define AD5413_REG_KEY_CODE_CALIB_MEM_REFRESH 0xFCBA
> +
> +/* DIGITAL_DIAG_RESULTS Register (Addr = 0x14) Busy Flags */
> +
> +#define AD5413_WR_FLAG_MSK(x) (0x80u | ((x) & 0x1F))
> +
> +/* CRC-8 polynomial x^8 + x^2 + x + 1, enabled by default on the device */
> +#define AD5413_CRC8_POLY 0x07
> +
> +#define AD5413_FULL_SCALE_MICRO (16383ULL * MICRO)
> +
> +struct ad5413_range {
> +	int reg;
> +	int min;
> +	int max;
> +};
> +
> +struct ad5413_state {
> +	struct spi_device *spi;
> +	struct regulator_bulk_data supplies[3];
> +	struct mutex lock; /* Protects all register access via SPI */

Not necessary to protect data accesses in general as spi will do that.
The point you want to make is about rmw sequences.

> +	const struct ad5413_range *out_range;
> +	unsigned int slew_time;
> +	bool pwr_down;
> +	u8 crc8_tbl[CRC8_TABLE_SIZE];
> +	__be32 d32[3] __aligned(IIO_DMA_MINALIGN);

As below. I'd throw in a d8 array to simplify the data mangling.

> +};
> +
> +/*
> + * Output ranges corresponding to DAC_CONFIG[3:0] bits.
> + * Defined by the following symbolic identifiers:
> + *   AD5413_RANGE_PLUSMINUS_10_5V - ±10.5 V voltage range
> + *   AD5413_RANGE_0MA_24MA        - 0 mA to 24 mA current range
> + */
> +enum ad5413_output_range {
> +	AD5413_RANGE_PLUSMINUS_10_5V = 0x3,
> +	AD5413_RANGE_0MA_24MA = 0x9,
> +};
> +
> +static const struct ad5413_range ad5413_voltage_range[] = { {

Given only one element, why the array?

> +	.reg = AD5413_RANGE_PLUSMINUS_10_5V,
> +	.min = -10500000,
> +	.max = 10500000,
> +} };
> +
> +static const struct ad5413_range ad5413_current_range[] = { {

Same on this one.

> +	.reg = AD5413_RANGE_0MA_24MA,
> +	.min = 0,
> +	.max = 24000,
> +} };

> +static int ad5413_spi_reg_read(struct ad5413_state *st, unsigned int addr,
> +			       u16 *val)
> +{
> +	struct spi_transfer t[] = {
> +		{
> +			.tx_buf = &st->d32[0],
> +			.len = 4,
> +			.cs_change = 1,
> +		},
> +		{
> +			.tx_buf = &st->d32[1],
> +			.rx_buf = &st->d32[2],
> +			.len = 4,
> +		},
> +	};
> +	u8 sel[3], nop[3];
> +	int ret;
> +
> +	/* Two-stage readback: select the register to read back. */
> +	sel[0] = AD5413_WR_FLAG_MSK(AD5413_REG_TWO_STAGE_READBACK_SELECT);
> +	sel[1] = 0;
> +	sel[2] = addr;
> +	st->d32[0] = cpu_to_be32((sel[0] << 24) | (sel[1] << 16) |
> +				 (sel[2] << 8) |
> +				 crc8(st->crc8_tbl, sel, sizeof(sel), 0));

See below. I suspect similarly overly complicated endian dance.

> +
> +	/* NOP frame clocks the selected register contents out on SDO. */
> +	nop[0] = AD5413_WR_FLAG_MSK(AD5413_REG_NOP);
> +	nop[1] = 0;
> +	nop[2] = 0;
> +	st->d32[1] = cpu_to_be32((nop[0] << 24) | (nop[1] << 16) |
> +				 (nop[2] << 8) |
> +				 crc8(st->crc8_tbl, nop, sizeof(nop), 0));
> +
> +	ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * The readback CRC covers the live FAULT pin status bit, which can
> +	 * toggle between capture and clock-out, so it is not validated here.
> +	 */
> +	*val = be32_to_cpu(st->d32[2]) >> 8;
> +
> +	return 0;
> +}
> +
> +static int ad5413_spi_reg_write(struct ad5413_state *st, unsigned int addr,
> +				unsigned int val)
> +{
> +	u8 buf[3] = { };
	u8 buf[3] = {
		[0] = AD...

> +
> +	buf[0] = AD5413_WR_FLAG_MSK(addr);
> +	buf[1] = val >> 8;
> +	buf[2] = val;

This is fun as that's a put_unaligned_be16 which you then do
another endian swap on.

> +
> +	st->d32[0] = cpu_to_be32((buf[0] << 24) | (buf[1] << 16) |
> +				 (buf[2] << 8) |

Ok. I'm lost.

In both cass we start with
	buf[0] = AD5413_WR_FLAG_MSK(addr),
	buf[1] = (val >> 8) & 0xff;
	buf[2] = val & 0xff;

Then you build a native endian integer. So on a little endian cpu that means
	
	v[0] = crc8(buf..)
	v[1] = val & 0xff;
	v[2] = (val >> 8) & 0xff;
	v[3] = AD5413_WR_FLAG_MSK(addr)

	d32 is that swapped (pretend it's u8 array)
	d32[0] = AD5413_WR_FLAG_MSK(addr);
	d32[1] = (val >> 8) & 0xff;
	d32[2] =  val & 0xff;
	d32[3] = crc8(buf..)

on a big endian
	v[0] = AD5413_WR_FLAG_MSK(addr)
	v[1] = (val >> 8) & 0xff;
	v[2] = val & 0xff;
	v[3] = crc8(buf..)
d32 is not swapped. (again pretend u8 array
	d32[0] = AD5413_WR_FLAG_MSK(addr);
	d32[1] = (val >> 8) & 0xff;
	d32[2] =  val & 0xff;
	d32[3] = crc8(buf..)
(snap which is good).

Fun bit, is the 3 bytes are right for computing the crc8 so... This is all
equivalent of:
static int ad5413_spi_reg_write(struct ad5413_state *st, unsigned int addr,
				unsigned int val)
{
	st->d8[0] = AD5413_WR_FLAG_MSK(addr);
	put_unaligned_be16(&st->d8[1], val);
	st->d8[3] = crc8(st->crc8_tbl, buf, 3, 0);

	return spi_write(st->spi, st->d8, sizeof(st->d8));
}
As long as you have a st->d8[4] array to use (and I didn't unwind
that all incorrectly).

> +				 crc8(st->crc8_tbl, buf, sizeof(buf), 0));
> +
> +	return spi_write(st->spi, &st->d32[0], sizeof(st->d32[0]));
> +}
> +
> +static int ad5413_spi_write_mask(struct ad5413_state *st, unsigned int addr,
> +				 unsigned long mask, unsigned int val)
> +{
> +	u16 regval;
> +	int ret;
> +
> +	ret = ad5413_spi_reg_read(st, addr, &regval);
> +	if (ret)
> +		return ret;
> +
> +	return ad5413_spi_reg_write(st, addr, (regval & ~mask) | val);
> +}


> +
> +static int ad5413_slew_rate_config(struct ad5413_state *st)
> +{
> +	unsigned int sr_clk_idx = 0, sr_step_idx = 0;
> +	int i, res;
> +	s64 diff_new, diff_old = S64_MAX;
> +	u64 sr_step, calc_slew_time;
> +
> +	/*
> +	 * The slew time can be determined by using the formula:
> +	 * Slew Time = (Full Scale Out / (Step Size x Update Clk Freq))
> +	 * where Slew time is expressed in microseconds
> +	 * Given the desired slew time, the following algorithm determines the
> +	 * best match for the step size and the update clock frequency.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(ad5413_sr_clk); i++) {

	for (int i = 0;

is acceptable these days and makes the scope explicit.

> +		/*
> +		 * Go through each valid update clock freq and determine a raw
> +		 * value for the step size by using the formula:
> +		 * Step Size = Full Scale Out / (Update Clk Freq * Slew Time)
> +		 */
> +		sr_step = div64_u64(AD5413_FULL_SCALE_MICRO,
> +				    (u64)ad5413_sr_clk[i] * st->slew_time);
> +		/*
> +		 * After a raw value for step size was determined, find the
> +		 * closest valid match
> +		 */
> +		res = find_closest(sr_step, ad5413_sr_step, ARRAY_SIZE(ad5413_sr_step));
> +
> +		/* Calculate the slew time */
> +		calc_slew_time = div_u64(AD5413_FULL_SCALE_MICRO,
> +					 ad5413_sr_step[res] * ad5413_sr_clk[i]);
> +		/*
> +		 * Determine with how many microseconds the calculated slew time
> +		 * is different from the desired slew time and store the diff
> +		 * for the next iteration
> +		 */
> +		diff_new = abs(st->slew_time - calc_slew_time);
> +		if (diff_new < diff_old) {
> +			diff_old = diff_new;
> +			sr_clk_idx = i;
> +			sr_step_idx = res;
> +		}
> +	}
> +
> +	return ad5413_slew_rate_set(st, sr_clk_idx, sr_step_idx);
> +}

> +
> +static int ad5413_internal_buffers_en(struct ad5413_state *st, bool enable)
> +{
> +	int ret;
> +
> +	ret = ad5413_spi_write_mask(st,
> +				    AD5413_REG_DAC_CONFIG,
> +				    AD5413_REG_DAC_CONFIG_INT_EN_MSK,
> +				    FIELD_PREP(AD5413_REG_DAC_CONFIG_INT_EN_MSK,
> +					       enable));
> +	if (ret < 0)
> +		return ret;
I'd not expect a function with that name to return > 0 so perhaps
just if (ret)?

> +
> +	/* Wait to allow time for the internal calibrations to complete */

The function name and parameters are probably clear enough that you can
drop the comment.

> +	return ad5413_wait_for_task_complete(st,
> +					     AD5413_REG_DIGITAL_DIAG_RESULTS,
> +					     AD5413_REG_DIGITAL_DIAG_CAL_MEM_UNREFRESHED_MSK);
> +}

> +static int ad5413_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long info)
> +{
> +	struct ad5413_state *st = iio_priv(indio_dev);
> +	int max, min, ret;
> +	u16 regval;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&st->lock);
> +		ret = ad5413_spi_reg_read(st, AD5413_REG_DAC_INPUT, &regval);
> +		if (ret)
> +			return ret;
> +
> +		/* 14-bit DAC data is in bits [15:2] of the register. */
> +		*val = regval >> 2;
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		min = st->out_range->min;
> +		max = st->out_range->max;
> +		*val = (max - min) / 1000;
> +		*val2 = 14;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	case IIO_CHAN_INFO_OFFSET:
> +		min = st->out_range->min;
> +		max = st->out_range->max;
> +		*val = div_s64((s64)min * BIT(14), max - min);
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ad5413_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int val, int val2,
> +			    long info)
> +{
> +	struct ad5413_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&st->lock);
> +		/* 14-bit DAC data is in bits [15:2] of the register. */
> +		ret = ad5413_spi_reg_write(st, AD5413_REG_DAC_INPUT, val << 2);
> +		return ret;

		return ad54...

> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static ssize_t ad5413_write_powerdown(struct iio_dev *indio_dev, uintptr_t priv,
> +				      struct iio_chan_spec const *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ad5413_state *st = iio_priv(indio_dev);
> +	bool pwr_down;
> +	unsigned int dac_config_mode, val;
> +	unsigned long dac_config_msk;

Where nothing else dictates ordering, reverse xmas tree preferred.

> +	int ret;
> +
> +	ret = kstrtobool(buf, &pwr_down);
> +	if (ret)
> +		return ret;
> +
> +	val = pwr_down ? 0 : 1;
> +
> +	dac_config_mode = FIELD_PREP(AD5413_REG_DAC_CONFIG_OUT_EN_MSK, (val)) |
> +			  FIELD_PREP(AD5413_REG_DAC_CONFIG_INT_EN_MSK, (val));
> +	dac_config_msk = AD5413_REG_DAC_CONFIG_OUT_EN_MSK |
> +			 AD5413_REG_DAC_CONFIG_INT_EN_MSK;
> +
> +	guard(mutex)(&st->lock);
> +	ret = ad5413_spi_write_mask(st, AD5413_REG_DAC_CONFIG, dac_config_msk,
> +				    dac_config_mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	st->pwr_down = pwr_down;
> +
> +	return len;
> +}

> +static const struct iio_chan_spec ad5413_voltage_ch[] = {
> +	AD5413_DAC_CHAN(IIO_VOLTAGE),
> +};
> +
> +static const struct iio_chan_spec ad5413_current_ch[] = {
> +	AD5413_DAC_CHAN(IIO_CURRENT),

Why an array?  For these, given you are just copying them in place
maybe use a macro that has (struct iio_chan_spec) { values }
so that you can just have the assignment in code without the
need for these two template arrays.

> +};
> +
> +static int ad5413_parse_fw(struct ad5413_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	int ret;
> +
> +	if (device_property_read_bool(dev, "adi,chan-current-mode"))
> +		st->out_range = &ad5413_current_range[0];

I ask this above, but it was motivated by this. If there is only
one structure why do we want an array for these?

> +	else
> +		st->out_range = &ad5413_voltage_range[0];
> +
> +	st->slew_time = 0;
> +	if (device_property_present(dev, "adi,slew-time-us")) {
> +		ret = device_property_read_u32(dev,
> +					       "adi,slew-time-us",
> +					       &st->slew_time);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to read adi,slew-time-us\n");
> +
> +		if (st->slew_time > 1 * MICRO)
> +			return dev_err_probe(dev, -EINVAL, "adi,slew-time-us out of range\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad5413_init(struct ad5413_state *st)
> +{
> +	struct reset_control *reset;
> +	u16 regval;
> +	int ret;
> +
> +	reset = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
> +	if (IS_ERR(reset))
> +		return PTR_ERR(reset);
> +
> +	if (reset) {
> +		/* Hard reset via the reset controller */
> +		ret = reset_control_assert(reset);
> +		if (ret)
> +			return ret;
> +
> +		fsleep(100);

For most sleeps a comment that refers to the datasheet is useful
so if people hit problems in future they understand why the sleeps take
particular values. If the datasheet is uninformative on a particular
value just say something about having found it to work in practice.

> +
> +		ret = reset_control_deassert(reset);
> +		if (ret)
> +			return ret;
> +
> +		fsleep(100);
> +	} else {
> +		/*
> +		 * CRC is enabled by default after power-on/reset. The SPI
> +		 * helpers always append the CRC byte, so the device accepts
> +		 * these soft-reset key frames without any bootstrap step.
> +		 */
> +		ret = ad5413_spi_reg_write(st, AD5413_REG_KEY,
> +					   AD5413_REG_KEY_CODE_RESET_1);
> +		if (ret)
> +			return ret;
> +
> +		ret = ad5413_spi_reg_write(st, AD5413_REG_KEY,
> +					   AD5413_REG_KEY_CODE_RESET_2);
> +		if (ret)
> +			return ret;
> +
> +		fsleep(100);
> +	}
> +
> +	ret = ad5413_calib_mem_refresh(st);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ad5413_spi_reg_read(st, AD5413_REG_DIGITAL_DIAG_RESULTS, &regval);
> +	if (ret)
> +		return ret;
> +
> +	ret = ad5413_spi_reg_write(st, AD5413_REG_DIGITAL_DIAG_RESULTS, regval);

Whilst it doesn't matter in practice, I'd rather this only wrote the write 1 to clear
bits rather than also writing the read only ones up the top. Acts as documentation
about what this is doing.

> +	if (ret < 0)
> +		return ret;
> +
> +	ret = ad5413_set_out_range(st, st->out_range->reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (st->slew_time) {
> +		ret = ad5413_slew_rate_config(st);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = ad5413_internal_buffers_en(st, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ad5413_spi_write_mask(st,
> +				    AD5413_REG_DAC_CONFIG,
> +				    AD5413_REG_DAC_CONFIG_OUT_EN_MSK,
> +				    FIELD_PREP(AD5413_REG_DAC_CONFIG_OUT_EN_MSK, 1));
> +}
}