Re: [PATCH] bus: mhi: Early MHI resume failure in non M3 state

[Bhaumik Bhatt <bbhatt@codeaurora.org>]

On 2021-03-05 08:14 AM, Loic Poulain wrote:
> MHI suspend/resume are symmetric and balanced procedures. If device is
> not in M3 state on a resume, that means something happened behind our
> back. In this case resume is aborded and error reported, to let the
s/aborded/aborted
> controller handling the situation.
> 
> This is mainly requested for system wide suspend-resume operation in
> PCI context which may lead to power-down/reset of the controller which
> will then lose its MHI context. In such cases, PCI driver is supposed
> to recover and reinitialize the device.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/bus/mhi/core/pm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index 0cd6445..725a0b2 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -916,6 +916,9 @@ int mhi_pm_resume(struct mhi_controller *mhi_cntrl)
>  	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state))
>  		return -EIO;
> 
> +	if (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_M3)
> +		return -EINVAL;
> +
>  	/* Notify clients about exiting LPM */
>  	list_for_each_entry_safe(itr, tmp, &mhi_cntrl->lpm_chans, node) {
>  		mutex_lock(&itr->mutex);
We've had this check internally in the past but instead of reading the 
register,
we just used if (mhi_cntrl->pm_state != MHI_PM_M3) to panic.

I like this version better. No reason why we shouldn't attempt to access 
the
link due to read_reg being defined in the controller.

Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project