Re: [PATCH v2] dt-bindings: usb: usb251xb: Convert to YAML schema

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 03/11/2022 21:37, Marek Vasut wrote:
> Convert the usb251xb hub DT bindings from text to yaml schema so it is
> possible to validate DTs against the schema.
> 
> Adjust the example to describe two different hubs at different I2C bus
> addresses, to avoid I2C address collission in the example.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>

Thank you for your patch. There is something to discuss/improve.

> +  dynamic-power-switching:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      enable auto-switching from self- to bus-powered operation if the
> +      local power source is removed or unavailable (boolean)
> +
> +  oc-delay-us:
> +    enum: [100, 4000, 8000, 16000]
> +    description: |
> +      Delay time (in microseconds) for filtering the over-current sense
> +      inputs. Valid values are 100, 4000, 8000 (default) and 16000. If

Drop this sentence and instead add:

    default: 8000

> +      an invalid value is given, the default is used instead.


> +
> +  compound-device:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      indicate the hub is part of a compound device (boolean)
> +
> +  port-mapping-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      enable port mapping mode (boolean)
> +
> +  led-usb-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      led usb/speed indication mode selection (boolean, default is speed mode)
> +
> +  led-speed-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      led usb/speed indication mode selection (boolean, default is speed mode)
> +
> +  string-support:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      enable string descriptor support (required for manufacturer, product
> +      and serial string configuration)
> +
> +  non-removable-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description: |
> +      Should specify the ports which have a non-removable device connected.
> +
> +  sp-disabled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description: |
> +      Specifies the ports which will be self-power disabled
> +
> +  bp-disabled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description: |
> +      Specifies the ports which will be bus-power disabled
> +
> +  sp-max-total-current-microamp:
> +    maximum: 100000
> +    description: |
> +      Specifies max current consumed by the hub from VBUS when
> +      operating in self-powered hub. It includes the hub silicon
> +      along with all associated circuitry including a permanently
> +      attached peripheral (range: 0 - 100000 uA, default 1000 uA)

Drop the range description.

default: 1000

and similarly in other places

> +
> +  bp-max-total-current-microamp:
> +    maximum: 510000
> +    description: |
> +      Specifies max current consumed by the hub from VBUS when
> +      operating in self-powered hub. It includes the hub silicon
> +      along with all associated circuitry including a permanently
> +      attached peripheral (range: 0 - 510000 uA, default 100000 uA)
> +
> +  sp-max-removable-current-microamp:
> +    maximum: 100000
> +    description: |
> +      Specifies max current consumed by the hub from VBUS when
> +      operating in self-powered hub. It includes the hub silicon
> +      along with all associated circuitry excluding a permanently
> +      attached peripheral (range: 0 - 100000 uA, default 1000 uA)
> +
> +  bp-max-removable-current-microamp:
> +    maximum: 510000
> +    description: |
> +      Specifies max current consumed by the hub from VBUS when
> +      operating in self-powered hub. It includes the hub silicon
> +      along with all associated circuitry excluding a permanently
> +      attached peripheral (range: 0 - 510000 uA, default 100000 uA)
> +
> +  power-on-time-ms:
> +    maximum: 510
> +    description: |
> +      Specifies the time it takes from the time the host initiates the
> +      power-on sequence to a port until the port has adequate power.
> +      The value is given in ms in a 0 - 510 range (default is 100ms).
> +
> +  swap-dx-lanes:
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    description: |
> +      Specifies the ports which will swap the differential-pair (D+/D-),
> +      default is not-swapped.
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      usb2512b@2c {

Generic node names, so "usb-hub"

> +        compatible = "microchip,usb2512b";
> +        reg = <0x2c>;
> +        reset-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
> +      };
> +
> +      usb2514b@2d {

usb-hub

> +        compatible = "microchip,usb2514b";
> +        reg = <0x2d>;
> +        vendor-id = /bits/ 16 <0x0000>;
> +        product-id = /bits/ 16 <0x0000>;
> +        string-support;
> +        manufacturer = "Foo";
> +        product = "Foo-Bar";
> +        serial = "1234567890A";
> +        /* correct misplaced usb connectors on port 1,2 */
> +        swap-dx-lanes = <1 2>;
> +      };
> +    };

Best regards,
Krzysztof