On Wed, Dec 18, 2019 at 11:21 AM John Stultz <john.stultz@xxxxxxxxxx> wrote: > > On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > > > On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote: > > > From: Yu Chen <chenyu56@xxxxxxxxxx> > > > > > > This patch adds binding documentation to support usb hub and usb > > > data role switch of Hisilicon HiKey960 Board. > > > > > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > > > Cc: Mark Rutland <mark.rutland@xxxxxxx> > > > CC: ShuFan Lee <shufan_lee@xxxxxxxxxxx> > > > Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > > Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx> > > > Cc: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> > > > Cc: Yu Chen <chenyu56@xxxxxxxxxx> > > > Cc: Felipe Balbi <balbi@xxxxxxxxxx> > > > Cc: Hans de Goede <hdegoede@xxxxxxxxxx> > > > Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > > Cc: Jun Li <lijun.kernel@xxxxxxxxx> > > > Cc: Valentin Schneider <valentin.schneider@xxxxxxx> > > > Cc: Guillaume Gardet <Guillaume.Gardet@xxxxxxx> > > > Cc: Jack Pham <jackp@xxxxxxxxxxxxxx> > > > Cc: linux-usb@xxxxxxxxxxxxxxx > > > Cc: devicetree@xxxxxxxxxxxxxxx > > > Signed-off-by: Yu Chen <chenyu56@xxxxxxxxxx> > > > Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> > > > --- > > > v3: Reworked as usb-role-switch intermediary > > > > > > v7: Switched over to YAML dt binding description > > > --- > > > .../bindings/misc/hisilicon-hikey-usb.yaml | 85 +++++++++++++++++++ > > > 1 file changed, 85 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml > > > new file mode 100644 > > > index 000000000000..1fc3b198ef73 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml > > > @@ -0,0 +1,85 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +# Copyright 2019 Linaro Ltd. > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/misc/hisilicon-hikey-usb.yaml#"; > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > > > + > > > +title: HiKey960 onboard USB GPIO Hub > > > + > > > +maintainers: > > > + - John Stultz <john.stultz@xxxxxxxxxx> > > > + > > > +description: | > > > + Supports the onboard HiKey960 USB GPIO hub, which acts as a > > > + role-switch intermediary to detect the state of the USB-C > > > + port, to switch the hub into dual-role USB-C or host mode, > > > + which enables the onboard USB-A host ports. > > > > Honestly I'm torn between whatever works for you because this is pretty > > "special" dev board design and it should more accurately match the > > hardware design. I think we can do the later and it doesn't really need > > anything new. > > > > > + > > > + Schematics about the hub can be found here: > > > + https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - const: hisilicon,gpio_hubv1 > > > > As a whole this is HiSilicon specific, but really it is not. It's really > > just a hub, a mux, and connectors for which we have bindings for. I > > think you need to model the actual hub in DT. We have 2 ways already to > > describe hubs in DT: a I2C device or USB device. > > > > AIUI, the board looks something like this: > > > > ctrl -> mux --> hub -> type-a connector > > +-> type-c connector > > > > If the hub I2C is not used, then you could do something like this: > > > > ctrl { > > mux-controls = <&usb_gpio_mux>; > > connector@0 { > > // type C connector binding > > }; > > hub@1 { > > // USB device binding > > }; > > }; > > I can't say I totally grok all this, but I'll go digging to try to > better understand it. > I don't believe there is any I2C involved here, so I'll try the > approach you outline above. Well, it is there in the schematics. > > Or if I2C is used and the hub is under the I2C controller: > > > > ctrl { > > port@0 { > > mux-controls = <&usb_gpio_mux>; > > endpoint@0 { // mux state 0 > > remote-endpoint = <&usb_c_connector_port>; > > }; > > endpoint@1 { // mux state 1 > > remote-endpoint = <&usb_hub_port>; > > }; > > }; > > > > The only new bindings you really need are adding 'mux-controls' to the > > USB host controller and the hub binding (we already have a few). > > > > If the USB2 and USB3 signals come from 2 different host controller > > nodes, then I think it will need to look like the 2nd case regardless > > of I2C. (It's strange that USB3 was not routed to Type-C connector. Can > > you do USB2 on Type-C and USB3 on hub simultaneously? You need USB2 to > > enumerate, right?) > > Yea, it is strange, and I unfortunately don't know why only USB2 was > exported to the type-c connector. > And to my knowledge, you cannot use both the type-c and hub simultaneously. > > > > > + > > > + typec-vbus-gpios: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle to the typec-vbus gpio > > > > This should be modeled as a GPIO regulator, and belongs as part of a > > connector node. See bindings/connector/usb-connector.txt. > > > > > + > > > + otg-switch-gpios: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle to the otg-switch gpio > > > > This would be the gpio-mux binding instead. > > > > > + > > > + hub-vdd33-en-gpios: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle to the hub 3.3v power enablement gpio > > > > This should be modeled as a GPIO regulator. > > > > What about the reset line on the hub? > > Unknown. I don't have any details on that. You might just be getting lucky that it is pulled to the right state. > > > + usb-role-switch: > > > + $ref: /schemas/types.yaml#/definitions/flag > > > + description: Support role switch. > > > > This normally is a controller property. Role switch is foreign to the > > hub, so doesn't really belong there for sure. > > So this part was critical to being able to get role switch > notification from the connector and to properly switch modes without > adding extra notifier gunk from the previous patch that folks didn't > like. > > Trying to understand further, your suggestion here is to re-model the > binding, as gpio regulators and gpio muxes, and use a usb-connector > node to describe them, but I'm missing how I connect that to the > driver implementation I have? Good question, but that shouldn't really dictate your binding design. > Is the idea to extend the rt1711h and > dwc3 drivers further to support the mux/hub bit (this part is fairly > foggy to me), completely removing the need for the misc driver? I imagine that you need some driver to determine the state of the mux. Perhaps a usb-mux driver which is instantiated by the host controller driver when it sees a mux-controls property. Sorry, haven't looked at the driver side of this at all. > I did take an attempt at something similar with an earlier iteration > of the patch set, where I was trying to move the vbus-gpio as a > gpio-regulator to be controlled by the rt1711h/tpcm core, but that > approach didn't work properly and Hans suggested I just go back to the > approach submitted here: > https://lkml.org/lkml/2019/10/22/42 I don't see why that would matter. If you need to sense the Vbus state, then you do need a GPIO typically. But for an enable line, it's just another level of abstraction. Rob
