[PATCH] Make sleep/wakeup work with USB on powerbooks

[PATCH] Make sleep/wakeup work with USB on powerbooks

[Paul Mackerras]

I am currently using this patch on my powerbook to fix the problems
that USB was causing with sleep and wakeup.  Basically one of the USB
controllers was getting a spurious wakeup immediately when put it
into the suspend state.  This would cause the resume routine to be run
after we had turned off the device, causing a machine check.

Also we had some races where we would turn off the clock to the apple
OHCI cell(s) and then try to access them.  With this patch, sleep and
wakeup are quite reliable.  The patch is against 2.6.11.

Paul.

diff -urN linux-2.6.11/drivers/usb/core/hcd-pci.c pmac-2.6.11/drivers/usb/core/hcd-pci.c
--- linux-2.6.11/drivers/usb/core/hcd-pci.c	2004-12-19 00:54:45.000000000 +1100
+++ pmac-2.6.11/drivers/usb/core/hcd-pci.c	2005-02-07 14:49:12.000000000 +1100
@@ -32,6 +32,12 @@
 #include <linux/usb.h>
 #include "hcd.h"
 
+#ifdef CONFIG_PMAC_PBOOK
+#include <asm/machdep.h>
+#include <asm/pmac_feature.h>
+#include <asm/pci-bridge.h>
+#include <asm/prom.h>
+#endif
 
 /* PCI-based HCs are normal, but custom bus glue should be ok */
 
@@ -360,6 +366,17 @@
 		break;
 	}
 
+#ifdef CONFIG_PMAC_PBOOK
+	if (retval == 0 && _machine == _MACH_Pmac) {
+	   	struct device_node	*of_node;
+ 
+		/* Disable USB PAD & cell clock */
+		of_node = pci_device_to_OF_node (dev);
+		if (of_node)
+			pmac_call_feature(PMAC_FTR_USB_ENABLE, of_node, 0, 0);
+	}
+#endif /* CONFIG_PMAC_PBOOK */
+
 	/* update power_state **ONLY** to make sysfs happier */
 	if (retval == 0)
 		dev->dev.power.power_state = state;
@@ -380,6 +397,20 @@
 	int			has_pci_pm;
 
 	hcd = pci_get_drvdata(dev);
+
+#ifdef CONFIG_PMAC_PBOOK
+	if (_machine == _MACH_Pmac) {
+		struct device_node *of_node;
+
+		/* Re-enable USB PAD & cell clock */
+		of_node = pci_device_to_OF_node(dev);
+		if (of_node) {
+			pmac_call_feature (PMAC_FTR_USB_ENABLE, of_node, 0, 1);
+			mdelay(10);
+		}
+	}
+#endif /* CONFIG_PMAC_PBOOK */
+
 	if (hcd->state != HCD_STATE_SUSPENDED) {
 		dev_dbg (hcd->self.controller, 
 				"can't resume, not suspended!\n");
@@ -396,14 +427,8 @@
 
 	if (has_pci_pm)
 		pci_set_power_state (dev, 0);
+	mdelay(1);
 	dev->dev.power.power_state = 0;
-	retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
-				hcd->driver->description, hcd);
-	if (retval < 0) {
-		dev_err (hcd->self.controller,
-			"can't restore IRQ after resume!\n");
-		return retval;
-	}
 	hcd->saw_irq = 0;
 	pci_restore_state (dev);
 #ifdef	CONFIG_USB_SUSPEND
@@ -417,6 +442,16 @@
 				"resume fail, retval %d\n", retval);
 		usb_hc_died (hcd);
 	}
+	if (retval)
+		return retval;
+
+	retval = request_irq (dev->irq, usb_hcd_irq, SA_SHIRQ,
+				hcd->driver->description, hcd);
+	if (retval < 0) {
+		dev_err (hcd->self.controller,
+			"can't restore IRQ after resume!\n");
+		return retval;
+	}
 
 	return retval;
 }
diff -urN linux-2.6.11/drivers/usb/host/ohci-hcd.c pmac-2.6.11/drivers/usb/host/ohci-hcd.c
--- linux-2.6.11/drivers/usb/host/ohci-hcd.c	2005-01-31 17:33:41.000000000 +1100
+++ pmac-2.6.11/drivers/usb/host/ohci-hcd.c	2005-02-07 16:27:33.000000000 +1100
@@ -609,8 +609,10 @@
  	ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
 	ohci_to_hcd(ohci)->state = USB_STATE_RUNNING;
 
+#ifdef CONFIG_USB_SUSPEND
 	/* wake on ConnectStatusChange, matching external hubs */
 	ohci_writel (ohci, RH_HS_DRWE, &ohci->regs->roothub.status);
+#endif
 
 	/* Choose the interrupts we care about now, others later on demand */
 	mask = OHCI_INTR_INIT;
@@ -720,7 +722,9 @@
 
 	if (ints & OHCI_INTR_RD) {
 		ohci_vdbg (ohci, "resume detect\n");
-		schedule_work(&ohci->rh_resume);
+		ohci_writel (ohci, OHCI_INTR_RD, &regs->intrstatus);
+		if (!(ohci->flags & OHCI_SUSPENDING))
+			schedule_work(&ohci->rh_resume);
 	}
 
 	if (ints & OHCI_INTR_WDH) {
diff -urN linux-2.6.11/drivers/usb/host/ohci-hub.c pmac-2.6.11/drivers/usb/host/ohci-hub.c
--- linux-2.6.11/drivers/usb/host/ohci-hub.c	2004-12-23 10:48:43.000000000 +1100
+++ pmac-2.6.11/drivers/usb/host/ohci-hub.c	2005-02-07 15:23:56.000000000 +1100
@@ -102,10 +102,12 @@
 	ohci_writel (ohci, ohci_readl (ohci, &ohci->regs->intrstatus),
 			&ohci->regs->intrstatus);
 
+#ifdef CONFIG_USB_SUSPEND
 	/* maybe resume can wake root hub */
 	if (hcd->remote_wakeup)
 		ohci->hc_control |= OHCI_CTRL_RWE;
 	else
+#endif
 		ohci->hc_control &= ~OHCI_CTRL_RWE;
 
 	/* Suspend hub */
diff -urN linux-2.6.11/drivers/usb/host/ohci-pci.c pmac-2.6.11/drivers/usb/host/ohci-pci.c
--- linux-2.6.11/drivers/usb/host/ohci-pci.c	2004-12-19 02:22:52.000000000 +1100
+++ pmac-2.6.11/drivers/usb/host/ohci-pci.c	2005-02-07 15:23:53.000000000 +1100
@@ -114,23 +114,22 @@
 	(void) usb_suspend_device (hcd->self.root_hub, state);
 #else
 	usb_lock_device (hcd->self.root_hub);
+
+	/* don't wake on ConnectStatusChange */
+	ohci_writel(ohci, RH_HS_CRWE, &ohci->regs->roothub.status);
+	ohci_readl(ohci, &ohci->regs->roothub.status);
+	ohci->flags |= OHCI_SUSPENDING;
+	msleep(10);
+
 	(void) ohci_hub_suspend (hcd);
 	usb_unlock_device (hcd->self.root_hub);
+
 #endif
 
 	/* let things settle down a bit */
+	flush_scheduled_work();
 	msleep (100);
 	
-#ifdef CONFIG_PMAC_PBOOK
-	if (_machine == _MACH_Pmac) {
-	   	struct device_node	*of_node;
- 
-		/* Disable USB PAD & cell clock */
-		of_node = pci_device_to_OF_node (to_pci_dev(hcd->self.controller));
-		if (of_node)
-			pmac_call_feature(PMAC_FTR_USB_ENABLE, of_node, 0, 0);
-	}
-#endif /* CONFIG_PMAC_PBOOK */
 	return 0;
 }
 
@@ -140,17 +139,6 @@
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
 	int			retval = 0;
 
-#ifdef CONFIG_PMAC_PBOOK
-	if (_machine == _MACH_Pmac) {
-		struct device_node *of_node;
-
-		/* Re-enable USB PAD & cell clock */
-		of_node = pci_device_to_OF_node (to_pci_dev(hcd->self.controller));
-		if (of_node)
-			pmac_call_feature (PMAC_FTR_USB_ENABLE, of_node, 0, 1);
-	}
-#endif /* CONFIG_PMAC_PBOOK */
-
 	/* resume root hub */
 	if (time_before (jiffies, ohci->next_statechange))
 		msleep (100);
@@ -159,6 +147,7 @@
 	retval = usb_resume_device (hcd->self.root_hub);
 #else
 	usb_lock_device (hcd->self.root_hub);
+	ohci->flags &= ~OHCI_SUSPENDING;
 	retval = ohci_hub_resume (hcd);
 	usb_unlock_device (hcd->self.root_hub);
 #endif
diff -urN linux-2.6.11/drivers/usb/host/ohci.h pmac-2.6.11/drivers/usb/host/ohci.h
--- linux-2.6.11/drivers/usb/host/ohci.h	2004-12-19 02:22:52.000000000 +1100
+++ pmac-2.6.11/drivers/usb/host/ohci.h	2005-02-06 12:32:37.000000000 +1100
@@ -397,6 +397,7 @@
 #define	OHCI_QUIRK_INITRESET	0x04			/* SiS, OPTi, ... */
 #define	OHCI_BIG_ENDIAN		0x08			/* big endian HC */
 	// there are also chip quirks/bugs in init logic
+#define OHCI_SUSPENDING		0x100
 
 };
 

Re: [PATCH] Make sleep/wakeup work with USB on powerbooks

[Colin Leroy]

On 17 Mar 2005 at 16h03, Paul Mackerras wrote:

Hi, 

> I am currently using this patch on my powerbook to fix the problems
> that USB was causing with sleep and wakeup.  Basically one of the USB
> controllers was getting a spurious wakeup immediately when put it
> into the suspend state.  This would cause the resume routine to be run
> after we had turned off the device, causing a machine check.
> 
> Also we had some races where we would turn off the clock to the apple
> OHCI cell(s) and then try to access them.  With this patch, sleep and
> wakeup are quite reliable.  The patch is against 2.6.11.

Would it be the "proper" patch I was looking for in the thread named
"[PATCH] Re-power USB ports on wakeup" ?

Or does it fix different issues ? Mine fixed, with a hack I think, the
fact that USB ports kept being powered down after one wakeup out of
two. I never got any machine check, however.

Thanks,
-- 
Colin

Re: [PATCH] Make sleep/wakeup work with USB on powerbooks

[Benjamin Herrenschmidt]

> Would it be the "proper" patch I was looking for in the thread named
> "[PATCH] Re-power USB ports on wakeup" ?
> 
> Or does it fix different issues ? Mine fixed, with a hack I think, the
> fact that USB ports kept being powered down after one wakeup out of
> two. I never got any machine check, however.

Test please, your patch shouldn't be necessary.

Ben.

Re: [PATCH] Make sleep/wakeup work with USB on powerbooks

[Colin Leroy]

On 17 Mar 2005 at 18h03, Benjamin Herrenschmidt wrote:

Hi, 

> > Would it be the "proper" patch I was looking for in the thread named
> > "[PATCH] Re-power USB ports on wakeup" ?
> > 
> > Or does it fix different issues ? Mine fixed, with a hack I think,
> > the fact that USB ports kept being powered down after one wakeup
> > out of two. I never got any machine check, however.
> 
> Test please, your patch shouldn't be necessary.

Just did so, it's not enough. My patch seems necessary so that usb
ports are powered at each wakeup (the second one, that doesn't touch
anything at sleep and sets power unconditionnaly at wakeup, works fine).
With Paul's patch, (dis)connecting USB devices during sleep isn't a
problem anymore.
-- 
Colin

Re: [linux-usb-devel] [PATCH] Make sleep/wakeup work with USB on powerbooks

[Alan Stern]

On Thu, 17 Mar 2005, Paul Mackerras wrote:

> I am currently using this patch on my powerbook to fix the problems
> that USB was causing with sleep and wakeup.  Basically one of the USB
> controllers was getting a spurious wakeup immediately when put it
> into the suspend state.  This would cause the resume routine to be run
> after we had turned off the device, causing a machine check.

Is this just a hardware glitch or does it go deeper?

> Also we had some races where we would turn off the clock to the apple
> OHCI cell(s) and then try to access them.  With this patch, sleep and
> wakeup are quite reliable.  The patch is against 2.6.11.

Aside from the ppc-specific portions, the changes you made to hcd-pci.c 
resemble those I just posted in

http://lists.osdl.org/pipermail/linux-pm/2005-March/000616.html

(meant for the gregkh-2.6 USB development tree).

Alan Stern

Re: [linux-usb-devel] [PATCH] Make sleep/wakeup work with USB on powerbooks

[David Brownell]

On Wednesday 16 March 2005 9:36 pm, Paul Mackerras wrote:
> I am currently using this patch on my powerbook to fix the problems
> that USB was causing with sleep and wakeup.  Basically one of the USB
> controllers was getting a spurious wakeup immediately when put it
> into the suspend state.  This would cause the resume routine to be run
> after we had turned off the device, causing a machine check.

Do you know yet why the spurious wakeup happened?  As I recall from
earlier discussions, that was indeed the root cause of the problem,
although there were a few other oddnesses that needed handling too.
But without the spurious wakeup, they'd have been quite rare.


> Also we had some races where we would turn off the clock to the apple
> OHCI cell(s) and then try to access them.  With this patch, sleep and
> wakeup are quite reliable.  The patch is against 2.6.11.

Thanks, I'll have a more detailed look soon.  Most of it looks fine,
except for the stuff related to this:

> +#ifdef CONFIG_USB_SUSPEND
>  	/* wake on ConnectStatusChange, matching external hubs */
>  	ohci_writel (ohci, RH_HS_DRWE, &ohci->regs->roothub.status);
> +#endif

Thing is, that change would also prevent us from getting rid of
the root hub timer for OHCI.   IRQ on connect status change is
not specific to USB_SUSPEND.

- Dave