diff mbox series

[v4,5/7] dt-bindings: usb: ci-hdrc-usb2: add compatible and clock-names restriction for imx93

Message ID 20240119071936.3481439-5-xu.yang_2@nxp.com (mailing list archive)
State Superseded
Headers show
Series [v4,1/7] dt-bindings: usb: usbmisc-imx: add fsl,imx8ulp-usbmisc compatible | expand

Commit Message

Xu Yang Jan. 19, 2024, 7:19 a.m. UTC
The i.MX93 needs a wakup clock to work properly. This will add compatible
and restriction for i.MX93 platform.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - no changes
Changes in v3:
 - add clocks restriction
Changes in v4:
 - use 'contains' rather 'items'
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 47 ++++++++++++++-----
 1 file changed, 34 insertions(+), 13 deletions(-)

Comments

Krzysztof Kozlowski Jan. 29, 2024, 3:51 p.m. UTC | #1
On 19/01/2024 08:19, Xu Yang wrote:
> The i.MX93 needs a wakup clock to work properly. This will add compatible
> and restriction for i.MX93 platform.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - no changes
> Changes in v3:
>  - add clocks restriction
> Changes in v4:
>  - use 'contains' rather 'items'
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 47 ++++++++++++++-----
>  1 file changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> index 78e30ca0a8ca..a86cb5de1688 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> @@ -57,6 +57,7 @@ properties:
>            - enum:
>                - fsl,imx8mm-usb
>                - fsl,imx8mn-usb
> +              - fsl,imx93-usb
>            - const: fsl,imx7d-usb
>            - const: fsl,imx27-usb
>        - items:
> @@ -455,19 +456,23 @@ allOf:
>    - if:
>        properties:
>          compatible:
> -          contains:
> -            oneOf:
> -              - const: chipidea,usb2
> -              - const: fsl,imx23-usb
> -              - const: fsl,imx28-usb
> -              - const: fsl,imx7d-usb
> -              - const: fsl,vf610-usb
> -              - const: lsi,zevio-usb
> -              - const: nuvoton,npcm750-udc
> -              - pattern: '^fsl,imx5[0-3]+-usb$'
> -              - pattern: '^fsl,imx6[a-z]+-usb$'
> -              - pattern: '^nvidia,tegra[0-9]+-ehci$'
> -              - pattern: '^nvidia,tegra[0-9]+-udc$'
> +          oneOf:
> +            - contains:
> +                oneOf:

No, this does not make sense.

> +                  - const: chipidea,usb2
> +                  - const: fsl,imx23-usb

Look here is a place for your compatible... unless imx93 is not really
compatible, but then why pretending it is?

> +                  - const: fsl,imx28-usb
> +                  - const: fsl,vf610-usb
> +                  - const: lsi,zevio-usb
> +                  - const: nuvoton,npcm750-udc
> +                  - pattern: '^fsl,imx5[0-3]+-usb$'
> +                  - pattern: '^fsl,imx6[a-z]+-usb$'
> +                  - pattern: '^fsl,imx8[a-z]+-usb$'
> +                  - pattern: '^nvidia,tegra[0-9]+-ehci$'
> +                  - pattern: '^nvidia,tegra[0-9]+-udc$'
> +            - items:
> +                - const: fsl,imx7d-usb
> +                - const: fsl,imx27-usb
>      then:
>        properties:
>          clocks:
> @@ -477,6 +482,22 @@ allOf:
>            minItems: 1
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx93-usb
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +        clock-names:
> +          minItems: 2
> +          maxItems: 2
> +          contains:
> +            const: usb_wakeup_clk

Drop "_clk" suffix.

Best regards,
Krzysztof
Xu Yang Jan. 31, 2024, 8:36 a.m. UTC | #2
Hi Krzysztof,

> 
> On 19/01/2024 08:19, Xu Yang wrote:
> > The i.MX93 needs a wakup clock to work properly. This will add compatible
> > and restriction for i.MX93 platform.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes in v2:
> >  - no changes
> > Changes in v3:
> >  - add clocks restriction
> > Changes in v4:
> >  - use 'contains' rather 'items'
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 47 ++++++++++++++-----
> >  1 file changed, 34 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
> hdrc-usb2.yaml
> > index 78e30ca0a8ca..a86cb5de1688 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
> > @@ -57,6 +57,7 @@ properties:
> >            - enum:
> >                - fsl,imx8mm-usb
> >                - fsl,imx8mn-usb
> > +              - fsl,imx93-usb
> >            - const: fsl,imx7d-usb
> >            - const: fsl,imx27-usb
> >        - items:
> > @@ -455,19 +456,23 @@ allOf:
> >    - if:
> >        properties:
> >          compatible:
> > -          contains:
> > -            oneOf:
> > -              - const: chipidea,usb2
> > -              - const: fsl,imx23-usb
> > -              - const: fsl,imx28-usb
> > -              - const: fsl,imx7d-usb
> > -              - const: fsl,vf610-usb
> > -              - const: lsi,zevio-usb
> > -              - const: nuvoton,npcm750-udc
> > -              - pattern: '^fsl,imx5[0-3]+-usb$'
> > -              - pattern: '^fsl,imx6[a-z]+-usb$'
> > -              - pattern: '^nvidia,tegra[0-9]+-ehci$'
> > -              - pattern: '^nvidia,tegra[0-9]+-udc$'
> > +          oneOf:
> > +            - contains:
> > +                oneOf:
> 
> No, this does not make sense.
> 
> > +                  - const: chipidea,usb2
> > +                  - const: fsl,imx23-usb
> 
> Look here is a place for your compatible... unless imx93 is not really
> compatible, but then why pretending it is?

I can't put fsl,imx93-usb here since imx93 needs 2 clocks. So I add another
if-then limitation for imx93 below this part.

> 
> > +                  - const: fsl,imx28-usb
> > +                  - const: fsl,vf610-usb
> > +                  - const: lsi,zevio-usb
> > +                  - const: nuvoton,npcm750-udc
> > +                  - pattern: '^fsl,imx5[0-3]+-usb$'
> > +                  - pattern: '^fsl,imx6[a-z]+-usb$'
> > +                  - pattern: '^fsl,imx8[a-z]+-usb$'
> > +                  - pattern: '^nvidia,tegra[0-9]+-ehci$'
> > +                  - pattern: '^nvidia,tegra[0-9]+-udc$'
> > +            - items:
> > +                - const: fsl,imx7d-usb
> > +                - const: fsl,imx27-usb
> >      then:
> >        properties:
> >          clocks:
> > @@ -477,6 +482,22 @@ allOf:
> >            minItems: 1
> >            maxItems: 1
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,imx93-usb
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 2
> > +        clock-names:
> > +          minItems: 2
> > +          maxItems: 2
> > +          contains:
> > +            const: usb_wakeup_clk
> 
> Drop "_clk" suffix.

Okay.

Thanks,
Xu Yang
Krzysztof Kozlowski Jan. 31, 2024, 8:45 a.m. UTC | #3
On 31/01/2024 09:36, Xu Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 19/01/2024 08:19, Xu Yang wrote:
>>> The i.MX93 needs a wakup clock to work properly. This will add compatible
>>> and restriction for i.MX93 platform.
>>>
>>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>>>
>>> ---
>>> Changes in v2:
>>>  - no changes
>>> Changes in v3:
>>>  - add clocks restriction
>>> Changes in v4:
>>>  - use 'contains' rather 'items'
>>> ---
>>>  .../devicetree/bindings/usb/ci-hdrc-usb2.yaml | 47 ++++++++++++++-----
>>>  1 file changed, 34 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-
>> hdrc-usb2.yaml
>>> index 78e30ca0a8ca..a86cb5de1688 100644
>>> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
>>> @@ -57,6 +57,7 @@ properties:
>>>            - enum:
>>>                - fsl,imx8mm-usb
>>>                - fsl,imx8mn-usb
>>> +              - fsl,imx93-usb
>>>            - const: fsl,imx7d-usb
>>>            - const: fsl,imx27-usb
>>>        - items:
>>> @@ -455,19 +456,23 @@ allOf:
>>>    - if:
>>>        properties:
>>>          compatible:
>>> -          contains:
>>> -            oneOf:
>>> -              - const: chipidea,usb2
>>> -              - const: fsl,imx23-usb
>>> -              - const: fsl,imx28-usb
>>> -              - const: fsl,imx7d-usb
>>> -              - const: fsl,vf610-usb
>>> -              - const: lsi,zevio-usb
>>> -              - const: nuvoton,npcm750-udc
>>> -              - pattern: '^fsl,imx5[0-3]+-usb$'
>>> -              - pattern: '^fsl,imx6[a-z]+-usb$'
>>> -              - pattern: '^nvidia,tegra[0-9]+-ehci$'
>>> -              - pattern: '^nvidia,tegra[0-9]+-udc$'
>>> +          oneOf:
>>> +            - contains:
>>> +                oneOf:
>>
>> No, this does not make sense.
>>
>>> +                  - const: chipidea,usb2
>>> +                  - const: fsl,imx23-usb
>>
>> Look here is a place for your compatible... unless imx93 is not really
>> compatible, but then why pretending it is?
> 
> I can't put fsl,imx93-usb here since imx93 needs 2 clocks. So I add another
> if-then limitation for imx93 below this part.

Hm, ok, but anyway I don't want to go deeper in this indentation. It
does not make things easier to read.

> 
>>
>>> +                  - const: fsl,imx28-usb
>>> +                  - const: fsl,vf610-usb
>>> +                  - const: lsi,zevio-usb
>>> +                  - const: nuvoton,npcm750-udc
>>> +                  - pattern: '^fsl,imx5[0-3]+-usb$'
>>> +                  - pattern: '^fsl,imx6[a-z]+-usb$'
>>> +                  - pattern: '^fsl,imx8[a-z]+-usb$'
>>> +                  - pattern: '^nvidia,tegra[0-9]+-ehci$'
>>> +                  - pattern: '^nvidia,tegra[0-9]+-udc$'
>>> +            - items:
>>> +                - const: fsl,imx7d-usb
>>> +                - const: fsl,imx27-usb
>>>      then:
>>>        properties:
>>>          clocks:
>>> @@ -477,6 +482,22 @@ allOf:
>>>            minItems: 1
>>>            maxItems: 1
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: fsl,imx93-usb
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 2
>>> +          maxItems: 2
>>> +        clock-names:
>>> +          minItems: 2
>>> +          maxItems: 2
>>> +          contains:
>>> +            const: usb_wakeup_clk
>>
>> Drop "_clk" suffix.
> 

Anyway neither this nor previous patch has the entries constrained. They
must be specific, so constrained.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
index 78e30ca0a8ca..a86cb5de1688 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
@@ -57,6 +57,7 @@  properties:
           - enum:
               - fsl,imx8mm-usb
               - fsl,imx8mn-usb
+              - fsl,imx93-usb
           - const: fsl,imx7d-usb
           - const: fsl,imx27-usb
       - items:
@@ -455,19 +456,23 @@  allOf:
   - if:
       properties:
         compatible:
-          contains:
-            oneOf:
-              - const: chipidea,usb2
-              - const: fsl,imx23-usb
-              - const: fsl,imx28-usb
-              - const: fsl,imx7d-usb
-              - const: fsl,vf610-usb
-              - const: lsi,zevio-usb
-              - const: nuvoton,npcm750-udc
-              - pattern: '^fsl,imx5[0-3]+-usb$'
-              - pattern: '^fsl,imx6[a-z]+-usb$'
-              - pattern: '^nvidia,tegra[0-9]+-ehci$'
-              - pattern: '^nvidia,tegra[0-9]+-udc$'
+          oneOf:
+            - contains:
+                oneOf:
+                  - const: chipidea,usb2
+                  - const: fsl,imx23-usb
+                  - const: fsl,imx28-usb
+                  - const: fsl,vf610-usb
+                  - const: lsi,zevio-usb
+                  - const: nuvoton,npcm750-udc
+                  - pattern: '^fsl,imx5[0-3]+-usb$'
+                  - pattern: '^fsl,imx6[a-z]+-usb$'
+                  - pattern: '^fsl,imx8[a-z]+-usb$'
+                  - pattern: '^nvidia,tegra[0-9]+-ehci$'
+                  - pattern: '^nvidia,tegra[0-9]+-udc$'
+            - items:
+                - const: fsl,imx7d-usb
+                - const: fsl,imx27-usb
     then:
       properties:
         clocks:
@@ -477,6 +482,22 @@  allOf:
           minItems: 1
           maxItems: 1
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx93-usb
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+        clock-names:
+          minItems: 2
+          maxItems: 2
+          contains:
+            const: usb_wakeup_clk
+
   - if:
       properties:
         compatible: