lkml.org 
[lkml]   [2018]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: Multiple generic PHY instances for DWC3 USB IP
Date

Hi,

Masahiro Yamada <yamada.masahiro@socionext.com> writes:
> Currently, DWC3 core IP (drivers/usb/dwc3/core.c)
> can take only one PHY phandle for each of SS, HS.
> (phy-names DT property is "usb2-phy" and "usb3-phy" for each)

We never had any other requirements :-)

> The DWC3 core IP is provided by Synopsys,
> but some SoC-dependent parts (a.k.a glue-layer)
> are implemented by SoC venders.
>
> The number of connected PHY instances are SoC-dependent.
>
> If you look at generic drivers such as
> drivers/usb/host/ehci-platform.c
> the driver can handle arbitrary number of PHY instances.
>
> However, as mentioned above, DWC3 core allows only one PHY phandle
> for each SS/HS.
> This can result in a strange DT structure.
>
> For example, Socionext PXs3 SoC is integrated with 2 instances of DWC3.
>
> The instance 0 of DWC3 is connected with 2 super-speed PHYs.

why 2 super-speed phys? Is this a two-port host-only implementation?

> The instance 1 of DWC3 is connected with 1 super-speed PHY.

Are both of these instances incapable of high/full/low-speed
communication?

> According to the feed-back from Felipe Balbe,
> (https://patchwork.kernel.org/patch/10180167/)
> Socionext is trying to split the glue layer into small chunks.
>
>
> The following is the DT under internal review of Socionext.
> The full DT is super long, so
> here is only snippet for the SS PHY parts.
>
>
> [ instance 0 (with 2 SS PHYs) ]
>
> dwc3@65a00000 {
> compatible = "snps,dwc3";
> reg = <0x65a00000 0xcd00>;
> interrupt-names = "host", "peripheral";
> interrupts = <0 134 4>, <0 135 4>;
> phy-names = "usb2-phy", "usb3-phy";
> phys = <&usb0_hsphy>, <&usb0_ssphy>;
> dr_mode = "host";
> };
>
> usb0_ssphy: ss-phy {
> compatible = "socionext,uniphier-pxs3-usb3-ssphy";
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> clock-names = "phy-clk0", "phy-clk1";
> clocks = <&sys_clk 17>, <&sys_clk 18>;
> reset-names = "phy-rst0", "phy-rst1";
> resets = <&sys_rst 17>, <&sys_rst 18>;
> port0-supply = <&usb0_vbus0>;
> port1-supply = <&usb0_vbus1>;
>
> port@0 {
> reg = <0>;
> };
> port@1 {
> reg = <1>;
> };
> };
>
> [ instance 1 (with 1 SS PHY) ]
>
> dwc3@65c00000 {
> compatible = "snps,dwc3";
> reg = <0x65c00000 0xcd00>;
> interrupt-names = "host", "peripheral";
> interrupts = <0 137 4>, <0 138 4>;
> phy-names = "usb2-phy", "usb3-phy";
> phys = <&usb1_hsphy>, <&usb1_ssphy>;
> dr_mode = "host";
> };
>
> usb1_ssphy: ss-phy {
> compatible = "socionext,uniphier-pxs3-usb3-ssphy";
> #address-cells = <1>;
> #size-cells = <0>;
> #phy-cells = <0>;
> clock-names = "phy-clk0";
> clocks = <&sys_clk 21>;
> reset-names = "phy-rst0";
> resets = <&sys_rst 21>;
> port0-supply = <&usb1_vbus0>;
>
> port@0 {
> reg = <0>;
> };
> };
>
>
> I think this is strange, but the PHY driver
> counts the number of sub-nodes ("port@0", "port@1" ...)
> and iterate the port settings.
>
>
>
>
>
> In my opinion, the structure like follows
> will be more natural.
> (flattening homogeneous PHY nodes)
>
>
> [ instance 0 (with 2 SS PHYs)]
>
> dwc3@65a00000 {
> compatible = "snps,dwc3";
> reg = <0x65a00000 0xcd00>;
> interrupt-names = "host", "peripheral";
> interrupts = <0 134 4>, <0 135 4>;
> phys = <&usb0_hsphy>, <&usb0_ssphy0>, <&usb0_ssphy1>;
> dr_mode = "host";
> };
>
> usb0_ssphy0: ss-phy0@65b00300 {
> compatible = "socionext,uniphier-dwc3-ssphy";
> reg = <0x65b00300 0x10>;
> #phy-cells = <0>;
> clocks = <&sys_clk 17>;
> resets = <&sys_rst 17>;
> port-supply = <&usb0_vbus0>;
> };
>
> usb0_ssphy1: ss-phy1@65b00310 {
> compatible = "socionext,uniphier-dwc3-ssphy";
> reg = <0x65b00310 0x10>;
> #phy-cells = <0>;
> clocks = <&sys_clk 18>;
> resets = <&sys_rst 18>;
> port-supply = <&usb0_vbus1>;
> };
>
>
>
>
> [ instance 1 (with 1 SS PHY) ]
>
> usb0: dwc3@65c00000 {
> compatible = "snps,dwc3";
> reg = <0x65c00000 0xcd00>;
> interrupt-names = "host", "peripheral";
> interrupts = <0 137 4>, <0 138 4>;
> phys = <&usb1_hsphy>, <&usb1_ssphy>;
> dr_mode = "host";
> };
>
> usb1_ssphy: ss-phy@65d00300 {
> compatible = "socionext,uniphier-dwc3-ssphy";
> reg = <0x65d00300 0x10>;
> #phy-cells = <0>;
> clocks = <&sys_clk 21>;
> resets = <&sys_rst 21>;
> port0-supply = <&usb1_vbus0>;
> };
>
>
> To achieve this, I need driver changes.
>
>
> My proposal is to support arbitrary number of PHY instances
> like ehci-platform.c does.
>
>
>
> @@ -894,8 +894,8 @@ struct dwc3 {
> struct usb_phy *usb2_phy;
> struct usb_phy *usb3_phy;
>
> - struct phy *usb2_generic_phy;
> - struct phy *usb3_generic_phy;
> + unsigned int num_phys;
> + struct phy **phys;
>
> bool phys_ready;
>
>
>
> Is this OK?

I don't know, I need a bit more details about your integration :-)

--
balbi

\
 
 \ /
  Last update: 2018-04-04 07:38    [W:0.047 / U:1.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site