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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
v2:
- The following excerpts are from the original cover letter:
- Added small style fixes per Andy's suggestions (Adding blank lines,
moving an if statement in a scoped_guard block).
- Switched out scoped_guard() for guard(mutex)() in certain commits.
- Fixed error in commit 4 where deadlocks could occur, as goto
ignores __attribute__((cleanup)). This has been fixed by the above.
v3:
- Added new helper-wrapper function rm3100_guarded_regmap_bulk_read(),
see commit message.
- Squashed commit into one commit rather than four.
v4:
- Changed function signature of rm3100_guarded_regmap_bulk_read() to
supply struct rm3100_data *data instead of struct mutex *lock per
Andy.
- Renamed rm3100_guarded_regmap_bulk_read() to
rm3100_regmap_bulk_read_locked() per Andy.
v5:
- Fix kernel doc of rm3100_regmap_bulk_read_locked() per Andy.
- Added Andy Shevchenko's reviewed-by tag.
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..c0ce2784e97e 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_regmap_bulk_read_locked() - 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.0On Thu, Apr 30, 2026 at 07:41:47AM -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. ... > + ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit, > + data->buffer, 3); Jonathan, if you feel to tweak this to make it shorter, I mentioned previously this: ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit, data->buffer, 3); > + if (ret < 0) > goto done; I confirm that my Rb stays with or without this change. Maxwell, no need to resend for this. -- With Best Regards, Andy Shevchenko
On Thu, Apr 30, 2026 at 7:58 AM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > I confirm that my Rb stays with or without this change. > Maxwell, no need to resend for this. > Alright, sounds good. Best regards, Maxwell Doose > -- > With Best Regards, > Andy Shevchenko > >
On Thu, 30 Apr 2026 10:43:58 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > On Thu, Apr 30, 2026 at 7:58 AM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > > > I confirm that my Rb stays with or without this change. > > Maxwell, no need to resend for this. > > > > Alright, sounds good. > > Best regards, > Maxwell Doose Applied to the testing branch of iio.git with that tweak. Thanks, Jonathan > > > > > > > -- > > With Best Regards, > > Andy Shevchenko > > > >
On Tue, May 5, 2026 at 7:45 AM Jonathan Cameron <jic23@kernel.org> wrote: > > Applied to the testing branch of iio.git with that tweak. > > Thanks, > > Jonathan > Hey Jonathan, I just noticed that I made a mistake in the commit message, it says "Add new helper-wrapper function rm3100_guarded_regmap_bulk_read()..." when it really should be "Add new helper-wrapper function rm3100_regmap_bulk_read_locked()..." I'm unfortunately not in a position to fix it myself since I'm away, but I just wanted to point it out so it could be fixed. best regards, max
On Tue, 5 May 2026 12:05:11 -0500 Maxwell Doose <m32285159@gmail.com> wrote: > On Tue, May 5, 2026 at 7:45 AM Jonathan Cameron <jic23@kernel.org> wrote: > > > > Applied to the testing branch of iio.git with that tweak. > > > > Thanks, > > > > Jonathan > > > > Hey Jonathan, I just noticed that I made a mistake in the commit > message, it says "Add new helper-wrapper function > rm3100_guarded_regmap_bulk_read()..." when it really should be "Add > new helper-wrapper function rm3100_regmap_bulk_read_locked()..." I'm > unfortunately not in a position to fix it myself since I'm away, but I > just wanted to point it out so it could be fixed. fixed up. > > best regards, > max
On Wed May 6, 2026 at 9:43 AM CDT, Jonathan Cameron wrote: > On Tue, 5 May 2026 12:05:11 -0500 > Maxwell Doose <m32285159@gmail.com> wrote: > >> On Tue, May 5, 2026 at 7:45 AM Jonathan Cameron <jic23@kernel.org> wrote: >> > >> > Applied to the testing branch of iio.git with that tweak. >> > >> > Thanks, >> > >> > Jonathan >> > >> >> Hey Jonathan, I just noticed that I made a mistake in the commit >> message, it says "Add new helper-wrapper function >> rm3100_guarded_regmap_bulk_read()..." when it really should be "Add >> new helper-wrapper function rm3100_regmap_bulk_read_locked()..." I'm >> unfortunately not in a position to fix it myself since I'm away, but I >> just wanted to point it out so it could be fixed. > fixed up. >> >> best regards, >> max Gosh, I'm so sorry to keep bothering you, but I noticed a stray change here: @@ -7,7 +7,7 @@ * User Manual available at * <https://www.pnicorp.com/download/rm3100-user-manual/> * - * TODO: event generation, pm. + * TODO: event generation, pm.da */ wishing vs code would quit messing with me, max
On Wed, 06 May 2026 10:28:29 -0500 "Maxwell Doose" <m32285159@gmail.com> wrote: > On Wed May 6, 2026 at 9:43 AM CDT, Jonathan Cameron wrote: > > On Tue, 5 May 2026 12:05:11 -0500 > > Maxwell Doose <m32285159@gmail.com> wrote: > > > >> On Tue, May 5, 2026 at 7:45 AM Jonathan Cameron <jic23@kernel.org> wrote: > >> > > >> > Applied to the testing branch of iio.git with that tweak. > >> > > >> > Thanks, > >> > > >> > Jonathan > >> > > >> > >> Hey Jonathan, I just noticed that I made a mistake in the commit > >> message, it says "Add new helper-wrapper function > >> rm3100_guarded_regmap_bulk_read()..." when it really should be "Add > >> new helper-wrapper function rm3100_regmap_bulk_read_locked()..." I'm > >> unfortunately not in a position to fix it myself since I'm away, but I > >> just wanted to point it out so it could be fixed. > > fixed up. > >> > >> best regards, > >> max > > Gosh, I'm so sorry to keep bothering you, but I noticed a stray change > here: > > @@ -7,7 +7,7 @@ > * User Manual available at > * <https://www.pnicorp.com/download/rm3100-user-manual/> > * > - * TODO: event generation, pm. > + * TODO: event generation, pm.da > */ > > wishing vs code would quit messing with me, > max Doh. That wasn't there in the original so I must have messed up. Good spot. Fixed up.
On Thu, Apr 30, 2026 at 7:41 AM Maxwell Doose <m32285159@gmail.com> 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. > > Suggested-by: Jonathan Cameron <jic23@kernel.org> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > Signed-off-by: Maxwell Doose <m32285159@gmail.com> > --- > v2: > - The following excerpts are from the original cover letter: > - Added small style fixes per Andy's suggestions (Adding blank lines, > moving an if statement in a scoped_guard block). > - Switched out scoped_guard() for guard(mutex)() in certain commits. > - Fixed error in commit 4 where deadlocks could occur, as goto > ignores __attribute__((cleanup)). This has been fixed by the above. > > v3: > - Added new helper-wrapper function rm3100_guarded_regmap_bulk_read(), > see commit message. > - Squashed commit into one commit rather than four. > > v4: > - Changed function signature of rm3100_guarded_regmap_bulk_read() to > supply struct rm3100_data *data instead of struct mutex *lock per > Andy. > - Renamed rm3100_guarded_regmap_bulk_read() to > rm3100_regmap_bulk_read_locked() per Andy. > > v5: > - Fix kernel doc of rm3100_regmap_bulk_read_locked() per Andy. > - Added Andy Shevchenko's reviewed-by tag. > > drivers/iio/magnetometer/rm3100-core.c | 93 ++++++++++++++------------ > 1 file changed, 50 insertions(+), 43 deletions(-) > [snip] Hi, just wanted to put the link to where Andy gave me permission to use his reviewed-by tag just to make sure no confusion arises. https://lore.kernel.org/linux-iio/CAKqfh0H1tq1eC5h54G1Vft26Qkaa4nYEdhwQAk=1R6CNXgnveQ@mail.gmail.com/T/#m5ad582163d5d9620c59edcd1a78eceee070677f7 best regards, maxwell
© 2016 - 2026 Red Hat, Inc.