Re: [PATCH v8 13/29] phy: rockchip: usbdp: Limit DP lane count to muxed lanes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch's fix is incomplete. The lane count validation added to prevent DP configurations from corrupting USB3 PHY state is bypassed during voltage-only updates. If the Type-C state dynamically reduces the available DP lanes, `udphy->dp_lanes` is updated but the cached `udphy->lanes` remains stale. A subsequent voltage-only update will iterate up to the stale `udphy->lanes` and write DP voltage configurations to the physically reassigned USB3 lanes, corrupting the USB3 PHY state.
--

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

phy: rockchip: usbdp: Limit DP lane count to muxed lanes

This commit limits the DisplayPort lane count to 2 if the PHY is
restricted due to USB3 usage. It aims to prevent USB3 state corruption
during voltage configuration.

Closes: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 5c38da0dadd22..6de8ac3f91441 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1132,6 +1132,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
>  static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
>  					struct phy_configure_opts_dp *dp)
>  {
> +	if (dp->lanes > udphy->dp_lanes)
> +		return -EINVAL;
> +

[Severity: High]
This isn't a bug introduced by this patch, but is the fix incomplete for
voltage-only updates?

If a Type-C event renegotiates the connection and restricts udphy->dp_lanes
to 2, udphy->lanes could still hold a stale value (like 4).

Looking at rk_udphy_dp_phy_configure(), the lane verification is skipped
if dp->set_lanes is false:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
	...
	if (dp->set_lanes) {
		ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
		if (ret)
			return ret;
	}
	...
}

If the driver then issues a phy_configure() with set_lanes=false and
set_voltages=true, the voltage loop will iterate using the stale
udphy->lanes value:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
	...
	if (dp->set_voltages) {
		for (i = 0; i < udphy->lanes; i++) {
			lane = udphy->dp_lane_sel[i];
	...
}

Could this still result in writing DP voltage configurations to reassigned
USB3 lanes, corrupting the USB3 PHY state?

>  	switch (dp->lanes) {
>  	case 1:
>  	case 2:
[ ... ]

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