[PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Neeraj Soni]

From: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>

Crypto reprogram all keys is called for each MMC runtime
suspend/resume in current upstream design. If this is implemented
as a non-interruptible call to TEE for security, the cpu core is
blocked for execution while this call executes although the crypto
engine already has the keys. For example, glitches in audio/video
streaming applications have been observed due to this. Add the flag
MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
keys to crypto engine for socs which dont require this feature.

Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

---
Changes in v4:
- Removed the "CONFIG_MMC_CRYPTO" encapsulation for "MMC_CAP2_CRYPTO" and
  "MMC_CAP2_CRYPTO_NO_REPROG".

Changes in v3:
- Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG
  in the commit message for clarity.
- Added parentheses around the condition: (host->caps2 & MMC_CAP2_CRYPTO)
  to improve readability and correctness.
- Updated the comment associated with MMC_CAP2_CRYPTO_NO_REPROG
  to better reflect its purpose.

Changes in v2:
- Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG for
  improved clarity.
- Defined MMC_CAP2_CRYPTO_NO_REPROG for MMC targets that do not support
  a Crypto Engine.
- Restricted the usage of struct crypto_profile to MMC devices that
  support a Crypto Engine.

Changes in v1:
- Addressed the comments from:
  https://lore.kernel.org/lkml/20241006135530.17363-3-
  quic_spuppala@quicinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
- Avoided reprogram of keys for Qualcomm SOCs only.
- Ensured reprogram of all keys on host controller reset.
---
 drivers/mmc/core/crypto.c    | 2 +-
 drivers/mmc/host/sdhci-msm.c | 6 ++++++
 include/linux/mmc/host.h     | 5 +----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
index fec4fbf16a5b..a5a90bfc634e 100644
--- a/drivers/mmc/core/crypto.c
+++ b/drivers/mmc/core/crypto.c
@@ -15,7 +15,7 @@
 void mmc_crypto_set_initial_state(struct mmc_host *host)
 {
 	/* Reset might clear all keys, so reprogram all the keys. */
-	if (host->caps2 & MMC_CAP2_CRYPTO)
+	if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
 		blk_crypto_reprogram_all_keys(&host->crypto_profile);
 }
 
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4e5edbf2fc9b..2ccb63dde9c1 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -1949,6 +1949,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
 	}
 
 	mmc->caps2 |= MMC_CAP2_CRYPTO;
+	mmc->caps2 |= MMC_CAP2_CRYPTO_NO_REPROG;
 	return 0;
 }
 
@@ -2526,6 +2527,11 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
 	usleep_range(200, 210);
 	reset_control_put(reset);
 
+#ifdef CONFIG_MMC_CRYPTO
+	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
+		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
+#endif
+
 	return ret;
 }
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index e0e2c265e5d1..2fd76f966e24 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -457,12 +457,9 @@ struct mmc_host {
 #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
 #define MMC_CAP2_AVOID_3_3V	(1 << 25)	/* Host must negotiate down from 3.3V */
 #define MMC_CAP2_MERGE_CAPABLE	(1 << 26)	/* Host can merge a segment over the segment size */
-#ifdef CONFIG_MMC_CRYPTO
 #define MMC_CAP2_CRYPTO		(1 << 27)	/* Host supports inline encryption */
-#else
-#define MMC_CAP2_CRYPTO		0
-#endif
 #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
+#define MMC_CAP2_CRYPTO_NO_REPROG (1 << 29)	/* Host handles inline crypto key reprogramming */
 
 	bool			uhs2_sd_tran;	/* UHS-II flag for SD_TRAN state */
 	bool			uhs2_app_cmd;	/* UHS-II flag for APP command */
-- 
2.34.1

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Adrian Hunter]

On 16/01/2026 14:10, Neeraj Soni wrote:
> From: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
> 
> Crypto reprogram all keys is called for each MMC runtime
> suspend/resume in current upstream design. If this is implemented
> as a non-interruptible call to TEE for security, the cpu core is
> blocked for execution while this call executes although the crypto
> engine already has the keys. For example, glitches in audio/video
> streaming applications have been observed due to this. Add the flag
> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
> keys to crypto engine for socs which dont require this feature.
> 
> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
> Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
> 

Would be nicer if mmc_crypto_set_initial_state() would be exposed
to host drivers, so it could be called instead of:

#ifdef CONFIG_MMC_CRYPTO
	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
#endif

Perhaps that could be tidied up later.

So, nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes in v4:
> - Removed the "CONFIG_MMC_CRYPTO" encapsulation for "MMC_CAP2_CRYPTO" and
>   "MMC_CAP2_CRYPTO_NO_REPROG".
> 
> Changes in v3:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG
>   in the commit message for clarity.
> - Added parentheses around the condition: (host->caps2 & MMC_CAP2_CRYPTO)
>   to improve readability and correctness.
> - Updated the comment associated with MMC_CAP2_CRYPTO_NO_REPROG
>   to better reflect its purpose.
> 
> Changes in v2:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG for
>   improved clarity.
> - Defined MMC_CAP2_CRYPTO_NO_REPROG for MMC targets that do not support
>   a Crypto Engine.
> - Restricted the usage of struct crypto_profile to MMC devices that
>   support a Crypto Engine.
> 
> Changes in v1:
> - Addressed the comments from:
>   https://lore.kernel.org/lkml/20241006135530.17363-3-
>   quic_spuppala@quicinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
> - Avoided reprogram of keys for Qualcomm SOCs only.
> - Ensured reprogram of all keys on host controller reset.
> ---
>  drivers/mmc/core/crypto.c    | 2 +-
>  drivers/mmc/host/sdhci-msm.c | 6 ++++++
>  include/linux/mmc/host.h     | 5 +----
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> index fec4fbf16a5b..a5a90bfc634e 100644
> --- a/drivers/mmc/core/crypto.c
> +++ b/drivers/mmc/core/crypto.c
> @@ -15,7 +15,7 @@
>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>  {
>  	/* Reset might clear all keys, so reprogram all the keys. */
> -	if (host->caps2 & MMC_CAP2_CRYPTO)
> +	if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>  		blk_crypto_reprogram_all_keys(&host->crypto_profile);
>  }
>  
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4e5edbf2fc9b..2ccb63dde9c1 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1949,6 +1949,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
>  	}
>  
>  	mmc->caps2 |= MMC_CAP2_CRYPTO;
> +	mmc->caps2 |= MMC_CAP2_CRYPTO_NO_REPROG;
>  	return 0;
>  }
>  
> @@ -2526,6 +2527,11 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>  	usleep_range(200, 210);
>  	reset_control_put(reset);
>  
> +#ifdef CONFIG_MMC_CRYPTO
> +	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
> +		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
> +#endif
> +
>  	return ret;
>  }
>  
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index e0e2c265e5d1..2fd76f966e24 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -457,12 +457,9 @@ struct mmc_host {
>  #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
>  #define MMC_CAP2_AVOID_3_3V	(1 << 25)	/* Host must negotiate down from 3.3V */
>  #define MMC_CAP2_MERGE_CAPABLE	(1 << 26)	/* Host can merge a segment over the segment size */
> -#ifdef CONFIG_MMC_CRYPTO
>  #define MMC_CAP2_CRYPTO		(1 << 27)	/* Host supports inline encryption */
> -#else
> -#define MMC_CAP2_CRYPTO		0
> -#endif
>  #define MMC_CAP2_ALT_GPT_TEGRA	(1 << 28)	/* Host with eMMC that has GPT entry at a non-standard location */
> +#define MMC_CAP2_CRYPTO_NO_REPROG (1 << 29)	/* Host handles inline crypto key reprogramming */
>  
>  	bool			uhs2_sd_tran;	/* UHS-II flag for SD_TRAN state */
>  	bool			uhs2_app_cmd;	/* UHS-II flag for APP command */

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Ulf Hansson]

On Fri, 16 Jan 2026 at 13:10, Neeraj Soni <neeraj.soni@oss.qualcomm.com> wrote:
>
> From: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
>
> Crypto reprogram all keys is called for each MMC runtime
> suspend/resume in current upstream design. If this is implemented
> as a non-interruptible call to TEE for security, the cpu core is
> blocked for execution while this call executes although the crypto
> engine already has the keys. For example, glitches in audio/video
> streaming applications have been observed due to this. Add the flag
> MMC_CAP2_CRYPTO_NO_REPROG as part of host->caps2 to control reprogramming
> keys to crypto engine for socs which dont require this feature.
>
> Signed-off-by: Seshu Madhavi Puppala <quic_spuppala@quicinc.com>
> Co-developed-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
> Co-developed-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
> Signed-off-by: Debraj Mukhopadhyay <quic_dmukhopa@quicinc.com>
> Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
>
> ---
> Changes in v4:
> - Removed the "CONFIG_MMC_CRYPTO" encapsulation for "MMC_CAP2_CRYPTO" and
>   "MMC_CAP2_CRYPTO_NO_REPROG".
>
> Changes in v3:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG
>   in the commit message for clarity.
> - Added parentheses around the condition: (host->caps2 & MMC_CAP2_CRYPTO)
>   to improve readability and correctness.
> - Updated the comment associated with MMC_CAP2_CRYPTO_NO_REPROG
>   to better reflect its purpose.
>
> Changes in v2:
> - Renamed MMC_CAP2_DONT_REPROGRAM to MMC_CAP2_CRYPTO_NO_REPROG for
>   improved clarity.
> - Defined MMC_CAP2_CRYPTO_NO_REPROG for MMC targets that do not support
>   a Crypto Engine.
> - Restricted the usage of struct crypto_profile to MMC devices that
>   support a Crypto Engine.
>
> Changes in v1:
> - Addressed the comments from:
>   https://lore.kernel.org/lkml/20241006135530.17363-3-
>   quic_spuppala@quicinc.com/T/#m69c9ab538bd9efd54515646952d0d7d1d7c17690
> - Avoided reprogram of keys for Qualcomm SOCs only.
> - Ensured reprogram of all keys on host controller reset.
> ---
>  drivers/mmc/core/crypto.c    | 2 +-
>  drivers/mmc/host/sdhci-msm.c | 6 ++++++
>  include/linux/mmc/host.h     | 5 +----
>  3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> index fec4fbf16a5b..a5a90bfc634e 100644
> --- a/drivers/mmc/core/crypto.c
> +++ b/drivers/mmc/core/crypto.c
> @@ -15,7 +15,7 @@
>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>  {
>         /* Reset might clear all keys, so reprogram all the keys. */
> -       if (host->caps2 & MMC_CAP2_CRYPTO)
> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);

As far as I understand, calling blk_crypto_reprogram_all_keys() would
only be needed for those mmc hosts that lose their corresponding ICE
context during runtime+system suspend, reset and possibly during
->probe().

In other words, calling mmc_crypto_set_initial_state() from
mmc_set_initial_state() looks like it's a mistake, as it has really
nothing to do with the card's initialization, unless I have understood
this wrong!?

That said, I would rather make the mtk-sd and sdhci-msm drivers to
handle this themselves, by explicitly calling
blk_crypto_reprogram_all_keys() when needed - and drop
mmc_crypto_set_initial_state() altogether.

For the sdhci-msm case, it seems like the only case we need to care
about is for the reset.

For mtk-sd I don't know what is needed, but possibly Eric can help out here?

>  }
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4e5edbf2fc9b..2ccb63dde9c1 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -1949,6 +1949,7 @@ static int sdhci_msm_ice_init(struct sdhci_msm_host *msm_host,
>         }
>
>         mmc->caps2 |= MMC_CAP2_CRYPTO;
> +       mmc->caps2 |= MMC_CAP2_CRYPTO_NO_REPROG;
>         return 0;
>  }
>
> @@ -2526,6 +2527,11 @@ static int sdhci_msm_gcc_reset(struct device *dev, struct sdhci_host *host)
>         usleep_range(200, 210);
>         reset_control_put(reset);
>
> +#ifdef CONFIG_MMC_CRYPTO
> +       if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
> +               blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
> +#endif
> +
>         return ret;
>  }
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index e0e2c265e5d1..2fd76f966e24 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -457,12 +457,9 @@ struct mmc_host {
>  #define MMC_CAP2_CQE_DCMD      (1 << 24)       /* CQE can issue a direct command */
>  #define MMC_CAP2_AVOID_3_3V    (1 << 25)       /* Host must negotiate down from 3.3V */
>  #define MMC_CAP2_MERGE_CAPABLE (1 << 26)       /* Host can merge a segment over the segment size */
> -#ifdef CONFIG_MMC_CRYPTO
>  #define MMC_CAP2_CRYPTO                (1 << 27)       /* Host supports inline encryption */
> -#else
> -#define MMC_CAP2_CRYPTO                0
> -#endif
>  #define MMC_CAP2_ALT_GPT_TEGRA (1 << 28)       /* Host with eMMC that has GPT entry at a non-standard location */
> +#define MMC_CAP2_CRYPTO_NO_REPROG (1 << 29)    /* Host handles inline crypto key reprogramming */
>
>         bool                    uhs2_sd_tran;   /* UHS-II flag for SD_TRAN state */
>         bool                    uhs2_app_cmd;   /* UHS-II flag for APP command */
> --
> 2.34.1
>

Kind regards
Uffe

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Eric Biggers]

On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
> > diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> > index fec4fbf16a5b..a5a90bfc634e 100644
> > --- a/drivers/mmc/core/crypto.c
> > +++ b/drivers/mmc/core/crypto.c
> > @@ -15,7 +15,7 @@
> >  void mmc_crypto_set_initial_state(struct mmc_host *host)
> >  {
> >         /* Reset might clear all keys, so reprogram all the keys. */
> > -       if (host->caps2 & MMC_CAP2_CRYPTO)
> > +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
> >                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
> 
> As far as I understand, calling blk_crypto_reprogram_all_keys() would
> only be needed for those mmc hosts that lose their corresponding ICE
> context during runtime+system suspend, reset and possibly during
> ->probe().
> 
> In other words, calling mmc_crypto_set_initial_state() from
> mmc_set_initial_state() looks like it's a mistake, as it has really
> nothing to do with the card's initialization, unless I have understood
> this wrong!?
> 
> That said, I would rather make the mtk-sd and sdhci-msm drivers to
> handle this themselves, by explicitly calling
> blk_crypto_reprogram_all_keys() when needed - and drop
> mmc_crypto_set_initial_state() altogether.
> 
> For the sdhci-msm case, it seems like the only case we need to care
> about is for the reset.
> 
> For mtk-sd I don't know what is needed, but possibly Eric can help out here?

The comment for mmc_set_initial_state() says "Set initial state after a
power cycle or a hw_reset."  I relied on that when I added the call to
mmc_crypto_set_initial_state() back in 2020.  In the following thread it
was also discussed that the code was intended to reprogram the keys on
reset, not runtime suspend as that shouldn't be needed:
https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u

If that is not what it actually does, it probably would be appropriate
to replace it with something else.  

- Eric

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Ulf Hansson]

On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
> > > diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> > > index fec4fbf16a5b..a5a90bfc634e 100644
> > > --- a/drivers/mmc/core/crypto.c
> > > +++ b/drivers/mmc/core/crypto.c
> > > @@ -15,7 +15,7 @@
> > >  void mmc_crypto_set_initial_state(struct mmc_host *host)
> > >  {
> > >         /* Reset might clear all keys, so reprogram all the keys. */
> > > -       if (host->caps2 & MMC_CAP2_CRYPTO)
> > > +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
> > >                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
> >
> > As far as I understand, calling blk_crypto_reprogram_all_keys() would
> > only be needed for those mmc hosts that lose their corresponding ICE
> > context during runtime+system suspend, reset and possibly during
> > ->probe().
> >
> > In other words, calling mmc_crypto_set_initial_state() from
> > mmc_set_initial_state() looks like it's a mistake, as it has really
> > nothing to do with the card's initialization, unless I have understood
> > this wrong!?
> >
> > That said, I would rather make the mtk-sd and sdhci-msm drivers to
> > handle this themselves, by explicitly calling
> > blk_crypto_reprogram_all_keys() when needed - and drop
> > mmc_crypto_set_initial_state() altogether.
> >
> > For the sdhci-msm case, it seems like the only case we need to care
> > about is for the reset.
> >
> > For mtk-sd I don't know what is needed, but possibly Eric can help out here?
>
> The comment for mmc_set_initial_state() says "Set initial state after a
> power cycle or a hw_reset."  I relied on that when I added the call to
> mmc_crypto_set_initial_state() back in 2020.  In the following thread it
> was also discussed that the code was intended to reprogram the keys on
> reset, not runtime suspend as that shouldn't be needed:
> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u

The comment in the mmc_set_initial_state() is referring to the card
and not the host controller. There have been some similar
misunderstandings in the past for other functions in the core, sorry.

In any case, I have been trying to understand where the ICE context
really belongs and recently Neeraj answered that question [1].

>
> If that is not what it actually does, it probably would be appropriate
> to replace it with something else.

I agree, the comment(s) could deserve some clarifications.

>
> - Eric

Kind regards
Uffe

[1]
https://lore.kernel.org/all/e1f689d0-523b-5842-3a6e-10b431d617ce@oss.qualcomm.com/

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Neeraj Soni]

On 1/22/2026 3:44 PM, Ulf Hansson wrote:
> On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@kernel.org> wrote:
>>
>> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
>>>> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
>>>> index fec4fbf16a5b..a5a90bfc634e 100644
>>>> --- a/drivers/mmc/core/crypto.c
>>>> +++ b/drivers/mmc/core/crypto.c
>>>> @@ -15,7 +15,7 @@
>>>>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>>>>  {
>>>>         /* Reset might clear all keys, so reprogram all the keys. */
>>>> -       if (host->caps2 & MMC_CAP2_CRYPTO)
>>>> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>>>>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
>>>
>>> As far as I understand, calling blk_crypto_reprogram_all_keys() would
>>> only be needed for those mmc hosts that lose their corresponding ICE
>>> context during runtime+system suspend, reset and possibly during
>>> ->probe().
>>>
>>> In other words, calling mmc_crypto_set_initial_state() from
>>> mmc_set_initial_state() looks like it's a mistake, as it has really
>>> nothing to do with the card's initialization, unless I have understood
>>> this wrong!?
>>>
>>> That said, I would rather make the mtk-sd and sdhci-msm drivers to
>>> handle this themselves, by explicitly calling
>>> blk_crypto_reprogram_all_keys() when needed - and drop
>>> mmc_crypto_set_initial_state() altogether.
>>>
>>> For the sdhci-msm case, it seems like the only case we need to care
>>> about is for the reset.
>>>
>>> For mtk-sd I don't know what is needed, but possibly Eric can help out here?
>>
>> The comment for mmc_set_initial_state() says "Set initial state after a
>> power cycle or a hw_reset."  I relied on that when I added the call to
>> mmc_crypto_set_initial_state() back in 2020.  In the following thread it
>> was also discussed that the code was intended to reprogram the keys on
>> reset, not runtime suspend as that shouldn't be needed:
>> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u
> 
> The comment in the mmc_set_initial_state() is referring to the card
> and not the host controller. There have been some similar
> misunderstandings in the past for other functions in the core, sorry.
> 
> In any case, I have been trying to understand where the ICE context
> really belongs and recently Neeraj answered that question [1].
> 
>>
>> If that is not what it actually does, it probably would be appropriate
>> to replace it with something else.
> 
> I agree, the comment(s) could deserve some clarifications.

Hello Ulf. Do you expect modifications in the comment for mmc_set_initial_state()?
I am sorry i did not follow up and assumed that no more changes are expected in
the patch but i stil see it is not yet reviewed or approved.
> 
>>
>> - Eric
> 
> Kind regards
> Uffe
> 
> [1]
> https://lore.kernel.org/all/e1f689d0-523b-5842-3a6e-10b431d617ce@oss.qualcomm.com/
> 

Regards,
Neeraj

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Neeraj Soni]

Hello Ulf,

Can you please help to clarify? If there is no more modifications expected
then wanted to understand when this patch can be picked for mmc-next.

Regards
Neeraj

On 5/14/2026 11:26 AM, Neeraj Soni wrote:
> 
> 
> On 1/22/2026 3:44 PM, Ulf Hansson wrote:
>> On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@kernel.org> wrote:
>>>
>>> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
>>>>> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
>>>>> index fec4fbf16a5b..a5a90bfc634e 100644
>>>>> --- a/drivers/mmc/core/crypto.c
>>>>> +++ b/drivers/mmc/core/crypto.c
>>>>> @@ -15,7 +15,7 @@
>>>>>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>>>>>  {
>>>>>         /* Reset might clear all keys, so reprogram all the keys. */
>>>>> -       if (host->caps2 & MMC_CAP2_CRYPTO)
>>>>> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>>>>>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
>>>>
>>>> As far as I understand, calling blk_crypto_reprogram_all_keys() would
>>>> only be needed for those mmc hosts that lose their corresponding ICE
>>>> context during runtime+system suspend, reset and possibly during
>>>> ->probe().
>>>>
>>>> In other words, calling mmc_crypto_set_initial_state() from
>>>> mmc_set_initial_state() looks like it's a mistake, as it has really
>>>> nothing to do with the card's initialization, unless I have understood
>>>> this wrong!?
>>>>
>>>> That said, I would rather make the mtk-sd and sdhci-msm drivers to
>>>> handle this themselves, by explicitly calling
>>>> blk_crypto_reprogram_all_keys() when needed - and drop
>>>> mmc_crypto_set_initial_state() altogether.
>>>>
>>>> For the sdhci-msm case, it seems like the only case we need to care
>>>> about is for the reset.
>>>>
>>>> For mtk-sd I don't know what is needed, but possibly Eric can help out here?
>>>
>>> The comment for mmc_set_initial_state() says "Set initial state after a
>>> power cycle or a hw_reset."  I relied on that when I added the call to
>>> mmc_crypto_set_initial_state() back in 2020.  In the following thread it
>>> was also discussed that the code was intended to reprogram the keys on
>>> reset, not runtime suspend as that shouldn't be needed:
>>> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u
>>
>> The comment in the mmc_set_initial_state() is referring to the card
>> and not the host controller. There have been some similar
>> misunderstandings in the past for other functions in the core, sorry.
>>
>> In any case, I have been trying to understand where the ICE context
>> really belongs and recently Neeraj answered that question [1].
>>
>>>
>>> If that is not what it actually does, it probably would be appropriate
>>> to replace it with something else.
>>
>> I agree, the comment(s) could deserve some clarifications.
> 
> Hello Ulf. Do you expect modifications in the comment for mmc_set_initial_state()?
> I am sorry i did not follow up and assumed that no more changes are expected in
> the patch but i stil see it is not yet reviewed or approved.
>>
>>>
>>> - Eric
>>
>> Kind regards
>> Uffe
>>
>> [1]
>> https://lore.kernel.org/all/e1f689d0-523b-5842-3a6e-10b431d617ce@oss.qualcomm.com/
>>
> 
> Regards,
> Neeraj
> 

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Ulf Hansson]

On Wed, May 20, 2026 at 12:50 PM Neeraj Soni
<neeraj.soni@oss.qualcomm.com> wrote:
>
> Hello Ulf,
>
> Can you please help to clarify? If there is no more modifications expected
> then wanted to understand when this patch can be picked for mmc-next.

Apologize for the delay. See more comments below.

>
> Regards
> Neeraj
>
> On 5/14/2026 11:26 AM, Neeraj Soni wrote:
> >
> >
> > On 1/22/2026 3:44 PM, Ulf Hansson wrote:
> >> On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@kernel.org> wrote:
> >>>
> >>> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
> >>>>> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
> >>>>> index fec4fbf16a5b..a5a90bfc634e 100644
> >>>>> --- a/drivers/mmc/core/crypto.c
> >>>>> +++ b/drivers/mmc/core/crypto.c
> >>>>> @@ -15,7 +15,7 @@
> >>>>>  void mmc_crypto_set_initial_state(struct mmc_host *host)
> >>>>>  {
> >>>>>         /* Reset might clear all keys, so reprogram all the keys. */
> >>>>> -       if (host->caps2 & MMC_CAP2_CRYPTO)
> >>>>> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
> >>>>>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
> >>>>
> >>>> As far as I understand, calling blk_crypto_reprogram_all_keys() would
> >>>> only be needed for those mmc hosts that lose their corresponding ICE
> >>>> context during runtime+system suspend, reset and possibly during
> >>>> ->probe().
> >>>>
> >>>> In other words, calling mmc_crypto_set_initial_state() from
> >>>> mmc_set_initial_state() looks like it's a mistake, as it has really
> >>>> nothing to do with the card's initialization, unless I have understood
> >>>> this wrong!?
> >>>>
> >>>> That said, I would rather make the mtk-sd and sdhci-msm drivers to
> >>>> handle this themselves, by explicitly calling
> >>>> blk_crypto_reprogram_all_keys() when needed - and drop
> >>>> mmc_crypto_set_initial_state() altogether.
> >>>>
> >>>> For the sdhci-msm case, it seems like the only case we need to care
> >>>> about is for the reset.
> >>>>
> >>>> For mtk-sd I don't know what is needed, but possibly Eric can help out here?
> >>>
> >>> The comment for mmc_set_initial_state() says "Set initial state after a
> >>> power cycle or a hw_reset."  I relied on that when I added the call to
> >>> mmc_crypto_set_initial_state() back in 2020.  In the following thread it
> >>> was also discussed that the code was intended to reprogram the keys on
> >>> reset, not runtime suspend as that shouldn't be needed:
> >>> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u
> >>
> >> The comment in the mmc_set_initial_state() is referring to the card
> >> and not the host controller. There have been some similar
> >> misunderstandings in the past for other functions in the core, sorry.
> >>
> >> In any case, I have been trying to understand where the ICE context
> >> really belongs and recently Neeraj answered that question [1].
> >>
> >>>
> >>> If that is not what it actually does, it probably would be appropriate
> >>> to replace it with something else.
> >>
> >> I agree, the comment(s) could deserve some clarifications.
> >
> > Hello Ulf. Do you expect modifications in the comment for mmc_set_initial_state()?
> > I am sorry i did not follow up and assumed that no more changes are expected in
> > the patch but i stil see it is not yet reviewed or approved.

There are a couple of things needed to get this merged.

1) Please split up the patch into two pieces. Let the core change be a
separate patch preceding the change to sdhci-msm.

2) Clarify in the commit messages that the register context for the
crypto profile belongs to the sdhci/cqhci host and not the mmc card,
hence re-programming should be managed by the host driver itself.
Typically that should be done from the driver's runtime suspend/resume
callbacks, even if that isn't needed for sdhci-msm, it seems.

3) To avoid breaking the support for mediatek drivers, I agree that
adding MMC_CAP2_CRYPTO_NO_REPROG as proposed, makes sense as an
intermediate step. We can look into the mtk-sd driver later on and
clean things up properly.

4) The call to blk_crypto_reprogram_all_keys() that's added to
sdhci_msm_gcc_reset(), gets called once during ->probe(), is that
sufficient then? In any I would prefer that we don't use "#ifdef
CONFIG_MMC_CRYPTO" more than needed in the code, so  please, either
add a new function within the existing section along with a stub
function or - if it works, call blk_crypto_reprogram_all_keys() from
sdhci_msm_ice_init() directly instead.

Kind regards
Uffe

Re: [PATCH v4] mmc: Avoid reprogram all keys to Inline Crypto Engine for MMC runtime suspend resume

[Neeraj Soni]

On 7/6/2026 6:30 PM, Ulf Hansson wrote:
> On Wed, May 20, 2026 at 12:50 PM Neeraj Soni
> <neeraj.soni@oss.qualcomm.com> wrote:
>>
>> Hello Ulf,
>>
>> Can you please help to clarify? If there is no more modifications expected
>> then wanted to understand when this patch can be picked for mmc-next.
> 
> Apologize for the delay. See more comments below.
> 
>>
>> Regards
>> Neeraj
>>
>> On 5/14/2026 11:26 AM, Neeraj Soni wrote:
>>>
>>>
>>> On 1/22/2026 3:44 PM, Ulf Hansson wrote:
>>>> On Thu, 22 Jan 2026 at 02:14, Eric Biggers <ebiggers@kernel.org> wrote:
>>>>>
>>>>> On Wed, Jan 21, 2026 at 03:12:43PM +0100, Ulf Hansson wrote:
>>>>>>> diff --git a/drivers/mmc/core/crypto.c b/drivers/mmc/core/crypto.c
>>>>>>> index fec4fbf16a5b..a5a90bfc634e 100644
>>>>>>> --- a/drivers/mmc/core/crypto.c
>>>>>>> +++ b/drivers/mmc/core/crypto.c
>>>>>>> @@ -15,7 +15,7 @@
>>>>>>>  void mmc_crypto_set_initial_state(struct mmc_host *host)
>>>>>>>  {
>>>>>>>         /* Reset might clear all keys, so reprogram all the keys. */
>>>>>>> -       if (host->caps2 & MMC_CAP2_CRYPTO)
>>>>>>> +       if ((host->caps2 & MMC_CAP2_CRYPTO) && !(host->caps2 & MMC_CAP2_CRYPTO_NO_REPROG))
>>>>>>>                 blk_crypto_reprogram_all_keys(&host->crypto_profile);
>>>>>>
>>>>>> As far as I understand, calling blk_crypto_reprogram_all_keys() would
>>>>>> only be needed for those mmc hosts that lose their corresponding ICE
>>>>>> context during runtime+system suspend, reset and possibly during
>>>>>> ->probe().
>>>>>>
>>>>>> In other words, calling mmc_crypto_set_initial_state() from
>>>>>> mmc_set_initial_state() looks like it's a mistake, as it has really
>>>>>> nothing to do with the card's initialization, unless I have understood
>>>>>> this wrong!?
>>>>>>
>>>>>> That said, I would rather make the mtk-sd and sdhci-msm drivers to
>>>>>> handle this themselves, by explicitly calling
>>>>>> blk_crypto_reprogram_all_keys() when needed - and drop
>>>>>> mmc_crypto_set_initial_state() altogether.
>>>>>>
>>>>>> For the sdhci-msm case, it seems like the only case we need to care
>>>>>> about is for the reset.
>>>>>>
>>>>>> For mtk-sd I don't know what is needed, but possibly Eric can help out here?
>>>>>
>>>>> The comment for mmc_set_initial_state() says "Set initial state after a
>>>>> power cycle or a hw_reset."  I relied on that when I added the call to
>>>>> mmc_crypto_set_initial_state() back in 2020.  In the following thread it
>>>>> was also discussed that the code was intended to reprogram the keys on
>>>>> reset, not runtime suspend as that shouldn't be needed:
>>>>> https://lore.kernel.org/linux-mmc/X7gQ9Y44iIgkiM64@sol.localdomain/T/#u
>>>>
>>>> The comment in the mmc_set_initial_state() is referring to the card
>>>> and not the host controller. There have been some similar
>>>> misunderstandings in the past for other functions in the core, sorry.
>>>>
>>>> In any case, I have been trying to understand where the ICE context
>>>> really belongs and recently Neeraj answered that question [1].
>>>>
>>>>>
>>>>> If that is not what it actually does, it probably would be appropriate
>>>>> to replace it with something else.
>>>>
>>>> I agree, the comment(s) could deserve some clarifications.
>>>
>>> Hello Ulf. Do you expect modifications in the comment for mmc_set_initial_state()?
>>> I am sorry i did not follow up and assumed that no more changes are expected in
>>> the patch but i stil see it is not yet reviewed or approved.
> 
> There are a couple of things needed to get this merged.
> 
> 1) Please split up the patch into two pieces. Let the core change be a
> separate patch preceding the change to sdhci-msm.

Ack.
>
> 2) Clarify in the commit messages that the register context for the
> crypto profile belongs to the sdhci/cqhci host and not the mmc card,
> hence re-programming should be managed by the host driver itself.
> Typically that should be done from the driver's runtime suspend/resume
> callbacks, even if that isn't needed for sdhci-msm, it seems.
> 
Ack. Yes this is not needed for sdhci-msm.

> 3) To avoid breaking the support for mediatek drivers, I agree that
> adding MMC_CAP2_CRYPTO_NO_REPROG as proposed, makes sense as an
> intermediate step. We can look into the mtk-sd driver later on and
> clean things up properly.
>
Okay.
 
> 4) The call to blk_crypto_reprogram_all_keys() that's added to
> sdhci_msm_gcc_reset(), gets called once during ->probe(), is that
> sufficient then? In any I would prefer that we don't use "#ifdef
> CONFIG_MMC_CRYPTO" more than needed in the code, so  please, either
> add a new function within the existing section along with a stub
> function or - if it works, call blk_crypto_reprogram_all_keys() from
> sdhci_msm_ice_init() directly instead.
>
Ack. Yes the intention is to re-program the keys only if the host undergoes
hardware reset. I will post v5 with the clean up you suggested.
 
> Kind regards
> Uffe
> 
Regards,
Neeraj