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. Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/usb/core/hub.c | 5 --- drivers/usb/core/hub.h | 5 +++ drivers/usb/core/port.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c463c09c0d37..1c9ad4ff5c3c 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. */ diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 6858a55eceb5..45959767db8f 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -125,6 +125,11 @@ 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 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 783a3bc6c4c9..d06a64b16ac1 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -79,12 +79,20 @@ 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 (!hub_is_superspeed(hdev) && peer) + pm_runtime_get_sync(&peer->dev); + usb_autopm_get_interface(intf); set_bit(port1, hub->busy_bits); @@ -106,6 +114,7 @@ static int usb_port_runtime_resume(struct device *dev) clear_bit(port1, hub->busy_bits); usb_autopm_put_interface(intf); + return retval; } @@ -115,6 +124,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; @@ -132,6 +142,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, asynchronously + * queue a suspend request to observe that this usb2 peer port + * is now off. + */ + if (!hub_is_superspeed(hdev) && peer) + pm_runtime_put(&peer->dev); + return retval; } #endif @@ -219,8 +238,26 @@ static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) return peer; } +/* + * Modifying ->peer affects usb_port_runtime_{suspend|resume} so make + * sure devices are active before the change and re-evaluate + * afterwards + */ +static void pre_modify_peers(struct usb_port *left, struct usb_port *right) +{ + pm_runtime_get_sync(&left->dev); + pm_runtime_get_sync(&right->dev); +} + +static void post_modify_peers(struct usb_port *left, struct usb_port *right) +{ + pm_runtime_put(&left->dev); + pm_runtime_put(&right->dev); +} + static int link_peers(struct usb_port *left, struct usb_port *right) { + struct usb_device *ldev, *rdev; int rc; if (left->peer == right && right->peer == left) @@ -246,9 +283,26 @@ static int link_peers(struct usb_port *left, struct usb_port *right) return rc; } + pre_modify_peers(left, right); left->peer = right; right->peer = left; + /* + * Ports are peer linked, hold a reference on the superspeed + * port which the hispeed port drops when it suspends. This + * ensures that superspeed ports only suspend after their + * hispeed peer. + */ + ldev = to_usb_device(left->dev.parent->parent); + rdev = to_usb_device(right->dev.parent->parent); + if (hub_is_superspeed(ldev)) + pm_runtime_get_noresume(&left->dev); + else { + WARN_ON(!hub_is_superspeed(rdev)); + pm_runtime_get_noresume(&right->dev); + } + post_modify_peers(left, right); + return 0; } @@ -268,14 +322,32 @@ 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_device *ldev, *rdev; + WARN(right->peer != left || left->peer != right, "%s and %s are not peers?\n", dev_name(&left->dev), dev_name(&right->dev)); + pre_modify_peers(left, right); sysfs_remove_link(&left->dev.kobj, "peer"); right->peer = NULL; sysfs_remove_link(&right->dev.kobj, "peer"); left->peer = NULL; + + /* + * Ports are no longer peer linked, drop the reference that + * keeps the superspeed port (may be 'right' or 'left') powered + * when its peer is active + */ + ldev = to_usb_device(left->dev.parent->parent); + rdev = to_usb_device(right->dev.parent->parent); + if (hub_is_superspeed(ldev)) + pm_runtime_put_noidle(&left->dev); + else { + WARN_ON(!hub_is_superspeed(rdev)); + pm_runtime_put_noidle(&right->dev); + } + post_modify_peers(left, right); } /** -- 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