[PATCH v5 0/4] Enable ICE clock scaling

[PATCH v5 0/4] 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 to
determine minimum and maximum supported frequencies 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.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@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 (4):
      dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE
      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

 .../bindings/crypto/qcom,inline-crypto-engine.yaml |  26 ++++
 drivers/soc/qcom/ice.c                             | 137 ++++++++++++++++++++-
 drivers/ufs/host/ufs-qcom.c                        |  17 +++
 include/soc/qcom/ice.h                             |   5 +
 4 files changed, 182 insertions(+), 3 deletions(-)
---
base-commit: fe4d0dea039f2befb93f27569593ec209843b0f5
change-id: 20251120-enable-ufs-ice-clock-scaling-b063caf3e6f9

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

[PATCH v5 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE

[Abhinaba Rakshit]

Add support for specifying OPPs for the Qualcomm Inline Crypto Engine
by allowing the use of the standard "operating-points-v2" property in
the ICE device node.

ICE clock management was handled by the storage drivers in legacy
bindings, so the ICE driver itself had no mechanism for clock scaling.
With the introduction of the new standalone ICE device node, clock
control must now be performed directly by the ICE driver. Enabling
operating-points-v2 allows the driver to describe and manage the
frequency and voltage requirements for proper DVFS operation.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
 .../bindings/crypto/qcom,inline-crypto-engine.yaml | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
index c3408dcf5d2057270a732fe0e6744f4aa6496e06..50bcf3309b9fa0a3f727f010301670e5de58366f 100644
--- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
@@ -30,6 +30,11 @@ properties:
   clocks:
     maxItems: 1
 
+  operating-points-v2: true
+
+  opp-table:
+    type: object
+
 required:
   - compatible
   - reg
@@ -46,5 +51,26 @@ examples:
                    "qcom,inline-crypto-engine";
       reg = <0x01d88000 0x8000>;
       clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>;
+
+      operating-points-v2 = <&ice_opp_table>;
+
+      ice_opp_table: opp-table {
+        compatible = "operating-points-v2";
+
+        opp-100000000 {
+          opp-hz = /bits/ 64 <100000000>;
+          required-opps = <&rpmhpd_opp_low_svs>;
+        };
+
+        opp-201500000 {
+          opp-hz = /bits/ 64 <201500000>;
+          required-opps = <&rpmhpd_opp_svs_l1>;
+        };
+
+        opp-403000000 {
+          opp-hz = /bits/ 64 <403000000>;
+          required-opps = <&rpmhpd_opp_nom>;
+        };
+      };
     };
 ...

-- 
2.34.1

[PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

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 based on the target frequency provided and if a valid
OPP-table is registered. Use flags (if provided) to decide on
the rounding of the clock freq against OPP-table. Incase no flags
are provided use default behaviour (CEIL incase of scale_up and FLOOR
incase of ~scale_up). 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 | 132 +++++++++++++++++++++++++++++++++++++++++++++++--
 include/soc/qcom/ice.h |   5 ++
 2 files changed, 134 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cadd21d6f96eb1799963a13db4b2b72..0bdc64db414a7028653c0f3327988b1554788fcf 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,10 @@ struct qcom_ice {
 	bool use_hwkm;
 	bool hwkm_init_complete;
 	u8 hwkm_version;
+	unsigned long max_freq;
+	unsigned long min_freq;
+	unsigned long core_clk_freq;
+	bool has_opp;
 };
 
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -310,12 +315,17 @@ 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",
 			err);
 		return err;
 	}
+
 	qcom_ice_hwkm_init(ice);
 	return qcom_ice_wait_bist_status(ice);
 }
@@ -324,6 +334,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 +564,79 @@ 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
+ * @scale_up: If @flags is 0, choose ceil (true) or floor (false)
+ * @flags: Rounding policy (ICE_CLOCK_ROUND_*); overrides @scale_up
+ *
+ * Clamps @target_freq to the OPP range (min/max), selects an OPP per rounding
+ * policy, then applies it via dev_pm_opp_set_rate() (including voltage/PD
+ * changes).
+ *
+ * Return: 0 on success; -EOPNOTSUPP if no OPP table; or error from
+ *         dev_pm_opp_set_rate()/OPP lookup.
+ */
+int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
+		       bool scale_up, unsigned int flags)
+{
+	int ret;
+	unsigned long ice_freq = target_freq;
+	struct dev_pm_opp *opp;
+
+	if (!ice->has_opp)
+		return -EOPNOTSUPP;
+
+	/* Clamp the freq to max if target_freq is beyond supported frequencies */
+	if (ice->max_freq && target_freq >= ice->max_freq) {
+		ice_freq = ice->max_freq;
+		goto scale_clock;
+	}
+
+	/* Clamp the freq to min if target_freq is below supported frequencies */
+	if (ice->min_freq && target_freq <= ice->min_freq) {
+		ice_freq = ice->min_freq;
+		goto scale_clock;
+	}
+
+	switch (flags) {
+	case ICE_CLOCK_ROUND_CEIL:
+		opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
+		break;
+	case ICE_CLOCK_ROUND_FLOOR:
+		opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
+		break;
+	default:
+		if (scale_up)
+			opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
+		else
+			opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
+		break;
+	}
+
+	if (IS_ERR(opp))
+		return -EINVAL;
+	dev_pm_opp_put(opp);
+
+scale_clock:
+
+	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;
+	struct dev_pm_opp *opp;
+	int err;
+	unsigned long rate;
 
 	if (!qcom_scm_is_available())
 		return ERR_PTR(-EPROBE_DEFER);
@@ -584,6 +668,48 @@ 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\n");
+	}
+
+	if (engine->has_opp) {
+		/* Find the ICE core clock min frequency */
+		rate = 0;
+		opp = dev_pm_opp_find_freq_ceil_indexed(dev, &rate, 0);
+		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);
+		}
+
+		/* Find the ICE core clock max frequency */
+		rate = ULONG_MAX;
+		opp = dev_pm_opp_find_freq_floor_indexed(dev, &rate, 0);
+		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);
+		}
+	}
+
+	engine->core_clk_freq = clk_get_rate(engine->core_clk);
 	if (!qcom_ice_check_supported(engine))
 		return ERR_PTR(-EOPNOTSUPP);
 
@@ -628,7 +754,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 +851,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..055edf3a704ff25a608a880cf9be35363f8a02d3 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -9,6 +9,9 @@
 #include <linux/blk-crypto.h>
 #include <linux/types.h>
 
+#define ICE_CLOCK_ROUND_CEIL	BIT(1)
+#define ICE_CLOCK_ROUND_FLOOR	BIT(2)
+
 struct qcom_ice;
 
 int qcom_ice_enable(struct qcom_ice *ice);
@@ -30,5 +33,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 scale_up, unsigned int flags);
 
 #endif /* __QCOM_ICE_H__ */

-- 
2.34.1

[PATCH v5 3/4] 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.

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

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 8d119b3223cbdaa3297d2beabced0962a1a847d5..00cb9cde760380e7e4213095b9c66657a23b13ee 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 scale_up, unsigned int flags)
+{
+	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
+		return qcom_ice_scale_clk(host->ice, target_freq, scale_up, flags);
+
+	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 scale_up, unsigned int flags)
+{
+	return 0;
+}
+
 #endif
 
 static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
@@ -1646,6 +1661,8 @@ 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, 0);
 
 		if (err) {
 			ufshcd_uic_hibern8_exit(hba);

-- 
2.34.1

[PATCH v5 4/4] 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.

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

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index 0bdc64db414a7028653c0f3327988b1554788fcf..3b69b5673ea93fa927e62a7f4b5ae52878d564c8 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -707,6 +707,11 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 			engine->max_freq = rate;
 			dev_pm_opp_put(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);

-- 
2.34.1

Re: [PATCH v5 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE

[Rob Herring (Arm)]

On Wed, 11 Feb 2026 15:17:44 +0530, Abhinaba Rakshit wrote:
> Add support for specifying OPPs for the Qualcomm Inline Crypto Engine
> by allowing the use of the standard "operating-points-v2" property in
> the ICE device node.
> 
> ICE clock management was handled by the storage drivers in legacy
> bindings, so the ICE driver itself had no mechanism for clock scaling.
> With the introduction of the new standalone ICE device node, clock
> control must now be performed directly by the ICE driver. Enabling
> operating-points-v2 allows the driver to describe and manage the
> frequency and voltage requirements for proper DVFS operation.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  .../bindings/crypto/qcom,inline-crypto-engine.yaml | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Konrad Dybcio]

On 2/11/26 10:47 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 based on the target frequency provided and if a valid
> OPP-table is registered. Use flags (if provided) to decide on
> the rounding of the clock freq against OPP-table. Incase no flags
> are provided use default behaviour (CEIL incase of scale_up and FLOOR
> incase of ~scale_up). 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>
> ---

[...]

> +/**
> + * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
> + * @ice: ICE driver data
> + * @target_freq: requested frequency in Hz
> + * @scale_up: If @flags is 0, choose ceil (true) or floor (false)
> + * @flags: Rounding policy (ICE_CLOCK_ROUND_*); overrides @scale_up
> + *
> + * Clamps @target_freq to the OPP range (min/max), selects an OPP per rounding
> + * policy, then applies it via dev_pm_opp_set_rate() (including voltage/PD
> + * changes).
> + *
> + * Return: 0 on success; -EOPNOTSUPP if no OPP table; or error from
> + *         dev_pm_opp_set_rate()/OPP lookup.
> + */
> +int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
> +		       bool scale_up, unsigned int flags)
> +{
> +	int ret;
> +	unsigned long ice_freq = target_freq;
> +	struct dev_pm_opp *opp;

Reverse-Christmas-tree ordering would be neat

> +
> +	if (!ice->has_opp)
> +		return -EOPNOTSUPP;
> +
> +	/* Clamp the freq to max if target_freq is beyond supported frequencies */
> +	if (ice->max_freq && target_freq >= ice->max_freq) {
> +		ice_freq = ice->max_freq;
> +		goto scale_clock;
> +	}
> +
> +	/* Clamp the freq to min if target_freq is below supported frequencies */
> +	if (ice->min_freq && target_freq <= ice->min_freq) {
> +		ice_freq = ice->min_freq;
> +		goto scale_clock;
> +	}

The OPP framework won't let you overclock the ICE if this is what these checks
are about. Plus the clk framework will perform rounding for you too

> +
> +	switch (flags) {

Are you going to use these flags? Currently they're dead code

> +	case ICE_CLOCK_ROUND_CEIL:
> +		opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);

You never use the index (hardcoded to 0)

> +		break;
> +	case ICE_CLOCK_ROUND_FLOOR:
> +		opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
> +		break;
> +	default:
> +		if (scale_up)
> +			opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
> +		else
> +			opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);

Is this distinction necessary?

Konrad

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
> On 2/11/26 10:47 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 based on the target frequency provided and if a valid
> > OPP-table is registered. Use flags (if provided) to decide on
> > the rounding of the clock freq against OPP-table. Incase no flags
> > are provided use default behaviour (CEIL incase of scale_up and FLOOR
> > incase of ~scale_up). 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>
> > ---
> 
> [...]
> 
> > +/**
> > + * qcom_ice_scale_clk() - Scale ICE clock for DVFS-aware operations
> > + * @ice: ICE driver data
> > + * @target_freq: requested frequency in Hz
> > + * @scale_up: If @flags is 0, choose ceil (true) or floor (false)
> > + * @flags: Rounding policy (ICE_CLOCK_ROUND_*); overrides @scale_up
> > + *
> > + * Clamps @target_freq to the OPP range (min/max), selects an OPP per rounding
> > + * policy, then applies it via dev_pm_opp_set_rate() (including voltage/PD
> > + * changes).
> > + *
> > + * Return: 0 on success; -EOPNOTSUPP if no OPP table; or error from
> > + *         dev_pm_opp_set_rate()/OPP lookup.
> > + */
> > +int qcom_ice_scale_clk(struct qcom_ice *ice, unsigned long target_freq,
> > +		       bool scale_up, unsigned int flags)
> > +{
> > +	int ret;
> > +	unsigned long ice_freq = target_freq;
> > +	struct dev_pm_opp *opp;
> 
> Reverse-Christmas-tree ordering would be neat

Ack, will update.

> 
> > +
> > +	if (!ice->has_opp)
> > +		return -EOPNOTSUPP;
> > +
> > +	/* Clamp the freq to max if target_freq is beyond supported frequencies */
> > +	if (ice->max_freq && target_freq >= ice->max_freq) {
> > +		ice_freq = ice->max_freq;
> > +		goto scale_clock;
> > +	}
> > +
> > +	/* Clamp the freq to min if target_freq is below supported frequencies */
> > +	if (ice->min_freq && target_freq <= ice->min_freq) {
> > +		ice_freq = ice->min_freq;
> > +		goto scale_clock;
> > +	}
> 
> The OPP framework won't let you overclock the ICE if this is what these checks
> are about. Plus the clk framework will perform rounding for you too

Right, maybe I can just add a check for 0 freq just to ensure the export API is
not miss used.
Something shown below:

if (!target_freq)
    return -EINVAL;

However, my main concern was for the corner cases, where:
(target_freq > max && ROUND_CEIL)
and
(target_freq < min && ROUND_FLOOR)
In both the cases, the OPP APIs will fail and the clock remains unchanged.
Hence, I added the checks to make the API as generic/robust as possible.

Please let me know, your thoughts.

> > +
> > +	switch (flags) {
> 
> Are you going to use these flags? Currently they're dead code

I agree, currently they are not used.
However, since its an export API, I want to keep the rounding FLAGS
support as it a common to have rounding flags in clock scaling APIs,
and to support any future use-cases as well.

> > +	case ICE_CLOCK_ROUND_CEIL:
> > +		opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
> 
You never use the index (hardcoded to 0)

Ack, will update.

> 
> > +		break;
> > +	case ICE_CLOCK_ROUND_FLOOR:
> > +		opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
> > +		break;
> > +	default:
> > +		if (scale_up)
> > +			opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
> > +		else
> > +			opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
> 
> Is this distinction necessary?

Well not necessary. However, I wanted to have the scale_up option as well and make use of
it in the API. Hence, I thought of having this to be harmless.

> Konrad

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
> On 2/11/26 10:47 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 based on the target frequency provided and if a valid
> > OPP-table is registered. Use flags (if provided) to decide on
> > the rounding of the clock freq against OPP-table. Incase no flags
> > are provided use default behaviour (CEIL incase of scale_up and FLOOR
> > incase of ~scale_up). 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>
> > ---
> 
> [...]
>  
> > +	switch (flags) {
> 
> Are you going to use these flags? Currently they're dead code

[...]

> > +		break;
> > +	case ICE_CLOCK_ROUND_FLOOR:
> > +		opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
> > +		break;
> > +	default:
> > +		if (scale_up)
> > +			opp = dev_pm_opp_find_freq_ceil_indexed(ice->dev, &ice_freq, 0);
> > +		else
> > +			opp = dev_pm_opp_find_freq_floor_indexed(ice->dev, &ice_freq, 0);
> 
> Is this distinction necessary?

Just giving it a second thought.
Well right, these are not necessary, infact we can completly remove the scele_up
from the export API and make use of the rounding flags eventually.
The ufs driver can pass the right flag, based on the check against scale_up variable.
Ack, will update this in next patchset. 

Abhinaba Rakshit

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Konrad Dybcio]

On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
>>> Register optional operation-points-v2 table for ICE device
>>> and aquire its minimum and maximum frequency during ICE
>>> device probe.

[...]

>>> +	if (!ice->has_opp)
>>> +		return -EOPNOTSUPP;
>>> +
>>> +	/* Clamp the freq to max if target_freq is beyond supported frequencies */
>>> +	if (ice->max_freq && target_freq >= ice->max_freq) {
>>> +		ice_freq = ice->max_freq;
>>> +		goto scale_clock;
>>> +	}
>>> +
>>> +	/* Clamp the freq to min if target_freq is below supported frequencies */
>>> +	if (ice->min_freq && target_freq <= ice->min_freq) {
>>> +		ice_freq = ice->min_freq;
>>> +		goto scale_clock;
>>> +	}
>>
>> The OPP framework won't let you overclock the ICE if this is what these checks
>> are about. Plus the clk framework will perform rounding for you too
> 
> Right, maybe I can just add a check for 0 freq just to ensure the export API is
> not miss used.
> Something shown below:
> 
> if (!target_freq)
>     return -EINVAL;
> 
> However, my main concern was for the corner cases, where:
> (target_freq > max && ROUND_CEIL)
> and
> (target_freq < min && ROUND_FLOOR)
> In both the cases, the OPP APIs will fail and the clock remains unchanged.

I would argue that's expected behavior, if the requested rate can not
be achieved, the "set_rate"-like function should fail

> Hence, I added the checks to make the API as generic/robust as possible.

AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
play, but the latter never goes above the FMAX of the former

For the second case, I'm not sure it's valid. For "find lowest rate" I would
expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
so I'm not sure _floor() is useful

> 
> Please let me know, your thoughts.
> 
>>> +
>>> +	switch (flags) {
>>
>> Are you going to use these flags? Currently they're dead code
> 
> I agree, currently they are not used.
> However, since its an export API, I want to keep the rounding FLAGS
> support as it a common to have rounding flags in clock scaling APIs,
> and to support any future use-cases as well.

I think you have a bit of a misconception - yes, this is an export API and
should be designed with the consumers in mind, but then it's consumed by
in-tree modules only ("what's not on the list doesn't exist"), so it's actually
generally *discouraged* (with varying levels of emphasis) to add any code that
is not immediately useful, as these functions can be updated at any point in
time down the line

Konrad

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
> > On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
> >> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
> >>> Register optional operation-points-v2 table for ICE device
> >>> and aquire its minimum and maximum frequency during ICE
> >>> device probe.
> 
> [...]
> 
> >>> +	if (!ice->has_opp)
> >>> +		return -EOPNOTSUPP;
> >>> +
> >>> +	/* Clamp the freq to max if target_freq is beyond supported frequencies */
> >>> +	if (ice->max_freq && target_freq >= ice->max_freq) {
> >>> +		ice_freq = ice->max_freq;
> >>> +		goto scale_clock;
> >>> +	}
> >>> +
> >>> +	/* Clamp the freq to min if target_freq is below supported frequencies */
> >>> +	if (ice->min_freq && target_freq <= ice->min_freq) {
> >>> +		ice_freq = ice->min_freq;
> >>> +		goto scale_clock;
> >>> +	}
> >>
> >> The OPP framework won't let you overclock the ICE if this is what these checks
> >> are about. Plus the clk framework will perform rounding for you too
> > 
> > Right, maybe I can just add a check for 0 freq just to ensure the export API is
> > not miss used.
> > Something shown below:
> > 
> > if (!target_freq)
> >     return -EINVAL;
> > 
> > However, my main concern was for the corner cases, where:
> > (target_freq > max && ROUND_CEIL)
> > and
> > (target_freq < min && ROUND_FLOOR)
> > In both the cases, the OPP APIs will fail and the clock remains unchanged.
> 
> I would argue that's expected behavior, if the requested rate can not
> be achieved, the "set_rate"-like function should fail
> 
> > Hence, I added the checks to make the API as generic/robust as possible.
> 
> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
> play, but the latter never goes above the FMAX of the former
> 
> For the second case, I'm not sure it's valid. For "find lowest rate" I would
> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
> so I'm not sure _floor() is useful

Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
and ice-clk >= storage-clk in case of scale_down.

Best would be to use, _floor for scale_up and _ceil for scale_down.
 
> > 
> > Please let me know, your thoughts.
> > 
> >>> +
> >>> +	switch (flags) {
> >>
> >> Are you going to use these flags? Currently they're dead code
> > 
> > I agree, currently they are not used.
> > However, since its an export API, I want to keep the rounding FLAGS
> > support as it a common to have rounding flags in clock scaling APIs,
> > and to support any future use-cases as well.
> 
> I think you have a bit of a misconception - yes, this is an export API and
> should be designed with the consumers in mind, but then it's consumed by
> in-tree modules only ("what's not on the list doesn't exist"), so it's actually
> generally *discouraged* (with varying levels of emphasis) to add any code that
> is not immediately useful, as these functions can be updated at any point in
> time down the line

Sure, understood
Will make sure to update in next patchset and make use of rounding flags.

Abhinaba Rakshit

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Konrad Dybcio]

On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:
> On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
>> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
>>> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
>>>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
>>>>> Register optional operation-points-v2 table for ICE device
>>>>> and aquire its minimum and maximum frequency during ICE
>>>>> device probe.

[...]

>>> However, my main concern was for the corner cases, where:
>>> (target_freq > max && ROUND_CEIL)
>>> and
>>> (target_freq < min && ROUND_FLOOR)
>>> In both the cases, the OPP APIs will fail and the clock remains unchanged.
>>
>> I would argue that's expected behavior, if the requested rate can not
>> be achieved, the "set_rate"-like function should fail
>>
>>> Hence, I added the checks to make the API as generic/robust as possible.
>>
>> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
>> play, but the latter never goes above the FMAX of the former
>>
>> For the second case, I'm not sure it's valid. For "find lowest rate" I would
>> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
>> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
>> so I'm not sure _floor() is useful
> 
> Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
> and ice-clk >= storage-clk in case of scale_down.

I don't quite understand the first case (ice <= storage for scale_up), could you
please elaborate?

Konrad

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

On Thu, Feb 19, 2026 at 03:20:31PM +0100, Konrad Dybcio wrote:
> On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:
> > On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
> >> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
> >>> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
> >>>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
> >>>>> Register optional operation-points-v2 table for ICE device
> >>>>> and aquire its minimum and maximum frequency during ICE
> >>>>> device probe.
> 
> [...]
> 
> >>> However, my main concern was for the corner cases, where:
> >>> (target_freq > max && ROUND_CEIL)
> >>> and
> >>> (target_freq < min && ROUND_FLOOR)
> >>> In both the cases, the OPP APIs will fail and the clock remains unchanged.
> >>
> >> I would argue that's expected behavior, if the requested rate can not
> >> be achieved, the "set_rate"-like function should fail
> >>
> >>> Hence, I added the checks to make the API as generic/robust as possible.
> >>
> >> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
> >> play, but the latter never goes above the FMAX of the former
> >>
> >> For the second case, I'm not sure it's valid. For "find lowest rate" I would
> >> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
> >> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
> >> so I'm not sure _floor() is useful
> > 
> > Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
> > and ice-clk >= storage-clk in case of scale_down.
> 
> I don't quite understand the first case (ice <= storage for scale_up), could you
> please elaborate?

Here I basically mean to say is that, as you mentioned "we generally set
storage_ctrl_rate == ice_clk_rate, but latter never goes above the FMAX of the former".
I guess, the ideal way to handle this is to ensure using _floor when we want to scale_up.
This ensures the ice_clk does not vote for more that what storage_ctrl is running on.

Also, this avoids the corner case, where target_freq provided is higher that the supporter
rates (descriped in ICE OPP-table) for ICE, using _ceil makes no sense.

Abhinaba Rakshit

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Konrad Dybcio]

On 2/20/26 8:33 AM, Abhinaba Rakshit wrote:
> On Thu, Feb 19, 2026 at 03:20:31PM +0100, Konrad Dybcio wrote:
>> On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:
>>> On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
>>>> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
>>>>> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
>>>>>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
>>>>>>> Register optional operation-points-v2 table for ICE device
>>>>>>> and aquire its minimum and maximum frequency during ICE
>>>>>>> device probe.
>>
>> [...]
>>
>>>>> However, my main concern was for the corner cases, where:
>>>>> (target_freq > max && ROUND_CEIL)
>>>>> and
>>>>> (target_freq < min && ROUND_FLOOR)
>>>>> In both the cases, the OPP APIs will fail and the clock remains unchanged.
>>>>
>>>> I would argue that's expected behavior, if the requested rate can not
>>>> be achieved, the "set_rate"-like function should fail
>>>>
>>>>> Hence, I added the checks to make the API as generic/robust as possible.
>>>>
>>>> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
>>>> play, but the latter never goes above the FMAX of the former
>>>>
>>>> For the second case, I'm not sure it's valid. For "find lowest rate" I would
>>>> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
>>>> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
>>>> so I'm not sure _floor() is useful
>>>
>>> Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
>>> and ice-clk >= storage-clk in case of scale_down.
>>
>> I don't quite understand the first case (ice <= storage for scale_up), could you
>> please elaborate?
> 
> Here I basically mean to say is that, as you mentioned "we generally set
> storage_ctrl_rate == ice_clk_rate, but latter never goes above the FMAX of the former".
> I guess, the ideal way to handle this is to ensure using _floor when we want to scale_up.
> This ensures the ice_clk does not vote for more that what storage_ctrl is running on.

Right, but what I was asking specifically is why we don't want that to happen

> Also, this avoids the corner case, where target_freq provided is higher that the supporter
> rates (descriped in ICE OPP-table) for ICE, using _ceil makes no sense.

This is potentially a valid concern, do we have cases of storage_clk > ice_clk?

Konrad

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Abhinaba Rakshit]

On Fri, Feb 20, 2026 at 10:42:58AM +0100, Konrad Dybcio wrote:
> On 2/20/26 8:33 AM, Abhinaba Rakshit wrote:
> > On Thu, Feb 19, 2026 at 03:20:31PM +0100, Konrad Dybcio wrote:
> >> On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:
> >>> On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
> >>>> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
> >>>>> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
> >>>>>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
> >>>>>>> Register optional operation-points-v2 table for ICE device
> >>>>>>> and aquire its minimum and maximum frequency during ICE
> >>>>>>> device probe.
> >>
> >> [...]
> >>
> >>>>> However, my main concern was for the corner cases, where:
> >>>>> (target_freq > max && ROUND_CEIL)
> >>>>> and
> >>>>> (target_freq < min && ROUND_FLOOR)
> >>>>> In both the cases, the OPP APIs will fail and the clock remains unchanged.
> >>>>
> >>>> I would argue that's expected behavior, if the requested rate can not
> >>>> be achieved, the "set_rate"-like function should fail
> >>>>
> >>>>> Hence, I added the checks to make the API as generic/robust as possible.
> >>>>
> >>>> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
> >>>> play, but the latter never goes above the FMAX of the former
> >>>>
> >>>> For the second case, I'm not sure it's valid. For "find lowest rate" I would
> >>>> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
> >>>> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
> >>>> so I'm not sure _floor() is useful
> >>>
> >>> Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
> >>> and ice-clk >= storage-clk in case of scale_down.
> >>
> >> I don't quite understand the first case (ice <= storage for scale_up), could you
> >> please elaborate?
> > 
> > Here I basically mean to say is that, as you mentioned "we generally set
> > storage_ctrl_rate == ice_clk_rate, but latter never goes above the FMAX of the former".
> > I guess, the ideal way to handle this is to ensure using _floor when we want to scale_up.
> > This ensures the ice_clk does not vote for more that what storage_ctrl is running on.
> 
> Right, but what I was asking specifically is why we don't want that to happen

I would argue saying that, having ice_clk higher than storage_ctrl_clk does
not makes sense, as it will not improve the throughput since the controller
clock rate will still be a bottle-neck and it will surely drain more power.
 
> > Also, this avoids the corner case, where target_freq provided is higher that the supporter
> > rates (descriped in ICE OPP-table) for ICE, using _ceil makes no sense.
> 
> This is potentially a valid concern, do we have cases of storage_clk > ice_clk?

As of now, on the UFS storages (targets KLMT) I dont see the
storage_clk (FMAX) > ice_clk (FMAX). They are mostly equal.
However, I am not sure, about all the other targets and cannot
call the same will persist on the upcommings as well.

Abhinaba Rakshit

Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE

[Konrad Dybcio]

On 2/20/26 12:15 PM, Abhinaba Rakshit wrote:
> On Fri, Feb 20, 2026 at 10:42:58AM +0100, Konrad Dybcio wrote:
>> On 2/20/26 8:33 AM, Abhinaba Rakshit wrote:
>>> On Thu, Feb 19, 2026 at 03:20:31PM +0100, Konrad Dybcio wrote:
>>>> On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:
>>>>> On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:
>>>>>> On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
>>>>>>> On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:
>>>>>>>> On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:
>>>>>>>>> Register optional operation-points-v2 table for ICE device
>>>>>>>>> and aquire its minimum and maximum frequency during ICE
>>>>>>>>> device probe.
>>>>
>>>> [...]
>>>>
>>>>>>> However, my main concern was for the corner cases, where:
>>>>>>> (target_freq > max && ROUND_CEIL)
>>>>>>> and
>>>>>>> (target_freq < min && ROUND_FLOOR)
>>>>>>> In both the cases, the OPP APIs will fail and the clock remains unchanged.
>>>>>>
>>>>>> I would argue that's expected behavior, if the requested rate can not
>>>>>> be achieved, the "set_rate"-like function should fail
>>>>>>
>>>>>>> Hence, I added the checks to make the API as generic/robust as possible.
>>>>>>
>>>>>> AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight
>>>>>> play, but the latter never goes above the FMAX of the former
>>>>>>
>>>>>> For the second case, I'm not sure it's valid. For "find lowest rate" I would
>>>>>> expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect
>>>>>> that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway
>>>>>> so I'm not sure _floor() is useful
>>>>>
>>>>> Clear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up
>>>>> and ice-clk >= storage-clk in case of scale_down.
>>>>
>>>> I don't quite understand the first case (ice <= storage for scale_up), could you
>>>> please elaborate?
>>>
>>> Here I basically mean to say is that, as you mentioned "we generally set
>>> storage_ctrl_rate == ice_clk_rate, but latter never goes above the FMAX of the former".
>>> I guess, the ideal way to handle this is to ensure using _floor when we want to scale_up.
>>> This ensures the ice_clk does not vote for more that what storage_ctrl is running on.
>>
>> Right, but what I was asking specifically is why we don't want that to happen
> 
> I would argue saying that, having ice_clk higher than storage_ctrl_clk does
> not makes sense, as it will not improve the throughput since the controller
> clock rate will still be a bottle-neck and it will surely drain more power.

Got it

Konrad