| Message ID | 20221022104357.1276740-7-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | R-Can CAN FD driver enhancements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
Hi Biju, On Sat, Oct 22, 2022 at 1:03 PM 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> Thanks for your patch! > --- 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 { > unsigned multi_global_irqs:1; /* Has multiple global irqs */ > unsigned clk_postdiv:1; /* Has CAN clk post divider */ > unsigned multi_channel_irqs:1; /* Has multiple channel irqs */ > + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */ Do you really need this flag? According to the RZ/G2L docs, the corresponding register bits are always read as zero. > }; > > /* Channel priv data */ > @@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct net_device *ndev) > { > struct rcar_canfd_channel *priv = netdev_priv(ndev); > struct rcar_canfd_global *gpriv = priv->gpriv; > + const struct rcar_canfd_hw_info *info = gpriv->info; > struct net_device_stats *stats = &ndev->stats; > u32 ch = priv->channel; > u32 gerfl, sts; > u32 ridx = ch + RCANFD_RFFIFO_IDX; > > gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL); > - if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) { > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch == 0) { > netdev_dbg(ndev, "Ch0: ECC Error flag\n"); > stats->tx_dropped++; > } > - if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) { > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch == 1) { > netdev_dbg(ndev, "Ch1: ECC Error flag\n"); > stats->tx_dropped++; > } Just wrap both checks inside a single "if (gpriv->info->has_gerfl) { ... }"? 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, Thanks for the feedback. > Subject: Re: [PATCH 6/6] can: rcar_canfd: Add has_gerfl_eef to struct > rcar_canfd_hw_info > > Hi Biju, > > On Sat, Oct 22, 2022 at 1:03 PM 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> > > Thanks for your patch! > > > --- 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 { > > unsigned multi_global_irqs:1; /* Has multiple global irqs > */ > > unsigned clk_postdiv:1; /* Has CAN clk post divider > */ > > unsigned multi_channel_irqs:1; /* Has multiple channel irqs > > */ > > + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */ > > Do you really need this flag? According to the RZ/G2L docs, the > corresponding register bits are always read as zero. These are reserved bits with 0 value, But it is not documented as ECC Error flag for RZ/G2L. So just want to be clear it is a hw feature that not supported for RZ/G2L. > > > }; > > > > /* Channel priv data */ > > > @@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct > > net_device *ndev) { > > struct rcar_canfd_channel *priv = netdev_priv(ndev); > > struct rcar_canfd_global *gpriv = priv->gpriv; > > + const struct rcar_canfd_hw_info *info = gpriv->info; > > struct net_device_stats *stats = &ndev->stats; > > u32 ch = priv->channel; > > u32 gerfl, sts; > > u32 ridx = ch + RCANFD_RFFIFO_IDX; > > > > gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL); > > - if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) { > > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch > > + == 0) { > > netdev_dbg(ndev, "Ch0: ECC Error flag\n"); > > stats->tx_dropped++; > > } > > - if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) { > > + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch > > + == 1) { > > netdev_dbg(ndev, "Ch1: ECC Error flag\n"); > > stats->tx_dropped++; > > } > > Just wrap both checks inside a single "if (gpriv->info->has_gerfl) { > ... }"? > OK, cheers, Biju
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index 0b6f14df2a43..bb825cce8acb 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 { unsigned multi_global_irqs:1; /* Has multiple global irqs */ unsigned clk_postdiv:1; /* Has CAN clk post divider */ unsigned multi_channel_irqs:1; /* Has multiple channel irqs */ + unsigned has_gerfl_eef:1; /* Has ECC Error Flag */ }; /* Channel priv data */ @@ -595,6 +596,7 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = { static const struct rcar_canfd_hw_info rcar_gen3_hw_info = { .max_channels = 2, .clk_postdiv = 1, + .has_gerfl_eef = 1, }; static const struct rcar_canfd_hw_info rzg2l_hw_info = { @@ -606,6 +608,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = { static const struct rcar_canfd_hw_info r8a779a0_hw_info = { .max_channels = 8, .clk_postdiv = 1, + .has_gerfl_eef = 1, }; /* Helper functions */ @@ -947,17 +950,18 @@ static void rcar_canfd_global_error(struct net_device *ndev) { struct rcar_canfd_channel *priv = netdev_priv(ndev); struct rcar_canfd_global *gpriv = priv->gpriv; + const struct rcar_canfd_hw_info *info = gpriv->info; struct net_device_stats *stats = &ndev->stats; u32 ch = priv->channel; u32 gerfl, sts; u32 ridx = ch + RCANFD_RFFIFO_IDX; gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL); - if ((gerfl & RCANFD_GERFL_EEF0) && (ch == 0)) { + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF0) && ch == 0) { netdev_dbg(ndev, "Ch0: ECC Error flag\n"); stats->tx_dropped++; } - if ((gerfl & RCANFD_GERFL_EEF1) && (ch == 1)) { + if (info->has_gerfl_eef && (gerfl & RCANFD_GERFL_EEF1) && ch == 1) { netdev_dbg(ndev, "Ch1: ECC Error flag\n"); stats->tx_dropped++; }
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> --- drivers/net/can/rcar/rcar_canfd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)