USB suspend/resume

[Hans Petter Selasky]

On Saturday 04 June 2011 19:08:22 Gavin Atkinson wrote:
> Hi all,
> 
> I've been trying to get a IBM ThinkPad X60 suspend/resume working
> correctly, and discovered that with uhci loaded, the machine would hang on
> resume.  This is on head, but I believe this is aslo the cause of the
> laptop not resuming on stable/8.
> 
> It turns out that this is because we sleep in the resume path.  The resume
> path appears to run with interrupts disabled, and as a result we never
> wake up.  A comment above uhci_suspend() saggests similar:
> 
> 	/* NOTE: suspend/resume is called from
> 	 * interrupt context and cannot sleep!
> 
> However, we do sleep.  We call usb_pause_mtx() several times during both
> tthe suspend and resume path, and on resume this is fatal.
> usb_pause_mtx() will use DELAY() when called before the machine has
> finished booting (ie cold != 0), but will call pause() once booted. To
> prove this, I've tested the following gross hack, which fixes
> suspend/resume on the ThinkPad X60:
> 
> Index: sys/dev/usb/controller/ehci_pci.c
> ===================================================================
> --- sys/dev/usb/controller/ehci_pci.c	(revision 222417)
> +++ sys/dev/usb/controller/ehci_pci.c	(working copy)
> @@ -118,10 +118,16 @@
>  ehci_pci_resume(device_t self)
>  {
>  	ehci_softc_t *sc = device_get_softc(self);
> +	int mycold;
> 
> +	mycold = cold;
> +	cold = 1;
> +
>  	ehci_pci_takecontroller(self);
>  	ehci_resume(sc);
> 
> +	cold = mycold;
> +
>  	bus_generic_resume(self);
> 
>  	return (0);
> Index: sys/dev/usb/controller/uhci_pci.c
> ===================================================================
> --- sys/dev/usb/controller/uhci_pci.c	(revision 222417)
> +++ sys/dev/usb/controller/uhci_pci.c	(working copy)
> @@ -104,11 +104,17 @@
>  uhci_pci_resume(device_t self)
>  {
>  	uhci_softc_t *sc = device_get_softc(self);
> +	int mycold;
> 
> +	mycold = cold;
> +	cold = 1;
> +
>  	pci_write_config(self, PCI_LEGSUP, PCI_LEGSUP_USBPIRQDEN, 2);
> 
>  	uhci_resume(sc);
> 
> +	cold = mycold;
> +
>  	bus_generic_resume(self);
>  	return (0);
>  }
> 
> 
> The above hack fixes this X60 and allows suspend/resume to work, and USB
> devices work perfectly after the suspend/resume.  Other controllers look
> like they may also be affected  - at least ohci does the same.
> 
> So, I guess my question to the USB experts is:  What is the correct fix?
> I see two possibilities:
> 
> 1) Have a variable somewhere called "suspres" or similar, which is set at
>    the start of the suspend, and cleared at the end of the resume.
>    usb_pause_mtx() then changes it's behaviour on (cold || suspres).
>    This may have to be a global variable, being incremented on suspend and
>    decremented on resume, as we don't pass a softc into usb_pause_mtx(),
>    however then we have issues relating to when one controller suspends
>    before the others, or resume before the others.
> 
> 2) Kick off a separate task to do the resume.  Problems with this is thet
>    it doesn't fix the suspend path, but also that we may need to rely on
>    USB being back before the rest of the resume completes. (What happens
>    if we have / or another filesystem mounted from a USB device?)
> 
> I guess #1 is the simplest solution, and should have fewest side effects,
> however it does mean that every attachment would need to be modified in
> order to set and clear the variable.  It sort of feels like this should be
> handled more centrally, but I'm not sure of the best way to achieve this.
> 
> Any [other] suggestions?
> 
> Gavin

Where is it documented that the resume/suspend callbacks are not run from a 
thread context?

--HPS