On Mon, 17 Feb 2025 10:48:03 +0000
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:
> On Sun, 2025-02-16 at 13:00 -0600, David Lechner wrote:
> > On 2/16/25 12:19 PM, Jonathan Cameron wrote:
> > > On Sun, 9 Feb 2025 18:06:08 +0000
> > > Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> > >
> > > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > >
> > > > This complex cleanup.h use case of conditional guards has proved
> > > > to be more trouble that it is worth in terms of false positive compiler
> > > > warnings and hard to read code.
> > > >
> > > > Move directly to the new claim/release_direct() that allow sparse
> > > > to check for unbalanced context. In some cases code is factored
> > > > out to utility functions that can do a direct return with the
> > > > claim and release around the call.
> > > >
> > > > Reviewed-by: David Lechner <dlechner@xxxxxxxxxxxx>
> > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > > ---
> > > > v2: Typo in commit description (David).
> > > > Note there are several sets current in flight that touch this driver.
> > > > I'll rebase as necessary depending on what order the dependencies resolve.
> > > I've done this rebase and applied on the testing branch of iio.git.
> > >
> > > Would appreciate a sanity check if anyone has time though!
> > >
> > > New code is as follows. The one corner I was not sure on was
> > > that for calibbias reading the direct mode claim was held for a long
> > > time. That seems to be unnecessary as we have a copy of osr anyway
> > > in that function used for other purposes.
> > >
> >
> > ...
> >
> > > case IIO_CHAN_INFO_CALIBBIAS:
> > > - switch (chan->type) {
> > > - case IIO_VOLTAGE:
> > > - iio_device_claim_direct_scoped(return -EBUSY,
> > > indio_dev) {
> > > - ret = regmap_read(st->regmap16,
> > > - AD4695_REG_OFFSET_IN(chan-
> > > >scan_index),
> > > - ®_val);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - tmp = sign_extend32(reg_val, 15);
> > > -
> > > - switch (cfg->oversampling_ratio) {
> > > - case 1:
> > > - *val = tmp / 4;
> > > - *val2 = abs(tmp) % 4 * MICRO / 4;
> > > - break;
> > > - case 4:
> > > - *val = tmp / 2;
> > > - *val2 = abs(tmp) % 2 * MICRO / 2;
> > > - break;
> > > - case 16:
> > > - *val = tmp;
> > > - *val2 = 0;
> > > - break;
> > > - case 64:
> > > - *val = tmp * 2;
> > > - *val2 = 0;
> > > - break;
> > > - default:
> > > - return -EINVAL;
> > > - }
> > > -
> > > - if (tmp < 0 && *val2) {
> > > - *val *= -1;
> > > - *val2 *= -1;
> > > - }
> > > -
> > > - return IIO_VAL_INT_PLUS_MICRO;
> > > + switch (chan->type)
> > > + case IIO_VOLTAGE: {
> > > + if (!iio_device_claim_direct(indio_dev))
> > > + return -EBUSY;
> > > + ret = regmap_read(st->regmap16,
> > > + AD4695_REG_OFFSET_IN(chan-
> > > >scan_index),
> > > + ®_val);
> > > + iio_device_release_direct(indio_dev);
> > > + if (ret)
> > > + return ret;
> > > ////THIS IS THE BIT I WOuLD LIKE EYES on.
> >
> > Looks fine to me.
>
> +1
Thanks to you both for the quick replies.
Added Nuno's tags (and the fixlet on the first path - doh!)
Jonathan
>
> - Nuno Sá
> >
> > > +
> > > + tmp = sign_extend32(reg_val, 15);
> > > +
> > > + switch (osr) {
> > > + case 1:
> > > + *val = tmp / 4;
> > > + *val2 = abs(tmp) % 4 * MICRO / 4;
> > > + break;
> > > + case 4:
> > > + *val = tmp / 2;
> > > + *val2 = abs(tmp) % 2 * MICRO / 2;
> > > + break;
> > > + case 16:
> > > + *val = tmp;
> > > + *val2 = 0;
> > > + break;
> > > + case 64:
> > > + *val = tmp * 2;
> > > + *val2 = 0;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (tmp < 0 && *val2) {
> > > + *val *= -1;
> > > + *val2 *= -1;
> > > }
> > > - unreachable();
> > > +
> > > + return IIO_VAL_INT_PLUS_MICRO;
> > > default:
> > > return -EINVAL;
> > > }
>
