Re: [PATCH] phy: qcom: qmp-combo: Move pipe_clk on/off to common

[Konrad Dybcio]

On 9/27/25 11:17 AM, Val Packett wrote:
> Keep the USB pipe clock working when the phy is in DP-only mode, because
> the dwc controller still needs it for USB 2.0 over the same Type-C port.
> 
> Tested with the BenQ RD280UA monitor which has a downstream-facing port
> for data passthrough that's manually switchable between USB 2 and 3,
> corresponding to 4-lane and 2-lane DP respectively.
> 
> Note: the suspend/resume callbacks were already gating the enable/disable
> of this clock only on init_count and not usb_init_count!
> 
> Signed-off-by: Val Packett <val@xxxxxxxxxxxx>
> ---
> o/
> 
> Just got my hands on a perfect test device for DP alt mode: a monitor with an
> on-demand toggle between 2 and 4 lanes. (Started digging because I thought
> I needed 4 lanes to use its full resolution and refresh rate, even though
> it turned out to be the dpu adjusted mode clock check rejecting the modes,
> patches for which are already posted.)
> 
> In [1] Konrad mentioned that "the hardware disagrees" with keeping the USB
> PLL always on. I'm not sure what exactly was meant by disagreement there,
> and I didn't find any specific code that touches that PLL in the driver,
> so I decided to just try it anyway.
> 
> Before the changes, 4-lane mode would actually kill the USB 2.0 functionality
> on the port, no recovery until reboot.
> 
> With this patch, I can switch the monitor between 4-lane and 2-lane modes
> (with an unplug-replug cycle..) and the USB 2.0 devices attached through
> the monitor keep working! (I verified the number of lanes used via dp_debug).
> 
> I'm sure it might not be that simple but from my limited and uninformed
> understanding without any internal knowledge, the "sneaky workaround"
> might actually be the intended way to do things?

+Bjorn, Wesley I wrote a wall of text for you, hopefully it makes sense

I now think I have some more knowledge about the setup.

The PIPE clock is an *output* of the PHY that feeds back into GCC, which
provides a XO-PHY mux, which then has a gate/branch downstream of it.
(that's 100% sure)

That branch (e.g. GCC_USB3_PRIM_PHY_PIPE_CLK) then feeds into the
destination core (PCIe, DWC3..) (that's *almost* 100% sure.. it's
surprisingly difficult to fully confirm)

Both msm-5.10 (and older) and upstream decided to keep that clock within
the PHY device though, and that makes.. some.. sense, because you're
not really supposed to turn it off (at least on recent chips), so much so
as switch its source from PHY to XO. That of course needs to happen when
the PHY is going to be disabled, as otherwise the PIPE clock is absent..

I can't really find information about when that branch is supposed to
be *disabled*, but one can probably assume with reasonable confidence
that if the PHY is offline (which generally only happens if the controller
is offline), that may be disabled as well.


The only remaining question remains then, where does the PIPE clock
come from? It's most likely not derived from the DP PLL.

The PHY has some slow onboard always-on clocks (for USB4 wakeup and
USB3 autonomous mode), but those are waay too slow to be driving PIPE
(which runs at 125 MHz @ SS, 312.5 MHz @ SS+).. so only the USB PLL
remains.

But then, in DP-only mode, SW_USB3PHY_RESET is asserted. The
documentation uses some imprecise wording with regards to what that
entails, but there's not many options left other than it may be
leaving the PLL in tact, while halting other parts of the USB3 logic.
Conversely, usb3phy_(nocsr_)reset is explicitly promised to reset the
USB_PLL.


I think the PLL also happens to be disabled when entering either of
the aforementioned sleep modes, since the table tells me the required
RPMH level is 'retention', which surely isn't enough to feed a big PLL.


The solution of teaching DWC3 how to switch clock sources is a little
involved, since we (to my understanding) have to/should omit programming
the parts that are conditional upon superspeed capabilities, and doing
that at runtime would probably realistically require making it type-C
aware (I don't think we want to go that route plus micro-b/MHL edge cases
*sigh*), or we could count on the suspend/resume work making it magically
click into place (since the dwc3 core does quite some re-programming upon
any sort of sus/res today anyway, it seems). If the latter happens, we may
end up reverting this patch as it may then turn out no longer necessary.

Today, i think this commit resolves this immediate issue and I think it's
the fix we should take (bumping my request of reordering the clock disables
and resubmitting..).

On an interesting note, if we take for granted my hypothesis about the
soft-reset not impacting the USB_PLL, this seems like an oversight in
programming guides, because USB4 transition sequences explicitly mention
the names which refer to resets coming from GCC, while DP/USB mix-and-
matching explicitly suggests using the soft-resets, but that might have
gone unnoticed, since dp-only hasn't been much of a prime use case for
in the past, as most of these chips were targetting mobile.

Konrad