diff mbox series

[v3,3/4] dt-bindings: usb: realtek,rts5411: Adapt usb-hub.yaml

Message ID 20250422082957.2058229-4-treapking@chromium.org
State New
Headers show
Series [v3,1/4] dt-bindings: usb: Introduce usb-hub.yaml | expand

Commit Message

Pin-yen Lin April 22, 2025, 8:28 a.m. UTC
Inherit usb-hub.yaml and remove duplicated schemas.

Signed-off-by: Pin-yen Lin <treapking@chromium.org>

---

Changes in v3:
- Remove redundant schemas
- Update the schema for downstream ports and devices

Changes in v2:
- New in v2

 .../bindings/usb/realtek,rts5411.yaml         | 52 +++++--------------
 1 file changed, 13 insertions(+), 39 deletions(-)

Comments

Rob Herring April 25, 2025, 8:05 p.m. UTC | #1
On Tue, 22 Apr 2025 16:28:29 +0800, Pin-yen Lin wrote:
> Inherit usb-hub.yaml and remove duplicated schemas.
> 
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> 
> ---
> 
> Changes in v3:
> - Remove redundant schemas
> - Update the schema for downstream ports and devices
> 
> Changes in v2:
> - New in v2
> 
>  .../bindings/usb/realtek,rts5411.yaml         | 52 +++++--------------
>  1 file changed, 13 insertions(+), 39 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Stephen Boyd April 28, 2025, 11:45 p.m. UTC | #2
Quoting Pin-yen Lin (2025-04-22 01:28:29)
> diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> index 6577a61cc07531..a020afaf2d6e7a 100644
> --- a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> +++ b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> @@ -10,7 +10,7 @@ maintainers:
>    - Matthias Kaehlcke <mka@chromium.org>
>
>  allOf:
> -  - $ref: usb-device.yaml#
> +  - $ref: usb-hub.yaml#
>
>  properties:
>    compatible:
> @@ -19,61 +19,35 @@ properties:
[...]
>
> -      port@4:
> -        $ref: /schemas/graph.yaml#/properties/port
> -        description:
> -          4th downstream facing USB port
> -
> -patternProperties:
> -  '^.*@[1-4]$':
> -    description: The hard wired USB devices
> -    type: object
> -    $ref: /schemas/usb/usb-device.yaml
> -    additionalProperties: true
> +additionalProperties:
> +  properties:
> +    reg:
> +      minimum: 1
> +      maximum: 4

Is this limiting the 'reg' property of the hub node and not the child
usb-device nodes?

>
>  required:
>    - peer-hub
>    - compatible
>    - reg

Can 'reg' be dropped because usb-hub.yaml requires it?
Stephen Boyd April 29, 2025, 6:24 p.m. UTC | #3
Quoting Pin-yen Lin (2025-04-28 21:57:16)
> Hi Stephen,
>
> On Tue, Apr 29, 2025 at 7:46 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Quoting Pin-yen Lin (2025-04-22 01:28:29)
> > > diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > index 6577a61cc07531..a020afaf2d6e7a 100644
> > > --- a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > @@ -10,7 +10,7 @@ maintainers:
> > >    - Matthias Kaehlcke <mka@chromium.org>
> > >
> > >  allOf:
> > > -  - $ref: usb-device.yaml#
> > > +  - $ref: usb-hub.yaml#
> > >
> > >  properties:
> > >    compatible:
> > > @@ -19,61 +19,35 @@ properties:
> > [...]
> > >
> > > -      port@4:
> > > -        $ref: /schemas/graph.yaml#/properties/port
> > > -        description:
> > > -          4th downstream facing USB port
> > > -
> > > -patternProperties:
> > > -  '^.*@[1-4]$':
> > > -    description: The hard wired USB devices
> > > -    type: object
> > > -    $ref: /schemas/usb/usb-device.yaml
> > > -    additionalProperties: true
> > > +additionalProperties:
> > > +  properties:
> > > +    reg:
> > > +      minimum: 1
> > > +      maximum: 4
> >
> > Is this limiting the 'reg' property of the hub node and not the child
> > usb-device nodes?
>
> Yes, but the regex pattern of patternProperties restricts the
> downstream device nodes to '^.*@[1-4]$'. So the 'reg's of the child
> usb-device nodes are also checked.

I'm confused. The path looks like it is removing patternProperties here
and limiting the reg property of the hub itself so it can only be at
port 1, 2, 3 or 4. Why is the patternProperties being removed? Don't we
need to keep the patternProperties around, or somehow get at the
patternProperties for the hard wired USB devices in the usb-hub schema
so we can constrain the reg property to be between 1 and 4?

> >
> > >
> > >  required:
> > >    - peer-hub
> > >    - compatible
> > >    - reg
> >
> > Can 'reg' be dropped because usb-hub.yaml requires it?
>
> I can drop 'reg' and 'compatible' in the next version, but I see other
> schemas referencing usb-device.yaml still set 'reg' as required. Is
> this some kind of convention, or people just didn't notice this?

I assume nobody noticed.
Pin-yen Lin April 30, 2025, 1:17 p.m. UTC | #4
On Wed, Apr 30, 2025 at 2:24 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Pin-yen Lin (2025-04-28 21:57:16)
> > Hi Stephen,
> >
> > On Tue, Apr 29, 2025 at 7:46 AM Stephen Boyd <swboyd@chromium.org> wrote:
> > >
> > > Quoting Pin-yen Lin (2025-04-22 01:28:29)
> > > > diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > > index 6577a61cc07531..a020afaf2d6e7a 100644
> > > > --- a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > > +++ b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
> > > > @@ -10,7 +10,7 @@ maintainers:
> > > >    - Matthias Kaehlcke <mka@chromium.org>
> > > >
> > > >  allOf:
> > > > -  - $ref: usb-device.yaml#
> > > > +  - $ref: usb-hub.yaml#
> > > >
> > > >  properties:
> > > >    compatible:
> > > > @@ -19,61 +19,35 @@ properties:
> > > [...]
> > > >
> > > > -      port@4:
> > > > -        $ref: /schemas/graph.yaml#/properties/port
> > > > -        description:
> > > > -          4th downstream facing USB port
> > > > -
> > > > -patternProperties:
> > > > -  '^.*@[1-4]$':
> > > > -    description: The hard wired USB devices
> > > > -    type: object
> > > > -    $ref: /schemas/usb/usb-device.yaml
> > > > -    additionalProperties: true
> > > > +additionalProperties:
> > > > +  properties:
> > > > +    reg:
> > > > +      minimum: 1
> > > > +      maximum: 4
> > >
> > > Is this limiting the 'reg' property of the hub node and not the child
> > > usb-device nodes?
> >
> > Yes, but the regex pattern of patternProperties restricts the
> > downstream device nodes to '^.*@[1-4]$'. So the 'reg's of the child
> > usb-device nodes are also checked.
>
> I'm confused. The path looks like it is removing patternProperties here
> and limiting the reg property of the hub itself so it can only be at
> port 1, 2, 3 or 4. Why is the patternProperties being removed? Don't we
> need to keep the patternProperties around, or somehow get at the
> patternProperties for the hard wired USB devices in the usb-hub schema
> so we can constrain the reg property to be between 1 and 4?

Sorry, I was confused by my own patch... Yes, the patternProperties is
removed in this patch so please ignore my previous mail.

However, it seems that the addtionalProperties here checks all the
properties that are not defined in this yaml file. I tried changing
the node in the example DT to device@5/reg=<5>, and make
dt_binding_check rejects the node as expected.
>
> > >
> > > >
> > > >  required:
> > > >    - peer-hub
> > > >    - compatible
> > > >    - reg
> > >
> > > Can 'reg' be dropped because usb-hub.yaml requires it?
> >
> > I can drop 'reg' and 'compatible' in the next version, but I see other
> > schemas referencing usb-device.yaml still set 'reg' as required. Is
> > this some kind of convention, or people just didn't notice this?
>
> I assume nobody noticed.

Okay, then I'll update this in the next version.

Pin-yen
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
index 6577a61cc07531..a020afaf2d6e7a 100644
--- a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
+++ b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
@@ -10,7 +10,7 @@  maintainers:
   - Matthias Kaehlcke <mka@chromium.org>
 
 allOf:
-  - $ref: usb-device.yaml#
+  - $ref: usb-hub.yaml#
 
 properties:
   compatible:
@@ -19,61 +19,35 @@  properties:
           - usbbda,5411
           - usbbda,411
 
-  reg: true
-
-  '#address-cells':
-    const: 1
-
-  '#size-cells':
-    const: 0
-
   vdd-supply:
     description:
       phandle to the regulator that provides power to the hub.
 
-  peer-hub:
-    $ref: /schemas/types.yaml#/definitions/phandle
-    description:
-      phandle to the peer hub on the controller.
+  peer-hub: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
-    properties:
-      port@1:
-        $ref: /schemas/graph.yaml#/properties/port
-        description:
-          1st downstream facing USB port
-
-      port@2:
+    patternProperties:
+      '^port@':
         $ref: /schemas/graph.yaml#/properties/port
-        description:
-          2nd downstream facing USB port
 
-      port@3:
-        $ref: /schemas/graph.yaml#/properties/port
-        description:
-          3rd downstream facing USB port
+        properties:
+          reg:
+            minimum: 1
+            maximum: 4
 
-      port@4:
-        $ref: /schemas/graph.yaml#/properties/port
-        description:
-          4th downstream facing USB port
-
-patternProperties:
-  '^.*@[1-4]$':
-    description: The hard wired USB devices
-    type: object
-    $ref: /schemas/usb/usb-device.yaml
-    additionalProperties: true
+additionalProperties:
+  properties:
+    reg:
+      minimum: 1
+      maximum: 4
 
 required:
   - peer-hub
   - compatible
   - reg
 
-additionalProperties: false
-
 examples:
   - |
     usb {