RE: [PATCH v3] usb: hub: lack of clearing xHC resources

[Pawel Laszczak]

>
>
>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@xxxxxxxxxxxxxxx>
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>
>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