[v3,1/2] dt-bindings: phy: rockchip,inno-usb2phy: add rk3576

Commit Message

Frank Wang Sept. 26, 2024, 10:32 a.m. UTC

From: Frank Wang <[e-mail]>

Add compatible for the USB2 phy in the Rockchip RK3576 SoC.

In addition, since the RK3576 introduced the USB MMU, this change
also add the related clock properties for it.

Signed-off-by: Frank Wang <[e-mail]>
---
Changelog:
v3:
 - narrowed rk3576 clocks by compatible property.

v2:
 - Categorize clock names by oneOf keyword.

v1:
 - https://patchwork.kernel.org/project/linux-phy/patch/[e-mail]/

 .../bindings/phy/rockchip,inno-usb2phy.yaml    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Krzysztof Kozlowski Sept. 26, 2024, 2:19 p.m. UTC | #1

On 26/09/2024 12:32, Frank Wang wrote:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3576-usb2phy
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +          maxItems: 3

Read one more time the example I gave you. Top-level constraints are
saying max one clock.

Best regards,
Krzysztof

Frank Wang Sept. 27, 2024, 7:01 a.m. UTC | #2

Hi Krzysztof,

On 2024/9/26 22:19, Krzysztof Kozlowski wrote:
> On 26/09/2024 12:32, Frank Wang wrote:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - rockchip,rk3576-usb2phy
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 3
>> +          maxItems: 3
> Read one more time the example I gave you. Top-level constraints are
> saying max one clock.
>
> Best regards,
> Krzysztof
>

Sorry for overlooking this, I will set both "clocks" and "clock-names" 
to true, and add the else case below the above codes for the "old" SoCs.
Just like the below.

-  clocks:
-    maxItems: 1
+  clocks: true

-  clock-names:
-    const: phyclk
+  clock-names: true

    assigned-clocks:
      description:
@@ -189,6 +187,13 @@ allOf:
              - const: phyclk
              - const: aclk
              - const: aclk_slv
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          const: phyclk
+


Best regards,
Frank

Krzysztof Kozlowski Sept. 27, 2024, 7:30 a.m. UTC | #3

On 27/09/2024 09:01, Frank Wang wrote:
> Hi Krzysztof,
> 
> On 2024/9/26 22:19, Krzysztof Kozlowski wrote:
>> On 26/09/2024 12:32, Frank Wang wrote:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - rockchip,rk3576-usb2phy
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 3
>>> +          maxItems: 3
>> Read one more time the example I gave you. Top-level constraints are
>> saying max one clock.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Sorry for overlooking this, I will set both "clocks" and "clock-names" 
> to true, and add the else case below the above codes for the "old" SoCs.
> Just like the below.
> 
> -  clocks:
> -    maxItems: 1
> +  clocks: true
> 
> -  clock-names:
> -    const: phyclk
> +  clock-names: true

For the third time, read the code I gave you. Do you see something like
this there? Why doing all the time something different than existing code?

Best regards,
Krzysztof

Frank Wang Sept. 27, 2024, 7:59 a.m. UTC | #4

Hi Krzysztof,

On 2024/9/27 15:30, Krzysztof Kozlowski wrote:
> On 27/09/2024 09:01, Frank Wang wrote:
>> Hi Krzysztof,
>>
>> On 2024/9/26 22:19, Krzysztof Kozlowski wrote:
>>> On 26/09/2024 12:32, Frank Wang wrote:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - rockchip,rk3576-usb2phy
>>>> +    then:
>>>> +      properties:
>>>> +        clocks:
>>>> +          minItems: 3
>>>> +          maxItems: 3
>>> Read one more time the example I gave you. Top-level constraints are
>>> saying max one clock.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Sorry for overlooking this, I will set both "clocks" and "clock-names"
>> to true, and add the else case below the above codes for the "old" SoCs.
>> Just like the below.
>>
>> -  clocks:
>> -    maxItems: 1
>> +  clocks: true
>>
>> -  clock-names:
>> -    const: phyclk
>> +  clock-names: true
> For the third time, read the code I gave you. Do you see something like
> this there? Why doing all the time something different than existing code?

Refer to the link you sent me that I must add minItems property for 
clocks, just like the below codes:

@@ -35,7 +35,8 @@ properties:
      const: 0

    clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3

That can pass dt_binding and dtb checking, however, "clocks" is the 
optional property for some old Rockchip PHYs,  I am not sure is it right 
to force set  minItems as 1 .
If just keep maxItems, the dt_binding checking is failure.


Best regards,
Frank

> Best regards,
> Krzysztof
>

Heiko Stuebner Sept. 27, 2024, 8:02 a.m. UTC | #5

Hi Krzysztof,

Am Freitag, 27. September 2024, 09:30:30 CEST schrieb Krzysztof Kozlowski:
> On 27/09/2024 09:01, Frank Wang wrote:
> > Hi Krzysztof,
> > 
> > On 2024/9/26 22:19, Krzysztof Kozlowski wrote:
> >> On 26/09/2024 12:32, Frank Wang wrote:
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - rockchip,rk3576-usb2phy
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          minItems: 3
> >>> +          maxItems: 3
> >> Read one more time the example I gave you. Top-level constraints are
> >> saying max one clock.
> >>
> >> Best regards,
> >> Krzysztof
> >>
> > 
> > Sorry for overlooking this, I will set both "clocks" and "clock-names" 
> > to true, and add the else case below the above codes for the "old" SoCs.
> > Just like the below.
> > 
> > -  clocks:
> > -    maxItems: 1
> > +  clocks: true
> > 
> > -  clock-names:
> > -    const: phyclk
> > +  clock-names: true
> 
> For the third time, read the code I gave you. Do you see something like
> this there? Why doing all the time something different than existing code?

On vacation right now so late to the party, and somewhat confused :-) .

I've tried to find the code you mentioned, but did fail.
In [0] you mention "maybe oneOf". The other replies in that version were
about the ordering needing to stay for the older phy variants.

[1] in v2 has that NAK thing and [2] from v3 references that example again

I am probably just blind, but could use a pointer.


Because I think going with a
  - if:
      properties:
        compatible:
          contains:
            enum:
              - rockchip,rk3576-usb2phy
    then:
      properties:
        clocks:
          minItems: 3
          maxItems: 3
        clock-names:
          items:
            - const: phyclk
            - const: aclk
            - const: aclk_slv
    else:
      properties:
        clocks:
          maxItems: 1
        clock-names:
          const: phyclk

block should actually make sure each variant will check for the appropriate
number of clocks?

And having clocks:true in the main part then makes sure that the property
is not getting marked as:
arch/arm64/boot/dts/rockchip/rk3576-armsom-sige5.dtb: usb2-phy@0: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
        from schema $id: http://devicetree.org/schemas/phy/rockchip,inno-usb2phy.yaml#


Heiko


[0] https://lore.kernel.org/lkml/snccizbw6thn3lhwad4xppp7vqii4p56ttl2gufwc3ke7vfckf@e4b7nvwwtdfr/
[1] https://lore.kernel.org/lkml/[e-mail]/#t
[2] https://lore.kernel.org/lkml/[e-mail]/

Krzysztof Kozlowski Sept. 27, 2024, 9:52 a.m. UTC | #6

On 27/09/2024 09:59, Frank Wang wrote:
>>>
>>> -  clocks:
>>> -    maxItems: 1
>>> +  clocks: true
>>>
>>> -  clock-names:
>>> -    const: phyclk
>>> +  clock-names: true
>> For the third time, read the code I gave you. Do you see something like
>> this there? Why doing all the time something different than existing code?
> 
> Refer to the link you sent me that I must add minItems property for 
> clocks, just like the below codes:
> 
> @@ -35,7 +35,8 @@ properties:
>       const: 0
> 
>     clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3

Yes, for all variable properties, so also names.

> 
> That can pass dt_binding and dtb checking, however, "clocks" is the 
> optional property for some old Rockchip PHYs,  I am not sure is it right 
> to force set  minItems as 1 .
> If just keep maxItems, the dt_binding checking is failure.

Please specify the question you want to ask.


Best regards,
Krzysztof

Krzysztof Kozlowski Sept. 27, 2024, 9:56 a.m. UTC | #7

On 27/09/2024 10:02, Heiko Stuebner wrote:
> Hi Krzysztof,
> 
> Am Freitag, 27. September 2024, 09:30:30 CEST schrieb Krzysztof Kozlowski:
>> On 27/09/2024 09:01, Frank Wang wrote:
>>> Hi Krzysztof,
>>>
>>> On 2024/9/26 22:19, Krzysztof Kozlowski wrote:
>>>> On 26/09/2024 12:32, Frank Wang wrote:
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - rockchip,rk3576-usb2phy
>>>>> +    then:
>>>>> +      properties:
>>>>> +        clocks:
>>>>> +          minItems: 3
>>>>> +          maxItems: 3
>>>> Read one more time the example I gave you. Top-level constraints are
>>>> saying max one clock.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Sorry for overlooking this, I will set both "clocks" and "clock-names" 
>>> to true, and add the else case below the above codes for the "old" SoCs.
>>> Just like the below.
>>>
>>> -  clocks:
>>> -    maxItems: 1
>>> +  clocks: true
>>>
>>> -  clock-names:
>>> -    const: phyclk
>>> +  clock-names: true
>>
>> For the third time, read the code I gave you. Do you see something like
>> this there? Why doing all the time something different than existing code?
> 
> On vacation right now so late to the party, and somewhat confused :-) .
> 
> I've tried to find the code you mentioned, but did fail.
> In [0] you mention "maybe oneOf". The other replies in that version were
> about the ordering needing to stay for the older phy variants.
> 
> [1] in v2 has that NAK thing and [2] from v3 references that example again
> 
> I am probably just blind, but could use a pointer.

Oh, maybe I did not provide the link?

I apologize. I thought I gave reference to standard example. My bad.

Here it goes:
https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127


Best regards,
Krzysztof

Frank Wang Sept. 27, 2024, 10:49 a.m. UTC | #8

Hi Krzysztof,

On 2024/9/27 17:52, Krzysztof Kozlowski wrote:
> On 27/09/2024 09:59, Frank Wang wrote:
>>>> -  clocks:
>>>> -    maxItems: 1
>>>> +  clocks: true
>>>>
>>>> -  clock-names:
>>>> -    const: phyclk
>>>> +  clock-names: true
>>> For the third time, read the code I gave you. Do you see something like
>>> this there? Why doing all the time something different than existing code?
>> Refer to the link you sent me that I must add minItems property for
>> clocks, just like the below codes:
>>
>> @@ -35,7 +35,8 @@ properties:
>>        const: 0
>>
>>      clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
> Yes, for all variable properties, so also names.
>
>> That can pass dt_binding and dtb checking, however, "clocks" is the
>> optional property for some old Rockchip PHYs,  I am not sure is it right
>> to force set  minItems as 1 .
>> If just keep maxItems, the dt_binding checking is failure.
> Please specify the question you want to ask.
>

Q1: The "clocks" is an optional property for some old Rockchip PHYs, so 
set "minItems: 1" likes the below, is this a suitable setting?

    clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3Q2: Do you want me to amend the code to like this? 
clocks: minItems: 1 maxItems: 3 clock-names: minItems: 1 maxItems: 3 
allOf: [...] - if: properties: compatible: contains: enum: - 
rockchip,px30-usb2phy - rockchip,rk3128-usb2phy - 
rockchip,rk3228-usb2phy - rockchip,rk3308-usb2phy - 
rockchip,rk3328-usb2phy - rockchip,rk3366-usb2phy - 
rockchip,rk3399-usb2phy - rockchip,rk3568-usb2phy - 
rockchip,rk3588-usb2phy - rockchip,rv1108-usb2phy then: properties: 
clocks: maxItems: 1 clock-names: const: phyclk - if: properties: 
compatible: contains: enum: - rockchip,rk3576-usb2phy then: properties: 
clocks: minItems: 3 maxItems: 3 clock-names: items: - const: phyclk - 
const: aclk - const: aclk_slv


Best regards,
Frank

> Best regards,
> Krzysztof
>

Frank Wang Sept. 27, 2024, 10:59 a.m. UTC | #9

Hi Krzysztof,

On 2024/9/27 17:52, Krzysztof Kozlowski wrote:
> On 27/09/2024 09:59, Frank Wang wrote:
>>>> -  clocks:
>>>> -    maxItems: 1
>>>> +  clocks: true
>>>>
>>>> -  clock-names:
>>>> -    const: phyclk
>>>> +  clock-names: true
>>> For the third time, read the code I gave you. Do you see something like
>>> this there? Why doing all the time something different than existing code?
>> Refer to the link you sent me that I must add minItems property for
>> clocks, just like the below codes:
>>
>> @@ -35,7 +35,8 @@ properties:
>>        const: 0
>>
>>      clocks:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 3
> Yes, for all variable properties, so also names.
>
>> That can pass dt_binding and dtb checking, however, "clocks" is the
>> optional property for some old Rockchip PHYs,  I am not sure is it right
>> to force set  minItems as 1 .
>> If just keep maxItems, the dt_binding checking is failure.
> Please specify the question you want to ask.
>

Please ignore my last garbled email as mailbox client issue.

Q1: The "clocks" is an optional property for some old Rockchip PHYs, so 
set "minItems: 1" likes the below, is this a suitable setting?

     clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3

Q2: Do you want me to amend the code to like this?

    clocks:
     minItems: 1
     maxItems: 3

    clock-names:
     minItems: 1
     maxItems: 3

allOf:

[...]

   - if:
       properties:
         compatible:
           contains:
             enum:
               - rockchip,px30-usb2phy
               - rockchip,rk3128-usb2phy
               - rockchip,rk3228-usb2phy
               - rockchip,rk3308-usb2phy
               - rockchip,rk3328-usb2phy
               - rockchip,rk3366-usb2phy
               - rockchip,rk3399-usb2phy
               - rockchip,rk3568-usb2phy
               - rockchip,rk3588-usb2phy
               - rockchip,rv1108-usb2phy
     then:
       properties:
         clocks:
           maxItems: 1
         clock-names:
           const: phyclk

   - if:
       properties:
         compatible:
           contains:
             enum:
               - rockchip,rk3576-usb2phy
     then:
       properties:
         clocks:
           minItems: 3
           maxItems: 3
         clock-names:
           items:
             - const: phyclk
             - const: aclk
             - const: aclk_slv

BR.
Frank

> Best regards,
> Krzysztof
>

Krzysztof Kozlowski Sept. 27, 2024, 12:05 p.m. UTC | #10

On 27/09/2024 12:59, Frank Wang wrote:
> Hi Krzysztof,
> 
> On 2024/9/27 17:52, Krzysztof Kozlowski wrote:
>> On 27/09/2024 09:59, Frank Wang wrote:
>>>>> -  clocks:
>>>>> -    maxItems: 1
>>>>> +  clocks: true
>>>>>
>>>>> -  clock-names:
>>>>> -    const: phyclk
>>>>> +  clock-names: true
>>>> For the third time, read the code I gave you. Do you see something like
>>>> this there? Why doing all the time something different than existing code?
>>> Refer to the link you sent me that I must add minItems property for
>>> clocks, just like the below codes:
>>>
>>> @@ -35,7 +35,8 @@ properties:
>>>        const: 0
>>>
>>>      clocks:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 3
>> Yes, for all variable properties, so also names.
>>
>>> That can pass dt_binding and dtb checking, however, "clocks" is the
>>> optional property for some old Rockchip PHYs,  I am not sure is it right
>>> to force set  minItems as 1 .
>>> If just keep maxItems, the dt_binding checking is failure.
>> Please specify the question you want to ask.
>>
> 
> Please ignore my last garbled email as mailbox client issue.
> 
> Q1: The "clocks" is an optional property for some old Rockchip PHYs, so 
> set "minItems: 1" likes the below, is this a suitable setting?
> 
>      clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3
> 
> Q2: Do you want me to amend the code to like this?
> 
>     clocks:
>      minItems: 1
>      maxItems: 3
> 
>     clock-names:
>      minItems: 1
>      maxItems: 3

List should be here with minItems. Then you only define the constraints
in if:then:

Best regards,
Krzysztof

Patch

diff --git a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
index 5254413137c6..d286753bd53f 100644
--- a/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip,inno-usb2phy.yaml
@@ -20,6 +20,7 @@  properties:
       - rockchip,rk3366-usb2phy
       - rockchip,rk3399-usb2phy
       - rockchip,rk3568-usb2phy
+      - rockchip,rk3576-usb2phy
       - rockchip,rk3588-usb2phy
       - rockchip,rv1108-usb2phy
 
@@ -143,6 +144,7 @@  allOf:
           contains:
             enum:
               - rockchip,rk3568-usb2phy
+              - rockchip,rk3576-usb2phy
               - rockchip,rk3588-usb2phy
 
     then:
@@ -171,6 +173,22 @@  allOf:
           required:
             - interrupts
             - interrupt-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3576-usb2phy
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: phyclk
+            - const: aclk
+            - const: aclk_slv
 
 additionalProperties: false