Re: [PATCH 2/2] Drivers: MUSB: Davinci MUSB: added DT support

[Petr Kulhavy <petr@barix.com>]

[-- Attachment #1: Type: text/plain, Size: 3824 bytes --]

Hi Rob,

thanks a lot for your comments. I admit I got inspired by the 
usb/omap-usb.txt  and usb/am33xx-usb.txt bindings
which use identical keywords, mode, num-eps, ram-bits, power and 
multipoint as all these chips use the same or similar MUSB core.
 From your comments I have the feeling these are not the best bindings 
to take as an example.
What would you recommend as a good example then?

Regards
Petr

On 22.01.2016 00:07, Rob Herring wrote:
> On Thu, Jan 21, 2016 at 03:53:19PM +0100, Petr Kulhavy wrote:
>> TI DaVinci MUSB driver equipped with DeviceTree support.
>> Tested with AM1808 board and USB2.0 (OTG) in host mode.
>>
>> Signed-off-by: Petr Kulhavy <petr@barix.com>
>> ---
>>   .../devicetree/bindings/usb/da8xx-usb.txt          |  52 +++++++
>>   drivers/usb/musb/da8xx.c                           | 166 +++++++++++++++++++++
>>   include/linux/platform_data/usb-davinci.h          |   3 +-
>>   3 files changed, 220 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..c81d665
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> Follow the compatible string for the filename (with wildcards is fine).
>
>> @@ -0,0 +1,52 @@
>> +TI DaVinci MUSB
>> +
>> +Required properties:
>> +
>> + - compatible : Should be "ti,da850-musb" or "ti,da830-musb"
>> +
>> + - mode : USB mode. "1" signifies HOST, "2" represents PERIPHERAL,
>> +     "3" represents OTG.
> Surely we have a similar property defined already. Don't create
> something new.
>
>> +
>> + - power : This signifies the maximum current the controller can
>> +     supply in host mode. The unit size is 2mA, the maximum value is 510mA.
> You should model this as a regulator.
>
>> + - num-eps : Specifies the number of endpoints. This is also a
>> +     MUSB configuration-specific setting.
> Just spell out endpoints.
>
>> +
>> + - multipoint : Should be "1" indicating the musb controller supports
>> +     multipoint. This is a MUSB configuration-specific setting.
> What does multipoint mean?
>
>> + - ram-bits : Specifies the ram address size.
> Needs a better description. Then it probably needs a better name too,
> but without a description I can't tell what that would be.
>
>> +
>> +
>> +Optional properties:
>> +
>> + - da8xx,phy20-clkmux-cfg: Integer. Defines the USB 2.0 PHY reference clock source.
>> +     Supported values: "0" for external pin, "1" for internal PLL.
> How about a boolean property instead. Name it based on the less used
> option (external pin I'm guessing).
>
>> + - da8xx,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
> da8xx is not a vendor.
>
> Rob

-- 


-- 
Petr Kulhavy, MSc
System Architect

*BARIX*

petr@barix.com <mailto:petr@barix.com> | Skype: brain.barix

Barix AG, Seefeldstrasse 303 | 8008 Zurich, Switzerland
T +41 43 43322 11 | www.barix.com <http://www.barix.com>

You have received this email because of your relationship Barix AG and 
its affiliated companies. Barix AG and its affiliated companies do not 
sell or exchange email addresses, or any other personal contact 
information provided by you with any third parties. All email 
distributions are managed and controlled by Barix AG and its affiliated 
companies.
Barix AG, Seefeldstr. 303, 8008 Zürich, Switzerland. Company Reg. No: 
CH-020.3.023.869-8, VAT Reg. No: CHE-105.687.663.

[-- Attachment #2: Type: text/html, Size: 5950 bytes --]