linux-kernel - Re: [PATCH v2 1/3] soc: qcom: ice: Add OPP-based clock scaling support for ICE

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: <aTZzD6ujz0+mZD7j@hu-arakshit-hyd.qualcomm.com>
Date: Mon, 8 Dec 2025 12:11:19 +0530
From: Abhinaba Rakshit <abhinaba.rakshit@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Manivannan Sadhasivam <mani@...nel.org>,
        "James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Neeraj Soni <neeraj.soni@....qualcomm.com>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-scsi@...r.kernel.org
Subject: Re: [PATCH v2 1/3] soc: qcom: ice: Add OPP-based clock scaling
 support for ICE

On Fri, Nov 21, 2025 at 02:46:52PM +0100, Konrad Dybcio wrote:
> On 11/21/25 11:36 AM, Abhinaba Rakshit wrote:
> > Register optional operation-points-v2 table for ICE device
> > and aquire its minimum and maximum frequency during ICE
> > device probe.
> > 
> > Introduce clock scaling API qcom_ice_scale_clk which scale ICE
> > core clock if valid (non-zero) frequencies are obtained from
> > OPP-table. Zero min and max (default values) frequencies depicts
> > clock scaling is disabled.
> > 
> > When an ICE-device specific OPP table is available, use the PM OPP
> > framework to manage frequency scaling and maintain proper power-domain
> > constraints. For legacy targets without an ICE-device specific OPP table,
> > fall back to the standard clock framework APIs to set the frequency.
> 
> You can still set a frequency through OPP APIs if the table is empty
> (and one is always created even if devm_pm_opp_of_add_table() fails)
> 

We observed that when devm_pm_opp_of_add_table() returns -ENODEV (indicating
that no OPP table is defined in the devicetree), subsequent calls to APIs
like dev_pm_opp_set_rate() fail because the device does not have an OPP table
registered. As a result, the clock rate cannot be set using OPP-based helpers.

Here is an dmesg ice driver logs for lemans device without opp-table defined in its devicetree.
sh-5.2# dmesg | grep qcom-ice
[    7.316366] qcom-ice 87c8000.crypto: dev_pm_opp_set_rate: device's opp table doesn't exist
[    7.325596] qcom-ice 87c8000.crypto: Failed boosting the ICE clk to TURBO
[    7.333288] qcom-ice 87c8000.crypto: _find_key: OPP table not found (-19)
[    7.340968] qcom-ice 87c8000.crypto: Unable to find ICE core clock min freq
[    7.348832] qcom-ice 87c8000.crypto: _find_key: OPP table not found (-19)
[    7.356510] qcom-ice 87c8000.crypto: Unable to find ICE core clock max freq
[    7.364377] qcom-ice 87c8000.crypto: Found QC Inline Crypto Engine (ICE) v3.2.0
[    7.372594] qcom-ice 87c8000.crypto: QC ICE HWKM (Hardware Key Manager) version = 1

Additionally, on legacy targets where ICE does not exist as a separate device,
the OPP table is managed through the storage subsystem. In such cases, using
OPP APIs directly for ICE would not be appropriate because the OPP table may
also control other clocks, leading to unintended side effects.

> [...]
> 
> >  	/*
> >  	 * Legacy DT binding uses different clk names for each consumer,
> > -	 * so lets try those first. If none of those are a match, it means
> > -	 * the we only have one clock and it is part of the dedicated DT node.
> > -	 * Also, enable the clock before we check what HW version the driver
> > -	 * supports.
> > +	 * so lets try those first. Also get its corresponding clock index.
> > +	 */
> 
> I would argue *not* setting the rate on targets utilizing a binding without
> an OPP table for the ICE is probably a smart thing to do, because we may
> brownout the SoC this way

Understand the concern here.
However, our approach is to scale the ICE clock only when the storage subsystem scales
its own clocks. Since the storage driver already manages the associated power domain
and voltage adjustments (even for targets without opp-table for ICE) —which are shared
with ICE—this ensures that any frequency changes occur in a safe context. As a result,
the risk of a SoC brownout condition should be effectively mitigated.

> 
> Konrad