[PATCH v6 07/23] phy: rockchip: usbdp: Keep clocks running on PHY re-init

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The Type-C mux callback completely ignores non-DP AltMode events, failing to reset lane mapping on AltMode exit.
- [High] Asynchronous `udphy->mode` updates cause `phy_exit` and `phy_power_off` to skip teardown, resulting in permanent clock and reset leaks.
--

commit ea283780e749d528861fc2fe55f998d22454a53b
Author: Sebastian Reichel <sebastian.reichel at collabora.com>

phy: rockchip: usbdp: Keep clocks running on PHY re-init

When a mode change is required rk_udphy_power_on() disables
the clocks and then calls rk_udphy_setup(), which then enables
all the clocks again before continuing with rk_udphy_init().

Considering that rk_udphy_init() does assert the reset lines,
re-enabling the clocks is just delaying things. Avoid it by
directly calling rk_udphy_init().

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 6dff4dfc9363e..c05ee65a9f73a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1011,8 +1011,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>  		if (udphy->mode == UDPHY_MODE_DP)
>  			rk_udphy_u3_port_disable(udphy, true);
>  
> -		rk_udphy_disable(udphy);
> -		ret = rk_udphy_setup(udphy);
> +		ret = rk_udphy_init(udphy);

[Severity: High]
This is a pre-existing issue, but the Type-C mux callback completely ignores
non-DP AltMode events, failing to reset lane mapping on AltMode exit.

When exiting DP AltMode (e.g., via sysfs or renegotiation without a physical
cable unplug), the Type-C subsystem calls typec_mux_set() with state->mode =
TYPEC_STATE_SAFE or TYPEC_STATE_USB and state->alt = NULL.

However, rk_udphy_typec_mux_set() immediately returns 0 if !state->alt:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_typec_mux_set() {
    ...
	if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
		return 0;
    ...
}

Does this leave udphy->mode stuck in the previous DP configuration (e.g.,
UDPHY_MODE_DP with 4 DP lanes), which breaks subsequent USB3 initialization
because rk_udphy_usb3_phy_init() checks udphy->mode and skips powering on
the U3 port?

[Severity: High]
This is a pre-existing issue, but asynchronous udphy->mode updates cause
phy_exit() and phy_power_off() to skip teardown, resulting in permanent clock
and reset leaks.

udphy->mode tracks the target hardware mux state and is updated asynchronously
by Type-C events. However, rk_udphy_usb3_phy_exit() checks this current
udphy->mode rather than the initialized state (udphy->status) to decide
whether to power off:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_usb3_phy_exit() {
    ...
	/* DP only or high-speed */
	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
		return 0;
    ...
}

The phy_power_off() implementation also suffers from this flaw,
short-circuiting before it can update udphy->status and tear down resources:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off() {
    ...
	if (!(udphy->mode & mode)) {
		dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
		return;
	}
    ...
}

If the Type-C mux switches from a DP+USB mode to a DP-only mode while USB3
is active, UDPHY_MODE_USB is cleared from udphy->mode. When the USB3
controller later shuts down and calls phy_exit(), rk_udphy_usb3_phy_exit()
sees the bit missing and returns 0 early, skipping rk_udphy_power_off().

This prevents UDPHY_MODE_USB from being cleared from udphy->status. When the
DP controller subsequently calls phy_power_off(), udphy->status remains
non-zero, skipping rk_udphy_disable().

Since there is no driver removal cleanup fallback, do the clock enable
references get permanently leaked?

>  		if (ret)
>  			return ret;
>  		udphy->mode_change = false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-rockchip-usbdp-cleanup-v6-0-3bb1f54b3f35@collabora.com?part=7