Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > Sent: 14 March 2025 14:47 > Subject: Re: [PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info > > Hi Biju, > > Thanks for your patch! > > On Thu, 6 Mar 2025 at 13:43, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > R-Car Gen3 and Gen4 has some differences in the mask bits. Add a > > have OK. > > > mask table to handle these differences. > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > Suggestions for improvements below. > > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > @@ -82,25 +82,24 @@ > > #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)->info->mask_table[RCANFD_EEF_MASK] | > > + RCANFD_GERFL_MES | \ > > ((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0))) > > RZ/G3E documents only EFF bits 0-5. > I think this can just be replaced by gpriv->channels_mask << 16, especially as there is only one user > of RCANFD_GERFL_ERR(). Agreed and will drop RCANFD_GERFL_EEF0_7 as it is unused. > > > > > /* AFL Rx rules registers */ > > > > /* RSCFDnCFDGAFLCFG0 / RSCFDnGAFLCFG0 */ #define > > RCANFD_GAFLCFG_SETRNC(gpriv, n, x) \ > > - (((x) & reg_gen4(gpriv, 0x1ff, 0xff)) << \ > > + (((x) & (gpriv)->info->mask_table[RCANFD_RNC_MASK]) << \ > > (reg_gen4(gpriv, 16, 24) - ((n) & 1) * reg_gen4(gpriv, 16, > > 8))) > > All other mask values are just the maximum value of a parameter. > E.g. the RNC mask is the number of supported rules - 1. > So perhaps store these as parameters instead? OK, will introduce num_supported_rules variable in struct rcar_canfd_hw_info. Cheers, Biju
