[PATCH 0/4] iio: magnetometer: rm3100: Modernize locking and control flow

[PATCH 0/4] iio: magnetometer: rm3100: Modernize locking and control flow

[Maxwell Doose]

The goal of this patch series is to replace the manual mutex_lock() and
mutex_unlock() calls in rm3100-core.c with their more modern
counterparts, guard(mutex)() and scoped_guard(). I've also done some
minor cleanups, removing what are now redundant gotos, and enabling
direct returns.

Following feedback on recent commits, I've learned that it's a better
idea to split changes to be more atomic, and I've done that here in
case any particular change *somehow* causes a build regression. The
changes have been test-compiled however, and according to make, smatch,
and sparse, should be sound.

Maxwell Doose (4):
  iio: magnetometer: rm3100: Use scoped_guard() in rm3100_read_mag()
  iio: magnetometer: rm3100: Use scoped_guard() in
    rm3100_get_samp_freq()
  iio: magnetometer: rm3100: Use guard(mutex)() in
    rm3100_set_samp_freq()
  iio: magnetometer: rm3100: Use scoped_guard in
    rm3100_trigger_handler()

 drivers/iio/magnetometer/rm3100-core.c | 129 +++++++++++--------------
 1 file changed, 56 insertions(+), 73 deletions(-)

-- 
2.53.0

[PATCH 1/4] iio: magnetometer: rm3100: Use scoped_guard() in rm3100_read_mag()

[Maxwell Doose]

Replace mutex_lock() and mutex_unlock() calls in rm3100_read_mag() with
the more modern scoped_guard(). This will help modernize the driver and
bring it up-to-date with modern available macros/functions.

While at it, remove the now unnecessary "unlock_return" goto and
directly return in if statements.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/magnetometer/rm3100-core.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 2b2884425746..a2bf6e504a15 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);
-	ret = regmap_write(regmap, RM3100_REG_POLL, BIT(4 + idx));
-	if (ret < 0)
-		goto unlock_return;
+	scoped_guard(mutex, &data->lock) {
+		ret = regmap_write(regmap, RM3100_REG_POLL, BIT(4 + idx));
+		if (ret < 0)
+			return ret;
 
-	ret = rm3100_wait_measurement(data);
-	if (ret < 0)
-		goto unlock_return;
+		ret = rm3100_wait_measurement(data);
+		if (ret < 0)
+			return ret;
 
-	ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * idx, buffer, 3);
-	if (ret < 0)
-		goto unlock_return;
-	mutex_unlock(&data->lock);
+		ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * idx, buffer, 3);
+		if (ret < 0)
+			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)					\
-- 
2.53.0

Re: [PATCH 1/4] iio: magnetometer: rm3100: Use scoped_guard() in rm3100_read_mag()

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:43:36PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in rm3100_read_mag() with
> the more modern scoped_guard(). This will help modernize the driver and
> bring it up-to-date with modern available macros/functions.
> 
> While at it, remove the now unnecessary "unlock_return" goto and
> directly return in if statements.

...

> +	scoped_guard(mutex, &data->lock) {

> +	}

While this is strictly correct change, I would prefer to see guard()() for the
sake of less unneeded noise in the change.

>  	*val = sign_extend32(get_unaligned_be24(&buffer[0]), 23);

Yep, this will become part of the critical section, but taking into account
the size of the rest (and how much CPU cycles it might take at run-time)
I do not believe this piece of memory access with sign extension algo will
anyhow affect the duration of execution of the critical section.

>  	return IIO_VAL_INT;

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/4] iio: magnetometer: rm3100: Use scoped_guard() in rm3100_get_samp_freq()

[Maxwell Doose]

Replace mutex_lock() and mutex_unlock() calls in rm3100_get_samp_freq()
with the more modern scoped_guard(). This will help modernize the
driver and bring it up-to-date with modern available macros/functions.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/magnetometer/rm3100-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index a2bf6e504a15..76824dcb1b73 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -280,9 +280,9 @@ static int rm3100_get_samp_freq(struct rm3100_data *data, int *val, int *val2)
 	unsigned int tmp;
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = regmap_read(data->regmap, RM3100_REG_TMRC, &tmp);
-	mutex_unlock(&data->lock);
+	scoped_guard(mutex, &data->lock) {
+		ret = regmap_read(data->regmap, RM3100_REG_TMRC, &tmp);
+	}
 	if (ret < 0)
 		return ret;
 	*val = rm3100_samp_rates[tmp - RM3100_TMRC_OFFSET][0];
-- 
2.53.0

Re: [PATCH 2/4] iio: magnetometer: rm3100: Use scoped_guard() in rm3100_get_samp_freq()

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:43:37PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in rm3100_get_samp_freq()
> with the more modern scoped_guard(). This will help modernize the
> driver and bring it up-to-date with modern available macros/functions.

...

> -	mutex_lock(&data->lock);
> -	ret = regmap_read(data->regmap, RM3100_REG_TMRC, &tmp);
> -	mutex_unlock(&data->lock);
> +	scoped_guard(mutex, &data->lock) {
> +		ret = regmap_read(data->regmap, RM3100_REG_TMRC, &tmp);
> +	}

{} are mot used for the single statement, but...

>  	if (ret < 0)
>  		return ret;

...move this inside the body instead.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/4] iio: magnetometer: rm3100: Use guard(mutex)() in rm3100_set_samp_freq()

[Maxwell Doose]

Replace mutex_lock() and mutex_unlock() calls in rm3100_set_samp_freq
with the more modern guard(mutex)(). This will help modernize the
driver and bring it up-to-date with modern available macros/functions.

While at it, remove unlock_return goto and use direct returns instead.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/magnetometer/rm3100-core.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 76824dcb1b73..a2c3448e296c 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -334,56 +334,49 @@ 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,
-- 
2.53.0

Re: [PATCH 3/4] iio: magnetometer: rm3100: Use guard(mutex)() in rm3100_set_samp_freq()

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:43:38PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in rm3100_set_samp_freq
> with the more modern guard(mutex)(). This will help modernize the
> driver and bring it up-to-date with modern available macros/functions.
> 
> While at it, remove unlock_return goto and use direct returns instead.

...

> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);

+ Blank line now.

>  	/* 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;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] iio: magnetometer: rm3100: Use guard(mutex)() in rm3100_set_samp_freq()

[Maxwell Doose]

On Tue, Apr 28, 2026 at 3:28 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> ...
>
> > -     mutex_lock(&data->lock);
> > +     guard(mutex)(&data->lock);
>
> + Blank line now.
>

Sounds good, I'll get that done while I'm fixing up some of the other patches.

best regards,
maxwell

[PATCH 4/4] iio: magnetometer: rm3100: Use scoped_guard in rm3100_trigger_handler()

[Maxwell Doose]

Replace mutex_lock() and mutex_unlock() calls in
rm3100_trigger_handler() with the more modern scoped_guard(). This will
help modernize the driver and bring it up-to-date with modern available
macros/functions.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/iio/magnetometer/rm3100-core.c | 72 ++++++++++++--------------
 1 file changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index a2c3448e296c..f55796e3ebef 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -457,48 +457,42 @@ static irqreturn_t rm3100_trigger_handler(int irq, void *p)
 	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);
-		if (ret < 0)
-			goto done;
-		/* Convert XXXYYYZZZxxx to XXXxYYYxZZZx. x for paddings. */
-		for (i = 2; i > 0; i--)
-			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);
-		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);
-		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);
-		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);
+	scoped_guard(mutex, &data->lock) {
+		switch (scan_mask) {
+		case BIT(0) | BIT(1) | BIT(2):
+			ret = regmap_bulk_read(regmap, RM3100_REG_MX2, data->buffer, 9);
+			if (ret < 0)
+				goto done;
+			/* Convert XXXYYYZZZxxx to XXXxYYYxZZZx. x for paddings. */
+			for (i = 2; i > 0; i--)
+				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);
+			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);
+			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);
+			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)
+					goto done;
 			}
 		}
-		mutex_unlock(&data->lock);
 	}
 	/*
 	 * Always using the same buffer so that we wouldn't need to set the
-- 
2.53.0

Re: [PATCH 4/4] iio: magnetometer: rm3100: Use scoped_guard in rm3100_trigger_handler()

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 09:43:39PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in
> rm3100_trigger_handler() with the more modern scoped_guard(). This will
> help modernize the driver and bring it up-to-date with modern available
> macros/functions.

...

> +	scoped_guard(mutex, &data->lock) {

> +			if (ret < 0)
> +				goto done;


This is simply wrong.

>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/4] iio: magnetometer: rm3100: Use scoped_guard in rm3100_trigger_handler()

[Maxwell Doose]

On Tue, Apr 28, 2026 at 5:05 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> > +     scoped_guard(mutex, &data->lock) {
>
> > +                     if (ret < 0)
> > +                             goto done;
>
>
> This is simply wrong.
>

Sorry, didn't realize goto was so blunt as to bypass
__attribute__((cleanup)), I rarely use it myself. I'll get that fixed
right away.

best regards
maxwell

[PATCH 0/4] iio: magnetometer: rm3100: Modernize locking and control flow

[Maxwell Doose]

The goal of this patch series is to replace the manual mutex_lock() and
mutex_unlock() calls in rm3100-core.c with their more modern
counterparts, guard(mutex)() and scoped_guard(). I've also done some
minor cleanups, removing what are now redundant gotos, and enabling
direct returns.

Following feedback on recent commits, I've learned that it's a better
idea to split changes to be more atomic, and I've done that here in
case any particular change *somehow* causes a build regression. The
changes have been test-compiled however, and according to make, smatch,
and sparse, should be sound.

=== Changelog ===
v2:
 - 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.

Maxwell Doose (4):
  iio: magnetometer: rm3100: Use guard(mutex)() in rm3100_read_mag()
  iio: magnetometer: rm3100: Use scoped_guard() in
    rm3100_get_samp_freq()
  iio: magnetometer: rm3100: Use guard(mutex)() in
    rm3100_set_samp_freq()
  iio: magnetometer: rm3100: Use guard(mutex)() in
    rm3100_trigger_handler()

 drivers/iio/magnetometer/rm3100-core.c | 63 ++++++++++----------------
 1 file changed, 24 insertions(+), 39 deletions(-)

-- 
2.53.0