[PATCH v17 0/7] phy: rockchip: usbdp: Fix runtime muxing between USB3 and DP

[PATCH v17 0/7] phy: rockchip: usbdp: Fix runtime muxing between USB3 and DP

[Sebastian Reichel]

This (final) cleanup part depends on new DWC3 glue driver series:

https://lore.kernel.org/linux-phy/20261114-b4-rockchip-dwc3-rockchip-glue-v19-0-b79fb5f4b42a@collabora.com/

Changes in v17:
- Link to v16: https://patch.msgid.link/20260930-b4-rockchip-usbdp-cleanup-part3-v16-0-bc144f80afc4@collabora.com
- Rebase to v7.4-rc1 and v19 of the glue driver series. This drops
  the first patch, as the PHY reset notifier has been replaced by
  direct calls added in the glue series, which now also introduces
  sw_mode.
- Add one more patch to gracefully handle DP lane remuxing after cable
  orientation change when the DP controller kept the display output (and
  thus PHY) enabled even with HPD=0. This can apparently happen with
  some DRM compositors.

Changes in v16:
- Link to v15: https://patch.msgid.link/20260915-b4-rockchip-usbdp-cleanup-part3-v15-0-9ad90a456953@collabora.com
- Split out "Disable USB3 port during normal power-on" from first patch
- Collect R-b tags from Mani

Changes in v15:
 - Link to v14: https://patch.msgid.link/20260813-rockchip-usbdp-cleanup-v14-0-b5ad9c68fa11@collabora.com
 - Split the series, no patch changes in this part

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Sebastian Reichel (7):
      phy: rockchip: usbdp: Disable USB3 port during normal power-on
      phy: rockchip: usbdp: Drop -EPROBE_DEFER hack
      phy: rockchip: usbdp: Rename mode to hw_mode
      phy: rockchip: usbdp: Fix power state handling
      phy: rockchip: usbdp: Re-init PHY on mux change
      phy: rockchip: usbdp: Add USB-C state without DP enabled
      phy: rockchip: usbdp: Add DP lane setup to power state update

 drivers/phy/rockchip/phy-rockchip-usbdp.c | 223 +++++++++++++++++-------------
 1 file changed, 125 insertions(+), 98 deletions(-)
---
base-commit: bc71431221f5ba8bb33918399552636e059c2c45
change-id: 20260914-b4-rockchip-usbdp-cleanup-part3-cca70383ebe3
prerequisite-message-id: 20261114-b4-rockchip-dwc3-rockchip-glue-v19-0-b79fb5f4b42a@collabora.com
prerequisite-patch-id: 4fa3aae8ccfa5383522070179c8777302411082c
prerequisite-patch-id: 0cfab94fd67b6304cd303a9810981aaca328c651
prerequisite-patch-id: c2e1c453454a7589a691fe391dc79e86f09ca7b7
prerequisite-patch-id: dadeaf9e369e04d8b3329493a1a0756ddd6140f0
prerequisite-patch-id: 2c727b8400980c760ca8f1e5fd989a3bf9404020
prerequisite-patch-id: cf4e7d51c77a5eccda6c687730eddd3135d94d91

Best regards,
--  
Sebastian Reichel <sebastian.reichel@collabora.com>


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

[PATCH v17 1/7] phy: rockchip: usbdp: Disable USB3 port during normal power-on

[Sebastian Reichel]

In the reset path, the USB3 port is disabled. This re-routes the clock
going from the PHY to the USB3 controller. This adds the same to the
normal power-on path, so that it behaves exactly the same (except that
the clocks routed to the PHY stay enabled in the reset path).

This hasn't been done so far, as it requires the recently introduced
PHY reset infrastructure to avoid the PHY PLL not locking during
startup.

Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 9c6d921b9191..d01a819bf794 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1051,6 +1051,9 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
 	if (udphy->status == UDPHY_MODE_NONE) {
 		rk_udphy_run_reset_handler(udphy, true);
 
+		rk_udphy_u3_port_disable(udphy, true);
+		udelay(10);
+
 		ret = rk_udphy_setup(udphy);
 		if (ret) {
 			rk_udphy_run_reset_handler(udphy, false);

-- 
2.53.0


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

[PATCH v17 2/7] phy: rockchip: usbdp: Drop -EPROBE_DEFER hack

[Sebastian Reichel]

The hack to return -EPROBE_DEFER when the lcpll lock timeouts is no
longer needed. The driver now does a reset during its PHY init, which
avoids the problem.

Since rk_udphy_status_check() is called after the probe, it should not
return -EPROBE_DEFER.

Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index d01a819bf794..025c8d52d72c 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -754,17 +754,7 @@ static int rk_udphy_status_check(struct rk_udphy *udphy)
 					       (val & CMN_ANA_LCPLL_LOCK_DONE), 200, 100000);
 		if (ret) {
 			dev_err(udphy->dev, "cmn ana lcpll lock timeout\n");
-			/*
-			 * If earlier software (U-Boot) enabled USB once already
-			 * the PLL may have problems locking on the first try.
-			 * It will be successful on the second try, so for the
-			 * time being a -EPROBE_DEFER will solve the issue.
-			 *
-			 * This requires further investigation to understand the
-			 * root cause, especially considering that the driver is
-			 * asserting all reset lines at probe time.
-			 */
-			return -EPROBE_DEFER;
+			return ret;
 		}
 
 		if (!udphy->flip) {

-- 
2.53.0


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

[PATCH v17 3/7] phy: rockchip: usbdp: Rename mode to hw_mode

[Sebastian Reichel]

Rename mode field to hw_mode to make clear that this is the modes
currently supported by the hardware, but not necessarily requested
by software. I.e. it is only set by either the USB-C state machine
or device-tree if the PHY is used in a fixed routing setup.

Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 48 +++++++++++++++----------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 025c8d52d72c..de8f01bd0467 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -174,7 +174,7 @@ struct rk_udphy {
 	/* PHY status management */
 	bool flip;
 	bool phy_needs_reinit;
-	u8 mode;
+	u8 hw_mode; /* modes currently supported by hardware */
 	u8 status;
 	u8 sw_mode; /* modes currently requested */
 
@@ -583,18 +583,18 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
 				   CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
 }
 
-static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
+static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode)
 {
-	if (udphy->mode == mode)
+	if (udphy->hw_mode == hw_mode)
 		return;
 
 	udphy->phy_needs_reinit = true;
-	udphy->mode = mode;
+	udphy->hw_mode = hw_mode;
 }
 
 static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
 {
-	u8 mode;
+	u8 hw_mode;
 
 	switch (state) {
 	case TYPEC_DP_STATE_C:
@@ -603,7 +603,7 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
 		udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
 		udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
 		udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
-		mode = UDPHY_MODE_DP;
+		hw_mode = UDPHY_MODE_DP;
 		udphy->dp_lanes = 4;
 		break;
 
@@ -620,12 +620,12 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
 			udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
 			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
 		}
-		mode = UDPHY_MODE_DP_USB;
+		hw_mode = UDPHY_MODE_DP_USB;
 		udphy->dp_lanes = 2;
 		break;
 	}
 
-	rk_udphy_mode_set(udphy, mode);
+	rk_udphy_mode_set(udphy, hw_mode);
 }
 
 static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
@@ -748,7 +748,7 @@ static int rk_udphy_status_check(struct rk_udphy *udphy)
 	int ret;
 
 	/* LCPLL check */
-	if (udphy->mode & UDPHY_MODE_USB) {
+	if (udphy->hw_mode & UDPHY_MODE_USB) {
 		ret = regmap_read_poll_timeout(udphy->pma_regmap, CMN_ANA_LCPLL_DONE_OFFSET,
 					       val, (val & CMN_ANA_LCPLL_AFC_DONE) &&
 					       (val & CMN_ANA_LCPLL_LOCK_DONE), 200, 100000);
@@ -783,15 +783,15 @@ static int rk_udphy_init(struct rk_udphy *udphy)
 	int ret;
 
 	dev_dbg(udphy->dev, "reinit PHY with USB3=%s and DP=%s (%u lanes) flipped=%s\n",
-		str_on_off(udphy->mode & UDPHY_MODE_USB),
-		str_on_off(udphy->mode & UDPHY_MODE_DP),
+		str_on_off(udphy->hw_mode & UDPHY_MODE_USB),
+		str_on_off(udphy->hw_mode & UDPHY_MODE_DP),
 		udphy->dp_lanes, str_yes_no(udphy->flip));
 
 	rk_udphy_reset_assert_all(udphy);
 	usleep_range(10000, 11000);
 
 	/* enable rx lfps for usb */
-	if (udphy->mode & UDPHY_MODE_USB)
+	if (udphy->hw_mode & UDPHY_MODE_USB)
 		rk_udphy_grfreg_write(udphy->udphygrf, &cfg->grfcfg.rx_lfps, true);
 
 	/* Step 1: power on pma and deassert apb rstn */
@@ -828,13 +828,13 @@ static int rk_udphy_init(struct rk_udphy *udphy)
 			   FIELD_PREP(CMN_DP_LANE_EN_ALL, 0));
 
 	/* Step 4: deassert init rstn and wait for 200ns from datasheet */
-	if (udphy->mode & UDPHY_MODE_USB) {
+	if (udphy->hw_mode & UDPHY_MODE_USB) {
 		ret = rk_udphy_reset_deassert(udphy, "init");
 		if (ret)
 			goto assert_resets;
 	}
 
-	if (udphy->mode & UDPHY_MODE_DP) {
+	if (udphy->hw_mode & UDPHY_MODE_DP) {
 		regmap_update_bits(udphy->pma_regmap, CMN_DP_RSTN_OFFSET,
 				   CMN_DP_INIT_RSTN,
 				   FIELD_PREP(CMN_DP_INIT_RSTN, 0x1));
@@ -843,7 +843,7 @@ static int rk_udphy_init(struct rk_udphy *udphy)
 	udelay(1);
 
 	/*  Step 5: deassert cmn/lane rstn */
-	if (udphy->mode & UDPHY_MODE_USB) {
+	if (udphy->hw_mode & UDPHY_MODE_USB) {
 		ret = rk_udphy_reset_deassert(udphy, "cmn");
 		if (ret)
 			goto assert_resets;
@@ -902,7 +902,7 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
 	num_lanes = device_property_count_u32(udphy->dev, "rockchip,dp-lane-mux");
 	if (num_lanes < 0) {
 		dev_dbg(udphy->dev, "no dp-lane-mux, following dp alt mode\n");
-		udphy->mode = UDPHY_MODE_USB;
+		udphy->hw_mode = UDPHY_MODE_USB;
 		return 0;
 	}
 
@@ -931,10 +931,10 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
 		}
 	}
 
-	udphy->mode = UDPHY_MODE_DP;
+	udphy->hw_mode = UDPHY_MODE_DP;
 	udphy->dp_lanes = num_lanes;
 	if (num_lanes == 1 || num_lanes == 2) {
-		udphy->mode |= UDPHY_MODE_USB;
+		udphy->hw_mode |= UDPHY_MODE_USB;
 		udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP) ||
 			      (udphy->lane_mux_sel[1] == PHY_LANE_MUX_DP);
 	}
@@ -1033,7 +1033,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
 {
 	int ret;
 
-	if (!(udphy->mode & mode)) {
+	if (!(udphy->hw_mode & mode)) {
 		dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
 		return 0;
 	}
@@ -1050,7 +1050,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
 			return ret;
 		}
 
-		if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
+		if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
 			rk_udphy_u3_port_disable(udphy, false);
 		udphy->phy_needs_reinit = false;
 
@@ -1067,7 +1067,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
 			return ret;
 		}
 
-		if (!udphy->hs && udphy->mode & UDPHY_MODE_USB)
+		if (!udphy->hs && udphy->hw_mode & UDPHY_MODE_USB)
 			rk_udphy_u3_port_disable(udphy, false);
 		udphy->phy_needs_reinit = false;
 
@@ -1081,7 +1081,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
 
 static void rk_udphy_power_off(struct rk_udphy *udphy, u8 mode)
 {
-	if (!(udphy->mode & mode)) {
+	if (!(udphy->hw_mode & mode)) {
 		dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
 		return;
 	}
@@ -1349,7 +1349,7 @@ 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->mode & UDPHY_MODE_USB) || udphy->hs) {
+	if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs) {
 		rk_udphy_u3_port_disable(udphy, true);
 		return 0;
 	}
@@ -1372,7 +1372,7 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
 	udphy->sw_mode &= ~UDPHY_MODE_USB;
 
 	/* DP only or high-speed */
-	if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
+	if (!(udphy->hw_mode & UDPHY_MODE_USB) || udphy->hs)
 		return 0;
 
 	rk_udphy_power_off(udphy, UDPHY_MODE_USB);

-- 
2.53.0


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

[PATCH v17 4/7] phy: rockchip: usbdp: Fix power state handling

[Sebastian Reichel]

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@gmail.com> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 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


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

[PATCH v17 5/7] phy: rockchip: usbdp: Re-init PHY on mux change

[Sebastian Reichel]

Ensure that the right part of the PHY are powered up when the
mode changes. This ensures the PHY is re-initialized in the
following two scenarios, which are currently broken:

 - cable orientation changes without DP being involved
 - switching from DP-only into a mode with USB support

Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index fc3c4104dcd7..b79b6db96a0e 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1434,7 +1434,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
 
 	rk_udphy_set_typec_state(udphy, state->mode);
 
-	return 0;
+	return rk_udphy_update_power_state(udphy);
 }
 
 static void rk_udphy_typec_mux_unregister(void *data)

-- 
2.53.0


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

[PATCH v17 6/7] phy: rockchip: usbdp: Add USB-C state without DP enabled

[Sebastian Reichel]

The driver currently only differs between 4 lanes DP mode or combined DP
+ USB3 mode. This makes sense from a lane routing point of view, as the
hardware only has 2 lanes of USB3.

But adding a separate state for USB-only helps with power management,
since we always power up all PHY parts according to the current hardware
setup to avoid data stream interruptions. Even if some lanes are muxed
to the DP controller there is no need to keep the DP side enabled if
something without DP AltMode is plugged into USB-C.

This potentially triggers some more USB reconnections during the PD
AltMode negotiation when switching from USB-only to combined USB+DP
mode. This should be fine, as the cable is freshly plugged at this
point.

Tested-by: Igor Paunovic <royalnet026@gmail.com> # Orange Pi 5 Plus
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 57 ++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index b79b6db96a0e..553b88c45108 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -583,32 +583,14 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
 				   CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
 }
 
-static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode)
+static void rk_udphy_set_lane_mux(struct rk_udphy *udphy)
 {
-	if (udphy->hw_mode == hw_mode)
-		return;
-
-	udphy->phy_needs_reinit = true;
-	udphy->hw_mode = hw_mode;
-}
-
-static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
-{
-	u8 hw_mode;
-
-	switch (state) {
-	case TYPEC_DP_STATE_C:
-	case TYPEC_DP_STATE_E:
+	if (udphy->dp_lanes == 4) {
 		udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
 		udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
 		udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
 		udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
-		hw_mode = UDPHY_MODE_DP;
-		udphy->dp_lanes = 4;
-		break;
-
-	case TYPEC_DP_STATE_D:
-	default:
+	} else {
 		if (udphy->flip) {
 			udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
 			udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
@@ -620,12 +602,39 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
 			udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
 			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
 		}
-		hw_mode = UDPHY_MODE_DP_USB;
-		udphy->dp_lanes = 2;
+	}
+}
+
+static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode, u8 dp_lanes)
+{
+	if (udphy->hw_mode == hw_mode && udphy->dp_lanes == dp_lanes)
+		return;
+
+	udphy->phy_needs_reinit = true;
+	udphy->hw_mode = hw_mode;
+	udphy->dp_lanes = dp_lanes;
+}
+
+static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
+{
+	switch (state) {
+	case TYPEC_DP_STATE_C:
+	case TYPEC_DP_STATE_E:
+		rk_udphy_mode_set(udphy, UDPHY_MODE_DP, 4);
+		break;
+
+	case TYPEC_DP_STATE_D:
+		rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB, 2);
+		break;
+
+	case TYPEC_STATE_SAFE:
+	case TYPEC_STATE_USB:
+	default:
+		rk_udphy_mode_set(udphy, UDPHY_MODE_USB, 0);
 		break;
 	}
 
-	rk_udphy_mode_set(udphy, hw_mode);
+	rk_udphy_set_lane_mux(udphy);
 }
 
 static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)

-- 
2.53.0


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

[PATCH v17 7/7] phy: rockchip: usbdp: Add DP lane setup to power state update

[Sebastian Reichel]

DP lane map and AUX orientation are only configured in the DP PHY
power_on callback right now. This means when the DP controller keeps
the display enabled after a cable is unplugged, it will not come back
properly with the cable re-plugged in swapped orientation.

To fix the issue, we update the DP lane mux configuration in the power
state update. This gets some special handling, because calling
rk_udphy_dp_lane_select() accesses the VOGRF which needs the DP
controller's power domain enabled. It is active while the DP controller
has the PHY powered on.

Apart from that VOGRF may lose its state when the DP controller is
disabled, so we keep the DP PHY power hook as-is instead of fully
relying on the power state update function.

Reported-by: Yury Smirnov <yurymonzon@gmail.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-usbdp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
index 553b88c45108..d4a46078d049 100644
--- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
+++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
@@ -1108,6 +1108,13 @@ static int rk_udphy_update_power_state(struct rk_udphy *udphy)
 		}
 	}
 
+	/* Setup DP lanes and the AUX polarity */
+	if ((target_mode & UDPHY_MODE_DP) && (udphy->sw_mode & UDPHY_MODE_DP)) {
+		phy_set_bus_width(udphy->phy_dp, udphy->dp_lanes);
+		rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
+		rk_udphy_dp_lane_select(udphy);
+	}
+
 	/* Ensure USB3 support is enabled when supported */
 	if (udphy->sw_mode & UDPHY_MODE_USB)
 		rk_udphy_u3_port_disable(udphy, !usb3_port_enable);

-- 
2.53.0


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

Re: [PATCH v17 4/7] phy: rockchip: usbdp: Fix power state handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] phy: rockchip: usbdp: skipped reset handler and port disable on
  USB3 exit
- [Medium] phy: rockchip: usbdp: clock and power leak when udphy->hs is
  true
- [Low] phy: rockchip: usbdp: state desync on error in
  rk_udphy_update_power_state()
--

commit 336f724e3ffaaeb893f1fa7b7dc4865cc0240d95
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Fix power state handling

This commit replaces rk_udphy_power_on() and rk_udphy_power_off() with a
single rk_udphy_update_power_state() helper that derives the target PHY
state from udphy->hw_mode and udphy->sw_mode.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index de8f01bd04672..fc3c4104dcd7e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -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);

[Severity: High]
Does clearing UDPHY_MODE_USB from udphy->sw_mode in rk_udphy_usb3_phy_exit()
before calling rk_udphy_update_power_state() skip both
rk_udphy_run_reset_handler() and rk_udphy_u3_port_disable(udphy, true) on
USB3 teardown and PHY disable?

rk_udphy_run_reset_handler() returns early when !(udphy->sw_mode &
UDPHY_MODE_USB), and target_mode only becomes UDPHY_MODE_NONE when
udphy->sw_mode == UDPHY_MODE_NONE.  When rk_udphy_usb3_phy_exit() runs, or
whenever rk_udphy_disable() is reached, UDPHY_MODE_USB is already clear in
udphy->sw_mode, so the DWC3 reset callback is never invoked and the USB3
port is left enabled in usbgrf without re-routing the DWC3 PIPE clock,
which can trigger SErrors if DWC3 registers are accessed while the PHY is
disabled.

> +	} else {
> +		/* Mode change => re-init */
> +		ret = rk_udphy_init(udphy);
> +		if (ret) {
> +			rk_udphy_run_reset_handler(udphy, false);
> +			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);
>  }

[Severity: Low]
If a pending re-init (udphy->phy_needs_reinit) fails inside
rk_udphy_init() when rk_udphy_dp_phy_power_off() or
rk_udphy_usb3_phy_exit() calls rk_udphy_update_power_state(), can state
become desynchronized?

On failure, rk_udphy_init() asserts all PHY resets while leaving the
clocks enabled, the USB3 port disabled, and udphy->status set to the old
mode.  Propagating that error causes phy_power_off() or phy_exit() to keep
phy->power_count or phy->init_count elevated even though the caller has
already cleared its bit in udphy->sw_mode.

> @@ -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);
>  }

[Severity: Medium]
When udphy->hs is true, does setting UDPHY_MODE_USB in udphy->sw_mode and
returning early from rk_udphy_usb3_phy_init() and rk_udphy_usb3_phy_exit()
leak the PHY clocks and leave the PHY powered on after DisplayPort is used?

If rk_udphy_usb3_phy_init() runs first with udphy->hs set, udphy->sw_mode
holds UDPHY_MODE_USB while udphy->status stays UDPHY_MODE_NONE.  Powering
DisplayPort on via rk_udphy_dp_phy_power_on() then enables udphy->clks in
rk_udphy_setup(), but rk_udphy_dp_phy_power_off() sees udphy->sw_mode ==
UDPHY_MODE_USB and keeps target_mode at udphy->hw_mode instead of disabling
the PHY.  A subsequent rk_udphy_usb3_phy_exit() clears UDPHY_MODE_USB and
returns early at if (udphy->hs) without calling
rk_udphy_update_power_state(), so rk_udphy_disable() is never called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-b4-rockchip-usbdp-cleanup-part3-v17-0-1af5f566408e@collabora.com?part=4

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

Re: [PATCH v17 6/7] phy: rockchip: usbdp: Add USB-C state without DP enabled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] phy: rockchip: usbdp: default mapping in
  rk_udphy_set_typec_default_mapping() still uses TYPEC_DP_STATE_D
--

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

phy: rockchip: usbdp: Add USB-C state without DP enabled

This commit splits out lane mux setup and adds a separate USB-only state
(UDPHY_MODE_USB with 0 DP lanes) for TYPEC_STATE_SAFE and TYPEC_STATE_USB
in rk_udphy_set_typec_state().

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index b79b6db96a0e6..553b88c45108a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -583,32 +583,14 @@ static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
>  				   CMN_DP_CMN_RSTN, FIELD_PREP(CMN_DP_CMN_RSTN, 0x0));
>  }
>  
> -static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode)
> +static void rk_udphy_set_lane_mux(struct rk_udphy *udphy)
>  {
> -	if (udphy->hw_mode == hw_mode)
> -		return;
> -
> -	udphy->phy_needs_reinit = true;
> -	udphy->hw_mode = hw_mode;
> -}
> -
> -static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
> -{
> -	u8 hw_mode;
> -
> -	switch (state) {
> -	case TYPEC_DP_STATE_C:
> -	case TYPEC_DP_STATE_E:
> +	if (udphy->dp_lanes == 4) {
>  		udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
>  		udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
>  		udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
>  		udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
> -		hw_mode = UDPHY_MODE_DP;
> -		udphy->dp_lanes = 4;
> -		break;
> -
> -	case TYPEC_DP_STATE_D:
> -	default:
> +	} else {
>  		if (udphy->flip) {
>  			udphy->lane_mux_sel[0] = PHY_LANE_MUX_DP;
>  			udphy->lane_mux_sel[1] = PHY_LANE_MUX_DP;
> @@ -620,12 +602,39 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
>  			udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
>  			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>  		}
> -		hw_mode = UDPHY_MODE_DP_USB;
> -		udphy->dp_lanes = 2;
> +	}
> +}
> +
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 hw_mode, u8 dp_lanes)
> +{
> +	if (udphy->hw_mode == hw_mode && udphy->dp_lanes == dp_lanes)
> +		return;
> +
> +	udphy->phy_needs_reinit = true;
> +	udphy->hw_mode = hw_mode;
> +	udphy->dp_lanes = dp_lanes;
> +}
> +
> +static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state)
> +{
> +	switch (state) {
> +	case TYPEC_DP_STATE_C:
> +	case TYPEC_DP_STATE_E:
> +		rk_udphy_mode_set(udphy, UDPHY_MODE_DP, 4);
> +		break;
> +
> +	case TYPEC_DP_STATE_D:
> +		rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB, 2);
> +		break;
> +
> +	case TYPEC_STATE_SAFE:
> +	case TYPEC_STATE_USB:
> +	default:
> +		rk_udphy_mode_set(udphy, UDPHY_MODE_USB, 0);
>  		break;
>  	}
>  
> -	rk_udphy_mode_set(udphy, hw_mode);
> +	rk_udphy_set_lane_mux(udphy);
>  }
>  
>  static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)

[Severity: Medium]
Should rk_udphy_set_typec_default_mapping() also pass TYPEC_STATE_USB (or
TYPEC_STATE_SAFE) instead of TYPEC_DP_STATE_D?

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_set_typec_default_mapping() {
	...
	/* default to USB3 + DP as 4 lane USB is not supported */
	rk_udphy_set_typec_state(udphy, TYPEC_DP_STATE_D);
}

Because rk_udphy_orien_sw_set() calls rk_udphy_set_typec_default_mapping()
on every attached orientation update, leaving TYPEC_DP_STATE_D there flips
udphy->hw_mode to UDPHY_MODE_DP_USB (dp_lanes = 2) and sets
udphy->phy_needs_reinit = true whenever the PHY was in UDPHY_MODE_USB.

When a Type-C port driver sets orientation before mode (such as
tcpm_mux_set()), this bounces hw_mode from UDPHY_MODE_USB to
UDPHY_MODE_DP_USB and back to UDPHY_MODE_USB, forcing an unnecessary PHY
and DWC3 reset even when neither orientation nor USB-only mode changed.
When a port driver sets mode before orientation (such as
ucsi_handle_connector_change()) or only registers an orientation switch,
rk_udphy_orien_sw_set() overwrites UDPHY_MODE_USB back to
UDPHY_MODE_DP_USB and leaves the DP side enabled for USB-only connections.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-b4-rockchip-usbdp-cleanup-part3-v17-0-1af5f566408e@collabora.com?part=6

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

Re: [PATCH v17 5/7] phy: rockchip: usbdp: Re-init PHY on mux change

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] phy: rockchip: usbdp: missing PHY re-init in
  rk_udphy_orien_sw_set()
--

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

phy: rockchip: usbdp: Re-init PHY on mux change

This commit calls rk_udphy_update_power_state() at the end of
rk_udphy_typec_mux_set() so that the PHY is re-initialized when the Type-C
mux state changes.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fc3c4104dcd7e..b79b6db96a0e6 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1434,7 +1434,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  
>  	rk_udphy_set_typec_state(udphy, state->mode);
>  
> -	return 0;
> +	return rk_udphy_update_power_state(udphy);
>  }

[Severity: Medium]
Should rk_udphy_orien_sw_set() also call rk_udphy_update_power_state()?

When cable orientation changes, rk_udphy_orien_sw_set() sets
udphy->phy_needs_reinit = true and updates udphy->lane_mux_sel[] via
rk_udphy_set_typec_default_mapping(), but returns 0 without re-initializing
the PHY:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_orien_sw_set() {
    ...
	if (udphy->flip != flipped) {
		dev_dbg(udphy->dev, "cable orientation changed, PHY re-init required.\n");
		udphy->phy_needs_reinit = true;
	}

	udphy->flip = flipped;
	rk_udphy_set_typec_default_mapping(udphy);
	rk_udphy_usb_bvalid_enable(udphy, true);

	return 0;
}

On boards that specify orientation-switch without mode-switch in Device
Tree (or with Type-C controller drivers that call typec_set_orientation()
without a subsequent typec_set_mode()), rk_udphy_typec_mux_set() is not
called after an orientation change, leaving the hardware lane mux register
unprogrammed and USB3 SuperSpeed broken on a flipped cable.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-b4-rockchip-usbdp-cleanup-part3-v17-0-1af5f566408e@collabora.com?part=5

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