[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 <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(-)
  

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
>