Re: [PATCH v10 36/36] phy: rockchip: usbdp: Add USB-C state without DP enabled

[sashiko-bot@kernel.org]

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

New issues:
- [High] Early return in `rk_udphy_mode_set` prevents `lane_mux_sel` from being updated when only the cable orientation changes.
- [Medium] Mapping `TYPEC_STATE_USB` to `UDPHY_MODE_USB` causes PHY resets and USB disconnections during DP Alt Mode entry.

Pre-existing issues:
- [High] Devres registration ordering causes a Use-After-Free of `phy_u3` during unbind.
--

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

phy: rockchip: usbdp: Add USB-C state without DP enabled

This commit adds a separate state for USB-only mode without DisplayPort enabled
to improve power management. It splits the previous DP + USB3 combined mode
when DP AltMode is not present.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -617,12 +599,40 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
>  			udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
>  			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>  		}
> -		hw_mode = UDPHY_MODE_DP_USB;
> -		udphy->dp_lanes = 2;
> +	}
> +}
> +
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode, u8 dp_lanes)
> +{
> +	if (udphy->hw_mode == hw_mode && udphy->dp_lanes == dp_lanes)
> +		return;

[Severity: High]
Does this early return prevent updating the lane mux if only the cable
orientation changes?

If a cable is physically flipped, the code updates udphy->flip and calls
rk_udphy_set_typec_default_mapping(), which then sets the state to
TYPEC_DP_STATE_D.

If the PHY was already in UDPHY_MODE_DP_USB with 2 DP lanes, this check
causes an early return, skipping the newly moved rk_udphy_set_lane_mux()
call below. This would leave stale inverted lane assignments when
rk_udphy_init() subsequently configures the hardware.

> +
> +	udphy->phy_needs_reinit = true;
> +	udphy->hw_mode = hw_mode;
> +	udphy->dp_lanes = dp_lanes;
> +
> +	rk_udphy_set_lane_mux(udphy);
> +}
> +
> +static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
> +{
> +	switch (state) {
> [ ... ]
> +	case TYPEC_STATE_SAFE:
> +	case TYPEC_STATE_USB:
> +	default:
> +		rk_udphy_mode_set(udphy, UDPHY_MODE_USB, 0);
> +		break;

[Severity: Medium]
Will mapping this intermediate state to USB-only force a disruptive hardware
mode change when transitioning to DP Alt Mode?

Type-C port managers typically request TYPEC_STATE_USB upon connection.
Setting this to UDPHY_MODE_USB configures 0 DP lanes.

When DP Alt Mode successfully negotiates shortly after and transitions to
TYPEC_DP_STATE_D, hw_mode changes to UDPHY_MODE_DP_USB. This triggers a
full PHY reset in rk_udphy_update_power_state() and interrupts active
USB transfers.

The commit message mentions keeping all PHY parts powered up to avoid data
stream interruptions, but could this intermediate state mapping negate that
benefit during initial connection?

>  	}
>  
>  }

[Severity: High]
This is a pre-existing issue, but does the devres registration ordering in
rk_udphy_probe() cause a use-after-free of phy_u3 during unbind?

The Type-C orientation switch and mux are registered with devres before
the PHYs are created:

rk_udphy_probe() {
    ...
    if (device_property_present(dev, "mode-switch")) {
        ret = rk_udphy_setup_typec_mux(udphy);
    ...
    udphy->phy_u3 = devm_phy_create(dev, dev->of_node, &rk_udphy_usb3_phy_ops);
    ...
}

During unbind, devres executes cleanup in reverse order. The PHY structures
are freed while the Type-C mux remains registered.

If a concurrent Type-C event arrives and calls rk_udphy_typec_mux_set(),
it invokes rk_udphy_update_power_state() which uses the freed pointer:

rk_udphy_update_power_state() {
    ...
    phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-rockchip-usbdp-cleanup-v10-0-a392711ca8a9@collabora.com?part=36