[PATCH v7 0/3] Enable ICE clock scaling

[PATCH v7 0/3] Enable ICE clock scaling

[Abhinaba Rakshit]

Introduce support for dynamic clock scaling of the ICE (Inline Crypto Engine)
using the OPP framework. During ICE device probe, the driver now attempts to
parse an optional OPP table from the ICE-specific device tree node for
DVFS-aware operations. API qcom_ice_scale_clk is exposed by ICE driver
and is invoked by UFS host controller driver in response to clock scaling
requests, ensuring coordination between ICE and host controller.

For MMC controllers that do not support clock scaling, the ICE clock frequency
is kept aligned with the MMC controller’s clock rate (TURBO) to ensure
consistent operation.

Dynamic clock scaling based on OPP tables enables better power-performance
trade-offs. By adjusting ICE clock frequencies according to workload and power
constraints, the system can achieve higher throughput when needed and
reduce power consumption during idle or low-load conditions.

The OPP table remains optional, absence of the table will not cause
probe failure. However, in the absence of an OPP table, ICE clocks will
remain at their default rates, which may limit performance under
high-load scenarios or prevent performance optimizations during idle periods.

Merge Order and Dependencies
============================

Patch 1/4 (dt-bindings) from the previous series
(https://lore.kernel.org/all/aaKt9PET6lVkBcif@gondor.apana.org.au/) has already
been applied. This v7 series therefore includes only the ICE driver and
UFS driver changes (previously patches 2–4).

Patch 1 is the change which should be merged first.

Patch 2 is dependent on patch 1 for the qcom_ice_scale_clk API to be available.
Patch 3 is dependent on patch 1 for opp-table parsing to be enabled in the
ICE driver.

Patch 2 and patch 3 are *not* dependent on each other. Once patch 1 is
merged, patch (2,3) can be applied independently by their respective
maintainers.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
Changes in v7:
- Replace the custom rounding flags with 'bool round_ceil' as suggested.
- Update the dev_info log-line.
- Dropped dt-bindings patch (already applied by in previous patchseries).
- Add merge order and dependencies as suggested.
- Link to v6: https://lore.kernel.org/r/20260219-enable-ufs-ice-clock-scaling-v6-0-0c5245117d45@oss.qualcomm.com

Changes in v6:
- Remove scale_up parameter from qcom_ice_scale_clk API.
- Remove having max_freq and min_freq as the checks for overclocking and underclocking is no-longer needed.
- UFS driver passes rounding flags depending on scale_up value.
- Ensure UFS driver does not fail devfreq requests if ICE OPP is not supported.
- Link to v5: https://lore.kernel.org/r/3ecb8d08-64cb-4fe1-bebd-1532dc5a86af@oss.qualcomm.com

Changes in v5:
- Update operating-points-v2 property in dtbindings as suggested.
- Fix comment styles.
- Add argument in qcom_ice_create to distinguish between legacy bindings and newer bindings.
- Ensure to drop votes in suspend and enable the last vote in resume.
- Link to v4: https://lore.kernel.org/r/20260128-enable-ufs-ice-clock-scaling-v4-0-260141e8fce6@oss.qualcomm.com

Changes in v4:
- Enable multiple frequency scaling based OPP-entries as suggested in v3 patchset.
- Include bindings change: https://lore.kernel.org/all/20260123-add-operating-points-v2-property-for-qcom-ice-bindings-v1-1-2155f7aacc28@oss.qualcomm.com/.
- Link to v3: https://lore.kernel.org/r/20260123-enable-ufs-ice-clock-scaling-v3-0-d0d8532abd98@oss.qualcomm.com

Changes in v3:
- Avoid clock scaling in case of legacy bindings as suggested.
- Use of_device_is_compatible to distinguish between legacy and non-legacy bindings.
- Link to v2: https://lore.kernel.org/r/20251121-enable-ufs-ice-clock-scaling-v2-0-66cb72998041@oss.qualcomm.com

Changes in v2:
- Use OPP-table instead of freq-table-hz for clock scaling.
- Enable clock scaling for legacy targets as well, by fetching frequencies from storage opp-table.
- Introduce has_opp variable in qcom_ice structure to keep track, if ICE instance has dedicated OPP-table registered.
- Combined the changes for patch-series <20251001-set-ice-clock-to-turbo-v1-1-7b802cf61dda@oss.qualcomm.com> as suggested.
- Link to v1: https://lore.kernel.org/r/20251001-enable-ufs-ice-clock-scaling-v1-0-ec956160b696@oss.qualcomm.com

---
Abhinaba Rakshit (3):
      soc: qcom: ice: Add OPP-based clock scaling support for ICE
      ufs: host: Add ICE clock scaling during UFS clock changes
      soc: qcom: ice: Set ICE clk to TURBO on probe

 drivers/soc/qcom/ice.c      | 89 +++++++++++++++++++++++++++++++++++++++++++--
 drivers/ufs/host/ufs-qcom.c | 19 +++++++++-
 include/soc/qcom/ice.h      |  2 +
 3 files changed, 106 insertions(+), 4 deletions(-)
---
base-commit: fe4d0dea039f2befb93f27569593ec209843b0f5
change-id: 20251120-enable-ufs-ice-clock-scaling-b063caf3e6f9

Best regards,
-- 
Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>

[PATCH v7 1/3] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

Register optional operation-points-v2 table for ICE device
during device probe.

Introduce clock scaling API qcom_ice_scale_clk which scale ICE
core clock based on the target frequency provided and if a valid
OPP-table is registered. Use round_ceil passed to decide on the
rounding of the clock freq against OPP-table. Disable clock scaling
if OPP-table is not registered.

When an ICE-device specific OPP table is available, use the PM OPP
framework to manage frequency scaling and maintain proper power-domain
constraints.

Also, ensure to drop the votes in suspend to prevent power/thermal
retention. Subsequently restore the frequency in resume from
core_clk_freq which stores the last ICE core clock operating frequency.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
 drivers/soc/qcom/ice.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/soc/qcom/ice.h |  2 ++
 2 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cadd21d6f96eb1799963a13db4b2b72..7976a18d9a4cda1ad6b62b66ce011e244d0f6856 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,8 @@ struct qcom_ice {
 	bool use_hwkm;
 	bool hwkm_init_complete;
 	u8 hwkm_version;
+	unsigned long core_clk_freq;
+	bool has_opp;
 };
 
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -310,6 +313,10 @@ int qcom_ice_resume(struct qcom_ice *ice)
 	struct device *dev = ice->dev;
 	int err;
 
+	/* Restore the ICE core clk freq */
+	if (ice->has_opp && ice->core_clk_freq)
+		dev_pm_opp_set_rate(ice->dev, ice->core_clk_freq);
+
 	err = clk_prepare_enable(ice->core_clk);
 	if (err) {
 		dev_err(dev, "failed to enable core clock (%d)\n",
@@ -324,6 +331,11 @@ EXPORT_SYMBOL_GPL(qcom_ice_resume);
 int qcom_ice_suspend(struct qcom_ice *ice)
 {
 	clk_disable_unprepare(ice->core_clk);
+
+	/* Drop the clock votes while suspend */
+	if (ice->has_opp)
+		dev_pm_opp_set_rate(ice->dev, 0);
+
 	ice->hwkm_init_complete = false;
 
 	return 0;
@@ -549,10 +561,54 @@ int qcom_ice_import_key(struct qcom_ice *ice,
 }
 EXPORT_SYMBOL_GPL(qcom_ice_import_key);
 
+/**
+ * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
+ * @ice: ICE driver data
+ * @target_freq: requested frequency in Hz
+ * @round_ceil: when true, selects nearest freq >= @target_freq;
+ *              otherwise, selects nearest freq <= @target_freq
+ *
+ * Selects an OPP frequency based on @target_freq and the rounding direction
+ * specified by @round_ceil, then programs it using dev_pm_opp_set_rate(),
+ * including any voltage or power-domain transitions handled by the OPP
+ * framework. Updates ice->core_clk_freq on success.
+ *
+ * Return: 0 on success; -EOPNOTSUPP if no OPP table; -EINVAL in-case of
+ *         incorrect flags; or error from dev_pm_opp_set_rate()/OPP lookup.
+ */
+int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
+		       bool round_ceil)
+{
+	unsigned long ice_freq = target_freq;
+	struct dev_pm_opp *opp;
+	int ret;
+
+	if (!ice->has_opp)
+		return -EOPNOTSUPP;
+
+	if (round_ceil)
+		opp = dev_pm_opp_find_freq_ceil(ice->dev, &ice_freq);
+	else
+		opp = dev_pm_opp_find_freq_floor(ice->dev, &ice_freq);
+
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+	dev_pm_opp_put(opp);
+
+	ret = dev_pm_opp_set_rate(ice->dev, ice_freq);
+	if (!ret)
+		ice->core_clk_freq = ice_freq;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
+
 static struct qcom_ice *qcom_ice_create(struct device *dev,
-					void __iomem *base)
+					void __iomem *base,
+					bool is_legacy_binding)
 {
 	struct qcom_ice *engine;
+	int err;
 
 	if (!qcom_scm_is_available())
 		return ERR_PTR(-EPROBE_DEFER);
@@ -584,6 +640,26 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 	if (IS_ERR(engine->core_clk))
 		return ERR_CAST(engine->core_clk);
 
+	/*
+	 * Register the OPP table only when ICE is described as a standalone
+	 * device node. Older platforms place ICE inside the storage controller
+	 * node, so they don't need an OPP table here, as they are handled in
+	 * storage controller.
+	 */
+	if (!is_legacy_binding) {
+		/* 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);
+
+		if (!engine->has_opp)
+			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
+	}
+
+	engine->core_clk_freq = clk_get_rate(engine->core_clk);
 	if (!qcom_ice_check_supported(engine))
 		return ERR_PTR(-EOPNOTSUPP);
 
@@ -628,7 +704,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 			return ERR_CAST(base);
 
 		/* create ICE instance using consumer dev */
-		return qcom_ice_create(&pdev->dev, base);
+		return qcom_ice_create(&pdev->dev, base, true);
 	}
 
 	/*
@@ -725,7 +801,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 	}
 
-	engine = qcom_ice_create(&pdev->dev, base);
+	engine = qcom_ice_create(&pdev->dev, base, false);
 	if (IS_ERR(engine))
 		return PTR_ERR(engine);
 
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..4eb58a264d416e71228ed4b13e7f53c549261fdc 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -30,5 +30,7 @@ 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, unsigned long target_freq,
+		       bool round_ceil);
 
 #endif /* __QCOM_ICE_H__ */

-- 
2.34.1

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

[Harshal Dev]

Hi Abhinaba,

On 3/2/2026 4:19 PM, Abhinaba Rakshit wrote:
> Register optional operation-points-v2 table for ICE device
> during device probe.
> 
> Introduce clock scaling API qcom_ice_scale_clk which scale ICE
> core clock based on the target frequency provided and if a valid
> OPP-table is registered. Use round_ceil passed to decide on the
> rounding of the clock freq against OPP-table. Disable clock scaling
> if OPP-table is not registered.
> 
> When an ICE-device specific OPP table is available, use the PM OPP
> framework to manage frequency scaling and maintain proper power-domain
> constraints.
> 
> Also, ensure to drop the votes in suspend to prevent power/thermal
> retention. Subsequently restore the frequency in resume from
> core_clk_freq which stores the last ICE core clock operating frequency.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/ice.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  include/soc/qcom/ice.h |  2 ++
>  2 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index b203bc685cadd21d6f96eb1799963a13db4b2b72..7976a18d9a4cda1ad6b62b66ce011e244d0f6856 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,8 @@ struct qcom_ice {
>  	bool use_hwkm;
>  	bool hwkm_init_complete;
>  	u8 hwkm_version;
> +	unsigned long core_clk_freq;
> +	bool has_opp;
>  };
>  
>  static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -310,6 +313,10 @@ int qcom_ice_resume(struct qcom_ice *ice)
>  	struct device *dev = ice->dev;
>  	int err;
>  
> +	/* Restore the ICE core clk freq */
> +	if (ice->has_opp && ice->core_clk_freq)
> +		dev_pm_opp_set_rate(ice->dev, ice->core_clk_freq);
> +
>  	err = clk_prepare_enable(ice->core_clk);
>  	if (err) {
>  		dev_err(dev, "failed to enable core clock (%d)\n",
> @@ -324,6 +331,11 @@ EXPORT_SYMBOL_GPL(qcom_ice_resume);
>  int qcom_ice_suspend(struct qcom_ice *ice)
>  {
>  	clk_disable_unprepare(ice->core_clk);
> +
> +	/* Drop the clock votes while suspend */
> +	if (ice->has_opp)
> +		dev_pm_opp_set_rate(ice->dev, 0);
> +
>  	ice->hwkm_init_complete = false;
>  
>  	return 0;
> @@ -549,10 +561,54 @@ int qcom_ice_import_key(struct qcom_ice *ice,
>  }
>  EXPORT_SYMBOL_GPL(qcom_ice_import_key);
>  
> +/**
> + * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
> + * @ice: ICE driver data
> + * @target_freq: requested frequency in Hz
> + * @round_ceil: when true, selects nearest freq >= @target_freq;
> + *              otherwise, selects nearest freq <= @target_freq
> + *
> + * Selects an OPP frequency based on @target_freq and the rounding direction
> + * specified by @round_ceil, then programs it using dev_pm_opp_set_rate(),
> + * including any voltage or power-domain transitions handled by the OPP
> + * framework. Updates ice->core_clk_freq on success.
> + *
> + * Return: 0 on success; -EOPNOTSUPP if no OPP table; -EINVAL in-case of
> + *         incorrect flags; or error from dev_pm_opp_set_rate()/OPP lookup.
> + */
> +int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
> +		       bool round_ceil)

Any particular reason for choosing round_ceil? Using round_floor would have
saved the need for caller to pass negation of scale_up.

> +{
> +	unsigned long ice_freq = target_freq;
> +	struct dev_pm_opp *opp;
> +	int ret;
> +
> +	if (!ice->has_opp)
> +		return -EOPNOTSUPP;
> +
> +	if (round_ceil)
> +		opp = dev_pm_opp_find_freq_ceil(ice->dev, &ice_freq);
> +	else
> +		opp = dev_pm_opp_find_freq_floor(ice->dev, &ice_freq);
> +
> +	if (IS_ERR(opp))
> +		return PTR_ERR(opp);
> +	dev_pm_opp_put(opp);
> +
> +	ret = dev_pm_opp_set_rate(ice->dev, ice_freq);
> +	if (!ret)
> +		ice->core_clk_freq = ice_freq;

Nit: Follow same error handling pattern everywhere in the driver.
	if (ret) {
		dev_err(dev, "error");
		return ret;
	}

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> +
>  static struct qcom_ice *qcom_ice_create(struct device *dev,
> -					void __iomem *base)
> +					void __iomem *base,
> +					bool is_legacy_binding)

You don't need to introduce is_legacy_binding.

Since you only need to add the OPP table when this function gets called from ICE probe,
you should not touch this function. Instead, you should call devm_pm_opp_of_add_table()
in ICE probe before calling qcom_ice_create() then once qcom_ice_create() is success, you
can store the clk rate in the returned qcom_ice *engine ptr by calling clk_get_rate().

>  {
>  	struct qcom_ice *engine;
> +	int err;
>  
>  	if (!qcom_scm_is_available())
>  		return ERR_PTR(-EPROBE_DEFER);
> @@ -584,6 +640,26 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  	if (IS_ERR(engine->core_clk))
>  		return ERR_CAST(engine->core_clk);
>  
> +	/*
> +	 * Register the OPP table only when ICE is described as a standalone
> +	 * device node. Older platforms place ICE inside the storage controller
> +	 * node, so they don't need an OPP table here, as they are handled in
> +	 * storage controller.
> +	 */
> +	if (!is_legacy_binding) {
> +		/* 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);

Let's keep it readable and simple. engine->has_opps = true; here and false in error handle above.

> +
> +		if (!engine->has_opp)
> +			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");

Since OPP table is optional, I don't understand the reason for requesting the user to add one.

> +	}
> +
> +	engine->core_clk_freq = clk_get_rate(engine->core_clk);
>  	if (!qcom_ice_check_supported(engine))
>  		return ERR_PTR(-EOPNOTSUPP);
>  
> @@ -628,7 +704,7 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
>  			return ERR_CAST(base);
>  
>  		/* create ICE instance using consumer dev */
> -		return qcom_ice_create(&pdev->dev, base);
> +		return qcom_ice_create(&pdev->dev, base, true);
>  	}
>  
>  	/*
> @@ -725,7 +801,7 @@ static int qcom_ice_probe(struct platform_device *pdev)
>  		return PTR_ERR(base);
>  	}
>  
> -	engine = qcom_ice_create(&pdev->dev, base);
> +	engine = qcom_ice_create(&pdev->dev, base, false);

Change not needed as per above comment.

Regards,
Harshal

>  	if (IS_ERR(engine))
>  		return PTR_ERR(engine);
>  
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..4eb58a264d416e71228ed4b13e7f53c549261fdc 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -30,5 +30,7 @@ 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, unsigned long target_freq,
> +		       bool round_ceil);
>  
>  #endif /* __QCOM_ICE_H__ */
> 

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

[Abhinaba Rakshit]

On Mon, Mar 30, 2026 at 08:09:35PM +0530, Harshal Dev wrote:
> > +/**
> > + * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
> > + * @ice: ICE driver data
> > + * @target_freq: requested frequency in Hz
> > + * @round_ceil: when true, selects nearest freq >= @target_freq;
> > + *              otherwise, selects nearest freq <= @target_freq
> > + *
> > + * Selects an OPP frequency based on @target_freq and the rounding direction
> > + * specified by @round_ceil, then programs it using dev_pm_opp_set_rate(),
> > + * including any voltage or power-domain transitions handled by the OPP
> > + * framework. Updates ice->core_clk_freq on success.
> > + *
> > + * Return: 0 on success; -EOPNOTSUPP if no OPP table; -EINVAL in-case of
> > + *         incorrect flags; or error from dev_pm_opp_set_rate()/OPP lookup.
> > + */
> > +int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
> > +		       bool round_ceil)
> 
> Any particular reason for choosing round_ceil? Using round_floor would have
> saved the need for caller to pass negation of scale_up.

There isn’t a strong technical reason for choosing round_ceil specifically.
The choice was mainly influenced by the earlier discussion here:
https://lore.kernel.org/all/15495f8a-37b0-4768-9ee1-05fd6c70034e@oss.qualcomm.com/
 
Also, this helper isn’t necessarily limited to the current caller.
We might see additional users in the future where the semantics align more
naturally with flags like scale_down, which map cleanly to a round_ceil‑style selection.
That said, I agree that using round_floor could simplify the current callsite by
avoiding the negation of scale_up.

I don’t have a strong objection to switching it if you feel that would be
more cleaner for now.
 
> > +{
> > +	unsigned long ice_freq = target_freq;
> > +	struct dev_pm_opp *opp;
> > +	int ret;
> > +
> > +	if (!ice->has_opp)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (round_ceil)
> > +		opp = dev_pm_opp_find_freq_ceil(ice->dev, &ice_freq);
> > +	else
> > +		opp = dev_pm_opp_find_freq_floor(ice->dev, &ice_freq);
> > +
> > +	if (IS_ERR(opp))
> > +		return PTR_ERR(opp);
> > +	dev_pm_opp_put(opp);
> > +
> > +	ret = dev_pm_opp_set_rate(ice->dev, ice_freq);
> > +	if (!ret)
> > +		ice->core_clk_freq = ice_freq;
> 
> Nit: Follow same error handling pattern everywhere in the driver.
> 	if (ret) {
> 		dev_err(dev, "error");
> 		return ret;
> 	}

Ack

> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> > +
> >  static struct qcom_ice *qcom_ice_create(struct device *dev,
> > -					void __iomem *base)
> > +					void __iomem *base,
> > +					bool is_legacy_binding)
> 
> You don't need to introduce is_legacy_binding.
> 
> Since you only need to add the OPP table when this function gets called from ICE probe,
> you should not touch this function. Instead, you should call devm_pm_opp_of_add_table()
> in ICE probe before calling qcom_ice_create() then once qcom_ice_create() is success, you
> can store the clk rate in the returned qcom_ice *engine ptr by calling clk_get_rate().

This was added as part of the review comment from Krzysztof:
https://lore.kernel.org/all/20260128-daft-seriema-of-promotion-c50eb5@quoll/
 
While I agree moving this to qcom_ice_probe would be more cleaner without needing
to change the API, most of our initializing code for driver by parsing the DT node
happens through qcom_ice_create, which keeps qcom_ice_probe much simpler.
Please let me know, if you think otherwise. 
 
Also, I don't see any reason for moving the clk_get_rate() logic to qcom_ice_probe
though as it will not be set on legacy targets in that case.

> >  {
> >  	struct qcom_ice *engine;
> > +	int err;
> >  
> >  	if (!qcom_scm_is_available())
> >  		return ERR_PTR(-EPROBE_DEFER);
> > @@ -584,6 +640,26 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >  	if (IS_ERR(engine->core_clk))
> >  		return ERR_CAST(engine->core_clk);
> >  
> > +	/*
> > +	 * Register the OPP table only when ICE is described as a standalone
> > +	 * device node. Older platforms place ICE inside the storage controller
> > +	 * node, so they don't need an OPP table here, as they are handled in
> > +	 * storage controller.
> > +	 */
> > +	if (!is_legacy_binding) {
> > +		/* 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);
> 
> Let's keep it readable and simple. engine->has_opps = true; here and false in error handle above.

Well there are 3 cases to it:

1. err == 0 which implies devm_pm_opp_of_add_table is successful and we can set engine->has_opp =true.
2. err == -ENODEV which implies there is no opp table in the DT node.
   In that case, we don't fail the driver simply go ahead and log in the check below.
   This is done since OPP-table is optional.
3. err == any other error code. Something very wrong happened with devm_pm_opp_of_add_table
   and driver should fail.

Hence, we have the condition (err == 0) for setting has_opp flag. 

> > +
> > +		if (!engine->has_opp)
> > +			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
> 
> Since OPP table is optional, I don't understand the reason for requesting the user to add one.

This was added as part of the review comment from Konrad:
https://lore.kernel.org/all/15495f8a-37b0-4768-9ee1-05fd6c70034e@oss.qualcomm.com/

OPP-table are mostly optional across kernel and I guess, this warning helps developers
to go ahead and update with the OPP-table.
 
Abhinaba Rakshit

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

[Harshal Dev]

On 4/3/2026 7:47 PM, Abhinaba Rakshit wrote:
> On Mon, Mar 30, 2026 at 08:09:35PM +0530, Harshal Dev wrote:
>>> +/**
>>> + * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
>>> + * @ice: ICE driver data
>>> + * @target_freq: requested frequency in Hz
>>> + * @round_ceil: when true, selects nearest freq >= @target_freq;
>>> + *              otherwise, selects nearest freq <= @target_freq
>>> + *
>>> + * Selects an OPP frequency based on @target_freq and the rounding direction
>>> + * specified by @round_ceil, then programs it using dev_pm_opp_set_rate(),
>>> + * including any voltage or power-domain transitions handled by the OPP
>>> + * framework. Updates ice->core_clk_freq on success.
>>> + *
>>> + * Return: 0 on success; -EOPNOTSUPP if no OPP table; -EINVAL in-case of
>>> + *         incorrect flags; or error from dev_pm_opp_set_rate()/OPP lookup.
>>> + */
>>> +int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
>>> +		       bool round_ceil)
>>
>> Any particular reason for choosing round_ceil? Using round_floor would have
>> saved the need for caller to pass negation of scale_up.
> 
> There isn’t a strong technical reason for choosing round_ceil specifically.
> The choice was mainly influenced by the earlier discussion here:
> https://lore.kernel.org/all/15495f8a-37b0-4768-9ee1-05fd6c70034e@oss.qualcomm.com/
>  
> Also, this helper isn’t necessarily limited to the current caller.
> We might see additional users in the future where the semantics align more
> naturally with flags like scale_down, which map cleanly to a round_ceil‑style selection.
> That said, I agree that using round_floor could simplify the current callsite by
> avoiding the negation of scale_up.
> 
> I don’t have a strong objection to switching it if you feel that would be
> more cleaner for now.
>

No issues, you can choose to do it if you spin a v8 of this patch series.
  
>>> +{
>>> +	unsigned long ice_freq = target_freq;
>>> +	struct dev_pm_opp *opp;
>>> +	int ret;
>>> +
>>> +	if (!ice->has_opp)
>>> +		return -EOPNOTSUPP;
>>> +

[...]

>>> +
>>>  static struct qcom_ice *qcom_ice_create(struct device *dev,
>>> -					void __iomem *base)
>>> +					void __iomem *base,
>>> +					bool is_legacy_binding)
>>
>> You don't need to introduce is_legacy_binding.
>>
>> Since you only need to add the OPP table when this function gets called from ICE probe,
>> you should not touch this function. Instead, you should call devm_pm_opp_of_add_table()
>> in ICE probe before calling qcom_ice_create() then once qcom_ice_create() is success, you
>> can store the clk rate in the returned qcom_ice *engine ptr by calling clk_get_rate().
> 
> This was added as part of the review comment from Krzysztof:
> https://lore.kernel.org/all/20260128-daft-seriema-of-promotion-c50eb5@quoll/
>  
> While I agree moving this to qcom_ice_probe would be more cleaner without needing
> to change the API, most of our initializing code for driver by parsing the DT node
> happens through qcom_ice_create, which keeps qcom_ice_probe much simpler.
> Please let me know, if you think otherwise. 
>

Seems like a suggestion from Krzysztof and not something based on strong opinion. Again,
you can choose to do this if you spin a v8, I feel it's cleaner.
  
> Also, I don't see any reason for moving the clk_get_rate() logic to qcom_ice_probe
> though as it will not be set on legacy targets in that case.

I thought only new DT nodes will be specifying the OPP table requiring us to store the
clk rate and restore later. If legacy DT nodes also possess the OPP table, then ignore
this comment.

> 
>>>  {
>>>  	struct qcom_ice *engine;
>>> +	int err;
>>>  
>>>  	if (!qcom_scm_is_available())
>>>  		return ERR_PTR(-EPROBE_DEFER);
>>> @@ -584,6 +640,26 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>>>  	if (IS_ERR(engine->core_clk))
>>>  		return ERR_CAST(engine->core_clk);
>>>  
>>> +	/*
>>> +	 * Register the OPP table only when ICE is described as a standalone
>>> +	 * device node. Older platforms place ICE inside the storage controller
>>> +	 * node, so they don't need an OPP table here, as they are handled in
>>> +	 * storage controller.
>>> +	 */
>>> +	if (!is_legacy_binding) {
>>> +		/* 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);
>>
>> Let's keep it readable and simple. engine->has_opps = true; here and false in error handle above.
> 
> Well there are 3 cases to it:
> 
> 1. err == 0 which implies devm_pm_opp_of_add_table is successful and we can set engine->has_opp =true.
> 2. err == -ENODEV which implies there is no opp table in the DT node.
>    In that case, we don't fail the driver simply go ahead and log in the check below.
>    This is done since OPP-table is optional.
> 3. err == any other error code. Something very wrong happened with devm_pm_opp_of_add_table
>    and driver should fail.
> 
> Hence, we have the condition (err == 0) for setting has_opp flag.

My suggestion is you either explain this in concise comments or simplify the assignment of has_opp
to make it obvious.

Regards,
Harshal

>  
> Abhinaba Rakshit

[PATCH v7 2/3] ufs: host: Add ICE clock scaling during UFS clock changes

[Abhinaba Rakshit]

Implement ICE (Inline Crypto Engine) clock scaling in sync with
UFS controller clock scaling. This ensures that the ICE operates at
an appropriate frequency when the UFS clocks are scaled up or down,
improving performance and maintaining stability for crypto operations.

For scale_up operation ensure to pass ~round_ceil (round_floor)
and vice-versa for scale_down operations.

Incase of OPP scaling is not supported by ICE, ensure to not prevent
devfreq for UFS, as ICE OPP-table is optional.

Acked-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
 drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 8d119b3223cbdaa3297d2beabced0962a1a847d5..776444f46fe5f00f947e4b0b4dfe6d64e2ad2150 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -305,6 +305,15 @@ static int ufs_qcom_ice_prepare_key(struct blk_crypto_profile *profile,
 	return qcom_ice_prepare_key(host->ice, lt_key, lt_key_size, eph_key);
 }
 
+static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
+				  bool round_ceil)
+{
+	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
+		return qcom_ice_scale_clk(host->ice, target_freq, round_ceil);
+
+	return 0;
+}
+
 static const struct blk_crypto_ll_ops ufs_qcom_crypto_ops = {
 	.keyslot_program	= ufs_qcom_ice_keyslot_program,
 	.keyslot_evict		= ufs_qcom_ice_keyslot_evict,
@@ -339,6 +348,12 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
 {
 }
 
+static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
+				  bool round_ceil)
+{
+	return 0;
+}
+
 #endif
 
 static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
@@ -1646,8 +1661,10 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
 		else
 			err = ufs_qcom_clk_scale_down_post_change(hba, target_freq);
 
+		if (!err)
+			err = ufs_qcom_ice_scale_clk(host, target_freq, !scale_up);
 
-		if (err) {
+		if (err && err != -EOPNOTSUPP) {
 			ufshcd_uic_hibern8_exit(hba);
 			return err;
 		}

-- 
2.34.1

Re: [PATCH v7 2/3] ufs: host: Add ICE clock scaling during UFS clock changes

[Harshal Dev]

Hi Abhinaba,

On 3/2/2026 4:19 PM, Abhinaba Rakshit wrote:
> Implement ICE (Inline Crypto Engine) clock scaling in sync with
> UFS controller clock scaling. This ensures that the ICE operates at
> an appropriate frequency when the UFS clocks are scaled up or down,
> improving performance and maintaining stability for crypto operations.
> 
> For scale_up operation ensure to pass ~round_ceil (round_floor)
> and vice-versa for scale_down operations.
> 
> Incase of OPP scaling is not supported by ICE, ensure to not prevent
> devfreq for UFS, as ICE OPP-table is optional.
> 
> Acked-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 8d119b3223cbdaa3297d2beabced0962a1a847d5..776444f46fe5f00f947e4b0b4dfe6d64e2ad2150 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -305,6 +305,15 @@ static int ufs_qcom_ice_prepare_key(struct blk_crypto_profile *profile,
>  	return qcom_ice_prepare_key(host->ice, lt_key, lt_key_size, eph_key);
>  }
>  
> +static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
> +				  bool round_ceil)
> +{
> +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> +		return qcom_ice_scale_clk(host->ice, target_freq, round_ceil);
> +
> +	return 0;
> +}
> +
>  static const struct blk_crypto_ll_ops ufs_qcom_crypto_ops = {
>  	.keyslot_program	= ufs_qcom_ice_keyslot_program,
>  	.keyslot_evict		= ufs_qcom_ice_keyslot_evict,
> @@ -339,6 +348,12 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
>  {
>  }
>  
> +static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
> +				  bool round_ceil)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> @@ -1646,8 +1661,10 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
>  		else
>  			err = ufs_qcom_clk_scale_down_post_change(hba, target_freq);
>  
> +		if (!err)
> +			err = ufs_qcom_ice_scale_clk(host, target_freq, !scale_up);
>  
> -		if (err) {
> +		if (err && err != -EOPNOTSUPP) {

Using -EOPNOTSUPP here works fine for now. But anyone touching any of the lower APIs called by
ufs_qcom_clk_scale_up/down_post_change() needs to ensure they don't return -EOPNOTSUPP, otherwise
hibernate exit will be skipped. So this carries a minor risk of breaking.

Since regardless of whether ufs_qcom_clk_scale_up/down_post_change() fails or ufs_qcom_ice_scale_clk()
fails, we exit from hibernate and return from this function, I suggest you handle the error for ice_scale
separately.

>  			ufshcd_uic_hibern8_exit(hba);
>  			return err;
>  		}
> 

Add the call to ufs_qcom_ice_scale_clk() along with error handle here, and let the above error handle
remain untouched.

		err = ufs_qcom_ice_scale_clk(host, target_freq, !scale_up);
		if (err && err != -EOPNOTSUPP) {
			ufshcd_uic_hibern8_exit(hba);
  			return err;
  		}

Regards,
Harshal

Re: [PATCH v7 2/3] ufs: host: Add ICE clock scaling during UFS clock changes

[Abhinaba Rakshit]

On Mon, Mar 30, 2026 at 08:09:51PM +0530, Harshal Dev wrote:
> >  drivers/ufs/host/ufs-qcom.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 8d119b3223cbdaa3297d2beabced0962a1a847d5..776444f46fe5f00f947e4b0b4dfe6d64e2ad2150 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -305,6 +305,15 @@ static int ufs_qcom_ice_prepare_key(struct blk_crypto_profile *profile,
> >  	return qcom_ice_prepare_key(host->ice, lt_key, lt_key_size, eph_key);
> >  }
> >  
> > +static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
> > +				  bool round_ceil)
> > +{
> > +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> > +		return qcom_ice_scale_clk(host->ice, target_freq, round_ceil);
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct blk_crypto_ll_ops ufs_qcom_crypto_ops = {
> >  	.keyslot_program	= ufs_qcom_ice_keyslot_program,
> >  	.keyslot_evict		= ufs_qcom_ice_keyslot_evict,
> > @@ -339,6 +348,12 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> >  {
> >  }
> >  
> > +static int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, unsigned long target_freq,
> > +				  bool round_ceil)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif
> >  
> >  static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> > @@ -1646,8 +1661,10 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, bool scale_up,
> >  		else
> >  			err = ufs_qcom_clk_scale_down_post_change(hba, target_freq);
> >  
> > +		if (!err)
> > +			err = ufs_qcom_ice_scale_clk(host, target_freq, !scale_up);
> >  
> > -		if (err) {
> > +		if (err && err != -EOPNOTSUPP) {
> 
> Using -EOPNOTSUPP here works fine for now. But anyone touching any of the lower APIs called by
> ufs_qcom_clk_scale_up/down_post_change() needs to ensure they don't return -EOPNOTSUPP, otherwise
> hibernate exit will be skipped. So this carries a minor risk of breaking.
> 
> Since regardless of whether ufs_qcom_clk_scale_up/down_post_change() fails or ufs_qcom_ice_scale_clk()
> fails, we exit from hibernate and return from this function, I suggest you handle the error for ice_scale
> separately.
> 
> >  			ufshcd_uic_hibern8_exit(hba);
> >  			return err;
> >  		}
> > 
> 
> Add the call to ufs_qcom_ice_scale_clk() along with error handle here, and let the above error handle
> remain untouched.
> 
> 		err = ufs_qcom_ice_scale_clk(host, target_freq, !scale_up);
> 		if (err && err != -EOPNOTSUPP) {
> 			ufshcd_uic_hibern8_exit(hba);
>   			return err;
>   		}
> 

Right, I did think of this later.
Yes, this needs change.
Ack will update in the v8 patchset.
Thanks.

Abhinaba Rakshit

[PATCH v7 3/3] soc: qcom: ice: Set ICE clk to TURBO on probe

[Abhinaba Rakshit]

MMC controller lacks a clock scaling mechanism, unlike the UFS
controller. By default, the MMC controller is set to TURBO mode
during probe, but the ICE clock remains at XO frequency,
leading to read/write performance degradation on eMMC.

To address this, set the ICE clock to TURBO during probe to
align it with the controller clock. This ensures consistent
performance and avoids mismatches between the controller
and ICE clock frequencies.

For platforms where ICE is represented as a separate device,
use the OPP framework to vote for TURBO mode, maintaining
proper voltage and power domain constraints.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
 drivers/soc/qcom/ice.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index 7976a18d9a4cda1ad6b62b66ce011e244d0f6856..e8ee02a709574afa4ebb8e4395a8d899bf1d4976 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -659,6 +659,13 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
 	}
 
+	if (engine->has_opp) {
+		/* Vote for maximum clock rate for maximum performance */
+		err = dev_pm_opp_set_rate(dev, INT_MAX);
+		if (err)
+			dev_warn(dev, "Failed boosting the ICE clk to TURBO\n");
+	}
+
 	engine->core_clk_freq = clk_get_rate(engine->core_clk);
 	if (!qcom_ice_check_supported(engine))
 		return ERR_PTR(-EOPNOTSUPP);

-- 
2.34.1

Re: [PATCH v7 3/3] soc: qcom: ice: Set ICE clk to TURBO on probe

[Abhinaba Rakshit]

On Mon, Mar 02, 2026 at 04:19:15PM +0530, Abhinaba Rakshit wrote:
> MMC controller lacks a clock scaling mechanism, unlike the UFS
> controller. By default, the MMC controller is set to TURBO mode
> during probe, but the ICE clock remains at XO frequency,
> leading to read/write performance degradation on eMMC.
> 
> To address this, set the ICE clock to TURBO during probe to
> align it with the controller clock. This ensures consistent
> performance and avoids mismatches between the controller
> and ICE clock frequencies.
> 
> For platforms where ICE is represented as a separate device,
> use the OPP framework to vote for TURBO mode, maintaining
> proper voltage and power domain constraints.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/ice.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index 7976a18d9a4cda1ad6b62b66ce011e244d0f6856..e8ee02a709574afa4ebb8e4395a8d899bf1d4976 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -659,6 +659,13 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
>  	}
>  
> +	if (engine->has_opp) {
> +		/* Vote for maximum clock rate for maximum performance */
> +		err = dev_pm_opp_set_rate(dev, INT_MAX);
> +		if (err)
> +			dev_warn(dev, "Failed boosting the ICE clk to TURBO\n");
> +	}
> +
>  	engine->core_clk_freq = clk_get_rate(engine->core_clk);
>  	if (!qcom_ice_check_supported(engine))
>  		return ERR_PTR(-EOPNOTSUPP);

Hi Konrad

Since you previously reviewed this change, I wanted to share an improved approach
that I recently realized for handling ICE clock scaling in the MMC use‑case.

So far, we have been voting for the maximum frequency during the ICE device probe
to align with MMC requirements.
But because the ICE probe is common across different storage clients, applying
the MAX vote at probe time may unintentionally impact other storage paths.

Now that we have a generic scaling API exposed, we can make this logic
MMC‑specific instead. In particular, within sdhci_msm_ice_init().
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/mmc/host/sdhci-msm.c#n1966,
we can invoke: qcom_ice_scale_clk(ice, INT_MAX, false);

This ensures the MAX clock vote is applied only in the MMC context,
without altering behavior for other storage clients relying on the ICE driver.

I believe this results in a cleaner and correctly scoped design.
Let me know your thoughts.

Abhinaba Rakshit

Re: [PATCH v7 3/3] soc: qcom: ice: Set ICE clk to TURBO on probe

[Harshal Dev]

On 3/23/2026 10:42 PM, Abhinaba Rakshit wrote:
> On Mon, Mar 02, 2026 at 04:19:15PM +0530, Abhinaba Rakshit wrote:
>> MMC controller lacks a clock scaling mechanism, unlike the UFS
>> controller. By default, the MMC controller is set to TURBO mode
>> during probe, but the ICE clock remains at XO frequency,
>> leading to read/write performance degradation on eMMC.
>>
>> To address this, set the ICE clock to TURBO during probe to
>> align it with the controller clock. This ensures consistent
>> performance and avoids mismatches between the controller
>> and ICE clock frequencies.
>>
>> For platforms where ICE is represented as a separate device,
>> use the OPP framework to vote for TURBO mode, maintaining
>> proper voltage and power domain constraints.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
>> ---
>>  drivers/soc/qcom/ice.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
>> index 7976a18d9a4cda1ad6b62b66ce011e244d0f6856..e8ee02a709574afa4ebb8e4395a8d899bf1d4976 100644
>> --- a/drivers/soc/qcom/ice.c
>> +++ b/drivers/soc/qcom/ice.c
>> @@ -659,6 +659,13 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>>  			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
>>  	}
>>  
>> +	if (engine->has_opp) {
>> +		/* Vote for maximum clock rate for maximum performance */
>> +		err = dev_pm_opp_set_rate(dev, INT_MAX);
>> +		if (err)
>> +			dev_warn(dev, "Failed boosting the ICE clk to TURBO\n");
>> +	}
>> +
>>  	engine->core_clk_freq = clk_get_rate(engine->core_clk);
>>  	if (!qcom_ice_check_supported(engine))
>>  		return ERR_PTR(-EOPNOTSUPP);
> 
> Hi Konrad
> 
> Since you previously reviewed this change, I wanted to share an improved approach
> that I recently realized for handling ICE clock scaling in the MMC use‑case.
> 
> So far, we have been voting for the maximum frequency during the ICE device probe
> to align with MMC requirements.
> But because the ICE probe is common across different storage clients, applying
> the MAX vote at probe time may unintentionally impact other storage paths.
> 
> Now that we have a generic scaling API exposed, we can make this logic
> MMC‑specific instead. In particular, within sdhci_msm_ice_init().
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/mmc/host/sdhci-msm.c#n1966,
> we can invoke: qcom_ice_scale_clk(ice, INT_MAX, false);
>

I agree, this is better instead of blindly turning the clk freq to maximum.

I was about to ask you to move this to qcom_ice_probe() as per comments in previous
commit. However, since you have mentioned this now, I suggest adding a call in
sdhci_msm_ice_init() to qcom_ice_clk_scale() immediately after it calls qcom_ice_create().

Regards,
Harshal

> This ensures the MAX clock vote is applied only in the MMC context,
> without altering behavior for other storage clients relying on the ICE driver.
> 
> I believe this results in a cleaner and correctly scoped design.
> Let me know your thoughts.
> 
> Abhinaba Rakshit
> 

Re: [PATCH v7 3/3] soc: qcom: ice: Set ICE clk to TURBO on probe

[Abhinaba Rakshit]

On Mon, Mar 30, 2026 at 08:14:13PM +0530, Harshal Dev wrote:
> >> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> >> index 7976a18d9a4cda1ad6b62b66ce011e244d0f6856..e8ee02a709574afa4ebb8e4395a8d899bf1d4976 100644
> >> --- a/drivers/soc/qcom/ice.c
> >> +++ b/drivers/soc/qcom/ice.c
> >> @@ -659,6 +659,13 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >>  			dev_info(dev, "ICE OPP table is not registered, please update your DT\n");
> >>  	}
> >>  
> >> +	if (engine->has_opp) {
> >> +		/* Vote for maximum clock rate for maximum performance */
> >> +		err = dev_pm_opp_set_rate(dev, INT_MAX);
> >> +		if (err)
> >> +			dev_warn(dev, "Failed boosting the ICE clk to TURBO\n");
> >> +	}
> >> +
> >>  	engine->core_clk_freq = clk_get_rate(engine->core_clk);
> >>  	if (!qcom_ice_check_supported(engine))
> >>  		return ERR_PTR(-EOPNOTSUPP);
> > 
> > Hi Konrad
> > 
> > Since you previously reviewed this change, I wanted to share an improved approach
> > that I recently realized for handling ICE clock scaling in the MMC use‑case.
> > 
> > So far, we have been voting for the maximum frequency during the ICE device probe
> > to align with MMC requirements.
> > But because the ICE probe is common across different storage clients, applying
> > the MAX vote at probe time may unintentionally impact other storage paths.
> > 
> > Now that we have a generic scaling API exposed, we can make this logic
> > MMC‑specific instead. In particular, within sdhci_msm_ice_init().
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/mmc/host/sdhci-msm.c#n1966,
> > we can invoke: qcom_ice_scale_clk(ice, INT_MAX, false);
> >
> 
> I agree, this is better instead of blindly turning the clk freq to maximum.
> 
> I was about to ask you to move this to qcom_ice_probe() as per comments in previous
> commit. However, since you have mentioned this now, I suggest adding a call in
> sdhci_msm_ice_init() to qcom_ice_clk_scale() immediately after it calls qcom_ice_create().

Sure, thanks.
Will update it in patchset v8.

Abinaba Rakshit

Re: [PATCH v7 0/3] Enable ICE clock scaling

[Abhinaba Rakshit]

On Mon, Mar 02, 2026 at 04:19:12PM +0530, Abhinaba Rakshit wrote:
> Introduce support for dynamic clock scaling of the ICE (Inline Crypto Engine)
> using the OPP framework. During ICE device probe, the driver now attempts to
> parse an optional OPP table from the ICE-specific device tree node for
> DVFS-aware operations. API qcom_ice_scale_clk is exposed by ICE driver
> and is invoked by UFS host controller driver in response to clock scaling
> requests, ensuring coordination between ICE and host controller.
> 
> For MMC controllers that do not support clock scaling, the ICE clock frequency
> is kept aligned with the MMC controller’s clock rate (TURBO) to ensure
> consistent operation.
> 
> Dynamic clock scaling based on OPP tables enables better power-performance
> trade-offs. By adjusting ICE clock frequencies according to workload and power
> constraints, the system can achieve higher throughput when needed and
> reduce power consumption during idle or low-load conditions.
> 
> The OPP table remains optional, absence of the table will not cause
> probe failure. However, in the absence of an OPP table, ICE clocks will
> remain at their default rates, which may limit performance under
> high-load scenarios or prevent performance optimizations during idle periods.
> 
> Merge Order and Dependencies
> ============================
> 
> Patch 1/4 (dt-bindings) from the previous series
> (https://lore.kernel.org/all/aaKt9PET6lVkBcif@gondor.apana.org.au/) has already
> been applied. This v7 series therefore includes only the ICE driver and
> UFS driver changes (previously patches 2–4).
> 
> Patch 1 is the change which should be merged first.
> 
> Patch 2 is dependent on patch 1 for the qcom_ice_scale_clk API to be available.
> Patch 3 is dependent on patch 1 for opp-table parsing to be enabled in the
> ICE driver.
> 
> Patch 2 and patch 3 are *not* dependent on each other. Once patch 1 is
> merged, patch (2,3) can be applied independently by their respective
> maintainers.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
> Changes in v7:
> - Replace the custom rounding flags with 'bool round_ceil' as suggested.
> - Update the dev_info log-line.
> - Dropped dt-bindings patch (already applied by in previous patchseries).
> - Add merge order and dependencies as suggested.
> - Link to v6: https://lore.kernel.org/r/20260219-enable-ufs-ice-clock-scaling-v6-0-0c5245117d45@oss.qualcomm.com
> 
> Changes in v6:
> - Remove scale_up parameter from qcom_ice_scale_clk API.
> - Remove having max_freq and min_freq as the checks for overclocking and underclocking is no-longer needed.
> - UFS driver passes rounding flags depending on scale_up value.
> - Ensure UFS driver does not fail devfreq requests if ICE OPP is not supported.
> - Link to v5: https://lore.kernel.org/r/3ecb8d08-64cb-4fe1-bebd-1532dc5a86af@oss.qualcomm.com
> 
> Changes in v5:
> - Update operating-points-v2 property in dtbindings as suggested.
> - Fix comment styles.
> - Add argument in qcom_ice_create to distinguish between legacy bindings and newer bindings.
> - Ensure to drop votes in suspend and enable the last vote in resume.
> - Link to v4: https://lore.kernel.org/r/20260128-enable-ufs-ice-clock-scaling-v4-0-260141e8fce6@oss.qualcomm.com
> 
> Changes in v4:
> - Enable multiple frequency scaling based OPP-entries as suggested in v3 patchset.
> - Include bindings change: https://lore.kernel.org/all/20260123-add-operating-points-v2-property-for-qcom-ice-bindings-v1-1-2155f7aacc28@oss.qualcomm.com/.
> - Link to v3: https://lore.kernel.org/r/20260123-enable-ufs-ice-clock-scaling-v3-0-d0d8532abd98@oss.qualcomm.com
> 
> Changes in v3:
> - Avoid clock scaling in case of legacy bindings as suggested.
> - Use of_device_is_compatible to distinguish between legacy and non-legacy bindings.
> - Link to v2: https://lore.kernel.org/r/20251121-enable-ufs-ice-clock-scaling-v2-0-66cb72998041@oss.qualcomm.com
> 
> Changes in v2:
> - Use OPP-table instead of freq-table-hz for clock scaling.
> - Enable clock scaling for legacy targets as well, by fetching frequencies from storage opp-table.
> - Introduce has_opp variable in qcom_ice structure to keep track, if ICE instance has dedicated OPP-table registered.
> - Combined the changes for patch-series <20251001-set-ice-clock-to-turbo-v1-1-7b802cf61dda@oss.qualcomm.com> as suggested.
> - Link to v1: https://lore.kernel.org/r/20251001-enable-ufs-ice-clock-scaling-v1-0-ec956160b696@oss.qualcomm.com
> 
> ---
> Abhinaba Rakshit (3):
>       soc: qcom: ice: Add OPP-based clock scaling support for ICE
>       ufs: host: Add ICE clock scaling during UFS clock changes
>       soc: qcom: ice: Set ICE clk to TURBO on probe
> 
>  drivers/soc/qcom/ice.c      | 89 +++++++++++++++++++++++++++++++++++++++++++--
>  drivers/ufs/host/ufs-qcom.c | 19 +++++++++-
>  include/soc/qcom/ice.h      |  2 +
>  3 files changed, 106 insertions(+), 4 deletions(-)
> ---
> base-commit: fe4d0dea039f2befb93f27569593ec209843b0f5
> change-id: 20251120-enable-ufs-ice-clock-scaling-b063caf3e6f9
> 
> Best regards,
> -- 
> Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>

Just a gentle reminder regarding this patch series.  
Whenever any maintainers got a chance, I’d appreciate a review.
Please let me know, anything pending from myside.

Abhinaba Rakshit