Message-ID: <2564cdec-9726-4efa-ba07-a2f2646168c6@packett.cool>
Date: Mon, 6 Oct 2025 13:13:42 -0300
From: Val Packett <val@...kett.cool>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
 Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, linux-arm-msm@...r.kernel.org,
 linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] phy: qcom: qmp-combo: Move pipe_clk on/off to common


On 10/6/25 11:44 AM, Konrad Dybcio wrote:
> 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.
>> [..]
>>
>> 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.
> So what I did was playing around with the RESET_OVRD settings, which
> dictate what parts of the PHY (and their associated PLLs) are kept online..
> but I totally forgot that there is a branch/gate clock in GCC that sits
> inbetween!
>
>> [..]
>> 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?
> Normally the clock which you're enabling is sourced from the QMPPHY.
> The other option (bar some debug outputs) is for it to be driven by
> the 19.2 MHz always-on crystal (instead of $lots_of_mhz from the PHY).
>
> For USB hosts without a USB3 phy connected to them, there's an option
> to mux the controller's PIPE clock to be sourced from the UTMI clock
> input. In those cases, the UTMI (and therefore PIPE) clock runs at..
> well, 19.2 MHz!
>
> (you can actually do that on USB3-phy-connected hosts too, at the cost
> of.. USB3, probably)
>
> So I'm not sure how much of that is well thought-out design and how
> much is luck, but this ends up working for us anyway, with seemingly
> no downsides.
>
> At least that's my understanding of the situation.

I wonder how Windows drivers handle this.

The ability to use the UTMI clock sounds more appropriate for when only 
a legacy USB2 device is plugged in and the entirety of QMPPHY is 
unnecessary and can be shut down to save power.

BTW I'm still seeing USB2 functionality die if I boot with the monitor 
cable *already* plugged in, but that sounds like a very different issue 
(the host controller starting to touch the bus before the PIPE clock is 
up? something something probe order?)

> The suspend logic is broken and unused anyway, but that's a nice catch,
> the PIPE clock in question is even conveniently called "usb3_pipe" in DT

Hmm. Is it unused? Oh, you mean the pm_runtime_forbid(), right.

Do you have any pointers about what exactly is broken there? I've been 
poking at the runtime PM stuff too 
(https://gitlab.com/Linaro/arm64-laptops/linux/-/issues/14 for USB), the 
PHYs are the biggest missing piece there overall..

>> [..]
>> @@ -3103,6 +3110,7 @@ static int qmp_combo_com_exit(struct qmp_combo *qmp, bool force)
>>   	reset_control_bulk_assert(cfg->num_resets, qmp->resets);
>>   
>>   	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>> +	clk_disable_unprepare(qmp->pipe_clk);
> Let's disable this one first, to preserve existing behavior (and it
> makes sense logically - if the PHY doesn't have its clocks, it can't
> really generate one either)

Sure, nice catch. Will send a V2 with this fixed.

Thanks,
~val

Powered by blists - more mailing lists