This patch series addresses 2 issues 1) Fix typo in pattern properties for R-Car V4M. 2) Fix page entries in the AFL list.
v1->v2: * Split fixes patches as separate series. * Added Rb tag from Geert for binding patch. * Added the tag Cc:stable@vger.kernel.org
Biju Das (2): dt-bindings: can: renesas,rcar-canfd: Fix typo in pattern properties for R-Car V4M can: rcar_canfd: Fix page entries in the AFL list
.../bindings/net/can/renesas,rcar-canfd.yaml | 2 +- drivers/net/can/rcar/rcar_canfd.c | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-)
The Renesas R-Car V4M(R8A779H0) SoC, supports up to four channels. Fix the typo 5->4 in pattern properties.
Fixes: ced52c6ed257 ("dt-bindings: can: renesas,rcar-canfd: Document R-Car V4M support") Cc: stable@vger.kernel.org Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be Signed-off-by: Biju Das biju.das.jz@bp.renesas.com --- v1->v2: * Added Rb tag from Geert * * Added the tag Cc:stable@vger.kernel.org --- .../devicetree/bindings/net/can/renesas,rcar-canfd.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml index 7c5ac5d2e880..f6884f6e59e7 100644 --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml @@ -170,7 +170,7 @@ allOf: const: renesas,r8a779h0-canfd then: patternProperties: - "^channel[5-7]$": false + "^channel[4-7]$": false else: if: not:
On Thu, 20 Feb 2025 09:45:11 +0000, Biju Das wrote:
The Renesas R-Car V4M(R8A779H0) SoC, supports up to four channels. Fix the typo 5->4 in pattern properties.
Fixes: ced52c6ed257 ("dt-bindings: can: renesas,rcar-canfd: Document R-Car V4M support") Cc: stable@vger.kernel.org Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be Signed-off-by: Biju Das biju.das.jz@bp.renesas.com
v1->v2:
- Added Rb tag from Geert
- Added the tag Cc:stable@vger.kernel.org
.../devicetree/bindings/net/can/renesas,rcar-canfd.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Rob Herring (Arm) robh@kernel.org
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 --- v1->v2: * Added the tag Cc:stable@vger.kernel.org --- drivers/net/can/rcar/rcar_canfd.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index df1a5d0b37b2..270f50d836f5 100644 --- 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) { @@ -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)); @@ -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 */
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!
I hope to give this a try on White Hawk and Gray Hawk Single, where I never got any channels beyond the first two to work...
Gr{oetje,eeting}s,
Geert
Hi Geert Uytterhoeven,
-----Original Message----- From: Geert Uytterhoeven geert@linux-m68k.org Sent: 24 February 2025 10:18 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@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!
I hope to give this a try on White Hawk and Gray Hawk Single, where I never got any channels beyond the first two to work...
Thanks for tying. I guess, on those boards start will get a value "0" for first channel and "1" for second channel like RZ/G2L.
Cheers, Biju
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
On Fri, 7 Mar 2025 at 15:41, Geert Uytterhoeven geert@linux-m68k.org wrote:
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
= 2.
of rules... Hmm...
Gr{oetje,eeting}s,
Geert
Hi Biju,
On Fri, 7 Mar 2025 at 15:41, Geert Uytterhoeven geert@linux-m68k.org wrote:
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);
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
On Fri, 7 Mar 2025 at 16:02, Geert Uytterhoeven geert@linux-m68k.org wrote:
On Fri, 7 Mar 2025 at 15:41, Geert Uytterhoeven geert@linux-m68k.org wrote:
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);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).
Sorry, forgot to add the link [1] https://github.com/renesas-rcar/linux-bsp/commit/e9ba9ac6fe77baa3
Gr{oetje,eeting}s,
Geert
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@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.
If you don't need this variable anymore, please remove it.
Marc
Hi Marc and Geert,
-----Original Message----- From: Marc Kleine-Budde mkl@pengutronix.de 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@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.
If you don't need this variable anymore, please remove it.
OK, I will remove unused start variable.
Cheers, Biju
Hi Geert,
-----Original Message----- From: Geert Uytterhoeven geert@linux-m68k.org 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@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
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
Hi Biju,
On Sat, 8 Mar 2025 at 07:30, Biju Das biju.das.jz@bp.renesas.com 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@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
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
Hi Geert,
Thanks for the feedback.
-----Original Message----- From: Geert Uytterhoeven geert@linux-m68k.org Sent: 10 March 2025 09:03 Subject: Re: [PATCH v2 2/2] can: rcar_canfd: Fix page entries in the AFL list
Hi Biju,
On Sat, 8 Mar 2025 at 07:30, Biju Das biju.das.jz@bp.renesas.com 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@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
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.
Maybe we need to check with R-Car BSP people to confirm? Let me check internally for contact person of the R-Car V4M BSP.
Q2) Does it by chance is in standby mode?
You mean the transceiver?
Yes, for some boards. we need to toggle GPIO to move it from stand by to normal mode, so that it can start communication.
All channels but channel zero use the same type of transceiver, and similar wiring. There might still be a pin control bug, though.
Maybe checking with BSP team will confirm this.
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 :^).
When I tried without this patch on RZ/G3E, using 1 channel ( disabling other one) always worked.
Q4) if you are testing in loopback mode, is failure happens CAN2->CAN3 or CAN3>CAN2?
Fails in both directions.
On RZ/G3E SMARC, without this patch. CAN2->CAN3 fails. But CAN3>CAN2 is mirroring messages, and I received 2 messages, when I sent 1.
Cheers, Biju
Hi Biju,
On Mon, 10 Mar 2025 at 10:28, Biju Das biju.das.jz@bp.renesas.com wrote:
From: Geert Uytterhoeven geert@linux-m68k.org
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.
Q2) Does it by chance is in standby mode?
You mean the transceiver?
Yes, for some boards. we need to toggle GPIO to move it from stand by to normal mode, so that it can start communication.
CAN0 on White/Gray Hawk variants uses TJR1443AT transceivers, which do have enable-gpios (and do work).
The other channels use MCP2558FD transceivers, and their standby pin is always deasserted (except during system reset).
All channels but channel zero use the same type of transceiver, and similar wiring. There might still be a pin control bug, though.
Gr{oetje,eeting}s,
Geert
linux-stable-mirror@lists.linaro.org
-
Biju Das -
Geert Uytterhoeven -
Marc Kleine-Budde -
Rob Herring (Arm)