diff mbox series

[1/2] dt-bindings: usb: Add binding for PS5511 hub controller

Message ID 20250328082950.1473406-1-treapking@chromium.org
State New
Headers show
Series [1/2] dt-bindings: usb: Add binding for PS5511 hub controller | expand

Commit Message

Pin-yen Lin March 28, 2025, 8:28 a.m. UTC
Parade PS5511 is USB hub with 4 USB 3.2 compliant 5Gbps downstream(DS)
ports, and 1 extra USB 2.0 downstream port. The hub has one reset pin
control and two power supplies (3V3 and 1V1).

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

 .../bindings/usb/parade,ps5511.yaml           | 138 ++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/parade,ps5511.yaml

Comments

Krzysztof Kozlowski March 31, 2025, 8:09 a.m. UTC | #1
On Fri, Mar 28, 2025 at 04:28:45PM +0800, Pin-yen Lin wrote:
> +
> +title: PS5511 4+1 Port USB 3.2 Gen 1 Hub Controller

s/PS5511/Parade PS5511/

> +
> +maintainers:
> +  - Pin-yen Lin <treapking@chromium.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - usb1da0,5511
> +      - usb1da0,55a1
> +
> +  reg: true
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  reset-gpios:
> +    items:
> +      - description: GPIO specifier for GRST# pin.
> +
> +  vddd11-supply:
> +    description:
> +      1V1 power supply to the hub
> +
> +  vdd33-supply:
> +    description:
> +      3V3 power supply to the hub
> +
> +  peer-hub:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to the peer hub on the controller.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports

I don't understand why do you need OF graph here. Children are already
defined in patternProperties as usb-devices.

Where is any upstream DTS using this schema?

> +
> +    properties:
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          1st downstream facing USB port
> +
> +      port@2:
> +        $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
> +
> +      port@4:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          4th downstream facing USB port
> +
> +      port@5:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          5th downstream facing USB port
> +
> +required:
> +  - compatible
> +  - reg
> +  - peer-hub
> +
> +patternProperties:

patternProperties follow immediately properties, so move it up.

> +  '^.*@[1-5]$':
> +    description: The hard wired USB devices
> +    type: object
> +    $ref: /schemas/usb/usb-device.yaml
> +    additionalProperties: true
> +
> +additionalProperties: false
> +
> +allOf:
> +  - $ref: usb-device.yaml#
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            enum:
> +              - usb1da0,usb55a1
> +    then:
> +      properties:
> +        port@5: false

No such property.

Best regards,
Krzysztof
Pin-yen Lin April 8, 2025, 4:30 a.m. UTC | #2
Hi Krzysztof,

Thanks for the review.

On Mon, Mar 31, 2025 at 4:09 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, Mar 28, 2025 at 04:28:45PM +0800, Pin-yen Lin wrote:
> > +
> > +title: PS5511 4+1 Port USB 3.2 Gen 1 Hub Controller
>
> s/PS5511/Parade PS5511/

I'll fix this in the next version.
>
> > +
> > +maintainers:
> > +  - Pin-yen Lin <treapking@chromium.org>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - usb1da0,5511
> > +      - usb1da0,55a1
> > +
> > +  reg: true
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +  reset-gpios:
> > +    items:
> > +      - description: GPIO specifier for GRST# pin.
> > +
> > +  vddd11-supply:
> > +    description:
> > +      1V1 power supply to the hub
> > +
> > +  vdd33-supply:
> > +    description:
> > +      3V3 power supply to the hub
> > +
> > +  peer-hub:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      phandle to the peer hub on the controller.
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
>
> I don't understand why do you need OF graph here. Children are already
> defined in patternProperties as usb-devices.

I would like to describe the connection between the hub and the USB
connectors, which is similar to commit c44d9dab31d6a9 ("dt-bindings:
usb: Add downstream facing ports to realtek binding").

I'm aware that Rob expects a usb-hub.yaml binding in [1]. Should I do that now?

[1]: https://lore.kernel.org/all/20240301175653.GA2469610-robh@kernel.org/
>
> Where is any upstream DTS using this schema?

The device using this schema is still under development. We plan to
upstream this schema first and then upstream the DT after the device
is ready.
>
> > +
> > +    properties:
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          1st downstream facing USB port
> > +
> > +      port@2:
> > +        $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
> > +
> > +      port@4:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          4th downstream facing USB port
> > +
> > +      port@5:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description:
> > +          5th downstream facing USB port
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - peer-hub
> > +
> > +patternProperties:
>
> patternProperties follow immediately properties, so move it up.

Will fix this in the next version.
>
> > +  '^.*@[1-5]$':
> > +    description: The hard wired USB devices
> > +    type: object
> > +    $ref: /schemas/usb/usb-device.yaml
> > +    additionalProperties: true
> > +
> > +additionalProperties: false
> > +
> > +allOf:
> > +  - $ref: usb-device.yaml#
> > +  - if:
> > +      not:
> > +        properties:
> > +          compatible:
> > +            enum:
> > +              - usb1da0,usb55a1
> > +    then:
> > +      properties:
> > +        port@5: false
>
> No such property.

Actually, I meant:

  - if:
      not:
        properties:
          compatible:
            enum:
              - usb1da0,usb55a1
    then:
      properties:
        ports:
          properties:
            port@5: false

I'll fix this in the next version.

>
> Best regards,
> Krzysztof
>

Best regards,
Pin-yen
Krzysztof Kozlowski April 9, 2025, 6:06 a.m. UTC | #3
On 08/04/2025 06:30, Pin-yen Lin wrote:
>>> +  peer-hub:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      phandle to the peer hub on the controller.
>>> +
>>> +  ports:
>>> +    $ref: /schemas/graph.yaml#/properties/ports
>>
>> I don't understand why do you need OF graph here. Children are already
>> defined in patternProperties as usb-devices.
> 
> I would like to describe the connection between the hub and the USB
> connectors, which is similar to commit c44d9dab31d6a9 ("dt-bindings:
> usb: Add downstream facing ports to realtek binding").
> 
> I'm aware that Rob expects a usb-hub.yaml binding in [1]. Should I do that now?
Yes, please, two devices using common pattern is already enough to
create common schema.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/parade,ps5511.yaml b/Documentation/devicetree/bindings/usb/parade,ps5511.yaml
new file mode 100644
index 00000000000000..605f94df561428
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/parade,ps5511.yaml
@@ -0,0 +1,138 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/parade,ps5511.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PS5511 4+1 Port USB 3.2 Gen 1 Hub Controller
+
+maintainers:
+  - Pin-yen Lin <treapking@chromium.org>
+
+properties:
+  compatible:
+    enum:
+      - usb1da0,5511
+      - usb1da0,55a1
+
+  reg: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  reset-gpios:
+    items:
+      - description: GPIO specifier for GRST# pin.
+
+  vddd11-supply:
+    description:
+      1V1 power supply to the hub
+
+  vdd33-supply:
+    description:
+      3V3 power supply to the hub
+
+  peer-hub:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to the peer hub on the controller.
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          1st downstream facing USB port
+
+      port@2:
+        $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
+
+      port@4:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          4th downstream facing USB port
+
+      port@5:
+        $ref: /schemas/graph.yaml#/properties/port
+        description:
+          5th downstream facing USB port
+
+required:
+  - compatible
+  - reg
+  - peer-hub
+
+patternProperties:
+  '^.*@[1-5]$':
+    description: The hard wired USB devices
+    type: object
+    $ref: /schemas/usb/usb-device.yaml
+    additionalProperties: true
+
+additionalProperties: false
+
+allOf:
+  - $ref: usb-device.yaml#
+  - if:
+      not:
+        properties:
+          compatible:
+            enum:
+              - usb1da0,usb55a1
+    then:
+      properties:
+        port@5: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    usb {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        /* 2.0 hub on port 1 */
+        hub_2_0: hub@1 {
+            compatible = "usb1da0,55a1";
+            reg = <1>;
+            peer-hub = <&hub_3_0>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            /* USB 2.0 device on port 5 */
+            device@5 {
+                reg = <5>;
+                compatible = "usb123,4567";
+            };
+        };
+
+        /* 3.0 hub on port 2 */
+        hub_3_0: hub@2 {
+            compatible = "usb1da0,5511";
+            reg = <2>;
+            peer-hub = <&hub_2_0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                /* Type-A connector on port 3 */
+                port@3 {
+                    reg = <3>;
+                    endpoint {
+                        remote-endpoint = <&usb_a0_ss>;
+                    };
+                };
+            };
+        };
+    };