* [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; 10+ 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] 10+ 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 16:37 ` Conor Dooley 2026-07-08 9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz 1 sibling, 1 reply; 10+ 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] 10+ 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 16:37 ` Conor Dooley 0 siblings, 0 replies; 10+ 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] 10+ 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 11:32 ` Andy Shevchenko 2026-07-08 15:58 ` Siratul Islam 1 sibling, 2 replies; 10+ 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, ®map_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] 10+ 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 11:32 ` Andy Shevchenko 2026-07-09 10:03 ` Lukas 2026-07-08 15:58 ` Siratul Islam 1 sibling, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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 11:32 ` Andy Shevchenko @ 2026-07-08 15:58 ` Siratul Islam 2026-07-08 17:33 ` Jonathan Cameron 1 sibling, 1 reply; 10+ 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, ®map_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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2026-07-09 10:19 UTC | newest] Thread overview: 10+ 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 16:37 ` Conor Dooley 2026-07-08 9:52 ` [PATCH v2 2/2] iio: dac: dac8163: Add driver for DAC8163 Lukas Metz 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox