[PATCH 0/4] arm64: dts: qcom: sc8280xp: Enable external display

[PATCH 0/4] arm64: dts: qcom: sc8280xp: Enable external display

[Bjorn Andersson]

This introduces support for external display on the SC8280XP laptops.

Support for swapping orientation and changing the mode of the SuperSpeed
lanes is being implemented in the QMP driver, so at this point in time
this is not supported.

Bjorn Andersson (4):
  arm64: dts: qcom: sc8280xp: Add USB-C-related DP blocks
  arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink
  arm64: dts: qcom: sc8280xp-x13s: Enable external display
  arm64: defconfig: Enable DisplayPort on SC8280XP laptops

 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts     | 191 +++++++++++++++++-
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 191 +++++++++++++++++-
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 170 +++++++++++++++-
 arch/arm64/configs/defconfig                  |   2 +
 4 files changed, 546 insertions(+), 8 deletions(-)

-- 
2.39.1

[PATCH 1/4] arm64: dts: qcom: sc8280xp: Add USB-C-related DP blocks

[Bjorn Andersson]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

Add the two DisplayPort controllers that are attached to QMP phys for
providing display output on USB Type-C.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 170 ++++++++++++++++++++++++-
 1 file changed, 166 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index dceb7eb3106b..fcd393444f47 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -3155,6 +3155,20 @@ ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
 
+					port@0 {
+						reg = <0>;
+						mdss0_intf0_out: endpoint {
+							remote-endpoint = <&mdss0_dp0_in>;
+						};
+					};
+
+					port@4 {
+						reg = <4>;
+						mdss0_intf4_out: endpoint {
+							remote-endpoint = <&mdss0_dp1_in>;
+						};
+					};
+
 					port@5 {
 						reg = <5>;
 						mdss0_intf5_out: endpoint {
@@ -3199,6 +3213,154 @@ opp-600000000 {
 				};
 			};
 
+			mdss0_dp0: displayport-controller@ae90000 {
+				compatible = "qcom,sc8280xp-dp";
+				reg = <0 0xae90000 0 0x200>,
+				      <0 0xae90200 0 0x200>,
+				      <0 0xae90400 0 0x600>,
+				      <0 0xae91000 0 0x400>,
+				      <0 0xae91400 0 0x400>;
+				interrupt-parent = <&mdss0>;
+				interrupts = <12>;
+				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX0_AUX_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
+				clock-names = "core_iface", "core_aux",
+					      "ctrl_link",
+					      "ctrl_link_iface", "stream_pixel";
+
+				assigned-clocks = <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
+						  <&dispcc0 DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
+				assigned-clock-parents = <&usb_0_qmpphy QMP_USB43DP_DP_LINK_CLK>,
+							 <&usb_0_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>;
+
+				phys = <&usb_0_qmpphy QMP_USB43DP_DP_PHY>;
+				phy-names = "dp";
+
+				#sound-dai-cells = <0>;
+
+				operating-points-v2 = <&mdss0_dp0_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_CX>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						mdss0_dp0_in: endpoint {
+							remote-endpoint = <&mdss0_intf0_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+
+				mdss0_dp0_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-160000000 {
+						opp-hz = /bits/ 64 <160000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-270000000 {
+						opp-hz = /bits/ 64 <270000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-540000000 {
+						opp-hz = /bits/ 64 <540000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+
+					opp-810000000 {
+						opp-hz = /bits/ 64 <810000000>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+				};
+			};
+
+			mdss0_dp1: displayport-controller@ae98000 {
+				compatible = "qcom,sc8280xp-dp";
+				reg = <0 0xae98000 0 0x200>,
+				      <0 0xae98200 0 0x200>,
+				      <0 0xae98400 0 0x600>,
+				      <0 0xae99000 0 0x400>,
+				      <0 0xae99400 0 0x400>;
+				interrupt-parent = <&mdss0>;
+				interrupts = <13>;
+				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX1_AUX_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_INTF_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_DPTX1_PIXEL0_CLK>;
+				clock-names = "core_iface", "core_aux",
+					      "ctrl_link",
+					      "ctrl_link_iface", "stream_pixel";
+
+				assigned-clocks = <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_CLK_SRC>,
+						  <&dispcc0 DISP_CC_MDSS_DPTX1_PIXEL0_CLK_SRC>;
+				assigned-clock-parents = <&usb_1_qmpphy QMP_USB43DP_DP_LINK_CLK>,
+							 <&usb_1_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>;
+
+				phys = <&usb_1_qmpphy QMP_USB43DP_DP_PHY>;
+				phy-names = "dp";
+
+				#sound-dai-cells = <0>;
+
+				operating-points-v2 = <&mdss0_dp1_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_CX>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						mdss0_dp1_in: endpoint {
+							remote-endpoint = <&mdss0_intf4_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+
+				mdss0_dp1_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-160000000 {
+						opp-hz = /bits/ 64 <160000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-270000000 {
+						opp-hz = /bits/ 64 <270000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-540000000 {
+						opp-hz = /bits/ 64 <540000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+
+					opp-810000000 {
+						opp-hz = /bits/ 64 <810000000>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+				};
+			};
+
 			mdss0_dp2: displayport-controller@ae9a000 {
 				compatible = "qcom,sc8280xp-dp";
 				reg = <0 0xae9a000 0 0x200>,
@@ -3387,10 +3549,10 @@ dispcc0: clock-controller@af00000 {
 			clocks = <&gcc GCC_DISP_AHB_CLK>,
 				 <&rpmhcc RPMH_CXO_CLK>,
 				 <&sleep_clk>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>,
+				 <&usb_0_qmpphy QMP_USB43DP_DP_LINK_CLK>,
+				 <&usb_0_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>,
+				 <&usb_1_qmpphy QMP_USB43DP_DP_LINK_CLK>,
+				 <&usb_1_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>,
 				 <&mdss0_dp2_phy 0>,
 				 <&mdss0_dp2_phy 1>,
 				 <&mdss0_dp3_phy 0>,
-- 
2.39.1

[PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Bjorn Andersson]

From: Bjorn Andersson <bjorn.andersson@linaro.org>

The SC8280XP CRD control over battery management and its two USB Type-C
port using pmic_glink and two GPIO-based SBU muxes.

Enable the two DisplayPort instances, GPIO SBU mux instance and
pmic_glink with the two connectors on the CRD.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 191 +++++++++++++++++++++-
 1 file changed, 189 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 3f116a879e22..35b63c3962f0 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -36,6 +36,77 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	pmic-glink {
+		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		connector@0 {
+			compatible = "usb-c-connector";
+			reg = <0>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					pmic_glink_con0_hs: endpoint {
+						remote-endpoint = <&usb_0_role_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					pmic_glink_con0_ss: endpoint {
+						remote-endpoint = <&mdss0_dp0_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					pmic_glink_con0_sbu: endpoint {
+						remote-endpoint = <&usb0_sbu_mux>;
+					};
+				};
+			};
+		};
+
+		connector@1 {
+			compatible = "usb-c-connector";
+			reg = <1>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					pmic_glink_con1_hs: endpoint {
+						remote-endpoint = <&usb_1_role_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					pmic_glink_con1_ss: endpoint {
+						remote-endpoint = <&mdss0_dp1_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					pmic_glink_con1_sbu: endpoint {
+						remote-endpoint = <&usb1_sbu_mux>;
+					};
+				};
+			};
+		};
+	};
+
 	vreg_edp_3p3: regulator-edp-3p3 {
 		compatible = "regulator-fixed";
 
@@ -139,6 +210,46 @@ linux,cma {
 			linux,cma-default;
 		};
 	};
+
+	usb0-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb0_sbu_default>;
+
+		mode-switch;
+		orientation-switch;
+		svid = /bits/ 16 <0xff01>;
+
+		port {
+			usb0_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con0_sbu>;
+			};
+		};
+	};
+
+	usb1-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb1_sbu_default>;
+
+		mode-switch;
+		orientation-switch;
+		svid = /bits/ 16 <0xff01>;
+
+		port {
+			usb1_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con1_sbu>;
+			};
+		};
+	};
 };
 
 &apps_rsc {
@@ -262,6 +373,36 @@ &mdss0 {
 	status = "okay";
 };
 
+&mdss0_dp0 {
+	status = "okay";
+
+	data-lanes = <0 1>;
+
+	ports {
+		port@1 {
+			reg = <1>;
+			mdss0_dp0_out: endpoint {
+				remote-endpoint = <&pmic_glink_con0_ss>;
+			};
+		};
+	};
+};
+
+&mdss0_dp1 {
+	status = "okay";
+
+	data-lanes = <0 1>;
+
+	ports {
+		port@1 {
+			reg = <1>;
+			mdss0_dp1_out: endpoint {
+				remote-endpoint = <&pmic_glink_con1_ss>;
+			};
+		};
+	};
+};
+
 &mdss0_dp3 {
 	compatible = "qcom,sc8280xp-edp";
 	/delete-property/ #sound-dai-cells;
@@ -480,8 +621,13 @@ &usb_0 {
 };
 
 &usb_0_dwc3 {
-	/* TODO: Define USB-C connector properly */
 	dr_mode = "host";
+
+	port {
+		usb_0_role_switch: endpoint {
+			remote-endpoint = <&pmic_glink_con0_hs>;
+		};
+	};
 };
 
 &usb_0_hsphy {
@@ -504,8 +650,13 @@ &usb_1 {
 };
 
 &usb_1_dwc3 {
-	/* TODO: Define USB-C connector properly */
 	dr_mode = "host";
+
+	port {
+		usb_1_role_switch: endpoint {
+			remote-endpoint = <&pmic_glink_con1_hs>;
+		};
+	};
 };
 
 &usb_1_hsphy {
@@ -709,4 +860,40 @@ reset-n-pins {
 			drive-strength = <16>;
 		};
 	};
+
+	usb0_sbu_default: usb0-sbu-state {
+		oe-n-pins {
+			pins = "gpio101";
+			function = "gpio";
+		};
+
+		sel-pins {
+			pins = "gpio164";
+			function = "gpio";
+		};
+
+		mode-pins {
+			pins = "gpio167";
+			function = "gpio";
+			output-high;
+		};
+	};
+
+	usb1_sbu_default: usb1-sbu-state {
+		oe-n-pins {
+			pins = "gpio48";
+			function = "gpio";
+		};
+
+		sel-pins {
+			pins = "gpio47";
+			function = "gpio";
+		};
+
+		mode-pins {
+			pins = "gpio50";
+			function = "gpio";
+			output-high;
+		};
+	};
 };
-- 
2.39.1

[PATCH 3/4] arm64: dts: qcom: sc8280xp-x13s: Enable external display

[Bjorn Andersson]

Like on the CRD, add the necessary nodes to enable USB Type-C
altmode-based external display on the Lenovo ThinkPad X13s.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 191 +++++++++++++++++-
 1 file changed, 189 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 9f1456fde932..4ef15edd6c0f 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -77,6 +77,77 @@ switch-lid {
 		};
 	};
 
+	pmic-glink {
+		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		connector@0 {
+			compatible = "usb-c-connector";
+			reg = <0>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					pmic_glink_con0_hs: endpoint {
+						remote-endpoint = <&usb_0_role_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					pmic_glink_con0_ss: endpoint {
+						remote-endpoint = <&mdss0_dp0_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					pmic_glink_con0_sbu: endpoint {
+						remote-endpoint = <&usb0_sbu_mux>;
+					};
+				};
+			};
+		};
+
+		connector@1 {
+			compatible = "usb-c-connector";
+			reg = <1>;
+			power-role = "dual";
+			data-role = "dual";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@0 {
+					reg = <0>;
+					pmic_glink_con1_hs: endpoint {
+						remote-endpoint = <&usb_1_role_switch>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					pmic_glink_con1_ss: endpoint {
+						remote-endpoint = <&mdss0_dp1_out>;
+					};
+				};
+
+				port@2 {
+					reg = <2>;
+					pmic_glink_con1_sbu: endpoint {
+						remote-endpoint = <&usb1_sbu_mux>;
+					};
+				};
+			};
+		};
+	};
+
 	vreg_edp_3p3: regulator-edp-3p3 {
 		compatible = "regulator-fixed";
 
@@ -238,6 +309,46 @@ map1 {
 			};
 		};
 	};
+
+	usb0-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb0_sbu_default>;
+
+		mode-switch;
+		orientation-switch;
+		svid = /bits/ 16 <0xff01>;
+
+		port {
+			usb0_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con0_sbu>;
+			};
+		};
+	};
+
+	usb1-sbu-mux {
+		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
+
+		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb1_sbu_default>;
+
+		mode-switch;
+		orientation-switch;
+		svid = /bits/ 16 <0xff01>;
+
+		port {
+			usb1_sbu_mux: endpoint {
+				remote-endpoint = <&pmic_glink_con1_sbu>;
+			};
+		};
+	};
 };
 
 &apps_rsc {
@@ -377,6 +488,36 @@ &mdss0 {
 	status = "okay";
 };
 
+&mdss0_dp0 {
+	status = "okay";
+
+	data-lanes = <0 1>;
+
+	ports {
+		port@1 {
+			reg = <1>;
+			mdss0_dp0_out: endpoint {
+				remote-endpoint = <&pmic_glink_con0_ss>;
+			};
+		};
+	};
+};
+
+&mdss0_dp1 {
+	status = "okay";
+
+	data-lanes = <0 1>;
+
+	ports {
+		port@1 {
+			reg = <1>;
+			mdss0_dp1_out: endpoint {
+				remote-endpoint = <&pmic_glink_con1_ss>;
+			};
+		};
+	};
+};
+
 &mdss0_dp3 {
 	compatible = "qcom,sc8280xp-edp";
 
@@ -871,8 +1012,13 @@ &usb_0 {
 };
 
 &usb_0_dwc3 {
-	/* TODO: Define USB-C connector properly */
 	dr_mode = "host";
+
+	port {
+		usb_0_role_switch: endpoint {
+			remote-endpoint = <&pmic_glink_con0_hs>;
+		};
+	};
 };
 
 &usb_0_hsphy {
@@ -895,8 +1041,13 @@ &usb_1 {
 };
 
 &usb_1_dwc3 {
-	/* TODO: Define USB-C connector properly */
 	dr_mode = "host";
+
+	port {
+		usb_1_role_switch: endpoint {
+			remote-endpoint = <&pmic_glink_con1_hs>;
+		};
+	};
 };
 
 &usb_1_hsphy {
@@ -1147,6 +1298,42 @@ reset-n-pins {
 		};
 	};
 
+	usb0_sbu_default: usb0-sbu-state {
+		oe-n-pins {
+			pins = "gpio101";
+			function = "gpio";
+		};
+
+		sel-pins {
+			pins = "gpio164";
+			function = "gpio";
+		};
+
+		mode-pins {
+			pins = "gpio167";
+			function = "gpio";
+			output-high;
+		};
+	};
+
+	usb1_sbu_default: usb1-sbu-state {
+		oe-n-pins {
+			pins = "gpio48";
+			function = "gpio";
+		};
+
+		sel-pins {
+			pins = "gpio47";
+			function = "gpio";
+		};
+
+		mode-pins {
+			pins = "gpio50";
+			function = "gpio";
+			output-high;
+		};
+	};
+
 	wcd_default: wcd-default-state {
 		reset-pins {
 			pins = "gpio106";
-- 
2.39.1

[PATCH 4/4] arm64: defconfig: Enable DisplayPort on SC8280XP laptops

[Bjorn Andersson]

The QCOM_PMIC_GLINK implements the parts of a TCPM necessary for
negotiating DP altmode and the TYPEC_MUX_GPIO_SBU driver is used for
controlling connection and orientation switching of the SBU lanes in the
USB-C connector  Enable these to enable USB Type-C DisplayPort on
SC8280XP laptops.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0ec37aeec02f..e1d06a0ffc4e 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -973,6 +973,7 @@ CONFIG_TYPEC_TPS6598X=m
 CONFIG_TYPEC_HD3SS3220=m
 CONFIG_TYPEC_UCSI=m
 CONFIG_UCSI_CCG=m
+CONFIG_TYPEC_MUX_GPIO_SBU=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y
@@ -1208,6 +1209,7 @@ CONFIG_QCOM_CPR=y
 CONFIG_QCOM_GENI_SE=y
 CONFIG_QCOM_LLCC=m
 CONFIG_QCOM_OCMEM=m
+CONFIG_QCOM_PMIC_GLINK=m
 CONFIG_QCOM_RMTFS_MEM=m
 CONFIG_QCOM_RPMH=y
 CONFIG_QCOM_RPMHPD=y
-- 
2.39.1

Re: [PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Konrad Dybcio]

On 8.02.2023 04:46, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> The SC8280XP CRD control over battery management and its two USB Type-C
> port using pmic_glink and two GPIO-based SBU muxes.
> 
> Enable the two DisplayPort instances, GPIO SBU mux instance and
> pmic_glink with the two connectors on the CRD.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 191 +++++++++++++++++++++-
>  1 file changed, 189 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index 3f116a879e22..35b63c3962f0 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> @@ -36,6 +36,77 @@ chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
>  
> +	pmic-glink {
> +		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		connector@0 {
> +			compatible = "usb-c-connector";
> +			reg = <0>;
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
Add a newline between the last propreties and first subnodes, please.

> +				port@0 {
> +					reg = <0>;
> +					pmic_glink_con0_hs: endpoint {
> +						remote-endpoint = <&usb_0_role_switch>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +					pmic_glink_con0_ss: endpoint {
> +						remote-endpoint = <&mdss0_dp0_out>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +					pmic_glink_con0_sbu: endpoint {
> +						remote-endpoint = <&usb0_sbu_mux>;
> +					};
> +				};
> +			};
> +		};
> +
> +		connector@1 {
> +			compatible = "usb-c-connector";
> +			reg = <1>;
> +			power-role = "dual";
> +			data-role = "dual";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				port@0 {
> +					reg = <0>;
> +					pmic_glink_con1_hs: endpoint {
> +						remote-endpoint = <&usb_1_role_switch>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +					pmic_glink_con1_ss: endpoint {
> +						remote-endpoint = <&mdss0_dp1_out>;
> +					};
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +					pmic_glink_con1_sbu: endpoint {
> +						remote-endpoint = <&usb1_sbu_mux>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
>  	vreg_edp_3p3: regulator-edp-3p3 {
>  		compatible = "regulator-fixed";
>  
> @@ -139,6 +210,46 @@ linux,cma {
>  			linux,cma-default;
>  		};
>  	};
> +
> +	usb0-sbu-mux {
> +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
> +
> +		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
> +		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usb0_sbu_default>;
> +
> +		mode-switch;
> +		orientation-switch;
> +		svid = /bits/ 16 <0xff01>;
> +
> +		port {
> +			usb0_sbu_mux: endpoint {
> +				remote-endpoint = <&pmic_glink_con0_sbu>;
> +			};
> +		};
> +	};
> +
> +	usb1-sbu-mux {
> +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
> +
> +		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
> +		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&usb1_sbu_default>;
> +
> +		mode-switch;
> +		orientation-switch;
> +		svid = /bits/ 16 <0xff01>;
> +
> +		port {
> +			usb1_sbu_mux: endpoint {
> +				remote-endpoint = <&pmic_glink_con1_sbu>;
> +			};
> +		};
> +	};
>  };
>  
>  &apps_rsc {
> @@ -262,6 +373,36 @@ &mdss0 {
>  	status = "okay";
>  };
>  
> +&mdss0_dp0 {
> +	status = "okay";
> +
> +	data-lanes = <0 1>;
Status last; is this really only 2 lanes?

> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			mdss0_dp0_out: endpoint {
> +				remote-endpoint = <&pmic_glink_con0_ss>;
> +			};
> +		};
> +	};
> +};
> +
> +&mdss0_dp1 {
> +	status = "okay";
> +
> +	data-lanes = <0 1>;
Ditto

> +
> +	ports {
> +		port@1 {
> +			reg = <1>;
> +			mdss0_dp1_out: endpoint {
> +				remote-endpoint = <&pmic_glink_con1_ss>;
> +			};
> +		};
> +	};
> +};
> +
>  &mdss0_dp3 {
>  	compatible = "qcom,sc8280xp-edp";
>  	/delete-property/ #sound-dai-cells;
> @@ -480,8 +621,13 @@ &usb_0 {
>  };
>  
>  &usb_0_dwc3 {
> -	/* TODO: Define USB-C connector properly */
>  	dr_mode = "host";
> +
> +	port {
> +		usb_0_role_switch: endpoint {
> +			remote-endpoint = <&pmic_glink_con0_hs>;
> +		};
This should be defined in the SoC DTSI, it's a standard dwc3 binding
with usb HS / SS / SBU ports. Especially since we can feed the endpoint
from any device now, as pmic-glink should work everywhere.

Or /omit-if-no-ref/, I suppose.

> +	};
>  };
>  
>  &usb_0_hsphy {
> @@ -504,8 +650,13 @@ &usb_1 {
>  };
>  
>  &usb_1_dwc3 {
> -	/* TODO: Define USB-C connector properly */
>  	dr_mode = "host";
> +
> +	port {
> +		usb_1_role_switch: endpoint {
> +			remote-endpoint = <&pmic_glink_con1_hs>;
> +		};
> +	};
>  };
>  
>  &usb_1_hsphy {
> @@ -709,4 +860,40 @@ reset-n-pins {
>  			drive-strength = <16>;
>  		};
>  	};
> +
> +	usb0_sbu_default: usb0-sbu-state {
> +		oe-n-pins {
> +			pins = "gpio101";
> +			function = "gpio";
No drive-strength/bias/i/o?

Konrad
> +		};
> +
> +		sel-pins {
> +			pins = "gpio164";
> +			function = "gpio";
> +		};
> +
> +		mode-pins {
> +			pins = "gpio167";
> +			function = "gpio";
> +			output-high;
> +		};
> +	};
> +
> +	usb1_sbu_default: usb1-sbu-state {
> +		oe-n-pins {
> +			pins = "gpio48";
> +			function = "gpio";
> +		};
> +
> +		sel-pins {
> +			pins = "gpio47";
> +			function = "gpio";
> +		};
> +
> +		mode-pins {
> +			pins = "gpio50";
> +			function = "gpio";
> +			output-high;
> +		};
> +	};
>  };

Re: [PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Bjorn Andersson]

On Wed, Feb 08, 2023 at 09:14:46AM +0100, Konrad Dybcio wrote:
> 
> 
> On 8.02.2023 04:46, Bjorn Andersson wrote:
> > From: Bjorn Andersson <bjorn.andersson@linaro.org>
> > 
> > The SC8280XP CRD control over battery management and its two USB Type-C
> > port using pmic_glink and two GPIO-based SBU muxes.
> > 
> > Enable the two DisplayPort instances, GPIO SBU mux instance and
> > pmic_glink with the two connectors on the CRD.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 191 +++++++++++++++++++++-
> >  1 file changed, 189 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > index 3f116a879e22..35b63c3962f0 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > @@ -36,6 +36,77 @@ chosen {
> >  		stdout-path = "serial0:115200n8";
> >  	};
> >  
> > +	pmic-glink {
> > +		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
> > +
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		connector@0 {
> > +			compatible = "usb-c-connector";
> > +			reg = <0>;
> > +			power-role = "dual";
> > +			data-role = "dual";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> Add a newline between the last propreties and first subnodes, please.
> 
> > +				port@0 {
> > +					reg = <0>;
> > +					pmic_glink_con0_hs: endpoint {
> > +						remote-endpoint = <&usb_0_role_switch>;
> > +					};
> > +				};
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +					pmic_glink_con0_ss: endpoint {
> > +						remote-endpoint = <&mdss0_dp0_out>;
> > +					};
> > +				};
> > +
> > +				port@2 {
> > +					reg = <2>;
> > +					pmic_glink_con0_sbu: endpoint {
> > +						remote-endpoint = <&usb0_sbu_mux>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +
> > +		connector@1 {
> > +			compatible = "usb-c-connector";
> > +			reg = <1>;
> > +			power-role = "dual";
> > +			data-role = "dual";
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +				port@0 {
> > +					reg = <0>;
> > +					pmic_glink_con1_hs: endpoint {
> > +						remote-endpoint = <&usb_1_role_switch>;
> > +					};
> > +				};
> > +
> > +				port@1 {
> > +					reg = <1>;
> > +					pmic_glink_con1_ss: endpoint {
> > +						remote-endpoint = <&mdss0_dp1_out>;
> > +					};
> > +				};
> > +
> > +				port@2 {
> > +					reg = <2>;
> > +					pmic_glink_con1_sbu: endpoint {
> > +						remote-endpoint = <&usb1_sbu_mux>;
> > +					};
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> >  	vreg_edp_3p3: regulator-edp-3p3 {
> >  		compatible = "regulator-fixed";
> >  
> > @@ -139,6 +210,46 @@ linux,cma {
> >  			linux,cma-default;
> >  		};
> >  	};
> > +
> > +	usb0-sbu-mux {
> > +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
> > +
> > +		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
> > +		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&usb0_sbu_default>;
> > +
> > +		mode-switch;
> > +		orientation-switch;
> > +		svid = /bits/ 16 <0xff01>;
> > +
> > +		port {
> > +			usb0_sbu_mux: endpoint {
> > +				remote-endpoint = <&pmic_glink_con0_sbu>;
> > +			};
> > +		};
> > +	};
> > +
> > +	usb1-sbu-mux {
> > +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
> > +
> > +		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
> > +		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&usb1_sbu_default>;
> > +
> > +		mode-switch;
> > +		orientation-switch;
> > +		svid = /bits/ 16 <0xff01>;
> > +
> > +		port {
> > +			usb1_sbu_mux: endpoint {
> > +				remote-endpoint = <&pmic_glink_con1_sbu>;
> > +			};
> > +		};
> > +	};
> >  };
> >  
> >  &apps_rsc {
> > @@ -262,6 +373,36 @@ &mdss0 {
> >  	status = "okay";
> >  };
> >  
> > +&mdss0_dp0 {
> > +	status = "okay";
> > +
> > +	data-lanes = <0 1>;
> Status last; is this really only 2 lanes?
> 
> > +
> > +	ports {
> > +		port@1 {
> > +			reg = <1>;
> > +			mdss0_dp0_out: endpoint {
> > +				remote-endpoint = <&pmic_glink_con0_ss>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&mdss0_dp1 {
> > +	status = "okay";
> > +
> > +	data-lanes = <0 1>;
> Ditto
> 
> > +
> > +	ports {
> > +		port@1 {
> > +			reg = <1>;
> > +			mdss0_dp1_out: endpoint {
> > +				remote-endpoint = <&pmic_glink_con1_ss>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> >  &mdss0_dp3 {
> >  	compatible = "qcom,sc8280xp-edp";
> >  	/delete-property/ #sound-dai-cells;
> > @@ -480,8 +621,13 @@ &usb_0 {
> >  };
> >  
> >  &usb_0_dwc3 {
> > -	/* TODO: Define USB-C connector properly */
> >  	dr_mode = "host";
> > +
> > +	port {
> > +		usb_0_role_switch: endpoint {
> > +			remote-endpoint = <&pmic_glink_con0_hs>;
> > +		};
> This should be defined in the SoC DTSI, it's a standard dwc3 binding
> with usb HS / SS / SBU ports. Especially since we can feed the endpoint
> from any device now, as pmic-glink should work everywhere.
> 

The sa8295p/sa8540p boards, derived from sc8280xp does not implement
pmic_glink, so it seems moving this to the soc.dtsi would be messy.

> Or /omit-if-no-ref/, I suppose.
> 

Or you're saying I should put the skeleton of the port definition in the
soc.dtsi and then fill it out the remote-endpoint here; and mark it
omit-if-no-ref to avoid binding warnings?

> > +	};
> >  };
> >  
> >  &usb_0_hsphy {
> > @@ -504,8 +650,13 @@ &usb_1 {
> >  };
> >  
> >  &usb_1_dwc3 {
> > -	/* TODO: Define USB-C connector properly */
> >  	dr_mode = "host";
> > +
> > +	port {
> > +		usb_1_role_switch: endpoint {
> > +			remote-endpoint = <&pmic_glink_con1_hs>;
> > +		};
> > +	};
> >  };
> >  
> >  &usb_1_hsphy {
> > @@ -709,4 +860,40 @@ reset-n-pins {
> >  			drive-strength = <16>;
> >  		};
> >  	};
> > +
> > +	usb0_sbu_default: usb0-sbu-state {
> > +		oe-n-pins {
> > +			pins = "gpio101";
> > +			function = "gpio";
> No drive-strength/bias/i/o?
> 

Seems like a reasonable ask...

Thanks,
Bjorn

> Konrad
> > +		};
> > +
> > +		sel-pins {
> > +			pins = "gpio164";
> > +			function = "gpio";
> > +		};
> > +
> > +		mode-pins {
> > +			pins = "gpio167";
> > +			function = "gpio";
> > +			output-high;
> > +		};
> > +	};
> > +
> > +	usb1_sbu_default: usb1-sbu-state {
> > +		oe-n-pins {
> > +			pins = "gpio48";
> > +			function = "gpio";
> > +		};
> > +
> > +		sel-pins {
> > +			pins = "gpio47";
> > +			function = "gpio";
> > +		};
> > +
> > +		mode-pins {
> > +			pins = "gpio50";
> > +			function = "gpio";
> > +			output-high;
> > +		};
> > +	};
> >  };

Re: [PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Konrad Dybcio]

On 8.02.2023 16:57, Bjorn Andersson wrote:
> On Wed, Feb 08, 2023 at 09:14:46AM +0100, Konrad Dybcio wrote:
>>
>>
>> On 8.02.2023 04:46, Bjorn Andersson wrote:
>>> From: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>
>>> The SC8280XP CRD control over battery management and its two USB Type-C
>>> port using pmic_glink and two GPIO-based SBU muxes.
>>>
>>> Enable the two DisplayPort instances, GPIO SBU mux instance and
>>> pmic_glink with the two connectors on the CRD.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 191 +++++++++++++++++++++-
>>>  1 file changed, 189 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>> index 3f116a879e22..35b63c3962f0 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>> @@ -36,6 +36,77 @@ chosen {
>>>  		stdout-path = "serial0:115200n8";
>>>  	};
>>>  
>>> +	pmic-glink {
>>> +		compatible = "qcom,sc8280xp-pmic-glink", "qcom,pmic-glink";
>>> +
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		connector@0 {
>>> +			compatible = "usb-c-connector";
>>> +			reg = <0>;
>>> +			power-role = "dual";
>>> +			data-role = "dual";
>>> +
>>> +			ports {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>> Add a newline between the last propreties and first subnodes, please.
>>
>>> +				port@0 {
>>> +					reg = <0>;
>>> +					pmic_glink_con0_hs: endpoint {
>>> +						remote-endpoint = <&usb_0_role_switch>;
>>> +					};
>>> +				};
>>> +
>>> +				port@1 {
>>> +					reg = <1>;
>>> +					pmic_glink_con0_ss: endpoint {
>>> +						remote-endpoint = <&mdss0_dp0_out>;
>>> +					};
>>> +				};
>>> +
>>> +				port@2 {
>>> +					reg = <2>;
>>> +					pmic_glink_con0_sbu: endpoint {
>>> +						remote-endpoint = <&usb0_sbu_mux>;
>>> +					};
>>> +				};
>>> +			};
>>> +		};
>>> +
>>> +		connector@1 {
>>> +			compatible = "usb-c-connector";
>>> +			reg = <1>;
>>> +			power-role = "dual";
>>> +			data-role = "dual";
>>> +
>>> +			ports {
>>> +				#address-cells = <1>;
>>> +				#size-cells = <0>;
>>> +				port@0 {
>>> +					reg = <0>;
>>> +					pmic_glink_con1_hs: endpoint {
>>> +						remote-endpoint = <&usb_1_role_switch>;
>>> +					};
>>> +				};
>>> +
>>> +				port@1 {
>>> +					reg = <1>;
>>> +					pmic_glink_con1_ss: endpoint {
>>> +						remote-endpoint = <&mdss0_dp1_out>;
>>> +					};
>>> +				};
>>> +
>>> +				port@2 {
>>> +					reg = <2>;
>>> +					pmic_glink_con1_sbu: endpoint {
>>> +						remote-endpoint = <&usb1_sbu_mux>;
>>> +					};
>>> +				};
>>> +			};
>>> +		};
>>> +	};
>>> +
>>>  	vreg_edp_3p3: regulator-edp-3p3 {
>>>  		compatible = "regulator-fixed";
>>>  
>>> @@ -139,6 +210,46 @@ linux,cma {
>>>  			linux,cma-default;
>>>  		};
>>>  	};
>>> +
>>> +	usb0-sbu-mux {
>>> +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
>>> +
>>> +		enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
>>> +		select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
>>> +
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&usb0_sbu_default>;
>>> +
>>> +		mode-switch;
>>> +		orientation-switch;
>>> +		svid = /bits/ 16 <0xff01>;
>>> +
>>> +		port {
>>> +			usb0_sbu_mux: endpoint {
>>> +				remote-endpoint = <&pmic_glink_con0_sbu>;
>>> +			};
>>> +		};
>>> +	};
>>> +
>>> +	usb1-sbu-mux {
>>> +		compatible = "pericom,pi3usb102", "gpio-sbu-mux";
>>> +
>>> +		enable-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
>>> +		select-gpios = <&tlmm 47 GPIO_ACTIVE_HIGH>;
>>> +
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&usb1_sbu_default>;
>>> +
>>> +		mode-switch;
>>> +		orientation-switch;
>>> +		svid = /bits/ 16 <0xff01>;
>>> +
>>> +		port {
>>> +			usb1_sbu_mux: endpoint {
>>> +				remote-endpoint = <&pmic_glink_con1_sbu>;
>>> +			};
>>> +		};
>>> +	};
>>>  };
>>>  
>>>  &apps_rsc {
>>> @@ -262,6 +373,36 @@ &mdss0 {
>>>  	status = "okay";
>>>  };
>>>  
>>> +&mdss0_dp0 {
>>> +	status = "okay";
>>> +
>>> +	data-lanes = <0 1>;
>> Status last; is this really only 2 lanes?
>>
>>> +
>>> +	ports {
>>> +		port@1 {
>>> +			reg = <1>;
>>> +			mdss0_dp0_out: endpoint {
>>> +				remote-endpoint = <&pmic_glink_con0_ss>;
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>> +
>>> +&mdss0_dp1 {
>>> +	status = "okay";
>>> +
>>> +	data-lanes = <0 1>;
>> Ditto
>>
>>> +
>>> +	ports {
>>> +		port@1 {
>>> +			reg = <1>;
>>> +			mdss0_dp1_out: endpoint {
>>> +				remote-endpoint = <&pmic_glink_con1_ss>;
>>> +			};
>>> +		};
>>> +	};
>>> +};
>>> +
>>>  &mdss0_dp3 {
>>>  	compatible = "qcom,sc8280xp-edp";
>>>  	/delete-property/ #sound-dai-cells;
>>> @@ -480,8 +621,13 @@ &usb_0 {
>>>  };
>>>  
>>>  &usb_0_dwc3 {
>>> -	/* TODO: Define USB-C connector properly */
>>>  	dr_mode = "host";
>>> +
>>> +	port {
>>> +		usb_0_role_switch: endpoint {
>>> +			remote-endpoint = <&pmic_glink_con0_hs>;
>>> +		};
>> This should be defined in the SoC DTSI, it's a standard dwc3 binding
>> with usb HS / SS / SBU ports. Especially since we can feed the endpoint
>> from any device now, as pmic-glink should work everywhere.
>>
> 
> The sa8295p/sa8540p boards, derived from sc8280xp does not implement
> pmic_glink, so it seems moving this to the soc.dtsi would be messy.
> 
>> Or /omit-if-no-ref/, I suppose.
>>
> 
> Or you're saying I should put the skeleton of the port definition in the
> soc.dtsi and then fill it out the remote-endpoint here; and mark it
> omit-if-no-ref to avoid binding warnings?
I thought omit-if-no-ref actually omits the node before resolving phandles
that it references, but it seems to not be the case.. Still, we can define
the ports in the SoC DTSI and assign endpoints per-device (like we do with
DSI for example).

Konrad
> 
>>> +	};
>>>  };
>>>  
>>>  &usb_0_hsphy {
>>> @@ -504,8 +650,13 @@ &usb_1 {
>>>  };
>>>  
>>>  &usb_1_dwc3 {
>>> -	/* TODO: Define USB-C connector properly */
>>>  	dr_mode = "host";
>>> +
>>> +	port {
>>> +		usb_1_role_switch: endpoint {
>>> +			remote-endpoint = <&pmic_glink_con1_hs>;
>>> +		};
>>> +	};
>>>  };
>>>  
>>>  &usb_1_hsphy {
>>> @@ -709,4 +860,40 @@ reset-n-pins {
>>>  			drive-strength = <16>;
>>>  		};
>>>  	};
>>> +
>>> +	usb0_sbu_default: usb0-sbu-state {
>>> +		oe-n-pins {
>>> +			pins = "gpio101";
>>> +			function = "gpio";
>> No drive-strength/bias/i/o?
>>
> 
> Seems like a reasonable ask...
> 
> Thanks,
> Bjorn
> 
>> Konrad
>>> +		};
>>> +
>>> +		sel-pins {
>>> +			pins = "gpio164";
>>> +			function = "gpio";
>>> +		};
>>> +
>>> +		mode-pins {
>>> +			pins = "gpio167";
>>> +			function = "gpio";
>>> +			output-high;
>>> +		};
>>> +	};
>>> +
>>> +	usb1_sbu_default: usb1-sbu-state {
>>> +		oe-n-pins {
>>> +			pins = "gpio48";
>>> +			function = "gpio";
>>> +		};
>>> +
>>> +		sel-pins {
>>> +			pins = "gpio47";
>>> +			function = "gpio";
>>> +		};
>>> +
>>> +		mode-pins {
>>> +			pins = "gpio50";
>>> +			function = "gpio";
>>> +			output-high;
>>> +		};
>>> +	};
>>>  };

Re: [PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Bjorn Andersson]

On Wed, Feb 08, 2023 at 05:40:44PM +0100, Konrad Dybcio wrote:
> On 8.02.2023 16:57, Bjorn Andersson wrote:
> > On Wed, Feb 08, 2023 at 09:14:46AM +0100, Konrad Dybcio wrote:
> >> On 8.02.2023 04:46, Bjorn Andersson wrote:
[..]
> >>>  &usb_0_dwc3 {
> >>> -	/* TODO: Define USB-C connector properly */
> >>>  	dr_mode = "host";
> >>> +
> >>> +	port {
> >>> +		usb_0_role_switch: endpoint {
> >>> +			remote-endpoint = <&pmic_glink_con0_hs>;
> >>> +		};
> >> This should be defined in the SoC DTSI, it's a standard dwc3 binding
> >> with usb HS / SS / SBU ports. Especially since we can feed the endpoint
> >> from any device now, as pmic-glink should work everywhere.
> >>
> > 
> > The sa8295p/sa8540p boards, derived from sc8280xp does not implement
> > pmic_glink, so it seems moving this to the soc.dtsi would be messy.
> > 
> >> Or /omit-if-no-ref/, I suppose.
> >>
> > 
> > Or you're saying I should put the skeleton of the port definition in the
> > soc.dtsi and then fill it out the remote-endpoint here; and mark it
> > omit-if-no-ref to avoid binding warnings?
> I thought omit-if-no-ref actually omits the node before resolving phandles
> that it references, but it seems to not be the case.. Still, we can define
> the ports in the SoC DTSI and assign endpoints per-device (like we do with
> DSI for example).
> 

You mean move the lines:

port {
	usb_0_role_switch: endpoint {
	};
};

to sc8280xp.dtsi and then do:

&usb_0_role_switch {
	remote-endpoint = <&pmic_glink_con0_hs>;
};

here?

Regards,
Bjorn

Re: [PATCH 2/4] arm64: dts: qcom: sc8280xp-crd: Introduce pmic_glink

[Konrad Dybcio]

On 8.02.2023 19:00, Bjorn Andersson wrote:
> On Wed, Feb 08, 2023 at 05:40:44PM +0100, Konrad Dybcio wrote:
>> On 8.02.2023 16:57, Bjorn Andersson wrote:
>>> On Wed, Feb 08, 2023 at 09:14:46AM +0100, Konrad Dybcio wrote:
>>>> On 8.02.2023 04:46, Bjorn Andersson wrote:
> [..]
>>>>>  &usb_0_dwc3 {
>>>>> -	/* TODO: Define USB-C connector properly */
>>>>>  	dr_mode = "host";
>>>>> +
>>>>> +	port {
>>>>> +		usb_0_role_switch: endpoint {
>>>>> +			remote-endpoint = <&pmic_glink_con0_hs>;
>>>>> +		};
>>>> This should be defined in the SoC DTSI, it's a standard dwc3 binding
>>>> with usb HS / SS / SBU ports. Especially since we can feed the endpoint
>>>> from any device now, as pmic-glink should work everywhere.
>>>>
>>>
>>> The sa8295p/sa8540p boards, derived from sc8280xp does not implement
>>> pmic_glink, so it seems moving this to the soc.dtsi would be messy.
>>>
>>>> Or /omit-if-no-ref/, I suppose.
>>>>
>>>
>>> Or you're saying I should put the skeleton of the port definition in the
>>> soc.dtsi and then fill it out the remote-endpoint here; and mark it
>>> omit-if-no-ref to avoid binding warnings?
>> I thought omit-if-no-ref actually omits the node before resolving phandles
>> that it references, but it seems to not be the case.. Still, we can define
>> the ports in the SoC DTSI and assign endpoints per-device (like we do with
>> DSI for example).
>>
> 
> You mean move the lines:
> 
> port {
> 	usb_0_role_switch: endpoint {
> 	};
> };
> 
> to sc8280xp.dtsi and then do:
> 
> &usb_0_role_switch {
> 	remote-endpoint = <&pmic_glink_con0_hs>;
> };
> 
> here?
Correct

Konrad
> 
> Regards,
> Bjorn

Re: [PATCH 1/4] arm64: dts: qcom: sc8280xp: Add USB-C-related DP blocks

[Konrad Dybcio]

On 8.02.2023 04:46, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Add the two DisplayPort controllers that are attached to QMP phys for
> providing display output on USB Type-C.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 170 ++++++++++++++++++++++++-
>  1 file changed, 166 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index dceb7eb3106b..fcd393444f47 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -3155,6 +3155,20 @@ ports {
>  					#address-cells = <1>;
>  					#size-cells = <0>;
>  
> +					port@0 {
> +						reg = <0>;
Generally, there should be a newline between properties and child
nodes.

> +						mdss0_intf0_out: endpoint {
> +							remote-endpoint = <&mdss0_dp0_in>;
> +						};
> +					};
> +
> +					port@4 {
> +						reg = <4>;
> +						mdss0_intf4_out: endpoint {
> +							remote-endpoint = <&mdss0_dp1_in>;
> +						};
> +					};
> +
>  					port@5 {
>  						reg = <5>;
>  						mdss0_intf5_out: endpoint {
> @@ -3199,6 +3213,154 @@ opp-600000000 {
>  				};
>  			};
>  
> +			mdss0_dp0: displayport-controller@ae90000 {
> +				compatible = "qcom,sc8280xp-dp";
> +				reg = <0 0xae90000 0 0x200>,
> +				      <0 0xae90200 0 0x200>,
> +				      <0 0xae90400 0 0x600>,
> +				      <0 0xae91000 0 0x400>,
> +				      <0 0xae91400 0 0x400>;
> +				interrupt-parent = <&mdss0>;
> +				interrupts = <12>;
> +				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX0_AUX_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_INTF_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX0_PIXEL0_CLK>;
> +				clock-names = "core_iface", "core_aux",
> +					      "ctrl_link",
> +					      "ctrl_link_iface", "stream_pixel";
I overlooked this previously, but please turn this into a vertical list.

With these fixed:

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

Konrad

> +
> +				assigned-clocks = <&dispcc0 DISP_CC_MDSS_DPTX0_LINK_CLK_SRC>,
> +						  <&dispcc0 DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC>;
> +				assigned-clock-parents = <&usb_0_qmpphy QMP_USB43DP_DP_LINK_CLK>,
> +							 <&usb_0_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>;
> +
> +				phys = <&usb_0_qmpphy QMP_USB43DP_DP_PHY>;
> +				phy-names = "dp";
> +
> +				#sound-dai-cells = <0>;
> +
> +				operating-points-v2 = <&mdss0_dp0_opp_table>;
> +				power-domains = <&rpmhpd SC8280XP_CX>;
> +
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						mdss0_dp0_in: endpoint {
> +							remote-endpoint = <&mdss0_intf0_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +					};
> +				};
> +
> +				mdss0_dp0_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-160000000 {
> +						opp-hz = /bits/ 64 <160000000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-270000000 {
> +						opp-hz = /bits/ 64 <270000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-540000000 {
> +						opp-hz = /bits/ 64 <540000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +
> +					opp-810000000 {
> +						opp-hz = /bits/ 64 <810000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +				};
> +			};
> +
> +			mdss0_dp1: displayport-controller@ae98000 {
> +				compatible = "qcom,sc8280xp-dp";
> +				reg = <0 0xae98000 0 0x200>,
> +				      <0 0xae98200 0 0x200>,
> +				      <0 0xae98400 0 0x600>,
> +				      <0 0xae99000 0 0x400>,
> +				      <0 0xae99400 0 0x400>;
> +				interrupt-parent = <&mdss0>;
> +				interrupts = <13>;
> +				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX1_AUX_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_INTF_CLK>,
> +					 <&dispcc0 DISP_CC_MDSS_DPTX1_PIXEL0_CLK>;
> +				clock-names = "core_iface", "core_aux",
> +					      "ctrl_link",
> +					      "ctrl_link_iface", "stream_pixel";
> +
> +				assigned-clocks = <&dispcc0 DISP_CC_MDSS_DPTX1_LINK_CLK_SRC>,
> +						  <&dispcc0 DISP_CC_MDSS_DPTX1_PIXEL0_CLK_SRC>;
> +				assigned-clock-parents = <&usb_1_qmpphy QMP_USB43DP_DP_LINK_CLK>,
> +							 <&usb_1_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>;
> +
> +				phys = <&usb_1_qmpphy QMP_USB43DP_DP_PHY>;
> +				phy-names = "dp";
> +
> +				#sound-dai-cells = <0>;
> +
> +				operating-points-v2 = <&mdss0_dp1_opp_table>;
> +				power-domains = <&rpmhpd SC8280XP_CX>;
> +
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +						mdss0_dp1_in: endpoint {
> +							remote-endpoint = <&mdss0_intf4_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +					};
> +				};
> +
> +				mdss0_dp1_opp_table: opp-table {
> +					compatible = "operating-points-v2";
> +
> +					opp-160000000 {
> +						opp-hz = /bits/ 64 <160000000>;
> +						required-opps = <&rpmhpd_opp_low_svs>;
> +					};
> +
> +					opp-270000000 {
> +						opp-hz = /bits/ 64 <270000000>;
> +						required-opps = <&rpmhpd_opp_svs>;
> +					};
> +
> +					opp-540000000 {
> +						opp-hz = /bits/ 64 <540000000>;
> +						required-opps = <&rpmhpd_opp_svs_l1>;
> +					};
> +
> +					opp-810000000 {
> +						opp-hz = /bits/ 64 <810000000>;
> +						required-opps = <&rpmhpd_opp_nom>;
> +					};
> +				};
> +			};
> +
>  			mdss0_dp2: displayport-controller@ae9a000 {
>  				compatible = "qcom,sc8280xp-dp";
>  				reg = <0 0xae9a000 0 0x200>,
> @@ -3387,10 +3549,10 @@ dispcc0: clock-controller@af00000 {
>  			clocks = <&gcc GCC_DISP_AHB_CLK>,
>  				 <&rpmhcc RPMH_CXO_CLK>,
>  				 <&sleep_clk>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> -				 <0>,
> +				 <&usb_0_qmpphy QMP_USB43DP_DP_LINK_CLK>,
> +				 <&usb_0_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>,
> +				 <&usb_1_qmpphy QMP_USB43DP_DP_LINK_CLK>,
> +				 <&usb_1_qmpphy QMP_USB43DP_DP_VCO_DIV_CLK>,
>  				 <&mdss0_dp2_phy 0>,
>  				 <&mdss0_dp2_phy 1>,
>  				 <&mdss0_dp3_phy 0>,