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] [thread-next>] [day] [month] [year] [list]
Message-ID: <306f6354-1502-4b9f-9a28-dcb7a882b367@oss.qualcomm.com>
Date: Tue, 18 Nov 2025 14:08:15 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Udit Tiwari <quic_utiwari@...cinc.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>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect
 bandwidth scaling support

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@...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.

[...]

>>> 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.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ