[PATCH 0/2] arm64: dts: qcom: sm8250: fix description of the Type-C signals

[PATCH 0/2] arm64: dts: qcom: sm8250: fix description of the Type-C signals

[Dmitry Baryshkov]

Rename the HS link between usb-c-connector and the DWC3 USB controller.
Add missing graph connection between the QMP PHY and DWC3 USB
controller.

Reported-by: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Dmitry Baryshkov (2):
      arm64: dts: qcom: sm8250: describe HS signals properly
      arm64: dts: qcom: sm8250: add a link between DWC3 and QMP PHY

 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts           |  8 ++++----
 .../boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi  |  8 ++++----
 arch/arm64/boot/dts/qcom/sm8250.dtsi               | 24 ++++++++++++++++++++--
 3 files changed, 30 insertions(+), 10 deletions(-)
---
base-commit: 226d3c72fcde130a99d760895ebdd20e78e02cb5
change-id: 20240322-typec-fix-sm8250-33c47a03a056

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

[PATCH 1/2] arm64: dts: qcom: sm8250: describe HS signals properly

[Dmitry Baryshkov]

The OF graph should describe physical signals. There is no 'role switch'
signal between Type-C connector and the DWC3 USB controller. Instead
there is a HighSpeed signal lane between DWC3 controller and the USB-C
connector. Rename endpoints in accordance to that (this follows the
example lead by other plaforms, including QRB2210 RB1, QRB4210 RB2 and
all PMIC GLINK platforms).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qrb5165-rb5.dts                 | 8 ++++----
 arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi | 8 ++++----
 arch/arm64/boot/dts/qcom/sm8250.dtsi                     | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index cd0db4f31d4a..44239d1e57e9 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -1356,8 +1356,8 @@ &usb_1_dwc3 {
 	usb-role-switch;
 };
 
-&usb_1_role_switch_out {
-	remote-endpoint = <&pm8150b_role_switch_in>;
+&usb_1_dwc3_hs_out {
+	remote-endpoint = <&pm8150b_hs_in>;
 };
 
 &usb_1_hsphy {
@@ -1465,8 +1465,8 @@ ports {
 
 			port@0 {
 				reg = <0>;
-				pm8150b_role_switch_in: endpoint {
-					remote-endpoint = <&usb_1_role_switch_out>;
+				pm8150b_hs_in: endpoint {
+					remote-endpoint = <&usb_1_dwc3_hs_out>;
 				};
 			};
 
diff --git a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
index 6f54f50a70b0..99915a2b97e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
@@ -652,8 +652,8 @@ ports {
 			port@0 {
 				reg = <0>;
 
-				pm8150b_role_switch_in: endpoint {
-					remote-endpoint = <&usb_1_role_switch_out>;
+				pm8150b_hs_in: endpoint {
+					remote-endpoint = <&usb_1_dwc3_hs_out>;
 				};
 			};
 		};
@@ -716,8 +716,8 @@ &usb_1_hsphy {
 	status = "okay";
 };
 
-&usb_1_role_switch_out {
-	remote-endpoint = <&pm8150b_role_switch_in>;
+&usb_1_dwc3_hs_out {
+	remote-endpoint = <&pm8150b_hs_in>;
 };
 
 &ufs_mem_hc {
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 39bd8f0eba1e..d57039a4c3aa 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -4196,7 +4196,7 @@ usb_1_dwc3: usb@a600000 {
 				phy-names = "usb2-phy", "usb3-phy";
 
 				port {
-					usb_1_role_switch_out: endpoint {};
+					usb_1_dwc3_hs_out: endpoint {};
 				};
 			};
 		};

-- 
2.39.2

[PATCH 2/2] arm64: dts: qcom: sm8250: add a link between DWC3 and QMP PHY

[Dmitry Baryshkov]

The SuperSpeed signals originate from the DWC3 host controller and then
are routed through the Combo QMP PHY, where they are multiplexed with
the DisplayPort signals. Add corresponding OF graph link.

Reported-by: Luca Weiss <luca.weiss@fairphone.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index d57039a4c3aa..e551e733ab94 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -3917,6 +3917,10 @@ port@0 {
 
 				port@1 {
 					reg = <1>;
+
+					usb_1_qmpphy_usb_ss_in: endpoint {
+						remote-endpoint = <&usb_1_dwc3_ss_out>;
+					};
 				};
 
 				port@2 {
@@ -4195,8 +4199,24 @@ usb_1_dwc3: usb@a600000 {
 				phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
 				phy-names = "usb2-phy", "usb3-phy";
 
-				port {
-					usb_1_dwc3_hs_out: endpoint {};
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						usb_1_dwc3_hs_out: endpoint {
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						usb_1_dwc3_ss_out: endpoint {
+							remote-endpoint = <&usb_1_qmpphy_usb_ss_in>;
+						};
+					};
 				};
 			};
 		};

-- 
2.39.2

Re: [PATCH 1/2] arm64: dts: qcom: sm8250: describe HS signals properly

[Bryan O'Donoghue]

On 22/03/2024 11:58, Dmitry Baryshkov wrote:
> The OF graph should describe physical signals. There is no 'role switch'
> signal between Type-C connector and the DWC3 USB controller. Instead
> there is a HighSpeed signal lane between DWC3 controller and the USB-C
> connector. Rename endpoints in accordance to that (this follows the
> example lead by other plaforms, including QRB2210 RB1, QRB4210 RB2 and
> all PMIC GLINK platforms).
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Hmm

I think if you are going to change the name here, you should also do so here

grep role_switch arch/arm64/* -r | wc -l
54

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod

Re: [PATCH 1/2] arm64: dts: qcom: sm8250: describe HS signals properly

[Dmitry Baryshkov]

On Fri, 22 Mar 2024 at 14:03, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 22/03/2024 11:58, Dmitry Baryshkov wrote:
> > The OF graph should describe physical signals. There is no 'role switch'
> > signal between Type-C connector and the DWC3 USB controller. Instead
> > there is a HighSpeed signal lane between DWC3 controller and the USB-C
> > connector. Rename endpoints in accordance to that (this follows the
> > example lead by other plaforms, including QRB2210 RB1, QRB4210 RB2 and
> > all PMIC GLINK platforms).
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Hmm
>
> I think if you are going to change the name here, you should also do so here
>
> grep role_switch arch/arm64/* -r | wc -l
> 54

Oh, my. I'll take a look at the qcom platforms for v2. And funny
enough sc8180x even uses old bindings for the QMP PHY.

> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH 2/2] arm64: dts: qcom: sm8250: add a link between DWC3 and QMP PHY

[Bryan O'Donoghue]

On 22/03/2024 11:58, Dmitry Baryshkov wrote:
> The SuperSpeed signals originate from the DWC3 host controller and then
> are routed through the Combo QMP PHY, where they are multiplexed with
> the DisplayPort signals. Add corresponding OF graph link.
> 
> Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index d57039a4c3aa..e551e733ab94 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3917,6 +3917,10 @@ port@0 {
>   
>   				port@1 {
>   					reg = <1>;
> +
> +					usb_1_qmpphy_usb_ss_in: endpoint {
> +						remote-endpoint = <&usb_1_dwc3_ss_out>;
> +					};
>   				};
>   
>   				port@2 {
> @@ -4195,8 +4199,24 @@ usb_1_dwc3: usb@a600000 {
>   				phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
>   				phy-names = "usb2-phy", "usb3-phy";
>   
> -				port {
> -					usb_1_dwc3_hs_out: endpoint {};
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +
> +						usb_1_dwc3_hs_out: endpoint {
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +
> +						usb_1_dwc3_ss_out: endpoint {
> +							remote-endpoint = <&usb_1_qmpphy_usb_ss_in>;
> +						};
> +					};
>   				};
>   			};
>   		};
> 

I think these should go into platform definitions, there's nothing at 
the SoC level that imposes the port constraint.

---
bod

Re: [PATCH 2/2] arm64: dts: qcom: sm8250: add a link between DWC3 and QMP PHY

[Dmitry Baryshkov]

On Fri, 22 Mar 2024 at 14:11, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 22/03/2024 11:58, Dmitry Baryshkov wrote:
> > The SuperSpeed signals originate from the DWC3 host controller and then
> > are routed through the Combo QMP PHY, where they are multiplexed with
> > the DisplayPort signals. Add corresponding OF graph link.
> >
> > Reported-by: Luca Weiss <luca.weiss@fairphone.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++--
> >   1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index d57039a4c3aa..e551e733ab94 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -3917,6 +3917,10 @@ port@0 {
> >
> >                               port@1 {
> >                                       reg = <1>;
> > +
> > +                                     usb_1_qmpphy_usb_ss_in: endpoint {
> > +                                             remote-endpoint = <&usb_1_dwc3_ss_out>;
> > +                                     };
> >                               };
> >
> >                               port@2 {
> > @@ -4195,8 +4199,24 @@ usb_1_dwc3: usb@a600000 {
> >                               phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
> >                               phy-names = "usb2-phy", "usb3-phy";
> >
> > -                             port {
> > -                                     usb_1_dwc3_hs_out: endpoint {};
> > +                             ports {
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     port@0 {
> > +                                             reg = <0>;
> > +
> > +                                             usb_1_dwc3_hs_out: endpoint {
> > +                                             };
> > +                                     };
> > +
> > +                                     port@1 {
> > +                                             reg = <1>;
> > +
> > +                                             usb_1_dwc3_ss_out: endpoint {
> > +                                                     remote-endpoint = <&usb_1_qmpphy_usb_ss_in>;
> > +                                             };
> > +                                     };
> >                               };
> >                       };
> >               };
> >
>
> I think these should go into platform definitions, there's nothing at
> the SoC level that imposes the port constraint.

The link between DWC3 and QMP PHY is fixed in the SoC, if I remember correctly.

-- 
With best wishes
Dmitry

Re: [PATCH 2/2] arm64: dts: qcom: sm8250: add a link between DWC3 and QMP PHY

[Konrad Dybcio]

On 22.03.2024 14:19, Dmitry Baryshkov wrote:
> On Fri, 22 Mar 2024 at 14:11, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 22/03/2024 11:58, Dmitry Baryshkov wrote:
>>> The SuperSpeed signals originate from the DWC3 host controller and then
>>> are routed through the Combo QMP PHY, where they are multiplexed with
>>> the DisplayPort signals. Add corresponding OF graph link.
>>>
>>> Reported-by: Luca Weiss <luca.weiss@fairphone.com>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 24 ++++++++++++++++++++++--
>>>   1 file changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>>> index d57039a4c3aa..e551e733ab94 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
>>> @@ -3917,6 +3917,10 @@ port@0 {
>>>
>>>                               port@1 {
>>>                                       reg = <1>;
>>> +
>>> +                                     usb_1_qmpphy_usb_ss_in: endpoint {
>>> +                                             remote-endpoint = <&usb_1_dwc3_ss_out>;
>>> +                                     };
>>>                               };
>>>
>>>                               port@2 {
>>> @@ -4195,8 +4199,24 @@ usb_1_dwc3: usb@a600000 {
>>>                               phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
>>>                               phy-names = "usb2-phy", "usb3-phy";
>>>
>>> -                             port {
>>> -                                     usb_1_dwc3_hs_out: endpoint {};
>>> +                             ports {
>>> +                                     #address-cells = <1>;
>>> +                                     #size-cells = <0>;
>>> +
>>> +                                     port@0 {
>>> +                                             reg = <0>;
>>> +
>>> +                                             usb_1_dwc3_hs_out: endpoint {
>>> +                                             };
>>> +                                     };
>>> +
>>> +                                     port@1 {
>>> +                                             reg = <1>;
>>> +
>>> +                                             usb_1_dwc3_ss_out: endpoint {
>>> +                                                     remote-endpoint = <&usb_1_qmpphy_usb_ss_in>;
>>> +                                             };
>>> +                                     };
>>>                               };
>>>                       };
>>>               };
>>>
>>
>> I think these should go into platform definitions, there's nothing at
>> the SoC level that imposes the port constraint.
> 
> The link between DWC3 and QMP PHY is fixed in the SoC, if I remember correctly.

Yes, I believe so too

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>