[PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[quic_utiwari]

From: Udit Tiwari <quic_utiwari@quicinc.com>

The Qualcomm Crypto Engine (QCE) driver currently lacks support for
runtime power management (PM) and interconnect bandwidth control.
As a result, the hardware remains fully powered and clocks stay
enabled even when the device is idle. Additionally, static
interconnect bandwidth votes are held indefinitely, preventing the
system from reclaiming unused bandwidth.

Address this by enabling runtime PM and dynamic interconnect
bandwidth scaling to allow the system to suspend the device when idle
and scale interconnect usage based on actual demand. Improve overall
system efficiency by reducing power usage and optimizing interconnect
resource allocation.

Make the following changes as part of this integration:

- Add support for pm_runtime APIs to manage device power state
  transitions.
- Implement runtime_suspend() and runtime_resume() callbacks to gate
  clocks and vote for interconnect bandwidth only when needed.
- Replace devm_clk_get_optional_enabled() with devm_pm_clk_create() +
  pm_clk_add() and let the PM core manage device clocks during runtime
  PM and system sleep.
- Register dev_pm_ops with the platform driver to hook into the PM
  framework.

Tested:

- Verify that ICC votes drop to zero after probe and upon request
  completion.
- Confirm that runtime PM usage count increments during active
  requests and decrements afterward.
- Observe that the device correctly enters the suspended state when
  idle.

Signed-off-by: Udit Tiwari <quic_utiwari@quicinc.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/
---
Changes in v4:
- Annotate runtime PM callbacks with __maybe_unused to silence W=1 warnings.
- Add Reported-by and Closes tags for kernel test robot warning.

Changes in v3:
- Switch from manual clock management to PM clock helpers
  (devm_pm_clk_create() + pm_clk_add()); no direct clk_* enable/disable
  in runtime callbacks.
- Replace pm_runtime_get_sync() with pm_runtime_resume_and_get(); remove
  pm_runtime_put_noidle() on error.
- Define PM ops using helper macros and reuse runtime callbacks for system
  sleep via pm_runtime_force_suspend()/pm_runtime_force_resume().
- Link to v2: https://lore.kernel.org/lkml/20250826110917.3383061-1-quic_utiwari@quicinc.com/

Changes in v2:
- Extend suspend/resume support to include runtime PM and ICC scaling.
- Register dev_pm_ops and implement runtime_suspend/resume callbacks.
- Link to v1: https://lore.kernel.org/lkml/20250606105808.2119280-1-quic_utiwari@quicinc.com/
---
 drivers/crypto/qce/core.c | 104 +++++++++++++++++++++++++++++++-------
 1 file changed, 87 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index b966f3365b7d..9cf8acedc325 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -12,6 +12,9 @@
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_clock.h>
 #include <linux/types.h>
 #include <crypto/algapi.h>
 #include <crypto/internal/hash.h>
@@ -90,13 +93,17 @@ static int qce_handle_queue(struct qce_device *qce,
 	struct crypto_async_request *async_req, *backlog;
 	int ret = 0, err;
 
+	ret = pm_runtime_resume_and_get(qce->dev);
+	if (ret < 0)
+		return ret;
+
 	scoped_guard(mutex, &qce->lock) {
 		if (req)
 			ret = crypto_enqueue_request(&qce->queue, req);
 
 		/* busy, do not dequeue request */
 		if (qce->req)
-			return ret;
+			goto qce_suspend;
 
 		backlog = crypto_get_backlog(&qce->queue);
 		async_req = crypto_dequeue_request(&qce->queue);
@@ -105,7 +112,7 @@ static int qce_handle_queue(struct qce_device *qce,
 	}
 
 	if (!async_req)
-		return ret;
+		goto qce_suspend;
 
 	if (backlog) {
 		scoped_guard(mutex, &qce->lock)
@@ -118,6 +125,8 @@ static int qce_handle_queue(struct qce_device *qce,
 		schedule_work(&qce->done_work);
 	}
 
+qce_suspend:
+	pm_runtime_put_autosuspend(qce->dev);
 	return ret;
 }
 
@@ -207,37 +216,48 @@ static int qce_crypto_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	qce->core = devm_clk_get_optional_enabled(qce->dev, "core");
-	if (IS_ERR(qce->core))
-		return PTR_ERR(qce->core);
+/* PM clock helpers: register device clocks */
+	ret = devm_pm_clk_create(dev);
+	if (ret)
+		return ret;
 
-	qce->iface = devm_clk_get_optional_enabled(qce->dev, "iface");
-	if (IS_ERR(qce->iface))
-		return PTR_ERR(qce->iface);
+	ret = pm_clk_add(dev, "core");
+	if (ret)
+		return ret;
 
-	qce->bus = devm_clk_get_optional_enabled(qce->dev, "bus");
-	if (IS_ERR(qce->bus))
-		return PTR_ERR(qce->bus);
+	ret = pm_clk_add(dev, "iface");
+	if (ret)
+		return ret;
 
-	qce->mem_path = devm_of_icc_get(qce->dev, "memory");
+	ret = pm_clk_add(dev, "bus");
+	if (ret)
+		return ret;
+
+	qce->mem_path = devm_of_icc_get(dev, "memory");
 	if (IS_ERR(qce->mem_path))
 		return PTR_ERR(qce->mem_path);
 
-	ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH);
+	/* Enable runtime PM after clocks and ICC are acquired */
+
+	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
 
-	ret = devm_qce_dma_request(qce->dev, &qce->dma);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
+	ret = devm_qce_dma_request(qce->dev, &qce->dma);
+	if (ret)
+		goto err_pm;
+
 	ret = qce_check_version(qce);
 	if (ret)
-		return ret;
+		goto err_pm;
 
 	ret = devm_mutex_init(qce->dev, &qce->lock);
 	if (ret)
-		return ret;
+		goto err_pm;
 
 	INIT_WORK(&qce->done_work, qce_req_done_work);
 	crypto_init_queue(&qce->queue, QCE_QUEUE_LENGTH);
@@ -245,9 +265,58 @@ static int qce_crypto_probe(struct platform_device *pdev)
 	qce->async_req_enqueue = qce_async_request_enqueue;
 	qce->async_req_done = qce_async_request_done;
 
-	return devm_qce_register_algs(qce);
+	ret = devm_qce_register_algs(qce);
+	if (ret)
+		goto err_pm;
+
+	/* Configure autosuspend after successful init */
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+
+err_pm:
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
+static int __maybe_unused qce_runtime_suspend(struct device *dev)
+{
+	struct qce_device *qce = dev_get_drvdata(dev);
+
+	icc_disable(qce->mem_path);
+
+	return 0;
+}
+
+static int __maybe_unused qce_runtime_resume(struct device *dev)
+{
+	struct qce_device *qce = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = icc_enable(qce->mem_path);
+	if (ret)
+		return ret;
+
+	ret = icc_set_bw(qce->mem_path, QCE_DEFAULT_MEM_BANDWIDTH, QCE_DEFAULT_MEM_BANDWIDTH);
+	if (ret)
+		goto err_icc;
+
+	return 0;
+
+err_icc:
+	icc_disable(qce->mem_path);
+	return ret;
+}
+
+static const struct dev_pm_ops qce_crypto_pm_ops = {
+	SET_RUNTIME_PM_OPS(qce_runtime_suspend, qce_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
+};
+
 static const struct of_device_id qce_crypto_of_match[] = {
 	{ .compatible = "qcom,crypto-v5.1", },
 	{ .compatible = "qcom,crypto-v5.4", },
@@ -261,6 +330,7 @@ static struct platform_driver qce_crypto_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = qce_crypto_of_match,
+		.pm = &qce_crypto_pm_ops,
 	},
 };
 module_platform_driver(qce_crypto_driver);
-- 
2.34.1

Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[Konrad Dybcio]

On 11/17/25 7:27 AM, quic_utiwari@quicinc.com wrote:
> From: Udit Tiwari <quic_utiwari@quicinc.com>
> 
> The Qualcomm Crypto Engine (QCE) driver currently lacks support for
> runtime power management (PM) and interconnect bandwidth control.
> As a result, the hardware remains fully powered and clocks stay
> enabled even when the device is idle. Additionally, static
> interconnect bandwidth votes are held indefinitely, preventing the
> system from reclaiming unused bandwidth.
> 
> Address this by enabling runtime PM and dynamic interconnect
> bandwidth scaling to allow the system to suspend the device when idle
> and scale interconnect usage based on actual demand. Improve overall
> system efficiency by reducing power usage and optimizing interconnect
> resource allocation.
> 
> Make the following changes as part of this integration:
> 
> - Add support for pm_runtime APIs to manage device power state
>   transitions.
> - Implement runtime_suspend() and runtime_resume() callbacks to gate
>   clocks and vote for interconnect bandwidth only when needed.
> - Replace devm_clk_get_optional_enabled() with devm_pm_clk_create() +
>   pm_clk_add() and let the PM core manage device clocks during runtime
>   PM and system sleep.
> - Register dev_pm_ops with the platform driver to hook into the PM
>   framework.
> 
> Tested:
> 
> - Verify that ICC votes drop to zero after probe and upon request
>   completion.
> - Confirm that runtime PM usage count increments during active
>   requests and decrements afterward.
> - Observe that the device correctly enters the suspended state when
>   idle.
> 
> Signed-off-by: Udit Tiwari <quic_utiwari@quicinc.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/
> ---
> Changes in v4:
> - Annotate runtime PM callbacks with __maybe_unused to silence W=1 warnings.
> - Add Reported-by and Closes tags for kernel test robot warning.

The tags are now saying

"The kernel test robot reported that the QCE driver does not have PM
operations and this patch fixes that."

Which doesn't have a reflection in reality..

[...]

> +/* PM clock helpers: register device clocks */

Missing \t

> +	ret = devm_pm_clk_create(dev);
> +	if (ret)
> +		return ret;
>  
> -	qce->iface = devm_clk_get_optional_enabled(qce->dev, "iface");
> -	if (IS_ERR(qce->iface))
> -		return PTR_ERR(qce->iface);
> +	ret = pm_clk_add(dev, "core");
> +	if (ret)
> +		return ret;
>  
> -	qce->bus = devm_clk_get_optional_enabled(qce->dev, "bus");
> -	if (IS_ERR(qce->bus))
> -		return PTR_ERR(qce->bus);
> +	ret = pm_clk_add(dev, "iface");
> +	if (ret)
> +		return ret;
>  
> -	qce->mem_path = devm_of_icc_get(qce->dev, "memory");
> +	ret = pm_clk_add(dev, "bus");
> +	if (ret)
> +		return ret;

Not all SoC have a pair of clocks. This is going to break those who don't

Konrad

Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[Udit Tiwari]

Hi Konrad,

Thanks for the review, please find my response inline.

On 11/17/2025 5:55 PM, Konrad Dybcio wrote:
> On 11/17/25 7:27 AM, quic_utiwari@quicinc.com wrote:
>> From: Udit Tiwari <quic_utiwari@quicinc.com>
>>
>> The Qualcomm Crypto Engine (QCE) driver currently lacks support for
>> runtime power management (PM) and interconnect bandwidth control.
>> As a result, the hardware remains fully powered and clocks stay
>> enabled even when the device is idle. Additionally, static
>> interconnect bandwidth votes are held indefinitely, preventing the
>> system from reclaiming unused bandwidth.
>>
>> Address this by enabling runtime PM and dynamic interconnect
>> bandwidth scaling to allow the system to suspend the device when idle
>> and scale interconnect usage based on actual demand. Improve overall
>> system efficiency by reducing power usage and optimizing interconnect
>> resource allocation.
>>
>> Make the following changes as part of this integration:
>>
>> - Add support for pm_runtime APIs to manage device power state
>>    transitions.
>> - Implement runtime_suspend() and runtime_resume() callbacks to gate
>>    clocks and vote for interconnect bandwidth only when needed.
>> - Replace devm_clk_get_optional_enabled() with devm_pm_clk_create() +
>>    pm_clk_add() and let the PM core manage device clocks during runtime
>>    PM and system sleep.
>> - Register dev_pm_ops with the platform driver to hook into the PM
>>    framework.
>>
>> Tested:
>>
>> - Verify that ICC votes drop to zero after probe and upon request
>>    completion.
>> - Confirm that runtime PM usage count increments during active
>>    requests and decrements afterward.
>> - Observe that the device correctly enters the suspended state when
>>    idle.
>>
>> Signed-off-by: Udit Tiwari <quic_utiwari@quicinc.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/
>> ---
>> Changes in v4:
>> - Annotate runtime PM callbacks with __maybe_unused to silence W=1 warnings.
>> - Add Reported-by and Closes tags for kernel test robot warning.
> 
> The tags are now saying
> 
> "The kernel test robot reported that the QCE driver does not have PM
> operations and this patch fixes that."
> 
> Which doesn't have a reflection in reality.
> 
> [...]
> 
I may be misunderstanding this comment but the bot flagged W=1 
unused-function warnings under !CONFIG_PM. In v4 I added __maybe_unused 
and Reported-by/Closes for that exact warning; I didn’t mean to imply 
the driver lacks PM ops.
>> +/* PM clock helpers: register device clocks */
> 
> Missing \t
> 
sure will fix it in v5 with other suggestions after my responses.

>> +	ret = devm_pm_clk_create(dev);
>> +	if (ret)
>> +		return ret;
>>   
>> -	qce->iface = devm_clk_get_optional_enabled(qce->dev, "iface");
>> -	if (IS_ERR(qce->iface))
>> -		return PTR_ERR(qce->iface);
>> +	ret = pm_clk_add(dev, "core");
>> +	if (ret)
>> +		return ret;
>>   
>> -	qce->bus = devm_clk_get_optional_enabled(qce->dev, "bus");
>> -	if (IS_ERR(qce->bus))
>> -		return PTR_ERR(qce->bus);
>> +	ret = pm_clk_add(dev, "iface");
>> +	if (ret)
>> +		return ret;
>>   
>> -	qce->mem_path = devm_of_icc_get(qce->dev, "memory");
>> +	ret = pm_clk_add(dev, "bus");
>> +	if (ret)
>> +		return ret;
> 
> Not all SoC have a pair of clocks. This is going to break those who don't
> 
> Konrad
On the concern that not all SoCs have "core/iface/bus" clocks and that 
this could break those platforms: i believe the PM clock helpers are 
tolerant of missing clock entries. If a clock is not described in DT, 
pm_clk_add will not cause the probe to fail, also on such platforms, 
runtime/system PM will simply not toggle that clock.

I’ve tested this on sc7280 where the QCE node has no clock entries, and 
the driver probes and operates correctly; runtime PM and interconnect 
behavior are as expected.

If you’d like this handled in a specific way, please let me know—I’m 
happy to implement that approach.

Udit

Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[Konrad Dybcio]

On 11/18/25 7:46 AM, Udit Tiwari wrote:
> Hi Konrad,
> 
> Thanks for the review, please find my response inline.
> 
> On 11/17/2025 5:55 PM, Konrad Dybcio wrote:
>> On 11/17/25 7:27 AM, quic_utiwari@quicinc.com wrote:
>>> From: Udit Tiwari <quic_utiwari@quicinc.com>
>>>
>>> The Qualcomm Crypto Engine (QCE) driver currently lacks support for
>>> runtime power management (PM) and interconnect bandwidth control.
>>> As a result, the hardware remains fully powered and clocks stay
>>> enabled even when the device is idle. Additionally, static
>>> interconnect bandwidth votes are held indefinitely, preventing the
>>> system from reclaiming unused bandwidth.

[...]

>>> Signed-off-by: Udit Tiwari <quic_utiwari@quicinc.com>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/
>>> ---
>>> Changes in v4:
>>> - Annotate runtime PM callbacks with __maybe_unused to silence W=1 warnings.
>>> - Add Reported-by and Closes tags for kernel test robot warning.
>>
>> The tags are now saying
>>
>> "The kernel test robot reported that the QCE driver does not have PM
>> operations and this patch fixes that."
>>
>> Which doesn't have a reflection in reality.
>>
>> [...]
>>
> I may be misunderstanding this comment but the bot flagged W=1 unused-function warnings under !CONFIG_PM. In v4 I added __maybe_unused and Reported-by/Closes for that exact warning; I didn’t mean to imply the driver lacks PM ops.

The case where the tags would apply would be:

A patch is submitted
The patch gets reviewed and applied to the tree
Kernel testing robot reports an issue
You send a fix-up patch (incl. robot's tags)

[...]

>>> +    ret = pm_clk_add(dev, "bus");
>>> +    if (ret)
>>> +        return ret;
>>
>> Not all SoC have a pair of clocks. This is going to break those who don't
>>
>> Konrad
> On the concern that not all SoCs have "core/iface/bus" clocks and that this could break those platforms: i believe the PM clock helpers are tolerant of missing clock entries. If a clock is not described in DT, pm_clk_add will not cause the probe to fail, also on such platforms, runtime/system PM will simply not toggle that clock.
> 
> I’ve tested this on sc7280 where the QCE node has no clock entries, and the driver probes and operates correctly; runtime PM and interconnect behavior are as expected.
> 
> If you’d like this handled in a specific way, please let me know—I’m happy to implement that approach.

No, you're right. I took a look at the pm_clk_add() call chain and noticed
that clk_get() (notably not _optional) is in there, but apparently its
retval is never propagated if things fail

(+RJW/Geert is that intended behavior?)

Konrad

Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect bandwidth scaling support

[Geert Uytterhoeven]

On Tue, 18 Nov 2025 at 14:08, Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
> On 11/18/25 7:46 AM, Udit Tiwari wrote:
> > Thanks for the review, please find my response inline.
> >
> > On 11/17/2025 5:55 PM, Konrad Dybcio wrote:
> >> On 11/17/25 7:27 AM, quic_utiwari@quicinc.com wrote:
> >>> From: Udit Tiwari <quic_utiwari@quicinc.com>
> >>>
> >>> The Qualcomm Crypto Engine (QCE) driver currently lacks support for
> >>> runtime power management (PM) and interconnect bandwidth control.
> >>> As a result, the hardware remains fully powered and clocks stay
> >>> enabled even when the device is idle. Additionally, static
> >>> interconnect bandwidth votes are held indefinitely, preventing the
> >>> system from reclaiming unused bandwidth.
>
> [...]
>
> >>> Signed-off-by: Udit Tiwari <quic_utiwari@quicinc.com>
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>> Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/
> >>> ---
> >>> Changes in v4:
> >>> - Annotate runtime PM callbacks with __maybe_unused to silence W=1 warnings.
> >>> - Add Reported-by and Closes tags for kernel test robot warning.
> >>
> >> The tags are now saying
> >>
> >> "The kernel test robot reported that the QCE driver does not have PM
> >> operations and this patch fixes that."
> >>
> >> Which doesn't have a reflection in reality.
> >>
> >> [...]
> >>
> > I may be misunderstanding this comment but the bot flagged W=1 unused-function warnings under !CONFIG_PM. In v4 I added __maybe_unused and Reported-by/Closes for that exact warning; I didn’t mean to imply the driver lacks PM ops.
>
> The case where the tags would apply would be:
>
> A patch is submitted
> The patch gets reviewed and applied to the tree
> Kernel testing robot reports an issue
> You send a fix-up patch (incl. robot's tags)

Exactly. The robot's report even says so:

    If you fix the issue in a separate patch/commit (i.e. not just a
new version of
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    the same patch/commit), kindly add following tags
    ^^^^^^^^^^^^^^^^^^^^^^
    | Reported-by: kernel test robot <lkp@intel.com>
    | Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@intel.com/

>
> [...]
>
> >>> +    ret = pm_clk_add(dev, "bus");
> >>> +    if (ret)
> >>> +        return ret;
> >>
> >> Not all SoC have a pair of clocks. This is going to break those who don't
> >>
> >> Konrad
> > On the concern that not all SoCs have "core/iface/bus" clocks and that this could break those platforms: i believe the PM clock helpers are tolerant of missing clock entries. If a clock is not described in DT, pm_clk_add will not cause the probe to fail, also on such platforms, runtime/system PM will simply not toggle that clock.
> >
> > I’ve tested this on sc7280 where the QCE node has no clock entries, and the driver probes and operates correctly; runtime PM and interconnect behavior are as expected.
> >
> > If you’d like this handled in a specific way, please let me know—I’m happy to implement that approach.
>
> No, you're right. I took a look at the pm_clk_add() call chain and noticed
> that clk_get() (notably not _optional) is in there, but apparently its
> retval is never propagated if things fail
>
> (+RJW/Geert is that intended behavior?)

I think most checking is done before calling pm_clk_add() or
pm_clk_add_clk(). of_pm_clk_add_clks() also calls of_clk_get() first.
So that looks intentional to me.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds