[PATCH v5 0/2] iio: magnetometer: bmc150_magn: cleanup and formatting

[PATCH v5 0/2] iio: magnetometer: bmc150_magn: cleanup and formatting

[Neel Bullywon]

This v5 series refactors the bmc150_magn driver to use automated cleanup
helpers for mutex operations and modernizes the delay function usage.

Changes in v5:
- Split into two patches as requested by maintainer:
  - Patch 1/2: All functional changes (guard/scoped_guard, fsleep)
  - Patch 2/2: Pure formatting/style cleanups
- Use scoped_guard() instead of { guard(); ... } for short scopes
- Left trigger_handler unchanged per maintainer feedback
- Use fsleep() instead of usleep_range() per maintainer feedback
- Dropped formatting changes that did not improve readability
- Added braces around guard() in case blocks for clear scope

Changes in v4:
- Replace scoped_guard() with guard() to avoid lexical scope issues with goto
  and return values which caused logic errors in previous versions.
- Replace msleep(5) with usleep_range(5000, 6000) to avoid checkpatch
  warning.
- Fix indentation and line wrapping to cleanliness.
- Extend guard() usage to all mutex_lock() instances in the driver.

Changes in v3:
- Add Reviewed-by tags.

Changes in v2:
- Use guard() for mutex protection in bmc150_magn_data_rdy_trigger_set_state.

[Patch 1/2] Converts manual mutex_lock/unlock patterns to guard() and
scoped_guard() helpers, and replaces msleep(5) with fsleep(5000).

[Patch 2/2] Adds spaces inside braces for initializer lists and fixes
scan_masks array indentation. No functional changes.

Neel Bullywon (2):
  iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  iio: magnetometer: bmc150_magn: minor formatting cleanup

 drivers/iio/magnetometer/bmc150_magn.c | 161 +++++++++++--------------
 1 file changed, 69 insertions(+), 92 deletions(-)

-- 
2.44.0

[PATCH v5 1/2] iio: magnetometer: bmc150_magn: use automated cleanup for mutex

[Neel Bullywon]

Use guard() and scoped_guard() to replace manual mutex lock/unlock
calls. This simplifies error handling and ensures RAII-style cleanup.

Additionally, replace msleep(5) with fsleep(5000) to allow the kernel
to select the most appropriate delay mechanism based on duration.

guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
and resume. Case blocks in read_raw and write_raw are wrapped in braces
to ensure clear scope for the cleanup guards.

scoped_guard() is used in remove, runtime_suspend, and suspend where a
short mutex-protected scope is needed for a single function call.

The trigger_handler function is left unchanged as mixing guard() with
goto error paths can be fragile.

Signed-off-by: Neel Bullywon <neelb2403@gmail.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 132 ++++++++++---------------
 1 file changed, 54 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 6a73f6e2f1f0..117bf70d6899 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -11,6 +11,7 @@
 
 #include <linux/module.h>
 #include <linux/i2c.h>
+#include <linux/cleanup.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
@@ -456,29 +457,24 @@ static int bmc150_magn_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		if (iio_buffer_enabled(indio_dev))
 			return -EBUSY;
-		mutex_lock(&data->mutex);
+		{
+			guard(mutex)(&data->mutex);
 
-		ret = bmc150_magn_set_power_state(data, true);
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
+			ret = bmc150_magn_set_power_state(data, true);
+			if (ret < 0)
+				return ret;
 
-		ret = bmc150_magn_read_xyz(data, values);
-		if (ret < 0) {
-			bmc150_magn_set_power_state(data, false);
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
-		*val = values[chan->scan_index];
+			ret = bmc150_magn_read_xyz(data, values);
+			if (ret < 0) {
+				bmc150_magn_set_power_state(data, false);
+				return ret;
+			}
+			*val = values[chan->scan_index];
 
-		ret = bmc150_magn_set_power_state(data, false);
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			return ret;
+			ret = bmc150_magn_set_power_state(data, false);
+			if (ret < 0)
+				return ret;
 		}
-
-		mutex_unlock(&data->mutex);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		/*
@@ -530,45 +526,37 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		if (val > data->max_odr)
 			return -EINVAL;
-		mutex_lock(&data->mutex);
-		ret = bmc150_magn_set_odr(data, val);
-		mutex_unlock(&data->mutex);
-		return ret;
+		guard(mutex)(&data->mutex);
+		return bmc150_magn_set_odr(data, val);
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		switch (chan->channel2) {
 		case IIO_MOD_X:
 		case IIO_MOD_Y:
 			if (val < 1 || val > 511)
 				return -EINVAL;
-			mutex_lock(&data->mutex);
-			ret = bmc150_magn_set_max_odr(data, val, 0, 0);
-			if (ret < 0) {
-				mutex_unlock(&data->mutex);
-				return ret;
+			{
+				guard(mutex)(&data->mutex);
+				ret = bmc150_magn_set_max_odr(data, val, 0, 0);
+				if (ret < 0)
+					return ret;
+				return regmap_update_bits(data->regmap,
+							 BMC150_MAGN_REG_REP_XY,
+							 BMC150_MAGN_REG_REP_DATAMASK,
+							 BMC150_MAGN_REPXY_TO_REGVAL(val));
 			}
-			ret = regmap_update_bits(data->regmap,
-						 BMC150_MAGN_REG_REP_XY,
-						 BMC150_MAGN_REG_REP_DATAMASK,
-						 BMC150_MAGN_REPXY_TO_REGVAL
-						 (val));
-			mutex_unlock(&data->mutex);
-			return ret;
 		case IIO_MOD_Z:
 			if (val < 1 || val > 256)
 				return -EINVAL;
-			mutex_lock(&data->mutex);
-			ret = bmc150_magn_set_max_odr(data, 0, val, 0);
-			if (ret < 0) {
-				mutex_unlock(&data->mutex);
-				return ret;
+			{
+				guard(mutex)(&data->mutex);
+				ret = bmc150_magn_set_max_odr(data, 0, val, 0);
+				if (ret < 0)
+					return ret;
+				return regmap_update_bits(data->regmap,
+							 BMC150_MAGN_REG_REP_Z,
+							 BMC150_MAGN_REG_REP_DATAMASK,
+							 BMC150_MAGN_REPZ_TO_REGVAL(val));
 			}
-			ret = regmap_update_bits(data->regmap,
-						 BMC150_MAGN_REG_REP_Z,
-						 BMC150_MAGN_REG_REP_DATAMASK,
-						 BMC150_MAGN_REPZ_TO_REGVAL
-						 (val));
-			mutex_unlock(&data->mutex);
-			return ret;
 		default:
 			return -EINVAL;
 		}
@@ -695,7 +683,7 @@ static int bmc150_magn_init(struct bmc150_magn_data *data)
 	 * 3ms power-on time according to datasheet, let's better
 	 * be safe than sorry and set this delay to 5ms.
 	 */
-	msleep(5);
+	fsleep(5000);
 
 	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND,
 					 false);
@@ -782,9 +770,8 @@ static void bmc150_magn_trig_reen(struct iio_trigger *trig)
 	if (!data->dready_trigger_on)
 		return;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 	ret = bmc150_magn_reset_intr(data);
-	mutex_unlock(&data->mutex);
 	if (ret)
 		dev_err(data->dev, "Failed to reset interrupt\n");
 }
@@ -794,32 +781,28 @@ static int bmc150_magn_data_rdy_trigger_set_state(struct iio_trigger *trig,
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
-	int ret = 0;
+	int ret;
+
+	guard(mutex)(&data->mutex);
 
-	mutex_lock(&data->mutex);
 	if (state == data->dready_trigger_on)
-		goto err_unlock;
+		return 0;
 
 	ret = regmap_update_bits(data->regmap, BMC150_MAGN_REG_INT_DRDY,
 				 BMC150_MAGN_MASK_DRDY_EN,
 				 state << BMC150_MAGN_SHIFT_DRDY_EN);
 	if (ret < 0)
-		goto err_unlock;
+		return ret;
 
 	data->dready_trigger_on = state;
 
 	if (state) {
 		ret = bmc150_magn_reset_intr(data);
 		if (ret < 0)
-			goto err_unlock;
+			return ret;
 	}
-	mutex_unlock(&data->mutex);
 
 	return 0;
-
-err_unlock:
-	mutex_unlock(&data->mutex);
-	return ret;
 }
 
 static const struct iio_trigger_ops bmc150_magn_trigger_ops = {
@@ -980,9 +963,8 @@ void bmc150_magn_remove(struct device *dev)
 	if (data->dready_trig)
 		iio_trigger_unregister(data->dready_trig);
 
-	mutex_lock(&data->mutex);
-	bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);
-	mutex_unlock(&data->mutex);
+	scoped_guard(mutex, &data->mutex)
+		bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);
 
 	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
 }
@@ -995,10 +977,9 @@ static int bmc150_magn_runtime_suspend(struct device *dev)
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&data->mutex);
-	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
-					 true);
-	mutex_unlock(&data->mutex);
+	scoped_guard(mutex, &data->mutex)
+		ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
+						 true);
 	if (ret < 0) {
 		dev_err(dev, "powering off device failed\n");
 		return ret;
@@ -1026,10 +1007,9 @@ static int bmc150_magn_suspend(struct device *dev)
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&data->mutex);
-	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
-					 true);
-	mutex_unlock(&data->mutex);
+	scoped_guard(mutex, &data->mutex)
+		ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
+						 true);
 
 	return ret;
 }
@@ -1038,14 +1018,10 @@ static int bmc150_magn_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmc150_magn_data *data = iio_priv(indio_dev);
-	int ret;
-
-	mutex_lock(&data->mutex);
-	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
-					 true);
-	mutex_unlock(&data->mutex);
 
-	return ret;
+	guard(mutex)(&data->mutex);
+	return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_NORMAL,
+					  true);
 }
 #endif
 
-- 
2.44.0

Re: [PATCH v5 1/2] iio: magnetometer: bmc150_magn: use automated cleanup for mutex

[Ethan Tidmore]

On Sat Feb 7, 2026 at 7:16 PM CST, Neel Bullywon wrote:
> Use guard() and scoped_guard() to replace manual mutex lock/unlock
> calls. This simplifies error handling and ensures RAII-style cleanup.
>
> Additionally, replace msleep(5) with fsleep(5000) to allow the kernel
> to select the most appropriate delay mechanism based on duration.

This should be its own patch. Please keep one logical change per patch.

>
> guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
> and resume. Case blocks in read_raw and write_raw are wrapped in braces
> to ensure clear scope for the cleanup guards.
>
> scoped_guard() is used in remove, runtime_suspend, and suspend where a
> short mutex-protected scope is needed for a single function call.
>
> The trigger_handler function is left unchanged as mixing guard() with
> goto error paths can be fragile.
>
> Signed-off-by: Neel Bullywon <neelb2403@gmail.com>

Thanks,

ET

Re: [PATCH v5 1/2] iio: magnetometer: bmc150_magn: use automated cleanup for mutex

[Andy Shevchenko]

On Sat, Feb 07, 2026 at 08:16:58PM -0500, Neel Bullywon wrote:
> Use guard() and scoped_guard() to replace manual mutex lock/unlock
> calls. This simplifies error handling and ensures RAII-style cleanup.
> 
> Additionally, replace msleep(5) with fsleep(5000) to allow the kernel
> to select the most appropriate delay mechanism based on duration.

Also, if you can fix checkpatch to make a suggesting for fsleep() it
would be even better, so we stop the flow of this checkpatch outdated
recommendations.

> guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
> and resume. Case blocks in read_raw and write_raw are wrapped in braces
> to ensure clear scope for the cleanup guards.
> 
> scoped_guard() is used in remove, runtime_suspend, and suspend where a
> short mutex-protected scope is needed for a single function call.
> 
> The trigger_handler function is left unchanged as mixing guard() with
> goto error paths can be fragile.

...

> +		{
> +			guard(mutex)(&data->mutex);

Hmm... This looks ugly, why not scoped_guard() to begin with?

> +			ret = bmc150_magn_set_power_state(data, true);
> +			if (ret < 0)
> +				return ret;
>  
> +			ret = bmc150_magn_read_xyz(data, values);
> +			if (ret < 0) {
> +				bmc150_magn_set_power_state(data, false);
> +				return ret;
> +			}
> +			*val = values[chan->scan_index];
>  
> +			ret = bmc150_magn_set_power_state(data, false);
> +			if (ret < 0)
> +				return ret;
>  		}
>  		return IIO_VAL_INT;

...

> -	msleep(5);
> +	fsleep(5000);

5 * USEC_PER_MSEC will tell the reader about the time and units used for
the API.

Should be in a separate change.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 2/2] iio: magnetometer: bmc150_magn: minor formatting cleanup

[Neel Bullywon]

Add spaces inside braces for initializer lists in the sampling
frequency and preset tables for consistency.

Fix indentation of bmc150_magn_scan_masks array and add trailing
comma. No functional changes.

Signed-off-by: Neel Bullywon <neelb2403@gmail.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 29 +++++++++++++-------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 117bf70d6899..8e249e25b799 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -149,14 +149,14 @@ struct bmc150_magn_data {
 static const struct {
 	int freq;
 	u8 reg_val;
-} bmc150_magn_samp_freq_table[] = { {2, 0x01},
-				    {6, 0x02},
-				    {8, 0x03},
-				    {10, 0x00},
-				    {15, 0x04},
-				    {20, 0x05},
-				    {25, 0x06},
-				    {30, 0x07} };
+} bmc150_magn_samp_freq_table[] = { { 2, 0x01 },
+				    { 6, 0x02 },
+				    { 8, 0x03 },
+				    { 10, 0x00 },
+				    { 15, 0x04 },
+				    { 20, 0x05 },
+				    { 25, 0x06 },
+				    { 30, 0x07 } };
 
 enum bmc150_magn_presets {
 	LOW_POWER_PRESET,
@@ -170,10 +170,10 @@ static const struct bmc150_magn_preset {
 	u8 rep_z;
 	u8 odr;
 } bmc150_magn_presets_table[] = {
-	[LOW_POWER_PRESET] = {3, 3, 10},
-	[REGULAR_PRESET] =  {9, 15, 10},
-	[ENHANCED_REGULAR_PRESET] =  {15, 27, 10},
-	[HIGH_ACCURACY_PRESET] =  {47, 83, 20},
+	[LOW_POWER_PRESET] = { 3, 3, 10 },
+	[REGULAR_PRESET] = { 9, 15, 10 },
+	[ENHANCED_REGULAR_PRESET] = { 15, 27, 10 },
+	[HIGH_ACCURACY_PRESET] = { 47, 83, 20 },
 };
 
 #define BMC150_MAGN_DEFAULT_PRESET REGULAR_PRESET
@@ -643,8 +643,9 @@ static const struct iio_info bmc150_magn_info = {
 };
 
 static const unsigned long bmc150_magn_scan_masks[] = {
-					BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
-					0};
+	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
+	0,
+};
 
 static irqreturn_t bmc150_magn_trigger_handler(int irq, void *p)
 {
-- 
2.44.0

Re: [PATCH v5 2/2] iio: magnetometer: bmc150_magn: minor formatting cleanup

[Andy Shevchenko]

On Sat, Feb 07, 2026 at 08:16:59PM -0500, Neel Bullywon wrote:
> Add spaces inside braces for initializer lists in the sampling
> frequency and preset tables for consistency.
> 
> Fix indentation of bmc150_magn_scan_masks array and add trailing
> comma. No functional changes.

...

> +} bmc150_magn_samp_freq_table[] = { { 2, 0x01 },
> +				    { 6, 0x02 },
> +				    { 8, 0x03 },
> +				    { 10, 0x00 },
> +				    { 15, 0x04 },
> +				    { 20, 0x05 },
> +				    { 25, 0x06 },
> +				    { 30, 0x07 } };

While at it, fix the style, the above is weird one.

} bmc150_magn_samp_freq_table[] = {
	{ 2, 0x01 },
	{ 6, 0x02 },
	{ 8, 0x03 },
	{ 10, 0x00 },
	{ 15, 0x04 },
	{ 20, 0x05 },
	{ 25, 0x06 },
	{ 30, 0x07 },
// also mind a comma
};

OR alternatively

} bmc150_magn_samp_freq_table[] = {
	{ 2, 0x01 }, { 6, 0x02 }, { 8, 0x03 }, { 10, 0x00 },		/* 0 - 3 */
	{ 15, 0x04 }, { 20, 0x05 }, { 25, 0x06 }, { 30, 0x07 },		/* 4 - 7 */
};

...

>  static const unsigned long bmc150_magn_scan_masks[] = {
> +	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z),
> +	0,

No comma for the terminator, please.

> +};

-- 
With Best Regards,
Andy Shevchenko