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

[Krzysztof Kozlowski]

On 08/03/2026 10:23, Krzysztof Kozlowski wrote:
> 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>

Also:

A nit, subject: drop second/last, redundant "binding support". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

And you cannot add support for bindings. The DT schema or some kernel
Makefile gave that support, not this file.


>> ---
>>  .../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.

I guess Rob was fine with this, so let's keep it in this file.

Best regards,
Krzysztof