linux-kernel - Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

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]

Date:	Wed, 29 Jun 2016 15:56:23 -0300
From:	Bruno Herrera <bruherrera@...il.com>
To:	Rob Herring <robh@...nel.org>
Cc:	pawel.moll@....com, mark.rutland@....com,
	ijc+devicetree@...lion.org.uk, Kumar Gala <galak@...eaurora.org>,
	linux@...linux.org.uk, Maxime Coquelin <mcoquelin.stm32@...il.com>,
	johnyoun@...opsys.com, gregkh@...uxfoundation.org,
	Felipe Balbi <balbi@...nel.org>,
	Zhangfei Gao <zhangfei.gao@...aro.org>,
	Antti Seppälä <a.seppala@...il.com>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-usb@...r.kernel.org,
	alexandre.torgue@...com
Subject: Re: [PATCH 3/3] dt-bindings: Document the STM32 USB OTG DWC2 core binding

On Tue, Jun 28, 2016 at 5:54 PM, Rob Herring <robh@...nel.org> wrote:
> On Fri, Jun 24, 2016 at 03:51:18PM -0300, Bruno Herrera wrote:
>> On Fri, Jun 24, 2016 at 12:41 PM, Rob Herring <robh@...nel.org> wrote:
>> > On Tue, Jun 21, 2016 at 11:25:49PM -0300, Bruno Herrera wrote:
>> >> Signed-off-by: Bruno Herrera <bruherrera@...il.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> index 20a68bf..79e5370 100644
>> >> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>> >> @@ -11,6 +11,7 @@ Required properties:
>> >>    - "lantiq,arx100-usb": The DWC2 USB controller instance in Lantiq ARX SoCs;
>> >>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>> >>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>> >> +  - st,stm32-fsotg: The DWC2 USB controller instance in STM32F4 SoCs in FS mode;
>> >
>> > This should go above snps,dwc2.
>> >
>> Ok, tks!
>>
>> > What determines FS mode vs. HS?
>> >
>> Its more HW design decision.
>> STM32F429/439/469 has two OTG controllers, one that is FS (internal
>> phy) and other that is HS (but can also work in FS mode with
>> internal/external phy)
>> This bind work with both cores FS and HS working with the internal PHY.
>>
>> I tested the following configurations:
>> 1 - STM32F429I-DISCOv1 board (OTG HS working in FS mode internal PHY)
>> 2 - STM32F469I-DISCO board (OTG FS)
>>
>> I did not tested OTG HS core working in FS mode with external PHY (I2C).
>
> You shouldn't be setting the compatible string based on which mode you
> want. So for the HS block, you need a different compatible string than
> the FS block and set the speed in another way (not sure if we have a
> standard way). Or perhaps the phy should determine the speed.

I understand but I dont see how to fix it properly unless we add a
some specific STM32 properties in the DT or in the dwc2_core_params.
In fact there are two cores, and they are different in terms of
functionality (despite of the type of the PHY).
One core is for FS and other is for HS, so they could/should have
different compatible strings because they have different
configurations and are different piece of IP/Hardware(The buffer size
are different, the number of end points and so one, DMA)

But the problem is that the HS core can also support the the FS mode
and in this case is misleading to have a HS core with an FS compatible
string.
Something like that (real case for STM32F429I-DISCO):

&usbotg_hs {
compatible = "st,stm32-fsotg", "snps,dwc2";
dr_mode = "host";
pinctrl-0 = <&usbotg_fs_pins_b>;
pinctrl-names = "default";
status = "okay";
};

Even if the decision is phy based it would lead to a STM32 specific
logic and we would need to figure out how to represent the internal
PHY.

Bruno
>
> Rob