[PATCH] phy: qcom: qmp-combo: Leave the USB PCS alone in DP-only mode

[PATCH] phy: qcom: qmp-combo: Leave the USB PCS alone in DP-only mode

[Joonhoe Kim]

In DP-only mode (DisplayPort pin assignments C and E) all four lanes
belong to the DisplayPort PHY and the USB PCS stays powered down. The
USB controller can still exit and re-init its PHY in this state, e.g.
across a system suspend that loses power. qmp_combo_usb_init() then
programs the USB serdes, lane and PCS tables over the DisplayPort
configuration and times out waiting for PHYSTATUS, and
qmp_combo_usb_exit() resets and powers down a PCS that is not in use.

In DP-only mode only take and drop the common reference, but keep
counting the USB user, so that qmp_combo_reconfigure_phy() powers the
USB PCS up when the port leaves DP-only mode.

On a Lenovo Y700 Gen 5 (TB323FU, Kaanapali) with a USB-C monitor
in pin assignment C, resume from s2idle failed with "phy initialization
timed-out", the dwc3 resume returned -110 and the following xHCI resume
hung the system.

Fixes: 896277138c13 ("phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE")
Signed-off-by: Joonhoe Kim <26rote@gmail.com>
---
Tested on a 7.3-rc4 based tree, where the mux switch itself still drops
usb_init_count when it enters DP-only mode, so the tested change skipped
the count as well. This version follows the reworked mux path in
phy-next and is build-tested only.

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 187f0cd11f6c..6e3406d519d2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4459,10 +4459,12 @@ static int qmp_combo_usb_init(struct phy *phy)
 	if (ret)
 		return ret;
 
-	ret = qmp_combo_usb_power_on(phy);
-	if (ret) {
-		qmp_combo_com_exit(qmp, false);
-		return ret;
+	if (qmp->qmpphy_mode != QMPPHY_MODE_DP_ONLY) {
+		ret = qmp_combo_usb_power_on(phy);
+		if (ret) {
+			qmp_combo_com_exit(qmp, false);
+			return ret;
+		}
 	}
 
 	qmp->usb_init_count++;
@@ -4484,9 +4486,11 @@ static int qmp_combo_usb_exit(struct phy *phy)
 		return 0;
 	}
 
-	ret = qmp_combo_usb_power_off(phy);
-	if (ret)
-		return ret;
+	if (qmp->qmpphy_mode != QMPPHY_MODE_DP_ONLY) {
+		ret = qmp_combo_usb_power_off(phy);
+		if (ret)
+			return ret;
+	}
 
 	ret = qmp_combo_com_exit(qmp, false);
 	if (ret)

base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64
-- 
2.43.0

Re: [PATCH] phy: qcom: qmp-combo: Leave the USB PCS alone in DP-only mode

[Val Packett]

On 10/22/26 1:52 PM, Joonhoe Kim wrote:
> In DP-only mode (DisplayPort pin assignments C and E) all four lanes
> belong to the DisplayPort PHY and the USB PCS stays powered down. The
> USB controller can still exit and re-init its PHY in this state, e.g.
> across a system suspend that loses power. qmp_combo_usb_init() then
> programs the USB serdes, lane and PCS tables over the DisplayPort
> configuration and times out waiting for PHYSTATUS, and
> qmp_combo_usb_exit() resets and powers down a PCS that is not in use.
>
> In DP-only mode only take and drop the common reference, but keep
> counting the USB user, so that qmp_combo_reconfigure_phy() powers the
> USB PCS up when the port leaves DP-only mode.
>
> On a Lenovo Y700 Gen 5 (TB323FU, Kaanapali) with a USB-C monitor
> in pin assignment C, resume from s2idle failed with "phy initialization
> timed-out", the dwc3 resume returned -110 and the following xHCI resume
> hung the system.
Good catch. I don't think I've seen system hangs from this on Hamoa, but 
I've had USB stop working on the ports where 4xDP+USB2 was left 
connected during suspend..
> Fixes: 896277138c13 ("phy: qcom: qmp-combo: register a typec mux to change the QMPPHY_MODE")
> Signed-off-by: Joonhoe Kim <26rote@gmail.com>
> ---
> Tested on a 7.3-rc4 based tree, where the mux switch itself still drops
> usb_init_count when it enters DP-only mode, so the tested change skipped
> the count as well. This version follows the reworked mux path in
> phy-next and is build-tested only.
>
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 187f0cd11f6c..6e3406d519d2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4459,10 +4459,12 @@ static int qmp_combo_usb_init(struct phy *phy)
>   	if (ret)
>   		return ret;
>   
> -	ret = qmp_combo_usb_power_on(phy);
> -	if (ret) {
> -		qmp_combo_com_exit(qmp, false);
> -		return ret;
> +	if (qmp->qmpphy_mode != QMPPHY_MODE_DP_ONLY) {
> +		ret = qmp_combo_usb_power_on(phy);
> +		if (ret) {
> +			qmp_combo_com_exit(qmp, false);
> +			return ret;
> +		}
> [..]

The USB4 chunk above (juuuust before the start of this diff's context) 
does a similar thing in a different style: it increments both init_count 
and usb_init_count and early-returns, skipping over qmp_combo_com_init 
as well.

I think both USB4 and DP should be written similarly but should 
qmp_combo_com_init be skipped here? No in both cases or does USB4 skip 
it for a good reason?

(I'm especially wondering about the impact of that on the USB HS PHY 
clock, since it's actually sourced from here without a properly modeled 
dependency right now…)


Thanks,
~val

Re: [PATCH] phy: qcom: qmp-combo: Leave the USB PCS alone in DP-only mode

[Joonhoe Kim]

On 10/22/26 9:14 PM, Val Packett wrote:
> Good catch. I don't think I've seen system hangs from this on Hamoa, but
> I've had USB stop working on the ports where 4xDP+USB2 was left
> connected during suspend..

That may be the same path: on resume the USB PHY init programs the USB
tables over the DisplayPort configuration and times out. On the Y700
the xHCI resume that followed hung; a USB port that stays dead
afterwards would fit as well. A "phy initialization timed-out" from the
QMP PHY on resume would confirm it.

> The USB4 chunk above (juuuust before the start of this diff's context)
> does a similar thing in a different style: it increments both init_count
> and usb_init_count and early-returns, skipping over qmp_combo_com_init
> as well.
>
> I think both USB4 and DP should be written similarly but should
> qmp_combo_com_init be skipped here? No in both cases or does USB4 skip
> it for a good reason?
>
> (I'm especially wondering about the impact of that on the USB HS PHY
> clock, since it's actually sourced from here without a properly modeled
> dependency right now…)

DP-only needs it, and USB4 does not really skip it: the bare
init_count++ is exactly what qmp_combo_com_init(qmp, false) does when
the count is already non-zero, and qmp_combo_usb4_init() holds that
first reference. In DP-only mode nothing guarantees another holder: on
resume the USB controller can init its PHY before DisplayPort does, and
then qmp_combo_com_init() is what turns the clocks, resets and
regulators on.

So I kept the two branches different: the USB4 one is a separate early
return because it must not reprogram the PHY at all, while DP-only only
leaves the USB PCS alone. The mux path already keeps the USB PCS
powered down in DP-only mode; v2 makes usb_init/usb_exit match it, and
the common block stays on while USB holds a reference, which looks like
the safer side for the HS PHY clock.

v2 also skips the USB PCS in qmp_combo_typec_switch_set() and
qmp_combo_reconfigure_phy() in DP-only mode, as Sashiko pointed out on
v1:

https://lore.kernel.org/all/20261023123512.1912-1-26rote@gmail.com/

Thanks,
Joonhoe Kim

Re: [PATCH] phy: qcom: qmp-combo: Leave the USB PCS alone in DP-only mode

[Joonhoe Kim]

On Thu, 22 Oct 2026 17:07:04 +0000, Sashiko AI review wrote:
> - [High] phy: qcom: qmp-combo: qmp_combo_typec_switch_set() powers USB in DP-only mode
[...]
> Would this overwrite the serdes, lane, and PCS tables, time out waiting
> for PHYSTATUS, and call clk_disable_unprepare(qmp->pipe_clk) on the
> error path while qmp->init_count remains non-zero?
[...]
> Should both qmp_combo_typec_switch_set() and the power-off path in
> qmp_combo_reconfigure_phy() also skip the USB PCS when qmp->qmpphy_mode is
> QMPPHY_MODE_DP_ONLY?

Yes. Since 89563a152973 ("phy: qualcomm: qmp-combo: Add preliminary
USB4 support") the mux keeps usb_init_count when it enters DP-only
mode, so an orientation change in that mode already re-ran the USB
PCS power-on over the DisplayPort configuration before this patch. v2
skips the USB PCS in qmp_combo_typec_switch_set() as well.

The power-off in qmp_combo_reconfigure_phy() when leaving DP-only mode
only asserts SW_RESET, clears the start bits and puts the PCS into
power-down, which it already is in DP-only mode, so it is harmless, but
v2 skips it too for consistency.

The pipe_clk handling on the error path of qmp_combo_usb_power_on() is
a separate issue: pipe_clk is enabled in qmp_combo_com_init() and
disabled in qmp_combo_com_exit(), so disabling it on a PHYSTATUS
timeout leaves it unbalanced whatever the mode. This patch does not
change that.

v2: https://lore.kernel.org/all/20261023123512.1912-1-26rote@gmail.com/

Thanks,
Joonhoe Kim