[PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Md Sadre Alam]

Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
without Command Queue Engine (CQE).This allows hardware-accelerated
encryption and decryption for standard (non-CMDQ) requests.

This patch:
- Adds ICE register definitions for non-CMDQ crypto configuration
- Implements a per-request crypto setup via sdhci_msm_ice_cfg()
- Hooks into the request path via mmc_host_ops.request

With this, non-CMDQ eMMC devices can benefit from inline encryption,
improving performance for encrypted I/O while maintaining compatibility
with existing CQE crypto support.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---

Change in [v4]

* Moved ICE initialization for non cmdq into sdhci_msm_ice_cfg() and made
  it conditional on mrq->crypto_ctx to enable lazy setup.

* Added msm_host->ice_init_done guard to prevent redundant initialization.

* Updated commit message

Change in [v3]

* Refactored logic to use separate code paths for crypto_ctx != NULL and
  crypto_ctx == NULL to improve readability.

* Renamed bypass to crypto_enable to align with bitfield semantics.

* Removed slot variable

* Added ICE initialization sequence for non-CMDQ eMMC devices before
  __sdhci_add_host()

Change in [v2]

* Moved NONCQ_CRYPTO_PARM and NONCQ_CRYPTO_DUN register definitions into
  sdhci-msm.c

* Introduced use of GENMASK() and FIELD_PREP() macros for cleaner and more
  maintainable bitfield handling in ICE configuration.

* Removed redundant if (!mrq || !cq_host) check from sdhci_msm_ice_cfg()
  as both are guaranteed to be valid in the current call path.

* Added assignment of host->mmc_host_ops.request = sdhci_msm_request; to
  integrate ICE configuration into the standard request path for non-CMDQ
  eMMC devices.

* Removed sdhci_crypto_cfg() from sdhci.c and its invocation in sdhci_request()

Change in [v1]

* Added initial support for Inline Crypto Engine (ICE) on non-CMDQ eMMC
  devices.

 drivers/mmc/host/sdhci-msm.c | 84 ++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4e5edbf2fc9b..3c1c20182ac7 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -157,6 +157,18 @@
 #define CQHCI_VENDOR_CFG1	0xA00
 #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN	(0x3 << 13)
 
+/* non command queue crypto enable register*/
+#define NONCQ_CRYPTO_PARM		0x70
+#define NONCQ_CRYPTO_DUN		0x74
+
+#define DISABLE_CRYPTO			BIT(15)
+#define CRYPTO_GENERAL_ENABLE		BIT(1)
+#define HC_VENDOR_SPECIFIC_FUNC4	0x260
+#define ICE_HCI_SUPPORT			BIT(28)
+
+#define ICE_HCI_PARAM_CCI	GENMASK(7, 0)
+#define ICE_HCI_PARAM_CE	GENMASK(8, 8)
+
 struct sdhci_msm_offset {
 	u32 core_hc_mode;
 	u32 core_mci_data_cnt;
@@ -300,6 +312,7 @@ struct sdhci_msm_host {
 	u32 dll_config;
 	u32 ddr_config;
 	bool vqmmc_enabled;
+	bool ice_init_done;
 };
 
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -2009,6 +2022,74 @@ static int sdhci_msm_ice_keyslot_evict(struct blk_crypto_profile *profile,
 	return qcom_ice_evict_key(msm_host->ice, slot);
 }
 
+static void sdhci_msm_non_cqe_ice_init(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_host *mmc = msm_host->mmc;
+	struct cqhci_host *cq_host = mmc->cqe_private;
+	u32 config;
+	u32 ice_cap;
+
+	config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
+	config &= ~DISABLE_CRYPTO;
+	sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
+	ice_cap = cqhci_readl(cq_host, CQHCI_CAP);
+	if (ice_cap & ICE_HCI_SUPPORT) {
+		config = cqhci_readl(cq_host, CQHCI_CFG);
+		config |= CRYPTO_GENERAL_ENABLE;
+		cqhci_writel(cq_host, config, CQHCI_CFG);
+	}
+	sdhci_msm_ice_enable(msm_host);
+}
+
+static int sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct mmc_host *mmc = msm_host->mmc;
+	struct cqhci_host *cq_host = mmc->cqe_private;
+	unsigned int crypto_params = 0;
+	int key_index;
+	bool crypto_enable;
+	u64 dun = 0;
+
+	if (mrq->crypto_ctx) {
+		if (!msm_host->ice_init_done) {
+			sdhci_msm_non_cqe_ice_init(host);
+			msm_host->ice_init_done = true;
+		}
+
+		crypto_enable = true;
+		dun = mrq->crypto_ctx->bc_dun[0];
+		key_index = mrq->crypto_key_slot;
+		crypto_params = FIELD_PREP(ICE_HCI_PARAM_CE, crypto_enable) |
+				FIELD_PREP(ICE_HCI_PARAM_CCI, key_index);
+
+		cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
+		cqhci_writel(cq_host, lower_32_bits(dun), NONCQ_CRYPTO_DUN);
+	} else {
+		crypto_enable = false;
+		key_index = 0;
+		cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
+	}
+
+	/* Ensure crypto configuration is written before proceeding */
+	wmb();
+
+	return 0;
+}
+
+static void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (mmc->caps2 & MMC_CAP2_CRYPTO)
+		sdhci_msm_ice_cfg(host, mrq);
+
+	sdhci_request(mmc, mrq);
+}
+
 static const struct blk_crypto_ll_ops sdhci_msm_crypto_ops = {
 	.keyslot_program	= sdhci_msm_ice_keyslot_program,
 	.keyslot_evict		= sdhci_msm_ice_keyslot_evict,
@@ -2759,6 +2840,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 
 	msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
 
+#ifdef CONFIG_MMC_CRYPTO
+	host->mmc_host_ops.request = sdhci_msm_request;
+#endif
 	/* Set the timeout value to max possible */
 	host->max_timeout_count = 0xF;
 
-- 
2.34.1

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Ulf Hansson]

+ Eric, Abel

On Tue, 11 Nov 2025 at 11:46, Md Sadre Alam <quic_mdalam@quicinc.com> wrote:
>
> Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
> without Command Queue Engine (CQE).This allows hardware-accelerated
> encryption and decryption for standard (non-CMDQ) requests.
>
> This patch:
> - Adds ICE register definitions for non-CMDQ crypto configuration
> - Implements a per-request crypto setup via sdhci_msm_ice_cfg()
> - Hooks into the request path via mmc_host_ops.request
>
> With this, non-CMDQ eMMC devices can benefit from inline encryption,
> improving performance for encrypted I/O while maintaining compatibility
> with existing CQE crypto support.
>
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Please add Eric/Abel for any future submission of the patch as it's
for ICE. I don't feel completely confident to apply this without some
of their acks.

Kind regards
Uffe

> ---
>
> Change in [v4]
>
> * Moved ICE initialization for non cmdq into sdhci_msm_ice_cfg() and made
>   it conditional on mrq->crypto_ctx to enable lazy setup.
>
> * Added msm_host->ice_init_done guard to prevent redundant initialization.
>
> * Updated commit message
>
> Change in [v3]
>
> * Refactored logic to use separate code paths for crypto_ctx != NULL and
>   crypto_ctx == NULL to improve readability.
>
> * Renamed bypass to crypto_enable to align with bitfield semantics.
>
> * Removed slot variable
>
> * Added ICE initialization sequence for non-CMDQ eMMC devices before
>   __sdhci_add_host()
>
> Change in [v2]
>
> * Moved NONCQ_CRYPTO_PARM and NONCQ_CRYPTO_DUN register definitions into
>   sdhci-msm.c
>
> * Introduced use of GENMASK() and FIELD_PREP() macros for cleaner and more
>   maintainable bitfield handling in ICE configuration.
>
> * Removed redundant if (!mrq || !cq_host) check from sdhci_msm_ice_cfg()
>   as both are guaranteed to be valid in the current call path.
>
> * Added assignment of host->mmc_host_ops.request = sdhci_msm_request; to
>   integrate ICE configuration into the standard request path for non-CMDQ
>   eMMC devices.
>
> * Removed sdhci_crypto_cfg() from sdhci.c and its invocation in sdhci_request()
>
> Change in [v1]
>
> * Added initial support for Inline Crypto Engine (ICE) on non-CMDQ eMMC
>   devices.
>
>  drivers/mmc/host/sdhci-msm.c | 84 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4e5edbf2fc9b..3c1c20182ac7 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -157,6 +157,18 @@
>  #define CQHCI_VENDOR_CFG1      0xA00
>  #define CQHCI_VENDOR_DIS_RST_ON_CQ_EN  (0x3 << 13)
>
> +/* non command queue crypto enable register*/
> +#define NONCQ_CRYPTO_PARM              0x70
> +#define NONCQ_CRYPTO_DUN               0x74
> +
> +#define DISABLE_CRYPTO                 BIT(15)
> +#define CRYPTO_GENERAL_ENABLE          BIT(1)
> +#define HC_VENDOR_SPECIFIC_FUNC4       0x260
> +#define ICE_HCI_SUPPORT                        BIT(28)
> +
> +#define ICE_HCI_PARAM_CCI      GENMASK(7, 0)
> +#define ICE_HCI_PARAM_CE       GENMASK(8, 8)
> +
>  struct sdhci_msm_offset {
>         u32 core_hc_mode;
>         u32 core_mci_data_cnt;
> @@ -300,6 +312,7 @@ struct sdhci_msm_host {
>         u32 dll_config;
>         u32 ddr_config;
>         bool vqmmc_enabled;
> +       bool ice_init_done;
>  };
>
>  static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> @@ -2009,6 +2022,74 @@ static int sdhci_msm_ice_keyslot_evict(struct blk_crypto_profile *profile,
>         return qcom_ice_evict_key(msm_host->ice, slot);
>  }
>
> +static void sdhci_msm_non_cqe_ice_init(struct sdhci_host *host)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +       struct mmc_host *mmc = msm_host->mmc;
> +       struct cqhci_host *cq_host = mmc->cqe_private;
> +       u32 config;
> +       u32 ice_cap;
> +
> +       config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
> +       config &= ~DISABLE_CRYPTO;
> +       sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
> +       ice_cap = cqhci_readl(cq_host, CQHCI_CAP);
> +       if (ice_cap & ICE_HCI_SUPPORT) {
> +               config = cqhci_readl(cq_host, CQHCI_CFG);
> +               config |= CRYPTO_GENERAL_ENABLE;
> +               cqhci_writel(cq_host, config, CQHCI_CFG);
> +       }
> +       sdhci_msm_ice_enable(msm_host);
> +}
> +
> +static int sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq)
> +{
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +       struct mmc_host *mmc = msm_host->mmc;
> +       struct cqhci_host *cq_host = mmc->cqe_private;
> +       unsigned int crypto_params = 0;
> +       int key_index;
> +       bool crypto_enable;
> +       u64 dun = 0;
> +
> +       if (mrq->crypto_ctx) {
> +               if (!msm_host->ice_init_done) {
> +                       sdhci_msm_non_cqe_ice_init(host);
> +                       msm_host->ice_init_done = true;
> +               }
> +
> +               crypto_enable = true;
> +               dun = mrq->crypto_ctx->bc_dun[0];
> +               key_index = mrq->crypto_key_slot;
> +               crypto_params = FIELD_PREP(ICE_HCI_PARAM_CE, crypto_enable) |
> +                               FIELD_PREP(ICE_HCI_PARAM_CCI, key_index);
> +
> +               cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
> +               cqhci_writel(cq_host, lower_32_bits(dun), NONCQ_CRYPTO_DUN);
> +       } else {
> +               crypto_enable = false;
> +               key_index = 0;
> +               cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
> +       }
> +
> +       /* Ensure crypto configuration is written before proceeding */
> +       wmb();
> +
> +       return 0;
> +}
> +
> +static void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       if (mmc->caps2 & MMC_CAP2_CRYPTO)
> +               sdhci_msm_ice_cfg(host, mrq);
> +
> +       sdhci_request(mmc, mrq);
> +}
> +
>  static const struct blk_crypto_ll_ops sdhci_msm_crypto_ops = {
>         .keyslot_program        = sdhci_msm_ice_keyslot_program,
>         .keyslot_evict          = sdhci_msm_ice_keyslot_evict,
> @@ -2759,6 +2840,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
>         msm_host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_NEED_RSP_BUSY;
>
> +#ifdef CONFIG_MMC_CRYPTO
> +       host->mmc_host_ops.request = sdhci_msm_request;
> +#endif
>         /* Set the timeout value to max possible */
>         host->max_timeout_count = 0xF;
>
> --
> 2.34.1
>

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Md Sadre Alam]

Hi,

On 11/11/2025 11:02 PM, Ulf Hansson wrote:
> + Eric, Abel
> 
> On Tue, 11 Nov 2025 at 11:46, Md Sadre Alam <quic_mdalam@quicinc.com> wrote:
>>
>> Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
>> without Command Queue Engine (CQE).This allows hardware-accelerated
>> encryption and decryption for standard (non-CMDQ) requests.
>>
>> This patch:
>> - Adds ICE register definitions for non-CMDQ crypto configuration
>> - Implements a per-request crypto setup via sdhci_msm_ice_cfg()
>> - Hooks into the request path via mmc_host_ops.request
>>
>> With this, non-CMDQ eMMC devices can benefit from inline encryption,
>> improving performance for encrypted I/O while maintaining compatibility
>> with existing CQE crypto support.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Please add Eric/Abel for any future submission of the patch as it's
> for ICE. I don't feel completely confident to apply this without some
> of their acks.
Thank you for the guidance. I completely understand your concern and 
will make sure to include Eric and Abel in any future submissions 
related to ICE.

Thanks,
Alam.

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Eric Biggers]

On Tue, Nov 11, 2025 at 04:16:04PM +0530, Md Sadre Alam wrote:
> Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
> without Command Queue Engine (CQE).This allows hardware-accelerated
> encryption and decryption for standard (non-CMDQ) requests.
> 
> This patch:
> - Adds ICE register definitions for non-CMDQ crypto configuration
> - Implements a per-request crypto setup via sdhci_msm_ice_cfg()
> - Hooks into the request path via mmc_host_ops.request
> 
> With this, non-CMDQ eMMC devices can benefit from inline encryption,
> improving performance for encrypted I/O while maintaining compatibility
> with existing CQE crypto support.

This really should explain that this patch actually applies only to host
controllers that *do* support CQE.  Just they are using a card that
doesn't support CQE or CQE was explicitly disabled.  Right?

> +static void sdhci_msm_non_cqe_ice_init(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct mmc_host *mmc = msm_host->mmc;
> +	struct cqhci_host *cq_host = mmc->cqe_private;
> +	u32 config;
> +	u32 ice_cap;
> +
> +	config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
> +	config &= ~DISABLE_CRYPTO;
> +	sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
> +	ice_cap = cqhci_readl(cq_host, CQHCI_CAP);
> +	if (ice_cap & ICE_HCI_SUPPORT) {
> +		config = cqhci_readl(cq_host, CQHCI_CFG);
> +		config |= CRYPTO_GENERAL_ENABLE;
> +		cqhci_writel(cq_host, config, CQHCI_CFG);
> +	}
> +	sdhci_msm_ice_enable(msm_host);
> +}
> +
> +static int sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +	struct mmc_host *mmc = msm_host->mmc;
> +	struct cqhci_host *cq_host = mmc->cqe_private;
> +	unsigned int crypto_params = 0;
> +	int key_index;
> +	bool crypto_enable;
> +	u64 dun = 0;
> +
> +	if (mrq->crypto_ctx) {
> +		if (!msm_host->ice_init_done) {
> +			sdhci_msm_non_cqe_ice_init(host);
> +			msm_host->ice_init_done = true;
> +		}

This means sdhci_msm_ice_enable() is called only once per host
controller.  It looks like the existing call to sdhci_msm_ice_enable()
happens each time after the host controller is resumed.  So there seems
to be an inconsistency there.  Which way is correct?

> +	} else {
> +		crypto_enable = false;
> +		key_index = 0;
> +		cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);

The values assigned to 'crypto_enable' and 'key_index' are never used.

> +static void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{

Could you leave a comment here that notes this is used only for non-CQE
requests and that crypto on CQE requests is handled elsewhere?

- Eric

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Md Sadre Alam]

Hi,

On 11/12/2025 2:22 AM, Eric Biggers wrote:
> On Tue, Nov 11, 2025 at 04:16:04PM +0530, Md Sadre Alam wrote:
>> Enable Inline Crypto Engine (ICE) support for eMMC devices that operate
>> without Command Queue Engine (CQE).This allows hardware-accelerated
>> encryption and decryption for standard (non-CMDQ) requests.
>>
>> This patch:
>> - Adds ICE register definitions for non-CMDQ crypto configuration
>> - Implements a per-request crypto setup via sdhci_msm_ice_cfg()
>> - Hooks into the request path via mmc_host_ops.request
>>
>> With this, non-CMDQ eMMC devices can benefit from inline encryption,
>> improving performance for encrypted I/O while maintaining compatibility
>> with existing CQE crypto support.
> 
> This really should explain that this patch actually applies only to host
> controllers that *do* support CQE.  Just they are using a card that
> doesn't support CQE or CQE was explicitly disabled.  Right?
Yes, you are absolutely correct. Thank you for pointing this out - the 
commit message should be clearer about this important detail.

This patch applies specifically to CQE-capable host controllers 
(sdhci-msm controllers that support CQHCI) when they are operating in 
non-CQE mode.
This can happen in two scenarios:

1. CQE-capable controller + non-CQE card: The host controller supports 
CQE, but the eMMC card doesn't support Command Queue Engine
2. CQE explicitly disabled: The host controller and card both support 
CQE, but CQE has been explicitly disabled (e.g., via device tree)

For the 2nd case I will post another path which will handle host side
CQE enable/disable.
> 
>> +static void sdhci_msm_non_cqe_ice_init(struct sdhci_host *host)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	struct mmc_host *mmc = msm_host->mmc;
>> +	struct cqhci_host *cq_host = mmc->cqe_private;
>> +	u32 config;
>> +	u32 ice_cap;
>> +
>> +	config = sdhci_readl(host, HC_VENDOR_SPECIFIC_FUNC4);
>> +	config &= ~DISABLE_CRYPTO;
>> +	sdhci_writel(host, config, HC_VENDOR_SPECIFIC_FUNC4);
>> +	ice_cap = cqhci_readl(cq_host, CQHCI_CAP);
>> +	if (ice_cap & ICE_HCI_SUPPORT) {
>> +		config = cqhci_readl(cq_host, CQHCI_CFG);
>> +		config |= CRYPTO_GENERAL_ENABLE;
>> +		cqhci_writel(cq_host, config, CQHCI_CFG);
>> +	}
>> +	sdhci_msm_ice_enable(msm_host);
>> +}
>> +
>> +static int sdhci_msm_ice_cfg(struct sdhci_host *host, struct mmc_request *mrq)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	struct mmc_host *mmc = msm_host->mmc;
>> +	struct cqhci_host *cq_host = mmc->cqe_private;
>> +	unsigned int crypto_params = 0;
>> +	int key_index;
>> +	bool crypto_enable;
>> +	u64 dun = 0;
>> +
>> +	if (mrq->crypto_ctx) {
>> +		if (!msm_host->ice_init_done) {
>> +			sdhci_msm_non_cqe_ice_init(host);
>> +			msm_host->ice_init_done = true;
>> +		}
> 
> This means sdhci_msm_ice_enable() is called only once per host
> controller.  It looks like the existing call to sdhci_msm_ice_enable()
> happens each time after the host controller is resumed.  So there seems
> to be an inconsistency there.  Which way is correct?
Thank you for highlighting this. After revisiting the code paths, I 
believe the behavior is consistent across both CQE and non-CQE modes.
ICE is re-enabled on every resume via the common 
sdhci_msm_runtime_resume() → sdhci_msm_ice_resume() → qcom_ice_resume() 
→ sdhci_msm_ice_enable() path.
The ice_init_done flag only governs one-time initialization in 
sdhci_msm_ice_cfg() and doesn’t interfere with the resume logic.

In summary:
CQE mode: ICE enabled during sdhci_msm_cqe_enable() + every resume
Non-CQE mode: ICE enabled on first crypto request + every resume
> 
>> +	} else {
>> +		crypto_enable = false;
>> +		key_index = 0;
>> +		cqhci_writel(cq_host, crypto_params, NONCQ_CRYPTO_PARM);
> 
> The values assigned to 'crypto_enable' and 'key_index' are never used.
Ok, will remove in next revision.
> 
>> +static void sdhci_msm_request(struct mmc_host *mmc, struct mmc_request *mrq)
>> +{
> 
> Could you leave a comment here that notes this is used only for non-CQE
> requests and that crypto on CQE requests is handled elsewhere?
Thank you very much — that’s a valuable suggestion. Adding a comment 
will make the code much clearer. Will add in next revision.

Thanks,
Alam.

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Eric Biggers]

On Thu, Nov 13, 2025 at 12:41:28PM +0530, Md Sadre Alam wrote:
> > > +	if (mrq->crypto_ctx) {
> > > +		if (!msm_host->ice_init_done) {
> > > +			sdhci_msm_non_cqe_ice_init(host);
> > > +			msm_host->ice_init_done = true;
> > > +		}
> > 
> > This means sdhci_msm_ice_enable() is called only once per host
> > controller.  It looks like the existing call to sdhci_msm_ice_enable()
> > happens each time after the host controller is resumed.  So there seems
> > to be an inconsistency there.  Which way is correct?
> Thank you for highlighting this. After revisiting the code paths, I believe
> the behavior is consistent across both CQE and non-CQE modes.
> ICE is re-enabled on every resume via the common sdhci_msm_runtime_resume()
> → sdhci_msm_ice_resume() → qcom_ice_resume() → sdhci_msm_ice_enable() path.
> The ice_init_done flag only governs one-time initialization in
> sdhci_msm_ice_cfg() and doesn’t interfere with the resume logic.
> 
> In summary:
> CQE mode: ICE enabled during sdhci_msm_cqe_enable() + every resume
> Non-CQE mode: ICE enabled on first crypto request + every resume

I was looking at sdhci_msm_cqe_enable().  Based on the caller, it seems
to be a per-resume thing too.  So it doesn't seem consistent.

- Eric

Re: [PATCH v4] mmc: sdhci-msm: Enable ICE support for non-cmdq eMMC devices

[Md Sadre Alam]

Hi,

On 11/13/2025 11:05 PM, Eric Biggers wrote:
> On Thu, Nov 13, 2025 at 12:41:28PM +0530, Md Sadre Alam wrote:
>>>> +	if (mrq->crypto_ctx) {
>>>> +		if (!msm_host->ice_init_done) {
>>>> +			sdhci_msm_non_cqe_ice_init(host);
>>>> +			msm_host->ice_init_done = true;
>>>> +		}
>>>
>>> This means sdhci_msm_ice_enable() is called only once per host
>>> controller.  It looks like the existing call to sdhci_msm_ice_enable()
>>> happens each time after the host controller is resumed.  So there seems
>>> to be an inconsistency there.  Which way is correct?
>> Thank you for highlighting this. After revisiting the code paths, I believe
>> the behavior is consistent across both CQE and non-CQE modes.
>> ICE is re-enabled on every resume via the common sdhci_msm_runtime_resume()
>> → sdhci_msm_ice_resume() → qcom_ice_resume() → sdhci_msm_ice_enable() path.
>> The ice_init_done flag only governs one-time initialization in
>> sdhci_msm_ice_cfg() and doesn’t interfere with the resume logic.
>>
>> In summary:
>> CQE mode: ICE enabled during sdhci_msm_cqe_enable() + every resume
>> Non-CQE mode: ICE enabled on first crypto request + every resume
> 
> I was looking at sdhci_msm_cqe_enable().  Based on the caller, it seems
> to be a per-resume thing too.  So it doesn't seem consistent.
You are absolutely correct. After reviewing the code paths more 
carefully, I see that sdhci_msm_cqe_enable() is indeed called per-resume 
(via cqhci_resume() → cqhci_enable() → sdhci_msm_cqe_enable()).

So For consistency, I Will remove the sdhci_msm_ice_enable() call from 
sdhci_msm_non_cqe_ice_init() ? since ICE is already enabled on every 
resume through the runtime PM path.

Thanks,
Alam.