[PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Imran Shaik]

clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
every clock, keeping them non-zero until a consumer takes over. If there
is no consumer, those clocks will remain active in the idle scenario as
well, and the sleep vote is never cleared, blocking XO shutdown.

Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
keeping other targets unaffected.

Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
---
This patch is dependent on [1], as crypto driver earlier was surviving on
the proxy votes on Qualcomm Shikra SoC and dropping the proxy votes will
impact the bootup. Now patch series [1] takes care of the resource
voting from the crypto driver side.

[1] https://lore.kernel.org/linux-arm-msm/20260907-b4-shikra_crypto_changse-v6-0-0676f61894b3@oss.qualcomm.com/
---
 drivers/clk/qcom/clk-smd-rpm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 487d715ed5d2..b78c8cc730b5 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -183,6 +183,7 @@ struct rpm_smd_clk_desc {
 	const struct clk_smd_rpm ** const icc_clks;
 	size_t num_icc_clks;
 	bool scaling_before_handover;
+	bool skip_clks_handoff;
 };
 
 static DEFINE_MUTEX(rpm_smd_clk_lock);
@@ -1280,7 +1281,8 @@ static const struct rpm_smd_clk_desc rpm_clk_qcm2290 = {
 	.clks = qcm2290_clks,
 	.num_clks = ARRAY_SIZE(qcm2290_clks),
 	.icc_clks = qcm2290_icc_clks,
-	.num_icc_clks = ARRAY_SIZE(qcm2290_icc_clks)
+	.num_icc_clks = ARRAY_SIZE(qcm2290_icc_clks),
+	.skip_clks_handoff = true,
 };
 
 static const struct of_device_id rpm_smd_clk_match_table[] = {
@@ -1358,13 +1360,15 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
 			goto err;
 	}
 
-	for (i = 0; i < num_clks; i++) {
-		if (!rpm_smd_clks[i])
-			continue;
+	if (!desc->skip_clks_handoff) {
+		for (i = 0; i < num_clks; i++) {
+			if (!rpm_smd_clks[i])
+				continue;
 
-		ret = clk_smd_rpm_handoff(rpm_smd_clks[i]);
-		if (ret)
-			goto err;
+			ret = clk_smd_rpm_handoff(rpm_smd_clks[i]);
+			if (ret)
+				goto err;
+		}
 	}
 
 	for (i = 0; i < desc->num_icc_clks; i++) {

---
base-commit: c68a982815dcce5464e3bf2a31ac94f5146c04ca
change-id: 20260909-clk-smd-rpm-skip-proxy-7725549ea74d
prerequisite-change-id: 20260701-b4-shikra_crypto_changse-f2d6d5bf04b5:v6
prerequisite-patch-id: e24447d3ab70f48b099251c3d6c1307e0aee9d2c
prerequisite-patch-id: 64e9302a1de7a654ffa65af12daff3866531a00c

Best regards,
--  
Imran Shaik <imran.shaik@oss.qualcomm.com>

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[sashiko-bot]

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

Pre-existing issues:
- [High] `clk_smd_rpm_set_rate` silently ignores rate updates when the clock is disabled, leading to incorrect hardware state upon enable.
--

commit 8065b2a650b0e543b5eb4069b2bca8805e5bbb71
Author: Imran Shaik <imran.shaik@oss.qualcomm.com>

    clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290
    
    clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
    every clock, keeping them non-zero until a consumer takes over. If there
    is no consumer, those clocks will remain active in the idle scenario as
    well, and the sleep vote is never cleared, blocking XO shutdown.
    
    Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
    keeping other targets unaffected.

> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> index 487d715ed5d21..b78c8cc730b59 100644
> --- a/drivers/clk/qcom/clk-smd-rpm.c
> +++ b/drivers/clk/qcom/clk-smd-rpm.c

[ ... ]

> @@ -1358,13 +1360,15 @@ static int rpm_smd_clk_probe(struct platform_device *pdev)
>  			goto err;
>  	}
>  
> -	for (i = 0; i < num_clks; i++) {
> -		if (!rpm_smd_clks[i])
> -			continue;
> +	if (!desc->skip_clks_handoff) {
> +		for (i = 0; i < num_clks; i++) {
> +			if (!rpm_smd_clks[i])
> +				continue;
>  
> -		ret = clk_smd_rpm_handoff(rpm_smd_clks[i]);
> -		if (ret)
> -			goto err;
> +			ret = clk_smd_rpm_handoff(rpm_smd_clks[i]);
> +			if (ret)
> +				goto err;
> +		}
>  	}
>  

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential issue in
how rate updates are handled for disabled clocks. In clk_smd_rpm_set_rate():

drivers/clk/qcom/clk-smd-rpm.c:clk_smd_rpm_set_rate() {
	...
	if (!r->enabled)
		return 0;
	...
	r->rate = rate;

	return 0;
}

If a consumer calls clk_set_rate() on a disabled clock and then calls
clk_prepare_enable(), doesn't the early return in clk_smd_rpm_set_rate() skip
saving the requested rate?

Since r->rate = rate is bypassed, wouldn't the subsequent enable operation
transmit the stale initialization rate to the RPM hardware instead of the newly
requested rate?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-clk-smd-rpm-skip-proxy-v1-1-1cb5694a99d9@oss.qualcomm.com?part=1

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Konrad Dybcio]

On 9/10/26 3:44 PM, Imran Shaik wrote:
> clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
> every clock, keeping them non-zero until a consumer takes over. If there
> is no consumer, those clocks will remain active in the idle scenario as
> well, and the sleep vote is never cleared, blocking XO shutdown.
> 
> Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
> keeping other targets unaffected.
> 
> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---

Are you booting with clk_ignore_unused?

Konrad

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Imran Shaik]

On 11-09-2026 02:36 pm, Konrad Dybcio wrote:
> On 9/10/26 3:44 PM, Imran Shaik wrote:
>> clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
>> every clock, keeping them non-zero until a consumer takes over. If there
>> is no consumer, those clocks will remain active in the idle scenario as
>> well, and the sleep vote is never cleared, blocking XO shutdown.
>>
>> Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
>> keeping other targets unaffected.
>>
>> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>> ---
> 
> Are you booting with clk_ignore_unused?
> 

No Konrad, clk_ignore_unused is not present.

Irrespective of clk_ignore_unused, the proxy votes are placed to RPM
during handoff. If no consumer takes over, those votes remain active,
keeping the resource ON in idle and preventing XO shutdown.

Thanks,
Imran

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Konrad Dybcio]

On 9/16/26 1:05 PM, Imran Shaik wrote:
> 
> 
> On 11-09-2026 02:36 pm, Konrad Dybcio wrote:
>> On 9/10/26 3:44 PM, Imran Shaik wrote:
>>> clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
>>> every clock, keeping them non-zero until a consumer takes over. If there
>>> is no consumer, those clocks will remain active in the idle scenario as
>>> well, and the sleep vote is never cleared, blocking XO shutdown.
>>>
>>> Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
>>> keeping other targets unaffected.
>>>
>>> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
>>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>>> ---
>>
>> Are you booting with clk_ignore_unused?
>>
> 
> No Konrad, clk_ignore_unused is not present.
> 
> Irrespective of clk_ignore_unused, the proxy votes are placed to RPM
> during handoff. If no consumer takes over, those votes remain active,
> keeping the resource ON in idle and preventing XO shutdown.

I re-read this and yeah you're right

Is the handoff functionality necessary at all for non-icc clocks?
I'm suspecting that this was just a port of the ancient msm-3.10
logic where the (modified) clock framework had a handoff mechanism
similar to today's sync_state, except the toning-down of these
clocks was never added

Konrad

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Imran Shaik]

On 21-09-2026 02:23 pm, Konrad Dybcio wrote:
> On 9/16/26 1:05 PM, Imran Shaik wrote:
>>
>>
>> On 11-09-2026 02:36 pm, Konrad Dybcio wrote:
>>> On 9/10/26 3:44 PM, Imran Shaik wrote:
>>>> clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
>>>> every clock, keeping them non-zero until a consumer takes over. If there
>>>> is no consumer, those clocks will remain active in the idle scenario as
>>>> well, and the sleep vote is never cleared, blocking XO shutdown.
>>>>
>>>> Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
>>>> keeping other targets unaffected.
>>>>
>>>> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
>>>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
>>>> ---
>>>
>>> Are you booting with clk_ignore_unused?
>>>
>>
>> No Konrad, clk_ignore_unused is not present.
>>
>> Irrespective of clk_ignore_unused, the proxy votes are placed to RPM
>> during handoff. If no consumer takes over, those votes remain active,
>> keeping the resource ON in idle and preventing XO shutdown.
> 
> I re-read this and yeah you're right
> 
> Is the handoff functionality necessary at all for non-icc clocks?

Actually, if any consumers surviving on these proxy votes on other
targets as of today, removing the handoff logic could impact boot-up as
well. Hence, introduced the skip_clk_handoff flag to maintain backward
compatibility on other targets.

Thanks,
Imran

> I'm suspecting that this was just a port of the ancient msm-3.10
> logic where the (modified) clock framework had a handoff mechanism
> similar to today's sync_state, except the toning-down of these
> clocks was never added
> 
> Konrad

Re: [PATCH] clk: qcom: smd-rpm: Skip proxy votes on clocks for QCM2290

[Stephan Gerhold]

On Mon, Sep 21, 2026 at 10:53:40AM +0200, Konrad Dybcio wrote:
> On 9/16/26 1:05 PM, Imran Shaik wrote:
> > 
> > 
> > On 11-09-2026 02:36 pm, Konrad Dybcio wrote:
> >> On 9/10/26 3:44 PM, Imran Shaik wrote:
> >>> clk_smd_rpm_handoff() votes both active and sleep RPM resource states for
> >>> every clock, keeping them non-zero until a consumer takes over. If there
> >>> is no consumer, those clocks will remain active in the idle scenario as
> >>> well, and the sleep vote is never cleared, blocking XO shutdown.
> >>>
> >>> Introduce the skip_clks_handoff flag to handle this on QCM2290 clocks,
> >>> keeping other targets unaffected.
> >>>
> >>> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks")
> >>> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> >>> ---
> >>
> >> Are you booting with clk_ignore_unused?
> >>
> > 
> > No Konrad, clk_ignore_unused is not present.
> > 
> > Irrespective of clk_ignore_unused, the proxy votes are placed to RPM
> > during handoff. If no consumer takes over, those votes remain active,
> > keeping the resource ON in idle and preventing XO shutdown.
> 
> I re-read this and yeah you're right
> 
> Is the handoff functionality necessary at all for non-icc clocks?
> I'm suspecting that this was just a port of the ancient msm-3.10
> logic where the (modified) clock framework had a handoff mechanism
> similar to today's sync_state, except the toning-down of these
> clocks was never added
> 

The purpose of the handoff functionality is to sync the RPM vote state
with the Linux vote state. Since we don't have any "read status"
implemented, we do need to make a vote for every RPM clock to guarantee
it is in the expected state.

Simply skipping the proxy/handover votes does not fully solve the
problem, since you will still leave unused clocks enabled by the boot
firmware always-on.

I don't think we need to force on all clocks though, we could probably
also unconditionally send a "disable clock" after sync_state (once/if
the clock unused cleanup is actually managed by sync_state).

Thanks,
Stephan