[PATCH v3 00/10] iio: dac: ad5446: Refactor and add support for AD5542

[PATCH v3 00/10] iio: dac: ad5446: Refactor and add support for AD5542

[Nuno Sá via B4 Relay]

Alright, what was suposed to be a simple one liner patch ended up being
a full refactor (modernization) of the whole thing :). I think the
changes are anyways fairly simple so hopefully nothing was broken.

I'm also aware of the checkpatch failure in Patch 6 ("iio: dac: ad5446:
Separate I2C/SPI into different drivers") but I'm really not seeing the
added value of adding the kconfig help text to the core symbol. 

---
Changes in v3:
- Patch 1:
   * Changed commit message as suggested by Conor;
   * Dropped formatting on the binding description;
   * Used typical Analog title in MAINTAINERS entry.
- Patch 2-4, 6-9
   * New patches.
- Link to v2: https://lore.kernel.org/r/20251023-ad5446-bindings-v2-0-27fab9891e86@analog.com

---
Michael Hennerich (1):
      iio: dac: ad5446: Add AD5542 to the spi id table

Nuno Sá (9):
      dt-bindings: iio: dac: Document AD5446 and similar devices
      iio: dac: ad5446: Use DMA safe buffer for transfers
      iio: dac: ad5446: Don't ignore missing regulator
      iio: dac: ad5446: Move to single chip_info structures
      iio: dac: ad5456: Add missing DT compatibles
      iio: dac: ad5446: Separate I2C/SPI into different drivers
      iio: dac: ad5446: Make use of devm_mutex_init()
      iio: dac: ad5446: Make use of the cleanup helpers
      iio: dac: ad5446: Fix coding style issues

 .../devicetree/bindings/iio/dac/adi,ad5446.yaml    | 138 ++++++
 MAINTAINERS                                        |  12 +
 drivers/iio/dac/Kconfig                            |  31 +-
 drivers/iio/dac/Makefile                           |   2 +
 drivers/iio/dac/ad5446-i2c.c                       |  99 ++++
 drivers/iio/dac/ad5446-spi.c                       | 254 +++++++++++
 drivers/iio/dac/ad5446.c                           | 498 +++------------------
 drivers/iio/dac/ad5446.h                           |  76 ++++
 8 files changed, 663 insertions(+), 447 deletions(-)
---
base-commit: 4b17a60d1e1c2d9d2ccbd58642f6f4ac2fa364ba
change-id: 20251014-dev-add-ad5542-8c8934de80ee
--

Thanks!
- Nuno Sá

[PATCH v3 01/10] dt-bindings: iio: dac: Document AD5446 and similar devices

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Add device tree binding documentation for the Analog Devices AD5446
family of Digital-to-Analog Converters and derivative devices from
Texas Instruments. There's both SPI and I2C interfaces and feature
resolutions ranging from 8-bit to 16-bit.

The binding covers 29 derivatives devices including the AD5446 series,
AD5600 series, AD5620/5640/5660 variants with different voltage ranges,
and TI DAC081s101/DAC101s101/DAC121s101 devices.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5446.yaml    | 138 +++++++++++++++++++++
 MAINTAINERS                                        |   8 ++
 2 files changed, 146 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5446.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5446.yaml
new file mode 100644
index 000000000000..2669d2c4948b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5446.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5446.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5446 and similar DACs
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+  - Nuno Sá <nuno.sa@analog.com>
+
+description:
+  Digital to Analog Converter devices supporting both SPI and I2C interfaces.
+  These devices feature a range of resolutions from 8-bit to 16-bit.
+
+properties:
+  compatible:
+    oneOf:
+      - description: SPI DACs
+        enum:
+          - adi,ad5300
+          - adi,ad5310
+          - adi,ad5320
+          - adi,ad5444
+          - adi,ad5446
+          - adi,ad5450
+          - adi,ad5451
+          - adi,ad5452
+          - adi,ad5453
+          - adi,ad5512a
+          - adi,ad5541a
+          - adi,ad5542
+          - adi,ad5542a
+          - adi,ad5543
+          - adi,ad5553
+          - adi,ad5600
+          - adi,ad5601
+          - adi,ad5611
+          - adi,ad5621
+          - adi,ad5641
+          - adi,ad5620-2500
+          - adi,ad5620-1250
+          - adi,ad5640-2500
+          - adi,ad5640-1250
+          - adi,ad5660-2500
+          - adi,ad5660-1250
+          - adi,ad5662
+          - ti,dac081s101
+          - ti,dac101s101
+          - ti,dac121s101
+      - description: I2C DACs
+        enum:
+          - adi,ad5301
+          - adi,ad5311
+          - adi,ad5321
+          - adi,ad5602
+          - adi,ad5612
+          - adi,ad5622
+
+  reg:
+    maxItems: 1
+
+  vcc-supply:
+    description:
+      Reference voltage supply. If not supplied, devices with internal
+      voltage reference will use that.
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad5300
+              - adi,ad5310
+              - adi,ad5320
+              - adi,ad5444
+              - adi,ad5446
+              - adi,ad5450
+              - adi,ad5451
+              - adi,ad5452
+              - adi,ad5453
+              - adi,ad5512a
+              - adi,ad5541a
+              - adi,ad5542
+              - adi,ad5542a
+              - adi,ad5543
+              - adi,ad5553
+              - adi,ad5600
+              - adi,ad5601
+              - adi,ad5611
+              - adi,ad5621
+              - adi,ad5641
+              - adi,ad5620-2500
+              - adi,ad5620-1250
+              - adi,ad5640-2500
+              - adi,ad5640-1250
+              - adi,ad5660-2500
+              - adi,ad5660-1250
+              - adi,ad5662
+              - ti,dac081s101
+              - ti,dac101s101
+              - ti,dac121s101
+    then:
+      allOf:
+        - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@0 {
+            compatible = "adi,ad5446";
+            reg = <0>;
+            vcc-supply = <&dac_vref>;
+        };
+    };
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@42 {
+            compatible = "adi,ad5622";
+            reg = <0x42>;
+            vcc-supply = <&dac_vref>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 8082081ea742..9654f0c25423 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -440,6 +440,14 @@ W:	http://wiki.analog.com/AD5398
 W:	https://ez.analog.com/linux-software-drivers
 F:	drivers/regulator/ad5398.c
 
+AD5446	ANALOG DEVICES INC AD5446 DAC DRIVER
+M:	Michael Hennerich <michael.hennerich@analog.com>
+M:	Nuno Sá <nuno.sa@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,ad5446.yaml
+
 AD714X CAPACITANCE TOUCH SENSOR DRIVER (AD7142/3/7/8/7A)
 M:	Michael Hennerich <michael.hennerich@analog.com>
 S:	Supported

-- 
2.51.0

[PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Make sure to use DMA safe buffer. While for i2c we could be fine without
them, we need it for spi anyways.

As we now have DMA safe buffers, use i2c_master_send_dmasafe().

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index ad304b0fec08..de5e4e0e397e 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -47,6 +47,10 @@ struct ad5446_state {
 	unsigned			pwr_down_mode;
 	unsigned			pwr_down;
 	struct mutex			lock;
+	union {
+		__be16 d16;
+		u8 d24[3];
+	} __aligned(IIO_DMA_MINALIGN);
 };
 
 /**
@@ -265,19 +269,18 @@ static int ad5446_probe(struct device *dev, const char *name,
 static int ad5446_write(struct ad5446_state *st, unsigned val)
 {
 	struct spi_device *spi = to_spi_device(st->dev);
-	__be16 data = cpu_to_be16(val);
+	st->d16 = cpu_to_be16(val);
 
-	return spi_write(spi, &data, sizeof(data));
+	return spi_write(spi, &st->d16, sizeof(st->d16));
 }
 
 static int ad5660_write(struct ad5446_state *st, unsigned val)
 {
 	struct spi_device *spi = to_spi_device(st->dev);
-	uint8_t data[3];
 
-	put_unaligned_be24(val, &data[0]);
+	put_unaligned_be24(val, &st->d24[0]);
 
-	return spi_write(spi, data, sizeof(data));
+	return spi_write(spi, st->d24, sizeof(st->d24));
 }
 
 /*
@@ -489,13 +492,13 @@ static inline void ad5446_spi_unregister_driver(void) { }
 static int ad5622_write(struct ad5446_state *st, unsigned val)
 {
 	struct i2c_client *client = to_i2c_client(st->dev);
-	__be16 data = cpu_to_be16(val);
+	st->d16 = cpu_to_be16(val);
 	int ret;
 
-	ret = i2c_master_send(client, (char *)&data, sizeof(data));
+	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
 	if (ret < 0)
 		return ret;
-	if (ret != sizeof(data))
+	if (ret != sizeof(st->d16))
 		return -EIO;
 
 	return 0;

-- 
2.51.0

[PATCH v3 03/10] iio: dac: ad5446: Don't ignore missing regulator

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

If the chip does not have an internal reference, do not ignore a missing
regulator as we won't be able to actually provide a proper scale for the
DAC. Since it's now seen as an error, flip the if() logic so errors are
treated first (which is the typical pattern).

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index de5e4e0e397e..5f9b6f82a981 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -253,10 +253,11 @@ static int ad5446_probe(struct device *dev, const char *name,
 	if (ret < 0 && ret != -ENODEV)
 		return ret;
 	if (ret == -ENODEV) {
-		if (chip_info->int_vref_mv)
-			st->vref_mv = chip_info->int_vref_mv;
-		else
-			dev_warn(dev, "reference voltage unspecified\n");
+		if (!chip_info->int_vref_mv)
+			return dev_err_probe(dev, ret,
+					     "reference voltage unspecified\n");
+
+		st->vref_mv = chip_info->int_vref_mv;
 	} else {
 		st->vref_mv = ret / 1000;
 	}

-- 
2.51.0

[PATCH v3 04/10] iio: dac: ad5446: Move to single chip_info structures

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Do not use an array with an enum id kind of thing. Use the more
maintainable chip_info variable per chip.

Adapt the probe functions to use the proper helpers (for SPI and I2c).
Note that in a following patch we'll also add the chip_info variables to
the of_device_id tables. Hence already use the helpers that internally use
device_get_match_data().

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 352 +++++++++++++++++++++++------------------------
 1 file changed, 176 insertions(+), 176 deletions(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 5f9b6f82a981..54702ba43805 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -291,165 +291,161 @@ static int ad5660_write(struct ad5446_state *st, unsigned val)
  * (and a bit cryptic), however this style is used to make clear which
  * parts are supported here.
  */
-enum ad5446_supported_spi_device_ids {
-	ID_AD5300,
-	ID_AD5310,
-	ID_AD5320,
-	ID_AD5444,
-	ID_AD5446,
-	ID_AD5450,
-	ID_AD5451,
-	ID_AD5541A,
-	ID_AD5512A,
-	ID_AD5553,
-	ID_AD5600,
-	ID_AD5601,
-	ID_AD5611,
-	ID_AD5621,
-	ID_AD5641,
-	ID_AD5620_2500,
-	ID_AD5620_1250,
-	ID_AD5640_2500,
-	ID_AD5640_1250,
-	ID_AD5660_2500,
-	ID_AD5660_1250,
-	ID_AD5662,
+
+static const struct ad5446_chip_info ad5300_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+	.write = ad5446_write,
 };
 
-static const struct ad5446_chip_info ad5446_spi_chip_info[] = {
-	[ID_AD5300] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5310] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5320] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5444] = {
-		.channel = AD5446_CHANNEL(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5446] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5450] = {
-		.channel = AD5446_CHANNEL(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5451] = {
-		.channel = AD5446_CHANNEL(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5541A] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5512A] = {
-		.channel = AD5446_CHANNEL(12, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5553] = {
-		.channel = AD5446_CHANNEL(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5600] = {
-		.channel = AD5446_CHANNEL(16, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5601] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
-		.write = ad5446_write,
-	},
-	[ID_AD5611] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
-		.write = ad5446_write,
-	},
-	[ID_AD5621] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.write = ad5446_write,
-	},
-	[ID_AD5641] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.write = ad5446_write,
-	},
-	[ID_AD5620_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5620_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5446_write,
-	},
-	[ID_AD5640_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5446_write,
-	},
-	[ID_AD5660_2500] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 2500,
-		.write = ad5660_write,
-	},
-	[ID_AD5660_1250] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.int_vref_mv = 1250,
-		.write = ad5660_write,
-	},
-	[ID_AD5662] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-		.write = ad5660_write,
-	},
+static const struct ad5446_chip_info ad5310_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5320_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5444_chip_info = {
+	.channel = AD5446_CHANNEL(12, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5446_chip_info = {
+	.channel = AD5446_CHANNEL(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5450_chip_info = {
+	.channel = AD5446_CHANNEL(8, 16, 6),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5451_chip_info = {
+	.channel = AD5446_CHANNEL(10, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5541a_chip_info = {
+	.channel = AD5446_CHANNEL(16, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5512a_chip_info = {
+	.channel = AD5446_CHANNEL(12, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5553_chip_info = {
+	.channel = AD5446_CHANNEL(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5600_chip_info = {
+	.channel = AD5446_CHANNEL(16, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5601_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5611_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5621_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5641_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5620_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.int_vref_mv = 2500,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5620_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.int_vref_mv = 1250,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5640_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.int_vref_mv = 2500,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5640_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.int_vref_mv = 1250,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5660_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.int_vref_mv = 2500,
+	.write = ad5660_write,
+};
+
+static const struct ad5446_chip_info ad5660_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.int_vref_mv = 1250,
+	.write = ad5660_write,
+};
+
+static const struct ad5446_chip_info ad5662_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.write = ad5660_write,
 };
 
 static const struct spi_device_id ad5446_spi_ids[] = {
-	{"ad5300", ID_AD5300},
-	{"ad5310", ID_AD5310},
-	{"ad5320", ID_AD5320},
-	{"ad5444", ID_AD5444},
-	{"ad5446", ID_AD5446},
-	{"ad5450", ID_AD5450},
-	{"ad5451", ID_AD5451},
-	{"ad5452", ID_AD5444}, /* ad5452 is compatible to the ad5444 */
-	{"ad5453", ID_AD5446}, /* ad5453 is compatible to the ad5446 */
-	{"ad5512a", ID_AD5512A},
-	{"ad5541a", ID_AD5541A},
-	{"ad5542a", ID_AD5541A}, /* ad5541a and ad5542a are compatible */
-	{"ad5543", ID_AD5541A}, /* ad5541a and ad5543 are compatible */
-	{"ad5553", ID_AD5553},
-	{"ad5600", ID_AD5600},
-	{"ad5601", ID_AD5601},
-	{"ad5611", ID_AD5611},
-	{"ad5621", ID_AD5621},
-	{"ad5641", ID_AD5641},
-	{"ad5620-2500", ID_AD5620_2500}, /* AD5620/40/60: */
-	{"ad5620-1250", ID_AD5620_1250}, /* part numbers may look differently */
-	{"ad5640-2500", ID_AD5640_2500},
-	{"ad5640-1250", ID_AD5640_1250},
-	{"ad5660-2500", ID_AD5660_2500},
-	{"ad5660-1250", ID_AD5660_1250},
-	{"ad5662", ID_AD5662},
-	{"dac081s101", ID_AD5300}, /* compatible Texas Instruments chips */
-	{"dac101s101", ID_AD5310},
-	{"dac121s101", ID_AD5320},
-	{"dac7512", ID_AD5320},
+	{"ad5300", (kernel_ulong_t)&ad5300_chip_info},
+	{"ad5310", (kernel_ulong_t)&ad5310_chip_info},
+	{"ad5320", (kernel_ulong_t)&ad5320_chip_info},
+	{"ad5444", (kernel_ulong_t)&ad5444_chip_info},
+	{"ad5446", (kernel_ulong_t)&ad5446_chip_info},
+	{"ad5450", (kernel_ulong_t)&ad5450_chip_info},
+	{"ad5451", (kernel_ulong_t)&ad5451_chip_info},
+	{"ad5452", (kernel_ulong_t)&ad5444_chip_info}, /* ad5452 is compatible to the ad5444 */
+	{"ad5453", (kernel_ulong_t)&ad5446_chip_info}, /* ad5453 is compatible to the ad5446 */
+	{"ad5512a", (kernel_ulong_t)&ad5512a_chip_info},
+	{"ad5541a", (kernel_ulong_t)&ad5541a_chip_info},
+	{"ad5542a", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5542a are compatible */
+	{"ad5543", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5543 are compatible */
+	{"ad5553", (kernel_ulong_t)&ad5553_chip_info},
+	{"ad5600", (kernel_ulong_t)&ad5600_chip_info},
+	{"ad5601", (kernel_ulong_t)&ad5601_chip_info},
+	{"ad5611", (kernel_ulong_t)&ad5611_chip_info},
+	{"ad5621", (kernel_ulong_t)&ad5621_chip_info},
+	{"ad5641", (kernel_ulong_t)&ad5641_chip_info},
+	{"ad5620-2500", (kernel_ulong_t)&ad5620_2500_chip_info}, /* AD5620/40/60: */
+	/* part numbers may look differently */
+	{"ad5620-1250", (kernel_ulong_t)&ad5620_1250_chip_info},
+	{"ad5640-2500", (kernel_ulong_t)&ad5640_2500_chip_info},
+	{"ad5640-1250", (kernel_ulong_t)&ad5640_1250_chip_info},
+	{"ad5660-2500", (kernel_ulong_t)&ad5660_2500_chip_info},
+	{"ad5660-1250", (kernel_ulong_t)&ad5660_1250_chip_info},
+	{"ad5662", (kernel_ulong_t)&ad5662_chip_info},
+	{"dac081s101", (kernel_ulong_t)&ad5300_chip_info}, /* compatible Texas Instruments chips */
+	{"dac101s101", (kernel_ulong_t)&ad5310_chip_info},
+	{"dac121s101", (kernel_ulong_t)&ad5320_chip_info},
+	{"dac7512", (kernel_ulong_t)&ad5320_chip_info},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
 
 static const struct of_device_id ad5446_of_ids[] = {
-	{ .compatible = "ti,dac7512" },
+	{ .compatible = "ti,dac7512", .data = &ad5320_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ad5446_of_ids);
@@ -457,9 +453,13 @@ MODULE_DEVICE_TABLE(of, ad5446_of_ids);
 static int ad5446_spi_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
+	const struct ad5446_chip_info *chip_info;
 
-	return ad5446_probe(&spi->dev, id->name,
-		&ad5446_spi_chip_info[id->driver_data]);
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
+
+	return ad5446_probe(&spi->dev, id->name, chip_info);
 }
 
 static struct spi_driver ad5446_spi_driver = {
@@ -512,41 +512,41 @@ static int ad5622_write(struct ad5446_state *st, unsigned val)
  * (and a bit cryptic), however this style is used to make clear which
  * parts are supported here.
  */
-enum ad5446_supported_i2c_device_ids {
-	ID_AD5602,
-	ID_AD5612,
-	ID_AD5622,
+
+static const struct ad5446_chip_info ad5602_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+	.write = ad5622_write,
 };
 
-static const struct ad5446_chip_info ad5446_i2c_chip_info[] = {
-	[ID_AD5602] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
-		.write = ad5622_write,
-	},
-	[ID_AD5612] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
-		.write = ad5622_write,
-	},
-	[ID_AD5622] = {
-		.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
-		.write = ad5622_write,
-	},
+static const struct ad5446_chip_info ad5612_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+	.write = ad5622_write,
+};
+
+static const struct ad5446_chip_info ad5622_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+	.write = ad5622_write,
 };
 
 static int ad5446_i2c_probe(struct i2c_client *i2c)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
-	return ad5446_probe(&i2c->dev, id->name,
-		&ad5446_i2c_chip_info[id->driver_data]);
+	const struct ad5446_chip_info *chip_info;
+
+	chip_info = i2c_get_match_data(i2c);
+	if (!chip_info)
+		return -ENODEV;
+
+	return ad5446_probe(&i2c->dev, id->name, chip_info);
 }
 
 static const struct i2c_device_id ad5446_i2c_ids[] = {
-	{"ad5301", ID_AD5602},
-	{"ad5311", ID_AD5612},
-	{"ad5321", ID_AD5622},
-	{"ad5602", ID_AD5602},
-	{"ad5612", ID_AD5612},
-	{"ad5622", ID_AD5622},
+	{"ad5301", (kernel_ulong_t)&ad5602_chip_info},
+	{"ad5311", (kernel_ulong_t)&ad5612_chip_info},
+	{"ad5321", (kernel_ulong_t)&ad5622_chip_info},
+	{"ad5602", (kernel_ulong_t)&ad5602_chip_info},
+	{"ad5612", (kernel_ulong_t)&ad5612_chip_info},
+	{"ad5622", (kernel_ulong_t)&ad5622_chip_info},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);

-- 
2.51.0

[PATCH v3 05/10] iio: dac: ad5456: Add missing DT compatibles

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Add missing of_device_id compatibles for the i2c and spi drivers.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 54702ba43805..0c318f23b6a4 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -445,6 +445,35 @@ static const struct spi_device_id ad5446_spi_ids[] = {
 MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
 
 static const struct of_device_id ad5446_of_ids[] = {
+	{ .compatible = "adi,ad5300", .data = &ad5300_chip_info },
+	{ .compatible = "adi,ad5310", .data = &ad5310_chip_info },
+	{ .compatible = "adi,ad5320", .data = &ad5320_chip_info },
+	{ .compatible = "adi,ad5444", .data = &ad5444_chip_info },
+	{ .compatible = "adi,ad5446", .data = &ad5446_chip_info },
+	{ .compatible = "adi,ad5450", .data = &ad5450_chip_info },
+	{ .compatible = "adi,ad5451", .data = &ad5451_chip_info },
+	{ .compatible = "adi,ad5452", .data = &ad5444_chip_info },
+	{ .compatible = "adi,ad5453", .data = &ad5446_chip_info },
+	{ .compatible = "adi,ad5512a", .data = &ad5512a_chip_info },
+	{ .compatible = "adi,ad5541a", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5542a", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5543", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5553", .data = &ad5553_chip_info },
+	{ .compatible = "adi,ad5600", .data = &ad5600_chip_info },
+	{ .compatible = "adi,ad5601", .data = &ad5601_chip_info },
+	{ .compatible = "adi,ad5611", .data = &ad5611_chip_info },
+	{ .compatible = "adi,ad5621", .data = &ad5621_chip_info },
+	{ .compatible = "adi,ad5641", .data = &ad5641_chip_info },
+	{ .compatible = "adi,ad5620-2500", .data = &ad5620_2500_chip_info },
+	{ .compatible = "adi,ad5620-1250", .data = &ad5620_1250_chip_info },
+	{ .compatible = "adi,ad5640-2500", .data = &ad5640_2500_chip_info },
+	{ .compatible = "adi,ad5640-1250", .data = &ad5640_1250_chip_info },
+	{ .compatible = "adi,ad5660-2500", .data = &ad5660_2500_chip_info },
+	{ .compatible = "adi,ad5660-1250", .data = &ad5660_1250_chip_info },
+	{ .compatible = "adi,ad5662", .data = &ad5662_chip_info },
+	{ .compatible = "ti,dac081s101", .data = &ad5300_chip_info },
+	{ .compatible = "ti,dac101s101", .data = &ad5310_chip_info },
+	{ .compatible = "ti,dac121s101", .data = &ad5320_chip_info },
 	{ .compatible = "ti,dac7512", .data = &ad5320_chip_info },
 	{ }
 };
@@ -551,9 +580,21 @@ static const struct i2c_device_id ad5446_i2c_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
 
+static const struct of_device_id ad5446_i2c_of_ids[] = {
+	{ .compatible = "adi,ad5301", .data = &ad5602_chip_info },
+	{ .compatible = "adi,ad5311", .data = &ad5612_chip_info },
+	{ .compatible = "adi,ad5321", .data = &ad5622_chip_info },
+	{ .compatible = "adi,ad5602", .data = &ad5602_chip_info },
+	{ .compatible = "adi,ad5612", .data = &ad5612_chip_info },
+	{ .compatible = "adi,ad5622", .data = &ad5622_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(OF, ad5446_i2c_of_ids);
+
 static struct i2c_driver ad5446_i2c_driver = {
 	.driver = {
 		   .name = "ad5446",
+		   .of_match_table = ad5446_i2c_of_ids,
 	},
 	.probe = ad5446_i2c_probe,
 	.id_table = ad5446_i2c_ids,

-- 
2.51.0

[PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Properly separate the I2C and SPI drivers into two different drivers
living in their own source file (as usual). So that no need for the
hacky ifdefery.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 MAINTAINERS                  |   4 +
 drivers/iio/dac/Kconfig      |  31 ++-
 drivers/iio/dac/Makefile     |   2 +
 drivers/iio/dac/ad5446-i2c.c |  99 ++++++++++
 drivers/iio/dac/ad5446-spi.c | 252 ++++++++++++++++++++++++
 drivers/iio/dac/ad5446.c     | 447 +------------------------------------------
 drivers/iio/dac/ad5446.h     |  76 ++++++++
 7 files changed, 465 insertions(+), 446 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9654f0c25423..c845ad7307fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -447,6 +447,10 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad5446.yaml
+F:	drivers/iio/dac/ad5446-i2c.c
+F:	drivers/iio/dac/ad5446-spi.c
+F:	drivers/iio/dac/ad5446.c
+F:	drivers/iio/dac/ad5446.h
 
 AD714X CAPACITANCE TOUCH SENSOR DRIVER (AD7142/3/7/8/7A)
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index e0996dc014a3..7cd3caec1262 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -97,17 +97,32 @@ config AD5421
 	  ad5421.
 
 config AD5446
-	tristate "Analog Devices AD5446 and similar single channel DACs driver"
-	depends on (SPI_MASTER && I2C!=m) || I2C
+	tristate
+
+config AD5446_SPI
+	tristate "Analog Devices AD5446 and similar single channel DACs driver (SPI)"
+	depends on SPI
+	select AD5446
 	help
-	  Say yes here to build support for Analog Devices AD5300, AD5301, AD5310,
-	  AD5311, AD5320, AD5321, AD5444, AD5446, AD5450, AD5451, AD5452, AD5453,
-	  AD5512A, AD5541A, AD5542A, AD5543, AD5553, AD5600, AD5601, AD5602, AD5611,
-	  AD5612, AD5620, AD5621, AD5622, AD5640, AD5641, AD5660, AD5662 DACs
-	  as well as Texas Instruments DAC081S101, DAC101S101, DAC121S101.
+	  Say yes here to build support for Analog Devices AD5300, AD5310,
+	  AD5320, AD5444, AD5446, AD5450, AD5451, AD5452, AD5453, AD5512A,
+	  AD5541A, AD5542A, AD5543, AD5553, AD5600, AD5601, AD5611, AD5620,
+	  AD5621, AD5640, AD5641, AD5660, AD5662 DACs as well as
+	  Texas Instruments DAC081S101, DAC101S101, DAC121S101.
 
 	  To compile this driver as a module, choose M here: the
-	  module will be called ad5446.
+	  module will be called ad5446-spi.
+
+config AD5446_I2C
+	tristate "Analog Devices AD5446 and similar single channel DACs driver (I2C)"
+	depends on I2C
+	select AD5446
+	help
+	  Say yes here to build support for Analog Devices AD5301, AD5311, AD5321,
+	  AD5602, AD5612, AD5622 DACs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5446-i2c.
 
 config AD5449
 	tristate "Analog Devices AD5449 and similar DACs driver"
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 3684cd52b7fa..e6ac4c67e337 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -15,6 +15,8 @@ obj-$(CONFIG_AD5624R_SPI) += ad5624r_spi.o
 obj-$(CONFIG_AD5064) += ad5064.o
 obj-$(CONFIG_AD5504) += ad5504.o
 obj-$(CONFIG_AD5446) += ad5446.o
+obj-$(CONFIG_AD5446_SPI) += ad5446-spi.o
+obj-$(CONFIG_AD5446_I2C) += ad5446-i2c.o
 obj-$(CONFIG_AD5449) += ad5449.o
 obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
 obj-$(CONFIG_AD5592R) += ad5592r.o
diff --git a/drivers/iio/dac/ad5446-i2c.c b/drivers/iio/dac/ad5446-i2c.c
new file mode 100644
index 000000000000..9d1054c3dd8e
--- /dev/null
+++ b/drivers/iio/dac/ad5446-i2c.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD5446 SPI I2C driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/i2c.h>
+
+#include "ad5446.h"
+
+static int ad5622_write(struct ad5446_state *st, unsigned int val)
+{
+	struct i2c_client *client = to_i2c_client(st->dev);
+	int ret;
+
+	st->d16 = cpu_to_be16(val);
+
+	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(st->d16))
+		return -EIO;
+
+	return 0;
+}
+
+static int ad5446_i2c_probe(struct i2c_client *i2c)
+{
+	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
+	const struct ad5446_chip_info *chip_info;
+
+	chip_info = i2c_get_match_data(i2c);
+	if (!chip_info)
+		return -ENODEV;
+
+	return ad5446_probe(&i2c->dev, id->name, chip_info);
+}
+
+/*
+ * ad5446_supported_i2c_device_ids:
+ * The AD5620/40/60 parts are available in different fixed internal reference
+ * voltage options. The actual part numbers may look differently
+ * (and a bit cryptic), however this style is used to make clear which
+ * parts are supported here.
+ */
+
+static const struct ad5446_chip_info ad5602_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+	.write = ad5622_write,
+};
+
+static const struct ad5446_chip_info ad5612_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+	.write = ad5622_write,
+};
+
+static const struct ad5446_chip_info ad5622_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+	.write = ad5622_write,
+};
+
+static const struct i2c_device_id ad5446_i2c_ids[] = {
+	{"ad5301", (kernel_ulong_t)&ad5602_chip_info},
+	{"ad5311", (kernel_ulong_t)&ad5612_chip_info},
+	{"ad5321", (kernel_ulong_t)&ad5622_chip_info},
+	{"ad5602", (kernel_ulong_t)&ad5602_chip_info},
+	{"ad5612", (kernel_ulong_t)&ad5612_chip_info},
+	{"ad5622", (kernel_ulong_t)&ad5622_chip_info},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
+
+static const struct of_device_id ad5446_i2c_of_ids[] = {
+	{ .compatible = "adi,ad5301", .data = &ad5602_chip_info },
+	{ .compatible = "adi,ad5311", .data = &ad5612_chip_info },
+	{ .compatible = "adi,ad5321", .data = &ad5622_chip_info },
+	{ .compatible = "adi,ad5602", .data = &ad5602_chip_info },
+	{ .compatible = "adi,ad5612", .data = &ad5612_chip_info },
+	{ .compatible = "adi,ad5622", .data = &ad5622_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(OF, ad5446_i2c_of_ids);
+
+static struct i2c_driver ad5446_i2c_driver = {
+	.driver = {
+		.name	= "ad5446",
+		.of_match_table = ad5446_i2c_of_ids,
+	},
+	.probe = ad5446_i2c_probe,
+	.id_table = ad5446_i2c_ids,
+};
+module_i2c_driver(ad5446_i2c_driver);
+
+MODULE_AUTHOR("Nuno Sá <nuno.sa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 I2C DACs");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_AD5446");
diff --git a/drivers/iio/dac/ad5446-spi.c b/drivers/iio/dac/ad5446-spi.c
new file mode 100644
index 000000000000..7419b17a455f
--- /dev/null
+++ b/drivers/iio/dac/ad5446-spi.c
@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD5446 SPI DAC driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.h>
+
+#include "ad5446.h"
+
+static int ad5446_write(struct ad5446_state *st, unsigned int val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+
+	st->d16 = cpu_to_be16(val);
+
+	return spi_write(spi, &st->d16, sizeof(st->d16));
+}
+
+static int ad5660_write(struct ad5446_state *st, unsigned int val)
+{
+	struct spi_device *spi = to_spi_device(st->dev);
+
+	put_unaligned_be24(val, &st->d24[0]);
+
+	return spi_write(spi, st->d24, sizeof(st->d24));
+}
+
+static int ad5446_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	const struct ad5446_chip_info *chip_info;
+
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
+
+	return ad5446_probe(&spi->dev, id->name, chip_info);
+}
+
+/*
+ * ad5446_supported_spi_device_ids:
+ * The AD5620/40/60 parts are available in different fixed internal reference
+ * voltage options. The actual part numbers may look differently
+ * (and a bit cryptic), however this style is used to make clear which
+ * parts are supported here.
+ */
+
+static const struct ad5446_chip_info ad5300_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5310_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5320_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5444_chip_info = {
+	.channel = AD5446_CHANNEL(12, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5446_chip_info = {
+	.channel = AD5446_CHANNEL(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5450_chip_info = {
+	.channel = AD5446_CHANNEL(8, 16, 6),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5451_chip_info = {
+	.channel = AD5446_CHANNEL(10, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5541a_chip_info = {
+	.channel = AD5446_CHANNEL(16, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5512a_chip_info = {
+	.channel = AD5446_CHANNEL(12, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5553_chip_info = {
+	.channel = AD5446_CHANNEL(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5600_chip_info = {
+	.channel = AD5446_CHANNEL(16, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5601_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5611_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5621_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5641_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5620_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.int_vref_mv = 2500,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5620_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
+	.int_vref_mv = 1250,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5640_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.int_vref_mv = 2500,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5640_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
+	.int_vref_mv = 1250,
+	.write = ad5446_write,
+};
+
+static const struct ad5446_chip_info ad5660_2500_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.int_vref_mv = 2500,
+	.write = ad5660_write,
+};
+
+static const struct ad5446_chip_info ad5660_1250_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.int_vref_mv = 1250,
+	.write = ad5660_write,
+};
+
+static const struct ad5446_chip_info ad5662_chip_info = {
+	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
+	.write = ad5660_write,
+};
+
+static const struct spi_device_id ad5446_spi_ids[] = {
+	{"ad5300", (kernel_ulong_t)&ad5300_chip_info},
+	{"ad5310", (kernel_ulong_t)&ad5310_chip_info},
+	{"ad5320", (kernel_ulong_t)&ad5320_chip_info},
+	{"ad5444", (kernel_ulong_t)&ad5444_chip_info},
+	{"ad5446", (kernel_ulong_t)&ad5446_chip_info},
+	{"ad5450", (kernel_ulong_t)&ad5450_chip_info},
+	{"ad5451", (kernel_ulong_t)&ad5451_chip_info},
+	{"ad5452", (kernel_ulong_t)&ad5444_chip_info}, /* ad5452 is compatible to the ad5444 */
+	{"ad5453", (kernel_ulong_t)&ad5446_chip_info}, /* ad5453 is compatible to the ad5446 */
+	{"ad5512a", (kernel_ulong_t)&ad5512a_chip_info},
+	{"ad5541a", (kernel_ulong_t)&ad5541a_chip_info},
+	{"ad5542a", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5542a are compatible */
+	{"ad5543", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5543 are compatible */
+	{"ad5553", (kernel_ulong_t)&ad5553_chip_info},
+	{"ad5600", (kernel_ulong_t)&ad5600_chip_info},
+	{"ad5601", (kernel_ulong_t)&ad5601_chip_info},
+	{"ad5611", (kernel_ulong_t)&ad5611_chip_info},
+	{"ad5621", (kernel_ulong_t)&ad5621_chip_info},
+	{"ad5641", (kernel_ulong_t)&ad5641_chip_info},
+	{"ad5620-2500", (kernel_ulong_t)&ad5620_2500_chip_info}, /* AD5620/40/60: */
+	/* part numbers may look differently */
+	{"ad5620-1250", (kernel_ulong_t)&ad5620_1250_chip_info},
+	{"ad5640-2500", (kernel_ulong_t)&ad5640_2500_chip_info},
+	{"ad5640-1250", (kernel_ulong_t)&ad5640_1250_chip_info},
+	{"ad5660-2500", (kernel_ulong_t)&ad5660_2500_chip_info},
+	{"ad5660-1250", (kernel_ulong_t)&ad5660_1250_chip_info},
+	{"ad5662", (kernel_ulong_t)&ad5662_chip_info},
+	{"dac081s101", (kernel_ulong_t)&ad5300_chip_info}, /* compatible Texas Instruments chips */
+	{"dac101s101", (kernel_ulong_t)&ad5310_chip_info},
+	{"dac121s101", (kernel_ulong_t)&ad5320_chip_info},
+	{"dac7512", (kernel_ulong_t)&ad5320_chip_info},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
+
+static const struct of_device_id ad5446_of_ids[] = {
+	{ .compatible = "adi,ad5300", .data = &ad5300_chip_info },
+	{ .compatible = "adi,ad5310", .data = &ad5310_chip_info },
+	{ .compatible = "adi,ad5320", .data = &ad5320_chip_info },
+	{ .compatible = "adi,ad5444", .data = &ad5444_chip_info },
+	{ .compatible = "adi,ad5446", .data = &ad5446_chip_info },
+	{ .compatible = "adi,ad5450", .data = &ad5450_chip_info },
+	{ .compatible = "adi,ad5451", .data = &ad5451_chip_info },
+	{ .compatible = "adi,ad5452", .data = &ad5444_chip_info },
+	{ .compatible = "adi,ad5453", .data = &ad5446_chip_info },
+	{ .compatible = "adi,ad5512a", .data = &ad5512a_chip_info },
+	{ .compatible = "adi,ad5541a", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5542a", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5543", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5553", .data = &ad5553_chip_info },
+	{ .compatible = "adi,ad5600", .data = &ad5600_chip_info },
+	{ .compatible = "adi,ad5601", .data = &ad5601_chip_info },
+	{ .compatible = "adi,ad5611", .data = &ad5611_chip_info },
+	{ .compatible = "adi,ad5621", .data = &ad5621_chip_info },
+	{ .compatible = "adi,ad5641", .data = &ad5641_chip_info },
+	{ .compatible = "adi,ad5620-2500", .data = &ad5620_2500_chip_info },
+	{ .compatible = "adi,ad5620-1250", .data = &ad5620_1250_chip_info },
+	{ .compatible = "adi,ad5640-2500", .data = &ad5640_2500_chip_info },
+	{ .compatible = "adi,ad5640-1250", .data = &ad5640_1250_chip_info },
+	{ .compatible = "adi,ad5660-2500", .data = &ad5660_2500_chip_info },
+	{ .compatible = "adi,ad5660-1250", .data = &ad5660_1250_chip_info },
+	{ .compatible = "adi,ad5662", .data = &ad5662_chip_info },
+	{ .compatible = "ti,dac081s101", .data = &ad5300_chip_info },
+	{ .compatible = "ti,dac101s101", .data = &ad5310_chip_info },
+	{ .compatible = "ti,dac121s101", .data = &ad5320_chip_info },
+	{ .compatible = "ti,dac7512", .data = &ad5320_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5446_of_ids);
+
+static struct spi_driver ad5446_spi_driver = {
+	.driver = {
+		.name	= "ad5446",
+		.of_match_table = ad5446_of_ids,
+	},
+	.probe = ad5446_spi_probe,
+	.id_table = ad5446_spi_ids,
+};
+module_spi_driver(ad5446_spi_driver);
+
+MODULE_AUTHOR("Nuno Sá <nuno.sa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 SPI DACs");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_AD5446");
diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 0c318f23b6a4..790e24770062 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * AD5446 SPI DAC driver
+ * AD5446 CORE DAC driver
  *
  * Copyright 2010 Analog Devices Inc.
  */
 
+#include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/workqueue.h>
 #include <linux/device.h>
@@ -12,60 +13,19 @@
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 #include <linux/list.h>
-#include <linux/spi/spi.h>
-#include <linux/i2c.h>
 #include <linux/regulator/consumer.h>
 #include <linux/err.h>
 #include <linux/module.h>
-#include <linux/mod_devicetable.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#include <linux/unaligned.h>
+#include "ad5446.h"
 
 #define MODE_PWRDWN_1k		0x1
 #define MODE_PWRDWN_100k	0x2
 #define MODE_PWRDWN_TRISTATE	0x3
 
-/**
- * struct ad5446_state - driver instance specific data
- * @dev:		this device
- * @chip_info:		chip model specific constants, available modes etc
- * @vref_mv:		actual reference voltage used
- * @cached_val:		store/retrieve values during power down
- * @pwr_down_mode:	power down mode (1k, 100k or tristate)
- * @pwr_down:		true if the device is in power down
- * @lock:		lock to protect the data buffer during write ops
- */
-
-struct ad5446_state {
-	struct device		*dev;
-	const struct ad5446_chip_info	*chip_info;
-	unsigned short			vref_mv;
-	unsigned			cached_val;
-	unsigned			pwr_down_mode;
-	unsigned			pwr_down;
-	struct mutex			lock;
-	union {
-		__be16 d16;
-		u8 d24[3];
-	} __aligned(IIO_DMA_MINALIGN);
-};
-
-/**
- * struct ad5446_chip_info - chip specific information
- * @channel:		channel spec for the DAC
- * @int_vref_mv:	AD5620/40/60: the internal reference voltage
- * @write:		chip specific helper function to write to the register
- */
-
-struct ad5446_chip_info {
-	struct iio_chan_spec	channel;
-	u16			int_vref_mv;
-	int			(*write)(struct ad5446_state *st, unsigned val);
-};
-
 static const char * const ad5446_powerdown_modes[] = {
 	"1kohm_to_gnd", "100kohm_to_gnd", "three_state"
 };
@@ -136,7 +96,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	return ret ? ret : len;
 }
 
-static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
+const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
 	{
 		.name = "powerdown",
 		.read = ad5446_read_dac_powerdown,
@@ -147,28 +107,7 @@ static const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
 	IIO_ENUM_AVAILABLE("powerdown_mode", IIO_SHARED_BY_TYPE, &ad5446_powerdown_mode_enum),
 	{ }
 };
-
-#define _AD5446_CHANNEL(bits, storage, _shift, ext) { \
-	.type = IIO_VOLTAGE, \
-	.indexed = 1, \
-	.output = 1, \
-	.channel = 0, \
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-	.scan_type = { \
-		.sign = 'u', \
-		.realbits = (bits), \
-		.storagebits = (storage), \
-		.shift = (_shift), \
-		}, \
-	.ext_info = (ext), \
-}
-
-#define AD5446_CHANNEL(bits, storage, shift) \
-	_AD5446_CHANNEL(bits, storage, shift, NULL)
-
-#define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
-	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
+EXPORT_SYMBOL_NS_GPL(ad5446_ext_info_powerdown, "IIO_AD5446");
 
 static int ad5446_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
@@ -223,8 +162,8 @@ static const struct iio_info ad5446_info = {
 	.write_raw = ad5446_write_raw,
 };
 
-static int ad5446_probe(struct device *dev, const char *name,
-			const struct ad5446_chip_info *chip_info)
+int ad5446_probe(struct device *dev, const char *name,
+		 const struct ad5446_chip_info *chip_info)
 {
 	struct ad5446_state *st;
 	struct iio_dev *indio_dev;
@@ -264,25 +203,7 @@ static int ad5446_probe(struct device *dev, const char *name,
 
 	return devm_iio_device_register(dev, indio_dev);
 }
-
-#if IS_ENABLED(CONFIG_SPI_MASTER)
-
-static int ad5446_write(struct ad5446_state *st, unsigned val)
-{
-	struct spi_device *spi = to_spi_device(st->dev);
-	st->d16 = cpu_to_be16(val);
-
-	return spi_write(spi, &st->d16, sizeof(st->d16));
-}
-
-static int ad5660_write(struct ad5446_state *st, unsigned val)
-{
-	struct spi_device *spi = to_spi_device(st->dev);
-
-	put_unaligned_be24(val, &st->d24[0]);
-
-	return spi_write(spi, st->d24, sizeof(st->d24));
-}
+EXPORT_SYMBOL_NS_GPL(ad5446_probe, "IIO_AD5446");
 
 /*
  * ad5446_supported_spi_device_ids:
@@ -292,356 +213,6 @@ static int ad5660_write(struct ad5446_state *st, unsigned val)
  * parts are supported here.
  */
 
-static const struct ad5446_chip_info ad5300_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5310_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5320_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5444_chip_info = {
-	.channel = AD5446_CHANNEL(12, 16, 2),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5446_chip_info = {
-	.channel = AD5446_CHANNEL(14, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5450_chip_info = {
-	.channel = AD5446_CHANNEL(8, 16, 6),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5451_chip_info = {
-	.channel = AD5446_CHANNEL(10, 16, 4),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5541a_chip_info = {
-	.channel = AD5446_CHANNEL(16, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5512a_chip_info = {
-	.channel = AD5446_CHANNEL(12, 16, 4),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5553_chip_info = {
-	.channel = AD5446_CHANNEL(14, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5600_chip_info = {
-	.channel = AD5446_CHANNEL(16, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5601_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5611_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5621_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5641_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5620_2500_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-	.int_vref_mv = 2500,
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5620_1250_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
-	.int_vref_mv = 1250,
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5640_2500_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-	.int_vref_mv = 2500,
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5640_1250_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
-	.int_vref_mv = 1250,
-	.write = ad5446_write,
-};
-
-static const struct ad5446_chip_info ad5660_2500_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-	.int_vref_mv = 2500,
-	.write = ad5660_write,
-};
-
-static const struct ad5446_chip_info ad5660_1250_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-	.int_vref_mv = 1250,
-	.write = ad5660_write,
-};
-
-static const struct ad5446_chip_info ad5662_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
-	.write = ad5660_write,
-};
-
-static const struct spi_device_id ad5446_spi_ids[] = {
-	{"ad5300", (kernel_ulong_t)&ad5300_chip_info},
-	{"ad5310", (kernel_ulong_t)&ad5310_chip_info},
-	{"ad5320", (kernel_ulong_t)&ad5320_chip_info},
-	{"ad5444", (kernel_ulong_t)&ad5444_chip_info},
-	{"ad5446", (kernel_ulong_t)&ad5446_chip_info},
-	{"ad5450", (kernel_ulong_t)&ad5450_chip_info},
-	{"ad5451", (kernel_ulong_t)&ad5451_chip_info},
-	{"ad5452", (kernel_ulong_t)&ad5444_chip_info}, /* ad5452 is compatible to the ad5444 */
-	{"ad5453", (kernel_ulong_t)&ad5446_chip_info}, /* ad5453 is compatible to the ad5446 */
-	{"ad5512a", (kernel_ulong_t)&ad5512a_chip_info},
-	{"ad5541a", (kernel_ulong_t)&ad5541a_chip_info},
-	{"ad5542a", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5542a are compatible */
-	{"ad5543", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5543 are compatible */
-	{"ad5553", (kernel_ulong_t)&ad5553_chip_info},
-	{"ad5600", (kernel_ulong_t)&ad5600_chip_info},
-	{"ad5601", (kernel_ulong_t)&ad5601_chip_info},
-	{"ad5611", (kernel_ulong_t)&ad5611_chip_info},
-	{"ad5621", (kernel_ulong_t)&ad5621_chip_info},
-	{"ad5641", (kernel_ulong_t)&ad5641_chip_info},
-	{"ad5620-2500", (kernel_ulong_t)&ad5620_2500_chip_info}, /* AD5620/40/60: */
-	/* part numbers may look differently */
-	{"ad5620-1250", (kernel_ulong_t)&ad5620_1250_chip_info},
-	{"ad5640-2500", (kernel_ulong_t)&ad5640_2500_chip_info},
-	{"ad5640-1250", (kernel_ulong_t)&ad5640_1250_chip_info},
-	{"ad5660-2500", (kernel_ulong_t)&ad5660_2500_chip_info},
-	{"ad5660-1250", (kernel_ulong_t)&ad5660_1250_chip_info},
-	{"ad5662", (kernel_ulong_t)&ad5662_chip_info},
-	{"dac081s101", (kernel_ulong_t)&ad5300_chip_info}, /* compatible Texas Instruments chips */
-	{"dac101s101", (kernel_ulong_t)&ad5310_chip_info},
-	{"dac121s101", (kernel_ulong_t)&ad5320_chip_info},
-	{"dac7512", (kernel_ulong_t)&ad5320_chip_info},
-	{ }
-};
-MODULE_DEVICE_TABLE(spi, ad5446_spi_ids);
-
-static const struct of_device_id ad5446_of_ids[] = {
-	{ .compatible = "adi,ad5300", .data = &ad5300_chip_info },
-	{ .compatible = "adi,ad5310", .data = &ad5310_chip_info },
-	{ .compatible = "adi,ad5320", .data = &ad5320_chip_info },
-	{ .compatible = "adi,ad5444", .data = &ad5444_chip_info },
-	{ .compatible = "adi,ad5446", .data = &ad5446_chip_info },
-	{ .compatible = "adi,ad5450", .data = &ad5450_chip_info },
-	{ .compatible = "adi,ad5451", .data = &ad5451_chip_info },
-	{ .compatible = "adi,ad5452", .data = &ad5444_chip_info },
-	{ .compatible = "adi,ad5453", .data = &ad5446_chip_info },
-	{ .compatible = "adi,ad5512a", .data = &ad5512a_chip_info },
-	{ .compatible = "adi,ad5541a", .data = &ad5541a_chip_info },
-	{ .compatible = "adi,ad5542a", .data = &ad5541a_chip_info },
-	{ .compatible = "adi,ad5543", .data = &ad5541a_chip_info },
-	{ .compatible = "adi,ad5553", .data = &ad5553_chip_info },
-	{ .compatible = "adi,ad5600", .data = &ad5600_chip_info },
-	{ .compatible = "adi,ad5601", .data = &ad5601_chip_info },
-	{ .compatible = "adi,ad5611", .data = &ad5611_chip_info },
-	{ .compatible = "adi,ad5621", .data = &ad5621_chip_info },
-	{ .compatible = "adi,ad5641", .data = &ad5641_chip_info },
-	{ .compatible = "adi,ad5620-2500", .data = &ad5620_2500_chip_info },
-	{ .compatible = "adi,ad5620-1250", .data = &ad5620_1250_chip_info },
-	{ .compatible = "adi,ad5640-2500", .data = &ad5640_2500_chip_info },
-	{ .compatible = "adi,ad5640-1250", .data = &ad5640_1250_chip_info },
-	{ .compatible = "adi,ad5660-2500", .data = &ad5660_2500_chip_info },
-	{ .compatible = "adi,ad5660-1250", .data = &ad5660_1250_chip_info },
-	{ .compatible = "adi,ad5662", .data = &ad5662_chip_info },
-	{ .compatible = "ti,dac081s101", .data = &ad5300_chip_info },
-	{ .compatible = "ti,dac101s101", .data = &ad5310_chip_info },
-	{ .compatible = "ti,dac121s101", .data = &ad5320_chip_info },
-	{ .compatible = "ti,dac7512", .data = &ad5320_chip_info },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, ad5446_of_ids);
-
-static int ad5446_spi_probe(struct spi_device *spi)
-{
-	const struct spi_device_id *id = spi_get_device_id(spi);
-	const struct ad5446_chip_info *chip_info;
-
-	chip_info = spi_get_device_match_data(spi);
-	if (!chip_info)
-		return -ENODEV;
-
-	return ad5446_probe(&spi->dev, id->name, chip_info);
-}
-
-static struct spi_driver ad5446_spi_driver = {
-	.driver = {
-		.name	= "ad5446",
-		.of_match_table = ad5446_of_ids,
-	},
-	.probe		= ad5446_spi_probe,
-	.id_table	= ad5446_spi_ids,
-};
-
-static int __init ad5446_spi_register_driver(void)
-{
-	return spi_register_driver(&ad5446_spi_driver);
-}
-
-static void ad5446_spi_unregister_driver(void)
-{
-	spi_unregister_driver(&ad5446_spi_driver);
-}
-
-#else
-
-static inline int ad5446_spi_register_driver(void) { return 0; }
-static inline void ad5446_spi_unregister_driver(void) { }
-
-#endif
-
-#if IS_ENABLED(CONFIG_I2C)
-
-static int ad5622_write(struct ad5446_state *st, unsigned val)
-{
-	struct i2c_client *client = to_i2c_client(st->dev);
-	st->d16 = cpu_to_be16(val);
-	int ret;
-
-	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
-	if (ret < 0)
-		return ret;
-	if (ret != sizeof(st->d16))
-		return -EIO;
-
-	return 0;
-}
-
-/*
- * ad5446_supported_i2c_device_ids:
- * The AD5620/40/60 parts are available in different fixed internal reference
- * voltage options. The actual part numbers may look differently
- * (and a bit cryptic), however this style is used to make clear which
- * parts are supported here.
- */
-
-static const struct ad5446_chip_info ad5602_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
-	.write = ad5622_write,
-};
-
-static const struct ad5446_chip_info ad5612_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
-	.write = ad5622_write,
-};
-
-static const struct ad5446_chip_info ad5622_chip_info = {
-	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
-	.write = ad5622_write,
-};
-
-static int ad5446_i2c_probe(struct i2c_client *i2c)
-{
-	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
-	const struct ad5446_chip_info *chip_info;
-
-	chip_info = i2c_get_match_data(i2c);
-	if (!chip_info)
-		return -ENODEV;
-
-	return ad5446_probe(&i2c->dev, id->name, chip_info);
-}
-
-static const struct i2c_device_id ad5446_i2c_ids[] = {
-	{"ad5301", (kernel_ulong_t)&ad5602_chip_info},
-	{"ad5311", (kernel_ulong_t)&ad5612_chip_info},
-	{"ad5321", (kernel_ulong_t)&ad5622_chip_info},
-	{"ad5602", (kernel_ulong_t)&ad5602_chip_info},
-	{"ad5612", (kernel_ulong_t)&ad5612_chip_info},
-	{"ad5622", (kernel_ulong_t)&ad5622_chip_info},
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
-
-static const struct of_device_id ad5446_i2c_of_ids[] = {
-	{ .compatible = "adi,ad5301", .data = &ad5602_chip_info },
-	{ .compatible = "adi,ad5311", .data = &ad5612_chip_info },
-	{ .compatible = "adi,ad5321", .data = &ad5622_chip_info },
-	{ .compatible = "adi,ad5602", .data = &ad5602_chip_info },
-	{ .compatible = "adi,ad5612", .data = &ad5612_chip_info },
-	{ .compatible = "adi,ad5622", .data = &ad5622_chip_info },
-	{ }
-};
-MODULE_DEVICE_TABLE(OF, ad5446_i2c_of_ids);
-
-static struct i2c_driver ad5446_i2c_driver = {
-	.driver = {
-		   .name = "ad5446",
-		   .of_match_table = ad5446_i2c_of_ids,
-	},
-	.probe = ad5446_i2c_probe,
-	.id_table = ad5446_i2c_ids,
-};
-
-static int __init ad5446_i2c_register_driver(void)
-{
-	return i2c_add_driver(&ad5446_i2c_driver);
-}
-
-static void __exit ad5446_i2c_unregister_driver(void)
-{
-	i2c_del_driver(&ad5446_i2c_driver);
-}
-
-#else
-
-static inline int ad5446_i2c_register_driver(void) { return 0; }
-static inline void ad5446_i2c_unregister_driver(void) { }
-
-#endif
-
-static int __init ad5446_init(void)
-{
-	int ret;
-
-	ret = ad5446_spi_register_driver();
-	if (ret)
-		return ret;
-
-	ret = ad5446_i2c_register_driver();
-	if (ret) {
-		ad5446_spi_unregister_driver();
-		return ret;
-	}
-
-	return 0;
-}
-module_init(ad5446_init);
-
-static void __exit ad5446_exit(void)
-{
-	ad5446_i2c_unregister_driver();
-	ad5446_spi_unregister_driver();
-}
-module_exit(ad5446_exit);
-
 MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
-MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
+MODULE_DESCRIPTION("Analog Devices CORE AD5444/AD5446 DAC");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/dac/ad5446.h b/drivers/iio/dac/ad5446.h
new file mode 100644
index 000000000000..ee3d2c7d1764
--- /dev/null
+++ b/drivers/iio/dac/ad5446.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_AD5446_H
+#define _LINUX_AD5446_H
+
+#include <linux/bits.h>
+#include <linux/iio/iio.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+struct device;
+
+extern const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[];
+
+#define _AD5446_CHANNEL(bits, storage, _shift, ext) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.output = 1, \
+	.channel = 0, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = (bits), \
+		.storagebits = (storage), \
+		.shift = (_shift), \
+		}, \
+	.ext_info = (ext), \
+}
+
+#define AD5446_CHANNEL(bits, storage, shift) \
+	_AD5446_CHANNEL(bits, storage, shift, NULL)
+
+#define AD5446_CHANNEL_POWERDOWN(bits, storage, shift) \
+	_AD5446_CHANNEL(bits, storage, shift, ad5446_ext_info_powerdown)
+
+/**
+ * struct ad5446_state - driver instance specific data
+ * @dev:		this device
+ * @chip_info:		chip model specific constants, available modes etc
+ * @vref_mv:		actual reference voltage used
+ * @cached_val:		store/retrieve values during power down
+ * @pwr_down_mode:	power down mode (1k, 100k or tristate)
+ * @pwr_down:		true if the device is in power down
+ * @lock:		lock to protect the data buffer during write ops
+ */
+struct ad5446_state {
+	struct device *dev;
+	const struct ad5446_chip_info *chip_info;
+	unsigned short vref_mv;
+	unsigned int cached_val;
+	unsigned int pwr_down_mode;
+	unsigned int pwr_down;
+	/* mutex to protect device shared data */
+	struct mutex lock;
+	union {
+		__be16 d16;
+		u8 d24[3];
+	} __aligned(IIO_DMA_MINALIGN);
+};
+
+/**
+ * struct ad5446_chip_info - chip specific information
+ * @channel:		channel spec for the DAC
+ * @int_vref_mv:	AD5620/40/60: the internal reference voltage
+ * @write:		chip specific helper function to write to the register
+ */
+struct ad5446_chip_info {
+	struct iio_chan_spec channel;
+	u16 int_vref_mv;
+	int (*write)(struct ad5446_state *st, unsigned int val);
+};
+
+int ad5446_probe(struct device *dev, const char *name,
+		 const struct ad5446_chip_info *chip_info);
+
+#endif

-- 
2.51.0

[PATCH v3 07/10] iio: dac: ad5446: Make use of devm_mutex_init()

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Use devm_mutex_init() which is helpful with CONFIG_DEBUG_MUTEXES.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index 790e24770062..d9fc69e1039f 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -184,7 +184,9 @@ int ad5446_probe(struct device *dev, const char *name,
 	indio_dev->channels = &st->chip_info->channel;
 	indio_dev->num_channels = 1;
 
-	mutex_init(&st->lock);
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
 
 	st->pwr_down_mode = MODE_PWRDWN_1k;
 

-- 
2.51.0

[PATCH v3 08/10] iio: dac: ad5446: Make use of the cleanup helpers

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Use the auto unlocking helpers from cleanup.h. Allows for some code
simplification.

While at it, don't use the ternary operator in
ad5446_write_dac_powerdown() and add an helper function to write the DAC
code. The reason for the function was purely to avoid having to use
unreachable().

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index d9fc69e1039f..d288fb56e324 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -5,6 +5,7 @@
  * Copyright 2010 Analog Devices Inc.
  */
 
+#include <linux/cleanup.h>
 #include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/workqueue.h>
@@ -80,7 +81,7 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	st->pwr_down = powerdown;
 
 	if (st->pwr_down) {
@@ -91,9 +92,10 @@ static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
 	}
 
 	ret = st->chip_info->write(st, val);
-	mutex_unlock(&st->lock);
+	if (ret)
+		return ret;
 
-	return ret ? ret : len;
+	return len;
 }
 
 const struct iio_chan_spec_ext_info ad5446_ext_info_powerdown[] = {
@@ -129,32 +131,37 @@ static int ad5446_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static int ad5446_write_dac_raw(struct iio_dev *indio_dev,
+				const struct iio_chan_spec *chan,
+				int val)
+{
+	struct ad5446_state *st = iio_priv(indio_dev);
+
+	if (val >= (1 << chan->scan_type.realbits) || val < 0)
+		return -EINVAL;
+
+	val <<= chan->scan_type.shift;
+	guard(mutex)(&st->lock);
+
+	st->cached_val = val;
+	if (st->pwr_down)
+		return 0;
+
+	return st->chip_info->write(st, val);
+}
+
 static int ad5446_write_raw(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int val,
 			       int val2,
 			       long mask)
 {
-	struct ad5446_state *st = iio_priv(indio_dev);
-	int ret = 0;
-
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val >= (1 << chan->scan_type.realbits) || val < 0)
-			return -EINVAL;
-
-		val <<= chan->scan_type.shift;
-		mutex_lock(&st->lock);
-		st->cached_val = val;
-		if (!st->pwr_down)
-			ret = st->chip_info->write(st, val);
-		mutex_unlock(&st->lock);
-		break;
+		return ad5446_write_dac_raw(indio_dev, chan, val);
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	return ret;
 }
 
 static const struct iio_info ad5446_info = {

-- 
2.51.0

[PATCH v3 09/10] iio: dac: ad5446: Fix coding style issues

[Nuno Sá via B4 Relay]

From: Nuno Sá <nuno.sa@analog.com>

Fix style issues as reported by checkpatch. Additionally, make sure
include files are given in alphabetical order and that we include the
ones that were missing and remove the one we don't really use.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446.c | 41 ++++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
index d288fb56e324..bf9f7edb1a81 100644
--- a/drivers/iio/dac/ad5446.c
+++ b/drivers/iio/dac/ad5446.c
@@ -5,21 +5,17 @@
  * Copyright 2010 Analog Devices Inc.
  */
 
+#include <linux/array_size.h>
 #include <linux/cleanup.h>
-#include <linux/export.h>
-#include <linux/interrupt.h>
-#include <linux/workqueue.h>
 #include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
-#include <linux/list.h>
-#include <linux/regulator/consumer.h>
 #include <linux/err.h>
-#include <linux/module.h>
-
+#include <linux/export.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sysfs.h>
 
 #include "ad5446.h"
 
@@ -32,7 +28,8 @@ static const char * const ad5446_powerdown_modes[] = {
 };
 
 static int ad5446_set_powerdown_mode(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *chan, unsigned int mode)
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 
@@ -42,7 +39,7 @@ static int ad5446_set_powerdown_mode(struct iio_dev *indio_dev,
 }
 
 static int ad5446_get_powerdown_mode(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *chan)
+				     const struct iio_chan_spec *chan)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 
@@ -57,9 +54,9 @@ static const struct iio_enum ad5446_powerdown_mode_enum = {
 };
 
 static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
-					   uintptr_t private,
-					   const struct iio_chan_spec *chan,
-					   char *buf)
+					 uintptr_t private,
+					 const struct iio_chan_spec *chan,
+					 char *buf)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 
@@ -67,9 +64,9 @@ static ssize_t ad5446_read_dac_powerdown(struct iio_dev *indio_dev,
 }
 
 static ssize_t ad5446_write_dac_powerdown(struct iio_dev *indio_dev,
-					    uintptr_t private,
-					    const struct iio_chan_spec *chan,
-					    const char *buf, size_t len)
+					  uintptr_t private,
+					  const struct iio_chan_spec *chan,
+					  const char *buf, size_t len)
 {
 	struct ad5446_state *st = iio_priv(indio_dev);
 	unsigned int shift;
@@ -151,10 +148,8 @@ static int ad5446_write_dac_raw(struct iio_dev *indio_dev,
 }
 
 static int ad5446_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val,
-			       int val2,
-			       long mask)
+			    struct iio_chan_spec const *chan, int val,
+			    int val2, long mask)
 {
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:

-- 
2.51.0

[PATCH v3 10/10] iio: dac: ad5446: Add AD5542 to the spi id table

[Nuno Sá via B4 Relay]

From: Michael Hennerich <michael.hennerich@analog.com>

This adds support for the AD5542 single channel Current Source and
Voltage Output DACs.

It is similar to the AD5542A model so just use the same id.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Co-developed-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5446-spi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/dac/ad5446-spi.c b/drivers/iio/dac/ad5446-spi.c
index 7419b17a455f..34d4a737156d 100644
--- a/drivers/iio/dac/ad5446-spi.c
+++ b/drivers/iio/dac/ad5446-spi.c
@@ -177,6 +177,7 @@ static const struct spi_device_id ad5446_spi_ids[] = {
 	{"ad5453", (kernel_ulong_t)&ad5446_chip_info}, /* ad5453 is compatible to the ad5446 */
 	{"ad5512a", (kernel_ulong_t)&ad5512a_chip_info},
 	{"ad5541a", (kernel_ulong_t)&ad5541a_chip_info},
+	{"ad5542", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5542 are compatible */
 	{"ad5542a", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5542a are compatible */
 	{"ad5543", (kernel_ulong_t)&ad5541a_chip_info}, /* ad5541a and ad5543 are compatible */
 	{"ad5553", (kernel_ulong_t)&ad5553_chip_info},
@@ -213,6 +214,7 @@ static const struct of_device_id ad5446_of_ids[] = {
 	{ .compatible = "adi,ad5453", .data = &ad5446_chip_info },
 	{ .compatible = "adi,ad5512a", .data = &ad5512a_chip_info },
 	{ .compatible = "adi,ad5541a", .data = &ad5541a_chip_info },
+	{ .compatible = "adi,ad5542", .data = &ad5541a_chip_info },
 	{ .compatible = "adi,ad5542a", .data = &ad5541a_chip_info },
 	{ .compatible = "adi,ad5543", .data = &ad5541a_chip_info },
 	{ .compatible = "adi,ad5553", .data = &ad5553_chip_info },

-- 
2.51.0

Re: [PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 12:31:23PM +0000, Nuno Sá via B4 Relay wrote:
> 
> Make sure to use DMA safe buffer. While for i2c we could be fine without
> them, we need it for spi anyways.
> 
> As we now have DMA safe buffers, use i2c_master_send_dmasafe().

...

> +	union {
> +		__be16 d16;
> +		u8 d24[3];

Why not __be32 d24; ? Yes, it will require explicit size to be provided, but at
least it will look consistent with the above. OR u8 d16[2]; ? But then it becomes
simply a u8 buf[3] __aligned...;

> +	} __aligned(IIO_DMA_MINALIGN);
>  };

...

>  static int ad5660_write(struct ad5446_state *st, unsigned val)
>  {
>  	struct spi_device *spi = to_spi_device(st->dev);
> -	uint8_t data[3];
>  
> -	put_unaligned_be24(val, &data[0]);
> +	put_unaligned_be24(val, &st->d24[0]);
>  
> -	return spi_write(spi, data, sizeof(data));
> +	return spi_write(spi, st->d24, sizeof(st->d24));
>  }

...

>  static int ad5622_write(struct ad5446_state *st, unsigned val)
>  {
>  	struct i2c_client *client = to_i2c_client(st->dev);
> -	__be16 data = cpu_to_be16(val);
> +	st->d16 = cpu_to_be16(val);

Not really, we don't mix code with definitions (with only few exceptions,
mostly related to cleanup.h).

>  	int ret;
>  
> -	ret = i2c_master_send(client, (char *)&data, sizeof(data));
> +	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));

This will add a quite an overhead to the transfer (not that I²C is super fast,
but rather the processor is going to do _a lot_ of additional work here instead
of doing something more useful.

>  	if (ret < 0)
>  		return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 03/10] iio: dac: ad5446: Don't ignore missing regulator

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 12:31:24PM +0000, Nuno Sá via B4 Relay wrote:
> 
> If the chip does not have an internal reference, do not ignore a missing
> regulator as we won't be able to actually provide a proper scale for the
> DAC. Since it's now seen as an error, flip the if() logic so errors are
> treated first (which is the typical pattern).

...

>  	if (ret < 0 && ret != -ENODEV)
>  		return ret;

Not exactly for your patch, but the whole chain can be refactored as

>  	if (ret == -ENODEV) {
> -		if (chip_info->int_vref_mv)
> -			st->vref_mv = chip_info->int_vref_mv;
> -		else
> -			dev_warn(dev, "reference voltage unspecified\n");
> +		if (!chip_info->int_vref_mv)
> +			return dev_err_probe(dev, ret,
> +					     "reference voltage unspecified\n");
> +
> +		st->vref_mv = chip_info->int_vref_mv;
>  	} else {
>  		st->vref_mv = ret / 1000;
>  	}

(assuming ret is never 0)

	if (ret == -ENODEV)
		...
	else if (ret)
		return ret;
	else
		...
	if (!st->vref_mv)
		return dev_err_probe(dev, ret, "reference voltage unspecified\n");

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 03/10] iio: dac: ad5446: Don't ignore missing regulator

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 03:40:11PM +0200, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 12:31:24PM +0000, Nuno Sá via B4 Relay wrote:

...

> >  	if (ret < 0 && ret != -ENODEV)
> >  		return ret;
> 
> Not exactly for your patch, but the whole chain can be refactored as
> 
> >  	if (ret == -ENODEV) {
> > -		if (chip_info->int_vref_mv)
> > -			st->vref_mv = chip_info->int_vref_mv;
> > -		else
> > -			dev_warn(dev, "reference voltage unspecified\n");
> > +		if (!chip_info->int_vref_mv)
> > +			return dev_err_probe(dev, ret,
> > +					     "reference voltage unspecified\n");
> > +
> > +		st->vref_mv = chip_info->int_vref_mv;
> >  	} else {
> >  		st->vref_mv = ret / 1000;
> >  	}
> 
> (assuming ret is never 0)
> 
> 	if (ret == -ENODEV)
> 		...
> 	else if (ret)
> 		return ret;
> 	else
> 		...
> 	if (!st->vref_mv)
> 		return dev_err_probe(dev, ret, "reference voltage unspecified\n");

Yeah, we still need to alter ret here... Whatever, just give a thought on it, maybe
you can up with something more beautiful.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 04/10] iio: dac: ad5446: Move to single chip_info structures

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 12:31:25PM +0000, Nuno Sá via B4 Relay wrote:
> 
> Do not use an array with an enum id kind of thing. Use the more
> maintainable chip_info variable per chip.
> 
> Adapt the probe functions to use the proper helpers (for SPI and I2c).
> Note that in a following patch we'll also add the chip_info variables to
> the of_device_id tables. Hence already use the helpers that internally use
> device_get_match_data().

...

> +static const struct ad5446_chip_info ad5310_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5320_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5444_chip_info = {
> +	.channel = AD5446_CHANNEL(12, 16, 2),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5446_chip_info = {
> +	.channel = AD5446_CHANNEL(14, 16, 0),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5450_chip_info = {
> +	.channel = AD5446_CHANNEL(8, 16, 6),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5451_chip_info = {
> +	.channel = AD5446_CHANNEL(10, 16, 4),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5541a_chip_info = {
> +	.channel = AD5446_CHANNEL(16, 16, 0),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5512a_chip_info = {
> +	.channel = AD5446_CHANNEL(12, 16, 4),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5553_chip_info = {
> +	.channel = AD5446_CHANNEL(14, 16, 0),
> +	.write = ad5446_write,
> +};

> +static const struct ad5446_chip_info ad5600_chip_info = {
> +	.channel = AD5446_CHANNEL(16, 16, 0),
> +	.write = ad5446_write,
> +};

Seems same as ad5541a_chip_info(). Do we need duplicates _now_?

> +static const struct ad5446_chip_info ad5601_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5611_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5621_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5641_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5620_2500_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +	.int_vref_mv = 2500,
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5620_1250_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
> +	.int_vref_mv = 1250,
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5640_2500_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> +	.int_vref_mv = 2500,
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5640_1250_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
> +	.int_vref_mv = 1250,
> +	.write = ad5446_write,
> +};
> +
> +static const struct ad5446_chip_info ad5660_2500_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +	.int_vref_mv = 2500,
> +	.write = ad5660_write,
> +};
> +
> +static const struct ad5446_chip_info ad5660_1250_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +	.int_vref_mv = 1250,
> +	.write = ad5660_write,
> +};
> +
> +static const struct ad5446_chip_info ad5662_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
> +	.write = ad5660_write,
>  };

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Nuno Sá]

On Fri, 2025-10-31 at 15:36 +0200, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 12:31:23PM +0000, Nuno Sá via B4 Relay wrote:
> > 
> > Make sure to use DMA safe buffer. While for i2c we could be fine without
> > them, we need it for spi anyways.
> > 
> > As we now have DMA safe buffers, use i2c_master_send_dmasafe().
> 
> ...
> 
> > +	union {
> > +		__be16 d16;
> > +		u8 d24[3];
> 
> Why not __be32 d24; ? Yes, it will require explicit size to be provided, but at
> least it will look consistent with the above. OR u8 d16[2]; ? But then it becomes
> simply a u8 buf[3] __aligned...;

Because I'm just keeping put_unaligned_be24() as before. In fact I'm just keeping the
same type. Sure we could do __be32 and the cpu_to_be32() with a proper shift but 
I'm already doing way too much than I signed up for when sending v1 :)

> 
> > +	} __aligned(IIO_DMA_MINALIGN);
> >  };
> 
> ...
> 
> >  static int ad5660_write(struct ad5446_state *st, unsigned val)
> >  {
> >  	struct spi_device *spi = to_spi_device(st->dev);
> > -	uint8_t data[3];
> >  
> > -	put_unaligned_be24(val, &data[0]);
> > +	put_unaligned_be24(val, &st->d24[0]);
> >  
> > -	return spi_write(spi, data, sizeof(data));
> > +	return spi_write(spi, st->d24, sizeof(st->d24));
> >  }
> 
> ...
> 
> >  static int ad5622_write(struct ad5446_state *st, unsigned val)
> >  {
> >  	struct i2c_client *client = to_i2c_client(st->dev);
> > -	__be16 data = cpu_to_be16(val);
> > +	st->d16 = cpu_to_be16(val);
> 
> Not really, we don't mix code with definitions (with only few exceptions,
> mostly related to cleanup.h).

I know, and that is fixed afterwards. That is a cleanup that does not belong to this
patch IMO (and checkpatch kind of agrees :))
> 
> >  	int ret;
> >  
> > -	ret = i2c_master_send(client, (char *)&data, sizeof(data));
> > +	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
> 
> This will add a quite an overhead to the transfer (not that I²C is super fast,
> but rather the processor is going to do _a lot_ of additional work here instead
> of doing something more useful.

No really. This exactly to tell the i2c to not do any bounce buffer if the adapter
calls i2c_get_dma_safe_msg_buf(). So I would say, it's actually faster.

- Nuno Sá

> 
> >  	if (ret < 0)
> >  		return ret;

Re: [PATCH v3 04/10] iio: dac: ad5446: Move to single chip_info structures

[Nuno Sá]

On Fri, 2025-10-31 at 15:47 +0200, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 12:31:25PM +0000, Nuno Sá via B4 Relay wrote:
> > 
> > Do not use an array with an enum id kind of thing. Use the more
> > maintainable chip_info variable per chip.
> > 
> > Adapt the probe functions to use the proper helpers (for SPI and I2c).
> > Note that in a following patch we'll also add the chip_info variables to
> > the of_device_id tables. Hence already use the helpers that internally use
> > device_get_match_data().
> 
> ...
> 
> > +static const struct ad5446_chip_info ad5310_chip_info = {
> > +	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5320_chip_info = {
> > +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5444_chip_info = {
> > +	.channel = AD5446_CHANNEL(12, 16, 2),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5446_chip_info = {
> > +	.channel = AD5446_CHANNEL(14, 16, 0),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5450_chip_info = {
> > +	.channel = AD5446_CHANNEL(8, 16, 6),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5451_chip_info = {
> > +	.channel = AD5446_CHANNEL(10, 16, 4),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5541a_chip_info = {
> > +	.channel = AD5446_CHANNEL(16, 16, 0),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5512a_chip_info = {
> > +	.channel = AD5446_CHANNEL(12, 16, 4),
> > +	.write = ad5446_write,
> > +};
> > +
> > +static const struct ad5446_chip_info ad5553_chip_info = {
> > +	.channel = AD5446_CHANNEL(14, 16, 0),
> > +	.write = ad5446_write,
> > +};
> 
> > +static const struct ad5446_chip_info ad5600_chip_info = {
> > +	.channel = AD5446_CHANNEL(16, 16, 0),
> > +	.write = ad5446_write,
> > +};
> 
> Seems same as ad5541a_chip_info(). Do we need duplicates _now_?
> 

Yeah, it seems it was a duplicate before. I guess it could be a precursor
patch. Or if there's no real need for a re-spin (other than this), it
could be a follow up one, maybe?

- Nuno Sá

> 

Re: [PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 03:00:07PM +0000, Nuno Sá wrote:
> On Fri, 2025-10-31 at 15:36 +0200, Andy Shevchenko wrote:
> > On Fri, Oct 31, 2025 at 12:31:23PM +0000, Nuno Sá via B4 Relay wrote:

...

> > > +	union {
> > > +		__be16 d16;
> > > +		u8 d24[3];
> > 
> > Why not __be32 d24; ? Yes, it will require explicit size to be provided, but at
> > least it will look consistent with the above. OR u8 d16[2]; ? But then it becomes
> > simply a u8 buf[3] __aligned...;
> 
> Because I'm just keeping put_unaligned_be24() as before. In fact I'm just keeping the
> same type. Sure we could do __be32 and the cpu_to_be32() with a proper shift but 
> I'm already doing way too much than I signed up for when sending v1 :)

I think no shift would be needed.

> > > +	} __aligned(IIO_DMA_MINALIGN);
> > >  };

...

> > > -	ret = i2c_master_send(client, (char *)&data, sizeof(data));
> > > +	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
> > 
> > This will add a quite an overhead to the transfer (not that I²C is super fast,
> > but rather the processor is going to do _a lot_ of additional work here instead
> > of doing something more useful.
> 
> No really. This exactly to tell the i2c to not do any bounce buffer if the adapter
> calls i2c_get_dma_safe_msg_buf(). So I would say, it's actually faster.

I might have forgotten the implementation of that, but does it hold for the
controllers (or cases) that never supported DMA?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 04/10] iio: dac: ad5446: Move to single chip_info structures

[Andy Shevchenko]

On Fri, Oct 31, 2025 at 03:05:41PM +0000, Nuno Sá wrote:
> On Fri, 2025-10-31 at 15:47 +0200, Andy Shevchenko wrote:
> > On Fri, Oct 31, 2025 at 12:31:25PM +0000, Nuno Sá via B4 Relay wrote:

...

> > > +static const struct ad5446_chip_info ad5310_chip_info = {
> > > +	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5320_chip_info = {
> > > +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5444_chip_info = {
> > > +	.channel = AD5446_CHANNEL(12, 16, 2),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5446_chip_info = {
> > > +	.channel = AD5446_CHANNEL(14, 16, 0),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5450_chip_info = {
> > > +	.channel = AD5446_CHANNEL(8, 16, 6),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5451_chip_info = {
> > > +	.channel = AD5446_CHANNEL(10, 16, 4),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5541a_chip_info = {
> > > +	.channel = AD5446_CHANNEL(16, 16, 0),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5512a_chip_info = {
> > > +	.channel = AD5446_CHANNEL(12, 16, 4),
> > > +	.write = ad5446_write,
> > > +};
> > > +
> > > +static const struct ad5446_chip_info ad5553_chip_info = {
> > > +	.channel = AD5446_CHANNEL(14, 16, 0),
> > > +	.write = ad5446_write,
> > > +};
> > 
> > > +static const struct ad5446_chip_info ad5600_chip_info = {
> > > +	.channel = AD5446_CHANNEL(16, 16, 0),
> > > +	.write = ad5446_write,
> > > +};
> > 
> > Seems same as ad5541a_chip_info(). Do we need duplicates _now_?
> 
> Yeah, it seems it was a duplicate before. I guess it could be a precursor
> patch. Or if there's no real need for a re-spin (other than this), it
> could be a follow up one, maybe?

Ideally, if possible to implement, this should be a precursor cleanup.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Jonathan Cameron]

On Fri, 31 Oct 2025 17:07:33 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Fri, Oct 31, 2025 at 03:00:07PM +0000, Nuno Sá wrote:
> > On Fri, 2025-10-31 at 15:36 +0200, Andy Shevchenko wrote:  
> > > On Fri, Oct 31, 2025 at 12:31:23PM +0000, Nuno Sá via B4 Relay wrote:  
> 
> ...
> 
> > > > +	union {
> > > > +		__be16 d16;
> > > > +		u8 d24[3];  
> > > 
> > > Why not __be32 d24; ? Yes, it will require explicit size to be provided, but at
> > > least it will look consistent with the above. OR u8 d16[2]; ? But then it becomes
> > > simply a u8 buf[3] __aligned...;  
> > 
> > Because I'm just keeping put_unaligned_be24() as before. In fact I'm just keeping the
> > same type. Sure we could do __be32 and the cpu_to_be32() with a proper shift but 
> > I'm already doing way too much than I signed up for when sending v1 :)  
> 
> I think no shift would be needed.

Using a __be32 to me implies things we don't want to imply. In particular
if it's read in to &d24 then if interpreted as a __be32 it would be 256x
too big (I'd not do this for le24 either though which doesn't need the shift)
The u8 [3] makes no such implication.

So I'd prefer sticking to storing be24 in a 3 byte location.

(just picking out this one bit to reply to).

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Jonathan Cameron]

On Fri, 31 Oct 2025 12:31:27 +0000
Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> Properly separate the I2C and SPI drivers into two different drivers
> living in their own source file (as usual). So that no need for the
> hacky ifdefery.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Looks good to me.  A few minor things inline.
In particular a question of which include is missing for the byteorder stuff.
Maybe Andy can help with a suggestion on that?


> diff --git a/drivers/iio/dac/ad5446-i2c.c b/drivers/iio/dac/ad5446-i2c.c
> new file mode 100644
> index 000000000000..9d1054c3dd8e
> --- /dev/null
> +++ b/drivers/iio/dac/ad5446-i2c.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AD5446 SPI I2C driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/i2c.h>
> +
> +#include "ad5446.h"
> +
> +static int ad5622_write(struct ad5446_state *st, unsigned int val)
> +{
> +	struct i2c_client *client = to_i2c_client(st->dev);
> +	int ret;
> +
> +	st->d16 = cpu_to_be16(val);

Should have an include for this.  Probably

linux/byteorder/generic.h
though the kernel (and iio) has a random mix of that and
asm/byteorder.h

Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better choice



> +
> +	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(st->d16))
> +		return -EIO;
include for the error codes as well.
> +
> +	return 0;
> +}
> +
> +static int ad5446_i2c_probe(struct i2c_client *i2c)
> +{
> +	const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
> +	const struct ad5446_chip_info *chip_info;
> +
> +	chip_info = i2c_get_match_data(i2c);
> +	if (!chip_info)
> +		return -ENODEV;
> +
> +	return ad5446_probe(&i2c->dev, id->name, chip_info);
> +}
> +
> +/*
> + * ad5446_supported_i2c_device_ids:
> + * The AD5620/40/60 parts are available in different fixed internal reference
> + * voltage options. The actual part numbers may look differently
> + * (and a bit cryptic), however this style is used to make clear which
> + * parts are supported here.
> + */
> +
> +static const struct ad5446_chip_info ad5602_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(8, 16, 4),
> +	.write = ad5622_write,
> +};
> +
> +static const struct ad5446_chip_info ad5612_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(10, 16, 2),
> +	.write = ad5622_write,
> +};
> +
> +static const struct ad5446_chip_info ad5622_chip_info = {
> +	.channel = AD5446_CHANNEL_POWERDOWN(12, 16, 0),
> +	.write = ad5622_write,
> +};
> +
> +static const struct i2c_device_id ad5446_i2c_ids[] = {
> +	{"ad5301", (kernel_ulong_t)&ad5602_chip_info},
> +	{"ad5311", (kernel_ulong_t)&ad5612_chip_info},
> +	{"ad5321", (kernel_ulong_t)&ad5622_chip_info},
> +	{"ad5602", (kernel_ulong_t)&ad5602_chip_info},
> +	{"ad5612", (kernel_ulong_t)&ad5612_chip_info},
> +	{"ad5622", (kernel_ulong_t)&ad5622_chip_info},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ad5446_i2c_ids);
> +
> +static const struct of_device_id ad5446_i2c_of_ids[] = {
> +	{ .compatible = "adi,ad5301", .data = &ad5602_chip_info },
> +	{ .compatible = "adi,ad5311", .data = &ad5612_chip_info },
> +	{ .compatible = "adi,ad5321", .data = &ad5622_chip_info },
> +	{ .compatible = "adi,ad5602", .data = &ad5602_chip_info },
> +	{ .compatible = "adi,ad5612", .data = &ad5612_chip_info },
> +	{ .compatible = "adi,ad5622", .data = &ad5622_chip_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(OF, ad5446_i2c_of_ids);
> +
> +static struct i2c_driver ad5446_i2c_driver = {
> +	.driver = {
> +		.name	= "ad5446",
> +		.of_match_table = ad5446_i2c_of_ids,
> +	},
> +	.probe = ad5446_i2c_probe,
> +	.id_table = ad5446_i2c_ids,
> +};
> +module_i2c_driver(ad5446_i2c_driver);
> +
> +MODULE_AUTHOR("Nuno Sá <nuno.sa@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 I2C DACs");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("IIO_AD5446");
> diff --git a/drivers/iio/dac/ad5446-spi.c b/drivers/iio/dac/ad5446-spi.c
> new file mode 100644
> index 000000000000..7419b17a455f
> --- /dev/null
> +++ b/drivers/iio/dac/ad5446-spi.c
> @@ -0,0 +1,252 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AD5446 SPI DAC driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + */
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>

Similar comment on includes probably applies.

> +
> +#include "ad5446.h"
> +
>

>  MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
> -MODULE_DESCRIPTION("Analog Devices AD5444/AD5446 DAC");
> +MODULE_DESCRIPTION("Analog Devices CORE AD5444/AD5446 DAC");

Maybe a good time to tweak this description to say "AD5444 DAC and similar"

>  MODULE_LICENSE("GPL v2");

Re: [PATCH v3 09/10] iio: dac: ad5446: Fix coding style issues

[Jonathan Cameron]

On Fri, 31 Oct 2025 12:31:30 +0000
Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> Fix style issues as reported by checkpatch. Additionally, make sure
> include files are given in alphabetical order and that we include the
> ones that were missing and remove the one we don't really use.
If it's only 1 say what it is.  I see quite a few so guessing that
was meant to be plural?

Headers and white space are different things so really should be
2 patches.

Actual changes all look good

Jonathan

> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/dac/ad5446.c | 41 ++++++++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> index d288fb56e324..bf9f7edb1a81 100644
> --- a/drivers/iio/dac/ad5446.c
> +++ b/drivers/iio/dac/ad5446.c
> @@ -5,21 +5,17 @@
>   * Copyright 2010 Analog Devices Inc.
>   */
>  
> +#include <linux/array_size.h>
>  #include <linux/cleanup.h>
> -#include <linux/export.h>
> -#include <linux/interrupt.h>
> -#include <linux/workqueue.h>
>  #include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/sysfs.h>
> -#include <linux/list.h>
> -#include <linux/regulator/consumer.h>
>  #include <linux/err.h>
> -#include <linux/module.h>
> -
> +#include <linux/export.h>
>  #include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> +#include <linux/kstrtox.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sysfs.h>
>  
>  #include "ad5446.h"

Re: [PATCH v3 01/10] dt-bindings: iio: dac: Document AD5446 and similar devices

[Krzysztof Kozlowski]

On 31/10/2025 13:31, Nuno Sá via B4 Relay wrote:
> From: Nuno Sá <nuno.sa@analog.com>
> 
> Add device tree binding documentation for the Analog Devices AD5446
> family of Digital-to-Analog Converters and derivative devices from
> Texas Instruments. There's both SPI and I2C interfaces and feature
> resolutions ranging from 8-bit to 16-bit.
> 
> The binding covers 29 derivatives devices including the AD5446 series,
> AD5600 series, AD5620/5640/5660 variants with different voltage ranges,
> and TI DAC081s101/DAC101s101/DAC121s101 devices.


<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>


Best regards,
Krzysztof

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Andy Shevchenko]

On Sat, Nov 01, 2025 at 04:46:12PM +0000, Jonathan Cameron wrote:
> On Fri, 31 Oct 2025 12:31:27 +0000
> Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> Looks good to me.  A few minor things inline.
> In particular a question of which include is missing for the byteorder stuff.
> Maybe Andy can help with a suggestion on that?

asm/byteorder.h

...

> > +	st->d16 = cpu_to_be16(val);
> 
> Should have an include for this.  Probably
> 
> linux/byteorder/generic.h
> though the kernel (and iio) has a random mix of that and
> asm/byteorder.h

Can somebody go and fix all of them to be asm/byteorder.h? Yeah, it might need
some thinking as in some _rare_ cases the author might imply the explicit
choice of the algo in use. But then it might be problematic on some architectures
as well...

> Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better choice

Yes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 01/10] dt-bindings: iio: dac: Document AD5446 and similar devices

[Nuno Sá]

On Sun, 2025-11-02 at 10:18 +0100, Krzysztof Kozlowski wrote:
> On 31/10/2025 13:31, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Add device tree binding documentation for the Analog Devices AD5446
> > family of Digital-to-Analog Converters and derivative devices from
> > Texas Instruments. There's both SPI and I2C interfaces and feature
> > resolutions ranging from 8-bit to 16-bit.
> > 
> > The binding covers 29 derivatives devices including the AD5446 series,
> > AD5600 series, AD5620/5640/5660 variants with different voltage ranges,
> > and TI DAC081s101/DAC101s101/DAC121s101 devices.
> 
> 
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
> 
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time.
> 
> Please kindly resend and include all necessary To/Cc entries.
> </form letter>
> 
> 
> Best regards,
> Krzysztof

Bah, no idea where did I messed up. v2 did included devicetree
Anyways, sorry about that!

- Nuno Sá

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Nuno Sá]

On Mon, 2025-11-03 at 09:39 +0200, Andy Shevchenko wrote:
> On Sat, Nov 01, 2025 at 04:46:12PM +0000, Jonathan Cameron wrote:
> > On Fri, 31 Oct 2025 12:31:27 +0000
> > Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> > 
> > Looks good to me.  A few minor things inline.
> > In particular a question of which include is missing for the byteorder stuff.
> > Maybe Andy can help with a suggestion on that?
> 
> asm/byteorder.h
> 
> ...
> 
> > > +	st->d16 = cpu_to_be16(val);
> > 
> > Should have an include for this.  Probably
> > 
> > linux/byteorder/generic.h
> > though the kernel (and iio) has a random mix of that and
> > asm/byteorder.h
> 

Yeah, I did tried linux/byteorder/generic.h but it fails to compile if it's the first 
thing we include so I did not wanted to go that way :)
> Can somebody go and fix all of them to be asm/byteorder.h? Yeah, it might need
> some thinking as in some _rare_ cases the author might imply the explicit
> choice of the algo in use. But then it might be problematic on some architectures
> as well...
> 
> > Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better choice
> 
> Yes.

Hmm, so linux/unaligned.h or asm/byteorder.h? For the spi version I do have
linux/unaligned.h beacuse we use some APIs but for i2c there's no use on unaligned so
it feels a bit odd to include it?

- Nuno Sá

Re: [PATCH v3 02/10] iio: dac: ad5446: Use DMA safe buffer for transfers

[Nuno Sá]

On Fri, 2025-10-31 at 17:07 +0200, Andy Shevchenko wrote:
> On Fri, Oct 31, 2025 at 03:00:07PM +0000, Nuno Sá wrote:
> > On Fri, 2025-10-31 at 15:36 +0200, Andy Shevchenko wrote:
> > > On Fri, Oct 31, 2025 at 12:31:23PM +0000, Nuno Sá via B4 Relay wrote:
> 
> ...
> 
> > > > +	union {
> > > > +		__be16 d16;
> > > > +		u8 d24[3];
> > > 
> > > Why not __be32 d24; ? Yes, it will require explicit size to be provided, but at
> > > least it will look consistent with the above. OR u8 d16[2]; ? But then it becomes
> > > simply a u8 buf[3] __aligned...;
> > 
> > Because I'm just keeping put_unaligned_be24() as before. In fact I'm just keeping the
> > same type. Sure we could do __be32 and the cpu_to_be32() with a proper shift but 
> > I'm already doing way too much than I signed up for when sending v1 :)
> 
> I think no shift would be needed.

The shift would be just to discard MSB...

> 
> > > > +	} __aligned(IIO_DMA_MINALIGN);
> > > >  };
> 
> ...
> 
> > > > -	ret = i2c_master_send(client, (char *)&data, sizeof(data));
> > > > +	ret = i2c_master_send_dmasafe(client, (char *)&st->d16, sizeof(st->d16));
> > > 
> > > This will add a quite an overhead to the transfer (not that I²C is super fast,
> > > but rather the processor is going to do _a lot_ of additional work here instead
> > > of doing something more useful.
> > 
> > No really. This exactly to tell the i2c to not do any bounce buffer if the adapter
> > calls i2c_get_dma_safe_msg_buf(). So I would say, it's actually faster.
> 
> I might have forgotten the implementation of that, but does it hold for the
> controllers (or cases) that never supported DMA?

When the DMA flag is given, it just means i2c_get_dma_safe_msg_buf() will give the 
buffer you passed to i2c and does not setup any safe, bounce for you. So, It should
work.

- Nuno Sá

Re: [PATCH v3 09/10] iio: dac: ad5446: Fix coding style issues

[Nuno Sá]

On Sat, 2025-11-01 at 16:51 +0000, Jonathan Cameron wrote:
> On Fri, 31 Oct 2025 12:31:30 +0000
> Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Fix style issues as reported by checkpatch. Additionally, make sure
> > include files are given in alphabetical order and that we include the
> > ones that were missing and remove the one we don't really use.
> If it's only 1 say what it is.  I see quite a few so guessing that
> was meant to be plural?
> 
> Headers and white space are different things so really should be
> 2 patches.
> 

Ok, will do. I actually had two patches but then feel into the temptation of making
the series a bit smaller.

- Nuno Sá

> Actual changes all look good
> 
> Jonathan
> 
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/iio/dac/ad5446.c | 41 ++++++++++++++++++-----------------------
> >  1 file changed, 18 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c
> > index d288fb56e324..bf9f7edb1a81 100644
> > --- a/drivers/iio/dac/ad5446.c
> > +++ b/drivers/iio/dac/ad5446.c
> > @@ -5,21 +5,17 @@
> >   * Copyright 2010 Analog Devices Inc.
> >   */
> >  
> > +#include <linux/array_size.h>
> >  #include <linux/cleanup.h>
> > -#include <linux/export.h>
> > -#include <linux/interrupt.h>
> > -#include <linux/workqueue.h>
> >  #include <linux/device.h>
> > -#include <linux/kernel.h>
> > -#include <linux/slab.h>
> > -#include <linux/sysfs.h>
> > -#include <linux/list.h>
> > -#include <linux/regulator/consumer.h>
> >  #include <linux/err.h>
> > -#include <linux/module.h>
> > -
> > +#include <linux/export.h>
> >  #include <linux/iio/iio.h>
> > -#include <linux/iio/sysfs.h>
> > +#include <linux/kstrtox.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/sysfs.h>
> >  
> >  #include "ad5446.h"

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Andy Shevchenko]

On Mon, Nov 03, 2025 at 10:40:20AM +0000, Nuno Sá wrote:
> On Mon, 2025-11-03 at 09:39 +0200, Andy Shevchenko wrote:
> > On Sat, Nov 01, 2025 at 04:46:12PM +0000, Jonathan Cameron wrote:
> > > On Fri, 31 Oct 2025 12:31:27 +0000
> > > Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

...

> > > > +	st->d16 = cpu_to_be16(val);
> > > 
> > > Should have an include for this.  Probably
> > > 
> > > linux/byteorder/generic.h
> > > though the kernel (and iio) has a random mix of that and
> > > asm/byteorder.h
> 
> Yeah, I did tried linux/byteorder/generic.h but it fails to compile if it's the first 
> thing we include so I did not wanted to go that way :)
> > Can somebody go and fix all of them to be asm/byteorder.h? Yeah, it might need
> > some thinking as in some _rare_ cases the author might imply the explicit
> > choice of the algo in use. But then it might be problematic on some architectures
> > as well...
> > 
> > > Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better choice
> > 
> > Yes.
> 
> Hmm, so linux/unaligned.h or asm/byteorder.h? For the spi version I do have
> linux/unaligned.h beacuse we use some APIs but for i2c there's no use on unaligned so
> it feels a bit odd to include it?

Maybe I misread what Jonathan said, but I meant that asm/byteorder.h is a
better choice over other *byteorder*.h variants.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Nuno Sá]

On Mon, 2025-11-03 at 21:30 +0200, Andy Shevchenko wrote:
> On Mon, Nov 03, 2025 at 10:40:20AM +0000, Nuno Sá wrote:
> > On Mon, 2025-11-03 at 09:39 +0200, Andy Shevchenko wrote:
> > > On Sat, Nov 01, 2025 at 04:46:12PM +0000, Jonathan Cameron wrote:
> > > > On Fri, 31 Oct 2025 12:31:27 +0000
> > > > Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> ...
> 
> > > > > +	st->d16 = cpu_to_be16(val);
> > > > 
> > > > Should have an include for this.  Probably
> > > > 
> > > > linux/byteorder/generic.h
> > > > though the kernel (and iio) has a random mix of that and
> > > > asm/byteorder.h
> > 
> > Yeah, I did tried linux/byteorder/generic.h but it fails to compile if it's the
> > first 
> > thing we include so I did not wanted to go that way :)
> > > Can somebody go and fix all of them to be asm/byteorder.h? Yeah, it might need
> > > some thinking as in some _rare_ cases the author might imply the explicit
> > > choice of the algo in use. But then it might be problematic on some
> > > architectures
> > > as well...
> > > 
> > > > Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better
> > > > choice
> > > 
> > > Yes.
> > 
> > Hmm, so linux/unaligned.h or asm/byteorder.h? For the spi version I do have
> > linux/unaligned.h beacuse we use some APIs but for i2c there's no use on
> > unaligned so
> > it feels a bit odd to include it?
> 
> Maybe I misread what Jonathan said, but I meant that asm/byteorder.h is a
> better choice over other *byteorder*.h variants.

Ack. I was the one that got confused with your reply so I wanted to confirm
before v4.

- Nuno Sá

Re: [PATCH v3 06/10] iio: dac: ad5446: Separate I2C/SPI into different drivers

[Jonathan Cameron]

On Tue, 04 Nov 2025 07:44:13 +0000
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2025-11-03 at 21:30 +0200, Andy Shevchenko wrote:
> > On Mon, Nov 03, 2025 at 10:40:20AM +0000, Nuno Sá wrote:  
> > > On Mon, 2025-11-03 at 09:39 +0200, Andy Shevchenko wrote:  
> > > > On Sat, Nov 01, 2025 at 04:46:12PM +0000, Jonathan Cameron wrote:  
> > > > > On Fri, 31 Oct 2025 12:31:27 +0000
> > > > > Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:  
> > 
> > ...
> >   
> > > > > > +	st->d16 = cpu_to_be16(val);  
> > > > > 
> > > > > Should have an include for this.  Probably
> > > > > 
> > > > > linux/byteorder/generic.h
> > > > > though the kernel (and iio) has a random mix of that and
> > > > > asm/byteorder.h  
> > > 
> > > Yeah, I did tried linux/byteorder/generic.h but it fails to compile if it's the
> > > first 
> > > thing we include so I did not wanted to go that way :)  
> > > > Can somebody go and fix all of them to be asm/byteorder.h? Yeah, it might need
> > > > some thinking as in some _rare_ cases the author might imply the explicit
> > > > choice of the algo in use. But then it might be problematic on some
> > > > architectures
> > > > as well...
> > > >   
> > > > > Hmm. linux/unaligned.h is using asm/byteorder.h so maybe that's the better
> > > > > choice  
> > > > 
> > > > Yes.  
> > > 
> > > Hmm, so linux/unaligned.h or asm/byteorder.h? For the spi version I do have
> > > linux/unaligned.h beacuse we use some APIs but for i2c there's no use on
> > > unaligned so
> > > it feels a bit odd to include it?  
> > 
> > Maybe I misread what Jonathan said, but I meant that asm/byteorder.h is a
> > better choice over other *byteorder*.h variants.  
> 
> Ack. I was the one that got confused with your reply so I wanted to confirm
> before v4.
> 
I only meant that it was probably right choice in unaligned.h to use asm/byteorder.h
So definitely agree fixing all cases up to use asm/byteorder.h is the way
to go based on Andy's feedback.

unaligned.h is only appropriate if the calls made are all the unaligned variants.
If it's a mix, should include that and asm/byteorder.h

Thanks,

Jonathan

> - Nuno Sá
>