On Sun, 2024-01-28 at 15:05 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> Switching to the iio_device_claim_direct_scoped() for state
> and to guard() based unlocking of mutexes simplifies error handling
> by allowing direct returns when an error is encountered.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> Since RFC:
> - Use unreachable() to stop complier moaning if we have
>
> iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> return 0;
> }
> unreachable(); /* can't get here */
> - Update some code from earlier attempt to handle this that was left
> behind from before iio_device_claim_direct_scoped()
> - Reduce scope of some local variables only used within
> iio_device_claim_direct_scoped() blocks.
> ---
> drivers/iio/accel/adxl367.c | 297 ++++++++++++++----------------------
> 1 file changed, 118 insertions(+), 179 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index 90b7ae6d42b7..834ee6d63947 100644
> --- a/drivers/iio/accel/adxl367.c
> +++ b/drivers/iio/accel/adxl367.c
> @@ -339,22 +339,17 @@ static int adxl367_set_act_threshold(struct
> adxl367_state *st,
> {
> int ret;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
>
> ret = adxl367_set_measure_en(st, false);
> if (ret)
> - goto out;
> + return ret;
>
> ret = _adxl367_set_act_threshold(st, act, threshold);
> if (ret)
> - goto out;
> -
> - ret = adxl367_set_measure_en(st, true);
> -
> -out:
> - mutex_unlock(&st->lock);
> + return ret;
>
> - return ret;
> + return adxl367_set_measure_en(st, true);
> }
>
> static int adxl367_set_act_proc_mode(struct adxl367_state *st,
> @@ -482,51 +477,45 @@ static int adxl367_set_fifo_watermark(struct
> adxl367_state *st,
> static int adxl367_set_range(struct iio_dev *indio_dev,
> enum adxl367_range range)
> {
> - struct adxl367_state *st = iio_priv(indio_dev);
> - int ret;
> + iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> + struct adxl367_state *st = iio_priv(indio_dev);
> + int ret;
>
> - ret = iio_device_claim_direct_mode(indio_dev);
> - if (ret)
> - return ret;
> -
> - mutex_lock(&st->lock);
> -
> - ret = adxl367_set_measure_en(st, false);
> - if (ret)
> - goto out;
> + guard(mutex)(&st->lock);
>
> - ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
> - ADXL367_FILTER_CTL_RANGE_MASK,
> - FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
> - range));
> - if (ret)
> - goto out;
> + ret = adxl367_set_measure_en(st, false);
> + if (ret)
> + return ret;
>
> - adxl367_scale_act_thresholds(st, st->range, range);
> + ret = regmap_update_bits(st->regmap, ADXL367_REG_FILTER_CTL,
> + ADXL367_FILTER_CTL_RANGE_MASK,
> +
> FIELD_PREP(ADXL367_FILTER_CTL_RANGE_MASK,
> + range));
> + if (ret)
> + return ret;
>
> - /* Activity thresholds depend on range */
> - ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> - st->act_threshold);
> - if (ret)
> - goto out;
> + adxl367_scale_act_thresholds(st, st->range, range);
>
> - ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> - st->inact_threshold);
> - if (ret)
> - goto out;
> -
> - ret = adxl367_set_measure_en(st, true);
> - if (ret)
> - goto out;
> + /* Activity thresholds depend on range */
> + ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> + st->act_threshold);
> + if (ret)
> + return ret;
>
> - st->range = range;
> + ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> + st->inact_threshold);
> + if (ret)
> + return ret;
>
> -out:
> - mutex_unlock(&st->lock);
> + ret = adxl367_set_measure_en(st, true);
> + if (ret)
> + return ret;
>
> - iio_device_release_direct_mode(indio_dev);
> + st->range = range;
>
> - return ret;
> + return 0;
> + }
> + unreachable();
> }
I do agree this is irritating. Personally I would prefer to return 0 (or the
last ret value) instead of the unusual unreachable() builtin. But that's me :)
- Nuno Sá
