Re: [PATCH 1/1] Drivers: USB: DA8xx MUSB: added DT support

[Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>]

On 02/08/2016 10:49 PM, Rob Herring wrote:

>> TI DA8xx MUSB driver equipped with DeviceTree support.
>> Tested with AM1808 board and USB2.0 (OTG) in host mode.
>>
>> Signed-off-by: Petr Kulhavy <petr-Qh/3xLP0EvwAvxtiuMwx3w@public.gmane.org>
>> ---
>>   .../devicetree/bindings/usb/da8xx-usb.txt          |  63 ++++++++
>>   drivers/usb/musb/da8xx.c                           | 162 +++++++++++++++++++++
>>   include/linux/platform_data/usb-davinci.h          |   3 +-
>>   3 files changed, 227 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> new file mode 100644
>> index 0000000..69c0961
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> @@ -0,0 +1,63 @@
>> +TI DA8xx MUSB
>> +~~~~~~~~~~~~~
>> +
>> +Required properties:
>> +~~~~~~~~~~~~~~~~~~~~
>> + - compatible : Should be "ti,da8xx-musb"
>> +
>> + - reg: offset and length of the usbss register sets
>> +
>> + - interrupts: USB interrupt number
>> +
>> + - interrupt-names: should be set to "mc"
>
> Kind of pointless with only one.

    Required by the driver.

>> + - dr_mode: The USB operation mode. Should be one of "host", "peripheral" or "otg".
>> +
>> + - mentor,power : Specifies the maximum current in milli-ampers the controller can
>> +     supply in host mode. The maximum configurable value is 510mA.
>> +
>> + - mentor,num-eps : Valid only in host mode. Specifies the number of target endpoints
>> +     supported by the controller. For DA8xx it is "5".
>
> If DA8xx is the only thing you support, then why is this needed?

    We already agreed that it should be deduced from the "compatible" prop. 
Unfortunately, the author had bad examples in omap2430.c and musb_dsps.c... :-(

>> + - mentor,multipoint : Valid only in host mode. Enables addressing of USB hubs,
>> +     which is normally something you want and therefore should be set to "1".
>> +     If set to "0" only point-to-point communication is enabled, i.e. only a single
>> +     device can be attached.

    This is author's fantasies, not the real meaning. :-)

>
> Given 1 is a superset of 0, why ever use 0?

    Already this way in the exsiting MUSB bindings. Actually 0/1 indicates 
that the target endpoint control regs are absent/present.

>> + - mentor,ram-bits : This 2-base logarithm value defines the RAM FIFO size of the controller.
>> +     The FIFO size is calculated in 32-bit words. E.g. if your controller has 4kiB of RAM FIFO
>> +     this value should be set to "10": 2^10 = 1024 words of 32-bits, i.e. 4096 bytes.
>> +     For the DA8xx controller this value should be set to 10.
>
> Again, if fixed for the compatible string, then don't put into the DT.

    Fully agreed. Bad examples exist, though...

>> +
>> +
>> +Optional properties:
>> +
>> + - ti,phy20-clkmux-cfg: Integer. Defines the USB 2.0 PHY reference clock source.
>> +     Supported values: "0" for external pin, "1" for internal PLL.
>> +
>> + - ti,phy20-refclock-frequency : Integer. Defines the USB 2.0 PHY reference clock input
>> +     frequency in Hz in case the clock is generated by the internal PLL.
>> +     Supported values are 12MHz, 13MHz, 19.2MHz, 20MHz, 24MHz, 26MHz, 38.4MHz, 40MHz, 48MHz
>
> Add units suffix (-hz).

    Doesn't "frequency" alteady tell that?

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html