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

[Peter Chen <peter.chen@oss.qualcomm.com>]

On 26-11-03 22:50:54, Joonhoe Kim wrote:
> 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. Such monitors often
> have a USB 2.0 hub. But the dwc3 controller still depends on the USB3
> PIPE clock. On Kaanapali the next xHCI reset in this state times out
> ("Host halt failed, -110"). USB stays lost until the cable is replugged.
> Commit f546912bcac6 ("phy: qcom: qmp-combo: Move pipe_clk on/off to
> common") keeps the pipe clock branch enabled. That 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.

Joonhoe, thanks for adding this generic features for Qcom platforms, it
is not only for Kaanapali, but also for platforms which have USB/DP combo
PHY.

We did an internal discussion for this use case, we may prefer the solution
that let the PHY notify the controller this PHY lane is going to change from
USB to DP (4lane) event, it could keep driver independent with dts for this
feature.

We more prefer an general solution like [1], but it seems PHY maintainer Vinod
prefers private interface between controller and PHY [2].

[1]: https://lore.kernel.org/all/20260924-b4-rockchip-dwc3-rockchip-glue-v16-1-126a2e9133c3@collabora.com/
[2]: https://lore.kernel.org/all/avZvYlYyL3Doc8qt@hu-petche-lv.qualcomm.com/

Peter

> 
> 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
> dwc3_qcom_pm_suspend() and dwc3_qcom_pm_resume(). Both add a lock
> there. Like [4], the mux callback defers the QSCRATCH write while the
> glue is suspended for system sleep. On top of [4] the two locks can
> become one.
> 
> 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 the series
> backported to v7.3, plus [2] and [3]. A USB-C monitor ran over 4-lane
> HBR3. Its USB 2.0 hub worked after 10 replugs and after a plug-in during
> s2idle. In the s2idle case the mux state came after resume, so the
> deferred clock switch was not exercised. With the monitor
> unplugged, a USB 3 stick enumerated at SuperSpeed. Quick replugs (under
> a second) worked 9 times out of 10. The other time no PD contract was
> made, so no alternate mode was entered.
> 
> Changes in v2:
> - Patch 1: use the common usb-switch schema
> - Patch 2: switch back to the PIPE clock in the mux callback, when a
>   USB state follows the safe state. v1 waited for the next role change,
>   which can come before the mux state (Sashiko)
> - Patch 2: keep the UTMI clock on a change of pin assignment while
>   DisplayPort is active, as the PHY keeps its mode then
> - Patch 2: while the glue is suspended for system sleep, only record
>   the mux state and select the clock on resume
> - Patch 2: depend on TYPEC || TYPEC=n. Skip the mux without
>   CONFIG_TYPEC. The arm64 defconfig has USB_DWC3_QCOM=y and TYPEC=m,
>   which failed to link (Sashiko). It now builds the glue as a module
> - Patch 2: no change to the role notifier
> - Patch 3: note the dependency on [1] for the PHY's "mode-switch"
> - Patch 3: add Reviewed-by from Abel
> - Link to v1: https://lore.kernel.org/all/20261102215121.738-1-26rote@gmail.com/
> 
> [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          |   3 +
>  arch/arm64/boot/dts/qcom/kaanapali.dtsi       |   1 +
>  drivers/usb/dwc3/Kconfig                      |   1 +
>  drivers/usb/dwc3/dwc3-qcom.c                  | 103 +++++++++++++++---
>  4 files changed, 95 insertions(+), 13 deletions(-)
> 
> 
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
> -- 
> 2.43.0
> 
> 

-- 

Thanks,
Peter Chen