diff mbox series

[1/2] dt-bindings: usb: renesas, usb3-peri: Document RZ/V2M r9a09g011 support

Message ID 20220718134458.19137-2-phil.edworthy@renesas.com
State Superseded
Headers show
Series Add usb gadget support for RZ/V2M | expand

Commit Message

Phil Edworthy July 18, 2022, 1:44 p.m. UTC 
Document the RZ/V2M SoC bindings.
The RZ/V2M SoC is a little different to the R-Car implementations.
A few DRD related registers and bits have moved, there is a separate
interrupt for DRD, an additional clock for register access and reset
lines for DRD and USBP.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/usb/renesas,usb3-peri.yaml       | 81 +++++++++++++++----
 1 file changed, 66 insertions(+), 15 deletions(-)

Comments

Phil Edworthy July 18, 2022, 3:24 p.m. UTC | #1 
Hi Krzysztof,

Thanks for your review!

On 18 July 2022 14:55 Krzysztof Kozlowski wrote:
> On 18/07/2022 15:44, Phil Edworthy wrote:
> > Document the RZ/V2M SoC bindings.
> > The RZ/V2M SoC is a little different to the R-Car implementations.
> > A few DRD related registers and bits have moved, there is a separate
> > interrupt for DRD, an additional clock for register access and reset
> > lines for DRD and USBP.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../bindings/usb/renesas,usb3-peri.yaml       | 81 +++++++++++++++----
> >  1 file changed, 66 insertions(+), 15 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3-
> peri.yaml b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> > index 9fcf54b10b07..28f785dd2012 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> > +++ b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> > @@ -11,27 +11,49 @@ maintainers:
<snip>

> >    interrupts:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: Combined interrupt for DMA, SYS and ERR
> > +      - description: Dual Role Device (DRD)
> > +
> > +  interrupt-names:
> 
> minItems:1
> 
> > +    items:
> > +      - const: all_p
> > +      - const: drd
> >
> >    clocks:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: Main clock
> > +      - description: Register access clock
> > +
> > +  clock-names:
> 
> minItems:1
> 
> > +    items:
> > +      - const: aclk
> > +      - const: reg
> >
> >    phys:
> >      maxItems: 1
> > @@ -43,7 +65,15 @@ properties:
> >      maxItems: 1
> >
> >    resets:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: Peripheral reset
> > +      - description: DRD reset
> > +
> > +  reset-names:
> > +    items:
> > +      - const: aresetn_p
> > +      - const: drd_reset
> >
> >    usb-role-switch:
> >      $ref: /schemas/types.yaml#/definitions/flag
> > @@ -78,6 +108,27 @@ required:
> >    - interrupts
> >    - clocks
> >
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - renesas,rzv2m-usb3-peri
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +        interrupts:
> > +          minItems: 2
> > +        resets:
> > +          minItems: 2
> > +      required:
> > +        - clock-names
> > +        - interrupt-names
> > +        - resets
> > +        - reset-names
> 
> else:
> narrow the number of items
Sorry, I don't understand why we need minItems: 1 for
interrupt-names/clock-names, but then I'm easily confused!

None of the existing users have any interrupt-names/clock-names
hence they are not in required. The rzv2m is the only device
that needs them so the driver can get them by name, and hence
it sets minItems: 2

Thanks
Phil
Krzysztof Kozlowski July 19, 2022, 6:37 a.m. UTC | #2 
On 18/07/2022 17:24, Phil Edworthy wrote:
>>>    phys:
>>>      maxItems: 1
>>> @@ -43,7 +65,15 @@ properties:
>>>      maxItems: 1
>>>
>>>    resets:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    items:
>>> +      - description: Peripheral reset
>>> +      - description: DRD reset
>>> +
>>> +  reset-names:
>>> +    items:
>>> +      - const: aresetn_p
>>> +      - const: drd_reset
>>>
>>>    usb-role-switch:
>>>      $ref: /schemas/types.yaml#/definitions/flag
>>> @@ -78,6 +108,27 @@ required:
>>>    - interrupts
>>>    - clocks
>>>
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - renesas,rzv2m-usb3-peri
>>> +    then:
>>> +      properties:
>>> +        clocks:
>>> +          minItems: 2
>>> +        interrupts:
>>> +          minItems: 2
>>> +        resets:
>>> +          minItems: 2
>>> +      required:
>>> +        - clock-names
>>> +        - interrupt-names
>>> +        - resets
>>> +        - reset-names
>>
>> else:
>> narrow the number of items
> Sorry, I don't understand why we need minItems: 1 for
> interrupt-names/clock-names, but then I'm easily confused!
> 
> None of the existing users have any interrupt-names/clock-names
> hence they are not in required. The rzv2m is the only device
> that needs them so the driver can get them by name, and hence
> it sets minItems: 2

They are not required but they can appear. Nothing prevents it, based on
your patch.


Best regards,
Krzysztof
Phil Edworthy July 19, 2022, 9:01 a.m. UTC | #3 
Hi Krzysztof,

On 19 July 2022 07:38 Krzysztof Kozlowski wrote:
> On 18/07/2022 17:24, Phil Edworthy wrote:
> >>>    phys:
> >>>      maxItems: 1
> >>> @@ -43,7 +65,15 @@ properties:
> >>>      maxItems: 1
> >>>
> >>>    resets:
> >>> -    maxItems: 1
> >>> +    minItems: 1
> >>> +    items:
> >>> +      - description: Peripheral reset
> >>> +      - description: DRD reset
> >>> +
> >>> +  reset-names:
> >>> +    items:
> >>> +      - const: aresetn_p
> >>> +      - const: drd_reset
> >>>
> >>>    usb-role-switch:
> >>>      $ref: /schemas/types.yaml#/definitions/flag
> >>> @@ -78,6 +108,27 @@ required:
> >>>    - interrupts
> >>>    - clocks
> >>>
> >>> +allOf:
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - renesas,rzv2m-usb3-peri
> >>> +    then:
> >>> +      properties:
> >>> +        clocks:
> >>> +          minItems: 2
+        clock-names:
+          minItems: 2
(See below)

> >>> +        interrupts:
> >>> +          minItems: 2
+        interrupt-names:
+          minItems: 2
(See below)

> >>> +        resets:
> >>> +          minItems: 2
> >>> +      required:
> >>> +        - clock-names
> >>> +        - interrupt-names
> >>> +        - resets
> >>> +        - reset-names
> >>
> >> else:
> >> narrow the number of items
> > Sorry, I don't understand why we need minItems: 1 for
> > interrupt-names/clock-names, but then I'm easily confused!
> >
> > None of the existing users have any interrupt-names/clock-names
> > hence they are not in required. The rzv2m is the only device
> > that needs them so the driver can get them by name, and hence
> > it sets minItems: 2
> 
> They are not required but they can appear. Nothing prevents it, based on
> your patch.

Ok, but instead of 'else: narrow the number of items', shouldn't I
set the clock-names/interrupt-names 'minItems: 2' for rzv2m as above?
This is because they will now default to 'minItems: 1' after your
comment?

Thanks
Phil
Krzysztof Kozlowski July 19, 2022, 9:13 a.m. UTC | #4 
On 19/07/2022 11:01, Phil Edworthy wrote:
> Hi Krzysztof,
> 
> On 19 July 2022 07:38 Krzysztof Kozlowski wrote:
>> On 18/07/2022 17:24, Phil Edworthy wrote:
>>>>>    phys:
>>>>>      maxItems: 1
>>>>> @@ -43,7 +65,15 @@ properties:
>>>>>      maxItems: 1
>>>>>
>>>>>    resets:
>>>>> -    maxItems: 1
>>>>> +    minItems: 1
>>>>> +    items:
>>>>> +      - description: Peripheral reset
>>>>> +      - description: DRD reset
>>>>> +
>>>>> +  reset-names:
>>>>> +    items:
>>>>> +      - const: aresetn_p
>>>>> +      - const: drd_reset
>>>>>
>>>>>    usb-role-switch:
>>>>>      $ref: /schemas/types.yaml#/definitions/flag
>>>>> @@ -78,6 +108,27 @@ required:
>>>>>    - interrupts
>>>>>    - clocks
>>>>>
>>>>> +allOf:
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - renesas,rzv2m-usb3-peri
>>>>> +    then:
>>>>> +      properties:
>>>>> +        clocks:
>>>>> +          minItems: 2
> +        clock-names:
> +          minItems: 2
> (See below)
> 
>>>>> +        interrupts:
>>>>> +          minItems: 2
> +        interrupt-names:
> +          minItems: 2
> (See below)
> 
>>>>> +        resets:
>>>>> +          minItems: 2
>>>>> +      required:
>>>>> +        - clock-names
>>>>> +        - interrupt-names
>>>>> +        - resets
>>>>> +        - reset-names
>>>>
>>>> else:
>>>> narrow the number of items
>>> Sorry, I don't understand why we need minItems: 1 for
>>> interrupt-names/clock-names, but then I'm easily confused!
>>>
>>> None of the existing users have any interrupt-names/clock-names
>>> hence they are not in required. The rzv2m is the only device
>>> that needs them so the driver can get them by name, and hence
>>> it sets minItems: 2
>>
>> They are not required but they can appear. Nothing prevents it, based on
>> your patch.
> 
> Ok, but instead of 'else: narrow the number of items', shouldn't I
> set the clock-names/interrupt-names 'minItems: 2' for rzv2m as above?

Yes.

Best regards,
Krzysztof
Phil Edworthy July 21, 2022, 10:58 a.m. UTC | #5 
Hi

On 18 July 2022 14:45 Phil Edworthy wrote:
> Document the RZ/V2M SoC bindings.
> The RZ/V2M SoC is a little different to the R-Car implementations.
> A few DRD related registers and bits have moved, there is a separate
> interrupt for DRD, an additional clock for register access and reset
> lines for DRD and USBP.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../bindings/usb/renesas,usb3-peri.yaml       | 81 +++++++++++++++----
>  1 file changed, 66 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> index 9fcf54b10b07..28f785dd2012 100644
> --- a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> +++ b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> @@ -11,27 +11,49 @@ maintainers:
> 
<snip>
>    interrupts:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: Combined interrupt for DMA, SYS and ERR
> +      - description: Dual Role Device (DRD)

Note to reviewers:
I have found two other interrupts related to DRD that need to be added
to the bindings: "Battery Charging" and "Global Purpose Input".
These will be added in the next version.

Rgds
Phil
Geert Uytterhoeven July 21, 2022, 12:12 p.m. UTC | #6 
Hi Phil,

On Mon, Jul 18, 2022 at 3:45 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> Document the RZ/V2M SoC bindings.
> The RZ/V2M SoC is a little different to the R-Car implementations.
> A few DRD related registers and bits have moved, there is a separate
> interrupt for DRD, an additional clock for register access and reset
> lines for DRD and USBP.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

The grant scheme looks good to me, so I'm awaiting your v2, incorporating
the review comments, and the additional interrupts.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
index 9fcf54b10b07..28f785dd2012 100644
--- a/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
+++ b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
@@ -11,27 +11,49 @@  maintainers:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - renesas,r8a774a1-usb3-peri # RZ/G2M
-          - renesas,r8a774b1-usb3-peri # RZ/G2N
-          - renesas,r8a774c0-usb3-peri # RZ/G2E
-          - renesas,r8a774e1-usb3-peri # RZ/G2H
-          - renesas,r8a7795-usb3-peri  # R-Car H3
-          - renesas,r8a7796-usb3-peri  # R-Car M3-W
-          - renesas,r8a77961-usb3-peri # R-Car M3-W+
-          - renesas,r8a77965-usb3-peri # R-Car M3-N
-          - renesas,r8a77990-usb3-peri # R-Car E3
-      - const: renesas,rcar-gen3-usb3-peri
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r8a774a1-usb3-peri # RZ/G2M
+              - renesas,r8a774b1-usb3-peri # RZ/G2N
+              - renesas,r8a774c0-usb3-peri # RZ/G2E
+              - renesas,r8a774e1-usb3-peri # RZ/G2H
+              - renesas,r8a7795-usb3-peri  # R-Car H3
+              - renesas,r8a7796-usb3-peri  # R-Car M3-W
+              - renesas,r8a77961-usb3-peri # R-Car M3-W+
+              - renesas,r8a77965-usb3-peri # R-Car M3-N
+              - renesas,r8a77990-usb3-peri # R-Car E3
+          - const: renesas,rcar-gen3-usb3-peri
+
+      - items:
+          - enum:
+              - renesas,r9a09g011-usb3-peri # RZ/V2M
+          - const: renesas,rzv2m-usb3-peri
 
   reg:
     maxItems: 1
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Combined interrupt for DMA, SYS and ERR
+      - description: Dual Role Device (DRD)
+
+  interrupt-names:
+    items:
+      - const: all_p
+      - const: drd
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Main clock
+      - description: Register access clock
+
+  clock-names:
+    items:
+      - const: aclk
+      - const: reg
 
   phys:
     maxItems: 1
@@ -43,7 +65,15 @@  properties:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: Peripheral reset
+      - description: DRD reset
+
+  reset-names:
+    items:
+      - const: aresetn_p
+      - const: drd_reset
 
   usb-role-switch:
     $ref: /schemas/types.yaml#/definitions/flag
@@ -78,6 +108,27 @@  required:
   - interrupts
   - clocks
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rzv2m-usb3-peri
+    then:
+      properties:
+        clocks:
+          minItems: 2
+        interrupts:
+          minItems: 2
+        resets:
+          minItems: 2
+      required:
+        - clock-names
+        - interrupt-names
+        - resets
+        - reset-names
+
 additionalProperties: false
 
 examples: