Re: [PATCH 6/7] arm64: dts: qcom: sc8280xp-crd: Add QMP to SuperSpeed graph

Bjorn Andersson
2023-04-27T19:48:26Z

On Thu, Apr 27, 2023 at 12:33:44AM +0100, 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
>

Sounds reasonable to me, will move it.

>
> 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!
>

In this particular case, sa8295p and sa8540p are derived from sc8280xp
dtsi, but those does not implement pmic_glink...
In most other cases I think your expectation would hold though.

Perhaps would be suitable to put the pmic_glink in sc8280xp.dtsi but
disable it, and then add connectors in the device.

Regards,
Bjorn

> 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 {