Re: [PATCH v15 11/16] phy: rockchip: usbdp: Support single-lane DP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] phy: rockchip: usbdp: errant DP lane enable in USB-only mode
--

commit eaea7c0e371c41abc4589c0ac17cb5d7fe7f1179
Author: Zhang Yubing <yubing.zhang@rock-chips.com>

phy: rockchip: usbdp: Support single-lane DP

This commit implements support for using just a single DisplayPort line in
the rockchip usbdp phy driver.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index b72daf80af15f..4fce9cd664735 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -1049,18 +1038,17 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
>  static int rk_udphy_dp_phy_power_on(struct phy *phy)
>  {
>  	struct rk_udphy *udphy = phy_get_drvdata(phy);
> -	int ret, dp_lanes;
> +	int ret;
>  
>  	mutex_lock(&udphy->mutex);
>  
> -	dp_lanes = rk_udphy_dplane_get(udphy);
> -	phy_set_bus_width(phy, dp_lanes);
> +	phy_set_bus_width(phy, udphy->dp_lanes);
>  
>  	ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
>  	if (ret)
>  		goto unlock;
>  
> -	rk_udphy_dplane_enable(udphy, dp_lanes);
> +	rk_udphy_dplane_enable(udphy, udphy->dp_lanes);

[Severity: Medium]
Could this unintentionally enable unclocked DP lane drivers on the physical
pins in USB-only mode?

If the DP controller calls phy_power_on(), rk_udphy_power_on() might detect
that UDPHY_MODE_DP is not supported in udphy->mode. If it does, it prints a
message and returns 0 (success) without powering up the DP PLLs. Execution
then proceeds to this rk_udphy_dplane_enable() call.

Prior to this patch, rk_udphy_dplane_get() returned 0 when DP was disabled,
which safely disabled the lanes and asserted CMN_DP_CMN_RSTN. Now, since
rk_udphy_typec_mux_set() unconditionally sets udphy->dp_lanes to 2 or 4 in
Type-C states (even without DP altmode), a non-zero value is passed here.

Does passing the decoupled udphy->dp_lanes skip asserting the reset and
erroneously enable unclocked DP lane drivers, potentially causing undefined
PHY behavior or electrical contention on shared pins?

[ ... ]

> @@ -1356,6 +1345,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>  		}
>  		mode = UDPHY_MODE_DP_USB;
> +		udphy->dp_lanes = 2;
>  		break;
>  	}

[Severity: Medium]
Does this unconditionally set dp_lanes to 2 for USB-only Type-C states without
updating udphy->mode, contributing to the DP lane enable issue mentioned above
in rk_udphy_dp_phy_power_on()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-rockchip-usbdp-cleanup-v15-0-80e2c11d1ce8@collabora.com?part=11

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