[PATCH 0/3] usb: dwc3: qcom: Keep USB 2.0 working in DisplayPort-only modes

[PATCH 0/3] usb: dwc3: qcom: Keep USB 2.0 working in DisplayPort-only modes

[Joonhoe Kim]

With a USB-C monitor in DisplayPort pin assignment C or E, the QMP
USB3-DP combo PHY gives all four lanes to DisplayPort and keeps its USB
PCS powered down. The USB 2.0 pair still works, and such monitors often
have a USB 2.0 hub behind them, but the dwc3 controller keeps depending on
the USB3 PIPE clock: on Kaanapali the next xHCI reset in this state
times out ("Host halt failed, -110") and USB is gone until the cable is
replugged. Commit f546912bcac6 ("phy: qcom: qmp-combo: Move pipe_clk
on/off to common") keeps the pipe clock branch enabled, which is not
enough here.

This series lets the controller be a Type-C mode switch next to the PHY
and moves it to the UTMI clock, the way "qcom,select-utmi-as-pipe-clk"
does statically, before the PHY switches to DP-only mode.

On Kaanapali the PHY becomes a mode switch with Balaji Kalkonda's
DisplayPort series [1]; patch 3 is the controller side for the same
SoC. Other SoCs whose PHY is already a mode switch (e.g. SM8650,
SM8750) may need the same property on their controller node (not
tested).

Related fixes sent separately: "phy: qcom: qmp-combo: Leave the USB PCS
alone in DP-only mode" (suspend in DP-only mode) [2] and "usb: typec:
mux: fsa4480: Keep the USB 2.0 switch on in DP-only modes" (boards with
an FSA4480 on the USB 2.0 pair) [3].

Patch 2 conflicts with Vladyslav Tymoshchuk's "usb: dwc3: qcom: Defer
VBUS override update during system suspend" [4] in struct dwc3_qcom and
dwc3_qcom_set_role_notifier(). On top of [4], the
dwc3_qcom_update_pipe_clk() call goes after its pm_suspended check, as
it also writes QSCRATCH.

Patches 1-2 are for the USB tree. Patch 3 must not be applied before
patch 2: with "mode-switch" on the controller and no mux registered by
dwc3-qcom, pmic_glink_altmode defers its probe indefinitely.

Tested on a Lenovo Y700 Gen 5 (TB323FU, Kaanapali) with a USB-C
monitor over 4-lane HBR3: the monitor's USB 2.0 hub works at boot,
after replugs and across s2idle (tested on v7.3 with these patches,
[2] and [3]).

[1] https://lore.kernel.org/all/20261022-kaanapali_dp-v3-0-b5ae8e6083c3@oss.qualcomm.com/
[2] https://lore.kernel.org/all/20261023123512.1912-1-26rote@gmail.com/
[3] https://lore.kernel.org/all/20261022165143.1327-1-26rote@gmail.com/
[4] https://lore.kernel.org/all/20261018143126.3978-1-tvlad744@gmail.com/

Joonhoe Kim (3):
  dt-bindings: usb: qcom,snps-dwc3: Add mode-switch
  usb: dwc3: qcom: Use the UTMI clock as PIPE clock in DisplayPort-only
    modes
  arm64: dts: qcom: kaanapali: Mark the USB controller as a Type-C mode
    switch

 .../bindings/usb/qcom,snps-dwc3.yaml          |  7 ++
 arch/arm64/boot/dts/qcom/kaanapali.dtsi       |  1 +
 drivers/usb/dwc3/dwc3-qcom.c                  | 94 +++++++++++++++++--
 3 files changed, 94 insertions(+), 8 deletions(-)


base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
-- 
2.43.0

[PATCH 1/3] dt-bindings: usb: qcom,snps-dwc3: Add mode-switch

[Joonhoe Kim]

The QMP USB3-DP combo PHY stops the USB3 PIPE clock when a Type-C
DisplayPort pin assignment gives it all four lanes for DisplayPort,
while the controller can keep running its USB 2.0 port. The controller
has to move to the UTMI clock before that happens, so it needs the
Type-C mux state as well. fwnode_typec_mux_get() only matches graph
nodes that have the "mode-switch" property, so allow it on the
controller node.

Signed-off-by: Joonhoe Kim <26rote@gmail.com>
---
 Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
index b77c02c64d45..1380b769c73c 100644
--- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
@@ -158,6 +158,13 @@ properties:
       HS/FS/LS modes are supported.
     type: boolean
 
+  mode-switch:
+    description:
+      Flag the controller as a USB Type-C mode switch, so it is notified
+      of altmode changes. In pin assignments that give all SuperSpeed
+      lanes to DisplayPort the USB3 PHY stops its PIPE clock.
+    type: boolean
+
   wakeup-source: true
 
 required:
-- 
2.43.0

[PATCH 2/3] usb: dwc3: qcom: Use the UTMI clock as PIPE clock in DisplayPort-only modes

[Joonhoe Kim]

With DisplayPort pin assignment C or E the QMP USB3-DP combo PHY gives
all four lanes to DisplayPort and powers its USB PCS down, while the
controller can still run USB 2.0, e.g. for the hub of a USB-C monitor.
Keeping the pipe clock branch enabled, as commit f546912bcac6 ("phy:
qcom: qmp-combo: Move pipe_clk on/off to common") does, is not enough on
Kaanapali: the next xHCI reset in this mode times out ("Host halt
failed, -110") and USB stops working until the cable is replugged.

Register the glue as a Type-C mode switch when the node has the
"mode-switch" property, and select the UTMI clock as PIPE clock in
DP-only modes, as "qcom,select-utmi-as-pipe-clk" does statically.
typec_mux_set() calls the switches in connector endpoint order, and the
controller's HS endpoint precedes the PHY's SS endpoint, so the
controller switches before the PHY powers its USB side down.

Switch back to the PIPE clock only on a role change after the mux has
gone from the safe state to a USB state: on unplug the PHY stays in
DP-only mode while DisplayPort is powered, and the role and mux
notifications come from different pmic_glink services in no fixed
order.

Signed-off-by: Joonhoe Kim <26rote@gmail.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 94 +++++++++++++++++++++++++++++++++---
 1 file changed, 86 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ac68b4218b56..4919deae682d 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -19,6 +19,9 @@
 #include <linux/iopoll.h>
 #include <linux/usb/hcd.h>
 #include <linux/usb.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_mux.h>
 #include "core.h"
 #include "glue.h"
 
@@ -85,6 +88,13 @@ struct dwc3_qcom {
 	struct icc_path		*icc_path_apps;
 
 	enum usb_role		current_role;
+	bool			ignore_pipe_clk;
+	bool			dp_active;
+	bool			dp_only;
+	bool			utmi_pipe_clk;
+	/* protects dp_active, dp_only and utmi_pipe_clk */
+	struct mutex		pipe_clk_lock;
+	struct typec_mux_dev	*mux;
 };
 
 #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
@@ -419,21 +429,77 @@ static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom)
+static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom, bool utmi)
 {
-	/* Configure dwc3 to use UTMI clock as PIPE clock not present */
 	dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
 			  PIPE_UTMI_CLK_DIS);
 
 	usleep_range(100, 1000);
 
-	dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
-			  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
+	if (utmi)
+		dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
+				  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
+	else
+		dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
+				  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
 
 	usleep_range(100, 1000);
 
 	dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
 			  PIPE_UTMI_CLK_DIS);
+
+	qcom->utmi_pipe_clk = utmi;
+}
+
+static void dwc3_qcom_update_pipe_clk(struct dwc3_qcom *qcom)
+{
+	if (qcom->ignore_pipe_clk || qcom->dp_only == qcom->utmi_pipe_clk)
+		return;
+
+	dwc3_qcom_select_utmi_clk(qcom, qcom->dp_only);
+}
+
+static int dwc3_qcom_mux_set(struct typec_mux_dev *mux,
+			     struct typec_mux_state *state)
+{
+	struct dwc3_qcom *qcom = typec_mux_get_drvdata(mux);
+
+	guard(mutex)(&qcom->pipe_clk_lock);
+
+	if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
+		qcom->dp_active = true;
+		qcom->dp_only = state->mode == TYPEC_DP_STATE_C ||
+				state->mode == TYPEC_DP_STATE_E;
+	} else if (state->mode == TYPEC_STATE_SAFE) {
+		qcom->dp_active = false;
+	} else if (!qcom->dp_active) {
+		qcom->dp_only = false;
+	}
+
+	/* The PHY stops the PIPE clock right after this in DP-only mode */
+	if (!qcom->dp_only || pm_runtime_resume_and_get(qcom->dev))
+		return 0;
+
+	dwc3_qcom_update_pipe_clk(qcom);
+	pm_runtime_put_autosuspend(qcom->dev);
+
+	return 0;
+}
+
+static int dwc3_qcom_mux_register(struct dwc3_qcom *qcom)
+{
+	struct typec_mux_desc desc = {};
+
+	if (!device_property_present(qcom->dev, "mode-switch"))
+		return 0;
+
+	desc.drvdata = qcom;
+	desc.fwnode = dev_fwnode(qcom->dev);
+	desc.set = dwc3_qcom_mux_set;
+
+	qcom->mux = typec_mux_register(qcom->dev, &desc);
+
+	return PTR_ERR_OR_ZERO(qcom->mux);
 }
 
 static int dwc3_qcom_request_irq(struct dwc3_qcom *qcom, int irq,
@@ -570,6 +636,10 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
 		return;
 	}
 
+	mutex_lock(&qcom->pipe_clk_lock);
+	dwc3_qcom_update_pipe_clk(qcom);
+	mutex_unlock(&qcom->pipe_clk_lock);
+
 	if (qcom->current_role == USB_ROLE_DEVICE)
 		dwc3_qcom_vbus_override_enable(qcom, false);
 	else if (qcom->current_role != USB_ROLE_DEVICE)
@@ -615,7 +685,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	struct resource		res;
 	struct resource		*r;
 	int			ret;
-	bool			ignore_pipe_clk;
 	bool			wakeup_source;
 
 	qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
@@ -678,10 +747,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	 * Disable pipe_clk requirement if specified. Used when dwc3
 	 * operates without SSPHY and only HS/FS/LS modes are supported.
 	 */
-	ignore_pipe_clk = device_property_read_bool(dev,
+	qcom->ignore_pipe_clk = device_property_read_bool(dev,
 				"qcom,select-utmi-as-pipe-clk");
-	if (ignore_pipe_clk)
-		dwc3_qcom_select_utmi_clk(qcom);
+	if (qcom->ignore_pipe_clk)
+		dwc3_qcom_select_utmi_clk(qcom, true);
 
 	qcom->mode = usb_get_dr_mode(dev);
 
@@ -698,6 +767,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 			qcom->current_role = USB_ROLE_DEVICE;
 	}
 
+	mutex_init(&qcom->pipe_clk_lock);
 	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
 
 	qcom->dwc.dev = dev;
@@ -715,6 +785,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_core;
 
+	ret = dwc3_qcom_mux_register(qcom);
+	if (ret)
+		goto interconnect_exit;
+
 	wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
 	device_init_wakeup(&pdev->dev, wakeup_source);
 
@@ -722,6 +796,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 
 	return 0;
 
+interconnect_exit:
+	dwc3_qcom_interconnect_exit(qcom);
 remove_core:
 	dwc3_core_remove(&qcom->dwc);
 clk_disable:
@@ -738,6 +814,8 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
 	if (pm_runtime_resume_and_get(qcom->dev) < 0)
 		return;
 
+	if (qcom->mux)
+		typec_mux_unregister(qcom->mux);
 	dwc3_core_remove(&qcom->dwc);
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 	dwc3_qcom_interconnect_exit(qcom);
-- 
2.43.0

[PATCH 3/3] arm64: dts: qcom: kaanapali: Mark the USB controller as a Type-C mode switch

[Joonhoe Kim]

When the USB3-DP combo PHY is a mode switch, DisplayPort pin
assignments C and E put it in DP-only mode, where it stops the USB3
PIPE clock. Make the USB controller a mode switch as well, so that it
moves to the UTMI clock first and keeps its USB 2.0 port working.

Signed-off-by: Joonhoe Kim <26rote@gmail.com>
---
 arch/arm64/boot/dts/qcom/kaanapali.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index e48a179d0722..a06d5d867488 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -3941,6 +3941,7 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			tx-fifo-resize;
 			dma-coherent;
 
+			mode-switch;
 			usb-role-switch;
 			wakeup-source;
 
-- 
2.43.0

Re: [PATCH 2/3] usb: dwc3: qcom: Use the UTMI clock as PIPE clock in DisplayPort-only modes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: dwc3: qcom: missing CONFIG_TYPEC handling in dwc3_qcom_mux_register()
- [High] usb: dwc3: qcom: UTMI clock stuck after DP-only exit in dwc3_qcom_mux_set()
--

commit 248cd44664911cc499b2fd0be17d977befe94173
Author: Joonhoe Kim <26rote@gmail.com>

usb: dwc3: qcom: Use the UTMI clock as PIPE clock in DisplayPort-only modes

This commit registers dwc3-qcom as a Type-C mode switch when the
"mode-switch" property is present and selects the UTMI clock as the PIPE
clock during DisplayPort-only pin assignments (C and E). It defers
switching back to the PHY PIPE clock until a role change notification
occurs after leaving the safe state.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ac68b4218b56b..4919deae682d9 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
[ ... ]
> @@ -419,21 +429,77 @@ static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom)
> +static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom, bool utmi)
>  {
> -	/* Configure dwc3 to use UTMI clock as PIPE clock not present */
>  	dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
>  			  PIPE_UTMI_CLK_DIS);
>  
>  	usleep_range(100, 1000);
>  
> -	dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
> -			  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
> +	if (utmi)
> +		dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
> +				  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
> +	else
> +		dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
> +				  PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
>  
>  	usleep_range(100, 1000);
>  
>  	dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
>  			  PIPE_UTMI_CLK_DIS);
> +
> +	qcom->utmi_pipe_clk = utmi;
> +}
> +
> +static void dwc3_qcom_update_pipe_clk(struct dwc3_qcom *qcom)
> +{
> +	if (qcom->ignore_pipe_clk || qcom->dp_only == qcom->utmi_pipe_clk)
> +		return;
> +
> +	dwc3_qcom_select_utmi_clk(qcom, qcom->dp_only);
> +}
> +
> +static int dwc3_qcom_mux_set(struct typec_mux_dev *mux,
> +			     struct typec_mux_state *state)
> +{
> +	struct dwc3_qcom *qcom = typec_mux_get_drvdata(mux);
> +
> +	guard(mutex)(&qcom->pipe_clk_lock);
> +
> +	if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
> +		qcom->dp_active = true;
> +		qcom->dp_only = state->mode == TYPEC_DP_STATE_C ||
> +				state->mode == TYPEC_DP_STATE_E;
> +	} else if (state->mode == TYPEC_STATE_SAFE) {
> +		qcom->dp_active = false;
> +	} else if (!qcom->dp_active) {
> +		qcom->dp_only = false;
> +	}
> +
> +	/* The PHY stops the PIPE clock right after this in DP-only mode */
> +	if (!qcom->dp_only || pm_runtime_resume_and_get(qcom->dev))
> +		return 0;
> +
> +	dwc3_qcom_update_pipe_clk(qcom);
> +	pm_runtime_put_autosuspend(qcom->dev);
> +
> +	return 0;
> +}

[Severity: High]
Can the controller get stuck on the UTMI clock after leaving a
DisplayPort-only mode?

In dwc3_qcom_mux_set(), when qcom->dp_only transitions to false, the
function returns early at:

	if (!qcom->dp_only || pm_runtime_resume_and_get(qcom->dev))
		return 0;

Switching back to the PHY PIPE clock via dwc3_qcom_select_utmi_clk(qcom,
false) therefore relies entirely on a subsequent call to
dwc3_qcom_set_role_notifier() where next_role != qcom->current_role.

However, as noted in the commit message, role and mux notifications arrive
from different services in no fixed order. After unplugging a 4-lane DP
monitor (where TYPEC_STATE_SAFE sets qcom->dp_active = false while leaving
qcom->dp_only = true and qcom->utmi_pipe_clk = true), plugging in a USB 3.x
device or 2-lane DP + USB 3.x dock can deliver the role change first:

1. dwc3_qcom_set_role_notifier(USB_ROLE_HOST) runs while qcom->dp_only is
   still true, so dwc3_qcom_update_pipe_clk() does nothing and
   qcom->current_role becomes USB_ROLE_HOST.
2. dwc3_qcom_mux_set() runs next and sets qcom->dp_only = false (for
   TYPEC_STATE_USB or TYPEC_DP_STATE_D), then returns early because
   !qcom->dp_only is true.
3. Any subsequent role notification for USB_ROLE_HOST returns early at
   if (qcom->current_role == next_role) in dwc3_qcom_set_role_notifier().

Similarly, if a dock reconfigures its DisplayPort pin assignment in-place
from 4-lane DP (TYPEC_DP_STATE_C or TYPEC_DP_STATE_E) to 2-lane DP + USB3
(TYPEC_DP_STATE_D or TYPEC_DP_STATE_F) without changing the USB role, or if
pmic_glink_altmode_sc8280xp_notify() coalesces MUX_CTRL_STATE_NO_CONN with
a subsequent USB3 state before its worker runs (leaving qcom->dp_active
true when TYPEC_STATE_USB arrives), qcom->utmi_pipe_clk will remain true
with PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW asserted in
QSCRATCH_GENERAL_CFG.

> +
> +static int dwc3_qcom_mux_register(struct dwc3_qcom *qcom)
> +{
> +	struct typec_mux_desc desc = {};
> +
> +	if (!device_property_present(qcom->dev, "mode-switch"))
> +		return 0;
> +
> +	desc.drvdata = qcom;
> +	desc.fwnode = dev_fwnode(qcom->dev);
> +	desc.set = dwc3_qcom_mux_set;
> +
> +	qcom->mux = typec_mux_register(qcom->dev, &desc);
> +
> +	return PTR_ERR_OR_ZERO(qcom->mux);
> +}

[Severity: High]
What happens here when CONFIG_USB_DWC3_QCOM=y and CONFIG_TYPEC=m, or when
CONFIG_TYPEC=n?

In drivers/usb/dwc3/Kconfig, USB_DWC3_QCOM does not depend on
TYPEC || !TYPEC, so built-in dwc3-qcom.o will fail to link against
typec_mux_register(), typec_mux_unregister(), and typec_mux_get_drvdata()
when CONFIG_TYPEC=m.

Additionally, when CONFIG_TYPEC=n, the stub for typec_mux_register() in
include/linux/usb/typec_mux.h returns ERR_PTR(-EOPNOTSUPP):

static inline struct typec_mux_dev *
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
{
	return ERR_PTR(-EOPNOTSUPP);
}

On a board with the "mode-switch" property in its device tree,
dwc3_qcom_mux_register() will return -EOPNOTSUPP to dwc3_qcom_probe() and
abort probing of the USB controller.

Should USB_DWC3_QCOM add a Kconfig dependency on TYPEC || !TYPEC (or
TYPEC || TYPEC=n, like PHY_QCOM_QMP_COMBO) and guard the mux registration
when CONFIG_TYPEC is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102215121.738-1-26rote@gmail.com?part=2

Re: [PATCH 3/3] arm64: dts: qcom: kaanapali: Mark the USB controller as a Type-C mode switch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] usb: dwc3: qcom: link and probe failures in dwc3_qcom_mux_register()
- [High] usb: dwc3: qcom: failure to restore PIPE clock in dwc3_qcom_mux_set()
- [Medium] arm64: dts: qcom: kaanapali: missing mode-switch on usb_dp_qmpphy node
--

commit 925e748782df0ec0b3e6681dff9bc8da6c1e0380
Author: Joonhoe Kim <26rote@gmail.com>

arm64: dts: qcom: kaanapali: Mark the USB controller as a Type-C mode switch

This commit adds the mode-switch property to the Kaanapali USB controller
node so that it switches to the UTMI clock when DisplayPort pin assignments
C and E stop the USB3 PIPE clock.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index e48a179d07220..a06d5d8674885 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> @@ -3941,6 +3941,7 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  			tx-fifo-resize;
>  			dma-coherent;
>  
> +			mode-switch;

[Severity: High]
Does adding mode-switch here expose build and probe regressions in
drivers/usb/dwc3/dwc3-qcom.c depending on CONFIG_TYPEC?

When mode-switch is present on the usb node, dwc3_qcom_probe() calls
dwc3_qcom_mux_register(), which invokes typec_mux_register():

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_mux_register() {
	if (!device_property_present(qcom->dev, "mode-switch"))
		return 0;
	...
	qcom->mux = typec_mux_register(qcom->dev, &desc);

	return PTR_ERR_OR_ZERO(qcom->mux);
}

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_probe() {
	...
	ret = dwc3_qcom_mux_register(qcom);
	if (ret)
		goto interconnect_exit;
}

In drivers/usb/dwc3/Kconfig, USB_DWC3_QCOM does not have a dependency on
TYPEC || !TYPEC (unlike PHY_QCOM_QMP_COMBO in drivers/phy/qualcomm/Kconfig).
With the default arm64 defconfig (CONFIG_USB_DWC3_QCOM=y and CONFIG_TYPEC=m),
built-in dwc3-qcom.o references typec_mux_register(), typec_mux_unregister(),
and typec_mux_get_drvdata() from the loadable module typec.ko, causing a
vmlinux link failure.

Furthermore, when CONFIG_TYPEC=n, the stub in include/linux/usb/typec_mux.h
returns ERR_PTR(-EOPNOTSUPP):

include/linux/usb/typec_mux.h:typec_mux_register() {
	return ERR_PTR(-EOPNOTSUPP);
}

Because dwc3_qcom_mux_register() propagates that error directly, won't
dwc3_qcom_probe() abort and disable USB on Kaanapali whenever CONFIG_TYPEC is
disabled?

[Severity: High]
Can enabling the mode switch on Kaanapali leave the controller stuck on the
19.2 MHz UTMI clock after exiting DisplayPort-only mode (Pin Assignment C or
E), breaking SuperSpeed USB 3.0?

In drivers/usb/dwc3/dwc3-qcom.c, dwc3_qcom_mux_set() updates qcom->dp_active
and qcom->dp_only, and then returns early without calling
dwc3_qcom_update_pipe_clk() whenever qcom->dp_only is false:

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_mux_set() {
	if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
		qcom->dp_active = true;
		qcom->dp_only = state->mode == TYPEC_DP_STATE_C ||
				state->mode == TYPEC_DP_STATE_E;
	} else if (state->mode == TYPEC_STATE_SAFE) {
		qcom->dp_active = false;
	} else if (!qcom->dp_active) {
		qcom->dp_only = false;
	}

	/* The PHY stops the PIPE clock right after this in DP-only mode */
	if (!qcom->dp_only || pm_runtime_resume_and_get(qcom->dev))
		return 0;

	mutex_lock(&qcom->pipe_clk_lock);
	dwc3_qcom_update_pipe_clk(qcom);
	mutex_unlock(&qcom->pipe_clk_lock);
	...
}

Because dwc3_qcom_mux_set() never calls dwc3_qcom_update_pipe_clk() when
qcom->dp_only becomes false, clearing PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW
in QSCRATCH_GENERAL_CFG relies entirely on a subsequent call to
dwc3_qcom_set_role_notifier() where next_role != qcom->current_role while
qcom->dp_only is already false:

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_set_role_notifier() {
	...
	if (qcom->current_role == next_role)
		return;
	...
	mutex_lock(&qcom->pipe_clk_lock);
	dwc3_qcom_update_pipe_clk(qcom);
	mutex_unlock(&qcom->pipe_clk_lock);
}

Doesn't this leave qcom->utmi_pipe_clk stuck at true in several cases:

1. If a connected partner switches pin assignment in place from 4-lane DP
(TYPEC_DP_STATE_C or TYPEC_DP_STATE_E) to 2-lane DP + USB3 (TYPEC_DP_STATE_D
or TYPEC_DP_STATE_F) without changing the USB data role, dwc3_qcom_mux_set()
sets qcom->dp_only = false and returns early, while
dwc3_qcom_set_role_notifier() either does not run or returns early because
qcom->current_role == next_role.

2. In drivers/soc/qcom/pmic_glink_altmode.c, pmic_glink_altmode_worker() can
transition directly to MUX_CTRL_STATE_USB3_ONLY (TYPEC_STATE_USB) or
MUX_CTRL_STATE_TUNNELING (TYPEC_MODE_USB4) without sending TYPEC_STATE_SAFE
first. Since qcom->dp_active is only cleared on TYPEC_STATE_SAFE, the
else if (!qcom->dp_active) branch is not taken and both qcom->dp_active and
qcom->dp_only remain true.

3. After unplugging a DP-only partner (which sends TYPEC_STATE_SAFE and sets
qcom->dp_active = false while leaving qcom->dp_only = true), on the next
plug-in of a USB 3.0 device, if the UCSI role notification arrives before the
altmode mux notification (or if ucsi_partner_change() in
drivers/usb/typec/ucsi/ucsi.c skips typec_set_mode(con->port, TYPEC_STATE_USB)
when UCSI_CONSTAT_PARTNER_FLAG_USB is not yet set),
dwc3_qcom_set_role_notifier() runs while qcom->dp_only is still true and
updates qcom->current_role = next_role. When dwc3_qcom_mux_set() subsequently
clears qcom->dp_only = false, it returns early without updating the clock, and
any later role notification for the same role is ignored.

[Severity: Medium]
The commit message notes that this change is needed when the USB3-DP combo PHY
is a mode switch. However, in arch/arm64/boot/dts/qcom/kaanapali.dtsi, the
companion usb_dp_qmpphy node only has orientation-switch and does not have
mode-switch (unlike sm8750.dtsi and eliza.dtsi):

arch/arm64/boot/dts/qcom/kaanapali.dtsi:usb_dp_qmpphy {
	...
	#clock-cells = <1>;
	#phy-cells = <1>;

	orientation-switch;

	status = "disabled";
}

Without mode-switch on usb_dp_qmpphy, fwnode_typec_mux_get() in
drivers/usb/typec/mux.c will not match the combo PHY, so the USB controller
will switch to the UTMI clock on DisplayPort pin assignments C and E while
usb_dp_qmpphy is never notified of the mode change. Should usb_dp_qmpphy also
have mode-switch here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102215121.738-1-26rote@gmail.com?part=3