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

[PATCH v2 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>
---
Changes in v2:
- add reg_defaults to initialize the output registers based on
  the device (dacxxx2 resets to 0, dacxxx3 resets to mid-scale)
- add variable gain to dac8163_state struct to report correct scale when
  internal reference is used (the device sets the gain register to 2
  when the internal reference is selected)
- do a full reset of the device when probing to ensure we have a known
  device state as pointed out by Jonathan Cameron
- use devm* calls where possible and omit remove callback
- make use of regmap and remove now unneeded things like manual locking
  and caching of output register values (thanks to Andy Shevchenko,
  David Lechner and others for the hint)
- remove internal_reference property
- add missing headers as suggested by Siratul Islam
- remove debug print from dac8163_write_raw (per Andy Shevchenko and
  David Lechner)
- switch ldac-gpio polarity for consistency with datasheet (as per David
  Lechner)
- rename dt properties ti,loaddacs-gpios to ldac-gpios and vref-supply to
  vrefin-supply
- add required spi property spi-cpha to binding (the device only works
  in spi mode 1 and 3)
- add missing spi properties spi-max-frequency and spi-rx-bus-width to
  binding
- add avdd-supply to binding and enable it in the driver (thanks to
  David Lechner)
- add clr-gpios to binding although not yet supported by the driver
- add nullptr check after calling spi_get_device_match_data (thanks to
  Andy Shevchenko for pointing this out)
- add comment for permanent ldac assertion
- switch to GPL-2.0-or-later
- return -ERANGE when raw write is out of range
- use named structures instead of chip_info array
- rename driver identifiers from dacxx6x to dac8163 and partly remove
  vendor prefix as suggested by David Lechner and Siratul Islam
- remove redundant call to gpiod_set_value
- Link to v1: https://patch.msgid.link/20260623-dac8163-work-v1-0-5b508158faa0@gmx.net

---
Lukas Metz (2):
      dt-bindings: iio: dac: Add DAC8163
      iio: dac: dac8163: Add driver for DAC8163

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

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

[PATCH v2 1/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    | 85 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 91 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..533cc7327c2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
@@ -0,0 +1,85 @@
+# 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.
+  The sync pin on the device acts as chip-select pin.
+
+maintainers:
+  - Lukas Metz <lukas.metz@gmx.net>
+
+properties:
+  compatible:
+    enum:
+      - ti,dac7562
+      - ti,dac7563
+      - ti,dac8162
+      - ti,dac8163
+      - ti,dac8562
+      - ti,dac8563
+
+  spi-rx-bus-width:
+    items:
+      - const: 0
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  spi-cpha: true
+
+  reg:
+    maxItems: 1
+
+  avdd-supply: true
+
+  clr-gpios:
+    description:
+      The device registers are reset when this pin is asserted. The output is
+      set to zero (xxx2 devices) or mid-scale (xxx3 devices).
+    maxItems: 1
+
+  ldac-gpios:
+    description:
+      Pin needs to be asserted permanently when updating the DAC synchronously.
+    maxItems: 1
+
+  vrefin-supply:
+    description:
+      External reference voltage supply for scaling. When no vrefin-supply
+      is present the internal voltage reference is used.
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - avdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        dac@1 {
+            compatible = "ti,dac8163";
+            reg = <0x1>; /* CS1 */
+            spi-cpha;
+            ldac-gpios = <&gpiog 8 GPIO_ACTIVE_LOW>;
+            avdd-supply = <&avdd_3v3>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d238590a31f2..314f235332f5 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:	Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
+
 TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
 M:	T Pratham <t-pratham@ti.com>
 L:	linux-crypto@vger.kernel.org

-- 
2.43.0

[PATCH v2 2/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                  |   1 +
 drivers/iio/dac/Kconfig      |  15 ++
 drivers/iio/dac/Makefile     |   1 +
 drivers/iio/dac/ti-dac8163.c | 373 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 390 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 314f235332f5..5512f5eaab44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26399,6 +26399,7 @@ 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
 M:	T Pratham <t-pratham@ti.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index db9f5c711b3d..de3b8fa5ffbf 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -632,6 +632,21 @@ 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 following variants
+	  - DAC7562 (2 channels, 12 bits, resets to zero)
+	  - DAC7563 (2 channels, 12 bits, resets to mid-scale)
+	  - DAC8162 (2 channels, 14 bits, resets to zero)
+	  - DAC8163 (2 channels, 14 bits, resets to mid-scale)
+	  - DAC8562 (2 channels, 16 bits, resets to zero)
+	  - DAC8563 (2 channels, 16 bits, resets to mid-scale)
+
+	  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..6be0aac2e875
--- /dev/null
+++ b/drivers/iio/dac/ti-dac8163.c
@@ -0,0 +1,373 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * DAC8163 IIO driver (SPI)
+ * https://www.ti.com/de/lit/gpn/dac8163
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/stddef.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#define COMMAND_MASK GENMASK(6, 3)
+#define ADDRESS_MASK GENMASK(2, 0)
+
+#define CMD_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_POWER_MODE		0x4
+#define CMD_SOFT_RST		0x5
+#define CMD_LDAC_MODE		0x6
+#define CMD_REF			0x7
+
+#define LDAC_CHANNEL_A_MASK BIT(0)
+#define LDAC_CHANNEL_B_MASK BIT(1)
+#define VREF_MASK BIT(0)
+
+#define DAC8163_INTERNAL_REF_mV 2500
+
+enum dac8163_reset_types {
+	OUTPUT_ONLY_RESET = 0,
+	FULL_RESET = 1,
+};
+
+enum dac8163_ldac_modes {
+	LDAC_ACTIVE = 0,
+	LDAC_INACTIVE = 1,
+};
+
+enum dac8163_voltage_reference {
+	VREF_EXTERNAL = 0,
+	VREF_INTERNAL = 1,
+};
+
+struct dac8163_state {
+	struct regmap *regmap;
+	struct regulator *vref;
+
+	int vref_mV;
+	int gain;
+};
+
+struct dac8163_chip_info {
+	const char *name;
+	const struct iio_chan_spec channels[2];
+	u16 default_output_reg;
+};
+
+#define DAC8163_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)			\
+		},							\
+	}
+
+#define DAC8163_MID_SCALE(resolution) \
+	(BIT((resolution) - 1) << (16 - (resolution)))
+
+static const struct dac8163_chip_info dac7562_chip_info = {
+	.name = "dac7562",
+	.channels = {
+		DAC8163_CHAN(0, 12),
+		DAC8163_CHAN(1, 12),
+	},
+	.default_output_reg = 0,
+};
+
+static const struct dac8163_chip_info dac7563_chip_info = {
+	.name = "dac7563",
+	.channels = {
+		DAC8163_CHAN(0, 12),
+		DAC8163_CHAN(1, 12),
+	},
+	.default_output_reg = DAC8163_MID_SCALE(12),
+};
+
+static const struct dac8163_chip_info dac8162_chip_info = {
+	.name = "dac8162",
+	.channels = {
+		DAC8163_CHAN(0, 14),
+		DAC8163_CHAN(1, 14),
+	},
+	.default_output_reg = 0,
+};
+
+static const struct dac8163_chip_info dac8163_chip_info = {
+	.name = "dac8163",
+	.channels = {
+		DAC8163_CHAN(0, 14),
+		DAC8163_CHAN(1, 14),
+	},
+	.default_output_reg = DAC8163_MID_SCALE(14),
+};
+
+static const struct dac8163_chip_info dac8562_chip_info = {
+	.name = "dac8562",
+	.channels = {
+		DAC8163_CHAN(0, 16),
+		DAC8163_CHAN(1, 16),
+	},
+	.default_output_reg = 0,
+};
+
+static const struct dac8163_chip_info dac8563_chip_info = {
+	.name = "dac8563",
+	.channels = {
+		DAC8163_CHAN(0, 16),
+		DAC8163_CHAN(1, 16),
+	},
+	.default_output_reg = DAC8163_MID_SCALE(16),
+};
+
+static int dac8163_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	struct dac8163_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(st->regmap,
+				  CMD_SET(CMD_WRITE_UPDATE, chan->channel),
+				  val);
+		if (ret)
+			return ret;
+		*val >>= chan->scan_type.shift;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_mV * st->gain;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int dac8163_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct dac8163_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (val2 != 0)
+			return -EINVAL;
+
+		if (val < 0 || val >= BIT(chan->scan_type.realbits))
+			return -ERANGE;
+
+		return regmap_write(st->regmap,
+				    CMD_SET(CMD_WRITE_UPDATE, chan->channel),
+				    (u16)val << chan->scan_type.shift);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info dac8163_iio_info = {
+	.write_raw = dac8163_write_raw,
+	.read_raw = dac8163_read_raw
+};
+
+static bool dac8163_reg_false(struct device *dev, unsigned int ref)
+{
+	return false;
+}
+
+static int dac8163_probe(struct spi_device *spi)
+{
+	const struct dac8163_chip_info *info;
+	struct gpio_desc *ldac_gpio;
+	struct iio_dev *indio_dev;
+	struct dac8163_state *st;
+	bool internal_reference;
+	int ret;
+
+	info = spi_get_device_match_data(spi);
+	if (!info)
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	indio_dev->name = info->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &dac8163_iio_info;
+	indio_dev->channels = info->channels;
+	indio_dev->num_channels = ARRAY_SIZE(info->channels);
+
+	const struct reg_default reg_defaults[] = {
+		{
+			.reg = CMD_SET(CMD_WRITE_UPDATE, 0),
+			.def = info->default_output_reg,
+		},
+		{
+			.reg = CMD_SET(CMD_WRITE_UPDATE, 1),
+			.def = info->default_output_reg,
+		},
+	};
+
+	const struct regmap_config regmap_config = {
+		.reg_bits = 8,
+		.val_bits = 16,
+
+		.max_register = CMD_REF << 3,
+		.cache_type = REGCACHE_MAPLE,
+
+		.volatile_reg = dac8163_reg_false,
+
+		.reg_defaults = reg_defaults,
+		.num_reg_defaults = ARRAY_SIZE(reg_defaults),
+	};
+
+	st->regmap = devm_regmap_init_spi(spi, &regmap_config);
+	if (IS_ERR(st->regmap))
+		return PTR_ERR(st->regmap);
+
+	// for now we keep the ldac pin asserted permanently so that the output
+	// is updated immediately after a write to the channels raw attribute
+	ldac_gpio = devm_gpiod_get_optional(&spi->dev, "ldac",
+					    GPIOD_OUT_HIGH);
+	if (IS_ERR(ldac_gpio))
+		return PTR_ERR(ldac_gpio);
+
+	if (device_property_present(&spi->dev, "vrefin-supply")) {
+		ret = devm_regulator_get_enable_read_voltage(&spi->dev,
+							     "vrefin");
+		if (ret < 0)
+			return ret;
+
+		st->vref_mV = ret / (MICRO / MILLI);
+		internal_reference = false;
+		st->gain = 1;
+	} else {
+		st->vref_mV = DAC8163_INTERNAL_REF_mV;
+		internal_reference = true;
+		st->gain = 2;
+	}
+
+	ret = devm_regulator_get_enable(&spi->dev, "avdd");
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(st->regmap, FIELD_PREP(COMMAND_MASK, CMD_SOFT_RST),
+			   FULL_RESET);
+
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(st->regmap, FIELD_PREP(COMMAND_MASK, CMD_LDAC_MODE),
+			   FIELD_PREP(LDAC_CHANNEL_A_MASK, LDAC_INACTIVE) |
+			   FIELD_PREP(LDAC_CHANNEL_B_MASK, LDAC_INACTIVE));
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(st->regmap, FIELD_PREP(COMMAND_MASK, CMD_REF),
+			   FIELD_PREP(VREF_MASK, internal_reference));
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id dac8163_of_match[] = {
+	{
+		.compatible = "ti,dac7562",
+		.data = &dac7562_chip_info,
+	},
+	{
+		.compatible = "ti,dac7563",
+		.data = &dac7563_chip_info,
+	},
+	{
+		.compatible = "ti,dac8162",
+		.data = &dac8162_chip_info,
+	},
+	{
+		.compatible = "ti,dac8163",
+		.data = &dac8163_chip_info,
+	},
+	{
+		.compatible = "ti,dac8562",
+		.data = &dac8562_chip_info,
+	},
+	{
+		.compatible = "ti,dac8563",
+		.data = &dac8563_chip_info,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dac8163_of_match);
+
+static const struct spi_device_id dac8163_id_table[] = {
+	{
+		.name = "dac7562",
+		.driver_data = (kernel_ulong_t)&dac7562_chip_info
+	},
+	{
+		.name = "dac7563",
+		.driver_data = (kernel_ulong_t)&dac7563_chip_info
+	},
+	{
+		.name = "dac8162",
+		.driver_data = (kernel_ulong_t)&dac8162_chip_info
+	},
+	{
+		.name = "dac8163",
+		.driver_data = (kernel_ulong_t)&dac8163_chip_info
+	},
+	{
+		.name = "dac8562",
+		.driver_data = (kernel_ulong_t)&dac8562_chip_info
+	},
+	{
+		.name = "dac8563",
+		.driver_data = (kernel_ulong_t)&dac8563_chip_info
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, dac8163_id_table);
+
+static struct spi_driver dac8163_driver = {
+	.driver = {
+		.name = "dac8163",
+		.of_match_table = dac8163_of_match,
+	},
+	.probe = dac8163_probe,
+	.id_table = dac8163_id_table,
+};
+module_spi_driver(dac8163_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

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

[Andy Shevchenko]

On Wed, Jul 08, 2026 at 11:52:44AM +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.

At the first glance it seems I already commented on the issues in this driver.
Yes, I see that I commented

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

and in several places in this version it has not been addressed.
Taking into account this I don't want to go through and check what
else is missing, so please go again to v1 and carefully read *all*
the comments and either address them, or explain why it's not done.

...

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

> +#include <linux/errno.h>

Not needed, err.h provides for standard error codes.

> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/stddef.h>
> +#include <linux/types.h>
> +#include <linux/units.h>

-- 
With Best Regards,
Andy Shevchenko

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

[Siratul Islam]

On Wed, 2026-07-08 at 11:52 +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.
>  
>  	  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
Missing "select REGMAP_SPI" here.
> +	help
> +	  Driver for the Texas Instruments digital-to-analog converter
> +	  family dacxx6x compatible with the following variants
> +	  - DAC7562 (2 channels, 12 bits, resets to zero)
> +	  - DAC7563 (2 channels, 12 bits, resets to mid-scale)
> 
> 
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * DAC8163 IIO driver (SPI)
> + * https://www.ti.com/de/lit/gpn/dac8163
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
I asked you to add this "mod_devicetable.h" header previously. But I noticed some reviews by Uwe 
to use more specific headers. 
In your case these should be 


#include <linux/device-id/spi.h>
#include <linux/device-id/of.h>

unless I'm missing something.

> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +#include <linux/stddef.h>
> +#include <linux/types.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define COMMAND_MASK GENMASK(6, 3)
> +#define ADDRESS_MASK GENMASK(2, 0)
> +
> +#define CMD_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_POWER_MODE		0x4
> +#define CMD_SOFT_RST		0x5
> +#define CMD_LDAC_MODE		0x6
> +#define CMD_REF			0x7
> +
> +#define LDAC_CHANNEL_A_MASK BIT(0)
> +#define LDAC_CHANNEL_B_MASK BIT(1)
> +#define VREF_MASK BIT(0)
Since the MASKs are defined together, align the values.

+#define LDAC_CHANNEL_A_MASK	BIT(0)
+#define LDAC_CHANNEL_B_MASK	BIT(1)
+#define VREF_MASK		BIT(0)

> +
> +#define DAC8163_INTERNAL_REF_mV 2500
> +
...
> +
> +#define DAC8163_MID_SCALE(resolution) \
> +	(BIT((resolution) - 1) << (16 - (resolution)))
> +
> +static const struct dac8163_chip_info dac7562_chip_info = {
> +	.name = "dac7562",
> +	.channels = {
> +		DAC8163_CHAN(0, 12),
> +		DAC8163_CHAN(1, 12),
> +	},
> +	.default_output_reg = 0,
> +};
> +
> +static const struct dac8163_chip_info dac7563_chip_info = {
> +	.name = "dac7563",
> +	.channels = {
> +		DAC8163_CHAN(0, 12),
> +		DAC8163_CHAN(1, 12),
> +	},
> +	.default_output_reg = DAC8163_MID_SCALE(12),
These resolution values are repeated multiple times. I think they could be defined as something like
#define DAC8163_RES_12_BIT	12 // 14, 16

> +};
> +
> +static const struct dac8163_chip_info dac8162_chip_info = {
> +	.name = "dac8162",
> +	.channels = {
> +		DAC8163_CHAN(0, 14),
> +		DAC8163_CHAN(1, 14),
> +	},
> +	.default_output_reg = 0,
> +};
> +
> +static const struct dac8163_chip_info dac8163_chip_info = {
> +	.name = "dac8163",
> +	.channels = {
> +		DAC8163_CHAN(0, 14),
> +		DAC8163_CHAN(1, 14),
> +	},
> +	.default_output_reg = DAC8163_MID_SCALE(14),
> +};
> +
> +static const struct dac8163_chip_info dac8562_chip_info = {
> +	.name = "dac8562",
> +	.channels = {
> +		DAC8163_CHAN(0, 16),
> +		DAC8163_CHAN(1, 16),
> +	},
> +	.default_output_reg = 0,
> +};
> +
> +static const struct dac8163_chip_info dac8563_chip_info = {
> +	.name = "dac8563",
> +	.channels = {
> +		DAC8163_CHAN(0, 16),
> +		DAC8163_CHAN(1, 16),
> +	},
> +	.default_output_reg = DAC8163_MID_SCALE(16),
> +};
> +
> +static int dac8163_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
Put val on the same line as val2. i.e.,
+static int dac8163_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)

> +{
> 
...
> +
> +static const struct iio_info dac8163_iio_info = {
> +	.write_raw = dac8163_write_raw,
> +	.read_raw = dac8163_read_raw
Missing trailing comma here.
> +};
> +
> +static bool dac8163_reg_false(struct device *dev, unsigned int ref)
> +{
> +	return false;
> +}
> +
> +static int dac8163_probe(struct spi_device *spi)
> +{
> 
...
> +
> +	const struct reg_default reg_defaults[] = {
> +		{
> +			.reg = CMD_SET(CMD_WRITE_UPDATE, 0),
> +			.def = info->default_output_reg,
> +		},
> +		{
> +			.reg = CMD_SET(CMD_WRITE_UPDATE, 1),
> +			.def = info->default_output_reg,
> +		},
> +	};
> +
> +	const struct regmap_config regmap_config = {
> +		.reg_bits = 8,
> +		.val_bits = 16,
> +
> +		.max_register = CMD_REF << 3,
> +		.cache_type = REGCACHE_MAPLE,
> +
> +		.volatile_reg = dac8163_reg_false,
> +
> +		.reg_defaults = reg_defaults,
> +		.num_reg_defaults = ARRAY_SIZE(reg_defaults),
> +	};
Why are these defined inside probe? Should be defined outside the function.
> +
> +	st->regmap = devm_regmap_init_spi(spi, &regmap_config);
> +	if (IS_ERR(st->regmap))
> +		return PTR_ERR(st->regmap);
Use "dev_err_probe"
return dev_err_probe(dev, PTR_ERR(st->regmap),
				     "regmap initialization failed\n");
> +
> +	// for now we keep the ldac pin asserted permanently so that the output
> +	// is updated immediately after a write to the channels raw attribute
> +	ldac_gpio = devm_gpiod_get_optional(&spi->dev, "ldac",
> +
...
> 
> +
> +static const struct spi_device_id dac8163_id_table[] = {
> +	{
> +		.name = "dac7562",
> +		.driver_data = (kernel_ulong_t)&dac7562_chip_info
Missing trailing commas here too, this and the ones below.
> +	},
> +	{
> +		.name = "dac7563",
> +		.driver_data = (kernel_ulong_t)&dac7563_chip_info
> +	},
> +	{
> +		.name = "dac8162",
> +		.driver_data = (kernel_ulong_t)&dac8162_chip_info
> +	},
> +	{
> +		.name = "dac8163",
> +		.driver_data = (kernel_ulong_t)&dac8163_chip_info
> +	},
> +	{
> +		.name = "dac8562",
> +		.driver_data = (kernel_ulong_t)&dac8562_chip_info
> +	},
> +	{
> +		.name = "dac8563",
> +		.driver_data = (kernel_ulong_t)&dac8563_chip_info
> +	},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, dac8163_id_table);
> +
> +static struct spi_driver dac8163_driver = {
> +	.driver = {
> +		.name = "dac8163",
> +		.of_match_table = dac8163_of_match,
> +	},
> +	.probe = dac8163_probe,
> +	.id_table = dac8163_id_table,
> +};
> +module_spi_driver(dac8163_driver);
> +
> +MODULE_AUTHOR("Lukas Metz <lukas.metz@gmx.net>");
> +MODULE_DESCRIPTION("Texas Instruments 12/14/16-bit 2-channel DAC driver");
> +MODULE_LICENSE("GPL");

--
Best regards,
Sirat

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

[Conor Dooley]

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

On Wed, Jul 08, 2026 at 11:52:43AM +0200, 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    | 85 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 ++
>  2 files changed, 91 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..533cc7327c2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> @@ -0,0 +1,85 @@
> +# 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.
> +  The sync pin on the device acts as chip-select pin.
> +
> +maintainers:
> +  - Lukas Metz <lukas.metz@gmx.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,dac7562
> +      - ti,dac7563
> +      - ti,dac8162
> +      - ti,dac8163
> +      - ti,dac8562
> +      - ti,dac8563

Please provide an explanation as to why none of these can use fallback
compatibles, or alternatively, use fallback compatibles.
pw-bot: changes-requested

> +
> +  spi-rx-bus-width:
> +    items:
> +      - const: 0
> +
> +  spi-max-frequency:
> +    maximum: 50000000
> +
> +  spi-cpha: true
> +
> +  reg:
> +    maxItems: 1
> +
> +  avdd-supply: true
> +
> +  clr-gpios:

Sounds like this should be reset-gpios?

Cheers,
Conor.

> +    description:
> +      The device registers are reset when this pin is asserted. The output is
> +      set to zero (xxx2 devices) or mid-scale (xxx3 devices).
> +    maxItems: 1
> +
> +  ldac-gpios:
> +    description:
> +      Pin needs to be asserted permanently when updating the DAC synchronously.
> +    maxItems: 1
> +
> +  vrefin-supply:
> +    description:
> +      External reference voltage supply for scaling. When no vrefin-supply
> +      is present the internal voltage reference is used.
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpha
> +  - avdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@1 {
> +            compatible = "ti,dac8163";
> +            reg = <0x1>; /* CS1 */
> +            spi-cpha;
> +            ldac-gpios = <&gpiog 8 GPIO_ACTIVE_LOW>;
> +            avdd-supply = <&avdd_3v3>;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d238590a31f2..314f235332f5 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:	Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
> +
>  TI DATA TRANSFORM AND HASHING ENGINE (DTHE) V2 CRYPTO DRIVER
>  M:	T Pratham <t-pratham@ti.com>
>  L:	linux-crypto@vger.kernel.org
> 
> -- 
> 2.43.0
> 

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

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

[Jonathan Cameron]

> > +
> > +#include <linux/array_size.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/mod_devicetable.h>  
> I asked you to add this "mod_devicetable.h" header previously. But I noticed some reviews by Uwe 
> to use more specific headers. 
> In your case these should be 
> 
> 
> #include <linux/device-id/spi.h>
> #include <linux/device-id/of.h>
> 
Not necessary given they are guaranteed to be include by the bus headers.

Take a look at what Uwe did in his series.  The actual device-id headers
are added to very few drivers - mostly the bus ones are enough.

Jonathan

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

[Siratul Islam]

On 26/07/08 06:33PM, Jonathan Cameron wrote:
> 
> > > +
> > > +#include <linux/array_size.h>
> > > +#include <linux/bitfield.h>
> > > +#include <linux/bits.h>
> > > +#include <linux/err.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/gpio/consumer.h>
> > > +#include <linux/mod_devicetable.h>  
> > I asked you to add this "mod_devicetable.h" header previously. But I noticed some reviews by Uwe 
> > to use more specific headers. 
> > In your case these should be 
> > 
> > 
> > #include <linux/device-id/spi.h>
> > #include <linux/device-id/of.h>
> > 
> Not necessary given they are guaranteed to be include by the bus headers.
> 
> Take a look at what Uwe did in his series.  The actual device-id headers
> are added to very few drivers - mostly the bus ones are enough.
> 
> Jonathan

Thanks for the clarification. I should have investigated more instead of
following half the story.

--
Best regards,
Sirat

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

[Lukas]

On Wed, Jul 08, 2026 at 02:32:01PM +0300, Andy Shevchenko wrote:
> 
> At the first glance it seems I already commented on the issues in this driver.
> Yes, I see that I commented
> 
>   "Leave trailing commas in the non-terminator entries here and there."
> 
> and in several places in this version it has not been addressed.
> Taking into account this I don't want to go through and check what
> else is missing, so please go again to v1 and carefully read *all*
> the comments and either address them, or explain why it's not done.
>

You are right, i forgot to change that in several places. I think i got
lost a little bit due to the amount of changes. I will make sure to 
carefully check that i didnt miss anything before sending v3.
Thanks for taking the time.

Best regards

Lukas

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

[Andy Shevchenko]

On Thu, Jul 09, 2026 at 12:03:14PM +0200, Lukas wrote:
> On Wed, Jul 08, 2026 at 02:32:01PM +0300, Andy Shevchenko wrote:

...

> > At the first glance it seems I already commented on the issues in this driver.
> > Yes, I see that I commented
> > 
> >   "Leave trailing commas in the non-terminator entries here and there."
> > 
> > and in several places in this version it has not been addressed.
> > Taking into account this I don't want to go through and check what
> > else is missing, so please go again to v1 and carefully read *all*
> > the comments and either address them, or explain why it's not done.
> 
> You are right, i forgot to change that in several places. I think i got
> lost a little bit due to the amount of changes.

The advice here is to slow down, and be not in a hurry, just take your time
for what needs to be done.

> I will make sure to carefully check that i didnt miss anything before sending
> v3.  

> Thanks for taking the time.

You're welcome!

-- 
With Best Regards,
Andy Shevchenko