ClearPortFeature(PORT_POWER) on a usb3 port places the port in either a DSPORT.Powered-off-detect / DSPORT.Powered-off-reset loop, or the DSPORT.Powered-off state. There is no way to ensure that RX terminations will persist in this state, so it is possible a device will degrade to its usb2 connection. Prevent this by blocking power-off of a usb3 port while its usb2 peer is active, and powering on a usb3 port before its usb2 peer.
By default the latency between peer power-on events is 0. In order for the device to not see usb2 active while usb3 is still powering up inject the hub recommended power_on_good delay. In support of satisfying the power_on_good delay outside of hub_power_on() refactor the places where the delay is consumed to call a new hub_power_on_good_delay() helper. Finally, because this introduces several new checks for whether a port is_superspeed, cache that disctinction at port creation so that we don't need to keep looking up the parent hub device. Acked-by: Alan Stern <st...@rowland.harvard.edu> [alan]: add a 'superspeed' flag to the port Signed-off-by: Dan Williams <dan.j.willi...@intel.com> --- drivers/usb/core/hub.c | 22 +++----------- drivers/usb/core/hub.h | 19 ++++++++++++ drivers/usb/core/port.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 17 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 5ab8ce870f5b..a00d786add94 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -36,11 +36,6 @@ #define USB_VENDOR_GENESYS_LOGIC 0x05e3 #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01 -static inline int hub_is_superspeed(struct usb_device *hdev) -{ - return (hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS); -} - /* Protect struct usb_device->state and ->children members * Note: Both are also protected by ->dev.sem, except that ->state can * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ @@ -823,14 +818,9 @@ int usb_hub_clear_tt_buffer(struct urb *urb) } EXPORT_SYMBOL_GPL(usb_hub_clear_tt_buffer); -/* If do_delay is false, return the number of milliseconds the caller - * needs to delay. - */ -static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) +static void hub_power_on(struct usb_hub *hub, bool do_delay) { int port1; - unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2; - unsigned delay; /* Enable power on each port. Some hubs have reserved values * of LPSM (> 2) in their descriptors, even though they are @@ -849,12 +839,8 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) else usb_clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER); - - /* Wait at least 100 msec for power to become stable */ - delay = max(pgood_delay, (unsigned) 100); if (do_delay) - msleep(delay); - return delay; + msleep(hub_power_on_good_delay(hub)); } static int hub_hub_status(struct usb_hub *hub, @@ -1058,7 +1044,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) * for HUB_POST_RESET, but it's easier not to. */ if (type == HUB_INIT) { - delay = hub_power_on(hub, false); + unsigned delay = hub_power_on_good_delay(hub); + + hub_power_on(hub, false); INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); queue_delayed_work(system_power_efficient_wq, &hub->init_work, diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index fabf05ea3de6..5c49fc46178d 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -97,12 +97,18 @@ struct usb_port { u8 portnum; #define USB_PORTDEV_POWER 0 #define USB_PORTDEV_DID_RUNTIME_PUT 1 + #define USB_PORTDEV_SUPERSPEED 2 unsigned long flags; }; #define to_usb_port(_dev) \ container_of(_dev, struct usb_port, dev) +static inline int usb_port_is_superspeed(struct usb_port *port_dev) +{ + return test_bit(USB_PORTDEV_SUPERSPEED, &port_dev->flags); +} + extern int usb_hub_create_port_device(struct usb_hub *hub, int port1); extern void usb_hub_remove_port_device(struct usb_hub *hub, @@ -125,6 +131,19 @@ static inline bool hub_is_port_power_switchable(struct usb_hub *hub) return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM; } +static inline int hub_is_superspeed(struct usb_device *hdev) +{ + return hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS; +} + +static inline unsigned hub_power_on_good_delay(struct usb_hub *hub) +{ + unsigned delay = hub->descriptor->bPwrOn2PwrGood * 2; + + /* Wait at least 100 msec for power to become stable */ + return max(delay, 100U); +} + static inline int hub_port_debounce_be_connected(struct usb_hub *hub, int port1) { diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 99de5acb1240..539555d627f1 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -76,16 +76,25 @@ static int usb_port_runtime_resume(struct device *dev) struct usb_device *hdev = to_usb_device(dev->parent->parent); struct usb_interface *intf = to_usb_interface(dev->parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev->peer; int port1 = port_dev->portnum; int retval; if (!hub) return -EINVAL; + /* + * Power on our usb3 peer before this usb2 port to prevent a usb3 + * device from degrading to its usb2 connection + */ + if (!usb_port_is_superspeed(port_dev) && peer) + pm_runtime_get_sync(&peer->dev); + usb_autopm_get_interface(intf); set_bit(port1, hub->busy_bits); retval = usb_hub_set_port_power(hdev, hub, port1, true); + msleep(hub_power_on_good_delay(hub)); if (port_dev->child && !retval) { /* * Attempt to wait for usb hub port to be reconnected in order @@ -103,6 +112,7 @@ static int usb_port_runtime_resume(struct device *dev) clear_bit(port1, hub->busy_bits); usb_autopm_put_interface(intf); + return retval; } @@ -112,6 +122,7 @@ static int usb_port_runtime_suspend(struct device *dev) struct usb_device *hdev = to_usb_device(dev->parent->parent); struct usb_interface *intf = to_usb_interface(dev->parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_port *peer = port_dev->peer; int port1 = port_dev->portnum; int retval; @@ -129,6 +140,15 @@ static int usb_port_runtime_suspend(struct device *dev) usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub->busy_bits); usb_autopm_put_interface(intf); + + /* + * Our peer usb3 port may now be able to suspend, so + * asynchronously queue a suspend request to observe that this + * usb2 port is now off. + */ + if (!usb_port_is_superspeed(port_dev) && peer) + pm_runtime_put(&peer->dev); + return retval; } #endif @@ -153,6 +173,7 @@ static struct device_driver usb_port_driver = { static int link_peers(struct usb_port *left, struct usb_port *right) { + struct usb_port *ss_port, *hs_port; int rc; if (left->peer == right && right->peer == left) @@ -178,9 +199,36 @@ static int link_peers(struct usb_port *left, struct usb_port *right) return rc; } + /* + * We need to wake the HiSpeed port to make sure we don't race + * setting ->peer with usb_port_runtime_suspend(). Otherwise we + * may miss a suspend event for the SuperSpeed port. + */ + if (usb_port_is_superspeed(left)) { + ss_port = left; + WARN_ON(usb_port_is_superspeed(right)); + hs_port = right; + } else { + ss_port = right; + WARN_ON(!usb_port_is_superspeed(right)); + hs_port = left; + } + pm_runtime_get_sync(&hs_port->dev); + left->peer = right; right->peer = left; + /* + * The SuperSpeed reference is dropped when the HiSpeed port in + * this relationship suspends, i.e. when it is safe to allow a + * SuperSpeed connection to drop since there is no risk of a + * device degrading to its powered-off HiSpeed connection. + * + * Also, drop the HiSpeed ref taken above. + */ + pm_runtime_get_sync(&ss_port->dev); + pm_runtime_put(&hs_port->dev); + return 0; } @@ -200,14 +248,37 @@ static void link_peers_report(struct usb_port *left, struct usb_port *right) static void unlink_peers(struct usb_port *left, struct usb_port *right) { + struct usb_port *ss_port, *hs_port; + WARN(right->peer != left || left->peer != right, "%s and %s are not peers?\n", dev_name(&left->dev), dev_name(&right->dev)); + /* + * We wake the HiSpeed port to make sure we don't race its + * usb_port_runtime_resume() event which takes a SuperSpeed ref + * when ->peer is !NULL. + */ + if (usb_port_is_superspeed(left)) { + ss_port = left; + hs_port = right; + } else { + ss_port = right; + hs_port = left; + } + + pm_runtime_get_sync(&hs_port->dev); + sysfs_remove_link(&left->dev.kobj, "peer"); right->peer = NULL; sysfs_remove_link(&right->dev.kobj, "peer"); left->peer = NULL; + + /* Drop the SuperSpeed ref held on behalf of the active HiSpeed port */ + pm_runtime_put(&ss_port->dev); + + /* Drop the ref taken above */ + pm_runtime_put(&hs_port->dev); } /* @@ -319,6 +390,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) port_dev->dev.groups = port_dev_group; port_dev->dev.type = &usb_port_device_type; port_dev->dev.driver = &usb_port_driver; + if (hub_is_superspeed(hub->hdev)) + set_bit(USB_PORTDEV_SUPERSPEED, &port_dev->flags); dev_set_name(&port_dev->dev, "%s-port%d", dev_name(&hub->hdev->dev), port1); retval = device_register(&port_dev->dev); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html