public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: "Nuno Sá" <noname.nuno@gmail.com>
To: "Alexis Czezar Torreno" <alexisczezar.torreno@analog.com>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] iio: dac: ad5706r: Add support for AD5706R DAC
Date: Wed, 25 Mar 2026 11:02:38 +0000	[thread overview]
Message-ID: <1920a90a223c1394004d269895df54eee720125a.camel@gmail.com> (raw)
In-Reply-To: <20260318-dev_ad5706r-v3-2-5d078f41e988@analog.com>

On Wed, 2026-03-18 at 13:13 +0800, Alexis Czezar Torreno wrote:
> Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> current output digital-to-analog converter with SPI interface.
> 
> Features:
>   - 4 independent DAC channels
>   - Hardware and software LDAC trigger
>   - Configurable output range
>   - PWM-based LDAC control
>   - Dither and toggle modes
>   - Dynamically configurable SPI speed
> 
> Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
> 
> ---

Not sure if everyone is aware of this but AI review in:

https://sashiko.dev/#/patchset/20260318-dev_ad5706r-v3-0-5d078f41e988%40analog.com

The DMA comment is an interesting one. Likely it needs improvement in the review-prompt repo given
it's one of those case we know what we're doing (at least we think we know :)). Still has a point
in that IIO_DMA_MINALIGN should be used.

At some point we need to add some IIO specific bits :)

- Nuno Sá

> Changes since v1:
>   - Removed PWM, GPIO, clock generator, debugfs, regmap, IIO_BUFFER
>   - Removed all custom ext_info sysfs attributes
>   - Simplified to basic raw read/write and read-only scale
>   - SPI read/write can handle multibyte registers
> ---
> ---
>  MAINTAINERS               |   1 +
>  drivers/iio/dac/Kconfig   |  10 +++
>  drivers/iio/dac/Makefile  |   1 +
>  drivers/iio/dac/ad5706r.c | 222 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 234 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 17a3d2d45fccb9cd3c93fd35666fb85d17d53cde..3d7bd98b4d1b55836e40687a9a3ac9f4935a8acb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1502,6 +1502,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/dac/adi,ad5706r.yaml
> +F:	drivers/iio/dac/ad5706r.c
>  
>  ANALOG DEVICES INC AD7091R DRIVER
>  M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index db9f5c711b3df90641f017652fbbef594cc1627d..8ccbdf6dfbca8640a47bf05b4afc6b4bf90a7e26 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -178,6 +178,16 @@ config AD5624R_SPI
>  	  Say yes here to build support for Analog Devices AD5624R, AD5644R and
>  	  AD5664R converters (DAC). This driver uses the common SPI interface.
>  
> +config AD5706R
> +	tristate "Analog Devices AD5706R DAC driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Analog Devices AD5706R 4-channel,
> +	  16-bit current output DAC.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ad5706r.
> +
>  config AD9739A
>  	tristate "Analog Devices AD9739A RF DAC spi driver"
>  	depends on SPI
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 2a80bbf4e80ad557da79ed916027cedff286984b..0034317984985035f7987a744899924bfd4612e3 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_AD5449) += ad5449.o
>  obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
>  obj-$(CONFIG_AD5592R) += ad5592r.o
>  obj-$(CONFIG_AD5593R) += ad5593r.o
> +obj-$(CONFIG_AD5706R) += ad5706r.o
>  obj-$(CONFIG_AD5755) += ad5755.o
>  obj-$(CONFIG_AD5758) += ad5758.o
>  obj-$(CONFIG_AD5761) += ad5761.o
> diff --git a/drivers/iio/dac/ad5706r.c b/drivers/iio/dac/ad5706r.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..840ee7b6db2e05ea35b27ff776f0c5c8a961d9c1
> --- /dev/null
> +++ b/drivers/iio/dac/ad5706r.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AD5706R 16-bit Current Output Digital to Analog Converter
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>
> +
> +/* SPI frame layout */
> +#define AD5706R_RD_MASK			BIT(15)
> +#define AD5706R_ADDR_MASK		GENMASK(11, 0)
> +
> +/* Registers */
> +#define AD5706R_REG_DAC_INPUT_A_CH(x)		(0x60 + ((x) * 2))
> +#define AD5706R_REG_DAC_DATA_READBACK_CH(x)	(0x68 + ((x) * 2))
> +
> +#define AD5706R_DAC_RESOLUTION		16
> +#define AD5706R_DAC_MAX_CODE		BIT(16)
> +#define AD5706R_MULTIBYTE_REG_START	0x14
> +#define AD5706R_MULTIBYTE_REG_END	0x71
> +#define AD5706R_SINGLE_BYTE_LEN		1
> +#define AD5706R_DOUBLE_BYTE_LEN		2
> +
> +struct ad5706r_state {
> +	struct spi_device *spi;
> +	struct mutex lock; /* Protects SPI transfers */
> +
> +	u8 tx_buf[4] __aligned(ARCH_DMA_MINALIGN);
> +	u8 rx_buf[2];
> +};
> +
> +static int ad5706r_reg_len(unsigned int reg)
> +{
> +	if (reg >= AD5706R_MULTIBYTE_REG_START && reg <= AD5706R_MULTIBYTE_REG_END)
> +		return AD5706R_DOUBLE_BYTE_LEN;
> +
> +	return AD5706R_SINGLE_BYTE_LEN;
> +}
> +
> +static int ad5706r_spi_write(struct ad5706r_state *st, u16 reg, u16 val)
> +{
> +	unsigned int num_bytes = ad5706r_reg_len(reg);
> +	struct spi_transfer xfer = {
> +		.tx_buf = st->tx_buf,
> +		.len = num_bytes + 2,
> +	};
> +
> +	put_unaligned_be16(reg, &st->tx_buf[0]);
> +
> +	if (num_bytes == 1)
> +		st->tx_buf[2] = val;
> +	else if (num_bytes == 2)
> +		put_unaligned_be16(val, &st->tx_buf[2]);
> +	else
> +		return -EINVAL;
> +
> +	return spi_sync_transfer(st->spi, &xfer, 1);
> +}
> +
> +static int ad5706r_spi_read(struct ad5706r_state *st, u16 reg, u16 *val)
> +{
> +	unsigned int num_bytes = ad5706r_reg_len(reg);
> +	u16 cmd;
> +	int ret;
> +
> +	struct spi_transfer xfer[] = {
> +		{
> +			.tx_buf = st->tx_buf,
> +			.len = 2,
> +		},
> +		{
> +			.rx_buf = st->rx_buf,
> +			.len = num_bytes,
> +		},
> +	};
> +
> +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> +	put_unaligned_be16(cmd, &st->tx_buf[0]);
> +
> +	ret = spi_sync_transfer(st->spi, xfer, ARRAY_SIZE(xfer));
> +	if (ret)
> +		return ret;
> +
> +	if (num_bytes == 1)
> +		*val = st->rx_buf[0];
> +	else if (num_bytes == 2)
> +		*val = get_unaligned_be16(st->rx_buf);
> +	else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int ad5706r_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +	u16 reg_val;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		scoped_guard(mutex, &st->lock) {
> +			ret = ad5706r_spi_read(st, AD5706R_REG_DAC_DATA_READBACK_CH(chan-
> >channel),
> +					       &reg_val);
> +
> +			if (ret)
> +				return ret;
> +
> +			*val = reg_val;
> +		}
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 50;
> +		*val2 = AD5706R_DAC_RESOLUTION;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ad5706r_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int val,
> +			     int val2, long mask)
> +{
> +	struct ad5706r_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (val < 0 || val >= AD5706R_DAC_MAX_CODE)
> +			return -EINVAL;
> +
> +		guard(mutex)(&st->lock);
> +		return ad5706r_spi_write(st,
> +					 AD5706R_REG_DAC_INPUT_A_CH(chan->channel),
> +					 val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info ad5706r_info = {
> +	.read_raw = ad5706r_read_raw,
> +	.write_raw = ad5706r_write_raw,
> +};
> +
> +#define AD5706R_CHAN(_channel) {				\
> +	.type = IIO_CURRENT,					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> +			      BIT(IIO_CHAN_INFO_SCALE),	\
> +	.output = 1,						\
> +	.indexed = 1,						\
> +	.channel = _channel,					\
> +}
> +
> +static const struct iio_chan_spec ad5706r_channels[] = {
> +	AD5706R_CHAN(0),
> +	AD5706R_CHAN(1),
> +	AD5706R_CHAN(2),
> +	AD5706R_CHAN(3),
> +};
> +
> +static int ad5706r_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ad5706r_state *st;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	ret = devm_mutex_init(&spi->dev, &st->lock);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->name = "ad5706r";
> +	indio_dev->info = &ad5706r_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ad5706r_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ad5706r_channels);
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct of_device_id ad5706r_of_match[] = {
> +	{ .compatible = "adi,ad5706r" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ad5706r_of_match);
> +
> +static const struct spi_device_id ad5706r_id[] = {
> +	{ "ad5706r" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, ad5706r_id);
> +
> +static struct spi_driver ad5706r_driver = {
> +	.driver = {
> +		.name = "ad5706r",
> +		.of_match_table = ad5706r_of_match,
> +	},
> +	.probe = ad5706r_probe,
> +	.id_table = ad5706r_id,
> +};
> +module_spi_driver(ad5706r_driver);
> +
> +MODULE_AUTHOR("Alexis Czezar Torreno <alexisczezar.torreno@analog.com>");
> +MODULE_DESCRIPTION("AD5706R 16-bit Current Output DAC driver");
> +MODULE_LICENSE("GPL");

      parent reply	other threads:[~2026-03-25 11:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-18  5:13 [PATCH v3 0/2] Add support for AD5706R DAC Alexis Czezar Torreno
2026-03-18  5:13 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add ADI AD5706R Alexis Czezar Torreno
2026-03-18  6:59   ` Krzysztof Kozlowski
2026-03-19  5:23     ` Torreno, Alexis Czezar
2026-03-18  5:13 ` [PATCH v3 2/2] iio: dac: ad5706r: Add support for AD5706R DAC Alexis Czezar Torreno
2026-03-18  8:16   ` Andy Shevchenko
2026-03-19  5:23     ` Torreno, Alexis Czezar
2026-03-19  7:07       ` Andy Shevchenko
2026-03-25  1:07         ` Torreno, Alexis Czezar
2026-03-25 10:13           ` Nuno Sá
2026-03-25 12:12           ` Andy Shevchenko
2026-03-27  9:01             ` Torreno, Alexis Czezar
2026-03-21 18:39   ` Jonathan Cameron
2026-03-25  1:22     ` Torreno, Alexis Czezar
2026-03-25  3:13       ` Torreno, Alexis Czezar
2026-03-25 10:14         ` Nuno Sá
2026-03-25 11:02   ` Nuno Sá [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1920a90a223c1394004d269895df54eee720125a.camel@gmail.com \
    --to=noname.nuno@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=alexisczezar.torreno@analog.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox