Re: [PATCH v5 34/42] arm64: dts: qcom: sm6350: Add GCC CX power domain

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PM regression: GCC device pins CX power domain ON permanently
--

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

arm64: dts: qcom: sm6350: Add GCC CX power domain

This commit adds the RPMh CX power domain reference to the GCC node on
SM6350. This allows votes from GCC GDSC consumers to propagate to the
RPMh CX domain.

> diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> index 15d38b7e803c5..ded5c3a45891b 100644
> --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> @@ -794,6 +794,7 @@ gcc: clock-controller@100000 {
>  			#clock-cells = <1>;
>  			#reset-cells = <1>;
>  			#power-domain-cells = <1>;
> +			power-domains = <&rpmhpd SM6350_CX>;

[Severity: High]
Does adding the PM domain to the DT node without the corresponding driver
supporting runtime PM cause a power management regression here?

The genpd framework attaches the GCC device and unconditionally powers ON the
CX domain during probe via genpd_dev_pm_attach(). In order for genpd to
subsequently power off the parent domain, all attached devices must be
suspended, which is evaluated via pm_runtime_suspended().

However, pm_runtime_suspended() requires runtime PM to be enabled. If
pm_runtime_enable() has never been called, the device disable_depth is > 0,
causing pm_runtime_suspended() to unconditionally return false.

Looking at gcc_sm6350_desc in drivers/clk/qcom/gcc-sm6350.c, it does not set
the use_rpm flag to true:

static const struct qcom_cc_desc gcc_sm6350_desc = {
	.config = &gcc_sm6350_regmap_config,
	.clks = gcc_sm6350_clocks,
	.num_clks = ARRAY_SIZE(gcc_sm6350_clocks),
	.resets = gcc_sm6350_resets,
	.num_resets = ARRAY_SIZE(gcc_sm6350_resets),
	.gdscs = gcc_sm6350_gdscs,
	.num_gdscs = ARRAY_SIZE(gcc_sm6350_gdscs),
};

Without use_rpm = true, devm_pm_runtime_enable() is never invoked for the
device. Does this leave the GCC device permanently considered active and the
CX domain irrevocably pinned ON?

>  			clock-names = "bi_tcxo",
>  				      "bi_tcxo_ao",
>  				      "sleep_clk";

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