On stm32mp15, the STM32 detection logic can remain enabled even though the other hardware resources are disabled during probe in peripheral mode: /* Gadget code manages lowlevel hw on its own */ if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) dwc2_lowlevel_hw_disable(hsotg); When entering system low power, the detection logic resources would then remain enabled because dwc2_suspend() checks: if (!dwc2->ll_hw_enabled) return 0; If the stm32mp15 enters standby, the register contents can be lost and not restored, leaving dwc2 unusable. This can happen when no gadget driver has been bound. Move the STM32 detection logic enable/disable procedures into dedicated dwc2_stm32_hw_[enable/disable]() routines and call them from the __dwc2_lowlevel_hw_[enable/disable]() routines. Factorize the probe code by replacing regulator_enable() and GGPIO register access with a call to dwc2_stm32_hw_enable(). The earlier dwc2_lowlevel_hw_enable() call during probe cannot do this because activate_stm_id_vb_detection is only known after dwc2_init_params() runs and the usb33d supply is acquired. Track VBUS/ID detection state in stm32_id_vb_detection_enabled so it is disabled only after successful enablement. Fixes: 2b94b054ac49 ("usb: dwc2: fix hang during suspend if set as peripheral") Fixes: a415083a11cc ("usb: dwc2: add support for STM32MP15 SoCs USB OTG HS and FS") Cc: stable@vger.kernel.org Signed-off-by: Fabrice Gasnier --- Changes in v4: - Track successful STM32 ID/VBUS detection enablement with stm32_id_vb_detection_enabled, avoiding an invalid regulator disable during probe error cleanup, as reported by Sashiko. Changes in v3: - Roll back if regulator_disable() fails in dwc2_stm32_hw_disable(), as reported by Sashiko. - Add a note in response to Sashiko's v2 review. Changes in v2: - Fix the probe error path if devm_regulator_get() or dwc2_stm32_hw_enable() fails, as reported by Sashiko. - Describe the hardware-disable error handling in the commit message. Notes: - If dwc2_stm32_hw_disable() fails in __dwc2_lowlevel_hw_disable(), there is nothing to roll back, so return immediately. Alternatively, the error could be ignored to continue disabling other resources. Similar behavior already exists for phy shutdown/exit/power_off errors; this could be addressed separately. - In host mode with remote wakeup enabled and a descendant enabled to wake the system, phy_off_for_suspend is false. The STM32 power domain is not expected to turn off, so registers should not be lost. --- drivers/usb/dwc2/core.h | 2 + drivers/usb/dwc2/platform.c | 185 ++++++++++++++++++++++++++------------------ 2 files changed, 112 insertions(+), 75 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index c97c958b19a5..7de080507b5a 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -875,6 +875,7 @@ struct dwc2_hregs_backup { * @reset_phy_on_wake: Quirk saying that we should assert PHY reset on a * remote wakeup. * @phy_off_for_suspend: Status of whether we turned the PHY off at suspend. + * @stm32_id_vb_detection_enabled: True if STM32 ID/VBUS detection is enabled. * @need_phy_for_wake: Quirk saying that we should keep the PHY on at * suspend if we need USB to wake us up. * @frame_number: Frame number read from the core. For both device @@ -1076,6 +1077,7 @@ struct dwc2_hsotg { unsigned int reset_phy_on_wake:1; unsigned int need_phy_for_wake:1; unsigned int phy_off_for_suspend:1; + bool stm32_id_vb_detection_enabled; u16 frame_number; struct phy *phy; diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index 968b4ba6d814..7aa8d7c58e75 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; +} + +static int dwc2_stm32_hw_disable(struct dwc2_hsotg *hsotg) +{ + unsigned long flags; + u32 ggpio, gotgctl; + int ret; + + if (!hsotg->stm32_id_vb_detection_enabled) + return 0; + + /* + * Need to force the mode to the current mode to avoid Mode + * Mismatch Interrupt when ID detection will be disabled. + */ + dwc2_force_mode(hsotg, !dwc2_is_device_mode(hsotg)); + + spin_lock_irqsave(&hsotg->lock, flags); + gotgctl = dwc2_readl(hsotg, GOTGCTL); + /* bypass debounce filter, enable overrides */ + gotgctl |= GOTGCTL_DBNCE_FLTR_BYPASS; + gotgctl |= GOTGCTL_BVALOEN | GOTGCTL_AVALOEN; + /* Force A / B session if needed */ + if (gotgctl & GOTGCTL_ASESVLD) + gotgctl |= GOTGCTL_AVALOVAL; + if (gotgctl & GOTGCTL_BSESVLD) + gotgctl |= GOTGCTL_BVALOVAL; + dwc2_writel(hsotg, gotgctl, GOTGCTL); + spin_unlock_irqrestore(&hsotg->lock, flags); + + ggpio = dwc2_readl(hsotg, GGPIO); + ggpio &= ~GGPIO_STM32_OTG_GCCFG_IDEN; + ggpio &= ~GGPIO_STM32_OTG_GCCFG_VBDEN; + dwc2_writel(hsotg, ggpio, GGPIO); + + ret = regulator_disable(hsotg->usb33d); + if (ret) { + dwc2_stm32_hw_activate_id_vb_detection(hsotg); + dwc2_force_dr_mode(hsotg); + return ret; + } + + hsotg->stm32_id_vb_detection_enabled = false; + + return 0; +} + static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg) { struct platform_device *pdev = to_platform_device(hsotg->dev); @@ -129,8 +218,22 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg *hsotg) if (ret) goto err_dis_clk; + ret = dwc2_stm32_hw_enable(hsotg); + if (ret) + goto err_dis_phy; + return 0; +err_dis_phy: + if (hsotg->uphy) { + usb_phy_shutdown(hsotg->uphy); + } else if (hsotg->plat && hsotg->plat->phy_exit) { + hsotg->plat->phy_exit(pdev, hsotg->plat->phy_type); + } else { + phy_power_off(hsotg->phy); + phy_exit(hsotg->phy); + } + err_dis_clk: if (hsotg->clk) clk_disable_unprepare(hsotg->clk); @@ -166,6 +269,10 @@ static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg *hsotg) struct platform_device *pdev = to_platform_device(hsotg->dev); int ret = 0; + ret = dwc2_stm32_hw_disable(hsotg); + if (ret) + return ret; + if (hsotg->uphy) { usb_phy_shutdown(hsotg->uphy); } else if (hsotg->plat && hsotg->plat->phy_exit) { @@ -346,9 +453,6 @@ static void dwc2_driver_remove(struct platform_device *dev) dwc2_drd_exit(hsotg); - if (hsotg->params.activate_stm_id_vb_detection) - regulator_disable(hsotg->usb33d); - if (hsotg->ll_hw_enabled) dwc2_lowlevel_hw_disable(hsotg); } @@ -542,33 +646,23 @@ static int dwc2_driver_probe(struct platform_device *dev) goto error; if (hsotg->params.activate_stm_id_vb_detection) { - u32 ggpio; - hsotg->usb33d = devm_regulator_get(hsotg->dev, "usb33d"); if (IS_ERR(hsotg->usb33d)) { retval = PTR_ERR(hsotg->usb33d); dev_err_probe(hsotg->dev, retval, "failed to request usb33d supply\n"); goto error; } - retval = regulator_enable(hsotg->usb33d); + retval = dwc2_stm32_hw_enable(hsotg); if (retval) { - dev_err_probe(hsotg->dev, retval, "failed to enable usb33d supply\n"); + dev_err_probe(hsotg->dev, retval, "dwc2_stm32_hw_enable failed\n"); goto error; } - - 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); } retval = dwc2_drd_init(hsotg); if (retval) { dev_err_probe(hsotg->dev, retval, "failed to initialize dual-role\n"); - goto error_init; + goto error; } if (hsotg->dr_mode != USB_DR_MODE_HOST) { @@ -638,9 +732,6 @@ static int dwc2_driver_probe(struct platform_device *dev) error_drd: dwc2_drd_exit(hsotg); -error_init: - if (hsotg->params.activate_stm_id_vb_detection) - regulator_disable(hsotg->usb33d); error: if (hsotg->ll_hw_enabled) dwc2_lowlevel_hw_disable(hsotg); @@ -662,37 +753,6 @@ static int __maybe_unused dwc2_suspend(struct device *dev) dwc2_drd_suspend(dwc2); - if (dwc2->params.activate_stm_id_vb_detection) { - unsigned long flags; - u32 ggpio, gotgctl; - - /* - * Need to force the mode to the current mode to avoid Mode - * Mismatch Interrupt when ID detection will be disabled. - */ - dwc2_force_mode(dwc2, !is_device_mode); - - spin_lock_irqsave(&dwc2->lock, flags); - gotgctl = dwc2_readl(dwc2, GOTGCTL); - /* bypass debounce filter, enable overrides */ - gotgctl |= GOTGCTL_DBNCE_FLTR_BYPASS; - gotgctl |= GOTGCTL_BVALOEN | GOTGCTL_AVALOEN; - /* Force A / B session if needed */ - if (gotgctl & GOTGCTL_ASESVLD) - gotgctl |= GOTGCTL_AVALOVAL; - if (gotgctl & GOTGCTL_BSESVLD) - gotgctl |= GOTGCTL_BVALOVAL; - dwc2_writel(dwc2, gotgctl, GOTGCTL); - spin_unlock_irqrestore(&dwc2->lock, flags); - - ggpio = dwc2_readl(dwc2, GGPIO); - ggpio &= ~GGPIO_STM32_OTG_GCCFG_IDEN; - ggpio &= ~GGPIO_STM32_OTG_GCCFG_VBDEN; - dwc2_writel(dwc2, ggpio, GGPIO); - - regulator_disable(dwc2->usb33d); - } - if (dwc2->ll_hw_enabled && (is_device_mode || dwc2_host_can_poweroff_phy(dwc2))) { ret = __dwc2_lowlevel_hw_disable(dwc2); @@ -718,31 +778,6 @@ static int __maybe_unused dwc2_resume(struct device *dev) } dwc2->phy_off_for_suspend = false; - if (dwc2->params.activate_stm_id_vb_detection) { - unsigned long flags; - u32 ggpio, gotgctl; - - ret = regulator_enable(dwc2->usb33d); - if (ret) - return ret; - - ggpio = dwc2_readl(dwc2, GGPIO); - ggpio |= GGPIO_STM32_OTG_GCCFG_IDEN; - ggpio |= GGPIO_STM32_OTG_GCCFG_VBDEN; - dwc2_writel(dwc2, ggpio, GGPIO); - - /* ID/VBUS detection startup time */ - usleep_range(5000, 7000); - - spin_lock_irqsave(&dwc2->lock, flags); - gotgctl = dwc2_readl(dwc2, GOTGCTL); - gotgctl &= ~GOTGCTL_DBNCE_FLTR_BYPASS; - gotgctl &= ~(GOTGCTL_BVALOEN | GOTGCTL_AVALOEN | - GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL); - dwc2_writel(dwc2, gotgctl, GOTGCTL); - spin_unlock_irqrestore(&dwc2->lock, flags); - } - if (!dwc2->role_sw) { /* Need to restore FORCEDEVMODE/FORCEHOSTMODE */ dwc2_force_dr_mode(dwc2); -- 2.43.0