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 properly released. 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: v3: - Changed patch title - Corrected typo - Moved hub_hc_release_resources above mutex_lock(hcd->address0_mutex)
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..dcba4281ea48 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 + * @udev: 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) @@ -6129,6 +6159,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev) bos = udev->bos; udev->bos = NULL;
+ if (udev->reset_resume) + hub_hc_release_resources(udev); + mutex_lock(hcd->address0_mutex);
for (i = 0; i < PORT_INIT_TRIES; ++i) {
On Fri, Feb 28, 2025 at 07:50:25AM +0000, Pawel Laszczak wrote:
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 properly released. 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: v3:
- Changed patch title
- Corrected typo
- Moved hub_hc_release_resources above mutex_lock(hcd->address0_mutex)
v2:
- Replaced disconnection procedure with releasing only the xHC resources
Reviewed-by: Alan Stern stern@rowland.harvard.edu
On 25-02-28 07:50:25, Pawel Laszczak wrote:
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 properly released. 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
Tested at Cixtech sky1 SoC which uses Cadence USB SSP IP.
Tested-by: Peter Chen peter.chen@cixtech.com
Peter
Changelog: v3:
- Changed patch title
- Corrected typo
- Moved hub_hc_release_resources above mutex_lock(hcd->address0_mutex)
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..dcba4281ea48 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
- @udev: 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)
@@ -6129,6 +6159,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev) bos = udev->bos; udev->bos = NULL;
- if (udev->reset_resume)
hub_hc_release_resources(udev);- mutex_lock(hcd->address0_mutex);
for (i = 0; i < PORT_INIT_TRIES; ++i) { -- 2.43.0
On Fri, 28 Feb 2025 07:50:25 +0000, Pawel Laszczak wrote:
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 properly released. 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
Taking discussion about this patch out of bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=220069#c42
As Mathias pointed out, this whole idea is an explicit spec violation, because it puts multiple Device Slots into the Default state.
(Which has nothing to do with actually resetting the devices, by the way. USB core will still do it from the root towards the leaves. It only means the xHC believes that they are reset when they are not.)
A reset-resume of a whole tree looks like a tricky case, because on one hand a hub must resume (here: be reset) before its children in order to reset them, but this apparently causes problems when some xHCs consider the hub "in use" by the children (or its TT in use by their endpoints, I suspect that's the case here and the reason why this patch helps).
I have done some experimentation with reset-resuming from autosuspend, either by causing Transaction Errors while resuming (full speed only) or simulating usb_get_std_status() error in finish_port_resume().
Either way, I noticed that the whole device tree ends up logically disconnected and reconnected during reset-resume, so perhaps it would be acceptable to disable all xHC Device Slots (leaf to root) before resetting everything and re-enable Slots (root to leaf) one by one?
By the way, it's highly unclear if bug 220069 is caused by this spec violation (I think it's not), but this is still very sloppy code.
Regards, Michal
On Fri, 28 Feb 2025 07:50:25 +0000, Pawel Laszczak wrote:
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 properly released. 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
Taking discussion about this patch out of bugzilla https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id=220 069*c42__;Iw!!EHscmS1ygiU1lA!FD7UdYLwKPptb8LI646boayHRFMR7zLGkto3 rhb0whLx1-CVUGaYVVgrG5Y6EyLj-QcTuuUHSpaZcVPaTTRM$
As Mathias pointed out, this whole idea is an explicit spec violation, because it puts multiple Device Slots into the Default state.
(Which has nothing to do with actually resetting the devices, by the way. USB core will still do it from the root towards the leaves. It only means the xHC believes that they are reset when they are not.)
A reset-resume of a whole tree looks like a tricky case, because on one hand a hub must resume (here: be reset) before its children in order to reset them, but this apparently causes problems when some xHCs consider the hub "in use" by the children (or its TT in use by their endpoints, I suspect that's the case here and the reason why this patch helps).
I have done some experimentation with reset-resuming from autosuspend, either by causing Transaction Errors while resuming (full speed only) or simulating usb_get_std_status() error in finish_port_resume().
Either way, I noticed that the whole device tree ends up logically disconnected and reconnected during reset-resume, so perhaps it would be acceptable to disable all xHC Device Slots (leaf to root) before resetting everything and re-enable Slots (root to leaf) one by one?
Are you able recreate this issue with different xHC controllers or only with one specific xHCI? I try to recreate this issue but without result.
Regards, Pawel
By the way, it's highly unclear if bug 220069 is caused by this spec violation (I think it's not), but this is still very sloppy code.
Regards, Michal
On Fri, 28 Feb 2025 07:50:25 +0000, Pawel Laszczak wrote:
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 properly released. 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
Taking discussion about this patch out of bugzilla https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id =220 069*c42__;Iw!!EHscmS1ygiU1lA!FD7UdYLwKPptb8LI646boayHRFMR7zLGkto
3
rhb0whLx1-CVUGaYVVgrG5Y6EyLj-QcTuuUHSpaZcVPaTTRM$
As Mathias pointed out, this whole idea is an explicit spec violation, because it puts multiple Device Slots into the Default state.
(Which has nothing to do with actually resetting the devices, by the way. USB core will still do it from the root towards the leaves. It only means the xHC believes that they are reset when they are not.)
A reset-resume of a whole tree looks like a tricky case, because on one hand a hub must resume (here: be reset) before its children in order to reset them, but this apparently causes problems when some xHCs consider the hub "in use" by the children (or its TT in use by their endpoints, I suspect that's the case here and the reason why this patch helps).
I have done some experimentation with reset-resuming from autosuspend, either by causing Transaction Errors while resuming (full speed only) or simulating usb_get_std_status() error in finish_port_resume().
Either way, I noticed that the whole device tree ends up logically disconnected and reconnected during reset-resume, so perhaps it would be acceptable to disable all xHC Device Slots (leaf to root) before resetting everything and re-enable Slots (root to leaf) one by one?
What about such fix:
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index cfb3abafeacd..46e640679eac 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -6093,8 +6093,8 @@ static void hub_hc_release_resources(struct usb_device *udev) if (hub->ports[i]->child) hub_hc_release_resources(hub->ports[i]->child);
- if (hcd->driver->reset_device) - hcd->driver->reset_device(hcd, udev); + if (hcd->driver->free_dev) + hcd->driver->free_dev(hcd, udev); }
It will free some resource and disable Slot from leaf to root. Later during resuming process one by one Slot will enabled in xhci_discover_or_reset_device function. This solution passed my test, but they are limited.
Pawel
Are you able recreate this issue with different xHC controllers or only with one specific xHCI? I try to recreate this issue but without result.
Regards, Pawel
By the way, it's highly unclear if bug 220069 is caused by this spec violation (I think it's not), but this is still very sloppy code.
Regards, Michal
linux-stable-mirror@lists.linaro.org
-
Alan Stern -
Michał Pecio -
Pawel Laszczak -
Peter Chen