netdev - RE: [PATCH linux-can-next/flexcan] can: flexcan: fix TDC feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 16 Sep 2020 02:26:52 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        Michael Walle <michael@...le.cc>
CC:     "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH linux-can-next/flexcan] can: flexcan: fix TDC feature


> -----Original Message-----
> From: Marc Kleine-Budde <mkl@...gutronix.de>
> Sent: 2020年9月16日 6:16
> To: Joakim Zhang <qiangqing.zhang@....com>; Michael Walle
> <michael@...le.cc>
> Cc: linux-can@...r.kernel.org; dl-linux-imx <linux-imx@....com>;
> netdev@...r.kernel.org
> Subject: Re: [PATCH linux-can-next/flexcan] can: flexcan: fix TDC feature
> 
> On 6/30/20 4:25 AM, Joakim Zhang wrote:
> > I have also noticed this difference, although this could not break
> > function, but IMO, using priv->can.ctrlmode should be better.
> >
[...]

> > 2) Clean timing register.
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -1167,6 +1167,14 @@ static void flexcan_set_bittiming_cbt(const
> struct net_device *dev)
> >         struct flexcan_regs __iomem *regs = priv->regs;
> >         u32 reg_cbt, reg_fdctrl;
> >
> > +       reg_cbt = priv->read(&regs->cbt);
> > +       reg_cbt &= ~(FLEXCAN_CBT_BTF |
> > +               FIELD_PREP(FLEXCAN_CBT_EPRESDIV_MASK, 0x3ff) |
> > +               FIELD_PREP(FLEXCAN_CBT_ERJW_MASK, 0x1f) |
> > +               FIELD_PREP(FLEXCAN_CBT_EPROPSEG_MASK, 0x3f) |
> > +               FIELD_PREP(FLEXCAN_CBT_EPSEG1_MASK, 0x1f) |
> > +               FIELD_PREP(FLEXCAN_CBT_EPSEG2_MASK, 0x1f));
> > +
> 
> Why is this needed? The "reg_cbt &=" sets reg_cbt basically to 0, as the fields
> and the BTF occupy all 32bit.
> 
> The only thing that's left over is the read()....

Yes, need not, I have not noticed it has occupy the whole 32bit.

There is a small improve patch to balance the usage_count if register flexcandev failed. Could you pick up it by the way this time?
https://www.spinics.net/lists/linux-can/msg03052.html


Best Regards,
Joakim Zhang