mhi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback
@ 2022-04-05 12:59 Manivannan Sadhasivam
  2022-04-05 20:23 ` Bhaumik Vasav Bhatt
  2022-04-11  6:07 ` Manivannan Sadhasivam
  0 siblings, 2 replies; 4+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-05 12:59 UTC (permalink / raw)
  To: mhi
  Cc: quic_hemantk, quic_bbhatt, linux-arm-msm, linux-kernel,
	loic.poulain, Manivannan Sadhasivam, stable

During hibernation process, once thaw() stage completes, the MHI endpoint
devices will be in M0 state post recovery. After that, the devices will be
powered down so that the system can enter the target sleep state. During
this stage, the PCI core will put the devices in D3hot. But this transition
is allowed by the MHI spec. The devices can only enter D3hot when it is in
M3 state.

So for fixing this issue, let's add the poweroff() callback that will get
executed before putting the system in target sleep state during
hibernation. This callback will power down the device properly so that it
could be restored during restore() or thaw() stage.

Cc: stable@vger.kernel.org
Fixes: 5f0c2ee1fe8d ("bus: mhi: pci-generic: Fix hibernation")
Reported-by: Hemant Kumar <quic_hemantk@quicinc.com>
Suggested-by: Hemant Kumar <quic_hemantk@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---

Changes in v2:

* Hemant suggested to use restore function for poweroff() callback as we can
make sure that the device gets powered down properly.

 drivers/bus/mhi/host/pci_generic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 9527b7d63840..ef85dbfb3216 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -1085,6 +1085,7 @@ static const struct dev_pm_ops mhi_pci_pm_ops = {
 	.resume = mhi_pci_resume,
 	.freeze = mhi_pci_freeze,
 	.thaw = mhi_pci_restore,
+	.poweroff = mhi_pci_freeze,
 	.restore = mhi_pci_restore,
 #endif
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback
  2022-04-05 12:59 [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback Manivannan Sadhasivam
@ 2022-04-05 20:23 ` Bhaumik Vasav Bhatt
  2022-04-07 14:00   ` Manivannan Sadhasivam
  2022-04-11  6:07 ` Manivannan Sadhasivam
  1 sibling, 1 reply; 4+ messages in thread
From: Bhaumik Vasav Bhatt @ 2022-04-05 20:23 UTC (permalink / raw)
  To: Manivannan Sadhasivam, mhi
  Cc: quic_hemantk, linux-arm-msm, linux-kernel, loic.poulain, stable


On 4/5/2022 5:59 AM, Manivannan Sadhasivam wrote:
> During hibernation process, once thaw() stage completes, the MHI endpoint
> devices will be in M0 state post recovery. After that, the devices will be
> powered down so that the system can enter the target sleep state. During
> this stage, the PCI core will put the devices in D3hot. But this transition
> is allowed by the MHI spec. The devices can only enter D3hot when it is in
> M3 state.
>
> So for fixing this issue, let's add the poweroff() callback that will get
> executed before putting the system in target sleep state during
> hibernation. This callback will power down the device properly so that it
> could be restored during restore() or thaw() stage.
>
> Cc: stable@vger.kernel.org
> Fixes: 5f0c2ee1fe8d ("bus: mhi: pci-generic: Fix hibernation")
> Reported-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Suggested-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>
> Changes in v2:
>
> * Hemant suggested to use restore function for poweroff() callback as we can
> make sure that the device gets powered down properly.
>
>   drivers/bus/mhi/host/pci_generic.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 9527b7d63840..ef85dbfb3216 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -1085,6 +1085,7 @@ static const struct dev_pm_ops mhi_pci_pm_ops = {
>   	.resume = mhi_pci_resume,
>   	.freeze = mhi_pci_freeze,
>   	.thaw = mhi_pci_restore,
> +	.poweroff = mhi_pci_freeze,

It is possible that .thaw() queues recovery work and recovery work is 
still running

while .poweroff() is called. I would suggest adding a flush_work() in 
freeze such that

we don't try to power off while we're also trying to power on MHI from 
recovery work.

>   	.restore = mhi_pci_restore,
>   #endif
>   };

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback
  2022-04-05 20:23 ` Bhaumik Vasav Bhatt
@ 2022-04-07 14:00   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 4+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-07 14:00 UTC (permalink / raw)
  To: Bhaumik Vasav Bhatt
  Cc: mhi, quic_hemantk, linux-arm-msm, linux-kernel, loic.poulain, stable

On Tue, Apr 05, 2022 at 01:23:22PM -0700, Bhaumik Vasav Bhatt wrote:
> 
> On 4/5/2022 5:59 AM, Manivannan Sadhasivam wrote:
> > During hibernation process, once thaw() stage completes, the MHI endpoint
> > devices will be in M0 state post recovery. After that, the devices will be
> > powered down so that the system can enter the target sleep state. During
> > this stage, the PCI core will put the devices in D3hot. But this transition
> > is allowed by the MHI spec. The devices can only enter D3hot when it is in
> > M3 state.
> > 
> > So for fixing this issue, let's add the poweroff() callback that will get
> > executed before putting the system in target sleep state during
> > hibernation. This callback will power down the device properly so that it
> > could be restored during restore() or thaw() stage.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 5f0c2ee1fe8d ("bus: mhi: pci-generic: Fix hibernation")
> > Reported-by: Hemant Kumar <quic_hemantk@quicinc.com>
> > Suggested-by: Hemant Kumar <quic_hemantk@quicinc.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> > 
> > Changes in v2:
> > 
> > * Hemant suggested to use restore function for poweroff() callback as we can
> > make sure that the device gets powered down properly.
> > 
> >   drivers/bus/mhi/host/pci_generic.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> > index 9527b7d63840..ef85dbfb3216 100644
> > --- a/drivers/bus/mhi/host/pci_generic.c
> > +++ b/drivers/bus/mhi/host/pci_generic.c
> > @@ -1085,6 +1085,7 @@ static const struct dev_pm_ops mhi_pci_pm_ops = {
> >   	.resume = mhi_pci_resume,
> >   	.freeze = mhi_pci_freeze,
> >   	.thaw = mhi_pci_restore,
> > +	.poweroff = mhi_pci_freeze,
> 
> It is possible that .thaw() queues recovery work and recovery work is still
> running
> 
> while .poweroff() is called. I would suggest adding a flush_work() in freeze
> such that
> 
> we don't try to power off while we're also trying to power on MHI from
> recovery work.
> 

These two cannot run in parallel because the power down will only happen if the
MHI_PCI_DEV_STARTED bit is set in mhi_pdev->status and that's the last step of
recovery. But the real issue I found is, if the recovery didn't finish during
powerdown() stage, then the function will just return and the device might be
powered on later.

So to avoid this scenario, we should have the flush_workqueue() in freeze.

I'll submit a different patch for that.

Thanks,
Mani

> >   	.restore = mhi_pci_restore,
> >   #endif
> >   };

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback
  2022-04-05 12:59 [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback Manivannan Sadhasivam
  2022-04-05 20:23 ` Bhaumik Vasav Bhatt
@ 2022-04-11  6:07 ` Manivannan Sadhasivam
  1 sibling, 0 replies; 4+ messages in thread
From: Manivannan Sadhasivam @ 2022-04-11  6:07 UTC (permalink / raw)
  To: mhi
  Cc: quic_hemantk, quic_bbhatt, linux-arm-msm, linux-kernel,
	loic.poulain, stable

On Tue, Apr 05, 2022 at 06:29:07PM +0530, Manivannan Sadhasivam wrote:
> During hibernation process, once thaw() stage completes, the MHI endpoint
> devices will be in M0 state post recovery. After that, the devices will be
> powered down so that the system can enter the target sleep state. During
> this stage, the PCI core will put the devices in D3hot. But this transition
> is allowed by the MHI spec. The devices can only enter D3hot when it is in
> M3 state.
> 
> So for fixing this issue, let's add the poweroff() callback that will get
> executed before putting the system in target sleep state during
> hibernation. This callback will power down the device properly so that it
> could be restored during restore() or thaw() stage.
> 
> Cc: stable@vger.kernel.org
> Fixes: 5f0c2ee1fe8d ("bus: mhi: pci-generic: Fix hibernation")
> Reported-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Suggested-by: Hemant Kumar <quic_hemantk@quicinc.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---

Applied to mhi-next!

Thanks,
Mani

> 
> Changes in v2:
> 
> * Hemant suggested to use restore function for poweroff() callback as we can
> make sure that the device gets powered down properly.
> 
>  drivers/bus/mhi/host/pci_generic.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 9527b7d63840..ef85dbfb3216 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -1085,6 +1085,7 @@ static const struct dev_pm_ops mhi_pci_pm_ops = {
>  	.resume = mhi_pci_resume,
>  	.freeze = mhi_pci_freeze,
>  	.thaw = mhi_pci_restore,
> +	.poweroff = mhi_pci_freeze,
>  	.restore = mhi_pci_restore,
>  #endif
>  };
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-11  6:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 12:59 [PATCH v2] bus: mhi: host: pci_generic: Add missing poweroff() PM callback Manivannan Sadhasivam
2022-04-05 20:23 ` Bhaumik Vasav Bhatt
2022-04-07 14:00   ` Manivannan Sadhasivam
2022-04-11  6:07 ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).