[RFC][PATCH] Power domains for platform bus type

[RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

Hi,

This is something we discussed during the last Linux Plumbers Conference.

The problem appears to be that the same device may be used in different
systems in different configurations such that actions necessary for the
device's power management can vary from one system to another.  In those
cases the drivers' power management callbacks are generally not sufficient,
because they can't take the configuration of the whole system into account.

I think this issue may be addressed by adding objects that will represent
power domains and will provide power management callbacks to be executed
in addition to the device driver's PM callbacks, which is done by the patch
below.

Please have a look at it and tell me what you think.

Thanks,
Rafael

---
The platform bus type is often used to represent Systems-on-a-Chip
(SoC) where all devices are represented by objects of type struct
platform_device.  In those cases the same "platform" device driver
may be used in multiple different system configurations, but the
actions needed to put the devices it handles into a low-power state
and back into the full-power state may depend on the design of the
SoC.  The driver, however, cannot possibly include all the
information necessary for the power management of its device on all
the systems it's used with.  Moreover, the device hierarchy also
isn't suitable for holding this kind of information.

The patch below attempts to address this problem by introducing
objects of type struct power_domain that can be used for representing
power domains inside of the SoC.  Every struct power_domain object
consists of two sets of device power management callbacks that
can be used to perform what's needed for device power management
in addition to the operations carried out by the device's driver.
Namely, if a struct power_domain object is pointed to by the domain
field in a struct platform_device, the callbacks provided by its
pre_ops member will be executed for the dev member of that
struct platform_device before executing the corresponding callbacks
provided by the device's driver.  Analogously, the power domain's
post_ops callbacks will be executed after the corresponding callbacks
provided by the device's driver.
---
 drivers/base/platform.c         |  266 ++++++++++++++++++++++++++++------------
 include/linux/platform_device.h |    6 
 2 files changed, 198 insertions(+), 74 deletions(-)

Index: linux-2.6/include/linux/platform_device.h
===================================================================
--- linux-2.6.orig/include/linux/platform_device.h
+++ linux-2.6/include/linux/platform_device.h
@@ -14,6 +14,11 @@
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 
+struct power_domain {
+	struct dev_pm_ops *pre_ops;
+	struct dev_pm_ops *post_ops;
+};
+
 struct platform_device {
 	const char	* name;
 	int		id;
@@ -22,6 +27,7 @@ struct platform_device {
 	struct resource	* resource;
 
 	const struct platform_device_id	*id_entry;
+	const struct power_domain *domain;
 
 	/* arch specific additions */
 	struct pdev_archdata	archdata;
Index: linux-2.6/drivers/base/platform.c
===================================================================
--- linux-2.6.orig/drivers/base/platform.c
+++ linux-2.6/drivers/base/platform.c
@@ -697,68 +697,98 @@ static void platform_pm_complete(struct
 int __weak platform_pm_suspend(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->suspend)
+		pd->pre_ops->suspend(dev);
 
-	if (drv->pm) {
-		if (drv->pm->suspend)
-			ret = drv->pm->suspend(dev);
-	} else {
-		ret = platform_legacy_suspend(dev, PMSG_SUSPEND);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->suspend)
+				ret = drv->pm->suspend(dev);
+		} else {
+			ret = platform_legacy_suspend(dev, PMSG_SUSPEND);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->suspend)
+		pd->post_ops->suspend(dev);
+
 	return ret;
 }
 
 int __weak platform_pm_suspend_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->suspend_noirq)
+		pd->pre_ops->suspend_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->suspend_noirq)
-			ret = drv->pm->suspend_noirq(dev);
+	if (drv && drv->pm && drv->pm->suspend_noirq) {
+		ret = drv->pm->suspend_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->suspend_noirq)
+		pd->post_ops->suspend_noirq(dev);
+
 	return ret;
 }
 
 int __weak platform_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->resume)
+		pd->pre_ops->resume(dev);
 
-	if (drv->pm) {
-		if (drv->pm->resume)
-			ret = drv->pm->resume(dev);
-	} else {
-		ret = platform_legacy_resume(dev);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->resume)
+				ret = drv->pm->resume(dev);
+		} else {
+			ret = platform_legacy_resume(dev);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->resume)
+		pd->post_ops->resume(dev);
+
 	return ret;
 }
 
 int __weak platform_pm_resume_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->resume_noirq)
+		pd->pre_ops->resume_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->resume_noirq)
-			ret = drv->pm->resume_noirq(dev);
+	if (drv && drv->pm && drv->pm->resume_noirq) {
+		ret = drv->pm->resume_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->resume_noirq)
+		pd->post_ops->resume_noirq(dev);
+
 	return ret;
 }
 
@@ -776,136 +806,196 @@ int __weak platform_pm_resume_noirq(stru
 static int platform_pm_freeze(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->freeze)
+		pd->pre_ops->freeze(dev);
 
-	if (drv->pm) {
-		if (drv->pm->freeze)
-			ret = drv->pm->freeze(dev);
-	} else {
-		ret = platform_legacy_suspend(dev, PMSG_FREEZE);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->freeze)
+				ret = drv->pm->freeze(dev);
+		} else {
+			ret = platform_legacy_suspend(dev, PMSG_FREEZE);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->freeze)
+		pd->post_ops->freeze(dev);
+
 	return ret;
 }
 
 static int platform_pm_freeze_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->freeze_noirq)
+		pd->pre_ops->freeze_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->freeze_noirq)
-			ret = drv->pm->freeze_noirq(dev);
+	if (drv && drv->pm && drv->pm->freeze_noirq) {
+		ret = drv->pm->freeze_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->freeze_noirq)
+		pd->post_ops->freeze_noirq(dev);
+
 	return ret;
 }
 
 static int platform_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->thaw)
+		pd->pre_ops->thaw(dev);
 
-	if (drv->pm) {
-		if (drv->pm->thaw)
-			ret = drv->pm->thaw(dev);
-	} else {
-		ret = platform_legacy_resume(dev);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->thaw)
+				ret = drv->pm->thaw(dev);
+		} else {
+			ret = platform_legacy_resume(dev);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->thaw)
+		pd->post_ops->thaw(dev);
+
 	return ret;
 }
 
 static int platform_pm_thaw_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->thaw_noirq)
+		pd->pre_ops->thaw_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->thaw_noirq)
-			ret = drv->pm->thaw_noirq(dev);
+	if (drv && drv->pm && drv->pm->thaw_noirq) {
+		ret = drv->pm->thaw_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->thaw_noirq)
+		pd->post_ops->thaw_noirq(dev);
+
 	return ret;
 }
 
 static int platform_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->poweroff)
+		pd->pre_ops->poweroff(dev);
 
-	if (drv->pm) {
-		if (drv->pm->poweroff)
-			ret = drv->pm->poweroff(dev);
-	} else {
-		ret = platform_legacy_suspend(dev, PMSG_HIBERNATE);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->poweroff)
+				ret = drv->pm->poweroff(dev);
+		} else {
+			ret = platform_legacy_suspend(dev, PMSG_HIBERNATE);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->poweroff)
+		pd->post_ops->poweroff(dev);
+
 	return ret;
 }
 
 static int platform_pm_poweroff_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->poweroff_noirq)
+		pd->pre_ops->poweroff_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->poweroff_noirq)
-			ret = drv->pm->poweroff_noirq(dev);
+	if (drv && drv->pm && drv->pm->poweroff_noirq) {
+		ret = drv->pm->poweroff_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->poweroff_noirq)
+		pd->post_ops->poweroff_noirq(dev);
+
 	return ret;
 }
 
 static int platform_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->restore)
+		pd->pre_ops->restore(dev);
 
-	if (drv->pm) {
-		if (drv->pm->restore)
-			ret = drv->pm->restore(dev);
-	} else {
-		ret = platform_legacy_resume(dev);
+	if (drv) {
+		if (drv->pm) {
+			if (drv->pm->restore)
+				ret = drv->pm->restore(dev);
+		} else {
+			ret = platform_legacy_resume(dev);
+		}
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->restore)
+		pd->post_ops->restore(dev);
+
 	return ret;
 }
 
 static int platform_pm_restore_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
 	int ret = 0;
 
-	if (!drv)
-		return 0;
+	if (pd && pd->pre_ops && pd->pre_ops->restore_noirq)
+		pd->pre_ops->restore_noirq(dev);
 
-	if (drv->pm) {
-		if (drv->pm->restore_noirq)
-			ret = drv->pm->restore_noirq(dev);
+	if (drv && drv->pm && drv->pm->restore_noirq) {
+		ret = drv->pm->restore_noirq(dev);
+		if (ret)
+			return ret;
 	}
 
+	if (pd && pd->post_ops && pd->post_ops->restore_noirq)
+		pd->post_ops->restore_noirq(dev);
+
 	return ret;
 }
 
@@ -926,12 +1016,40 @@ static int platform_pm_restore_noirq(str
 
 int __weak platform_pm_runtime_suspend(struct device *dev)
 {
-	return pm_generic_runtime_suspend(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
+	int ret;
+
+	if (pd && pd->pre_ops && pd->pre_ops->runtime_suspend)
+		pd->pre_ops->runtime_suspend(dev);
+
+	ret = pm_generic_runtime_suspend(dev);
+	if (ret)
+		return ret;
+
+	if (pd && pd->post_ops && pd->post_ops->runtime_suspend)
+		pd->post_ops->runtime_suspend(dev);
+
+	return 0;
 };
 
 int __weak platform_pm_runtime_resume(struct device *dev)
 {
-	return pm_generic_runtime_resume(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct power_domain *pd = pdev->domain;
+	int ret;
+
+	if (pd && pd->pre_ops && pd->pre_ops->runtime_resume)
+		pd->pre_ops->runtime_resume(dev);
+
+	ret = pm_generic_runtime_resume(dev);
+	if (ret)
+		return ret;
+
+	if (pd && pd->post_ops && pd->post_ops->runtime_resume)
+		pd->post_ops->runtime_resume(dev);
+
+	return 0;
 };
 
 int __weak platform_pm_runtime_idle(struct device *dev)

Re: [RFC][PATCH] Power domains for platform bus type

[Alan Stern]

On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:

> Hi,
> 
> This is something we discussed during the last Linux Plumbers Conference.
> 
> The problem appears to be that the same device may be used in different
> systems in different configurations such that actions necessary for the
> device's power management can vary from one system to another.  In those
> cases the drivers' power management callbacks are generally not sufficient,
> because they can't take the configuration of the whole system into account.
> 
> I think this issue may be addressed by adding objects that will represent
> power domains and will provide power management callbacks to be executed
> in addition to the device driver's PM callbacks, which is done by the patch
> below.
> 
> Please have a look at it and tell me what you think.

One thing about this implementation is slightly questionable.  The new
power_domain callbacks were added to the __weak platform PM routines,
which means they will have to be included in every overriding routine
provided by a platform imiplementation.

Would it be better to separate these things?  Have the power_domain 
callbacks occur in a static outer function which then calls a public 
__weak inner function that can be overridden?

Alan Stern

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Sunday, January 30, 2011, Alan Stern wrote:
> On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > This is something we discussed during the last Linux Plumbers Conference.
> > 
> > The problem appears to be that the same device may be used in different
> > systems in different configurations such that actions necessary for the
> > device's power management can vary from one system to another.  In those
> > cases the drivers' power management callbacks are generally not sufficient,
> > because they can't take the configuration of the whole system into account.
> > 
> > I think this issue may be addressed by adding objects that will represent
> > power domains and will provide power management callbacks to be executed
> > in addition to the device driver's PM callbacks, which is done by the patch
> > below.
> > 
> > Please have a look at it and tell me what you think.
> 
> One thing about this implementation is slightly questionable.  The new
> power_domain callbacks were added to the __weak platform PM routines,
> which means they will have to be included in every overriding routine
> provided by a platform imiplementation.
> 
> Would it be better to separate these things?  Have the power_domain 
> callbacks occur in a static outer function which then calls a public 
> __weak inner function that can be overridden?

That certainly is a good idea, but I wasn't sure how to do that.  It looks
like I could keep the __weak functions as they are and modify
platform_dev_pm_ops instead to point to a new set of function that in turn
would call the __weak ones.  For example, the .suspend pointer in
platform_dev_pm_ops might point to a new function, say
platform_pm_full_suspend() that would call the power domain functions and
the "original" platform_pm_suspend().  Is that what you mean?

Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Mark Brown]

On Sun, Jan 30, 2011 at 01:07:19AM +0100, Rafael J. Wysocki wrote:

> This is something we discussed during the last Linux Plumbers Conference.

Adding CC to Kukjin as the Samsung CPUs also have some power domain
support and not cutting text for him.

> The problem appears to be that the same device may be used in different
> systems in different configurations such that actions necessary for the
> device's power management can vary from one system to another.  In those
> cases the drivers' power management callbacks are generally not sufficient,
> because they can't take the configuration of the whole system into account.
> 
> I think this issue may be addressed by adding objects that will represent
> power domains and will provide power management callbacks to be executed
> in addition to the device driver's PM callbacks, which is done by the patch
> below.
> 
> Please have a look at it and tell me what you think.
> 
> Thanks,
> Rafael
> 
> ---
> The platform bus type is often used to represent Systems-on-a-Chip
> (SoC) where all devices are represented by objects of type struct
> platform_device.  In those cases the same "platform" device driver
> may be used in multiple different system configurations, but the
> actions needed to put the devices it handles into a low-power state
> and back into the full-power state may depend on the design of the
> SoC.  The driver, however, cannot possibly include all the
> information necessary for the power management of its device on all
> the systems it's used with.  Moreover, the device hierarchy also
> isn't suitable for holding this kind of information.
> 
> The patch below attempts to address this problem by introducing
> objects of type struct power_domain that can be used for representing
> power domains inside of the SoC.  Every struct power_domain object
> consists of two sets of device power management callbacks that
> can be used to perform what's needed for device power management
> in addition to the operations carried out by the device's driver.
> Namely, if a struct power_domain object is pointed to by the domain
> field in a struct platform_device, the callbacks provided by its
> pre_ops member will be executed for the dev member of that
> struct platform_device before executing the corresponding callbacks
> provided by the device's driver.  Analogously, the power domain's
> post_ops callbacks will be executed after the corresponding callbacks
> provided by the device's driver.
> ---
>  drivers/base/platform.c         |  266 ++++++++++++++++++++++++++++------------
>  include/linux/platform_device.h |    6 
>  2 files changed, 198 insertions(+), 74 deletions(-)
> 
> Index: linux-2.6/include/linux/platform_device.h
> ===================================================================
> --- linux-2.6.orig/include/linux/platform_device.h
> +++ linux-2.6/include/linux/platform_device.h
> @@ -14,6 +14,11 @@
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>  
> +struct power_domain {
> +	struct dev_pm_ops *pre_ops;
> +	struct dev_pm_ops *post_ops;
> +};
> +
>  struct platform_device {
>  	const char	* name;
>  	int		id;
> @@ -22,6 +27,7 @@ struct platform_device {
>  	struct resource	* resource;
>  
>  	const struct platform_device_id	*id_entry;
> +	const struct power_domain *domain;
>  
>  	/* arch specific additions */
>  	struct pdev_archdata	archdata;
> Index: linux-2.6/drivers/base/platform.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/platform.c
> +++ linux-2.6/drivers/base/platform.c
> @@ -697,68 +697,98 @@ static void platform_pm_complete(struct
>  int __weak platform_pm_suspend(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->suspend)
> +		pd->pre_ops->suspend(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->suspend)
> -			ret = drv->pm->suspend(dev);
> -	} else {
> -		ret = platform_legacy_suspend(dev, PMSG_SUSPEND);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->suspend)
> +				ret = drv->pm->suspend(dev);
> +		} else {
> +			ret = platform_legacy_suspend(dev, PMSG_SUSPEND);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->suspend)
> +		pd->post_ops->suspend(dev);
> +
>  	return ret;
>  }
>  
>  int __weak platform_pm_suspend_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->suspend_noirq)
> +		pd->pre_ops->suspend_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->suspend_noirq)
> -			ret = drv->pm->suspend_noirq(dev);
> +	if (drv && drv->pm && drv->pm->suspend_noirq) {
> +		ret = drv->pm->suspend_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->suspend_noirq)
> +		pd->post_ops->suspend_noirq(dev);
> +
>  	return ret;
>  }
>  
>  int __weak platform_pm_resume(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->resume)
> +		pd->pre_ops->resume(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->resume)
> -			ret = drv->pm->resume(dev);
> -	} else {
> -		ret = platform_legacy_resume(dev);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->resume)
> +				ret = drv->pm->resume(dev);
> +		} else {
> +			ret = platform_legacy_resume(dev);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->resume)
> +		pd->post_ops->resume(dev);
> +
>  	return ret;
>  }
>  
>  int __weak platform_pm_resume_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->resume_noirq)
> +		pd->pre_ops->resume_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->resume_noirq)
> -			ret = drv->pm->resume_noirq(dev);
> +	if (drv && drv->pm && drv->pm->resume_noirq) {
> +		ret = drv->pm->resume_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->resume_noirq)
> +		pd->post_ops->resume_noirq(dev);
> +
>  	return ret;
>  }
>  
> @@ -776,136 +806,196 @@ int __weak platform_pm_resume_noirq(stru
>  static int platform_pm_freeze(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->freeze)
> +		pd->pre_ops->freeze(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->freeze)
> -			ret = drv->pm->freeze(dev);
> -	} else {
> -		ret = platform_legacy_suspend(dev, PMSG_FREEZE);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->freeze)
> +				ret = drv->pm->freeze(dev);
> +		} else {
> +			ret = platform_legacy_suspend(dev, PMSG_FREEZE);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->freeze)
> +		pd->post_ops->freeze(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_freeze_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->freeze_noirq)
> +		pd->pre_ops->freeze_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->freeze_noirq)
> -			ret = drv->pm->freeze_noirq(dev);
> +	if (drv && drv->pm && drv->pm->freeze_noirq) {
> +		ret = drv->pm->freeze_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->freeze_noirq)
> +		pd->post_ops->freeze_noirq(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_thaw(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->thaw)
> +		pd->pre_ops->thaw(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->thaw)
> -			ret = drv->pm->thaw(dev);
> -	} else {
> -		ret = platform_legacy_resume(dev);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->thaw)
> +				ret = drv->pm->thaw(dev);
> +		} else {
> +			ret = platform_legacy_resume(dev);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->thaw)
> +		pd->post_ops->thaw(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_thaw_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->thaw_noirq)
> +		pd->pre_ops->thaw_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->thaw_noirq)
> -			ret = drv->pm->thaw_noirq(dev);
> +	if (drv && drv->pm && drv->pm->thaw_noirq) {
> +		ret = drv->pm->thaw_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->thaw_noirq)
> +		pd->post_ops->thaw_noirq(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_poweroff(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->poweroff)
> +		pd->pre_ops->poweroff(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->poweroff)
> -			ret = drv->pm->poweroff(dev);
> -	} else {
> -		ret = platform_legacy_suspend(dev, PMSG_HIBERNATE);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->poweroff)
> +				ret = drv->pm->poweroff(dev);
> +		} else {
> +			ret = platform_legacy_suspend(dev, PMSG_HIBERNATE);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->poweroff)
> +		pd->post_ops->poweroff(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_poweroff_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->poweroff_noirq)
> +		pd->pre_ops->poweroff_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->poweroff_noirq)
> -			ret = drv->pm->poweroff_noirq(dev);
> +	if (drv && drv->pm && drv->pm->poweroff_noirq) {
> +		ret = drv->pm->poweroff_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->poweroff_noirq)
> +		pd->post_ops->poweroff_noirq(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_restore(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->restore)
> +		pd->pre_ops->restore(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->restore)
> -			ret = drv->pm->restore(dev);
> -	} else {
> -		ret = platform_legacy_resume(dev);
> +	if (drv) {
> +		if (drv->pm) {
> +			if (drv->pm->restore)
> +				ret = drv->pm->restore(dev);
> +		} else {
> +			ret = platform_legacy_resume(dev);
> +		}
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->restore)
> +		pd->post_ops->restore(dev);
> +
>  	return ret;
>  }
>  
>  static int platform_pm_restore_noirq(struct device *dev)
>  {
>  	struct device_driver *drv = dev->driver;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
>  	int ret = 0;
>  
> -	if (!drv)
> -		return 0;
> +	if (pd && pd->pre_ops && pd->pre_ops->restore_noirq)
> +		pd->pre_ops->restore_noirq(dev);
>  
> -	if (drv->pm) {
> -		if (drv->pm->restore_noirq)
> -			ret = drv->pm->restore_noirq(dev);
> +	if (drv && drv->pm && drv->pm->restore_noirq) {
> +		ret = drv->pm->restore_noirq(dev);
> +		if (ret)
> +			return ret;
>  	}
>  
> +	if (pd && pd->post_ops && pd->post_ops->restore_noirq)
> +		pd->post_ops->restore_noirq(dev);
> +
>  	return ret;
>  }
>  
> @@ -926,12 +1016,40 @@ static int platform_pm_restore_noirq(str
>  
>  int __weak platform_pm_runtime_suspend(struct device *dev)
>  {
> -	return pm_generic_runtime_suspend(dev);
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
> +	int ret;
> +
> +	if (pd && pd->pre_ops && pd->pre_ops->runtime_suspend)
> +		pd->pre_ops->runtime_suspend(dev);
> +
> +	ret = pm_generic_runtime_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (pd && pd->post_ops && pd->post_ops->runtime_suspend)
> +		pd->post_ops->runtime_suspend(dev);
> +
> +	return 0;
>  };
>  
>  int __weak platform_pm_runtime_resume(struct device *dev)
>  {
> -	return pm_generic_runtime_resume(dev);
> +	struct platform_device *pdev = to_platform_device(dev);
> +	const struct power_domain *pd = pdev->domain;
> +	int ret;
> +
> +	if (pd && pd->pre_ops && pd->pre_ops->runtime_resume)
> +		pd->pre_ops->runtime_resume(dev);
> +
> +	ret = pm_generic_runtime_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (pd && pd->post_ops && pd->post_ops->runtime_resume)
> +		pd->post_ops->runtime_resume(dev);
> +
> +	return 0;
>  };
>  
>  int __weak platform_pm_runtime_idle(struct device *dev)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

Re: [RFC][PATCH] Power domains for platform bus type

[Alan Stern]

On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:

> > One thing about this implementation is slightly questionable.  The new
> > power_domain callbacks were added to the __weak platform PM routines,
> > which means they will have to be included in every overriding routine
> > provided by a platform imiplementation.
> > 
> > Would it be better to separate these things?  Have the power_domain 
> > callbacks occur in a static outer function which then calls a public 
> > __weak inner function that can be overridden?
> 
> That certainly is a good idea, but I wasn't sure how to do that.  It looks
> like I could keep the __weak functions as they are and modify
> platform_dev_pm_ops instead to point to a new set of function that in turn
> would call the __weak ones.  For example, the .suspend pointer in
> platform_dev_pm_ops might point to a new function, say
> platform_pm_full_suspend() that would call the power domain functions and
> the "original" platform_pm_suspend().  Is that what you mean?

Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
platform-specific replacements for the pm_ops functions also include
the power_domain callbacks?

Alan Stern

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Monday, January 31, 2011, Alan Stern wrote:
> On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> 
> > > One thing about this implementation is slightly questionable.  The new
> > > power_domain callbacks were added to the __weak platform PM routines,
> > > which means they will have to be included in every overriding routine
> > > provided by a platform imiplementation.
> > > 
> > > Would it be better to separate these things?  Have the power_domain 
> > > callbacks occur in a static outer function which then calls a public 
> > > __weak inner function that can be overridden?
> > 
> > That certainly is a good idea, but I wasn't sure how to do that.  It looks
> > like I could keep the __weak functions as they are and modify
> > platform_dev_pm_ops instead to point to a new set of function that in turn
> > would call the __weak ones.  For example, the .suspend pointer in
> > platform_dev_pm_ops might point to a new function, say
> > platform_pm_full_suspend() that would call the power domain functions and
> > the "original" platform_pm_suspend().  Is that what you mean?
> 
> Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
> platform-specific replacements for the pm_ops functions also include
> the power_domain callbacks?

Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
domains from being used by not defining them in the first place. :-)

Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Alan Stern]

On Mon, 31 Jan 2011, Rafael J. Wysocki wrote:

> On Monday, January 31, 2011, Alan Stern wrote:
> > On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> > 
> > > > One thing about this implementation is slightly questionable.  The new
> > > > power_domain callbacks were added to the __weak platform PM routines,
> > > > which means they will have to be included in every overriding routine
> > > > provided by a platform imiplementation.
> > > > 
> > > > Would it be better to separate these things?  Have the power_domain 
> > > > callbacks occur in a static outer function which then calls a public 
> > > > __weak inner function that can be overridden?
> > > 
> > > That certainly is a good idea, but I wasn't sure how to do that.  It looks
> > > like I could keep the __weak functions as they are and modify
> > > platform_dev_pm_ops instead to point to a new set of function that in turn
> > > would call the __weak ones.  For example, the .suspend pointer in
> > > platform_dev_pm_ops might point to a new function, say
> > > platform_pm_full_suspend() that would call the power domain functions and
> > > the "original" platform_pm_suspend().  Is that what you mean?
> > 
> > Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
> > platform-specific replacements for the pm_ops functions also include
> > the power_domain callbacks?
> 
> Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
> domains from being used by not defining them in the first place. :-)

But what about the case where the user _does_ want to have power
domains?  Do you want to make the replacement routines responsible for
invoking the power-domain callbacks, or should the platform core handle
this automatically?

Alan Stern

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Monday, January 31, 2011, Alan Stern wrote:
> On Mon, 31 Jan 2011, Rafael J. Wysocki wrote:
> 
> > On Monday, January 31, 2011, Alan Stern wrote:
> > > On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> > > 
> > > > > One thing about this implementation is slightly questionable.  The new
> > > > > power_domain callbacks were added to the __weak platform PM routines,
> > > > > which means they will have to be included in every overriding routine
> > > > > provided by a platform imiplementation.
> > > > > 
> > > > > Would it be better to separate these things?  Have the power_domain 
> > > > > callbacks occur in a static outer function which then calls a public 
> > > > > __weak inner function that can be overridden?
> > > > 
> > > > That certainly is a good idea, but I wasn't sure how to do that.  It looks
> > > > like I could keep the __weak functions as they are and modify
> > > > platform_dev_pm_ops instead to point to a new set of function that in turn
> > > > would call the __weak ones.  For example, the .suspend pointer in
> > > > platform_dev_pm_ops might point to a new function, say
> > > > platform_pm_full_suspend() that would call the power domain functions and
> > > > the "original" platform_pm_suspend().  Is that what you mean?
> > > 
> > > Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
> > > platform-specific replacements for the pm_ops functions also include
> > > the power_domain callbacks?
> > 
> > Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
> > domains from being used by not defining them in the first place. :-)
> 
> But what about the case where the user _does_ want to have power
> domains?

Ah, OK.  The caller of platform_bus_set_pm_ops() will replace the original
platform_dev_pm_ops with his own set of operations, so he will not see the
power domains.

> Do you want to make the replacement routines responsible for
> invoking the power-domain callbacks, or should the platform core handle
> this automatically?

Well, if someone replaces the entire platform_dev_pm_ops object, this means
that on his platform power management is substantially different from the
generic one.  In that case, IMO, he should be responsible for handling all
of the subsystem-level aspects of power management, including power domains.

Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Grant Likely]

On Mon, Jan 31, 2011 at 11:16:51PM +0100, Rafael J. Wysocki wrote:
> On Monday, January 31, 2011, Alan Stern wrote:
> > On Mon, 31 Jan 2011, Rafael J. Wysocki wrote:
> > 
> > > On Monday, January 31, 2011, Alan Stern wrote:
> > > > On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> > > > 
> > > > > > One thing about this implementation is slightly questionable.  The new
> > > > > > power_domain callbacks were added to the __weak platform PM routines,
> > > > > > which means they will have to be included in every overriding routine
> > > > > > provided by a platform imiplementation.
> > > > > > 
> > > > > > Would it be better to separate these things?  Have the power_domain 
> > > > > > callbacks occur in a static outer function which then calls a public 
> > > > > > __weak inner function that can be overridden?
> > > > > 
> > > > > That certainly is a good idea, but I wasn't sure how to do that.  It looks
> > > > > like I could keep the __weak functions as they are and modify
> > > > > platform_dev_pm_ops instead to point to a new set of function that in turn
> > > > > would call the __weak ones.  For example, the .suspend pointer in
> > > > > platform_dev_pm_ops might point to a new function, say
> > > > > platform_pm_full_suspend() that would call the power domain functions and
> > > > > the "original" platform_pm_suspend().  Is that what you mean?
> > > > 
> > > > Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
> > > > platform-specific replacements for the pm_ops functions also include
> > > > the power_domain callbacks?
> > > 
> > > Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
> > > domains from being used by not defining them in the first place. :-)
> > 
> > But what about the case where the user _does_ want to have power
> > domains?
> 
> Ah, OK.  The caller of platform_bus_set_pm_ops() will replace the original
> platform_dev_pm_ops with his own set of operations, so he will not see the
> power domains.
> 
> > Do you want to make the replacement routines responsible for
> > invoking the power-domain callbacks, or should the platform core handle
> > this automatically?
> 
> Well, if someone replaces the entire platform_dev_pm_ops object, this means
> that on his platform power management is substantially different from the
> generic one.  In that case, IMO, he should be responsible for handling all
> of the subsystem-level aspects of power management, including power domains.

Part of point of doing something like power_domain is to *get rid* of
platform_bus_set_pm_ops().  It is a horrid, stop-gap interface that
doesn't scale.  I don't think much consideration needs to be made for
users of platform_bus_set_pm_ops() in this regard.

g.

Re: [RFC][PATCH] Power domains for platform bus type

[Kevin Hilman]

Grant Likely <grant.likely@secretlab.ca> writes:

> On Mon, Jan 31, 2011 at 11:16:51PM +0100, Rafael J. Wysocki wrote:
>> On Monday, January 31, 2011, Alan Stern wrote:
>> > On Mon, 31 Jan 2011, Rafael J. Wysocki wrote:
>> > 
>> > > On Monday, January 31, 2011, Alan Stern wrote:
>> > > > On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
>> > > > 
>> > > > > > One thing about this implementation is slightly questionable.  The new
>> > > > > > power_domain callbacks were added to the __weak platform PM routines,
>> > > > > > which means they will have to be included in every overriding routine
>> > > > > > provided by a platform imiplementation.
>> > > > > > 
>> > > > > > Would it be better to separate these things?  Have the power_domain 
>> > > > > > callbacks occur in a static outer function which then calls a public 
>> > > > > > __weak inner function that can be overridden?
>> > > > > 
>> > > > > That certainly is a good idea, but I wasn't sure how to do that.  It looks
>> > > > > like I could keep the __weak functions as they are and modify
>> > > > > platform_dev_pm_ops instead to point to a new set of function that in turn
>> > > > > would call the __weak ones.  For example, the .suspend pointer in
>> > > > > platform_dev_pm_ops might point to a new function, say
>> > > > > platform_pm_full_suspend() that would call the power domain functions and
>> > > > > the "original" platform_pm_suspend().  Is that what you mean?
>> > > > 
>> > > > Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
>> > > > platform-specific replacements for the pm_ops functions also include
>> > > > the power_domain callbacks?
>> > > 
>> > > Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
>> > > domains from being used by not defining them in the first place. :-)
>> > 
>> > But what about the case where the user _does_ want to have power
>> > domains?
>> 
>> Ah, OK.  The caller of platform_bus_set_pm_ops() will replace the original
>> platform_dev_pm_ops with his own set of operations, so he will not see the
>> power domains.
>> 
>> > Do you want to make the replacement routines responsible for
>> > invoking the power-domain callbacks, or should the platform core handle
>> > this automatically?
>> 
>> Well, if someone replaces the entire platform_dev_pm_ops object, this means
>> that on his platform power management is substantially different from the
>> generic one.  In that case, IMO, he should be responsible for handling all
>> of the subsystem-level aspects of power management, including power domains.
>
> Part of point of doing something like power_domain is to *get rid* of
> platform_bus_set_pm_ops().  It is a horrid, stop-gap interface that
> doesn't scale.  I don't think much consideration needs to be made for
> users of platform_bus_set_pm_ops() in this regard.

As the author of platform_bus_set_pm_ops(), I humbly agree.  

Also, the __weak functions here were obsoleted by
platform_bus_set_pm_ops().  Once Magnus moves to
platform_bus_set_pm_ops() (or this new interface) the __weak attributes
should be removed (c.f. commit log below[1] where
platform_bus_set_pm_ops() was added.)

Kevin

commit c64a0926710153b9d44c979d2942f4a8648fd74e
Author: Kevin Hilman <khilman@ti.com>
Date:   Wed Aug 25 12:50:00 2010 -0700

    driver core: platform_bus: allow runtime override of dev_pm_ops
    
    Currently, the platform_bus allows customization of several of the
    busses dev_pm_ops methods by using weak symbols so that platform code
    can override them.  The weak-symbol approach is not scalable when
    wanting to support multiple platforms in a single kernel binary.
    
    Instead, provide __init methods for platform code to customize the
    dev_pm_ops methods at runtime.
    
    NOTE: after these dynamic methods are merged, the weak symbols should
          be removed from drivers/base/platform.c.  AFAIK, this will only
          affect SH and sh-mobile which should be converted to use this
          runtime approach instead of the weak symbols.  After SH &
          sh-mobile are converted, the weak symobols could be removed.

Re: [RFC][PATCH] Power domains for platform bus type

[Grant Likely]

On Sun, Jan 30, 2011 at 01:07:19AM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> This is something we discussed during the last Linux Plumbers Conference.
> 
> The problem appears to be that the same device may be used in different
> systems in different configurations such that actions necessary for the
> device's power management can vary from one system to another.  In those
> cases the drivers' power management callbacks are generally not sufficient,
> because they can't take the configuration of the whole system into account.
> 
> I think this issue may be addressed by adding objects that will represent
> power domains and will provide power management callbacks to be executed
> in addition to the device driver's PM callbacks, which is done by the patch
> below.
> 
> Please have a look at it and tell me what you think.

In general it looks okay.  I agree with Alan's comment that it
probably belongs outside the platform device pm ops.  It's the sort of
thing that should be available to *any* device, regardless of bus
type.  ie. it is conceivable that some spi and i2c devices would be in
need to be in the same power_domain.

It slightly worries me about the amount of code required to manage all the
nested levels of pm_ops.  I wonder if there is a better way to manage
them.

Also, what is the use case for having 2 sets of power_domain ops?  My
gut tells me that you'd only want to do post ops on the
{freeze,suspend,poweroff} path and pre ops on the {resume,thaw,restore}
path.  It seems overly engineered to me, but I may be missing
something fundamental.

g.

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Monday, January 31, 2011, Kevin Hilman wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
> 
> > On Mon, Jan 31, 2011 at 11:16:51PM +0100, Rafael J. Wysocki wrote:
> >> On Monday, January 31, 2011, Alan Stern wrote:
> >> > On Mon, 31 Jan 2011, Rafael J. Wysocki wrote:
> >> > 
> >> > > On Monday, January 31, 2011, Alan Stern wrote:
> >> > > > On Sun, 30 Jan 2011, Rafael J. Wysocki wrote:
> >> > > > 
> >> > > > > > One thing about this implementation is slightly questionable.  The new
> >> > > > > > power_domain callbacks were added to the __weak platform PM routines,
> >> > > > > > which means they will have to be included in every overriding routine
> >> > > > > > provided by a platform imiplementation.
> >> > > > > > 
> >> > > > > > Would it be better to separate these things?  Have the power_domain 
> >> > > > > > callbacks occur in a static outer function which then calls a public 
> >> > > > > > __weak inner function that can be overridden?
> >> > > > > 
> >> > > > > That certainly is a good idea, but I wasn't sure how to do that.  It looks
> >> > > > > like I could keep the __weak functions as they are and modify
> >> > > > > platform_dev_pm_ops instead to point to a new set of function that in turn
> >> > > > > would call the __weak ones.  For example, the .suspend pointer in
> >> > > > > platform_dev_pm_ops might point to a new function, say
> >> > > > > platform_pm_full_suspend() that would call the power domain functions and
> >> > > > > the "original" platform_pm_suspend().  Is that what you mean?
> >> > > > 
> >> > > > Yes.  But what about the platform_bus_set_pm_ops() interface?  Should
> >> > > > platform-specific replacements for the pm_ops functions also include
> >> > > > the power_domain callbacks?
> >> > > 
> >> > > Well, whoever uses platform_bus_set_pm_ops(), he can simply prevent power
> >> > > domains from being used by not defining them in the first place. :-)
> >> > 
> >> > But what about the case where the user _does_ want to have power
> >> > domains?
> >> 
> >> Ah, OK.  The caller of platform_bus_set_pm_ops() will replace the original
> >> platform_dev_pm_ops with his own set of operations, so he will not see the
> >> power domains.
> >> 
> >> > Do you want to make the replacement routines responsible for
> >> > invoking the power-domain callbacks, or should the platform core handle
> >> > this automatically?
> >> 
> >> Well, if someone replaces the entire platform_dev_pm_ops object, this means
> >> that on his platform power management is substantially different from the
> >> generic one.  In that case, IMO, he should be responsible for handling all
> >> of the subsystem-level aspects of power management, including power domains.
> >
> > Part of point of doing something like power_domain is to *get rid* of
> > platform_bus_set_pm_ops().  It is a horrid, stop-gap interface that
> > doesn't scale.  I don't think much consideration needs to be made for
> > users of platform_bus_set_pm_ops() in this regard.
> 
> As the author of platform_bus_set_pm_ops(), I humbly agree.  
> 
> Also, the __weak functions here were obsoleted by
> platform_bus_set_pm_ops().  Once Magnus moves to
> platform_bus_set_pm_ops() (or this new interface) the __weak attributes
> should be removed (c.f. commit log below[1] where
> platform_bus_set_pm_ops() was added.)
> 
> Kevin
> 
> commit c64a0926710153b9d44c979d2942f4a8648fd74e
> Author: Kevin Hilman <khilman@ti.com>
> Date:   Wed Aug 25 12:50:00 2010 -0700
> 
>     driver core: platform_bus: allow runtime override of dev_pm_ops
>     
>     Currently, the platform_bus allows customization of several of the
>     busses dev_pm_ops methods by using weak symbols so that platform code
>     can override them.  The weak-symbol approach is not scalable when
>     wanting to support multiple platforms in a single kernel binary.
>     
>     Instead, provide __init methods for platform code to customize the
>     dev_pm_ops methods at runtime.
>     
>     NOTE: after these dynamic methods are merged, the weak symbols should
>           be removed from drivers/base/platform.c.  AFAIK, this will only
>           affect SH and sh-mobile which should be converted to use this
>           runtime approach instead of the weak symbols.  After SH &
>           sh-mobile are converted, the weak symobols could be removed.
> --

So, it seems there are two possibilities, either (1) keep the $subject patch
as is in the hope that Magnus will use power domains instead of overriding
the __weak callbacks and remove the _weak attribute when that happens, or (2)
modify it along the lines suggested by Alan (ie. so that the _weak callbacks
stay as they are, but they won't be pointed to by platform_dev_pm_ops directly).
I'm pretty much fine with each of them, so I'd prefer to do whichever is
generally more useful.  Magnus?

Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Monday, January 31, 2011, Grant Likely wrote:
> On Sun, Jan 30, 2011 at 01:07:19AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > This is something we discussed during the last Linux Plumbers Conference.
> > 
> > The problem appears to be that the same device may be used in different
> > systems in different configurations such that actions necessary for the
> > device's power management can vary from one system to another.  In those
> > cases the drivers' power management callbacks are generally not sufficient,
> > because they can't take the configuration of the whole system into account.
> > 
> > I think this issue may be addressed by adding objects that will represent
> > power domains and will provide power management callbacks to be executed
> > in addition to the device driver's PM callbacks, which is done by the patch
> > below.
> > 
> > Please have a look at it and tell me what you think.
> 
> In general it looks okay.  I agree with Alan's comment that it
> probably belongs outside the platform device pm ops.  It's the sort of
> thing that should be available to *any* device, regardless of bus
> type.

I'd rather say any subsystem.  Anyway, I'm not sure how many subsystems
will find it useful at the core level except for platform.

> ie. it is conceivable that some spi and i2c devices would be in
> need to be in the same power_domain.

So there's spi and i2c.  Anything else?

> It slightly worries me about the amount of code required to manage all the
> nested levels of pm_ops.  I wonder if there is a better way to manage
> them.

I'm not really sure.  The amount of code is kind of proportional to the
number of different callbacks in struct dev_pm_ops ...

> Also, what is the use case for having 2 sets of power_domain ops?  My
> gut tells me that you'd only want to do post ops on the
> {freeze,suspend,poweroff} path and pre ops on the {resume,thaw,restore}
> path.  It seems overly engineered to me, but I may be missing
> something fundamental.

Well, that's a part of the RFC, actually. :-)

For the subsystems I've worked with (PCI, ACPI, PNP to some extent) one set
would be sufficient, but I don't know of every possible use case.

Thanks,
Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Kevin Hilman]

Hi Rafael,

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> Hi,
>
> This is something we discussed during the last Linux Plumbers Conference.
>
> The problem appears to be that the same device may be used in different
> systems in different configurations such that actions necessary for the
> device's power management can vary from one system to another.  In those
> cases the drivers' power management callbacks are generally not sufficient,
> because they can't take the configuration of the whole system into account.
>
> I think this issue may be addressed by adding objects that will represent
> power domains and will provide power management callbacks to be executed
> in addition to the device driver's PM callbacks, which is done by the patch
> below.
>
> Please have a look at it and tell me what you think.

Very nice.  I like the approach and the fact that it allows grouping of
only devices we care about customizing, instead of every device on the
platform_bus (I know Grant will like that part too.  :)

I experimented with something similar before I took the easy way out
with platform_bus_set_pm_ops() :/

This approach might also solve the problem(s) we've been discussing
around the conflicts between runtime PM callbacks and system PM
callbacks (that RTC vs. i2c issue.)  With this approach, I shouldn't
have to to add the callbacks like I did for the i2c driver, but rather
handle them in common code.  I'll experiment with this...

The primary question for me is whether this should rather be at the
'struct device' level instead of at the platform_device level.  While
we're currently only using this customization on platform_devices, I
don't think we should limit it to that.  Part of these discussions at
LPC was also about whether or not to move to using custom SoC-specific
devices and busses.  If/when we go that route, we'd want these power
domains part of struct device, not platform_device.

It would be a rather minor change to your patch, but would be more
flexible for the future.

Some other minor comments below...

> ---
> The platform bus type is often used to represent Systems-on-a-Chip
> (SoC) where all devices are represented by objects of type struct
> platform_device.  In those cases the same "platform" device driver
> may be used in multiple different system configurations, but the
> actions needed to put the devices it handles into a low-power state
> and back into the full-power state may depend on the design of the
> SoC.  The driver, however, cannot possibly include all the
> information necessary for the power management of its device on all
> the systems it's used with.  Moreover, the device hierarchy also
> isn't suitable for holding this kind of information.
>
> The patch below attempts to address this problem by introducing
> objects of type struct power_domain that can be used for representing
> power domains inside of the SoC.  Every struct power_domain object
> consists of two sets of device power management callbacks that
> can be used to perform what's needed for device power management
> in addition to the operations carried out by the device's driver.
> Namely, if a struct power_domain object is pointed to by the domain
> field in a struct platform_device, the callbacks provided by its
> pre_ops member will be executed for the dev member of that
> struct platform_device before executing the corresponding callbacks
> provided by the device's driver.  Analogously, the power domain's
> post_ops callbacks will be executed after the corresponding callbacks
> provided by the device's driver.

You should probably add here that the power_domain callbacks will be
called even if a driver is not present for the device, which may or may
not be expected.

> ---
>  drivers/base/platform.c         |  266 ++++++++++++++++++++++++++++------------
>  include/linux/platform_device.h |    6 
>  2 files changed, 198 insertions(+), 74 deletions(-)
>
> Index: linux-2.6/include/linux/platform_device.h
> ===================================================================
> --- linux-2.6.orig/include/linux/platform_device.h
> +++ linux-2.6/include/linux/platform_device.h
> @@ -14,6 +14,11 @@
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>  
> +struct power_domain {
> +	struct dev_pm_ops *pre_ops;
> +	struct dev_pm_ops *post_ops;
> +};

Since most SoC code is also going to have something called 'struct
power_domain', I'd suggest naming this 'struct dev_power_domain' or
something to indicate it's part of the driver model, so it wouldn't get
confused with other SoC specific power_domain structs.

Thanks,

Kevin

Re: [RFC][PATCH] Power domains for platform bus type

[Grant Likely]

On Mon, Jan 31, 2011 at 03:16:15PM -0800, Kevin Hilman wrote:
> Hi Rafael,
> 
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > Hi,
> >
> > This is something we discussed during the last Linux Plumbers Conference.
> >
> > The problem appears to be that the same device may be used in different
> > systems in different configurations such that actions necessary for the
> > device's power management can vary from one system to another.  In those
> > cases the drivers' power management callbacks are generally not sufficient,
> > because they can't take the configuration of the whole system into account.
> >
> > I think this issue may be addressed by adding objects that will represent
> > power domains and will provide power management callbacks to be executed
> > in addition to the device driver's PM callbacks, which is done by the patch
> > below.
> >
> > Please have a look at it and tell me what you think.
> 
> Very nice.  I like the approach and the fact that it allows grouping of
> only devices we care about customizing, instead of every device on the
> platform_bus (I know Grant will like that part too.  :)

:-D

> I experimented with something similar before I took the easy way out
> with platform_bus_set_pm_ops() :/
> 
> This approach might also solve the problem(s) we've been discussing
> around the conflicts between runtime PM callbacks and system PM
> callbacks (that RTC vs. i2c issue.)  With this approach, I shouldn't
> have to to add the callbacks like I did for the i2c driver, but rather
> handle them in common code.  I'll experiment with this...
> 
> The primary question for me is whether this should rather be at the
> 'struct device' level instead of at the platform_device level.  While
> we're currently only using this customization on platform_devices, I
> don't think we should limit it to that.  Part of these discussions at
> LPC was also about whether or not to move to using custom SoC-specific
> devices and busses.  If/when we go that route, we'd want these power
> domains part of struct device, not platform_device.

Definitely should be at the struct device level IMNSHO.

Re: [RFC][PATCH] Power domains for platform bus type

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

>> Also, what is the use case for having 2 sets of power_domain ops?  My
>> gut tells me that you'd only want to do post ops on the
>> {freeze,suspend,poweroff} path and pre ops on the {resume,thaw,restore}
>> path.  It seems overly engineered to me, but I may be missing
>> something fundamental.
>
> Well, that's a part of the RFC, actually. :-)
>
> For the subsystems I've worked with (PCI, ACPI, PNP to some extent) one set
> would be sufficient, but I don't know of every possible use case.

For the on-chip SoC devices we're managing with OMAP, we're currently
only using one set: post ops on [runtime_]suspend and pre ops on
[runtime_]resume.

However, I could imagine (at least conceptually) using the pre ops on
suspend to do some constraints checking and/or possibly some
management/notification of dependent devices.  Another possiblity
(although possibly racy) would be using the pre ops on suspend to
initiate some high-latency operations.

I guess the main problem with two sets is wasted space.  e.g, if I move
OMAP to this (already hacking on it) there will be only 2 functions used
in post ops: [runtime_]suspend() and 2 used in pre ops [runtime_]_resume().

Kevin

Re: [RFC][PATCH] Power domains for platform bus type

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> Hi,
>
> This is something we discussed during the last Linux Plumbers Conference.
>
> The problem appears to be that the same device may be used in different
> systems in different configurations such that actions necessary for the
> device's power management can vary from one system to another.  In those
> cases the drivers' power management callbacks are generally not sufficient,
> because they can't take the configuration of the whole system into account.
>
> I think this issue may be addressed by adding objects that will represent
> power domains and will provide power management callbacks to be executed
> in addition to the device driver's PM callbacks, which is done by the patch
> below.
>
> Please have a look at it and tell me what you think.
>

FYI... I just tested this patch on OMAP by converting our existing use
of platform_bus_set_ops() to use this approach by adding a powerdomain
to each omap_device.   

Note we're currently only overriding the runtime_[suspend|resume]
methods, so those are the only paths I've tested.

Kevin

Re: [RFC][PATCH] Power domains for platform bus type

[Grant Likely]

On Mon, Jan 31, 2011 at 4:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
>>> Also, what is the use case for having 2 sets of power_domain ops?  My
>>> gut tells me that you'd only want to do post ops on the
>>> {freeze,suspend,poweroff} path and pre ops on the {resume,thaw,restore}
>>> path.  It seems overly engineered to me, but I may be missing
>>> something fundamental.
>>
>> Well, that's a part of the RFC, actually. :-)
>>
>> For the subsystems I've worked with (PCI, ACPI, PNP to some extent) one set
>> would be sufficient, but I don't know of every possible use case.
>
> For the on-chip SoC devices we're managing with OMAP, we're currently
> only using one set: post ops on [runtime_]suspend and pre ops on
> [runtime_]resume.
>
> However, I could imagine (at least conceptually) using the pre ops on
> suspend to do some constraints checking and/or possibly some
> management/notification of dependent devices.  Another possiblity
> (although possibly racy) would be using the pre ops on suspend to
> initiate some high-latency operations.
>
> I guess the main problem with two sets is wasted space.  e.g, if I move
> OMAP to this (already hacking on it) there will be only 2 functions used
> in post ops: [runtime_]suspend() and 2 used in pre ops [runtime_]_resume().

There's a conceptual load added to the (human) reader too.  Every
additional hook point is an additional piece other engineers need to
fit into their mental model.  I'm resistant to having the two sets of
ops without a definite use case for it when conceptually I can only
imagine a need for one set.

g.

Re: [RFC][PATCH] Power domains for platform bus type

[Alan Stern]

On Mon, 31 Jan 2011, Kevin Hilman wrote:

> For the on-chip SoC devices we're managing with OMAP, we're currently
> only using one set: post ops on [runtime_]suspend and pre ops on
> [runtime_]resume.
> 
> However, I could imagine (at least conceptually) using the pre ops on
> suspend to do some constraints checking and/or possibly some
> management/notification of dependent devices.  Another possiblity
> (although possibly racy) would be using the pre ops on suspend to
> initiate some high-latency operations.

Dependency management is very relevant here, since we're talking about
relations that explicitly aren't of the parent-child type.  If any of
the devices in question get marked for async suspend/resume, for
example, they certainly will need dependency handling.

> I guess the main problem with two sets is wasted space.  e.g, if I move
> OMAP to this (already hacking on it) there will be only 2 functions used
> in post ops: [runtime_]suspend() and 2 used in pre ops [runtime_]_resume().

The wasted space is minimal; we're only talking about one extra pm_ops
structure for each power domain.  Presumably any reasonable SoC isn't
going to have a tremendous number of separate power domains.  Or am I
wrong about this?

Alan Stern

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Tuesday, February 01, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > Hi,
> >
> > This is something we discussed during the last Linux Plumbers Conference.
> >
> > The problem appears to be that the same device may be used in different
> > systems in different configurations such that actions necessary for the
> > device's power management can vary from one system to another.  In those
> > cases the drivers' power management callbacks are generally not sufficient,
> > because they can't take the configuration of the whole system into account.
> >
> > I think this issue may be addressed by adding objects that will represent
> > power domains and will provide power management callbacks to be executed
> > in addition to the device driver's PM callbacks, which is done by the patch
> > below.
> >
> > Please have a look at it and tell me what you think.
> >
> 
> FYI... I just tested this patch on OMAP by converting our existing use
> of platform_bus_set_ops() to use this approach by adding a powerdomain
> to each omap_device.   
> 
> Note we're currently only overriding the runtime_[suspend|resume]
> methods, so those are the only paths I've tested.

Thanks a lot for the testing!

I'm going to prepare a new version of the patch that will add power domain
objects at the core level (ie. struct device).

Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Tuesday, February 01, 2011, Grant Likely wrote:
> On Mon, Jan 31, 2011 at 4:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> > "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> >
> >>> Also, what is the use case for having 2 sets of power_domain ops?  My
> >>> gut tells me that you'd only want to do post ops on the
> >>> {freeze,suspend,poweroff} path and pre ops on the {resume,thaw,restore}
> >>> path.  It seems overly engineered to me, but I may be missing
> >>> something fundamental.
> >>
> >> Well, that's a part of the RFC, actually. :-)
> >>
> >> For the subsystems I've worked with (PCI, ACPI, PNP to some extent) one set
> >> would be sufficient, but I don't know of every possible use case.
> >
> > For the on-chip SoC devices we're managing with OMAP, we're currently
> > only using one set: post ops on [runtime_]suspend and pre ops on
> > [runtime_]resume.
> >
> > However, I could imagine (at least conceptually) using the pre ops on
> > suspend to do some constraints checking and/or possibly some
> > management/notification of dependent devices.  Another possiblity
> > (although possibly racy) would be using the pre ops on suspend to
> > initiate some high-latency operations.
> >
> > I guess the main problem with two sets is wasted space.  e.g, if I move
> > OMAP to this (already hacking on it) there will be only 2 functions used
> > in post ops: [runtime_]suspend() and 2 used in pre ops [runtime_]_resume().

I agree with Alan that the wasted space is not substantial.

> There's a conceptual load added to the (human) reader too.  Every
> additional hook point is an additional piece other engineers need to
> fit into their mental model.  I'm resistant to having the two sets of
> ops without a definite use case for it when conceptually I can only
> imagine a need for one set.

There are two possibilities I think.  First, we can use one set for now
and possibly add the other one in the future if it proves necessary.  Second,
we can use two sets to start with and then drop one of them if no one finds it
useful.

I guess the first option is a cleaner one, so I'm going to move forward along
this line.

Also, I'm going to move the power domain hook to the struct device level, so
that we have:

struct device {
    ...
    struct power_domain *domain;
    ...
};

struct power_domain {
    struct dev_pm_ops ops;
};

This way, if there's a need to add the second set of operations, it will be
relatively easy to do that without major redesign.

Thanks,
Rafael

Re: [RFC][PATCH] Power domains for platform bus type

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

[...]

> Also, I'm going to move the power domain hook to the struct device level, so
> that we have:
>
> struct device {
>     ...
>     struct power_domain *domain;
>     ...
> };
>
> struct power_domain {
>     struct dev_pm_ops ops;
> };
>

Great, thanks.

Any opinion on my idea to name this 'struct dev_power_domain' to avoid
confusion with existing powerdomain structs in SoC code?  For example,
on OMAP we already have a 'struct powerdomain'.

Kevin

Re: [RFC][PATCH] Power domains for platform bus type

[Rafael J. Wysocki]

On Tuesday, February 01, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> [...]
> 
> > Also, I'm going to move the power domain hook to the struct device level, so
> > that we have:
> >
> > struct device {
> >     ...
> >     struct power_domain *domain;
> >     ...
> > };
> >
> > struct power_domain {
> >     struct dev_pm_ops ops;
> > };
> >
> 
> Great, thanks.
> 
> Any opinion on my idea to name this 'struct dev_power_domain' to avoid
> confusion with existing powerdomain structs in SoC code?  For example,
> on OMAP we already have a 'struct powerdomain'.

Fine by me.

Thanks,
Rafael

[RFC][PATCH 0/2] PM: Core power management modifications

[Rafael J. Wysocki]

Hi,

The two patches in this series modify the core power management code in the
following way:

[1/2] - Adds support for device power domains.
[2/2] - Reworks the core PM routines so that subsystems are treated in the
        same way by the system-wide transitions code and the runtime PM code.

Comments welcome!

Thanks,
Rafael

[RFC][PATCH 1/2] PM: Add support for device power domains

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The platform bus type is often used to handle Systems-on-a-Chip (SoC)
where all devices are represented by objects of type struct
platform_device.  In those cases the same "platform" device driver
may be used with multiple different system configurations, but the
actions needed to put the devices it handles into a low-power state
and back into the full-power state may depend on the design of the
given SoC.  The driver, however, cannot possibly include all the
information necessary for the power management of its device on all
the systems it is used with.  Moreover, the device hierarchy in its
current form also is not suitable for representing this kind of
information.

The patch below attempts to address this problem by introducing
objects of type struct dev_power_domain that can be used for
representing power domains within a SoC.  Every struct
dev_power_domain object provides a sets of device power
management callbacks that can be used to perform what's needed for
device power management in addition to the operations carried out by
the device's driver and subsystem.

Namely, if a struct dev_power_domain object is pointed to by the
pwr_domain field in a struct device, the callbacks provided by its
ops member will be executed in addition to the corresponding
callbacks provided by the device's subsystem and driver during all
power transitions.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt |   45 +++++++++++++++++++++++++++++++++++++++-
 drivers/base/power/main.c       |   37 ++++++++++++++++++++++++++++++++
 drivers/base/power/runtime.c    |   19 +++++++++++++++-
 include/linux/device.h          |    1 
 include/linux/pm.h              |    8 +++++++
 5 files changed, 107 insertions(+), 3 deletions(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -463,6 +463,14 @@ struct dev_pm_info {
 
 extern void update_pm_runtime_accounting(struct device *dev);
 
+/*
+ * Power domains provide callbacks that are executed during system suspend,
+ * hibernation, system resume and during runtime PM transitions along with
+ * subsystem-level and driver-level callbacks.
+ */
+struct dev_power_domain {
+	struct dev_pm_ops	ops;
+};
 
 /*
  * The PM_EVENT_ messages are also used by drivers implementing the legacy
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -422,6 +422,7 @@ struct device {
 	void		*platform_data;	/* Platform specific data, device
 					   core doesn't touch it */
 	struct dev_pm_info	power;
+	struct dev_power_domain	*pwr_domain;
 
 #ifdef CONFIG_NUMA
 	int		numa_node;	/* NUMA node this device is close to */
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -168,6 +168,7 @@ static int rpm_check_suspend_allowed(str
 static int rpm_idle(struct device *dev, int rpmflags)
 {
 	int (*callback)(struct device *);
+	int (*domain_callback)(struct device *);
 	int retval;
 
 	retval = rpm_check_suspend_allowed(dev);
@@ -222,10 +223,19 @@ static int rpm_idle(struct device *dev,
 	else
 		callback = NULL;
 
-	if (callback) {
+	if (dev->pwr_domain)
+		domain_callback = dev->pwr_domain->ops.runtime_idle;
+	else
+		domain_callback = NULL;
+
+	if (callback || domain_callback) {
 		spin_unlock_irq(&dev->power.lock);
 
-		callback(dev);
+		if (domain_callback)
+			retval = domain_callback(dev);
+
+		if (!retval && callback)
+			callback(dev);
 
 		spin_lock_irq(&dev->power.lock);
 	}
@@ -390,6 +400,8 @@ static int rpm_suspend(struct device *de
 		else
 			pm_runtime_cancel_pending(dev);
 	} else {
+		if (dev->pwr_domain)
+			rpm_callback(dev->pwr_domain->ops.runtime_suspend, dev);
  no_callback:
 		__update_runtime_status(dev, RPM_SUSPENDED);
 		pm_runtime_deactivate_timer(dev);
@@ -569,6 +581,9 @@ static int rpm_resume(struct device *dev
 
 	__update_runtime_status(dev, RPM_RESUMING);
 
+	if (dev->pwr_domain)
+		rpm_callback(dev->pwr_domain->ops.runtime_resume, dev);
+
 	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume)
 		callback = dev->bus->pm->runtime_resume;
 	else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume)
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -423,6 +423,11 @@ static int device_resume_noirq(struct de
 	TRACE_DEVICE(dev);
 	TRACE_RESUME(0);
 
+	if (dev->pwr_domain) {
+		pm_dev_dbg(dev, state, "EARLY power domain ");
+		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
+	}
+
 	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "EARLY ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
@@ -518,6 +523,11 @@ static int device_resume(struct device *
 
 	dev->power.in_suspend = false;
 
+	if (dev->pwr_domain) {
+		pm_dev_dbg(dev, state, "power domain ");
+		pm_op(dev, &dev->pwr_domain->ops, state);
+	}
+
 	if (dev->bus) {
 		if (dev->bus->pm) {
 			pm_dev_dbg(dev, state, "");
@@ -629,6 +639,11 @@ static void device_complete(struct devic
 {
 	device_lock(dev);
 
+	if (dev->pwr_domain && dev->pwr_domain->ops.complete) {
+		pm_dev_dbg(dev, state, "completing power domain ");
+		dev->pwr_domain->ops.complete(dev);
+	}
+
 	if (dev->class && dev->class->pm && dev->class->pm->complete) {
 		pm_dev_dbg(dev, state, "completing class ");
 		dev->class->pm->complete(dev);
@@ -745,6 +760,13 @@ static int device_suspend_noirq(struct d
 	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "LATE ");
 		error = pm_noirq_op(dev, dev->bus->pm, state);
+		if (error)
+			goto End;
+	}
+
+	if (dev->pwr_domain) {
+		pm_dev_dbg(dev, state, "LATE power domain ");
+		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
 	}
 
 End:
@@ -864,6 +886,13 @@ static int __device_suspend(struct devic
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
 		}
+		if (error)
+			goto End;
+	}
+
+	if (dev->pwr_domain) {
+		pm_dev_dbg(dev, state, "power domain ");
+		pm_op(dev, &dev->pwr_domain->ops, state);
 	}
 
  End:
@@ -976,7 +1005,15 @@ static int device_prepare(struct device
 		pm_dev_dbg(dev, state, "preparing class ");
 		error = dev->class->pm->prepare(dev);
 		suspend_report_result(dev->class->pm->prepare, error);
+		if (error)
+			goto End;
+	}
+
+	if (dev->pwr_domain && dev->pwr_domain->ops.prepare) {
+		pm_dev_dbg(dev, state, "preparing power domain ");
+		dev->pwr_domain->ops.prepare(dev);
 	}
+
  End:
 	device_unlock(dev);
 
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -1,6 +1,6 @@
 Device Power Management
 
-Copyright (c) 2010 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+Copyright (c) 2010-2011 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
 Copyright (c) 2010 Alan Stern <stern@rowland.harvard.edu>
 
 
@@ -507,6 +507,49 @@ routines.  Nevertheless, different callb
 situation where it actually matters.
 
 
+Device Power Domains
+--------------------
+Sometimes devices share reference clocks or other power resources.  In those
+cases it generally is not possible to put devices into low-power states
+individually.  Instead, a set of devices sharing a power resource can be put
+into a low-power state together at the same time by turning off the shared
+power resource.  Of course, they also need to be put into the full-power state
+together, by turning the shared power resource on.  A set of devices with this
+property is often referred to as a power domain.
+
+Support for power domains is provided through the pwr_domain field of struct
+device.  This field is a pointer to an object of type struct dev_power_domain,
+defined in include/linux/pm.h, providing a set of power management callbacks
+analogous to the subsystem-level and device driver callbacks that are executed
+for the given device during all power transitions, in addition to the respective
+subsystem-level callbacks.  Specifically, the power domain "suspend" callbacks
+(i.e. ->runtime_suspend(), ->suspend(), ->freeze(), ->poweroff(), etc.) are
+executed after the analogous subsystem-level callbacks, while the power domain
+"resume" callbacks (i.e. ->runtime_resume(), ->resume(), ->thaw(), ->restore,
+etc.) are executed before the analogous subsystem-level callbacks.  Error codes
+returned by the "suspend" and "resume" power domain callbacks are ignored.
+
+Power domain ->runtime_idle() callback is executed before the subsystem-level
+->runtime_idle() callback and the result returned by it is not ignored.  Namely,
+if it returns error code, the subsystem-level ->runtime_idle() callback will not
+be called and the helper function rpm_idle() executing it will return error
+code.  This mechanism is intended to help platforms where saving device state
+is a time consuming operation and should only be carried out if all devices
+in the power domain are idle, before turning off the shared power resource(s).
+Namely, the power domain ->runtime_idle() callback may return error code until
+the pm_runtime_idle() helper (or its asychronous version) has been called for
+all devices in the power domain (it is recommended that the returned error code
+be -EBUSY in those cases), preventing the subsystem-level ->runtime_idle()
+callback from being run prematurely.
+
+The support for device power domains is only relevant to platforms needing to
+use the same subsystem-level (e.g. platform bus type) and device driver power
+management callbacks in many different power domain configurations and wanting
+to avoid incorporating the support for power domains into the subsystem-level
+callbacks.  The other platforms need not implement it or take it into account
+in any way.
+
+
 System Devices
 --------------
 System devices (sysdevs) follow a slightly different API, which can be found in

[RFC][PATCH 2/2] PM: Make system-wide PM and runtime PM handle subsystems consistently

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The code handling system-wide power transitions (eg. suspend-to-RAM)
can in theory execute callbacks provided by the device's bus type,
device type and class in each phase of the power transition.  In
turn, the runtime PM core code only calls one of those callbacks at
a time, preferring bus type callbacks to device type or class
callbacks and device type callbacks to class callbacks.

It seems reasonable to make them both behave in the same way in that
respect.  Moreover, even though a device may belong to two subsystems
(eg. bus type and device class) simultaneously, in practice power
management callbacks for system-wide power transitions are always
provided by only one of them (ie. if the bus type callbacks are
defined, the device class ones are not and vice versa).  Thus it is
possible to modify the code handling system-wide power transitions
so that it follows the core runtime PM code (ie. treats the
subsystem callbacks as mutually exclusive).

On the other hand, the core runtime PM code will choose to execute,
for example, a runtime suspend callback provided by the device type
even if the bus type's struct dev_pm_ops object exists, but the
runtime_suspend pointer in it happens to be NULL.  This is confusing,
because it may lead to the execution of callbacks from different
subsystems during different operations (eg. the bus type suspend
callback may be executed during runtime suspend, while the device
type callback will be executed during runtime resume).

Make all of the power management code treat subsystem callbacks in
a consistent way, such that:
(1) If the device's bus type is defined (eg. dev->bus is not NULL)
    and its pm pointer is not NULL, the callbacks from dev->bus->pm
    will be used.
(2) If dev->bus is NULL or dev->bus->pm is NULL, but the device's
    device type is defined (eg. dev->type is not NULL) and its pm
    pointer is not NULL, the callbacks from dev->type->pm will be
    used.
(3) If dev->bus is NULL or dev->bus->pm is NULL and dev->type is
    NULL or dev->type->pm is NULL, the callbacks from dev->class->pm
    will be used provided that both dev->class and dev->class->pm
    are not NULL.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/devices.txt |   28 +++----
 drivers/base/power/main.c       |  141 +++++++++++++++++-----------------------
 drivers/base/power/runtime.c    |   12 +--
 3 files changed, 79 insertions(+), 102 deletions(-)

Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -433,21 +433,17 @@ static int device_resume_noirq(struct de
 		error = pm_noirq_op(dev, dev->bus->pm, state);
 		if (error)
 			goto End;
-	}
-
-	if (dev->type && dev->type->pm) {
+	} else if (dev->type && dev->type->pm) {
 		pm_dev_dbg(dev, state, "EARLY type ");
 		error = pm_noirq_op(dev, dev->type->pm, state);
 		if (error)
 			goto End;
-	}
-
-	if (dev->class && dev->class->pm) {
+	} else if (dev->class && dev->class->pm) {
 		pm_dev_dbg(dev, state, "EARLY class ");
 		error = pm_noirq_op(dev, dev->class->pm, state);
 	}
 
-End:
+ End:
 	TRACE_RESUME(error);
 	return error;
 }
@@ -532,21 +528,18 @@ static int device_resume(struct device *
 		if (dev->bus->pm) {
 			pm_dev_dbg(dev, state, "");
 			error = pm_op(dev, dev->bus->pm, state);
+			goto End;
 		} else if (dev->bus->resume) {
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_resume(dev, dev->bus->resume);
-		}
-		if (error)
 			goto End;
+		}
 	}
 
-	if (dev->type) {
-		if (dev->type->pm) {
-			pm_dev_dbg(dev, state, "type ");
-			error = pm_op(dev, dev->type->pm, state);
-		}
-		if (error)
-			goto End;
+	if (dev->type && dev->type->pm) {
+		pm_dev_dbg(dev, state, "type ");
+		error = pm_op(dev, dev->type->pm, state);
+		goto End;
 	}
 
 	if (dev->class) {
@@ -558,6 +551,7 @@ static int device_resume(struct device *
 			error = legacy_resume(dev, dev->class->resume);
 		}
 	}
+
  End:
 	device_unlock(dev);
 	complete_all(&dev->power.completion);
@@ -644,19 +638,18 @@ static void device_complete(struct devic
 		dev->pwr_domain->ops.complete(dev);
 	}
 
-	if (dev->class && dev->class->pm && dev->class->pm->complete) {
-		pm_dev_dbg(dev, state, "completing class ");
-		dev->class->pm->complete(dev);
-	}
-
-	if (dev->type && dev->type->pm && dev->type->pm->complete) {
-		pm_dev_dbg(dev, state, "completing type ");
-		dev->type->pm->complete(dev);
-	}
-
-	if (dev->bus && dev->bus->pm && dev->bus->pm->complete) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "completing ");
-		dev->bus->pm->complete(dev);
+		if (dev->bus->pm->complete)
+			dev->bus->pm->complete(dev);
+	} else if (dev->type && dev->type->pm) {
+		pm_dev_dbg(dev, state, "completing type ");
+		if (dev->type->pm->complete)
+			dev->type->pm->complete(dev);
+	} else if (dev->class && dev->class->pm) {
+		pm_dev_dbg(dev, state, "completing class ");
+		if (dev->class->pm->complete)
+			dev->class->pm->complete(dev);
 	}
 
 	device_unlock(dev);
@@ -741,27 +734,23 @@ static pm_message_t resume_event(pm_mess
  */
 static int device_suspend_noirq(struct device *dev, pm_message_t state)
 {
-	int error = 0;
+	int error;
 
-	if (dev->class && dev->class->pm) {
-		pm_dev_dbg(dev, state, "LATE class ");
-		error = pm_noirq_op(dev, dev->class->pm, state);
+	if (dev->bus && dev->bus->pm) {
+		pm_dev_dbg(dev, state, "LATE ");
+		error = pm_noirq_op(dev, dev->bus->pm, state);
 		if (error)
-			goto End;
-	}
-
-	if (dev->type && dev->type->pm) {
+			return error;
+	} else if (dev->type && dev->type->pm) {
 		pm_dev_dbg(dev, state, "LATE type ");
 		error = pm_noirq_op(dev, dev->type->pm, state);
 		if (error)
-			goto End;
-	}
-
-	if (dev->bus && dev->bus->pm) {
-		pm_dev_dbg(dev, state, "LATE ");
-		error = pm_noirq_op(dev, dev->bus->pm, state);
+			return error;
+	} else if (dev->class && dev->class->pm) {
+		pm_dev_dbg(dev, state, "LATE class ");
+		error = pm_noirq_op(dev, dev->class->pm, state);
 		if (error)
-			goto End;
+			return error;
 	}
 
 	if (dev->pwr_domain) {
@@ -769,8 +758,7 @@ static int device_suspend_noirq(struct d
 		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
 	}
 
-End:
-	return error;
+	return 0;
 }
 
 /**
@@ -857,40 +845,36 @@ static int __device_suspend(struct devic
 		goto End;
 	}
 
-	if (dev->class) {
-		if (dev->class->pm) {
-			pm_dev_dbg(dev, state, "class ");
-			error = pm_op(dev, dev->class->pm, state);
-		} else if (dev->class->suspend) {
-			pm_dev_dbg(dev, state, "legacy class ");
-			error = legacy_suspend(dev, state, dev->class->suspend);
-		}
-		if (error)
-			goto End;
-	}
-
-	if (dev->type) {
-		if (dev->type->pm) {
-			pm_dev_dbg(dev, state, "type ");
-			error = pm_op(dev, dev->type->pm, state);
-		}
-		if (error)
-			goto End;
-	}
-
 	if (dev->bus) {
 		if (dev->bus->pm) {
 			pm_dev_dbg(dev, state, "");
 			error = pm_op(dev, dev->bus->pm, state);
+			goto Domain;
 		} else if (dev->bus->suspend) {
 			pm_dev_dbg(dev, state, "legacy ");
 			error = legacy_suspend(dev, state, dev->bus->suspend);
+			goto Domain;
 		}
-		if (error)
-			goto End;
 	}
 
-	if (dev->pwr_domain) {
+	if (dev->type && dev->type->pm) {
+		pm_dev_dbg(dev, state, "type ");
+		error = pm_op(dev, dev->type->pm, state);
+		goto Domain;
+	}
+
+	if (dev->class) {
+		if (dev->class->pm) {
+			pm_dev_dbg(dev, state, "class ");
+			error = pm_op(dev, dev->class->pm, state);
+		} else if (dev->class->suspend) {
+			pm_dev_dbg(dev, state, "legacy class ");
+			error = legacy_suspend(dev, state, dev->class->suspend);
+		}
+	}
+
+ Domain:
+	if (!error && dev->pwr_domain) {
 		pm_dev_dbg(dev, state, "power domain ");
 		pm_op(dev, &dev->pwr_domain->ops, state);
 	}
@@ -985,25 +969,24 @@ static int device_prepare(struct device
 
 	device_lock(dev);
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->prepare) {
+	if (dev->bus && dev->bus->pm) {
 		pm_dev_dbg(dev, state, "preparing ");
-		error = dev->bus->pm->prepare(dev);
+		if (dev->bus->pm->prepare)
+			error = dev->bus->pm->prepare(dev);
 		suspend_report_result(dev->bus->pm->prepare, error);
 		if (error)
 			goto End;
-	}
-
-	if (dev->type && dev->type->pm && dev->type->pm->prepare) {
+	} else if (dev->type && dev->type->pm) {
 		pm_dev_dbg(dev, state, "preparing type ");
-		error = dev->type->pm->prepare(dev);
+		if (dev->type->pm->prepare)
+			error = dev->type->pm->prepare(dev);
 		suspend_report_result(dev->type->pm->prepare, error);
 		if (error)
 			goto End;
-	}
-
-	if (dev->class && dev->class->pm && dev->class->pm->prepare) {
+	} else if (dev->class && dev->class->pm) {
 		pm_dev_dbg(dev, state, "preparing class ");
-		error = dev->class->pm->prepare(dev);
+		if (dev->class->pm->prepare)
+			error = dev->class->pm->prepare(dev);
 		suspend_report_result(dev->class->pm->prepare, error);
 		if (error)
 			goto End;
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -214,9 +214,9 @@ static int rpm_idle(struct device *dev,
 
 	dev->power.idle_notification = true;
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_idle)
+	if (dev->bus && dev->bus->pm)
 		callback = dev->bus->pm->runtime_idle;
-	else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle)
+	else if (dev->type && dev->type->pm)
 		callback = dev->type->pm->runtime_idle;
 	else if (dev->class && dev->class->pm)
 		callback = dev->class->pm->runtime_idle;
@@ -382,9 +382,9 @@ static int rpm_suspend(struct device *de
 
 	__update_runtime_status(dev, RPM_SUSPENDING);
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend)
+	if (dev->bus && dev->bus->pm)
 		callback = dev->bus->pm->runtime_suspend;
-	else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend)
+	else if (dev->type && dev->type->pm)
 		callback = dev->type->pm->runtime_suspend;
 	else if (dev->class && dev->class->pm)
 		callback = dev->class->pm->runtime_suspend;
@@ -584,9 +584,9 @@ static int rpm_resume(struct device *dev
 	if (dev->pwr_domain)
 		rpm_callback(dev->pwr_domain->ops.runtime_resume, dev);
 
-	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume)
+	if (dev->bus && dev->bus->pm)
 		callback = dev->bus->pm->runtime_resume;
-	else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume)
+	else if (dev->type && dev->type->pm)
 		callback = dev->type->pm->runtime_resume;
 	else if (dev->class && dev->class->pm)
 		callback = dev->class->pm->runtime_resume;
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -249,23 +249,17 @@ various phases always run after tasks ha
 unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
 been disabled (except for those marked with the IRQ_WAKEUP flag).
 
-Most phases use bus, type, and class callbacks (that is, methods defined in
-dev->bus->pm, dev->type->pm, and dev->class->pm).  The prepare and complete
-phases are exceptions; they use only bus callbacks.  When multiple callbacks
-are used in a phase, they are invoked in the order: <class, type, bus> during
-power-down transitions and in the opposite order during power-up transitions.
-For example, during the suspend phase the PM core invokes
-
-	dev->class->pm.suspend(dev);
-	dev->type->pm.suspend(dev);
-	dev->bus->pm.suspend(dev);
-
-before moving on to the next device, whereas during the resume phase the core
-invokes
-
-	dev->bus->pm.resume(dev);
-	dev->type->pm.resume(dev);
-	dev->class->pm.resume(dev);
+All phases use bus, type, or class callbacks (that is, methods defined in
+dev->bus->pm, dev->type->pm, or dev->class->pm).  These callbacks are mutually
+exclusive, so if the bus provides a struct dev_pm_ops object pointed to by its
+pm field (i.e. both dev->bus and dev->bus->pm are defined), the callbacks
+included in that object (i.e. dev->bus->pm) will be used.  In turn, if the
+device type provides a struct dev_pm_ops object pointed to by its pm field
+(i.e. both dev->type and dev->type->pm are defined), the PM core will used the
+callbacks from that object (i.e. dev->type->pm).  Finally, if the pm fields of
+both the bus and device type objects are NULL (or those objects do not exist),
+the callbacks provided by the class (that is, the callbacks from dev->class->pm)
+will be used.
 
 These callbacks may in turn invoke device- or driver-specific methods stored in
 dev->driver->pm, but they don't have to.

Re: [RFC][PATCH 1/2] PM: Add support for device power domains

[Alan Stern]

On Sat, 12 Feb 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The platform bus type is often used to handle Systems-on-a-Chip (SoC)
> where all devices are represented by objects of type struct
> platform_device.  In those cases the same "platform" device driver
> may be used with multiple different system configurations, but the
> actions needed to put the devices it handles into a low-power state
> and back into the full-power state may depend on the design of the
> given SoC.  The driver, however, cannot possibly include all the
> information necessary for the power management of its device on all
> the systems it is used with.  Moreover, the device hierarchy in its
> current form also is not suitable for representing this kind of
> information.
> 
> The patch below attempts to address this problem by introducing
> objects of type struct dev_power_domain that can be used for
> representing power domains within a SoC.  Every struct
> dev_power_domain object provides a sets of device power
> management callbacks that can be used to perform what's needed for
> device power management in addition to the operations carried out by
> the device's driver and subsystem.
> 
> Namely, if a struct dev_power_domain object is pointed to by the
> pwr_domain field in a struct device, the callbacks provided by its
> ops member will be executed in addition to the corresponding
> callbacks provided by the device's subsystem and driver during all
> power transitions.

Overall this looks very good.

> Index: linux-2.6/include/linux/pm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pm.h
> +++ linux-2.6/include/linux/pm.h
> @@ -463,6 +463,14 @@ struct dev_pm_info {
>  
>  extern void update_pm_runtime_accounting(struct device *dev);
>  
> +/*
> + * Power domains provide callbacks that are executed during system suspend,
> + * hibernation, system resume and during runtime PM transitions along with
> + * subsystem-level and driver-level callbacks.
> + */
> +struct dev_power_domain {
> +	struct dev_pm_ops	ops;
> +};

I don't have a clear picture of how people are going to want to use 
these dev_power_domain structures.  Should there be a

	void			*priv;

member as well?

Alan Stern

Re: [RFC][PATCH 2/2] PM: Make system-wide PM and runtime PM handle subsystems consistently

[Alan Stern]

On Sat, 12 Feb 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The code handling system-wide power transitions (eg. suspend-to-RAM)
> can in theory execute callbacks provided by the device's bus type,
> device type and class in each phase of the power transition.  In
> turn, the runtime PM core code only calls one of those callbacks at
> a time, preferring bus type callbacks to device type or class
> callbacks and device type callbacks to class callbacks.
> 
> It seems reasonable to make them both behave in the same way in that
> respect.  Moreover, even though a device may belong to two subsystems
> (eg. bus type and device class) simultaneously, in practice power
> management callbacks for system-wide power transitions are always
> provided by only one of them (ie. if the bus type callbacks are
> defined, the device class ones are not and vice versa).  Thus it is
> possible to modify the code handling system-wide power transitions
> so that it follows the core runtime PM code (ie. treats the
> subsystem callbacks as mutually exclusive).
> 
> On the other hand, the core runtime PM code will choose to execute,
> for example, a runtime suspend callback provided by the device type
> even if the bus type's struct dev_pm_ops object exists, but the
> runtime_suspend pointer in it happens to be NULL.  This is confusing,
> because it may lead to the execution of callbacks from different
> subsystems during different operations (eg. the bus type suspend
> callback may be executed during runtime suspend, while the device
> type callback will be executed during runtime resume).
> 
> Make all of the power management code treat subsystem callbacks in
> a consistent way, such that:
> (1) If the device's bus type is defined (eg. dev->bus is not NULL)
>     and its pm pointer is not NULL, the callbacks from dev->bus->pm
>     will be used.
> (2) If dev->bus is NULL or dev->bus->pm is NULL, but the device's
>     device type is defined (eg. dev->type is not NULL) and its pm
>     pointer is not NULL, the callbacks from dev->type->pm will be
>     used.
> (3) If dev->bus is NULL or dev->bus->pm is NULL and dev->type is
>     NULL or dev->type->pm is NULL, the callbacks from dev->class->pm
>     will be used provided that both dev->class and dev->class->pm
>     are not NULL.

It looks okay, but I haven't tested it.  Just one minor change needed 
in the documentation:

> +All phases use bus, type, or class callbacks (that is, methods defined in
> +dev->bus->pm, dev->type->pm, or dev->class->pm).  These callbacks are mutually
> +exclusive, so if the bus provides a struct dev_pm_ops object pointed to by its
> +pm field (i.e. both dev->bus and dev->bus->pm are defined), the callbacks
> +included in that object (i.e. dev->bus->pm) will be used.  In turn, if the

s/In turn/Otherwise/

> +device type provides a struct dev_pm_ops object pointed to by its pm field
> +(i.e. both dev->type and dev->type->pm are defined), the PM core will used the
> +callbacks from that object (i.e. dev->type->pm).  Finally, if the pm fields of
> +both the bus and device type objects are NULL (or those objects do not exist),
> +the callbacks provided by the class (that is, the callbacks from dev->class->pm)
> +will be used.
>  
>  These callbacks may in turn invoke device- or driver-specific methods stored in
>  dev->driver->pm, but they don't have to.

Alan Stern

Re: [RFC][PATCH 2/2] PM: Make system-wide PM and runtime PM handle subsystems consistently

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> The code handling system-wide power transitions (eg. suspend-to-RAM)
> can in theory execute callbacks provided by the device's bus type,
> device type and class in each phase of the power transition.  In
> turn, the runtime PM core code only calls one of those callbacks at
> a time, preferring bus type callbacks to device type or class
> callbacks and device type callbacks to class callbacks.
>
> It seems reasonable to make them both behave in the same way in that
> respect.  Moreover, even though a device may belong to two subsystems
> (eg. bus type and device class) simultaneously, in practice power
> management callbacks for system-wide power transitions are always
> provided by only one of them (ie. if the bus type callbacks are
> defined, the device class ones are not and vice versa).  Thus it is
> possible to modify the code handling system-wide power transitions
> so that it follows the core runtime PM code (ie. treats the
> subsystem callbacks as mutually exclusive).
>
> On the other hand, the core runtime PM code will choose to execute,
> for example, a runtime suspend callback provided by the device type
> even if the bus type's struct dev_pm_ops object exists, but the
> runtime_suspend pointer in it happens to be NULL.  This is confusing,
> because it may lead to the execution of callbacks from different
> subsystems during different operations (eg. the bus type suspend
> callback may be executed during runtime suspend, while the device
> type callback will be executed during runtime resume).
>
> Make all of the power management code treat subsystem callbacks in
> a consistent way, such that:
> (1) If the device's bus type is defined (eg. dev->bus is not NULL)
>     and its pm pointer is not NULL, the callbacks from dev->bus->pm
>     will be used.
> (2) If dev->bus is NULL or dev->bus->pm is NULL, but the device's
>     device type is defined (eg. dev->type is not NULL) and its pm
>     pointer is not NULL, the callbacks from dev->type->pm will be
>     used.
> (3) If dev->bus is NULL or dev->bus->pm is NULL and dev->type is
>     NULL or dev->type->pm is NULL, the callbacks from dev->class->pm
>     will be used provided that both dev->class and dev->class->pm
>     are not NULL.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

This looks good,  consistency between system and runtime PM is a great
help to readability.

Acked-by: Kevin Hilman <khilman@ti.com>


> ---
>  Documentation/power/devices.txt |   28 +++----
>  drivers/base/power/main.c       |  141 +++++++++++++++++-----------------------
>  drivers/base/power/runtime.c    |   12 +--
>  3 files changed, 79 insertions(+), 102 deletions(-)
>
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -433,21 +433,17 @@ static int device_resume_noirq(struct de
>  		error = pm_noirq_op(dev, dev->bus->pm, state);
>  		if (error)
>  			goto End;
> -	}
> -
> -	if (dev->type && dev->type->pm) {
> +	} else if (dev->type && dev->type->pm) {
>  		pm_dev_dbg(dev, state, "EARLY type ");
>  		error = pm_noirq_op(dev, dev->type->pm, state);
>  		if (error)
>  			goto End;
> -	}
> -
> -	if (dev->class && dev->class->pm) {
> +	} else if (dev->class && dev->class->pm) {
>  		pm_dev_dbg(dev, state, "EARLY class ");
>  		error = pm_noirq_op(dev, dev->class->pm, state);
>  	}
>  
> -End:
> + End:
>  	TRACE_RESUME(error);
>  	return error;
>  }
> @@ -532,21 +528,18 @@ static int device_resume(struct device *
>  		if (dev->bus->pm) {
>  			pm_dev_dbg(dev, state, "");
>  			error = pm_op(dev, dev->bus->pm, state);
> +			goto End;
>  		} else if (dev->bus->resume) {
>  			pm_dev_dbg(dev, state, "legacy ");
>  			error = legacy_resume(dev, dev->bus->resume);
> -		}
> -		if (error)
>  			goto End;
> +		}
>  	}
>  
> -	if (dev->type) {
> -		if (dev->type->pm) {
> -			pm_dev_dbg(dev, state, "type ");
> -			error = pm_op(dev, dev->type->pm, state);
> -		}
> -		if (error)
> -			goto End;
> +	if (dev->type && dev->type->pm) {
> +		pm_dev_dbg(dev, state, "type ");
> +		error = pm_op(dev, dev->type->pm, state);
> +		goto End;
>  	}
>  
>  	if (dev->class) {
> @@ -558,6 +551,7 @@ static int device_resume(struct device *
>  			error = legacy_resume(dev, dev->class->resume);
>  		}
>  	}
> +
>   End:
>  	device_unlock(dev);
>  	complete_all(&dev->power.completion);
> @@ -644,19 +638,18 @@ static void device_complete(struct devic
>  		dev->pwr_domain->ops.complete(dev);
>  	}
>  
> -	if (dev->class && dev->class->pm && dev->class->pm->complete) {
> -		pm_dev_dbg(dev, state, "completing class ");
> -		dev->class->pm->complete(dev);
> -	}
> -
> -	if (dev->type && dev->type->pm && dev->type->pm->complete) {
> -		pm_dev_dbg(dev, state, "completing type ");
> -		dev->type->pm->complete(dev);
> -	}
> -
> -	if (dev->bus && dev->bus->pm && dev->bus->pm->complete) {
> +	if (dev->bus && dev->bus->pm) {
>  		pm_dev_dbg(dev, state, "completing ");
> -		dev->bus->pm->complete(dev);
> +		if (dev->bus->pm->complete)
> +			dev->bus->pm->complete(dev);
> +	} else if (dev->type && dev->type->pm) {
> +		pm_dev_dbg(dev, state, "completing type ");
> +		if (dev->type->pm->complete)
> +			dev->type->pm->complete(dev);
> +	} else if (dev->class && dev->class->pm) {
> +		pm_dev_dbg(dev, state, "completing class ");
> +		if (dev->class->pm->complete)
> +			dev->class->pm->complete(dev);
>  	}
>  
>  	device_unlock(dev);
> @@ -741,27 +734,23 @@ static pm_message_t resume_event(pm_mess
>   */
>  static int device_suspend_noirq(struct device *dev, pm_message_t state)
>  {
> -	int error = 0;
> +	int error;
>  
> -	if (dev->class && dev->class->pm) {
> -		pm_dev_dbg(dev, state, "LATE class ");
> -		error = pm_noirq_op(dev, dev->class->pm, state);
> +	if (dev->bus && dev->bus->pm) {
> +		pm_dev_dbg(dev, state, "LATE ");
> +		error = pm_noirq_op(dev, dev->bus->pm, state);
>  		if (error)
> -			goto End;
> -	}
> -
> -	if (dev->type && dev->type->pm) {
> +			return error;
> +	} else if (dev->type && dev->type->pm) {
>  		pm_dev_dbg(dev, state, "LATE type ");
>  		error = pm_noirq_op(dev, dev->type->pm, state);
>  		if (error)
> -			goto End;
> -	}
> -
> -	if (dev->bus && dev->bus->pm) {
> -		pm_dev_dbg(dev, state, "LATE ");
> -		error = pm_noirq_op(dev, dev->bus->pm, state);
> +			return error;
> +	} else if (dev->class && dev->class->pm) {
> +		pm_dev_dbg(dev, state, "LATE class ");
> +		error = pm_noirq_op(dev, dev->class->pm, state);
>  		if (error)
> -			goto End;
> +			return error;
>  	}
>  
>  	if (dev->pwr_domain) {
> @@ -769,8 +758,7 @@ static int device_suspend_noirq(struct d
>  		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
>  	}
>  
> -End:
> -	return error;
> +	return 0;
>  }
>  
>  /**
> @@ -857,40 +845,36 @@ static int __device_suspend(struct devic
>  		goto End;
>  	}
>  
> -	if (dev->class) {
> -		if (dev->class->pm) {
> -			pm_dev_dbg(dev, state, "class ");
> -			error = pm_op(dev, dev->class->pm, state);
> -		} else if (dev->class->suspend) {
> -			pm_dev_dbg(dev, state, "legacy class ");
> -			error = legacy_suspend(dev, state, dev->class->suspend);
> -		}
> -		if (error)
> -			goto End;
> -	}
> -
> -	if (dev->type) {
> -		if (dev->type->pm) {
> -			pm_dev_dbg(dev, state, "type ");
> -			error = pm_op(dev, dev->type->pm, state);
> -		}
> -		if (error)
> -			goto End;
> -	}
> -
>  	if (dev->bus) {
>  		if (dev->bus->pm) {
>  			pm_dev_dbg(dev, state, "");
>  			error = pm_op(dev, dev->bus->pm, state);
> +			goto Domain;
>  		} else if (dev->bus->suspend) {
>  			pm_dev_dbg(dev, state, "legacy ");
>  			error = legacy_suspend(dev, state, dev->bus->suspend);
> +			goto Domain;
>  		}
> -		if (error)
> -			goto End;
>  	}
>  
> -	if (dev->pwr_domain) {
> +	if (dev->type && dev->type->pm) {
> +		pm_dev_dbg(dev, state, "type ");
> +		error = pm_op(dev, dev->type->pm, state);
> +		goto Domain;
> +	}
> +
> +	if (dev->class) {
> +		if (dev->class->pm) {
> +			pm_dev_dbg(dev, state, "class ");
> +			error = pm_op(dev, dev->class->pm, state);
> +		} else if (dev->class->suspend) {
> +			pm_dev_dbg(dev, state, "legacy class ");
> +			error = legacy_suspend(dev, state, dev->class->suspend);
> +		}
> +	}
> +
> + Domain:
> +	if (!error && dev->pwr_domain) {
>  		pm_dev_dbg(dev, state, "power domain ");
>  		pm_op(dev, &dev->pwr_domain->ops, state);
>  	}
> @@ -985,25 +969,24 @@ static int device_prepare(struct device
>  
>  	device_lock(dev);
>  
> -	if (dev->bus && dev->bus->pm && dev->bus->pm->prepare) {
> +	if (dev->bus && dev->bus->pm) {
>  		pm_dev_dbg(dev, state, "preparing ");
> -		error = dev->bus->pm->prepare(dev);
> +		if (dev->bus->pm->prepare)
> +			error = dev->bus->pm->prepare(dev);
>  		suspend_report_result(dev->bus->pm->prepare, error);
>  		if (error)
>  			goto End;
> -	}
> -
> -	if (dev->type && dev->type->pm && dev->type->pm->prepare) {
> +	} else if (dev->type && dev->type->pm) {
>  		pm_dev_dbg(dev, state, "preparing type ");
> -		error = dev->type->pm->prepare(dev);
> +		if (dev->type->pm->prepare)
> +			error = dev->type->pm->prepare(dev);
>  		suspend_report_result(dev->type->pm->prepare, error);
>  		if (error)
>  			goto End;
> -	}
> -
> -	if (dev->class && dev->class->pm && dev->class->pm->prepare) {
> +	} else if (dev->class && dev->class->pm) {
>  		pm_dev_dbg(dev, state, "preparing class ");
> -		error = dev->class->pm->prepare(dev);
> +		if (dev->class->pm->prepare)
> +			error = dev->class->pm->prepare(dev);
>  		suspend_report_result(dev->class->pm->prepare, error);
>  		if (error)
>  			goto End;
> Index: linux-2.6/drivers/base/power/runtime.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/runtime.c
> +++ linux-2.6/drivers/base/power/runtime.c
> @@ -214,9 +214,9 @@ static int rpm_idle(struct device *dev,
>  
>  	dev->power.idle_notification = true;
>  
> -	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_idle)
> +	if (dev->bus && dev->bus->pm)
>  		callback = dev->bus->pm->runtime_idle;
> -	else if (dev->type && dev->type->pm && dev->type->pm->runtime_idle)
> +	else if (dev->type && dev->type->pm)
>  		callback = dev->type->pm->runtime_idle;
>  	else if (dev->class && dev->class->pm)
>  		callback = dev->class->pm->runtime_idle;
> @@ -382,9 +382,9 @@ static int rpm_suspend(struct device *de
>  
>  	__update_runtime_status(dev, RPM_SUSPENDING);
>  
> -	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend)
> +	if (dev->bus && dev->bus->pm)
>  		callback = dev->bus->pm->runtime_suspend;
> -	else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend)
> +	else if (dev->type && dev->type->pm)
>  		callback = dev->type->pm->runtime_suspend;
>  	else if (dev->class && dev->class->pm)
>  		callback = dev->class->pm->runtime_suspend;
> @@ -584,9 +584,9 @@ static int rpm_resume(struct device *dev
>  	if (dev->pwr_domain)
>  		rpm_callback(dev->pwr_domain->ops.runtime_resume, dev);
>  
> -	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume)
> +	if (dev->bus && dev->bus->pm)
>  		callback = dev->bus->pm->runtime_resume;
> -	else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume)
> +	else if (dev->type && dev->type->pm)
>  		callback = dev->type->pm->runtime_resume;
>  	else if (dev->class && dev->class->pm)
>  		callback = dev->class->pm->runtime_resume;
> Index: linux-2.6/Documentation/power/devices.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/devices.txt
> +++ linux-2.6/Documentation/power/devices.txt
> @@ -249,23 +249,17 @@ various phases always run after tasks ha
>  unfrozen.  Furthermore, the *_noirq phases run at a time when IRQ handlers have
>  been disabled (except for those marked with the IRQ_WAKEUP flag).
>  
> -Most phases use bus, type, and class callbacks (that is, methods defined in
> -dev->bus->pm, dev->type->pm, and dev->class->pm).  The prepare and complete
> -phases are exceptions; they use only bus callbacks.  When multiple callbacks
> -are used in a phase, they are invoked in the order: <class, type, bus> during
> -power-down transitions and in the opposite order during power-up transitions.
> -For example, during the suspend phase the PM core invokes
> -
> -	dev->class->pm.suspend(dev);
> -	dev->type->pm.suspend(dev);
> -	dev->bus->pm.suspend(dev);
> -
> -before moving on to the next device, whereas during the resume phase the core
> -invokes
> -
> -	dev->bus->pm.resume(dev);
> -	dev->type->pm.resume(dev);
> -	dev->class->pm.resume(dev);
> +All phases use bus, type, or class callbacks (that is, methods defined in
> +dev->bus->pm, dev->type->pm, or dev->class->pm).  These callbacks are mutually
> +exclusive, so if the bus provides a struct dev_pm_ops object pointed to by its
> +pm field (i.e. both dev->bus and dev->bus->pm are defined), the callbacks
> +included in that object (i.e. dev->bus->pm) will be used.  In turn, if the
> +device type provides a struct dev_pm_ops object pointed to by its pm field
> +(i.e. both dev->type and dev->type->pm are defined), the PM core will used the
> +callbacks from that object (i.e. dev->type->pm).  Finally, if the pm fields of
> +both the bus and device type objects are NULL (or those objects do not exist),
> +the callbacks provided by the class (that is, the callbacks from dev->class->pm)
> +will be used.
>  
>  These callbacks may in turn invoke device- or driver-specific methods stored in
>  dev->driver->pm, but they don't have to.

Re: [RFC][PATCH 1/2] PM: Add support for device power domains

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> The platform bus type is often used to handle Systems-on-a-Chip (SoC)
> where all devices are represented by objects of type struct
> platform_device.  In those cases the same "platform" device driver
> may be used with multiple different system configurations, but the
> actions needed to put the devices it handles into a low-power state
> and back into the full-power state may depend on the design of the
> given SoC.  The driver, however, cannot possibly include all the
> information necessary for the power management of its device on all
> the systems it is used with.  Moreover, the device hierarchy in its
> current form also is not suitable for representing this kind of
> information.
>
> The patch below attempts to address this problem by introducing
> objects of type struct dev_power_domain that can be used for
> representing power domains within a SoC.  Every struct
> dev_power_domain object provides a sets of device power
> management callbacks that can be used to perform what's needed for
> device power management in addition to the operations carried out by
> the device's driver and subsystem.
>
> Namely, if a struct dev_power_domain object is pointed to by the
> pwr_domain field in a struct device, the callbacks provided by its
> ops member will be executed in addition to the corresponding
> callbacks provided by the device's subsystem and driver during all
> power transitions.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Acked-by: Kevin Hilman <khilman@ti.com>

Tested by converting OMAP platform_bus dev_pm_ops overrides to use this
method, and works great.

Tested-by: Kevin Hilman <khilman@ti.com>

> ---
>  Documentation/power/devices.txt |   45 +++++++++++++++++++++++++++++++++++++++-
>  drivers/base/power/main.c       |   37 ++++++++++++++++++++++++++++++++
>  drivers/base/power/runtime.c    |   19 +++++++++++++++-
>  include/linux/device.h          |    1 
>  include/linux/pm.h              |    8 +++++++
>  5 files changed, 107 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/include/linux/pm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pm.h
> +++ linux-2.6/include/linux/pm.h
> @@ -463,6 +463,14 @@ struct dev_pm_info {
>  
>  extern void update_pm_runtime_accounting(struct device *dev);
>  
> +/*
> + * Power domains provide callbacks that are executed during system suspend,
> + * hibernation, system resume and during runtime PM transitions along with
> + * subsystem-level and driver-level callbacks.
> + */
> +struct dev_power_domain {
> +	struct dev_pm_ops	ops;
> +};
>  
>  /*
>   * The PM_EVENT_ messages are also used by drivers implementing the legacy
> Index: linux-2.6/include/linux/device.h
> ===================================================================
> --- linux-2.6.orig/include/linux/device.h
> +++ linux-2.6/include/linux/device.h
> @@ -422,6 +422,7 @@ struct device {
>  	void		*platform_data;	/* Platform specific data, device
>  					   core doesn't touch it */
>  	struct dev_pm_info	power;
> +	struct dev_power_domain	*pwr_domain;
>  
>  #ifdef CONFIG_NUMA
>  	int		numa_node;	/* NUMA node this device is close to */
> Index: linux-2.6/drivers/base/power/runtime.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/runtime.c
> +++ linux-2.6/drivers/base/power/runtime.c
> @@ -168,6 +168,7 @@ static int rpm_check_suspend_allowed(str
>  static int rpm_idle(struct device *dev, int rpmflags)
>  {
>  	int (*callback)(struct device *);
> +	int (*domain_callback)(struct device *);
>  	int retval;
>  
>  	retval = rpm_check_suspend_allowed(dev);
> @@ -222,10 +223,19 @@ static int rpm_idle(struct device *dev,
>  	else
>  		callback = NULL;
>  
> -	if (callback) {
> +	if (dev->pwr_domain)
> +		domain_callback = dev->pwr_domain->ops.runtime_idle;
> +	else
> +		domain_callback = NULL;
> +
> +	if (callback || domain_callback) {
>  		spin_unlock_irq(&dev->power.lock);
>  
> -		callback(dev);
> +		if (domain_callback)
> +			retval = domain_callback(dev);
> +
> +		if (!retval && callback)
> +			callback(dev);
>  
>  		spin_lock_irq(&dev->power.lock);
>  	}
> @@ -390,6 +400,8 @@ static int rpm_suspend(struct device *de
>  		else
>  			pm_runtime_cancel_pending(dev);
>  	} else {
> +		if (dev->pwr_domain)
> +			rpm_callback(dev->pwr_domain->ops.runtime_suspend, dev);
>   no_callback:
>  		__update_runtime_status(dev, RPM_SUSPENDED);
>  		pm_runtime_deactivate_timer(dev);
> @@ -569,6 +581,9 @@ static int rpm_resume(struct device *dev
>  
>  	__update_runtime_status(dev, RPM_RESUMING);
>  
> +	if (dev->pwr_domain)
> +		rpm_callback(dev->pwr_domain->ops.runtime_resume, dev);
> +
>  	if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_resume)
>  		callback = dev->bus->pm->runtime_resume;
>  	else if (dev->type && dev->type->pm && dev->type->pm->runtime_resume)
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -423,6 +423,11 @@ static int device_resume_noirq(struct de
>  	TRACE_DEVICE(dev);
>  	TRACE_RESUME(0);
>  
> +	if (dev->pwr_domain) {
> +		pm_dev_dbg(dev, state, "EARLY power domain ");
> +		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
> +	}
> +
>  	if (dev->bus && dev->bus->pm) {
>  		pm_dev_dbg(dev, state, "EARLY ");
>  		error = pm_noirq_op(dev, dev->bus->pm, state);
> @@ -518,6 +523,11 @@ static int device_resume(struct device *
>  
>  	dev->power.in_suspend = false;
>  
> +	if (dev->pwr_domain) {
> +		pm_dev_dbg(dev, state, "power domain ");
> +		pm_op(dev, &dev->pwr_domain->ops, state);
> +	}
> +
>  	if (dev->bus) {
>  		if (dev->bus->pm) {
>  			pm_dev_dbg(dev, state, "");
> @@ -629,6 +639,11 @@ static void device_complete(struct devic
>  {
>  	device_lock(dev);
>  
> +	if (dev->pwr_domain && dev->pwr_domain->ops.complete) {
> +		pm_dev_dbg(dev, state, "completing power domain ");
> +		dev->pwr_domain->ops.complete(dev);
> +	}
> +
>  	if (dev->class && dev->class->pm && dev->class->pm->complete) {
>  		pm_dev_dbg(dev, state, "completing class ");
>  		dev->class->pm->complete(dev);
> @@ -745,6 +760,13 @@ static int device_suspend_noirq(struct d
>  	if (dev->bus && dev->bus->pm) {
>  		pm_dev_dbg(dev, state, "LATE ");
>  		error = pm_noirq_op(dev, dev->bus->pm, state);
> +		if (error)
> +			goto End;
> +	}
> +
> +	if (dev->pwr_domain) {
> +		pm_dev_dbg(dev, state, "LATE power domain ");
> +		pm_noirq_op(dev, &dev->pwr_domain->ops, state);
>  	}
>  
>  End:
> @@ -864,6 +886,13 @@ static int __device_suspend(struct devic
>  			pm_dev_dbg(dev, state, "legacy ");
>  			error = legacy_suspend(dev, state, dev->bus->suspend);
>  		}
> +		if (error)
> +			goto End;
> +	}
> +
> +	if (dev->pwr_domain) {
> +		pm_dev_dbg(dev, state, "power domain ");
> +		pm_op(dev, &dev->pwr_domain->ops, state);
>  	}
>  
>   End:
> @@ -976,7 +1005,15 @@ static int device_prepare(struct device
>  		pm_dev_dbg(dev, state, "preparing class ");
>  		error = dev->class->pm->prepare(dev);
>  		suspend_report_result(dev->class->pm->prepare, error);
> +		if (error)
> +			goto End;
> +	}
> +
> +	if (dev->pwr_domain && dev->pwr_domain->ops.prepare) {
> +		pm_dev_dbg(dev, state, "preparing power domain ");
> +		dev->pwr_domain->ops.prepare(dev);
>  	}
> +
>   End:
>  	device_unlock(dev);
>  
> Index: linux-2.6/Documentation/power/devices.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/power/devices.txt
> +++ linux-2.6/Documentation/power/devices.txt
> @@ -1,6 +1,6 @@
>  Device Power Management
>  
> -Copyright (c) 2010 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
> +Copyright (c) 2010-2011 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
>  Copyright (c) 2010 Alan Stern <stern@rowland.harvard.edu>
>  
>  
> @@ -507,6 +507,49 @@ routines.  Nevertheless, different callb
>  situation where it actually matters.
>  
>  
> +Device Power Domains
> +--------------------
> +Sometimes devices share reference clocks or other power resources.  In those
> +cases it generally is not possible to put devices into low-power states
> +individually.  Instead, a set of devices sharing a power resource can be put
> +into a low-power state together at the same time by turning off the shared
> +power resource.  Of course, they also need to be put into the full-power state
> +together, by turning the shared power resource on.  A set of devices with this
> +property is often referred to as a power domain.
> +
> +Support for power domains is provided through the pwr_domain field of struct
> +device.  This field is a pointer to an object of type struct dev_power_domain,
> +defined in include/linux/pm.h, providing a set of power management callbacks
> +analogous to the subsystem-level and device driver callbacks that are executed
> +for the given device during all power transitions, in addition to the respective
> +subsystem-level callbacks.  Specifically, the power domain "suspend" callbacks
> +(i.e. ->runtime_suspend(), ->suspend(), ->freeze(), ->poweroff(), etc.) are
> +executed after the analogous subsystem-level callbacks, while the power domain
> +"resume" callbacks (i.e. ->runtime_resume(), ->resume(), ->thaw(), ->restore,
> +etc.) are executed before the analogous subsystem-level callbacks.  Error codes
> +returned by the "suspend" and "resume" power domain callbacks are ignored.
> +
> +Power domain ->runtime_idle() callback is executed before the subsystem-level
> +->runtime_idle() callback and the result returned by it is not ignored.  Namely,
> +if it returns error code, the subsystem-level ->runtime_idle() callback will not
> +be called and the helper function rpm_idle() executing it will return error
> +code.  This mechanism is intended to help platforms where saving device state
> +is a time consuming operation and should only be carried out if all devices
> +in the power domain are idle, before turning off the shared power resource(s).
> +Namely, the power domain ->runtime_idle() callback may return error code until
> +the pm_runtime_idle() helper (or its asychronous version) has been called for
> +all devices in the power domain (it is recommended that the returned error code
> +be -EBUSY in those cases), preventing the subsystem-level ->runtime_idle()
> +callback from being run prematurely.
> +
> +The support for device power domains is only relevant to platforms needing to
> +use the same subsystem-level (e.g. platform bus type) and device driver power
> +management callbacks in many different power domain configurations and wanting
> +to avoid incorporating the support for power domains into the subsystem-level
> +callbacks.  The other platforms need not implement it or take it into account
> +in any way.
> +
> +
>  System Devices
>  --------------
>  System devices (sysdevs) follow a slightly different API, which can be found in

Re: [RFC][PATCH 2/2] PM: Make system-wide PM and runtime PM handle subsystems consistently

[Grant Likely]

On Tue, Feb 15, 2011 at 10:10:06AM -0800, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > The code handling system-wide power transitions (eg. suspend-to-RAM)
> > can in theory execute callbacks provided by the device's bus type,
> > device type and class in each phase of the power transition.  In
> > turn, the runtime PM core code only calls one of those callbacks at
> > a time, preferring bus type callbacks to device type or class
> > callbacks and device type callbacks to class callbacks.
> >
> > It seems reasonable to make them both behave in the same way in that
> > respect.  Moreover, even though a device may belong to two subsystems
> > (eg. bus type and device class) simultaneously, in practice power
> > management callbacks for system-wide power transitions are always
> > provided by only one of them (ie. if the bus type callbacks are
> > defined, the device class ones are not and vice versa).  Thus it is
> > possible to modify the code handling system-wide power transitions
> > so that it follows the core runtime PM code (ie. treats the
> > subsystem callbacks as mutually exclusive).
> >
> > On the other hand, the core runtime PM code will choose to execute,
> > for example, a runtime suspend callback provided by the device type
> > even if the bus type's struct dev_pm_ops object exists, but the
> > runtime_suspend pointer in it happens to be NULL.  This is confusing,
> > because it may lead to the execution of callbacks from different
> > subsystems during different operations (eg. the bus type suspend
> > callback may be executed during runtime suspend, while the device
> > type callback will be executed during runtime resume).
> >
> > Make all of the power management code treat subsystem callbacks in
> > a consistent way, such that:
> > (1) If the device's bus type is defined (eg. dev->bus is not NULL)
> >     and its pm pointer is not NULL, the callbacks from dev->bus->pm
> >     will be used.
> > (2) If dev->bus is NULL or dev->bus->pm is NULL, but the device's
> >     device type is defined (eg. dev->type is not NULL) and its pm
> >     pointer is not NULL, the callbacks from dev->type->pm will be
> >     used.
> > (3) If dev->bus is NULL or dev->bus->pm is NULL and dev->type is
> >     NULL or dev->type->pm is NULL, the callbacks from dev->class->pm
> >     will be used provided that both dev->class and dev->class->pm
> >     are not NULL.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> This looks good,  consistency between system and runtime PM is a great
> help to readability.
> 
> Acked-by: Kevin Hilman <khilman@ti.com>

Reasoning-sounds-sane-to: Grant Likely <grant.likely@secretlab.ca>