Re: [PATCH v5 02/18] phy: rockchip: usbdp: Do not lose USB3 PHY status
From: sashiko-bot@kernel.org
Date: 2026-06-12 16:38:36
Also in:
linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] State desynchronization on `rk_udphy_init` failure causes subsequent power-on retries to falsely return success without re-initializing the hardware, leading to dead PHY or bus hangs. Pre-existing issues: - [High] Subsystem power-off requests permanently leak `udphy->status` bits if the Type-C mode has physically changed, causing a perpetual resource leak of the PHY clocks. -- commit 93e296ece1bf03449fcdb7d0485cb77da80dd9b7 Author: Sebastian Reichel [off-list ref] phy: rockchip: usbdp: Do not lose USB3 PHY status This commit fixes an issue where the USB3 tracking information is lost when the DisplayPort is re-initialized. It does this by preventing the status variable from being cleared during mode changes in the PHY driver.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c index fba35510d88ce..744cc7c642f49 100644 --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c@@ -1009,7 +1009,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode) rk_udphy_u3_port_disable(udphy, false); } else if (udphy->mode_change) { udphy->mode_change = false; - udphy->status = UDPHY_MODE_NONE;
[Severity: High]
If rk_udphy_init() fails (for example, returning -EPROBE_DEFER after a lock
timeout), will this lead to a permanent state desynchronization?
By removing the clearing of udphy->status, if the initialization fails,
udphy->mode_change is already set to false but udphy->status remains
non-zero.
If the subsystem later retries rk_udphy_power_on(), it will see
udphy->status != UDPHY_MODE_NONE and udphy->mode_change == false. Could
this cause it to bypass all initialization, blindly update the status, and
falsely return success while the PHY hardware remains disabled and in reset?
[Severity: High]
This is a pre-existing issue, but removing the forced clearing of
udphy->status here exposes a potential permanent resource leak of the PHY
clocks.
Looking at rk_udphy_power_off():
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
and rk_udphy_usb3_phy_exit():
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
If the Type-C multiplexer switches from DP_USB to a DP_ONLY mode,
udphy->mode loses the USB bit. If the USB controller then unbinds and calls
phy_exit() while in DP_ONLY mode, the early returns above prevent clearing
the UDPHY_MODE_USB bit from udphy->status.
When DP eventually powers off, will udphy->status remain non-zero due to
this orphaned USB bit, preventing rk_udphy_disable() from ever executing
and leaking the PHY clocks and power?
if (udphy->mode == UDPHY_MODE_DP) rk_udphy_u3_port_disable(udphy, true);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=2