Re: [PATCH v14 20/38] phy: rockchip: usbdp: Factor out lane_mux_sel setup

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

Pre-existing issues:
- [High] Data race in `rk_udphy_dp_phy_configure` allows DP link training to corrupt USB PHY lanes.
--

--- Patch [20]: [PATCH v14 20/38] phy: rockchip: usbdp: Factor out lane_mux_sel setup ---
commit 312855a280db1af8a51a65d7c305801cd55a506d
Author: Sebastian Reichel <[email protected]>

phy: rockchip: usbdp: Factor out lane_mux_sel setup

Avoid describing the USB+DP lane_mux_sel logic twice by introducing
a helper function to reduce code duplication.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4b454798c4ed4..4e54020d17551 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -582,6 +582,42 @@ static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
>  	udphy->mode = mode;
>  }
>  
> +static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
> +{
> +	u8 mode;
> +
> +	switch (state) {
> +	case TYPEC_DP_STATE_C:
> +	case TYPEC_DP_STATE_E:
> +		udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
> +		udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
> +		udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
> +		udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
> +		mode = UDPHY_MODE_DP;
> +		udphy->dp_lanes = 4;
> +		break;

[Severity: High]
This is a pre-existing issue, but can this code race with DisplayPort link
training?

If a user rapidly unplugs or rotates the Type-C cable while a display is
attached and undergoing link training, the Type-C subsystem could
asynchronously call rk_udphy_typec_mux_set(). This modifies lane_mux_sel and
dp_lanes here in rk_udphy_set_typec_state() while holding udphy->mutex.

Meanwhile, the DP MAC driver might be calling phy_configure() to perform
link training, which invokes rk_udphy_dp_phy_configure():

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
		for (i = 0; i < udphy->lanes; i++) {
			lane = udphy->dp_lane_sel[i];
			switch (udphy->link_rate) {
			case 1620:
			case 2700:
				regmap_update_bits(udphy->pma_regmap,
						   TRSV_ANA_TX_CLK_OFFSET_N(lane),
						   LN_ANA_TX_SER_TXCLK_INV,
						   FIELD_PREP(LN_ANA_TX_SER_TXCLK_INV,
						   udphy->lane_mux_sel[lane]));
...
}

If rk_udphy_dp_phy_configure() locklessly verifies dp->lanes <= udphy->dp_lanes
and loops over udphy->lanes, could it read a torn lane_mux_sel array or a stale
dp_lanes value? This might result in applying DisplayPort voltages to PHY lanes
that have just been reassigned to USB, potentially corrupting the USB link and
leaving the PHY in an inconsistent state.

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

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