Re: [PATCH v8 29/29] phy: rockchip: usbdp: Disable USB3 on probe - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: devicetree@vger.kernel.org, linux-phy@lists.infradead.org,
	vkoul@kernel.org, robh@kernel.org, neil.armstrong@linaro.org,
	olteanv@gmail.com, conor+dt@kernel.org
Subject: Re: [PATCH v8 29/29] phy: rockchip: usbdp: Disable USB3 on probe
Date: Fri, 26 Jun 2026 21:24:56 +0000	[thread overview]
Message-ID: <20260626212456.8FE1C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-29-47f682987895@collabora.com>

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing USB3 port disable in the power-off path recreates the U-Boot failure state on subsequent initialization, which will now permanently fail due to the removal of the -EPROBE_DEFER retry mechanism.
--

Note: The format of this report has been altered to use a free-form summary
due to strict anti-recitation policies preventing direct quoting of the patch diff.

commit 8c774704e1cc3fa0c1dd6e73117e4641a08efb29
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Disable USB3 on probe

This commit disables USB3 support during probe to start with a clean state,
and removes the -EPROBE_DEFER retry mechanism for cmn ana lcpll lock timeouts
since they should only happen on the first try if U-Boot had enabled USB.

[Severity: High]
This isn't a bug introduced by this patch, but removing the -EPROBE_DEFER
retry mechanism makes a pre-existing power-off issue permanently fatal.

In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off(), when the
PHY transitions to UDPHY_MODE_NONE, the code disables the PHY but does not
disable the USB3 port in the GRF:

rk_udphy_power_off()
    udphy->status &= ~mode;
    if (udphy->status == UDPHY_MODE_NONE)
        rk_udphy_disable(udphy);

When the USB subsystem resumes from suspend or driver unbind/bind, it calls
rk_udphy_power_on(). Since udphy->status is UDPHY_MODE_NONE, it calls
rk_udphy_setup(), which executes the PLL lock:

rk_udphy_power_on()
    if (udphy->status == UDPHY_MODE_NONE) {
        ret = rk_udphy_setup(udphy);

Because the USB3 port was never disabled in the GRF during power-off, this
recreates the exact unclean state that caused the U-Boot lock timeout. Without
the -EPROBE_DEFER retry mechanism removed by this patch, the PLL lock will
time out and result in a permanent failure to resume.

Should rk_udphy_power_off() also disable the USB3 port by calling
rk_udphy_u3_port_disable(udphy, true) when the status reaches UDPHY_MODE_NONE,
so the next power-on starts with a clean state?

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

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