On Mon, 9 Apr 2007 [EMAIL PROTECTED] wrote: > > This is a note to let you know that I've just added the patch titled > > Subject: USB: jiffies is an unsigned long so treat it that way > > to my gregkh-2.6 tree. Its filename is > > usb-jiffies-long.patch > > This tree can be found at > http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/ > > > >From [EMAIL PROTECTED] Tue Apr 9 12:12:43 2002 > Date: Mon, 9 Apr 2007 11:52:31 -0400 (EDT) > To: Greg KH <[EMAIL PROTECTED]> > From: Greg Kroah-Hartman <[EMAIL PROTECTED]> > Subject: USB: jiffies is an unsigned long so treat it that way > > In the suspend logic, we are using jiffies as a signed long, which can > cause problems. This patch fixes this, and uses the proper function to > determine if something has expired or not, instead of attempting to roll > it ourselves. > > Thanks to Oliver for helping with the unsigned/signed issues in thinking > this patch through. > > Cc: Alan Stern <[EMAIL PROTECTED]> > Cc: Oliver Neukum <[EMAIL PROTECTED]> > Cc: David Brownell <[EMAIL PROTECTED]> > Cc: linux-usb-devel <linux-usb-devel@lists.sourceforge.net> > Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> > > --- > drivers/usb/core/driver.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > --- gregkh-2.6.orig/drivers/usb/core/driver.c > +++ gregkh-2.6/drivers/usb/core/driver.c > @@ -932,7 +932,7 @@ static int autosuspend_check(struct usb_ > { > int i; > struct usb_interface *intf; > - long suspend_time; > + unsigned long suspend_time; > > /* For autosuspend, fail fast if anything is in use or autosuspend > * is disabled. Also fail if any interfaces require remote wakeup > @@ -944,7 +944,6 @@ static int autosuspend_check(struct usb_ > if (udev->autosuspend_delay < 0 || udev->autosuspend_disabled) > return -EPERM; > > - suspend_time = udev->last_busy + udev->autosuspend_delay; > if (udev->actconfig) { > for (i = 0; i < udev->actconfig->desc.bNumInterfaces; i++) { > intf = udev->actconfig->interface[i]; > @@ -964,8 +963,8 @@ static int autosuspend_check(struct usb_ > /* If everything is okay but the device hasn't been idle for long > * enough, queue a delayed autosuspend request. > */ > - suspend_time -= jiffies; > - if (suspend_time > 0) { > + suspend_time = udev->last_busy + udev->autosuspend_delay; > + if (time_after(suspend_time, jiffies)) { > if (!timer_pending(&udev->autosuspend.timer)) > queue_delayed_work(ksuspend_usb_wq, &udev->autosuspend, > suspend_time);
This change is wrong. The last argument to queue_delayed_work() is supposed to be a delay value, not an absolute time. I suppose you could simply replace "suspend_time" with "suspend_time - jiffies" in the last line above. It's a little suspect because the value of jiffies might change between the the timer_after() comparison and the function call. (That's why I wrote the code in a roundabout way -- so it wouldn't read jiffies more than once.) I suppose that wouldn't hurt; setting a timer to go off at the present moment or in the recent past does behave sanely. However it would be a good idea to add a comment explaining why it's okay. Note: If at some future point we decide to add last_busy and autosuspend_delay values for individual interfaces, then the calculation will have to change anyway. At the very least, the initial assignment to suspend_time will have to move back to where I had it originally. Alan Stern ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
