Re: [PATCH v3 16/47] clk: qcom: gcc-sar2130p: 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:

Pre-existing issues:
- [High] In gdsc_register(), when an error occurs during gdsc_init() or subsequent subdomain attachment, the function fails to unregister the previously initialized pm_genpd domains, leading to a permanent leak and linked-list corruption upon driver reprobe.
--

--- Patch [16]: [PATCH v3 16/47] clk: qcom: gcc-sar2130p: Tie the CX power domain to controller ---
commit a5913ce7b2e1ade74a139dc203b812733f756b77
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

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

This commit enables runtime PM for the controller by setting use_rpm to true
in the gcc_sar2130p_desc structure. This allows the CX power domain to be
powered down once all GCC consumers release their clocks and GDSCs.

> diff --git a/drivers/clk/qcom/gcc-sar2130p.c b/drivers/clk/qcom/gcc-sar2130p.c
> index 475e2cda3618b..1a16f9d68babb 100644
> --- a/drivers/clk/qcom/gcc-sar2130p.c
> +++ b/drivers/clk/qcom/gcc-sar2130p.c
> @@ -2308,6 +2308,7 @@ static const struct qcom_cc_desc gcc_sar2130p_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_sar2130p_resets),
>  	.gdscs = gcc_sar2130p_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_sar2130p_gdscs),

[Severity: High]
This is a pre-existing issue, but I noticed something when looking at how
GDSCs are registered for this controller.

If gdsc_register() in drivers/clk/qcom/gdsc.c fails, could this lead to a
memory leak and linked-list corruption on a subsequent driver reprobe?

drivers/clk/qcom/gdsc.c:gdsc_register() {
    ...
		ret = gdsc_init(scs[i]);
		if (ret)
			return ret;
    ...
err_pm_subdomain_remove:
	gdsc_pm_subdomain_remove(desc, i);

	return ret;
}

When an error occurs during gdsc_init() or subsequent subdomain attachment,
the function returns without unregistering the previously initialized pm_genpd
domains.

Since the gdsc structs and their embedded generic_pm_domain structs are
typically statically allocated by clock drivers, a subsequent driver reprobe
(such as from deferred probe) will call pm_genpd_init() again on the same
structs.

Will this attempt to list_add() the node into gpd_list when it is already
linked, corrupting the list?

> +	.use_rpm = true,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-tie-cx-to-gcc-v3-0-f1205a92682b@oss.qualcomm.com?part=16