* [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
@ 2025-07-24 9:24 ` Jimmy Assarsson
2025-07-24 18:26 ` Simon Horman
2025-07-24 9:24 ` [PATCH v2 02/11] can: kvaser_usb: Add support for ethtool set_phys_id() Jimmy Assarsson
` (10 subsequent siblings)
11 siblings, 1 reply; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:24 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Add support to turn on/off CAN LEDs on device.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 9 ++++
.../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 51 ++++++++++++++++++
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 54 +++++++++++++++++++
3 files changed, 114 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index f6c77eca9f43..032dc1821f04 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -54,6 +54,11 @@ enum kvaser_usb_leaf_family {
KVASER_USBCAN,
};
+enum kvaser_usb_led_state {
+ KVASER_USB_LED_ON = 0,
+ KVASER_USB_LED_OFF = 1,
+};
+
#define KVASER_USB_HYDRA_MAX_CMD_LEN 128
struct kvaser_usb_dev_card_data_hydra {
u8 channel_to_he[KVASER_USB_MAX_NET_DEVICES];
@@ -149,6 +154,7 @@ struct kvaser_usb_net_priv {
* @dev_get_software_details: get software details
* @dev_get_card_info: get card info
* @dev_get_capabilities: discover device capabilities
+ * @dev_set_led: turn on/off device LED
*
* @dev_set_opt_mode: set ctrlmod
* @dev_start_chip: start the CAN controller
@@ -176,6 +182,9 @@ struct kvaser_usb_dev_ops {
int (*dev_get_software_details)(struct kvaser_usb *dev);
int (*dev_get_card_info)(struct kvaser_usb *dev);
int (*dev_get_capabilities)(struct kvaser_usb *dev);
+ int (*dev_set_led)(struct kvaser_usb_net_priv *priv,
+ enum kvaser_usb_led_state state,
+ u16 duration_ms);
int (*dev_set_opt_mode)(const struct kvaser_usb_net_priv *priv);
int (*dev_start_chip)(struct kvaser_usb_net_priv *priv);
int (*dev_stop_chip)(struct kvaser_usb_net_priv *priv);
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index 8e88b5917796..758fd13f1bf4 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -12,6 +12,7 @@
* distinguish between ERROR_WARNING and ERROR_ACTIVE.
*/
+#include <linux/bitfield.h>
#include <linux/completion.h>
#include <linux/device.h>
#include <linux/gfp.h>
@@ -67,6 +68,8 @@ static const struct kvaser_usb_dev_cfg kvaser_usb_hydra_dev_cfg_rt;
#define CMD_SET_BUSPARAMS_RESP 85
#define CMD_GET_CAPABILITIES_REQ 95
#define CMD_GET_CAPABILITIES_RESP 96
+#define CMD_LED_ACTION_REQ 101
+#define CMD_LED_ACTION_RESP 102
#define CMD_RX_MESSAGE 106
#define CMD_MAP_CHANNEL_REQ 200
#define CMD_MAP_CHANNEL_RESP 201
@@ -217,6 +220,22 @@ struct kvaser_cmd_get_busparams_res {
u8 reserved[20];
} __packed;
+/* The device has two LEDs per CAN channel
+ * The LSB of action field controls the state:
+ * 0 = ON
+ * 1 = OFF
+ * The remaining bits of action field is the LED index
+ */
+#define KVASER_USB_HYDRA_LED_IDX_MASK GENMASK(31, 1)
+#define KVASER_USB_HYDRA_LED_YELLOW_CH0_IDX 3
+#define KVASER_USB_HYDRA_LEDS_PER_CHANNEL 2
+struct kvaser_cmd_led_action_req {
+ u8 action;
+ u8 padding;
+ __le16 duration_ms;
+ u8 reserved[24];
+} __packed;
+
/* Ctrl modes */
#define KVASER_USB_HYDRA_CTRLMODE_NORMAL 0x01
#define KVASER_USB_HYDRA_CTRLMODE_LISTEN 0x02
@@ -299,6 +318,8 @@ struct kvaser_cmd {
struct kvaser_cmd_get_busparams_req get_busparams_req;
struct kvaser_cmd_get_busparams_res get_busparams_res;
+ struct kvaser_cmd_led_action_req led_action_req;
+
struct kvaser_cmd_chip_state_event chip_state_event;
struct kvaser_cmd_set_ctrlmode set_ctrlmode;
@@ -1390,6 +1411,7 @@ static void kvaser_usb_hydra_handle_cmd_std(const struct kvaser_usb *dev,
/* Ignored commands */
case CMD_SET_BUSPARAMS_RESP:
case CMD_SET_BUSPARAMS_FD_RESP:
+ case CMD_LED_ACTION_RESP:
break;
default:
@@ -1946,6 +1968,34 @@ static int kvaser_usb_hydra_get_capabilities(struct kvaser_usb *dev)
return 0;
}
+static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
+ enum kvaser_usb_led_state state,
+ u16 duration_ms)
+{
+ struct kvaser_usb *dev = priv->dev;
+ struct kvaser_cmd *cmd;
+ int ret;
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ cmd->header.cmd_no = CMD_LED_ACTION_REQ;
+ kvaser_usb_hydra_set_cmd_dest_he(cmd, dev->card_data.hydra.sysdbg_he);
+ kvaser_usb_hydra_set_cmd_transid(cmd, kvaser_usb_hydra_get_next_transid(dev));
+
+ cmd->led_action_req.duration_ms = cpu_to_le16(duration_ms);
+ cmd->led_action_req.action = state |
+ FIELD_PREP(KVASER_USB_HYDRA_LED_IDX_MASK,
+ KVASER_USB_HYDRA_LED_YELLOW_CH0_IDX +
+ KVASER_USB_HYDRA_LEDS_PER_CHANNEL * priv->channel);
+
+ ret = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+ kfree(cmd);
+
+ return ret;
+}
+
static int kvaser_usb_hydra_set_opt_mode(const struct kvaser_usb_net_priv *priv)
{
struct kvaser_usb *dev = priv->dev;
@@ -2149,6 +2199,7 @@ const struct kvaser_usb_dev_ops kvaser_usb_hydra_dev_ops = {
.dev_get_software_details = kvaser_usb_hydra_get_software_details,
.dev_get_card_info = kvaser_usb_hydra_get_card_info,
.dev_get_capabilities = kvaser_usb_hydra_get_capabilities,
+ .dev_set_led = kvaser_usb_hydra_set_led,
.dev_set_opt_mode = kvaser_usb_hydra_set_opt_mode,
.dev_start_chip = kvaser_usb_hydra_start_chip,
.dev_stop_chip = kvaser_usb_hydra_stop_chip,
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
index 6a45adcc45bd..a67855521ccc 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -10,6 +10,7 @@
* Copyright (C) 2015 Valeo S.A.
*/
+#include <linux/bitfield.h>
#include <linux/completion.h>
#include <linux/device.h>
#include <linux/gfp.h>
@@ -81,6 +82,8 @@
#define CMD_FLUSH_QUEUE_REPLY 68
#define CMD_GET_CAPABILITIES_REQ 95
#define CMD_GET_CAPABILITIES_RESP 96
+#define CMD_LED_ACTION_REQ 101
+#define CMD_LED_ACTION_RESP 102
#define CMD_LEAF_LOG_MESSAGE 106
@@ -173,6 +176,21 @@ struct kvaser_cmd_busparams {
struct kvaser_usb_busparams busparams;
} __packed;
+/* The device has one LED per CAN channel
+ * The LSB of action field controls the state:
+ * 0 = ON
+ * 1 = OFF
+ * The remaining bits of action field is the LED index
+ */
+#define KVASER_USB_LEAF_LED_IDX_MASK GENMASK(31, 1)
+#define KVASER_USB_LEAF_LED_YELLOW_CH0_IDX 2
+struct kvaser_cmd_led_action_req {
+ u8 tid;
+ u8 action;
+ __le16 duration_ms;
+ u8 padding[24];
+} __packed;
+
struct kvaser_cmd_tx_can {
u8 channel;
u8 tid;
@@ -359,6 +377,8 @@ struct kvaser_cmd {
struct kvaser_cmd_cardinfo cardinfo;
struct kvaser_cmd_busparams busparams;
+ struct kvaser_cmd_led_action_req led_action_req;
+
struct kvaser_cmd_rx_can_header rx_can_header;
struct kvaser_cmd_tx_acknowledge_header tx_acknowledge_header;
@@ -409,6 +429,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = {
[CMD_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event),
/* ignored events: */
[CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY,
+ [CMD_LED_ACTION_RESP] = CMD_SIZE_ANY,
};
static const u8 kvaser_usb_leaf_cmd_sizes_usbcan[] = {
@@ -423,6 +444,8 @@ static const u8 kvaser_usb_leaf_cmd_sizes_usbcan[] = {
[CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.usbcan.can_error_event),
[CMD_ERROR_EVENT] = kvaser_fsize(u.usbcan.error_event),
[CMD_USBCAN_CLOCK_OVERFLOW_EVENT] = kvaser_fsize(u.usbcan.clk_overflow_event),
+ /* ignored events: */
+ [CMD_LED_ACTION_RESP] = CMD_SIZE_ANY,
};
/* Summary of a kvaser error event, for a unified Leaf/Usbcan error
@@ -924,6 +947,34 @@ static int kvaser_usb_leaf_get_capabilities_leaf(struct kvaser_usb *dev)
return 0;
}
+static int kvaser_usb_leaf_set_led(struct kvaser_usb_net_priv *priv,
+ enum kvaser_usb_led_state state,
+ u16 duration_ms)
+{
+ struct kvaser_usb *dev = priv->dev;
+ struct kvaser_cmd *cmd;
+ int ret;
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ cmd->id = CMD_LED_ACTION_REQ;
+ cmd->len = CMD_HEADER_LEN + sizeof(struct kvaser_cmd_led_action_req);
+ cmd->u.led_action_req.tid = 0xff;
+
+ cmd->u.led_action_req.duration_ms = cpu_to_le16(duration_ms);
+ cmd->u.led_action_req.action = state |
+ FIELD_PREP(KVASER_USB_LEAF_LED_IDX_MASK,
+ KVASER_USB_LEAF_LED_YELLOW_CH0_IDX +
+ priv->channel);
+
+ ret = kvaser_usb_send_cmd(dev, cmd, cmd->len);
+ kfree(cmd);
+
+ return ret;
+}
+
static int kvaser_usb_leaf_get_capabilities(struct kvaser_usb *dev)
{
int err = 0;
@@ -1638,6 +1689,8 @@ static void kvaser_usb_leaf_handle_command(struct kvaser_usb *dev,
if (dev->driver_info->family != KVASER_LEAF)
goto warn;
break;
+ case CMD_LED_ACTION_RESP:
+ break;
default:
warn: dev_warn(&dev->intf->dev, "Unhandled command (%d)\n", cmd->id);
@@ -1927,6 +1980,7 @@ const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops = {
.dev_get_software_details = NULL,
.dev_get_card_info = kvaser_usb_leaf_get_card_info,
.dev_get_capabilities = kvaser_usb_leaf_get_capabilities,
+ .dev_set_led = kvaser_usb_leaf_set_led,
.dev_set_opt_mode = kvaser_usb_leaf_set_opt_mode,
.dev_start_chip = kvaser_usb_leaf_start_chip,
.dev_stop_chip = kvaser_usb_leaf_stop_chip,
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device
2025-07-24 9:24 ` [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device Jimmy Assarsson
@ 2025-07-24 18:26 ` Simon Horman
2025-07-25 12:44 ` Jimmy Assarsson
0 siblings, 1 reply; 19+ messages in thread
From: Simon Horman @ 2025-07-24 18:26 UTC (permalink / raw)
To: Jimmy Assarsson
Cc: linux-can, Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol,
netdev
On Thu, Jul 24, 2025 at 11:24:55AM +0200, Jimmy Assarsson wrote:
> Add support to turn on/off CAN LEDs on device.
>
> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
...
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
...
> +static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
> + enum kvaser_usb_led_state state,
> + u16 duration_ms)
> +{
> + struct kvaser_usb *dev = priv->dev;
> + struct kvaser_cmd *cmd;
> + int ret;
> +
> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> + if (!cmd)
> + return -ENOMEM;
> +
> + cmd->header.cmd_no = CMD_LED_ACTION_REQ;
> + kvaser_usb_hydra_set_cmd_dest_he(cmd, dev->card_data.hydra.sysdbg_he);
> + kvaser_usb_hydra_set_cmd_transid(cmd, kvaser_usb_hydra_get_next_transid(dev));
> +
> + cmd->led_action_req.duration_ms = cpu_to_le16(duration_ms);
> + cmd->led_action_req.action = state |
> + FIELD_PREP(KVASER_USB_HYDRA_LED_IDX_MASK,
> + KVASER_USB_HYDRA_LED_YELLOW_CH0_IDX +
> + KVASER_USB_HYDRA_LEDS_PER_CHANNEL * priv->channel);
> +
> + ret = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
When building this file with GCC 15.1.0 with KCFLAGS=-Warray-bounds
I see the following:
In file included from ./include/linux/byteorder/little_endian.h:5,
from ./arch/x86/include/uapi/asm/byteorder.h:5,
from ./include/linux/bitfield.h:12,
from drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:15:
In function 'kvaser_usb_hydra_cmd_size',
inlined from 'kvaser_usb_hydra_set_led' at drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:1993:38:
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:532:65: warning: array subscript 'struct kvaser_cmd_ext[0]' is partly outside array bounds of 'unsigned char[32]' [-Warray-bounds=]
532 | ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
./include/uapi/linux/byteorder/little_endian.h:37:51: note: in definition of macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:532:23: note: in expansion of macro 'le16_to_cpu'
532 | ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
| ^~~~~~~~~~~
In file included from ./include/linux/fs.h:46,
from ./include/linux/compat.h:17,
from ./arch/x86/include/asm/ia32.h:7,
from ./arch/x86/include/asm/elf.h:10,
from ./include/linux/elf.h:6,
from ./include/linux/module.h:19,
from ./include/linux/device/driver.h:21,
from ./include/linux/device.h:32,
from drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:17:
In function 'kmalloc_noprof',
inlined from 'kzalloc_noprof' at ./include/linux/slab.h:1039:9,
inlined from 'kvaser_usb_hydra_set_led' at drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:1979:8:
./include/linux/slab.h:905:24: note: object of size 32 allocated by '__kmalloc_cache_noprof'
905 | return __kmalloc_cache_noprof(
| ^~~~~~~~~~~~~~~~~~~~~~~
906 | kmalloc_caches[kmalloc_type(flags, _RET_IP_)][index],
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
907 | flags, size);
| ~~~~~~~~~~~~
if (cmd->header.cmd_no == CMD_EXTENDED)
ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
GCC seems to know that:
* cmd was allocated sizeof(*cmd) = 32 bytes
* struct kvaser_cmd_ext is larger than this (96 bytes)
And it thinks that cmd->header.cmd_no might be CMD_EXTENDED.
This is not true, becuae .cmd_no is set to CMD_LED_ACTION_REQ
earlier in kvaser_usb_hydra_set_led. But still, GCC produces
a big fat warning.
On the one hand we might say this is a shortcoming in GCC,
a position I agree with. But on the other hand, we might follow
the pattern used elsewhere in this file for similar functions,
which seems to make GCC happy, I guess, and it is strictly a guess,
because less context is needed for it to analyse things correctly.
That is, do this:
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index 758fd13f1bf4..a4402b4845c6 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1974,6 +1974,7 @@ static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
{
struct kvaser_usb *dev = priv->dev;
struct kvaser_cmd *cmd;
+ size_t cmd_len;
int ret;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1981,6 +1982,7 @@ static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
return -ENOMEM;
cmd->header.cmd_no = CMD_LED_ACTION_REQ;
+ cmd_len = kvaser_usb_hydra_cmd_size(cmd);
kvaser_usb_hydra_set_cmd_dest_he(cmd, dev->card_data.hydra.sysdbg_he);
kvaser_usb_hydra_set_cmd_transid(cmd, kvaser_usb_hydra_get_next_transid(dev));
@@ -1990,7 +1992,7 @@ static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
KVASER_USB_HYDRA_LED_YELLOW_CH0_IDX +
KVASER_USB_HYDRA_LEDS_PER_CHANNEL * priv->channel);
- ret = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+ ret = kvaser_usb_send_cmd(dev, cmd, cmd_len);
kfree(cmd);
return ret;
> + kfree(cmd);
> +
> + return ret;
> +}
...
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device
2025-07-24 18:26 ` Simon Horman
@ 2025-07-25 12:44 ` Jimmy Assarsson
2025-07-26 19:48 ` Simon Horman
0 siblings, 1 reply; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-25 12:44 UTC (permalink / raw)
To: Simon Horman, Jimmy Assarsson
Cc: linux-can, Marc Kleine-Budde, Vincent Mailhol, netdev
On 7/24/25 8:26 PM, Simon Horman wrote:
> On Thu, Jul 24, 2025 at 11:24:55AM +0200, Jimmy Assarsson wrote:
>> Add support to turn on/off CAN LEDs on device.
>>
>> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
>
> ...
>
>> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
>
> ...
>
>> +static int kvaser_usb_hydra_set_led(struct kvaser_usb_net_priv *priv,
>> + enum kvaser_usb_led_state state,
>> + u16 duration_ms)
>> +{
>> + struct kvaser_usb *dev = priv->dev;
>> + struct kvaser_cmd *cmd;
>> + int ret;
>> +
>> + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
>> + if (!cmd)
>> + return -ENOMEM;
>> +
>> + cmd->header.cmd_no = CMD_LED_ACTION_REQ;
>> + kvaser_usb_hydra_set_cmd_dest_he(cmd, dev->card_data.hydra.sysdbg_he);
>> + kvaser_usb_hydra_set_cmd_transid(cmd, kvaser_usb_hydra_get_next_transid(dev));
>> +
>> + cmd->led_action_req.duration_ms = cpu_to_le16(duration_ms);
>> + cmd->led_action_req.action = state |
>> + FIELD_PREP(KVASER_USB_HYDRA_LED_IDX_MASK,
>> + KVASER_USB_HYDRA_LED_YELLOW_CH0_IDX +
>> + KVASER_USB_HYDRA_LEDS_PER_CHANNEL * priv->channel);
>> +
>> + ret = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
>
> When building this file with GCC 15.1.0 with KCFLAGS=-Warray-bounds
> I see the following:
>
> In file included from ./include/linux/byteorder/little_endian.h:5,
> from ./arch/x86/include/uapi/asm/byteorder.h:5,
> from ./include/linux/bitfield.h:12,
> from drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:15:
> In function 'kvaser_usb_hydra_cmd_size',
> inlined from 'kvaser_usb_hydra_set_led' at drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:1993:38:
> drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:532:65: warning: array subscript 'struct kvaser_cmd_ext[0]' is partly outside array bounds of 'unsigned char[32]' [-Warray-bounds=]
> 532 | ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
> ./include/uapi/linux/byteorder/little_endian.h:37:51: note: in definition of macro '__le16_to_cpu'
> 37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
> | ^
> drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:532:23: note: in expansion of macro 'le16_to_cpu'
> 532 | ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
> | ^~~~~~~~~~~
> In file included from ./include/linux/fs.h:46,
> from ./include/linux/compat.h:17,
> from ./arch/x86/include/asm/ia32.h:7,
> from ./arch/x86/include/asm/elf.h:10,
> from ./include/linux/elf.h:6,
> from ./include/linux/module.h:19,
> from ./include/linux/device/driver.h:21,
> from ./include/linux/device.h:32,
> from drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:17:
> In function 'kmalloc_noprof',
> inlined from 'kzalloc_noprof' at ./include/linux/slab.h:1039:9,
> inlined from 'kvaser_usb_hydra_set_led' at drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:1979:8:
> ./include/linux/slab.h:905:24: note: object of size 32 allocated by '__kmalloc_cache_noprof'
> 905 | return __kmalloc_cache_noprof(
> | ^~~~~~~~~~~~~~~~~~~~~~~
> 906 | kmalloc_caches[kmalloc_type(flags, _RET_IP_)][index],
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 907 | flags, size);
> | ~~~~~~~~~~~~
>
> if (cmd->header.cmd_no == CMD_EXTENDED)
> ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);
>
> GCC seems to know that:
> * cmd was allocated sizeof(*cmd) = 32 bytes
> * struct kvaser_cmd_ext is larger than this (96 bytes)
>
> And it thinks that cmd->header.cmd_no might be CMD_EXTENDED.
> This is not true, becuae .cmd_no is set to CMD_LED_ACTION_REQ
> earlier in kvaser_usb_hydra_set_led. But still, GCC produces
> a big fat warning.
>
> On the one hand we might say this is a shortcoming in GCC,
> a position I agree with. But on the other hand, we might follow
> the pattern used elsewhere in this file for similar functions,
> which seems to make GCC happy, I guess, and it is strictly a guess,
> because less context is needed for it to analyse things correctly.
Thanks for finding this!
Marc Kleine-Budde actually sorted this out for other commands some years ago [1],
but I had completely forgotten.
[1] https://lore.kernel.org/all/20221219110104.1073881-1-mkl@pengutronix.de
...
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device
2025-07-25 12:44 ` Jimmy Assarsson
@ 2025-07-26 19:48 ` Simon Horman
0 siblings, 0 replies; 19+ messages in thread
From: Simon Horman @ 2025-07-26 19:48 UTC (permalink / raw)
To: Jimmy Assarsson
Cc: Jimmy Assarsson, linux-can, Marc Kleine-Budde, Vincent Mailhol,
netdev
On Fri, Jul 25, 2025 at 02:44:52PM +0200, Jimmy Assarsson wrote:
> On 7/24/25 8:26 PM, Simon Horman wrote:
> > On Thu, Jul 24, 2025 at 11:24:55AM +0200, Jimmy Assarsson wrote:
...
> > GCC seems to know that:
> > * cmd was allocated sizeof(*cmd) = 32 bytes
> > * struct kvaser_cmd_ext is larger than this (96 bytes)
> >
> > And it thinks that cmd->header.cmd_no might be CMD_EXTENDED.
> > This is not true, becuae .cmd_no is set to CMD_LED_ACTION_REQ
> > earlier in kvaser_usb_hydra_set_led. But still, GCC produces
> > a big fat warning.
> >
> > On the one hand we might say this is a shortcoming in GCC,
> > a position I agree with. But on the other hand, we might follow
> > the pattern used elsewhere in this file for similar functions,
> > which seems to make GCC happy, I guess, and it is strictly a guess,
> > because less context is needed for it to analyse things correctly.
>
> Thanks for finding this!
>
> Marc Kleine-Budde actually sorted this out for other commands some years ago [1],
> but I had completely forgotten.
>
> [1] https://lore.kernel.org/all/20221219110104.1073881-1-mkl@pengutronix.de
Nice, thanks for digging that up.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 02/11] can: kvaser_usb: Add support for ethtool set_phys_id()
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device Jimmy Assarsson
@ 2025-07-24 9:24 ` Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 03/11] can: kvaser_usb: Assign netdev.dev_port based on device channel index Jimmy Assarsson
` (9 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:24 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Add support for ethtool set_phys_id(), to physically locate devices by
flashing a LED on the device.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- Return inside the switch-case. Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#md10566c624e75c59ec735fed16d5ec4cbdb38430
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index daf42080f942..c74875f978c4 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -753,6 +753,31 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
return ret;
}
+static int kvaser_usb_set_phys_id(struct net_device *netdev,
+ enum ethtool_phys_id_state state)
+{
+ struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+ const struct kvaser_usb_dev_ops *ops = priv->dev->driver_info->ops;
+
+ switch (state) {
+ case ETHTOOL_ID_ACTIVE:
+ return 3; /* 3 On/Off cycles per second */
+
+ case ETHTOOL_ID_ON:
+ return ops->dev_set_led(priv, KVASER_USB_LED_ON, 1000);
+
+ case ETHTOOL_ID_OFF:
+ return ops->dev_set_led(priv, KVASER_USB_LED_OFF, 1000);
+
+ case ETHTOOL_ID_INACTIVE:
+ /* Turn LED off and restore standard function after 1ms */
+ return ops->dev_set_led(priv, KVASER_USB_LED_OFF, 1);
+
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct net_device_ops kvaser_usb_netdev_ops = {
.ndo_open = kvaser_usb_open,
.ndo_stop = kvaser_usb_close,
@@ -763,6 +788,7 @@ static const struct net_device_ops kvaser_usb_netdev_ops = {
static const struct ethtool_ops kvaser_usb_ethtool_ops = {
.get_ts_info = can_ethtool_op_get_ts_info_hwts,
+ .set_phys_id = kvaser_usb_set_phys_id,
};
static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 03/11] can: kvaser_usb: Assign netdev.dev_port based on device channel index
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 01/11] can: kvaser_usb: Add support to control CAN LEDs on device Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 02/11] can: kvaser_usb: Add support for ethtool set_phys_id() Jimmy Assarsson
@ 2025-07-24 9:24 ` Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 04/11] can: kvaser_usb: Add intermediate variables Jimmy Assarsson
` (8 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:24 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Assign netdev.dev_port based on the device channel index, to indicate the
port number of the network device.
While this driver already uses netdev.dev_id for that purpose, dev_port is
more appropriate. However, retain dev_id to avoid potential regressions.
Fixes: 3e66d0138c05 ("can: populate netdev::dev_id for udev discrimination")
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index c74875f978c4..7be8604bf760 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -878,6 +878,7 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
netdev->ethtool_ops = &kvaser_usb_ethtool_ops;
SET_NETDEV_DEV(netdev, &dev->intf->dev);
netdev->dev_id = channel;
+ netdev->dev_port = channel;
dev->nets[channel] = priv;
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 04/11] can: kvaser_usb: Add intermediate variables
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (2 preceding siblings ...)
2025-07-24 9:24 ` [PATCH v2 03/11] can: kvaser_usb: Assign netdev.dev_port based on device channel index Jimmy Assarsson
@ 2025-07-24 9:24 ` Jimmy Assarsson
2025-07-24 9:24 ` [PATCH v2 05/11] can: kvaser_usb: Move comment regarding max_tx_urbs Jimmy Assarsson
` (7 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:24 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Add intermediate variables, for readability and to simplify future patches.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 20 ++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index 7be8604bf760..46e6cda0bf8d 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -364,10 +364,13 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
err = usb_submit_urb(urb, GFP_ATOMIC);
if (err == -ENODEV) {
for (i = 0; i < dev->nchannels; i++) {
- if (!dev->nets[i])
+ struct kvaser_usb_net_priv *priv;
+
+ priv = dev->nets[i];
+ if (!priv)
continue;
- netif_device_detach(dev->nets[i]->netdev);
+ netif_device_detach(priv->netdev);
}
} else if (err) {
dev_err(&dev->intf->dev,
@@ -795,24 +798,27 @@ static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
{
const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
int i;
+ struct kvaser_usb_net_priv *priv;
for (i = 0; i < dev->nchannels; i++) {
- if (!dev->nets[i])
+ priv = dev->nets[i];
+ if (!priv)
continue;
- unregister_candev(dev->nets[i]->netdev);
+ unregister_candev(priv->netdev);
}
kvaser_usb_unlink_all_urbs(dev);
for (i = 0; i < dev->nchannels; i++) {
- if (!dev->nets[i])
+ priv = dev->nets[i];
+ if (!priv)
continue;
if (ops->dev_remove_channel)
- ops->dev_remove_channel(dev->nets[i]);
+ ops->dev_remove_channel(priv);
- free_candev(dev->nets[i]->netdev);
+ free_candev(priv->netdev);
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 05/11] can: kvaser_usb: Move comment regarding max_tx_urbs
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (3 preceding siblings ...)
2025-07-24 9:24 ` [PATCH v2 04/11] can: kvaser_usb: Add intermediate variables Jimmy Assarsson
@ 2025-07-24 9:24 ` Jimmy Assarsson
2025-07-24 9:25 ` [PATCH v2 06/11] can: kvaser_usb: Store the different firmware version components in a struct Jimmy Assarsson
` (6 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:24 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Move comment regarding max_tx_urbs, to where the struct member is declared.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index 032dc1821f04..fba972e7220d 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -101,12 +101,12 @@ struct kvaser_usb {
struct usb_endpoint_descriptor *bulk_in, *bulk_out;
struct usb_anchor rx_submitted;
+ u32 fw_version;
+ unsigned int nchannels;
/* @max_tx_urbs: Firmware-reported maximum number of outstanding,
* not yet ACKed, transmissions on this device. This value is
* also used as a sentinel for marking free tx contexts.
*/
- u32 fw_version;
- unsigned int nchannels;
unsigned int max_tx_urbs;
struct kvaser_usb_dev_card_data card_data;
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 06/11] can: kvaser_usb: Store the different firmware version components in a struct
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (4 preceding siblings ...)
2025-07-24 9:24 ` [PATCH v2 05/11] can: kvaser_usb: Move comment regarding max_tx_urbs Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-24 9:25 ` [PATCH v2 07/11] can: kvaser_usb: Store additional device information Jimmy Assarsson
` (5 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Store firmware version in kvaser_usb_fw_version struct, specifying the
different components of the version number.
And drop debug prinout of firmware version, since later patches will expose
it via the devlink interface.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- Drop debug prinout, since it will be exposed via devlink.
Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#m2003548deedfeafee5c57ee2e2f610d364220fae
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 12 +++++++++++-
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 5 -----
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 6 +++++-
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 15 +++++++++++++--
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index fba972e7220d..a36d86494113 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -47,6 +47,10 @@
#define KVASER_USB_CAP_EXT_CAP 0x02
#define KVASER_USB_HYDRA_CAP_EXT_CMD 0x04
+#define KVASER_USB_SW_VERSION_MAJOR_MASK GENMASK(31, 24)
+#define KVASER_USB_SW_VERSION_MINOR_MASK GENMASK(23, 16)
+#define KVASER_USB_SW_VERSION_BUILD_MASK GENMASK(15, 0)
+
struct kvaser_usb_dev_cfg;
enum kvaser_usb_leaf_family {
@@ -83,6 +87,12 @@ struct kvaser_usb_tx_urb_context {
u32 echo_index;
};
+struct kvaser_usb_fw_version {
+ u8 major;
+ u8 minor;
+ u16 build;
+};
+
struct kvaser_usb_busparams {
__le32 bitrate;
u8 tseg1;
@@ -101,7 +111,7 @@ struct kvaser_usb {
struct usb_endpoint_descriptor *bulk_in, *bulk_out;
struct usb_anchor rx_submitted;
- u32 fw_version;
+ struct kvaser_usb_fw_version fw_version;
unsigned int nchannels;
/* @max_tx_urbs: Firmware-reported maximum number of outstanding,
* not yet ACKed, transmissions on this device. This value is
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index 46e6cda0bf8d..2313fbc1a2c3 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -963,11 +963,6 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (WARN_ON(!dev->cfg))
return -ENODEV;
- dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
- ((dev->fw_version >> 24) & 0xff),
- ((dev->fw_version >> 16) & 0xff),
- (dev->fw_version & 0xffff));
-
dev_dbg(&intf->dev, "Max outstanding tx = %d URBs\n", dev->max_tx_urbs);
err = ops->dev_get_card_info(dev);
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index 758fd13f1bf4..4107b39e396b 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1839,6 +1839,7 @@ static int kvaser_usb_hydra_get_software_details(struct kvaser_usb *dev)
size_t cmd_len;
int err;
u32 flags;
+ u32 fw_version;
struct kvaser_usb_dev_card_data *card_data = &dev->card_data;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1863,7 +1864,10 @@ static int kvaser_usb_hydra_get_software_details(struct kvaser_usb *dev)
if (err)
goto end;
- dev->fw_version = le32_to_cpu(cmd->sw_detail_res.sw_version);
+ fw_version = le32_to_cpu(cmd->sw_detail_res.sw_version);
+ dev->fw_version.major = FIELD_GET(KVASER_USB_SW_VERSION_MAJOR_MASK, fw_version);
+ dev->fw_version.minor = FIELD_GET(KVASER_USB_SW_VERSION_MINOR_MASK, fw_version);
+ dev->fw_version.build = FIELD_GET(KVASER_USB_SW_VERSION_BUILD_MASK, fw_version);
flags = le32_to_cpu(cmd->sw_detail_res.sw_flags);
if (flags & KVASER_USB_HYDRA_SW_FLAG_FW_BAD) {
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
index a67855521ccc..b4f5d4fba630 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -741,9 +741,13 @@ static int kvaser_usb_leaf_send_simple_cmd(const struct kvaser_usb *dev,
static void kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
const struct leaf_cmd_softinfo *softinfo)
{
+ u32 fw_version;
u32 sw_options = le32_to_cpu(softinfo->sw_options);
- dev->fw_version = le32_to_cpu(softinfo->fw_version);
+ fw_version = le32_to_cpu(softinfo->fw_version);
+ dev->fw_version.major = FIELD_GET(KVASER_USB_SW_VERSION_MAJOR_MASK, fw_version);
+ dev->fw_version.minor = FIELD_GET(KVASER_USB_SW_VERSION_MINOR_MASK, fw_version);
+ dev->fw_version.build = FIELD_GET(KVASER_USB_SW_VERSION_BUILD_MASK, fw_version);
dev->max_tx_urbs = le16_to_cpu(softinfo->max_outstanding_tx);
if (sw_options & KVASER_USB_LEAF_SWOPTION_EXT_CAP)
@@ -784,6 +788,7 @@ static int kvaser_usb_leaf_get_software_info_inner(struct kvaser_usb *dev)
{
struct kvaser_cmd cmd;
int err;
+ u32 fw_version;
err = kvaser_usb_leaf_send_simple_cmd(dev, CMD_GET_SOFTWARE_INFO, 0);
if (err)
@@ -798,7 +803,13 @@ static int kvaser_usb_leaf_get_software_info_inner(struct kvaser_usb *dev)
kvaser_usb_leaf_get_software_info_leaf(dev, &cmd.u.leaf.softinfo);
break;
case KVASER_USBCAN:
- dev->fw_version = le32_to_cpu(cmd.u.usbcan.softinfo.fw_version);
+ fw_version = le32_to_cpu(cmd.u.usbcan.softinfo.fw_version);
+ dev->fw_version.major = FIELD_GET(KVASER_USB_SW_VERSION_MAJOR_MASK,
+ fw_version);
+ dev->fw_version.minor = FIELD_GET(KVASER_USB_SW_VERSION_MINOR_MASK,
+ fw_version);
+ dev->fw_version.build = FIELD_GET(KVASER_USB_SW_VERSION_BUILD_MASK,
+ fw_version);
dev->max_tx_urbs =
le16_to_cpu(cmd.u.usbcan.softinfo.max_outstanding_tx);
dev->cfg = &kvaser_usb_leaf_usbcan_dev_cfg;
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 07/11] can: kvaser_usb: Store additional device information
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (5 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 06/11] can: kvaser_usb: Store the different firmware version components in a struct Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-24 9:25 ` [PATCH v2 08/11] can: kvaser_usb: Add devlink support Jimmy Assarsson
` (4 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Store additional device information; EAN (product number), serial_number
and hardware revision.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 3 +++
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 6 +++++-
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 6 +++++-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index a36d86494113..35c2cf3d4486 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -111,7 +111,10 @@ struct kvaser_usb {
struct usb_endpoint_descriptor *bulk_in, *bulk_out;
struct usb_anchor rx_submitted;
+ u32 ean[2];
+ u32 serial_number;
struct kvaser_usb_fw_version fw_version;
+ u8 hw_revision;
unsigned int nchannels;
/* @max_tx_urbs: Firmware-reported maximum number of outstanding,
* not yet ACKed, transmissions on this device. This value is
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index 4107b39e396b..a8930bf933a7 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -114,7 +114,7 @@ struct kvaser_cmd_card_info {
__le32 clock_res;
__le32 mfg_date;
__le32 ean[2];
- u8 hw_version;
+ u8 hw_revision;
u8 usb_mode;
u8 hw_type;
u8 reserved0;
@@ -1918,6 +1918,10 @@ static int kvaser_usb_hydra_get_card_info(struct kvaser_usb *dev)
err = kvaser_usb_hydra_wait_cmd(dev, CMD_GET_CARD_INFO_RESP, &cmd);
if (err)
return err;
+ dev->ean[1] = le32_to_cpu(cmd.card_info.ean[1]);
+ dev->ean[0] = le32_to_cpu(cmd.card_info.ean[0]);
+ dev->serial_number = le32_to_cpu(cmd.card_info.serial_number);
+ dev->hw_revision = cmd.card_info.hw_revision;
dev->nchannels = cmd.card_info.nchannels;
if (dev->nchannels > KVASER_USB_MAX_NET_DEVICES)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
index b4f5d4fba630..c29828a94ad0 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -138,7 +138,7 @@ struct kvaser_cmd_cardinfo {
__le32 padding0;
__le32 clock_resolution;
__le32 mfgdate;
- u8 ean[8];
+ __le32 ean[2];
u8 hw_revision;
union {
struct {
@@ -854,6 +854,10 @@ static int kvaser_usb_leaf_get_card_info(struct kvaser_usb *dev)
(dev->driver_info->family == KVASER_USBCAN &&
dev->nchannels > MAX_USBCAN_NET_DEVICES))
return -EINVAL;
+ dev->ean[1] = le32_to_cpu(cmd.u.cardinfo.ean[1]);
+ dev->ean[0] = le32_to_cpu(cmd.u.cardinfo.ean[0]);
+ dev->serial_number = le32_to_cpu(cmd.u.cardinfo.serial_number);
+ dev->hw_revision = cmd.u.cardinfo.hw_revision;
return 0;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 08/11] can: kvaser_usb: Add devlink support
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (6 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 07/11] can: kvaser_usb: Store additional device information Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-24 9:25 ` [PATCH v2 09/11] can: kvaser_usb: Expose device information via devlink info_get() Jimmy Assarsson
` (3 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Add devlink support at device level.
Example output:
$ devlink dev
usb/1-1.3:1.0
$ devlink dev info
usb/1-1.3:1.0:
driver kvaser_usb
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- Add two space indentation to terminal output.
Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#m31ee4aad13ee29d5559b56fdce842609ae4f67c5
drivers/net/can/usb/Kconfig | 1 +
drivers/net/can/usb/kvaser_usb/Makefile | 2 +-
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 3 +
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 72 ++++++++++++-------
.../can/usb/kvaser_usb/kvaser_usb_devlink.c | 10 +++
5 files changed, 62 insertions(+), 26 deletions(-)
create mode 100644 drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index 9dae0c71a2e1..a7547a83120e 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -66,6 +66,7 @@ config CAN_GS_USB
config CAN_KVASER_USB
tristate "Kvaser CAN/USB interface"
+ select NET_DEVLINK
help
This driver adds support for Kvaser CAN/USB devices like Kvaser
Leaf Light, Kvaser USBcan II and Kvaser Memorator Pro 5xHS.
diff --git a/drivers/net/can/usb/kvaser_usb/Makefile b/drivers/net/can/usb/kvaser_usb/Makefile
index cf260044f0b9..41b4a11555aa 100644
--- a/drivers/net/can/usb/kvaser_usb/Makefile
+++ b/drivers/net/can/usb/kvaser_usb/Makefile
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
-kvaser_usb-y = kvaser_usb_core.o kvaser_usb_leaf.o kvaser_usb_hydra.o
+kvaser_usb-y = kvaser_usb_core.o kvaser_usb_devlink.o kvaser_usb_leaf.o kvaser_usb_hydra.o
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index 35c2cf3d4486..d5f913ac9b44 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -27,6 +27,7 @@
#include <linux/spinlock.h>
#include <linux/types.h>
#include <linux/usb.h>
+#include <net/devlink.h>
#include <linux/can.h>
#include <linux/can/dev.h>
@@ -226,6 +227,8 @@ struct kvaser_usb_dev_cfg {
extern const struct kvaser_usb_dev_ops kvaser_usb_hydra_dev_ops;
extern const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops;
+extern const struct devlink_ops kvaser_usb_devlink_ops;
+
void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv);
int kvaser_usb_recv_cmd(const struct kvaser_usb *dev, void *cmd, int len,
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index 2313fbc1a2c3..b9b2e120a5cd 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -914,6 +914,7 @@ static int kvaser_usb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct kvaser_usb *dev;
+ struct devlink *devlink;
int err;
int i;
const struct kvaser_usb_driver_info *driver_info;
@@ -923,17 +924,20 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!driver_info)
return -ENODEV;
- dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
+ devlink = devlink_alloc(&kvaser_usb_devlink_ops, sizeof(*dev), &intf->dev);
+ if (!devlink)
return -ENOMEM;
+ dev = devlink_priv(devlink);
dev->intf = intf;
dev->driver_info = driver_info;
ops = driver_info->ops;
err = ops->dev_setup_endpoints(dev);
- if (err)
- return dev_err_probe(&intf->dev, err, "Cannot get usb endpoint(s)");
+ if (err) {
+ dev_err_probe(&intf->dev, err, "Cannot get usb endpoint(s)");
+ goto free_devlink;
+ }
dev->udev = interface_to_usbdev(intf);
@@ -944,50 +948,66 @@ static int kvaser_usb_probe(struct usb_interface *intf,
dev->card_data.ctrlmode_supported = 0;
dev->card_data.capabilities = 0;
err = ops->dev_init_card(dev);
- if (err)
- return dev_err_probe(&intf->dev, err,
- "Failed to initialize card\n");
+ if (err) {
+ dev_err_probe(&intf->dev, err,
+ "Failed to initialize card\n");
+ goto free_devlink;
+ }
err = ops->dev_get_software_info(dev);
- if (err)
- return dev_err_probe(&intf->dev, err,
- "Cannot get software info\n");
+ if (err) {
+ dev_err_probe(&intf->dev, err,
+ "Cannot get software info\n");
+ goto free_devlink;
+ }
if (ops->dev_get_software_details) {
err = ops->dev_get_software_details(dev);
- if (err)
- return dev_err_probe(&intf->dev, err,
- "Cannot get software details\n");
+ if (err) {
+ dev_err_probe(&intf->dev, err,
+ "Cannot get software details\n");
+ goto free_devlink;
+ }
}
- if (WARN_ON(!dev->cfg))
- return -ENODEV;
+ if (WARN_ON(!dev->cfg)) {
+ err = -ENODEV;
+ goto free_devlink;
+ }
dev_dbg(&intf->dev, "Max outstanding tx = %d URBs\n", dev->max_tx_urbs);
err = ops->dev_get_card_info(dev);
- if (err)
- return dev_err_probe(&intf->dev, err,
- "Cannot get card info\n");
+ if (err) {
+ dev_err_probe(&intf->dev, err,
+ "Cannot get card info\n");
+ goto free_devlink;
+ }
if (ops->dev_get_capabilities) {
err = ops->dev_get_capabilities(dev);
if (err) {
- kvaser_usb_remove_interfaces(dev);
- return dev_err_probe(&intf->dev, err,
- "Cannot get capabilities\n");
+ dev_err_probe(&intf->dev, err,
+ "Cannot get capabilities\n");
+ goto remove_interfaces;
}
}
for (i = 0; i < dev->nchannels; i++) {
err = kvaser_usb_init_one(dev, i);
- if (err) {
- kvaser_usb_remove_interfaces(dev);
- return err;
- }
+ if (err)
+ goto remove_interfaces;
}
+ devlink_register(devlink);
return 0;
+
+remove_interfaces:
+ kvaser_usb_remove_interfaces(dev);
+free_devlink:
+ devlink_free(devlink);
+
+ return err;
}
static void kvaser_usb_disconnect(struct usb_interface *intf)
@@ -1000,6 +1020,8 @@ static void kvaser_usb_disconnect(struct usb_interface *intf)
return;
kvaser_usb_remove_interfaces(dev);
+ devlink_unregister(priv_to_devlink(dev));
+ devlink_free(priv_to_devlink(dev));
}
static struct usb_driver kvaser_usb_driver = {
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
new file mode 100644
index 000000000000..9a3a8966a0a1
--- /dev/null
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/* kvaser_usb devlink functions
+ *
+ * Copyright (C) 2025 KVASER AB, Sweden. All rights reserved.
+ */
+
+#include <net/devlink.h>
+
+const struct devlink_ops kvaser_usb_devlink_ops = {
+};
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 09/11] can: kvaser_usb: Expose device information via devlink info_get()
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (7 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 08/11] can: kvaser_usb: Add devlink support Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-25 3:23 ` Vincent Mailhol
2025-07-24 9:25 ` [PATCH v2 10/11] can: kvaser_usb: Add devlink port support Jimmy Assarsson
` (2 subsequent siblings)
11 siblings, 1 reply; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Expose device information via devlink info_get():
* Serial number
* Firmware version
* Hardware revision
* EAN (product number)
Example output:
$ devlink dev
usb/1-1.2:1.0
$ devlink dev info
usb/1-1.2:1.0:
driver kvaser_usb
serial_number 1020
versions:
fixed:
board.rev 1
board.id 7330130009653
running:
fw 3.22.527
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- Add two space indentation to terminal output.
Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#m31ee4aad13ee29d5559b56fdce842609ae4f67c5
.../can/usb/kvaser_usb/kvaser_usb_devlink.c | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
index 9a3a8966a0a1..3568485a3e84 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
@@ -6,5 +6,58 @@
#include <net/devlink.h>
+#include "kvaser_usb.h"
+
+#define KVASER_USB_EAN_MSB 0x00073301
+
+static int kvaser_usb_devlink_info_get(struct devlink *devlink,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct kvaser_usb *dev = devlink_priv(devlink);
+ char buf[] = "73301XXXXXXXXXX";
+ int ret;
+
+ if (dev->serial_number) {
+ snprintf(buf, sizeof(buf), "%u", dev->serial_number);
+ ret = devlink_info_serial_number_put(req, buf);
+ if (ret)
+ return ret;
+ }
+
+ if (dev->fw_version.major) {
+ snprintf(buf, sizeof(buf), "%u.%u.%u",
+ dev->fw_version.major,
+ dev->fw_version.minor,
+ dev->fw_version.build);
+ ret = devlink_info_version_running_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_FW,
+ buf);
+ if (ret)
+ return ret;
+ }
+
+ if (dev->hw_revision) {
+ snprintf(buf, sizeof(buf), "%u", dev->hw_revision);
+ ret = devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
+ buf);
+ if (ret)
+ return ret;
+ }
+
+ if (dev->ean[1] == KVASER_USB_EAN_MSB) {
+ snprintf(buf, sizeof(buf), "%x%08x", dev->ean[1], dev->ean[0]);
+ ret = devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
+ buf);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
const struct devlink_ops kvaser_usb_devlink_ops = {
+ .info_get = kvaser_usb_devlink_info_get,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 09/11] can: kvaser_usb: Expose device information via devlink info_get()
2025-07-24 9:25 ` [PATCH v2 09/11] can: kvaser_usb: Expose device information via devlink info_get() Jimmy Assarsson
@ 2025-07-25 3:23 ` Vincent Mailhol
0 siblings, 0 replies; 19+ messages in thread
From: Vincent Mailhol @ 2025-07-25 3:23 UTC (permalink / raw)
To: Jimmy Assarsson, linux-can
Cc: Jimmy Assarsson, Marc Kleine-Budde, netdev, Simon Horman
+CC: Simon
On 24/07/2025 at 18:25, Jimmy Assarsson wrote:
> Expose device information via devlink info_get():
> * Serial number
> * Firmware version
> * Hardware revision
> * EAN (product number)
>
> Example output:
> $ devlink dev
> usb/1-1.2:1.0
>
> $ devlink dev info
> usb/1-1.2:1.0:
> driver kvaser_usb
> serial_number 1020
> versions:
> fixed:
> board.rev 1
> board.id 7330130009653
> running:
> fw 3.22.527
>
> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
> ---
> Changes in v2:
> - Add two space indentation to terminal output.
> Suggested by Vincent Mailhol [1]
>
> [1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#m31ee4aad13ee29d5559b56fdce842609ae4f67c5
>
> .../can/usb/kvaser_usb/kvaser_usb_devlink.c | 53 +++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
> index 9a3a8966a0a1..3568485a3e84 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
> @@ -6,5 +6,58 @@
>
> #include <net/devlink.h>
>
> +#include "kvaser_usb.h"
I guess this has the same issue as the one pointed by Simon here:
https://lore.kernel.org/linux-can/20250724135224.GA1266901@horms.kernel.org/
> +#define KVASER_USB_EAN_MSB 0x00073301
> +
> +static int kvaser_usb_devlink_info_get(struct devlink *devlink,
> + struct devlink_info_req *req,
> + struct netlink_ext_ack *extack)
> +{
> + struct kvaser_usb *dev = devlink_priv(devlink);
> + char buf[] = "73301XXXXXXXXXX";
> + int ret;
> +
> + if (dev->serial_number) {
> + snprintf(buf, sizeof(buf), "%u", dev->serial_number);
> + ret = devlink_info_serial_number_put(req, buf);
> + if (ret)
> + return ret;
> + }
> +
> + if (dev->fw_version.major) {
> + snprintf(buf, sizeof(buf), "%u.%u.%u",
> + dev->fw_version.major,
> + dev->fw_version.minor,
> + dev->fw_version.build);
> + ret = devlink_info_version_running_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_FW,
> + buf);
> + if (ret)
> + return ret;
> + }
> +
> + if (dev->hw_revision) {
> + snprintf(buf, sizeof(buf), "%u", dev->hw_revision);
> + ret = devlink_info_version_fixed_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
> + buf);
> + if (ret)
> + return ret;
> + }
> +
> + if (dev->ean[1] == KVASER_USB_EAN_MSB) {
> + snprintf(buf, sizeof(buf), "%x%08x", dev->ean[1], dev->ean[0]);
> + ret = devlink_info_version_fixed_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
> + buf);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> const struct devlink_ops kvaser_usb_devlink_ops = {
> + .info_get = kvaser_usb_devlink_info_get,
> };
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 10/11] can: kvaser_usb: Add devlink port support
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (8 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 09/11] can: kvaser_usb: Expose device information via devlink info_get() Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-24 9:25 ` [PATCH v2 11/11] Documentation: devlink: add devlink documentation for the kvaser_usb driver Jimmy Assarsson
2025-07-25 3:29 ` [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Vincent Mailhol
11 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
Register each CAN channel of the device as an devlink physical port.
This makes it easier to get device information for a given network
interface (i.e. can2).
Example output:
$ devlink dev
usb/1-1.3:1.0
$ devlink port
usb/1-1.3:1.0/0: type eth netdev can0 flavour physical port 0 splittable false
usb/1-1.3:1.0/1: type eth netdev can1 flavour physical port 1 splittable false
$ devlink port show can1
usb/1-1.3:1.0/1: type eth netdev can1 flavour physical port 0 splittable false
$ devlink dev info
usb/1-1.3:1.0:
driver kvaser_usb
serial_number 1020
versions:
fixed:
board.rev 1
board.id 7330130009653
running:
fw 3.22.527
$ ethtool -i can1
driver: kvaser_usb
version: 6.12.10-arch1-1
firmware-version: 3.22.527
expansion-rom-version:
bus-info: 1-1.3:1.0
supports-statistics: no
supports-test: no
supports-eeprom-access: no
supports-register-dump: no
supports-priv-flags: no
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- Add two space indentation to terminal output.
Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/20250723083236.9-1-extja@kvaser.com/T/#m31ee4aad13ee29d5559b56fdce842609ae4f67c5
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 4 +++
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 15 ++++++++---
.../can/usb/kvaser_usb/kvaser_usb_devlink.c | 25 +++++++++++++++++++
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index d5f913ac9b44..46a1b6907a50 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -131,6 +131,7 @@ struct kvaser_usb {
struct kvaser_usb_net_priv {
struct can_priv can;
+ struct devlink_port devlink_port;
struct can_berr_counter bec;
/* subdriver-specific data */
@@ -229,6 +230,9 @@ extern const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops;
extern const struct devlink_ops kvaser_usb_devlink_ops;
+int kvaser_usb_devlink_port_register(struct kvaser_usb_net_priv *priv);
+void kvaser_usb_devlink_port_unregister(struct kvaser_usb_net_priv *priv);
+
void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv);
int kvaser_usb_recv_cmd(const struct kvaser_usb *dev, void *cmd, int len,
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index b9b2e120a5cd..90e77fa0ff4a 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -818,6 +818,7 @@ static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
if (ops->dev_remove_channel)
ops->dev_remove_channel(priv);
+ kvaser_usb_devlink_port_unregister(priv);
free_candev(priv->netdev);
}
}
@@ -891,20 +892,28 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
if (ops->dev_init_channel) {
err = ops->dev_init_channel(priv);
if (err)
- goto err;
+ goto candev_free;
+ }
+
+ err = kvaser_usb_devlink_port_register(priv);
+ if (err) {
+ dev_err(&dev->intf->dev, "Failed to register devlink port\n");
+ goto candev_free;
}
err = register_candev(netdev);
if (err) {
dev_err(&dev->intf->dev, "Failed to register CAN device\n");
- goto err;
+ goto unregister_devlink_port;
}
netdev_dbg(netdev, "device registered\n");
return 0;
-err:
+unregister_devlink_port:
+ kvaser_usb_devlink_port_unregister(priv);
+candev_free:
free_candev(netdev);
dev->nets[channel] = NULL;
return err;
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
index 3568485a3e84..3728e5d5793a 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_devlink.c
@@ -4,6 +4,7 @@
* Copyright (C) 2025 KVASER AB, Sweden. All rights reserved.
*/
+#include <linux/netdevice.h>
#include <net/devlink.h>
#include "kvaser_usb.h"
@@ -61,3 +62,27 @@ static int kvaser_usb_devlink_info_get(struct devlink *devlink,
const struct devlink_ops kvaser_usb_devlink_ops = {
.info_get = kvaser_usb_devlink_info_get,
};
+
+int kvaser_usb_devlink_port_register(struct kvaser_usb_net_priv *priv)
+{
+ int ret;
+ struct devlink_port_attrs attrs = {
+ .flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL,
+ .phys.port_number = priv->channel,
+ };
+ devlink_port_attrs_set(&priv->devlink_port, &attrs);
+
+ ret = devlink_port_register(priv_to_devlink(priv->dev),
+ &priv->devlink_port, priv->channel);
+ if (ret)
+ return ret;
+
+ SET_NETDEV_DEVLINK_PORT(priv->netdev, &priv->devlink_port);
+
+ return 0;
+}
+
+void kvaser_usb_devlink_port_unregister(struct kvaser_usb_net_priv *priv)
+{
+ devlink_port_unregister(&priv->devlink_port);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v2 11/11] Documentation: devlink: add devlink documentation for the kvaser_usb driver
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (9 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 10/11] can: kvaser_usb: Add devlink port support Jimmy Assarsson
@ 2025-07-24 9:25 ` Jimmy Assarsson
2025-07-25 3:27 ` Vincent Mailhol
2025-07-25 3:29 ` [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Vincent Mailhol
11 siblings, 1 reply; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-24 9:25 UTC (permalink / raw)
To: linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, Vincent Mailhol, netdev
List the version information reported by the kvaser_usb driver
through devlink.
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
- New in v2. Suggested by Vincent Mailhol [1]
[1] https://lore.kernel.org/linux-can/5cdca1d7-c875-40ee-b44d-51a161f42761@wanadoo.fr/T/#mb9ede2edcf5f7adcb76bc6331f5f27bafb79294f
---
Documentation/networking/devlink/index.rst | 1 +
.../networking/devlink/kvaser_usb.rst | 33 +++++++++++++++++++
2 files changed, 34 insertions(+)
create mode 100644 Documentation/networking/devlink/kvaser_usb.rst
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 8319f43b5933..26c28d0e8905 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -85,6 +85,7 @@ parameters, info versions, and other features it supports.
ionic
ice
ixgbe
+ kvaser_usb
mlx4
mlx5
mlxsw
diff --git a/Documentation/networking/devlink/kvaser_usb.rst b/Documentation/networking/devlink/kvaser_usb.rst
new file mode 100644
index 000000000000..403db3766cb4
--- /dev/null
+++ b/Documentation/networking/devlink/kvaser_usb.rst
@@ -0,0 +1,33 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========================
+kvaser_usb devlink support
+==========================
+
+This document describes the devlink features implemented by the
+``kvaser_usb`` device driver.
+
+Info versions
+=============
+
+The ``kvaser_usb`` driver reports the following versions
+
+.. list-table:: devlink info versions implemented
+ :widths: 5 5 90
+
+ * - Name
+ - Type
+ - Description
+ * - ``fw``
+ - running
+ - Version of the firmware running on the device. Also available
+ through ``ethtool -i`` as ``firmware-version``.
+ * - ``board.rev``
+ - fixed
+ - The device hardware revision.
+ * - ``board.id``
+ - fixed
+ - The device EAN (product number).
+ * - ``serial_number``
+ - fixed
+ - The device serial number.
--
2.49.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v2 11/11] Documentation: devlink: add devlink documentation for the kvaser_usb driver
2025-07-24 9:25 ` [PATCH v2 11/11] Documentation: devlink: add devlink documentation for the kvaser_usb driver Jimmy Assarsson
@ 2025-07-25 3:27 ` Vincent Mailhol
0 siblings, 0 replies; 19+ messages in thread
From: Vincent Mailhol @ 2025-07-25 3:27 UTC (permalink / raw)
To: Jimmy Assarsson, linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, netdev
On 24/07/2025 at 18:25, Jimmy Assarsson wrote:
> List the version information reported by the kvaser_usb driver
> through devlink.
>
> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
> ---
> Changes in v2:
> - New in v2. Suggested by Vincent Mailhol [1]
>
> [1] https://lore.kernel.org/linux-can/5cdca1d7-c875-40ee-b44d-51a161f42761@wanadoo.fr/T/#mb9ede2edcf5f7adcb76bc6331f5f27bafb79294f
> ---
> Documentation/networking/devlink/index.rst | 1 +
> .../networking/devlink/kvaser_usb.rst | 33 +++++++++++++++++++
> 2 files changed, 34 insertions(+)
> create mode 100644 Documentation/networking/devlink/kvaser_usb.rst
>
> diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
> index 8319f43b5933..26c28d0e8905 100644
> --- a/Documentation/networking/devlink/index.rst
> +++ b/Documentation/networking/devlink/index.rst
> @@ -85,6 +85,7 @@ parameters, info versions, and other features it supports.
> ionic
> ice
> ixgbe
> + kvaser_usb
This will create a tiny merge conflict when applying both the kvaser_pciefd and
this series.
I think it would be smoother if you rebase either of these on top of the other.
Just add a note in the cover letter of which one should be merged first.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces
2025-07-24 9:24 [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Jimmy Assarsson
` (10 preceding siblings ...)
2025-07-24 9:25 ` [PATCH v2 11/11] Documentation: devlink: add devlink documentation for the kvaser_usb driver Jimmy Assarsson
@ 2025-07-25 3:29 ` Vincent Mailhol
2025-07-25 12:45 ` Jimmy Assarsson
11 siblings, 1 reply; 19+ messages in thread
From: Vincent Mailhol @ 2025-07-25 3:29 UTC (permalink / raw)
To: Jimmy Assarsson, linux-can; +Cc: Jimmy Assarsson, Marc Kleine-Budde, netdev
On 24/07/2025 at 18:24, Jimmy Assarsson wrote:
> This patch series simplifies the process of identifying which network
> interface (can0..canX) corresponds to which physical CAN channel on
> Kvaser USB based CAN interfaces.
Same as for the kvaser_pciefd, there is a tiny transient issue on a missing
header include. The rest is OK, so, for the full series:
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces
2025-07-25 3:29 ` [PATCH v2 00/11] can: kvaser_usb: Simplify identification of physical CAN interfaces Vincent Mailhol
@ 2025-07-25 12:45 ` Jimmy Assarsson
0 siblings, 0 replies; 19+ messages in thread
From: Jimmy Assarsson @ 2025-07-25 12:45 UTC (permalink / raw)
To: Vincent Mailhol, Jimmy Assarsson, linux-can; +Cc: Marc Kleine-Budde, netdev
On 7/25/25 5:29 AM, Vincent Mailhol wrote:
> On 24/07/2025 at 18:24, Jimmy Assarsson wrote:
>> This patch series simplifies the process of identifying which network
>> interface (can0..canX) corresponds to which physical CAN channel on
>> Kvaser USB based CAN interfaces.
>
>
> Same as for the kvaser_pciefd, there is a tiny transient issue on a missing
> header include. The rest is OK, so, for the full series:
>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
>
>
> Yours sincerely,
> Vincent Mailhol
Think all issues should be resolved.
Thanks for reviewing!
Best regards,
jimmy
^ permalink raw reply [flat|nested] 19+ messages in thread