Re: USB suspend issue with 2.6.38-rc2 -- still there in RC4-g795abaf

On Mon, 14 Feb 2011, Dirk Hohndel wrote:

On Mon, 14 Feb 2011 11:33:01 -0800, Linus Torvalds [off-list ref] wrote:

quoted

Does it suspend reliably if we just get rid of the crap that looks for errors?

Yes. I tried all combinations I could think of. Docked. Undocked.
Docking while suspended. Undocking while suspended. Did something like
20 suspend / resume cycles in a row. All works.

I will continue to play with this but for now I consider this solved

quoted

As mentioned, there's no _point_ to looking at errors. Reacting to
errors in suspend can only make things worse, not better. If the
suspend of some device fails, then it fails. Big deal. Take the
machine down regardless.

Trial patch attached. Untested. It may do nasty things to your pets.

Pets didn't mind at all (but I kept them away from the machine, just in
case)

/D

quoted

 drivers/usb/core/driver.c |   33 +++++++++------------------------
 1 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index fca6172..e302189 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c

@@ -1173,7 +1173,6 @@ done:
  */
 static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 {
-	int			status = 0;
 	int			i = 0, n = 0;
 	struct usb_interface	*intf;
 

@@ -1186,36 +1185,22 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 		n = udev->actconfig->desc.bNumInterfaces;
 		for (i = n - 1; i >= 0; --i) {
 			intf = udev->actconfig->interface[i];
-			status = usb_suspend_interface(udev, intf, msg);
-			if (status != 0)
-				break;
+			usb_suspend_interface(udev, intf, msg);
 		}
 	}
-	if (status == 0)
-		status = usb_suspend_device(udev, msg);
-
-	/* If the suspend failed, resume interfaces that did get suspended */
-	if (status != 0) {
-		msg.event ^= (PM_EVENT_SUSPEND | PM_EVENT_RESUME);
-		while (++i < n) {
-			intf = udev->actconfig->interface[i];
-			usb_resume_interface(udev, intf, msg, 0);
-		}
+	usb_suspend_device(udev, msg);
 
-	/* If the suspend succeeded then prevent any more URB submissions
-	 * and flush any outstanding URBs.
+	/* Prevent any more URB submissions and flush any outstanding URBs.
 	 */
-	} else {
-		udev->can_submit = 0;
-		for (i = 0; i < 16; ++i) {
-			usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
-			usb_hcd_flush_endpoint(udev, udev->ep_in[i]);
-		}
+	udev->can_submit = 0;
+	for (i = 0; i < 16; ++i) {
+		usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
+		usb_hcd_flush_endpoint(udev, udev->ep_in[i]);
 	}
 
  done:
-	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
-	return status;
+	dev_vdbg(&udev->dev, "%s: suspend\n", __func__);
+	return 0;
 }
 
 /**

This doesn't differentiate between system sleep and runtime suspend;  
the patch below is better.  I'll submit it in addition to the change to
the hub driver.  After all, other drivers may also fail during a sleep
transition.

Alan Stern



Index: usb-2.6/drivers/usb/core/driver.c
===================================================================

--- usb-2.6.orig/drivers/usb/core/driver.c
+++ usb-2.6/drivers/usb/core/driver.c

@@ -1187,6 +1187,10 @@ static int usb_suspend_both(struct usb_d
 		for (i = n - 1; i >= 0; --i) {
 			intf = udev->actconfig->interface[i];
 			status = usb_suspend_interface(udev, intf, msg);
+
+			/* Ignore errors during system sleep transitions */
+			if (!(msg.event & PM_EVENT_AUTO))
+				status = 0;
 			if (status != 0)
 				break;
 		}

@@ -1195,7 +1199,7 @@ static int usb_suspend_both(struct usb_d
 		status = usb_suspend_device(udev, msg);
 
 	/* If the suspend failed, resume interfaces that did get suspended */
-	if (status != 0) {
+	if (status != 0 && (msg.event & PM_EVENT_AUTO)) {
 		msg.event ^= (PM_EVENT_SUSPEND | PM_EVENT_RESUME);
 		while (++i < n) {
 			intf = udev->actconfig->interface[i];

@@ -1303,7 +1307,9 @@ int usb_suspend(struct device *dev, pm_m
 
 	do_unbind_rebind(udev, DO_UNBIND);
 	choose_wakeup(udev, msg);
-	return usb_suspend_both(udev, msg);
+	usb_suspend_both(udev, msg);
+
+	return 0;	/* Allow system to go to sleep regardless of errors */
 }
 
 /* The device lock is held by the PM core */