Re: [PATCH v9 37/38] phy: rockchip: usbdp: Re-init PHY on mux change

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `rk_udphy_update_power_state()` is ignored in `rk_udphy_typec_mux_set()`, silently masking hardware initialization failures from the Type-C framework.
- [High] A logic flaw in `rk_udphy_update_power_state()` causes a permanent clock reference leak and PHY state desynchronization if a mux change occurs when no PHY consumers are active.
--

commit 53a5cecf0aaba3a0de5eba24ab8177288b8ff3ea
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Re-init PHY on mux change

Ensure that the right part of the PHY are powered up when the
mode changes. This ensures the PHY is re-initialized when cable
orientation changes or when switching from DP-only to a mode with
USB support.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 735ce1bfac7af..d056da53cb1c8 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1372,6 +1372,8 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  
>  	rk_udphy_set_typec_state(udphy, state->mode);
>  
> +	rk_udphy_update_power_state(udphy);
> +

[Severity: High]
Is it safe to ignore the return value of rk_udphy_update_power_state() here? 

If hardware setup steps like PLL lock timeouts or clock enablement fail, the
error is discarded and the Type-C framework is left unaware. Could this lead
to asynchronous SErrors or kernel panics if the DWC3 controller is accessed
while the PHY is broken or disabled?

[Severity: High]
Does calling this function during a mux change when no PHY consumers
are active lead to a permanent clock reference leak?

If a Type-C mux or orientation change occurs while no PHY consumers are active
(for example, the DP driver isn't bound or USB is in runtime suspend),
udphy->sw_mode is 0, making target_mode 0.

Looking at rk_udphy_update_power_state(), it bypasses the early exit because
udphy->orientation_changed is true. Since udphy->status is 0, it calls
rk_udphy_setup() which increments clock refcounts:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_update_power_state() {
    ...
	if (udphy->status == UDPHY_MODE_NONE) {
		/* Power up (incl. clocks) */
		ret = rk_udphy_setup(udphy, target_mode);
    ...
}

It then assigns udphy->status = target_mode (which is 0). Because the status
remains 0 despite clocks being enabled, the next time a consumer activates
the PHY, rk_udphy_setup() is called again, double-incrementing the clock
refcounts. 

During teardown, rk_udphy_disable() would only be called once, preventing the
SoC from entering deeper low-power states. Can this logic be adjusted to
properly track when clocks are enabled or avoid enabling them if target_mode
is 0?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-rockchip-usbdp-cleanup-v9-0-e31efbb62d2e@collabora.com?part=37