Patchwork Active URB submitted twice in pegasus driver

login
register
mail settings
Submitter Petko Manolov
Date April 5, 2013, 5:01 p.m.
Message ID <alpine.DEB.2.02.1304051946020.7054@fry>
Download mbox | patch
Permalink /patch/234202/
State RFC
Delegated to: David Miller
Headers show

Comments

Petko Manolov - April 5, 2013, 5:01 p.m.
On Wed, 27 Mar 2013, Petko Manolov wrote:

> On Tue, 26 Mar 2013, Sarah Sharp wrote:
>
>> ctrl_callback is still reading the URB status, and using it in the
>> switch statement.  It's also using the urb->context as well, to dig out
>> a pointer (pegasus_t) that the pegasus_set_multicast already has access
>> to.  What happens if an URB to get the registers completes at the same
>> time pegasus_set_multicast calls ctrl_callback?  If the URB failed for
>> some reason (e.g. bad cable, stall), you'll miss that if statement.  I
>> don't think that's what you intended to do.
>> 
>> I think the fix should be to just to move the if block into a new
>> function, and call it both in ctrl_callback() and
>> pegasus_set_multicast().
>
> The _real_ fix would be to rework the whole callback mechanism, but this is a 
> different story.  It is in my todo list.

Attached you'll find two patches against Linus' 3.9.0-rc5.  These should 
effectively cure the broken control requests callback logic.  The patch is 
so big because i also cleaned up a lot of old cruft.

The new driver actually compiles and runs fine with Pegasus-II based 
device.  Please let me know if you run into problems.


cheers,
Petko
Sarah Sharp - April 8, 2013, 3:49 p.m.
On Fri, Apr 05, 2013 at 08:01:03PM +0300, Petko Manolov wrote:
> On Wed, 27 Mar 2013, Petko Manolov wrote:
> >On Tue, 26 Mar 2013, Sarah Sharp wrote:
> The new driver actually compiles and runs fine with Pegasus-II based
> device.  Please let me know if you run into problems.

I'm away for a week, so I won't be able to test for a while.  I'll let
you know the results when I get back.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c

index 73051d1..64b7a04 100644

--- a/drivers/net/usb/pegasus.c

+++ b/drivers/net/usb/pegasus.c

@@ -1,5 +1,5 @@ 

 /*
- *  Copyright (c) 1999-2005 Petko Manolov (petkan@users.sourceforge.net)

+ *  Copyright (c) 1999-2013 Petko Manolov (petkan@nucleusys.com)

  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -26,6 +26,9 @@ 

  *		v0.5.1	ethtool support added
  *		v0.5.5	rx socket buffers are in a pool and the their allocation
  *			is out of the interrupt routine.
+ *		...

+ *		v0.9.1	simplified [get|set]_register(s), async update registers

+ *			logic revisited, receive skb_pool removed.

  */
 
 #include <linux/sched.h>
@@ -45,8 +48,8 @@ 

 /*
  * Version Information
  */
-#define DRIVER_VERSION "v0.6.14 (2006/09/27)"

-#define DRIVER_AUTHOR "Petko Manolov <petkan@users.sourceforge.net>"

+#define DRIVER_VERSION "v0.9.1 (2013/04/04)"

+#define DRIVER_AUTHOR "Petko Manolov <petkan@nucleusys.com>"

 #define DRIVER_DESC "Pegasus/Pegasus II USB Ethernet driver"
 
 static const char driver_name[] = "pegasus";
@@ -108,88 +111,43 @@  MODULE_PARM_DESC(msg_level, "Override default message level");

 MODULE_DEVICE_TABLE(usb, pegasus_ids);
 static const struct net_device_ops pegasus_netdev_ops;
 
-static int update_eth_regs_async(pegasus_t *);

-/* Aargh!!! I _really_ hate such tweaks */

-static void ctrl_callback(struct urb *urb)

+

+static void async_ctrl_callback(struct urb *urb)

 {
 	pegasus_t *pegasus = urb->context;
 	int status = urb->status;
 
 	if (!pegasus)
-		return;

+		goto out;

 
 	switch (status) {
 	case 0:
-		if (pegasus->flags & ETH_REGS_CHANGE) {

-			pegasus->flags &= ~ETH_REGS_CHANGE;

-			pegasus->flags |= ETH_REGS_CHANGED;

-			update_eth_regs_async(pegasus);

-			return;

-		}

 		break;
 	case -EINPROGRESS:
-		return;

+		goto out;

 	case -ENOENT:
 		break;
 	default:
 		if (net_ratelimit())
 			netif_dbg(pegasus, drv, pegasus->net,
-				  "%s, status %d\n", __func__, status);

+			          "%s, status %d\n", __func__, status);

 		break;
 	}
-	pegasus->flags &= ~ETH_REGS_CHANGED;

-	wake_up(&pegasus->ctrl_wait);

+out:

+	usb_free_urb(urb);

 }
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 void *data)
 {
 	int ret;
-	char *buffer;

-	DECLARE_WAITQUEUE(wait, current);

-

-	buffer = kmalloc(size, GFP_KERNEL);

-	if (!buffer)

-		return -ENOMEM;

-

-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-	while (pegasus->flags & ETH_REGS_CHANGED)

-		schedule();

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_RUNNING);

-

-	pegasus->dr.bRequestType = PEGASUS_REQT_READ;

-	pegasus->dr.bRequest = PEGASUS_REQ_GET_REGS;

-	pegasus->dr.wValue = cpu_to_le16(0);

-	pegasus->dr.wIndex = cpu_to_le16(indx);

-	pegasus->dr.wLength = cpu_to_le16(size);

-	pegasus->ctrl_urb->transfer_buffer_length = size;

-

-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,

-			     usb_rcvctrlpipe(pegasus->usb, 0),

-			     (char *) &pegasus->dr,

-			     buffer, size, ctrl_callback, pegasus);

 
-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-

-	/* using ATOMIC, we'd never wake up if we slept */

-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {

-		set_current_state(TASK_RUNNING);

-		if (ret == -ENODEV)

-			netif_device_detach(pegasus->net);

-		if (net_ratelimit())

-			netif_err(pegasus, drv, pegasus->net,

-				  "%s, status %d\n", __func__, ret);

-		goto out;

-	}

-

-	schedule();

-out:

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	memcpy(data, buffer, size);

-	kfree(buffer);

+	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),

+	                      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,

+	                      indx, data, size, 1000);

+	if (ret < 0)

+		netif_dbg(pegasus, drv, pegasus->net,

+		          "%s returned %d\n", __func__, ret);

 
 	return ret;
 }
@@ -198,126 +156,55 @@  static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,

 			 void *data)
 {
 	int ret;
-	char *buffer;

-	DECLARE_WAITQUEUE(wait, current);

-

-	buffer = kmemdup(data, size, GFP_KERNEL);

-	if (!buffer) {

-		netif_warn(pegasus, drv, pegasus->net,

-			   "out of memory in %s\n", __func__);

-		return -ENOMEM;

-	}

-

-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-	while (pegasus->flags & ETH_REGS_CHANGED)

-		schedule();

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_RUNNING);

-

-	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;

-	pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS;

-	pegasus->dr.wValue = cpu_to_le16(0);

-	pegasus->dr.wIndex = cpu_to_le16(indx);

-	pegasus->dr.wLength = cpu_to_le16(size);

-	pegasus->ctrl_urb->transfer_buffer_length = size;

-

-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,

-			     usb_sndctrlpipe(pegasus->usb, 0),

-			     (char *) &pegasus->dr,

-			     buffer, size, ctrl_callback, pegasus);

-

-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-

-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {

-		if (ret == -ENODEV)

-			netif_device_detach(pegasus->net);

-		netif_err(pegasus, drv, pegasus->net,

-			  "%s, status %d\n", __func__, ret);

-		goto out;

-	}

-

-	schedule();

-out:

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	kfree(buffer);

 
+	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),

+	                      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,

+	                      indx, data, size, 100);

+	if (ret < 0)

+		netif_dbg(pegasus, drv, pegasus->net,

+		          "%s returned %d\n", __func__, ret);

 	return ret;
 }
 
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
 	int ret;
-	char *tmp;

-	DECLARE_WAITQUEUE(wait, current);

-

-	tmp = kmemdup(&data, 1, GFP_KERNEL);

-	if (!tmp) {

-		netif_warn(pegasus, drv, pegasus->net,

-			   "out of memory in %s\n", __func__);

-		return -ENOMEM;

-	}

-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-	while (pegasus->flags & ETH_REGS_CHANGED)

-		schedule();

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_RUNNING);

 
-	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;

-	pegasus->dr.bRequest = PEGASUS_REQ_SET_REG;

-	pegasus->dr.wValue = cpu_to_le16(data);

-	pegasus->dr.wIndex = cpu_to_le16(indx);

-	pegasus->dr.wLength = cpu_to_le16(1);

-	pegasus->ctrl_urb->transfer_buffer_length = 1;

-

-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,

-			     usb_sndctrlpipe(pegasus->usb, 0),

-			     (char *) &pegasus->dr,

-			     tmp, 1, ctrl_callback, pegasus);

-

-	add_wait_queue(&pegasus->ctrl_wait, &wait);

-	set_current_state(TASK_UNINTERRUPTIBLE);

-

-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {

-		if (ret == -ENODEV)

-			netif_device_detach(pegasus->net);

-		if (net_ratelimit())

-			netif_err(pegasus, drv, pegasus->net,

-				  "%s, status %d\n", __func__, ret);

-		goto out;

-	}

-

-	schedule();

-out:

-	remove_wait_queue(&pegasus->ctrl_wait, &wait);

-	kfree(tmp);

+	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),

+	                      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,

+	                      indx, &data, 1, 1000);

+	if (ret < 0)

+		netif_dbg(pegasus, drv, pegasus->net,

+		          "%s returned %d\n", __func__, ret);

 
 	return ret;
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
 {
-	int ret;

+	int ret = -ENOMEM;

+	struct urb *async_urb;

+

+	if ((async_urb = usb_alloc_urb(0, GFP_ATOMIC)) == NULL)

+		return ret;

 
 	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;
 	pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS;
 	pegasus->dr.wValue = cpu_to_le16(0);
 	pegasus->dr.wIndex = cpu_to_le16(EthCtrl0);
 	pegasus->dr.wLength = cpu_to_le16(3);
-	pegasus->ctrl_urb->transfer_buffer_length = 3;

+	async_urb->transfer_buffer_length = 3;

 
-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,

-			     usb_sndctrlpipe(pegasus->usb, 0),

-			     (char *) &pegasus->dr,

-			     pegasus->eth_regs, 3, ctrl_callback, pegasus);

+	usb_fill_control_urb(async_urb, pegasus->usb,

+	                     usb_sndctrlpipe(pegasus->usb, 0),

+	                     (char *) &pegasus->dr, pegasus->eth_regs,

+	                     3, async_ctrl_callback, pegasus);

 
-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {

+	if ((ret = usb_submit_urb(async_urb, GFP_ATOMIC))) {

 		if (ret == -ENODEV)
 			netif_device_detach(pegasus->net);
 		netif_err(pegasus, drv, pegasus->net,
-			  "%s, status %d\n", __func__, ret);

+		          "%s returned %d\n", __func__, ret);

 	}
 
 	return ret;
@@ -342,8 +229,10 @@  static int read_mii_word(pegasus_t *pegasus, __u8 phy, __u8 indx, __u16 *regd)

 			break;
 	}
 
-	if (i >= REG_TIMEOUT)

+	if (i >= REG_TIMEOUT) {

+		ret = -EBUSY;

 		goto fail;
+	}

 
 	ret = get_registers(pegasus, PhyData, 2, &regdi);
 	*regd = le16_to_cpu(regdi);
@@ -480,7 +369,7 @@  fail:

 	netif_warn(pegasus, drv, pegasus->net, "%s failed\n", __func__);
 	return -ETIMEDOUT;
 }
-#endif				/* PEGASUS_WRITE_EEPROM */

+#endif	/* PEGASUS_WRITE_EEPROM */

 
 static inline void get_node_id(pegasus_t *pegasus, __u8 *id)
 {
@@ -543,12 +432,11 @@  static inline int reset_mac(pegasus_t *pegasus)

 	return 0;
 }
 
-static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)

+static void enable_net_traffic(struct net_device *dev, struct usb_device *usb)

 {
 	__u16 linkpart;
 	__u8 data[4];
 	pegasus_t *pegasus = netdev_priv(dev);
-	int ret;

 
 	read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
 	data[0] = 0xc9;
@@ -562,7 +450,7 @@  static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)

 	data[2] = loopback ? 0x09 : 0x01;
 
 	memcpy(pegasus->eth_regs, data, sizeof(data));
-	ret = set_registers(pegasus, EthCtrl0, 3, data);

+	set_registers(pegasus, EthCtrl0, 3, data);

 
 	if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
 	    usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
@@ -571,53 +459,6 @@  static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)

 		read_mii_word(pegasus, 0, 0x1b, &auxmode);
 		write_mii_word(pegasus, 0, 0x1b, auxmode | 4);
 	}
-

-	return ret;

-}

-

-static void fill_skb_pool(pegasus_t *pegasus)

-{

-	int i;

-

-	for (i = 0; i < RX_SKBS; i++) {

-		if (pegasus->rx_pool[i])

-			continue;

-		pegasus->rx_pool[i] = dev_alloc_skb(PEGASUS_MTU + 2);

-		/*

-		 ** we give up if the allocation fail. the tasklet will be

-		 ** rescheduled again anyway...

-		 */

-		if (pegasus->rx_pool[i] == NULL)

-			return;

-		skb_reserve(pegasus->rx_pool[i], 2);

-	}

-}

-

-static void free_skb_pool(pegasus_t *pegasus)

-{

-	int i;

-

-	for (i = 0; i < RX_SKBS; i++) {

-		if (pegasus->rx_pool[i]) {

-			dev_kfree_skb(pegasus->rx_pool[i]);

-			pegasus->rx_pool[i] = NULL;

-		}

-	}

-}

-

-static inline struct sk_buff *pull_skb(pegasus_t * pegasus)

-{

-	int i;

-	struct sk_buff *skb;

-

-	for (i = 0; i < RX_SKBS; i++) {

-		if (likely(pegasus->rx_pool[i] != NULL)) {

-			skb = pegasus->rx_pool[i];

-			pegasus->rx_pool[i] = NULL;

-			return skb;

-		}

-	}

-	return NULL;

 }
 
 static void read_bulk_callback(struct urb *urb)
@@ -643,7 +484,7 @@  static void read_bulk_callback(struct urb *urb)

 		netif_dbg(pegasus, rx_err, net, "reset MAC\n");
 		pegasus->flags &= ~PEGASUS_RX_BUSY;
 		break;
-	case -EPIPE:		/* stall, or disconnect from TT */

+	case -EPIPE:	/* stall, or disconnect from TT */

 		/* FIXME schedule work to clear the halt */
 		netif_warn(pegasus, rx_err, net, "no rx stall recovery\n");
 		return;
@@ -663,7 +504,7 @@  static void read_bulk_callback(struct urb *urb)

 	rx_status = buf[count - 2];
 	if (rx_status & 0x1e) {
 		netif_dbg(pegasus, rx_err, net,
-			  "RX packet error %x\n", rx_status);

+		          "RX packet error %x\n", rx_status);

 		pegasus->stats.rx_errors++;
 		if (rx_status & 0x06)	/* long or runt	*/
 			pegasus->stats.rx_length_errors++;
@@ -704,9 +545,8 @@  static void read_bulk_callback(struct urb *urb)

 	if (pegasus->flags & PEGASUS_UNPLUG)
 		return;
 
-	spin_lock(&pegasus->rx_pool_lock);

-	pegasus->rx_skb = pull_skb(pegasus);

-	spin_unlock(&pegasus->rx_pool_lock);

+	pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net,

+	                                              PEGASUS_MTU, GFP_ATOMIC);

 
 	if (pegasus->rx_skb == NULL)
 		goto tl_sched;
@@ -734,24 +574,23 @@  tl_sched:

 static void rx_fixup(unsigned long data)
 {
 	pegasus_t *pegasus;
-	unsigned long flags;

 	int status;
 
 	pegasus = (pegasus_t *) data;
 	if (pegasus->flags & PEGASUS_UNPLUG)
 		return;
 
-	spin_lock_irqsave(&pegasus->rx_pool_lock, flags);

-	fill_skb_pool(pegasus);

 	if (pegasus->flags & PEGASUS_RX_URB_FAIL)
 		if (pegasus->rx_skb)
 			goto try_again;
 	if (pegasus->rx_skb == NULL)
-		pegasus->rx_skb = pull_skb(pegasus);

+		pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net,

+		                                              PEGASUS_MTU,

+		                                              GFP_ATOMIC);

 	if (pegasus->rx_skb == NULL) {
 		netif_warn(pegasus, rx_err, pegasus->net, "low on memory\n");
 		tasklet_schedule(&pegasus->rx_tl);
-		goto done;

+		return;

 	}
 	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
 			  usb_rcvbulkpipe(pegasus->usb, 1),
@@ -767,8 +606,6 @@  try_again:

 	} else {
 		pegasus->flags &= ~PEGASUS_RX_URB_FAIL;
 	}
-done:

-	spin_unlock_irqrestore(&pegasus->rx_pool_lock, flags);

 }
 
 static void write_bulk_callback(struct urb *urb)
@@ -860,7 +697,7 @@  static void intr_callback(struct urb *urb)

 		netif_device_detach(pegasus->net);
 	if (res)
 		netif_err(pegasus, timer, net,
-			  "can't resubmit interrupt urb, %d\n", res);

+		          "can't resubmit interrupt urb, %d\n", res);

 }
 
 static void pegasus_tx_timeout(struct net_device *net)
@@ -890,10 +727,10 @@  static netdev_tx_t pegasus_start_xmit(struct sk_buff *skb,

 	if ((res = usb_submit_urb(pegasus->tx_urb, GFP_ATOMIC))) {
 		netif_warn(pegasus, tx_err, net, "fail tx, %d\n", res);
 		switch (res) {
-		case -EPIPE:		/* stall, or disconnect from TT */

+		case -EPIPE:	/* stall, or disconnect from TT */

 			/* cleanup should already have been scheduled */
 			break;
-		case -ENODEV:		/* disconnect() upcoming */

+		case -ENODEV:	/* disconnect() upcoming */

 		case -EPERM:
 			netif_device_detach(pegasus->net);
 			break;
@@ -932,8 +769,8 @@  static inline void get_interrupt_interval(pegasus_t *pegasus)

 	if (pegasus->usb->speed != USB_SPEED_HIGH) {
 		if (interval < 0x80) {
 			netif_info(pegasus, timer, pegasus->net,
-				   "intr interval changed from %ums to %ums\n",

-				   interval, 0x80);

+			           "intr interval changed from %ums to %ums\n",

+			           interval, 0x80);

 			interval = 0x80;
 			data = (data & 0x00FF) | ((u16)interval << 8);
 #ifdef PEGASUS_WRITE_EEPROM
@@ -951,7 +788,6 @@  static void set_carrier(struct net_device *net)

 
 	if (read_mii_word(pegasus, pegasus->phy, MII_BMSR, &tmp))
 		return;
-

 	if (tmp & BMSR_LSTATUS)
 		netif_carrier_on(net);
 	else
@@ -963,7 +799,6 @@  static void free_all_urbs(pegasus_t *pegasus)

 	usb_free_urb(pegasus->intr_urb);
 	usb_free_urb(pegasus->tx_urb);
 	usb_free_urb(pegasus->rx_urb);
-	usb_free_urb(pegasus->ctrl_urb);

 }
 
 static void unlink_all_urbs(pegasus_t *pegasus)
@@ -971,30 +806,23 @@  static void unlink_all_urbs(pegasus_t *pegasus)

 	usb_kill_urb(pegasus->intr_urb);
 	usb_kill_urb(pegasus->tx_urb);
 	usb_kill_urb(pegasus->rx_urb);
-	usb_kill_urb(pegasus->ctrl_urb);

 }
 
 static int alloc_urbs(pegasus_t *pegasus)
 {
-	pegasus->ctrl_urb = usb_alloc_urb(0, GFP_KERNEL);

-	if (!pegasus->ctrl_urb)

-		return 0;

 	pegasus->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->rx_urb) {
-		usb_free_urb(pegasus->ctrl_urb);

 		return 0;
 	}
 	pegasus->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->tx_urb) {
 		usb_free_urb(pegasus->rx_urb);
-		usb_free_urb(pegasus->ctrl_urb);

 		return 0;
 	}
 	pegasus->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->intr_urb) {
 		usb_free_urb(pegasus->tx_urb);
 		usb_free_urb(pegasus->rx_urb);
-		usb_free_urb(pegasus->ctrl_urb);

 		return 0;
 	}
 
@@ -1004,15 +832,12 @@  static int alloc_urbs(pegasus_t *pegasus)

 static int pegasus_open(struct net_device *net)
 {
 	pegasus_t *pegasus = netdev_priv(net);
-	int res;

+	int res=-ENOMEM;

 
 	if (pegasus->rx_skb == NULL)
-		pegasus->rx_skb = pull_skb(pegasus);

-	/*

-	 ** Note: no point to free the pool.  it is empty :-)

-	 */

+		pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net, PEGASUS_MTU, GFP_KERNEL);

 	if (!pegasus->rx_skb)
-		return -ENOMEM;

+		goto exit;

 
 	res = set_registers(pegasus, EthID, 6, net->dev_addr);
 
@@ -1038,18 +863,9 @@  static int pegasus_open(struct net_device *net)

 		usb_kill_urb(pegasus->rx_urb);
 		goto exit;
 	}
-	if ((res = enable_net_traffic(net, pegasus->usb))) {

-		netif_dbg(pegasus, ifup, net,

-			  "can't enable_net_traffic() - %d\n", res);

-		res = -EIO;

-		usb_kill_urb(pegasus->rx_urb);

-		usb_kill_urb(pegasus->intr_urb);

-		free_skb_pool(pegasus);

-		goto exit;

-	}

+	enable_net_traffic(net, pegasus->usb);

 	set_carrier(net);
 	netif_start_queue(net);
-	netif_dbg(pegasus, ifup, net, "open\n");

 	res = 0;
 exit:
 	return res;
@@ -1113,8 +929,7 @@  pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)

 
 	ret = set_register(pegasus, WakeupControl, reg78);
 	if (!ret)
-		ret = device_set_wakeup_enable(&pegasus->usb->dev,

-						wol->wolopts);

+		ret = device_set_wakeup_enable(&pegasus->usb->dev, wol->wolopts);

 	return ret;
 }
 
@@ -1214,16 +1029,13 @@  static void pegasus_set_multicast(struct net_device *net)

 	} else if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI)) {
 		pegasus->eth_regs[EthCtrl0] |= RX_MULTICAST;
 		pegasus->eth_regs[EthCtrl2] &= ~RX_PROMISCUOUS;
-		netif_dbg(pegasus, link, net, "set allmulti\n");

+		netif_info(pegasus, link, net, "set allmulti\n");

 	} else {
 		pegasus->eth_regs[EthCtrl0] &= ~RX_MULTICAST;
 		pegasus->eth_regs[EthCtrl2] &= ~RX_PROMISCUOUS;
+		netif_info(pegasus, link, net, "general mode\n");

 	}
-

-	pegasus->ctrl_urb->status = 0;

-

-	pegasus->flags |= ETH_REGS_CHANGE;

-	ctrl_callback(pegasus->ctrl_urb);

+	update_eth_regs_async(pegasus);

 }
 
 static __u8 mii_phy_probe(pegasus_t *pegasus)
@@ -1281,10 +1093,9 @@  static void check_carrier(struct work_struct *work)

 {
 	pegasus_t *pegasus = container_of(work, pegasus_t, carrier_check.work);
 	set_carrier(pegasus->net);
-	if (!(pegasus->flags & PEGASUS_UNPLUG)) {

+	if (!(pegasus->flags & PEGASUS_UNPLUG))

 		queue_delayed_work(pegasus_workqueue, &pegasus->carrier_check,
-			CARRIER_CHECK_DELAY);

-	}

+		                   CARRIER_CHECK_DELAY);

 }
 
 static int pegasus_blacklisted(struct usb_device *udev)
@@ -1303,7 +1114,8 @@  static int pegasus_blacklisted(struct usb_device *udev)

 	return 0;
 }
 
-/* we rely on probe() and remove() being serialized so we

+/*

+ * we rely on probe() and remove() being serialized so we

  * don't need extra locking on pegasus_count.
  */
 static void pegasus_dec_workqueue(void)
@@ -1340,7 +1152,6 @@  static int pegasus_probe(struct usb_interface *intf,

 
 	pegasus = netdev_priv(net);
 	pegasus->dev_index = dev_index;
-	init_waitqueue_head(&pegasus->ctrl_wait);

 
 	if (!alloc_urbs(pegasus)) {
 		dev_err(&intf->dev, "can't allocate %s\n", "urbs");
@@ -1364,7 +1175,6 @@  static int pegasus_probe(struct usb_interface *intf,

 	pegasus->mii.mdio_write = mdio_write;
 	pegasus->mii.phy_id_mask = 0x1f;
 	pegasus->mii.reg_num_mask = 0x1f;
-	spin_lock_init(&pegasus->rx_pool_lock);

 	pegasus->msg_enable = netif_msg_init(msg_level, NETIF_MSG_DRV
 				| NETIF_MSG_PROBE | NETIF_MSG_LINK);
 
@@ -1376,7 +1186,6 @@  static int pegasus_probe(struct usb_interface *intf,

 		goto out2;
 	}
 	set_ethernet_addr(pegasus);
-	fill_skb_pool(pegasus);

 	if (pegasus->features & PEGASUS_II) {
 		dev_info(&intf->dev, "setup Pegasus II specific registers\n");
 		setup_pegasus_II(pegasus);
@@ -1396,15 +1205,12 @@  static int pegasus_probe(struct usb_interface *intf,

 	queue_delayed_work(pegasus_workqueue, &pegasus->carrier_check,
 				CARRIER_CHECK_DELAY);
 
-	dev_info(&intf->dev, "%s, %s, %pM\n",

-		 net->name,

-		 usb_dev_id[dev_index].name,

-		 net->dev_addr);

+	dev_info(&intf->dev, "%s, %s, %pM\n", net->name,

+	         usb_dev_id[dev_index].name, net->dev_addr);

 	return 0;
 
 out3:
 	usb_set_intfdata(intf, NULL);
-	free_skb_pool(pegasus);

 out2:
 	free_all_urbs(pegasus);
 out1:
@@ -1429,7 +1235,6 @@  static void pegasus_disconnect(struct usb_interface *intf)

 	unregister_netdev(pegasus->net);
 	unlink_all_urbs(pegasus);
 	free_all_urbs(pegasus);
-	free_skb_pool(pegasus);

 	if (pegasus->rx_skb != NULL) {
 		dev_kfree_skb(pegasus->rx_skb);
 		pegasus->rx_skb = NULL;