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

[Neeraj Soni <neeraj.soni@oss.qualcomm.com>]

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