* [PATCH v7 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting
@ 2026-02-16 1:54 Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Neel Bullywon @ 2026-02-16 1:54 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 v7:
- Place cleanup.h include just before delay.h (Andy).
- Use guard() with case-level braces instead of scoped_guard() for the
IIO_CHAN_INFO_RAW case block in read_raw (Andy).
- Apply case-level braces consistently to all case blocks using guard()
in write_raw (Andy).
- Use guard() instead of scoped_guard() for suspend, returning
directly (David).
- Keep samp_freq_table as one entry per line with fixed indent and
spaces inside braces (David, Andy).
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.
base-commit: e7aa57247700733e52a8e2e4dee6a52c2a76de02
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 | 142 ++++++++++---------------
1 file changed, 59 insertions(+), 83 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
2026-02-16 1:54 [PATCH v7 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
@ 2026-02-16 1:54 ` Neel Bullywon
2026-02-16 8:33 ` Andy Shevchenko
2026-02-16 1:54 ` [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
2 siblings, 1 reply; 10+ messages in thread
From: Neel Bullywon @ 2026-02-16 1:54 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.
guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
suspend, and resume. Case blocks using guard() in read_raw and
write_raw are wrapped in braces at the case label level to ensure
clear scope for the cleanup guards.
scoped_guard() is used in remove and runtime_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 | 109 ++++++++++---------------
1 file changed, 41 insertions(+), 68 deletions(-)
diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 6a73f6e2f1f0..ff6196c86e6b 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -12,6 +12,7 @@
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/pm.h>
@@ -453,33 +454,29 @@ static int bmc150_magn_read_raw(struct iio_dev *indio_dev,
s32 values[AXIS_XYZ_MAX];
switch (mask) {
- case IIO_CHAN_INFO_RAW:
+ 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);
+ 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_set_power_state(data, false);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
+ if (ret < 0)
return ret;
- }
- mutex_unlock(&data->mutex);
return IIO_VAL_INT;
+ }
case IIO_CHAN_INFO_SCALE:
/*
* The API/driver performs an off-chip temperature
@@ -527,48 +524,39 @@ static int bmc150_magn_write_raw(struct iio_dev *indio_dev,
int ret;
switch (mask) {
- case IIO_CHAN_INFO_SAMP_FREQ:
+ 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:
+ case IIO_MOD_Y: {
if (val < 1 || val > 511)
return -EINVAL;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
ret = bmc150_magn_set_max_odr(data, val, 0, 0);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
+ if (ret < 0)
return ret;
- }
- ret = regmap_update_bits(data->regmap,
+ return 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:
+ BMC150_MAGN_REPXY_TO_REGVAL(val));
+ }
+ case IIO_MOD_Z: {
if (val < 1 || val > 256)
return -EINVAL;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
ret = bmc150_magn_set_max_odr(data, 0, val, 0);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
+ if (ret < 0)
return ret;
- }
- ret = regmap_update_bits(data->regmap,
+ return 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;
+ BMC150_MAGN_REPZ_TO_REGVAL(val));
+ }
default:
return -EINVAL;
}
@@ -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;
@@ -1024,28 +1005,20 @@ static int bmc150_magn_suspend(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_SLEEP,
- true);
- mutex_unlock(&data->mutex);
- return ret;
+ guard(mutex)(&data->mutex);
+ return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
+ true);
}
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] 10+ messages in thread
* [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep
2026-02-16 1:54 [PATCH v7 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
@ 2026-02-16 1:54 ` Neel Bullywon
2026-02-20 10:37 ` Jonathan Cameron
2026-02-16 1:54 ` [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
2 siblings, 1 reply; 10+ messages in thread
From: Neel Bullywon @ 2026-02-16 1:54 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Neel Bullywon,
Andy Shevchenko
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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.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 ff6196c86e6b..e7f8d118242b 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -683,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(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] 10+ messages in thread
* [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
2026-02-16 1:54 [PATCH v7 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
@ 2026-02-16 1:54 ` Neel Bullywon
2026-02-16 8:35 ` Andy Shevchenko
2 siblings, 1 reply; 10+ messages in thread
From: Neel Bullywon @ 2026-02-16 1:54 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 using one entry per line
with proper indentation and spaces inside braces.
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 | 31 ++++++++++++++------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index e7f8d118242b..6e154b55ca18 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -149,14 +149,16 @@ 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 +172,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 +645,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] 10+ messages in thread
* Re: [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
2026-02-16 1:54 ` [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
@ 2026-02-16 8:33 ` Andy Shevchenko
2026-02-20 10:34 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-16 8:33 UTC (permalink / raw)
To: Neel Bullywon; +Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Sun, Feb 15, 2026 at 08:54:52PM -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.
>
> guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
> suspend, and resume. Case blocks using guard() in read_raw and
> write_raw are wrapped in braces at the case label level to ensure
> clear scope for the cleanup guards.
>
> scoped_guard() is used in remove and runtime_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.
...
> - 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);
Wonder if we can move this to a helper:
static int bmc150_magn_set_power_mode_locked(data, mode) // locked or unlocked, dunno with proper naming
{
guard(mutex)(&data->mutex)
return bmc150_magn_set_power_mode(data, mode, true);
}
...
> - 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;
> }
Ditto.
...
> {
> 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_SLEEP,
> - true);
> - mutex_unlock(&data->mutex);
>
> - return ret;
> + guard(mutex)(&data->mutex);
> + return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> + true);
Ditto.
> }
>
> 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);
Ditto.
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
2026-02-16 1:54 ` [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
@ 2026-02-16 8:35 ` Andy Shevchenko
2026-02-20 10:37 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-16 8:35 UTC (permalink / raw)
To: Neel Bullywon; +Cc: jic23, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Sun, Feb 15, 2026 at 08:54:54PM -0500, Neel Bullywon wrote:
> Improve initializer list style for bmc150_magn_samp_freq_table by
> moving the opening brace to its own line and using one entry per line
> with proper indentation and spaces inside braces.
>
> Add spaces inside braces for initializer lists in the preset table
> for consistency.
>
> Fix indentation of bmc150_magn_scan_masks array. No functional changes.
LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
2026-02-16 8:33 ` Andy Shevchenko
@ 2026-02-20 10:34 ` Jonathan Cameron
2026-02-20 19:29 ` Neel Bullywon
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-20 10:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Neel Bullywon, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Mon, 16 Feb 2026 10:33:25 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Sun, Feb 15, 2026 at 08:54:52PM -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.
> >
> > guard() is used in read_raw, write_raw, trig_reen, trigger_set_state,
> > suspend, and resume. Case blocks using guard() in read_raw and
> > write_raw are wrapped in braces at the case label level to ensure
> > clear scope for the cleanup guards.
> >
> > scoped_guard() is used in remove and runtime_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.
>
> ...
>
> > - 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);
>
> Wonder if we can move this to a helper:
>
> static int bmc150_magn_set_power_mode_locked(data, mode) // locked or unlocked, dunno with proper naming
> {
> guard(mutex)(&data->mutex)
> return bmc150_magn_set_power_mode(data, mode, true);
> }
>
> ...
Whilst it involves taking the mutex in probe() before it's strictly
necessary (as everything is serialized anyway until we register the
userspace interfaces, I wonder if a simpler option is to rely on the
mutex_init() being early enough and just put the guard unconditionally
in bmc150_magn_set_power_mode()?
The snag is in runtime PM. The locking in there is really odd
(smells broken to me)
It requires the lock to already be held for resume, but must not
be held for runtime_suspend. Superficially I suspect this only works
because the autosuspend delay gets us past the unlock in suspend path.
I think we can unwind that mess though by changing read_raw() to
not hold the lock across both power and reading phases (I don't think
it matters if we drop it and grab it again in quick succession as
read_raw is never really a fast path).
The usage of runtime pm in buffer_preenable / postdisable should be
fine but at this point I'm getting nervous. Neel, do you have the
hardware for this one to check we don't break anything trying to clean
this up? Or does anyone else who is willing to test any changes?
Jonathan
>
> > - 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;
> > }
>
> Ditto.
>
>
> ...
>
> > {
> > 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_SLEEP,
> > - true);
> > - mutex_unlock(&data->mutex);
> >
> > - return ret;
> > + guard(mutex)(&data->mutex);
> > + return bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SLEEP,
> > + true);
>
> Ditto.
>
> > }
> >
> > 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);
>
> Ditto.
>
> > }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep
2026-02-16 1:54 ` [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
@ 2026-02-20 10:37 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-20 10:37 UTC (permalink / raw)
To: Neel Bullywon
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Andy Shevchenko
On Sun, 15 Feb 2026 20:54:53 -0500
Neel Bullywon <neelb2403@gmail.com> 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.
>
> Signed-off-by: Neel Bullywon <neelb2403@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
This is independent of patch 1, so I'll pick it up now.
Please base next version of patch 1 on top of the testing branch
of iio.git (or just reorder the patches locally and only post
that one for v8!)
Applied to the testing branch of iio.git. Note I'll rebase
that on rc1 sometime next week and then it'll go out for linux-next
to pick up.
Thanks,
Jonathan
> ---
> 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 ff6196c86e6b..e7f8d118242b 100644
> --- a/drivers/iio/magnetometer/bmc150_magn.c
> +++ b/drivers/iio/magnetometer/bmc150_magn.c
> @@ -683,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(5 * USEC_PER_MSEC);
>
> ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND,
> false);
#
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup
2026-02-16 8:35 ` Andy Shevchenko
@ 2026-02-20 10:37 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-20 10:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Neel Bullywon, dlechner, nuno.sa, andy, linux-iio, linux-kernel
On Mon, 16 Feb 2026 10:35:07 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Sun, Feb 15, 2026 at 08:54:54PM -0500, Neel Bullywon wrote:
> > Improve initializer list style for bmc150_magn_samp_freq_table by
> > moving the opening brace to its own line and using one entry per line
> > with proper indentation and spaces inside braces.
> >
> > Add spaces inside braces for initializer lists in the preset table
> > for consistency.
> >
> > Fix indentation of bmc150_magn_scan_masks array. No functional changes.
>
> LGTM, FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
This is also independent of patch 1 so I've picked it up now.
Applied to the testing branch of iio.git
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex
2026-02-20 10:34 ` Jonathan Cameron
@ 2026-02-20 19:29 ` Neel Bullywon
0 siblings, 0 replies; 10+ messages in thread
From: Neel Bullywon @ 2026-02-20 19:29 UTC (permalink / raw)
To: jic23; +Cc: andriy.shevchenko, dlechner, nuno.sa, andy, linux-iio,
linux-kernel
I don't have the BMC150 hardware unfortunately, so I can't test runtime PM behavior changes directly.
For the v8 of patch 1, I see two approaches based on the feedback given:
Andy's suggestion: add a bmc150_magn_set_power_mode_locked() helper to deduplicate the lock+call pattern in remove, runtime_suspend, suspend, and resume.
Your suggestion: move the guard() unconditionally into bmc150_magn_set_power_mode() itself, which is cleaner but touches the runtime PM locking that you flagged as suspicious.
Given I can't test runtime PM, would you prefer I go with Andy's suggestion for now, and leave the deeper runtime PM cleanup for a separate series if someone with hardware can verify it?
Or if you'd rather I take a shot at yours with the read_raw() rework you described, I'm happy to do that — just would need someone to test it.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-02-20 19:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-16 1:54 [PATCH v7 0/3] iio: magnetometer: bmc150_magn: cleanup and formatting Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 1/3] iio: magnetometer: bmc150_magn: use automated cleanup for mutex Neel Bullywon
2026-02-16 8:33 ` Andy Shevchenko
2026-02-20 10:34 ` Jonathan Cameron
2026-02-20 19:29 ` Neel Bullywon
2026-02-16 1:54 ` [PATCH v7 2/3] iio: magnetometer: bmc150_magn: replace msleep with fsleep Neel Bullywon
2026-02-20 10:37 ` Jonathan Cameron
2026-02-16 1:54 ` [PATCH v7 3/3] iio: magnetometer: bmc150_magn: minor formatting cleanup Neel Bullywon
2026-02-16 8:35 ` Andy Shevchenko
2026-02-20 10:37 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox