From 9f952e26295d977dbfc6fedeaf8c4f112c818d37 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 22 Apr 2020 16:09:51 -0400 Subject: USB: hub: Fix handling of connect changes during sleep Commit 8099f58f1ecd ("USB: hub: Don't record a connect-change event during reset-resume") wasn't very well conceived. The problem it tried to fix was that if a connect-change event occurred while the system was asleep (such as a device disconnecting itself from the bus when it is suspended and then reconnecting when it resumes) requiring a reset-resume during the system wakeup transition, the hub port's change_bit entry would remain set afterward. This would cause the hub driver to believe another connect-change event had occurred after the reset-resume, which was wrong and would lead the driver to send unnecessary requests to the device (which could interfere with a firmware update). The commit tried to fix this by not setting the change_bit during the wakeup. But this was the wrong thing to do; it means that when a device is unplugged while the system is asleep, the hub driver doesn't realize anything has happened: The change_bit flag which would tell it to handle the disconnect event is clear. The commit needs to be reverted and the problem fixed in a different way. Fortunately an alternative solution was noted in the commit's Changelog: We can continue to set the change_bit entry in hub_activate() but then clear it when a reset-resume occurs. That way the the hub driver will see the change_bit when a device is disconnected but won't see it when the device is still present. That's what this patch does. Reported-and-tested-by: Peter Chen Signed-off-by: Alan Stern Fixes: 8099f58f1ecd ("USB: hub: Don't record a connect-change event during reset-resume") Tested-by: Paul Zimmerman CC: Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.2004221602480.11262-100000@iolanthe.rowland.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 54cd8ef795ec..83549f009ced 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1223,6 +1223,11 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) #ifdef CONFIG_PM udev->reset_resume = 1; #endif + /* Don't set the change_bits when the device + * was powered off. + */ + if (test_bit(port1, hub->power_bits)) + set_bit(port1, hub->change_bits); } else { /* The power session is gone; tell hub_wq */ @@ -3088,6 +3093,15 @@ static int check_port_resume_type(struct usb_device *udev, if (portchange & USB_PORT_STAT_C_ENABLE) usb_clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_ENABLE); + + /* + * Whatever made this reset-resume necessary may have + * turned on the port1 bit in hub->change_bits. But after + * a successful reset-resume we want the bit to be clear; + * if it was on it would indicate that something happened + * following the reset-resume. + */ + clear_bit(port1, hub->change_bits); } return status; -- cgit v1.2.1 From 3155f4f40811c5d7e3c686215051acf504e05565 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 22 Apr 2020 16:13:08 -0400 Subject: USB: hub: Revert commit bd0e6c9614b9 ("usb: hub: try old enumeration scheme first for high speed devices") Commit bd0e6c9614b9 ("usb: hub: try old enumeration scheme first for high speed devices") changed the way the hub driver enumerates high-speed devices. Instead of using the "new" enumeration scheme first and switching to the "old" scheme if that doesn't work, we start with the "old" scheme. In theory this is better because the "old" scheme is slightly faster -- it involves resetting the device only once instead of twice. However, for a long time Windows used only the "new" scheme. Zeng Tao said that Windows 8 and later use the "old" scheme for high-speed devices, but apparently there are some devices that don't like it. William Bader reports that the Ricoh webcam built into his Sony Vaio laptop not only doesn't enumerate under the "old" scheme, it gets hung up so badly that it won't then enumerate under the "new" scheme! Only a cold reset will fix it. Therefore we will revert the commit and go back to trying the "new" scheme first for high-speed devices. Reported-and-tested-by: William Bader Ref: https://bugzilla.kernel.org/show_bug.cgi?id=207219 Signed-off-by: Alan Stern Fixes: bd0e6c9614b9 ("usb: hub: try old enumeration scheme first for high speed devices") CC: Zeng Tao CC: Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.2004221611230.11262-100000@iolanthe.rowland.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 83549f009ced..2b6565c06c23 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2728,13 +2728,11 @@ static bool use_new_scheme(struct usb_device *udev, int retry, { int old_scheme_first_port = port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME; - int quick_enumeration = (udev->speed == USB_SPEED_HIGH); if (udev->speed >= USB_SPEED_SUPER) return false; - return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first - || quick_enumeration); + return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first); } /* Is a USB 3.0 port in the Inactive or Compliance Mode state? -- cgit v1.2.1