All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs
@ 2023-06-16 10:49 Imran Shaik
  2023-06-16 10:49 ` [PATCH 1/2] dt-bindings: clock: " Imran Shaik
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Imran Shaik @ 2023-06-16 10:49 UTC (permalink / raw
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, Imran Shaik, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

Update GCC clocks and add support for GDSCs for QDU1000 and QRU1000 SoCs.
Also, add support for v2 variant as well.

Imran Shaik (2):
  dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
  clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs

 .../bindings/clock/qcom,qdu1000-gcc.yaml      |   6 +-
 drivers/clk/qcom/gcc-qdu1000.c                | 162 ++++++++++++------
 include/dt-bindings/clock/qcom,qdu1000-gcc.h  |   4 +-
 3 files changed, 118 insertions(+), 54 deletions(-)

-- 
2.25.1


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

* [PATCH 1/2] dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-16 10:49 [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Imran Shaik
@ 2023-06-16 10:49 ` Imran Shaik
  2023-06-16 11:33   ` Krzysztof Kozlowski
  2023-06-16 10:49 ` [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs Imran Shaik
  2023-06-16 11:21 ` [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Konrad Dybcio
  2 siblings, 1 reply; 13+ messages in thread
From: Imran Shaik @ 2023-06-16 10:49 UTC (permalink / raw
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, Imran Shaik, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

Update the qcom GCC clock bindings and add v2 compatible string for QDU1000
and QRU1000 SoCs.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
---
 .../devicetree/bindings/clock/qcom,qdu1000-gcc.yaml         | 6 +++++-
 include/dt-bindings/clock/qcom,qdu1000-gcc.h                | 4 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
index 767a9d03aa32..030953d258c1 100644
--- a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
@@ -8,6 +8,8 @@ title: Qualcomm Global Clock & Reset Controller for QDU1000 and QRU1000
 
 maintainers:
   - Melody Olvera <quic_molvera@quicinc.com>
+  - Taniya Das <quic_tdas@quicinc.com>
+  - Imran Shaik <quic_imrashai@quicinc.com>
 
 description: |
   Qualcomm global clock control module which supports the clocks, resets and
@@ -17,7 +19,9 @@ description: |
 
 properties:
   compatible:
-    const: qcom,qdu1000-gcc
+    enum:
+      - qcom,qdu1000-gcc
+      - qcom,qdu1000-gcc-v2
 
   clocks:
     items:
diff --git a/include/dt-bindings/clock/qcom,qdu1000-gcc.h b/include/dt-bindings/clock/qcom,qdu1000-gcc.h
index ddbc6b825e80..2fd36cbfddbb 100644
--- a/include/dt-bindings/clock/qcom,qdu1000-gcc.h
+++ b/include/dt-bindings/clock/qcom,qdu1000-gcc.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
 /*
- * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2023, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #ifndef _DT_BINDINGS_CLK_QCOM_GCC_QDU1000_H
@@ -138,6 +138,8 @@
 #define GCC_AGGRE_NOC_ECPRI_GSI_CLK			128
 #define GCC_PCIE_0_PIPE_CLK_SRC				129
 #define GCC_PCIE_0_PHY_AUX_CLK_SRC			130
+#define GCC_GPLL1_OUT_EVEN				131
+#define GCC_DDRSS_ECPRI_GSI_CLK				132
 
 /* GCC resets */
 #define GCC_ECPRI_CC_BCR				0
-- 
2.25.1


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

* [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-16 10:49 [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Imran Shaik
  2023-06-16 10:49 ` [PATCH 1/2] dt-bindings: clock: " Imran Shaik
@ 2023-06-16 10:49 ` Imran Shaik
  2023-06-16 11:20   ` Dmitry Baryshkov
  2023-06-16 11:22   ` Konrad Dybcio
  2023-06-16 11:21 ` [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Konrad Dybcio
  2 siblings, 2 replies; 13+ messages in thread
From: Imran Shaik @ 2023-06-16 10:49 UTC (permalink / raw
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, Imran Shaik, linux-arm-msm, linux-clk,
	devicetree, linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

Update the GCC clocks and add support for GDSCs for QDU1000 and
QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
add support for v2 variant.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
---
 drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
 1 file changed, 110 insertions(+), 52 deletions(-)

diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
index 5051769ad90c..5d8125c0eacc 100644
--- a/drivers/clk/qcom/gcc-qdu1000.c
+++ b/drivers/clk/qcom/gcc-qdu1000.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
 #include <linux/clk-provider.h>
@@ -17,6 +17,7 @@
 #include "clk-regmap-divider.h"
 #include "clk-regmap-mux.h"
 #include "clk-regmap-phy-mux.h"
+#include "gdsc.h"
 #include "reset.h"
 
 enum {
@@ -370,16 +371,6 @@ static const struct clk_parent_data gcc_parent_data_6[] = {
 	{ .index = DT_TCXO_IDX },
 };
 
-static const struct parent_map gcc_parent_map_7[] = {
-	{ P_PCIE_0_PIPE_CLK, 0 },
-	{ P_BI_TCXO, 2 },
-};
-
-static const struct clk_parent_data gcc_parent_data_7[] = {
-	{ .index = DT_PCIE_0_PIPE_CLK_IDX },
-	{ .index = DT_TCXO_IDX },
-};
-
 static const struct parent_map gcc_parent_map_8[] = {
 	{ P_BI_TCXO, 0 },
 	{ P_GCC_GPLL0_OUT_MAIN, 1 },
@@ -439,16 +430,15 @@ static struct clk_regmap_mux gcc_pcie_0_phy_aux_clk_src = {
 	},
 };
 
-static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
+static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
 	.reg = 0x9d064,
-	.shift = 0,
-	.width = 2,
-	.parent_map = gcc_parent_map_7,
 	.clkr = {
 		.hw.init = &(const struct clk_init_data) {
 			.name = "gcc_pcie_0_pipe_clk_src",
-			.parent_data = gcc_parent_data_7,
-			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
+			.parent_data = &(const struct clk_parent_data){
+				.index = DT_PCIE_0_PIPE_CLK_IDX,
+			},
+			.num_parents = 1,
 			.ops = &clk_regmap_phy_mux_ops,
 		},
 	},
@@ -485,7 +475,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_dma_clk_src = {
 		.name = "gcc_aggre_noc_ecpri_dma_clk_src",
 		.parent_data = gcc_parent_data_4,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_4),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -505,7 +495,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_gsi_clk_src = {
 		.name = "gcc_aggre_noc_ecpri_gsi_clk_src",
 		.parent_data = gcc_parent_data_5,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
 		.name = "gcc_gp1_clk_src",
 		.parent_data = gcc_parent_data_1,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -538,7 +528,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
 		.name = "gcc_gp2_clk_src",
 		.parent_data = gcc_parent_data_1,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -552,7 +542,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
 		.name = "gcc_gp3_clk_src",
 		.parent_data = gcc_parent_data_1,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -571,7 +561,7 @@ static struct clk_rcg2 gcc_pcie_0_aux_clk_src = {
 		.name = "gcc_pcie_0_aux_clk_src",
 		.parent_data = gcc_parent_data_3,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -591,7 +581,7 @@ static struct clk_rcg2 gcc_pcie_0_phy_rchng_clk_src = {
 		.name = "gcc_pcie_0_phy_rchng_clk_src",
 		.parent_data = gcc_parent_data_0,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -610,7 +600,7 @@ static struct clk_rcg2 gcc_pdm2_clk_src = {
 		.name = "gcc_pdm2_clk_src",
 		.parent_data = gcc_parent_data_0,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -632,7 +622,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s0_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s0_clk_src = {
@@ -648,7 +638,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s1_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s1_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s1_clk_src = {
@@ -664,7 +654,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s2_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s2_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s2_clk_src = {
@@ -680,7 +670,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s3_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s3_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s3_clk_src = {
@@ -696,7 +686,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s4_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s4_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s4_clk_src = {
@@ -717,7 +707,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s5_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s5_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s5_clk_src = {
@@ -733,7 +723,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s6_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s6_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s6_clk_src = {
@@ -749,7 +739,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s7_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s7_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap0_s7_clk_src = {
@@ -765,7 +755,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s0_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s0_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s0_clk_src = {
@@ -781,7 +771,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s1_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s1_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s1_clk_src = {
@@ -797,7 +787,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s2_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s2_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s2_clk_src = {
@@ -813,7 +803,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s3_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s3_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s3_clk_src = {
@@ -829,7 +819,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s4_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s4_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s4_clk_src = {
@@ -845,7 +835,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s5_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s5_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s5_clk_src = {
@@ -861,7 +851,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s6_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s6_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s6_clk_src = {
@@ -877,7 +867,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s7_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s7_clk_src",
 	.parent_data = gcc_parent_data_0,
 	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-	.ops = &clk_rcg2_ops,
+	.ops = &clk_rcg2_shared_ops,
 };
 
 static struct clk_rcg2 gcc_qupv3_wrap1_s7_clk_src = {
@@ -913,7 +903,7 @@ static struct clk_rcg2 gcc_sdcc5_apps_clk_src = {
 		.name = "gcc_sdcc5_apps_clk_src",
 		.parent_data = gcc_parent_data_8,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_8),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_floor_ops,
 	},
 };
 
@@ -932,7 +922,7 @@ static struct clk_rcg2 gcc_sdcc5_ice_core_clk_src = {
 		.name = "gcc_sdcc5_ice_core_clk_src",
 		.parent_data = gcc_parent_data_2,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_floor_ops,
 	},
 };
 
@@ -946,7 +936,7 @@ static struct clk_rcg2 gcc_sm_bus_xo_clk_src = {
 		.name = "gcc_sm_bus_xo_clk_src",
 		.parent_data = gcc_parent_data_2,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -965,7 +955,7 @@ static struct clk_rcg2 gcc_tsc_clk_src = {
 		.name = "gcc_tsc_clk_src",
 		.parent_data = gcc_parent_data_9,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_9),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -985,7 +975,7 @@ static struct clk_rcg2 gcc_usb30_prim_master_clk_src = {
 		.name = "gcc_usb30_prim_master_clk_src",
 		.parent_data = gcc_parent_data_0,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -999,7 +989,7 @@ static struct clk_rcg2 gcc_usb30_prim_mock_utmi_clk_src = {
 		.name = "gcc_usb30_prim_mock_utmi_clk_src",
 		.parent_data = gcc_parent_data_0,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -1013,7 +1003,7 @@ static struct clk_rcg2 gcc_usb3_prim_phy_aux_clk_src = {
 		.name = "gcc_usb3_prim_phy_aux_clk_src",
 		.parent_data = gcc_parent_data_3,
 		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -1142,6 +1132,26 @@ static struct clk_branch gcc_ddrss_ecpri_dma_clk = {
 	},
 };
 
+static struct clk_branch gcc_ddrss_ecpri_gsi_clk = {
+	.halt_reg = 0x54298,
+	.halt_check = BRANCH_HALT_VOTED,
+	.hwcg_reg = 0x54298,
+	.hwcg_bit = 1,
+	.clkr = {
+		.enable_reg = 0x54298,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "gcc_ddrss_ecpri_gsi_clk",
+			.parent_hws = (const struct clk_hw*[]) {
+				&gcc_aggre_noc_ecpri_gsi_clk_src.clkr.hw,
+			},
+			.num_parents = 1,
+			.flags = CLK_SET_RATE_PARENT,
+			.ops = &clk_branch2_aon_ops,
+		},
+	},
+};
+
 static struct clk_branch gcc_ecpri_ahb_clk = {
 	.halt_reg = 0x3a008,
 	.halt_check = BRANCH_HALT_VOTED,
@@ -1458,14 +1468,13 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = {
 
 static struct clk_branch gcc_pcie_0_clkref_en = {
 	.halt_reg = 0x9c004,
-	.halt_bit = 31,
 	.halt_check = BRANCH_HALT_ENABLE,
 	.clkr = {
 		.enable_reg = 0x9c004,
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data) {
 			.name = "gcc_pcie_0_clkref_en",
-			.ops = &clk_branch_ops,
+			.ops = &clk_branch2_ops,
 		},
 	},
 };
@@ -2285,14 +2294,13 @@ static struct clk_branch gcc_tsc_etu_clk = {
 
 static struct clk_branch gcc_usb2_clkref_en = {
 	.halt_reg = 0x9c008,
-	.halt_bit = 31,
 	.halt_check = BRANCH_HALT_ENABLE,
 	.clkr = {
 		.enable_reg = 0x9c008,
 		.enable_mask = BIT(0),
 		.hw.init = &(const struct clk_init_data) {
 			.name = "gcc_usb2_clkref_en",
-			.ops = &clk_branch_ops,
+			.ops = &clk_branch2_ops,
 		},
 	},
 };
@@ -2402,6 +2410,39 @@ static struct clk_branch gcc_usb3_prim_phy_pipe_clk = {
 	},
 };
 
+static struct gdsc pcie_0_gdsc = {
+	.gdscr = 0x9d004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
+	.pd = {
+		.name = "gcc_pcie_0_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+};
+
+static struct gdsc pcie_0_phy_gdsc = {
+	.gdscr = 0x7c004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x2,
+	.pd = {
+		.name = "gcc_pcie_0_phy_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+};
+
+static struct gdsc usb30_prim_gdsc = {
+	.gdscr = 0x49004,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
+	.pd = {
+		.name = "gcc_usb30_prim_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+};
+
 static struct clk_regmap *gcc_qdu1000_clocks[] = {
 	[GCC_AGGRE_NOC_ECPRI_DMA_CLK] = &gcc_aggre_noc_ecpri_dma_clk.clkr,
 	[GCC_AGGRE_NOC_ECPRI_DMA_CLK_SRC] = &gcc_aggre_noc_ecpri_dma_clk_src.clkr,
@@ -2534,6 +2575,14 @@ static struct clk_regmap *gcc_qdu1000_clocks[] = {
 	[GCC_AGGRE_NOC_ECPRI_GSI_CLK] = &gcc_aggre_noc_ecpri_gsi_clk.clkr,
 	[GCC_PCIE_0_PHY_AUX_CLK_SRC] = &gcc_pcie_0_phy_aux_clk_src.clkr,
 	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
+	[GCC_GPLL1_OUT_EVEN] = &gcc_gpll1_out_even.clkr,
+	[GCC_DDRSS_ECPRI_GSI_CLK] = NULL,
+};
+
+static struct gdsc *gcc_qdu1000_gdscs[] = {
+	[PCIE_0_GDSC] = &pcie_0_gdsc,
+	[PCIE_0_PHY_GDSC] = &pcie_0_phy_gdsc,
+	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
 };
 
 static const struct qcom_reset_map gcc_qdu1000_resets[] = {
@@ -2597,10 +2646,13 @@ static const struct qcom_cc_desc gcc_qdu1000_desc = {
 	.num_clks = ARRAY_SIZE(gcc_qdu1000_clocks),
 	.resets = gcc_qdu1000_resets,
 	.num_resets = ARRAY_SIZE(gcc_qdu1000_resets),
+	.gdscs = gcc_qdu1000_gdscs,
+	.num_gdscs = ARRAY_SIZE(gcc_qdu1000_gdscs),
 };
 
 static const struct of_device_id gcc_qdu1000_match_table[] = {
 	{ .compatible = "qcom,qdu1000-gcc" },
+	{ .compatible = "qcom,qdu1000-gcc-v2" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, gcc_qdu1000_match_table);
@@ -2617,6 +2669,12 @@ static int gcc_qdu1000_probe(struct platform_device *pdev)
 	/* Update FORCE_MEM_CORE_ON for gcc_pcie_0_mstr_axi_clk */
 	regmap_update_bits(regmap, 0x9d024, BIT(14), BIT(14));
 
+	if (of_device_is_compatible(pdev->dev.of_node, "qcom,qdu1000-gcc-v2")) {
+		gcc_qdu1000_clocks[GCC_DDRSS_ECPRI_GSI_CLK] = &gcc_ddrss_ecpri_gsi_clk.clkr;
+		gcc_pcie_0_clkref_en.halt_check = BRANCH_HALT_DELAY;
+		gcc_usb2_clkref_en.halt_check = BRANCH_HALT_DELAY;
+	}
+
 	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
 				       ARRAY_SIZE(gcc_dfs_clocks));
 	if (ret)
-- 
2.25.1


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

* Re: [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-16 10:49 ` [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs Imran Shaik
@ 2023-06-16 11:20   ` Dmitry Baryshkov
  2023-06-23 10:08     ` Imran Shaik
  2023-06-16 11:22   ` Konrad Dybcio
  1 sibling, 1 reply; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-06-16 11:20 UTC (permalink / raw
  To: Imran Shaik, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On 16/06/2023 13:49, Imran Shaik wrote:
> Update the GCC clocks and add support for GDSCs for QDU1000 and
> QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
> add support for v2 variant.

Please split this into individual chunks instead of squashing everything 
together. For each change please describe the logic behind the change in 
the commit message. Please describe why, not what is changed.

> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>

This doesn't look fully logical. Who is the author of the patch? If 
there are two authors, please add Co-developed-by tag.

> ---
>   drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
>   1 file changed, 110 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
> index 5051769ad90c..5d8125c0eacc 100644
> --- a/drivers/clk/qcom/gcc-qdu1000.c
> +++ b/drivers/clk/qcom/gcc-qdu1000.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /*
> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>    */
>   
>   #include <linux/clk-provider.h>
> @@ -17,6 +17,7 @@
>   #include "clk-regmap-divider.h"
>   #include "clk-regmap-mux.h"
>   #include "clk-regmap-phy-mux.h"
> +#include "gdsc.h"
>   #include "reset.h"
>   
>   enum {
> @@ -370,16 +371,6 @@ static const struct clk_parent_data gcc_parent_data_6[] = {
>   	{ .index = DT_TCXO_IDX },
>   };
>   
> -static const struct parent_map gcc_parent_map_7[] = {
> -	{ P_PCIE_0_PIPE_CLK, 0 },
> -	{ P_BI_TCXO, 2 },
> -};
> -
> -static const struct clk_parent_data gcc_parent_data_7[] = {
> -	{ .index = DT_PCIE_0_PIPE_CLK_IDX },
> -	{ .index = DT_TCXO_IDX },
> -};
> -
>   static const struct parent_map gcc_parent_map_8[] = {
>   	{ P_BI_TCXO, 0 },
>   	{ P_GCC_GPLL0_OUT_MAIN, 1 },
> @@ -439,16 +430,15 @@ static struct clk_regmap_mux gcc_pcie_0_phy_aux_clk_src = {
>   	},
>   };
>   
> -static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
> +static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
>   	.reg = 0x9d064,
> -	.shift = 0,
> -	.width = 2,
> -	.parent_map = gcc_parent_map_7,
>   	.clkr = {
>   		.hw.init = &(const struct clk_init_data) {
>   			.name = "gcc_pcie_0_pipe_clk_src",
> -			.parent_data = gcc_parent_data_7,
> -			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
> +			.parent_data = &(const struct clk_parent_data){
> +				.index = DT_PCIE_0_PIPE_CLK_IDX,
> +			},
> +			.num_parents = 1,
>   			.ops = &clk_regmap_phy_mux_ops,
>   		},
>   	},
> @@ -485,7 +475,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_dma_clk_src = {
>   		.name = "gcc_aggre_noc_ecpri_dma_clk_src",
>   		.parent_data = gcc_parent_data_4,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_4),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -505,7 +495,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_gsi_clk_src = {
>   		.name = "gcc_aggre_noc_ecpri_gsi_clk_src",
>   		.parent_data = gcc_parent_data_5,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,

This is probably some kind of NoC or NIU clock. If it is not to be 
touched by Linux, the recent clk_rcg2_ro_ops patch looks promising here.

>   	},
>   };
>   
> @@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
>   		.name = "gcc_gp1_clk_src",
>   		.parent_data = gcc_parent_data_1,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,

But why? GP clocks are not shared.
The 'why?' question applies to all such changes. As I wrote, please 
split & describe the reason.

>   	},
>   };
>   
> @@ -538,7 +528,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
>   		.name = "gcc_gp2_clk_src",
>   		.parent_data = gcc_parent_data_1,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -552,7 +542,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
>   		.name = "gcc_gp3_clk_src",
>   		.parent_data = gcc_parent_data_1,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -571,7 +561,7 @@ static struct clk_rcg2 gcc_pcie_0_aux_clk_src = {
>   		.name = "gcc_pcie_0_aux_clk_src",
>   		.parent_data = gcc_parent_data_3,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -591,7 +581,7 @@ static struct clk_rcg2 gcc_pcie_0_phy_rchng_clk_src = {
>   		.name = "gcc_pcie_0_phy_rchng_clk_src",
>   		.parent_data = gcc_parent_data_0,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -610,7 +600,7 @@ static struct clk_rcg2 gcc_pdm2_clk_src = {
>   		.name = "gcc_pdm2_clk_src",
>   		.parent_data = gcc_parent_data_0,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -632,7 +622,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s0_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s0_clk_src = {
> @@ -648,7 +638,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s1_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s1_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s1_clk_src = {
> @@ -664,7 +654,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s2_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s2_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s2_clk_src = {
> @@ -680,7 +670,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s3_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s3_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s3_clk_src = {
> @@ -696,7 +686,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s4_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s4_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s4_clk_src = {
> @@ -717,7 +707,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s5_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s5_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s5_clk_src = {
> @@ -733,7 +723,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s6_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s6_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s6_clk_src = {
> @@ -749,7 +739,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s7_clk_src_init = {
>   	.name = "gcc_qupv3_wrap0_s7_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap0_s7_clk_src = {
> @@ -765,7 +755,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s0_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s0_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s0_clk_src = {
> @@ -781,7 +771,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s1_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s1_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s1_clk_src = {
> @@ -797,7 +787,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s2_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s2_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s2_clk_src = {
> @@ -813,7 +803,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s3_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s3_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s3_clk_src = {
> @@ -829,7 +819,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s4_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s4_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s4_clk_src = {
> @@ -845,7 +835,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s5_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s5_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s5_clk_src = {
> @@ -861,7 +851,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s6_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s6_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s6_clk_src = {
> @@ -877,7 +867,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s7_clk_src_init = {
>   	.name = "gcc_qupv3_wrap1_s7_clk_src",
>   	.parent_data = gcc_parent_data_0,
>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>   };
>   
>   static struct clk_rcg2 gcc_qupv3_wrap1_s7_clk_src = {
> @@ -913,7 +903,7 @@ static struct clk_rcg2 gcc_sdcc5_apps_clk_src = {
>   		.name = "gcc_sdcc5_apps_clk_src",
>   		.parent_data = gcc_parent_data_8,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_8),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_floor_ops,
>   	},
>   };
>   
> @@ -932,7 +922,7 @@ static struct clk_rcg2 gcc_sdcc5_ice_core_clk_src = {
>   		.name = "gcc_sdcc5_ice_core_clk_src",
>   		.parent_data = gcc_parent_data_2,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_floor_ops,
>   	},
>   };
>   
> @@ -946,7 +936,7 @@ static struct clk_rcg2 gcc_sm_bus_xo_clk_src = {
>   		.name = "gcc_sm_bus_xo_clk_src",
>   		.parent_data = gcc_parent_data_2,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -965,7 +955,7 @@ static struct clk_rcg2 gcc_tsc_clk_src = {
>   		.name = "gcc_tsc_clk_src",
>   		.parent_data = gcc_parent_data_9,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_9),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -985,7 +975,7 @@ static struct clk_rcg2 gcc_usb30_prim_master_clk_src = {
>   		.name = "gcc_usb30_prim_master_clk_src",
>   		.parent_data = gcc_parent_data_0,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -999,7 +989,7 @@ static struct clk_rcg2 gcc_usb30_prim_mock_utmi_clk_src = {
>   		.name = "gcc_usb30_prim_mock_utmi_clk_src",
>   		.parent_data = gcc_parent_data_0,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -1013,7 +1003,7 @@ static struct clk_rcg2 gcc_usb3_prim_phy_aux_clk_src = {
>   		.name = "gcc_usb3_prim_phy_aux_clk_src",
>   		.parent_data = gcc_parent_data_3,
>   		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>   	},
>   };
>   
> @@ -1142,6 +1132,26 @@ static struct clk_branch gcc_ddrss_ecpri_dma_clk = {
>   	},
>   };
>   
> +static struct clk_branch gcc_ddrss_ecpri_gsi_clk = {
> +	.halt_reg = 0x54298,
> +	.halt_check = BRANCH_HALT_VOTED,
> +	.hwcg_reg = 0x54298,
> +	.hwcg_bit = 1,
> +	.clkr = {
> +		.enable_reg = 0x54298,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(const struct clk_init_data) {
> +			.name = "gcc_ddrss_ecpri_gsi_clk",
> +			.parent_hws = (const struct clk_hw*[]) {
> +				&gcc_aggre_noc_ecpri_gsi_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT,
> +			.ops = &clk_branch2_aon_ops,
> +		},
> +	},
> +};
> +
>   static struct clk_branch gcc_ecpri_ahb_clk = {
>   	.halt_reg = 0x3a008,
>   	.halt_check = BRANCH_HALT_VOTED,
> @@ -1458,14 +1468,13 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = {
>   
>   static struct clk_branch gcc_pcie_0_clkref_en = {
>   	.halt_reg = 0x9c004,
> -	.halt_bit = 31,

Why?

>   	.halt_check = BRANCH_HALT_ENABLE,
>   	.clkr = {
>   		.enable_reg = 0x9c004,
>   		.enable_mask = BIT(0),
>   		.hw.init = &(const struct clk_init_data) {
>   			.name = "gcc_pcie_0_clkref_en",
> -			.ops = &clk_branch_ops,
> +			.ops = &clk_branch2_ops,

Separate commit, Fixes tag.

>   		},
>   	},
>   };
> @@ -2285,14 +2294,13 @@ static struct clk_branch gcc_tsc_etu_clk = {
>   
>   static struct clk_branch gcc_usb2_clkref_en = {
>   	.halt_reg = 0x9c008,
> -	.halt_bit = 31,

Why?

>   	.halt_check = BRANCH_HALT_ENABLE,
>   	.clkr = {
>   		.enable_reg = 0x9c008,
>   		.enable_mask = BIT(0),
>   		.hw.init = &(const struct clk_init_data) {
>   			.name = "gcc_usb2_clkref_en",
> -			.ops = &clk_branch_ops,
> +			.ops = &clk_branch2_ops,

And here.

>   		},
>   	},
>   };
> @@ -2402,6 +2410,39 @@ static struct clk_branch gcc_usb3_prim_phy_pipe_clk = {
>   	},
>   };
>   
> +static struct gdsc pcie_0_gdsc = {
> +	.gdscr = 0x9d004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
> +	.pd = {
> +		.name = "gcc_pcie_0_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
> +static struct gdsc pcie_0_phy_gdsc = {
> +	.gdscr = 0x7c004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0x2,
> +	.pd = {
> +		.name = "gcc_pcie_0_phy_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
> +static struct gdsc usb30_prim_gdsc = {
> +	.gdscr = 0x49004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
> +	.pd = {
> +		.name = "gcc_usb30_prim_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
>   static struct clk_regmap *gcc_qdu1000_clocks[] = {
>   	[GCC_AGGRE_NOC_ECPRI_DMA_CLK] = &gcc_aggre_noc_ecpri_dma_clk.clkr,
>   	[GCC_AGGRE_NOC_ECPRI_DMA_CLK_SRC] = &gcc_aggre_noc_ecpri_dma_clk_src.clkr,
> @@ -2534,6 +2575,14 @@ static struct clk_regmap *gcc_qdu1000_clocks[] = {
>   	[GCC_AGGRE_NOC_ECPRI_GSI_CLK] = &gcc_aggre_noc_ecpri_gsi_clk.clkr,
>   	[GCC_PCIE_0_PHY_AUX_CLK_SRC] = &gcc_pcie_0_phy_aux_clk_src.clkr,
>   	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
> +	[GCC_GPLL1_OUT_EVEN] = &gcc_gpll1_out_even.clkr,
> +	[GCC_DDRSS_ECPRI_GSI_CLK] = NULL,
> +};
> +
> +static struct gdsc *gcc_qdu1000_gdscs[] = {
> +	[PCIE_0_GDSC] = &pcie_0_gdsc,
> +	[PCIE_0_PHY_GDSC] = &pcie_0_phy_gdsc,
> +	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
>   };
>   
>   static const struct qcom_reset_map gcc_qdu1000_resets[] = {
> @@ -2597,10 +2646,13 @@ static const struct qcom_cc_desc gcc_qdu1000_desc = {
>   	.num_clks = ARRAY_SIZE(gcc_qdu1000_clocks),
>   	.resets = gcc_qdu1000_resets,
>   	.num_resets = ARRAY_SIZE(gcc_qdu1000_resets),
> +	.gdscs = gcc_qdu1000_gdscs,
> +	.num_gdscs = ARRAY_SIZE(gcc_qdu1000_gdscs),
>   };
>   
>   static const struct of_device_id gcc_qdu1000_match_table[] = {
>   	{ .compatible = "qcom,qdu1000-gcc" },
> +	{ .compatible = "qcom,qdu1000-gcc-v2" },

What is the actual hardware version being shipped in the end-user 
devices? Generally we do support only the latest hw revision. If you 
want to support v1 for some reason, please invert the logic here:
make "qcom,qdu1000-gcc" be the latest and grates and provide a reason 
for supporting v1 (via "qcom,qdu1000-gcc-v1").

>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, gcc_qdu1000_match_table);
> @@ -2617,6 +2669,12 @@ static int gcc_qdu1000_probe(struct platform_device *pdev)
>   	/* Update FORCE_MEM_CORE_ON for gcc_pcie_0_mstr_axi_clk */
>   	regmap_update_bits(regmap, 0x9d024, BIT(14), BIT(14));
>   
> +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,qdu1000-gcc-v2")) {
> +		gcc_qdu1000_clocks[GCC_DDRSS_ECPRI_GSI_CLK] = &gcc_ddrss_ecpri_gsi_clk.clkr;
> +		gcc_pcie_0_clkref_en.halt_check = BRANCH_HALT_DELAY;
> +		gcc_usb2_clkref_en.halt_check = BRANCH_HALT_DELAY;
> +	}
> +
>   	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>   				       ARRAY_SIZE(gcc_dfs_clocks));
>   	if (ret)

-- 
With best wishes
Dmitry


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

* Re: [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-16 10:49 [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Imran Shaik
  2023-06-16 10:49 ` [PATCH 1/2] dt-bindings: clock: " Imran Shaik
  2023-06-16 10:49 ` [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs Imran Shaik
@ 2023-06-16 11:21 ` Konrad Dybcio
  2023-06-23 10:07   ` Imran Shaik
  2 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2023-06-16 11:21 UTC (permalink / raw
  To: Imran Shaik, Andy Gross, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On 16.06.2023 12:49, Imran Shaik wrote:
> Update GCC clocks and add support for GDSCs for QDU1000 and QRU1000 SoCs.
> Also, add support for v2 variant as well.
Does that imply the first submission concerned v1/pre-mass-production chips?

We usually don't support these upstream, as they are rather short-lived and
never (officially, anyway) escape Qualcomm internal..

Konrad
> 
> Imran Shaik (2):
>   dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
>   clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
> 
>  .../bindings/clock/qcom,qdu1000-gcc.yaml      |   6 +-
>  drivers/clk/qcom/gcc-qdu1000.c                | 162 ++++++++++++------
>  include/dt-bindings/clock/qcom,qdu1000-gcc.h  |   4 +-
>  3 files changed, 118 insertions(+), 54 deletions(-)
> 

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

* Re: [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-16 10:49 ` [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs Imran Shaik
  2023-06-16 11:20   ` Dmitry Baryshkov
@ 2023-06-16 11:22   ` Konrad Dybcio
  2023-06-23 10:12     ` Imran Shaik
  1 sibling, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2023-06-16 11:22 UTC (permalink / raw
  To: Imran Shaik, Andy Gross, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On 16.06.2023 12:49, Imran Shaik wrote:
> Update the GCC clocks and add support for GDSCs for QDU1000 and
> QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
> add support for v2 variant.
Too much for a single change. If somebody wants to bisect an issue, they
would have to edit chunks of this patch.

Konrad
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
> ---
>  drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
>  1 file changed, 110 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
> index 5051769ad90c..5d8125c0eacc 100644
> --- a/drivers/clk/qcom/gcc-qdu1000.c
> +++ b/drivers/clk/qcom/gcc-qdu1000.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  #include <linux/clk-provider.h>
> @@ -17,6 +17,7 @@
>  #include "clk-regmap-divider.h"
>  #include "clk-regmap-mux.h"
>  #include "clk-regmap-phy-mux.h"
> +#include "gdsc.h"
>  #include "reset.h"
>  
>  enum {
> @@ -370,16 +371,6 @@ static const struct clk_parent_data gcc_parent_data_6[] = {
>  	{ .index = DT_TCXO_IDX },
>  };
>  
> -static const struct parent_map gcc_parent_map_7[] = {
> -	{ P_PCIE_0_PIPE_CLK, 0 },
> -	{ P_BI_TCXO, 2 },
> -};
> -
> -static const struct clk_parent_data gcc_parent_data_7[] = {
> -	{ .index = DT_PCIE_0_PIPE_CLK_IDX },
> -	{ .index = DT_TCXO_IDX },
> -};
> -
>  static const struct parent_map gcc_parent_map_8[] = {
>  	{ P_BI_TCXO, 0 },
>  	{ P_GCC_GPLL0_OUT_MAIN, 1 },
> @@ -439,16 +430,15 @@ static struct clk_regmap_mux gcc_pcie_0_phy_aux_clk_src = {
>  	},
>  };
>  
> -static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
> +static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
>  	.reg = 0x9d064,
> -	.shift = 0,
> -	.width = 2,
> -	.parent_map = gcc_parent_map_7,
>  	.clkr = {
>  		.hw.init = &(const struct clk_init_data) {
>  			.name = "gcc_pcie_0_pipe_clk_src",
> -			.parent_data = gcc_parent_data_7,
> -			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
> +			.parent_data = &(const struct clk_parent_data){
> +				.index = DT_PCIE_0_PIPE_CLK_IDX,
> +			},
> +			.num_parents = 1,
>  			.ops = &clk_regmap_phy_mux_ops,
>  		},
>  	},
> @@ -485,7 +475,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_dma_clk_src = {
>  		.name = "gcc_aggre_noc_ecpri_dma_clk_src",
>  		.parent_data = gcc_parent_data_4,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_4),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -505,7 +495,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_gsi_clk_src = {
>  		.name = "gcc_aggre_noc_ecpri_gsi_clk_src",
>  		.parent_data = gcc_parent_data_5,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
>  		.name = "gcc_gp1_clk_src",
>  		.parent_data = gcc_parent_data_1,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -538,7 +528,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
>  		.name = "gcc_gp2_clk_src",
>  		.parent_data = gcc_parent_data_1,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -552,7 +542,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
>  		.name = "gcc_gp3_clk_src",
>  		.parent_data = gcc_parent_data_1,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -571,7 +561,7 @@ static struct clk_rcg2 gcc_pcie_0_aux_clk_src = {
>  		.name = "gcc_pcie_0_aux_clk_src",
>  		.parent_data = gcc_parent_data_3,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -591,7 +581,7 @@ static struct clk_rcg2 gcc_pcie_0_phy_rchng_clk_src = {
>  		.name = "gcc_pcie_0_phy_rchng_clk_src",
>  		.parent_data = gcc_parent_data_0,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -610,7 +600,7 @@ static struct clk_rcg2 gcc_pdm2_clk_src = {
>  		.name = "gcc_pdm2_clk_src",
>  		.parent_data = gcc_parent_data_0,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -632,7 +622,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s0_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s0_clk_src = {
> @@ -648,7 +638,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s1_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s1_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s1_clk_src = {
> @@ -664,7 +654,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s2_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s2_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s2_clk_src = {
> @@ -680,7 +670,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s3_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s3_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s3_clk_src = {
> @@ -696,7 +686,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s4_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s4_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s4_clk_src = {
> @@ -717,7 +707,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s5_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s5_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s5_clk_src = {
> @@ -733,7 +723,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s6_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s6_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s6_clk_src = {
> @@ -749,7 +739,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s7_clk_src_init = {
>  	.name = "gcc_qupv3_wrap0_s7_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap0_s7_clk_src = {
> @@ -765,7 +755,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s0_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s0_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s0_clk_src = {
> @@ -781,7 +771,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s1_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s1_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s1_clk_src = {
> @@ -797,7 +787,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s2_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s2_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s2_clk_src = {
> @@ -813,7 +803,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s3_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s3_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s3_clk_src = {
> @@ -829,7 +819,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s4_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s4_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s4_clk_src = {
> @@ -845,7 +835,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s5_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s5_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s5_clk_src = {
> @@ -861,7 +851,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s6_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s6_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s6_clk_src = {
> @@ -877,7 +867,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s7_clk_src_init = {
>  	.name = "gcc_qupv3_wrap1_s7_clk_src",
>  	.parent_data = gcc_parent_data_0,
>  	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -	.ops = &clk_rcg2_ops,
> +	.ops = &clk_rcg2_shared_ops,
>  };
>  
>  static struct clk_rcg2 gcc_qupv3_wrap1_s7_clk_src = {
> @@ -913,7 +903,7 @@ static struct clk_rcg2 gcc_sdcc5_apps_clk_src = {
>  		.name = "gcc_sdcc5_apps_clk_src",
>  		.parent_data = gcc_parent_data_8,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_8),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_floor_ops,
>  	},
>  };
>  
> @@ -932,7 +922,7 @@ static struct clk_rcg2 gcc_sdcc5_ice_core_clk_src = {
>  		.name = "gcc_sdcc5_ice_core_clk_src",
>  		.parent_data = gcc_parent_data_2,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_floor_ops,
>  	},
>  };
>  
> @@ -946,7 +936,7 @@ static struct clk_rcg2 gcc_sm_bus_xo_clk_src = {
>  		.name = "gcc_sm_bus_xo_clk_src",
>  		.parent_data = gcc_parent_data_2,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -965,7 +955,7 @@ static struct clk_rcg2 gcc_tsc_clk_src = {
>  		.name = "gcc_tsc_clk_src",
>  		.parent_data = gcc_parent_data_9,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_9),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -985,7 +975,7 @@ static struct clk_rcg2 gcc_usb30_prim_master_clk_src = {
>  		.name = "gcc_usb30_prim_master_clk_src",
>  		.parent_data = gcc_parent_data_0,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -999,7 +989,7 @@ static struct clk_rcg2 gcc_usb30_prim_mock_utmi_clk_src = {
>  		.name = "gcc_usb30_prim_mock_utmi_clk_src",
>  		.parent_data = gcc_parent_data_0,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -1013,7 +1003,7 @@ static struct clk_rcg2 gcc_usb3_prim_phy_aux_clk_src = {
>  		.name = "gcc_usb3_prim_phy_aux_clk_src",
>  		.parent_data = gcc_parent_data_3,
>  		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_shared_ops,
>  	},
>  };
>  
> @@ -1142,6 +1132,26 @@ static struct clk_branch gcc_ddrss_ecpri_dma_clk = {
>  	},
>  };
>  
> +static struct clk_branch gcc_ddrss_ecpri_gsi_clk = {
> +	.halt_reg = 0x54298,
> +	.halt_check = BRANCH_HALT_VOTED,
> +	.hwcg_reg = 0x54298,
> +	.hwcg_bit = 1,
> +	.clkr = {
> +		.enable_reg = 0x54298,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(const struct clk_init_data) {
> +			.name = "gcc_ddrss_ecpri_gsi_clk",
> +			.parent_hws = (const struct clk_hw*[]) {
> +				&gcc_aggre_noc_ecpri_gsi_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			.flags = CLK_SET_RATE_PARENT,
> +			.ops = &clk_branch2_aon_ops,
> +		},
> +	},
> +};
> +
>  static struct clk_branch gcc_ecpri_ahb_clk = {
>  	.halt_reg = 0x3a008,
>  	.halt_check = BRANCH_HALT_VOTED,
> @@ -1458,14 +1468,13 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = {
>  
>  static struct clk_branch gcc_pcie_0_clkref_en = {
>  	.halt_reg = 0x9c004,
> -	.halt_bit = 31,
>  	.halt_check = BRANCH_HALT_ENABLE,
>  	.clkr = {
>  		.enable_reg = 0x9c004,
>  		.enable_mask = BIT(0),
>  		.hw.init = &(const struct clk_init_data) {
>  			.name = "gcc_pcie_0_clkref_en",
> -			.ops = &clk_branch_ops,
> +			.ops = &clk_branch2_ops,
>  		},
>  	},
>  };
> @@ -2285,14 +2294,13 @@ static struct clk_branch gcc_tsc_etu_clk = {
>  
>  static struct clk_branch gcc_usb2_clkref_en = {
>  	.halt_reg = 0x9c008,
> -	.halt_bit = 31,
>  	.halt_check = BRANCH_HALT_ENABLE,
>  	.clkr = {
>  		.enable_reg = 0x9c008,
>  		.enable_mask = BIT(0),
>  		.hw.init = &(const struct clk_init_data) {
>  			.name = "gcc_usb2_clkref_en",
> -			.ops = &clk_branch_ops,
> +			.ops = &clk_branch2_ops,
>  		},
>  	},
>  };
> @@ -2402,6 +2410,39 @@ static struct clk_branch gcc_usb3_prim_phy_pipe_clk = {
>  	},
>  };
>  
> +static struct gdsc pcie_0_gdsc = {
> +	.gdscr = 0x9d004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
> +	.pd = {
> +		.name = "gcc_pcie_0_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
> +static struct gdsc pcie_0_phy_gdsc = {
> +	.gdscr = 0x7c004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0x2,
> +	.pd = {
> +		.name = "gcc_pcie_0_phy_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
> +static struct gdsc usb30_prim_gdsc = {
> +	.gdscr = 0x49004,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
> +	.pd = {
> +		.name = "gcc_usb30_prim_gdsc",
> +	},
> +	.pwrsts = PWRSTS_OFF_ON,
> +};
> +
>  static struct clk_regmap *gcc_qdu1000_clocks[] = {
>  	[GCC_AGGRE_NOC_ECPRI_DMA_CLK] = &gcc_aggre_noc_ecpri_dma_clk.clkr,
>  	[GCC_AGGRE_NOC_ECPRI_DMA_CLK_SRC] = &gcc_aggre_noc_ecpri_dma_clk_src.clkr,
> @@ -2534,6 +2575,14 @@ static struct clk_regmap *gcc_qdu1000_clocks[] = {
>  	[GCC_AGGRE_NOC_ECPRI_GSI_CLK] = &gcc_aggre_noc_ecpri_gsi_clk.clkr,
>  	[GCC_PCIE_0_PHY_AUX_CLK_SRC] = &gcc_pcie_0_phy_aux_clk_src.clkr,
>  	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
> +	[GCC_GPLL1_OUT_EVEN] = &gcc_gpll1_out_even.clkr,
> +	[GCC_DDRSS_ECPRI_GSI_CLK] = NULL,
> +};
> +
> +static struct gdsc *gcc_qdu1000_gdscs[] = {
> +	[PCIE_0_GDSC] = &pcie_0_gdsc,
> +	[PCIE_0_PHY_GDSC] = &pcie_0_phy_gdsc,
> +	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
>  };
>  
>  static const struct qcom_reset_map gcc_qdu1000_resets[] = {
> @@ -2597,10 +2646,13 @@ static const struct qcom_cc_desc gcc_qdu1000_desc = {
>  	.num_clks = ARRAY_SIZE(gcc_qdu1000_clocks),
>  	.resets = gcc_qdu1000_resets,
>  	.num_resets = ARRAY_SIZE(gcc_qdu1000_resets),
> +	.gdscs = gcc_qdu1000_gdscs,
> +	.num_gdscs = ARRAY_SIZE(gcc_qdu1000_gdscs),
>  };
>  
>  static const struct of_device_id gcc_qdu1000_match_table[] = {
>  	{ .compatible = "qcom,qdu1000-gcc" },
> +	{ .compatible = "qcom,qdu1000-gcc-v2" },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, gcc_qdu1000_match_table);
> @@ -2617,6 +2669,12 @@ static int gcc_qdu1000_probe(struct platform_device *pdev)
>  	/* Update FORCE_MEM_CORE_ON for gcc_pcie_0_mstr_axi_clk */
>  	regmap_update_bits(regmap, 0x9d024, BIT(14), BIT(14));
>  
> +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,qdu1000-gcc-v2")) {
> +		gcc_qdu1000_clocks[GCC_DDRSS_ECPRI_GSI_CLK] = &gcc_ddrss_ecpri_gsi_clk.clkr;
> +		gcc_pcie_0_clkref_en.halt_check = BRANCH_HALT_DELAY;
> +		gcc_usb2_clkref_en.halt_check = BRANCH_HALT_DELAY;
> +	}
> +
>  	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>  				       ARRAY_SIZE(gcc_dfs_clocks));
>  	if (ret)

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

* Re: [PATCH 1/2] dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-16 10:49 ` [PATCH 1/2] dt-bindings: clock: " Imran Shaik
@ 2023-06-16 11:33   ` Krzysztof Kozlowski
  2023-06-22 13:45     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-16 11:33 UTC (permalink / raw
  To: Imran Shaik, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On 16/06/2023 12:49, Imran Shaik wrote:
> Update the qcom GCC clock bindings and add v2 compatible string for QDU1000
> and QRU1000 SoCs.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
> ---
>  .../devicetree/bindings/clock/qcom,qdu1000-gcc.yaml         | 6 +++++-
>  include/dt-bindings/clock/qcom,qdu1000-gcc.h                | 4 +++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
> index 767a9d03aa32..030953d258c1 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
> @@ -8,6 +8,8 @@ title: Qualcomm Global Clock & Reset Controller for QDU1000 and QRU1000
>  
>  maintainers:
>    - Melody Olvera <quic_molvera@quicinc.com>
> +  - Taniya Das <quic_tdas@quicinc.com>
> +  - Imran Shaik <quic_imrashai@quicinc.com>

I appreciate adding more maintainers, it is welcomed and needed.

However many of Qualcomm folks, including some of you, did not care
enough to fix their old/incorrect email in existing entries, thus we
have hundreds of wrong addresses and email bounces.

We already raised this internally and publicly, with just small effect,
so I am not sure what to do more. For me, allowing to have outdated
email in maintainers is an easiest proof that maintainer does not care.
Adding more maintainer entries, while maintainer does not care, would
not feel right. Maybe let's start with fixing existing entries?

>  
>  description: |
>    Qualcomm global clock control module which supports the clocks, resets and
> @@ -17,7 +19,9 @@ description: |
>  
>  properties:
>    compatible:
> -    const: qcom,qdu1000-gcc
> +    enum:
> +      - qcom,qdu1000-gcc
> +      - qcom,qdu1000-gcc-v2

It's the same block, isn't it? What is the "v2" exactly?

Best regards,
Krzysztof


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

* Re: [PATCH 1/2] dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-16 11:33   ` Krzysztof Kozlowski
@ 2023-06-22 13:45     ` Krzysztof Kozlowski
  2023-06-23 10:07       ` Imran Shaik
  0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-22 13:45 UTC (permalink / raw
  To: Imran Shaik, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On 16/06/2023 13:33, Krzysztof Kozlowski wrote:
> On 16/06/2023 12:49, Imran Shaik wrote:
>> Update the qcom GCC clock bindings and add v2 compatible string for QDU1000
>> and QRU1000 SoCs.
>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
>> ---
>>  .../devicetree/bindings/clock/qcom,qdu1000-gcc.yaml         | 6 +++++-
>>  include/dt-bindings/clock/qcom,qdu1000-gcc.h                | 4 +++-
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>> index 767a9d03aa32..030953d258c1 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>> @@ -8,6 +8,8 @@ title: Qualcomm Global Clock & Reset Controller for QDU1000 and QRU1000
>>  
>>  maintainers:
>>    - Melody Olvera <quic_molvera@quicinc.com>
>> +  - Taniya Das <quic_tdas@quicinc.com>
>> +  - Imran Shaik <quic_imrashai@quicinc.com>
> 
> I appreciate adding more maintainers, it is welcomed and needed.
> 
> However many of Qualcomm folks, including some of you, did not care
> enough to fix their old/incorrect email in existing entries, thus we
> have hundreds of wrong addresses and email bounces.
> 
> We already raised this internally and publicly, with just small effect,
> so I am not sure what to do more. For me, allowing to have outdated
> email in maintainers is an easiest proof that maintainer does not care.
> Adding more maintainer entries, while maintainer does not care, would
> not feel right. Maybe let's start with fixing existing entries?

+Cc Alex,

Let me emphasize more, because I did not see any follow up patches since
my previous email - there are many, many stale maintainer entries from
Qualcomm. Several of them have codeaurora.org email. Some have just old
emails of folks who left.

Can we expect fixing these?

Best regards,
Krzysztof


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

* Re: [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-16 11:21 ` [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Konrad Dybcio
@ 2023-06-23 10:07   ` Imran Shaik
  0 siblings, 0 replies; 13+ messages in thread
From: Imran Shaik @ 2023-06-23 10:07 UTC (permalink / raw
  To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey



On 6/16/2023 4:51 PM, Konrad Dybcio wrote:
> On 16.06.2023 12:49, Imran Shaik wrote:
>> Update GCC clocks and add support for GDSCs for QDU1000 and QRU1000 SoCs.
>> Also, add support for v2 variant as well.
> Does that imply the first submission concerned v1/pre-mass-production chips?
> 
> We usually don't support these upstream, as they are rather short-lived and
> never (officially, anyway) escape Qualcomm internal..
> 
> Konrad

Sure, will update the next series to support only the latest hardware 
version.

Thanks,
Imran

>>
>> Imran Shaik (2):
>>    dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
>>    clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
>>
>>   .../bindings/clock/qcom,qdu1000-gcc.yaml      |   6 +-
>>   drivers/clk/qcom/gcc-qdu1000.c                | 162 ++++++++++++------
>>   include/dt-bindings/clock/qcom,qdu1000-gcc.h  |   4 +-
>>   3 files changed, 118 insertions(+), 54 deletions(-)
>>

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

* Re: [PATCH 1/2] dt-bindings: clock: Update GCC clocks for QDU1000 and QRU1000 SoCs
  2023-06-22 13:45     ` Krzysztof Kozlowski
@ 2023-06-23 10:07       ` Imran Shaik
  0 siblings, 0 replies; 13+ messages in thread
From: Imran Shaik @ 2023-06-23 10:07 UTC (permalink / raw
  To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey



On 6/22/2023 7:15 PM, Krzysztof Kozlowski wrote:
> On 16/06/2023 13:33, Krzysztof Kozlowski wrote:
>> On 16/06/2023 12:49, Imran Shaik wrote:
>>> Update the qcom GCC clock bindings and add v2 compatible string for QDU1000
>>> and QRU1000 SoCs.
>>>
>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>>> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
>>> ---
>>>   .../devicetree/bindings/clock/qcom,qdu1000-gcc.yaml         | 6 +++++-
>>>   include/dt-bindings/clock/qcom,qdu1000-gcc.h                | 4 +++-
>>>   2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>>> index 767a9d03aa32..030953d258c1 100644
>>> --- a/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,qdu1000-gcc.yaml
>>> @@ -8,6 +8,8 @@ title: Qualcomm Global Clock & Reset Controller for QDU1000 and QRU1000
>>>   
>>>   maintainers:
>>>     - Melody Olvera <quic_molvera@quicinc.com>
>>> +  - Taniya Das <quic_tdas@quicinc.com>
>>> +  - Imran Shaik <quic_imrashai@quicinc.com>
>>
>> I appreciate adding more maintainers, it is welcomed and needed.
>>
>> However many of Qualcomm folks, including some of you, did not care
>> enough to fix their old/incorrect email in existing entries, thus we
>> have hundreds of wrong addresses and email bounces.
>>
>> We already raised this internally and publicly, with just small effect,
>> so I am not sure what to do more. For me, allowing to have outdated
>> email in maintainers is an easiest proof that maintainer does not care.
>> Adding more maintainer entries, while maintainer does not care, would
>> not feel right. Maybe let's start with fixing existing entries?
> 
> +Cc Alex,
> 
> Let me emphasize more, because I did not see any follow up patches since
> my previous email - there are many, many stale maintainer entries from
> Qualcomm. Several of them have codeaurora.org email. Some have just old
> emails of folks who left.
> 
> Can we expect fixing these?

Sure, will post a separate clean up patch for fixing all the 
old/incorrect maintainers email addresses from all the binding files.

Thanks,
Imran

> 
> Best regards,
> Krzysztof
> 

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

* Re: [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-16 11:20   ` Dmitry Baryshkov
@ 2023-06-23 10:08     ` Imran Shaik
  2023-06-23 14:06       ` Dmitry Baryshkov
  0 siblings, 1 reply; 13+ messages in thread
From: Imran Shaik @ 2023-06-23 10:08 UTC (permalink / raw
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey



On 6/16/2023 4:50 PM, Dmitry Baryshkov wrote:
> On 16/06/2023 13:49, Imran Shaik wrote:
>> Update the GCC clocks and add support for GDSCs for QDU1000 and
>> QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
>> add support for v2 variant.
> 
> Please split this into individual chunks instead of squashing everything 
> together. For each change please describe the logic behind the change in 
> the commit message. Please describe why, not what is changed.
> 

Sure, will split this patch and post the next series.

>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
> 
> This doesn't look fully logical. Who is the author of the patch? If 
> there are two authors, please add Co-developed-by tag.
> 

Yes, will use Co-developed-by tag in the next patch series.

>> ---
>>   drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
>>   1 file changed, 110 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-qdu1000.c 
>> b/drivers/clk/qcom/gcc-qdu1000.c
>> index 5051769ad90c..5d8125c0eacc 100644
>> --- a/drivers/clk/qcom/gcc-qdu1000.c
>> +++ b/drivers/clk/qcom/gcc-qdu1000.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights 
>> reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All 
>> rights reserved.
>>    */
>>   #include <linux/clk-provider.h>
>> @@ -17,6 +17,7 @@
>>   #include "clk-regmap-divider.h"
>>   #include "clk-regmap-mux.h"
>>   #include "clk-regmap-phy-mux.h"
>> +#include "gdsc.h"
>>   #include "reset.h"
>>   enum {
>> @@ -370,16 +371,6 @@ static const struct clk_parent_data 
>> gcc_parent_data_6[] = {
>>       { .index = DT_TCXO_IDX },
>>   };
>> -static const struct parent_map gcc_parent_map_7[] = {
>> -    { P_PCIE_0_PIPE_CLK, 0 },
>> -    { P_BI_TCXO, 2 },
>> -};
>> -
>> -static const struct clk_parent_data gcc_parent_data_7[] = {
>> -    { .index = DT_PCIE_0_PIPE_CLK_IDX },
>> -    { .index = DT_TCXO_IDX },
>> -};
>> -
>>   static const struct parent_map gcc_parent_map_8[] = {
>>       { P_BI_TCXO, 0 },
>>       { P_GCC_GPLL0_OUT_MAIN, 1 },
>> @@ -439,16 +430,15 @@ static struct clk_regmap_mux 
>> gcc_pcie_0_phy_aux_clk_src = {
>>       },
>>   };
>> -static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
>> +static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
>>       .reg = 0x9d064,
>> -    .shift = 0,
>> -    .width = 2,
>> -    .parent_map = gcc_parent_map_7,
>>       .clkr = {
>>           .hw.init = &(const struct clk_init_data) {
>>               .name = "gcc_pcie_0_pipe_clk_src",
>> -            .parent_data = gcc_parent_data_7,
>> -            .num_parents = ARRAY_SIZE(gcc_parent_data_7),
>> +            .parent_data = &(const struct clk_parent_data){
>> +                .index = DT_PCIE_0_PIPE_CLK_IDX,
>> +            },
>> +            .num_parents = 1,
>>               .ops = &clk_regmap_phy_mux_ops,
>>           },
>>       },
>> @@ -485,7 +475,7 @@ static struct clk_rcg2 
>> gcc_aggre_noc_ecpri_dma_clk_src = {
>>           .name = "gcc_aggre_noc_ecpri_dma_clk_src",
>>           .parent_data = gcc_parent_data_4,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_4),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -505,7 +495,7 @@ static struct clk_rcg2 
>> gcc_aggre_noc_ecpri_gsi_clk_src = {
>>           .name = "gcc_aggre_noc_ecpri_gsi_clk_src",
>>           .parent_data = gcc_parent_data_5,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_5),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
> 
> This is probably some kind of NoC or NIU clock. If it is not to be 
> touched by Linux, the recent clk_rcg2_ro_ops patch looks promising here.
> 

This is not a NoC or NIU clock and Linux will use this clock.

>>       },
>>   };
>> @@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
>>           .name = "gcc_gp1_clk_src",
>>           .parent_data = gcc_parent_data_1,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
> 
> But why? GP clocks are not shared.
> The 'why?' question applies to all such changes. As I wrote, please 
> split & describe the reason.
> 

We want to park all the RCGs at safe clock source (XO) during disable. 
Hence using the clk_rcg2_shared_ops for all the RCGs.

Will split and post the next patch series.

>>       },
>>   };
>> @@ -538,7 +528,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
>>           .name = "gcc_gp2_clk_src",
>>           .parent_data = gcc_parent_data_1,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -552,7 +542,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
>>           .name = "gcc_gp3_clk_src",
>>           .parent_data = gcc_parent_data_1,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -571,7 +561,7 @@ static struct clk_rcg2 gcc_pcie_0_aux_clk_src = {
>>           .name = "gcc_pcie_0_aux_clk_src",
>>           .parent_data = gcc_parent_data_3,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_3),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -591,7 +581,7 @@ static struct clk_rcg2 
>> gcc_pcie_0_phy_rchng_clk_src = {
>>           .name = "gcc_pcie_0_phy_rchng_clk_src",
>>           .parent_data = gcc_parent_data_0,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -610,7 +600,7 @@ static struct clk_rcg2 gcc_pdm2_clk_src = {
>>           .name = "gcc_pdm2_clk_src",
>>           .parent_data = gcc_parent_data_0,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -632,7 +622,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s0_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s0_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s0_clk_src = {
>> @@ -648,7 +638,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s1_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s1_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s1_clk_src = {
>> @@ -664,7 +654,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s2_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s2_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s2_clk_src = {
>> @@ -680,7 +670,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s3_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s3_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s3_clk_src = {
>> @@ -696,7 +686,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s4_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s4_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s4_clk_src = {
>> @@ -717,7 +707,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s5_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s5_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s5_clk_src = {
>> @@ -733,7 +723,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s6_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s6_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s6_clk_src = {
>> @@ -749,7 +739,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap0_s7_clk_src_init = {
>>       .name = "gcc_qupv3_wrap0_s7_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s7_clk_src = {
>> @@ -765,7 +755,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s0_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s0_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s0_clk_src = {
>> @@ -781,7 +771,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s1_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s1_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s1_clk_src = {
>> @@ -797,7 +787,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s2_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s2_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s2_clk_src = {
>> @@ -813,7 +803,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s3_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s3_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s3_clk_src = {
>> @@ -829,7 +819,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s4_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s4_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s4_clk_src = {
>> @@ -845,7 +835,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s5_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s5_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s5_clk_src = {
>> @@ -861,7 +851,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s6_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s6_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s6_clk_src = {
>> @@ -877,7 +867,7 @@ static struct clk_init_data 
>> gcc_qupv3_wrap1_s7_clk_src_init = {
>>       .name = "gcc_qupv3_wrap1_s7_clk_src",
>>       .parent_data = gcc_parent_data_0,
>>       .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -    .ops = &clk_rcg2_ops,
>> +    .ops = &clk_rcg2_shared_ops,
>>   };
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s7_clk_src = {
>> @@ -913,7 +903,7 @@ static struct clk_rcg2 gcc_sdcc5_apps_clk_src = {
>>           .name = "gcc_sdcc5_apps_clk_src",
>>           .parent_data = gcc_parent_data_8,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_8),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_floor_ops,
>>       },
>>   };
>> @@ -932,7 +922,7 @@ static struct clk_rcg2 gcc_sdcc5_ice_core_clk_src = {
>>           .name = "gcc_sdcc5_ice_core_clk_src",
>>           .parent_data = gcc_parent_data_2,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_2),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_floor_ops,
>>       },
>>   };
>> @@ -946,7 +936,7 @@ static struct clk_rcg2 gcc_sm_bus_xo_clk_src = {
>>           .name = "gcc_sm_bus_xo_clk_src",
>>           .parent_data = gcc_parent_data_2,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_2),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -965,7 +955,7 @@ static struct clk_rcg2 gcc_tsc_clk_src = {
>>           .name = "gcc_tsc_clk_src",
>>           .parent_data = gcc_parent_data_9,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_9),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -985,7 +975,7 @@ static struct clk_rcg2 
>> gcc_usb30_prim_master_clk_src = {
>>           .name = "gcc_usb30_prim_master_clk_src",
>>           .parent_data = gcc_parent_data_0,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -999,7 +989,7 @@ static struct clk_rcg2 
>> gcc_usb30_prim_mock_utmi_clk_src = {
>>           .name = "gcc_usb30_prim_mock_utmi_clk_src",
>>           .parent_data = gcc_parent_data_0,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -1013,7 +1003,7 @@ static struct clk_rcg2 
>> gcc_usb3_prim_phy_aux_clk_src = {
>>           .name = "gcc_usb3_prim_phy_aux_clk_src",
>>           .parent_data = gcc_parent_data_3,
>>           .num_parents = ARRAY_SIZE(gcc_parent_data_3),
>> -        .ops = &clk_rcg2_ops,
>> +        .ops = &clk_rcg2_shared_ops,
>>       },
>>   };
>> @@ -1142,6 +1132,26 @@ static struct clk_branch 
>> gcc_ddrss_ecpri_dma_clk = {
>>       },
>>   };
>> +static struct clk_branch gcc_ddrss_ecpri_gsi_clk = {
>> +    .halt_reg = 0x54298,
>> +    .halt_check = BRANCH_HALT_VOTED,
>> +    .hwcg_reg = 0x54298,
>> +    .hwcg_bit = 1,
>> +    .clkr = {
>> +        .enable_reg = 0x54298,
>> +        .enable_mask = BIT(0),
>> +        .hw.init = &(const struct clk_init_data) {
>> +            .name = "gcc_ddrss_ecpri_gsi_clk",
>> +            .parent_hws = (const struct clk_hw*[]) {
>> +                &gcc_aggre_noc_ecpri_gsi_clk_src.clkr.hw,
>> +            },
>> +            .num_parents = 1,
>> +            .flags = CLK_SET_RATE_PARENT,
>> +            .ops = &clk_branch2_aon_ops,
>> +        },
>> +    },
>> +};
>> +
>>   static struct clk_branch gcc_ecpri_ahb_clk = {
>>       .halt_reg = 0x3a008,
>>       .halt_check = BRANCH_HALT_VOTED,
>> @@ -1458,14 +1468,13 @@ static struct clk_branch 
>> gcc_pcie_0_cfg_ahb_clk = {
>>   static struct clk_branch gcc_pcie_0_clkref_en = {
>>       .halt_reg = 0x9c004,
>> -    .halt_bit = 31,
> 
> Why?
> 

clk_branch2_ops doesn't require the halt_bit. Hence removed the same.

>>       .halt_check = BRANCH_HALT_ENABLE,
>>       .clkr = {
>>           .enable_reg = 0x9c004,
>>           .enable_mask = BIT(0),
>>           .hw.init = &(const struct clk_init_data) {
>>               .name = "gcc_pcie_0_clkref_en",
>> -            .ops = &clk_branch_ops,
>> +            .ops = &clk_branch2_ops,
> 
> Separate commit, Fixes tag.
> 

Sure, will push this change in a seperate commit in the next series.

>>           },
>>       },
>>   };
>> @@ -2285,14 +2294,13 @@ static struct clk_branch gcc_tsc_etu_clk = {
>>   static struct clk_branch gcc_usb2_clkref_en = {
>>       .halt_reg = 0x9c008,
>> -    .halt_bit = 31,
> 
> Why?
> 

clk_branch2_ops doesn't require the halt_bit. Hence removed the same.

>>       .halt_check = BRANCH_HALT_ENABLE,
>>       .clkr = {
>>           .enable_reg = 0x9c008,
>>           .enable_mask = BIT(0),
>>           .hw.init = &(const struct clk_init_data) {
>>               .name = "gcc_usb2_clkref_en",
>> -            .ops = &clk_branch_ops,
>> +            .ops = &clk_branch2_ops,
> 
> And here.
> 

Will push this change in a seperate commit in the next series.

>>           },
>>       },
>>   };
>> @@ -2402,6 +2410,39 @@ static struct clk_branch 
>> gcc_usb3_prim_phy_pipe_clk = {
>>       },
>>   };
>> +static struct gdsc pcie_0_gdsc = {
>> +    .gdscr = 0x9d004,
>> +    .en_rest_wait_val = 0x2,
>> +    .en_few_wait_val = 0x2,
>> +    .clk_dis_wait_val = 0xf,
>> +    .pd = {
>> +        .name = "gcc_pcie_0_gdsc",
>> +    },
>> +    .pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>> +static struct gdsc pcie_0_phy_gdsc = {
>> +    .gdscr = 0x7c004,
>> +    .en_rest_wait_val = 0x2,
>> +    .en_few_wait_val = 0x2,
>> +    .clk_dis_wait_val = 0x2,
>> +    .pd = {
>> +        .name = "gcc_pcie_0_phy_gdsc",
>> +    },
>> +    .pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>> +static struct gdsc usb30_prim_gdsc = {
>> +    .gdscr = 0x49004,
>> +    .en_rest_wait_val = 0x2,
>> +    .en_few_wait_val = 0x2,
>> +    .clk_dis_wait_val = 0xf,
>> +    .pd = {
>> +        .name = "gcc_usb30_prim_gdsc",
>> +    },
>> +    .pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>>   static struct clk_regmap *gcc_qdu1000_clocks[] = {
>>       [GCC_AGGRE_NOC_ECPRI_DMA_CLK] = &gcc_aggre_noc_ecpri_dma_clk.clkr,
>>       [GCC_AGGRE_NOC_ECPRI_DMA_CLK_SRC] = 
>> &gcc_aggre_noc_ecpri_dma_clk_src.clkr,
>> @@ -2534,6 +2575,14 @@ static struct clk_regmap *gcc_qdu1000_clocks[] = {
>>       [GCC_AGGRE_NOC_ECPRI_GSI_CLK] = &gcc_aggre_noc_ecpri_gsi_clk.clkr,
>>       [GCC_PCIE_0_PHY_AUX_CLK_SRC] = &gcc_pcie_0_phy_aux_clk_src.clkr,
>>       [GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
>> +    [GCC_GPLL1_OUT_EVEN] = &gcc_gpll1_out_even.clkr,
>> +    [GCC_DDRSS_ECPRI_GSI_CLK] = NULL,
>> +};
>> +
>> +static struct gdsc *gcc_qdu1000_gdscs[] = {
>> +    [PCIE_0_GDSC] = &pcie_0_gdsc,
>> +    [PCIE_0_PHY_GDSC] = &pcie_0_phy_gdsc,
>> +    [USB30_PRIM_GDSC] = &usb30_prim_gdsc,
>>   };
>>   static const struct qcom_reset_map gcc_qdu1000_resets[] = {
>> @@ -2597,10 +2646,13 @@ static const struct qcom_cc_desc 
>> gcc_qdu1000_desc = {
>>       .num_clks = ARRAY_SIZE(gcc_qdu1000_clocks),
>>       .resets = gcc_qdu1000_resets,
>>       .num_resets = ARRAY_SIZE(gcc_qdu1000_resets),
>> +    .gdscs = gcc_qdu1000_gdscs,
>> +    .num_gdscs = ARRAY_SIZE(gcc_qdu1000_gdscs),
>>   };
>>   static const struct of_device_id gcc_qdu1000_match_table[] = {
>>       { .compatible = "qcom,qdu1000-gcc" },
>> +    { .compatible = "qcom,qdu1000-gcc-v2" },
> 
> What is the actual hardware version being shipped in the end-user 
> devices? Generally we do support only the latest hw revision. If you 
> want to support v1 for some reason, please invert the logic here:
> make "qcom,qdu1000-gcc" be the latest and grates and provide a reason 
> for supporting v1 (via "qcom,qdu1000-gcc-v1").
> 

Will update the next series with the changes to support only latest 
hardware version.

Thanks,
Imran

>>       { }
>>   };
>>   MODULE_DEVICE_TABLE(of, gcc_qdu1000_match_table);
>> @@ -2617,6 +2669,12 @@ static int gcc_qdu1000_probe(struct 
>> platform_device *pdev)
>>       /* Update FORCE_MEM_CORE_ON for gcc_pcie_0_mstr_axi_clk */
>>       regmap_update_bits(regmap, 0x9d024, BIT(14), BIT(14));
>> +    if (of_device_is_compatible(pdev->dev.of_node, 
>> "qcom,qdu1000-gcc-v2")) {
>> +        gcc_qdu1000_clocks[GCC_DDRSS_ECPRI_GSI_CLK] = 
>> &gcc_ddrss_ecpri_gsi_clk.clkr;
>> +        gcc_pcie_0_clkref_en.halt_check = BRANCH_HALT_DELAY;
>> +        gcc_usb2_clkref_en.halt_check = BRANCH_HALT_DELAY;
>> +    }
>> +
>>       ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>>                          ARRAY_SIZE(gcc_dfs_clocks));
>>       if (ret)
> 

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

* Re: [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-16 11:22   ` Konrad Dybcio
@ 2023-06-23 10:12     ` Imran Shaik
  0 siblings, 0 replies; 13+ messages in thread
From: Imran Shaik @ 2023-06-23 10:12 UTC (permalink / raw
  To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey



On 6/16/2023 4:52 PM, Konrad Dybcio wrote:
> On 16.06.2023 12:49, Imran Shaik wrote:
>> Update the GCC clocks and add support for GDSCs for QDU1000 and
>> QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
>> add support for v2 variant.
> Too much for a single change. If somebody wants to bisect an issue, they
> would have to edit chunks of this patch.
> 
> Konrad

Yes, will split and push the next series.

Thanks,
Imran

>>
>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
>> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
>> ---
>>   drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
>>   1 file changed, 110 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
>> index 5051769ad90c..5d8125c0eacc 100644
>> --- a/drivers/clk/qcom/gcc-qdu1000.c
>> +++ b/drivers/clk/qcom/gcc-qdu1000.c
>> @@ -1,6 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   /*
>> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>>    */
>>   
>>   #include <linux/clk-provider.h>
>> @@ -17,6 +17,7 @@
>>   #include "clk-regmap-divider.h"
>>   #include "clk-regmap-mux.h"
>>   #include "clk-regmap-phy-mux.h"
>> +#include "gdsc.h"
>>   #include "reset.h"
>>   
>>   enum {
>> @@ -370,16 +371,6 @@ static const struct clk_parent_data gcc_parent_data_6[] = {
>>   	{ .index = DT_TCXO_IDX },
>>   };
>>   
>> -static const struct parent_map gcc_parent_map_7[] = {
>> -	{ P_PCIE_0_PIPE_CLK, 0 },
>> -	{ P_BI_TCXO, 2 },
>> -};
>> -
>> -static const struct clk_parent_data gcc_parent_data_7[] = {
>> -	{ .index = DT_PCIE_0_PIPE_CLK_IDX },
>> -	{ .index = DT_TCXO_IDX },
>> -};
>> -
>>   static const struct parent_map gcc_parent_map_8[] = {
>>   	{ P_BI_TCXO, 0 },
>>   	{ P_GCC_GPLL0_OUT_MAIN, 1 },
>> @@ -439,16 +430,15 @@ static struct clk_regmap_mux gcc_pcie_0_phy_aux_clk_src = {
>>   	},
>>   };
>>   
>> -static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
>> +static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
>>   	.reg = 0x9d064,
>> -	.shift = 0,
>> -	.width = 2,
>> -	.parent_map = gcc_parent_map_7,
>>   	.clkr = {
>>   		.hw.init = &(const struct clk_init_data) {
>>   			.name = "gcc_pcie_0_pipe_clk_src",
>> -			.parent_data = gcc_parent_data_7,
>> -			.num_parents = ARRAY_SIZE(gcc_parent_data_7),
>> +			.parent_data = &(const struct clk_parent_data){
>> +				.index = DT_PCIE_0_PIPE_CLK_IDX,
>> +			},
>> +			.num_parents = 1,
>>   			.ops = &clk_regmap_phy_mux_ops,
>>   		},
>>   	},
>> @@ -485,7 +475,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_dma_clk_src = {
>>   		.name = "gcc_aggre_noc_ecpri_dma_clk_src",
>>   		.parent_data = gcc_parent_data_4,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_4),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -505,7 +495,7 @@ static struct clk_rcg2 gcc_aggre_noc_ecpri_gsi_clk_src = {
>>   		.name = "gcc_aggre_noc_ecpri_gsi_clk_src",
>>   		.parent_data = gcc_parent_data_5,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_5),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
>>   		.name = "gcc_gp1_clk_src",
>>   		.parent_data = gcc_parent_data_1,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -538,7 +528,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
>>   		.name = "gcc_gp2_clk_src",
>>   		.parent_data = gcc_parent_data_1,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -552,7 +542,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
>>   		.name = "gcc_gp3_clk_src",
>>   		.parent_data = gcc_parent_data_1,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_1),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -571,7 +561,7 @@ static struct clk_rcg2 gcc_pcie_0_aux_clk_src = {
>>   		.name = "gcc_pcie_0_aux_clk_src",
>>   		.parent_data = gcc_parent_data_3,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -591,7 +581,7 @@ static struct clk_rcg2 gcc_pcie_0_phy_rchng_clk_src = {
>>   		.name = "gcc_pcie_0_phy_rchng_clk_src",
>>   		.parent_data = gcc_parent_data_0,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -610,7 +600,7 @@ static struct clk_rcg2 gcc_pdm2_clk_src = {
>>   		.name = "gcc_pdm2_clk_src",
>>   		.parent_data = gcc_parent_data_0,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -632,7 +622,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s0_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s0_clk_src = {
>> @@ -648,7 +638,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s1_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s1_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s1_clk_src = {
>> @@ -664,7 +654,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s2_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s2_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s2_clk_src = {
>> @@ -680,7 +670,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s3_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s3_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s3_clk_src = {
>> @@ -696,7 +686,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s4_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s4_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s4_clk_src = {
>> @@ -717,7 +707,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s5_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s5_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s5_clk_src = {
>> @@ -733,7 +723,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s6_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s6_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s6_clk_src = {
>> @@ -749,7 +739,7 @@ static struct clk_init_data gcc_qupv3_wrap0_s7_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap0_s7_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap0_s7_clk_src = {
>> @@ -765,7 +755,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s0_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s0_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s0_clk_src = {
>> @@ -781,7 +771,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s1_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s1_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s1_clk_src = {
>> @@ -797,7 +787,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s2_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s2_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s2_clk_src = {
>> @@ -813,7 +803,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s3_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s3_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s3_clk_src = {
>> @@ -829,7 +819,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s4_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s4_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s4_clk_src = {
>> @@ -845,7 +835,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s5_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s5_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s5_clk_src = {
>> @@ -861,7 +851,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s6_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s6_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s6_clk_src = {
>> @@ -877,7 +867,7 @@ static struct clk_init_data gcc_qupv3_wrap1_s7_clk_src_init = {
>>   	.name = "gcc_qupv3_wrap1_s7_clk_src",
>>   	.parent_data = gcc_parent_data_0,
>>   	.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -	.ops = &clk_rcg2_ops,
>> +	.ops = &clk_rcg2_shared_ops,
>>   };
>>   
>>   static struct clk_rcg2 gcc_qupv3_wrap1_s7_clk_src = {
>> @@ -913,7 +903,7 @@ static struct clk_rcg2 gcc_sdcc5_apps_clk_src = {
>>   		.name = "gcc_sdcc5_apps_clk_src",
>>   		.parent_data = gcc_parent_data_8,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_8),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_floor_ops,
>>   	},
>>   };
>>   
>> @@ -932,7 +922,7 @@ static struct clk_rcg2 gcc_sdcc5_ice_core_clk_src = {
>>   		.name = "gcc_sdcc5_ice_core_clk_src",
>>   		.parent_data = gcc_parent_data_2,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_floor_ops,
>>   	},
>>   };
>>   
>> @@ -946,7 +936,7 @@ static struct clk_rcg2 gcc_sm_bus_xo_clk_src = {
>>   		.name = "gcc_sm_bus_xo_clk_src",
>>   		.parent_data = gcc_parent_data_2,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_2),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -965,7 +955,7 @@ static struct clk_rcg2 gcc_tsc_clk_src = {
>>   		.name = "gcc_tsc_clk_src",
>>   		.parent_data = gcc_parent_data_9,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_9),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -985,7 +975,7 @@ static struct clk_rcg2 gcc_usb30_prim_master_clk_src = {
>>   		.name = "gcc_usb30_prim_master_clk_src",
>>   		.parent_data = gcc_parent_data_0,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -999,7 +989,7 @@ static struct clk_rcg2 gcc_usb30_prim_mock_utmi_clk_src = {
>>   		.name = "gcc_usb30_prim_mock_utmi_clk_src",
>>   		.parent_data = gcc_parent_data_0,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -1013,7 +1003,7 @@ static struct clk_rcg2 gcc_usb3_prim_phy_aux_clk_src = {
>>   		.name = "gcc_usb3_prim_phy_aux_clk_src",
>>   		.parent_data = gcc_parent_data_3,
>>   		.num_parents = ARRAY_SIZE(gcc_parent_data_3),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_shared_ops,
>>   	},
>>   };
>>   
>> @@ -1142,6 +1132,26 @@ static struct clk_branch gcc_ddrss_ecpri_dma_clk = {
>>   	},
>>   };
>>   
>> +static struct clk_branch gcc_ddrss_ecpri_gsi_clk = {
>> +	.halt_reg = 0x54298,
>> +	.halt_check = BRANCH_HALT_VOTED,
>> +	.hwcg_reg = 0x54298,
>> +	.hwcg_bit = 1,
>> +	.clkr = {
>> +		.enable_reg = 0x54298,
>> +		.enable_mask = BIT(0),
>> +		.hw.init = &(const struct clk_init_data) {
>> +			.name = "gcc_ddrss_ecpri_gsi_clk",
>> +			.parent_hws = (const struct clk_hw*[]) {
>> +				&gcc_aggre_noc_ecpri_gsi_clk_src.clkr.hw,
>> +			},
>> +			.num_parents = 1,
>> +			.flags = CLK_SET_RATE_PARENT,
>> +			.ops = &clk_branch2_aon_ops,
>> +		},
>> +	},
>> +};
>> +
>>   static struct clk_branch gcc_ecpri_ahb_clk = {
>>   	.halt_reg = 0x3a008,
>>   	.halt_check = BRANCH_HALT_VOTED,
>> @@ -1458,14 +1468,13 @@ static struct clk_branch gcc_pcie_0_cfg_ahb_clk = {
>>   
>>   static struct clk_branch gcc_pcie_0_clkref_en = {
>>   	.halt_reg = 0x9c004,
>> -	.halt_bit = 31,
>>   	.halt_check = BRANCH_HALT_ENABLE,
>>   	.clkr = {
>>   		.enable_reg = 0x9c004,
>>   		.enable_mask = BIT(0),
>>   		.hw.init = &(const struct clk_init_data) {
>>   			.name = "gcc_pcie_0_clkref_en",
>> -			.ops = &clk_branch_ops,
>> +			.ops = &clk_branch2_ops,
>>   		},
>>   	},
>>   };
>> @@ -2285,14 +2294,13 @@ static struct clk_branch gcc_tsc_etu_clk = {
>>   
>>   static struct clk_branch gcc_usb2_clkref_en = {
>>   	.halt_reg = 0x9c008,
>> -	.halt_bit = 31,
>>   	.halt_check = BRANCH_HALT_ENABLE,
>>   	.clkr = {
>>   		.enable_reg = 0x9c008,
>>   		.enable_mask = BIT(0),
>>   		.hw.init = &(const struct clk_init_data) {
>>   			.name = "gcc_usb2_clkref_en",
>> -			.ops = &clk_branch_ops,
>> +			.ops = &clk_branch2_ops,
>>   		},
>>   	},
>>   };
>> @@ -2402,6 +2410,39 @@ static struct clk_branch gcc_usb3_prim_phy_pipe_clk = {
>>   	},
>>   };
>>   
>> +static struct gdsc pcie_0_gdsc = {
>> +	.gdscr = 0x9d004,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0xf,
>> +	.pd = {
>> +		.name = "gcc_pcie_0_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>> +static struct gdsc pcie_0_phy_gdsc = {
>> +	.gdscr = 0x7c004,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0x2,
>> +	.pd = {
>> +		.name = "gcc_pcie_0_phy_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>> +static struct gdsc usb30_prim_gdsc = {
>> +	.gdscr = 0x49004,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0xf,
>> +	.pd = {
>> +		.name = "gcc_usb30_prim_gdsc",
>> +	},
>> +	.pwrsts = PWRSTS_OFF_ON,
>> +};
>> +
>>   static struct clk_regmap *gcc_qdu1000_clocks[] = {
>>   	[GCC_AGGRE_NOC_ECPRI_DMA_CLK] = &gcc_aggre_noc_ecpri_dma_clk.clkr,
>>   	[GCC_AGGRE_NOC_ECPRI_DMA_CLK_SRC] = &gcc_aggre_noc_ecpri_dma_clk_src.clkr,
>> @@ -2534,6 +2575,14 @@ static struct clk_regmap *gcc_qdu1000_clocks[] = {
>>   	[GCC_AGGRE_NOC_ECPRI_GSI_CLK] = &gcc_aggre_noc_ecpri_gsi_clk.clkr,
>>   	[GCC_PCIE_0_PHY_AUX_CLK_SRC] = &gcc_pcie_0_phy_aux_clk_src.clkr,
>>   	[GCC_PCIE_0_PIPE_CLK_SRC] = &gcc_pcie_0_pipe_clk_src.clkr,
>> +	[GCC_GPLL1_OUT_EVEN] = &gcc_gpll1_out_even.clkr,
>> +	[GCC_DDRSS_ECPRI_GSI_CLK] = NULL,
>> +};
>> +
>> +static struct gdsc *gcc_qdu1000_gdscs[] = {
>> +	[PCIE_0_GDSC] = &pcie_0_gdsc,
>> +	[PCIE_0_PHY_GDSC] = &pcie_0_phy_gdsc,
>> +	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
>>   };
>>   
>>   static const struct qcom_reset_map gcc_qdu1000_resets[] = {
>> @@ -2597,10 +2646,13 @@ static const struct qcom_cc_desc gcc_qdu1000_desc = {
>>   	.num_clks = ARRAY_SIZE(gcc_qdu1000_clocks),
>>   	.resets = gcc_qdu1000_resets,
>>   	.num_resets = ARRAY_SIZE(gcc_qdu1000_resets),
>> +	.gdscs = gcc_qdu1000_gdscs,
>> +	.num_gdscs = ARRAY_SIZE(gcc_qdu1000_gdscs),
>>   };
>>   
>>   static const struct of_device_id gcc_qdu1000_match_table[] = {
>>   	{ .compatible = "qcom,qdu1000-gcc" },
>> +	{ .compatible = "qcom,qdu1000-gcc-v2" },
>>   	{ }
>>   };
>>   MODULE_DEVICE_TABLE(of, gcc_qdu1000_match_table);
>> @@ -2617,6 +2669,12 @@ static int gcc_qdu1000_probe(struct platform_device *pdev)
>>   	/* Update FORCE_MEM_CORE_ON for gcc_pcie_0_mstr_axi_clk */
>>   	regmap_update_bits(regmap, 0x9d024, BIT(14), BIT(14));
>>   
>> +	if (of_device_is_compatible(pdev->dev.of_node, "qcom,qdu1000-gcc-v2")) {
>> +		gcc_qdu1000_clocks[GCC_DDRSS_ECPRI_GSI_CLK] = &gcc_ddrss_ecpri_gsi_clk.clkr;
>> +		gcc_pcie_0_clkref_en.halt_check = BRANCH_HALT_DELAY;
>> +		gcc_usb2_clkref_en.halt_check = BRANCH_HALT_DELAY;
>> +	}
>> +
>>   	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
>>   				       ARRAY_SIZE(gcc_dfs_clocks));
>>   	if (ret)

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

* Re: [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs
  2023-06-23 10:08     ` Imran Shaik
@ 2023-06-23 14:06       ` Dmitry Baryshkov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Baryshkov @ 2023-06-23 14:06 UTC (permalink / raw
  To: Imran Shaik
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Melody Olvera, Taniya Das, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Jagadeesh Kona, Satya Priya Kakitapalli,
	Ajit Pandey

On Fri, 23 Jun 2023 at 13:08, Imran Shaik <quic_imrashai@quicinc.com> wrote:
>
>
>
> On 6/16/2023 4:50 PM, Dmitry Baryshkov wrote:
> > On 16/06/2023 13:49, Imran Shaik wrote:
> >> Update the GCC clocks and add support for GDSCs for QDU1000 and
> >> QRU1000 SoCs. While at it, fix the PCIe pipe clock handling and
> >> add support for v2 variant.
> >
> > Please split this into individual chunks instead of squashing everything
> > together. For each change please describe the logic behind the change in
> > the commit message. Please describe why, not what is changed.
> >
>
> Sure, will split this patch and post the next series.
>
> >>
> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> >> Signed-off-by: Imran Shaik <quic_imrashai@quicinc.com>
> >
> > This doesn't look fully logical. Who is the author of the patch? If
> > there are two authors, please add Co-developed-by tag.
> >
>
> Yes, will use Co-developed-by tag in the next patch series.
>
> >> ---
> >>   drivers/clk/qcom/gcc-qdu1000.c | 162 ++++++++++++++++++++++-----------
> >>   1 file changed, 110 insertions(+), 52 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/gcc-qdu1000.c
> >> b/drivers/clk/qcom/gcc-qdu1000.c
> >> index 5051769ad90c..5d8125c0eacc 100644
> >> --- a/drivers/clk/qcom/gcc-qdu1000.c
> >> +++ b/drivers/clk/qcom/gcc-qdu1000.c
> >> @@ -1,6 +1,6 @@
> >>   // SPDX-License-Identifier: GPL-2.0-only
> >>   /*
> >> - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights
> >> reserved.
> >> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All
> >> rights reserved.
> >>    */
> >>   #include <linux/clk-provider.h>
> >> @@ -17,6 +17,7 @@
> >>   #include "clk-regmap-divider.h"
> >>   #include "clk-regmap-mux.h"
> >>   #include "clk-regmap-phy-mux.h"
> >> +#include "gdsc.h"
> >>   #include "reset.h"
> >>   enum {
> >> @@ -370,16 +371,6 @@ static const struct clk_parent_data
> >> gcc_parent_data_6[] = {
> >>       { .index = DT_TCXO_IDX },
> >>   };
> >> -static const struct parent_map gcc_parent_map_7[] = {
> >> -    { P_PCIE_0_PIPE_CLK, 0 },
> >> -    { P_BI_TCXO, 2 },
> >> -};
> >> -
> >> -static const struct clk_parent_data gcc_parent_data_7[] = {
> >> -    { .index = DT_PCIE_0_PIPE_CLK_IDX },
> >> -    { .index = DT_TCXO_IDX },
> >> -};
> >> -
> >>   static const struct parent_map gcc_parent_map_8[] = {
> >>       { P_BI_TCXO, 0 },
> >>       { P_GCC_GPLL0_OUT_MAIN, 1 },
> >> @@ -439,16 +430,15 @@ static struct clk_regmap_mux
> >> gcc_pcie_0_phy_aux_clk_src = {
> >>       },
> >>   };
> >> -static struct clk_regmap_mux gcc_pcie_0_pipe_clk_src = {
> >> +static struct clk_regmap_phy_mux gcc_pcie_0_pipe_clk_src = {
> >>       .reg = 0x9d064,
> >> -    .shift = 0,
> >> -    .width = 2,
> >> -    .parent_map = gcc_parent_map_7,
> >>       .clkr = {
> >>           .hw.init = &(const struct clk_init_data) {
> >>               .name = "gcc_pcie_0_pipe_clk_src",
> >> -            .parent_data = gcc_parent_data_7,
> >> -            .num_parents = ARRAY_SIZE(gcc_parent_data_7),
> >> +            .parent_data = &(const struct clk_parent_data){
> >> +                .index = DT_PCIE_0_PIPE_CLK_IDX,
> >> +            },
> >> +            .num_parents = 1,
> >>               .ops = &clk_regmap_phy_mux_ops,
> >>           },
> >>       },
> >> @@ -485,7 +475,7 @@ static struct clk_rcg2
> >> gcc_aggre_noc_ecpri_dma_clk_src = {
> >>           .name = "gcc_aggre_noc_ecpri_dma_clk_src",
> >>           .parent_data = gcc_parent_data_4,
> >>           .num_parents = ARRAY_SIZE(gcc_parent_data_4),
> >> -        .ops = &clk_rcg2_ops,
> >> +        .ops = &clk_rcg2_shared_ops,
> >>       },
> >>   };
> >> @@ -505,7 +495,7 @@ static struct clk_rcg2
> >> gcc_aggre_noc_ecpri_gsi_clk_src = {
> >>           .name = "gcc_aggre_noc_ecpri_gsi_clk_src",
> >>           .parent_data = gcc_parent_data_5,
> >>           .num_parents = ARRAY_SIZE(gcc_parent_data_5),
> >> -        .ops = &clk_rcg2_ops,
> >> +        .ops = &clk_rcg2_shared_ops,
> >
> > This is probably some kind of NoC or NIU clock. If it is not to be
> > touched by Linux, the recent clk_rcg2_ro_ops patch looks promising here.
> >
>
> This is not a NoC or NIU clock and Linux will use this clock.

The clock name ("gcc_aggre_noc_ecpri_gsi_clk_src") seems to disagree with you.

> >>       },
> >>   };
> >> @@ -524,7 +514,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
> >>           .name = "gcc_gp1_clk_src",
> >>           .parent_data = gcc_parent_data_1,
> >>           .num_parents = ARRAY_SIZE(gcc_parent_data_1),
> >> -        .ops = &clk_rcg2_ops,
> >> +        .ops = &clk_rcg2_shared_ops,
> >
> > But why? GP clocks are not shared.
> > The 'why?' question applies to all such changes. As I wrote, please
> > split & describe the reason.
> >
>
> We want to park all the RCGs at safe clock source (XO) during disable.
> Hence using the clk_rcg2_shared_ops for all the RCGs.
>
> Will split and post the next patch series.

For this (and for all other changes) please describe the reason behind
the change in the commit message.
For example, for GP clocks there seems to be no reason to park them.
On other platforms it was perfectly fine to disable them instead.


-- 
With best wishes
Dmitry

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

end of thread, other threads:[~2023-06-23 14:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-16 10:49 [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Imran Shaik
2023-06-16 10:49 ` [PATCH 1/2] dt-bindings: clock: " Imran Shaik
2023-06-16 11:33   ` Krzysztof Kozlowski
2023-06-22 13:45     ` Krzysztof Kozlowski
2023-06-23 10:07       ` Imran Shaik
2023-06-16 10:49 ` [PATCH 2/2] clk: qcom: gcc-qdu1000: Update GCC clocks and add support for GDSCs Imran Shaik
2023-06-16 11:20   ` Dmitry Baryshkov
2023-06-23 10:08     ` Imran Shaik
2023-06-23 14:06       ` Dmitry Baryshkov
2023-06-16 11:22   ` Konrad Dybcio
2023-06-23 10:12     ` Imran Shaik
2023-06-16 11:21 ` [PATCH 0/2] Update GCC clocks for QDU1000 and QRU1000 SoCs Konrad Dybcio
2023-06-23 10:07   ` Imran Shaik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.