* [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements
@ 2022-10-10 18:52 Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion Jimmy Assarsson
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
Changes in v5:
- Split series [1], since v4 got rejected [2].
This part only contains non-critical fixes and improvements.
Note: This series depend on changes in [3]
[1]
https://lore.kernel.org/linux-can/20220903182344.139-1-extja@kvaser.com
[2]
https://lore.kernel.org/linux-can/20220920192708.jcvyph3ec7lscuqj@pengutronix.de
[3]
https://lore.kernel.org/linux-can/20221010150829.199676-1-extja@kvaser.com/T/#m77a503db16c46024601e0d41fed60a806b02c15f
Anssi Hannula (6):
can: kvaser_usb: Fix possible completions during init_completion
can: kvaser_usb_leaf: Set Warning state even without bus errors
can: kvaser_usb_leaf: Fix improved state not being reported
can: kvaser_usb_leaf: Fix wrong CAN state after stopping
can: kvaser_usb_leaf: Ignore stale bus-off after start
can: kvaser_usb_leaf: Fix bogus restart events
Jimmy Assarsson (5):
can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device
can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event
to {leaf,usbcan}_cmd_can_error_event
can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT
can: kvaser_usb: Add struct kvaser_usb_busparams
can: kvaser_usb: Compare requested bittiming parameters with actual
parameters in do_set_{,data}_bittiming
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 30 +-
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 115 ++++-
.../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 164 ++++--
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 468 ++++++++++++++++--
4 files changed, 682 insertions(+), 95 deletions(-)
--
2.38.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-26 8:14 ` Marc Kleine-Budde
2022-10-10 18:52 ` [PATCH can-next v5 02/11] can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device Jimmy Assarsson
` (9 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
kvaser_usb uses completions to signal when a response event is received
for outgoing commands.
However, it uses init_completion() to reinitialize the start_comp and
stop_comp completions before sending the start/stop commands.
In case the device sends the corresponding response just before the
actual command is sent, complete() may be called concurrently with
init_completion() which is not safe.
This might be triggerable even with a properly functioning device by
stopping the interface (CMD_STOP_CHIP) just after it goes bus-off (which
also causes the driver to send CMD_STOP_CHIP when restart-ms is off),
but that was not tested.
Fix the issue by using reinit_completion() instead.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 4 ++--
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
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 7b52fda73d82..66f672ea631b 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -1875,7 +1875,7 @@ static int kvaser_usb_hydra_start_chip(struct kvaser_usb_net_priv *priv)
{
int err;
- init_completion(&priv->start_comp);
+ reinit_completion(&priv->start_comp);
err = kvaser_usb_hydra_send_simple_cmd(priv->dev, CMD_START_CHIP_REQ,
priv->channel);
@@ -1893,7 +1893,7 @@ static int kvaser_usb_hydra_stop_chip(struct kvaser_usb_net_priv *priv)
{
int err;
- init_completion(&priv->stop_comp);
+ reinit_completion(&priv->stop_comp);
/* Make sure we do not report invalid BUS_OFF from CMD_CHIP_STATE_EVENT
* see comment in kvaser_usb_hydra_update_state()
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 50f2ac8319ff..19958037720f 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -1320,7 +1320,7 @@ static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
{
int err;
- init_completion(&priv->start_comp);
+ reinit_completion(&priv->start_comp);
err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_START_CHIP,
priv->channel);
@@ -1338,7 +1338,7 @@ static int kvaser_usb_leaf_stop_chip(struct kvaser_usb_net_priv *priv)
{
int err;
- init_completion(&priv->stop_comp);
+ reinit_completion(&priv->stop_comp);
err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_STOP_CHIP,
priv->channel);
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 02/11] can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 03/11] can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event Jimmy Assarsson
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
Use the CMD_GET_CAPABILITIES_REQ command to query the device for certain
capabilities. We are only interested in LISTENONLY mode and wither the
device reports CAN error counters.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Reported-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- Remove commit message about removal of hard coded capabilities.
- Add Tested-by Anssi Hannula
Changes in v3
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Re-add hard coded capabilities for Leaf M32C devices, to fix regression
found by Anssi Hannula in v2 [1].
Changes in v2:
- New in v2. Replaces [PATCH 04/12] can: kvaser_usb: Mark Mini PCIe 2xHS as supporting
error counters
- Fixed Anssi's comments; https://lore.kernel.org/linux-can/9742e7ab-3650-74d8-5a44-136555788c08@bitwise.fi/
[1] https://lore.kernel.org/linux-can/b25bc059-d776-146d-0b3c-41aecf4bd9f8@bitwise.fi/
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 144 +++++++++++++++++-
1 file changed, 143 insertions(+), 1 deletion(-)
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 19958037720f..33ff62cd1729 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -74,6 +74,8 @@
#define CMD_TX_ACKNOWLEDGE 50
#define CMD_CAN_ERROR_EVENT 51
#define CMD_FLUSH_QUEUE_REPLY 68
+#define CMD_GET_CAPABILITIES_REQ 95
+#define CMD_GET_CAPABILITIES_RESP 96
#define CMD_LEAF_LOG_MESSAGE 106
@@ -83,6 +85,8 @@
#define KVASER_USB_LEAF_SWOPTION_FREQ_32_MHZ_CLK BIT(5)
#define KVASER_USB_LEAF_SWOPTION_FREQ_24_MHZ_CLK BIT(6)
+#define KVASER_USB_LEAF_SWOPTION_EXT_CAP BIT(12)
+
/* error factors */
#define M16C_EF_ACKE BIT(0)
#define M16C_EF_CRCE BIT(1)
@@ -278,6 +282,28 @@ struct leaf_cmd_log_message {
u8 data[8];
} __packed;
+/* Sub commands for cap_req and cap_res */
+#define KVASER_USB_LEAF_CAP_CMD_LISTEN_MODE 0x02
+#define KVASER_USB_LEAF_CAP_CMD_ERR_REPORT 0x05
+struct kvaser_cmd_cap_req {
+ __le16 padding0;
+ __le16 cap_cmd;
+ __le16 padding1;
+ __le16 channel;
+} __packed;
+
+/* Status codes for cap_res */
+#define KVASER_USB_LEAF_CAP_STAT_OK 0x00
+#define KVASER_USB_LEAF_CAP_STAT_NOT_IMPL 0x01
+#define KVASER_USB_LEAF_CAP_STAT_UNAVAIL 0x02
+struct kvaser_cmd_cap_res {
+ __le16 padding;
+ __le16 cap_cmd;
+ __le16 status;
+ __le32 mask;
+ __le32 value;
+} __packed;
+
struct kvaser_cmd {
u8 len;
u8 id;
@@ -295,6 +321,8 @@ struct kvaser_cmd {
struct leaf_cmd_chip_state_event chip_state_event;
struct leaf_cmd_error_event error_event;
struct leaf_cmd_log_message log_message;
+ struct kvaser_cmd_cap_req cap_req;
+ struct kvaser_cmd_cap_res cap_res;
} __packed leaf;
union {
@@ -324,6 +352,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = {
[CMD_LEAF_LOG_MESSAGE] = kvaser_fsize(u.leaf.log_message),
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.leaf.chip_state_event),
[CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event),
+ [CMD_GET_CAPABILITIES_RESP] = kvaser_fsize(u.leaf.cap_res),
/* ignored events: */
[CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY,
};
@@ -606,6 +635,9 @@ static void kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
dev->fw_version = le32_to_cpu(softinfo->fw_version);
dev->max_tx_urbs = le16_to_cpu(softinfo->max_outstanding_tx);
+ if (sw_options & KVASER_USB_LEAF_SWOPTION_EXT_CAP)
+ dev->card_data.capabilities |= KVASER_USB_CAP_EXT_CAP;
+
if (dev->driver_info->quirks & KVASER_USB_QUIRK_IGNORE_CLK_FREQ) {
/* Firmware expects bittiming parameters calculated for 16MHz
* clock, regardless of the actual clock
@@ -693,6 +725,116 @@ static int kvaser_usb_leaf_get_card_info(struct kvaser_usb *dev)
return 0;
}
+static int kvaser_usb_leaf_get_single_capability(struct kvaser_usb *dev,
+ u16 cap_cmd_req, u16 *status)
+{
+ struct kvaser_usb_dev_card_data *card_data = &dev->card_data;
+ struct kvaser_cmd *cmd;
+ u32 value = 0;
+ u32 mask = 0;
+ u16 cap_cmd_res;
+ int err;
+ int i;
+
+ cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ cmd->id = CMD_GET_CAPABILITIES_REQ;
+ cmd->u.leaf.cap_req.cap_cmd = cpu_to_le16(cap_cmd_req);
+ cmd->len = CMD_HEADER_LEN + sizeof(struct kvaser_cmd_cap_req);
+
+ err = kvaser_usb_send_cmd(dev, cmd, cmd->len);
+ if (err)
+ goto end;
+
+ err = kvaser_usb_leaf_wait_cmd(dev, CMD_GET_CAPABILITIES_RESP, cmd);
+ if (err)
+ goto end;
+
+ *status = le16_to_cpu(cmd->u.leaf.cap_res.status);
+
+ if (*status != KVASER_USB_LEAF_CAP_STAT_OK)
+ goto end;
+
+ cap_cmd_res = le16_to_cpu(cmd->u.leaf.cap_res.cap_cmd);
+ switch (cap_cmd_res) {
+ case KVASER_USB_LEAF_CAP_CMD_LISTEN_MODE:
+ case KVASER_USB_LEAF_CAP_CMD_ERR_REPORT:
+ value = le32_to_cpu(cmd->u.leaf.cap_res.value);
+ mask = le32_to_cpu(cmd->u.leaf.cap_res.mask);
+ break;
+ default:
+ dev_warn(&dev->intf->dev, "Unknown capability command %u\n",
+ cap_cmd_res);
+ break;
+ }
+
+ for (i = 0; i < dev->nchannels; i++) {
+ if (BIT(i) & (value & mask)) {
+ switch (cap_cmd_res) {
+ case KVASER_USB_LEAF_CAP_CMD_LISTEN_MODE:
+ card_data->ctrlmode_supported |=
+ CAN_CTRLMODE_LISTENONLY;
+ break;
+ case KVASER_USB_LEAF_CAP_CMD_ERR_REPORT:
+ card_data->capabilities |=
+ KVASER_USB_CAP_BERR_CAP;
+ break;
+ }
+ }
+ }
+
+end:
+ kfree(cmd);
+
+ return err;
+}
+
+static int kvaser_usb_leaf_get_capabilities_leaf(struct kvaser_usb *dev)
+{
+ int err;
+ u16 status;
+
+ if (!(dev->card_data.capabilities & KVASER_USB_CAP_EXT_CAP)) {
+ dev_info(&dev->intf->dev,
+ "No extended capability support. Upgrade device firmware.\n");
+ return 0;
+ }
+
+ err = kvaser_usb_leaf_get_single_capability(dev,
+ KVASER_USB_LEAF_CAP_CMD_LISTEN_MODE,
+ &status);
+ if (err)
+ return err;
+ if (status)
+ dev_info(&dev->intf->dev,
+ "KVASER_USB_LEAF_CAP_CMD_LISTEN_MODE failed %u\n",
+ status);
+
+ err = kvaser_usb_leaf_get_single_capability(dev,
+ KVASER_USB_LEAF_CAP_CMD_ERR_REPORT,
+ &status);
+ if (err)
+ return err;
+ if (status)
+ dev_info(&dev->intf->dev,
+ "KVASER_USB_LEAF_CAP_CMD_ERR_REPORT failed %u\n",
+ status);
+
+ return 0;
+}
+
+static int kvaser_usb_leaf_get_capabilities(struct kvaser_usb *dev)
+{
+ int err = 0;
+
+ if (dev->driver_info->family == KVASER_LEAF)
+ err = kvaser_usb_leaf_get_capabilities_leaf(dev);
+
+ return err;
+}
+
static void kvaser_usb_leaf_tx_acknowledge(const struct kvaser_usb *dev,
const struct kvaser_cmd *cmd)
{
@@ -1486,7 +1628,7 @@ const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops = {
.dev_get_software_info = kvaser_usb_leaf_get_software_info,
.dev_get_software_details = NULL,
.dev_get_card_info = kvaser_usb_leaf_get_card_info,
- .dev_get_capabilities = NULL,
+ .dev_get_capabilities = kvaser_usb_leaf_get_capabilities,
.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.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 03/11] can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 02/11] can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 04/11] can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT Jimmy Assarsson
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
Prepare for handling CMD_ERROR_EVENT. Rename struct
{leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Reported-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add Tested-by Anssi Hannula
Changes in v2:
- New in v2.
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 38 +++++++++----------
1 file changed, 19 insertions(+), 19 deletions(-)
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 33ff62cd1729..a1d4ac8d4a08 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -234,7 +234,7 @@ struct kvaser_cmd_tx_acknowledge_header {
u8 tid;
} __packed;
-struct leaf_cmd_error_event {
+struct leaf_cmd_can_error_event {
u8 tid;
u8 flags;
__le16 time[3];
@@ -246,7 +246,7 @@ struct leaf_cmd_error_event {
u8 error_factor;
} __packed;
-struct usbcan_cmd_error_event {
+struct usbcan_cmd_can_error_event {
u8 tid;
u8 padding;
u8 tx_errors_count_ch0;
@@ -319,7 +319,7 @@ struct kvaser_cmd {
struct leaf_cmd_softinfo softinfo;
struct leaf_cmd_rx_can rx_can;
struct leaf_cmd_chip_state_event chip_state_event;
- struct leaf_cmd_error_event error_event;
+ struct leaf_cmd_can_error_event can_error_event;
struct leaf_cmd_log_message log_message;
struct kvaser_cmd_cap_req cap_req;
struct kvaser_cmd_cap_res cap_res;
@@ -329,7 +329,7 @@ struct kvaser_cmd {
struct usbcan_cmd_softinfo softinfo;
struct usbcan_cmd_rx_can rx_can;
struct usbcan_cmd_chip_state_event chip_state_event;
- struct usbcan_cmd_error_event error_event;
+ struct usbcan_cmd_can_error_event can_error_event;
} __packed usbcan;
struct kvaser_cmd_tx_can tx_can;
@@ -351,7 +351,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = {
[CMD_RX_EXT_MESSAGE] = kvaser_fsize(u.leaf.rx_can),
[CMD_LEAF_LOG_MESSAGE] = kvaser_fsize(u.leaf.log_message),
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.leaf.chip_state_event),
- [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event),
+ [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.can_error_event),
[CMD_GET_CAPABILITIES_RESP] = kvaser_fsize(u.leaf.cap_res),
/* ignored events: */
[CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY,
@@ -366,7 +366,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_usbcan[] = {
[CMD_RX_STD_MESSAGE] = kvaser_fsize(u.usbcan.rx_can),
[CMD_RX_EXT_MESSAGE] = kvaser_fsize(u.usbcan.rx_can),
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.usbcan.chip_state_event),
- [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.usbcan.error_event),
+ [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.usbcan.can_error_event),
/* ignored events: */
[CMD_USBCAN_CLOCK_OVERFLOW_EVENT] = CMD_SIZE_ANY,
};
@@ -1132,11 +1132,11 @@ static void kvaser_usb_leaf_usbcan_rx_error(const struct kvaser_usb *dev,
case CMD_CAN_ERROR_EVENT:
es.channel = 0;
- es.status = cmd->u.usbcan.error_event.status_ch0;
- es.txerr = cmd->u.usbcan.error_event.tx_errors_count_ch0;
- es.rxerr = cmd->u.usbcan.error_event.rx_errors_count_ch0;
+ es.status = cmd->u.usbcan.can_error_event.status_ch0;
+ es.txerr = cmd->u.usbcan.can_error_event.tx_errors_count_ch0;
+ es.rxerr = cmd->u.usbcan.can_error_event.rx_errors_count_ch0;
es.usbcan.other_ch_status =
- cmd->u.usbcan.error_event.status_ch1;
+ cmd->u.usbcan.can_error_event.status_ch1;
kvaser_usb_leaf_usbcan_conditionally_rx_error(dev, &es);
/* The USBCAN firmware supports up to 2 channels.
@@ -1144,13 +1144,13 @@ static void kvaser_usb_leaf_usbcan_rx_error(const struct kvaser_usb *dev,
*/
if (dev->nchannels == MAX_USBCAN_NET_DEVICES) {
es.channel = 1;
- es.status = cmd->u.usbcan.error_event.status_ch1;
+ es.status = cmd->u.usbcan.can_error_event.status_ch1;
es.txerr =
- cmd->u.usbcan.error_event.tx_errors_count_ch1;
+ cmd->u.usbcan.can_error_event.tx_errors_count_ch1;
es.rxerr =
- cmd->u.usbcan.error_event.rx_errors_count_ch1;
+ cmd->u.usbcan.can_error_event.rx_errors_count_ch1;
es.usbcan.other_ch_status =
- cmd->u.usbcan.error_event.status_ch0;
+ cmd->u.usbcan.can_error_event.status_ch0;
kvaser_usb_leaf_usbcan_conditionally_rx_error(dev, &es);
}
break;
@@ -1167,11 +1167,11 @@ static void kvaser_usb_leaf_leaf_rx_error(const struct kvaser_usb *dev,
switch (cmd->id) {
case CMD_CAN_ERROR_EVENT:
- es.channel = cmd->u.leaf.error_event.channel;
- es.status = cmd->u.leaf.error_event.status;
- es.txerr = cmd->u.leaf.error_event.tx_errors_count;
- es.rxerr = cmd->u.leaf.error_event.rx_errors_count;
- es.leaf.error_factor = cmd->u.leaf.error_event.error_factor;
+ es.channel = cmd->u.leaf.can_error_event.channel;
+ es.status = cmd->u.leaf.can_error_event.status;
+ es.txerr = cmd->u.leaf.can_error_event.tx_errors_count;
+ es.rxerr = cmd->u.leaf.can_error_event.rx_errors_count;
+ es.leaf.error_factor = cmd->u.leaf.can_error_event.error_factor;
break;
case CMD_LEAF_LOG_MESSAGE:
es.channel = cmd->u.leaf.log_message.channel;
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 04/11] can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (2 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 03/11] can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 05/11] can: kvaser_usb_leaf: Set Warning state even without bus errors Jimmy Assarsson
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
The device will send an error event command, to indicate certain errors.
This indicates a misbehaving driver, and should never occur.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Co-developed-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add Tested-by Anssi Hannula
Changes in v2:
- New in v2. Replaces parts of [PATCH 09/12] can: kvaser_usb_leaf: Fix silently failing bus params
setup
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 99 +++++++++++++++++++
1 file changed, 99 insertions(+)
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 a1d4ac8d4a08..5d96ab85d04c 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -70,6 +70,7 @@
#define CMD_GET_CARD_INFO_REPLY 35
#define CMD_GET_SOFTWARE_INFO 38
#define CMD_GET_SOFTWARE_INFO_REPLY 39
+#define CMD_ERROR_EVENT 45
#define CMD_FLUSH_QUEUE 48
#define CMD_TX_ACKNOWLEDGE 50
#define CMD_CAN_ERROR_EVENT 51
@@ -258,6 +259,28 @@ struct usbcan_cmd_can_error_event {
__le16 time;
} __packed;
+/* CMD_ERROR_EVENT error codes */
+#define KVASER_USB_LEAF_ERROR_EVENT_TX_QUEUE_FULL 0x8
+#define KVASER_USB_LEAF_ERROR_EVENT_PARAM 0x9
+
+struct leaf_cmd_error_event {
+ u8 tid;
+ u8 error_code;
+ __le16 timestamp[3];
+ __le16 padding;
+ __le16 info1;
+ __le16 info2;
+} __packed;
+
+struct usbcan_cmd_error_event {
+ u8 tid;
+ u8 error_code;
+ __le16 info1;
+ __le16 info2;
+ __le16 timestamp;
+ __le16 padding;
+} __packed;
+
struct kvaser_cmd_ctrl_mode {
u8 tid;
u8 channel;
@@ -321,6 +344,7 @@ struct kvaser_cmd {
struct leaf_cmd_chip_state_event chip_state_event;
struct leaf_cmd_can_error_event can_error_event;
struct leaf_cmd_log_message log_message;
+ struct leaf_cmd_error_event error_event;
struct kvaser_cmd_cap_req cap_req;
struct kvaser_cmd_cap_res cap_res;
} __packed leaf;
@@ -330,6 +354,7 @@ struct kvaser_cmd {
struct usbcan_cmd_rx_can rx_can;
struct usbcan_cmd_chip_state_event chip_state_event;
struct usbcan_cmd_can_error_event can_error_event;
+ struct usbcan_cmd_error_event error_event;
} __packed usbcan;
struct kvaser_cmd_tx_can tx_can;
@@ -353,6 +378,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = {
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.leaf.chip_state_event),
[CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.can_error_event),
[CMD_GET_CAPABILITIES_RESP] = kvaser_fsize(u.leaf.cap_res),
+ [CMD_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event),
/* ignored events: */
[CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY,
};
@@ -367,6 +393,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_usbcan[] = {
[CMD_RX_EXT_MESSAGE] = kvaser_fsize(u.usbcan.rx_can),
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.usbcan.chip_state_event),
[CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.usbcan.can_error_event),
+ [CMD_ERROR_EVENT] = kvaser_fsize(u.usbcan.error_event),
/* ignored events: */
[CMD_USBCAN_CLOCK_OVERFLOW_EVENT] = CMD_SIZE_ANY,
};
@@ -1304,6 +1331,74 @@ static void kvaser_usb_leaf_rx_can_msg(const struct kvaser_usb *dev,
netif_rx(skb);
}
+static void kvaser_usb_leaf_error_event_parameter(const struct kvaser_usb *dev,
+ const struct kvaser_cmd *cmd)
+{
+ u16 info1 = 0;
+
+ switch (dev->driver_info->family) {
+ case KVASER_LEAF:
+ info1 = le16_to_cpu(cmd->u.leaf.error_event.info1);
+ break;
+ case KVASER_USBCAN:
+ info1 = le16_to_cpu(cmd->u.usbcan.error_event.info1);
+ break;
+ }
+
+ /* info1 will contain the offending cmd_no */
+ switch (info1) {
+ case CMD_SET_CTRL_MODE:
+ dev_warn(&dev->intf->dev,
+ "CMD_SET_CTRL_MODE error in parameter\n");
+ break;
+
+ case CMD_SET_BUS_PARAMS:
+ dev_warn(&dev->intf->dev,
+ "CMD_SET_BUS_PARAMS error in parameter\n");
+ break;
+
+ default:
+ dev_warn(&dev->intf->dev,
+ "Unhandled parameter error event cmd_no (%u)\n",
+ info1);
+ break;
+ }
+}
+
+static void kvaser_usb_leaf_error_event(const struct kvaser_usb *dev,
+ const struct kvaser_cmd *cmd)
+{
+ u8 error_code = 0;
+
+ switch (dev->driver_info->family) {
+ case KVASER_LEAF:
+ error_code = cmd->u.leaf.error_event.error_code;
+ break;
+ case KVASER_USBCAN:
+ error_code = cmd->u.usbcan.error_event.error_code;
+ break;
+ }
+
+ switch (error_code) {
+ case KVASER_USB_LEAF_ERROR_EVENT_TX_QUEUE_FULL:
+ /* Received additional CAN message, when firmware TX queue is
+ * already full. Something is wrong with the driver.
+ * This should never happen!
+ */
+ dev_err(&dev->intf->dev,
+ "Received error event TX_QUEUE_FULL\n");
+ break;
+ case KVASER_USB_LEAF_ERROR_EVENT_PARAM:
+ kvaser_usb_leaf_error_event_parameter(dev, cmd);
+ break;
+
+ default:
+ dev_warn(&dev->intf->dev,
+ "Unhandled error event (%d)\n", error_code);
+ break;
+ }
+}
+
static void kvaser_usb_leaf_start_chip_reply(const struct kvaser_usb *dev,
const struct kvaser_cmd *cmd)
{
@@ -1382,6 +1477,10 @@ static void kvaser_usb_leaf_handle_command(const struct kvaser_usb *dev,
kvaser_usb_leaf_tx_acknowledge(dev, cmd);
break;
+ case CMD_ERROR_EVENT:
+ kvaser_usb_leaf_error_event(dev, cmd);
+ break;
+
/* Ignored commands */
case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
if (dev->driver_info->family != KVASER_USBCAN)
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 05/11] can: kvaser_usb_leaf: Set Warning state even without bus errors
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (3 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 04/11] can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 06/11] can: kvaser_usb_leaf: Fix improved state not being reported Jimmy Assarsson
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
kvaser_usb_leaf_rx_error_update_can_state() sets error state according
to error counters when the hardware does not indicate a specific state
directly.
However, this is currently gated behind a check for
M16C_STATE_BUS_ERROR which does not always seem to be set when error
counters are increasing, and may not be set when error counters are
decreasing.
This causes the CAN_STATE_ERROR_WARNING state to not be set in some
cases even when appropriate.
Change the code to set error state from counters even without
M16C_STATE_BUS_ERROR.
The Error-Passive case seems superfluous as it is already set via
M16C_STATE_BUS_PASSIVE flag above, but it is kept for now.
Tested with 0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 20 ++++++++-----------
1 file changed, 8 insertions(+), 12 deletions(-)
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 5d96ab85d04c..a6a26085bc15 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -961,20 +961,16 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
new_state = CAN_STATE_BUS_OFF;
} else if (es->status & M16C_STATE_BUS_PASSIVE) {
new_state = CAN_STATE_ERROR_PASSIVE;
- } else if (es->status & M16C_STATE_BUS_ERROR) {
+ } else if ((es->status & M16C_STATE_BUS_ERROR) &&
+ cur_state >= CAN_STATE_BUS_OFF) {
/* Guard against spurious error events after a busoff */
- if (cur_state < CAN_STATE_BUS_OFF) {
- if (es->txerr >= 128 || es->rxerr >= 128)
- new_state = CAN_STATE_ERROR_PASSIVE;
- else if (es->txerr >= 96 || es->rxerr >= 96)
- new_state = CAN_STATE_ERROR_WARNING;
- else if (cur_state > CAN_STATE_ERROR_ACTIVE)
- new_state = CAN_STATE_ERROR_ACTIVE;
- }
- }
-
- if (!es->status)
+ } else if (es->txerr >= 128 || es->rxerr >= 128) {
+ new_state = CAN_STATE_ERROR_PASSIVE;
+ } else if (es->txerr >= 96 || es->rxerr >= 96) {
+ new_state = CAN_STATE_ERROR_WARNING;
+ } else {
new_state = CAN_STATE_ERROR_ACTIVE;
+ }
if (new_state != cur_state) {
tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 06/11] can: kvaser_usb_leaf: Fix improved state not being reported
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (4 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 05/11] can: kvaser_usb_leaf: Set Warning state even without bus errors Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 07/11] can: kvaser_usb_leaf: Fix wrong CAN state after stopping Jimmy Assarsson
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
The tested 0bfd:0017 Kvaser Memorator Professional HS/HS FW 2.0.50 and
0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778 do not seem to send
any unsolicited events when error counters decrease or when the device
transitions from ERROR_PASSIVE to ERROR_ACTIVE (or WARNING).
This causes the interface to e.g. indefinitely stay in the ERROR_PASSIVE
state.
Fix that by asking for chip state (inc. counters) event every 0.5 secs
when error counters are non-zero.
Since there are non-error-counter devices, also always poll in
ERROR_PASSIVE even if the counters show zero.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
- Rephrase commit message regarding non-error-counter devices.
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 7 +++
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 19 +++++-
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 58 +++++++++++++++++++
3 files changed, 81 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 f6c0938027ec..d9c5dd5da908 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -104,6 +104,9 @@ struct kvaser_usb_net_priv {
struct can_priv can;
struct can_berr_counter bec;
+ /* subdriver-specific data */
+ void *sub_priv;
+
struct kvaser_usb *dev;
struct net_device *netdev;
int channel;
@@ -125,6 +128,8 @@ struct kvaser_usb_net_priv {
*
* @dev_setup_endpoints: setup USB in and out endpoints
* @dev_init_card: initialize card
+ * @dev_init_channel: initialize channel
+ * @dev_remove_channel: uninitialize channel
* @dev_get_software_info: get software info
* @dev_get_software_details: get software details
* @dev_get_card_info: get card info
@@ -146,6 +151,8 @@ struct kvaser_usb_dev_ops {
struct can_berr_counter *bec);
int (*dev_setup_endpoints)(struct kvaser_usb *dev);
int (*dev_init_card)(struct kvaser_usb *dev);
+ int (*dev_init_channel)(struct kvaser_usb_net_priv *priv);
+ void (*dev_remove_channel)(struct kvaser_usb_net_priv *priv);
int (*dev_get_software_info)(struct kvaser_usb *dev);
int (*dev_get_software_details)(struct kvaser_usb *dev);
int (*dev_get_card_info)(struct kvaser_usb *dev);
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 e91648ed7386..19df05887166 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -684,6 +684,7 @@ static const struct ethtool_ops kvaser_usb_ethtool_ops_hwts = {
static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
{
+ const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
int i;
for (i = 0; i < dev->nchannels; i++) {
@@ -699,6 +700,9 @@ static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
if (!dev->nets[i])
continue;
+ if (ops->dev_remove_channel)
+ ops->dev_remove_channel(dev->nets[i]);
+
free_candev(dev->nets[i]->netdev);
}
}
@@ -772,17 +776,26 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
dev->nets[channel] = priv;
+ if (ops->dev_init_channel) {
+ err = ops->dev_init_channel(priv);
+ if (err)
+ goto err;
+ }
+
err = register_candev(netdev);
if (err) {
dev_err(&dev->intf->dev, "Failed to register CAN device\n");
- free_candev(netdev);
- dev->nets[channel] = NULL;
- return err;
+ goto err;
}
netdev_dbg(netdev, "device registered\n");
return 0;
+
+err:
+ free_candev(netdev);
+ dev->nets[channel] = NULL;
+ return err;
}
static int kvaser_usb_probe(struct usb_interface *intf,
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 a6a26085bc15..993fcc19637d 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -21,6 +21,7 @@
#include <linux/types.h>
#include <linux/units.h>
#include <linux/usb.h>
+#include <linux/workqueue.h>
#include <linux/can.h>
#include <linux/can/dev.h>
@@ -56,6 +57,7 @@
#define CMD_RX_EXT_MESSAGE 14
#define CMD_TX_EXT_MESSAGE 15
#define CMD_SET_BUS_PARAMS 16
+#define CMD_GET_CHIP_STATE 19
#define CMD_CHIP_STATE_EVENT 20
#define CMD_SET_CTRL_MODE 21
#define CMD_RESET_CHIP 24
@@ -421,6 +423,12 @@ struct kvaser_usb_err_summary {
};
};
+struct kvaser_usb_net_leaf_priv {
+ struct kvaser_usb_net_priv *net;
+
+ struct delayed_work chip_state_req_work;
+};
+
static const struct can_bittiming_const kvaser_usb_leaf_m16c_bittiming_const = {
.name = "kvaser_usb_ucii",
.tseg1_min = 4,
@@ -943,6 +951,16 @@ static int kvaser_usb_leaf_simple_cmd_async(struct kvaser_usb_net_priv *priv,
return err;
}
+static void kvaser_usb_leaf_chip_state_req_work(struct work_struct *work)
+{
+ struct kvaser_usb_net_leaf_priv *leaf =
+ container_of(work, struct kvaser_usb_net_leaf_priv,
+ chip_state_req_work.work);
+ struct kvaser_usb_net_priv *priv = leaf->net;
+
+ kvaser_usb_leaf_simple_cmd_async(priv, CMD_GET_CHIP_STATE);
+}
+
static void
kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
const struct kvaser_usb_err_summary *es,
@@ -1014,6 +1032,7 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
struct sk_buff *skb;
struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
+ struct kvaser_usb_net_leaf_priv *leaf;
enum can_state old_state, new_state;
if (es->channel >= dev->nchannels) {
@@ -1023,6 +1042,7 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
}
priv = dev->nets[es->channel];
+ leaf = priv->sub_priv;
stats = &priv->netdev->stats;
/* Update all of the CAN interface's state and error counters before
@@ -1039,6 +1059,14 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
kvaser_usb_leaf_rx_error_update_can_state(priv, es, &tmp_cf);
new_state = priv->can.state;
+ /* If there are errors, request status updates periodically as we do
+ * not get automatic notifications of improved state.
+ */
+ if (new_state < CAN_STATE_BUS_OFF &&
+ (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE))
+ schedule_delayed_work(&leaf->chip_state_req_work,
+ msecs_to_jiffies(500));
+
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -1573,10 +1601,13 @@ static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
static int kvaser_usb_leaf_stop_chip(struct kvaser_usb_net_priv *priv)
{
+ struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
int err;
reinit_completion(&priv->stop_comp);
+ cancel_delayed_work(&leaf->chip_state_req_work);
+
err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_STOP_CHIP,
priv->channel);
if (err)
@@ -1623,6 +1654,31 @@ static int kvaser_usb_leaf_init_card(struct kvaser_usb *dev)
return 0;
}
+static int kvaser_usb_leaf_init_channel(struct kvaser_usb_net_priv *priv)
+{
+ struct kvaser_usb_net_leaf_priv *leaf;
+
+ leaf = devm_kzalloc(&priv->dev->intf->dev, sizeof(*leaf), GFP_KERNEL);
+ if (!leaf)
+ return -ENOMEM;
+
+ leaf->net = priv;
+ INIT_DELAYED_WORK(&leaf->chip_state_req_work,
+ kvaser_usb_leaf_chip_state_req_work);
+
+ priv->sub_priv = leaf;
+
+ return 0;
+}
+
+static void kvaser_usb_leaf_remove_channel(struct kvaser_usb_net_priv *priv)
+{
+ struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
+
+ if (leaf)
+ cancel_delayed_work_sync(&leaf->chip_state_req_work);
+}
+
static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
{
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
@@ -1720,6 +1776,8 @@ const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops = {
.dev_get_berr_counter = kvaser_usb_leaf_get_berr_counter,
.dev_setup_endpoints = kvaser_usb_leaf_setup_endpoints,
.dev_init_card = kvaser_usb_leaf_init_card,
+ .dev_init_channel = kvaser_usb_leaf_init_channel,
+ .dev_remove_channel = kvaser_usb_leaf_remove_channel,
.dev_get_software_info = kvaser_usb_leaf_get_software_info,
.dev_get_software_details = NULL,
.dev_get_card_info = kvaser_usb_leaf_get_card_info,
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 07/11] can: kvaser_usb_leaf: Fix wrong CAN state after stopping
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (5 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 06/11] can: kvaser_usb_leaf: Fix improved state not being reported Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 08/11] can: kvaser_usb_leaf: Ignore stale bus-off after start Jimmy Assarsson
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778 sends a
CMD_CHIP_STATE_EVENT indicating bus-off after stopping the device,
causing a stopped device to appear as CAN_STATE_BUS_OFF instead of
CAN_STATE_STOPPED.
Fix that by not handling error events on stopped devices.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 4 ++++
1 file changed, 4 insertions(+)
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 993fcc19637d..4f9c76f4d0da 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -1045,6 +1045,10 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
leaf = priv->sub_priv;
stats = &priv->netdev->stats;
+ /* Ignore e.g. state change to bus-off reported just after stopping */
+ if (!netif_running(priv->netdev))
+ return;
+
/* Update all of the CAN interface's state and error counters before
* trying any memory allocation that can actually fail with -ENOMEM.
*
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 08/11] can: kvaser_usb_leaf: Ignore stale bus-off after start
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (6 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 07/11] can: kvaser_usb_leaf: Fix wrong CAN state after stopping Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 09/11] can: kvaser_usb_leaf: Fix bogus restart events Jimmy Assarsson
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
With 0bfd:0124 Kvaser Mini PCI Express 2xHS FW 4.18.778 it was observed
that if the device was bus-off when stopped, at next start (either via
interface down/up or manual bus-off restart) the initial
CMD_CHIP_STATE_EVENT received just after CMD_START_CHIP_REPLY will have
the M16C_STATE_BUS_OFF bit still set, causing the interface to
immediately go bus-off again.
The bit seems to internally clear quickly afterwards but we do not get
another CMD_CHIP_STATE_EVENT.
Fix the issue by ignoring any initial bus-off state until we see at
least one bus-on state. Also, poll the state periodically until that
occurs.
It is possible we lose one actual immediately occurring bus-off event
here in which case the HW will auto-recover and we see the recovery
event. We will then catch the next bus-off event, if any.
This issue did not reproduce with 0bfd:0017 Kvaser Memorator
Professional HS/HS FW 2.0.50.
Fixes: 71873a9b38d1 ("can: kvaser_usb: Add support for more Kvaser Leaf v2 devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 31 ++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
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 4f9c76f4d0da..f8a12a285050 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -427,6 +427,9 @@ struct kvaser_usb_net_leaf_priv {
struct kvaser_usb_net_priv *net;
struct delayed_work chip_state_req_work;
+
+ /* started but not reported as bus-on yet */
+ bool joining_bus;
};
static const struct can_bittiming_const kvaser_usb_leaf_m16c_bittiming_const = {
@@ -966,6 +969,7 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
const struct kvaser_usb_err_summary *es,
struct can_frame *cf)
{
+ struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
struct kvaser_usb *dev = priv->dev;
struct net_device_stats *stats = &priv->netdev->stats;
enum can_state cur_state, new_state, tx_state, rx_state;
@@ -990,6 +994,22 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
new_state = CAN_STATE_ERROR_ACTIVE;
}
+ /* 0bfd:0124 FW 4.18.778 was observed to send the initial
+ * CMD_CHIP_STATE_EVENT after CMD_START_CHIP with M16C_STATE_BUS_OFF
+ * bit set if the channel was bus-off when it was last stopped (even
+ * across chip resets). This bit will clear shortly afterwards, without
+ * triggering a second unsolicited chip state event.
+ * Ignore this initial bus-off.
+ */
+ if (leaf->joining_bus) {
+ if (new_state == CAN_STATE_BUS_OFF) {
+ netdev_dbg(priv->netdev, "ignoring bus-off during startup");
+ new_state = cur_state;
+ } else {
+ leaf->joining_bus = false;
+ }
+ }
+
if (new_state != cur_state) {
tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
@@ -1065,9 +1085,12 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
/* If there are errors, request status updates periodically as we do
* not get automatic notifications of improved state.
+ * Also request updates if we saw a stale BUS_OFF during startup
+ * (joining_bus).
*/
if (new_state < CAN_STATE_BUS_OFF &&
- (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE))
+ (es->rxerr || es->txerr || new_state == CAN_STATE_ERROR_PASSIVE ||
+ leaf->joining_bus))
schedule_delayed_work(&leaf->chip_state_req_work,
msecs_to_jiffies(500));
@@ -1587,8 +1610,11 @@ static int kvaser_usb_leaf_set_opt_mode(const struct kvaser_usb_net_priv *priv)
static int kvaser_usb_leaf_start_chip(struct kvaser_usb_net_priv *priv)
{
+ struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
int err;
+ leaf->joining_bus = true;
+
reinit_completion(&priv->start_comp);
err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_START_CHIP,
@@ -1719,12 +1745,15 @@ static int kvaser_usb_leaf_set_mode(struct net_device *netdev,
enum can_mode mode)
{
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+ struct kvaser_usb_net_leaf_priv *leaf = priv->sub_priv;
int err;
switch (mode) {
case CAN_MODE_START:
kvaser_usb_unlink_tx_urbs(priv);
+ leaf->joining_bus = true;
+
err = kvaser_usb_leaf_simple_cmd_async(priv, CMD_START_CHIP);
if (err)
return err;
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 09/11] can: kvaser_usb_leaf: Fix bogus restart events
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (7 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 08/11] can: kvaser_usb_leaf: Ignore stale bus-off after start Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 11/11] can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming Jimmy Assarsson
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
From: Anssi Hannula <anssi.hannula@bitwise.fi>
When auto-restart is enabled, the kvaser_usb_leaf driver considers
transition from any state >= CAN_STATE_BUS_OFF as a bus-off recovery
event (restart).
However, these events may occur at interface startup time before
kvaser_usb_open() has set the state to CAN_STATE_ERROR_ACTIVE, causing
restarts counter to increase and CAN_ERR_RESTARTED to be sent despite no
actual restart having occurred.
Fix that by making the auto-restart condition checks more strict so that
they only trigger when the interface was actually in the BUS_OFF state.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add S-o-b
Changes in v2:
- Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits")
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
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 f8a12a285050..3e31a9ebea88 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -901,7 +901,7 @@ static void kvaser_usb_leaf_tx_acknowledge(const struct kvaser_usb *dev,
context = &priv->tx_contexts[tid % dev->max_tx_urbs];
/* Sometimes the state change doesn't come after a bus-off event */
- if (priv->can.restart_ms && priv->can.state >= CAN_STATE_BUS_OFF) {
+ if (priv->can.restart_ms && priv->can.state == CAN_STATE_BUS_OFF) {
struct sk_buff *skb;
struct can_frame *cf;
@@ -1018,7 +1018,7 @@ kvaser_usb_leaf_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
}
if (priv->can.restart_ms &&
- cur_state >= CAN_STATE_BUS_OFF &&
+ cur_state == CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF)
priv->can.can_stats.restarts++;
@@ -1111,7 +1111,7 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev,
}
if (priv->can.restart_ms &&
- old_state >= CAN_STATE_BUS_OFF &&
+ old_state == CAN_STATE_BUS_OFF &&
new_state < CAN_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_RESTARTED;
netif_carrier_on(priv->netdev);
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (8 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 09/11] can: kvaser_usb_leaf: Fix bogus restart events Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
2022-10-19 21:57 ` Marc Kleine-Budde
2022-10-10 18:52 ` [PATCH can-next v5 11/11] can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming Jimmy Assarsson
10 siblings, 1 reply; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
Add struct kvaser_usb_busparams containing the busparameters used in
CMD_{SET,GET}_BUSPARAMS* commands.
Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add Tested-by Anssi Hannula
Changes in v2:
- New in v2.
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 8 +++++
.../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 32 +++++++------------
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 18 ++++-------
3 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index d9c5dd5da908..040885c7d0c4 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -76,6 +76,14 @@ struct kvaser_usb_tx_urb_context {
u32 echo_index;
};
+struct kvaser_usb_busparams {
+ __le32 bitrate;
+ u8 tseg1;
+ u8 tseg2;
+ u8 sjw;
+ u8 nsamples;
+};
+
struct kvaser_usb {
struct usb_device *udev;
struct usb_interface *intf;
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 66f672ea631b..3319af181e0e 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -196,17 +196,9 @@ struct kvaser_cmd_chip_state_event {
#define KVASER_USB_HYDRA_BUS_MODE_CANFD_ISO 0x01
#define KVASER_USB_HYDRA_BUS_MODE_NONISO 0x02
struct kvaser_cmd_set_busparams {
- __le32 bitrate;
- u8 tseg1;
- u8 tseg2;
- u8 sjw;
- u8 nsamples;
+ struct kvaser_usb_busparams busparams_arb;
u8 reserved0[4];
- __le32 bitrate_d;
- u8 tseg1_d;
- u8 tseg2_d;
- u8 sjw_d;
- u8 nsamples_d;
+ struct kvaser_usb_busparams busparams_data;
u8 canfd_mode;
u8 reserved1[7];
} __packed;
@@ -1538,11 +1530,11 @@ static int kvaser_usb_hydra_set_bittiming(struct net_device *netdev)
return -ENOMEM;
cmd->header.cmd_no = CMD_SET_BUSPARAMS_REQ;
- cmd->set_busparams_req.bitrate = cpu_to_le32(bt->bitrate);
- cmd->set_busparams_req.sjw = (u8)sjw;
- cmd->set_busparams_req.tseg1 = (u8)tseg1;
- cmd->set_busparams_req.tseg2 = (u8)tseg2;
- cmd->set_busparams_req.nsamples = 1;
+ cmd->set_busparams_req.busparams_arb.bitrate = cpu_to_le32(bt->bitrate);
+ cmd->set_busparams_req.busparams_arb.sjw = (u8)sjw;
+ cmd->set_busparams_req.busparams_arb.tseg1 = (u8)tseg1;
+ cmd->set_busparams_req.busparams_arb.tseg2 = (u8)tseg2;
+ cmd->set_busparams_req.busparams_arb.nsamples = 1;
kvaser_usb_hydra_set_cmd_dest_he
(cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
@@ -1572,11 +1564,11 @@ static int kvaser_usb_hydra_set_data_bittiming(struct net_device *netdev)
return -ENOMEM;
cmd->header.cmd_no = CMD_SET_BUSPARAMS_FD_REQ;
- cmd->set_busparams_req.bitrate_d = cpu_to_le32(dbt->bitrate);
- cmd->set_busparams_req.sjw_d = (u8)sjw;
- cmd->set_busparams_req.tseg1_d = (u8)tseg1;
- cmd->set_busparams_req.tseg2_d = (u8)tseg2;
- cmd->set_busparams_req.nsamples_d = 1;
+ cmd->set_busparams_req.busparams_data.bitrate = cpu_to_le32(dbt->bitrate);
+ cmd->set_busparams_req.busparams_data.sjw = (u8)sjw;
+ cmd->set_busparams_req.busparams_data.tseg1 = (u8)tseg1;
+ cmd->set_busparams_req.busparams_data.tseg2 = (u8)tseg2;
+ cmd->set_busparams_req.busparams_data.nsamples = 1;
if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
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 3e31a9ebea88..bb59ee01a093 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -164,11 +164,7 @@ struct usbcan_cmd_softinfo {
struct kvaser_cmd_busparams {
u8 tid;
u8 channel;
- __le32 bitrate;
- u8 tseg1;
- u8 tseg2;
- u8 sjw;
- u8 no_samp;
+ struct kvaser_usb_busparams busparams;
} __packed;
struct kvaser_cmd_tx_can {
@@ -1725,15 +1721,15 @@ static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
cmd->len = CMD_HEADER_LEN + sizeof(struct kvaser_cmd_busparams);
cmd->u.busparams.channel = priv->channel;
cmd->u.busparams.tid = 0xff;
- cmd->u.busparams.bitrate = cpu_to_le32(bt->bitrate);
- cmd->u.busparams.sjw = bt->sjw;
- cmd->u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1;
- cmd->u.busparams.tseg2 = bt->phase_seg2;
+ cmd->u.busparams.busparams.bitrate = cpu_to_le32(bt->bitrate);
+ cmd->u.busparams.busparams.sjw = bt->sjw;
+ cmd->u.busparams.busparams.tseg1 = bt->prop_seg + bt->phase_seg1;
+ cmd->u.busparams.busparams.tseg2 = bt->phase_seg2;
if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
- cmd->u.busparams.no_samp = 3;
+ cmd->u.busparams.busparams.nsamples = 3;
else
- cmd->u.busparams.no_samp = 1;
+ cmd->u.busparams.busparams.nsamples = 1;
rc = kvaser_usb_send_cmd(dev, cmd, cmd->len);
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH can-next v5 11/11] can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
` (9 preceding siblings ...)
2022-10-10 18:52 ` [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams Jimmy Assarsson
@ 2022-10-10 18:52 ` Jimmy Assarsson
10 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-10 18:52 UTC (permalink / raw)
To: linux-can, Marc Kleine-Budde, Anssi Hannula
Cc: Jimmy Assarsson, Jimmy Assarsson
The device will respond with a CMD_ERROR_EVENT command, with error_code
KVASER_USB_{LEAF,HYDRA}_ERROR_EVENT_PARAM, if the CMD_SET_BUSPARAMS_REQ
contains invalid bittiming parameters.
However, this command does not contain any channel reference.
To check if the CMD_SET_BUSPARAMS_REQ was successful, redback and compare
the requested bittiming parameters with the device reported parameters.
Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices")
Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Co-developed-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v5:
- Split series and rebased on linux-can-next
- Remove stable from CC
Changes in v4:
- No changes
Changes in v3:
- Rebased on 1d5eeda23f36 ("can: kvaser_usb: advertise timestamping capabilities and add ioctl support")
- Add stable to CC
- Add Tested-by Anssi Hannula
Changes in v2:
- New in v2. Replaces parts of [PATCH 09/12] can: kvaser_usb_leaf: Fix silently failing bus params
setup
drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 15 +-
.../net/can/usb/kvaser_usb/kvaser_usb_core.c | 96 ++++++++++-
.../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 150 +++++++++++++++---
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 64 ++++++--
4 files changed, 284 insertions(+), 41 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index 040885c7d0c4..8c4c2f00fb9f 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -119,9 +119,12 @@ struct kvaser_usb_net_priv {
struct net_device *netdev;
int channel;
- struct completion start_comp, stop_comp, flush_comp;
+ struct completion start_comp, stop_comp, flush_comp,
+ get_busparams_comp;
struct usb_anchor tx_submitted;
+ struct kvaser_usb_busparams busparams_nominal, busparams_data;
+
spinlock_t tx_contexts_lock; /* lock for active_tx_contexts */
int active_tx_contexts;
struct kvaser_usb_tx_urb_context tx_contexts[];
@@ -131,7 +134,9 @@ struct kvaser_usb_net_priv {
* struct kvaser_usb_dev_ops - Device specific functions
* @dev_set_mode: used for can.do_set_mode
* @dev_set_bittiming: used for can.do_set_bittiming
+ * @dev_get_busparams: readback arbitration busparams
* @dev_set_data_bittiming: used for can.do_set_data_bittiming
+ * @dev_get_data_busparams: readback data busparams
* @dev_get_berr_counter: used for can.do_get_berr_counter
*
* @dev_setup_endpoints: setup USB in and out endpoints
@@ -153,8 +158,12 @@ struct kvaser_usb_net_priv {
*/
struct kvaser_usb_dev_ops {
int (*dev_set_mode)(struct net_device *netdev, enum can_mode mode);
- int (*dev_set_bittiming)(struct net_device *netdev);
- int (*dev_set_data_bittiming)(struct net_device *netdev);
+ int (*dev_set_bittiming)(const struct net_device *netdev,
+ const struct kvaser_usb_busparams *busparams);
+ int (*dev_get_busparams)(struct kvaser_usb_net_priv *priv);
+ int (*dev_set_data_bittiming)(const struct net_device *netdev,
+ const struct kvaser_usb_busparams *busparams);
+ int (*dev_get_data_busparams)(struct kvaser_usb_net_priv *priv);
int (*dev_get_berr_counter)(const struct net_device *netdev,
struct can_berr_counter *bec);
int (*dev_setup_endpoints)(struct kvaser_usb *dev);
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 19df05887166..989e75351062 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -440,10 +440,6 @@ static int kvaser_usb_open(struct net_device *netdev)
if (err)
return err;
- err = kvaser_usb_setup_rx_urbs(dev);
- if (err)
- goto error;
-
err = ops->dev_set_opt_mode(priv);
if (err)
goto error;
@@ -534,6 +530,93 @@ static int kvaser_usb_close(struct net_device *netdev)
return 0;
}
+static int kvaser_usb_set_bittiming(struct net_device *netdev)
+{
+ struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+ struct kvaser_usb *dev = priv->dev;
+ const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
+ struct can_bittiming *bt = &priv->can.bittiming;
+
+ struct kvaser_usb_busparams busparams;
+ int tseg1 = bt->prop_seg + bt->phase_seg1;
+ int tseg2 = bt->phase_seg2;
+ int sjw = bt->sjw;
+ int err = -EOPNOTSUPP;
+
+ busparams.bitrate = cpu_to_le32(bt->bitrate);
+ busparams.sjw = (u8)sjw;
+ busparams.tseg1 = (u8)tseg1;
+ busparams.tseg2 = (u8)tseg2;
+ if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+ busparams.nsamples = 3;
+ else
+ busparams.nsamples = 1;
+
+ err = ops->dev_set_bittiming(netdev, &busparams);
+ if (err)
+ return err;
+
+ err = kvaser_usb_setup_rx_urbs(priv->dev);
+ if (err)
+ return err;
+
+ err = ops->dev_get_busparams(priv);
+ if (err) {
+ /* Treat EOPNOTSUPP as success */
+ if (err == -EOPNOTSUPP)
+ err = 0;
+ return err;
+ }
+
+ if (memcmp(&busparams, &priv->busparams_nominal,
+ sizeof(priv->busparams_nominal)) != 0)
+ err = -EINVAL;
+
+ return err;
+}
+
+static int kvaser_usb_set_data_bittiming(struct net_device *netdev)
+{
+ struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+ struct kvaser_usb *dev = priv->dev;
+ const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
+ struct can_bittiming *dbt = &priv->can.data_bittiming;
+
+ struct kvaser_usb_busparams busparams;
+ int tseg1 = dbt->prop_seg + dbt->phase_seg1;
+ int tseg2 = dbt->phase_seg2;
+ int sjw = dbt->sjw;
+ int err;
+
+ if (!ops->dev_set_data_bittiming ||
+ !ops->dev_get_data_busparams)
+ return -EOPNOTSUPP;
+
+ busparams.bitrate = cpu_to_le32(dbt->bitrate);
+ busparams.sjw = (u8)sjw;
+ busparams.tseg1 = (u8)tseg1;
+ busparams.tseg2 = (u8)tseg2;
+ busparams.nsamples = 1;
+
+ err = ops->dev_set_data_bittiming(netdev, &busparams);
+ if (err)
+ return err;
+
+ err = kvaser_usb_setup_rx_urbs(priv->dev);
+ if (err)
+ return err;
+
+ err = ops->dev_get_data_busparams(priv);
+ if (err)
+ return err;
+
+ if (memcmp(&busparams, &priv->busparams_data,
+ sizeof(priv->busparams_data)) != 0)
+ err = -EINVAL;
+
+ return err;
+}
+
static void kvaser_usb_write_bulk_callback(struct urb *urb)
{
struct kvaser_usb_tx_urb_context *context = urb->context;
@@ -734,6 +817,7 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
init_completion(&priv->start_comp);
init_completion(&priv->stop_comp);
init_completion(&priv->flush_comp);
+ init_completion(&priv->get_busparams_comp);
priv->can.ctrlmode_supported = 0;
priv->dev = dev;
@@ -746,7 +830,7 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
priv->can.state = CAN_STATE_STOPPED;
priv->can.clock.freq = dev->cfg->clock.freq;
priv->can.bittiming_const = dev->cfg->bittiming_const;
- priv->can.do_set_bittiming = ops->dev_set_bittiming;
+ priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
priv->can.do_set_mode = ops->dev_set_mode;
if ((driver_info->quirks & KVASER_USB_QUIRK_HAS_TXRX_ERRORS) ||
(priv->dev->card_data.capabilities & KVASER_USB_CAP_BERR_CAP))
@@ -758,7 +842,7 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
if (priv->can.ctrlmode_supported & CAN_CTRLMODE_FD) {
priv->can.data_bittiming_const = dev->cfg->data_bittiming_const;
- priv->can.do_set_data_bittiming = ops->dev_set_data_bittiming;
+ priv->can.do_set_data_bittiming = kvaser_usb_set_data_bittiming;
}
netdev->flags |= IFF_ECHO;
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 3319af181e0e..f688124d6d66 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -45,6 +45,8 @@ static const struct kvaser_usb_dev_cfg kvaser_usb_hydra_dev_cfg_rt;
/* Minihydra command IDs */
#define CMD_SET_BUSPARAMS_REQ 16
+#define CMD_GET_BUSPARAMS_REQ 17
+#define CMD_GET_BUSPARAMS_RESP 18
#define CMD_GET_CHIP_STATE_REQ 19
#define CMD_CHIP_STATE_EVENT 20
#define CMD_SET_DRIVERMODE_REQ 21
@@ -196,13 +198,26 @@ struct kvaser_cmd_chip_state_event {
#define KVASER_USB_HYDRA_BUS_MODE_CANFD_ISO 0x01
#define KVASER_USB_HYDRA_BUS_MODE_NONISO 0x02
struct kvaser_cmd_set_busparams {
- struct kvaser_usb_busparams busparams_arb;
+ struct kvaser_usb_busparams busparams_nominal;
u8 reserved0[4];
struct kvaser_usb_busparams busparams_data;
u8 canfd_mode;
u8 reserved1[7];
} __packed;
+/* Busparam type */
+#define KVASER_USB_HYDRA_BUSPARAM_TYPE_CAN 0x00
+#define KVASER_USB_HYDRA_BUSPARAM_TYPE_CANFD 0x01
+struct kvaser_cmd_get_busparams_req {
+ u8 type;
+ u8 reserved[27];
+} __packed;
+
+struct kvaser_cmd_get_busparams_res {
+ struct kvaser_usb_busparams busparams;
+ u8 reserved[20];
+} __packed;
+
/* Ctrl modes */
#define KVASER_USB_HYDRA_CTRLMODE_NORMAL 0x01
#define KVASER_USB_HYDRA_CTRLMODE_LISTEN 0x02
@@ -273,6 +288,8 @@ struct kvaser_cmd {
struct kvaser_cmd_error_event error_event;
struct kvaser_cmd_set_busparams set_busparams_req;
+ struct kvaser_cmd_get_busparams_req get_busparams_req;
+ struct kvaser_cmd_get_busparams_res get_busparams_res;
struct kvaser_cmd_chip_state_event chip_state_event;
@@ -355,6 +372,10 @@ struct kvaser_cmd_ext {
} __packed;
} __packed;
+struct kvaser_usb_net_hydra_priv {
+ int pending_get_busparams_type;
+};
+
static const struct can_bittiming_const kvaser_usb_hydra_kcan_bittiming_c = {
.name = "kvaser_usb_kcan",
.tseg1_min = 1,
@@ -832,6 +853,39 @@ static void kvaser_usb_hydra_flush_queue_reply(const struct kvaser_usb *dev,
complete(&priv->flush_comp);
}
+static void kvaser_usb_hydra_get_busparams_reply(const struct kvaser_usb *dev,
+ const struct kvaser_cmd *cmd)
+{
+ struct kvaser_usb_net_priv *priv;
+ struct kvaser_usb_net_hydra_priv *hydra;
+
+ priv = kvaser_usb_hydra_net_priv_from_cmd(dev, cmd);
+ if (!priv)
+ return;
+
+ hydra = priv->sub_priv;
+ if (!hydra)
+ return;
+
+ switch (hydra->pending_get_busparams_type) {
+ case KVASER_USB_HYDRA_BUSPARAM_TYPE_CAN:
+ memcpy(&priv->busparams_nominal, &cmd->get_busparams_res.busparams,
+ sizeof(priv->busparams_nominal));
+ break;
+ case KVASER_USB_HYDRA_BUSPARAM_TYPE_CANFD:
+ memcpy(&priv->busparams_data, &cmd->get_busparams_res.busparams,
+ sizeof(priv->busparams_nominal));
+ break;
+ default:
+ dev_warn(&dev->intf->dev, "Unknown get_busparams_type %d\n",
+ hydra->pending_get_busparams_type);
+ break;
+ }
+ hydra->pending_get_busparams_type = -1;
+
+ complete(&priv->get_busparams_comp);
+}
+
static void
kvaser_usb_hydra_bus_status_to_can_state(const struct kvaser_usb_net_priv *priv,
u8 bus_status,
@@ -1318,6 +1372,10 @@ static void kvaser_usb_hydra_handle_cmd_std(const struct kvaser_usb *dev,
kvaser_usb_hydra_state_event(dev, cmd);
break;
+ case CMD_GET_BUSPARAMS_RESP:
+ kvaser_usb_hydra_get_busparams_reply(dev, cmd);
+ break;
+
case CMD_ERROR_EVENT:
kvaser_usb_hydra_error_event(dev, cmd);
break;
@@ -1514,15 +1572,58 @@ static int kvaser_usb_hydra_set_mode(struct net_device *netdev,
return err;
}
-static int kvaser_usb_hydra_set_bittiming(struct net_device *netdev)
+static int kvaser_usb_hydra_get_busparams(struct kvaser_usb_net_priv *priv,
+ int busparams_type)
+{
+ struct kvaser_usb *dev = priv->dev;
+ struct kvaser_usb_net_hydra_priv *hydra = priv->sub_priv;
+ struct kvaser_cmd *cmd;
+ int err;
+
+ if (!hydra)
+ return -EINVAL;
+
+ cmd = kcalloc(1, sizeof(struct kvaser_cmd), GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ cmd->header.cmd_no = CMD_GET_BUSPARAMS_REQ;
+ kvaser_usb_hydra_set_cmd_dest_he
+ (cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
+ kvaser_usb_hydra_set_cmd_transid
+ (cmd, kvaser_usb_hydra_get_next_transid(dev));
+ cmd->get_busparams_req.type = busparams_type;
+ hydra->pending_get_busparams_type = busparams_type;
+
+ reinit_completion(&priv->get_busparams_comp);
+
+ err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+ if (err)
+ return err;
+
+ if (!wait_for_completion_timeout(&priv->get_busparams_comp,
+ msecs_to_jiffies(KVASER_USB_TIMEOUT)))
+ return -ETIMEDOUT;
+
+ return err;
+}
+
+static int kvaser_usb_hydra_get_nominal_busparams(struct kvaser_usb_net_priv *priv)
+{
+ return kvaser_usb_hydra_get_busparams(priv, KVASER_USB_HYDRA_BUSPARAM_TYPE_CAN);
+}
+
+static int kvaser_usb_hydra_get_data_busparams(struct kvaser_usb_net_priv *priv)
+{
+ return kvaser_usb_hydra_get_busparams(priv, KVASER_USB_HYDRA_BUSPARAM_TYPE_CANFD);
+}
+
+static int kvaser_usb_hydra_set_bittiming(const struct net_device *netdev,
+ const struct kvaser_usb_busparams *busparams)
{
struct kvaser_cmd *cmd;
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
- struct can_bittiming *bt = &priv->can.bittiming;
struct kvaser_usb *dev = priv->dev;
- int tseg1 = bt->prop_seg + bt->phase_seg1;
- int tseg2 = bt->phase_seg2;
- int sjw = bt->sjw;
int err;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1530,11 +1631,8 @@ static int kvaser_usb_hydra_set_bittiming(struct net_device *netdev)
return -ENOMEM;
cmd->header.cmd_no = CMD_SET_BUSPARAMS_REQ;
- cmd->set_busparams_req.busparams_arb.bitrate = cpu_to_le32(bt->bitrate);
- cmd->set_busparams_req.busparams_arb.sjw = (u8)sjw;
- cmd->set_busparams_req.busparams_arb.tseg1 = (u8)tseg1;
- cmd->set_busparams_req.busparams_arb.tseg2 = (u8)tseg2;
- cmd->set_busparams_req.busparams_arb.nsamples = 1;
+ memcpy(&cmd->set_busparams_req.busparams_nominal, busparams,
+ sizeof(cmd->set_busparams_req.busparams_nominal));
kvaser_usb_hydra_set_cmd_dest_he
(cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
@@ -1548,15 +1646,12 @@ static int kvaser_usb_hydra_set_bittiming(struct net_device *netdev)
return err;
}
-static int kvaser_usb_hydra_set_data_bittiming(struct net_device *netdev)
+static int kvaser_usb_hydra_set_data_bittiming(const struct net_device *netdev,
+ const struct kvaser_usb_busparams *busparams)
{
struct kvaser_cmd *cmd;
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
- struct can_bittiming *dbt = &priv->can.data_bittiming;
struct kvaser_usb *dev = priv->dev;
- int tseg1 = dbt->prop_seg + dbt->phase_seg1;
- int tseg2 = dbt->phase_seg2;
- int sjw = dbt->sjw;
int err;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1564,11 +1659,8 @@ static int kvaser_usb_hydra_set_data_bittiming(struct net_device *netdev)
return -ENOMEM;
cmd->header.cmd_no = CMD_SET_BUSPARAMS_FD_REQ;
- cmd->set_busparams_req.busparams_data.bitrate = cpu_to_le32(dbt->bitrate);
- cmd->set_busparams_req.busparams_data.sjw = (u8)sjw;
- cmd->set_busparams_req.busparams_data.tseg1 = (u8)tseg1;
- cmd->set_busparams_req.busparams_data.tseg2 = (u8)tseg2;
- cmd->set_busparams_req.busparams_data.nsamples = 1;
+ memcpy(&cmd->set_busparams_req.busparams_data, busparams,
+ sizeof(cmd->set_busparams_req.busparams_data));
if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
if (priv->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
@@ -1675,6 +1767,19 @@ static int kvaser_usb_hydra_init_card(struct kvaser_usb *dev)
return 0;
}
+static int kvaser_usb_hydra_init_channel(struct kvaser_usb_net_priv *priv)
+{
+ struct kvaser_usb_net_hydra_priv *hydra;
+
+ hydra = devm_kzalloc(&priv->dev->intf->dev, sizeof(*hydra), GFP_KERNEL);
+ if (!hydra)
+ return -ENOMEM;
+
+ priv->sub_priv = hydra;
+
+ return 0;
+}
+
static int kvaser_usb_hydra_get_software_info(struct kvaser_usb *dev)
{
struct kvaser_cmd cmd;
@@ -2019,10 +2124,13 @@ kvaser_usb_hydra_frame_to_cmd(const struct kvaser_usb_net_priv *priv,
const struct kvaser_usb_dev_ops kvaser_usb_hydra_dev_ops = {
.dev_set_mode = kvaser_usb_hydra_set_mode,
.dev_set_bittiming = kvaser_usb_hydra_set_bittiming,
+ .dev_get_busparams = kvaser_usb_hydra_get_nominal_busparams,
.dev_set_data_bittiming = kvaser_usb_hydra_set_data_bittiming,
+ .dev_get_data_busparams = kvaser_usb_hydra_get_data_busparams,
.dev_get_berr_counter = kvaser_usb_hydra_get_berr_counter,
.dev_setup_endpoints = kvaser_usb_hydra_setup_endpoints,
.dev_init_card = kvaser_usb_hydra_init_card,
+ .dev_init_channel = kvaser_usb_hydra_init_channel,
.dev_get_software_info = kvaser_usb_hydra_get_software_info,
.dev_get_software_details = kvaser_usb_hydra_get_software_details,
.dev_get_card_info = kvaser_usb_hydra_get_card_info,
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 bb59ee01a093..1c2f99ce4c6c 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
@@ -57,6 +57,8 @@
#define CMD_RX_EXT_MESSAGE 14
#define CMD_TX_EXT_MESSAGE 15
#define CMD_SET_BUS_PARAMS 16
+#define CMD_GET_BUS_PARAMS 17
+#define CMD_GET_BUS_PARAMS_REPLY 18
#define CMD_GET_CHIP_STATE 19
#define CMD_CHIP_STATE_EVENT 20
#define CMD_SET_CTRL_MODE 21
@@ -376,6 +378,7 @@ static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = {
[CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.leaf.chip_state_event),
[CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.can_error_event),
[CMD_GET_CAPABILITIES_RESP] = kvaser_fsize(u.leaf.cap_res),
+ [CMD_GET_BUS_PARAMS_REPLY] = kvaser_fsize(u.busparams),
[CMD_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event),
/* ignored events: */
[CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY,
@@ -1486,6 +1489,25 @@ static void kvaser_usb_leaf_stop_chip_reply(const struct kvaser_usb *dev,
complete(&priv->stop_comp);
}
+static void kvaser_usb_leaf_get_busparams_reply(const struct kvaser_usb *dev,
+ const struct kvaser_cmd *cmd)
+{
+ struct kvaser_usb_net_priv *priv;
+ u8 channel = cmd->u.busparams.channel;
+
+ if (channel >= dev->nchannels) {
+ dev_err(&dev->intf->dev,
+ "Invalid channel number (%d)\n", channel);
+ return;
+ }
+
+ priv = dev->nets[channel];
+ memcpy(&priv->busparams_nominal, &cmd->u.busparams.busparams,
+ sizeof(priv->busparams_nominal));
+
+ complete(&priv->get_busparams_comp);
+}
+
static void kvaser_usb_leaf_handle_command(const struct kvaser_usb *dev,
const struct kvaser_cmd *cmd)
{
@@ -1528,6 +1550,10 @@ static void kvaser_usb_leaf_handle_command(const struct kvaser_usb *dev,
kvaser_usb_leaf_error_event(dev, cmd);
break;
+ case CMD_GET_BUS_PARAMS_REPLY:
+ kvaser_usb_leaf_get_busparams_reply(dev, cmd);
+ break;
+
/* Ignored commands */
case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
if (dev->driver_info->family != KVASER_USBCAN)
@@ -1705,10 +1731,10 @@ static void kvaser_usb_leaf_remove_channel(struct kvaser_usb_net_priv *priv)
cancel_delayed_work_sync(&leaf->chip_state_req_work);
}
-static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
+static int kvaser_usb_leaf_set_bittiming(const struct net_device *netdev,
+ const struct kvaser_usb_busparams *busparams)
{
struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
- struct can_bittiming *bt = &priv->can.bittiming;
struct kvaser_usb *dev = priv->dev;
struct kvaser_cmd *cmd;
int rc;
@@ -1721,15 +1747,8 @@ static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
cmd->len = CMD_HEADER_LEN + sizeof(struct kvaser_cmd_busparams);
cmd->u.busparams.channel = priv->channel;
cmd->u.busparams.tid = 0xff;
- cmd->u.busparams.busparams.bitrate = cpu_to_le32(bt->bitrate);
- cmd->u.busparams.busparams.sjw = bt->sjw;
- cmd->u.busparams.busparams.tseg1 = bt->prop_seg + bt->phase_seg1;
- cmd->u.busparams.busparams.tseg2 = bt->phase_seg2;
-
- if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
- cmd->u.busparams.busparams.nsamples = 3;
- else
- cmd->u.busparams.busparams.nsamples = 1;
+ memcpy(&cmd->u.busparams.busparams, busparams,
+ sizeof(cmd->u.busparams.busparams));
rc = kvaser_usb_send_cmd(dev, cmd, cmd->len);
@@ -1737,6 +1756,27 @@ static int kvaser_usb_leaf_set_bittiming(struct net_device *netdev)
return rc;
}
+static int kvaser_usb_leaf_get_busparams(struct kvaser_usb_net_priv *priv)
+{
+ int err;
+
+ if (priv->dev->driver_info->family == KVASER_USBCAN)
+ return -EOPNOTSUPP;
+
+ reinit_completion(&priv->get_busparams_comp);
+
+ err = kvaser_usb_leaf_send_simple_cmd(priv->dev, CMD_GET_BUS_PARAMS,
+ priv->channel);
+ if (err)
+ return err;
+
+ if (!wait_for_completion_timeout(&priv->get_busparams_comp,
+ msecs_to_jiffies(KVASER_USB_TIMEOUT)))
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
static int kvaser_usb_leaf_set_mode(struct net_device *netdev,
enum can_mode mode)
{
@@ -1801,7 +1841,9 @@ static int kvaser_usb_leaf_setup_endpoints(struct kvaser_usb *dev)
const struct kvaser_usb_dev_ops kvaser_usb_leaf_dev_ops = {
.dev_set_mode = kvaser_usb_leaf_set_mode,
.dev_set_bittiming = kvaser_usb_leaf_set_bittiming,
+ .dev_get_busparams = kvaser_usb_leaf_get_busparams,
.dev_set_data_bittiming = NULL,
+ .dev_get_data_busparams = NULL,
.dev_get_berr_counter = kvaser_usb_leaf_get_berr_counter,
.dev_setup_endpoints = kvaser_usb_leaf_setup_endpoints,
.dev_init_card = kvaser_usb_leaf_init_card,
--
2.38.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams
2022-10-10 18:52 ` [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams Jimmy Assarsson
@ 2022-10-19 21:57 ` Marc Kleine-Budde
0 siblings, 0 replies; 15+ messages in thread
From: Marc Kleine-Budde @ 2022-10-19 21:57 UTC (permalink / raw)
To: Jimmy Assarsson; +Cc: linux-can, Anssi Hannula, Jimmy Assarsson
[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]
On 10.10.2022 20:52:36, Jimmy Assarsson wrote:
> Add struct kvaser_usb_busparams containing the busparameters used in
> CMD_{SET,GET}_BUSPARAMS* commands.
>
> Tested-by: Anssi Hannula <anssi.hannula@bitwise.fi>
> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
> ---
[...]
> +struct kvaser_usb_busparams {
> + __le32 bitrate;
> + u8 tseg1;
> + u8 tseg2;
> + u8 sjw;
> + u8 nsamples;
> +};
| drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c:169:30: error: field
| busparams within 'struct kvaser_cmd_busparams' is less aligned than
| 'struct kvaser_usb_busparams' and is usually due to 'struct
| kvaser_cmd_busparams' being packed, which can lead to unaligned
| accesses [-Werror,-Wunaligned-access]
| struct kvaser_usb_busparams busparams;
| ^
Fixed (again) while applying.
diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
index 8c4c2f00fb9f..ff10b3790d84 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb.h
@@ -82,7 +82,7 @@ struct kvaser_usb_busparams {
u8 tseg2;
u8 sjw;
u8 nsamples;
-};
+} __packed;
struct kvaser_usb {
struct usb_device *udev;
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion
2022-10-10 18:52 ` [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion Jimmy Assarsson
@ 2022-10-26 8:14 ` Marc Kleine-Budde
2022-10-26 18:04 ` Jimmy Assarsson
0 siblings, 1 reply; 15+ messages in thread
From: Marc Kleine-Budde @ 2022-10-26 8:14 UTC (permalink / raw)
To: Jimmy Assarsson; +Cc: linux-can, Anssi Hannula, Jimmy Assarsson
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
On 10.10.2022 20:52:27, Jimmy Assarsson wrote:
> From: Anssi Hannula <anssi.hannula@bitwise.fi>
>
> kvaser_usb uses completions to signal when a response event is received
> for outgoing commands.
>
> However, it uses init_completion() to reinitialize the start_comp and
> stop_comp completions before sending the start/stop commands.
>
> In case the device sends the corresponding response just before the
> actual command is sent, complete() may be called concurrently with
> init_completion() which is not safe.
>
> This might be triggerable even with a properly functioning device by
> stopping the interface (CMD_STOP_CHIP) just after it goes bus-off (which
> also causes the driver to send CMD_STOP_CHIP when restart-ms is off),
> but that was not tested.
>
> Fix the issue by using reinit_completion() instead.
Looks like this should go to net instead of net-next.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion
2022-10-26 8:14 ` Marc Kleine-Budde
@ 2022-10-26 18:04 ` Jimmy Assarsson
0 siblings, 0 replies; 15+ messages in thread
From: Jimmy Assarsson @ 2022-10-26 18:04 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Anssi Hannula, Jimmy Assarsson
On 10/26/22 10:14, Marc Kleine-Budde wrote:
> On 10.10.2022 20:52:27, Jimmy Assarsson wrote:
>> From: Anssi Hannula <anssi.hannula@bitwise.fi>
>>
>> kvaser_usb uses completions to signal when a response event is received
>> for outgoing commands.
>>
>> However, it uses init_completion() to reinitialize the start_comp and
>> stop_comp completions before sending the start/stop commands.
>>
>> In case the device sends the corresponding response just before the
>> actual command is sent, complete() may be called concurrently with
>> init_completion() which is not safe.
>>
>> This might be triggerable even with a properly functioning device by
>> stopping the interface (CMD_STOP_CHIP) just after it goes bus-off (which
>> also causes the driver to send CMD_STOP_CHIP when restart-ms is off),
>> but that was not tested.
>>
>> Fix the issue by using reinit_completion() instead.
>
> Looks like this should go to net instead of net-next.
Yes, I agree.
Thanks!
/jimmy
> Marc
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-10-26 18:04 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-10 18:52 [PATCH can-next v5 00/11] can: kvaser_usb: Fixes and improvements Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion Jimmy Assarsson
2022-10-26 8:14 ` Marc Kleine-Budde
2022-10-26 18:04 ` Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 02/11] can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 03/11] can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 04/11] can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 05/11] can: kvaser_usb_leaf: Set Warning state even without bus errors Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 06/11] can: kvaser_usb_leaf: Fix improved state not being reported Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 07/11] can: kvaser_usb_leaf: Fix wrong CAN state after stopping Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 08/11] can: kvaser_usb_leaf: Ignore stale bus-off after start Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 09/11] can: kvaser_usb_leaf: Fix bogus restart events Jimmy Assarsson
2022-10-10 18:52 ` [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams Jimmy Assarsson
2022-10-19 21:57 ` Marc Kleine-Budde
2022-10-10 18:52 ` [PATCH can-next v5 11/11] can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming Jimmy Assarsson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.