[PATCH 3/9] can: netlink: add initial CAN XL support

Vincent Mailhol posted 9 patches 3 months, 4 weeks ago
There is a newer version of this series
Re: [PATCH 3/9] can: netlink: add initial CAN XL support
Posted by Oliver Hartkopp 2 months, 3 weeks ago
Hi Vincent,

On 13.10.25 13:01, Vincent Mailhol wrote:
> CAN XL uses bittiming parameters different from Classical CAN and CAN
> FD. Thus, all the data bittiming parameters, including TDC, need to be
> duplicated for CAN XL.
> 
> Add the CAN XL netlink interface for all the features which are common
> with CAN FD. Any new CAN XL specific features are added later on.
> 
> Add a check that CAN XL capable nodes correctly provide
> CAN_CTRLMODE_RESTRIC_OP as mandated by ISO 11898-1:2024 §6.6.19.
> 
> The first time CAN XL is activated, the MTU is set by default to
> CANXL_MAX_MTU. The user may then configure a custom MTU within the
> CANXL_MIN_MTU to CANXL_MIN_MTU range, in which case, the custom MTU
> value will be kept as long as CAN XL remains active.
> 
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> Changelog:
> 
> RFC -> v1:
> 
>    - Correctly wipe out the CAN XL data bittiming and TDC parameters
>      when switching CAN_CTRLMODE_XL off.
> 
>    - Add one level on nesting for xl parameters so that:
> 
>       - bittiming are under priv->xl.data_bittiming{,_const}¨
>       - pwm are under priv->xl.pwm{,_const}
> 
>    - Many other code refactors.
> ---
>   drivers/net/can/dev/dev.c        | 14 ++++++-
>   drivers/net/can/dev/netlink.c    | 87 ++++++++++++++++++++++++++++++++--------
>   include/linux/can/bittiming.h    |  6 ++-
>   include/linux/can/dev.h          |  7 +++-
>   include/uapi/linux/can/netlink.h |  7 ++++
>   5 files changed, 100 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 3377afb6f1c4..32f11db88295 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -117,6 +117,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>   		return "fd-tdc-manual";
>   	case CAN_CTRLMODE_RESTRICTED:
>   		return "restricted-operation";
> +	case CAN_CTRLMODE_XL:
> +		return "xl";
> +	case CAN_CTRLMODE_XL_TDC_AUTO:
> +		return "xl-tdc-auto";
> +	case CAN_CTRLMODE_XL_TDC_MANUAL:
> +		return "xl-tdc-manual";
>   	default:
>   		return "<unknown>";
>   	}
> @@ -350,7 +356,13 @@ void can_set_default_mtu(struct net_device *dev)
>   {
>   	struct can_priv *priv = netdev_priv(dev);
>   
> -	if (priv->ctrlmode & CAN_CTRLMODE_FD) {
> +	if (priv->ctrlmode & CAN_CTRLMODE_XL) {
> +		if (can_is_canxl_dev_mtu(dev->mtu))
> +			return;
> +		dev->mtu = CANXL_MTU;
> +		dev->min_mtu = CANXL_MIN_MTU;
> +		dev->max_mtu = CANXL_MAX_MTU;
> +	} else if (priv->ctrlmode & CAN_CTRLMODE_FD) {
>   		dev->mtu = CANFD_MTU;
>   		dev->min_mtu = CANFD_MTU;
>   		dev->max_mtu = CANFD_MTU;
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index f44b5dffa176..2405f1265488 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -2,7 +2,7 @@
>   /* Copyright (C) 2005 Marc Kleine-Budde, Pengutronix
>    * Copyright (C) 2006 Andrey Volkov, Varma Electronics
>    * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
> - * Copyright (C) 2021 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> + * Copyright (C) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
>    */
>   
>   #include <linux/can/dev.h>
> @@ -22,6 +22,9 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
>   	[IFLA_CAN_TERMINATION] = { .type = NLA_U16 },
>   	[IFLA_CAN_TDC] = { .type = NLA_NESTED },
>   	[IFLA_CAN_CTRLMODE_EXT] = { .type = NLA_NESTED },
> +	[IFLA_CAN_XL_DATA_BITTIMING] = { .len = sizeof(struct can_bittiming) },
> +	[IFLA_CAN_XL_DATA_BITTIMING_CONST] = { .len = sizeof(struct can_bittiming_const) },
> +	[IFLA_CAN_XL_TDC] = { .type = NLA_NESTED },
>   };
>   
>   static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
> @@ -70,7 +73,7 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	/* If one of the CAN_CTRLMODE_TDC_* flag is set then TDC
> +	/* If one of the CAN_CTRLMODE_{,XL}_TDC_* flags is set then TDC
>   	 * must be set and vice-versa
>   	 */
>   	if ((tdc_auto || tdc_manual) && !data_tdc) {
> @@ -82,8 +85,8 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	/* If providing TDC parameters, at least TDCO is needed. TDCV
> -	 * is needed if and only if CAN_CTRLMODE_TDC_MANUAL is set
> +	/* If providing TDC parameters, at least TDCO is needed. TDCV is
> +	 * needed if and only if CAN_CTRLMODE_{,XL}_TDC_MANUAL is set
>   	 */
>   	if (data_tdc) {
>   		struct nlattr *tb_tdc[IFLA_CAN_TDC_MAX + 1];
> @@ -126,10 +129,10 @@ static int can_validate_databittiming(struct nlattr *data[],
>   	bool is_on;
>   	int err;
>   
> -	/* Make sure that valid CAN FD configurations always consist of
> +	/* Make sure that valid CAN FD/XL configurations always consist of
>   	 * - nominal/arbitration bittiming
>   	 * - data bittiming
> -	 * - control mode with CAN_CTRLMODE_FD set
> +	 * - control mode with CAN_CTRLMODE_{FD,XL} set
>   	 * - TDC parameters are coherent (details in can_validate_tdc())
>   	 */
>   
> @@ -139,7 +142,10 @@ static int can_validate_databittiming(struct nlattr *data[],
>   		is_on = flags & CAN_CTRLMODE_FD;
>   		type = "FD";
>   	} else {
> -		return -EOPNOTSUPP; /* Place holder for CAN XL */
> +		data_tdc = data[IFLA_CAN_XL_TDC];
> +		tdc_flags = flags & CAN_CTRLMODE_XL_TDC_MASK;
> +		is_on = flags & CAN_CTRLMODE_XL;
> +		type = "XL";
>   	}
>   
>   	if (is_on) {
> @@ -206,6 +212,11 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
>   	if (err)
>   		return err;
>   
> +	err = can_validate_databittiming(data, extack,
> +					 IFLA_CAN_XL_DATA_BITTIMING, flags);
> +	if (err)
> +		return err;
> +
>   	return 0;
>   }
>   
> @@ -215,7 +226,8 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   {
>   	struct can_priv *priv = netdev_priv(dev);
>   	struct can_ctrlmode *cm;
> -	u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
> +	const u32 xl_mandatory = CAN_CTRLMODE_RESTRICTED;
> +	u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing, xl_missing;
>   
>   	if (!data[IFLA_CAN_CTRLMODE])
>   		return 0;
> @@ -229,6 +241,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   	maskedflags = cm->flags & cm->mask;
>   	notsupp = maskedflags & ~(priv->ctrlmode_supported | ctrlstatic);
>   	ctrlstatic_missing = (maskedflags & ctrlstatic) ^ ctrlstatic;
> +	xl_missing = (priv->ctrlmode_supported & xl_mandatory) ^ xl_mandatory;
>   
>   	if (notsupp) {
>   		NL_SET_ERR_MSG_FMT(extack,
> @@ -248,21 +261,36 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	if ((priv->ctrlmode_supported & CAN_CTRLMODE_XL) && xl_missing) {
> +		NL_SET_ERR_MSG_FMT(extack,
> +				   "bad device: CAN XL capable nodes must support the %s mode",
> +				   can_get_ctrlmode_str(xl_missing));
> +		return -EOPNOTSUPP;
> +	}
> +

I'm not sure if it is our job to check for ISO 11898-1:2024 compliance 
of CAN controllers when CAN_CTRLMODE_RESTRICTED is not supported.

IMO an appropriate error message (only) when the user requests this 
feature would be better and that is already the standard behaviour.

Best regards,
Oliver

>   	/* 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);
>   
>   	/* clear bits to be modified and copy the flag values */
>   	priv->ctrlmode &= ~cm->mask;
>   	priv->ctrlmode |= maskedflags;
>   
> -	/* Wipe potential leftovers from previous CAN FD config */
> +	/* Wipe potential leftovers from previous CAN FD/XL config */
>   	if (!(priv->ctrlmode & CAN_CTRLMODE_FD)) {
>   		memset(&priv->fd.data_bittiming, 0,
>   		       sizeof(priv->fd.data_bittiming));
>   		priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
>   		memset(&priv->fd.tdc, 0, sizeof(priv->fd.tdc));
>   	}
> +	if (!(priv->ctrlmode & CAN_CTRLMODE_XL)) {
> +		memset(&priv->xl.data_bittiming, 0,
> +		       sizeof(priv->fd.data_bittiming));
> +		priv->ctrlmode &= ~CAN_CTRLMODE_XL_TDC_MASK;
> +		memset(&priv->xl.tdc, 0, sizeof(priv->xl.tdc));
> +	}
>   
>   	can_set_default_mtu(dev);
>   
> @@ -337,7 +365,10 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
>   		dbt_params = &priv->fd;
>   		tdc_mask = CAN_CTRLMODE_FD_TDC_MASK;
>   	} else {
> -		return -EOPNOTSUPP; /* Place holder for CAN XL */
> +		data_bittiming = data[IFLA_CAN_XL_DATA_BITTIMING];
> +		data_tdc = data[IFLA_CAN_XL_TDC];
> +		dbt_params = &priv->xl;
> +		tdc_mask = CAN_CTRLMODE_XL_TDC_MASK;
>   	}
>   
>   	if (!data_bittiming)
> @@ -388,7 +419,7 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
>   		 */
>   		can_calc_tdco(&dbt_params->tdc, dbt_params->tdc_const, &dbt,
>   			      tdc_mask, &priv->ctrlmode, priv->ctrlmode_supported);
> -	} /* else: both CAN_CTRLMODE_TDC_{AUTO,MANUAL} are explicitly
> +	} /* else: both CAN_CTRLMODE_{,XL}_TDC_{AUTO,MANUAL} are explicitly
>   	   * turned off. TDC is disabled: do nothing
>   	   */
>   
> @@ -491,6 +522,11 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
>   	if (err)
>   		return err;
>   
> +	/* CAN XL */
> +	err = can_dbt_changelink(dev, data, false, extack);
> +	if (err)
> +		return err;
> +
>   	if (data[IFLA_CAN_TERMINATION]) {
>   		const u16 termval = nla_get_u16(data[IFLA_CAN_TERMINATION]);
>   		const unsigned int num_term = priv->termination_const_cnt;
> @@ -558,14 +594,14 @@ static size_t can_data_bittiming_get_size(struct data_bittiming_params *dbt_para
>   {
>   	size_t size = 0;
>   
> -	if (dbt_params->data_bittiming.bitrate)		/* IFLA_CAN_DATA_BITTIMING */
> +	if (dbt_params->data_bittiming.bitrate)		/* IFLA_CAN_{,XL}_DATA_BITTIMING */
>   		size += nla_total_size(sizeof(dbt_params->data_bittiming));
> -	if (dbt_params->data_bittiming_const)		/* IFLA_CAN_DATA_BITTIMING_CONST */
> +	if (dbt_params->data_bittiming_const)		/* IFLA_CAN_{,XL}_DATA_BITTIMING_CONST */
>   		size += nla_total_size(sizeof(*dbt_params->data_bittiming_const));
> -	if (dbt_params->data_bitrate_const)		/* IFLA_CAN_DATA_BITRATE_CONST */
> +	if (dbt_params->data_bitrate_const)		/* IFLA_CAN_{,XL}_DATA_BITRATE_CONST */
>   		size += nla_total_size(sizeof(*dbt_params->data_bitrate_const) *
>   				       dbt_params->data_bitrate_const_cnt);
> -	size += can_tdc_get_size(dbt_params, tdc_flags);/* IFLA_CAN_TDC */
> +	size += can_tdc_get_size(dbt_params, tdc_flags);/* IFLA_CAN_{,XL}_TDC */
>   
>   	return size;
>   }
> @@ -605,6 +641,9 @@ static size_t can_get_size(const struct net_device *dev)
>   	size += can_data_bittiming_get_size(&priv->fd,
>   					    priv->ctrlmode & CAN_CTRLMODE_FD_TDC_MASK);
>   
> +	size += can_data_bittiming_get_size(&priv->xl,
> +					    priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK);
> +
>   	return size;
>   }
>   
> @@ -649,7 +688,9 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev,
>   		tdc_is_enabled = can_fd_tdc_is_enabled(priv);
>   		tdc_manual = priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL;
>   	} else {
> -		return -EOPNOTSUPP; /* Place holder for CAN XL */
> +		dbt_params = &priv->xl;
> +		tdc_is_enabled = can_xl_tdc_is_enabled(priv);
> +		tdc_manual = priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MANUAL;
>   	}
>   	tdc_const = dbt_params->tdc_const;
>   	tdc = &dbt_params->tdc;
> @@ -771,7 +812,19 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
>   
>   	    can_tdc_fill_info(skb, dev, IFLA_CAN_TDC) ||
>   
> -	    can_ctrlmode_ext_fill_info(skb, priv)
> +	    can_ctrlmode_ext_fill_info(skb, priv) ||
> +
> +	    can_bittiming_fill_info(skb, IFLA_CAN_XL_DATA_BITTIMING,
> +				    &priv->xl.data_bittiming) ||
> +
> +	    can_bittiming_const_fill_info(skb, IFLA_CAN_XL_DATA_BITTIMING_CONST,
> +					  priv->xl.data_bittiming_const) ||
> +
> +	    can_bitrate_const_fill_info(skb, IFLA_CAN_XL_DATA_BITRATE_CONST,
> +					priv->xl.data_bitrate_const,
> +					priv->xl.data_bitrate_const_cnt) ||
> +
> +	    can_tdc_fill_info(skb, dev, IFLA_CAN_XL_TDC)
>   	    )
>   
>   		return -EMSGSIZE;
> diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
> index d30816dd93c7..68d533bb7856 100644
> --- a/include/linux/can/bittiming.h
> +++ b/include/linux/can/bittiming.h
> @@ -16,10 +16,12 @@
>   
>   #define CAN_CTRLMODE_FD_TDC_MASK				\
>   	(CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_TDC_MANUAL)
> +#define CAN_CTRLMODE_XL_TDC_MASK				\
> +	(CAN_CTRLMODE_XL_TDC_AUTO | CAN_CTRLMODE_XL_TDC_MANUAL)
>   #define CAN_CTRLMODE_TDC_AUTO_MASK				\
> -	(CAN_CTRLMODE_TDC_AUTO)
> +	(CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_XL_TDC_AUTO)
>   #define CAN_CTRLMODE_TDC_MANUAL_MASK				\
> -	(CAN_CTRLMODE_TDC_MANUAL)
> +	(CAN_CTRLMODE_TDC_MANUAL | CAN_CTRLMODE_XL_TDC_MANUAL)
>   
>   /*
>    * struct can_tdc - CAN FD Transmission Delay Compensation parameters
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> index 9de8fde3ec9d..945c16743702 100644
> --- a/include/linux/can/dev.h
> +++ b/include/linux/can/dev.h
> @@ -47,7 +47,7 @@ struct can_priv {
>   
>   	const struct can_bittiming_const *bittiming_const;
>   	struct can_bittiming bittiming;
> -	struct data_bittiming_params fd;
> +	struct data_bittiming_params fd, xl;
>   	unsigned int bitrate_const_cnt;
>   	const u32 *bitrate_const;
>   	u32 bitrate_max;
> @@ -85,6 +85,11 @@ static inline bool can_fd_tdc_is_enabled(const struct can_priv *priv)
>   	return !!(priv->ctrlmode & CAN_CTRLMODE_FD_TDC_MASK);
>   }
>   
> +static inline bool can_xl_tdc_is_enabled(const struct can_priv *priv)
> +{
> +	return !!(priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK);
> +}
> +
>   static inline u32 can_get_static_ctrlmode(struct can_priv *priv)
>   {
>   	return priv->ctrlmode & ~priv->ctrlmode_supported;
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index fafd1cce4798..c2c96c5978a8 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -104,6 +104,9 @@ struct can_ctrlmode {
>   #define CAN_CTRLMODE_TDC_AUTO		0x200	/* FD transceiver automatically calculates TDCV */
>   #define CAN_CTRLMODE_TDC_MANUAL		0x400	/* FD TDCV is manually set up by user */
>   #define CAN_CTRLMODE_RESTRICTED		0x800	/* Restricted operation mode */
> +#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 */
>   
>   /*
>    * CAN device statistics
> @@ -139,6 +142,10 @@ enum {
>   	IFLA_CAN_BITRATE_MAX,
>   	IFLA_CAN_TDC, /* FD */
>   	IFLA_CAN_CTRLMODE_EXT,
> +	IFLA_CAN_XL_DATA_BITTIMING,
> +	IFLA_CAN_XL_DATA_BITTIMING_CONST,
> +	IFLA_CAN_XL_DATA_BITRATE_CONST,
> +	IFLA_CAN_XL_TDC,
>   
>   	/* add new constants above here */
>   	__IFLA_CAN_MAX,
> 

Re: [PATCH 3/9] can: netlink: add initial CAN XL support
Posted by Vincent Mailhol 2 months, 3 weeks ago
On 14/11/2025 at 14:19, Oliver Hartkopp wrote:
> Hi Vincent,
> 
> On 13.10.25 13:01, Vincent Mailhol wrote:
>> CAN XL uses bittiming parameters different from Classical CAN and CAN
>> FD. Thus, all the data bittiming parameters, including TDC, need to be
>> duplicated for CAN XL.
>>
>> Add the CAN XL netlink interface for all the features which are common
>> with CAN FD. Any new CAN XL specific features are added later on.
>>
>> Add a check that CAN XL capable nodes correctly provide
>> CAN_CTRLMODE_RESTRIC_OP as mandated by ISO 11898-1:2024 §6.6.19.
>>
>> The first time CAN XL is activated, the MTU is set by default to
>> CANXL_MAX_MTU. The user may then configure a custom MTU within the
>> CANXL_MIN_MTU to CANXL_MIN_MTU range, in which case, the custom MTU
>> value will be kept as long as CAN XL remains active.
>>
>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>> ---
>> Changelog:
>>
>> RFC -> v1:
>>
>>    - Correctly wipe out the CAN XL data bittiming and TDC parameters
>>      when switching CAN_CTRLMODE_XL off.
>>
>>    - Add one level on nesting for xl parameters so that:
>>
>>       - bittiming are under priv->xl.data_bittiming{,_const}¨
>>       - pwm are under priv->xl.pwm{,_const}
>>
>>    - Many other code refactors.
>> ---
>>   drivers/net/can/dev/dev.c        | 14 ++++++-
>>   drivers/net/can/dev/netlink.c    | 87 ++++++++++++++++++++++++++++++++--------
>>   include/linux/can/bittiming.h    |  6 ++-
>>   include/linux/can/dev.h          |  7 +++-
>>   include/uapi/linux/can/netlink.h |  7 ++++
>>   5 files changed, 100 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>> index 3377afb6f1c4..32f11db88295 100644
>> --- a/drivers/net/can/dev/dev.c
>> +++ b/drivers/net/can/dev/dev.c
>> @@ -117,6 +117,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>>           return "fd-tdc-manual";
>>       case CAN_CTRLMODE_RESTRICTED:
>>           return "restricted-operation";
>> +    case CAN_CTRLMODE_XL:
>> +        return "xl";
>> +    case CAN_CTRLMODE_XL_TDC_AUTO:
>> +        return "xl-tdc-auto";
>> +    case CAN_CTRLMODE_XL_TDC_MANUAL:
>> +        return "xl-tdc-manual";
>>       default:
>>           return "<unknown>";
>>       }
>> @@ -350,7 +356,13 @@ void can_set_default_mtu(struct net_device *dev)
>>   {
>>       struct can_priv *priv = netdev_priv(dev);
>>   -    if (priv->ctrlmode & CAN_CTRLMODE_FD) {
>> +    if (priv->ctrlmode & CAN_CTRLMODE_XL) {
>> +        if (can_is_canxl_dev_mtu(dev->mtu))
>> +            return;
>> +        dev->mtu = CANXL_MTU;
>> +        dev->min_mtu = CANXL_MIN_MTU;
>> +        dev->max_mtu = CANXL_MAX_MTU;
>> +    } else if (priv->ctrlmode & CAN_CTRLMODE_FD) {
>>           dev->mtu = CANFD_MTU;
>>           dev->min_mtu = CANFD_MTU;
>>           dev->max_mtu = CANFD_MTU;
>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>> index f44b5dffa176..2405f1265488 100644
>> --- a/drivers/net/can/dev/netlink.c
>> +++ b/drivers/net/can/dev/netlink.c
>> @@ -2,7 +2,7 @@
>>   /* Copyright (C) 2005 Marc Kleine-Budde, Pengutronix
>>    * Copyright (C) 2006 Andrey Volkov, Varma Electronics
>>    * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
>> - * Copyright (C) 2021 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>> + * Copyright (C) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
>>    */
>>     #include <linux/can/dev.h>
>> @@ -22,6 +22,9 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
>>       [IFLA_CAN_TERMINATION] = { .type = NLA_U16 },
>>       [IFLA_CAN_TDC] = { .type = NLA_NESTED },
>>       [IFLA_CAN_CTRLMODE_EXT] = { .type = NLA_NESTED },
>> +    [IFLA_CAN_XL_DATA_BITTIMING] = { .len = sizeof(struct can_bittiming) },
>> +    [IFLA_CAN_XL_DATA_BITTIMING_CONST] = { .len = sizeof(struct
>> can_bittiming_const) },
>> +    [IFLA_CAN_XL_TDC] = { .type = NLA_NESTED },
>>   };
>>     static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
>> @@ -70,7 +73,7 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>>           return -EOPNOTSUPP;
>>       }
>>   -    /* If one of the CAN_CTRLMODE_TDC_* flag is set then TDC
>> +    /* If one of the CAN_CTRLMODE_{,XL}_TDC_* flags is set then TDC
>>        * must be set and vice-versa
>>        */
>>       if ((tdc_auto || tdc_manual) && !data_tdc) {
>> @@ -82,8 +85,8 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>>           return -EOPNOTSUPP;
>>       }
>>   -    /* If providing TDC parameters, at least TDCO is needed. TDCV
>> -     * is needed if and only if CAN_CTRLMODE_TDC_MANUAL is set
>> +    /* If providing TDC parameters, at least TDCO is needed. TDCV is
>> +     * needed if and only if CAN_CTRLMODE_{,XL}_TDC_MANUAL is set
>>        */
>>       if (data_tdc) {
>>           struct nlattr *tb_tdc[IFLA_CAN_TDC_MAX + 1];
>> @@ -126,10 +129,10 @@ static int can_validate_databittiming(struct nlattr
>> *data[],
>>       bool is_on;
>>       int err;
>>   -    /* Make sure that valid CAN FD configurations always consist of
>> +    /* Make sure that valid CAN FD/XL configurations always consist of
>>        * - nominal/arbitration bittiming
>>        * - data bittiming
>> -     * - control mode with CAN_CTRLMODE_FD set
>> +     * - control mode with CAN_CTRLMODE_{FD,XL} set
>>        * - TDC parameters are coherent (details in can_validate_tdc())
>>        */
>>   @@ -139,7 +142,10 @@ static int can_validate_databittiming(struct nlattr
>> *data[],
>>           is_on = flags & CAN_CTRLMODE_FD;
>>           type = "FD";
>>       } else {
>> -        return -EOPNOTSUPP; /* Place holder for CAN XL */
>> +        data_tdc = data[IFLA_CAN_XL_TDC];
>> +        tdc_flags = flags & CAN_CTRLMODE_XL_TDC_MASK;
>> +        is_on = flags & CAN_CTRLMODE_XL;
>> +        type = "XL";
>>       }
>>         if (is_on) {
>> @@ -206,6 +212,11 @@ static int can_validate(struct nlattr *tb[], struct
>> nlattr *data[],
>>       if (err)
>>           return err;
>>   +    err = can_validate_databittiming(data, extack,
>> +                     IFLA_CAN_XL_DATA_BITTIMING, flags);
>> +    if (err)
>> +        return err;
>> +
>>       return 0;
>>   }
>>   @@ -215,7 +226,8 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>   {
>>       struct can_priv *priv = netdev_priv(dev);
>>       struct can_ctrlmode *cm;
>> -    u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
>> +    const u32 xl_mandatory = CAN_CTRLMODE_RESTRICTED;
>> +    u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing, xl_missing;
>>         if (!data[IFLA_CAN_CTRLMODE])
>>           return 0;
>> @@ -229,6 +241,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>       maskedflags = cm->flags & cm->mask;
>>       notsupp = maskedflags & ~(priv->ctrlmode_supported | ctrlstatic);
>>       ctrlstatic_missing = (maskedflags & ctrlstatic) ^ ctrlstatic;
>> +    xl_missing = (priv->ctrlmode_supported & xl_mandatory) ^ xl_mandatory;
>>         if (notsupp) {
>>           NL_SET_ERR_MSG_FMT(extack,
>> @@ -248,21 +261,36 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>           return -EOPNOTSUPP;
>>       }
>>   +    if ((priv->ctrlmode_supported & CAN_CTRLMODE_XL) && xl_missing) {
>> +        NL_SET_ERR_MSG_FMT(extack,
>> +                   "bad device: CAN XL capable nodes must support the %s mode",
>> +                   can_get_ctrlmode_str(xl_missing));
>> +        return -EOPNOTSUPP;
>> +    }
>> +
> 
> I'm not sure if it is our job to check for ISO 11898-1:2024 compliance of CAN
> controllers when CAN_CTRLMODE_RESTRICTED is not supported.
> 
> IMO an appropriate error message (only) when the user requests this feature
> would be better and that is already the standard behaviour.

For this one, the initial logic was to avoid some incorrect configurations such
as providing CAN_CTRLMODE_XL_TMS without CAN_CTRLMODE_XL_ERR_SIGNAL.

Contrarily to the other flags, CAN XL introduces some intricacies between some
of the flags and giving full freedom on what must be set or not is asking for
troubles.

That said, the CAN_CTRLMODE_RESTRICTED is different and does not depend on the
other flags. I just added it at the end after adding the check logic for the
other XL flags (this appears first in the series because of some rebase, but
trust me, I implemented this last).

Then, should we go for this or not? The benefit I see to add this check would be
less effort on the patch reviews for new devices (which I am mostly doing
recently). I can foreseen that people will wrongly use CAN_CTRLMODE_LISTENONLY
instead of CAN_CTRLMODE_RESTRICTED. This piece of code will make sure that they
will implement CAN_CTRLMODE_RESTRICTED. And because this feature is mandatory, I
thought this wasn't a too bad idea. This will also save me the effort to ask if
to people submitting patches if this feature is supported or not.

If someone comes with a non-compliant device, we can still statically set the
flag. So this isn't even a blocker for non compliant devices.

So, at the end, this is mostly a selfish feature to remove some patch reviewing
effort (while still pushing toward ISO compliance, which isn't a bad thing either).

I prefer to have it like this, but this isn't a strong opinion either.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 3/9] can: netlink: add initial CAN XL support
Posted by Stéphane Grosjean 2 months, 3 weeks ago
Hi Vincent,

> Then, should we go for this or not? The benefit I see to add this
> check would be
> less effort on the patch reviews for new devices (which I am mostly
> doing
> recently). I can foreseen that people will wrongly use
> CAN_CTRLMODE_LISTENONLY
> instead of CAN_CTRLMODE_RESTRICTED. This piece of code will make sure
> that they
> will implement CAN_CTRLMODE_RESTRICTED. And because this feature is
> mandatory, I
> thought this wasn't a too bad idea. This will also save me the effort
> to ask if
> to people submitting patches if this feature is supported or not.

I understand the benefits for you. But I see the disadvantages for the user. Regardless of whether RESTRICTED mode is mandatory, not requesting this mode should not result in what looks like an error message for the user. As it stands, the fact that it is even a warning is even more confusing, since the interface is configured anyway. And I don't think I'm an average user. Given that the interface will be configured anyway, this warning is more disruptive than useful to me. And that's really a strong opinion.
 
> If someone comes with a non-compliant device, we can still statically
> set the
> flag. So this isn't even a blocker for non compliant devices.
> 

After reading the very strict interpretation of the ES bit (which I also agree with, despite being the originator of the initial simplification proposal), I admit that I don't understand this:

Speaking of my case: our CANXL core does not currently include RESTRICTED mode (which is considered more of an option to be added in the future). If CANXL grows in popularity, it will be much more due to its finally relevant throughputs and payload length than to this mode, which is close to LISTEN_ONLY in my opinion.

Furthermore, I find it hard to imagine that the RESTRICTED flag would be added statically, whereas in reality, considering it as an option, the driver could add it as a feature finally supported by new firmware versions.

I see it more like CAN_CTRLMODE_CC_LEN8_DLC: if I'm not mistaken, DLC values > 8 have always been allowed by the standard, and yet it was only added in 5.11.

At worst, let's define a flag such as CAN_CTRLMODE_FD_NON_ISO, i.e., CAN_CTRLMODE_XL_NON_RESTRICTED.


> So, at the end, this is mostly a selfish feature to remove some patch
> reviewing
> effort (while still pushing toward ISO compliance, which isn't a bad
> thing either).
> 
> I prefer to have it like this, but this isn't a strong opinion
> either.
> 
> 
> Yours sincerely,
> Vincent Mailhol
> 

Looking forward to hearing from you,

-- Stéphane 
Re: [PATCH 3/9] can: netlink: add initial CAN XL support
Posted by Oliver Hartkopp 2 months, 3 weeks ago

On 15.11.25 14:35, Vincent Mailhol wrote:
> On 14/11/2025 at 14:19, Oliver Hartkopp wrote:
>> Hi Vincent,
>>
>> On 13.10.25 13:01, Vincent Mailhol wrote:
>>> CAN XL uses bittiming parameters different from Classical CAN and CAN
>>> FD. Thus, all the data bittiming parameters, including TDC, need to be
>>> duplicated for CAN XL.
>>>
>>> Add the CAN XL netlink interface for all the features which are common
>>> with CAN FD. Any new CAN XL specific features are added later on.
>>>
>>> Add a check that CAN XL capable nodes correctly provide
>>> CAN_CTRLMODE_RESTRIC_OP as mandated by ISO 11898-1:2024 §6.6.19.
>>>
>>> The first time CAN XL is activated, the MTU is set by default to
>>> CANXL_MAX_MTU. The user may then configure a custom MTU within the
>>> CANXL_MIN_MTU to CANXL_MIN_MTU range, in which case, the custom MTU
>>> value will be kept as long as CAN XL remains active.
>>>
>>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>>> ---
>>> Changelog:
>>>
>>> RFC -> v1:
>>>
>>>     - Correctly wipe out the CAN XL data bittiming and TDC parameters
>>>       when switching CAN_CTRLMODE_XL off.
>>>
>>>     - Add one level on nesting for xl parameters so that:
>>>
>>>        - bittiming are under priv->xl.data_bittiming{,_const}¨
>>>        - pwm are under priv->xl.pwm{,_const}
>>>
>>>     - Many other code refactors.
>>> ---
>>>    drivers/net/can/dev/dev.c        | 14 ++++++-
>>>    drivers/net/can/dev/netlink.c    | 87 ++++++++++++++++++++++++++++++++--------
>>>    include/linux/can/bittiming.h    |  6 ++-
>>>    include/linux/can/dev.h          |  7 +++-
>>>    include/uapi/linux/can/netlink.h |  7 ++++
>>>    5 files changed, 100 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>>> index 3377afb6f1c4..32f11db88295 100644
>>> --- a/drivers/net/can/dev/dev.c
>>> +++ b/drivers/net/can/dev/dev.c
>>> @@ -117,6 +117,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>>>            return "fd-tdc-manual";
>>>        case CAN_CTRLMODE_RESTRICTED:
>>>            return "restricted-operation";
>>> +    case CAN_CTRLMODE_XL:
>>> +        return "xl";
>>> +    case CAN_CTRLMODE_XL_TDC_AUTO:
>>> +        return "xl-tdc-auto";
>>> +    case CAN_CTRLMODE_XL_TDC_MANUAL:
>>> +        return "xl-tdc-manual";
>>>        default:
>>>            return "<unknown>";
>>>        }
>>> @@ -350,7 +356,13 @@ void can_set_default_mtu(struct net_device *dev)
>>>    {
>>>        struct can_priv *priv = netdev_priv(dev);
>>>    -    if (priv->ctrlmode & CAN_CTRLMODE_FD) {
>>> +    if (priv->ctrlmode & CAN_CTRLMODE_XL) {
>>> +        if (can_is_canxl_dev_mtu(dev->mtu))
>>> +            return;
>>> +        dev->mtu = CANXL_MTU;
>>> +        dev->min_mtu = CANXL_MIN_MTU;
>>> +        dev->max_mtu = CANXL_MAX_MTU;
>>> +    } else if (priv->ctrlmode & CAN_CTRLMODE_FD) {
>>>            dev->mtu = CANFD_MTU;
>>>            dev->min_mtu = CANFD_MTU;
>>>            dev->max_mtu = CANFD_MTU;
>>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>>> index f44b5dffa176..2405f1265488 100644
>>> --- a/drivers/net/can/dev/netlink.c
>>> +++ b/drivers/net/can/dev/netlink.c
>>> @@ -2,7 +2,7 @@
>>>    /* Copyright (C) 2005 Marc Kleine-Budde, Pengutronix
>>>     * Copyright (C) 2006 Andrey Volkov, Varma Electronics
>>>     * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
>>> - * Copyright (C) 2021 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>>> + * Copyright (C) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
>>>     */
>>>      #include <linux/can/dev.h>
>>> @@ -22,6 +22,9 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
>>>        [IFLA_CAN_TERMINATION] = { .type = NLA_U16 },
>>>        [IFLA_CAN_TDC] = { .type = NLA_NESTED },
>>>        [IFLA_CAN_CTRLMODE_EXT] = { .type = NLA_NESTED },
>>> +    [IFLA_CAN_XL_DATA_BITTIMING] = { .len = sizeof(struct can_bittiming) },
>>> +    [IFLA_CAN_XL_DATA_BITTIMING_CONST] = { .len = sizeof(struct
>>> can_bittiming_const) },
>>> +    [IFLA_CAN_XL_TDC] = { .type = NLA_NESTED },
>>>    };
>>>      static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
>>> @@ -70,7 +73,7 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>>>            return -EOPNOTSUPP;
>>>        }
>>>    -    /* If one of the CAN_CTRLMODE_TDC_* flag is set then TDC
>>> +    /* If one of the CAN_CTRLMODE_{,XL}_TDC_* flags is set then TDC
>>>         * must be set and vice-versa
>>>         */
>>>        if ((tdc_auto || tdc_manual) && !data_tdc) {
>>> @@ -82,8 +85,8 @@ static int can_validate_tdc(struct nlattr *data_tdc,
>>>            return -EOPNOTSUPP;
>>>        }
>>>    -    /* If providing TDC parameters, at least TDCO is needed. TDCV
>>> -     * is needed if and only if CAN_CTRLMODE_TDC_MANUAL is set
>>> +    /* If providing TDC parameters, at least TDCO is needed. TDCV is
>>> +     * needed if and only if CAN_CTRLMODE_{,XL}_TDC_MANUAL is set
>>>         */
>>>        if (data_tdc) {
>>>            struct nlattr *tb_tdc[IFLA_CAN_TDC_MAX + 1];
>>> @@ -126,10 +129,10 @@ static int can_validate_databittiming(struct nlattr
>>> *data[],
>>>        bool is_on;
>>>        int err;
>>>    -    /* Make sure that valid CAN FD configurations always consist of
>>> +    /* Make sure that valid CAN FD/XL configurations always consist of
>>>         * - nominal/arbitration bittiming
>>>         * - data bittiming
>>> -     * - control mode with CAN_CTRLMODE_FD set
>>> +     * - control mode with CAN_CTRLMODE_{FD,XL} set
>>>         * - TDC parameters are coherent (details in can_validate_tdc())
>>>         */
>>>    @@ -139,7 +142,10 @@ static int can_validate_databittiming(struct nlattr
>>> *data[],
>>>            is_on = flags & CAN_CTRLMODE_FD;
>>>            type = "FD";
>>>        } else {
>>> -        return -EOPNOTSUPP; /* Place holder for CAN XL */
>>> +        data_tdc = data[IFLA_CAN_XL_TDC];
>>> +        tdc_flags = flags & CAN_CTRLMODE_XL_TDC_MASK;
>>> +        is_on = flags & CAN_CTRLMODE_XL;
>>> +        type = "XL";
>>>        }
>>>          if (is_on) {
>>> @@ -206,6 +212,11 @@ static int can_validate(struct nlattr *tb[], struct
>>> nlattr *data[],
>>>        if (err)
>>>            return err;
>>>    +    err = can_validate_databittiming(data, extack,
>>> +                     IFLA_CAN_XL_DATA_BITTIMING, flags);
>>> +    if (err)
>>> +        return err;
>>> +
>>>        return 0;
>>>    }
>>>    @@ -215,7 +226,8 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>>    {
>>>        struct can_priv *priv = netdev_priv(dev);
>>>        struct can_ctrlmode *cm;
>>> -    u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
>>> +    const u32 xl_mandatory = CAN_CTRLMODE_RESTRICTED;
>>> +    u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing, xl_missing;
>>>          if (!data[IFLA_CAN_CTRLMODE])
>>>            return 0;
>>> @@ -229,6 +241,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>>        maskedflags = cm->flags & cm->mask;
>>>        notsupp = maskedflags & ~(priv->ctrlmode_supported | ctrlstatic);
>>>        ctrlstatic_missing = (maskedflags & ctrlstatic) ^ ctrlstatic;
>>> +    xl_missing = (priv->ctrlmode_supported & xl_mandatory) ^ xl_mandatory;
>>>          if (notsupp) {
>>>            NL_SET_ERR_MSG_FMT(extack,
>>> @@ -248,21 +261,36 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>>            return -EOPNOTSUPP;
>>>        }
>>>    +    if ((priv->ctrlmode_supported & CAN_CTRLMODE_XL) && xl_missing) {
>>> +        NL_SET_ERR_MSG_FMT(extack,
>>> +                   "bad device: CAN XL capable nodes must support the %s mode",
>>> +                   can_get_ctrlmode_str(xl_missing));
>>> +        return -EOPNOTSUPP;
>>> +    }
>>> +
>>
>> I'm not sure if it is our job to check for ISO 11898-1:2024 compliance of CAN
>> controllers when CAN_CTRLMODE_RESTRICTED is not supported.
>>
>> IMO an appropriate error message (only) when the user requests this feature
>> would be better and that is already the standard behaviour.
> 
> For this one, the initial logic was to avoid some incorrect configurations such
> as providing CAN_CTRLMODE_XL_TMS without CAN_CTRLMODE_XL_ERR_SIGNAL.
> 
> Contrarily to the other flags, CAN XL introduces some intricacies between some
> of the flags and giving full freedom on what must be set or not is asking for
> troubles.
> 
> That said, the CAN_CTRLMODE_RESTRICTED is different and does not depend on the
> other flags. I just added it at the end after adding the check logic for the
> other XL flags (this appears first in the series because of some rebase, but
> trust me, I implemented this last).
> 
> Then, should we go for this or not? The benefit I see to add this check would be
> less effort on the patch reviews for new devices (which I am mostly doing
> recently). I can foreseen that people will wrongly use CAN_CTRLMODE_LISTENONLY
> instead of CAN_CTRLMODE_RESTRICTED. This piece of code will make sure that they
> will implement CAN_CTRLMODE_RESTRICTED. And because this feature is mandatory, I
> thought this wasn't a too bad idea. This will also save me the effort to ask if
> to people submitting patches if this feature is supported or not.
> 
> If someone comes with a non-compliant device, we can still statically set the
> flag. So this isn't even a blocker for non compliant devices.

We had the same issue with non-iso CAN FD devices.

IMO we should provide the CAN_CTRLMODE_RESTRICTED feature like any other 
feature in the control modes (like e.g. triple-sampling). And when a 
device shows up that needs a special handling beyond our good working 
netlink API capabilities we talk about it in we will find a solution. As 
always.

Best regards,
Oliver
  >
> So, at the end, this is mostly a selfish feature to remove some patch reviewing
> effort (while still pushing toward ISO compliance, which isn't a bad thing either).
> 
> I prefer to have it like this, but this isn't a strong opinion either.
> 
> 
> Yours sincerely,
> Vincent Mailhol
> 

Re: [PATCH 3/9] can: netlink: add initial CAN XL support
Posted by Stéphane Grosjean 2 months, 3 weeks ago
Hi all,

I completely agree with Oliver: an error or warning message should only appear when the feature is requested but not supported.

In the opposite case, displaying a warning about an unsupported feature when it has not been requested—but still configuring the interface—leaves the user (me ;-) ) in doubt (speaking from experience ;-)).

Best regards,

-- Stéphane

----- Mail original ----- 

De: "Oliver Hartkopp" <socketcan@hartkopp.net> 
À: "Vincent Mailhol" <mailhol@kernel.org>, "Marc Kleine-Budde" <mkl@pengutronix.de> 
Cc: "Stéphane Grosjean" <stephane.grosjean@hms-networks.com>, "Robert Nawrath" <mbro1689@gmail.com>, "Minh Le" <minh.le.aj@renesas.com>, "Duy Nguyen" <duy.nguyen.rh@renesas.com>, linux-can@vger.kernel.org, linux-kernel@vger.kernel.org 
Envoyé: Vendredi 14 Novembre 2025 14:19:04 
Objet: Re: [PATCH 3/9] can: netlink: add initial CAN XL support 

Hi Vincent, 

On 13.10.25 13:01, Vincent Mailhol wrote: 
> CAN XL uses bittiming parameters different from Classical CAN and CAN 
> FD. Thus, all the data bittiming parameters, including TDC, need to be 
> duplicated for CAN XL. 
> 
> Add the CAN XL netlink interface for all the features which are common 
> with CAN FD. Any new CAN XL specific features are added later on. 
> 
> Add a check that CAN XL capable nodes correctly provide 
> CAN_CTRLMODE_RESTRIC_OP as mandated by ISO 11898-1:2024 §6.6.19. 
> 
> The first time CAN XL is activated, the MTU is set by default to 
> CANXL_MAX_MTU. The user may then configure a custom MTU within the 
> CANXL_MIN_MTU to CANXL_MIN_MTU range, in which case, the custom MTU 
> value will be kept as long as CAN XL remains active. 
> 
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org> 
> --- 
> Changelog: 
> 
> RFC -> v1: 
> 
> - Correctly wipe out the CAN XL data bittiming and TDC parameters 
> when switching CAN_CTRLMODE_XL off. 
> 
> - Add one level on nesting for xl parameters so that: 
> 
> - bittiming are under priv->xl.data_bittiming{,_const}¨ 
> - pwm are under priv->xl.pwm{,_const} 
> 
> - Many other code refactors. 
> --- 
> drivers/net/can/dev/dev.c | 14 ++++++- 
> drivers/net/can/dev/netlink.c | 87 ++++++++++++++++++++++++++++++++-------- 
> include/linux/can/bittiming.h | 6 ++- 
> include/linux/can/dev.h | 7 +++- 
> include/uapi/linux/can/netlink.h | 7 ++++ 
> 5 files changed, 100 insertions(+), 21 deletions(-) 
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c 
> index 3377afb6f1c4..32f11db88295 100644 
> --- a/drivers/net/can/dev/dev.c 
> +++ b/drivers/net/can/dev/dev.c 
> @@ -117,6 +117,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode) 
> return "fd-tdc-manual"; 
> case CAN_CTRLMODE_RESTRICTED: 
> return "restricted-operation"; 
> + case CAN_CTRLMODE_XL: 
> + return "xl"; 
> + case CAN_CTRLMODE_XL_TDC_AUTO: 
> + return "xl-tdc-auto"; 
> + case CAN_CTRLMODE_XL_TDC_MANUAL: 
> + return "xl-tdc-manual"; 
> default: 
> return "<unknown>"; 
> } 
> @@ -350,7 +356,13 @@ void can_set_default_mtu(struct net_device *dev) 
> { 
> struct can_priv *priv = netdev_priv(dev); 
> 
> - if (priv->ctrlmode & CAN_CTRLMODE_FD) { 
> + if (priv->ctrlmode & CAN_CTRLMODE_XL) { 
> + if (can_is_canxl_dev_mtu(dev->mtu)) 
> + return; 
> + dev->mtu = CANXL_MTU; 
> + dev->min_mtu = CANXL_MIN_MTU; 
> + dev->max_mtu = CANXL_MAX_MTU; 
> + } else if (priv->ctrlmode & CAN_CTRLMODE_FD) { 
> dev->mtu = CANFD_MTU; 
> dev->min_mtu = CANFD_MTU; 
> dev->max_mtu = CANFD_MTU; 
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c 
> index f44b5dffa176..2405f1265488 100644 
> --- a/drivers/net/can/dev/netlink.c 
> +++ b/drivers/net/can/dev/netlink.c 
> @@ -2,7 +2,7 @@ 
> /* Copyright (C) 2005 Marc Kleine-Budde, Pengutronix 
> * Copyright (C) 2006 Andrey Volkov, Varma Electronics 
> * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com> 
> - * Copyright (C) 2021 Vincent Mailhol <mailhol.vincent@wanadoo.fr> 
> + * Copyright (C) 2021-2025 Vincent Mailhol <mailhol@kernel.org> 
> */ 
> 
> #include <linux/can/dev.h> 
> @@ -22,6 +22,9 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = { 
> [IFLA_CAN_TERMINATION] = { .type = NLA_U16 }, 
> [IFLA_CAN_TDC] = { .type = NLA_NESTED }, 
> [IFLA_CAN_CTRLMODE_EXT] = { .type = NLA_NESTED }, 
> + [IFLA_CAN_XL_DATA_BITTIMING] = { .len = sizeof(struct can_bittiming) }, 
> + [IFLA_CAN_XL_DATA_BITTIMING_CONST] = { .len = sizeof(struct can_bittiming_const) }, 
> + [IFLA_CAN_XL_TDC] = { .type = NLA_NESTED }, 
> }; 
> 
> static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = { 
> @@ -70,7 +73,7 @@ static int can_validate_tdc(struct nlattr *data_tdc, 
> return -EOPNOTSUPP; 
> } 
> 
> - /* If one of the CAN_CTRLMODE_TDC_* flag is set then TDC 
> + /* If one of the CAN_CTRLMODE_{,XL}_TDC_* flags is set then TDC 
> * must be set and vice-versa 
> */ 
> if ((tdc_auto || tdc_manual) && !data_tdc) { 
> @@ -82,8 +85,8 @@ static int can_validate_tdc(struct nlattr *data_tdc, 
> return -EOPNOTSUPP; 
> } 
> 
> - /* If providing TDC parameters, at least TDCO is needed. TDCV 
> - * is needed if and only if CAN_CTRLMODE_TDC_MANUAL is set 
> + /* If providing TDC parameters, at least TDCO is needed. TDCV is 
> + * needed if and only if CAN_CTRLMODE_{,XL}_TDC_MANUAL is set 
> */ 
> if (data_tdc) { 
> struct nlattr *tb_tdc[IFLA_CAN_TDC_MAX + 1]; 
> @@ -126,10 +129,10 @@ static int can_validate_databittiming(struct nlattr *data[], 
> bool is_on; 
> int err; 
> 
> - /* Make sure that valid CAN FD configurations always consist of 
> + /* Make sure that valid CAN FD/XL configurations always consist of 
> * - nominal/arbitration bittiming 
> * - data bittiming 
> - * - control mode with CAN_CTRLMODE_FD set 
> + * - control mode with CAN_CTRLMODE_{FD,XL} set 
> * - TDC parameters are coherent (details in can_validate_tdc()) 
> */ 
> 
> @@ -139,7 +142,10 @@ static int can_validate_databittiming(struct nlattr *data[], 
> is_on = flags & CAN_CTRLMODE_FD; 
> type = "FD"; 
> } else { 
> - return -EOPNOTSUPP; /* Place holder for CAN XL */ 
> + data_tdc = data[IFLA_CAN_XL_TDC]; 
> + tdc_flags = flags & CAN_CTRLMODE_XL_TDC_MASK; 
> + is_on = flags & CAN_CTRLMODE_XL; 
> + type = "XL"; 
> } 
> 
> if (is_on) { 
> @@ -206,6 +212,11 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[], 
> if (err) 
> return err; 
> 
> + err = can_validate_databittiming(data, extack, 
> + IFLA_CAN_XL_DATA_BITTIMING, flags); 
> + if (err) 
> + return err; 
> + 
> return 0; 
> } 
> 
> @@ -215,7 +226,8 @@ static int can_ctrlmode_changelink(struct net_device *dev, 
> { 
> struct can_priv *priv = netdev_priv(dev); 
> struct can_ctrlmode *cm; 
> - u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing; 
> + const u32 xl_mandatory = CAN_CTRLMODE_RESTRICTED; 
> + u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing, xl_missing; 
> 
> if (!data[IFLA_CAN_CTRLMODE]) 
> return 0; 
> @@ -229,6 +241,7 @@ static int can_ctrlmode_changelink(struct net_device *dev, 
> maskedflags = cm->flags & cm->mask; 
> notsupp = maskedflags & ~(priv->ctrlmode_supported | ctrlstatic); 
> ctrlstatic_missing = (maskedflags & ctrlstatic) ^ ctrlstatic; 
> + xl_missing = (priv->ctrlmode_supported & xl_mandatory) ^ xl_mandatory; 
> 
> if (notsupp) { 
> NL_SET_ERR_MSG_FMT(extack, 
> @@ -248,21 +261,36 @@ static int can_ctrlmode_changelink(struct net_device *dev, 
> return -EOPNOTSUPP; 
> } 
> 
> + if ((priv->ctrlmode_supported & CAN_CTRLMODE_XL) && xl_missing) { 
> + NL_SET_ERR_MSG_FMT(extack, 
> + "bad device: CAN XL capable nodes must support the %s mode", 
> + can_get_ctrlmode_str(xl_missing)); 
> + return -EOPNOTSUPP; 
> + } 
> + 

I'm not sure if it is our job to check for ISO 11898-1:2024 compliance 
of CAN controllers when CAN_CTRLMODE_RESTRICTED is not supported. 

IMO an appropriate error message (only) when the user requests this 
feature would be better and that is already the standard behaviour. 

Best regards, 
Oliver 

> /* 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); 
> 
> /* clear bits to be modified and copy the flag values */ 
> priv->ctrlmode &= ~cm->mask; 
> priv->ctrlmode |= maskedflags; 
> 
> - /* Wipe potential leftovers from previous CAN FD config */ 
> + /* Wipe potential leftovers from previous CAN FD/XL config */ 
> if (!(priv->ctrlmode & CAN_CTRLMODE_FD)) { 
> memset(&priv->fd.data_bittiming, 0, 
> sizeof(priv->fd.data_bittiming)); 
> priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK; 
> memset(&priv->fd.tdc, 0, sizeof(priv->fd.tdc)); 
> } 
> + if (!(priv->ctrlmode & CAN_CTRLMODE_XL)) { 
> + memset(&priv->xl.data_bittiming, 0, 
> + sizeof(priv->fd.data_bittiming)); 
> + priv->ctrlmode &= ~CAN_CTRLMODE_XL_TDC_MASK; 
> + memset(&priv->xl.tdc, 0, sizeof(priv->xl.tdc)); 
> + } 
> 
> can_set_default_mtu(dev); 
> 
> @@ -337,7 +365,10 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[], 
> dbt_params = &priv->fd; 
> tdc_mask = CAN_CTRLMODE_FD_TDC_MASK; 
> } else { 
> - return -EOPNOTSUPP; /* Place holder for CAN XL */ 
> + data_bittiming = data[IFLA_CAN_XL_DATA_BITTIMING]; 
> + data_tdc = data[IFLA_CAN_XL_TDC]; 
> + dbt_params = &priv->xl; 
> + tdc_mask = CAN_CTRLMODE_XL_TDC_MASK; 
> } 
> 
> if (!data_bittiming) 
> @@ -388,7 +419,7 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[], 
> */ 
> can_calc_tdco(&dbt_params->tdc, dbt_params->tdc_const, &dbt, 
> tdc_mask, &priv->ctrlmode, priv->ctrlmode_supported); 
> - } /* else: both CAN_CTRLMODE_TDC_{AUTO,MANUAL} are explicitly 
> + } /* else: both CAN_CTRLMODE_{,XL}_TDC_{AUTO,MANUAL} are explicitly 
> * turned off. TDC is disabled: do nothing 
> */ 
> 
> @@ -491,6 +522,11 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[], 
> if (err) 
> return err; 
> 
> + /* CAN XL */ 
> + err = can_dbt_changelink(dev, data, false, extack); 
> + if (err) 
> + return err; 
> + 
> if (data[IFLA_CAN_TERMINATION]) { 
> const u16 termval = nla_get_u16(data[IFLA_CAN_TERMINATION]); 
> const unsigned int num_term = priv->termination_const_cnt; 
> @@ -558,14 +594,14 @@ static size_t can_data_bittiming_get_size(struct data_bittiming_params *dbt_para 
> { 
> size_t size = 0; 
> 
> - if (dbt_params->data_bittiming.bitrate) /* IFLA_CAN_DATA_BITTIMING */ 
> + if (dbt_params->data_bittiming.bitrate) /* IFLA_CAN_{,XL}_DATA_BITTIMING */ 
> size += nla_total_size(sizeof(dbt_params->data_bittiming)); 
> - if (dbt_params->data_bittiming_const) /* IFLA_CAN_DATA_BITTIMING_CONST */ 
> + if (dbt_params->data_bittiming_const) /* IFLA_CAN_{,XL}_DATA_BITTIMING_CONST */ 
> size += nla_total_size(sizeof(*dbt_params->data_bittiming_const)); 
> - if (dbt_params->data_bitrate_const) /* IFLA_CAN_DATA_BITRATE_CONST */ 
> + if (dbt_params->data_bitrate_const) /* IFLA_CAN_{,XL}_DATA_BITRATE_CONST */ 
> size += nla_total_size(sizeof(*dbt_params->data_bitrate_const) * 
> dbt_params->data_bitrate_const_cnt); 
> - size += can_tdc_get_size(dbt_params, tdc_flags);/* IFLA_CAN_TDC */ 
> + size += can_tdc_get_size(dbt_params, tdc_flags);/* IFLA_CAN_{,XL}_TDC */ 
> 
> return size; 
> } 
> @@ -605,6 +641,9 @@ static size_t can_get_size(const struct net_device *dev) 
> size += can_data_bittiming_get_size(&priv->fd, 
> priv->ctrlmode & CAN_CTRLMODE_FD_TDC_MASK); 
> 
> + size += can_data_bittiming_get_size(&priv->xl, 
> + priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK); 
> + 
> return size; 
> } 
> 
> @@ -649,7 +688,9 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev, 
> tdc_is_enabled = can_fd_tdc_is_enabled(priv); 
> tdc_manual = priv->ctrlmode & CAN_CTRLMODE_TDC_MANUAL; 
> } else { 
> - return -EOPNOTSUPP; /* Place holder for CAN XL */ 
> + dbt_params = &priv->xl; 
> + tdc_is_enabled = can_xl_tdc_is_enabled(priv); 
> + tdc_manual = priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MANUAL; 
> } 
> tdc_const = dbt_params->tdc_const; 
> tdc = &dbt_params->tdc; 
> @@ -771,7 +812,19 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev) 
> 
> can_tdc_fill_info(skb, dev, IFLA_CAN_TDC) || 
> 
> - can_ctrlmode_ext_fill_info(skb, priv) 
> + can_ctrlmode_ext_fill_info(skb, priv) || 
> + 
> + can_bittiming_fill_info(skb, IFLA_CAN_XL_DATA_BITTIMING, 
> + &priv->xl.data_bittiming) || 
> + 
> + can_bittiming_const_fill_info(skb, IFLA_CAN_XL_DATA_BITTIMING_CONST, 
> + priv->xl.data_bittiming_const) || 
> + 
> + can_bitrate_const_fill_info(skb, IFLA_CAN_XL_DATA_BITRATE_CONST, 
> + priv->xl.data_bitrate_const, 
> + priv->xl.data_bitrate_const_cnt) || 
> + 
> + can_tdc_fill_info(skb, dev, IFLA_CAN_XL_TDC) 
> ) 
> 
> return -EMSGSIZE; 
> diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h 
> index d30816dd93c7..68d533bb7856 100644 
> --- a/include/linux/can/bittiming.h 
> +++ b/include/linux/can/bittiming.h 
> @@ -16,10 +16,12 @@ 
> 
> #define CAN_CTRLMODE_FD_TDC_MASK \ 
> (CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_TDC_MANUAL) 
> +#define CAN_CTRLMODE_XL_TDC_MASK \ 
> + (CAN_CTRLMODE_XL_TDC_AUTO | CAN_CTRLMODE_XL_TDC_MANUAL) 
> #define CAN_CTRLMODE_TDC_AUTO_MASK \ 
> - (CAN_CTRLMODE_TDC_AUTO) 
> + (CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_XL_TDC_AUTO) 
> #define CAN_CTRLMODE_TDC_MANUAL_MASK \ 
> - (CAN_CTRLMODE_TDC_MANUAL) 
> + (CAN_CTRLMODE_TDC_MANUAL | CAN_CTRLMODE_XL_TDC_MANUAL) 
> 
> /* 
> * struct can_tdc - CAN FD Transmission Delay Compensation parameters 
> diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h 
> index 9de8fde3ec9d..945c16743702 100644 
> --- a/include/linux/can/dev.h 
> +++ b/include/linux/can/dev.h 
> @@ -47,7 +47,7 @@ struct can_priv { 
> 
> const struct can_bittiming_const *bittiming_const; 
> struct can_bittiming bittiming; 
> - struct data_bittiming_params fd; 
> + struct data_bittiming_params fd, xl; 
> unsigned int bitrate_const_cnt; 
> const u32 *bitrate_const; 
> u32 bitrate_max; 
> @@ -85,6 +85,11 @@ static inline bool can_fd_tdc_is_enabled(const struct can_priv *priv) 
> return !!(priv->ctrlmode & CAN_CTRLMODE_FD_TDC_MASK); 
> } 
> 
> +static inline bool can_xl_tdc_is_enabled(const struct can_priv *priv) 
> +{ 
> + return !!(priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK); 
> +} 
> + 
> static inline u32 can_get_static_ctrlmode(struct can_priv *priv) 
> { 
> return priv->ctrlmode & ~priv->ctrlmode_supported; 
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h 
> index fafd1cce4798..c2c96c5978a8 100644 
> --- a/include/uapi/linux/can/netlink.h 
> +++ b/include/uapi/linux/can/netlink.h 
> @@ -104,6 +104,9 @@ struct can_ctrlmode { 
> #define CAN_CTRLMODE_TDC_AUTO 0x200 /* FD transceiver automatically calculates TDCV */ 
> #define CAN_CTRLMODE_TDC_MANUAL 0x400 /* FD TDCV is manually set up by user */ 
> #define CAN_CTRLMODE_RESTRICTED 0x800 /* Restricted operation mode */ 
> +#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 */ 
> 
> /* 
> * CAN device statistics 
> @@ -139,6 +142,10 @@ enum { 
> IFLA_CAN_BITRATE_MAX, 
> IFLA_CAN_TDC, /* FD */ 
> IFLA_CAN_CTRLMODE_EXT, 
> + IFLA_CAN_XL_DATA_BITTIMING, 
> + IFLA_CAN_XL_DATA_BITTIMING_CONST, 
> + IFLA_CAN_XL_DATA_BITRATE_CONST, 
> + IFLA_CAN_XL_TDC, 
> 
> /* add new constants above here */ 
> __IFLA_CAN_MAX, 
>