[PATCH v12 27/36] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Fri Jul 10 14:17:13 PDT 2026
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] If PHY re-initialization fails during a mode change, the U3 port is left permanently disabled and in reset, but the software state incorrectly remains active.
--
--- Patch [27]: [PATCH v12 27/36] phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors ---
commit 00e32198a00a3780b193e546b8f16d2218cefb74
Author: Sebastian Reichel <sebastian.reichel at collabora.com>
phy: rockchip: usbdp: Handle rk_udphy_reset_deassert errors
This commit modifies the initialization sequence to handle errors returned
by reset deassertion, avoiding a theoretical SError.
Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot at kernel.org>
Closes: https://sashiko.dev/#/message/20260626211151.2332F1F000E9%40smtp.kernel.org
> 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
> @@ -803,8 +803,12 @@ static int rk_udphy_init(struct rk_udphy *udphy)
> /* Step 1: power on pma and deassert apb rstn */
> rk_udphy_grfreg_write(udphy->udphygrf, &cfg->grfcfg.low_pwrn, true);
>
> - rk_udphy_reset_deassert(udphy, "pma_apb");
> - rk_udphy_reset_deassert(udphy, "pcs_apb");
> + ret = rk_udphy_reset_deassert(udphy, "pma_apb");
> + if (ret)
> + goto assert_resets;
[Severity: Medium]
This is a pre-existing issue, but returning an error here propagates up to
rk_udphy_power_on(). Does this leave the U3 port in a permanently disabled
state?
When USB3 is active and a new mode is enabled, rk_udphy_power_on() triggers
re-initialization. It disables the U3 port and calls rk_udphy_init(). If
rk_udphy_init() fails and returns an error here, the caller propagates it
immediately:
rk_udphy_power_on() {
...
ret = rk_udphy_init(udphy);
if (ret)
return ret;
if (udphy->mode & UDPHY_MODE_USB)
rk_udphy_u3_port_disable(udphy, false);
...
}
If this path is taken, the U3 port is left permanently disabled and in reset,
but the software state (udphy->status) incorrectly remains active. The USB3
controller is unaware that the PHY has been forced into reset.
Could this result in a silently dropped USB3 connection, potentially causing
I/O errors for active storage devices?
While a previous patch in the series mitigates the SError by safely routing
the PIPE clock to UTMI, the state inconsistency and dead USB connection seem
to remain.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-rockchip-usbdp-cleanup-v12-0-8b41a9a9bef0@collabora.com?part=27
More information about the linux-phy
mailing list