Re: [PATCH v4 6/6] usb: dwc2: platform: fix stm32mp15 detection logic power management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: dwc2: missing forced mode restore in
  dwc2_stm32_hw_enable()
--

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

usb: dwc2: platform: fix stm32mp15 detection logic power management

This commit moves the STM32 ID/VBUS detection enable and disable logic into
dedicated helpers called from the low-level hardware enable/disable paths,
and tracks its state in stm32_id_vb_detection_enabled.

> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 968b4ba6d8141..7aa8d7c58e75f 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -91,6 +91,95 @@ static int dwc2_get_dr_mode(struct dwc2_hsotg *hsotg)
>  	return 0;
>  }
>  
> +static void dwc2_stm32_hw_activate_id_vb_detection(struct dwc2_hsotg *hsotg)
> +{
> +	unsigned long flags;
> +	u32 ggpio, gotgctl;
> +
> +	ggpio = dwc2_readl(hsotg, GGPIO);
> +	ggpio |= GGPIO_STM32_OTG_GCCFG_IDEN;
> +	ggpio |= GGPIO_STM32_OTG_GCCFG_VBDEN;
> +	dwc2_writel(hsotg, ggpio, GGPIO);
> +
> +	/* ID/VBUS detection startup time */
> +	usleep_range(5000, 7000);
> +
> +	spin_lock_irqsave(&hsotg->lock, flags);
> +	/* Remove overrides when resuming from the disable */
> +	gotgctl = dwc2_readl(hsotg, GOTGCTL);
> +	gotgctl &= ~GOTGCTL_DBNCE_FLTR_BYPASS;
> +	gotgctl &= ~(GOTGCTL_BVALOEN | GOTGCTL_AVALOEN |
> +		     GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL);
> +	dwc2_writel(hsotg, gotgctl, GOTGCTL);
> +	spin_unlock_irqrestore(&hsotg->lock, flags);
> +}
> +
> +static int dwc2_stm32_hw_enable(struct dwc2_hsotg *hsotg)
> +{
> +	int ret;
> +
> +	/* params are only known, once dwc2_init_params() has been probed */
> +	if (!hsotg->params.activate_stm_id_vb_detection)
> +		return 0;
> +
> +	if (hsotg->stm32_id_vb_detection_enabled)
> +		return 0;
> +
> +	ret = regulator_enable(hsotg->usb33d);
> +	if (ret)
> +		return ret;
> +
> +	dwc2_stm32_hw_activate_id_vb_detection(hsotg);
> +	hsotg->stm32_id_vb_detection_enabled = true;
> +
> +	return 0;
> +}

[Severity: Medium]
Does dwc2_stm32_hw_enable() leave GUSBCFG_FORCEDEVMODE cleared when a gadget
driver is bound after resuming from standby in peripheral mode?

When the system enters standby in USB_DR_MODE_PERIPHERAL with no gadget
driver bound, ll_hw_enabled is false, so dwc2_resume() returns early without
calling dwc2_force_dr_mode() and the controller registers reset during
standby.  When a gadget driver is later bound, dwc2_hsotg_udc_start() ->
dwc2_lowlevel_hw_enable() -> dwc2_stm32_hw_enable() enables
GGPIO_STM32_OTG_GCCFG_IDEN and clears the GOTGCTL overrides without
restoring GUSBCFG_FORCEDEVMODE.

If the ID pin is low, the OTG controller switches into host mode instead of
staying forced into device mode, breaking peripheral operation and hitting
WARN_ON(host && hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) in
dwc2_force_mode() on the next dwc2_stm32_hw_disable() call.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-dwc2-low-power-fixes-stm32-v4-0-08a8c962a09e@foss.st.com?part=6