Re: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check for requested DP lanes allows out-of-bounds register corruption.
- [High] Data race and unprotected register access in `rk_udphy_dp_phy_configure()` due to missing mutex lock.
--

--- Patch [11]: [PATCH v11 11/36] phy: rockchip: usbdp: Support single-lane DP ---
commit 543954c74eaa703430fe27a5fd2cfce0594a6d78
Author: Zhang Yubing <yubing.zhang@rock-chips.com>

phy: rockchip: usbdp: Support single-lane DP

Implement support for using just a single DisplayPort line.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 7540c897dde13..c4d62b234ea80 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -193,6 +193,7 @@ struct rk_udphy {
>  	int id;
>  
>  	bool dp_in_use;
> +	int dp_lanes;

[Severity: High]
This isn't a bug introduced by this patch, but does the requested lane
count need to be bounded against the newly added udphy->dp_lanes?

If a DRM bridge calls phy_configure() with dp->lanes = 4 when
udphy->dp_lanes = 2, it seems rk_udphy_dp_phy_verify_lanes() will
accept it:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_verify_lanes() {
	switch (dp->lanes) {
	case 1:
	case 2:
	case 4:
		/* valid lane count. */
		udphy->lanes = dp->lanes;
		break;
...
}

Would this cause rk_udphy_dp_phy_configure() to iterate out-of-bounds of
the DP-assigned lanes and apply DP voltage/pre-emphasis settings to USB
PMA analog registers, potentially corrupting the USB3 PHY state?

[ ... ]

> @@ -1050,18 +1039,17 @@ static int rk_udphy_dp_phy_exit(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);

[Severity: High]
This is a pre-existing issue, but while rk_udphy_dp_phy_power_on() acquires
udphy->mutex before modifying driver state, rk_udphy_dp_phy_configure()
appears to access shared state and hardware registers without this lock:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
	struct rk_udphy *udphy = phy_get_drvdata(phy);
	struct phy_configure_opts_dp *dp = &opts->dp;
        ...
        /* No mutex_lock(&udphy->mutex) here */
        ...
}

Can a Type-C event (like a cable flip or alt mode change) concurrently
trigger rk_udphy_typec_mux_set() and race with rk_udphy_dp_phy_configure()?

Could this data race on udphy->lane_mux_sel and concurrent writes to the
pma_regmap lead to hardware corruption or link training failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=11