Hi Biju,
On Fri, 7 Mar 2025 at 15:41, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> 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.
>
> 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;
>
> > @@ -802,7 +803,7 @@ static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv,
> > }
> >
> > /* Enable write access to entry */
> > - page = RCANFD_GAFL_PAGENUM(start);
> > + page = RCANFD_GAFL_PAGENUM(rule_entry);
The similar fix in the BSP[1] keeps the old value of start here.
However, it does not make a difference for me (both R-Car V4H and V4M).
> > rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLECTR,
> > (RCANFD_GAFLECTR_AFLPN(gpriv, page) |
> > RCANFD_GAFLECTR_AFLDAE));
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
