linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] can: netlink: add CAN XL
@ 2025-10-21 15:47 Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 01/10] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Vincent Mailhol
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

Following all the refactoring on the CAN netlink done in series [1],
[2] and [3], this is now time to finally introduce the CAN XL netlink
interface.

Similarly to how CAN FD reuses the bittiming logic of Classical CAN,
CAN XL also reuses the entirety of CAN FD features, and, on top of
that, adds new features which are specific to CAN XL.

Patch #1 is a small clean-up which makes can_calc_bittiming() use
NL_SET_ERR_MSG() instead of netdev_err().

Patch #2 adds a check in can_dev_dropped_skb() to drop CAN FD frames
when CAN FD is turned off.

Patch #3 adds CAN_CTRLMODE_RESTRICTED. Note that contrary to the other
CAN_CTRL_MODE_XL_* that are introduced in the later patches, this
control mode is not specific to CAN XL. The nuance is that because
this restricted mode was only added in ISO 11898-1:2024, it is made
mandatory for CAN XL devices but optional for other protocols. This is
why this patch is added as a preparation before introducing the core
CAN XL logic.

Patch #4 adds all the CAN XL features which are inherited from CAN FD:
the nominal bittiming, the data bittiming and the TDC.

Patch #5 and #6 add two new CAN control modes which are specific to
CAN XL: CAN_CTRLMODE_XL_TMS, CAN_CTRLMODE_XL_ERR_SIGNAL respectively.

Finally, patch #7 to #10 add the PWM logic.

[1] can: netlink: preparation before introduction of CAN XL
Link: https://lore.kernel.org/linux-can/20241112165118.586613-7-mailhol.vincent@wanadoo.fr/

[2] can: rework the CAN MTU logic (CAN XL preparation step 2/3)
Link: https://lore.kernel.org/linux-can/20250923-can-fix-mtu-v3-0-581bde113f52@kernel.org/

[3] can: netlink: preparation before introduction of CAN XL step 3/3
Link: https://lore.kernel.org/linux-can/20250923-canxl-netlink-prep-v4-0-e720d28f66fe@kernel.org/

---
Changes in v2:

  - Add a new patch #1.

  - In patch #9, add a return statement to can_calc_tdco() when
    CONFIG_CAN_CALC_BITTIMING is not set. This fixes a warning as
    reported by the kernel test robot:

      Link: https://lore.kernel.org/linux-can/202510140553.qo3f0I9s-lkp@intel.com/

    While at it, add an error message.

Link to v1: https://lore.kernel.org/r/20251013-canxl-netlink-v1-0-f422b7e2729f@kernel.org

Changes in v1:

   - Add PWM

   - Add the CAN_CTRLMODE_RESTRICTED, CAN_CTRLMODE_XL_TMS and
     CAN_CTRLMODE_XL_ERR_SIGNAL control modes.

   - A lot has changed since the original RFC was sent in November
     last year.  The preparation patches went in a separate series as
     explained in the cover letter, and what used to be a single patch
     to introduce CAN XL is now a full series. A few additional
     details are added to the individual patches, but overall I did
     not keep track of all the changes over the last year. You may as
     well consider this as a new series.
   
Link to RFC: https://lore.kernel.org/linux-can/20241110155902.72807-16-mailhol.vincent@wanadoo.fr/

---
Vincent Mailhol (10):
      can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
      can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
      can: netlink: add CAN_CTRLMODE_RESTRICTED
      can: netlink: add initial CAN XL support
      can: netlink: add CAN_CTRLMODE_XL_TMS flag
      can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
      can: bittiming: add PWM parameters
      can: bittiming: add PWM validation
      can: calc_bittiming: add PWM calculation
      can: netlink: add PWM netlink interface

 drivers/net/can/dev/bittiming.c      |  63 +++++++
 drivers/net/can/dev/calc_bittiming.c |  36 ++++
 drivers/net/can/dev/dev.c            |  20 +-
 drivers/net/can/dev/netlink.c        | 357 +++++++++++++++++++++++++++++++++--
 include/linux/can/bittiming.h        |  81 +++++++-
 include/linux/can/dev.h              |  49 +++--
 include/uapi/linux/can/netlink.h     |  35 ++++
 7 files changed, 599 insertions(+), 42 deletions(-)
---
base-commit: ffee675aceb9f44b0502a8bec912abb0c4f4af62
change-id: 20241229-canxl-netlink-bc640af10673

Best regards,
-- 
Vincent Mailhol <mailhol@kernel.org>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 01/10] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

When CONFIG_CAN_CALC_BITTIMING is disabled, the can_calc_bittiming()
functions can not be used and the user needs to provide all the
bittiming parameters.

Currently, can_calc_bittiming() prints an error message to the kernel
log. Instead use NL_SET_ERR_MSG() to make it return the error message
through the netlink interface so that the user can directly see it.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Changelog:

  v1 -> v2:

    - New patch.
---
 include/linux/can/bittiming.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index d30816dd93c7..3926c78b2222 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -141,7 +141,7 @@ static inline int
 can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 		   const struct can_bittiming_const *btc, struct netlink_ext_ack *extack)
 {
-	netdev_err(dev, "bit-timing calculation not available\n");
+	NL_SET_ERR_MSG(extack, "bit-timing calculation not available\n");
 	return -EINVAL;
 }
 

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 01/10] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 03/10] can: netlink: add CAN_CTRLMODE_RESTRICTED Vincent Mailhol
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

Currently, the CAN FD skb validation logic is based on the MTU: the
interface is deemed FD capable if and only if its MTU is greater or
equal to CANFD_MTU.

This logic is showing its limit with the introduction of CAN XL. For
example, consider the two scenarios below:

  1. An interface configured with CAN FD on and CAN XL on

  2. An interface configured with CAN FD off and CAN XL on

In those two scenarios, the interfaces would have the same MTU:

  CANXL_MTU

making it impossible to differentiate which one has CAN FD turned on
and which one has it off.

Because of the limitation, the only non-UAPI-breaking workaround is to
do the check at the device level using the can_priv->ctrlmode flags.
Unfortunately, the virtual interfaces (vcan, vxcan), which do not have
a can_priv, are left behind.

Add a check on the CAN_CTRLMODE_FD flag in can_dev_dropped_skb() and
drop FD frames whenever the feature is turned off.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Changelog:

RFC -> v1:

  - add an netdev_info_once() message.

  - this was sent as a standalone patch for discussion, it is now
    integrated in the CAN XL series.

  Link: https://lore.kernel.org/linux-can/20250907080504.598419-2-mailhol@kernel.org/
---
 include/linux/can/dev.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 0fe8f80f223e..d59b283c981a 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -103,12 +103,20 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
 	if (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		netdev_info_once(dev,
 				 "interface in listen only mode, dropping skb\n");
-		kfree_skb(skb);
-		dev->stats.tx_dropped++;
-		return true;
+		goto invalid_skb;
+	}
+
+	if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
+		netdev_info_once(dev, "CAN FD is disabled, dropping skb\n");
+		goto invalid_skb;
 	}
 
 	return can_dropped_invalid_skb(dev, skb);
+
+invalid_skb:
+	kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return true;
 }
 
 void can_setup(struct net_device *dev);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 03/10] can: netlink: add CAN_CTRLMODE_RESTRICTED
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 01/10] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 04/10] can: netlink: add initial CAN XL support Vincent Mailhol
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

ISO 11898-1:2024 adds a new restricted operation mode. This mode is
added as a mandatory feature for nodes which support CAN XL and is
retrofitted as optional for legacy nodes (i.e. the ones which only
support Classical CAN and CAN FD).

The restricted operation mode is nearly the same as the listen only
mode: the node can not send data frames or remote frames and can not
send dominant bits if an error occurs. The only exception is that the
node shall still send the acknowledgment bit. A second niche exception
is that the node may still send a data frame containing a time
reference message if the node is a primary time provider, but because
the time provider feature is not yet implemented in the kernel, this
second exception is not relevant to us at the moment.

Add the CAN_CTRLMODE_RESTRICTED control mode flag and update the
can_dev_dropped_skb() helper function accordingly.

Finally, bail out if both CAN_CTRLMODE_LISTENONLY and
CAN_CTRLMODE_RESTRICTED are provided.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/dev/dev.c        |  2 ++
 drivers/net/can/dev/netlink.c    |  7 ++++++
 include/linux/can/dev.h          | 50 +++++++++++++++++++++-------------------
 include/uapi/linux/can/netlink.h |  1 +
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 0cc3d008adb3..3377afb6f1c4 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -115,6 +115,8 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
 		return "fd-tdc-auto";
 	case CAN_CTRLMODE_TDC_MANUAL:
 		return "fd-tdc-manual";
+	case CAN_CTRLMODE_RESTRICTED:
+		return "restricted-operation";
 	default:
 		return "<unknown>";
 	}
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 0591406b6f32..f44b5dffa176 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -188,6 +188,13 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
 		struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]);
 
 		flags = cm->flags & cm->mask;
+
+		if ((flags & CAN_CTRLMODE_LISTENONLY) &&
+		    (flags & CAN_CTRLMODE_RESTRICTED)) {
+			NL_SET_ERR_MSG(extack,
+				       "Listen-only and restricted modes are mutually exclusive");
+			return -EOPNOTSUPP;
+		}
 	}
 
 	err = can_validate_bittiming(data, extack, IFLA_CAN_BITTIMING);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index d59b283c981a..9de8fde3ec9d 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -95,30 +95,6 @@ static inline bool can_is_canxl_dev_mtu(unsigned int mtu)
 	return (mtu >= CANXL_MIN_MTU && mtu <= CANXL_MAX_MTU);
 }
 
-/* drop skb if it does not contain a valid CAN frame for sending */
-static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *skb)
-{
-	struct can_priv *priv = netdev_priv(dev);
-
-	if (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY) {
-		netdev_info_once(dev,
-				 "interface in listen only mode, dropping skb\n");
-		goto invalid_skb;
-	}
-
-	if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
-		netdev_info_once(dev, "CAN FD is disabled, dropping skb\n");
-		goto invalid_skb;
-	}
-
-	return can_dropped_invalid_skb(dev, skb);
-
-invalid_skb:
-	kfree_skb(skb);
-	dev->stats.tx_dropped++;
-	return true;
-}
-
 void can_setup(struct net_device *dev);
 
 struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
@@ -150,6 +126,32 @@ void can_bus_off(struct net_device *dev);
 const char *can_get_state_str(const enum can_state state);
 const char *can_get_ctrlmode_str(u32 ctrlmode);
 
+/* drop skb if it does not contain a valid CAN frame for sending */
+static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *skb)
+{
+	struct can_priv *priv = netdev_priv(dev);
+	u32 silent_mode = priv->ctrlmode & (CAN_CTRLMODE_LISTENONLY |
+					    CAN_CTRLMODE_RESTRICTED);
+
+	if (silent_mode) {
+		netdev_info_once(dev, "interface in %s mode, dropping skb\n",
+				 can_get_ctrlmode_str(silent_mode));
+		goto invalid_skb;
+	}
+
+	if (!(priv->ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
+		netdev_info_once(dev, "CAN FD is disabled, dropping skb\n");
+		goto invalid_skb;
+	}
+
+	return can_dropped_invalid_skb(dev, skb);
+
+invalid_skb:
+	kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return true;
+}
+
 void can_state_get_by_berr_counter(const struct net_device *dev,
 				   const struct can_berr_counter *bec,
 				   enum can_state *tx_state,
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index ef62f56eaaef..fafd1cce4798 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -103,6 +103,7 @@ struct can_ctrlmode {
 #define CAN_CTRLMODE_CC_LEN8_DLC	0x100	/* Classic CAN DLC option */
 #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 */
 
 /*
  * CAN device statistics

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 04/10] can: netlink: add initial CAN XL support
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (2 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 03/10] can: netlink: add CAN_CTRLMODE_RESTRICTED Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag Vincent Mailhol
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

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;
+	}
+
 	/* 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 3926c78b2222..b6cd2476ffd7 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,

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (3 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 04/10] can: netlink: add initial CAN XL support Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-11-06  8:42   ` Oliver Hartkopp
  2025-10-21 15:47 ` [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL Vincent Mailhol
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

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.

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@kernel.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():

	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;
	}

My current assumption is that this is possible. But the standard being
not crystal clear on that point, I want to double check this with you!
---
 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

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (4 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-11-06  8:50   ` Oliver Hartkopp
  2025-10-21 15:47 ` [PATCH v2 07/10] can: bittiming: add PWM parameters Vincent Mailhol
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

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
	XL TMS off	configurable (default on)
	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");
+			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

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 07/10] can: bittiming: add PWM parameters
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (5 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 08/10] can: bittiming: add PWM validation Vincent Mailhol
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

In CAN XL, higher data bit rates require the CAN transceiver to switch
its operation mode to use Pulse-Width Modulation (PWM) transmission
mode instead of the classic dominant/recessive transmission mode.

The PWM parameters are:

  - PWMS: pulse width modulation short phase
  - PWML: pulse width modulation long phase
  - PWMO: pulse width modulation offset

CiA 612-2 specifies PWMS and PWML to be at least 1 (arguably, PWML
shall be at least 2 to respect the PWMS < PWML rule). PWMO's minimum
is expected to always be zero. It is added more for consistency than
anything else.

Add struct can_pwm_const so that the different devices can provide
their minimum and maximum values.

When TMS is on, the runtime PWMS, PWML and PWMO are needed (either
calculated or provided by the user): add struct can_pwm to store
these.

TDC and PWM can not be used at the same time (TDC can only be used
when TMS is off and PWM only when TMS is on). struct can_pwm is thus
put together with struct can_tdc inside a union to save some space.

The netlink logic will be added in an upcoming change.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 include/linux/can/bittiming.h | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index b6cd2476ffd7..967d76689c4f 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /* Copyright (c) 2020 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
- * Copyright (c) 2021 Vincent Mailhol <mailhol.vincent@wanadoo.fr>
+ * Copyright (c) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
  */
 
 #ifndef _CAN_BITTIMING_H
@@ -120,11 +120,48 @@ struct can_tdc_const {
 	u32 tdcf_max;
 };
 
+/*
+ * struct can_pwm - CAN Pulse-Width Modulation (PWM) parameters
+ *
+ * @pwms: pulse width modulation short phase
+ * @pwml: pulse width modulation long phase
+ * @pwmo: pulse width modulation offset
+ */
+struct can_pwm {
+	u32 pwms;
+	u32 pwml;
+	u32 pwmo;
+};
+
+/*
+ * struct can_pwm - CAN hardware-dependent constants for Pulse-Width
+ *	Modulation (PWM)
+ *
+ * @pwms_min: PWM short phase minimum value. Must be at least 1.
+ * @pwms_max: PWM short phase maximum value
+ * @pwml_min: PWM long phase minimum value. Must be at least 1.
+ * @pwml_max: PWM long phase maximum value
+ * @pwmo_min: PWM offset phase minimum value
+ * @pwmo_max: PWM offset phase maximum value
+ */
+struct can_pwm_const {
+	u32 pwms_min;
+	u32 pwms_max;
+	u32 pwml_min;
+	u32 pwml_max;
+	u32 pwmo_min;
+	u32 pwmo_max;
+};
+
 struct data_bittiming_params {
 	const struct can_bittiming_const *data_bittiming_const;
 	struct can_bittiming data_bittiming;
 	const struct can_tdc_const *tdc_const;
-	struct can_tdc tdc;
+	const struct can_pwm_const *pwm_const;
+	union {
+		struct can_tdc tdc;
+		struct can_pwm pwm;
+	};
 	const u32 *data_bitrate_const;
 	unsigned int data_bitrate_const_cnt;
 	int (*do_set_data_bittiming)(struct net_device *dev);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 08/10] can: bittiming: add PWM validation
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (6 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 07/10] can: bittiming: add PWM parameters Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 09/10] can: calc_bittiming: add PWM calculation Vincent Mailhol
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

Add can_validate_pwm() to validate the values pwms, pwml and pwml.
Error messages are added to each of the checks to inform the user on
what went wrong. Refer to those error messages to understand the
validation logic.

The boundary values CAN_PWM_DECODE_NS (the transceiver minimum
decoding margin) and CAN_PWM_NS_MAX (the maximum PWM symbol duration)
are hardcoded for the moment. Note that a transceiver capable of
bitrates higher than 20 Mbps may be able to handle a CAN_PWM_DECODE_NS
below 5 ns. If such transceivers become commercially available, this
code could be revisited to make this parameter configurable. For now,
leave it static.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/dev/bittiming.c | 63 +++++++++++++++++++++++++++++++++++++++++
 include/linux/can/bittiming.h   | 22 ++++++++++++++
 2 files changed, 85 insertions(+)

diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c
index 0b93900b1dfa..730b1b254460 100644
--- a/drivers/net/can/dev/bittiming.c
+++ b/drivers/net/can/dev/bittiming.c
@@ -2,6 +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) 2025 Vincent Mailhol <mailhol@kernel.org>
  */
 
 #include <linux/can/dev.h>
@@ -151,3 +152,65 @@ int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 
 	return -EINVAL;
 }
+
+int can_validate_pwm_bittiming(const struct net_device *dev,
+			       const struct can_pwm *pwm,
+			       struct netlink_ext_ack *extack)
+{
+	const struct can_priv *priv = netdev_priv(dev);
+	u32 xl_bit_time_tqmin = can_bit_time_tqmin(&priv->xl.data_bittiming);
+	u32 nom_bit_time_tqmin = can_bit_time_tqmin(&priv->bittiming);
+	u32 pwms_ns = can_tqmin_to_ns(pwm->pwms, priv->clock.freq);
+	u32 pwml_ns = can_tqmin_to_ns(pwm->pwml, priv->clock.freq);
+
+	if (pwms_ns + pwml_ns > CAN_PWM_NS_MAX) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "The PWM symbol duration: %u ns may no exceed %u ns",
+				   pwms_ns + pwml_ns, CAN_PWM_NS_MAX);
+		return -EINVAL;
+	}
+
+	if (pwms_ns < CAN_PWM_DECODE_NS) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "PWMS: %u ns shall be at least %u ns",
+				   pwms_ns, CAN_PWM_DECODE_NS);
+		return -EINVAL;
+	}
+
+	if (pwm->pwms >= pwm->pwml) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "PWMS: %u tqmin shall be smaller than PWML: %u tqmin",
+				   pwm->pwms, pwm->pwml);
+		return -EINVAL;
+	}
+
+	if (pwml_ns - pwms_ns < 2 * CAN_PWM_DECODE_NS) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "At least %u ns shall separate PWMS: %u ns from PMWL: %u ns",
+				   2 * CAN_PWM_DECODE_NS, pwms_ns, pwml_ns);
+		return -EINVAL;
+	}
+
+	if (xl_bit_time_tqmin % (pwm->pwms + pwm->pwml) != 0) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "PWM duration: %u tqmin does not divide XL's bit time: %u tqmin",
+				   pwm->pwms + pwm->pwml, xl_bit_time_tqmin);
+		return -EINVAL;
+	}
+
+	if (pwm->pwmo >= pwm->pwms + pwm->pwml) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "PWMO: %u tqmin can not be greater than PWMS + PWML: %u tqmin",
+				   pwm->pwmo, pwm->pwms + pwm->pwml);
+		return -EINVAL;
+	}
+
+	if (nom_bit_time_tqmin % (pwm->pwms + pwm->pwml) != pwm->pwmo) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "Can not assemble nominal bit time: %u tqmin out of PWMS + PMWL and PWMO",
+				   nom_bit_time_tqmin);
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index 967d76689c4f..2504fafc72e4 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -87,6 +87,11 @@ struct can_tdc {
 	u32 tdcf;
 };
 
+/* The transceiver decoding margin corresponds to t_Decode in ISO 11898-2 */
+#define CAN_PWM_DECODE_NS 5
+/* Maximum PWM symbol duration. Corresponds to t_SymbolNom_MAX - t_Decode */
+#define CAN_PWM_NS_MAX (205 - CAN_PWM_DECODE_NS)
+
 /*
  * struct can_tdc_const - CAN hardware-dependent constant for
  *	Transmission Delay Compensation
@@ -203,6 +208,10 @@ int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 		      const unsigned int bitrate_const_cnt,
 		      struct netlink_ext_ack *extack);
 
+int can_validate_pwm_bittiming(const struct net_device *dev,
+			       const struct can_pwm *pwm,
+			       struct netlink_ext_ack *extack);
+
 /*
  * can_get_relative_tdco() - TDCO relative to the sample point
  *
@@ -245,4 +254,17 @@ static inline unsigned int can_bit_time(const struct can_bittiming *bt)
 	return CAN_SYNC_SEG + bt->prop_seg + bt->phase_seg1 + bt->phase_seg2;
 }
 
+/* Duration of one bit in minimum time quantum */
+static inline unsigned int can_bit_time_tqmin(const struct can_bittiming *bt)
+{
+	return can_bit_time(bt) * bt->brp;
+}
+
+/* Convert a duration from minimum a minimum time quantum to nano seconds */
+static inline u32 can_tqmin_to_ns(u32 tqmin, u32 clock_freq)
+{
+	return DIV_U64_ROUND_CLOSEST(mul_u32_u32(tqmin, NSEC_PER_SEC),
+				     clock_freq);
+}
+
 #endif /* !_CAN_BITTIMING_H */

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 09/10] can: calc_bittiming: add PWM calculation
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (7 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 08/10] can: bittiming: add PWM validation Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-21 15:47 ` [PATCH v2 10/10] can: netlink: add PWM netlink interface Vincent Mailhol
  2025-10-31 21:17 ` [PATCH v2 00/10] can: netlink: add CAN XL Oliver Hartkopp
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

Perform the PWM calculation according to CiA recommendations.

Note that for databitrates greater than 5 MBPS, tqmin is less than
CAN_PWM_NS_MAX (which is defined to 200 nano seconds), consequently,
the result of the division:

  DIV_ROUND_UP(xl_ns, CAN_PWM_NS_MAX)

is one and thus the for loop automatically stops on the first
iteration giving a single PWM symbol per bit as expected. Because of
that, there is no actual need for a separate conditional branch for
when the databitrate is greater than 5 MBPS.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Changelog:

  v1 -> v2:

    - Add a return statement to can_calc_tdco() when
      CONFIG_CAN_CALC_BITTIMING is not set. This fixes a warning as
      reported by the kernel test robot:

        Link: https://lore.kernel.org/linux-can/202510140553.qo3f0I9s-lkp@intel.com/

      While at it, add an error message.
---
 drivers/net/can/dev/calc_bittiming.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/can/bittiming.h        | 10 ++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 394d6974f481..268ec6fa7c49 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -2,6 +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-2025 Vincent Mailhol <mailhol@kernel.org>
  */
 
 #include <linux/units.h>
@@ -198,3 +199,38 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 		*ctrlmode |= tdc_auto;
 	}
 }
+
+int can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
+{
+	struct can_priv *priv = netdev_priv(dev);
+	const struct can_pwm_const *pwm_const = priv->xl.pwm_const;
+	struct can_pwm *pwm = &priv->xl.pwm;
+	u32 xl_tqmin = can_bit_time_tqmin(&priv->xl.data_bittiming);
+	u32 xl_ns = can_tqmin_to_ns(xl_tqmin, priv->clock.freq);
+	u32 nom_tqmin = can_bit_time_tqmin(&priv->bittiming);
+	int pwm_per_bit_max = xl_tqmin / (pwm_const->pwms_min + pwm_const->pwml_min);
+	int pwm_per_bit;
+	u32 pwm_tqmin;
+
+	/* For 5 MB/s databitrate or greater, xl_ns < CAN_PWM_NS_MAX
+	 * giving us a pwm_per_bit of 1 and the loop immediately breaks
+	 */
+	for (pwm_per_bit = DIV_ROUND_UP(xl_ns, CAN_PWM_NS_MAX);
+	     pwm_per_bit <= pwm_per_bit_max; pwm_per_bit++)
+		if (xl_tqmin % pwm_per_bit == 0)
+			break;
+
+	if (pwm_per_bit > pwm_per_bit_max) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "Can not divide the XL data phase's bit time: %u tqmin into multiple PWM symbols",
+				   xl_tqmin);
+		return -EINVAL;
+	}
+
+	pwm_tqmin = xl_tqmin / pwm_per_bit;
+	pwm->pwms = DIV_ROUND_UP_POW2(pwm_tqmin, 4);
+	pwm->pwml = pwm_tqmin - pwm->pwms;
+	pwm->pwmo = nom_tqmin % pwm_tqmin;
+
+	return 0;
+}
diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h
index 2504fafc72e4..726d909e87ce 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -180,6 +180,8 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
 void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 		   const struct can_bittiming *dbt,
 		   u32 tdc_mask, u32 *ctrlmode, u32 ctrlmode_supported);
+
+int can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack);
 #else /* !CONFIG_CAN_CALC_BITTIMING */
 static inline int
 can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
@@ -195,6 +197,14 @@ can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
 	      u32 tdc_mask, u32 *ctrlmode, u32 ctrlmode_supported)
 {
 }
+
+static inline int
+can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
+{
+	NL_SET_ERR_MSG(extack,
+		       "bit-timing calculation not available: manually provide PWML and PWMS\n");
+	return -EINVAL;
+}
 #endif /* CONFIG_CAN_CALC_BITTIMING */
 
 void can_sjw_set_default(struct can_bittiming *bt);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 10/10] can: netlink: add PWM netlink interface
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (8 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 09/10] can: calc_bittiming: add PWM calculation Vincent Mailhol
@ 2025-10-21 15:47 ` Vincent Mailhol
  2025-10-31 21:17 ` [PATCH v2 00/10] can: netlink: add CAN XL Oliver Hartkopp
  10 siblings, 0 replies; 23+ messages in thread
From: Vincent Mailhol @ 2025-10-21 15:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Oliver Hartkopp
  Cc: Vincent Mailhol, Stéphane Grosjean, Robert Nawrath, Minh Le,
	Duy Nguyen, linux-can, linux-kernel

When the TMS is switched on, the node uses PWM (Pulse Width
Modulation) during the data phase instead of the classic NRZ (Non
Return to Zero) encoding.

PWM is configured by three parameters:

  - PWMS: Pulse Width Modulation Short phase
  - PWML: Pulse Width Modulation Long phase
  - PWMO: Pulse Width Modulation Offset time

For each of these parameters, define three IFLA symbols:

  - IFLA_CAN_PWM_PWM*_MIN: the minimum allowed value.
  - IFLA_CAN_PWM_PWM*_MAX: the maximum allowed value.
  - IFLA_CAN_PWM_PWM*: the runtime value.

This results in a total of nine IFLA symbols which are all nested in a
parent IFLA_CAN_XL_PWM symbol.

IFLA_CAN_PWM_PWM*_MIN and IFLA_CAN_PWM_PWM*_MAX define the range of
allowed values and will match the value statically configured by the
device in struct can_pwm_const.

IFLA_CAN_PWM_PWM* match the runtime values stored in struct can_pwm.
Those parameters may only be configured when the tms mode is on. If
the PWMS, PWML and PWMO parameters are provided, check that all the
needed parameters are present using can_validate_pwm(), then check
their value using can_validate_pwm_bittiming(). PWMO defaults to zero
if omitted. Otherwise, if CAN_CTRLMODE_XL_TMS is true but none of the
PWM parameters are provided, calculate them using can_calc_pwm().

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 drivers/net/can/dev/netlink.c    | 192 ++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/can/netlink.h |  25 +++++
 2 files changed, 215 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 6126b191fea0..7f6d853fc550 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -25,6 +25,7 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
 	[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 },
+	[IFLA_CAN_XL_PWM] = { .type = NLA_NESTED },
 };
 
 static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
@@ -39,6 +40,18 @@ static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
 	[IFLA_CAN_TDC_TDCF] = { .type = NLA_U32 },
 };
 
+static const struct nla_policy can_pwm_policy[IFLA_CAN_PWM_MAX + 1] = {
+	[IFLA_CAN_PWM_PWMS_MIN] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWMS_MAX] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWML_MIN] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWML_MAX] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWMO_MIN] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWMO_MAX] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWMS] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWML] = { .type = NLA_U32 },
+	[IFLA_CAN_PWM_PWMO] = { .type = NLA_U32 },
+};
+
 static int can_validate_bittiming(struct nlattr *data[],
 				  struct netlink_ext_ack *extack,
 				  int ifla_can_bittiming)
@@ -119,6 +132,40 @@ static int can_validate_tdc(struct nlattr *data_tdc,
 	return 0;
 }
 
+static int can_validate_pwm(struct nlattr *data[],
+			    struct netlink_ext_ack *extack, u32 flags)
+{
+	struct nlattr *tb_pwm[IFLA_CAN_PWM_MAX + 1];
+	int err;
+
+	if (!data[IFLA_CAN_XL_PWM])
+		return 0;
+
+	if (!(flags & CAN_CTRLMODE_XL_TMS)) {
+		NL_SET_ERR_MSG(extack, "PWM requires TMS");
+		return -EOPNOTSUPP;
+	}
+
+	err = nla_parse_nested(tb_pwm, IFLA_CAN_PWM_MAX, data[IFLA_CAN_XL_PWM],
+			       can_pwm_policy, extack);
+	if (err)
+		return err;
+
+	if (!tb_pwm[IFLA_CAN_PWM_PWMS] != !tb_pwm[IFLA_CAN_PWM_PWML]) {
+		NL_SET_ERR_MSG(extack,
+			       "Provide either both PWMS and PWML, or none for automic calculation");
+		return -EOPNOTSUPP;
+	}
+
+	if (tb_pwm[IFLA_CAN_PWM_PWMO] &&
+	    (!tb_pwm[IFLA_CAN_PWM_PWMS] || !tb_pwm[IFLA_CAN_PWM_PWML])) {
+		NL_SET_ERR_MSG(extack, "PWMO requires both PWMS and PWML");
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
 static int can_validate_databittiming(struct nlattr *data[],
 				      struct netlink_ext_ack *extack,
 				      int ifla_can_data_bittiming, u32 flags)
@@ -264,6 +311,10 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
 	if (err)
 		return err;
 
+	err = can_validate_pwm(data, extack, flags);
+	if (err)
+		return err;
+
 	return 0;
 }
 
@@ -360,6 +411,7 @@ static int can_ctrlmode_changelink(struct net_device *dev,
 		       sizeof(priv->fd.data_bittiming));
 		priv->ctrlmode &= ~CAN_CTRLMODE_XL_TDC_MASK;
 		memset(&priv->xl.tdc, 0, sizeof(priv->xl.tdc));
+		memset(&priv->xl.pwm, 0, sizeof(priv->xl.pwm));
 	}
 
 	can_set_default_mtu(dev);
@@ -506,6 +558,76 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
 	return 0;
 }
 
+static int can_pwm_changelink(struct net_device *dev,
+			      const struct nlattr *pwm_nla,
+			      struct netlink_ext_ack *extack)
+{
+	struct can_priv *priv = netdev_priv(dev);
+	const struct can_pwm_const *pwm_const = priv->xl.pwm_const;
+	struct nlattr *tb_pwm[IFLA_CAN_PWM_MAX + 1];
+	struct can_pwm pwm = { 0 };
+	int err;
+
+	if (!(priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
+		return 0;
+
+	if (!pwm_const) {
+		NL_SET_ERR_MSG(extack, "The device does not support PWM");
+		return -EOPNOTSUPP;
+	}
+
+	if (!pwm_nla)
+		return can_calc_pwm(dev, extack);
+
+	err = nla_parse_nested(tb_pwm, IFLA_CAN_PWM_MAX, pwm_nla,
+			       can_pwm_policy, extack);
+	if (err)
+		return err;
+
+	if (tb_pwm[IFLA_CAN_PWM_PWMS]) {
+		pwm.pwms = nla_get_u32(tb_pwm[IFLA_CAN_PWM_PWMS]);
+		if (pwm.pwms < pwm_const->pwms_min ||
+		    pwm.pwms > pwm_const->pwms_max) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "PWMS: %u tqmin is out of range: %u...%u",
+					   pwm.pwms, pwm_const->pwms_min,
+					   pwm_const->pwms_max);
+			return -EINVAL;
+		}
+	}
+
+	if (tb_pwm[IFLA_CAN_PWM_PWML]) {
+		pwm.pwml = nla_get_u32(tb_pwm[IFLA_CAN_PWM_PWML]);
+		if (pwm.pwml < pwm_const->pwml_min ||
+		    pwm.pwml > pwm_const->pwml_max) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "PWML: %u tqmin is out of range: %u...%u",
+					   pwm.pwml, pwm_const->pwml_min,
+					   pwm_const->pwml_max);
+			return -EINVAL;
+		}
+	}
+
+	if (tb_pwm[IFLA_CAN_PWM_PWMO]) {
+		pwm.pwmo = nla_get_u32(tb_pwm[IFLA_CAN_PWM_PWMO]);
+		if (pwm.pwmo < pwm_const->pwmo_min ||
+		    pwm.pwmo > pwm_const->pwmo_max) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "PWMO: %u tqmin is out of range: %u...%u",
+					   pwm.pwmo, pwm_const->pwmo_min,
+					   pwm_const->pwmo_max);
+			return -EINVAL;
+		}
+	}
+
+	err = can_validate_pwm_bittiming(dev, &pwm, extack);
+	if (err)
+		return err;
+
+	priv->xl.pwm = pwm;
+	return 0;
+}
+
 static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 			  struct nlattr *data[],
 			  struct netlink_ext_ack *extack)
@@ -595,6 +717,9 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 
 	/* CAN XL */
 	err = can_dbt_changelink(dev, data, false, extack);
+	if (err)
+		return err;
+	err = can_pwm_changelink(dev, data[IFLA_CAN_XL_PWM], extack);
 	if (err)
 		return err;
 
@@ -683,6 +808,30 @@ static size_t can_ctrlmode_ext_get_size(void)
 		nla_total_size(sizeof(u32));	/* IFLA_CAN_CTRLMODE_SUPPORTED */
 }
 
+static size_t can_pwm_get_size(const struct can_pwm_const *pwm_const,
+			       bool pwm_on)
+{
+	size_t size;
+
+	if (!pwm_const || !pwm_on)
+		return 0;
+
+	size = nla_total_size(0);			/* nest IFLA_CAN_PWM */
+
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMS_MIN */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMS_MAX */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWML_MIN */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWML_MAX */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MIN */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO_MAX */
+
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMS */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWML */
+	size += nla_total_size(sizeof(u32));		/* IFLA_CAN_PWM_PWMO */
+
+	return size;
+}
+
 static size_t can_get_size(const struct net_device *dev)
 {
 	struct can_priv *priv = netdev_priv(dev);
@@ -714,6 +863,8 @@ static size_t can_get_size(const struct net_device *dev)
 
 	size += can_data_bittiming_get_size(&priv->xl,
 					    priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK);
+	size += can_pwm_get_size(priv->xl.pwm_const,		/* IFLA_CAN_XL_PWM */
+				 priv->ctrlmode & CAN_CTRLMODE_XL_TMS);
 
 	return size;
 }
@@ -812,6 +963,42 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev,
 	return -EMSGSIZE;
 }
 
+static int can_pwm_fill_info(struct sk_buff *skb, const struct can_priv *priv)
+{
+	const struct can_pwm_const *pwm_const = priv->xl.pwm_const;
+	const struct can_pwm *pwm = &priv->xl.pwm;
+	struct nlattr *nest;
+
+	if (!pwm_const)
+		return 0;
+
+	nest = nla_nest_start(skb, IFLA_CAN_XL_PWM);
+	if (!nest)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(skb, IFLA_CAN_PWM_PWMS_MIN, pwm_const->pwms_min) ||
+	    nla_put_u32(skb, IFLA_CAN_PWM_PWMS_MAX, pwm_const->pwms_max) ||
+	    nla_put_u32(skb, IFLA_CAN_PWM_PWML_MIN, pwm_const->pwml_min) ||
+	    nla_put_u32(skb, IFLA_CAN_PWM_PWML_MAX, pwm_const->pwml_max) ||
+	    nla_put_u32(skb, IFLA_CAN_PWM_PWMO_MIN, pwm_const->pwmo_min) ||
+	    nla_put_u32(skb, IFLA_CAN_PWM_PWMO_MAX, pwm_const->pwmo_max))
+		goto err_cancel;
+
+	if (priv->ctrlmode & CAN_CTRLMODE_XL_TMS) {
+		if (nla_put_u32(skb, IFLA_CAN_PWM_PWMS, pwm->pwms) ||
+		    nla_put_u32(skb, IFLA_CAN_PWM_PWML, pwm->pwml) ||
+		    nla_put_u32(skb, IFLA_CAN_PWM_PWMO, pwm->pwmo))
+			goto err_cancel;
+	}
+
+	nla_nest_end(skb, nest);
+	return 0;
+
+err_cancel:
+	nla_nest_cancel(skb, nest);
+	return -EMSGSIZE;
+}
+
 static int can_ctrlmode_ext_fill_info(struct sk_buff *skb,
 				      const struct can_priv *priv)
 {
@@ -895,9 +1082,10 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
 					priv->xl.data_bitrate_const,
 					priv->xl.data_bitrate_const_cnt) ||
 
-	    can_tdc_fill_info(skb, dev, IFLA_CAN_XL_TDC)
-	    )
+	    can_tdc_fill_info(skb, dev, IFLA_CAN_XL_TDC) ||
 
+	    can_pwm_fill_info(skb, priv)
+	    )
 		return -EMSGSIZE;
 
 	return 0;
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index 30d446921dc4..4497e3b4210f 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -5,6 +5,7 @@
  * Definitions for the CAN netlink interface
  *
  * Copyright (c) 2009 Wolfgang Grandegger <wg@grandegger.com>
+ * Copyright (c) 2021-2025 Vincent Mailhol <mailhol@kernel.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the version 2 of the GNU General Public License
@@ -148,6 +149,7 @@ enum {
 	IFLA_CAN_XL_DATA_BITTIMING_CONST,
 	IFLA_CAN_XL_DATA_BITRATE_CONST,
 	IFLA_CAN_XL_TDC,
+	IFLA_CAN_XL_PWM,
 
 	/* add new constants above here */
 	__IFLA_CAN_MAX,
@@ -189,6 +191,29 @@ enum {
 	IFLA_CAN_CTRLMODE_MAX = __IFLA_CAN_CTRLMODE - 1
 };
 
+/*
+ * CAN FD/XL Pulse-Width Modulation (PWM)
+ *
+ * Please refer to struct can_pwm_const and can_pwm in
+ * include/linux/can/bittiming.h for further details.
+ */
+enum {
+	IFLA_CAN_PWM_UNSPEC,
+	IFLA_CAN_PWM_PWMS_MIN,	/* u32 */
+	IFLA_CAN_PWM_PWMS_MAX,	/* u32 */
+	IFLA_CAN_PWM_PWML_MIN,	/* u32 */
+	IFLA_CAN_PWM_PWML_MAX,	/* u32 */
+	IFLA_CAN_PWM_PWMO_MIN,	/* u32 */
+	IFLA_CAN_PWM_PWMO_MAX,	/* u32 */
+	IFLA_CAN_PWM_PWMS,	/* u32 */
+	IFLA_CAN_PWM_PWML,	/* u32 */
+	IFLA_CAN_PWM_PWMO,	/* u32 */
+
+	/* add new constants above here */
+	__IFLA_CAN_PWM,
+	IFLA_CAN_PWM_MAX = __IFLA_CAN_PWM - 1
+};
+
 /* u16 termination range: 1..65535 Ohms */
 #define CAN_TERMINATION_DISABLED 0
 

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
                   ` (9 preceding siblings ...)
  2025-10-21 15:47 ` [PATCH v2 10/10] can: netlink: add PWM netlink interface Vincent Mailhol
@ 2025-10-31 21:17 ` Oliver Hartkopp
  2025-11-02 22:11   ` Vincent Mailhol
  10 siblings, 1 reply; 23+ messages in thread
From: Oliver Hartkopp @ 2025-10-31 21:17 UTC (permalink / raw)
  To: Vincent Mailhol, Marc Kleine-Budde, Stéphane Grosjean
  Cc: Robert Nawrath, Minh Le, Duy Nguyen, linux-can

Hi Vincent,

I managed to have my DE1SoC FPGA boards working including TMS after 
fixing a weird typo with the help from Bosch colleagues, the PCAN USB XL 
from Stephane (which worked as a correct node) and my scope.

First of all: The configuration and features are great any easy to use 
and give excellent feedback now.

Things that need to be changed:

1. The xsample-point calculation follows the standard CiA sample-points:

https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/tree/drivers/net/can/dev/calc_bittiming.c?h=b4/canxl-netlink#n82

         /* Use CiA recommended sample points */
         if (bt->sample_point) {
                 sample_point_nominal = bt->sample_point;
         } else {
                 if (bt->bitrate > 800 * KILO /* BPS */)
                         sample_point_nominal = 750;
                 else if (bt->bitrate > 500 * KILO /* BPS */)
                         sample_point_nominal = 800;
                 else
                         sample_point_nominal = 875;
         }

But with "TMS on" the PWM method is used to transfer the bit-values.
Therefore the sample-points are near to 50% - 60%, see Table 3 here:

https://www.can-cia.org/fileadmin/cia/documents/publications/cnlm/december_2024/cnlm_24-4_p18_can_xl_system_design_clock_tolerances_and_edge_deviations_dr_arthur_mutter_bosch.pdf

In my case I used ...

# ip link set can0 type can bitrate 1000000 sample-point 0.80 fd off 
xbitrate 10000000 xsample-point 0.57 xl on tms on err-signal off

... to get the correct xsample-point 0.562:

# ip -det link show can0
11: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 2060 qdisc pfifo_fast state UP 
mode DEFAULT group default qlen 10
     link/can  promiscuity 0 allmulti 0 minmtu 16 maxmtu 16
     can <XL,XL-TMS> state STOPPED restart-ms 0
	  bitrate 1000000 sample-point 0.800
	  tq 6 prop-seg 63 phase-seg1 64 phase-seg2 32 sjw 16 brp 1
	  xcanb_can_nl: tseg1 2..512 tseg2 2..128 sjw 1..128 brp 1..32 brp_inc 1
	  xcanb_can_nl: dtseg1 1..256 dtseg2 2..128 dsjw 1..128 dbrp 1..32 
dbrp_inc 1
	  tdco 0..255 tdcf 0..255
	  xbitrate 10000000 xsample-point 0.562
	  xtq 6 xprop-seg 4 xphase-seg1 4 xphase-seg2 7 xsjw 3 xbrp 1
	  pwms 4 pwml 12 pwmo 0
	  xcanb_can_nl: xtseg1 1..256 xtseg2 2..128 xsjw 1..128 xbrp 1..32 
xbrp_inc 1
	  xtdco 0..255 xtdcf 0..255
	  pwms 1..8 pwml 2..24 pwmo 0..16
	  clock 160000000 addrgenmode eui64 numtxqueues 1 numrxqueues 1 
gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 
65535 gro_max_size 65536 gso_ipv4_max_size 65536 gro_ipv4_max_size 65536


2. In the "xl on" "tms on" mode only CAN XL frames can be sent. So we 
need to drop CC and FD frames when they are sent, e.g. via CAN_RAW sockets.

Therefore

[PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD 
is off

has to be extended. And my proposed patch too:

[RFC PATCH v5 2/2] can: reject CAN FD content when disabled on CAN XL 
interfaces

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

I'll continue testing next week.

Best regards,
Oliver

On 21.10.25 17:47, Vincent Mailhol wrote:
> Following all the refactoring on the CAN netlink done in series [1],
> [2] and [3], this is now time to finally introduce the CAN XL netlink
> interface.
> 
> Similarly to how CAN FD reuses the bittiming logic of Classical CAN,
> CAN XL also reuses the entirety of CAN FD features, and, on top of
> that, adds new features which are specific to CAN XL.
> 
> Patch #1 is a small clean-up which makes can_calc_bittiming() use
> NL_SET_ERR_MSG() instead of netdev_err().
> 
> Patch #2 adds a check in can_dev_dropped_skb() to drop CAN FD frames
> when CAN FD is turned off.
> 
> Patch #3 adds CAN_CTRLMODE_RESTRICTED. Note that contrary to the other
> CAN_CTRL_MODE_XL_* that are introduced in the later patches, this
> control mode is not specific to CAN XL. The nuance is that because
> this restricted mode was only added in ISO 11898-1:2024, it is made
> mandatory for CAN XL devices but optional for other protocols. This is
> why this patch is added as a preparation before introducing the core
> CAN XL logic.
> 
> Patch #4 adds all the CAN XL features which are inherited from CAN FD:
> the nominal bittiming, the data bittiming and the TDC.
> 
> Patch #5 and #6 add two new CAN control modes which are specific to
> CAN XL: CAN_CTRLMODE_XL_TMS, CAN_CTRLMODE_XL_ERR_SIGNAL respectively.
> 
> Finally, patch #7 to #10 add the PWM logic.
> 
> [1] can: netlink: preparation before introduction of CAN XL
> Link: https://lore.kernel.org/linux-can/20241112165118.586613-7-mailhol.vincent@wanadoo.fr/
> 
> [2] can: rework the CAN MTU logic (CAN XL preparation step 2/3)
> Link: https://lore.kernel.org/linux-can/20250923-can-fix-mtu-v3-0-581bde113f52@kernel.org/
> 
> [3] can: netlink: preparation before introduction of CAN XL step 3/3
> Link: https://lore.kernel.org/linux-can/20250923-canxl-netlink-prep-v4-0-e720d28f66fe@kernel.org/
> 
> ---
> Changes in v2:
> 
>    - Add a new patch #1.
> 
>    - In patch #9, add a return statement to can_calc_tdco() when
>      CONFIG_CAN_CALC_BITTIMING is not set. This fixes a warning as
>      reported by the kernel test robot:
> 
>        Link: https://lore.kernel.org/linux-can/202510140553.qo3f0I9s-lkp@intel.com/
> 
>      While at it, add an error message.
> 
> Link to v1: https://lore.kernel.org/r/20251013-canxl-netlink-v1-0-f422b7e2729f@kernel.org
> 
> Changes in v1:
> 
>     - Add PWM
> 
>     - Add the CAN_CTRLMODE_RESTRICTED, CAN_CTRLMODE_XL_TMS and
>       CAN_CTRLMODE_XL_ERR_SIGNAL control modes.
> 
>     - A lot has changed since the original RFC was sent in November
>       last year.  The preparation patches went in a separate series as
>       explained in the cover letter, and what used to be a single patch
>       to introduce CAN XL is now a full series. A few additional
>       details are added to the individual patches, but overall I did
>       not keep track of all the changes over the last year. You may as
>       well consider this as a new series.
>     
> Link to RFC: https://lore.kernel.org/linux-can/20241110155902.72807-16-mailhol.vincent@wanadoo.fr/
> 
> ---
> Vincent Mailhol (10):
>        can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
>        can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
>        can: netlink: add CAN_CTRLMODE_RESTRICTED
>        can: netlink: add initial CAN XL support
>        can: netlink: add CAN_CTRLMODE_XL_TMS flag
>        can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
>        can: bittiming: add PWM parameters
>        can: bittiming: add PWM validation
>        can: calc_bittiming: add PWM calculation
>        can: netlink: add PWM netlink interface
> 
>   drivers/net/can/dev/bittiming.c      |  63 +++++++
>   drivers/net/can/dev/calc_bittiming.c |  36 ++++
>   drivers/net/can/dev/dev.c            |  20 +-
>   drivers/net/can/dev/netlink.c        | 357 +++++++++++++++++++++++++++++++++--
>   include/linux/can/bittiming.h        |  81 +++++++-
>   include/linux/can/dev.h              |  49 +++--
>   include/uapi/linux/can/netlink.h     |  35 ++++
>   7 files changed, 599 insertions(+), 42 deletions(-)
> ---
> base-commit: ffee675aceb9f44b0502a8bec912abb0c4f4af62
> change-id: 20241229-canxl-netlink-bc640af10673
> 
> Best regards,


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-10-31 21:17 ` [PATCH v2 00/10] can: netlink: add CAN XL Oliver Hartkopp
@ 2025-11-02 22:11   ` Vincent Mailhol
  2025-11-03 19:15     ` Oliver Hartkopp
  0 siblings, 1 reply; 23+ messages in thread
From: Vincent Mailhol @ 2025-11-02 22:11 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Robert Nawrath, Minh Le, Duy Nguyen, linux-can, Marc Kleine-Budde,
	Stéphane Grosjean

Hi Oliver,

Thanks for your feedback. Nice catches!

On 31/10/2025 at 22:17, Oliver Hartkopp wrote:
> Hi Vincent,
> 
> I managed to have my DE1SoC FPGA boards working including TMS after fixing a
> weird typo with the help from Bosch colleagues, the PCAN USB XL from Stephane
> (which worked as a correct node) and my scope.
> 
> First of all: The configuration and features are great any easy to use and give
> excellent feedback now.
> 
> Things that need to be changed:
> 
> 1. The xsample-point calculation follows the standard CiA sample-points:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/tree/drivers/
> net/can/dev/calc_bittiming.c?h=b4/canxl-netlink#n82
> 
>         /* Use CiA recommended sample points */
>         if (bt->sample_point) {
>                 sample_point_nominal = bt->sample_point;
>         } else {
>                 if (bt->bitrate > 800 * KILO /* BPS */)
>                         sample_point_nominal = 750;
>                 else if (bt->bitrate > 500 * KILO /* BPS */)
>                         sample_point_nominal = 800;
>                 else
>                         sample_point_nominal = 875;
>         }
> 
> But with "TMS on" the PWM method is used to transfer the bit-values.
> Therefore the sample-points are near to 50% - 60%, see Table 3 here:
> 
> https://www.can-cia.org/fileadmin/cia/documents/publications/cnlm/december_2024/
> cnlm_24-4_p18_can_xl_system_design_clock_tolerances_and_edge_deviations_dr_arthur_mutter_bosch.pdf
> 
> In my case I used ...
> 
> # ip link set can0 type can bitrate 1000000 sample-point 0.80 fd off xbitrate
> 10000000 xsample-point 0.57 xl on tms on err-signal off
> 
> ... to get the correct xsample-point 0.562:
> 
> # ip -det link show can0
> 11: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 2060 qdisc pfifo_fast state UP mode
> DEFAULT group default qlen 10
>     link/can  promiscuity 0 allmulti 0 minmtu 16 maxmtu 16
>     can <XL,XL-TMS> state STOPPED restart-ms 0
>       bitrate 1000000 sample-point 0.800
>       tq 6 prop-seg 63 phase-seg1 64 phase-seg2 32 sjw 16 brp 1
>       xcanb_can_nl: tseg1 2..512 tseg2 2..128 sjw 1..128 brp 1..32 brp_inc 1
>       xcanb_can_nl: dtseg1 1..256 dtseg2 2..128 dsjw 1..128 dbrp 1..32 dbrp_inc 1
>       tdco 0..255 tdcf 0..255
>       xbitrate 10000000 xsample-point 0.562
>       xtq 6 xprop-seg 4 xphase-seg1 4 xphase-seg2 7 xsjw 3 xbrp 1
>       pwms 4 pwml 12 pwmo 0
>       xcanb_can_nl: xtseg1 1..256 xtseg2 2..128 xsjw 1..128 xbrp 1..32 xbrp_inc 1
>       xtdco 0..255 xtdcf 0..255
>       pwms 1..8 pwml 2..24 pwmo 0..16
>       clock 160000000 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size
> 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 65535 gro_max_size
> 65536 gso_ipv4_max_size 65536 gro_ipv4_max_size 65536

I totally missed that. Thanks for raising this point! I just sent

https://lore.kernel.org/linux-can/20251102-pwm_sample_point-v1-0-3bbea180f59e@kernel.org/

to address the problem. Let me know if this works in your test environment and I
will merge it to the main series.

> 2. In the "xl on" "tms on" mode only CAN XL frames can be sent. So we need to
> drop CC and FD frames when they are sent, e.g. via CAN_RAW sockets.
> 
> Therefore
> 
> [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
> 
> has to be extended. And my proposed patch too:
> 
> [RFC PATCH v5 2/2] can: reject CAN FD content when disabled on CAN XL interfaces
> 
> https://lore.kernel.org/linux-can/20250909092433.30546-1-socketcan@hartkopp.net/
> T/#mcb0ebd94e45c34a2d0590ded2dfeed97edd05adf

Thanks for re-sending the patch! I remember we discussed this in September but
it somehow went out of my mind. I just have a quick look so far on the new patch
and so far it looks good. I will do a few more tests next week before adding it
to the XL main series.


Yours sincerely,
Vincent Mailhol


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-11-02 22:11   ` Vincent Mailhol
@ 2025-11-03 19:15     ` Oliver Hartkopp
  2025-11-04  7:28       ` Marc Kleine-Budde
  0 siblings, 1 reply; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-03 19:15 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: Robert Nawrath, Minh Le, Duy Nguyen, linux-can, Marc Kleine-Budde,
	Stéphane Grosjean

Hi Vincent,

On 02.11.25 23:11, Vincent Mailhol wrote:

> I just sent
> 
> https://lore.kernel.org/linux-can/20251102-pwm_sample_point-v1-0-3bbea180f59e@kernel.org/
> 
> to address the problem. Let me know if this works in your test environment and I
> will merge it to the main series.

Very elegant solution. I started some coding into this direction too.
Good that I didn't post it ;-)

Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>

For the series. The 'nominal' left over was also a good improvement.

>> 2. In the "xl on" "tms on" mode only CAN XL frames can be sent. So we need to
>> drop CC and FD frames when they are sent, e.g. via CAN_RAW sockets.
>>
>> Therefore
>>
>> [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
>>
>> has to be extended. And my proposed patch too:
>>
>> [RFC PATCH v5 2/2] can: reject CAN FD content when disabled on CAN XL interfaces
>>
>> https://lore.kernel.org/linux-can/20250909092433.30546-1-socketcan@hartkopp.net/
>> T/#mcb0ebd94e45c34a2d0590ded2dfeed97edd05adf
> 
> Thanks for re-sending the patch! I remember we discussed this in September but
> it somehow went out of my mind. I just have a quick look so far on the new patch
> and so far it looks good. I will do a few more tests next week before adding it
> to the XL main series.
I just sent a V2 patch for it:

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

The so-called 'mixed mode' with TMS off and ERR_SIGNAL on allows the 
transmission of CC/FD/XL frames.
But it looks like, that the combination CC/XL is not intended for the 
'mixed mode'. In all the documentations and videos it is the combination 
of FD and XL.

The CANXL-only modes are TMS on OR ERR_SIGNAL off. Both cases currently 
correctly force FD off. But it also should force CAN CC to be disabled.

Long story short:

# ip link set can0 type can bitrate 1000000 fd off xbitrate 4000000 xl 
on tms off

is currently allowed and leads to

# ip -det link show can0
8: can0: <NOARP,ECHO> mtu 2060 qdisc pfifo_fast state DOWN mode DEFAULT 
group default qlen 10
     link/can  promiscuity 0 allmulti 0 minmtu 16 maxmtu 16
     can <XL,XL-TDC-AUTO,XL-ERR-SIGNAL> state STOPPED restart-ms 0
	  bitrate 1000000 sample-point 0.750
	  tq 6 prop-seg 59 phase-seg1 60 phase-seg2 40 sjw 20 brp 1
	  xcanb_can_nl: tseg1 2..512 tseg2 2..128 sjw 1..128 brp 1..32 brp_inc 1
	  xcanb_can_nl: dtseg1 1..256 dtseg2 2..128 dsjw 1..128 dbrp 1..32 
dbrp_inc 1
	  tdco 0..255 tdcf 0..255
	  xbitrate 4000000 xsample-point 0.750
	  xtq 6 xprop-seg 14 xphase-seg1 15 xphase-seg2 10 xsjw 5 xbrp 1
	  xtdco 30 xtdcf 0
	  xcanb_can_nl: xtseg1 1..256 xtseg2 2..128 xsjw 1..128 xbrp 1..32 
xbrp_inc 1
	  xtdco 0..255 xtdcf 0..255
	  pwms 1..8 pwml 2..24 pwmo 0..16
	  clock 160000000 addrgenmode eui64 numtxqueues 1 numrxqueues 1 
gso_max_size 65536 gso_max_segs 65535 tso_max_size 65536 tso_max_segs 
65535 gro_max_size 65536 gso_ipv4_max_size 65536 gro_ipv4_max_size 65536

But IMO the 'mixed-mode' must have a CAN FD mode too:

# ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd on 
xbitrate 4000000 xl on tms off

Which is currently not enforced.

And then my V2 patch also works as intended.

Best regards,
Oliver


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-11-03 19:15     ` Oliver Hartkopp
@ 2025-11-04  7:28       ` Marc Kleine-Budde
  2025-11-04  8:01         ` Oliver Hartkopp
  0 siblings, 1 reply; 23+ messages in thread
From: Marc Kleine-Budde @ 2025-11-04  7:28 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Vincent Mailhol, Robert Nawrath, Minh Le, Duy Nguyen, linux-can,
	Stéphane Grosjean

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

On 03.11.2025 20:15:59, Oliver Hartkopp wrote:
> > I just sent
> >
> > https://lore.kernel.org/linux-can/20251102-pwm_sample_point-v1-0-3bbea180f59e@kernel.org/
> >
> > to address the problem. Let me know if this works in your test environment and I
> > will merge it to the main series.
>
> Very elegant solution. I started some coding into this direction too.
> Good that I didn't post it ;-)
>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
>
> For the series. The 'nominal' left over was also a good improvement.

Can you post your tags under the cover letter, so that b4 will pick it
up automatically?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-11-04  7:28       ` Marc Kleine-Budde
@ 2025-11-04  8:01         ` Oliver Hartkopp
  2025-11-04 13:13           ` Marc Kleine-Budde
  0 siblings, 1 reply; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-04  8:01 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Vincent Mailhol, Robert Nawrath, Minh Le, Duy Nguyen, linux-can,
	Stéphane Grosjean



On 04.11.25 08:28, Marc Kleine-Budde wrote:
> On 03.11.2025 20:15:59, Oliver Hartkopp wrote:
>>> I just sent
>>>
>>> https://lore.kernel.org/linux-can/20251102-pwm_sample_point-v1-0-3bbea180f59e@kernel.org/
>>>
>>> to address the problem. Let me know if this works in your test environment and I
>>> will merge it to the main series.
>>
>> Very elegant solution. I started some coding into this direction too.
>> Good that I didn't post it ;-)
>>
>> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>
>> For the series. The 'nominal' left over was also a good improvement.
> 
> Can you post your tags under the cover letter, so that b4 will pick it
> up automatically?
> 

Done!

Didn't know that there is an automatism for that.

Best regards,
Oliver


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 00/10] can: netlink: add CAN XL
  2025-11-04  8:01         ` Oliver Hartkopp
@ 2025-11-04 13:13           ` Marc Kleine-Budde
  0 siblings, 0 replies; 23+ messages in thread
From: Marc Kleine-Budde @ 2025-11-04 13:13 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Vincent Mailhol, Robert Nawrath, Minh Le, Duy Nguyen, linux-can,
	Stéphane Grosjean

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

On 04.11.2025 09:01:00, Oliver Hartkopp wrote:
> > Can you post your tags under the cover letter, so that b4 will pick it
> > up automatically?
>
> Done!
>
> Didn't know that there is an automatism for that.

With "b4 shazam -k -l -M $MSG_ID" b4 will fetch a patch series, run
local checks and apply it with all review headers.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag
  2025-10-21 15:47 ` [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag Vincent Mailhol
@ 2025-11-06  8:42   ` Oliver Hartkopp
  2025-11-09 14:28     ` Vincent Mailhol
  0 siblings, 1 reply; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-06  8:42 UTC (permalink / raw)
  To: Vincent Mailhol, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel

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@kernel.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
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
  2025-10-21 15:47 ` [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL Vincent Mailhol
@ 2025-11-06  8:50   ` Oliver Hartkopp
  2025-11-09 14:54     ` Vincent Mailhol
  0 siblings, 1 reply; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-06  8:50 UTC (permalink / raw)
  To: Vincent Mailhol, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel



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


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag
  2025-11-06  8:42   ` Oliver Hartkopp
@ 2025-11-09 14:28     ` Vincent Mailhol
  2025-11-09 17:40       ` Oliver Hartkopp
  0 siblings, 1 reply; 23+ messages in thread
From: Vincent Mailhol @ 2025-11-09 14:28 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel

Hello Oliver,

On 06/11/2025 at 09:42, Oliver Hartkopp wrote:
> 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)

OK. "TMS omitted" will be interpreted as "TMS off" in v2.

>> 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@kernel.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

I see. I was under the assumption that CC and FD could be used with error
signalling off in mixed mode. Thanks!

> 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.

Why? Will your device reject the configuration if you omit the FD bitrate? I did
not see anything in this direction in the ISO standard.

Did you have any source for this? Maybe the CiA provided some clarification
which I am not aware of?


Yours sincerely,
Vincent Mailhol


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
  2025-11-06  8:50   ` Oliver Hartkopp
@ 2025-11-09 14:54     ` Vincent Mailhol
  2025-11-09 17:46       ` Oliver Hartkopp
  0 siblings, 1 reply; 23+ messages in thread
From: Vincent Mailhol @ 2025-11-09 14:54 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag
  2025-11-09 14:28     ` Vincent Mailhol
@ 2025-11-09 17:40       ` Oliver Hartkopp
  0 siblings, 0 replies; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-09 17:40 UTC (permalink / raw)
  To: Vincent Mailhol, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel

Hi Vincent,

On 09.11.25 15:28, Vincent Mailhol wrote:
> Hello Oliver,
> 
> On 06/11/2025 at 09:42, Oliver Hartkopp wrote:
>> 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)
> 
> OK. "TMS omitted" will be interpreted as "TMS off" in v2.
> 

Thx!

>>> 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@kernel.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
> 
> I see. I was under the assumption that CC and FD could be used with error
> signalling off in mixed mode. Thanks!
> 
>> 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.
> 
> Why? Will your device reject the configuration if you omit the FD bitrate? I did
> not see anything in this direction in the ISO standard.
> 
> Did you have any source for this? Maybe the CiA provided some clarification
> which I am not aware of?

It wasn't clear to me either.

Please take a look into the X_CAN user manual here:

https://github.com/linux-can/can-doc/blob/master/x_can/xcan_user_manual_v350.pdf

1.5.5.3 Operating Mode

There you can see the valid (and invalid) modes & combinations.

When XL is on, FD also has to be on. Of course this is Bosch specific in 
the first place. But there are two modes (ERR_SIGNAL off either with TMS 
on or off) that are "CANXL-Only" which means no CC and no FD traffic as 
those need error-signalling. CAN XL is using the (CC) arbitration 
bitrate in this XL-only mode but CAN CC frames can not be sent.

I would recommend this CAN CiA Webinar from Dr. Arthur Mutter:
https://www.youtube.com/watch?v=OSEj6ISJKAM

As (only!) the CAN FD controllers are CAN XL tolerant (see video 
timestamp 18:53) the "mixed-mode" basically means "a mix of XL and FD 
controllers and protocols".

The fact that FD&XL controllers then can also send CC frames in this 
mixed-mode is just a side-effect IMO. The idea is FD & XL.

Therefore the two XL modes "XL-Only" and "mixed-mode" leads to FD on in 
the mixed-mode IMO. And this finally can be used in my "[PATCH 
b4/canxl-netlink v2] can: drop unsupported CAN frames on socket
and netdev level" patch.

Best regards,
Oliver

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL
  2025-11-09 14:54     ` Vincent Mailhol
@ 2025-11-09 17:46       ` Oliver Hartkopp
  0 siblings, 0 replies; 23+ messages in thread
From: Oliver Hartkopp @ 2025-11-09 17:46 UTC (permalink / raw)
  To: Vincent Mailhol, Marc Kleine-Budde
  Cc: Stéphane Grosjean, Robert Nawrath, Minh Le, Duy Nguyen,
	linux-can, linux-kernel



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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2025-11-09 17:46 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-21 15:47 [PATCH v2 00/10] can: netlink: add CAN XL Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 01/10] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 02/10] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 03/10] can: netlink: add CAN_CTRLMODE_RESTRICTED Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 04/10] can: netlink: add initial CAN XL support Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 05/10] can: netlink: add CAN_CTRLMODE_XL_TMS flag Vincent Mailhol
2025-11-06  8:42   ` Oliver Hartkopp
2025-11-09 14:28     ` Vincent Mailhol
2025-11-09 17:40       ` Oliver Hartkopp
2025-10-21 15:47 ` [PATCH v2 06/10] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL Vincent Mailhol
2025-11-06  8:50   ` Oliver Hartkopp
2025-11-09 14:54     ` Vincent Mailhol
2025-11-09 17:46       ` Oliver Hartkopp
2025-10-21 15:47 ` [PATCH v2 07/10] can: bittiming: add PWM parameters Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 08/10] can: bittiming: add PWM validation Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 09/10] can: calc_bittiming: add PWM calculation Vincent Mailhol
2025-10-21 15:47 ` [PATCH v2 10/10] can: netlink: add PWM netlink interface Vincent Mailhol
2025-10-31 21:17 ` [PATCH v2 00/10] can: netlink: add CAN XL Oliver Hartkopp
2025-11-02 22:11   ` Vincent Mailhol
2025-11-03 19:15     ` Oliver Hartkopp
2025-11-04  7:28       ` Marc Kleine-Budde
2025-11-04  8:01         ` Oliver Hartkopp
2025-11-04 13:13           ` Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).