| Message ID | 20240618-cleanup-ad7606-v1-7-f1854d5c779d@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7606: Improvements | expand |
Hi Guillaume,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 07d4d0bb4a8ddcc463ed599b22f510d5926c2495]
url: https://github.com/intel-lab-lkp/linux/commits/Guillaume-Stols/dt-bindings-iio-adc-adi-ad7606-add-missing-datasheet-link/20240618-223010
base: 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
patch link: https://lore.kernel.org/r/20240618-cleanup-ad7606-v1-7-f1854d5c779d%40baylibre.com
patch subject: [PATCH 7/9] iio: adc: ad7606: switch mutexes to scoped_guard
config: x86_64-randconfig-101-20240619 (https://download.01.org/0day-ci/archive/20240619/202406191142.rs8moLqC-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240619/202406191142.rs8moLqC-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406191142.rs8moLqC-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/iio/adc/ad7606.o: warning: objtool: ad7606_reg_access+0x5a: sibling call from callable instruction with modified stack frame
On Tue, 18 Jun 2024 14:02:39 +0000 Guillaume Stols <gstols@baylibre.com> wrote: > Switching to scoped_guard simplifies the code and avoids to take care to > unlock the mutex in case of premature return. > > Signed-off-by: Guillaume Stols <gstols@baylibre.com> > --- > drivers/iio/adc/ad7606.c | 71 ++++++++++++++++++++++-------------------------- > 1 file changed, 33 insertions(+), 38 deletions(-) > > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c > index 3a417595294f..e3426287edf6 100644 > --- a/drivers/iio/adc/ad7606.c > +++ b/drivers/iio/adc/ad7606.c > @@ -69,19 +69,18 @@ static int ad7606_reg_access(struct iio_dev *indio_dev, > struct ad7606_state *st = iio_priv(indio_dev); > int ret; > > - mutex_lock(&st->lock); > - if (readval) { > - ret = st->bops->reg_read(st, reg); > - if (ret < 0) > - goto err_unlock; > - *readval = ret; > - ret = 0; > - } else { > - ret = st->bops->reg_write(st, reg, writeval); > + scoped_guard(mutex, &st->lock) { > + if (readval) { > + ret = st->bops->reg_read(st, reg); > + if (ret < 0) > + return ret; > + *readval = ret; > + return 0; > + } else { > + return st->bops->reg_write(st, reg, writeval); > + } > } > -err_unlock: > - mutex_unlock(&st->lock); > - return ret; > + unreachable(); Unless you are going to add more code in this function later in the series then a guard(mutex)(&st->lock); is more appropriate in this function as the scope is effectively the whole function. Also avoids the always ugly need for an unreachable() marking. I'm not sure what the build bot warning means though. Otherwise looks good to me J
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c index 3a417595294f..e3426287edf6 100644 --- a/drivers/iio/adc/ad7606.c +++ b/drivers/iio/adc/ad7606.c @@ -69,19 +69,18 @@ static int ad7606_reg_access(struct iio_dev *indio_dev, struct ad7606_state *st = iio_priv(indio_dev); int ret; - mutex_lock(&st->lock); - if (readval) { - ret = st->bops->reg_read(st, reg); - if (ret < 0) - goto err_unlock; - *readval = ret; - ret = 0; - } else { - ret = st->bops->reg_write(st, reg, writeval); + scoped_guard(mutex, &st->lock) { + if (readval) { + ret = st->bops->reg_read(st, reg); + if (ret < 0) + return ret; + *readval = ret; + return 0; + } else { + return st->bops->reg_write(st, reg, writeval); + } } -err_unlock: - mutex_unlock(&st->lock); - return ret; + unreachable(); } static int ad7606_read_samples(struct ad7606_state *st) @@ -124,18 +123,18 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p) struct ad7606_state *st = iio_priv(indio_dev); int ret; - mutex_lock(&st->lock); + scoped_guard(mutex, &st->lock) { + ret = ad7606_read_samples(st); + if (ret) + goto error_ret; - ret = ad7606_read_samples(st); - if (ret == 0) iio_push_to_buffers_with_timestamp(indio_dev, st->data, iio_get_time_ns(indio_dev)); - - iio_trigger_notify_done(indio_dev->trig); - /* The rising edge of the CONVST signal starts a new conversion. */ - gpiod_set_value(st->gpio_convst, 1); - - mutex_unlock(&st->lock); +error_ret: + iio_trigger_notify_done(indio_dev->trig); + /* The rising edge of the CONVST signal starts a new conversion. */ + gpiod_set_value(st->gpio_convst, 1); + } return IRQ_HANDLED; } @@ -259,17 +258,15 @@ static int ad7606_write_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_SCALE: - mutex_lock(&st->lock); - i = find_closest(val2, st->scale_avail, st->num_scales); - if (st->sw_mode_en) - ch = chan->address; - ret = st->write_scale(indio_dev, ch, i); - if (ret < 0) { - mutex_unlock(&st->lock); - return ret; + scoped_guard(mutex, &st->lock) { + i = find_closest(val2, st->scale_avail, st->num_scales); + if (st->sw_mode_en) + ch = chan->address; + ret = st->write_scale(indio_dev, ch, i); + if (ret < 0) + return ret; + st->range[ch] = i; } - st->range[ch] = i; - mutex_unlock(&st->lock); return 0; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: @@ -277,14 +274,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev, return -EINVAL; i = find_closest(val, st->oversampling_avail, st->num_os_ratios); - mutex_lock(&st->lock); - ret = st->write_os(indio_dev, i); - if (ret < 0) { - mutex_unlock(&st->lock); - return ret; + scoped_guard(mutex, &st->lock) { + ret = st->write_os(indio_dev, i); + if (ret < 0) + return ret; + st->oversampling = st->oversampling_avail[i]; } - st->oversampling = st->oversampling_avail[i]; - mutex_unlock(&st->lock); return 0; default:
Switching to scoped_guard simplifies the code and avoids to take care to unlock the mutex in case of premature return. Signed-off-by: Guillaume Stols <gstols@baylibre.com> --- drivers/iio/adc/ad7606.c | 71 ++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 38 deletions(-)