On Tue, Oct 16, 2012 at 10:14:43AM -0400, Alan Stern wrote: > On Tue, 16 Oct 2012, Peter Chen wrote: > > > Without this condition, all controllers will do this delay, > > and increase the resume time. > > > > Only enabled and unsuspended port needs this delay, but > > Some buggy hardware(like Synopsys usb controller) will > > clear suspend bit once they receive/send resume signal, > > so it takes resume bit as consideration. > > > > Tested it at Freescale i.mx6q Sabrelite board. > > This can be improved a little bit. > > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> > > --- > > drivers/usb/host/ehci-hub.c | 22 +++++++++++++++++----- > > 1 files changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c > > index c788022..6505063 100644 > > --- a/drivers/usb/host/ehci-hub.c > > +++ b/drivers/usb/host/ehci-hub.c > > @@ -384,11 +384,23 @@ static int ehci_bus_resume (struct usb_hcd *hcd) > > ehci_writel(ehci, ehci->command, &ehci->regs->command); > > ehci->rh_state = EHCI_RH_RUNNING; > > > > - /* Some controller/firmware combinations need a delay during which > > - * they set up the port statuses. See Bugzilla #8190. */ > > - spin_unlock_irq(&ehci->lock); > > - msleep(8); > > - spin_lock_irq(&ehci->lock); > > + /* > > + * According to Bugzilla #8190, the port status for some controllers > > + * will be wrong without a delay. At their wrong status, the port > > + * is enabled, but not suspended neither resumed. > > + */ > > + i = HCS_N_PORTS (ehci->hcs_params); > > + while (i--) { > > + temp = ehci_readl(ehci, &ehci->regs->port_status [i]); > > No space before the '['. A lot of place has this problem at this file, besides, there is a space before ')', like "struct ehci_hcd *ehci = hcd_to_ehci (hcd);", Do you need I submit a patch to fix this coding style problem? > > > + if ((temp & PORT_PE) && > > + !(temp & (PORT_SUSPEND | PORT_RESUME))) { > > + ehci_warn(ehci, "Port status(0x%x) is error\n", temp); > > s/is error/is wrong/ > > Also, use ehci_dbg(), not ehci_warn(). This isn't an indication of a > failure so it doesn't need to be a warning. > > > + spin_unlock_irq(&ehci->lock); > > + msleep(8); > > + spin_lock_irq(&ehci->lock); > > Add "break;" here. You don't want to have multiple delays. :-) > > > + } > > + } > > + > > Alan Stern > > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html