Re: [PATCH 2/9] dt-bindings: phy: qcom,qmp-usb: Add Glymur USB UNI PHY compatible

From: Wesley Cheng

Date: Tue Sep 23 2025 - 17:51:21 EST




On 9/23/2025 12:27 AM, Johan Hovold wrote:
On Mon, Sep 22, 2025 at 06:00:04PM -0700, Wesley Cheng wrote:


On 9/20/2025 8:22 AM, Dmitry Baryshkov wrote:
On Fri, Sep 19, 2025 at 08:21:01PM -0700, Wesley Cheng wrote:
The Glymur USB subsystem contains a multiport controller, which utilizes

two QMP UNI PHYs. Add the proper compatible string for the Glymur SoC.
@@ -16,6 +16,7 @@ description:
properties:
compatible:
enum:
+ - qcom,glymur-qmp-usb3-uni-phy

Odd indentation?

- qcom,ipq5424-qmp-usb3-phy
- qcom,ipq6018-qmp-usb3-phy
- qcom,ipq8074-qmp-usb3-phy
@@ -62,6 +63,8 @@ properties:
vdda-pll-supply: true
+ refgen-supply: true

You've added it, but it's not referenced as required. Why is it so?

The refgen clock isn't always required on each and every platform unlike
the .9v and 1.2v rail/supply, which directly power the QMP PHY. It only
really depends on how the refclk/CXO network is built for that
particular chipset. The refgen ensures that we're properly voting for
the supply that is powering our CXO buffer.

I thought we discussed this before and concluded that this is not an
accurate description of the hardware (even if you now call this supply
refgen instead of qref):
Hi Johan,

refgen and qrefs are different things. I will try to clarify as much as I can from the discussion you linked below. (based on my understanding of the reference clock network) The refgen is the main supply that controls the reference clock (CXO) into a specific branch. Within each of these branches there are clock repeaters that are supplied by QREFs, and is basically the supply to the clkref switch controlled by the TCSR registers.

The way some of the tech blocks are connected, the QREFs/refgen may share the same regulator as some of the PHY's core supply. Some may not even have QREFs at all. One example is the QMP PHY that is associated to the primary controller on Glymur. It has a refgen regulator, but no QREFs, hence we only need to vote the refgen accordingly.

I don't know if that helps you understand it a bit more to convince you of the new regulator addition. If anything we may need to add an explicit QREF supply also :).

Thanks
Wesley Cheng

https://lore.kernel.org/lkml/aEBfV2M-ZqDF7aRz@xxxxxxxxxxxxxxxxxxxx/

Given your description above this still looks wrong (at least after a
quick look).

Johan