Re: [PATCH v8 03/29] phy: rockchip: usbdp: Do not lose USB3 PHY status - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: robh@kernel.org, conor+dt@kernel.org,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	vkoul@kernel.org, neil.armstrong@linaro.org, olteanv@gmail.com
Subject: Re: [PATCH v8 03/29] phy: rockchip: usbdp: Do not lose USB3 PHY status
Date: Fri, 26 Jun 2026 21:16:23 +0000	[thread overview]
Message-ID: <20260626211623.7A1B61F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-3-47f682987895@collabora.com>

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

Pre-existing issues:
- [High] USB3 reverse orientation changes are never programmed into the PHY hardware because `rk_udphy_orien_sw_set` updates the software structs but fails to trigger a hardware re-initialization.
--

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

phy: rockchip: usbdp: Do not lose USB3 PHY status

This commit fixes an issue where the PHY status is lost during a DisplayPort
mode change. It ensures that the tracking information for USB3 is preserved
so the PHY isn't inadvertently disabled when DP is toggled off.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a5ffdf7a6e76b..12dfdd470a782 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1008,7 +1008,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>  			rk_udphy_u3_port_disable(udphy, false);
>  		udphy->mode_change = false;
>  	} else if (udphy->mode_change) {
> -		udphy->status = UDPHY_MODE_NONE;

[Severity: High]
This isn't a bug introduced by this patch, but does keeping the USB mode bit
in the status flag prevent USB3 reverse orientation changes from being
programmed into the PHY hardware?

When a USB-C cable is flipped, the orientation switch handler 
rk_udphy_orien_sw_set() updates the software state and requests a re-init:

    udphy->phy_needs_reinit = true;
    udphy->flip = flipped;
    rk_udphy_set_typec_default_mapping(udphy);

Later, rk_udphy_typec_mux_set() evaluates whether to apply this to the
hardware. However, it only calls rk_udphy_power_on() if the USB mode is not
already set in the status flag:

    if (... && !(udphy->status & UDPHY_MODE_USB) && ...)
        return rk_udphy_power_on(udphy, UDPHY_MODE_USB);

Since DWC3 keeps the PHY enabled permanently and the status flag retains
the USB mode, it appears rk_udphy_power_on() is never invoked to apply the
new lane mapping.

Could this cause the USB3 port to silently fail when plugged in reverse
orientation, since the hardware is never reconfigured?

>  		if (udphy->mode == UDPHY_MODE_DP)
>  			rk_udphy_u3_port_disable(udphy, true);
>

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

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