Re: [PATCH] i2c: Hook up runtime PM support

2010-02-15 Thread Jean Delvare
Hi Mark,

On Fri,  5 Feb 2010 12:30:11 +, Mark Brown wrote:
 Allow I2C drivers to make use of the runtime PM framework by adding
 bus implementations of the runtime PM operations. These simply
 immediately suspend when the device is idle. The runtime PM framework
 provides drivers with off the shelf refcounts for enables and sysfs
 control for managing runtime suspend from userspace so is useful even
 without meaningful input from the bus.
 
 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
 ---
 
 Changed to allow drivers to use the idle callback to veto suspend.
 
  drivers/i2c/i2c-core.c |   50 
 
  1 files changed, 50 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index 10be7b5..4131698 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -34,6 +34,7 @@
  #include linux/hardirq.h
  #include linux/irqflags.h
  #include linux/rwsem.h
 +#include linux/pm_runtime.h
  #include asm/uaccess.h
  
  #include i2c-core.h
 @@ -184,6 +185,52 @@ static int i2c_device_pm_resume(struct device *dev)
  #define i2c_device_pm_resume NULL
  #endif
  
 +#ifdef CONFIG_PM_RUNTIME
 +static int i2c_device_runtime_suspend(struct device *dev)
 +{
 + const struct dev_pm_ops *pm;
 +
 + if (!dev-driver)
 + return 0;
 + pm = dev-driver-pm;
 + if (!pm || !pm-runtime_suspend)
 + return 0;
 + return pm-runtime_suspend(dev);
 +}
 +
 +static int i2c_device_runtime_resume(struct device *dev)
 +{
 + const struct dev_pm_ops *pm;
 +
 + if (!dev-driver)
 + return 0;
 + pm = dev-driver-pm;
 + if (!pm || !pm-runtime_resume)
 + return 0;
 + return pm-runtime_resume(dev);
 +}
 +
 +static int i2c_device_runtime_idle(struct device *dev)
 +{
 + const struct dev_pm_ops *pm = NULL;
 + int ret;
 +
 + if (dev-driver)
 + pm = dev-driver-pm;
 + if (pm  pm-runtime_idle) {
 + ret = pm-runtime_idle(dev);
 + if (ret)
 + return ret;
 + }
 +
 + return pm_runtime_suspend(dev);
 +}
 +#else
 +#define i2c_device_runtime_suspend   NULL
 +#define i2c_device_runtime_resumeNULL
 +#define i2c_device_runtime_idle  NULL
 +#endif
 +
  static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
  {
   struct i2c_client *client = i2c_verify_client(dev);
 @@ -251,6 +298,9 @@ static const struct attribute_group 
 *i2c_dev_attr_groups[] = {
  static const struct dev_pm_ops i2c_device_pm_ops = {
   .suspend = i2c_device_pm_suspend,
   .resume = i2c_device_pm_resume,
 + .runtime_suspend = i2c_device_runtime_suspend,
 + .runtime_resume = i2c_device_runtime_resume,
 + .runtime_idle = i2c_device_runtime_idle,
  };
  
  struct bus_type i2c_bus_type = {

Applied, thanks. I am a little surprised to see that these functions
have nothing i2c-specific, so I am wondering why we have to duplicate
them in every bus type... Shouldn't the functions above be part of
drivers/base/power/runtime.c and exported so that all bus types that
want them can reuse them?

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: Hook up runtime PM support

2010-02-15 Thread Rafael J. Wysocki
On Monday 15 February 2010, Mark Brown wrote:
 On Mon, Feb 15, 2010 at 07:14:09PM +0100, Jean Delvare wrote:
 
  Applied, thanks. I am a little surprised to see that these functions
  have nothing i2c-specific, so I am wondering why we have to duplicate
  them in every bus type... Shouldn't the functions above be part of
  drivers/base/power/runtime.c and exported so that all bus types that
  want them can reuse them?

In fact this is the first bus type that doesn't need anything specific in these
routines so fat.

 I tend to agree - in fact I'd been a little surprised the default for
 buses that don't provide anything was to refuse to do runtime PM (though
 I can see the transition issues when a bus does want to go and do its
 own thing).
 
 My actual plan here was to implement this for a couple of buses and then
 present a patch factoring out the common code.

That's a good approach IMO.

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: Hook up runtime PM support

2010-02-15 Thread Mark Brown
On Mon, Feb 15, 2010 at 08:08:24PM +0100, Rafael J. Wysocki wrote:
 On Monday 15 February 2010, Mark Brown wrote:
  On Mon, Feb 15, 2010 at 07:14:09PM +0100, Jean Delvare wrote:

   Applied, thanks. I am a little surprised to see that these functions
   have nothing i2c-specific, so I am wondering why we have to duplicate
   them in every bus type... Shouldn't the functions above be part of
   drivers/base/power/runtime.c and exported so that all bus types that
   want them can reuse them?

 In fact this is the first bus type that doesn't need anything specific in 
 these
 routines so fat.

Not really - the platform bus is the same, it's just that some platforms
are also able to do some neat stuff with the information they glean via
runtime PM.  The platform bus is actually a bit of a problem here since
it gets used both for on-SoC devices where that sort of stuff is
possible and also for things like MFDs and random memory mapped things
on the system, meaning that drivers can't take advantage of runtime PM
unless someone goes through and does something per-SoC which is a bit
inconvnient.

There's other buses like SPI that are in the same boat as I2C - they
don't really have a cohesive bus level power management structure, or
the power management is fully handled by with noop methods on the bus
and holding the parent open until all the children are idle.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-pm] [PATCH] i2c: Hook up runtime PM support

2010-02-05 Thread Rafael J. Wysocki
On Friday 05 February 2010, you wrote:
 On Thu, Feb 04, 2010 at 10:57:27PM +0100, Rafael J. Wysocki wrote:
  On Wednesday 03 February 2010, Mark Brown wrote:
   Allow I2C drivers to make use of the runtime PM framework by adding
   bus implementations of the runtime PM operations. These simply
   immediately suspend when the device is idle.
 
  Perhaps it would be a good idea to give the driver a chance to veto
  the suspend by calling its _idle callback?  We do that for PCI and turns 
  out to
  be quite useful.
 
 Hrm, I guess.  It seems an odd thing to want to use - for a bus like I2C
 there's nothing other than the driver involved so the request that is
 being vetoed will have been initiated by the driver which seems wrong.

Not really.  _idle is also called automatically by the runtime PM core after
_resume in case the device may be suspended again.  Your _idle has to handle
this case as well and that's when the driver may want to veto the suspend.

 Out of interest do you have any pointers to drivers using this (my greps
 aren't turning anything up in -next)?

There aren't any in -next, because I'm waiting for the base PCI runtime PM
code to settle down.  I have two in my private queue at the moment , for r8169
and e1000e (I posted some older versions of them some time ago, but they
wouldn't fit the current framework too well, which is the basic reason I'm
sitting on them, so that I don't have to post too many updates :-)).

I can send them to you privately, if needed.

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: Hook up runtime PM support

2010-02-04 Thread Rafael J. Wysocki
On Wednesday 03 February 2010, Mark Brown wrote:
 Allow I2C drivers to make use of the runtime PM framework by adding
 bus implementations of the runtime PM operations. These simply
 immediately suspend when the device is idle.

Perhaps it would be a good idea to give the driver a chance to veto
the suspend by calling its _idle callback?  We do that for PCI and turns out to
be quite useful.

 The runtime PM framework provides drivers with off the shelf refcounts for
 enables and sysfs control for managing runtime suspend from userspace so is
 useful even without meaningful input from the bus.
 
 Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
 ---
  drivers/i2c/i2c-core.c |   41 +
  1 files changed, 41 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 index 10be7b5..985eaac 100644
 --- a/drivers/i2c/i2c-core.c
 +++ b/drivers/i2c/i2c-core.c
 @@ -34,6 +34,7 @@
  #include linux/hardirq.h
  #include linux/irqflags.h
  #include linux/rwsem.h
 +#include linux/pm_runtime.h
  #include asm/uaccess.h
  
  #include i2c-core.h
 @@ -184,6 +185,43 @@ static int i2c_device_pm_resume(struct device *dev)
  #define i2c_device_pm_resume NULL
  #endif
  
 +#ifdef CONFIG_PM_RUNTIME
 +static int i2c_device_runtime_suspend(struct device *dev)
 +{
 + const struct dev_pm_ops *pm;
 +
 + if (!dev-driver)
 + return 0;
 + pm = dev-driver-pm;
 + if (!pm || !pm-runtime_suspend)
 + return 0;
 + return pm-runtime_suspend(dev);
 +}
 +
 +static int i2c_device_runtime_resume(struct device *dev)
 +{
 + const struct dev_pm_ops *pm;
 +
 + if (!dev-driver)
 + return 0;
 + pm = dev-driver-pm;
 + if (!pm || !pm-runtime_resume)
 + return 0;
 + return pm-runtime_resume(dev);
 +}
 +
 +static int i2c_device_runtime_idle(struct device *dev)
 +{
 + /* I2C devices are very independent of each other so we
 +  * suspend them immediately. */
 + return pm_runtime_suspend(dev);
 +}
 +#else
 +#define i2c_device_runtime_suspend   NULL
 +#define i2c_device_runtime_resumeNULL
 +#define i2c_device_runtime_idle  NULL
 +#endif
 +
  static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
  {
   struct i2c_client *client = i2c_verify_client(dev);
 @@ -251,6 +289,9 @@ static const struct attribute_group 
 *i2c_dev_attr_groups[] = {
  static const struct dev_pm_ops i2c_device_pm_ops = {
   .suspend = i2c_device_pm_suspend,
   .resume = i2c_device_pm_resume,
 + .runtime_suspend = i2c_device_runtime_suspend,
 + .runtime_resume = i2c_device_runtime_resume,
 + .runtime_idle = i2c_device_runtime_idle,
  };
  
  struct bus_type i2c_bus_type = {
 

Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] i2c: Hook up runtime PM support

2010-02-03 Thread Mark Brown
Allow I2C drivers to make use of the runtime PM framework by adding
bus implementations of the runtime PM operations. These simply
immediately suspend when the device is idle. The runtime PM framework
provides drivers with off the shelf refcounts for enables and sysfs
control for managing runtime suspend from userspace so is useful even
without meaningful input from the bus.

Signed-off-by: Mark Brown broo...@opensource.wolfsonmicro.com
---
 drivers/i2c/i2c-core.c |   41 +
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 10be7b5..985eaac 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -34,6 +34,7 @@
 #include linux/hardirq.h
 #include linux/irqflags.h
 #include linux/rwsem.h
+#include linux/pm_runtime.h
 #include asm/uaccess.h
 
 #include i2c-core.h
@@ -184,6 +185,43 @@ static int i2c_device_pm_resume(struct device *dev)
 #define i2c_device_pm_resume   NULL
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
+static int i2c_device_runtime_suspend(struct device *dev)
+{
+   const struct dev_pm_ops *pm;
+
+   if (!dev-driver)
+   return 0;
+   pm = dev-driver-pm;
+   if (!pm || !pm-runtime_suspend)
+   return 0;
+   return pm-runtime_suspend(dev);
+}
+
+static int i2c_device_runtime_resume(struct device *dev)
+{
+   const struct dev_pm_ops *pm;
+
+   if (!dev-driver)
+   return 0;
+   pm = dev-driver-pm;
+   if (!pm || !pm-runtime_resume)
+   return 0;
+   return pm-runtime_resume(dev);
+}
+
+static int i2c_device_runtime_idle(struct device *dev)
+{
+   /* I2C devices are very independent of each other so we
+* suspend them immediately. */
+   return pm_runtime_suspend(dev);
+}
+#else
+#define i2c_device_runtime_suspend NULL
+#define i2c_device_runtime_resume  NULL
+#define i2c_device_runtime_idleNULL
+#endif
+
 static int i2c_device_suspend(struct device *dev, pm_message_t mesg)
 {
struct i2c_client *client = i2c_verify_client(dev);
@@ -251,6 +289,9 @@ static const struct attribute_group *i2c_dev_attr_groups[] 
= {
 static const struct dev_pm_ops i2c_device_pm_ops = {
.suspend = i2c_device_pm_suspend,
.resume = i2c_device_pm_resume,
+   .runtime_suspend = i2c_device_runtime_suspend,
+   .runtime_resume = i2c_device_runtime_resume,
+   .runtime_idle = i2c_device_runtime_idle,
 };
 
 struct bus_type i2c_bus_type = {
-- 
1.6.6

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html