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

[sashiko-bot@kernel.org]

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 <sebastian.reichel@collabora.com>

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.

> 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

[ ... ]

> @@ -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