On Wed, 18 Sept 2024 at 22:31, Corey Minyard <co...@minyard.net> wrote:
> Generally it's frowned upon to have a ton of extra stuff that's not
> used. I would think some is ok, like defining bits in registers that
> aren't used yet, but I have no idea how all the enums below here
> actually tie into anything. I'm guessing these are just bitmasks into
> registers, but really, it's a lot easier to read if you have something
> like:
>
> /*
> * The I2C Master function enable. When disabled, the Master
> * configuration settings are not changed, but the Master function is
> * internally reset.
> */
> #define FLEXCOMM_I2C_CFG_MSTEN (1 << 4)
The FIELD macro already gives you that:
FIELD(FLEXCOMM_I2C_CFG, MSTEN, startbit, len);
will define an R_FLEXCOMM_I2C_CFG_MSTEN_MASK (which is
(1 << startbit) for the 'len == 1' case).
You can also set and read a 1 bit field the same as
any other, with the FIELD_DP32/FIELD_EX32 macros, so
you don't often need to directly use the MASK macro:
s->cfg = FIELD_DP32(s->cfg, CFG, MSTEN, 1);
and
if (FIELD_EX32(s->cfg, CFG, MSTEN)) {
...
}
The FIELD() macros are a bit unwieldy sometimes but the
advantage over ad-hoc #defines is that they're consistent
for every field in every register.
I agree that providing enums for the possible values for 1-bit
fields is a bit superfluous.
thanks
-- PMM
