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 there is a convenient wrapper function to which
> the handling can be moved. Do that instead of introducing
> another layer of wrappers. In others an outer wrapper is added
> which claims direct mode, runs the original function with the
> scoped claim logic removed, releases direct mode and then checks
> for errors.
>
> Cc: Cosmin Tanislav <demonsingur@xxxxxxxxx>
> Reviewed-by: David Lechner <dlechner@xxxxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
Reviewed-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> drivers/iio/accel/adxl367.c | 194 ++++++++++++++++++++----------------
> 1 file changed, 106 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index a48ac0d7bd96..add4053e7a02 100644
> --- a/drivers/iio/accel/adxl367.c
> +++ b/drivers/iio/accel/adxl367.c
> @@ -477,45 +477,42 @@ static int adxl367_set_fifo_watermark(struct
> adxl367_state *st,
> static int adxl367_set_range(struct iio_dev *indio_dev,
> enum adxl367_range range)
> {
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - struct adxl367_state *st = iio_priv(indio_dev);
> - int ret;
> + struct adxl367_state *st = iio_priv(indio_dev);
> + int ret;
>
> - guard(mutex)(&st->lock);
> + guard(mutex)(&st->lock);
>
> - ret = adxl367_set_measure_en(st, false);
> - if (ret)
> - return ret;
> + ret = adxl367_set_measure_en(st, false);
> + if (ret)
> + return ret;
>
> - 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;
> + 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;
>
> - adxl367_scale_act_thresholds(st, st->range, range);
> + adxl367_scale_act_thresholds(st, st->range, range);
>
> - /* Activity thresholds depend on range */
> - ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> - st->act_threshold);
> - if (ret)
> - return ret;
> + /* Activity thresholds depend on range */
> + ret = _adxl367_set_act_threshold(st, ADXL367_ACTIVITY,
> + st->act_threshold);
> + if (ret)
> + return ret;
>
> - ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> - st->inact_threshold);
> - if (ret)
> - return ret;
> + ret = _adxl367_set_act_threshold(st, ADXL367_INACTIVITY,
> + st->inact_threshold);
> + if (ret)
> + return ret;
>
> - ret = adxl367_set_measure_en(st, true);
> - if (ret)
> - return ret;
> + ret = adxl367_set_measure_en(st, true);
> + if (ret)
> + return ret;
>
> - st->range = range;
> + st->range = range;
>
> - return 0;
> - }
> - unreachable();
> + return 0;
> }
>
> static int adxl367_time_ms_to_samples(struct adxl367_state *st, unsigned int
> ms)
> @@ -620,23 +617,20 @@ static int _adxl367_set_odr(struct adxl367_state *st,
> enum adxl367_odr odr)
>
> static int adxl367_set_odr(struct iio_dev *indio_dev, enum adxl367_odr odr)
> {
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - struct adxl367_state *st = iio_priv(indio_dev);
> - int ret;
> + struct adxl367_state *st = iio_priv(indio_dev);
> + int ret;
>
> - guard(mutex)(&st->lock);
> + guard(mutex)(&st->lock);
>
> - ret = adxl367_set_measure_en(st, false);
> - if (ret)
> - return ret;
> + ret = adxl367_set_measure_en(st, false);
> + if (ret)
> + return ret;
>
> - ret = _adxl367_set_odr(st, odr);
> - if (ret)
> - return ret;
> + ret = _adxl367_set_odr(st, odr);
> + if (ret)
> + return ret;
>
> - return adxl367_set_measure_en(st, true);
> - }
> - unreachable();
> + return adxl367_set_measure_en(st, true);
> }
>
> static int adxl367_set_temp_adc_en(struct adxl367_state *st, unsigned int
> reg,
> @@ -725,32 +719,29 @@ static int adxl367_read_sample(struct iio_dev
> *indio_dev,
> struct iio_chan_spec const *chan,
> int *val)
> {
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - struct adxl367_state *st = iio_priv(indio_dev);
> - u16 sample;
> - int ret;
> + struct adxl367_state *st = iio_priv(indio_dev);
> + u16 sample;
> + int ret;
>
> - guard(mutex)(&st->lock);
> + guard(mutex)(&st->lock);
>
> - ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
> - if (ret)
> - return ret;
> + ret = adxl367_set_temp_adc_reg_en(st, chan->address, true);
> + if (ret)
> + return ret;
>
> - ret = regmap_bulk_read(st->regmap, chan->address, &st-
> >sample_buf,
> - sizeof(st->sample_buf));
> - if (ret)
> - return ret;
> + ret = regmap_bulk_read(st->regmap, chan->address, &st->sample_buf,
> + sizeof(st->sample_buf));
> + if (ret)
> + return ret;
>
> - sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st-
> >sample_buf));
> - *val = sign_extend32(sample, chan->scan_type.realbits - 1);
> + sample = FIELD_GET(ADXL367_DATA_MASK, be16_to_cpu(st->sample_buf));
> + *val = sign_extend32(sample, chan->scan_type.realbits - 1);
>
> - ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
> - if (ret)
> - return ret;
> + ret = adxl367_set_temp_adc_reg_en(st, chan->address, false);
> + if (ret)
> + return ret;
>
> - return IIO_VAL_INT;
> - }
> - unreachable();
> + return IIO_VAL_INT;
> }
>
> static int adxl367_get_status(struct adxl367_state *st, u8 *status,
> @@ -852,10 +843,15 @@ static int adxl367_read_raw(struct iio_dev *indio_dev,
> int *val, int *val2, long info)
> {
> struct adxl367_state *st = iio_priv(indio_dev);
> + int ret;
>
> switch (info) {
> case IIO_CHAN_INFO_RAW:
> - return adxl367_read_sample(indio_dev, chan, val);
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = adxl367_read_sample(indio_dev, chan, val);
> + iio_device_release_direct(indio_dev);
> + return ret;
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
> case IIO_ACCEL: {
> @@ -912,7 +908,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
> if (ret)
> return ret;
>
> - return adxl367_set_odr(indio_dev, odr);
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + ret = adxl367_set_odr(indio_dev, odr);
> + iio_device_release_direct(indio_dev);
> + return ret;
> }
> case IIO_CHAN_INFO_SCALE: {
> enum adxl367_range range;
> @@ -921,7 +922,12 @@ static int adxl367_write_raw(struct iio_dev *indio_dev,
> if (ret)
> return ret;
>
> - return adxl367_set_range(indio_dev, range);
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + ret = adxl367_set_range(indio_dev, range);
> + iio_device_release_direct(indio_dev);
> + return ret;
> }
> default:
> return -EINVAL;
> @@ -1069,13 +1075,15 @@ static int adxl367_read_event_config(struct iio_dev
> *indio_dev,
> }
> }
>
> -static int adxl367_write_event_config(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - enum iio_event_type type,
> - enum iio_event_direction dir,
> - bool state)
> +static int __adxl367_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> {
> + struct adxl367_state *st = iio_priv(indio_dev);
> enum adxl367_activity_type act;
> + int ret;
>
> switch (dir) {
> case IIO_EV_DIR_RISING:
> @@ -1088,28 +1096,38 @@ static int adxl367_write_event_config(struct iio_dev
> *indio_dev,
> return -EINVAL;
> }
>
> - iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
> - struct adxl367_state *st = iio_priv(indio_dev);
> - int ret;
> + guard(mutex)(&st->lock);
> +
> + ret = adxl367_set_measure_en(st, false);
> + if (ret)
> + return ret;
>
> - guard(mutex)(&st->lock);
> + ret = adxl367_set_act_interrupt_en(st, act, state);
> + if (ret)
> + return ret;
>
> - ret = adxl367_set_measure_en(st, false);
> - if (ret)
> - return ret;
> + ret = adxl367_set_act_en(st, act, state ? ADCL367_ACT_REF_ENABLED
> + : ADXL367_ACT_DISABLED);
> + if (ret)
> + return ret;
>
> - ret = adxl367_set_act_interrupt_en(st, act, state);
> - if (ret)
> - return ret;
> + return adxl367_set_measure_en(st, true);
> +}
>
> - ret = adxl367_set_act_en(st, act, state ?
> ADCL367_ACT_REF_ENABLED
> - : ADXL367_ACT_DISABLED);
> - if (ret)
> - return ret;
> +static int adxl367_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> +{
> + int ret;
>
> - return adxl367_set_measure_en(st, true);
> - }
> - unreachable();
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + ret = __adxl367_write_event_config(indio_dev, chan, type, dir,
> state);
> + iio_device_release_direct(indio_dev);
> + return ret;
> }
>
> static ssize_t adxl367_get_fifo_enabled(struct device *dev,
