The Glymur USB subsystem contains a multiport controller, which utilizes
two QMP UNI PHYs. Add the proper compatible string for the Glymur SoC, and
the required clkref clock name.
Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com>
---
.../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 35 +++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
index a1b55168e050..b0ce803d2b49 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml
@@ -16,6 +16,7 @@ description:
properties:
compatible:
enum:
+ - qcom,glymur-qmp-usb3-uni-phy
- 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
+
"#clock-cells":
const: 0
@@ -157,6 +160,25 @@ allOf:
compatible:
contains:
enum:
+ - qcom,glymur-qmp-usb3-uni-phy
+ then:
+ properties:
+ clocks:
+ maxItems: 5
+ clock-names:
+ items:
+ - const: aux
+ - const: clkref
+ - const: ref
+ - const: com_aux
+ - const: pipe
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,glymur-qmp-usb3-uni-phy
- qcom,sa8775p-qmp-usb3-uni-phy
- qcom,sc8180x-qmp-usb3-uni-phy
- qcom,sc8280xp-qmp-usb3-uni-phy
@@ -165,6 +187,19 @@ allOf:
required:
- power-domains
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,glymur-qmp-usb3-uni-phy
+ then:
+ required:
+ - refgen-supply
+ else:
+ properties:
+ refgen-supply: false
+
additionalProperties: false
examples:On 07/10/2025 00:19, 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, and > the required clkref clock name. > > Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com> > --- > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 35 +++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml > index a1b55168e050..b0ce803d2b49 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml > @@ -16,6 +16,7 @@ description: > properties: > compatible: > enum: > + - qcom,glymur-qmp-usb3-uni-phy > - 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 > + > "#clock-cells": > const: 0 > > @@ -157,6 +160,25 @@ allOf: > compatible: > contains: > enum: > + - qcom,glymur-qmp-usb3-uni-phy > + then: > + properties: > + clocks: Missing minItems. > + maxItems: 5 > + clock-names: > + items: > + - const: aux > + - const: clkref > + - const: ref What is the difference between these two? Which block INPUTs (important!) they represent? > + - const: com_aux > + - const: pipe > + > + - if: Best regards, Krzysztof
On 10/10/2025 5:04 PM, Krzysztof Kozlowski wrote: > On 07/10/2025 00:19, 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, and >> the required clkref clock name. >> >> Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com> >> --- >> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 35 +++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >> index a1b55168e050..b0ce803d2b49 100644 >> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >> @@ -16,6 +16,7 @@ description: >> properties: >> compatible: >> enum: >> + - qcom,glymur-qmp-usb3-uni-phy >> - 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 >> + >> "#clock-cells": >> const: 0 >> >> @@ -157,6 +160,25 @@ allOf: >> compatible: >> contains: >> enum: >> + - qcom,glymur-qmp-usb3-uni-phy >> + then: >> + properties: >> + clocks: > > Missing minItems. > Hi Krzysztof, Won't the minItems be inherited by the base definition? >> + maxItems: 5 >> + clock-names: >> + items: >> + - const: aux >> + - const: clkref >> + - const: ref > > What is the difference between these two? Which block INPUTs > (important!) they represent? > clkref is the TCSR reference clock switch, and the ref is the actual CXO handle. Thanks Wesley Cheng
On 10/13/2025 4:44 PM, Wesley Cheng wrote: > > > On 10/10/2025 5:04 PM, Krzysztof Kozlowski wrote: >> On 07/10/2025 00:19, 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, and >>> the required clkref clock name. >>> >>> Signed-off-by: Wesley Cheng <wesley.cheng@oss.qualcomm.com> >>> --- >>> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 35 +++++++++++++++++++ >>> 1 file changed, 35 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >>> index a1b55168e050..b0ce803d2b49 100644 >>> --- >>> a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >>> +++ >>> b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml >>> @@ -16,6 +16,7 @@ description: >>> properties: >>> compatible: >>> enum: >>> + - qcom,glymur-qmp-usb3-uni-phy >>> - 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 >>> + >>> "#clock-cells": >>> const: 0 >>> @@ -157,6 +160,25 @@ allOf: >>> compatible: >>> contains: >>> enum: >>> + - qcom,glymur-qmp-usb3-uni-phy >>> + then: >>> + properties: >>> + clocks: >> >> Missing minItems. >> > > Hi Krzysztof, > > Won't the minItems be inherited by the base definition? > Ah...are you saying to define minItems to 5 as well, since we need to have all 5 clocks handles defined to work? Thanks Wesley Cheng >>> + maxItems: 5 >>> + clock-names: >>> + items: >>> + - const: aux >>> + - const: clkref >>> + - const: ref >> >> What is the difference between these two? Which block INPUTs >> (important!) they represent? >> > > clkref is the TCSR reference clock switch, and the ref is the actual CXO > handle. > > Thanks > Wesley Cheng
On 14/10/2025 01:46, Wesley Cheng wrote: >>>> "#clock-cells": >>>> const: 0 >>>> @@ -157,6 +160,25 @@ allOf: >>>> compatible: >>>> contains: >>>> enum: >>>> + - qcom,glymur-qmp-usb3-uni-phy >>>> + then: >>>> + properties: >>>> + clocks: >>> >>> Missing minItems. >>> >> >> Hi Krzysztof, >> >> Won't the minItems be inherited by the base definition? >> > > Ah...are you saying to define minItems to 5 as well, since we need to > have all 5 clocks handles defined to work? You need the minItems constraint as well, to define the dimension accurately. > > Thanks > Wesley Cheng > >>>> + maxItems: 5 >>>> + clock-names: >>>> + items: >>>> + - const: aux >>>> + - const: clkref >>>> + - const: ref >>> >>> What is the difference between these two? Which block INPUTs >>> (important!) they represent? >>> >> >> clkref is the TCSR reference clock switch, and the ref is the actual CXO >> handle. Then this should be named somehow differently. CXO is clock. Reference clock is clock... To me it feels like you are describing the same clock, just missing some gate in TCSR. But in case these are not the same clocks, you need to name it accurately. Best regards, Krzysztof
On 10/13/2025 4:50 PM, Krzysztof Kozlowski wrote: > On 14/10/2025 01:46, Wesley Cheng wrote: >>>>> "#clock-cells": >>>>> const: 0 >>>>> @@ -157,6 +160,25 @@ allOf: >>>>> compatible: >>>>> contains: >>>>> enum: >>>>> + - qcom,glymur-qmp-usb3-uni-phy >>>>> + then: >>>>> + properties: >>>>> + clocks: >>>> >>>> Missing minItems. >>>> >>> >>> Hi Krzysztof, >>> >>> Won't the minItems be inherited by the base definition? >>> >> >> Ah...are you saying to define minItems to 5 as well, since we need to >> have all 5 clocks handles defined to work? > > > You need the minItems constraint as well, to define the dimension > accurately. > >> >> Thanks >> Wesley Cheng >> >>>>> + maxItems: 5 >>>>> + clock-names: >>>>> + items: >>>>> + - const: aux >>>>> + - const: clkref >>>>> + - const: ref >>>> >>>> What is the difference between these two? Which block INPUTs >>>> (important!) they represent? >>>> >>> >>> clkref is the TCSR reference clock switch, and the ref is the actual CXO >>> handle. > > > Then this should be named somehow differently. CXO is clock. Reference > clock is clock... To me it feels like you are describing the same clock, > just missing some gate in TCSR. But in case these are not the same > clocks, you need to name it accurately. > Technically its all handling the same clock branch (CXO), we have the TCSR clkref register that allows us to gate the CXO to the USB PHY, as CXO is shared across several HW blocks, so it allows us to properly powerdown the PHY even though other clients are voting for CXO on. Then we obviously have to remove our vote to the overall CXO, so that it can potentially be shutdown. Maybe we can rename it to "clkref" for the CXO handle and "clkref_switch" for the TCSRCC handle? Thanks Wesley Cheng
On 14/10/2025 03:16, Wesley Cheng wrote: >>> >>>>>> + maxItems: 5 >>>>>> + clock-names: >>>>>> + items: >>>>>> + - const: aux >>>>>> + - const: clkref >>>>>> + - const: ref >>>>> >>>>> What is the difference between these two? Which block INPUTs >>>>> (important!) they represent? >>>>> >>>> >>>> clkref is the TCSR reference clock switch, and the ref is the actual CXO >>>> handle. >> >> >> Then this should be named somehow differently. CXO is clock. Reference >> clock is clock... To me it feels like you are describing the same clock, >> just missing some gate in TCSR. But in case these are not the same >> clocks, you need to name it accurately. >> > > Technically its all handling the same clock branch (CXO), we have the > TCSR clkref register that allows us to gate the CXO to the USB PHY, as Ah, exactly. Then clkref is not a clock. You need rather proper clock hierarchy. > CXO is shared across several HW blocks, so it allows us to properly > powerdown the PHY even though other clients are voting for CXO on. Then > we obviously have to remove our vote to the overall CXO, so that it can > potentially be shutdown. > > Maybe we can rename it to "clkref" for the CXO handle and > "clkref_switch" for the TCSRCC handle? Naming is better, but it is still not correct. This is not independent clock signal. It is the same clock. Best regards, Krzysztof
On 10/13/2025 9:36 PM, Krzysztof Kozlowski wrote: > On 14/10/2025 03:16, Wesley Cheng wrote: >>>> >>>>>>> + maxItems: 5 >>>>>>> + clock-names: >>>>>>> + items: >>>>>>> + - const: aux >>>>>>> + - const: clkref >>>>>>> + - const: ref >>>>>> >>>>>> What is the difference between these two? Which block INPUTs >>>>>> (important!) they represent? >>>>>> >>>>> >>>>> clkref is the TCSR reference clock switch, and the ref is the actual CXO >>>>> handle. >>> >>> >>> Then this should be named somehow differently. CXO is clock. Reference >>> clock is clock... To me it feels like you are describing the same clock, >>> just missing some gate in TCSR. But in case these are not the same >>> clocks, you need to name it accurately. >>> >> >> Technically its all handling the same clock branch (CXO), we have the >> TCSR clkref register that allows us to gate the CXO to the USB PHY, as > > > Ah, exactly. Then clkref is not a clock. You need rather proper clock > hierarchy. > >> CXO is shared across several HW blocks, so it allows us to properly >> powerdown the PHY even though other clients are voting for CXO on. Then >> we obviously have to remove our vote to the overall CXO, so that it can >> potentially be shutdown. >> >> Maybe we can rename it to "clkref" for the CXO handle and >> "clkref_switch" for the TCSRCC handle? > > Naming is better, but it is still not correct. This is not independent > clock signal. It is the same clock. > Hmmm... I guess that's why I kept the same clkref tag, to denote that its the same clock, but one is a switch/gate for it. Would you happen to have any suggestions you might have that makes it clearer for everyone to understand? Thanks Wesley Cheng
On 17/10/2025 02:15, Wesley Cheng wrote: >>> Technically its all handling the same clock branch (CXO), we have the >>> TCSR clkref register that allows us to gate the CXO to the USB PHY, as >> >> >> Ah, exactly. Then clkref is not a clock. You need rather proper clock >> hierarchy. >> >>> CXO is shared across several HW blocks, so it allows us to properly >>> powerdown the PHY even though other clients are voting for CXO on. Then >>> we obviously have to remove our vote to the overall CXO, so that it can >>> potentially be shutdown. >>> >>> Maybe we can rename it to "clkref" for the CXO handle and >>> "clkref_switch" for the TCSRCC handle? >> >> Naming is better, but it is still not correct. This is not independent >> clock signal. It is the same clock. >> > > Hmmm... I guess that's why I kept the same clkref tag, to denote that > its the same clock, but one is a switch/gate for it. Would you happen > to have any suggestions you might have that makes it clearer for > everyone to understand? To me it looks like: |-----| |-----------| |------------------| |clock|------------|TCSRCC gate|-----------|clkref to this dev| |-----| |-----------| |------------------| So you need proper clock controller for TCSR (TCSR Clock Controller, in short TCSRCC, what a surprise!) which will take input, add gate and produce clock for this device. Nothing non-standard, all Qualcomm SoCs have it, every other platform has it in some way. Best regards, Krzysztof
On 10/16/2025 9:41 PM, Krzysztof Kozlowski wrote: > On 17/10/2025 02:15, Wesley Cheng wrote: >>>> Technically its all handling the same clock branch (CXO), we have the >>>> TCSR clkref register that allows us to gate the CXO to the USB PHY, as >>> >>> >>> Ah, exactly. Then clkref is not a clock. You need rather proper clock >>> hierarchy. >>> >>>> CXO is shared across several HW blocks, so it allows us to properly >>>> powerdown the PHY even though other clients are voting for CXO on. Then >>>> we obviously have to remove our vote to the overall CXO, so that it can >>>> potentially be shutdown. >>>> >>>> Maybe we can rename it to "clkref" for the CXO handle and >>>> "clkref_switch" for the TCSRCC handle? >>> >>> Naming is better, but it is still not correct. This is not independent >>> clock signal. It is the same clock. >>> >> >> Hmmm... I guess that's why I kept the same clkref tag, to denote that >> its the same clock, but one is a switch/gate for it. Would you happen >> to have any suggestions you might have that makes it clearer for >> everyone to understand? > To me it looks like: > > |-----| |-----------| |------------------| > |clock|------------|TCSRCC gate|-----------|clkref to this dev| > |-----| |-----------| |------------------| > > So you need proper clock controller for TCSR (TCSR Clock Controller, in > short TCSRCC, what a surprise!) which will take input, add gate and > produce clock for this device. > > Nothing non-standard, all Qualcomm SoCs have it, every other platform > has it in some way. > Hi Krzystof, Yes, the design is exactly how you outlined it above. How about clkref for the clock and tcsrcc_switch for the clkref switch? That removes any notation that the gate/switch is an actual clock... Thanks Wesley Cheng
On 18/10/2025 02:20, Wesley Cheng wrote: > > > On 10/16/2025 9:41 PM, Krzysztof Kozlowski wrote: >> On 17/10/2025 02:15, Wesley Cheng wrote: >>>>> Technically its all handling the same clock branch (CXO), we have the >>>>> TCSR clkref register that allows us to gate the CXO to the USB PHY, as >>>> >>>> >>>> Ah, exactly. Then clkref is not a clock. You need rather proper clock >>>> hierarchy. >>>> >>>>> CXO is shared across several HW blocks, so it allows us to properly >>>>> powerdown the PHY even though other clients are voting for CXO on. Then >>>>> we obviously have to remove our vote to the overall CXO, so that it can >>>>> potentially be shutdown. >>>>> >>>>> Maybe we can rename it to "clkref" for the CXO handle and >>>>> "clkref_switch" for the TCSRCC handle? >>>> >>>> Naming is better, but it is still not correct. This is not independent >>>> clock signal. It is the same clock. >>>> >>> >>> Hmmm... I guess that's why I kept the same clkref tag, to denote that >>> its the same clock, but one is a switch/gate for it. Would you happen >>> to have any suggestions you might have that makes it clearer for >>> everyone to understand? >> To me it looks like: >> >> |-----| |-----------| |------------------| >> |clock|------------|TCSRCC gate|-----------|clkref to this dev| >> |-----| |-----------| |------------------| >> >> So you need proper clock controller for TCSR (TCSR Clock Controller, in >> short TCSRCC, what a surprise!) which will take input, add gate and >> produce clock for this device. >> >> Nothing non-standard, all Qualcomm SoCs have it, every other platform >> has it in some way. >> > > Hi Krzystof, > > Yes, the design is exactly how you outlined it above. How about clkref Hm? There is no connection between the clock and the device. Do you see any line going there? > for the clock and tcsrcc_switch for the clkref switch? That removes any > notation that the gate/switch is an actual clock... You really did not get the point of this entire discussion. Best regards, Krzysztof
On 10/18/2025 8:39 AM, Krzysztof Kozlowski wrote: > On 18/10/2025 02:20, Wesley Cheng wrote: >> >> >> On 10/16/2025 9:41 PM, Krzysztof Kozlowski wrote: >>> On 17/10/2025 02:15, Wesley Cheng wrote: >>>>>> Technically its all handling the same clock branch (CXO), we have the >>>>>> TCSR clkref register that allows us to gate the CXO to the USB PHY, as >>>>> >>>>> >>>>> Ah, exactly. Then clkref is not a clock. You need rather proper clock >>>>> hierarchy. >>>>> >>>>>> CXO is shared across several HW blocks, so it allows us to properly >>>>>> powerdown the PHY even though other clients are voting for CXO on. Then >>>>>> we obviously have to remove our vote to the overall CXO, so that it can >>>>>> potentially be shutdown. >>>>>> >>>>>> Maybe we can rename it to "clkref" for the CXO handle and >>>>>> "clkref_switch" for the TCSRCC handle? >>>>> >>>>> Naming is better, but it is still not correct. This is not independent >>>>> clock signal. It is the same clock. >>>>> >>>> >>>> Hmmm... I guess that's why I kept the same clkref tag, to denote that >>>> its the same clock, but one is a switch/gate for it. Would you happen >>>> to have any suggestions you might have that makes it clearer for >>>> everyone to understand? >>> To me it looks like: >>> >>> |-----| |-----------| |------------------| >>> |clock|------------|TCSRCC gate|-----------|clkref to this dev| >>> |-----| |-----------| |------------------| >>> >>> So you need proper clock controller for TCSR (TCSR Clock Controller, in >>> short TCSRCC, what a surprise!) which will take input, add gate and >>> produce clock for this device. >>> >>> Nothing non-standard, all Qualcomm SoCs have it, every other platform >>> has it in some way. >>> >> >> Hi Krzystof, >> >> Yes, the design is exactly how you outlined it above. How about clkref > > Hm? There is no connection between the clock and the device. Do you see > any line going there? > >> for the clock and tcsrcc_switch for the clkref switch? That removes any >> notation that the gate/switch is an actual clock... > > You really did not get the point of this entire discussion. > I won't argue how I interpreted this conversation vs your understanding. We can remove our vote to the clock itself, since tcsrcc registers the CXO as its parent, so it'll handle the reference counting for us whenever we vote on the tcsr clkref. Thank you for taking the time to explain what you were looking for. Thanks Wesley Cheng
© 2016 - 2025 Red Hat, Inc.