On Tue, 24 Dec 2024 10:34:30 +0100
Julien Stephan <jstephan@xxxxxxxxxxxx> wrote:
> Rollback and remove the scoped version of iio_dvice_claim_direct_mode.
>
> Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx>
> ---
> drivers/iio/adc/ad7380.c | 89 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 53 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index 4f32cb22f140442b831dc9a4f275e88e4ab2388e..4e26e0e7ac1d5a1c4c67118dbc34f2921fc171a4 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -675,15 +675,21 @@ static const struct regmap_config ad7380_regmap_config = {
> static int ad7380_debugfs_reg_access(struct iio_dev *indio_dev, u32 reg,
> u32 writeval, u32 *readval)
> {
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - struct ad7380_state *st = iio_priv(indio_dev);
> + struct ad7380_state *st = iio_priv(indio_dev);
> + int ret;
>
> - if (readval)
> - return regmap_read(st->regmap, reg, readval);
> - else
> - return regmap_write(st->regmap, reg, writeval);
> - }
> - unreachable();
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + if (readval)
> + ret = regmap_read(st->regmap, reg, readval);
> + else
> + ret = regmap_write(st->regmap, reg, writeval);
> +
> + iio_device_release_direct_mode(indio_dev);
> +
> + return ret;
> }
>
> /*
> @@ -920,6 +926,7 @@ static int ad7380_read_raw(struct iio_dev *indio_dev,
> {
> struct ad7380_state *st = iio_priv(indio_dev);
> const struct iio_scan_type *scan_type;
> + int ret;
>
> scan_type = iio_get_current_scan_type(indio_dev, chan);
>
> @@ -928,11 +935,16 @@ static int ad7380_read_raw(struct iio_dev *indio_dev,
>
> switch (info) {
> case IIO_CHAN_INFO_RAW:
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - return ad7380_read_direct(st, chan->scan_index,
> - scan_type, val);
> - }
> - unreachable();
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> +
> + ret = ad7380_read_direct(st, chan->scan_index,
> + scan_type, val);
> +
> + iio_device_release_direct_mode(indio_dev);
> +
> + return ret;
> case IIO_CHAN_INFO_SCALE:
> /*
> * According to the datasheet, the LSB size is:
> @@ -1024,31 +1036,36 @@ static int ad7380_write_raw(struct iio_dev *indio_dev,
> /* always enable resolution boost when oversampling is enabled */
> boost = osr > 0 ? 1 : 0;
>
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - ret = regmap_update_bits(st->regmap,
> - AD7380_REG_ADDR_CONFIG1,
> - AD7380_CONFIG1_OSR | AD7380_CONFIG1_RES,
> - FIELD_PREP(AD7380_CONFIG1_OSR, osr) |
> - FIELD_PREP(AD7380_CONFIG1_RES, boost));
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
>
> - if (ret)
> - return ret;
> + ret = regmap_update_bits(st->regmap,
> + AD7380_REG_ADDR_CONFIG1,
> + AD7380_CONFIG1_OSR | AD7380_CONFIG1_RES,
> + FIELD_PREP(AD7380_CONFIG1_OSR, osr) |
> + FIELD_PREP(AD7380_CONFIG1_RES, boost));
>
> - st->oversampling_ratio = val;
> - st->resolution_boost_enabled = boost;
> -
> - /*
> - * Perform a soft reset. This will flush the oversampling
> - * block and FIFO but will maintain the content of the
> - * configurable registers.
> - */
> - return regmap_update_bits(st->regmap,
> - AD7380_REG_ADDR_CONFIG2,
> - AD7380_CONFIG2_RESET,
> - FIELD_PREP(AD7380_CONFIG2_RESET,
> - AD7380_CONFIG2_RESET_SOFT));
> - }
> - unreachable();
> + if (ret)
> + goto err;
> +
> + st->oversampling_ratio = val;
> + st->resolution_boost_enabled = boost;
> +
> + /*
> + * Perform a soft reset. This will flush the oversampling
> + * block and FIFO but will maintain the content of the
> + * configurable registers.
> + */
> + ret = regmap_update_bits(st->regmap,
> + AD7380_REG_ADDR_CONFIG2,
> + AD7380_CONFIG2_RESET,
> + FIELD_PREP(AD7380_CONFIG2_RESET,
> + AD7380_CONFIG2_RESET_SOFT));
> +err:
Labels within switch statements can become hard to maintainer / read.
Id' suggest factoring out the bits between claim and release as a single
__ad7380_write_oversample() or something along those lines.
Otherwise looks good to me and thanks for doing this.
Jonathan
> + iio_device_release_direct_mode(indio_dev);
> +
> + return ret;
> default:
> return -EINVAL;
> }
>
