Re: [PATCH v3 2/5] dt-bindings: usb: generic-ohci: add AT91RM9200 OHCI binding support

[Krzysztof Kozlowski]

On Sat, Mar 07, 2026 at 09:16:19AM +0000, Charan Pedumuru wrote:
> Add binding support for the Atmel AT91RM9200 OHCI USB host controller
> to the generic OHCI schema.
> 
> Signed-off-by: Charan Pedumuru <charan.pedumuru@xxxxxxxxx>
> ---
>  .../devicetree/bindings/usb/generic-ohci.yaml      | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> index 961cbf85eeb5..a8a94b9c1fee 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> @@ -55,6 +55,7 @@ properties:
>            - ti,ohci-omap3
>        - items:
>            - enum:
> +              - atmel,at91rm9200-ohci
>                - cavium,octeon-6335-ohci
>                - nintendo,hollywood-usb-ohci
>                - nxp,ohci-nxp
> @@ -137,6 +138,16 @@ properties:
>        The associated ISP1301 device. Necessary for the UDC controller for
>        connecting to the USB physical layer.
>  
> +  atmel,vbus-gpio:

gpio is deprecated. All bindings use gpios. Also, pins do not use vendor
prefixes.


> +    description: GPIO used to control or sense the USB VBUS power.
> +    minItems: 1
> +    maxItems: 3

Why is this flexible? There is only one VBUS, no? Which pin is it
exactly on this device?

> +
> +  atmel,oc-gpio:
> +    description: GPIO used to signal USB overcurrent condition.
> +    minItems: 1
> +    maxItems: 3

Same question here - how is the pin called in the schematics?

> +
>  required:
>    - compatible
>    - reg
> @@ -144,6 +155,28 @@ required:
>  
>  allOf:
>    - $ref: usb-hcd.yaml
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: atmel,at91rm9200-ohci
> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: ohci_clk
> +            - const: hclk
> +            - const: uhpck
> +
> +      required:
> +        - clocks
> +        - clock-names

There is already if:then:else covering clocks, so this makes multiple
clauses being applied to same device. That's not really readable.
Unfortunately that's a bit of a mess from existing binding. This can be
solved by moving this to separate schema, especially that you want to
add some specific properties to this device.

> +
> +    else:
> +      properties:
> +        atmel,vbus-gpio: false
> +        atmel,oc-gpio: false
> +
>    - if:
>        not:
>          properties:
> 
> -- 
> 2.53.0
>