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@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.
> >
> > 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@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.
> > +/* 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