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 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