If a port is powered-off, or in the process of being powered-off, prevent
khubd from operating on it. Otherwise, the following sequence of events
leading to an unintended disconnect may occur:
Events:
(0) <set pm_qos_no_poweroff to '0' for port1>
(1) hub 2-2:1.0: hub_resume
(2) hub 2-2:1.0: port 1: status 0301 change 0000
(3) hub 2-2:1.0: state 7 ports 4 chg 0002 evt 0000
(4) hub 2-2:1.0: port 1, power off status 0000, change 0000, 12 Mb/s
(5) usb 2-2.1: USB disconnect, device number 5
Description:
(1) hub is resumed before sending a ClearPortFeature request
(2) hub_activate() notices the port is connected and sets
hub->change_bits for the port
(3) hub_events() starts, but at the same time the port suspends
(4) hub_connect_change() sees the disabled port and triggers disconnect
Most of the code thrash here is just indenting the portions of
port_event() that require the port to be runtime pm active.
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/usb/core/hub.c | 91 +++++++++++++++++++++++++++---------------------
1 files changed, 51 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e0c593ea7a0e..10f420626204 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4761,51 +4761,56 @@ static void port_event(struct usb_hub *hub, int port1)
USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
}
- if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
- connect_change = 1;
-
- /*
- * Warm reset a USB3 protocol port if it's in
- * SS.Inactive state.
- */
- if (hub_port_warm_reset_required(hub, portstatus)) {
- int status;
+ /* take port actions that require the port to be powered on */
+ if (pm_runtime_active(&port_dev->dev)) {
+ if (hub_handle_remote_wakeup(hub, port1,
+ portstatus, portchange))
+ connect_change = 1;
- dev_dbg(&port_dev->dev, "do warm reset\n");
- if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
- || udev->state == USB_STATE_NOTATTACHED) {
- status = hub_port_reset(hub, port1, NULL,
- HUB_BH_RESET_TIME, true);
- if (status < 0)
- hub_port_disable(hub, port1, 1);
- } else {
+ /*
+ * Warm reset a USB3 protocol port if it's in
+ * SS.Inactive state.
+ */
+ if (hub_port_warm_reset_required(hub, portstatus)) {
+ int status;
+
+ dev_dbg(&port_dev->dev, "do warm reset\n");
+ if (!udev || !(portstatus & USB_PORT_STAT_CONNECTION)
+ || udev->state == USB_STATE_NOTATTACHED) {
+ status = hub_port_reset(hub, port1, NULL,
+ HUB_BH_RESET_TIME, true);
+ if (status < 0)
+ hub_port_disable(hub, port1, 1);
+ } else {
+ usb_lock_device(udev);
+ status = usb_reset_device(udev);
+ usb_unlock_device(udev);
+ connect_change = 0;
+ }
+ /*
+ * On disconnect USB3 protocol ports transit from U0 to
+ * SS.Inactive to Rx.Detect. If this happens a warm-
+ * reset is not needed, but a (re)connect may happen
+ * before khubd runs and sees the disconnect, and the
+ * device may be an unknown state.
+ *
+ * If the port went through SS.Inactive without khubd
+ * seeing it the C_LINK_STATE change flag will be set,
+ * and we reset the dev to put it in a known state.
+ */
+ } else if (udev && hub_is_superspeed(hub->hdev) &&
+ (portchange & USB_PORT_STAT_C_LINK_STATE) &&
+ (portstatus & USB_PORT_STAT_CONNECTION)) {
usb_lock_device(udev);
- status = usb_reset_device(udev);
+ usb_reset_device(udev);
usb_unlock_device(udev);
connect_change = 0;
}
- /*
- * On disconnect USB3 protocol ports transit from U0 to
- * SS.Inactive to Rx.Detect. If this happens a warm-
- * reset is not needed, but a (re)connect may happen
- * before khubd runs and sees the disconnect, and the
- * device may be an unknown state.
- *
- * If the port went through SS.Inactive without khubd
- * seeing it the C_LINK_STATE change flag will be set,
- * and we reset the dev to put it in a known state.
- */
- } else if (udev && hub_is_superspeed(hub->hdev) &&
- (portchange & USB_PORT_STAT_C_LINK_STATE) &&
- (portstatus & USB_PORT_STAT_CONNECTION)) {
- usb_lock_device(udev);
- usb_reset_device(udev);
- usb_unlock_device(udev);
- connect_change = 0;
- }
- if (connect_change)
- hub_port_connect_change(hub, port1, portstatus, portchange);
+ if (connect_change)
+ hub_port_connect_change(hub, port1, portstatus,
+ portchange);
+ }
}
@@ -4892,11 +4897,17 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i <= hdev->maxchild; i++) {
+ struct usb_port *port_dev = hub->ports[i - 1];
+
if (!test_bit(i, hub->busy_bits)
&& (test_and_clear_bit(i, hub->event_bits)
|| test_bit(i, hub->change_bits)
- || test_bit(i, hub->wakeup_bits)))
+ || test_bit(i, hub->wakeup_bits))) {
+ pm_runtime_get_noresume(&port_dev->dev);
+ pm_runtime_barrier(&port_dev->dev);
port_event(hub, i);
+ pm_runtime_put_sync(&port_dev->dev);
+ }
}
/* deal with hub status changes */
--
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
