[PATCH v17 4/7] phy: rockchip: usbdp: Fix power state handling
From: Sebastian Reichel
Date: Fri Nov 13 2026 - 21:01:23 EST
Restructure power state handling, so that the function uses sw_mode and
hw_mode to figure out the actual required hardware state. This allows
any path updating either of those modes to simply call
rk_udphy_update_power_state() to get the right hardware state.
This makes it a lot more obvious what is going on and also fixes a few
potential resource leaks identified by Sashiko as a side-effect. For
example if USB3 is requested by software while the USB-C is in DP-only
mode, things are decently handled after this.
Tested-by: Igor Paunovic <royalnet026@xxxxxxxxx> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 122 +++++++++++++++++-------------
1 file changed, 70 insertions(+), 52 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index de8f01bd0467..fc3c4104dcd7 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -173,10 +173,10 @@ struct rk_udphy {
/* PHY status management */
bool flip;
- bool phy_needs_reinit;
+ bool phy_needs_reinit; /* lane mux changed */
u8 hw_mode; /* modes currently supported by hardware */
- u8 status;
u8 sw_mode; /* modes currently requested */
+ u8 status;
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -1029,70 +1029,93 @@ void rk_udphy_register_reset_handler(struct phy *phy,
}
EXPORT_SYMBOL_GPL(rk_udphy_register_reset_handler);
-static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
+static int rk_udphy_update_power_state(struct rk_udphy *udphy)
{
+ bool usb3_port_enable;
+ u8 target_mode;
int ret;
- if (!(udphy->hw_mode & mode)) {
- dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
+ /*
+ * Initialize PHY mode according to the hardware setup (either described
+ * in DT or negotiated via the Type-C controller) instead of requesting
+ * only the needed PHY side, because that would break the USB/DP data
+ * streams when the other PHY is being requested. This is not an issue
+ * during the Type-C negotiation as that happens during the hotplug phase
+ * and not during normal operation. Also disable everything if the
+ * software has not requested anything, as there shouldn't be any active
+ * data streams in that case.
+ */
+ target_mode = udphy->hw_mode;
+ if (udphy->sw_mode == UDPHY_MODE_NONE)
+ target_mode = UDPHY_MODE_NONE;
+
+ usb3_port_enable = !udphy->hs && (target_mode & UDPHY_MODE_USB);
+
+ if (!udphy->phy_needs_reinit && udphy->status == target_mode) {
+ if (udphy->sw_mode & UDPHY_MODE_USB)
+ rk_udphy_u3_port_disable(udphy, !usb3_port_enable);
return 0;
}
- if (udphy->status == UDPHY_MODE_NONE) {
- rk_udphy_run_reset_handler(udphy, true);
+ /* Avoid to re-init disabled PHY */
+ if (udphy->status == target_mode && target_mode == UDPHY_MODE_NONE)
+ return 0;
+
+ /*
+ * Inform DWC3 driver, that we are about to reset the PHY, so that it can
+ * assert its PIPE reset lines and avoid DWC3 getting into a buggy state.
+ * This is intentionally done for a PHY disable, since that also changes
+ * the clocks routed to the PHY.
+ */
+ rk_udphy_run_reset_handler(udphy, true);
+ /*
+ * Disable USB3 port, which among other things re-routes a DWC3 clock to
+ * avoid SErrors when the DWC3 registers are accessed while the PHY is
+ * disabled. This is only done, when the DWC3 is running as the accessed
+ * GRF registers and in PD_USB.
+ */
+ if (udphy->sw_mode & UDPHY_MODE_USB) {
rk_udphy_u3_port_disable(udphy, true);
udelay(10);
+ }
+ if (udphy->status == UDPHY_MODE_NONE) {
+ /* Power up (incl. clocks) */
ret = rk_udphy_setup(udphy);
if (ret) {
rk_udphy_run_reset_handler(udphy, false);
return ret;
}
-
- if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
- rk_udphy_u3_port_disable(udphy, false);
- udphy->phy_needs_reinit = false;
-
- rk_udphy_run_reset_handler(udphy, false);
- } else if (udphy->phy_needs_reinit) {
- rk_udphy_run_reset_handler(udphy, true);
-
- rk_udphy_u3_port_disable(udphy, true);
- udelay(10);
-
+ } else if (target_mode == UDPHY_MODE_NONE) {
+ /* Power down (incl. clocks) */
+ rk_udphy_disable(udphy);
+ } else {
+ /* Mode change => re-init */
ret = rk_udphy_init(udphy);
if (ret) {
rk_udphy_run_reset_handler(udphy, false);
return ret;
}
-
- if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
- rk_udphy_u3_port_disable(udphy, false);
- udphy->phy_needs_reinit = false;
-
- rk_udphy_run_reset_handler(udphy, false);
}
- udphy->status |= mode;
-
- return 0;
-}
-
-static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
-{
- if (!(udphy->hw_mode & mode)) {
- dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
- return;
- }
+ /* Ensure USB3 support is enabled when supported */
+ if (udphy->sw_mode & UDPHY_MODE_USB)
+ rk_udphy_u3_port_disable(udphy, !usb3_port_enable);
- if (!udphy->status)
- return;
+ /*
+ * Inform DWC3, that we are done with the reset, so that it can deassert
+ * its PIPE reset line. This is sent in pair with a PRE_RESET allowing
+ * consumer driver to do paired resource requests (e.g. clocks) in their
+ * notification handlers. As we reroute the clocks, its also fine to
+ * send this after completely disabling the PHY.
+ */
+ rk_udphy_run_reset_handler(udphy, false);
- udphy->status &= ~mode;
+ udphy->status = target_mode;
+ udphy->phy_needs_reinit = false;
- if (udphy->status == UDPHY_MODE_NONE)
- rk_udphy_disable(udphy);
+ return 0;
}
static int rk_udphy_dp_phy_power_on(struct phy *phy)
@@ -1105,7 +1128,7 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
phy_set_bus_width(phy, udphy->dp_lanes);
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
+ ret = rk_udphy_update_power_state(udphy);
if (ret) {
udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
@@ -1135,9 +1158,7 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
udphy->sw_mode &= ~UDPHY_MODE_DP;
rk_udphy_dp_lane_enable(udphy, 0);
- rk_udphy_power_off(udphy, UDPHY_MODE_DP);
-
- return 0;
+ return rk_udphy_update_power_state(udphy);
}
/*
@@ -1349,12 +1370,12 @@ static int rk_udphy_usb3_phy_init(struct phy *phy)
udphy->sw_mode |= UDPHY_MODE_USB;
/* DP only or high-speed, disable U3 port */
- if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
+ if (udphy->hs) {
rk_udphy_u3_port_disable(udphy, true);
return 0;
}
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ ret = rk_udphy_update_power_state(udphy);
if (ret) {
udphy->sw_mode &= ~UDPHY_MODE_USB;
return ret;
@@ -1371,13 +1392,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
udphy->sw_mode &= ~UDPHY_MODE_USB;
- /* DP only or high-speed */
- if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs)
+ if (udphy->hs)
return 0;
- rk_udphy_power_off(udphy, UDPHY_MODE_USB);
-
- return 0;
+ return rk_udphy_update_power_state(udphy);
}
static const struct phy_ops rk_udphy_usb3_phy_ops = {
--
2.53.0