[PATCH v2 00/11] clk: qcom: sc8180x: PM-related fixes (and refactoring)

[PATCH v2 00/11] clk: qcom: sc8180x: PM-related fixes (and refactoring)

[Val Packett]

v2:
- collect tags
- bring in dispcc patch mentioned in cover letter
- add a couple new discoveries (mostly camcc related)
- add refactoring to current style (qcom_cc_driver_data)

v1: https://lore.kernel.org/all/20260309010214.224621-1-val@packett.cool/

Also bonus new question: I found
https://git.codelinaro.org/clo/la/kernel/msm-4.14/-/commit/489da2db487312fdaf9441056745490031b59f55
and almost included a patch adding `.skip_retention_level = true` to MMCX
in rpmhpd for the SoC, but.. it should not be necessary, right?
Just having low_svs as the requested OPP in the consumers in the DT
shooould be enough to make sure it's always at least at low_svs?
(I *thought* it might've helped with the GDSC stuck-off issue for
both Titan and MDSS but it's more likely the other fixes..)

~val

Val Packett (11):
  dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs
  clk: qcom: gcc-sc8180x: Add missing GDSCs
  clk: qcom: gcc-sc8180x: Use retention for USB power domains
  clk: qcom: gcc-sc8180x: Use retention for PCIe power domains
  clk: qcom: gcc-sc8180x: Enable runtime PM support
  clk: qcom: gcc-sc8180x: Refactor to use qcom_cc_driver_data
  clk: qcom: dispcc-sm8250: Use shared ops on the mdss vsync clk
  clk: qcom: dispcc-sm8250: Enable parents for pixel clocks
  clk: qcom: camcc-sc8180x: Remove wait_val for Titan GDSC
  clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag
  clk: qcom: camcc-sc8180x: Refactor to use qcom_cc_driver_data

 drivers/clk/qcom/camcc-sc8180x.c             |  76 ++++++-----
 drivers/clk/qcom/dispcc-sm8250.c             |   6 +-
 drivers/clk/qcom/gcc-sc8180x.c               | 126 +++++++++++++------
 include/dt-bindings/clock/qcom,gcc-sc8180x.h |   5 +
 4 files changed, 132 insertions(+), 81 deletions(-)

-- 
2.52.0

[PATCH v2 01/11] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs

[Val Packett]

There are 5 more GDSCs that we were ignoring and not putting to sleep,
which are listed in downstream DTS. Add them.

Signed-off-by: Val Packett <val@packett.cool>
---
 include/dt-bindings/clock/qcom,gcc-sc8180x.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
index b9d8438a15ff..9ed7b794aacc 100644
--- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
+++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
@@ -322,5 +322,10 @@
 #define USB30_MP_GDSC						8
 #define USB30_PRIM_GDSC						9
 #define USB30_SEC_GDSC						10
+#define HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC		11
+#define HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC		12
+#define HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC		13
+#define HLOS1_VOTE_TURING_MMU_TBU0_GDSC			14
+#define HLOS1_VOTE_TURING_MMU_TBU1_GDSC			15
 
 #endif
-- 
2.52.0

Re: [PATCH v2 01/11] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs

[Krzysztof Kozlowski]

On Thu, Mar 12, 2026 at 08:12:06AM -0300, Val Packett wrote:
> There are 5 more GDSCs that we were ignoring and not putting to sleep,
> which are listed in downstream DTS. Add them.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  include/dt-bindings/clock/qcom,gcc-sc8180x.h | 5 +++++
>  1 file changed, 5 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v2 02/11] clk: qcom: gcc-sc8180x: Add missing GDSCs

[Val Packett]

There are 5 more GDSCs that we were ignoring and not putting to sleep,
which are listed in downstream DTS. Add them.

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 50 ++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 31e788e22ab4..55dabf6259b2 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4266,6 +4266,51 @@ static struct gdsc usb30_mp_gdsc = {
 	.flags = POLL_CFG_GDSCR,
 };
 
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
+	.gdscr = 0x7d050,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc = {
+	.gdscr = 0x7d058,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_sf_gdsc = {
+	.gdscr = 0x7d054,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_sf_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_turing_mmu_tbu0_gdsc = {
+	.gdscr = 0x7d05c,
+	.pd = {
+		.name = "hlos1_vote_turing_mmu_tbu0_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_turing_mmu_tbu1_gdsc = {
+	.gdscr = 0x7d060,
+	.pd = {
+		.name = "hlos1_vote_turing_mmu_tbu1_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
 static struct clk_regmap *gcc_sc8180x_clocks[] = {
 	[GCC_AGGRE_NOC_PCIE_TBU_CLK] = &gcc_aggre_noc_pcie_tbu_clk.clkr,
 	[GCC_AGGRE_UFS_CARD_AXI_CLK] = &gcc_aggre_ufs_card_axi_clk.clkr,
@@ -4595,6 +4640,11 @@ static struct gdsc *gcc_sc8180x_gdscs[] = {
 	[USB30_MP_GDSC] = &usb30_mp_gdsc,
 	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
 	[USB30_SEC_GDSC] = &usb30_sec_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_sf_gdsc,
+	[HLOS1_VOTE_TURING_MMU_TBU0_GDSC] = &hlos1_vote_turing_mmu_tbu0_gdsc,
+	[HLOS1_VOTE_TURING_MMU_TBU1_GDSC] = &hlos1_vote_turing_mmu_tbu1_gdsc,
 };
 
 static const struct regmap_config gcc_sc8180x_regmap_config = {
-- 
2.52.0

[PATCH v2 03/11] clk: qcom: gcc-sc8180x: Use retention for USB power domains

[Val Packett]

The USB subsystem does not expect to lose its state on suspend:

    xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
    usb usb1: root hub lost power or was reset

(The reinitialization usually succeeds, but it does slow down resume.)

To maintain state during suspend, the relevant GDSCs need to stay in
retention mode, like they do on other similar SoCs. Change the mode to
PWRSTS_RET_ON to fix.

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 55dabf6259b2..b116a9c0b2d9 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4172,7 +4172,7 @@ static struct gdsc usb30_sec_gdsc = {
 	.pd = {
 		.name = "usb30_sec_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4190,7 +4190,7 @@ static struct gdsc usb30_prim_gdsc = {
 	.pd = {
 		.name = "usb30_prim_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4262,7 +4262,7 @@ static struct gdsc usb30_mp_gdsc = {
 	.pd = {
 		.name = "usb30_mp_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
-- 
2.52.0

[PATCH v2 04/11] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains

[Val Packett]

As the PCIe host controller driver does not yet support dealing with the
loss of state during suspend, use retention for relevant GDSCs.

This fixes the link not surviving upon resume:

    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
    nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
    nvme nvme0: Disabling device after reset failure: -19

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index b116a9c0b2d9..4095a1f54a09 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4199,7 +4199,7 @@ static struct gdsc pcie_0_gdsc = {
 	.pd = {
 		.name = "pcie_0_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4226,7 +4226,7 @@ static struct gdsc pcie_1_gdsc = {
 	.pd = {
 		.name = "pcie_1_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4235,7 +4235,7 @@ static struct gdsc pcie_2_gdsc = {
 	.pd = {
 		.name = "pcie_2_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4253,7 +4253,7 @@ static struct gdsc pcie_3_gdsc = {
 	.pd = {
 		.name = "pcie_3_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
-- 
2.52.0

Re: [PATCH v2 04/11] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains

[Manivannan Sadhasivam]

On Thu, Mar 12, 2026 at 08:12:09AM -0300, Val Packett wrote:
> As the PCIe host controller driver does not yet support dealing with the
> loss of state during suspend, use retention for relevant GDSCs.
> 
> This fixes the link not surviving upon resume:
> 
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: Disabling device after reset failure: -19
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Val Packett <val@packett.cool>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
> index b116a9c0b2d9..4095a1f54a09 100644
> --- a/drivers/clk/qcom/gcc-sc8180x.c
> +++ b/drivers/clk/qcom/gcc-sc8180x.c
> @@ -4199,7 +4199,7 @@ static struct gdsc pcie_0_gdsc = {
>  	.pd = {
>  		.name = "pcie_0_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  	.flags = POLL_CFG_GDSCR,
>  };
>  
> @@ -4226,7 +4226,7 @@ static struct gdsc pcie_1_gdsc = {
>  	.pd = {
>  		.name = "pcie_1_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  	.flags = POLL_CFG_GDSCR,
>  };
>  
> @@ -4235,7 +4235,7 @@ static struct gdsc pcie_2_gdsc = {
>  	.pd = {
>  		.name = "pcie_2_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  	.flags = POLL_CFG_GDSCR,
>  };
>  
> @@ -4253,7 +4253,7 @@ static struct gdsc pcie_3_gdsc = {
>  	.pd = {
>  		.name = "pcie_3_gdsc",
>  	},
> -	.pwrsts = PWRSTS_OFF_ON,
> +	.pwrsts = PWRSTS_RET_ON,
>  	.flags = POLL_CFG_GDSCR,
>  };
>  
> -- 
> 2.52.0
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 04/11] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains

[Konrad Dybcio]

On 3/12/26 12:12 PM, Val Packett wrote:
> As the PCIe host controller driver does not yet support dealing with the
> loss of state during suspend, use retention for relevant GDSCs.
> 
> This fixes the link not surviving upon resume:
> 
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: Disabling device after reset failure: -19
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH v2 05/11] clk: qcom: gcc-sc8180x: Enable runtime PM support

[Val Packett]

The GCC block on SC8180X is powered by the CX rail. We need to ensure
that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 4095a1f54a09..2888c4ebd5e8 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4663,6 +4663,7 @@ static const struct qcom_cc_desc gcc_sc8180x_desc = {
 	.num_resets = ARRAY_SIZE(gcc_sc8180x_resets),
 	.gdscs = gcc_sc8180x_gdscs,
 	.num_gdscs = ARRAY_SIZE(gcc_sc8180x_gdscs),
+	.use_rpm = true,
 };
 
 static const struct of_device_id gcc_sc8180x_match_table[] = {
-- 
2.52.0

Re: [PATCH v2 05/11] clk: qcom: gcc-sc8180x: Enable runtime PM support

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 08:12:10AM -0300, Val Packett wrote:
> The GCC block on SC8180X is powered by the CX rail. We need to ensure
> that it's enabled to prevent unwanted power collapse.
> 
> Enable runtime PM to keep the power flowing only when necessary.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH v2 06/11] clk: qcom: gcc-sc8180x: Refactor to use qcom_cc_driver_data

[Val Packett]

Use a qcom_cc_driver_data struct instead of a long custom probe
callback to align with modern qcom/gcc-*.c style.

No functional change intended.

Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 61 +++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 2888c4ebd5e8..88b95d5326d9 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4605,7 +4605,7 @@ static const struct qcom_reset_map gcc_sc8180x_resets[] = {
 	[GCC_VIDEO_AXI1_CLK_BCR] = { .reg = 0xb028, .bit = 2, .udelay = 150 },
 };
 
-static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
+static const struct clk_rcg_dfs_data gcc_sc8180x_dfs_clocks[] = {
 	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk_src),
 	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s1_clk_src),
 	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s2_clk_src),
@@ -4647,6 +4647,19 @@ static struct gdsc *gcc_sc8180x_gdscs[] = {
 	[HLOS1_VOTE_TURING_MMU_TBU1_GDSC] = &hlos1_vote_turing_mmu_tbu1_gdsc,
 };
 
+static u32 gcc_sc8180x_critical_cbcrs[] = {
+	0xb004,  /* GCC_VIDEO_AHB_CLK */
+	0xb008,  /* GCC_CAMERA_AHB_CLK */
+	0xb00c,  /* GCC_DISP_AHB_CLK */
+	0xb040,  /* GCC_VIDEO_XO_CLK */
+	0xb044,  /* GCC_CAMERA_XO_CLK */
+	0xb048,  /* GCC_DISP_XO_CLK */
+	0x48004, /* GCC_CPUSS_GNOC_CLK */
+	0x48190, /* GCC_CPUSS_DVM_BUS_CLK */
+	0x4d004, /* GCC_NPU_CFG_AHB_CLK */
+	0x71004, /* GCC_GPU_CFG_AHB_CLK */
+};
+
 static const struct regmap_config gcc_sc8180x_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
@@ -4655,6 +4668,21 @@ static const struct regmap_config gcc_sc8180x_regmap_config = {
 	.fast_io	= true,
 };
 
+static void clk_sc8180x_regs_configure(struct device *dev, struct regmap *regmap)
+{
+	/* Disable the GPLL0 active input to NPU and GPU via MISC registers */
+	regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
+	regmap_update_bits(regmap, 0x71028, 0x3, 0x3);
+}
+
+static struct qcom_cc_driver_data gcc_sc8180x_driver_data = {
+	.clk_cbcrs = gcc_sc8180x_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(gcc_sc8180x_critical_cbcrs),
+	.dfs_rcgs = gcc_sc8180x_dfs_clocks,
+	.num_dfs_rcgs = ARRAY_SIZE(gcc_sc8180x_dfs_clocks),
+	.clk_regs_configure = clk_sc8180x_regs_configure,
+};
+
 static const struct qcom_cc_desc gcc_sc8180x_desc = {
 	.config = &gcc_sc8180x_regmap_config,
 	.clks = gcc_sc8180x_clocks,
@@ -4664,6 +4692,7 @@ static const struct qcom_cc_desc gcc_sc8180x_desc = {
 	.gdscs = gcc_sc8180x_gdscs,
 	.num_gdscs = ARRAY_SIZE(gcc_sc8180x_gdscs),
 	.use_rpm = true,
+	.driver_data = &gcc_sc8180x_driver_data,
 };
 
 static const struct of_device_id gcc_sc8180x_match_table[] = {
@@ -4674,35 +4703,7 @@ MODULE_DEVICE_TABLE(of, gcc_sc8180x_match_table);
 
 static int gcc_sc8180x_probe(struct platform_device *pdev)
 {
-	struct regmap *regmap;
-	int ret;
-
-	regmap = qcom_cc_map(pdev, &gcc_sc8180x_desc);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
-
-	/* Keep some clocks always-on */
-	qcom_branch_set_clk_en(regmap, 0xb004); /* GCC_VIDEO_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0xb008); /* GCC_CAMERA_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0xb00c); /* GCC_DISP_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0xb040); /* GCC_VIDEO_XO_CLK */
-	qcom_branch_set_clk_en(regmap, 0xb044); /* GCC_CAMERA_XO_CLK */
-	qcom_branch_set_clk_en(regmap, 0xb048); /* GCC_DISP_XO_CLK */
-	qcom_branch_set_clk_en(regmap, 0x48004); /* GCC_CPUSS_GNOC_CLK */
-	qcom_branch_set_clk_en(regmap, 0x48190); /* GCC_CPUSS_DVM_BUS_CLK */
-	qcom_branch_set_clk_en(regmap, 0x4d004); /* GCC_NPU_CFG_AHB_CLK */
-	qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
-
-	/* Disable the GPLL0 active input to NPU and GPU via MISC registers */
-	regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
-	regmap_update_bits(regmap, 0x71028, 0x3, 0x3);
-
-	ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
-					ARRAY_SIZE(gcc_dfs_clocks));
-	if (ret)
-		return ret;
-
-	return qcom_cc_really_probe(&pdev->dev, &gcc_sc8180x_desc, regmap);
+	return qcom_cc_probe(pdev, &gcc_sc8180x_desc);
 }
 
 static struct platform_driver gcc_sc8180x_driver = {
-- 
2.52.0

Re: [PATCH v2 06/11] clk: qcom: gcc-sc8180x: Refactor to use qcom_cc_driver_data

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 08:12:11AM -0300, Val Packett wrote:
> Use a qcom_cc_driver_data struct instead of a long custom probe
> callback to align with modern qcom/gcc-*.c style.
> 
> No functional change intended.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 61 +++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 30 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 06/11] clk: qcom: gcc-sc8180x: Refactor to use qcom_cc_driver_data

[Konrad Dybcio]

On 3/12/26 12:12 PM, Val Packett wrote:
> Use a qcom_cc_driver_data struct instead of a long custom probe
> callback to align with modern qcom/gcc-*.c style.
> 
> No functional change intended.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH v2 07/11] clk: qcom: dispcc-sm8250: Use shared ops on the mdss vsync clk

[Val Packett]

mdss_gdsc can get stuck on boot due to RCGs being left on from last boot.
As a fix, commit 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon
registration") introduced a callback to ensure the RCG is off upon init.
However, the fix depends on all shared RCGs being marked as such in code.

For SM8150/SC8180X/SM8250 the MDSS vsync clock was using regular ops,
unlike the same clock in the SC7180 code. This was causing display to
frequently fail to initialize after rebooting on the Surface Pro X.
Fix by using shared ops for this clock.

Fixes: 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/dispcc-sm8250.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index 8f433e1e7028..cdfdb2cfb02b 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -632,7 +632,7 @@ static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
 		.parent_data = disp_cc_parent_data_1,
 		.num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
 		.flags = CLK_SET_RATE_PARENT,
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_shared_ops,
 	},
 };
 
-- 
2.52.0

Re: [PATCH v2 07/11] clk: qcom: dispcc-sm8250: Use shared ops on the mdss vsync clk

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 08:12:12AM -0300, Val Packett wrote:
> mdss_gdsc can get stuck on boot due to RCGs being left on from last boot.
> As a fix, commit 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon
> registration") introduced a callback to ensure the RCG is off upon init.
> However, the fix depends on all shared RCGs being marked as such in code.
> 
> For SM8150/SC8180X/SM8250 the MDSS vsync clock was using regular ops,
> unlike the same clock in the SC7180 code. This was causing display to
> frequently fail to initialize after rebooting on the Surface Pro X.
> Fix by using shared ops for this clock.
> 
> Fixes: 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/dispcc-sm8250.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH v2 08/11] clk: qcom: dispcc-sm8250: Enable parents for pixel clocks

[Val Packett]

Add CLK_OPS_PARENT_ENABLE to MDSS pixel clock sources to ensure parent
clocks are enabled during clock operations, preventing potential
stability issues during display configuration.

Fixes: 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index cdfdb2cfb02b..e59cdadd5647 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -578,7 +578,7 @@ static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
 		.name = "disp_cc_mdss_pclk0_clk_src",
 		.parent_data = disp_cc_parent_data_6,
 		.num_parents = ARRAY_SIZE(disp_cc_parent_data_6),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 		.ops = &clk_pixel_ops,
 	},
 };
@@ -592,7 +592,7 @@ static struct clk_rcg2 disp_cc_mdss_pclk1_clk_src = {
 		.name = "disp_cc_mdss_pclk1_clk_src",
 		.parent_data = disp_cc_parent_data_6,
 		.num_parents = ARRAY_SIZE(disp_cc_parent_data_6),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 		.ops = &clk_pixel_ops,
 	},
 };
-- 
2.52.0

Re: [PATCH v2 08/11] clk: qcom: dispcc-sm8250: Enable parents for pixel clocks

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 08:12:13AM -0300, Val Packett wrote:
> Add CLK_OPS_PARENT_ENABLE to MDSS pixel clock sources to ensure parent
> clocks are enabled during clock operations, preventing potential
> stability issues during display configuration.
> 
> Fixes: 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 08/11] clk: qcom: dispcc-sm8250: Enable parents for pixel clocks

[Marek Szyprowski]

Hi,

On 12.03.2026 12:12, Val Packett wrote:
> Add CLK_OPS_PARENT_ENABLE to MDSS pixel clock sources to ensure parent
> clocks are enabled during clock operations, preventing potential
> stability issues during display configuration.
>
> Fixes: 80a18f4a8567 ("clk: qcom: Add display clock controller driver for SM8150 and SM8250")
> Signed-off-by: Val Packett <val@packett.cool>

This patch landed in yesterday's linux-next as commit. In my tests I 
found that it triggers the following warning on RB5 board:

------------[ cut here ]------------
dsi0_phy_pll_out_dsiclk already disabled
WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0x1ac/0x1bc, CPU#1: 
kworker/u32:6/120
Modules linked in: ...
CPU: 1 UID: 0 PID: 120 Comm: kworker/u32:6 Not tainted 7.0.0-rc1+ #12425 
PREEMPT
Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_core_disable+0x1ac/0x1bc
lr : clk_core_disable+0x1ac/0x1bc
sp : ffff800081a2b990
...
Call trace:
  clk_core_disable+0x1ac/0x1bc (P)
  __clk_set_parent_after+0x9c/0xf0
  clk_core_set_parent_nolock+0x1f8/0x230
  clk_set_parent+0x40/0x84
  of_clk_set_defaults+0x130/0x4b8
  platform_probe+0x38/0xac
  really_probe+0xbc/0x298
  __driver_probe_device+0x78/0x12c
  driver_probe_device+0x40/0x164
  __device_attach_driver+0xa4/0x148
  bus_for_each_drv+0x80/0xdc
  __device_attach+0xa8/0x1b0
  device_initial_probe+0x50/0x54
  bus_probe_device+0x38/0xa8
  deferred_probe_work_func+0x8c/0xc8
  process_one_work+0x20c/0x78c
  worker_thread+0x244/0x388
  kthread+0x140/0x14c
  ret_from_fork+0x10/0x20
irq event stamp: 336832
hardirqs last  enabled at (336831): [<ffffad391268b854>] 
_raw_spin_unlock_irqrestore+0x74/0x78
hardirqs last disabled at (336832): [<ffffad3911b6d8b4>] 
clk_enable_lock+0xcc/0x110
softirqs last  enabled at (336804): [<ffffad391134b884>] 
handle_softirqs+0x624/0x63c
softirqs last disabled at (336793): [<ffffad3911290674>] 
__do_softirq+0x14/0x20
---[ end trace 0000000000000000 ]---
------------[ cut here ]------------
dsi0_phy_pll_out_dsiclk already unprepared
WARNING: drivers/clk/clk.c:1047 at clk_core_unprepare+0x1f0/0x210, 
CPU#6: kworker/u32:6/120

It looks like a bug in the code performing the default clocks 
configuration though.

> ---
>   drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> index cdfdb2cfb02b..e59cdadd5647 100644
> --- a/drivers/clk/qcom/dispcc-sm8250.c
> +++ b/drivers/clk/qcom/dispcc-sm8250.c
> @@ -578,7 +578,7 @@ static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
>   		.name = "disp_cc_mdss_pclk0_clk_src",
>   		.parent_data = disp_cc_parent_data_6,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_6),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>   		.ops = &clk_pixel_ops,
>   	},
>   };
> @@ -592,7 +592,7 @@ static struct clk_rcg2 disp_cc_mdss_pclk1_clk_src = {
>   		.name = "disp_cc_mdss_pclk1_clk_src",
>   		.parent_data = disp_cc_parent_data_6,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_6),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>   		.ops = &clk_pixel_ops,
>   	},
>   };

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH v2 09/11] clk: qcom: camcc-sc8180x: Remove wait_val for Titan GDSC

[Val Packett]

These values seem to have been copied from newer SoCs, but e.g. SC7180
and SDM845 do not use them. They seem to play a role in causing the
"titan_top_gdsc stuck at off" errors on boot on the Surface Pro X.

Remove to fix.

Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/camcc-sc8180x.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
index 388fedf1dc81..cd4c40a81c28 100644
--- a/drivers/clk/qcom/camcc-sc8180x.c
+++ b/drivers/clk/qcom/camcc-sc8180x.c
@@ -2538,9 +2538,6 @@ static struct clk_branch cam_cc_mclk7_clk = {
 
 static struct gdsc titan_top_gdsc = {
 	.gdscr = 0xc1bc,
-	.en_rest_wait_val = 0x2,
-	.en_few_wait_val = 0x2,
-	.clk_dis_wait_val = 0xf,
 	.pd = {
 		.name = "titan_top_gdsc",
 	},
-- 
2.52.0

Re: [PATCH v2 09/11] clk: qcom: camcc-sc8180x: Remove wait_val for Titan GDSC

[Konrad Dybcio]

On 3/12/26 12:12 PM, Val Packett wrote:
> These values seem to have been copied from newer SoCs, but e.g. SC7180
> and SDM845 do not use them. They seem to play a role in causing the
> "titan_top_gdsc stuck at off" errors on boot on the Surface Pro X.
> 
> Remove to fix.
> 
> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/camcc-sc8180x.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
> index 388fedf1dc81..cd4c40a81c28 100644
> --- a/drivers/clk/qcom/camcc-sc8180x.c
> +++ b/drivers/clk/qcom/camcc-sc8180x.c
> @@ -2538,9 +2538,6 @@ static struct clk_branch cam_cc_mclk7_clk = {
>  
>  static struct gdsc titan_top_gdsc = {
>  	.gdscr = 0xc1bc,
> -	.en_rest_wait_val = 0x2,
> -	.en_few_wait_val = 0x2,
> -	.clk_dis_wait_val = 0xf,

Those are the 'correct' / on-reset values which if absent, are going to
be overriden by the driver, with values destined for MSM8974, so please
drop this one

Konrad

Re: [PATCH v2 09/11] clk: qcom: camcc-sc8180x: Remove wait_val for Titan GDSC

[Dmitry Baryshkov]

On Fri, Mar 13, 2026 at 11:50:03AM +0100, Konrad Dybcio wrote:
> On 3/12/26 12:12 PM, Val Packett wrote:
> > These values seem to have been copied from newer SoCs, but e.g. SC7180
> > and SDM845 do not use them. They seem to play a role in causing the
> > "titan_top_gdsc stuck at off" errors on boot on the Surface Pro X.
> > 
> > Remove to fix.
> > 
> > Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
> > Signed-off-by: Val Packett <val@packett.cool>
> > ---
> >  drivers/clk/qcom/camcc-sc8180x.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
> > index 388fedf1dc81..cd4c40a81c28 100644
> > --- a/drivers/clk/qcom/camcc-sc8180x.c
> > +++ b/drivers/clk/qcom/camcc-sc8180x.c
> > @@ -2538,9 +2538,6 @@ static struct clk_branch cam_cc_mclk7_clk = {
> >  
> >  static struct gdsc titan_top_gdsc = {
> >  	.gdscr = 0xc1bc,
> > -	.en_rest_wait_val = 0x2,
> > -	.en_few_wait_val = 0x2,
> > -	.clk_dis_wait_val = 0xf,
> 
> Those are the 'correct' / on-reset values which if absent, are going to
> be overriden by the driver, with values destined for MSM8974, so please
> drop this one

Probably we should fix the drivers to stop showing in MSM8974 values...

-- 
With best wishes
Dmitry

[PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

[Val Packett]

In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
HW_CTRL flag, but it was missing on SC8180X.

Fix by setting it on all applicable GDSC entries.

Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
index cd4c40a81c28..67b2055bd212 100644
--- a/drivers/clk/qcom/camcc-sc8180x.c
+++ b/drivers/clk/qcom/camcc-sc8180x.c
@@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct gdsc ife_0_gdsc = {
@@ -2620,7 +2620,7 @@ static struct gdsc ipe_0_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct gdsc ipe_1_gdsc = {
@@ -2633,7 +2633,7 @@ static struct gdsc ipe_1_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.parent = &titan_top_gdsc.pd,
-	.flags = POLL_CFG_GDSCR,
+	.flags = HW_CTRL | POLL_CFG_GDSCR,
 };
 
 static struct clk_regmap *cam_cc_sc8180x_clocks[] = {
-- 
2.52.0

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

[Konrad Dybcio]

On 3/12/26 12:12 PM, Val Packett wrote:
> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
> HW_CTRL flag, but it was missing on SC8180X.
> 
> Fix by setting it on all applicable GDSC entries.
> 
> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
> index cd4c40a81c28..67b2055bd212 100644
> --- a/drivers/clk/qcom/camcc-sc8180x.c
> +++ b/drivers/clk/qcom/camcc-sc8180x.c
> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
>  	.parent = &titan_top_gdsc.pd,
> -	.flags = POLL_CFG_GDSCR,
> +	.flags = HW_CTRL | POLL_CFG_GDSCR,

FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
the driver) would be better suited

Konrad

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

[Val Packett]

On 3/13/26 7:53 AM, Konrad Dybcio wrote:
> On 3/12/26 12:12 PM, Val Packett wrote:
>> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
>> HW_CTRL flag, but it was missing on SC8180X.
>>
>> Fix by setting it on all applicable GDSC entries.
>>
>> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>>   drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
>> index cd4c40a81c28..67b2055bd212 100644
>> --- a/drivers/clk/qcom/camcc-sc8180x.c
>> +++ b/drivers/clk/qcom/camcc-sc8180x.c
>> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>>   	},
>>   	.pwrsts = PWRSTS_OFF_ON,
>>   	.parent = &titan_top_gdsc.pd,
>> -	.flags = POLL_CFG_GDSCR,
>> +	.flags = HW_CTRL | POLL_CFG_GDSCR,
> FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
> suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
> the driver) would be better suited

Hmm what exactly would that "patch to the driver" be? Not sure I 
understood that part…

~val

Re: [PATCH v2 10/11] clk: qcom: camcc-sc8180x: Add missing HW_CTRL GDSC flag

[Konrad Dybcio]

On 3/14/26 1:13 AM, Val Packett wrote:
> 
> On 3/13/26 7:53 AM, Konrad Dybcio wrote:
>> On 3/12/26 12:12 PM, Val Packett wrote:
>>> In all other Qualcomm SoC camcc drivers, the BPS and IPE GDSCs use the
>>> HW_CTRL flag, but it was missing on SC8180X.
>>>
>>> Fix by setting it on all applicable GDSC entries.
>>>
>>> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
>>> Signed-off-by: Val Packett <val@packett.cool>
>>> ---
>>>   drivers/clk/qcom/camcc-sc8180x.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
>>> index cd4c40a81c28..67b2055bd212 100644
>>> --- a/drivers/clk/qcom/camcc-sc8180x.c
>>> +++ b/drivers/clk/qcom/camcc-sc8180x.c
>>> @@ -2555,7 +2555,7 @@ static struct gdsc bps_gdsc = {
>>>       },
>>>       .pwrsts = PWRSTS_OFF_ON,
>>>       .parent = &titan_top_gdsc.pd,
>>> -    .flags = POLL_CFG_GDSCR,
>>> +    .flags = HW_CTRL | POLL_CFG_GDSCR,
>> FWIW downstream seems to use HW_MODE at runtime and SW_MODE+collapse on
>> suspend for both BPS and IPE, so perhaps HW_CTRL_TRIGGER (and a patch to
>> the driver) would be better suited
> 
> Hmm what exactly would that "patch to the driver" be? Not sure I understood that part…

There's now a dev_pm_genpd_set_hwmode() that on GDSCs translates to
toggling the HW_CTRL bit in the register at runtime

Konrad

[PATCH v2 11/11] clk: qcom: camcc-sc8180x: Refactor to use qcom_cc_driver_data

[Val Packett]

Use a qcom_cc_driver_data struct instead of a long custom probe
callback to align with modern qcom/gcc-*.c style.

No functional change intended.

Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/camcc-sc8180x.c | 67 +++++++++++++++-----------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
index 67b2055bd212..bbd7add69cb0 100644
--- a/drivers/clk/qcom/camcc-sc8180x.c
+++ b/drivers/clk/qcom/camcc-sc8180x.c
@@ -7,7 +7,6 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,sc8180x-camcc.h>
@@ -63,6 +62,7 @@ static const struct alpha_pll_config cam_cc_pll0_config = {
 
 static struct clk_alpha_pll cam_cc_pll0 = {
 	.offset = 0x0,
+	.config = &cam_cc_pll0_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -138,6 +138,7 @@ static const struct alpha_pll_config cam_cc_pll1_config = {
 
 static struct clk_alpha_pll cam_cc_pll1 = {
 	.offset = 0x1000,
+	.config = &cam_cc_pll1_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -167,6 +168,7 @@ static const struct alpha_pll_config cam_cc_pll2_config = {
 
 static struct clk_alpha_pll cam_cc_pll2 = {
 	.offset = 0x2000,
+	.config = &cam_cc_pll2_config,
 	.vco_table = regera_vco,
 	.num_vco = ARRAY_SIZE(regera_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_REGERA],
@@ -219,6 +221,7 @@ static const struct alpha_pll_config cam_cc_pll3_config = {
 
 static struct clk_alpha_pll cam_cc_pll3 = {
 	.offset = 0x3000,
+	.config = &cam_cc_pll3_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -248,6 +251,7 @@ static const struct alpha_pll_config cam_cc_pll4_config = {
 
 static struct clk_alpha_pll cam_cc_pll4 = {
 	.offset = 0x4000,
+	.config = &cam_cc_pll4_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -277,6 +281,7 @@ static const struct alpha_pll_config cam_cc_pll5_config = {
 
 static struct clk_alpha_pll cam_cc_pll5 = {
 	.offset = 0x4078,
+	.config = &cam_cc_pll5_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -306,6 +311,7 @@ static const struct alpha_pll_config cam_cc_pll6_config = {
 
 static struct clk_alpha_pll cam_cc_pll6 = {
 	.offset = 0x40f0,
+	.config = &cam_cc_pll6_config,
 	.vco_table = trion_vco,
 	.num_vco = ARRAY_SIZE(trion_vco),
 	.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION],
@@ -2810,6 +2816,21 @@ static const struct qcom_reset_map cam_cc_sc8180x_resets[] = {
 	[CAM_CC_MCLK7_BCR] = { 0x50e0 },
 };
 
+static struct clk_alpha_pll *cam_cc_sc8180x_plls[] = {
+	&cam_cc_pll0,
+	&cam_cc_pll1,
+	&cam_cc_pll2,
+	&cam_cc_pll3,
+	&cam_cc_pll4,
+	&cam_cc_pll5,
+	&cam_cc_pll6,
+};
+
+static u32 cam_cc_sc8180x_critical_cbcrs[] = {
+	0xc1e4, /* CAM_CC_GDSC_CLK */
+	0xc200, /* CAM_CC_SLEEP_CLK */
+};
+
 static const struct regmap_config cam_cc_sc8180x_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -2818,6 +2839,13 @@ static const struct regmap_config cam_cc_sc8180x_regmap_config = {
 	.fast_io = true,
 };
 
+static struct qcom_cc_driver_data cam_cc_sc8180x_driver_data = {
+	.alpha_plls = cam_cc_sc8180x_plls,
+	.num_alpha_plls = ARRAY_SIZE(cam_cc_sc8180x_plls),
+	.clk_cbcrs = cam_cc_sc8180x_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(cam_cc_sc8180x_critical_cbcrs),
+};
+
 static const struct qcom_cc_desc cam_cc_sc8180x_desc = {
 	.config = &cam_cc_sc8180x_regmap_config,
 	.clks = cam_cc_sc8180x_clocks,
@@ -2826,6 +2854,8 @@ static const struct qcom_cc_desc cam_cc_sc8180x_desc = {
 	.num_resets = ARRAY_SIZE(cam_cc_sc8180x_resets),
 	.gdscs = cam_cc_sc8180x_gdscs,
 	.num_gdscs = ARRAY_SIZE(cam_cc_sc8180x_gdscs),
+	.use_rpm = true,
+	.driver_data = &cam_cc_sc8180x_driver_data,
 };
 
 static const struct of_device_id cam_cc_sc8180x_match_table[] = {
@@ -2836,40 +2866,7 @@ MODULE_DEVICE_TABLE(of, cam_cc_sc8180x_match_table);
 
 static int cam_cc_sc8180x_probe(struct platform_device *pdev)
 {
-	struct regmap *regmap;
-	int ret;
-
-	ret = devm_pm_runtime_enable(&pdev->dev);
-	if (ret)
-		return ret;
-
-	ret = pm_runtime_resume_and_get(&pdev->dev);
-	if (ret)
-		return ret;
-
-	regmap = qcom_cc_map(pdev, &cam_cc_sc8180x_desc);
-	if (IS_ERR(regmap)) {
-		pm_runtime_put(&pdev->dev);
-		return PTR_ERR(regmap);
-	}
-
-	clk_trion_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
-	clk_trion_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config);
-	clk_regera_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config);
-	clk_trion_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config);
-	clk_trion_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config);
-	clk_trion_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config);
-	clk_trion_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config);
-
-	/* Keep some clocks always enabled */
-	qcom_branch_set_clk_en(regmap, 0xc1e4); /* CAM_CC_GDSC_CLK */
-	qcom_branch_set_clk_en(regmap, 0xc200); /* CAM_CC_SLEEP_CLK */
-
-	ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_sc8180x_desc, regmap);
-
-	pm_runtime_put(&pdev->dev);
-
-	return ret;
+	return qcom_cc_probe(pdev, &cam_cc_sc8180x_desc);
 }
 
 static struct platform_driver cam_cc_sc8180x_driver = {
-- 
2.52.0

Re: [PATCH v2 11/11] clk: qcom: camcc-sc8180x: Refactor to use qcom_cc_driver_data

[Dmitry Baryshkov]

On Thu, Mar 12, 2026 at 08:12:16AM -0300, Val Packett wrote:
> Use a qcom_cc_driver_data struct instead of a long custom probe
> callback to align with modern qcom/gcc-*.c style.
> 
> No functional change intended.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/camcc-sc8180x.c | 67 +++++++++++++++-----------------
>  1 file changed, 32 insertions(+), 35 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v2 11/11] clk: qcom: camcc-sc8180x: Refactor to use qcom_cc_driver_data

[Konrad Dybcio]

On 3/12/26 12:12 PM, Val Packett wrote:
> Use a qcom_cc_driver_data struct instead of a long custom probe
> callback to align with modern qcom/gcc-*.c style.
> 
> No functional change intended.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: (subset) [PATCH v2 00/11] clk: qcom: sc8180x: PM-related fixes (and refactoring)

[Bjorn Andersson]

On Thu, 12 Mar 2026 08:12:05 -0300, Val Packett wrote:
> v2:
> - collect tags
> - bring in dispcc patch mentioned in cover letter
> - add a couple new discoveries (mostly camcc related)
> - add refactoring to current style (qcom_cc_driver_data)
> 
> v1: https://lore.kernel.org/all/20260309010214.224621-1-val@packett.cool/
> 
> [...]

Applied, thanks!

[01/11] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs
        commit: 76404ffbf07f28a5ec04748e18fce3dac2e78ef6
[02/11] clk: qcom: gcc-sc8180x: Add missing GDSCs
        commit: 3565741eb985a8a7cc6656eb33496195468cb99e
[03/11] clk: qcom: gcc-sc8180x: Use retention for USB power domains
        commit: 25bc96f26cd6c19dde13a0b9859183e531d6fbfc
[04/11] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains
        commit: ccb92c78b42edd26225b4d5920847dfee3e1b093
[05/11] clk: qcom: gcc-sc8180x: Enable runtime PM support
        commit: 733220662679da538c5c416b3367acc7cb212f29
[06/11] clk: qcom: gcc-sc8180x: Refactor to use qcom_cc_driver_data
        commit: f641773e10fa5e85154554b15f2aff30e050bdcb
[07/11] clk: qcom: dispcc-sm8250: Use shared ops on the mdss vsync clk
        commit: 8c522da70f0c2e5148c4c13ccb1c64cca57a6fdb
[08/11] clk: qcom: dispcc-sm8250: Enable parents for pixel clocks
        commit: acf7a91d0b0e9e3ef374944021de62062125b7e4
[11/11] clk: qcom: camcc-sc8180x: Refactor to use qcom_cc_driver_data
        commit: b39ae8c2f3de2a2429caad9dd414db14f84bcc8e

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>