Hi Marc and Geert,
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: 07 March 2025 15:09
> Subject: Re: [PATCH v2 2/2] can: rcar_canfd: Fix page entries in the AFL list
>
> On 07.03.2025 15:41:52, Geert Uytterhoeven wrote:
> > Hi Biju,
> >
> > On Thu, 20 Feb 2025 at 10:45, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > There are a total of 96 AFL pages and each page has 16 entries with
> > > registers CFDGAFLIDr, CFDGAFLMr, CFDGAFLP0r, CFDGAFLP1r holding the
> > > rule entries (r = 0..15).
> > >
> > > Currently, RCANFD_GAFL* macros use a start variable to find AFL
> > > entries, which is incorrect as the testing on RZ/G3E shows ch1 and
> > > ch4 gets a start value of 0 and the register contents are overwritten.
> > >
> > > Fix this issue by using rule_entry corresponding to the channel to
> > > find the page entries in the AFL list.
> > >
> > > Fixes: dd3bd23eb438 ("can: rcar_canfd: Add Renesas R-Car CAN FD
> > > driver")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> > This finally fixes CAN2 and CAN3 on the White Hawk and White Hawk
> > Single development boards based on R-Car V4H with 8 CAN channels (the
> > transceivers for CAN4-7 are not mounted), so
> > Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> >
> > Unfortunately, it does not fix CAN2 and CAN3 on the Gray Hawk Single
> > development board, which is based on R-Car V4M with 4 CAN channels.
> >
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > > @@ -787,10 +787,11 @@ static void
> > > rcar_canfd_configure_controller(struct rcar_canfd_global *gpriv) }
> > >
> > > static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv,
> > > - u32 ch)
> > > + u32 ch, u32 rule_entry)
> > > {
> > > u32 cfg;
> > > int offset, start, page, num_rules =
> > > RCANFD_CHANNEL_NUMRULES;
> > > + u32 rule_entry_index = rule_entry % 16;
> > > u32 ridx = ch + RCANFD_RFFIFO_IDX;
> > >
> > > if (ch == 0) {
> >
> > The out-of-context code does:
> >
> > start = 0; /* Channel 0 always starts from 0th rule */
> > } else {
> > /* Get number of Channel 0 rules and adjust */
> > cfg = rcar_canfd_read(gpriv->base, RCANFD_GAFLCFG(ch));
> > start = RCANFD_GAFLCFG_GETRNC(gpriv, 0, cfg);
> > }
> >
> > After your changes below, "start" is set but never used.
>
> If you don't need this variable anymore, please remove it.
OK, I will remove unused start variable.
Cheers,
Biju
