From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <noname.nuno@gmail.com>
Cc: Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org>,
nuno.sa@analog.com, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Alexandru Ardelean <alexandru.ardelean@analog.com>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
Olivier Moysan <olivier.moysan@foss.st.com>
Subject: Re: [PATCH v2 7/7] iio: adc: ad9467: support digital interface calibration
Date: Mon, 29 Apr 2024 20:51:03 +0100 [thread overview]
Message-ID: <20240429205103.69d5388b@jic23-huawei> (raw)
In-Reply-To: <b988e9b88eb7589ed38a5f50a2b1ad10007fa1ce.camel@gmail.com>
On Mon, 29 Apr 2024 09:24:21 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Sun, 2024-04-28 at 18:32 +0100, Jonathan Cameron wrote:
> > On Fri, 26 Apr 2024 17:42:16 +0200
> > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> >
> > > From: Nuno Sa <nuno.sa@analog.com>
> > >
> > > To make sure that we have the best timings on the serial data interface
> > > we should calibrate it. This means going through the device supported
> > > values and see for which ones we get a successful result. To do that, we
> > > use a prbs test pattern both in the IIO backend and in the frontend
> > > devices. Then for each of the test points we see if there are any
> > > errors. Note that the backend is responsible to look for those errors.
> > >
> > > As calibrating the interface also requires that the data format is disabled
> > > (the one thing being done in ad9467_setup()), ad9467_setup() was removed
> > > and configuring the data fomat is now part of the calibration process.
> > >
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> >
> > One trivial comment.
> >
> > I'd have picked up the whole series, but it feels too big to do on a Sunday
> > when you only posted on Friday. Hence, lets let it sit for at least
> > a few more days to see if others have comments.
>
> Yeah, I kind of waited till the last moment to see if you had any important
> comment (on the first version open discussions) that could affect v2 :).
> >
> > It might not make this cycle as a result. I've picked up the 2 fixes
> > today to increase the chances those make it.
> >
> > Jonathan
> >
> >
> > > static int ad9467_read_raw(struct iio_dev *indio_dev,
> > > struct iio_chan_spec const *chan,
> > > int *val, int *val2, long m)
> > > @@ -345,7 +606,9 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
> > > {
> > > struct ad9467_state *st = iio_priv(indio_dev);
> > > const struct ad9467_chip_info *info = st->info;
> > > + unsigned long sample_rate;
> > > long r_clk;
> > > + int ret;
> > >
> > > switch (mask) {
> > > case IIO_CHAN_INFO_SCALE:
> > > @@ -358,7 +621,23 @@ static int ad9467_write_raw(struct iio_dev *indio_dev,
> > > return -EINVAL;
> > > }
> > >
> > > - return clk_set_rate(st->clk, r_clk);
> > > + sample_rate = clk_get_rate(st->clk);
> > > + /*
> > > + * clk_set_rate() would also do this but since we would
> > > still
> > > + * need it for avoiding an unnecessary calibration, do it
> > > now.
> > > + */
> > > + if (sample_rate == r_clk)
> > > + return 0;
> > > +
> > > + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> > > + ret = clk_set_rate(st->clk, r_clk);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + guard(mutex)(&st->lock);
> > > + ret = ad9467_calibrate(st);
> > return ad9467_calibrate(st);
> > > + }
> > unreachable();
> >
> > not totally elegant but I think the early return makes more sense and we
> > should
> > just use an unreachable() to squash the resulting compiler warning.
> >
>
> As you might remember I'm not a big fan of the unreachable() but also no strong
> feelings about it :). Do you expect a v3 for this or is this something you can
> fix up while applying (assuming no other things pop by)?
I changed my mind and didn't bother adjusting this.
I've queued this up and pushed it out as testing on basis I can always drop
it again if reviews come in within the next 2-3 days, but in meantime I can
let 0-day have at it.
Jonathan
>
> - Nuno Sá
> >
prev parent reply other threads:[~2024-04-29 19:51 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 15:42 [PATCH v2 0/7] iio: ad9467: support interface tuning Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 1/7] iio: backend: change docs padding Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 2/7] iio: backend: add API for interface tuning Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 3/7] dt-bindings: adc: axi-adc: add clocks property Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 4/7] iio: adc: axi-adc: make sure AXI clock is enabled Nuno Sa via B4 Relay
2024-04-28 17:31 ` Jonathan Cameron
2024-04-26 15:42 ` [PATCH v2 5/7] iio: adc: adi-axi-adc: remove regmap max register Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 6/7] iio: adc: adi-axi-adc: support digital interface calibration Nuno Sa via B4 Relay
2024-04-26 15:42 ` [PATCH v2 7/7] iio: adc: ad9467: " Nuno Sa via B4 Relay
2024-04-28 17:32 ` Jonathan Cameron
2024-04-29 7:24 ` Nuno Sá
2024-04-29 19:51 ` 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=20240429205103.69d5388b@jic23-huawei \
--to=jic23@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=Michael.Hennerich@analog.com \
--cc=alexandru.ardelean@analog.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=devnull+nuno.sa.analog.com@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=noname.nuno@gmail.com \
--cc=nuno.sa@analog.com \
--cc=olivier.moysan@foss.st.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;
as well as URLs for NNTP newsgroup(s).