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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <283e7a7d-c69b-4931-8e54-d473f0209abe@quicinc.com>
Date: Tue, 18 Nov 2025 12:16:10 +0530
From: Udit Tiwari <quic_utiwari@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        <herbert@...dor.apana.org.au>, <thara.gopinath@...il.com>,
        <davem@...emloft.net>
CC: <linux-crypto@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <quic_neersoni@...cinc.com>,
        "kernel test
 robot" <lkp@...el.com>
Subject: Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect
 bandwidth scaling support

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@...cinc.com wrote:
>> From: Udit Tiwari <quic_utiwari@...cinc.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@...cinc.com>
>> Reported-by: kernel test robot <lkp@...el.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