On Tue, 29 Mar 2011, Andiry Xu wrote:
> In the past, we used USB2.0 request to suspend and resume a USB3.0 device.
> Actually, USB3.0 hub does not support Set/Clear PORT_SUSPEND request,
> instead, it uses Set PORT_LINK_STATE request. This patch makes USB3.0 device
> suspend/resume comply with USB3.0 specification.
>
> This patch fixes the issue that USB3.0 device can not be suspended when
> connected to a USB3.0 external hub.
>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
> drivers/usb/core/hub.c | 29 +++++++++++++++++++-----
> drivers/usb/host/xhci-hub.c | 51 ++++++++++++++++++------------------------
> 2 files changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index bc6f39c..dde4783 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2305,7 +2305,13 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
> }
>
> /* see 7.1.7.6 */
> - status = set_port_feature(hub->hdev, port1, USB_PORT_FEAT_SUSPEND);
> + if (hub_is_superspeed(hub->hdev))
> + status = set_port_feature(hub->hdev,
> + port1 | (USB_SS_PORT_LS_U3 << 3),
> + USB_PORT_FEAT_LINK_STATE);
> + else
> + status = set_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_SUSPEND);
> if (status) {
> dev_dbg(hub->intfdev, "can't suspend port %d, status %d\n",
> port1, status);
> @@ -2457,8 +2463,13 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> set_bit(port1, hub->busy_bits);
>
> /* see 7.1.7.7; affects power usage, but not budgeting */
> - status = clear_port_feature(hub->hdev,
> - port1, USB_PORT_FEAT_SUSPEND);
> + if (hub_is_superspeed(hub->hdev))
> + status = set_port_feature(hub->hdev,
> + port1 | (USB_SS_PORT_LS_U0 << 3),
> + USB_PORT_FEAT_LINK_STATE);
> + else
> + status = clear_port_feature(hub->hdev,
> + port1, USB_PORT_FEAT_SUSPEND);
> if (status) {
> dev_dbg(hub->intfdev, "can't resume port %d, status %d\n",
> port1, status);
> @@ -2480,9 +2491,15 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>
> SuspendCleared:
> if (status == 0) {
> - if (portchange & USB_PORT_STAT_C_SUSPEND)
> - clear_port_feature(hub->hdev, port1,
> - USB_PORT_FEAT_C_SUSPEND);
> + if (hub_is_superspeed(hub->hdev)) {
> + if (portchange & USB_PORT_STAT_C_LINK_STATE)
> + clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_PORT_LINK_STATE);
> + } else {
> + if (portchange & USB_PORT_STAT_C_SUSPEND)
> + clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_SUSPEND);
> + }
> }
>
> clear_bit(port1, hub->busy_bits);
Although I haven't kept a complete set of your patches, it is
noticeable that just about every place hub_is_superspeed() gets called,
the argument is hub->hdev. Things would be simpler if the argument was
hub and the function computed hub->hdev internally.
Of course, this can be done as a separate cleanup patch after
everything else settles down.
Alan Stern
--
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
