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

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Aug 07, 2026 at 07:49:24PM +0530, 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 <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.
> >
> But MMC_CPA2_CRYPTO=0 if !CONFIG_MMC_CRYPTO so crypto_profile field is
> not accessed if !CONFIG_MMC_CRYPTO. Does it need encapsulation?

Yes, because 'if (a & 0)' just makes the code under it be optimized out
and able to "use" undefined symbols.  It still otherwise has to be valid
code and refer to declared symbols and fields.  See my suggestion above.

- Eric