[PATCH v3 0/3] clk: qcom: gcc-glymur: Tie the CX power domain to controller

[PATCH v3 0/3] clk: qcom: gcc-glymur: Tie the CX power domain to controller

[Abel Vesa]

The suggestion of having the CX power domain tied up to the GCC
controller started here:

https://lore.kernel.org/lkml/fe210f0b-692b-4c45-afc4-fc2bc5e57854@oss.qualcomm.com/

and then, for Glymur, it has been brought up here as well:

https://lore.kernel.org/all/0248dc51-1036-426c-b1de-dbc71696e2c1@oss.qualcomm.com/

These plus a discussion off-list led to this patchset being done.

Sorry for such a late respin. This got lost somehow in the need-to-respin queue.

Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
Changes in v3:
- Rebased on next-20260713.
- Did not pick Konrad's R-b tag intentionally because the commit has
  been reworded.
- Link to v2: https://patch.msgid.link/20260309-glymur-fix-gcc-cx-scaling-v2-0-d7a58a0a9ecb@oss.qualcomm.com

Changes in v2:
- Dropped the required-opps property from both the schema and DT node.
- Link to v1: https://patch.msgid.link/20260309-glymur-fix-gcc-cx-scaling-v1-0-f682c82f116f@oss.qualcomm.com

---
Abel Vesa (3):
      dt-bindings: clock: qcom: glymur-gcc: Add missing CX power domain
      clk: qcom: gcc-glymur: Enable runtime PM
      arm64: dts: qcom: glymur: Add CX power domain to GCC

 Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml | 8 ++++++++
 arch/arm64/boot/dts/qcom/glymur.dtsi                         | 1 +
 drivers/clk/qcom/gcc-glymur.c                                | 1 +
 3 files changed, 10 insertions(+)
---
base-commit: 49362394dad7df66c274c867a271394c10ca2bb8
change-id: 20260309-glymur-fix-gcc-cx-scaling-a0b350cd5741

Best regards,
--  
Abel Vesa <abel.vesa@oss.qualcomm.com>

[PATCH v3 1/3] dt-bindings: clock: qcom: glymur-gcc: Add missing CX power domain

[Abel Vesa]

The GDSCs provided by the Glymur GCC are supplied by the RPMh CX power
domain. Model that parent domain in the GCC binding so the provider can
describe the dependency in devicetree.

Add a single CX power-domain entry to the binding and make it required,
matching the hardware description needed by the GCC node.

Fixes: ee2d967030fe ("dt-bindings: clock: qcom: document the Glymur Global Clock Controller")
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml
index b05b0e6c4483..7a4054c9f215 100644
--- a/Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,glymur-gcc.yaml
@@ -65,9 +65,15 @@ properties:
       - description: USB4 PHY 2 pcie pipe clock source
       - description: USB4 PHY 2 Max pipe clock source
 
+  power-domains:
+    description:
+      A phandle and PM domain specifier for the CX power domain.
+    maxItems: 1
+
 required:
   - compatible
   - clocks
+  - power-domains
   - '#power-domain-cells'
 
 allOf:
@@ -78,6 +84,7 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
     clock-controller@100000 {
       compatible = "qcom,glymur-gcc";
       reg = <0x100000 0x1f9000>;
@@ -113,6 +120,7 @@ examples:
                <&usb4_phy_0_pcie_pipe>, <&usb4_phy_0_max_pipe>,
                <&usb4_phy_1_pcie_pipe>, <&usb4_phy_1_max_pipe>,
                <&usb4_phy_2_pcie_pipe>, <&usb4_phy_2_max_pipe>;
+      power-domains = <&rpmhpd RPMHPD_CX>;
       #clock-cells = <1>;
       #reset-cells = <1>;
       #power-domain-cells = <1>;

-- 
2.54.0

[PATCH v3 2/3] clk: qcom: gcc-glymur: Enable runtime PM

[Abel Vesa]

Enable runtime PM for the controller so the common GCC probe path resumes
the attached domain while registering clocks, resets and GDSCs.

This lets GDSC consumers propagate their votes through the GCC provider to
the CX parent domain.

Fixes: efe504300a17 ("clk: qcom: gcc: Add support for Global Clock Controller")
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/clk/qcom/gcc-glymur.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gcc-glymur.c b/drivers/clk/qcom/gcc-glymur.c
index 6925c6865089..2ee4820b6fdf 100644
--- a/drivers/clk/qcom/gcc-glymur.c
+++ b/drivers/clk/qcom/gcc-glymur.c
@@ -8548,6 +8548,7 @@ static const struct qcom_cc_desc gcc_glymur_desc = {
 	.num_resets = ARRAY_SIZE(gcc_glymur_resets),
 	.gdscs = gcc_glymur_gdscs,
 	.num_gdscs = ARRAY_SIZE(gcc_glymur_gdscs),
+	.use_rpm = true,
 	.driver_data = &gcc_glymur_driver_data,
 };
 

-- 
2.54.0

[PATCH v3 3/3] arm64: dts: qcom: glymur: Add CX power domain to GCC

[Abel Vesa]

The GCC GDSCs on Glymur are backed by the RPMh CX power domain. Without
describing that parent domain, consumers of GCC-provided GDSCs can enable
their local domain without causing the required CX vote to be held.

Add the CX power-domain reference to the GCC node so votes from GCC GDSC
consumers can propagate to RPMh CX.

Fixes: 41b6e8db400c ("arm64: dts: qcom: Introduce Glymur base dtsi")
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
index 55d91c696a3a..db2d5cd3d98b 100644
--- a/arch/arm64/boot/dts/qcom/glymur.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
@@ -829,6 +829,7 @@ gcc: clock-controller@100000 {
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 			#power-domain-cells = <1>;
+			power-domains = <&rpmhpd RPMHPD_CX>;
 		};
 
 		gpi_dma2: dma-controller@800000 {

-- 
2.54.0

Re: [PATCH v3 3/3] arm64: dts: qcom: glymur: Add CX power domain to GCC

[Konrad Dybcio]

On 7/15/26 3:29 PM, Abel Vesa wrote:
> The GCC GDSCs on Glymur are backed by the RPMh CX power domain. Without
> describing that parent domain, consumers of GCC-provided GDSCs can enable
> their local domain without causing the required CX vote to be held.
> 
> Add the CX power-domain reference to the GCC node so votes from GCC GDSC
> consumers can propagate to RPMh CX.
> 
> Fixes: 41b6e8db400c ("arm64: dts: qcom: Introduce Glymur base dtsi")
> Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 2/3] clk: qcom: gcc-glymur: Enable runtime PM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The GDSC registration framework initializes power domains using `pm_genpd_init()` but fails to call `pm_genpd_remove()` during unregistration or on probe error paths. This leaves dangling pointers in the global `gpd_list` when the module is unloaded, leading to a Use-After-Free (UAF).
- [High] The error path in `qcom_cc_really_probe()` uses the asynchronous `pm_runtime_put()` before returning an error, triggering devres cleanup which immediately calls `pm_runtime_disable()`. This cancels the pending asynchronous idle/suspend work, permanently leaking the power domain's ON state.
--

--- Patch [2]: [PATCH v3 2/3] clk: qcom: gcc-glymur: Enable runtime PM ---
commit 5dd235050afe17992844a83f5c4a8e644843386b
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

clk: qcom: gcc-glymur: Enable runtime PM

This commit enables runtime PM for the controller so the common GCC probe
path resumes the attached domain while registering clocks, resets and GDSCs.

> diff --git a/drivers/clk/qcom/gcc-glymur.c b/drivers/clk/qcom/gcc-glymur.c
> index f4ede4a3a1c07..1d52150721adc 100644
> --- a/drivers/clk/qcom/gcc-glymur.c
> +++ b/drivers/clk/qcom/gcc-glymur.c
> @@ -8576,6 +8576,7 @@ static const struct qcom_cc_desc gcc_glymur_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_glymur_resets),
>  	.gdscs = gcc_glymur_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_glymur_gdscs),

[Severity: Critical]
This isn't a bug introduced by this patch, but does registering these GDSCs
expose a use-after-free vulnerability during module unload or probe failure?

Looking at gdsc_unregister() in drivers/clk/qcom/gdsc.c:

gdsc_unregister() {
    ...
	gdsc_pm_subdomain_remove(desc, num);
	of_genpd_del_provider(dev->of_node);
}

It appears pm_genpd_remove() is never called to unlink the domains from the
global gpd_list. If gdsc_register() fails halfway or the module is unloaded,
the memory backing the GDSC structs is freed. 

Could any subsequent iteration of gpd_list (e.g., by
genpd_power_off_unused() or debugfs) dereference this freed memory and
cause a kernel panic?

> +	.use_rpm = true,

[Severity: High]
This is a pre-existing issue, but by setting use_rpm to true, does this
expose the driver to a power domain leak in the probe error path?

Looking at qcom_cc_really_probe() in drivers/clk/qcom/common.c, the error
path uses the asynchronous pm_runtime_put():

qcom_cc_really_probe() {
    ...
put_rpm:
	if (desc->use_rpm)
		pm_runtime_put(dev);

	return ret;
}

Since devm_pm_runtime_enable() is used, devres will automatically execute
pm_runtime_disable() on probe failure. 

Does this cancel the pending asynchronous idle/suspend work queued by
pm_runtime_put(), leaving the hardware permanently in an active state?

>  	.driver_data = &gcc_glymur_driver_data,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-glymur-fix-gcc-cx-scaling-v3-0-72eb5adad156@oss.qualcomm.com?part=2