public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Enable ICE clock scaling
@ 2026-01-28  8:46 Abhinaba Rakshit
  2026-01-28  8:46 ` [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE Abhinaba Rakshit
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-01-28  8:46 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree,
	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 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 |  29 ++++++
 drivers/soc/qcom/ice.c                             | 112 +++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.c                        |  17 ++++
 include/soc/qcom/ice.h                             |   5 +
 4 files changed, 163 insertions(+)
---
base-commit: fe4d0dea039f2befb93f27569593ec209843b0f5
change-id: 20251120-enable-ufs-ice-clock-scaling-b063caf3e6f9

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE
  2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
@ 2026-01-28  8:46 ` Abhinaba Rakshit
  2026-01-28 11:00   ` Krzysztof Kozlowski
  2026-01-28  8:46 ` [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE Abhinaba Rakshit
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-01-28  8:46 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree,
	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. OPP-tabel is kept as an optional property.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
 .../bindings/crypto/qcom,inline-crypto-engine.yaml | 29 ++++++++++++++++++++++
 1 file changed, 29 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..1e849def1e0078feb45874a436411188d26cf37f 100644
--- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
@@ -30,6 +30,14 @@ properties:
   clocks:
     maxItems: 1
 
+  operating-points-v2:
+    description:
+      Each OPP entry contains the frequency configuration for the ICE device
+      clock(s).
+
+  opp-table:
+    type: object
+
 required:
   - compatible
   - reg
@@ -46,5 +54,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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
  2026-01-28  8:46 ` [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE Abhinaba Rakshit
@ 2026-01-28  8:46 ` Abhinaba Rakshit
  2026-01-28 11:04   ` Krzysztof Kozlowski
  2026-01-28  8:46 ` [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes Abhinaba Rakshit
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-01-28  8:46 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree,
	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.

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

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cadd21d6f96eb1799963a13db4b2b72..90106186c15e644527fdf75a186a2e8adeb299a3 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,9 @@ struct qcom_ice {
 	bool use_hwkm;
 	bool hwkm_init_complete;
 	u8 hwkm_version;
+	unsigned long max_freq;
+	unsigned long min_freq;
+	bool has_opp;
 };
 
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -549,10 +553,73 @@ 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)
+{
+	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:
+
+	return dev_pm_opp_set_rate(ice->dev, ice_freq);
+}
+EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
+
 static struct qcom_ice *qcom_ice_create(struct device *dev,
 					void __iomem *base)
 {
 	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 +651,46 @@ 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 (of_device_is_compatible(dev->of_node, "qcom,inline-crypto-engine")) {
+		/* 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);
+		}
+	}
+
 	if (!qcom_ice_check_supported(engine))
 		return ERR_PTR(-EOPNOTSUPP);
 
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes
  2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
  2026-01-28  8:46 ` [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE Abhinaba Rakshit
  2026-01-28  8:46 ` [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE Abhinaba Rakshit
@ 2026-01-28  8:46 ` Abhinaba Rakshit
  2026-01-28 13:59   ` Manivannan Sadhasivam
  2026-01-28  8:46 ` [PATCH v4 4/4] soc: qcom: ice: Set ICE clk to TURBO on probe Abhinaba Rakshit
  2026-01-29 12:17 ` [PATCH v4 0/4] Enable ICE clock scaling Konrad Dybcio
  4 siblings, 1 reply; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-01-28  8:46 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree,
	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.

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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 4/4] soc: qcom: ice: Set ICE clk to TURBO on probe
  2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
                   ` (2 preceding siblings ...)
  2026-01-28  8:46 ` [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes Abhinaba Rakshit
@ 2026-01-28  8:46 ` Abhinaba Rakshit
  2026-01-29 12:17 ` [PATCH v4 0/4] Enable ICE clock scaling Konrad Dybcio
  4 siblings, 0 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-01-28  8:46 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree,
	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 90106186c15e644527fdf75a186a2e8adeb299a3..2b0e577fb4c9ed9b746fe70ebccb45da9c52b006 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -689,6 +689,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");
 	}
 
 	if (!qcom_ice_check_supported(engine))

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE
  2026-01-28  8:46 ` [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE Abhinaba Rakshit
@ 2026-01-28 11:00   ` Krzysztof Kozlowski
  2026-02-02  6:37     ` Abhinaba Rakshit
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-28 11:00 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Wed, Jan 28, 2026 at 02:16:40PM +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. OPP-tabel is kept as an optional property.

Last two lines are redundant. Instead explain the hardware - why it did
not support clock scaling before?

> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  .../bindings/crypto/qcom,inline-crypto-engine.yaml | 29 ++++++++++++++++++++++
>  1 file changed, 29 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..1e849def1e0078feb45874a436411188d26cf37f 100644
> --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> @@ -30,6 +30,14 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  operating-points-v2:
> +    description:
> +      Each OPP entry contains the frequency configuration for the ICE device
> +      clock(s).

Drop description, please look how other bindings define this.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-01-28  8:46 ` [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE Abhinaba Rakshit
@ 2026-01-28 11:04   ` Krzysztof Kozlowski
  2026-02-02  6:32     ` Abhinaba Rakshit
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-28 11:04 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
>  	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 +651,46 @@ 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

This is not netdev...

> +	 * 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 (of_device_is_compatible(dev->of_node, "qcom,inline-crypto-engine")) {

Just add additional argument to qcom_ice_create().

> +		/* 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");
> +	}

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes
  2026-01-28  8:46 ` [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes Abhinaba Rakshit
@ 2026-01-28 13:59   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 20+ messages in thread
From: Manivannan Sadhasivam @ 2026-01-28 13:59 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, James E.J. Bottomley,
	Martin K. Petersen, Neeraj Soni, Herbert Xu, David S. Miller,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
	linux-kernel, linux-scsi, linux-crypto, devicetree

On Wed, Jan 28, 2026 at 02:16:42PM +0530, 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.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  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
> 

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
                   ` (3 preceding siblings ...)
  2026-01-28  8:46 ` [PATCH v4 4/4] soc: qcom: ice: Set ICE clk to TURBO on probe Abhinaba Rakshit
@ 2026-01-29 12:17 ` Konrad Dybcio
  2026-02-02  6:36   ` Abhinaba Rakshit
  4 siblings, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-01-29 12:17 UTC (permalink / raw)
  To: Abhinaba Rakshit, Bjorn Andersson, Konrad Dybcio,
	Manivannan Sadhasivam, James E.J. Bottomley, Martin K. Petersen,
	Neeraj Soni, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On 1/28/26 9:46 AM, 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 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.

You skipped that bit, so I had to do a little digging..

This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
would absolutely wreck the power/thermal profile of a running device,
however sdhci-msm's autosuspend functions quiesce the ICE by calling
qcom_ice_suspend()

I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
and a mirrored restore in _resume

Konrad


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-01-28 11:04   ` Krzysztof Kozlowski
@ 2026-02-02  6:32     ` Abhinaba Rakshit
  2026-02-02  9:23       ` Konrad Dybcio
  2026-02-05 11:26       ` Krzysztof Kozlowski
  0 siblings, 2 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-02  6:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Wed, Jan 28, 2026 at 12:04:26PM +0100, Krzysztof Kozlowski wrote:
> On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
> >  	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 +651,46 @@ 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
> 
> This is not netdev...

Okay, if I understand it correct, its not conventional to use of_device_is_compatible
outside netdev subsystem. Will update as mentioned below.

> 
> > +	 * 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 (of_device_is_compatible(dev->of_node, "qcom,inline-crypto-engine")) {
> 
> Just add additional argument to qcom_ice_create().

Sure, that makes more sense.
Will update in the next patchset.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-01-29 12:17 ` [PATCH v4 0/4] Enable ICE clock scaling Konrad Dybcio
@ 2026-02-02  6:36   ` Abhinaba Rakshit
  2026-02-02 15:01     ` Konrad Dybcio
  0 siblings, 1 reply; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-02  6:36 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Thu, Jan 29, 2026 at 01:17:51PM +0100, Konrad Dybcio wrote:
> On 1/28/26 9:46 AM, 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 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.
> 
> You skipped that bit, so I had to do a little digging..
> 
> This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
> would absolutely wreck the power/thermal profile of a running device,
> however sdhci-msm's autosuspend functions quiesce the ICE by calling
> qcom_ice_suspend()
> 
> I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
> and a mirrored restore in _resume

Thanks for pointing this out, its an important piece which is missed.
We can use dev_pm_opp_set_rate(dev, 0/min_freq) in _suspend and restore the
suspended frequency in the _resume. Something similar which is used by sdhci-msm.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE
  2026-01-28 11:00   ` Krzysztof Kozlowski
@ 2026-02-02  6:37     ` Abhinaba Rakshit
  0 siblings, 0 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-02  6:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Wed, Jan 28, 2026 at 12:00:57PM +0100, Krzysztof Kozlowski wrote:
> On Wed, Jan 28, 2026 at 02:16:40PM +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. OPP-tabel is kept as an optional property.
> 
> Last two lines are redundant. Instead explain the hardware - why it did
> not support clock scaling before?

Sure, will remove out redundant lines and update the commit message as requested.

> 
> > 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >  .../bindings/crypto/qcom,inline-crypto-engine.yaml | 29 ++++++++++++++++++++++
> >  1 file changed, 29 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..1e849def1e0078feb45874a436411188d26cf37f 100644
> > --- a/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> > +++ b/Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml
> > @@ -30,6 +30,14 @@ properties:
> >    clocks:
> >      maxItems: 1
> >  
> > +  operating-points-v2:
> > +    description:
> > +      Each OPP entry contains the frequency configuration for the ICE device
> > +      clock(s).
> 
> Drop description, please look how other bindings define this.

Sure, will check and update this.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-02-02  6:32     ` Abhinaba Rakshit
@ 2026-02-02  9:23       ` Konrad Dybcio
  2026-02-02  9:33         ` Abhinaba Rakshit
  2026-02-05 11:26       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-02-02  9:23 UTC (permalink / raw)
  To: Abhinaba Rakshit, Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On 2/2/26 7:32 AM, Abhinaba Rakshit wrote:
> On Wed, Jan 28, 2026 at 12:04:26PM +0100, Krzysztof Kozlowski wrote:
>> On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
>>>  	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 +651,46 @@ 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
>>
>> This is not netdev...
> 
> Okay, if I understand it correct, its not conventional to use of_device_is_compatible
> outside netdev subsystem. Will update as mentioned below.

In Linux

/*
 * This style of comments is generally preferred

unless

/* You're contributing to netdev for weird legacy reasons
 * that nobody seems to understand

Konrad


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-02-02  9:23       ` Konrad Dybcio
@ 2026-02-02  9:33         ` Abhinaba Rakshit
  0 siblings, 0 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-02  9:33 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio,
	Manivannan Sadhasivam, James E.J. Bottomley, Martin K. Petersen,
	Neeraj Soni, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
	linux-scsi, linux-crypto, devicetree

On Mon, Feb 02, 2026 at 10:23:43AM +0100, Konrad Dybcio wrote:
> On 2/2/26 7:32 AM, Abhinaba Rakshit wrote:
> > On Wed, Jan 28, 2026 at 12:04:26PM +0100, Krzysztof Kozlowski wrote:
> >> On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
> >>>  	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 +651,46 @@ 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
> >>
> >> This is not netdev...
> > 
> > Okay, if I understand it correct, its not conventional to use of_device_is_compatible
> > outside netdev subsystem. Will update as mentioned below.
> 
> In Linux
> 
> /*
>  * This style of comments is generally preferred
> 
> unless
> 
> /* You're contributing to netdev for weird legacy reasons
>  * that nobody seems to understand

I see. Thanks for correcting me.
Will keep this in mind while submitting subsequent patches. 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-02-02  6:36   ` Abhinaba Rakshit
@ 2026-02-02 15:01     ` Konrad Dybcio
  2026-02-06 12:03       ` Abhinaba Rakshit
  0 siblings, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-02-02 15:01 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On 2/2/26 7:36 AM, Abhinaba Rakshit wrote:
> On Thu, Jan 29, 2026 at 01:17:51PM +0100, Konrad Dybcio wrote:
>> On 1/28/26 9:46 AM, 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 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.
>>
>> You skipped that bit, so I had to do a little digging..
>>
>> This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
>> would absolutely wreck the power/thermal profile of a running device,
>> however sdhci-msm's autosuspend functions quiesce the ICE by calling
>> qcom_ice_suspend()
>>
>> I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
>> and a mirrored restore in _resume
> 
> Thanks for pointing this out, its an important piece which is missed.
> We can use dev_pm_opp_set_rate(dev, 0/min_freq) in _suspend and restore the

FWIW

dev_pm_opp_set_rate(0) will drop the rpmh vote altogether and NOT
disable the clock or change its rate

dev_pm_opp_set_rate(min_freq) will *lower* the rpmh vote and DO
set_rate (the clock is also left on)

Konrad

> suspended frequency in the _resume. Something similar which is used by sdhci-msm.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-02-02  6:32     ` Abhinaba Rakshit
  2026-02-02  9:23       ` Konrad Dybcio
@ 2026-02-05 11:26       ` Krzysztof Kozlowski
  2026-02-06 13:23         ` Abhinaba Rakshit
  1 sibling, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-05 11:26 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On 02/02/2026 07:32, Abhinaba Rakshit wrote:
> On Wed, Jan 28, 2026 at 12:04:26PM +0100, Krzysztof Kozlowski wrote:
>> On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
>>>  	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 +651,46 @@ 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
>>
>> This is not netdev...
> 
> Okay, if I understand it correct, its not conventional to use of_device_is_compatible
> outside netdev subsystem. Will update as mentioned below.

No, please read entire coding style, although the comment was about
comment style.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-02-02 15:01     ` Konrad Dybcio
@ 2026-02-06 12:03       ` Abhinaba Rakshit
  2026-02-06 12:14         ` Konrad Dybcio
  0 siblings, 1 reply; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-06 12:03 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Mon, Feb 02, 2026 at 04:01:38PM +0100, Konrad Dybcio wrote:
> On 2/2/26 7:36 AM, Abhinaba Rakshit wrote:
> > On Thu, Jan 29, 2026 at 01:17:51PM +0100, Konrad Dybcio wrote:
> >> On 1/28/26 9:46 AM, 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 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.
> >>
> >> You skipped that bit, so I had to do a little digging..
> >>
> >> This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
> >> would absolutely wreck the power/thermal profile of a running device,
> >> however sdhci-msm's autosuspend functions quiesce the ICE by calling
> >> qcom_ice_suspend()
> >>
> >> I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
> >> and a mirrored restore in _resume
> > 
> > Thanks for pointing this out, its an important piece which is missed.
> > We can use dev_pm_opp_set_rate(dev, 0/min_freq) in _suspend and restore the
> 
> FWIW
> 
> dev_pm_opp_set_rate(0) will drop the rpmh vote altogether and NOT
> disable the clock or change its rate
> 
> dev_pm_opp_set_rate(min_freq) will *lower* the rpmh vote and DO
> set_rate (the clock is also left on)
> 
> Konrad
>

Thanks for the info.
I guess, dev_pm_opp_set_rate(dev, 0) seems more ideal as this is
API is for full quiesce mode and the clocks are anyway gated in
the suspend call (clk_disable_unprepare).
 
Abhinaba Rakshit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-02-06 12:03       ` Abhinaba Rakshit
@ 2026-02-06 12:14         ` Konrad Dybcio
  2026-02-06 13:22           ` Abhinaba Rakshit
  0 siblings, 1 reply; 20+ messages in thread
From: Konrad Dybcio @ 2026-02-06 12:14 UTC (permalink / raw)
  To: Abhinaba Rakshit
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On 2/6/26 1:03 PM, Abhinaba Rakshit wrote:
> On Mon, Feb 02, 2026 at 04:01:38PM +0100, Konrad Dybcio wrote:
>> On 2/2/26 7:36 AM, Abhinaba Rakshit wrote:
>>> On Thu, Jan 29, 2026 at 01:17:51PM +0100, Konrad Dybcio wrote:
>>>> On 1/28/26 9:46 AM, 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 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.
>>>>
>>>> You skipped that bit, so I had to do a little digging..
>>>>
>>>> This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
>>>> would absolutely wreck the power/thermal profile of a running device,
>>>> however sdhci-msm's autosuspend functions quiesce the ICE by calling
>>>> qcom_ice_suspend()
>>>>
>>>> I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
>>>> and a mirrored restore in _resume
>>>
>>> Thanks for pointing this out, its an important piece which is missed.
>>> We can use dev_pm_opp_set_rate(dev, 0/min_freq) in _suspend and restore the
>>
>> FWIW
>>
>> dev_pm_opp_set_rate(0) will drop the rpmh vote altogether and NOT
>> disable the clock or change its rate
>>
>> dev_pm_opp_set_rate(min_freq) will *lower* the rpmh vote and DO
>> set_rate (the clock is also left on)
>>
>> Konrad
>>
> 
> Thanks for the info.
> I guess, dev_pm_opp_set_rate(dev, 0) seems more ideal as this is
> API is for full quiesce mode and the clocks are anyway gated in
> the suspend call (clk_disable_unprepare).

Yeah, please make sure to call dev_pm_opp_set_rate(0) *after* you
disable the clock though, to make sure we don't brownout

Konrad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/4] Enable ICE clock scaling
  2026-02-06 12:14         ` Konrad Dybcio
@ 2026-02-06 13:22           ` Abhinaba Rakshit
  0 siblings, 0 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-06 13:22 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Fri, Feb 06, 2026 at 01:14:28PM +0100, Konrad Dybcio wrote:
> On 2/6/26 1:03 PM, Abhinaba Rakshit wrote:
> > On Mon, Feb 02, 2026 at 04:01:38PM +0100, Konrad Dybcio wrote:
> >> On 2/2/26 7:36 AM, Abhinaba Rakshit wrote:
> >>> On Thu, Jan 29, 2026 at 01:17:51PM +0100, Konrad Dybcio wrote:
> >>>> On 1/28/26 9:46 AM, 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 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.
> >>>>
> >>>> You skipped that bit, so I had to do a little digging..
> >>>>
> >>>> This paragraph sounds scary on the surface, as leaving a TURBO vote hanging
> >>>> would absolutely wreck the power/thermal profile of a running device,
> >>>> however sdhci-msm's autosuspend functions quiesce the ICE by calling
> >>>> qcom_ice_suspend()
> >>>>
> >>>> I think you're missing a dev_pm_opp_set(dev, NULL) or so in that function
> >>>> and a mirrored restore in _resume
> >>>
> >>> Thanks for pointing this out, its an important piece which is missed.
> >>> We can use dev_pm_opp_set_rate(dev, 0/min_freq) in _suspend and restore the
> >>
> >> FWIW
> >>
> >> dev_pm_opp_set_rate(0) will drop the rpmh vote altogether and NOT
> >> disable the clock or change its rate
> >>
> >> dev_pm_opp_set_rate(min_freq) will *lower* the rpmh vote and DO
> >> set_rate (the clock is also left on)
> >>
> >> Konrad
> >>
> > 
> > Thanks for the info.
> > I guess, dev_pm_opp_set_rate(dev, 0) seems more ideal as this is
> > API is for full quiesce mode and the clocks are anyway gated in
> > the suspend call (clk_disable_unprepare).
> 
> Yeah, please make sure to call dev_pm_opp_set_rate(0) *after* you
> disable the clock though, to make sure we don't brownout

Sure, that makes sense.

Abhinaba Rakshit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
  2026-02-05 11:26       ` Krzysztof Kozlowski
@ 2026-02-06 13:23         ` Abhinaba Rakshit
  0 siblings, 0 replies; 20+ messages in thread
From: Abhinaba Rakshit @ 2026-02-06 13:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Manivannan Sadhasivam,
	James E.J. Bottomley, Martin K. Petersen, Neeraj Soni, Herbert Xu,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-msm, linux-kernel, linux-scsi, linux-crypto, devicetree

On Thu, Feb 05, 2026 at 12:26:25PM +0100, Krzysztof Kozlowski wrote:
> On 02/02/2026 07:32, Abhinaba Rakshit wrote:
> > On Wed, Jan 28, 2026 at 12:04:26PM +0100, Krzysztof Kozlowski wrote:
> >> On Wed, Jan 28, 2026 at 02:16:41PM +0530, Abhinaba Rakshit wrote:
> >>>  	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 +651,46 @@ 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
> >>
> >> This is not netdev...
> > 
> > Okay, if I understand it correct, its not conventional to use of_device_is_compatible
> > outside netdev subsystem. Will update as mentioned below.
> 
> No, please read entire coding style, although the comment was about
> comment style.

Sure, will ensure to use the correct comment styles.

Abhinaba Rakshit

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-02-06 13:23 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28  8:46 [PATCH v4 0/4] Enable ICE clock scaling Abhinaba Rakshit
2026-01-28  8:46 ` [PATCH v4 1/4] dt-bindings: crypto: ice: add operating-points-v2 property for QCOM ICE Abhinaba Rakshit
2026-01-28 11:00   ` Krzysztof Kozlowski
2026-02-02  6:37     ` Abhinaba Rakshit
2026-01-28  8:46 ` [PATCH v4 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE Abhinaba Rakshit
2026-01-28 11:04   ` Krzysztof Kozlowski
2026-02-02  6:32     ` Abhinaba Rakshit
2026-02-02  9:23       ` Konrad Dybcio
2026-02-02  9:33         ` Abhinaba Rakshit
2026-02-05 11:26       ` Krzysztof Kozlowski
2026-02-06 13:23         ` Abhinaba Rakshit
2026-01-28  8:46 ` [PATCH v4 3/4] ufs: host: Add ICE clock scaling during UFS clock changes Abhinaba Rakshit
2026-01-28 13:59   ` Manivannan Sadhasivam
2026-01-28  8:46 ` [PATCH v4 4/4] soc: qcom: ice: Set ICE clk to TURBO on probe Abhinaba Rakshit
2026-01-29 12:17 ` [PATCH v4 0/4] Enable ICE clock scaling Konrad Dybcio
2026-02-02  6:36   ` Abhinaba Rakshit
2026-02-02 15:01     ` Konrad Dybcio
2026-02-06 12:03       ` Abhinaba Rakshit
2026-02-06 12:14         ` Konrad Dybcio
2026-02-06 13:22           ` Abhinaba Rakshit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox