Hi Geert,
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 07 March 2025 14:42
> Subject: Re: [PATCH v2 2/2] can: rcar_canfd: Fix page entries in the AFL list
>
> 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>
Thanks for testing.
So, the fix now works on 3 different boards with channel number > 2
RZ/G3E SMARC(6 channels), White Hawk and White Hawk Single development boards based on
R-Car V4H(8 Channels).
>
> 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.
Q1) Does it worked with downstream BSP?
Q2) Does it by chance is in standby mode?
Q3) Does it work if you just configure single channel by connecting to an external CAN device?
Q4) if you are testing in loopback mode, is failure happens CAN2->CAN3 or CAN3>CAN2?
>
> > --- 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.
>
> Looking at the actual behavior of your patch, the same can be achieved by updating start, by adding a
> single line here:
>
> start += (ch & -2) * num_rules;
start is not readable at all. I need to work out from " 102_uciaprcn0140_IPSpec_v1p06_r2.pdf"
section 7.7 to understand, what is page, rule_index_entry etc...
Cheers,
Biju
