| Message ID | 20170714214005.14967-4-stephen.boyd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 --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 {
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(-)