linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/2] Add display support for QCS615 platform
@ 2025-08-27 13:08 Fange Zhang
  2025-08-27 13:08 ` [PATCH v7 1/2] arm64: dts: qcom: Add display support for QCS615 Fange Zhang
  2025-08-27 13:08 ` [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board Fange Zhang
  0 siblings, 2 replies; 11+ messages in thread
From: Fange Zhang @ 2025-08-27 13:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, xiangxu.yin,
	tingwei.zhang, Li Liu, Fange Zhang, Dmitry Baryshkov,
	Dmitry Baryshkov

1.Add MDSS & DPU support for QCS615
2.Add DSI support for QCS615

QCS615 platform supports DisplayPort, and this feature will be added in a future patch
Dropped patches 1–7, which have already been merged upstream

The dependency has already been reviewed
- dispcc dts
https://lore.kernel.org/all/20250814-qcs615-mm-cpu-dt-v6-v6-0-a06f69928ab5@oss.qualcomm.com/

Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
---
Changes in v7:
- Added anx7625 bridge supplies as fixed reulators [Bjorn]
- Change gpio to pinctrl for io_expander [Bjorn]
- Change vdds-supply to vcca-supply for mdss_dsi0_phy [Bjorn]
- Remove "wakeup-source" for bridge@58 [Bjorn]
- Link to v6: https://lore.kernel.org/all/20250818-add-display-support-for-qcs615-platform-v6-0-62aad5138a78@oss.qualcomm.com

Changes in v6:
- Add qcom,dsi-phy-28nm.h header and update dispcc DSI clocks [Konrad]
- Change mdss_dsi0_phy reg size from 0x188 to 0x124 [Konrad]
- Remove assigned-clocks and assigned-clocks-rates from display-controller [Konrad]
- Remove gpio header [Krzysztof]
- Replace legacy `interrupt-parent` + `interrupts` with `interrupts-extended` for display-controller [Konrad]
- Update mdp_opp_table clk [Konrad]
- Link to v5: https://lore.kernel.org/r/20250718-add-display-support-for-qcs615-platform-v5-0-8579788ea195@oss.qualcomm.com

Changes in v5:
- Drop patches 1–7, which have already been merged upstream
- Rename dp-connector to dp-dsi0-connector
- Rename dp_connector_out to dp_dsi0_connector_in
- Rename label from DP to DSI0 for dp-dsi0-connector
- Rename anx_7625 to bridge
- Rename anx_7625_in to dsi2dp_bridge_in
- Rename anx_7625_out to dsi2dp_bridge_out
- Rename ioexp to io_expander
- Replace legacy `interrupt-parent` + `interrupts` with `interrupts-extended` for bridge [Dmitry]
- Replace legacy `interrupt-parent` + `interrupts` with `interrupts-extended` for io_expander [Dmitry]
- Update interrupt type for bridge [Dmitry]
- Update interrupt type for io_expander [Dmitry]
- Link to v4: https://lore.kernel.org/all/20241210-add-display-support-for-qcs615-platform-v4-0-2d875a67602d@quicinc.com

Changes in v4:
- Add dp-connector node for anx_7625_out [Dmitry]
- Add missing qcom,sm6150-dsi-ctrl for dsi-controller-main.yaml [Krzysztof]
- Change VIG_SDM845_MASK to VIG_SDM845_MASK_SDMA for sm6150_sspp [Abhinav]
- Change DMA_SDM845_MASK to DMA_SDM845_MASK_SDMA for sm6150_sspp [Abhinav]
- Remove redundant annotation from sdm845_dsi_cfg [Dmitry]
- Remove redundant blocks from sm6150_intf [Dmitry]
- Update mdp_opp_table opp clk to correct value
- Link to v3: https://lore.kernel.org/r/20241122-add-display-support-for-qcs615-platform-v3-0-35252e3a51fe@quicinc.com

Changes in v3:
- Add reg_bus_bw for sm6150_data [Dmitry]
- Remove patch for SX150X defconfig [Dmitry]
- Remove dsi0_hpd_cfg_pins from ioexp [Dmitry]
- Remove dsi0_cdet_cfg_pins from ioexpa [Dmitry]
- Remove tlmm node for ioexp_intr_active and ioAexp_reset_active [Dmitry]
- Remove qcs615_dsi_regulators and reuse sdm845_dsi_cfg [Dmitry, Konrad]
- Rename qcs615/QCS615 to sm6150/SM6150 for whole patch [Dmitry]
- Rename qcom,dsi-phy-14nm-615 to qcom,sm6150-dsi-phy-14nm [Dmitry]
- Rename qcom,qcs615-dsi-ctrl to qcom,sm6150-dsi-ctrl [Dmitry]
- Rename qcom,qcs615-dpu to qcom,sm6150-dpu [Dmitry]
- Rename qcom,qcs615-mdss to qcom,sm6150-mdss [Dmitry]
- Split drm dsi patch to dsi and dsi phy [Dmitry]
- Update yaml clocks node with ephemeral nodes and remove unsed include [Dmitry, Rob]
- Link to v2: https://lore.kernel.org/r/20241113-add-display-support-for-qcs615-platform-v2-0-2873eb6fb869@quicinc.com

Changes in v2:
- Add QCS615 DP controller comment in commit message [Dmitry]
- Add comments for dsi_dp_hpd_cfg_pins and dsi_dp_cdet_cfg_pins [Dmitry]
- Add missing port@1 for connector for anx7625 [Dmitry]
- Change 0 to QCOM_ICC_TAG_ALWAYS for mdss interconnects [Dmitry]
- Change 0 to GPIO_ACTIVE_HIGH for GPIO flags [Dmitry]
- Move anx_7625 to same node [Dmitry]
- Move status to last in mdss_dsi0 [Dmitry]
- Rename dsi0_hpd_cfg_pins to dsi_dp_hpd_cfg_pins in ioexp [Dmitry]
- Rename dsi0_cdet_cfg_pins to dsi_dp_cdet_cfg_pins in ioexp [Dmitry]
- Rename anx_7625_1 to dsi_anx_7625 in ioexp [Dmitry]
- Remove absent block in qcs615_lm [Dmitry]
- Remove merge_3d value in qcs615_pp [Dmitry]
- Remove redundant annotation in qcs615_sspp [Dmitry]
- Remove unsupported dsi clk from dsi0_opp_table [Dmitry]
- Remove dp_hpd_cfg_pins node from ioexp [Dmitry]
- Splite drm driver patches to mdss, dpu and dsi [Dmitry]
- Link to v1: https://lore.kernel.org/r/20241014-add_display_support_for_qcs615-v1-0-4efa191dbdd4@quicinc.com

Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>

---
Li Liu (2):
      arm64: dts: qcom: Add display support for QCS615
      arm64: dts: qcom: Add display support for QCS615 RIDE board

 arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sm6150.dtsi     | 182 ++++++++++++++++++++++++++++++-
 2 files changed, 330 insertions(+), 2 deletions(-)
---
base-commit: d0630b758e593506126e8eda6c3d56097d1847c5
change-id: 20250827-add-display-support-for-qcs615-platform-213111c51778

Best regards,
-- 
Fange Zhang <fange.zhang@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v7 1/2] arm64: dts: qcom: Add display support for QCS615
  2025-08-27 13:08 [PATCH v7 0/2] Add display support for QCS615 platform Fange Zhang
@ 2025-08-27 13:08 ` Fange Zhang
  2025-08-27 13:08 ` [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board Fange Zhang
  1 sibling, 0 replies; 11+ messages in thread
From: Fange Zhang @ 2025-08-27 13:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, xiangxu.yin,
	tingwei.zhang, Li Liu, Fange Zhang, Dmitry Baryshkov

From: Li Liu <li.liu@oss.qualcomm.com>

Add display MDSS and DSI configuration for QCS615 platform.
QCS615 has a DP port, and DP support will be added in a later patch.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sm6150.dtsi | 182 ++++++++++++++++++++++++++++++++++-
 1 file changed, 180 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6150.dtsi b/arch/arm64/boot/dts/qcom/sm6150.dtsi
index 53496241479a05fec7bffa893b96b2d12b2d7614..c0e6485c148a059f6c0b2d221a9ee34b0220ea06 100644
--- a/arch/arm64/boot/dts/qcom/sm6150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6150.dtsi
@@ -3,6 +3,7 @@
  * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
+#include <dt-bindings/clock/qcom,dsi-phy-28nm.h>
 #include <dt-bindings/clock/qcom,qcs615-camcc.h>
 #include <dt-bindings/clock/qcom,qcs615-dispcc.h>
 #include <dt-bindings/clock/qcom,qcs615-gcc.h>
@@ -3579,14 +3580,191 @@ camcc: clock-controller@ad00000 {
 			#power-domain-cells = <1>;
 		};
 
+		mdss: display-subsystem@ae00000 {
+			compatible = "qcom,sm6150-mdss";
+			reg = <0x0 0x0ae00000 0x0 0x1000>;
+			reg-names = "mdss";
+
+			interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_DISPLAY_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "mdp0-mem",
+					     "cpu-cfg";
+
+			power-domains = <&dispcc MDSS_CORE_GDSC>;
+
+			clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+				 <&gcc GCC_DISP_HF_AXI_CLK>,
+				 <&dispcc DISP_CC_MDSS_MDP_CLK>;
+
+			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+
+			iommus = <&apps_smmu 0x800 0x0>;
+
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			status = "disabled";
+
+			mdss_mdp: display-controller@ae01000 {
+				compatible = "qcom,sm6150-dpu";
+				reg = <0x0 0x0ae01000 0x0 0x8f000>,
+				      <0x0 0x0aeb0000 0x0 0x2008>;
+				reg-names = "mdp",
+					    "vbif";
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>,
+					 <&dispcc DISP_CC_MDSS_MDP_CLK>,
+					 <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+				clock-names = "iface",
+					      "bus",
+					      "core",
+					      "vsync";
+
+				operating-points-v2 = <&mdp_opp_table>;
+				power-domains = <&rpmhpd RPMHPD_CX>;
+
+				interrupts-extended = <&mdss 0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						dpu_intf0_out: endpoint {
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						dpu_intf1_out: endpoint {
+							remote-endpoint = <&mdss_dsi0_in>;
+						};
+					};
+				};
+
+				mdp_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-192000000 {
+						opp-hz = /bits/ 64 <192000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-256000000 {
+						opp-hz = /bits/ 64 <256000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-307200000 {
+						opp-hz = /bits/ 64 <307200000>;
+						required-opps = <&rpmhpd_opp_nom>;
+					};
+				};
+			};
+
+			mdss_dsi0: dsi@ae94000 {
+				compatible = "qcom,sm6150-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+				reg = <0x0 0x0ae94000 0x0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupts-extended = <&mdss 4>;
+
+				clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
+					 <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
+					 <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
+					 <&dispcc DISP_CC_MDSS_ESC0_CLK>,
+					 <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>,
+						  <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
+				assigned-clock-parents = <&mdss_dsi0_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss_dsi0_phy DSI_PIXEL_PLL_CLK>;
+
+				operating-points-v2 = <&dsi0_opp_table>;
+				power-domains = <&rpmhpd RPMHPD_CX>;
+
+				phys = <&mdss_dsi0_phy>;
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				status = "disabled";
+
+				dsi0_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-164000000 {
+						opp-hz = /bits/ 64 <164000000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+				};
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						mdss_dsi0_in: endpoint {
+							remote-endpoint = <&dpu_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss_dsi0_out: endpoint {
+						};
+					};
+				};
+			};
+
+			mdss_dsi0_phy: phy@ae94400 {
+				compatible = "qcom,sm6150-dsi-phy-14nm";
+				reg = <0x0 0x0ae94400 0x0 0x100>,
+				      <0x0 0x0ae94500 0x0 0x300>,
+				      <0x0 0x0ae94800 0x0 0x124>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface",
+					      "ref";
+
+				status = "disabled";
+			};
+		};
+
 		dispcc: clock-controller@af00000 {
 			compatible = "qcom,qcs615-dispcc";
 			reg = <0 0x0af00000 0 0x20000>;
 
 			clocks = <&rpmhcc RPMH_CXO_CLK>,
 				 <&gcc GCC_DISP_GPLL0_DIV_CLK_SRC>,
-				 <0>,
-				 <0>,
+				 <&mdss_dsi0_phy DSI_BYTE_PLL_CLK>,
+				 <&mdss_dsi0_phy DSI_PIXEL_PLL_CLK>,
 				 <0>,
 				 <0>,
 				 <0>;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-27 13:08 [PATCH v7 0/2] Add display support for QCS615 platform Fange Zhang
  2025-08-27 13:08 ` [PATCH v7 1/2] arm64: dts: qcom: Add display support for QCS615 Fange Zhang
@ 2025-08-27 13:08 ` Fange Zhang
  2025-08-27 20:01   ` Dmitry Baryshkov
  1 sibling, 1 reply; 11+ messages in thread
From: Fange Zhang @ 2025-08-27 13:08 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, devicetree, linux-kernel, xiangxu.yin,
	tingwei.zhang, Li Liu, Fange Zhang, Dmitry Baryshkov

From: Li Liu <li.liu@oss.qualcomm.com>

Add display MDSS and DSI configuration for QCS615 RIDE board.
QCS615 has a DP port, and DP support will be added in a later patch.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
--- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
@@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
 		};
 	};
 
+	dp-dsi0-connector {
+		compatible = "dp-connector";
+		label = "DSI0";
+		type = "mini";
+
+		port {
+			dp_dsi0_connector_in: endpoint {
+				remote-endpoint = <&dsi2dp_bridge_out>;
+			};
+		};
+	};
+
+	vreg_12p0: vreg-12p0-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_12P0";
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+	};
+
+	vreg_5p0: vreg-5p0-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_5P0";
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+
+		vin-supply = <&vreg_12p0>;
+	};
+
+	vreg_1p8: vreg-1p8-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_1P8";
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+
+		vin-supply = <&vreg_5p0>;
+	};
+
+	vreg_1p0: vreg-1p0-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_1P0";
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+
+		vin-supply = <&vreg_1p8>;
+	};
+
+	vreg_3p0: vreg-3p0-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "VREG_3P0";
+
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <3000000>;
+		regulator-max-microvolt = <3000000>;
+
+		vin-supply = <&vreg_12p0>;
+	};
+
 	vreg_conn_1p8: regulator-conn-1p8 {
 		compatible = "regulator-fixed";
 		regulator-name = "vreg_conn_1p8";
@@ -288,6 +358,86 @@ vreg_l17a: ldo17 {
 	};
 };
 
+&i2c2 {
+	clock-frequency = <400000>;
+	status = "okay";
+
+	io_expander: pinctrl@3e {
+		compatible = "semtech,sx1509q";
+		reg = <0x3e>;
+		interrupts-extended = <&tlmm 58 IRQ_TYPE_EDGE_FALLING>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		semtech,probe-reset;
+	};
+
+	i2c-mux@77 {
+		compatible = "nxp,pca9542";
+		reg = <0x77>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			bridge@58 {
+				compatible = "analogix,anx7625";
+				reg = <0x58>;
+				interrupts-extended = <&io_expander 0 IRQ_TYPE_EDGE_FALLING>;
+				enable-gpios = <&tlmm 4 GPIO_ACTIVE_HIGH>;
+				reset-gpios = <&tlmm 5 GPIO_ACTIVE_HIGH>;
+				vdd10-supply = <&vreg_1p0>;
+				vdd18-supply = <&vreg_1p8>;
+				vdd33-supply = <&vreg_3p0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						dsi2dp_bridge_in: endpoint {
+							remote-endpoint = <&mdss_dsi0_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						dsi2dp_bridge_out: endpoint {
+							remote-endpoint = <&dp_dsi0_connector_in>;
+						};
+					};
+				};
+			};
+		};
+	};
+};
+
+&mdss {
+	status = "okay";
+};
+
+&mdss_dsi0 {
+	vdda-supply = <&vreg_l11a>;
+	status = "okay";
+};
+
+&mdss_dsi0_out {
+	remote-endpoint = <&dsi2dp_bridge_in>;
+	data-lanes = <0 1 2 3>;
+};
+
+&mdss_dsi0_phy {
+	vcca-supply = <&vreg_l5a>;
+	status = "okay";
+};
+
 &pcie {
 	perst-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 100 GPIO_ACTIVE_HIGH>;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-27 13:08 ` [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board Fange Zhang
@ 2025-08-27 20:01   ` Dmitry Baryshkov
  2025-08-28  2:57     ` Fange Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2025-08-27 20:01 UTC (permalink / raw)
  To: Fange Zhang
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu

On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
> From: Li Liu <li.liu@oss.qualcomm.com>
> 
> Add display MDSS and DSI configuration for QCS615 RIDE board.
> QCS615 has a DP port, and DP support will be added in a later patch.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
> Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
>  1 file changed, 150 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
>  		};
>  	};
>  
> +	dp-dsi0-connector {
> +		compatible = "dp-connector";
> +		label = "DSI0";
> +		type = "mini";
> +
> +		port {
> +			dp_dsi0_connector_in: endpoint {
> +				remote-endpoint = <&dsi2dp_bridge_out>;
> +			};
> +		};
> +	};
> +
> +	vreg_12p0: vreg-12p0-regulator {

I should be more carefull when doing reviews. I thought that it was
pointed out already and didn't some of the obvious things...

First of all, the nodes are sorted. By the name, not by the label.
Second, there are already regulators in this file. Why are the new nodes
not following the existing pattern and why are they not placed at a
proper place?


[.... skipped all defined regulators ...]

> +	};
> +
>  	vreg_conn_1p8: regulator-conn-1p8 {

Tadam! It's even a part of the patch.

>  		compatible = "regulator-fixed";
>  		regulator-name = "vreg_conn_1p8";

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-27 20:01   ` Dmitry Baryshkov
@ 2025-08-28  2:57     ` Fange Zhang
  2025-08-28  4:41       ` Dmitry Baryshkov
  0 siblings, 1 reply; 11+ messages in thread
From: Fange Zhang @ 2025-08-28  2:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu



On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
> On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
>> From: Li Liu <li.liu@oss.qualcomm.com>
>>
>> Add display MDSS and DSI configuration for QCS615 RIDE board.
>> QCS615 has a DP port, and DP support will be added in a later patch.
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
>> Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
>>   1 file changed, 150 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>> @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
>>   		};
>>   	};
>>   
>> +	dp-dsi0-connector {
>> +		compatible = "dp-connector";
>> +		label = "DSI0";
>> +		type = "mini";
>> +
>> +		port {
>> +			dp_dsi0_connector_in: endpoint {
>> +				remote-endpoint = <&dsi2dp_bridge_out>;
>> +			};
>> +		};
>> +	};
>> +
>> +	vreg_12p0: vreg-12p0-regulator {
> 
> I should be more carefull when doing reviews. I thought that it was
> pointed out already and didn't some of the obvious things...
> 
> First of all, the nodes are sorted. By the name, not by the label.
> Second, there are already regulators in this file. Why are the new nodes
> not following the existing pattern and why are they not placed at a
> proper place?

Initially, we referred to 
https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/ 
as a reference, but its node ordering seems a bit unconventional.

Would this revised ordering be acceptable?

...
+ dp-dsi0-connector

vreg_conn_1p8: regulator-conn-1p8
vreg_conn_pa: regulator-conn-pa
regulator-usb2-vbus

+ vreg_12p0: vreg-12p0-regulator
+ vreg_1p0: vreg-1p0-regulator
+ vreg_1p8: vreg-1p8-regulator
+ vreg_3p0: vreg-3p0-regulator
+ vreg_5p0: vreg-5p0-regulator
wcn6855-pmu
...

> 
> 
> [.... skipped all defined regulators ...]
> 
>> +	};
>> +
>>   	vreg_conn_1p8: regulator-conn-1p8 {
> 
> Tadam! It's even a part of the patch.
> 
>>   		compatible = "regulator-fixed";
>>   		regulator-name = "vreg_conn_1p8";
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-28  2:57     ` Fange Zhang
@ 2025-08-28  4:41       ` Dmitry Baryshkov
  2025-08-28  5:12         ` Fange Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2025-08-28  4:41 UTC (permalink / raw)
  To: Fange Zhang
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu

On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
> 
> 
> On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
> > On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
> > > From: Li Liu <li.liu@oss.qualcomm.com>
> > > 
> > > Add display MDSS and DSI configuration for QCS615 RIDE board.
> > > QCS615 has a DP port, and DP support will be added in a later patch.
> > > 
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
> > > Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
> > > ---
> > >   arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
> > >   1 file changed, 150 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
> > >   		};
> > >   	};
> > > +	dp-dsi0-connector {
> > > +		compatible = "dp-connector";
> > > +		label = "DSI0";
> > > +		type = "mini";
> > > +
> > > +		port {
> > > +			dp_dsi0_connector_in: endpoint {
> > > +				remote-endpoint = <&dsi2dp_bridge_out>;
> > > +			};
> > > +		};
> > > +	};
> > > +
> > > +	vreg_12p0: vreg-12p0-regulator {
> > 
> > I should be more carefull when doing reviews. I thought that it was
> > pointed out already and didn't some of the obvious things...
> > 
> > First of all, the nodes are sorted. By the name, not by the label.
> > Second, there are already regulators in this file. Why are the new nodes
> > not following the existing pattern and why are they not placed at a
> > proper place?
> 
> Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
> as a reference, but its node ordering seems a bit unconventional.
> 
> Would this revised ordering be acceptable?
> 
> ...
> + dp-dsi0-connector
> 
> vreg_conn_1p8: regulator-conn-1p8
> vreg_conn_pa: regulator-conn-pa
> regulator-usb2-vbus

So... Existing regulator nodes have the name of 'regulator-foo-bar'.

> 
> + vreg_12p0: vreg-12p0-regulator
> + vreg_1p0: vreg-1p0-regulator
> + vreg_1p8: vreg-1p8-regulator
> + vreg_3p0: vreg-3p0-regulator
> + vreg_5p0: vreg-5p0-regulator

While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
other platforms.

> wcn6855-pmu
> ...
> 
> > 
> > 
> > [.... skipped all defined regulators ...]
> > 
> > > +	};
> > > +
> > >   	vreg_conn_1p8: regulator-conn-1p8 {
> > 
> > Tadam! It's even a part of the patch.
> > 
> > >   		compatible = "regulator-fixed";
> > >   		regulator-name = "vreg_conn_1p8";
> > 
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-28  4:41       ` Dmitry Baryshkov
@ 2025-08-28  5:12         ` Fange Zhang
  2025-08-28 11:02           ` Dmitry Baryshkov
  0 siblings, 1 reply; 11+ messages in thread
From: Fange Zhang @ 2025-08-28  5:12 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu



On 8/28/2025 12:41 PM, Dmitry Baryshkov wrote:
> On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
>>
>>
>> On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
>>> On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
>>>> From: Li Liu <li.liu@oss.qualcomm.com>
>>>>
>>>> Add display MDSS and DSI configuration for QCS615 RIDE board.
>>>> QCS615 has a DP port, and DP support will be added in a later patch.
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>> Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
>>>> Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
>>>>    1 file changed, 150 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>> index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
>>>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>> @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
>>>>    		};
>>>>    	};
>>>> +	dp-dsi0-connector {
>>>> +		compatible = "dp-connector";
>>>> +		label = "DSI0";
>>>> +		type = "mini";
>>>> +
>>>> +		port {
>>>> +			dp_dsi0_connector_in: endpoint {
>>>> +				remote-endpoint = <&dsi2dp_bridge_out>;
>>>> +			};
>>>> +		};
>>>> +	};
>>>> +
>>>> +	vreg_12p0: vreg-12p0-regulator {
>>>
>>> I should be more carefull when doing reviews. I thought that it was
>>> pointed out already and didn't some of the obvious things...
>>>
>>> First of all, the nodes are sorted. By the name, not by the label.
>>> Second, there are already regulators in this file. Why are the new nodes
>>> not following the existing pattern and why are they not placed at a
>>> proper place?
>>
>> Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
>> as a reference, but its node ordering seems a bit unconventional.
>>
>> Would this revised ordering be acceptable?
>>
>> ...
>> + dp-dsi0-connector
>>
>> vreg_conn_1p8: regulator-conn-1p8
>> vreg_conn_pa: regulator-conn-pa
>> regulator-usb2-vbus
> 
> So... Existing regulator nodes have the name of 'regulator-foo-bar'.
> 
>>
>> + vreg_12p0: vreg-12p0-regulator
>> + vreg_1p0: vreg-1p0-regulator
>> + vreg_1p8: vreg-1p8-regulator
>> + vreg_3p0: vreg-3p0-regulator
>> + vreg_5p0: vreg-5p0-regulator
> 
> While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
> other platforms.

Got it, The revised format will be:

+ vreg_12p0: regulator-vreg-12p0
+ vreg_1p0: regulator-vreg-1p0
+ vreg_1p8: regulator-vreg-1p8
+ vreg_3p0: regulator-vreg-3p0
+ vreg_5p0: regulator-vreg-5p0

Let me know if you have any further suggestions.

Thanks,
Fange

> 
>> wcn6855-pmu
>> ...
>>
>>>
>>>
>>> [.... skipped all defined regulators ...]
>>>
>>>> +	};
>>>> +
>>>>    	vreg_conn_1p8: regulator-conn-1p8 {
>>>
>>> Tadam! It's even a part of the patch.
>>>
>>>>    		compatible = "regulator-fixed";
>>>>    		regulator-name = "vreg_conn_1p8";
>>>
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-28  5:12         ` Fange Zhang
@ 2025-08-28 11:02           ` Dmitry Baryshkov
  2025-09-01  3:23             ` Fange Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2025-08-28 11:02 UTC (permalink / raw)
  To: Fange Zhang
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu

On Thu, Aug 28, 2025 at 01:12:14PM +0800, Fange Zhang wrote:
> 
> 
> On 8/28/2025 12:41 PM, Dmitry Baryshkov wrote:
> > On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
> > > 
> > > 
> > > On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
> > > > On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
> > > > > From: Li Liu <li.liu@oss.qualcomm.com>
> > > > > 
> > > > > Add display MDSS and DSI configuration for QCS615 RIDE board.
> > > > > QCS615 has a DP port, and DP support will be added in a later patch.
> > > > > 
> > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > > > Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
> > > > > Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
> > > > > ---
> > > > >    arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
> > > > >    1 file changed, 150 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
> > > > >    		};
> > > > >    	};
> > > > > +	dp-dsi0-connector {
> > > > > +		compatible = "dp-connector";
> > > > > +		label = "DSI0";
> > > > > +		type = "mini";
> > > > > +
> > > > > +		port {
> > > > > +			dp_dsi0_connector_in: endpoint {
> > > > > +				remote-endpoint = <&dsi2dp_bridge_out>;
> > > > > +			};
> > > > > +		};
> > > > > +	};
> > > > > +
> > > > > +	vreg_12p0: vreg-12p0-regulator {
> > > > 
> > > > I should be more carefull when doing reviews. I thought that it was
> > > > pointed out already and didn't some of the obvious things...
> > > > 
> > > > First of all, the nodes are sorted. By the name, not by the label.
> > > > Second, there are already regulators in this file. Why are the new nodes
> > > > not following the existing pattern and why are they not placed at a
> > > > proper place?
> > > 
> > > Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
> > > as a reference, but its node ordering seems a bit unconventional.
> > > 
> > > Would this revised ordering be acceptable?
> > > 
> > > ...
> > > + dp-dsi0-connector
> > > 
> > > vreg_conn_1p8: regulator-conn-1p8
> > > vreg_conn_pa: regulator-conn-pa
> > > regulator-usb2-vbus
> > 
> > So... Existing regulator nodes have the name of 'regulator-foo-bar'.
> > 
> > > 
> > > + vreg_12p0: vreg-12p0-regulator
> > > + vreg_1p0: vreg-1p0-regulator
> > > + vreg_1p8: vreg-1p8-regulator
> > > + vreg_3p0: vreg-3p0-regulator
> > > + vreg_5p0: vreg-5p0-regulator
> > 
> > While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
> > other platforms.
> 
> Got it, The revised format will be:
> 
> + vreg_12p0: regulator-vreg-12p0
> + vreg_1p0: regulator-vreg-1p0
> + vreg_1p8: regulator-vreg-1p8
> + vreg_3p0: regulator-vreg-3p0
> + vreg_5p0: regulator-vreg-5p0
> 
> Let me know if you have any further suggestions.

What's the name of power rail in the schematics? vreg-Np0?


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-08-28 11:02           ` Dmitry Baryshkov
@ 2025-09-01  3:23             ` Fange Zhang
  2025-09-02 13:56               ` Dmitry Baryshkov
  0 siblings, 1 reply; 11+ messages in thread
From: Fange Zhang @ 2025-09-01  3:23 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu



On 8/28/2025 7:02 PM, Dmitry Baryshkov wrote:
> On Thu, Aug 28, 2025 at 01:12:14PM +0800, Fange Zhang wrote:
>>
>>
>> On 8/28/2025 12:41 PM, Dmitry Baryshkov wrote:
>>> On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
>>>>
>>>>
>>>> On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
>>>>> On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
>>>>>> From: Li Liu <li.liu@oss.qualcomm.com>
>>>>>>
>>>>>> Add display MDSS and DSI configuration for QCS615 RIDE board.
>>>>>> QCS615 has a DP port, and DP support will be added in a later patch.
>>>>>>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>>>> Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
>>>>>> Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
>>>>>> ---
>>>>>>     arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 150 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>> index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
>>>>>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>> @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
>>>>>>     		};
>>>>>>     	};
>>>>>> +	dp-dsi0-connector {
>>>>>> +		compatible = "dp-connector";
>>>>>> +		label = "DSI0";
>>>>>> +		type = "mini";
>>>>>> +
>>>>>> +		port {
>>>>>> +			dp_dsi0_connector_in: endpoint {
>>>>>> +				remote-endpoint = <&dsi2dp_bridge_out>;
>>>>>> +			};
>>>>>> +		};
>>>>>> +	};
>>>>>> +
>>>>>> +	vreg_12p0: vreg-12p0-regulator {
>>>>>
>>>>> I should be more carefull when doing reviews. I thought that it was
>>>>> pointed out already and didn't some of the obvious things...
>>>>>
>>>>> First of all, the nodes are sorted. By the name, not by the label.
>>>>> Second, there are already regulators in this file. Why are the new nodes
>>>>> not following the existing pattern and why are they not placed at a
>>>>> proper place?
>>>>
>>>> Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
>>>> as a reference, but its node ordering seems a bit unconventional.
>>>>
>>>> Would this revised ordering be acceptable?
>>>>
>>>> ...
>>>> + dp-dsi0-connector
>>>>
>>>> vreg_conn_1p8: regulator-conn-1p8
>>>> vreg_conn_pa: regulator-conn-pa
>>>> regulator-usb2-vbus
>>>
>>> So... Existing regulator nodes have the name of 'regulator-foo-bar'.
>>>
>>>>
>>>> + vreg_12p0: vreg-12p0-regulator
>>>> + vreg_1p0: vreg-1p0-regulator
>>>> + vreg_1p8: vreg-1p8-regulator
>>>> + vreg_3p0: vreg-3p0-regulator
>>>> + vreg_5p0: vreg-5p0-regulator
>>>
>>> While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
>>> other platforms.
>>
>> Got it, The revised format will be:
>>
>> + vreg_12p0: regulator-vreg-12p0
>> + vreg_1p0: regulator-vreg-1p0
>> + vreg_1p8: regulator-vreg-1p8
>> + vreg_3p0: regulator-vreg-3p0
>> + vreg_5p0: regulator-vreg-5p0
>>
>> Let me know if you have any further suggestions.
> 
> What's the name of power rail in the schematics? vreg-Np0?

I reviewed the Ride board schematics and found the following power rail 
mappings:

VREG_1P0 -> DSI0_DVDD10 / DSI0_AVDD10 -> ANX7625 AVDD10 / DVDD10
VREG_1P8 -> DSI0_AVDD18 -> ANX7625 AVDD18
VREG_S4A_1P8 -> DSI0_DVDD18 -> ANX7625 DVDD18
VIDEO_OUT_VREG_3P3 -> DSI0_AVDD30 -> ANX7625 AVDD30

would the current approach also be acceptable?
or we need configure the power supplies strictly according to this mapping.
Appreciate your guidance.

> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-09-01  3:23             ` Fange Zhang
@ 2025-09-02 13:56               ` Dmitry Baryshkov
  2025-09-03  1:45                 ` Fange Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2025-09-02 13:56 UTC (permalink / raw)
  To: Fange Zhang
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu

On Mon, Sep 01, 2025 at 11:23:28AM +0800, Fange Zhang wrote:
> 
> 
> On 8/28/2025 7:02 PM, Dmitry Baryshkov wrote:
> > On Thu, Aug 28, 2025 at 01:12:14PM +0800, Fange Zhang wrote:
> > > 
> > > 
> > > On 8/28/2025 12:41 PM, Dmitry Baryshkov wrote:
> > > > On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
> > > > > 
> > > > > 
> > > > > On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
> > > > > > On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
> > > > > > > From: Li Liu <li.liu@oss.qualcomm.com>
> > > > > > > 
> > > > > > > Add display MDSS and DSI configuration for QCS615 RIDE board.
> > > > > > > QCS615 has a DP port, and DP support will be added in a later patch.
> > > > > > > 
> > > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > > > > > Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
> > > > > > > Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
> > > > > > > ---
> > > > > > >     arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
> > > > > > >     1 file changed, 150 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > > > index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
> > > > > > > --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > > > +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> > > > > > > @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
> > > > > > >     		};
> > > > > > >     	};
> > > > > > > +	dp-dsi0-connector {
> > > > > > > +		compatible = "dp-connector";
> > > > > > > +		label = "DSI0";
> > > > > > > +		type = "mini";
> > > > > > > +
> > > > > > > +		port {
> > > > > > > +			dp_dsi0_connector_in: endpoint {
> > > > > > > +				remote-endpoint = <&dsi2dp_bridge_out>;
> > > > > > > +			};
> > > > > > > +		};
> > > > > > > +	};
> > > > > > > +
> > > > > > > +	vreg_12p0: vreg-12p0-regulator {
> > > > > > 
> > > > > > I should be more carefull when doing reviews. I thought that it was
> > > > > > pointed out already and didn't some of the obvious things...
> > > > > > 
> > > > > > First of all, the nodes are sorted. By the name, not by the label.
> > > > > > Second, there are already regulators in this file. Why are the new nodes
> > > > > > not following the existing pattern and why are they not placed at a
> > > > > > proper place?
> > > > > 
> > > > > Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
> > > > > as a reference, but its node ordering seems a bit unconventional.
> > > > > 
> > > > > Would this revised ordering be acceptable?
> > > > > 
> > > > > ...
> > > > > + dp-dsi0-connector
> > > > > 
> > > > > vreg_conn_1p8: regulator-conn-1p8
> > > > > vreg_conn_pa: regulator-conn-pa
> > > > > regulator-usb2-vbus
> > > > 
> > > > So... Existing regulator nodes have the name of 'regulator-foo-bar'.
> > > > 
> > > > > 
> > > > > + vreg_12p0: vreg-12p0-regulator
> > > > > + vreg_1p0: vreg-1p0-regulator
> > > > > + vreg_1p8: vreg-1p8-regulator
> > > > > + vreg_3p0: vreg-3p0-regulator
> > > > > + vreg_5p0: vreg-5p0-regulator
> > > > 
> > > > While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
> > > > other platforms.
> > > 
> > > Got it, The revised format will be:
> > > 
> > > + vreg_12p0: regulator-vreg-12p0
> > > + vreg_1p0: regulator-vreg-1p0
> > > + vreg_1p8: regulator-vreg-1p8
> > > + vreg_3p0: regulator-vreg-3p0
> > > + vreg_5p0: regulator-vreg-5p0
> > > 
> > > Let me know if you have any further suggestions.
> > 
> > What's the name of power rail in the schematics? vreg-Np0?
> 
> I reviewed the Ride board schematics and found the following power rail
> mappings:
> 
> VREG_1P0 -> DSI0_DVDD10 / DSI0_AVDD10 -> ANX7625 AVDD10 / DVDD10
> VREG_1P8 -> DSI0_AVDD18 -> ANX7625 AVDD18
> VREG_S4A_1P8 -> DSI0_DVDD18 -> ANX7625 DVDD18
> VIDEO_OUT_VREG_3P3 -> DSI0_AVDD30 -> ANX7625 AVDD30

Then it looks like regulator-vreg-NpM is okay

> 
> would the current approach also be acceptable?
> or we need configure the power supplies strictly according to this mapping.
> Appreciate your guidance.
> 
> > 
> > 
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board
  2025-09-02 13:56               ` Dmitry Baryshkov
@ 2025-09-03  1:45                 ` Fange Zhang
  0 siblings, 0 replies; 11+ messages in thread
From: Fange Zhang @ 2025-09-03  1:45 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-arm-msm, devicetree, linux-kernel,
	xiangxu.yin, tingwei.zhang, Li Liu



On 9/2/2025 9:56 PM, Dmitry Baryshkov wrote:
> On Mon, Sep 01, 2025 at 11:23:28AM +0800, Fange Zhang wrote:
>>
>>
>> On 8/28/2025 7:02 PM, Dmitry Baryshkov wrote:
>>> On Thu, Aug 28, 2025 at 01:12:14PM +0800, Fange Zhang wrote:
>>>>
>>>>
>>>> On 8/28/2025 12:41 PM, Dmitry Baryshkov wrote:
>>>>> On Thu, Aug 28, 2025 at 10:57:41AM +0800, Fange Zhang wrote:
>>>>>>
>>>>>>
>>>>>> On 8/28/2025 4:01 AM, Dmitry Baryshkov wrote:
>>>>>>> On Wed, Aug 27, 2025 at 09:08:39PM +0800, Fange Zhang wrote:
>>>>>>>> From: Li Liu <li.liu@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> Add display MDSS and DSI configuration for QCS615 RIDE board.
>>>>>>>> QCS615 has a DP port, and DP support will be added in a later patch.
>>>>>>>>
>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>>>>>> Signed-off-by: Li Liu <li.liu@oss.qualcomm.com>
>>>>>>>> Signed-off-by: Fange Zhang <fange.zhang@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>>      arch/arm64/boot/dts/qcom/qcs615-ride.dts | 150 +++++++++++++++++++++++++++++++
>>>>>>>>      1 file changed, 150 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>>>> index e663343df75d59481786192cde647017a83c4191..f6e0c82cf85459d8989332497ded8b6ea3670c76 100644
>>>>>>>> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
>>>>>>>> @@ -39,6 +39,76 @@ xo_board_clk: xo-board-clk {
>>>>>>>>      		};
>>>>>>>>      	};
>>>>>>>> +	dp-dsi0-connector {
>>>>>>>> +		compatible = "dp-connector";
>>>>>>>> +		label = "DSI0";
>>>>>>>> +		type = "mini";
>>>>>>>> +
>>>>>>>> +		port {
>>>>>>>> +			dp_dsi0_connector_in: endpoint {
>>>>>>>> +				remote-endpoint = <&dsi2dp_bridge_out>;
>>>>>>>> +			};
>>>>>>>> +		};
>>>>>>>> +	};
>>>>>>>> +
>>>>>>>> +	vreg_12p0: vreg-12p0-regulator {
>>>>>>>
>>>>>>> I should be more carefull when doing reviews. I thought that it was
>>>>>>> pointed out already and didn't some of the obvious things...
>>>>>>>
>>>>>>> First of all, the nodes are sorted. By the name, not by the label.
>>>>>>> Second, there are already regulators in this file. Why are the new nodes
>>>>>>> not following the existing pattern and why are they not placed at a
>>>>>>> proper place?
>>>>>>
>>>>>> Initially, we referred to https://patchwork.kernel.org/project/linux-arm-msm/patch/20250604071851.1438612-3-quic_amakhija@quicinc.com/
>>>>>> as a reference, but its node ordering seems a bit unconventional.
>>>>>>
>>>>>> Would this revised ordering be acceptable?
>>>>>>
>>>>>> ...
>>>>>> + dp-dsi0-connector
>>>>>>
>>>>>> vreg_conn_1p8: regulator-conn-1p8
>>>>>> vreg_conn_pa: regulator-conn-pa
>>>>>> regulator-usb2-vbus
>>>>>
>>>>> So... Existing regulator nodes have the name of 'regulator-foo-bar'.
>>>>>
>>>>>>
>>>>>> + vreg_12p0: vreg-12p0-regulator
>>>>>> + vreg_1p0: vreg-1p0-regulator
>>>>>> + vreg_1p8: vreg-1p8-regulator
>>>>>> + vreg_3p0: vreg-3p0-regulator
>>>>>> + vreg_5p0: vreg-5p0-regulator
>>>>>
>>>>> While yours use 'vreg-baz-regulator'. Why? Don't blindly c&p data from
>>>>> other platforms.
>>>>
>>>> Got it, The revised format will be:
>>>>
>>>> + vreg_12p0: regulator-vreg-12p0
>>>> + vreg_1p0: regulator-vreg-1p0
>>>> + vreg_1p8: regulator-vreg-1p8
>>>> + vreg_3p0: regulator-vreg-3p0
>>>> + vreg_5p0: regulator-vreg-5p0
>>>>
>>>> Let me know if you have any further suggestions.
>>>
>>> What's the name of power rail in the schematics? vreg-Np0?
>>
>> I reviewed the Ride board schematics and found the following power rail
>> mappings:
>>
>> VREG_1P0 -> DSI0_DVDD10 / DSI0_AVDD10 -> ANX7625 AVDD10 / DVDD10
>> VREG_1P8 -> DSI0_AVDD18 -> ANX7625 AVDD18
>> VREG_S4A_1P8 -> DSI0_DVDD18 -> ANX7625 DVDD18
>> VIDEO_OUT_VREG_3P3 -> DSI0_AVDD30 -> ANX7625 AVDD30
> 
> Then it looks like regulator-vreg-NpM is okay

Got it, I'll update v8 today following this format. Thanks!

> 
>>
>> would the current approach also be acceptable?
>> or we need configure the power supplies strictly according to this mapping.
>> Appreciate your guidance.
>>
>>>
>>>
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-03  1:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-27 13:08 [PATCH v7 0/2] Add display support for QCS615 platform Fange Zhang
2025-08-27 13:08 ` [PATCH v7 1/2] arm64: dts: qcom: Add display support for QCS615 Fange Zhang
2025-08-27 13:08 ` [PATCH v7 2/2] arm64: dts: qcom: Add display support for QCS615 RIDE board Fange Zhang
2025-08-27 20:01   ` Dmitry Baryshkov
2025-08-28  2:57     ` Fange Zhang
2025-08-28  4:41       ` Dmitry Baryshkov
2025-08-28  5:12         ` Fange Zhang
2025-08-28 11:02           ` Dmitry Baryshkov
2025-09-01  3:23             ` Fange Zhang
2025-09-02 13:56               ` Dmitry Baryshkov
2025-09-03  1:45                 ` Fange Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).