* [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
* 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 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 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
* [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, ®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] 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 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 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 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, ®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] 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 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
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.