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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> 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