From: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
Cc: "Nuno Sá" <noname.nuno@gmail.com>,
"Michael Hennerich" <michael.hennerich@analog.com>,
"Nuno Sá" <nuno.sa@analog.com>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: adc: ad7944: Add support for "3-wire mode"
Date: Thu, 14 Mar 2024 15:41:37 +0000 [thread overview]
Message-ID: <20240314154137.2fdf82ba@jic23-huawei> (raw)
In-Reply-To: <CAMknhBE8OwrtbJ9xYVZ8ObsZTnxmn9Fpk2a-gj1aCSaN-whDRg@mail.gmail.com>
On Tue, 12 Mar 2024 09:13:56 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On Tue, Mar 12, 2024 at 4:08 AM Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > On Mon, 2024-03-11 at 16:26 -0500, David Lechner wrote:
>
> ...
>
> > > /*
> > > * ad7944_4wire_mode_conversion - Perform a 4-wire mode conversion and acquisition
> > > * @adc: The ADC device structure
> > > @@ -167,9 +246,22 @@ static int ad7944_single_conversion(struct ad7944_adc *adc,
> > > {
> > > int ret;
> > >
> > > - ret = ad7944_4wire_mode_conversion(adc, chan);
> > > - if (ret)
> > > - return ret;
> > > + switch (adc->spi_mode) {
> > > + case AD7944_SPI_MODE_DEFAULT:
> > > + ret = ad7944_4wire_mode_conversion(adc, chan);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + break;
> > > + case AD7944_SPI_MODE_SINGLE:
> > > + ret = ad7944_3wire_cs_mode_conversion(adc, chan);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + break;
> > > + case AD7944_SPI_MODE_CHAIN:
> > > + return -EOPNOTSUPP;
> >
> > This mode is not really supported for now and in theory we can't really have
> > adc->spi_mode = AD7944_SPI_MODE_CHAIN, right? So, I would just make this the
> > 'default' branch and not care about chain mode (implementing it when adding it).
>
> The compiler was happy with this, but yeah, default: is probably safer.
>
> ...
>
> > > + if (!adc->cnv && adc->spi_mode == AD7944_SPI_MODE_DEFAULT)
> > > + return dev_err_probe(&spi->dev, -EINVAL, "CNV GPIO is
> > > required\n");
> > > + else if (adc->cnv && adc->spi_mode != AD7944_SPI_MODE_DEFAULT)
> > > + return dev_err_probe(&spi->dev, -EINVAL,
> > > + "CNV GPIO in single and chain mode is not
> > > currently supported\n");
> > > +
> >
> > Redundant else...
>
> yup
>
> >
> > > adc->turbo = devm_gpiod_get_optional(dev, "turbo", GPIOD_OUT_LOW);
> > > if (IS_ERR(adc->turbo))
> > > return dev_err_probe(dev, PTR_ERR(adc->turbo),
> > > @@ -369,6 +486,10 @@ static int ad7944_probe(struct spi_device *spi)
> > > return dev_err_probe(dev, -EINVAL,
> > > "cannot have both turbo-gpios and adi,always-turbo\n");
> > >
> > > + if (adc->spi_mode == AD7944_SPI_MODE_CHAIN && adc->always_turbo)
> > > + return dev_err_probe(dev, -EINVAL,
> > > + "cannot have both chain mode and always turbo\n");
> > > +
> >
> >
> > I'm fine in having this now but shouldn't we only have the above when we do support
> > chain mode? A bit odd having it when we don't even allow chain mode.
> >
>
> Yeah, we could wait to add this. It seemed like something easy to
> overlook though if we don't add chain mode right away, so I just went
> ahead and added it now.
Seems reasonable - maybe just mention it in the patch description.
Other than the tidying up Nuno pointed out looks good to me, so I'll pick up
v2 once posted.
Jonathan
prev parent reply other threads:[~2024-03-14 15:41 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-11 21:26 [PATCH] iio: adc: ad7944: Add support for "3-wire mode" David Lechner
2024-03-12 9:08 ` Nuno Sá
2024-03-12 14:13 ` David Lechner
2024-03-14 15:41 ` 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=20240314154137.2fdf82ba@jic23-huawei \
--to=jic23@kernel.org \
--cc=dlechner@baylibre.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael.hennerich@analog.com \
--cc=noname.nuno@gmail.com \
--cc=nuno.sa@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 read-only IMAP folder(s) and NNTP newsgroup(s).