[PATCH] phy: exynos5-usbdrd: fix HS phy init count imbalance on Exynos2200

[PATCH] phy: exynos5-usbdrd: fix HS phy init count imbalance on Exynos2200

[Soheyb]

exynos2200_usbdrd_utmi_init() calls phy_init() on the high-speed phy,
but exynos2200_usbdrd_phy_exit() never calls phy_exit() on it. The init
count of the high-speed phy therefore never drops back to zero: after
the first init, every later phy_init() of it returns early, and its
clocks and supplies stay enabled across an exit of the UTMI phy.

When the high-speed phy loses power while the controller is suspended,
it is thus not set up again on resume, and the DWC3 core soft reset
times out:

  dwc3 11210000.usb: DWC3 controller soft reset failed

Exit the high-speed phy when the UTMI phy exits, first, in the reverse
order of the init.

Fixes: cc52a697f87e ("phy: exynos5-usbdrd: support Exynos USBDRD 3.2 4nm controller")
Cc: stable@vger.kernel.org
Signed-off-by: Soheyb <soheyb_boulaa@proton.me>
---
Tested on a Google Pixel 9 (Tensor G4) with an out-of-tree port that
uses the Exynos2200 phy ops and powers the USB block off in system
sleep: without this change the soft reset failed on the first resume;
with it, USB came back after each of two system sleeps. Not tested on
Exynos2200 hardware.

 drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 087d552..0bee928 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -1486,6 +1486,13 @@ static int exynos2200_usbdrd_phy_exit(struct phy *phy)
 	u32 reg;
 	int ret;
 
+	/*
+	 * The UTMI init initializes the high-speed phy; exit it in turn, so
+	 * that the next init sets it up again after it lost power.
+	 */
+	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI)
+		phy_exit(phy_drd->hs_phy);
+
 	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
 	if (ret)
 		return ret;
-- 
2.56.0



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: exynos5-usbdrd: fix HS phy init count imbalance on Exynos2200

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] phy: exynos5-usbdrd: init_count imbalance in
  exynos2200_usbdrd_phy_exit()
- [Medium] phy: exynos5-usbdrd: unchecked phy_init() in
  exynos2200_usbdrd_utmi_init()
--

commit 7267261ad8128548bbc38d39a400e9086f676fce
Author: Soheyb <soheyb_boulaa@proton.me>

phy: exynos5-usbdrd: fix HS phy init count imbalance on Exynos2200

This commit calls phy_exit() on phy_drd->hs_phy when exiting the UTMI PHY
in exynos2200_usbdrd_phy_exit() so that the high-speed PHY init count returns
to zero and the PHY is re-initialized on resume.

> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 087d552eb5119..0bee92844245d 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -1486,6 +1486,13 @@ static int exynos2200_usbdrd_phy_exit(struct phy *phy)
>  	u32 reg;
>  	int ret;
>  
> +	/*
> +	 * The UTMI init initializes the high-speed phy; exit it in turn, so
> +	 * that the next init sets it up again after it lost power.
> +	 */
> +	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI)
> +		phy_exit(phy_drd->hs_phy);

[Severity: Medium]
Can this underflow phy_drd->hs_phy->init_count to -1 if the earlier
phy_init(phy_drd->hs_phy) failed?

exynos2200_usbdrd_utmi_init() ignores the return value of phy_init():

drivers/phy/samsung/phy-exynos5-usbdrd.c:exynos2200_usbdrd_utmi_init() {
	/* Configure non-Samsung IP PHY, responsible for UTMI */
	phy_init(phy_drd->hs_phy);

	return 0;
}

When phy_init(phy_drd->hs_phy) fails, hs_phy->init_count stays at 0 while
the outer UTMI PHY init succeeds and increments its init_count to 1.
Calling phy_exit() on the outer UTMI PHY then calls
phy_exit(phy_drd->hs_phy) with hs_phy->init_count == 0, which skips
ops->exit() and decrements hs_phy->init_count to -1, preventing future
phy_init(phy_drd->hs_phy) calls from ever running ops->init().

exynos2200_usbdrd_utmi_init() should return the result of
phy_init(phy_drd->hs_phy).

> +
>  	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
>  	if (ret)
>  		return ret;

[Severity: Medium]
Does calling phy_exit(phy_drd->hs_phy) before clk_bulk_prepare_enable()
and ignoring its return value leave the outer UTMI PHY and phy_drd->hs_phy
init_count values out of sync on error?

If clk_bulk_prepare_enable() fails, phy_drd->hs_phy has already been
exited (hs_phy->init_count == 0), while phy_exit() on the outer UTMI PHY
sees the error from ops->exit() and leaves the outer PHY's init_count at
1.  A later phy_exit() on the outer PHY then decrements hs_phy->init_count
to -1, and a later phy_init() skips exynos2200_usbdrd_phy_init() while
leaving phy_drd->hs_phy uninitialized.  Conversely, if
phy_exit(phy_drd->hs_phy) itself fails, the outer PHY's init_count drops
to 0 while hs_phy->init_count remains 1.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109221522.2796426-1-soheyb_boulaa@proton.me?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: exynos5-usbdrd: fix HS phy init count imbalance on Exynos2200

[Ivaylo Ivanov]

On 11/10/26 02:15, Soheyb wrote:
> exynos2200_usbdrd_utmi_init() calls phy_init() on the high-speed phy,
> but exynos2200_usbdrd_phy_exit() never calls phy_exit() on it. The init
> count of the high-speed phy therefore never drops back to zero: after
> the first init, every later phy_init() of it returns early, and its
> clocks and supplies stay enabled across an exit of the UTMI phy.
>
> When the high-speed phy loses power while the controller is suspended,
> it is thus not set up again on resume, and the DWC3 core soft reset
> times out:
>
>    dwc3 11210000.usb: DWC3 controller soft reset failed
>
> Exit the high-speed phy when the UTMI phy exits, first, in the reverse
> order of the init.
>
> Fixes: cc52a697f87e ("phy: exynos5-usbdrd: support Exynos USBDRD 3.2 4nm controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Soheyb <soheyb_boulaa@proton.me>
> ---
> Tested on a Google Pixel 9 (Tensor G4) with an out-of-tree port that
> uses the Exynos2200 phy ops and powers the USB block off in system
> sleep: without this change the soft reset failed on the first resume;
> with it, USB came back after each of two system sleeps. Not tested on
> Exynos2200 hardware.
>
>   drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 087d552..0bee928 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -1486,6 +1486,13 @@ static int exynos2200_usbdrd_phy_exit(struct phy *phy)
>   	u32 reg;
>   	int ret;
>   
> +	/*
> +	 * The UTMI init initializes the high-speed phy; exit it in turn, so
> +	 * that the next init sets it up again after it lost power.
> +	 */

I think the function is pretty self-explanatory, so could drop the
comment. Otherwise looks good to me, nice catch!

Acked-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>

Best regards,
Ivaylo

> +	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI)
> +		phy_exit(phy_drd->hs_phy);
> +
>   	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
>   	if (ret)
>   		return ret;


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy