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

[Charan Pedumuru]

On 08-03-2026 14:58, Krzysztof Kozlowski wrote:
> 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.

Okay, I will change that.

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

Yes, it should be defined here as the fallback compatible is already defined in this existing YAML.

> 
> Best regards,
> Krzysztof

-- 
Best Regards,
Charan.