* [PATCH 0/2] X1E80100 multiport USB controller
@ 2024-08-09 13:18 Konrad Dybcio
2024-08-09 13:18 ` [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller Konrad Dybcio
2024-08-09 13:18 ` [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller Konrad Dybcio
0 siblings, 2 replies; 11+ messages in thread
From: Konrad Dybcio @ 2024-08-09 13:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio
Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree,
linux-kernel, Krishna Kurapati, Konrad Dybcio, Konrad Dybcio
This series configures the multiport USB controller on X Elite. No
driver changes seem necessary, tested on the Surface Laptop 7.
Signed-off-by: Konrad Dybcio <quic_kdybcio@quicinc.com>
---
Konrad Dybcio (2):
dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller
arm64: dts: qcom: x1e80100: Add USB Multiport controller
.../devicetree/bindings/usb/qcom,dwc3.yaml | 3 +
arch/arm64/boot/dts/qcom/x1e80100.dtsi | 170 +++++++++++++++++++++
2 files changed, 173 insertions(+)
---
base-commit: 1e391b34f6aa043c7afa40a2103163a0ef06d179
change-id: 20240809-topic-h_mp-52839d894530
Best regards,
--
Konrad Dybcio <quic_kdybcio@quicinc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller 2024-08-09 13:18 [PATCH 0/2] X1E80100 multiport USB controller Konrad Dybcio @ 2024-08-09 13:18 ` Konrad Dybcio 2024-08-10 12:47 ` Krzysztof Kozlowski 2024-08-09 13:18 ` [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller Konrad Dybcio 1 sibling, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2024-08-09 13:18 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio From: Konrad Dybcio <quic_kdybcio@quicinc.com> The X1E80100, just like its predecessors, has a Multiport controller. This time around, 2 HS (eUSB) and 2 SS PHYs are attached. Document it. Signed-off-by: Konrad Dybcio <quic_kdybcio@quicinc.com> --- Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml index e99c55bb4e9c..18758efb8d29 100644 --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml @@ -52,6 +52,7 @@ properties: - qcom,sm8550-dwc3 - qcom,sm8650-dwc3 - qcom,x1e80100-dwc3 + - qcom,x1e80100-dwc3-mp - const: qcom,dwc3 reg: @@ -289,6 +290,7 @@ allOf: - qcom,sc8280xp-dwc3 - qcom,sc8280xp-dwc3-mp - qcom,x1e80100-dwc3 + - qcom,x1e80100-dwc3-mp then: properties: clocks: @@ -501,6 +503,7 @@ allOf: contains: enum: - qcom,sc8180x-dwc3-mp + - qcom,x1e80100-dwc3-mp then: properties: interrupts: -- 2.46.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller 2024-08-09 13:18 ` [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller Konrad Dybcio @ 2024-08-10 12:47 ` Krzysztof Kozlowski 0 siblings, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2024-08-10 12:47 UTC (permalink / raw) To: Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 09/08/2024 15:18, Konrad Dybcio wrote: > From: Konrad Dybcio <quic_kdybcio@quicinc.com> > > The X1E80100, just like its predecessors, has a Multiport controller. > This time around, 2 HS (eUSB) and 2 SS PHYs are attached. > > Document it. > > Signed-off-by: Konrad Dybcio <quic_kdybcio@quicinc.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-09 13:18 [PATCH 0/2] X1E80100 multiport USB controller Konrad Dybcio 2024-08-09 13:18 ` [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller Konrad Dybcio @ 2024-08-09 13:18 ` Konrad Dybcio 2024-08-10 12:48 ` Krzysztof Kozlowski [not found] ` <21fffb71-d559-4973-8028-d9c9b9f67001@quicinc.com> 1 sibling, 2 replies; 11+ messages in thread From: Konrad Dybcio @ 2024-08-09 13:18 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson, Konrad Dybcio Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio, Konrad Dybcio X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs attached to it. It's commonly used for USB-A ports and internally routed devices. Configure it to support such functionality. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- arch/arm64/boot/dts/qcom/x1e80100.dtsi | 170 +++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi index 326283822aee..b6dd2f47341b 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi @@ -3842,6 +3842,90 @@ usb_2_hsphy: phy@88e0000 { status = "disabled"; }; + usb_mp_hsphy0: phy@88e1000 { + compatible = "qcom,x1e80100-snps-eusb2-phy", + "qcom,sm8550-snps-eusb2-phy"; + reg = <0 0x088e1000 0 0x154>; + #phy-cells = <0>; + + clocks = <&tcsr TCSR_USB3_MP0_CLKREF_EN>; + clock-names = "ref"; + + resets = <&gcc GCC_QUSB2PHY_HS0_MP_BCR>; + + status = "disabled"; + }; + + usb_mp_hsphy1: phy@88e2000 { + compatible = "qcom,x1e80100-snps-eusb2-phy", + "qcom,sm8550-snps-eusb2-phy"; + reg = <0 0x088e2000 0 0x154>; + #phy-cells = <0>; + + clocks = <&tcsr TCSR_USB3_MP1_CLKREF_EN>; + clock-names = "ref"; + + resets = <&gcc GCC_QUSB2PHY_HS1_MP_BCR>; + + status = "disabled"; + }; + + usb_mp_qmpphy0: phy@88e3000 { + compatible = "qcom,x1e80100-qmp-usb3-uni-phy"; + reg = <0 0x088e3000 0 0x2000>; + + clocks = <&gcc GCC_USB3_MP_PHY_AUX_CLK>, + <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_USB3_MP_PHY_COM_AUX_CLK>, + <&gcc GCC_USB3_MP_PHY_PIPE_0_CLK>; + clock-names = "aux", + "ref", + "com_aux", + "pipe"; + + resets = <&gcc GCC_USB3_UNIPHY_MP0_BCR>, + <&gcc GCC_USB3UNIPHY_PHY_MP0_BCR>; + reset-names = "phy", + "phy_phy"; + + power-domains = <&gcc GCC_USB3_MP_SS0_PHY_GDSC>; + + #clock-cells = <0>; + clock-output-names = "usb_mp_phy0_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + + usb_mp_qmpphy1: phy@88e5000 { + compatible = "qcom,x1e80100-qmp-usb3-uni-phy"; + reg = <0 0x088e5000 0 0x2000>; + + clocks = <&gcc GCC_USB3_MP_PHY_AUX_CLK>, + <&rpmhcc RPMH_CXO_CLK>, + <&gcc GCC_USB3_MP_PHY_COM_AUX_CLK>, + <&gcc GCC_USB3_MP_PHY_PIPE_1_CLK>; + clock-names = "aux", + "ref", + "com_aux", + "pipe"; + + resets = <&gcc GCC_USB3_UNIPHY_MP1_BCR>, + <&gcc GCC_USB3UNIPHY_PHY_MP1_BCR>; + reset-names = "phy", + "phy_phy"; + + power-domains = <&gcc GCC_USB3_MP_SS1_PHY_GDSC>; + + #clock-cells = <0>; + clock-output-names = "usb_mp_phy1_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + usb_1_ss2: usb@a0f8800 { compatible = "qcom,x1e80100-dwc3", "qcom,dwc3"; reg = <0 0x0a0f8800 0 0x400>; @@ -4016,6 +4100,92 @@ usb_2_dwc3_hs: endpoint { }; }; + usb_mp: usb@a4f8800 { + compatible = "qcom,x1e80100-dwc3-mp", "qcom,dwc3"; + reg = <0 0x0a4f8800 0 0x400>; + + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, + <&gcc GCC_USB30_MP_MASTER_CLK>, + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, + <&gcc GCC_USB30_MP_SLEEP_CLK>, + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, + <&gcc GCC_AGGRE_NOC_USB_NORTH_AXI_CLK>, + <&gcc GCC_AGGRE_NOC_USB_SOUTH_AXI_CLK>, + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; + clock-names = "cfg_noc", + "core", + "iface", + "sleep", + "mock_utmi", + "noc_aggr", + "noc_aggr_north", + "noc_aggr_south", + "noc_sys"; + + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, + <&gcc GCC_USB30_MP_MASTER_CLK>; + assigned-clock-rates = <19200000>, + <200000000>; + + interrupts-extended = <&intc GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>, + <&pdc 52 IRQ_TYPE_EDGE_BOTH>, + <&pdc 51 IRQ_TYPE_EDGE_BOTH>, + <&pdc 54 IRQ_TYPE_EDGE_BOTH>, + <&pdc 53 IRQ_TYPE_EDGE_BOTH>, + <&pdc 55 IRQ_TYPE_LEVEL_HIGH>, + <&pdc 56 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "pwr_event_1", "pwr_event_2", + "hs_phy_1", "hs_phy_2", + "dp_hs_phy_1", "dm_hs_phy_1", + "dp_hs_phy_2", "dm_hs_phy_2", + "ss_phy_1", "ss_phy_2"; + + power-domains = <&gcc GCC_USB30_MP_GDSC>; + required-opps = <&rpmhpd_opp_nom>; + + resets = <&gcc GCC_USB30_MP_BCR>; + + interconnects = <&usb_north_anoc MASTER_USB3_MP QCOM_ICC_TAG_ALWAYS + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS + &config_noc SLAVE_USB3_MP QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "usb-ddr", + "apps-usb"; + + wakeup-source; + + #address-cells = <2>; + #size-cells = <2>; + ranges; + + status = "disabled"; + + usb_mp_dwc3: usb@a400000 { + compatible = "snps,dwc3"; + reg = <0 0x0a400000 0 0xcd00>; + + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>; + + iommus = <&apps_smmu 0x1400 0x0>; + + phys = <&usb_mp_hsphy0>, <&usb_mp_qmpphy0>, + <&usb_mp_hsphy1>, <&usb_mp_qmpphy1>; + phy-names = "usb2-0", "usb3-0", + "usb2-1", "usb3-1"; + dr_mode = "host"; + + snps,dis_u2_susphy_quirk; + snps,dis_enblslpm_quirk; + snps,usb3_lpm_capable; + + dma-coherent; + }; + }; + usb_1_ss0: usb@a6f8800 { compatible = "qcom,x1e80100-dwc3", "qcom,dwc3"; reg = <0 0x0a6f8800 0 0x400>; -- 2.46.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-09 13:18 ` [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller Konrad Dybcio @ 2024-08-10 12:48 ` Krzysztof Kozlowski 2024-08-13 14:09 ` Konrad Dybcio [not found] ` <21fffb71-d559-4973-8028-d9c9b9f67001@quicinc.com> 1 sibling, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2024-08-10 12:48 UTC (permalink / raw) To: Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 09/08/2024 15:18, Konrad Dybcio wrote: > X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs > attached to it. It's commonly used for USB-A ports and internally > routed devices. Configure it to support such functionality. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> You have from and SoB mismatch. This was sent some odd way, because both b4 and git send-email would produce correct From field. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-10 12:48 ` Krzysztof Kozlowski @ 2024-08-13 14:09 ` Konrad Dybcio 2024-08-13 14:35 ` Krzysztof Kozlowski 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2024-08-13 14:09 UTC (permalink / raw) To: Krzysztof Kozlowski, Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 10.08.2024 2:48 PM, Krzysztof Kozlowski wrote: > On 09/08/2024 15:18, Konrad Dybcio wrote: >> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >> attached to it. It's commonly used for USB-A ports and internally >> routed devices. Configure it to support such functionality. >> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > You have from and SoB mismatch. This was sent some odd way, because both > b4 and git send-email would produce correct From field. So, I'm: - sending from @kernel because it has a good email server - authoring from @quicinc because that's my employer - sending some older @linaro patches because I made them before switching jobs and it's only fair to do so like this (and I sometimes reply from @gmail because thunderbird works funnily) I noticed locally that if you switch emails & edit author too often, git gets confused and `git show` Author: / `git send` From: don't get updated properly, but if you do git format-patch, the resulting file has what you would expect.. Should I resend this? Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-13 14:09 ` Konrad Dybcio @ 2024-08-13 14:35 ` Krzysztof Kozlowski 2024-08-13 15:21 ` Konrad Dybcio 0 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2024-08-13 14:35 UTC (permalink / raw) To: Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 13/08/2024 16:09, Konrad Dybcio wrote: > On 10.08.2024 2:48 PM, Krzysztof Kozlowski wrote: >> On 09/08/2024 15:18, Konrad Dybcio wrote: >>> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >>> attached to it. It's commonly used for USB-A ports and internally >>> routed devices. Configure it to support such functionality. >>> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> >> You have from and SoB mismatch. This was sent some odd way, because both >> b4 and git send-email would produce correct From field. > > So, I'm: > > - sending from @kernel because it has a good email server > - authoring from @quicinc because that's my employer > - sending some older @linaro patches because I made them before > switching jobs and it's only fair to do so like this > (and I sometimes reply from @gmail because thunderbird works funnily) This is all fine, but you created commit with one identity and signed off with other. That's not fine. > > I noticed locally that if you switch emails & edit author too often, git > gets confused and `git show` Author: / `git send` From: don't get updated > properly, but if you do git format-patch, the resulting file has what you > would expect.. > > Should I resend this? Yes, so there will be proper From field with proper author matching at least one SoBs. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-13 14:35 ` Krzysztof Kozlowski @ 2024-08-13 15:21 ` Konrad Dybcio 0 siblings, 0 replies; 11+ messages in thread From: Konrad Dybcio @ 2024-08-13 15:21 UTC (permalink / raw) To: Krzysztof Kozlowski, Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 13.08.2024 4:35 PM, Krzysztof Kozlowski wrote: > On 13/08/2024 16:09, Konrad Dybcio wrote: >> On 10.08.2024 2:48 PM, Krzysztof Kozlowski wrote: >>> On 09/08/2024 15:18, Konrad Dybcio wrote: >>>> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >>>> attached to it. It's commonly used for USB-A ports and internally >>>> routed devices. Configure it to support such functionality. >>>> >>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> >>> You have from and SoB mismatch. This was sent some odd way, because both >>> b4 and git send-email would produce correct From field. >> >> So, I'm: >> >> - sending from @kernel because it has a good email server >> - authoring from @quicinc because that's my employer >> - sending some older @linaro patches because I made them before >> switching jobs and it's only fair to do so like this >> (and I sometimes reply from @gmail because thunderbird works funnily) > > This is all fine, but you created commit with one identity and signed > off with other. That's not fine. > >> >> I noticed locally that if you switch emails & edit author too often, git >> gets confused and `git show` Author: / `git send` From: don't get updated >> properly, but if you do git format-patch, the resulting file has what you >> would expect.. >> >> Should I resend this? > > Yes, so there will be proper From field with proper author matching at > least one SoBs. OK I know why it happens.. since my linaro address is in .mailmap, I can't do any new git operations with it.. Removing it "fixes" it.. but then b4 base-commit needs to be altered.. Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <21fffb71-d559-4973-8028-d9c9b9f67001@quicinc.com>]
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller [not found] ` <21fffb71-d559-4973-8028-d9c9b9f67001@quicinc.com> @ 2024-08-14 10:24 ` Konrad Dybcio 2024-08-14 11:56 ` Song Xue 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2024-08-14 10:24 UTC (permalink / raw) To: Song Xue, Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 14.08.2024 12:08 PM, Song Xue wrote: > > On 8/9/2024 9:18 PM, Konrad Dybcio wrote: >> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >> attached to it. It's commonly used for USB-A ports and internally >> routed devices. Configure it to support such functionality. >> >> Signed-off-by: Konrad Dybcio<konrad.dybcio@linaro.org> >> --- [...] >> + >> + phys = <&usb_mp_hsphy0>, <&usb_mp_qmpphy0>, >> + <&usb_mp_hsphy1>, <&usb_mp_qmpphy1>; >> + phy-names = "usb2-0", "usb3-0", >> + "usb2-1", "usb3-1"; >> + dr_mode = "host"; > > Why do we add the dr_mode definition in dtsi file rather than in corresponding board dts file? Could we follow the node "usb_1_ss1_dwc3" in x1e80100-crd.dtsi? That is because the MP controller is host-only and it doesn't make sense to ensure the OS of that in each board file separately. That's also how it's done on other platforms with a MP controller description. > > BTW, how do we verify the function of multiport controller?From my test on x1e80100-crd, the eusb6 which is from usb_mp_hsphy1 attaches the third-party repeater, do we need a new repeater node/driver to verify the function of eusb6? I have a X1E Surface Laptop 7 with a USB-A port with a NXP PTN3222 in front of it. Tested with a smoke test, with both SS and HS USB-A devices. Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-14 10:24 ` Konrad Dybcio @ 2024-08-14 11:56 ` Song Xue 2024-08-17 11:32 ` Konrad Dybcio 0 siblings, 1 reply; 11+ messages in thread From: Song Xue @ 2024-08-14 11:56 UTC (permalink / raw) To: Konrad Dybcio, Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 8/14/2024 6:24 PM, Konrad Dybcio wrote: > On 14.08.2024 12:08 PM, Song Xue wrote: >> >> On 8/9/2024 9:18 PM, Konrad Dybcio wrote: >>> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >>> attached to it. It's commonly used for USB-A ports and internally >>> routed devices. Configure it to support such functionality. >>> >>> Signed-off-by: Konrad Dybcio<konrad.dybcio@linaro.org> >>> --- > > [...] > >>> + >>> + phys = <&usb_mp_hsphy0>, <&usb_mp_qmpphy0>, >>> + <&usb_mp_hsphy1>, <&usb_mp_qmpphy1>; >>> + phy-names = "usb2-0", "usb3-0", >>> + "usb2-1", "usb3-1"; >>> + dr_mode = "host"; >> >> Why do we add the dr_mode definition in dtsi file rather than in corresponding board dts file? Could we follow the node "usb_1_ss1_dwc3" in x1e80100-crd.dtsi? > > That is because the MP controller is host-only and it doesn't make sense > to ensure the OS of that in each board file separately. That's also how > it's done on other platforms with a MP controller description. > >> >> BTW, how do we verify the function of multiport controller?From my test on x1e80100-crd, the eusb6 which is from usb_mp_hsphy1 attaches the third-party repeater, do we need a new repeater node/driver to verify the function of eusb6? > > I have a X1E Surface Laptop 7 with a USB-A port with a NXP PTN3222 in > front of it. Tested with a smoke test, with both SS and HS USB-A devices. > What is detailed information on smoke test. From my end, I also have two questions. 1. I found the usb_mp_hsphy1 is using the driver "phy-qcom-snps-eusb2". However, the driver requires a repeater node from DT. At present, we don't have the node or driver for NXP repeater and it is not working on eusb6 to detect the NXP repeater. So, is it possible for us to have complete function involving with MP DT and repeater node for CRD board, and then we push patches together? 2. The usb_mp_dwc3 node has four phys. When enabling the driver for the node, we must need enable all four phys in borad's DT. Howerver, if the board is only using one phy like eusb6, is it suitable to enable other three phys? Regards, Song Xue > Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller 2024-08-14 11:56 ` Song Xue @ 2024-08-17 11:32 ` Konrad Dybcio 0 siblings, 0 replies; 11+ messages in thread From: Konrad Dybcio @ 2024-08-17 11:32 UTC (permalink / raw) To: Song Xue, Konrad Dybcio, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Bjorn Andersson Cc: Marijn Suijten, linux-arm-msm, linux-usb, devicetree, linux-kernel, Krishna Kurapati, Konrad Dybcio On 14.08.2024 1:56 PM, Song Xue wrote: > > > On 8/14/2024 6:24 PM, Konrad Dybcio wrote: >> On 14.08.2024 12:08 PM, Song Xue wrote: >>> >>> On 8/9/2024 9:18 PM, Konrad Dybcio wrote: >>>> X1E80100 has a multiport controller with 2 HS (eUSB) and 2 SS PHYs >>>> attached to it. It's commonly used for USB-A ports and internally >>>> routed devices. Configure it to support such functionality. >>>> >>>> Signed-off-by: Konrad Dybcio<konrad.dybcio@linaro.org> >>>> --- >> >> [...] >> >>>> + >>>> + phys = <&usb_mp_hsphy0>, <&usb_mp_qmpphy0>, >>>> + <&usb_mp_hsphy1>, <&usb_mp_qmpphy1>; >>>> + phy-names = "usb2-0", "usb3-0", >>>> + "usb2-1", "usb3-1"; >>>> + dr_mode = "host"; >>> >>> Why do we add the dr_mode definition in dtsi file rather than in corresponding board dts file? Could we follow the node "usb_1_ss1_dwc3" in x1e80100-crd.dtsi? >> >> That is because the MP controller is host-only and it doesn't make sense >> to ensure the OS of that in each board file separately. That's also how >> it's done on other platforms with a MP controller description. >> >>> >>> BTW, how do we verify the function of multiport controller?From my test on x1e80100-crd, the eusb6 which is from usb_mp_hsphy1 attaches the third-party repeater, do we need a new repeater node/driver to verify the function of eusb6? >> >> I have a X1E Surface Laptop 7 with a USB-A port with a NXP PTN3222 in >> front of it. Tested with a smoke test, with both SS and HS USB-A devices. >> > What is detailed information on smoke test. > From my end, I also have two questions. > 1. I found the usb_mp_hsphy1 is using the driver "phy-qcom-snps-eusb2". However, the driver requires a repeater node from DT. At present, we don't have the node or driver for NXP repeater and it is not working on eusb6 to detect the NXP repeater. So, is it possible for us to have complete function involving with MP DT and repeater node for CRD board, and then we push patches together? I believe you're a bit confused about the upstreaming process. Describing hardware in Device Tree vs doing the same plus enabling it on some upstream board are of equal value, and this patch is very much in the spirit of "release early, release often". There's no need to delay patches that are correct within their own confinement (which they should be [1]) just so that the series is bigger. That may even be discouraged by some folks.. > 2. The usb_mp_dwc3 node has four phys. When enabling the driver for the node, we must need enable all four phys in borad's DT. Howerver, if the board is only using one phy like eusb6, is it suitable to enable other three phys? Yes, they will simply be registered, configured and since there won't be any interrupts (as the pins are N/C, it will not do much). But these PHYs are physically on the SoC regardless of them being connected, so I see no issue. [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes Konrad ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-08-17 11:32 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-09 13:18 [PATCH 0/2] X1E80100 multiport USB controller Konrad Dybcio
2024-08-09 13:18 ` [PATCH 1/2] dt-bindings: usb: qcom,dwc3: Document X1E80100 MP controller Konrad Dybcio
2024-08-10 12:47 ` Krzysztof Kozlowski
2024-08-09 13:18 ` [PATCH 2/2] arm64: dts: qcom: x1e80100: Add USB Multiport controller Konrad Dybcio
2024-08-10 12:48 ` Krzysztof Kozlowski
2024-08-13 14:09 ` Konrad Dybcio
2024-08-13 14:35 ` Krzysztof Kozlowski
2024-08-13 15:21 ` Konrad Dybcio
[not found] ` <21fffb71-d559-4973-8028-d9c9b9f67001@quicinc.com>
2024-08-14 10:24 ` Konrad Dybcio
2024-08-14 11:56 ` Song Xue
2024-08-17 11:32 ` Konrad Dybcio
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox