| 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
| ||
|
Date: Wed, 04 Apr 2018 08:36:53 +0300
From: Felipe Balbi <felipe.balbi@...ux.intel.com>
To: Masahiro Yamada <yamada.masahiro@...ionext.com>,
linux-usb@...r.kernel.org
Cc: Lee Jones <lee.jones@...aro.org>, Arnd Bergmann <arnd@...db.de>,
Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
Andrew Lunn <andrew@...n.ch>,
Masami Hiramatsu <masami.hiramatsu@...aro.org>,
Jassi Brar <jaswinder.singh@...aro.org>,
Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Multiple generic PHY instances for DWC3 USB IP
Hi,
Masahiro Yamada <yamada.masahiro@...ionext.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@...00000 {
> 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@...00000 {
> 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@...00000 {
> 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@...00300 {
> 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@...00310 {
> 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@...00000 {
> 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@...00300 {
> 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
Powered by blists - more mailing lists