On Thu, 22 Sep 2022, Sergiu Moga wrote: > Convert all open-coded instances of bitfields retrieval/setting > to FIELD_PREP/FIELD_GET where possible. > > Signed-off-by: Sergiu Moga <sergiu.moga@xxxxxxxxxxxxx> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> With one caveat below which I leave up to you to decide as it might be more a matter of taste thing. > @@ -82,7 +84,7 @@ > #define ATMEL_US_INACK BIT(20) /* Inhibit Non Acknowledge */ > #define ATMEL_US_DSNACK BIT(21) /* Disable Successive NACK */ > #define ATMEL_US_MAX_ITER_MASK GENMASK(26, 24) /* Max Iterations */ > -#define ATMEL_US_MAX_ITER(n) (((n) << 24) & ATMEL_US_MAX_ITER_MASK) > +#define ATMEL_US_MAX_ITER(n) FIELD_PREP(ATMEL_US_MAX_ITER_MASK, (n)) > #define ATMEL_US_FILTER BIT(28) /* Infrared Receive Line Filter */ > > #define ATMEL_US_IER 0x08 /* Interrupt Enable Register */ > @@ -134,19 +136,19 @@ > > #define ATMEL_US_CMPR 0x90 /* Comparaison Register */ > #define ATMEL_US_FMR 0xa0 /* FIFO Mode Register */ > -#define ATMEL_US_TXRDYM(data) (((data) & 0x3) << 0) /* TX Ready Mode */ > -#define ATMEL_US_RXRDYM(data) (((data) & 0x3) << 4) /* RX Ready Mode */ > +#define ATMEL_US_TXRDYM(data) FIELD_PREP(GENMASK(1, 0), (data)) /* TX Ready Mode */ > +#define ATMEL_US_RXRDYM(data) FIELD_PREP(GENMASK(5, 4), (data)) /* RX Ready Mode */ > #define ATMEL_US_ONE_DATA 0x0 > #define ATMEL_US_TWO_DATA 0x1 > #define ATMEL_US_FOUR_DATA 0x2 > #define ATMEL_US_FRTSC BIT(7) /* FIFO RTS pin Control */ > -#define ATMEL_US_TXFTHRES(thr) (((thr) & 0x3f) << 8) /* TX FIFO Threshold */ > -#define ATMEL_US_RXFTHRES(thr) (((thr) & 0x3f) << 16) /* RX FIFO Threshold */ > -#define ATMEL_US_RXFTHRES2(thr) (((thr) & 0x3f) << 24) /* RX FIFO Threshold2 */ > +#define ATMEL_US_TXFTHRES(thr) FIELD_PREP(GENMASK(13, 8), (thr)) /* TX FIFO Threshold */ > +#define ATMEL_US_RXFTHRES(thr) FIELD_PREP(GENMASK(21, 16), (thr)) /* RX FIFO Threshold */ > +#define ATMEL_US_RXFTHRES2(thr) FIELD_PREP(GENMASK(29, 24), (thr)) /* RX FIFO Threshold2 */ > > #define ATMEL_US_FLR 0xa4 /* FIFO Level Register */ > -#define ATMEL_US_TXFL(reg) (((reg) >> 0) & 0x3f) /* TX FIFO Level */ > -#define ATMEL_US_RXFL(reg) (((reg) >> 16) & 0x3f) /* RX FIFO Level */ > +#define ATMEL_US_TXFL(reg) FIELD_GET(GENMASK(5, 0), (reg)) /* TX FIFO Level */ > +#define ATMEL_US_RXFL(reg) FIELD_GET(GENMASK(21, 16), (reg)) /* RX FIFO Level */ I don't particularly like this kind of constructs. IMHO, all these would be nice after removing the macro argument and moving the FIELD_PREP() to .c file. That is, .h: #define ATMEL_US_RXFL GENMASK(21, 16) .c: ... FIELD_PREP(ATMEL_US_RXFL, arg) ... But I guess there might be differing opinions on it (and both are correct from purely technical perspective). -- i.
