Re: [PATCH 3/5] usb: dwc2: hcd: fix possible successive vbus supply enables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: dwc2: regulator leak due to symmetric XOR logic
--

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

usb: dwc2: hcd: fix possible successive vbus supply enables

External VBUS supply may have already been enabled, before entering
dwc2_hcd_reinit. This results in vbus enable count to increase every
time this occurs, and dwc2_vbus_supply_exit won't disable vbus upon
suspend request. Fix this by reading PWR bit before and after
dwc2_hcd_reinit() call.

> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index d5275cd658d4c..b7b4ff466d2b5 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4264,9 +4264,11 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>  		return 0;	/* why 0 ?? */
>  	}
>  
> +	hprt0 = dwc2_read_hprt0(hsotg);
> +
>  	dwc2_hcd_reinit(hsotg);
>  
> -	hprt0 = dwc2_read_hprt0(hsotg);
> +	hprt0 ^= dwc2_read_hprt0(hsotg);
>  	/* Has vbus power been turned on in dwc2_core_host_init ? */
>  	if (hprt0 & HPRT0_PWR) {
>  		/* Enable external vbus supply before resuming root hub */
>  		spin_unlock_irqrestore(&hsotg->lock, flags);
>  		ret = dwc2_vbus_supply_init(hsotg);

[Severity: Medium]
Since this XOR operation is symmetric, does it also evaluate to true if
HPRT0_PWR transitions from 1 to 0?

A 1 to 0 transition seems structurally possible if the hardware
asynchronously clears the power bit (e.g. due to an overcurrent fault
HPRT0_OVRCURRCHG) during the execution window.

If this occurs, could the driver erroneously call dwc2_vbus_supply_init()
when power was lost, improperly incrementing the regulator enable count?

Later cleanup paths like dwc2_hcd_stop() or ClearPortFeature
USB_PORT_FEAT_POWER will observe HPRT0_PWR == 0 and skip calling
dwc2_vbus_supply_exit(), which appears to lead to a permanent regulator
use count leak. The issue is not fixed in subsequent patches in the series.

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