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

[Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>]

Hi Rob,

On Fri, Jan 13, 2017 at 9:21 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Rob,
>
> On Fri, Jan 13, 2017 at 9:08 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Jan 11, 2017 at 04:29:46PM +0100, Martin Blumenstingl wrote:
>>> 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
>>
>>> +- 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
>
>>> +  - 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?

>>> +                     #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
>
>>> +                             reg = <1>;
>>> +                             usb-phy = <&usb2_phy1>, <&usb3_phy1>;
>>
>> s/usb-phy/phys/
> thanks for spotting this
>
>>> +                             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