Re: [PATCH v9 37/38] phy: rockchip: usbdp: Re-init PHY on mux change
From: sashiko-bot@kernel.org
Date: 2026-07-02 00:12:15
Also in:
linux-phy
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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