diff mbox series

bus: mhi: Early MHI resume failure in non M3 state

Message ID 1614960841-20233-1-git-send-email-loic.poulain@linaro.org (mailing list archive)
State Accepted
Commit 020d3b26c07abe274ac17f64999bbd3bf3342195
Headers show
Series bus: mhi: Early MHI resume failure in non M3 state | expand

Commit Message

Loic Poulain March 5, 2021, 4:14 p.m. UTC
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
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(+)

Comments

Bhaumik Bhatt March 5, 2021, 5:26 p.m. UTC | #1
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
Manivannan Sadhasivam March 10, 2021, 1:58 p.m. UTC | #2
On Fri, Mar 05, 2021 at 05:14:01PM +0100, 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
> controller handling the situation.
> 

Will fix the spelling mistakes while applying. Please try to avoid them...

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

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  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);
> -- 
> 2.7.4
>
patchwork-bot+linux-arm-msm@kernel.org May 26, 2021, 7:03 p.m. UTC | #3
Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Fri,  5 Mar 2021 17:14:01 +0100 you 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
> 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.
> 
> [...]

Here is the summary with links:
  - bus: mhi: Early MHI resume failure in non M3 state
    https://git.kernel.org/qcom/c/020d3b26c07a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

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);