Re: [PATCH v13 10/35] phy: rockchip: usbdp: Add missing mode_change update

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incomplete bug fix: rk_udphy_mode_set early return prevents PHY re-initialization on cable flip.

Pre-existing issues:
- [High] Exiting DisplayPort Alt Mode bypasses mode update, permanently breaking USB 3.0.
--

--- Patch [10]: [PATCH v13 10/35] phy: rockchip: usbdp: Add missing mode_change update ---
commit 2fda37a8a08c68a5b07d269f553cf22878463e2b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Add missing mode_change update

This patch introduces a helper to update available modes and set the
mode_change flag for the Rockchip USBD PHY. This resolves missing
re-initialization of the PHY to ensure DisplayPort functions correctly.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 982315535caf7..7540c897dde13 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -617,6 +617,15 @@ static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
>  	rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
>  }
>  
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
> +{
> +	if (udphy->mode == mode)
> +		return;

[Severity: High]
Does this early return prevent PHY re-initialization when the cable is
flipped?

If the user flips a Type-C cable while the USB controller keeps the PHY
powered on, rk_udphy_set_typec_default_mapping() is called and it updates
the software lane assignments. It then calls
rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB).

If the mode is already UDPHY_MODE_DP_USB, this helper returns early, leaving
mode_change set to false. Subsequent calls might then skip initializing the
PHY, causing the hardware lane routing to miss the new orientation.

> +
> +	udphy->mode_change = true;
> +	udphy->mode = mode;
> +}
> +
>  static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
>  {
>  	if (udphy->flip) {
[ ... ]
> @@ -1361,10 +1370,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  			usleep_range(750, 800);
>  			rk_udphy_dp_hpd_event_trigger(udphy, true);
>  		} else if (data->status & DP_STATUS_HPD_STATE) {
> -			if (udphy->mode != mode) {
> -				udphy->mode = mode;
> -				udphy->mode_change = true;
> -			}
> +			rk_udphy_mode_set(udphy, mode);

[Severity: High]
This is a pre-existing issue, but does exiting DisplayPort Alt Mode
permanently break USB 3.0?

When the Type-C port exits Alt Mode (for example, the monitor goes to sleep
or is disconnected logically), the framework calls typec_mux_set with
state->mode = TYPEC_STATE_USB and state->alt = NULL.

This function calculates the mode correctly, but the update itself is inside
an if block:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_typec_mux_set() {
    ...
    if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
        ...
        } else if (data->status & DP_STATUS_HPD_STATE) {
            rk_udphy_mode_set(udphy, mode);
            rk_udphy_dp_hpd_event_trigger(udphy, true);
        ...
    }
    ...
}

Because state->alt is NULL, the update is skipped and udphy->mode remains
stuck in UDPHY_MODE_DP. Later, when the USB controller tries to initialize,
it checks !(udphy->mode & UDPHY_MODE_USB) and aborts, leaving USB broken
until the device is replugged.

>  			rk_udphy_dp_hpd_event_trigger(udphy, true);
>  		} else {
>  			rk_udphy_dp_hpd_event_trigger(udphy, false);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=10

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