2023-04-05 11:43:48

by Varadarajan Narayanan

[permalink] [raw]
Subject: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574

Signed-off-by: Varadarajan Narayanan <[email protected]>
---
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:
+ - 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>
--
2.7.4


2023-04-06 07:43:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

On 05/04/2023 13:41, Varadarajan Narayanan wrote:
> Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574
>
> Signed-off-by: Varadarajan Narayanan <[email protected]>
> ---
> 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

2023-04-06 07:44:45

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

On 05/04/2023 13:41, Varadarajan Narayanan wrote:
> Add dt-bindings for USB3 PHY found on Qualcomm IPQ9574
>
> Signed-off-by: Varadarajan Narayanan <[email protected]>
> ---
> 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

2023-04-17 08:05:55

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
> > ---
> > 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

2023-04-21 10:04:25

by Varadarajan Narayanan

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
> > > ---
> > > 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

2023-04-21 10:15:35

by Varadarajan Narayanan

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
> > ---
> > 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
>

2023-04-21 14:28:32

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
>>> ---
>>> 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
>>

--
With best wishes
Dmitry

2023-04-21 16:32:54

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
>>>> ---
>>>> 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

2023-04-24 05:27:34

by Varadarajan Narayanan

[permalink] [raw]
Subject: Re: [PATCH v8 2/8] dt-bindings: phy: qcom,qmp-usb: Add IPQ9574 USB3 PHY

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 <[email protected]>
> >>>---
> >>> 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
>