[PATCH] bus: mhi: core: Resume success on SYS_ERR

[PATCH] bus: mhi: core: Resume success on SYS_ERR

[Loic Poulain]

When a crash occurs while in suspended state M3/D3hot, the host
only discovers SYS_ERR pm-state change on resume. Handle this
state as valid transition for resume procedure, giving a chance
to the MHI core to manage the state (sbl transition, etc...).

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/bus/mhi/core/pm.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 704a5e2..e8eae4f 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -898,8 +898,27 @@ 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)
+	if (mhi_get_mhi_state(mhi_cntrl) == MHI_STATE_SYS_ERR) {
+		/* If we are in SYS_ERR state, let MHI stack manages the error
+		 * and resume successfully.
+		 */
+		dev_warn(dev, "Entered error while suspended\n");
+
+		write_lock_irq(&mhi_cntrl->pm_lock);
+		cur_state = mhi_tryset_pm_state(mhi_cntrl, MHI_PM_SYS_ERR_DETECT);
+		write_unlock_irq(&mhi_cntrl->pm_lock);
+
+		if (cur_state == MHI_PM_SYS_ERR_DETECT) {
+			mhi_pm_sys_err_handler(mhi_cntrl);
+			return 0;
+		}
+	}
+
+	if (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_M3) {
+		dev_warn(dev, "Is not in M3 state, resume failed %d\n",
+			 mhi_get_mhi_state(mhi_cntrl));
 		return -EINVAL;
+	}
 
 	/* Notify clients about exiting LPM */
 	list_for_each_entry_safe(itr, tmp, &mhi_cntrl->lpm_chans, node) {
-- 
2.7.4

Re: [PATCH] bus: mhi: core: Resume success on SYS_ERR

[Bhaumik Bhatt]

On 2021-05-31 01:30 AM, Loic Poulain wrote:
> When a crash occurs while in suspended state M3/D3hot, the host
> only discovers SYS_ERR pm-state change on resume. Handle this
> state as valid transition for resume procedure, giving a chance
> to the MHI core to manage the state (sbl transition, etc...).
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/bus/mhi/core/pm.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index 704a5e2..e8eae4f 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -898,8 +898,27 @@ 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)
> +	if (mhi_get_mhi_state(mhi_cntrl) == MHI_STATE_SYS_ERR) {
> +		/* If we are in SYS_ERR state, let MHI stack manages the error
Nitpick: manage (instead of manages)
> +		 * and resume successfully.
> +		 */
> +		dev_warn(dev, "Entered error while suspended\n");
> +
> +		write_lock_irq(&mhi_cntrl->pm_lock);
> +		cur_state = mhi_tryset_pm_state(mhi_cntrl, MHI_PM_SYS_ERR_DETECT);
> +		write_unlock_irq(&mhi_cntrl->pm_lock);
> +
> +		if (cur_state == MHI_PM_SYS_ERR_DETECT) {
> +			mhi_pm_sys_err_handler(mhi_cntrl);
> +			return 0;
> +		}
> +	}
> +
> +	if (mhi_get_mhi_state(mhi_cntrl) != MHI_STATE_M3) {
> +		dev_warn(dev, "Is not in M3 state, resume failed %d\n",
Maybe this log can be improved to say, "Resume failed. Current state: 
%d, expected M3.\n"
> +			 mhi_get_mhi_state(mhi_cntrl));
>  		return -EINVAL;
> +	}
> 
>  	/* Notify clients about exiting LPM */
>  	list_for_each_entry_safe(itr, tmp, &mhi_cntrl->lpm_chans, node) {

We can add RDDM handling here as well since this hole exists in RDDM 
handling too.
This means we can include the below within if (cur_state == 
MHI_PM_SYS_ERR_DETECT):

if (mhi_cntrl->rddm_image && mhi_is_active(mhi_cntrl)) {
	mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
	mhi_cntrl->ee = ee;
	wake_up_all(&mhi_cntrl->state_event);
	return 0;
}

It can be added in a separate patch as well following this one if you're 
fine
with that. In its current state + nitpick addressed, you can add my:

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