On 09/21/2012 11:54 AM, Marc Kleine-Budde wrote: > On 09/20/2012 07:06 AM, Olivier Sobrie wrote: >> This driver provides support for several Kvaser CAN/USB devices. >> Such kind of devices supports up to three CAN network interfaces. >> >> It has been tested with a Kvaser USB Leaf Light (one network interface) >> connected to a pch_can interface. >> The firmware version of the Kvaser device was 2.5.205. > > I don't remember, have the USB people already had a look on your driver > and gave some comments? IIRC, Oliver Neukum commented on v2. Actually we ignored v3 :(, sorry! > From the CAN and network point of view looks good, some comments inline. > Would be fine, if Wolfgang can give comments or Ack about the error > frame generation. Olivier already sent candump traces for no cable connected and short-circuiting CAN high and low. The error handling looked good, IIRC, at least as good as the firmware can do... more comments inline... >> List of Kvaser devices supported by the driver: >> - Kvaser Leaf prototype (P010v2 and v3) >> - Kvaser Leaf Light (P010v3) >> - Kvaser Leaf Professional HS >> - Kvaser Leaf SemiPro HS >> - Kvaser Leaf Professional LS >> - Kvaser Leaf Professional SWC >> - Kvaser Leaf Professional LIN >> - Kvaser Leaf SemiPro LS >> - Kvaser Leaf SemiPro SWC >> - Kvaser Memorator II, Prototype >> - Kvaser Memorator II HS/HS >> - Kvaser USBcan Professional HS/HS >> - Kvaser Leaf Light GI >> - Kvaser Leaf Professional HS (OBD-II connector) >> - Kvaser Memorator Professional HS/LS >> - Kvaser Leaf Light "China" >> - Kvaser BlackBird SemiPro >> - Kvaser OEM Mercury >> - Kvaser OEM Leaf >> - Kvaser USBcan R >> >> Signed-off-by: Olivier Sobrie <olivier@xxxxxxxxx> >> --- >> Changes since v3: >> - add support for CMD_LOG_MESSAGE >> >> drivers/net/can/usb/Kconfig | 33 + >> drivers/net/can/usb/Makefile | 1 + >> drivers/net/can/usb/kvaser_usb.c | 1555 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 1589 insertions(+) >> create mode 100644 drivers/net/can/usb/kvaser_usb.c >> >> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig >> index 0a68768..578955f 100644 >> --- a/drivers/net/can/usb/Kconfig >> +++ b/drivers/net/can/usb/Kconfig >> @@ -13,6 +13,39 @@ config CAN_ESD_USB2 >> This driver supports the CAN-USB/2 interface >> from esd electronic system design gmbh (http://www.esd.eu). >> >> +config CAN_KVASER_USB >> + tristate "Kvaser CAN/USB interface" >> + ---help--- >> + This driver adds support for Kvaser CAN/USB devices like Kvaser >> + Leaf Light. >> + >> + The driver gives support for the following devices: >> + - Kvaser Leaf prototype (P010v2 and v3) >> + - Kvaser Leaf Light (P010v3) >> + - Kvaser Leaf Professional HS >> + - Kvaser Leaf SemiPro HS >> + - Kvaser Leaf Professional LS >> + - Kvaser Leaf Professional SWC >> + - Kvaser Leaf Professional LIN >> + - Kvaser Leaf SemiPro LS >> + - Kvaser Leaf SemiPro SWC >> + - Kvaser Memorator II, Prototype >> + - Kvaser Memorator II HS/HS >> + - Kvaser USBcan Professional HS/HS >> + - Kvaser Leaf Light GI >> + - Kvaser Leaf Professional HS (OBD-II connector) >> + - Kvaser Memorator Professional HS/LS >> + - Kvaser Leaf Light "China" >> + - Kvaser BlackBird SemiPro >> + - Kvaser OEM Mercury >> + - Kvaser OEM Leaf >> + - Kvaser USBcan R >> + >> + If unsure, say N. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called kvaser_usb. >> + >> config CAN_PEAK_USB >> tristate "PEAK PCAN-USB/USB Pro interfaces" >> ---help--- >> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile >> index da6d1d3..80a2ee4 100644 >> --- a/drivers/net/can/usb/Makefile >> +++ b/drivers/net/can/usb/Makefile >> @@ -4,6 +4,7 @@ >> >> obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o >> obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o >> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o >> obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/ >> >> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG >> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c >> new file mode 100644 >> index 0000000..3509ca5 >> --- /dev/null >> +++ b/drivers/net/can/usb/kvaser_usb.c >> @@ -0,0 +1,1555 @@ >> +/* >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation version 2. >> + * >> + * Parts of this driver are based on the following: >> + * - Kvaser linux leaf driver (version 4.78) >> + * - CAN driver for esd CAN-USB/2 >> + * >> + * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved. >> + * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@xxxxxx>, esd gmbh >> + * Copyright (C) 2012 Olivier Sobrie <olivier@xxxxxxxxx> >> + */ >> + >> +#include <linux/init.h> >> +#include <linux/completion.h> >> +#include <linux/module.h> >> +#include <linux/netdevice.h> >> +#include <linux/usb.h> >> + >> +#include <linux/can.h> >> +#include <linux/can/dev.h> >> +#include <linux/can/error.h> >> + >> +#define MAX_TX_URBS 16 >> +#define MAX_RX_URBS 4 >> +#define START_TIMEOUT 1000 /* msecs */ >> +#define STOP_TIMEOUT 1000 /* msecs */ >> +#define USB_SEND_TIMEOUT 1000 /* msecs */ >> +#define USB_RECV_TIMEOUT 1000 /* msecs */ >> +#define RX_BUFFER_SIZE 3072 >> +#define CAN_USB_CLOCK 8000000 >> +#define MAX_NET_DEVICES 3 >> + >> +/* Kvaser USB devices */ >> +#define KVASER_VENDOR_ID 0x0bfd >> +#define USB_LEAF_DEVEL_PRODUCT_ID 10 >> +#define USB_LEAF_LITE_PRODUCT_ID 11 >> +#define USB_LEAF_PRO_PRODUCT_ID 12 >> +#define USB_LEAF_SPRO_PRODUCT_ID 14 >> +#define USB_LEAF_PRO_LS_PRODUCT_ID 15 >> +#define USB_LEAF_PRO_SWC_PRODUCT_ID 16 >> +#define USB_LEAF_PRO_LIN_PRODUCT_ID 17 >> +#define USB_LEAF_SPRO_LS_PRODUCT_ID 18 >> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID 19 >> +#define USB_MEMO2_DEVEL_PRODUCT_ID 22 >> +#define USB_MEMO2_HSHS_PRODUCT_ID 23 >> +#define USB_UPRO_HSHS_PRODUCT_ID 24 >> +#define USB_LEAF_LITE_GI_PRODUCT_ID 25 >> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID 26 >> +#define USB_MEMO2_HSLS_PRODUCT_ID 27 >> +#define USB_LEAF_LITE_CH_PRODUCT_ID 28 >> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID 29 >> +#define USB_OEM_MERCURY_PRODUCT_ID 34 >> +#define USB_OEM_LEAF_PRODUCT_ID 35 >> +#define USB_CAN_R_PRODUCT_ID 39 >> + >> +/* USB devices features */ >> +#define KVASER_HAS_SILENT_MODE BIT(0) >> +#define KVASER_HAS_TXRX_ERRORS BIT(1) >> + >> +/* Message header size */ >> +#define MSG_HEADER_LEN 2 >> + >> +/* Can message flags */ >> +#define MSG_FLAG_ERROR_FRAME BIT(0) >> +#define MSG_FLAG_OVERRUN BIT(1) >> +#define MSG_FLAG_NERR BIT(2) >> +#define MSG_FLAG_WAKEUP BIT(3) >> +#define MSG_FLAG_REMOTE_FRAME BIT(4) >> +#define MSG_FLAG_RESERVED BIT(5) >> +#define MSG_FLAG_TX_ACK BIT(6) >> +#define MSG_FLAG_TX_REQUEST BIT(7) >> + >> +/* Can states */ >> +#define M16C_STATE_BUS_RESET BIT(0) >> +#define M16C_STATE_BUS_ERROR BIT(4) >> +#define M16C_STATE_BUS_PASSIVE BIT(5) >> +#define M16C_STATE_BUS_OFF BIT(6) >> + >> +/* Can msg ids */ >> +#define CMD_RX_STD_MESSAGE 12 >> +#define CMD_TX_STD_MESSAGE 13 >> +#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 >> +#define CMD_GET_CTRL_MODE 22 >> +#define CMD_GET_CTRL_MODE_REPLY 23 >> +#define CMD_RESET_CHIP 24 >> +#define CMD_RESET_CARD 25 >> +#define CMD_START_CHIP 26 >> +#define CMD_START_CHIP_REPLY 27 >> +#define CMD_STOP_CHIP 28 >> +#define CMD_STOP_CHIP_REPLY 29 >> +#define CMD_GET_CARD_INFO2 32 >> +#define CMD_GET_CARD_INFO 34 >> +#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_RESET_ERROR_COUNTER 49 >> +#define CMD_TX_ACKNOWLEDGE 50 >> +#define CMD_CAN_ERROR_EVENT 51 >> +#define CMD_USB_THROTTLE 77 >> +#define CMD_LOG_MESSAGE 106 >> + >> +/* error factors */ >> +#define M16C_EF_ACKE BIT(0) >> +#define M16C_EF_CRCE BIT(1) >> +#define M16C_EF_FORME BIT(2) >> +#define M16C_EF_STFE BIT(3) >> +#define M16C_EF_BITE0 BIT(4) >> +#define M16C_EF_BITE1 BIT(5) >> +#define M16C_EF_RCVE BIT(6) >> +#define M16C_EF_TRE BIT(7) >> + >> +/* bittiming parameters */ >> +#define KVASER_USB_TSEG1_MIN 1 >> +#define KVASER_USB_TSEG1_MAX 16 >> +#define KVASER_USB_TSEG2_MIN 1 >> +#define KVASER_USB_TSEG2_MAX 8 >> +#define KVASER_USB_SJW_MAX 4 >> +#define KVASER_USB_BRP_MIN 1 >> +#define KVASER_USB_BRP_MAX 64 >> +#define KVASER_USB_BRP_INC 1 >> + >> +/* ctrl modes */ >> +#define KVASER_CTRL_MODE_NORMAL 1 >> +#define KVASER_CTRL_MODE_SILENT 2 >> +#define KVASER_CTRL_MODE_SELFRECEPTION 3 >> +#define KVASER_CTRL_MODE_OFF 4 >> + >> +struct kvaser_msg_simple { >> + u8 tid; >> + u8 channel; >> +} __packed; >> + >> +struct kvaser_msg_cardinfo { >> + u8 tid; >> + u8 nchannels; >> + __le32 serial_number; >> + __le32 padding; >> + __le32 clock_resolution; >> + __le32 mfgdate; >> + u8 ean[8]; >> + u8 hw_revision; >> + u8 usb_hs_mode; >> + __le16 padding2; >> +} __packed; >> + >> +struct kvaser_msg_cardinfo2 { >> + u8 tid; >> + u8 channel; >> + u8 pcb_id[24]; >> + __le32 oem_unlock_code; >> +} __packed; >> + >> +struct kvaser_msg_softinfo { >> + u8 tid; >> + u8 channel; >> + __le32 sw_options; >> + __le32 fw_version; >> + __le16 max_outstanding_tx; >> + __le16 padding[9]; >> +} __packed; >> + >> +struct kvaser_msg_busparams { >> + u8 tid; >> + u8 channel; >> + __le32 bitrate; >> + u8 tseg1; >> + u8 tseg2; >> + u8 sjw; >> + u8 no_samp; >> +} __packed; >> + >> +struct kvaser_msg_tx_can { >> + u8 channel; >> + u8 tid; >> + u8 msg[14]; >> + u8 padding; >> + u8 flags; >> +} __packed; >> + >> +struct kvaser_msg_rx_can { >> + u8 channel; >> + u8 flag; >> + __le16 time[3]; >> + u8 msg[14]; >> +} __packed; >> + >> +struct kvaser_msg_chip_state_event { >> + u8 tid; >> + u8 channel; >> + __le16 time[3]; >> + u8 tx_errors_count; >> + u8 rx_errors_count; >> + u8 status; >> + u8 padding[3]; >> +} __packed; >> + >> +struct kvaser_msg_tx_acknowledge { >> + u8 channel; >> + u8 tid; >> + __le16 time[3]; >> + u8 flags; >> + u8 time_offset; >> +} __packed; >> + >> +struct kvaser_msg_error_event { >> + u8 tid; >> + u8 flags; >> + __le16 time[3]; >> + u8 channel; >> + u8 padding; >> + u8 tx_errors_count; >> + u8 rx_errors_count; >> + u8 status; >> + u8 error_factor; >> +} __packed; >> + >> +struct kvaser_msg_ctrl_mode { >> + u8 tid; >> + u8 channel; >> + u8 ctrl_mode; >> + u8 padding[3]; >> +} __packed; >> + >> +struct kvaser_msg_flush_queue { >> + u8 tid; >> + u8 channel; >> + u8 flags; >> + u8 padding[3]; >> +} __packed; >> + >> +struct kvaser_msg_log_message { >> + u8 channel; >> + u8 flags; >> + __le16 time[3]; >> + u8 dlc; >> + u8 time_offset; >> + __le32 id; >> + u8 data[8]; >> +} __packed; >> + >> +struct kvaser_msg { >> + u8 len; >> + u8 id; >> + union { >> + struct kvaser_msg_simple simple; >> + struct kvaser_msg_cardinfo cardinfo; >> + struct kvaser_msg_cardinfo2 cardinfo2; >> + struct kvaser_msg_softinfo softinfo; >> + struct kvaser_msg_busparams busparams; >> + struct kvaser_msg_tx_can tx_can; >> + struct kvaser_msg_rx_can rx_can; >> + struct kvaser_msg_chip_state_event chip_state_event; >> + struct kvaser_msg_tx_acknowledge tx_acknowledge; >> + struct kvaser_msg_error_event error_event; >> + struct kvaser_msg_ctrl_mode ctrl_mode; >> + struct kvaser_msg_flush_queue flush_queue; >> + struct kvaser_msg_log_message log_message; >> + } u; >> +} __packed; >> + >> +struct kvaser_usb_tx_urb_context { >> + struct kvaser_usb_net_priv *priv; >> + u32 echo_index; >> + int dlc; >> +}; >> + >> +struct kvaser_usb { >> + struct usb_device *udev; >> + struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES]; >> + >> + struct usb_endpoint_descriptor *bulk_in, *bulk_out; >> + struct usb_anchor rx_submitted; >> + >> + u32 fw_version; >> + unsigned int nchannels; >> + >> + bool rxinitdone; >> + void *rxbuf[MAX_RX_URBS]; >> + dma_addr_t rxbuf_dma[MAX_RX_URBS]; >> +}; >> + >> +struct kvaser_usb_net_priv { >> + struct can_priv can; >> + >> + atomic_t active_tx_urbs; >> + struct usb_anchor tx_submitted; >> + struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS]; >> + >> + struct completion start_comp, stop_comp; >> + >> + struct kvaser_usb *dev; >> + struct net_device *netdev; >> + int channel; >> + >> + struct can_berr_counter bec; >> +}; >> + >> +static struct usb_device_id kvaser_usb_table[] = { >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS | >> + KVASER_HAS_SILENT_MODE }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID), >> + .driver_info = KVASER_HAS_TXRX_ERRORS }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table); >> + >> +static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev, >> + struct kvaser_msg *msg) >> +{ >> + int actual_len; >> + >> + return usb_bulk_msg(dev->udev, >> + usb_sndbulkpipe(dev->udev, >> + dev->bulk_out->bEndpointAddress), >> + msg, msg->len, &actual_len, >> + USB_SEND_TIMEOUT); >> +} >> + >> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id, >> + struct kvaser_msg *msg) >> +{ >> + struct kvaser_msg *tmp; >> + void *buf; >> + int actual_len; >> + int err; >> + int pos = 0; >> + >> + buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL); >> + if (!buf) >> + return -ENOMEM; >> + >> + err = usb_bulk_msg(dev->udev, >> + usb_rcvbulkpipe(dev->udev, >> + dev->bulk_in->bEndpointAddress), >> + buf, RX_BUFFER_SIZE, &actual_len, >> + USB_RECV_TIMEOUT); >> + if (err < 0) >> + goto end; >> + >> + while (pos <= actual_len - MSG_HEADER_LEN) { >> + tmp = buf + pos; >> + >> + if (!tmp->len) >> + break; >> + >> + if (pos + tmp->len > actual_len) { >> + dev_err(dev->udev->dev.parent, "Format error\n"); >> + break; >> + } >> + >> + if (tmp->id == id) { >> + memcpy(msg, tmp, tmp->len); >> + goto end; >> + } >> + >> + pos += tmp->len; >> + } >> + >> + err = -EINVAL; >> + >> +end: >> + kfree(buf); >> + >> + return err; >> +} >> + >> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev, >> + u8 msg_id, int channel) >> +{ >> + struct kvaser_msg msg = { >> + .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple), >> + .id = msg_id, >> + .u.simple.channel = channel, >> + .u.simple.tid = 0xff, >> + }; >> + >> + return kvaser_usb_send_msg(dev, &msg); >> +} >> + >> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev) >> +{ >> + struct kvaser_msg msg; >> + int err; >> + >> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0); >> + if (err) >> + return err; >> + >> + err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg); >> + if (err) >> + return err; >> + >> + dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version); >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev) >> +{ >> + struct kvaser_msg msg; >> + int err; >> + >> + err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0); >> + if (err) >> + return err; >> + >> + err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg); >> + if (err) >> + return err; >> + >> + dev->nchannels = msg.u.cardinfo.nchannels; >> + >> + return 0; >> +} >> + >> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + struct net_device_stats *stats; >> + struct kvaser_usb_tx_urb_context *context; >> + struct kvaser_usb_net_priv *priv; >> + struct sk_buff *skb; >> + struct can_frame *cf; >> + u8 channel = msg->u.tx_acknowledge.channel; >> + u8 tid = msg->u.tx_acknowledge.tid; >> + >> + if (channel >= dev->nchannels) { >> + dev_err(dev->udev->dev.parent, >> + "Invalid channel number (%d)\n", channel); >> + return; >> + } >> + >> + priv = dev->nets[channel]; >> + >> + if (!netif_device_present(priv->netdev)) >> + return; >> + >> + stats = &priv->netdev->stats; >> + >> + context = &priv->tx_contexts[tid % 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)) { >> + skb = alloc_can_err_skb(priv->netdev, &cf); >> + if (skb) { >> + cf->can_id |= CAN_ERR_RESTARTED; >> + netif_rx(skb); >> + >> + stats->rx_packets++; >> + stats->rx_bytes += cf->can_dlc; >> + } else { >> + netdev_err(priv->netdev, >> + "No memory left for err_skb\n"); >> + } >> + >> + priv->can.can_stats.restarts++; >> + netif_carrier_on(priv->netdev); >> + >> + priv->can.state = CAN_STATE_ERROR_ACTIVE; >> + } >> + >> + stats->tx_packets++; >> + stats->tx_bytes += context->dlc; >> + can_get_echo_skb(priv->netdev, context->echo_index); >> + >> + context->echo_index = MAX_TX_URBS; >> + atomic_dec(&priv->active_tx_urbs); >> + >> + netif_wake_queue(priv->netdev); >> +} >> + >> +static void kvaser_usb_simple_msg_callback(struct urb *urb) >> +{ >> + struct net_device *netdev = urb->context; >> + >> + kfree(urb->transfer_buffer); >> + >> + if (urb->status) >> + netdev_warn(netdev, "urb status received: %d\n", >> + urb->status); >> +} >> + >> +static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv, >> + u8 msg_id) >> +{ >> + struct kvaser_usb *dev = priv->dev; >> + struct net_device *netdev = priv->netdev; >> + struct kvaser_msg *msg; >> + struct urb *urb; >> + void *buf; >> + int err; >> + >> + urb = usb_alloc_urb(0, GFP_ATOMIC); >> + if (!urb) { >> + netdev_err(netdev, "No memory left for URBs\n"); >> + return -ENOMEM; >> + } >> + >> + buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC); >> + if (!buf) { > > Do you have to free the usb you just allocated? > >> + netdev_err(netdev, "No memory left for USB buffer\n"); >> + return -ENOMEM; >> + } >> + >> + msg = (struct kvaser_msg *)buf; >> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple); >> + msg->id = msg_id; >> + msg->u.simple.channel = priv->channel; >> + >> + usb_fill_bulk_urb(urb, dev->udev, >> + usb_sndbulkpipe(dev->udev, >> + dev->bulk_out->bEndpointAddress), >> + buf, msg->len, >> + kvaser_usb_simple_msg_callback, priv); >> + usb_anchor_urb(urb, &priv->tx_submitted); >> + >> + err = usb_submit_urb(urb, GFP_ATOMIC); >> + if (err) { >> + netdev_err(netdev, "Error transmitting URB\n"); >> + usb_unanchor_urb(urb); >> + kfree(buf); >> + return err; > > and here? > >> + } >> + >> + usb_free_urb(urb); >> + >> + return 0; >> +} >> + >> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv) >> +{ >> + int i; >> + >> + usb_kill_anchored_urbs(&priv->tx_submitted); >> + atomic_set(&priv->active_tx_urbs, 0); >> + >> + for (i = 0; i < MAX_TX_URBS; i++) >> + priv->tx_contexts[i].echo_index = MAX_TX_URBS; >> +} >> + >> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + struct can_frame *cf; >> + struct sk_buff *skb; >> + struct net_device_stats *stats; >> + struct kvaser_usb_net_priv *priv; >> + unsigned int new_state; >> + u8 channel, status, txerr, rxerr, error_factor; >> + >> + switch (msg->id) { >> + case CMD_CAN_ERROR_EVENT: >> + channel = msg->u.error_event.channel; >> + status = msg->u.error_event.status; >> + txerr = msg->u.error_event.tx_errors_count; >> + rxerr = msg->u.error_event.rx_errors_count; >> + error_factor = msg->u.error_event.error_factor; >> + break; >> + case CMD_LOG_MESSAGE: >> + channel = msg->u.log_message.channel; >> + status = msg->u.log_message.data[0]; >> + txerr = msg->u.log_message.data[2]; >> + rxerr = msg->u.log_message.data[3]; >> + error_factor = msg->u.log_message.data[1]; >> + break; >> + case CMD_CHIP_STATE_EVENT: >> + channel = msg->u.chip_state_event.channel; >> + status = msg->u.chip_state_event.status; >> + txerr = msg->u.chip_state_event.tx_errors_count; >> + rxerr = msg->u.chip_state_event.rx_errors_count; >> + error_factor = 0; >> + break; >> + default: >> + dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n", >> + msg->id); >> + return; >> + } >> + >> + if (channel >= dev->nchannels) { >> + dev_err(dev->udev->dev.parent, >> + "Invalid channel number (%d)\n", channel); >> + return; >> + } >> + >> + priv = dev->nets[channel]; >> + stats = &priv->netdev->stats; >> + >> + if (status & M16C_STATE_BUS_RESET) { >> + kvaser_usb_unlink_tx_urbs(priv); >> + return; >> + } >> + >> + skb = alloc_can_err_skb(priv->netdev, &cf); >> + if (!skb) { >> + stats->rx_dropped++; >> + return; >> + } >> + >> + cf->can_id |= CAN_ERR_BUSERROR; At state change is *not* a bus error. The line above should e moved into the "if (error_factor)" block below. >> + >> + new_state = priv->can.state; >> + >> + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status); >> + >> + if (status & M16C_STATE_BUS_OFF) { >> + cf->can_id |= CAN_ERR_BUSOFF; >> + >> + priv->can.can_stats.bus_off++; >> + if (!priv->can.restart_ms) >> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP); >> + >> + netif_carrier_off(priv->netdev); >> + >> + new_state = CAN_STATE_BUS_OFF; >> + } >> + >> + if (status & M16C_STATE_BUS_PASSIVE) { > > else if () > > as bus passive and bus off is mutually exclusive. > >> + if (priv->can.state != CAN_STATE_ERROR_PASSIVE) { >> + cf->can_id |= CAN_ERR_CRTL; >> + >> + if ((txerr > 0) || (rxerr > 0)) if (txerr | rxerr) ? >> + cf->data[1] = (txerr > rxerr) >> + ? CAN_ERR_CRTL_TX_PASSIVE >> + : CAN_ERR_CRTL_RX_PASSIVE; >> + else >> + cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE | >> + CAN_ERR_CRTL_RX_PASSIVE; >> + >> + priv->can.can_stats.error_passive++; >> + } >> + >> + new_state = CAN_STATE_ERROR_PASSIVE; >> + } >> + >> + if (status == M16C_STATE_BUS_ERROR) { >> + if ((priv->can.state < CAN_STATE_ERROR_WARNING) && >> + ((txerr > 96) || (rxerr > 96))) { > > Is is >= 96 ? Yep. >> + cf->can_id |= CAN_ERR_CRTL; >> + cf->data[1] = (txerr > rxerr) >> + ? CAN_ERR_CRTL_TX_WARNING >> + : CAN_ERR_CRTL_RX_WARNING; >> + >> + priv->can.can_stats.error_warning++; >> + new_state = CAN_STATE_ERROR_WARNING; >> + } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) { >> + cf->can_id |= CAN_ERR_PROT; >> + cf->data[2] = CAN_ERR_PROT_ACTIVE; >> + >> + new_state = CAN_STATE_ERROR_ACTIVE; >> + } >> + } >> + >> + if (!status) { >> + cf->can_id |= CAN_ERR_PROT; >> + cf->data[2] = CAN_ERR_PROT_ACTIVE; >> + >> + new_state = CAN_STATE_ERROR_ACTIVE; >> + } >> + >> + if (priv->can.restart_ms && >> + (priv->can.state >= CAN_STATE_BUS_OFF) && >> + (new_state < CAN_STATE_BUS_OFF)) { >> + cf->can_id |= CAN_ERR_RESTARTED; >> + netif_carrier_on(priv->netdev); >> + >> + priv->can.can_stats.restarts++; >> + } >> + >> + if (error_factor) { >> + priv->can.can_stats.bus_error++; >> + stats->rx_errors++; >> + >> + cf->can_id |= CAN_ERR_PROT; >> + >> + if (error_factor & M16C_EF_ACKE) >> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK); >> + if (error_factor & M16C_EF_CRCE) >> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ | >> + CAN_ERR_PROT_LOC_CRC_DEL); >> + if (error_factor & M16C_EF_FORME) >> + cf->data[2] |= CAN_ERR_PROT_FORM; >> + if (error_factor & M16C_EF_STFE) >> + cf->data[2] |= CAN_ERR_PROT_STUFF; >> + if (error_factor & M16C_EF_BITE0) >> + cf->data[2] |= CAN_ERR_PROT_BIT0; >> + if (error_factor & M16C_EF_BITE1) >> + cf->data[2] |= CAN_ERR_PROT_BIT1; >> + if (error_factor & M16C_EF_TRE) >> + cf->data[2] |= CAN_ERR_PROT_TX; >> + } >> + >> + cf->data[6] = txerr; >> + cf->data[7] = rxerr; >> + >> + priv->bec.txerr = txerr; >> + priv->bec.rxerr = rxerr; >> + >> + priv->can.state = new_state; >> + >> + netif_rx(skb); >> + >> + stats->rx_packets++; >> + stats->rx_bytes += cf->can_dlc; >> +} >> + >> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + struct kvaser_usb_net_priv *priv; >> + struct can_frame *cf; >> + struct sk_buff *skb; >> + struct net_device_stats *stats; >> + u8 channel = msg->u.rx_can.channel; >> + >> + if (channel >= dev->nchannels) { >> + dev_err(dev->udev->dev.parent, >> + "Invalid channel number (%d)\n", channel); >> + return; >> + } >> + >> + priv = dev->nets[channel]; >> + stats = &priv->netdev->stats; >> + >> + skb = alloc_can_skb(priv->netdev, &cf); >> + if (!skb) { >> + stats->tx_dropped++; >> + return; >> + } >> + >> + cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) | >> + (msg->u.rx_can.msg[1] & 0x3f); >> + cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]); >> + >> + if (msg->id == CMD_RX_EXT_MESSAGE) { >> + cf->can_id <<= 18; >> + cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) | >> + ((msg->u.rx_can.msg[3] & 0xff) << 6) | >> + (msg->u.rx_can.msg[4] & 0x3f); >> + cf->can_id |= CAN_EFF_FLAG; >> + } >> + >> + if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) { >> + cf->can_id |= CAN_RTR_FLAG; >> + } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME | >> + MSG_FLAG_NERR)) { >> + cf->can_id |= CAN_ERR_FLAG; >> + cf->can_dlc = CAN_ERR_DLC; > Please move the error skb creation handling into a subfunction, use > can_alloc_err_skb() in that function. Please move the: > > if (msg->u.rx_can.flag & ...) > > up in this function, before the alloc_can_skb(). > >> + >> + netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n", >> + msg->u.rx_can.flag); >> + >> + stats->rx_errors++; This an *error* which does normally not happen, IIRC. Therefore I do not see a need to pass it to the user as error message. >> + } else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) { >> + cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL; >> + cf->can_dlc = CAN_ERR_DLC; >> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; >> + >> + stats->rx_over_errors++; >> + stats->rx_errors++; > > This should go into the error skb generation function, too. > >> + } else if (!msg->u.rx_can.flag) { >> + memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc); > > Please don't copy the contents of RTR frames. > >> + } else { >> + kfree_skb(skb); > > After you have moved the error skb generation into a seperate function, > you should get rid of the kfree(skb), too. The function should look like > this (pseude code): > > static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev, > const struct kvaser_msg *msg) > { > if (channel_invalid()) > return; > > if (msg->u.rx_can.flag & ERROR) { > kvaser_usb_rx_can_err_msg(); > return; > } else if (msg->u.rx_can.flag & INVALID_FRAME) { > return; > } > > skb = alloc_can_skb(); > > /* existing dlc, rtr and data handling code */ > ... > } > > >> + return; >> + } >> + >> + netif_rx(skb); >> + >> + stats->rx_packets++; >> + stats->rx_bytes += cf->can_dlc; >> +} >> + >> +static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + struct kvaser_usb_net_priv *priv; >> + u8 channel = msg->u.simple.channel; >> + >> + if (channel >= dev->nchannels) { >> + dev_err(dev->udev->dev.parent, >> + "Invalid channel number (%d)\n", channel); >> + return; >> + } >> + >> + priv = dev->nets[channel]; >> + >> + if (completion_done(&priv->start_comp) && >> + netif_queue_stopped(priv->netdev)) { >> + netif_wake_queue(priv->netdev); >> + } else { >> + netif_start_queue(priv->netdev); >> + complete(&priv->start_comp); >> + } >> +} >> + >> +static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + struct kvaser_usb_net_priv *priv; >> + u8 channel = msg->u.simple.channel; >> + >> + if (channel >= dev->nchannels) { >> + dev_err(dev->udev->dev.parent, >> + "Invalid channel number (%d)\n", channel); >> + return; >> + } >> + >> + priv = dev->nets[channel]; >> + >> + complete(&priv->stop_comp); >> +} >> + >> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev, >> + const struct kvaser_msg *msg) >> +{ >> + switch (msg->id) { >> + case CMD_START_CHIP_REPLY: >> + kvaser_usb_start_chip_reply(dev, msg); >> + break; >> + >> + case CMD_STOP_CHIP_REPLY: >> + kvaser_usb_stop_chip_reply(dev, msg); >> + break; >> + >> + case CMD_RX_STD_MESSAGE: >> + case CMD_RX_EXT_MESSAGE: >> + kvaser_usb_rx_can_msg(dev, msg); >> + break; >> + >> + case CMD_CHIP_STATE_EVENT: >> + case CMD_CAN_ERROR_EVENT: >> + kvaser_usb_rx_error(dev, msg); >> + break; >> + >> + case CMD_LOG_MESSAGE: >> + if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME) >> + kvaser_usb_rx_error(dev, msg); >> + break; >> + >> + case CMD_TX_ACKNOWLEDGE: >> + kvaser_usb_tx_acknowledge(dev, msg); >> + break; >> + >> + default: >> + dev_warn(dev->udev->dev.parent, >> + "Unhandled message (%d)\n", msg->id); >> + break; >> + } >> +} >> + >> +static void kvaser_usb_read_bulk_callback(struct urb *urb) >> +{ >> + struct kvaser_usb *dev = urb->context; >> + struct kvaser_msg *msg; >> + int pos = 0; >> + int err, i; >> + >> + switch (urb->status) { >> + case 0: >> + break; >> + case -ENOENT: >> + case -ESHUTDOWN: >> + return; >> + default: >> + dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n", >> + urb->status); >> + goto resubmit_urb; >> + } >> + >> + while (pos <= urb->actual_length - MSG_HEADER_LEN) { >> + msg = urb->transfer_buffer + pos; >> + >> + if (!msg->len) >> + break; >> + >> + if (pos + msg->len > urb->actual_length) { >> + dev_err(dev->udev->dev.parent, "Format error\n"); >> + break; >> + } >> + >> + kvaser_usb_handle_message(dev, msg); >> + >> + pos += msg->len; >> + } >> + >> +resubmit_urb: >> + usb_fill_bulk_urb(urb, dev->udev, >> + usb_rcvbulkpipe(dev->udev, >> + dev->bulk_in->bEndpointAddress), >> + urb->transfer_buffer, RX_BUFFER_SIZE, >> + kvaser_usb_read_bulk_callback, dev); >> + >> + err = usb_submit_urb(urb, GFP_ATOMIC); >> + if (err == -ENODEV) { >> + for (i = 0; i < dev->nchannels; i++) { >> + if (!dev->nets[i]) >> + continue; >> + >> + netif_device_detach(dev->nets[i]->netdev); >> + } >> + } else if (err) { >> + dev_err(dev->udev->dev.parent, >> + "Failed resubmitting read bulk urb: %d\n", err); >> + } >> + >> + return; >> +} >> + >> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev) >> +{ >> + int i, err = 0; >> + >> + if (dev->rxinitdone) >> + return 0; >> + >> + for (i = 0; i < MAX_RX_URBS; i++) { >> + struct urb *urb = NULL; >> + u8 *buf = NULL; >> + dma_addr_t buf_dma; >> + >> + urb = usb_alloc_urb(0, GFP_KERNEL); >> + if (!urb) { >> + dev_warn(dev->udev->dev.parent, >> + "No memory left for URBs\n"); >> + err = -ENOMEM; >> + break; >> + } >> + >> + buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, >> + GFP_KERNEL, &buf_dma); >> + if (!buf) { >> + dev_warn(dev->udev->dev.parent, >> + "No memory left for USB buffer\n"); >> + usb_free_urb(urb); >> + err = -ENOMEM; >> + break; >> + } >> + >> + usb_fill_bulk_urb(urb, dev->udev, >> + usb_rcvbulkpipe(dev->udev, >> + dev->bulk_in->bEndpointAddress), >> + buf, RX_BUFFER_SIZE, >> + kvaser_usb_read_bulk_callback, >> + dev); >> + urb->transfer_dma = buf_dma; >> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; >> + usb_anchor_urb(urb, &dev->rx_submitted); >> + >> + err = usb_submit_urb(urb, GFP_KERNEL); >> + if (err) { >> + usb_unanchor_urb(urb); >> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf, >> + buf_dma); > > Do you have to call usb_free_usb() here, or does the usb frame work take > care of this? > >> + break; >> + } >> + >> + dev->rxbuf[i] = buf; >> + dev->rxbuf_dma[i] = buf_dma; >> + >> + usb_free_urb(urb); >> + } >> + >> + if (i == 0) { >> + dev_warn(dev->udev->dev.parent, >> + "Cannot setup read URBs, error %d\n", err); >> + return err; >> + } else if (i < MAX_RX_URBS) { >> + dev_warn(dev->udev->dev.parent, >> + "RX performances may be slow\n"); >> + } >> + >> + dev->rxinitdone = true; >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv) >> +{ >> + struct kvaser_msg msg = { >> + .id = CMD_SET_CTRL_MODE, >> + .len = MSG_HEADER_LEN + >> + sizeof(struct kvaser_msg_ctrl_mode), >> + .u.ctrl_mode.tid = 0xff, >> + .u.ctrl_mode.channel = priv->channel, >> + }; >> + >> + if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) >> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT; >> + else >> + msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL; >> + >> + return kvaser_usb_send_msg(priv->dev, &msg); >> +} >> + >> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv) >> +{ >> + int err; >> + >> + init_completion(&priv->start_comp); >> + >> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP, >> + priv->channel); >> + if (err) >> + return err; >> + >> + if (!wait_for_completion_timeout(&priv->start_comp, >> + msecs_to_jiffies(START_TIMEOUT))) >> + return -ETIMEDOUT; >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_open(struct net_device *netdev) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + struct kvaser_usb *dev = priv->dev; >> + int err; >> + >> + err = open_candev(netdev); >> + if (err) >> + return err; >> + >> + err = kvaser_usb_setup_rx_urbs(dev); >> + if (err) >> + goto error; >> + >> + err = kvaser_usb_set_opt_mode(priv); >> + if (err) >> + goto error; >> + >> + err = kvaser_usb_start_chip(priv); >> + if (err) { >> + netdev_warn(netdev, "Cannot start device, error %d\n", err); >> + goto error; >> + } >> + >> + priv->can.state = CAN_STATE_ERROR_ACTIVE; >> + >> + return 0; >> + >> +error: >> + close_candev(netdev); >> + return err; >> +} >> + >> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev) >> +{ >> + int i; >> + >> + usb_kill_anchored_urbs(&dev->rx_submitted); >> + >> + for (i = 0; i < MAX_RX_URBS; i++) >> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, >> + dev->rxbuf[i], >> + dev->rxbuf_dma[i]); >> + >> + for (i = 0; i < MAX_NET_DEVICES; i++) { >> + struct kvaser_usb_net_priv *priv = dev->nets[i]; >> + >> + if (priv) >> + kvaser_usb_unlink_tx_urbs(priv); >> + } >> +} >> + >> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv) >> +{ >> + int err; >> + >> + init_completion(&priv->stop_comp); >> + >> + err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP, >> + priv->channel); >> + if (err) >> + return err; >> + >> + if (!wait_for_completion_timeout(&priv->stop_comp, >> + msecs_to_jiffies(STOP_TIMEOUT))) >> + return -ETIMEDOUT; >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv) >> +{ >> + struct kvaser_msg msg = { >> + .id = CMD_FLUSH_QUEUE, >> + .len = MSG_HEADER_LEN + >> + sizeof(struct kvaser_msg_flush_queue), >> + .u.flush_queue.channel = priv->channel, >> + .u.flush_queue.flags = 0x00, >> + }; >> + >> + return kvaser_usb_send_msg(priv->dev, &msg); >> +} >> + >> +static int kvaser_usb_close(struct net_device *netdev) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + struct kvaser_usb *dev = priv->dev; >> + int err; >> + >> + netif_stop_queue(netdev); >> + >> + err = kvaser_usb_flush_queue(priv); >> + if (err) >> + netdev_warn(netdev, "Cannot flush queue, error %d\n", err); >> + >> + if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel)) >> + netdev_warn(netdev, "Cannot reset card, error %d\n", err); >> + >> + err = kvaser_usb_stop_chip(priv); >> + if (err) >> + netdev_warn(netdev, "Cannot stop device, error %d\n", err); >> + >> + priv->can.state = CAN_STATE_STOPPED; >> + close_candev(priv->netdev); >> + >> + return 0; >> +} >> + >> +static void kvaser_usb_write_bulk_callback(struct urb *urb) >> +{ >> + struct kvaser_usb_tx_urb_context *context = urb->context; >> + struct kvaser_usb_net_priv *priv; >> + struct net_device *netdev; >> + >> + if (WARN_ON(!context)) >> + return; >> + >> + priv = context->priv; >> + netdev = priv->netdev; >> + >> + kfree(urb->transfer_buffer); >> + >> + if (!netif_device_present(netdev)) >> + return; >> + >> + if (urb->status) >> + netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status); >> +} >> + >> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb, >> + struct net_device *netdev) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + struct kvaser_usb *dev = priv->dev; >> + struct net_device_stats *stats = &netdev->stats; >> + struct can_frame *cf = (struct can_frame *)skb->data; >> + struct kvaser_usb_tx_urb_context *context = NULL; >> + struct urb *urb; >> + void *buf; >> + struct kvaser_msg *msg; >> + int i, err; >> + int ret = NETDEV_TX_OK; >> + >> + if (can_dropped_invalid_skb(netdev, skb)) >> + return NETDEV_TX_OK; >> + >> + urb = usb_alloc_urb(0, GFP_ATOMIC); >> + if (!urb) { >> + netdev_err(netdev, "No memory left for URBs\n"); >> + stats->tx_dropped++; >> + dev_kfree_skb(skb); >> + return NETDEV_TX_OK; >> + } >> + >> + buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC); >> + if (!buf) { >> + netdev_err(netdev, "No memory left for USB buffer\n"); >> + stats->tx_dropped++; >> + dev_kfree_skb(skb); > What about the urb? >> + goto nobufmem; >> + } >> + >> + msg = (struct kvaser_msg *)buf; > > nitpick: cast is not needed, as buf is void * > >> + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can); >> + msg->u.tx_can.flags = 0; >> + msg->u.tx_can.channel = priv->channel; >> + >> + if (cf->can_id & CAN_EFF_FLAG) { >> + msg->id = CMD_TX_EXT_MESSAGE; >> + msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f; >> + msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f; >> + msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f; >> + msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff; >> + msg->u.tx_can.msg[4] = cf->can_id & 0x3f; >> + } else { >> + msg->id = CMD_TX_STD_MESSAGE; >> + msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f; >> + msg->u.tx_can.msg[1] = cf->can_id & 0x3f; >> + } >> + >> + msg->u.tx_can.msg[5] = cf->can_dlc; >> + memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc); >> + >> + if (cf->can_id & CAN_RTR_FLAG) >> + msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME; >> + >> + for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) { >> + if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) { >> + context = &priv->tx_contexts[i]; >> + break; >> + } >> + } >> + >> + if (!context) { >> + netdev_warn(netdev, "cannot find free context\n"); >> + ret = NETDEV_TX_BUSY; >> + goto releasebuf; >> + } >> + >> + context->priv = priv; >> + context->echo_index = i; >> + context->dlc = cf->can_dlc; >> + >> + msg->u.tx_can.tid = context->echo_index; >> + >> + usb_fill_bulk_urb(urb, dev->udev, >> + usb_sndbulkpipe(dev->udev, >> + dev->bulk_out->bEndpointAddress), >> + buf, msg->len, >> + kvaser_usb_write_bulk_callback, context); >> + usb_anchor_urb(urb, &priv->tx_submitted); >> + >> + can_put_echo_skb(skb, netdev, context->echo_index); >> + >> + atomic_inc(&priv->active_tx_urbs); >> + >> + if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS) >> + netif_stop_queue(netdev); >> + >> + err = usb_submit_urb(urb, GFP_ATOMIC); >> + if (unlikely(err)) { >> + can_free_echo_skb(netdev, context->echo_index); >> + >> + atomic_dec(&priv->active_tx_urbs); >> + usb_unanchor_urb(urb); >> + >> + stats->tx_dropped++; >> + >> + if (err == -ENODEV) >> + netif_device_detach(netdev); >> + else >> + netdev_warn(netdev, "Failed tx_urb %d\n", err); >> + >> + goto releasebuf; >> + } >> + >> + netdev->trans_start = jiffies; > > Is this still needed? > >> + >> + usb_free_urb(urb); >> + >> + return NETDEV_TX_OK; >> + >> +releasebuf: >> + kfree(buf); >> +nobufmem: >> + usb_free_urb(urb); >> + return ret; >> +} >> + >> +static const struct net_device_ops kvaser_usb_netdev_ops = { >> + .ndo_open = kvaser_usb_open, >> + .ndo_stop = kvaser_usb_close, >> + .ndo_start_xmit = kvaser_usb_start_xmit, >> +}; >> + >> +static struct can_bittiming_const kvaser_usb_bittiming_const = { >> + .name = "kvaser_usb", >> + .tseg1_min = KVASER_USB_TSEG1_MIN, >> + .tseg1_max = KVASER_USB_TSEG1_MAX, >> + .tseg2_min = KVASER_USB_TSEG2_MIN, >> + .tseg2_max = KVASER_USB_TSEG2_MAX, >> + .sjw_max = KVASER_USB_SJW_MAX, >> + .brp_min = KVASER_USB_BRP_MIN, >> + .brp_max = KVASER_USB_BRP_MAX, >> + .brp_inc = KVASER_USB_BRP_INC, >> +}; >> + >> +static int kvaser_usb_set_bittiming(struct net_device *netdev) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + struct can_bittiming *bt = &priv->can.bittiming; >> + struct kvaser_usb *dev = priv->dev; >> + struct kvaser_msg msg = { >> + .id = CMD_SET_BUS_PARAMS, >> + .len = MSG_HEADER_LEN + >> + sizeof(struct kvaser_msg_busparams), >> + .u.busparams.channel = priv->channel, >> + .u.busparams.tid = 0xff, >> + .u.busparams.bitrate = cpu_to_le32(bt->bitrate), >> + .u.busparams.sjw = bt->sjw, >> + .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1, >> + .u.busparams.tseg2 = bt->phase_seg2, >> + }; >> + >> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) >> + msg.u.busparams.no_samp = 3; >> + else >> + msg.u.busparams.no_samp = 1; >> + >> + return kvaser_usb_send_msg(dev, &msg); >> +} >> + >> +static int kvaser_usb_set_mode(struct net_device *netdev, >> + enum can_mode mode) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + int err; >> + >> + switch (mode) { >> + case CAN_MODE_START: >> + err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP); >> + if (err) >> + return err; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev, >> + struct can_berr_counter *bec) >> +{ >> + struct kvaser_usb_net_priv *priv = netdev_priv(netdev); >> + >> + *bec = priv->bec; >> + >> + return 0; >> +} >> + >> +static int kvaser_usb_init_one(struct usb_interface *intf, >> + const struct usb_device_id *id, int channel) >> +{ >> + struct kvaser_usb *dev = usb_get_intfdata(intf); >> + struct net_device *netdev; >> + struct kvaser_usb_net_priv *priv; >> + int i, err; >> + >> + netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS); >> + if (!netdev) { >> + dev_err(&intf->dev, "Cannot alloc candev\n"); >> + return -ENOMEM; >> + } >> + >> + priv = netdev_priv(netdev); >> + >> + init_completion(&priv->start_comp); >> + init_completion(&priv->stop_comp); >> + >> + init_usb_anchor(&priv->tx_submitted); >> + atomic_set(&priv->active_tx_urbs, 0); >> + >> + for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) >> + priv->tx_contexts[i].echo_index = MAX_TX_URBS; >> + >> + priv->dev = dev; >> + priv->netdev = netdev; >> + priv->channel = channel; >> + >> + priv->can.state = CAN_STATE_STOPPED; >> + priv->can.clock.freq = CAN_USB_CLOCK; >> + priv->can.bittiming_const = &kvaser_usb_bittiming_const; >> + priv->can.do_set_bittiming = kvaser_usb_set_bittiming; >> + priv->can.do_set_mode = kvaser_usb_set_mode; >> + if (id->driver_info & KVASER_HAS_TXRX_ERRORS) >> + priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter; >> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES; >> + if (id->driver_info & KVASER_HAS_SILENT_MODE) >> + priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY; >> + >> + netdev->flags |= IFF_ECHO; >> + >> + netdev->netdev_ops = &kvaser_usb_netdev_ops; >> + >> + SET_NETDEV_DEV(netdev, &intf->dev); >> + >> + dev->nets[channel] = priv; >> + >> + err = register_candev(netdev); >> + if (err) { >> + dev_err(&intf->dev, "Failed to register can device\n"); >> + free_candev(netdev); >> + return err; >> + } >> + >> + netdev_dbg(netdev, "device registered\n"); >> + >> + return 0; >> +} >> + >> +static void kvaser_usb_get_endpoints(const struct usb_interface *intf, >> + struct usb_endpoint_descriptor **in, >> + struct usb_endpoint_descriptor **out) >> +{ >> + const struct usb_host_interface *iface_desc; >> + struct usb_endpoint_descriptor *endpoint; >> + int i; >> + >> + iface_desc = &intf->altsetting[0]; >> + >> + for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) { >> + endpoint = &iface_desc->endpoint[i].desc; >> + >> + if (usb_endpoint_is_bulk_in(endpoint)) >> + *in = endpoint; >> + >> + if (usb_endpoint_is_bulk_out(endpoint)) >> + *out = endpoint; >> + } >> +} >> + >> +static int kvaser_usb_probe(struct usb_interface *intf, >> + const struct usb_device_id *id) >> +{ >> + struct kvaser_usb *dev; >> + int err = -ENOMEM; >> + int i; >> + >> + dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL); >> + if (!dev) >> + return -ENOMEM; >> + >> + kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out); >> + if (!dev->bulk_in || !dev->bulk_out) { >> + dev_err(&intf->dev, "Cannot get usb endpoint(s)"); >> + return err; >> + } >> + >> + dev->udev = interface_to_usbdev(intf); >> + >> + init_usb_anchor(&dev->rx_submitted); >> + >> + usb_set_intfdata(intf, dev); >> + >> + for (i = 0; i < MAX_NET_DEVICES; i++) >> + kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i); >> + >> + err = kvaser_usb_get_software_info(dev); >> + if (err) { >> + dev_err(&intf->dev, >> + "Cannot get software infos, error %d\n", err); >> + return err; >> + } >> + >> + err = kvaser_usb_get_card_info(dev); >> + if (err) { >> + dev_err(&intf->dev, >> + "Cannot get card infos, error %d\n", err); >> + return err; >> + } >> + >> + dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n", >> + ((dev->fw_version >> 24) & 0xff), >> + ((dev->fw_version >> 16) & 0xff), >> + (dev->fw_version & 0xffff)); >> + >> + for (i = 0; i < dev->nchannels; i++) >> + kvaser_usb_init_one(intf, id, i); Error checking is not needed? >> + >> + return 0; >> +} >> + >> +static void kvaser_usb_disconnect(struct usb_interface *intf) >> +{ >> + struct kvaser_usb *dev = usb_get_intfdata(intf); >> + int i; >> + >> + usb_set_intfdata(intf, NULL); >> + >> + if (!dev) >> + return; >> + >> + for (i = 0; i < dev->nchannels; i++) { >> + if (!dev->nets[i]) >> + continue; >> + >> + unregister_netdev(dev->nets[i]->netdev); >> + } >> + >> + kvaser_usb_unlink_all_urbs(dev); >> + >> + for (i = 0; i < dev->nchannels; i++) >> + free_candev(dev->nets[i]->netdev); >> +} >> + >> +static struct usb_driver kvaser_usb_driver = { >> + .name = "kvaser_usb", >> + .probe = kvaser_usb_probe, >> + .disconnect = kvaser_usb_disconnect, >> + .id_table = kvaser_usb_table >> +}; >> + >> +module_usb_driver(kvaser_usb_driver); >> + >> +MODULE_AUTHOR("Olivier Sobrie <olivier@xxxxxxxxx>"); >> +MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices"); >> +MODULE_LICENSE("GPL v2"); Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
