public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting
@ 2026-02-10 23:39 Neel Bullywon
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Neel Bullywon @ 2026-02-10 23:39 UTC (permalink / raw)
  To: jic23; +Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Neel Bullywon

This series cleans up the bmc150_magn driver in three patches:

1/3: Replace manual mutex lock/unlock with guard()/scoped_guard()
2/3: Replace msleep(5) with fsleep(5 * USEC_PER_MSEC)
3/3: Minor formatting cleanup for initializer lists and indentation

Changes in v6:
- Split fsleep change into its own patch (Andy).
- Use scoped_guard() instead of { guard() } for the IIO_CHAN_INFO_RAW
  case block in read_raw (Andy).
- Use 5 * USEC_PER_MSEC instead of 5000 for fsleep (Andy).
- Restyle bmc150_magn_samp_freq_table to group four entries per line
  with index comments (Andy).
- Remove trailing comma from scan_masks terminator (Andy).

Changes in v5:
- Split into two patches: functional and formatting (Jonathan).
- Use fsleep() instead of usleep_range() (Jonathan).
- Use scoped_guard() for short single-statement scopes (Jonathan).
- Add {} scope for guard() in case blocks (Jonathan).
- Leave trigger_handler unchanged (Jonathan).
- Drop all unnecessary reformatting (Jonathan).

Changes in v4:
- Extend guard() usage to all mutex_lock() instances in the driver.
- Replace msleep(5) with usleep_range(5000, 6000).

Changes in v3:
- Add Reviewed-by tags.

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

Neel Bullywon (3):
  iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  iio: magnetometer: bmc150_magn: replace msleep with fsleep
  iio: magnetometer: bmc150_magn: minor formatting cleanup

 drivers/iio/magnetometer/bmc150_magn.c | 157 ++++++++++---------------
 1 file changed, 64 insertions(+), 93 deletions(-)


base-commit: e7aa57247700733e52a8e2e4dee6a52c2a76de02
-- 
2.44.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  2026-02-10 23:39 [PATCH v6 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
@ 2026-02-10 23:39 ` Neel Bullywon
  2026-02-11  8:39   ` Andy Shevchenko
                     ` (2 more replies)
  2026-02-10 23:39 ` [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
  2026-02-10 23:39 ` [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
  2 siblings, 3 replies; 12+ messages in thread
From: Neel Bullywon @ 2026-02-10 23:39 UTC (permalink / raw)
  To: jic23; +Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Neel Bullywon

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

scoped_guard() is used in read_raw IIO_CHAN_INFO_RAW case for a
multi-statement mutex-protected block, as well as in remove,
runtime_suspend, and suspend where a short mutex-protected scope
is needed for a single function call.

guard() is used in write_raw and trig_reen where the mutex scope
extends to the end of the function or case block. Case blocks in
write_raw are wrapped in braces to ensure clear scope for the
cleanup guards.

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 | 130 ++++++++++---------------
 1 file changed, 52 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 6a73f6e2f1f0..5eb710eb020d 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,22 @@ 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);
-
-		ret = bmc150_magn_set_power_state(data, true);
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
+		scoped_guard(mutex, &data->mutex) {
+			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 +524,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;
 		}
@@ -782,9 +768,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 +779,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 +961,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 +975,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 +1005,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 +1016,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep
  2026-02-10 23:39 [PATCH v6 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
@ 2026-02-10 23:39 ` Neel Bullywon
  2026-02-11  8:40   ` Andy Shevchenko
  2026-02-10 23:39 ` [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
  2 siblings, 1 reply; 12+ messages in thread
From: Neel Bullywon @ 2026-02-10 23:39 UTC (permalink / raw)
  To: jic23; +Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Neel Bullywon

Replace msleep(5) with fsleep(5 * USEC_PER_MSEC) to allow the kernel
to select the most appropriate delay mechanism based on duration.
Using USEC_PER_MSEC makes the unit conversion explicit.

Signed-off-by: Neel Bullywon <neelb2403@gmail.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 5eb710eb020d..f254774d28e5 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -681,7 +681,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(5 * USEC_PER_MSEC);
 
 	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND,
 					 false);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
  2026-02-10 23:39 [PATCH v6 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
  2026-02-10 23:39 ` [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
@ 2026-02-10 23:39 ` Neel Bullywon
  2026-02-11  8:41   ` Andy Shevchenko
  2026-02-14 16:31   ` David Lechner
  2 siblings, 2 replies; 12+ messages in thread
From: Neel Bullywon @ 2026-02-10 23:39 UTC (permalink / raw)
  To: jic23; +Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Neel Bullywon

Improve initializer list style for bmc150_magn_samp_freq_table by
moving the opening brace to its own line and grouping four entries
per line with index comments.

Add spaces inside braces for initializer lists in the preset table
for consistency.

Fix indentation of bmc150_magn_scan_masks array. No functional changes.

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

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index f254774d28e5..942c2f5e97e1 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -149,14 +149,10 @@ 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 },		/* 0 - 3 */
+	{ 15, 0x04 }, { 20, 0x05 }, { 25, 0x06 }, { 30, 0x07 },	    /* 4 - 7 */
+};
 
 enum bmc150_magn_presets {
 	LOW_POWER_PRESET,
@@ -170,10 +166,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
@@ -641,8 +637,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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
@ 2026-02-11  8:39   ` Andy Shevchenko
  2026-02-13 23:31   ` kernel test robot
  2026-02-14 16:28   ` David Lechner
  2 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2026-02-11  8:39 UTC (permalink / raw)
  To: Neel Bullywon; +Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel

On Tue, Feb 10, 2026 at 06:39:43PM -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.
> 
> scoped_guard() is used in read_raw IIO_CHAN_INFO_RAW case for a
> multi-statement mutex-protected block, as well as in remove,
> runtime_suspend, and suspend where a short mutex-protected scope
> is needed for a single function call.
> 
> guard() is used in write_raw and trig_reen where the mutex scope
> extends to the end of the function or case block. Case blocks in
> write_raw are wrapped in braces to ensure clear scope for the
> cleanup guards.
> 
> The trigger_handler function is left unchanged as mixing guard()
> with goto error paths can be fragile.

...

>  #include <linux/module.h>
>  #include <linux/i2c.h>
> +#include <linux/cleanup.h>
>  #include <linux/interrupt.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>

With a given context I would place the new one just before the delay.h.


...

>  	case IIO_CHAN_INFO_RAW:
>  		if (iio_buffer_enabled(indio_dev))
>  			return -EBUSY;
> -		mutex_lock(&data->mutex);
> -
> -		ret = bmc150_magn_set_power_state(data, true);
> -		if (ret < 0) {
> -			mutex_unlock(&data->mutex);
> -			return ret;
> -		}
> +		scoped_guard(mutex, &data->mutex) {
> +			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;

Actually looking again at this, I think we may use guard()(), but move the {}
to the whole case:

	case IIO_CHAN_INFO_RAW: {
		if (iio_buffer_enabled(indio_dev))
			return -EBUSY;

		guard(mutex)(&data->mutex);
		...
	}

This will make the change much more cleaner.

...

>  		case IIO_MOD_Z:
> +			{
> +				guard(mutex)(&data->mutex);

Why do you change only one case?! If the comment is given, it applies
to all similar places. But see also above.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep
  2026-02-10 23:39 ` [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
@ 2026-02-11  8:40   ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2026-02-11  8:40 UTC (permalink / raw)
  To: Neel Bullywon; +Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel

On Tue, Feb 10, 2026 at 06:39:44PM -0500, Neel Bullywon wrote:
> Replace msleep(5) with fsleep(5 * USEC_PER_MSEC) to allow the kernel
> to select the most appropriate delay mechanism based on duration.
> Using USEC_PER_MSEC makes the unit conversion explicit.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
  2026-02-10 23:39 ` [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
@ 2026-02-11  8:41   ` Andy Shevchenko
  2026-02-14 16:31   ` David Lechner
  1 sibling, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2026-02-11  8:41 UTC (permalink / raw)
  To: Neel Bullywon; +Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel

On Tue, Feb 10, 2026 at 06:39:45PM -0500, Neel Bullywon wrote:
> Improve initializer list style for bmc150_magn_samp_freq_table by
> moving the opening brace to its own line and grouping four entries
> per line with index comments.
> 
> Add spaces inside braces for initializer lists in the preset table
> for consistency.
> 
> Fix indentation of bmc150_magn_scan_masks array. No functional changes.

...

> +} 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 */

Problem with indentation of the comment in the last line.

> +};

Otherwise, LGTM.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
  2026-02-11  8:39   ` Andy Shevchenko
@ 2026-02-13 23:31   ` kernel test robot
  2026-02-14 16:34     ` Jonathan Cameron
  2026-02-14 16:28   ` David Lechner
  2 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2026-02-13 23:31 UTC (permalink / raw)
  To: Neel Bullywon, jic23
  Cc: llvm, oe-kbuild-all, dlechner, nuno.sa, andy, linux-iio,
	linux-kernel, Neel Bullywon

Hi Neel,

kernel test robot noticed the following build errors:

[auto build test ERROR on e7aa57247700733e52a8e2e4dee6a52c2a76de02]

url:    https://github.com/intel-lab-lkp/linux/commits/Neel-Bullywon/iio-magnetometer-bmc150_magn-use-automated-cleanup-for-mutex/20260211-074324
base:   e7aa57247700733e52a8e2e4dee6a52c2a76de02
patch link:    https://lore.kernel.org/r/20260210233945.40975-2-neelb2403%40gmail.com
patch subject: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20260214/202602140708.3KGC8ZJJ-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260214/202602140708.3KGC8ZJJ-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/202602140708.3KGC8ZJJ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/magnetometer/bmc150_magn.c:561:2: error: cannot jump from switch statement to this case label
     561 |         default:
         |         ^
   drivers/iio/magnetometer/bmc150_magn.c:527:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     527 |                 guard(mutex)(&data->mutex);
         |                 ^
   include/linux/cleanup.h:414:15: note: expanded from macro 'guard'
     414 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:168:2: note: expanded from macro '__UNIQUE_ID'
     168 |         __PASTE(__UNIQUE_ID_,                                   \
         |         ^
   include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
      16 | #define __PASTE(a, b) ___PASTE(a, b)
         |                       ^
   include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
      15 | #define ___PASTE(a, b) a##b
         |                        ^
   <scratch space>:122:1: note: expanded from here
     122 | __UNIQUE_ID_guard_677
         | ^
   drivers/iio/magnetometer/bmc150_magn.c:529:2: error: cannot jump from switch statement to this case label
     529 |         case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
         |         ^
   drivers/iio/magnetometer/bmc150_magn.c:527:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     527 |                 guard(mutex)(&data->mutex);
         |                 ^
   include/linux/cleanup.h:414:15: note: expanded from macro 'guard'
     414 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:168:2: note: expanded from macro '__UNIQUE_ID'
     168 |         __PASTE(__UNIQUE_ID_,                                   \
         |         ^
   include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
      16 | #define __PASTE(a, b) ___PASTE(a, b)
         |                       ^
   include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
      15 | #define ___PASTE(a, b) a##b
         |                        ^
   <scratch space>:122:1: note: expanded from here
     122 | __UNIQUE_ID_guard_677
         | ^
   2 errors generated.


vim +561 drivers/iio/magnetometer/bmc150_magn.c

c91746a2361d75 Irina Tirdea   2015-04-29  515  
c91746a2361d75 Irina Tirdea   2015-04-29  516  static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
c91746a2361d75 Irina Tirdea   2015-04-29  517  				 struct iio_chan_spec const *chan,
c91746a2361d75 Irina Tirdea   2015-04-29  518  				 int val, int val2, long mask)
c91746a2361d75 Irina Tirdea   2015-04-29  519  {
c91746a2361d75 Irina Tirdea   2015-04-29  520  	struct bmc150_magn_data *data = iio_priv(indio_dev);
c91746a2361d75 Irina Tirdea   2015-04-29  521  	int ret;
c91746a2361d75 Irina Tirdea   2015-04-29  522  
c91746a2361d75 Irina Tirdea   2015-04-29  523  	switch (mask) {
c91746a2361d75 Irina Tirdea   2015-04-29  524  	case IIO_CHAN_INFO_SAMP_FREQ:
5990dc9703679a Irina Tirdea   2015-04-29  525  		if (val > data->max_odr)
5990dc9703679a Irina Tirdea   2015-04-29  526  			return -EINVAL;
d65a37daae6aa8 Neel Bullywon  2026-02-10  527  		guard(mutex)(&data->mutex);
d65a37daae6aa8 Neel Bullywon  2026-02-10  528  		return bmc150_magn_set_odr(data, val);
5990dc9703679a Irina Tirdea   2015-04-29  529  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
5990dc9703679a Irina Tirdea   2015-04-29  530  		switch (chan->channel2) {
5990dc9703679a Irina Tirdea   2015-04-29  531  		case IIO_MOD_X:
5990dc9703679a Irina Tirdea   2015-04-29  532  		case IIO_MOD_Y:
5990dc9703679a Irina Tirdea   2015-04-29  533  			if (val < 1 || val > 511)
5990dc9703679a Irina Tirdea   2015-04-29  534  				return -EINVAL;
d65a37daae6aa8 Neel Bullywon  2026-02-10  535  			{
d65a37daae6aa8 Neel Bullywon  2026-02-10  536  				guard(mutex)(&data->mutex);
5990dc9703679a Irina Tirdea   2015-04-29  537  				ret = bmc150_magn_set_max_odr(data, val, 0, 0);
d65a37daae6aa8 Neel Bullywon  2026-02-10  538  				if (ret < 0)
5990dc9703679a Irina Tirdea   2015-04-29  539  					return ret;
d65a37daae6aa8 Neel Bullywon  2026-02-10  540  				return regmap_update_bits(data->regmap,
5990dc9703679a Irina Tirdea   2015-04-29  541  							 BMC150_MAGN_REG_REP_XY,
1506f3cd0b8169 Hartmut Knaack 2015-07-17  542  							 BMC150_MAGN_REG_REP_DATAMASK,
d65a37daae6aa8 Neel Bullywon  2026-02-10  543  							 BMC150_MAGN_REPXY_TO_REGVAL(val));
d65a37daae6aa8 Neel Bullywon  2026-02-10  544  			}
5990dc9703679a Irina Tirdea   2015-04-29  545  		case IIO_MOD_Z:
5990dc9703679a Irina Tirdea   2015-04-29  546  			if (val < 1 || val > 256)
5990dc9703679a Irina Tirdea   2015-04-29  547  				return -EINVAL;
d65a37daae6aa8 Neel Bullywon  2026-02-10  548  			{
d65a37daae6aa8 Neel Bullywon  2026-02-10  549  				guard(mutex)(&data->mutex);
5990dc9703679a Irina Tirdea   2015-04-29  550  				ret = bmc150_magn_set_max_odr(data, 0, val, 0);
d65a37daae6aa8 Neel Bullywon  2026-02-10  551  				if (ret < 0)
5990dc9703679a Irina Tirdea   2015-04-29  552  					return ret;
d65a37daae6aa8 Neel Bullywon  2026-02-10  553  				return regmap_update_bits(data->regmap,
5990dc9703679a Irina Tirdea   2015-04-29  554  							 BMC150_MAGN_REG_REP_Z,
1506f3cd0b8169 Hartmut Knaack 2015-07-17  555  							 BMC150_MAGN_REG_REP_DATAMASK,
d65a37daae6aa8 Neel Bullywon  2026-02-10  556  							 BMC150_MAGN_REPZ_TO_REGVAL(val));
d65a37daae6aa8 Neel Bullywon  2026-02-10  557  			}
5990dc9703679a Irina Tirdea   2015-04-29  558  		default:
5990dc9703679a Irina Tirdea   2015-04-29  559  			return -EINVAL;
5990dc9703679a Irina Tirdea   2015-04-29  560  		}
c91746a2361d75 Irina Tirdea   2015-04-29 @561  	default:
c91746a2361d75 Irina Tirdea   2015-04-29  562  		return -EINVAL;
c91746a2361d75 Irina Tirdea   2015-04-29  563  	}
c91746a2361d75 Irina Tirdea   2015-04-29  564  }
c91746a2361d75 Irina Tirdea   2015-04-29  565  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
  2026-02-11  8:39   ` Andy Shevchenko
  2026-02-13 23:31   ` kernel test robot
@ 2026-02-14 16:28   ` David Lechner
  2 siblings, 0 replies; 12+ messages in thread
From: David Lechner @ 2026-02-14 16:28 UTC (permalink / raw)
  To: Neel Bullywon, jic23; +Cc: nuno.sa, andy, linux-iio, linux-kernel

On 2/10/26 5:39 PM, Neel Bullywon wrote:
> Use guard() and scoped_guard() to replace manual mutex lock/unlock
> calls. This simplifies error handling and ensures RAII-style cleanup.
> 
> scoped_guard() is used in read_raw IIO_CHAN_INFO_RAW case for a
> multi-statement mutex-protected block, as well as in remove,
> runtime_suspend, and suspend where a short mutex-protected scope
> is needed for a single function call.
> 
> guard() is used in write_raw and trig_reen where the mutex scope
> extends to the end of the function or case block. Case blocks in
> write_raw are wrapped in braces to ensure clear scope for the
> cleanup guards.
> 
> 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 | 130 ++++++++++---------------
>  1 file changed, 52 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
> index 6a73f6e2f1f0..5eb710eb020d 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,22 @@ 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);
> -
> -		ret = bmc150_magn_set_power_state(data, true);
> -		if (ret < 0) {
> -			mutex_unlock(&data->mutex);
> -			return ret;
> -		}
> +		scoped_guard(mutex, &data->mutex) {

I am very wary of scoped_guard(), especially in case statements.
There is a hidden for loop in the macro, so if `break;` is used
inside of the scoped_guard() {}, it will only break out of the
guard scope (for loop) and not the case.

It would be better if we could use `guard()` instead (in the style
that Andy suggested).


>  static const struct iio_trigger_ops bmc150_magn_trigger_ops = {
> @@ -980,9 +961,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 +975,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);


This way of using scoped_guard() is OK since there is no { }.


>  	if (ret < 0) {
>  		dev_err(dev, "powering off device failed\n");
>  		return ret;
> @@ -1026,10 +1005,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);

This one can be regular guard() and return directly.

>  
>  	return ret;
>  }

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
  2026-02-10 23:39 ` [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
  2026-02-11  8:41   ` Andy Shevchenko
@ 2026-02-14 16:31   ` David Lechner
  2026-02-14 18:00     ` Andy Shevchenko
  1 sibling, 1 reply; 12+ messages in thread
From: David Lechner @ 2026-02-14 16:31 UTC (permalink / raw)
  To: Neel Bullywon, jic23; +Cc: nuno.sa, andy, linux-iio, linux-kernel

On 2/10/26 5:39 PM, Neel Bullywon wrote:
> Improve initializer list style for bmc150_magn_samp_freq_table by
> moving the opening brace to its own line and grouping four entries
> per line with index comments.
> 
> Add spaces inside braces for initializer lists in the preset table
> for consistency.
> 
> Fix indentation of bmc150_magn_scan_masks array. No functional changes.
> 
> Signed-off-by: Neel Bullywon <neelb2403@gmail.com>
> ---
>  drivers/iio/magnetometer/bmc150_magn.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
> index f254774d28e5..942c2f5e97e1 100644
> --- a/drivers/iio/magnetometer/bmc150_magn.c
> +++ b/drivers/iio/magnetometer/bmc150_magn.c
> @@ -149,14 +149,10 @@ 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 },		/* 0 - 3 */
> +	{ 15, 0x04 }, { 20, 0x05 }, { 25, 0x06 }, { 30, 0x07 },	    /* 4 - 7 */
> +};
>  
Just an opinion: I think it is easier to see the patterns in
the numbers if we leave it as one struct per line. (Fixing
the indent and padding is still an improvement.)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
  2026-02-13 23:31   ` kernel test robot
@ 2026-02-14 16:34     ` Jonathan Cameron
  0 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2026-02-14 16:34 UTC (permalink / raw)
  To: kernel test robot
  Cc: Neel Bullywon, llvm, oe-kbuild-all, dlechner, nuno.sa, andy,
	linux-iio, linux-kernel

On Sat, 14 Feb 2026 07:31:03 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Neel,
> 
> kernel test robot noticed the following build errors:

Just replying to this to highlight what I the whole scope thing is about.

> 
> [auto build test ERROR on e7aa57247700733e52a8e2e4dee6a52c2a76de02]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Neel-Bullywon/iio-magnetometer-bmc150_magn-use-automated-cleanup-for-mutex/20260211-074324
> base:   e7aa57247700733e52a8e2e4dee6a52c2a76de02
> patch link:    https://lore.kernel.org/r/20260210233945.40975-2-neelb2403%40gmail.com
> patch subject: [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
> config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20260214/202602140708.3KGC8ZJJ-lkp@intel.com/config)
> compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260214/202602140708.3KGC8ZJJ-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/202602140708.3KGC8ZJJ-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/iio/magnetometer/bmc150_magn.c:561:2: error: cannot jump from switch statement to this case label  
>      561 |         default:
>          |         ^
>    drivers/iio/magnetometer/bmc150_magn.c:527:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
>      527 |                 guard(mutex)(&data->mutex);
>          |                 ^
>    include/linux/cleanup.h:414:15: note: expanded from macro 'guard'
>      414 |         CLASS(_name, __UNIQUE_ID(guard))
>          |                      ^
>    include/linux/compiler.h:168:2: note: expanded from macro '__UNIQUE_ID'
>      168 |         __PASTE(__UNIQUE_ID_,                                   \
>          |         ^
>    include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
>       16 | #define __PASTE(a, b) ___PASTE(a, b)
>          |                       ^
>    include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
>       15 | #define ___PASTE(a, b) a##b
>          |                        ^
>    <scratch space>:122:1: note: expanded from here
>      122 | __UNIQUE_ID_guard_677
>          | ^
>    drivers/iio/magnetometer/bmc150_magn.c:529:2: error: cannot jump from switch statement to this case label
>      529 |         case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>          |         ^
>    drivers/iio/magnetometer/bmc150_magn.c:527:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
>      527 |                 guard(mutex)(&data->mutex);
>          |                 ^
>    include/linux/cleanup.h:414:15: note: expanded from macro 'guard'
>      414 |         CLASS(_name, __UNIQUE_ID(guard))
>          |                      ^
>    include/linux/compiler.h:168:2: note: expanded from macro '__UNIQUE_ID'
>      168 |         __PASTE(__UNIQUE_ID_,                                   \
>          |         ^
>    include/linux/compiler_types.h:16:23: note: expanded from macro '__PASTE'
>       16 | #define __PASTE(a, b) ___PASTE(a, b)
>          |                       ^
>    include/linux/compiler_types.h:15:24: note: expanded from macro '___PASTE'
>       15 | #define ___PASTE(a, b) a##b
>          |                        ^
>    <scratch space>:122:1: note: expanded from here
>      122 | __UNIQUE_ID_guard_677
>          | ^
>    2 errors generated.
> 
> 
> vim +561 drivers/iio/magnetometer/bmc150_magn.c
> 
> c91746a2361d75 Irina Tirdea   2015-04-29  515  
> c91746a2361d75 Irina Tirdea   2015-04-29  516  static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
> c91746a2361d75 Irina Tirdea   2015-04-29  517  				 struct iio_chan_spec const *chan,
> c91746a2361d75 Irina Tirdea   2015-04-29  518  				 int val, int val2, long mask)
> c91746a2361d75 Irina Tirdea   2015-04-29  519  {
> c91746a2361d75 Irina Tirdea   2015-04-29  520  	struct bmc150_magn_data *data = iio_priv(indio_dev);
> c91746a2361d75 Irina Tirdea   2015-04-29  521  	int ret;
> c91746a2361d75 Irina Tirdea   2015-04-29  522  
> c91746a2361d75 Irina Tirdea   2015-04-29  523  	switch (mask) {
> c91746a2361d75 Irina Tirdea   2015-04-29  524  	case IIO_CHAN_INFO_SAMP_FREQ:
> 5990dc9703679a Irina Tirdea   2015-04-29  525  		if (val > data->max_odr)
> 5990dc9703679a Irina Tirdea   2015-04-29  526  			return -EINVAL;
> d65a37daae6aa8 Neel Bullywon  2026-02-10  527  		guard(mutex)(&data->mutex);

It's hard to spot from the error message, but here is the bug.  This is in the same
scope as default label below that is hightlighted.
Thus wonderfully the __free() that hides under the guard() magic is
initialized but because we don't go down this path to reach that
default, we can the __free() (ultimately mutex_unlock())
on an unassigned pointer.

Hence scope for these really matters!

Jonathan


> d65a37daae6aa8 Neel Bullywon  2026-02-10  528  		return bmc150_magn_set_odr(data, val);
> 5990dc9703679a Irina Tirdea   2015-04-29  529  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> 5990dc9703679a Irina Tirdea   2015-04-29  530  		switch (chan->channel2) {
> 5990dc9703679a Irina Tirdea   2015-04-29  531  		case IIO_MOD_X:
> 5990dc9703679a Irina Tirdea   2015-04-29  532  		case IIO_MOD_Y:
> 5990dc9703679a Irina Tirdea   2015-04-29  533  			if (val < 1 || val > 511)
> 5990dc9703679a Irina Tirdea   2015-04-29  534  				return -EINVAL;
> d65a37daae6aa8 Neel Bullywon  2026-02-10  535  			{
> d65a37daae6aa8 Neel Bullywon  2026-02-10  536  				guard(mutex)(&data->mutex);
> 5990dc9703679a Irina Tirdea   2015-04-29  537  				ret = bmc150_magn_set_max_odr(data, val, 0, 0);
> d65a37daae6aa8 Neel Bullywon  2026-02-10  538  				if (ret < 0)
> 5990dc9703679a Irina Tirdea   2015-04-29  539  					return ret;
> d65a37daae6aa8 Neel Bullywon  2026-02-10  540  				return regmap_update_bits(data->regmap,
> 5990dc9703679a Irina Tirdea   2015-04-29  541  							 BMC150_MAGN_REG_REP_XY,
> 1506f3cd0b8169 Hartmut Knaack 2015-07-17  542  							 BMC150_MAGN_REG_REP_DATAMASK,
> d65a37daae6aa8 Neel Bullywon  2026-02-10  543  							 BMC150_MAGN_REPXY_TO_REGVAL(val));
> d65a37daae6aa8 Neel Bullywon  2026-02-10  544  			}
> 5990dc9703679a Irina Tirdea   2015-04-29  545  		case IIO_MOD_Z:
> 5990dc9703679a Irina Tirdea   2015-04-29  546  			if (val < 1 || val > 256)
> 5990dc9703679a Irina Tirdea   2015-04-29  547  				return -EINVAL;
> d65a37daae6aa8 Neel Bullywon  2026-02-10  548  			{
> d65a37daae6aa8 Neel Bullywon  2026-02-10  549  				guard(mutex)(&data->mutex);
> 5990dc9703679a Irina Tirdea   2015-04-29  550  				ret = bmc150_magn_set_max_odr(data, 0, val, 0);
> d65a37daae6aa8 Neel Bullywon  2026-02-10  551  				if (ret < 0)
> 5990dc9703679a Irina Tirdea   2015-04-29  552  					return ret;
> d65a37daae6aa8 Neel Bullywon  2026-02-10  553  				return regmap_update_bits(data->regmap,
> 5990dc9703679a Irina Tirdea   2015-04-29  554  							 BMC150_MAGN_REG_REP_Z,
> 1506f3cd0b8169 Hartmut Knaack 2015-07-17  555  							 BMC150_MAGN_REG_REP_DATAMASK,
> d65a37daae6aa8 Neel Bullywon  2026-02-10  556  							 BMC150_MAGN_REPZ_TO_REGVAL(val));
> d65a37daae6aa8 Neel Bullywon  2026-02-10  557  			}
> 5990dc9703679a Irina Tirdea   2015-04-29  558  		default:
> 5990dc9703679a Irina Tirdea   2015-04-29  559  			return -EINVAL;
> 5990dc9703679a Irina Tirdea   2015-04-29  560  		}
> c91746a2361d75 Irina Tirdea   2015-04-29 @561  	default:
> c91746a2361d75 Irina Tirdea   2015-04-29  562  		return -EINVAL;
> c91746a2361d75 Irina Tirdea   2015-04-29  563  	}
> c91746a2361d75 Irina Tirdea   2015-04-29  564  }
> c91746a2361d75 Irina Tirdea   2015-04-29  565  
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
  2026-02-14 16:31   ` David Lechner
@ 2026-02-14 18:00     ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2026-02-14 18:00 UTC (permalink / raw)
  To: David Lechner
  Cc: Neel Bullywon, jic23, nuno.sa, andy, linux-iio, linux-kernel

On Sat, Feb 14, 2026 at 10:31:44AM -0600, David Lechner wrote:
> On 2/10/26 5:39 PM, Neel Bullywon wrote:

...

> > +} 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 */
> > +};
> >  
> Just an opinion: I think it is easier to see the patterns in
> the numbers if we leave it as one struct per line. (Fixing
> the indent and padding is still an improvement.)

I proposed both, so I'm okay with that too.


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-02-14 18:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-10 23:39 [PATCH v6 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
2026-02-10 23:39 ` [PATCH v6 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
2026-02-11  8:39   ` Andy Shevchenko
2026-02-13 23:31   ` kernel test robot
2026-02-14 16:34     ` Jonathan Cameron
2026-02-14 16:28   ` David Lechner
2026-02-10 23:39 ` [PATCH v6 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
2026-02-11  8:40   ` Andy Shevchenko
2026-02-10 23:39 ` [PATCH v6 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
2026-02-11  8:41   ` Andy Shevchenko
2026-02-14 16:31   ` David Lechner
2026-02-14 18:00     ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox