drivers/iio/magnetometer/rm3100-core.c | 93 ++++++++++++++------------ 1 file changed, 50 insertions(+), 43 deletions(-)
Replace mutex_lock() and mutex_unlock() calls in rm3100-core.c with
the more modern guard(mutex)() family. This will help modernize the
driver and bring it up-to-date with modern available macros/functions.
While replacing mutex_lock() and mutex_unlock(), the critical sections
of rm3100_read_mag() and rm3100_get_samp_freq() have been extended to
include negligible operations for cleaner logic.
Add new helper-wrapper function rm3100_guarded_regmap_bulk_read() to
help keep rm3100_trigger_handler() switch-cases clean while maintaining
mutex locking and avoiding re-entrancy risks from potential callbacks.
While at it, remove redundant gotos where applicable, and use direct
returns instead. In addition, remove regmap variable in
rm3100_trigger_handler() as its references have been replaced with
variable data.
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
drivers/iio/magnetometer/rm3100-core.c | 93 ++++++++++++++------------
1 file changed, 50 insertions(+), 43 deletions(-)
diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 2b2884425746..ffa9b46278ed 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -204,27 +204,23 @@ static int rm3100_read_mag(struct rm3100_data *data, int idx, int *val)
u8 buffer[3];
int ret;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
+
ret = regmap_write(regmap, RM3100_REG_POLL, BIT(4 + idx));
if (ret < 0)
- goto unlock_return;
+ return ret;
ret = rm3100_wait_measurement(data);
if (ret < 0)
- goto unlock_return;
+ return ret;
ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * idx, buffer, 3);
if (ret < 0)
- goto unlock_return;
- mutex_unlock(&data->lock);
+ return ret;
*val = sign_extend32(get_unaligned_be24(&buffer[0]), 23);
return IIO_VAL_INT;
-
-unlock_return:
- mutex_unlock(&data->lock);
- return ret;
}
#define RM3100_CHANNEL(axis, idx) \
@@ -284,11 +280,12 @@ static int rm3100_get_samp_freq(struct rm3100_data *data, int *val, int *val2)
unsigned int tmp;
int ret;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
+
ret = regmap_read(data->regmap, RM3100_REG_TMRC, &tmp);
- mutex_unlock(&data->lock);
if (ret < 0)
return ret;
+
*val = rm3100_samp_rates[tmp - RM3100_TMRC_OFFSET][0];
*val2 = rm3100_samp_rates[tmp - RM3100_TMRC_OFFSET][1];
@@ -338,56 +335,50 @@ static int rm3100_set_samp_freq(struct iio_dev *indio_dev, int val, int val2)
int ret;
int i;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
+
/* All cycle count registers use the same value. */
ret = regmap_read(regmap, RM3100_REG_CC_X, &cycle_count);
if (ret < 0)
- goto unlock_return;
+ return ret;
for (i = 0; i < RM3100_SAMP_NUM; i++) {
if (val == rm3100_samp_rates[i][0] &&
val2 == rm3100_samp_rates[i][1])
break;
}
- if (i == RM3100_SAMP_NUM) {
- ret = -EINVAL;
- goto unlock_return;
- }
+ if (i == RM3100_SAMP_NUM)
+ return -EINVAL;
ret = regmap_write(regmap, RM3100_REG_TMRC, i + RM3100_TMRC_OFFSET);
if (ret < 0)
- goto unlock_return;
+ return ret;
/* Checking if cycle count registers need changing. */
if (val == 600 && cycle_count == 200) {
ret = rm3100_set_cycle_count(data, 100);
if (ret < 0)
- goto unlock_return;
+ return ret;
} else if (val != 600 && cycle_count == 100) {
ret = rm3100_set_cycle_count(data, 200);
if (ret < 0)
- goto unlock_return;
+ return ret;
}
if (iio_buffer_enabled(indio_dev)) {
/* Writing TMRC registers requires CMM reset. */
ret = regmap_write(regmap, RM3100_REG_CMM, 0);
if (ret < 0)
- goto unlock_return;
+ return ret;
ret = regmap_write(data->regmap, RM3100_REG_CMM,
(*indio_dev->active_scan_mask & 0x7) <<
RM3100_CMM_AXIS_SHIFT | RM3100_CMM_START);
if (ret < 0)
- goto unlock_return;
+ return ret;
}
- mutex_unlock(&data->lock);
data->conversion_time = rm3100_samp_rates[i][2] * 2;
return 0;
-
-unlock_return:
- mutex_unlock(&data->lock);
- return ret;
}
static int rm3100_read_raw(struct iio_dev *indio_dev,
@@ -458,6 +449,27 @@ static const struct iio_buffer_setup_ops rm3100_buffer_ops = {
.postdisable = rm3100_buffer_postdisable,
};
+/**
+ * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex
+ *
+ * @data: Data structure containing regmap and mutex
+ * @reg: First register to be read from, passed to regmap_bulk_read()
+ * @val: Pointer to store read value, in native register size for device,
+ * passed to regmap_bulk_read()
+ * @val_count: Number of registers to read, passed to regmap_bulk_read()
+ *
+ * Intended for use only in rm3100_trigger_handler().
+ *
+ * Return:
+ * A value of zero on success, a negative errno in error cases.
+ */
+static int rm3100_regmap_bulk_read_locked(struct rm3100_data *data, unsigned int reg,
+ void *val, size_t val_count)
+{
+ guard(mutex)(&data->lock);
+ return regmap_bulk_read(data->regmap, reg, val, val_count);
+}
+
static irqreturn_t rm3100_trigger_handler(int irq, void *p)
{
struct iio_poll_func *pf = p;
@@ -465,14 +477,12 @@ static irqreturn_t rm3100_trigger_handler(int irq, void *p)
unsigned long scan_mask = *indio_dev->active_scan_mask;
unsigned int mask_len = iio_get_masklength(indio_dev);
struct rm3100_data *data = iio_priv(indio_dev);
- struct regmap *regmap = data->regmap;
int ret, i, bit;
- mutex_lock(&data->lock);
switch (scan_mask) {
case BIT(0) | BIT(1) | BIT(2):
- ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9);
- mutex_unlock(&data->lock);
+ ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2,
+ data->buffer, 9);
if (ret < 0)
goto done;
/* Convert XXXYYYZZZxxx to XXXxYYYxZZZx. x for paddings. */
@@ -480,36 +490,33 @@ static irqreturn_t rm3100_trigger_handler(int irq, void *p)
memmove(data->buffer + i * 4, data->buffer + i * 3, 3);
break;
case BIT(0) | BIT(1):
- ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 6);
- mutex_unlock(&data->lock);
+ ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2,
+ data->buffer, 6);
if (ret < 0)
goto done;
memmove(data->buffer + 4, data->buffer + 3, 3);
break;
case BIT(1) | BIT(2):
- ret = regmap_bulk_read(regmap, RM3100_REG_MY2, data->buffer, 6);
- mutex_unlock(&data->lock);
+ ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MY2,
+ data->buffer, 6);
if (ret < 0)
goto done;
memmove(data->buffer + 4, data->buffer + 3, 3);
break;
case BIT(0) | BIT(2):
- ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9);
- mutex_unlock(&data->lock);
+ ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2,
+ data->buffer, 9);
if (ret < 0)
goto done;
memmove(data->buffer + 4, data->buffer + 6, 3);
break;
default:
for_each_set_bit(bit, &scan_mask, mask_len) {
- ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * bit,
- data->buffer, 3);
- if (ret < 0) {
- mutex_unlock(&data->lock);
+ ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit,
+ data->buffer, 3);
+ if (ret < 0)
goto done;
- }
}
- mutex_unlock(&data->lock);
}
/*
* Always using the same buffer so that we wouldn't need to set the
--
2.53.0Hi Maxwell, kernel test robot noticed the following build warnings: [auto build test WARNING on jic23-iio/togreg] [also build test WARNING on linus/master v7.1-rc1 next-20260430] [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/Maxwell-Doose/iio-magnetometer-rm3100-Modernize-locking-and-refactor-control-flow/20260502-014808 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg patch link: https://lore.kernel.org/r/20260430021439.67591-1-m32285159%40gmail.com patch subject: [PATCH v4] iio: magnetometer: rm3100: Modernize locking and refactor control flow config: s390-randconfig-r071-20260503 (https://download.01.org/0day-ci/archive/20260503/202605030817.kUz9B6MC-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 15.2.0 smatch: v0.5.0-9065-ge9cc34fd reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260503/202605030817.kUz9B6MC-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/202605030817.kUz9B6MC-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: drivers/iio/magnetometer/rm3100-core.c:467 expecting prototype for rm3100_guarded_regmap_bulk_read(). Prototype was for rm3100_regmap_bulk_read_locked() instead -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
On Wed, Apr 29, 2026 at 09:14:39PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in rm3100-core.c with
> the more modern guard(mutex)() family. This will help modernize the
> driver and bring it up-to-date with modern available macros/functions.
>
> While replacing mutex_lock() and mutex_unlock(), the critical sections
> of rm3100_read_mag() and rm3100_get_samp_freq() have been extended to
> include negligible operations for cleaner logic.
>
> Add new helper-wrapper function rm3100_guarded_regmap_bulk_read() to
> help keep rm3100_trigger_handler() switch-cases clean while maintaining
> mutex locking and avoiding re-entrancy risks from potential callbacks.
>
> While at it, remove redundant gotos where applicable, and use direct
> returns instead. In addition, remove regmap variable in
> rm3100_trigger_handler() as its references have been replaced with
> variable data.
Almost there, a couple of minor issues and feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
(Side note:
compare this change to ACPI PMIC one, you see what I have told about
in that thread.)
...
> +/**
> + * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex
Always validate kernel-doc
scripts/kernel-doc -v -none -Wall -- $FILE
You forgot to update function name here.
> + * @data: Data structure containing regmap and mutex
> + * @reg: First register to be read from, passed to regmap_bulk_read()
> + * @val: Pointer to store read value, in native register size for device,
> + * passed to regmap_bulk_read()
> + * @val_count: Number of registers to read, passed to regmap_bulk_read()
> + *
> + * Intended for use only in rm3100_trigger_handler().
> + *
> + * Return:
> + * A value of zero on success, a negative errno in error cases.
> + */
...
> for_each_set_bit(bit, &scan_mask, mask_len) {
> - ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * bit,
> - data->buffer, 3);
> - if (ret < 0) {
> - mutex_unlock(&data->lock);
> + ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit,
> + data->buffer, 3);
ret = rm3100_regmap_bulk_read_locked(data,
RM3100_REG_MX2 + 3 * bit,
data->buffer, 3);
> + if (ret < 0)
> goto done;
> - }
> }
--
With Best Regards,
Andy Shevchenko
On Thu, Apr 30, 2026 at 1:31 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> Almost there, a couple of minor issues and feel free to add
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
Alright sounds good, I'll make sure to add the recommended changes
below, but right now it's incredibly late over here, so I'll get back
with that patch later.
> (Side note:
> compare this change to ACPI PMIC one, you see what I have told about
> in that thread.)
I probably should stop wasting my time over there, I'm tripping over
my own commit details there and that driver seems to be at the point
of diminishing returns.
>
> ...
>
> > +/**
> > + * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex
>
> Always validate kernel-doc
>
> scripts/kernel-doc -v -none -Wall -- $FILE
Thanks for informing me about that, I didn't realize there was a
script for that.
>
>
> You forgot to update function name here.
Whoops, nice catch.
Anyways, I think I need to sincerely thank you for your help on this,
it's been great.
Best Regards,
Maxwell Doose
>
> > + * @data: Data structure containing regmap and mutex
> > + * @reg: First register to be read from, passed to regmap_bulk_read()
> > + * @val: Pointer to store read value, in native register size for device,
> > + * passed to regmap_bulk_read()
> > + * @val_count: Number of registers to read, passed to regmap_bulk_read()
> > + *
> > + * Intended for use only in rm3100_trigger_handler().
> > + *
> > + * Return:
> > + * A value of zero on success, a negative errno in error cases.
> > + */
>
> ...
>
> > for_each_set_bit(bit, &scan_mask, mask_len) {
> > - ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * bit,
> > - data->buffer, 3);
> > - if (ret < 0) {
> > - mutex_unlock(&data->lock);
> > + ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit,
> > + data->buffer, 3);
>
> ret = rm3100_regmap_bulk_read_locked(data,
> RM3100_REG_MX2 + 3 * bit,
> data->buffer, 3);
>
> > + if (ret < 0)
> > goto done;
> > - }
> > }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
© 2016 - 2026 Red Hat, Inc.