linux-kernel - Re: [PATCH v1 1/2] dt/bindings/usb: Add bindings for PIC32 MUSB driver.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57065F7F.6030902@microchip.com>
Date:	Thu, 7 Apr 2016 18:54:15 +0530
From:	Purna Chandra Mandal <purna.mandal@...rochip.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	<linux-kernel@...r.kernel.org>
CC:	Rob Herring <robh+dt@...nel.org>, <linux-usb@...r.kernel.org>,
	Joshua Henderson <digitalpeer@...italpeer.com>,
	<devicetree@...r.kernel.org>, Kumar Gala <galak@...eaurora.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v1 1/2] dt/bindings/usb: Add bindings for PIC32 MUSB
 driver.

On 04/07/2016 06:42 PM, Sergei Shtylyov wrote:

> On 4/7/2016 4:02 PM, Purna Chandra Mandal wrote:
>
>>>> Document devicetree binding for the USB controller
>>>
>>>     Device tree.
>>>
>> ack.
>>
>>>> and USB Phy found on Microchip PIC32 class devices.
>>>
>>>     PHY.
>>>
>> ack.
>>
>>>> Signed-off-by: Purna Chandra Mandal <purna.mandal@...rochip.com>
>>>>
>>>> ---
>>>>
>>>>    .../bindings/usb/microchip,pic32-musb.txt          | 67 ++++++++++++++++++++++
>>>>    1 file changed, 67 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt b/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt
>>>> new file mode 100644
>>>> index 0000000..e1cec9d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/usb/microchip,pic32-musb.txt
>>>> @@ -0,0 +1,67 @@
>>>> +Microchip PIC32 MUSB DRC/OTG controller
>>>> +-------------------------------------------
>>>> +
>>>> +Required properties:
>>>> + - compatible       : should be "microchip,pic32mzda-usb".
>>>> + - reg              : offset and length of "MUSB Core Registers" and
>>>> +                      "USB Clock & Reset Registers".
>>>> + - reg-names        : should be "mc", and "usbcr" in order
>>>> + - clocks           : clock specifier for the musb controller clock
>>>> + - clock-names      : should be "usb_clk"
>>>> + - interrupts       : interrupt number for MUSB Core General interrupt
>>>> +                      and DMA interrupt
>>>> + - interrupt-names  : must be "mc" and "dma" in order.
>>>> + - phys             : phy specifier for the otg phy.
>>>> + - dr_mode          : should be one of "host", "peripheral" or "otg".
>>>> + - mentor,multipoint: Should be "1" indicating the musb controller supports
>>>> +                      multipoint. This is MUSB configuration-specific setting.
>>>> + - mentor,num-eps   : Specifies the number of endpoints. This is also a
>>>> +                      MUSB configuration-specific setting. Should be set to "8".
>>>> + - mentor,ram-bits  : Specifies the ram address size. Should be set to "11".
>>>> + - mentor,power     : Should be "500". This signifies the controller can supply
>>>> +                      up to 500mA when operating in host mode.
>>>
>>>     No, these "nentor" prefixed parameters must be determined from the "compatible" prop.
>>>
>> Prefix "mentor" here is used to signify configuration of the MUSB controller IP, not
>> specifics of the chip or glue logic.
>
>    I know.
>
>> Please suggest if replacing with "microchip" makes it better.
>
>    No, nothing of that sort. These parameters are probably fixed for the said PIC32 implementation? If so, they shouldn't appear as the node props but should instead be hard-coded in the glue layer. Don't look at the OMAP glues, they are a bad example.

ack. Will hard code.

>
>>>> + - phys             : phandle of the USB phy.
>
>    "phys" are reserved for use by the drivers/phy/, while your PHY seems to be controlled by drivers/usb/phy/. Please rename this property to "usb-phy".
>
Will rename.

>>>> +                      interrupt number for over-current detection logic.
>>>> +
>>>> +Optional properties:
>>>> + - microchip,fifo-mode: Specifies layout of internal SRAM for end-point fifos.
>>>> +                        Should be 0 (default) or 1.
>
>    Probably would be better as a boolean prop...

This is software defined configuration of SRAM layout. In future, we might add
more variants depending on use-cases to extract better performance.

Will prefer to keep it as integer property. And update description accordingly;
like "Specifies layout of internal SRAM for end-point fifos. Defaults 0."

>
> [...]
>
> MBR, Sergei
>
Thanks, Purna