* [canxl v2 00/15] CAN XL support for review (full series)
@ 2025-11-15 16:37 Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 01/15] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
` (15 more replies)
0 siblings, 16 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
This series is based on Vincents CAN XL patches 2025-10-13 11:01
https://lore.kernel.org/linux-can/20251017-enchanted-quiet-civet-84dd47-mkl@pengutronix.de/T/#mdecc959e0ef7c16c64f35e9dd3d687954e15c8ac
For a better review here is the complete series of available patches with
some changes, especially the error-signalling handling.
The changes to Vincents original patches are documented below:
Oliver Hartkopp (2):
can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode
(replaces can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL flag patch)
can: raw: instantly reject unsupported CAN frames
(the adapted version using can_dev_in_xl_only_mode() helper)
Vincent Mailhol (13):
can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
(no change)
can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
(no change)
can: netlink: add CAN_CTRLMODE_RESTRICTED
(no change)
can: netlink: add initial CAN XL support
(remove the "bad device" warning for CAN_CTRLMODE_RESTRICTED)
can: netlink: add CAN_CTRLMODE_XL_TMS flag
(remove the requirement that TMS MUST be set. Use defaults: off)
can: bittiming: add PWM parameters
(no change)
can: bittiming: add PWM validation
(no change)
can: calc_bittiming: add PWM calculation
(no change)
can: netlink: add PWM netlink interface
(no change)
can: calc_bittiming: get rid of the incorrect "nominal" word
(no change)
can: calc_bittiming: add can_calc_sample_point_nrz()
(no change)
can: calc_bittiming: add can_calc_sample_point_pwm()
(no change)
can: add dummy_can driver
(remove CAN_CTRLMODE_XL_ERR_SIGNAL but print error-signalling state)
drivers/net/can/Kconfig | 17 ++
drivers/net/can/Makefile | 1 +
drivers/net/can/dev/bittiming.c | 63 ++++++
drivers/net/can/dev/calc_bittiming.c | 104 +++++++--
drivers/net/can/dev/dev.c | 18 +-
drivers/net/can/dev/netlink.c | 319 +++++++++++++++++++++++++--
drivers/net/can/dummy_can.c | 284 ++++++++++++++++++++++++
include/linux/can/bittiming.h | 81 ++++++-
include/linux/can/dev.h | 68 ++++--
include/uapi/linux/can/netlink.h | 34 +++
net/can/raw.c | 54 ++++-
11 files changed, 970 insertions(+), 73 deletions(-)
create mode 100644 drivers/net/can/dummy_can.c
--
2.47.3
^ permalink raw reply [flat|nested] 29+ messages in thread
* [canxl v2 01/15] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 02/15] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Oliver Hartkopp
` (14 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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>
---
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
@@ -139,11 +139,11 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
#else /* !CONFIG_CAN_CALC_BITTIMING */
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;
}
static inline void
can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 02/15] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 01/15] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 03/15] can: netlink: add CAN_CTRLMODE_RESTRICTED Oliver Hartkopp
` (13 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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>
---
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 bd7410b5d8a6..a7a39a6101d9 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -101,16 +101,24 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
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");
- 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);
struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 03/15] can: netlink: add CAN_CTRLMODE_RESTRICTED
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 01/15] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 02/15] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 04/15] can: netlink: add initial CAN XL support Oliver Hartkopp
` (12 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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 80e1ab18de87..5e24bc332a50 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -113,10 +113,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
return "cc-len8-dlc";
case CAN_CTRLMODE_TDC_AUTO:
return "fd-tdc-auto";
case CAN_CTRLMODE_TDC_MANUAL:
return "fd-tdc-manual";
+ case CAN_CTRLMODE_RESTRICTED:
+ return "restricted-operation";
default:
return "<unknown>";
}
}
EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 6f83b87d54fc..7cb57b0df726 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -186,10 +186,17 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
if (data[IFLA_CAN_CTRLMODE]) {
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);
if (err)
return err;
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index a7a39a6101d9..ab11c0e9111b 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -93,34 +93,10 @@ static inline u32 can_get_static_ctrlmode(struct can_priv *priv)
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,
unsigned int txqs, unsigned int rxqs);
#define alloc_candev(sizeof_priv, echo_skb_max) \
@@ -152,10 +128,36 @@ int can_restart_now(struct net_device *dev);
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,
enum can_state *rx_state);
void can_change_state(struct net_device *dev, struct can_frame *cf,
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
@@ -101,10 +101,11 @@ struct can_ctrlmode {
#define CAN_CTRLMODE_PRESUME_ACK 0x40 /* Ignore missing CAN ACKs */
#define CAN_CTRLMODE_FD_NON_ISO 0x80 /* CAN FD in non-ISO mode */
#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
*/
struct can_device_stats {
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 04/15] can: netlink: add initial CAN XL support
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (2 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 03/15] can: netlink: add CAN_CTRLMODE_RESTRICTED Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
` (11 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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.
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>
---
drivers/net/can/dev/dev.c | 14 +++++-
drivers/net/can/dev/netlink.c | 76 +++++++++++++++++++++++++-------
include/linux/can/bittiming.h | 6 ++-
include/linux/can/dev.h | 7 ++-
include/uapi/linux/can/netlink.h | 7 +++
5 files changed, 90 insertions(+), 20 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 5e24bc332a50..443692587217 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -115,10 +115,16 @@ 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";
+ 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>";
}
}
EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
@@ -348,11 +354,17 @@ EXPORT_SYMBOL_GPL(free_candev);
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;
} else {
dev->mtu = CAN_MTU;
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 7cb57b0df726..26c25e660e31 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -1,10 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-only
/* 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>
#include <net/rtnetlink.h>
@@ -20,10 +20,13 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
[IFLA_CAN_DATA_BITTIMING] = { .len = sizeof(struct can_bittiming) },
[IFLA_CAN_DATA_BITTIMING_CONST] = { .len = sizeof(struct can_bittiming_const) },
[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] = {
[IFLA_CAN_TDC_TDCV_MIN] = { .type = NLA_U32 },
[IFLA_CAN_TDC_TDCV_MAX] = { .type = NLA_U32 },
@@ -68,11 +71,11 @@ static int can_validate_tdc(struct nlattr *data_tdc,
NL_SET_ERR_MSG(extack,
"TDC manual and auto modes are mutually exclusive");
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) {
NL_SET_ERR_MSG(extack, "TDC parameters are missing");
return -EOPNOTSUPP;
@@ -80,12 +83,12 @@ static int can_validate_tdc(struct nlattr *data_tdc,
if (!(tdc_auto || tdc_manual) && data_tdc) {
NL_SET_ERR_MSG(extack, "TDC mode (auto or manual) is missing");
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];
err = nla_parse_nested(tb_tdc, IFLA_CAN_TDC_MAX,
@@ -124,24 +127,27 @@ static int can_validate_databittiming(struct nlattr *data[],
const char *type;
u32 tdc_flags;
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())
*/
if (ifla_can_data_bittiming == IFLA_CAN_DATA_BITTIMING) {
data_tdc = data[IFLA_CAN_TDC];
tdc_flags = flags & CAN_CTRLMODE_FD_TDC_MASK;
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) {
if (!data[IFLA_CAN_BITTIMING] || !data[ifla_can_data_bittiming]) {
NL_SET_ERR_MSG_FMT(extack,
@@ -204,10 +210,15 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
err = can_validate_databittiming(data, extack,
IFLA_CAN_DATA_BITTIMING, flags);
if (err)
return err;
+ err = can_validate_databittiming(data, extack,
+ IFLA_CAN_XL_DATA_BITTIMING, flags);
+ if (err)
+ return err;
+
return 0;
}
static int can_ctrlmode_changelink(struct net_device *dev,
struct nlattr *data[],
@@ -249,22 +260,30 @@ static int can_ctrlmode_changelink(struct net_device *dev,
}
/* 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);
return 0;
}
@@ -335,11 +354,14 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
data_bittiming = data[IFLA_CAN_DATA_BITTIMING];
data_tdc = data[IFLA_CAN_TDC];
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)
return 0;
@@ -386,11 +408,11 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
/* Neither of TDC parameters nor TDC flags are provided:
* do calculation
*/
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
*/
memcpy(&dbt_params->data_bittiming, &dbt, sizeof(dbt));
@@ -491,10 +513,15 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
/* CAN FD */
err = can_dbt_changelink(dev, data, true, extack);
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;
unsigned int i;
@@ -558,18 +585,18 @@ static size_t can_tdc_get_size(struct data_bittiming_params *dbt_params,
static size_t can_data_bittiming_get_size(struct data_bittiming_params *dbt_params,
u32 tdc_flags)
{
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;
}
static size_t can_ctrlmode_ext_get_size(void)
@@ -605,10 +632,13 @@ static size_t can_get_size(const struct net_device *dev)
size += can_ctrlmode_ext_get_size(); /* IFLA_CAN_CTRLMODE_EXT */
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;
}
static int can_bittiming_fill_info(struct sk_buff *skb, int ifla_can_bittiming,
struct can_bittiming *bittiming)
@@ -649,11 +679,13 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev,
if (ifla_can_tdc == IFLA_CAN_TDC) {
dbt_params = &priv->fd;
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;
if (!tdc_const)
@@ -771,11 +803,23 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
sizeof(priv->bitrate_max),
&priv->bitrate_max)) ||
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;
return 0;
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
@@ -14,14 +14,16 @@
#define CAN_BITRATE_UNSET 0
#define CAN_BITRATE_UNKNOWN (-1U)
#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
*
* At high bit rates, the propagation delay from the TX pin to the RX
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index ab11c0e9111b..f15879bd818d 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -45,11 +45,11 @@ struct can_priv {
struct net_device *dev;
struct can_device_stats can_stats;
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;
struct can_clock clock;
@@ -83,10 +83,15 @@ struct can_priv {
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
@@ -102,10 +102,13 @@ struct can_ctrlmode {
#define CAN_CTRLMODE_FD_NON_ISO 0x80 /* CAN FD in non-ISO mode */
#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 */
+#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
*/
struct can_device_stats {
@@ -137,10 +140,14 @@ enum {
IFLA_CAN_BITRATE_CONST,
IFLA_CAN_DATA_BITRATE_CONST, /* FD */
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,
IFLA_CAN_MAX = __IFLA_CAN_MAX - 1
};
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (3 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 04/15] can: netlink: add initial CAN XL support Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-16 19:31 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 06/15] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode Oliver Hartkopp
` (10 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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.
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>
---
drivers/net/can/dev/dev.c | 2 ++
drivers/net/can/dev/netlink.c | 48 ++++++++++++++++++++++++++++++--
include/uapi/linux/can/netlink.h | 1 +
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 443692587217..9da3da8c6225 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -121,10 +121,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
return "xl";
case CAN_CTRLMODE_XL_TDC_AUTO:
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>";
}
}
EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index 26c25e660e31..5a628c629109 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -179,10 +179,36 @@ static int can_validate_databittiming(struct nlattr *data[],
return err;
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 (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)
{
u32 flags = 0;
int err;
@@ -199,10 +225,14 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
(flags & CAN_CTRLMODE_RESTRICTED)) {
NL_SET_ERR_MSG(extack,
"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);
if (err)
return err;
@@ -224,11 +254,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
struct can_priv *priv = netdev_priv(dev);
struct can_ctrlmode *cm;
- u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
+ u32 ctrlstatic, maskedflags, deactivated, notsupp, ctrlstatic_missing;
if (!data[IFLA_CAN_CTRLMODE])
return 0;
/* Do not allow changing controller mode while running */
@@ -236,10 +266,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
return -EBUSY;
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;
if (notsupp) {
NL_SET_ERR_MSG_FMT(extack,
@@ -257,15 +288,25 @@ static int can_ctrlmode_changelink(struct net_device *dev,
"missing required %s static control mode",
can_get_ctrlmode_str(ctrlstatic_missing));
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;
priv->ctrlmode |= maskedflags;
@@ -393,11 +434,12 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
memset(&dbt_params->tdc, 0, sizeof(dbt_params->tdc));
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 */
err = can_tdc_changelink(dbt_params, data_tdc, extack);
if (err) {
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
@@ -105,10 +105,11 @@ struct can_ctrlmode {
#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 */
+#define CAN_CTRLMODE_XL_TMS 0x8000 /* Transceiver Mode Switching */
/*
* CAN device statistics
*/
struct can_device_stats {
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 06/15] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (4 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 07/15] can: bittiming: add PWM parameters Oliver Hartkopp
` (9 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
The error-signalling is a mandatory functionality for CAN CC and CAN FD to
report CAN frame format violations by sending an error-frame signal.
CAN XL has an improved error detection [1] which does not need to destroy
the sent frame with error-signalling. Therefore the CANXL-only mode
disables the error-signalling in the CAN XL controller. The CANXL-only mode
additionally offers a CAN XL transceiver mode switching (TMS).
The 'mixed-mode' allows CAN segments with CAN XL and CAN FD controllers
where the latter are CAN XL frame format tolerant. This mixed-mode
utilizes the error-signalling for sending CC/FD/XL frames.
Configured with CAN_CTRLMODE_FD and CAN_CTRLMODE_XL this leads to:
FD=1 XL=1 -> CC/FD/XL mixed-mode (error-signalling on)
FD=0 XL=1 -> CANXL-only mode (error-signalling off, optional TMS)
This helper function can_dev_in_xl_only_mode() determines the required
value to disable error signalling in the CAN XL controller.
[1] https://can-cia.org/fileadmin/cia/documents/proceedings/2020_mutter.pdf
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can/dev.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index f15879bd818d..52c8be5c160e 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -133,10 +133,23 @@ int can_restart_now(struct net_device *dev);
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);
+static inline bool can_dev_in_xl_only_mode(struct can_priv *priv)
+{
+ const u32 mixed_mode = CAN_CTRLMODE_FD | CAN_CTRLMODE_XL;
+
+ /* When CAN XL is enabled but FD is disabled we are running in
+ * the so-called 'CANXL-only mode' where the error signalling is
+ * disabled. This helper function determines the required value
+ * to disable error signalling in the CAN XL controller.
+ * The so-called CC/FD/XL 'mixed mode' requires error signalling.
+ */
+ return ((priv->ctrlmode & mixed_mode) == CAN_CTRLMODE_XL);
+}
+
/* 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 |
@@ -151,10 +164,16 @@ static inline bool can_dev_dropped_skb(struct net_device *dev, struct sk_buff *s
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;
}
+ if (can_dev_in_xl_only_mode(priv) && !can_is_canxl_skb(skb)) {
+ netdev_info_once(dev,
+ "Error signaling is disabled, dropping skb\n");
+ goto invalid_skb;
+ }
+
return can_dropped_invalid_skb(dev, skb);
invalid_skb:
kfree_skb(skb);
dev->stats.tx_dropped++;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 07/15] can: bittiming: add PWM parameters
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (5 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 06/15] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 08/15] can: bittiming: add PWM validation Oliver Hartkopp
` (8 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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,8 +1,8 @@
/* 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
#define _CAN_BITTIMING_H
@@ -118,15 +118,52 @@ struct can_tdc_const {
u32 tdco_max;
u32 tdcf_min;
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);
int (*do_get_auto_tdcv)(const struct net_device *dev, u32 *tdcv);
};
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 08/15] can: bittiming: add PWM validation
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (6 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 07/15] can: bittiming: add PWM parameters Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 09/15] can: calc_bittiming: add PWM calculation Oliver Hartkopp
` (7 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-only
/* 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>
void can_sjw_set_default(struct can_bittiming *bt)
@@ -149,5 +150,67 @@ int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt,
return can_validate_bitrate(dev, bt, bitrate_const,
bitrate_const_cnt, extack);
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
@@ -85,10 +85,15 @@ struct can_tdc {
u32 tdcv;
u32 tdco;
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
*
* @tdcv_min: Transmitter Delay Compensation Value minimum value. If
@@ -201,10 +206,14 @@ int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt,
const struct can_bittiming_const *btc,
const u32 *bitrate_const,
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
*
* struct can_tdc::tdco represents the absolute offset from TDCV. Some
* controllers use instead an offset relative to the Sample Point (SP)
@@ -243,6 +252,19 @@ static inline s32 can_get_relative_tdco(const struct data_bittiming_params *dbt_
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.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 09/15] can: calc_bittiming: add PWM calculation
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (7 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 08/15] can: bittiming: add PWM validation Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 10/15] can: netlink: add PWM netlink interface Oliver Hartkopp
` (6 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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 | 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
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-only
/* 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>
#include <linux/can/dev.h>
@@ -196,5 +197,40 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const,
return;
tdc->tdco = min(sample_point_in_tc, tdc_const->tdco_max);
*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
@@ -178,10 +178,12 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
const struct can_bittiming_const *btc, struct netlink_ext_ack *extack);
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,
const struct can_bittiming_const *btc, struct netlink_ext_ack *extack)
{
@@ -193,10 +195,18 @@ static inline 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)
{
}
+
+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);
int can_sjw_check(const struct net_device *dev, const struct can_bittiming *bt,
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 10/15] can: netlink: add PWM netlink interface
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (8 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 09/15] can: calc_bittiming: add PWM calculation Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 11/15] can: calc_bittiming: get rid of the incorrect "nominal" word Oliver Hartkopp
` (5 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
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 5a628c629109..72b9a094ea83 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -23,10 +23,11 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
[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 },
+ [IFLA_CAN_XL_PWM] = { .type = NLA_NESTED },
};
static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
[IFLA_CAN_TDC_TDCV_MIN] = { .type = NLA_U32 },
[IFLA_CAN_TDC_TDCV_MAX] = { .type = NLA_U32 },
@@ -37,10 +38,22 @@ static const struct nla_policy can_tdc_policy[IFLA_CAN_TDC_MAX + 1] = {
[IFLA_CAN_TDC_TDCV] = { .type = NLA_U32 },
[IFLA_CAN_TDC_TDCO] = { .type = NLA_U32 },
[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)
{
struct can_bittiming *bt;
@@ -117,10 +130,44 @@ 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)
{
struct nlattr *data_tdc;
@@ -245,10 +292,14 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
err = can_validate_databittiming(data, extack,
IFLA_CAN_XL_DATA_BITTIMING, flags);
if (err)
return err;
+ err = can_validate_pwm(data, extack, flags);
+ if (err)
+ return err;
+
return 0;
}
static int can_ctrlmode_changelink(struct net_device *dev,
struct nlattr *data[],
@@ -320,10 +371,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
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));
+ memset(&priv->xl.pwm, 0, sizeof(priv->xl.pwm));
}
can_set_default_mtu(dev);
return 0;
@@ -466,10 +518,80 @@ 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)
{
struct can_priv *priv = netdev_priv(dev);
@@ -557,10 +679,13 @@ 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;
+ err = can_pwm_changelink(dev, data[IFLA_CAN_XL_PWM], extack);
if (err)
return err;
if (data[IFLA_CAN_TERMINATION]) {
const u16 termval = nla_get_u16(data[IFLA_CAN_TERMINATION]);
@@ -645,10 +770,34 @@ static size_t can_ctrlmode_ext_get_size(void)
{
return nla_total_size(0) + /* nest IFLA_CAN_CTRLMODE_EXT */
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);
size_t size = 0;
@@ -676,10 +825,12 @@ 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);
+ size += can_pwm_get_size(priv->xl.pwm_const, /* IFLA_CAN_XL_PWM */
+ priv->ctrlmode & CAN_CTRLMODE_XL_TMS);
return size;
}
static int can_bittiming_fill_info(struct sk_buff *skb, int ifla_can_bittiming,
@@ -774,10 +925,46 @@ static int can_tdc_fill_info(struct sk_buff *skb, const struct net_device *dev,
err_cancel:
nla_nest_cancel(skb, nest);
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)
{
struct nlattr *nest;
@@ -857,13 +1044,14 @@ static int can_fill_info(struct sk_buff *skb, const struct net_device *dev)
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)
- )
+ 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 ebafb091d80f..c30d16746159 100644
--- a/include/uapi/linux/can/netlink.h
+++ b/include/uapi/linux/can/netlink.h
@@ -3,10 +3,11 @@
* linux/can/netlink.h
*
* 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
* as published by the Free Software Foundation
*
@@ -145,10 +146,11 @@ enum {
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,
+ IFLA_CAN_XL_PWM,
/* add new constants above here */
__IFLA_CAN_MAX,
IFLA_CAN_MAX = __IFLA_CAN_MAX - 1
};
@@ -186,9 +188,32 @@ enum {
/* add new constants above here */
__IFLA_CAN_CTRLMODE,
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
#endif /* !_UAPI_CAN_NETLINK_H */
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 11/15] can: calc_bittiming: get rid of the incorrect "nominal" word
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (9 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 10/15] can: netlink: add PWM netlink interface Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 12/15] can: calc_bittiming: add can_calc_sample_point_nrz() Oliver Hartkopp
` (4 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
The functions can_update_sample_point() and can_calc_bittiming() are
generic and meant to be used for both the nominal and the data
bittiming calculation.
However, those functions use terminologies such as "bitrate nominal"
or "sample point nominal". This is a leftover from when only Classical
CAN was supported and now became incorrect.
Remove or replace any occurrences of the word "nominal" with something
more accurate.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
drivers/net/can/dev/calc_bittiming.c | 30 +++++++++++++---------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 268ec6fa7c49..222117596704 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -22,35 +22,34 @@
* registers of the CAN controller. You can find more information
* in the header file linux/can/netlink.h.
*/
static int
can_update_sample_point(const struct can_bittiming_const *btc,
- const unsigned int sample_point_nominal, const unsigned int tseg,
+ unsigned int sp_origin, unsigned int tseg,
unsigned int *tseg1_ptr, unsigned int *tseg2_ptr,
unsigned int *sample_point_error_ptr)
{
unsigned int sample_point_error, best_sample_point_error = UINT_MAX;
unsigned int sample_point, best_sample_point = 0;
unsigned int tseg1, tseg2;
int i;
for (i = 0; i <= 1; i++) {
tseg2 = tseg + CAN_SYNC_SEG -
- (sample_point_nominal * (tseg + CAN_SYNC_SEG)) /
- 1000 - i;
+ (sp_origin * (tseg + CAN_SYNC_SEG)) / 1000 - i;
tseg2 = clamp(tseg2, btc->tseg2_min, btc->tseg2_max);
tseg1 = tseg - tseg2;
if (tseg1 > btc->tseg1_max) {
tseg1 = btc->tseg1_max;
tseg2 = tseg - tseg1;
}
sample_point = 1000 * (tseg + CAN_SYNC_SEG - tseg2) /
(tseg + CAN_SYNC_SEG);
- sample_point_error = abs(sample_point_nominal - sample_point);
+ sample_point_error = abs(sp_origin - sample_point);
- if (sample_point <= sample_point_nominal &&
+ if (sample_point <= sp_origin &&
sample_point_error < best_sample_point_error) {
best_sample_point = sample_point;
best_sample_point_error = sample_point_error;
*tseg1_ptr = tseg1;
*tseg2_ptr = tseg2;
@@ -66,31 +65,31 @@ can_update_sample_point(const struct can_bittiming_const *btc,
int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
const struct can_bittiming_const *btc, struct netlink_ext_ack *extack)
{
struct can_priv *priv = netdev_priv(dev);
unsigned int bitrate; /* current bitrate */
- unsigned int bitrate_error; /* difference between current and nominal value */
+ unsigned int bitrate_error; /* difference between current and calculated value */
unsigned int best_bitrate_error = UINT_MAX;
- unsigned int sample_point_error; /* difference between current and nominal value */
+ unsigned int sample_point_error; /* difference between current and calculated value */
unsigned int best_sample_point_error = UINT_MAX;
- unsigned int sample_point_nominal; /* nominal sample point */
+ unsigned int sample_point;
unsigned int best_tseg = 0; /* current best value for tseg */
unsigned int best_brp = 0; /* current best value for brp */
unsigned int brp, tsegall, tseg, tseg1 = 0, tseg2 = 0;
u64 v64;
int err;
/* Use CiA recommended sample points */
if (bt->sample_point) {
- sample_point_nominal = bt->sample_point;
+ sample_point = bt->sample_point;
} else {
if (bt->bitrate > 800 * KILO /* BPS */)
- sample_point_nominal = 750;
+ sample_point = 750;
else if (bt->bitrate > 500 * KILO /* BPS */)
- sample_point_nominal = 800;
+ sample_point = 800;
else
- sample_point_nominal = 875;
+ sample_point = 875;
}
/* tseg even = round down, odd = round up */
for (tseg = (btc->tseg1_max + btc->tseg2_max) * 2 + 1;
tseg >= (btc->tseg1_min + btc->tseg2_min) * 2; tseg--) {
@@ -113,11 +112,11 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
/* reset sample point error if we have a better bitrate */
if (bitrate_error < best_bitrate_error)
best_sample_point_error = UINT_MAX;
- can_update_sample_point(btc, sample_point_nominal, tseg / 2,
+ can_update_sample_point(btc, sample_point, tseg / 2,
&tseg1, &tseg2, &sample_point_error);
if (sample_point_error >= best_sample_point_error)
continue;
best_sample_point_error = sample_point_error;
@@ -144,13 +143,12 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
"bitrate error: %u.%u%%",
bitrate_error / 10, bitrate_error % 10);
}
/* real sample point */
- bt->sample_point = can_update_sample_point(btc, sample_point_nominal,
- best_tseg, &tseg1, &tseg2,
- NULL);
+ bt->sample_point = can_update_sample_point(btc, sample_point, best_tseg,
+ &tseg1, &tseg2, NULL);
v64 = (u64)best_brp * 1000 * 1000 * 1000;
do_div(v64, priv->clock.freq);
bt->tq = (u32)v64;
bt->prop_seg = tseg1 / 2;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 12/15] can: calc_bittiming: add can_calc_sample_point_nrz()
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (10 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 11/15] can: calc_bittiming: get rid of the incorrect "nominal" word Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
` (3 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
CAN XL optimal sample point for PWM encoding (when TMS is on) differs
from the NRZ optimal one. There is thus a need to calculate a
different sample point depending whether TMS is on or off.
This is a preparation change: move the sample point calculation from
can_calc_bittiming() into the new can_calc_sample_point_nrz()
function.
In an upcoming change, a function will be added to calculate the
sample point for PWM encoding.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
drivers/net/can/dev/calc_bittiming.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 222117596704..9b2d0e458518 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -8,10 +8,22 @@
#include <linux/units.h>
#include <linux/can/dev.h>
#define CAN_CALC_MAX_ERROR 50 /* in one-tenth of a percent */
+/* CiA recommended sample points for Non Return to Zero encoding. */
+static int can_calc_sample_point_nrz(const struct can_bittiming *bt)
+{
+ if (bt->bitrate > 800 * KILO /* BPS */)
+ return 750;
+
+ if (bt->bitrate > 500 * KILO /* BPS */)
+ return 800;
+
+ return 875;
+}
+
/* Bit-timing calculation derived from:
*
* Code based on LinCAN sources and H8S2638 project
* Copyright 2004-2006 Pavel Pisa - DCE FELK CVUT cz
* Copyright 2005 Stanislav Marek
@@ -76,21 +88,14 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
unsigned int best_brp = 0; /* current best value for brp */
unsigned int brp, tsegall, tseg, tseg1 = 0, tseg2 = 0;
u64 v64;
int err;
- /* Use CiA recommended sample points */
- if (bt->sample_point) {
+ if (bt->sample_point)
sample_point = bt->sample_point;
- } else {
- if (bt->bitrate > 800 * KILO /* BPS */)
- sample_point = 750;
- else if (bt->bitrate > 500 * KILO /* BPS */)
- sample_point = 800;
- else
- sample_point = 875;
- }
+ else
+ sample_point = can_calc_sample_point_nrz(bt);
/* tseg even = round down, odd = round up */
for (tseg = (btc->tseg1_max + btc->tseg2_max) * 2 + 1;
tseg >= (btc->tseg1_min + btc->tseg2_min) * 2; tseg--) {
tsegall = CAN_SYNC_SEG + tseg / 2;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm()
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (11 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 12/15] can: calc_bittiming: add can_calc_sample_point_nrz() Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-16 20:05 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 14/15] can: add dummy_can driver Oliver Hartkopp
` (2 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
The optimum sample point value depends on the bit symmetry. The more
asymmetric the bit is, the more the sample point would be located
towards the end of the bit. On the contrary, if the transceiver only
has a small asymmetry, the optimal sample point would be slightly
after the centre of the bit.
For NRZ encoding (used by Classical CAN, CAN FD and CAN XL with TMS
off), the optimum sample points values are above 70% as implemented in
can_calc_sample_point_nrz().
When TMS is on, CAN XL optimum sample points are near to 50% or
60% [1]. Add can_calc_sample_point_pwm() which returns a sample point
which is suitable for PWM encoding. We crafted the formula to make it
return the same values as below table (source: table 3 of [1]).
Bit rate (Mbits/s) Sample point
-------------------------------------
2.0 51.3%
5.0 53.1%
8.0 55.0%
10.0 56.3%
12.3 53.8%
13.3 58.3%
14.5 54.5%
16.0 60.0%
17.7 55.6%
20.0 62.5%
The calculation simply consists of setting a slightly too high sample
point and then letting can_update_sample_point() correct the values.
For now, it is just a formula up our sleeves which matches the
empirical observations of [1]. Once CiA recommendations become
available, can_calc_sample_point_pwm() should be updated accordingly.
[1] CAN XL system design: Clock tolerances and edge deviations edge
deviations
Link: 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
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
drivers/net/can/dev/calc_bittiming.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 9b2d0e458518..be6726dcd7e7 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -20,10 +20,25 @@ static int can_calc_sample_point_nrz(const struct can_bittiming *bt)
return 800;
return 875;
}
+/* Sample points for Pulse-Width Modulation encoding. */
+static int can_calc_sample_point_pwm(const struct can_bittiming *bt)
+{
+ if (bt->bitrate > 15 * MEGA /* BPS */)
+ return 625;
+
+ if (bt->bitrate > 9 * MEGA /* BPS */)
+ return 600;
+
+ if (bt->bitrate > 4 * MEGA /* BPS */)
+ return 560;
+
+ return 520;
+}
+
/* Bit-timing calculation derived from:
*
* Code based on LinCAN sources and H8S2638 project
* Copyright 2004-2006 Pavel Pisa - DCE FELK CVUT cz
* Copyright 2005 Stanislav Marek
@@ -90,10 +105,14 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
u64 v64;
int err;
if (bt->sample_point)
sample_point = bt->sample_point;
+
+ else if (btc == priv->xl.data_bittiming_const &&
+ (priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
+ sample_point = can_calc_sample_point_pwm(bt);
else
sample_point = can_calc_sample_point_nrz(bt);
/* tseg even = round down, odd = round up */
for (tseg = (btc->tseg1_max + btc->tseg2_max) * 2 + 1;
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 14/15] can: add dummy_can driver
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (12 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 15/15] can: raw: instantly reject unsupported CAN frames Oliver Hartkopp
2025-11-19 18:17 ` Mainlining of [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
From: Vincent Mailhol <mailhol@kernel.org>
During the development of CAN XL, we found the need of creating a
dummy CAN XL driver in order to test the new netlink interface. While
this code was initially intended to be some throwaway, it received
some positive feedback.
Add the dummy_can driver. This driver acts similarly to the vcan
interface in the sense that it will echo back any packet it receives.
The difference is that it exposes a set on bittiming parameters as a
real device would and thus must be configured as if it was a real
physical interface.
The driver comes with a debug mode. If debug message are enabled (for
example by enabling CONFIG_CAN_DEBUG_DEVICES), it will print in the
kernel log all the bittiming values, similar to what a:
ip --details link show can0
would do.
This driver is mostly intended for debugging and testing, but some
developers also may want to look at it as a simple reference
implementation.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
drivers/net/can/Kconfig | 17 +++
drivers/net/can/Makefile | 1 +
drivers/net/can/dummy_can.c | 284 ++++++++++++++++++++++++++++++++++++
3 files changed, 302 insertions(+)
create mode 100644 drivers/net/can/dummy_can.c
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index d43d56694667..e15e320db476 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -122,10 +122,27 @@ config CAN_CAN327
Please refer to the documentation for information on how to use it:
Documentation/networking/device_drivers/can/can327.rst
If this driver is built as a module, it will be called can327.
+config CAN_DUMMY
+ tristate "Dummy CAN"
+ help
+ A dummy CAN module supporting Classical CAN, CAN FD and CAN XL. It
+ exposes bittiming values which can be configured through the netlink
+ interface.
+
+ The module will simply echo any frame sent to it. If debug messages
+ are activated, it prints all the CAN bittiming information in the
+ kernel log. Aside from that it does nothing.
+
+ This is convenient for testing the CAN netlink interface. Most of the
+ users will never need this. If unsure, say NO.
+
+ To compile this driver as a module, choose M here: the module will be
+ called dummy-can.
+
config CAN_FLEXCAN
tristate "Support for Freescale FLEXCAN based chips"
depends on OF || COLDFIRE || COMPILE_TEST
depends on HAS_IOMEM
select CAN_RX_OFFLOAD
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 56138d8ddfd2..d7bc10a6b8ea 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -19,10 +19,11 @@ obj-$(CONFIG_CAN_AT91) += at91_can.o
obj-$(CONFIG_CAN_BXCAN) += bxcan.o
obj-$(CONFIG_CAN_CAN327) += can327.o
obj-$(CONFIG_CAN_CC770) += cc770/
obj-$(CONFIG_CAN_C_CAN) += c_can/
obj-$(CONFIG_CAN_CTUCANFD) += ctucanfd/
+obj-$(CONFIG_CAN_DUMMY) += dummy_can.o
obj-$(CONFIG_CAN_FLEXCAN) += flexcan/
obj-$(CONFIG_CAN_GRCAN) += grcan.o
obj-$(CONFIG_CAN_IFI_CANFD) += ifi_canfd/
obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o
obj-$(CONFIG_CAN_KVASER_PCIEFD) += kvaser_pciefd/
diff --git a/drivers/net/can/dummy_can.c b/drivers/net/can/dummy_can.c
new file mode 100644
index 000000000000..97af0847db00
--- /dev/null
+++ b/drivers/net/can/dummy_can.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* Copyright (c) 2025 Vincent Mailhol <mailhol@kernel.org> */
+
+#include <linux/array_size.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/units.h>
+
+#include <linux/can.h>
+#include <linux/can/bittiming.h>
+#include <linux/can/dev.h>
+#include <linux/can/skb.h>
+
+struct dummy_can {
+ struct can_priv can;
+ struct net_device *dev;
+};
+
+static struct dummy_can *dummy_can;
+
+static const struct can_bittiming_const dummy_can_bittiming_const = {
+ .name = "dummy_can CC",
+ .tseg1_min = 2,
+ .tseg1_max = 256,
+ .tseg2_min = 2,
+ .tseg2_max = 128,
+ .sjw_max = 128,
+ .brp_min = 1,
+ .brp_max = 512,
+ .brp_inc = 1
+};
+
+static const struct can_bittiming_const dummy_can_fd_databittiming_const = {
+ .name = "dummy_can FD",
+ .tseg1_min = 2,
+ .tseg1_max = 256,
+ .tseg2_min = 2,
+ .tseg2_max = 128,
+ .sjw_max = 128,
+ .brp_min = 1,
+ .brp_max = 512,
+ .brp_inc = 1
+};
+
+static const struct can_tdc_const dummy_can_fd_tdc_const = {
+ .tdcv_min = 0,
+ .tdcv_max = 0, /* Manual mode not supported. */
+ .tdco_min = 0,
+ .tdco_max = 127,
+ .tdcf_min = 0,
+ .tdcf_max = 127
+};
+
+static const struct can_bittiming_const dummy_can_xl_databittiming_const = {
+ .name = "dummy_can XL",
+ .tseg1_min = 2,
+ .tseg1_max = 256,
+ .tseg2_min = 2,
+ .tseg2_max = 128,
+ .sjw_max = 128,
+ .brp_min = 1,
+ .brp_max = 512,
+ .brp_inc = 1
+};
+
+static const struct can_tdc_const dummy_can_xl_tdc_const = {
+ .tdcv_min = 0,
+ .tdcv_max = 0, /* Manual mode not supported. */
+ .tdco_min = 0,
+ .tdco_max = 127,
+ .tdcf_min = 0,
+ .tdcf_max = 127
+};
+
+static const struct can_pwm_const dummy_can_pwm_const = {
+ .pwms_min = 1,
+ .pwms_max = 8,
+ .pwml_min = 2,
+ .pwml_max = 24,
+ .pwmo_min = 0,
+ .pwmo_max = 16,
+};
+
+static void dummy_can_print_bittiming(struct net_device *dev,
+ struct can_bittiming *bt)
+{
+ netdev_dbg(dev, "\tbitrate: %u\n", bt->bitrate);
+ netdev_dbg(dev, "\tsample_point: %u\n", bt->sample_point);
+ netdev_dbg(dev, "\ttq: %u\n", bt->tq);
+ netdev_dbg(dev, "\tprop_seg: %u\n", bt->prop_seg);
+ netdev_dbg(dev, "\tphase_seg1: %u\n", bt->phase_seg1);
+ netdev_dbg(dev, "\tphase_seg2: %u\n", bt->phase_seg2);
+ netdev_dbg(dev, "\tsjw: %u\n", bt->sjw);
+ netdev_dbg(dev, "\tbrp: %u\n", bt->brp);
+}
+
+static void dummy_can_print_tdc(struct net_device *dev, struct can_tdc *tdc)
+{
+ netdev_dbg(dev, "\t\ttdcv: %u\n", tdc->tdcv);
+ netdev_dbg(dev, "\t\ttdco: %u\n", tdc->tdco);
+ netdev_dbg(dev, "\t\ttdcf: %u\n", tdc->tdcf);
+}
+
+static void dummy_can_print_pwm(struct net_device *dev, struct can_pwm *pwm,
+ struct can_bittiming *dbt)
+{
+ netdev_dbg(dev, "\t\tpwms: %u\n", pwm->pwms);
+ netdev_dbg(dev, "\t\tpwml: %u\n", pwm->pwml);
+ netdev_dbg(dev, "\t\tpwmo: %u\n", pwm->pwmo);
+}
+
+static void dummy_can_print_ctrlmode(struct net_device *dev)
+{
+ struct dummy_can *priv = netdev_priv(dev);
+ struct can_priv *can_priv = &priv->can;
+ unsigned long supported = can_priv->ctrlmode_supported;
+ u32 enabled = can_priv->ctrlmode;
+
+ netdev_dbg(dev, "Control modes:\n");
+ netdev_dbg(dev, "\tsupported: 0x%08x\n", (u32)supported);
+ netdev_dbg(dev, "\tenabled: 0x%08x\n", enabled);
+
+ if (supported) {
+ int idx;
+
+ netdev_dbg(dev, "\tlist:");
+ for_each_set_bit(idx, &supported, BITS_PER_TYPE(u32))
+ netdev_dbg(dev, "\t\t%s: %s\n",
+ can_get_ctrlmode_str(BIT(idx)),
+ enabled & BIT(idx) ? "on" : "off");
+ }
+}
+
+static void dummy_can_print_bittiming_info(struct net_device *dev)
+{
+ struct dummy_can *priv = netdev_priv(dev);
+ struct can_priv *can_priv = &priv->can;
+
+ netdev_dbg(dev, "Clock frequency: %u\n", can_priv->clock.freq);
+ netdev_dbg(dev, "Maximum bitrate: %u\n", can_priv->bitrate_max);
+ netdev_dbg(dev, "MTU: %u\n", dev->mtu);
+ netdev_dbg(dev, "\n");
+
+ dummy_can_print_ctrlmode(dev);
+ netdev_dbg(dev, "\n");
+
+ netdev_dbg(dev, "Classical CAN nominal bittiming:\n");
+ dummy_can_print_bittiming(dev, &can_priv->bittiming);
+ netdev_dbg(dev, "\n");
+
+ if (can_priv->ctrlmode & CAN_CTRLMODE_FD) {
+ netdev_dbg(dev, "CAN FD databittiming:\n");
+ dummy_can_print_bittiming(dev, &can_priv->fd.data_bittiming);
+ if (can_fd_tdc_is_enabled(can_priv)) {
+ netdev_dbg(dev, "\tCAN FD TDC:\n");
+ dummy_can_print_tdc(dev, &can_priv->fd.tdc);
+ }
+ }
+ netdev_dbg(dev, "\n");
+
+ if (can_priv->ctrlmode & CAN_CTRLMODE_XL) {
+ netdev_dbg(dev, "CAN XL databittiming:\n");
+ dummy_can_print_bittiming(dev, &can_priv->xl.data_bittiming);
+ if (can_xl_tdc_is_enabled(can_priv)) {
+ netdev_dbg(dev, "\tCAN XL TDC:\n");
+ dummy_can_print_tdc(dev, &can_priv->xl.tdc);
+ }
+ if (can_priv->ctrlmode & CAN_CTRLMODE_XL_TMS) {
+ netdev_dbg(dev, "\tCAN XL PWM:\n");
+ dummy_can_print_pwm(dev, &can_priv->xl.pwm,
+ &can_priv->xl.data_bittiming);
+ }
+ }
+ netdev_dbg(dev, "\n");
+}
+
+static int dummy_can_netdev_open(struct net_device *dev)
+{
+ int ret;
+ struct can_priv *priv = netdev_priv(dev);
+
+ dummy_can_print_bittiming_info(dev);
+ netdev_dbg(dev, "error-signalling is %sabled\n",
+ can_dev_in_xl_only_mode(priv)?"dis":"en");
+
+ ret = open_candev(dev);
+ if (ret)
+ return ret;
+ netif_start_queue(dev);
+ netdev_dbg(dev, "dummy-can is up\n");
+
+ return 0;
+}
+
+static int dummy_can_netdev_close(struct net_device *dev)
+{
+ netif_stop_queue(dev);
+ close_candev(dev);
+ netdev_dbg(dev, "dummy-can is down\n");
+
+ return 0;
+}
+
+static netdev_tx_t dummy_can_start_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ if (can_dev_dropped_skb(dev, skb))
+ return NETDEV_TX_OK;
+
+ can_put_echo_skb(skb, dev, 0, 0);
+ dev->stats.tx_packets++;
+ dev->stats.tx_bytes += can_get_echo_skb(dev, 0, NULL);
+
+ return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops dummy_can_netdev_ops = {
+ .ndo_open = dummy_can_netdev_open,
+ .ndo_stop = dummy_can_netdev_close,
+ .ndo_start_xmit = dummy_can_start_xmit,
+};
+
+static const struct ethtool_ops dummy_can_ethtool_ops = {
+ .get_ts_info = ethtool_op_get_ts_info,
+};
+
+static int __init dummy_can_init(void)
+{
+ struct net_device *dev;
+ struct dummy_can *priv;
+ int ret;
+
+ dev = alloc_candev(sizeof(*priv), 1);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->netdev_ops = &dummy_can_netdev_ops;
+ dev->ethtool_ops = &dummy_can_ethtool_ops;
+ priv = netdev_priv(dev);
+ priv->can.bittiming_const = &dummy_can_bittiming_const;
+ priv->can.bitrate_max = 20 * MEGA /* BPS */;
+ priv->can.clock.freq = 160 * MEGA /* Hz */;
+ priv->can.fd.data_bittiming_const = &dummy_can_fd_databittiming_const;
+ priv->can.fd.tdc_const = &dummy_can_fd_tdc_const;
+ priv->can.xl.data_bittiming_const = &dummy_can_xl_databittiming_const;
+ priv->can.xl.tdc_const = &dummy_can_xl_tdc_const;
+ priv->can.xl.pwm_const = &dummy_can_pwm_const;
+ priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
+ CAN_CTRLMODE_FD | CAN_CTRLMODE_TDC_AUTO |
+ CAN_CTRLMODE_RESTRICTED | CAN_CTRLMODE_XL |
+ CAN_CTRLMODE_XL_TDC_AUTO | CAN_CTRLMODE_XL_TMS;
+ priv->dev = dev;
+
+ ret = register_candev(priv->dev);
+ if (ret) {
+ free_candev(priv->dev);
+ return ret;
+ }
+
+ dummy_can = priv;
+ netdev_dbg(dev, "dummy-can ready\n");
+
+ return 0;
+}
+
+static void __exit dummy_can_exit(void)
+{
+ struct net_device *dev = dummy_can->dev;
+
+ netdev_dbg(dev, "dummy-can bye bye\n");
+ unregister_candev(dev);
+ free_candev(dev);
+}
+
+module_init(dummy_can_init);
+module_exit(dummy_can_exit);
+
+MODULE_DESCRIPTION("A dummy CAN driver, mainly to test the netlink interface");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Vincent Mailhol <mailhol@kernel.org>");
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [canxl v2 15/15] can: raw: instantly reject unsupported CAN frames
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (13 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 14/15] can: add dummy_can driver Oliver Hartkopp
@ 2025-11-15 16:37 ` Oliver Hartkopp
2025-11-19 18:17 ` Mainlining of [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
15 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-15 16:37 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
For real CAN interfaces the CAN_CTRLMODE_FD and CAN_CTRLMODE_XL control
modes indicate whether an interface can handle those CAN FD/XL frames.
In the case a CAN XL interface is configured in CANXL-only mode with
disabled error-signalling neither CAN CC nor CAN FD frames can be sent.
The checks are performed on CAN_RAW sockets to give an instant feedback
to the user when writing unsupported CAN frames to the interface.
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
net/can/raw.c | 54 +++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 8 deletions(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index f36a83d3447c..be1ef7cf4204 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -890,24 +890,62 @@ static void raw_put_canxl_vcid(struct raw_sock *ro, struct sk_buff *skb)
cxl->prio &= CANXL_PRIO_MASK;
cxl->prio |= ro->tx_vcid_shifted;
}
}
-static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb, int mtu)
+static inline bool raw_dev_cc_enabled(struct net_device *dev,
+ struct can_priv *priv)
{
- /* Classical CAN -> no checks for flags and device capabilities */
- if (can_is_can_skb(skb))
+ /* The CANXL-only mode disables error-signalling on the CAN bus
+ * which is needed to send CAN CC/FD frames
+ */
+ if (priv)
+ return !can_dev_in_xl_only_mode(priv);
+
+ /* virtual CAN interfaces always support CAN CC */
+ return true;
+}
+
+static inline bool raw_dev_fd_enabled(struct net_device *dev,
+ struct can_priv *priv)
+{
+ /* check FD ctrlmode on real CAN interfaces */
+ if (priv)
+ return (priv->ctrlmode & CAN_CTRLMODE_FD);
+
+ /* check MTU for virtual CAN FD interfaces */
+ return (READ_ONCE(dev->mtu) >= CANFD_MTU);
+}
+
+static inline bool raw_dev_xl_enabled(struct net_device *dev,
+ struct can_priv *priv)
+{
+ /* check XL ctrlmode on real CAN interfaces */
+ if (priv)
+ return (priv->ctrlmode & CAN_CTRLMODE_XL);
+
+ /* check MTU for virtual CAN XL interfaces */
+ return can_is_canxl_dev_mtu(READ_ONCE(dev->mtu));
+}
+
+static unsigned int raw_check_txframe(struct raw_sock *ro, struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct can_priv *priv = safe_candev_priv(dev);
+
+ /* Classical CAN */
+ if (can_is_can_skb(skb) && raw_dev_cc_enabled(dev, priv))
return CAN_MTU;
- /* CAN FD -> needs to be enabled and a CAN FD or CAN XL device */
+ /* CAN FD */
if (ro->fd_frames && can_is_canfd_skb(skb) &&
- (mtu == CANFD_MTU || can_is_canxl_dev_mtu(mtu)))
+ raw_dev_fd_enabled(dev, priv))
return CANFD_MTU;
- /* CAN XL -> needs to be enabled and a CAN XL device */
+ /* CAN XL */
if (ro->xl_frames && can_is_canxl_skb(skb) &&
- can_is_canxl_dev_mtu(mtu))
+ raw_dev_xl_enabled(dev, priv))
return CANXL_MTU;
return 0;
}
@@ -959,11 +997,11 @@ static int raw_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
goto free_skb;
err = -EINVAL;
/* check for valid CAN (CC/FD/XL) frame content */
- txmtu = raw_check_txframe(ro, skb, READ_ONCE(dev->mtu));
+ txmtu = raw_check_txframe(ro, skb, dev);
if (!txmtu)
goto free_skb;
/* only CANXL: clear/forward/set VCID value */
if (txmtu == CANXL_MTU)
--
2.47.3
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-15 16:37 ` [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
@ 2025-11-16 19:31 ` Oliver Hartkopp
2025-11-16 21:54 ` Vincent Mailhol
0 siblings, 1 reply; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-16 19:31 UTC (permalink / raw)
To: Vincent Mailhol; +Cc: linux-can
Hi Vincent,
On 15.11.25 17:37, Oliver Hartkopp wrote:
> From: Vincent Mailhol <mailhol@kernel.org>
>
> 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.
>
> 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>
> ---
> drivers/net/can/dev/dev.c | 2 ++
> drivers/net/can/dev/netlink.c | 48 ++++++++++++++++++++++++++++++--
> include/uapi/linux/can/netlink.h | 1 +
> 3 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 443692587217..9da3da8c6225 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -121,10 +121,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
> return "xl";
> case CAN_CTRLMODE_XL_TDC_AUTO:
> 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>";
> }
> }
> EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> index 26c25e660e31..5a628c629109 100644
> --- a/drivers/net/can/dev/netlink.c
> +++ b/drivers/net/can/dev/netlink.c
> @@ -179,10 +179,36 @@ static int can_validate_databittiming(struct nlattr *data[],
> return err;
>
> 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 (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));
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 dbitrate
2000000 fd on xbitrate 4000000 xl on tms on
Error: TMS and fd are mutually exclusive.
The error messages should look consistent in terms of capitalization.
Maybe can_get_ctrlmode_str() should deliver capitalized strings as we
see it in the 'ip' tool output:
root@de1soc1:~# ./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 <FD,TDC-AUTO,XL,XL-TDC-AUTO> state STOPPED restart-ms 0
> + return -EOPNOTSUPP;
> + }
> + }
> + } else {
> + if (mask & CAN_CTRLMODE_XL_TMS) {
> + NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
This looks good btw.
> + return -EOPNOTSUPP;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int can_validate(struct nlattr *tb[], struct nlattr *data[],
> struct netlink_ext_ack *extack)
> {
> u32 flags = 0;
> int err;
> @@ -199,10 +225,14 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
> (flags & CAN_CTRLMODE_RESTRICTED)) {
> NL_SET_ERR_MSG(extack,
> "Listen-only and restricted modes are mutually exclusive");
IMO this should also be capitalized ...
"LISTEN-MODE 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);
> if (err)
> return err;
> @@ -224,11 +254,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
> struct nlattr *data[],
> struct netlink_ext_ack *extack)
> {
> struct can_priv *priv = netdev_priv(dev);
> struct can_ctrlmode *cm;
> - u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
> + u32 ctrlstatic, maskedflags, deactivated, notsupp, ctrlstatic_missing;
>
> if (!data[IFLA_CAN_CTRLMODE])
> return 0;
>
> /* Do not allow changing controller mode while running */
> @@ -236,10 +266,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
> return -EBUSY;
>
> 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;
>
> if (notsupp) {
> NL_SET_ERR_MSG_FMT(extack,
> @@ -257,15 +288,25 @@ static int can_ctrlmode_changelink(struct net_device *dev,
> "missing required %s static control mode",
> can_get_ctrlmode_str(ctrlstatic_missing));
> 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");
"TMS can not be activated while FD is on");
And this also.
Best regards,
Oliver
> + 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;
> priv->ctrlmode |= maskedflags;
>
> @@ -393,11 +434,12 @@ static int can_dbt_changelink(struct net_device *dev, struct nlattr *data[],
>
> memset(&dbt_params->tdc, 0, sizeof(dbt_params->tdc));
> 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 */
> err = can_tdc_changelink(dbt_params, data_tdc, extack);
> if (err) {
> 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
> @@ -105,10 +105,11 @@ struct can_ctrlmode {
> #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 */
> +#define CAN_CTRLMODE_XL_TMS 0x8000 /* Transceiver Mode Switching */
>
> /*
> * CAN device statistics
> */
> struct can_device_stats {
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm()
2025-11-15 16:37 ` [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
@ 2025-11-16 20:05 ` Oliver Hartkopp
2025-11-16 22:18 ` Vincent Mailhol
0 siblings, 1 reply; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-16 20:05 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol
Hi Vincent,
On 15.11.25 17:37, Oliver Hartkopp wrote:
> From: Vincent Mailhol <mailhol@kernel.org>
>
> The optimum sample point value depends on the bit symmetry. The more
> asymmetric the bit is, the more the sample point would be located
> towards the end of the bit. On the contrary, if the transceiver only
> has a small asymmetry, the optimal sample point would be slightly
> after the centre of the bit.
>
> For NRZ encoding (used by Classical CAN, CAN FD and CAN XL with TMS
> off), the optimum sample points values are above 70% as implemented in
> can_calc_sample_point_nrz().
>
> When TMS is on, CAN XL optimum sample points are near to 50% or
> 60% [1]. Add can_calc_sample_point_pwm() which returns a sample point
> which is suitable for PWM encoding. We crafted the formula to make it
> return the same values as below table (source: table 3 of [1]).
>
> Bit rate (Mbits/s) Sample point
> -------------------------------------
> 2.0 51.3%
> 5.0 53.1%
> 8.0 55.0%
> 10.0 56.3%
> 12.3 53.8%
> 13.3 58.3%
> 14.5 54.5%
> 16.0 60.0%
> 17.7 55.6%
> 20.0 62.5%
I tested all these examples.
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
4000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
2000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
5000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
8000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
10000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
12300000 xl on tms on
Warning: bitrate error: 0.0%.
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
13300000 xl on tms on
Warning: bitrate error: 0.2%.
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
14500000 xl on tms on
Warning: bitrate error: 0.3%.
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
16000000 xl on tms on
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
17700000 xl on tms on
Warning: bitrate error: 0.4%.
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
20000000 xl on tms on
root@de1soc1:~#
In the case of xbitrate 12300000 the feedback is
Warning: bitrate error: 0.0%.
The calculated bitrate is:
xbitrate 12307692 xsample-point 0.538
12307692/12300000 = 1.00062536585
So it is 0.06%
root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate
13300000 xl on tms on
Warning: bitrate error: 0.2%.
The calculated bitrate is:
xbitrate 13333333 xsample-point 0.583
13333333/13300000 = 1.0025062406
So it is 0.25%
Would it make sense to add another digit and probably additionally some
round-up to omit a 0.0% warning?
Best regards,
Oliver
>
> The calculation simply consists of setting a slightly too high sample
> point and then letting can_update_sample_point() correct the values.
>
> For now, it is just a formula up our sleeves which matches the
> empirical observations of [1]. Once CiA recommendations become
> available, can_calc_sample_point_pwm() should be updated accordingly.
>
> [1] CAN XL system design: Clock tolerances and edge deviations edge
> deviations
> Link: 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
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> drivers/net/can/dev/calc_bittiming.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
> index 9b2d0e458518..be6726dcd7e7 100644
> --- a/drivers/net/can/dev/calc_bittiming.c
> +++ b/drivers/net/can/dev/calc_bittiming.c
> @@ -20,10 +20,25 @@ static int can_calc_sample_point_nrz(const struct can_bittiming *bt)
> return 800;
>
> return 875;
> }
>
> +/* Sample points for Pulse-Width Modulation encoding. */
> +static int can_calc_sample_point_pwm(const struct can_bittiming *bt)
> +{
> + if (bt->bitrate > 15 * MEGA /* BPS */)
> + return 625;
> +
> + if (bt->bitrate > 9 * MEGA /* BPS */)
> + return 600;
> +
> + if (bt->bitrate > 4 * MEGA /* BPS */)
> + return 560;
> +
> + return 520;
> +}
> +
> /* Bit-timing calculation derived from:
> *
> * Code based on LinCAN sources and H8S2638 project
> * Copyright 2004-2006 Pavel Pisa - DCE FELK CVUT cz
> * Copyright 2005 Stanislav Marek
> @@ -90,10 +105,14 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> u64 v64;
> int err;
>
> if (bt->sample_point)
> sample_point = bt->sample_point;
> +
> + else if (btc == priv->xl.data_bittiming_const &&
> + (priv->ctrlmode & CAN_CTRLMODE_XL_TMS))
> + sample_point = can_calc_sample_point_pwm(bt);
> else
> sample_point = can_calc_sample_point_nrz(bt);
>
> /* tseg even = round down, odd = round up */
> for (tseg = (btc->tseg1_max + btc->tseg2_max) * 2 + 1;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-16 19:31 ` Oliver Hartkopp
@ 2025-11-16 21:54 ` Vincent Mailhol
2025-11-16 22:53 ` Vincent Mailhol
2025-11-17 8:40 ` Oliver Hartkopp
0 siblings, 2 replies; 29+ messages in thread
From: Vincent Mailhol @ 2025-11-16 21:54 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
Hi Oliver,
On 16/11/2025 à 20:31, Oliver Hartkopp wrote:
> Hi Vincent,
>
> On 15.11.25 17:37, Oliver Hartkopp wrote:
>> From: Vincent Mailhol <mailhol@kernel.org>
>>
>> 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.
>>
>> 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>
>> ---
>> drivers/net/can/dev/dev.c | 2 ++
>> drivers/net/can/dev/netlink.c | 48 ++++++++++++++++++++++++++++++--
>> include/uapi/linux/can/netlink.h | 1 +
>> 3 files changed, 48 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>> index 443692587217..9da3da8c6225 100644
>> --- a/drivers/net/can/dev/dev.c
>> +++ b/drivers/net/can/dev/dev.c
>> @@ -121,10 +121,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>> return "xl";
>> case CAN_CTRLMODE_XL_TDC_AUTO:
>> 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>";
>> }
>> }
>> EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>> index 26c25e660e31..5a628c629109 100644
>> --- a/drivers/net/can/dev/netlink.c
>> +++ b/drivers/net/can/dev/netlink.c
>> @@ -179,10 +179,36 @@ static int can_validate_databittiming(struct nlattr
>> *data[],
>> return err;
>> 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 (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));
>
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd
> on xbitrate 4000000 xl on tms on
> Error: TMS and fd are mutually exclusive.
>
> The error messages should look consistent in terms of capitalization.
>
> Maybe can_get_ctrlmode_str() should deliver capitalized strings as we see it in
> the 'ip' tool output:
In a full English sentence, I tend to see ALL CAPITALIZED WORDS as kind of
aggressive. Whereas I find this to be OK in other things which are not sentences
(like the ip link show). So I opted for the lower case option in
can_get_ctrlmode_str(). And then, I messed up here by mixing both as you pointed
here…
This is not really something I care about. The thing is that
can_get_ctrlmode_str() is already upstream, so fixing this would require a
separate patch, which is a tiny bit more work, but not a blocker either.
> root@de1soc1:~# ./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 <FD,TDC-AUTO,XL,XL-TDC-AUTO> state STOPPED restart-ms 0
>
>> + return -EOPNOTSUPP;
>> + }
>> + }
>> + } else {
>> + if (mask & CAN_CTRLMODE_XL_TMS) {
>> + NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
>
> This looks good btw.
>
>> + return -EOPNOTSUPP;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int can_validate(struct nlattr *tb[], struct nlattr *data[],
>> struct netlink_ext_ack *extack)
>> {
>> u32 flags = 0;
>> int err;
>> @@ -199,10 +225,14 @@ static int can_validate(struct nlattr *tb[], struct
>> nlattr *data[],
>> (flags & CAN_CTRLMODE_RESTRICTED)) {
>> NL_SET_ERR_MSG(extack,
>> "Listen-only and restricted modes are mutually
>> exclusive");
>
> IMO this should also be capitalized ...
>
> "LISTEN-MODE and RESTRICTED modes are mutually exclusive");
This is typically the kind of thing where I prefer the lower case. The above
seems as if the error message is shouting at me.
Well, if you still prefer upper case after my explanation, I will change.
>> return -EOPNOTSUPP;
>> }
>> +
>> + err = can_validate_xl_flags(extack, flags, cm->mask);
>> + if (err)
>> + return err;
>> }
>> err = can_validate_bittiming(data, extack, IFLA_CAN_BITTIMING);
>> if (err)
>> return err;
>> @@ -224,11 +254,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>> struct nlattr *data[],
>> struct netlink_ext_ack *extack)
>> {
>> struct can_priv *priv = netdev_priv(dev);
>> struct can_ctrlmode *cm;
>> - u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
>> + u32 ctrlstatic, maskedflags, deactivated, notsupp, ctrlstatic_missing;
>> if (!data[IFLA_CAN_CTRLMODE])
>> return 0;
>> /* Do not allow changing controller mode while running */
>> @@ -236,10 +266,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>> return -EBUSY;
>> 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;
>> if (notsupp) {
>> NL_SET_ERR_MSG_FMT(extack,
>> @@ -257,15 +288,25 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>> "missing required %s static control mode",
>> can_get_ctrlmode_str(ctrlstatic_missing));
>> 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");
> "TMS can not be activated while FD is on");
>
> And this also.
Ack.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm()
2025-11-16 20:05 ` Oliver Hartkopp
@ 2025-11-16 22:18 ` Vincent Mailhol
2025-11-17 8:59 ` Oliver Hartkopp
0 siblings, 1 reply; 29+ messages in thread
From: Vincent Mailhol @ 2025-11-16 22:18 UTC (permalink / raw)
To: Oliver Hartkopp, linux-can
On 16/11/2025 at 21:05, Oliver Hartkopp wrote:
> Hi Vincent,
>
> On 15.11.25 17:37, Oliver Hartkopp wrote:
>> From: Vincent Mailhol <mailhol@kernel.org>
>>
>> The optimum sample point value depends on the bit symmetry. The more
>> asymmetric the bit is, the more the sample point would be located
>> towards the end of the bit. On the contrary, if the transceiver only
>> has a small asymmetry, the optimal sample point would be slightly
>> after the centre of the bit.
>>
>> For NRZ encoding (used by Classical CAN, CAN FD and CAN XL with TMS
>> off), the optimum sample points values are above 70% as implemented in
>> can_calc_sample_point_nrz().
>>
>> When TMS is on, CAN XL optimum sample points are near to 50% or
>> 60% [1]. Add can_calc_sample_point_pwm() which returns a sample point
>> which is suitable for PWM encoding. We crafted the formula to make it
>> return the same values as below table (source: table 3 of [1]).
>>
>> Bit rate (Mbits/s) Sample point
>> -------------------------------------
>> 2.0 51.3%
>> 5.0 53.1%
>> 8.0 55.0%
>> 10.0 56.3%
>> 12.3 53.8%
>> 13.3 58.3%
>> 14.5 54.5%
>> 16.0 60.0%
>> 17.7 55.6%
>> 20.0 62.5%
>
> I tested all these examples.
>
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 4000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 2000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 5000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 8000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 10000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 12300000 xl
> on tms on
> Warning: bitrate error: 0.0%.
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 13300000 xl
> on tms on
> Warning: bitrate error: 0.2%.
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 14500000 xl
> on tms on
> Warning: bitrate error: 0.3%.
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 16000000 xl
> on tms on
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 17700000 xl
> on tms on
> Warning: bitrate error: 0.4%.
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 20000000 xl
> on tms on
> root@de1soc1:~#
>
> In the case of xbitrate 12300000 the feedback is
> Warning: bitrate error: 0.0%.
>
> The calculated bitrate is:
> xbitrate 12307692 xsample-point 0.538
>
> 12307692/12300000 = 1.00062536585
>
> So it is 0.06%
>
> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 13300000 xl
> on tms on
> Warning: bitrate error: 0.2%.
Random unrelated comment: I figured out that if you have both a
NL_SET_ERR_MSG_FMT() Warning and a NL_SET_ERR_MSG() Error message (note the _FMT
suffix only on the warning side), the warning may take precedence over the
Error. Because netlink can only carry one message, it would be better to only
report the error when both error and warning occurs. Note that this is not an
issue in our tree. I wanted to look at it in more details once the CAN XL is
merged (although I do not expect the fix to be complex).
> The calculated bitrate is:
> xbitrate 13333333 xsample-point 0.583
>
> 13333333/13300000 = 1.0025062406
>
> So it is 0.25%
>
> Would it make sense to add another digit and probably additionally some round-up
> to omit a 0.0% warning?
I drafted this:
------------8<------------
can: calc_bittiming: add one decimal place in bitrate error messages
---
drivers/net/can/dev/calc_bittiming.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/dev/calc_bittiming.c
b/drivers/net/can/dev/calc_bittiming.c
index 9c8154859513..45d1dc286197 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -152,19 +152,19 @@ int can_calc_bittiming(const struct net_device *dev,
struct can_bittiming *bt,
}
if (best_bitrate_error) {
- /* Error in one-tenth of a percent */
- v64 = (u64)best_bitrate_error * 1000;
+ /* Error in one-hundredth of a percent */
+ v64 = (u64)best_bitrate_error * 10000;
do_div(v64, bt->bitrate);
bitrate_error = (u32)v64;
if (bitrate_error > CAN_CALC_MAX_ERROR) {
NL_SET_ERR_MSG_FMT(extack,
"bitrate error: %u.%u%% too high",
- bitrate_error / 10, bitrate_error % 10);
+ bitrate_error / 100, bitrate_error % 100);
return -EINVAL;
}
NL_SET_ERR_MSG_FMT(extack,
"bitrate error: %u.%u%%",
- bitrate_error / 10, bitrate_error % 10);
+ bitrate_error / 100, bitrate_error % 100);
}
/* real sample point */
------------8<------------
No time to test at the moment (maybe I will be able to test in a couple days),
but does it look good to you?
Also, this isn't the only place in which we are getting out of significant
digit. This was already a couple months ago, but when I started calculating the
PWM symbols, I figured out that on very high bitrates, can_bittiming->tq will
lack some significant digits because of the decimal truncation. Using this in
CAN XL can become problematic. That's another TODO…
Yours sincerely,
Vincent Mailhol
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-16 21:54 ` Vincent Mailhol
@ 2025-11-16 22:53 ` Vincent Mailhol
2025-11-17 9:06 ` Oliver Hartkopp
2025-11-17 8:40 ` Oliver Hartkopp
1 sibling, 1 reply; 29+ messages in thread
From: Vincent Mailhol @ 2025-11-16 22:53 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can
On 16/11/2025 at 22:54, Vincent Mailhol wrote:
> Hi Oliver,
>
> On 16/11/2025 à 20:31, Oliver Hartkopp wrote:
>> Hi Vincent,
>>
>> On 15.11.25 17:37, Oliver Hartkopp wrote:
>>> From: Vincent Mailhol <mailhol@kernel.org>
>>>
>>> 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.
>>>
>>> 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>
>>> ---
>>> drivers/net/can/dev/dev.c | 2 ++
>>> drivers/net/can/dev/netlink.c | 48 ++++++++++++++++++++++++++++++--
>>> include/uapi/linux/can/netlink.h | 1 +
>>> 3 files changed, 48 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>>> index 443692587217..9da3da8c6225 100644
>>> --- a/drivers/net/can/dev/dev.c
>>> +++ b/drivers/net/can/dev/dev.c
>>> @@ -121,10 +121,12 @@ const char *can_get_ctrlmode_str(u32 ctrlmode)
>>> return "xl";
>>> case CAN_CTRLMODE_XL_TDC_AUTO:
>>> 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>";
>>> }
>>> }
>>> EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
>>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
>>> index 26c25e660e31..5a628c629109 100644
>>> --- a/drivers/net/can/dev/netlink.c
>>> +++ b/drivers/net/can/dev/netlink.c
>>> @@ -179,10 +179,36 @@ static int can_validate_databittiming(struct nlattr
>>> *data[],
>>> return err;
>>> 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 (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));
>>
>> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd
>> on xbitrate 4000000 xl on tms on
>> Error: TMS and fd are mutually exclusive.
>>
>> The error messages should look consistent in terms of capitalization.
>>
>> Maybe can_get_ctrlmode_str() should deliver capitalized strings as we see it in
>> the 'ip' tool output:
>
> In a full English sentence, I tend to see ALL CAPITALIZED WORDS as kind of
> aggressive. Whereas I find this to be OK in other things which are not sentences
> (like the ip link show). So I opted for the lower case option in
> can_get_ctrlmode_str(). And then, I messed up here by mixing both as you pointed
> here…
>
> This is not really something I care about. The thing is that
> can_get_ctrlmode_str() is already upstream, so fixing this would require a
> separate patch, which is a tiny bit more work, but not a blocker either.
I just remembered that there are a couple more reasons why I preferred to use
lower case here:
- can_get_state_str() which is just above can_get_ctrlmode_str() also uses
lower case. Because can_get_ctrlmode_str() may be used by for things other
than the netlink error reporting, consistency with can_get_state_str() also
matters.
- the ip tool command line also use lower case (xl on tms on…). So better to
match what the user just entered on the command line just above the error
message rather than the output of "ip link show".
I actually wrote can_get_ctrlmode_str() a very long time ago (somewhere in
December or January) before putting the series on hiatus because of the unknown
PWM calculation. So it is hard to remember what went through my mind at that time ;)
>> root@de1soc1:~# ./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 <FD,TDC-AUTO,XL,XL-TDC-AUTO> state STOPPED restart-ms 0
>>
>>> + return -EOPNOTSUPP;
>>> + }
>>> + }
>>> + } else {
>>> + if (mask & CAN_CTRLMODE_XL_TMS) {
>>> + NL_SET_ERR_MSG(extack, "TMS requires CAN XL");
>>
>> This looks good btw.
>>
>>> + return -EOPNOTSUPP;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int can_validate(struct nlattr *tb[], struct nlattr *data[],
>>> struct netlink_ext_ack *extack)
>>> {
>>> u32 flags = 0;
>>> int err;
>>> @@ -199,10 +225,14 @@ static int can_validate(struct nlattr *tb[], struct
>>> nlattr *data[],
>>> (flags & CAN_CTRLMODE_RESTRICTED)) {
>>> NL_SET_ERR_MSG(extack,
>>> "Listen-only and restricted modes are mutually
>>> exclusive");
>>
>> IMO this should also be capitalized ...
>>
>> "LISTEN-MODE and RESTRICTED modes are mutually exclusive");
>
> This is typically the kind of thing where I prefer the lower case. The above
> seems as if the error message is shouting at me.
>
> Well, if you still prefer upper case after my explanation, I will change.
>
>>> return -EOPNOTSUPP;
>>> }
>>> +
>>> + err = can_validate_xl_flags(extack, flags, cm->mask);
>>> + if (err)
>>> + return err;
>>> }
>>> err = can_validate_bittiming(data, extack, IFLA_CAN_BITTIMING);
>>> if (err)
>>> return err;
>>> @@ -224,11 +254,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>> struct nlattr *data[],
>>> struct netlink_ext_ack *extack)
>>> {
>>> struct can_priv *priv = netdev_priv(dev);
>>> struct can_ctrlmode *cm;
>>> - u32 ctrlstatic, maskedflags, notsupp, ctrlstatic_missing;
>>> + u32 ctrlstatic, maskedflags, deactivated, notsupp, ctrlstatic_missing;
>>> if (!data[IFLA_CAN_CTRLMODE])
>>> return 0;
>>> /* Do not allow changing controller mode while running */
>>> @@ -236,10 +266,11 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>> return -EBUSY;
>>> 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;
>>> if (notsupp) {
>>> NL_SET_ERR_MSG_FMT(extack,
>>> @@ -257,15 +288,25 @@ static int can_ctrlmode_changelink(struct net_device *dev,
>>> "missing required %s static control mode",
>>> can_get_ctrlmode_str(ctrlstatic_missing));
>>> 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");
>> "TMS can not be activated while FD is on");
>>
>> And this also.
>
> Ack.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-16 21:54 ` Vincent Mailhol
2025-11-16 22:53 ` Vincent Mailhol
@ 2025-11-17 8:40 ` Oliver Hartkopp
1 sibling, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-17 8:40 UTC (permalink / raw)
To: Vincent Mailhol; +Cc: linux-can
Hi Vincent,
On 16.11.25 22:54, Vincent Mailhol wrote:
>>> + if (tms_conflicts) {
>>> + NL_SET_ERR_MSG_FMT(extack,
>>> + "TMS and %s are mutually exclusive",
>>> + can_get_ctrlmode_str(tms_conflicts));
>>
>> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 dbitrate 2000000 fd
>> on xbitrate 4000000 xl on tms on
>> Error: TMS and fd are mutually exclusive.
>>
>> The error messages should look consistent in terms of capitalization.
>>
>> Maybe can_get_ctrlmode_str() should deliver capitalized strings as we see it in
>> the 'ip' tool output:
>
> In a full English sentence, I tend to see ALL CAPITALIZED WORDS as kind of
> aggressive.
Really? Kind of aggressive?
This is not a poetry contest here.
We are talking about error messages and warnings from the ip tool, that
already shows the specified defines in capital letters:
can <FD,TDC-AUTO,XL,XL-TDC-AUTO>
Take a look at:
"TMS and fd are mutually exclusive."
fd = file descriptor? It gets lost in the text.
or
"Listen-only and restricted modes are mutually exclusive"
Where do you find the defines you know from 'ip -det link show can0' ?
"LISTEN-MODE and RESTRICTED modes are mutually exclusive"
(..)
> This is typically the kind of thing where I prefer the lower case. The above
> seems as if the error message is shouting at me.
Maybe the problem is on your side then. For me it looks fine. I'm robust
against error messages shouting at me to clearly show the defined key words.
> Well, if you still prefer upper case after my explanation, I will change.
It is not about my personal opinion. Not finding the known defines in
the text gets an usability score of -10000.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm()
2025-11-16 22:18 ` Vincent Mailhol
@ 2025-11-17 8:59 ` Oliver Hartkopp
0 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-17 8:59 UTC (permalink / raw)
To: Vincent Mailhol, linux-can
Hi Vincent,
On 16.11.25 23:18, Vincent Mailhol wrote:
>>
>> In the case of xbitrate 12300000 the feedback is
>> Warning: bitrate error: 0.0%.
>>
>> The calculated bitrate is:
>> xbitrate 12307692 xsample-point 0.538
>>
>> 12307692/12300000 = 1.00062536585
>>
>> So it is 0.06%
>>
>> root@de1soc1:~# ./ip link set can0 type can bitrate 1000000 xbitrate 13300000 xl
>> on tms on
>> Warning: bitrate error: 0.2%.
>
> Random unrelated comment: I figured out that if you have both a
> NL_SET_ERR_MSG_FMT() Warning and a NL_SET_ERR_MSG() Error message (note the _FMT
> suffix only on the warning side), the warning may take precedence over the
> Error. Because netlink can only carry one message, it would be better to only
> report the error when both error and warning occurs. Note that this is not an
> issue in our tree. I wanted to look at it in more details once the CAN XL is
> merged (although I do not expect the fix to be complex).
>
>> The calculated bitrate is:
>> xbitrate 13333333 xsample-point 0.583
>>
>> 13333333/13300000 = 1.0025062406
>>
>> So it is 0.25%
>>
>> Would it make sense to add another digit and probably additionally some round-up
>> to omit a 0.0% warning?
>
> I drafted this:
>
> ------------8<------------
> can: calc_bittiming: add one decimal place in bitrate error messages
> ---
> drivers/net/can/dev/calc_bittiming.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/dev/calc_bittiming.c
> b/drivers/net/can/dev/calc_bittiming.c
> index 9c8154859513..45d1dc286197 100644
> --- a/drivers/net/can/dev/calc_bittiming.c
> +++ b/drivers/net/can/dev/calc_bittiming.c
> @@ -152,19 +152,19 @@ int can_calc_bittiming(const struct net_device *dev,
> struct can_bittiming *bt,
> }
>
> if (best_bitrate_error) {
> - /* Error in one-tenth of a percent */
> - v64 = (u64)best_bitrate_error * 1000;
> + /* Error in one-hundredth of a percent */
> + v64 = (u64)best_bitrate_error * 10000;
> do_div(v64, bt->bitrate);
> bitrate_error = (u32)v64;
> if (bitrate_error > CAN_CALC_MAX_ERROR) {
> NL_SET_ERR_MSG_FMT(extack,
> "bitrate error: %u.%u%% too high",
> - bitrate_error / 10, bitrate_error % 10);
> + bitrate_error / 100, bitrate_error % 100);
> return -EINVAL;
> }
> NL_SET_ERR_MSG_FMT(extack,
> "bitrate error: %u.%u%%",
> - bitrate_error / 10, bitrate_error % 10);
> + bitrate_error / 100, bitrate_error % 100);
> }
>
> /* real sample point */
> ------------8<------------
>
> No time to test at the moment (maybe I will be able to test in a couple days),
> but does it look good to you?
Yes. I will test it.
I was thinking of this for a round-up so that we never get a 0.00%:
NL_SET_ERR_MSG_FMT(extack,
"bitrate error: %u.%02u%%", // always 2 digits
bitrate_error / 100,
((bitrate_error / 100) || (bitrate_error % 100))?(bitrate_error % 100):1);
Best regards,
Oliver
>
> Also, this isn't the only place in which we are getting out of significant
> digit. This was already a couple months ago, but when I started calculating the
> PWM symbols, I figured out that on very high bitrates, can_bittiming->tq will
> lack some significant digits because of the decimal truncation. Using this in
> CAN XL can become problematic. That's another TODO…
>
>
> Yours sincerely,
> Vincent Mailhol
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-16 22:53 ` Vincent Mailhol
@ 2025-11-17 9:06 ` Oliver Hartkopp
0 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-17 9:06 UTC (permalink / raw)
To: Vincent Mailhol; +Cc: linux-can
Hi Vincent,
On 16.11.25 23:53, Vincent Mailhol wrote:
>
> I just remembered that there are a couple more reasons why I preferred to use
> lower case here:
>
> - can_get_state_str() which is just above can_get_ctrlmode_str() also uses
> lower case. Because can_get_ctrlmode_str() may be used by for things other
> than the netlink error reporting, consistency with can_get_state_str() also
> matters.
>
> - the ip tool command line also use lower case (xl on tms on…). So better to
> match what the user just entered on the command line just above the error
> message rather than the output of "ip link show".
Of course I expected this argument of the command line.
But the string becomes hard to read as I stated in my previous answer.
What about a wrapper can_get_ctrlmode_str_cap() that delivers
capitalized control mode key words for the defines?
Best regards,
Oliver
^ permalink raw reply [flat|nested] 29+ messages in thread
* Mainlining of [canxl v2 00/15] CAN XL support for review (full series)
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
` (14 preceding siblings ...)
2025-11-15 16:37 ` [canxl v2 15/15] can: raw: instantly reject unsupported CAN frames Oliver Hartkopp
@ 2025-11-19 18:17 ` Oliver Hartkopp
2025-11-19 21:08 ` Marc Kleine-Budde
2025-11-20 10:34 ` Stéphane Grosjean
15 siblings, 2 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-19 18:17 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Stéphane Grosjean; +Cc: linux-can
Hello Marc/Vincent/Stéphane!
We are right before Linux 6.18-rc7 and I would like to have the CAN XL
support ready for the 6.19 merge window.
Unfortunately the reaction time and feedback from Vincent is currently
very intermittent. This is no criticism but risky for catching the 6.19
merge window.
This v2 patch set is feature complete and tested.
Finalized discussions (code complete in v2 patch set and tested):
- make RESTRICTED a normal ctrlmode_supported option
- make TMS a normal ctrlmode_supported option
- omit CAN_CTRLMODE_XL_ERR_SIGNAL in netlink API
Open discussions / review results:
- not removing "const" in can_update_sample_point()
- have the ctrlmode names in ip feedback messages capitalized
- increase the resolution to two decimal places in can_calc_bittiming()
- can_calc_pwm() has no return value (kernel test robot report)
The latter are tiny fixes and beautifications that potentially can also
be done after the merge window.
Therefore I would propose to mainline the current v2 patch set right now
and see what we can improve until the merge window closes.
@Vincent: If you are currently busy I can offer to work on the open
points for you. So it would just be a review-job for you and I would
send a v3 patch set until Friday (latest).
Best regards,
Oliver
On 15.11.25 17:37, Oliver Hartkopp wrote:
> This series is based on Vincents CAN XL patches 2025-10-13 11:01
>
> https://lore.kernel.org/linux-can/20251017-enchanted-quiet-civet-84dd47-mkl@pengutronix.de/T/#mdecc959e0ef7c16c64f35e9dd3d687954e15c8ac
>
> For a better review here is the complete series of available patches with
> some changes, especially the error-signalling handling.
>
> The changes to Vincents original patches are documented below:
>
> Oliver Hartkopp (2):
> can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode
> (replaces can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL flag patch)
> can: raw: instantly reject unsupported CAN frames
> (the adapted version using can_dev_in_xl_only_mode() helper)
>
> Vincent Mailhol (13):
> can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
> (no change)
> can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
> (no change)
> can: netlink: add CAN_CTRLMODE_RESTRICTED
> (no change)
> can: netlink: add initial CAN XL support
> (remove the "bad device" warning for CAN_CTRLMODE_RESTRICTED)
> can: netlink: add CAN_CTRLMODE_XL_TMS flag
> (remove the requirement that TMS MUST be set. Use defaults: off)
> can: bittiming: add PWM parameters
> (no change)
> can: bittiming: add PWM validation
> (no change)
> can: calc_bittiming: add PWM calculation
> (no change)
> can: netlink: add PWM netlink interface
> (no change)
> can: calc_bittiming: get rid of the incorrect "nominal" word
> (no change)
> can: calc_bittiming: add can_calc_sample_point_nrz()
> (no change)
> can: calc_bittiming: add can_calc_sample_point_pwm()
> (no change)
> can: add dummy_can driver
> (remove CAN_CTRLMODE_XL_ERR_SIGNAL but print error-signalling state)
>
> drivers/net/can/Kconfig | 17 ++
> drivers/net/can/Makefile | 1 +
> drivers/net/can/dev/bittiming.c | 63 ++++++
> drivers/net/can/dev/calc_bittiming.c | 104 +++++++--
> drivers/net/can/dev/dev.c | 18 +-
> drivers/net/can/dev/netlink.c | 319 +++++++++++++++++++++++++--
> drivers/net/can/dummy_can.c | 284 ++++++++++++++++++++++++
> include/linux/can/bittiming.h | 81 ++++++-
> include/linux/can/dev.h | 68 ++++--
> include/uapi/linux/can/netlink.h | 34 +++
> net/can/raw.c | 54 ++++-
> 11 files changed, 970 insertions(+), 73 deletions(-)
> create mode 100644 drivers/net/can/dummy_can.c
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Mainlining of [canxl v2 00/15] CAN XL support for review (full series)
2025-11-19 18:17 ` Mainlining of [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
@ 2025-11-19 21:08 ` Marc Kleine-Budde
2025-11-20 11:27 ` Oliver Hartkopp
2025-11-20 10:34 ` Stéphane Grosjean
1 sibling, 1 reply; 29+ messages in thread
From: Marc Kleine-Budde @ 2025-11-19 21:08 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Vincent Mailhol, Stéphane Grosjean, linux-can
[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]
On 19.11.2025 19:17:05, Oliver Hartkopp wrote:
> Hello Marc/Vincent/Stéphane!
>
> We are right before Linux 6.18-rc7 and I would like to have the CAN XL
> support ready for the 6.19 merge window.
>
> Unfortunately the reaction time and feedback from Vincent is currently very
> intermittent. This is no criticism but risky for catching the 6.19 merge
> window.
>
> This v2 patch set is feature complete and tested.
>
> Finalized discussions (code complete in v2 patch set and tested):
> - make RESTRICTED a normal ctrlmode_supported option
> - make TMS a normal ctrlmode_supported option
> - omit CAN_CTRLMODE_XL_ERR_SIGNAL in netlink API
>
> Open discussions / review results:
> - not removing "const" in can_update_sample_point()
> - have the ctrlmode names in ip feedback messages capitalized
> - increase the resolution to two decimal places in can_calc_bittiming()
> - can_calc_pwm() has no return value (kernel test robot report)
>
> The latter are tiny fixes and beautifications that potentially can also be
> done after the merge window.
>
> Therefore I would propose to mainline the current v2 patch set right now and
> see what we can improve until the merge window closes.
>
> @Vincent: If you are currently busy I can offer to work on the open points
> for you. So it would just be a review-job for you and I would send a v3
> patch set until Friday (latest).
It's probably a week or so until the last PR to net-next.
For the next iteration of the series, please include you S-o-b,
otherwise I cannot take it.
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 --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Mainlining of [canxl v2 00/15] CAN XL support for review (full series)
2025-11-19 18:17 ` Mainlining of [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
2025-11-19 21:08 ` Marc Kleine-Budde
@ 2025-11-20 10:34 ` Stéphane Grosjean
2025-11-20 11:21 ` Oliver Hartkopp
1 sibling, 1 reply; 29+ messages in thread
From: Stéphane Grosjean @ 2025-11-20 10:34 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: linux-can, Marc Kleine-Budde, Vincent Mailhol,
Stéphane Grosjean
Hello,
Only small nitpicks from my side:
> - make RESTRICTED a normal ctrlmode_supported option
> - make TMS a normal ctrlmode_supported option
If I understand “normal” (for RESTRICTED) as an option that is not solely linked to CANXL, I no longer understand it for TMS. Could you explain please?
The BOSCH communication refers to “pure-CANXL” mode rather than “CANXL-only” mode as used here. Is this an intentional choice on your part?
> Open discussions / review results:
> - not removing "const" in can_update_sample_point()
This topic has nothing to do with CANXL, and I have already expressed my opinion on it: “const” has its place in the parameter declaration of a function in C.
> - have the ctrlmode names in ip feedback messages capitalized
The use of capital letters here is simply a matter of adopting the formalism generally used in C for constant symbols. Which itself is inspired by the formalism generally used for acronyms. I don't see anything else to add here. IMHO.
And finally, as little as my opinion on the various patches in the v2 series may be of use, I didn't see anything terrible. I did have some doubts about:
[canxl v2 08/15] can: bittiming: add PWM validation
+ if (pwm->pwmo >= pwm->pwms + pwm->pwml) {
but it turns out that >= is actually more sensible than >
Regards,
-- Stéphane
----- Mail original -----
> Hello Marc/Vincent/Stéphane!
>
> We are right before Linux 6.18-rc7 and I would like to have the CAN
> XL
> support ready for the 6.19 merge window.
>
> Unfortunately the reaction time and feedback from Vincent is
> currently
> very intermittent. This is no criticism but risky for catching the
> 6.19
> merge window.
>
> This v2 patch set is feature complete and tested.
>
> Finalized discussions (code complete in v2 patch set and tested):
> - make RESTRICTED a normal ctrlmode_supported option
> - make TMS a normal ctrlmode_supported option
> - omit CAN_CTRLMODE_XL_ERR_SIGNAL in netlink API
>
> Open discussions / review results:
> - not removing "const" in can_update_sample_point()
> - have the ctrlmode names in ip feedback messages capitalized
> - increase the resolution to two decimal places in
> can_calc_bittiming()
> - can_calc_pwm() has no return value (kernel test robot report)
>
> The latter are tiny fixes and beautifications that potentially can
> also
> be done after the merge window.
>
> Therefore I would propose to mainline the current v2 patch set right
> now
> and see what we can improve until the merge window closes.
>
> @Vincent: If you are currently busy I can offer to work on the open
> points for you. So it would just be a review-job for you and I would
> send a v3 patch set until Friday (latest).
>
> Best regards,
> Oliver
>
> On 15.11.25 17:37, Oliver Hartkopp wrote:
> > This series is based on Vincents CAN XL patches 2025-10-13 11:01
> >
> > https://lore.kernel.org/linux-can/20251017-enchanted-quiet-civet-84dd47-mkl@pengutronix.de/T/#mdecc959e0ef7c16c64f35e9dd3d687954e15c8ac
> >
> > For a better review here is the complete series of available
> > patches with
> > some changes, especially the error-signalling handling.
> >
> > The changes to Vincents original patches are documented below:
> >
> > Oliver Hartkopp (2):
> > can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only
> > mode
> > (replaces can: netlink: add CAN_CTRLMODE_XL_ERR_SIGNAL flag
> > patch)
> > can: raw: instantly reject unsupported CAN frames
> > (the adapted version using can_dev_in_xl_only_mode() helper)
> >
> > Vincent Mailhol (13):
> > can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
> > (no change)
> > can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
> > (no change)
> > can: netlink: add CAN_CTRLMODE_RESTRICTED
> > (no change)
> > can: netlink: add initial CAN XL support
> > (remove the "bad device" warning for CAN_CTRLMODE_RESTRICTED)
> > can: netlink: add CAN_CTRLMODE_XL_TMS flag
> > (remove the requirement that TMS MUST be set. Use defaults: off)
> > can: bittiming: add PWM parameters
> > (no change)
> > can: bittiming: add PWM validation
> > (no change)
> > can: calc_bittiming: add PWM calculation
> > (no change)
> > can: netlink: add PWM netlink interface
> > (no change)
> > can: calc_bittiming: get rid of the incorrect "nominal" word
> > (no change)
> > can: calc_bittiming: add can_calc_sample_point_nrz()
> > (no change)
> > can: calc_bittiming: add can_calc_sample_point_pwm()
> > (no change)
> > can: add dummy_can driver
> > (remove CAN_CTRLMODE_XL_ERR_SIGNAL but print error-signalling
> > state)
> >
> > drivers/net/can/Kconfig | 17 ++
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/dev/bittiming.c | 63 ++++++
> > drivers/net/can/dev/calc_bittiming.c | 104 +++++++--
> > drivers/net/can/dev/dev.c | 18 +-
> > drivers/net/can/dev/netlink.c | 319
> > +++++++++++++++++++++++++--
> > drivers/net/can/dummy_can.c | 284
> > ++++++++++++++++++++++++
> > include/linux/can/bittiming.h | 81 ++++++-
> > include/linux/can/dev.h | 68 ++++--
> > include/uapi/linux/can/netlink.h | 34 +++
> > net/can/raw.c | 54 ++++-
> > 11 files changed, 970 insertions(+), 73 deletions(-)
> > create mode 100644 drivers/net/can/dummy_can.c
> >
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Mainlining of [canxl v2 00/15] CAN XL support for review (full series)
2025-11-20 10:34 ` Stéphane Grosjean
@ 2025-11-20 11:21 ` Oliver Hartkopp
0 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-20 11:21 UTC (permalink / raw)
To: Stéphane Grosjean
Cc: linux-can, Marc Kleine-Budde, Vincent Mailhol,
Stéphane Grosjean
Hi Stéphane,
On 20.11.25 11:34, Stéphane Grosjean wrote:
> Only small nitpicks from my side:
>
>> - make RESTRICTED a normal ctrlmode_supported option
>> - make TMS a normal ctrlmode_supported option
>
> If I understand “normal” (for RESTRICTED) as an option that is not solely linked to CANXL, I no longer understand it for TMS. Could you explain please?
Right. The RESTRICTED mode is not linked to CAN XL but is intended to be
mandatory for XL nodes. See Vincent's commit message:
https://lore.kernel.org/linux-can/20251115163740.7875-4-socketcan@hartkopp.net/T/#u
OTOH the TMS option only works in the CANXL-only mode.
My change to the original TMS patch from Vincent
https://git.kernel.org/pub/scm/linux/kernel/git/mailhol/linux.git/commit/?h=b4/canxl-netlink&id=c931f5efd2ec0e6cdabc968d67f8a758519e736e
is, that I removed that TMS always has to be set on the ip command line.
You needed to specify either "tms on" or "tms off" when "xl on" is set.
Now TMS has the default behavior again. Like any other ctrlmode flag it
is default "off" and can be set with "tms on".
> The BOSCH communication refers to “pure-CANXL” mode rather than “CANXL-only” mode as used here. Is this an intentional choice on your part?
I have no strong opinions about the naming here. But "pure" sounds like
pure water to me ;-D
>> Open discussions / review results:
>> - not removing "const" in can_update_sample_point()
>
> This topic has nothing to do with CANXL, and I have already expressed my opinion on it: “const” has its place in the parameter declaration of a function in C.
Right. But this was a discussion when Vincent removed the "const"
statement where Marc and you wanted to change that back.
https://lore.kernel.org/linux-can/20251117-transparent-exotic-myna-bd77c9-mkl@pengutronix.de/T/#u
>> - have the ctrlmode names in ip feedback messages capitalized
>
> The use of capital letters here is simply a matter of adopting the formalism generally used in C for constant symbols. Which itself is inspired by the formalism generally used for acronyms. I don't see anything else to add here. IMHO.
Agreed! Thx!
> And finally, as little as my opinion on the various patches in the v2 series may be of use, I didn't see anything terrible. I did have some doubts about:
>
> [canxl v2 08/15] can: bittiming: add PWM validation
>
> + if (pwm->pwmo >= pwm->pwms + pwm->pwml) {
>
> but it turns out that >= is actually more sensible than >
??
Would you suggest
if (pwm->pwmo > pwm->pwms + pwm->pwml)
instead of
if (pwm->pwmo >= pwm->pwms + pwm->pwml)
which would also fit the error message?
"PWMO: %u tqmin can not be greater than PWMS + PWML: %u tqmin",
Best regards,
Oliver
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Mainlining of [canxl v2 00/15] CAN XL support for review (full series)
2025-11-19 21:08 ` Marc Kleine-Budde
@ 2025-11-20 11:27 ` Oliver Hartkopp
0 siblings, 0 replies; 29+ messages in thread
From: Oliver Hartkopp @ 2025-11-20 11:27 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Vincent Mailhol, Stéphane Grosjean, linux-can
On 19.11.25 22:08, Marc Kleine-Budde wrote:
> On 19.11.2025 19:17:05, Oliver Hartkopp wrote:
>> Hello Marc/Vincent/Stéphane!
>>
>> We are right before Linux 6.18-rc7 and I would like to have the CAN XL
>> support ready for the 6.19 merge window.
>>
>> Unfortunately the reaction time and feedback from Vincent is currently very
>> intermittent. This is no criticism but risky for catching the 6.19 merge
>> window.
>>
>> This v2 patch set is feature complete and tested.
>>
>> Finalized discussions (code complete in v2 patch set and tested):
>> - make RESTRICTED a normal ctrlmode_supported option
>> - make TMS a normal ctrlmode_supported option
>> - omit CAN_CTRLMODE_XL_ERR_SIGNAL in netlink API
>>
>> Open discussions / review results:
>> - not removing "const" in can_update_sample_point()
>> - have the ctrlmode names in ip feedback messages capitalized
>> - increase the resolution to two decimal places in can_calc_bittiming()
>> - can_calc_pwm() has no return value (kernel test robot report)
>>
>> The latter are tiny fixes and beautifications that potentially can also be
>> done after the merge window.
>>
>> Therefore I would propose to mainline the current v2 patch set right now and
>> see what we can improve until the merge window closes.
>>
>> @Vincent: If you are currently busy I can offer to work on the open points
>> for you. So it would just be a review-job for you and I would send a v3
>> patch set until Friday (latest).
>
> It's probably a week or so until the last PR to net-next.
Ok.
> For the next iteration of the series, please include you S-o-b,
> otherwise I cannot take it.
I'll send a V3 patch series with the fixes and correct S-o-b tags.
One of my patches needs some better description too.
So Vincent can see the results and we can discuss on that new basis.
Many thanks!
Oliver
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2025-11-20 11:27 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-15 16:37 [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 01/15] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 02/15] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 03/15] can: netlink: add CAN_CTRLMODE_RESTRICTED Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 04/15] can: netlink: add initial CAN XL support Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 05/15] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
2025-11-16 19:31 ` Oliver Hartkopp
2025-11-16 21:54 ` Vincent Mailhol
2025-11-16 22:53 ` Vincent Mailhol
2025-11-17 9:06 ` Oliver Hartkopp
2025-11-17 8:40 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 06/15] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 07/15] can: bittiming: add PWM parameters Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 08/15] can: bittiming: add PWM validation Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 09/15] can: calc_bittiming: add PWM calculation Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 10/15] can: netlink: add PWM netlink interface Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 11/15] can: calc_bittiming: get rid of the incorrect "nominal" word Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 12/15] can: calc_bittiming: add can_calc_sample_point_nrz() Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 13/15] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
2025-11-16 20:05 ` Oliver Hartkopp
2025-11-16 22:18 ` Vincent Mailhol
2025-11-17 8:59 ` Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 14/15] can: add dummy_can driver Oliver Hartkopp
2025-11-15 16:37 ` [canxl v2 15/15] can: raw: instantly reject unsupported CAN frames Oliver Hartkopp
2025-11-19 18:17 ` Mainlining of [canxl v2 00/15] CAN XL support for review (full series) Oliver Hartkopp
2025-11-19 21:08 ` Marc Kleine-Budde
2025-11-20 11:27 ` Oliver Hartkopp
2025-11-20 10:34 ` Stéphane Grosjean
2025-11-20 11:21 ` Oliver Hartkopp
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).