[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210818084312.6i5buvdp7hsu4kmy@pengutronix.de>
Date: Wed, 18 Aug 2021 10:43:12 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Vincent MAILHOL <mailhol.vincent@...adoo.fr>
Cc: linux-can <linux-can@...r.kernel.org>,
Stefan Mätje <Stefan.Maetje@....eu>,
netdev <netdev@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 5/7] can: netlink: add interface for CAN-FD
Transmitter Delay Compensation (TDC)
On 18.08.2021 17:37:17, Vincent MAILHOL wrote:
> > It's not a race. Consider this command:
> >
> > | ip up tdc-mode manual tdco 111 tdcv 33 # 111 is out of range, 33 is valid
> >
> > tdcv is checked first and valid, then it's assigned to the priv->tdc.
> > tdco is checked second and invalid, then can_tdc_changelink() returns -EINVAL.
> >
> > tdc ends up being half set :(
> >
> > So the setting of tdc is inconsistent and when you do a "ip down" "ip
> > up" then it results in a tdco=0 tdcv=33 mode=manual.
>
> My bad. Now I understand the issue.
> I was confused because tdco=111 is in the valid range of my driver...
:D
> I will squash your patch.
>
> Actually, I think that there is one more thing which needs to be
> fixed: If can_tdc_changelink() fails (e.g. value out of range),
> the CAN_CTRLMODE_TDC_AUTO or CAN_CTRLMODE_TDC_MANUAL would still
> be set, meaning that can_tdc_is_enabled() would return true. So I
> will add a "fail" branch to clear the flags.
Ok.
regard,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists