[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

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

[Taniya Das]

On 7/15/2026 6:59 PM, Abel Vesa wrote:
> 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(+)


Abel, I feel we should add the 'required-opps' as well which will ensure
the clock controllers minimum voltage requirement.

-- 
Thanks,
Taniya Das

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

[Abel Vesa]

On 26-07-16 10:13:48, Taniya Das wrote:
> 
> 
> On 7/15/2026 6:59 PM, Abel Vesa wrote:
> > 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(+)
> 
> 
> Abel, I feel we should add the 'required-opps' as well which will ensure
> the clock controllers minimum voltage requirement.

But it was agreed in v1 that we should not add the required-opps. So we
dropped it in v2.

Can you give more details to why you think it is needed ?

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

[Taniya Das]

On 7/16/2026 12:52 PM, Abel Vesa wrote:
> On 26-07-16 10:13:48, Taniya Das wrote:
>>
>>
>> On 7/15/2026 6:59 PM, Abel Vesa wrote:
>>> 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(+)
>>
>>
>> Abel, I feel we should add the 'required-opps' as well which will ensure
>> the clock controllers minimum voltage requirement.
> 
> But it was agreed in v1 that we should not add the required-opps. So we
> dropped it in v2.
> 

Okay, sorry, may be I missed that comment.


> Can you give more details to why you think it is needed ?

My reasoning is that if a clock controller requires a rail, it is
preferable to explicitly specify the required operating level rather
than rely on system/client vote. This makes the dependency
self-contained and easier to reason about. Clients that require a higher
operating level than the clock controller's minimum requirement remain
free to vote for a higher level as needed.


-- 
Thanks,
Taniya Das

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

[Bjorn Andersson]

On Thu, Jul 16, 2026 at 04:42:11PM +0530, Taniya Das wrote:
> 
> 
> On 7/16/2026 12:52 PM, Abel Vesa wrote:
> > On 26-07-16 10:13:48, Taniya Das wrote:
> >>
> >>
> >> On 7/15/2026 6:59 PM, Abel Vesa wrote:
> >>> 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(+)
> >>
> >>
> >> Abel, I feel we should add the 'required-opps' as well which will ensure
> >> the clock controllers minimum voltage requirement.
> > 
> > But it was agreed in v1 that we should not add the required-opps. So we
> > dropped it in v2.
> > 
> 
> Okay, sorry, may be I missed that comment.
> 
> 
> > Can you give more details to why you think it is needed ?
> 
> My reasoning is that if a clock controller requires a rail, it is
> preferable to explicitly specify the required operating level rather
> than rely on system/client vote. This makes the dependency
> self-contained and easier to reason about. Clients that require a higher
> operating level than the clock controller's minimum requirement remain
> free to vote for a higher level as needed.
> 

You're expressing this in generic terms, can you please provide the
concrete example of what this would look for in the Glymur GCC case? 

What is the required-opps for the Glymur GCC?

Regards,
Bjorn

> 
> -- 
> Thanks,
> Taniya Das
> 

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

[Bjorn Andersson]

On Wed, 15 Jul 2026 16:29:13 +0300, Abel Vesa wrote:
> 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:
> 
> [...]

Applied, thanks!

[1/3] dt-bindings: clock: qcom: glymur-gcc: Add missing CX power domain
      commit: d22a37e9fba7838cdcb8c55b913b1a37a7a4a198
[2/3] clk: qcom: gcc-glymur: Enable runtime PM
      commit: 8d4f342369d0d77f32a0211692442d3b6d455872

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

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

[Taniya Das]

On 7/17/2026 6:45 AM, Bjorn Andersson wrote:
> On Thu, Jul 16, 2026 at 04:42:11PM +0530, Taniya Das wrote:
>>
>>
>> On 7/16/2026 12:52 PM, Abel Vesa wrote:
>>> On 26-07-16 10:13:48, Taniya Das wrote:
>>>>
>>>>
>>>> On 7/15/2026 6:59 PM, Abel Vesa wrote:
>>>>> 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(+)
>>>>
>>>>
>>>> Abel, I feel we should add the 'required-opps' as well which will ensure
>>>> the clock controllers minimum voltage requirement.
>>>
>>> But it was agreed in v1 that we should not add the required-opps. So we
>>> dropped it in v2.
>>>
>>
>> Okay, sorry, may be I missed that comment.
>>
>>
>>> Can you give more details to why you think it is needed ?
>>
>> My reasoning is that if a clock controller requires a rail, it is
>> preferable to explicitly specify the required operating level rather
>> than rely on system/client vote. This makes the dependency
>> self-contained and easier to reason about. Clients that require a higher
>> operating level than the clock controller's minimum requirement remain
>> free to vote for a higher level as needed.
>>
> 
> You're expressing this in generic terms, can you please provide the
> concrete example of what this would look for in the Glymur GCC case? 
> 

Yes, Bjorn, this is indeed a general requirement for a clock controller.

In Glymur I do see that CX could drop to retention corner in case there
is no vote from any subsystem and so it is better to vote from clock
controller a level above retention using the required-opps.

> What is the required-opps for the Glymur GCC?

LOW_SVS(rpmhpd_opp_low_svs) corner should be good for GCC controller.

-- 
Thanks,
Taniya Das

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

[Konrad Dybcio]

On 7/17/26 11:30 AM, Taniya Das wrote:
> 
> 
> On 7/17/2026 6:45 AM, Bjorn Andersson wrote:
>> On Thu, Jul 16, 2026 at 04:42:11PM +0530, Taniya Das wrote:
>>>
>>>
>>> On 7/16/2026 12:52 PM, Abel Vesa wrote:
>>>> On 26-07-16 10:13:48, Taniya Das wrote:

[...]

>> You're expressing this in generic terms, can you please provide the
>> concrete example of what this would look for in the Glymur GCC case? 
>>
> 
> Yes, Bjorn, this is indeed a general requirement for a clock controller.
> 
> In Glymur I do see that CX could drop to retention corner in case there
> is no vote from any subsystem and so it is better to vote from clock
> controller a level above retention using the required-opps.
> 

Is this still a valid concern in light of

Commit 7178817f1904 ("pmdomain: qcom: rpmhpd: Skip retention by default")

?

Konrad