Hi Rob,
>On Mon, Dec 10, 2018 at 12:39:14PM +0000, Pawel Laszczak wrote:
>> This patch aim at documenting USB related dt-bindings for the
>> Cadence USBSS-DRD controller.
>>
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>> ---
>> .../devicetree/bindings/usb/cdns3-usb.txt | 31 +++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/cdns3-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/cdns3-usb.txt b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>> new file mode 100644
>> index 000000000000..ae4a255f0b10
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/cdns3-usb.txt
>
>cdns-usb3.txt?
In all functions we use prefix cdns3, so it was a reason for cdns3-usb.txt.
I agree with you and I think that cdns-usb3.txt in this place will be better.
3 in cdns3 could be associated with company name, but that's not true.
>
>> @@ -0,0 +1,31 @@
>> +Binding for the Cadence USBSS-DRD controller
>> +
>> +Required properties:
>> + - reg: Physical base address and size of the controller's register areas.
>> + Controller has 3 different regions:
>> + region 1 - HOST registers area
>> + region 2 - DEVICE registers area
>> + region 3 - OTG/DRD registers area
>> + - compatible: Should contain: "cdns,usb3"
>
>Only 1 version of the IP block?
I will add one more. Now I know that we should have
at least one additional version.
Controller version will be recognized at runtime, but in my opinion
we should also have information about this in dt-binding documentation.
>
>> + - interrupts: Interrupt specifier. Refer to interrupt bindings.
>> + Driver supports only single interrupt line.
>> + This single interrupt is shared between Device,
>> + host and OTG/DRD part of driver.
>> +
>> +Optional propertiesi:
>
>typo
Thank, I know that already. Someone has already reported me this typo.
"i" - insert mode in vim :)
>
>> + - maximum-speed : valid arguments are "super-speed", "high-speed" and
>> + "full-speed"; refer to usb/generic.txt
>> + - dr_mode: Should be one of "host", "peripheral" or "otg".
>> + - phys: reference to the USB PHY
>> + - phy-names: name of the USB PHY, should be "cdns3,usbphy"
>
>Don't need -names when there is only one.
But in the future probably we will need to add next phy version.
So maybe it's better to leave this name ?
>
>> +
>> +
>> +Example:
>> + usb@f3000000 {
>> + compatible = "cdns,usb3";
>> + interrupts = <USB_IRQ 7 IRQ_TYPE_LEVEL_HIGH>;
>> + reg = <0xf3000000 0x10000 //memory area for HOST registers
>> + 0xf3010000 0x10000 //memory area for DEVICE registers
>> + 0xf3020000 0x10000>; //memory area for OTG/DRD registers
>> + };
>> +
>> --
>> 2.17.1
>>
Thanks for comments
Cheers,
Pawel
