Re: Multiple generic PHY instances for DWC3 USB IP
Hi Arnd, 2018-04-04 17:43 GMT+09:00 Arnd Bergmann : > On Wed, Apr 4, 2018 at 10:00 AM, Felipe Balbi > wrote: >> >> Hi, >> >> Masahiro Yamada writes: > Each DWC3 instance is connected with > multiple HS PHYs and multiple SS PHYs, > depending on the number of ports. in that case, you shouldn't need dwc3 at all. A Host-only dwc3 is xHCI compliant. If you really don't have the gadget block, there's no need for you to use dwc3. Just use xhci-plat directly. >>> >>> Sorry, I was misunderstanding. >>> >>> Some of our SoCs support gadget, >>> so we need to use the dwc3 driver. >> >> fair enough. Now we need to figure out how to pass multiply PHYs to a >> multi-port dwc3 instance. >> >> Kishon, any ideas? How do you think DT should look like? > > See this series from Martin Blumenstingl: > > https://www.spinics.net/lists/linux-usb/msg166281.html > > Arnd Very useful information! Not tested yet, but I quickly reviewed the series visually, and probably this will work for us. Thanks! -- Best Regards Masahiro Yamada
Re: Multiple generic PHY instances for DWC3 USB IP
On Wed, Apr 4, 2018 at 10:00 AM, Felipe Balbi wrote: > > Hi, > > Masahiro Yamada writes: Each DWC3 instance is connected with multiple HS PHYs and multiple SS PHYs, depending on the number of ports. >>> >>> in that case, you shouldn't need dwc3 at all. A Host-only dwc3 is xHCI >>> compliant. If you really don't have the gadget block, there's no need >>> for you to use dwc3. Just use xhci-plat directly. >> >> Sorry, I was misunderstanding. >> >> Some of our SoCs support gadget, >> so we need to use the dwc3 driver. > > fair enough. Now we need to figure out how to pass multiply PHYs to a > multi-port dwc3 instance. > > Kishon, any ideas? How do you think DT should look like? See this series from Martin Blumenstingl: https://www.spinics.net/lists/linux-usb/msg166281.html Arnd
Re: Multiple generic PHY instances for DWC3 USB IP
Hi, Masahiro Yamada writes: >>> Each DWC3 instance is connected with >>> multiple HS PHYs and multiple SS PHYs, >>> depending on the number of ports. >> >> in that case, you shouldn't need dwc3 at all. A Host-only dwc3 is xHCI >> compliant. If you really don't have the gadget block, there's no need >> for you to use dwc3. Just use xhci-plat directly. > > Sorry, I was misunderstanding. > > Some of our SoCs support gadget, > so we need to use the dwc3 driver. fair enough. Now we need to figure out how to pass multiply PHYs to a multi-port dwc3 instance. Kishon, any ideas? How do you think DT should look like? -- balbi
Re: Multiple generic PHY instances for DWC3 USB IP
2018-04-04 15:04 GMT+09:00 Felipe Balbi : > > Hi, > > Masahiro Yamada writes: >> 2018-04-04 14:36 GMT+09:00 Felipe Balbi : >>> >>> Hi, >>> >>> Masahiro Yamada 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? >> >> >> Socionext SoCs only support the host-mode. >> >> >> The instance 0 has 2 ports. >> In our integration, 1 SS PHY is needed for each port. >> That's why it needs 2 SS PHYs. >> >> Each DWC3 instance is connected with >> multiple HS PHYs and multiple SS PHYs, >> depending on the number of ports. > > in that case, you shouldn't need dwc3 at all. A Host-only dwc3 is xHCI > compliant. If you really don't have the gadget block, there's no need > for you to use dwc3. Just use xhci-plat directly. Sorry, I was misunderstanding. Some of our SoCs support gadget, so we need to use the dwc3 driver. Is this OK? >>> >>> I don't know, I need a bit more details about your integration :-) >> >> >> I can send a patch. >> >> My concern is the following commit. >> I do not know which parts are using this lookups. > > Samsung SoCs, probably ;-) > > Anyway, if your IP really is host-only, then you don't need dwc3 for > anything. Just go for xHCI directly. If xHCI needs to be extended when > it comes to PHY, then you can discuss with Mathias Nyman :-) > > -- > balbi -- Best Regards Masahiro Yamada
Re: Multiple generic PHY instances for DWC3 USB IP
Hi, Masahiro Yamada writes: > 2018-04-04 14:36 GMT+09:00 Felipe Balbi : >> >> Hi, >> >> Masahiro Yamada 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? > > > Socionext SoCs only support the host-mode. > > > The instance 0 has 2 ports. > In our integration, 1 SS PHY is needed for each port. > That's why it needs 2 SS PHYs. > > Each DWC3 instance is connected with > multiple HS PHYs and multiple SS PHYs, > depending on the number of ports. in that case, you shouldn't need dwc3 at all. A Host-only dwc3 is xHCI compliant. If you really don't have the gadget block, there's no need for you to use dwc3. Just use xhci-plat directly. >>> Is this OK? >> >> I don't know, I need a bit more details about your integration :-) > > > I can send a patch. > > My concern is the following commit. > I do not know which parts are using this lookups. Samsung SoCs, probably ;-) Anyway, if your IP really is host-only, then you don't need dwc3 for anything. Just go for xHCI directly. If xHCI needs to be extended when it comes to PHY, then you can discuss with Mathias Nyman :-) -- balbi
Re: Multiple generic PHY instances for DWC3 USB IP
2018-04-04 14:36 GMT+09:00 Felipe Balbi :
>
> Hi,
>
> Masahiro Yamada 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?
Socionext SoCs only support the host-mode.
The instance 0 has 2 ports.
In our integration, 1 SS PHY is needed for each port.
That's why it needs 2 SS PHYs.
Each DWC3 instance is connected with
multiple HS PHYs and multiple SS PHYs,
depending on the number of ports.
>> The instance 1 of DWC3 is connected with 1 super-speed PHY.
>
> Are both of these instances incapable of high/full/low-speed
> communication?
Also HS PHYs are connected.
To narrow down the problem,
I picked up the DT snippet only for SS PHY device nodes.
>> @@ -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 intnum_phys;
>> + struct phy **phys;
>>
>> boolphys_ready;
>>
>>
>>
>> Is this OK?
>
> I don't know, I need a bit more details about your integration :-)
I can send a patch.
My concern is the following commit.
I do not know which parts are using this lookups.
commit 08f871a3aca252b15107fc37dedcdacbac80fdb5
Author: Heikki Krogerus
Date: Wed Nov 19 17:28:23 2014 +0200
usb: dwc3: host: convey the PHYs to xhci
On some platforms a PHY may need to be handled also in the
host controller driver. Exynos5420 SoC requires some "PHY
tuning" based on the USB speed. This patch delivers dwc3's
PHYs to the xhci platform device when it's created.
Signed-off-by: Heikki Krogerus
Tested-by: Vivek Gautam
Acked-by: Felipe Balbi
Signed-off-by: Kishon Vijay Abraham I
--
Best Regards
Masahiro Yamada
Re: Multiple generic PHY instances for DWC3 USB IP
Hi,
Masahiro Yamada 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@65a0 {
> compatible = "snps,dwc3";
> reg = <0x65a0 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@65c0 {
> compatible = "snps,dwc3";
> reg = <0x65c0 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@65a0 {
> compatible = "snps,dwc3";
> reg = <0x65a0 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@65c0 {
> compatible = "snps,dwc3";
> reg = <0x65c0 0xcd00>;
> interrupt-names =
Multiple generic PHY instances for DWC3 USB IP
Hi.
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)
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.
The instance 1 of DWC3 is connected with 1 super-speed PHY.
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@65a0 {
compatible = "snps,dwc3";
reg = <0x65a0 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@65c0 {
compatible = "snps,dwc3";
reg = <0x65c0 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@65a0 {
compatible = "snps,dwc3";
reg = <0x65a0 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@65c0 {
compatible = "snps,dwc3";
reg = <0x65c0 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_
