Re: [PATCH v5 13/18] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: robh@kernel.org, vkoul@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, neil.armstrong@linaro.org,
	olteanv@gmail.com, linux-phy@lists.infradead.org
Subject: Re: [PATCH v5 13/18] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
Date: Fri, 12 Jun 2026 16:52:02 +0000	[thread overview]
Message-ID: <20260612165202.A480A1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-13-efc83069869f@collabora.com>

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

Pre-existing issues:
- [High] If `rk_udphy_init()` fails during a mode change or orientation switch, the PHY is left in a reset state but `udphy->status` is not updated and `phy_needs_reinit` is permanently lost. Subsequent operations will silently fail to reinitialize the PHY.
--

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

phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit

This patch renames the mode_change property to phy_needs_reinit. The rename
prepares the driver to also trigger a PHY re-initialization when the cable
orientation changes, not just when the mode changes.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 7255c80e0fe2f..a3b4e2e0f5786 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -968,15 +968,15 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>  	}
>  
>  	if (udphy->status == UDPHY_MODE_NONE) {
> -		udphy->mode_change = false;
> +		udphy->phy_needs_reinit = false;
>  		ret = rk_udphy_setup(udphy);
>  		if (ret)
>  			return ret;
>  
>  		if (udphy->mode & UDPHY_MODE_USB)
>  			rk_udphy_u3_port_disable(udphy, false);
> -	} else if (udphy->mode_change) {
> -		udphy->mode_change = false;
> +	} else if (udphy->phy_needs_reinit) {
> +		udphy->phy_needs_reinit = false;
>  		if (udphy->mode == UDPHY_MODE_DP)
>  			rk_udphy_u3_port_disable(udphy, true);
>  

[Severity: High]
This isn't a bug introduced by this patch, but if rk_udphy_init() fails
immediately after this block, the PHY is left in a reset state while the
udphy->phy_needs_reinit flag is permanently lost.

When changing modes on an already active PHY, rk_udphy_power_on() calls
rk_udphy_init() which immediately asserts all PHY resets. If rk_udphy_init()
subsequently fails (e.g., due to a PLL lock timeout), it leaves the resets
asserted and returns an error.

However, udphy->phy_needs_reinit is cleared right before this, and
udphy->status is not rolled back. As a result, the driver's state tracker
still believes the previously active modes are operational, but the hardware is
completely in reset.

Will future calls to rk_udphy_power_on() see phy_needs_reinit == false and
skip initialization, permanently breaking the active interfaces until a full
system reboot or module reload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=13

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