[PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[Andy Shevchenko]

One time in regmap and already a few times over the whole kernel
the users want to extract a mask from the regmap field data type.
Provide a helper and convert regmap core to use it. Others may
follow later on.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

Inspired by a patch SN1PR12MB25282EC93A27F3577BB9B454A5772@SN1PR12MB2528.namprd12.prod.outlook.com
that leaves the driver to handle the custom macro for that.

 drivers/base/regmap/regmap.c | 2 +-
 include/linux/regmap.h       | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 83f1f5942135..48e96bc9e7b6 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1215,7 +1215,7 @@ static void regmap_field_init(struct regmap_field *rm_field,
 	rm_field->regmap = regmap;
 	rm_field->reg = reg_field.reg;
 	rm_field->shift = reg_field.lsb;
-	rm_field->mask = GENMASK(reg_field.msb, reg_field.lsb);
+	rm_field->mask = REG_FIELD_GENMASK(reg_field);
 
 	WARN_ONCE(rm_field->mask == 0, "invalid empty mask defined\n");
 
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index a72f17ab6264..8f3034881f8a 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -10,6 +10,7 @@
  * Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
  */
 
+#include <linux/bits.h>
 #include <linux/bug.h>
 #include <linux/cleanup.h>
 #include <linux/delay.h>
@@ -1477,6 +1478,8 @@ struct reg_field {
 				.id_offset = _offset,		\
 				}
 
+#define REG_FIELD_GENMASK(_reg_field)	GENMASK(_reg_field.msb, _reg_field.lsb)
+
 struct regmap_field *regmap_field_alloc(struct regmap *regmap,
 		struct reg_field reg_field);
 void regmap_field_free(struct regmap_field *field);
-- 
2.50.1

Re: [PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[Victor Chernyakin]

> #define REG_FIELD_GENMASK(_reg_field)	GENMASK(_reg_field.msb, _reg_field.lsb)

Macro parameter should have parens around it:

+ #define REG_FIELD_GENMASK(_reg_field)	GENMASK((_reg_field).msb, (_reg_field).lsb)

(In the patch you link to, I wrote it without parens at first too,
then had to figure out why I was getting "too many arguments to macro"
error messages)




BTW, I've been thinking about making reg_field store the mask directly,
by putting GENMASK(lsb, msb) in the REG_FIELD macro, but that's probably
a more involved change.

Re: [PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 09:39:45AM -0600, Victor Chernyakin wrote:
> > #define REG_FIELD_GENMASK(_reg_field)	GENMASK(_reg_field.msb, _reg_field.lsb)
> 
> Macro parameter should have parens around it:

In this case no, it shouldn't. It a shortcut which assumes that _reg_field is
not an expression.

> + #define REG_FIELD_GENMASK(_reg_field)	GENMASK((_reg_field).msb, (_reg_field).lsb)

This doesn't look good as it implies that _reg_field can be an expression that
will be evaluated twice.

> (In the patch you link to, I wrote it without parens at first too,
> then had to figure out why I was getting "too many arguments to macro"
> error messages)

This is strange.

I have compiled it with GCC-15 and clang-22 (in both cases sparse enabled),
no such a problem (It's also -Werror build). Can you elaborate?

> BTW, I've been thinking about making reg_field store the mask directly,
> by putting GENMASK(lsb, msb) in the REG_FIELD macro, but that's probably
> a more involved change.

But it's already there as you see in the regmap core. What you want is to make
it at the beginning. Not sure if it's feasible and needed change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[Victor Chernyakin]

> > + #define REG_FIELD_GENMASK(_reg_field)       GENMASK((_reg_field).msb, (_reg_field).lsb)
>
> This doesn't look good as it implies that _reg_field can be an expression that
> will be evaluated twice.
>
> > (In the patch you link to, I wrote it without parens at first too,
> > then had to figure out why I was getting "too many arguments to macro"
> > error messages)
>
> I have compiled it with GCC-15 and clang-22 (in both cases sparse enabled),
> no such a problem (It's also -Werror build). Can you elaborate?

In that patch we do:

#define MSA311_F_SOFT_RESET_I2C	REG_FIELD(MSA311_SOFT_RESET_REG, 2, 2)
...
MSA311_GENMASK(MSA311_F_SOFT_RESET_I2C)

which, without extra parens, expands like so:

GENMASK((struct reg_field){.lsb = ..., .msb = ...}.msb, (struct reg_field){lsb = ..., .msb = ...}.lsb)

which breaks because of the commas between the designated initializers.

> > BTW, I've been thinking about making reg_field store the mask directly,
> > by putting GENMASK(lsb, msb) in the REG_FIELD macro, but that's probably
> > a more involved change.
>
> But it's already there as you see in the regmap core. What you want is to make
> it at the beginning. Not sure if it's feasible and needed change.

If all the places where reg_field is used want a mask and not an msb+lsb pair
(I'm not certain that's the case, would have to check), I do think it would be a small
but worthwhile improvement to store it directly.

Re: [PATCH v1 1/1] regmap: Add REG_FIELD_GENMASK() helper macro

[Victor Chernyakin]

> > > + #define REG_FIELD_GENMASK(_reg_field)       GENMASK((_reg_field).msb, (_reg_field).lsb)
> >
> > This doesn't look good as it implies that _reg_field can be an expression that
> > will be evaluated twice.
> >
> > > (In the patch you link to, I wrote it without parens at first too,
> > > then had to figure out why I was getting "too many arguments to macro"
> > > error messages)
> >
> > I have compiled it with GCC-15 and clang-22 (in both cases sparse enabled),
> > no such a problem (It's also -Werror build). Can you elaborate?
> 
> In that patch we do:
> 
> #define MSA311_F_SOFT_RESET_I2C	REG_FIELD(MSA311_SOFT_RESET_REG, 2, 2)
> ...
> MSA311_GENMASK(MSA311_F_SOFT_RESET_I2C)
> 
> which, without extra parens, expands like so:
> 
> GENMASK((struct reg_field){.lsb = ..., .msb = ...}.msb, (struct reg_field){lsb = ..., .msb = ...}.lsb)
> 
> which breaks because of the commas between the designated initializers.

Actually now that I think about it, the better solution would probably
be to put the parens in the REG_FIELD macro:

#define REG_FIELD(reg, msb, lsb) ((struct reg_field){ ... })

So I take back what I said, I think your patch is good as-is.