drivers/iio/adc/ad7280a.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)
Use guard(mutex) instead of mutex_lock()/mutex_unlock(),
ensuring the mutex is released automatically when leaving
the function scope. The change improves error handling and
avoids issues such as missing unlocks. It also simplifies the
code by removing 'err_unlock' label and several mutex_unlock()
calls.
As suggested, in ad7280_read_raw(), wrap the IIO_CHAN_INFO_RAW
case in braces, providing a scope for the implicit variable
declared by guard(mutex).
Also, limit guard(mutex) scope in ad7280_show_balance_timer()
to keep it locked just when necessary.
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Matheus Giarola <matheusgiarola@usp.br>
---
v4:
- limit guard(mutex) scope in ad7280_show_balance_timer()
v3:
- correct v2 patch tags
v2:
- fix missing scope in ad7280_read_raw() by wrapping IIO_CHAN_INFO_RAW
case in braces so as to fix build errors.
drivers/iio/adc/ad7280a.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/iio/adc/ad7280a.c b/drivers/iio/adc/ad7280a.c
index ba12a3796e2b..f3cbf8810daa 100644
--- a/drivers/iio/adc/ad7280a.c
+++ b/drivers/iio/adc/ad7280a.c
@@ -496,7 +496,7 @@ static ssize_t ad7280_store_balance_sw(struct iio_dev *indio_dev,
devaddr = chan->address >> 8;
ch = chan->address & 0xFF;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
if (readin)
st->cb_mask[devaddr] |= BIT(ch);
else
@@ -505,7 +505,6 @@ static ssize_t ad7280_store_balance_sw(struct iio_dev *indio_dev,
ret = ad7280_write(st, devaddr, AD7280A_CELL_BALANCE_REG, 0,
FIELD_PREP(AD7280A_CELL_BALANCE_CHAN_BITMAP_MSK,
st->cb_mask[devaddr]));
- mutex_unlock(&st->lock);
return ret ? ret : len;
}
@@ -519,10 +518,11 @@ static ssize_t ad7280_show_balance_timer(struct iio_dev *indio_dev,
unsigned int msecs;
int ret;
- mutex_lock(&st->lock);
- ret = ad7280_read_reg(st, chan->address >> 8,
+ {
+ guard(mutex)(&st->lock);
+ ret = ad7280_read_reg(st, chan->address >> 8,
(chan->address & 0xFF) + AD7280A_CB1_TIMER_REG);
- mutex_unlock(&st->lock);
+ }
if (ret < 0)
return ret;
@@ -551,11 +551,10 @@ static ssize_t ad7280_store_balance_timer(struct iio_dev *indio_dev,
if (val > 31)
return -EINVAL;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = ad7280_write(st, chan->address >> 8,
(chan->address & 0xFF) + AD7280A_CB1_TIMER_REG, 0,
FIELD_PREP(AD7280A_CB_TIMER_VAL_MSK, val));
- mutex_unlock(&st->lock);
return ret ? ret : len;
}
@@ -737,7 +736,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
if (val2 != 0)
return -EINVAL;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
switch (chan->type) {
case IIO_VOLTAGE:
value = ((val - 1000) * 100) / 1568; /* LSB 15.68mV */
@@ -760,8 +759,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
st->cell_threshlow = value;
break;
default:
- ret = -EINVAL;
- goto err_unlock;
+ return -EINVAL;
}
break;
case IIO_TEMP:
@@ -785,18 +783,12 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
st->aux_threshlow = value;
break;
default:
- ret = -EINVAL;
- goto err_unlock;
+ return -EINVAL;
}
break;
default:
- ret = -EINVAL;
- goto err_unlock;
+ return -EINVAL;
}
-
-err_unlock:
- mutex_unlock(&st->lock);
-
return ret;
}
@@ -884,14 +876,13 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
int ret;
switch (m) {
- case IIO_CHAN_INFO_RAW:
- mutex_lock(&st->lock);
+ case IIO_CHAN_INFO_RAW: {
+ guard(mutex)(&st->lock);
if (chan->address == AD7280A_ALL_CELLS)
ret = ad7280_read_all_channels(st, st->scan_cnt, NULL);
else
ret = ad7280_read_channel(st, chan->address >> 8,
chan->address & 0xFF);
- mutex_unlock(&st->lock);
if (ret < 0)
return ret;
@@ -899,6 +890,7 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
*val = ret;
return IIO_VAL_INT;
+ }
case IIO_CHAN_INFO_SCALE:
if ((chan->address & 0xFF) <= AD7280A_CELL_VOLTAGE_6_REG)
*val = 4000;
--
2.47.3On Sun, 12 Apr 2026 12:47:48 -0300
Matheus Giarola <matheustpgiarola@gmail.com> wrote:
> Use guard(mutex) instead of mutex_lock()/mutex_unlock(),
> ensuring the mutex is released automatically when leaving
> the function scope. The change improves error handling and
> avoids issues such as missing unlocks. It also simplifies the
> code by removing 'err_unlock' label and several mutex_unlock()
> calls.
>
> As suggested, in ad7280_read_raw(), wrap the IIO_CHAN_INFO_RAW
> case in braces, providing a scope for the implicit variable
> declared by guard(mutex).
>
> Also, limit guard(mutex) scope in ad7280_show_balance_timer()
> to keep it locked just when necessary.
>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Matheus Giarola <matheusgiarola@usp.br>
> ---
> v4:
> - limit guard(mutex) scope in ad7280_show_balance_timer()
Some confusion has occured here. The braces thing works for some
places (where we bring only trivial extra operations under the lock)
but don't use it where scoped_guard() is cleaner.
>
> v3:
> - correct v2 patch tags
>
> v2:
> - fix missing scope in ad7280_read_raw() by wrapping IIO_CHAN_INFO_RAW
> case in braces so as to fix build errors.
>
> drivers/iio/adc/ad7280a.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7280a.c b/drivers/iio/adc/ad7280a.c
> index ba12a3796e2b..f3cbf8810daa 100644
> --- a/drivers/iio/adc/ad7280a.c
> +++ b/drivers/iio/adc/ad7280a.c
...
> @@ -519,10 +518,11 @@ static ssize_t ad7280_show_balance_timer(struct iio_dev *indio_dev,
> unsigned int msecs;
> int ret;
>
> - mutex_lock(&st->lock);
> - ret = ad7280_read_reg(st, chan->address >> 8,
> + {
> + guard(mutex)(&st->lock);
> + ret = ad7280_read_reg(st, chan->address >> 8,
> (chan->address & 0xFF) + AD7280A_CB1_TIMER_REG);
> - mutex_unlock(&st->lock);
Not this. For this case you scoped_guard() or just take the view
that it doesn't matter if we hold the lock a bit longer.
> + }
>
> if (ret < 0)
> return ret;
> @@ -551,11 +551,10 @@ static ssize_t ad7280_store_balance_timer(struct iio_dev *indio_dev,
> if (val > 31)
> return -EINVAL;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = ad7280_write(st, chan->address >> 8,
> (chan->address & 0xFF) + AD7280A_CB1_TIMER_REG, 0,
> FIELD_PREP(AD7280A_CB_TIMER_VAL_MSK, val));
> - mutex_unlock(&st->lock);
>
> return ret ? ret : len;
> }
> @@ -737,7 +736,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
> if (val2 != 0)
> return -EINVAL;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> switch (chan->type) {
> case IIO_VOLTAGE:
> value = ((val - 1000) * 100) / 1568; /* LSB 15.68mV */
> @@ -760,8 +759,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
Just above this is:
if (ret)
break;
Make that
if (ret)
return ret;
and same for other such breaks on error.
> st->cell_threshlow = value;
> break;
> default:
> - ret = -EINVAL;
> - goto err_unlock;
> + return -EINVAL;
> }
> break;
> case IIO_TEMP:
> @@ -785,18 +783,12 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
> st->aux_threshlow = value;
> break;
> default:
> - ret = -EINVAL;
> - goto err_unlock;
> + return -EINVAL;
> }
> break;
> default:
> - ret = -EINVAL;
> - goto err_unlock;
> + return -EINVAL;
> }
> -
> -err_unlock:
> - mutex_unlock(&st->lock);
> -
> return ret;
maybe with the changes suggested above used throughout we might not
be able to get here with out ret == 0;
In which case make that obvious with
return 0;
> }
>
> @@ -884,14 +876,13 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
> int ret;
>
> switch (m) {
> - case IIO_CHAN_INFO_RAW:
> - mutex_lock(&st->lock);
> + case IIO_CHAN_INFO_RAW: {
> + guard(mutex)(&st->lock);
> if (chan->address == AD7280A_ALL_CELLS)
> ret = ad7280_read_all_channels(st, st->scan_cnt, NULL);
> else
> ret = ad7280_read_channel(st, chan->address >> 8,
> chan->address & 0xFF);
> - mutex_unlock(&st->lock);
>
> if (ret < 0)
> return ret;
> @@ -899,6 +890,7 @@ static int ad7280_read_raw(struct iio_dev *indio_dev,
> *val = ret;
>
> return IIO_VAL_INT;
> + }
> case IIO_CHAN_INFO_SCALE:
> if ((chan->address & 0xFF) <= AD7280A_CELL_VOLTAGE_6_REG)
> *val = 4000;
On Mon, Apr 13, 2026 at 4:50 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 12 Apr 2026 12:47:48 -0300
> Matheus Giarola <matheustpgiarola@gmail.com> wrote:
>
> > Use guard(mutex) instead of mutex_lock()/mutex_unlock(),
> > ensuring the mutex is released automatically when leaving
> > the function scope. The change improves error handling and
> > avoids issues such as missing unlocks. It also simplifies the
> > code by removing 'err_unlock' label and several mutex_unlock()
> > calls.
> >
> > As suggested, in ad7280_read_raw(), wrap the IIO_CHAN_INFO_RAW
> > case in braces, providing a scope for the implicit variable
> > declared by guard(mutex).
> >
> > Also, limit guard(mutex) scope in ad7280_show_balance_timer()
> > to keep it locked just when necessary.
> >
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Matheus Giarola <matheusgiarola@usp.br>
> > ---
> > v4:
> > - limit guard(mutex) scope in ad7280_show_balance_timer()
>
> Some confusion has occured here. The braces thing works for some
> places (where we bring only trivial extra operations under the lock)
> but don't use it where scoped_guard() is cleaner.
That makes sense. Sorry about the misunderstanding and thanks for the
correction.
>
> >
> > v3:
> > - correct v2 patch tags
> >
> > v2:
> > - fix missing scope in ad7280_read_raw() by wrapping IIO_CHAN_INFO_RAW
> > case in braces so as to fix build errors.
> >
> > drivers/iio/adc/ad7280a.c | 34 +++++++++++++---------------------
> > 1 file changed, 13 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7280a.c b/drivers/iio/adc/ad7280a.c
> > index ba12a3796e2b..f3cbf8810daa 100644
> > --- a/drivers/iio/adc/ad7280a.c
> > +++ b/drivers/iio/adc/ad7280a.c
> ...
>
> > @@ -519,10 +518,11 @@ static ssize_t ad7280_show_balance_timer(struct iio_dev *indio_dev,
> > unsigned int msecs;
> > int ret;
> >
> > - mutex_lock(&st->lock);
> > - ret = ad7280_read_reg(st, chan->address >> 8,
> > + {
> > + guard(mutex)(&st->lock);
> > + ret = ad7280_read_reg(st, chan->address >> 8,
> > (chan->address & 0xFF) + AD7280A_CB1_TIMER_REG);
> > - mutex_unlock(&st->lock);
>
> Not this. For this case you scoped_guard() or just take the view
> that it doesn't matter if we hold the lock a bit longer.
Alright. Since Andy pointed out that it would be better if we kept the lock
only where it was necessary in previous versions, I'll change it to
scoped_guard().
>
>
> > + }
> >
> > if (ret < 0)
> > return ret;
> > @@ -551,11 +551,10 @@ static ssize_t ad7280_store_balance_timer(struct iio_dev *indio_dev,
> > if (val > 31)
> > return -EINVAL;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = ad7280_write(st, chan->address >> 8,
> > (chan->address & 0xFF) + AD7280A_CB1_TIMER_REG, 0,
> > FIELD_PREP(AD7280A_CB_TIMER_VAL_MSK, val));
> > - mutex_unlock(&st->lock);
> >
> > return ret ? ret : len;
> > }
> > @@ -737,7 +736,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
> > if (val2 != 0)
> > return -EINVAL;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > switch (chan->type) {
> > case IIO_VOLTAGE:
> > value = ((val - 1000) * 100) / 1568; /* LSB 15.68mV */
> > @@ -760,8 +759,7 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
> Just above this is:
> if (ret)
> break;
>
> Make that
> if (ret)
> return ret;
>
> and same for other such breaks on error.
>
>
> > st->cell_threshlow = value;
> > break;
> > default:
> > - ret = -EINVAL;
> > - goto err_unlock;
> > + return -EINVAL;
> > }
> > break;
> > case IIO_TEMP:
> > @@ -785,18 +783,12 @@ static int ad7280a_write_thresh(struct iio_dev *indio_dev,
> > st->aux_threshlow = value;
> > break;
> > default:
> > - ret = -EINVAL;
> > - goto err_unlock;
> > + return -EINVAL;
> > }
> > break;
> > default:
> > - ret = -EINVAL;
> > - goto err_unlock;
> > + return -EINVAL;
> > }
> > -
> > -err_unlock:
> > - mutex_unlock(&st->lock);
> > -
> > return ret;
> maybe with the changes suggested above used throughout we might not
> be able to get here with out ret == 0;
> In which case make that obvious with
> return 0;
>
Got it, I'll send a v5 with all these changes.
Thanks for all the support and suggestions,
Matheus.
© 2016 - 2026 Red Hat, Inc.