| Message ID | 20191212014233.32799-8-john.stultz@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | HiKey960 USB support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote: > From: Yu Chen <chenyu56@huawei.com> > > This patch adds binding documentation to support usb hub and usb > data role switch of Hisilicon HiKey960 Board. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > CC: ShuFan Lee <shufan_lee@richtek.com> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> > Cc: Yu Chen <chenyu56@huawei.com> > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Hans de Goede <hdegoede@redhat.com> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Jun Li <lijun.kernel@gmail.com> > Cc: Valentin Schneider <valentin.schneider@arm.com> > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com> > Cc: Jack Pham <jackp@codeaurora.org> > Cc: linux-usb@vger.kernel.org > Cc: devicetree@vger.kernel.org > Signed-off-by: Yu Chen <chenyu56@huawei.com> > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > 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@linaro.org> > + > +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 }; }; 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?) > + > + 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? > + > + 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. > + > + port: > + description: | > + any connector to the data bus of this controller should be modelled > + using the OF graph bindings specified, if the "usb-role-switch" > + property is used. Note for this driver, two ports are supported, > + the first being the endpoint that will be notified by this driver, > + and the second being the endpoint that notifies this driver of a > + role switch. > + > + > +required: > + - compatible > + - typec-vbus-gpios > + - otg-switch-gpios > + - hub-vdd33-en-gpios > + - usb-role-switch > + - port > + > +additionalProperties: false > + > +examples: > + - | > + hisi_hikey_usb: hisi_hikey_usb { > + compatible = "hisilicon,gpio_hubv1"; > + typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>; > + otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>; > + hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>; > + usb-role-switch; > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + hikey_usb_ep0: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&dwc3_role_switch>; > + }; > + hikey_usb_ep1: endpoint@1 { > + reg = <1>; > + remote-endpoint = <&rt1711h_ep>; > + }; > + }; > + }; > -- > 2.17.1 >
On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote: > > On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote: > > From: Yu Chen <chenyu56@huawei.com> > > > > This patch adds binding documentation to support usb hub and usb > > data role switch of Hisilicon HiKey960 Board. > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > CC: ShuFan Lee <shufan_lee@richtek.com> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> > > Cc: Yu Chen <chenyu56@huawei.com> > > Cc: Felipe Balbi <balbi@kernel.org> > > Cc: Hans de Goede <hdegoede@redhat.com> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > > Cc: Jun Li <lijun.kernel@gmail.com> > > Cc: Valentin Schneider <valentin.schneider@arm.com> > > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com> > > Cc: Jack Pham <jackp@codeaurora.org> > > Cc: linux-usb@vger.kernel.org > > Cc: devicetree@vger.kernel.org > > Signed-off-by: Yu Chen <chenyu56@huawei.com> > > Signed-off-by: John Stultz <john.stultz@linaro.org> > > --- > > 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@linaro.org> > > + > > +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. > 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. > > + > > + 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? 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 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 thanks -john
On Wed, Dec 18, 2019 at 11:21 AM John Stultz <john.stultz@linaro.org> wrote: > > On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Dec 12, 2019 at 01:42:32AM +0000, John Stultz wrote: > > > From: Yu Chen <chenyu56@huawei.com> > > > > > > This patch adds binding documentation to support usb hub and usb > > > data role switch of Hisilicon HiKey960 Board. > > > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Cc: Rob Herring <robh+dt@kernel.org> > > > Cc: Mark Rutland <mark.rutland@arm.com> > > > CC: ShuFan Lee <shufan_lee@richtek.com> > > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com> > > > Cc: Yu Chen <chenyu56@huawei.com> > > > Cc: Felipe Balbi <balbi@kernel.org> > > > Cc: Hans de Goede <hdegoede@redhat.com> > > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Cc: Jun Li <lijun.kernel@gmail.com> > > > Cc: Valentin Schneider <valentin.schneider@arm.com> > > > Cc: Guillaume Gardet <Guillaume.Gardet@arm.com> > > > Cc: Jack Pham <jackp@codeaurora.org> > > > Cc: linux-usb@vger.kernel.org > > > Cc: devicetree@vger.kernel.org > > > Signed-off-by: Yu Chen <chenyu56@huawei.com> > > > Signed-off-by: John Stultz <john.stultz@linaro.org> > > > --- > > > 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@linaro.org> > > > + > > > +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
(Sorry for the slow reply. The holidays and other priorities struck and I'm only now just getting back to this!) On Wed, Dec 18, 2019 at 11:57 AM Rob Herring <robh@kernel.org> wrote: > On Wed, Dec 18, 2019 at 11:21 AM John Stultz <john.stultz@linaro.org> wrote: > > On Wed, Dec 18, 2019 at 8:37 AM Rob Herring <robh@kernel.org> wrote: > > > 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. Fair point. Though at least for the USB5734 hub, I don't believe we've made use of the i2c on it (at least that I can see). From the existing driver, the control is basically just 3 GPIOs: type-c power, hub power, and the mux/switch. Trying to get my head around your suggestion: ctrl { mux-controls = <&usb_gpio_mux>; connector@0 { // type C connector binding }; hub@1 { // USB device binding }; }; The usb_gpio_mux would be the gpio mux switch. For the "type C connector binding", that would probably be the rt1711h type-c chip. For the "USB device binding" that would be a yet to be implemented USB5734 hub driver, I'm guessing? Though I'm not sure I understand how the hub driver gets a signal to power-on/power-off its gpio-regulator from the mux state? I'm maybe still missing some details on the mux infrastructure. > > > 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). So this is a little confusing to me. Why is the host controller involved? It seems to me all of this is down-stream of the controller, and it's just taking whatever the switch gives it. I think the switch needs to be signalled from the rt1711h type-c connector (when it detects the cable and determines the role). That said, I'm not sure how it would think to control the mux in that case (as that's pretty special case for this specific hardware). That's why we're using the role notifier intermediary trick in the current code. So I guess we could still have the roll notifier intermediary driver which then controls the mux? I know that's more a driver implementation detail and not really hardware description, :) but I'm just trying to figure out how it's going to come together and actually work. > > 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. My concern is that I suspect the issue we had then was that the order and the timing that we switch the 3 GPIOs ends up being important. In the current implementation we can adjust them linearly together, where as when I took a stab at having the vbus gpio was modeled as a regulator and controlled independently by the rt1711h, the typec state machine would get confused as I'm guessing the mux/switch wasn't where it expected it to be when it wanted to change the state of the type-c vbus. Thoughts? Thanks again for the review and feedback. And sorry to let so much time (and mental context) pass. -john
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@linaro.org> + +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. + + 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 + + typec-vbus-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the typec-vbus gpio + + otg-switch-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the otg-switch gpio + + hub-vdd33-en-gpios: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle to the hub 3.3v power enablement gpio + + usb-role-switch: + $ref: /schemas/types.yaml#/definitions/flag + description: Support role switch. + + port: + description: | + any connector to the data bus of this controller should be modelled + using the OF graph bindings specified, if the "usb-role-switch" + property is used. Note for this driver, two ports are supported, + the first being the endpoint that will be notified by this driver, + and the second being the endpoint that notifies this driver of a + role switch. + + +required: + - compatible + - typec-vbus-gpios + - otg-switch-gpios + - hub-vdd33-en-gpios + - usb-role-switch + - port + +additionalProperties: false + +examples: + - | + hisi_hikey_usb: hisi_hikey_usb { + compatible = "hisilicon,gpio_hubv1"; + typec-vbus-gpios = <&gpio25 2 GPIO_ACTIVE_HIGH>; + otg-switch-gpios = <&gpio25 6 GPIO_ACTIVE_HIGH>; + hub-vdd33-en-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>; + usb-role-switch; + + port { + #address-cells = <1>; + #size-cells = <0>; + + hikey_usb_ep0: endpoint@0 { + reg = <0>; + remote-endpoint = <&dwc3_role_switch>; + }; + hikey_usb_ep1: endpoint@1 { + reg = <1>; + remote-endpoint = <&rt1711h_ep>; + }; + }; + };