[PATCH 0/2] Add driver for DAC8163:

[PATCH 0/2] Add driver for DAC8163:

[Lukas Metz]

This series adds an IIO driver for the Texas Instruments DAC7562, DAC7563,
DAC8162, DAC8163, DAC8562, and DAC8563 dual-channel voltage-output DACs.

These devices are pin-compatible 12-, 14-, and 16-bit variants sharing the
same 24-bit SPI command interface. Each device provides two independently
addressable output channels and includes a 2.5 V, 4 ppm/°C internal
reference that can be enabled via device tree, or an external reference
supplied through a regulator. The register and command structure differs
from already existing drivers which makes adding a new driver a
reasonable choice in my opinion.

The driver supports:
 - All six device variants via a shared chip info table
 - DAC updates in synchronous mode
 - Configurable internal or external voltage reference
 - Optional LDAC GPIO which has to be asserted permanently when using
   synchronous updates.
 - IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE attributes per channel

Datasheet (DAC8163):
  https://www.ti.com/lit/gpn/dac8163

The driver was tested with a DAC8163 on a custom STM32MP157F board with
external reference enabled.

Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
---
Lukas Metz (2):
      iio: dac: dac8163: Add driver for DAC8163
      dt-bindings: iio: dac: Add DAC8163

 .../devicetree/bindings/iio/dac/ti,dac8163.yaml    |  75 +++++
 MAINTAINERS                                        |   7 +
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ti-dac8163.c                       | 339 +++++++++++++++++++++
 5 files changed, 432 insertions(+)
---
base-commit: 76b6720279964612111352ca5d09f5bd61e41ce4
change-id: 20260413-dac8163-work-2138a775b515

Best regards,
--  
Lukas <lukas.metz@gmx.net>

[PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163

[Lukas Metz]

The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
respectively. These devices include a 2.5-V, 4-ppm/°C internal
reference, giving a full-scale output voltage range of 2.5 V or 5 V.

Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
---
 MAINTAINERS                  |   6 +
 drivers/iio/dac/Kconfig      |  10 ++
 drivers/iio/dac/Makefile     |   1 +
 drivers/iio/dac/ti-dac8163.c | 339 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 356 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d238590a31f2..e82cc28e1bc3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26394,6 +26394,12 @@ S:	Odd Fixes
 F:	drivers/clk/ti/
 F:	include/linux/clk/ti.h
 
+TI DAC8163 DAC DRIVER
+M:	Lukas Metz <lukas.metz@gmx.net>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	drivers/iio/dac/ti-dac8163.c
+
 TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
 M:	T Pratham <t-pratham@ti.com>
 L:	linux-crypto@vger.kernel.org
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index db9f5c711b3d..6b6e5ee0732a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -632,6 +632,16 @@ config TI_DAC7612
 
 	  If compiled as a module, it will be called ti-dac7612.
 
+config TI_DAC8163
+	tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
+	depends on SPI_MASTER
+	help
+	  Driver for the Texas Instruments digital-to-analog converter
+	  family dacxx6x compatible with the variants DAC7562,
+	  DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.
+
+	  If compiled as a module, it will be called ti-dac8163.
+
 config VF610_DAC
 	tristate "Vybrid vf610 DAC driver"
 	depends on HAS_IOMEM
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 2a80bbf4e80a..359cde446623 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -62,4 +62,5 @@ obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
 obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
 obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o
 obj-$(CONFIG_TI_DAC7612) += ti-dac7612.o
+obj-$(CONFIG_TI_DAC8163) += ti-dac8163.o
 obj-$(CONFIG_VF610_DAC) += vf610_dac.o
diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
new file mode 100644
index 000000000000..84a9dfb5347d
--- /dev/null
+++ b/drivers/iio/dac/ti-dac8163.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DACxx6x IIO driver (SPI)
+ */
+
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+#include <linux/gpio/consumer.h>
+#include <linux/printk.h>
+#include <linux/bitfield.h>
+
+#define COMMAND_MASK GENMASK(6, 3)
+#define ADDRESS_MASK GENMASK(2, 0)
+
+#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
+							FIELD_PREP(ADDRESS_MASK, (y)))
+
+#define CMD_WRITE_INPUT_REG	0x0
+#define CMD_UPDATE_DAC	0x1
+#define CMD_WRITE_UPDATE_ALL	0x2
+#define CMD_WRITE_UPDATE	0x3
+#define CMD_SET_PWR_MODE		0x4
+#define CMD_SOFT_RST			0x5
+
+#define CMD_LDAC_MODE		0x6
+#define LDAC_MODE_CHANNEL_A_MASK BIT(0)
+#define LDAC_MODE_CHANNEL_B_MASK BIT(1)
+
+#define CMD_SEL_REFERENCE	0x7
+#define VOLTAGE_REFERENCE_MASK BIT(0)
+
+enum dacxx6x_ldac_modes {
+	LDAC_MODE_ACTIVE = 0,
+	LDAC_MODE_INACTIVE = 1
+};
+
+enum dacxx6x_voltage_reference {
+	VOLTAGE_REFERENCE_EXTERNAL = 0,
+	VOLTAGE_REFERENCE_INTERNAL = 1
+};
+
+enum dacxx6x_supported_device_ids {
+	ID_DAC7562,
+	ID_DAC7563,
+	ID_DAC8162,
+	ID_DAC8163,
+	ID_DAC8562,
+	ID_DAC8563
+};
+
+struct dacxx6x_state {
+	struct spi_device *spi;
+
+	struct regulator *vref;
+	struct gpio_desc *loaddacs;
+
+	bool internal_ref;
+	int vref_uv;
+
+	unsigned int cached[2];
+
+	/*
+	 * Lock to protect the state of the device from potential concurrent
+	 * write accesses from userspace.
+	 */
+	struct mutex lock;
+};
+
+struct dacxx6x_chip_info {
+	const char *name;
+	const struct iio_chan_spec channels[2];
+};
+
+#define DACXX6X_CHAN(id, resolution)                                        \
+	{                                                                   \
+		.type = IIO_VOLTAGE, .channel = (id), .output = 1,          \
+		.indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),       \
+		.scan_type = { .realbits = (resolution),                    \
+			       .shift = 16 - (resolution) },                \
+	}
+
+static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
+	[ID_DAC7562] = {
+		.name = "dac7562",
+		.channels = {
+			DACXX6X_CHAN(0, 12),
+			DACXX6X_CHAN(1, 12),
+		}
+	},
+	[ID_DAC7563] = {
+		.name = "dac7563",
+		.channels = {
+			DACXX6X_CHAN(0, 12),
+			DACXX6X_CHAN(1, 12),
+		}
+	},
+	[ID_DAC8162] = {
+		.name = "dac8162",
+		.channels = {
+			DACXX6X_CHAN(0, 14),
+			DACXX6X_CHAN(1, 14),
+		}
+	},
+	[ID_DAC8163] = {
+		.name = "dac8163",
+		.channels = {
+			DACXX6X_CHAN(0, 14),
+			DACXX6X_CHAN(1, 14),
+		}
+	},
+	[ID_DAC8562] = {
+		.name = "dac8562",
+		.channels = {
+			DACXX6X_CHAN(0, 16),
+			DACXX6X_CHAN(1, 16),
+		}
+	},
+	[ID_DAC8563] = {
+		.name = "dac8563",
+		.channels = {
+			DACXX6X_CHAN(0, 16),
+			DACXX6X_CHAN(1, 16),
+		}
+	},
+};
+
+static int dacxx6x_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	struct dacxx6x_state *st;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		st = iio_priv(indio_dev);
+		mutex_lock(&st->lock);
+		*val = st->cached[chan->channel];
+		mutex_unlock(&st->lock);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		st = iio_priv(indio_dev);
+		*val = st->vref_uv / MILLI; /* vref in mV */
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
+			     unsigned int val)
+{
+	u8 tx[3];
+
+	tx[0] = COMMAND_SET(reg, addr);
+	tx[1] = (val >> 8) & 0xff;
+	tx[2] = val & 0xff;
+
+	return spi_write(st->spi, tx, sizeof(tx));
+}
+
+static int dacxx6x_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int val,
+			     int val2, long mask)
+{
+	struct dacxx6x_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);
+		if (val2 != 0)
+			return -EINVAL;
+
+		if (val < 0 || val >= BIT(chan->scan_type.realbits))
+			return -EINVAL;
+
+		mutex_lock(&st->lock);
+		int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
+					    (unsigned int)val
+						    << chan->scan_type.shift);
+
+		if (!ret)
+			st->cached[chan->channel] = val;
+		mutex_unlock(&st->lock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info dacxx6x_iio_info = {
+	.write_raw = dacxx6x_write_raw,
+	.read_raw = dacxx6x_read_raw
+};
+
+static int dacxx6x_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct dacxx6x_state *st;
+	const struct dacxx6x_chip_info *info;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	spi_set_drvdata(spi, indio_dev);
+
+	st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(st->loaddacs))
+		return PTR_ERR(st->loaddacs);
+
+	st->internal_ref =
+		device_property_read_bool(&spi->dev, "ti,internal-ref");
+
+	if (!st->internal_ref) {
+		st->vref = devm_regulator_get(&spi->dev, "vref");
+		if (IS_ERR(st->vref))
+			return PTR_ERR(st->vref);
+
+		ret = regulator_enable(st->vref);
+		if (ret < 0)
+			return ret;
+	}
+
+	mutex_init(&st->lock);
+
+	if (st->internal_ref) {
+		st->vref_uv = 2500000; /* 2.5V internal reference */
+	} else {
+		st->vref_uv = regulator_get_voltage(st->vref);
+		if (st->vref_uv < 0) {
+			ret = st->vref_uv;
+			goto err;
+		}
+	}
+
+	gpiod_set_value(st->loaddacs, 0);
+
+	ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
+				FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
+				FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
+
+	if (ret < 0)
+		goto err;
+
+	ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
+				FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
+
+	if (ret < 0)
+		goto err;
+
+	info = spi_get_device_match_data(spi);
+
+	indio_dev->name = info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &dacxx6x_iio_info;
+	indio_dev->channels = info->channels;
+	indio_dev->num_channels = 2;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto err;
+
+	return 0;
+
+err:
+	if (!st->internal_ref)
+		regulator_disable(st->vref);
+	mutex_destroy(&st->lock);
+	return ret;
+}
+
+static void dacxx6x_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct dacxx6x_state *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	mutex_destroy(&st->lock);
+	if (!st->internal_ref)
+		regulator_disable(st->vref);
+}
+
+#define DACXX6X_COMPATIBLE(of_compatible, id)        \
+	{                                            \
+		.compatible = of_compatible,         \
+		.data = &dacxx6x_chip_info_table[id] \
+	}
+
+static const struct of_device_id dacxx6x_of_match[] = {
+	DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
+	DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
+	DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
+	DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
+	DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
+	DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
+	{}
+};
+MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
+
+static const struct spi_device_id dacxx6x_id_table[] = {
+	{ "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
+	{ "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
+	{ "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
+	{ "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
+	{ "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
+	{ "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
+	{}
+};
+
+MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
+
+static struct spi_driver dacxx6x_driver = {
+	.driver = {
+		.name = "ti-dacxx6x",
+		.of_match_table = dacxx6x_of_match,
+	},
+	.probe = dacxx6x_probe,
+	.remove = dacxx6x_remove,
+	.id_table = dacxx6x_id_table,
+};
+
+module_spi_driver(dacxx6x_driver);
+
+MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
+MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

[PATCH 2/2] dt-bindings: iio: dac: Add DAC8163

[Lukas Metz]

Add device tree binding for the Texas Instruments DAC8163 family
including the DAC7562, DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.

Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
---
 .../devicetree/bindings/iio/dac/ti,dac8163.yaml    | 75 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 76 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
new file mode 100644
index 000000000000..bb4bad389323
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/ti,dac8163.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments DAC8163 family of DACs
+
+description:
+  The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
+  dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
+  respectively. These devices include a 2.5-V, 4-ppm/°C internal
+  reference, giving a full-scale output voltage range of 2.5 V or 5 V.
+
+maintainers:
+  - Lukas Metz <lukas.metz@gmx.net>
+
+properties:
+  compatible:
+    enum:
+      - ti,dac7562
+      - ti,dac7563
+      - ti,dac8162
+      - ti,dac8163
+      - ti,dac8562
+      - ti,dac8563
+
+  reg:
+    maxItems: 1
+
+  ti,loaddacs-gpios:
+    description:
+      Pin needs to be asserted permanently when updating the DAC synchronously.
+    maxItems: 1
+
+  vref-supply:
+    description:
+      Reference voltage for scaling if an external reference is used.
+
+  ti,internal-ref:
+    type: boolean
+    description:
+      Flag if the internal reference is used (external otherwise).
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - if:
+      not:
+        required:
+          - ti,internal-ref
+    then:
+      required:
+        - vref-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@1 {
+            compatible = "ti,dac8163";
+            reg = <0x1>; /* CS1 */
+            ti,loaddacs-gpios = <&gpiog 8 GPIO_ACTIVE_HIGH>;
+            ti,internal-ref; /* internal reference used*/
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e82cc28e1bc3..5512f5eaab44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26398,6 +26398,7 @@ TI DAC8163 DAC DRIVER
 M:	Lukas Metz <lukas.metz@gmx.net>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
 F:	drivers/iio/dac/ti-dac8163.c
 
 TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER

-- 
2.43.0

Re: [PATCH 0/2] Add driver for DAC8163:

[Andy Shevchenko]

On Tue, Jun 23, 2026 at 06:07:26PM +0200, Lukas Metz wrote:
> This series adds an IIO driver for the Texas Instruments DAC7562, DAC7563,
> DAC8162, DAC8163, DAC8562, and DAC8563 dual-channel voltage-output DACs.
> 
> These devices are pin-compatible 12-, 14-, and 16-bit variants sharing the
> same 24-bit SPI command interface. Each device provides two independently
> addressable output channels and includes a 2.5 V, 4 ppm/°C internal
> reference that can be enabled via device tree, or an external reference
> supplied through a regulator. The register and command structure differs
> from already existing drivers which makes adding a new driver a
> reasonable choice in my opinion.
> 
> The driver supports:
>  - All six device variants via a shared chip info table
>  - DAC updates in synchronous mode
>  - Configurable internal or external voltage reference
>  - Optional LDAC GPIO which has to be asserted permanently when using
>    synchronous updates.
>  - IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE attributes per channel
> 
> Datasheet (DAC8163):
>   https://www.ti.com/lit/gpn/dac8163

Why do we need a brand new driver? Do we have an existing one that may be
expanded to support this HW? (Note, not all existing drivers are under IIO
folder, some of them might be found in hwmon, input, or drivers/misc.)

> The driver was tested with a DAC8163 on a custom STM32MP157F board with
> external reference enabled.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] Add driver for DAC8163:

[David Lechner]

On 6/23/26 1:35 PM, Andy Shevchenko wrote:
> On Tue, Jun 23, 2026 at 06:07:26PM +0200, Lukas Metz wrote:
>> This series adds an IIO driver for the Texas Instruments DAC7562, DAC7563,
>> DAC8162, DAC8163, DAC8562, and DAC8563 dual-channel voltage-output DACs.
>>
>> These devices are pin-compatible 12-, 14-, and 16-bit variants sharing the
>> same 24-bit SPI command interface. Each device provides two independently
>> addressable output channels and includes a 2.5 V, 4 ppm/°C internal
>> reference that can be enabled via device tree, or an external reference
>> supplied through a regulator. The register and command structure differs
>> from already existing drivers which makes adding a new driver a
>> reasonable choice in my opinion.
>>
>> The driver supports:
>>  - All six device variants via a shared chip info table
>>  - DAC updates in synchronous mode
>>  - Configurable internal or external voltage reference
>>  - Optional LDAC GPIO which has to be asserted permanently when using
>>    synchronous updates.
>>  - IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE attributes per channel
>>
>> Datasheet (DAC8163):
>>   https://www.ti.com/lit/gpn/dac8163
> 
> Why do we need a brand new driver? Do we have an existing one that may be
> expanded to support this HW? (Note, not all existing drivers are under IIO
> folder, some of them might be found in hwmon, input, or drivers/misc.)

I thought the statement above is clear that there are not any compatible
drivers already. And I would not expect a DAC to have a driver in hwmon
or input since it is an output device.

> 
>> The driver was tested with a DAC8163 on a custom STM32MP157F board with
>> external reference enabled.
> 

Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163

[Siratul Islam]

On Tue, 2026-06-23 at 18:07 +0200, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.
> 
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
>  MAINTAINERS                  |   6 +
>  drivers/iio/dac/Kconfig      |  10 ++
>  drivers/iio/dac/Makefile     |   1 +
>  drivers/iio/dac/ti-dac8163.c | 339 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 356 insertions(+)
Hi! I took a quick look, and probably missed a lot of stuff. But here are my thoughts.
> diff --git a/MAINTAINERS b/MAINTAINERS
...
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DACxx6x IIO driver (SPI)
> + */
A link to the datasheet here would be nice.
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/printk.h>
> +#include <linux/bitfield.h>
Sort the includes alphabetically. And include what you use. "mod_devicetable.h" is missing for example.
While at it, separate the core headers from "<linux/iio/iio.h>". add ir in a sepatate line.
> +
> +#define COMMAND_MASK GENMASK(6, 3)
> +#define ADDRESS_MASK GENMASK(2, 0)
> +
> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> +							FIELD_PREP(ADDRESS_MASK, (y)))
I'd align the FIELD_PREPs to make it look better. It may also fit in a single line.
> +
> +#define CMD_WRITE_INPUT_REG	0x0
> +#define CMD_UPDATE_DAC	0x1
> +#define CMD_WRITE_UPDATE_ALL	0x2
> +#define CMD_WRITE_UPDATE	0x3
> +#define CMD_SET_PWR_MODE		0x4
> +#define CMD_SOFT_RST			0x5
> +
> +#define CMD_LDAC_MODE		0x6
> +#define LDAC_MODE_CHANNEL_A_MASK BIT(0)
> +#define LDAC_MODE_CHANNEL_B_MASK BIT(1)
> +
> +#define CMD_SEL_REFERENCE	0x7
> +#define VOLTAGE_REFERENCE_MASK BIT(0)
> +
Group the CMD values together, also all these values would look better aligned.
> +enum dacxx6x_ldac_modes {
> +	LDAC_MODE_ACTIVE = 0,
> +	LDAC_MODE_INACTIVE = 1
> +};
A trailing comma would be nice.
> +
> +enum dacxx6x_voltage_reference {
> +	VOLTAGE_REFERENCE_EXTERNAL = 0,
> +	VOLTAGE_REFERENCE_INTERNAL = 1
> +};
Same here
> +
> +enum dacxx6x_supported_device_ids {
> +	ID_DAC7562,
> +	ID_DAC7563,
> +	ID_DAC8162,
> +	ID_DAC8163,
> +	ID_DAC8562,
> +	ID_DAC8563
> +};
> +
Here too.
> 
> +struct dacxx6x_state {
Since the filename is dac8163.c, how about naming the functions/structs/other symbols that as well instead of dacxx6x?
> +	struct spi_device *spi;
> +
How about use regmap?
> +	struct regulator *vref;
> +	struct gpio_desc *loaddacs;
> +
> +	bool internal_ref;
> +	int vref_uv;
> +
> +	unsigned int cached[2];
> +
> +	/*
> +	 * Lock to protect the state of the device from potential concurrent
> +	 * write accesses from userspace.
> +	 */
> +	struct mutex lock;
> +};
> +
> +struct dacxx6x_chip_info {
> +	const char *name;
> +	const struct iio_chan_spec channels[2];
> +};
> +
> +#define DACXX6X_CHAN(id, resolution)                                        \
> +	{                                                                   \
> +		.type = IIO_VOLTAGE, .channel = (id), .output = 1,          \
> +		.indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),       \
> +		.scan_type = { .realbits = (resolution),                    \
> +			       .shift = 16 - (resolution) },                \
> +	}
> +
> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
> +	[ID_DAC7562] = {
> +		.name = "dac7562",
> +		.channels = {
> +			DACXX6X_CHAN(0, 12),
> +			DACXX6X_CHAN(1, 12),
> +		}
> +	},
> +	[ID_DAC7563] = {
> +		.name = "dac7563",
> +		.channels = {
> +			DACXX6X_CHAN(0, 12),
> +			DACXX6X_CHAN(1, 12),
> +		}
> +	},
> +	[ID_DAC8162] = {
> +		.name = "dac8162",
> +		.channels = {
> +			DACXX6X_CHAN(0, 14),
> +			DACXX6X_CHAN(1, 14),
> +		}
> +	},
> +	[ID_DAC8163] = {
> +		.name = "dac8163",
> +		.channels = {
> +			DACXX6X_CHAN(0, 14),
> +			DACXX6X_CHAN(1, 14),
> +		}
> +	},
> +	[ID_DAC8562] = {
> +		.name = "dac8562",
> +		.channels = {
> +			DACXX6X_CHAN(0, 16),
> +			DACXX6X_CHAN(1, 16),
> +		}
> +	},
> +	[ID_DAC8563] = {
> +		.name = "dac8563",
> +		.channels = {
> +			DACXX6X_CHAN(0, 16),
> +			DACXX6X_CHAN(1, 16),
> +		}
> +	},
> +};
> +
> +static int dacxx6x_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct dacxx6x_state *st;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		st = iio_priv(indio_dev);
Could this be assigned  before the switch?
> +		mutex_lock(&st->lock);
> +		*val = st->cached[chan->channel];
> +		mutex_unlock(&st->lock);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		st = iio_priv(indio_dev);
> +		*val = st->vref_uv / MILLI; /* vref in mV */
> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> +			     unsigned int val)
> +{
> +	u8 tx[3];
> +
> +	tx[0] = COMMAND_SET(reg, addr);
> +	tx[1] = (val >> 8) & 0xff;
How about put_unaligned_be16?
> 
> +	tx[2] = val & 0xff;
> +
> +	return spi_write(st->spi, tx, sizeof(tx));
> +}
> +
> +static int dacxx6x_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int val,
> +			     int val2, long mask)
> +{
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->spi->dev;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);
> +		if (val2 != 0)
> +			return -EINVAL;
> +
> +		if (val < 0 || val >= BIT(chan->scan_type.realbits))
> +			return -EINVAL;
> +
> +		mutex_lock(&st->lock);
> +		int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
> +					    (unsigned int)val
> +						    << chan->scan_type.shift);
This case should be enclosed { }. Also, Use guard() from "cleanup.h" instead of manual mutex lock/unlock. Here and in
other places.
> +
> +		if (!ret)
> +			st->cached[chan->channel] = val;
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info dacxx6x_iio_info = {
> +	.write_raw = dacxx6x_write_raw,
> +	.read_raw = dacxx6x_read_raw
Trailing comma here
> +};
> +
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct dacxx6x_state *st;
> +	const struct dacxx6x_chip_info *info;
> +	int ret;
Sort these in a reverse christmas tree order.
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> +					       GPIOD_OUT_LOW);
Vendor prefixes are not needed here.
> +	if (IS_ERR(st->loaddacs))
> +		return PTR_ERR(st->loaddacs);
> +
> +	st->internal_ref =
> +		device_property_read_bool(&spi->dev, "ti,internal-ref");
> +
> +	if (!st->internal_ref) {
> +		st->vref = devm_regulator_get(&spi->dev, "vref");
> +		if (IS_ERR(st->vref))
> +			return PTR_ERR(st->vref);
Maybe use return dev_err_probe?
> +
> +		ret = regulator_enable(st->vref);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	mutex_init(&st->lock);
use devm_mutex_init.
> +
> +	if (st->internal_ref) {
> +		st->vref_uv = 2500000; /* 2.5V internal reference */
A note on where this value came from or why this was chosen, or a reference to datasheet would be better.
> +	} else {
> +		st->vref_uv = regulator_get_voltage(st->vref);
> +		if (st->vref_uv < 0) {
> +			ret = st->vref_uv;
> +			goto err;
> +		}
> +	}
> +
You have a CMD_SOFT_RST defined but not used. Should this be used to reset before doing any configuration?
> +	gpiod_set_value(st->loaddacs, 0);
> +
> +	ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> +				FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> +				FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> +				FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	info = spi_get_device_match_data(spi);
> +
> +	indio_dev->name = info->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &dacxx6x_iio_info;
> +	indio_dev->channels = info->channels;
> +	indio_dev->num_channels = 2;
use ARRAY_SIZE(info->channels) and include linux/array_size.h
> +
> +	ret = iio_device_register(indio_dev);
Use devm_iio_device_register
> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);
> +	mutex_destroy(&st->lock);
> +	return ret;
> +}
> +
> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
Using devm would help here too. No need to unregister manually
> +	mutex_destroy(&st->lock);
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);
> +}
> +
> +#define DACXX6X_COMPATIBLE(of_compatible, id)        \
> +	{                                            \
> +		.compatible = of_compatible,         \
> +		.data = &dacxx6x_chip_info_table[id] \
> +	}
> +
> +static const struct of_device_id dacxx6x_of_match[] = {
> +	DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> +	DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> +	DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> +	DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> +	DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> +	DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> +	{}
{} should have a space "{ }"
> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> +	{ "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> +	{ "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> +	{ "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> +	{ "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> +	{ "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> +	{ "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> +	{}
Same here
> +};
> +
> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> +	.driver = {
> +		.name = "ti-dacxx6x",
Name doesn't need vendor prefix.
> +		.of_match_table = dacxx6x_of_match,
> +	},
> +	.probe = dacxx6x_probe,
> +	.remove = dacxx6x_remove,
> +	.id_table = dacxx6x_id_table,
> +};
> +
No space here. 
> +module_spi_driver(dacxx6x_driver);
> +
> +MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
> +MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
> +MODULE_LICENSE("GPL");

Thanks
Sirat

Re: [PATCH 2/2] dt-bindings: iio: dac: Add DAC8163

[David Lechner]

It is more logical to put the dt-bindings patch first in the series
before the driver that makes use of it.

On 6/23/26 11:07 AM, Lukas Metz wrote:
> Add device tree binding for the Texas Instruments DAC8163 family
> including the DAC7562, DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.
> 
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
>  .../devicetree/bindings/iio/dac/ti,dac8163.yaml    | 75 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> new file mode 100644
> index 000000000000..bb4bad389323
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/ti,dac8163.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments DAC8163 family of DACs
> +
> +description:
> +  The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> +  dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> +  respectively. These devices include a 2.5-V, 4-ppm/°C internal
> +  reference, giving a full-scale output voltage range of 2.5 V or 5 V.
> +
> +maintainers:
> +  - Lukas Metz <lukas.metz@gmx.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,dac7562
> +      - ti,dac7563
> +      - ti,dac8162
> +      - ti,dac8163
> +      - ti,dac8562
> +      - ti,dac8563
> +
> +  reg:
> +    maxItems: 1
> +

There are a couple of more SPI properties needed since this is not a "normal"
SPI device. We can only write and not read because there is no D_OUT pin. So

spi-rx-bus-width:
  items:
    - const: 0

will describe this.

There is also no chip select, but now that I am looking, there doesn't seem to
be anything to describe this.

> +  ti,loaddacs-gpios:

This should match the actual pin name. And we don't need vendor prefix on gpios.
So `ldac-gpios`.

We also want the binding to be complete even if the driver doesn't all of it, so
`clear-gpios` and `sync-gpios` probably make sense too.

Usually for SPI devices we also add:

spi-max-frequency:
  maximum: ...

> +    description:
> +      Pin needs to be asserted permanently when updating the DAC synchronously.
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      Reference voltage for scaling if an external reference is used.

I prefer to use the actual pin name, i.e. vrefin-supply.

And for the description, it is the actual external reference voltage
supply, not just the voltage.

And we also need `avdd-supply: true`.

> +
> +  ti,internal-ref:
> +    type: boolean
> +    description:
> +      Flag if the internal reference is used (external otherwise).

Usually, we don't bother with a property like this since it is redundant.
If an external reference supply is given, then it gets used, otherwise
the internal reference is used.

> +
> +required:
> +  - compatible
> +  - reg

avdd-supply will be required too

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      not:
> +        required:
> +          - ti,internal-ref
> +    then:
> +      required:
> +        - vref-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@1 {
> +            compatible = "ti,dac8163";
> +            reg = <0x1>; /* CS1 */

These chips don't appear to have a chip select pin, so this comment
doesn't make sense to me. More logical would be to just use dac@0
and reg = <0>; since it should just be ignored.

> +            ti,loaddacs-gpios = <&gpiog 8 GPIO_ACTIVE_HIGH>;

The pin is marked active low in the datasheet, so I would expect
this to be GPIO_ACTIVE_LOW.

> +            ti,internal-ref; /* internal reference used*/
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e82cc28e1bc3..5512f5eaab44 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26398,6 +26398,7 @@ TI DAC8163 DAC DRIVER
>  M:	Lukas Metz <lukas.metz@gmx.net>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
>  F:	drivers/iio/dac/ti-dac8163.c
>  
>  TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
> 

Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163

[Andy Shevchenko]

On Tue, Jun 23, 2026 at 06:07:27PM +0200, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.

> +config TI_DAC8163
> +	tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Driver for the Texas Instruments digital-to-analog converter
> +	  family dacxx6x compatible with the variants DAC7562,
> +	  DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.

This list doesn't scale. Put one part number per line as
	  - DAC7... (channel bits)
	  - DAC8... (...)
	  - ...

> +	  If compiled as a module, it will be called ti-dac8163.

...

> +#include <linux/module.h>
> +#include <linux/spi/spi.h>

> +#include <linux/of.h>

No, for this header in new drivers. Can't we use device and/or fwnode property
APIs instead?

> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>

> +#include <linux/printk.h>

? Usually one wants dev_printk.h.

> +#include <linux/bitfield.h>

Keep the list sorted, and also follow the IWYU principle.

...

> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> +							FIELD_PREP(ADDRESS_MASK, (y)))

This is ugly indentation. Compare to

#define COMMAND_SET(x, y)			\
	(FIELD_PREP(COMMAND_MASK, (x)) | FIELD_PREP(ADDRESS_MASK, (y)))

...

> +enum dacxx6x_ldac_modes {
> +	LDAC_MODE_ACTIVE = 0,
> +	LDAC_MODE_INACTIVE = 1

Leave trailing commas in the non-terminator entries here and there.

> +};

...

> +enum dacxx6x_supported_device_ids {
> +	ID_DAC7562,
> +	ID_DAC7563,
> +	ID_DAC8162,
> +	ID_DAC8163,
> +	ID_DAC8562,
> +	ID_DAC8563
> +};

This is solely used to make indexed array with chip_info. Instead kill this
enum and use separate structures.

...

> +struct dacxx6x_state {
> +	struct spi_device *spi;

Why not regmap?

> +	struct regulator *vref;
> +	struct gpio_desc *loaddacs;
> +
> +	bool internal_ref;
> +	int vref_uv;

_uV

> +	unsigned int cached[2];
> +
> +	/*
> +	 * Lock to protect the state of the device from potential concurrent
> +	 * write accesses from userspace.
> +	 */
> +	struct mutex lock;
> +};

...

> +#define DACXX6X_CHAN(id, resolution)                                        \
> +	{                                                                   \
> +		.type = IIO_VOLTAGE, .channel = (id), .output = 1,          \
> +		.indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \

Do not put two or more things on the same line, it's unreadable.

> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),       \

> +		.scan_type = { .realbits = (resolution),                    \
> +			       .shift = 16 - (resolution) },                \

Make these to be 4 lines.

> +	}

...

> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {

Drop the number in the square brackets, let compiler do that job. But see
above, this has to be just 6 different data structures.

> +};

...

> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> +			     unsigned int val)
> +{
> +	u8 tx[3];

Are you sure about this? How would it work with DMA?

> +	tx[0] = COMMAND_SET(reg, addr);
> +	tx[1] = (val >> 8) & 0xff;
> +	tx[2] = val & 0xff;

Use proper unaligned setters: put_unaligned_be16() from linux/unaligned.h.

> +	return spi_write(st->spi, tx, sizeof(tx));
> +}

...

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

Split logically:

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

> +{
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->spi->dev;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:

> +		dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);

No. Is it RFC? PoC? Or production-ready? If not the latter, come when it will
be production-ready.

> +		if (val2 != 0)
> +			return -EINVAL;

> +		if (val < 0 || val >= BIT(chan->scan_type.realbits))
> +			return -EINVAL;

Why not ERANGE or something like this?

> +
> +		mutex_lock(&st->lock);

Why not guard()()?

> +		int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,
> +					    (unsigned int)val
> +						    << chan->scan_type.shift);

Awful indentation. Please, check

> +		if (!ret)

No, use regular pattern: Check for error first.

> +			st->cached[chan->channel] = val;
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct dacxx6x_state *st;
> +	const struct dacxx6x_chip_info *info;
> +	int ret;

Reversed xmas tree order, please.

> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;

> +	spi_set_drvdata(spi, indio_dev);

Is it being used?

> +	st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> +					       GPIOD_OUT_LOW);

In the above you used temporary variable for struct device, why not here?

> +	if (IS_ERR(st->loaddacs))
> +		return PTR_ERR(st->loaddacs);
> +
> +	st->internal_ref =
> +		device_property_read_bool(&spi->dev, "ti,internal-ref");

And here (and it becomes a single line as well).

> +	if (!st->internal_ref) {
> +		st->vref = devm_regulator_get(&spi->dev, "vref");
> +		if (IS_ERR(st->vref))
> +			return PTR_ERR(st->vref);
> +
> +		ret = regulator_enable(st->vref);
> +		if (ret < 0)
> +			return ret;
> +	}

Can't you get just voltage and be done with it for now?

> +	mutex_init(&st->lock);

devm_mutex_init.

> +	if (st->internal_ref) {
> +		st->vref_uv = 2500000; /* 2.5V internal reference */
> +	} else {
> +		st->vref_uv = regulator_get_voltage(st->vref);
> +		if (st->vref_uv < 0) {
> +			ret = st->vref_uv;
> +			goto err;
> +		}
> +	}
> +
> +	gpiod_set_value(st->loaddacs, 0);
> +
> +	ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> +				FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> +				FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));

> +

Stray blank line.

> +	if (ret < 0)
> +		goto err;
> +
> +	ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> +				FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +

Ditto.

> +	if (ret < 0)
> +		goto err;
> +
> +	info = spi_get_device_match_data(spi);

We need to check for NULL here due to driver_override.

> +	indio_dev->name = info->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &dacxx6x_iio_info;
> +	indio_dev->channels = info->channels;
> +	indio_dev->num_channels = 2;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto err;
> +
> +	return 0;

> +err:
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);
> +	mutex_destroy(&st->lock);
> +	return ret;
> +}

...

> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	mutex_destroy(&st->lock);
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);
> +}

Finish devm conversion and drop this function.

...

> +#define DACXX6X_COMPATIBLE(of_compatible, id)        \
> +	{                                            \
> +		.compatible = of_compatible,         \
> +		.data = &dacxx6x_chip_info_table[id] \
> +	}

No, just use directly, so drop this macro.

> +static const struct of_device_id dacxx6x_of_match[] = {
> +	DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> +	DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> +	DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> +	DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> +	DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> +	DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> +	{ "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> +	{ "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> +	{ "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> +	{ "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> +	{ "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> +	{ "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> +	{}
> +};

> +

Unneeded blank line.

> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> +	.driver = {
> +		.name = "ti-dacxx6x",
> +		.of_match_table = dacxx6x_of_match,
> +	},
> +	.probe = dacxx6x_probe,
> +	.remove = dacxx6x_remove,
> +	.id_table = dacxx6x_id_table,
> +};

> +

Ditto.

> +module_spi_driver(dacxx6x_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/2] Add driver for DAC8163:

[Andy Shevchenko]

On Tue, Jun 23, 2026 at 01:50:27PM -0500, David Lechner wrote:
> On 6/23/26 1:35 PM, Andy Shevchenko wrote:
> > On Tue, Jun 23, 2026 at 06:07:26PM +0200, Lukas Metz wrote:
> >> This series adds an IIO driver for the Texas Instruments DAC7562, DAC7563,
> >> DAC8162, DAC8163, DAC8562, and DAC8563 dual-channel voltage-output DACs.
> >>
> >> These devices are pin-compatible 12-, 14-, and 16-bit variants sharing the
> >> same 24-bit SPI command interface. Each device provides two independently
> >> addressable output channels and includes a 2.5 V, 4 ppm/°C internal
> >> reference that can be enabled via device tree, or an external reference
> >> supplied through a regulator. The register and command structure differs
> >> from already existing drivers which makes adding a new driver a
> >> reasonable choice in my opinion.
> >>
> >> The driver supports:
> >>  - All six device variants via a shared chip info table
> >>  - DAC updates in synchronous mode
> >>  - Configurable internal or external voltage reference
> >>  - Optional LDAC GPIO which has to be asserted permanently when using
> >>    synchronous updates.
> >>  - IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE attributes per channel
> >>
> >> Datasheet (DAC8163):
> >>   https://www.ti.com/lit/gpn/dac8163
> > 
> > Why do we need a brand new driver? Do we have an existing one that may be
> > expanded to support this HW? (Note, not all existing drivers are under IIO
> > folder, some of them might be found in hwmon, input, or drivers/misc.)
> 
> I thought the statement above is clear that there are not any compatible
> drivers already. And I would not expect a DAC to have a driver in hwmon
> or input since it is an output device.

Ah, my bad, I haven't read carefully. Fair point, I see the answer is already
here.

> >> The driver was tested with a DAC8163 on a custom STM32MP157F board with
> >> external reference enabled.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] iio: dac: dac8163: Add driver for DAC8163

[David Lechner]

On 6/23/26 11:07 AM, Lukas Metz wrote:
> The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> respectively. These devices include a 2.5-V, 4-ppm/°C internal
> reference, giving a full-scale output voltage range of 2.5 V or 5 V.

Nice and simple driver. Mostly just needs better alignment with usual
IIO conventions.

> 
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
>  MAINTAINERS                  |   6 +
>  drivers/iio/dac/Kconfig      |  10 ++
>  drivers/iio/dac/Makefile     |   1 +
>  drivers/iio/dac/ti-dac8163.c | 339 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 356 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d238590a31f2..e82cc28e1bc3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26394,6 +26394,12 @@ S:	Odd Fixes
>  F:	drivers/clk/ti/
>  F:	include/linux/clk/ti.h
>  
> +TI DAC8163 DAC DRIVER
> +M:	Lukas Metz <lukas.metz@gmx.net>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	drivers/iio/dac/ti-dac8163.c
> +
>  TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
>  M:	T Pratham <t-pratham@ti.com>
>  L:	linux-crypto@vger.kernel.org
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index db9f5c711b3d..6b6e5ee0732a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -632,6 +632,16 @@ config TI_DAC7612
>  
>  	  If compiled as a module, it will be called ti-dac7612.
>  
> +config TI_DAC8163
> +	tristate "Texas Instruments 12/14/16-bit 2-channel DAC driver"
> +	depends on SPI_MASTER
> +	help
> +	  Driver for the Texas Instruments digital-to-analog converter
> +	  family dacxx6x compatible with the variants DAC7562,
> +	  DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.
> +
> +	  If compiled as a module, it will be called ti-dac8163.
> +
>  config VF610_DAC
>  	tristate "Vybrid vf610 DAC driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 2a80bbf4e80a..359cde446623 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -62,4 +62,5 @@ obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
>  obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
>  obj-$(CONFIG_TI_DAC7311) += ti-dac7311.o
>  obj-$(CONFIG_TI_DAC7612) += ti-dac7612.o
> +obj-$(CONFIG_TI_DAC8163) += ti-dac8163.o
>  obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
> new file mode 100644
> index 000000000000..84a9dfb5347d
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac8163.c
> @@ -0,0 +1,339 @@
> +// SPDX-License-Identifier: GPL-2.0

Prefer GPL-2.0-only or GPL-2.0-or-later.

> +/*
> + * DACxx6x IIO driver (SPI)
> + */
> +
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/iio/iio.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/printk.h>
> +#include <linux/bitfield.h>
> +
> +#define COMMAND_MASK GENMASK(6, 3)
> +#define ADDRESS_MASK GENMASK(2, 0)
> +
> +#define COMMAND_SET(x, y) (FIELD_PREP(COMMAND_MASK, (x)) | \
> +							FIELD_PREP(ADDRESS_MASK, (y)))

Usually, we try to avoid putting FIELD_PREP() in macros. This
is only used once in the code anyway, so dropping the macro
will actually save a line or two.

> +
> +#define CMD_WRITE_INPUT_REG	0x0
> +#define CMD_UPDATE_DAC	0x1
> +#define CMD_WRITE_UPDATE_ALL	0x2
> +#define CMD_WRITE_UPDATE	0x3
> +#define CMD_SET_PWR_MODE		0x4
> +#define CMD_SOFT_RST			0x5
> +
> +#define CMD_LDAC_MODE		0x6
> +#define LDAC_MODE_CHANNEL_A_MASK BIT(0)
> +#define LDAC_MODE_CHANNEL_B_MASK BIT(1)
> +
> +#define CMD_SEL_REFERENCE	0x7
> +#define VOLTAGE_REFERENCE_MASK BIT(0)
> +
> +enum dacxx6x_ldac_modes {

In IIO, we always avoid putting xx in identifier names. Just use dac8163
everwhere instead since that is the name of the driver.

> +	LDAC_MODE_ACTIVE = 0,
> +	LDAC_MODE_INACTIVE = 1
> +};
> +
> +enum dacxx6x_voltage_reference {
> +	VOLTAGE_REFERENCE_EXTERNAL = 0,
> +	VOLTAGE_REFERENCE_INTERNAL = 1
> +};
> +
> +enum dacxx6x_supported_device_ids {
> +	ID_DAC7562,
> +	ID_DAC7563,
> +	ID_DAC8162,
> +	ID_DAC8163,
> +	ID_DAC8562,
> +	ID_DAC8563
> +};
> +
> +struct dacxx6x_state {
> +	struct spi_device *spi;
> +
> +	struct regulator *vref;

This is 

> +	struct gpio_desc *loaddacs;

LDAC is very common in DACs, so I would call this ldac_gpio. Also, it isn't
currently used outside of probe, so we don't really need it here.

> +
> +	bool internal_ref;
> +	int vref_uv;

As in a later comment, we should be able to drop these as well if we can
get rid of the remove() callback.

> +
> +	unsigned int cached[2];

Can we get a more descriptive name for this? Is it raw value?

> +
> +	/*
> +	 * Lock to protect the state of the device from potential concurrent
> +	 * write accesses from userspace.
> +	 */
> +	struct mutex lock;
> +};
> +
> +struct dacxx6x_chip_info {
> +	const char *name;
> +	const struct iio_chan_spec channels[2];
> +};
> +
> +#define DACXX6X_CHAN(id, resolution)                                        \
> +	{                                                                   \
> +		.type = IIO_VOLTAGE, .channel = (id), .output = 1,          \

Please put each field on a new line.

> +		.indexed = 1, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),       \
> +		.scan_type = { .realbits = (resolution),                    \
> +			       .shift = 16 - (resolution) },                \
> +	}
> +
> +static const struct dacxx6x_chip_info dacxx6x_chip_info_table[6] = {
> +	[ID_DAC7562] = {
> +		.name = "dac7562",
> +		.channels = {
> +			DACXX6X_CHAN(0, 12),
> +			DACXX6X_CHAN(1, 12),
> +		}
> +	},
> +	[ID_DAC7563] = {
> +		.name = "dac7563",
> +		.channels = {
> +			DACXX6X_CHAN(0, 12),
> +			DACXX6X_CHAN(1, 12),
> +		}
> +	},
> +	[ID_DAC8162] = {
> +		.name = "dac8162",
> +		.channels = {
> +			DACXX6X_CHAN(0, 14),
> +			DACXX6X_CHAN(1, 14),
> +		}
> +	},
> +	[ID_DAC8163] = {
> +		.name = "dac8163",
> +		.channels = {
> +			DACXX6X_CHAN(0, 14),
> +			DACXX6X_CHAN(1, 14),
> +		}
> +	},
> +	[ID_DAC8562] = {
> +		.name = "dac8562",
> +		.channels = {
> +			DACXX6X_CHAN(0, 16),
> +			DACXX6X_CHAN(1, 16),
> +		}
> +	},
> +	[ID_DAC8563] = {
> +		.name = "dac8563",
> +		.channels = {
> +			DACXX6X_CHAN(0, 16),
> +			DACXX6X_CHAN(1, 16),
> +		}
> +	},
> +};

We are trying to get rid of arrays like this in drivers. We can just make
individual structs instead.

> +
> +static int dacxx6x_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct dacxx6x_state *st;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		st = iio_priv(indio_dev);

This can be moved out of the case statement so we don't have to
repeat it each time.

> +		mutex_lock(&st->lock);
> +		*val = st->cached[chan->channel];
> +		mutex_unlock(&st->lock);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		st = iio_priv(indio_dev);
> +		*val = st->vref_uv / MILLI; /* vref in mV */

We've been writing this like:

		*val = st->vref_uV / (MICRO/ MILLI);

Then we don't really need a comment.

> +		*val2 = chan->scan_type.realbits;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int dacxx6x_write_reg(struct dacxx6x_state *st, int reg, int addr,
> +			     unsigned int val)
> +{
> +	u8 tx[3];
> +
> +	tx[0] = COMMAND_SET(reg, addr);
> +	tx[1] = (val >> 8) & 0xff;
> +	tx[2] = val & 0xff;
> +
> +	return spi_write(st->spi, tx, sizeof(tx));
> +}
> +
> +static int dacxx6x_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int val,
> +			     int val2, long mask)
> +{
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +	struct device *dev = &st->spi->dev;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		dev_dbg(dev, "%s: val=%d val2=%d\n", __func__, val, val2);

Do we really need to keep this debug print?

> +		if (val2 != 0)
> +			return -EINVAL;
> +
> +		if (val < 0 || val >= BIT(chan->scan_type.realbits))
> +			return -EINVAL;
> +
> +		mutex_lock(&st->lock);
> +		int ret = dacxx6x_write_reg(st, CMD_WRITE_UPDATE, chan->channel,

Usually, we would declare `int ret;` at the top of the function.

> +					    (unsigned int)val

Using u32 type instead of unsigned int would make this probably fit
on one line too.

> +						    << chan->scan_type.shift);
> +
> +		if (!ret)
> +			st->cached[chan->channel] = val;
> +		mutex_unlock(&st->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info dacxx6x_iio_info = {
> +	.write_raw = dacxx6x_write_raw,
> +	.read_raw = dacxx6x_read_raw
> +};
> +
> +static int dacxx6x_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct dacxx6x_state *st;
> +	const struct dacxx6x_chip_info *info;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	st->loaddacs = devm_gpiod_get_optional(&spi->dev, "ti,loaddacs",
> +					       GPIOD_OUT_LOW);

As in the dt-bindings, we would expect the bindings to be active low
so this would be GPIOD_OUT_HIGH to assert the LDAC signal.

Could also use a comment to say that for now we are just holding this
asserted so that individual outputs are updated when we each raw attribute.

> +	if (IS_ERR(st->loaddacs))
> +		return PTR_ERR(st->loaddacs);
> +
> +	st->internal_ref =
> +		device_property_read_bool(&spi->dev, "ti,internal-ref");
> +
> +	if (!st->internal_ref) {
> +		st->vref = devm_regulator_get(&spi->dev, "vref");
> +		if (IS_ERR(st->vref))
> +			return PTR_ERR(st->vref);
> +
> +		ret = regulator_enable(st->vref);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	mutex_init(&st->lock);

devm_mutex_init(). Also should not be in the middle of regulator code.

> +
> +	if (st->internal_ref) {
> +		st->vref_uv = 2500000; /* 2.5V internal reference */
> +	} else {
> +		st->vref_uv = regulator_get_voltage(st->vref);
> +		if (st->vref_uv < 0) {
> +			ret = st->vref_uv;
> +			goto err;
> +		}
> +	}

The way we've been doing optional reference voltage lately is like this:

	if (device_property_present(dev, "refin-supply")) {
		ret = devm_regulator_get_enable_read_voltage(dev, "refin");
		if (ret < 0)
			return ret;

		st->vref_mV = ret / (MICRO / MILLI);
	} else {
		st->vref_mV = DAC8163_INTERNAL_REF_mV;
	}

This avoids the need for the ti,internal-ref DT property, avoid the need to convert
uV to mV later and the macro for the internal reference makes it self-documenting
so we don't need a comment. And it automacially cleans up after itself.

> +
> +	gpiod_set_value(st->loaddacs, 0);

devm_gpiod_get_optional() already set this, so this is redundant.

> +
> +	ret = dacxx6x_write_reg(st, CMD_LDAC_MODE, 0,
> +				FIELD_PREP(LDAC_MODE_CHANNEL_A_MASK, LDAC_MODE_INACTIVE) |
> +				FIELD_PREP(LDAC_MODE_CHANNEL_B_MASK, LDAC_MODE_INACTIVE));
> +
> +	if (ret < 0)
> +		goto err;
> +
> +	ret = dacxx6x_write_reg(st, CMD_SEL_REFERENCE, 0,
> +				FIELD_PREP(VOLTAGE_REFERENCE_MASK, st->internal_ref));
> +

Some of these lines are getting a bit long. We try to stick to close
to 80 columns in IIO when we can.

> +	if (ret < 0)
> +		goto err;
> +
> +	info = spi_get_device_match_data(spi);

	if (!info)
		return -EINVAL;

> +
> +	indio_dev->name = info->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &dacxx6x_iio_info;
> +	indio_dev->channels = info->channels;
> +	indio_dev->num_channels = 2;
> +
> +	ret = iio_device_register(indio_dev);

	return devm_iio_device_register();

> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);
> +	mutex_destroy(&st->lock);
> +	return ret;
> +}
> +
> +static void dacxx6x_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct dacxx6x_state *st = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	mutex_destroy(&st->lock);
> +	if (!st->internal_ref)
> +		regulator_disable(st->vref);

We can use devm_* functions and avoid the need for the remove callback.

> +}
> +
> +#define DACXX6X_COMPATIBLE(of_compatible, id)        \
> +	{                                            \
> +		.compatible = of_compatible,         \
> +		.data = &dacxx6x_chip_info_table[id] \
> +	}
> +
> +static const struct of_device_id dacxx6x_of_match[] = {
> +	DACXX6X_COMPATIBLE("ti,dac7562", ID_DAC7562),
> +	DACXX6X_COMPATIBLE("ti,dac7563", ID_DAC7563),
> +	DACXX6X_COMPATIBLE("ti,dac8162", ID_DAC8162),
> +	DACXX6X_COMPATIBLE("ti,dac8163", ID_DAC8163),
> +	DACXX6X_COMPATIBLE("ti,dac8562", ID_DAC8562),
> +	DACXX6X_COMPATIBLE("ti,dac8563", ID_DAC8563),
> +	{}

IIO style is `{ }` (with space inbetween braces)

> +};
> +MODULE_DEVICE_TABLE(of, dacxx6x_of_match);
> +
> +static const struct spi_device_id dacxx6x_id_table[] = {
> +	{ "dac7562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7562] },
> +	{ "dac7563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC7563] },
> +	{ "dac8162", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8162] },
> +	{ "dac8163", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8163] },
> +	{ "dac8562", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8562] },
> +	{ "dac8563", (kernel_ulong_t)&dacxx6x_chip_info_table[ID_DAC8563] },
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(spi, dacxx6x_id_table);
> +
> +static struct spi_driver dacxx6x_driver = {
> +	.driver = {
> +		.name = "ti-dacxx6x",
> +		.of_match_table = dacxx6x_of_match,
> +	},
> +	.probe = dacxx6x_probe,
> +	.remove = dacxx6x_remove,
> +	.id_table = dacxx6x_id_table,
> +};
> +
> +module_spi_driver(dacxx6x_driver);
> +
> +MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
> +MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 2/2] dt-bindings: iio: dac: Add DAC8163

[David Lechner]

On 6/23/26 11:07 AM, Lukas Metz wrote:
> Add device tree binding for the Texas Instruments DAC8163 family
> including the DAC7562, DAC7563, DAC8162, DAC8163, DAC8562 and DAC8563.

One other thing worth mentioning here as to why none of these are fallback
compatible (in addition to the resolution bits) is that the xxx2 chips
and xxx3 chips have a different output state when CLR is asserted.

> 
> Signed-off-by: Lukas Metz <lukas.metz@gmx.net>
> ---
>  .../devicetree/bindings/iio/dac/ti,dac8163.yaml    | 75 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> new file mode 100644
> index 000000000000..bb4bad389323
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/ti,dac8163.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments DAC8163 family of DACs
> +
> +description:
> +  The DAC756x, DAC816x, and DAC856x devices are low-power, voltage-output,
> +  dual-channel, 12-, 14-, and 16-bit digital-to-analog converters (DACs),
> +  respectively. These devices include a 2.5-V, 4-ppm/°C internal
> +  reference, giving a full-scale output voltage range of 2.5 V or 5 V.
> +
> +maintainers:
> +  - Lukas Metz <lukas.metz@gmx.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,dac7562
> +      - ti,dac7563
> +      - ti,dac8162
> +      - ti,dac8163
> +      - ti,dac8562
> +      - ti,dac8563
> +