linux-kernel - Re: [PATCH 1/1] dwc2: Delay resume until device connection is stable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2025122354-musket-hydrated-0e13@gregkh>
Date: Tue, 23 Dec 2025 15:29:13 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: sunqian <sunqian@...arytech.com>
Cc: hminas@...opsys.com, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] dwc2: Delay resume until device connection is stable

On Mon, Dec 15, 2025 at 08:53:17PM +0800, sunqian wrote:
> When DWC2 is used as a host with a dynamically controlled VBUS supply,
> VBUS may be turned off during system suspend and enabled again during
> resume. Some USB devices (e.g. USB mass storage) require additional time
> after VBUS is restored to power up and for the D+/D- line state to settle.
> 
> The resume sequence may proceed too early, observe HPRT0.CONNSTS as 0,
> and treat an already-connected device as disconnected. This can lead to
> a spurious USB disconnect followed by re-enumeration, making an
> already-mounted filesystem unusable after resume. Even if the device
> reconnects later, the resume does not behave like a true restoration of
> the pre-suspend device state.
> 
> Poll HPRT0.CONNSTS for a short, bounded period after enabling the
> external VBUS supply, allowing the controller to report a stable
> connection state and prevent spurious disconnects during resume.
> 
> Without this change, resume often results in a disconnect and a new
> device enumeration:
> 
>     dwc2_enable_host_interrupts()
>     ClearPortFeature USB_PORT_FEAT_C_SUSPEND
>     ClearPortFeature USB_PORT_FEAT_ENABLE
>     usb 1-1: USB disconnect, device number 3
>     ...
>     usb 1-1: new high-speed USB device number 4 using dwc2
> 
> With this change applied, the controller reliably detects the device
> after resume and restores the link without triggering a full disconnect
> and re-enumeration cycle:
> 
>     dwc2_enable_host_interrupts()
>     gintsts=05000021  gintmsk=f3000806
>     Device connected after 9 retries
>     ClearPortFeature USB_PORT_FEAT_C_CONNECTION
>     ClearPortFeature USB_PORT_FEAT_C_SUSPEND
>     ...
>     usb 1-1: reset high-speed USB device number 4 using dwc2
> 
> As a side effect, when an OTG host adapter is connected but no USB
> device is present, HPRT0.CONNSTS remains deasserted and the polling
> reaches the timeout. In this case, system resume latency may increase
> by the duration of the bounded wait, which is considered an acceptable
> tradeoff to avoid spurious disconnects and filesystem corruption.
> 
> Tested on:
>   - Kernel: v5.15.140
>   - Suspend mode: suspend-to-RAM (STR)
>   - dr_mode: OTG (dual-role), host mode via OTG adapter
>   - Devices:
>       * USB mass storage (Aigo, Kingston, SanDisk)
>       * USB HID (mouse, keyboard)
> 
> Signed-off-by: sunqian <sunqian@...arytech.com>
> ---
>  drivers/usb/dwc2/hcd.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 60ef8092259a..96345eeb9e2f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4404,11 +4404,15 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
>  	return ret;
>  }
>  
> +#define CONNSTS_POLL_RETRIES       80
> +#define CONNSTS_POLL_DELAY_US_MIN  3000
> +#define CONNSTS_POLL_DELAY_US_MAX  5000
>  static int _dwc2_hcd_resume(struct usb_hcd *hcd)
>  {
>  	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
>  	unsigned long flags;
>  	u32 hprt0;
> +	int retry;
>  	int ret = 0;
>  
>  	spin_lock_irqsave(&hsotg->lock, flags);
> @@ -4501,8 +4505,26 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
>  	dwc2_vbus_supply_init(hsotg);
>  
> -	/* Wait for controller to correctly update D+/D- level */
> -	usleep_range(3000, 5000);
> +	/*
> +	 * Wait for device connection to stabilize after VBUS is restored.
> +	 * Some externally powered devices may need time for D+/D- lines to settle.
> +	 * This runs in the resume path where sleeping is allowed.
> +	 */
> +	for (retry = 0; retry < CONNSTS_POLL_RETRIES; retry++) {
> +		spin_lock_irqsave(&hsotg->lock, flags);
> +		hprt0 = dwc2_read_hprt0(hsotg);
> +		spin_unlock_irqrestore(&hsotg->lock, flags);
> +
> +		if (hprt0 & HPRT0_CONNSTS) {
> +			dev_dbg(hsotg->dev,
> +				"Device connected after %d retries\n", retry);
> +			break;
> +		}
> +
> +		usleep_range(CONNSTS_POLL_DELAY_US_MIN,
> +			     CONNSTS_POLL_DELAY_US_MAX);
> +	}
> +
>  	spin_lock_irqsave(&hsotg->lock, flags);
>  
>  	/*
> -- 
> 2.25.1
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file,
  Documentation/process/submitting-patches.rst for how to do this
  correctly.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot