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.
Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
drivers/soc/qcom/ice.c | 106 ++++++++++++++++++++++++++++++++++++++++++++-----
include/soc/qcom/ice.h | 1 +
2 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cadd21d6f96eb1799963a13db4b2b72..c352446707ab5e90e6baf159c86a0914ff4bfc53 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -16,6 +16,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
#include <linux/firmware/qcom/qcom_scm.h>
@@ -111,6 +112,14 @@ struct qcom_ice {
bool use_hwkm;
bool hwkm_init_complete;
u8 hwkm_version;
+ unsigned long max_freq;
+ unsigned long min_freq;
+ bool has_opp;
+};
+
+static const char * const legacy_ice_clk_names[] = {
+ "ice_core_clk",
+ "ice",
};
static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -549,10 +558,29 @@ int qcom_ice_import_key(struct qcom_ice *ice,
}
EXPORT_SYMBOL_GPL(qcom_ice_import_key);
+int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
+{
+ int ret = 0;
+
+ if (scale_up && ice->max_freq)
+ ret = (ice->has_opp) ? dev_pm_opp_set_rate(ice->dev, ice->max_freq)
+ : clk_set_rate(ice->core_clk, ice->max_freq);
+ else if (!scale_up && ice->min_freq)
+ ret = (ice->has_opp) ? dev_pm_opp_set_rate(ice->dev, ice->min_freq)
+ : clk_set_rate(ice->core_clk, ice->min_freq);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
+
static struct qcom_ice *qcom_ice_create(struct device *dev,
void __iomem *base)
{
struct qcom_ice *engine;
+ u32 clk_index;
+ struct dev_pm_opp *opp;
+ int err;
+ unsigned long rate;
if (!qcom_scm_is_available())
return ERR_PTR(-EPROBE_DEFER);
@@ -571,18 +599,74 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
/*
* 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.
+ */
+ for (int i = 0; i < ARRAY_SIZE(legacy_ice_clk_names); i++) {
+ engine->core_clk = devm_clk_get_optional(dev, legacy_ice_clk_names[i]);
+ if (!engine->core_clk)
+ continue;
+
+ if (IS_ERR(engine->core_clk))
+ return ERR_CAST(engine->core_clk);
+
+ /* Get the ICE clk index */
+ clk_index = of_property_match_string(dev->of_node,
+ "clock-names",
+ legacy_ice_clk_names[i]);
+ if (clk_index < 0)
+ return ERR_PTR(clk_index);
+
+ break;
+ }
+
+ /* When it does not match the legacy DT bindings
+ * it must have only one clock and it is part of
+ * decicated DT node
*/
- engine->core_clk = devm_clk_get_optional_enabled(dev, "ice_core_clk");
- if (!engine->core_clk)
- engine->core_clk = devm_clk_get_optional_enabled(dev, "ice");
- if (!engine->core_clk)
- engine->core_clk = devm_clk_get_enabled(dev, NULL);
- if (IS_ERR(engine->core_clk))
- return ERR_CAST(engine->core_clk);
+ if (!engine->core_clk) {
+ engine->core_clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(engine->core_clk))
+ return ERR_CAST(engine->core_clk);
+
+ /* OPP table is optional */
+ err = devm_pm_opp_of_add_table(dev);
+ if (err && err != -ENODEV) {
+ dev_err(dev, "Invalid OPP table in Device tree\n");
+ return ERR_PTR(err);
+ }
+ engine->has_opp = (err == 0);
+
+ /* Since, there is only one clock
+ * index can be set as 0
+ */
+ clk_index = 0;
+ }
+
+ /* Find the ICE core clock min and max frequencies */
+ rate = 0;
+ opp = dev_pm_opp_find_freq_ceil_indexed(dev, &rate, clk_index);
+ if (IS_ERR(opp)) {
+ dev_warn(dev, "Unable to find ICE core clock min freq\n");
+ } else {
+ engine->min_freq = rate;
+ dev_pm_opp_put(opp);
+ }
+
+ rate = ULONG_MAX;
+ opp = dev_pm_opp_find_freq_floor_indexed(dev, &rate, clk_index);
+ if (IS_ERR(opp)) {
+ dev_warn(dev, "Unable to find ICE core clock max freq\n");
+ } else {
+ engine->max_freq = rate;
+ dev_pm_opp_put(opp);
+ }
+
+ /* Enable the clock before we check what HW version the driver supports */
+ err = clk_prepare_enable(engine->core_clk);
+ if (err) {
+ dev_err(dev, "Failed to enable ICE core clock\n");
+ return ERR_PTR(err);
+ }
if (!qcom_ice_check_supported(engine))
return ERR_PTR(-EOPNOTSUPP);
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..b701ec9e062f70152f6dea8bf6c4637ab6ef20f1 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -30,5 +30,6 @@ int qcom_ice_import_key(struct qcom_ice *ice,
const u8 *raw_key, size_t raw_key_size,
u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
+int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up);
#endif /* __QCOM_ICE_H__ */
--
2.34.1On 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) [...] > /* > * 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 Konrad
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) > > [...] > > > /* > > * 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 Also, adding to the previous reply, if we completly remove clock scaling for any target we see a major degradation in IO throughput. On targets like Talos (target with legacy binding), we observed around ~50%-70% degradation of IO performance if we don't keep the ICE clock on par with storage clock and leaving it operate at default frequency. > > Konrad
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
On 12/8/25 7:41 AM, Abhinaba Rakshit wrote: > 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 Hm, perhaps I missed something.. > 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. This is a more convincing argument. But it also pushes me towards the opinion that it may not be worth supporting the older one on the grounds of the description being bogus.. > >> [...] >> >>> /* >>> * 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. Can you guarantee that this can be taken for granted for every SoC we've ever released? Konrad
© 2016 - 2026 Red Hat, Inc.