Hi Biju,
On Sat, 8 Mar 2025 at 07:30, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH v2 2/2] can: rcar_canfd: Fix page entries in the AFL list
> >
> > 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).
Note that only channels 0-3 are tested on White Hawk, as the latter
have no transceivers mounted.
Note to self: test channels 4-7 on the CAN board connector,
using a transceiver-less bus from Siemens ApNote AP2921.
> > 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?
IIRC, it did not at the time CAN support for R-Car V4M was upstreamed.
> Q2) Does it by chance is in standby mode?
You mean the transceiver?
All channels but channel zero use the same type of transceiver,
and similar wiring. There might still be a pin control bug, though.
> Q3) Does it work if you just configure single channel by connecting to an external CAN device?
Haven't tried that yet, same for logic analyzer.
(before this patch, the logic analyzer showed that channel 2 did not
seem to work at all, and channel 3 worked for transmit only, but that
info probably doesn't help much :^).
> Q4) if you are testing in loopback mode, is failure happens CAN2->CAN3 or CAN3>CAN2?
Fails in both directions.
> > > --- 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...
Thanks, and good luck!
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
