[PATCH 0/2] Enable UFS ICE clock scaling

[PATCH 0/2] Enable UFS ICE clock scaling

[Abhinaba Rakshit]

This API enables dynamic scaling of the ICE (Inline Crypto Engine) clock,
which is tightly integrated with the host controller. It is invoked by the UFS 
host controller driver in response to clock scaling requests, ensuring
coordination between ICE and the host controller.

This API helps prevent degradation in storage read/write KPIs,
maintaining consistent I/O throughput performance.

The implementation has been tested using tiotest to verify that enabling ICE
does not negatively impact host controller I/O performance during
read/write operations.

Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
---
Abhinaba Rakshit (2):
      soc: qcom: ice: enable ICE clock scaling API
      ufs: host: scale ICE clock

 drivers/soc/qcom/ice.c      | 25 +++++++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
 include/soc/qcom/ice.h      |  1 +
 3 files changed, 40 insertions(+)
---
base-commit: 3b9b1f8df454caa453c7fb07689064edb2eda90a
change-id: 20251001-enable-ufs-ice-clock-scaling-9c55598295f6

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

[PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Abhinaba Rakshit]

Add ICE clock scaling API based on the parsed clk supported
frequencies from dt entry.

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

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index c467b55b41744ebec0680f5112cc4bb1ba00c513..ec8d6bb9f426deee1038616282176bfc8e5b9ec1 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -97,6 +97,8 @@ struct qcom_ice {
 	struct clk *core_clk;
 	bool use_hwkm;
 	bool hwkm_init_complete;
+	u32 max_freq;
+	u32 min_freq;
 };
 
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
@@ -514,10 +516,25 @@ int qcom_ice_import_key(struct qcom_ice *ice,
 }
 EXPORT_SYMBOL_GPL(qcom_ice_import_key);
 
+int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
+{
+	int ret = 0;
+
+	if (scale_up && ice->max_freq)
+		ret = clk_set_rate(ice->core_clk, ice->max_freq);
+	else if (!scale_up && ice->min_freq)
+		ret = clk_set_rate(ice->core_clk, ice->min_freq);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
+
 static struct qcom_ice *qcom_ice_create(struct device *dev,
 					void __iomem *base)
 {
 	struct qcom_ice *engine;
+	const __be32 *prop;
+	int len;
 
 	if (!qcom_scm_is_available())
 		return ERR_PTR(-EPROBE_DEFER);
@@ -549,6 +566,14 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 	if (IS_ERR(engine->core_clk))
 		return ERR_CAST(engine->core_clk);
 
+	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
+	if (!prop || len < 2 * sizeof(uint32_t)) {
+		dev_err(dev, "Freq-hz property not found or invalid length\n");
+	} else {
+		engine->min_freq = be32_to_cpu(prop[0]);
+		engine->max_freq = be32_to_cpu(prop[1]);
+	}
+
 	if (!qcom_ice_check_supported(engine))
 		return ERR_PTR(-EOPNOTSUPP);
 
diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..b701ec9e062f70152f6dea8bf6c4637ab6ef20f1 100644
--- a/include/soc/qcom/ice.h
+++ b/include/soc/qcom/ice.h
@@ -30,5 +30,6 @@ int qcom_ice_import_key(struct qcom_ice *ice,
 			const u8 *raw_key, size_t raw_key_size,
 			u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
 struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
+int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up);
 
 #endif /* __QCOM_ICE_H__ */

-- 
2.34.1

[PATCH 2/2] ufs: host: scale ICE clock

[Abhinaba Rakshit]

Scale ICE clock from ufs controller.

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

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -305,6 +305,13 @@ 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, bool scale_up)
+{
+	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
+		return qcom_ice_scale_clk(host->ice, scale_up);
+	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
 {
 }
 
+static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
+{
+	return 0;
+}
+
 #endif
 
 static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
@@ -1636,6 +1648,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, scale_up);
 
 		if (err) {
 			ufshcd_uic_hibern8_exit(hba);

-- 
2.34.1

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Bryan O'Donoghue]

On 01/10/2025 12:38, Abhinaba Rakshit wrote:
> Add ICE clock scaling API based on the parsed clk supported
> frequencies from dt entry.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>   drivers/soc/qcom/ice.c | 25 +++++++++++++++++++++++++
>   include/soc/qcom/ice.h |  1 +
>   2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index c467b55b41744ebec0680f5112cc4bb1ba00c513..ec8d6bb9f426deee1038616282176bfc8e5b9ec1 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -97,6 +97,8 @@ struct qcom_ice {
>   	struct clk *core_clk;
>   	bool use_hwkm;
>   	bool hwkm_init_complete;
> +	u32 max_freq;
> +	u32 min_freq;
>   };
> 
>   static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -514,10 +516,25 @@ int qcom_ice_import_key(struct qcom_ice *ice,
>   }
>   EXPORT_SYMBOL_GPL(qcom_ice_import_key);
> 
> +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
> +{
> +	int ret = 0;
> +
> +	if (scale_up && ice->max_freq)
> +		ret = clk_set_rate(ice->core_clk, ice->max_freq);
> +	else if (!scale_up && ice->min_freq)
> +		ret = clk_set_rate(ice->core_clk, ice->min_freq);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> +
>   static struct qcom_ice *qcom_ice_create(struct device *dev,
>   					void __iomem *base)
>   {
>   	struct qcom_ice *engine;
> +	const __be32 *prop;
> +	int len;
> 
>   	if (!qcom_scm_is_available())
>   		return ERR_PTR(-EPROBE_DEFER);
> @@ -549,6 +566,14 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>   	if (IS_ERR(engine->core_clk))
>   		return ERR_CAST(engine->core_clk);
> 
> +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> +	if (!prop || len < 2 * sizeof(uint32_t)) {
> +		dev_err(dev, "Freq-hz property not found or invalid length\n");

If this error really happened you should pus the result code up the call 
stack also since either case can be an error you can inform the user of 
which error happened in your output string.

> +	} else {
> +		engine->min_freq = be32_to_cpu(prop[0]);
> +		engine->max_freq = be32_to_cpu(prop[1]);

You check for zero later on in the code but, is zero a valid value to be 
returned here ?

e.g. is it valid to specify "freq-table-hz" in your DT but then set max 
to zero ? min to zero ?

If not you may as well reject zero and dispense with the checks later on.

> +	}
> +
>   	if (!qcom_ice_check_supported(engine))
>   		return ERR_PTR(-EOPNOTSUPP);
> 
> diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..b701ec9e062f70152f6dea8bf6c4637ab6ef20f1 100644
> --- a/include/soc/qcom/ice.h
> +++ b/include/soc/qcom/ice.h
> @@ -30,5 +30,6 @@ int qcom_ice_import_key(struct qcom_ice *ice,
>   			const u8 *raw_key, size_t raw_key_size,
>   			u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
>   struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
> +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up);
> 
>   #endif /* __QCOM_ICE_H__ */
> 
> --
> 2.34.1
> 
> 

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Bryan O'Donoghue]

On 01/10/2025 12:38, Abhinaba Rakshit wrote:
> Scale ICE clock from ufs controller.

UFS

> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>   drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -305,6 +305,13 @@ 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, bool scale_up)
> +{
> +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> +		return qcom_ice_scale_clk(host->ice, scale_up);
> +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
>   {
>   }
> 
> +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
> +{
> +	return 0;
> +}
> +
>   #endif
> 
>   static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> @@ -1636,6 +1648,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, scale_up);
> 
>   		if (err) {
>   			ufshcd_uic_hibern8_exit(hba);
> 
> --
> 2.34.1
> 
> 

Once fixed.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Bjorn Andersson]

On Wed, Oct 01, 2025 at 05:08:20PM +0530, Abhinaba Rakshit wrote:
> Scale ICE clock from ufs controller.
> 

This isn't a good commit message.

Regards,
Bjorn

> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -305,6 +305,13 @@ 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, bool scale_up)
> +{
> +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> +		return qcom_ice_scale_clk(host->ice, scale_up);
> +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
>  {
>  }
>  
> +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> @@ -1636,6 +1648,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, scale_up);
>  
>  		if (err) {
>  			ufshcd_uic_hibern8_exit(hba);
> 
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Manivannan Sadhasivam]

On Wed, Oct 01, 2025 at 05:08:19PM +0530, Abhinaba Rakshit wrote:
> Add ICE clock scaling API based on the parsed clk supported
> frequencies from dt entry.
> 

Explain the purpose.

> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/ice.c | 25 +++++++++++++++++++++++++
>  include/soc/qcom/ice.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> index c467b55b41744ebec0680f5112cc4bb1ba00c513..ec8d6bb9f426deee1038616282176bfc8e5b9ec1 100644
> --- a/drivers/soc/qcom/ice.c
> +++ b/drivers/soc/qcom/ice.c
> @@ -97,6 +97,8 @@ struct qcom_ice {
>  	struct clk *core_clk;
>  	bool use_hwkm;
>  	bool hwkm_init_complete;
> +	u32 max_freq;
> +	u32 min_freq;
>  };
>  
>  static bool qcom_ice_check_supported(struct qcom_ice *ice)
> @@ -514,10 +516,25 @@ int qcom_ice_import_key(struct qcom_ice *ice,
>  }
>  EXPORT_SYMBOL_GPL(qcom_ice_import_key);
>  
> +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
> +{
> +	int ret = 0;
> +
> +	if (scale_up && ice->max_freq)
> +		ret = clk_set_rate(ice->core_clk, ice->max_freq);
> +	else if (!scale_up && ice->min_freq)
> +		ret = clk_set_rate(ice->core_clk, ice->min_freq);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> +
>  static struct qcom_ice *qcom_ice_create(struct device *dev,
>  					void __iomem *base)
>  {
>  	struct qcom_ice *engine;
> +	const __be32 *prop;
> +	int len;
>  
>  	if (!qcom_scm_is_available())
>  		return ERR_PTR(-EPROBE_DEFER);
> @@ -549,6 +566,14 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
>  	if (IS_ERR(engine->core_clk))
>  		return ERR_CAST(engine->core_clk);
>  
> +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> +	if (!prop || len < 2 * sizeof(uint32_t)) {
> +		dev_err(dev, "Freq-hz property not found or invalid length\n");

We have deprecated the 'freq-table-hz' property in favor of
'operating-points-v2'. So you should not be using it in new code. Also, throwing
error in the absence of this property is a no-go.

- Mani

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

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Manivannan Sadhasivam]

On Wed, Oct 01, 2025 at 05:08:20PM +0530, Abhinaba Rakshit wrote:
> Scale ICE clock from ufs controller.
> 

Explain the purpose.

> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -305,6 +305,13 @@ 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, bool scale_up)
> +{
> +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> +		return qcom_ice_scale_clk(host->ice, scale_up);

newline

> +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
>  {

> +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)

Drop the 'inline' keyword.

- Mani

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

Re: [PATCH 0/2] Enable UFS ICE clock scaling

[Manivannan Sadhasivam]

On Wed, Oct 01, 2025 at 05:08:18PM +0530, Abhinaba Rakshit wrote:
> This API enables dynamic scaling of the ICE (Inline Crypto Engine) clock,

Which API?

> which is tightly integrated with the host controller. It is invoked by the UFS 
> host controller driver in response to clock scaling requests, ensuring
> coordination between ICE and the host controller.
> 
> This API helps prevent degradation in storage read/write KPIs,
> maintaining consistent I/O throughput performance.

I'd expect clock scaling to save some power, than 'preventing degradation in
performance'.

> 
> The implementation has been tested using tiotest to verify that enabling ICE
> does not negatively impact host controller I/O performance during
> read/write operations.
> 

On which platform?

- Mani

> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---
> Abhinaba Rakshit (2):
>       soc: qcom: ice: enable ICE clock scaling API
>       ufs: host: scale ICE clock
> 
>  drivers/soc/qcom/ice.c      | 25 +++++++++++++++++++++++++
>  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>  include/soc/qcom/ice.h      |  1 +
>  3 files changed, 40 insertions(+)
> ---
> base-commit: 3b9b1f8df454caa453c7fb07689064edb2eda90a
> change-id: 20251001-enable-ufs-ice-clock-scaling-9c55598295f6
> 
> Best regards,
> -- 
> Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> 

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

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Konrad Dybcio]

On 10/1/25 1:38 PM, Abhinaba Rakshit wrote:
> Add ICE clock scaling API based on the parsed clk supported
> frequencies from dt entry.
> 
> Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> ---

[...]

> +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> +	if (!prop || len < 2 * sizeof(uint32_t)) {
> +		dev_err(dev, "Freq-hz property not found or invalid length\n");
> +	} else {
> +		engine->min_freq = be32_to_cpu(prop[0]);
> +		engine->max_freq = be32_to_cpu(prop[1]);
> +	}

As I suggested in <fca8355e-9b34-4df1-a7e6-459bdad8b1ff@oss.qualcomm.com>,
you should really use an OPP table if you want to do any sort of clock
scaling here.

There are then nice APIs associated with that construct that won't make
you pull your hair out..

Konrad

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Abhinaba Rakshit]

On Thu, Oct 02, 2025 at 12:23:37AM +0000, Bryan O'Donoghue wrote:
> On 01/10/2025 12:38, Abhinaba Rakshit wrote:
> > Scale ICE clock from ufs controller.
> 
> UFS

Sure, will take car of it in patchset v2.

> 
> > 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >   drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -305,6 +305,13 @@ 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, bool scale_up)
> > +{
> > +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> > +		return qcom_ice_scale_clk(host->ice, scale_up);
> > +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> >   {
> >   }
> > 
> > +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
> > +{
> > +	return 0;
> > +}
> > +
> >   #endif
> > 
> >   static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> > @@ -1636,6 +1648,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, scale_up);
> > 
> >   		if (err) {
> >   			ufshcd_uic_hibern8_exit(hba);
> > 
> > --
> > 2.34.1
> > 
> > 
> 
> Once fixed.

Sure, we can bubble up the error log here. Will update in patchset v2.

> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Abhinaba Rakshit]

On Wed, Oct 01, 2025 at 10:15:27PM -0500, Bjorn Andersson wrote:
> On Wed, Oct 01, 2025 at 05:08:20PM +0530, Abhinaba Rakshit wrote:
> > Scale ICE clock from ufs controller.
> > 
> 
> This isn't a good commit message.

Sure, will add more details in patchset v2.

> 
> Regards,
> Bjorn
> 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -305,6 +305,13 @@ 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, bool scale_up)
> > +{
> > +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> > +		return qcom_ice_scale_clk(host->ice, scale_up);
> > +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> >  {
> >  }
> >  
> > +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif
> >  
> >  static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> > @@ -1636,6 +1648,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, scale_up);
> >  
> >  		if (err) {
> >  			ufshcd_uic_hibern8_exit(hba);
> > 
> > -- 
> > 2.34.1
> > 

Re: [PATCH 2/2] ufs: host: scale ICE clock

[Abhinaba Rakshit]

On Fri, Oct 03, 2025 at 10:14:44PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 01, 2025 at 05:08:20PM +0530, Abhinaba Rakshit wrote:
> > Scale ICE clock from ufs controller.
> > 
> 
> Explain the purpose.

Sure, will add more details in patchset v2.

> 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 3e83dc51d53857d5a855df4e4dfa837747559dad..2964b95a4423e887c0414ed9399cc02d37b5229a 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -305,6 +305,13 @@ 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, bool scale_up)
> > +{
> > +	if (host->hba->caps & UFSHCD_CAP_CRYPTO)
> > +		return qcom_ice_scale_clk(host->ice, scale_up);
> 
> newline
> 
> > +	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 +346,11 @@ static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> >  {
> 
> > +static inline int ufs_qcom_ice_scale_clk(struct ufs_qcom_host *host, bool scale_up)
> 
> Drop the 'inline' keyword.

Will update this in patchset v2.

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

Re: [PATCH 0/2] Enable UFS ICE clock scaling

[Abhinaba Rakshit]

On Fri, Oct 03, 2025 at 10:18:15PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 01, 2025 at 05:08:18PM +0530, Abhinaba Rakshit wrote:
> > This API enables dynamic scaling of the ICE (Inline Crypto Engine) clock,
> 
> Which API?

Will keep it more specific and update in patchset v2.

> 
> > which is tightly integrated with the host controller. It is invoked by the UFS 
> > host controller driver in response to clock scaling requests, ensuring
> > coordination between ICE and the host controller.
> > 
> > This API helps prevent degradation in storage read/write KPIs,
> > maintaining consistent I/O throughput performance.
> 
> I'd expect clock scaling to save some power, than 'preventing degradation in
> performance'.

The solution should achieve both objectives: maintaining functionality while
dynamically downscaling the clock during periods of low read/write activity to
reduce power consumption.
Will update with more details in patchset v2.

> 
> > 
> > The implementation has been tested using tiotest to verify that enabling ICE
> > does not negatively impact host controller I/O performance during
> > read/write operations.
> > 
> 
> On which platform?

Currently we have tested on Kodiak and Talos, with additional device-tree changes.
Will also update the tests details better in patchset v2.

> 
> - Mani
> 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> > Abhinaba Rakshit (2):
> >       soc: qcom: ice: enable ICE clock scaling API
> >       ufs: host: scale ICE clock
> > 
> >  drivers/soc/qcom/ice.c      | 25 +++++++++++++++++++++++++
> >  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
> >  include/soc/qcom/ice.h      |  1 +
> >  3 files changed, 40 insertions(+)
> > ---
> > base-commit: 3b9b1f8df454caa453c7fb07689064edb2eda90a
> > change-id: 20251001-enable-ufs-ice-clock-scaling-9c55598295f6
> > 
> > Best regards,
> > -- 
> > Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Abhinaba Rakshit]

On Thu, Oct 02, 2025 at 01:21:22AM +0100, Bryan O'Donoghue wrote:
> On 01/10/2025 12:38, Abhinaba Rakshit wrote:
> > Add ICE clock scaling API based on the parsed clk supported
> > frequencies from dt entry.
> > 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >   drivers/soc/qcom/ice.c | 25 +++++++++++++++++++++++++
> >   include/soc/qcom/ice.h |  1 +
> >   2 files changed, 26 insertions(+)
> > 
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > index c467b55b41744ebec0680f5112cc4bb1ba00c513..ec8d6bb9f426deee1038616282176bfc8e5b9ec1 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -97,6 +97,8 @@ struct qcom_ice {
> >   	struct clk *core_clk;
> >   	bool use_hwkm;
> >   	bool hwkm_init_complete;
> > +	u32 max_freq;
> > +	u32 min_freq;
> >   };
> > 
> >   static bool qcom_ice_check_supported(struct qcom_ice *ice)
> > @@ -514,10 +516,25 @@ int qcom_ice_import_key(struct qcom_ice *ice,
> >   }
> >   EXPORT_SYMBOL_GPL(qcom_ice_import_key);
> > 
> > +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
> > +{
> > +	int ret = 0;
> > +
> > +	if (scale_up && ice->max_freq)
> > +		ret = clk_set_rate(ice->core_clk, ice->max_freq);
> > +	else if (!scale_up && ice->min_freq)
> > +		ret = clk_set_rate(ice->core_clk, ice->min_freq);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> > +
> >   static struct qcom_ice *qcom_ice_create(struct device *dev,
> >   					void __iomem *base)
> >   {
> >   	struct qcom_ice *engine;
> > +	const __be32 *prop;
> > +	int len;
> > 
> >   	if (!qcom_scm_is_available())
> >   		return ERR_PTR(-EPROBE_DEFER);
> > @@ -549,6 +566,14 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >   	if (IS_ERR(engine->core_clk))
> >   		return ERR_CAST(engine->core_clk);
> > 
> > +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> > +	if (!prop || len < 2 * sizeof(uint32_t)) {
> > +		dev_err(dev, "Freq-hz property not found or invalid length\n");
> 
> If this error really happened you should pus the result code up the call
> stack also since either case can be an error you can inform the user of
> which error happened in your output string.

Okay.
However, we will move to using OPP table and hence, this logic might change.

> 
> > +	} else {
> > +		engine->min_freq = be32_to_cpu(prop[0]);
> > +		engine->max_freq = be32_to_cpu(prop[1]);
> 
> You check for zero later on in the code but, is zero a valid value to be
> returned here ?
> 
> e.g. is it valid to specify "freq-table-hz" in your DT but then set max to
> zero ? min to zero ?
> 
> If not you may as well reject zero and dispense with the checks later on.

Yes, zero is valid here (means "no scaling"), will document it in patchset v2. 

> 
> > +	}
> > +
> >   	if (!qcom_ice_check_supported(engine))
> >   		return ERR_PTR(-EOPNOTSUPP);
> > 
> > diff --git a/include/soc/qcom/ice.h b/include/soc/qcom/ice.h
> > index 4bee553f0a59d86ec6ce20f7c7b4bce28a706415..b701ec9e062f70152f6dea8bf6c4637ab6ef20f1 100644
> > --- a/include/soc/qcom/ice.h
> > +++ b/include/soc/qcom/ice.h
> > @@ -30,5 +30,6 @@ int qcom_ice_import_key(struct qcom_ice *ice,
> >   			const u8 *raw_key, size_t raw_key_size,
> >   			u8 lt_key[BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE]);
> >   struct qcom_ice *devm_of_qcom_ice_get(struct device *dev);
> > +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up);
> > 
> >   #endif /* __QCOM_ICE_H__ */
> > 
> > --
> > 2.34.1
> > 
> > 
> 

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Abhinaba Rakshit]

On Fri, Oct 03, 2025 at 10:10:28PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 01, 2025 at 05:08:19PM +0530, Abhinaba Rakshit wrote:
> > Add ICE clock scaling API based on the parsed clk supported
> > frequencies from dt entry.
> > 
> 
> Explain the purpose.
> 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> >  drivers/soc/qcom/ice.c | 25 +++++++++++++++++++++++++
> >  include/soc/qcom/ice.h |  1 +
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
> > index c467b55b41744ebec0680f5112cc4bb1ba00c513..ec8d6bb9f426deee1038616282176bfc8e5b9ec1 100644
> > --- a/drivers/soc/qcom/ice.c
> > +++ b/drivers/soc/qcom/ice.c
> > @@ -97,6 +97,8 @@ struct qcom_ice {
> >  	struct clk *core_clk;
> >  	bool use_hwkm;
> >  	bool hwkm_init_complete;
> > +	u32 max_freq;
> > +	u32 min_freq;
> >  };
> >  
> >  static bool qcom_ice_check_supported(struct qcom_ice *ice)
> > @@ -514,10 +516,25 @@ int qcom_ice_import_key(struct qcom_ice *ice,
> >  }
> >  EXPORT_SYMBOL_GPL(qcom_ice_import_key);
> >  
> > +int qcom_ice_scale_clk(struct qcom_ice *ice, bool scale_up)
> > +{
> > +	int ret = 0;
> > +
> > +	if (scale_up && ice->max_freq)
> > +		ret = clk_set_rate(ice->core_clk, ice->max_freq);
> > +	else if (!scale_up && ice->min_freq)
> > +		ret = clk_set_rate(ice->core_clk, ice->min_freq);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_ice_scale_clk);
> > +
> >  static struct qcom_ice *qcom_ice_create(struct device *dev,
> >  					void __iomem *base)
> >  {
> >  	struct qcom_ice *engine;
> > +	const __be32 *prop;
> > +	int len;
> >  
> >  	if (!qcom_scm_is_available())
> >  		return ERR_PTR(-EPROBE_DEFER);
> > @@ -549,6 +566,14 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
> >  	if (IS_ERR(engine->core_clk))
> >  		return ERR_CAST(engine->core_clk);
> >  
> > +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> > +	if (!prop || len < 2 * sizeof(uint32_t)) {
> > +		dev_err(dev, "Freq-hz property not found or invalid length\n");
> 
> We have deprecated the 'freq-table-hz' property in favor of
> 'operating-points-v2'. So you should not be using it in new code. Also, throwing
> error in the absence of this property is a no-go.

Sure, will move the soultion using OPP-table in patchset v2.

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

Re: [PATCH 1/2] soc: qcom: ice: enable ICE clock scaling API

[Abhinaba Rakshit]

On Mon, Oct 06, 2025 at 12:14:57PM +0200, Konrad Dybcio wrote:
> On 10/1/25 1:38 PM, Abhinaba Rakshit wrote:
> > Add ICE clock scaling API based on the parsed clk supported
> > frequencies from dt entry.
> > 
> > Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > +	prop = of_get_property(dev->of_node, "freq-table-hz", &len);
> > +	if (!prop || len < 2 * sizeof(uint32_t)) {
> > +		dev_err(dev, "Freq-hz property not found or invalid length\n");
> > +	} else {
> > +		engine->min_freq = be32_to_cpu(prop[0]);
> > +		engine->max_freq = be32_to_cpu(prop[1]);
> > +	}
> 
> As I suggested in <fca8355e-9b34-4df1-a7e6-459bdad8b1ff@oss.qualcomm.com>,
> you should really use an OPP table if you want to do any sort of clock
> scaling here.
> 
> There are then nice APIs associated with that construct that won't make
> you pull your hair out..

Sure, will move the solution to patchset v2.

> 
> Konrad