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: <CAMuHMdXk91aKo6n97y8eGhfB3Ngu66mzqWhgwe3u86sPx2iaSg@mail.gmail.com>
Date: Tue, 18 Nov 2025 14:37:12 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Udit Tiwari <quic_utiwari@...cinc.com>, herbert@...dor.apana.org.au, 
	thara.gopinath@...il.com, davem@...emloft.net, 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>
Subject: Re: [PATCH v4] crypto: qce - Add runtime PM and interconnect
 bandwidth scaling support

On Tue, 18 Nov 2025 at 14:08, Konrad Dybcio
<konrad.dybcio@....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@...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)

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