Re: [PATCH v8 28/29] phy: rockchip: usbdp: Avoid xHCI SErrors - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: neil.armstrong@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, robh@kernel.org,
	linux-phy@lists.infradead.org, olteanv@gmail.com,
	vkoul@kernel.org
Subject: Re: [PATCH v8 28/29] phy: rockchip: usbdp: Avoid xHCI SErrors
Date: Fri, 26 Jun 2026 21:24:24 +0000	[thread overview]
Message-ID: <20260626212424.C215E1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-28-47f682987895@collabora.com>

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

Pre-existing issues:
- [High] Unconditionally re-enabling the USB3 port during PHY re-initialization overrides the High-Speed only (hs) configuration, potentially leading to an SError on subsequent PHY power off.
--

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

phy: rockchip: usbdp: Avoid xHCI SErrors

This patch avoids xHCI SErrors by disabling the USB3 port before
re-initializing it when switching between USB3-only, USB3 + DP and DP-only
modes.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index dc166392ba199..7c8b9eaaf3526 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1033,8 +1033,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>  			rk_udphy_u3_port_disable(udphy, false);

[Severity: High]
This is a pre-existing issue, but does this unconditionally re-enable the
USB3 port and override the High-Speed only (hs) configuration?

If the system is configured with a high-speed only USB device, the PHY
initialization via rk_udphy_usb3_phy_init() correctly disables the USB3 port
to safely route the xHCI PIPE clock to the UTMI fallback.

When a DisplayPort cable is plugged in, rk_udphy_power_on() is called. Since
udphy->mode still contains UDPHY_MODE_USB for combo modes, this code
switches the clock source back to the PHY, overriding the disablement.

When the DP cable is disconnected, rk_udphy_dp_phy_power_off() drops the
status to UDPHY_MODE_NONE and powers off the PHY. At this point, the active
xHCI controller runs without a clock, causing an immediate SError on its
next register access.

Should the re-enable conditions here, and similarly after the PHY
re-initialization further down in this function, check if high-speed only
is disabled?

For example:
if ((udphy->mode & UDPHY_MODE_USB) && !udphy->hs)

>  		udphy->phy_needs_reinit = false;
>  	} else if (udphy->phy_needs_reinit) {
> -		if (udphy->mode == UDPHY_MODE_DP)
> -			rk_udphy_u3_port_disable(udphy, true);
> +		rk_udphy_u3_port_disable(udphy, true);
> +		udelay(10);
>  
>  		ret = rk_udphy_init(udphy);
>  		if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=28

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