| Message ID | 20221027082158.95895-7-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | R-Car CAN FD driver enhancements | expand |
Hi Biju, On Thu, Oct 27, 2022 at 10:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > R-Car has ECC error flags in global error interrupts whereas it is > not available on RZ/G2L. > > Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_ > global_error() will process ECC errors only for R-Car. > > whilst, this patch fixes the below checkpatch warnings > CHECK: Unnecessary parentheses around 'ch == 0' > CHECK: Unnecessary parentheses around 'ch == 1' > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/net/can/rcar/rcar_canfd.c > +++ b/drivers/net/can/rcar/rcar_canfd.c > @@ -955,13 +958,15 @@ 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"); > - stats->tx_dropped++; > + if (gpriv->info->has_gerfl_eef) { > + 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"); > + stats->tx_dropped++; > + } BTW, this fails to check the ECC error flags for channels 2-7 on R-Car V3U, which is a pre-existing problem. As that is a bug, I have sent a fix[1], which unfortunately conflicts with your patch. Sorry for that. > } > if (gerfl & RCANFD_GERFL_MES) { > sts = rcar_canfd_read(priv->base, [1] "[PATCH] can: rcar_canfd: Add missing ECC error checks for channels 2-7" https://lore.kernel.org/r/4edb2ea46cc64d0532a08a924179827481e14b4f.1666951503.git.geert+renesas@glider.be 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
Hi Geert, > Subject: Re: [PATCH v3 6/6] can: rcar_canfd: Add has_gerfl_eef to > struct rcar_canfd_hw_info > > Hi Biju, > > On Thu, Oct 27, 2022 at 10:22 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > R-Car has ECC error flags in global error interrupts whereas it is > not > > available on RZ/G2L. > > > > Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_ > > global_error() will process ECC errors only for R-Car. > > > > whilst, this patch fixes the below checkpatch warnings > > CHECK: Unnecessary parentheses around 'ch == 0' > > CHECK: Unnecessary parentheses around 'ch == 1' > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > @@ -955,13 +958,15 @@ 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"); > > - stats->tx_dropped++; > > + if (gpriv->info->has_gerfl_eef) { > > + 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"); > > + stats->tx_dropped++; > > + } > > BTW, this fails to check the ECC error flags for channels 2-7 on R-Car > V3U, which is a pre-existing problem. As that is a bug, I have sent a > fix[1], which unfortunately conflicts with your patch. Sorry for that. > > > } > > if (gerfl & RCANFD_GERFL_MES) { > > sts = rcar_canfd_read(priv->base, > > [1] "[PATCH] can: rcar_canfd: Add missing ECC error checks for > channels 2-7" OK. I will rebase with this patch. Cheers, Biju
On 28.10.2022 12:12:22, Geert Uytterhoeven wrote: > Hi Biju, > > On Thu, Oct 27, 2022 at 10:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > R-Car has ECC error flags in global error interrupts whereas it is > > not available on RZ/G2L. > > > > Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_ > > global_error() will process ECC errors only for R-Car. > > > > whilst, this patch fixes the below checkpatch warnings > > CHECK: Unnecessary parentheses around 'ch == 0' > > CHECK: Unnecessary parentheses around 'ch == 1' > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/drivers/net/can/rcar/rcar_canfd.c > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > @@ -955,13 +958,15 @@ 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"); > > - stats->tx_dropped++; > > + if (gpriv->info->has_gerfl_eef) { > > + 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"); > > + stats->tx_dropped++; > > + } > > BTW, this fails to check the ECC error flags for channels 2-7 on R-Car > V3U, which is a pre-existing problem. As that is a bug, I have sent > a fix[1], which unfortunately conflicts with your patch. Sorry for that. I'll add Geert's fix to can/main and upstream via net/main. Please re-spin this series after net/main has been merged to net-next/main. This way we'll avoid a merge conflict. Marc
Hi Marc, Thanks for the feedback. > Subject: Re: [PATCH v3 6/6] can: rcar_canfd: Add has_gerfl_eef to > struct rcar_canfd_hw_info > > On 28.10.2022 12:12:22, Geert Uytterhoeven wrote: > > Hi Biju, > > > > On Thu, Oct 27, 2022 at 10:22 AM Biju Das > <biju.das.jz@bp.renesas.com> wrote: > > > R-Car has ECC error flags in global error interrupts whereas it is > > > not available on RZ/G2L. > > > > > > Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_ > > > global_error() will process ECC errors only for R-Car. > > > > > > whilst, this patch fixes the below checkpatch warnings > > > CHECK: Unnecessary parentheses around 'ch == 0' > > > CHECK: Unnecessary parentheses around 'ch == 1' > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > --- a/drivers/net/can/rcar/rcar_canfd.c > > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > > @@ -955,13 +958,15 @@ 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"); > > > - stats->tx_dropped++; > > > + if (gpriv->info->has_gerfl_eef) { > > > + 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"); > > > + stats->tx_dropped++; > > > + } > > > > BTW, this fails to check the ECC error flags for channels 2-7 on R- > Car > > V3U, which is a pre-existing problem. As that is a bug, I have sent > a > > fix[1], which unfortunately conflicts with your patch. Sorry for > that. > > I'll add Geert's fix to can/main and upstream via net/main. Please re- > spin this series after net/main has been merged to net-next/main. > > This way we'll avoid a merge conflict. Agreed. Cheers, Biju
Hi Marc, > Subject: RE: [PATCH v3 6/6] can: rcar_canfd: Add has_gerfl_eef to > struct rcar_canfd_hw_info > > Hi Marc, > > Thanks for the feedback. > > > Subject: Re: [PATCH v3 6/6] can: rcar_canfd: Add has_gerfl_eef to > > struct rcar_canfd_hw_info > > > > On 28.10.2022 12:12:22, Geert Uytterhoeven wrote: > > > Hi Biju, > > > > > > On Thu, Oct 27, 2022 at 10:22 AM Biju Das > > <biju.das.jz@bp.renesas.com> wrote: > > > > R-Car has ECC error flags in global error interrupts whereas it > is > > > > not available on RZ/G2L. > > > > > > > > Add has_gerfl_eef to struct rcar_canfd_hw_info so that > rcar_canfd_ > > > > global_error() will process ECC errors only for R-Car. > > > > > > > > whilst, this patch fixes the below checkpatch warnings > > > > CHECK: Unnecessary parentheses around 'ch == 0' > > > > CHECK: Unnecessary parentheses around 'ch == 1' > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > --- a/drivers/net/can/rcar/rcar_canfd.c > > > > +++ b/drivers/net/can/rcar/rcar_canfd.c > > > > @@ -955,13 +958,15 @@ 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"); > > > > - stats->tx_dropped++; > > > > + if (gpriv->info->has_gerfl_eef) { > > > > + 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"); > > > > + stats->tx_dropped++; > > > > + } > > > > > > BTW, this fails to check the ECC error flags for channels 2-7 on > R- > > Car > > > V3U, which is a pre-existing problem. As that is a bug, I have > sent > > a > > > fix[1], which unfortunately conflicts with your patch. Sorry for > > that. > > > > I'll add Geert's fix to can/main and upstream via net/main. Please > re- > > spin this series after net/main has been merged to net-next/main. > > > > This way we'll avoid a merge conflict. Is it OK, if I send all other patches ie, patch#1 to patch#5 in [1] and later once net/main merged to net-next/main, will send patch#6? Please let me know. [1] https://lore.kernel.org/linux-renesas-soc/20221027082158.95895-1-biju.das.jz@bp.renesas.com/T/#t Cheers, Biju
On 31.10.2022 14:57:43, Biju Das wrote: [...] > > > This way we'll avoid a merge conflict. > > Is it OK, if I send all other patches ie, patch#1 to patch#5 in [1] and later > once net/main merged to net-next/main, will send patch#6? > > Please let me know. I picked patches 1...5 for can-next/main. regards, Marc
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index f8eafb132b39..00242eac377d 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -523,6 +523,7 @@ struct rcar_canfd_hw_info { /* hardware features */ unsigned shared_global_irqs:1; /* Has shared global irqs */ unsigned multi_channel_irqs:1; /* Has multiple channel irqs */ + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */ }; /* Channel priv data */ @@ -596,6 +597,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = { .max_channels = 2, .postdiv = 2, .shared_global_irqs = 1, + .has_gerfl_eef = 1, }; static const struct rcar_canfd_hw_info rzg2l_hw_info = { @@ -608,6 +610,7 @@ static const struct rcar_canfd_hw_info r8a779a0_hw_info = { .max_channels = 8, .postdiv = 2, .shared_global_irqs = 1, + .has_gerfl_eef = 1, }; /* Helper functions */ @@ -955,13 +958,15 @@ 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"); - stats->tx_dropped++; + if (gpriv->info->has_gerfl_eef) { + 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"); + stats->tx_dropped++; + } } if (gerfl & RCANFD_GERFL_MES) { sts = rcar_canfd_read(priv->base,
R-Car has ECC error flags in global error interrupts whereas it is not available on RZ/G2L. Add has_gerfl_eef to struct rcar_canfd_hw_info so that rcar_canfd_ global_error() will process ECC errors only for R-Car. whilst, this patch fixes the below checkpatch warnings CHECK: Unnecessary parentheses around 'ch == 0' CHECK: Unnecessary parentheses around 'ch == 1' Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v2->v3: * No change. v1->v2: * Replaced info->has_gerfl to gpriv->info->has_gerfl and wrapped the ECC error flag check within single if statement. --- drivers/net/can/rcar/rcar_canfd.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)