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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
>
> 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
