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.