[PATCH v11 34/36] phy: rockchip: usbdp: Fix power state handling
Sebastian Reichel
sebastian.reichel at collabora.com
Thu Jul 9 12:02:57 PDT 2026
Restructure power state handling by introducing sw_mode in addition
to the hw_mode field, so that the PHY knows about the currently
supported modes from the hardware perspective, the current modes
requested by software and the actual hardware status.
Now anything updating either the hardware or software state can simply
update the status field and call rk_udphy_update_power_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.
Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.com>
---
drivers/phy/rockchip/phy-rockchip-usbdp.c | 144 ++++++++++++++++++------------
1 file changed, 87 insertions(+), 57 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index e2cd72643a7d..c5ad300620a6 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -173,9 +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; /* current PHY power state */
/* utilized for USB */
bool hs; /* flag for high-speed */
@@ -985,66 +986,86 @@ static int rk_udphy_parse_dt(struct rk_udphy *udphy)
return rk_udphy_reset_init(udphy, dev);
}
-static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
+static int rk_udphy_update_power_state(struct rk_udphy *udphy)
{
+ 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;
+
+ if (!udphy->phy_needs_reinit && udphy->status == target_mode)
return 0;
- }
- if (udphy->status == UDPHY_MODE_NONE) {
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+ /* Avoid to re-init disabled PHY */
+ if (udphy->status == target_mode && target_mode == UDPHY_MODE_NONE)
+ return 0;
- rk_udphy_u3_port_disable(udphy, true);
- udelay(10);
+ /*
+ * 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.
+ */
+ ret = phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
+ if (ret)
+ return ret;
+
+ /*
+ * 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.
+ */
+ 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)
+ if (ret) {
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
return ret;
-
- if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
- rk_udphy_u3_port_disable(udphy, false);
- udphy->phy_needs_reinit = false;
-
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
- } else if (udphy->phy_needs_reinit) {
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_PRE_RESET);
-
- 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) {
phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
return ret;
}
-
- phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
-
- udphy->phy_needs_reinit = false;
}
- udphy->status |= mode;
+ /* Ensure USB3 support is enabled when supported */
+ if (!udphy->hs && target_mode & UDPHY_MODE_USB)
+ rk_udphy_u3_port_disable(udphy, false);
- 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;
- }
-
- 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.
+ */
+ phy_notify_reset(udphy->phy_u3, PHY_NOTIFY_POST_RESET);
- 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)
@@ -1053,11 +1074,15 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
int ret;
scoped_guard(mutex, &udphy->mutex) {
+ udphy->sw_mode |= UDPHY_MODE_DP;
+
phy_set_bus_width(phy, udphy->dp_lanes);
- ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
- if (ret)
+ ret = rk_udphy_update_power_state(udphy);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
return ret;
+ }
rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
@@ -1080,10 +1105,10 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
guard(mutex)(&udphy->mutex);
- rk_udphy_dp_lane_enable(udphy, 0);
- rk_udphy_power_off(udphy, UDPHY_MODE_DP);
+ udphy->sw_mode &= ~UDPHY_MODE_DP;
- return 0;
+ rk_udphy_dp_lane_enable(udphy, 0);
+ return rk_udphy_update_power_state(udphy);
}
/*
@@ -1288,16 +1313,24 @@ static const struct phy_ops rk_udphy_dp_phy_ops = {
static int rk_udphy_usb3_phy_init(struct phy *phy)
{
struct rk_udphy *udphy = phy_get_drvdata(phy);
+ int ret;
guard(mutex)(&udphy->mutex);
- /* 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;
}
- return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
+ udphy->sw_mode |= UDPHY_MODE_USB;
+
+ ret = rk_udphy_update_power_state(udphy);
+ if (ret) {
+ udphy->sw_mode &= ~UDPHY_MODE_USB;
+ return ret;
+ }
+
+ return 0;
}
static int rk_udphy_usb3_phy_exit(struct phy *phy)
@@ -1306,15 +1339,12 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
guard(mutex)(&udphy->mutex);
- /* DP only or high-speed */
- if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
- udphy->status &= ~UDPHY_MODE_USB;
+ if (udphy->hs)
return 0;
- }
- rk_udphy_power_off(udphy, UDPHY_MODE_USB);
+ udphy->sw_mode &= ~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
More information about the linux-phy
mailing list