[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [linux-usb-devel] [PATCH] Re-power USB ports on wakeup



On Tuesday 15 March 2005 8:34 am, Colin Leroy wrote:
> Hi list,
> 
> Most of us have problems with USB unresponsive after one out of two
> wakeups. This patch fixes it for me; it looks like the port status
> isn't automatically updated (to ~PORT_POWER) when sleeping...
> 
> So this patch just unpowers the ports manually at the end of ehci_suspend.
> I don't know if it's really correct: Ben & Greg, I'd like your comments. if
> it's fine, can you push it upstream?

Well, neither Ben nor Greg maintain that driver, so I'll stick my
nose in instead.

This patch is self-evidently incorrect:  by definition, a suspended
port maintains VBUS power.  Otherwise the device connected to the
port will get disconnected, rather than suspended ... and it won't
be able to issue wakeup events.  All suspending does is stop sending
USB traffic down the port, notably SOF tokens; the device connected
to the port then enters a low power mode, typically consuming at most
500uA from VBUS (vs typically 100mA when it's active/configured).

There's something else going on here.  Please provide a more complete
problem description to linux-usb-devel.  For example, compile with
CONFIG_USB_DEBUG and provide the /sys/class/usb_host/usbN/registers
file on initial boot, then after a resume that worked, then after
one that didn't.

Is this S1, S2, or S3 suspend?  What are the relevant BIOS settings
for USB?  (BIOS modes are particularly relevant with "swsusp" paths,
which are actually reboot paths not suspend paths.)  Is this witth
CONFIG_USB_SUSPEND or not?

- Dave



> Signed-off-by: Colin Leroy <colin@colino.net>
> --- a/drivers/usb/host/ehci-hcd.c	2005-03-15 17:28:52.000000000 +0100
> +++ b/drivers/usb/host/ehci-hcd.c	2005-03-15 17:29:52.000000000 +0100
> @@ -701,6 +701,7 @@
>  static int ehci_suspend (struct usb_hcd *hcd, u32 state)
>  {
>  	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
> +	unsigned port;
>  
>  	if (time_before (jiffies, ehci->next_statechange))
>  		msleep (100);
> @@ -716,6 +717,16 @@
>  	// save (PCI) FLADJ in case of Vaux power loss
>  	// ... we'd only use it to handle clock skew
>  
> +	/* Suspend all ports manually (to get them powered at resume) */
> +	for (port = HCS_N_PORTS (ehci->hcs_params); port > 0; ) {
> +		u32	status;
> +		port--;
> +		status = readl (&ehci->regs->port_status [port]);
> +		writel (status & ~PORT_POWER,
> +					&ehci->regs->port_status [port]);
> +		ehci_dbg(ehci, "port %d unpowered\n", port);
> +	}
> +
>  	return 0;
>  }
>  
> 
> 



Reply to: