* [canxl v4 01/17] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming()
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
@ 2025-11-21 8:33 ` Oliver Hartkopp
2025-11-21 8:33 ` [canxl v4 02/17] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Oliver Hartkopp
` (15 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:33 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* [canxl v4 02/17] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
2025-11-21 8:33 ` [canxl v4 01/17] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
@ 2025-11-21 8:33 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 03/17] can: netlink: add CAN_CTRLMODE_RESTRICTED Oliver Hartkopp
` (14 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:33 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
include/linux/can/dev.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 0fe8f80f223e..d59b283c981a 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -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] 30+ messages in thread* [canxl v4 03/17] can: netlink: add CAN_CTRLMODE_RESTRICTED
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
2025-11-21 8:33 ` [canxl v4 01/17] can: bittiming: apply NL_SET_ERR_MSG() to can_calc_bittiming() Oliver Hartkopp
2025-11-21 8:33 ` [canxl v4 02/17] can: dev: can_dev_dropped_skb: drop CAN FD skbs if FD is off Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 04/17] can: netlink: add initial CAN XL support Oliver Hartkopp
` (13 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
drivers/net/can/dev/dev.c | 2 ++
drivers/net/can/dev/netlink.c | 7 +++++
include/linux/can/dev.h | 50 +++++++++++++++++---------------
include/uapi/linux/can/netlink.h | 1 +
4 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 0cc3d008adb3..3377afb6f1c4 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -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 d59b283c981a..9de8fde3ec9d 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) \
@@ -148,10 +124,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] 30+ messages in thread* [canxl v4 04/17] can: netlink: add initial CAN XL support
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (2 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 03/17] can: netlink: add CAN_CTRLMODE_RESTRICTED Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 05/17] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
` (12 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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 3377afb6f1c4..32f11db88295 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 9de8fde3ec9d..945c16743702 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] 30+ messages in thread* [canxl v4 05/17] can: netlink: add CAN_CTRLMODE_XL_TMS flag
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (3 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 04/17] can: netlink: add initial CAN XL support Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 06/17] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode Oliver Hartkopp
` (11 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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 32f11db88295..1de5babcc4f3 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] 30+ messages in thread* [canxl v4 06/17] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (4 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 05/17] can: netlink: add CAN_CTRLMODE_XL_TMS flag Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 07/17] can: bittiming: add PWM parameters Oliver Hartkopp
` (10 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp
The error-signalling (ES) is a mandatory functionality for CAN CC and
CAN FD to report CAN frame format violations by sending an error-frame
signal on the bus.
A so-called 'mixed-mode' is intended to have (XL-tolerant) CAN FD nodes
and CAN XL nodes on one CAN segment, where the FD-controllers can talk
CC/FD and the XL-controllers can talk CC/FD/XL. This mixed-mode
utilizes the error-signalling for sending CC/FD/XL frames.
The CANXL-only mode disables the error-signalling in the CAN XL
controller. This mode does not allow CC/FD frames to be sent but
additionally offers a CAN XL transceiver mode switching (TMS).
Configured with CAN_CTRLMODE_FD and CAN_CTRLMODE_XL this leads to:
FD=0 XL=0 CC-only mode (ES=1)
FD=1 XL=0 FD/CC mixed-mode (ES=1)
FD=1 XL=1 XL/FD/CC mixed-mode (ES=1)
FD=0 XL=1 XL-only mode (ES=0, TMS optional)
The helper function can_dev_in_xl_only_mode() determines the required
value to disable error signalling in the CAN XL controller.
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 945c16743702..13b25b0dceeb 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -129,10 +129,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 |
@@ -147,10 +160,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] 30+ messages in thread* [canxl v4 07/17] can: bittiming: add PWM parameters
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (5 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 06/17] can: dev: can_dev_dropped_skb: drop CC/FD frames in CANXL-only mode Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 08/17] can: bittiming: add PWM validation Oliver Hartkopp
` (9 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* [canxl v4 08/17] can: bittiming: add PWM validation
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (6 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 07/17] can: bittiming: add PWM parameters Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 9:14 ` Marc Kleine-Budde
2025-11-21 8:34 ` [canxl v4 09/17] can: calc_bittiming: add PWM calculation Oliver Hartkopp
` (8 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* Re: [canxl v4 08/17] can: bittiming: add PWM validation
2025-11-21 8:34 ` [canxl v4 08/17] can: bittiming: add PWM validation Oliver Hartkopp
@ 2025-11-21 9:14 ` Marc Kleine-Budde
2025-11-21 9:23 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 9:14 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Vincent Mailhol
[-- Attachment #1: Type: text/plain, Size: 2915 bytes --]
On 21.11.2025 09:34:05, Oliver Hartkopp wrote:
> 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>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> 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",
not
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] 30+ messages in thread* Re: [canxl v4 08/17] can: bittiming: add PWM validation
2025-11-21 9:14 ` Marc Kleine-Budde
@ 2025-11-21 9:23 ` Oliver Hartkopp
0 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:23 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol
On 21.11.25 10:14, Marc Kleine-Budde wrote:
> On 21.11.2025 09:34:05, Oliver Hartkopp wrote:
>> 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>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> 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",
> not
Will fix it for v5.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 30+ messages in thread
* [canxl v4 09/17] can: calc_bittiming: add PWM calculation
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (7 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 08/17] can: bittiming: add PWM validation Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 10/17] can: netlink: add PWM netlink interface Oliver Hartkopp
` (7 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* [canxl v4 10/17] can: netlink: add PWM netlink interface
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (8 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 09/17] can: calc_bittiming: add PWM calculation Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 9:19 ` Marc Kleine-Budde
2025-11-21 8:34 ` [canxl v4 11/17] can: calc_bittiming: get rid of the incorrect "nominal" word Oliver Hartkopp
` (6 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* Re: [canxl v4 10/17] can: netlink: add PWM netlink interface
2025-11-21 8:34 ` [canxl v4 10/17] can: netlink: add PWM netlink interface Oliver Hartkopp
@ 2025-11-21 9:19 ` Marc Kleine-Budde
2025-11-21 9:24 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 9:19 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Vincent Mailhol
[-- Attachment #1: Type: text/plain, Size: 4973 bytes --]
On 21.11.2025 09:34:07, Oliver Hartkopp wrote:
> 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>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> 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");
● checkpatch.pl: WARNING: 'automic' may be misspelled - perhaps 'atomic'?
● checkpatch.pl: #188: FILE: drivers/net/can/dev/netlink.c:156:
● checkpatch.pl: + "Provide either both PWMS and PWML, or none for automic calculation");
● checkpatch.pl:
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] 30+ messages in thread* Re: [canxl v4 10/17] can: netlink: add PWM netlink interface
2025-11-21 9:19 ` Marc Kleine-Budde
@ 2025-11-21 9:24 ` Oliver Hartkopp
0 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:24 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol
On 21.11.25 10:19, Marc Kleine-Budde wrote:
> On 21.11.2025 09:34:07, Oliver Hartkopp wrote:
>> 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>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> 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");
>
> ● checkpatch.pl: WARNING: 'automic' may be misspelled - perhaps 'atomic'?
> ● checkpatch.pl: #188: FILE: drivers/net/can/dev/netlink.c:156:
> ● checkpatch.pl: + "Provide either both PWMS and PWML, or none for automic calculation");
> ● checkpatch.pl:
>
automatic will help ;-)
Best regards,
Oliver
^ permalink raw reply [flat|nested] 30+ messages in thread
* [canxl v4 11/17] can: calc_bittiming: get rid of the incorrect "nominal" word
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (9 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 10/17] can: netlink: add PWM netlink interface Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 12/17] can: calc_bittiming: add can_calc_sample_point_nrz() Oliver Hartkopp
` (5 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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..46f6f5942abb 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,
+ const unsigned int sp_origin, const 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] 30+ messages in thread* [canxl v4 12/17] can: calc_bittiming: add can_calc_sample_point_nrz()
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (10 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 11/17] can: calc_bittiming: get rid of the incorrect "nominal" word Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 13/17] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
` (4 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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 46f6f5942abb..35db90be9c9a 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] 30+ messages in thread* [canxl v4 13/17] can: calc_bittiming: add can_calc_sample_point_pwm()
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (11 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 12/17] can: calc_bittiming: add can_calc_sample_point_nrz() Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 14/17] can: add dummy_can driver Oliver Hartkopp
` (3 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
drivers/net/can/dev/calc_bittiming.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 35db90be9c9a..0b11c4e98172 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,13 @@ 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] 30+ messages in thread* [canxl v4 14/17] can: add dummy_can driver
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (12 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 13/17] can: calc_bittiming: add can_calc_sample_point_pwm() Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 9:21 ` Marc Kleine-Budde
2025-11-21 8:34 ` [canxl v4 15/17] can: raw: instantly reject unsupported CAN frames Oliver Hartkopp
` (2 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Vincent Mailhol, Oliver Hartkopp
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>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
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] 30+ messages in thread* Re: [canxl v4 14/17] can: add dummy_can driver
2025-11-21 8:34 ` [canxl v4 14/17] can: add dummy_can driver Oliver Hartkopp
@ 2025-11-21 9:21 ` Marc Kleine-Budde
2025-11-21 9:25 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 9:21 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Vincent Mailhol
[-- Attachment #1: Type: text/plain, Size: 9702 bytes --]
On 21.11.2025 09:34:11, Oliver Hartkopp wrote:
> 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>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> 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");
please make use of string_choices.h
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] 30+ messages in thread* Re: [canxl v4 14/17] can: add dummy_can driver
2025-11-21 9:21 ` Marc Kleine-Budde
@ 2025-11-21 9:25 ` Oliver Hartkopp
0 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:25 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol
On 21.11.25 10:21, Marc Kleine-Budde wrote:
> On 21.11.2025 09:34:11, Oliver Hartkopp wrote:
>> 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>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> 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");
>
> please make use of string_choices.h
Ok. I'll take a look at it.
Best regards,
Oliver
^ permalink raw reply [flat|nested] 30+ messages in thread
* [canxl v4 15/17] can: raw: instantly reject unsupported CAN frames
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (13 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 14/17] can: add dummy_can driver Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings Oliver Hartkopp
2025-11-21 8:34 ` [canxl v4 17/17] can: dev: print bitrate error with two decimal digits Oliver Hartkopp
16 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 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 a53853f5e9af..223630f0f9e9 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] 30+ messages in thread* [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (14 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 15/17] can: raw: instantly reject unsupported CAN frames Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 8:47 ` Marc Kleine-Budde
2025-11-21 8:34 ` [canxl v4 17/17] can: dev: print bitrate error with two decimal digits Oliver Hartkopp
16 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp, Stephane Grosjean
Unify the ctrlmode related strings to the command line options of the
'ip' tool from the iproute2 package. The capitalized strings are also
shown when the detailed interface configuration is printed by 'ip'.
Suggested-by: Stephane Grosjean <stephane.grosjean@hms-networks.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
drivers/net/can/dev/dev.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index 1de5babcc4f3..32db9f69844d 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -90,43 +90,43 @@ EXPORT_SYMBOL_GPL(can_get_state_str);
const char *can_get_ctrlmode_str(u32 ctrlmode)
{
switch (ctrlmode & ~(ctrlmode - 1)) {
case 0:
- return "none";
+ return "(none)";
case CAN_CTRLMODE_LOOPBACK:
- return "loopback";
+ return "LOOPBACK";
case CAN_CTRLMODE_LISTENONLY:
- return "listen-only";
+ return "LISTEN-ONLY";
case CAN_CTRLMODE_3_SAMPLES:
- return "triple-sampling";
+ return "TRIPLE-SAMPLING";
case CAN_CTRLMODE_ONE_SHOT:
- return "one-shot";
+ return "ONE-SHOT";
case CAN_CTRLMODE_BERR_REPORTING:
- return "berr-reporting";
+ return "BERR-REPORTING";
case CAN_CTRLMODE_FD:
- return "fd";
+ return "FD";
case CAN_CTRLMODE_PRESUME_ACK:
- return "presume-ack";
+ return "PRESUME-ACK";
case CAN_CTRLMODE_FD_NON_ISO:
- return "fd-non-iso";
+ return "FD-NON-ISO";
case CAN_CTRLMODE_CC_LEN8_DLC:
- return "cc-len8-dlc";
+ return "CC-LEN8-DLC";
case CAN_CTRLMODE_TDC_AUTO:
- return "fd-tdc-auto";
+ return "TDC-AUTO";
case CAN_CTRLMODE_TDC_MANUAL:
- return "fd-tdc-manual";
+ return "TDC-MANUAL";
case CAN_CTRLMODE_RESTRICTED:
- return "restricted-operation";
+ return "RESTRICTED";
case CAN_CTRLMODE_XL:
- return "xl";
+ return "XL";
case CAN_CTRLMODE_XL_TDC_AUTO:
- return "xl-tdc-auto";
+ return "XL-TDC-AUTO";
case CAN_CTRLMODE_XL_TDC_MANUAL:
- return "xl-tdc-manual";
+ return "XL-TDC-MANUAL";
case CAN_CTRLMODE_XL_TMS:
- return "xl-tms";
+ return "TMS";
default:
return "<unknown>";
}
}
EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
--
2.47.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings
2025-11-21 8:34 ` [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings Oliver Hartkopp
@ 2025-11-21 8:47 ` Marc Kleine-Budde
2025-11-21 9:19 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 8:47 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Stephane Grosjean
[-- Attachment #1: Type: text/plain, Size: 2909 bytes --]
On 21.11.2025 09:34:13, Oliver Hartkopp wrote:
> Unify the ctrlmode related strings to the command line options of the
> 'ip' tool from the iproute2 package. The capitalized strings are also
> shown when the detailed interface configuration is printed by 'ip'.
>
> Suggested-by: Stephane Grosjean <stephane.grosjean@hms-networks.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> drivers/net/can/dev/dev.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 1de5babcc4f3..32db9f69844d 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -90,43 +90,43 @@ EXPORT_SYMBOL_GPL(can_get_state_str);
>
> const char *can_get_ctrlmode_str(u32 ctrlmode)
> {
> switch (ctrlmode & ~(ctrlmode - 1)) {
> case 0:
> - return "none";
> + return "(none)";
> case CAN_CTRLMODE_LOOPBACK:
> - return "loopback";
> + return "LOOPBACK";
> case CAN_CTRLMODE_LISTENONLY:
> - return "listen-only";
> + return "LISTEN-ONLY";
> case CAN_CTRLMODE_3_SAMPLES:
> - return "triple-sampling";
> + return "TRIPLE-SAMPLING";
> case CAN_CTRLMODE_ONE_SHOT:
> - return "one-shot";
> + return "ONE-SHOT";
> case CAN_CTRLMODE_BERR_REPORTING:
> - return "berr-reporting";
> + return "BERR-REPORTING";
> case CAN_CTRLMODE_FD:
> - return "fd";
> + return "FD";
> case CAN_CTRLMODE_PRESUME_ACK:
> - return "presume-ack";
> + return "PRESUME-ACK";
> case CAN_CTRLMODE_FD_NON_ISO:
> - return "fd-non-iso";
> + return "FD-NON-ISO";
> case CAN_CTRLMODE_CC_LEN8_DLC:
> - return "cc-len8-dlc";
> + return "CC-LEN8-DLC";
> case CAN_CTRLMODE_TDC_AUTO:
> - return "fd-tdc-auto";
> + return "TDC-AUTO";
> case CAN_CTRLMODE_TDC_MANUAL:
> - return "fd-tdc-manual";
> + return "TDC-MANUAL";
> case CAN_CTRLMODE_RESTRICTED:
> - return "restricted-operation";
> + return "RESTRICTED";
> case CAN_CTRLMODE_XL:
> - return "xl";
> + return "XL";
> case CAN_CTRLMODE_XL_TDC_AUTO:
> - return "xl-tdc-auto";
> + return "XL-TDC-AUTO";
> case CAN_CTRLMODE_XL_TDC_MANUAL:
> - return "xl-tdc-manual";
> + return "XL-TDC-MANUAL";
> case CAN_CTRLMODE_XL_TMS:
> - return "xl-tms";
> + return "TMS";
Here the prefix "XL-" is dropped. Was that intentional?
We should move this patch to the front, so that new members could be
added in uppercase from the beginning.
Marc
> default:
> return "<unknown>";
> }
> }
> EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
> --
> 2.47.3
>
>
>
--
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] 30+ messages in thread* Re: [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings
2025-11-21 8:47 ` Marc Kleine-Budde
@ 2025-11-21 9:19 ` Oliver Hartkopp
2025-11-21 9:30 ` Marc Kleine-Budde
0 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:19 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Stephane Grosjean
On 21.11.25 09:47, Marc Kleine-Budde wrote:
> On 21.11.2025 09:34:13, Oliver Hartkopp wrote:
>> Unify the ctrlmode related strings to the command line options of the
>> 'ip' tool from the iproute2 package. The capitalized strings are also
>> shown when the detailed interface configuration is printed by 'ip'.
>>
>> Suggested-by: Stephane Grosjean <stephane.grosjean@hms-networks.com>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> drivers/net/can/dev/dev.c | 34 +++++++++++++++++-----------------
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
>> index 1de5babcc4f3..32db9f69844d 100644
>> --- a/drivers/net/can/dev/dev.c
>> +++ b/drivers/net/can/dev/dev.c
>> @@ -90,43 +90,43 @@ EXPORT_SYMBOL_GPL(can_get_state_str);
>>
>> const char *can_get_ctrlmode_str(u32 ctrlmode)
>> {
>> switch (ctrlmode & ~(ctrlmode - 1)) {
>> case 0:
>> - return "none";
>> + return "(none)";
>> case CAN_CTRLMODE_LOOPBACK:
>> - return "loopback";
>> + return "LOOPBACK";
>> case CAN_CTRLMODE_LISTENONLY:
>> - return "listen-only";
>> + return "LISTEN-ONLY";
>> case CAN_CTRLMODE_3_SAMPLES:
>> - return "triple-sampling";
>> + return "TRIPLE-SAMPLING";
>> case CAN_CTRLMODE_ONE_SHOT:
>> - return "one-shot";
>> + return "ONE-SHOT";
>> case CAN_CTRLMODE_BERR_REPORTING:
>> - return "berr-reporting";
>> + return "BERR-REPORTING";
>> case CAN_CTRLMODE_FD:
>> - return "fd";
>> + return "FD";
>> case CAN_CTRLMODE_PRESUME_ACK:
>> - return "presume-ack";
>> + return "PRESUME-ACK";
>> case CAN_CTRLMODE_FD_NON_ISO:
>> - return "fd-non-iso";
>> + return "FD-NON-ISO";
>> case CAN_CTRLMODE_CC_LEN8_DLC:
>> - return "cc-len8-dlc";
>> + return "CC-LEN8-DLC";
>> case CAN_CTRLMODE_TDC_AUTO:
>> - return "fd-tdc-auto";
>> + return "TDC-AUTO";
>> case CAN_CTRLMODE_TDC_MANUAL:
>> - return "fd-tdc-manual";
>> + return "TDC-MANUAL";
>> case CAN_CTRLMODE_RESTRICTED:
>> - return "restricted-operation";
>> + return "RESTRICTED";
>> case CAN_CTRLMODE_XL:
>> - return "xl";
>> + return "XL";
>> case CAN_CTRLMODE_XL_TDC_AUTO:
>> - return "xl-tdc-auto";
>> + return "XL-TDC-AUTO";
>> case CAN_CTRLMODE_XL_TDC_MANUAL:
>> - return "xl-tdc-manual";
>> + return "XL-TDC-MANUAL";
>> case CAN_CTRLMODE_XL_TMS:
>> - return "xl-tms";
>> + return "TMS";
>
> Here the prefix "XL-" is dropped. Was that intentional?
Yes. The patches for iproute2-next and for the kernel are inconsistent.
The command line (and its help text) uses:
[ tms { on | off } ]
The ctrlmode is named:
CAN_CTRLMODE_XL_TMS
And the output of 'ip -det -link show can0' currently prints:
can <XL,XL-TMS> state STOPPED restart-ms 0
Which needs to be changed to <XL,TMS> IMO.
I think 'tms' is better in the command line than xl-tms as it is clear
that tms only works with XL-only and if you try it otherwise you get an
error.
>> case CAN_CTRLMODE_3_SAMPLES:
>> - return "triple-sampling";
>> + return "TRIPLE-SAMPLING";
There's not always a 1:1 name mapping.
IMO CAN_CTRLMODE_XL_TMS together with "TMS" looks fine for the internal
and external representation.
> We should move this patch to the front, so that new members could be
> added in uppercase from the beginning.
This is a useless effort IMO.
When we decided to unify the capitalization as a clean-up four weeks
later (what we did) than the patch sequence would look like this in the
tree.
Best regards,
Oliver
>
> Marc
>
>> default:
>> return "<unknown>";
>> }
>> }
>> EXPORT_SYMBOL_GPL(can_get_ctrlmode_str);
>> --
>> 2.47.3
>>
>>
>>
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings
2025-11-21 9:19 ` Oliver Hartkopp
@ 2025-11-21 9:30 ` Marc Kleine-Budde
2025-11-21 9:40 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 9:30 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Stephane Grosjean
[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]
On 21.11.2025 10:19:39, Oliver Hartkopp wrote:
> > Here the prefix "XL-" is dropped. Was that intentional?
>
> Yes. The patches for iproute2-next and for the kernel are inconsistent.
>
> The command line (and its help text) uses:
>
> [ tms { on | off } ]
>
> The ctrlmode is named:
>
> CAN_CTRLMODE_XL_TMS
>
> And the output of 'ip -det -link show can0' currently prints:
>
> can <XL,XL-TMS> state STOPPED restart-ms 0
>
> Which needs to be changed to <XL,TMS> IMO.
That's iproute2, it can be patched later.
> I think 'tms' is better in the command line than xl-tms as it is clear that
> tms only works with XL-only and if you try it otherwise you get an error.
>
> >> case CAN_CTRLMODE_3_SAMPLES:
> >> - return "triple-sampling";
> >> + return "TRIPLE-SAMPLING";
>
> There's not always a 1:1 name mapping.
>
> IMO CAN_CTRLMODE_XL_TMS together with "TMS" looks fine for the internal and
> external representation.
>
> > We should move this patch to the front, so that new members could be
> > added in uppercase from the beginning.
>
> This is a useless effort IMO.
It's really bad practice to change things in a series that has been
added in the same series. If you don't want to do it, I'll do that.
> When we decided to unify the capitalization as a clean-up four weeks later
> (what we did) than the patch sequence would look like this in the tree.
As long as you mean "upstream tree" - yes.
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] 30+ messages in thread* Re: [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings
2025-11-21 9:30 ` Marc Kleine-Budde
@ 2025-11-21 9:40 ` Oliver Hartkopp
0 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:40 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Stephane Grosjean
On 21.11.25 10:30, Marc Kleine-Budde wrote:
> On 21.11.2025 10:19:39, Oliver Hartkopp wrote:
>>> Here the prefix "XL-" is dropped. Was that intentional?
>>
>> Yes. The patches for iproute2-next and for the kernel are inconsistent.
>>
>> The command line (and its help text) uses:
>>
>> [ tms { on | off } ]
>>
>> The ctrlmode is named:
>>
>> CAN_CTRLMODE_XL_TMS
>>
>> And the output of 'ip -det -link show can0' currently prints:
>>
>> can <XL,XL-TMS> state STOPPED restart-ms 0
>>
>> Which needs to be changed to <XL,TMS> IMO.
>
> That's iproute2, it can be patched later.
>
>> I think 'tms' is better in the command line than xl-tms as it is clear that
>> tms only works with XL-only and if you try it otherwise you get an error.
>>
>>>> case CAN_CTRLMODE_3_SAMPLES:
>>>> - return "triple-sampling";
>>>> + return "TRIPLE-SAMPLING";
>>
>> There's not always a 1:1 name mapping.
>>
>> IMO CAN_CTRLMODE_XL_TMS together with "TMS" looks fine for the internal and
>> external representation.
>>
>>> We should move this patch to the front, so that new members could be
>>> added in uppercase from the beginning.
>>
>> This is a useless effort IMO.
>
> It's really bad practice to change things in a series that has been
> added in the same series. If you don't want to do it, I'll do that.
No, it's ok.
Will do.
Best regards,
Oliver
>
>> When we decided to unify the capitalization as a clean-up four weeks later
>> (what we did) than the patch sequence would look like this in the tree.
>
> As long as you mean "upstream tree" - yes.
>
> Marc
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [canxl v4 17/17] can: dev: print bitrate error with two decimal digits
2025-11-21 8:33 [canxl v4 00/17] CAN XL support for review (full series) Oliver Hartkopp
` (15 preceding siblings ...)
2025-11-21 8:34 ` [canxl v4 16/17] can: dev: can_get_ctrlmode_str: use capitalized ctrlmode strings Oliver Hartkopp
@ 2025-11-21 8:34 ` Oliver Hartkopp
2025-11-21 9:13 ` Marc Kleine-Budde
16 siblings, 1 reply; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 8:34 UTC (permalink / raw)
To: linux-can; +Cc: Oliver Hartkopp, Vincent Mailhol
Increase the resolution when printing the bitrate error and round-up the
value to 0.01% in the case the resolution would still provide values
which would lead to 0.00%.
Suggested-by: Vincent Mailhol <mailhol@kernel.org>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
drivers/net/can/dev/calc_bittiming.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
index 0b11c4e98172..103128773a7d 100644
--- a/drivers/net/can/dev/calc_bittiming.c
+++ b/drivers/net/can/dev/calc_bittiming.c
@@ -150,23 +150,26 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
if (bitrate_error == 0 && sample_point_error == 0)
break;
}
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: %u.%02u%% too high",
+ 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: %u.%02u%%",
+ bitrate_error / 100,
+ ((bitrate_error / 100) || (bitrate_error % 100))?
+ (bitrate_error % 100):1);
}
/* real sample point */
bt->sample_point = can_update_sample_point(btc, sample_point, best_tseg,
&tseg1, &tseg2, NULL);
--
2.47.3
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [canxl v4 17/17] can: dev: print bitrate error with two decimal digits
2025-11-21 8:34 ` [canxl v4 17/17] can: dev: print bitrate error with two decimal digits Oliver Hartkopp
@ 2025-11-21 9:13 ` Marc Kleine-Budde
2025-11-21 9:22 ` Oliver Hartkopp
0 siblings, 1 reply; 30+ messages in thread
From: Marc Kleine-Budde @ 2025-11-21 9:13 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, Vincent Mailhol
[-- Attachment #1: Type: text/plain, Size: 2252 bytes --]
On 21.11.2025 09:34:14, Oliver Hartkopp wrote:
> Increase the resolution when printing the bitrate error and round-up the
> value to 0.01% in the case the resolution would still provide values
> which would lead to 0.00%.
>
> Suggested-by: Vincent Mailhol <mailhol@kernel.org>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> drivers/net/can/dev/calc_bittiming.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
> index 0b11c4e98172..103128773a7d 100644
> --- a/drivers/net/can/dev/calc_bittiming.c
> +++ b/drivers/net/can/dev/calc_bittiming.c
> @@ -150,23 +150,26 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
> if (bitrate_error == 0 && sample_point_error == 0)
> break;
> }
>
> 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: %u.%02u%% too high",
> + bitrate_error / 100,
> + bitrate_error % 100);
> return -EINVAL;
> }
We already know we have a bitrate error, so we can set it to at least 1:
bitrate_error = max(bitrate_error, 1U);
Please add a comment describe the reasoning.
> NL_SET_ERR_MSG_FMT(extack,
> - "bitrate error: %u.%u%%",
> - bitrate_error / 10, bitrate_error % 10);
> + "bitrate error: %u.%02u%%",
> + bitrate_error / 100,
> + ((bitrate_error / 100) || (bitrate_error % 100))?
> + (bitrate_error % 100):1);
So we can avoid this.
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] 30+ messages in thread* Re: [canxl v4 17/17] can: dev: print bitrate error with two decimal digits
2025-11-21 9:13 ` Marc Kleine-Budde
@ 2025-11-21 9:22 ` Oliver Hartkopp
0 siblings, 0 replies; 30+ messages in thread
From: Oliver Hartkopp @ 2025-11-21 9:22 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Vincent Mailhol
On 21.11.25 10:13, Marc Kleine-Budde wrote:
> On 21.11.2025 09:34:14, Oliver Hartkopp wrote:
>> Increase the resolution when printing the bitrate error and round-up the
>> value to 0.01% in the case the resolution would still provide values
>> which would lead to 0.00%.
>>
>> Suggested-by: Vincent Mailhol <mailhol@kernel.org>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
>> drivers/net/can/dev/calc_bittiming.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/can/dev/calc_bittiming.c b/drivers/net/can/dev/calc_bittiming.c
>> index 0b11c4e98172..103128773a7d 100644
>> --- a/drivers/net/can/dev/calc_bittiming.c
>> +++ b/drivers/net/can/dev/calc_bittiming.c
>> @@ -150,23 +150,26 @@ int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt,
>> if (bitrate_error == 0 && sample_point_error == 0)
>> break;
>> }
>>
>> 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: %u.%02u%% too high",
>> + bitrate_error / 100,
>> + bitrate_error % 100);
>> return -EINVAL;
>> }
>
> We already know we have a bitrate error, so we can set it to at least 1:
>
> bitrate_error = max(bitrate_error, 1U);
>
> Please add a comment describe the reasoning.
Excellent idea!
Will do.
Best regards,
Oliver
>
>> NL_SET_ERR_MSG_FMT(extack,
>> - "bitrate error: %u.%u%%",
>> - bitrate_error / 10, bitrate_error % 10);
>> + "bitrate error: %u.%02u%%",
>> + bitrate_error / 100,
>> + ((bitrate_error / 100) || (bitrate_error % 100))?
>> + (bitrate_error % 100):1);
>
> So we can avoid this.
>
> Marc
>
^ permalink raw reply [flat|nested] 30+ messages in thread