Re: [PATCH v8 24/29] phy: rockchip: usbdp: Support going from DP-only mode to USB mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic introduced to restore USB functionality fails on subsequent adapter replugs because `udphy->status` permanently retains the `UDPHY_MODE_USB` flag.
--

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

phy: rockchip: usbdp: Support going from DP-only mode to USB mode

When a USB-C adapter, which maps all Superspeed lanes to DP is plugged
in, the USB support is disabled in the PHY. When the adapter is
unplugged and a different adapter with USB functionality is plugged in
afterwards, USB functionality is not restored as the USB controller
keeps the PHY enabled for the entire time.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 837a4cb3e4b60..4566822d70c4a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -1347,6 +1362,17 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  
>  	rk_udphy_set_typec_state(udphy, state->mode);
>  
> +	/*
> +	 * If the new mode includes USB, but it has not yet been powered
> +	 * (because the previous mode was DP-only) and the USB PHY was
> +	 * already initialized by the USB controller, we need to power on
> +	 * the USB side now since no subsequent phy_init call will come
> +	 * from the controller.
> +	 */
> +	if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
> +	    udphy->usb_in_use && !udphy->hs)
> +		return rk_udphy_power_on(udphy, UDPHY_MODE_USB);

[Severity: High]
Does this logic handle subsequent replugs correctly if a USB device is plugged
in first?

Since the USB controller keeps the PHY enabled during hotplug (as noted in the
commit message), the UDPHY_MODE_USB flag in udphy->status is not cleared.

If the user then plugs in a DP-only adapter, the PHY is reinitialized for DP
and physically disables the USB3 port.

If the user later unplugs the DP-only adapter and plugs in a USB-only adapter,
won't !(udphy->status & UDPHY_MODE_USB) evaluate to false because the flag was
never cleared? It seems this would skip the call to rk_udphy_power_on() and
leave the USB3 port disabled.

> +
>  	return 0;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=24