From: David Lechner <dlechner@baylibre.com>
To: Alisa-Dariana Roman <alisadariana@gmail.com>
Cc: michael.hennerich@analog.com, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
alexandru.tachici@analog.com, lars@metafoo.de, jic23@kernel.org,
robh@kernel.org, krzysztof.kozlowski+dt@linaro.org,
conor+dt@kernel.org, lgirdwood@gmail.com, broonie@kernel.org,
andy@kernel.org, nuno.sa@analog.com, marcelo.schmitt@analog.com,
bigunclemax@gmail.com, okan.sahin@analog.com,
fr0st61te@gmail.com, alisa.roman@analog.com,
marcus.folkesson@gmail.com, schnelle@linux.ibm.com,
liambeguin@gmail.com
Subject: Re: [PATCH v7 6/6] iio: adc: ad7192: Add AD7194 support
Date: Wed, 1 May 2024 12:54:20 -0500 [thread overview]
Message-ID: <CAMknhBFAM-fvb1t5b6dn1tkDjP4QfTT7uo+-PMfAQyE8KaFbVw@mail.gmail.com> (raw)
In-Reply-To: <20240430162946.589423-7-alisa.roman@analog.com>
On Tue, Apr 30, 2024 at 11:30 AM Alisa-Dariana Roman
<alisadariana@gmail.com> wrote:
>
> Unlike the other AD719Xs, AD7194 has configurable channels. The user can
> dynamically configure them in the devicetree.
>
> Also modify config AD7192 description for better scaling.
>
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
> drivers/iio/adc/Kconfig | 11 +++-
> drivers/iio/adc/ad7192.c | 129 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 133 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 8db68b80b391..74fecc284f1a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -88,12 +88,17 @@ config AD7173
> called ad7173.
>
> config AD7192
> - tristate "Analog Devices AD7190 AD7192 AD7193 AD7195 ADC driver"
> + tristate "Analog Devices AD7192 and similar ADC driver"
> depends on SPI
> select AD_SIGMA_DELTA
> help
> - Say yes here to build support for Analog Devices AD7190,
> - AD7192, AD7193 or AD7195 SPI analog to digital converters (ADC).
> + Say yes here to build support for Analog Devices SPI analog to digital
> + converters (ADC):
> + - AD7190
> + - AD7192
> + - AD7193
> + - AD7194
> + - AD7195
> If unsure, say N (but it's safe to say "Y").
>
> To compile this driver as a module, choose M here: the
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 3e797ff48086..0f6ecf953559 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * AD7190 AD7192 AD7193 AD7195 SPI ADC driver
> + * AD7192 and similar SPI ADC driver
> *
> * Copyright 2011-2015 Analog Devices Inc.
> */
> @@ -129,10 +129,21 @@
> #define AD7193_CH_AIN8 0x480 /* AIN7 - AINCOM */
> #define AD7193_CH_AINCOM 0x600 /* AINCOM - AINCOM */
>
> +#define AD7194_CH_POS(x) (((x) - 1) << 4)
> +#define AD7194_CH_NEG(x) ((x) - 1)
> +#define AD7194_CH(pos, neg) \
> + (((neg) == 0 ? BIT(10) : AD7194_CH_NEG(neg)) | AD7194_CH_POS(pos))
I think this needs a comment that BIT(10) is the CON18 flag for
pseudo-differential channels.
Also, it would probably be easier to understand if there were two
different macros, one for "single-channel" and one for "diff-channels"
rather than having neg==0 magically turn in to the pseudo flag.
> +#define AD7194_CH_TEMP 0x100 /* Temp sensor */
> +#define AD7194_CH_BASE_NR 2
> +#define AD7194_CH_AIN_START 1
> +#define AD7194_CH_AIN_NR 16
> +#define AD7194_CH_MAX_NR 272
> +
> /* ID Register Bit Designations (AD7192_REG_ID) */
> #define CHIPID_AD7190 0x4
> #define CHIPID_AD7192 0x0
> #define CHIPID_AD7193 0x2
> +#define CHIPID_AD7194 0x3
> #define CHIPID_AD7195 0x6
> #define AD7192_ID_MASK GENMASK(3, 0)
>
> @@ -170,6 +181,7 @@ enum {
> ID_AD7190,
> ID_AD7192,
> ID_AD7193,
> + ID_AD7194,
> ID_AD7195,
> };
>
> @@ -179,6 +191,7 @@ struct ad7192_chip_info {
> const struct iio_chan_spec *channels;
> u8 num_channels;
> const struct iio_info *info;
> + int (*parse_channels)(struct iio_dev *indio_dev);
> };
>
> struct ad7192_state {
> @@ -932,6 +945,15 @@ static const struct iio_info ad7192_info = {
> .update_scan_mode = ad7192_update_scan_mode,
> };
>
> +static const struct iio_info ad7194_info = {
> + .read_raw = ad7192_read_raw,
> + .write_raw = ad7192_write_raw,
> + .write_raw_get_fmt = ad7192_write_raw_get_fmt,
> + .read_avail = ad7192_read_avail,
> + .validate_trigger = ad_sd_validate_trigger,
> + .update_scan_mode = ad7192_update_scan_mode,
It looks like ad7192_update_scan_mode() won't work on ad7194 since the
channels are specified via DT for this chip and could be anything.
Each scan_index no longer corresponds a specific bit in
AD7192_REG_CONF so it would end up with some random configuration.
> +};
> +
The rest looks fine other than what Andy mentioned already.
next prev parent reply other threads:[~2024-05-01 17:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-30 16:29 [PATCH v7 0/6] iio: adc: ad7192: Add AD7194 support Alisa-Dariana Roman
2024-04-30 16:29 ` [PATCH v7 1/6] iio: adc: ad7192: Use standard attribute Alisa-Dariana Roman
2024-04-30 16:29 ` [PATCH v7 2/6] dt-bindings: iio: adc: ad7192: Add aincom supply Alisa-Dariana Roman
2024-04-30 16:29 ` [PATCH v7 3/6] " Alisa-Dariana Roman
2024-04-30 20:28 ` Andy Shevchenko
2024-05-01 16:54 ` David Lechner
2024-04-30 16:29 ` [PATCH v7 4/6] dt-bindings: iio: adc: Add single-channel property Alisa-Dariana Roman
2024-04-30 17:15 ` Conor Dooley
2024-04-30 16:29 ` [PATCH v7 5/6] dt-bindings: iio: adc: ad7192: Add AD7194 support Alisa-Dariana Roman
2024-04-30 17:21 ` Conor Dooley
2024-05-05 19:46 ` Jonathan Cameron
2024-05-06 15:54 ` Conor Dooley
2024-05-10 10:05 ` Alisa-Dariana Roman
2024-05-10 14:21 ` David Lechner
2024-05-10 21:26 ` Conor Dooley
2024-05-11 11:29 ` Jonathan Cameron
2024-04-30 17:26 ` Conor Dooley
2024-04-30 16:29 ` [PATCH v7 6/6] " Alisa-Dariana Roman
2024-04-30 20:33 ` Andy Shevchenko
2024-05-01 17:54 ` David Lechner [this message]
2024-05-06 13:39 ` Jonathan Cameron
2024-04-30 17:19 ` [PATCH v7 0/6] " Conor Dooley
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=CAMknhBFAM-fvb1t5b6dn1tkDjP4QfTT7uo+-PMfAQyE8KaFbVw@mail.gmail.com \
--to=dlechner@baylibre.com \
--cc=alexandru.tachici@analog.com \
--cc=alisa.roman@analog.com \
--cc=alisadariana@gmail.com \
--cc=andy@kernel.org \
--cc=bigunclemax@gmail.com \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=fr0st61te@gmail.com \
--cc=jic23@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=liambeguin@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcelo.schmitt@analog.com \
--cc=marcus.folkesson@gmail.com \
--cc=michael.hennerich@analog.com \
--cc=nuno.sa@analog.com \
--cc=okan.sahin@analog.com \
--cc=robh@kernel.org \
--cc=schnelle@linux.ibm.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 read-only IMAP folder(s) and NNTP newsgroup(s).