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@xxxxxxxxxx> wrote: > > > From: Nuno Sa <nuno.sa@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > 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)? - Nuno Sá >