Re: [PATCH v5 27/42] clk: qcom: gcc-x1e80100: 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:
- [High] clk: qcom: gcc-x1e80100: missing register restore after CX power down
--

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

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

This commit enables runtime PM for the GCC controller by setting use_rpm to
true. This allows the CX power domain to be powered down when all GCC clocks
and GDSCs are released by their consumers.

> diff --git a/drivers/clk/qcom/gcc-x1e80100.c b/drivers/clk/qcom/gcc-x1e80100.c
> index 9402fd7fddb8f..b979b74153774 100644
> --- a/drivers/clk/qcom/gcc-x1e80100.c
> +++ b/drivers/clk/qcom/gcc-x1e80100.c
> @@ -7449,6 +7449,7 @@ static const struct qcom_cc_desc gcc_x1e80100_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_x1e80100_resets),
>  	.gdscs = gcc_x1e80100_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_x1e80100_gdscs),
> +	.use_rpm = true,

[Severity: High]
Will the manual register configurations performed during gcc_x1e80100_probe()
be lost here when the CX domain collapses?

During probe, several manual register writes are made that are not tied to
any specific clock structure:

gcc_x1e80100_probe() {
    ...
    qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
    qcom_branch_set_clk_en(regmap, 0x7d01c); /* GCC_HLOS1_VOTE_AGGRE_NOC... */
    
    /* Clear GDSC_SLEEP_ENA_VOTE to stop votes being auto-removed in sleep. */
    regmap_write(regmap, 0x52224, 0x0);
    
    /* FORCE_MEM_CORE_ON for ufs phy ice core and gcc ufs phy axi clocks  */
    qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);
    qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_axi_clk, true);
    ...
}

When runtime PM is enabled, the CX power domain can power down during deep
sleep when consumers release their clocks. Since the driver has no pm
operations to manually restore these writes upon resuming, and
gcc_x1e80100_regmap_config does not configure a regmap cache (such as
REGCACHE_MAPLE) to auto-restore them, won't these registers revert to their
hardware default states on wake?

Could this cause dependent subsystems like the GPU and UFS to malfunction or
crash after resuming from sleep?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-qcom-tie-cx-to-gcc-v5-0-38732033e5b5@oss.qualcomm.com?part=27