Thread (50 messages) 50 messages, 2 authors, 2d ago

Re: [PATCH v9 36/38] phy: rockchip: usbdp: Rename phy_needs_reinit to orientation_changed

From: sashiko-bot@kernel.org
Date: 2026-07-02 00:08:50
Also in: linux-devicetree

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

Pre-existing issues:
- [High] rk_udphy_update_power_state() will unexpectedly power up the PHY and leak clock references if called when status and target_mode are both UDPHY_MODE_NONE but orientation_changed is true. This problem wasn't introduced by this patch, but was introduced by a preceding patch in this series.
--

commit b963e04ecd7343df15170db1d1feeff729b0f62c
Author: Sebastian Reichel [off-list ref]

phy: rockchip: usbdp: Rename phy_needs_reinit to orientation_changed

This commit renames phy_needs_reinit to orientation_changed to better
reflect its purpose, as mode changes are now tracked separately.
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1020,7 +1020,7 @@ static int rk_udphy_update_power_state(struct rk_udphy *udphy)
 	u8 target_mode = udphy->hw_mode & udphy->sw_mode;
 	int ret;
 
-	if (!udphy->phy_needs_reinit && udphy->status == target_mode)
+	if (!udphy->orientation_changed && udphy->status == target_mode)
 		return 0;
 
 	/*
[ ... ]
 	if (udphy->status == UDPHY_MODE_NONE) {
 		/* Power up (incl. clocks) */
 		ret = rk_udphy_setup(udphy, target_mode);
[Severity: High]
This is a pre-existing issue, but does this logic leak clock references if
called when both udphy->status and target_mode are UDPHY_MODE_NONE (0)?

If a Type-C cable is attached or the orientation is toggled while the DP
PHY client is powered off and the port negotiates a DP-only connection,
udphy->orientation_changed becomes true.

When rk_udphy_update_power_state() is subsequently called, the early return
at the top of the function is bypassed because udphy->orientation_changed
is true. Since udphy->status is UDPHY_MODE_NONE, the code takes this branch
and calls rk_udphy_setup(udphy, 0), which unconditionally prepares and
enables the clocks.

Because target_mode is 0, udphy->status remains UDPHY_MODE_NONE, meaning
subsequent genuine power-offs will fail to decrement the clock counts via
rk_udphy_disable(). Can this eventually overflow the clock reference counters?

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help