iio: dac: ad5337: Add driver for the AD5337 DAC
Subsystems (3)
PATCH 1/2 iio: dac: ad5337: Add driver for the AD5337 DAC
Add driver for the AD5337 dual 8-bit I2C voltage-output DAC. This part uses the pointer-byte protocol and is not compatible with the AD5337R nanoDAC+ driver (adi,ad5337r).
--- MAINTAINERS | 5 ++ drivers/iio/dac/Kconfig | 12 ++- drivers/iio/dac/Makefile | 1 + drivers/iio/dac/ad5337.c | 159 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/dac/ad5337.c
MAINTAINERS +5 -0
@@ -438,6 +438,11 @@ W: http://wiki.analog.com/AD5398 W: https://ez.analog.com/linux-software-drivers F: drivers/regulator/ad5398.c +AD5337 ANALOG DEVICES INC AD5337 DAC DRIVER +M: Marcin Bis <[email protected]> +S: Maintained +F: drivers/iio/dac/ad5337.c + AD5446 ANALOG DEVICES INC AD5446 DAC DRIVER M: Michael Hennerich <[email protected]> M: Nuno Sá <[email protected]>
drivers/iio/dac/Kconfig +11 -1
@@ -62,6 +62,16 @@ config AD5064 To compile this driver as a module, choose M here: the module will be called ad5064. +config AD5337 + tristate "Analog Devices AD5337 I2C DAC" + depends on I2C + help + Say yes here to build support for the Analog Devices AD5337 dual + 8-bit I2C voltage-output DAC. + + To compile this driver as a module, choose M here: the module will be + called ad5337. + config AD5360 tristate "Analog Devices AD5360/61/62/63/70/71/73 DAC driver" depends on SPI
@@ -247,7 +257,7 @@ config AD5696_I2C depends on I2C select AD5686 help - Say yes here to build support for Analog Devices AD5311R, AD5337, + Say yes here to build support for Analog Devices AD5311R, AD5337R, AD5338R, AD5671R, AD5673R, AD5675R, AD5677R, AD5691R, AD5692R, AD5693, AD5693R, AD5694, AD5694R, AD5695R, AD5696, and AD5696R Digital to Analog converters.
drivers/iio/dac/Makefile +1 -0
@@ -8,6 +8,7 @@ obj-$(CONFIG_AD3530R) += ad3530r.o obj-$(CONFIG_AD3552R_HS) += ad3552r-hs.o obj-$(CONFIG_AD3552R_LIB) += ad3552r-common.o obj-$(CONFIG_AD3552R) += ad3552r.o +obj-$(CONFIG_AD5337) += ad5337.o obj-$(CONFIG_AD5360) += ad5360.o obj-$(CONFIG_AD5380) += ad5380.o obj-$(CONFIG_AD5421) += ad5421.o
drivers/iio/dac/ad5337.c new file +159 -0
159 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH 2/2 dt-bindings: iio: dac: adi,ad5337: add dt-bindings
Add dt-bindings for the AD5337 dual 8-bit I2C voltage-output DAC driver.
--- .../bindings/iio/dac/adi,ad5337.yaml | 48 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5337.yaml
Documentation/devicetree/bindings/iio/dac/adi,ad5337.yaml new file +48 -0
@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/adi,ad5337.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD5337 I2C DAC + +maintainers: + - Marcin Bis <[email protected]> + +description: + Binding for the AD5337 dual 8-bit I2C voltage-output DAC. + This part uses the pointer-byte protocol and is not compatible with + the AD5337R nanoDAC+ driver (adi,ad5337r). + +properties: + compatible: + const: adi,ad5337 + + reg: + maxItems: 1 + description: + 7-bit I2C address. 0x0c when A0 is low, 0x0d when A0 is high. + + vcc-supply: + description: DAC reference voltage supply. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + dac@0c { + compatible = "adi,ad5337"; + reg = <0x0c>; + vcc-supply = <&dac_3v3>; + }; + }; + +...
MAINTAINERS +1 -0
@@ -441,6 +441,7 @@ F: drivers/regulator/ad5398.c AD5337 ANALOG DEVICES INC AD5337 DAC DRIVER M: Marcin Bis <[email protected]> S: Maintained +F: Documentation/devicetree/bindings/iio/dac/adi,ad5337.yaml F: drivers/iio/dac/ad5337.c AD5446 ANALOG DEVICES INC AD5446 DAC DRIVER
Signature · 1 line
2.43.0
Re: [PATCH 2/2] dt-bindings: iio: dac: adi,ad5337: add dt-bindings
Quoted 4 lines · On Fri, 12 Jun 2026 15:31:22 +0200 Marcin Bis <[email protected]> wrote:
Add dt-bindings for the AD5337 dual 8-bit I2C voltage-output DAC driver.
Signed-off-by Marcin Bis <[email protected]>---
If you're adding new device support, any dt-binding patches should go first in the series.
Another big problem is that you haven't cc'd any devicetree maintainers/reviewers, nor the devicetree mailing list, therefore they probably won't see this patch and their automated tooling definitely won't check this. If you're unsure where to send this patch to, use the get_maintainer.pl script.
Also, it seems that you've replied with this patch to the first patch in the series, making it appear strangely in my (and probably other people's) email clients. Use a cover letter when sending patch series, it prevents things like this from happening.
+cc'd [email protected]
Signature · 3 lines
Kind regards CJD
Jonathan Cameron
maintainerRe: [PATCH 2/2] dt-bindings: iio: dac: adi,ad5337: add dt-bindings
In reply to Marcin BisQuoted 2 lines · On Fri, 12 Jun 2026 15:31:22 +0200 Marcin Bis <[email protected]> wrote:
Add dt-bindings for the AD5337 dual 8-bit I2C voltage-output DAC driver.
What is the relationship between this part and the ad5337r which is supported by the ad5696-i2c.c driver and adi,ad5696.yaml?
I'm wondering if there actually is an ad5337r or whether it was always meant to be this device. I can't find a datasheet for it.
Jonathan
Quoted 73 lines
Signed-off-by Marcin Bis <[email protected]>--- .../bindings/iio/dac/adi,ad5337.yaml | 48 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad5337.yamlDocumentation/devicetree/bindings/iio/dac/adi,ad5337.yaml new file +48
-0@@ -0,0 +1,48 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/dac/adi,ad5337.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD5337 I2C DAC + +maintainers: + - Marcin Bis <[email protected]> + +description: + Binding for the AD5337 dual 8-bit I2C voltage-output DAC. + This part uses the pointer-byte protocol and is not compatible with + the AD5337R nanoDAC+ driver (adi,ad5337r). + +properties: + compatible: + const: adi,ad5337 + + reg: + maxItems: 1 + description: + 7-bit I2C address. 0x0c when A0 is low, 0x0d when A0 is high. + + vcc-supply: + description: DAC reference voltage supply. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + dac@0c { + compatible = "adi,ad5337"; + reg = <0x0c>; + vcc-supply = <&dac_3v3>; + }; + }; + +...MAINTAINERS +1
-0@@ -441,6 +441,7 @@ F: drivers/regulator/ad5398.c AD5337 ANALOG DEVICES INC AD5337 DAC DRIVER M: Marcin Bis <[email protected]> S: Maintained +F: Documentation/devicetree/bindings/iio/dac/adi,ad5337.yaml F: drivers/iio/dac/ad5337.c AD5446 ANALOG DEVICES INC AD5446 DAC DRIVER
Re: [PATCH 2/2] dt-bindings: iio: dac: adi,ad5337: add dt-bindings
In reply to Marcin BisQuoted 3 lines · Marcin Bis
Add dt-bindings for the AD5337 dual 8-bit I2C voltage-output DAC driver.
Signed-off-by Marcin Bis <[email protected]>
Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about b4 prep --auto-to-cc if you added new patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time.
Please kindly resend and include all necessary To/Cc entries.
Please organize the patch documenting the compatible (DT bindings) before the patch using that compatible.
See also:
https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46
Best regards,
Krzysztof
Quoted 18 lines · On Fri, 12 Jun 2026 15:31:21 +0200 Marcin Bis <[email protected]> wrote:
Add driver for the AD5337 dual 8-bit I2C voltage-output DAC. This part uses the pointer-byte protocol and is not compatible with the AD5337R nanoDAC+ driver (adi,ad5337r).
Signed-off-by Marcin Bis <[email protected]>--- MAINTAINERS | 5 ++ drivers/iio/dac/Kconfig | 12 ++- drivers/iio/dac/Makefile | 1 + drivers/iio/dac/ad5337.c | 159 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/dac/ad5337.cMAINTAINERS +0
-0
MAINTAINERS W: http://wiki.analog.com/AD5398 · line 444
Quoted 7 lines
MAINTAINERS +4
-0@@ -438,6 +438,11 @@ W: http://wiki.analog.com/AD5398 W: https://ez.analog.com/linux-software-drivers F: drivers/regulator/ad5398.c +AD5337 ANALOG DEVICES INC AD5337 DAC DRIVER +M: Marcin Bis <[email protected]> +S: Maintained +F: drivers/iio/dac/ad5337.c
Once you reorder your patches, this should go into the (first) dt-binding patch.
MAINTAINERS W: http://wiki.analog.com/AD5398
Quoted 11 lines
MAINTAINERS +11
-0+// SPDX-License-Identifier: GPL-2.0 +/* + * Analog Devices AD5337 I2C DAC driver + * + * Copyright 2017 Marcin Bis <[email protected]> + */ + +#include <linux/i2c.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/regulator/consumer.h>
You're definitely missing array_size.h, types.h, mod_devicetable.h and err.h.
MAINTAINERS
Quoted 7 lines
MAINTAINERS +7
-0+#include <linux/iio/iio.h> + +#define AD5337_NUM_CHANNELS 2 +#define AD5337_RESOLUTION 8 +/* + * AD5338 - 10bit, AD5339 - 12bit + */
This comment can just be on one line.
MAINTAINERS
Quoted 7 lines
MAINTAINERS +7
-0+ +#define AD5337_PTR_DAC_A 0x01 +#define AD5337_PTR_DAC_B 0x02 + +struct ad5337_state { + struct i2c_client *client; + struct mutex lock;
Add a comment on why you need a mutex.
Quoted 7 lines
MAINTAINERS +7
-0+ unsigned int vref_mv; + u8 cache[AD5337_NUM_CHANNELS]; +}; + +static int ad5337_write_dac(struct ad5337_state *st, int channel, u8 value) +{ + u8 buf[3];
MAINTAINERS
Quoted 10 lines
MAINTAINERS +10
-0+ int ret; + + if (channel) + buf[0] = AD5337_PTR_DAC_B; + else + buf[0] = AD5337_PTR_DAC_A; + + /* PD1=0, PD0=0, CLR=1, LDAC=0, D[7:4] */ + buf[1] = 0x20 | (value >> 4); + buf[2] = (value & 0x0f) << 4;
You should definitely use FIELD_GET/FIELD_PREP macros from the <linux/bitfield.h> header, it improves readability by a lot!
Also, use macros instead of just defining magic numbers like this.
MAINTAINERS
Quoted 2 lines
MAINTAINERS +2
-0+ + ret = i2c_master_send(st->client, buf, 3);
Use sizeof(buf) instead of just hardcoding the size.
MAINTAINERS
Quoted 2 lines
MAINTAINERS +2
-0+ + return (ret < 0) ? ret : (ret == 3 ? 0 : -EIO);
I'd definitely abandon using a terniary operator and just break it into if statements, on first glance this is really hard to read.
Quoted 25 lines
MAINTAINERS +25
-0+} + +static int ad5337_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct ad5337_state *st = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_RAW: + *val = st->cache[chan->channel]; + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + *val = st->vref_mv; + *val2 = AD5337_RESOLUTION; + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } +} + +static int ad5337_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{
MAINTAINERS
Quoted 10 lines
MAINTAINERS +10
-0+ struct ad5337_state *st = iio_priv(indio_dev); + int ret; + + if (mask != IIO_CHAN_INFO_RAW) + return -EINVAL; + + if (val < 0 || val > ((1 << AD5337_RESOLUTION) - 1)) + return -EINVAL; + + mutex_lock(&st->lock);
Consider using the guard(mutex)() macro from cleanup.h, it saves lines and prevents future developers from accidentally forgetting to release locks (don't forget to add the header as well though)!
Quoted 4 lines
MAINTAINERS +4
-0+ ret = ad5337_write_dac(st, chan->channel, val); + if (!ret) + st->cache[chan->channel] = val; + mutex_unlock(&st->lock);
MAINTAINERS
Quoted 10 lines
MAINTAINERS +10
-0+ + return ret; +} + +static const struct iio_info ad5337_info = { + .read_raw = ad5337_read_raw, + .write_raw = ad5337_write_raw, +}; + +#define AD5337_CHANNEL(_chan) { \
A very tiny nitpick, but you don't need to align your backslashes when defining a macro.
Quoted 7 lines
MAINTAINERS +7
-0+ .type = IIO_VOLTAGE, \ + .indexed = 1, \ + .output = 1, \ + .channel = (_chan), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \ + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \ +}
MAINTAINERS
Quoted 10 lines
MAINTAINERS +10
-0+ +static const struct iio_chan_spec ad5337_channels[] = { + AD5337_CHANNEL(0), + AD5337_CHANNEL(1), +}; + +static int ad5337_probe(struct i2c_client *client) +{ + struct ad5337_state *st; + struct iio_dev *indio_dev;
Very much a nit, but swap these lines.
MAINTAINERS
Quoted 12 lines
MAINTAINERS +12
-0+ int ret; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) + return -ENODEV; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); + if (!indio_dev) + return -ENOMEM; + + st = iio_priv(indio_dev); + st->client = client; + mutex_init(&st->lock);
Use devm_mutex_init().
MAINTAINERS
Quoted 4 lines
MAINTAINERS +4
-0+ + ret = devm_regulator_get_enable_read_voltage(&client->dev, "vcc"); + if (ret < 0 && ret != -ENODEV) + return ret;
Return dev_err_probe() here.
MAINTAINERS
Quoted 2 lines
MAINTAINERS +2
-0+ + st->vref_mv = ret > 0 ? ret / 1000 : 3300;
More magic numbers. If you're converting units, consider using macros from <linux/units.h>.
Signature · 3 lines
Kind regards CJD
Jonathan Cameron
maintainerQuoted 3 lines · On Fri, 12 Jun 2026 15:31:21 +0200 Marcin Bis <[email protected]> wrote:
Add driver for the AD5337 dual 8-bit I2C voltage-output DAC. This part uses the pointer-byte protocol and is not compatible with the AD5337R nanoDAC+ driver (adi,ad5337r).
Ah. Here's the explanation. Make sure that's in the dt binding patch description as well! However, the binding could be shared. Note that sharing a binding doesn't imply a shared driver.
+CC Fabio who sent the original patch to ad the ad5337r.
Fabio, what was the history of that part? I can't find any reference to it any more.
Quoted 48 lines
Signed-off-by Marcin Bis <[email protected]>--- MAINTAINERS | 5 ++ drivers/iio/dac/Kconfig | 12 ++- drivers/iio/dac/Makefile | 1 + drivers/iio/dac/ad5337.c | 159 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/dac/ad5337.cMAINTAINERS +5
-0@@ -438,6 +438,11 @@ W: http://wiki.analog.com/AD5398 W: https://ez.analog.com/linux-software-drivers F: drivers/regulator/ad5398.c +AD5337 ANALOG DEVICES INC AD5337 DAC DRIVER +M: Marcin Bis <[email protected]> +S: Maintained +F: drivers/iio/dac/ad5337.c + AD5446 ANALOG DEVICES INC AD5446 DAC DRIVER M: Michael Hennerich <[email protected]> M: Nuno Sá <[email protected]>drivers/iio/dac/Kconfig +10
-0@@ -62,6 +62,16 @@ config AD5064 To compile this driver as a module, choose M here: the module will be called ad5064. +config AD5337 + tristate "Analog Devices AD5337 I2C DAC" + depends on I2C + help + Say yes here to build support for the Analog Devices AD5337 dual + 8-bit I2C voltage-output DAC. + + To compile this driver as a module, choose M here: the module will be + called ad5337. + config AD5360 tristate "Analog Devices AD5360/61/62/63/70/71/73 DAC driver" depends on SPI
drivers/iio/dac/Kconfig config AD5696_I2C · line 260
Quoted 5 lines
drivers/iio/dac/Kconfig +1
-1@@ -247,7 +257,7 @@ config AD5696_I2C depends on I2C select AD5686 help - Say yes here to build support for Analog Devices AD5311R, AD5337, + Say yes here to build support for Analog Devices AD5311R, AD5337R,
Separate patch before this one please.
Quoted 9 lines
drivers/iio/dac/Kconfig +0
-0AD5338R, AD5671R, AD5673R, AD5675R, AD5677R, AD5691R, AD5692R, AD5693, AD5693R, AD5694, AD5694R, AD5695R, AD5696, and AD5696R Digital to Analog converters.drivers/iio/dac/ad5337.c new file +0
-0
drivers/iio/dac/ad5337.c line 19
Quoted 9 lines
drivers/iio/dac/ad5337.c new file +8
-0+ +#include <linux/iio/iio.h> + +#define AD5337_NUM_CHANNELS 2 +#define AD5337_RESOLUTION 8 +/* +* AD5338 - 10bit, AD5339 - 12bit +*/
What is this doing here? Add support for them
or drop the comment.
drivers/iio/dac/ad5337.c line 26
Quoted 7 lines
drivers/iio/dac/ad5337.c new file +7
-0+ +#define AD5337_PTR_DAC_A 0x01 +#define AD5337_PTR_DAC_B 0x02 + +struct ad5337_state { +struct i2c_client *client; +struct mutex lock;
Locks needs a comment on what data it is there to protect.
Quoted 10 lines
drivers/iio/dac/ad5337.c new file +8
-0+unsigned int vref_mv; +u8 cache[AD5337_NUM_CHANNELS ]; +}; + +static int ad5337_write_raw (struct iio_dev *indio_dev, +struct iio_chan_spec const *chan, +int val,int val2,long mask) +{
drivers/iio/dac/ad5337.c line 82
Quoted 10 lines
drivers/iio/dac/ad5337.c new file +10
-0+struct ad5337_state *st =iio_priv (indio_dev); +int ret; + +if (mask !=IIO_CHAN_INFO_RAW ) +return -EINVAL ; + +if (val <0 || val > ((1 <<AD5337_RESOLUTION ) -1 )) +return -EINVAL ; + +mutex_lock (&st->lock);
guard(mutex)(&st->lock); ret = ad5337_write_dac(st, chan->channel, val); if (ret) return ret st->cache[chan->channel] = val; return 0;
Quoted 7 lines
drivers/iio/dac/ad5337.c new file +7
-0+ ret =ad5337_write_dac (st, chan->channel, val); +if (!ret) + st->cache[chan->channel] = val; +mutex_unlock (&st->lock); + +return ret; +}