On Sun, 2025-02-09 at 18:06 +0000, Jonathan Cameron 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.
> ---
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> drivers/iio/adc/ad4695.c | 240 ++++++++++++++++++++++-----------------
> 1 file changed, 133 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index 13cf01d35301..4bb22f4d739b 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -738,6 +738,25 @@ static int ad4695_read_one_sample(struct ad4695_state
> *st, unsigned int address)
> return spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> }
>
> +static int __ad4695_read_info_raw(struct ad4695_state *st,
> + struct iio_chan_spec const *chan,
> + int *val)
> +{
> + u8 realbits = chan->scan_type.realbits;
> + int ret;
> +
> + ret = ad4695_read_one_sample(st, chan->address);
> + if (ret)
> + return ret;
> +
> + if (chan->scan_type.sign == 's')
> + *val = sign_extend32(st->raw_data, realbits - 1);
> + else
> + *val = st->raw_data;
> +
> + return IIO_VAL_INT;
> +}
> +
> static int ad4695_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -750,19 +769,12 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - ret = ad4695_read_one_sample(st, chan->address);
> - if (ret)
> - return ret;
> -
> - if (chan->scan_type.sign == 's')
> - *val = sign_extend32(st->raw_data, realbits -
> 1);
> - else
> - *val = st->raw_data;
>
> - return IIO_VAL_INT;
> - }
> - unreachable();
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = __ad4695_read_info_raw(st, chan, val);
> + iio_device_release_direct(indio_dev);
> + return ret;
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
> case IIO_VOLTAGE:
> @@ -800,45 +812,45 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_CALIBSCALE:
> switch (chan->type) {
> case IIO_VOLTAGE:
> - iio_device_claim_direct_scoped(return -EBUSY,
> indio_dev) {
> - ret = regmap_read(st->regmap16,
> - AD4695_REG_GAIN_IN(chan->scan_index),
> - ®_val);
> - if (ret)
> - return ret;
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = regmap_read(st->regmap16,
> + AD4695_REG_GAIN_IN(chan-
> >scan_index),
> + ®_val);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
>
> - *val = reg_val;
> - *val2 = 15;
> + *val = reg_val;
> + *val2 = 15;
>
> - return IIO_VAL_FRACTIONAL_LOG2;
> - }
> - unreachable();
> + return IIO_VAL_FRACTIONAL_LOG2;
> default:
> return -EINVAL;
> }
> 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);
> + 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;
>
> - *val = tmp / 4;
> - *val2 = abs(tmp) % 4 * MICRO / 4;
> + tmp = sign_extend32(reg_val, 15);
>
> - if (tmp < 0 && *val2) {
> - *val *= -1;
> - *val2 *= -1;
> - }
> + *val = tmp / 4;
> + *val2 = abs(tmp) % 4 * MICRO / 4;
>
> - return IIO_VAL_INT_PLUS_MICRO;
> + if (tmp < 0 && *val2) {
> + *val *= -1;
> + *val2 *= -1;
> }
> - unreachable();
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> default:
> return -EINVAL;
> }
> @@ -847,64 +859,75 @@ static int ad4695_read_raw(struct iio_dev *indio_dev,
> }
> }
>
> -static int ad4695_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val, int val2, long mask)
> +static int __ad4695_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> {
> struct ad4695_state *st = iio_priv(indio_dev);
> unsigned int reg_val;
>
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - switch (mask) {
> - case IIO_CHAN_INFO_CALIBSCALE:
> - switch (chan->type) {
> - case IIO_VOLTAGE:
> - if (val < 0 || val2 < 0)
> - reg_val = 0;
> - else if (val > 1)
> - reg_val = U16_MAX;
> - else
> - reg_val = (val * (1 << 16) +
> - mul_u64_u32_div(val2, 1 <<
> 16,
> - MICRO)) /
> 2;
> -
> - return regmap_write(st->regmap16,
> - AD4695_REG_GAIN_IN(chan->scan_index),
> - reg_val);
> - default:
> - return -EINVAL;
> - }
> - case IIO_CHAN_INFO_CALIBBIAS:
> - switch (chan->type) {
> - case IIO_VOLTAGE:
> - if (val2 >= 0 && val > S16_MAX / 4)
> - reg_val = S16_MAX;
> - else if ((val2 < 0 ? -val : val) < S16_MIN /
> 4)
> - reg_val = S16_MIN;
> - else if (val2 < 0)
> - reg_val = clamp_t(int,
> - -(val * 4 + -val2 * 4 /
> MICRO),
> - S16_MIN, S16_MAX);
> - else if (val < 0)
> - reg_val = clamp_t(int,
> - val * 4 - val2 * 4 / MICRO,
> - S16_MIN, S16_MAX);
> - else
> - reg_val = clamp_t(int,
> - val * 4 + val2 * 4 / MICRO,
> - S16_MIN, S16_MAX);
> -
> - return regmap_write(st->regmap16,
> - AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> - reg_val);
> - default:
> - return -EINVAL;
> - }
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBSCALE:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + if (val < 0 || val2 < 0)
> + reg_val = 0;
> + else if (val > 1)
> + reg_val = U16_MAX;
> + else
> + reg_val = (val * (1 << 16) +
> + mul_u64_u32_div(val2, 1 << 16,
> + MICRO)) / 2;
> +
> + return regmap_write(st->regmap16,
> + AD4695_REG_GAIN_IN(chan-
> >scan_index),
> + reg_val);
> + default:
> + return -EINVAL;
> + }
> + case IIO_CHAN_INFO_CALIBBIAS:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + if (val2 >= 0 && val > S16_MAX / 4)
> + reg_val = S16_MAX;
> + else if ((val2 < 0 ? -val : val) < S16_MIN / 4)
> + reg_val = S16_MIN;
> + else if (val2 < 0)
> + reg_val = clamp_t(int,
> + -(val * 4 + -val2 * 4 /
> MICRO),
> + S16_MIN, S16_MAX);
> + else if (val < 0)
> + reg_val = clamp_t(int,
> + val * 4 - val2 * 4 / MICRO,
> + S16_MIN, S16_MAX);
> + else
> + reg_val = clamp_t(int,
> + val * 4 + val2 * 4 / MICRO,
> + S16_MIN, S16_MAX);
> +
> + return regmap_write(st->regmap16,
> + AD4695_REG_OFFSET_IN(chan-
> >scan_index),
> + reg_val);
> default:
> return -EINVAL;
> }
> + default:
> + return -EINVAL;
> }
> - unreachable();
> +}
> +
> +static int ad4695_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + int ret;
> +
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = __ad4695_write_raw(indio_dev, chan, val, val2, mask);
> + iio_device_release_direct(indio_dev);
> +
> + return ret;
> }
>
> static int ad4695_read_avail(struct iio_dev *indio_dev,
> @@ -954,26 +977,29 @@ static int ad4695_debugfs_reg_access(struct iio_dev
> *indio_dev,
> unsigned int *readval)
> {
> struct ad4695_state *st = iio_priv(indio_dev);
> -
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - if (readval) {
> - if (regmap_check_range_table(st->regmap, reg,
> -
> &ad4695_regmap_rd_table))
> - return regmap_read(st->regmap, reg, readval);
> - if (regmap_check_range_table(st->regmap16, reg,
> -
> &ad4695_regmap16_rd_table))
> - return regmap_read(st->regmap16, reg,
> readval);
> - } else {
> - if (regmap_check_range_table(st->regmap, reg,
> -
> &ad4695_regmap_wr_table))
> - return regmap_write(st->regmap, reg,
> writeval);
> - if (regmap_check_range_table(st->regmap16, reg,
> -
> &ad4695_regmap16_wr_table))
> - return regmap_write(st->regmap16, reg,
> writeval);
> - }
> + int ret = -EINVAL;
> +
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + if (readval) {
> + if (regmap_check_range_table(st->regmap, reg,
> + &ad4695_regmap_rd_table))
> + ret = regmap_read(st->regmap, reg, readval);
> + if (regmap_check_range_table(st->regmap16, reg,
> + &ad4695_regmap16_rd_table))
> + ret = regmap_read(st->regmap16, reg, readval);
> + } else {
> + if (regmap_check_range_table(st->regmap, reg,
> + &ad4695_regmap_wr_table))
> + ret = regmap_write(st->regmap, reg, writeval);
> + if (regmap_check_range_table(st->regmap16, reg,
> + &ad4695_regmap16_wr_table))
> + ret = regmap_write(st->regmap16, reg, writeval);
> }
> + iio_device_release_direct(indio_dev);
>
> - return -EINVAL;
> + return ret;
> }
>
> static const struct iio_info ad4695_info = {
