[PATCH V1 0/3] Add support for multiple ICE algorithms

[PATCH V1 0/3] Add support for multiple ICE algorithms

[Ram Kumar Dwivedi]

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.

Ram Kumar Dwivedi (3):
  dt-bindings: ufs: qcom: Document ice configuration table
  arm64: dts: qcom: sm8650: Add ICE algorithm entries
  scsi: ufs: qcom: Add support for multiple ICE algorithms

 .../devicetree/bindings/ufs/qcom,ufs.yaml     |  24 ++
 arch/arm64/boot/dts/qcom/sm8650.dtsi          |  19 ++
 drivers/ufs/host/ufs-qcom.c                   | 232 ++++++++++++++++++
 drivers/ufs/host/ufs-qcom.h                   |  38 ++-
 4 files changed, 312 insertions(+), 1 deletion(-)

-- 
2.46.0

[PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Ram Kumar Dwivedi]

There are three algorithms supported for inline crypto engine:
Floor based, Static and Instantaneous algorithm.

Document the compatible used for the algorithm configurations
for inline crypto engine found.

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>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 .../devicetree/bindings/ufs/qcom,ufs.yaml     | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 25a5edeea164..5ac56e164643 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -108,6 +108,11 @@ properties:
     description:
       GPIO connected to the RESET pin of the UFS memory device.
 
+  ice-config:
+    type: object
+    description:
+      ICE configuration table for Qualcom SOC
+
 required:
   - compatible
   - reg
@@ -350,5 +355,24 @@ examples:
                             <0 0>,
                             <0 0>;
             qcom,ice = <&ice>;
+
+            ice_cfg: ice-config {
+                alg1 {
+                     alg-name = "alg1";
+                     rx-alloc-percent = <60>;
+                     status = "disabled";
+                };
+
+                alg2 {
+                     alg-name = "alg2";
+                     status = "disabled";
+                };
+
+                alg3 {
+                     alg-name = "alg3";
+                     num-core = <28 28 15 13>;
+                     status = "ok";
+                };
+            };
         };
     };
-- 
2.46.0

[PATCH V1 2/3] arm64: dts: qcom: sm8650: Add ICE algorithm entries

[Ram Kumar Dwivedi]

There are three algorithms supported for inline crypto engine:
Floor based, Static and Instantaneous algorithm.

Add ice algorithm entries and enable instantaneous algorithm
by default.

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>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 9d9bbb9aca64..56a7ca6a3af4 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -2590,6 +2590,25 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			#reset-cells = <1>;
 
 			status = "disabled";
+
+			ice_cfg: ice-config {
+				alg1 {
+					alg-name = "alg1";
+					rx-alloc-percent = <60>;
+					status = "disabled";
+				};
+
+				alg2 {
+					alg-name = "alg2";
+					status = "disabled";
+				};
+
+				alg3 {
+					alg-name = "alg3";
+					num-core = <28 28 15 13>;
+					status = "ok";
+				};
+			};
 		};
 
 		ice: crypto@1d88000 {
-- 
2.46.0

[PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms

[Ram Kumar Dwivedi]

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

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Rob Herring (Arm)]

On Sat, 05 Oct 2024 12:13:05 +0530, Ram Kumar Dwivedi wrote:
> There are three algorithms supported for inline crypto engine:
> Floor based, Static and Instantaneous algorithm.
> 
> Document the compatible used for the algorithm configurations
> for inline crypto engine found.
> 
> 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>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.example.dtb: alg3: status: 'oneOf' conditional failed, one must be fixed:
	['ok'] is not of type 'object'
	'ok' is not one of ['okay', 'disabled', 'reserved', 'fail', 'fail-needs-probe']
	from schema $id: http://devicetree.org/schemas/dt-core.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241005064307.18972-2-quic_rdwivedi@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Rob Herring]

On Sat, Oct 05, 2024 at 12:13:05PM +0530, Ram Kumar Dwivedi wrote:
> There are three algorithms supported for inline crypto engine:
> Floor based, Static and Instantaneous algorithm.
> 
> Document the compatible used for the algorithm configurations
> for inline crypto engine found.
> 
> 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>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index 25a5edeea164..5ac56e164643 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -108,6 +108,11 @@ properties:
>      description:
>        GPIO connected to the RESET pin of the UFS memory device.
>  
> +  ice-config:
> +    type: object
> +    description:
> +      ICE configuration table for Qualcom SOC

What goes in this node?

> +
>  required:
>    - compatible
>    - reg
> @@ -350,5 +355,24 @@ examples:
>                              <0 0>,
>                              <0 0>;
>              qcom,ice = <&ice>;
> +
> +            ice_cfg: ice-config {
> +                alg1 {
> +                     alg-name = "alg1";
> +                     rx-alloc-percent = <60>;
> +                     status = "disabled";

Examples should be enabled.

> +                };
> +
> +                alg2 {
> +                     alg-name = "alg2";
> +                     status = "disabled";
> +                };
> +
> +                alg3 {
> +                     alg-name = "alg3";
> +                     num-core = <28 28 15 13>;
> +                     status = "ok";
> +                };
> +            };
>          };
>      };
> -- 
> 2.46.0
> 

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Eric Biggers]

On Sat, Oct 05, 2024 at 12:13:05PM +0530, Ram Kumar Dwivedi wrote:
> There are three algorithms supported for inline crypto engine:
> Floor based, Static and Instantaneous algorithm.

No.  The algorithms supported by ICE are AES-XTS, AES-ECB, AES-CBC, etc.  So I'm
afraid this terminology is already taken.

This new thing seems to be about how work is distributed among different
hardware cores, so calling these "ICE schedulers" or something might make sense.

- Eric

Re: [PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms

[Christophe JAILLET]

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

Re: [PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms

[kernel test robot]

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

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Krzysztof Kozlowski]

On 05/10/2024 08:43, Ram Kumar Dwivedi wrote:
> There are three algorithms supported for inline crypto engine:
> Floor based, Static and Instantaneous algorithm.
> 
> Document the compatible used for the algorithm configurations
> for inline crypto engine found.
> 
> 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>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>

Three people developed the code, but none of them cared to test it? Or
make it even slightly correct?

Sorry, this code is not ready for upstream. Please work internally on
doing meaningful review and basic tests BEFORE posting.

Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Add ICE algorithm entries

[Krzysztof Kozlowski]

On 05/10/2024 08:43, Ram Kumar Dwivedi wrote:
> There are three algorithms supported for inline crypto engine:
> Floor based, Static and Instantaneous algorithm.
> 
> Add ice algorithm entries and enable instantaneous algorithm
> by default.
> 
> 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>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 9d9bbb9aca64..56a7ca6a3af4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -2590,6 +2590,25 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  			#reset-cells = <1>;
>  
>  			status = "disabled";
> +
> +			ice_cfg: ice-config {
> +				alg1 {
> +					alg-name = "alg1";
> +					rx-alloc-percent = <60>;
> +					status = "disabled";
> +				};
> +
> +				alg2 {
> +					alg-name = "alg2";
> +					status = "disabled";
> +				};
> +
> +				alg3 {
> +					alg-name = "alg3";
> +					num-core = <28 28 15 13>;
> +					status = "ok";

NAK. This has so many issues... First, describes OS policy. Second,
there is no "ok".

Best regards,
Krzysztof

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Ram Kumar Dwivedi]

On 05-Oct-24 7:54 PM, Rob Herring (Arm) wrote:
> 
> On Sat, 05 Oct 2024 12:13:05 +0530, Ram Kumar Dwivedi wrote:
>> There are three algorithms supported for inline crypto engine:
>> Floor based, Static and Instantaneous algorithm.
>>
>> Document the compatible used for the algorithm configurations
>> for inline crypto engine found.
>>
>> 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>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 24 +++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/qcom,ufs.example.dtb: alg3: status: 'oneOf' conditional failed, one must be fixed:
> 	['ok'] is not of type 'object'
> 	'ok' is not one of ['okay', 'disabled', 'reserved', 'fail', 'fail-needs-probe']
> 	from schema $id: http://devicetree.org/schemas/dt-core.yaml#
> 

Hi Rob,
	I have addressed the comment in the latest patch set. Now the dt binding is successfully compiling.
Thanks,
Ram.

> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241005064307.18972-2-quic_rdwivedi@quicinc.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Add ICE algorithm entries

[Ram Kumar Dwivedi]

On 06-Oct-24 2:02 PM, Krzysztof Kozlowski wrote:
> On 05/10/2024 08:43, Ram Kumar Dwivedi wrote:
>> There are three algorithms supported for inline crypto engine:
>> Floor based, Static and Instantaneous algorithm.
>>
>> Add ice algorithm entries and enable instantaneous algorithm
>> by default.
>>
>> 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>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index 9d9bbb9aca64..56a7ca6a3af4 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -2590,6 +2590,25 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>  			#reset-cells = <1>;
>>  
>>  			status = "disabled";
>> +
>> +			ice_cfg: ice-config {
>> +				alg1 {
>> +					alg-name = "alg1";
>> +					rx-alloc-percent = <60>;
>> +					status = "disabled";
>> +				};
>> +
>> +				alg2 {
>> +					alg-name = "alg2";
>> +					status = "disabled";
>> +				};
>> +
>> +				alg3 {
>> +					alg-name = "alg3";
>> +					num-core = <28 28 15 13>;
>> +					status = "ok";
> 
> NAK. This has so many issues... First, describes OS policy. Second,
> there is no "ok".
> 
Hi Krzysztof,
	I have updated the status to "okay" in latest patchset and updated the alg-name with actual allocator name.
	I have already mentioned default allocator as instantaneous. Sorry, I did not understand OS policy comment, could you please explain?
Thanks,
Ram.

> Best regards,
> Krzysztof
> 

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Document ice configuration table

[Ram Kumar Dwivedi]

On 06-Oct-24 12:45 AM, Eric Biggers wrote:
> On Sat, Oct 05, 2024 at 12:13:05PM +0530, Ram Kumar Dwivedi wrote:
>> There are three algorithms supported for inline crypto engine:
>> Floor based, Static and Instantaneous algorithm.
> 
> No.  The algorithms supported by ICE are AES-XTS, AES-ECB, AES-CBC, etc.  So I'm
> afraid this terminology is already taken.
> 
> This new thing seems to be about how work is distributed among different
> hardware cores, so calling these "ICE schedulers" or something might make sense.
> 
> - Eric

Hi Eric,
	I have rephrased patch commit description. Used terminology as ICE allocator instead of ICE algorithm.
Thanks,
Ram.

Re: [PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms

[Ram Kumar Dwivedi]

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.

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Add ICE algorithm entries

[Krzysztof Kozlowski]

On 29/10/2024 12:06, Ram Kumar Dwivedi wrote:
> 
> 
> On 06-Oct-24 2:02 PM, Krzysztof Kozlowski wrote:
>> On 05/10/2024 08:43, Ram Kumar Dwivedi wrote:
>>> There are three algorithms supported for inline crypto engine:
>>> Floor based, Static and Instantaneous algorithm.
>>>
>>> Add ice algorithm entries and enable instantaneous algorithm
>>> by default.
>>>
>>> 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>
>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> index 9d9bbb9aca64..56a7ca6a3af4 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -2590,6 +2590,25 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>>  			#reset-cells = <1>;
>>>  
>>>  			status = "disabled";
>>> +
>>> +			ice_cfg: ice-config {
>>> +				alg1 {
>>> +					alg-name = "alg1";
>>> +					rx-alloc-percent = <60>;
>>> +					status = "disabled";
>>> +				};
>>> +
>>> +				alg2 {
>>> +					alg-name = "alg2";
>>> +					status = "disabled";
>>> +				};
>>> +
>>> +				alg3 {
>>> +					alg-name = "alg3";
>>> +					num-core = <28 28 15 13>;
>>> +					status = "ok";
>>
>> NAK. This has so many issues... First, describes OS policy. Second,
>> there is no "ok".
>>
> Hi Krzysztof,
> 	I have updated the status to "okay" in latest patchset

Still no. Why this node needs it?

> and updated the alg-name with actual allocator name.

Please wrap your replies according to mailing list style.

But anyway, all your algs sound like OS policy.


> 	I have already mentioned default allocator as instantaneous. Sorry, I did not understand OS policy comment, could you please explain?

This looks like OS policy, OS choice. DT does not describe such things.

Best regards,
Krzysztof