On Mon, 2011-08-29 at 11:51 -0700, Sarah Sharp wrote:
> On Wed, Aug 17, 2011 at 06:01:39PM +0800, Andiry Xu wrote:
> > When put a port into U3 state, driver needs to stop all the endpoints of
> > the device. However, if a device is in U1/U2 state, it may not respond to
> > stop endpoints command, and it does not need to stop the endpoints.
>
> Why does the host controller you're testing on not stop the endpoint if
> the device is in U1/U2?
>
When enable hardware LPM, the host put the device into U2 state. Then if
I put the system into suspend, xhci hub driver will suspend the port and
stop its endpoints - it will report a message like "timeout while
waiting for stop endpoint command".
> Does it still respond to the stop endpoint ring command if the suspend
> bit is not set when the device is in U1/U2? Otherwise there would be no
> way for software to cancel the interrupt URB for a hub before suspending
> the device. If your host can't stop an endpoint without the suspend bit
> when the device is in U1/U2, I think that's a hardware bug.
>
The host can not stop the endpoints without suspend bit when the device
is in U2 state. But I doubt if it's a hardware bug.
> (Also, did you mean L1 instead of U1/U2 or does this patchset also
> implement USB 3.0 LPM?)
>
No, I mean L1 here.
> > So only stop the endpoints if device is in U0 state.
>
> This doesn't seem right. If the USB core wants to put the device into
> suspend, the xHCI driver should turn off automatic link PM for that
> port, bring the device into U0, and then drive the port into U3. When
> the device resumes, the xHCI driver can re-enable LPM.
>
> Device suspend is always going to be a deeper power savings than either
> LPM state, so we want the device in U3 when the core asks for it to be
> in U3. I think we also want the device to be U3 for the case where
> the system is suspending, and we want to enable remote wakeup from USB
> devices.
>
I don't quite get you. The port is in U2 when system wants to suspend,
and driver wants to put the port into U3. Do you mean driver needs to
resume the port into U0 first and then put it into U3? Why? It can enter
U3 from U2 directly. It just does not respond to stop endpoint command.
And if a device is in U2 state, do we need to stop its endpoints? If any
endpoint is running, it can not be in U2 state.
Thanks,
Andiry
>
> >
> > Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> > ---
> > drivers/usb/host/xhci-hub.c | 8 +++++---
> > 1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> > index aec098a..d25aea1 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -592,9 +592,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> > goto error;
> > }
> > /* unlock to execute stop endpoint commands */
> > - spin_unlock_irqrestore(&xhci->lock, flags);
> > - xhci_stop_device(xhci, slot_id, 1);
> > - spin_lock_irqsave(&xhci->lock, flags);
> > + if ((temp & PORT_PLS_MASK) == XDEV_U0) {
> > + spin_unlock_irqrestore(&xhci->lock, flags);
> > + xhci_stop_device(xhci, slot_id, 1);
> > + spin_lock_irqsave(&xhci->lock, flags);
> > + }
> >
> > xhci_set_link_state(xhci, port_array, wIndex, XDEV_U3);
> >
> > --
> > 1.7.1
> >
> >
>
--
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
