Matthias Fuchs wrote:
> This patch adds support for esd's CAN/USB2 high speed USB 2.0 
> CAN interface module. Multiple CAN interfaces per module are supported.
> 
> This driver has been tested on x86 and PowerPC.
> 
> Q: I am not absolutely sure if some locking is required inside
> the driver code. Please have a look.

Sorry I cannot say anything to this, you might want to ask the linux-usb
people about this.

However inline you find some random blurps about the driver.

cheers, Marc

> Signed-off-by: Matthias Fuchs <[email protected]>

> Index: kernel/2.6/Makefile
> ===================================================================
> --- kernel/2.6/Makefile       (revision 1095)
> +++ kernel/2.6/Makefile       (working copy)
> @@ -24,6 +24,7 @@
>  export CONFIG_CAN_EMS_104M=m
>  export CONFIG_CAN_ESD_PCI=m
>  export CONFIG_CAN_ESD_331=m
> +export CONFIG_CAN_ESD_USB2=m
>  export CONFIG_CAN_PIPCAN=m
>  export CONFIG_CAN_SOFTING=m
>  export CONFIG_CAN_SOFTING_CS=m
> Index: kernel/2.6/drivers/net/can/usb/Kconfig
> ===================================================================
> --- kernel/2.6/drivers/net/can/usb/Kconfig    (revision 1095)
> +++ kernel/2.6/drivers/net/can/usb/Kconfig    (working copy)
> @@ -7,4 +7,10 @@
>         This driver is for the one channel CPC-USB/ARM7 CAN/USB interface
>         from from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
>  
> +config CAN_ESD_USB2
> +     tristate "ESD USB/2 CAN/USB interface"
> +     ---help---
> +       This driver supports the CAN-USB/2 interface
> +       from esd electronic system design gmbh (http://www.esd.eu).
> +
>  endmenu
> Index: kernel/2.6/drivers/net/can/usb/esd_usb2.c
> ===================================================================
> --- kernel/2.6/drivers/net/can/usb/esd_usb2.c (revision 0)
> +++ kernel/2.6/drivers/net/can/usb/esd_usb2.c (revision 0)
> @@ -0,0 +1,1113 @@
> +/*
> + * CAN driver for esd CAN-USB/2
> + *
> + * Copyright (C) 2009 Matthias Fuchs <[email protected]>, esd gmbh
> + *
> + * 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.
> + */
> +#undef DEBUG

is this intentional?

> +#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 <socketcan/can.h>
> +#include <socketcan/can/dev.h>
> +#include <socketcan/can/error.h>
> +
> +MODULE_AUTHOR("Matthias Fuchs <[email protected]>");
> +MODULE_DESCRIPTION("CAN driver for esd CAN-USB/2 interfaces");
> +MODULE_LICENSE("GPL v2");
> +
> +/* Define these values to match your devices */
> +#define USB_ESDGMBH_VENDOR_ID        0x0ab4
> +#define USB_CANUSB2_PRODUCT_ID       0x0010
> +
> +#define ESD_USB2_CAN_CLOCK   60000000
> +#define ESD_USB2_MAX_NETS    2
> +
> +/* USB2 commands */
> +#define CMD_VERSION  1 /* also used for VERSION_REPLY */
> +#define CMD_CAN_RX   2 /* device to host only */
> +#define CMD_CAN_TX   3 /* also used for TX_DONE */
> +#define CMD_SETBAUD  4 /* also used for SETBAUD_REPLY */
> +#define CMD_TS               5 /* also used for TS_REPLY */
> +#define CMD_IDADD    6 /* also used for IDADD_REPLY */
> +
> +/* esd CAN message flags - dlc field */
> +#define ESD_RTR              0x10
> +
> +/* esd CAN message flags - id field */
> +#define ESD_EXTID    0x20000000
> +#define ESD_EVENT    0x40000000
> +#define ESD_IDMASK   0x1fffffff
> +
> +/* esd CAN event ids used by this driver */
> +#define ESD_EV_CAN_ERROR_EXT 2
> +
> +/* baudrate message flags */
> +#define ESD_UBR              0x80000000
> +#define ESD_LOM              0x40000000
> +#define ESD_NO_BAUDRATE 0x7fffffff
                          ^
use tab here as-well

> +
> +/* esd IDADD message */
> +#define ESD_ID_ENABLE        0x80
> +
> +struct header_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      rsvd[2];
> +};
> +
> +struct version_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      rsvd;
> +     u8      flags;
> +     u32     drv_version;
> +};
> +
> +struct version_reply_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      nets;
> +     u8      features;
> +     u32     version;
> +     u8      name[16];
> +     u32     rsvd;
> +     u32     ts;
> +};
> +
> +struct rx_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      dlc;
> +     u32     ts;
> +     u32     id; /* upper 3 bits contain flags */
> +     u8      data[8];
> +};
> +
> +struct tx_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      dlc;
> +     u32     hnd;
> +     u32     id; /* upper 3 bits contain flags */
> +     u8      data[8];
> +};
> +
> +struct tx_done_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      status;
> +     u32     hnd;
> +     u32     ts;
> +};
> +
> +struct id_filter_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      option;
> +     u32     mask[65];
> +};
> +
> +struct id_filter_reply_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      option;
> +     u16     added;
> +     u16     removed;
> +     u16     error;
> +     u16     active;
> +};
> +
> +struct set_baudrate_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      rsvd;
> +     u32     baud;
> +};
> +
> +struct set_baudrate_reply_msg {
> +     u8      len;
> +     u8      cmd;
> +     u8      net;
> +     u8      rsvd;
> +     u32     baud;
> +     u32     ts;
> +};

if you use cpu_to_le32 and vice versa you might want to define the vars
__le32

compile your driver with "make .... C=2" to see if sparse finds some
problems.

> +/* Main message type used between library and application */
> +struct __attribute__ ((packed)) esd_usb2_msg {
> +     union {
> +             struct header_msg hdr;
> +             struct version_msg version;
> +             struct version_reply_msg version_reply;
> +             struct rx_msg rx;
> +             struct tx_msg tx;
> +             struct tx_done_msg txdone;
> +             struct set_baudrate_msg setbaud;
> +             struct set_baudrate_reply_msg setbaud_reply;
> +             struct id_filter_msg filter;
> +             struct id_filter_reply_msg filter_reply;
> +     } msg;
> +};
> +
> +static struct usb_device_id esd_usb2_table[] = {
> +     {USB_DEVICE(USB_ESDGMBH_VENDOR_ID, USB_CANUSB2_PRODUCT_ID)},
> +     {}
> +};
> +
> +MODULE_DEVICE_TABLE(usb, esd_usb2_table);
> +
> +#define RX_BUFFER_SIZE       1024
> +#define MAX_RX_URBS  4
> +#define MAX_TX_URBS  10
> +
> +struct esd_usb2_net;
> +
> +struct esd_tx_urb_context {
> +     struct esd_usb2_net *net;
> +     u32 echo_index;
> +};
> +
> +struct esd_usb2 {
> +     struct usb_device *udev;
> +     struct esd_usb2_net *nets[ESD_USB2_MAX_NETS];
> +
> +     struct usb_anchor rx_submitted;
> +
> +     int no_nets;
> +        u32 version;
   ^^^^^^^^
use tab here, too

> +     int rxinitdone;
> +};
> +
> +struct esd_usb2_net {
> +     struct can_priv can; /* must be the first member */
> +
> +     atomic_t active_tx_urbs;
> +     struct usb_anchor tx_submitted;
> +     struct esd_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> +     int open_time;
> +     struct esd_usb2 *usb2;
> +     struct net_device *netdev;
> +     int index;
> +     u8 old_state;
> +};

you might want to add tabs between the type and the variable in these
struct in order to align them.

> +
> +/* SJA1000 ECC register (emulated by usb2 firmware) */
> +#define SJA1000_ECC_SEG   0x1F
> +#define SJA1000_ECC_DIR   0x20
> +#define SJA1000_ECC_ERR   0x06
> +#define SJA1000_ECC_BIT   0x00
> +#define SJA1000_ECC_FORM  0x40
> +#define SJA1000_ECC_STUFF 0x80
> +#define SJA1000_ECC_MASK  0xc0
> +
> +static void esd_usb2_rx_event(struct esd_usb2_net *net,
> +                           struct esd_usb2_msg *msg)
> +{
> +     struct can_frame *cf;
> +     struct sk_buff *skb;
> +     struct net_device_stats *stats = &net->netdev->stats;
> +     u32 id;
> +
> +     id = le32_to_cpu(msg->msg.rx.id) & ESD_IDMASK;
> +
> +     dev_dbg(net->usb2->udev->dev.parent,
> +             "event: id=%08x, len=%d, data=%02x %02x %02x %02x\n",
> +             id, get_can_dlc(msg->msg.rx.dlc),
> +             msg->msg.rx.data[0], msg->msg.rx.data[1],
> +             msg->msg.rx.data[2], msg->msg.rx.data[3]);
> +
> +     switch (id) {
> +     case ESD_EV_CAN_ERROR_EXT:
> +     {
> +             u8 state = msg->msg.rx.data[0];
> +             u8 ecc = msg->msg.rx.data[1];
> +             u8 txerr = msg->msg.rx.data[2];
> +             u8 rxerr = msg->msg.rx.data[3];
> +
> +             skb = alloc_can_err_skb(net->netdev, &cf);
> +             if (skb == NULL)
> +                     return;
> +
> +             if (state != net->old_state) {
> +                     net->old_state = state;
> +
> +                     switch (state & 0xc0) {
> +                     case 0xc0:
> +                             net->can.state = CAN_STATE_BUS_OFF;
> +                             cf->can_id |= CAN_ERR_BUSOFF;
> +                             can_bus_off(net->netdev);
> +                             break;
> +                     case 0x40:
> +                             net->can.state = CAN_STATE_ERROR_WARNING;
> +                             net->can.can_stats.error_warning++;
> +                             break;
> +                     case 0x80:
> +                             net->can.state = CAN_STATE_ERROR_PASSIVE;
> +                             net->can.can_stats.error_passive++;
> +                             break;
> +                     default:
> +                             net->can.state = CAN_STATE_ERROR_ACTIVE;
> +                             break;
> +                     }
> +             } else {
> +                     net->can.can_stats.bus_error++;
> +                     stats->rx_errors++;
> +
> +                     cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> +                     switch (ecc & SJA1000_ECC_MASK) {
> +                     case SJA1000_ECC_BIT:
> +                             cf->data[2] |= CAN_ERR_PROT_BIT;
> +                             break;
> +                     case SJA1000_ECC_FORM:
> +                             cf->data[2] |= CAN_ERR_PROT_FORM;
> +                             break;
> +                     case SJA1000_ECC_STUFF:
> +                             cf->data[2] |= CAN_ERR_PROT_STUFF;
> +                             break;
> +                     default:
> +                             cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> +                             cf->data[3] = ecc & SJA1000_ECC_SEG;
> +                             break;
> +                     }
> +
> +                     /* Error occured during transmission? */
> +                     if ((ecc & SJA1000_ECC_DIR) == 0)

we usually don't check against 0, better:

                        if (!(ecc & SJA1000_ECC_DIR))
> +                             cf->data[2] |= CAN_ERR_PROT_TX;
> +
> +                     if (net->can.state == CAN_STATE_ERROR_WARNING ||
> +                         net->can.state == CAN_STATE_ERROR_PASSIVE) {
> +                             cf->data[1] = (txerr > rxerr) ?
> +                                     CAN_ERR_CRTL_TX_PASSIVE :
> +                                     CAN_ERR_CRTL_RX_PASSIVE;
> +                     }
> +             }
> +
> +             netif_rx(skb);
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
> +             net->netdev->last_rx = jiffies;
> +#endif
> +             stats->rx_packets++;
> +             stats->rx_bytes += cf->can_dlc;
> +
> +             break;
> +     }
> +
> +     default:
> +             break;
> +
> +     }
> +}
> +
> +static void esd_usb2_rx_can_msg(struct esd_usb2_net *net,
> +                             struct esd_usb2_msg *msg)
> +{
> +     struct can_frame *cf;
> +     struct sk_buff *skb;
> +     int i;
> +     struct net_device_stats *stats = &net->netdev->stats;
> +     u32 id;
> +
> +     if (!netif_device_present(net->netdev))
> +             return;
> +
> +     id = le32_to_cpu(msg->msg.rx.id);
> +
> +     if (id & ESD_EVENT) {
> +             esd_usb2_rx_event(net, msg);
> +     } else {
> +             skb = alloc_can_skb(net->netdev, &cf);
> +             if (skb == NULL)
> +                     return;
> +
> +             cf->can_id = id & ESD_IDMASK;
> +             cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
> +
> +             if (id & ESD_EXTID)
> +                     cf->can_id |= CAN_EFF_FLAG;
> +
> +             if (msg->msg.rx.dlc & ESD_RTR) {
> +                     cf->can_id |= CAN_RTR_FLAG;
> +             } else {
> +                     for (i = 0; i < cf->can_dlc; i++)
> +                             cf->data[i] = msg->msg.rx.data[i];
> +             }
> +
> +             netif_rx(skb);
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
> +             net->netdev->last_rx = jiffies;
> +#endif
> +             stats->rx_packets++;
> +             stats->rx_bytes += cf->can_dlc;
> +     }
> +
> +     return;
> +}
> +
> +static void esd_usb2_tx_done_msg(struct esd_usb2_net *net,
> +                              struct esd_usb2_msg *msg)
> +{
> +     struct net_device_stats *stats = &net->netdev->stats;
> +
> +     if (!netif_device_present(net->netdev))
> +             return;
> +
> +     if (msg->msg.txdone.status == 0) {
dito.
> +             stats->tx_packets++;
> +             stats->tx_bytes += msg->msg.txdone.hnd & 0xf;
> +     }
> +}
> +
> +static void esd_usb2_read_bulk_callback(struct urb *urb)
> +{
> +     struct esd_usb2 *dev = urb->context;
> +     int retval;
> +     int pos = 0;
> +     int i;
> +
> +     switch (urb->status) {
> +     case 0: /* success */
> +             break;
> +
> +     case -ENOENT:
> +     case -ESHUTDOWN:
> +             return;
> +
> +     default:
> +             dev_info(dev->udev->dev.parent,
> +                      "Rx URB aborted (%d)\n", urb->status);
> +             goto resubmit_urb;
> +     }
> +
> +     while (pos < urb->actual_length) {
> +             struct esd_usb2_msg *msg;
> +             u8 *ibuf = urb->transfer_buffer;
> +
> +             msg = (struct esd_usb2_msg *)&ibuf[pos];
> +
> +             dev_dbg(dev->udev->dev.parent, "%d/%d msg=%02x\n",
> +                     pos, urb->actual_length, msg->msg.hdr.cmd);
> +
> +             switch (msg->msg.hdr.cmd) {
> +             case CMD_CAN_RX:
> +                     esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
> +                     break;
> +
> +             case CMD_CAN_TX:
> +                     esd_usb2_tx_done_msg(dev->nets[msg->msg.txdone.net],
> +                                          msg);
> +                     break;
> +
> +             default:
> +                     break;
> +             }
> +
> +             pos += msg->msg.hdr.len << 2;
> +
> +             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,
> +                       esd_usb2_read_bulk_callback, dev);
> +
> +     retval = usb_submit_urb(urb, GFP_ATOMIC);
> +     if (retval == -ENODEV) {
> +             for (i = 0; i < dev->no_nets; i++)
> +                     netif_device_detach(dev->nets[i]->netdev);

what is one dev->nets hasn't be initialized correctly in
esd_usb2_probe_one_net()?

> +     } else if (retval) {
> +             dev_err(dev->udev->dev.parent,
> +                     "failed resubmitting read bulk urb: %d\n", retval);
> +     }
> +
> +     return;
> +}
> +
> +/*
> + * callback for bulk IN urb
> + */
> +static void esd_usb2_write_bulk_callback(struct urb *urb)
> +{
> +     struct esd_tx_urb_context *context = urb->context;
> +     struct esd_usb2_net *net;
> +     struct esd_usb2 *dev;
> +     struct net_device *netdev;
> +     size_t size = sizeof(struct esd_usb2_msg);
> +
> +     BUG_ON(!context);
> +
> +     net = context->net;
> +     netdev = net->netdev;
> +     dev = net->usb2;
> +
> +     /* free up our allocated buffer */
> +     usb_buffer_free(urb->dev, size,
> +                     urb->transfer_buffer, urb->transfer_dma);
> +
> +     atomic_dec(&net->active_tx_urbs);
> +
> +     if (!netif_device_present(netdev))
> +             return;
> +
> +     if (urb->status)
> +             dev_info(ND2D(netdev), "Tx URB aborted (%d)\n",
> +                      urb->status);
> +
> +     netdev->trans_start = jiffies;
> +
> +     can_get_echo_skb(netdev, context->echo_index);
> +
> +     /* Release context */
> +     context->echo_index = MAX_TX_URBS;
> +
> +     if (netif_queue_stopped(netdev))
this is not needed, wake_queue does the check anyway
> +             netif_wake_queue(netdev);
> +}
> +
> +#ifdef CONFIG_SYSFS
> +static ssize_t show_firmware(struct device *d,
> +                          struct device_attribute *attr, char *buf)
> +{
> +     struct usb_interface *intf = to_usb_interface(d);
> +     struct esd_usb2 *dev = usb_get_intfdata(intf);
> +
> +     return sprintf(buf, "%d.%d.%d\n",
> +                    (dev->version >> 12) & 0xf,
> +                    (dev->version >> 8) & 0xf,
> +                    dev->version & 0xff);
> +}
> +static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
> +
> +static ssize_t show_hardware(struct device *d,
> +                          struct device_attribute *attr, char *buf)
> +{
> +     struct usb_interface *intf = to_usb_interface(d);
> +     struct esd_usb2 *dev = usb_get_intfdata(intf);
> +
> +     return sprintf(buf, "%d.%d.%d\n",
> +                    (dev->version >> 28) & 0xf,
> +                    (dev->version >> 24) & 0xf,
> +                    (dev->version >> 16) & 0xff);
> +}
> +static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
> +
> +static ssize_t show_nets(struct device *d,
> +                      struct device_attribute *attr, char *buf)
> +{
> +     struct usb_interface *intf = to_usb_interface(d);
> +     struct esd_usb2 *dev = usb_get_intfdata(intf);
> +
> +     return sprintf(buf, "%d", dev->no_nets);
> +}
> +static DEVICE_ATTR(nets, S_IRUGO, show_nets, NULL);
> +#endif
> +
> +static int esd_usb2_send_msg(struct esd_usb2 *dev, struct esd_usb2_msg *msg)
> +{
> +     int actual_length;
> +
> +     return usb_bulk_msg(dev->udev,
> +                         usb_sndbulkpipe(dev->udev, 2),
> +                         msg,
> +                         msg->msg.hdr.len << 2,
> +                         &actual_length,
> +                         1000);
> +}
> +
> +static int esd_usb2_wait_msg(struct esd_usb2 *dev,
> +                          struct esd_usb2_msg *msg)
> +{
> +     int actual_length;
> +
> +     return usb_bulk_msg(dev->udev,
> +                         usb_rcvbulkpipe(dev->udev, 1),
> +                         msg,
> +                         sizeof(*msg),
> +                         &actual_length,
> +                         1000);
> +}
> +
> +static int esd_usb2_setup_rx_urbs(struct esd_usb2 *dev)
> +{
> +     int i, err;
> +
> +     if (dev->rxinitdone)
> +             return 0;
> +
> +     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(dev->udev->dev.parent,
> +                             "No memory left for URBs\n");
> +                     return -ENOMEM;

you don't free the mem alloated if i != 0
> +             }
> +
> +             buf = usb_buffer_alloc(dev->udev, RX_BUFFER_SIZE, GFP_KERNEL,
> +                                    &urb->transfer_dma);
> +             if (!buf) {
> +                     dev_err(dev->udev->dev.parent,
> +                             "No memory left for USB buffer\n");
> +                     usb_free_urb(urb);
> +                     return -ENOMEM;

dito
> +             }
> +
> +             usb_fill_bulk_urb(urb, dev->udev,
> +                               usb_rcvbulkpipe(dev->udev, 1),
> +                               buf, RX_BUFFER_SIZE,
> +                               esd_usb2_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) {
> +                     usb_unanchor_urb(urb);
> +                     usb_buffer_free(dev->udev, RX_BUFFER_SIZE, buf,
> +                                     urb->transfer_dma);

dito
> +                     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(dev->udev->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(dev->udev->dev.parent,
> +                      "rx performance may be slow\n");
> +     }
> +
> +     dev->rxinitdone = 1;
> +
> +     return 0;
> +}
> +
> +
> +/*
> + * Start interface
> + */
> +static int esd_usb2_start(struct esd_usb2_net *net)
> +{
> +     struct esd_usb2 *dev = net->usb2;
> +     struct net_device *netdev = net->netdev;
> +     int err, i;
> +     struct esd_usb2_msg msg;
> +
> +     /* enable all (!) IDs */
> +     msg.msg.hdr.cmd = CMD_IDADD;
> +     msg.msg.hdr.len = 1 + 64 + 1;
> +     msg.msg.filter.net = net->index;
> +     msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
> +     for (i = 0; i < 64; i++)
> +             msg.msg.filter.mask[i] = cpu_to_le32(0xffffffff);
> +     msg.msg.filter.mask[64] = cpu_to_le32(0x00000001);

you probably don't want to hardcode the 64 here

> +
> +     err = esd_usb2_send_msg(dev, &msg);
> +     if (err)
> +             goto failed;
> +
> +     esd_usb2_setup_rx_urbs(dev);
> +
> +     net->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +     return 0;
> +
> +failed:
> +     if (err == -ENODEV)
> +             netif_device_detach(netdev);
> +
> +     dev_warn(ND2D(netdev), "couldn't submit control: %d\n", err);
> +
> +     return err;
> +}
> +
> +static void unlink_all_urbs(struct esd_usb2 *dev)
> +{
> +     int i;
> +     struct esd_usb2_net *net;
> +
> +     usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> +     for (i = 0; i < dev->no_nets; i++) {
> +             net = dev->nets[i];
> +             if (net) {
> +                     usb_kill_anchored_urbs(&net->tx_submitted);
> +                     atomic_set(&net->active_tx_urbs, 0);
> +
> +                     for (i = 0; i < MAX_TX_URBS; i++)
> +                             net->tx_contexts[i].echo_index = MAX_TX_URBS;
> +             }
> +     }
> +}
> +
> +
> +static int esd_usb2_open(struct net_device *netdev)
> +{
> +     struct esd_usb2_net *net = netdev_priv(netdev);
> +     int err;
> +
> +     /* common open */
> +     err = open_candev(netdev);
> +     if (err)
> +             return err;
> +
> +     /* finally start device */
> +     err = esd_usb2_start(net);
> +     if (err) {
> +             if (err == -ENODEV)
> +                     netif_device_detach(netdev);

isn't this done in the esd_usb2_start() function, if not, it should be

> +
> +             dev_warn(ND2D(netdev), "couldn't start device: %d\n", err);
> +
> +             close_candev(netdev);
> +
> +             return err;
> +     }
> +
> +     net->open_time = jiffies;
> +
> +     netif_start_queue(netdev);
> +
> +     return 0;
> +}
> +
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
> +static int esd_usb2_start_xmit(struct sk_buff *skb, struct net_device 
> *netdev)
> +#else
> +static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
> +                                   struct net_device *netdev)
> +#endif
> +{
> +     struct esd_usb2_net *net = netdev_priv(netdev);
> +     struct esd_usb2 *dev = net->usb2;
> +     struct esd_tx_urb_context *context = NULL;
> +     struct net_device_stats *stats = &netdev->stats;
> +     struct can_frame *cf = (struct can_frame *)skb->data;
> +     struct esd_usb2_msg *msg;
> +     struct urb *urb;
> +     u8 *buf;
> +     int i, err;
> +     size_t size = sizeof(struct esd_usb2_msg);
> +
> +     /* 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(ND2D(netdev), "No memory left for URBs\n");
> +             goto nomem;
> +     }
> +
> +     buf = usb_buffer_alloc(dev->udev, size, GFP_ATOMIC, &urb->transfer_dma);
> +     if (!buf) {
> +             dev_err(ND2D(netdev), "No memory left for USB buffer\n");
> +             usb_free_urb(urb);
                ^^^^^^^^^^^^^^^^^

this should go into the error handling at the end of this function. you
may want to introduce another jump label for  it.

> +             goto nomem;
> +     }
> +
> +     msg = (struct esd_usb2_msg *)buf;

On a quick look you might be able to get rid of the "buf" variable and
use msg directly.

> +
> +     msg->msg.hdr.len = 3; /* minimal lenght */
> +     msg->msg.hdr.cmd = CMD_CAN_TX;
> +     msg->msg.tx.net = net->index;
> +     msg->msg.tx.dlc = cf->can_dlc;
> +     /* hnd must not be 0 */
> +     msg->msg.tx.hnd = 0x80000000 | cf->can_dlc;
> +     msg->msg.tx.id = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
> +
> +     if (cf->can_id & CAN_RTR_FLAG)
> +             msg->msg.tx.dlc |= ESD_RTR;
> +
> +     if (cf->can_id & CAN_EFF_FLAG)
> +             msg->msg.tx.id |= cpu_to_le32(ESD_EXTID);
> +
> +     for (i = 0; i < cf->can_dlc; i++)
> +             msg->msg.tx.data[i] = cf->data[i];
> +
> +     msg->msg.hdr.len += (cf->can_dlc + 3) >> 2;
> +
> +     for (i = 0; i < MAX_TX_URBS; i++) {
> +             if (net->tx_contexts[i].echo_index == MAX_TX_URBS) {
> +                     context = &net->tx_contexts[i];
> +                     break;
> +             }
> +     }
> +
> +     if (!context) {
> +             usb_buffer_free(dev->udev, size, buf, urb->transfer_dma);
> +             usb_free_urb(urb);
> +             dev_warn(ND2D(netdev), "couldn't find free context\n");
> +             return NETDEV_TX_BUSY;

you might use the common error handling path at the end of the function.

> +     }
> +
> +     context->net = net;
> +     context->echo_index = i;
> +
> +     usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
> +                       msg->msg.hdr.len << 2,
> +                       esd_usb2_write_bulk_callback, context);
> +
> +     urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +     usb_anchor_urb(urb, &net->tx_submitted);
> +
> +     can_put_echo_skb(skb, netdev, context->echo_index);
> +
> +     atomic_inc(&net->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_buffer_free(dev->udev, size, buf, urb->transfer_dma);
> +             dev_kfree_skb(skb);

use common error handling path

> +
> +             atomic_dec(&net->active_tx_urbs);
> +
> +             if (err == -ENODEV) {
> +                     netif_device_detach(netdev);
> +             } else {
> +                     dev_warn(ND2D(netdev), "failed tx_urb %d\n", err);
> +                     stats->tx_dropped++;
> +             }
> +     } else {
> +             netdev->trans_start = jiffies;
> +
> +             /* Slow down tx path */
> +             if (atomic_read(&net->active_tx_urbs) >= MAX_TX_URBS)
> +                     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:
> +     if (skb)
> +             dev_kfree_skb(skb);
> +
> +     stats->tx_dropped++;
> +
> +     return NETDEV_TX_OK;

this is probalby wrong for an error case
> +}
> +
> +static int esd_usb2_close(struct net_device *netdev)
> +{
> +     struct esd_usb2_net *net = netdev_priv(netdev);
> +     int i;
> +     struct esd_usb2_msg msg;
> +
> +     /* Disable all IDs */
> +     msg.msg.hdr.cmd = CMD_IDADD;
> +     msg.msg.hdr.len = 1 + 64 + 1;
> +     msg.msg.filter.net = net->index;
> +     msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
> +     for (i = 0; i < 64; i++)
> +             msg.msg.filter.mask[i] = 0;
> +     msg.msg.filter.mask[64] = 0;

you might want to use C99 initializer for msg.

> +     esd_usb2_send_msg(net->usb2, &msg);
> +
> +     /* set CAN controller to reset mode */
> +     msg.msg.setbaud.len = 2;
> +     msg.msg.setbaud.cmd = CMD_SETBAUD;
> +     msg.msg.setbaud.net = net->index;
> +     msg.msg.setbaud.rsvd = 0;
> +     msg.msg.setbaud.baud = cpu_to_le32(ESD_NO_BAUDRATE);
> +     esd_usb2_send_msg(net->usb2, &msg);
> +
> +     netif_stop_queue(netdev);
> +
> +     close_candev(netdev);
> +
> +     net->open_time = 0;
> +
> +     return 0;
> +}
> +
> +#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,28)
> +static const struct net_device_ops esd_usb2_netdev_ops = {
> +     .ndo_open = esd_usb2_open,
> +     .ndo_stop = esd_usb2_close,
> +     .ndo_start_xmit = esd_usb2_start_xmit,
> +};
> +#endif

you may want to allign these with tabs.

> +static struct can_bittiming_const esd_usb2_bittiming_const = {
> +     .name = "esd_usb2",
> +     .tseg1_min = 1,
> +     .tseg1_max = 16,
> +     .tseg2_min = 1,
> +     .tseg2_max = 8,
> +     .sjw_max = 4,
> +     .brp_min = 1,
> +     .brp_max = 1024,
> +     .brp_inc = 1,
> +};

dito

> +
> +static int esd_usb2_set_mode(struct net_device *netdev, enum can_mode mode)
> +{
> +     struct esd_usb2_net *net = netdev_priv(netdev);
> +
> +     if (!net->open_time)
> +             return -EINVAL;
> +
> +     switch (mode) {
> +     case CAN_MODE_START:
> +             if (netif_queue_stopped(netdev))
> +                     netif_wake_queue(netdev);
> +             break;
> +
> +     default:
> +             return -EOPNOTSUPP;
> +     }
> +
> +     return 0;
> +}
> +
> +static int esd_usb2_set_bittiming(struct net_device *netdev)
> +{
> +     struct esd_usb2_net *net = netdev_priv(netdev);
> +     struct can_bittiming *bt = &net->can.bittiming;
> +     struct esd_usb2_msg msg;
> +     u32 canbtr;
> +
> +     canbtr = ESD_UBR;
> +     canbtr |= (bt->brp - 1) & 0x3ff;
> +     canbtr |= ((bt->sjw - 1) & 0x3) << 14;
> +     canbtr |= ((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) << 16;
> +     canbtr |= ((bt->phase_seg2 - 1) & 0x7) << 20;

what about defining some constans for these shift values?

The other drivers prints the btr value written into the HW with dev_info.

> +
> +     if (net->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> +             canbtr |= 0x00800000;
> +
> +     msg.msg.setbaud.len = 2;
> +     msg.msg.setbaud.cmd = CMD_SETBAUD;
> +     msg.msg.setbaud.net = net->index;
> +     msg.msg.setbaud.rsvd = 0;
> +     msg.msg.setbaud.baud = cpu_to_le32(canbtr);

use c99 initializer for msg?
> +
> +     return esd_usb2_send_msg(net->usb2, &msg);
> +}
> +
> +static int esd_usb2_probe_one_net(struct usb_interface *intf, int index)
> +{
> +     struct esd_usb2 *dev = usb_get_intfdata(intf);
> +     struct net_device *netdev;
> +     struct esd_usb2_net *net;
> +     int err;
> +     int i;
> +
> +     netdev = alloc_candev(sizeof(struct esd_usb2_net), MAX_TX_URBS);
> +     if (!netdev) {
> +             dev_err(&intf->dev, "couldn't alloc candev\n");
> +             return -ENOMEM;
> +     }
> +
> +     net = netdev_priv(netdev);
> +
> +     init_usb_anchor(&net->tx_submitted);
> +     atomic_set(&net->active_tx_urbs, 0);
> +
> +     for (i = 0; i < MAX_TX_URBS; i++)
> +             net->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> +     net->usb2 = dev;
> +     net->netdev = netdev;
> +     net->index = index;
> +
> +     net->can.state = CAN_STATE_STOPPED;
> +     net->can.clock.freq = ESD_USB2_CAN_CLOCK;
> +     net->can.bittiming_const = &esd_usb2_bittiming_const;
> +     net->can.do_set_bittiming = esd_usb2_set_bittiming;
> +     net->can.do_set_mode = esd_usb2_set_mode;
> +
> +     netdev->flags |= IFF_ECHO; /* we support local echo */
> +
> +#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,28)
> +     netdev->netdev_ops = &esd_usb2_netdev_ops;
> +#else
> +     netdev->open = esd_usb2_open;
> +     netdev->stop = esd_usb2_close;
> +     netdev->hard_start_xmit = esd_usb2_start_xmit;
> +#endif
> +
> +     netdev->flags |= IFF_ECHO; /* we support local echo */

one time shoud be enough

> +
> +     SET_NETDEV_DEV(netdev, &intf->dev);
> +
> +     err = register_candev(netdev);
> +     if (err) {
> +             dev_err(netdev->dev.parent,
> +                     "couldn't register CAN device: %d\n", err);
> +             free_candev(netdev);
> +             return -ENOMEM;
> +     }
> +
> +     dev->nets[index] = net;
> +     return 0;
> +}
> +
> +/*
> + * probe function for new USB2 devices
> + *
> + * check version information and number of available
> + * CAN interfaces
> + */
> +static int esd_usb2_probe(struct usb_interface *intf,
> +                      const struct usb_device_id *id)
> +{
> +     struct esd_usb2 *dev;
> +     struct esd_usb2_msg msg;
> +     int i, err = -ENOMEM;
> +
> +     dev = kzalloc(sizeof(struct esd_usb2), GFP_KERNEL);
> +     if (!dev)
> +             return -ENOMEM;
> +
> +     dev->udev = interface_to_usbdev(intf);
> +
> +     init_usb_anchor(&dev->rx_submitted);
> +
> +     usb_set_intfdata(intf, dev);
> +
> +     /* query number of CAN interfaces (nets) */
> +     msg.msg.hdr.cmd = CMD_VERSION;
> +     msg.msg.hdr.len = 2;
> +     msg.msg.version.rsvd = 0;
> +     msg.msg.version.flags = 0;
> +     msg.msg.version.drv_version = 0;

consider using C99 initializer
> +
> +     if (esd_usb2_send_msg(dev, &msg) < 0) {
> +             dev_err(&intf->dev, "sending version message failed\n");
> +             goto free_dev;
> +     }
> +
> +     if (esd_usb2_wait_msg(dev, &msg) < 0) {
> +             dev_err(&intf->dev, "no version message answer\n");
> +             goto free_dev;
> +     }
> +
> +     dev->no_nets = (int)msg.msg.version_reply.nets;
> +     dev->version = le32_to_cpu(msg.msg.version_reply.version);
> +
> +#ifdef CONFIG_SYSFS
> +     device_create_file(&intf->dev, &dev_attr_firmware);
> +     device_create_file(&intf->dev, &dev_attr_hardware);
> +     device_create_file(&intf->dev, &dev_attr_nets);
> +#endif
> +
> +     /* do per device probing */
> +     for (i = 0; i < dev->no_nets; i++)
> +             esd_usb2_probe_one_net(intf, i);
> +
> +     return 0;
> +
> +free_dev:
> +     kfree(dev);
> +     return err;
> +}
> +
> +/*
> + * called by the usb core when the device is removed from the system
> + */
> +static void esd_usb2_disconnect(struct usb_interface *intf)
> +{
> +     struct esd_usb2 *dev = usb_get_intfdata(intf);
> +     struct net_device *netdev;
> +     int i;
> +
> +#ifdef CONFIG_SYSFS
> +     device_remove_file(&intf->dev, &dev_attr_firmware);
> +     device_remove_file(&intf->dev, &dev_attr_hardware);
> +     device_remove_file(&intf->dev, &dev_attr_nets);
> +#endif
> +     usb_set_intfdata(intf, NULL);
> +
> +     if (dev) {
> +             for (i = 0; i < dev->no_nets; i++) {
> +                     if (dev->nets[i]) {
> +                             netdev = dev->nets[i]->netdev;
> +                             unregister_netdev(netdev);
> +                             free_candev(netdev);
> +                     }
> +             }
> +             unlink_all_urbs(dev);
> +     }
> +}
> +
> +/* usb specific object needed to register this driver with the usb subsystem 
> */
> +static struct usb_driver esd_usb2_driver = {
> +     .name = "esd_usb2",
> +     .probe = esd_usb2_probe,
> +     .disconnect = esd_usb2_disconnect,
> +     .id_table = esd_usb2_table,
> +};
> +
> +static int __init esd_usb2_init(void)
> +{
> +     int err;
> +
> +     printk(KERN_INFO "esd USB Mini/2 kernel driver\n");
> +
> +     /* register this driver with the USB subsystem */
> +     err = usb_register(&esd_usb2_driver);
> +
> +     if (err) {
> +             err("usb_register failed. Error number %d\n", err);
> +             return err;
> +     }
> +
> +     return 0;
> +}
> +
> +static void __exit esd_usb2_exit(void)
> +{
> +     /* deregister this driver with the USB subsystem */
> +     usb_deregister(&esd_usb2_driver);
> +}
> +
> +module_init(esd_usb2_init);
> +module_exit(esd_usb2_exit);
> Index: kernel/2.6/drivers/net/can/usb/Makefile
> ===================================================================
> --- kernel/2.6/drivers/net/can/usb/Makefile   (revision 1095)
> +++ kernel/2.6/drivers/net/can/usb/Makefile   (working copy)
> @@ -16,6 +16,7 @@
>  -include $(TOPDIR)/Makefile.common
>  
>  obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> +obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
>  
>  ifeq ($(CONFIG_CAN_DEBUG_DEVICES),y)
>       EXTRA_CFLAGS += -DDEBUG
> Index: kernel/2.6/drivers/net/can/Makefile
> ===================================================================
> --- kernel/2.6/drivers/net/can/Makefile       (revision 1095)
> +++ kernel/2.6/drivers/net/can/Makefile       (working copy)
> @@ -19,6 +19,7 @@
>  export CONFIG_CAN_EMS_PCI=m
>  export CONFIG_CAN_EMS_PCMCIA=m
>  export CONFIG_CAN_ESD_PCI331=m
> +export CONFIG_CAN_ESD_USB2=m
>  export CONFIG_CAN_PIPCAN=m
>  export CONFIG_CAN_SOFTING=m
>  export CONFIG_CAN_SOFTING_CS=m
> _______________________________________________
> Socketcan-core mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/socketcan-core


-- 
Pengutronix e.K.                         | Marc Kleine-Budde           |
Linux Solutions for Science and Industry | Phone: +49-231-2826-924     |
Vertretung West/Dortmund                 | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686         | http://www.pengutronix.de   |

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to