[PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Tomas Winkler]

This fix enables a platform to enter the idle state (suspend-to-idle)

The driver needs to request explicitly go_idle upon completion
from the pm suspend handler.
The runtime pm is disabled on suspend during prepare state by calling
pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
the device in low power state. Symmetrically cmdReady is called
upon resume.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/char/tpm/tpm_crb.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 86f355b6df1d..d8dfff0fdfce 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -479,10 +479,33 @@ static int crb_pm_runtime_resume(struct device *dev)
 
 	return crb_cmd_ready(dev, priv);
 }
+
+static int crb_pm_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = tpm_pm_suspend(dev);
+	if (ret)
+		return ret;
+
+	return crb_pm_runtime_suspend(dev);
+}
+
+static int crb_pm_resume(struct device *dev)
+{
+	int ret;
+
+	ret = crb_pm_runtime_resume(dev);
+	if (ret)
+		return ret;
+
+	return tpm_pm_resume(dev);
+}
+
 #endif /* CONFIG_PM */
 
 static const struct dev_pm_ops crb_pm = {
-	SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
 	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
 };
 
-- 
2.7.4

Re: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Jarkko Sakkinen]

On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> This fix enables a platform to enter the idle state (suspend-to-idle)
> 
> The driver needs to request explicitly go_idle upon completion
> from the pm suspend handler.
> The runtime pm is disabled on suspend during prepare state by calling
> pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> the device in low power state. Symmetrically cmdReady is called
> upon resume.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

LGTM

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

PS. What about Fixes? Does this need it?

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 86f355b6df1d..d8dfff0fdfce 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -479,10 +479,33 @@ static int crb_pm_runtime_resume(struct device *dev)
>  
>  	return crb_cmd_ready(dev, priv);
>  }
> +
> +static int crb_pm_suspend(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = tpm_pm_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	return crb_pm_runtime_suspend(dev);
> +}
> +
> +static int crb_pm_resume(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = crb_pm_runtime_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	return tpm_pm_resume(dev);
> +}
> +
>  #endif /* CONFIG_PM */
>  
>  static const struct dev_pm_ops crb_pm = {
> -	SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Jarkko Sakkinen]

On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> This fix enables a platform to enter the idle state (suspend-to-idle)
> 
> The driver needs to request explicitly go_idle upon completion
> from the pm suspend handler.
> The runtime pm is disabled on suspend during prepare state by calling
> pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> the device in low power state. Symmetrically cmdReady is called
> upon resume.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

LGTM

Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

PS. What about Fixes? Does this need it?

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 86f355b6df1d..d8dfff0fdfce 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -479,10 +479,33 @@ static int crb_pm_runtime_resume(struct device *dev)
>  
>  	return crb_cmd_ready(dev, priv);
>  }
> +
> +static int crb_pm_suspend(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = tpm_pm_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	return crb_pm_runtime_suspend(dev);
> +}
> +
> +static int crb_pm_resume(struct device *dev)
> +{
> +	int ret;
> +
> +	ret = crb_pm_runtime_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	return tpm_pm_resume(dev);
> +}
> +
>  #endif /* CONFIG_PM */
>  
>  static const struct dev_pm_ops crb_pm = {
> -	SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
>  };
>  
> -- 
> 2.7.4
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

RE: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Winkler, Tomas]

> 
> On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> > This fix enables a platform to enter the idle state (suspend-to-idle)
> >
> > The driver needs to request explicitly go_idle upon completion from
> > the pm suspend handler.
> > The runtime pm is disabled on suspend during prepare state by calling
> > pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> > the device in low power state. Symmetrically cmdReady is called upon
> > resume.
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> 
> LGTM
> 
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> PS. What about Fixes? Does this need it?

No, I don't think this is a regression caused by any patch, rather the pm solution was not complete.
Thanks  
Tomas

 
> /Jarkko
> 
> > ---
> >  drivers/char/tpm/tpm_crb.c | 25 ++++++++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 86f355b6df1d..d8dfff0fdfce 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -479,10 +479,33 @@ static int crb_pm_runtime_resume(struct device
> > *dev)
> >
> >  	return crb_cmd_ready(dev, priv);
> >  }
> > +
> > +static int crb_pm_suspend(struct device *dev) {
> > +	int ret;
> > +
> > +	ret = tpm_pm_suspend(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return crb_pm_runtime_suspend(dev);
> > +}
> > +
> > +static int crb_pm_resume(struct device *dev) {
> > +	int ret;
> > +
> > +	ret = crb_pm_runtime_resume(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return tpm_pm_resume(dev);
> > +}
> > +
> >  #endif /* CONFIG_PM */
> >
> >  static const struct dev_pm_ops crb_pm = {
> > -	SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
> > +	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
> >  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend,
> crb_pm_runtime_resume,
> > NULL)  };
> >
> > --
> > 2.7.4
> >

Re: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Winkler, Tomas]

> 
> On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> > This fix enables a platform to enter the idle state (suspend-to-idle)
> >
> > The driver needs to request explicitly go_idle upon completion from
> > the pm suspend handler.
> > The runtime pm is disabled on suspend during prepare state by calling
> > pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> > the device in low power state. Symmetrically cmdReady is called upon
> > resume.
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> LGTM
> 
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> PS. What about Fixes? Does this need it?

No, I don't think this is a regression caused by any patch, rather the pm solution was not complete.
Thanks  
Tomas

 
> /Jarkko
> 
> > ---
> >  drivers/char/tpm/tpm_crb.c | 25 ++++++++++++++++++++++++-
> >  1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 86f355b6df1d..d8dfff0fdfce 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -479,10 +479,33 @@ static int crb_pm_runtime_resume(struct device
> > *dev)
> >
> >  	return crb_cmd_ready(dev, priv);
> >  }
> > +
> > +static int crb_pm_suspend(struct device *dev) {
> > +	int ret;
> > +
> > +	ret = tpm_pm_suspend(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return crb_pm_runtime_suspend(dev);
> > +}
> > +
> > +static int crb_pm_resume(struct device *dev) {
> > +	int ret;
> > +
> > +	ret = crb_pm_runtime_resume(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return tpm_pm_resume(dev);
> > +}
> > +
> >  #endif /* CONFIG_PM */
> >
> >  static const struct dev_pm_ops crb_pm = {
> > -	SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
> > +	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
> >  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend,
> crb_pm_runtime_resume,
> > NULL)  };
> >
> > --
> > 2.7.4
> >

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Jarkko Sakkinen]

On Mon, Mar 06, 2017 at 06:19:59PM +0000, Winkler, Tomas wrote:
> 
> > 
> > On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> > > This fix enables a platform to enter the idle state (suspend-to-idle)
> > >
> > > The driver needs to request explicitly go_idle upon completion from
> > > the pm suspend handler.
> > > The runtime pm is disabled on suspend during prepare state by calling
> > > pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> > > the device in low power state. Symmetrically cmdReady is called upon
> > > resume.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > 
> > LGTM
> > 
> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > PS. What about Fixes? Does this need it?
> 
> No, I don't think this is a regression caused by any patch, rather the pm solution was not complete.
> Thanks  
> Tomas

It's in my tree now (and should be in linux-next too soon).

/Jarkko

Re: [PATCH] tpm/tpm_crb: enter the low power state upon device suspend

[Jarkko Sakkinen]

On Mon, Mar 06, 2017 at 06:19:59PM +0000, Winkler, Tomas wrote:
> 
> > 
> > On Mon, Mar 06, 2017 at 01:53:35AM +0200, Tomas Winkler wrote:
> > > This fix enables a platform to enter the idle state (suspend-to-idle)
> > >
> > > The driver needs to request explicitly go_idle upon completion from
> > > the pm suspend handler.
> > > The runtime pm is disabled on suspend during prepare state by calling
> > > pm_runtime_get_noresume, hence we cannot relay on runtime pm to leave
> > > the device in low power state. Symmetrically cmdReady is called upon
> > > resume.
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > 
> > LGTM
> > 
> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > 
> > PS. What about Fixes? Does this need it?
> 
> No, I don't think this is a regression caused by any patch, rather the pm solution was not complete.
> Thanks  
> Tomas

It's in my tree now (and should be in linux-next too soon).

/Jarkko

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford