[PATCH v4 0/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[PATCH v4 0/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Hungyu Lin]

Replace open-coded bitfield operations with FIELD_PREP() and
FIELD_GET() helpers where appropriate.

Also simplify bmc150_magn_set_odr() by returning directly from
the matching table entry.

v4:
 - Split include sorting into a separate prerequisite patch
 - Remove the extra blank line in bmc150_magn_get_odr()

v3:
 - Add missing include <linux/bitfield.h>

v2:
 - Use FIELD_PREP() and FIELD_GET() helpers as suggested by
   Jonathan Cameron
 - Simplify bmc150_magn_set_odr() by returning directly from
   the matching table entry

Hungyu Lin (2):
  iio: magnetometer: bmc150: sort includes
  iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

 drivers/iio/magnetometer/bmc150_magn.c | 50 +++++++++++++-------------
 1 file changed, 24 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH 1/2] iio: magnetometer: bmc150: sort includes

[Hungyu Lin]

Sort the include list alphabetically.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index bf2551988008..6f2a4a863fbe 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -9,23 +9,23 @@
  * (C) Copyright 2011~2014 Bosch Sensortec GmbH All Rights Reserved
  */
 
-#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>
-#include <linux/pm_runtime.h>
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
+#include <linux/i2c.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 
 #include "bmc150_magn.h"
 
-- 
2.34.1

[PATCH 2/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Hungyu Lin]

Replace open-coded bitfield operations with FIELD_PREP() and
FIELD_GET() helpers where appropriate.

Also simplify bmc150_magn_set_odr() by returning directly from
the matching table entry.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/iio/magnetometer/bmc150_magn.c | 34 ++++++++++++--------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index 6f2a4a863fbe..c402ac852efb 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -9,6 +9,7 @@
  * (C) Copyright 2011~2014 Bosch Sensortec GmbH All Rights Reserved
  */
 
+#include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
@@ -245,14 +246,14 @@ static int bmc150_magn_set_power_mode(struct bmc150_magn_data *data,
 		return regmap_update_bits(data->regmap,
 					  BMC150_MAGN_REG_OPMODE_ODR,
 					  BMC150_MAGN_MASK_OPMODE,
-					  BMC150_MAGN_MODE_SLEEP <<
-					  BMC150_MAGN_SHIFT_OPMODE);
+					  FIELD_PREP(BMC150_MAGN_MASK_OPMODE,
+						  BMC150_MAGN_MODE_SLEEP));
 	case BMC150_MAGN_POWER_MODE_NORMAL:
 		return regmap_update_bits(data->regmap,
 					  BMC150_MAGN_REG_OPMODE_ODR,
 					  BMC150_MAGN_MASK_OPMODE,
-					  BMC150_MAGN_MODE_NORMAL <<
-					  BMC150_MAGN_SHIFT_OPMODE);
+					  FIELD_PREP(BMC150_MAGN_MASK_OPMODE,
+						  BMC150_MAGN_MODE_NORMAL));
 	}
 
 	return -EINVAL;
@@ -290,7 +291,8 @@ static int bmc150_magn_get_odr(struct bmc150_magn_data *data, int *val)
 	ret = regmap_read(data->regmap, BMC150_MAGN_REG_OPMODE_ODR, &reg_val);
 	if (ret < 0)
 		return ret;
-	odr_val = (reg_val & BMC150_MAGN_MASK_ODR) >> BMC150_MAGN_SHIFT_ODR;
+
+	odr_val = FIELD_GET(BMC150_MAGN_MASK_ODR, reg_val);
 
 	for (i = 0; i < ARRAY_SIZE(bmc150_magn_samp_freq_table); i++)
 		if (bmc150_magn_samp_freq_table[i].reg_val == odr_val) {
@@ -303,21 +305,17 @@ static int bmc150_magn_get_odr(struct bmc150_magn_data *data, int *val)
 
 static int bmc150_magn_set_odr(struct bmc150_magn_data *data, int val)
 {
-	int ret;
 	u8 i;
 
 	for (i = 0; i < ARRAY_SIZE(bmc150_magn_samp_freq_table); i++) {
-		if (bmc150_magn_samp_freq_table[i].freq == val) {
-			ret = regmap_update_bits(data->regmap,
-						 BMC150_MAGN_REG_OPMODE_ODR,
-						 BMC150_MAGN_MASK_ODR,
-						 bmc150_magn_samp_freq_table[i].
-						 reg_val <<
-						 BMC150_MAGN_SHIFT_ODR);
-			if (ret < 0)
-				return ret;
-			return 0;
-		}
+		if (bmc150_magn_samp_freq_table[i].freq != val)
+			continue;
+
+		return regmap_update_bits(data->regmap,
+					 BMC150_MAGN_REG_OPMODE_ODR,
+					 BMC150_MAGN_MASK_ODR,
+					 FIELD_PREP(BMC150_MAGN_MASK_ODR,
+						 bmc150_magn_samp_freq_table[i].reg_val));
 	}
 
 	return -EINVAL;
@@ -800,7 +798,7 @@ static int bmc150_magn_data_rdy_trigger_set_state(struct iio_trigger *trig,
 
 	ret = regmap_update_bits(data->regmap, BMC150_MAGN_REG_INT_DRDY,
 				 BMC150_MAGN_MASK_DRDY_EN,
-				 state << BMC150_MAGN_SHIFT_DRDY_EN);
+				 FIELD_PREP(BMC150_MAGN_MASK_DRDY_EN, state));
 	if (ret < 0)
 		return ret;
 
-- 
2.34.1

Re: [PATCH 2/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Hungyu Lin]

> -       odr_val = (reg_val & BMC150_MAGN_MASK_ODR) >> BMC150_MAGN_SHIFT_ODR;
> +
> +       odr_val = FIELD_GET(BMC150_MAGN_MASK_ODR, reg_val);

Thanks Andy.

The blank line issue was inadvertently retained in v4 and
has been fixed in v5.

Thanks,
Hungyu

Re: [PATCH 1/2] iio: magnetometer: bmc150: sort includes

[Andy Shevchenko]

On Wed, Jun 10, 2026 at 04:24:17AM +0000, Hungyu Lin wrote:
> Sort the include list alphabetically.

...

> -#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>
> -#include <linux/pm_runtime.h>
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> +#include <linux/i2c.h>

>  #include <linux/iio/buffer.h>
>  #include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
>  #include <linux/iio/trigger.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>

Oh, I didn't noticed this one is mixed with the other linux/*.h.

> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>

Please, make it in two groups

linux/*.h
...blank line...
linux/iio/*.h
...blank line...
"...internal header..."

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Andy Shevchenko]

On Wed, Jun 10, 2026 at 04:24:18AM +0000, Hungyu Lin wrote:
> Replace open-coded bitfield operations with FIELD_PREP() and
> FIELD_GET() helpers where appropriate.
> 
> Also simplify bmc150_magn_set_odr() by returning directly from
> the matching table entry.

...

> -	odr_val = (reg_val & BMC150_MAGN_MASK_ODR) >> BMC150_MAGN_SHIFT_ODR;
> +
> +	odr_val = FIELD_GET(BMC150_MAGN_MASK_ODR, reg_val);

Same comment as per previous few versions. I don't understand why you haven't
addressed it. Did I miss your reply on this?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Andy Shevchenko]

On Wed, Jun 10, 2026 at 04:24:16AM +0000, Hungyu Lin wrote:
> Replace open-coded bitfield operations with FIELD_PREP() and
> FIELD_GET() helpers where appropriate.
> 
> Also simplify bmc150_magn_set_odr() by returning directly from
> the matching table entry.

Thanks, v4 is almost good. A couple of nit-picks, and if get addressed,
I will give my tag for that new version (presumably v5).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] iio: magnetometer: bmc150: use FIELD_PREP and FIELD_GET helpers

[Hungyu Lin]

> > -     odr_val = (reg_val & BMC150_MAGN_MASK_ODR) >> BMC150_MAGN_SHIFT_ODR;
> > +
> > +     odr_val = FIELD_GET(BMC150_MAGN_MASK_ODR, reg_val);
>
> Same comment as per previous few versions. I don't understand why you haven't
> addressed it. Did I miss your reply on this?

You didn't miss it.

After sending v4, I noticed that the blank line issue had not
actually been fixed and sent v5 shortly afterwards.

The include grouping issue you pointed out was then addressed
in v6.

Thanks,
Hungyu