> -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Saturday, April 16, 2011 12:22 AM > To: Xu, Andiry > Cc: linux-usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxx > Subject: Re: [PATCH] xHCI: Clear PLC in xhci_bus_resume() > > On Fri, Apr 15, 2011 at 01:53:43PM +0800, Andiry Xu wrote: > > This patch clears PORT_PLC if xhci_bus_resume() resumes a previous > suspended > > port, because if a port transition from U3 to U0 state, it will > report a > > port link state change, and software should clear the corresponding > PLC bit. > > > > It also uses hcd->speed to check if a port is a USB2 protocol port. > > Do you mean that you've changed the check to look for if this port can > handle low speed, full speed, or high speed devices? Because it looks > like the previous check only looked for high speed devices, and this > new > check will match all ports that are not SuperSpeed ports (i.e. > LS/FS/HS). > It's USB2 protocol port PMSC register, so I think it's applied to LS/FS/HS devices. > > The patch fixes the issue that USB keyboard can not wakeup system > from > > hibernation. > > > > Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx> > > --- > > drivers/usb/host/xhci-hub.c | 16 ++++++++++++++-- > > 1 files changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci- > hub.c > > index a78f2eb..8647008 100644 > > --- a/drivers/usb/host/xhci-hub.c > > +++ b/drivers/usb/host/xhci-hub.c > > @@ -777,7 +777,7 @@ int xhci_bus_suspend(struct usb_hcd *hcd) > > if (t1 != t2) > > xhci_writel(xhci, t2, port_array[port_index]); > > > > - if (DEV_HIGHSPEED(t1)) { > > + if (hcd->speed != HCD_USB3) { > > /* enable remote wake up for USB 2.0 */ > > u32 __iomem *addr; > > u32 tmp; > > @@ -866,6 +866,18 @@ int xhci_bus_resume(struct usb_hcd *hcd) > > temp |= PORT_LINK_STROBE | XDEV_U0; > > xhci_writel(xhci, temp, port_array[port_index]); > > } > > + spin_unlock_irqrestore(&xhci->lock, flags); > > + msleep(20); > > + spin_lock_irqsave(&xhci->lock, flags); > > What's the purpose of this msleep? Are you waiting for the port link > state to change? Is that time constant, or can it vary on different > host controllers? > Yes, msleep waiting for PLS change. xHCI only specify the suspend time (as long as 10ms), not the resume time. I think 20ms is enough. What's your opinion? > Can you add some documentation about the msleep, preferably as a > comment > in the code? > OK. > > + > > + /* Clear PLC */ > > + temp = xhci_readl(xhci, port_array[port_index]); > > + if (temp & PORT_PLC) { > > + temp = xhci_port_state_to_neutral(temp); > > + temp |= PORT_PLC; > > + xhci_writel(xhci, temp, port_array[port_index]); > > + } > > + > > slot_id = xhci_find_slot_id_by_port(hcd, > > xhci, port_index + 1); > > if (slot_id) > > @@ -873,7 +885,7 @@ int xhci_bus_resume(struct usb_hcd *hcd) > > } else > > xhci_writel(xhci, temp, port_array[port_index]); > > > > - if (DEV_HIGHSPEED(temp)) { > > + if (hcd->speed != HCD_USB3) { > > /* disable remote wake up for USB 2.0 */ > > u32 __iomem *addr; > > u32 tmp; > > Sarah Sharp -- 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
