Message-ID: <a50a77c8-85ef-4ac8-b649-33b880ec4b17@hartkopp.net>
Date: Thu, 6 Nov 2025 09:42:41 +0100
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Vincent Mailhol <mailhol@...nel.org>,
 Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Stéphane Grosjean <stephane.grosjean@...-networks.com>,
 Robert Nawrath <mbro1689@...il.com>, Minh Le <minh.le.aj@...esas.com>,
 Duy Nguyen <duy.nguyen.rh@...esas.com>, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag

Hello Vincent,

On 21.10.25 17:47, Vincent Mailhol wrote:
> The Transceiver Mode Switching (TMS) indicates whether the CAN XL
> controller shall use the PWM or NRZ encoding during the data phase.
> 
> The term "transceiver mode switching" is used in both ISO 11898-1 and
> CiA 612-2 (although only the latter one uses the abbreviation TMS). We
> adopt the same naming convention here for consistency.
> 
> Add the CAN_CTRLMODE_XL_TMS flag to the list of the CAN control modes.
> 
> In the netlink interface, each boolean option is in reality a tristate
> in disguise: on, off or omitted. For the moment, TMS is implemented as
> below:
> 
>    - CAN_CTRLMODE_XL_TMS is set to false: TMS is disabled.
>    - CAN_CTRLMODE_XL_TMS is set to true: TMS is enabled.
>    - CAN_CTRLMODE_XL_TMS is omitted: return -EOPNOTSUPP.

I would propose to follow the usual pattern:

- TMS off (default)
- TMS on (when selected on the command line)

> For most of the other control modes, omitting a flag default to the
> option turned off. It could also be possible to provide a default
> behaviour if the TMS flag is omitted (i.e. either default to TMS off
> or on). However, it is not clear for the moment which default
> behaviour is preferable. If the usage shows a clear trend (for example
> if the vast majority of the users want TMS on by default), it is still
> possible to revisit that choice in the future. Whereas once a default
> option is provided, we can not change it back without breaking the
> interface.
> 
> As a corollary, for the moment, the users will be forced to specify
> the TMS in the ip tool when using CAN XL.
> 
> Add can_validate_xl_flags() to check the coherency of the TMS flag.
> That function will be reused in upcoming changes to validate the other
> CAN XL flags.
> 
> Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> ---
> Question:
> 
> Is it still possible to send Classical CAN frames when TMS is on? If
> not, we need to also add this filter in can_dev_dropped_skb():

No.

I've now learned there are two "CANXL-only" modes:

1. TMS on -> no CC/FD traffic
2. TMS off and ERR_SIG off -> no CC/FD traffic, because CC/FD require 
ERR_SIG on for a compliant transmission

And there is a "mixed-mode" with CC/FD/XL with TMS off ('ERR_SIG on' is 
default anyway).

This "mixed-mode" requires all bitrates for CC/FD/XL to be set and all 
these CAN protocols can be sent.

Currently the "mixed mode" consistency check does not insist on having 
FD on.

> 	if ((priv->ctrlmode & CAN_CTRLMODE_XL_TMS) && !can_is_canxl_skb(skb)) {
> 		netdev_info_once(dev,
> 				 "Classical CAN frames are not allowed under CAN XL's TMS mode\n");
> 		goto invalid_skb;
> 	}

Disabling CC & FD traffic with TMS on resp. ERR_SIG off is addressed in 
my patch:

[PATCH b4/canxl-netlink v2] can: drop unsupported CAN frames on socket 
and netdev level

https://lore.kernel.org/linux-can/20251103185336.32772-1-socketcan@hartkopp.net/T/#u

TMS on resp. ERR_SIG off urges FD to be off. And with this condition CC 
traffic is also disabled by that patch.

> My current assumption is that this is possible.

No.

> But the standard being
> not crystal clear on that point, I want to double check this with you!

Done ;-)

Best regards,
Oliver

> ---
>   drivers/net/can/dev/dev.c        |  2 ++
>   drivers/net/can/dev/netlink.c    | 52 +++++++++++++++++++++++++++++++++++++---
>   include/uapi/linux/can/netlink.h |  1 +
>   3 files changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 32f11db88295..1de5babcc4f3 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -123,6 +123,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>   		return "xl-tdc-auto";
>   	case CAN_CTRLMODE_XL_TDC_MANUAL:
>   		return "xl-tdc-manual";
> +	case CAN_CTRLMODE_XL_TMS:
> +		return "xl-tms";
>   	default:
>   		return "<unknown>";
>   	}
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 2405f1265488..8afd2baa03cf 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -181,6 +181,36 @@ static int can_validate_databittiming(struct nlattr *data[],
>   	return 0;
>   }
>   
> +static int can_validate_xl_flags(struct netlink_ext_ack *extack,
> +				 u32 masked_flags, u32 mask)
> +{
> +	if (masked_flags & CAN_CTRLMODE_XL) {
> +		if (!(mask & CAN_CTRLMODE_XL_TMS)) {
> +			NL_SET_ERR_MSG(extack, "Specify whether TMS is on or off");
> +			return -EOPNOTSUPP;
> +		}
> +		if (masked_flags & CAN_CTRLMODE_XL_TMS) {
> +			const u32 tms_conflicts_mask = CAN_CTRLMODE_FD |
> +				CAN_CTRLMODE_XL_TDC_MASK;
> +			u32 tms_conflicts = masked_flags & tms_conflicts_mask;
> +
> +			if (tms_conflicts) {
> +				NL_SET_ERR_MSG_FMT(extack,
> +						   "TMS and %s are mutually exclusive",
> +						   can_get_ctrlmode_str(tms_conflicts));
> +				return -EOPNOTSUPP;
> +			}
> +		}
> +	} else {
> +		if (mask & CAN_CTRLMODE_XL_TMS) {
> +			NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static int can_validate(struct nlattr *tb[], struct nlattr *data[],
>   			struct netlink_ext_ack *extack)
>   {
> @@ -201,6 +231,10 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
>   				       "Listen-only and restricted modes are mutually exclusive");
>   			return -EOPNOTSUPP;
>   		}
> +
> +		err = can_validate_xl_flags(extack, flags, cm->mask);
> +		if (err)
> +			return err;
>   	}
>   
>   	err = can_validate_bittiming(data, extack, IFLA_CAN_BITTIMING);
> @@ -227,7 +261,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   	struct can_priv *priv = netdev_priv(dev);
>   	struct can_ctrlmode *cm;
>   	const u32 xl_mandatory = CAN_CTRLMODE_RESTRICTED;
> -	u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing, xl_missing;
> +	u32 ctrlstatic, maskedflags, deactivated, notsupp, ctrlstatic_missing, xl_missing;
>   
>   	if (!data[IFLA_CAN_CTRLMODE])
>   		return 0;
> @@ -239,6 +273,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   	cm = nla_data(data[IFLA_CAN_CTRLMODE]);
>   	ctrlstatic = can_get_static_ctrlmode(priv);
>   	maskedflags = cm->flags & cm->mask;
> +	deactivated = ~cm->flags & cm->mask;
>   	notsupp = maskedflags & ~(priv->ctrlmode_supported | ctrlstatic);
>   	ctrlstatic_missing = (maskedflags & ctrlstatic) ^ ctrlstatic;
>   	xl_missing = (priv->ctrlmode_supported & xl_mandatory) ^ xl_mandatory;
> @@ -268,11 +303,21 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	/* If FD was active and is not turned off, check for XL conflicts */
> +	if (priv->ctrlmode & CAN_CTRLMODE_FD & ~deactivated) {
> +		if (maskedflags & CAN_CTRLMODE_XL_TMS) {
> +			NL_SET_ERR_MSG(extack,
> +				       "TMS can not be activated while CAN FD is on");
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
>   	/* If a top dependency flag is provided, reset all its dependencies */
>   	if (cm->mask & CAN_CTRLMODE_FD)
>   		priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
>   	if (cm->mask & CAN_CTRLMODE_XL)
> -		priv->ctrlmode &= ~(CAN_CTRLMODE_XL_TDC_MASK);
> +		priv->ctrlmode &= ~(CAN_CTRLMODE_XL_TDC_MASK |
> +				    CAN_CTRLMODE_XL_TMS);
>   
>   	/* clear bits to be modified and copy the flag values */
>   	priv->ctrlmode &= ~cm->mask;
> @@ -404,7 +449,8 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
>   	if (data[IFLA_CAN_CTRLMODE]) {
>   		struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]);
>   
> -		need_tdc_calc = !(cm->mask & tdc_mask);
> +		if (fd || !(priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
> +			need_tdc_calc = !(cm->mask & tdc_mask);
>   	}
>   	if (data_tdc) {
>   		/* TDC parameters are provided: use them */
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index c2c96c5978a8..ebafb091d80f 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -107,6 +107,7 @@ struct can_ctrlmode {
>   #define CAN_CTRLMODE_XL			0x1000	/* CAN XL mode */
>   #define CAN_CTRLMODE_XL_TDC_AUTO	0x2000	/* XL transceiver automatically calculates TDCV */
>   #define CAN_CTRLMODE_XL_TDC_MANUAL	0x4000	/* XL TDCV is manually set up by user */
> +#define CAN_CTRLMODE_XL_TMS		0x8000	/* Transceiver Mode Switching */
>   
>   /*
>    * CAN device statistics
> 

Powered by blists - more mailing lists