[PATCH v2 0/5] clk: qcom: Add DISPCC and GPUCC support for the Qualcomm Shikra SoC

[PATCH v2 0/5] clk: qcom: Add DISPCC and GPUCC support for the Qualcomm Shikra SoC

[Imran Shaik]

This series adds support for the Display clock controller (DISPCC) and
GPU Clock Controller (GPUCC) on Qualcomm Shikra SoC.

Shikra GCC series link:
- https://lore.kernel.org/linux-clk/20260508-shikra-gcc-rpmcc-clks-v2-0-83238ba24060@oss.qualcomm.com/

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
Changes in v2:
- Dropped QCM2290 GCC critical clocks modelling to kept them ON from probe.
- Updated the QCM2290 DISPCC/GPUCC bindings to align for Shikra drivers reuse.
- Reused the QCM2290 DISPCC driver for Shikra without modernizing
  (keeping the clock-names approach) for now to avoid potential bindings ABI breakage.
- Modernized QCM2290 GPUCC driver to use commmon qcom_cc_probe() model
  and reuse for Shikra. 
- Link to v1: https://lore.kernel.org/r/20260513-shikra-dispcc-gpucc-v1-0-5fd673146ab2@oss.qualcomm.com

---
Imran Shaik (5):
      clk: qcom: gcc-qcm2290: Drop modelling of critical clocks
      dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller
      dt-bindings: clock: qcom: Add Qualcomm Shikra GPU clock controller
      clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC
      clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

 .../bindings/clock/qcom,qcm2290-dispcc.yaml        |  75 +++++++--
 .../bindings/clock/qcom,qcm2290-gpucc.yaml         |   4 +-
 drivers/clk/qcom/dispcc-qcm2290.c                  |  18 ++-
 drivers/clk/qcom/gcc-qcm2290.c                     | 153 ++----------------
 drivers/clk/qcom/gpucc-qcm2290.c                   | 174 +++++++++------------
 5 files changed, 161 insertions(+), 263 deletions(-)
---
base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
change-id: 20260513-shikra-dispcc-gpucc-6f59c23020f5
prerequisite-change-id: 20260429-shikra-gcc-rpmcc-clks-2094edfff3b0:v2
prerequisite-patch-id: 5a0fbdd458785da2d0e850c851a05046672ecadf
prerequisite-patch-id: 1f98e515a52bbeb25e2a960a804afe16c6a472a1
prerequisite-patch-id: a64476b2ba6e0f2a55928baf72ec32672ee0123c
prerequisite-patch-id: d0c8651205232862b40f942929e1efdaa3084eb3

Best regards,
-- 
Imran Shaik <imran.shaik@oss.qualcomm.com>

[PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[Imran Shaik]

Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
enabled from probe as per the latest convention. This helps to drop the
pm_clk handling in QCM2290 GPUCC driver, and the same can be re-used for
Shikra SoC.

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
 drivers/clk/qcom/gcc-qcm2290.c | 153 +++--------------------------------------
 1 file changed, 11 insertions(+), 142 deletions(-)

diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
index 6684cab63ae1160848631d1f8cd3c9cb691ff4ec..937db68a45b17b190aca7f3164e0dad8deabbfc8 100644
--- a/drivers/clk/qcom/gcc-qcm2290.c
+++ b/drivers/clk/qcom/gcc-qcm2290.c
@@ -1397,36 +1397,6 @@ static struct clk_branch gcc_cam_throttle_rt_clk = {
 	},
 };
 
-static struct clk_branch gcc_camera_ahb_clk = {
-	.halt_reg = 0x17008,
-	.halt_check = BRANCH_HALT_DELAY,
-	.hwcg_reg = 0x17008,
-	.hwcg_bit = 1,
-	.clkr = {
-		.enable_reg = 0x17008,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_camera_ahb_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
-static struct clk_branch gcc_camera_xo_clk = {
-	.halt_reg = 0x17028,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x17028,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_camera_xo_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_camss_axi_clk = {
 	.halt_reg = 0x58044,
 	.halt_check = BRANCH_HALT,
@@ -1825,22 +1795,6 @@ static struct clk_branch gcc_cfg_noc_usb3_prim_axi_clk = {
 	},
 };
 
-static struct clk_branch gcc_disp_ahb_clk = {
-	.halt_reg = 0x1700c,
-	.halt_check = BRANCH_HALT,
-	.hwcg_reg = 0x1700c,
-	.hwcg_bit = 1,
-	.clkr = {
-		.enable_reg = 0x1700c,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_disp_ahb_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_regmap_div gcc_disp_gpll0_clk_src = {
 	.reg = 0x17058,
 	.shift = 0,
@@ -1899,20 +1853,6 @@ static struct clk_branch gcc_disp_throttle_core_clk = {
 	},
 };
 
-static struct clk_branch gcc_disp_xo_clk = {
-	.halt_reg = 0x1702c,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x1702c,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_disp_xo_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_gp1_clk = {
 	.halt_reg = 0x4d000,
 	.halt_check = BRANCH_HALT,
@@ -1964,22 +1904,6 @@ static struct clk_branch gcc_gp3_clk = {
 	},
 };
 
-static struct clk_branch gcc_gpu_cfg_ahb_clk = {
-	.halt_reg = 0x36004,
-	.halt_check = BRANCH_HALT,
-	.hwcg_reg = 0x36004,
-	.hwcg_bit = 1,
-	.clkr = {
-		.enable_reg = 0x36004,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_gpu_cfg_ahb_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_gpu_gpll0_clk_src = {
 	.halt_check = BRANCH_HALT_DELAY,
 	.clkr = {
@@ -2012,19 +1936,6 @@ static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
 	},
 };
 
-static struct clk_branch gcc_gpu_iref_clk = {
-	.halt_reg = 0x36100,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x36100,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_gpu_iref_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_gpu_memnoc_gfx_clk = {
 	.halt_reg = 0x3600c,
 	.halt_check = BRANCH_VOTED,
@@ -2439,22 +2350,6 @@ static struct clk_branch gcc_sdcc2_apps_clk = {
 	},
 };
 
-static struct clk_branch gcc_sys_noc_cpuss_ahb_clk = {
-	.halt_reg = 0x2b06c,
-	.halt_check = BRANCH_HALT_VOTED,
-	.hwcg_reg = 0x2b06c,
-	.hwcg_bit = 1,
-	.clkr = {
-		.enable_reg = 0x79004,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_sys_noc_cpuss_ahb_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_sys_noc_usb3_prim_axi_clk = {
 	.halt_reg = 0x1a080,
 	.halt_check = BRANCH_HALT,
@@ -2605,21 +2500,6 @@ static struct clk_branch gcc_venus_ctl_axi_clk = {
 	},
 };
 
-static struct clk_branch gcc_video_ahb_clk = {
-	.halt_reg = 0x17004,
-	.halt_check = BRANCH_HALT,
-	.hwcg_reg = 0x17004,
-	.hwcg_bit = 1,
-	.clkr = {
-		.enable_reg = 0x17004,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_video_ahb_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_video_axi0_clk = {
 	.halt_reg = 0x1701c,
 	.halt_check = BRANCH_HALT,
@@ -2686,19 +2566,6 @@ static struct clk_branch gcc_video_venus_ctl_clk = {
 	},
 };
 
-static struct clk_branch gcc_video_xo_clk = {
-	.halt_reg = 0x17024,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x17024,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_video_xo_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct gdsc gcc_camss_top_gdsc = {
 	.gdscr = 0x58004,
 	.pd = {
@@ -2775,8 +2642,6 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
 	[GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr,
 	[GCC_CAM_THROTTLE_NRT_CLK] = &gcc_cam_throttle_nrt_clk.clkr,
 	[GCC_CAM_THROTTLE_RT_CLK] = &gcc_cam_throttle_rt_clk.clkr,
-	[GCC_CAMERA_AHB_CLK] = &gcc_camera_ahb_clk.clkr,
-	[GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr,
 	[GCC_CAMSS_AXI_CLK] = &gcc_camss_axi_clk.clkr,
 	[GCC_CAMSS_AXI_CLK_SRC] = &gcc_camss_axi_clk_src.clkr,
 	[GCC_CAMSS_CAMNOC_ATB_CLK] = &gcc_camss_camnoc_atb_clk.clkr,
@@ -2817,22 +2682,18 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
 	[GCC_CAMSS_TOP_AHB_CLK] = &gcc_camss_top_ahb_clk.clkr,
 	[GCC_CAMSS_TOP_AHB_CLK_SRC] = &gcc_camss_top_ahb_clk_src.clkr,
 	[GCC_CFG_NOC_USB3_PRIM_AXI_CLK] = &gcc_cfg_noc_usb3_prim_axi_clk.clkr,
-	[GCC_DISP_AHB_CLK] = &gcc_disp_ahb_clk.clkr,
 	[GCC_DISP_GPLL0_CLK_SRC] = &gcc_disp_gpll0_clk_src.clkr,
 	[GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr,
 	[GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr,
 	[GCC_DISP_THROTTLE_CORE_CLK] = &gcc_disp_throttle_core_clk.clkr,
-	[GCC_DISP_XO_CLK] = &gcc_disp_xo_clk.clkr,
 	[GCC_GP1_CLK] = &gcc_gp1_clk.clkr,
 	[GCC_GP1_CLK_SRC] = &gcc_gp1_clk_src.clkr,
 	[GCC_GP2_CLK] = &gcc_gp2_clk.clkr,
 	[GCC_GP2_CLK_SRC] = &gcc_gp2_clk_src.clkr,
 	[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
 	[GCC_GP3_CLK_SRC] = &gcc_gp3_clk_src.clkr,
-	[GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
 	[GCC_GPU_GPLL0_CLK_SRC] = &gcc_gpu_gpll0_clk_src.clkr,
 	[GCC_GPU_GPLL0_DIV_CLK_SRC] = &gcc_gpu_gpll0_div_clk_src.clkr,
-	[GCC_GPU_IREF_CLK] = &gcc_gpu_iref_clk.clkr,
 	[GCC_GPU_MEMNOC_GFX_CLK] = &gcc_gpu_memnoc_gfx_clk.clkr,
 	[GCC_GPU_SNOC_DVM_GFX_CLK] = &gcc_gpu_snoc_dvm_gfx_clk.clkr,
 	[GCC_GPU_THROTTLE_CORE_CLK] = &gcc_gpu_throttle_core_clk.clkr,
@@ -2870,7 +2731,6 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
 	[GCC_SDCC2_AHB_CLK] = &gcc_sdcc2_ahb_clk.clkr,
 	[GCC_SDCC2_APPS_CLK] = &gcc_sdcc2_apps_clk.clkr,
 	[GCC_SDCC2_APPS_CLK_SRC] = &gcc_sdcc2_apps_clk_src.clkr,
-	[GCC_SYS_NOC_CPUSS_AHB_CLK] = &gcc_sys_noc_cpuss_ahb_clk.clkr,
 	[GCC_SYS_NOC_USB3_PRIM_AXI_CLK] = &gcc_sys_noc_usb3_prim_axi_clk.clkr,
 	[GCC_USB30_PRIM_MASTER_CLK] = &gcc_usb30_prim_master_clk.clkr,
 	[GCC_USB30_PRIM_MASTER_CLK_SRC] = &gcc_usb30_prim_master_clk_src.clkr,
@@ -2887,13 +2747,11 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
 	[GCC_VCODEC0_AXI_CLK] = &gcc_vcodec0_axi_clk.clkr,
 	[GCC_VENUS_AHB_CLK] = &gcc_venus_ahb_clk.clkr,
 	[GCC_VENUS_CTL_AXI_CLK] = &gcc_venus_ctl_axi_clk.clkr,
-	[GCC_VIDEO_AHB_CLK] = &gcc_video_ahb_clk.clkr,
 	[GCC_VIDEO_AXI0_CLK] = &gcc_video_axi0_clk.clkr,
 	[GCC_VIDEO_THROTTLE_CORE_CLK] = &gcc_video_throttle_core_clk.clkr,
 	[GCC_VIDEO_VCODEC0_SYS_CLK] = &gcc_video_vcodec0_sys_clk.clkr,
 	[GCC_VIDEO_VENUS_CLK_SRC] = &gcc_video_venus_clk_src.clkr,
 	[GCC_VIDEO_VENUS_CTL_CLK] = &gcc_video_venus_ctl_clk.clkr,
-	[GCC_VIDEO_XO_CLK] = &gcc_video_xo_clk.clkr,
 	[GPLL0] = &gpll0.clkr,
 	[GPLL0_OUT_AUX2] = &gpll0_out_aux2.clkr,
 	[GPLL1] = &gpll1.clkr,
@@ -2990,6 +2848,17 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	/* Keep some clocks always-on */
+	qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
+	qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
+	qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
+	qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
+	qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
+	qcom_branch_set_clk_en(regmap, 0x36100); /* GCC_GPU_IREF_CLK */
+	qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
+	qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */
+	qcom_branch_set_clk_en(regmap, 0x17024); /* GCC_VIDEO_XO_CLK */
+
 	clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
 	clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);
 	clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);

-- 
2.34.1

[PATCH v2 2/5] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

[Imran Shaik]

The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
but has minor differences with the number of input clocks. Update the
existing QCM2290 DISPCC bindings using conditional schema so that the
QCM2290 requirements remain unchanged while accommodating the additional
clocks required for Shikra.

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
 .../bindings/clock/qcom,qcm2290-dispcc.yaml        | 75 +++++++++++++++++-----
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
index 4a533b45eec2d8e7b866c3436bfe6f80fcd714fb..b24095814d9e67a355321d9013e144f245077322 100644
--- a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
@@ -17,25 +17,21 @@ description: |
 
 properties:
   compatible:
-    const: qcom,qcm2290-dispcc
+    oneOf:
+      - items:
+          - enum:
+              - qcom,shikra-dispcc
+          - const: qcom,qcm2290-dispcc
+      - enum:
+          - qcom,qcm2290-dispcc
 
   clocks:
-    items:
-      - description: Board XO source
-      - description: Board active-only XO source
-      - description: GPLL0 source from GCC
-      - description: GPLL0 div source from GCC
-      - description: Byte clock from DSI PHY
-      - description: Pixel clock from DSI PHY
+    minItems: 6
+    maxItems: 9
 
   clock-names:
-    items:
-      - const: bi_tcxo
-      - const: bi_tcxo_ao
-      - const: gcc_disp_gpll0_clk_src
-      - const: gcc_disp_gpll0_div_clk_src
-      - const: dsi0_phy_pll_out_byteclk
-      - const: dsi0_phy_pll_out_dsiclk
+    minItems: 6
+    maxItems: 9
 
 required:
   - compatible
@@ -45,6 +41,55 @@ required:
 
 allOf:
   - $ref: qcom,gcc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,shikra-dispcc
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Board XO source
+            - description: Board active-only XO source
+            - description: GPLL0 source from GCC
+            - description: GPLL0 div source from GCC
+            - description: Byte clock from DSI PHY0
+            - description: Pixel clock from DSI PHY0
+            - description: Byte clock from DSI PHY1
+            - description: Pixel clock from DSI PHY1
+            - description: Board sleep clock
+
+        clock-names:
+          items:
+            - const: bi_tcxo
+            - const: bi_tcxo_ao
+            - const: gcc_disp_gpll0_clk_src
+            - const: gcc_disp_gpll0_div_clk_src
+            - const: dsi0_phy_pll_out_byteclk
+            - const: dsi0_phy_pll_out_dsiclk
+            - const: dsi1_phy_pll_out_byteclk
+            - const: dsi1_phy_pll_out_dsiclk
+            - const: sleep_clk
+    else:
+      properties:
+        clocks:
+          items:
+            - description: Board XO source
+            - description: Board active-only XO source
+            - description: GPLL0 source from GCC
+            - description: GPLL0 div source from GCC
+            - description: Byte clock from DSI PHY
+            - description: Pixel clock from DSI PHY
+
+        clock-names:
+          items:
+            - const: bi_tcxo
+            - const: bi_tcxo_ao
+            - const: gcc_disp_gpll0_clk_src
+            - const: gcc_disp_gpll0_div_clk_src
+            - const: dsi0_phy_pll_out_byteclk
+            - const: dsi0_phy_pll_out_dsiclk
 
 unevaluatedProperties: false
 

-- 
2.34.1

[PATCH v2 3/5] dt-bindings: clock: qcom: Add Qualcomm Shikra GPU clock controller

[Imran Shaik]

The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
hardware block, with minor differences. Hence, reuse the QCM2290 GPUCC
bindings for Qualcomm Shikra SoC.

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
index 734880805c1b981a1c899d85435f83f4f3dd3ea9..1bd70d091fcd7b6d7805ac090aaf840a415c123b 100644
--- a/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-gpucc.yaml
@@ -18,7 +18,9 @@ description: |
 
 properties:
   compatible:
-    const: qcom,qcm2290-gpucc
+    enum:
+      - qcom,qcm2290-gpucc
+      - qcom,shikra-gpucc
 
   reg:
     maxItems: 1

-- 
2.34.1

[PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[Imran Shaik]

The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
but has minor differences. Update the parent data of mdss ahb/mdp clocks
accordingly to the hardware clock plan and correct the GDSC *_wait_val and
flags which are applicable for both QCM2290 and Shikra SoC, and add the
support for DSI1 PHY source.

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
 drivers/clk/qcom/dispcc-qcm2290.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
index 6d88d067337fa132114b0d8666931b449f86de17..19c997f3fe9f197d2c252a9dd1e8169947200f5f 100644
--- a/drivers/clk/qcom/dispcc-qcm2290.c
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2020, The Linux Foundation. All rights reserved.
  * Copyright (c) 2021, Linaro Ltd.
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #include <linux/clk-provider.h>
@@ -32,6 +33,8 @@ enum {
 	P_GPLL0_OUT_DIV,
 	P_GPLL0_OUT_MAIN,
 	P_SLEEP_CLK,
+	P_DSI1_PHY_PLL_OUT_BYTECLK,
+	P_DSI1_PHY_PLL_OUT_DSICLK,
 };
 
 static const struct pll_vco spark_vco[] = {
@@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
 
 static const struct parent_map disp_cc_parent_map_2[] = {
 	{ P_BI_TCXO_AO, 0 },
-	{ P_GPLL0_OUT_DIV, 4 },
+	{ P_GPLL0_OUT_MAIN, 4 },
 };
 
 static const struct clk_parent_data disp_cc_parent_data_2[] = {
@@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
 static const struct clk_parent_data disp_cc_parent_data_3[] = {
 	{ .fw_name = "bi_tcxo" },
 	{ .hw = &disp_cc_pll0.clkr.hw },
-	{ .fw_name = "gcc_disp_gpll0_clk_src" },
+	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
 };
 
 static const struct parent_map disp_cc_parent_map_4[] = {
 	{ P_BI_TCXO, 0 },
 	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
+	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
 };
 
 static const struct clk_parent_data disp_cc_parent_data_4[] = {
 	{ .fw_name = "bi_tcxo" },
 	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
+	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
 };
 
 static const struct parent_map disp_cc_parent_map_5[] = {
@@ -153,8 +158,8 @@ static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
 
 static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
 	F(19200000, P_BI_TCXO_AO, 1, 0, 0),
-	F(37500000, P_GPLL0_OUT_DIV, 8, 0, 0),
-	F(75000000, P_GPLL0_OUT_DIV, 4, 0, 0),
+	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
+	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
 	{ }
 };
 
@@ -450,11 +455,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
 
 static struct gdsc mdss_gdsc = {
 	.gdscr = 0x3000,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "mdss_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = HW_CTRL,
+	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc *disp_cc_qcm2290_gdscs[] = {

-- 
2.34.1

[PATCH v2 5/5] clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

[Imran Shaik]

The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
hardware block, with minor differences. Drop modelling of critical
clocks and keep them enabled at probe time, update the QCM2290 GPUCC
driver to align with the latest common qcom_cc_probe() model. Update the
GDSC *_wait_val and flags which are applicable for both QCM2290 and Shikra.

Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
 drivers/clk/qcom/gpucc-qcm2290.c | 174 +++++++++++++++++----------------------
 1 file changed, 74 insertions(+), 100 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-qcm2290.c b/drivers/clk/qcom/gpucc-qcm2290.c
index dc369dff882e69a8c0acd260953d5fcae9453120..296afcbfe0c9e4e147dcbd8bca807e8e4c50e185 100644
--- a/drivers/clk/qcom/gpucc-qcm2290.c
+++ b/drivers/clk/qcom/gpucc-qcm2290.c
@@ -2,14 +2,13 @@
 /*
  * Copyright (c) 2020, The Linux Foundation. All rights reserved.
  * Copyright (c) 2024, Linaro Limited
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
  */
 
 #include <linux/clk-provider.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_clock.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,qcm2290-gpucc.h>
@@ -20,7 +19,7 @@
 #include "clk-regmap.h"
 #include "clk-regmap-divider.h"
 #include "clk-regmap-mux.h"
-#include "clk-regmap-phy-mux.h"
+#include "common.h"
 #include "gdsc.h"
 #include "reset.h"
 
@@ -46,6 +45,7 @@ static const struct pll_vco huayra_vco[] = {
 	{ 600000000, 2200000000, 1 },
 };
 
+/* 710.4 MHz Configuration */
 static const struct alpha_pll_config gpu_cc_pll0_config = {
 	.l = 0x25,
 	.config_ctl_val = 0x200d4828,
@@ -57,11 +57,12 @@ static const struct alpha_pll_config gpu_cc_pll0_config = {
 
 static struct clk_alpha_pll gpu_cc_pll0 = {
 	.offset = 0x0,
+	.config = &gpu_cc_pll0_config,
 	.vco_table = huayra_vco,
 	.num_vco = ARRAY_SIZE(huayra_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_HUAYRA_2290],
 	.clkr = {
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_pll0",
 			.parent_data = &(const struct clk_parent_data) {
 				.index = DT_BI_TCXO,
@@ -80,10 +81,10 @@ static const struct parent_map gpu_cc_parent_map_0[] = {
 };
 
 static const struct clk_parent_data gpu_cc_parent_data_0[] = {
-	{ .index = DT_BI_TCXO, },
-	{ .hw = &gpu_cc_pll0.clkr.hw, },
-	{ .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
-	{ .index = DT_GCC_GPU_GPLL0_DIV_CLK_SRC, },
+	{ .index = DT_BI_TCXO },
+	{ .hw = &gpu_cc_pll0.clkr.hw },
+	{ .index = DT_GCC_GPU_GPLL0_CLK_SRC },
+	{ .index = DT_GCC_GPU_GPLL0_DIV_CLK_SRC },
 };
 
 static const struct parent_map gpu_cc_parent_map_1[] = {
@@ -95,11 +96,11 @@ static const struct parent_map gpu_cc_parent_map_1[] = {
 };
 
 static const struct clk_parent_data gpu_cc_parent_data_1[] = {
-	{ .index = DT_BI_TCXO, },
-	{ .hw = &gpu_cc_pll0.clkr.hw, },
-	{ .hw = &gpu_cc_pll0.clkr.hw, },
-	{ .hw = &gpu_cc_pll0.clkr.hw, },
-	{ .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
+	{ .index = DT_BI_TCXO },
+	{ .hw = &gpu_cc_pll0.clkr.hw },
+	{ .hw = &gpu_cc_pll0.clkr.hw },
+	{ .hw = &gpu_cc_pll0.clkr.hw },
+	{ .index = DT_GCC_GPU_GPLL0_CLK_SRC },
 };
 
 static const struct freq_tbl ftbl_gpu_cc_gmu_clk_src[] = {
@@ -113,7 +114,8 @@ static struct clk_rcg2 gpu_cc_gmu_clk_src = {
 	.hid_width = 5,
 	.parent_map = gpu_cc_parent_map_0,
 	.freq_tbl = ftbl_gpu_cc_gmu_clk_src,
-	.clkr.hw.init = &(struct clk_init_data){
+	.hw_clk_ctrl = true,
+	.clkr.hw.init = &(const struct clk_init_data) {
 		.name = "gpu_cc_gmu_clk_src",
 		.parent_data = gpu_cc_parent_data_0,
 		.num_parents = ARRAY_SIZE(gpu_cc_parent_data_0),
@@ -133,32 +135,30 @@ static const struct freq_tbl ftbl_gpu_cc_gx_gfx3d_clk_src[] = {
 	{ }
 };
 
+static const struct freq_tbl ftbl_gpu_cc_gx_gfx3d_clk_src_shikra[] = {
+	F(355200000, P_GPU_CC_PLL0_OUT_AUX, 2, 0, 0),
+	F(537600000, P_GPU_CC_PLL0_OUT_AUX, 2, 0, 0),
+	F(672000000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+	F(844800000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+	F(921600000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+	F(1017600000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+	F(1142400000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+	{ }
+};
+
 static struct clk_rcg2 gpu_cc_gx_gfx3d_clk_src = {
 	.cmd_rcgr = 0x101c,
 	.mnd_width = 0,
 	.hid_width = 5,
 	.parent_map = gpu_cc_parent_map_1,
 	.freq_tbl = ftbl_gpu_cc_gx_gfx3d_clk_src,
-	.clkr.hw.init = &(struct clk_init_data){
+	.hw_clk_ctrl = true,
+	.clkr.hw.init = &(const struct clk_init_data) {
 		.name = "gpu_cc_gx_gfx3d_clk_src",
 		.parent_data = gpu_cc_parent_data_1,
 		.num_parents = ARRAY_SIZE(gpu_cc_parent_data_1),
 		.flags = CLK_SET_RATE_PARENT,
-		.ops = &clk_rcg2_ops,
-	},
-};
-
-static struct clk_branch gpu_cc_ahb_clk = {
-	.halt_reg = 0x1078,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x1078,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gpu_cc_ahb_clk",
-			.flags = CLK_IS_CRITICAL,
-			.ops = &clk_branch2_ops,
-		},
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
@@ -168,7 +168,7 @@ static struct clk_branch gpu_cc_crc_ahb_clk = {
 	.clkr = {
 		.enable_reg = 0x107c,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_crc_ahb_clk",
 			.ops = &clk_branch2_ops,
 		},
@@ -181,10 +181,10 @@ static struct clk_branch gpu_cc_cx_gfx3d_clk = {
 	.clkr = {
 		.enable_reg = 0x10a4,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_cx_gfx3d_clk",
-			.parent_data = &(const struct clk_parent_data){
-				.hw = &gpu_cc_gx_gfx3d_clk_src.clkr.hw,
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpu_cc_gx_gfx3d_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -199,10 +199,10 @@ static struct clk_branch gpu_cc_cx_gmu_clk = {
 	.clkr = {
 		.enable_reg = 0x1098,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_cx_gmu_clk",
-			.parent_data = &(const struct clk_parent_data){
-				.hw = &gpu_cc_gmu_clk_src.clkr.hw,
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpu_cc_gmu_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -217,33 +217,20 @@ static struct clk_branch gpu_cc_cx_snoc_dvm_clk = {
 	.clkr = {
 		.enable_reg = 0x108c,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_cx_snoc_dvm_clk",
 			.ops = &clk_branch2_ops,
 		},
 	},
 };
 
-static struct clk_branch gpu_cc_cxo_aon_clk = {
-	.halt_reg = 0x1004,
-	.halt_check = BRANCH_HALT_DELAY,
-	.clkr = {
-		.enable_reg = 0x1004,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gpu_cc_cxo_aon_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gpu_cc_cxo_clk = {
 	.halt_reg = 0x109c,
 	.halt_check = BRANCH_HALT,
 	.clkr = {
 		.enable_reg = 0x109c,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_cxo_clk",
 			.ops = &clk_branch2_ops,
 		},
@@ -256,10 +243,10 @@ static struct clk_branch gpu_cc_gx_gfx3d_clk = {
 	.clkr = {
 		.enable_reg = 0x1054,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_gx_gfx3d_clk",
-			.parent_data = &(const struct clk_parent_data){
-				.hw = &gpu_cc_gx_gfx3d_clk_src.clkr.hw,
+			.parent_hws = (const struct clk_hw*[]) {
+				&gpu_cc_gx_gfx3d_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
 			.flags = CLK_SET_RATE_PARENT,
@@ -270,11 +257,11 @@ static struct clk_branch gpu_cc_gx_gfx3d_clk = {
 
 static struct clk_branch gpu_cc_sleep_clk = {
 	.halt_reg = 0x1090,
-	.halt_check = BRANCH_VOTED,
+	.halt_check = BRANCH_HALT_VOTED,
 	.clkr = {
 		.enable_reg = 0x1090,
 		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
+		.hw.init = &(const struct clk_init_data) {
 			.name = "gpu_cc_sleep_clk",
 			.ops = &clk_branch2_ops,
 		},
@@ -297,11 +284,14 @@ static struct clk_branch gpu_cc_hlos1_vote_gpu_smmu_clk = {
 static struct gdsc gpu_cx_gdsc = {
 	.gdscr = 0x106c,
 	.gds_hw_ctrl = 0x1540,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x2,
 	.pd = {
 		.name = "gpu_cx_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = VOTABLE,
+	.flags = RETAIN_FF_ENABLE | VOTABLE,
 };
 
 static struct gdsc gpu_gx_gdsc = {
@@ -309,21 +299,22 @@ static struct gdsc gpu_gx_gdsc = {
 	.clamp_io_ctrl = 0x1508,
 	.resets = (unsigned int []){ GPU_GX_BCR },
 	.reset_count = 1,
+	.en_rest_wait_val = 0x2,
+	.en_few_wait_val = 0x2,
+	.clk_dis_wait_val = 0x2,
 	.pd = {
 		.name = "gpu_gx_gdsc",
 	},
 	.parent = &gpu_cx_gdsc.pd,
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = CLAMP_IO | AON_RESET | SW_RESET,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE | SW_RESET | CLAMP_IO | AON_RESET,
 };
 
 static struct clk_regmap *gpu_cc_qcm2290_clocks[] = {
-	[GPU_CC_AHB_CLK] = &gpu_cc_ahb_clk.clkr,
 	[GPU_CC_CRC_AHB_CLK] = &gpu_cc_crc_ahb_clk.clkr,
 	[GPU_CC_CX_GFX3D_CLK] = &gpu_cc_cx_gfx3d_clk.clkr,
 	[GPU_CC_CX_GMU_CLK] = &gpu_cc_cx_gmu_clk.clkr,
 	[GPU_CC_CX_SNOC_DVM_CLK] = &gpu_cc_cx_snoc_dvm_clk.clkr,
-	[GPU_CC_CXO_AON_CLK] = &gpu_cc_cxo_aon_clk.clkr,
 	[GPU_CC_CXO_CLK] = &gpu_cc_cxo_clk.clkr,
 	[GPU_CC_GMU_CLK_SRC] = &gpu_cc_gmu_clk_src.clkr,
 	[GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,
@@ -342,14 +333,30 @@ static struct gdsc *gpu_cc_qcm2290_gdscs[] = {
 	[GPU_GX_GDSC] = &gpu_gx_gdsc,
 };
 
+static struct clk_alpha_pll *gpu_cc_qcm2290_plls[] = {
+	&gpu_cc_pll0,
+};
+
+static const u32 gpu_cc_qcm2290_critical_cbcrs[] = {
+	0x1078, /* GPU_CC_AHB_CLK */
+	0x1004, /* GPU_CC_CXO_AON_CLK */
+	0x1060, /* GPU_CC_GX_CXO_CLK */
+};
+
 static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
 	.val_bits = 32,
-	.max_register = 0x9000,
+	.max_register = 0x7008,
 	.fast_io = true,
 };
 
+static const struct qcom_cc_driver_data gpu_cc_qcm2290_driver_data = {
+	.alpha_plls = gpu_cc_qcm2290_plls,
+	.num_alpha_plls = ARRAY_SIZE(gpu_cc_qcm2290_plls),
+	.clk_cbcrs = gpu_cc_qcm2290_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(gpu_cc_qcm2290_critical_cbcrs),
+};
 
 static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
 	.config = &gpu_cc_qcm2290_regmap_config,
@@ -359,55 +366,22 @@ static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
 	.num_resets = ARRAY_SIZE(gpu_cc_qcm2290_resets),
 	.gdscs = gpu_cc_qcm2290_gdscs,
 	.num_gdscs = ARRAY_SIZE(gpu_cc_qcm2290_gdscs),
+	.driver_data = &gpu_cc_qcm2290_driver_data,
 };
 
 static const struct of_device_id gpu_cc_qcm2290_match_table[] = {
 	{ .compatible = "qcom,qcm2290-gpucc" },
+	{ .compatible = "qcom,shikra-gpucc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, gpu_cc_qcm2290_match_table);
 
 static int gpu_cc_qcm2290_probe(struct platform_device *pdev)
 {
-	struct regmap *regmap;
-	int ret;
-
-	regmap = qcom_cc_map(pdev, &gpu_cc_qcm2290_desc);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
-
-	ret = devm_pm_runtime_enable(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = devm_pm_clk_create(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = pm_clk_add(&pdev->dev, NULL);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to acquire ahb clock\n");
-		return ret;
-	}
-
-	ret = pm_runtime_resume_and_get(&pdev->dev);
-	if (ret)
-		return ret;
-
-	clk_huayra_2290_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
-
-	regmap_update_bits(regmap, 0x1060, BIT(0), BIT(0)); /* GPU_CC_GX_CXO_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &gpu_cc_qcm2290_desc, regmap);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to register display clock controller\n");
-		goto out_pm_runtime_put;
-	}
-
-out_pm_runtime_put:
-	pm_runtime_put_sync(&pdev->dev);
+	if (device_is_compatible(&pdev->dev, "qcom,shikra-gpucc"))
+		gpu_cc_gx_gfx3d_clk_src.freq_tbl = ftbl_gpu_cc_gx_gfx3d_clk_src_shikra;
 
-	return 0;
+	return qcom_cc_probe(pdev, &gpu_cc_qcm2290_desc);
 }
 
 static struct platform_driver gpu_cc_qcm2290_driver = {

-- 
2.34.1

Re: [PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Device Tree ABI breakage causing consumer drivers to fail probing
- [Medium] Inaccurate commit message regarding the previous state of the removed clocks
- [Low] Misuse of `qcom_branch_set_clk_en` macro for an APCS vote register
--

commit fe7153ece7b1b29e7cf0efe410ca1855c6dc922e
Author: Imran Shaik <imran.shaik@oss.qualcomm.com>

clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

This patch drops the modeling of certain clocks on the QCM2290 SoC, instead
enabling them unconditionally from probe to simplify PM clock handling in
consumer drivers.

> Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
> enabled from probe as per the latest convention. This helps to drop the

[Severity: Medium]
Is this description completely accurate? Several of the removed clocks, such
as gcc_gpu_iref_clk, gcc_video_ahb_clk, and gcc_video_xo_clk, did not have
the CLK_IS_CRITICAL flag set in their init data. 

Could the commit message be clarified to reflect that both critical and
non-critical clocks are being dropped?

> diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
> index 6684cab63ae11..937db68a45b17 100644
> --- a/drivers/clk/qcom/gcc-qcm2290.c
> +++ b/drivers/clk/qcom/gcc-qcm2290.c

[ ... ]

> @@ -2775,8 +2642,6 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
>  	[GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr,
>  	[GCC_CAM_THROTTLE_NRT_CLK] = &gcc_cam_throttle_nrt_clk.clkr,
>  	[GCC_CAM_THROTTLE_RT_CLK] = &gcc_cam_throttle_rt_clk.clkr,
> -	[GCC_CAMERA_AHB_CLK] = &gcc_camera_ahb_clk.clkr,
> -	[GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr,

[Severity: High]
Does removing these clocks completely from the provider array break Device
Tree ABI compatibility? Older device trees specify these clocks in the
'clocks' property of their consumer nodes (e.g., qcom,qcm2290-mdss requests
GCC_DISP_AHB_CLK).

When these consumer drivers boot with a newer kernel using the old DT,
functions like devm_clk_bulk_get_all() might fail with -ENOENT when they
encounter a missing clock index. 

Should these indices remain populated with a dummy clock or a basic AON clock
to prevent consumer driver probe failures?

[ ... ]

> @@ -2990,6 +2848,17 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/* Keep some clocks always-on */
> +	qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36100); /* GCC_GPU_IREF_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */

[Severity: Low]
Is qcom_branch_set_clk_en() the correct macro for GCC_SYS_NOC_CPUSS_AHB_CLK?
This macro sets the CBCR_CLOCK_ENABLE bit (BIT(0)) in a standard Clock Branch
Control Register. Register 0x79004 is an APCS vote register, not a CBCR.

While it functionally works because this specific clock's vote mask is also
BIT(0), using a CBCR-specific macro for an APCS vote register might be
brittle. 

Should direct regmap_update_bits() be used instead?

> +	qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17024); /* GCC_VIDEO_XO_CLK */
> +
>  	clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
>  	clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-shikra-dispcc-gpucc-v2-0-953f246a0fbb@oss.qualcomm.com?part=1

Re: [PATCH v2 2/5] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The conditional schema for 'qcom,shikra-dispcc' fails to enforce the presence of its 3 newly required input clocks because `minItems` is not overridden in the `then` block.
--

Note: Due to system recitation restrictions, this review does not use the
standard inline diff quoting format. Instead, a free-form summary of the
findings is provided below.

commit 6ba505cef231c7d845e3f3af3b33919e232b9b05
Author: Imran Shaik <imran.shaik@oss.qualcomm.com>

dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

This commit adds the Shikra Display clock controller by updating the
existing QCM2290 DISPCC bindings. It uses a conditional schema to support
the additional clocks required for Shikra while preserving QCM2290
constraints.

[Severity: Medium]
In the conditional block added to differentiate qcom,shikra-dispcc from
qcom,qcm2290-dispcc, does this introduce a regression in schema validation
by missing explicit minItems constraints for the clocks and clock-names
arrays?

The top-level schema defines minItems: 6 and maxItems: 9. In json-schema,
providing an items list in a conditional block validates the elements at
those indices but does not implicitly enforce the overall array length.
Could a device tree node for qcom,shikra-dispcc that provides only 6 clocks
silently pass validation because it satisfies the top-level minItems
constraint?

To properly enforce the 9 required clocks for Shikra, should minItems: 9 be
explicitly added to both clocks and clock-names inside the then block?
Similarly, could maxItems: 6 be added to the else block to ensure
qcom,qcm2290-dispcc remains restricted to exactly 6 clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-shikra-dispcc-gpucc-v2-0-953f246a0fbb@oss.qualcomm.com?part=2

Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing parent clock for DSI1 byte clock breaks DSI1 display operation.
- [Low] Semantic mismatch and dead code regarding GPLL0 divided clock.
--

commit f4a6601cc643cfa42c191307206d5dca13a8299a
Author: Imran Shaik <imran.shaik@oss.qualcomm.com>

clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

This commit updates the Qualcomm Shikra Display clock controller parent
data for mdss ahb/mdp clocks. It corrects the GDSC wait values and flags,
and introduces support for the DSI1 PHY source.

> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> index 6d88d067337fa..19c997f3fe9f1 100644
> --- a/drivers/clk/qcom/dispcc-qcm2290.c
> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
[ ... ]
> @@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
>  
>  static const struct parent_map disp_cc_parent_map_2[] = {
>  	{ P_BI_TCXO_AO, 0 },
> -	{ P_GPLL0_OUT_DIV, 4 },
> +	{ P_GPLL0_OUT_MAIN, 4 },
>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_2[] = {

[Severity: Low]
Does this change create a mismatch between the enum and the clock source?

The map now uses P_GPLL0_OUT_MAIN, but disp_cc_parent_data_2 still references
gcc_disp_gpll0_div_clk_src. This change also appears to leave P_GPLL0_OUT_DIV
entirely unused in the codebase.

> @@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
>  static const struct clk_parent_data disp_cc_parent_data_3[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .hw = &disp_cc_pll0.clkr.hw },
> -	{ .fw_name = "gcc_disp_gpll0_clk_src" },
> +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
>  };

[Severity: Low]
Similar to the map above, disp_cc_parent_data_3 now points to the divided
clock source, but its corresponding map continues to use P_GPLL0_OUT_MAIN.

Is it intended for P_GPLL0_OUT_MAIN to represent the divided clock for
both multiplexers?

>  static const struct parent_map disp_cc_parent_map_4[] = {
>  	{ P_BI_TCXO, 0 },
>  	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
> +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_4[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
> +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
>  };

[Severity: High]
Does the DSI1 PHY source also require parent data for the byte clock?

The P_DSI1_PHY_PLL_OUT_DSICLK source is added to disp_cc_parent_map_4 and
disp_cc_parent_data_4 here. However, P_DSI1_PHY_PLL_OUT_BYTECLK was added
to the enum earlier in this patch but omitted from disp_cc_parent_map_0 and
disp_cc_parent_data_0.

Since a DSI interface structurally requires both a pixel clock and a
synchronously derived byte clock from the same active PHY, could this omission
prevent the hardware from configuring the byte and escape clocks to use the
DSI1 PHY, resulting in broken DSI1 functionality?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-shikra-dispcc-gpucc-v2-0-953f246a0fbb@oss.qualcomm.com?part=4

Re: [PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:02PM +0530, Imran Shaik wrote:
> Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
> enabled from probe as per the latest convention. This helps to drop the
> pm_clk handling in QCM2290 GPUCC driver, and the same can be re-used for
> Shikra SoC.
> 
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/gcc-qcm2290.c | 153 +++--------------------------------------
>  1 file changed, 11 insertions(+), 142 deletions(-)
> 
> @@ -2012,19 +1936,6 @@ static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
>  	},
>  };
>  
> -static struct clk_branch gcc_gpu_iref_clk = {
> -	.halt_reg = 0x36100,
> -	.halt_check = BRANCH_HALT_DELAY,
> -	.clkr = {
> -		.enable_reg = 0x36100,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_gpu_iref_clk",
> -			.ops = &clk_branch2_ops,
> -		},
> -	},
> -};

This clock is not critical. Why is it being dropped?

> -
>  static struct clk_branch gcc_gpu_memnoc_gfx_clk = {
>  	.halt_reg = 0x3600c,
>  	.halt_check = BRANCH_VOTED,
> @@ -2605,21 +2500,6 @@ static struct clk_branch gcc_venus_ctl_axi_clk = {
>  	},
>  };
>  
> -static struct clk_branch gcc_video_ahb_clk = {
> -	.halt_reg = 0x17004,
> -	.halt_check = BRANCH_HALT,
> -	.hwcg_reg = 0x17004,
> -	.hwcg_bit = 1,
> -	.clkr = {
> -		.enable_reg = 0x17004,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_video_ahb_clk",
> -			.ops = &clk_branch2_ops,

This clock isn't marked as CRITICAL, why is it being dropped?

> -		},
> -	},
> -};
> -
>  static struct clk_branch gcc_video_axi0_clk = {
>  	.halt_reg = 0x1701c,
>  	.halt_check = BRANCH_HALT,
> @@ -2686,19 +2566,6 @@ static struct clk_branch gcc_video_venus_ctl_clk = {
>  	},
>  };
>  
> -static struct clk_branch gcc_video_xo_clk = {
> -	.halt_reg = 0x17024,
> -	.halt_check = BRANCH_HALT,
> -	.clkr = {
> -		.enable_reg = 0x17024,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_video_xo_clk",
> -			.ops = &clk_branch2_ops,

This clock isn't marked as CRITICAL, why is it being dropped?

> -		},
> -	},
> -};
> -
>  static struct gdsc gcc_camss_top_gdsc = {
>  	.gdscr = 0x58004,
>  	.pd = {
> @@ -2990,6 +2848,17 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/* Keep some clocks always-on */
> +	qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36100); /* GCC_GPU_IREF_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17024); /* GCC_VIDEO_XO_CLK */

If you are chancing the driver, why are you not using .clk_cbcrs?

> +
>  	clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
>  	clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);
>  	clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:05PM +0530, Imran Shaik wrote:
> The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
> but has minor differences. Update the parent data of mdss ahb/mdp clocks
> accordingly to the hardware clock plan and correct the GDSC *_wait_val and
> flags which are applicable for both QCM2290 and Shikra SoC, and add the
> support for DSI1 PHY source.
> 
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/dispcc-qcm2290.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> index 6d88d067337fa132114b0d8666931b449f86de17..19c997f3fe9f197d2c252a9dd1e8169947200f5f 100644
> --- a/drivers/clk/qcom/dispcc-qcm2290.c
> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>   * Copyright (c) 2021, Linaro Ltd.
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>   */
>  
>  #include <linux/clk-provider.h>
> @@ -32,6 +33,8 @@ enum {
>  	P_GPLL0_OUT_DIV,
>  	P_GPLL0_OUT_MAIN,
>  	P_SLEEP_CLK,
> +	P_DSI1_PHY_PLL_OUT_BYTECLK,
> +	P_DSI1_PHY_PLL_OUT_DSICLK,
>  };
>  
>  static const struct pll_vco spark_vco[] = {
> @@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
>  
>  static const struct parent_map disp_cc_parent_map_2[] = {
>  	{ P_BI_TCXO_AO, 0 },
> -	{ P_GPLL0_OUT_DIV, 4 },
> +	{ P_GPLL0_OUT_MAIN, 4 },

Why?

>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_2[] = {
> @@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
>  static const struct clk_parent_data disp_cc_parent_data_3[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .hw = &disp_cc_pll0.clkr.hw },
> -	{ .fw_name = "gcc_disp_gpll0_clk_src" },
> +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },

Do you realize that this is an undocumented ABI chance?

>  };
>  
>  static const struct parent_map disp_cc_parent_map_4[] = {
>  	{ P_BI_TCXO, 0 },
>  	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
> +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
>  };
>  
>  static const struct clk_parent_data disp_cc_parent_data_4[] = {
>  	{ .fw_name = "bi_tcxo" },
>  	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
> +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
>  };
>  
>  static const struct parent_map disp_cc_parent_map_5[] = {
> @@ -153,8 +158,8 @@ static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
>  
>  static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
>  	F(19200000, P_BI_TCXO_AO, 1, 0, 0),
> -	F(37500000, P_GPLL0_OUT_DIV, 8, 0, 0),
> -	F(75000000, P_GPLL0_OUT_DIV, 4, 0, 0),
> +	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
> +	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),

Why? It's not mentioned in the commit message.

>  	{ }
>  };
>  
> @@ -450,11 +455,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
>  
>  static struct gdsc mdss_gdsc = {
>  	.gdscr = 0x3000,
> +	.en_rest_wait_val = 0x2,
> +	.en_few_wait_val = 0x2,
> +	.clk_dis_wait_val = 0xf,
>  	.pd = {
>  		.name = "mdss_gdsc",
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = HW_CTRL,
> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,

And this also needs explanation. 

>  };

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/5] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:03PM +0530, Imran Shaik wrote:
> The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
> but has minor differences with the number of input clocks. Update the
> existing QCM2290 DISPCC bindings using conditional schema so that the
> QCM2290 requirements remain unchanged while accommodating the additional
> clocks required for Shikra.

According to the IPcat, display clock controller also has (unused)
inputs for the DSI1. Please extend the ABI for Agatti, then extend add
Shikra.

> 
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
>  .../bindings/clock/qcom,qcm2290-dispcc.yaml        | 75 +++++++++++++++++-----
>  1 file changed, 60 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> index 4a533b45eec2d8e7b866c3436bfe6f80fcd714fb..b24095814d9e67a355321d9013e144f245077322 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,qcm2290-dispcc.yaml
> @@ -17,25 +17,21 @@ description: |
>  
>  properties:
>    compatible:
> -    const: qcom,qcm2290-dispcc
> +    oneOf:
> +      - items:
> +          - enum:
> +              - qcom,shikra-dispcc
> +          - const: qcom,qcm2290-dispcc
> +      - enum:
> +          - qcom,qcm2290-dispcc
>  
>    clocks:
> -    items:
> -      - description: Board XO source
> -      - description: Board active-only XO source
> -      - description: GPLL0 source from GCC
> -      - description: GPLL0 div source from GCC
> -      - description: Byte clock from DSI PHY
> -      - description: Pixel clock from DSI PHY
> +    minItems: 6
> +    maxItems: 9
>  
>    clock-names:
> -    items:
> -      - const: bi_tcxo
> -      - const: bi_tcxo_ao
> -      - const: gcc_disp_gpll0_clk_src
> -      - const: gcc_disp_gpll0_div_clk_src
> -      - const: dsi0_phy_pll_out_byteclk
> -      - const: dsi0_phy_pll_out_dsiclk
> +    minItems: 6
> +    maxItems: 9
>  
>  required:
>    - compatible
> @@ -45,6 +41,55 @@ required:
>  
>  allOf:
>    - $ref: qcom,gcc.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,shikra-dispcc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Board XO source
> +            - description: Board active-only XO source
> +            - description: GPLL0 source from GCC
> +            - description: GPLL0 div source from GCC
> +            - description: Byte clock from DSI PHY0
> +            - description: Pixel clock from DSI PHY0
> +            - description: Byte clock from DSI PHY1
> +            - description: Pixel clock from DSI PHY1
> +            - description: Board sleep clock
> +
> +        clock-names:
> +          items:
> +            - const: bi_tcxo
> +            - const: bi_tcxo_ao
> +            - const: gcc_disp_gpll0_clk_src
> +            - const: gcc_disp_gpll0_div_clk_src
> +            - const: dsi0_phy_pll_out_byteclk
> +            - const: dsi0_phy_pll_out_dsiclk
> +            - const: dsi1_phy_pll_out_byteclk
> +            - const: dsi1_phy_pll_out_dsiclk
> +            - const: sleep_clk
> +    else:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Board XO source
> +            - description: Board active-only XO source
> +            - description: GPLL0 source from GCC
> +            - description: GPLL0 div source from GCC
> +            - description: Byte clock from DSI PHY
> +            - description: Pixel clock from DSI PHY
> +
> +        clock-names:
> +          items:
> +            - const: bi_tcxo
> +            - const: bi_tcxo_ao
> +            - const: gcc_disp_gpll0_clk_src
> +            - const: gcc_disp_gpll0_div_clk_src
> +            - const: dsi0_phy_pll_out_byteclk
> +            - const: dsi0_phy_pll_out_dsiclk
>  
>  unevaluatedProperties: false
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:06PM +0530, Imran Shaik wrote:
> The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
> hardware block, with minor differences. Drop modelling of critical
> clocks and keep them enabled at probe time, update the QCM2290 GPUCC
> driver to align with the latest common qcom_cc_probe() model. Update the
> GDSC *_wait_val and flags which are applicable for both QCM2290 and Shikra.

There are a lot of separate, unrelated changes. Split your patches (not
only this one) following the 'atomic logical changes' approach.

> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/gpucc-qcm2290.c | 174 +++++++++++++++++----------------------
>  1 file changed, 74 insertions(+), 100 deletions(-)
> 
-- 
With best wishes
Dmitry

Re: [PATCH v2 5/5] clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Device Tree ABI Regression / Broken Probe
- [High] Missing PM Runtime Support / Hardware Access Fault
- [Medium] Global State Mutation / State Pollution
--

commit 5b994f8a64d2619757c52d0d0d48b2155beacbbb
Author: Imran Shaik <imran.shaik@oss.qualcomm.com>

clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
hardware block, with minor differences. Drop modelling of critical
clocks and keep them enabled at probe time, update the QCM2290 GPUCC
driver to align with the latest common qcom_cc_probe() model. Update the
GDSC *_wait_val and flags which are applicable for both QCM2290 and Shikra.

> diff --git a/drivers/clk/qcom/gpucc-qcm2290.c b/drivers/clk/qcom/gpucc-qcm2290.c
> index dc369dff882e6..296afcbfe0c9e 100644
> --- a/drivers/clk/qcom/gpucc-qcm2290.c
> +++ b/drivers/clk/qcom/gpucc-qcm2290.c
[ ... ]
>  static struct clk_regmap *gpu_cc_qcm2290_clocks[] = {
> -	[GPU_CC_AHB_CLK] = &gpu_cc_ahb_clk.clkr,
>  	[GPU_CC_CRC_AHB_CLK] = &gpu_cc_crc_ahb_clk.clkr,
>  	[GPU_CC_CX_GFX3D_CLK] = &gpu_cc_cx_gfx3d_clk.clkr,
>  	[GPU_CC_CX_GMU_CLK] = &gpu_cc_cx_gmu_clk.clkr,
>  	[GPU_CC_CX_SNOC_DVM_CLK] = &gpu_cc_cx_snoc_dvm_clk.clkr,
> -	[GPU_CC_CXO_AON_CLK] = &gpu_cc_cxo_aon_clk.clkr,
>  	[GPU_CC_CXO_CLK] = &gpu_cc_cxo_clk.clkr,
>  	[GPU_CC_GMU_CLK_SRC] = &gpu_cc_gmu_clk_src.clkr,
>  	[GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,

[Severity: Critical]
Does removing these clocks from the array break the established Device Tree ABI
for qcom,qcm2290-gpucc?

Because their indices in the array are now uninitialized, the clock provider's
qcom_cc_clk_hw_get() will return NULL, causing dependent drivers referencing
these clocks to fail to probe.

[ ... ]
> @@ -359,55 +366,22 @@ static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
>  	.config = &gpu_cc_qcm2290_regmap_config,
>  	.clks = gpu_cc_qcm2290_clocks,
>  	.num_clks = ARRAY_SIZE(gpu_cc_qcm2290_clocks),
>  	.resets = gpu_cc_qcm2290_resets,
>  	.num_resets = ARRAY_SIZE(gpu_cc_qcm2290_resets),
>  	.gdscs = gpu_cc_qcm2290_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gpu_cc_qcm2290_gdscs),
> +	.driver_data = &gpu_cc_qcm2290_driver_data,
>  };

[Severity: High]
Does this description need to set .use_rpm = true?

Prior to this patch, the driver explicitly enabled PM runtime and called
pm_runtime_resume_and_get(). By omitting .use_rpm = true here,
qcom_cc_really_probe() will skip the PM runtime resume sequence.

Accessing unclocked or unpowered registers to configure PLLs and critical
CBCRs could trigger a synchronous external abort.

[ ... ]
>  static int gpu_cc_qcm2290_probe(struct platform_device *pdev)
>  {
> -	struct regmap *regmap;
> -	int ret;
[ ... ]
> -out_pm_runtime_put:
> -	pm_runtime_put_sync(&pdev->dev);
> +	if (device_is_compatible(&pdev->dev, "qcom,shikra-gpucc"))
> +		gpu_cc_gx_gfx3d_clk_src.freq_tbl = ftbl_gpu_cc_gx_gfx3d_clk_src_shikra;
>  
> -	return 0;
> +	return qcom_cc_probe(pdev, &gpu_cc_qcm2290_desc);
>  }

[Severity: Medium]
Does this code mutate static module data?

Because gpu_cc_gx_gfx3d_clk_src is a static global structure, dynamically
assigning a new frequency table here will permanently pollute the shared table
for the entire driver module.

Could this cause issues during driver unbind/rebind cycles?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260528-shikra-dispcc-gpucc-v2-0-953f246a0fbb@oss.qualcomm.com?part=5

Re: [PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[Imran Shaik]

On 28-05-2026 06:52 pm, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 03:37:02PM +0530, Imran Shaik wrote:
>> Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
>> enabled from probe as per the latest convention. This helps to drop the
>> pm_clk handling in QCM2290 GPUCC driver, and the same can be re-used for
>> Shikra SoC.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/clk/qcom/gcc-qcm2290.c | 153 +++--------------------------------------
>>   1 file changed, 11 insertions(+), 142 deletions(-)
>>
>> @@ -2012,19 +1936,6 @@ static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
>>   	},
>>   };
>>   
>> -static struct clk_branch gcc_gpu_iref_clk = {
>> -	.halt_reg = 0x36100,
>> -	.halt_check = BRANCH_HALT_DELAY,
>> -	.clkr = {
>> -		.enable_reg = 0x36100,
>> -		.enable_mask = BIT(0),
>> -		.hw.init = &(struct clk_init_data){
>> -			.name = "gcc_gpu_iref_clk",
>> -			.ops = &clk_branch2_ops,
>> -		},
>> -	},
>> -};
> 
> This clock is not critical. Why is it being dropped?
> 
>> -
>>   static struct clk_branch gcc_gpu_memnoc_gfx_clk = {
>>   	.halt_reg = 0x3600c,
>>   	.halt_check = BRANCH_VOTED,
>> @@ -2605,21 +2500,6 @@ static struct clk_branch gcc_venus_ctl_axi_clk = {
>>   	},
>>   };
>>   
>> -static struct clk_branch gcc_video_ahb_clk = {
>> -	.halt_reg = 0x17004,
>> -	.halt_check = BRANCH_HALT,
>> -	.hwcg_reg = 0x17004,
>> -	.hwcg_bit = 1,
>> -	.clkr = {
>> -		.enable_reg = 0x17004,
>> -		.enable_mask = BIT(0),
>> -		.hw.init = &(struct clk_init_data){
>> -			.name = "gcc_video_ahb_clk",
>> -			.ops = &clk_branch2_ops,
> 
> This clock isn't marked as CRITICAL, why is it being dropped?
> 
>> -		},
>> -	},
>> -};
>> -
>>   static struct clk_branch gcc_video_axi0_clk = {
>>   	.halt_reg = 0x1701c,
>>   	.halt_check = BRANCH_HALT,
>> @@ -2686,19 +2566,6 @@ static struct clk_branch gcc_video_venus_ctl_clk = {
>>   	},
>>   };
>>   
>> -static struct clk_branch gcc_video_xo_clk = {
>> -	.halt_reg = 0x17024,
>> -	.halt_check = BRANCH_HALT,
>> -	.clkr = {
>> -		.enable_reg = 0x17024,
>> -		.enable_mask = BIT(0),
>> -		.hw.init = &(struct clk_init_data){
>> -			.name = "gcc_video_xo_clk",
>> -			.ops = &clk_branch2_ops,
> 
> This clock isn't marked as CRITICAL, why is it being dropped?
> 

The above clocks also should have been marked as CRITICAL. Not sure why 
those are not marked in the QCM2290. Since we are updating the critical 
clock set, moving all the required clocks to the critical list in line 
with the latest conventions.

>> -		},
>> -	},
>> -};
>> -
>>   static struct gdsc gcc_camss_top_gdsc = {
>>   	.gdscr = 0x58004,
>>   	.pd = {
>> @@ -2990,6 +2848,17 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	/* Keep some clocks always-on */
>> +	qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x36100); /* GCC_GPU_IREF_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */
>> +	qcom_branch_set_clk_en(regmap, 0x17024); /* GCC_VIDEO_XO_CLK */
> 
> If you are chancing the driver, why are you not using .clk_cbcrs?
> 

Sure, will add these to .clk_cbcrs in next series.

Thanks,
Imran

>> +
>>   	clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
>>   	clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);
>>   	clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v2 2/5] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

[Imran Shaik]

On 28-05-2026 07:14 pm, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 03:37:03PM +0530, Imran Shaik wrote:
>> The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
>> but has minor differences with the number of input clocks. Update the
>> existing QCM2290 DISPCC bindings using conditional schema so that the
>> QCM2290 requirements remain unchanged while accommodating the additional
>> clocks required for Shikra.
> 
> According to the IPcat, display clock controller also has (unused)
> inputs for the DSI1. Please extend the ABI for Agatti, then extend add
> Shikra.
> 

Sure, will add the DSI1 to Agatti as well. Since we are extending the 
ABI for Agatti, I will remove the clock-names from the bindings and 
update the DISPCC driver to use the indices-based approach instead.

With these updates, the bindings, driver, and Agatti DT must be picked 
together to avoid any functional issues or ABI breakage. I will 
explicitly document these details in the commit message in the next series.

Thanks,
Imran

Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[Imran Shaik]

On 28-05-2026 07:12 pm, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 03:37:05PM +0530, Imran Shaik wrote:
>> The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
>> but has minor differences. Update the parent data of mdss ahb/mdp clocks
>> accordingly to the hardware clock plan and correct the GDSC *_wait_val and
>> flags which are applicable for both QCM2290 and Shikra SoC, and add the
>> support for DSI1 PHY source.
>>
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/clk/qcom/dispcc-qcm2290.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
>> index 6d88d067337fa132114b0d8666931b449f86de17..19c997f3fe9f197d2c252a9dd1e8169947200f5f 100644
>> --- a/drivers/clk/qcom/dispcc-qcm2290.c
>> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
>> @@ -2,6 +2,7 @@
>>   /*
>>    * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>>    * Copyright (c) 2021, Linaro Ltd.
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>    */
>>   
>>   #include <linux/clk-provider.h>
>> @@ -32,6 +33,8 @@ enum {
>>   	P_GPLL0_OUT_DIV,
>>   	P_GPLL0_OUT_MAIN,
>>   	P_SLEEP_CLK,
>> +	P_DSI1_PHY_PLL_OUT_BYTECLK,
>> +	P_DSI1_PHY_PLL_OUT_DSICLK,
>>   };
>>   
>>   static const struct pll_vco spark_vco[] = {
>> @@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
>>   
>>   static const struct parent_map disp_cc_parent_map_2[] = {
>>   	{ P_BI_TCXO_AO, 0 },
>> -	{ P_GPLL0_OUT_DIV, 4 },
>> +	{ P_GPLL0_OUT_MAIN, 4 },
> 
> Why?
> 
>>   };
>>   
>>   static const struct clk_parent_data disp_cc_parent_data_2[] = {
>> @@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
>>   static const struct clk_parent_data disp_cc_parent_data_3[] = {
>>   	{ .fw_name = "bi_tcxo" },
>>   	{ .hw = &disp_cc_pll0.clkr.hw },
>> -	{ .fw_name = "gcc_disp_gpll0_clk_src" },
>> +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
> 
> Do you realize that this is an undocumented ABI chance?
> 
>>   };
>>   
>>   static const struct parent_map disp_cc_parent_map_4[] = {
>>   	{ P_BI_TCXO, 0 },
>>   	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
>> +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
>>   };
>>   
>>   static const struct clk_parent_data disp_cc_parent_data_4[] = {
>>   	{ .fw_name = "bi_tcxo" },
>>   	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
>> +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
>>   };
>>   
>>   static const struct parent_map disp_cc_parent_map_5[] = {
>> @@ -153,8 +158,8 @@ static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
>>   
>>   static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
>>   	F(19200000, P_BI_TCXO_AO, 1, 0, 0),
>> -	F(37500000, P_GPLL0_OUT_DIV, 8, 0, 0),
>> -	F(75000000, P_GPLL0_OUT_DIV, 4, 0, 0),
>> +	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
>> +	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
> 
> Why? It's not mentioned in the commit message.
> 

As per the HW clock plan, the ahb(parent_*_2)/mdp(parent_*_3) clock RCG 
parent must be GPLL0_OUT_MAIN (gcc_disp_gpll0_div_clk_src). Updated them 
accordingly and mentioned the same in the commit text below:

"Update the parent data of mdss ahb/mdp clocks accordingly to the 
hardware clock plan"

Will move these to indices approach, and add in a separate commit as you 
mentioned in the other patch comment.

>>   	{ }
>>   };
>>   
>> @@ -450,11 +455,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
>>   
>>   static struct gdsc mdss_gdsc = {
>>   	.gdscr = 0x3000,
>> +	.en_rest_wait_val = 0x2,
>> +	.en_few_wait_val = 0x2,
>> +	.clk_dis_wait_val = 0xf,
>>   	.pd = {
>>   		.name = "mdss_gdsc",
>>   	},
>>   	.pwrsts = PWRSTS_OFF_ON,
>> -	.flags = HW_CTRL,
>> +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> 
> And this also needs explanation.
> 

Sure, will add these GDSC fixes in separate commit in next series.

Thanks,
Imran

>>   };
> 

Re: [PATCH v2 5/5] clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra

[Imran Shaik]

On 28-05-2026 07:16 pm, Dmitry Baryshkov wrote:
> On Thu, May 28, 2026 at 03:37:06PM +0530, Imran Shaik wrote:
>> The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
>> hardware block, with minor differences. Drop modelling of critical
>> clocks and keep them enabled at probe time, update the QCM2290 GPUCC
>> driver to align with the latest common qcom_cc_probe() model. Update the
>> GDSC *_wait_val and flags which are applicable for both QCM2290 and Shikra.
> 
> There are a lot of separate, unrelated changes. Split your patches (not
> only this one) following the 'atomic logical changes' approach.
> 

Sure, will split the patches in next series.

Thanks,
Imran

>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
>>   drivers/clk/qcom/gpucc-qcm2290.c | 174 +++++++++++++++++----------------------
>>   1 file changed, 74 insertions(+), 100 deletions(-)
>>

Re: [PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 02:52:39PM +0530, Imran Shaik wrote:
> 
> 
> On 28-05-2026 06:52 pm, Dmitry Baryshkov wrote:
> > On Thu, May 28, 2026 at 03:37:02PM +0530, Imran Shaik wrote:
> > > Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
> > > enabled from probe as per the latest convention. This helps to drop the
> > > pm_clk handling in QCM2290 GPUCC driver, and the same can be re-used for
> > > Shikra SoC.
> > > 
> > > Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> > > ---
> > >   drivers/clk/qcom/gcc-qcm2290.c | 153 +++--------------------------------------
> > >   1 file changed, 11 insertions(+), 142 deletions(-)
> > > 
> > > @@ -2012,19 +1936,6 @@ static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
> > >   	},
> > >   };
> > > -static struct clk_branch gcc_gpu_iref_clk = {
> > > -	.halt_reg = 0x36100,
> > > -	.halt_check = BRANCH_HALT_DELAY,
> > > -	.clkr = {
> > > -		.enable_reg = 0x36100,
> > > -		.enable_mask = BIT(0),
> > > -		.hw.init = &(struct clk_init_data){
> > > -			.name = "gcc_gpu_iref_clk",
> > > -			.ops = &clk_branch2_ops,
> > > -		},
> > > -	},
> > > -};
> > 
> > This clock is not critical. Why is it being dropped?
> > 
> > > -
> > >   static struct clk_branch gcc_gpu_memnoc_gfx_clk = {
> > >   	.halt_reg = 0x3600c,
> > >   	.halt_check = BRANCH_VOTED,
> > > @@ -2605,21 +2500,6 @@ static struct clk_branch gcc_venus_ctl_axi_clk = {
> > >   	},
> > >   };
> > > -static struct clk_branch gcc_video_ahb_clk = {
> > > -	.halt_reg = 0x17004,
> > > -	.halt_check = BRANCH_HALT,
> > > -	.hwcg_reg = 0x17004,
> > > -	.hwcg_bit = 1,
> > > -	.clkr = {
> > > -		.enable_reg = 0x17004,
> > > -		.enable_mask = BIT(0),
> > > -		.hw.init = &(struct clk_init_data){
> > > -			.name = "gcc_video_ahb_clk",
> > > -			.ops = &clk_branch2_ops,
> > 
> > This clock isn't marked as CRITICAL, why is it being dropped?
> > 
> > > -		},
> > > -	},
> > > -};
> > > -
> > >   static struct clk_branch gcc_video_axi0_clk = {
> > >   	.halt_reg = 0x1701c,
> > >   	.halt_check = BRANCH_HALT,
> > > @@ -2686,19 +2566,6 @@ static struct clk_branch gcc_video_venus_ctl_clk = {
> > >   	},
> > >   };
> > > -static struct clk_branch gcc_video_xo_clk = {
> > > -	.halt_reg = 0x17024,
> > > -	.halt_check = BRANCH_HALT,
> > > -	.clkr = {
> > > -		.enable_reg = 0x17024,
> > > -		.enable_mask = BIT(0),
> > > -		.hw.init = &(struct clk_init_data){
> > > -			.name = "gcc_video_xo_clk",
> > > -			.ops = &clk_branch2_ops,
> > 
> > This clock isn't marked as CRITICAL, why is it being dropped?
> > 
> 
> The above clocks also should have been marked as CRITICAL. Not sure why
> those are not marked in the QCM2290. Since we are updating the critical
> clock set, moving all the required clocks to the critical list in line with
> the latest conventions.

Sure, but it doesn't match the commit message. Explain that you are
making them always-on, because...


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/5] dt-bindings: clock: qcom: Add Qualcomm Shikra Display clock controller

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 02:53:29PM +0530, Imran Shaik wrote:
> 
> 
> On 28-05-2026 07:14 pm, Dmitry Baryshkov wrote:
> > On Thu, May 28, 2026 at 03:37:03PM +0530, Imran Shaik wrote:
> > > The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
> > > but has minor differences with the number of input clocks. Update the
> > > existing QCM2290 DISPCC bindings using conditional schema so that the
> > > QCM2290 requirements remain unchanged while accommodating the additional
> > > clocks required for Shikra.
> > 
> > According to the IPcat, display clock controller also has (unused)
> > inputs for the DSI1. Please extend the ABI for Agatti, then extend add
> > Shikra.
> > 
> 
> Sure, will add the DSI1 to Agatti as well. Since we are extending the ABI
> for Agatti, I will remove the clock-names from the bindings

Why?

> and update the
> DISPCC driver to use the indices-based approach instead.
> 
> With these updates, the bindings, driver, and Agatti DT must be picked
> together to avoid any functional issues or ABI breakage. I will explicitly
> document these details in the commit message in the next series.

Well, no. DT comes through a separate tree anyway. Be sure that the
updated driver works with the old DT. Also make sure to not change the
order of the clocks, you can't break the ABI.

> 
> Thanks,
> Imran

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/5] clk: qcom: dispcc-qcm2290: Add support for Qualcomm Shikra DISPCC

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 02:53:45PM +0530, Imran Shaik wrote:
> 
> 
> On 28-05-2026 07:12 pm, Dmitry Baryshkov wrote:
> > On Thu, May 28, 2026 at 03:37:05PM +0530, Imran Shaik wrote:
> > > The Qualcomm Shikra Display clock controller reuses the QCM2290 DISPCC,
> > > but has minor differences. Update the parent data of mdss ahb/mdp clocks
> > > accordingly to the hardware clock plan and correct the GDSC *_wait_val and
> > > flags which are applicable for both QCM2290 and Shikra SoC, and add the
> > > support for DSI1 PHY source.
> > > 
> > > Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> > > ---
> > >   drivers/clk/qcom/dispcc-qcm2290.c | 18 +++++++++++++-----
> > >   1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> > > index 6d88d067337fa132114b0d8666931b449f86de17..19c997f3fe9f197d2c252a9dd1e8169947200f5f 100644
> > > --- a/drivers/clk/qcom/dispcc-qcm2290.c
> > > +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> > > @@ -2,6 +2,7 @@
> > >   /*
> > >    * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> > >    * Copyright (c) 2021, Linaro Ltd.
> > > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > >    */
> > >   #include <linux/clk-provider.h>
> > > @@ -32,6 +33,8 @@ enum {
> > >   	P_GPLL0_OUT_DIV,
> > >   	P_GPLL0_OUT_MAIN,
> > >   	P_SLEEP_CLK,
> > > +	P_DSI1_PHY_PLL_OUT_BYTECLK,
> > > +	P_DSI1_PHY_PLL_OUT_DSICLK,
> > >   };
> > >   static const struct pll_vco spark_vco[] = {
> > > @@ -84,7 +87,7 @@ static const struct clk_parent_data disp_cc_parent_data_1[] = {
> > >   static const struct parent_map disp_cc_parent_map_2[] = {
> > >   	{ P_BI_TCXO_AO, 0 },
> > > -	{ P_GPLL0_OUT_DIV, 4 },
> > > +	{ P_GPLL0_OUT_MAIN, 4 },
> > 
> > Why?
> > 
> > >   };
> > >   static const struct clk_parent_data disp_cc_parent_data_2[] = {
> > > @@ -101,17 +104,19 @@ static const struct parent_map disp_cc_parent_map_3[] = {
> > >   static const struct clk_parent_data disp_cc_parent_data_3[] = {
> > >   	{ .fw_name = "bi_tcxo" },
> > >   	{ .hw = &disp_cc_pll0.clkr.hw },
> > > -	{ .fw_name = "gcc_disp_gpll0_clk_src" },
> > > +	{ .fw_name = "gcc_disp_gpll0_div_clk_src" },
> > 
> > Do you realize that this is an undocumented ABI chance?
> > 
> > >   };
> > >   static const struct parent_map disp_cc_parent_map_4[] = {
> > >   	{ P_BI_TCXO, 0 },
> > >   	{ P_DSI0_PHY_PLL_OUT_DSICLK, 1 },
> > > +	{ P_DSI1_PHY_PLL_OUT_DSICLK, 2 },
> > >   };
> > >   static const struct clk_parent_data disp_cc_parent_data_4[] = {
> > >   	{ .fw_name = "bi_tcxo" },
> > >   	{ .fw_name = "dsi0_phy_pll_out_dsiclk" },
> > > +	{ .fw_name = "dsi1_phy_pll_out_dsiclk" },
> > >   };
> > >   static const struct parent_map disp_cc_parent_map_5[] = {
> > > @@ -153,8 +158,8 @@ static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
> > >   static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
> > >   	F(19200000, P_BI_TCXO_AO, 1, 0, 0),
> > > -	F(37500000, P_GPLL0_OUT_DIV, 8, 0, 0),
> > > -	F(75000000, P_GPLL0_OUT_DIV, 4, 0, 0),
> > > +	F(37500000, P_GPLL0_OUT_MAIN, 8, 0, 0),
> > > +	F(75000000, P_GPLL0_OUT_MAIN, 4, 0, 0),
> > 
> > Why? It's not mentioned in the commit message.
> > 
> 
> As per the HW clock plan, the ahb(parent_*_2)/mdp(parent_*_3) clock RCG
> parent must be GPLL0_OUT_MAIN (gcc_disp_gpll0_div_clk_src). Updated them
> accordingly and mentioned the same in the commit text below:
> 
> "Update the parent data of mdss ahb/mdp clocks accordingly to the hardware
> clock plan"
> 
> Will move these to indices approach, and add in a separate commit as you
> mentioned in the other patch comment.


Why are you trying to move to indices?

> > >   	{ }
> > >   };
> > > @@ -450,11 +455,14 @@ static const struct qcom_reset_map disp_cc_qcm2290_resets[] = {
> > >   static struct gdsc mdss_gdsc = {
> > >   	.gdscr = 0x3000,
> > > +	.en_rest_wait_val = 0x2,
> > > +	.en_few_wait_val = 0x2,
> > > +	.clk_dis_wait_val = 0xf,
> > >   	.pd = {
> > >   		.name = "mdss_gdsc",
> > >   	},
> > >   	.pwrsts = PWRSTS_OFF_ON,
> > > -	.flags = HW_CTRL,
> > > +	.flags = HW_CTRL_TRIGGER | POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
> > 
> > And this also needs explanation.
> > 
> 
> Sure, will add these GDSC fixes in separate commit in next series.

With the proper justification, please.

> 
> Thanks,
> Imran
> 
> > >   };
> > 
> 

-- 
With best wishes
Dmitry