RE: [PATCH v4 07/11] can: rcar_canfd: Add mask table to struct rcar_canfd_hw_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux