[PATCH 1/2] PM: Prototype the pm_generic_ operations

[PATCH 1/2] PM: Prototype the pm_generic_ operations

[Mark Brown]

The pm_generic_ operations are all exported but are not prototyped in any
header file for direct use. Do so.

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 include/linux/pm.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 40f3f45..f7b5bed 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -610,4 +610,11 @@ extern unsigned int	pm_flags;
 #define PM_APM	1
 #define PM_ACPI	2
 
+int pm_generic_suspend(struct device *dev);
+int pm_generic_resume(struct device *dev);
+int pm_generic_freeze(struct device *dev);
+int pm_generic_thaw(struct device *dev);
+int pm_generic_restore(struct device *dev);
+int pm_generic_poweroff(struct device *dev);
+
 #endif /* _LINUX_PM_H */
-- 
1.7.2.3

[PATCH 2/2] i2c: Factor out runtime suspend checks from PM operations

[Mark Brown]

When devices use dev_pm_ops the I2C API is implementing standard functionality
for integration with runtime PM and for checking for the presence of a per
device op. The PM core provides pm_generic_ functions implementing this
behaviour - use them to reduce coupling with future PM updates.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/i2c/i2c-core.c |   57 ++++++++++++++++--------------------------------
 1 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 6b4cc56..4478557 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -196,69 +196,50 @@ static int i2c_device_pm_suspend(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm) {
-		if (pm_runtime_suspended(dev))
-			return 0;
-		else
-			return pm->suspend ? pm->suspend(dev) : 0;
-	}
-
-	return i2c_legacy_suspend(dev, PMSG_SUSPEND);
+	if (pm)
+		return pm_generic_suspend(dev);
+	else
+		return i2c_legacy_suspend(dev, PMSG_SUSPEND);
 }
 
 static int i2c_device_pm_resume(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
-	int ret;
 
 	if (pm)
-		ret = pm->resume ? pm->resume(dev) : 0;
+		return pm_generic_resume(dev);
 	else
-		ret = i2c_legacy_resume(dev);
-
-	return ret;
+		return i2c_legacy_resume(dev);
 }
 
 static int i2c_device_pm_freeze(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm) {
-		if (pm_runtime_suspended(dev))
-			return 0;
-		else
-			return pm->freeze ? pm->freeze(dev) : 0;
-	}
-
-	return i2c_legacy_suspend(dev, PMSG_FREEZE);
+	if (pm)
+		return pm_generic_freeze(dev);
+	else
+		return i2c_legacy_suspend(dev, PMSG_FREEZE);
 }
 
 static int i2c_device_pm_thaw(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm) {
-		if (pm_runtime_suspended(dev))
-			return 0;
-		else
-			return pm->thaw ? pm->thaw(dev) : 0;
-	}
-
-	return i2c_legacy_resume(dev);
+	if (pm)
+		return pm_generic_thaw(dev);
+	else
+		return i2c_legacy_resume(dev);
 }
 
 static int i2c_device_pm_poweroff(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm) {
-		if (pm_runtime_suspended(dev))
-			return 0;
-		else
-			return pm->poweroff ? pm->poweroff(dev) : 0;
-	}
-
-	return i2c_legacy_suspend(dev, PMSG_HIBERNATE);
+	if (pm)
+		return pm_generic_poweroff(dev);
+	else
+		return i2c_legacy_suspend(dev, PMSG_HIBERNATE);
 }
 
 static int i2c_device_pm_restore(struct device *dev)
@@ -267,7 +248,7 @@ static int i2c_device_pm_restore(struct device *dev)
 	int ret;
 
 	if (pm)
-		ret = pm->restore ? pm->restore(dev) : 0;
+		ret = pm_generic_restore(dev);
 	else
 		ret = i2c_legacy_resume(dev);
 
-- 
1.7.2.3

Re: [PATCH 1/2] PM: Prototype the pm_generic_ operations

[Rafael J. Wysocki]

On Monday, December 20, 2010, Mark Brown wrote:
> The pm_generic_ operations are all exported but are not prototyped in any
> header file for direct use. Do so.
> 
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

I'd take this one, but the second one depends on it.

I can take both if Jean agrees.

Thanks,
Rafael


> ---
>  include/linux/pm.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 40f3f45..f7b5bed 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -610,4 +610,11 @@ extern unsigned int	pm_flags;
>  #define PM_APM	1
>  #define PM_ACPI	2
>  
> +int pm_generic_suspend(struct device *dev);
> +int pm_generic_resume(struct device *dev);
> +int pm_generic_freeze(struct device *dev);
> +int pm_generic_thaw(struct device *dev);
> +int pm_generic_restore(struct device *dev);
> +int pm_generic_poweroff(struct device *dev);
> +
>  #endif /* _LINUX_PM_H */
> 

Re: [linux-pm] [PATCH 1/2] PM: Prototype the pm_generic_ operations

[Mark Brown]

On Mon, Dec 20, 2010 at 10:08:58PM +0100, Rafael J. Wysocki wrote:

> I'd take this one, but the second one depends on it.

> I can take both if Jean agrees.

Given the nearness of the merge window another option is to apply this
one now and punt the I2C one until 2.6.38, I guess?

Re: [linux-pm] [PATCH 1/2] PM: Prototype the pm_generic_ operations

[Rafael J. Wysocki]

On Wednesday, December 22, 2010, Mark Brown wrote:
> On Mon, Dec 20, 2010 at 10:08:58PM +0100, Rafael J. Wysocki wrote:
> 
> > I'd take this one, but the second one depends on it.
> 
> > I can take both if Jean agrees.
> 
> Given the nearness of the merge window another option is to apply this
> one now and punt the I2C one until 2.6.38, I guess?

Except that I don't have any more fixes for 2.6.37 pending
and this one isn't really a fix. :-)

If there's anything else I should push to Linus before 2.6.37, I'll
include this patch in the pull request.  Otherwise I think it shoul go in
during the merge window.

Thanks,
Rafael

Re: [linux-pm] [PATCH 1/2] PM: Prototype the pm_generic_ operations

[Mark Brown]

On Wed, Dec 22, 2010 at 01:15:06PM +0100, Rafael J. Wysocki wrote:
> On Wednesday, December 22, 2010, Mark Brown wrote:

> > Given the nearness of the merge window another option is to apply this
> > one now and punt the I2C one until 2.6.38, I guess?

> Except that I don't have any more fixes for 2.6.37 pending
> and this one isn't really a fix. :-)

Oh, sorry - I mean during the merge window rather than for 2.6.37.

Re: [linux-pm] [PATCH 2/2] i2c: Factor out runtime suspend checks from PM operations

[Rabin Vincent]

On Mon, Dec 20, 2010 at 8:03 PM, Mark Brown
<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>  static int i2c_device_pm_restore(struct device *dev)
> @@ -267,7 +248,7 @@ static int i2c_device_pm_restore(struct device *dev)
>        int ret;
>
>        if (pm)
> -               ret = pm->restore ? pm->restore(dev) : 0;
> +               ret = pm_generic_restore(dev);
>        else
>                ret = i2c_legacy_resume(dev);

The full function is:

static int i2c_device_pm_restore(struct device *dev)
{
	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
	int ret;

	if (pm)
		ret = pm->restore ? pm->restore(dev) : 0;
	else
		ret = i2c_legacy_resume(dev);

	if (!ret) {
		pm_runtime_disable(dev);
		pm_runtime_set_active(dev);
		pm_runtime_enable(dev);
	}

	return ret;
}

Shouldn't you be deleting the pm_runtime_* stuff from here?  There is
already done in pm_generic_restore() iff the callback exists and returns
zero.

Re: [linux-pm] [PATCH 2/2] i2c: Factor out runtime suspend checks from PM operations

[Mark Brown]

On Wed, Dec 22, 2010 at 08:40:23PM +0530, Rabin Vincent wrote:

> Shouldn't you be deleting the pm_runtime_* stuff from here?  There is
> already done in pm_generic_restore() iff the callback exists and returns
> zero.

I guess; it's not clear why this is being done by the bus at all or how
it interacts with the legacy stuff.

Re: [linux-pm] [PATCH 2/2] i2c: Factor out runtime suspend checks from PM operations

[Rafael J. Wysocki]

On Wednesday, December 22, 2010, Mark Brown wrote:
> On Wed, Dec 22, 2010 at 08:40:23PM +0530, Rabin Vincent wrote:
> 
> > Shouldn't you be deleting the pm_runtime_* stuff from here?  There is
> > already done in pm_generic_restore() iff the callback exists and returns
> > zero.
> 
> I guess; it's not clear why this is being done by the bus at all or how
> it interacts with the legacy stuff.

It is done, because when the driver's ->resume() or ->restore() brings the
device up (which should have happened if it returned 0), we need to mark
the device as "active" for runtime PM.

Since __pm_generic_resume() does that, it's not necessary to do it in the
bus type callbacks (in the "legacy" case we known that runtime PM is not
supported).

Thanks,
Rafael

Re: [PATCH 1/2] PM: Prototype the pm_generic_ operations

[Rafael J. Wysocki]

On Monday, December 20, 2010, Mark Brown wrote:
> The pm_generic_ operations are all exported but are not prototyped in any
> header file for direct use. Do so.
> 
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

Applied to suspend-2.6/linux-next.

Thanks,
Rafael


> ---
>  include/linux/pm.h |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 40f3f45..f7b5bed 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -610,4 +610,11 @@ extern unsigned int	pm_flags;
>  #define PM_APM	1
>  #define PM_ACPI	2
>  
> +int pm_generic_suspend(struct device *dev);
> +int pm_generic_resume(struct device *dev);
> +int pm_generic_freeze(struct device *dev);
> +int pm_generic_thaw(struct device *dev);
> +int pm_generic_restore(struct device *dev);
> +int pm_generic_poweroff(struct device *dev);
> +
>  #endif /* _LINUX_PM_H */
>