Re: [PATCH v14 25/38] phy: rockchip: usbdp: Avoid xHCI SErrors

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

Pre-existing issues:
- [High] The initial power-on path (`udphy->status == UDPHY_MODE_NONE`) in `rk_udphy_power_on` fails to verify `!udphy->hs` before re-enabling the U3 port, which can incorrectly enable the USB3 port for high-speed-only configurations.
--

--- Patch [25]: [PATCH v14 25/38] phy: rockchip: usbdp: Avoid xHCI SErrors ---
commit a6a982bf14ce7b1d3186386afd971d5fcada88b4
Author: Sebastian Reichel <[email protected]>

phy: rockchip: usbdp: Avoid xHCI SErrors

The USBDP PHY provides the PIPE clock to the USB3 controller, which
means the PHY must be fully running when anything tries to access
the xHCI registers. This patch avoids the problems by disabling the USB3
port before re-initializing it.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e67c1df792ed2..c7f09040185b6 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1000,12 +1000,15 @@ 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 branch also need to check for
!udphy->hs before re-enabling the U3 port?

If a system is restricted to high-speed and the DisplayPort PHY powers on
while udphy->status is UDPHY_MODE_NONE, could this incorrectly enable the
USB3 port and cause xHCI SErrors?

The patch correctly adds the !udphy->hs check to the re-init branch below,
but it appears to be missing here in the initial setup branch of
rk_udphy_power_on().

>  		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)
>  			return ret;
> +
> +		if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
> +			rk_udphy_u3_port_disable(udphy, false);
>  		udphy->phy_needs_reinit = false;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=25

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy