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

[Pin-yen Lin]

Hi Krzysztof,

Thanks for the review.

On Mon, Mar 31, 2025 at 4:09 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > +
> > +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@xxxxxxxxxx/
>
> 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