can: kvaser_usb: Fixes and improvements
v5 cover can: kvaser_usb: Fixes and improvements
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/[email protected]
[2]
https://lore.kernel.org/linux-can/[email protected]
[3]
https://lore.kernel.org/linux-can/[email protected]/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(-)
Signature · 1 line
2.38.0
PATCH v5 1/11 can: kvaser_usb: Fix possible completions during init_completion
From: Anssi Hannula <[email protected]>
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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +2 -2
@@ -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()
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +2 -2
@@ -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);
Signature · 1 line
2.38.0
Marc Kleine-Budde
maintainerRe: [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion
Quoted 12 lines · Jimmy Assarsson
From: Anssi Hannula <[email protected]>
kvaser_usbuses completions to signal when a response event is received for outgoing commands.However, it uses
init_completion()to reinitialize thestart_compandstop_compcompletions 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 withinit_completion()which is not safe.
Quoted 6 lines
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 sendCMD_STOP_CHIPwhen 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
Signature · 4 lines
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 |
Re: [PATCH can-next v5 01/11] can: kvaser_usb: Fix possible completions during init_completion
Quoted 13 lines · Marc Kleine-Budde
Quoted 12 lines · Jimmy Assarsson
From: Anssi Hannula <[email protected]>
kvaser_usbuses completions to signal when a response event is received for outgoing commands.However, it uses
init_completion()to reinitialize thestart_compandstop_compcompletions 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 withinit_completion()which is not safe.
Quoted 8 lines
Quoted 6 lines
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 sendCMD_STOP_CHIPwhen 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
Quoted 1 line
Marc
PATCH v5 2/11 can: kvaser_usb: kvaser_usb_leaf: Get capabilities from device
In reply to Jimmy AssarssonUse 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.
---
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/[email protected]/
[1] https://lore.kernel.org/linux-can/[email protected]/
.../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 144 +++++++++++++++++- 1 file changed, 143 insertions(+), 1 deletion(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +143 -1
@@ -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,
Signature · 1 line
2.38.0
PATCH v5 3/11 can: kvaser_usb: kvaser_usb_leaf: Rename {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event
In reply to Jimmy AssarssonPrepare for handling CMD_ERROR_EVENT. Rename struct {leaf,usbcan}_cmd_error_event to {leaf,usbcan}_cmd_can_error_event.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +19 -19
@@ -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;
Signature · 1 line
2.38.0
PATCH v5 4/11 can: kvaser_usb: kvaser_usb_leaf: Handle CMD_ERROR_EVENT
In reply to Jimmy AssarssonThe device will send an error event command, to indicate certain errors. This indicates a misbehaving driver, and should never occur.
---
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(+)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +99 -0
@@ -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 )
Signature · 1 line
2.38.0
PATCH v5 5/11 can: kvaser_usb_leaf: Set Warning state even without bus errors
In reply to Jimmy AssarssonFrom: Anssi Hannula <[email protected]>
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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +8 -12
@@ -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 ;
Signature · 1 line
2.38.0
PATCH v5 6/11 can: kvaser_usb_leaf: Fix improved state not being reported
In reply to Jimmy AssarssonFrom: Anssi Hannula <[email protected]>
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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb.h +7 -0
@@ -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);
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c +16 -3
@@ -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,
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +58 -0
122 lines not on this page · raw message
Signature · 1 line
2.38.0
PATCH v5 7/11 can: kvaser_usb_leaf: Fix wrong CAN state after stopping
In reply to Jimmy AssarssonFrom: Anssi Hannula <[email protected]>
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.
---
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(+)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +4 -0
@@ -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. *
Signature · 1 line
2.38.0
PATCH v5 8/11 can: kvaser_usb_leaf: Ignore stale bus-off after start
In reply to Jimmy AssarssonFrom: Anssi Hannula <[email protected]>
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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +30 -1
@@ -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;
Signature · 1 line
2.38.0
PATCH v5 9/11 can: kvaser_usb_leaf: Fix bogus restart events
In reply to Jimmy AssarssonFrom: Anssi Hannula <[email protected]>
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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +3 -3
@@ -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);
Signature · 1 line
2.38.0
PATCH v5 10/11 can: kvaser_usb: Add struct kvaser_usb_busparams
In reply to Jimmy AssarssonAdd struct kvaser_usb_busparams containing the busparameters used in CMD_{SET,GET}_BUSPARAMS* commands.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb.h +8 -0
@@ -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;
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +12 -20
@@ -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 0x02struct 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 )
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +7 -11
@@ -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);
Signature · 1 line
2.38.0
Marc Kleine-Budde
maintainerRe: [PATCH can-next v5 10/11] can: kvaser_usb: Add struct kvaser_usb_busparams
Quoted 6 lines · Jimmy Assarsson
Add struct
kvaser_usb_busparamscontaining the busparameters used in CMD_{SET,GET}_BUSPARAMS* commands.Tested-by Anssi Hannula <[email protected]>Signed-off-by Jimmy Assarsson <[email protected]>---
[...]
Review
Quoted 7 lines
(quoted diff) +7
-0+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.
drivers/net/can/usb/kvaser_usb/kvaser_usb.h +1 -1
@@ -82,7 +82,7 @@ struct kvaser_usb_busparams {u8 tseg2;u8 sjw;u8 nsamples; -}; +} __packed;struct kvaser_usb {struct usb_device *udev;
regards,
Marc
Signature · 4 lines
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 |
PATCH v5 11/11 can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming
In reply to Jimmy AssarssonThe 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.
---
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(-)
drivers/net/can/usb/kvaser_usb/kvaser_usb.h +12 -3
@@ -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);
drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c +90 -6
126 lines not on this page · raw message
drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c +129 -21
240 lines not on this page · raw message
drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +53 -11
115 lines not on this page · raw message
Signature · 1 line
2.38.0