Re: [PATCH v14 21/38] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB

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

Pre-existing issues:
- [High] Missing PHY hardware re-initialization on DP AltMode exit leaves USB 3.0 broken.
- [High] Failure to re-enable U3 port (USB3) when transitioning from DP-only mode back to DP+USB mode in rk_udphy_power_on.
--

--- Patch [21]: [PATCH v14 21/38] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB ---
commit bc1b433620764837d185aacbce6b35af43522e3a
Author: Sebastian Reichel <[email protected]>

phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB

This patch updates the Type-C mux set callback to handle safe state and USB
state events. It ensures the software state is updated when exiting DP
AltMode.

Reported-by: Sashiko <[email protected]>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org
Tested-by: Igor Paunovic <[email protected]> # Orange Pi 5 Plus
Signed-off-by: Sebastian Reichel <[email protected]>

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4e54020d17551..0399cbf96e195 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1305,11 +1314,10 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  	struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
>  
> -	/*
> -	 * Ignore mux events not involving DP AltMode, because
> -	 * the mode field is being reused, e.g. state->mode == 4
> -	 * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> -	 */
> -	if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> +	/* Ignore mux events not involving USB or DP */
> +	if (!rk_udphy_is_supported_mode(state))
>  		return 0;
>  
>  	mutex_lock(&udphy->mutex);

[Severity: High]
This isn't a bug introduced by this patch, but does this logic leave USB 3.0
broken when exiting DP AltMode?

When the Type-C mux transitions to TYPEC_STATE_SAFE or TYPEC_STATE_USB,
rk_udphy_set_typec_state() updates the software state and sets
phy_needs_reinit = true.

Since the USB subsystem is already powered on, it will not call
phy_power_on() again. The DP subsystem calls phy_power_off(), but
rk_udphy_power_off() only clears the UDPHY_MODE_DP flag from the status
without reinitializing the hardware.

Can the PHY hardware remain stuck in 4-lane DP mode because the required
re-initialization is never executed?

There is another pre-existing issue in the re-initialization path that might
prevent recovery.

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
[ ... ]
        } else if (udphy->phy_needs_reinit) {
                if (udphy->mode == UDPHY_MODE_DP)
                        rk_udphy_u3_port_disable(udphy, true);

                ret = rk_udphy_init(udphy);
[ ... ]
}

[Severity: High]
This is a pre-existing issue, but does the code fail to re-enable the U3
port when transitioning from DP-only mode back to DP+USB mode?

When transitioning into UDPHY_MODE_DP, rk_udphy_power_on() disables the
U3 port. However, when transitioning back to a mode supporting USB and
hitting the udphy->phy_needs_reinit path, it merely calls rk_udphy_init()
without calling rk_udphy_u3_port_disable(udphy, false) first.

Would this omission cause a permanent regression for USB 3.0 functionality
even if the PHY hardware was successfully re-initialized?

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

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