Re: [PATCH v4 25/52] clk: qcom: gcc-sdm845: Tie the CX power domain to controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The GCC controller will never runtime suspend (and thus never power down the CX domain) because CLK_IS_CRITICAL clocks hold a permanent runtime PM reference.
--

commit 276e3a012e0f4a45e79abff9518b1f5b28d649cd
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

clk: qcom: gcc-sdm845: Tie the CX power domain to controller

This commit enables runtime PM for the controller, allowing the CX power
domain to be powered down once all GCC consumers have released their clocks
and GDSCs.

> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index 57d54b45fa527..cde863dcf1851 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -3959,6 +3959,7 @@ static const struct qcom_cc_desc gcc_sdm845_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_sdm845_resets),
>  	.gdscs = gcc_sdm845_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_sdm845_gdscs),
> +	.use_rpm = true,

[Severity: Medium]
Will the GCC controller ever actually runtime suspend to power down the CX
domain with this change?

The driver has 13 clocks marked as CLK_IS_CRITICAL, for example:

static struct clk_branch gcc_camera_ahb_clk = {
	...
	.clkr = {
		...
		.hw.init = &(struct clk_init_data){
			.name = "gcc_camera_ahb_clk",
			.flags = CLK_IS_CRITICAL,
			...
		},
	},
};

When qcom_cc_really_probe() registers clocks with use_rpm = true, the common
clock framework's clk_pm_runtime_init() sets rpm_enabled = true.

For critical clocks, __clk_core_init() automatically calls clk_core_prepare().
This triggers clk_pm_runtime_get(), which calls pm_runtime_resume_and_get()
and increments the GCC controller's runtime PM usage counter.

Since critical clocks are never unprepared, these references are never
released. Because of this, it appears the controller's usage count will never
drop to zero and the CX domain will permanently remain active, defeating the
purpose of enabling runtime PM.

>  };
>  
>  static const struct of_device_id gcc_sdm845_match_table[] = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-qcom-tie-cx-to-gcc-v4-0-9d5a606ada33@oss.qualcomm.com?part=25