Re: [PATCH v10 23/36] phy: rockchip: usbdp: Clear USB status on PHY exit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clearing `udphy->status` directly without checking if the PHY hardware should be disabled causes an unbounded clock reference count leak.
--

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

phy: rockchip: usbdp: Clear USB status on PHY exit

This commit ensures the USB status flag is cleared when the USB3 PHY is
exited while the system is in DP-only mode. This happens if the USB3
controller device is unbound while a DP-only adapter is plugged in.

Closes: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com

> 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
> @@ -1273,8 +1273,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
>  	guard(mutex)(&udphy->mutex);
>  
>  	/* DP only or high-speed */
> -	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> +	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> +		udphy->status &= ~UDPHY_MODE_USB;
>  		return 0;
> +	}
>  
>  	rk_udphy_power_off(udphy, UDPHY_MODE_USB);

[Severity: High]
Does clearing udphy->status without actually disabling the hardware cause an
unbounded clock reference count leak?

When the system switches to DP-only mode, udphy->mode loses the
UDPHY_MODE_USB bit. If the USB controller is then unbound, we hit this path
and clear UDPHY_MODE_USB from udphy->status, but return early. The hardware
clocks remain enabled since we bypass the hardware disable call.

If the system then switches back to USB mode, rk_udphy_usb3_phy_init() will
see udphy->status as UDPHY_MODE_NONE and call rk_udphy_setup(). This calls
clk_bulk_prepare_enable() again, incrementing the clock refcount indefinitely
on every cycle.

I see this issue is addressed later in the series by commit
e0219fd5f4d784e78ff969af266e10196367be69 ("phy: rockchip: usbdp: Simplify
power state handling"), but doesn't this patch introduce a regression here
that breaks bisectability?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-rockchip-usbdp-cleanup-v10-0-a392711ca8a9@collabora.com?part=23

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy