| Message ID | 20200311191501.8165-3-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DWC3/Qualcomm connector based role-switching | expand |
Quoting Bryan O'Donoghue (2020-03-11 12:14:56) > A USB connector should be a child node of the USB controller > connector/usb-connector.txt. This patch adds an example of how to do this > to the dwc3 binding descriptions. I read that as a child of the USB interface controller, which is not the same as the USB controller. For example, we're talking about having the usb connector be a child of the EC on ChromeOS devices because that manages the connector > > It is necessary to declare a connector as a child-node of a USB controller > for role-switching to work, so this example should be helpful to others > implementing that. Maybe it should be a virtual node at the root of the DT if it's GPIO controlled? And then the phy can be connected to the usb connector through the graph binding. > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: linux-usb@vger.kernel.org > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Acked-by: Felipe Balbi <balbi@kernel.org> > Reviewed-by: Rob Herring <robh@kernel.org> > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt > index 66780a47ad85..4e1e4afccee6 100644 > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > @@ -121,4 +121,12 @@ dwc3@4a030000 { > interrupts = <0 92 4> > usb-phy = <&usb2_phy>, <&usb3,phy>; Weird that there's a comma here ^ Not a problem with this patch, just noticing it while reading the diff. > snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>; > + > + usb_con: connector { > + compatible = "gpio-usb-b-connector"; > + id-gpios = <&tlmm 116 GPIO_ACTIVE_HIGH>; > + vbus-supply = <&usb3_vbus_reg>; > + pinctrl-names = "default"; > + pinctrl-0 = <&usb3_id_pin>, <&usb3_vbus_pin>; > + };
On 19/03/2020 01:08, Stephen Boyd wrote: > Quoting Bryan O'Donoghue (2020-03-11 12:14:56) >> A USB connector should be a child node of the USB controller >> connector/usb-connector.txt. This patch adds an example of how to do this >> to the dwc3 binding descriptions. > > I read that as a child of the USB interface controller, which is not the > same as the USB controller. For example, we're talking about having the > usb connector be a child of the EC on ChromeOS devices because that > manages the connector > >> >> It is necessary to declare a connector as a child-node of a USB controller >> for role-switching to work, so this example should be helpful to others >> implementing that. > > Maybe it should be a virtual node at the root of the DT if it's GPIO > controlled? And then the phy can be connected to the usb connector > through the graph binding. Graph binding can probably work. Re: the PHY. For myself the hardware model is Connector -> PHY -> Host controller -> Host controller wrapper Only Connector -> Host controller -> Host controller wrapper care about the USB role though. If your PHY did care about the role, you'd really need to write a connector/phy type-c type driver, to detect the state and toggle your PHY bits before doing usb_role_switch_set_role() back to DWC3. At least that's my understanding. --- bod
Quoting Bryan O'Donoghue (2020-03-19 08:22:14) > On 19/03/2020 01:08, Stephen Boyd wrote: > > > > Maybe it should be a virtual node at the root of the DT if it's GPIO > > controlled? And then the phy can be connected to the usb connector > > through the graph binding. > > Graph binding can probably work. > > Re: the PHY. > > For myself the hardware model is > > Connector -> PHY -> Host controller -> Host controller wrapper > > Only > > Connector -> Host controller -> Host controller wrapper > > care about the USB role though. > > If your PHY did care about the role, you'd really need to write a > connector/phy type-c type driver, to detect the state and toggle your > PHY bits before doing usb_role_switch_set_role() back to DWC3. > Yes some PHYs do care about the role. Sometimes they have to toggle some bit to switch between host and gadget mode for example. I haven't fully read this patch series but maybe the PHY can be the one that controls the gpio for the connector? We (ChromeOS) need to integrate the type-c connector class, etc. on sc7180 with the dwc3 driver and the current thinking has the type-c connectors underneath the cros_ec node because the EC is the type-c manager. The EC will have a type-c driver associated with it.
On 19/03/2020 16:40, Stephen Boyd wrote: > Quoting Bryan O'Donoghue (2020-03-19 08:22:14) >> On 19/03/2020 01:08, Stephen Boyd wrote: >>> >>> Maybe it should be a virtual node at the root of the DT if it's GPIO >>> controlled? And then the phy can be connected to the usb connector >>> through the graph binding. >> >> Graph binding can probably work. >> >> Re: the PHY. >> >> For myself the hardware model is >> >> Connector -> PHY -> Host controller -> Host controller wrapper >> >> Only >> >> Connector -> Host controller -> Host controller wrapper >> >> care about the USB role though. >> >> If your PHY did care about the role, you'd really need to write a >> connector/phy type-c type driver, to detect the state and toggle your >> PHY bits before doing usb_role_switch_set_role() back to DWC3. >> > > Yes some PHYs do care about the role. Sometimes they have to toggle some > bit to switch between host and gadget mode for example. I haven't fully > read this patch series but maybe the PHY can be the one that controls > the gpio for the connector? Previous version of the PHY from 2019 had extcon toggling vbus. Since extcon is going away, we moved go usb-gpio https://lwn.net/ml/devicetree/20190905175802.GA19599@jackp-linux.qualcomm.com/ https://lwn.net/ml/devicetree/5d71edf5.1c69fb81.1f307.fdd6@mx.google.com/ usb-gpio-conn handle VBUS and notifies via the USB role switch API. Which if the connector is a child of the controller "just works" but, maybe with a little bit of work DT <port> references could do the same thing and the connector wouldn't need to be declared as a child. > We (ChromeOS) need to integrate the type-c connector class, etc. on > sc7180 with the dwc3 driver and the current thinking has the type-c > connectors underneath the cros_ec node because the EC is the type-c > manager. The EC will have a type-c driver associated with it. right and you don't want, doesn't work or doesn't make sense, to declare cros_ec as a child of DWC3, fair enough. I guess a DT remote-endpoint{} will do the job. Something like: arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi --- bod
Quoting Bryan O'Donoghue (2020-03-19 11:03:58) > On 19/03/2020 16:40, Stephen Boyd wrote: > > the gpio for the connector? > > Previous version of the PHY from 2019 had extcon toggling vbus. > > Since extcon is going away, we moved go usb-gpio > > https://lwn.net/ml/devicetree/20190905175802.GA19599@jackp-linux.qualcomm.com/ > > https://lwn.net/ml/devicetree/5d71edf5.1c69fb81.1f307.fdd6@mx.google.com/ > > usb-gpio-conn handle VBUS and notifies via the USB role switch API. > > Which if the connector is a child of the controller "just works" but, > maybe with a little bit of work DT <port> references could do the same > thing and the connector wouldn't need to be declared as a child. > > > We (ChromeOS) need to integrate the type-c connector class, etc. on > > sc7180 with the dwc3 driver and the current thinking has the type-c > > connectors underneath the cros_ec node because the EC is the type-c > > manager. The EC will have a type-c driver associated with it. > > right and you don't want, doesn't work or doesn't make sense, to declare > cros_ec as a child of DWC3, fair enough. > > I guess a DT remote-endpoint{} will do the job. > > Something like: > arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi > Yes something like that, but it looks like that dtsi file has the usb host controller connected through a remote-endpoint to the type-c connector. I was under the impression that it would only be the phy that is connected this way because it's possible for there to be multiple different phys that drive data out through one connector. For example, in type-c the DP phy can be different from the USB2 phy or USB3 phy and there could even be other things like an HDMI phy too that all go to the same connector.
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt index 66780a47ad85..4e1e4afccee6 100644 --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ b/Documentation/devicetree/bindings/usb/dwc3.txt @@ -121,4 +121,12 @@ dwc3@4a030000 { interrupts = <0 92 4> usb-phy = <&usb2_phy>, <&usb3,phy>; snps,incr-burst-type-adjustment = <1>, <4>, <8>, <16>; + + usb_con: connector { + compatible = "gpio-usb-b-connector"; + id-gpios = <&tlmm 116 GPIO_ACTIVE_HIGH>; + vbus-supply = <&usb3_vbus_reg>; + pinctrl-names = "default"; + pinctrl-0 = <&usb3_id_pin>, <&usb3_vbus_pin>; + }; };