The hub status endpoint has a long 'bInterval', which is 255ms for FS/LS device and 256ms for HS device according to USB 2.0 spec, so the device connection change may be reported later more than 255ms via status pipe. The connection change in hub may have been happened already on the downstream ports, but no status URB completes when it is killed in hub_suspend(auto), so the connection change may be missed by some buggy hub devices, which won't generate remote wakeup signal after their remote wakeup is enabled and they are put into suspend state. The problem can be observed at least on the below Genesys Logic, Inc. hub devices: 0x05e3,0x0606 0x05e3,0x0608 In theory, there is no way to fix the problem completely, but we can make it less likely to occur by this patch. This patch introduces one quirk of HUB_QUIRK_CHECK_PORTS_AUTOSUSPEND to check ports' change during hub_suspend(auto) for the buggy devices. If ports' change is found, terminate the auto suspend and return to working state. So for the buggy hubs, if the connection change happend before the ports' check, it can be handled correctly. If it happens between the ports' check and enabling remote wakeup/entering suspend, it will be missed. Considered the interval is quite short, it is very less likely to happen during the window. Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx> --- drivers/usb/core/hub.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 6dc41c6..2a331f9 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -39,6 +39,8 @@ #endif #endif +#define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01 + enum port_power_policy { USB_PORT_POWER_ON = 0, USB_PORT_POWER_OFF, @@ -92,6 +94,8 @@ struct usb_hub { unsigned quiescing:1; unsigned disconnected:1; + unsigned quirk_check_port_auto_suspend:1; + unsigned has_indicators:1; u8 indicator[USB_MAXCHILDREN]; struct delayed_work leds; @@ -1683,6 +1687,9 @@ descriptor_error: if (hdev->speed == USB_SPEED_HIGH) highspeed_hubs++; + if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND) + hub->quirk_check_port_auto_suspend = 1; + if (hub_configure(hub, endpoint) >= 0) return 0; @@ -3158,6 +3165,21 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) #endif +static int check_ports_changed(struct usb_hub *hub) +{ + int port1; + + for (port1 = 1; port1 <= hub->hdev->maxchild; ++port1) { + u16 portstatus, portchange; + int status; + + status = hub_port_status(hub, port1, &portstatus, &portchange); + if (!status && portchange) + return 1; + } + return 0; +} + static int hub_suspend(struct usb_interface *intf, pm_message_t msg) { struct usb_hub *hub = usb_get_intfdata (intf); @@ -3176,6 +3198,16 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg) return -EBUSY; } } + + if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) { + /* check if there are changes pending on hub ports */ + if (check_ports_changed(hub)) { + if (PMSG_IS_AUTO(msg)) + return -EBUSY; + pm_wakeup_event(&hdev->dev, 2000); + } + } + if (hub_is_superspeed(hdev) && hdev->do_remote_wakeup) { /* Enable hub to send remote wakeup for all ports. */ for (port1 = 1; port1 <= hdev->maxchild; port1++) { @@ -4766,6 +4798,11 @@ static const struct attribute_group *port_dev_group[] = { }; static const struct usb_device_id hub_id_table[] = { + { .match_flags = USB_DEVICE_ID_MATCH_VENDOR + | USB_DEVICE_ID_MATCH_INT_CLASS, + .idVendor = 0x05e3, + .bInterfaceClass = USB_CLASS_HUB, + .driver_info = HUB_QUIRK_CHECK_PORT_AUTOSUSPEND}, { .match_flags = USB_DEVICE_ID_MATCH_DEV_CLASS, .bDeviceClass = USB_CLASS_HUB}, { .match_flags = USB_DEVICE_ID_MATCH_INT_CLASS, -- 1.7.9.5 -- 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