diff mbox series

can: rcar_canfd: Add missing ECC error checks for channels 2-7

Message ID 4edb2ea46cc64d0532a08a924179827481e14b4f.1666951503.git.geert+renesas@glider.be (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series can: rcar_canfd: Add missing ECC error checks for channels 2-7 | expand

Checks

Context Check Description
netdev/tree_selection success Series ignored based on subject

Commit Message

Geert Uytterhoeven Oct. 28, 2022, 10:06 a.m. UTC
When introducing support for R-Car V3U, which has 8 instead of 2
channels, the ECC error bitmask was extended to take into account the
extra channels, but rcar_canfd_global_error() was not updated to act
upon the extra bits.

Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes the
channel number, fixing R-Car V3U while simplifying the code.

Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for r8a779a0 SoC")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only.

This patch conflicts with "[PATCH v3 6/6] can: rcar_canfd: Add
has_gerfl_eef to struct rcar_canfd_hw_info"[1].  Sorry for that.

[1] https://lore.kernel.org/all/20221027082158.95895-7-biju.das.jz@bp.renesas.com/
---
 drivers/net/can/rcar/rcar_canfd.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Comments

Biju Das Oct. 28, 2022, 10:21 a.m. UTC | #1
Hi Geert,

> Subject: [PATCH] can: rcar_canfd: Add missing ECC error checks for
> channels 2-7
> 
> When introducing support for R-Car V3U, which has 8 instead of 2
> channels, the ECC error bitmask was extended to take into account the
> extra channels, but rcar_canfd_global_error() was not updated to act
> upon the extra bits.
> 
> Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes the
> channel number, fixing R-Car V3U while simplifying the code.
> 
> Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for r8a779a0
> SoC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Compile-tested only.
> 
> This patch conflicts with "[PATCH v3 6/6] can: rcar_canfd: Add
> has_gerfl_eef to struct rcar_canfd_hw_info"[1].  Sorry for that.

Ok. I will add dependency on 6/6 with this patch.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju

> 
> [1]
> ---
>  drivers/net/can/rcar/rcar_canfd.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/rcar/rcar_canfd.c
> b/drivers/net/can/rcar/rcar_canfd.c
> index 710bd0e9c3c08c02..7cca9b7507cc6805 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -81,8 +81,7 @@ enum rcanfd_chip_id {
> 
>  /* RSCFDnCFDGERFL / RSCFDnGERFL */
>  #define RCANFD_GERFL_EEF0_7		GENMASK(23, 16)
> -#define RCANFD_GERFL_EEF1		BIT(17)
> -#define RCANFD_GERFL_EEF0		BIT(16)
> +#define RCANFD_GERFL_EEF(ch)		BIT(16 + (ch))
>  #define RCANFD_GERFL_CMPOF		BIT(3)	/* CAN FD only */
>  #define RCANFD_GERFL_THLES		BIT(2)
>  #define RCANFD_GERFL_MES		BIT(1)
> @@ -90,7 +89,7 @@ enum rcanfd_chip_id {
> 
>  #define RCANFD_GERFL_ERR(gpriv, x) \
>  	((x) & (reg_v3u(gpriv, RCANFD_GERFL_EEF0_7, \
> -			RCANFD_GERFL_EEF0 | RCANFD_GERFL_EEF1) | \
> +			RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \
>  		RCANFD_GERFL_MES | \
>  		((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0)))
> 
> @@ -936,12 +935,8 @@ static void rcar_canfd_global_error(struct
> net_device *ndev)
>  	u32 ridx = ch + RCANFD_RFFIFO_IDX;
> 
>  	gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
> -	if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
> -		netdev_dbg(ndev, "Ch0: ECC Error flag\n");
> -		stats->tx_dropped++;
> -	}
> -	if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
> -		netdev_dbg(ndev, "Ch1: ECC Error flag\n");
> +	if (gerfl & RCANFD_GERFL_EEF(ch)) {
> +		netdev_dbg(ndev, "Ch%u: ECC Error flag\n", ch);
>  		stats->tx_dropped++;
>  	}
>  	if (gerfl & RCANFD_GERFL_MES) {
Marc Kleine-Budde Oct. 28, 2022, 10:29 a.m. UTC | #2
On 28.10.2022 12:06:45, Geert Uytterhoeven wrote:
> When introducing support for R-Car V3U, which has 8 instead of 2
> channels, the ECC error bitmask was extended to take into account the
> extra channels, but rcar_canfd_global_error() was not updated to act
> upon the extra bits.
> 
> Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes the
> channel number, fixing R-Car V3U while simplifying the code.
> 
> Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for r8a779a0 SoC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Is this stable material?

Marc
Geert Uytterhoeven Oct. 28, 2022, 11:06 a.m. UTC | #3
Hi Marc,

On Fri, Oct 28, 2022 at 12:29 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 28.10.2022 12:06:45, Geert Uytterhoeven wrote:
> > When introducing support for R-Car V3U, which has 8 instead of 2
> > channels, the ECC error bitmask was extended to take into account the
> > extra channels, but rcar_canfd_global_error() was not updated to act
> > upon the extra bits.
> >
> > Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes the
> > channel number, fixing R-Car V3U while simplifying the code.
> >
> > Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for r8a779a0 SoC")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Is this stable material?

Upstream DTS[1] has only the first two channels enabled, so it's not
critical. But it never hurts to end up in stable, helping e.g. CiP.

[1] arch/arm64/boot/dts/renesas/r8a779a0-falcon.dts

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
Biju Das Oct. 28, 2022, 11:27 a.m. UTC | #4
Hi Geert,

> Subject: Re: [PATCH] can: rcar_canfd: Add missing ECC error checks for
> channels 2-7
> 
> Hi Marc,
> 
> On Fri, Oct 28, 2022 at 12:29 PM Marc Kleine-Budde
> <mkl@pengutronix.de> wrote:
> > On 28.10.2022 12:06:45, Geert Uytterhoeven wrote:
> > > When introducing support for R-Car V3U, which has 8 instead of 2
> > > channels, the ECC error bitmask was extended to take into account
> > > the extra channels, but rcar_canfd_global_error() was not updated
> to
> > > act upon the extra bits.
> > >
> > > Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes
> > > the channel number, fixing R-Car V3U while simplifying the code.
> > >
> > > Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for
> r8a779a0
> > > SoC")
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Is this stable material?
> 
> Upstream DTS[1] has only the first two channels enabled, so it's not
> critical. But it never hurts to end up in stable, helping e.g. CiP.

Yes, That will avoid backporting effort if it goes via stable.

FYI,  We have a plan to backport the whole CAN FD fixes/enhancement series to
5.10 cip [2] once it hits Linux mainline-rc.

[2] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/log/?h=linux-5.10.y-cip

Cheers,
Biju
Marc Kleine-Budde Oct. 28, 2022, 12:11 p.m. UTC | #5
On 28.10.2022 12:06:45, Geert Uytterhoeven wrote:
> When introducing support for R-Car V3U, which has 8 instead of 2
> channels, the ECC error bitmask was extended to take into account the
> extra channels, but rcar_canfd_global_error() was not updated to act
> upon the extra bits.
> 
> Replace the RCANFD_GERFL_EEF[01] macros by a new macro that takes the
> channel number, fixing R-Car V3U while simplifying the code.
> 
> Fixes: 45721c406dcf50d4 ("can: rcar_canfd: Add support for r8a779a0 SoC")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Added stable on Cc and added to linux-can.

Marc
diff mbox series

Patch

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 710bd0e9c3c08c02..7cca9b7507cc6805 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -81,8 +81,7 @@  enum rcanfd_chip_id {
 
 /* RSCFDnCFDGERFL / RSCFDnGERFL */
 #define RCANFD_GERFL_EEF0_7		GENMASK(23, 16)
-#define RCANFD_GERFL_EEF1		BIT(17)
-#define RCANFD_GERFL_EEF0		BIT(16)
+#define RCANFD_GERFL_EEF(ch)		BIT(16 + (ch))
 #define RCANFD_GERFL_CMPOF		BIT(3)	/* CAN FD only */
 #define RCANFD_GERFL_THLES		BIT(2)
 #define RCANFD_GERFL_MES		BIT(1)
@@ -90,7 +89,7 @@  enum rcanfd_chip_id {
 
 #define RCANFD_GERFL_ERR(gpriv, x) \
 	((x) & (reg_v3u(gpriv, RCANFD_GERFL_EEF0_7, \
-			RCANFD_GERFL_EEF0 | RCANFD_GERFL_EEF1) | \
+			RCANFD_GERFL_EEF(0) | RCANFD_GERFL_EEF(1)) | \
 		RCANFD_GERFL_MES | \
 		((gpriv)->fdmode ? RCANFD_GERFL_CMPOF : 0)))
 
@@ -936,12 +935,8 @@  static void rcar_canfd_global_error(struct net_device *ndev)
 	u32 ridx = ch + RCANFD_RFFIFO_IDX;
 
 	gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
-	if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) {
-		netdev_dbg(ndev, "Ch0: ECC Error flag\n");
-		stats->tx_dropped++;
-	}
-	if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) {
-		netdev_dbg(ndev, "Ch1: ECC Error flag\n");
+	if (gerfl & RCANFD_GERFL_EEF(ch)) {
+		netdev_dbg(ndev, "Ch%u: ECC Error flag\n", ch);
 		stats->tx_dropped++;
 	}
 	if (gerfl & RCANFD_GERFL_MES) {