Re: [RFC] hw/registerfields: add `FIELDx_1CLEAR()` macro

[Philippe Mathieu-Daudé]

On 1/9/22 07:32, Richard Henderson wrote:
> On 9/1/22 02:02, Wilfred Mallawa wrote:
> > From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> >
> > Adds a helper macro that implements the `rw1c`
> > behaviour.
> >
> > Ex:
> >    uint32_t data = FIELD32_1CLEAR(val, REG, FIELD);
> >
> > if the specified `FIELD` is set (single/multi bit all fields)
> > then the respective field is cleared and returned to `data`.
> >
> > If ALL bits of the bitfield are not set, then no change and
> > val is returned.
> >
> > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> Why do these operations need to go into hw/registerfields.h?
> It's not a common operation, since we've never needed it so far.

I suggested it to improve readability of this patch:
https://lore.kernel.org/qemu-devel/c33257a3-645f-9386-52e5-21a15ef0ebe5@amsat.org/

> > ---
> >   include/hw/registerfields.h | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> >
> > diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> > index 1330ca77de..5a804f72e3 100644
> > --- a/include/hw/registerfields.h
> > +++ b/include/hw/registerfields.h
> > @@ -115,6 +115,34 @@
> >                     R_ ## reg ## _ ## field ## _LENGTH, 
> > _v.v);              \
> >       _d; })
> > +/* Get the max value (uint) discribed by `num_bits` bits */
> > +#define MAX_N_BITS(num_bits) ((1 << (num_bits)) - 1)
> > +
> > +/*
> > + * Clear the specified field in reg_val if
> > + * all field bits are set, else no changes made. Implements
> > + * single/multi-bit `rw1c`
> > + */
> > +#define FIELD8_1CLEAR(reg_val, reg, 
> > field)                                \
> > +    ((FIELD_EX8(reg_val, reg, field) 
> > ==                                   \
> > +      MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) 
> > ?                   \
> > +      FIELD_DP8(reg_val, reg, field, 0x00) : reg_val)
> > +
> > +#define FIELD16_1CLEAR(reg_val, reg, 
> > field)                               \
> > +    ((FIELD_EX16(reg_val, reg, field) 
> > ==                                  \
> > +      MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) 
> > ?                   \
> > +      FIELD_DP16(reg_val, reg, field, 0x00) : reg_val)
> > +
> > +#define FIELD32_1CLEAR(reg_val, reg, 
> > field)                               \
> > +    ((FIELD_EX32(reg_val, reg, field) 
> > ==                                  \
> > +      MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) 
> > ?                   \
> > +      FIELD_DP32(reg_val, reg, field, 0x00) : reg_val)
> > +
> > +#define FIELD64_1CLEAR(reg_val, reg, 
> > field)                               \
> > +    ((FIELD_EX64(reg_val, reg, field) 
> > ==                                  \
> > +      MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) 
> > ?                   \
> > +      FIELD_DP64(reg_val, reg, field, 0x00) : reg_val)
> > +
> >   #define FIELD_SDP8(storage, reg, field, val) 
> > ({                           \
> >       struct 
> > {                                                              \
> >           signed int v:R_ ## reg ## _ ## field ## 
> > _LENGTH;                  \