Patchwork Dot not insert RX urbs if EVENT_DEV_ASLEEP flag is set.

login
register
mail settings
Submitter Indrek Peri
Date Dec. 6, 2010, 10:24 a.m.
Message ID <1291631047-23766-1-git-send-email-Indrek.Peri@Ericsson.com>
Download mbox | patch
Permalink /patch/74331/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Indrek Peri - Dec. 6, 2010, 10:24 a.m.
Signed-off-by: Indrek Peri <Indrek.Peri@Ericsson.com>
---
 drivers/net/usb/usbnet.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
David Brownell - Dec. 7, 2010, 2:49 a.m.
I realize it's not all your doing, but that's an EVENT not a state flag,

But you're treating it as a state flag.  (Like the original somewhat
goofy 69ee472f2706371ca639de49b06df91615c07d8d from Oliver).  Even as
a flag, its semantics seem fuzzy... like they should be covered by
other state in the network and/or power management stacks.

I'd like to see this botch fixed, not worsened.  Maybe you and Oliver
could collaborate on fixing it, and getting other state flags out ofthe
word used to track and signal events?  If you've ever worked with
hardware
that makes that kind of mistake, you know how error-prone it is ...

Meanwhile, NAK.

ISTR related Comments came up when a previous incarnation of this patch
circulated, from at least Alan Stern.  What happened to that?

Events trigger state transitions and similar actions.  So
A "leave sleep state" event" might be a place to trigger RX urb
insertion as part of wake-up processing.  (Ideally, there'd be
no need to set an event bit to trigger it as deferred work ... the
reason to have a word of event bits for some other events.




--
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/usbnet.c b/drivers/net/usb/usbnet.c
index c04d49e..d83adcc 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1172,7 +1172,8 @@  static void usbnet_bh (unsigned long param)
 	} else if (netif_running (dev->net) &&
 		   netif_device_present (dev->net) &&
 		   !timer_pending (&dev->delay) &&
-		   !test_bit (EVENT_RX_HALT, &dev->flags)) {
+		   !test_bit (EVENT_RX_HALT, &dev->flags) &&
+		   !test_bit (EVENT_DEV_ASLEEP, &dev->flags)) {
 		int	temp = dev->rxq.qlen;
 		int	qlen = RX_QLEN (dev);