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 | 50 +++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4e967f613e70..479abbe0ba09 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 e7a9666e7cd6..87efea1424d3 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -129,6 +129,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 86e78bbd2e4e..1e38f123ed8c 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -77,12 +77,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); @@ -104,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; } @@ -113,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; @@ -130,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, 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 @@ -198,13 +217,12 @@ static struct usb_port *find_default_peer(struct usb_hub *hub, int port1) static int link_peers(struct usb_port *left, struct usb_port *right) { - struct device *rdev; - struct device *ldev; + struct device *ldev = left->dev.parent->parent; + struct device *rdev = right->dev.parent->parent; int rc; if (left->peer) { right = left->peer; - ldev = left->dev.parent->parent; rdev = right->dev.parent->parent; WARN_ONCE(1, "%s port%d already peered with %s %d\n", @@ -214,7 +232,6 @@ static int link_peers(struct usb_port *left, struct usb_port *right) } else if (right->peer) { left = right->peer; ldev = left->dev.parent->parent; - rdev = right->dev.parent->parent; WARN_ONCE(1, "%s port%d already peered with %s %d\n", dev_name(rdev), right->portnum, dev_name(ldev), @@ -236,6 +253,19 @@ static int link_peers(struct usb_port *left, struct usb_port *right) get_device(&left->dev); 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. + */ + if (hub_is_superspeed(to_usb_device(ldev))) + pm_runtime_get_noresume(&left->dev); + else { + WARN_ON_ONCE(!hub_is_superspeed(to_usb_device(rdev))); + pm_runtime_get_noresume(&right->dev); + } + return 0; } @@ -273,6 +303,18 @@ static void unlink_peers(struct usb_port *left, struct usb_port *right) sysfs_remove_link(&right->dev.kobj, "peer"); put_device(&right->dev); 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 + */ + if (hub_is_superspeed(to_usb_device(ldev))) + pm_runtime_put_noidle(&left->dev); + else { + WARN_ON_ONCE(!hub_is_superspeed(to_usb_device(rdev))); + pm_runtime_put_noidle(&right->dev); + } } /** -- 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