Re: [PATCH v6 2/2] mmc: sdhci-msm: Use pm ops instead of macro to restore crypto keys

On 8/7/2026 7:49 PM, Neeraj Soni wrote:
> 
> 
> On 8/4/2026 1:48 AM, Eric Biggers wrote:
>> On Mon, Aug 03, 2026 at 10:01:23PM +0530, Neeraj Soni wrote:
>>> From: Ram Prakash Gupta <[email protected]>
>>>
>>> Inline Crypto Engine (ICE) keys are lost after hibernation entry and this
>>> needs to be restored when hibernation exits. ICE keys are re-programmed
>>> during sdhci_msm_ice_init() but it may not cover cases where the
>>> hibernation image is already restored.
>>>
>>> Unwrap the pm ops and use directly in driver to add the call to restore
>>> Inline Crypto Engine (ICE) keys. This ensures that ICE is brought into
>>> same state as before hibernation.
>>>
>>> Also set MMC_CAP2_CRYPTO_NO_REPROG to indicate that re-programming of ICE
>>> keys is not needed during MMC runtime suspend/resume.
>>>
>>> Signed-off-by: Ram Prakash Gupta <[email protected]>
>>> Signed-off-by: Seshu Madhavi Puppala <[email protected]>
>>> Co-developed-by: Ram Prakash Gupta <[email protected]>
>>> Signed-off-by: Ram Prakash Gupta <[email protected]>
>>> Co-developed-by: Sarthak Garg <[email protected]>
>>> Signed-off-by: Sarthak Garg <[email protected]>
>>> Signed-off-by: Debraj Mukhopadhyay <[email protected]>
>>> Signed-off-by: Neeraj Soni <[email protected]>
>>> ---
>>>  drivers/mmc/host/sdhci-msm.c | 23 ++++++++++++++++++++++-
>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 4aff965f0e2e..f64788a70309 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -1959,6 +1959,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;
>>>  }
>>>  
>>> @@ -2983,9 +2984,29 @@ static int sdhci_msm_runtime_resume(struct device *dev)
>>>  	return ret;
>>>  }
>>>  
>>> +static int sdhci_msm_restore(struct device *dev)
>>> +{
>>> +	struct sdhci_host *host = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = pm_runtime_force_resume(dev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (host->mmc->caps2 & MMC_CAP2_CRYPTO)
>>> +		blk_crypto_reprogram_all_keys(&host->mmc->crypto_profile);
>>> +
>>> +	return ret;
>>> +}
>>
>> Sorry, it looks like this actually does need an #ifdef because the
>> crypto_profile field is conditional.  It doesn't need to be a new #ifdef
>> though, as you could add a function sdhci_msm_ice_restore() within the
>> existing CONFIG_MMC_CRYPTO section of this file (along with a no-op stub
>> in the !CONFIG_MMC_CRYPTO section), then call it from here.  Similar to
>> how the existing sdhci_msm_ice_resume() works, for example.
>>
> But MMC_CPA2_CRYPTO=0 if !CONFIG_MMC_CRYPTO so crypto_profile field is
> not accessed if !CONFIG_MMC_CRYPTO. Does it need encapsulation?
>  
>>>  static const struct dev_pm_ops sdhci_msm_pm_ops = {
>>> -	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
>>>  	RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume, NULL)
>>> +	.suspend	= pm_runtime_force_suspend,
>>> +	.resume		= pm_runtime_force_resume,
>>> +	.freeze		= pm_runtime_force_suspend,
>>> +	.restore	= sdhci_msm_restore,
>>> +	.thaw		= pm_runtime_force_resume,
>>> +	.poweroff	= pm_runtime_force_suspend,
>>
>> Should these use pm_sleep_ptr() to allow dead code elimination when
>> !CONFIG_PM_SLEEP?
>>
> Ram can you please check this. Looks valid to me. 
>  
>> - Eric

Agree with this, we will update this in next patchset using pm_sleep_ptr().

>>
> Regards
> Neeraj