Add support for ICE algorithms for Qualcomm UFS V5.0 and above which
uses a pool of crypto cores for TX stream (UFS Write – Encryption)
and RX stream (UFS Read – Decryption).

Using these algorithms, crypto cores can be dynamically allocated
to either RX stream or TX stream based on algorithm selected.
Qualcomm UFS controller supports three ICE algorithms:
Floor based algorithm, Static Algorithm and Instantaneous algorithm
to share crypto cores between TX and RX stream.

Floor Based allocation is selected by default after power On or Reset.

Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
Co-developed-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Co-developed-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 232 ++++++++++++++++++++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h |  38 +++++-
 2 files changed, 269 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 810e637047d0..c0ca835f13f3 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
 }
 
 #ifdef CONFIG_SCSI_UFS_CRYPTO
+/*
+ * Default overrides:
+ * There're 10 sets of settings for floor-based algorithm
+ */
+static struct ice_alg2_config alg2_config[] = {
+	{"G0", {5, 12, 0, 0, 32, 0}},
+	{"G1", {12, 5, 32, 0, 0, 0}},
+	{"G2", {6, 11, 4, 1, 32, 1}},
+	{"G3", {6, 11, 7, 1, 32, 1}},
+	{"G4", {7, 10, 11, 1, 32, 1}},
+	{"G5", {7, 10, 14, 1, 32, 1}},
+	{"G6", {8, 9, 18, 1, 32, 1}},
+	{"G7", {9, 8, 21, 1, 32, 1}},
+	{"G8", {10, 7, 24, 1, 32, 1}},
+	{"G9", {10, 7, 32, 1, 32, 1}},
+};
+
+/**
+ * Refer struct ice_alg2_config
+ */
+static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t)
+{
+	*c = ((val[0] << 8) | val[1] | (1 << 31));
+	*t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]);
+}
+
+static inline void get_alg2_grp_params(unsigned int group, int *core, int *task)
+{
+	struct ice_alg2_config *p = &alg2_config[group];
+
+	 __get_alg2_grp_params(p->val, core, task);
+}
+
+/**
+ * ufs_qcom_ice_config_alg1 - Static ICE Algorithm
+ *
+ * @hba: host controller instance
+ * Return: zero for success and non-zero in case of a failure.
+ */
+static int ufs_qcom_ice_config_alg1(struct ufs_hba *hba)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	unsigned int val, rx_aes;
+	unsigned int num_aes_cores;
+	int ret;
+
+	ret = of_property_read_u32(host->ice_conf, "rx-alloc-percent", &val);
+	if (ret)
+		return ret;
+
+	num_aes_cores = ufshcd_readl(hba, REG_UFS_MEM_ICE_NUM_AES_CORES);
+	ufshcd_writel(hba, STATIC_ALLOC_ALG1, REG_UFS_MEM_ICE_CONFIG);
+
+	/*
+	 * DTS specifies the percent allocation to rx stream
+	 * Calculation -
+	 *  Num Tx stream = N_TOT - (N_TOT * percent of rx stream allocation)
+	 */
+	rx_aes = DIV_ROUND_CLOSEST(num_aes_cores * val, 100);
+	val = rx_aes | ((num_aes_cores - rx_aes) << 8);
+	ufshcd_writel(hba, val, REG_UFS_MEM_ICE_ALG1_NUM_CORE);
+
+	return 0;
+}
+
+/**
+ * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm
+ *
+ * @hba: host controller instance
+ * Return: zero for success and non-zero in case of a failure.
+ */
+static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0;
+	/* 6 values for each group, refer struct ice_alg2_config */
+	unsigned int override_val[ICE_ALG2_NUM_PARAMS];
+	char name[8] = {0};
+	int i, ret;
+
+	ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG);
+	for (i = 0; i < ARRAY_SIZE(alg2_config); i++) {
+		int core = 0, task = 0;
+
+		if (host->ice_conf) {
+			snprintf(name, sizeof(name), "%s%d", "g", i);
+			ret = of_property_read_variable_u32_array(host->ice_conf,
+								  name,
+								  override_val,
+								  ICE_ALG2_NUM_PARAMS,
+								  ICE_ALG2_NUM_PARAMS);
+			/* Some/All parameters may be overwritten */
+			if (ret > 0)
+				__get_alg2_grp_params(override_val, &core,
+						      &task);
+			else
+				get_alg2_grp_params(i, &core, &task);
+		} else {
+			get_alg2_grp_params(i, &core, &task);
+		}
+
+		/* Num Core and Num task are contiguous & configured for a group together */
+		ufshcd_writel(hba, core, reg);
+		reg += 4;
+		ufshcd_writel(hba, task, reg);
+		reg += 4;
+	}
+
+	return 0;
+}
+
+/**
+ * ufs_qcom_ice_config_alg3 - Instantaneous ICE Algorithm
+ *
+ * @hba: host controller instance
+ * Return: zero for success and non-zero in case of a failure.
+ */
+static int ufs_qcom_ice_config_alg3(struct ufs_hba *hba)
+{
+	unsigned int val[4];
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	unsigned int config;
+	int ret;
+
+	ret = of_property_read_variable_u32_array(host->ice_conf, "num-core", val,
+						  4, 4);
+	if (ret < 0)
+		return ret;
+
+	config = val[0] | (val[1] << 8) | (val[2] << 16) | (val[3] << 24);
+
+	ufshcd_writel(hba, INSTANTANEOUS_ALG3, REG_UFS_MEM_ICE_CONFIG);
+	ufshcd_writel(hba, config, REG_UFS_MEM_ICE_ALG3_NUM_CORE);
+
+	return 0;
+}
+
+static int ufs_qcom_parse_ice_config(struct ufs_hba *hba)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	struct device_node *np;
+	struct device_node *ice_np;
+	const char *alg_name;
+	int ret;
+
+	np = hba->dev->of_node;
+	if (!np)
+		return -ENOENT;
+
+	ice_np = of_get_next_available_child(np, NULL);
+	if (!ice_np)
+		return -ENOENT;
+
+	/* Only 1 algo can be enabled, pick the first */
+	host->ice_conf = of_get_next_available_child(ice_np, NULL);
+	if (!host->ice_conf) {
+		/* No overrides, use floor based as default */
+		host->chosen_algo = FLOOR_BASED_ALG2;
+		dev_info(hba->dev, "Resort to default ice alg2\n");
+		return 0;
+	}
+
+	ret = of_property_read_string(host->ice_conf, "alg-name", &alg_name);
+	if (ret < 0)
+		return ret;
+
+	if (!strcmp(alg_name, "alg1"))
+		host->chosen_algo = STATIC_ALLOC_ALG1;
+	else if (!strcmp(alg_name, "alg2"))
+		host->chosen_algo = FLOOR_BASED_ALG2;
+	else if (!strcmp(alg_name, "alg3"))
+		host->chosen_algo = INSTANTANEOUS_ALG3;
+	else {
+		dev_err(hba->dev, "Failed to find a valid ice alg name\n");
+		ret = -ENODATA;
+	}
+
+	return ret;
+}
+
+static int ufs_qcom_config_ice(struct ufs_qcom_host *host)
+{
+	if (!is_ice_config_supported(host))
+		return 0;
+
+	switch (host->chosen_algo) {
+	case STATIC_ALLOC_ALG1:
+		return ufs_qcom_ice_config_alg1(host->hba);
+	case FLOOR_BASED_ALG2:
+		return ufs_qcom_ice_config_alg2(host->hba);
+	case INSTANTANEOUS_ALG3:
+		return ufs_qcom_ice_config_alg3(host->hba);
+	}
+
+	return -EINVAL;
+}
+
+static int ufs_qcom_ice_config_init(struct ufs_qcom_host *host)
+{
+	struct ufs_hba *hba = host->hba;
+	int ret;
+
+	if (!is_ice_config_supported(host))
+		return 0;
+
+	ret = ufs_qcom_parse_ice_config(hba);
+	if (!ret)
+		dev_dbg(hba->dev, "ice config initialization success!!");
+
+	return ret;
+}
 
 static inline void ufs_qcom_ice_enable(struct ufs_qcom_host *host)
 {
@@ -117,6 +328,7 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host)
 	struct ufs_hba *hba = host->hba;
 	struct device *dev = hba->dev;
 	struct qcom_ice *ice;
+	int ret;
 
 	ice = of_qcom_ice_get(dev);
 	if (ice == ERR_PTR(-EOPNOTSUPP)) {
@@ -130,6 +342,10 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host)
 	host->ice = ice;
 	hba->caps |= UFSHCD_CAP_CRYPTO;
 
+	ret = ufs_qcom_ice_config_init(host);
+	if (ret)
+		dev_info(dev, "Continue with default ice configuration\n");
+
 	return 0;
 }
 
@@ -196,6 +412,12 @@ static inline int ufs_qcom_ice_suspend(struct ufs_qcom_host *host)
 {
 	return 0;
 }
+
+static int ufs_qcom_config_ice(struct ufs_qcom_host *host)
+{
+	return 0;
+}
+
 #endif
 
 static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
@@ -435,6 +657,11 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
 		err = ufs_qcom_enable_lane_clks(host);
 		break;
 	case POST_CHANGE:
+		err = ufs_qcom_config_ice(host);
+		if (err) {
+			dev_err(hba->dev, "failed to configure ice, ret=%d\n", err);
+			break;
+		}
 		/* check if UFS PHY moved from DISABLED to HIBERN8 */
 		err = ufs_qcom_check_hibern8(hba);
 		ufs_qcom_enable_hw_clk_gating(hba);
@@ -914,12 +1141,17 @@ static void ufs_qcom_set_host_params(struct ufs_hba *hba)
 
 static void ufs_qcom_set_caps(struct ufs_hba *hba)
 {
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
 	hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
 	hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING;
 	hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
 	hba->caps |= UFSHCD_CAP_WB_EN;
 	hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE;
 	hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
+
+	if (host->hw_ver.major >= 0x5)
+		host->caps |= UFS_QCOM_CAP_ICE_CONFIG;
 }
 
 /**
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index b9de170983c9..6884408eb807 100644
--- a/drivers/ufs/host/ufs-qcom.h
+++ b/drivers/ufs/host/ufs-qcom.h
@@ -195,8 +195,11 @@ struct ufs_qcom_host {
 
 #ifdef CONFIG_SCSI_UFS_CRYPTO
 	struct qcom_ice *ice;
+	struct device_node *ice_conf;
+	int chosen_algo;
 #endif
-
+	#define UFS_QCOM_CAP_ICE_CONFIG BIT(4)
+	u32 caps;
 	void __iomem *dev_ref_clk_ctrl_mmio;
 	bool is_dev_ref_clk_enabled;
 	struct ufs_hw_version hw_ver;
@@ -226,6 +229,39 @@ ufs_qcom_get_debug_reg_offset(struct ufs_qcom_host *host, u32 reg)
 	return UFS_CNTLR_3_x_x_VEN_REGS_OFFSET(reg);
 };
 
+#ifdef CONFIG_SCSI_UFS_CRYPTO
+static inline bool is_ice_config_supported(struct ufs_qcom_host *host)
+{
+	return (host->caps & UFS_QCOM_CAP_ICE_CONFIG);
+}
+
+/* Algorithm Selection */
+#define STATIC_ALLOC_ALG1 0x0
+#define FLOOR_BASED_ALG2 BIT(0)
+#define INSTANTANEOUS_ALG3 BIT(1)
+
+enum {
+	REG_UFS_MEM_ICE_NUM_AES_CORES = 0x2608,
+	REG_UFS_MEM_ICE_CONFIG = 0x260C,
+	REG_UFS_MEM_ICE_ALG1_NUM_CORE = 0x2610,
+	REG_UFS_MEM_ICE_ALG2_NUM_CORE_0 = 0x2614,
+	REG_UFS_MEM_ICE_ALG3_NUM_CORE = 0x2664,
+};
+
+#define ICE_ALG2_NAME_LEN 3
+#define ICE_ALG2_NUM_PARAMS 6
+
+struct ice_alg2_config {
+	/* group names */
+	char name[ICE_ALG2_NAME_LEN];
+	/*
+	 * num_core_tx_stream, num_core_rx_stream, num_wr_task_max,
+	 * num_wr_task_min, num_rd_task_max, num_rd_task_min
+	 */
+	unsigned int val[ICE_ALG2_NUM_PARAMS];
+};
+#endif
+
 #define ufs_qcom_is_link_off(hba) ufshcd_is_link_off(hba)
 #define ufs_qcom_is_link_active(hba) ufshcd_is_link_active(hba)
 #define ufs_qcom_is_link_hibern8(hba) ufshcd_is_link_hibern8(hba)
-- 
2.46.0

Hi Ram,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.12-rc1 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ram-Kumar-Dwivedi/dt-bindings-ufs-qcom-Document-ice-configuration-table/20241005-144559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20241005064307.18972-4-quic_rdwivedi%40quicinc.com
patch subject: [PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms
config: arm-randconfig-001-20241006 (https://download.01.org/0day-ci/archive/20241006/202410061425.FamovVLB-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241006/202410061425.FamovVLB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410061425.FamovVLB-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/ufs/host/ufs-qcom.c:126: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * Refer struct ice_alg2_config


vim +126 drivers/ufs/host/ufs-qcom.c

   124	
   125	/**
 > 126	 * Refer struct ice_alg2_config
   127	 */
   128	static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t)
   129	{
   130		*c = ((val[0] << 8) | val[1] | (1 << 31));
   131		*t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]);
   132	}
   133	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Le 05/10/2024 à 08:43, Ram Kumar Dwivedi a écrit :
> Add support for ICE algorithms for Qualcomm UFS V5.0 and above which
> uses a pool of crypto cores for TX stream (UFS Write – Encryption)
> and RX stream (UFS Read – Decryption).
> 
> Using these algorithms, crypto cores can be dynamically allocated
> to either RX stream or TX stream based on algorithm selected.
> Qualcomm UFS controller supports three ICE algorithms:
> Floor based algorithm, Static Algorithm and Instantaneous algorithm
> to share crypto cores between TX and RX stream.
> 
> Floor Based allocation is selected by default after power On or Reset.
> 
> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
> Co-developed-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> Co-developed-by: Can Guo <quic_cang@quicinc.com>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>   drivers/ufs/host/ufs-qcom.c | 232 ++++++++++++++++++++++++++++++++++++
>   drivers/ufs/host/ufs-qcom.h |  38 +++++-
>   2 files changed, 269 insertions(+), 1 deletion(-)

Hi,

a few nitpicks below.

> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 810e637047d0..c0ca835f13f3 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
>   }
>   
>   #ifdef CONFIG_SCSI_UFS_CRYPTO
> +/*
> + * Default overrides:
> + * There're 10 sets of settings for floor-based algorithm
> + */
> +static struct ice_alg2_config alg2_config[] = {

I think that this could easily be a const struct.

> +	{"G0", {5, 12, 0, 0, 32, 0}},
> +	{"G1", {12, 5, 32, 0, 0, 0}},
> +	{"G2", {6, 11, 4, 1, 32, 1}},
> +	{"G3", {6, 11, 7, 1, 32, 1}},
> +	{"G4", {7, 10, 11, 1, 32, 1}},
> +	{"G5", {7, 10, 14, 1, 32, 1}},
> +	{"G6", {8, 9, 18, 1, 32, 1}},
> +	{"G7", {9, 8, 21, 1, 32, 1}},
> +	{"G8", {10, 7, 24, 1, 32, 1}},
> +	{"G9", {10, 7, 32, 1, 32, 1}},
> +};
> +
> +/**

This does nor look like a kernel-doc. Just /* ?

> + * Refer struct ice_alg2_config
> + */
> +static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t)
> +{
> +	*c = ((val[0] << 8) | val[1] | (1 << 31));
> +	*t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]);
> +}

...

> +/**
> + * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm
> + *
> + * @hba: host controller instance
> + * Return: zero for success and non-zero in case of a failure.
> + */
> +static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0;
> +	/* 6 values for each group, refer struct ice_alg2_config */
> +	unsigned int override_val[ICE_ALG2_NUM_PARAMS];
> +	char name[8] = {0};
> +	int i, ret;
> +
> +	ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG);
> +	for (i = 0; i < ARRAY_SIZE(alg2_config); i++) {
> +		int core = 0, task = 0;
> +
> +		if (host->ice_conf) {
> +			snprintf(name, sizeof(name), "%s%d", "g", i);

Why not just "g%d"?

> +			ret = of_property_read_variable_u32_array(host->ice_conf,
> +								  name,
> +								  override_val,
> +								  ICE_ALG2_NUM_PARAMS,
> +								  ICE_ALG2_NUM_PARAMS);

...

CJ

On 06-Oct-24 1:03 AM, Christophe JAILLET wrote:
> Le 05/10/2024 à 08:43, Ram Kumar Dwivedi a écrit :
>> Add support for ICE algorithms for Qualcomm UFS V5.0 and above which
>> uses a pool of crypto cores for TX stream (UFS Write – Encryption)
>> and RX stream (UFS Read – Decryption).
>>
>> Using these algorithms, crypto cores can be dynamically allocated
>> to either RX stream or TX stream based on algorithm selected.
>> Qualcomm UFS controller supports three ICE algorithms:
>> Floor based algorithm, Static Algorithm and Instantaneous algorithm
>> to share crypto cores between TX and RX stream.
>>
>> Floor Based allocation is selected by default after power On or Reset.
>>
>> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
>> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com>
>> Co-developed-by: Nitin Rawat <quic_nitirawa@quicinc.com>
>> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
>> Co-developed-by: Can Guo <quic_cang@quicinc.com>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>   drivers/ufs/host/ufs-qcom.c | 232 ++++++++++++++++++++++++++++++++++++
>>   drivers/ufs/host/ufs-qcom.h |  38 +++++-
>>   2 files changed, 269 insertions(+), 1 deletion(-)
> 
> Hi,
> 
> a few nitpicks below.
> 
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 810e637047d0..c0ca835f13f3 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
>>   }
>>     #ifdef CONFIG_SCSI_UFS_CRYPTO
>> +/*
>> + * Default overrides:
>> + * There're 10 sets of settings for floor-based algorithm
>> + */
>> +static struct ice_alg2_config alg2_config[] = {
> 
> I think that this could easily be a const struct.
> 
>> +    {"G0", {5, 12, 0, 0, 32, 0}},
>> +    {"G1", {12, 5, 32, 0, 0, 0}},
>> +    {"G2", {6, 11, 4, 1, 32, 1}},
>> +    {"G3", {6, 11, 7, 1, 32, 1}},
>> +    {"G4", {7, 10, 11, 1, 32, 1}},
>> +    {"G5", {7, 10, 14, 1, 32, 1}},
>> +    {"G6", {8, 9, 18, 1, 32, 1}},
>> +    {"G7", {9, 8, 21, 1, 32, 1}},
>> +    {"G8", {10, 7, 24, 1, 32, 1}},
>> +    {"G9", {10, 7, 32, 1, 32, 1}},
>> +};
>> +
>> +/**
> 
> This does nor look like a kernel-doc. Just /* ?
> 
>> + * Refer struct ice_alg2_config
>> + */
>> +static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t)
>> +{
>> +    *c = ((val[0] << 8) | val[1] | (1 << 31));
>> +    *t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]);
>> +}
> 
> ...
> 
>> +/**
>> + * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm
>> + *
>> + * @hba: host controller instance
>> + * Return: zero for success and non-zero in case of a failure.
>> + */
>> +static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba)
>> +{
>> +    struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +    unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0;
>> +    /* 6 values for each group, refer struct ice_alg2_config */
>> +    unsigned int override_val[ICE_ALG2_NUM_PARAMS];
>> +    char name[8] = {0};
>> +    int i, ret;
>> +
>> +    ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG);
>> +    for (i = 0; i < ARRAY_SIZE(alg2_config); i++) {
>> +        int core = 0, task = 0;
>> +
>> +        if (host->ice_conf) {
>> +            snprintf(name, sizeof(name), "%s%d", "g", i);
> 
> Why not just "g%d"?
> 
>> +            ret = of_property_read_variable_u32_array(host->ice_conf,
>> +                                  name,
>> +                                  override_val,
>> +                                  ICE_ALG2_NUM_PARAMS,
>> +                                  ICE_ALG2_NUM_PARAMS);
> 
> ...
> 
> CJ
> 


Hi Christophe,

I have addressed your comments in latest patchset.

Thanks,
Ram.