From david-b@pacbell.net Wed Aug 31 11:06:01 2005 From: David Brownell Subject: USB: EHCI port tweaks Date: Wed, 31 Aug 2005 10:55:38 -0700 Cc: Greg KH Message-Id: <200508311055.38522.david-b@pacbell.net> One change may improve some S1 or S3 resume cases, and the other seems mostly to explain some strange state "lsusb" would show. Two fixes: - On resume, don't think about resuming any unpowered port, or resetting any port with OWNER set to the OHCI/UHCI companion. This will make some S1 and S3 resume scenarios work better. - PORT_CSC was not being cleared correctly in ehci_hub_status_data. This was visible at least through current versions of "lsusb", and might have caused some other hub related strangeness. The fix addresses all three write-to-clear bits, using the same approach that UHCI happens to use: a mask of bits that are cleared in most writes to that port status register. Original patch seems to have been from from William.Morrow@amd.com and this version (from David) finishes the write-to-clear changes. Signed-off-by: Jordan Crouse Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/ehci-hcd.c | 8 ++++++-- drivers/usb/host/ehci-hub.c | 27 ++++++++++++++++----------- drivers/usb/host/ehci.h | 1 + 3 files changed, 23 insertions(+), 13 deletions(-) --- gregkh-2.6.orig/drivers/usb/host/ehci-hcd.c 2005-09-12 10:47:36.000000000 -0700 +++ gregkh-2.6/drivers/usb/host/ehci-hcd.c 2005-09-12 11:01:56.000000000 -0700 @@ -759,12 +759,16 @@ static int ehci_resume (struct usb_hcd * if (time_before (jiffies, ehci->next_statechange)) msleep (100); - /* If any port is suspended, we know we can/must resume the HC. */ + /* If any port is suspended (or owned by the companion), + * we know we can/must resume the HC (and mustn't reset it). + */ for (port = HCS_N_PORTS (ehci->hcs_params); port > 0; ) { u32 status; port--; status = readl (&ehci->regs->port_status [port]); - if (status & PORT_SUSPEND) { + if (!(status & PORT_POWER)) + continue; + if (status & (PORT_SUSPEND | PORT_OWNER)) { down (&hcd->self.root_hub->serialize); retval = ehci_hub_resume (hcd); up (&hcd->self.root_hub->serialize); --- gregkh-2.6.orig/drivers/usb/host/ehci.h 2005-09-12 10:47:36.000000000 -0700 +++ gregkh-2.6/drivers/usb/host/ehci.h 2005-09-12 11:01:56.000000000 -0700 @@ -263,6 +263,7 @@ struct ehci_regs { #define PORT_PE (1<<2) /* port enable */ #define PORT_CSC (1<<1) /* connect status change */ #define PORT_CONNECT (1<<0) /* device connected */ +#define PORT_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC) } __attribute__ ((packed)); /* Appendix C, Debug port ... intended for use with special "debug devices" --- gregkh-2.6.orig/drivers/usb/host/ehci-hub.c 2005-08-28 16:41:01.000000000 -0700 +++ gregkh-2.6/drivers/usb/host/ehci-hub.c 2005-09-12 11:01:56.000000000 -0700 @@ -54,7 +54,7 @@ static int ehci_hub_suspend (struct usb_ /* suspend any active/unsuspended ports, maybe allow wakeup */ while (port--) { u32 __iomem *reg = &ehci->regs->port_status [port]; - u32 t1 = readl (reg); + u32 t1 = readl (reg) & ~PORT_RWC_BITS; u32 t2 = t1; if ((t1 & PORT_PE) && !(t1 & PORT_OWNER)) @@ -115,7 +115,8 @@ static int ehci_hub_resume (struct usb_h i = HCS_N_PORTS (ehci->hcs_params); while (i--) { temp = readl (&ehci->regs->port_status [i]); - temp &= ~(PORT_WKOC_E|PORT_WKDISC_E|PORT_WKCONN_E); + temp &= ~(PORT_RWC_BITS + | PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E); if (temp & PORT_SUSPEND) { ehci->reset_done [i] = jiffies + msecs_to_jiffies (20); temp |= PORT_RESUME; @@ -128,7 +129,7 @@ static int ehci_hub_resume (struct usb_h temp = readl (&ehci->regs->port_status [i]); if ((temp & PORT_SUSPEND) == 0) continue; - temp &= ~PORT_RESUME; + temp &= ~(PORT_RWC_BITS | PORT_RESUME); writel (temp, &ehci->regs->port_status [i]); ehci_vdbg (ehci, "resumed port %d\n", i + 1); } @@ -191,6 +192,7 @@ static int check_reset_complete ( // what happens if HCS_N_CC(params) == 0 ? port_status |= PORT_OWNER; + port_status &= ~PORT_RWC_BITS; writel (port_status, &ehci->regs->port_status [index]); } else @@ -233,7 +235,8 @@ ehci_hub_status_data (struct usb_hcd *hc if (temp & PORT_OWNER) { /* don't report this in GetPortStatus */ if (temp & PORT_CSC) { - temp &= ~PORT_CSC; + temp &= ~PORT_RWC_BITS; + temp |= PORT_CSC; writel (temp, &ehci->regs->port_status [i]); } continue; @@ -343,7 +346,7 @@ static int ehci_hub_control ( &ehci->regs->port_status [wIndex]); break; case USB_PORT_FEAT_C_ENABLE: - writel (temp | PORT_PEC, + writel((temp & ~PORT_RWC_BITS) | PORT_PEC, &ehci->regs->port_status [wIndex]); break; case USB_PORT_FEAT_SUSPEND: @@ -353,7 +356,8 @@ static int ehci_hub_control ( if ((temp & PORT_PE) == 0) goto error; /* resume signaling for 20 msec */ - writel ((temp & ~PORT_WAKE_BITS) | PORT_RESUME, + temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS); + writel (temp | PORT_RESUME, &ehci->regs->port_status [wIndex]); ehci->reset_done [wIndex] = jiffies + msecs_to_jiffies (20); @@ -364,15 +368,15 @@ static int ehci_hub_control ( break; case USB_PORT_FEAT_POWER: if (HCS_PPC (ehci->hcs_params)) - writel (temp & ~PORT_POWER, + writel (temp & ~(PORT_RWC_BITS | PORT_POWER), &ehci->regs->port_status [wIndex]); break; case USB_PORT_FEAT_C_CONNECTION: - writel (temp | PORT_CSC, + writel((temp & ~PORT_RWC_BITS) | PORT_CSC, &ehci->regs->port_status [wIndex]); break; case USB_PORT_FEAT_C_OVER_CURRENT: - writel (temp | PORT_OCC, + writel((temp & ~PORT_RWC_BITS) | PORT_OCC, &ehci->regs->port_status [wIndex]); break; case USB_PORT_FEAT_C_RESET: @@ -416,7 +420,7 @@ static int ehci_hub_control ( /* stop resume signaling */ temp = readl (&ehci->regs->port_status [wIndex]); - writel (temp & ~PORT_RESUME, + writel (temp & ~(PORT_RWC_BITS | PORT_RESUME), &ehci->regs->port_status [wIndex]); retval = handshake ( &ehci->regs->port_status [wIndex], @@ -437,7 +441,7 @@ static int ehci_hub_control ( ehci->reset_done [wIndex] = 0; /* force reset to complete */ - writel (temp & ~PORT_RESET, + writel (temp & ~(PORT_RWC_BITS | PORT_RESET), &ehci->regs->port_status [wIndex]); /* REVISIT: some hardware needs 550+ usec to clear * this bit; seems too long to spin routinely... @@ -500,6 +504,7 @@ static int ehci_hub_control ( if (temp & PORT_OWNER) break; + temp &= ~PORT_RWC_BITS; switch (wValue) { case USB_PORT_FEAT_SUSPEND: if ((temp & PORT_PE) == 0