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@xxxxxxxxxxx wrote:
>>> From: Udit Tiwari <quic_utiwari@xxxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxxx>
>>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202511160711.Q6ytYvlG-lkp@xxxxxxxxx/
>>> ---
>>> 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