| Message ID | 1efa9a64499767d939efadd0aef897ac4a6e54eb.1680693149.git.quic_varada@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable IPQ9754 USB | expand |
On 05/04/2023 13:41, Varadarajan Narayanan wrote: > Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- > Changes in v8: > - Update clock names for ipq9574 > > Changes in v6: > - Made power-domains optional > > Note: In the earlier patch sets, had used the (legacy) > specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > --- > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > 1 file changed, 37 insertions(+), 6 deletions(-) > > 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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy > - qcom,sc8280xp-qmp-usb3-uni-phy > > reg: > @@ -25,11 +26,7 @@ properties: > maxItems: 4 > > clock-names: > - items: > - - const: aux > - - const: ref > - - const: com_aux > - - const: pipe > + maxItems: 4 > > power-domains: > maxItems: 1 > @@ -60,7 +57,6 @@ required: > - reg > - clocks > - clock-names > - - power-domains > - resets > - reset-names > - vdda-phy-supply > @@ -71,6 +67,41 @@ required: > > additionalProperties: false > > +allOf: As you can see in example-schema, allOf goes before additionalProperties: false. > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,ipq9574-qmp-usb3-phy > + then: > + properties: > + clocks: > + maxItems: 4 Don't need clocks here. > + clock-names: > + items: > + - const: aux > + - const: ref > + - const: cfg_ahb > + - const: pipe > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sc8280xp-qmp-usb3-uni-phy > + then: > + properties: > + clocks: > + maxItems: 4 Neither here. > + clock-names: > + items: > + - const: aux > + - const: ref > + - const: com_aux Can anyone explain me why do we name these (here and other Qualcomm bindings) based on clock name, not input? Just because different clock is fed to the block, does not necessarily mean the input should be named differently. > + - const: pipe > + > examples: > - | > #include <dt-bindings/clock/qcom,gcc-sc8280xp.h> Best regards, Krzysztof
On 05/04/2023 13:41, Varadarajan Narayanan wrote: > Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > --- > Changes in v8: > - Update clock names for ipq9574 > > Changes in v6: > - Made power-domains optional > > Note: In the earlier patch sets, had used the (legacy) > specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > --- > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > 1 file changed, 37 insertions(+), 6 deletions(-) > > 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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy > - qcom,sc8280xp-qmp-usb3-uni-phy > > reg: > @@ -25,11 +26,7 @@ properties: > maxItems: 4 > > clock-names: > - items: > - - const: aux > - - const: ref > - - const: com_aux > - - const: pipe > + maxItems: 4 > > power-domains: > maxItems: 1 > @@ -60,7 +57,6 @@ required: > - reg > - clocks > - clock-names > - - power-domains Power domains are required. Commit msg does not explain why this should be now optional. Best regards, Krzysztof
On Thu, Apr 06, 2023 at 09:41:49AM +0200, Krzysztof Kozlowski wrote: > On 05/04/2023 13:41, Varadarajan Narayanan wrote: > > Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > --- > > Changes in v8: > > - Update clock names for ipq9574 > > > > Changes in v6: > > - Made power-domains optional > > > > Note: In the earlier patch sets, had used the (legacy) > > specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > > to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > > --- > > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > > 1 file changed, 37 insertions(+), 6 deletions(-) > > + clock-names: > > + items: > > + - const: aux > > + - const: ref > > + - const: com_aux > > Can anyone explain me why do we name these (here and other Qualcomm > bindings) based on clock name, not input? Just because different clock > is fed to the block, does not necessarily mean the input should be named > differently. I guess part of the answer is that this has just been copied from the vendor dts and (almost) no one but Qualcomm has access to the documentation. What would the input names be here? Also note that there are SoCs that enable both 'cfg_ahb' and 'com_aux' (e.g. sc7180). > > + - const: pipe > > + > > examples: > > - | > > #include <dt-bindings/clock/qcom,gcc-sc8280xp.h> Johan
On Mon, Apr 17, 2023 at 10:05:11AM +0200, Johan Hovold wrote: > On Thu, Apr 06, 2023 at 09:41:49AM +0200, Krzysztof Kozlowski wrote: > > On 05/04/2023 13:41, Varadarajan Narayanan wrote: > > > Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > > --- > > > Changes in v8: > > > - Update clock names for ipq9574 > > > > > > Changes in v6: > > > - Made power-domains optional > > > > > > Note: In the earlier patch sets, had used the (legacy) > > > specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > > > to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > > > --- > > > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > > > 1 file changed, 37 insertions(+), 6 deletions(-) > > > > + clock-names: > > > + items: > > > + - const: aux > > > + - const: ref > > > + - const: com_aux > > > > Can anyone explain me why do we name these (here and other Qualcomm > > bindings) based on clock name, not input? Just because different clock > > is fed to the block, does not necessarily mean the input should be named > > differently. > > I guess part of the answer is that this has just been copied from the > vendor dts and (almost) no one but Qualcomm has access to the > documentation. What would the input names be here? > > Also note that there are SoCs that enable both 'cfg_ahb' and 'com_aux' > (e.g. sc7180). The clock name definitions are auto-generated based on the clock tree definitions provided by the h/w team. We followed the naming pattern done in the previous SoCs. Thanks Varada > > > > + - const: pipe > > > + > > > examples: > > > - | > > > #include <dt-bindings/clock/qcom,gcc-sc8280xp.h> > > Johan
On Thu, Apr 06, 2023 at 09:42:31AM +0200, Krzysztof Kozlowski wrote: > On 05/04/2023 13:41, Varadarajan Narayanan wrote: > > Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > > --- > > Changes in v8: > > - Update clock names for ipq9574 > > > > Changes in v6: > > - Made power-domains optional > > > > Note: In the earlier patch sets, had used the (legacy) > > specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > > to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > > --- > > .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > > 1 file changed, 37 insertions(+), 6 deletions(-) > > > > 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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy > > - qcom,sc8280xp-qmp-usb3-uni-phy > > > > reg: > > @@ -25,11 +26,7 @@ properties: > > maxItems: 4 > > > > clock-names: > > - items: > > - - const: aux > > - - const: ref > > - - const: com_aux > > - - const: pipe > > + maxItems: 4 > > > > power-domains: > > maxItems: 1 > > @@ -60,7 +57,6 @@ required: > > - reg > > - clocks > > - clock-names > > - - power-domains > > Power domains are required. Commit msg does not explain why this should > be now optional. Since IPQ9574 doesn't have power switches couldn't provide power-domains details. So, had to make it optional to pass 'make dtbs_check'. Thanks Varada > Best regards, > Krzysztof >
On 21/04/2023 13:13, Varadarajan Narayanan wrote: > On Thu, Apr 06, 2023 at 09:42:31AM +0200, Krzysztof Kozlowski wrote: >> On 05/04/2023 13:41, Varadarajan Narayanan wrote: >>> Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 >>> >>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> >>> --- >>> Changes in v8: >>> - Update clock names for ipq9574 >>> >>> Changes in v6: >>> - Made power-domains optional >>> >>> Note: In the earlier patch sets, had used the (legacy) >>> specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved >>> to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml >>> --- >>> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- >>> 1 file changed, 37 insertions(+), 6 deletions(-) >>> >>> 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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy >>> - qcom,sc8280xp-qmp-usb3-uni-phy >>> >>> reg: >>> @@ -25,11 +26,7 @@ properties: >>> maxItems: 4 >>> >>> clock-names: >>> - items: >>> - - const: aux >>> - - const: ref >>> - - const: com_aux >>> - - const: pipe >>> + maxItems: 4 >>> >>> power-domains: >>> maxItems: 1 >>> @@ -60,7 +57,6 @@ required: >>> - reg >>> - clocks >>> - clock-names >>> - - power-domains >> >> Power domains are required. Commit msg does not explain why this should >> be now optional. > > Since IPQ9574 doesn't have power switches couldn't provide power-domains details. > So, had to make it optional to pass 'make dtbs_check'. This should be a part of the commit message, so that the next developer understands your intentions without going to mail archives. > > Thanks > Varada > >> Best regards, >> Krzysztof >>
On 21/04/2023 11:58, Varadarajan Narayanan wrote: > On Mon, Apr 17, 2023 at 10:05:11AM +0200, Johan Hovold wrote: >> On Thu, Apr 06, 2023 at 09:41:49AM +0200, Krzysztof Kozlowski wrote: >>> On 05/04/2023 13:41, Varadarajan Narayanan wrote: >>>> Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 >>>> >>>> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> >>>> --- >>>> Changes in v8: >>>> - Update clock names for ipq9574 >>>> >>>> Changes in v6: >>>> - Made power-domains optional >>>> >>>> Note: In the earlier patch sets, had used the (legacy) >>>> specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved >>>> to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml >>>> --- >>>> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- >>>> 1 file changed, 37 insertions(+), 6 deletions(-) >> >>>> + clock-names: >>>> + items: >>>> + - const: aux >>>> + - const: ref >>>> + - const: com_aux >>> >>> Can anyone explain me why do we name these (here and other Qualcomm >>> bindings) based on clock name, not input? Just because different clock >>> is fed to the block, does not necessarily mean the input should be named >>> differently. >> >> I guess part of the answer is that this has just been copied from the >> vendor dts and (almost) no one but Qualcomm has access to the >> documentation. What would the input names be here? >> >> Also note that there are SoCs that enable both 'cfg_ahb' and 'com_aux' >> (e.g. sc7180). > > The clock name definitions are auto-generated based on the clock > tree definitions provided by the h/w team. We followed the naming > pattern done in the previous SoCs. Are you sure? We talk about clock inputs here. Best regards, Krzysztof
On Fri, Apr 21, 2023 at 05:19:58PM +0300, Dmitry Baryshkov wrote: > On 21/04/2023 13:13, Varadarajan Narayanan wrote: > >On Thu, Apr 06, 2023 at 09:42:31AM +0200, Krzysztof Kozlowski wrote: > >>On 05/04/2023 13:41, Varadarajan Narayanan wrote: > >>>Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 > >>> > >>>Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> > >>>--- > >>> Changes in v8: > >>> - Update clock names for ipq9574 > >>> > >>> Changes in v6: > >>> - Made power-domains optional > >>> > >>>Note: In the earlier patch sets, had used the (legacy) > >>>specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved > >>>to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml > >>>--- > >>> .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- > >>> 1 file changed, 37 insertions(+), 6 deletions(-) > >>> > >>>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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy > >>> - qcom,sc8280xp-qmp-usb3-uni-phy > >>> > >>> reg: > >>>@@ -25,11 +26,7 @@ properties: > >>> maxItems: 4 > >>> > >>> clock-names: > >>>- items: > >>>- - const: aux > >>>- - const: ref > >>>- - const: com_aux > >>>- - const: pipe > >>>+ maxItems: 4 > >>> > >>> power-domains: > >>> maxItems: 1 > >>>@@ -60,7 +57,6 @@ required: > >>> - reg > >>> - clocks > >>> - clock-names > >>>- - power-domains > >> > >>Power domains are required. Commit msg does not explain why this should > >>be now optional. > > > >Since IPQ9574 doesn't have power switches couldn't provide power-domains details. > >So, had to make it optional to pass 'make dtbs_check'. > > This should be a part of the commit message, so that the next developer > understands your intentions without going to mail archives. Thanks for the feedback. Have posted v9 that includes the above in commit message. https://lore.kernel.org/lkml/b00042df41420ac337703ca99ac7876c46552946.1682092324.git.quic_varada@quicinc.com/ Thanks Varada > >>Best regards, > >>Krzysztof > >> > > -- > With best wishes > Dmitry >
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 16fce10..e902a0d 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,ipq9574-qmp-usb3-phy - qcom,sc8280xp-qmp-usb3-uni-phy reg: @@ -25,11 +26,7 @@ properties: maxItems: 4 clock-names: - items: - - const: aux - - const: ref - - const: com_aux - - const: pipe + maxItems: 4 power-domains: maxItems: 1 @@ -60,7 +57,6 @@ required: - reg - clocks - clock-names - - power-domains - resets - reset-names - vdda-phy-supply @@ -71,6 +67,41 @@ required: additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + enum: + - qcom,ipq9574-qmp-usb3-phy + then: + properties: + clocks: + maxItems: 4 + clock-names: + items: + - const: aux + - const: ref + - const: cfg_ahb + - const: pipe + + - if: + properties: + compatible: + contains: + enum: + - qcom,sc8280xp-qmp-usb3-uni-phy + then: + properties: + clocks: + maxItems: 4 + clock-names: + items: + - const: aux + - const: ref + - const: com_aux + - const: pipe + examples: - | #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574 Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com> --- Changes in v8: - Update clock names for ipq9574 Changes in v6: - Made power-domains optional Note: In the earlier patch sets, had used the (legacy) specification available in qcom,msm8996-qmp-usb3-phy.yaml. Moved to newer specification in qcom,sc8280xp-qmp-usb3-uni-phy.yaml --- .../phy/qcom,sc8280xp-qmp-usb3-uni-phy.yaml | 43 +++++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-)