Re: [PATCH v17 1/7] phy: rockchip: usbdp: Track PHY power state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: state tracking leak in rk_udphy_usb3_phy_exit()
--

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

phy: rockchip: usbdp: Track PHY power state

Track PHY state requested by software for DP and USB3 individually
in preparation to rework and fix up power state handling.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e748acdd2d313..db69dd24f73b3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -1286,16 +1293,25 @@ static const struct phy_ops rk_udphy_dp_phy_ops = {
>  static int rk_udphy_usb3_phy_init(struct phy *phy)
>  {
>  	struct rk_udphy *udphy = phy_get_drvdata(phy);
> +	int ret;
>  
>  	guard(mutex)(&udphy->mutex);
>  
> +	udphy->sw_mode |= UDPHY_MODE_USB;
> +
>  	/* DP only or high-speed, disable U3 port */
>  	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
>  		rk_udphy_u3_port_disable(udphy, true);
>  		return 0;
>  	}
>  
> -	return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> +	ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
> +	if (ret) {
> +		udphy->sw_mode &= ~UDPHY_MODE_USB;
> +		return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static int rk_udphy_usb3_phy_exit(struct phy *phy)
> @@ -1308,6 +1324,8 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
>  	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
>  		return 0;
>  
> +	udphy->sw_mode &= ~UDPHY_MODE_USB;
> +

[Severity: Medium]
In rk_udphy_usb3_phy_init(), UDPHY_MODE_USB is unconditionally added to
sw_mode before the early return for DP-only or high-speed cases.

However, in rk_udphy_usb3_phy_exit(), the corresponding early return happens
before clearing UDPHY_MODE_USB from sw_mode.

Will this cause the sw_mode state to leak? If so, could subsequent operations
like rk_udphy_power_on() called during DP PHY initialization incorrectly see
UDPHY_MODE_USB as requested, potentially executing the DWC3 reset handler
unnecessarily?

>  	rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-b4-rockchip-dwc3-rockchip-glue-v17-0-876f96893e2f@collabora.com?part=1