Re: [PATCH v4 30/52] clk: qcom: gcc-sm8250: 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 `CLK_IS_CRITICAL` flag on `gcc_cpuss_ahb_clk` prevents the GCC controller from ever entering runtime suspend, defeating the patch's purpose.
--

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

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

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

> diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c
> index 1c59d70e0f96c..44f6e197433eb 100644
> --- a/drivers/clk/qcom/gcc-sm8250.c
> +++ b/drivers/clk/qcom/gcc-sm8250.c

[ ... ]

> @@ -3619,6 +3619,7 @@ static const struct qcom_cc_desc gcc_sm8250_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_sm8250_resets),
>  	.gdscs = gcc_sm8250_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_sm8250_gdscs),
> +	.use_rpm = true,

[Severity: Medium]
Will this controller ever actually enter runtime suspend to achieve the
intended power savings?

Because gcc_cpuss_ahb_clk is defined in this file with the CLK_IS_CRITICAL
flag, the clock framework automatically prepares it at registration and never
unprepares it. During clk_core_prepare(), the framework calls
pm_runtime_resume_and_get() on the provider device (the GCC controller)
because it is now runtime PM enabled.

Since this critical clock is never disabled, won't the runtime PM usage
counter for the GCC controller permanently remain above zero? This would
prevent the controller from entering runtime suspend, meaning the CX domain
will never dynamically power down during runtime as intended.

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