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@bp.renesas.com 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@vger.kernel.org Signed-off-by: Biju Das biju.das.jz@bp.renesas.com

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@glider.be

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);
    rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLECTR,
                       (RCANFD_GAFLECTR_AFLPN(gpriv, page) |
                        RCANFD_GAFLECTR_AFLDAE));
  

Out of context code:

/* Write number of rules for channel */ rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLCFG(ch), RCANFD_GAFLCFG_SETRNC(gpriv, ch, num_rules));

So the old code calculated the page number based on the pre-configured number of rules in the RCANFD_GAFLCFGw register, not taking into account ch > 2, and then reprogrammed the register with the new number of rules... Hmm...

@@ -818,13 +819,13 @@ static void rcar_canfd_configure_afl_rules(struct rcar_canfd_global *gpriv, offset = RCANFD_C_GAFL_OFFSET;

    /* Accept all IDs */

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLID(offset, start), 0);
  

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLID(offset, rule_entry_index), 0);
    /* IDE or RTR is not considered for matching */
  

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLM(offset, start), 0);
  

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLM(offset, rule_entry_index), 0);
    /* Any data length accepted */
  

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLP0(offset, start), 0);
  

•   rcar_canfd_write(gpriv->base, RCANFD_GAFLP0(offset, rule_entry_index), 0);
    /* Place the msg in corresponding Rx FIFO entry */
  

•   rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLP1(offset, start),
  

•   rcar_canfd_set_bit(gpriv->base, RCANFD_GAFLP1(offset, rule_entry_index),
                       RCANFD_GAFLP1_GAFLFDP(ridx));
  
    /* Disable write access to page */
  

@@ -1851,6 +1852,7 @@ static int rcar_canfd_probe(struct platform_device *pdev) unsigned long channels_mask = 0; int err, ch_irq, g_irq; int g_err_irq, g_recc_irq;

•   u32 rule_entry = 0;
    bool fdmode = true;                     /* CAN FD only mode - default */
    char name[9] = "channelX";
    int i;
  

@@ -2023,7 +2025,8 @@ static int rcar_canfd_probe(struct platform_device *pdev) rcar_canfd_configure_tx(gpriv, ch);

            /* Configure receive rules */

•           rcar_canfd_configure_afl_rules(gpriv, ch);
  

•           rcar_canfd_configure_afl_rules(gpriv, ch, rule_entry);
  

•           rule_entry += RCANFD_CHANNEL_NUMRULES;
    }
  
    /* Configure common interrupts */
  

Gr{oetje,eeting}s,

Geert