* [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
@ 2012-12-02 9:25 krumboeck
2012-12-02 10:36 ` Oliver Hartkopp
2012-12-02 13:35 ` Wolfgang Grandegger
0 siblings, 2 replies; 10+ messages in thread
From: krumboeck @ 2012-12-02 9:25 UTC (permalink / raw)
To: linux-can; +Cc: info, gediminas
Add device driver for USB2CAN interface from "8 devices" (http://www.8devices.com).
Signed-off-by: Bernd Krumboeck <krumboeck@universalnet.at>
---
drivers/net/can/usb/Kconfig | 6 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/usb2can.c | 1323 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 1330 insertions(+)
create mode 100644 drivers/net/can/usb/usb2can.c
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index a4e4bee..2068c99 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -48,4 +48,10 @@ config CAN_PEAK_USB
This driver supports the PCAN-USB and PCAN-USB Pro adapters
from PEAK-System Technik (http://www.peak-system.com).
+config CAN_USB2CAN
+ tristate "8 devices USB2CAN interface"
+ ---help---
+ This driver supports the USB2CAN interface
+ from 8 devices (http://www.8devices.com).
+
endmenu
diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index 80a2ee4..3c0a378 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -6,5 +6,6 @@ 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/
+obj-$(CONFIG_CAN_USB2CAN) += usb2can.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/usb2can.c b/drivers/net/can/usb/usb2can.c
new file mode 100644
index 0000000..5a09141
--- /dev/null
+++ b/drivers/net/can/usb/usb2can.c
@@ -0,0 +1,1323 @@
+/*
+ * CAN driver for UAB "8 devices" USB2CAN converter
+ *
+ * Copyright (C) 2012 Bernd Krumboeck (krumboeck@universalnet.at)
+ *
+ * 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 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * This driver is based on the 3.2.0 version of drivers/net/can/usb/ems_usb.c
+ * and drivers/net/can/usb/esd_usb2.c
+ *
+ * Many thanks to Gerhard Bertelsmann (info@gerhard-bertelsmann.de)
+ * for testing and fixing this driver. Also many thanks to "8 devices",
+ * who were very cooperative and answered my questions.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/signal.h>
+#include <linux/slab.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>
+
+
+/* driver constants */
+#define MAX_RX_URBS 10
+#define MAX_TX_URBS 10
+#define RX_BUFFER_SIZE 64
+
+/* vendor and product id */
+#define USB2CAN_VENDOR_ID 0x0483
+#define USB2CAN_PRODUCT_ID 0x1234
+
+/* bittiming constants */
+#define USB2CAN_ABP_CLOCK 32000000
+#define USB2CAN_BAUD_MANUAL 0x09
+#define USB2CAN_TSEG1_MIN 1
+#define USB2CAN_TSEG1_MAX 16
+#define USB2CAN_TSEG2_MIN 1
+#define USB2CAN_TSEG2_MAX 8
+#define USB2CAN_SJW_MAX 4
+#define USB2CAN_BRP_MIN 1
+#define USB2CAN_BRP_MAX 1024
+#define USB2CAN_BRP_INC 1
+
+/* setup flags */
+#define USB2CAN_SILENT 0x00000001
+#define USB2CAN_LOOPBACK 0x00000002
+#define USB2CAN_DISABLE_AUTO_RESTRANS 0x00000004
+#define USB2CAN_STATUS_FRAME 0x00000008
+
+/* commands */
+#define USB2CAN_RESET 1
+#define USB2CAN_OPEN 2
+#define USB2CAN_CLOSE 3
+#define USB2CAN_SET_SPEED 4
+#define USB2CAN_SET_MASK_FILTER 5
+#define USB2CAN_GET_STATUS 6
+#define USB2CAN_GET_STATISTICS 7
+#define USB2CAN_GET_SERIAL 8
+#define USB2CAN_GET_SOFTW_VER 9
+#define USB2CAN_GET_HARDW_VER 10
+#define USB2CAN_RESET_TIMESTAMP 11
+#define USB2CAN_GET_SOFTW_HARDW_VER 12
+
+#define USB2CAN_CMD_START 0x11
+#define USB2CAN_CMD_END 0x22
+
+#define USB2CAN_CMD_SUCCESS 0
+#define USB2CAN_CMD_ERROR 255
+
+/* statistics */
+#define USB2CAN_STAT_RX_FRAMES 0
+#define USB2CAN_STAT_RX_BYTES 1
+#define USB2CAN_STAT_TX_FRAMES 2
+#define USB2CAN_STAT_TX_BYTES 3
+#define USB2CAN_STAT_OVERRUNS 4
+#define USB2CAN_STAT_WARNINGS 5
+#define USB2CAN_STAT_BUS_OFF 6
+#define USB2CAN_STAT_RESET_STAT 7
+
+/* frames */
+#define USB2CAN_DATA_START 0x55
+#define USB2CAN_DATA_END 0xAA
+
+#define USB2CAN_TYPE_CAN_FRAME 0
+#define USB2CAN_TYPE_ERROR_FRAME 3
+
+#define USB2CAN_EXTID 0x01
+#define USB2CAN_RTR 0x02
+#define USB2CAN_ERR_FLAG 0x04
+
+/* status */
+#define USB2CAN_STATUSMSG_OK 0x00 /* Normal condition. */
+#define USB2CAN_STATUSMSG_OVERRUN 0x01 /* Overrun occured when sending */
+#define USB2CAN_STATUSMSG_BUSLIGHT 0x02 /* Error counter has reached 96 */
+#define USB2CAN_STATUSMSG_BUSHEAVY 0x03 /* Error count. has reached 128 */
+#define USB2CAN_STATUSMSG_BUSOFF 0x04 /* Device is in BUSOFF */
+#define USB2CAN_STATUSMSG_STUFF 0x20 /* Stuff Error */
+#define USB2CAN_STATUSMSG_FORM 0x21 /* Form Error */
+#define USB2CAN_STATUSMSG_ACK 0x23 /* Ack Error */
+#define USB2CAN_STATUSMSG_BIT0 0x24 /* Bit1 Error */
+#define USB2CAN_STATUSMSG_BIT1 0x25 /* Bit0 Error */
+#define USB2CAN_STATUSMSG_CRC 0x26 /* CRC Error */
+
+#define USB2CAN_RP_MASK 0x7F /* Mask for Receive Error Bit */
+
+
+/* table of devices that work with this driver */
+static struct usb_device_id usb2can_table[] = {
+ { USB_DEVICE(USB2CAN_VENDOR_ID, USB2CAN_PRODUCT_ID) },
+ { } /* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, usb2can_table);
+
+struct usb2can_tx_urb_context {
+ struct usb2can *dev;
+
+ u32 echo_index;
+ u8 dlc;
+};
+
+/* Structure to hold all of our device specific stuff */
+struct usb2can {
+ struct can_priv can; /* must be the first member */
+
+ struct sk_buff *echo_skb[MAX_TX_URBS];
+
+ struct usb_device *udev;
+ struct net_device *netdev;
+
+ atomic_t active_tx_urbs;
+ struct usb_anchor tx_submitted;
+ struct usb2can_tx_urb_context tx_contexts[MAX_TX_URBS];
+
+ struct usb_anchor rx_submitted;
+
+ u8 *cmd_msg_buffer;
+
+ unsigned int free_slots; /* remember number of available slots */
+
+ unsigned int dar; /* disable automatic restransmission */
+
+ struct mutex usb2can_cmd_lock;
+};
+
+/* tx frame */
+struct __packed usb2can_tx_msg {
+ u8 begin;
+ u8 flags; /* RTR and EXT_ID flag */
+ __be32 id; /* upper 3 bits not used */
+ u8 dlc; /* data length code 0-8 bytes */
+ u8 data[8]; /* 64-bit data */
+ u8 end;
+};
+
+/* rx frame */
+struct __packed usb2can_rx_msg {
+ u8 begin;
+ u8 type; /* frame type */
+ u8 flags; /* RTR and EXT_ID flag */
+ __be32 id; /* upper 3 bits not used */
+ u8 dlc; /* data length code 0-8 bytes */
+ u8 data[8]; /* 64-bit data */
+ __be32 timestamp; /* 32-bit timestamp */
+ u8 end;
+};
+
+/* command frame */
+struct __packed usb2can_cmd_msg {
+ u8 begin;
+ u8 channel; /* unkown - always 0 */
+ u8 command; /* command to execute */
+ u8 opt1; /* optional parameter / return value */
+ u8 opt2; /* optional parameter 2 */
+ u8 data[10]; /* optional parameter and data */
+ u8 end;
+};
+
+static struct usb_driver usb2can_driver;
+
+static int usb2can_send_cmd_msg(struct usb2can *dev, u8 *msg, int size)
+{
+ int actual_length;
+
+ return usb_bulk_msg(dev->udev,
+ usb_sndbulkpipe(dev->udev, 4),
+ msg,
+ size,
+ &actual_length,
+ 1000);
+}
+
+static int usb2can_wait_cmd_msg(struct usb2can *dev, u8 *msg, int size,
+ int *actual_length)
+{
+ return usb_bulk_msg(dev->udev,
+ usb_rcvbulkpipe(dev->udev, 3),
+ msg,
+ size,
+ actual_length,
+ 1000);
+}
+
+/* Send command to device and receive result.
+ * Command was successful When opt1 = 0.
+ */
+static int usb2can_send_cmd(struct usb2can *dev, struct usb2can_cmd_msg *out,
+ struct usb2can_cmd_msg *in)
+{
+ int err;
+ int nBytesRead;
+ struct net_device *netdev;
+
+ netdev = dev->netdev;
+
+ out->begin = USB2CAN_CMD_START;
+ out->end = USB2CAN_CMD_END;
+
+ memcpy(&dev->cmd_msg_buffer[0], out,
+ sizeof(struct usb2can_cmd_msg));
+
+ mutex_lock(&dev->usb2can_cmd_lock);
+
+ err = usb2can_send_cmd_msg(dev, &dev->cmd_msg_buffer[0],
+ sizeof(struct usb2can_cmd_msg));
+ if (err < 0) {
+ dev_err(netdev->dev.parent, "sending command message failed\n");
+ return err;
+ }
+
+ err = usb2can_wait_cmd_msg(dev, &dev->cmd_msg_buffer[0],
+ sizeof(struct usb2can_cmd_msg), &nBytesRead);
+ if (err < 0) {
+ dev_err(netdev->dev.parent, "no command message answer\n");
+ return err;
+ }
+
+ mutex_unlock(&dev->usb2can_cmd_lock);
+
+ memcpy(in, &dev->cmd_msg_buffer[0],
+ sizeof(struct usb2can_cmd_msg));
+
+ if (in->begin != USB2CAN_CMD_START || in->end != USB2CAN_CMD_END ||
+ nBytesRead != 16 || in->opt1 != 0)
+ return -EPROTO;
+
+ return 0;
+}
+
+/* Send open command to device */
+static int usb2can_cmd_open(struct usb2can *dev)
+{
+ struct can_bittiming *bt = &dev->can.bittiming;
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ u32 flags = 0x00000000;
+ u32 beflags;
+ u16 bebrp;
+ u32 ctrlmode = dev->can.ctrlmode;
+
+ if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
+ flags |= USB2CAN_LOOPBACK;
+ if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
+ flags |= USB2CAN_SILENT;
+ if (dev->dar == 1)
+ flags |= USB2CAN_DISABLE_AUTO_RESTRANS;
+
+ flags |= USB2CAN_STATUS_FRAME;
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_OPEN;
+ outmsg.opt1 = USB2CAN_BAUD_MANUAL;
+ outmsg.data[0] = (u8) (bt->prop_seg + bt->phase_seg1);
+ outmsg.data[1] = (u8) bt->phase_seg2;
+ outmsg.data[2] = (u8) bt->sjw;
+
+ /* BRP */
+ bebrp = cpu_to_be16((u16) bt->brp);
+ memcpy(&outmsg.data[3], &bebrp, sizeof(bebrp));
+
+ /* flags */
+ beflags = cpu_to_be32(flags);
+ memcpy(&outmsg.data[5], &beflags, sizeof(beflags));
+
+ return usb2can_send_cmd(dev, &outmsg, &inmsg);
+}
+
+/* Send close command to device */
+static int usb2can_cmd_close(struct usb2can *dev)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_CLOSE;
+
+ return usb2can_send_cmd(dev, &outmsg, &inmsg);
+}
+
+/* Get firmware and hardware version */
+static int usb2can_cmd_version(struct usb2can *dev, u32 *res)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ u32 *value;
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_SOFTW_HARDW_VER;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return err;
+
+ value = (u32 *) inmsg.data;
+ *res = be32_to_cpu(*value);
+
+ return err;
+}
+
+/* Get firmware version */
+static ssize_t show_firmware(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ u16 *value;
+ u16 result;
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_SOFTW_VER;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return -EIO;
+
+ value = (u16 *) inmsg.data;
+ result = be16_to_cpu(*value);
+
+ return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
+}
+
+/* Get hardware version */
+static ssize_t show_hardware(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ u16 *value;
+ u16 result;
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_HARDW_VER;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return -EIO;
+
+ value = (u16 *) inmsg.data;
+ result = be16_to_cpu(*value);
+
+ return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
+}
+
+/* Get status
+ *
+ * Returns:
+ * STATUS_NONE 0x00000000
+ * STATUS_BUS_OFF 0x80000000
+ * STATUS_PASSIVE 0x40000000
+ * STATUS_BUS_WARN 0x20000000
+ * STATUS_ACTIVE 0x10000000
+ * STATUS_PHY_FAULT 0x08000000
+ * STATUS_PHY_H 0x04000000
+ * STATUS_PHY_L 0x02000000
+ * STATUS_SLEEPING 0x01000000
+ * STATUS_STOPPED 0x00800000
+ */
+static ssize_t show_status(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ u32 *value;
+ u32 result;
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_STATUS;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return -EIO;
+
+ value = (u32 *) inmsg.data;
+ result = be32_to_cpu(*value);
+
+ return sprintf(buf, "0x%08x\n", result);
+}
+
+/* Get statistic values */
+static ssize_t show_statistics(struct device *d, struct device_attribute *attr,
+ u8 statistic, char *buf)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ u32 *value;
+ u32 result;
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_STATISTICS;
+ outmsg.opt1 = statistic;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return -EIO;
+
+ value = (u32 *) inmsg.data;
+ result = be32_to_cpu(*value);
+
+ return sprintf(buf, "%d\n", result);
+}
+
+static ssize_t show_rx_frames(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_RX_FRAMES, buf);
+}
+
+static ssize_t show_rx_bytes(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
+}
+
+static ssize_t show_tx_frames(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_TX_FRAMES, buf);
+}
+
+static ssize_t show_tx_bytes(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
+}
+
+static ssize_t show_overruns(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_OVERRUNS, buf);
+}
+
+static ssize_t show_warnings(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_WARNINGS, buf);
+}
+
+static ssize_t show_bus_off(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ return show_statistics(d, attr, USB2CAN_STAT_BUS_OFF, buf);
+}
+
+/* Reset statistics */
+static ssize_t reset_statistics(struct device *d, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb2can_cmd_msg outmsg;
+ struct usb2can_cmd_msg inmsg;
+ int err = 0;
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ if (buf[0] == '1') {
+ memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
+ outmsg.command = USB2CAN_GET_STATISTICS;
+ outmsg.opt1 = USB2CAN_STAT_RESET_STAT;
+
+ err = usb2can_send_cmd(dev, &outmsg, &inmsg);
+ if (err)
+ return -EIO;
+ }
+
+ return count;
+}
+
+/* Get "disable automatic retransmission" flag */
+static ssize_t show_dar(struct device *d, struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ return sprintf(buf, "%d\n", dev->dar);
+}
+
+/* Set "disable automatic retransmission" flag */
+static ssize_t set_dar(struct device *d, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_interface *intf = to_usb_interface(d);
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ if (dev->can.state != CAN_STATE_STOPPED) {
+ dev_err(&intf->dev,
+ "DAR flag can only be set when device is stopped\n");
+ return -EIO;
+ }
+
+ if (buf[0] == '0')
+ dev->dar = 0;
+ else if (buf[0] == '1')
+ dev->dar = 1;
+ else
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
+static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
+static DEVICE_ATTR(can_state, S_IRUGO, show_status, NULL);
+static DEVICE_ATTR(can_rx_frames, S_IRUGO, show_rx_frames, NULL);
+static DEVICE_ATTR(can_rx_bytes, S_IRUGO, show_rx_bytes, NULL);
+static DEVICE_ATTR(can_tx_frames, S_IRUGO, show_tx_frames, NULL);
+static DEVICE_ATTR(can_tx_bytes, S_IRUGO, show_tx_bytes, NULL);
+static DEVICE_ATTR(can_overruns, S_IRUGO, show_overruns, NULL);
+static DEVICE_ATTR(can_warnings, S_IRUGO, show_warnings, NULL);
+static DEVICE_ATTR(can_bus_off_counter, S_IRUGO, show_bus_off, NULL);
+
+static DEVICE_ATTR(can_reset_statistics, S_IWUSR, NULL, reset_statistics);
+
+static DEVICE_ATTR(can_disable_automatic_retransmission, S_IRUGO | S_IWUSR,
+ show_dar, set_dar);
+
+/* Set network device mode
+ *
+ * Maybe we should leave this function empty, because the device
+ * set mode variable with open command.
+ */
+static int usb2can_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+ struct usb2can *dev = netdev_priv(netdev);
+ int err = 0;
+
+ switch (mode) {
+ case CAN_MODE_START:
+ err = usb2can_cmd_open(dev);
+ if (err)
+ dev_warn(netdev->dev.parent, "couldn't start device");
+
+ if (netif_queue_stopped(netdev))
+ netif_wake_queue(netdev);
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+/* Empty function: We set bittiming when we start the interface.
+ * This is a firmware limitation.
+ */
+static int usb2can_set_bittiming(struct net_device *netdev)
+{
+ return 0;
+}
+
+/* Read data and status frames */
+static void usb2can_rx_can_msg(struct usb2can *dev, struct usb2can_rx_msg *msg)
+{
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ int i;
+ struct net_device_stats *stats = &dev->netdev->stats;
+
+ skb = alloc_can_skb(dev->netdev, &cf);
+ if (skb == NULL)
+ return;
+
+ if (msg->type == USB2CAN_TYPE_CAN_FRAME) {
+ cf->can_id = be32_to_cpu(msg->id);
+ cf->can_dlc = get_can_dlc(msg->dlc & 0xF);
+
+ if (msg->flags & USB2CAN_EXTID)
+ cf->can_id |= CAN_EFF_FLAG;
+
+ if (msg->flags & USB2CAN_RTR)
+ cf->can_id |= CAN_RTR_FLAG;
+ else
+ for (i = 0; i < cf->can_dlc; i++)
+ cf->data[i] = msg->data[i];
+ } else if (msg->type == USB2CAN_TYPE_ERROR_FRAME &&
+ msg->flags == USB2CAN_ERR_FLAG) {
+
+ /* Error message:
+ byte 0: Status
+ byte 1: bit 7: Receive Passive
+ byte 1: bit 0-6: Receive Error Counter
+ byte 2: Transmit Error Counter
+ byte 3: Always 0 (maybe reserved for future use)
+ */
+
+ u8 state = msg->data[0];
+ u8 rxerr = msg->data[1] & USB2CAN_RP_MASK;
+ u8 txerr = msg->data[2];
+ int rx_errors = 0;
+ int tx_errors = 0;
+
+ dev->can.can_stats.bus_error++;
+
+ cf->can_id = CAN_ERR_FLAG;
+ cf->can_dlc = CAN_ERR_DLC;
+
+ switch (state) {
+ case USB2CAN_STATUSMSG_OK:
+ dev->can.state = CAN_STATE_ERROR_ACTIVE;
+ cf->can_id |= CAN_ERR_PROT;
+ cf->data[2] = CAN_ERR_PROT_ACTIVE;
+ break;
+ case USB2CAN_STATUSMSG_BUSOFF:
+ dev->can.state = CAN_STATE_BUS_OFF;
+ cf->can_id |= CAN_ERR_BUSOFF;
+ can_bus_off(dev->netdev);
+ break;
+ default:
+ dev->can.state = CAN_STATE_ERROR_WARNING;
+ cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+ break;
+ }
+
+ switch (state) {
+ case USB2CAN_STATUSMSG_ACK:
+ cf->can_id |= CAN_ERR_ACK;
+ tx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_CRC:
+ cf->data[2] |= CAN_ERR_PROT_BIT;
+ rx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_BIT0:
+ cf->data[2] |= CAN_ERR_PROT_BIT0;
+ tx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_BIT1:
+ cf->data[2] |= CAN_ERR_PROT_BIT1;
+ tx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_FORM:
+ cf->data[2] |= CAN_ERR_PROT_FORM;
+ rx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_STUFF:
+ cf->data[2] |= CAN_ERR_PROT_STUFF;
+ rx_errors = 1;
+ break;
+ case USB2CAN_STATUSMSG_OVERRUN:
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = (txerr > rxerr) ?
+ CAN_ERR_CRTL_TX_OVERFLOW :
+ CAN_ERR_CRTL_RX_OVERFLOW;
+ cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
+ stats->rx_over_errors++;
+ break;
+ case USB2CAN_STATUSMSG_BUSLIGHT:
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = (txerr > rxerr) ?
+ CAN_ERR_CRTL_TX_WARNING :
+ CAN_ERR_CRTL_RX_WARNING;
+ dev->can.can_stats.error_warning++;
+ break;
+ case USB2CAN_STATUSMSG_BUSHEAVY:
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = (txerr > rxerr) ?
+ CAN_ERR_CRTL_TX_PASSIVE :
+ CAN_ERR_CRTL_RX_PASSIVE;
+ dev->can.can_stats.error_passive++;
+ break;
+ default:
+ cf->data[2] |= CAN_ERR_PROT_UNSPEC;
+ break;
+ }
+
+ if (tx_errors) {
+ cf->data[2] |= CAN_ERR_PROT_TX;
+ stats->tx_errors++;
+ }
+
+ if (rx_errors)
+ stats->rx_errors++;
+
+ cf->data[6] = txerr;
+ cf->data[7] = rxerr;
+
+ } else {
+ dev_warn(dev->udev->dev.parent, "frame type %d unknown",
+ msg->type);
+ }
+
+ netif_rx(skb);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;
+}
+
+/* Callback for reading data from device
+ *
+ * Check urb status, call read function and resubmit urb read operation.
+ */
+static void usb2can_read_bulk_callback(struct urb *urb)
+{
+ struct usb2can *dev = urb->context;
+ struct net_device *netdev;
+ int retval;
+ int pos = 0;
+
+ netdev = dev->netdev;
+
+ if (!netif_device_present(netdev))
+ return;
+
+ switch (urb->status) {
+ case 0: /* success */
+ break;
+
+ case -ENOENT:
+ case -ESHUTDOWN:
+ return;
+
+ default:
+ dev_info(netdev->dev.parent, "Rx URB aborted (%d)\n",
+ urb->status);
+ goto resubmit_urb;
+ }
+
+ while (pos < urb->actual_length) {
+ struct usb2can_rx_msg *msg;
+
+ msg = (struct usb2can_rx_msg *)(urb->transfer_buffer + pos);
+
+ usb2can_rx_can_msg(dev, msg);
+
+ pos += sizeof(struct usb2can_rx_msg);
+
+ if (pos > urb->actual_length) {
+ dev_err(dev->udev->dev.parent, "format error\n");
+ break;
+ }
+ }
+
+resubmit_urb:
+ usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
+ urb->transfer_buffer, RX_BUFFER_SIZE,
+ usb2can_read_bulk_callback, dev);
+
+ retval = usb_submit_urb(urb, GFP_ATOMIC);
+
+ if (retval == -ENODEV)
+ netif_device_detach(netdev);
+ else if (retval)
+ dev_err(netdev->dev.parent,
+ "failed resubmitting read bulk urb: %d\n", retval);
+}
+
+/* Callback handler for write operations
+ *
+ * Free allocated buffers, check transmit status and
+ * calculate statistic.
+ */
+static void usb2can_write_bulk_callback(struct urb *urb)
+{
+ struct usb2can_tx_urb_context *context = urb->context;
+ struct usb2can *dev;
+ struct net_device *netdev;
+
+ BUG_ON(!context);
+
+ dev = context->dev;
+ netdev = dev->netdev;
+
+ /* free up our allocated buffer */
+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);
+
+ atomic_dec(&dev->active_tx_urbs);
+
+ if (!netif_device_present(netdev))
+ return;
+
+ if (urb->status)
+ dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
+ urb->status);
+
+ netdev->trans_start = jiffies;
+
+ netdev->stats.tx_packets++;
+ netdev->stats.tx_bytes += context->dlc;
+
+ can_get_echo_skb(netdev, context->echo_index);
+
+ /* Release context */
+ context->echo_index = MAX_TX_URBS;
+
+ if (netif_queue_stopped(netdev))
+ netif_wake_queue(netdev);
+}
+
+/* Send data to device */
+static netdev_tx_t usb2can_start_xmit(struct sk_buff *skb,
+ struct net_device *netdev)
+{
+ struct usb2can *dev = netdev_priv(netdev);
+ struct net_device_stats *stats = &netdev->stats;
+ struct can_frame *cf = (struct can_frame *)skb->data;
+ struct usb2can_tx_msg *msg;
+ struct urb *urb;
+ struct usb2can_tx_urb_context *context = NULL;
+ u8 *buf;
+ int i, err;
+ size_t size = sizeof(struct usb2can_tx_msg);
+
+ if (can_dropped_invalid_skb(netdev, skb))
+ return NETDEV_TX_OK;
+
+ /* create a URB, and a buffer for it, and copy the data to the URB */
+ urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!urb) {
+ dev_err(netdev->dev.parent, "No memory left for URBs\n");
+ goto nomem;
+ }
+
+ buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC,
+ &urb->transfer_dma);
+ if (!buf) {
+ dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+ usb_free_urb(urb);
+ goto nomem;
+ }
+
+ memset(buf, 0, size);
+
+ msg = (struct usb2can_tx_msg *)buf;
+ msg->begin = USB2CAN_DATA_START;
+ msg->flags = 0x00;
+
+ if (cf->can_id & CAN_RTR_FLAG)
+ msg->flags |= USB2CAN_RTR;
+
+ if (cf->can_id & CAN_EFF_FLAG)
+ msg->flags |= USB2CAN_EXTID;
+
+ msg->id = cpu_to_be32(cf->can_id & CAN_ERR_MASK);
+
+ msg->dlc = cf->can_dlc;
+
+ for (i = 0; i < cf->can_dlc; i++)
+ msg->data[i] = cf->data[i];
+
+ msg->end = USB2CAN_DATA_END;
+
+
+ for (i = 0; i < MAX_TX_URBS; i++) {
+ if (dev->tx_contexts[i].echo_index == MAX_TX_URBS) {
+ context = &dev->tx_contexts[i];
+ break;
+ }
+ }
+
+ /* May never happen! When this happens we'd more URBs in flight as
+ * allowed (MAX_TX_URBS).
+ */
+ if (!context) {
+ usb_unanchor_urb(urb);
+ usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
+
+ dev_warn(netdev->dev.parent, "couldn't find free context");
+
+ return NETDEV_TX_BUSY;
+ }
+
+ context->dev = dev;
+ context->echo_index = i;
+ context->dlc = cf->can_dlc;
+
+ usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
+ size, usb2can_write_bulk_callback, context);
+ urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+ usb_anchor_urb(urb, &dev->tx_submitted);
+
+ can_put_echo_skb(skb, netdev, context->echo_index);
+
+ atomic_inc(&dev->active_tx_urbs);
+
+ err = usb_submit_urb(urb, GFP_ATOMIC);
+ if (unlikely(err)) {
+ can_free_echo_skb(netdev, context->echo_index);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
+ dev_kfree_skb(skb);
+
+ atomic_dec(&dev->active_tx_urbs);
+
+ if (err == -ENODEV) {
+ netif_device_detach(netdev);
+ } else {
+ dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
+
+ stats->tx_dropped++;
+ }
+ } else {
+ netdev->trans_start = jiffies;
+
+ /* Slow down tx path */
+ if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS ||
+ dev->free_slots < 5) {
+ netif_stop_queue(netdev);
+ }
+ }
+
+ /* Release our reference to this URB, the USB core will eventually free
+ * it entirely.
+ */
+ usb_free_urb(urb);
+
+ return NETDEV_TX_OK;
+
+nomem:
+ dev_kfree_skb(skb);
+ stats->tx_dropped++;
+
+ return NETDEV_TX_OK;
+}
+
+
+/* Start USB device */
+static int usb2can_start(struct usb2can *dev)
+{
+ struct net_device *netdev = dev->netdev;
+ int err, i;
+
+ dev->free_slots = 15; /* initial size */
+
+ for (i = 0; i < MAX_RX_URBS; i++) {
+ struct urb *urb = NULL;
+ u8 *buf = NULL;
+
+ /* create a URB, and a buffer for it */
+ urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!urb) {
+ dev_err(netdev->dev.parent,
+ "No memory left for URBs\n");
+ return -ENOMEM;
+ }
+
+ buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
+ &urb->transfer_dma);
+ if (!buf) {
+ dev_err(netdev->dev.parent,
+ "No memory left for USB buffer\n");
+ usb_free_urb(urb);
+ return -ENOMEM;
+ }
+
+ usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
+ buf, RX_BUFFER_SIZE,
+ usb2can_read_bulk_callback, dev);
+ urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+ usb_anchor_urb(urb, &dev->rx_submitted);
+
+ err = usb_submit_urb(urb, GFP_KERNEL);
+ if (err) {
+ if (err == -ENODEV)
+ netif_device_detach(dev->netdev);
+
+ usb_unanchor_urb(urb);
+ usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
+ urb->transfer_dma);
+ break;
+ }
+
+ /* Drop reference, USB core will take care of freeing it */
+ usb_free_urb(urb);
+ }
+
+ /* Did we submit any URBs */
+ if (i == 0) {
+ dev_warn(netdev->dev.parent, "couldn't setup read URBs\n");
+ return err;
+ }
+
+ /* Warn if we've couldn't transmit all the URBs */
+ if (i < MAX_RX_URBS)
+ dev_warn(netdev->dev.parent, "rx performance may be slow\n");
+
+ err = usb2can_cmd_open(dev);
+ if (err)
+ goto failed;
+
+ dev->can.state = CAN_STATE_ERROR_ACTIVE;
+
+ return 0;
+
+failed:
+ if (err == -ENODEV)
+ netif_device_detach(dev->netdev);
+
+ dev_warn(netdev->dev.parent, "couldn't submit control: %d\n", err);
+
+ return err;
+}
+
+/* Open USB device */
+static int usb2can_open(struct net_device *netdev)
+{
+ struct usb2can *dev = netdev_priv(netdev);
+ int err;
+
+ /* common open */
+ err = open_candev(netdev);
+ if (err)
+ return err;
+
+ /* finally start device */
+ err = usb2can_start(dev);
+ if (err) {
+ if (err == -ENODEV)
+ netif_device_detach(dev->netdev);
+
+ dev_warn(netdev->dev.parent, "couldn't start device: %d\n",
+ err);
+
+ close_candev(netdev);
+
+ return err;
+ }
+
+ netif_start_queue(netdev);
+
+ return 0;
+}
+
+static void unlink_all_urbs(struct usb2can *dev)
+{
+ int i;
+
+ usb_kill_anchored_urbs(&dev->rx_submitted);
+
+ usb_kill_anchored_urbs(&dev->tx_submitted);
+ atomic_set(&dev->active_tx_urbs, 0);
+
+ for (i = 0; i < MAX_TX_URBS; i++)
+ dev->tx_contexts[i].echo_index = MAX_TX_URBS;
+}
+
+/* Close USB device */
+static int usb2can_close(struct net_device *netdev)
+{
+ struct usb2can *dev = netdev_priv(netdev);
+ int err = 0;
+
+ /* Send CLOSE command to CAN controller */
+ err = usb2can_cmd_close(dev);
+ if (err)
+ dev_warn(netdev->dev.parent, "couldn't stop device");
+
+ dev->can.state = CAN_STATE_STOPPED;
+
+ /* Stop polling */
+ unlink_all_urbs(dev);
+
+ netif_stop_queue(netdev);
+
+ close_candev(netdev);
+
+ return err;
+}
+
+static const struct net_device_ops usb2can_netdev_ops = {
+ .ndo_open = usb2can_open,
+ .ndo_stop = usb2can_close,
+ .ndo_start_xmit = usb2can_start_xmit,
+};
+
+static struct can_bittiming_const usb2can_bittiming_const = {
+ .name = "usb2can",
+ .tseg1_min = USB2CAN_TSEG1_MIN,
+ .tseg1_max = USB2CAN_TSEG1_MAX,
+ .tseg2_min = USB2CAN_TSEG2_MIN,
+ .tseg2_max = USB2CAN_TSEG2_MAX,
+ .sjw_max = USB2CAN_SJW_MAX,
+ .brp_min = USB2CAN_BRP_MIN,
+ .brp_max = USB2CAN_BRP_MAX,
+ .brp_inc = USB2CAN_BRP_INC,
+};
+
+/* Probe USB device
+ *
+ * Check device and firmware.
+ * Set supported modes and bittiming constants.
+ * Allocate some memory.
+ */
+static int usb2can_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ struct net_device *netdev;
+ struct usb2can *dev;
+ int i, err = -ENOMEM;
+ u32 version;
+ char buf[18];
+ struct usb_device *usbdev = interface_to_usbdev(intf);
+
+ /* product id looks strange, better we also check iProdukt string */
+ if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
+ sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
+ dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
+ return -ENODEV;
+ }
+
+ netdev = alloc_candev(sizeof(struct usb2can), MAX_TX_URBS);
+ if (!netdev) {
+ dev_err(&intf->dev, "Couldn't alloc candev\n");
+ return -ENOMEM;
+ }
+
+ dev = netdev_priv(netdev);
+
+ dev->udev = usbdev;
+ dev->netdev = netdev;
+
+ dev->dar = 0;
+
+ dev->can.state = CAN_STATE_STOPPED;
+ dev->can.clock.freq = USB2CAN_ABP_CLOCK;
+ dev->can.bittiming_const = &usb2can_bittiming_const;
+ dev->can.do_set_bittiming = usb2can_set_bittiming;
+ dev->can.do_set_mode = usb2can_set_mode;
+ dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
+ CAN_CTRLMODE_LISTENONLY;
+
+ netdev->netdev_ops = &usb2can_netdev_ops;
+
+ netdev->flags |= IFF_ECHO; /* we support local echo */
+
+ init_usb_anchor(&dev->rx_submitted);
+
+ init_usb_anchor(&dev->tx_submitted);
+ atomic_set(&dev->active_tx_urbs, 0);
+
+ for (i = 0; i < MAX_TX_URBS; i++)
+ dev->tx_contexts[i].echo_index = MAX_TX_URBS;
+
+ dev->cmd_msg_buffer = kzalloc(sizeof(struct usb2can_cmd_msg),
+ GFP_KERNEL);
+ if (!dev->cmd_msg_buffer) {
+ dev_err(&intf->dev, "Couldn't alloc Tx buffer\n");
+ goto cleanup_candev;
+ }
+
+ usb_set_intfdata(intf, dev);
+
+ SET_NETDEV_DEV(netdev, &intf->dev);
+
+ mutex_init(&dev->usb2can_cmd_lock);
+
+ err = usb2can_cmd_version(dev, &version);
+ if (err) {
+ dev_err(netdev->dev.parent, "can't get firmware version\n");
+ goto cleanup_cmd_msg_buffer;
+ } else {
+ dev_info(netdev->dev.parent,
+ "firmware: %d.%d, hardware: %d.%d\n",
+ (u8)(version>>24), (u8)(version>>16),
+ (u8)(version>>8), (u8)version);
+ }
+
+ err = register_candev(netdev);
+ if (err) {
+ dev_err(netdev->dev.parent,
+ "couldn't register CAN device: %d\n", err);
+ goto cleanup_cmd_msg_buffer;
+ }
+
+ if (device_create_file(&intf->dev, &dev_attr_firmware))
+ dev_err(&intf->dev,
+ "Couldn't create device file for firmware\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_hardware))
+ dev_err(&intf->dev,
+ "Couldn't create device file for hardware\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_state))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_state\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_rx_frames))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_rx_frames\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_rx_bytes))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_rx_bytes\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_tx_frames))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_tx_frames\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_tx_bytes))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_tx_bytes\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_overruns))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_overruns\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_warnings))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_warnings\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_bus_off_counter))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_bus_off_counter\n");
+
+ if (device_create_file(&intf->dev, &dev_attr_can_reset_statistics))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_reset_statistics\n");
+
+ if (device_create_file(&intf->dev,
+ &dev_attr_can_disable_automatic_retransmission))
+ dev_err(&intf->dev,
+ "Couldn't create device file for can_disable_automatic_retransmission\n");
+
+ /* let the user know what node this device is now attached to */
+ dev_info(netdev->dev.parent, "device registered as %s\n", netdev->name);
+ return 0;
+
+cleanup_cmd_msg_buffer:
+ kfree(dev->cmd_msg_buffer);
+
+cleanup_candev:
+ free_candev(netdev);
+
+ return err;
+
+}
+
+/* Called by the usb core when driver is unloaded or device is removed */
+static void usb2can_disconnect(struct usb_interface *intf)
+{
+ struct usb2can *dev = usb_get_intfdata(intf);
+
+ device_remove_file(&intf->dev, &dev_attr_firmware);
+ device_remove_file(&intf->dev, &dev_attr_hardware);
+ device_remove_file(&intf->dev, &dev_attr_can_state);
+ device_remove_file(&intf->dev, &dev_attr_can_rx_frames);
+ device_remove_file(&intf->dev, &dev_attr_can_rx_bytes);
+ device_remove_file(&intf->dev, &dev_attr_can_tx_frames);
+ device_remove_file(&intf->dev, &dev_attr_can_tx_bytes);
+ device_remove_file(&intf->dev, &dev_attr_can_overruns);
+ device_remove_file(&intf->dev, &dev_attr_can_warnings);
+ device_remove_file(&intf->dev, &dev_attr_can_bus_off_counter);
+ device_remove_file(&intf->dev, &dev_attr_can_reset_statistics);
+ device_remove_file(&intf->dev,
+ &dev_attr_can_disable_automatic_retransmission);
+
+ usb_set_intfdata(intf, NULL);
+
+ if (dev) {
+ dev_info(&intf->dev, "disconnect %s\n", dev->netdev->name);
+
+ unregister_netdev(dev->netdev);
+ free_candev(dev->netdev);
+
+ unlink_all_urbs(dev);
+ }
+
+}
+
+static struct usb_driver usb2can_driver = {
+ .name = "usb2can",
+ .probe = usb2can_probe,
+ .disconnect = usb2can_disconnect,
+ .id_table = usb2can_table,
+};
+
+module_usb_driver(usb2can_driver);
+
+MODULE_AUTHOR("Bernd Krumboeck <krumboeck@universalnet.at>");
+MODULE_DESCRIPTION("CAN driver for UAB 8 devices USB2CAN interfaces");
+MODULE_LICENSE("GPL v2");
+
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-02 9:25 [PATCH] usb2can: Add support for USB2CAN interface from 8 devices krumboeck
@ 2012-12-02 10:36 ` Oliver Hartkopp
2012-12-02 11:45 ` Kurt Van Dijck
2012-12-02 13:35 ` Wolfgang Grandegger
1 sibling, 1 reply; 10+ messages in thread
From: Oliver Hartkopp @ 2012-12-02 10:36 UTC (permalink / raw)
To: krumboeck@universalnet.at; +Cc: linux-can, info, gediminas
Hallo Bernd,
nice to see that you have been able to send the patch that fast :-)
In general please rename the Kconfig entries and the driver to 8dev_usb as
there are various USB-to-CAN adapters available.
This follows the <vendor>_usb rule like peak_usb, esd_usb, etc.
Some more comments inline.
On 02.12.2012 10:25, krumboeck@universalnet.at wrote:
> Add device driver for USB2CAN interface from "8 devices"
> (http://www.8devices.com).
>
> Signed-off-by: Bernd Krumboeck <krumboeck@universalnet.at>
> ---
> drivers/net/can/usb/Kconfig | 6 +
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/usb2can.c | 1323 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1330 insertions(+)
> create mode 100644 drivers/net/can/usb/usb2can.c
>
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index a4e4bee..2068c99 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -48,4 +48,10 @@ config CAN_PEAK_USB
> This driver supports the PCAN-USB and PCAN-USB Pro adapters
> from PEAK-System Technik (http://www.peak-system.com).
>
> +config CAN_USB2CAN
> + tristate "8 devices USB2CAN interface"
> + ---help---
> + This driver supports the USB2CAN interface
> + from 8 devices (http://www.8devices.com).
> +
> endmenu
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index 80a2ee4..3c0a378 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -6,5 +6,6 @@ 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/
> +obj-$(CONFIG_CAN_USB2CAN) += usb2can.o
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/usb2can.c b/drivers/net/can/usb/usb2can.c
> new file mode 100644
> index 0000000..5a09141
> --- /dev/null
> +++ b/drivers/net/can/usb/usb2can.c
> @@ -0,0 +1,1323 @@
> +/*
> + * CAN driver for UAB "8 devices" USB2CAN converter
> + *
> + * Copyright (C) 2012 Bernd Krumboeck (krumboeck@universalnet.at)
> + *
> + * 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 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + * This driver is based on the 3.2.0 version of drivers/net/can/usb/ems_usb.c
> + * and drivers/net/can/usb/esd_usb2.c
> + *
> + * Many thanks to Gerhard Bertelsmann (info@gerhard-bertelsmann.de)
> + * for testing and fixing this driver. Also many thanks to "8 devices",
> + * who were very cooperative and answered my questions.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/signal.h>
> +#include <linux/slab.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>
> +
> +
One empty line is enough here ...
> +/* driver constants */
> +#define MAX_RX_URBS 10
> +#define MAX_TX_URBS 10
> +#define RX_BUFFER_SIZE 64
> +
> +/* vendor and product id */
> +#define USB2CAN_VENDOR_ID 0x0483
> +#define USB2CAN_PRODUCT_ID 0x1234
As 8DEV_USB_VENDOR_ID will not work and will create an error message like
drivers/net/can/usb/8dev_usb.c:46:9: error: macro names must be identifiers
you might stay on USB2CAN_ as prefix here.
> +
> +/* bittiming constants */
> +#define USB2CAN_ABP_CLOCK 32000000
> +#define USB2CAN_BAUD_MANUAL 0x09
> +#define USB2CAN_TSEG1_MIN 1
> +#define USB2CAN_TSEG1_MAX 16
> +#define USB2CAN_TSEG2_MIN 1
> +#define USB2CAN_TSEG2_MAX 8
> +#define USB2CAN_SJW_MAX 4
> +#define USB2CAN_BRP_MIN 1
> +#define USB2CAN_BRP_MAX 1024
> +#define USB2CAN_BRP_INC 1
> +
> +/* setup flags */
> +#define USB2CAN_SILENT 0x00000001
> +#define USB2CAN_LOOPBACK 0x00000002
> +#define USB2CAN_DISABLE_AUTO_RESTRANS 0x00000004
> +#define USB2CAN_STATUS_FRAME 0x00000008
Why not using 0x1 .. 0x8 without all the '0's ?
> +
> +/* commands */
> +#define USB2CAN_RESET 1
> +#define USB2CAN_OPEN 2
> +#define USB2CAN_CLOSE 3
> +#define USB2CAN_SET_SPEED 4
> +#define USB2CAN_SET_MASK_FILTER 5
> +#define USB2CAN_GET_STATUS 6
> +#define USB2CAN_GET_STATISTICS 7
> +#define USB2CAN_GET_SERIAL 8
> +#define USB2CAN_GET_SOFTW_VER 9
> +#define USB2CAN_GET_HARDW_VER 10
> +#define USB2CAN_RESET_TIMESTAMP 11
> +#define USB2CAN_GET_SOFTW_HARDW_VER 12
> +
> +#define USB2CAN_CMD_START 0x11
> +#define USB2CAN_CMD_END 0x22
> +
> +#define USB2CAN_CMD_SUCCESS 0
> +#define USB2CAN_CMD_ERROR 255
> +
> +/* statistics */
> +#define USB2CAN_STAT_RX_FRAMES 0
> +#define USB2CAN_STAT_RX_BYTES 1
> +#define USB2CAN_STAT_TX_FRAMES 2
> +#define USB2CAN_STAT_TX_BYTES 3
> +#define USB2CAN_STAT_OVERRUNS 4
> +#define USB2CAN_STAT_WARNINGS 5
> +#define USB2CAN_STAT_BUS_OFF 6
> +#define USB2CAN_STAT_RESET_STAT 7
> +
> +/* frames */
> +#define USB2CAN_DATA_START 0x55
> +#define USB2CAN_DATA_END 0xAA
> +
> +#define USB2CAN_TYPE_CAN_FRAME 0
> +#define USB2CAN_TYPE_ERROR_FRAME 3
> +
> +#define USB2CAN_EXTID 0x01
> +#define USB2CAN_RTR 0x02
> +#define USB2CAN_ERR_FLAG 0x04
> +
> +/* status */
> +#define USB2CAN_STATUSMSG_OK 0x00 /* Normal condition. */
> +#define USB2CAN_STATUSMSG_OVERRUN 0x01 /* Overrun occured when sending */
> +#define USB2CAN_STATUSMSG_BUSLIGHT 0x02 /* Error counter has reached 96 */
> +#define USB2CAN_STATUSMSG_BUSHEAVY 0x03 /* Error count. has reached 128 */
> +#define USB2CAN_STATUSMSG_BUSOFF 0x04 /* Device is in BUSOFF */
> +#define USB2CAN_STATUSMSG_STUFF 0x20 /* Stuff Error */
> +#define USB2CAN_STATUSMSG_FORM 0x21 /* Form Error */
> +#define USB2CAN_STATUSMSG_ACK 0x23 /* Ack Error */
> +#define USB2CAN_STATUSMSG_BIT0 0x24 /* Bit1 Error */
> +#define USB2CAN_STATUSMSG_BIT1 0x25 /* Bit0 Error */
> +#define USB2CAN_STATUSMSG_CRC 0x26 /* CRC Error */
> +
> +#define USB2CAN_RP_MASK 0x7F /* Mask for Receive Error Bit */
> +
> +
> +/* table of devices that work with this driver */
> +static struct usb_device_id usb2can_table[] = {
> + { USB_DEVICE(USB2CAN_VENDOR_ID, USB2CAN_PRODUCT_ID) },
> + { } /* Terminating entry */
> +};
> +
> +MODULE_DEVICE_TABLE(usb, usb2can_table);
> +
> +struct usb2can_tx_urb_context {
> + struct usb2can *dev;
> +
> + u32 echo_index;
> + u8 dlc;
> +};
> +
> +/* Structure to hold all of our device specific stuff */
> +struct usb2can {
> + struct can_priv can; /* must be the first member */
> +
> + struct sk_buff *echo_skb[MAX_TX_URBS];
> +
> + struct usb_device *udev;
> + struct net_device *netdev;
> +
> + atomic_t active_tx_urbs;
> + struct usb_anchor tx_submitted;
> + struct usb2can_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> + struct usb_anchor rx_submitted;
> +
> + u8 *cmd_msg_buffer;
> +
> + unsigned int free_slots; /* remember number of available slots */
> +
> + unsigned int dar; /* disable automatic restransmission */
> +
> + struct mutex usb2can_cmd_lock;
> +};
> +
> +/* tx frame */
> +struct __packed usb2can_tx_msg {
> + u8 begin;
> + u8 flags; /* RTR and EXT_ID flag */
> + __be32 id; /* upper 3 bits not used */
> + u8 dlc; /* data length code 0-8 bytes */
> + u8 data[8]; /* 64-bit data */
> + u8 end;
> +};
> +
> +/* rx frame */
> +struct __packed usb2can_rx_msg {
> + u8 begin;
> + u8 type; /* frame type */
> + u8 flags; /* RTR and EXT_ID flag */
> + __be32 id; /* upper 3 bits not used */
> + u8 dlc; /* data length code 0-8 bytes */
> + u8 data[8]; /* 64-bit data */
> + __be32 timestamp; /* 32-bit timestamp */
> + u8 end;
> +};
> +
> +/* command frame */
> +struct __packed usb2can_cmd_msg {
> + u8 begin;
> + u8 channel; /* unkown - always 0 */
> + u8 command; /* command to execute */
> + u8 opt1; /* optional parameter / return value */
> + u8 opt2; /* optional parameter 2 */
> + u8 data[10]; /* optional parameter and data */
> + u8 end;
> +};
> +
> +static struct usb_driver usb2can_driver;
> +
> +static int usb2can_send_cmd_msg(struct usb2can *dev, u8 *msg, int size)
> +{
> + int actual_length;
> +
> + return usb_bulk_msg(dev->udev,
> + usb_sndbulkpipe(dev->udev, 4),
> + msg,
> + size,
> + &actual_length,
> + 1000);
> +}
> +
> +static int usb2can_wait_cmd_msg(struct usb2can *dev, u8 *msg, int size,
> + int *actual_length)
> +{
> + return usb_bulk_msg(dev->udev,
> + usb_rcvbulkpipe(dev->udev, 3),
> + msg,
> + size,
> + actual_length,
> + 1000);
> +}
> +
> +/* Send command to device and receive result.
> + * Command was successful When opt1 = 0.
> + */
> +static int usb2can_send_cmd(struct usb2can *dev, struct usb2can_cmd_msg *out,
> + struct usb2can_cmd_msg *in)
> +{
> + int err;
> + int nBytesRead;
Remove tabs here.
> + struct net_device *netdev;
> +
> + netdev = dev->netdev;
> +
> + out->begin = USB2CAN_CMD_START;
> + out->end = USB2CAN_CMD_END;
> +
> + memcpy(&dev->cmd_msg_buffer[0], out,
> + sizeof(struct usb2can_cmd_msg));
> +
> + mutex_lock(&dev->usb2can_cmd_lock);
> +
> + err = usb2can_send_cmd_msg(dev, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg));
> + if (err < 0) {
> + dev_err(netdev->dev.parent, "sending command message failed\n");
> + return err;
> + }
> +
> + err = usb2can_wait_cmd_msg(dev, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg), &nBytesRead);
> + if (err < 0) {
> + dev_err(netdev->dev.parent, "no command message answer\n");
> + return err;
> + }
> +
> + mutex_unlock(&dev->usb2can_cmd_lock);
> +
> + memcpy(in, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg));
> +
> + if (in->begin != USB2CAN_CMD_START || in->end != USB2CAN_CMD_END ||
> + nBytesRead != 16 || in->opt1 != 0)
> + return -EPROTO;
> +
> + return 0;
> +}
> +
> +/* Send open command to device */
> +static int usb2can_cmd_open(struct usb2can *dev)
> +{
> + struct can_bittiming *bt = &dev->can.bittiming;
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
remove tabs
> + u32 flags = 0x00000000;
0
> + u32 beflags;
> + u16 bebrp;
> + u32 ctrlmode = dev->can.ctrlmode;
> +
> + if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
> + flags |= USB2CAN_LOOPBACK;
> + if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + flags |= USB2CAN_SILENT;
> + if (dev->dar == 1)
> + flags |= USB2CAN_DISABLE_AUTO_RESTRANS;
> +
> + flags |= USB2CAN_STATUS_FRAME;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_OPEN;
> + outmsg.opt1 = USB2CAN_BAUD_MANUAL;
> + outmsg.data[0] = (u8) (bt->prop_seg + bt->phase_seg1);
> + outmsg.data[1] = (u8) bt->phase_seg2;
> + outmsg.data[2] = (u8) bt->sjw;
> +
> + /* BRP */
> + bebrp = cpu_to_be16((u16) bt->brp);
> + memcpy(&outmsg.data[3], &bebrp, sizeof(bebrp));
> +
> + /* flags */
> + beflags = cpu_to_be32(flags);
> + memcpy(&outmsg.data[5], &beflags, sizeof(beflags));
> +
> + return usb2can_send_cmd(dev, &outmsg, &inmsg);
> +}
> +
> +/* Send close command to device */
> +static int usb2can_cmd_close(struct usb2can *dev)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_CLOSE;
> +
> + return usb2can_send_cmd(dev, &outmsg, &inmsg);
> +}
> +
> +/* Get firmware and hardware version */
> +static int usb2can_cmd_version(struct usb2can *dev, u32 *res)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_SOFTW_HARDW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return err;
> +
> + value = (u32 *) inmsg.data;
> + *res = be32_to_cpu(*value);
> +
> + return err;
> +}
> +
> +/* Get firmware version */
> +static ssize_t show_firmware(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u16 *value;
> + u16 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_SOFTW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u16 *) inmsg.data;
> + result = be16_to_cpu(*value);
> +
> + return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
> +}
> +
> +/* Get hardware version */
> +static ssize_t show_hardware(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u16 *value;
> + u16 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_HARDW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u16 *) inmsg.data;
> + result = be16_to_cpu(*value);
> +
> + return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
> +}
> +
> +/* Get status
> + *
> + * Returns:
> + * STATUS_NONE 0x00000000
> + * STATUS_BUS_OFF 0x80000000
> + * STATUS_PASSIVE 0x40000000
> + * STATUS_BUS_WARN 0x20000000
> + * STATUS_ACTIVE 0x10000000
> + * STATUS_PHY_FAULT 0x08000000
> + * STATUS_PHY_H 0x04000000
> + * STATUS_PHY_L 0x02000000
> + * STATUS_SLEEPING 0x01000000
> + * STATUS_STOPPED 0x00800000
> + */
> +static ssize_t show_status(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> + u32 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATUS;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u32 *) inmsg.data;
> + result = be32_to_cpu(*value);
> +
> + return sprintf(buf, "0x%08x\n", result);
> +}
> +
> +/* Get statistic values */
> +static ssize_t show_statistics(struct device *d, struct device_attribute *attr,
> + u8 statistic, char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> + u32 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATISTICS;
> + outmsg.opt1 = statistic;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u32 *) inmsg.data;
> + result = be32_to_cpu(*value);
> +
> + return sprintf(buf, "%d\n", result);
> +}
> +
> +static ssize_t show_rx_frames(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_FRAMES, buf);
> +}
> +
> +static ssize_t show_rx_bytes(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
> +}
> +
> +static ssize_t show_tx_frames(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_TX_FRAMES, buf);
> +}
> +
> +static ssize_t show_tx_bytes(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
> +}
> +
> +static ssize_t show_overruns(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_OVERRUNS, buf);
> +}
> +
> +static ssize_t show_warnings(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_WARNINGS, buf);
> +}
> +
> +static ssize_t show_bus_off(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_BUS_OFF, buf);
> +}
> +
> +/* Reset statistics */
> +static ssize_t reset_statistics(struct device *d, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + if (buf[0] == '1') {
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATISTICS;
> + outmsg.opt1 = USB2CAN_STAT_RESET_STAT;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> + }
> +
> + return count;
> +}
> +
> +/* Get "disable automatic retransmission" flag */
> +static ssize_t show_dar(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + return sprintf(buf, "%d\n", dev->dar);
> +}
> +
> +/* Set "disable automatic retransmission" flag */
> +static ssize_t set_dar(struct device *d, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + if (dev->can.state != CAN_STATE_STOPPED) {
> + dev_err(&intf->dev,
> + "DAR flag can only be set when device is stopped\n");
> + return -EIO;
> + }
> +
> + if (buf[0] == '0')
> + dev->dar = 0;
> + else if (buf[0] == '1')
> + dev->dar = 1;
> + else
> + return -EIO;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
> +static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
> +static DEVICE_ATTR(can_state, S_IRUGO, show_status, NULL);
> +static DEVICE_ATTR(can_rx_frames, S_IRUGO, show_rx_frames, NULL);
> +static DEVICE_ATTR(can_rx_bytes, S_IRUGO, show_rx_bytes, NULL);
> +static DEVICE_ATTR(can_tx_frames, S_IRUGO, show_tx_frames, NULL);
> +static DEVICE_ATTR(can_tx_bytes, S_IRUGO, show_tx_bytes, NULL);
> +static DEVICE_ATTR(can_overruns, S_IRUGO, show_overruns, NULL);
> +static DEVICE_ATTR(can_warnings, S_IRUGO, show_warnings, NULL);
> +static DEVICE_ATTR(can_bus_off_counter, S_IRUGO, show_bus_off, NULL);
I don't think that creating all these files are a good idea.
Probably there is a more generic approach for that.
Let's wait for other peoples feedback.
Regards,
Oliver
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-02 10:36 ` Oliver Hartkopp
@ 2012-12-02 11:45 ` Kurt Van Dijck
0 siblings, 0 replies; 10+ messages in thread
From: Kurt Van Dijck @ 2012-12-02 11:45 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: krumboeck@universalnet.at, linux-can, info, gediminas
Hello,
>>
>> +/* Get statistic values */
>> +static ssize_t show_statistics(struct device *d, struct device_attribute *attr,
>> + u8 statistic, char *buf)
>> +{
>> + struct usb2can_cmd_msg outmsg;
>> + struct usb2can_cmd_msg inmsg;
>> + int err = 0;
>> + u32 *value;
>> + u32 result;
>> + struct usb_interface *intf = to_usb_interface(d);
>> + struct usb2can *dev = usb_get_intfdata(intf);
>> +
>> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
>> + outmsg.command = USB2CAN_GET_STATISTICS;
>> + outmsg.opt1 = statistic;
>> +
>> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
>> + if (err)
>> + return -EIO;
>> +
>> + value = (u32 *) inmsg.data;
>> + result = be32_to_cpu(*value);
>> +
>> + return sprintf(buf, "%d\n", result);
>> +}
>> +
>> +static ssize_t show_rx_frames(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_RX_FRAMES, buf);
>> +}
>> +
>> +static ssize_t show_rx_bytes(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
>> +}
>> +
>> +static ssize_t show_tx_frames(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_TX_FRAMES, buf);
>> +}
>> +
>> +static ssize_t show_tx_bytes(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
>> +}
>> +
>> +static ssize_t show_overruns(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_OVERRUNS, buf);
>> +}
>> +
>> +static ssize_t show_warnings(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_WARNINGS, buf);
>> +}
>> +
>> +static ssize_t show_bus_off(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + return show_statistics(d, attr, USB2CAN_STAT_BUS_OFF, buf);
>> +}
>> +
>> +/* Reset statistics */
>> +static ssize_t reset_statistics(struct device *d, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct usb2can_cmd_msg outmsg;
>> + struct usb2can_cmd_msg inmsg;
>> + int err = 0;
>> + struct usb_interface *intf = to_usb_interface(d);
>> + struct usb2can *dev = usb_get_intfdata(intf);
>> +
>> + if (buf[0] == '1') {
>> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
>> + outmsg.command = USB2CAN_GET_STATISTICS;
>> + outmsg.opt1 = USB2CAN_STAT_RESET_STAT;
>> +
>> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
>> + if (err)
>> + return -EIO;
>> + }
>> +
>> + return count;
>> +}
>> +
>> +/* Get "disable automatic retransmission" flag */
>> +static ssize_t show_dar(struct device *d, struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct usb_interface *intf = to_usb_interface(d);
>> + struct usb2can *dev = usb_get_intfdata(intf);
>> +
>> + return sprintf(buf, "%d\n", dev->dar);
>> +}
>> +
>> +/* Set "disable automatic retransmission" flag */
>> +static ssize_t set_dar(struct device *d, struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct usb_interface *intf = to_usb_interface(d);
>> + struct usb2can *dev = usb_get_intfdata(intf);
>> +
>> + if (dev->can.state != CAN_STATE_STOPPED) {
>> + dev_err(&intf->dev,
>> + "DAR flag can only be set when device is stopped\n");
>> + return -EIO;
>> + }
>> +
>> + if (buf[0] == '0')
>> + dev->dar = 0;
>> + else if (buf[0] == '1')
>> + dev->dar = 1;
>> + else
>> + return -EIO;
>> +
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
>> +static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
>> +static DEVICE_ATTR(can_state, S_IRUGO, show_status, NULL);
>> +static DEVICE_ATTR(can_rx_frames, S_IRUGO, show_rx_frames, NULL);
>> +static DEVICE_ATTR(can_rx_bytes, S_IRUGO, show_rx_bytes, NULL);
>> +static DEVICE_ATTR(can_tx_frames, S_IRUGO, show_tx_frames, NULL);
>> +static DEVICE_ATTR(can_tx_bytes, S_IRUGO, show_tx_bytes, NULL);
>> +static DEVICE_ATTR(can_overruns, S_IRUGO, show_overruns, NULL);
>> +static DEVICE_ATTR(can_warnings, S_IRUGO, show_warnings, NULL);
>> +static DEVICE_ATTR(can_bus_off_counter, S_IRUGO, show_bus_off, NULL);
>
>
> I don't think that creating all these files are a good idea.
> Probably there is a more generic approach for that.
>
> Let's wait for other peoples feedback.
Statistics are exported already via 'ip link'. What is the added value
of these sysfs?
Kurt?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-02 9:25 [PATCH] usb2can: Add support for USB2CAN interface from 8 devices krumboeck
2012-12-02 10:36 ` Oliver Hartkopp
@ 2012-12-02 13:35 ` Wolfgang Grandegger
2012-12-03 0:43 ` krumboeck
1 sibling, 1 reply; 10+ messages in thread
From: Wolfgang Grandegger @ 2012-12-02 13:35 UTC (permalink / raw)
To: krumboeck@universalnet.at; +Cc: linux-can, info, gediminas
Hi Bernd,
nice to see this driver being pushed mainline. As Oliver already pointed
out, there are a few general naming and coding style issues:
- usb2can is to general, a more specific name would be nice, e.g.
usb_8dev, also as prefix for macro, function and variable names.
[For the latter, the name is not allowed to start with a number and
therefore the prefix 8dev_usb is not an option].
- Tabs are OK just for aligning macro definition. In all other case
please use just *one* space, e.g.:
s/int err;/int err;/
s/outmsg.opt1 = USB2CAN/outmsg.opt1 = USB2CAN/
- The preferred multi line comment styles is:
/*
* A Comment
*/
- The patch does not yet apply to David Millers "net-next" GIT tree.
There are problems with Kconfig and Makefile.
- Care about casts. Try to avoid them.
- Sooner than later, you should also add a CC to the Linux USB ml.
More comments inline...
On 12/02/2012 10:25 AM, krumboeck@universalnet.at wrote:
> Add device driver for USB2CAN interface from "8 devices"
> (http://www.8devices.com).
>
> Signed-off-by: Bernd Krumboeck <krumboeck@universalnet.at>
> ---
> drivers/net/can/usb/Kconfig | 6 +
> drivers/net/can/usb/Makefile | 1 +
> drivers/net/can/usb/usb2can.c | 1323
> +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1330 insertions(+)
> create mode 100644 drivers/net/can/usb/usb2can.c
>
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index a4e4bee..2068c99 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -48,4 +48,10 @@ config CAN_PEAK_USB
> This driver supports the PCAN-USB and PCAN-USB Pro adapters
> from PEAK-System Technik (http://www.peak-system.com).
>
> +config CAN_USB2CAN
> + tristate "8 devices USB2CAN interface"
> + ---help---
> + This driver supports the USB2CAN interface
> + from 8 devices (http://www.8devices.com).
> +
> endmenu
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index 80a2ee4..3c0a378 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -6,5 +6,6 @@ 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/
> +obj-$(CONFIG_CAN_USB2CAN) += usb2can.o
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/usb2can.c b/drivers/net/can/usb/usb2can.c
> new file mode 100644
> index 0000000..5a09141
> --- /dev/null
> +++ b/drivers/net/can/usb/usb2can.c
> @@ -0,0 +1,1323 @@
> +/*
> + * CAN driver for UAB "8 devices" USB2CAN converter
> + *
> + * Copyright (C) 2012 Bernd Krumboeck (krumboeck@universalnet.at)
> + *
> + * 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 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + * This driver is based on the 3.2.0 version of
> drivers/net/can/usb/ems_usb.c
> + * and drivers/net/can/usb/esd_usb2.c
If you say "based on", you should honor the copyrights. "inspired by"
sounds better or just drop these lines.
Please check if there are fixes since 3.2.0.
> + *
> + * Many thanks to Gerhard Bertelsmann (info@gerhard-bertelsmann.de)
> + * for testing and fixing this driver. Also many thanks to "8 devices",
> + * who were very cooperative and answered my questions.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/signal.h>
> +#include <linux/slab.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>
> +
> +
> +/* driver constants */
> +#define MAX_RX_URBS 10
> +#define MAX_TX_URBS 10
> +#define RX_BUFFER_SIZE 64
> +
> +/* vendor and product id */
> +#define USB2CAN_VENDOR_ID 0x0483
> +#define USB2CAN_PRODUCT_ID 0x1234
> +
> +/* bittiming constants */
> +#define USB2CAN_ABP_CLOCK 32000000
> +#define USB2CAN_BAUD_MANUAL 0x09
> +#define USB2CAN_TSEG1_MIN 1
> +#define USB2CAN_TSEG1_MAX 16
> +#define USB2CAN_TSEG2_MIN 1
> +#define USB2CAN_TSEG2_MAX 8
> +#define USB2CAN_SJW_MAX 4
> +#define USB2CAN_BRP_MIN 1
> +#define USB2CAN_BRP_MAX 1024
> +#define USB2CAN_BRP_INC 1
> +
> +/* setup flags */
> +#define USB2CAN_SILENT 0x00000001
> +#define USB2CAN_LOOPBACK 0x00000002
> +#define USB2CAN_DISABLE_AUTO_RESTRANS 0x00000004
> +#define USB2CAN_STATUS_FRAME 0x00000008
> +
> +/* commands */
> +#define USB2CAN_RESET 1
> +#define USB2CAN_OPEN 2
> +#define USB2CAN_CLOSE 3
> +#define USB2CAN_SET_SPEED 4
> +#define USB2CAN_SET_MASK_FILTER 5
> +#define USB2CAN_GET_STATUS 6
> +#define USB2CAN_GET_STATISTICS 7
> +#define USB2CAN_GET_SERIAL 8
> +#define USB2CAN_GET_SOFTW_VER 9
> +#define USB2CAN_GET_HARDW_VER 10
> +#define USB2CAN_RESET_TIMESTAMP 11
> +#define USB2CAN_GET_SOFTW_HARDW_VER 12
This looks like an enumeration.
> +#define USB2CAN_CMD_START 0x11
> +#define USB2CAN_CMD_END 0x22
> +
> +#define USB2CAN_CMD_SUCCESS 0
> +#define USB2CAN_CMD_ERROR 255
> +
> +/* statistics */
> +#define USB2CAN_STAT_RX_FRAMES 0
> +#define USB2CAN_STAT_RX_BYTES 1
> +#define USB2CAN_STAT_TX_FRAMES 2
> +#define USB2CAN_STAT_TX_BYTES 3
> +#define USB2CAN_STAT_OVERRUNS 4
> +#define USB2CAN_STAT_WARNINGS 5
> +#define USB2CAN_STAT_BUS_OFF 6
> +#define USB2CAN_STAT_RESET_STAT 7
> +
> +/* frames */
> +#define USB2CAN_DATA_START 0x55
> +#define USB2CAN_DATA_END 0xAA
> +
> +#define USB2CAN_TYPE_CAN_FRAME 0
> +#define USB2CAN_TYPE_ERROR_FRAME 3
> +
> +#define USB2CAN_EXTID 0x01
> +#define USB2CAN_RTR 0x02
> +#define USB2CAN_ERR_FLAG 0x04
> +
> +/* status */
> +#define USB2CAN_STATUSMSG_OK 0x00 /* Normal condition. */
> +#define USB2CAN_STATUSMSG_OVERRUN 0x01 /* Overrun occured when
> sending */
> +#define USB2CAN_STATUSMSG_BUSLIGHT 0x02 /* Error counter has
> reached 96 */
> +#define USB2CAN_STATUSMSG_BUSHEAVY 0x03 /* Error count. has reached
> 128 */
> +#define USB2CAN_STATUSMSG_BUSOFF 0x04 /* Device is in BUSOFF */
> +#define USB2CAN_STATUSMSG_STUFF 0x20 /* Stuff Error */
> +#define USB2CAN_STATUSMSG_FORM 0x21 /* Form Error */
> +#define USB2CAN_STATUSMSG_ACK 0x23 /* Ack Error */
> +#define USB2CAN_STATUSMSG_BIT0 0x24 /* Bit1 Error */
> +#define USB2CAN_STATUSMSG_BIT1 0x25 /* Bit0 Error */
> +#define USB2CAN_STATUSMSG_CRC 0x26 /* CRC Error */
> +
> +#define USB2CAN_RP_MASK 0x7F /* Mask for Receive Error Bit */
> +
> +
> +/* table of devices that work with this driver */
> +static struct usb_device_id usb2can_table[] = {
> + { USB_DEVICE(USB2CAN_VENDOR_ID, USB2CAN_PRODUCT_ID) },
> + { } /* Terminating entry */
> +};
> +
> +MODULE_DEVICE_TABLE(usb, usb2can_table);
> +
> +struct usb2can_tx_urb_context {
> + struct usb2can *dev;
> +
> + u32 echo_index;
> + u8 dlc;
> +};
> +
> +/* Structure to hold all of our device specific stuff */
> +struct usb2can {
> + struct can_priv can; /* must be the first member */
> +
> + struct sk_buff *echo_skb[MAX_TX_URBS];
> +
> + struct usb_device *udev;
> + struct net_device *netdev;
> +
> + atomic_t active_tx_urbs;
> + struct usb_anchor tx_submitted;
> + struct usb2can_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> + struct usb_anchor rx_submitted;
> +
> + u8 *cmd_msg_buffer;
> +
> + unsigned int free_slots; /* remember number of available slots */
> +
> + unsigned int dar; /* disable automatic restransmission */
> +
> + struct mutex usb2can_cmd_lock;
> +};
> +
> +/* tx frame */
> +struct __packed usb2can_tx_msg {
> + u8 begin;
> + u8 flags; /* RTR and EXT_ID flag */
> + __be32 id; /* upper 3 bits not used */
> + u8 dlc; /* data length code 0-8 bytes */
> + u8 data[8]; /* 64-bit data */
> + u8 end;
> +};
> +
> +/* rx frame */
> +struct __packed usb2can_rx_msg {
> + u8 begin;
> + u8 type; /* frame type */
> + u8 flags; /* RTR and EXT_ID flag */
> + __be32 id; /* upper 3 bits not used */
> + u8 dlc; /* data length code 0-8 bytes */
> + u8 data[8]; /* 64-bit data */
> + __be32 timestamp; /* 32-bit timestamp */
> + u8 end;
> +};
> +
> +/* command frame */
> +struct __packed usb2can_cmd_msg {
> + u8 begin;
> + u8 channel; /* unkown - always 0 */
> + u8 command; /* command to execute */
> + u8 opt1; /* optional parameter / return value */
> + u8 opt2; /* optional parameter 2 */
> + u8 data[10]; /* optional parameter and data */
> + u8 end;
> +};
> +
> +static struct usb_driver usb2can_driver;
> +
> +static int usb2can_send_cmd_msg(struct usb2can *dev, u8 *msg, int size)
> +{
> + int actual_length;
> +
> + return usb_bulk_msg(dev->udev,
> + usb_sndbulkpipe(dev->udev, 4),
> + msg,
> + size,
> + &actual_length,
> + 1000);
Does fit on less lines!
> +}
> +
> +static int usb2can_wait_cmd_msg(struct usb2can *dev, u8 *msg, int size,
> + int *actual_length)
> +{
> + return usb_bulk_msg(dev->udev,
> + usb_rcvbulkpipe(dev->udev, 3),
> + msg,
> + size,
> + actual_length,
> + 1000);
Ditto
> +}
> +
> +/* Send command to device and receive result.
> + * Command was successful When opt1 = 0.
> + */
> +static int usb2can_send_cmd(struct usb2can *dev, struct usb2can_cmd_msg
> *out,
> + struct usb2can_cmd_msg *in)
> +{
> + int err;
> + int nBytesRead;
Please use the usual variable name style. All lowercase, eventuell with
"_". Here and in other places as well.
> + struct net_device *netdev;
> +
> + netdev = dev->netdev;
> +
> + out->begin = USB2CAN_CMD_START;
> + out->end = USB2CAN_CMD_END;
> +
> + memcpy(&dev->cmd_msg_buffer[0], out,
> + sizeof(struct usb2can_cmd_msg));
> +
> + mutex_lock(&dev->usb2can_cmd_lock);
> +
> + err = usb2can_send_cmd_msg(dev, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg));
> + if (err < 0) {
> + dev_err(netdev->dev.parent, "sending command message failed\n");
> + return err;
> + }
> +
> + err = usb2can_wait_cmd_msg(dev, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg), &nBytesRead);
> + if (err < 0) {
> + dev_err(netdev->dev.parent, "no command message answer\n");
> + return err;
> + }
> +
> + mutex_unlock(&dev->usb2can_cmd_lock);
> +
> + memcpy(in, &dev->cmd_msg_buffer[0],
> + sizeof(struct usb2can_cmd_msg));
> +
> + if (in->begin != USB2CAN_CMD_START || in->end != USB2CAN_CMD_END ||
> + nBytesRead != 16 || in->opt1 != 0)
> + return -EPROTO;
> +
> + return 0;
> +}
> +
> +/* Send open command to device */
> +static int usb2can_cmd_open(struct usb2can *dev)
> +{
> + struct can_bittiming *bt = &dev->can.bittiming;
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + u32 flags = 0x00000000;
> + u32 beflags;
> + u16 bebrp;
> + u32 ctrlmode = dev->can.ctrlmode;
> +
> + if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
> + flags |= USB2CAN_LOOPBACK;
> + if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + flags |= USB2CAN_SILENT;
> + if (dev->dar == 1)
> + flags |= USB2CAN_DISABLE_AUTO_RESTRANS;
> +
> + flags |= USB2CAN_STATUS_FRAME;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_OPEN;
> + outmsg.opt1 = USB2CAN_BAUD_MANUAL;
> + outmsg.data[0] = (u8) (bt->prop_seg + bt->phase_seg1);
> + outmsg.data[1] = (u8) bt->phase_seg2;
> + outmsg.data[2] = (u8) bt->sjw;
I don't think you need the casts here.
> + /* BRP */
> + bebrp = cpu_to_be16((u16) bt->brp);
> + memcpy(&outmsg.data[3], &bebrp, sizeof(bebrp));
> +
> + /* flags */
> + beflags = cpu_to_be32(flags);
> + memcpy(&outmsg.data[5], &beflags, sizeof(beflags));
> +
> + return usb2can_send_cmd(dev, &outmsg, &inmsg);
> +}
> +
> +/* Send close command to device */
> +static int usb2can_cmd_close(struct usb2can *dev)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_CLOSE;
> +
> + return usb2can_send_cmd(dev, &outmsg, &inmsg);
> +}
> +
> +/* Get firmware and hardware version */
> +static int usb2can_cmd_version(struct usb2can *dev, u32 *res)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_SOFTW_HARDW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return err;
> +
> + value = (u32 *) inmsg.data;
> + *res = be32_to_cpu(*value);
I do not see a need for the extra variable "value" here.
> +
> + return err;
> +}
> +
> +/* Get firmware version */
> +static ssize_t show_firmware(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u16 *value;
> + u16 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_SOFTW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u16 *) inmsg.data;
> + result = be16_to_cpu(*value);
Ditto.
> + return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
> +}
> +
> +/* Get hardware version */
> +static ssize_t show_hardware(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u16 *value;
> + u16 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_HARDW_VER;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u16 *) inmsg.data;
> + result = be16_to_cpu(*value);
Ditto.
> +
> + return sprintf(buf, "%d.%d\n", (u8)(result>>8), (u8)result);
> +}
> +
> +/* Get status
> + *
> + * Returns:
> + * STATUS_NONE 0x00000000
> + * STATUS_BUS_OFF 0x80000000
> + * STATUS_PASSIVE 0x40000000
> + * STATUS_BUS_WARN 0x20000000
> + * STATUS_ACTIVE 0x10000000
> + * STATUS_PHY_FAULT 0x08000000
> + * STATUS_PHY_H 0x04000000
> + * STATUS_PHY_L 0x02000000
> + * STATUS_SLEEPING 0x01000000
> + * STATUS_STOPPED 0x00800000
> + */
> +static ssize_t show_status(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> + u32 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATUS;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u32 *) inmsg.data;
> + result = be32_to_cpu(*value);
Ditto.
> + return sprintf(buf, "0x%08x\n", result);
> +}
> +
> +/* Get statistic values */
> +static ssize_t show_statistics(struct device *d, struct
> device_attribute *attr,
> + u8 statistic, char *buf)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + u32 *value;
> + u32 result;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATISTICS;
> + outmsg.opt1 = statistic;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> +
> + value = (u32 *) inmsg.data;
> + result = be32_to_cpu(*value);
> +
> + return sprintf(buf, "%d\n", result);
> +}
> +
> +static ssize_t show_rx_frames(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_FRAMES, buf);
> +}
> +
> +static ssize_t show_rx_bytes(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
> +}
> +
> +static ssize_t show_tx_frames(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_TX_FRAMES, buf);
> +}
> +
> +static ssize_t show_tx_bytes(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_RX_BYTES, buf);
> +}
> +
> +static ssize_t show_overruns(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_OVERRUNS, buf);
> +}
> +
> +static ssize_t show_warnings(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_WARNINGS, buf);
> +}
> +
> +static ssize_t show_bus_off(struct device *d, struct device_attribute
> *attr,
> + char *buf)
> +{
> + return show_statistics(d, attr, USB2CAN_STAT_BUS_OFF, buf);
> +}
> +
> +/* Reset statistics */
> +static ssize_t reset_statistics(struct device *d, struct
> device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb2can_cmd_msg outmsg;
> + struct usb2can_cmd_msg inmsg;
> + int err = 0;
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + if (buf[0] == '1') {
> + memset(&outmsg, 0, sizeof(struct usb2can_cmd_msg));
> + outmsg.command = USB2CAN_GET_STATISTICS;
> + outmsg.opt1 = USB2CAN_STAT_RESET_STAT;
> +
> + err = usb2can_send_cmd(dev, &outmsg, &inmsg);
> + if (err)
> + return -EIO;
> + }
> +
> + return count;
> +}
> +
> +/* Get "disable automatic retransmission" flag */
> +static ssize_t show_dar(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + return sprintf(buf, "%d\n", dev->dar);
> +}
> +
> +/* Set "disable automatic retransmission" flag */
> +static ssize_t set_dar(struct device *d, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_interface *intf = to_usb_interface(d);
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + if (dev->can.state != CAN_STATE_STOPPED) {
> + dev_err(&intf->dev,
> + "DAR flag can only be set when device is stopped\n");
> + return -EIO;
> + }
> +
> + if (buf[0] == '0')
> + dev->dar = 0;
> + else if (buf[0] == '1')
> + dev->dar = 1;
> + else
> + return -EIO;
> +
> + return count;
> +}
Automatic retransmission means one-shot mode? We handle this usually via
"ctrlmode_supported" flag CAN_CTRLMODE_ONE_SHOT.
> +static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
> +static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
> +static DEVICE_ATTR(can_state, S_IRUGO, show_status, NULL);
> +static DEVICE_ATTR(can_rx_frames, S_IRUGO, show_rx_frames, NULL);
> +static DEVICE_ATTR(can_rx_bytes, S_IRUGO, show_rx_bytes, NULL);
> +static DEVICE_ATTR(can_tx_frames, S_IRUGO, show_tx_frames, NULL);
> +static DEVICE_ATTR(can_tx_bytes, S_IRUGO, show_tx_bytes, NULL);
> +static DEVICE_ATTR(can_overruns, S_IRUGO, show_overruns, NULL);
> +static DEVICE_ATTR(can_warnings, S_IRUGO, show_warnings, NULL);
> +static DEVICE_ATTR(can_bus_off_counter, S_IRUGO, show_bus_off, NULL);
> +
> +static DEVICE_ATTR(can_reset_statistics, S_IWUSR, NULL, reset_statistics);
> +
> +static DEVICE_ATTR(can_disable_automatic_retransmission, S_IRUGO |
> S_IWUSR,
> + show_dar, set_dar);
> +
> +/* Set network device mode
> + *
> + * Maybe we should leave this function empty, because the device
> + * set mode variable with open command.
> + */
> +static int usb2can_set_mode(struct net_device *netdev, enum can_mode mode)
> +{
> + struct usb2can *dev = netdev_priv(netdev);
> + int err = 0;
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + err = usb2can_cmd_open(dev);
> + if (err)
> + dev_warn(netdev->dev.parent, "couldn't start device");
> +
> + if (netif_queue_stopped(netdev))
> + netif_wake_queue(netdev);
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +/* Empty function: We set bittiming when we start the interface.
> + * This is a firmware limitation.
> + */
> +static int usb2can_set_bittiming(struct net_device *netdev)
> +{
> + return 0;
> +}
> +
> +/* Read data and status frames */
> +static void usb2can_rx_can_msg(struct usb2can *dev, struct
> usb2can_rx_msg *msg)
> +{
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + int i;
> + struct net_device_stats *stats = &dev->netdev->stats;
> +
> + skb = alloc_can_skb(dev->netdev, &cf);
> + if (skb == NULL)
s/skb == NULL/!skb/ is preferred.
Here and in maybe in other places as well.
> + return;
PLease use alloc_can_skb for real messages ...-
> + if (msg->type == USB2CAN_TYPE_CAN_FRAME) {
> + cf->can_id = be32_to_cpu(msg->id);
> + cf->can_dlc = get_can_dlc(msg->dlc & 0xF);
> +
> + if (msg->flags & USB2CAN_EXTID)
> + cf->can_id |= CAN_EFF_FLAG;
> +
> + if (msg->flags & USB2CAN_RTR)
> + cf->can_id |= CAN_RTR_FLAG;
> + else
> + for (i = 0; i < cf->can_dlc; i++)
> + cf->data[i] = msg->data[i];
> + } else if (msg->type == USB2CAN_TYPE_ERROR_FRAME &&
> + msg->flags == USB2CAN_ERR_FLAG) {
... and alloc_can_err_skb for error messages.
> +
> + /* Error message:
> + byte 0: Status
> + byte 1: bit 7: Receive Passive
> + byte 1: bit 0-6: Receive Error Counter
> + byte 2: Transmit Error Counter
> + byte 3: Always 0 (maybe reserved for future use)
> + */
> +
> + u8 state = msg->data[0];
> + u8 rxerr = msg->data[1] & USB2CAN_RP_MASK;
> + u8 txerr = msg->data[2];
In principle, you could then also support the "do_get_berr_counter"
callback.
> + int rx_errors = 0;
> + int tx_errors = 0;
> +
> + dev->can.can_stats.bus_error++;
> +
> + cf->can_id = CAN_ERR_FLAG;
> + cf->can_dlc = CAN_ERR_DLC;
... making the above two lines obsolete.
> +
> + switch (state) {
> + case USB2CAN_STATUSMSG_OK:
> + dev->can.state = CAN_STATE_ERROR_ACTIVE;
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> + break;
> + case USB2CAN_STATUSMSG_BUSOFF:
> + dev->can.state = CAN_STATE_BUS_OFF;
> + cf->can_id |= CAN_ERR_BUSOFF;
> + can_bus_off(dev->netdev);
> + break;
> + default:
> + dev->can.state = CAN_STATE_ERROR_WARNING;
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> + break;
> + }
> +
> + switch (state) {
> + case USB2CAN_STATUSMSG_ACK:
> + cf->can_id |= CAN_ERR_ACK;
> + tx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_CRC:
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + rx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_BIT0:
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + tx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_BIT1:
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + tx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_FORM:
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + rx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_STUFF:
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + rx_errors = 1;
> + break;
> + case USB2CAN_STATUSMSG_OVERRUN:
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = (txerr > rxerr) ?
> + CAN_ERR_CRTL_TX_OVERFLOW :
> + CAN_ERR_CRTL_RX_OVERFLOW;
> + cf->data[2] |= CAN_ERR_PROT_OVERLOAD;
> + stats->rx_over_errors++;
> + break;
> + case USB2CAN_STATUSMSG_BUSLIGHT:
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = (txerr > rxerr) ?
> + CAN_ERR_CRTL_TX_WARNING :
> + CAN_ERR_CRTL_RX_WARNING;
> + dev->can.can_stats.error_warning++;
> + break;
> + case USB2CAN_STATUSMSG_BUSHEAVY:
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = (txerr > rxerr) ?
> + CAN_ERR_CRTL_TX_PASSIVE :
> + CAN_ERR_CRTL_RX_PASSIVE;
> + dev->can.can_stats.error_passive++;
> + break;
> + default:
> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> + break;
> + }
> +
> + if (tx_errors) {
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + stats->tx_errors++;
> + }
> +
> + if (rx_errors)
> + stats->rx_errors++;
> +
> + cf->data[6] = txerr;
> + cf->data[7] = rxerr;
> +
> + } else {
> + dev_warn(dev->udev->dev.parent, "frame type %d unknown",
> + msg->type);
> + }
Could you please show the output of "candump -e any,0:0,#FFFFFFFF" when
the device goes 1) error passive and 2) bus-off. You could provoke
these states by 1) sending without connection to the bus or 2)
short-circuiting CAN high and low.
Another question: is it possible to switch off bus-error reporting?
> + netif_rx(skb);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> +}
> +
> +/* Callback for reading data from device
> + *
> + * Check urb status, call read function and resubmit urb read operation.
> + */
> +static void usb2can_read_bulk_callback(struct urb *urb)
> +{
> + struct usb2can *dev = urb->context;
> + struct net_device *netdev;
> + int retval;
> + int pos = 0;
> +
> + netdev = dev->netdev;
> +
> + if (!netif_device_present(netdev))
> + return;
> +
> + switch (urb->status) {
> + case 0: /* success */
> + break;
> +
> + case -ENOENT:
> + case -ESHUTDOWN:
> + return;
> +
> + default:
> + dev_info(netdev->dev.parent, "Rx URB aborted (%d)\n",
> + urb->status);
> + goto resubmit_urb;
> + }
> +
> + while (pos < urb->actual_length) {
> + struct usb2can_rx_msg *msg;
> +
> + msg = (struct usb2can_rx_msg *)(urb->transfer_buffer + pos);
> +
> + usb2can_rx_can_msg(dev, msg);
> +
> + pos += sizeof(struct usb2can_rx_msg);
> +
> + if (pos > urb->actual_length) {
> + dev_err(dev->udev->dev.parent, "format error\n");
> + break;
> + }
> + }
> +
> +resubmit_urb:
> + usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
> + urb->transfer_buffer, RX_BUFFER_SIZE,
> + usb2can_read_bulk_callback, dev);
> +
> + retval = usb_submit_urb(urb, GFP_ATOMIC);
> +
> + if (retval == -ENODEV)
> + netif_device_detach(netdev);
> + else if (retval)
> + dev_err(netdev->dev.parent,
> + "failed resubmitting read bulk urb: %d\n", retval);
> +}
> +
> +/* Callback handler for write operations
> + *
> + * Free allocated buffers, check transmit status and
> + * calculate statistic.
> + */
> +static void usb2can_write_bulk_callback(struct urb *urb)
> +{
> + struct usb2can_tx_urb_context *context = urb->context;
> + struct usb2can *dev;
> + struct net_device *netdev;
> +
> + BUG_ON(!context);
> +
> + dev = context->dev;
> + netdev = dev->netdev;
> +
> + /* free up our allocated buffer */
> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> + urb->transfer_buffer, urb->transfer_dma);
> +
> + atomic_dec(&dev->active_tx_urbs);
> +
> + if (!netif_device_present(netdev))
> + return;
> +
> + if (urb->status)
> + dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
> + urb->status);
> +
> + netdev->trans_start = jiffies;
Hm, what is the status of trans_start? Some drivers still do set it,
others don't:
$ find . -name '*.c'|xargs grep -l trans_start
./mscan/mscan.c
./usb/esd_usb2.c
./usb/peak_usb/pcan_usb_core.c
./usb/ems_usb.c
There was some cleanup "1ae5dc342ac78d7a42965fd1f323815f6f5ef2c1"
sometime ago.
> +
> + netdev->stats.tx_packets++;
> + netdev->stats.tx_bytes += context->dlc;
> +
> + can_get_echo_skb(netdev, context->echo_index);
> +
> + /* Release context */
> + context->echo_index = MAX_TX_URBS;
> +
> + if (netif_queue_stopped(netdev))
> + netif_wake_queue(netdev);
> +}
> +
> +/* Send data to device */
> +static netdev_tx_t usb2can_start_xmit(struct sk_buff *skb,
> + struct net_device *netdev)
> +{
> + struct usb2can *dev = netdev_priv(netdev);
> + struct net_device_stats *stats = &netdev->stats;
> + struct can_frame *cf = (struct can_frame *)skb->data;
Is the cast needed?
> + struct usb2can_tx_msg *msg;
> + struct urb *urb;
> + struct usb2can_tx_urb_context *context = NULL;
> + u8 *buf;
> + int i, err;
> + size_t size = sizeof(struct usb2can_tx_msg);
> +
> + if (can_dropped_invalid_skb(netdev, skb))
> + return NETDEV_TX_OK;
> +
> + /* create a URB, and a buffer for it, and copy the data to the URB */
> + urb = usb_alloc_urb(0, GFP_ATOMIC);
> + if (!urb) {
> + dev_err(netdev->dev.parent, "No memory left for URBs\n");
> + goto nomem;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC,
> + &urb->transfer_dma);
> + if (!buf) {
> + dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
> + usb_free_urb(urb);
> + goto nomem;
> + }
> +
> + memset(buf, 0, size);
> +
> + msg = (struct usb2can_tx_msg *)buf;
> + msg->begin = USB2CAN_DATA_START;
> + msg->flags = 0x00;
> +
> + if (cf->can_id & CAN_RTR_FLAG)
> + msg->flags |= USB2CAN_RTR;
> +
> + if (cf->can_id & CAN_EFF_FLAG)
> + msg->flags |= USB2CAN_EXTID;
> +
> + msg->id = cpu_to_be32(cf->can_id & CAN_ERR_MASK);
> +
> + msg->dlc = cf->can_dlc;
> +
> + for (i = 0; i < cf->can_dlc; i++)
> + msg->data[i] = cf->data[i];
> +
> + msg->end = USB2CAN_DATA_END;
> +
> +
> + for (i = 0; i < MAX_TX_URBS; i++) {
> + if (dev->tx_contexts[i].echo_index == MAX_TX_URBS) {
> + context = &dev->tx_contexts[i];
> + break;
> + }
> + }
> +
> + /* May never happen! When this happens we'd more URBs in flight as
> + * allowed (MAX_TX_URBS).
> + */
> + if (!context) {
> + usb_unanchor_urb(urb);
> + usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
> +
> + dev_warn(netdev->dev.parent, "couldn't find free context");
> +
> + return NETDEV_TX_BUSY;
> + }
> +
> + context->dev = dev;
> + context->echo_index = i;
> + context->dlc = cf->can_dlc;
> +
> + usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
> + size, usb2can_write_bulk_callback, context);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &dev->tx_submitted);
> +
> + can_put_echo_skb(skb, netdev, context->echo_index);
> +
> + atomic_inc(&dev->active_tx_urbs);
> +
> + err = usb_submit_urb(urb, GFP_ATOMIC);
> + if (unlikely(err)) {
> + can_free_echo_skb(netdev, context->echo_index);
> +
> + usb_unanchor_urb(urb);
> + usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
> + dev_kfree_skb(skb);
> +
> + atomic_dec(&dev->active_tx_urbs);
> +
> + if (err == -ENODEV) {
> + netif_device_detach(netdev);
> + } else {
> + dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
> +
> + stats->tx_dropped++;
> + }
> + } else {
> + netdev->trans_start = jiffies;
> +
> + /* Slow down tx path */
> + if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS ||
> + dev->free_slots < 5) {
> + netif_stop_queue(netdev);
> + }
> + }
> +
> + /* Release our reference to this URB, the USB core will eventually
> free
> + * it entirely.
> + */
> + usb_free_urb(urb);
> +
> + return NETDEV_TX_OK;
> +
> +nomem:
> + dev_kfree_skb(skb);
> + stats->tx_dropped++;
> +
> + return NETDEV_TX_OK;
> +}
> +
> +
> +/* Start USB device */
> +static int usb2can_start(struct usb2can *dev)
> +{
> + struct net_device *netdev = dev->netdev;
> + int err, i;
> +
> + dev->free_slots = 15; /* initial size */
> +
> + for (i = 0; i < MAX_RX_URBS; i++) {
> + struct urb *urb = NULL;
> + u8 *buf = NULL;
> +
> + /* create a URB, and a buffer for it */
> + urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!urb) {
> + dev_err(netdev->dev.parent,
> + "No memory left for URBs\n");
> + return -ENOMEM;
> + }
> +
> + buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
> + &urb->transfer_dma);
> + if (!buf) {
> + dev_err(netdev->dev.parent,
> + "No memory left for USB buffer\n");
> + usb_free_urb(urb);
> + return -ENOMEM;
> + }
> +
> + usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
> + buf, RX_BUFFER_SIZE,
> + usb2can_read_bulk_callback, dev);
> + urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> + usb_anchor_urb(urb, &dev->rx_submitted);
> +
> + err = usb_submit_urb(urb, GFP_KERNEL);
> + if (err) {
> + if (err == -ENODEV)
> + netif_device_detach(dev->netdev);
> +
> + usb_unanchor_urb(urb);
> + usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> + urb->transfer_dma);
> + break;
> + }
> +
> + /* Drop reference, USB core will take care of freeing it */
> + usb_free_urb(urb);
> + }
> +
> + /* Did we submit any URBs */
> + if (i == 0) {
> + dev_warn(netdev->dev.parent, "couldn't setup read URBs\n");
> + return err;
> + }
> +
> + /* Warn if we've couldn't transmit all the URBs */
> + if (i < MAX_RX_URBS)
> + dev_warn(netdev->dev.parent, "rx performance may be slow\n");
> +
> + err = usb2can_cmd_open(dev);
> + if (err)
> + goto failed;
> +
> + dev->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> + return 0;
> +
> +failed:
> + if (err == -ENODEV)
> + netif_device_detach(dev->netdev);
> +
> + dev_warn(netdev->dev.parent, "couldn't submit control: %d\n", err);
> +
> + return err;
> +}
> +
> +/* Open USB device */
> +static int usb2can_open(struct net_device *netdev)
> +{
> + struct usb2can *dev = netdev_priv(netdev);
> + int err;
> +
> + /* common open */
> + err = open_candev(netdev);
> + if (err)
> + return err;
> +
> + /* finally start device */
> + err = usb2can_start(dev);
> + if (err) {
> + if (err == -ENODEV)
> + netif_device_detach(dev->netdev);
> +
> + dev_warn(netdev->dev.parent, "couldn't start device: %d\n",
> + err);
> +
> + close_candev(netdev);
> +
> + return err;
> + }
> +
> + netif_start_queue(netdev);
> +
> + return 0;
> +}
> +
> +static void unlink_all_urbs(struct usb2can *dev)
> +{
> + int i;
> +
> + usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> + usb_kill_anchored_urbs(&dev->tx_submitted);
> + atomic_set(&dev->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + dev->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +/* Close USB device */
> +static int usb2can_close(struct net_device *netdev)
> +{
> + struct usb2can *dev = netdev_priv(netdev);
> + int err = 0;
> +
> + /* Send CLOSE command to CAN controller */
> + err = usb2can_cmd_close(dev);
> + if (err)
> + dev_warn(netdev->dev.parent, "couldn't stop device");
> +
> + dev->can.state = CAN_STATE_STOPPED;
> +
> + /* Stop polling */
> + unlink_all_urbs(dev);
> +
> + netif_stop_queue(netdev);
> +
> + close_candev(netdev);
> +
> + return err;
> +}
> +
> +static const struct net_device_ops usb2can_netdev_ops = {
> + .ndo_open = usb2can_open,
> + .ndo_stop = usb2can_close,
> + .ndo_start_xmit = usb2can_start_xmit,
> +};
> +
> +static struct can_bittiming_const usb2can_bittiming_const = {
> + .name = "usb2can",
> + .tseg1_min = USB2CAN_TSEG1_MIN,
> + .tseg1_max = USB2CAN_TSEG1_MAX,
> + .tseg2_min = USB2CAN_TSEG2_MIN,
> + .tseg2_max = USB2CAN_TSEG2_MAX,
> + .sjw_max = USB2CAN_SJW_MAX,
> + .brp_min = USB2CAN_BRP_MIN,
> + .brp_max = USB2CAN_BRP_MAX,
> + .brp_inc = USB2CAN_BRP_INC,
> +};
> +
> +/* Probe USB device
> + *
> + * Check device and firmware.
> + * Set supported modes and bittiming constants.
> + * Allocate some memory.
> + */
> +static int usb2can_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct net_device *netdev;
> + struct usb2can *dev;
> + int i, err = -ENOMEM;
> + u32 version;
> + char buf[18];
> + struct usb_device *usbdev = interface_to_usbdev(intf);
> +
> + /* product id looks strange, better we also check iProdukt string */
> + if (usb_string(usbdev, usbdev->descriptor.iProduct, buf,
> + sizeof(buf)) > 0 && strcmp(buf, "USB2CAN converter")) {
> + dev_info(&usbdev->dev, "ignoring: not an USB2CAN converter\n");
> + return -ENODEV;
> + }
> +
> + netdev = alloc_candev(sizeof(struct usb2can), MAX_TX_URBS);
> + if (!netdev) {
> + dev_err(&intf->dev, "Couldn't alloc candev\n");
> + return -ENOMEM;
> + }
> +
> + dev = netdev_priv(netdev);
> +
> + dev->udev = usbdev;
> + dev->netdev = netdev;
> +
> + dev->dar = 0;
> +
> + dev->can.state = CAN_STATE_STOPPED;
> + dev->can.clock.freq = USB2CAN_ABP_CLOCK;
> + dev->can.bittiming_const = &usb2can_bittiming_const;
> + dev->can.do_set_bittiming = usb2can_set_bittiming;
> + dev->can.do_set_mode = usb2can_set_mode;
> + dev->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY;
This reminds me: in listen-only mode, we should not start the tx-queue
on open. Other drivers do have this issue as well.
> + netdev->netdev_ops = &usb2can_netdev_ops;
> +
> + netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> + init_usb_anchor(&dev->rx_submitted);
> +
> + init_usb_anchor(&dev->tx_submitted);
> + atomic_set(&dev->active_tx_urbs, 0);
> +
> + for (i = 0; i < MAX_TX_URBS; i++)
> + dev->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> + dev->cmd_msg_buffer = kzalloc(sizeof(struct usb2can_cmd_msg),
> + GFP_KERNEL);
> + if (!dev->cmd_msg_buffer) {
> + dev_err(&intf->dev, "Couldn't alloc Tx buffer\n");
> + goto cleanup_candev;
> + }
> +
> + usb_set_intfdata(intf, dev);
> +
> + SET_NETDEV_DEV(netdev, &intf->dev);
> +
> + mutex_init(&dev->usb2can_cmd_lock);
> +
> + err = usb2can_cmd_version(dev, &version);
> + if (err) {
> + dev_err(netdev->dev.parent, "can't get firmware version\n");
> + goto cleanup_cmd_msg_buffer;
> + } else {
> + dev_info(netdev->dev.parent,
> + "firmware: %d.%d, hardware: %d.%d\n",
> + (u8)(version>>24), (u8)(version>>16),
> + (u8)(version>>8), (u8)version);
I would prefer "& 0xff" vs. cast here.
> + }
> +
> + err = register_candev(netdev);
> + if (err) {
> + dev_err(netdev->dev.parent,
> + "couldn't register CAN device: %d\n", err);
> + goto cleanup_cmd_msg_buffer;
> + }
> +
> + if (device_create_file(&intf->dev, &dev_attr_firmware))
> + dev_err(&intf->dev,
> + "Couldn't create device file for firmware\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_hardware))
> + dev_err(&intf->dev,
> + "Couldn't create device file for hardware\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_state))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_state\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_rx_frames))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_rx_frames\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_rx_bytes))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_rx_bytes\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_tx_frames))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_tx_frames\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_tx_bytes))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_tx_bytes\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_overruns))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_overruns\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_warnings))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_warnings\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_bus_off_counter))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_bus_off_counter\n");
> +
> + if (device_create_file(&intf->dev, &dev_attr_can_reset_statistics))
> + dev_err(&intf->dev,
> + "Couldn't create device file for can_reset_statistics\n");
> +
> + if (device_create_file(&intf->dev,
> + &dev_attr_can_disable_automatic_retransmission))
> + dev_err(&intf->dev,
> + "Couldn't create device file for
> can_disable_automatic_retransmission\n");
> +
> + /* let the user know what node this device is now attached to */
> + dev_info(netdev->dev.parent, "device registered as %s\n",
> netdev->name);
> + return 0;
> +
> +cleanup_cmd_msg_buffer:
> + kfree(dev->cmd_msg_buffer);
> +
> +cleanup_candev:
> + free_candev(netdev);
> +
> + return err;
> +
> +}
> +
> +/* Called by the usb core when driver is unloaded or device is removed */
> +static void usb2can_disconnect(struct usb_interface *intf)
> +{
> + struct usb2can *dev = usb_get_intfdata(intf);
> +
> + device_remove_file(&intf->dev, &dev_attr_firmware);
> + device_remove_file(&intf->dev, &dev_attr_hardware);
> + device_remove_file(&intf->dev, &dev_attr_can_state);
> + device_remove_file(&intf->dev, &dev_attr_can_rx_frames);
> + device_remove_file(&intf->dev, &dev_attr_can_rx_bytes);
> + device_remove_file(&intf->dev, &dev_attr_can_tx_frames);
> + device_remove_file(&intf->dev, &dev_attr_can_tx_bytes);
> + device_remove_file(&intf->dev, &dev_attr_can_overruns);
> + device_remove_file(&intf->dev, &dev_attr_can_warnings);
> + device_remove_file(&intf->dev, &dev_attr_can_bus_off_counter);
> + device_remove_file(&intf->dev, &dev_attr_can_reset_statistics);
> + device_remove_file(&intf->dev,
> + &dev_attr_can_disable_automatic_retransmission);
The driver does collect similar statistics and therefore I do not see a
need for these sysfs files... apart from printing the firmware version
when the device is probed. BTW: what is "show_hardware" good for?
Thanks for your contribution.
Wolfgang.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-02 13:35 ` Wolfgang Grandegger
@ 2012-12-03 0:43 ` krumboeck
2012-12-03 7:26 ` Wolfgang Grandegger
2012-12-03 8:15 ` Wolfgang Grandegger
0 siblings, 2 replies; 10+ messages in thread
From: krumboeck @ 2012-12-03 0:43 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linux-can, info, gediminas
Am 2012-12-02 14:35, schrieb Wolfgang Grandegger:
> Hi Bernd,
>
> nice to see this driver being pushed mainline. As Oliver already pointed
> out, there are a few general naming and coding style issues:
>
> - The preferred multi line comment styles is:
>
> /*
> * A Comment
> */
The Script checkpatch.pl didn't like this comment style. I'll change it again.
>
> - The patch does not yet apply to David Millers "net-next" GIT tree.
> There are problems with Kconfig and Makefile.
I assume this is the right command and repository url:
git clone "http://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git"
>
> Could you please show the output of "candump -e any,0:0,#FFFFFFFF" when
> the device goes 1) error passive and 2) bus-off. You could provoke
> these states by 1) sending without connection to the bus or 2)
> short-circuiting CAN high and low.
bek@debian:~/software/socketcan/trunk/can-utils$ ./candump -e any,0:0,#FFFFFFFF
can0 20000088 [8] 00 00 04 00 00 00 00 01 ERRORFRAME
protocol-violation{{bit-stuffing-error}{}}
bus-error
error-counter-tx-rx{{0}{1}}
can0 2000008C [8] 00 04 00 00 00 00 00 63 ERRORFRAME
controller-problem{rx-error-warning}
protocol-violation{{}{}}
bus-error
error-counter-tx-rx{{0}{99}}
can0 2000008C [8] 00 10 00 00 00 00 00 7F ERRORFRAME
controller-problem{rx-error-passive}
protocol-violation{{}{}}
bus-error
error-counter-tx-rx{{0}{127}}
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
can0 500 [4] 00 01 02 03
In my tests I was not able to provoke "bus-off".
One of the tests from Gerd provoked "bus-off":
can0 12345678 [8] 55 55 55 55 33 33 33 33
can0 20000088 [8] 00 00 90 00 00 00 10 00 ERRORFRAME
can0 2000008C [8] 00 08 00 00 00 00 68 00 ERRORFRAME
can0 2000008C [8] 00 20 00 00 00 00 88 00 ERRORFRAME
can0 20000040 [8] 00 00 00 00 00 00 F8 00 ERRORFRAME
> Another question: is it possible to switch off bus-error reporting?
No. At least I didn't found anything in the firmware.
>> +
>> +/* Send data to device */
>> +static netdev_tx_t usb2can_start_xmit(struct sk_buff *skb,
>> + struct net_device *netdev)
>> +{
>> + struct usb2can *dev = netdev_priv(netdev);
>> + struct net_device_stats *stats = &netdev->stats;
>> + struct can_frame *cf = (struct can_frame *)skb->data;
>
> Is the cast needed?
Yes.
>
> The driver does collect similar statistics and therefore I do not see a
> need for these sysfs files... apart from printing the firmware version
> when the device is probed. BTW: what is "show_hardware" good for?
>
I don't exactly know, but I assume it is the hardware revision.
best regards,
Bernd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-03 0:43 ` krumboeck
@ 2012-12-03 7:26 ` Wolfgang Grandegger
[not found] ` <50BCF810.6060108@universalnet.at>
2012-12-03 20:41 ` krumboeck
2012-12-03 8:15 ` Wolfgang Grandegger
1 sibling, 2 replies; 10+ messages in thread
From: Wolfgang Grandegger @ 2012-12-03 7:26 UTC (permalink / raw)
To: krumboeck@universalnet.at; +Cc: linux-can, info, gediminas
On 12/03/2012 01:43 AM, krumboeck@universalnet.at wrote:
> Am 2012-12-02 14:35, schrieb Wolfgang Grandegger:
>> Hi Bernd,
>>
>> nice to see this driver being pushed mainline. As Oliver already pointed
>> out, there are a few general naming and coding style issues:
>>
>> - The preferred multi line comment styles is:
>>
>> /*
>> * A Comment
>> */
>
> The Script checkpatch.pl didn't like this comment style. I'll change it
> again.
I'm confused. Could you please show the comment and the checkpatch.pl
message. I hope it does not argue against:
http://lxr.linux.no/#linux+v3.6.8/Documentation/CodingStyle#L446
>>
>> - The patch does not yet apply to David Millers "net-next" GIT tree.
>> There are problems with Kconfig and Makefile.
>
> I assume this is the right command and repository url:
> git clone
> "http://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git"
>
>>
>> Could you please show the output of "candump -e any,0:0,#FFFFFFFF" when
>> the device goes 1) error passive and 2) bus-off. You could provoke
>> these states by 1) sending without connection to the bus or 2)
>> short-circuiting CAN high and low.
>
> bek@debian:~/software/socketcan/trunk/can-utils$ ./candump -e
> any,0:0,#FFFFFFFF
> can0 20000088 [8] 00 00 04 00 00 00 00 01 ERRORFRAME
> protocol-violation{{bit-stuffing-error}{}}
> bus-error
> error-counter-tx-rx{{0}{1}}
> can0 2000008C [8] 00 04 00 00 00 00 00 63 ERRORFRAME
> controller-problem{rx-error-warning}
> protocol-violation{{}{}}
> bus-error
> error-counter-tx-rx{{0}{99}}
> can0 2000008C [8] 00 10 00 00 00 00 00 7F ERRORFRAME
> controller-problem{rx-error-passive}
> protocol-violation{{}{}}
> bus-error
> error-counter-tx-rx{{0}{127}}
The last two messages just seem to be "controller-problem". You should
drop the "bus-error" and "protocol-violation" bits then.
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
> can0 500 [4] 00 01 02 03
These messages should up after reconnecting the cable?
> In my tests I was not able to provoke "bus-off".
Even by short-circuiting CAN high and low while sending messages out to
the bus? This should always work.
> One of the tests from Gerd provoked "bus-off":
> can0 12345678 [8] 55 55 55 55 33 33 33 33
> can0 20000088 [8] 00 00 90 00 00 00 10 00 ERRORFRAME
BUSERROR, PROT, PROT_BIT1, PROT_TX
> can0 2000008C [8] 00 08 00 00 00 00 68 00 ERRORFRAME
BUSERROR, PROT, CTRL, TX_WARNING
> can0 2000008C [8] 00 20 00 00 00 00 88 00 ERRORFRAME
BUSERROR, PROT, CTRL, TX_PASSIVE
> can0 20000040 [8] 00 00 00 00 00 00 F8 00 ERRORFRAME
BUS-OFF
>> Another question: is it possible to switch off bus-error reporting?
>
> No. At least I didn't found anything in the firmware.
>
>
>>> +
>>> +/* Send data to device */
>>> +static netdev_tx_t usb2can_start_xmit(struct sk_buff *skb,
>>> + struct net_device *netdev)
>>> +{
>>> + struct usb2can *dev = netdev_priv(netdev);
>>> + struct net_device_stats *stats = &netdev->stats;
>>> + struct can_frame *cf = (struct can_frame *)skb->data;
>>
>> Is the cast needed?
>
> Yes.
>
>>
>> The driver does collect similar statistics and therefore I do not see a
>> need for these sysfs files... apart from printing the firmware version
>> when the device is probed. BTW: what is "show_hardware" good for?
>>
>
> I don't exactly know, but I assume it is the hardware revision.
OK, I think it shouöd be fine if it's printed when the device is probed.
Wolfgang.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-03 0:43 ` krumboeck
2012-12-03 7:26 ` Wolfgang Grandegger
@ 2012-12-03 8:15 ` Wolfgang Grandegger
1 sibling, 0 replies; 10+ messages in thread
From: Wolfgang Grandegger @ 2012-12-03 8:15 UTC (permalink / raw)
To: krumboeck@universalnet.at; +Cc: linux-can, info, gediminas
On 12/03/2012 01:43 AM, krumboeck@universalnet.at wrote:
> Am 2012-12-02 14:35, schrieb Wolfgang Grandegger:
>> Hi Bernd,
>>
>> nice to see this driver being pushed mainline. As Oliver already pointed
>> out, there are a few general naming and coding style issues:
>>
>> - The preferred multi line comment styles is:
>>
>> /*
>> * A Comment
>> */
>
> The Script checkpatch.pl didn't like this comment style. I'll change it
> again.
>
>>
>> - The patch does not yet apply to David Millers "net-next" GIT tree.
>> There are problems with Kconfig and Makefile.
>
> I assume this is the right command and repository url:
> git clone
> "http://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git"
Yes. You may want to add
"--reference=<a-recent-local-linux-kernel-git-tree>" to save bandwidth
and space.
Wolfgang.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
[not found] ` <50BCF810.6060108@universalnet.at>
@ 2012-12-03 20:12 ` Wolfgang Grandegger
0 siblings, 0 replies; 10+ messages in thread
From: Wolfgang Grandegger @ 2012-12-03 20:12 UTC (permalink / raw)
To: krumboeck@universalnet.at; +Cc: Linux-CAN
On 12/03/2012 08:05 PM, krumboeck@universalnet.at wrote:
> Hi Wolfgang!
>
>
>>> The Script checkpatch.pl didn't like this comment style. I'll change it
>>> again.
>>
>> I'm confused. Could you please show the comment and the checkpatch.pl
>> message. I hope it does not argue against:
>>
>> http://lxr.linux.no/#linux+v3.6.8/Documentation/CodingStyle#L446
>
> ...
> /*
> * Send command to device and receive result.
> * Command was successful when opt1 = 0.
> */
> static int usb_8dev_send_cmd(struct usb_8dev *dev, struct
> usb_8dev_cmd_msg *out,
> struct usb_8dev_cmd_msg *in)
> {
> ...
>
> WARNING: networking block comments don't use an empty /* line, use /*
> Comment...
> #248: FILE: drivers/net/can/usb/usb_8dev.c:204:
> +
> +/*
>
> ...
> /*
> * Set network device mode
> *
> * Maybe we should leave this function empty, because the device
> * set mode variable with open command.
> */
> static int usb_8dev_set_mode(struct net_device *netdev, enum can_mode mode)
> {
> struct usb_8dev *dev = netdev_priv(netdev);
> ...
>
> WARNING: networking block comments don't use an empty /* line, use /*
> Comment...
> #413: FILE: drivers/net/can/usb/usb_8dev.c:369:
> +
> +/*
>
Wow, I obviously missed the following commit:
commit c4ff1b5f8bf09d77d7329cbff224f0237646c90e
Author: Joe Perches <joe@perches.com>
Date: Thu Oct 4 17:13:36 2012 -0700
CodingStyle: add networking specific block comment style
The block comment style in net/ and drivers/net is non-standard.
Document it.
Signed-off-by: Joe Perches <joe@perches.com>
Cc: "Allan, Bruce W" <bruce.w.allan@intel.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index cb9258b..495e5ba 100644
index cb9258b..495e5ba 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -454,6 +454,16 @@ The preferred style for long (multi-line) comments is:
* with beginning and ending almost-blank lines.
*/
+For files in net/ and drivers/net/ the preferred style for long (multi-line)
+comments is a little different.
+
+ /* The preferred comment style for files in net/ and drivers/net
+ * looks like this.
+ *
+ * It is nearly the same as the generally preferred comment style,
+ * but there is no initial almost-blank line.
+ */
+
It's also important to comment data, whether they are basic types or derived
types. To this end, use just one data declaration per line (no commas for
multiple data declarations). This leaves you room for a small comment on each
Well, no comment. Sorry for the noise then. Feel free to choose what
you like (from my point of view). A *common* style seems not really to be
important.
Wolfgang.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
2012-12-03 7:26 ` Wolfgang Grandegger
[not found] ` <50BCF810.6060108@universalnet.at>
@ 2012-12-03 20:41 ` krumboeck
1 sibling, 0 replies; 10+ messages in thread
From: krumboeck @ 2012-12-03 20:41 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: linux-usb, linux-can, info, gediminas
Hi Wolfgang!
>> The Script checkpatch.pl didn't like this comment style. I'll change it
>> again.
>
> I'm confused. Could you please show the comment and the checkpatch.pl
> message. I hope it does not argue against:
>
> http://lxr.linux.no/#linux+v3.6.8/Documentation/CodingStyle#L446
...
/*
* Send command to device and receive result.
* Command was successful when opt1 = 0.
*/
static int usb_8dev_send_cmd(struct usb_8dev *dev, struct usb_8dev_cmd_msg *out,
struct usb_8dev_cmd_msg *in)
{
...
WARNING: networking block comments don't use an empty /* line, use /* Comment...
#248: FILE: drivers/net/can/usb/usb_8dev.c:204:
+
+/*
...
/*
* Set network device mode
*
* Maybe we should leave this function empty, because the device
* set mode variable with open command.
*/
static int usb_8dev_set_mode(struct net_device *netdev, enum can_mode mode)
{
struct usb_8dev *dev = netdev_priv(netdev);
...
WARNING: networking block comments don't use an empty /* line, use /* Comment...
#413: FILE: drivers/net/can/usb/usb_8dev.c:369:
+
+/*
regards,
Bernd
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] usb2can: Add support for USB2CAN interface from 8 devices
@ 2012-12-13 7:44 "Bernd Krumböck"
0 siblings, 0 replies; 10+ messages in thread
From: "Bernd Krumböck" @ 2012-12-13 7:44 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: krumboeck@universalnet.at, linux-can, info, gediminas
Hi!
> On 12/03/2012 01:43 AM, krumboeck@universalnet.at wrote:
>> Am 2012-12-02 14:35, schrieb Wolfgang Grandegger:
...
>>>
>>> Could you please show the output of "candump -e any,0:0,#FFFFFFFF" when
>>> the device goes 1) error passive and 2) bus-off. You could provoke
>>> these states by 1) sending without connection to the bus or 2)
>>> short-circuiting CAN high and low.
...
>>
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>> can0 500 [4] 00 01 02 03
>
> These messages should up after reconnecting the cable?
>
>> In my tests I was not able to provoke "bus-off".
>
>
> Even by short-circuiting CAN high and low while sending messages out to
> the bus? This should always work.
>
This was my fault, because I forgot to connect a power supply and the
controller circuit use galvanic isolation.
regards,
Bernd
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-13 7:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-02 9:25 [PATCH] usb2can: Add support for USB2CAN interface from 8 devices krumboeck
2012-12-02 10:36 ` Oliver Hartkopp
2012-12-02 11:45 ` Kurt Van Dijck
2012-12-02 13:35 ` Wolfgang Grandegger
2012-12-03 0:43 ` krumboeck
2012-12-03 7:26 ` Wolfgang Grandegger
[not found] ` <50BCF810.6060108@universalnet.at>
2012-12-03 20:12 ` Wolfgang Grandegger
2012-12-03 20:41 ` krumboeck
2012-12-03 8:15 ` Wolfgang Grandegger
-- strict thread matches above, loose matches on Subject: below --
2012-12-13 7:44 "Bernd Krumböck"
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).