From: Jonathan Cameron <jic23@kernel.org>
To: Mircea Caprioru <mircea.caprioru@analog.com>
Cc: <Michael.Hennerich@analog.com>, <lars@metafoo.de>,
<gregkh@linuxfoundation.org>, <linux-kernel@vger.kernel.org>,
<linux-iio@vger.kernel.org>
Subject: Re: [PATCH] iio: dac: ad5686: Add support for AD5674R/AD5679R
Date: Sat, 12 Jan 2019 18:57:49 +0000 [thread overview]
Message-ID: <20190112185749.15eb02e3@archlinux> (raw)
In-Reply-To: <20190109091416.12060-1-mircea.caprioru@analog.com>
On Wed, 9 Jan 2019 11:14:16 +0200
Mircea Caprioru <mircea.caprioru@analog.com> wrote:
> The AD5674R/AD5679R are low power, 16-channel, 12-/16-bit buffered voltage
> output digital-to-analog converters (DACs). They include a 2.5 V internal
> reference (enabled by default).
>
> These devices are very similar to AD5684R/AD5686R, except that they have 16
> channels.
>
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
Looks good to me.
One comment inline, but just ignore it ;)
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/dac/Kconfig | 6 +++---
> drivers/iio/dac/ad5686-spi.c | 7 ++++--
> drivers/iio/dac/ad5686.c | 42 ++++++++++++++++++++++++++++++++++--
> drivers/iio/dac/ad5686.h | 2 ++
> 4 files changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 851b61eaf3da..f28daf67db6a 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -148,9 +148,9 @@ config AD5686_SPI
> depends on SPI
> select AD5686
> help
> - Say yes here to build support for Analog Devices AD5672R, AD5676,
> - AD5676R, AD5684, AD5684R, AD5684R, AD5685R, AD5686, AD5686R.
> - Voltage Output Digital to Analog Converter.
> + Say yes here to build support for Analog Devices AD5672R, AD5674R,
> + AD5676, AD5676R, AD5679R, AD5684, AD5684R, AD5684R, AD5685R, AD5686,
> + AD5686R Voltage Output Digital to Analog Converter.
>
> To compile this driver as a module, choose M here: the
> module will be called ad5686.
> diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
> index 665fa6bd9ced..4d857c8da2d2 100644
> --- a/drivers/iio/dac/ad5686-spi.c
> +++ b/drivers/iio/dac/ad5686-spi.c
> @@ -1,7 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
> - * AD5672R, AD5676, AD5676R, AD5681R, AD5682R, AD5683, AD5683R,
> - * AD5684, AD5684R, AD5685R, AD5686, AD5686R
> + * AD5672R, AD5674R, AD5676, AD5676R, AD5679R,
> + * AD5681R, AD5682R, AD5683, AD5683R, AD5684,
> + * AD5684R, AD5685R, AD5686, AD5686R
This goes to show that listing the parts supported by the driver at the top
is more trouble than it's worth. The nice id tables tell us the same
thing and have much less churn ;)
oh well. I don't really care obviously.
> * Digital to analog converters driver
> *
> * Copyright 2018 Analog Devices Inc.
> @@ -102,8 +103,10 @@ static int ad5686_spi_remove(struct spi_device *spi)
> static const struct spi_device_id ad5686_spi_id[] = {
> {"ad5310r", ID_AD5310R},
> {"ad5672r", ID_AD5672R},
> + {"ad5674r", ID_AD5674R},
> {"ad5676", ID_AD5676},
> {"ad5676r", ID_AD5676R},
> + {"ad5679r", ID_AD5679R},
> {"ad5681r", ID_AD5681R},
> {"ad5682r", ID_AD5682R},
> {"ad5683", ID_AD5683},
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index a332b93ca2c4..6dd2759b9092 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -71,7 +71,7 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
> int ret;
> struct ad5686_state *st = iio_priv(indio_dev);
> unsigned int val, ref_bit_msk;
> - u8 shift;
> + u8 shift, address = 0;
>
> ret = strtobool(buf, &readin);
> if (ret)
> @@ -94,6 +94,9 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
> case AD5686_REGMAP:
> shift = 0;
> ref_bit_msk = 0;
> + /* AD5674R/AD5679R have 16 channels and 2 powerdown registers */
> + if (chan->channel > 0x7)
> + address = 0x8;
> break;
> case AD5693_REGMAP:
> shift = 13;
> @@ -107,7 +110,8 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev *indio_dev,
> if (!st->use_internal_vref)
> val |= ref_bit_msk;
>
> - ret = st->write(st, AD5686_CMD_POWERDOWN_DAC, 0, val);
> + ret = st->write(st, AD5686_CMD_POWERDOWN_DAC,
> + address, val >> (address * 2));
>
> return ret ? ret : len;
> }
> @@ -226,10 +230,32 @@ static struct iio_chan_spec name[] = { \
> AD5868_CHANNEL(7, 7, bits, _shift), \
> }
>
> +#define DECLARE_AD5679_CHANNELS(name, bits, _shift) \
> +static struct iio_chan_spec name[] = { \
> + AD5868_CHANNEL(0, 0, bits, _shift), \
> + AD5868_CHANNEL(1, 1, bits, _shift), \
> + AD5868_CHANNEL(2, 2, bits, _shift), \
> + AD5868_CHANNEL(3, 3, bits, _shift), \
> + AD5868_CHANNEL(4, 4, bits, _shift), \
> + AD5868_CHANNEL(5, 5, bits, _shift), \
> + AD5868_CHANNEL(6, 6, bits, _shift), \
> + AD5868_CHANNEL(7, 7, bits, _shift), \
> + AD5868_CHANNEL(8, 8, bits, _shift), \
> + AD5868_CHANNEL(9, 9, bits, _shift), \
> + AD5868_CHANNEL(10, 10, bits, _shift), \
> + AD5868_CHANNEL(11, 11, bits, _shift), \
> + AD5868_CHANNEL(12, 12, bits, _shift), \
> + AD5868_CHANNEL(13, 13, bits, _shift), \
> + AD5868_CHANNEL(14, 14, bits, _shift), \
> + AD5868_CHANNEL(15, 15, bits, _shift), \
> +}
> +
> DECLARE_AD5693_CHANNELS(ad5310r_channels, 10, 2);
> DECLARE_AD5693_CHANNELS(ad5311r_channels, 10, 6);
> DECLARE_AD5676_CHANNELS(ad5672_channels, 12, 4);
> +DECLARE_AD5679_CHANNELS(ad5674r_channels, 12, 4);
> DECLARE_AD5676_CHANNELS(ad5676_channels, 16, 0);
> +DECLARE_AD5679_CHANNELS(ad5679r_channels, 16, 0);
> DECLARE_AD5686_CHANNELS(ad5684_channels, 12, 4);
> DECLARE_AD5686_CHANNELS(ad5685r_channels, 14, 2);
> DECLARE_AD5686_CHANNELS(ad5686_channels, 16, 0);
> @@ -262,6 +288,12 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> .num_channels = 8,
> .regmap_type = AD5686_REGMAP,
> },
> + [ID_AD5674R] = {
> + .channels = ad5674r_channels,
> + .int_vref_mv = 2500,
> + .num_channels = 16,
> + .regmap_type = AD5686_REGMAP,
> + },
> [ID_AD5675R] = {
> .channels = ad5676_channels,
> .int_vref_mv = 2500,
> @@ -279,6 +311,12 @@ static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> .num_channels = 8,
> .regmap_type = AD5686_REGMAP,
> },
> + [ID_AD5679R] = {
> + .channels = ad5679r_channels,
> + .int_vref_mv = 2500,
> + .num_channels = 16,
> + .regmap_type = AD5686_REGMAP,
> + },
> [ID_AD5681R] = {
> .channels = ad5691r_channels,
> .int_vref_mv = 2500,
> diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
> index 19f6917d4738..4c3e171ce0cc 100644
> --- a/drivers/iio/dac/ad5686.h
> +++ b/drivers/iio/dac/ad5686.h
> @@ -54,9 +54,11 @@ enum ad5686_supported_device_ids {
> ID_AD5311R,
> ID_AD5671R,
> ID_AD5672R,
> + ID_AD5674R,
> ID_AD5675R,
> ID_AD5676,
> ID_AD5676R,
> + ID_AD5679R,
> ID_AD5681R,
> ID_AD5682R,
> ID_AD5683,
prev parent reply other threads:[~2019-01-12 18:57 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-09 9:14 [PATCH] iio: dac: ad5686: Add support for AD5674R/AD5679R Mircea Caprioru
2019-01-12 18:57 ` Jonathan Cameron [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=20190112185749.15eb02e3@archlinux \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=gregkh@linuxfoundation.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mircea.caprioru@analog.com \
/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;
as well as URLs for NNTP newsgroup(s).