Re: [PATCH v12 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of -EPROBE_DEFER fallback causes permanent initialization failures due to missing probe-time reset
--

--- Patch [32]: [PATCH v12 32/36] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack ---
commit 98ed05cf436c46697d07d16fcc060eda63439bf7
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Drop -EPROBE_DEFER hack

This commit removes the -EPROBE_DEFER fallback for LCPLL lock timeouts in
rk_udphy_status_check(). It states that a reset during the probe routine
now avoids the problem.

>     The hack to return -EPROBE_DEFER when the lcpll lock timeouts is no
>     longer needed. The driver now does a reset during its probe routine
>     marking everything as off and later on does a re-init, which avoids
>     the problem.

[Severity: High]
Is this description accurate? Looking at rk_udphy_probe(), it appears the
probe-time reset was removed in a prior patch, meaning the driver does not
actually perform a reset during probe:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_probe() {
    ...
	udphy->dev = dev;
	ret = rk_udphy_parse_dt(udphy);
	if (ret)
		return ret;

	mutex_init(&udphy->mutex);
	platform_set_drvdata(pdev, udphy);
    ...
}

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 0333e846ce343..cf173276bfc01 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -750,17 +750,7 @@ static int rk_udphy_status_check(struct rk_udphy *udphy)
>  					       (val & CMN_ANA_LCPLL_LOCK_DONE), 200, 100000);
>  		if (ret) {
>  			dev_err(udphy->dev, "cmn ana lcpll lock timeout\n");
> -			/*
> -			 * If earlier software (U-Boot) enabled USB once already
> -			 * the PLL may have problems locking on the first try.
> -			 * It will be successful on the second try, so for the
> -			 * time being a -EPROBE_DEFER will solve the issue.
> -			 *
> -			 * This requires further investigation to understand the
> -			 * root cause, especially considering that the driver is
> -			 * asserting all reset lines at probe time.
> -			 */
> -			return -EPROBE_DEFER;
> +			return ret;
>  		}

[Severity: High]
Does returning the actual error code instead of -EPROBE_DEFER cause a permanent
initialization failure here?

Since the probe routine lacks the hardware reset assertion, if the bootloader
leaves the DWC3 controller running, it can corrupt the PHY state when the PHY
is later reset.

If another consumer like DisplayPort probes first, or DWC3 is not loaded,
phy_power_on() will assert the PHY reset.

Because DWC3 wouldn't have registered its pre-reset notifier to isolate the
PIPE interface, the PHY LCPLL will fail to lock. Without the -EPROBE_DEFER
fallback, this lock timeout becomes a fatal -ETIMEDOUT error and permanently
breaks the driver.

>  
>  		if (!udphy->flip) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-rockchip-usbdp-cleanup-v12-0-8b41a9a9bef0@collabora.com?part=32