Netdev List
 help / color / mirror / Atom feed
* checkpatch run on net/ethernet
From: matthew @ 2012-10-02 11:41 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, davem


This is my first patch, it's just a checkpatch run. I figured I would also clean up the code as there appears to be long-since-obsoleted comments and code in there.

A fairly trivial change, but worthy of inclusion IMO all the same!

Matthew Walster

^ permalink raw reply

* [PATCH 1/2] net: ethernet: clean out braces / old code (found via checkpatch)
From: matthew @ 2012-10-02 11:41 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, davem, Matthew Walster
In-Reply-To: <1349178105-28269-1-git-send-email-matthew@walster.org>

From: Matthew Walster <matthew@walster.org>

Remove an old commented out piece of code.
Remove an if(true) statement.
Remove a set of braces that weren't strictly necessary.

All found by running checkpatch.pl against the code.

Signed-off-by: Matthew Walster <matthew@walster.org>
---
 net/ethernet/eth.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 4efad53..a9f8531 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -178,11 +178,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
 	 *      seems to set IFF_PROMISC.
 	 */
 
-	else if (1 /*dev->flags&IFF_PROMISC */ ) {
-		if (unlikely(!ether_addr_equal_64bits(eth->h_dest,
-						      dev->dev_addr)))
-			skb->pkt_type = PACKET_OTHERHOST;
-	}
+	else if (unlikely(!ether_addr_equal_64bits(eth->h_dest, dev->dev_addr)))
+		skb->pkt_type = PACKET_OTHERHOST;
 
 	/*
 	 * Some variants of DSA tagging don't have an ethertype field
-- 
1.7.10.4

^ permalink raw reply related

* Re: tg3 driver upgrade (Linux 2.6.32 -> 3.2) breaks IBM Bladecenter SoL
From: Ferenc Wagner @ 2012-10-02  9:31 UTC (permalink / raw)
  To: Michael Chan
  Cc: netdev, Matt Carlson, Grant Likely, Rob Herring, linux-kernel
In-Reply-To: <1349083982.5420.16.camel@LTIRV-MCHAN1.corp.ad.broadcom.com>

"Michael Chan" <mchan@broadcom.com> writes:

> On Fri, 2012-09-28 at 22:45 +0200, Ferenc Wagner wrote: 
> 
>> Upgrading the kernel on our HS20 blades resulted in their SoL (serial
>> over LAN) connection being broken.  The disconnection happens when eth0
>> (the interface involved in SoL) is brought up during the boot sequence.
>> If I later "ip link set eth0 down", then the connection is restored, but
>> "ip link set eth0 up" breaks it again on 3.2.  ethtool -a, -c, -g, -k
>> and -u show no difference; ethtool -i on the 2.6.32 kernel reports:
>> 
>> driver: tg3
>> version: 3.116
>> firmware-version: 5704s-v3.38, ASFIPMIs v2.47
>> bus-info: 0000:05:01.0
>> 
>> In the 3.2 kernel the driver version is 3.121.
>
> 2.6.32 to 3.2 is a big jump.  Can you narrow this down further?  It will
> be hard for us to find a HS20 with 5704 to test this.  Thanks.

Certainly, I'm bisecting it now, but I thought I would drop in the
question in case it rings some bells somewhere.  Given the nature of the
problem, it isn't much fun to bisect, and the stripped down kernel I'm
testing with breaks the SoL connection for a couple of seconds even in
the "good" cases.  I'm already down to 13 steps...
-- 
Thanks,
Feri.

^ permalink raw reply

* [PATCH net-next v2] netxen: write IP address to firmware when using bonding
From: Nikolay Aleksandrov @ 2012-10-02  9:16 UTC (permalink / raw)
  To: sony.chacko; +Cc: agospoda, rajesh.borundia, davem, netdev
In-Reply-To: <1348562883-14780-1-git-send-email-nikolay@redhat.com>

This patch allows LRO aggregation on bonded devices that contain an NX3031
device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
which executes for each slave that has bond as master.

V2: Remove local ip caching, retrieve addresses dynamically and
    restore them if necessary.

Note: Tested with NX3031 adapter.

Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |   6 -
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 192 +++++++++++----------
 include/linux/netdevice.h                          |   3 +
 3 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
index eb3dfdb..cb4f2ce 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
@@ -955,11 +955,6 @@ typedef struct nx_mac_list_s {
 	uint8_t mac_addr[ETH_ALEN+2];
 } nx_mac_list_t;
 
-struct nx_vlan_ip_list {
-	struct list_head list;
-	__be32 ip_addr;
-};
-
 /*
  * Interrupt coalescing defaults. The defaults are for 1500 MTU. It is
  * adjusted based on configured MTU.
@@ -1605,7 +1600,6 @@ struct netxen_adapter {
 	struct net_device *netdev;
 	struct pci_dev *pdev;
 	struct list_head mac_list;
-	struct list_head vlan_ip_list;
 
 	spinlock_t tx_clean_lock;
 
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index e2a4858..8e3eb61 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -90,7 +90,6 @@ static irqreturn_t netxen_intr(int irq, void *data);
 static irqreturn_t netxen_msi_intr(int irq, void *data);
 static irqreturn_t netxen_msix_intr(int irq, void *data);
 
-static void netxen_free_vlan_ip_list(struct netxen_adapter *);
 static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
 static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
 						      struct rtnl_link_stats64 *stats);
@@ -1451,7 +1450,6 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	spin_lock_init(&adapter->tx_clean_lock);
 	INIT_LIST_HEAD(&adapter->mac_list);
-	INIT_LIST_HEAD(&adapter->vlan_ip_list);
 
 	err = netxen_setup_pci_map(adapter);
 	if (err)
@@ -1586,7 +1584,7 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
 
 	cancel_work_sync(&adapter->tx_timeout_task);
 
-	netxen_free_vlan_ip_list(adapter);
+	netxen_restore_indev_addr(netdev, NETDEV_DOWN);
 	netxen_nic_detach(adapter);
 
 	nx_decr_dev_ref_cnt(adapter);
@@ -3135,66 +3133,22 @@ netxen_destip_supported(struct netxen_adapter *adapter)
 	return 1;
 }
 
-static void
-netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
+static inline __be32
+netxen_get_addr(struct net_device *dev)
 {
-	struct nx_vlan_ip_list  *cur;
-	struct list_head *head = &adapter->vlan_ip_list;
-
-	while (!list_empty(head)) {
-		cur = list_entry(head->next, struct nx_vlan_ip_list, list);
-		netxen_config_ipaddr(adapter, cur->ip_addr, NX_IP_DOWN);
-		list_del(&cur->list);
-		kfree(cur);
-	}
-
-}
-static void
-netxen_list_config_vlan_ip(struct netxen_adapter *adapter,
-		struct in_ifaddr *ifa, unsigned long event)
-{
-	struct net_device *dev;
-	struct nx_vlan_ip_list *cur, *tmp_cur;
-	struct list_head *head;
-
-	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
-
-	if (dev == NULL)
-		return;
-
-	if (!is_vlan_dev(dev))
-		return;
+	struct in_device *in_dev;
+	__be32 addr = 0;
 
-	switch (event) {
-	case NX_IP_UP:
-		list_for_each(head, &adapter->vlan_ip_list) {
-			cur = list_entry(head, struct nx_vlan_ip_list, list);
+	rcu_read_lock();
+	in_dev = __in_dev_get_rcu(dev);
 
-			if (cur->ip_addr == ifa->ifa_address)
-				return;
-		}
+	if (in_dev)
+		addr = inet_confirm_addr(in_dev, 0, 0, RT_SCOPE_HOST);
 
-		cur = kzalloc(sizeof(struct nx_vlan_ip_list), GFP_ATOMIC);
-		if (cur == NULL) {
-			printk(KERN_ERR "%s: failed to add vlan ip to list\n",
-					adapter->netdev->name);
-			return;
-		}
-
-		cur->ip_addr = ifa->ifa_address;
-		list_add_tail(&cur->list, &adapter->vlan_ip_list);
-		break;
-	case NX_IP_DOWN:
-		list_for_each_entry_safe(cur, tmp_cur,
-					&adapter->vlan_ip_list, list) {
-			if (cur->ip_addr == ifa->ifa_address) {
-				list_del(&cur->list);
-				kfree(cur);
-				break;
-			}
-		}
-	}
+	rcu_read_unlock();
+	return addr;
 }
+
 static void
 netxen_config_indev_addr(struct netxen_adapter *adapter,
 		struct net_device *dev, unsigned long event)
@@ -3213,12 +3167,10 @@ netxen_config_indev_addr(struct netxen_adapter *adapter,
 		case NETDEV_UP:
 			netxen_config_ipaddr(adapter,
 					ifa->ifa_address, NX_IP_UP);
-			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
 			break;
 		case NETDEV_DOWN:
 			netxen_config_ipaddr(adapter,
 					ifa->ifa_address, NX_IP_DOWN);
-			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
 			break;
 		default:
 			break;
@@ -3233,15 +3185,54 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
 
 {
 	struct netxen_adapter *adapter = netdev_priv(netdev);
-	struct nx_vlan_ip_list *pos, *tmp_pos;
+	struct net_device *vlan_dev, *real_dev;
 	unsigned long ip_event;
+	__be32 addr = 0;
 
 	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
 	netxen_config_indev_addr(adapter, netdev, event);
 
-	list_for_each_entry_safe(pos, tmp_pos, &adapter->vlan_ip_list, list) {
-		netxen_config_ipaddr(adapter, pos->ip_addr, ip_event);
+	rcu_read_lock();
+	for_each_netdev_rcu(&init_net, vlan_dev) {
+		if (vlan_dev->priv_flags & IFF_802_1Q_VLAN) {
+			real_dev = vlan_dev_real_dev(vlan_dev);
+
+			if (real_dev == netdev) {
+				addr = netxen_get_addr(vlan_dev);
+
+				if (addr) {
+					netxen_config_ipaddr(adapter, addr,
+							     ip_event);
+				}
+			}
+		}
 	}
+
+	if (netdev->master) {
+		addr = netxen_get_addr(netdev->master);
+		if (addr)
+			netxen_config_ipaddr(adapter, addr, ip_event);
+	}
+	rcu_read_unlock();
+}
+
+static inline int
+netxen_config_checkdev(struct net_device *dev)
+{
+	struct netxen_adapter *adapter;
+
+	if (!is_netxen_netdev(dev))
+		return -ENODEV;
+
+	adapter = netdev_priv(dev);
+
+	if (!adapter)
+		return -ENODEV;
+
+	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+		return -ENODEV;
+
+	return 0;
 }
 
 static int netxen_netdev_event(struct notifier_block *this,
@@ -3260,18 +3251,26 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
 
-	if (!adapter)
-		goto done;
-
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+			adapter = netdev_priv(slave);
+			netxen_config_indev_addr(adapter, orig_dev, event);
+		}
+		rcu_read_unlock();
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
 
-	netxen_config_indev_addr(adapter, orig_dev, event);
+		adapter = netdev_priv(dev);
+		netxen_config_indev_addr(adapter, orig_dev, event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3282,10 +3281,11 @@ netxen_inetaddr_event(struct notifier_block *this,
 {
 	struct netxen_adapter *adapter;
 	struct net_device *dev;
-
+	unsigned long ip_event;
 	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
 
 	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
+	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
 
 recheck:
 	if (dev == NULL)
@@ -3296,30 +3296,35 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
 
-	if (!adapter || !netxen_destip_supported(adapter))
-		goto done;
+			adapter = netdev_priv(slave);
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+			if (!netxen_destip_supported(adapter))
+				continue;
 
-	switch (event) {
-	case NETDEV_UP:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
-		break;
-	case NETDEV_DOWN:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
-		break;
-	default:
-		break;
-	}
+			netxen_config_ipaddr(adapter, ifa->ifa_address,
+					     ip_event);
+		}
+		rcu_read_unlock();
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
+
+		adapter = netdev_priv(dev);
 
+		if (!netxen_destip_supported(adapter))
+			goto done;
+
+		netxen_config_ipaddr(adapter, ifa->ifa_address, ip_event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3335,9 +3340,6 @@ static struct notifier_block netxen_inetaddr_cb = {
 static void
 netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
 { }
-static void
-netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
-{ }
 #endif
 
 static struct pci_error_handlers netxen_err_handler = {
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 59dc05f3..463bb40 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1578,6 +1578,9 @@ extern rwlock_t				dev_base_lock;		/* Device list lock */
 		list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
 #define for_each_netdev_continue_rcu(net, d)		\
 	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_in_bond_rcu(bond, slave)	\
+	for_each_netdev_rcu(&init_net, slave)		\
+		if (slave->master == bond)
 #define net_device_entry(lh)	list_entry(lh, struct net_device, dev_list)
 
 static inline struct net_device *next_net_device(struct net_device *dev)
-- 
1.7.11.4

^ permalink raw reply related

* [PATCH v2] ipv6: don't add link local route when there is no link local address
From: Nicolas Dichtel @ 2012-10-02  9:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, yoshfuji, Nicolas Dichtel
In-Reply-To: <20121001.165527.1025640918619977740.davem@davemloft.net>

When an address is added on loopback (ip -6 a a 2002::1/128 dev lo), a route
to fe80::/64 is added in the main table:
  unreachable fe80::/64 dev lo  proto kernel  metric 256  error -101

This route does not match any prefix (no fe80:: address on lo). In fact,
addrconf_dev_config() will not add link local address because this function
filters interfaces by type. If the link local address is added manually, the
route to the link local prefix will be automatically added by
addrconf_add_linklocal().
Note also, that this route is not deleted when the address is removed.

After looking at the code, it seems that addrconf_add_lroute() is redundant with
addrconf_add_linklocal(), because this function will add the link local route
when the link local address is configured.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v2: remove useless curly braces after the if statement in
    addrconf_sit_config()

 net/ipv6/addrconf.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ea3e9af..caa8035 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1782,14 +1782,6 @@ static void sit_route_add(struct net_device *dev)
 }
 #endif
 
-static void addrconf_add_lroute(struct net_device *dev)
-{
-	struct in6_addr addr;
-
-	ipv6_addr_set(&addr,  htonl(0xFE800000), 0, 0, 0);
-	addrconf_prefix_route(&addr, 64, dev, 0, 0);
-}
-
 static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
 {
 	struct inet6_dev *idev;
@@ -1807,8 +1799,6 @@ static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
 	if (!(dev->flags & IFF_LOOPBACK))
 		addrconf_add_mroute(dev);
 
-	/* Add link local route */
-	addrconf_add_lroute(dev);
 	return idev;
 }
 
@@ -2487,10 +2477,9 @@ static void addrconf_sit_config(struct net_device *dev)
 
 	sit_add_v4_addrs(idev);
 
-	if (dev->flags&IFF_POINTOPOINT) {
+	if (dev->flags&IFF_POINTOPOINT)
 		addrconf_add_mroute(dev);
-		addrconf_add_lroute(dev);
-	} else
+	else
 		sit_route_add(dev);
 }
 #endif
-- 
1.7.12

^ permalink raw reply related

* Re: net/ipv4/ipconfig.c:1590:2: error: implicit declaration of function 'ic_nameservers_predef'
From: Fengguang Wu @ 2012-10-02  8:54 UTC (permalink / raw)
  To: Rami Rosen; +Cc: Christoph Fritz, kernel-janitors, netdev, andriy.shevchenko
In-Reply-To: <CAKoUArkDK8sQ=DRgrPhEnnWPsdMkgNtZE6S2o5rx6tXhfB3SsA@mail.gmail.com>

On Tue, Oct 02, 2012 at 08:24:05AM +0200, Rami Rosen wrote:
> Hi,
> commit 842b08bbee448b2069aaebb7f18b40942ad2a1bd
> fixes it.
> 
> git show 842b08bbee448b2069aaebb7f18b40942ad2a1bd
> 
> commit 842b08bbee448b2069aaebb7f18b40942ad2a1bd
> 
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Mon Sep 24 22:09:58 2012 +0000
> 
>     ipconfig: fix trivial build error
> 
>     The commit 5e953778a2aab04929a5e7b69f53dc26e39b079e ("ipconfig: add nameserv
>     IPs to kernel-parameter ip=") introduces ic_nameservers_predef() that define
>     only for BOOTP. However it is used by ip_auto_config_setup() as well. This
>     patch moves it outside of #ifdef BOOTP.

Sorry, I get this bug while doing bisectibility tests on the linux-can
tree.  Obviously I should not count the net tree's commits as part of
the linux-can tree. Fixed.

Thanks,
Fengguang

> On Tue, Oct 2, 2012 at 5:27 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
> > Hi Christoph,
> >
> > FYI, kernel build failed on
> >
> > commit: 5e953778a2aab04929a5e7b69f53dc26e39b079e ipconfig: add nameserver IPs to kernel-parameter ip=
> > config: i386-randconfig-b106 (attached as .config)
> >
> > All error/warnings:
> >
> > net/ipv4/ipconfig.c: In function 'ip_auto_config_setup':
> > net/ipv4/ipconfig.c:1590:2: error: implicit declaration of function 'ic_nameservers_predef' [-Werror=implicit-function-declaration]
> > cc1: some warnings being treated as errors
> >
> > vim +1590 net/ipv4/ipconfig.c
> >
> > 92ffb85d (Amos Waterland  2008-01-05  1578)      */
> > ^1da177e (Linus Torvalds  2005-04-16  1579)     if (ic_proto_name(addrs))
> > ^1da177e (Linus Torvalds  2005-04-16  1580)             return 1;
> > ^1da177e (Linus Torvalds  2005-04-16  1581)
> > 92ffb85d (Amos Waterland  2008-01-05  1582)     /* If no static IP is given, turn off autoconfig and bail.  */
> > 92ffb85d (Amos Waterland  2008-01-05  1583)     if (*addrs == 0 ||
> > 92ffb85d (Amos Waterland  2008-01-05  1584)         strcmp(addrs, "off") == 0 ||
> > 92ffb85d (Amos Waterland  2008-01-05  1585)         strcmp(addrs, "none") == 0) {
> > 92ffb85d (Amos Waterland  2008-01-05  1586)             ic_enable = 0;
> > 92ffb85d (Amos Waterland  2008-01-05  1587)             return 1;
> > 92ffb85d (Amos Waterland  2008-01-05  1588)     }
> > 92ffb85d (Amos Waterland  2008-01-05  1589)
> > 5e953778 (Christoph Fritz 2012-09-21 @1590)     ic_nameservers_predef();
> > 5e953778 (Christoph Fritz 2012-09-21  1591)
> > 92ffb85d (Amos Waterland  2008-01-05  1592)     /* Parse string for static IP assignment.  */
> > ^1da177e (Linus Torvalds  2005-04-16  1593)     ip = addrs;
> > ^1da177e (Linus Torvalds  2005-04-16  1594)     while (ip && *ip) {
> > ^1da177e (Linus Torvalds  2005-04-16  1595)             if ((cp = strchr(ip, ':')))
> > ^1da177e (Linus Torvalds  2005-04-16  1596)                     *cp++ = '\0';
> >
> > ---
> > 0-DAY kernel build testing backend         Open Source Technology Centre
> > Fengguang Wu, Yuanhan Liu                              Intel Corporation

^ permalink raw reply

* Re: drivers/net/ethernet/chelsio/cxgb4/t4_hw.c:363:64: sparse: incorrect type in argument 3 (different base types)
From: Dan Carpenter @ 2012-10-02  8:46 UTC (permalink / raw)
  To: Jay Hernandez; +Cc: Fengguang Wu, Vipul Pandya, kernel-janitors, netdev
In-Reply-To: <8A71B368A89016469F72CD08050AD3340C527934@maui.asicdesigners.com>

On Mon, Oct 01, 2012 at 05:48:55PM -0700, Jay Hernandez wrote:
> Hi Fengguang,
> 
> Thanks for pointing this out we have a fix which addresses these issues.
> What's interesting is I did not see these warnings I tried to reproduce
> the errors on my setup. Is there a special flag we can use to reproduce
> the sparse warnings.
> 

Yup.

http://lwn.net/Articles/205624/

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH net, 3/3] hyperv: Fix page buffer handling in rndis_filter_send_request()
From: Dan Carpenter @ 2012-10-02  8:38 UTC (permalink / raw)
  To: Haiyang Zhang; +Cc: olaf, netdev, jasowang, linux-kernel, devel, davem
In-Reply-To: <1349130657-7987-3-git-send-email-haiyangz@microsoft.com>

On Mon, Oct 01, 2012 at 03:30:57PM -0700, Haiyang Zhang wrote:
> Add another page buffer if the request message crossed page boundary.
> 

What are the user visible effects of this bug fix?  Please put that
in the commit message.

regards,
dan carpenter

^ permalink raw reply

* RE: [PATCH 3/3] net/mlx4_en: Add HW timestamping (TS) support
From: Yevgeny Petrilin @ 2012-10-02  7:32 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Eugenia Emantayev
In-Reply-To: <1349104125.2577.7.camel@bwh-desktop.uk.solarflarecom.com>

> > +
> > +	if (rx_filter != HWTSTAMP_FILTER_NONE)
> > +		dev->features &= ~NETIF_F_HW_VLAN_RX;
> > +	else
> > +		dev->features |= NETIF_F_HW_VLAN_RX;
> [...]
> 
> If you change dev->features you should also call
> netdev_features_change(dev) (holding only the RTNL lock, so after you
> unlock mdev->state_lock).
> 
> Ben.
> 
Thanks Ben,
Will fix.

Yevgeny

^ permalink raw reply

* [PATCH v5] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-10-02  7:16 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can, Kvaser Support
  Cc: netdev, linux-usb, Olivier Sobrie, Daniel Berglund
In-Reply-To: <1343626352-24760-1-git-send-email-olivier@sobrie.be>

This driver provides support for several Kvaser CAN/USB devices.
Such kind of devices supports up to three CAN network interfaces.

It has been tested with a Kvaser USB Leaf Light (one network interface)
connected to a pch_can interface.
The firmware version of the Kvaser device was 2.5.205.

List of Kvaser devices supported by the driver:
  - Kvaser Leaf Light
  - Kvaser Leaf Professional HS
  - Kvaser Leaf SemiPro HS
  - Kvaser Leaf Professional LS
  - Kvaser Leaf Professional SWC
  - Kvaser Leaf Professional LIN
  - Kvaser Leaf SemiPro LS
  - Kvaser Leaf SemiPro SWC
  - Kvaser Memorator II HS/HS
  - Kvaser USBcan Professional HS/HS
  - Kvaser Leaf Light GI
  - Kvaser Leaf Professional HS (OBD-II connector)
  - Kvaser Memorator Professional HS/LS
  - Kvaser Leaf Light "China"
  - Kvaser BlackBird SemiPro
  - Kvaser USBcan R

Signed-off-by: Daniel Berglund <db@kvaser.com>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
Hi,

This new patch fixes the errors pointed out by Marc and Wolfgang.

Changes since v4:
 - add missing usb_free_urb()
 - put error message in a separate function
 - handle return code of kvaser_usb_init_one() in probe function

Olivier

 drivers/net/can/usb/Kconfig      |   29 +
 drivers/net/can/usb/Makefile     |    1 +
 drivers/net/can/usb/kvaser_usb.c | 1596 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 1626 insertions(+)
 create mode 100644 drivers/net/can/usb/kvaser_usb.c

diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index 0a68768..a4e4bee 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -13,6 +13,35 @@ config CAN_ESD_USB2
           This driver supports the CAN-USB/2 interface
           from esd electronic system design gmbh (http://www.esd.eu).
 
+config CAN_KVASER_USB
+	tristate "Kvaser CAN/USB interface"
+	---help---
+	  This driver adds support for Kvaser CAN/USB devices like Kvaser
+	  Leaf Light.
+
+	  The driver gives support for the following devices:
+	    - Kvaser Leaf Light
+	    - Kvaser Leaf Professional HS
+	    - Kvaser Leaf SemiPro HS
+	    - Kvaser Leaf Professional LS
+	    - Kvaser Leaf Professional SWC
+	    - Kvaser Leaf Professional LIN
+	    - Kvaser Leaf SemiPro LS
+	    - Kvaser Leaf SemiPro SWC
+	    - Kvaser Memorator II HS/HS
+	    - Kvaser USBcan Professional HS/HS
+	    - Kvaser Leaf Light GI
+	    - Kvaser Leaf Professional HS (OBD-II connector)
+	    - Kvaser Memorator Professional HS/LS
+	    - Kvaser Leaf Light "China"
+	    - Kvaser BlackBird SemiPro
+	    - Kvaser USBcan R
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called kvaser_usb.
+
 config CAN_PEAK_USB
 	tristate "PEAK PCAN-USB/USB Pro interfaces"
 	---help---
diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index da6d1d3..80a2ee4 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
 obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
+obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
 obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
new file mode 100644
index 0000000..bd48807
--- /dev/null
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -0,0 +1,1596 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * Parts of this driver are based on the following:
+ *  - Kvaser linux leaf driver (version 4.78)
+ *  - CAN driver for esd CAN-USB/2
+ *
+ * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
+ * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
+ * Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
+ */
+
+#include <linux/init.h>
+#include <linux/completion.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/usb.h>
+
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+
+#define MAX_TX_URBS			16
+#define MAX_RX_URBS			4
+#define START_TIMEOUT			1000 /* msecs */
+#define STOP_TIMEOUT			1000 /* msecs */
+#define USB_SEND_TIMEOUT		1000 /* msecs */
+#define USB_RECV_TIMEOUT		1000 /* msecs */
+#define RX_BUFFER_SIZE			3072
+#define CAN_USB_CLOCK			8000000
+#define MAX_NET_DEVICES			3
+
+/* Kvaser USB devices */
+#define KVASER_VENDOR_ID		0x0bfd
+#define USB_LEAF_DEVEL_PRODUCT_ID	10
+#define USB_LEAF_LITE_PRODUCT_ID	11
+#define USB_LEAF_PRO_PRODUCT_ID		12
+#define USB_LEAF_SPRO_PRODUCT_ID	14
+#define USB_LEAF_PRO_LS_PRODUCT_ID	15
+#define USB_LEAF_PRO_SWC_PRODUCT_ID	16
+#define USB_LEAF_PRO_LIN_PRODUCT_ID	17
+#define USB_LEAF_SPRO_LS_PRODUCT_ID	18
+#define USB_LEAF_SPRO_SWC_PRODUCT_ID	19
+#define USB_MEMO2_DEVEL_PRODUCT_ID	22
+#define USB_MEMO2_HSHS_PRODUCT_ID	23
+#define USB_UPRO_HSHS_PRODUCT_ID	24
+#define USB_LEAF_LITE_GI_PRODUCT_ID	25
+#define USB_LEAF_PRO_OBDII_PRODUCT_ID	26
+#define USB_MEMO2_HSLS_PRODUCT_ID	27
+#define USB_LEAF_LITE_CH_PRODUCT_ID	28
+#define USB_BLACKBIRD_SPRO_PRODUCT_ID	29
+#define USB_OEM_MERCURY_PRODUCT_ID	34
+#define USB_OEM_LEAF_PRODUCT_ID		35
+#define USB_CAN_R_PRODUCT_ID		39
+
+/* USB devices features */
+#define KVASER_HAS_SILENT_MODE		BIT(0)
+#define KVASER_HAS_TXRX_ERRORS		BIT(1)
+
+/* Message header size */
+#define MSG_HEADER_LEN			2
+
+/* Can message flags */
+#define MSG_FLAG_ERROR_FRAME		BIT(0)
+#define MSG_FLAG_OVERRUN		BIT(1)
+#define MSG_FLAG_NERR			BIT(2)
+#define MSG_FLAG_WAKEUP			BIT(3)
+#define MSG_FLAG_REMOTE_FRAME		BIT(4)
+#define MSG_FLAG_RESERVED		BIT(5)
+#define MSG_FLAG_TX_ACK			BIT(6)
+#define MSG_FLAG_TX_REQUEST		BIT(7)
+
+/* Can states */
+#define M16C_STATE_BUS_RESET		BIT(0)
+#define M16C_STATE_BUS_ERROR		BIT(4)
+#define M16C_STATE_BUS_PASSIVE		BIT(5)
+#define M16C_STATE_BUS_OFF		BIT(6)
+
+/* Can msg ids */
+#define CMD_RX_STD_MESSAGE		12
+#define CMD_TX_STD_MESSAGE		13
+#define CMD_RX_EXT_MESSAGE		14
+#define CMD_TX_EXT_MESSAGE		15
+#define CMD_SET_BUS_PARAMS		16
+#define CMD_GET_BUS_PARAMS		17
+#define CMD_GET_BUS_PARAMS_REPLY	18
+#define CMD_GET_CHIP_STATE		19
+#define CMD_CHIP_STATE_EVENT		20
+#define CMD_SET_CTRL_MODE		21
+#define CMD_GET_CTRL_MODE		22
+#define CMD_GET_CTRL_MODE_REPLY		23
+#define CMD_RESET_CHIP			24
+#define CMD_RESET_CARD			25
+#define CMD_START_CHIP			26
+#define CMD_START_CHIP_REPLY		27
+#define CMD_STOP_CHIP			28
+#define CMD_STOP_CHIP_REPLY		29
+#define CMD_GET_CARD_INFO2		32
+#define CMD_GET_CARD_INFO		34
+#define CMD_GET_CARD_INFO_REPLY		35
+#define CMD_GET_SOFTWARE_INFO		38
+#define CMD_GET_SOFTWARE_INFO_REPLY	39
+#define CMD_ERROR_EVENT			45
+#define CMD_FLUSH_QUEUE			48
+#define CMD_RESET_ERROR_COUNTER		49
+#define CMD_TX_ACKNOWLEDGE		50
+#define CMD_CAN_ERROR_EVENT		51
+#define CMD_USB_THROTTLE		77
+#define CMD_LOG_MESSAGE			106
+
+/* error factors */
+#define M16C_EF_ACKE			BIT(0)
+#define M16C_EF_CRCE			BIT(1)
+#define M16C_EF_FORME			BIT(2)
+#define M16C_EF_STFE			BIT(3)
+#define M16C_EF_BITE0			BIT(4)
+#define M16C_EF_BITE1			BIT(5)
+#define M16C_EF_RCVE			BIT(6)
+#define M16C_EF_TRE			BIT(7)
+
+/* bittiming parameters */
+#define KVASER_USB_TSEG1_MIN		1
+#define KVASER_USB_TSEG1_MAX		16
+#define KVASER_USB_TSEG2_MIN		1
+#define KVASER_USB_TSEG2_MAX		8
+#define KVASER_USB_SJW_MAX		4
+#define KVASER_USB_BRP_MIN		1
+#define KVASER_USB_BRP_MAX		64
+#define KVASER_USB_BRP_INC		1
+
+/* ctrl modes */
+#define KVASER_CTRL_MODE_NORMAL		1
+#define KVASER_CTRL_MODE_SILENT		2
+#define KVASER_CTRL_MODE_SELFRECEPTION	3
+#define KVASER_CTRL_MODE_OFF		4
+
+struct kvaser_msg_simple {
+	u8 tid;
+	u8 channel;
+} __packed;
+
+struct kvaser_msg_cardinfo {
+	u8 tid;
+	u8 nchannels;
+	__le32 serial_number;
+	__le32 padding;
+	__le32 clock_resolution;
+	__le32 mfgdate;
+	u8 ean[8];
+	u8 hw_revision;
+	u8 usb_hs_mode;
+	__le16 padding2;
+} __packed;
+
+struct kvaser_msg_cardinfo2 {
+	u8 tid;
+	u8 channel;
+	u8 pcb_id[24];
+	__le32 oem_unlock_code;
+} __packed;
+
+struct kvaser_msg_softinfo {
+	u8 tid;
+	u8 channel;
+	__le32 sw_options;
+	__le32 fw_version;
+	__le16 max_outstanding_tx;
+	__le16 padding[9];
+} __packed;
+
+struct kvaser_msg_busparams {
+	u8 tid;
+	u8 channel;
+	__le32 bitrate;
+	u8 tseg1;
+	u8 tseg2;
+	u8 sjw;
+	u8 no_samp;
+} __packed;
+
+struct kvaser_msg_tx_can {
+	u8 channel;
+	u8 tid;
+	u8 msg[14];
+	u8 padding;
+	u8 flags;
+} __packed;
+
+struct kvaser_msg_rx_can {
+	u8 channel;
+	u8 flag;
+	__le16 time[3];
+	u8 msg[14];
+} __packed;
+
+struct kvaser_msg_chip_state_event {
+	u8 tid;
+	u8 channel;
+	__le16 time[3];
+	u8 tx_errors_count;
+	u8 rx_errors_count;
+	u8 status;
+	u8 padding[3];
+} __packed;
+
+struct kvaser_msg_tx_acknowledge {
+	u8 channel;
+	u8 tid;
+	__le16 time[3];
+	u8 flags;
+	u8 time_offset;
+} __packed;
+
+struct kvaser_msg_error_event {
+	u8 tid;
+	u8 flags;
+	__le16 time[3];
+	u8 channel;
+	u8 padding;
+	u8 tx_errors_count;
+	u8 rx_errors_count;
+	u8 status;
+	u8 error_factor;
+} __packed;
+
+struct kvaser_msg_ctrl_mode {
+	u8 tid;
+	u8 channel;
+	u8 ctrl_mode;
+	u8 padding[3];
+} __packed;
+
+struct kvaser_msg_flush_queue {
+	u8 tid;
+	u8 channel;
+	u8 flags;
+	u8 padding[3];
+} __packed;
+
+struct kvaser_msg_log_message {
+	u8 channel;
+	u8 flags;
+	__le16 time[3];
+	u8 dlc;
+	u8 time_offset;
+	__le32 id;
+	u8 data[8];
+} __packed;
+
+struct kvaser_msg {
+	u8 len;
+	u8 id;
+	union	{
+		struct kvaser_msg_simple simple;
+		struct kvaser_msg_cardinfo cardinfo;
+		struct kvaser_msg_cardinfo2 cardinfo2;
+		struct kvaser_msg_softinfo softinfo;
+		struct kvaser_msg_busparams busparams;
+		struct kvaser_msg_tx_can tx_can;
+		struct kvaser_msg_rx_can rx_can;
+		struct kvaser_msg_chip_state_event chip_state_event;
+		struct kvaser_msg_tx_acknowledge tx_acknowledge;
+		struct kvaser_msg_error_event error_event;
+		struct kvaser_msg_ctrl_mode ctrl_mode;
+		struct kvaser_msg_flush_queue flush_queue;
+		struct kvaser_msg_log_message log_message;
+	} u;
+} __packed;
+
+struct kvaser_usb_tx_urb_context {
+	struct kvaser_usb_net_priv *priv;
+	u32 echo_index;
+	int dlc;
+};
+
+struct kvaser_usb {
+	struct usb_device *udev;
+	struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
+
+	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+	struct usb_anchor rx_submitted;
+
+	u32 fw_version;
+	unsigned int nchannels;
+
+	bool rxinitdone;
+	void *rxbuf[MAX_RX_URBS];
+	dma_addr_t rxbuf_dma[MAX_RX_URBS];
+};
+
+struct kvaser_usb_net_priv {
+	struct can_priv can;
+
+	atomic_t active_tx_urbs;
+	struct usb_anchor tx_submitted;
+	struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
+
+	struct completion start_comp, stop_comp;
+
+	struct kvaser_usb *dev;
+	struct net_device *netdev;
+	int channel;
+
+	struct can_berr_counter bec;
+};
+
+static struct usb_device_id kvaser_usb_table[] = {
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS |
+			       KVASER_HAS_SILENT_MODE },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID),
+		.driver_info = KVASER_HAS_TXRX_ERRORS },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
+
+static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev,
+				      struct kvaser_msg *msg)
+{
+	int actual_len;
+
+	return usb_bulk_msg(dev->udev,
+			    usb_sndbulkpipe(dev->udev,
+					dev->bulk_out->bEndpointAddress),
+			    msg, msg->len, &actual_len,
+			    USB_SEND_TIMEOUT);
+}
+
+static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
+			       struct kvaser_msg *msg)
+{
+	struct kvaser_msg *tmp;
+	void *buf;
+	int actual_len;
+	int err;
+	int pos = 0;
+
+	buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	err = usb_bulk_msg(dev->udev,
+			   usb_rcvbulkpipe(dev->udev,
+					   dev->bulk_in->bEndpointAddress),
+			   buf, RX_BUFFER_SIZE, &actual_len,
+			   USB_RECV_TIMEOUT);
+	if (err < 0)
+		goto end;
+
+	while (pos <= actual_len - MSG_HEADER_LEN) {
+		tmp = buf + pos;
+
+		if (!tmp->len)
+			break;
+
+		if (pos + tmp->len > actual_len) {
+			dev_err(dev->udev->dev.parent, "Format error\n");
+			break;
+		}
+
+		if (tmp->id == id) {
+			memcpy(msg, tmp, tmp->len);
+			goto end;
+		}
+
+		pos += tmp->len;
+	}
+
+	err = -EINVAL;
+
+end:
+	kfree(buf);
+
+	return err;
+}
+
+static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
+				      u8 msg_id, int channel)
+{
+	struct kvaser_msg msg = {
+		.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
+		.id = msg_id,
+		.u.simple.channel = channel,
+		.u.simple.tid = 0xff,
+	};
+
+	return kvaser_usb_send_msg(dev, &msg);
+}
+
+static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
+{
+	struct kvaser_msg msg;
+	int err;
+
+	err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
+	if (err)
+		return err;
+
+	err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
+	if (err)
+		return err;
+
+	dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
+
+	return 0;
+}
+
+static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
+{
+	struct kvaser_msg msg;
+	int err;
+
+	err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
+	if (err)
+		return err;
+
+	err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
+	if (err)
+		return err;
+
+	dev->nchannels = msg.u.cardinfo.nchannels;
+
+	return 0;
+}
+
+static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
+				      const struct kvaser_msg *msg)
+{
+	struct net_device_stats *stats;
+	struct kvaser_usb_tx_urb_context *context;
+	struct kvaser_usb_net_priv *priv;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	u8 channel = msg->u.tx_acknowledge.channel;
+	u8 tid = msg->u.tx_acknowledge.tid;
+
+	if (channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", channel);
+		return;
+	}
+
+	priv = dev->nets[channel];
+
+	if (!netif_device_present(priv->netdev))
+		return;
+
+	stats = &priv->netdev->stats;
+
+	context = &priv->tx_contexts[tid % MAX_TX_URBS];
+
+	/* Sometimes the state change doesn't come after a bus-off event */
+	if (priv->can.restart_ms &&
+	    (priv->can.state >= CAN_STATE_BUS_OFF)) {
+		skb = alloc_can_err_skb(priv->netdev, &cf);
+		if (skb) {
+			cf->can_id |= CAN_ERR_RESTARTED;
+			netif_rx(skb);
+
+			stats->rx_packets++;
+			stats->rx_bytes += cf->can_dlc;
+		} else {
+			netdev_err(priv->netdev,
+				   "No memory left for err_skb\n");
+		}
+
+		priv->can.can_stats.restarts++;
+		netif_carrier_on(priv->netdev);
+
+		priv->can.state = CAN_STATE_ERROR_ACTIVE;
+	}
+
+	stats->tx_packets++;
+	stats->tx_bytes += context->dlc;
+	can_get_echo_skb(priv->netdev, context->echo_index);
+
+	context->echo_index = MAX_TX_URBS;
+	atomic_dec(&priv->active_tx_urbs);
+
+	netif_wake_queue(priv->netdev);
+}
+
+static void kvaser_usb_simple_msg_callback(struct urb *urb)
+{
+	struct net_device *netdev = urb->context;
+
+	kfree(urb->transfer_buffer);
+
+	if (urb->status)
+		netdev_warn(netdev, "urb status received: %d\n",
+			    urb->status);
+}
+
+static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
+				       u8 msg_id)
+{
+	struct kvaser_usb *dev = priv->dev;
+	struct net_device *netdev = priv->netdev;
+	struct kvaser_msg *msg;
+	struct urb *urb;
+	void *buf;
+	int err;
+
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		netdev_err(netdev, "No memory left for URBs\n");
+		return -ENOMEM;
+	}
+
+	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
+	if (!buf) {
+		netdev_err(netdev, "No memory left for USB buffer\n");
+		usb_free_urb(urb);
+		return -ENOMEM;
+	}
+
+	msg = (struct kvaser_msg *)buf;
+	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
+	msg->id = msg_id;
+	msg->u.simple.channel = priv->channel;
+
+	usb_fill_bulk_urb(urb, dev->udev,
+			  usb_sndbulkpipe(dev->udev,
+					  dev->bulk_out->bEndpointAddress),
+			  buf, msg->len,
+			  kvaser_usb_simple_msg_callback, priv);
+	usb_anchor_urb(urb, &priv->tx_submitted);
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err) {
+		netdev_err(netdev, "Error transmitting URB\n");
+		usb_unanchor_urb(urb);
+		usb_free_urb(urb);
+		kfree(buf);
+		return err;
+	}
+
+	usb_free_urb(urb);
+
+	return 0;
+}
+
+static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
+{
+	int i;
+
+	usb_kill_anchored_urbs(&priv->tx_submitted);
+	atomic_set(&priv->active_tx_urbs, 0);
+
+	for (i = 0; i < MAX_TX_URBS; i++)
+		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
+}
+
+static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
+				const struct kvaser_msg *msg)
+{
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	struct net_device_stats *stats;
+	struct kvaser_usb_net_priv *priv;
+	unsigned int new_state;
+	u8 channel, status, txerr, rxerr, error_factor;
+
+	switch (msg->id) {
+	case CMD_CAN_ERROR_EVENT:
+		channel = msg->u.error_event.channel;
+		status =  msg->u.error_event.status;
+		txerr = msg->u.error_event.tx_errors_count;
+		rxerr = msg->u.error_event.rx_errors_count;
+		error_factor = msg->u.error_event.error_factor;
+		break;
+	case CMD_LOG_MESSAGE:
+		channel = msg->u.log_message.channel;
+		status = msg->u.log_message.data[0];
+		txerr = msg->u.log_message.data[2];
+		rxerr = msg->u.log_message.data[3];
+		error_factor = msg->u.log_message.data[1];
+		break;
+	case CMD_CHIP_STATE_EVENT:
+		channel = msg->u.chip_state_event.channel;
+		status =  msg->u.chip_state_event.status;
+		txerr = msg->u.chip_state_event.tx_errors_count;
+		rxerr = msg->u.chip_state_event.rx_errors_count;
+		error_factor = 0;
+		break;
+	default:
+		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+			msg->id);
+		return;
+	}
+
+	if (channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", channel);
+		return;
+	}
+
+	priv = dev->nets[channel];
+	stats = &priv->netdev->stats;
+
+	if (status & M16C_STATE_BUS_RESET) {
+		kvaser_usb_unlink_tx_urbs(priv);
+		return;
+	}
+
+	skb = alloc_can_err_skb(priv->netdev, &cf);
+	if (!skb) {
+		stats->rx_dropped++;
+		return;
+	}
+
+	new_state = priv->can.state;
+
+	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
+
+	if (status & M16C_STATE_BUS_OFF) {
+		cf->can_id |= CAN_ERR_BUSOFF;
+
+		priv->can.can_stats.bus_off++;
+		if (!priv->can.restart_ms)
+			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
+
+		netif_carrier_off(priv->netdev);
+
+		new_state = CAN_STATE_BUS_OFF;
+	} else if (status & M16C_STATE_BUS_PASSIVE) {
+		if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
+			cf->can_id |= CAN_ERR_CRTL;
+
+			if (txerr || rxerr)
+				cf->data[1] = (txerr > rxerr)
+						? CAN_ERR_CRTL_TX_PASSIVE
+						: CAN_ERR_CRTL_RX_PASSIVE;
+			else
+				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
+					      CAN_ERR_CRTL_RX_PASSIVE;
+
+			priv->can.can_stats.error_passive++;
+		}
+
+		new_state = CAN_STATE_ERROR_PASSIVE;
+	}
+
+	if (status == M16C_STATE_BUS_ERROR) {
+		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
+		    ((txerr >= 96) || (rxerr >= 96))) {
+			cf->can_id |= CAN_ERR_CRTL;
+			cf->data[1] = (txerr > rxerr)
+					? CAN_ERR_CRTL_TX_WARNING
+					: CAN_ERR_CRTL_RX_WARNING;
+
+			priv->can.can_stats.error_warning++;
+			new_state = CAN_STATE_ERROR_WARNING;
+		} else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
+			cf->can_id |= CAN_ERR_PROT;
+			cf->data[2] = CAN_ERR_PROT_ACTIVE;
+
+			new_state = CAN_STATE_ERROR_ACTIVE;
+		}
+	}
+
+	if (!status) {
+		cf->can_id |= CAN_ERR_PROT;
+		cf->data[2] = CAN_ERR_PROT_ACTIVE;
+
+		new_state = CAN_STATE_ERROR_ACTIVE;
+	}
+
+	if (priv->can.restart_ms &&
+	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
+	    (new_state < CAN_STATE_BUS_OFF)) {
+		cf->can_id |= CAN_ERR_RESTARTED;
+		netif_carrier_on(priv->netdev);
+
+		priv->can.can_stats.restarts++;
+	}
+
+	if (error_factor) {
+		priv->can.can_stats.bus_error++;
+		stats->rx_errors++;
+
+		cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
+
+		if (error_factor & M16C_EF_ACKE)
+			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
+		if (error_factor & M16C_EF_CRCE)
+			cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
+					CAN_ERR_PROT_LOC_CRC_DEL);
+		if (error_factor & M16C_EF_FORME)
+			cf->data[2] |= CAN_ERR_PROT_FORM;
+		if (error_factor & M16C_EF_STFE)
+			cf->data[2] |= CAN_ERR_PROT_STUFF;
+		if (error_factor & M16C_EF_BITE0)
+			cf->data[2] |= CAN_ERR_PROT_BIT0;
+		if (error_factor & M16C_EF_BITE1)
+			cf->data[2] |= CAN_ERR_PROT_BIT1;
+		if (error_factor & M16C_EF_TRE)
+			cf->data[2] |= CAN_ERR_PROT_TX;
+	}
+
+	cf->data[6] = txerr;
+	cf->data[7] = rxerr;
+
+	priv->bec.txerr = txerr;
+	priv->bec.rxerr = rxerr;
+
+	priv->can.state = new_state;
+
+	netif_rx(skb);
+
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
+				  const struct kvaser_msg *msg)
+{
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	struct net_device_stats *stats = &priv->netdev->stats;
+
+	if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
+					 MSG_FLAG_NERR)) {
+		netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
+			   msg->u.rx_can.flag);
+
+		stats->rx_errors++;
+		return;
+	}
+
+	if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+		skb = alloc_can_err_skb(priv->netdev, &cf);
+		if (!skb) {
+			stats->tx_dropped++;
+			return;
+		}
+
+		cf->can_id |= CAN_ERR_CRTL;
+		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+
+		stats->rx_over_errors++;
+		stats->rx_errors++;
+
+		netif_rx(skb);
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	}
+}
+
+static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
+				  const struct kvaser_msg *msg)
+{
+	struct kvaser_usb_net_priv *priv;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	struct net_device_stats *stats;
+	u8 channel = msg->u.rx_can.channel;
+
+	if (channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", channel);
+		return;
+	}
+
+	priv = dev->nets[channel];
+	stats = &priv->netdev->stats;
+
+	if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME | MSG_FLAG_NERR |
+				  MSG_FLAG_OVERRUN)) {
+		kvaser_usb_rx_can_err(priv, msg);
+		return;
+	} else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
+		netdev_warn(priv->netdev,
+			    "Unhandled frame (flags: 0x%02x)",
+			    msg->u.rx_can.flag);
+		return;
+	}
+
+	skb = alloc_can_skb(priv->netdev, &cf);
+	if (!skb) {
+		stats->tx_dropped++;
+		return;
+	}
+
+	cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
+		     (msg->u.rx_can.msg[1] & 0x3f);
+	cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+
+	if (msg->id == CMD_RX_EXT_MESSAGE) {
+		cf->can_id <<= 18;
+		cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
+			      ((msg->u.rx_can.msg[3] & 0xff) << 6) |
+			      (msg->u.rx_can.msg[4] & 0x3f);
+		cf->can_id |= CAN_EFF_FLAG;
+	}
+
+	if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
+		cf->can_id |= CAN_RTR_FLAG;
+	else
+		memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
+
+	netif_rx(skb);
+
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
+					const struct kvaser_msg *msg)
+{
+	struct kvaser_usb_net_priv *priv;
+	u8 channel = msg->u.simple.channel;
+
+	if (channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", channel);
+		return;
+	}
+
+	priv = dev->nets[channel];
+
+	if (completion_done(&priv->start_comp) &&
+	    netif_queue_stopped(priv->netdev)) {
+		netif_wake_queue(priv->netdev);
+	} else {
+		netif_start_queue(priv->netdev);
+		complete(&priv->start_comp);
+	}
+}
+
+static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev,
+				       const struct kvaser_msg *msg)
+{
+	struct kvaser_usb_net_priv *priv;
+	u8 channel = msg->u.simple.channel;
+
+	if (channel >= dev->nchannels) {
+		dev_err(dev->udev->dev.parent,
+			"Invalid channel number (%d)\n", channel);
+		return;
+	}
+
+	priv = dev->nets[channel];
+
+	complete(&priv->stop_comp);
+}
+
+static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
+				      const struct kvaser_msg *msg)
+{
+	switch (msg->id) {
+	case CMD_START_CHIP_REPLY:
+		kvaser_usb_start_chip_reply(dev, msg);
+		break;
+
+	case CMD_STOP_CHIP_REPLY:
+		kvaser_usb_stop_chip_reply(dev, msg);
+		break;
+
+	case CMD_RX_STD_MESSAGE:
+	case CMD_RX_EXT_MESSAGE:
+		kvaser_usb_rx_can_msg(dev, msg);
+		break;
+
+	case CMD_CHIP_STATE_EVENT:
+	case CMD_CAN_ERROR_EVENT:
+		kvaser_usb_rx_error(dev, msg);
+		break;
+
+	case CMD_LOG_MESSAGE:
+		if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME)
+			kvaser_usb_rx_error(dev, msg);
+		break;
+
+	case CMD_TX_ACKNOWLEDGE:
+		kvaser_usb_tx_acknowledge(dev, msg);
+		break;
+
+	default:
+		dev_warn(dev->udev->dev.parent,
+			 "Unhandled message (%d)\n", msg->id);
+		break;
+	}
+}
+
+static void kvaser_usb_read_bulk_callback(struct urb *urb)
+{
+	struct kvaser_usb *dev = urb->context;
+	struct kvaser_msg *msg;
+	int pos = 0;
+	int err, i;
+
+	switch (urb->status) {
+	case 0:
+		break;
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+	default:
+		dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
+			 urb->status);
+		goto resubmit_urb;
+	}
+
+	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
+		msg = urb->transfer_buffer + pos;
+
+		if (!msg->len)
+			break;
+
+		if (pos + msg->len > urb->actual_length) {
+			dev_err(dev->udev->dev.parent, "Format error\n");
+			break;
+		}
+
+		kvaser_usb_handle_message(dev, msg);
+
+		pos += msg->len;
+	}
+
+resubmit_urb:
+	usb_fill_bulk_urb(urb, dev->udev,
+			  usb_rcvbulkpipe(dev->udev,
+					  dev->bulk_in->bEndpointAddress),
+			  urb->transfer_buffer, RX_BUFFER_SIZE,
+			  kvaser_usb_read_bulk_callback, dev);
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err == -ENODEV) {
+		for (i = 0; i < dev->nchannels; i++) {
+			if (!dev->nets[i])
+				continue;
+
+			netif_device_detach(dev->nets[i]->netdev);
+		}
+	} else if (err) {
+		dev_err(dev->udev->dev.parent,
+			"Failed resubmitting read bulk urb: %d\n", err);
+	}
+
+	return;
+}
+
+static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
+{
+	int i, err = 0;
+
+	if (dev->rxinitdone)
+		return 0;
+
+	for (i = 0; i < MAX_RX_URBS; i++) {
+		struct urb *urb = NULL;
+		u8 *buf = NULL;
+		dma_addr_t buf_dma;
+
+		urb = usb_alloc_urb(0, GFP_KERNEL);
+		if (!urb) {
+			dev_warn(dev->udev->dev.parent,
+				 "No memory left for URBs\n");
+			err = -ENOMEM;
+			break;
+		}
+
+		buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
+					 GFP_KERNEL, &buf_dma);
+		if (!buf) {
+			dev_warn(dev->udev->dev.parent,
+				 "No memory left for USB buffer\n");
+			usb_free_urb(urb);
+			err = -ENOMEM;
+			break;
+		}
+
+		usb_fill_bulk_urb(urb, dev->udev,
+				  usb_rcvbulkpipe(dev->udev,
+					  dev->bulk_in->bEndpointAddress),
+				  buf, RX_BUFFER_SIZE,
+				  kvaser_usb_read_bulk_callback,
+				  dev);
+		urb->transfer_dma = buf_dma;
+		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+		usb_anchor_urb(urb, &dev->rx_submitted);
+
+		err = usb_submit_urb(urb, GFP_KERNEL);
+		if (err) {
+			usb_unanchor_urb(urb);
+			usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
+					  buf_dma);
+			usb_free_urb(urb);
+			break;
+		}
+
+		dev->rxbuf[i] = buf;
+		dev->rxbuf_dma[i] = buf_dma;
+
+		usb_free_urb(urb);
+	}
+
+	if (i == 0) {
+		dev_warn(dev->udev->dev.parent,
+			 "Cannot setup read URBs, error %d\n", err);
+		return err;
+	} else if (i < MAX_RX_URBS) {
+		dev_warn(dev->udev->dev.parent,
+			 "RX performances may be slow\n");
+	}
+
+	dev->rxinitdone = true;
+
+	return 0;
+}
+
+static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
+{
+	struct kvaser_msg msg = {
+		.id = CMD_SET_CTRL_MODE,
+		.len = MSG_HEADER_LEN +
+		       sizeof(struct kvaser_msg_ctrl_mode),
+		.u.ctrl_mode.tid = 0xff,
+		.u.ctrl_mode.channel = priv->channel,
+	};
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
+	else
+		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
+
+	return kvaser_usb_send_msg(priv->dev, &msg);
+}
+
+static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
+{
+	int err;
+
+	init_completion(&priv->start_comp);
+
+	err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
+					 priv->channel);
+	if (err)
+		return err;
+
+	if (!wait_for_completion_timeout(&priv->start_comp,
+					 msecs_to_jiffies(START_TIMEOUT)))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int kvaser_usb_open(struct net_device *netdev)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+	struct kvaser_usb *dev = priv->dev;
+	int err;
+
+	err = open_candev(netdev);
+	if (err)
+		return err;
+
+	err = kvaser_usb_setup_rx_urbs(dev);
+	if (err)
+		goto error;
+
+	err = kvaser_usb_set_opt_mode(priv);
+	if (err)
+		goto error;
+
+	err = kvaser_usb_start_chip(priv);
+	if (err) {
+		netdev_warn(netdev, "Cannot start device, error %d\n", err);
+		goto error;
+	}
+
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	return 0;
+
+error:
+	close_candev(netdev);
+	return err;
+}
+
+static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
+{
+	int i;
+
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+
+	for (i = 0; i < MAX_RX_URBS; i++)
+		usb_free_coherent(dev->udev, RX_BUFFER_SIZE,
+				  dev->rxbuf[i],
+				  dev->rxbuf_dma[i]);
+
+	for (i = 0; i < MAX_NET_DEVICES; i++) {
+		struct kvaser_usb_net_priv *priv = dev->nets[i];
+
+		if (priv)
+			kvaser_usb_unlink_tx_urbs(priv);
+	}
+}
+
+static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
+{
+	int err;
+
+	init_completion(&priv->stop_comp);
+
+	err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
+					 priv->channel);
+	if (err)
+		return err;
+
+	if (!wait_for_completion_timeout(&priv->stop_comp,
+					 msecs_to_jiffies(STOP_TIMEOUT)))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
+{
+	struct kvaser_msg msg = {
+		.id = CMD_FLUSH_QUEUE,
+		.len = MSG_HEADER_LEN +
+		       sizeof(struct kvaser_msg_flush_queue),
+		.u.flush_queue.channel = priv->channel,
+		.u.flush_queue.flags = 0x00,
+	};
+
+	return kvaser_usb_send_msg(priv->dev, &msg);
+}
+
+static int kvaser_usb_close(struct net_device *netdev)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+	struct kvaser_usb *dev = priv->dev;
+	int err;
+
+	netif_stop_queue(netdev);
+
+	err = kvaser_usb_flush_queue(priv);
+	if (err)
+		netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
+
+	if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel))
+		netdev_warn(netdev, "Cannot reset card, error %d\n", err);
+
+	err = kvaser_usb_stop_chip(priv);
+	if (err)
+		netdev_warn(netdev, "Cannot stop device, error %d\n", err);
+
+	priv->can.state = CAN_STATE_STOPPED;
+	close_candev(priv->netdev);
+
+	return 0;
+}
+
+static void kvaser_usb_write_bulk_callback(struct urb *urb)
+{
+	struct kvaser_usb_tx_urb_context *context = urb->context;
+	struct kvaser_usb_net_priv *priv;
+	struct net_device *netdev;
+
+	if (WARN_ON(!context))
+		return;
+
+	priv = context->priv;
+	netdev = priv->netdev;
+
+	kfree(urb->transfer_buffer);
+
+	if (!netif_device_present(netdev))
+		return;
+
+	if (urb->status)
+		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
+}
+
+static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
+					 struct net_device *netdev)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+	struct kvaser_usb *dev = priv->dev;
+	struct net_device_stats *stats = &netdev->stats;
+	struct can_frame *cf = (struct can_frame *)skb->data;
+	struct kvaser_usb_tx_urb_context *context = NULL;
+	struct urb *urb;
+	void *buf;
+	struct kvaser_msg *msg;
+	int i, err;
+	int ret = NETDEV_TX_OK;
+
+	if (can_dropped_invalid_skb(netdev, skb))
+		return NETDEV_TX_OK;
+
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		netdev_err(netdev, "No memory left for URBs\n");
+		stats->tx_dropped++;
+		dev_kfree_skb(skb);
+		return NETDEV_TX_OK;
+	}
+
+	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
+	if (!buf) {
+		netdev_err(netdev, "No memory left for USB buffer\n");
+		stats->tx_dropped++;
+		dev_kfree_skb(skb);
+		usb_free_urb(urb);
+		goto nobufmem;
+	}
+
+	msg = buf;
+	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
+	msg->u.tx_can.flags = 0;
+	msg->u.tx_can.channel = priv->channel;
+
+	if (cf->can_id & CAN_EFF_FLAG) {
+		msg->id = CMD_TX_EXT_MESSAGE;
+		msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
+		msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
+		msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
+		msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
+		msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
+	} else {
+		msg->id = CMD_TX_STD_MESSAGE;
+		msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
+		msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
+	}
+
+	msg->u.tx_can.msg[5] = cf->can_dlc;
+	memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
+
+	if (cf->can_id & CAN_RTR_FLAG)
+		msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
+
+	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
+		if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
+			context = &priv->tx_contexts[i];
+			break;
+		}
+	}
+
+	if (!context) {
+		netdev_warn(netdev, "cannot find free context\n");
+		ret =  NETDEV_TX_BUSY;
+		goto releasebuf;
+	}
+
+	context->priv = priv;
+	context->echo_index = i;
+	context->dlc = cf->can_dlc;
+
+	msg->u.tx_can.tid = context->echo_index;
+
+	usb_fill_bulk_urb(urb, dev->udev,
+			  usb_sndbulkpipe(dev->udev,
+					  dev->bulk_out->bEndpointAddress),
+			  buf, msg->len,
+			  kvaser_usb_write_bulk_callback, context);
+	usb_anchor_urb(urb, &priv->tx_submitted);
+
+	can_put_echo_skb(skb, netdev, context->echo_index);
+
+	atomic_inc(&priv->active_tx_urbs);
+
+	if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
+		netif_stop_queue(netdev);
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (unlikely(err)) {
+		can_free_echo_skb(netdev, context->echo_index);
+
+		atomic_dec(&priv->active_tx_urbs);
+		usb_unanchor_urb(urb);
+
+		stats->tx_dropped++;
+
+		if (err == -ENODEV)
+			netif_device_detach(netdev);
+		else
+			netdev_warn(netdev, "Failed tx_urb %d\n", err);
+
+		goto releasebuf;
+	}
+
+	usb_free_urb(urb);
+
+	return NETDEV_TX_OK;
+
+releasebuf:
+	kfree(buf);
+nobufmem:
+	usb_free_urb(urb);
+	return ret;
+}
+
+static const struct net_device_ops kvaser_usb_netdev_ops = {
+	.ndo_open = kvaser_usb_open,
+	.ndo_stop = kvaser_usb_close,
+	.ndo_start_xmit = kvaser_usb_start_xmit,
+};
+
+static struct can_bittiming_const kvaser_usb_bittiming_const = {
+	.name = "kvaser_usb",
+	.tseg1_min = KVASER_USB_TSEG1_MIN,
+	.tseg1_max = KVASER_USB_TSEG1_MAX,
+	.tseg2_min = KVASER_USB_TSEG2_MIN,
+	.tseg2_max = KVASER_USB_TSEG2_MAX,
+	.sjw_max = KVASER_USB_SJW_MAX,
+	.brp_min = KVASER_USB_BRP_MIN,
+	.brp_max = KVASER_USB_BRP_MAX,
+	.brp_inc = KVASER_USB_BRP_INC,
+};
+
+static int kvaser_usb_set_bittiming(struct net_device *netdev)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	struct kvaser_usb *dev = priv->dev;
+	struct kvaser_msg msg = {
+		.id = CMD_SET_BUS_PARAMS,
+		.len = MSG_HEADER_LEN +
+		       sizeof(struct kvaser_msg_busparams),
+		.u.busparams.channel = priv->channel,
+		.u.busparams.tid = 0xff,
+		.u.busparams.bitrate = cpu_to_le32(bt->bitrate),
+		.u.busparams.sjw = bt->sjw,
+		.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
+		.u.busparams.tseg2 = bt->phase_seg2,
+	};
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		msg.u.busparams.no_samp = 3;
+	else
+		msg.u.busparams.no_samp = 1;
+
+	return kvaser_usb_send_msg(dev, &msg);
+}
+
+static int kvaser_usb_set_mode(struct net_device *netdev,
+			       enum can_mode mode)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+	int err;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP);
+		if (err)
+			return err;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
+				       struct can_berr_counter *bec)
+{
+	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
+
+	*bec = priv->bec;
+
+	return 0;
+}
+
+static void kvaser_usb_remove_interfaces(struct kvaser_usb *dev)
+{
+	int i;
+
+	for (i = 0; i < dev->nchannels; i++) {
+		if (!dev->nets[i])
+			continue;
+
+		unregister_netdev(dev->nets[i]->netdev);
+	}
+
+	kvaser_usb_unlink_all_urbs(dev);
+
+	for (i = 0; i < dev->nchannels; i++) {
+		if (!dev->nets[i])
+			continue;
+
+		free_candev(dev->nets[i]->netdev);
+	}
+}
+
+static int kvaser_usb_init_one(struct usb_interface *intf,
+			       const struct usb_device_id *id, int channel)
+{
+	struct kvaser_usb *dev = usb_get_intfdata(intf);
+	struct net_device *netdev;
+	struct kvaser_usb_net_priv *priv;
+	int i, err;
+
+	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
+	if (!netdev) {
+		dev_err(&intf->dev, "Cannot alloc candev\n");
+		return -ENOMEM;
+	}
+
+	priv = netdev_priv(netdev);
+
+	init_completion(&priv->start_comp);
+	init_completion(&priv->stop_comp);
+
+	init_usb_anchor(&priv->tx_submitted);
+	atomic_set(&priv->active_tx_urbs, 0);
+
+	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++)
+		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
+
+	priv->dev = dev;
+	priv->netdev = netdev;
+	priv->channel = channel;
+
+	priv->can.state = CAN_STATE_STOPPED;
+	priv->can.clock.freq = CAN_USB_CLOCK;
+	priv->can.bittiming_const = &kvaser_usb_bittiming_const;
+	priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
+	priv->can.do_set_mode = kvaser_usb_set_mode;
+	if (id->driver_info & KVASER_HAS_TXRX_ERRORS)
+		priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
+	if (id->driver_info & KVASER_HAS_SILENT_MODE)
+		priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
+
+	netdev->flags |= IFF_ECHO;
+
+	netdev->netdev_ops = &kvaser_usb_netdev_ops;
+
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
+	dev->nets[channel] = priv;
+
+	err = register_candev(netdev);
+	if (err) {
+		dev_err(&intf->dev, "Failed to register can device\n");
+		free_candev(netdev);
+		dev->nets[channel] = NULL;
+		return err;
+	}
+
+	netdev_dbg(netdev, "device registered\n");
+
+	return 0;
+}
+
+static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
+				     struct usb_endpoint_descriptor **in,
+				     struct usb_endpoint_descriptor **out)
+{
+	const struct usb_host_interface *iface_desc;
+	struct usb_endpoint_descriptor *endpoint;
+	int i;
+
+	iface_desc = &intf->altsetting[0];
+
+	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
+		endpoint = &iface_desc->endpoint[i].desc;
+
+		if (usb_endpoint_is_bulk_in(endpoint))
+			*in = endpoint;
+
+		if (usb_endpoint_is_bulk_out(endpoint))
+			*out = endpoint;
+	}
+}
+
+static int kvaser_usb_probe(struct usb_interface *intf,
+			    const struct usb_device_id *id)
+{
+	struct kvaser_usb *dev;
+	int err = -ENOMEM;
+	int i;
+
+	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
+	if (!dev->bulk_in || !dev->bulk_out) {
+		dev_err(&intf->dev, "Cannot get usb endpoint(s)");
+		return err;
+	}
+
+	dev->udev = interface_to_usbdev(intf);
+
+	init_usb_anchor(&dev->rx_submitted);
+
+	usb_set_intfdata(intf, dev);
+
+	for (i = 0; i < MAX_NET_DEVICES; i++)
+		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
+
+	err = kvaser_usb_get_software_info(dev);
+	if (err) {
+		dev_err(&intf->dev,
+			"Cannot get software infos, error %d\n", err);
+		return err;
+	}
+
+	err = kvaser_usb_get_card_info(dev);
+	if (err) {
+		dev_err(&intf->dev,
+			"Cannot get card infos, error %d\n", err);
+		return err;
+	}
+
+	dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
+		((dev->fw_version >> 24) & 0xff),
+		((dev->fw_version >> 16) & 0xff),
+		(dev->fw_version & 0xffff));
+
+	for (i = 0; i < dev->nchannels; i++) {
+		err = kvaser_usb_init_one(intf, id, i);
+		if (err) {
+			kvaser_usb_remove_interfaces(dev);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static void kvaser_usb_disconnect(struct usb_interface *intf)
+{
+	struct kvaser_usb *dev = usb_get_intfdata(intf);
+
+	usb_set_intfdata(intf, NULL);
+
+	if (!dev)
+		return;
+
+	kvaser_usb_remove_interfaces(dev);
+}
+
+static struct usb_driver kvaser_usb_driver = {
+	.name = "kvaser_usb",
+	.probe = kvaser_usb_probe,
+	.disconnect = kvaser_usb_disconnect,
+	.id_table = kvaser_usb_table
+};
+
+module_usb_driver(kvaser_usb_driver);
+
+MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
+MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH v4] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-10-02  7:14 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Marc Kleine-Budde, linux-can-u79uwXL29TY76Z2rM5mHXA,
	Kvaser Support, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <505DE106.6060405-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hi Marc and Wolfgang,

On Sat, Sep 22, 2012 at 06:02:14PM +0200, Wolfgang Grandegger wrote:
> On 09/21/2012 11:54 AM, Marc Kleine-Budde wrote:
> > On 09/20/2012 07:06 AM, Olivier Sobrie wrote:
> >> This driver provides support for several Kvaser CAN/USB devices.
> >> Such kind of devices supports up to three CAN network interfaces.
> >>
> >> It has been tested with a Kvaser USB Leaf Light (one network interface)
> >> connected to a pch_can interface.
> >> The firmware version of the Kvaser device was 2.5.205.
> > 
> > I don't remember, have the USB people already had a look on your driver
> > and gave some comments?
> 
> IIRC, Oliver Neukum commented on v2. Actually we ignored v3 :(, sorry!

No problem. Thanks for the review.

> 
> > From the CAN and network point of view looks good, some comments inline.
> > Would be fine, if Wolfgang can give comments or Ack about the error
> > frame generation.
> 
> Olivier already sent candump traces for no cable connected and
> short-circuiting CAN high and low. The error handling looked good, IIRC,
> at least as good as the firmware can do... more comments inline...
> 
> >> List of Kvaser devices supported by the driver:
> >>   - Kvaser Leaf prototype (P010v2 and v3)
> >>   - Kvaser Leaf Light (P010v3)
> >>   - Kvaser Leaf Professional HS
> >>   - Kvaser Leaf SemiPro HS
> >>   - Kvaser Leaf Professional LS
> >>   - Kvaser Leaf Professional SWC
> >>   - Kvaser Leaf Professional LIN
> >>   - Kvaser Leaf SemiPro LS
> >>   - Kvaser Leaf SemiPro SWC
> >>   - Kvaser Memorator II, Prototype
> >>   - Kvaser Memorator II HS/HS
> >>   - Kvaser USBcan Professional HS/HS
> >>   - Kvaser Leaf Light GI
> >>   - Kvaser Leaf Professional HS (OBD-II connector)
> >>   - Kvaser Memorator Professional HS/LS
> >>   - Kvaser Leaf Light "China"
> >>   - Kvaser BlackBird SemiPro
> >>   - Kvaser OEM Mercury
> >>   - Kvaser OEM Leaf
> >>   - Kvaser USBcan R
> >>
> >> Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
> >> ---
> >> Changes since v3:
> >>  - add support for CMD_LOG_MESSAGE
> >>
> >>  drivers/net/can/usb/Kconfig      |   33 +
> >>  drivers/net/can/usb/Makefile     |    1 +
> >>  drivers/net/can/usb/kvaser_usb.c | 1555 ++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 1589 insertions(+)
> >>  create mode 100644 drivers/net/can/usb/kvaser_usb.c
> >>
> >> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> >> index 0a68768..578955f 100644
> >> --- a/drivers/net/can/usb/Kconfig
> >> +++ b/drivers/net/can/usb/Kconfig
> >> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
> >>            This driver supports the CAN-USB/2 interface
> >>            from esd electronic system design gmbh (http://www.esd.eu).
> >>  
> >> +config CAN_KVASER_USB
> >> +	tristate "Kvaser CAN/USB interface"
> >> +	---help---
> >> +	  This driver adds support for Kvaser CAN/USB devices like Kvaser
> >> +	  Leaf Light.
> >> +
> >> +	  The driver gives support for the following devices:
> >> +	    - Kvaser Leaf prototype (P010v2 and v3)
> >> +	    - Kvaser Leaf Light (P010v3)
> >> +	    - Kvaser Leaf Professional HS
> >> +	    - Kvaser Leaf SemiPro HS
> >> +	    - Kvaser Leaf Professional LS
> >> +	    - Kvaser Leaf Professional SWC
> >> +	    - Kvaser Leaf Professional LIN
> >> +	    - Kvaser Leaf SemiPro LS
> >> +	    - Kvaser Leaf SemiPro SWC
> >> +	    - Kvaser Memorator II, Prototype
> >> +	    - Kvaser Memorator II HS/HS
> >> +	    - Kvaser USBcan Professional HS/HS
> >> +	    - Kvaser Leaf Light GI
> >> +	    - Kvaser Leaf Professional HS (OBD-II connector)
> >> +	    - Kvaser Memorator Professional HS/LS
> >> +	    - Kvaser Leaf Light "China"
> >> +	    - Kvaser BlackBird SemiPro
> >> +	    - Kvaser OEM Mercury
> >> +	    - Kvaser OEM Leaf
> >> +	    - Kvaser USBcan R
> >> +
> >> +	  If unsure, say N.
> >> +
> >> +	  To compile this driver as a module, choose M here: the
> >> +	  module will be called kvaser_usb.
> >> +
> >>  config CAN_PEAK_USB
> >>  	tristate "PEAK PCAN-USB/USB Pro interfaces"
> >>  	---help---
> >> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> >> index da6d1d3..80a2ee4 100644
> >> --- a/drivers/net/can/usb/Makefile
> >> +++ b/drivers/net/can/usb/Makefile
> >> @@ -4,6 +4,7 @@
> >>  
> >>  obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
> >>  obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
> >> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
> >>  obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
> >>  
> >>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> >> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> >> new file mode 100644
> >> index 0000000..3509ca5
> >> --- /dev/null
> >> +++ b/drivers/net/can/usb/kvaser_usb.c
> >> @@ -0,0 +1,1555 @@
> >> +/*
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License as
> >> + * published by the Free Software Foundation version 2.
> >> + *
> >> + * Parts of this driver are based on the following:
> >> + *  - Kvaser linux leaf driver (version 4.78)
> >> + *  - CAN driver for esd CAN-USB/2
> >> + *
> >> + * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
> >> + * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>, esd gmbh
> >> + * Copyright (C) 2012 Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
> >> + */
> >> +
> >> +#include <linux/init.h>
> >> +#include <linux/completion.h>
> >> +#include <linux/module.h>
> >> +#include <linux/netdevice.h>
> >> +#include <linux/usb.h>
> >> +
> >> +#include <linux/can.h>
> >> +#include <linux/can/dev.h>
> >> +#include <linux/can/error.h>
> >> +
> >> +#define MAX_TX_URBS			16
> >> +#define MAX_RX_URBS			4
> >> +#define START_TIMEOUT			1000 /* msecs */
> >> +#define STOP_TIMEOUT			1000 /* msecs */
> >> +#define USB_SEND_TIMEOUT		1000 /* msecs */
> >> +#define USB_RECV_TIMEOUT		1000 /* msecs */
> >> +#define RX_BUFFER_SIZE			3072
> >> +#define CAN_USB_CLOCK			8000000
> >> +#define MAX_NET_DEVICES			3
> >> +
> >> +/* Kvaser USB devices */
> >> +#define KVASER_VENDOR_ID		0x0bfd
> >> +#define USB_LEAF_DEVEL_PRODUCT_ID	10
> >> +#define USB_LEAF_LITE_PRODUCT_ID	11
> >> +#define USB_LEAF_PRO_PRODUCT_ID		12
> >> +#define USB_LEAF_SPRO_PRODUCT_ID	14
> >> +#define USB_LEAF_PRO_LS_PRODUCT_ID	15
> >> +#define USB_LEAF_PRO_SWC_PRODUCT_ID	16
> >> +#define USB_LEAF_PRO_LIN_PRODUCT_ID	17
> >> +#define USB_LEAF_SPRO_LS_PRODUCT_ID	18
> >> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID	19
> >> +#define USB_MEMO2_DEVEL_PRODUCT_ID	22
> >> +#define USB_MEMO2_HSHS_PRODUCT_ID	23
> >> +#define USB_UPRO_HSHS_PRODUCT_ID	24
> >> +#define USB_LEAF_LITE_GI_PRODUCT_ID	25
> >> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID	26
> >> +#define USB_MEMO2_HSLS_PRODUCT_ID	27
> >> +#define USB_LEAF_LITE_CH_PRODUCT_ID	28
> >> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID	29
> >> +#define USB_OEM_MERCURY_PRODUCT_ID	34
> >> +#define USB_OEM_LEAF_PRODUCT_ID		35
> >> +#define USB_CAN_R_PRODUCT_ID		39
> >> +
> >> +/* USB devices features */
> >> +#define KVASER_HAS_SILENT_MODE		BIT(0)
> >> +#define KVASER_HAS_TXRX_ERRORS		BIT(1)
> >> +
> >> +/* Message header size */
> >> +#define MSG_HEADER_LEN			2
> >> +
> >> +/* Can message flags */
> >> +#define MSG_FLAG_ERROR_FRAME		BIT(0)
> >> +#define MSG_FLAG_OVERRUN		BIT(1)
> >> +#define MSG_FLAG_NERR			BIT(2)
> >> +#define MSG_FLAG_WAKEUP			BIT(3)
> >> +#define MSG_FLAG_REMOTE_FRAME		BIT(4)
> >> +#define MSG_FLAG_RESERVED		BIT(5)
> >> +#define MSG_FLAG_TX_ACK			BIT(6)
> >> +#define MSG_FLAG_TX_REQUEST		BIT(7)
> >> +
> >> +/* Can states */
> >> +#define M16C_STATE_BUS_RESET		BIT(0)
> >> +#define M16C_STATE_BUS_ERROR		BIT(4)
> >> +#define M16C_STATE_BUS_PASSIVE		BIT(5)
> >> +#define M16C_STATE_BUS_OFF		BIT(6)
> >> +
> >> +/* Can msg ids */
> >> +#define CMD_RX_STD_MESSAGE		12
> >> +#define CMD_TX_STD_MESSAGE		13
> >> +#define CMD_RX_EXT_MESSAGE		14
> >> +#define CMD_TX_EXT_MESSAGE		15
> >> +#define CMD_SET_BUS_PARAMS		16
> >> +#define CMD_GET_BUS_PARAMS		17
> >> +#define CMD_GET_BUS_PARAMS_REPLY	18
> >> +#define CMD_GET_CHIP_STATE		19
> >> +#define CMD_CHIP_STATE_EVENT		20
> >> +#define CMD_SET_CTRL_MODE		21
> >> +#define CMD_GET_CTRL_MODE		22
> >> +#define CMD_GET_CTRL_MODE_REPLY		23
> >> +#define CMD_RESET_CHIP			24
> >> +#define CMD_RESET_CARD			25
> >> +#define CMD_START_CHIP			26
> >> +#define CMD_START_CHIP_REPLY		27
> >> +#define CMD_STOP_CHIP			28
> >> +#define CMD_STOP_CHIP_REPLY		29
> >> +#define CMD_GET_CARD_INFO2		32
> >> +#define CMD_GET_CARD_INFO		34
> >> +#define CMD_GET_CARD_INFO_REPLY		35
> >> +#define CMD_GET_SOFTWARE_INFO		38
> >> +#define CMD_GET_SOFTWARE_INFO_REPLY	39
> >> +#define CMD_ERROR_EVENT			45
> >> +#define CMD_FLUSH_QUEUE			48
> >> +#define CMD_RESET_ERROR_COUNTER		49
> >> +#define CMD_TX_ACKNOWLEDGE		50
> >> +#define CMD_CAN_ERROR_EVENT		51
> >> +#define CMD_USB_THROTTLE		77
> >> +#define CMD_LOG_MESSAGE			106
> >> +
> >> +/* error factors */
> >> +#define M16C_EF_ACKE			BIT(0)
> >> +#define M16C_EF_CRCE			BIT(1)
> >> +#define M16C_EF_FORME			BIT(2)
> >> +#define M16C_EF_STFE			BIT(3)
> >> +#define M16C_EF_BITE0			BIT(4)
> >> +#define M16C_EF_BITE1			BIT(5)
> >> +#define M16C_EF_RCVE			BIT(6)
> >> +#define M16C_EF_TRE			BIT(7)
> >> +
> >> +/* bittiming parameters */
> >> +#define KVASER_USB_TSEG1_MIN		1
> >> +#define KVASER_USB_TSEG1_MAX		16
> >> +#define KVASER_USB_TSEG2_MIN		1
> >> +#define KVASER_USB_TSEG2_MAX		8
> >> +#define KVASER_USB_SJW_MAX		4
> >> +#define KVASER_USB_BRP_MIN		1
> >> +#define KVASER_USB_BRP_MAX		64
> >> +#define KVASER_USB_BRP_INC		1
> >> +
> >> +/* ctrl modes */
> >> +#define KVASER_CTRL_MODE_NORMAL		1
> >> +#define KVASER_CTRL_MODE_SILENT		2
> >> +#define KVASER_CTRL_MODE_SELFRECEPTION	3
> >> +#define KVASER_CTRL_MODE_OFF		4
> >> +
> >> +struct kvaser_msg_simple {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_cardinfo {
> >> +	u8 tid;
> >> +	u8 nchannels;
> >> +	__le32 serial_number;
> >> +	__le32 padding;
> >> +	__le32 clock_resolution;
> >> +	__le32 mfgdate;
> >> +	u8 ean[8];
> >> +	u8 hw_revision;
> >> +	u8 usb_hs_mode;
> >> +	__le16 padding2;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_cardinfo2 {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	u8 pcb_id[24];
> >> +	__le32 oem_unlock_code;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_softinfo {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	__le32 sw_options;
> >> +	__le32 fw_version;
> >> +	__le16 max_outstanding_tx;
> >> +	__le16 padding[9];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_busparams {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	__le32 bitrate;
> >> +	u8 tseg1;
> >> +	u8 tseg2;
> >> +	u8 sjw;
> >> +	u8 no_samp;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_tx_can {
> >> +	u8 channel;
> >> +	u8 tid;
> >> +	u8 msg[14];
> >> +	u8 padding;
> >> +	u8 flags;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_rx_can {
> >> +	u8 channel;
> >> +	u8 flag;
> >> +	__le16 time[3];
> >> +	u8 msg[14];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_chip_state_event {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	__le16 time[3];
> >> +	u8 tx_errors_count;
> >> +	u8 rx_errors_count;
> >> +	u8 status;
> >> +	u8 padding[3];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_tx_acknowledge {
> >> +	u8 channel;
> >> +	u8 tid;
> >> +	__le16 time[3];
> >> +	u8 flags;
> >> +	u8 time_offset;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_error_event {
> >> +	u8 tid;
> >> +	u8 flags;
> >> +	__le16 time[3];
> >> +	u8 channel;
> >> +	u8 padding;
> >> +	u8 tx_errors_count;
> >> +	u8 rx_errors_count;
> >> +	u8 status;
> >> +	u8 error_factor;
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_ctrl_mode {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	u8 ctrl_mode;
> >> +	u8 padding[3];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_flush_queue {
> >> +	u8 tid;
> >> +	u8 channel;
> >> +	u8 flags;
> >> +	u8 padding[3];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg_log_message {
> >> +	u8 channel;
> >> +	u8 flags;
> >> +	__le16 time[3];
> >> +	u8 dlc;
> >> +	u8 time_offset;
> >> +	__le32 id;
> >> +	u8 data[8];
> >> +} __packed;
> >> +
> >> +struct kvaser_msg {
> >> +	u8 len;
> >> +	u8 id;
> >> +	union	{
> >> +		struct kvaser_msg_simple simple;
> >> +		struct kvaser_msg_cardinfo cardinfo;
> >> +		struct kvaser_msg_cardinfo2 cardinfo2;
> >> +		struct kvaser_msg_softinfo softinfo;
> >> +		struct kvaser_msg_busparams busparams;
> >> +		struct kvaser_msg_tx_can tx_can;
> >> +		struct kvaser_msg_rx_can rx_can;
> >> +		struct kvaser_msg_chip_state_event chip_state_event;
> >> +		struct kvaser_msg_tx_acknowledge tx_acknowledge;
> >> +		struct kvaser_msg_error_event error_event;
> >> +		struct kvaser_msg_ctrl_mode ctrl_mode;
> >> +		struct kvaser_msg_flush_queue flush_queue;
> >> +		struct kvaser_msg_log_message log_message;
> >> +	} u;
> >> +} __packed;
> >> +
> >> +struct kvaser_usb_tx_urb_context {
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	u32 echo_index;
> >> +	int dlc;
> >> +};
> >> +
> >> +struct kvaser_usb {
> >> +	struct usb_device *udev;
> >> +	struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
> >> +
> >> +	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> >> +	struct usb_anchor rx_submitted;
> >> +
> >> +	u32 fw_version;
> >> +	unsigned int nchannels;
> >> +
> >> +	bool rxinitdone;
> >> +	void *rxbuf[MAX_RX_URBS];
> >> +	dma_addr_t rxbuf_dma[MAX_RX_URBS];
> >> +};
> >> +
> >> +struct kvaser_usb_net_priv {
> >> +	struct can_priv can;
> >> +
> >> +	atomic_t active_tx_urbs;
> >> +	struct usb_anchor tx_submitted;
> >> +	struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
> >> +
> >> +	struct completion start_comp, stop_comp;
> >> +
> >> +	struct kvaser_usb *dev;
> >> +	struct net_device *netdev;
> >> +	int channel;
> >> +
> >> +	struct can_berr_counter bec;
> >> +};
> >> +
> >> +static struct usb_device_id kvaser_usb_table[] = {
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> >> +			       KVASER_HAS_SILENT_MODE },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID),
> >> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> >> +	{ }
> >> +};
> >> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> >> +
> >> +static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev,
> >> +				      struct kvaser_msg *msg)
> >> +{
> >> +	int actual_len;
> >> +
> >> +	return usb_bulk_msg(dev->udev,
> >> +			    usb_sndbulkpipe(dev->udev,
> >> +					dev->bulk_out->bEndpointAddress),
> >> +			    msg, msg->len, &actual_len,
> >> +			    USB_SEND_TIMEOUT);
> >> +}
> >> +
> >> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
> >> +			       struct kvaser_msg *msg)
> >> +{
> >> +	struct kvaser_msg *tmp;
> >> +	void *buf;
> >> +	int actual_len;
> >> +	int err;
> >> +	int pos = 0;
> >> +
> >> +	buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
> >> +	if (!buf)
> >> +		return -ENOMEM;
> >> +
> >> +	err = usb_bulk_msg(dev->udev,
> >> +			   usb_rcvbulkpipe(dev->udev,
> >> +					   dev->bulk_in->bEndpointAddress),
> >> +			   buf, RX_BUFFER_SIZE, &actual_len,
> >> +			   USB_RECV_TIMEOUT);
> >> +	if (err < 0)
> >> +		goto end;
> >> +
> >> +	while (pos <= actual_len - MSG_HEADER_LEN) {
> >> +		tmp = buf + pos;
> >> +
> >> +		if (!tmp->len)
> >> +			break;
> >> +
> >> +		if (pos + tmp->len > actual_len) {
> >> +			dev_err(dev->udev->dev.parent, "Format error\n");
> >> +			break;
> >> +		}
> >> +
> >> +		if (tmp->id == id) {
> >> +			memcpy(msg, tmp, tmp->len);
> >> +			goto end;
> >> +		}
> >> +
> >> +		pos += tmp->len;
> >> +	}
> >> +
> >> +	err = -EINVAL;
> >> +
> >> +end:
> >> +	kfree(buf);
> >> +
> >> +	return err;
> >> +}
> >> +
> >> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> >> +				      u8 msg_id, int channel)
> >> +{
> >> +	struct kvaser_msg msg = {
> >> +		.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
> >> +		.id = msg_id,
> >> +		.u.simple.channel = channel,
> >> +		.u.simple.tid = 0xff,
> >> +	};
> >> +
> >> +	return kvaser_usb_send_msg(dev, &msg);
> >> +}
> >> +
> >> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> >> +{
> >> +	struct kvaser_msg msg;
> >> +	int err;
> >> +
> >> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> >> +{
> >> +	struct kvaser_msg msg;
> >> +	int err;
> >> +
> >> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	dev->nchannels = msg.u.cardinfo.nchannels;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> >> +				      const struct kvaser_msg *msg)
> >> +{
> >> +	struct net_device_stats *stats;
> >> +	struct kvaser_usb_tx_urb_context *context;
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	struct sk_buff *skb;
> >> +	struct can_frame *cf;
> >> +	u8 channel = msg->u.tx_acknowledge.channel;
> >> +	u8 tid = msg->u.tx_acknowledge.tid;
> >> +
> >> +	if (channel >= dev->nchannels) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Invalid channel number (%d)\n", channel);
> >> +		return;
> >> +	}
> >> +
> >> +	priv = dev->nets[channel];
> >> +
> >> +	if (!netif_device_present(priv->netdev))
> >> +		return;
> >> +
> >> +	stats = &priv->netdev->stats;
> >> +
> >> +	context = &priv->tx_contexts[tid % MAX_TX_URBS];
> >> +
> >> +	/* Sometimes the state change doesn't come after a bus-off event */
> >> +	if (priv->can.restart_ms &&
> >> +	    (priv->can.state >= CAN_STATE_BUS_OFF)) {
> >> +		skb = alloc_can_err_skb(priv->netdev, &cf);
> >> +		if (skb) {
> >> +			cf->can_id |= CAN_ERR_RESTARTED;
> >> +			netif_rx(skb);
> >> +
> >> +			stats->rx_packets++;
> >> +			stats->rx_bytes += cf->can_dlc;
> >> +		} else {
> >> +			netdev_err(priv->netdev,
> >> +				   "No memory left for err_skb\n");
> >> +		}
> >> +
> >> +		priv->can.can_stats.restarts++;
> >> +		netif_carrier_on(priv->netdev);
> >> +
> >> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >> +	}
> >> +
> >> +	stats->tx_packets++;
> >> +	stats->tx_bytes += context->dlc;
> >> +	can_get_echo_skb(priv->netdev, context->echo_index);
> >> +
> >> +	context->echo_index = MAX_TX_URBS;
> >> +	atomic_dec(&priv->active_tx_urbs);
> >> +
> >> +	netif_wake_queue(priv->netdev);
> >> +}
> >> +
> >> +static void kvaser_usb_simple_msg_callback(struct urb *urb)
> >> +{
> >> +	struct net_device *netdev = urb->context;
> >> +
> >> +	kfree(urb->transfer_buffer);
> >> +
> >> +	if (urb->status)
> >> +		netdev_warn(netdev, "urb status received: %d\n",
> >> +			    urb->status);
> >> +}
> >> +
> >> +static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
> >> +				       u8 msg_id)
> >> +{
> >> +	struct kvaser_usb *dev = priv->dev;
> >> +	struct net_device *netdev = priv->netdev;
> >> +	struct kvaser_msg *msg;
> >> +	struct urb *urb;
> >> +	void *buf;
> >> +	int err;
> >> +
> >> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
> >> +	if (!urb) {
> >> +		netdev_err(netdev, "No memory left for URBs\n");
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
> >> +	if (!buf) {
> > 
> > Do you have to free the usb you just allocated?

Yes it's missing.

> > 
> >> +		netdev_err(netdev, "No memory left for USB buffer\n");
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	msg = (struct kvaser_msg *)buf;
> >> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> >> +	msg->id = msg_id;
> >> +	msg->u.simple.channel = priv->channel;
> >> +
> >> +	usb_fill_bulk_urb(urb, dev->udev,
> >> +			  usb_sndbulkpipe(dev->udev,
> >> +					  dev->bulk_out->bEndpointAddress),
> >> +			  buf, msg->len,
> >> +			  kvaser_usb_simple_msg_callback, priv);
> >> +	usb_anchor_urb(urb, &priv->tx_submitted);
> >> +
> >> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> >> +	if (err) {
> >> +		netdev_err(netdev, "Error transmitting URB\n");
> >> +		usb_unanchor_urb(urb);
> >> +		kfree(buf);
> >> +		return err;
> > 
> > and here?

missing too.

> > 
> >> +	}
> >> +
> >> +	usb_free_urb(urb);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> >> +{
> >> +	int i;
> >> +
> >> +	usb_kill_anchored_urbs(&priv->tx_submitted);
> >> +	atomic_set(&priv->active_tx_urbs, 0);
> >> +
> >> +	for (i = 0; i < MAX_TX_URBS; i++)
> >> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> >> +}
> >> +
> >> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >> +				const struct kvaser_msg *msg)
> >> +{
> >> +	struct can_frame *cf;
> >> +	struct sk_buff *skb;
> >> +	struct net_device_stats *stats;
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	unsigned int new_state;
> >> +	u8 channel, status, txerr, rxerr, error_factor;
> >> +
> >> +	switch (msg->id) {
> >> +	case CMD_CAN_ERROR_EVENT:
> >> +		channel = msg->u.error_event.channel;
> >> +		status =  msg->u.error_event.status;
> >> +		txerr = msg->u.error_event.tx_errors_count;
> >> +		rxerr = msg->u.error_event.rx_errors_count;
> >> +		error_factor = msg->u.error_event.error_factor;
> >> +		break;
> >> +	case CMD_LOG_MESSAGE:
> >> +		channel = msg->u.log_message.channel;
> >> +		status = msg->u.log_message.data[0];
> >> +		txerr = msg->u.log_message.data[2];
> >> +		rxerr = msg->u.log_message.data[3];
> >> +		error_factor = msg->u.log_message.data[1];
> >> +		break;
> >> +	case CMD_CHIP_STATE_EVENT:
> >> +		channel = msg->u.chip_state_event.channel;
> >> +		status =  msg->u.chip_state_event.status;
> >> +		txerr = msg->u.chip_state_event.tx_errors_count;
> >> +		rxerr = msg->u.chip_state_event.rx_errors_count;
> >> +		error_factor = 0;
> >> +		break;
> >> +	default:
> >> +		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> >> +			msg->id);
> >> +		return;
> >> +	}
> >> +
> >> +	if (channel >= dev->nchannels) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Invalid channel number (%d)\n", channel);
> >> +		return;
> >> +	}
> >> +
> >> +	priv = dev->nets[channel];
> >> +	stats = &priv->netdev->stats;
> >> +
> >> +	if (status & M16C_STATE_BUS_RESET) {
> >> +		kvaser_usb_unlink_tx_urbs(priv);
> >> +		return;
> >> +	}
> >> +
> >> +	skb = alloc_can_err_skb(priv->netdev, &cf);
> >> +	if (!skb) {
> >> +		stats->rx_dropped++;
> >> +		return;
> >> +	}
> >> +
> >> +	cf->can_id |= CAN_ERR_BUSERROR;
> 
> At state change is *not* a bus error. The line above should e moved into
> the "if (error_factor)" block below.

ok

> 
> >> +
> >> +	new_state = priv->can.state;
> >> +
> >> +	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> >> +
> >> +	if (status & M16C_STATE_BUS_OFF) {
> >> +		cf->can_id |= CAN_ERR_BUSOFF;
> >> +
> >> +		priv->can.can_stats.bus_off++;
> >> +		if (!priv->can.restart_ms)
> >> +			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> >> +
> >> +		netif_carrier_off(priv->netdev);
> >> +
> >> +		new_state = CAN_STATE_BUS_OFF;
> >> +	}
> >> +
> >> +	if (status & M16C_STATE_BUS_PASSIVE) {
> > 
> > else if ()
> > 
> > as bus passive and bus off is mutually exclusive.

ok

> > 
> >> +		if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> >> +			cf->can_id |= CAN_ERR_CRTL;
> >> +
> >> +			if ((txerr > 0) || (rxerr > 0))
> 
> if (txerr | rxerr) ?

ok

> 
> >> +				cf->data[1] = (txerr > rxerr)
> >> +						? CAN_ERR_CRTL_TX_PASSIVE
> >> +						: CAN_ERR_CRTL_RX_PASSIVE;
> >> +			else
> >> +				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> >> +					      CAN_ERR_CRTL_RX_PASSIVE;
> >> +
> >> +			priv->can.can_stats.error_passive++;
> >> +		}
> >> +
> >> +		new_state = CAN_STATE_ERROR_PASSIVE;
> >> +	}
> >> +
> >> +	if (status == M16C_STATE_BUS_ERROR) {
> >> +		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> >> +		    ((txerr > 96) || (rxerr > 96))) {
> > 
> > Is is >= 96 ?
> 
> Yep.

ok

> 
> >> +			cf->can_id |= CAN_ERR_CRTL;
> >> +			cf->data[1] = (txerr > rxerr)
> >> +					? CAN_ERR_CRTL_TX_WARNING
> >> +					: CAN_ERR_CRTL_RX_WARNING;
> >> +
> >> +			priv->can.can_stats.error_warning++;
> >> +			new_state = CAN_STATE_ERROR_WARNING;
> >> +		} else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
> >> +			cf->can_id |= CAN_ERR_PROT;
> >> +			cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >> +
> >> +			new_state = CAN_STATE_ERROR_ACTIVE;
> >> +		}
> >> +	}
> >> +
> >> +	if (!status) {
> >> +		cf->can_id |= CAN_ERR_PROT;
> >> +		cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >> +
> >> +		new_state = CAN_STATE_ERROR_ACTIVE;
> >> +	}
> >> +
> >> +	if (priv->can.restart_ms &&
> >> +	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
> >> +	    (new_state < CAN_STATE_BUS_OFF)) {
> >> +		cf->can_id |= CAN_ERR_RESTARTED;
> >> +		netif_carrier_on(priv->netdev);
> >> +
> >> +		priv->can.can_stats.restarts++;
> >> +	}
> >> +
> >> +	if (error_factor) {
> >> +		priv->can.can_stats.bus_error++;
> >> +		stats->rx_errors++;
> >> +
> >> +		cf->can_id |= CAN_ERR_PROT;
> >> +
> >> +		if (error_factor & M16C_EF_ACKE)
> >> +			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> >> +		if (error_factor & M16C_EF_CRCE)
> >> +			cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> >> +					CAN_ERR_PROT_LOC_CRC_DEL);
> >> +		if (error_factor & M16C_EF_FORME)
> >> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> >> +		if (error_factor & M16C_EF_STFE)
> >> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> >> +		if (error_factor & M16C_EF_BITE0)
> >> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> >> +		if (error_factor & M16C_EF_BITE1)
> >> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> >> +		if (error_factor & M16C_EF_TRE)
> >> +			cf->data[2] |= CAN_ERR_PROT_TX;
> >> +	}
> >> +
> >> +	cf->data[6] = txerr;
> >> +	cf->data[7] = rxerr;
> >> +
> >> +	priv->bec.txerr = txerr;
> >> +	priv->bec.rxerr = rxerr;
> >> +
> >> +	priv->can.state = new_state;
> >> +
> >> +	netif_rx(skb);
> >> +
> >> +	stats->rx_packets++;
> >> +	stats->rx_bytes += cf->can_dlc;
> >> +}
> >> +
> >> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> >> +				  const struct kvaser_msg *msg)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	struct can_frame *cf;
> >> +	struct sk_buff *skb;
> >> +	struct net_device_stats *stats;
> >> +	u8 channel = msg->u.rx_can.channel;
> >> +
> >> +	if (channel >= dev->nchannels) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Invalid channel number (%d)\n", channel);
> >> +		return;
> >> +	}
> >> +
> >> +	priv = dev->nets[channel];
> >> +	stats = &priv->netdev->stats;
> >> +
> >> +	skb = alloc_can_skb(priv->netdev, &cf);
> >> +	if (!skb) {
> >> +		stats->tx_dropped++;
> >> +		return;
> >> +	}
> >> +
> >> +	cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> >> +		     (msg->u.rx_can.msg[1] & 0x3f);
> >> +	cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> >> +
> >> +	if (msg->id == CMD_RX_EXT_MESSAGE) {
> >> +		cf->can_id <<= 18;
> >> +		cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> >> +			      ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> >> +			      (msg->u.rx_can.msg[4] & 0x3f);
> >> +		cf->can_id |= CAN_EFF_FLAG;
> >> +	}
> >> +
> >> +	if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> >> +		cf->can_id |= CAN_RTR_FLAG;
> >> +	} else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> >> +					 MSG_FLAG_NERR)) {
> >> +		cf->can_id |= CAN_ERR_FLAG;
> >> +		cf->can_dlc = CAN_ERR_DLC;
> 
> > Please move the error skb creation handling into a subfunction, use
> > can_alloc_err_skb() in that function. Please move the:
> > 
> >     if (msg->u.rx_can.flag & ...)
> > 
> > up in this function, before the alloc_can_skb().
> > 
> >> +
> >> +		netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> >> +			   msg->u.rx_can.flag);
> >> +
> >> +		stats->rx_errors++;
> 
> This an *error* which does normally not happen, IIRC. Therefore I do not
> see a need to pass it to the user as error message.

ok I do not pass an error frame for such kind of message.

> 
> >> +	} else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> >> +		cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
> >> +		cf->can_dlc = CAN_ERR_DLC;
> >> +		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> >> +
> >> +		stats->rx_over_errors++;
> >> +		stats->rx_errors++;
> > 
> > This should go into the error skb generation function, too.
> > 
> >> +	} else if (!msg->u.rx_can.flag) {
> >> +		memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
> > 
> > Please don't copy the contents of RTR frames.

It's not copied. When it's a RTR frame, "msg->u.rx_can.flag" is set to
MSG_FLAG_REMOTE_FRAME.

> > 
> >> +	} else {
> >> +		kfree_skb(skb);
> > 
> > After you have moved the error skb generation into a seperate function,
> > you should get rid of the kfree(skb), too. The function should look like
> > this (pseude code):
> > 
> > static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> > 				  const struct kvaser_msg *msg)
> > {
> > 	if (channel_invalid())
> > 		return;
> > 
> > 	if (msg->u.rx_can.flag & ERROR) {
> > 		kvaser_usb_rx_can_err_msg();
> > 		return;
> > 	} else if (msg->u.rx_can.flag & INVALID_FRAME) {
> > 		return;
> > 	}
> > 
> > 	skb = alloc_can_skb();
> > 
> > 	/* existing dlc, rtr and data handling code */
> > 	...
> > }

ok

> > 
> > 
> >> +		return;
> >> +	}
> >> +
> >> +	netif_rx(skb);
> >> +
> >> +	stats->rx_packets++;
> >> +	stats->rx_bytes += cf->can_dlc;
> >> +}
> >> +
> >> +static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
> >> +					const struct kvaser_msg *msg)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	u8 channel = msg->u.simple.channel;
> >> +
> >> +	if (channel >= dev->nchannels) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Invalid channel number (%d)\n", channel);
> >> +		return;
> >> +	}
> >> +
> >> +	priv = dev->nets[channel];
> >> +
> >> +	if (completion_done(&priv->start_comp) &&
> >> +	    netif_queue_stopped(priv->netdev)) {
> >> +		netif_wake_queue(priv->netdev);
> >> +	} else {
> >> +		netif_start_queue(priv->netdev);
> >> +		complete(&priv->start_comp);
> >> +	}
> >> +}
> >> +
> >> +static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev,
> >> +				       const struct kvaser_msg *msg)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	u8 channel = msg->u.simple.channel;
> >> +
> >> +	if (channel >= dev->nchannels) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Invalid channel number (%d)\n", channel);
> >> +		return;
> >> +	}
> >> +
> >> +	priv = dev->nets[channel];
> >> +
> >> +	complete(&priv->stop_comp);
> >> +}
> >> +
> >> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> >> +				      const struct kvaser_msg *msg)
> >> +{
> >> +	switch (msg->id) {
> >> +	case CMD_START_CHIP_REPLY:
> >> +		kvaser_usb_start_chip_reply(dev, msg);
> >> +		break;
> >> +
> >> +	case CMD_STOP_CHIP_REPLY:
> >> +		kvaser_usb_stop_chip_reply(dev, msg);
> >> +		break;
> >> +
> >> +	case CMD_RX_STD_MESSAGE:
> >> +	case CMD_RX_EXT_MESSAGE:
> >> +		kvaser_usb_rx_can_msg(dev, msg);
> >> +		break;
> >> +
> >> +	case CMD_CHIP_STATE_EVENT:
> >> +	case CMD_CAN_ERROR_EVENT:
> >> +		kvaser_usb_rx_error(dev, msg);
> >> +		break;
> >> +
> >> +	case CMD_LOG_MESSAGE:
> >> +		if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME)
> >> +			kvaser_usb_rx_error(dev, msg);
> >> +		break;
> >> +
> >> +	case CMD_TX_ACKNOWLEDGE:
> >> +		kvaser_usb_tx_acknowledge(dev, msg);
> >> +		break;
> >> +
> >> +	default:
> >> +		dev_warn(dev->udev->dev.parent,
> >> +			 "Unhandled message (%d)\n", msg->id);
> >> +		break;
> >> +	}
> >> +}
> >> +
> >> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
> >> +{
> >> +	struct kvaser_usb *dev = urb->context;
> >> +	struct kvaser_msg *msg;
> >> +	int pos = 0;
> >> +	int err, i;
> >> +
> >> +	switch (urb->status) {
> >> +	case 0:
> >> +		break;
> >> +	case -ENOENT:
> >> +	case -ESHUTDOWN:
> >> +		return;
> >> +	default:
> >> +		dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
> >> +			 urb->status);
> >> +		goto resubmit_urb;
> >> +	}
> >> +
> >> +	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
> >> +		msg = urb->transfer_buffer + pos;
> >> +
> >> +		if (!msg->len)
> >> +			break;
> >> +
> >> +		if (pos + msg->len > urb->actual_length) {
> >> +			dev_err(dev->udev->dev.parent, "Format error\n");
> >> +			break;
> >> +		}
> >> +
> >> +		kvaser_usb_handle_message(dev, msg);
> >> +
> >> +		pos += msg->len;
> >> +	}
> >> +
> >> +resubmit_urb:
> >> +	usb_fill_bulk_urb(urb, dev->udev,
> >> +			  usb_rcvbulkpipe(dev->udev,
> >> +					  dev->bulk_in->bEndpointAddress),
> >> +			  urb->transfer_buffer, RX_BUFFER_SIZE,
> >> +			  kvaser_usb_read_bulk_callback, dev);
> >> +
> >> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> >> +	if (err == -ENODEV) {
> >> +		for (i = 0; i < dev->nchannels; i++) {
> >> +			if (!dev->nets[i])
> >> +				continue;
> >> +
> >> +			netif_device_detach(dev->nets[i]->netdev);
> >> +		}
> >> +	} else if (err) {
> >> +		dev_err(dev->udev->dev.parent,
> >> +			"Failed resubmitting read bulk urb: %d\n", err);
> >> +	}
> >> +
> >> +	return;
> >> +}
> >> +
> >> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
> >> +{
> >> +	int i, err = 0;
> >> +
> >> +	if (dev->rxinitdone)
> >> +		return 0;
> >> +
> >> +	for (i = 0; i < MAX_RX_URBS; i++) {
> >> +		struct urb *urb = NULL;
> >> +		u8 *buf = NULL;
> >> +		dma_addr_t buf_dma;
> >> +
> >> +		urb = usb_alloc_urb(0, GFP_KERNEL);
> >> +		if (!urb) {
> >> +			dev_warn(dev->udev->dev.parent,
> >> +				 "No memory left for URBs\n");
> >> +			err = -ENOMEM;
> >> +			break;
> >> +		}
> >> +
> >> +		buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
> >> +					 GFP_KERNEL, &buf_dma);
> >> +		if (!buf) {
> >> +			dev_warn(dev->udev->dev.parent,
> >> +				 "No memory left for USB buffer\n");
> >> +			usb_free_urb(urb);
> >> +			err = -ENOMEM;
> >> +			break;
> >> +		}
> >> +
> >> +		usb_fill_bulk_urb(urb, dev->udev,
> >> +				  usb_rcvbulkpipe(dev->udev,
> >> +					  dev->bulk_in->bEndpointAddress),
> >> +				  buf, RX_BUFFER_SIZE,
> >> +				  kvaser_usb_read_bulk_callback,
> >> +				  dev);
> >> +		urb->transfer_dma = buf_dma;
> >> +		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >> +		usb_anchor_urb(urb, &dev->rx_submitted);
> >> +
> >> +		err = usb_submit_urb(urb, GFP_KERNEL);
> >> +		if (err) {
> >> +			usb_unanchor_urb(urb);
> >> +			usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> >> +					  buf_dma);
> > 
> > Do you have to call usb_free_usb() here, or does the usb frame work take
> > care of this?

It's missing. Sorry.

> > 
> >> +			break;
> >> +		}
> >> +
> >> +		dev->rxbuf[i] = buf;
> >> +		dev->rxbuf_dma[i] = buf_dma;
> >> +
> >> +		usb_free_urb(urb);
> >> +	}
> >> +
> >> +	if (i == 0) {
> >> +		dev_warn(dev->udev->dev.parent,
> >> +			 "Cannot setup read URBs, error %d\n", err);
> >> +		return err;
> >> +	} else if (i < MAX_RX_URBS) {
> >> +		dev_warn(dev->udev->dev.parent,
> >> +			 "RX performances may be slow\n");
> >> +	}
> >> +
> >> +	dev->rxinitdone = true;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
> >> +{
> >> +	struct kvaser_msg msg = {
> >> +		.id = CMD_SET_CTRL_MODE,
> >> +		.len = MSG_HEADER_LEN +
> >> +		       sizeof(struct kvaser_msg_ctrl_mode),
> >> +		.u.ctrl_mode.tid = 0xff,
> >> +		.u.ctrl_mode.channel = priv->channel,
> >> +	};
> >> +
> >> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> >> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> >> +	else
> >> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> >> +
> >> +	return kvaser_usb_send_msg(priv->dev, &msg);
> >> +}
> >> +
> >> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> >> +{
> >> +	int err;
> >> +
> >> +	init_completion(&priv->start_comp);
> >> +
> >> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
> >> +					 priv->channel);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	if (!wait_for_completion_timeout(&priv->start_comp,
> >> +					 msecs_to_jiffies(START_TIMEOUT)))
> >> +		return -ETIMEDOUT;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_open(struct net_device *netdev)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +	struct kvaser_usb *dev = priv->dev;
> >> +	int err;
> >> +
> >> +	err = open_candev(netdev);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	err = kvaser_usb_setup_rx_urbs(dev);
> >> +	if (err)
> >> +		goto error;
> >> +
> >> +	err = kvaser_usb_set_opt_mode(priv);
> >> +	if (err)
> >> +		goto error;
> >> +
> >> +	err = kvaser_usb_start_chip(priv);
> >> +	if (err) {
> >> +		netdev_warn(netdev, "Cannot start device, error %d\n", err);
> >> +		goto error;
> >> +	}
> >> +
> >> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >> +
> >> +	return 0;
> >> +
> >> +error:
> >> +	close_candev(netdev);
> >> +	return err;
> >> +}
> >> +
> >> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> >> +{
> >> +	int i;
> >> +
> >> +	usb_kill_anchored_urbs(&dev->rx_submitted);
> >> +
> >> +	for (i = 0; i < MAX_RX_URBS; i++)
> >> +		usb_free_coherent(dev->udev, RX_BUFFER_SIZE,
> >> +				  dev->rxbuf[i],
> >> +				  dev->rxbuf_dma[i]);
> >> +
> >> +	for (i = 0; i < MAX_NET_DEVICES; i++) {
> >> +		struct kvaser_usb_net_priv *priv = dev->nets[i];
> >> +
> >> +		if (priv)
> >> +			kvaser_usb_unlink_tx_urbs(priv);
> >> +	}
> >> +}
> >> +
> >> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
> >> +{
> >> +	int err;
> >> +
> >> +	init_completion(&priv->stop_comp);
> >> +
> >> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
> >> +					 priv->channel);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	if (!wait_for_completion_timeout(&priv->stop_comp,
> >> +					 msecs_to_jiffies(STOP_TIMEOUT)))
> >> +		return -ETIMEDOUT;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
> >> +{
> >> +	struct kvaser_msg msg = {
> >> +		.id = CMD_FLUSH_QUEUE,
> >> +		.len = MSG_HEADER_LEN +
> >> +		       sizeof(struct kvaser_msg_flush_queue),
> >> +		.u.flush_queue.channel = priv->channel,
> >> +		.u.flush_queue.flags = 0x00,
> >> +	};
> >> +
> >> +	return kvaser_usb_send_msg(priv->dev, &msg);
> >> +}
> >> +
> >> +static int kvaser_usb_close(struct net_device *netdev)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +	struct kvaser_usb *dev = priv->dev;
> >> +	int err;
> >> +
> >> +	netif_stop_queue(netdev);
> >> +
> >> +	err = kvaser_usb_flush_queue(priv);
> >> +	if (err)
> >> +		netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
> >> +
> >> +	if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel))
> >> +		netdev_warn(netdev, "Cannot reset card, error %d\n", err);
> >> +
> >> +	err = kvaser_usb_stop_chip(priv);
> >> +	if (err)
> >> +		netdev_warn(netdev, "Cannot stop device, error %d\n", err);
> >> +
> >> +	priv->can.state = CAN_STATE_STOPPED;
> >> +	close_candev(priv->netdev);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
> >> +{
> >> +	struct kvaser_usb_tx_urb_context *context = urb->context;
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	struct net_device *netdev;
> >> +
> >> +	if (WARN_ON(!context))
> >> +		return;
> >> +
> >> +	priv = context->priv;
> >> +	netdev = priv->netdev;
> >> +
> >> +	kfree(urb->transfer_buffer);
> >> +
> >> +	if (!netif_device_present(netdev))
> >> +		return;
> >> +
> >> +	if (urb->status)
> >> +		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
> >> +}
> >> +
> >> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> >> +					 struct net_device *netdev)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +	struct kvaser_usb *dev = priv->dev;
> >> +	struct net_device_stats *stats = &netdev->stats;
> >> +	struct can_frame *cf = (struct can_frame *)skb->data;
> >> +	struct kvaser_usb_tx_urb_context *context = NULL;
> >> +	struct urb *urb;
> >> +	void *buf;
> >> +	struct kvaser_msg *msg;
> >> +	int i, err;
> >> +	int ret = NETDEV_TX_OK;
> >> +
> >> +	if (can_dropped_invalid_skb(netdev, skb))
> >> +		return NETDEV_TX_OK;
> >> +
> >> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
> >> +	if (!urb) {
> >> +		netdev_err(netdev, "No memory left for URBs\n");
> >> +		stats->tx_dropped++;
> >> +		dev_kfree_skb(skb);
> >> +		return NETDEV_TX_OK;
> >> +	}
> >> +
> >> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
> >> +	if (!buf) {
> >> +		netdev_err(netdev, "No memory left for USB buffer\n");
> >> +		stats->tx_dropped++;
> >> +		dev_kfree_skb(skb);
> > What about the urb?

Same as above.

> >> +		goto nobufmem;
> >> +	}
> >> +
> >> +	msg = (struct kvaser_msg *)buf;
> > 
> > nitpick: cast is not needed, as buf is void *

Indeed.

> > 
> >> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> >> +	msg->u.tx_can.flags = 0;
> >> +	msg->u.tx_can.channel = priv->channel;
> >> +
> >> +	if (cf->can_id & CAN_EFF_FLAG) {
> >> +		msg->id = CMD_TX_EXT_MESSAGE;
> >> +		msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> >> +		msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
> >> +		msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
> >> +		msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
> >> +		msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
> >> +	} else {
> >> +		msg->id = CMD_TX_STD_MESSAGE;
> >> +		msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
> >> +		msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
> >> +	}
> >> +
> >> +	msg->u.tx_can.msg[5] = cf->can_dlc;
> >> +	memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> >> +
> >> +	if (cf->can_id & CAN_RTR_FLAG)
> >> +		msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
> >> +		if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> >> +			context = &priv->tx_contexts[i];
> >> +			break;
> >> +		}
> >> +	}
> >> +
> >> +	if (!context) {
> >> +		netdev_warn(netdev, "cannot find free context\n");
> >> +		ret =  NETDEV_TX_BUSY;
> >> +		goto releasebuf;
> >> +	}
> >> +
> >> +	context->priv = priv;
> >> +	context->echo_index = i;
> >> +	context->dlc = cf->can_dlc;
> >> +
> >> +	msg->u.tx_can.tid = context->echo_index;
> >> +
> >> +	usb_fill_bulk_urb(urb, dev->udev,
> >> +			  usb_sndbulkpipe(dev->udev,
> >> +					  dev->bulk_out->bEndpointAddress),
> >> +			  buf, msg->len,
> >> +			  kvaser_usb_write_bulk_callback, context);
> >> +	usb_anchor_urb(urb, &priv->tx_submitted);
> >> +
> >> +	can_put_echo_skb(skb, netdev, context->echo_index);
> >> +
> >> +	atomic_inc(&priv->active_tx_urbs);
> >> +
> >> +	if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
> >> +		netif_stop_queue(netdev);
> >> +
> >> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> >> +	if (unlikely(err)) {
> >> +		can_free_echo_skb(netdev, context->echo_index);
> >> +
> >> +		atomic_dec(&priv->active_tx_urbs);
> >> +		usb_unanchor_urb(urb);
> >> +
> >> +		stats->tx_dropped++;
> >> +
> >> +		if (err == -ENODEV)
> >> +			netif_device_detach(netdev);
> >> +		else
> >> +			netdev_warn(netdev, "Failed tx_urb %d\n", err);
> >> +
> >> +		goto releasebuf;
> >> +	}
> >> +
> >> +	netdev->trans_start = jiffies;
> > 
> > Is this still needed?

No.

> > 
> >> +
> >> +	usb_free_urb(urb);
> >> +
> >> +	return NETDEV_TX_OK;
> >> +
> >> +releasebuf:
> >> +	kfree(buf);
> >> +nobufmem:
> >> +	usb_free_urb(urb);
> >> +	return ret;
> >> +}
> >> +
> >> +static const struct net_device_ops kvaser_usb_netdev_ops = {
> >> +	.ndo_open = kvaser_usb_open,
> >> +	.ndo_stop = kvaser_usb_close,
> >> +	.ndo_start_xmit = kvaser_usb_start_xmit,
> >> +};
> >> +
> >> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
> >> +	.name = "kvaser_usb",
> >> +	.tseg1_min = KVASER_USB_TSEG1_MIN,
> >> +	.tseg1_max = KVASER_USB_TSEG1_MAX,
> >> +	.tseg2_min = KVASER_USB_TSEG2_MIN,
> >> +	.tseg2_max = KVASER_USB_TSEG2_MAX,
> >> +	.sjw_max = KVASER_USB_SJW_MAX,
> >> +	.brp_min = KVASER_USB_BRP_MIN,
> >> +	.brp_max = KVASER_USB_BRP_MAX,
> >> +	.brp_inc = KVASER_USB_BRP_INC,
> >> +};
> >> +
> >> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +	struct can_bittiming *bt = &priv->can.bittiming;
> >> +	struct kvaser_usb *dev = priv->dev;
> >> +	struct kvaser_msg msg = {
> >> +		.id = CMD_SET_BUS_PARAMS,
> >> +		.len = MSG_HEADER_LEN +
> >> +		       sizeof(struct kvaser_msg_busparams),
> >> +		.u.busparams.channel = priv->channel,
> >> +		.u.busparams.tid = 0xff,
> >> +		.u.busparams.bitrate = cpu_to_le32(bt->bitrate),
> >> +		.u.busparams.sjw = bt->sjw,
> >> +		.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
> >> +		.u.busparams.tseg2 = bt->phase_seg2,
> >> +	};
> >> +
> >> +	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> >> +		msg.u.busparams.no_samp = 3;
> >> +	else
> >> +		msg.u.busparams.no_samp = 1;
> >> +
> >> +	return kvaser_usb_send_msg(dev, &msg);
> >> +}
> >> +
> >> +static int kvaser_usb_set_mode(struct net_device *netdev,
> >> +			       enum can_mode mode)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +	int err;
> >> +
> >> +	switch (mode) {
> >> +	case CAN_MODE_START:
> >> +		err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP);
> >> +		if (err)
> >> +			return err;
> >> +		break;
> >> +	default:
> >> +		return -EOPNOTSUPP;
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
> >> +				       struct can_berr_counter *bec)
> >> +{
> >> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >> +
> >> +	*bec = priv->bec;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int kvaser_usb_init_one(struct usb_interface *intf,
> >> +			       const struct usb_device_id *id, int channel)
> >> +{
> >> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
> >> +	struct net_device *netdev;
> >> +	struct kvaser_usb_net_priv *priv;
> >> +	int i, err;
> >> +
> >> +	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
> >> +	if (!netdev) {
> >> +		dev_err(&intf->dev, "Cannot alloc candev\n");
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	priv = netdev_priv(netdev);
> >> +
> >> +	init_completion(&priv->start_comp);
> >> +	init_completion(&priv->stop_comp);
> >> +
> >> +	init_usb_anchor(&priv->tx_submitted);
> >> +	atomic_set(&priv->active_tx_urbs, 0);
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++)
> >> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> >> +
> >> +	priv->dev = dev;
> >> +	priv->netdev = netdev;
> >> +	priv->channel = channel;
> >> +
> >> +	priv->can.state = CAN_STATE_STOPPED;
> >> +	priv->can.clock.freq = CAN_USB_CLOCK;
> >> +	priv->can.bittiming_const = &kvaser_usb_bittiming_const;
> >> +	priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
> >> +	priv->can.do_set_mode = kvaser_usb_set_mode;
> >> +	if (id->driver_info & KVASER_HAS_TXRX_ERRORS)
> >> +		priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
> >> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> >> +	if (id->driver_info & KVASER_HAS_SILENT_MODE)
> >> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
> >> +
> >> +	netdev->flags |= IFF_ECHO;
> >> +
> >> +	netdev->netdev_ops = &kvaser_usb_netdev_ops;
> >> +
> >> +	SET_NETDEV_DEV(netdev, &intf->dev);
> >> +
> >> +	dev->nets[channel] = priv;
> >> +
> >> +	err = register_candev(netdev);
> >> +	if (err) {
> >> +		dev_err(&intf->dev, "Failed to register can device\n");
> >> +		free_candev(netdev);
> >> +		return err;
> >> +	}
> >> +
> >> +	netdev_dbg(netdev, "device registered\n");
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
> >> +				     struct usb_endpoint_descriptor **in,
> >> +				     struct usb_endpoint_descriptor **out)
> >> +{
> >> +	const struct usb_host_interface *iface_desc;
> >> +	struct usb_endpoint_descriptor *endpoint;
> >> +	int i;
> >> +
> >> +	iface_desc = &intf->altsetting[0];
> >> +
> >> +	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> >> +		endpoint = &iface_desc->endpoint[i].desc;
> >> +
> >> +		if (usb_endpoint_is_bulk_in(endpoint))
> >> +			*in = endpoint;
> >> +
> >> +		if (usb_endpoint_is_bulk_out(endpoint))
> >> +			*out = endpoint;
> >> +	}
> >> +}
> >> +
> >> +static int kvaser_usb_probe(struct usb_interface *intf,
> >> +			    const struct usb_device_id *id)
> >> +{
> >> +	struct kvaser_usb *dev;
> >> +	int err = -ENOMEM;
> >> +	int i;
> >> +
> >> +	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
> >> +	if (!dev)
> >> +		return -ENOMEM;
> >> +
> >> +	kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> >> +	if (!dev->bulk_in || !dev->bulk_out) {
> >> +		dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> >> +		return err;
> >> +	}
> >> +
> >> +	dev->udev = interface_to_usbdev(intf);
> >> +
> >> +	init_usb_anchor(&dev->rx_submitted);
> >> +
> >> +	usb_set_intfdata(intf, dev);
> >> +
> >> +	for (i = 0; i < MAX_NET_DEVICES; i++)
> >> +		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
> >> +
> >> +	err = kvaser_usb_get_software_info(dev);
> >> +	if (err) {
> >> +		dev_err(&intf->dev,
> >> +			"Cannot get software infos, error %d\n", err);
> >> +		return err;
> >> +	}
> >> +
> >> +	err = kvaser_usb_get_card_info(dev);
> >> +	if (err) {
> >> +		dev_err(&intf->dev,
> >> +			"Cannot get card infos, error %d\n", err);
> >> +		return err;
> >> +	}
> >> +
> >> +	dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
> >> +		((dev->fw_version >> 24) & 0xff),
> >> +		((dev->fw_version >> 16) & 0xff),
> >> +		(dev->fw_version & 0xffff));
> >> +
> >> +	for (i = 0; i < dev->nchannels; i++)
> >> +		kvaser_usb_init_one(intf, id, i);
> 
> Error checking is not needed?

Yes it is better to check that, I added it in the new version.
If I cannot initialize one of the "dev->nchannels" net interfaces, I
return an error in the probing function.

> 
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static void kvaser_usb_disconnect(struct usb_interface *intf)
> >> +{
> >> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
> >> +	int i;
> >> +
> >> +	usb_set_intfdata(intf, NULL);
> >> +
> >> +	if (!dev)
> >> +		return;
> >> +
> >> +	for (i = 0; i < dev->nchannels; i++) {
> >> +		if (!dev->nets[i])
> >> +			continue;
> >> +
> >> +		unregister_netdev(dev->nets[i]->netdev);
> >> +	}
> >> +
> >> +	kvaser_usb_unlink_all_urbs(dev);
> >> +
> >> +	for (i = 0; i < dev->nchannels; i++)
> >> +		free_candev(dev->nets[i]->netdev);
> >> +}
> >> +
> >> +static struct usb_driver kvaser_usb_driver = {
> >> +	.name = "kvaser_usb",
> >> +	.probe = kvaser_usb_probe,
> >> +	.disconnect = kvaser_usb_disconnect,
> >> +	.id_table = kvaser_usb_table
> >> +};
> >> +
> >> +module_usb_driver(kvaser_usb_driver);
> >> +
> >> +MODULE_AUTHOR("Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>");
> >> +MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices");
> >> +MODULE_LICENSE("GPL v2");
> 
> Wolfgang.
> 

Sorry for the delay... I was quite busy these last days.
I did the changes and fixes you proposed and will post right now a new
version of the driver. I hope I didn't forget anything.

Thank you,

-- 
Olivier
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH net-next] tcp: introduce tcp_tw_interval to specifiy the time of TIME-WAIT
From: Cong Wang @ 2012-10-02  7:04 UTC (permalink / raw)
  To: Neil Horman
  Cc: netdev, David S. Miller, Alexey Kuznetsov, Patrick McHardy,
	Eric Dumazet
In-Reply-To: <20120928131642.GA31568@hmsreliant.think-freely.org>

On Fri, 2012-09-28 at 09:16 -0400, Neil Horman wrote:
> On Fri, Sep 28, 2012 at 02:33:07PM +0800, Cong Wang wrote:
> > On Thu, 2012-09-27 at 10:23 -0400, Neil Horman wrote:
> > > On Thu, Sep 27, 2012 at 04:41:01PM +0800, Cong Wang wrote:
> > > > Some customer requests this feature, as they stated:
> > > > 
> > > > 	"This parameter is necessary, especially for software that continually 
> > > >         creates many ephemeral processes which open sockets, to avoid socket 
> > > >         exhaustion. In many cases, the risk of the exhaustion can be reduced by 
> > > >         tuning reuse interval to allow sockets to be reusable earlier.
> > > > 
> > > >         In commercial Unix systems, this kind of parameters, such as 
> > > >         tcp_timewait in AIX and tcp_time_wait_interval in HP-UX, have 
> > > >         already been available. Their implementations allow users to tune 
> > > >         how long they keep TCP connection as TIME-WAIT state on the 
> > > >         millisecond time scale."
> > > > 
> > > > We indeed have "tcp_tw_reuse" and "tcp_tw_recycle", but these tunings
> > > > are not equivalent in that they cannot be tuned directly on the time
> > > > scale nor in a safe way, as some combinations of tunings could still
> > > > cause some problem in NAT. And, I think second scale is enough, we don't
> > > > have to make it in millisecond time scale.
> > > > 
> > > I think I have a little difficultly seeing how this does anything other than
> > > pay lip service to actually having sockets spend time in TIME_WAIT state.  That
> > > is to say, while I see users using this to just make the pain stop.  If we wait
> > > less time than it takes to be sure that a connection isn't being reused (either
> > > by waiting two segment lifetimes, or by checking timestamps), then you might as
> > > well not wait at all.  I see how its tempting to be able to say "Just don't wait
> > > as long", but it seems that theres no difference between waiting half as long as
> > > the RFC mandates, and waiting no time at all.  Neither is a good idea.
> > 
> > I don't think reducing TIME_WAIT is a good idea either, but there must
> > be some reason behind as several UNIX provides a microsecond-scale
> > tuning interface, or maybe in non-recycle mode, their RTO is much less
> > than 2*MSL?
> > 
> My guess?  Cash was the reason.  I certainly wasn't there for any of those
> developments, but a setting like this just smells to me like some customer waved
> some cash under IBM's/HP's/Sun's nose and said, "We'd like to get our tcp
> sockets back to CLOSED state faster, what can you do for us?"

Yeah, maybe. But it still doesn't make sense even if they are sure their
packets are impossible to linger in their high-speed LAN for 2*MSL?

> 
> > > 
> > > Given the problem you're trying to solve here, I'll ask the standard question in
> > > response: How does using SO_REUSEADDR not solve the problem?  Alternatively, in
> > > a pinch, why not reduce the tcp_max_tw_buckets sufficiently to start forcing
> > > TIME_WAIT sockets back into CLOSED state?
> > > 
> > > The code looks fine, but the idea really doesn't seem like a good plan to me.
> > > I'm sure HPUX/Solaris/AIX/etc have done this in response to customer demand, but
> > > that doesn't make it the right solution.
> > > 
> > 
> > *I think* the customer doesn't want to modify their applications, so
> > that is why they don't use SO_REUSERADDR.
> > 
> Well, ok, thats a legitimate distro problem.  What its not is an upstream
> problem.  Fixing the appilcation is the right thing to do, wether or not they
> want to. 
> 
> > I didn't know tcp_max_tw_buckets can do the trick, nor the customer, so
> > this is a side effect of tcp_max_tw_buckets? Is it documented?
> man 7 tcp:
> tcp_max_tw_buckets (integer; default: see below; since Linux 2.4)
> 	The maximum number of sockets in TIME_WAIT state allowed in the
> 	system.  This limit exists only  to  prevent  simple
> 	denial-of-service attacks.   The  default  value of NR_FILE*2 is
>         adjusted depending on the memory in the system.  If this number
> 	is exceeded, the socket is closed and a warning is printed.
> 

Hey, "a warning is printed" seems not very friendly. ;)

Thanks!

^ permalink raw reply

* [PATCH 12/12] usbnet: make device out of suspend before calling usbnet_read/write_cmd
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>

This patche gets the runtime PM reference count before calling
usb_control_msg, and puts it after completion of the
usb_control_msg, so that the usb control message can always be
sent to one active device.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/usbnet.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3b51554..3f4bc69 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1609,9 +1609,11 @@ int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 			goto out;
 	}
 
+	usb_autopm_get_interface(dev->intf);
 	err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
 			      cmd, reqtype, value, index, buf, size,
 			      USB_CTRL_GET_TIMEOUT);
+	usb_autopm_put_interface(dev->intf);
 	if (err > 0 && err <= size)
 		memcpy(data, buf, err);
 	kfree(buf);
@@ -1636,9 +1638,11 @@ int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 			goto out;
 	}
 
+	usb_autopm_get_interface(dev->intf);
 	err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
 			      cmd, reqtype, value, index, buf, size,
 			      USB_CTRL_SET_TIMEOUT);
+	usb_autopm_put_interface(dev->intf);
 	kfree(buf);
 
 out:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 10/12] usbnet: smsc75xx: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/smsc75xx.c |   39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index b77ae76..1baa53a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -85,26 +85,21 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
 					  u32 *data)
 {
-	u32 *buf = kmalloc(4, GFP_KERNEL);
+	u32 buf;
 	int ret;
 
 	BUG_ON(!dev);
 
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-		USB_VENDOR_REQUEST_READ_REGISTER,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+	ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+			      USB_DIR_IN | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE,
+			      0, index, &buf, 4);
 	if (unlikely(ret < 0))
 		netdev_warn(dev->net,
 			"Failed to read reg index 0x%08x: %d", index, ret);
 
-	le32_to_cpus(buf);
-	*data = *buf;
-	kfree(buf);
+	le32_to_cpus(&buf);
+	*data = buf;
 
 	return ret;
 }
@@ -112,28 +107,22 @@ static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
 static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
 					   u32 data)
 {
-	u32 *buf = kmalloc(4, GFP_KERNEL);
+	u32 buf;
 	int ret;
 
 	BUG_ON(!dev);
 
-	if (!buf)
-		return -ENOMEM;
-
-	*buf = data;
-	cpu_to_le32s(buf);
-
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		USB_VENDOR_REQUEST_WRITE_REGISTER,
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
+	buf = data;
+	cpu_to_le32s(&buf);
 
+	ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+			       USB_DIR_OUT | USB_TYPE_VENDOR |
+			       USB_RECIP_DEVICE,
+			       0, index, &buf, 4);
 	if (unlikely(ret < 0))
 		netdev_warn(dev->net,
 			"Failed to write reg index 0x%08x: %d", index, ret);
 
-	kfree(buf);
-
 	return ret;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 09/12] usbnet: sierra_net: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/sierra_net.c |   45 ++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index c27d277..eb5c7a8 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -311,10 +311,9 @@ static int sierra_net_send_cmd(struct usbnet *dev,
 	struct sierra_net_data *priv = sierra_net_get_private(dev);
 	int  status;
 
-	status = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-			USB_CDC_SEND_ENCAPSULATED_COMMAND,
-			USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,	0,
-			priv->ifnum, cmd, cmdlen, USB_CTRL_SET_TIMEOUT);
+	status = usbnet_write_cmd(dev, USB_CDC_SEND_ENCAPSULATED_COMMAND,
+				  USB_DIR_OUT|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
+				  0, priv->ifnum, cmd, cmdlen);
 
 	if (status != cmdlen && status != -ENODEV)
 		netdev_err(dev->net, "Submit %s failed %d\n", cmd_name, status);
@@ -632,32 +631,22 @@ static int sierra_net_change_mtu(struct net_device *net, int new_mtu)
 static int sierra_net_get_fw_attr(struct usbnet *dev, u16 *datap)
 {
 	int result = 0;
-	u16 *attrdata;
-
-	attrdata = kmalloc(sizeof(*attrdata), GFP_KERNEL);
-	if (!attrdata)
-		return -ENOMEM;
-
-	result = usb_control_msg(
-			dev->udev,
-			usb_rcvctrlpipe(dev->udev, 0),
-			/* _u8 vendor specific request */
-			SWI_USB_REQUEST_GET_FW_ATTR,
-			USB_DIR_IN | USB_TYPE_VENDOR,	/* __u8 request type */
-			0x0000,		/* __u16 value not used */
-			0x0000,		/* __u16 index  not used */
-			attrdata,	/* char *data */
-			sizeof(*attrdata),		/* __u16 size */
-			USB_CTRL_SET_TIMEOUT);	/* int timeout */
-
-	if (result < 0) {
-		kfree(attrdata);
+	u16 attrdata;
+
+	result = usbnet_read_cmd(dev,
+				/* _u8 vendor specific request */
+				SWI_USB_REQUEST_GET_FW_ATTR,
+				USB_DIR_IN | USB_TYPE_VENDOR,	/* __u8 request type */
+				0x0000,		/* __u16 value not used */
+				0x0000,		/* __u16 index  not used */
+				&attrdata,	/* char *data */
+				sizeof(attrdata)	/* __u16 size */
+				);
+
+	if (result < 0)
 		return -EIO;
-	}
-
-	*datap = le16_to_cpu(*attrdata);
 
-	kfree(attrdata);
+	*datap = le16_to_cpu(attrdata);
 	return result;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 06/12] usbnet: mcs7830: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/mcs7830.c |   85 ++++-----------------------------------------
 1 file changed, 6 insertions(+), 79 deletions(-)

diff --git a/drivers/net/usb/mcs7830.c b/drivers/net/usb/mcs7830.c
index 03c2d8d..db46a68 100644
--- a/drivers/net/usb/mcs7830.c
+++ b/drivers/net/usb/mcs7830.c
@@ -123,93 +123,20 @@ static const char driver_name[] = "MOSCHIP usb-ethernet driver";
 
 static int mcs7830_get_reg(struct usbnet *dev, u16 index, u16 size, void *data)
 {
-	struct usb_device *xdev = dev->udev;
-	int ret;
-	void *buffer;
-
-	buffer = kmalloc(size, GFP_NOIO);
-	if (buffer == NULL)
-		return -ENOMEM;
-
-	ret = usb_control_msg(xdev, usb_rcvctrlpipe(xdev, 0), MCS7830_RD_BREQ,
-			      MCS7830_RD_BMREQ, 0x0000, index, buffer,
-			      size, MCS7830_CTRL_TIMEOUT);
-	memcpy(data, buffer, size);
-	kfree(buffer);
-
-	return ret;
+	return usbnet_read_cmd(dev, MCS7830_RD_BREQ, MCS7830_RD_BMREQ,
+				0x0000, index, data, size);
 }
 
 static int mcs7830_set_reg(struct usbnet *dev, u16 index, u16 size, const void *data)
 {
-	struct usb_device *xdev = dev->udev;
-	int ret;
-	void *buffer;
-
-	buffer = kmemdup(data, size, GFP_NOIO);
-	if (buffer == NULL)
-		return -ENOMEM;
-
-	ret = usb_control_msg(xdev, usb_sndctrlpipe(xdev, 0), MCS7830_WR_BREQ,
-			      MCS7830_WR_BMREQ, 0x0000, index, buffer,
-			      size, MCS7830_CTRL_TIMEOUT);
-	kfree(buffer);
-	return ret;
-}
-
-static void mcs7830_async_cmd_callback(struct urb *urb)
-{
-	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-	int status = urb->status;
-
-	if (status < 0)
-		printk(KERN_DEBUG "%s() failed with %d\n",
-		       __func__, status);
-
-	kfree(req);
-	usb_free_urb(urb);
+	return usbnet_write_cmd(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+				0x0000, index, data, size);
 }
 
 static void mcs7830_set_reg_async(struct usbnet *dev, u16 index, u16 size, void *data)
 {
-	struct usb_ctrlrequest *req;
-	int ret;
-	struct urb *urb;
-
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		dev_dbg(&dev->udev->dev,
-			"Error allocating URB in write_cmd_async!\n");
-		return;
-	}
-
-	req = kmalloc(sizeof *req, GFP_ATOMIC);
-	if (!req) {
-		dev_err(&dev->udev->dev,
-			"Failed to allocate memory for control request\n");
-		goto out;
-	}
-	req->bRequestType = MCS7830_WR_BMREQ;
-	req->bRequest = MCS7830_WR_BREQ;
-	req->wValue = 0;
-	req->wIndex = cpu_to_le16(index);
-	req->wLength = cpu_to_le16(size);
-
-	usb_fill_control_urb(urb, dev->udev,
-			     usb_sndctrlpipe(dev->udev, 0),
-			     (void *)req, data, size,
-			     mcs7830_async_cmd_callback, req);
-
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret < 0) {
-		dev_err(&dev->udev->dev,
-			"Error submitting the control message: ret=%d\n", ret);
-		goto out;
-	}
-	return;
-out:
-	kfree(req);
-	usb_free_urb(urb);
+	usbnet_write_cmd_async(dev, MCS7830_WR_BREQ, MCS7830_WR_BMREQ,
+				0x0000, index, data, size);
 }
 
 static int mcs7830_hif_get_mac_address(struct usbnet *dev, unsigned char *addr)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 05/12] usbnet: int51x1: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/int51x1.c |   52 +++------------------------------------------
 1 file changed, 3 insertions(+), 49 deletions(-)

diff --git a/drivers/net/usb/int51x1.c b/drivers/net/usb/int51x1.c
index 8de6417..ace9e74 100644
--- a/drivers/net/usb/int51x1.c
+++ b/drivers/net/usb/int51x1.c
@@ -116,23 +116,8 @@ static struct sk_buff *int51x1_tx_fixup(struct usbnet *dev,
 	return skb;
 }
 
-static void int51x1_async_cmd_callback(struct urb *urb)
-{
-	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-	int status = urb->status;
-
-	if (status < 0)
-		dev_warn(&urb->dev->dev, "async callback failed with %d\n", status);
-
-	kfree(req);
-	usb_free_urb(urb);
-}
-
 static void int51x1_set_multicast(struct net_device *netdev)
 {
-	struct usb_ctrlrequest *req;
-	int status;
-	struct urb *urb;
 	struct usbnet *dev = netdev_priv(netdev);
 	u16 filter = PACKET_TYPE_DIRECTED | PACKET_TYPE_BROADCAST;
 
@@ -149,40 +134,9 @@ static void int51x1_set_multicast(struct net_device *netdev)
 		netdev_dbg(dev->net, "receive own packets only\n");
 	}
 
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		netdev_warn(dev->net, "Error allocating URB\n");
-		return;
-	}
-
-	req = kmalloc(sizeof(*req), GFP_ATOMIC);
-	if (!req) {
-		netdev_warn(dev->net, "Error allocating control msg\n");
-		goto out;
-	}
-
-	req->bRequestType = USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE;
-	req->bRequest = SET_ETHERNET_PACKET_FILTER;
-	req->wValue = cpu_to_le16(filter);
-	req->wIndex = 0;
-	req->wLength = 0;
-
-	usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		(void *)req, NULL, 0,
-		int51x1_async_cmd_callback,
-		(void *)req);
-
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status < 0) {
-		netdev_warn(dev->net, "Error submitting control msg, sts=%d\n",
-			    status);
-		goto out1;
-	}
-	return;
-out1:
-	kfree(req);
-out:
-	usb_free_urb(urb);
+	usbnet_write_cmd_async(dev, SET_ETHERNET_PACKET_FILTER,
+			       USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			       filter, 0, NULL, 0);
 }
 
 static const struct net_device_ops int51x1_netdev_ops = {
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 04/12] usbnet: dm9601: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/dm9601.c |  107 +++++++---------------------------------------
 1 file changed, 15 insertions(+), 92 deletions(-)

diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index e0433ce..3f554c1 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -56,27 +56,12 @@
 
 static int dm_read(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-	void *buf;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "dm_read() reg=0x%02x length=%d\n", reg, length);
-
-	buf = kmalloc(length, GFP_KERNEL);
-	if (!buf)
-		goto out;
-
-	err = usb_control_msg(dev->udev,
-			      usb_rcvctrlpipe(dev->udev, 0),
-			      DM_READ_REGS,
-			      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-			      0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-	if (err == length)
-		memcpy(data, buf, length);
-	else if (err >= 0)
+	int err;
+	err = usbnet_read_cmd(dev, DM_READ_REGS,
+			       USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			       0, reg, data, length);
+	if(err != length && err >= 0)
 		err = -EINVAL;
-	kfree(buf);
-
- out:
 	return err;
 }
 
@@ -87,91 +72,29 @@ static int dm_read_reg(struct usbnet *dev, u8 reg, u8 *value)
 
 static int dm_write(struct usbnet *dev, u8 reg, u16 length, void *data)
 {
-	void *buf = NULL;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "dm_write() reg=0x%02x, length=%d\n", reg, length);
+	int err;
+	err = usbnet_write_cmd(dev, DM_WRITE_REGS,
+				USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				0, reg, data, length);
 
-	if (data) {
-		buf = kmemdup(data, length, GFP_KERNEL);
-		if (!buf)
-			goto out;
-	}
-
-	err = usb_control_msg(dev->udev,
-			      usb_sndctrlpipe(dev->udev, 0),
-			      DM_WRITE_REGS,
-			      USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
-			      0, reg, buf, length, USB_CTRL_SET_TIMEOUT);
-	kfree(buf);
 	if (err >= 0 && err < length)
 		err = -EINVAL;
- out:
 	return err;
 }
 
 static int dm_write_reg(struct usbnet *dev, u8 reg, u8 value)
 {
-	netdev_dbg(dev->net, "dm_write_reg() reg=0x%02x, value=0x%02x\n",
-		   reg, value);
-	return usb_control_msg(dev->udev,
-			       usb_sndctrlpipe(dev->udev, 0),
-			       DM_WRITE_REG,
-			       USB_DIR_OUT | USB_TYPE_VENDOR |USB_RECIP_DEVICE,
-			       value, reg, NULL, 0, USB_CTRL_SET_TIMEOUT);
-}
-
-static void dm_write_async_callback(struct urb *urb)
-{
-	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-	int status = urb->status;
-
-	if (status < 0)
-		printk(KERN_DEBUG "dm_write_async_callback() failed with %d\n",
-		       status);
-
-	kfree(req);
-	usb_free_urb(urb);
+	return usbnet_write_cmd(dev, DM_WRITE_REGS,
+				USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				value, reg, NULL, 0);
 }
 
 static void dm_write_async_helper(struct usbnet *dev, u8 reg, u8 value,
 				  u16 length, void *data)
 {
-	struct usb_ctrlrequest *req;
-	struct urb *urb;
-	int status;
-
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		netdev_err(dev->net, "Error allocating URB in dm_write_async_helper!\n");
-		return;
-	}
-
-	req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-	if (!req) {
-		netdev_err(dev->net, "Failed to allocate memory for control request\n");
-		usb_free_urb(urb);
-		return;
-	}
-
-	req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	req->bRequest = length ? DM_WRITE_REGS : DM_WRITE_REG;
-	req->wValue = cpu_to_le16(value);
-	req->wIndex = cpu_to_le16(reg);
-	req->wLength = cpu_to_le16(length);
-
-	usb_fill_control_urb(urb, dev->udev,
-			     usb_sndctrlpipe(dev->udev, 0),
-			     (void *)req, data, length,
-			     dm_write_async_callback, req);
-
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status < 0) {
-		netdev_err(dev->net, "Error submitting the control message: status=%d\n",
-			   status);
-		kfree(req);
-		usb_free_urb(urb);
-	}
+	usbnet_write_cmd_async(dev, DM_WRITE_REGS,
+			       USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			       value, reg, data, length);
 }
 
 static void dm_write_async(struct usbnet *dev, u8 reg, u16 length, void *data)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 03/12] usbnet: cdc-ncm: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei@canonical.com>


Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/cdc_ncm.c |   73 ++++++++++++++-------------------------------
 1 file changed, 23 insertions(+), 50 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..429a2ad 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -159,16 +159,16 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	u8 iface_no;
 	int err;
 	u16 ntb_fmt_supported;
+	struct usbnet *dev = netdev_priv(ctx->netdev);
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
-	err = usb_control_msg(ctx->udev,
-				usb_rcvctrlpipe(ctx->udev, 0),
+	err = usbnet_read_cmd(dev,
 				USB_CDC_GET_NTB_PARAMETERS,
 				USB_TYPE_CLASS | USB_DIR_IN
 				 | USB_RECIP_INTERFACE,
 				0, iface_no, &ctx->ncm_parm,
-				sizeof(ctx->ncm_parm), 10000);
+				sizeof(ctx->ncm_parm));
 	if (err < 0) {
 		pr_debug("failed GET_NTB_PARAMETERS\n");
 		return 1;
@@ -217,40 +217,23 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
 
 		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
-			struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
+			struct usb_cdc_ncm_ndp_input_size ndp_in_sz;
 
-			ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
-			if (!ndp_in_sz) {
-				err = -ENOMEM;
-				goto size_err;
-			}
-
-			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
+			memset(&ndp_in_sz, 0, sizeof(ndp_in_sz));
+			err = usbnet_write_cmd(dev,
 					USB_CDC_SET_NTB_INPUT_SIZE,
 					USB_TYPE_CLASS | USB_DIR_OUT
 					 | USB_RECIP_INTERFACE,
-					0, iface_no, ndp_in_sz, 8, 1000);
-			kfree(ndp_in_sz);
+					0, iface_no, &ndp_in_sz, 8);
 		} else {
-			__le32 *dwNtbInMaxSize;
-			dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize),
-					GFP_KERNEL);
-			if (!dwNtbInMaxSize) {
-				err = -ENOMEM;
-				goto size_err;
-			}
-			*dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
+			__le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
 
-			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
+			err = usbnet_write_cmd(dev,
 					USB_CDC_SET_NTB_INPUT_SIZE,
 					USB_TYPE_CLASS | USB_DIR_OUT
 					 | USB_RECIP_INTERFACE,
-					0, iface_no, dwNtbInMaxSize, 4, 1000);
-			kfree(dwNtbInMaxSize);
+					0, iface_no, &dwNtbInMaxSize, 4);
 		}
-size_err:
 		if (err < 0)
 			pr_debug("Setting NTB Input Size failed\n");
 	}
@@ -306,23 +289,23 @@ size_err:
 
 	/* set CRC Mode */
 	if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
-		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+		err = usbnet_write_cmd(dev,
 				USB_CDC_SET_CRC_MODE,
 				USB_TYPE_CLASS | USB_DIR_OUT
 				 | USB_RECIP_INTERFACE,
 				USB_CDC_NCM_CRC_NOT_APPENDED,
-				iface_no, NULL, 0, 1000);
+				iface_no, NULL, 0);
 		if (err < 0)
 			pr_debug("Setting CRC mode off failed\n");
 	}
 
 	/* set NTB format, if both formats are supported */
 	if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
-		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+		err = usbnet_write_cmd(dev,
 				USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
 				 | USB_DIR_OUT | USB_RECIP_INTERFACE,
 				USB_CDC_NCM_NTB16_FORMAT,
-				iface_no, NULL, 0, 1000);
+				iface_no, NULL, 0);
 		if (err < 0)
 			pr_debug("Setting NTB format to 16-bit failed\n");
 	}
@@ -331,28 +314,21 @@ size_err:
 
 	/* set Max Datagram Size (MTU) */
 	if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
-		__le16 *max_datagram_size;
+		__le16 max_datagram_size;
 		u16 eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
 
-		max_datagram_size = kzalloc(sizeof(*max_datagram_size),
-				GFP_KERNEL);
-		if (!max_datagram_size) {
-			err = -ENOMEM;
-			goto max_dgram_err;
-		}
-
-		err = usb_control_msg(ctx->udev, usb_rcvctrlpipe(ctx->udev, 0),
+		err = usbnet_write_cmd(dev,
 				USB_CDC_GET_MAX_DATAGRAM_SIZE,
 				USB_TYPE_CLASS | USB_DIR_IN
 				 | USB_RECIP_INTERFACE,
-				0, iface_no, max_datagram_size,
-				2, 1000);
+				0, iface_no, &max_datagram_size,
+				2);
 		if (err < 0) {
 			pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
 						CDC_NCM_MIN_DATAGRAM_SIZE);
 		} else {
 			ctx->max_datagram_size =
-				le16_to_cpu(*max_datagram_size);
+				le16_to_cpu(max_datagram_size);
 			/* Check Eth descriptor value */
 			if (ctx->max_datagram_size > eth_max_sz)
 					ctx->max_datagram_size = eth_max_sz;
@@ -367,23 +343,20 @@ size_err:
 
 			/* if value changed, update device */
 			if (ctx->max_datagram_size !=
-					le16_to_cpu(*max_datagram_size)) {
-				err = usb_control_msg(ctx->udev,
-						usb_sndctrlpipe(ctx->udev, 0),
+					le16_to_cpu(max_datagram_size)) {
+				err = usbnet_write_cmd(dev,
 						USB_CDC_SET_MAX_DATAGRAM_SIZE,
 						USB_TYPE_CLASS | USB_DIR_OUT
 						 | USB_RECIP_INTERFACE,
 						0,
-						iface_no, max_datagram_size,
-						2, 1000);
+						iface_no, &max_datagram_size,
+						2);
 				if (err < 0)
 					pr_debug("SET_MAX_DGRAM_SIZE failed\n");
 			}
 		}
-		kfree(max_datagram_size);
 	}
 
-max_dgram_err:
 	if (ctx->netdev->mtu != (ctx->max_datagram_size - ETH_HLEN))
 		ctx->netdev->mtu = ctx->max_datagram_size - ETH_HLEN;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 00/12] usbnet: usb_control_msg cleanup
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman; +Cc: Oliver Neukum, netdev, linux-usb

Hi,

This patch set introduces 3 helpers for handling usb read, write
and write_async command, and replaces the low level's implemention
with the generic ones.

Firstly, it is a cleanup and about 300 lines code can be saved.

Secondly, the patch fixes DMA on the buffer which is embedded inside
one dynamic allocated buffer, and such usages can be found
in cdc-ncm, sierra_net, mcs7830 and asix drivers.

Finally, almost all low level drivers don't consider runtime
PM situation when reading/writing control command to device via
usb_control_message. For example, 'ethtool' may touch a suspended
device and kind of below message will be dumped:

	[tom@tom-pandaboard ~]$ ethtool eth0
	Settings for eth[  117.084411] smsc95xx 1-1.1:1.0 eth0: Failed to read register index 0x00000114
	0:
	[  117.093139] smsc95xx 1-1.1:1.0 eth0: Error reading MII_ACCESS
	[  117.099395] smsc95xx 1-1.1:1.0 eth0: MII is busy in smsc95xx_mdio_read

This patch fixes the problem above by holding runtime PM referece
count before calling usb_control_msg.

 drivers/net/usb/asix_common.c |  117 ++++-------------------------------
 drivers/net/usb/cdc_ncm.c     |   73 +++++++---------------
 drivers/net/usb/dm9601.c      |  107 +++++---------------------------
 drivers/net/usb/int51x1.c     |   52 +---------------
 drivers/net/usb/mcs7830.c     |   85 ++-----------------------
 drivers/net/usb/net1080.c     |  110 +++++++++------------------------
 drivers/net/usb/plusb.c       |   11 ++--
 drivers/net/usb/sierra_net.c  |   45 +++++---------
 drivers/net/usb/smsc75xx.c    |   39 +++++-------
 drivers/net/usb/smsc95xx.c    |  115 ++++++++--------------------------
 drivers/net/usb/usbnet.c      |  137 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/usbnet.h    |    6 ++
 12 files changed, 295 insertions(+), 602 deletions(-)


Thanks,
--
Ming Lei

^ permalink raw reply

* [PATCH 11/12] usbnet: smsc95xx: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/smsc95xx.c |  115 +++++++++++---------------------------------
 1 file changed, 27 insertions(+), 88 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 7479a57..1730f75 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -65,11 +65,6 @@ struct smsc95xx_priv {
 	spinlock_t mac_cr_lock;
 };
 
-struct usb_context {
-	struct usb_ctrlrequest req;
-	struct usbnet *dev;
-};
-
 static bool turbo_mode = true;
 module_param(turbo_mode, bool, 0644);
 MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
@@ -77,25 +72,20 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
 					  u32 *data)
 {
-	u32 *buf = kmalloc(4, GFP_KERNEL);
+	u32 buf;
 	int ret;
 
 	BUG_ON(!dev);
 
-	if (!buf)
-		return -ENOMEM;
-
-	ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-		USB_VENDOR_REQUEST_READ_REGISTER,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, buf, 4, USB_CTRL_GET_TIMEOUT);
-
+	ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
+			      USB_DIR_IN | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE,
+			      0, index, &buf, 4);
 	if (unlikely(ret < 0))
 		netdev_warn(dev->net, "Failed to read register index 0x%08x\n", index);
 
-	le32_to_cpus(buf);
-	*data = *buf;
-	kfree(buf);
+	le32_to_cpus(&buf);
+	*data = buf;
 
 	return ret;
 }
@@ -103,27 +93,22 @@ static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
 static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
 					   u32 data)
 {
-	u32 *buf = kmalloc(4, GFP_KERNEL);
+	u32 buf;
 	int ret;
 
 	BUG_ON(!dev);
 
-	if (!buf)
-		return -ENOMEM;
-
-	*buf = data;
-	cpu_to_le32s(buf);
+	buf = data;
+	cpu_to_le32s(&buf);
 
-	ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		USB_VENDOR_REQUEST_WRITE_REGISTER,
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, buf, 4, USB_CTRL_SET_TIMEOUT);
 
+	ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
+			       USB_DIR_OUT | USB_TYPE_VENDOR |
+			       USB_RECIP_DEVICE,
+			       0, index, &buf, 4);
 	if (unlikely(ret < 0))
 		netdev_warn(dev->net, "Failed to write register index 0x%08x\n", index);
 
-	kfree(buf);
-
 	return ret;
 }
 
@@ -132,11 +117,8 @@ static int smsc95xx_set_feature(struct usbnet *dev, u32 feature)
 	if (WARN_ON_ONCE(!dev))
 		return -EINVAL;
 
-	cpu_to_le32s(&feature);
-
-	return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-		USB_CTRL_SET_TIMEOUT);
+	return usbnet_write_cmd(dev, USB_REQ_SET_FEATURE,
+				USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -144,11 +126,8 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
 	if (WARN_ON_ONCE(!dev))
 		return -EINVAL;
 
-	cpu_to_le32s(&feature);
-
-	return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
-		USB_CTRL_SET_TIMEOUT);
+	return usbnet_write_cmd(dev, USB_REQ_CLEAR_FEATURE,
+				USB_RECIP_DEVICE, feature, 0, NULL, 0);
 }
 
 /* Loop until the read is completed with timeout
@@ -350,60 +329,20 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 	return 0;
 }
 
-static void smsc95xx_async_cmd_callback(struct urb *urb)
-{
-	struct usb_context *usb_context = urb->context;
-	struct usbnet *dev = usb_context->dev;
-	int status = urb->status;
-
-	check_warn(status, "async callback failed with %d\n", status);
-
-	kfree(usb_context);
-	usb_free_urb(urb);
-}
-
 static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
 						 u32 *data)
 {
-	struct usb_context *usb_context;
-	int status;
-	struct urb *urb;
 	const u16 size = 4;
+	int ret;
 
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		netdev_warn(dev->net, "Error allocating URB\n");
-		return -ENOMEM;
-	}
-
-	usb_context = kmalloc(sizeof(struct usb_context), GFP_ATOMIC);
-	if (usb_context == NULL) {
-		netdev_warn(dev->net, "Error allocating control msg\n");
-		usb_free_urb(urb);
-		return -ENOMEM;
-	}
-
-	usb_context->req.bRequestType =
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	usb_context->req.bRequest = USB_VENDOR_REQUEST_WRITE_REGISTER;
-	usb_context->req.wValue = 00;
-	usb_context->req.wIndex = cpu_to_le16(index);
-	usb_context->req.wLength = cpu_to_le16(size);
-
-	usb_fill_control_urb(urb, dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		(void *)&usb_context->req, data, size,
-		smsc95xx_async_cmd_callback,
-		(void *)usb_context);
-
-	status = usb_submit_urb(urb, GFP_ATOMIC);
-	if (status < 0) {
-		netdev_warn(dev->net, "Error submitting control msg, sts=%d\n",
-			    status);
-		kfree(usb_context);
-		usb_free_urb(urb);
-	}

^ permalink raw reply related

* [PATCH 08/12] usbnet: plusb: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/plusb.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/plusb.c b/drivers/net/usb/plusb.c
index 4584b9a..0fcc8e6 100644
--- a/drivers/net/usb/plusb.c
+++ b/drivers/net/usb/plusb.c
@@ -71,13 +71,10 @@
 static inline int
 pl_vendor_req(struct usbnet *dev, u8 req, u8 val, u8 index)
 {
-	return usb_control_msg(dev->udev,
-		usb_rcvctrlpipe(dev->udev, 0),
-		req,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		val, index,
-		NULL, 0,
-		USB_CTRL_GET_TIMEOUT);
+	return usbnet_read_cmd(dev, req,
+				USB_DIR_IN | USB_TYPE_VENDOR |
+				USB_RECIP_DEVICE,
+				val, index, NULL, 0);
 }
 
 static inline int
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 07/12] usbnet: net1080: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/net1080.c |  110 +++++++++++++--------------------------------
 1 file changed, 30 insertions(+), 80 deletions(-)

diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index c062a3e..93e0716 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c
@@ -109,13 +109,11 @@ struct nc_trailer {
 static int
 nc_vendor_read(struct usbnet *dev, u8 req, u8 regnum, u16 *retval_ptr)
 {
-	int status = usb_control_msg(dev->udev,
-		usb_rcvctrlpipe(dev->udev, 0),
-		req,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		0, regnum,
-		retval_ptr, sizeof *retval_ptr,
-		USB_CTRL_GET_TIMEOUT);
+	int status = usbnet_read_cmd(dev, req,
+				     USB_DIR_IN | USB_TYPE_VENDOR |
+				     USB_RECIP_DEVICE,
+				     0, regnum, retval_ptr,
+				     sizeof *retval_ptr);
 	if (status > 0)
 		status = 0;
 	if (!status)
@@ -133,13 +131,9 @@ nc_register_read(struct usbnet *dev, u8 regnum, u16 *retval_ptr)
 static void
 nc_vendor_write(struct usbnet *dev, u8 req, u8 regnum, u16 value)
 {
-	usb_control_msg(dev->udev,
-		usb_sndctrlpipe(dev->udev, 0),
-		req,
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		value, regnum,
-		NULL, 0,			// data is in setup packet
-		USB_CTRL_SET_TIMEOUT);
+	usbnet_write_cmd(dev, req,
+			 USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			 value, regnum, NULL, 0);
 }
 
 static inline void
@@ -288,37 +282,34 @@ static inline void nc_dump_ttl(struct usbnet *dev, u16 ttl)
 static int net1080_reset(struct usbnet *dev)
 {
 	u16		usbctl, status, ttl;
-	u16		*vp = kmalloc(sizeof (u16), GFP_KERNEL);
+	u16		vp;
 	int		retval;
 
-	if (!vp)
-		return -ENOMEM;
-
 	// nc_dump_registers(dev);
 
-	if ((retval = nc_register_read(dev, REG_STATUS, vp)) < 0) {
+	if ((retval = nc_register_read(dev, REG_STATUS, &vp)) < 0) {
 		netdev_dbg(dev->net, "can't read %s-%s status: %d\n",
 			   dev->udev->bus->bus_name, dev->udev->devpath, retval);
 		goto done;
 	}
-	status = *vp;
+	status = vp;
 	nc_dump_status(dev, status);
 
-	if ((retval = nc_register_read(dev, REG_USBCTL, vp)) < 0) {
+	if ((retval = nc_register_read(dev, REG_USBCTL, &vp)) < 0) {
 		netdev_dbg(dev->net, "can't read USBCTL, %d\n", retval);
 		goto done;
 	}
-	usbctl = *vp;
+	usbctl = vp;
 	nc_dump_usbctl(dev, usbctl);
 
 	nc_register_write(dev, REG_USBCTL,
 			USBCTL_FLUSH_THIS | USBCTL_FLUSH_OTHER);
 
-	if ((retval = nc_register_read(dev, REG_TTL, vp)) < 0) {
+	if ((retval = nc_register_read(dev, REG_TTL, &vp)) < 0) {
 		netdev_dbg(dev->net, "can't read TTL, %d\n", retval);
 		goto done;
 	}
-	ttl = *vp;
+	ttl = vp;
 	// nc_dump_ttl(dev, ttl);
 
 	nc_register_write(dev, REG_TTL,
@@ -331,7 +322,6 @@ static int net1080_reset(struct usbnet *dev)
 	retval = 0;
 
 done:
-	kfree(vp);
 	return retval;
 }
 
@@ -339,13 +329,10 @@ static int net1080_check_connect(struct usbnet *dev)
 {
 	int			retval;
 	u16			status;
-	u16			*vp = kmalloc(sizeof (u16), GFP_KERNEL);
+	u16			vp;
 
-	if (!vp)
-		return -ENOMEM;
-	retval = nc_register_read(dev, REG_STATUS, vp);
-	status = *vp;
-	kfree(vp);
+	retval = nc_register_read(dev, REG_STATUS, &vp);
+	status = vp;
 	if (retval != 0) {
 		netdev_dbg(dev->net, "net1080_check_conn read - %d\n", retval);
 		return retval;
@@ -355,59 +342,22 @@ static int net1080_check_connect(struct usbnet *dev)
 	return 0;
 }
 
-static void nc_flush_complete(struct urb *urb)
-{
-	kfree(urb->context);
-	usb_free_urb(urb);
-}
-
 static void nc_ensure_sync(struct usbnet *dev)
 {
-	dev->frame_errors++;
-	if (dev->frame_errors > 5) {
-		struct urb		*urb;
-		struct usb_ctrlrequest	*req;
-		int			status;
-
-		/* Send a flush */
-		urb = usb_alloc_urb(0, GFP_ATOMIC);
-		if (!urb)
-			return;
-
-		req = kmalloc(sizeof *req, GFP_ATOMIC);
-		if (!req) {
-			usb_free_urb(urb);
-			return;
-		}
+	if (++dev->frame_errors <= 5)
+		return;
 
-		req->bRequestType = USB_DIR_OUT
-			| USB_TYPE_VENDOR
-			| USB_RECIP_DEVICE;
-		req->bRequest = REQUEST_REGISTER;
-		req->wValue = cpu_to_le16(USBCTL_FLUSH_THIS
-				| USBCTL_FLUSH_OTHER);
-		req->wIndex = cpu_to_le16(REG_USBCTL);
-		req->wLength = cpu_to_le16(0);
-
-		/* queue an async control request, we don't need
-		 * to do anything when it finishes except clean up.
-		 */
-		usb_fill_control_urb(urb, dev->udev,
-			usb_sndctrlpipe(dev->udev, 0),
-			(unsigned char *) req,
-			NULL, 0,
-			nc_flush_complete, req);
-		status = usb_submit_urb(urb, GFP_ATOMIC);
-		if (status) {
-			kfree(req);
-			usb_free_urb(urb);
-			return;
-		}
+	if (usbnet_write_cmd_async(dev, REQUEST_REGISTER,
+					USB_DIR_OUT | USB_TYPE_VENDOR |
+					USB_RECIP_DEVICE,
+					USBCTL_FLUSH_THIS |
+					USBCTL_FLUSH_OTHER,
+					REG_USBCTL, NULL, 0))
+		return;
 
-		netif_dbg(dev, rx_err, dev->net,
-			  "flush net1080; too many framing errors\n");
-		dev->frame_errors = 0;
-	}
+	netif_dbg(dev, rx_err, dev->net,
+		  "flush net1080; too many framing errors\n");
+	dev->frame_errors = 0;
 }
 
 static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 02/12] usbnet: asix: apply introduced usb command APIs
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/asix_common.c |  117 +++++------------------------------------
 1 file changed, 13 insertions(+), 104 deletions(-)

diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 774d9ce..50d1673 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -25,121 +25,30 @@
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 		  u16 size, void *data)
 {
-	void *buf;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "asix_read_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	buf = kmalloc(size, GFP_KERNEL);
-	if (!buf)
-		goto out;
-
-	err = usb_control_msg(
-		dev->udev,
-		usb_rcvctrlpipe(dev->udev, 0),
-		cmd,
-		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		value,
-		index,
-		buf,
-		size,
-		USB_CTRL_GET_TIMEOUT);
-	if (err == size)
-		memcpy(data, buf, size);
-	else if (err >= 0)
-		err = -EINVAL;
-	kfree(buf);
+	int ret;
+	ret = usbnet_read_cmd(dev, cmd,
+			       USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			       value, index, data, size);
 
-out:
-	return err;
+	if (ret != size && ret >= 0)
+		return -EINVAL;
+	return ret;
 }
 
 int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 		   u16 size, void *data)
 {
-	void *buf = NULL;
-	int err = -ENOMEM;
-
-	netdev_dbg(dev->net, "asix_write_cmd() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	if (data) {
-		buf = kmemdup(data, size, GFP_KERNEL);
-		if (!buf)
-			goto out;
-	}
-
-	err = usb_control_msg(
-		dev->udev,
-		usb_sndctrlpipe(dev->udev, 0),
-		cmd,
-		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		value,
-		index,
-		buf,
-		size,
-		USB_CTRL_SET_TIMEOUT);
-	kfree(buf);
-
-out:
-	return err;
-}
-
-static void asix_async_cmd_callback(struct urb *urb)
-{
-	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
-	int status = urb->status;
-
-	if (status < 0)
-		printk(KERN_DEBUG "asix_async_cmd_callback() failed with %d",
-			status);
-
-	kfree(req);
-	usb_free_urb(urb);
+	return usbnet_write_cmd(dev, cmd,
+				USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+				value, index, data, size);
 }
 
 void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 			  u16 size, void *data)
 {
-	struct usb_ctrlrequest *req;
-	int status;
-	struct urb *urb;
-
-	netdev_dbg(dev->net, "asix_write_cmd_async() cmd=0x%02x value=0x%04x index=0x%04x size=%d\n",
-		   cmd, value, index, size);
-
-	urb = usb_alloc_urb(0, GFP_ATOMIC);
-	if (!urb) {
-		netdev_err(dev->net, "Error allocating URB in write_cmd_async!\n");
-		return;
-	}
-
-	req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
-	if (!req) {
-		netdev_err(dev->net, "Failed to allocate memory for control request\n");
-		usb_free_urb(urb);
-		return;
-	}
-
-	req->bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	req->bRequest = cmd;
-	req->wValue = cpu_to_le16(value);
-	req->wIndex = cpu_to_le16(index);
-	req->wLength = cpu_to_le16(size);
-
-	usb_fill_control_urb(urb, dev->udev,
-			     usb_sndctrlpipe(dev->udev, 0),
-			     (void *)req, data, size,
-			     asix_async_cmd_callback, req);

^ permalink raw reply related

* [PATCH 01/12] usbnet: introduce usbnet 3 command helpers
From: Ming Lei @ 2012-10-02  6:51 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1349160684-6627-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

This patch introduces the below 3 usb command helpers:

	usbnet_read_cmd / usbnet_write_cmd / usbnet_write_cmd_async

so that each low level driver doesn't need to implement them
by itself, and the dma buffer allocation for usb transfer and
runtime PM things can be handled just in one place.

Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/usbnet.c   |  133 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/usbnet.h |    6 ++
 2 files changed, 139 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index fc9f578..3b51554 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1592,6 +1592,139 @@ int usbnet_resume (struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usbnet_resume);
 
+/*-------------------------------------------------------------------------*/
+int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+		    u16 value, u16 index, void *data, u16 size)
+{
+	void *buf = NULL;
+	int err = -ENOMEM;
+
+	netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
+		   " value=0x%04x index=0x%04x size=%d\n",
+		   cmd, reqtype, value, index, size);
+
+	if (data) {
+		buf = kmalloc(size, GFP_KERNEL);
+		if (!buf)
+			goto out;
+	}
+
+	err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+			      cmd, reqtype, value, index, buf, size,
+			      USB_CTRL_GET_TIMEOUT);
+	if (err > 0 && err <= size)
+		memcpy(data, buf, err);
+	kfree(buf);
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd);
+
+int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+		     u16 value, u16 index, const void *data, u16 size)
+{
+	void *buf = NULL;
+	int err = -ENOMEM;
+
+	netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+		   " value=0x%04x index=0x%04x size=%d\n",
+		   cmd, reqtype, value, index, size);
+
+	if (data) {
+		buf = kmemdup(data, size, GFP_KERNEL);
+		if (!buf)
+			goto out;
+	}
+
+	err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+			      cmd, reqtype, value, index, buf, size,
+			      USB_CTRL_SET_TIMEOUT);
+	kfree(buf);
+
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd);
+
+static void usbnet_async_cmd_cb(struct urb *urb)
+{
+	struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
+	int status = urb->status;
+
+	if (status < 0)
+		dev_dbg(&urb->dev->dev, "%s failed with %d",
+			__func__, status);
+
+	kfree(req);
+	usb_free_urb(urb);
+}
+
+int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+			   u16 value, u16 index, const void *data, u16 size)
+{
+	struct usb_ctrlrequest *req = NULL;
+	struct urb *urb;
+	int err = -ENOMEM;
+	void *buf = NULL;
+
+	netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
+		   " value=0x%04x index=0x%04x size=%d\n",
+		   cmd, reqtype, value, index, size);
+
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		netdev_err(dev->net, "Error allocating URB in"
+			   " %s!\n", __func__);
+		goto fail;
+	}
+
+	if (data) {
+		buf = kmemdup(data, size, GFP_ATOMIC);
+		if (!buf) {
+			netdev_err(dev->net, "Error allocating buffer"
+				   " in %s!\n", __func__);
+			goto fail_free;
+		}
+	}
+
+	req = kmalloc(sizeof(struct usb_ctrlrequest), GFP_ATOMIC);
+	if (!req) {
+		netdev_err(dev->net, "Failed to allocate memory for %s\n",
+			   __func__);
+		goto fail_free_buf;
+	}
+
+	req->bRequestType = reqtype;
+	req->bRequest = cmd;
+	req->wValue = cpu_to_le16(value);
+	req->wIndex = cpu_to_le16(index);
+	req->wLength = cpu_to_le16(size);
+
+	usb_fill_control_urb(urb, dev->udev,
+			     usb_sndctrlpipe(dev->udev, 0),
+			     (void *)req, buf, size,
+			     usbnet_async_cmd_cb, req);
+	urb->transfer_flags |= URB_FREE_BUFFER;
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err < 0) {
+		netdev_err(dev->net, "Error submitting the control"
+			   " message: status=%d\n", err);
+		goto fail_free;
+	}
+	return 0;
+
+fail_free_buf:
+	kfree(buf);
+fail_free:
+	kfree(req);
+	usb_free_urb(urb);
+fail:
+	return err;
+
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd_async);
+
 
 /*-------------------------------------------------------------------------*/
 
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f87cf62..32a57dd 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -161,6 +161,12 @@ extern int usbnet_suspend(struct usb_interface *, pm_message_t);
 extern int usbnet_resume(struct usb_interface *);
 extern void usbnet_disconnect(struct usb_interface *);
 
+extern int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+		    u16 value, u16 index, void *data, u16 size);
+extern int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+		    u16 value, u16 index, const void *data, u16 size);
+extern int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
+		    u16 value, u16 index, const void *data, u16 size);
 
 /* Drivers that reuse some of the standard USB CDC infrastructure
  * (notably, using multiple interfaces according to the CDC
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox