[PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL

Vincent Mailhol posted 10 patches 3 months, 2 weeks ago
Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
Posted by Oliver Hartkopp 3 months ago

On 21.10.25 17:47, Vincent Mailhol wrote:
> Classical CAN and CAN FD must generate error frames on the CAN bus
> when detecting a protocol violation.
> 
> CAN XL's error signaling is different and works as follows:
> 
>    - In interoperability mode (both FD and XL), error signaling must be
>      on.
> 
>    - When operating a CAN controller in CAN XL only mode but with TMS
>      off, the user can decide whether the error signalling is enabled
>      or disabled.
> 
>    - On the contrary, when using TMS, error signalling must be off.
> 
> Introduce the new CAN_CTRLMODE_XL_ERR_SIGNAL control mode. This new
> option is only made available for CAN XL, so despite the error
> signalling being always on for Classical CAN and CAN FD, forbid the
> use of this flag when CAN XL is off.
> 
> If the user provides the error signalling flag, check its validity. If
> the flag is omitted, activate error signalling by default whenever
> possible. This is summarized in below table:
> 
> 			CAN_CTRLMODE_XL_ERR_SIGNAL
> 	-------------------------------------------
> 	CC/FD		option not available
> 	CC/FD/XL	on

Yes. This is the 'mixed-mode'
I would propose to use the 'mixed-mode' expression in the patch description.

> 	XL TMS off	configurable (default on)

Good default.

> 	XL TMS on	off
> 
> Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Link: https://lore.kernel.org/linux-can/20250527195625.65252-9-socketcan@hartkopp.net/
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
>   drivers/net/can/dev/dev.c        |  2 ++
>   drivers/net/can/dev/netlink.c    | 29 +++++++++++++++++++++++++++--
>   include/uapi/linux/can/netlink.h |  1 +
>   3 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 1de5babcc4f3..0c16d0174f7f 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -125,6 +125,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>   		return "xl-tdc-manual";
>   	case CAN_CTRLMODE_XL_TMS:
>   		return "xl-tms";
> +	case CAN_CTRLMODE_XL_ERR_SIGNAL:
> +		return "xl-error-signalling";
>   	default:
>   		return "<unknown>";
>   	}
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 8afd2baa03cf..6126b191fea0 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -191,7 +191,8 @@ static int can_validate_xl_flags(struct netlink_ext_ack *extack,
>   		}
>   		if (masked_flags & CAN_CTRLMODE_XL_TMS) {
>   			const u32 tms_conflicts_mask = CAN_CTRLMODE_FD |
> -				CAN_CTRLMODE_XL_TDC_MASK;
> +				CAN_CTRLMODE_XL_TDC_MASK |
> +				CAN_CTRLMODE_XL_ERR_SIGNAL;
>   			u32 tms_conflicts = masked_flags & tms_conflicts_mask;
>   
>   			if (tms_conflicts) {
> @@ -201,11 +202,23 @@ static int can_validate_xl_flags(struct netlink_ext_ack *extack,
>   				return -EOPNOTSUPP;
>   			}
>   		}
> +		if ((masked_flags & CAN_CTRLMODE_FD) &&
> +		    (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
> +		    !(masked_flags & CAN_CTRLMODE_XL_ERR_SIGNAL)) {
> +			NL_SET_ERR_MSG(extack,
> +				       "When using both CAN FD and XL, error signalling must be on");

This implicitly tells us that mixed-mode is CC/FD/XL ;-)
  > +			return -EOPNOTSUPP;
> +		}
>   	} else {
>   		if (mask & CAN_CTRLMODE_XL_TMS) {
>   			NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
>   			return -EOPNOTSUPP;
>   		}
> +		if (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) {
> +			NL_SET_ERR_MSG(extack,
> +				       "Error signalling is only configurable with CAN XL");
> +			return -EOPNOTSUPP;
> +		}
>   	}
>   
>   	return 0;
> @@ -310,6 +323,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   				       "TMS can not be activated while CAN FD is on");
>   			return -EOPNOTSUPP;
>   		}
> +		if (deactivated & CAN_CTRLMODE_XL_ERR_SIGNAL) {
> +			NL_SET_ERR_MSG(extack,
> +				       "Error signalling can not be deactivated while CAN FD is on");
> +			return -EOPNOTSUPP;
> +		}
>   	}
>   
>   	/* If a top dependency flag is provided, reset all its dependencies */
> @@ -317,12 +335,19 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>   		priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
>   	if (cm->mask & CAN_CTRLMODE_XL)
>   		priv->ctrlmode &= ~(CAN_CTRLMODE_XL_TDC_MASK |
> -				    CAN_CTRLMODE_XL_TMS);
> +				    CAN_CTRLMODE_XL_TMS |
> +				    CAN_CTRLMODE_XL_ERR_SIGNAL);
>   
>   	/* clear bits to be modified and copy the flag values */
>   	priv->ctrlmode &= ~cm->mask;
>   	priv->ctrlmode |= maskedflags;
>   
> +	/* If omitted, set error signalling on if possible */
> +	if ((maskedflags & CAN_CTRLMODE_XL) &&
> +	    !(cm->mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
> +	    !(priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
> +		priv->ctrlmode |= CAN_CTRLMODE_XL_ERR_SIGNAL;
> +
>   	/* Wipe potential leftovers from previous CAN FD/XL config */
>   	if (!(priv->ctrlmode & CAN_CTRLMODE_FD)) {
>   		memset(&priv->fd.data_bittiming, 0,
> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
> index ebafb091d80f..30d446921dc4 100644
> --- a/include/uapi/linux/can/netlink.h
> +++ b/include/uapi/linux/can/netlink.h
> @@ -108,6 +108,7 @@ struct can_ctrlmode {
>   #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 */
> +#define CAN_CTRLMODE_XL_ERR_SIGNAL	0x10000	/* XL error signalling */
>   
>   /*
>    * CAN device statistics
> 
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Best regards,
Oliver
Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
Posted by Vincent Mailhol 3 months ago
On 06/11/2025 at 09:50, Oliver Hartkopp wrote:
> On 21.10.25 17:47, Vincent Mailhol wrote:
>> Classical CAN and CAN FD must generate error frames on the CAN bus
>> when detecting a protocol violation.
>>
>> CAN XL's error signaling is different and works as follows:
>>
>>    - In interoperability mode (both FD and XL), error signaling must be
>>      on.
>>
>>    - When operating a CAN controller in CAN XL only mode but with TMS
>>      off, the user can decide whether the error signalling is enabled
>>      or disabled.
>>
>>    - On the contrary, when using TMS, error signalling must be off.
>>
>> Introduce the new CAN_CTRLMODE_XL_ERR_SIGNAL control mode. This new
>> option is only made available for CAN XL, so despite the error
>> signalling being always on for Classical CAN and CAN FD, forbid the
>> use of this flag when CAN XL is off.
>>
>> If the user provides the error signalling flag, check its validity. If
>> the flag is omitted, activate error signalling by default whenever
>> possible. This is summarized in below table:
>>
>>             CAN_CTRLMODE_XL_ERR_SIGNAL
>>     -------------------------------------------
>>     CC/FD        option not available
>>     CC/FD/XL    on
> 
> Yes. This is the 'mixed-mode'
> I would propose to use the 'mixed-mode' expression in the patch description.

Ack!

>>     XL TMS off    configurable (default on)
> 
> Good default.
> 
>>     XL TMS on    off
>>
>> Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> Link: https://lore.kernel.org/linux-can/20250527195625.65252-9-
>> socketcan@hartkopp.net/
>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>> ---
>>   drivers/net/can/dev/dev.c        |  2 ++
>>   drivers/net/can/dev/netlink.c    | 29 +++++++++++++++++++++++++++--
>>   include/uapi/linux/can/netlink.h |  1 +
>>   3 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>> index 1de5babcc4f3..0c16d0174f7f 100644
>> --- a/drivers/net/can/dev/dev.c
>> +++ b/drivers/net/can/dev/dev.c
>> @@ -125,6 +125,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>>           return "xl-tdc-manual";
>>       case CAN_CTRLMODE_XL_TMS:
>>           return "xl-tms";
>> +    case CAN_CTRLMODE_XL_ERR_SIGNAL:
>> +        return "xl-error-signalling";
>>       default:
>>           return "<unknown>";
>>       }
>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>> index 8afd2baa03cf..6126b191fea0 100644
>> --- a/drivers/net/can/dev/netlink.c
>> +++ b/drivers/net/can/dev/netlink.c
>> @@ -191,7 +191,8 @@ static int can_validate_xl_flags(struct netlink_ext_ack
>> *extack,
>>           }
>>           if (masked_flags & CAN_CTRLMODE_XL_TMS) {
>>               const u32 tms_conflicts_mask = CAN_CTRLMODE_FD |
>> -                CAN_CTRLMODE_XL_TDC_MASK;
>> +                CAN_CTRLMODE_XL_TDC_MASK |
>> +                CAN_CTRLMODE_XL_ERR_SIGNAL;
>>               u32 tms_conflicts = masked_flags & tms_conflicts_mask;
>>                 if (tms_conflicts) {
>> @@ -201,11 +202,23 @@ static int can_validate_xl_flags(struct netlink_ext_ack
>> *extack,
>>                   return -EOPNOTSUPP;
>>               }
>>           }
>> +        if ((masked_flags & CAN_CTRLMODE_FD) &&
>> +            (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
>> +            !(masked_flags & CAN_CTRLMODE_XL_ERR_SIGNAL)) {
>> +            NL_SET_ERR_MSG(extack,
>> +                       "When using both CAN FD and XL, error signalling must
>> be on");

I changed that error message to:

	NL_SET_ERR_MSG(extack, "Mixed mode requires error signalling");

> This implicitly tells us that mixed-mode is CC/FD/XL ;-)

I was under the assumption that Classical CAN was always allowed, even under
TMS. The arbitration still uses the nominal bittiming anyway, so I still have
some issue understanding why an XL nodes operating under TMS wouldn't be able to
send a classical CAN frame.

The restriction seems rather arbitrary to me. I would be curious to understand
what the issue would be to allow Classical CAN under TMS.

>  > +            return -EOPNOTSUPP;
>> +        }
>>       } else {
>>           if (mask & CAN_CTRLMODE_XL_TMS) {
>>               NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
>>               return -EOPNOTSUPP;
>>           }
>> +        if (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) {
>> +            NL_SET_ERR_MSG(extack,
>> +                       "Error signalling is only configurable with CAN XL");
>> +            return -EOPNOTSUPP;
>> +        }
>>       }
>>         return 0;
>> @@ -310,6 +323,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>                          "TMS can not be activated while CAN FD is on");
>>               return -EOPNOTSUPP;
>>           }
>> +        if (deactivated & CAN_CTRLMODE_XL_ERR_SIGNAL) {
>> +            NL_SET_ERR_MSG(extack,
>> +                       "Error signalling can not be deactivated while CAN FD
>> is on");
>> +            return -EOPNOTSUPP;
>> +        }
>>       }
>>         /* If a top dependency flag is provided, reset all its dependencies */
>> @@ -317,12 +335,19 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>           priv->ctrlmode &= ~CAN_CTRLMODE_FD_TDC_MASK;
>>       if (cm->mask & CAN_CTRLMODE_XL)
>>           priv->ctrlmode &= ~(CAN_CTRLMODE_XL_TDC_MASK |
>> -                    CAN_CTRLMODE_XL_TMS);
>> +                    CAN_CTRLMODE_XL_TMS |
>> +                    CAN_CTRLMODE_XL_ERR_SIGNAL);
>>         /* clear bits to be modified and copy the flag values */
>>       priv->ctrlmode &= ~cm->mask;
>>       priv->ctrlmode |= maskedflags;
>>   +    /* If omitted, set error signalling on if possible */
>> +    if ((maskedflags & CAN_CTRLMODE_XL) &&
>> +        !(cm->mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
>> +        !(priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
>> +        priv->ctrlmode |= CAN_CTRLMODE_XL_ERR_SIGNAL;
>> +
>>       /* Wipe potential leftovers from previous CAN FD/XL config */
>>       if (!(priv->ctrlmode & CAN_CTRLMODE_FD)) {
>>           memset(&priv->fd.data_bittiming, 0,
>> diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
>> index ebafb091d80f..30d446921dc4 100644
>> --- a/include/uapi/linux/can/netlink.h
>> +++ b/include/uapi/linux/can/netlink.h
>> @@ -108,6 +108,7 @@ struct can_ctrlmode {
>>   #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 */
>> +#define CAN_CTRLMODE_XL_ERR_SIGNAL    0x10000    /* XL error signalling */
>>     /*
>>    * CAN device statistics
>>
> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>

Thanks!


Yours sincerely,
Vincent Mailhol
Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
Posted by Oliver Hartkopp 3 months ago

On 09.11.25 15:54, Vincent Mailhol wrote:
> On 06/11/2025 at 09:50, Oliver Hartkopp wrote:
>> On 21.10.25 17:47, Vincent Mailhol wrote:
>>> Classical CAN and CAN FD must generate error frames on the CAN bus
>>> when detecting a protocol violation.
>>>
>>> CAN XL's error signaling is different and works as follows:
>>>
>>>     - In interoperability mode (both FD and XL), error signaling must be
>>>       on.
>>>
>>>     - When operating a CAN controller in CAN XL only mode but with TMS
>>>       off, the user can decide whether the error signalling is enabled
>>>       or disabled.
>>>
>>>     - On the contrary, when using TMS, error signalling must be off.
>>>
>>> Introduce the new CAN_CTRLMODE_XL_ERR_SIGNAL control mode. This new
>>> option is only made available for CAN XL, so despite the error
>>> signalling being always on for Classical CAN and CAN FD, forbid the
>>> use of this flag when CAN XL is off.
>>>
>>> If the user provides the error signalling flag, check its validity. If
>>> the flag is omitted, activate error signalling by default whenever
>>> possible. This is summarized in below table:
>>>
>>>              CAN_CTRLMODE_XL_ERR_SIGNAL
>>>      -------------------------------------------
>>>      CC/FD        option not available
>>>      CC/FD/XL    on
>>
>> Yes. This is the 'mixed-mode'
>> I would propose to use the 'mixed-mode' expression in the patch description.
> 
> Ack!
> 
>>>      XL TMS off    configurable (default on)
>>
>> Good default.
>>
>>>      XL TMS on    off
>>>
>>> Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>> Link: https://lore.kernel.org/linux-can/20250527195625.65252-9-
>>> socketcan@hartkopp.net/
>>> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
>>> ---
>>>    drivers/net/can/dev/dev.c        |  2 ++
>>>    drivers/net/can/dev/netlink.c    | 29 +++++++++++++++++++++++++++--
>>>    include/uapi/linux/can/netlink.h |  1 +
>>>    3 files changed, 30 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>>> index 1de5babcc4f3..0c16d0174f7f 100644
>>> --- a/drivers/net/can/dev/dev.c
>>> +++ b/drivers/net/can/dev/dev.c
>>> @@ -125,6 +125,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>>>            return "xl-tdc-manual";
>>>        case CAN_CTRLMODE_XL_TMS:
>>>            return "xl-tms";
>>> +    case CAN_CTRLMODE_XL_ERR_SIGNAL:
>>> +        return "xl-error-signalling";
>>>        default:
>>>            return "<unknown>";
>>>        }
>>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>>> index 8afd2baa03cf..6126b191fea0 100644
>>> --- a/drivers/net/can/dev/netlink.c
>>> +++ b/drivers/net/can/dev/netlink.c
>>> @@ -191,7 +191,8 @@ static int can_validate_xl_flags(struct netlink_ext_ack
>>> *extack,
>>>            }
>>>            if (masked_flags & CAN_CTRLMODE_XL_TMS) {
>>>                const u32 tms_conflicts_mask = CAN_CTRLMODE_FD |
>>> -                CAN_CTRLMODE_XL_TDC_MASK;
>>> +                CAN_CTRLMODE_XL_TDC_MASK |
>>> +                CAN_CTRLMODE_XL_ERR_SIGNAL;
>>>                u32 tms_conflicts = masked_flags & tms_conflicts_mask;
>>>                  if (tms_conflicts) {
>>> @@ -201,11 +202,23 @@ static int can_validate_xl_flags(struct netlink_ext_ack
>>> *extack,
>>>                    return -EOPNOTSUPP;
>>>                }
>>>            }
>>> +        if ((masked_flags & CAN_CTRLMODE_FD) &&
>>> +            (mask & CAN_CTRLMODE_XL_ERR_SIGNAL) &&
>>> +            !(masked_flags & CAN_CTRLMODE_XL_ERR_SIGNAL)) {
>>> +            NL_SET_ERR_MSG(extack,
>>> +                       "When using both CAN FD and XL, error signalling must
>>> be on");
> 
> I changed that error message to:
> 
> 	NL_SET_ERR_MSG(extack, "Mixed mode requires error signalling");
> 

Good!

>> This implicitly tells us that mixed-mode is CC/FD/XL ;-)
> 
> I was under the assumption that Classical CAN was always allowed, even under
> TMS. The arbitration still uses the nominal bittiming anyway, so I still have
> some issue understanding why an XL nodes operating under TMS wouldn't be able to
> send a classical CAN frame.

TMS is XL-Only and has error-signalling off. Therefore no CC/FD traffic.

> The restriction seems rather arbitrary to me. I would be curious to understand
> what the issue would be to allow Classical CAN under TMS.

I tried to clarify this in the other answer.

Best regards,
Oliver