Re: [PATCH v2] dt-bindings: usb: ti,keystone-dwc3.yaml: Improve schema

[Rob Herring <robh@kernel.org>]

On Fri, Jun 05, 2020 at 10:52:15AM +0300, Roger Quadros wrote:
> There were some review comments after the patch was integrated.
> Address those.
> 
> Fixes: 1883a934e156 ("dt-bindings: usb: convert keystone-usb.txt to YAML")
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> 
> Changelog:
> v2
> - don't use quotes for enum/const string
> - use phandle instead of phandle-array for phys
> - add maxItems for phy-names
> 
>  .../bindings/usb/ti,keystone-dwc3.yaml        | 50 ++++++++++++++-----
>  1 file changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> index f127535feb0b..394e47d2f5d7 100644
> --- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml
> @@ -11,64 +11,88 @@ maintainers:
>  properties:
>    compatible:
> -    oneOf:
> -      - const: "ti,keystone-dwc3"
> -      - const: "ti,am654-dwc3"
> +    items:
> +      - enum:
> +        - ti,keystone-dwc3
> +        - ti,am654-dwc3
>    reg:
>      maxItems: 1
> -    description: Address and length of the register set for the USB subsystem on
> -      the SOC.
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true

blank line

>    interrupts:
>      maxItems: 1
> -    description: The irq number of this device that is used to interrupt the MPU.
> -
>    clocks:
> +    $ref: /schemas/types.yaml#definitions/phandle-array

Common property. It already has a type and doesn't need to be 
redefined here.

Just "maxItems: 1" if it is a single clock is enough, or an 'items' list 
for each entry if more than 1.

>      description: Clock ID for USB functional clock.

Drop.

> +  assigned-clocks:
> +    $ref: /schemas/types.yaml#definitions/phandle-array


> +
> +  assigned-clock-parents:
> +    $ref: /schemas/types.yaml#definitions/phandle-array
> +
>    power-domains:
> +    $ref: /schemas/types.yaml#definitions/phandle-array

Same as 'clocks'.

>      description: Should contain a phandle to a PM domain provider node
>        and an args specifier containing the USB device id
>        value. This property is as per the binding,
>        Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>    phys:
> +    $ref: /schemas/types.yaml#/definitions/phandle

Same as 'clocks'.

>      description:
>        PHY specifier for the USB3.0 PHY. Some SoCs need the USB3.0 PHY
>        to be turned on before the controller.
>        Documentation/devicetree/bindings/phy/phy-bindings.txt
>    phy-names:
> +    maxItems: 1
>      items:
> -      - const: "usb3-phy"
> +      - const: usb3-phy

Don't need maxItems as that's implied by the length of 'items'.

> +
> +  dma-coherent: true
> -  dwc3:
> +  dma-ranges: true
> +
> +patternProperties:
> +  "usb@[a-f0-9]+$":
> +    type: object
>      description: This is the node representing the DWC3 controller instance
>        Documentation/devicetree/bindings/usb/dwc3.txt
>  required:
>    - compatible
>    - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
>    - interrupts
> -  - clocks
> +
> +additionalProperties: false
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> -    usb: usb@2680000 {
> +    dwc3@2680000 {
>        compatible = "ti,keystone-dwc3";
>        #address-cells = <1>;
>        #size-cells = <1>;
>        reg = <0x2680000 0x10000>;
>        clocks = <&clkusb>;
> -      clock-names = "usb";
>        interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
>        ranges;
> -      dwc3@2690000 {
> +      usb@2690000 {
>          compatible = "synopsys,dwc3";
>          reg = <0x2690000 0x70000>;
>          interrupts = <GIC_SPI 393 IRQ_TYPE_EDGE_RISING>;
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
>