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

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Aug 03, 2026 at 10:01:23PM +0530, Neeraj Soni wrote:
> From: Ram Prakash Gupta <ram.gupta@oss.qualcomm.com>
> 
> 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 <ram.gupta@oss.qualcomm.com>
> 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>
> ---
>  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.

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

- Eric