All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add driver for DAC8163:
@ 2026-07-08  9:52 Lukas Metz
  2026-07-08  9:52 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 Lukas Metz
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
  0 siblings, 2 replies; 16+ messages in thread
From: Lukas Metz @ 2026-07-08  9:52 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, 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>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-08  9:52 [PATCH v2 0/2] Add driver for DAC8163: Lukas Metz
@ 2026-07-08  9:52 ` Lukas Metz
  2026-07-08 10:06   ` sashiko-bot
  2026-07-08 16:37   ` Conor Dooley
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
  1 sibling, 2 replies; 16+ messages in thread
From: Lukas Metz @ 2026-07-08  9:52 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08  9:52 [PATCH v2 0/2] Add driver for DAC8163: Lukas Metz
  2026-07-08  9:52 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 Lukas Metz
@ 2026-07-08  9:52 ` Lukas Metz
  2026-07-08 10:04   ` sashiko-bot
                     ` (3 more replies)
  1 sibling, 4 replies; 16+ messages in thread
From: Lukas Metz @ 2026-07-08  9:52 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
@ 2026-07-08 10:04   ` sashiko-bot
  2026-07-08 11:32   ` Andy Shevchenko
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-08 10:04 UTC (permalink / raw)
  To: Lukas Metz; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware sequencing risk: external reference voltage is enabled before the analog power supply.
- [Low] Incompatible pointer type passed to `regmap_read()` causing a build failure.
- [Low] Missing `select REGMAP_SPI` in Kconfig leads to build or probe failures.
--

--- Patch [2]: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 ---
commit e7b20377f202f6c225fbb971eaeb3c40d160ef56
Author: Lukas Metz <lukas.metz@gmx.net>

iio: dac: dac8163: Add driver for DAC8163

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.

> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 657c68e75542e..257cb92f43fde 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -644,6 +644,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

[Severity: Low]
Should this Kconfig select REGMAP_SPI?  If CONFIG_TI_DAC8163=y is selected
without any other kernel configuration enabling REGMAP_SPI, the kernel build
might fail with a linker error for __devm_regmap_init_spi, or the driver might
fail to probe.

> +	help
> +	  Driver for the Texas Instruments digital-to-analog converter
> +	  family dacxx6x compatible with the following variants

[ ... ]

> diff --git a/drivers/iio/dac/ti-dac8163.c b/drivers/iio/dac/ti-dac8163.c
> new file mode 100644
> index 0000000000000..6be0aac2e8753
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac8163.c
> @@ -0,0 +1,373 @@

[ ... ]

> +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);

[Severity: Low]
Can this cause a build failure on compilers that treat incompatible pointer
types as errors?  The dac8163_read_raw() function passes val (which is of
type int *) directly to regmap_read(), but regmap_read() strictly expects an
unsigned int *.

> +		if (ret)
> +			return ret;
> +		*val >>= chan->scan_type.shift;
> +		return IIO_VAL_INT;

[ ... ]

> +static int dac8163_probe(struct spi_device *spi)
> +{

[ ... ]

> +	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");

[Severity: High]
Could this power sequencing damage the hardware?  During dac8163_probe(),
the external reference voltage (vrefin) is enabled before the primary analog
power supply (avdd).

Applying an external reference voltage before the analog supply is active might
forward-bias internal ESD protection diodes, which could lead to excessive
current draw or latch-up.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-dac8163-work-v2-0-3acd1bf20182@gmx.net?part=2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-08  9:52 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 Lukas Metz
@ 2026-07-08 10:06   ` sashiko-bot
  2026-07-08 16:37   ` Conor Dooley
  1 sibling, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-07-08 10:06 UTC (permalink / raw)
  To: Lukas Metz; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree binding uses the non-standard `clr-gpios` property name for a reset line instead of the standard `reset-gpios`.
--

--- Patch [1]: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 ---
commit f211d5a7de6b208a53302eba555c43258dd0918b
Author: Lukas Metz <lukas.metz@gmx.net>

dt-bindings: iio: dac: Add DAC8163

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

> 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 0000000000000..533cc7327c2b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac8163.yaml
[ ... ]
> +  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

[Severity: Low]
Is clr-gpios the correct name for this property? The description indicates it
functions as a reset line. Should this use the standard reset-gpios property
name instead of a datasheet-specific pin name?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-dac8163-work-v2-0-3acd1bf20182@gmx.net?part=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
  2026-07-08 10:04   ` sashiko-bot
@ 2026-07-08 11:32   ` Andy Shevchenko
  2026-07-09 10:03     ` Lukas
  2026-07-08 15:58   ` Siratul Islam
  2026-07-12 22:23   ` Uwe Kleine-König
  3 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2026-07-08 11:32 UTC (permalink / raw)
  To: Lukas Metz
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
  2026-07-08 10:04   ` sashiko-bot
  2026-07-08 11:32   ` Andy Shevchenko
@ 2026-07-08 15:58   ` Siratul Islam
  2026-07-08 17:33     ` Jonathan Cameron
  2026-07-12 22:23   ` Uwe Kleine-König
  3 siblings, 1 reply; 16+ messages in thread
From: Siratul Islam @ 2026-07-08 15:58 UTC (permalink / raw)
  To: Lukas Metz
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-08  9:52 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 Lukas Metz
  2026-07-08 10:06   ` sashiko-bot
@ 2026-07-08 16:37   ` Conor Dooley
  2026-07-14 15:01     ` Lukas
  1 sibling, 1 reply; 16+ messages in thread
From: Conor Dooley @ 2026-07-08 16:37 UTC (permalink / raw)
  To: Lukas Metz
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08 15:58   ` Siratul Islam
@ 2026-07-08 17:33     ` Jonathan Cameron
  2026-07-08 19:04       ` Siratul Islam
  0 siblings, 1 reply; 16+ messages in thread
From: Jonathan Cameron @ 2026-07-08 17:33 UTC (permalink / raw)
  To: Siratul Islam
  Cc: Lukas Metz, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree


> > +
> > +#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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08 17:33     ` Jonathan Cameron
@ 2026-07-08 19:04       ` Siratul Islam
  0 siblings, 0 replies; 16+ messages in thread
From: Siratul Islam @ 2026-07-08 19:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Lukas Metz, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08 11:32   ` Andy Shevchenko
@ 2026-07-09 10:03     ` Lukas
  2026-07-09 10:19       ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Lukas @ 2026-07-09 10:03 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-09 10:03     ` Lukas
@ 2026-07-09 10:19       ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-07-09 10:19 UTC (permalink / raw)
  To: Lukas
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163
  2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
                     ` (2 preceding siblings ...)
  2026-07-08 15:58   ` Siratul Islam
@ 2026-07-12 22:23   ` Uwe Kleine-König
  3 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2026-07-12 22:23 UTC (permalink / raw)
  To: Lukas Metz
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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

On Wed, Jul 08, 2026 at 11:52:44AM +0200, Lukas Metz wrote:
> +#include <linux/mod_devicetable.h>

Please don't add new users for this header file. Only use those
<linux/device-id/*.h> that you actually need (if any).

Thanks
Uwe

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-08 16:37   ` Conor Dooley
@ 2026-07-14 15:01     ` Lukas
  2026-07-14 16:49       ` Conor Dooley
  0 siblings, 1 reply; 16+ messages in thread
From: Lukas @ 2026-07-14 15:01 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

On Wed, Jul 08, 2026 at 05:37:44PM +0100, Conor Dooley wrote:
> > +  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
> > +
> 
> Sounds like this should be reset-gpios?
> 

Thanks for pointing this out. The description is actually wrong. The
datasheet says: "On activation of CLR, zero scale (DACxx62) or 
mid-scale (DACxx63) is loaded to all input and DAC registers".
That means other registers are unaffected. I will update the description
but the clr-gpios is correct in this case i guess?

Best regards
Lukas

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-14 15:01     ` Lukas
@ 2026-07-14 16:49       ` Conor Dooley
  2026-07-14 19:25         ` David Lechner
  0 siblings, 1 reply; 16+ messages in thread
From: Conor Dooley @ 2026-07-14 16:49 UTC (permalink / raw)
  To: Lukas
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

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

On Tue, Jul 14, 2026 at 05:01:58PM +0200, Lukas wrote:
> On Wed, Jul 08, 2026 at 05:37:44PM +0100, Conor Dooley wrote:
> > > +  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
> > > +
> > 
> > Sounds like this should be reset-gpios?
> > 
> 
> Thanks for pointing this out. The description is actually wrong. The
> datasheet says: "On activation of CLR, zero scale (DACxx62) or 
> mid-scale (DACxx63) is loaded to all input and DAC registers".
> That means other registers are unaffected. I will update the description
> but the clr-gpios is correct in this case i guess?

Sounds about right.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163
  2026-07-14 16:49       ` Conor Dooley
@ 2026-07-14 19:25         ` David Lechner
  0 siblings, 0 replies; 16+ messages in thread
From: David Lechner @ 2026-07-14 19:25 UTC (permalink / raw)
  To: Conor Dooley, Lukas
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio,
	devicetree

On 7/14/26 11:49 AM, Conor Dooley wrote:
> On Tue, Jul 14, 2026 at 05:01:58PM +0200, Lukas wrote:
>> On Wed, Jul 08, 2026 at 05:37:44PM +0100, Conor Dooley wrote:
>>>> +  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
>>>> +
>>>
>>> Sounds like this should be reset-gpios?
>>>
>>
>> Thanks for pointing this out. The description is actually wrong. The
>> datasheet says: "On activation of CLR, zero scale (DACxx62) or 
>> mid-scale (DACxx63) is loaded to all input and DAC registers".
>> That means other registers are unaffected. I will update the description
>> but the clr-gpios is correct in this case i guess?
> 
> Sounds about right.

For ad5791, we used clear-gpios for a pin with the same function
(even though the pin there is labeled CLR just like here).

So would be good to stick with that convention. (Same reason
we always use reset-gpios even when a pin is labeled RST, it
is the same function.)


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-07-14 19:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08  9:52 [PATCH v2 0/2] Add driver for DAC8163: Lukas Metz
2026-07-08  9:52 ` [PATCH v2 1/2] dt-bindings: iio: dac: Add DAC8163 Lukas Metz
2026-07-08 10:06   ` sashiko-bot
2026-07-08 16:37   ` Conor Dooley
2026-07-14 15:01     ` Lukas
2026-07-14 16:49       ` Conor Dooley
2026-07-14 19:25         ` David Lechner
2026-07-08  9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz
2026-07-08 10:04   ` sashiko-bot
2026-07-08 11:32   ` Andy Shevchenko
2026-07-09 10:03     ` Lukas
2026-07-09 10:19       ` Andy Shevchenko
2026-07-08 15:58   ` Siratul Islam
2026-07-08 17:33     ` Jonathan Cameron
2026-07-08 19:04       ` Siratul Islam
2026-07-12 22:23   ` Uwe Kleine-König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.