Re: [PATCH 5/9 v2] xHCI: stop device only in U0 state

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

 



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?

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.

(Also, did you mean L1 instead of U1/U2 or does this patchset also
implement USB 3.0 LPM?)

> 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.

Sarah Sharp

> 
> 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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux