Re: [PATCH v1 1/2] usb: xhci: Skip xhci_reset in xhci_resume if xhci is being removed

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Hi Mathias, Roy,

On Thu, May 22, 2025, Roy Luo wrote:
> xhci_reset() currently returns -ENODEV if XHCI_STATE_REMOVING is
> set, without completing the xhci handshake, unless the reset completes
> exceptionally quickly. This behavior causes a regression on Synopsys
> DWC3 USB controllers with dual-role capabilities.
> 
> Specifically, when a DWC3 controller exits host mode and removes xhci
> while a reset is still in progress, and then attempts to configure its
> hardware for device mode, the ongoing, incomplete reset leads to
> critical register access issues. All register reads return zero, not
> just within the xHCI register space (which might be expected during a
> reset), but across the entire DWC3 IP block.
> 
> This patch addresses the issue by preventing xhci_reset() from being
> called in xhci_resume() and bailing out early in the reinit flow when
> XHCI_STATE_REMOVING is set.
> 
> Cc: stable@vger.kernel.org
> Fixes: 6ccb83d6c497 ("usb: xhci: Implement xhci_handshake_check_state() helper")
> Suggested-by: Mathias Nyman <mathias.nyman@intel.com>
> Signed-off-by: Roy Luo <royluo@google.com>
> ---
>  drivers/usb/host/xhci.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 90eb491267b5..244b12eafd95 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1084,7 +1084,10 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
>  		xhci_dbg(xhci, "Stop HCD\n");
>  		xhci_halt(xhci);
>  		xhci_zero_64b_regs(xhci);
> -		retval = xhci_reset(xhci, XHCI_RESET_LONG_USEC);
> +		if (xhci->xhc_state & XHCI_STATE_REMOVING)
> +			retval = -ENODEV;
> +		else
> +			retval = xhci_reset(xhci, XHCI_RESET_LONG_USEC);

How can this prevent the xhc_state from changing while in reset? There's
no locking in xhci-plat.

I would suggest to simply revert the commit 6ccb83d6c497 that causes
regression first. We can investigate and look into a solution to the
specific Qcom issue afterward.

Note that this commit may impact role-switching flow for all DRD dwc3
(and perhaps others), which may also impact other Qcom DRD platforms.

Thanks,
Thinh

>  		spin_unlock_irq(&xhci->lock);
>  		if (retval)
>  			return retval;
> -- 
> 2.49.0.1204.g71687c7c1d-goog
>