Re: [PATCH v5 5/7] can: netlink: add interface for CAN-FD Transmitter Delay Compensation (TDC)

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

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 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]