[PATCH 00/10] clk: qcom: several fixes for stability on MSM8996/98 platforms

[PATCH 00/10] clk: qcom: several fixes for stability on MSM8996/98 platforms

[Dmitry Baryshkov]

The MSM8996/MSM8998/SDM630/SDM660 platforms face several issues with
stability (e.g. setting running at the halved frequency after certain
frequency switches, resetting if run without clk_ignore_unused, failing
to scale GPU clock, etc.). In preparation for scaling core, cluster, CBF
and GPU voltages, fix these issues affecting platform stability.

Compared to other patches in the series the patches for MSM8996-Pro were
lightly tested, as a part of the larger series and only remotely.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (10):
      clk: qcom: gcc-msm8998: leave the GPU BIMC source and SNOC DVM clocks on
      clk: qcom: gcc-msm8998: leave the MMSS QM clocks on
      clk: qcom: gpucc-msm8998: set the droop detector bit in the right register
      clk: qcom: clk-rcg2: forward the gfx3d rate request to the parent
      clk: qcom: cpu-8996: move off the ACD leg while the PLL speeds up
      clk: qcom: mmcc-msm8996: keep mmss_mmagic_maxi_clk always enabled
      clk: qcom: cbf-msm8996: program the MSM8996 Pro post-divider
      dt-bindings: clock: qcom,msm8996-apcc: add the MSM8996 Pro compatible
      clk: qcom: cpu-8996: program the MSM8996 Pro droop code register
      arm64: dts: qcom: msm8996pro: use the MSM8996 Pro CPU clock compatible

 .../bindings/clock/qcom,msm8996-apcc.yaml          |  7 ++-
 arch/arm64/boot/dts/qcom/msm8996pro.dtsi           |  4 ++
 drivers/clk/qcom/clk-cbf-8996.c                    | 10 ++--
 drivers/clk/qcom/clk-cpu-8996.c                    | 36 ++++++++---
 drivers/clk/qcom/clk-rcg2.c                        | 21 +++----
 drivers/clk/qcom/gcc-msm8998.c                     | 69 ++++------------------
 drivers/clk/qcom/gpucc-msm8998.c                   |  2 +-
 drivers/clk/qcom/mmcc-msm8996.c                    |  2 +-
 8 files changed, 65 insertions(+), 86 deletions(-)
---
base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64
change-id: 20261019-msm8998-fix-clocks-6355058bcab0

Best regards,
--  
With best wishes
Dmitry

[PATCH 01/10] clk: qcom: gcc-msm8998: leave the GPU BIMC source and SNOC DVM clocks on

[Dmitry Baryshkov]

Booting MSM8998 without clk_ignore_unused resets the SoC shortly after
the unused clocks are turned off. Nothing references
gcc_gpu_bimc_gfx_src_clk, so it gets gated, and the SoC goes down
within a fraction of a second, with the GPU idle and with or without
Venus. Listing it as a GPU SMMU clock does not help: the SoC then
resets after the SMMU first runtime-suspends.

Downstream registered this clock and gcc_gpu_snoc_dvm_gfx_clk at
first, then dropped both because they need to be left at their default
ON state ("clk: msm: clock: Remove controlling some graphics clocks in
Linux"). SDM660 got the same treatment, and the upstream SDM660 driver
never registered them.

Do the same: stop registering both clocks and enable them at probe.

Fixes: b5f5f525c547 ("clk: qcom: Add MSM8998 Global Clock Control (GCC) driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/gcc-msm8998.c | 39 +++++++++++----------------------------
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index c9701f7f6e18..97d4968c5bdd 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2135,19 +2135,6 @@ static struct clk_branch gcc_gpu_bimc_gfx_clk = {
 	},
 };
 
-static struct clk_branch gcc_gpu_bimc_gfx_src_clk = {
-	.halt_reg = 0x7100c,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x7100c,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_gpu_bimc_gfx_src_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_gpu_cfg_ahb_clk = {
 	.halt_reg = 0x71004,
 	.halt_check = BRANCH_HALT_SKIP,
@@ -2167,19 +2154,6 @@ static struct clk_branch gcc_gpu_cfg_ahb_clk = {
 	},
 };
 
-static struct clk_branch gcc_gpu_snoc_dvm_gfx_clk = {
-	.halt_reg = 0x71018,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x71018,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_gpu_snoc_dvm_gfx_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_hmss_ahb_clk = {
 	.halt_reg = 0x48000,
 	.halt_check = BRANCH_HALT_VOTED,
@@ -3088,9 +3062,7 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
 	[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
 	[GCC_BIMC_GFX_CLK] = &gcc_bimc_gfx_clk.clkr,
 	[GCC_GPU_BIMC_GFX_CLK] = &gcc_gpu_bimc_gfx_clk.clkr,
-	[GCC_GPU_BIMC_GFX_SRC_CLK] = &gcc_gpu_bimc_gfx_src_clk.clkr,
 	[GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
-	[GCC_GPU_SNOC_DVM_GFX_CLK] = &gcc_gpu_snoc_dvm_gfx_clk.clkr,
 	[GCC_HMSS_AHB_CLK] = &gcc_hmss_ahb_clk.clkr,
 	[GCC_HMSS_AT_CLK] = &gcc_hmss_at_clk.clkr,
 	[GCC_HMSS_RBCPR_CLK] = &gcc_hmss_rbcpr_clk.clkr,
@@ -3323,6 +3295,16 @@ static const struct regmap_config gcc_msm8998_regmap_config = {
 	.fast_io	= true,
 };
 
+static const u32 gcc_msm8998_critical_cbcrs[] = {
+	0x7100c, /* GCC_GPU_BIMC_GFX_SRC_CLK */
+	0x71018, /* GCC_GPU_SNOC_DVM_GFX_CLK */
+};
+
+static const struct qcom_cc_driver_data gcc_msm8998_driver_data = {
+	.clk_cbcrs = gcc_msm8998_critical_cbcrs,
+	.num_clk_cbcrs = ARRAY_SIZE(gcc_msm8998_critical_cbcrs),
+};
+
 static const struct qcom_cc_desc gcc_msm8998_desc = {
 	.config = &gcc_msm8998_regmap_config,
 	.clks = gcc_msm8998_clocks,
@@ -3331,6 +3313,7 @@ static const struct qcom_cc_desc gcc_msm8998_desc = {
 	.num_resets = ARRAY_SIZE(gcc_msm8998_resets),
 	.gdscs = gcc_msm8998_gdscs,
 	.num_gdscs = ARRAY_SIZE(gcc_msm8998_gdscs),
+	.driver_data = &gcc_msm8998_driver_data,
 };
 
 static int gcc_msm8998_probe(struct platform_device *pdev)

-- 
2.47.3

Re: [PATCH 01/10] clk: qcom: gcc-msm8998: leave the GPU BIMC source and SNOC DVM clocks on

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> Booting MSM8998 without clk_ignore_unused resets the SoC shortly after
> the unused clocks are turned off. Nothing references
> gcc_gpu_bimc_gfx_src_clk, so it gets gated, and the SoC goes down
> within a fraction of a second, with the GPU idle and with or without
> Venus. Listing it as a GPU SMMU clock does not help: the SoC then
> resets after the SMMU first runtime-suspends.
> 
> Downstream registered this clock and gcc_gpu_snoc_dvm_gfx_clk at
> first, then dropped both because they need to be left at their default
> ON state ("clk: msm: clock: Remove controlling some graphics clocks in
> Linux"). SDM660 got the same treatment, and the upstream SDM660 driver
> never registered them.
> 
> Do the same: stop registering both clocks and enable them at probe.

FWIW downstream just leaves them at reset state so it's not exactly
the same but writing shouldn't hurt either

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

Konrad

[PATCH 02/10] clk: qcom: gcc-msm8998: leave the MMSS QM clocks on

[Dmitry Baryshkov]

Booting MSM8998 without clk_ignore_unused, the SoC resets while the
modem is being started: restarting it in a loop takes the board down
within a few dozen cycles. Nothing references gcc_mmss_qm_core_clk, so
it gets gated, and keeping just that clock on is enough to survive.

Downstream never registered gcc_mmss_qm_core_clk, and dropped
gcc_mmss_qm_ahb_clk because XBL controls it ("clk: msm: clock: Remove
support for the gcc_mmss_qm_ahb_clk clock"). Treat both like the GPU
BIMC source and SNOC DVM clocks: stop registering them and enable them
at probe.

Fixes: b5f5f525c547 ("clk: qcom: Add MSM8998 Global Clock Control (GCC) driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/gcc-msm8998.c | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8998.c b/drivers/clk/qcom/gcc-msm8998.c
index 97d4968c5bdd..f07868b35da9 100644
--- a/drivers/clk/qcom/gcc-msm8998.c
+++ b/drivers/clk/qcom/gcc-msm8998.c
@@ -2254,32 +2254,6 @@ static struct clk_branch gcc_mmss_noc_cfg_ahb_clk = {
 	},
 };
 
-static struct clk_branch gcc_mmss_qm_ahb_clk = {
-	.halt_reg = 0x9030,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x9030,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_mmss_qm_ahb_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
-static struct clk_branch gcc_mmss_qm_core_clk = {
-	.halt_reg = 0x900c,
-	.halt_check = BRANCH_HALT,
-	.clkr = {
-		.enable_reg = 0x900c,
-		.enable_mask = BIT(0),
-		.hw.init = &(struct clk_init_data){
-			.name = "gcc_mmss_qm_core_clk",
-			.ops = &clk_branch2_ops,
-		},
-	},
-};
-
 static struct clk_branch gcc_mmss_sys_noc_axi_clk = {
 	.halt_reg = 0x9000,
 	.halt_check = BRANCH_HALT,
@@ -3068,8 +3042,6 @@ static struct clk_regmap *gcc_msm8998_clocks[] = {
 	[GCC_HMSS_RBCPR_CLK] = &gcc_hmss_rbcpr_clk.clkr,
 	[GCC_HMSS_TRIG_CLK] = &gcc_hmss_trig_clk.clkr,
 	[GCC_MMSS_NOC_CFG_AHB_CLK] = &gcc_mmss_noc_cfg_ahb_clk.clkr,
-	[GCC_MMSS_QM_AHB_CLK] = &gcc_mmss_qm_ahb_clk.clkr,
-	[GCC_MMSS_QM_CORE_CLK] = &gcc_mmss_qm_core_clk.clkr,
 	[GCC_MMSS_SYS_NOC_AXI_CLK] = &gcc_mmss_sys_noc_axi_clk.clkr,
 	[GCC_MSS_AT_CLK] = &gcc_mss_at_clk.clkr,
 	[GCC_PCIE_0_AUX_CLK] = &gcc_pcie_0_aux_clk.clkr,
@@ -3298,6 +3270,8 @@ static const struct regmap_config gcc_msm8998_regmap_config = {
 static const u32 gcc_msm8998_critical_cbcrs[] = {
 	0x7100c, /* GCC_GPU_BIMC_GFX_SRC_CLK */
 	0x71018, /* GCC_GPU_SNOC_DVM_GFX_CLK */
+	0x9030, /* GCC_MMSS_QM_AHB_CLK */
+	0x900c, /* GCC_MMSS_QM_CORE_CLK */
 };
 
 static const struct qcom_cc_driver_data gcc_msm8998_driver_data = {

-- 
2.47.3

Re: [PATCH 02/10] clk: qcom: gcc-msm8998: leave the MMSS QM clocks on

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> Booting MSM8998 without clk_ignore_unused, the SoC resets while the
> modem is being started: restarting it in a loop takes the board down
> within a few dozen cycles. Nothing references gcc_mmss_qm_core_clk, so
> it gets gated, and keeping just that clock on is enough to survive.
> 
> Downstream never registered gcc_mmss_qm_core_clk, and dropped
> gcc_mmss_qm_ahb_clk because XBL controls it ("clk: msm: clock: Remove
> support for the gcc_mmss_qm_ahb_clk clock"). Treat both like the GPU
> BIMC source and SNOC DVM clocks: stop registering them and enable them
> at probe.

[...]

>  static const u32 gcc_msm8998_critical_cbcrs[] = {
>  	0x7100c, /* GCC_GPU_BIMC_GFX_SRC_CLK */
>  	0x71018, /* GCC_GPU_SNOC_DVM_GFX_CLK */
> +	0x9030, /* GCC_MMSS_QM_AHB_CLK */
> +	0x900c, /* GCC_MMSS_QM_CORE_CLK */
>  };

ideally this list could be sorted

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

Konrad

[PATCH 03/10] clk: qcom: gpucc-msm8998: set the droop detector bit in the right register

[Dmitry Baryshkov]

The probe means to power down the GPU droop detector to cut the leakage
between the graphics and CX rails, but it sets bit 0 of the GFX3D branch
register instead of GPUCC_GPU_DD_WRAP_CTRL. That force-enables the GFX3D
clock and leaves the droop detector alone.

Write the bit to GPUCC_GPU_DD_WRAP_CTRL, as the vendor driver does.

Fixes: 3f7df5baa259 ("clk: qcom: Add MSM8998 GPU Clock Controller (GPUCC) driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/gpucc-msm8998.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
index 066793e47f79..8da10027f1f2 100644
--- a/drivers/clk/qcom/gpucc-msm8998.c
+++ b/drivers/clk/qcom/gpucc-msm8998.c
@@ -330,7 +330,7 @@ static int gpucc_msm8998_probe(struct platform_device *pdev)
 	/* force periph logic on to avoid perf counter corruption */
 	regmap_write_bits(regmap, gfx3d_clk.clkr.enable_reg, BIT(13), BIT(13));
 	/* tweak droop detector (GPUCC_GPU_DD_WRAP_CTRL) to reduce leakage */
-	regmap_write_bits(regmap, gfx3d_clk.clkr.enable_reg, BIT(0), BIT(0));
+	regmap_write_bits(regmap, 0x430, BIT(0), BIT(0));
 
 	return qcom_cc_really_probe(&pdev->dev, &gpucc_msm8998_desc, regmap);
 }

-- 
2.47.3

Re: [PATCH 03/10] clk: qcom: gpucc-msm8998: set the droop detector bit in the right register

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> The probe means to power down the GPU droop detector to cut the leakage
> between the graphics and CX rails, but it sets bit 0 of the GFX3D branch
> register instead of GPUCC_GPU_DD_WRAP_CTRL. That force-enables the GFX3D
> clock and leaves the droop detector alone.
> 
> Write the bit to GPUCC_GPU_DD_WRAP_CTRL, as the vendor driver does.
> 
> Fixes: 3f7df5baa259 ("clk: qcom: Add MSM8998 GPU Clock Controller (GPUCC) driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/gpucc-msm8998.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
> index 066793e47f79..8da10027f1f2 100644
> --- a/drivers/clk/qcom/gpucc-msm8998.c
> +++ b/drivers/clk/qcom/gpucc-msm8998.c
> @@ -330,7 +330,7 @@ static int gpucc_msm8998_probe(struct platform_device *pdev)
>  	/* force periph logic on to avoid perf counter corruption */
>  	regmap_write_bits(regmap, gfx3d_clk.clkr.enable_reg, BIT(13), BIT(13));
>  	/* tweak droop detector (GPUCC_GPU_DD_WRAP_CTRL) to reduce leakage */
> -	regmap_write_bits(regmap, gfx3d_clk.clkr.enable_reg, BIT(0), BIT(0));
> +	regmap_write_bits(regmap, 0x430, BIT(0), BIT(0));

regmap_set_bits(), would also be nice if this was moved to the new
driver_data. with or without that:

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

Konrad

[PATCH 04/10] clk: qcom: clk-rcg2: forward the gfx3d rate request to the parent

[Dmitry Baryshkov]

GPU DVFS on SDM660 never changed the GFX3D rate. clk_gfx3d_determine_rate()
builds the parent request by hand and leaves best_parent_rate at zero, so
the PLL is asked to determine a rate for a zero parent rate and the new
rate is dropped on the floor.

Use clk_hw_forward_rate_request(), which fills in the parent's limits and
rate the way the core expects them.

Fixes: af1e62f2ffe2 ("clk: qcom: clk-rcg2: Take clock boundaries into consideration for gfx3d")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-rcg2.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index c44c51d860e1..9e3d7e5c81e5 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -1207,10 +1207,10 @@ EXPORT_SYMBOL_GPL(clk_pixel_ops);
 static int clk_gfx3d_determine_rate(struct clk_hw *hw,
 				    struct clk_rate_request *req)
 {
-	struct clk_rate_request parent_req = { .min_rate = 0, .max_rate = ULONG_MAX };
+	struct clk_rate_request parent_req;
 	struct clk_rcg2_gfx3d *cgfx = to_clk_rcg2_gfx3d(hw);
 	struct clk_hw *xo, *p0, *p1, *p2;
-	unsigned long p0_rate;
+	unsigned long p0_rate, parent_rate;
 	u8 mux_div = cgfx->div;
 	int ret;
 
@@ -1234,12 +1234,12 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
 	if (mux_div == 0)
 		mux_div = 1;
 
-	parent_req.rate = req->rate * mux_div;
+	parent_rate = req->rate * mux_div;
 
 	/* This has to be a fixed rate PLL */
 	p0_rate = clk_hw_get_rate(p0);
 
-	if (parent_req.rate == p0_rate) {
+	if (parent_rate == p0_rate) {
 		req->rate = req->best_parent_rate = p0_rate;
 		req->best_parent_hw = p0;
 		return 0;
@@ -1247,7 +1247,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
 
 	if (req->best_parent_hw == p0) {
 		/* Are we going back to a previously used rate? */
-		if (clk_hw_get_rate(p2) == parent_req.rate)
+		if (clk_hw_get_rate(p2) == parent_rate)
 			req->best_parent_hw = p2;
 		else
 			req->best_parent_hw = p1;
@@ -1257,16 +1257,9 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
 		req->best_parent_hw = p2;
 	}
 
-	clk_hw_get_rate_range(req->best_parent_hw,
-			      &parent_req.min_rate, &parent_req.max_rate);
+	clk_hw_forward_rate_request(hw, req, req->best_parent_hw, &parent_req,
+				    parent_rate);
 
-	if (req->min_rate > parent_req.min_rate)
-		parent_req.min_rate = req->min_rate;
-
-	if (req->max_rate < parent_req.max_rate)
-		parent_req.max_rate = req->max_rate;
-
-	parent_req.best_parent_hw = req->best_parent_hw;
 	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
 	if (ret)
 		return ret;

-- 
2.47.3

Re: [PATCH 04/10] clk: qcom: clk-rcg2: forward the gfx3d rate request to the parent

[johannes.goede]

Hi,

On 19-Oct-26 3:41 PM, Dmitry Baryshkov wrote:
> GPU DVFS on SDM660 never changed the GFX3D rate. clk_gfx3d_determine_rate()
> builds the parent request by hand and leaves best_parent_rate at zero, so
> the PLL is asked to determine a rate for a zero parent rate and the new
> rate is dropped on the floor.
> 
> Use clk_hw_forward_rate_request(), which fills in the parent's limits and
> rate the way the core expects them.
> 
> Fixes: af1e62f2ffe2 ("clk: qcom: clk-rcg2: Take clock boundaries into consideration for gfx3d")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/clk/qcom/clk-rcg2.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index c44c51d860e1..9e3d7e5c81e5 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -1207,10 +1207,10 @@ EXPORT_SYMBOL_GPL(clk_pixel_ops);
>  static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  				    struct clk_rate_request *req)
>  {
> -	struct clk_rate_request parent_req = { .min_rate = 0, .max_rate = ULONG_MAX };
> +	struct clk_rate_request parent_req;

Just a random drive-by comment: You are now leaving parent_req entirely uninitialized
filling it with whatever is on the stack. Where as before all values except for
max_rate would be filled with 0.

Are you sure this is what you want ? Maybe:

	struct clk_rate_request parent_req = {};

?

Regards,

Hans




>  	struct clk_rcg2_gfx3d *cgfx = to_clk_rcg2_gfx3d(hw);
>  	struct clk_hw *xo, *p0, *p1, *p2;
> -	unsigned long p0_rate;
> +	unsigned long p0_rate, parent_rate;
>  	u8 mux_div = cgfx->div;
>  	int ret;
>  
> @@ -1234,12 +1234,12 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  	if (mux_div == 0)
>  		mux_div = 1;
>  
> -	parent_req.rate = req->rate * mux_div;
> +	parent_rate = req->rate * mux_div;
>  
>  	/* This has to be a fixed rate PLL */
>  	p0_rate = clk_hw_get_rate(p0);
>  
> -	if (parent_req.rate == p0_rate) {
> +	if (parent_rate == p0_rate) {
>  		req->rate = req->best_parent_rate = p0_rate;
>  		req->best_parent_hw = p0;
>  		return 0;
> @@ -1247,7 +1247,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  
>  	if (req->best_parent_hw == p0) {
>  		/* Are we going back to a previously used rate? */
> -		if (clk_hw_get_rate(p2) == parent_req.rate)
> +		if (clk_hw_get_rate(p2) == parent_rate)
>  			req->best_parent_hw = p2;
>  		else
>  			req->best_parent_hw = p1;
> @@ -1257,16 +1257,9 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>  		req->best_parent_hw = p2;
>  	}
>  
> -	clk_hw_get_rate_range(req->best_parent_hw,
> -			      &parent_req.min_rate, &parent_req.max_rate);
> +	clk_hw_forward_rate_request(hw, req, req->best_parent_hw, &parent_req,
> +				    parent_rate);
>  
> -	if (req->min_rate > parent_req.min_rate)
> -		parent_req.min_rate = req->min_rate;
> -
> -	if (req->max_rate < parent_req.max_rate)
> -		parent_req.max_rate = req->max_rate;
> -
> -	parent_req.best_parent_hw = req->best_parent_hw;
>  	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
>  	if (ret)
>  		return ret;
> 

Re: [PATCH 04/10] clk: qcom: clk-rcg2: forward the gfx3d rate request to the parent

[Hans de Goede]

Hi Again :)

On 19-Oct-26 4:33 PM, johannes.goede@oss.qualcomm.com wrote:
> Hi,
> 
> On 19-Oct-26 3:41 PM, Dmitry Baryshkov wrote:
>> GPU DVFS on SDM660 never changed the GFX3D rate. clk_gfx3d_determine_rate()
>> builds the parent request by hand and leaves best_parent_rate at zero, so
>> the PLL is asked to determine a rate for a zero parent rate and the new
>> rate is dropped on the floor.
>>
>> Use clk_hw_forward_rate_request(), which fills in the parent's limits and
>> rate the way the core expects them.
>>
>> Fixes: af1e62f2ffe2 ("clk: qcom: clk-rcg2: Take clock boundaries into consideration for gfx3d")
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>>  drivers/clk/qcom/clk-rcg2.c | 21 +++++++--------------
>>  1 file changed, 7 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>> index c44c51d860e1..9e3d7e5c81e5 100644
>> --- a/drivers/clk/qcom/clk-rcg2.c
>> +++ b/drivers/clk/qcom/clk-rcg2.c
>> @@ -1207,10 +1207,10 @@ EXPORT_SYMBOL_GPL(clk_pixel_ops);
>>  static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>>  				    struct clk_rate_request *req)
>>  {
>> -	struct clk_rate_request parent_req = { .min_rate = 0, .max_rate = ULONG_MAX };
>> +	struct clk_rate_request parent_req;
> 
> Just a random drive-by comment: You are now leaving parent_req entirely uninitialized
> filling it with whatever is on the stack. Where as before all values except for
> max_rate would be filled with 0.
> 
> Are you sure this is what you want ? Maybe:
> 
> 	struct clk_rate_request parent_req = {};
> 
> ?

Hmm, I guess that clk_hw_forward_rate_request() now fully fills in the passed
in parent_req, so this change is ok.

Regards,
 
Hans




> 
> 
> 
> 
>>  	struct clk_rcg2_gfx3d *cgfx = to_clk_rcg2_gfx3d(hw);
>>  	struct clk_hw *xo, *p0, *p1, *p2;
>> -	unsigned long p0_rate;
>> +	unsigned long p0_rate, parent_rate;
>>  	u8 mux_div = cgfx->div;
>>  	int ret;
>>  
>> @@ -1234,12 +1234,12 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>>  	if (mux_div == 0)
>>  		mux_div = 1;
>>  
>> -	parent_req.rate = req->rate * mux_div;
>> +	parent_rate = req->rate * mux_div;
>>  
>>  	/* This has to be a fixed rate PLL */
>>  	p0_rate = clk_hw_get_rate(p0);
>>  
>> -	if (parent_req.rate == p0_rate) {
>> +	if (parent_rate == p0_rate) {
>>  		req->rate = req->best_parent_rate = p0_rate;
>>  		req->best_parent_hw = p0;
>>  		return 0;
>> @@ -1247,7 +1247,7 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>>  
>>  	if (req->best_parent_hw == p0) {
>>  		/* Are we going back to a previously used rate? */
>> -		if (clk_hw_get_rate(p2) == parent_req.rate)
>> +		if (clk_hw_get_rate(p2) == parent_rate)
>>  			req->best_parent_hw = p2;
>>  		else
>>  			req->best_parent_hw = p1;
>> @@ -1257,16 +1257,9 @@ static int clk_gfx3d_determine_rate(struct clk_hw *hw,
>>  		req->best_parent_hw = p2;
>>  	}
>>  
>> -	clk_hw_get_rate_range(req->best_parent_hw,
>> -			      &parent_req.min_rate, &parent_req.max_rate);
>> +	clk_hw_forward_rate_request(hw, req, req->best_parent_hw, &parent_req,
>> +				    parent_rate);
>>  
>> -	if (req->min_rate > parent_req.min_rate)
>> -		parent_req.min_rate = req->min_rate;
>> -
>> -	if (req->max_rate < parent_req.max_rate)
>> -		parent_req.max_rate = req->max_rate;
>> -
>> -	parent_req.best_parent_hw = req->best_parent_hw;
>>  	ret = __clk_determine_rate(req->best_parent_hw, &parent_req);
>>  	if (ret)
>>  		return ret;
>>
> 

Re: [PATCH 04/10] clk: qcom: clk-rcg2: forward the gfx3d rate request to the parent

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> GPU DVFS on SDM660 never changed the GFX3D rate. clk_gfx3d_determine_rate()
> builds the parent request by hand and leaves best_parent_rate at zero, so
> the PLL is asked to determine a rate for a zero parent rate and the new
> rate is dropped on the floor.
> 
> Use clk_hw_forward_rate_request(), which fills in the parent's limits and
> rate the way the core expects them.
> 
> Fixes: af1e62f2ffe2 ("clk: qcom: clk-rcg2: Take clock boundaries into consideration for gfx3d")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

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

Konrad

[PATCH 05/10] clk: qcom: cpu-8996: move off the ACD leg while the PLL speeds up

[Dmitry Baryshkov]

The MSM8996 CPU clusters sometimes run at half the rate they report
after going up between two frequencies above 600 MHz, and stay there
until the cluster drops below 600 MHz. The PLL, the muxes and the rest
of the cluster clock registers are all as expected: it is the ACD that
latches at half the rate when the PLL is reprogrammed to a higher rate
underneath it, and selecting the ACD leg again clears it. The driver
only moves off the ACD leg when going below 600 MHz.

Like the vendor kernel, run from PLL/2 while the PLL goes up and return
to the ACD leg afterwards. Point the SMUX at PLL/2 at probe time instead
of XO, so that the cluster does not drop to 19.2 MHz on the first such
switch.

Fixes: 03e342dc45c9 ("clk: qcom: Add CPU clock driver for msm8996")
Reported-by: Christopher Obbard <christopher.obbard@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index 21d13c0841ed..f22dd80919fa 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -477,9 +477,9 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
 	regmap_write(regmap, PWRCL_REG_OFFSET + PSCTL_OFFSET, 0x00030005);
 	regmap_write(regmap, PERFCL_REG_OFFSET + PSCTL_OFFSET, 0x00030005);
 
-	/* Switch clusters to use the ACD leg */
-	regmap_write(regmap, PWRCL_REG_OFFSET + MUX_OFFSET, 0x32);
-	regmap_write(regmap, PERFCL_REG_OFFSET + MUX_OFFSET, 0x32);
+	/* Switch clusters to use the ACD leg, SMUX to PLL/2 for rate changes */
+	regmap_write(regmap, PWRCL_REG_OFFSET + MUX_OFFSET, 0x36);
+	regmap_write(regmap, PERFCL_REG_OFFSET + MUX_OFFSET, 0x36);
 
 	for (i = 0; i < ARRAY_SIZE(cpu_msm8996_hw_clks); i++) {
 		ret = devm_clk_hw_register(dev, cpu_msm8996_hw_clks[i]);
@@ -542,6 +542,12 @@ static void qcom_cpu_clk_msm8996_acd_init(struct regmap *regmap)
 	spin_unlock_irqrestore(&qcom_clk_acd_lock, flags);
 }
 
+static bool cpu_clk_8996_leave_acd(struct clk_notifier_data *cnd)
+{
+	return cnd->old_rate >= DIV_2_THRESHOLD &&
+	       (cnd->new_rate < DIV_2_THRESHOLD || cnd->new_rate > cnd->old_rate);
+}
+
 static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 			       void *data)
 {
@@ -558,17 +564,22 @@ static int cpu_clk_notifier_cb(struct notifier_block *nb, unsigned long event,
 		 * chaging the parent of PMUX. This can result in pmux getting
 		 * clocked twice the expected rate.
 		 *
+		 * The ACD may also latch at half the rate when the PLL goes up
+		 * underneath it, until the ACD leg is selected again.
+		 *
 		 * Manually switch to PLL/2 here.
 		 */
-		if (cnd->new_rate < DIV_2_THRESHOLD &&
-		    cnd->old_rate > DIV_2_THRESHOLD)
+		if (cpu_clk_8996_leave_acd(cnd))
 			clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, SMUX_INDEX);
 
 		break;
+	case POST_RATE_CHANGE:
+		if (cnd->new_rate >= DIV_2_THRESHOLD && cpu_clk_8996_leave_acd(cnd))
+			clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ACD_INDEX);
+		break;
 	case ABORT_RATE_CHANGE:
 		/* Revert manual change */
-		if (cnd->new_rate < DIV_2_THRESHOLD &&
-		    cnd->old_rate > DIV_2_THRESHOLD)
+		if (cpu_clk_8996_leave_acd(cnd))
 			clk_cpu_8996_pmux_set_parent(&cpuclk->clkr.hw, ACD_INDEX);
 		break;
 	default:

-- 
2.47.3

Re: [PATCH 05/10] clk: qcom: cpu-8996: move off the ACD leg while the PLL speeds up

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> The MSM8996 CPU clusters sometimes run at half the rate they report
> after going up between two frequencies above 600 MHz, and stay there
> until the cluster drops below 600 MHz. The PLL, the muxes and the rest
> of the cluster clock registers are all as expected: it is the ACD that
> latches at half the rate when the PLL is reprogrammed to a higher rate
> underneath it, and selecting the ACD leg again clears it. The driver
> only moves off the ACD leg when going below 600 MHz.
> 
> Like the vendor kernel, run from PLL/2 while the PLL goes up and return
> to the ACD leg afterwards. Point the SMUX at PLL/2 at probe time instead
> of XO, so that the cluster does not drop to 19.2 MHz on the first such
> switch.
> 
> Fixes: 03e342dc45c9 ("clk: qcom: Add CPU clock driver for msm8996")
> Reported-by: Christopher Obbard <christopher.obbard@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

LLM has the following comments:

For old rates above 1190.4 MHz the SMUX output is old/2, which
exceeds MAX_PLL_MAIN_FREQ (595.2 MHz). Downstream does not do this.
clock-cpu-8996.c acd_safe_freq() returns freq/2 only up to
1190.4 MHz and MAX_PLL_MAIN_FREQ above that, and pre_set_rate()
retunes the PLL to that value before the increase. The comment there
says the main output is what keeps voltage constraints satisfied,
which implies a 595.2 MHz limit on that output.

Up to 1190.4 MHz the two approaches are equivalent: both move the
cluster off the ACD leg and run it from the non-ACD path at half the
old rate. Above that they diverge.

Either clamp the SMUX choice to match acd_safe_freq(), or explain why
old/2 is safe above 1190.4 MHz. The second needs the hardware limit
for the non-ACD output, which is not in the patch or the cover letter.

Konrad

Re: [PATCH 05/10] clk: qcom: cpu-8996: move off the ACD leg while the PLL speeds up

[Dmitry Baryshkov]

On Tue, Oct 20, 2026 at 08:52:49AM +0200, Konrad Dybcio wrote:
> On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> > The MSM8996 CPU clusters sometimes run at half the rate they report
> > after going up between two frequencies above 600 MHz, and stay there
> > until the cluster drops below 600 MHz. The PLL, the muxes and the rest
> > of the cluster clock registers are all as expected: it is the ACD that
> > latches at half the rate when the PLL is reprogrammed to a higher rate
> > underneath it, and selecting the ACD leg again clears it. The driver
> > only moves off the ACD leg when going below 600 MHz.
> > 
> > Like the vendor kernel, run from PLL/2 while the PLL goes up and return
> > to the ACD leg afterwards. Point the SMUX at PLL/2 at probe time instead
> > of XO, so that the cluster does not drop to 19.2 MHz on the first such
> > switch.
> > 
> > Fixes: 03e342dc45c9 ("clk: qcom: Add CPU clock driver for msm8996")
> > Reported-by: Christopher Obbard <christopher.obbard@linaro.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> 
> LLM has the following comments:
> 
> For old rates above 1190.4 MHz the SMUX output is old/2, which
> exceeds MAX_PLL_MAIN_FREQ (595.2 MHz). Downstream does not do this.
> clock-cpu-8996.c acd_safe_freq() returns freq/2 only up to
> 1190.4 MHz and MAX_PLL_MAIN_FREQ above that, and pre_set_rate()
> retunes the PLL to that value before the increase. The comment there
> says the main output is what keeps voltage constraints satisfied,
> which implies a 595.2 MHz limit on that output.

The 595.2 MHz isn't a hardware limit. It's the rate at which it is
possible to start using non-divided PLL output.

> 
> Up to 1190.4 MHz the two approaches are equivalent: both move the
> cluster off the ACD leg and run it from the non-ACD path at half the
> old rate. Above that they diverge.
> 
> Either clamp the SMUX choice to match acd_safe_freq(), or explain why
> old/2 is safe above 1190.4 MHz. The second needs the hardware limit
> for the non-ACD output, which is not in the patch or the cover letter.

Downstream isn't keeping the main output below 1190.4 either. After
pre_set_rate(), cpu_clk_8996_set_rate() asks the HF mux for the target
rate, which makes mux_set_rate() set the PLL rate and then switch the mux.
So the main output follows the PLL up to new/2, e.g. 1075.2 MHz on the
way to 2150.4 MHz, on every increase above 1190.4 MHz.

The main difference is where the cluster waits before the PLL moves: downstream
first lowers the PLL to 1190.4 MHz and parks the output at 595.2 MHz, while
this patch parks it at old/2. Both are lower than the target rate. Following
the downstream approach would mean reprogramming the PLL from inside the pmux
notifier while CCF is changing the rate of the same tree, which feels incorrect
to me. Also it wouldn't lower the peak.

-- 
With best wishes
Dmitry

[PATCH 06/10] clk: qcom: mmcc-msm8996: keep mmss_mmagic_maxi_clk always enabled

[Dmitry Baryshkov]

Enabling vmem_maxi_clk fails with -EBUSY, the branch never leaving halt
("vmem_maxi_clk status stuck at 'off'"), which makes the VMEM block
unusable. The MAXI branches sit behind the MMAGIC aggregator, so they
cannot leave halt unless mmss_mmagic_maxi_clk is running.

Commit 7705bb7176b9 ("clk: qcom: mmcc-msm8996: leave all mmagic gdscs
and clocks always enabled") marked the MMAGIC clocks CLK_IS_CRITICAL
because there is no bus driver to manage them, but missed this one: its
siblings mmss_mmagic_ahb_clk, mmagic_video_axi_clk and
mmagic_video_noc_cfg_ahb_clk all carry the flag while
mmss_mmagic_maxi_clk does not.

Mark it CLK_IS_CRITICAL as well.

Note that the halt bit is truthful here and must not be papered over
with BRANCH_HALT_SKIP: touching the block while its AXI clock is off
silently resets the SoC.

Fixes: 7705bb7176b9 ("clk: qcom: mmcc-msm8996: leave all mmagic gdscs and clocks always enabled")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/mmcc-msm8996.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/mmcc-msm8996.c b/drivers/clk/qcom/mmcc-msm8996.c
index ef7bde361fc3..b8dd374df2e4 100644
--- a/drivers/clk/qcom/mmcc-msm8996.c
+++ b/drivers/clk/qcom/mmcc-msm8996.c
@@ -1341,7 +1341,7 @@ static struct clk_branch mmss_mmagic_maxi_clk = {
 				&maxi_clk_src.clkr.hw
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},

-- 
2.47.3

Re: [PATCH 06/10] clk: qcom: mmcc-msm8996: keep mmss_mmagic_maxi_clk always enabled

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> Enabling vmem_maxi_clk fails with -EBUSY, the branch never leaving halt
> ("vmem_maxi_clk status stuck at 'off'"), which makes the VMEM block
> unusable. The MAXI branches sit behind the MMAGIC aggregator, so they
> cannot leave halt unless mmss_mmagic_maxi_clk is running.
> 
> Commit 7705bb7176b9 ("clk: qcom: mmcc-msm8996: leave all mmagic gdscs
> and clocks always enabled") marked the MMAGIC clocks CLK_IS_CRITICAL
> because there is no bus driver to manage them, but missed this one: its
> siblings mmss_mmagic_ahb_clk, mmagic_video_axi_clk and
> mmagic_video_noc_cfg_ahb_clk all carry the flag while
> mmss_mmagic_maxi_clk does not.
> 
> Mark it CLK_IS_CRITICAL as well.
> 
> Note that the halt bit is truthful here and must not be papered over
> with BRANCH_HALT_SKIP: touching the block while its AXI clock is off
> silently resets the SoC.

Will the right fix forward be to grab a handle to all of mmagic
for MNoC and activate them as needed? CLK_IS_CRITICAL is obviously
a workaround

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

Konrad

Re: [PATCH 06/10] clk: qcom: mmcc-msm8996: keep mmss_mmagic_maxi_clk always enabled

[Dmitry Baryshkov]

On Tue, Oct 20, 2026 at 10:35:36AM +0200, Konrad Dybcio wrote:
> On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> > Enabling vmem_maxi_clk fails with -EBUSY, the branch never leaving halt
> > ("vmem_maxi_clk status stuck at 'off'"), which makes the VMEM block
> > unusable. The MAXI branches sit behind the MMAGIC aggregator, so they
> > cannot leave halt unless mmss_mmagic_maxi_clk is running.
> > 
> > Commit 7705bb7176b9 ("clk: qcom: mmcc-msm8996: leave all mmagic gdscs
> > and clocks always enabled") marked the MMAGIC clocks CLK_IS_CRITICAL
> > because there is no bus driver to manage them, but missed this one: its
> > siblings mmss_mmagic_ahb_clk, mmagic_video_axi_clk and
> > mmagic_video_noc_cfg_ahb_clk all carry the flag while
> > mmss_mmagic_maxi_clk does not.
> > 
> > Mark it CLK_IS_CRITICAL as well.
> > 
> > Note that the halt bit is truthful here and must not be papered over
> > with BRANCH_HALT_SKIP: touching the block while its AXI clock is off
> > silently resets the SoC.
> 
> Will the right fix forward be to grab a handle to all of mmagic
> for MNoC and activate them as needed? CLK_IS_CRITICAL is obviously
> a workaround

Most likely. Some of the clocks might need to be removed from the voting
model at all. I'd say that it's a separate set of fixes, possibly
affecting other platforms too. I have it on my todo list, but it's
long...

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

-- 
With best wishes
Dmitry

[PATCH 07/10] clk: qcom: cbf-msm8996: program the MSM8996 Pro post-divider

[Dmitry Baryshkov]

On MSM8996 Pro the CBF runs at twice the requested rate below 600 MHz,
at the voltage of the requested rate, and the SoC hangs once the fabric
is allowed to scale down. The Pro post-divider of 4 is only set in the
PLL configuration after clk_alpha_pll_configure() has already
programmed the hardware, so the PLL keeps dividing by 2 while the clock
tree divides its rate by 4.

Set up the Pro post-divider before the PLL is configured.

Fixes: bc48641a68dc ("clk: qcom: cbf-msm8996: Add support for MSM8996 Pro")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-cbf-8996.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
index 0b40ed601f9a..1bedd14aa3b0 100644
--- a/drivers/clk/qcom/clk-cbf-8996.c
+++ b/drivers/clk/qcom/clk-cbf-8996.c
@@ -287,6 +287,11 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
 			   CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_MASK,
 			   CBF_MUX_AUTO_CLK_SEL_ALWAYS_ON_GPLL0_SEL);
 
+	if (of_device_is_compatible(dev->of_node, "qcom,msm8996pro-cbf")) {
+		cbfpll_config.post_div_val = 0x3 << 8;
+		cbf_pll_postdiv.div = 4;
+	}
+
 	clk_alpha_pll_configure(&cbf_pll, regmap, &cbfpll_config);
 
 	/* Wait for PLL(s) to lock */
@@ -303,11 +308,6 @@ static int qcom_msm8996_cbf_probe(struct platform_device *pdev)
 	/* Switch CBF to use the primary PLL */
 	regmap_update_bits(regmap, CBF_MUX_OFFSET, CBF_MUX_PARENT_MASK, 0x1);
 
-	if (of_device_is_compatible(dev->of_node, "qcom,msm8996pro-cbf")) {
-		cbfpll_config.post_div_val = 0x3 << 8;
-		cbf_pll_postdiv.div = 4;
-	}
-
 	for (i = 0; i < ARRAY_SIZE(cbf_msm8996_hw_clks); i++) {
 		ret = devm_clk_hw_register(dev, cbf_msm8996_hw_clks[i]);
 		if (ret)

-- 
2.47.3

Re: [PATCH 07/10] clk: qcom: cbf-msm8996: program the MSM8996 Pro post-divider

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> On MSM8996 Pro the CBF runs at twice the requested rate below 600 MHz,
> at the voltage of the requested rate, and the SoC hangs once the fabric
> is allowed to scale down. The Pro post-divider of 4 is only set in the
> PLL configuration after clk_alpha_pll_configure() has already
> programmed the hardware, so the PLL keeps dividing by 2 while the clock
> tree divides its rate by 4.
> 
> Set up the Pro post-divider before the PLL is configured.
> 
> Fixes: bc48641a68dc ("clk: qcom: cbf-msm8996: Add support for MSM8996 Pro")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

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

Konrad

[PATCH 08/10] dt-bindings: clock: qcom,msm8996-apcc: add the MSM8996 Pro compatible

[Dmitry Baryshkov]

The MSM8996 Pro CPU clock controller needs a setting the MSM8996 one
does not, but the binding offers no way to tell the two apart.

Add a compatible for MSM8996 Pro, falling back to the MSM8996 one.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
index fcace96c72eb..e036749626f7 100644
--- a/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml
@@ -15,8 +15,11 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - qcom,msm8996-apcc
+    oneOf:
+      - const: qcom,msm8996-apcc
+      - items:
+          - const: qcom,msm8996pro-apcc
+          - const: qcom,msm8996-apcc
 
   reg:
     maxItems: 1

-- 
2.47.3

Re: [PATCH 08/10] dt-bindings: clock: qcom,msm8996-apcc: add the MSM8996 Pro compatible

[Krzysztof Kozlowski]

On Mon, Oct 19, 2026 at 04:41:07PM +0300, Dmitry Baryshkov wrote:
> The MSM8996 Pro CPU clock controller needs a setting the MSM8996 one
> does not, but the binding offers no way to tell the two apart.
> 
> Add a compatible for MSM8996 Pro, falling back to the MSM8996 one.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/clock/qcom,msm8996-apcc.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

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

Best regards,
Krzysztof

[PATCH 09/10] clk: qcom: cpu-8996: program the MSM8996 Pro droop code register

[Dmitry Baryshkov]

When the vendor kernel enables ACD on MSM8996 Pro, it also programs the
MDD droop code register of both clusters, so that they switch to the
always-on clock source when entering the D2-D5 low power states and
gate the limits management clock while asleep. This driver enables ACD
on MSM8996 Pro as well, but leaves the register at its reset value.

Program it on MSM8996 Pro before the clusters switch to the ACD leg.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-cpu-8996.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
index f22dd80919fa..d428043ecbee 100644
--- a/drivers/clk/qcom/clk-cpu-8996.c
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -56,6 +56,7 @@
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <soc/qcom/kryo-l2-accessors.h>
@@ -80,6 +81,7 @@ enum _pmux_input {
 #define MUX_OFFSET	0x40
 #define CLK_CTL_OFFSET 0x44
 #define CLK_CTL_AUTO_CLK_SEL BIT(8)
+#define MDD_DROOP_CODE_OFFSET	0x7c
 #define ALT_PLL_OFFSET	0x100
 #define SSSCTL_OFFSET 0x160
 #define PSCTL_OFFSET 0x164
@@ -471,6 +473,15 @@ static int qcom_cpu_clk_msm8996_register_clks(struct device *dev,
 	/* Ensure write goes through before muxes are switched */
 	udelay(5);
 
+	if (of_device_is_compatible(dev->of_node, "qcom,msm8996pro-apcc")) {
+		/*
+		 * Switch to the always-on source on D2-D5 entry and gate the
+		 * limits management clock in sleep.
+		 */
+		regmap_write(regmap, PWRCL_REG_OFFSET + MDD_DROOP_CODE_OFFSET, 0x3);
+		regmap_write(regmap, PERFCL_REG_OFFSET + MDD_DROOP_CODE_OFFSET, 0x3);
+	}
+
 	qcom_cpu_clk_msm8996_acd_init(regmap);
 
 	/* Pulse swallower and soft-start settings */

-- 
2.47.3

Re: [PATCH 09/10] clk: qcom: cpu-8996: program the MSM8996 Pro droop code register

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> When the vendor kernel enables ACD on MSM8996 Pro, it also programs the
> MDD droop code register of both clusters, so that they switch to the
> always-on clock source when entering the D2-D5 low power states and
> gate the limits management clock while asleep. This driver enables ACD
> on MSM8996 Pro as well, but leaves the register at its reset value.
> 
> Program it on MSM8996 Pro before the clusters switch to the ACD leg.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

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

Konrad

[PATCH 10/10] arm64: dts: qcom: msm8996pro: use the MSM8996 Pro CPU clock compatible

[Dmitry Baryshkov]

The CPU clock controller of MSM8996 Pro needs its own setup, which the
driver can only apply when the DT tells it the SoC is the Pro one.

Use the MSM8996 Pro compatible.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/msm8996pro.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8996pro.dtsi b/arch/arm64/boot/dts/qcom/msm8996pro.dtsi
index b74cff06f300..60e2d4270f08 100644
--- a/arch/arm64/boot/dts/qcom/msm8996pro.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996pro.dtsi
@@ -340,3 +340,7 @@ opp-560000000 {
 &cbf {
 	compatible = "qcom,msm8996pro-cbf";
 };
+
+&kryocc {
+	compatible = "qcom,msm8996pro-apcc", "qcom,msm8996-apcc";
+};

-- 
2.47.3

Re: [PATCH 10/10] arm64: dts: qcom: msm8996pro: use the MSM8996 Pro CPU clock compatible

[Konrad Dybcio]

On 10/19/26 3:41 PM, Dmitry Baryshkov wrote:
> The CPU clock controller of MSM8996 Pro needs its own setup, which the
> driver can only apply when the DT tells it the SoC is the Pro one.
> 
> Use the MSM8996 Pro compatible.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---

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

Konrad