| Message ID | 20230425034010.3789376-7-quic_bjorande@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/7] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Add ports and orientation-switch | expand |
On 27.04.2023 15:27, Neil Armstrong wrote: > On 27/04/2023 01:33, Konrad Dybcio wrote: >> >> On 4/25/23 04:40, Bjorn Andersson wrote: >>> With support for the QMP combo phy to react to USB Type-C switch events, >>> introduce it as the next hop for the SuperSpeed lanes of the two USB >>> Type-C connectors, and connect the output of the DisplayPort controller >>> to the QMP combo phy. >>> >>> This allows the TCPM to perform orientation switching of both USB and >>> DisplayPort signals. >>> >>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 28 ++++++++++++++++--- >>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 34 +++++++++++++++++++++++ >>> 2 files changed, 58 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >>> index 547277924ea3..33c973661fa5 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts >>> @@ -64,7 +64,7 @@ port@1 { >>> reg = <1>; >>> pmic_glink_con0_ss: endpoint { >>> - remote-endpoint = <&mdss0_dp0_out>; >>> + remote-endpoint = <&usb_0_qmpphy_out>; >>> }; >>> }; >>> @@ -99,7 +99,7 @@ port@1 { >>> reg = <1>; >>> pmic_glink_con1_ss: endpoint { >>> - remote-endpoint = <&mdss0_dp1_out>; >>> + remote-endpoint = <&usb_1_qmpphy_out>; >>> }; >>> }; >>> @@ -412,7 +412,7 @@ &mdss0_dp0 { >>> &mdss0_dp0_out { >>> data-lanes = <0 1>; >>> - remote-endpoint = <&pmic_glink_con0_ss>; >>> + remote-endpoint = <&usb_0_qmpphy_dp_in>; >>> }; >>> &mdss0_dp1 { >>> @@ -421,7 +421,7 @@ &mdss0_dp1 { >>> &mdss0_dp1_out { >>> data-lanes = <0 1>; >>> - remote-endpoint = <&pmic_glink_con1_ss>; >>> + remote-endpoint = <&usb_1_qmpphy_dp_in>; >>> }; >>> &mdss0_dp3 { >>> @@ -670,9 +670,19 @@ &usb_0_qmpphy { >>> vdda-phy-supply = <&vreg_l9d>; >>> vdda-pll-supply = <&vreg_l4d>; >>> + orientation-switch; >> >> I believe this belongs in the SoC DTSI, as it's supported by >> the PHY block itself >> >> >> The rest seems to lgtm.. >> >> >> On a note, why did we end up placing pmic_glink in device >> DTs? It's already assumed that we're using the full Qualcomm >> stack as we use PAS for remoteprocs so I *think* we can always >> assume pmic_glink would be there! > > As we did on other board, I think because having pmic_glink depends > on the board firmware capabilities ? Boards without USB-C won't need/have > pmic_link right ? PMIC_GLINK takes care of USB-C, but also enables battmgr and friends Konrad > > Neil > >> >> Konrad >> >>> + >>> status = "okay"; >>> }; >>> +&usb_0_qmpphy_dp_in { >>> + remote-endpoint = <&mdss0_dp0_out>; >>> +}; >>> + >>> +&usb_0_qmpphy_out { >>> + remote-endpoint = <&pmic_glink_con0_ss>; >>> +}; >>> + >>> &usb_0_role_switch { >>> remote-endpoint = <&pmic_glink_con0_hs>; >>> }; >>> @@ -697,9 +707,19 @@ &usb_1_qmpphy { >>> vdda-phy-supply = <&vreg_l4b>; >>> vdda-pll-supply = <&vreg_l3b>; >>> + orientation-switch; >>> + >>> status = "okay"; >>> }; >>> +&usb_1_qmpphy_dp_in { >>> + remote-endpoint = <&mdss0_dp1_out>; >>> +}; >>> + >>> +&usb_1_qmpphy_out { >>> + remote-endpoint = <&pmic_glink_con1_ss>; >>> +}; >>> + >>> &usb_1_role_switch { >>> remote-endpoint = <&pmic_glink_con1_hs>; >>> }; >>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> index 0e691bb0120c..1eb3a295e8fa 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >>> @@ -3006,6 +3006,23 @@ usb_0_qmpphy: phy@88eb000 { >>> #phy-cells = <1>; >>> status = "disabled"; >>> + >>> + ports { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + >>> + usb_0_qmpphy_out: endpoint {}; >>> + }; >>> + >>> + port@1 { >>> + reg = <1>; >>> + >>> + usb_0_qmpphy_dp_in: endpoint {}; >>> + }; >>> + }; >>> }; >>> usb_1_hsphy: phy@8902000 { >>> @@ -3042,6 +3059,23 @@ usb_1_qmpphy: phy@8903000 { >>> #phy-cells = <1>; >>> status = "disabled"; >>> + >>> + ports { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + port@0 { >>> + reg = <0>; >>> + >>> + usb_1_qmpphy_out: endpoint {}; >>> + }; >>> + >>> + port@1 { >>> + reg = <1>; >>> + >>> + usb_1_qmpphy_dp_in: endpoint {}; >>> + }; >>> + }; >>> }; >>> mdss1_dp0_phy: phy@8909a00 { >
On Mon, Apr 24, 2023 at 08:40:09PM -0700, Bjorn Andersson wrote: > With support for the QMP combo phy to react to USB Type-C switch events, > introduce it as the next hop for the SuperSpeed lanes of the two USB > Type-C connectors, and connect the output of the DisplayPort controller > to the QMP combo phy. > > This allows the TCPM to perform orientation switching of both USB and > DisplayPort signals. > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 28 ++++++++++++++++--- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 34 +++++++++++++++++++++++ > 2 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > index 547277924ea3..33c973661fa5 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts > @@ -64,7 +64,7 @@ port@1 { > reg = <1>; > > pmic_glink_con0_ss: endpoint { > - remote-endpoint = <&mdss0_dp0_out>; > + remote-endpoint = <&usb_0_qmpphy_out>; > }; > }; > > @@ -99,7 +99,7 @@ port@1 { > reg = <1>; > > pmic_glink_con1_ss: endpoint { > - remote-endpoint = <&mdss0_dp1_out>; > + remote-endpoint = <&usb_1_qmpphy_out>; > }; > }; > > @@ -412,7 +412,7 @@ &mdss0_dp0 { > > &mdss0_dp0_out { > data-lanes = <0 1>; > - remote-endpoint = <&pmic_glink_con0_ss>; > + remote-endpoint = <&usb_0_qmpphy_dp_in>; > }; It's a bit hard to follow what going on when using place holder nodes from the dtsi like this (instead of describing all the ports directly in the board dts). IIRC we went a bit back and forth over this earlier and we already use this scheme for the display port controllers, so I guess this is the price we pay for being consistent. > &mdss0_dp1 { > @@ -421,7 +421,7 @@ &mdss0_dp1 { > > &mdss0_dp1_out { > data-lanes = <0 1>; > - remote-endpoint = <&pmic_glink_con1_ss>; > + remote-endpoint = <&usb_1_qmpphy_dp_in>; > }; > > &mdss0_dp3 { > @@ -670,9 +670,19 @@ &usb_0_qmpphy { > vdda-phy-supply = <&vreg_l9d>; > vdda-pll-supply = <&vreg_l4d>; > > + orientation-switch; > + > status = "okay"; > }; > > +&usb_0_qmpphy_dp_in { > + remote-endpoint = <&mdss0_dp0_out>; > +}; > + > +&usb_0_qmpphy_out { > + remote-endpoint = <&pmic_glink_con0_ss>; > +}; > + > &usb_0_role_switch { > remote-endpoint = <&pmic_glink_con0_hs>; > }; > @@ -697,9 +707,19 @@ &usb_1_qmpphy { > vdda-phy-supply = <&vreg_l4b>; > vdda-pll-supply = <&vreg_l3b>; > > + orientation-switch; > + > status = "okay"; > }; > > +&usb_1_qmpphy_dp_in { > + remote-endpoint = <&mdss0_dp1_out>; > +}; > + > +&usb_1_qmpphy_out { > + remote-endpoint = <&pmic_glink_con1_ss>; > +}; > + > &usb_1_role_switch { > remote-endpoint = <&pmic_glink_con1_hs>; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 0e691bb0120c..1eb3a295e8fa 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -3006,6 +3006,23 @@ usb_0_qmpphy: phy@88eb000 { > #phy-cells = <1>; > > status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + usb_0_qmpphy_out: endpoint {}; > + }; > + > + port@1 { > + reg = <1>; > + > + usb_0_qmpphy_dp_in: endpoint {}; > + }; > + }; > }; The binding describes three ports, where dp-in is port 2. Perhaps you don't need to describe ss-in yet, but shouldn't the port numbers match? Should some of these be described as required in the binding? Johan
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts index 547277924ea3..33c973661fa5 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts @@ -64,7 +64,7 @@ port@1 { reg = <1>; pmic_glink_con0_ss: endpoint { - remote-endpoint = <&mdss0_dp0_out>; + remote-endpoint = <&usb_0_qmpphy_out>; }; }; @@ -99,7 +99,7 @@ port@1 { reg = <1>; pmic_glink_con1_ss: endpoint { - remote-endpoint = <&mdss0_dp1_out>; + remote-endpoint = <&usb_1_qmpphy_out>; }; }; @@ -412,7 +412,7 @@ &mdss0_dp0 { &mdss0_dp0_out { data-lanes = <0 1>; - remote-endpoint = <&pmic_glink_con0_ss>; + remote-endpoint = <&usb_0_qmpphy_dp_in>; }; &mdss0_dp1 { @@ -421,7 +421,7 @@ &mdss0_dp1 { &mdss0_dp1_out { data-lanes = <0 1>; - remote-endpoint = <&pmic_glink_con1_ss>; + remote-endpoint = <&usb_1_qmpphy_dp_in>; }; &mdss0_dp3 { @@ -670,9 +670,19 @@ &usb_0_qmpphy { vdda-phy-supply = <&vreg_l9d>; vdda-pll-supply = <&vreg_l4d>; + orientation-switch; + status = "okay"; }; +&usb_0_qmpphy_dp_in { + remote-endpoint = <&mdss0_dp0_out>; +}; + +&usb_0_qmpphy_out { + remote-endpoint = <&pmic_glink_con0_ss>; +}; + &usb_0_role_switch { remote-endpoint = <&pmic_glink_con0_hs>; }; @@ -697,9 +707,19 @@ &usb_1_qmpphy { vdda-phy-supply = <&vreg_l4b>; vdda-pll-supply = <&vreg_l3b>; + orientation-switch; + status = "okay"; }; +&usb_1_qmpphy_dp_in { + remote-endpoint = <&mdss0_dp1_out>; +}; + +&usb_1_qmpphy_out { + remote-endpoint = <&pmic_glink_con1_ss>; +}; + &usb_1_role_switch { remote-endpoint = <&pmic_glink_con1_hs>; }; diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi index 0e691bb0120c..1eb3a295e8fa 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi @@ -3006,6 +3006,23 @@ usb_0_qmpphy: phy@88eb000 { #phy-cells = <1>; status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + usb_0_qmpphy_out: endpoint {}; + }; + + port@1 { + reg = <1>; + + usb_0_qmpphy_dp_in: endpoint {}; + }; + }; }; usb_1_hsphy: phy@8902000 { @@ -3042,6 +3059,23 @@ usb_1_qmpphy: phy@8903000 { #phy-cells = <1>; status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + usb_1_qmpphy_out: endpoint {}; + }; + + port@1 { + reg = <1>; + + usb_1_qmpphy_dp_in: endpoint {}; + }; + }; }; mdss1_dp0_phy: phy@8909a00 {
With support for the QMP combo phy to react to USB Type-C switch events, introduce it as the next hop for the SuperSpeed lanes of the two USB Type-C connectors, and connect the output of the DisplayPort controller to the QMP combo phy. This allows the TCPM to perform orientation switching of both USB and DisplayPort signals. Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 28 ++++++++++++++++--- arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 34 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-)