Re: [RFC PATCH 1/2] usb: host: add a generic platform USB roothub driver

Hi Rob,

On Fri, Jan 13, 2017 at 9:21 PM, Martin Blumenstingl
[off-list ref] wrote:

Hi Rob,

On Fri, Jan 13, 2017 at 9:08 PM, Rob Herring [off-list ref] wrote:

quoted

On Wed, Jan 11, 2017 at 04:29:46PM +0100, Martin Blumenstingl wrote:

quoted

Many SoC platforms have separate devices for the USB PHY which are
registered through the generic PHY framework. These PHYs have to be
enabled to make the USB controller actually work. They also have to be
disabled again on shutdown/suspend.

Currently (at least) the following HCI platform drivers are using custom
code to obtain all PHYs via devicetree for the roothub/controller and
disable/enable them when required:
- ehci-platform.c has ehci_platform_power_{on,off}
- xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
- ohci-platform.c has ohci_platform_power_{on,off}

These drivers are not using the generic devicetree USB device bindings
yet which were only introduced recently (documentation is available in
devicetree/bindings/usb/usb-device.txt).
With this new driver the usb2-phy and usb3-phy can be specified directly
in the child-node of the corresponding port of the roothub via
devicetree. This can be extended by not just parsing PHYs (some of the
other drivers listed above are for example also parsing a list of clocks
as well) when required.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../devicetree/bindings/usb/usb-roothub.txt        |  41 ++++++
 drivers/usb/host/Kconfig                           |   3 +
 drivers/usb/host/Makefile                          |   2 +
 drivers/usb/host/platform-roothub.c                | 146 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  14 ++
 5 files changed, 206 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt b/Documentation/devicetree/bindings/usb/usb-roothub.txt
new file mode 100644
index 000000000000..96e152d3901c
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt

@@ -0,0 +1,41 @@
+Generic USB root-hub Properties
+
+similar to the USB device bindings (documented in usb-device.txt from the
+current directory) this provides support for configuring the root-hub.
+
+Required properties:
+- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
+- reg: must be 0.
+- address-cells: must be 1
+- size-cells: must be 0

quoted

+- a sub-node per port supports the following properties:

Make this another section with required and optional sections.

I can do that, but let's wait for the results if we want the PHYs to
be specified at the grand-child or child level

quoted

quoted

+  - reg: the port number on the root-hub (mandatory)
+  - phys: optional, from the *Generic PHY* bindings (mandatory needed when
+    phy-names is given)
+  - phy-names: optional, from the *Generic PHY* bindings; supported names
+    are "usb2-phy" or "usb3-phy"
+
+Example:
+     &usb1 {
+             #address-cells = <1>;
+             #size-cells = <0>;
+
+             roothub@0 {
+                     compatible = "usb1d6b,3", "usb1d6b,2";

Is this discoverable? IIRC, we had decided that ports on the root hub
are just children of the USB controller node (rather than
grandchildren). Why does that not work?

if I understand you correctly you are thinking of something like this:
&usb1 {
    ...cells...

    port@1 {
        reg = <1>;
        phys = <&phy2>
    }

    port@2 {
        reg = <2>;
        phys = <&phy2>
    }
}

in that case we need a way to differentiate between "actual device at
port 1" and "configuration for root-hub port 1".
in that example I also cannot specify a compatible string since I
don't know which device might be plugged into that port.

could you please share your thoughts on this? can you point me to any
documentation or a discussion on a mailing-list which describes how
ports on the root hub should be modeled?

quoted

quoted

+                     #address-cells = <1>;
+                     #size-cells = <0>;
+                     reg = <0>;
+
+                     port@1 {

Wouldn't this normally be 0 and 1. This should probably be usb-port
rather than port to avoid OF graph overlap.

the USB subsystem starts counting at 1, there can never be a "device
with device-number 0" - so I think we should stay with 1 and 2 for a
2-port roothub.
I'm fine with changing the name to "usb-port" though

quoted

quoted

+                             reg = <1>;
+                             usb-phy = <&usb2_phy1>, <&usb3_phy1>;

s/usb-phy/phys/

thanks for spotting this

quoted

quoted

+                             phy-names = "usb2-phy", "usb3-phy";
+                     };
+
+                     port@2 {
+                             reg = <2>;
+                             usb-phy = <&usb2_phy2>, <&usb3_phy2>;
+                             phy-names = "usb2-phy", "usb3-phy";
+                     };
+             };
+     }

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html