...

The xHC resources allocated for USB devices are not released in correct order after resuming in case when while suspend device was

reconnected.

...

This issue has been detected during the fallowing scenario:

• connect hub HS to root port
• connect LS/FS device to hub port
• wait for enumeration to finish
• force host to suspend
• reconnect hub attached to root port
• wake host

For this scenario during enumeration of USB LS/FS device the Cadence xHC reports completion error code for xHC commands because the xHC resources used for devices has not been property released.

s/property/properly/

...

XHCI specification doesn't mention that device can be reset in any order so, we should not treat this issue as Cadence xHC controller bug. Similar as during disconnecting in this case the device resources should be cleared starting form the last usb device in tree toward the root

hub.

...

To fix this issue usbcore driver should call hcd->driver->reset_device for all USB devices connected to hub which was reconnected while suspending.

Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver") cc: stable@vger.kernel.org Signed-off-by: Pawel Laszczak pawell@cadence.com

---

Changelog: v2:

• Replaced disconnection procedure with releasing only the xHC

resources

drivers/usb/core/hub.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index a76bb50b6202..d3f89528a414 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -6065,6 +6065,36 @@ void usb_hub_cleanup(void) usb_deregister(&hub_driver); } /* usb_hub_cleanup() */

+/**

• • hub_hc_release_resources - clear resources used by host controller
• • @pdev: pointer to device being released
• • Context: task context, might sleep
• • Function releases the host controller resources in correct order

+before

• • making any operation on resuming usb device. The host controller

+resources

• • allocated for devices in tree should be released starting from the

+last

• • usb device in tree toward the root hub. This function is used only

+during

• • resuming device when usb device require reinitialization - that

+is, when

• • flag udev->reset_resume is set.
• • This call is synchronous, and may not be used in an interrupt context.
• */

+static void hub_hc_release_resources(struct usb_device *udev) {

• struct usb_hub *hub = usb_hub_to_struct_hub(udev);
• struct usb_hcd *hcd = bus_to_hcd(udev->bus);
• int i;
• /* Release up resources for all children before this device */
• for (i = 0; i < udev->maxchild; i++)

• if (hub->ports[i]->child)
  

• 	hub_hc_release_resources(hub->ports[i]->child);
  

• if (hcd->driver->reset_device)

• hcd->driver->reset_device(hcd, udev); }
  

/**

• usb_reset_and_verify_device - perform a USB port reset to reinitialize a

device

...

• @udev: device to reset (not in SUSPENDED or NOTATTACHED state) @@

-6131,6 +6161,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev)

mutex_lock(hcd->address0_mutex);

• if (udev->reset_resume)

• hub_hc_release_resources(udev);
  

Don't you want to do this before taking the address0_mutex lock?

I will move it.

...

• for (i = 0; i < PORT_INIT_TRIES; ++i) { if (hub_port_stop_enumerate(parent_hub, port1, i)) { ret = -ENODEV;

Doing it this way, you will call hcd->driver->reset_device() multiple times for the same device: once for the hub(s) above the device and then once for the device itself. But since this isn't a hot path, maybe that doesn't matter.

Yes, it true but the function xhci_discover_or_reset_device which is associated with hcd->driver->reset_device() include the checking whether device is in SLOT_STATE_DISABLED. It allows to detect whether device has been already reset and do not repeat the reset procedure.

Thanks Pawel Laszczak

Alan Stern