Re: [PATCH 4/5] usb: dwc2: hcd: don't disable/enable external vbus by system suspend/resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: dwc2: port power state desync causing VBUS loss
--

commit 54bc47ce99a8e233efd17f79eba5998d68323c1d
Author: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

usb: dwc2: hcd: don't disable/enable external vbus by system suspend/resume

This commit removes the dwc2_vbus_supply_exit and dwc2_vbus_supply_init
calls during system suspend and resume. It aims to prevent incorrectly
increasing the regulator enable count when no device is connected to the
port.

Cc: stable@vger.kernel.org

> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c

[ ... ]

> @@ -4499,10 +4495,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
>  		spin_lock_irqsave(&hsotg->lock, flags);
>  	}
>  
> -	/* Enable external vbus supply after resuming the port. */
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
> -	dwc2_vbus_supply_init(hsotg);
> -
>  	/* Wait for controller to correctly update D+/D- level */
>  	usleep_range(3000, 5000);
>  	spin_lock_irqsave(&hsotg->lock, flags);

[Severity: High]
Does removing the physical regulator initialization here cause a state
desynchronization with the HPRT0_PWR bit?

Later in _dwc2_hcd_resume(), the HPRT0_PWR bit is unconditionally set to 1:

	dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
			HPRT0_ENACHG, HPRT0);

If the port is unpowered prior to this (e.g. at boot before the hub driver
powers it on), HPRT0_PWR becomes 1 without the physical regulator being
turned on.

When the hub driver subsequently tries to power on the port in
dwc2_hcd_hub_control(), it checks the prior state of HPRT0_PWR:

		pwr = hprt0 & HPRT0_PWR;
		hprt0 |= HPRT0_PWR;
		dwc2_writel(hsotg, hprt0, HPRT0);
		if (!pwr)
			dwc2_vbus_supply_init(hsotg);

Since _dwc2_hcd_resume() already forced HPRT0_PWR to 1, the condition (!pwr)
is false and dwc2_vbus_supply_init() is incorrectly skipped.

Could this leave the physical VBUS permanently off and prevent USB device
detection from functioning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-dwc2-low-power-fixes-stm32-v1-0-5b303df762e6@foss.st.com?part=4