Use guard(mutex)(&st->lock) for handling mutex lock instead of
manually locking and unlocking the mutex. This prevents forgotten
locks due to early exits and remove the need of gotos.
Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
Co-developed-by: Gustavo Bastos <gustavobastos@usp.br>
Signed-off-by: Gustavo Bastos <gustavobastos@usp.br>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
---
For this one, there are two cases where the previous implementation
was a smalllocking portion of the code and now it's locking the whole
function. I don't know if this is a desired behavior.
drivers/iio/accel/sca3000.c | 177 ++++++++++++------------------------
1 file changed, 57 insertions(+), 120 deletions(-)
diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index d41759c68fb4..098d45bad389 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -405,17 +405,14 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
int ret;
struct sca3000_state *st = iio_priv(indio_dev);
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_REVID_ADDR));
if (ret < 0)
- goto error_ret;
+ return ret;
dev_info(&indio_dev->dev,
"sca3000 revision major=%lu, minor=%lu\n",
ret & SCA3000_REG_REVID_MAJOR_MASK,
ret & SCA3000_REG_REVID_MINOR_MASK);
-error_ret:
- mutex_unlock(&st->lock);
-
return ret;
}
@@ -699,32 +696,25 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
if (chan->type == IIO_ACCEL) {
- if (st->mo_det_use_count) {
- mutex_unlock(&st->lock);
+ if (st->mo_det_use_count)
return -EBUSY;
- }
address = sca3000_addresses[chan->address][0];
ret = spi_w8r16be(st->us, SCA3000_READ_REG(address));
- if (ret < 0) {
- mutex_unlock(&st->lock);
+ if (ret < 0)
return ret;
- }
*val = sign_extend32(ret >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
} else {
/* get the temperature when available */
ret = spi_w8r16be(st->us,
SCA3000_READ_REG(SCA3000_REG_TEMP_MSB_ADDR));
- if (ret < 0) {
- mutex_unlock(&st->lock);
+ if (ret < 0)
return ret;
- }
*val = (ret >> chan->scan_type.shift) &
GENMASK(chan->scan_type.realbits - 1, 0);
}
- mutex_unlock(&st->lock);
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
*val = 0;
@@ -738,14 +728,12 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
*val2 = 600000;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_SAMP_FREQ:
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = sca3000_read_raw_samp_freq(st, val);
- mutex_unlock(&st->lock);
return ret ? ret : IIO_VAL_INT;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = sca3000_read_3db_freq(st, val);
- mutex_unlock(&st->lock);
return ret;
default:
return -EINVAL;
@@ -763,22 +751,16 @@ static int sca3000_write_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SAMP_FREQ:
if (val2)
return -EINVAL;
- mutex_lock(&st->lock);
- ret = sca3000_write_raw_samp_freq(st, val);
- mutex_unlock(&st->lock);
- return ret;
+ guard(mutex)(&st->lock);
+ return sca3000_write_raw_samp_freq(st, val);
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
if (val2)
return -EINVAL;
- mutex_lock(&st->lock);
- ret = sca3000_write_3db_freq(st, val);
- mutex_unlock(&st->lock);
- return ret;
+ guard(mutex)(&st->lock);
+ return sca3000_write_3db_freq(st, val);
default:
return -EINVAL;
}
-
- return ret;
}
/**
@@ -800,9 +782,8 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
struct sca3000_state *st = iio_priv(indio_dev);
int len = 0, ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
- mutex_unlock(&st->lock);
if (ret)
return ret;
@@ -851,10 +832,9 @@ static int sca3000_read_event_value(struct iio_dev *indio_dev,
switch (info) {
case IIO_EV_INFO_VALUE:
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = sca3000_read_ctrl_reg(st,
sca3000_addresses[chan->address][1]);
- mutex_unlock(&st->lock);
if (ret < 0)
return ret;
*val = 0;
@@ -918,13 +898,10 @@ static int sca3000_write_event_value(struct iio_dev *indio_dev,
}
}
- mutex_lock(&st->lock);
- ret = sca3000_write_ctrl_reg(st,
+ guard(mutex)(&st->lock);
+ return sca3000_write_ctrl_reg(st,
sca3000_addresses[chan->address][1],
nonlinear);
- mutex_unlock(&st->lock);
-
- return ret;
}
static struct attribute *sca3000_attributes[] = {
@@ -969,12 +946,12 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
struct sca3000_state *st = iio_priv(indio_dev);
int ret, i, num_available;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
if (val & SCA3000_REG_INT_STATUS_HALF) {
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_BUF_COUNT_ADDR));
if (ret)
- goto error_ret;
+ return;
num_available = ret;
/*
* num_available is the total number of samples available
@@ -983,7 +960,7 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR,
num_available * 2);
if (ret)
- goto error_ret;
+ return;
for (i = 0; i < num_available / 3; i++) {
/*
* Dirty hack to cover for 11 bit in fifo, 13 bit
@@ -995,8 +972,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev *indio_dev)
iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
}
}
-error_ret:
- mutex_unlock(&st->lock);
}
/**
@@ -1022,9 +997,8 @@ static irqreturn_t sca3000_event_handler(int irq, void *private)
* Could lead if badly timed to an extra read of status reg,
* but ensures no interrupt is missed.
*/
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_STATUS_ADDR));
- mutex_unlock(&st->lock);
if (ret)
goto done;
@@ -1081,16 +1055,15 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
struct sca3000_state *st = iio_priv(indio_dev);
int ret;
/* read current value of mode register */
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
if (ret)
- goto error_ret;
+ return ret;
switch (chan->channel2) {
case IIO_MOD_X_AND_Y_AND_Z:
- ret = !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
- break;
+ return !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
case IIO_MOD_X:
case IIO_MOD_Y:
case IIO_MOD_Z:
@@ -1100,24 +1073,18 @@ static int sca3000_read_event_config(struct iio_dev *indio_dev,
*/
if ((ret & SCA3000_REG_MODE_MODE_MASK)
!= SCA3000_REG_MODE_MEAS_MODE_MOT_DET) {
- ret = 0;
+ return 0;
} else {
ret = sca3000_read_ctrl_reg(st,
SCA3000_REG_CTRL_SEL_MD_CTRL);
if (ret < 0)
- goto error_ret;
+ return ret;
/* only supporting logical or's for now */
- ret = !!(ret & sca3000_addresses[chan->address][2]);
+ return !!(ret & sca3000_addresses[chan->address][2]);
}
- break;
default:
- ret = -EINVAL;
+ return -EINVAL;
}
-
-error_ret:
- mutex_unlock(&st->lock);
-
- return ret;
}
static int sca3000_freefall_set_state(struct iio_dev *indio_dev, bool state)
@@ -1220,26 +1187,19 @@ static int sca3000_write_event_config(struct iio_dev *indio_dev,
struct sca3000_state *st = iio_priv(indio_dev);
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
switch (chan->channel2) {
case IIO_MOD_X_AND_Y_AND_Z:
- ret = sca3000_freefall_set_state(indio_dev, state);
- break;
-
+ return sca3000_freefall_set_state(indio_dev, state);
case IIO_MOD_X:
case IIO_MOD_Y:
case IIO_MOD_Z:
- ret = sca3000_motion_detect_set_state(indio_dev,
+ return sca3000_motion_detect_set_state(indio_dev,
chan->address,
state);
- break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- mutex_unlock(&st->lock);
-
- return ret;
}
static inline
@@ -1248,23 +1208,19 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
struct sca3000_state *st = iio_priv(indio_dev);
int ret;
- mutex_lock(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
if (ret)
- goto error_ret;
+ return ret;
+
if (state) {
dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
- ret = sca3000_write_reg(st,
+ return sca3000_write_reg(st,
SCA3000_REG_MODE_ADDR,
(ret | SCA3000_REG_MODE_RING_BUF_ENABLE));
- } else
- ret = sca3000_write_reg(st,
- SCA3000_REG_MODE_ADDR,
- (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
-error_ret:
- mutex_unlock(&st->lock);
-
- return ret;
+ }
+ return sca3000_write_reg(st,
+ SCA3000_REG_MODE_ADDR,
+ (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
}
/**
@@ -1281,26 +1237,18 @@ static int sca3000_hw_ring_preenable(struct iio_dev *indio_dev)
int ret;
struct sca3000_state *st = iio_priv(indio_dev);
- mutex_lock(&st->lock);
-
+ guard(mutex)(&st->lock);
/* Enable the 50% full interrupt */
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
if (ret)
- goto error_unlock;
+ return ret;
ret = sca3000_write_reg(st,
SCA3000_REG_INT_MASK_ADDR,
ret | SCA3000_REG_INT_MASK_RING_HALF);
if (ret)
- goto error_unlock;
-
- mutex_unlock(&st->lock);
+ return ret;
return __sca3000_hw_ring_state_set(indio_dev, 1);
-
-error_unlock:
- mutex_unlock(&st->lock);
-
- return ret;
}
static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
@@ -1308,22 +1256,18 @@ static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
int ret;
struct sca3000_state *st = iio_priv(indio_dev);
+ guard(mutex)(&st->lock);
ret = __sca3000_hw_ring_state_set(indio_dev, 0);
if (ret)
return ret;
/* Disable the 50% full interrupt */
- mutex_lock(&st->lock);
-
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
if (ret)
- goto unlock;
- ret = sca3000_write_reg(st,
+ return ret;
+ return sca3000_write_reg(st,
SCA3000_REG_INT_MASK_ADDR,
ret & ~SCA3000_REG_INT_MASK_RING_HALF);
-unlock:
- mutex_unlock(&st->lock);
- return ret;
}
static const struct iio_buffer_setup_ops sca3000_ring_setup_ops = {
@@ -1343,25 +1287,25 @@ static int sca3000_clean_setup(struct sca3000_state *st)
{
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
/* Ensure all interrupts have been acknowledged */
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
if (ret)
- goto error_ret;
+ return ret;
/* Turn off all motion detection channels */
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL);
if (ret < 0)
- goto error_ret;
+ return ret;
ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL,
ret & SCA3000_MD_CTRL_PROT_MASK);
if (ret)
- goto error_ret;
+ return ret;
/* Disable ring buffer */
ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
if (ret < 0)
- goto error_ret;
+ return ret;
ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
(ret & SCA3000_REG_OUT_CTRL_PROT_MASK)
| SCA3000_REG_OUT_CTRL_BUF_X_EN
@@ -1369,17 +1313,17 @@ static int sca3000_clean_setup(struct sca3000_state *st)
| SCA3000_REG_OUT_CTRL_BUF_Z_EN
| SCA3000_REG_OUT_CTRL_BUF_DIV_4);
if (ret)
- goto error_ret;
+ return ret;
/* Enable interrupts, relevant to mode and set up as active low */
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
if (ret)
- goto error_ret;
+ return ret;
ret = sca3000_write_reg(st,
SCA3000_REG_INT_MASK_ADDR,
(ret & SCA3000_REG_INT_MASK_PROT_MASK)
| SCA3000_REG_INT_MASK_ACTIVE_LOW);
if (ret)
- goto error_ret;
+ return ret;
/*
* Select normal measurement mode, free fall off, ring off
* Ring in 12 bit mode - it is fine to overwrite reserved bits 3,5
@@ -1387,13 +1331,9 @@ static int sca3000_clean_setup(struct sca3000_state *st)
*/
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
if (ret)
- goto error_ret;
- ret = sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
+ return ret;
+ return sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
ret & SCA3000_MODE_PROT_MASK);
-
-error_ret:
- mutex_unlock(&st->lock);
- return ret;
}
static const struct iio_info sca3000_info = {
@@ -1471,19 +1411,16 @@ static int sca3000_stop_all_interrupts(struct sca3000_state *st)
{
int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
if (ret)
- goto error_ret;
+ return ret;
- ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
+ return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
ret &
~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
SCA3000_REG_INT_MASK_RING_HALF |
SCA3000_REG_INT_MASK_ALL_INTS));
-error_ret:
- mutex_unlock(&st->lock);
- return ret;
}
static void sca3000_remove(struct spi_device *spi)
--
2.49.0On Wed, 11 Jun 2025 16:39:21 -0300
Andrew Ijano <andrew.ijano@gmail.com> wrote:
> Use guard(mutex)(&st->lock) for handling mutex lock instead of
> manually locking and unlocking the mutex. This prevents forgotten
> locks due to early exits and remove the need of gotos.
>
> Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
> Co-developed-by: Gustavo Bastos <gustavobastos@usp.br>
> Signed-off-by: Gustavo Bastos <gustavobastos@usp.br>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> ---
> For this one, there are two cases where the previous implementation
> was a smalllocking portion of the code and now it's locking the whole
> function. I don't know if this is a desired behavior.
I'd call out more specifically that you are going from
lock
stuff
unlock
call which contains lock over whole useful scope
to
lock
stuff
call with lock no longer done inside
unlock
In at least one case. Looks cleaner to me. I'd guess this is a silly
bit of code evolution that you are tidying up as that lock dance looks
pointless to me.
Otherwise just the {} for cases thing needs fixing up.
Jonathan
On Sat, Jun 14, 2025 at 9:01 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 11 Jun 2025 16:39:21 -0300
> Andrew Ijano <andrew.ijano@gmail.com> wrote:
>
> > Use guard(mutex)(&st->lock) for handling mutex lock instead of
> > manually locking and unlocking the mutex. This prevents forgotten
> > locks due to early exits and remove the need of gotos.
> >
> > Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
> > Co-developed-by: Gustavo Bastos <gustavobastos@usp.br>
> > Signed-off-by: Gustavo Bastos <gustavobastos@usp.br>
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > ---
> > For this one, there are two cases where the previous implementation
> > was a smalllocking portion of the code and now it's locking the whole
> > function. I don't know if this is a desired behavior.
>
> I'd call out more specifically that you are going from
>
> lock
> stuff
> unlock
> call which contains lock over whole useful scope
>
> to
> lock
> stuff
> call with lock no longer done inside
> unlock
>
> In at least one case. Looks cleaner to me. I'd guess this is a silly
> bit of code evolution that you are tidying up as that lock dance looks
> pointless to me.
Yes! That's something I noticed when making this change and it looked
like a clean up for me too.
What bothered me were cases where we originally had a lock /
spi_w8r8() / unlock and then several operations like iio_push_event()
or sprintf(). I found these cases in sca3000_event_handler() and
sca3000_read_av_freq().
Maybe this isn't as problematic as I'm making them up to be, but it
seems that applying the suggestion of Nino to use scoped_guard()
instead in these cases would already solve this problem.
>
> Otherwise just the {} for cases thing needs fixing up.
Great! I'll fix that too.
Thanks,
Andrew
Hi Andrew,
kernel test robot noticed the following build errors:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.16-rc1 next-20250612]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrew-Ijano/iio-accel-sca3000-replace-error_ret-labels-by-simple-returns/20250612-034940
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20250611194648.18133-4-andrew.lopes%40alumni.usp.br
patch subject: [PATCH v5 3/3] iio: accel: sca3000: use guard(mutex)() for handling mutex lock
config: nios2-randconfig-002-20250612 (https://download.01.org/0day-ci/archive/20250612/202506122309.FvJPaMhh-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250612/202506122309.FvJPaMhh-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/202506122309.FvJPaMhh-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/irqflags.h:17,
from include/asm-generic/bitops.h:14,
from ./arch/nios2/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:68,
from include/linux/kernel.h:23,
from include/linux/interrupt.h:6,
from drivers/iio/accel/sca3000.c:10:
drivers/iio/accel/sca3000.c: In function 'sca3000_read_raw':
>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
^~~~~~
include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
CLASS(_name, __UNIQUE_ID(guard))
^~~~~
drivers/iio/accel/sca3000.c:699:3: note: in expansion of macro 'guard'
guard(mutex)(&st->lock);
^~~~~
>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
^~~~~~
include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
CLASS(_name, __UNIQUE_ID(guard))
^~~~~
drivers/iio/accel/sca3000.c:731:3: note: in expansion of macro 'guard'
guard(mutex)(&st->lock);
^~~~~
>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
^~~~~~
include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
CLASS(_name, __UNIQUE_ID(guard))
^~~~~
drivers/iio/accel/sca3000.c:735:3: note: in expansion of macro 'guard'
guard(mutex)(&st->lock);
^~~~~
drivers/iio/accel/sca3000.c: In function 'sca3000_write_raw':
drivers/iio/accel/sca3000.c:748:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^~~
In file included from include/linux/irqflags.h:17,
from include/asm-generic/bitops.h:14,
from ./arch/nios2/include/generated/asm/bitops.h:1,
from include/linux/bitops.h:68,
from include/linux/kernel.h:23,
from include/linux/interrupt.h:6,
from drivers/iio/accel/sca3000.c:10:
drivers/iio/accel/sca3000.c: In function 'sca3000_read_event_value':
>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
class_##_name##_t var __cleanup(class_##_name##_destructor) = \
^~~~~~
include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
CLASS(_name, __UNIQUE_ID(guard))
^~~~~
drivers/iio/accel/sca3000.c:835:3: note: in expansion of macro 'guard'
guard(mutex)(&st->lock);
^~~~~
drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_value':
drivers/iio/accel/sca3000.c:881:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^~~
drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_config':
drivers/iio/accel/sca3000.c:1188:6: warning: unused variable 'ret' [-Wunused-variable]
int ret;
^~~
vim +258 include/linux/cleanup.h
54da6a0924311c Peter Zijlstra 2023-05-26 256
54da6a0924311c Peter Zijlstra 2023-05-26 257 #define CLASS(_name, var) \
54da6a0924311c Peter Zijlstra 2023-05-26 @258 class_##_name##_t var __cleanup(class_##_name##_destructor) = \
54da6a0924311c Peter Zijlstra 2023-05-26 259 class_##_name##_constructor
54da6a0924311c Peter Zijlstra 2023-05-26 260
54da6a0924311c Peter Zijlstra 2023-05-26 261
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 6/12/25 10:52 AM, kernel test robot wrote:
> Hi Andrew,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on jic23-iio/togreg]
> [also build test ERROR on linus/master v6.16-rc1 next-20250612]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Andrew-Ijano/iio-accel-sca3000-replace-error_ret-labels-by-simple-returns/20250612-034940
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link: https://lore.kernel.org/r/20250611194648.18133-4-andrew.lopes%40alumni.usp.br
> patch subject: [PATCH v5 3/3] iio: accel: sca3000: use guard(mutex)() for handling mutex lock
> config: nios2-randconfig-002-20250612 (https://download.01.org/0day-ci/archive/20250612/202506122309.FvJPaMhh-lkp@intel.com/config)
> compiler: nios2-linux-gcc (GCC) 8.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250612/202506122309.FvJPaMhh-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/202506122309.FvJPaMhh-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/irqflags.h:17,
> from include/asm-generic/bitops.h:14,
> from ./arch/nios2/include/generated/asm/bitops.h:1,
> from include/linux/bitops.h:68,
> from include/linux/kernel.h:23,
> from include/linux/interrupt.h:6,
> from drivers/iio/accel/sca3000.c:10:
> drivers/iio/accel/sca3000.c: In function 'sca3000_read_raw':
>>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
> class_##_name##_t var __cleanup(class_##_name##_destructor) = \
> ^~~~~~
> include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
> CLASS(_name, __UNIQUE_ID(guard))
> ^~~~~
> drivers/iio/accel/sca3000.c:699:3: note: in expansion of macro 'guard'
> guard(mutex)(&st->lock);
> ^~~~~
>>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
> class_##_name##_t var __cleanup(class_##_name##_destructor) = \
> ^~~~~~
> include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
> CLASS(_name, __UNIQUE_ID(guard))
> ^~~~~
> drivers/iio/accel/sca3000.c:731:3: note: in expansion of macro 'guard'
> guard(mutex)(&st->lock);
> ^~~~~
>>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
> class_##_name##_t var __cleanup(class_##_name##_destructor) = \
> ^~~~~~
> include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
> CLASS(_name, __UNIQUE_ID(guard))
> ^~~~~
> drivers/iio/accel/sca3000.c:735:3: note: in expansion of macro 'guard'
> guard(mutex)(&st->lock);
> ^~~~~
> drivers/iio/accel/sca3000.c: In function 'sca3000_write_raw':
> drivers/iio/accel/sca3000.c:748:6: warning: unused variable 'ret' [-Wunused-variable]
> int ret;
> ^~~
> In file included from include/linux/irqflags.h:17,
> from include/asm-generic/bitops.h:14,
> from ./arch/nios2/include/generated/asm/bitops.h:1,
> from include/linux/bitops.h:68,
> from include/linux/kernel.h:23,
> from include/linux/interrupt.h:6,
> from drivers/iio/accel/sca3000.c:10:
> drivers/iio/accel/sca3000.c: In function 'sca3000_read_event_value':
>>> include/linux/cleanup.h:258:2: error: a label can only be part of a statement and a declaration is not a statement
> class_##_name##_t var __cleanup(class_##_name##_destructor) = \
> ^~~~~~
> include/linux/cleanup.h:319:2: note: in expansion of macro 'CLASS'
> CLASS(_name, __UNIQUE_ID(guard))
> ^~~~~
> drivers/iio/accel/sca3000.c:835:3: note: in expansion of macro 'guard'
> guard(mutex)(&st->lock);
> ^~~~~
> drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_value':
> drivers/iio/accel/sca3000.c:881:6: warning: unused variable 'ret' [-Wunused-variable]
> int ret;
> ^~~
> drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_config':
> drivers/iio/accel/sca3000.c:1188:6: warning: unused variable 'ret' [-Wunused-variable]
> int ret;
> ^~~
>
>
> vim +258 include/linux/cleanup.h
>
> 54da6a0924311c Peter Zijlstra 2023-05-26 256
> 54da6a0924311c Peter Zijlstra 2023-05-26 257 #define CLASS(_name, var) \
> 54da6a0924311c Peter Zijlstra 2023-05-26 @258 class_##_name##_t var __cleanup(class_##_name##_destructor) = \
> 54da6a0924311c Peter Zijlstra 2023-05-26 259 class_##_name##_constructor
> 54da6a0924311c Peter Zijlstra 2023-05-26 260
> 54da6a0924311c Peter Zijlstra 2023-05-26 261
>
These error messages aren't particularity helpful, but what I think
this is try to say is that you have to be careful with guard() in
switch statements.
The guard() macro is declaring a new local variable, which shouldn't
be done in a case: statement without enclosing it in a separate scope.
Some compilers complain and some don't so even if it worked for you
locally, we need to make it work for all supported compilers.
So the code needs to looks something like this:
case IIO_CHAN_INFO_SAMP_FREQ: {
guard(mutex)(&st->lock);
ret = sca3000_read_raw_samp_freq(st, val);
return ret ? ret : IIO_VAL_INT;
}
On Thu, Jun 12, 2025 at 1:06 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 6/12/25 10:52 AM, kernel test robot wrote:
> > Hi Andrew,
...
>
> These error messages aren't particularity helpful, but what I think
> this is try to say is that you have to be careful with guard() in
> switch statements.
>
> The guard() macro is declaring a new local variable, which shouldn't
> be done in a case: statement without enclosing it in a separate scope.
> Some compilers complain and some don't so even if it worked for you
> locally, we need to make it work for all supported compilers.
>
> So the code needs to looks something like this:
>
> case IIO_CHAN_INFO_SAMP_FREQ: {
> guard(mutex)(&st->lock);
> ret = sca3000_read_raw_samp_freq(st, val);
> return ret ? ret : IIO_VAL_INT;
> }
Hi, David! Thanks a lot for the explanation. I'll make this change right away.
Thanks,
Andrew
Hi Andrew,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.16-rc1 next-20250612]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrew-Ijano/iio-accel-sca3000-replace-error_ret-labels-by-simple-returns/20250612-034940
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20250611194648.18133-4-andrew.lopes%40alumni.usp.br
patch subject: [PATCH v5 3/3] iio: accel: sca3000: use guard(mutex)() for handling mutex lock
config: x86_64-randconfig-001-20250612 (https://download.01.org/0day-ci/archive/20250612/202506122259.4MrANF8h-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250612/202506122259.4MrANF8h-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/202506122259.4MrANF8h-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/iio/accel/sca3000.c: In function 'sca3000_write_raw':
>> drivers/iio/accel/sca3000.c:748:13: warning: unused variable 'ret' [-Wunused-variable]
748 | int ret;
| ^~~
drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_value':
drivers/iio/accel/sca3000.c:881:13: warning: unused variable 'ret' [-Wunused-variable]
881 | int ret;
| ^~~
drivers/iio/accel/sca3000.c: In function 'sca3000_write_event_config':
drivers/iio/accel/sca3000.c:1188:13: warning: unused variable 'ret' [-Wunused-variable]
1188 | int ret;
| ^~~
vim +/ret +748 drivers/iio/accel/sca3000.c
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 742
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 743 static int sca3000_write_raw(struct iio_dev *indio_dev,
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 744 struct iio_chan_spec const *chan,
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 745 int val, int val2, long mask)
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 746 {
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 747 struct sca3000_state *st = iio_priv(indio_dev);
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 @748 int ret;
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 749
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 750 switch (mask) {
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 751 case IIO_CHAN_INFO_SAMP_FREQ:
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 752 if (val2)
e0f3fc9b47e61b drivers/staging/iio/accel/sca3000_core.c Ico Doornekamp 2016-09-13 753 return -EINVAL;
926150211ef327 drivers/iio/accel/sca3000.c Andrew Ijano 2025-06-11 754 guard(mutex)(&st->lock);
926150211ef327 drivers/iio/accel/sca3000.c Andrew Ijano 2025-06-11 755 return sca3000_write_raw_samp_freq(st, val);
626f971b5b0729 drivers/staging/iio/accel/sca3000.c Jonathan Cameron 2016-10-08 756 case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
626f971b5b0729 drivers/staging/iio/accel/sca3000.c Jonathan Cameron 2016-10-08 757 if (val2)
626f971b5b0729 drivers/staging/iio/accel/sca3000.c Jonathan Cameron 2016-10-08 758 return -EINVAL;
926150211ef327 drivers/iio/accel/sca3000.c Andrew Ijano 2025-06-11 759 guard(mutex)(&st->lock);
926150211ef327 drivers/iio/accel/sca3000.c Andrew Ijano 2025-06-11 760 return sca3000_write_3db_freq(st, val);
25888dc51163a5 drivers/staging/iio/accel/sca3000_core.c Jonathan Cameron 2011-05-18 761 default:
25888dc51163a5 drivers/staging/iio/accel/sca3000_core.c Jonathan Cameron 2011-05-18 762 return -EINVAL;
25888dc51163a5 drivers/staging/iio/accel/sca3000_core.c Jonathan Cameron 2011-05-18 763 }
25888dc51163a5 drivers/staging/iio/accel/sca3000_core.c Jonathan Cameron 2011-05-18 764 }
574fb258d63658 drivers/staging/iio/accel/sca3000_core.c Jonathan Cameron 2009-08-18 765
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, 2025-06-11 at 16:39 -0300, Andrew Ijano wrote:
> Use guard(mutex)(&st->lock) for handling mutex lock instead of
> manually locking and unlocking the mutex. This prevents forgotten
> locks due to early exits and remove the need of gotos.
>
> Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
> Co-developed-by: Gustavo Bastos <gustavobastos@usp.br>
> Signed-off-by: Gustavo Bastos <gustavobastos@usp.br>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> ---
> For this one, there are two cases where the previous implementation
> was a smalllocking portion of the code and now it's locking the whole
> function. I don't know if this is a desired behavior.
>
In theory, it should not break anything. But you can always refactor things (like
small helpers) to lock only the code you want. There's also scoped_guard(). I would
say, up to you for re-spinning a new version because of the above :).
Just have something that I'm not totatlly sure... Did you made sure to compile?
AFAIR, guard() had some complains when used in switch() case statements. Maybe that
got improved.
If the above is not an issue:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> drivers/iio/accel/sca3000.c | 177 ++++++++++++------------------------
> 1 file changed, 57 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
> index d41759c68fb4..098d45bad389 100644
> --- a/drivers/iio/accel/sca3000.c
> +++ b/drivers/iio/accel/sca3000.c
> @@ -405,17 +405,14 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
> int ret;
> struct sca3000_state *st = iio_priv(indio_dev);
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_REVID_ADDR));
> if (ret < 0)
> - goto error_ret;
> + return ret;
> dev_info(&indio_dev->dev,
> "sca3000 revision major=%lu, minor=%lu\n",
> ret & SCA3000_REG_REVID_MAJOR_MASK,
> ret & SCA3000_REG_REVID_MINOR_MASK);
> -error_ret:
> - mutex_unlock(&st->lock);
> -
> return ret;
> }
>
> @@ -699,32 +696,25 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> if (chan->type == IIO_ACCEL) {
> - if (st->mo_det_use_count) {
> - mutex_unlock(&st->lock);
> + if (st->mo_det_use_count)
> return -EBUSY;
> - }
> address = sca3000_addresses[chan->address][0];
> ret = spi_w8r16be(st->us, SCA3000_READ_REG(address));
> - if (ret < 0) {
> - mutex_unlock(&st->lock);
> + if (ret < 0)
> return ret;
> - }
> *val = sign_extend32(ret >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> } else {
> /* get the temperature when available */
> ret = spi_w8r16be(st->us,
> SCA3000_READ_REG(SCA3000_REG_TEMP_
> MSB_ADDR));
> - if (ret < 0) {
> - mutex_unlock(&st->lock);
> + if (ret < 0)
> return ret;
> - }
> *val = (ret >> chan->scan_type.shift) &
> GENMASK(chan->scan_type.realbits - 1, 0);
> }
> - mutex_unlock(&st->lock);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> *val = 0;
> @@ -738,14 +728,12 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
> *val2 = 600000;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = sca3000_read_raw_samp_freq(st, val);
> - mutex_unlock(&st->lock);
> return ret ? ret : IIO_VAL_INT;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = sca3000_read_3db_freq(st, val);
> - mutex_unlock(&st->lock);
> return ret;
> default:
> return -EINVAL;
> @@ -763,22 +751,16 @@ static int sca3000_write_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_SAMP_FREQ:
> if (val2)
> return -EINVAL;
> - mutex_lock(&st->lock);
> - ret = sca3000_write_raw_samp_freq(st, val);
> - mutex_unlock(&st->lock);
> - return ret;
> + guard(mutex)(&st->lock);
> + return sca3000_write_raw_samp_freq(st, val);
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> if (val2)
> return -EINVAL;
> - mutex_lock(&st->lock);
> - ret = sca3000_write_3db_freq(st, val);
> - mutex_unlock(&st->lock);
> - return ret;
> + guard(mutex)(&st->lock);
> + return sca3000_write_3db_freq(st, val);
> default:
> return -EINVAL;
> }
> -
> - return ret;
> }
>
> /**
> @@ -800,9 +782,8 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
> struct sca3000_state *st = iio_priv(indio_dev);
> int len = 0, ret;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> - mutex_unlock(&st->lock);
> if (ret)
> return ret;
>
> @@ -851,10 +832,9 @@ static int sca3000_read_event_value(struct iio_dev *indio_dev,
>
> switch (info) {
> case IIO_EV_INFO_VALUE:
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = sca3000_read_ctrl_reg(st,
> sca3000_addresses[chan->address][1]);
> - mutex_unlock(&st->lock);
> if (ret < 0)
> return ret;
> *val = 0;
> @@ -918,13 +898,10 @@ static int sca3000_write_event_value(struct iio_dev
> *indio_dev,
> }
> }
>
> - mutex_lock(&st->lock);
> - ret = sca3000_write_ctrl_reg(st,
> + guard(mutex)(&st->lock);
> + return sca3000_write_ctrl_reg(st,
> sca3000_addresses[chan->address][1],
> nonlinear);
> - mutex_unlock(&st->lock);
> -
> - return ret;
> }
>
> static struct attribute *sca3000_attributes[] = {
> @@ -969,12 +946,12 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> *indio_dev)
> struct sca3000_state *st = iio_priv(indio_dev);
> int ret, i, num_available;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
>
> if (val & SCA3000_REG_INT_STATUS_HALF) {
> ret = spi_w8r8(st->us,
> SCA3000_READ_REG(SCA3000_REG_BUF_COUNT_ADDR));
> if (ret)
> - goto error_ret;
> + return;
> num_available = ret;
> /*
> * num_available is the total number of samples available
> @@ -983,7 +960,7 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> *indio_dev)
> ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR,
> num_available * 2);
> if (ret)
> - goto error_ret;
> + return;
> for (i = 0; i < num_available / 3; i++) {
> /*
> * Dirty hack to cover for 11 bit in fifo, 13 bit
> @@ -995,8 +972,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> *indio_dev)
> iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
> }
> }
> -error_ret:
> - mutex_unlock(&st->lock);
> }
>
> /**
> @@ -1022,9 +997,8 @@ static irqreturn_t sca3000_event_handler(int irq, void
> *private)
> * Could lead if badly timed to an extra read of status reg,
> * but ensures no interrupt is missed.
> */
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_STATUS_ADDR));
> - mutex_unlock(&st->lock);
> if (ret)
> goto done;
>
> @@ -1081,16 +1055,15 @@ static int sca3000_read_event_config(struct iio_dev
> *indio_dev,
> struct sca3000_state *st = iio_priv(indio_dev);
> int ret;
> /* read current value of mode register */
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
>
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> if (ret)
> - goto error_ret;
> + return ret;
>
> switch (chan->channel2) {
> case IIO_MOD_X_AND_Y_AND_Z:
> - ret = !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
> - break;
> + return !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
> case IIO_MOD_X:
> case IIO_MOD_Y:
> case IIO_MOD_Z:
> @@ -1100,24 +1073,18 @@ static int sca3000_read_event_config(struct iio_dev
> *indio_dev,
> */
> if ((ret & SCA3000_REG_MODE_MODE_MASK)
> != SCA3000_REG_MODE_MEAS_MODE_MOT_DET) {
> - ret = 0;
> + return 0;
> } else {
> ret = sca3000_read_ctrl_reg(st,
> SCA3000_REG_CTRL_SEL_MD_CTRL);
> if (ret < 0)
> - goto error_ret;
> + return ret;
> /* only supporting logical or's for now */
> - ret = !!(ret & sca3000_addresses[chan->address][2]);
> + return !!(ret & sca3000_addresses[chan->address][2]);
> }
> - break;
> default:
> - ret = -EINVAL;
> + return -EINVAL;
> }
> -
> -error_ret:
> - mutex_unlock(&st->lock);
> -
> - return ret;
> }
>
> static int sca3000_freefall_set_state(struct iio_dev *indio_dev, bool state)
> @@ -1220,26 +1187,19 @@ static int sca3000_write_event_config(struct iio_dev
> *indio_dev,
> struct sca3000_state *st = iio_priv(indio_dev);
> int ret;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> switch (chan->channel2) {
> case IIO_MOD_X_AND_Y_AND_Z:
> - ret = sca3000_freefall_set_state(indio_dev, state);
> - break;
> -
> + return sca3000_freefall_set_state(indio_dev, state);
> case IIO_MOD_X:
> case IIO_MOD_Y:
> case IIO_MOD_Z:
> - ret = sca3000_motion_detect_set_state(indio_dev,
> + return sca3000_motion_detect_set_state(indio_dev,
> chan->address,
> state);
> - break;
> default:
> - ret = -EINVAL;
> - break;
> + return -EINVAL;
> }
> - mutex_unlock(&st->lock);
> -
> - return ret;
> }
>
> static inline
> @@ -1248,23 +1208,19 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev,
> bool state)
> struct sca3000_state *st = iio_priv(indio_dev);
> int ret;
>
> - mutex_lock(&st->lock);
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> if (ret)
> - goto error_ret;
> + return ret;
> +
> if (state) {
> dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
> - ret = sca3000_write_reg(st,
> + return sca3000_write_reg(st,
> SCA3000_REG_MODE_ADDR,
> (ret | SCA3000_REG_MODE_RING_BUF_ENABLE));
> - } else
> - ret = sca3000_write_reg(st,
> - SCA3000_REG_MODE_ADDR,
> - (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
> -error_ret:
> - mutex_unlock(&st->lock);
> -
> - return ret;
> + }
> + return sca3000_write_reg(st,
> + SCA3000_REG_MODE_ADDR,
> + (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
> }
>
> /**
> @@ -1281,26 +1237,18 @@ static int sca3000_hw_ring_preenable(struct iio_dev
> *indio_dev)
> int ret;
> struct sca3000_state *st = iio_priv(indio_dev);
>
> - mutex_lock(&st->lock);
> -
> + guard(mutex)(&st->lock);
> /* Enable the 50% full interrupt */
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> if (ret)
> - goto error_unlock;
> + return ret;
> ret = sca3000_write_reg(st,
> SCA3000_REG_INT_MASK_ADDR,
> ret | SCA3000_REG_INT_MASK_RING_HALF);
> if (ret)
> - goto error_unlock;
> -
> - mutex_unlock(&st->lock);
> + return ret;
>
> return __sca3000_hw_ring_state_set(indio_dev, 1);
> -
> -error_unlock:
> - mutex_unlock(&st->lock);
> -
> - return ret;
> }
>
> static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
> @@ -1308,22 +1256,18 @@ static int sca3000_hw_ring_postdisable(struct iio_dev
> *indio_dev)
> int ret;
> struct sca3000_state *st = iio_priv(indio_dev);
>
> + guard(mutex)(&st->lock);
> ret = __sca3000_hw_ring_state_set(indio_dev, 0);
> if (ret)
> return ret;
>
> /* Disable the 50% full interrupt */
> - mutex_lock(&st->lock);
> -
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> if (ret)
> - goto unlock;
> - ret = sca3000_write_reg(st,
> + return ret;
> + return sca3000_write_reg(st,
> SCA3000_REG_INT_MASK_ADDR,
> ret & ~SCA3000_REG_INT_MASK_RING_HALF);
> -unlock:
> - mutex_unlock(&st->lock);
> - return ret;
> }
>
> static const struct iio_buffer_setup_ops sca3000_ring_setup_ops = {
> @@ -1343,25 +1287,25 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> {
> int ret;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> /* Ensure all interrupts have been acknowledged */
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> if (ret)
> - goto error_ret;
> + return ret;
>
> /* Turn off all motion detection channels */
> ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL);
> if (ret < 0)
> - goto error_ret;
> + return ret;
> ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL,
> ret & SCA3000_MD_CTRL_PROT_MASK);
> if (ret)
> - goto error_ret;
> + return ret;
>
> /* Disable ring buffer */
> ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
> if (ret < 0)
> - goto error_ret;
> + return ret;
> ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
> (ret & SCA3000_REG_OUT_CTRL_PROT_MASK)
> | SCA3000_REG_OUT_CTRL_BUF_X_EN
> @@ -1369,17 +1313,17 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> | SCA3000_REG_OUT_CTRL_BUF_Z_EN
> | SCA3000_REG_OUT_CTRL_BUF_DIV_4);
> if (ret)
> - goto error_ret;
> + return ret;
> /* Enable interrupts, relevant to mode and set up as active low */
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> if (ret)
> - goto error_ret;
> + return ret;
> ret = sca3000_write_reg(st,
> SCA3000_REG_INT_MASK_ADDR,
> (ret & SCA3000_REG_INT_MASK_PROT_MASK)
> | SCA3000_REG_INT_MASK_ACTIVE_LOW);
> if (ret)
> - goto error_ret;
> + return ret;
> /*
> * Select normal measurement mode, free fall off, ring off
> * Ring in 12 bit mode - it is fine to overwrite reserved bits 3,5
> @@ -1387,13 +1331,9 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> */
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> if (ret)
> - goto error_ret;
> - ret = sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
> + return ret;
> + return sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
> ret & SCA3000_MODE_PROT_MASK);
> -
> -error_ret:
> - mutex_unlock(&st->lock);
> - return ret;
> }
>
> static const struct iio_info sca3000_info = {
> @@ -1471,19 +1411,16 @@ static int sca3000_stop_all_interrupts(struct sca3000_state
> *st)
> {
> int ret;
>
> - mutex_lock(&st->lock);
> + guard(mutex)(&st->lock);
> ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> if (ret)
> - goto error_ret;
> + return ret;
>
> - ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
> + return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
> ret &
> ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
> SCA3000_REG_INT_MASK_RING_HALF |
> SCA3000_REG_INT_MASK_ALL_INTS));
> -error_ret:
> - mutex_unlock(&st->lock);
> - return ret;
> }
>
> static void sca3000_remove(struct spi_device *spi)
On Thu, Jun 12, 2025 at 4:37 AM Nuno Sá <noname.nuno@gmail.com> wrote: > > On Wed, 2025-06-11 at 16:39 -0300, Andrew Ijano wrote: > > --- > > For this one, there are two cases where the previous implementation > > was a smalllocking portion of the code and now it's locking the whole > > function. I don't know if this is a desired behavior. > > > > In theory, it should not break anything. But you can always refactor things (like > small helpers) to lock only the code you want. There's also scoped_guard(). I would > say, up to you for re-spinning a new version because of the above :). Oh, thanks for the suggestion! scoped_guard() is exactly what I was looking for. > Just have something that I'm not totatlly sure... Did you made sure to compile? > AFAIR, guard() had some complains when used in switch() case statements. Maybe that > got improved. > Well, it did compile for me, but as David and Jonathan suggested, it's better to create a scope for that. In fact, when implementing this I looked to other examples that used guard() in switch() case statements, but I didn't notice that they added a local scope too. Thanks, Andrew
On Thu, 12 Jun 2025 07:38:18 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Wed, 2025-06-11 at 16:39 -0300, Andrew Ijano wrote:
> > Use guard(mutex)(&st->lock) for handling mutex lock instead of
> > manually locking and unlocking the mutex. This prevents forgotten
> > locks due to early exits and remove the need of gotos.
> >
> > Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
> > Co-developed-by: Gustavo Bastos <gustavobastos@usp.br>
> > Signed-off-by: Gustavo Bastos <gustavobastos@usp.br>
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > ---
> > For this one, there are two cases where the previous implementation
> > was a smalllocking portion of the code and now it's locking the whole
> > function. I don't know if this is a desired behavior.
> >
>
> In theory, it should not break anything. But you can always refactor things (like
> small helpers) to lock only the code you want. There's also scoped_guard(). I would
> say, up to you for re-spinning a new version because of the above :).
>
> Just have something that I'm not totatlly sure... Did you made sure to compile?
> AFAIR, guard() had some complains when used in switch() case statements. Maybe that
> got improved.
yes, - Needs scope {} to be defined to limit it to the case.
>
> If the above is not an issue:
>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
> > drivers/iio/accel/sca3000.c | 177 ++++++++++++------------------------
> > 1 file changed, 57 insertions(+), 120 deletions(-)
> >
> > diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
> > index d41759c68fb4..098d45bad389 100644
> > --- a/drivers/iio/accel/sca3000.c
> > +++ b/drivers/iio/accel/sca3000.c
> > @@ -405,17 +405,14 @@ static int sca3000_print_rev(struct iio_dev *indio_dev)
> > int ret;
> > struct sca3000_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_REVID_ADDR));
> > if (ret < 0)
> > - goto error_ret;
> > + return ret;
> > dev_info(&indio_dev->dev,
> > "sca3000 revision major=%lu, minor=%lu\n",
> > ret & SCA3000_REG_REVID_MAJOR_MASK,
> > ret & SCA3000_REG_REVID_MINOR_MASK);
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > -
> > return ret;
> > }
> >
> > @@ -699,32 +696,25 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
> >
> > switch (mask) {
> > case IIO_CHAN_INFO_RAW:
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > if (chan->type == IIO_ACCEL) {
> > - if (st->mo_det_use_count) {
> > - mutex_unlock(&st->lock);
> > + if (st->mo_det_use_count)
> > return -EBUSY;
> > - }
> > address = sca3000_addresses[chan->address][0];
> > ret = spi_w8r16be(st->us, SCA3000_READ_REG(address));
> > - if (ret < 0) {
> > - mutex_unlock(&st->lock);
> > + if (ret < 0)
> > return ret;
> > - }
> > *val = sign_extend32(ret >> chan->scan_type.shift,
> > chan->scan_type.realbits - 1);
> > } else {
> > /* get the temperature when available */
> > ret = spi_w8r16be(st->us,
> > SCA3000_READ_REG(SCA3000_REG_TEMP_
> > MSB_ADDR));
> > - if (ret < 0) {
> > - mutex_unlock(&st->lock);
> > + if (ret < 0)
> > return ret;
> > - }
> > *val = (ret >> chan->scan_type.shift) &
> > GENMASK(chan->scan_type.realbits - 1, 0);
> > }
> > - mutex_unlock(&st->lock);
> > return IIO_VAL_INT;
> > case IIO_CHAN_INFO_SCALE:
> > *val = 0;
> > @@ -738,14 +728,12 @@ static int sca3000_read_raw(struct iio_dev *indio_dev,
> > *val2 = 600000;
> > return IIO_VAL_INT_PLUS_MICRO;
> > case IIO_CHAN_INFO_SAMP_FREQ:
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = sca3000_read_raw_samp_freq(st, val);
> > - mutex_unlock(&st->lock);
> > return ret ? ret : IIO_VAL_INT;
> > case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = sca3000_read_3db_freq(st, val);
> > - mutex_unlock(&st->lock);
> > return ret;
> > default:
> > return -EINVAL;
> > @@ -763,22 +751,16 @@ static int sca3000_write_raw(struct iio_dev *indio_dev,
> > case IIO_CHAN_INFO_SAMP_FREQ:
> > if (val2)
> > return -EINVAL;
> > - mutex_lock(&st->lock);
> > - ret = sca3000_write_raw_samp_freq(st, val);
> > - mutex_unlock(&st->lock);
> > - return ret;
> > + guard(mutex)(&st->lock);
> > + return sca3000_write_raw_samp_freq(st, val);
> > case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> > if (val2)
> > return -EINVAL;
> > - mutex_lock(&st->lock);
> > - ret = sca3000_write_3db_freq(st, val);
> > - mutex_unlock(&st->lock);
> > - return ret;
> > + guard(mutex)(&st->lock);
> > + return sca3000_write_3db_freq(st, val);
> > default:
> > return -EINVAL;
> > }
> > -
> > - return ret;
> > }
> >
> > /**
> > @@ -800,9 +782,8 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
> > struct sca3000_state *st = iio_priv(indio_dev);
> > int len = 0, ret;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> > - mutex_unlock(&st->lock);
> > if (ret)
> > return ret;
> >
> > @@ -851,10 +832,9 @@ static int sca3000_read_event_value(struct iio_dev *indio_dev,
> >
> > switch (info) {
> > case IIO_EV_INFO_VALUE:
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = sca3000_read_ctrl_reg(st,
> > sca3000_addresses[chan->address][1]);
> > - mutex_unlock(&st->lock);
> > if (ret < 0)
> > return ret;
> > *val = 0;
> > @@ -918,13 +898,10 @@ static int sca3000_write_event_value(struct iio_dev
> > *indio_dev,
> > }
> > }
> >
> > - mutex_lock(&st->lock);
> > - ret = sca3000_write_ctrl_reg(st,
> > + guard(mutex)(&st->lock);
> > + return sca3000_write_ctrl_reg(st,
> > sca3000_addresses[chan->address][1],
> > nonlinear);
> > - mutex_unlock(&st->lock);
> > -
> > - return ret;
> > }
> >
> > static struct attribute *sca3000_attributes[] = {
> > @@ -969,12 +946,12 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> > *indio_dev)
> > struct sca3000_state *st = iio_priv(indio_dev);
> > int ret, i, num_available;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> >
> > if (val & SCA3000_REG_INT_STATUS_HALF) {
> > ret = spi_w8r8(st->us,
> > SCA3000_READ_REG(SCA3000_REG_BUF_COUNT_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return;
> > num_available = ret;
> > /*
> > * num_available is the total number of samples available
> > @@ -983,7 +960,7 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> > *indio_dev)
> > ret = sca3000_read_data(st, SCA3000_REG_RING_OUT_ADDR,
> > num_available * 2);
> > if (ret)
> > - goto error_ret;
> > + return;
> > for (i = 0; i < num_available / 3; i++) {
> > /*
> > * Dirty hack to cover for 11 bit in fifo, 13 bit
> > @@ -995,8 +972,6 @@ static void sca3000_ring_int_process(u8 val, struct iio_dev
> > *indio_dev)
> > iio_push_to_buffers(indio_dev, st->rx + i * 3 * 2);
> > }
> > }
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > }
> >
> > /**
> > @@ -1022,9 +997,8 @@ static irqreturn_t sca3000_event_handler(int irq, void
> > *private)
> > * Could lead if badly timed to an extra read of status reg,
> > * but ensures no interrupt is missed.
> > */
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_STATUS_ADDR));
> > - mutex_unlock(&st->lock);
> > if (ret)
> > goto done;
> >
> > @@ -1081,16 +1055,15 @@ static int sca3000_read_event_config(struct iio_dev
> > *indio_dev,
> > struct sca3000_state *st = iio_priv(indio_dev);
> > int ret;
> > /* read current value of mode register */
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> >
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return ret;
> >
> > switch (chan->channel2) {
> > case IIO_MOD_X_AND_Y_AND_Z:
> > - ret = !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
> > - break;
> > + return !!(ret & SCA3000_REG_MODE_FREE_FALL_DETECT);
> > case IIO_MOD_X:
> > case IIO_MOD_Y:
> > case IIO_MOD_Z:
> > @@ -1100,24 +1073,18 @@ static int sca3000_read_event_config(struct iio_dev
> > *indio_dev,
> > */
> > if ((ret & SCA3000_REG_MODE_MODE_MASK)
> > != SCA3000_REG_MODE_MEAS_MODE_MOT_DET) {
> > - ret = 0;
> > + return 0;
> > } else {
> > ret = sca3000_read_ctrl_reg(st,
> > SCA3000_REG_CTRL_SEL_MD_CTRL);
> > if (ret < 0)
> > - goto error_ret;
> > + return ret;
> > /* only supporting logical or's for now */
> > - ret = !!(ret & sca3000_addresses[chan->address][2]);
> > + return !!(ret & sca3000_addresses[chan->address][2]);
> > }
> > - break;
> > default:
> > - ret = -EINVAL;
> > + return -EINVAL;
> > }
> > -
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > -
> > - return ret;
> > }
> >
> > static int sca3000_freefall_set_state(struct iio_dev *indio_dev, bool state)
> > @@ -1220,26 +1187,19 @@ static int sca3000_write_event_config(struct iio_dev
> > *indio_dev,
> > struct sca3000_state *st = iio_priv(indio_dev);
> > int ret;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > switch (chan->channel2) {
> > case IIO_MOD_X_AND_Y_AND_Z:
> > - ret = sca3000_freefall_set_state(indio_dev, state);
> > - break;
> > -
> > + return sca3000_freefall_set_state(indio_dev, state);
> > case IIO_MOD_X:
> > case IIO_MOD_Y:
> > case IIO_MOD_Z:
> > - ret = sca3000_motion_detect_set_state(indio_dev,
> > + return sca3000_motion_detect_set_state(indio_dev,
> > chan->address,
> > state);
> > - break;
> > default:
> > - ret = -EINVAL;
> > - break;
> > + return -EINVAL;
> > }
> > - mutex_unlock(&st->lock);
> > -
> > - return ret;
> > }
> >
> > static inline
> > @@ -1248,23 +1208,19 @@ int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev,
> > bool state)
> > struct sca3000_state *st = iio_priv(indio_dev);
> > int ret;
> >
> > - mutex_lock(&st->lock);
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return ret;
> > +
> > if (state) {
> > dev_info(&indio_dev->dev, "supposedly enabling ring buffer\n");
> > - ret = sca3000_write_reg(st,
> > + return sca3000_write_reg(st,
> > SCA3000_REG_MODE_ADDR,
> > (ret | SCA3000_REG_MODE_RING_BUF_ENABLE));
> > - } else
> > - ret = sca3000_write_reg(st,
> > - SCA3000_REG_MODE_ADDR,
> > - (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > -
> > - return ret;
> > + }
> > + return sca3000_write_reg(st,
> > + SCA3000_REG_MODE_ADDR,
> > + (ret & ~SCA3000_REG_MODE_RING_BUF_ENABLE));
> > }
> >
> > /**
> > @@ -1281,26 +1237,18 @@ static int sca3000_hw_ring_preenable(struct iio_dev
> > *indio_dev)
> > int ret;
> > struct sca3000_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(&st->lock);
> > -
> > + guard(mutex)(&st->lock);
> > /* Enable the 50% full interrupt */
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> > if (ret)
> > - goto error_unlock;
> > + return ret;
> > ret = sca3000_write_reg(st,
> > SCA3000_REG_INT_MASK_ADDR,
> > ret | SCA3000_REG_INT_MASK_RING_HALF);
> > if (ret)
> > - goto error_unlock;
> > -
> > - mutex_unlock(&st->lock);
> > + return ret;
> >
> > return __sca3000_hw_ring_state_set(indio_dev, 1);
> > -
> > -error_unlock:
> > - mutex_unlock(&st->lock);
> > -
> > - return ret;
> > }
> >
> > static int sca3000_hw_ring_postdisable(struct iio_dev *indio_dev)
> > @@ -1308,22 +1256,18 @@ static int sca3000_hw_ring_postdisable(struct iio_dev
> > *indio_dev)
> > int ret;
> > struct sca3000_state *st = iio_priv(indio_dev);
> >
> > + guard(mutex)(&st->lock);
> > ret = __sca3000_hw_ring_state_set(indio_dev, 0);
> > if (ret)
> > return ret;
> >
> > /* Disable the 50% full interrupt */
> > - mutex_lock(&st->lock);
> > -
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> > if (ret)
> > - goto unlock;
> > - ret = sca3000_write_reg(st,
> > + return ret;
> > + return sca3000_write_reg(st,
> > SCA3000_REG_INT_MASK_ADDR,
> > ret & ~SCA3000_REG_INT_MASK_RING_HALF);
> > -unlock:
> > - mutex_unlock(&st->lock);
> > - return ret;
> > }
> >
> > static const struct iio_buffer_setup_ops sca3000_ring_setup_ops = {
> > @@ -1343,25 +1287,25 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> > {
> > int ret;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > /* Ensure all interrupts have been acknowledged */
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return ret;
> >
> > /* Turn off all motion detection channels */
> > ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL);
> > if (ret < 0)
> > - goto error_ret;
> > + return ret;
> > ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_MD_CTRL,
> > ret & SCA3000_MD_CTRL_PROT_MASK);
> > if (ret)
> > - goto error_ret;
> > + return ret;
> >
> > /* Disable ring buffer */
> > ret = sca3000_read_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL);
> > if (ret < 0)
> > - goto error_ret;
> > + return ret;
> > ret = sca3000_write_ctrl_reg(st, SCA3000_REG_CTRL_SEL_OUT_CTRL,
> > (ret & SCA3000_REG_OUT_CTRL_PROT_MASK)
> > | SCA3000_REG_OUT_CTRL_BUF_X_EN
> > @@ -1369,17 +1313,17 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> > | SCA3000_REG_OUT_CTRL_BUF_Z_EN
> > | SCA3000_REG_OUT_CTRL_BUF_DIV_4);
> > if (ret)
> > - goto error_ret;
> > + return ret;
> > /* Enable interrupts, relevant to mode and set up as active low */
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return ret;
> > ret = sca3000_write_reg(st,
> > SCA3000_REG_INT_MASK_ADDR,
> > (ret & SCA3000_REG_INT_MASK_PROT_MASK)
> > | SCA3000_REG_INT_MASK_ACTIVE_LOW);
> > if (ret)
> > - goto error_ret;
> > + return ret;
> > /*
> > * Select normal measurement mode, free fall off, ring off
> > * Ring in 12 bit mode - it is fine to overwrite reserved bits 3,5
> > @@ -1387,13 +1331,9 @@ static int sca3000_clean_setup(struct sca3000_state *st)
> > */
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_MODE_ADDR));
> > if (ret)
> > - goto error_ret;
> > - ret = sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
> > + return ret;
> > + return sca3000_write_reg(st, SCA3000_REG_MODE_ADDR,
> > ret & SCA3000_MODE_PROT_MASK);
> > -
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > - return ret;
> > }
> >
> > static const struct iio_info sca3000_info = {
> > @@ -1471,19 +1411,16 @@ static int sca3000_stop_all_interrupts(struct sca3000_state
> > *st)
> > {
> > int ret;
> >
> > - mutex_lock(&st->lock);
> > + guard(mutex)(&st->lock);
> > ret = spi_w8r8(st->us, SCA3000_READ_REG(SCA3000_REG_INT_MASK_ADDR));
> > if (ret)
> > - goto error_ret;
> > + return ret;
> >
> > - ret = sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
> > + return sca3000_write_reg(st, SCA3000_REG_INT_MASK_ADDR,
> > ret &
> > ~(SCA3000_REG_INT_MASK_RING_THREE_QUARTER |
> > SCA3000_REG_INT_MASK_RING_HALF |
> > SCA3000_REG_INT_MASK_ALL_INTS));
> > -error_ret:
> > - mutex_unlock(&st->lock);
> > - return ret;
> > }
> >
> > static void sca3000_remove(struct spi_device *spi)
>
© 2016 - 2025 Red Hat, Inc.