Re: [PATCH v12 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Deferred hardware initialization via `phy_needs_reinit` fixes DisplayPort but leaves USB-only replugs broken if the USB PHY remains powered on.
- [Medium] The function `rk_udphy_dp_phy_configure` accesses shared states locklessly, leading to a data race with concurrent orientation switches.
--

--- Patch [19]: [PATCH v12 19/36] phy: rockchip: usbdp: Re-init the PHY on orientation change ---
commit c45d3419a6f4b9509b416ffd7965a918e7fad8e4
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Re-init the PHY on orientation change

Changing the cable orientation reconfigures the lane muxing, which requires
re-initializing the PHY. The patch introduces a flag to trigger PHY
re-initialization upon orientation change.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index e44d19c9d1193..4b454798c4ed4 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -620,6 +620,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
>  				 enum typec_orientation orien)
>  {
>  	struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
> +	bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
>  
>  	mutex_lock(&udphy->mutex);
>  

[ ... ]

> @@ -631,7 +632,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
>  		goto unlock_ret;
>  	}
>  
> -	udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> +	if (udphy->flip != flipped)
> +		udphy->phy_needs_reinit = true;

[Severity: High]
This is a pre-existing issue, but does this deferred initialization handle
USB-only replugs correctly if the USB PHY remains powered on?

When a user unplugs and replugs a Type-C cable, this sets
udphy->phy_needs_reinit = true. However, in configurations where the DWC3
controller remains active (e.g., fixed host mode) or during rapid replugs,
the USB controller doesn't restart and thus never calls phy_init() again.

Because rk_udphy_usb3_phy_ops only implements .init and has no .power_on
callback, rk_udphy_power_on() is never invoked to process the deferred
initialization flag:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
    ...
	} else if (udphy->phy_needs_reinit) {
		...
		ret = rk_udphy_init(udphy);
    ...
}

Will this cause the USB SuperSpeed connection to fail to train if the
cable is inserted in reverse orientation?

> +
> +	udphy->flip = flipped;
>  	rk_udphy_set_typec_default_mapping(udphy);

[Severity: Medium]
This is a pre-existing issue, but can this lead to a data race with concurrent
orientation switches?

While rk_udphy_orien_sw_set() acquires udphy->mutex and updates the shared lane
and mux selections here via rk_udphy_set_typec_default_mapping(),
rk_udphy_dp_phy_configure() iterates over and accesses the shared lane state
locklessly:

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];
    ...
						   udphy->lane_mux_sel[lane]));
    ...
}

Could this cause rk_udphy_dp_phy_configure() to read partially updated arrays
if the DP controller triggers link training concurrently with an orientation
change?

>  	rk_udphy_usb_bvalid_enable(udphy, true);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-rockchip-usbdp-cleanup-v12-0-8b41a9a9bef0@collabora.com?part=19