diff mbox series

[v2,3/3] arm64: dts: qcom: Collapse usb support into one node

Message ID 20170714214005.14967-4-stephen.boyd@linaro.org
State New
Headers show
Series [v2,1/3] mux: Add mux_control_get_optional() API | expand

Commit Message

Stephen Boyd July 14, 2017, 9:40 p.m. UTC 
We currently have three device nodes for the same USB hardware
block, as evident by the reuse of the same reg address multiple
times. Now that the chipidea driver fully supports OTG with the
MSM wrapper we can collapse all these nodes into one USB device
node, reflecting the true nature of the hardware.

Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

---
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 38 ++++++++++---------
 arch/arm64/boot/dts/qcom/msm8916.dtsi     | 62 +++++++++++++++----------------
 2 files changed, 50 insertions(+), 50 deletions(-)

-- 
2.10.0.297.gf6727b0

Comments

Shawn Guo Aug. 17, 2017, 6:43 a.m. UTC | #1 
Hi Stephen,

On Fri, Jul 14, 2017 at 02:40:05PM -0700, Stephen Boyd wrote:
> We currently have three device nodes for the same USB hardware

> block, as evident by the reuse of the same reg address multiple

> times. Now that the chipidea driver fully supports OTG with the

> MSM wrapper we can collapse all these nodes into one USB device

> node, reflecting the true nature of the hardware.

> 

> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

> ---

>  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 38 ++++++++++---------

>  arch/arm64/boot/dts/qcom/msm8916.dtsi     | 62 +++++++++++++++----------------

>  2 files changed, 50 insertions(+), 50 deletions(-)

> 

> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> index f326f4fb4d72..494560a1a6ef 100644

> --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> @@ -213,24 +213,20 @@

>  		};

>  

>  		usb@78d9000 {

> -			extcon = <&usb_id>, <&usb_id>;

> +			extcon = <&usb_id>;


I'm trying to play the series on db410c, and it doesn't seem to work as
expected.  Here is basically what I did:

- Revert ed75d6a96905, and apply the series.
- Turn on the following options:
	CONFIG_USB_CHIPIDEA_ULPI
	CONFIG_USB_ULPI_BUS
	CONFIG_PHY_QCOM_USB_HS
	CONFIG_MUX_GPIO

The role switching doesn't happen when I connect/disconnect cable
to/from micro-usb port.  But if I revert above extcon change (keep two
usb_id phandle for extcon), the role switching happens.  However, host
driver still fails to enumerate the usb mouse attached to Type-A port.

[   15.400074] ci_hdrc ci_hdrc.0: EHCI Host Controller
[   15.400119] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
[   15.419847] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
[   15.420700] hub 1-0:1.0: USB hub found
[   15.425820] hub 1-0:1.0: 1 port detected
[   15.759862] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[   31.943942] usb 1-1: device not accepting address 2, error -110
[   32.063938] usb 1-1: new high-speed USB device number 3 using ci_hdrc
[   48.071943] usb 1-1: device not accepting address 3, error -110
[   48.191935] usb 1-1: new high-speed USB device number 4 using ci_hdrc
[   58.823934] usb 1-1: device not accepting address 4, error -110
[   58.943936] usb 1-1: new high-speed USB device number 5 using ci_hdrc
[   69.575935] usb 1-1: device not accepting address 5, error -110
[   69.576001] usb usb1-port1: unable to enumerate USB device

I must have missed something.  Can you please advice?  Thanks.

Shawn
Stephen Boyd Aug. 30, 2017, 8:45 p.m. UTC | #2 
Quoting Shawn Guo (2017-08-16 23:43:49)
> Hi Stephen,

> 

> On Fri, Jul 14, 2017 at 02:40:05PM -0700, Stephen Boyd wrote:

> > We currently have three device nodes for the same USB hardware

> > block, as evident by the reuse of the same reg address multiple

> > times. Now that the chipidea driver fully supports OTG with the

> > MSM wrapper we can collapse all these nodes into one USB device

> > node, reflecting the true nature of the hardware.

> > 

> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

> > ---

> >  arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 38 ++++++++++---------

> >  arch/arm64/boot/dts/qcom/msm8916.dtsi     | 62 +++++++++++++++----------------

> >  2 files changed, 50 insertions(+), 50 deletions(-)

> > 

> > diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> > index f326f4fb4d72..494560a1a6ef 100644

> > --- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> > +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

> > @@ -213,24 +213,20 @@

> >               };

> >  

> >               usb@78d9000 {

> > -                     extcon = <&usb_id>, <&usb_id>;

> > +                     extcon = <&usb_id>;

> 

> I'm trying to play the series on db410c, and it doesn't seem to work as

> expected.  Here is basically what I did:

> 

> - Revert ed75d6a96905, and apply the series.

> - Turn on the following options:

>         CONFIG_USB_CHIPIDEA_ULPI

>         CONFIG_USB_ULPI_BUS

>         CONFIG_PHY_QCOM_USB_HS

>         CONFIG_MUX_GPIO

> 

> The role switching doesn't happen when I connect/disconnect cable

> to/from micro-usb port.


Good. That's expected.

> But if I revert above extcon change (keep two

> usb_id phandle for extcon), the role switching happens.  However, host

> driver still fails to enumerate the usb mouse attached to Type-A port.

> 

> [   15.400074] ci_hdrc ci_hdrc.0: EHCI Host Controller

> [   15.400119] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1

> [   15.419847] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00

> [   15.420700] hub 1-0:1.0: USB hub found

> [   15.425820] hub 1-0:1.0: 1 port detected

> [   15.759862] usb 1-1: new high-speed USB device number 2 using ci_hdrc

> [   31.943942] usb 1-1: device not accepting address 2, error -110

> [   32.063938] usb 1-1: new high-speed USB device number 3 using ci_hdrc

> [   48.071943] usb 1-1: device not accepting address 3, error -110

> [   48.191935] usb 1-1: new high-speed USB device number 4 using ci_hdrc

> [   58.823934] usb 1-1: device not accepting address 4, error -110

> [   58.943936] usb 1-1: new high-speed USB device number 5 using ci_hdrc

> [   69.575935] usb 1-1: device not accepting address 5, error -110

> [   69.576001] usb usb1-port1: unable to enumerate USB device

> 

> I must have missed something.  Can you please advice?  Thanks.


Yes. The role switch happens now by userspace writing different values
to the chipidea node sysfs file. For db410c it's located at
/sys/bus/platform/devices/ci_hdrc.0/role and by echoing 'host' or
'gadget' into that file you can change the role. Unplugging the cable
won't do anything anymore, because the micro-usb port is only indicating
vbus presence, and not the ID pin. At least that is my reading of the
schematic.

Obviously, this changes behavior from previous designs where
disconnecting the cable changed the role, but I don't know if we want to
support this method via the kernel alone. Instead, it seems simpler to
have userspace decide to change the role whenever it wants with some
policy. Do you have any suggestion on how this can be integrated into
userspace so we write this file at boot? That way, we can switch to host
mode by default on db410c and then users can decide to make a gadget if
they want to lose the host ports while the gadget is active. We could
probably have an extcon event handler in userspace as well to change the
role when the micro-usb cable is disconnected. That way, old behavior
could be maintained but it would be a pure policy decision in userspace.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index f326f4fb4d72..494560a1a6ef 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -213,24 +213,20 @@ 
 		};
 
 		usb@78d9000 {
-			extcon = <&usb_id>, <&usb_id>;
+			extcon = <&usb_id>;
 			status = "okay";
-		};
-
-		ehci@78d9000 {
-			status = "okay";
-		};
-
-		phy@78d9000 {
-			v1p8-supply = <&pm8916_l7>;
-			v3p3-supply = <&pm8916_l13>;
-			vddcx-supply = <&pm8916_s1>;
-			extcon = <&usb_id>, <&usb_id>;
-			dr_mode = "otg";
-			status = "okay";
-			switch-gpio = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&usb_sw_sel_pm>;
+			adp-disable;
+			hnp-disable;
+			srp-disable;
+			mux-controls = <&usb_switch>;
+			mux-control-names = "usb_switch";
+
+			ulpi {
+				phy {
+					v1p8-supply = <&pm8916_l7>;
+					v3p3-supply = <&pm8916_l13>;
+				};
+			};
 		};
 
 		lpass@07708000 {
@@ -348,6 +344,14 @@ 
 		pinctrl-0 = <&usb_id_default>;
 	};
 
+	usb_switch: usb-switch {
+		compatible = "gpio-mux";
+		mux-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
+		#mux-control-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_sw_sel_pm>;
+	};
+
 	hdmi-out {
 		compatible = "hdmi-connector";
 		type = "a";
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 17691abea608..039991f80831 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -546,44 +546,40 @@ 
 			status = "disabled";
 		};
 
-		usb_dev: usb@78d9000 {
+		otg: usb@78d9000 {
 			compatible = "qcom,ci-hdrc";
-			reg = <0x78d9000 0x400>;
-			dr_mode = "peripheral";
-			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
-			usb-phy = <&usb_otg>;
-			status = "disabled";
-		};
-
-		usb_host: ehci@78d9000 {
-			compatible = "qcom,ehci-host";
-			reg = <0x78d9000 0x400>;
-			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
-			usb-phy = <&usb_otg>;
-			status = "disabled";
-		};
-
-		usb_otg: phy@78d9000 {
-			compatible = "qcom,usb-otg-snps";
-			reg = <0x78d9000 0x400>;
+			reg = <0x78d9000 0x200>,
+			      <0x78d9200 0x200>;
 			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
-
-			qcom,vdd-levels = <500000 1000000 1320000>;
-			qcom,phy-init-sequence = <0x44 0x6B 0x24 0x13>;
-			dr_mode = "peripheral";
-			qcom,otg-control = <2>; // PMIC
-			qcom,manual-pullup;
-
 			clocks = <&gcc GCC_USB_HS_AHB_CLK>,
-				 <&gcc GCC_USB_HS_SYSTEM_CLK>,
-				 <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
-			clock-names = "iface", "core", "sleep";
-
-			resets = <&gcc GCC_USB2A_PHY_BCR>,
-				 <&gcc GCC_USB_HS_BCR>;
-			reset-names = "phy", "link";
+				 <&gcc GCC_USB_HS_SYSTEM_CLK>;
+			clock-names = "iface", "core";
+			assigned-clocks = <&gcc GCC_USB_HS_SYSTEM_CLK>;
+			assigned-clock-rates = <80000000>;
+			resets = <&gcc GCC_USB_HS_BCR>;
+			reset-names = "core";
+			phy_type = "ulpi";
+			dr_mode = "otg";
+			ahb-burst-config = <0>;
+			phy-names = "usb-phy";
+			phys = <&usb_hs_phy>;
 			status = "disabled";
+			#reset-cells = <1>;
+
+			ulpi {
+				usb_hs_phy: phy {
+					compatible = "qcom,usb-hs-phy-msm8916",
+						     "qcom,usb-hs-phy";
+					#phy-cells = <0>;
+					clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
+					clock-names = "ref", "sleep";
+					resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
+					reset-names = "phy", "por";
+					qcom,init-seq = /bits/ 8 <0x0 0x44
+						0x1 0x6b 0x2 0x24 0x3 0x13>;
+				};
+			};
 		};
 
 		intc: interrupt-controller@b000000 {