[PATCH 0/9] can: netlink: add CAN XL

[PATCH 0/9] can: netlink: add CAN XL

[Vincent Mailhol]

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 adds a check in can_dev_dropped_skb() to drop CAN FD frames
when CAN FD is turned off.

Patch #2 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 #3 adds all the CAN XL features which are inherited from CAN FD:
the nominal bittiming, the data bittiming and the TDC.

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

Finally, patch #6 to #9 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 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 (9):
      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        |  76 +++++++-
 include/linux/can/dev.h              |  49 +++--
 include/uapi/linux/can/netlink.h     |  35 ++++
 7 files changed, 595 insertions(+), 41 deletions(-)
---
base-commit: cb6649f6217c0331b885cf787f1d175963e2a1d2
change-id: 20241229-canxl-netlink-bc640af10673

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

[PATCH 1/9] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Vincent Mailhol]

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

[PATCH 2/9] can: netlink: add CAN_CTRLMODE_RESTRICTED

[Vincent Mailhol]

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

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

[Vincent Mailhol]

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 d30816dd93c7..68d533bb7856 100644
--- a/include/linux/can/bittiming.h
+++ b/include/linux/can/bittiming.h
@@ -16,10 +16,12 @@
 
 #define CAN_CTRLMODE_FD_TDC_MASK				\
 	(CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_TDC_MANUAL)
+#define CAN_CTRLMODE_XL_TDC_MASK				\
+	(CAN_CTRLMODE_XL_TDC_AUTO | CAN_CTRLMODE_XL_TDC_MANUAL)
 #define CAN_CTRLMODE_TDC_AUTO_MASK				\
-	(CAN_CTRLMODE_TDC_AUTO)
+	(CAN_CTRLMODE_TDC_AUTO | CAN_CTRLMODE_XL_TDC_AUTO)
 #define CAN_CTRLMODE_TDC_MANUAL_MASK				\
-	(CAN_CTRLMODE_TDC_MANUAL)
+	(CAN_CTRLMODE_TDC_MANUAL | CAN_CTRLMODE_XL_TDC_MANUAL)
 
 /*
  * struct can_tdc - CAN FD Transmission Delay Compensation parameters
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 9de8fde3ec9d..945c16743702 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -47,7 +47,7 @@ struct can_priv {
 
 	const struct can_bittiming_const *bittiming_const;
 	struct can_bittiming bittiming;
-	struct data_bittiming_params fd;
+	struct data_bittiming_params fd, xl;
 	unsigned int bitrate_const_cnt;
 	const u32 *bitrate_const;
 	u32 bitrate_max;
@@ -85,6 +85,11 @@ static inline bool can_fd_tdc_is_enabled(const struct can_priv *priv)
 	return !!(priv->ctrlmode & CAN_CTRLMODE_FD_TDC_MASK);
 }
 
+static inline bool can_xl_tdc_is_enabled(const struct can_priv *priv)
+{
+	return !!(priv->ctrlmode & CAN_CTRLMODE_XL_TDC_MASK);
+}
+
 static inline u32 can_get_static_ctrlmode(struct can_priv *priv)
 {
 	return priv->ctrlmode & ~priv->ctrlmode_supported;
diff --git a/include/uapi/linux/can/netlink.h b/include/uapi/linux/can/netlink.h
index fafd1cce4798..c2c96c5978a8 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -104,6 +104,9 @@ struct can_ctrlmode {
 #define CAN_CTRLMODE_TDC_AUTO		0x200	/* FD transceiver automatically calculates TDCV */
 #define CAN_CTRLMODE_TDC_MANUAL		0x400	/* FD TDCV is manually set up by user */
 #define CAN_CTRLMODE_RESTRICTED		0x800	/* Restricted operation mode */
+#define CAN_CTRLMODE_XL			0x1000	/* CAN XL mode */
+#define CAN_CTRLMODE_XL_TDC_AUTO	0x2000	/* XL transceiver automatically calculates TDCV */
+#define CAN_CTRLMODE_XL_TDC_MANUAL	0x4000	/* XL TDCV is manually set up by user */
 
 /*
  * CAN device statistics
@@ -139,6 +142,10 @@ enum {
 	IFLA_CAN_BITRATE_MAX,
 	IFLA_CAN_TDC, /* FD */
 	IFLA_CAN_CTRLMODE_EXT,
+	IFLA_CAN_XL_DATA_BITTIMING,
+	IFLA_CAN_XL_DATA_BITTIMING_CONST,
+	IFLA_CAN_XL_DATA_BITRATE_CONST,
+	IFLA_CAN_XL_TDC,
 
 	/* add new constants above here */
 	__IFLA_CAN_MAX,

-- 
2.49.1

[PATCH 4/9] can: netlink: add CAN_CTRLMODE_XL_TMS flag

[Vincent Mailhol]

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

[PATCH 5/9] can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL

[Vincent Mailhol]

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

[PATCH 6/9] can: bittiming: add PWM parameters

[Vincent Mailhol]

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 68d533bb7856..14ad249aa8cd 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.49.1

[PATCH 7/9] can: bittiming: add PWM validation

[Vincent Mailhol]

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 14ad249aa8cd..02cf9541a1dd 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.49.1

[PATCH 8/9] can: calc_bittiming: add PWM calculation

[Vincent Mailhol]

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>
---
 drivers/net/can/dev/calc_bittiming.c | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/can/bittiming.h        |  7 +++++++
 2 files changed, 43 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 02cf9541a1dd..e7671a86afcc 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,11 @@ 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)
+{
+}
 #endif /* CONFIG_CAN_CALC_BITTIMING */
 
 void can_sjw_set_default(struct can_bittiming *bt);

-- 
2.49.1

[PATCH 9/9] can: netlink: add PWM netlink interface

[Vincent Mailhol]

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

Re: [PATCH 0/9] can: netlink: add CAN XL

[Vincent Mailhol]

On 13/10/2025 at 20:01, 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.

I am sending this extra message to give a few additional hints on how
to test.

In addition to the mailing list, I also push this series and the
dummy_can driver to:

  https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/log/?h=b4/canxl-netlink

I also have a work in progress for iproute2 here:

  https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/iproute2-next.git/log/?h=canxl-netlink

I will submit the iproute2 series later on, after receiving comments
on this series. For the moment, the iproute2 canxl is only available
through the link above.

To test, after cloning and compiling above branches, do:

  modprobe dummy-can

to load the driver. Then configure it, for example, this is a 500 KB/s
nominal bittiming and a 10 MB/s XL databittiming with TMS on:

  ./ip/ip link set can0 up type can bitrate 500000 xl on xbitrate 10000000 tms on

If you have debug log enabled (e.g. with CONFIG_CAN_DEBUG_DEVICES),
this is what you should see in the kernel log:

  can0: Clock frequency: 160000000
  can0: Maximum bitrate: 20000000
  can0: MTU: 2060
  can0: 
  can0: Control modes:
  can0: 	supported: 0x0001ba22
  can0: 	enabled: 0x00009000
  can0: 	list:
  can0: 		listen-only: off
  can0: 		fd: off
  can0: 		fd-tdc-auto: off
  can0: 		restricted-operation: off
  can0: 		xl: on
  can0: 		xl-tdc-auto: off
  can0: 		xl-tms: on
  can0: 		xl-error-signalling: off
  can0: 
  can0: Classical CAN nominal bittiming:
  can0: 	bitrate: 500000
  can0: 	sample_point: 875
  can0: 	tq: 12
  can0: 	prop_seg: 69
  can0: 	phase_seg1: 70
  can0: 	phase_seg2: 20
  can0: 	sjw: 10
  can0: 	brp: 2
  can0: 
  can0: 
  can0: CAN XL databittiming:
  can0: 	bitrate: 10000000
  can0: 	sample_point: 750
  can0: 	tq: 6
  can0: 	prop_seg: 5
  can0: 	phase_seg1: 6
  can0: 	phase_seg2: 4
  can0: 	sjw: 2
  can0: 	brp: 1
  can0: 	CAN XL PWM:
  can0: 		pwms: 4
  can0: 		pwml: 12
  can0: 		pwmo: 0
  can0: 
  can0: dummy-can is up

Finally, you can use a recent version of can-utils to generate some
traffic. The driver will echo back anything it receives.

I will continue to update the above branches according to the comments
received. See these as work in progress. Use the series as posted on
the mailing if you want something more stable.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

[kernel test robot]

Hi Vincent,

kernel test robot noticed the following build warnings:

[auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]

url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/can/dev.h:18,
                    from net/can/raw.c:53:
   include/linux/can/bittiming.h: In function 'can_calc_pwm':
>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
     204 | }
         | ^

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
   Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
   Selected by [y]:
   - RISCV [=y]


vim +204 include/linux/can/bittiming.h

   200	
   201	static inline int
   202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
   203	{
 > 204	}
   205	#endif /* CONFIG_CAN_CALC_BITTIMING */
   206	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

[Vincent Mailhol]

On 14/10/2025 at 06:21, kernel test robot wrote:
> Hi Vincent,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
> compiler: riscv64-linux-gcc (GCC) 10.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from include/linux/can/dev.h:18,
>                     from net/can/raw.c:53:
>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>      204 | }
>          | ^
> 
> Kconfig warnings: (for reference only)
>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>    Selected by [y]:
>    - RISCV [=y]
> 
> 
> vim +204 include/linux/can/bittiming.h
> 
>    200	
>    201	static inline int
>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>    203	{
>  > 204	}
>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>    206	

I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.

What is surprising enough is that can_calc_tdco() has the exact same
issue but we never received a report.

I will first send a separate fix for can_calc_tdco() targeting
net/main and send the v2 for this series once the can_calc_tdco() fix
reaches net-next/main.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

[Vincent Mailhol]

On 14/10/2025 at 11:05, Vincent Mailhol wrote:
> On 14/10/2025 at 06:21, kernel test robot wrote:
>> Hi Vincent,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
>> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
>> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
>> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
>> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
>> compiler: riscv64-linux-gcc (GCC) 10.5.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>    In file included from include/linux/can/dev.h:18,
>>                     from net/can/raw.c:53:
>>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>>      204 | }
>>          | ^
>>
>> Kconfig warnings: (for reference only)
>>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>>    Selected by [y]:
>>    - RISCV [=y]
>>
>>
>> vim +204 include/linux/can/bittiming.h
>>
>>    200	
>>    201	static inline int
>>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>>    203	{
>>  > 204	}
>>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>>    206	
> 
> I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.
> 
> What is surprising enough is that can_calc_tdco() has the exact same
> issue but we never received a report.

Actually, can_calc_tdco() is not getting a warning because it returns
void. But this doesn't change the fact that it should be returning an
error when CONFIG_CAN_CALC_BITTIMING is not set.

> I will first send a separate fix for can_calc_tdco() targeting
> net/main and send the v2 for this series once the can_calc_tdco() fix
> reaches net-next/main.

N.B. this issue does not impact the code when
CONFIG_CAN_CALC_BITTIMING is set. So you can continue to review while
this problem is getting addressed.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 8/9] can: calc_bittiming: add PWM calculation

[Vincent Mailhol]

On 14/10/2025 at 11:19, Vincent Mailhol wrote:
> On 14/10/2025 at 11:05, Vincent Mailhol wrote:
>> On 14/10/2025 at 06:21, kernel test robot wrote:
>>> Hi Vincent,
>>>
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on cb6649f6217c0331b885cf787f1d175963e2a1d2]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Vincent-Mailhol/can-dev-can_dev_dropped_skb-drop-CAN-FD-skbs-if-FD-is-off/20251013-191232
>>> base:   cb6649f6217c0331b885cf787f1d175963e2a1d2
>>> patch link:    https://lore.kernel.org/r/20251013-canxl-netlink-v1-8-f422b7e2729f%40kernel.org
>>> patch subject: [PATCH 8/9] can: calc_bittiming: add PWM calculation
>>> config: riscv-randconfig-001-20251014 (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/config)
>>> compiler: riscv64-linux-gcc (GCC) 10.5.0
>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251014/202510140553.qo3f0I9s-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202510140553.qo3f0I9s-lkp@intel.com/
>>>
>>> All warnings (new ones prefixed by >>):
>>>
>>>    In file included from include/linux/can/dev.h:18,
>>>                     from net/can/raw.c:53:
>>>    include/linux/can/bittiming.h: In function 'can_calc_pwm':
>>>>> include/linux/can/bittiming.h:204:1: warning: no return statement in function returning non-void [-Wreturn-type]
>>>      204 | }
>>>          | ^
>>>
>>> Kconfig warnings: (for reference only)
>>>    WARNING: unmet direct dependencies detected for ARCH_HAS_ELF_CORE_EFLAGS
>>>    Depends on [n]: BINFMT_ELF [=y] && ELF_CORE [=n]
>>>    Selected by [y]:
>>>    - RISCV [=y]
>>>
>>>
>>> vim +204 include/linux/can/bittiming.h
>>>
>>>    200	
>>>    201	static inline int
>>>    202	can_calc_pwm(struct net_device *dev, struct netlink_ext_ack *extack)
>>>    203	{
>>>  > 204	}
>>>    205	#endif /* CONFIG_CAN_CALC_BITTIMING */
>>>    206	
>>
>> I see, this only occurs if CONFIG_CAN_CALC_BITTIMING is not set.
>>
>> What is surprising enough is that can_calc_tdco() has the exact same
>> issue but we never received a report.
> 
> Actually, can_calc_tdco() is not getting a warning because it returns
> void. But this doesn't change the fact that it should be returning an
> error when CONFIG_CAN_CALC_BITTIMING is not set.

I spoke too quickly. can_calc_tdco() is only called when no TDC
parameters are provided. In that case, if CONFIG_CAN_CALC_BITTIMING is
not set, doing nothing is the correct approach. i.e. there is nothing
to fix here. So there will be no fix patch to net/main and I will
directly incorporate the fix in a v2.

Sorry for the noise!


Yours sincerely,
Vincent Mailhol

Re: [PATCH 1/9] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Marc Kleine-Budde]

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

On 13.10.2025 20:01:23, Vincent Mailhol wrote:
> 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>

What about merging both can_dev_dropped_skb() an
can_dropped_invalid_skb() in the skb.c, so that there is no stub in the
header file anymore.

Someone (i.e. me) used can_dropped_invalid_skb() in a driver, that means
the check for CAN_CTRLMODE_LISTENONLY is missing :/ (I'll send a fix).

regards,
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 --]

Re: [PATCH 1/9] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Marc Kleine-Budde]

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

On 17.10.2025 10:28:38, Marc Kleine-Budde wrote:
> On 13.10.2025 20:01:23, Vincent Mailhol wrote:
> > 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>
> 
> What about merging both can_dev_dropped_skb() an
> can_dropped_invalid_skb() in the skb.c, so that there is no stub in the
> header file anymore.

Ouch! Don't do this. We still need can_dropped_invalid_skb() for virtual
interfaces. See commit ae64438be192 ("can: dev: fix skb drop check").

But then I'm asking: Why is there the difference between virtual and
non-virtual interface in the first place? Can we get rid of it?

> Someone (i.e. me) used can_dropped_invalid_skb() in a driver, that means
> the check for CAN_CTRLMODE_LISTENONLY is missing :/ (I'll send a fix).

These drivers need this fix:

| drivers/net/can/bxcan.c:845:     if (can_dropped_invalid_skb(ndev, skb))
| drivers/net/can/esd/esdacc.c:257:        if (can_dropped_invalid_skb(netdev, skb))
| drivers/net/can/rockchip/rockchip_canfd-tx.c:75: if (can_dropped_invalid_skb(ndev, skb))

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

Re: [PATCH 0/9] can: netlink: add CAN XL

[Marc Kleine-Budde]

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

On 13.10.2025 20:01:22, 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 adds a check in can_dev_dropped_skb() to drop CAN FD frames
> when CAN FD is turned off.
> 
> Patch #2 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.

What about merging patches 1+2 now?

regards,
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 --]

Re: [PATCH 1/9] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Vincent Mailhol]

On 17/10/2025 at 22:27, Marc Kleine-Budde wrote:
> On 17.10.2025 10:28:38, Marc Kleine-Budde wrote:
>> On 13.10.2025 20:01:23, Vincent Mailhol wrote:
>>> 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>
>>
>> What about merging both can_dev_dropped_skb() an
>> can_dropped_invalid_skb() in the skb.c, so that there is no stub in the
>> header file anymore.
> 
> Ouch! Don't do this. We still need can_dropped_invalid_skb() for virtual
> interfaces. See commit ae64438be192 ("can: dev: fix skb drop check").

Exactly!

> But then I'm asking: Why is there the difference between virtual and
> non-virtual interface in the first place? Can we get rid of it?

The fact is that:

  - We need a function for the physical interfaces to check the
    CAN_CTRLMODE_LISTENONLY, i.e. with an access to struct can_priv.

  - We need a similar function but which work for the virtual
    interfaces which do not have a can_priv member.

So unless we do a major code refactor so that the virtual interfaces,
I do not see how we could get rid of it.

>> Someone (i.e. me) used can_dropped_invalid_skb() in a driver, that means
>> the check for CAN_CTRLMODE_LISTENONLY is missing :/ (I'll send a fix).

At least, this does not seem like a security breach like it was the
case for the missing net_device_ops->ndo_change_mtu().

> These drivers need this fix:
> 
> | drivers/net/can/bxcan.c:845:     if (can_dropped_invalid_skb(ndev, skb))
> | drivers/net/can/esd/esdacc.c:257:        if (can_dropped_invalid_skb(netdev, skb))
> | drivers/net/can/rockchip/rockchip_canfd-tx.c:75: if (can_dropped_invalid_skb(ndev, skb))

Yeah, I think that this is a pitfall, but at the same time, I do not
see how to get rid of this can_dev_dropped_skb() and
can_dropped_invalid_skb() pair. The best I can think of it to be
careful on this problem doing the code review now that we are aware
about it.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 1/9] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off

[Marc Kleine-Budde]

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

On 18.10.2025 00:30:45, Vincent Mailhol wrote:
> >> What about merging both can_dev_dropped_skb() an
> >> can_dropped_invalid_skb() in the skb.c, so that there is no stub in the
> >> header file anymore.
> > 
> > Ouch! Don't do this. We still need can_dropped_invalid_skb() for virtual
> > interfaces. See commit ae64438be192 ("can: dev: fix skb drop check").
> 
> Exactly!
> 
> > But then I'm asking: Why is there the difference between virtual and
> > non-virtual interface in the first place? Can we get rid of it?
> 
> The fact is that:
> 
>   - We need a function for the physical interfaces to check the
>     CAN_CTRLMODE_LISTENONLY, i.e. with an access to struct can_priv.
> 
>   - We need a similar function but which work for the virtual
>     interfaces which do not have a can_priv member.
> 
> So unless we do a major code refactor so that the virtual interfaces,
> I do not see how we could get rid of it.

Ack...There are more interesting/important tasks.

> >> Someone (i.e. me) used can_dropped_invalid_skb() in a driver, that means
> >> the check for CAN_CTRLMODE_LISTENONLY is missing :/ (I'll send a fix).
> 
> At least, this does not seem like a security breach like it was the
> case for the missing net_device_ops->ndo_change_mtu().
> 
> > These drivers need this fix:
> > 
> > | drivers/net/can/bxcan.c:845:     if (can_dropped_invalid_skb(ndev, skb))
> > | drivers/net/can/esd/esdacc.c:257:        if (can_dropped_invalid_skb(netdev, skb))
> > | drivers/net/can/rockchip/rockchip_canfd-tx.c:75: if (can_dropped_invalid_skb(ndev, skb))
> 
> Yeah, I think that this is a pitfall, but at the same time, I do not
> see how to get rid of this can_dev_dropped_skb() and
> can_dropped_invalid_skb() pair. The best I can think of it to be
> careful on this problem doing the code review now that we are aware
> about it.

Ack.

regards,
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 --]

Re: [PATCH 0/9] can: netlink: add CAN XL

[Vincent Mailhol]

On 17/10/2025 at 22:53, Marc Kleine-Budde wrote:
> On 13.10.2025 20:01:22, 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 adds a check in can_dev_dropped_skb() to drop CAN FD frames
>> when CAN FD is turned off.
>>
>> Patch #2 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.
> 
> What about merging patches 1+2 now?

If patch 1 had to be squashed, it should probably be in patch 3
"can: netlink: add initial CAN XL support". The MTU workaround as
introduced in patch 1 does not share any of the logic of the
CAN_CTRLMODE_RESTRICTED as introduced in patch 2. Patch 1 is really
just a preparation for CAN XL. You could remove patch 2 from the
series and it will still work (aside from missing one of ISO mandatory
features). Remove patch 1, and the thing breaks apart because it is
required by patch 3.

If I were to squash 1 and 2, I am not sure how I would describe those
two different changes in a single patch message.


Yours sincerely,
Vincent Mailhol

Re: [PATCH 0/9] can: netlink: add CAN XL

[Marc Kleine-Budde]

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

On 18.10.2025 00:40:22, Vincent Mailhol wrote:
> On 17/10/2025 at 22:53, Marc Kleine-Budde wrote:
> > On 13.10.2025 20:01:22, 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 adds a check in can_dev_dropped_skb() to drop CAN FD frames
> >> when CAN FD is turned off.
> >>
> >> Patch #2 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.
> > 
> > What about merging patches 1+2 now?
> 
> If patch 1 had to be squashed,

Sorry - I was offering you to take patches 1+2 into can-next-testing
now.

> it should probably be in patch 3
> "can: netlink: add initial CAN XL support". The MTU workaround as
> introduced in patch 1 does not share any of the logic of the
> CAN_CTRLMODE_RESTRICTED as introduced in patch 2. Patch 1 is really
> just a preparation for CAN XL. You could remove patch 2 from the
> series and it will still work (aside from missing one of ISO mandatory
> features). Remove patch 1, and the thing breaks apart because it is
> required by patch 3.
> 
> If I were to squash 1 and 2, I am not sure how I would describe those
> two different changes in a single patch message.

regards,
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 --]

Re: [PATCH 0/9] can: netlink: add CAN XL

[Vincent Mailhol]

On 18/10/2025 at 01:02, Marc Kleine-Budde wrote:
> On 18.10.2025 00:40:22, Vincent Mailhol wrote:
>> On 17/10/2025 at 22:53, Marc Kleine-Budde wrote:
>>> On 13.10.2025 20:01:22, 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 adds a check in can_dev_dropped_skb() to drop CAN FD frames
>>>> when CAN FD is turned off.
>>>>
>>>> Patch #2 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.
>>>
>>> What about merging patches 1+2 now?
>>
>> If patch 1 had to be squashed,
> 
> Sorry - I was offering you to take patches 1+2 into can-next-testing
> now.

Ah! This makes more sense. Sorry for misreading you.

Yes, you can pick those two. But could you just push your
can-next-testing branch to git.kernel.org after picking those? This
way, I can rebase my series on top of it instead of dealing with some
complex dependencies.

 
Yours sincerely,
Vincent Mailhol

Re: [PATCH 0/9] can: netlink: add CAN XL

[Marc Kleine-Budde]

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

On 18.10.2025 01:20:31, Vincent Mailhol wrote:
> >>> What about merging patches 1+2 now?
> >>
> >> If patch 1 had to be squashed,
> > 
> > Sorry - I was offering you to take patches 1+2 into can-next-testing
> > now.
> 
> Ah! This makes more sense. Sorry for misreading you.
> 
> Yes, you can pick those two. But could you just push your
> can-next-testing branch to git.kernel.org after picking those? This
> way, I can rebase my series on top of it instead of dealing with some
> complex dependencies.

Right, taking only part of the series makes things more complicated.
Better keep them for now.

regards,
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 --]