[PATCH 3/3] dt-bindings: usb: add documentation for aspeed udc

[Krzysztof Kozlowski]

On 13/05/2022 17:39, Neal Liu wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
>> Sent: Friday, May 13, 2022 5:07 PM
>> To: Neal Liu <neal_liu at aspeedtech.com>; Greg Kroah-Hartman
>> <gregkh at linuxfoundation.org>; Rob Herring <robh+dt at kernel.org>;
>> Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>; Joel Stanley
>> <joel at jms.id.au>; Andrew Jeffery <andrew at aj.id.au>; Felipe Balbi
>> <balbi at kernel.org>; Sumit Semwal <sumit.semwal at linaro.org>; Christian
>> König <christian.koenig at amd.com>; Geert Uytterhoeven <geert at linux-
>> m68k.org>; Li Yang <leoyang.li at nxp.com>
>> Cc: linux-aspeed at lists.ozlabs.org; linux-usb at vger.kernel.org;
>> devicetree at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
>> kernel at vger.kernel.org; linux-media at vger.kernel.org; dri-
>> devel at lists.freedesktop.org; linaro-mm-sig at lists.linaro.org; BMC-SW <BMC-
>> SW at aspeedtech.com>
>> Subject: Re: [PATCH 3/3] dt-bindings: usb: add documentation for aspeed
>> udc
>>
>> On 13/05/2022 08:57, Neal Liu wrote:
>>> Add device tree binding documentation for the Aspeed USB2.0 Device
>>> Controller.
>>>
>>> Signed-off-by: Neal Liu <neal_liu at aspeedtech.com>
>>> ---
>>>  .../devicetree/bindings/usb/aspeed,udc.yaml   | 52
>> +++++++++++++++++++
>>>  1 file changed, 52 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/usb/aspeed,udc.yaml
>>
>> Please name the file as first compatible, so "aspeed,ast2600-udc.yaml"
> 
> Okay, I could rename it for next patch if you preferred.
> But there are lots of yaml files which are not named as first compatible.

Yes, I know, I quite likely I also produced such bindings, but a
specific name is rather preferred. Otherwise you will have a difficult
naming choice when your next Aspeed UDC requires new bindings file
because of some differences (not yet known now).

> 
>>
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/aspeed,udc.yaml
>>> b/Documentation/devicetree/bindings/usb/aspeed,udc.yaml
>>> new file mode 100644
>>> index 000000000000..d1d2f77d1c54
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/aspeed,udc.yaml
>>> @@ -0,0 +1,52 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
>>> +(c) 2020 Facebook Inc.
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/usb/aspeed,udc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: ASPEED USB 2.0 Device Controller
>>> +
>>> +maintainers:
>>> +  - Neal Liu <neal_liu at aspeedtech.com>
>>> +
>>> +description: |+
>>> +  The ASPEED USB 2.0 Device Controller implements 1 control endpoint
>>> +and
>>> +  4 generic endpoints for AST260x.
>>> +
>>> +  Supports independent DMA channel for each generic endpoint.
>>> +  Supports 32/256 stages descriptor mode for all generic endpoints.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - aspeed,ast2600-udc
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>
>> No child properties? No ports or any other devices? No usb-hcd.yaml?
> 
> Aspeed udc only has 1 port, no need extra properties for now.

OK

> 
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - interrupts
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/clock/aspeed-clock.h>
>>> +    udc: udc at 1e6a2000 {
>>
>> Node name: usb
>  
> "udc" is more recognizable than "usb" I think. "usb" is too general, can be various like host or device.

It's still required by schema for most of USB host controllers. Existing
USB device controllers use usb as well (except Atmel mentioning gadget)
Generic name is also expected by Devicetree spec and "udc" is not on a
list of examples of generic names (usb is).


Best regards,
Krzysztof