On Fri, 25 Mar 2011, Andiry Xu wrote:
> USB3.0 specification has different wPortStatus and wPortChange definitions
> from USB2.0 specification. Since USB3 root hub and USB2 root hub are split
> now and USB3 hub only has USB3 protocol ports, we should modify the
> portstatus and portchange report of USB3 ports to comply with USB3.0
> specification.
...
> @@ -2171,16 +2163,30 @@ static int check_port_resume_type(struct usb_device *udev,
> struct usb_hub *hub, int port1,
> int status, unsigned portchange, unsigned portstatus)
> {
> + bool lost = false;
> +
> /* Is the device still present? */
> - if (status || (portstatus & MASK_BITS) != WANT_BITS) {
> - if (status >= 0)
> - status = -ENODEV;
> + if (hub_is_superspeed(hub->hdev)) {
> + if (status || (portstatus & MASK_SS_BITS) != MASK_SS_BITS ||
> + (portstatus & USB_PORT_STAT_LINK_STATE)
> + == USB_SS_PORT_LS_U3) {
> + lost = true;
> + if (status >= 0)
> + status = -ENODEV;
> + }
> + } else {
> + if (status || (portstatus & MASK_BITS) != WANT_BITS) {
> + lost = true;
> + if (status >= 0)
> + status = -ENODEV;
> + }
> }
You could factor out some common code:
if (lost && status >= 0)
status = -ENODEV;
> /* Can't do a normal resume if the port isn't enabled,
> * so try a reset-resume instead.
> */
> - else if (!(portstatus & USB_PORT_STAT_ENABLE) && !udev->reset_resume) {
> + if (lost && !(portstatus & USB_PORT_STAT_ENABLE) &&
This should be !lost.
> + !udev->reset_resume) {
This line should be indented by another tab stop.
> if (udev->persist_enabled)
> udev->reset_resume = 1;
> else
> @@ -2427,8 +2433,14 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
>
> /* Skip the initial Clear-Suspend step for a remote wakeup */
> status = hub_port_status(hub, port1, &portstatus, &portchange);
> - if (status == 0 && !(portstatus & USB_PORT_STAT_SUSPEND))
> - goto SuspendCleared;
> + if (hub_is_superspeed(hub->hdev)) {
> + if (status == 0 && ((portstatus & USB_PORT_STAT_LINK_STATE)
> + != USB_SS_PORT_LS_U3))
> + goto SuspendCleared;
> + } else {
> + if (status == 0 && !(portstatus & USB_PORT_STAT_SUSPEND))
> + goto SuspendCleared;
> + }
The port-suspend-status test occurs in more than one place. It would
be clearer to encapsulate this in a separate function.
> // dev_dbg(hub->intfdev, "resume port %d\n", port1);
>
> @@ -3139,8 +3151,12 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
>
> /* maybe switch power back on (e.g. root hub was reset) */
> if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2
> - && !(portstatus & USB_PORT_STAT_POWER))
> + && ((hub_is_superspeed(hub->hdev) &&
> + !(portstatus & USB_SS_PORT_STAT_POWER))
> + || (!hub_is_superspeed(hub->hdev) &&
> + !(portstatus & USB_PORT_STAT_POWER)))) {
Even though the port-power-status test occurs only once, it also might
be clearer if it were encapsulated in a separate function.
> set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> + }
>
> if (portstatus & USB_PORT_STAT_ENABLE)
> goto done;
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
