Hi Vincent, Thanks for the feedback. > -----Original Message----- > From: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> > Sent: 28 March 2025 09:10 > Subject: Re: [PATCH v7 05/18] can: rcar_canfd: Drop RCANFD_GERFL_EEF* macros in RCANFD_GERFL_ERR > > On 26/03/2025 à 21:19, Biju Das wrote: > > The macros RCANFD_GERFL_EEF* in RCANFD_GERFL_ERR can be replaced by > > gpriv->channels_mask << 16. > > > > After this drop the macro RCANFD_GERFL_EEF0_7 as it is unused. > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > --- > > v6->v7: > > * No change. > > v5->v6: > > * Collected tag. > > v5: > > * New patch. > > --- > > drivers/net/can/rcar/rcar_canfd.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/net/can/rcar/rcar_canfd.c > > b/drivers/net/can/rcar/rcar_canfd.c > > index 565a91c2ca83..a9e962a1510e 100644 > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > @@ -74,7 +74,6 @@ > > #define RCANFD_GSTS_GNOPM (BIT(0) | BIT(1) | BIT(2) | BIT(3)) > > > > /* RSCFDnCFDGERFL / RSCFDnGERFL */ > > -#define RCANFD_GERFL_EEF0_7 GENMASK(23, 16) > > #define RCANFD_GERFL_EEF(ch) BIT(16 + (ch)) > > #define RCANFD_GERFL_CMPOF BIT(3) /* CAN FD only */ > > #define RCANFD_GERFL_THLES BIT(2) > > @@ -82,9 +81,7 @@ > > #define RCANFD_GERFL_DEF BIT(0) > > > > #define RCANFD_GERFL_ERR(gpriv, x) \ > > - ((x) & (reg_gen4(gpriv, RCANFD_GERFL_EEF0_7, \ > > - RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \ > > - RCANFD_GERFL_MES | \ > > + ((x) & ((gpriv->channels_mask << 16) | RCANFD_GERFL_MES | \ > > The previous RCANFD_GERFL_EEF0_7 macro documented that the register was from bit index 16 to bit index > 23. Here, we loose this piece of information. > > What about: > > FIELD_PREP(RCANFD_GERFL_EEF0_7, gpriv->channels_mask) For all SoCs, ECC Error flag for Channel x (a.k.a. EEFx) starts from BIT position 16. By using gpriv->channels_mask, we allow only enabled channels and << 16 says it is from Bit position 16. Gen4 has 7 channels G3E has 6 channels V4M has 4 channels V3H_2 has 3 channels All other SoCs has 2 channels Am I missing anything here? Cheers, Biju
