Usb to can driver

Usb to can driver

[Max S.]

[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]

Hello,

I've been working on a device and a Linux socket can driver for it.

Its a dual USB to CAN adapter in two variants external USB device with
cable, or a mini pci form factor expansion card.

This is my first Linux driver, and I'd like to have someone look at it.
Feel free to criticize. I have not done any massive testing, but am able
to send and receive frames at 250k +full echo :). Higher speeds should
easily be possible, but i have not tested.

The firmware I have written myself. Here's how it enumerates:

$ lsusb -v -d beef:1234 # :P have yet to get proper USB vender code.

Bus 003 Device 003: ID beef:1234  
Couldn't open device, some information will be missing
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0xbeef 
  idProduct          0x1234 
  bcdDevice            1.00
  iManufacturer           1 
  iProduct                2 
  iSerial                 3 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           32
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0


Basic rundown of what the driver does now:
config information is passed between the host/device using the control
endpoint using brequest as a switch to indicate the command to run or
information requested.

Can frames are exchanged via bulk endpoints in a native format. meaning
a small header and a struct can_frame. The device is responsible for
packaging the can frame in a host readable format. when can frames
arrive, they are simply copied over by linux.

so:
1) When probed config data is exchanged between host and device.
indicating to the device what the 'host frame' format is, and what byte
order to use.
2) Two candevs are registered.

when an interface is brought up/configured:
3) settings are passed on to the device.
4) can controllers get initialized by the device.

When sending messages:
4) an echo id is associated with the frame, copied into the host frame
and sent to the device. bulk ep 2

When receiving messages:
5) data from bulk ep1 is received.
when echo_id == 0: this message was successfully received by the
controller. 
when echo_id != 0: this message was successfully sent by the controller.
echo_id for full echo support. 

Code attached,
Best regards,
Max S.

[-- Attachment #2: Makefile --]
[-- Type: text/x-makefile, Size: 393 bytes --]

#called from kbuild system
ifneq ($(KERNELRELEASE),)
obj-m := ss_usb.o

#called from command line
else
KERNELDIR ?= /lib/modules/$(shell uname -r)/build
PWD := $(shell pwd)
default:
	$(MAKE) -C $(KERNELDIR) M=$(PWD) modules

endif

#sudo modprobe pcan
#sudo modprobe candev
#sudo modprobe netconsole netconsole=6666@192.168.1.54/eth0,6666@192.168.1.39/aa:00:04:00:0a:04
#sudo insmod ss_usb.ko

[-- Attachment #3: ss_usb.c --]
[-- Type: text/x-csrc, Size: 22148 bytes --]

#include <linux/init.h>
#include <linux/signal.h>
//#include <linux/slab.h>
//#include <linux/kernel.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>

/* Device specific constants */
#define USB_SSUSB_VENDOR_ID         0xbeef
#define USB_SSUSB_AT32_PRODUCT_ID   0x1234

#define SS_USB_ENDPOINT_IN          1
#define SS_USB_ENDPOINT_OUT         2

#define SS_USB_BREQ_HOST_FORMAT     0
#define SS_USB_BREQ_BITTIMING       1
#define SS_USB_BREQ_MODE            2
#define SS_USB_BREQ_BERR            3
#define SS_USB_BREQ_BT_CONST        4


#define SS_CAN_MODE_RESET           0
#define SS_CAN_MODE_START           1

#define SS_CAN_STATE_ERROR_ACTIVE   0
#define SS_CAN_STATE_ERROR_WARNING  1
#define SS_CAN_STATE_ERROR_PASSIVE  2
#define SS_CAN_STATE_BUS_OFF        3
#define SS_CAN_STATE_STOPPED        4
#define SS_CAN_STATE_SLEEPING       5

/* data types passed between host and device */
struct __packed device_config {
	u16 byte_order;

	u32 hf_size;	/* size of a host frame */
	u32 hf_can_id;
	u32 hf_can_dlc;
	u32 hf_data;
	u32 hf_echo_id;
	u32 hf_channel;
};

struct __packed device_mode {
	u8  mode;
};

struct __packed device_state {
	u8  state;
	u32 rxerr;
	u32 txerr;
};

struct __packed device_bittiming {
	u32 prop_seg;
	u32 phase_seg1;
	u32 phase_seg2;
	u32 sjw;
	u32 brp;
};

struct __packed device_bt_const {
	u32 fclk_can;
	u32 tseg1_min;
	u32 tseg1_max;
	u32 tseg2_min;
	u32 tseg2_max;
	u32 sjw_max;
	u32 brp_min;
	u32 brp_max;
	u32 brp_inc;
};

struct ss_host_frame {
	u8  channel;
	u32 echo_id;
	struct can_frame frame;
};

#define MAX_TX_URBS 10
#define MAX_RX_URBS 10

#define MAX_INTF 2

struct ss_tx_context {
	struct ss_can *dev;
	unsigned int echo_id;
};

struct ss_can {
	struct can_priv can; /* must be the first member */

	struct ss_usb *parent;

	struct net_device *netdev;
	struct usb_device *udev;
	struct usb_interface *iface;

	struct can_bittiming_const bt_const;
	unsigned int channel;	/* channel number */

	struct ss_tx_context tx_context[MAX_TX_URBS];

	struct usb_anchor tx_submitted;
	atomic_t active_tx_urbs;

	int open_time;
};

/* usb interface struct */
struct ss_usb {
//	struct usb_device *udev;
//	struct usb_interface *iface;

	struct ss_can *canch[MAX_INTF];

	struct usb_anchor rx_submitted;

	atomic_t active_channels;
};

/* 'allocate' a tx context.
 * returns a valid tx context or NULL if there is no space.
 */
static struct ss_tx_context *ss_alloc_tx_context( struct ss_can *dev )
{
	int ii=0;
	for(;ii<MAX_TX_URBS;ii++){
		if(dev->tx_context[ii].echo_id==MAX_TX_URBS){
			dev->tx_context[ii].echo_id=ii;
			return(&dev->tx_context[ii]);
		}
	}
	return NULL;
}

/* releases a tx context
 */
static void ss_free_tx_context(struct ss_tx_context *txc)
{
	txc->echo_id = MAX_TX_URBS;
}

/* Get a tx context by id. 
 */
static struct ss_tx_context *ss_get_tx_context( struct ss_can *dev, unsigned int id )
{
	if(id<MAX_TX_URBS){
		if(dev->tx_context[id].echo_id==id){
			return &dev->tx_context[id];
		}
	}

	return NULL;
}

/* Get a tx contexts id. 
 */
static unsigned int ss_tx_context_id( struct ss_tx_context *txc ){
	return(txc->echo_id);
}

#define BUFFER_SIZE sizeof(struct ss_host_frame)

/**************************** USB CALLBACKS **************************/
static void ss_usb_recieve_bulk_callback( struct urb *urb )
{
	struct ss_usb *usbcan = urb->context;
	struct ss_can *dev;
	struct net_device *netdev;
	int rc;
	struct net_device_stats *stats;
	struct sk_buff *skb;
	struct can_frame *cf;
	struct ss_host_frame *hf = urb->transfer_buffer;
	struct ss_tx_context *txc;

	BUG_ON(!usbcan);

	switch( urb->status ){
	case 0: /* success */
		break;
	case -ENOENT:
	case -ESHUTDOWN:
		return;
	default:
		printk(KERN_INFO "Rx URB aborted (%d)\n", urb->status);
		return;	//do not resubmit aborted urbs... happens when device goes down
		//goto resubmit_urb;
	}

	/* device reports out of range channel id */
	if(hf->channel>=MAX_INTF){
		printk(KERN_INFO "%d] strange channel #\n",hf->channel);
		goto resubmit_urb;
	}

	/* device reports bad channel id */
	dev = usbcan->canch[hf->channel];
	if( !dev )		//FIXME use assert instead
		goto resubmit_urb;

	netdev = dev->netdev;
	if( !netdev )		//FIXME use assert instead
		goto resubmit_urb;

	if( !netif_device_present(netdev) )
		return;

	stats = &dev->netdev->stats;

	if(hf->echo_id == 0){ /* normal */
		printk(KERN_INFO "%d] got msg\n",hf->channel);
		skb = alloc_can_skb(dev->netdev, &cf);
		if( skb==NULL )
			return;

		*cf = hf->frame;
		cf->can_dlc = get_can_dlc(hf->frame.can_dlc);

		netif_rx(skb);

		stats->rx_packets++;
		stats->rx_bytes += cf->can_dlc;
	}else{		/* echo_id = hf->echo_id-1 //echo ids start at 1 */
		txc = ss_get_tx_context(dev,hf->echo_id-1);

		/* bad devices send bad echo_ids. */
		if(!txc){
			printk(KERN_INFO "%d] bad echo %d\n",hf->channel,hf->echo_id);
			goto resubmit_urb;
		}else{
			printk(KERN_INFO "%d] got echo %d\n",hf->channel,hf->echo_id);
		}

		can_get_echo_skb(netdev, ss_tx_context_id(txc));

		ss_free_tx_context(txc);

		netdev->stats.tx_packets++;
//FIXME: report dlc
//		stats->tx_bytes += cf->can_dlc;

		netdev->trans_start = jiffies;

		if (netif_queue_stopped(netdev))
			netif_wake_queue(netdev);
	}


resubmit_urb:
//	do i really need to refill? the urb should still be valid
//	usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, SS_USB_ENDPOINT_IN(dev->iface->altsetting[0].desc.bInterfaceNumber)), urb->transfer_buffer, BUFFER_SIZE, ss_usb_recieve_bulk_callback, dev);

	rc = usb_submit_urb(urb, GFP_ATOMIC);

	if( rc==-ENODEV )
		printk(KERN_INFO "resubmit error -ENODEV\n");
		//netif_device_detach(netdev);
	else if( rc )
		printk(KERN_INFO "resubmit error %d\n",rc);
}

static void ss_usb_xmit_callback( struct urb *urb )
{
	struct ss_tx_context *txc = urb->context;
	struct ss_can *dev;
	struct net_device *netdev;

	BUG_ON(!txc);

	dev = txc->dev;
	BUG_ON(!dev);

	netdev = dev->netdev;
	BUG_ON(!netdev);

	if (urb->status){
		printk(KERN_INFO "%d] xmit err %d\n",dev->channel,txc->echo_id+1);
	}else{
		printk(KERN_INFO "%d] xmit ok  %d\n",dev->channel,txc->echo_id+1);
	}

	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;

	/* The urb never made it to the device. Drop this packet. */
	if (urb->status){
		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n", urb->status);
		can_free_echo_skb(netdev, ss_tx_context_id(txc));
		ss_free_tx_context(txc);
		netdev->stats.tx_dropped++;
	}

	if (netif_queue_stopped(netdev))
		netif_wake_queue(netdev);
}

/****************************** UBS CAN ******************************/
static int ss_usb_set_bittiming( struct net_device *netdev )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct can_bittiming *bt = &dev->can.bittiming;
	struct usb_interface *intf = dev->iface;
	int rc;

	struct __packed device_bittiming dbt = {
		.prop_seg = bt->prop_seg,
		.phase_seg1 = bt->phase_seg1,
		.phase_seg2 = bt->phase_seg2,
		.sjw = bt->sjw,
		.brp = bt->brp
	};

	printk(KERN_INFO "CH: %d\n",dev->channel);
	printk(KERN_INFO "PRS %d\n",bt->prop_seg);
	printk(KERN_INFO "PH1 %d\n",bt->phase_seg1);
	printk(KERN_INFO "PH2 %d\n",bt->phase_seg2);
	printk(KERN_INFO "SJW %d\n",bt->sjw);
	printk(KERN_INFO "BRP %d\n",bt->brp);

	rc = usb_control_msg(interface_to_usbdev(intf),
			usb_sndctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
			SS_USB_BREQ_BITTIMING,					/* request */
			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,	/* request type */
			dev->channel,						/* value */
			0,							/* interface */
			&dbt,							/* data */
			sizeof(dbt),						/* len */
			1000);
	if(rc<0){
		dev_warn(netdev->dev.parent, "Couldn't set bittimings %d",rc);
	}

	return rc;
}

static int ss_usb_set_mode( struct net_device *netdev, enum can_mode mode )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct usb_interface *intf = dev->iface;
	struct device_mode dm;
	int rc;
	printk(KERN_INFO "ss can mode %d\n",mode);

	if( !dev->open_time )
		return -EINVAL;


	switch( mode ){
	case CAN_MODE_START:
		dm.mode = SS_CAN_MODE_RESET;

		rc = usb_control_msg(interface_to_usbdev(intf),
				usb_sndctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
				SS_USB_BREQ_MODE,					/* request */
				USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,	/* request type */
				dev->channel,						/* value */
				0,							/* index */
				&dm,							/* data */
				sizeof(dm),						/* len */
				1000);
		if( rc<0 ){
			dev_warn(netdev->dev.parent, "Couldn't start device %d",rc);
			return rc;
		}

		if( netif_queue_stopped(netdev) )
			netif_wake_queue(netdev);
		break;
	case CAN_MODE_STOP:
	case CAN_MODE_SLEEP:
	default:
		return -EOPNOTSUPP;
	}

	return 0;
}

int ss_usb_get_state( const struct net_device *netdev, enum can_state *state )
{

	struct ss_can *dev = netdev_priv(netdev);
	struct usb_interface *intf = dev->iface;
	struct device_state dstate;
	int rc;
	printk(KERN_INFO "ss can state\n");

	rc = usb_control_msg(interface_to_usbdev(intf),
			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
			SS_USB_BREQ_BERR,					/* request */
			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,		/* request type */
			dev->channel,						/* value */
			0,							/* index */
			&dstate,						/* data */
			sizeof(dstate),						/* len */
			1000);
	if( rc<0 ){
		dev_warn(netdev->dev.parent, "Couldn't get state %d",rc);
		return rc;
	}

	printk(KERN_INFO "state: %d\n",dstate.state);

	switch(dstate.state){
		case SS_CAN_STATE_ERROR_ACTIVE:;
			*state = CAN_STATE_ERROR_ACTIVE;     /* RX/TX error count < 96 */
			return 0;
		case SS_CAN_STATE_ERROR_WARNING:;
			*state = CAN_STATE_ERROR_WARNING;    /* RX/TX error count < 128 */
			return 0;
		case SS_CAN_STATE_ERROR_PASSIVE:;
			*state = CAN_STATE_ERROR_PASSIVE;    /* RX/TX error count < 256 */
			return 0;
		case SS_CAN_STATE_BUS_OFF:;
			*state = CAN_STATE_BUS_OFF;          /* RX/TX error count >= 256 */
			return 0;
		case SS_CAN_STATE_STOPPED:;
			*state = CAN_STATE_STOPPED;          /* Device is stopped */
			return 0;
		case SS_CAN_STATE_SLEEPING:;
			*state = CAN_STATE_SLEEPING;         /* Device is sleeping */
			return 0;
	}

	return -ENOTSUPP;
}

static int ss_usb_get_berr_counter( const struct net_device *netdev, struct can_berr_counter *bec )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct usb_interface *intf = dev->iface;
	struct device_state dstate;
	int rc;

	rc = usb_control_msg(interface_to_usbdev(intf),
			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
			SS_USB_BREQ_BERR,					/* request */
			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,		/* request type */
			dev->channel,						/* value */
			0,							/* index */
			&dstate,						/* data */
			sizeof(dstate),						/* len */
			1000);
	if( rc<0 ){
		dev_warn(netdev->dev.parent, "Couldn't get error counters %d",rc);
		return rc;
	}

	printk(KERN_INFO "txerr: %d rxerr: %d\n",dstate.txerr,dstate.rxerr);

	bec->txerr = dstate.txerr;
	bec->rxerr = dstate.rxerr;

	return 0;
}

static netdev_tx_t ss_can_start_xmit( struct sk_buff *skb, struct net_device *netdev )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct net_device_stats *stats = &dev->netdev->stats;
	struct urb *urb;
	struct ss_host_frame *hf;
	int rc;
	struct ss_tx_context *txc;

	if (can_dropped_invalid_skb(netdev, skb))
		return NETDEV_TX_OK;

	/* find an empty context to keep track of transmission */
	txc = ss_alloc_tx_context(dev);
	if (!txc) {
		//dev_kfree_skb(skb);
		return NETDEV_TX_BUSY;
	}

	/* create a URB, and a buffer for it */
	urb = usb_alloc_urb(0, GFP_ATOMIC);
	if (!urb) {
		ss_free_tx_context(txc);
		dev_err(netdev->dev.parent, "No memory left for URBs\n");
		goto nomem;
	}

	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC, &urb->transfer_dma);
	if (!hf) {
		ss_free_tx_context(txc);
		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
		usb_free_urb(urb);
		goto nomem;
	}

	hf->frame = *(struct can_frame *)skb->data;
	hf->echo_id = ss_tx_context_id(txc)+1;
	hf->channel = dev->channel;

	usb_fill_bulk_urb(urb, dev->udev,
			usb_sndbulkpipe(dev->udev, SS_USB_ENDPOINT_OUT),
			hf,
			sizeof(*hf),
			ss_usb_xmit_callback,
			txc);

	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
	usb_anchor_urb(urb, &dev->tx_submitted);

	can_put_echo_skb(skb, netdev, ss_tx_context_id(txc));

	atomic_inc(&dev->active_tx_urbs);

//	printk(KERN_INFO "%d] xmit new %d\n",hf->channel,hf->echo_id);

	rc = usb_submit_urb(urb, GFP_ATOMIC);
	if (unlikely(rc)) {			// usb send failed

		printk(KERN_INFO "usb_submit failed %d\n",rc);

		can_free_echo_skb(netdev, ss_tx_context_id(txc));
		ss_free_tx_context(txc);

		usb_unanchor_urb(urb);
		usb_free_coherent(dev->udev, sizeof(*hf), hf, urb->transfer_dma);
		dev_kfree_skb(skb);

		atomic_dec(&dev->active_tx_urbs);

		if (rc == -ENODEV) {
			netif_device_detach(netdev);
		} else {
			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", rc);

			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);
		}
	}

	/* let usb core take care of this urb */
	usb_free_urb(urb);

	return NETDEV_TX_OK;

nomem:
	dev_kfree_skb(skb);
	stats->tx_dropped++;
	return NETDEV_TX_OK;
}

static int ss_can_open( struct net_device *netdev )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct ss_usb *parent = dev->parent;
	struct device_mode dm;
	int rc,i;

	/* common open */
	rc = open_candev(netdev);
	if( rc )
		return rc;

	if( atomic_add_return( 1, &parent->active_channels )==1 ){
		for( i = 0; i < MAX_RX_URBS; i++ ){
			struct urb *urb = NULL;
			u8 *buf = NULL;

			/* alloc rx urb */
			urb = usb_alloc_urb(0, GFP_KERNEL);
			if (!urb) {
				dev_err(netdev->dev.parent, "No memory left for URBs\n");
				return -ENOMEM;
			}

			/* alloc rx buffer */
			//FIXME: free this memory
			buf = usb_alloc_coherent(dev->udev, 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;
			}


			/* fill, anchor, and submit rx urb */
			usb_fill_bulk_urb(urb,
					dev->udev,
					usb_rcvbulkpipe(dev->udev, SS_USB_ENDPOINT_IN),
					buf,
					BUFFER_SIZE,
					ss_usb_recieve_bulk_callback,
					parent
					);
			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

			usb_anchor_urb(urb, &parent->rx_submitted);

			rc = usb_submit_urb(urb, GFP_KERNEL);
			if( rc ){
				if( rc == -ENODEV )
					netif_device_detach( dev->netdev );

					//FIXME: free rx buffer/urb maybe?
					usb_unanchor_urb(urb);
				break;
			}

			/* Drop reference, USB core will take care of freeing it */
			usb_free_urb(urb);
		}
	}

	/* finally start device */
	dm.mode = SS_CAN_MODE_START;
	rc = usb_control_msg(interface_to_usbdev(dev->iface),
			usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),/* pipe */
			SS_USB_BREQ_MODE,					/* request */
			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,	/* request type */
			dev->channel,						/* value */
			0,							/* index */
			&dm,							/* data */
			sizeof(dm),						/* len */
			1000);

	if(rc<0){
		dev_warn(netdev->dev.parent, "Couldn't start device %d",rc);
	}

	dev->open_time = jiffies;

	netif_start_queue(netdev);

	printk(KERN_INFO "ss can open\n");
	return 0;
}

/* think: ip link set canx down */
static int ss_can_close( struct net_device *netdev )
{
	struct ss_can *dev = netdev_priv(netdev);
	struct ss_usb *parent = dev->parent;

	netif_stop_queue(netdev);

	/* Stop polling */
	if( atomic_dec_and_test( &parent->active_channels ) ){
		usb_kill_anchored_urbs(&parent->rx_submitted);
	//	foreach child
	//	usb_kill_anchored_urbs(&parent->tx_submitted);
		//CLEANUP OTHER
		/* Set CAN controller to reset mode */
	}

	close_candev(netdev);

	dev->open_time = 0;

	printk(KERN_INFO "ss can close\n");
	return 0;
}

/******************************** USB ********************************/

static const struct net_device_ops ss_usb_netdev_ops = {
	.ndo_open = ss_can_open,
	.ndo_stop = ss_can_close,
	.ndo_start_xmit = ss_can_start_xmit,
};

static int ss_make_candev( struct ss_can **dev, unsigned int channel, struct usb_interface *intf );
static void ss_destroy_candev( struct ss_can *dev );

static int ss_usb_probe( struct usb_interface *intf, const struct usb_device_id *id )
{
	struct ss_usb *dev;
	int rc = -ENOMEM;
	unsigned int icount,i;

	struct device_config conf;

	/* send host config */
	conf = (struct device_config){
		.byte_order = 0xbeef,
		.hf_size = sizeof(struct ss_host_frame),
		.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
		.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
		.hf_data = offsetof(struct ss_host_frame, frame.data),
		.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
		.hf_channel = offsetof(struct ss_host_frame, channel),
	};

	rc = usb_control_msg(interface_to_usbdev(intf),
			usb_sndctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
			SS_USB_BREQ_HOST_FORMAT,				/* request */
			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,	/* request type */
			1,							/* value */
			intf->altsetting[0].desc.bInterfaceNumber,		/* index */
			&conf,							/* data */
			sizeof(conf),						/* len */
			1000);

	if( rc<0 ){
		dev_err(&intf->dev, "ss_usb: Couldn't send data format %d\n",rc);
		return rc;
	}

	/* fetch interface count and global settings etc.. */
	//TODO: --^
	icount = 2;

	if(icount>MAX_INTF){
		dev_err(&intf->dev, "ss_usb: Cannot handle more that %d Can interfaces\n",MAX_INTF);
		return -EINVAL;
	}

	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
	init_usb_anchor(&dev->rx_submitted);

	atomic_set(&dev->active_channels, 0);

	usb_set_intfdata(intf, dev);

	for(i=0;i<icount;i++){
		rc = ss_make_candev(&dev->canch[i],i,intf);
		if( rc ){
			icount = i;
			for(i=0;i<icount;i++){
				ss_destroy_candev(dev->canch[i]);
				dev->canch[icount] = NULL;
			}
			return rc;
		}
		dev->canch[i]->parent = dev;
	}

	printk(KERN_INFO "ss usb probe\n");
	return 0;
}

static void ss_usb_disconnect( struct usb_interface *intf )
{
	unsigned i;
	struct ss_usb *dev = usb_get_intfdata(intf);
	usb_set_intfdata(intf, NULL);

	if( !dev ){
		printk(KERN_INFO "ss usb disconnect (nodata)\n");
		return;
	}

	for(i=0;i<MAX_INTF;i++){
		struct ss_can *can=dev->canch[i];

		if(!can)
			continue;

		//ss_destroy_candev(dev->canch[i]);

		unregister_netdev(can->netdev);
		free_candev(can->netdev);

		usb_kill_anchored_urbs(&can->tx_submitted);

//		unlink_all_urbs(can->);

//		usb_free_urb(dev->intr_urb);
//		kfree(dev);
	}

	usb_kill_anchored_urbs(&dev->rx_submitted);

	printk(KERN_INFO "ss usb disconnect\n");
}

/******************* EVERYTHING BELLOW HERE WORKS! *******************/

MODULE_DEVICE_TABLE(usb, ss_usb_table);
static struct usb_device_id ss_usb_table[] = {
	{USB_DEVICE(USB_SSUSB_VENDOR_ID, USB_SSUSB_AT32_PRODUCT_ID)},
	{} /* Terminating entry */
};

static struct usb_driver ss_usb_driver = {
	.name       = "ss_usb",
	.probe      = ss_usb_probe,
	.disconnect = ss_usb_disconnect,
	.id_table   = ss_usb_table,
	//FIXME: suspend, resume, etc...
};

static int __init ss_usb_init( void )
{
	printk(KERN_INFO "ss usb init\n");
	return usb_register(&ss_usb_driver);
}

static void __exit ss_usb_exit( void )
{
	printk(KERN_INFO "ss usb exit\n");
	usb_deregister(&ss_usb_driver);
}

module_init(ss_usb_init);
module_exit(ss_usb_exit);

MODULE_AUTHOR("Maximilian Schneider <mws@schneidersoft.net>");
MODULE_DESCRIPTION("socket CAN device driver for SchneiderSoft USB2.0 to CAN interfaces.");
MODULE_LICENSE("GPL v2");





static int ss_make_candev( struct ss_can **out, unsigned int channel, struct usb_interface *intf )
{
	struct ss_can *dev;
	struct device_bt_const bt_const;
	struct net_device *netdev;
	int rc;
	/* fetch bit timing constants */
	rc = usb_control_msg(interface_to_usbdev(intf),
			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),		/* pipe */
			SS_USB_BREQ_BT_CONST,					/* request */
			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,		/* request type */
			channel,						/* value */
			0,							/* index */
			&bt_const,						/* data */
			sizeof(bt_const),					/* len */
			1000);

	if( rc<0 ){
		dev_err(&intf->dev, "ss_usb: Couldn't get bit timing const for channel %d\n",rc);
		return rc;
	}

	printk(KERN_INFO "fclock: %d\n",bt_const.fclk_can);

	/* create netdev */
	netdev = alloc_candev(sizeof(struct ss_can),MAX_TX_URBS);
	if( !netdev ){
		dev_err(&intf->dev, "ss_usb: Couldn't alloc candev\n");
		return -ENOMEM;
	}

	dev = netdev_priv(netdev);

	netdev->netdev_ops = &ss_usb_netdev_ops;
	netdev->flags |= IFF_ECHO; /* we support full roundtrip echo */

	/* dev settup */
	dev->bt_const = (struct can_bittiming_const){
		.name = "ss_usb",
		.tseg1_min = bt_const.tseg1_min,
		.tseg1_max = bt_const.tseg1_max,
		.tseg2_min = bt_const.tseg2_min,
		.tseg2_max = bt_const.tseg2_max,
		.sjw_max = bt_const.sjw_max,
		.brp_min = bt_const.brp_min,
		.brp_max = bt_const.brp_max,
		.brp_inc = bt_const.brp_inc
	};

	dev->udev = interface_to_usbdev(intf);
	dev->iface = intf;
	dev->netdev = netdev;
	dev->channel = channel;

	init_usb_anchor(&dev->tx_submitted);
	atomic_set(&dev->active_tx_urbs, 0);
	for(rc=0;rc<MAX_TX_URBS;rc++){
		dev->tx_context[rc].dev = dev;
		dev->tx_context[rc].echo_id = MAX_TX_URBS;
	}

	/* can settup */
	dev->can.state = CAN_STATE_STOPPED;
	dev->can.clock.freq = bt_const.fclk_can;
	dev->can.bittiming_const = &dev->bt_const;
	dev->can.do_set_bittiming = ss_usb_set_bittiming;
	dev->can.do_set_mode = ss_usb_set_mode;
	dev->can.do_get_state = ss_usb_get_state;
	dev->can.do_get_berr_counter = ss_usb_get_berr_counter;
	dev->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;

	SET_NETDEV_DEV(netdev, &intf->dev);

	rc = register_candev(dev->netdev);
	if( rc ){
		free_candev(dev->netdev);
		dev_err(&intf->dev, "ss_usb: Couldn't register candev\n");
		return(rc);
	}

	*out = dev;
	return(0);
}

static void ss_destroy_candev( struct ss_can *dev )
{
	unregister_candev(dev->netdev);
	free_candev(dev->netdev);
}

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Hello Max,

On 04/23/2013 07:15 PM, Max S. wrote:
> I've been working on a device and a Linux socket can driver for it.
> 
> Its a dual USB to CAN adapter in two variants external USB device with
> cable, or a mini pci form factor expansion card.
> 
> This is my first Linux driver, and I'd like to have someone look at it.
> Feel free to criticize. I have not done any massive testing, but am able
> to send and receive frames at 250k +full echo :). Higher speeds should
> easily be possible, but i have not tested.

Please use "./scripts/checkpatch.pl" from the kernel and fix all coding
style errors. The compile your code with "make C=1", fix all errors.
Then repost :).

Can you explain how the "struct device_config" works.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Max S.]

[-- Attachment #1: Type: text/plain, Size: 2668 bytes --]

Hello Marc,

On Tue, 2013-04-23 at 23:47 +0200, Marc Kleine-Budde wrote:
Hello Max,
> 
> On 04/23/2013 07:15 PM, Max S. wrote:
> > I've been working on a device and a Linux socket can driver for it.
> > 
> > Its a dual USB to CAN adapter in two variants external USB device
with
> > cable, or a mini pci form factor expansion card.
> > 
> > This is my first Linux driver, and I'd like to have someone look at
it.
> > Feel free to criticize. I have not done any massive testing, but am
able
> > to send and receive frames at 250k +full echo :). Higher speeds
should
> > easily be possible, but i have not tested.
> 
> Please use "./scripts/checkpatch.pl" from the kernel and fix all
coding
> style errors. The compile your code with "make C=1", fix all errors.
> Then repost :).
> 
OK. attached.


> Can you explain how the "struct device_config" works.
> 
struct __packed device_config {
	u16 byte_order;

	u32 hf_size;	/* size of a host frame */
	u32 hf_can_id;
	u32 hf_can_dlc;
	u32 hf_data;
	u32 hf_echo_id;
	u32 hf_channel;
} conf;

conf = (struct device_config){
	.byte_order = 0xbeef,
	.hf_size = sizeof(struct ss_host_frame),
	.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
	.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
	.hf_data = offsetof(struct ss_host_frame, frame.data),
	.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
	.hf_channel = offsetof(struct ss_host_frame, channel),
};

... send conf to device, packed that should be 26 bytes.

The device uses byte_order which will appear as either 0xbeef or 0xefbe
to convert the other members to device byte order. (and to convert all
future messages)

The device can then tell how the host prefers its can frames formatted.
The idea is that the struct device_config holds enough data to allow the
device to construct a struct ss_host_frame in host format, including
padding & byte order.

since a struct ss_host_frame is simply a wrapper that includes a struct
can_frame, when a message arrives from the device, the host can copy the
struct can frame member without modification from the struct
ss_host_frame into the skb.

assumptions that are made:
* The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
* It is also assumed that existing defines like CAN_EFF_FLAG in can.h
and can/error.h don't change, as they are used by the device to
construct the can_id field.
* additionally it is assumes that sizeof(struct ss_host_frame) is small
enough not to consume more bandwidth than is available to a usb2.0
device at full CAN speeds (which is the case, unless gcc decides to add
an unrealistic amount of padding).

Thanks for the audience,
Max Schneider.


[-- Attachment #2: 0001-added-the-ss_usb-driver-source-files.patch --]
[-- Type: text/x-patch, Size: 22979 bytes --]

From 23094effcef481efff7b8107ac452a28448a923b Mon Sep 17 00:00:00 2001
From: blackbox <maximilian@blackbox>
Date: Wed, 24 Apr 2013 13:50:41 +0000
Subject: [PATCH 1/2] added the ss_usb driver source files.

---
 drivers/net/can/usb/Makefile |    1 +
 drivers/net/can/usb/ss_usb.c |  939 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 940 insertions(+)
 create mode 100644 drivers/net/can/usb/ss_usb.c

diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index becef46..c341a1b 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -7,5 +7,6 @@ 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_8DEV_USB) += usb_8dev.o
+obj-$(CONFIG_CAN_SS_USB) += ss_usb.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/ss_usb.c b/drivers/net/can/usb/ss_usb.c
new file mode 100644
index 0000000..cc86e0e
--- /dev/null
+++ b/drivers/net/can/usb/ss_usb.c
@@ -0,0 +1,939 @@
+#include <linux/init.h>
+#include <linux/signal.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>
+
+/* Device specific constants */
+#define USB_SSUSB_VENDOR_ID         0xbeef
+#define USB_SSUSB_AT32_PRODUCT_ID   0x1234
+
+#define SS_USB_ENDPOINT_IN          1
+#define SS_USB_ENDPOINT_OUT         2
+
+#define SS_USB_BREQ_HOST_FORMAT     0
+#define SS_USB_BREQ_BITTIMING       1
+#define SS_USB_BREQ_MODE            2
+#define SS_USB_BREQ_BERR            3
+#define SS_USB_BREQ_BT_CONST        4
+
+
+#define SS_CAN_MODE_RESET           0
+#define SS_CAN_MODE_START           1
+
+#define SS_CAN_STATE_ERROR_ACTIVE   0
+#define SS_CAN_STATE_ERROR_WARNING  1
+#define SS_CAN_STATE_ERROR_PASSIVE  2
+#define SS_CAN_STATE_BUS_OFF        3
+#define SS_CAN_STATE_STOPPED        4
+#define SS_CAN_STATE_SLEEPING       5
+
+/* data types passed between host and device */
+struct __packed device_config {
+	u16 byte_order;
+
+	u32 hf_size;	/* size of a host frame */
+	u32 hf_can_id;
+	u32 hf_can_dlc;
+	u32 hf_data;
+	u32 hf_echo_id;
+	u32 hf_channel;
+};
+
+struct __packed device_mode {
+	u8  mode;
+};
+
+struct __packed device_state {
+	u8  state;
+	u32 rxerr;
+	u32 txerr;
+};
+
+struct __packed device_bittiming {
+	u32 prop_seg;
+	u32 phase_seg1;
+	u32 phase_seg2;
+	u32 sjw;
+	u32 brp;
+};
+
+struct __packed device_bt_const {
+	u32 fclk_can;
+	u32 tseg1_min;
+	u32 tseg1_max;
+	u32 tseg2_min;
+	u32 tseg2_max;
+	u32 sjw_max;
+	u32 brp_min;
+	u32 brp_max;
+	u32 brp_inc;
+};
+
+struct ss_host_frame {
+	u8  channel;
+	u32 echo_id;
+	struct can_frame frame;
+};
+
+#define MAX_TX_URBS 10
+#define MAX_RX_URBS 10
+
+#define MAX_INTF 2
+
+struct ss_tx_context {
+	struct ss_can *dev;
+	unsigned int echo_id;
+};
+
+struct ss_can {
+	struct can_priv can; /* must be the first member */
+
+	struct ss_usb *parent;
+
+	struct net_device *netdev;
+	struct usb_device *udev;
+	struct usb_interface *iface;
+
+	struct can_bittiming_const bt_const;
+	unsigned int channel;	/* channel number */
+
+	struct ss_tx_context tx_context[MAX_TX_URBS];
+
+	struct usb_anchor tx_submitted;
+	atomic_t active_tx_urbs;
+
+	int open_time;
+};
+
+/* usb interface struct */
+struct ss_usb {
+//	struct usb_device *udev;
+//	struct usb_interface *iface;
+
+	struct ss_can *canch[MAX_INTF];
+
+	struct usb_anchor rx_submitted;
+
+	atomic_t active_channels;
+};
+
+/* 'allocate' a tx context.
+ * returns a valid tx context or NULL if there is no space.
+ */
+static struct ss_tx_context *ss_alloc_tx_context( struct ss_can *dev )
+{
+	int ii=0;
+	for(;ii<MAX_TX_URBS;ii++){
+		if(dev->tx_context[ii].echo_id==MAX_TX_URBS){
+			dev->tx_context[ii].echo_id=ii;
+			return(&dev->tx_context[ii]);
+		}
+	}
+	return NULL;
+}
+
+/* releases a tx context
+ */
+static void ss_free_tx_context(struct ss_tx_context *txc)
+{
+	txc->echo_id = MAX_TX_URBS;
+}
+
+/* Get a tx context by id. 
+ */
+static struct ss_tx_context *ss_get_tx_context(
+	struct ss_can *dev,
+	unsigned int id )
+{
+	if(id<MAX_TX_URBS){
+		if(dev->tx_context[id].echo_id==id){
+			return &dev->tx_context[id];
+		}
+	}
+
+	return NULL;
+}
+
+/* Get a tx contexts id. 
+ */
+static unsigned int ss_tx_context_id( struct ss_tx_context *txc ){
+	return(txc->echo_id);
+}
+
+#define BUFFER_SIZE sizeof(struct ss_host_frame)
+
+/**************************** USB CALLBACKS **************************/
+static void ss_usb_recieve_bulk_callback( struct urb *urb )
+{
+	struct ss_usb *usbcan = urb->context;
+	struct ss_can *dev;
+	struct net_device *netdev;
+	int rc;
+	struct net_device_stats *stats;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	struct ss_host_frame *hf = urb->transfer_buffer;
+	struct ss_tx_context *txc;
+
+	BUG_ON(!usbcan);
+
+	switch( urb->status ){
+	case 0: /* success */
+		break;
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+	default:
+		printk(KERN_INFO "Rx URB aborted (%d)\n", urb->status);
+		/* do not resubmit aborted urbs. eg: when device goes down */
+		return;
+	}
+
+	/* device reports out of range channel id */
+	if(hf->channel>=MAX_INTF){
+		printk(KERN_INFO "%d] strange channel #\n",hf->channel);
+		goto resubmit_urb;
+	}
+
+	/* device reports bad channel id */
+	dev = usbcan->canch[hf->channel];
+
+	BUG_ON( !dev );
+
+	netdev = dev->netdev;
+	BUG_ON( !netdev );
+
+	if( !netif_device_present(netdev) )
+		return;
+
+	stats = &dev->netdev->stats;
+
+	if(hf->echo_id == 0){ /* normal */
+		printk(KERN_INFO "%d] got msg\n",hf->channel);
+		skb = alloc_can_skb(dev->netdev, &cf);
+		if( skb==NULL )
+			return;
+
+		*cf = hf->frame;
+		cf->can_dlc = get_can_dlc(hf->frame.can_dlc);
+
+		netif_rx(skb);
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	}else{		/* echo_id = hf->echo_id-1 //echo ids start at 1 */
+		txc = ss_get_tx_context(dev,hf->echo_id-1);
+
+		/* bad devices send bad echo_ids. */
+		if(!txc){
+			printk(KERN_INFO "%d] bad echo %d\n",hf->channel,hf->echo_id);
+			goto resubmit_urb;
+		}else{
+			printk(KERN_INFO "%d] got echo %d\n",hf->channel,hf->echo_id);
+		}
+
+		can_get_echo_skb(netdev, ss_tx_context_id(txc));
+
+		ss_free_tx_context(txc);
+
+		netdev->stats.tx_packets++;
+		/* FIXME: report dlc:
+		stats->tx_bytes += cf->can_dlc; */
+
+		netdev->trans_start = jiffies;
+
+		if (netif_queue_stopped(netdev))
+			netif_wake_queue(netdev);
+	}
+
+
+resubmit_urb:
+	/* do i really need to refill? the urb should still be valid:
+	usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, SS_USB_ENDPOINT_IN(dev->iface->altsetting[0].desc.bInterfaceNumber)), urb->transfer_buffer, BUFFER_SIZE, ss_usb_recieve_bulk_callback, dev); */
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+
+	if( rc==-ENODEV )
+		printk(KERN_INFO "resubmit error -ENODEV\n");
+		/* FIXME: necessary?
+		netif_device_detach(netdev); */
+	else if( rc )
+		printk(KERN_INFO "resubmit error %d\n",rc);
+}
+
+static void ss_usb_xmit_callback( struct urb *urb )
+{
+	struct ss_tx_context *txc = urb->context;
+	struct ss_can *dev;
+	struct net_device *netdev;
+
+	BUG_ON(!txc);
+
+	dev = txc->dev;
+	BUG_ON(!dev);
+
+	netdev = dev->netdev;
+	BUG_ON(!netdev);
+
+	if (urb->status){
+		printk(KERN_INFO "%d] xmit err %d\n",
+			dev->channel,txc->echo_id+1);
+	}else{
+		printk(KERN_INFO "%d] xmit ok  %d\n",
+			dev->channel,txc->echo_id+1);
+	}
+
+	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;
+
+	/* The urb never made it to the device. Drop this packet. */
+	if (urb->status){
+		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
+			urb->status);
+		can_free_echo_skb(netdev, ss_tx_context_id(txc));
+		ss_free_tx_context(txc);
+		netdev->stats.tx_dropped++;
+	}
+
+	if (netif_queue_stopped(netdev))
+		netif_wake_queue(netdev);
+}
+
+/****************************** UBS CAN ******************************/
+static int ss_usb_set_bittiming( struct net_device *netdev )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct can_bittiming *bt = &dev->can.bittiming;
+	struct usb_interface *intf = dev->iface;
+	int rc;
+
+	struct __packed device_bittiming dbt = {
+		.prop_seg = bt->prop_seg,
+		.phase_seg1 = bt->phase_seg1,
+		.phase_seg2 = bt->phase_seg2,
+		.sjw = bt->sjw,
+		.brp = bt->brp
+	};
+
+	printk(KERN_INFO "CH: %d\n",dev->channel);
+	printk(KERN_INFO "PRS %d\n",bt->prop_seg);
+	printk(KERN_INFO "PH1 %d\n",bt->phase_seg1);
+	printk(KERN_INFO "PH2 %d\n",bt->phase_seg2);
+	printk(KERN_INFO "SJW %d\n",bt->sjw);
+	printk(KERN_INFO "BRP %d\n",bt->brp);
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BITTIMING,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dbt,
+			sizeof(dbt),
+			1000);
+	if(rc<0){
+		dev_warn(netdev->dev.parent, "Couldn't set bittimings %d",rc);
+	}
+
+	return rc;
+}
+
+static int ss_usb_set_mode( struct net_device *netdev, enum can_mode mode )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_mode dm;
+	int rc;
+	printk(KERN_INFO "ss can mode %d\n",mode);
+
+	if( !dev->open_time )
+		return -EINVAL;
+
+
+	switch( mode ){
+	case CAN_MODE_START:
+		dm.mode = SS_CAN_MODE_RESET;
+
+		rc = usb_control_msg(interface_to_usbdev(intf),
+				usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+				SS_USB_BREQ_MODE,
+				USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+				dev->channel,
+				0,
+				&dm,
+				sizeof(dm),
+				1000);
+		if( rc<0 ){
+			dev_warn(netdev->dev.parent,"Couldn't start device %d",
+				rc);
+			return rc;
+		}
+
+		if( netif_queue_stopped(netdev) )
+			netif_wake_queue(netdev);
+		break;
+	case CAN_MODE_STOP:
+	case CAN_MODE_SLEEP:
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int ss_usb_get_state( const struct net_device *netdev, enum can_state *state )
+{
+
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_state dstate;
+	int rc;
+	printk(KERN_INFO "ss can state\n");
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dstate,
+			sizeof(dstate),
+			1000);
+	if( rc<0 ){
+		dev_warn(netdev->dev.parent, "Couldn't get state %d",rc);
+		return rc;
+	}
+
+	printk(KERN_INFO "state: %d\n",dstate.state);
+
+	switch(dstate.state){
+		/* RX/TX error count < 96 */
+		case SS_CAN_STATE_ERROR_ACTIVE:;
+			*state = CAN_STATE_ERROR_ACTIVE;
+			return 0;
+		/* RX/TX error count < 128 */
+		case SS_CAN_STATE_ERROR_WARNING:;
+			*state = CAN_STATE_ERROR_WARNING;
+			return 0;
+		/* RX/TX error count < 256 */
+		case SS_CAN_STATE_ERROR_PASSIVE:;
+			*state = CAN_STATE_ERROR_PASSIVE;
+			return 0;
+		/* RX/TX error count >= 256 */
+		case SS_CAN_STATE_BUS_OFF:;
+			*state = CAN_STATE_BUS_OFF;
+			return 0;
+		/* Device is stopped */
+		case SS_CAN_STATE_STOPPED:;
+			*state = CAN_STATE_STOPPED;
+			return 0;
+		/* Device is sleeping */
+		case SS_CAN_STATE_SLEEPING:;
+			*state = CAN_STATE_SLEEPING;
+			return 0;
+	}
+
+	return -ENOTSUPP;
+}
+
+static int ss_usb_get_berr_counter( const struct net_device *netdev,
+	struct can_berr_counter *bec )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_state dstate;
+	int rc;
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dstate,
+			sizeof(dstate),
+			1000);
+	if( rc<0 ){
+		dev_warn(netdev->dev.parent, "Couldn't get error counters %d",
+			rc);
+		return rc;
+	}
+
+	printk(KERN_INFO "txerr: %d rxerr: %d\n",dstate.txerr,dstate.rxerr);
+
+	bec->txerr = dstate.txerr;
+	bec->rxerr = dstate.rxerr;
+
+	return 0;
+}
+
+static netdev_tx_t ss_can_start_xmit( struct sk_buff *skb,
+	struct net_device *netdev )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct net_device_stats *stats = &dev->netdev->stats;
+	struct urb *urb;
+	struct ss_host_frame *hf;
+	int rc;
+	struct ss_tx_context *txc;
+
+	if (can_dropped_invalid_skb(netdev, skb))
+		return NETDEV_TX_OK;
+
+	/* find an empty context to keep track of transmission */
+	txc = ss_alloc_tx_context(dev);
+	if (!txc) {
+		//dev_kfree_skb(skb);
+		return NETDEV_TX_BUSY;
+	}
+
+	/* create a URB, and a buffer for it */
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		ss_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		goto nomem;
+	}
+
+	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
+		&urb->transfer_dma);
+	if (!hf) {
+		ss_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		usb_free_urb(urb);
+		goto nomem;
+	}
+
+	hf->frame = *(struct can_frame *)skb->data;
+	hf->echo_id = ss_tx_context_id(txc)+1;
+	hf->channel = dev->channel;
+
+	usb_fill_bulk_urb(urb, dev->udev,
+			usb_sndbulkpipe(dev->udev, SS_USB_ENDPOINT_OUT),
+			hf,
+			sizeof(*hf),
+			ss_usb_xmit_callback,
+			txc);
+
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+	usb_anchor_urb(urb, &dev->tx_submitted);
+
+	can_put_echo_skb(skb, netdev, ss_tx_context_id(txc));
+
+	atomic_inc(&dev->active_tx_urbs);
+
+//	printk(KERN_INFO "%d] xmit new %d\n",hf->channel,hf->echo_id);
+
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+	if (unlikely(rc)) {			// usb send failed
+
+		printk(KERN_INFO "usb_submit failed %d\n",rc);
+
+		can_free_echo_skb(netdev, ss_tx_context_id(txc));
+		ss_free_tx_context(txc);
+
+		usb_unanchor_urb(urb);
+		usb_free_coherent(dev->udev,
+			sizeof(*hf),
+			hf,
+			urb->transfer_dma);
+
+		dev_kfree_skb(skb);
+
+		atomic_dec(&dev->active_tx_urbs);
+
+		if (rc == -ENODEV) {
+			netif_device_detach(netdev);
+		} else {
+			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", rc);
+
+			stats->tx_dropped++;
+		}
+	} else {
+		netdev->trans_start = jiffies;
+
+		/* Slow down tx path */
+		if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS ){
+			netif_stop_queue(netdev);
+		}
+	}
+
+	/* let usb core take care of this urb */
+	usb_free_urb(urb);
+
+	return NETDEV_TX_OK;
+
+nomem:
+	dev_kfree_skb(skb);
+	stats->tx_dropped++;
+	return NETDEV_TX_OK;
+}
+
+static int ss_can_open( struct net_device *netdev )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct ss_usb *parent = dev->parent;
+	struct device_mode dm;
+	int rc,i;
+
+	/* common open */
+	rc = open_candev(netdev);
+	if( rc )
+		return rc;
+
+	if( atomic_add_return( 1, &parent->active_channels )==1 ){
+		for( i = 0; i < MAX_RX_URBS; i++ ){
+			struct urb *urb = NULL;
+			u8 *buf = NULL;
+
+			/* alloc rx urb */
+			urb = usb_alloc_urb(0, GFP_KERNEL);
+			if (!urb) {
+				dev_err(netdev->dev.parent,
+					"No memory left for URBs\n");
+				return -ENOMEM;
+			}
+
+			/* alloc rx buffer */
+			//FIXME: free this memory
+			buf = usb_alloc_coherent(dev->udev,
+				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;
+			}
+
+
+			/* fill, anchor, and submit rx urb */
+			usb_fill_bulk_urb(urb,
+					dev->udev,
+					usb_rcvbulkpipe(dev->udev,
+						SS_USB_ENDPOINT_IN),
+					buf,
+					BUFFER_SIZE,
+					ss_usb_recieve_bulk_callback,
+					parent
+					);
+			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+			usb_anchor_urb(urb, &parent->rx_submitted);
+
+			rc = usb_submit_urb(urb, GFP_KERNEL);
+			if( rc ){
+				if( rc == -ENODEV )
+					netif_device_detach( dev->netdev );
+
+					//FIXME: free rx buffer/urb maybe?
+					usb_unanchor_urb(urb);
+				break;
+			}
+
+			/* Drop reference,
+			USB core will take care of freeing it */
+			usb_free_urb(urb);
+		}
+	}
+
+	/* finally start device */
+	dm.mode = SS_CAN_MODE_START;
+	rc = usb_control_msg(interface_to_usbdev(dev->iface),
+			usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),
+			SS_USB_BREQ_MODE,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dm,
+			sizeof(dm),
+			1000);
+
+	if(rc<0)
+		dev_warn(netdev->dev.parent, "Couldn't start device %d",rc);
+
+	dev->open_time = jiffies;
+
+	netif_start_queue(netdev);
+
+	printk(KERN_INFO "ss can open\n");
+	return 0;
+}
+
+/* think: ip link set canx down */
+static int ss_can_close( struct net_device *netdev )
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct ss_usb *parent = dev->parent;
+
+	netif_stop_queue(netdev);
+
+	/* Stop polling */
+	if( atomic_dec_and_test( &parent->active_channels ) ){
+		usb_kill_anchored_urbs(&parent->rx_submitted);
+		/* FIXME: Set CAN controller to reset mode */
+	}
+
+	close_candev(netdev);
+
+	dev->open_time = 0;
+
+	printk(KERN_INFO "ss can close\n");
+	return 0;
+}
+
+/******************************** USB ********************************/
+
+static const struct net_device_ops ss_usb_netdev_ops = {
+	.ndo_open = ss_can_open,
+	.ndo_stop = ss_can_close,
+	.ndo_start_xmit = ss_can_start_xmit,
+};
+
+static int ss_make_candev( struct ss_can **dev, unsigned int channel,
+	struct usb_interface *intf );
+static void ss_destroy_candev( struct ss_can *dev );
+
+static int ss_usb_probe( struct usb_interface *intf,
+	const struct usb_device_id *id )
+{
+	struct ss_usb *dev;
+	int rc = -ENOMEM;
+	unsigned int icount,i;
+
+	struct device_config conf;
+
+	/* send host config */
+	conf = (struct device_config){
+		.byte_order = 0xbeef,
+		.hf_size = sizeof(struct ss_host_frame),
+		.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
+		.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
+		.hf_data = offsetof(struct ss_host_frame, frame.data),
+		.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
+		.hf_channel = offsetof(struct ss_host_frame, channel),
+	};
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_HOST_FORMAT,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			1,
+			intf->altsetting[0].desc.bInterfaceNumber,
+			&conf,
+			sizeof(conf),
+			1000);
+
+	if( rc<0 ){
+		dev_err(&intf->dev, "ss_usb: Couldn't send data format %d\n",
+			rc);
+		return rc;
+	}
+
+	/* FIXME: fetch interface count and global settings etc.. */
+	icount = 2;
+
+	if(icount>MAX_INTF){
+		dev_err(&intf->dev,
+			"ss_usb: Cannot handle more that %d Can interfaces\n",
+			MAX_INTF);
+		return -EINVAL;
+	}
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	init_usb_anchor(&dev->rx_submitted);
+
+	atomic_set(&dev->active_channels, 0);
+
+	usb_set_intfdata(intf, dev);
+
+	for(i=0;i<icount;i++){
+		rc = ss_make_candev(&dev->canch[i],i,intf);
+		if( rc ){
+			icount = i;
+			for(i=0;i<icount;i++){
+				ss_destroy_candev(dev->canch[i]);
+				dev->canch[icount] = NULL;
+			}
+			return rc;
+		}
+		dev->canch[i]->parent = dev;
+	}
+
+	printk(KERN_INFO "ss usb probe\n");
+	return 0;
+}
+
+static void ss_usb_disconnect( struct usb_interface *intf )
+{
+	unsigned i;
+	struct ss_usb *dev = usb_get_intfdata(intf);
+	usb_set_intfdata(intf, NULL);
+
+	if( !dev ){
+		printk(KERN_INFO "ss usb disconnect (nodata)\n");
+		return;
+	}
+
+	for(i=0;i<MAX_INTF;i++){
+		struct ss_can *can=dev->canch[i];
+
+		if(!can)
+			continue;
+
+		unregister_netdev(can->netdev);
+		free_candev(can->netdev);
+
+		usb_kill_anchored_urbs(&can->tx_submitted);
+	}
+
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+
+	printk(KERN_INFO "ss usb disconnect\n");
+}
+
+MODULE_DEVICE_TABLE(usb, ss_usb_table);
+static struct usb_device_id ss_usb_table[] = {
+	{USB_DEVICE(USB_SSUSB_VENDOR_ID, USB_SSUSB_AT32_PRODUCT_ID)},
+	{} /* Terminating entry */
+};
+
+static struct usb_driver ss_usb_driver = {
+	.name       = "ss_usb",
+	.probe      = ss_usb_probe,
+	.disconnect = ss_usb_disconnect,
+	.id_table   = ss_usb_table,
+	/* FIXME: suspend, resume, etc... */
+};
+
+static int __init ss_usb_init( void )
+{
+	printk(KERN_INFO "ss usb init\n");
+	return usb_register(&ss_usb_driver);
+}
+
+static void __exit ss_usb_exit( void )
+{
+	printk(KERN_INFO "ss usb exit\n");
+	usb_deregister(&ss_usb_driver);
+}
+
+module_init(ss_usb_init);
+module_exit(ss_usb_exit);
+
+MODULE_AUTHOR("Maximilian Schneider <mws@schneidersoft.net>");
+MODULE_DESCRIPTION("socket CAN device driver for "
+	"SchneiderSoft USB2.0 to CAN interfaces.");
+MODULE_LICENSE("GPL v2");
+
+
+
+
+
+static int ss_make_candev( struct ss_can **out,
+	unsigned int channel,
+	struct usb_interface *intf )
+{
+	struct ss_can *dev;
+	struct device_bt_const bt_const;
+	struct net_device *netdev;
+	int rc;
+	/* fetch bit timing constants */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BT_CONST,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			channel,
+			0,
+			&bt_const,
+			sizeof(bt_const),
+			1000);
+
+	if( rc<0 ){
+		dev_err(&intf->dev,
+			"ss_usb: Couldn't get bit timing const for channel %d\n",
+			rc);
+		return rc;
+	}
+
+	printk(KERN_INFO "fclock: %d\n",bt_const.fclk_can);
+
+	/* create netdev */
+	netdev = alloc_candev(sizeof(struct ss_can),MAX_TX_URBS);
+	if( !netdev ){
+		dev_err(&intf->dev, "ss_usb: Couldn't alloc candev\n");
+		return -ENOMEM;
+	}
+
+	dev = netdev_priv(netdev);
+
+	netdev->netdev_ops = &ss_usb_netdev_ops;
+
+	netdev->flags |= IFF_ECHO; /* we support full roundtrip echo */
+
+	/* dev settup */
+	dev->bt_const = (struct can_bittiming_const){
+		.name = "ss_usb",
+		.tseg1_min = bt_const.tseg1_min,
+		.tseg1_max = bt_const.tseg1_max,
+		.tseg2_min = bt_const.tseg2_min,
+		.tseg2_max = bt_const.tseg2_max,
+		.sjw_max = bt_const.sjw_max,
+		.brp_min = bt_const.brp_min,
+		.brp_max = bt_const.brp_max,
+		.brp_inc = bt_const.brp_inc
+	};
+
+	dev->udev = interface_to_usbdev(intf);
+	dev->iface = intf;
+	dev->netdev = netdev;
+	dev->channel = channel;
+
+	init_usb_anchor(&dev->tx_submitted);
+	atomic_set(&dev->active_tx_urbs, 0);
+	for(rc=0;rc<MAX_TX_URBS;rc++){
+		dev->tx_context[rc].dev = dev;
+		dev->tx_context[rc].echo_id = MAX_TX_URBS;
+	}
+
+	/* can settup */
+	dev->can.state = CAN_STATE_STOPPED;
+	dev->can.clock.freq = bt_const.fclk_can;
+	dev->can.bittiming_const = &dev->bt_const;
+	dev->can.do_set_bittiming = ss_usb_set_bittiming;
+	dev->can.do_set_mode = ss_usb_set_mode;
+	dev->can.do_get_state = ss_usb_get_state;
+	dev->can.do_get_berr_counter = ss_usb_get_berr_counter;
+	dev->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
+
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
+	rc = register_candev(dev->netdev);
+	if( rc ){
+		free_candev(dev->netdev);
+		dev_err(&intf->dev, "ss_usb: Couldn't register candev\n");
+		return(rc);
+	}
+
+	*out = dev;
+	return(0);
+}
+
+static void ss_destroy_candev( struct ss_can *dev )
+{
+	unregister_candev(dev->netdev);
+	free_candev(dev->netdev);
+}
+
-- 
1.7.10.4

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2444 bytes --]

On 04/24/2013 05:48 PM, Max S. wrote:
>> Please use "./scripts/checkpatch.pl" from the kernel and fix all
>> coding style errors. Then repost :).

Please check your patch with ./scripts/checkpatch.pl, fix the errors,
then repost.

>> Can you explain how the "struct device_config" works.
>>
> struct __packed device_config {
> 	u16 byte_order;
> 
> 	u32 hf_size;	/* size of a host frame */
> 	u32 hf_can_id;
> 	u32 hf_can_dlc;
> 	u32 hf_data;
> 	u32 hf_echo_id;
> 	u32 hf_channel;
> } conf;
> 
> conf = (struct device_config){
> 	.byte_order = 0xbeef,
> 	.hf_size = sizeof(struct ss_host_frame),
> 	.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
> 	.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
> 	.hf_data = offsetof(struct ss_host_frame, frame.data),
> 	.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
> 	.hf_channel = offsetof(struct ss_host_frame, channel),
> };
> 
> ... send conf to device, packed that should be 26 bytes.
> 
> The device uses byte_order which will appear as either 0xbeef or 0xefbe
> to convert the other members to device byte order. (and to convert all
> future messages)

Use a u32 here instead of an u16, this way the struct is properly aligned.

> The device can then tell how the host prefers its can frames formatted.
> The idea is that the struct device_config holds enough data to allow the
> device to construct a struct ss_host_frame in host format, including
> padding & byte order.

Nice.

> since a struct ss_host_frame is simply a wrapper that includes a struct
> can_frame, when a message arrives from the device, the host can copy the
> struct can frame member without modification from the struct
> ss_host_frame into the skb.
> 
> assumptions that are made:
> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
> and can/error.h don't change, as they are used by the device to
> construct the can_id field.

You cannot rely in your firmware, that the struct can_frame and
CAN_*_FLAG doesn't change. Please define your own struct.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Oliver Hartkopp]

On 24.04.2013 18:07, Marc Kleine-Budde wrote:

> On 04/24/2013 05:48 PM, Max S. wrote:


>> The device can then tell how the host prefers its can frames formatted.
>> The idea is that the struct device_config holds enough data to allow the
>> device to construct a struct ss_host_frame in host format, including
>> padding & byte order.
> 
> Nice.
> 
>> since a struct ss_host_frame is simply a wrapper that includes a struct
>> can_frame, when a message arrives from the device, the host can copy the
>> struct can frame member without modification from the struct
>> ss_host_frame into the skb.
>>
>> assumptions that are made:
>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
>> and can/error.h don't change, as they are used by the device to
>> construct the can_id field.
> 
> You cannot rely in your firmware, that the struct can_frame and
> CAN_*_FLAG doesn't change. Please define your own struct.
> 


Hm - i really appreciate the memcopy-only approach which can cope with the
host byte order directly. This is a real improvement on the host side.

The struct can_frame and the error message content is official Kernel API and
therefore can be assumed to be fix.

Btw. extensions like the introduction of the struct canfd_frame can always
take place. But this comes along with really new functionality then.

I would like to continue with the memcopy approach - but we probably need some
frame type identifier to be able to handle different host CAN frames, like
struct can_frame and struct canfd_frame, ...

Regards,
Oliver

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
>>> assumptions that are made:
>>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
>>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
>>> and can/error.h don't change, as they are used by the device to
>>> construct the can_id field.
>>
>> You cannot rely in your firmware, that the struct can_frame and
>> CAN_*_FLAG doesn't change. Please define your own struct.
>>
> 
> 
> Hm - i really appreciate the memcopy-only approach which can cope with the
> host byte order directly. This is a real improvement on the host side.
> 
> The struct can_frame and the error message content is official Kernel API and
> therefore can be assumed to be fix.

Yes but no. The struct can_frame to the _userspace_ is official Kernel
API/ABI, but within the kernel the is no stable API (see
stable_api_nonsense.txt). But if we go this way we should make compile
time checks, so that the compilation breaks if the struct can_frame changes.

> Btw. extensions like the introduction of the struct canfd_frame can always
> take place. But this comes along with really new functionality then.

> I would like to continue with the memcopy approach - but we probably need some
> frame type identifier to be able to handle different host CAN frames, like
> struct can_frame and struct canfd_frame, ...

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Max S.]

On Wed, 2013-04-24 at 23:24 +0200, Marc Kleine-Budde wrote:
> On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
> >>> assumptions that are made:
> >>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
> >>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
> >>> and can/error.h don't change, as they are used by the device to
> >>> construct the can_id field.
> >>
> >> You cannot rely in your firmware, that the struct can_frame and
> >> CAN_*_FLAG doesn't change. Please define your own struct.
> >>
> > 
> > 
> > Hm - i really appreciate the memcopy-only approach which can cope with the
> > host byte order directly. This is a real improvement on the host side.
> > 
> > The struct can_frame and the error message content is official Kernel API and
> > therefore can be assumed to be fix.
> 
> Yes but no. The struct can_frame to the _userspace_ is official Kernel
> API/ABI, but within the kernel the is no stable API (see
> stable_api_nonsense.txt). But if we go this way we should make compile
> time checks, so that the compilation breaks if the struct can_frame changes.

ok. tell me if i understood correctly.
A user-space program can safely assume:
sizeof((struct can_frame *)NULL)->can_id) == 4
and:
CAN_EFF_FLAG == 0x80000000U 

but a kernel-space driver can not?

If I cannot make sufficient assumptions about the struct can_frame and
CAN_*_FLAGs It will make the code harder to maintain in the long run.
the speed gains derived from a direct memcpy/assignment will later be
eaten up by the code designed to convert the old struct can_frame
received from the device to the new struct can_frame found in the
kernel.

The question is in the end, where should i draw the line.

One option would be to define a new struct ss_frame to standardize
communication between device & host, but still use the device_config to
allow the changing of byte order.
The driver would then need to do three assignments to rearrange padding,
and adjust any difference in the id flags:

* arrange for correct byte order beforehand...

can_frame->can_id  = CONVERT_ME_FLAGS(ss_frame->can_id);
can_frame->can_dlc = ss_frame->can_dlc;
can_frame->data    = ss_frame->data;

In this case the 'line' would be explicitly between the struct ss_frame
and the struct can_frame not floating somewhere vague between the kernel
and firmware...
Naturally you will no longer be able to simply:
*can_frame = frame_wrapper->can_frame;
As i am doing now.

What do you think?
Max Schneider.

Re: Usb to can driver

[Oliver Hartkopp]

On 26.04.2013 01:35, Max S. wrote:

> On Wed, 2013-04-24 at 23:24 +0200, Marc Kleine-Budde wrote:
>> On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
>>>>> assumptions that are made:
>>>>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
>>>>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
>>>>> and can/error.h don't change, as they are used by the device to
>>>>> construct the can_id field.
>>>>
>>>> You cannot rely in your firmware, that the struct can_frame and
>>>> CAN_*_FLAG doesn't change. Please define your own struct.
>>>>
>>>
>>>
>>> Hm - i really appreciate the memcopy-only approach which can cope with the
>>> host byte order directly. This is a real improvement on the host side.
>>>
>>> The struct can_frame and the error message content is official Kernel API and
>>> therefore can be assumed to be fix.
>>
>> Yes but no. The struct can_frame to the _userspace_ is official Kernel
>> API/ABI, but within the kernel the is no stable API (see
>> stable_api_nonsense.txt). But if we go this way we should make compile
>> time checks, so that the compilation breaks if the struct can_frame changes.
> 
> ok. tell me if i understood correctly.
> A user-space program can safely assume:
> sizeof((struct can_frame *)NULL)->can_id) == 4
> and:
> CAN_EFF_FLAG == 0x80000000U 
> 
> but a kernel-space driver can not?


The point is, that exposed data structures to the userspace have to be fixed.

Data structures in the kernel might be changed, when it's needed.
BUT: The interface between the network layer (PF_CAN / PF_PACKET) and the
CAN netdevice driver is the transfer of a struct can(fd)_frame.

This interface is not 'guaranteed' to be fix but it will definitely depend on
the exposed data structures for the CAN frame.

> 
> If I cannot make sufficient assumptions about the struct can_frame and
> CAN_*_FLAGs It will make the code harder to maintain in the long run.
> the speed gains derived from a direct memcpy/assignment will later be
> eaten up by the code designed to convert the old struct can_frame
> received from the device to the new struct can_frame found in the
> kernel.
> 
> The question is in the end, where should i draw the line.
> 
> One option would be to define a new struct ss_frame to standardize
> communication between device & host, but still use the device_config to
> allow the changing of byte order.
> The driver would then need to do three assignments to rearrange padding,
> and adjust any difference in the id flags:
> 
> * arrange for correct byte order beforehand...
> 
> can_frame->can_id  = CONVERT_ME_FLAGS(ss_frame->can_id);
> can_frame->can_dlc = ss_frame->can_dlc;
> can_frame->data    = ss_frame->data;
> 
> In this case the 'line' would be explicitly between the struct ss_frame
> and the struct can_frame not floating somewhere vague between the kernel
> and firmware...
> Naturally you will no longer be able to simply:
> *can_frame = frame_wrapper->can_frame;
> As i am doing now.
> 
> What do you think?


Indeed we had the same problem when introducing the CAN FD support last year.

The solution was to define a 'backward compatible' struct canfd_frame, that
allows to transport either a CAN frame and a CAN FD frame in the same data
structure.

See picture in:

http://can-newsletter.org/engineering/standardization/nr_stand_can-fd_linux3.6_120703/ 

The information whether it is a CAN FD frame or not is defined in the length
of the data structure. This structure length information would need to be
transported apart from the struct can(fd)_frame itself.

IMHO using the struct can(fd)_frame can considered to be stable.
The 'stable API nonsense' mainly focuses kernel internal function definitions
and structures that may change in names and variables - or be removed at all.

Struct can(fd)_frame is a CAN specific data structure. And CAN is an
ISO standard that even Linux hackers are not able to change :-)

Regards,
Oliver

Re: Usb to can driver

[Kurt Van Dijck]

On Fri, Apr 26, 2013 at 07:25:58AM +0200, Oliver Hartkopp wrote:
> On 26.04.2013 01:35, Max S. wrote:
> 
> > On Wed, 2013-04-24 at 23:24 +0200, Marc Kleine-Budde wrote:
> >> On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
> >>>>> assumptions that are made:
> >>>>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
> >>>>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
> >>>>> and can/error.h don't change, as they are used by the device to
> >>>>> construct the can_id field.
> 
[...]
> IMHO using the struct can(fd)_frame can considered to be stable.
> The 'stable API nonsense' mainly focuses kernel internal function definitions
> and structures that may change in names and variables - or be removed at all.
> 
> Struct can(fd)_frame is a CAN specific data structure. And CAN is an
> ISO standard that even Linux hackers are not able to change :-)

modifying struct can(fd)_frame serves not purpose.
If you drop something, it's insufficient to deal with the ISO standards.
If you add something, it's add overhead.
If you modify the order of the members, you'll still be very close to the standard,
but what's the use?

So I agree with Oliver that these structs are very likely to stay stable.

Kurt

-- 
Kurt Van Dijck
GRAMMER EiA ELECTRONICS
http://www.eia.be
kurt.van.dijck@eia.be
+32-38708534

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 4355 bytes --]

On 04/26/2013 01:35 AM, Max S. wrote:
> On Wed, 2013-04-24 at 23:24 +0200, Marc Kleine-Budde wrote:
>> On 04/24/2013 07:40 PM, Oliver Hartkopp wrote:
>>>>> assumptions that are made:
>>>>> * The sizes of the struct can_frame members are id:u32 dlc:u8 data:u64 .
>>>>> * It is also assumed that existing defines like CAN_EFF_FLAG in can.h
>>>>> and can/error.h don't change, as they are used by the device to
>>>>> construct the can_id field.
>>>>
>>>> You cannot rely in your firmware, that the struct can_frame and
>>>> CAN_*_FLAG doesn't change. Please define your own struct.
>>>>
>>>
>>>
>>> Hm - i really appreciate the memcopy-only approach which can cope with the
>>> host byte order directly. This is a real improvement on the host side.
>>>
>>> The struct can_frame and the error message content is official Kernel API and
>>> therefore can be assumed to be fix.
>>
>> Yes but no. The struct can_frame to the _userspace_ is official Kernel
>> API/ABI, but within the kernel the is no stable API (see
>> stable_api_nonsense.txt). But if we go this way we should make compile
>> time checks, so that the compilation breaks if the struct can_frame changes.
> 
> ok. tell me if i understood correctly.
> A user-space program can safely assume:
> sizeof((struct can_frame *)NULL)->can_id) == 4
> and:
> CAN_EFF_FLAG == 0x80000000U 
> 
> but a kernel-space driver can not?

Yes, in the Kernel we don't have a stable ABI and the API can change,
too. However, if someone changes the API he's not allowed to introduce
any breakage, but that's not so easy if she/he has to recompile your
firmware.

> If I cannot make sufficient assumptions about the struct can_frame and
> CAN_*_FLAGs It will make the code harder to maintain in the long run.

From my point of view it's just the other way round. It's all about
layers/interfaces here. You define how your hardware is accessed, once
the Linux driver and the CAN firmware is stable that interface is carved
in stone. As long as the interface doesn't change everyone can make
changes inside their layers without breaking the whole system (modulo
bugs). If the interface turns out to be outdated, you can create a new one.

Having thought so far, it's a good idea to add a interface version and a
feature bitfield (u32) register. These two registers should be at a
fixed address. In USB terms this probably translates into some initial
USB message with two variables at a fixed offset in that message ;)

> the speed gains derived from a direct memcpy/assignment will later be
> eaten up by the code designed to convert the old struct can_frame
> received from the device to the new struct can_frame found in the
> kernel.

Your device is attached via USB, so I doubt there is any measurable
difference between a memcpy the whole struct can_frame and a member by
member copy.

> The question is in the end, where should i draw the line.
> 
> One option would be to define a new struct ss_frame to standardize
> communication between device & host, but still use the device_config to
> allow the changing of byte order.

Sounds like a good idea.

> The driver would then need to do three assignments to rearrange padding,
> and adjust any difference in the id flags:
> 
> * arrange for correct byte order beforehand...

If you tell the USB device your byte order with device_config there's no
need to change the byte order in each frame, am I right?

> can_frame->can_id  = CONVERT_ME_FLAGS(ss_frame->can_id);

Assuming you encode the extended frame identifier in can_id, too. But's
that is no problem.

> can_frame->can_dlc = ss_frame->can_dlc;
> can_frame->data    = ss_frame->data;
> 
> In this case the 'line' would be explicitly between the struct ss_frame
> and the struct can_frame not floating somewhere vague between the kernel
> and firmware...

Yes.

> Naturally you will no longer be able to simply:
> *can_frame = frame_wrapper->can_frame;
> As i am doing now.

Yes

> What do you think?

We're getting there!

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Max S.]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Hello Marc,

On Wed, 2013-04-24 at 18:07 +0200, Marc Kleine-Budde wrote:
> On 04/24/2013 05:48 PM, Max S. wrote:
> >> Please use "./scripts/checkpatch.pl" from the kernel and fix all
> >> coding style errors. Then repost :).
> 
> Please check your patch with ./scripts/checkpatch.pl, fix the errors,
> then repost.

I think i got the patch correct this time. Third times the charm?

> Use a u32 here instead of an u16, this way the struct is properly aligned.

Ok. Will do.

Max Schneider.

[-- Attachment #2: 0001-Added-ss_usb-source.patch --]
[-- Type: text/x-patch, Size: 23091 bytes --]

From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
From: Maximilian Schneider <max@schneidersoft.net>
Date: Wed, 24 Apr 2013 21:03:31 +0000
Subject: [PATCH] Added ss_usb source.


Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
---
 drivers/net/can/usb/Makefile |    1 +
 drivers/net/can/usb/ss_usb.c |  944 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 945 insertions(+)
 create mode 100644 drivers/net/can/usb/ss_usb.c

diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index becef46..c341a1b 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -7,5 +7,6 @@ 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_8DEV_USB) += usb_8dev.o
+obj-$(CONFIG_CAN_SS_USB) += ss_usb.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/ss_usb.c b/drivers/net/can/usb/ss_usb.c
new file mode 100644
index 0000000..69710ff
--- /dev/null
+++ b/drivers/net/can/usb/ss_usb.c
@@ -0,0 +1,944 @@
+#include <linux/init.h>
+#include <linux/signal.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>
+
+/* Device specific constants */
+#define USB_SSUSB_VENDOR_ID         0xbeef
+#define USB_SSUSB_AT32_PRODUCT_ID   0x1234
+
+#define SS_USB_ENDPOINT_IN          1
+#define SS_USB_ENDPOINT_OUT         2
+
+#define SS_USB_BREQ_HOST_FORMAT     0
+#define SS_USB_BREQ_BITTIMING       1
+#define SS_USB_BREQ_MODE            2
+#define SS_USB_BREQ_BERR            3
+#define SS_USB_BREQ_BT_CONST        4
+
+
+#define SS_CAN_MODE_RESET           0
+#define SS_CAN_MODE_START           1
+
+#define SS_CAN_STATE_ERROR_ACTIVE   0
+#define SS_CAN_STATE_ERROR_WARNING  1
+#define SS_CAN_STATE_ERROR_PASSIVE  2
+#define SS_CAN_STATE_BUS_OFF        3
+#define SS_CAN_STATE_STOPPED        4
+#define SS_CAN_STATE_SLEEPING       5
+
+/* data types passed between host and device */
+struct __packed device_config {
+	u16 byte_order;
+
+	u32 hf_size;	/* size of a host frame */
+	u32 hf_can_id;
+	u32 hf_can_dlc;
+	u32 hf_data;
+	u32 hf_echo_id;
+	u32 hf_channel;
+};
+
+struct __packed device_mode {
+	u8  mode;
+};
+
+struct __packed device_state {
+	u8  state;
+	u32 rxerr;
+	u32 txerr;
+};
+
+struct __packed device_bittiming {
+	u32 prop_seg;
+	u32 phase_seg1;
+	u32 phase_seg2;
+	u32 sjw;
+	u32 brp;
+};
+
+struct __packed device_bt_const {
+	u32 fclk_can;
+	u32 tseg1_min;
+	u32 tseg1_max;
+	u32 tseg2_min;
+	u32 tseg2_max;
+	u32 sjw_max;
+	u32 brp_min;
+	u32 brp_max;
+	u32 brp_inc;
+};
+
+struct ss_host_frame {
+	u8  channel;
+	u32 echo_id;
+	struct can_frame frame;
+};
+
+#define MAX_TX_URBS 10
+#define MAX_RX_URBS 10
+
+#define MAX_INTF 2
+
+struct ss_tx_context {
+	struct ss_can *dev;
+	unsigned int echo_id;
+};
+
+struct ss_can {
+	struct can_priv can; /* must be the first member */
+
+	struct ss_usb *parent;
+
+	struct net_device *netdev;
+	struct usb_device *udev;
+	struct usb_interface *iface;
+
+	struct can_bittiming_const bt_const;
+	unsigned int channel;	/* channel number */
+
+	struct ss_tx_context tx_context[MAX_TX_URBS];
+
+	struct usb_anchor tx_submitted;
+	atomic_t active_tx_urbs;
+
+	int open_time;
+};
+
+/* usb interface struct */
+struct ss_usb {
+	struct ss_can *canch[MAX_INTF];
+
+	struct usb_anchor rx_submitted;
+
+	atomic_t active_channels;
+};
+
+/* 'allocate' a tx context.
+ * returns a valid tx context or NULL if there is no space.
+ */
+static struct ss_tx_context *ss_alloc_tx_context(struct ss_can *dev)
+{
+	int ii = 0;
+	for (; ii < MAX_TX_URBS; ii++)
+		if (dev->tx_context[ii].echo_id == MAX_TX_URBS) {
+			dev->tx_context[ii].echo_id = ii;
+			return &dev->tx_context[ii];
+		}
+	return NULL;
+}
+
+/* releases a tx context
+ */
+static void ss_free_tx_context(struct ss_tx_context *txc)
+{
+	txc->echo_id = MAX_TX_URBS;
+}
+
+/* Get a tx context by id.
+ */
+static struct ss_tx_context *ss_get_tx_context(
+	struct ss_can *dev,
+	unsigned int id)
+{
+	if (id < MAX_TX_URBS)
+		if (dev->tx_context[id].echo_id == id)
+			return &dev->tx_context[id];
+
+	return NULL;
+}
+
+/* Get a tx contexts id.
+ */
+static unsigned int ss_tx_context_id(struct ss_tx_context *txc)
+{
+	return txc->echo_id;
+}
+
+#define BUFFER_SIZE sizeof(struct ss_host_frame)
+
+/**************************** USB CALLBACKS **************************/
+static void ss_usb_recieve_bulk_callback(struct urb *urb)
+{
+	struct ss_usb *usbcan = urb->context;
+	struct ss_can *dev;
+	struct net_device *netdev;
+	int rc;
+	struct net_device_stats *stats;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	struct ss_host_frame *hf = urb->transfer_buffer;
+	struct ss_tx_context *txc;
+
+	BUG_ON(!usbcan);
+
+	switch (urb->status) {
+	case 0: /* success */
+		break;
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+	default:
+		/* printk(KERN_INFO "Rx URB aborted (%d)\n", urb->status); */
+		/* do not resubmit aborted urbs. eg: when device goes down */
+		return;
+	}
+
+	/* device reports out of range channel id */
+	if (hf->channel >= MAX_INTF)
+		/* printk(KERN_INFO "%d] strange channel #\n", hf->channel); */
+		goto resubmit_urb;
+
+	/* device reports bad channel id */
+	dev = usbcan->canch[hf->channel];
+
+	BUG_ON(!dev);
+
+	netdev = dev->netdev;
+	BUG_ON(!netdev);
+
+	if (!netif_device_present(netdev))
+		return;
+
+	stats = &dev->netdev->stats;
+
+	if (hf->echo_id == 0) { /* normal */
+		netdev_info(netdev, "%d] got msg\n", hf->channel);
+		skb = alloc_can_skb(dev->netdev, &cf);
+		if (skb == NULL)
+			return;
+
+		*cf = hf->frame;
+		cf->can_dlc = get_can_dlc(hf->frame.can_dlc);
+
+		netif_rx(skb);
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	} else {		/* echo_id = hf->echo_id-1 */
+		txc = ss_get_tx_context(dev, hf->echo_id-1);
+
+		/* bad devices send bad echo_ids. */
+		if (!txc) {
+			netdev_info(netdev, "%d] bad echo %d\n",
+				    hf->channel, hf->echo_id);
+			goto resubmit_urb;
+		} else {
+			netdev_info(netdev, "%d] got echo %d\n",
+				    hf->channel, hf->echo_id);
+		}
+
+		can_get_echo_skb(netdev, ss_tx_context_id(txc));
+
+		ss_free_tx_context(txc);
+
+		netdev->stats.tx_packets++;
+		/* FIXME: report dlc: stats->tx_bytes += cf->can_dlc; */
+
+		netdev->trans_start = jiffies;
+
+		if (netif_queue_stopped(netdev))
+			netif_wake_queue(netdev);
+	}
+
+
+resubmit_urb:
+	/* do i really need to refill? the urb should still be valid:
+	usb_fill_bulk_urb(urb,
+	dev->udev,
+	usb_rcvbulkpipe(dev->udev,
+	SS_USB_ENDPOINT_IN(dev->iface->altsetting[0].desc.bInterfaceNumber)),
+	urb->transfer_buffer,
+	BUFFER_SIZE, ss_usb_recieve_bulk_callback,
+	dev);
+	*/
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+
+/* How to best handle errore?
+	if (rc == -ENODEV) {
+		printk(KERN_INFO "resubmit error -ENODEV\n");
+		FIXME: necessary?
+		netif_device_detach(netdev);
+
+	} else if (rc) {
+		printk(KERN_INFO "resubmit error %d\n", rc);
+	}
+*/
+}
+
+static void ss_usb_xmit_callback(struct urb *urb)
+{
+	struct ss_tx_context *txc = urb->context;
+	struct ss_can *dev;
+	struct net_device *netdev;
+
+	BUG_ON(!txc);
+
+	dev = txc->dev;
+	BUG_ON(!dev);
+
+	netdev = dev->netdev;
+	BUG_ON(!netdev);
+
+	if (urb->status) {
+		dev_info(netdev->dev.parent, "%d] xmit err %d\n",
+			dev->channel, txc->echo_id+1);
+	} else {
+		dev_info(netdev->dev.parent, "%d] xmit ok  %d\n",
+			dev->channel, txc->echo_id+1);
+	}
+
+	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;
+
+	/* The urb never made it to the device. Drop this packet. */
+	if (urb->status) {
+		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
+			urb->status);
+		can_free_echo_skb(netdev, ss_tx_context_id(txc));
+		ss_free_tx_context(txc);
+		netdev->stats.tx_dropped++;
+	}
+
+	if (netif_queue_stopped(netdev))
+		netif_wake_queue(netdev);
+}
+
+/****************************** UBS CAN ******************************/
+static int ss_usb_set_bittiming(struct net_device *netdev)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct can_bittiming *bt = &dev->can.bittiming;
+	struct usb_interface *intf = dev->iface;
+	int rc;
+
+	struct __packed device_bittiming dbt = {
+		.prop_seg = bt->prop_seg,
+		.phase_seg1 = bt->phase_seg1,
+		.phase_seg2 = bt->phase_seg2,
+		.sjw = bt->sjw,
+		.brp = bt->brp
+	};
+
+/*	printk(KERN_INFO "CH: %d\n", dev->channel);
+	printk(KERN_INFO "PRS %d\n", bt->prop_seg);
+	printk(KERN_INFO "PH1 %d\n", bt->phase_seg1);
+	printk(KERN_INFO "PH2 %d\n", bt->phase_seg2);
+	printk(KERN_INFO "SJW %d\n", bt->sjw);
+	printk(KERN_INFO "BRP %d\n", bt->brp);
+*/
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BITTIMING,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dbt,
+			sizeof(dbt),
+			1000);
+	if (rc < 0)
+		dev_warn(netdev->dev.parent, "Couldn't set bittimings %d", rc);
+
+	return rc;
+}
+
+static int ss_usb_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_mode dm;
+	int rc;
+	dev_warn(netdev->dev.parent, "ss can mode %d\n", mode);
+
+	if (!dev->open_time)
+		return -EINVAL;
+
+
+	switch (mode) {
+	case CAN_MODE_START:
+		dm.mode = SS_CAN_MODE_RESET;
+
+		rc = usb_control_msg(interface_to_usbdev(intf),
+				usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+				SS_USB_BREQ_MODE,
+				USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+				dev->channel,
+				0,
+				&dm,
+				sizeof(dm),
+				1000);
+		if (rc < 0) {
+			dev_warn(netdev->dev.parent, "Couldn't start device %d",
+				rc);
+			return rc;
+		}
+
+		if (netif_queue_stopped(netdev))
+			netif_wake_queue(netdev);
+		break;
+	case CAN_MODE_STOP:
+	case CAN_MODE_SLEEP:
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int ss_usb_get_state(const struct net_device *netdev,
+	enum can_state *state)
+{
+
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_state dstate;
+	int rc;
+	dev_warn(netdev->dev.parent, "ss can state");
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dstate,
+			sizeof(dstate),
+			1000);
+	if (rc < 0) {
+		dev_warn(netdev->dev.parent, "Couldn't get state %d", rc);
+		return rc;
+	}
+
+	netdev_info(netdev, "state: %d\n", dstate.state);
+
+	switch (dstate.state) {
+	/* RX/TX error count < 96 */
+	case SS_CAN_STATE_ERROR_ACTIVE:
+		*state = CAN_STATE_ERROR_ACTIVE;
+		return 0;
+	/* RX/TX error count < 128 */
+	case SS_CAN_STATE_ERROR_WARNING:
+		*state = CAN_STATE_ERROR_WARNING;
+		return 0;
+	/* RX/TX error count < 256 */
+	case SS_CAN_STATE_ERROR_PASSIVE:
+		*state = CAN_STATE_ERROR_PASSIVE;
+		return 0;
+	/* RX/TX error count >= 256 */
+	case SS_CAN_STATE_BUS_OFF:
+		*state = CAN_STATE_BUS_OFF;
+		return 0;
+	/* Device is stopped */
+	case SS_CAN_STATE_STOPPED:
+		*state = CAN_STATE_STOPPED;
+		return 0;
+	/* Device is sleeping */
+	case SS_CAN_STATE_SLEEPING:
+		*state = CAN_STATE_SLEEPING;
+		return 0;
+	}
+
+	return -ENOTSUPP;
+}
+
+static int ss_usb_get_berr_counter(const struct net_device *netdev,
+	struct can_berr_counter *bec)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	struct device_state dstate;
+	int rc;
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dstate,
+			sizeof(dstate),
+			1000);
+	if (rc < 0) {
+		dev_warn(netdev->dev.parent, "Couldn't get error counters %d",
+			rc);
+		return rc;
+	}
+
+	dev_info(netdev->dev.parent, "txerr: %d rxerr: %d\n",
+		dstate.txerr, dstate.rxerr);
+
+	bec->txerr = dstate.txerr;
+	bec->rxerr = dstate.rxerr;
+
+	return 0;
+}
+
+static netdev_tx_t ss_can_start_xmit(struct sk_buff *skb,
+	struct net_device *netdev)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct net_device_stats *stats = &dev->netdev->stats;
+	struct urb *urb;
+	struct ss_host_frame *hf;
+	int rc;
+	struct ss_tx_context *txc;
+
+	if (can_dropped_invalid_skb(netdev, skb))
+		return NETDEV_TX_OK;
+
+	/* find an empty context to keep track of transmission */
+	txc = ss_alloc_tx_context(dev);
+	if (!txc) {
+		/* dev_kfree_skb(skb); */
+		return NETDEV_TX_BUSY;
+	}
+
+	/* create a URB, and a buffer for it */
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		ss_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		goto nomem;
+	}
+
+	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
+		&urb->transfer_dma);
+	if (!hf) {
+		ss_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		usb_free_urb(urb);
+		goto nomem;
+	}
+
+	hf->frame = *(struct can_frame *)skb->data;
+	hf->echo_id = ss_tx_context_id(txc)+1;
+	hf->channel = dev->channel;
+
+	usb_fill_bulk_urb(urb, dev->udev,
+			usb_sndbulkpipe(dev->udev, SS_USB_ENDPOINT_OUT),
+			hf,
+			sizeof(*hf),
+			ss_usb_xmit_callback,
+			txc);
+
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+	usb_anchor_urb(urb, &dev->tx_submitted);
+
+	can_put_echo_skb(skb, netdev, ss_tx_context_id(txc));
+
+	atomic_inc(&dev->active_tx_urbs);
+
+/*	printk(KERN_INFO "%d] xmit new %d\n",hf->channel,hf->echo_id); */
+
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+	if (unlikely(rc)) {			/* usb send failed */
+		dev_err(netdev->dev.parent, "usb_submit failed %d\n", rc);
+
+		can_free_echo_skb(netdev, ss_tx_context_id(txc));
+		ss_free_tx_context(txc);
+
+		usb_unanchor_urb(urb);
+		usb_free_coherent(dev->udev,
+			sizeof(*hf),
+			hf,
+			urb->transfer_dma);
+
+		dev_kfree_skb(skb);
+
+		atomic_dec(&dev->active_tx_urbs);
+
+		if (rc == -ENODEV) {
+			netif_device_detach(netdev);
+		} else {
+			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", rc);
+
+			stats->tx_dropped++;
+		}
+	} else {
+		netdev->trans_start = jiffies;
+
+		/* Slow down tx path */
+		if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS)
+			netif_stop_queue(netdev);
+	}
+
+	/* let usb core take care of this urb */
+	usb_free_urb(urb);
+
+	return NETDEV_TX_OK;
+
+nomem:
+	dev_kfree_skb(skb);
+	stats->tx_dropped++;
+	return NETDEV_TX_OK;
+}
+
+static int ss_can_open(struct net_device *netdev)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct ss_usb *parent = dev->parent;
+	struct device_mode dm;
+	int rc, i;
+
+	/* common open */
+	rc = open_candev(netdev);
+	if (rc)
+		return rc;
+
+	if (atomic_add_return(1, &parent->active_channels) == 1) {
+		for (i = 0; i < MAX_RX_URBS; i++) {
+			struct urb *urb = NULL;
+			u8 *buf = NULL;
+
+			/* alloc rx urb */
+			urb = usb_alloc_urb(0, GFP_KERNEL);
+			if (!urb) {
+				dev_err(netdev->dev.parent,
+					"No memory left for URBs\n");
+				return -ENOMEM;
+			}
+
+			/* alloc rx buffer */
+			/* FIXME: free this memory */
+			buf = usb_alloc_coherent(dev->udev,
+				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;
+			}
+
+
+			/* fill, anchor, and submit rx urb */
+			usb_fill_bulk_urb(urb,
+					dev->udev,
+					usb_rcvbulkpipe(dev->udev,
+						SS_USB_ENDPOINT_IN),
+					buf,
+					BUFFER_SIZE,
+					ss_usb_recieve_bulk_callback,
+					parent
+					);
+			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+			usb_anchor_urb(urb, &parent->rx_submitted);
+
+			rc = usb_submit_urb(urb, GFP_KERNEL);
+			if (rc) {
+				if (rc == -ENODEV)
+					netif_device_detach(dev->netdev);
+
+				/* FIXME: free rx buffer/urb maybe? */
+				usb_unanchor_urb(urb);
+				break;
+			}
+
+			/* Drop reference,
+			 * USB core will take care of freeing it
+			 */
+			usb_free_urb(urb);
+		}
+	}
+
+	/* finally start device */
+	dm.mode = SS_CAN_MODE_START;
+	rc = usb_control_msg(interface_to_usbdev(dev->iface),
+			usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),
+			SS_USB_BREQ_MODE,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			&dm,
+			sizeof(dm),
+			1000);
+
+	if (rc < 0)
+		dev_warn(netdev->dev.parent, "Couldn't start device %d", rc);
+
+	dev->open_time = jiffies;
+
+	netif_start_queue(netdev);
+
+	netdev_info(netdev, "ss can close\n");
+	return 0;
+}
+
+/* think: ip link set canx down */
+static int ss_can_close(struct net_device *netdev)
+{
+	struct ss_can *dev = netdev_priv(netdev);
+	struct ss_usb *parent = dev->parent;
+
+	netif_stop_queue(netdev);
+
+	/* Stop polling */
+	if (atomic_dec_and_test(&parent->active_channels)) {
+		usb_kill_anchored_urbs(&parent->rx_submitted);
+		/* FIXME: Set CAN controller to reset mode */
+	}
+
+	close_candev(netdev);
+
+	dev->open_time = 0;
+
+	netdev_info(netdev, "ss can close\n");
+	return 0;
+}
+
+/******************************** USB ********************************/
+
+static const struct net_device_ops ss_usb_netdev_ops = {
+	.ndo_open = ss_can_open,
+	.ndo_stop = ss_can_close,
+	.ndo_start_xmit = ss_can_start_xmit,
+};
+
+static int ss_make_candev(struct ss_can **dev, unsigned int channel,
+	struct usb_interface *intf);
+static void ss_destroy_candev(struct ss_can *dev);
+
+static int ss_usb_probe(struct usb_interface *intf,
+	const struct usb_device_id *id)
+{
+	struct ss_usb *dev;
+	int rc = -ENOMEM;
+	unsigned int icount, i;
+
+	struct device_config conf;
+
+	/* send host config */
+	conf = (struct device_config){
+		.byte_order = 0xbeef,
+		.hf_size = sizeof(struct ss_host_frame),
+		.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
+		.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
+		.hf_data = offsetof(struct ss_host_frame, frame.data),
+		.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
+		.hf_channel = offsetof(struct ss_host_frame, channel),
+	};
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_HOST_FORMAT,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			1,
+			intf->altsetting[0].desc.bInterfaceNumber,
+			&conf,
+			sizeof(conf),
+			1000);
+
+	if (rc < 0) {
+		dev_err(&intf->dev, "ss_usb: Couldn't send data format %d\n",
+			rc);
+		return rc;
+	}
+
+	/* FIXME: fetch interface count and global settings etc.. */
+	icount = 2;
+
+	if (icount > MAX_INTF) {
+		dev_err(&intf->dev,
+			"ss_usb: Cannot handle more that %d Can interfaces\n",
+			MAX_INTF);
+		return -EINVAL;
+	}
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	init_usb_anchor(&dev->rx_submitted);
+
+	atomic_set(&dev->active_channels, 0);
+
+	usb_set_intfdata(intf, dev);
+
+	for (i = 0; i < icount; i++) {
+		rc = ss_make_candev(&dev->canch[i], i, intf);
+		if (rc) {
+			icount = i;
+			for (i = 0; i < icount; i++) {
+				ss_destroy_candev(dev->canch[i]);
+				dev->canch[icount] = NULL;
+			}
+			return rc;
+		}
+		dev->canch[i]->parent = dev;
+	}
+
+	dev_info(&intf->dev, "ss usb probe\n");
+	return 0;
+}
+
+static void ss_usb_disconnect(struct usb_interface *intf)
+{
+	unsigned i;
+	struct ss_usb *dev = usb_get_intfdata(intf);
+	usb_set_intfdata(intf, NULL);
+
+	if (!dev) {
+		dev_info(&intf->dev, "ss usb disconnect (nodata)\n");
+		return;
+	}
+
+	for (i = 0; i < MAX_INTF; i++) {
+		struct ss_can *can = dev->canch[i];
+
+		if (!can)
+			continue;
+
+		unregister_netdev(can->netdev);
+		free_candev(can->netdev);
+
+		usb_kill_anchored_urbs(&can->tx_submitted);
+	}
+
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+
+	dev_info(&intf->dev, "ss usb disconnect\n");
+}
+
+MODULE_DEVICE_TABLE(usb, ss_usb_table);
+static struct usb_device_id ss_usb_table[] = {
+	{USB_DEVICE(USB_SSUSB_VENDOR_ID, USB_SSUSB_AT32_PRODUCT_ID)},
+	{} /* Terminating entry */
+};
+
+static struct usb_driver ss_usb_driver = {
+	.name       = "ss_usb",
+	.probe      = ss_usb_probe,
+	.disconnect = ss_usb_disconnect,
+	.id_table   = ss_usb_table,
+	/* FIXME: suspend, resume, etc... */
+};
+
+static int __init ss_usb_init(void)
+{
+	return usb_register(&ss_usb_driver);
+}
+
+static void __exit ss_usb_exit(void)
+{
+	usb_deregister(&ss_usb_driver);
+}
+
+module_init(ss_usb_init);
+module_exit(ss_usb_exit);
+
+MODULE_AUTHOR("Maximilian Schneider <mws@schneidersoft.net>");
+MODULE_DESCRIPTION(
+"socket CAN device driver for SchneiderSoft USB2.0 to CAN interfaces.");
+MODULE_LICENSE("GPL v2");
+
+
+
+
+
+static int ss_make_candev(struct ss_can **out,
+	unsigned int channel,
+	struct usb_interface *intf)
+{
+	struct ss_can *dev;
+	struct device_bt_const bt_const;
+	struct net_device *netdev;
+	int rc;
+	/* fetch bit timing constants */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			SS_USB_BREQ_BT_CONST,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			channel,
+			0,
+			&bt_const,
+			sizeof(bt_const),
+			1000);
+
+	if (rc < 0) {
+		dev_err(&intf->dev,
+			"ss_usb: Couldn't get bit timing const for channel %d\n",
+			rc);
+		return rc;
+	}
+
+	dev_info(&intf->dev, "fclock: %d\n", bt_const.fclk_can);
+
+	/* create netdev */
+	netdev = alloc_candev(sizeof(struct ss_can), MAX_TX_URBS);
+	if (!netdev) {
+		dev_err(&intf->dev, "ss_usb: Couldn't alloc candev\n");
+		return -ENOMEM;
+	}
+
+	dev = netdev_priv(netdev);
+
+	netdev->netdev_ops = &ss_usb_netdev_ops;
+
+	netdev->flags |= IFF_ECHO; /* we support full roundtrip echo */
+
+	/* dev settup */
+	dev->bt_const = (struct can_bittiming_const){
+		.name = "ss_usb",
+		.tseg1_min = bt_const.tseg1_min,
+		.tseg1_max = bt_const.tseg1_max,
+		.tseg2_min = bt_const.tseg2_min,
+		.tseg2_max = bt_const.tseg2_max,
+		.sjw_max = bt_const.sjw_max,
+		.brp_min = bt_const.brp_min,
+		.brp_max = bt_const.brp_max,
+		.brp_inc = bt_const.brp_inc
+	};
+
+	dev->udev = interface_to_usbdev(intf);
+	dev->iface = intf;
+	dev->netdev = netdev;
+	dev->channel = channel;
+
+	init_usb_anchor(&dev->tx_submitted);
+	atomic_set(&dev->active_tx_urbs, 0);
+	for (rc = 0; rc < MAX_TX_URBS; rc++) {
+		dev->tx_context[rc].dev = dev;
+		dev->tx_context[rc].echo_id = MAX_TX_URBS;
+	}
+
+	/* can settup */
+	dev->can.state = CAN_STATE_STOPPED;
+	dev->can.clock.freq = bt_const.fclk_can;
+	dev->can.bittiming_const = &dev->bt_const;
+	dev->can.do_set_bittiming = ss_usb_set_bittiming;
+	dev->can.do_set_mode = ss_usb_set_mode;
+	dev->can.do_get_state = ss_usb_get_state;
+	dev->can.do_get_berr_counter = ss_usb_get_berr_counter;
+	dev->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
+
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
+	rc = register_candev(dev->netdev);
+	if (rc) {
+		free_candev(dev->netdev);
+		dev_err(&intf->dev, "ss_usb: Couldn't register candev\n");
+		return rc;
+	}
+
+	*out = dev;
+	return 0;
+}
+
+static void ss_destroy_candev(struct ss_can *dev)
+{
+	unregister_candev(dev->netdev);
+	free_candev(dev->netdev);
+}
+
-- 
1.7.10.4

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 27709 bytes --]

On 04/24/2013 11:33 PM, Max S. wrote:
> From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
> From: Maximilian Schneider <max@schneidersoft.net>
> Date: Wed, 24 Apr 2013 21:03:31 +0000
> Subject: [PATCH] Added ss_usb source.

Comments inline.

> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
> ---
>  drivers/net/can/usb/Makefile |    1 +

You should add your driver to Kconfig, too.

>  drivers/net/can/usb/ss_usb.c |  944 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 945 insertions(+)
>  create mode 100644 drivers/net/can/usb/ss_usb.c
> 
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index becef46..c341a1b 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -7,5 +7,6 @@ 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_8DEV_USB) += usb_8dev.o
> +obj-$(CONFIG_CAN_SS_USB) += ss_usb.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/ss_usb.c b/drivers/net/can/usb/ss_usb.c
> new file mode 100644
> index 0000000..69710ff
> --- /dev/null
> +++ b/drivers/net/can/usb/ss_usb.c
> @@ -0,0 +1,944 @@

Please add a header stating your license.

> +#include <linux/init.h>
> +#include <linux/signal.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>
> +
> +/* Device specific constants */
> +#define USB_SSUSB_VENDOR_ID         0xbeef
> +#define USB_SSUSB_AT32_PRODUCT_ID   0x1234

Please get in touch with the Openmoko people for a real vendor/product
id as Sven suggested. Sven can you arrange a contect?

> +
> +#define SS_USB_ENDPOINT_IN          1
> +#define SS_USB_ENDPOINT_OUT         2
> +
> +#define SS_USB_BREQ_HOST_FORMAT     0
> +#define SS_USB_BREQ_BITTIMING       1
> +#define SS_USB_BREQ_MODE            2
> +#define SS_USB_BREQ_BERR            3
> +#define SS_USB_BREQ_BT_CONST        4
> +
> +
> +#define SS_CAN_MODE_RESET           0
> +#define SS_CAN_MODE_START           1
> +
> +#define SS_CAN_STATE_ERROR_ACTIVE   0
> +#define SS_CAN_STATE_ERROR_WARNING  1
> +#define SS_CAN_STATE_ERROR_PASSIVE  2
> +#define SS_CAN_STATE_BUS_OFF        3
> +#define SS_CAN_STATE_STOPPED        4
> +#define SS_CAN_STATE_SLEEPING       5

Can you pelase create an enum for each of these.

> +
> +/* data types passed between host and device */
> +struct __packed device_config {
> +	u16 byte_order;

u32, byteorder please, so that you have natural alignment for this struct.

> +
> +	u32 hf_size;	/* size of a host frame */
> +	u32 hf_can_id;
> +	u32 hf_can_dlc;
> +	u32 hf_data;
> +	u32 hf_echo_id;
> +	u32 hf_channel;
> +};

If your struct is always aligned naturally in mem, please add a
"__attribute__ ((packed, aligned(4)));"

The two above comments apply to all struct definitions here.

> +
> +struct __packed device_mode {
> +	u8  mode;
u32
> +};
> +
> +struct __packed device_state {
> +	u8  state;
u32
> +	u32 rxerr;
> +	u32 txerr;
> +};
> +
> +struct __packed device_bittiming {
> +	u32 prop_seg;
> +	u32 phase_seg1;
> +	u32 phase_seg2;
> +	u32 sjw;
> +	u32 brp;
> +};
> +
> +struct __packed device_bt_const {
> +	u32 fclk_can;
> +	u32 tseg1_min;
> +	u32 tseg1_max;
> +	u32 tseg2_min;
> +	u32 tseg2_max;
> +	u32 sjw_max;
> +	u32 brp_min;
> +	u32 brp_max;
> +	u32 brp_inc;
> +};
> +
> +struct ss_host_frame {
> +	u8  channel;
u32
> +	u32 echo_id;
> +	struct can_frame frame;
> +};
> +
> +#define MAX_TX_URBS 10
> +#define MAX_RX_URBS 10
> +
> +#define MAX_INTF 2
> +
> +struct ss_tx_context {
> +	struct ss_can *dev;
> +	unsigned int echo_id;
> +};
> +
> +struct ss_can {
> +	struct can_priv can; /* must be the first member */
> +
> +	struct ss_usb *parent;
> +
> +	struct net_device *netdev;
> +	struct usb_device *udev;
> +	struct usb_interface *iface;
> +
> +	struct can_bittiming_const bt_const;
> +	unsigned int channel;	/* channel number */
> +
> +	struct ss_tx_context tx_context[MAX_TX_URBS];
> +
> +	struct usb_anchor tx_submitted;
> +	atomic_t active_tx_urbs;
> +
> +	int open_time;
Please remove it and all references to it, not needed anymore
> +};
> +
> +/* usb interface struct */
> +struct ss_usb {
> +	struct ss_can *canch[MAX_INTF];
> +
> +	struct usb_anchor rx_submitted;
> +
> +	atomic_t active_channels;
> +};
> +
> +/* 'allocate' a tx context.
> + * returns a valid tx context or NULL if there is no space.
> + */
> +static struct ss_tx_context *ss_alloc_tx_context(struct ss_can *dev)
> +{
> +	int ii = 0;

We usually only use 'i' for simple for loops.

> +	for (; ii < MAX_TX_URBS; ii++)
> +		if (dev->tx_context[ii].echo_id == MAX_TX_URBS) {
> +			dev->tx_context[ii].echo_id = ii;
> +			return &dev->tx_context[ii];
> +		}
> +	return NULL;
> +}
> +
> +/* releases a tx context
> + */
> +static void ss_free_tx_context(struct ss_tx_context *txc)
> +{
> +	txc->echo_id = MAX_TX_URBS;
> +}
> +
> +/* Get a tx context by id.
> + */
> +static struct ss_tx_context *ss_get_tx_context(
> +	struct ss_can *dev,
> +	unsigned int id)

Does this fit into one line? I don't enforce the 90 columns rule for
functions prototypes.

> +{
> +	if (id < MAX_TX_URBS)
> +		if (dev->tx_context[id].echo_id == id)
> +			return &dev->tx_context[id];
> +
> +	return NULL;
> +}
> +
> +/* Get a tx contexts id.
> + */
> +static unsigned int ss_tx_context_id(struct ss_tx_context *txc)
> +{
> +	return txc->echo_id;
> +}
> +
> +#define BUFFER_SIZE sizeof(struct ss_host_frame)
> +
> +/**************************** USB CALLBACKS **************************/
> +static void ss_usb_recieve_bulk_callback(struct urb *urb)
> +{
> +	struct ss_usb *usbcan = urb->context;
> +	struct ss_can *dev;
> +	struct net_device *netdev;
> +	int rc;
> +	struct net_device_stats *stats;
> +	struct sk_buff *skb;
> +	struct can_frame *cf;
> +	struct ss_host_frame *hf = urb->transfer_buffer;
> +	struct ss_tx_context *txc;
> +
> +	BUG_ON(!usbcan);
> +
> +	switch (urb->status) {
> +	case 0: /* success */
> +		break;
> +	case -ENOENT:
> +	case -ESHUTDOWN:
> +		return;
> +	default:
> +		/* printk(KERN_INFO "Rx URB aborted (%d)\n", urb->status); */

Use netdev_info() and friends or remove the code. Have a look at the
other usb drivers e.g. the drivers/net/can/usb/usb_8dev.c, which is
quite new.

> +		/* do not resubmit aborted urbs. eg: when device goes down */
> +		return;
> +	}
> +
> +	/* device reports out of range channel id */
> +	if (hf->channel >= MAX_INTF)
> +		/* printk(KERN_INFO "%d] strange channel #\n", hf->channel); */
same here
> +		goto resubmit_urb;
> +
> +	/* device reports bad channel id */
> +	dev = usbcan->canch[hf->channel];
> +
> +	BUG_ON(!dev);
> +
> +	netdev = dev->netdev;
> +	BUG_ON(!netdev);

I think you can remove most of the BUG_ONs in the code.

> +
> +	if (!netif_device_present(netdev))
> +		return;
> +
> +	stats = &dev->netdev->stats;
> +
> +	if (hf->echo_id == 0) { /* normal */
> +		netdev_info(netdev, "%d] got msg\n", hf->channel);
> +		skb = alloc_can_skb(dev->netdev, &cf);
> +		if (skb == NULL)
> +			return;
> +
> +		*cf = hf->frame;
> +		cf->can_dlc = get_can_dlc(hf->frame.can_dlc);
> +
> +		netif_rx(skb);
> +
> +		stats->rx_packets++;
> +		stats->rx_bytes += cf->can_dlc;
> +	} else {		/* echo_id = hf->echo_id-1 */
> +		txc = ss_get_tx_context(dev, hf->echo_id-1);
> +
> +		/* bad devices send bad echo_ids. */
> +		if (!txc) {
> +			netdev_info(netdev, "%d] bad echo %d\n",
> +				    hf->channel, hf->echo_id);
> +			goto resubmit_urb;
> +		} else {
> +			netdev_info(netdev, "%d] got echo %d\n",
> +				    hf->channel, hf->echo_id);
> +		}
> +
> +		can_get_echo_skb(netdev, ss_tx_context_id(txc));
> +
> +		ss_free_tx_context(txc);
> +
> +		netdev->stats.tx_packets++;
> +		/* FIXME: report dlc: stats->tx_bytes += cf->can_dlc; */
> +
> +		netdev->trans_start = jiffies;

should not be needed

> +
> +		if (netif_queue_stopped(netdev))
> +			netif_wake_queue(netdev);

just call netif_wake_queue(netdev); unconditionally

> +	}
> +
> +
> +resubmit_urb:
> +	/* do i really need to refill? the urb should still be valid:
> +	usb_fill_bulk_urb(urb,
> +	dev->udev,
> +	usb_rcvbulkpipe(dev->udev,
> +	SS_USB_ENDPOINT_IN(dev->iface->altsetting[0].desc.bInterfaceNumber)),
> +	urb->transfer_buffer,
> +	BUFFER_SIZE, ss_usb_recieve_bulk_callback,
> +	dev);
> +	*/
> +	rc = usb_submit_urb(urb, GFP_ATOMIC);
> +
> +/* How to best handle errore?

Have a look at existing CAN USB drivers

> +	if (rc == -ENODEV) {
> +		printk(KERN_INFO "resubmit error -ENODEV\n");
> +		FIXME: necessary?
> +		netif_device_detach(netdev);
> +
> +	} else if (rc) {
> +		printk(KERN_INFO "resubmit error %d\n", rc);
> +	}
> +*/
> +}
> +
> +static void ss_usb_xmit_callback(struct urb *urb)
> +{
> +	struct ss_tx_context *txc = urb->context;
> +	struct ss_can *dev;
> +	struct net_device *netdev;
> +
> +	BUG_ON(!txc);
> +
> +	dev = txc->dev;
> +	BUG_ON(!dev);
> +
> +	netdev = dev->netdev;
> +	BUG_ON(!netdev);
> +
> +	if (urb->status) {
> +		dev_info(netdev->dev.parent, "%d] xmit err %d\n",
> +			dev->channel, txc->echo_id+1);
> +	} else {
> +		dev_info(netdev->dev.parent, "%d] xmit ok  %d\n",
> +			dev->channel, txc->echo_id+1);
> +	}
> +
> +	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;
> +
> +	/* The urb never made it to the device. Drop this packet. */
> +	if (urb->status) {
> +		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
> +			urb->status);
> +		can_free_echo_skb(netdev, ss_tx_context_id(txc));
> +		ss_free_tx_context(txc);
> +		netdev->stats.tx_dropped++;
> +	}
> +
> +	if (netif_queue_stopped(netdev))
> +		netif_wake_queue(netdev);
> +}
> +
> +/****************************** UBS CAN ******************************/
> +static int ss_usb_set_bittiming(struct net_device *netdev)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct can_bittiming *bt = &dev->can.bittiming;
> +	struct usb_interface *intf = dev->iface;
> +	int rc;
> +
> +	struct __packed device_bittiming dbt = {
> +		.prop_seg = bt->prop_seg,
> +		.phase_seg1 = bt->phase_seg1,
> +		.phase_seg2 = bt->phase_seg2,
> +		.sjw = bt->sjw,
> +		.brp = bt->brp
> +	};
> +
> +/*	printk(KERN_INFO "CH: %d\n", dev->channel);
> +	printk(KERN_INFO "PRS %d\n", bt->prop_seg);
> +	printk(KERN_INFO "PH1 %d\n", bt->phase_seg1);
> +	printk(KERN_INFO "PH2 %d\n", bt->phase_seg2);
> +	printk(KERN_INFO "SJW %d\n", bt->sjw);
> +	printk(KERN_INFO "BRP %d\n", bt->brp);
> +*/

please remove

> +
> +	rc = usb_control_msg(interface_to_usbdev(intf),
> +			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
> +			SS_USB_BREQ_BITTIMING,
> +			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			dev->channel,
> +			0,
> +			&dbt,
> +			sizeof(dbt),
> +			1000);
> +	if (rc < 0)
> +		dev_warn(netdev->dev.parent, "Couldn't set bittimings %d", rc);
> +
> +	return rc;
> +}
> +
> +static int ss_usb_set_mode(struct net_device *netdev, enum can_mode mode)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct usb_interface *intf = dev->iface;
> +	struct device_mode dm;
> +	int rc;
> +	dev_warn(netdev->dev.parent, "ss can mode %d\n", mode);
> +
> +	if (!dev->open_time)
> +		return -EINVAL;
> +
> +
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		dm.mode = SS_CAN_MODE_RESET;
> +
> +		rc = usb_control_msg(interface_to_usbdev(intf),
> +				usb_sndctrlpipe(interface_to_usbdev(intf), 0),
> +				SS_USB_BREQ_MODE,
> +				USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +				dev->channel,
> +				0,
> +				&dm,
> +				sizeof(dm),
> +				1000);
> +		if (rc < 0) {
> +			dev_warn(netdev->dev.parent, "Couldn't start device %d",
> +				rc);
> +			return rc;
> +		}
> +
> +		if (netif_queue_stopped(netdev))
> +			netif_wake_queue(netdev);

call unconditionally

> +		break;
> +	case CAN_MODE_STOP:
> +	case CAN_MODE_SLEEP:
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ss_usb_get_state(const struct net_device *netdev,
> +	enum can_state *state)
> +{
> +
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct usb_interface *intf = dev->iface;
> +	struct device_state dstate;
> +	int rc;
> +	dev_warn(netdev->dev.parent, "ss can state");
> +
> +	rc = usb_control_msg(interface_to_usbdev(intf),
> +			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
> +			SS_USB_BREQ_BERR,
> +			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			dev->channel,
> +			0,
> +			&dstate,
> +			sizeof(dstate),
> +			1000);
> +	if (rc < 0) {
> +		dev_warn(netdev->dev.parent, "Couldn't get state %d", rc);
> +		return rc;
> +	}
> +
> +	netdev_info(netdev, "state: %d\n", dstate.state);
> +
> +	switch (dstate.state) {
> +	/* RX/TX error count < 96 */
> +	case SS_CAN_STATE_ERROR_ACTIVE:
> +		*state = CAN_STATE_ERROR_ACTIVE;
> +		return 0;
> +	/* RX/TX error count < 128 */
> +	case SS_CAN_STATE_ERROR_WARNING:
> +		*state = CAN_STATE_ERROR_WARNING;
> +		return 0;
> +	/* RX/TX error count < 256 */
> +	case SS_CAN_STATE_ERROR_PASSIVE:
> +		*state = CAN_STATE_ERROR_PASSIVE;
> +		return 0;
> +	/* RX/TX error count >= 256 */
> +	case SS_CAN_STATE_BUS_OFF:
> +		*state = CAN_STATE_BUS_OFF;
> +		return 0;
> +	/* Device is stopped */
> +	case SS_CAN_STATE_STOPPED:
> +		*state = CAN_STATE_STOPPED;
> +		return 0;
> +	/* Device is sleeping */
> +	case SS_CAN_STATE_SLEEPING:
> +		*state = CAN_STATE_SLEEPING;
> +		return 0;
> +	}
> +
> +	return -ENOTSUPP;
> +}
> +
> +static int ss_usb_get_berr_counter(const struct net_device *netdev,
> +	struct can_berr_counter *bec)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct usb_interface *intf = dev->iface;
> +	struct device_state dstate;
> +	int rc;
> +
> +	rc = usb_control_msg(interface_to_usbdev(intf),
> +			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
> +			SS_USB_BREQ_BERR,
> +			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			dev->channel,
> +			0,
> +			&dstate,
> +			sizeof(dstate),
> +			1000);
> +	if (rc < 0) {
> +		dev_warn(netdev->dev.parent, "Couldn't get error counters %d",
> +			rc);
> +		return rc;
> +	}
> +
> +	dev_info(netdev->dev.parent, "txerr: %d rxerr: %d\n",
> +		dstate.txerr, dstate.rxerr);
> +
> +	bec->txerr = dstate.txerr;
> +	bec->rxerr = dstate.rxerr;
> +
> +	return 0;
> +}
> +
> +static netdev_tx_t ss_can_start_xmit(struct sk_buff *skb,
> +	struct net_device *netdev)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct net_device_stats *stats = &dev->netdev->stats;
> +	struct urb *urb;
> +	struct ss_host_frame *hf;
> +	int rc;
> +	struct ss_tx_context *txc;
> +
> +	if (can_dropped_invalid_skb(netdev, skb))
> +		return NETDEV_TX_OK;
> +
> +	/* find an empty context to keep track of transmission */
> +	txc = ss_alloc_tx_context(dev);
> +	if (!txc) {
> +		/* dev_kfree_skb(skb); */
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	/* create a URB, and a buffer for it */
> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
> +	if (!urb) {
> +		ss_free_tx_context(txc);
> +		dev_err(netdev->dev.parent, "No memory left for URBs\n");
> +		goto nomem;
> +	}
> +
> +	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
> +		&urb->transfer_dma);
> +	if (!hf) {
> +		ss_free_tx_context(txc);
> +		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
> +		usb_free_urb(urb);
> +		goto nomem;
> +	}
> +
> +	hf->frame = *(struct can_frame *)skb->data;
> +	hf->echo_id = ss_tx_context_id(txc)+1;
> +	hf->channel = dev->channel;
> +
> +	usb_fill_bulk_urb(urb, dev->udev,
> +			usb_sndbulkpipe(dev->udev, SS_USB_ENDPOINT_OUT),
> +			hf,
> +			sizeof(*hf),
> +			ss_usb_xmit_callback,
> +			txc);
> +
> +	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +	usb_anchor_urb(urb, &dev->tx_submitted);
> +
> +	can_put_echo_skb(skb, netdev, ss_tx_context_id(txc));
> +
> +	atomic_inc(&dev->active_tx_urbs);
> +
> +/*	printk(KERN_INFO "%d] xmit new %d\n",hf->channel,hf->echo_id); */
> +
> +	rc = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (unlikely(rc)) {			/* usb send failed */
> +		dev_err(netdev->dev.parent, "usb_submit failed %d\n", rc);
> +
> +		can_free_echo_skb(netdev, ss_tx_context_id(txc));
> +		ss_free_tx_context(txc);
> +
> +		usb_unanchor_urb(urb);
> +		usb_free_coherent(dev->udev,
> +			sizeof(*hf),
> +			hf,
> +			urb->transfer_dma);
> +
> +		dev_kfree_skb(skb);
> +
> +		atomic_dec(&dev->active_tx_urbs);
> +
> +		if (rc == -ENODEV) {
> +			netif_device_detach(netdev);
> +		} else {
> +			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", rc);
> +
> +			stats->tx_dropped++;
> +		}
> +	} else {
> +		netdev->trans_start = jiffies;
> +
> +		/* Slow down tx path */
> +		if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS)
> +			netif_stop_queue(netdev);
> +	}
> +
> +	/* let usb core take care of this urb */
> +	usb_free_urb(urb);
> +
> +	return NETDEV_TX_OK;
> +
> +nomem:
> +	dev_kfree_skb(skb);
> +	stats->tx_dropped++;
> +	return NETDEV_TX_OK;
> +}
> +
> +static int ss_can_open(struct net_device *netdev)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct ss_usb *parent = dev->parent;
> +	struct device_mode dm;
> +	int rc, i;
> +
> +	/* common open */
> +	rc = open_candev(netdev);
> +	if (rc)
> +		return rc;
> +
> +	if (atomic_add_return(1, &parent->active_channels) == 1) {
> +		for (i = 0; i < MAX_RX_URBS; i++) {
> +			struct urb *urb = NULL;
> +			u8 *buf = NULL;
> +
> +			/* alloc rx urb */
> +			urb = usb_alloc_urb(0, GFP_KERNEL);
> +			if (!urb) {
> +				dev_err(netdev->dev.parent,
> +					"No memory left for URBs\n");
> +				return -ENOMEM;
> +			}
> +
> +			/* alloc rx buffer */
> +			/* FIXME: free this memory */
> +			buf = usb_alloc_coherent(dev->udev,
> +				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;
> +			}
> +
> +
> +			/* fill, anchor, and submit rx urb */
> +			usb_fill_bulk_urb(urb,
> +					dev->udev,
> +					usb_rcvbulkpipe(dev->udev,
> +						SS_USB_ENDPOINT_IN),
> +					buf,
> +					BUFFER_SIZE,
> +					ss_usb_recieve_bulk_callback,
> +					parent
> +					);
> +			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +
> +			usb_anchor_urb(urb, &parent->rx_submitted);
> +
> +			rc = usb_submit_urb(urb, GFP_KERNEL);
> +			if (rc) {
> +				if (rc == -ENODEV)
> +					netif_device_detach(dev->netdev);
> +
> +				/* FIXME: free rx buffer/urb maybe? */
> +				usb_unanchor_urb(urb);
> +				break;
> +			}
> +
> +			/* Drop reference,
> +			 * USB core will take care of freeing it
> +			 */
> +			usb_free_urb(urb);
> +		}
> +	}
> +
> +	/* finally start device */
> +	dm.mode = SS_CAN_MODE_START;
> +	rc = usb_control_msg(interface_to_usbdev(dev->iface),
> +			usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),
> +			SS_USB_BREQ_MODE,
> +			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			dev->channel,
> +			0,
> +			&dm,
> +			sizeof(dm),
> +			1000);
> +
> +	if (rc < 0)
> +		dev_warn(netdev->dev.parent, "Couldn't start device %d", rc);
> +
> +	dev->open_time = jiffies;
> +
> +	netif_start_queue(netdev);
> +
> +	netdev_info(netdev, "ss can close\n");
> +	return 0;
> +}
> +
> +/* think: ip link set canx down */
> +static int ss_can_close(struct net_device *netdev)
> +{
> +	struct ss_can *dev = netdev_priv(netdev);
> +	struct ss_usb *parent = dev->parent;
> +
> +	netif_stop_queue(netdev);
> +
> +	/* Stop polling */
> +	if (atomic_dec_and_test(&parent->active_channels)) {
> +		usb_kill_anchored_urbs(&parent->rx_submitted);
> +		/* FIXME: Set CAN controller to reset mode */
> +	}
> +
> +	close_candev(netdev);
> +
> +	dev->open_time = 0;
> +
> +	netdev_info(netdev, "ss can close\n");
> +	return 0;
> +}
> +
> +/******************************** USB ********************************/
> +
> +static const struct net_device_ops ss_usb_netdev_ops = {
> +	.ndo_open = ss_can_open,
> +	.ndo_stop = ss_can_close,
> +	.ndo_start_xmit = ss_can_start_xmit,
> +};
> +
> +static int ss_make_candev(struct ss_can **dev, unsigned int channel,
> +	struct usb_interface *intf);
> +static void ss_destroy_candev(struct ss_can *dev);

Please rearrange your code, that you don't need forward decarations.

> +
> +static int ss_usb_probe(struct usb_interface *intf,
> +	const struct usb_device_id *id)
> +{
> +	struct ss_usb *dev;
> +	int rc = -ENOMEM;
> +	unsigned int icount, i;
> +
> +	struct device_config conf;
> +
> +	/* send host config */
> +	conf = (struct device_config){
> +		.byte_order = 0xbeef,
> +		.hf_size = sizeof(struct ss_host_frame),
> +		.hf_can_id = offsetof(struct ss_host_frame, frame.can_id),
> +		.hf_can_dlc = offsetof(struct ss_host_frame, frame.can_dlc),
> +		.hf_data = offsetof(struct ss_host_frame, frame.data),
> +		.hf_echo_id = offsetof(struct ss_host_frame, echo_id),
> +		.hf_channel = offsetof(struct ss_host_frame, channel),
> +	};

please use C99 initializers.
struct device_config conf = {
	.hf_size = sizeof(struct ss_host_frame),
	...
};

> +
> +	rc = usb_control_msg(interface_to_usbdev(intf),
> +			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
> +			SS_USB_BREQ_HOST_FORMAT,
> +			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			1,
> +			intf->altsetting[0].desc.bInterfaceNumber,
> +			&conf,
> +			sizeof(conf),
> +			1000);
> +
> +	if (rc < 0) {
> +		dev_err(&intf->dev, "ss_usb: Couldn't send data format %d\n",
> +			rc);
> +		return rc;
> +	}
> +
> +	/* FIXME: fetch interface count and global settings etc.. */
> +	icount = 2;
> +
> +	if (icount > MAX_INTF) {
> +		dev_err(&intf->dev,
> +			"ss_usb: Cannot handle more that %d Can interfaces\n",
> +			MAX_INTF);
> +		return -EINVAL;
> +	}
> +
> +	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> +	init_usb_anchor(&dev->rx_submitted);
> +
> +	atomic_set(&dev->active_channels, 0);
> +
> +	usb_set_intfdata(intf, dev);
> +
> +	for (i = 0; i < icount; i++) {
> +		rc = ss_make_candev(&dev->canch[i], i, intf);
> +		if (rc) {
> +			icount = i;
> +			for (i = 0; i < icount; i++) {
> +				ss_destroy_candev(dev->canch[i]);
> +				dev->canch[icount] = NULL;
> +			}
> +			return rc;
> +		}
> +		dev->canch[i]->parent = dev;
> +	}
> +
> +	dev_info(&intf->dev, "ss usb probe\n");
> +	return 0;
> +}
> +
> +static void ss_usb_disconnect(struct usb_interface *intf)
> +{
> +	unsigned i;
> +	struct ss_usb *dev = usb_get_intfdata(intf);
> +	usb_set_intfdata(intf, NULL);
> +
> +	if (!dev) {
> +		dev_info(&intf->dev, "ss usb disconnect (nodata)\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < MAX_INTF; i++) {
> +		struct ss_can *can = dev->canch[i];
> +
> +		if (!can)
> +			continue;
> +
> +		unregister_netdev(can->netdev);
> +		free_candev(can->netdev);
> +
> +		usb_kill_anchored_urbs(&can->tx_submitted);
> +	}
> +
> +	usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> +	dev_info(&intf->dev, "ss usb disconnect\n");
> +}
> +
> +MODULE_DEVICE_TABLE(usb, ss_usb_table);
> +static struct usb_device_id ss_usb_table[] = {
> +	{USB_DEVICE(USB_SSUSB_VENDOR_ID, USB_SSUSB_AT32_PRODUCT_ID)},
> +	{} /* Terminating entry */
> +};
> +
> +static struct usb_driver ss_usb_driver = {
> +	.name       = "ss_usb",
> +	.probe      = ss_usb_probe,
> +	.disconnect = ss_usb_disconnect,
> +	.id_table   = ss_usb_table,
> +	/* FIXME: suspend, resume, etc... */
> +};
> +
> +static int __init ss_usb_init(void)
> +{
> +	return usb_register(&ss_usb_driver);
> +}
> +
> +static void __exit ss_usb_exit(void)
> +{
> +	usb_deregister(&ss_usb_driver);
> +}
> +
> +module_init(ss_usb_init);
> +module_exit(ss_usb_exit);
> +
> +MODULE_AUTHOR("Maximilian Schneider <mws@schneidersoft.net>");
> +MODULE_DESCRIPTION(
> +"socket CAN device driver for SchneiderSoft USB2.0 to CAN interfaces.");
> +MODULE_LICENSE("GPL v2");
> +
> +
> +
> +
> +
> +static int ss_make_candev(struct ss_can **out,
> +	unsigned int channel,
> +	struct usb_interface *intf)
> +{
> +	struct ss_can *dev;
> +	struct device_bt_const bt_const;
> +	struct net_device *netdev;
> +	int rc;
> +	/* fetch bit timing constants */
> +	rc = usb_control_msg(interface_to_usbdev(intf),
> +			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
> +			SS_USB_BREQ_BT_CONST,
> +			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
> +			channel,
> +			0,
> +			&bt_const,
> +			sizeof(bt_const),
> +			1000);
> +
> +	if (rc < 0) {
> +		dev_err(&intf->dev,
> +			"ss_usb: Couldn't get bit timing const for channel %d\n",
> +			rc);
> +		return rc;
> +	}
> +
> +	dev_info(&intf->dev, "fclock: %d\n", bt_const.fclk_can);
> +
> +	/* create netdev */
> +	netdev = alloc_candev(sizeof(struct ss_can), MAX_TX_URBS);
> +	if (!netdev) {
> +		dev_err(&intf->dev, "ss_usb: Couldn't alloc candev\n");
> +		return -ENOMEM;
> +	}
> +
> +	dev = netdev_priv(netdev);
> +
> +	netdev->netdev_ops = &ss_usb_netdev_ops;
> +
> +	netdev->flags |= IFF_ECHO; /* we support full roundtrip echo */
> +
> +	/* dev settup */
> +	dev->bt_const = (struct can_bittiming_const){
> +		.name = "ss_usb",
> +		.tseg1_min = bt_const.tseg1_min,
> +		.tseg1_max = bt_const.tseg1_max,
> +		.tseg2_min = bt_const.tseg2_min,
> +		.tseg2_max = bt_const.tseg2_max,
> +		.sjw_max = bt_const.sjw_max,
> +		.brp_min = bt_const.brp_min,
> +		.brp_max = bt_const.brp_max,
> +		.brp_inc = bt_const.brp_inc
> +	};
> +
> +	dev->udev = interface_to_usbdev(intf);
> +	dev->iface = intf;
> +	dev->netdev = netdev;
> +	dev->channel = channel;
> +
> +	init_usb_anchor(&dev->tx_submitted);
> +	atomic_set(&dev->active_tx_urbs, 0);
> +	for (rc = 0; rc < MAX_TX_URBS; rc++) {
> +		dev->tx_context[rc].dev = dev;
> +		dev->tx_context[rc].echo_id = MAX_TX_URBS;
> +	}
> +
> +	/* can settup */
> +	dev->can.state = CAN_STATE_STOPPED;
> +	dev->can.clock.freq = bt_const.fclk_can;
> +	dev->can.bittiming_const = &dev->bt_const;
> +	dev->can.do_set_bittiming = ss_usb_set_bittiming;
> +	dev->can.do_set_mode = ss_usb_set_mode;
> +	dev->can.do_get_state = ss_usb_get_state;
> +	dev->can.do_get_berr_counter = ss_usb_get_berr_counter;
> +	dev->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> +
> +	SET_NETDEV_DEV(netdev, &intf->dev);
> +
> +	rc = register_candev(dev->netdev);
> +	if (rc) {
> +		free_candev(dev->netdev);
> +		dev_err(&intf->dev, "ss_usb: Couldn't register candev\n");
> +		return rc;
> +	}
> +
> +	*out = dev;
> +	return 0;
> +}
> +
> +static void ss_destroy_candev(struct ss_can *dev)
> +{
> +	unregister_candev(dev->netdev);
> +	free_candev(dev->netdev);
> +}
> +
> -- 1.7.10.4
> 

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3439 bytes --]

On 05/02/2013 01:07 PM, Marc Kleine-Budde wrote:
> On 04/24/2013 11:33 PM, Max S. wrote:
>> From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
>> From: Maximilian Schneider <max@schneidersoft.net>
>> Date: Wed, 24 Apr 2013 21:03:31 +0000
>> Subject: [PATCH] Added ss_usb source.
> 
> Comments inline.
> 
>> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
>> ---
>>  drivers/net/can/usb/Makefile |    1 +
> 
> You should add your driver to Kconfig, too.
> 
>>  drivers/net/can/usb/ss_usb.c |  944 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 945 insertions(+)
>>  create mode 100644 drivers/net/can/usb/ss_usb.c
>>
>> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
>> index becef46..c341a1b 100644
>> --- a/drivers/net/can/usb/Makefile
>> +++ b/drivers/net/can/usb/Makefile
>> @@ -7,5 +7,6 @@ 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_8DEV_USB) += usb_8dev.o
>> +obj-$(CONFIG_CAN_SS_USB) += ss_usb.o
>>  
>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>> diff --git a/drivers/net/can/usb/ss_usb.c b/drivers/net/can/usb/ss_usb.c
>> new file mode 100644
>> index 0000000..69710ff
>> --- /dev/null
>> +++ b/drivers/net/can/usb/ss_usb.c
>> @@ -0,0 +1,944 @@
> 
> Please add a header stating your license.
> 
>> +#include <linux/init.h>
>> +#include <linux/signal.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>
>> +
>> +/* Device specific constants */
>> +#define USB_SSUSB_VENDOR_ID         0xbeef
>> +#define USB_SSUSB_AT32_PRODUCT_ID   0x1234
> 
> Please get in touch with the Openmoko people for a real vendor/product
> id as Sven suggested. Sven can you arrange a contect?
> 
>> +
>> +#define SS_USB_ENDPOINT_IN          1
>> +#define SS_USB_ENDPOINT_OUT         2
>> +
>> +#define SS_USB_BREQ_HOST_FORMAT     0
>> +#define SS_USB_BREQ_BITTIMING       1
>> +#define SS_USB_BREQ_MODE            2
>> +#define SS_USB_BREQ_BERR            3
>> +#define SS_USB_BREQ_BT_CONST        4
>> +
>> +
>> +#define SS_CAN_MODE_RESET           0
>> +#define SS_CAN_MODE_START           1
>> +
>> +#define SS_CAN_STATE_ERROR_ACTIVE   0
>> +#define SS_CAN_STATE_ERROR_WARNING  1
>> +#define SS_CAN_STATE_ERROR_PASSIVE  2
>> +#define SS_CAN_STATE_BUS_OFF        3
>> +#define SS_CAN_STATE_STOPPED        4
>> +#define SS_CAN_STATE_SLEEPING       5
> 
> Can you pelase create an enum for each of these.
> 
>> +
>> +/* data types passed between host and device */
>> +struct __packed device_config {
>> +	u16 byte_order;
> 
> u32, byteorder please, so that you have natural alignment for this struct.
> 
>> +
>> +	u32 hf_size;	/* size of a host frame */
>> +	u32 hf_can_id;
>> +	u32 hf_can_dlc;
>> +	u32 hf_data;
>> +	u32 hf_echo_id;
>> +	u32 hf_channel;
>> +};

Please give your struct a common prefix "ss_" like your defines already
have.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Wolfgang Grandegger]

On 05/02/2013 01:07 PM, Marc Kleine-Budde wrote:
> On 04/24/2013 11:33 PM, Max S. wrote:
>> From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
>> From: Maximilian Schneider <max@schneidersoft.net>
>> Date: Wed, 24 Apr 2013 21:03:31 +0000
>> Subject: [PATCH] Added ss_usb source.
> 
> Comments inline.
> 
>> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
>> ---
>>  drivers/net/can/usb/Makefile |    1 +
> 
> You should add your driver to Kconfig, too.
> 
>>  drivers/net/can/usb/ss_usb.c |  944 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 945 insertions(+)
>>  create mode 100644 drivers/net/can/usb/ss_usb.c

What stands "ss" for? I would prefer "ssoft" or "schneidersoft".

Wolfgang.

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On 05/02/2013 01:30 PM, Wolfgang Grandegger wrote:
> On 05/02/2013 01:07 PM, Marc Kleine-Budde wrote:
>> On 04/24/2013 11:33 PM, Max S. wrote:
>>> From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
>>> From: Maximilian Schneider <max@schneidersoft.net>
>>> Date: Wed, 24 Apr 2013 21:03:31 +0000
>>> Subject: [PATCH] Added ss_usb source.
>>
>> Comments inline.
>>
>>> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
>>> ---
>>>  drivers/net/can/usb/Makefile |    1 +
>>
>> You should add your driver to Kconfig, too.
>>
>>>  drivers/net/can/usb/ss_usb.c |  944 ++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 945 insertions(+)
>>>  create mode 100644 drivers/net/can/usb/ss_usb.c
> 
> What stands "ss" for? I would prefer "ssoft" or "schneidersoft".

Good idea, ad the term is historically biased in Germany.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]

On 04/24/2013 11:33 PM, Max S. wrote:
> From 8a83080bcddc558cd016e58e408b56ca68bc5bc8 Mon Sep 17 00:00:00 2001
> From: Maximilian Schneider <max@schneidersoft.net>
> Date: Wed, 24 Apr 2013 21:03:31 +0000
> Subject: [PATCH] Added ss_usb source.
> 
> 
> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>

Please fix these errors, too:

> drivers/net/can/usb/ss_usb.c:343 ss_usb_set_bittiming() error: doing dma on the stack (&dbt)
> drivers/net/can/usb/ss_usb.c:374 ss_usb_set_mode() error: doing dma on the stack (&dm)
> drivers/net/can/usb/ss_usb.c:411 ss_usb_get_state() error: doing dma on the stack (&dstate)
> drivers/net/can/usb/ss_usb.c:465 ss_usb_get_berr_counter() error: doing dma on the stack (&dstate)
> drivers/net/can/usb/ss_usb.c:661 ss_can_open() error: doing dma on the stack (&dm)
> drivers/net/can/usb/ss_usb.c:736 ss_usb_probe() error: doing dma on the stack (&conf)
> drivers/net/can/usb/ss_usb.c:859 ss_make_candev() error: doing dma on the stack (&bt_const)

You should not use memory on stack for usb transfers. Use kmalloc(), or
kzalloc() (and kfree) instead.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Max S.]

[-- Attachment #1: Type: text/plain, Size: 8686 bytes --]

Hello all,

I've 'rebranded' the entire driver to Geschwister Schneider
Technologie-, Entwicklungs- und Vertriebs UG. using gs_ for structs,
enums and function names.

Regarding the dma on the stack errors, where do you see these messages? I have modified the code (see patch attached), and they should no longer appear.

I have sent an email to the openmoko people, and will update the product/vender code when i get a response.

There are still some things I'd like to work out:
In the driver i do netdev->flags |= IFF_ECHO; to tell the netdev subsystem i will be handling message delivery tracking.
via can_free_echo_skb, can_get_echo_skb, can_put_echo_skb...

I read in the 8devices driver:
dev->can.ctrlmode_supported = ...
				CAN_CTRLMODE_LOOPBACK |
				...

An in documentation/networking/can.txt:
To reflect the correct* traffic on the node the loopback of the sent data has to be performed right after a successful transmission.

what is the difference between IFF_ECHO and CAN_CTRLMODE_LOOPBACK? loopback is default behavior...
How should my driver/device behave differently when loopback is not enabled?

Currently when the CAN controller fails to send a can frame, for example when there is no other node to ack it, my firmware will produce an error frame and queue it for the host.
Since the can controller will immediately retry transmission, if the error condition persists this means error frames will continually be sent to the host.
Is this acceptable behavior? I think it would be better to find a way to throttle repetitive error frames.

I have done some testing with higher can speeds and run into the following kernel panic (netconsole extract):

[ 1784.427960] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
occupied!
[ 1784.493869] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
occupied!
[ 1784.745668] ------------[ cut here ]------------
[ 1784.745701] kernel BUG
at /build/buildd-linux_3.2.35-2-amd64-v9djlH/linux-3.2.35/drivers/net/can/dev.c:283!
[ 1784.745719] invalid opcode: 0000 [#1] SMP 
[ 1784.745740] CPU 2 
[ 1784.745750] Modules linked in: can_raw can gs_usb(O) netconsole
configfs can_dev ppdev lp rfcomm bnep bluetooth crc16 cpufreq_stats
cpufreq_userspace cpufreq_powersave cpufreq_conservative fuse nfsd nfs
nfs_acl auth_rpcgss fscache lockd sunrpc usbhid hid pcan(O) loop arc4
rt2800usb rt2x00usb rt2800lib rt2x00lib mac80211 cfg80211 rfkill
crc_ccitt evdev psmouse snd_hda_intel coretemp serio_raw snd_hda_codec
i2c_i801 snd_hwdep pcspkr i2c_core snd_pcm snd_page_alloc snd_seq
snd_seq_device snd_timer parport_pc iTCO_wdt parport iTCO_vendor_support
snd uhci_hcd video acpi_cpufreq mperf ehci_hcd usbcore e1000e soundcore
button usb_common processor thermal_sys ext2 mbcache sd_mod crc_t10dif
ahci libahci libata scsi_mod
[ 1784.746265] 
[ 1784.746276] Pid: 7786, comm: cansequence Tainted: G           O
3.2.0-4-amd64 #1 Debian 3.2.35-2                  /DN2800MT
[ 1784.746297] RIP: 0010:[<ffffffffa03d9552>]  [<ffffffffa03d9552>]
can_put_echo_skb+0x16/0x106 [can_dev]
[ 1784.746319] RSP: 0018:ffff880117265b38  EFLAGS: 00010246
[ 1784.746327] RAX: ffff880117126938 RBX: ffff88012946e8c0 RCX:
ffff88012a8da090
[ 1784.746336] RDX: 000000000000000a RSI: ffff880117126000 RDI:
ffff88012946e8c0
[ 1784.746344] RBP: ffff880117126000 R08: ffff88012a8da090 R09:
0000000000000018
[ 1784.746352] R10: 000000000000ffff R11: 0000000000000246 R12:
ffff880037c102a0
[ 1784.746361] R13: ffff88012946e8c0 R14: ffff880117126870 R15:
ffff880117126000
[ 1784.746370] FS:  00007fcfbc52c700(0000) GS:ffff88012fd00000(0000)
knlGS:0000000000000000
[ 1784.746379] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 1784.746387] CR2: 00007fc450074e38 CR3: 0000000129e6f000 CR4:
00000000000006e0
[ 1784.746396] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 1784.746404] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
0000000000000400
[ 1784.746414] Process cansequence (pid: 7786, threadinfo
ffff880117264000, task ffff880120f54140)
[ 1784.746428] Stack:
[ 1784.746438]  ffff880117126000 ffff88012742b680 ffff880037c102a0
ffff88012946e8c0
[ 1784.746475]  ffff880117126870 ffffffffa03f102a ffff88012908dbc0
00000000000f41ff
[ 1784.746504]  0000000001000000 ffff88012946e8c0 0000000000000010
ffff880117126000
[ 1784.746527] Call Trace:
[ 1784.746546]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
[gs_usb]
[ 1784.746569]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
[ 1784.746589]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
[ 1784.746607]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
[ 1784.746625]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
[ 1784.746644]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
[ 1784.746665]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
[ 1784.746685]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
[ 1784.746705]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
[ 1784.746726]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
[ 1784.746746]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
[ 1784.746769]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
[ 1784.746790]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
[ 1784.746809]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
[ 1784.746832]  [<ffffffff81163995>] ? security_file_permission
+0x16/0x2d
[ 1784.746852]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
[ 1784.746872]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
[ 1784.746895]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
[ 1784.746910] Code: e0 48 8b 83 10 08 00 00 48 c7 04 e8 00 00 00 00 41
5b 5b 5d c3 41 56 41 55 41 54 55 48 89 f5 53 3b 96 08 08 00 00 48 89 fb
72 02 <0f> 0b f6 86 b2 01 00 00 04 0f 84 c8 00 00 00 8a 47 7d 83 e0 07 
[ 1784.747301] RIP  [<ffffffffa03d9552>] can_put_echo_skb+0x16/0x106
[can_dev]
[ 1784.747324]  RSP <ffff880117265b38>
[ 1784.747345] ---[ end trace 811113d28006e99a ]---
[ 1784.747357] Kernel panic - not syncing: Fatal exception in interrupt
[ 1784.747368] Pid: 7786, comm: cansequence Tainted: G      D    O
3.2.0-4-amd64 #1 Debian 3.2.35-2
[ 1784.747377] Call Trace:
[ 1784.747390]  [<ffffffff813467bc>] ? panic+0x95/0x1a5
[ 1784.747403]  [<ffffffff8134de86>] ? oops_end+0xa9/0xb6
[ 1784.747416]  [<ffffffff8100e965>] ? do_invalid_op+0x87/0x91
[ 1784.747431]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
[can_dev]
[ 1784.747444]  [<ffffffff81037e96>] ? set_next_entity+0x32/0x55
[ 1784.747456]  [<ffffffff8100d025>] ? paravirt_write_msr+0xb/0xe
[ 1784.747468]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
[ 1784.747480]  [<ffffffff810397dd>] ? finish_task_switch+0x4e/0xb9
[ 1784.747493]  [<ffffffff81353feb>] ? invalid_op+0x1b/0x20
[ 1784.747507]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
[can_dev]
[ 1784.747561]  [<ffffffffa01800b0>] ? usb_anchor_urb+0x25/0x65
[usbcore]
[ 1784.747577]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
[gs_usb]
[ 1784.747592]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
[ 1784.747606]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
[ 1784.747617]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
[ 1784.747629]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
[ 1784.747641]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
[ 1784.747655]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
[ 1784.747669]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
[ 1784.747682]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
[ 1784.747695]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
[ 1784.747707]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
[ 1784.747721]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
[ 1784.747734]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
[ 1784.747746]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
[ 1784.747763]  [<ffffffff81163995>] ? security_file_permission
+0x16/0x2d
[ 1784.747775]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
[ 1784.747786]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
[ 1784.747799]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
[42043.689364] BUG: scheduling while atomic: swapper/0/0/0x10000100
[42043.690985] Stack:
[42043.691116] Call Trace:
[42043.691211] Code: d2 65 48 8b 04 25 c8 c6 00 00 48 89 d1 48 2d c8 1f
00 00 0f 01 c8 0f ae f0 e8 c2 fc ff ff 85 c0 75 0b 31 c0 48 89 c1 fb 0f
01 c9 <eb> 05 e8 98 fc ff ff 65 8b 3c 25 08 dc 00 00 e8 9b fd ff ff 65

The first two lines tell me I'm not using can_put_echo_skb correctly.
I can only imagine there is a kind of race condition in the
gs_*_tx_context group of functions, but i don't know how to solve it.
The fact that the driver works fine at lower CAN speeds again makes me
want to think race condition.

What do you think?

Regards,
Max Schneider.

[-- Attachment #2: GS_USB.patch --]
[-- Type: text/x-patch, Size: 25754 bytes --]

From 304b5af4c34d38d061437ff4b02823803e83afe4 Mon Sep 17 00:00:00 2001
From: Maximilian Schneider <max@schneidersoft.net>
Date: Tue, 4 Jun 2013 13:07:47 +0000
Subject: [PATCH 1/1] Geschwister Schneider USB2CAN driver source.


Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
---
 drivers/net/can/usb/Kconfig  |    6 +
 drivers/net/can/usb/Makefile |    1 +
 drivers/net/can/usb/gs_usb.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1049 insertions(+)
 create mode 100644 drivers/net/can/usb/gs_usb.c

diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index fc96a3d..b830014 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -54,4 +54,10 @@ config CAN_8DEV_USB
 	  This driver supports the USB2CAN interface
 	  from 8 devices (http://www.8devices.com).
 
+config CAN_GS_USB
+	tristate "Geschwister Schneider USB2CAN interface"
+	---help---
+	  This driver supports the USB2CAN interface
+	  from Geschwister Schneider (http://schneidersoft.net)
+
 endmenu
diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index becef46..c21a521 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -7,5 +7,6 @@ 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_8DEV_USB) += usb_8dev.o
+obj-$(CONFIG_CAN_GS_USB) += gs_usb.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
new file mode 100644
index 0000000..f25648c
--- /dev/null
+++ b/drivers/net/can/usb/gs_usb.c
@@ -0,0 +1,1042 @@
+/*
+ * CAN driver for Geschwister Schneider USB/CAN devices.
+ *
+ * Copyright (C) 2013 Geschwister Schneider Technologie-,
+ * Entwicklungs- und Vertriebs UG (Haftungsbeschränkt).
+ *
+ * Many thanks to all socketcan devs
+ *
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/signal.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>
+
+/* Device specific constants */
+#define USB_GSUSB_1_VENDOR_ID      0xbeef
+#define USB_GSUSB_1_PRODUCT_ID     0x1234
+
+#define GSUSB_ENDPOINT_IN          1
+#define GSUSB_ENDPOINT_OUT         2
+
+enum {
+	GS_USB_BREQ_HOST_FORMAT = 0,
+	GS_USB_BREQ_BITTIMING,
+	GS_USB_BREQ_MODE,
+	GS_USB_BREQ_BERR,
+	GS_USB_BREQ_BT_CONST,
+	GS_USB_BREQ_DEVICE_CONFIG
+};
+
+#define GS_DCONF_IFACEMASK          0x7
+
+
+enum {
+	GS_CAN_MODE_RESET = 0,
+	GS_CAN_MODE_START
+};
+
+#define GS_USB_NORMAL               0
+#define GS_USB_LISTENONLY           1
+#define GS_USB_LOOPBACK             2
+
+#define GS_USB_ONE_SHOT             4
+#define GS_USB_TRIPLE_SAMPLE        8
+
+enum {
+	GS_CAN_STATE_ERROR_ACTIVE = 0,
+	GS_CAN_STATE_ERROR_WARNING,
+	GS_CAN_STATE_ERROR_PASSIVE,
+	GS_CAN_STATE_BUS_OFF,
+	GS_CAN_STATE_STOPPED,
+	GS_CAN_STATE_SLEEPING
+};
+
+/* data types passed between host and device */
+struct __packed gs_host_config {
+	u32 byte_order;
+};
+
+struct __packed gs_device_config {
+	u32 feature;
+};
+
+struct __packed gs_device_mode {
+	u32 mode;
+	u32 flags;
+};
+
+struct __packed gs_device_state {
+	u32 state;
+	u32 rxerr;
+	u32 txerr;
+};
+
+struct __packed gs_device_bittiming {
+	u32 prop_seg;
+	u32 phase_seg1;
+	u32 phase_seg2;
+	u32 sjw;
+	u32 brp;
+};
+
+struct __packed gs_device_bt_const {
+	u32 fclk_can;
+	u32 tseg1_min;
+	u32 tseg1_max;
+	u32 tseg2_min;
+	u32 tseg2_max;
+	u32 sjw_max;
+	u32 brp_min;
+	u32 brp_max;
+	u32 brp_inc;
+};
+
+struct __packed gs_host_frame {
+	u32  channel;
+	u32 echo_id;
+
+	u32 can_id;
+	u32 can_dlc;
+	u8 data[8];
+};
+
+/* special address description flags for the CAN_ID */
+#define GS_EFF_FLAG 0x80000000U /* EFF/SFF is set in the MSB */
+#define GS_RTR_FLAG 0x40000000U /* remote transmission request */
+#define GS_ERR_FLAG 0x20000000U /* error frame */
+
+/* valid bits in CAN ID for frame formats */
+#define GS_SFF_MASK 0x000007FFU /* standard frame format (SFF) */
+#define GS_EFF_MASK 0x1FFFFFFFU /* extended frame format (EFF) */
+#define GS_ERR_MASK 0x1FFFFFFFU /* omit EFF, RTR, ERR flags */
+
+/* FIXME: this macro is only a placeholder for a better function */
+#define CONVERT_FLAGS(x) x
+
+#define MAX_TX_URBS 10
+#define MAX_RX_URBS 10
+
+#define MAX_INTF 2
+
+struct gs_tx_context {
+	struct gs_can *dev;
+	unsigned int echo_id;
+};
+
+struct gs_can {
+	struct can_priv can; /* must be the first member */
+
+	struct gs_usb *parent;
+
+	struct net_device *netdev;
+	struct usb_device *udev;
+	struct usb_interface *iface;
+
+	struct can_bittiming_const bt_const;
+	unsigned int channel;	/* channel number */
+
+	struct gs_tx_context tx_context[MAX_TX_URBS];
+
+	struct usb_anchor tx_submitted;
+	atomic_t active_tx_urbs;
+};
+
+/* usb interface struct */
+struct gs_usb {
+	struct gs_can *canch[MAX_INTF];
+
+	struct usb_anchor rx_submitted;
+
+	atomic_t active_channels;
+};
+
+/* 'allocate' a tx context.
+ * returns a valid tx context or NULL if there is no space.
+ */
+static struct gs_tx_context *gs_alloc_tx_context(struct gs_can *dev)
+{
+	int i = 0;
+	for (; i < MAX_TX_URBS; i++)
+		if (dev->tx_context[i].echo_id == MAX_TX_URBS) {
+			dev->tx_context[i].echo_id = i;
+			return &dev->tx_context[i];
+		}
+	return NULL;
+}
+
+/* releases a tx context
+ */
+static void gs_free_tx_context(struct gs_tx_context *txc)
+{
+	txc->echo_id = MAX_TX_URBS;
+}
+
+/* Get a tx context by id.
+ */
+static struct gs_tx_context *gs_get_tx_context(
+	struct gs_can *dev,
+	unsigned int id)
+{
+	if (id < MAX_TX_URBS)
+		if (dev->tx_context[id].echo_id == id)
+			return &dev->tx_context[id];
+
+	return NULL;
+}
+
+/* Get a tx contexts id.
+ */
+static unsigned int gs_tx_context_id(struct gs_tx_context *txc)
+{
+	return txc->echo_id;
+}
+
+#define BUFFER_SIZE sizeof(struct gs_host_frame)
+
+/**************************** USB CALLBACKS **************************/
+static void gs_usb_recieve_bulk_callback(struct urb *urb)
+{
+	struct gs_usb *usbcan = urb->context;
+	struct gs_can *dev;
+	struct net_device *netdev;
+	int rc;
+	struct net_device_stats *stats;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	struct gs_host_frame *hf = urb->transfer_buffer;
+	struct gs_tx_context *txc;
+
+	BUG_ON(!usbcan);
+
+	switch (urb->status) {
+	case 0: /* success */
+		break;
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+	default:
+		/* printk(KERN_INFO "Rx URB aborted (%d)\n", urb->status); */
+		/* do not resubmit aborted urbs. eg: when device goes down */
+		return;
+	}
+
+	/* device reports out of range channel id */
+	if (hf->channel >= MAX_INTF)
+		/* printk(KERN_INFO "%d] strange channel #\n", hf->channel); */
+		goto resubmit_urb;
+
+	/* device reports bad channel id */
+	dev = usbcan->canch[hf->channel];
+
+	BUG_ON(!dev);
+
+	netdev = dev->netdev;
+	BUG_ON(!netdev);
+
+	if (!netif_device_present(netdev))
+		return;
+
+	stats = &dev->netdev->stats;
+
+	if (hf->echo_id == -1) { /* normal */
+		skb = alloc_can_skb(dev->netdev, &cf);
+		if (skb == NULL)
+			return;
+
+		cf->can_id = CONVERT_FLAGS(hf->can_id);
+		/* is it necessary to use get_can_dlc? */
+		cf->can_dlc = get_can_dlc(hf->can_dlc);
+		memcpy(cf->data, hf->data, 8);
+
+		netif_rx(skb);
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	} else {		/* echo_id = hf->echo_id */
+		txc = gs_get_tx_context(dev, hf->echo_id);
+
+		/* bad devices send bad echo_ids. */
+		if (!txc) {
+			netdev_info(netdev, "bad echo %d\n", hf->echo_id);
+			goto resubmit_urb;
+		}
+
+		can_get_echo_skb(netdev, gs_tx_context_id(txc));
+
+		gs_free_tx_context(txc);
+
+		stats->tx_packets++;
+		/* FIXME: stats->tx_bytes += cf->can_dlc; */
+
+		netif_wake_queue(netdev);
+	}
+
+
+resubmit_urb:
+	/* do i really need to refill? the urb should still be valid:
+	usb_fill_bulk_urb(urb,
+	dev->udev,
+	usb_rcvbulkpipe(dev->udev,GSUSB_ENDPOINT_IN(
+		dev->iface->altsetting[0].desc.bInterfaceNumber)),
+	urb->transfer_buffer,
+	BUFFER_SIZE, gs_usb_recieve_bulk_callback,
+	dev);
+	*/
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+
+	/* USB failure take down all interfaces */
+	if (rc == -ENODEV) {
+		for (rc = 0; rc < MAX_INTF; rc++) {
+			if (usbcan->canch[rc])
+				netif_device_detach(usbcan->canch[rc]->netdev);
+		}
+	}
+}
+
+static void gs_usb_xmit_callback(struct urb *urb)
+{
+	struct gs_tx_context *txc = urb->context;
+	struct gs_can *dev;
+	struct net_device *netdev;
+
+	BUG_ON(!txc);
+
+	dev = txc->dev;
+	BUG_ON(!dev);
+
+	netdev = dev->netdev;
+	BUG_ON(!netdev);
+
+	if (urb->status) {
+		dev_info(netdev->dev.parent, "%d] xmit err %d\n",
+			dev->channel, txc->echo_id);
+	}
+
+	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;
+
+	/* The urb never made it to the device. Drop this packet. */
+	if (urb->status) {
+		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
+			urb->status);
+		can_free_echo_skb(netdev, gs_tx_context_id(txc));
+		gs_free_tx_context(txc);
+		netdev->stats.tx_dropped++;
+	}
+
+	if (netif_queue_stopped(netdev))
+		netif_wake_queue(netdev);
+}
+
+static int gs_usb_set_bittiming(struct net_device *netdev)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct can_bittiming *bt = &dev->can.bittiming;
+	struct usb_interface *intf = dev->iface;
+	int rc;
+	struct gs_device_bittiming *dbt;
+
+	dbt = kmalloc(sizeof(*dbt), GFP_KERNEL);
+	if (!dbt)
+		return -ENOMEM;
+
+	dbt->prop_seg = bt->prop_seg;
+	dbt->phase_seg1 = bt->phase_seg1;
+	dbt->phase_seg2 = bt->phase_seg2;
+	dbt->sjw = bt->sjw;
+	dbt->brp = bt->brp;
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_BITTIMING,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			dbt,
+			sizeof(*dbt),
+			1000);
+	if (rc < 0)
+		dev_warn(netdev->dev.parent, "Couldn't set bittimings %d", rc);
+
+	kfree(dbt);
+
+	return rc;
+}
+
+static int gs_usb_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	int rc = -EOPNOTSUPP;
+	struct gs_device_mode *dm;
+
+	dev_warn(netdev->dev.parent, "CAN mode %d\n", mode);
+
+	dm = kmalloc(sizeof(*dm), GFP_KERNEL);
+	if (!dm)
+		return -ENOMEM;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		dm->mode = GS_CAN_MODE_RESET;
+
+		rc = usb_control_msg(interface_to_usbdev(intf),
+				usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+				GS_USB_BREQ_MODE,
+				USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+				dev->channel,
+				0,
+				dm,
+				sizeof(*dm),
+				1000);
+
+		if (rc < 0) {
+			dev_warn(netdev->dev.parent, "Couldn't start device %d",
+				rc);
+			break;
+		}
+
+		netif_wake_queue(netdev);
+		break;
+	case CAN_MODE_STOP:
+	case CAN_MODE_SLEEP:
+	default:
+		break;
+	}
+
+	kfree(dm);
+	return rc;
+}
+
+static int gs_usb_get_state(const struct net_device *netdev,
+enum can_state *state)
+{
+
+	struct gs_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	int rc;
+	struct gs_device_state *dstate;
+
+	dev_warn(netdev->dev.parent, "CAN state");
+
+	dstate = kmalloc(sizeof(*dstate), GFP_KERNEL);
+	if (!dstate)
+		return -ENOMEM;
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			dstate,
+			sizeof(*dstate),
+			1000);
+
+	if (rc < 0) {
+		dev_warn(netdev->dev.parent, "Couldn't get state %d", rc);
+		kfree(dstate);
+		return rc;
+	}
+
+	netdev_info(netdev, "state: %d\n", dstate->state);
+
+	switch (dstate->state) {
+	/* RX/TX error count < 96 */
+	case GS_CAN_STATE_ERROR_ACTIVE:
+		*state = CAN_STATE_ERROR_ACTIVE;
+		break;
+	/* RX/TX error count < 128 */
+	case GS_CAN_STATE_ERROR_WARNING:
+		*state = CAN_STATE_ERROR_WARNING;
+		break;
+	/* RX/TX error count < 256 */
+	case GS_CAN_STATE_ERROR_PASSIVE:
+		*state = CAN_STATE_ERROR_PASSIVE;
+		break;
+	/* RX/TX error count >= 256 */
+	case GS_CAN_STATE_BUS_OFF:
+		*state = CAN_STATE_BUS_OFF;
+		break;
+	/* Device is stopped */
+	case GS_CAN_STATE_STOPPED:
+		*state = CAN_STATE_STOPPED;
+		break;
+	/* Device is sleeping */
+	case GS_CAN_STATE_SLEEPING:
+		*state = CAN_STATE_SLEEPING;
+		break;
+	default:
+		kfree(dstate);
+		return -ENOTSUPP;
+	}
+
+	kfree(dstate);
+	return 0;
+}
+
+static int gs_usb_get_berr_counter(const struct net_device *netdev,
+struct can_berr_counter *bec)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct usb_interface *intf = dev->iface;
+	int rc;
+	struct gs_device_state *dstate;
+
+	dstate = kmalloc(sizeof(*dstate), GFP_KERNEL);
+	if (!dstate)
+		return -ENOMEM;
+
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_BERR,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			dstate,
+			sizeof(*dstate),
+			1000);
+	if (rc < 0) {
+		dev_warn(netdev->dev.parent, "Couldn't get error counters %d",
+			rc);
+		kfree(dstate);
+		return rc;
+	}
+
+	dev_info(netdev->dev.parent, "txerr: %d rxerr: %d\n",
+		dstate->txerr, dstate->rxerr);
+
+	bec->txerr = dstate->txerr;
+	bec->rxerr = dstate->rxerr;
+
+	kfree(dstate);
+	return 0;
+}
+
+static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
+struct net_device *netdev)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct net_device_stats *stats = &dev->netdev->stats;
+	struct urb *urb;
+	struct gs_host_frame *hf;
+	struct can_frame *cf;
+	int rc;
+	struct gs_tx_context *txc;
+
+	if (can_dropped_invalid_skb(netdev, skb))
+		return NETDEV_TX_OK;
+
+	/* find an empty context to keep track of transmission */
+	txc = gs_alloc_tx_context(dev);
+	if (!txc) {
+		/* dev_kfree_skb(skb); */
+		return NETDEV_TX_BUSY;
+	}
+
+	/* create a URB, and a buffer for it */
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		gs_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		goto nomem;
+	}
+
+	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
+		&urb->transfer_dma);
+	if (!hf) {
+		gs_free_tx_context(txc);
+		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		usb_free_urb(urb);
+		goto nomem;
+	}
+
+	cf = (struct can_frame *)skb->data;
+
+	hf->echo_id = gs_tx_context_id(txc);
+	hf->channel = dev->channel;
+
+	hf->can_id = CONVERT_FLAGS(cf->can_id);
+	hf->can_dlc = cf->can_dlc;
+	memcpy(hf->data, cf->data, 8);
+
+	usb_fill_bulk_urb(urb, dev->udev,
+			usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
+			hf,
+			sizeof(*hf),
+			gs_usb_xmit_callback,
+			txc);
+
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+	usb_anchor_urb(urb, &dev->tx_submitted);
+
+	can_put_echo_skb(skb, netdev, gs_tx_context_id(txc));
+
+	atomic_inc(&dev->active_tx_urbs);
+
+/*	printk(KERN_INFO "%d] xmit new %d\n",hf->channel,hf->echo_id); */
+
+	rc = usb_submit_urb(urb, GFP_ATOMIC);
+	if (unlikely(rc)) {			/* usb send failed */
+		dev_err(netdev->dev.parent, "usb_submit failed %d\n", rc);
+
+		can_free_echo_skb(netdev, gs_tx_context_id(txc));
+		gs_free_tx_context(txc);
+
+		usb_unanchor_urb(urb);
+		usb_free_coherent(dev->udev,
+			sizeof(*hf),
+			hf,
+			urb->transfer_dma);
+
+		dev_kfree_skb(skb);
+
+		atomic_dec(&dev->active_tx_urbs);
+
+		if (rc == -ENODEV) {
+			netif_device_detach(netdev);
+		} else {
+			dev_warn(netdev->dev.parent, "Failed tx_urb %d\n", rc);
+
+			stats->tx_dropped++;
+		}
+	} else {
+		netdev->trans_start = jiffies;
+
+		/* Slow down tx path */
+		if (atomic_read(&dev->active_tx_urbs) >= MAX_TX_URBS)
+			netif_stop_queue(netdev);
+	}
+
+	/* let usb core take care of this urb */
+	usb_free_urb(urb);
+
+	return NETDEV_TX_OK;
+
+nomem:
+	dev_kfree_skb(skb);
+	stats->tx_dropped++;
+	return NETDEV_TX_OK;
+}
+
+static int gs_can_open(struct net_device *netdev)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct gs_usb *parent = dev->parent;
+	int rc, i;
+	struct gs_device_mode *dm;
+	u32 ctrlmode;
+
+	/* common open */
+	rc = open_candev(netdev);
+	if (rc)
+		return rc;
+
+	if (atomic_add_return(1, &parent->active_channels) == 1) {
+		for (i = 0; i < MAX_RX_URBS; i++) {
+			struct urb *urb = NULL;
+			u8 *buf = NULL;
+
+			/* alloc rx urb */
+			urb = usb_alloc_urb(0, GFP_KERNEL);
+			if (!urb) {
+				dev_err(netdev->dev.parent,
+					"No memory left for URBs\n");
+				return -ENOMEM;
+			}
+
+			/* alloc rx buffer */
+			/* FIXME: free this memory */
+			buf = usb_alloc_coherent(dev->udev,
+				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;
+			}
+
+
+			/* fill, anchor, and submit rx urb */
+			usb_fill_bulk_urb(urb,
+					dev->udev,
+					usb_rcvbulkpipe(dev->udev,
+						GSUSB_ENDPOINT_IN),
+					buf,
+					BUFFER_SIZE,
+					gs_usb_recieve_bulk_callback,
+					parent
+					);
+			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+			usb_anchor_urb(urb, &parent->rx_submitted);
+
+			rc = usb_submit_urb(urb, GFP_KERNEL);
+			if (rc) {
+				if (rc == -ENODEV)
+					netif_device_detach(dev->netdev);
+
+				/* FIXME: free rx buffer/urb maybe? */
+				usb_unanchor_urb(urb);
+				break;
+			}
+
+			/* Drop reference,
+			 * USB core will take care of freeing it
+			 */
+			usb_free_urb(urb);
+		}
+	}
+
+	dm = kmalloc(sizeof(*dm), GFP_KERNEL);
+	if (!dm)
+		return -ENOMEM;
+
+	/* flags */
+	ctrlmode = dev->can.ctrlmode;
+	if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
+		dm->flags |= GS_USB_LOOPBACK;
+
+	if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
+		dm->flags |= GS_USB_LISTENONLY;
+
+	if (ctrlmode & CAN_CTRLMODE_ONE_SHOT)
+		dm->flags |= GS_USB_ONE_SHOT;
+
+	if (ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		dm->flags |= GS_USB_TRIPLE_SAMPLE;
+
+
+	/* finally start device */
+	dm->mode = GS_CAN_MODE_START;
+	rc = usb_control_msg(interface_to_usbdev(dev->iface),
+			usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0),
+			GS_USB_BREQ_MODE,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			dev->channel,
+			0,
+			dm,
+			sizeof(*dm),
+			1000);
+
+	kfree(dm);
+
+	if (rc < 0)
+		dev_warn(netdev->dev.parent, "Couldn't start device %d", rc);
+
+	netif_start_queue(netdev);
+
+	netdev_info(netdev, "CAN open\n");
+	return 0;
+}
+
+/* think: ip link set canx down */
+static int gs_can_close(struct net_device *netdev)
+{
+	struct gs_can *dev = netdev_priv(netdev);
+	struct gs_usb *parent = dev->parent;
+
+	netif_stop_queue(netdev);
+
+	/* Stop polling */
+	if (atomic_dec_and_test(&parent->active_channels)) {
+		usb_kill_anchored_urbs(&parent->rx_submitted);
+		/* FIXME: Set CAN controller to reset mode */
+	}
+
+	close_candev(netdev);
+
+	netdev_info(netdev, "CAN close\n");
+	return 0;
+}
+
+/******************************** USB ********************************/
+
+static const struct net_device_ops gs_usb_netdev_ops = {
+	.ndo_open = gs_can_open,
+	.ndo_stop = gs_can_close,
+	.ndo_start_xmit = gs_can_start_xmit,
+};
+
+static int gs_make_candev(struct gs_can **out, unsigned int channel,
+struct usb_interface *intf)
+{
+	struct gs_can *dev;
+	struct net_device *netdev;
+	int rc;
+	struct gs_device_bt_const *bt_const;
+
+	bt_const = kmalloc(sizeof(*bt_const), GFP_KERNEL);
+	if (!bt_const)
+		return -ENOMEM;
+
+	/* fetch bit timing constants */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_BT_CONST,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			channel,
+			0,
+			bt_const,
+			sizeof(*bt_const),
+			1000);
+
+	if (rc < 0) {
+		dev_err(&intf->dev,
+			"Couldn't get bit timing const for channel %d\n",
+			rc);
+		kfree(bt_const);
+		return rc;
+	}
+
+	dev_info(&intf->dev, "fclock: %d\n", bt_const->fclk_can);
+
+	/* create netdev */
+	netdev = alloc_candev(sizeof(struct gs_can), MAX_TX_URBS);
+	if (!netdev) {
+		dev_err(&intf->dev, "Couldn't alloc candev\n");
+		kfree(bt_const);
+		return -ENOMEM;
+	}
+
+	dev = netdev_priv(netdev);
+
+	netdev->netdev_ops = &gs_usb_netdev_ops;
+
+	netdev->flags |= IFF_ECHO; /* we support full roundtrip echo */
+
+	/* dev settup */
+	strcpy(dev->bt_const.name, "gs_usb");
+	dev->bt_const.tseg1_min = bt_const->tseg1_min;
+	dev->bt_const.tseg1_max = bt_const->tseg1_max;
+	dev->bt_const.tseg2_min = bt_const->tseg2_min;
+	dev->bt_const.tseg2_max = bt_const->tseg2_max;
+	dev->bt_const.sjw_max = bt_const->sjw_max;
+	dev->bt_const.brp_min = bt_const->brp_min;
+	dev->bt_const.brp_max = bt_const->brp_max;
+	dev->bt_const.brp_inc = bt_const->brp_inc;
+
+	dev->udev = interface_to_usbdev(intf);
+	dev->iface = intf;
+	dev->netdev = netdev;
+	dev->channel = channel;
+
+	init_usb_anchor(&dev->tx_submitted);
+	atomic_set(&dev->active_tx_urbs, 0);
+	for (rc = 0; rc < MAX_TX_URBS; rc++) {
+		dev->tx_context[rc].dev = dev;
+		dev->tx_context[rc].echo_id = MAX_TX_URBS;
+	}
+
+	/* can settup */
+	dev->can.state = CAN_STATE_STOPPED;
+	dev->can.clock.freq = bt_const->fclk_can;
+	dev->can.bittiming_const = &dev->bt_const;
+	dev->can.do_set_bittiming = gs_usb_set_bittiming;
+	dev->can.do_set_mode = gs_usb_set_mode;
+	dev->can.do_get_state = gs_usb_get_state;
+	dev->can.do_get_berr_counter = gs_usb_get_berr_counter;
+	dev->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
+					CAN_CTRLMODE_LOOPBACK |
+					CAN_CTRLMODE_LISTENONLY |
+					CAN_CTRLMODE_ONE_SHOT;
+
+	kfree(bt_const);
+
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
+	rc = register_candev(dev->netdev);
+	if (rc) {
+		free_candev(dev->netdev);
+		dev_err(&intf->dev, "Couldn't register candev\n");
+		return rc;
+	}
+
+	*out = dev;
+	return 0;
+}
+
+static void gs_destroy_candev(struct gs_can *dev)
+{
+	unregister_candev(dev->netdev);
+	free_candev(dev->netdev);
+}
+
+static int gs_usb_probe(struct usb_interface *intf,
+const struct usb_device_id *id)
+{
+	struct gs_usb *dev;
+	int rc = -ENOMEM;
+	unsigned int icount, i;
+	struct gs_host_config *hconf;
+	struct gs_device_config *dconf;
+
+	hconf = kmalloc(sizeof(*hconf), GFP_KERNEL);
+	if (!hconf)
+		return -ENOMEM;
+
+	hconf->byte_order = 0x0000beef;
+
+
+	/* send host config */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_HOST_FORMAT,
+			USB_DIR_OUT|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			1,
+			intf->altsetting[0].desc.bInterfaceNumber,
+			hconf,
+			sizeof(*hconf),
+			1000);
+
+	kfree(hconf);
+
+	if (rc < 0) {
+		dev_err(&intf->dev, "Couldn't send data format: %d\n",
+			rc);
+		return rc;
+	}
+
+
+	dconf = kmalloc(sizeof(*dconf), GFP_KERNEL);
+	if (!dconf)
+		return -ENOMEM;
+
+	/* read device config */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+			GS_USB_BREQ_DEVICE_CONFIG,
+			USB_DIR_IN|USB_TYPE_VENDOR|USB_RECIP_INTERFACE,
+			1,
+			intf->altsetting[0].desc.bInterfaceNumber,
+			dconf,
+			sizeof(*dconf),
+			1000);
+	if (rc < 0) {
+		dev_err(&intf->dev, "Couldn't recieve device config: %d\n",
+			rc);
+		return rc;
+	}
+
+	icount = (dconf->feature&GS_DCONF_IFACEMASK)+1;
+
+	kfree(dconf);
+
+	dev_info(&intf->dev, "Configuring for %d interfaces\n", icount);
+
+	if (icount > MAX_INTF) {
+		dev_err(&intf->dev,
+			"Driver cannot handle more that %d Can interfaces\n",
+			MAX_INTF);
+		return -EINVAL;
+	}
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	init_usb_anchor(&dev->rx_submitted);
+
+	atomic_set(&dev->active_channels, 0);
+
+	usb_set_intfdata(intf, dev);
+
+	for (i = 0; i < icount; i++) {
+		rc = gs_make_candev(&dev->canch[i], i, intf);
+		if (rc) {
+			icount = i;
+			for (i = 0; i < icount; i++) {
+				gs_destroy_candev(dev->canch[i]);
+				dev->canch[icount] = NULL;
+			}
+			return rc;
+		}
+		dev->canch[i]->parent = dev;
+	}
+
+	dev_info(&intf->dev, "Probe\n");
+	return 0;
+}
+
+static void gs_usb_disconnect(struct usb_interface *intf)
+{
+	unsigned i;
+	struct gs_usb *dev = usb_get_intfdata(intf);
+	usb_set_intfdata(intf, NULL);
+
+	if (!dev) {
+		dev_info(&intf->dev, "Disconnect (nodata)\n");
+		return;
+	}
+
+	for (i = 0; i < MAX_INTF; i++) {
+		struct gs_can *can = dev->canch[i];
+
+		if (!can)
+			continue;
+
+		unregister_netdev(can->netdev);
+		free_candev(can->netdev);
+
+		usb_kill_anchored_urbs(&can->tx_submitted);
+	}
+
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+
+	dev_info(&intf->dev, "Disconnect\n");
+}
+
+MODULE_DEVICE_TABLE(usb, gs_usb_table);
+static struct usb_device_id gs_usb_table[] = {
+	{USB_DEVICE(USB_GSUSB_1_VENDOR_ID, USB_GSUSB_1_PRODUCT_ID)},
+	{} /* Terminating entry */
+};
+
+static struct usb_driver gs_usb_driver = {
+	.name       = "gs_usb",
+	.probe      = gs_usb_probe,
+	.disconnect = gs_usb_disconnect,
+	.id_table   = gs_usb_table,
+	/* FIXME: suspend, resume, etc... */
+};
+
+static int __init gs_usb_init(void)
+{
+	return usb_register(&gs_usb_driver);
+}
+
+static void __exit gs_usb_exit(void)
+{
+	usb_deregister(&gs_usb_driver);
+}
+
+module_init(gs_usb_init);
+module_exit(gs_usb_exit);
+
+MODULE_AUTHOR("Maximilian Schneider <mws@schneidersoft.net>");
+MODULE_DESCRIPTION(
+"Socket CAN device driver for Geschwister Schneider Technologie-, "
+"Entwicklungs- und Vertriebs UG. USB2.0 to CAN interfaces.");
+MODULE_LICENSE("GPL v2");
+
-- 
1.7.10.4

Re: Usb to can driver

[Wolfgang Grandegger]

On 06/04/2013 03:18 PM, Max S. wrote:
> Hello all,
> 
> I've 'rebranded' the entire driver to Geschwister Schneider
> Technologie-, Entwicklungs- und Vertriebs UG. using gs_ for structs,
> enums and function names.
> 
> Regarding the dma on the stack errors, where do you see these messages? I have modified the code (see patch attached), and they should no longer appear.
> 
> I have sent an email to the openmoko people, and will update the product/vender code when i get a response.
> 
> There are still some things I'd like to work out:
> In the driver i do netdev->flags |= IFF_ECHO; to tell the netdev subsystem i will be handling message delivery tracking.
> via can_free_echo_skb, can_get_echo_skb, can_put_echo_skb...
> 
> I read in the 8devices driver:
> dev->can.ctrlmode_supported = ...
> 				CAN_CTRLMODE_LOOPBACK |
> 				...
> 
> An in documentation/networking/can.txt:
> To reflect the correct* traffic on the node the loopback of the sent data has to be performed right after a successful transmission.
> 
> what is the difference between IFF_ECHO and CAN_CTRLMODE_LOOPBACK? loopback is default behavior...
> How should my driver/device behave differently when loopback is not enabled?

CAN_CTRLMODE_LOOPBACK allows to use the CAN loopback of the controller,
if available. All CAN messages send to the bus will be looped back.

IFF_ECHO emulates the network behavior: messages are visible on the same
host by other sockets as well. This is usually a pure software loopback.

> Currently when the CAN controller fails to send a can frame, for example when there is no other node to ack it, my firmware will produce an error frame and queue it for the host.
> Since the can controller will immediately retry transmission, if the error condition persists this means error frames will continually be sent to the host.

That's normal (the infamous received no ACK on transmission bus error).

> Is this acceptable behavior? I think it would be better to find a way to throttle repetitive error frames.

Some CAN controller allow to disable bus error reporting to avoid
interrupt and error message flooding.

> I have done some testing with higher can speeds and run into the following kernel panic (netconsole extract):
> 
> [ 1784.427960] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
> occupied!
> [ 1784.493869] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
> occupied!
> [ 1784.745668] ------------[ cut here ]------------
> [ 1784.745701] kernel BUG
> at /build/buildd-linux_3.2.35-2-amd64-v9djlH/linux-3.2.35/drivers/net/can/dev.c:283!
> [ 1784.745719] invalid opcode: 0000 [#1] SMP 
> [ 1784.745740] CPU 2 
> [ 1784.745750] Modules linked in: can_raw can gs_usb(O) netconsole
> configfs can_dev ppdev lp rfcomm bnep bluetooth crc16 cpufreq_stats
> cpufreq_userspace cpufreq_powersave cpufreq_conservative fuse nfsd nfs
> nfs_acl auth_rpcgss fscache lockd sunrpc usbhid hid pcan(O) loop arc4
> rt2800usb rt2x00usb rt2800lib rt2x00lib mac80211 cfg80211 rfkill
> crc_ccitt evdev psmouse snd_hda_intel coretemp serio_raw snd_hda_codec
> i2c_i801 snd_hwdep pcspkr i2c_core snd_pcm snd_page_alloc snd_seq
> snd_seq_device snd_timer parport_pc iTCO_wdt parport iTCO_vendor_support
> snd uhci_hcd video acpi_cpufreq mperf ehci_hcd usbcore e1000e soundcore
> button usb_common processor thermal_sys ext2 mbcache sd_mod crc_t10dif
> ahci libahci libata scsi_mod
> [ 1784.746265] 
> [ 1784.746276] Pid: 7786, comm: cansequence Tainted: G           O
> 3.2.0-4-amd64 #1 Debian 3.2.35-2                  /DN2800MT
> [ 1784.746297] RIP: 0010:[<ffffffffa03d9552>]  [<ffffffffa03d9552>]
> can_put_echo_skb+0x16/0x106 [can_dev]
> [ 1784.746319] RSP: 0018:ffff880117265b38  EFLAGS: 00010246
> [ 1784.746327] RAX: ffff880117126938 RBX: ffff88012946e8c0 RCX:
> ffff88012a8da090
> [ 1784.746336] RDX: 000000000000000a RSI: ffff880117126000 RDI:
> ffff88012946e8c0
> [ 1784.746344] RBP: ffff880117126000 R08: ffff88012a8da090 R09:
> 0000000000000018
> [ 1784.746352] R10: 000000000000ffff R11: 0000000000000246 R12:
> ffff880037c102a0
> [ 1784.746361] R13: ffff88012946e8c0 R14: ffff880117126870 R15:
> ffff880117126000
> [ 1784.746370] FS:  00007fcfbc52c700(0000) GS:ffff88012fd00000(0000)
> knlGS:0000000000000000
> [ 1784.746379] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1784.746387] CR2: 00007fc450074e38 CR3: 0000000129e6f000 CR4:
> 00000000000006e0
> [ 1784.746396] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1784.746404] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [ 1784.746414] Process cansequence (pid: 7786, threadinfo
> ffff880117264000, task ffff880120f54140)
> [ 1784.746428] Stack:
> [ 1784.746438]  ffff880117126000 ffff88012742b680 ffff880037c102a0
> ffff88012946e8c0
> [ 1784.746475]  ffff880117126870 ffffffffa03f102a ffff88012908dbc0
> 00000000000f41ff
> [ 1784.746504]  0000000001000000 ffff88012946e8c0 0000000000000010
> ffff880117126000
> [ 1784.746527] Call Trace:
> [ 1784.746546]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
> [gs_usb]
> [ 1784.746569]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
> [ 1784.746589]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
> [ 1784.746607]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
> [ 1784.746625]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
> [ 1784.746644]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
> [ 1784.746665]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
> [ 1784.746685]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
> [ 1784.746705]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
> [ 1784.746726]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
> [ 1784.746746]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
> [ 1784.746769]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
> [ 1784.746790]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.746809]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
> [ 1784.746832]  [<ffffffff81163995>] ? security_file_permission
> +0x16/0x2d
> [ 1784.746852]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
> [ 1784.746872]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
> [ 1784.746895]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
> [ 1784.746910] Code: e0 48 8b 83 10 08 00 00 48 c7 04 e8 00 00 00 00 41
> 5b 5b 5d c3 41 56 41 55 41 54 55 48 89 f5 53 3b 96 08 08 00 00 48 89 fb
> 72 02 <0f> 0b f6 86 b2 01 00 00 04 0f 84 c8 00 00 00 8a 47 7d 83 e0 07 
> [ 1784.747301] RIP  [<ffffffffa03d9552>] can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747324]  RSP <ffff880117265b38>
> [ 1784.747345] ---[ end trace 811113d28006e99a ]---
> [ 1784.747357] Kernel panic - not syncing: Fatal exception in interrupt
> [ 1784.747368] Pid: 7786, comm: cansequence Tainted: G      D    O
> 3.2.0-4-amd64 #1 Debian 3.2.35-2
> [ 1784.747377] Call Trace:
> [ 1784.747390]  [<ffffffff813467bc>] ? panic+0x95/0x1a5
> [ 1784.747403]  [<ffffffff8134de86>] ? oops_end+0xa9/0xb6
> [ 1784.747416]  [<ffffffff8100e965>] ? do_invalid_op+0x87/0x91
> [ 1784.747431]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747444]  [<ffffffff81037e96>] ? set_next_entity+0x32/0x55
> [ 1784.747456]  [<ffffffff8100d025>] ? paravirt_write_msr+0xb/0xe
> [ 1784.747468]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.747480]  [<ffffffff810397dd>] ? finish_task_switch+0x4e/0xb9
> [ 1784.747493]  [<ffffffff81353feb>] ? invalid_op+0x1b/0x20
> [ 1784.747507]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747561]  [<ffffffffa01800b0>] ? usb_anchor_urb+0x25/0x65
> [usbcore]
> [ 1784.747577]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
> [gs_usb]
> [ 1784.747592]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
> [ 1784.747606]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
> [ 1784.747617]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
> [ 1784.747629]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
> [ 1784.747641]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
> [ 1784.747655]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
> [ 1784.747669]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
> [ 1784.747682]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
> [ 1784.747695]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
> [ 1784.747707]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
> [ 1784.747721]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
> [ 1784.747734]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.747746]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
> [ 1784.747763]  [<ffffffff81163995>] ? security_file_permission
> +0x16/0x2d
> [ 1784.747775]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
> [ 1784.747786]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
> [ 1784.747799]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
> [42043.689364] BUG: scheduling while atomic: swapper/0/0/0x10000100
> [42043.690985] Stack:
> [42043.691116] Call Trace:
> [42043.691211] Code: d2 65 48 8b 04 25 c8 c6 00 00 48 89 d1 48 2d c8 1f
> 00 00 0f 01 c8 0f ae f0 e8 c2 fc ff ff 85 c0 75 0b 31 c0 48 89 c1 fb 0f
> 01 c9 <eb> 05 e8 98 fc ff ff 65 8b 3c 25 08 dc 00 00 e8 9b fd ff ff 65
> 
> The first two lines tell me I'm not using can_put_echo_skb correctly.

Yep.

> I can only imagine there is a kind of race condition in the
> gs_*_tx_context group of functions, but i don't know how to solve it.

spin_lock_irqsave/spin_unlock_irqrestore

> The fact that the driver works fine at lower CAN speeds again makes me
> want to think race condition.
> 
> What do you think?

You could use function tracing and stop the trace on oops to understand
what's going on. Let me know if you need more help on that.

Wolfgang.

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 7908 bytes --]

On 06/04/2013 03:18 PM, Max S. wrote:
[...]
> [ 1784.427960] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
> occupied!
> [ 1784.493869] gs_usb 4-1:1.0: can_put_echo_skb: BUG! echo_skb is
> occupied!
> [ 1784.745668] ------------[ cut here ]------------
> [ 1784.745701] kernel BUG
> at /build/buildd-linux_3.2.35-2-amd64-v9djlH/linux-3.2.35/drivers/net/can/dev.c:283!
> [ 1784.745719] invalid opcode: 0000 [#1] SMP 
> [ 1784.745740] CPU 2 
> [ 1784.745750] Modules linked in: can_raw can gs_usb(O) netconsole
> configfs can_dev ppdev lp rfcomm bnep bluetooth crc16 cpufreq_stats
> cpufreq_userspace cpufreq_powersave cpufreq_conservative fuse nfsd nfs
> nfs_acl auth_rpcgss fscache lockd sunrpc usbhid hid pcan(O) loop arc4
> rt2800usb rt2x00usb rt2800lib rt2x00lib mac80211 cfg80211 rfkill
> crc_ccitt evdev psmouse snd_hda_intel coretemp serio_raw snd_hda_codec
> i2c_i801 snd_hwdep pcspkr i2c_core snd_pcm snd_page_alloc snd_seq
> snd_seq_device snd_timer parport_pc iTCO_wdt parport iTCO_vendor_support
> snd uhci_hcd video acpi_cpufreq mperf ehci_hcd usbcore e1000e soundcore
> button usb_common processor thermal_sys ext2 mbcache sd_mod crc_t10dif
> ahci libahci libata scsi_mod
> [ 1784.746265] 
> [ 1784.746276] Pid: 7786, comm: cansequence Tainted: G           O
> 3.2.0-4-amd64 #1 Debian 3.2.35-2                  /DN2800MT
> [ 1784.746297] RIP: 0010:[<ffffffffa03d9552>]  [<ffffffffa03d9552>]
> can_put_echo_skb+0x16/0x106 [can_dev]
> [ 1784.746319] RSP: 0018:ffff880117265b38  EFLAGS: 00010246
> [ 1784.746327] RAX: ffff880117126938 RBX: ffff88012946e8c0 RCX:
> ffff88012a8da090
> [ 1784.746336] RDX: 000000000000000a RSI: ffff880117126000 RDI:
> ffff88012946e8c0
> [ 1784.746344] RBP: ffff880117126000 R08: ffff88012a8da090 R09:
> 0000000000000018
> [ 1784.746352] R10: 000000000000ffff R11: 0000000000000246 R12:
> ffff880037c102a0
> [ 1784.746361] R13: ffff88012946e8c0 R14: ffff880117126870 R15:
> ffff880117126000
> [ 1784.746370] FS:  00007fcfbc52c700(0000) GS:ffff88012fd00000(0000)
> knlGS:0000000000000000
> [ 1784.746379] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1784.746387] CR2: 00007fc450074e38 CR3: 0000000129e6f000 CR4:
> 00000000000006e0
> [ 1784.746396] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1784.746404] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
> 0000000000000400
> [ 1784.746414] Process cansequence (pid: 7786, threadinfo
> ffff880117264000, task ffff880120f54140)
> [ 1784.746428] Stack:
> [ 1784.746438]  ffff880117126000 ffff88012742b680 ffff880037c102a0
> ffff88012946e8c0
> [ 1784.746475]  ffff880117126870 ffffffffa03f102a ffff88012908dbc0
> 00000000000f41ff
> [ 1784.746504]  0000000001000000 ffff88012946e8c0 0000000000000010
> ffff880117126000
> [ 1784.746527] Call Trace:
> [ 1784.746546]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
> [gs_usb]
> [ 1784.746569]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
> [ 1784.746589]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
> [ 1784.746607]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
> [ 1784.746625]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
> [ 1784.746644]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
> [ 1784.746665]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
> [ 1784.746685]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
> [ 1784.746705]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
> [ 1784.746726]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
> [ 1784.746746]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
> [ 1784.746769]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
> [ 1784.746790]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.746809]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
> [ 1784.746832]  [<ffffffff81163995>] ? security_file_permission
> +0x16/0x2d
> [ 1784.746852]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
> [ 1784.746872]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
> [ 1784.746895]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
> [ 1784.746910] Code: e0 48 8b 83 10 08 00 00 48 c7 04 e8 00 00 00 00 41
> 5b 5b 5d c3 41 56 41 55 41 54 55 48 89 f5 53 3b 96 08 08 00 00 48 89 fb
> 72 02 <0f> 0b f6 86 b2 01 00 00 04 0f 84 c8 00 00 00 8a 47 7d 83 e0 07 
> [ 1784.747301] RIP  [<ffffffffa03d9552>] can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747324]  RSP <ffff880117265b38>
> [ 1784.747345] ---[ end trace 811113d28006e99a ]---
> [ 1784.747357] Kernel panic - not syncing: Fatal exception in interrupt
> [ 1784.747368] Pid: 7786, comm: cansequence Tainted: G      D    O
> 3.2.0-4-amd64 #1 Debian 3.2.35-2
> [ 1784.747377] Call Trace:
> [ 1784.747390]  [<ffffffff813467bc>] ? panic+0x95/0x1a5
> [ 1784.747403]  [<ffffffff8134de86>] ? oops_end+0xa9/0xb6
> [ 1784.747416]  [<ffffffff8100e965>] ? do_invalid_op+0x87/0x91
> [ 1784.747431]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747444]  [<ffffffff81037e96>] ? set_next_entity+0x32/0x55
> [ 1784.747456]  [<ffffffff8100d025>] ? paravirt_write_msr+0xb/0xe
> [ 1784.747468]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.747480]  [<ffffffff810397dd>] ? finish_task_switch+0x4e/0xb9
> [ 1784.747493]  [<ffffffff81353feb>] ? invalid_op+0x1b/0x20
> [ 1784.747507]  [<ffffffffa03d9552>] ? can_put_echo_skb+0x16/0x106
> [can_dev]
> [ 1784.747561]  [<ffffffffa01800b0>] ? usb_anchor_urb+0x25/0x65
> [usbcore]
> [ 1784.747577]  [<ffffffffa03f102a>] ? gs_can_start_xmit+0x19a/0x299
> [gs_usb]
> [ 1784.747592]  [<ffffffff8128f2ff>] ? dev_hard_start_xmit+0x3cd/0x513
> [ 1784.747606]  [<ffffffff812a597f>] ? sch_direct_xmit+0x61/0x135
> [ 1784.747617]  [<ffffffff812a5b4d>] ? __qdisc_run+0xfa/0x112
> [ 1784.747629]  [<ffffffff8128f763>] ? dev_queue_xmit+0x31e/0x45b
> [ 1784.747641]  [<ffffffff810363d8>] ? should_resched+0x5/0x23
> [ 1784.747655]  [<ffffffffa03f6435>] ? can_send+0xe9/0x134 [can]
> [ 1784.747669]  [<ffffffff8127bca1>] ? sock_tx_timestamp+0x29/0x35
> [ 1784.747682]  [<ffffffffa04053a2>] ? raw_sendmsg+0xf3/0x130 [can_raw]
> [ 1784.747695]  [<ffffffff8127f7a4>] ? sock_update_classid+0x2c/0x42
> [ 1784.747707]  [<ffffffff8127d0e2>] ? sock_aio_write+0x121/0x135
> [ 1784.747721]  [<ffffffff810f95d4>] ? do_sync_write+0xb4/0xec
> [ 1784.747734]  [<ffffffff8100d6ec>] ? __switch_to+0x181/0x258
> [ 1784.747746]  [<ffffffff81039817>] ? finish_task_switch+0x88/0xb9
> [ 1784.747763]  [<ffffffff81163995>] ? security_file_permission
> +0x16/0x2d
> [ 1784.747775]  [<ffffffff810f9ccc>] ? vfs_write+0xa9/0xe9
> [ 1784.747786]  [<ffffffff810f9ea2>] ? sys_write+0x45/0x6b
> [ 1784.747799]  [<ffffffff81352012>] ? system_call_fastpath+0x16/0x1b
> [42043.689364] BUG: scheduling while atomic: swapper/0/0/0x10000100
> [42043.690985] Stack:
> [42043.691116] Call Trace:
> [42043.691211] Code: d2 65 48 8b 04 25 c8 c6 00 00 48 89 d1 48 2d c8 1f
> 00 00 0f 01 c8 0f ae f0 e8 c2 fc ff ff 85 c0 75 0b 31 c0 48 89 c1 fb 0f
> 01 c9 <eb> 05 e8 98 fc ff ff 65 8b 3c 25 08 dc 00 00 e8 9b fd ff ff 65
> 
> The first two lines tell me I'm not using can_put_echo_skb correctly.
> I can only imagine there is a kind of race condition in the
> gs_*_tx_context group of functions, but i don't know how to solve it.
> The fact that the driver works fine at lower CAN speeds again makes me
> want to think race condition.

http://lxr.free-electrons.com/source/drivers/net/can/dev.c?v=3.2#L283

The "index" parameter is bigger than the MAX_TX_URBS, as defined here:

+	netdev = alloc_candev(sizeof(struct gs_can), MAX_TX_URBS);

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Usb to can driver

[Sven Geggus]

Max S. <max@schneidersoft.net> wrote:

> $ lsusb -v -d beef:1234 # :P have yet to get proper USB vender code.

There is a possibility to get real USB vendor IDs from the Openmoko range for
open hardware projects.

Sven

-- 
Das Internet wird vor allem von Leuten genutzt, die sich Pornografie
ansehen, während sie Bier trinken, es ist daher für Wahlen nicht
geeignet (Jaroslaw Kaczynski)
/me is giggls@ircnet, http://sven.gegg.us/ on the Web

Usb to can driver

[Max S.]

Some more questions regarding usb to can driver.

When CAN_CTRLMODE_LISTENONLY is set; why does the start_xmit function still get called?
Its not really a problem, I'm just wondering why when a socket is listen only, it still attempts to TX...

My firmware implements LOOPBACK by default. That is all frames sent by
the can hardware are also returned over usb. I use this effect to call
can_get_echo_skb() during bulk in. To support LOOPBACK mode as per
socketcan; should i also call netif_rx() durring bulk in if CAN_CTRLMODE_LOOPBACK is
set?

I am currently using:
struct __packed gs_host_frame {
	u32 echo_id;
	u32 can_id;

	u32 can_dlc;
	u32 channel;
	u32 flags;

	u8 data[8];
};
as the data format between host/device. Using u32 for
memory alignment as suggested. Is there any way I can reduce the amount
of wasted space? At 1Mhz std rtr frame is ~21K frames/s. For two
channels both ways that's 21x2x2=84k frames/s. At 28 bytes per frame
that's 84x28x8=18.816Mbit/s too much for a USB full-speed device. I
want to reduce gs_host_frame size and get closer to 12Mbit/s.

What is the purpose of enum can_mode and set mode? I see that allot of drivers more or less ignore it.
reacting to only START but not SLEEP or STOP...

Max Schnedier.

Re: Usb to can driver

[wg]

Hi Max,



On Tue, 25 Jun 2013 23:59:18 +0000, "Max S." <max@schneidersoft.net>

wrote:

> Some more questions regarding usb to can driver.

> 

> When CAN_CTRLMODE_LISTENONLY is set; why does the start_xmit function

> still get called?

> Its not really a problem, I'm just wondering why when a socket is listen

> only, it still attempts to TX...



Good point. Would be more clever to not start the tx queue in that case.



> My firmware implements LOOPBACK by default. That is all frames sent by

> the can hardware are also returned over usb. I use this effect to call

> can_get_echo_skb() during bulk in. To support LOOPBACK mode as per

> socketcan; should i also call netif_rx() durring bulk in if

> CAN_CTRLMODE_LOOPBACK is

> set?



Message echo (normally via echo_skbs) and loopback (via

CAN_CTRLMODE_LOOPBACK)

are somehow different things. The first is a local loopback of sent

frames,

which is described here:



  http://lxr.linux.no/#linux+v3.9.7/Documentation/networking/can.txt#L178



Be aware that this feature can be switched off by the app via setsockopt

as

described in 4.1.3 and 4.1.4:



  http://lxr.linux.no/#linux+v3.9.7/Documentation/networking/can.txt#L496



Does the hardware communicate a TX done event as well?



CAN_CTRLMODE_LOOPBACK is a real hardware loopback. Normally the message

does

not even go out to the bus mainly useful for testing. For the software the

loopback message should be handled like any other incoming message.



> I am currently using:

> struct __packed gs_host_frame {

> 	u32 echo_id;

> 	u32 can_id;

> 

> 	u32 can_dlc;

> 	u32 channel;

> 	u32 flags;

> 

> 	u8 data[8];

> };

> as the data format between host/device. Using u32 for

> memory alignment as suggested. Is there any way I can reduce the amount

> of wasted space? At 1Mhz std rtr frame is ~21K frames/s. For two

> channels both ways that's 21x2x2=84k frames/s. At 28 bytes per frame

> that's 84x28x8=18.816Mbit/s too much for a USB full-speed device. I

> want to reduce gs_host_frame size and get closer to 12Mbit/s.



I think u8 is ok for echo_id, can_dlc, channel and flags. Just take care

that these 4 bytes are on a 32-bit boundary.



> What is the purpose of enum can_mode and set mode? I see that allot of

> drivers more or less ignore it.

> reacting to only START but not SLEEP or STOP...



It is used for bus-off-recovery as desribed here:



  http://lxr.linux.no/#linux+v3.9.7/Documentation/networking/can.txt#L891



Then CAN_MODE_START is used here:



  http://lxr.linux.no/#linux+v3.9.7/drivers/net/can/dev.c#L440



STOP and SLEEP are currently unused.



Wolfgang.

Re: Usb to can driver

[Max S.]

On Wed, 2013-06-26 at 09:10 +0200, wg wrote:
> Hi Max,
...
> > My firmware implements LOOPBACK by default. That is all frames sent by
> > the can hardware are also returned over usb. I use this effect to call
> > can_get_echo_skb() during bulk in. To support LOOPBACK mode as per
> > socketcan; should i also call netif_rx() durring bulk in if
> > CAN_CTRLMODE_LOOPBACK is
> > set?
> 
> Message echo (normally via echo_skbs) and loopback (via
> CAN_CTRLMODE_LOOPBACK)
> are somehow different things. The first is a local loopback of sent
> frames,
> which is described here:
> 
>   http://lxr.linux.no/#linux+v3.9.7/Documentation/networking/can.txt#L178

I was confused because i was not sure if 'sent frame' meant either a)
'frame that has left Linux', or b) 'frame that actually made it onto the
CAN bus'. Appears I can choose either :P

For usb_8dev and ems_usb, echo via put_echo_skb happens in the USB write
callback. (a)

For esd_usb2 a frame is only recognized as sent when the device reports
it as sent. (b)

I also noticed esd_usb2 does: dev->nets[msg->msg.txdone.net]
Is that safe? msg->msg.txdone.net is never check if its in range...

> Does the hardware communicate a TX done event as well?
Yes, The controller produces a TX event which happens after successful
frame transmission. I place successfully TXed frames in the USB IN fifo
with their corresponding echo_ids.

> CAN_CTRLMODE_LOOPBACK is a real hardware loopback. Normally the message
> does not even go out to the bus mainly useful for testing. For the software the
> loopback message should be handled like any other incoming message.

Ah. so CAN_CTRLMODE_LOOPBACK could be used to test data path from
application->Linux->driver->can_hardware and back. without the need of
an actual physical CAN bus.

Thanks Wolfgang,
Max Schneider.

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

On 06/26/2013 08:55 PM, Max S. wrote:
>>> My firmware implements LOOPBACK by default. That is all frames sent by
>>> the can hardware are also returned over usb. I use this effect to call
>>> can_get_echo_skb() during bulk in. To support LOOPBACK mode as per
>>> socketcan; should i also call netif_rx() durring bulk in if
>>> CAN_CTRLMODE_LOOPBACK is
>>> set?
>>
>> Message echo (normally via echo_skbs) and loopback (via
>> CAN_CTRLMODE_LOOPBACK)
>> are somehow different things. The first is a local loopback of sent
>> frames,
>> which is described here:
>>
>>   http://lxr.linux.no/#linux+v3.9.7/Documentation/networking/can.txt#L178
> 
> I was confused because i was not sure if 'sent frame' meant either a)
> 'frame that has left Linux', or b) 'frame that actually made it onto the
> CAN bus'. Appears I can choose either :P

b) is preferred

> For usb_8dev and ems_usb, echo via put_echo_skb happens in the USB write
> callback. (a)
> 
> For esd_usb2 a frame is only recognized as sent when the device reports
> it as sent. (b)
> 
> I also noticed esd_usb2 does: dev->nets[msg->msg.txdone.net]
> Is that safe? msg->msg.txdone.net is never check if its in range...

Can you send a patch? :)

>> Does the hardware communicate a TX done event as well?
> Yes, The controller produces a TX event which happens after successful
> frame transmission. I place successfully TXed frames in the USB IN fifo
> with their corresponding echo_ids.

This means you implement b).

>> CAN_CTRLMODE_LOOPBACK is a real hardware loopback. Normally the message
>> does not even go out to the bus mainly useful for testing. For the software the
>> loopback message should be handled like any other incoming message.
> 
> Ah. so CAN_CTRLMODE_LOOPBACK could be used to test data path from
> application->Linux->driver->can_hardware and back. without the need of
> an actual physical CAN bus.

Yes, this is a usecase.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: Usb to can driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1693 bytes --]

On 07/03/2013 02:14 AM, Max S. wrote:
>> > Can you send a patch? :)
>> > 
> sure. attached.

Thanks. Please the mailinglist for all linux can related patches.

> However, since i don't have the hardware; I can't test this.

Me neither, but the patch looks good.

> From f6f2ec1b1c2fa4f2d29fb48e8a315ff814ed2be9 Mon Sep 17 00:00:00 2001
> From: Maximilian Schneider <max@schneidersoft.net>
> Date: Tue, 2 Jul 2013 23:43:29 +0000
> Subject: [PATCH] use of raw input in net lookup
> 
> 
> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
> ---
>  drivers/net/can/usb/esd_usb2.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 6aa7b32..ee89b19 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -412,10 +412,16 @@ static void esd_usb2_read_bulk_callback(struct urb *urb)
>  
>  		switch (msg->msg.hdr.cmd) {
>  		case CMD_CAN_RX:
> +			if (msg->msg.rx.net >= dev->net_count)

I've added an error message here.

> +				break;
> +
>  			esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
>  			break;
>  
>  		case CMD_CAN_TX:
> +			if (msg->msg.txdone.net >= dev->net_count)

and here, too.

> +				break;
> +
>  			esd_usb2_tx_done_msg(dev->nets[msg->msg.txdone.net],
>  					     msg);
>  			break;
> -- 1.7.10.4
> 

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]