Hi, On Tue, Jul 18, 2017 at 12:33 PM, Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> wrote: > On Tue, 2017-07-18 at 10:19 +0200, Martin Blumenstingl wrote: >> Hi, >> >> On Tue, Jul 18, 2017 at 3:40 AM, Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> wrote: >> > Hi, >> > On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote: >> >> Hi, >> >> >> >> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> wrote: >> >> > Hi, >> >> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote: >> >> >> Hi, >> >> >> >> >> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun >> >> >> <chunfeng.yun@xxxxxxxxxxxx> wrote: >> >> >> > Hi Martin, >> >> >> > >> >> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote: >> >> >> >> This series is the outcome of a discussion with Felipe Balbi, >> >> >> >> see [0] and [1]. >> >> >> >> The quick-summary of this is: >> >> >> >> - dwc3 already takes one USB2 and one USB3 PHY and initializes these >> >> >> >> correct >> >> >> >> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and >> >> >> >> ohci-platform.c) do not have a limitation on the number of PHYs - they >> >> >> >> support one PHY per actual host port >> >> >> >> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two >> >> >> >> or three USB2 ports enabled on the internal root-hub. The SoCs also >> >> >> >> provide separate USB2 PHYs, one per port. All USB2 PHYs (which are >> >> >> >> internally "connected" to the dwc3 roothub) need to be powered on, >> >> >> >> otherwise USB devices cannot be enumerated (even if just one PHY is >> >> >> >> disabled and if the device is plugged into another, enabled port) >> >> >> >> >> >> >> >> In my first attempt to get USB supported on the GXL and GXM SoCs I tried >> >> >> >> to work-around the problem that I could not pass multiple PHYs to the >> >> >> >> dwc3 controller. >> >> >> >> This was rejected by Rob Herring (which was definitely the thing to do in >> >> >> >> my opinion), see [2] >> >> >> >> >> >> >> >> This series adds a new "platform-roothub". This can be configured through >> >> >> >> devicetree by passing a child-node with "reg = <0>" to the USB >> >> >> >> controller. Additionally there has to be a child-node for each port on >> >> >> >> the root-hub. Each of the child-nodes takes a "phys" and "phy-names" >> >> >> >> property. This allows modeling the root-hub in devicetree similar to the >> >> >> >> USB device binding (documented in devicetree/bindings/usb/usb-device.txt) >> >> >> >> This avoids and backwards-compatibility problems (which was a concern >> >> >> >> regardless of the solution, see [3]) since the binding for the root-hub >> >> >> >> was previously not specified (and we're not using the "phys" property of >> >> >> >> the controller, which might have served different purposes before, >> >> >> >> depending on the drivers). >> >> >> >> >> >> >> >> Additionally this integrates the new platform-roothub into xhci-plat.c >> >> >> >> which automatically enables it for the dwc3 driver (in host-mode). >> >> >> >> >> >> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support >> >> >> > dual-role mode? leave them to peripheral side as felipe suggested >> >> >> > before? If so, no port1 node for roothub, is there any problem when >> >> >> > change the port1 to host-only mode? >> >> >> on Amlogic Meson GXL we have the following IP blocks: >> >> >> - 2x USB2 PHYs, some external component has to tell them which mode >> >> >> (host/device) they should operate in >> >> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic >> >> >> >> >> >> on Amlogic Meson GXL it could work like this: >> >> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the >> >> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then >> >> >> tell the USB2 PHY in which mode it should operate. >> >> >> >> >> >> please note that device mode and OTG support on Amlogic Meson GXL is >> >> >> more complicated, as it uses dwc2 and dwc3 controllers in combination: >> >> >> - dwc3 is reponsible for host-only mode >> >> >> - dwc2 is responsible for device-only mode >> >> >> - OTG detection is done by the USB3 PHY >> >> >> >> >> >> would you mind sharing a short overview of host/device/OTG support >> >> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL >> >> >> implementation is quite special, so comparing this with another >> >> >> implementation (for example the Mediatek one) may help spotting >> >> >> potential issues. >> >> >> >> >> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work >> >> > as following: >> >> thank you for sharing this! >> >> >> >> > 1. device mode works as HS only: >> >> > >> >> > u2phy0 --- dual-role/OTG >> >> > >> >> > u2phy1 ---| >> >> > + U3 host-only >> >> > u3phy0 ---| >> >> > >> >> > ... >> >> > u2phy4 ---| >> >> > + U3 host-only >> >> > u3phy3 ---| >> >> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as >> >> > dual-role mode, u2phy1 & u3phy0 are host-only) >> >> > >> >> > 2. device mode works as HS & SS, or host only: >> >> > >> >> > u2phy0 ---| >> >> > + dual-role or host-only >> >> > u3phy0 ---| >> >> > >> >> > ... >> >> > u2phy3 ---| >> >> > + U3 host-only >> >> > u3phy3 ---| >> >> > >> >> > u2phy4 --- U2 host-only >> >> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode) >> >> OK, so in both cases only one port (with one u2phy and one u3phy) is >> >> dual-role capable >> > Yes >> >> >> >> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG >> >> > pin), and will take all phys it needed, include host-only phys; >> >> > But if just host-only mode is supported, we can skip mtu3 driver and >> >> > make use of xhci-mtk driver directly, then xhci-mtk will take all phys. >> >> I see, in your example it's the mtu3 >> >> (Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the >> >> mode switching. I assume that you're doing the host/device mode >> >> switching through the extcon phandle (for example together with the >> >> extcon-usb-gpio driver). >> >> >> >> in you example: can't we *always* describe the roothub via devicetree >> >> (just like in my example: [0])? >> >> this means that (as you already mentioned) USB host-only support is now covered. >> >> to handle dual-role (host/device switching) we now need to pass the >> >> dual-role capable PHYs to whatever IP can detect the mode it should >> >> operate in (in your case: mtu3, in Amlogic's case: the u3phy with >> >> built-in mode detection logic -> the driver for this IP block should >> >> call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly). >> >> here's a skeleton (stripped-down) of how the .dts could look like: >> >> mtu3: usb@11271000 { >> >> compatible = "mediatek,mt8173-mtu3"; >> >> ... >> >> /* MT2701 = 2nd example, for MT8173 = 1st example we would skip >> >> the u3phy0 */ >> >> /* only list the dual role capable PHYs here */ >> >> phys = <&u3phy0>, <&u2phy0>; >> >> phy-names = "usb3-phy", "usb2-phy"; >> >> >> >> usb_host: xhci@11270000 { >> >> compatible = "mediatek,mt8173-xhci"; >> >> ... >> >> roothub@0 { >> >> /* includes all PHYs, including the dual role capable ones */ >> > If here include dual-role capable phys which are also taken by parent >> > node(11271000), will cause phy_init/_power_on them twice. >> the PHY framework does ref-counting for all calls [0], so >> phy_ops.init/power_on will only be called for the first time >> phy_{init,power_on} is called >> also phy_ops.exit/power_off is only called when the last consumer >> calls phy_{exit,power_off} > Sorry, I forgot it >> >> > it seems ok if here include host-only phys, but I need test it. >> > If here include host-only phys, there will not be port1 for roothub >> > node, maybe it's a problem. >> on Amlogic's GXL SoC it also makes sense to pass the OTG capable USB2 >> PHY to the roothub (remember: the USB3 PHY does mode-detection for the >> USB2 PHY) >> this way host-only mode works even if the USB3 PHY driver is not >> loaded (or the kernel module is unloaded). and it gets even worse: if >> one of the USB2 PHYs is powered off then host-only mode is broken for >> the other PHYs as well! > It's better if all phys are independent >> >> additionally the device-tree should describe the hardware - and the if >> the dual-role capable PHYs are controlled by two different IP blocks: >> why not add them to both (and let the PHY framework or the IP block - >> like the Mediatek USB PHY IP does - handle this) > Ok, I will test these patches. it would be awesome if you could test my patches and report if they work fine for the Mediatek SoCs as well! > thanks a lot thank you again for sharing details about your SoC and starting a discussion about my implementation >> >> >> }; >> >> }; >> >> }; >> >> >> >> do you think that this would work for the Mediatek SoCs? >> >> I've seen that the phy-mt65xx-usb3.c PHY driver does not have any >> >> .set_mode callback - I assume it's simply because it doesn't need it >> >> (as this is either managed by the hardware/IP block internally, or >> >> through some firmware/mailbox mechanism). >> > Hardware can automatically switch host/device mode by IDDIG pin, but >> > also we can manually switch them. Phy driver makes use of auto way. >> OK, that makes it easy for the driver developers :) >> >> >> >> >> >> >> Regards, >> >> Mart >> >> >> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html >> > >> > >> >> Regards, >> Martin >> >> [0] http://elixir.free-electrons.com/linux/latest/source/drivers/phy/phy-core.c#L218 > > Regards, Martin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html