| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <505DE106.6060405@grandegger.com>
Date: Sat, 22 Sep 2012 18:02:14 +0200
From: Wolfgang Grandegger <wg@...ndegger.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
CC: Olivier Sobrie <olivier@...rie.be>, linux-can@...r.kernel.org,
Kvaser Support <support@...ser.com>, netdev@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH v4] can: kvaser_usb: Add support for Kvaser CAN/USB devices
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@...rie.be>
>> ---
>> 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@....eu>, esd gmbh
>> + * Copyright (C) 2012 Olivier Sobrie <olivier@...rie.be>
>> + */
>> +
>> +#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@...rie.be>");
>> +MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices");
>> +MODULE_LICENSE("GPL v2");
Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists