On Fri, 27 Dec 2024 11:51:50 +0000 Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Replace raw bitmask values with the `GENMASK()` macro in the `i2c-riic` > driver to improve readability and maintain consistency. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > Reviewed-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > Tested-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > --- > v2->v3 > - Collected RB and tested tags > > v1->v2 > - Collected RB tag from Geert > --- > drivers/i2c/busses/i2c-riic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c > index 954e066d61a8..ddae4b74a86b 100644 > --- a/drivers/i2c/busses/i2c-riic.c > +++ b/drivers/i2c/busses/i2c-riic.c > @@ -55,7 +55,7 @@ > #define ICCR2_RS BIT(2) > #define ICCR2_ST BIT(1) > > -#define ICMR1_CKS_MASK 0x70 > +#define ICMR1_CKS_MASK GENMASK(6, 4) > #define ICMR1_BCWP BIT(3) > #define ICMR1_CKS(_x) ((((_x) << 4) & ICMR1_CKS_MASK) | ICMR1_BCWP) I'm really not at all sure how this makes it 'more readable'. Call me 'old fashioned' but I like hex constants - especially for bytes. In this case it might be best to be consistent with the hardware datasheet. Pretty much the only time I've actually used anything like BIT() was for a spec that numbered the bits from 1 to 32 with bit 1 being the most significant. > > @@ -73,7 +73,7 @@ > > #define ICSR2_NACKF BIT(4) > > -#define ICBR_RESERVED 0xe0 /* Should be 1 on writes */ > +#define ICBR_RESERVED GENMASK(7, 5) /* Should be 1 on writes */ 'Should all be set on writes' ? David > > #define RIIC_INIT_MSG -1 >
