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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Critical clocks permanently hold a runtime PM reference on the controller, preventing it from suspending and defeating the patch's purpose to power down the CX domain.
- [High] Enabling runtime PM breaks the manual boot handoff of the GPU GDSC if the controller actually suspends.
--

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

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

Enable 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-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
> index 4a49ad7a9e5be..f3a245470a4de 100644
> --- a/drivers/clk/qcom/gcc-sc7180.c
> +++ b/drivers/clk/qcom/gcc-sc7180.c
> @@ -2418,6 +2418,7 @@ static const struct qcom_cc_desc gcc_sc7180_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_sc7180_resets),
>  	.gdscs = gcc_sc7180_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_sc7180_gdscs),
> +	.use_rpm = true,

[Severity: High]
Since this controller uses critical clocks like gcc_cpuss_ahb_clk and
gcc_sys_noc_cpuss_ahb_clk (registered with CLK_IS_CRITICAL), will the
Common Clock Framework indefinitely hold a runtime PM reference on the
controller?

During clock registration, the CCF calls clk_core_prepare_enable() for
critical clocks, which increments the provider device's PM runtime usage
counter via clk_pm_runtime_get().

Because critical clocks are never unprepared, the PM usage counter will never
drop to zero. Will this prevent the GCC controller from runtime suspending,
thereby permanently keeping the CX power domain on and defeating the purpose
of this patch?

[Severity: High]
If the controller does manage to runtime suspend, does this break the
manual boot handoff of the GPU GDSC?

In gcc_sc7180_probe(), raw register writes are used to enable certain clocks:

    qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */

Because this bypasses the Common Clock Framework, no PM runtime reference is
taken to keep the controller active for these blocks.

If pm_runtime_put_sync() at the end of probe actually suspends the device,
cutting power to the CX domain, will these manual writes be rendered
ineffective and cause a crash when downstream drivers probe?

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

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