Hi there,

rtpm_status_show() is still there in the code. You found that I removed the rtpm_status_show() in the patch because that after back port, there are two rtpm_status_show() in the code. So I removed one of them to resolve the conflict.
Please look at patched source code.

Yong

On Wed, Sep 15, 2010 at 6:33 AM, Leann Ogasawara <leann.ogasawara@canonical.com> wrote:
On Tue, 2010-09-14 at 10:23 -0600, Tim Gardner wrote:
> On 09/13/2010 12:31 AM, yong.shen@linaro.org wrote:
> > From: Arjan van de Ven<arjan@linux.intel.com>
> >
> > In order for PowerTOP to be able to report how well the new runtime PM is
> > working for the various drivers, the kernel needs to export some basic
> > statistics in sysfs.
> >
> > This patch adds two sysfs files in the runtime PM domain that expose the
> > total time a device has been active, and the time a device has been
> > suspended.
> >
> > With this PowerTOP can compute the activity percentage
> >
> > Active %age = 100 * (delta active) / (delta active + delta suspended)
> >
> > and present the information to the user.
> >
> > I've written the PowerTOP code (slated for version 1.12) already, and the
> > output looks like this:
> >
> > Runtime Device Power Management statistics
> > Active  Device name
> >   10.0%     06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8101E/RTL8102E PCI Express Fast Ethernet controller
> >
> > [version 2: fix stat update bugs noticed by Alan Stern]
> > [version 3: rebase to -next and move the sysfs declaration]
> >
> > Signed-off-by: Arjan van de Ven<arjan@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki<rjw@sisk.pl>
> > Signed-off-by: Shen Yong<yong.shen@linaro.org>
> > ---
> >   drivers/base/power/runtime.c |   54 ++++++++++++++++++++++++----
> >   drivers/base/power/sysfs.c   |   83 ++++++++++++++++++++++++++++++++----------
> >   include/linux/pm.h           |    6 +++
> >   3 files changed, 116 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index b0ec0e9..b78c401 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -123,6 +123,45 @@ int pm_runtime_idle(struct device *dev)
> >   }
> >   EXPORT_SYMBOL_GPL(pm_runtime_idle);
> >
> > +
> > +/**
> > + * update_pm_runtime_accounting - Update the time accounting of power states
> > + * @dev: Device to update the accounting for
> > + *
> > + * In order to be able to have time accounting of the various power states
> > + * (as used by programs such as PowerTOP to show the effectiveness of runtime
> > + * PM), we need to track the time spent in each state.
> > + * update_pm_runtime_accounting must be called each time before the
> > + * runtime_status field is updated, to account the time in the old state
> > + * correctly.
> > + */
> > +void update_pm_runtime_accounting(struct device *dev)
> > +{
> > +   unsigned long now = jiffies;
> > +   int delta;
> > +
> > +   delta = now - dev->power.accounting_timestamp;
> > +
> > +   if (delta<  0)
> > +           delta = 0;
> > +
> > +   dev->power.accounting_timestamp = now;
> > +
> > +   if (dev->power.disable_depth>  0)
> > +           return;
> > +
> > +   if (dev->power.runtime_status == RPM_SUSPENDED)
> > +           dev->power.suspended_jiffies += delta;
> > +   else
> > +           dev->power.active_jiffies += delta;
> > +}
> > +
> > +static void __update_runtime_status(struct device *dev, enum rpm_status status)
> > +{
> > +   update_pm_runtime_accounting(dev);
> > +   dev->power.runtime_status = status;
> > +}
> > +
> >   /**
> >    * __pm_runtime_suspend - Carry out run-time suspend of given device.
> >    * @dev: Device to suspend.
> > @@ -197,7 +236,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq)
> >             goto repeat;
> >     }
> >
> > -   dev->power.runtime_status = RPM_SUSPENDING;
> > +   __update_runtime_status(dev, RPM_SUSPENDING);
> >     dev->power.deferred_resume = false;
> >
> >     if (dev->bus&&  dev->bus->pm&&  dev->bus->pm->runtime_suspend) {
> > @@ -228,7 +267,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq)
> >     }
> >
> >     if (retval) {
> > -           dev->power.runtime_status = RPM_ACTIVE;
> > +           __update_runtime_status(dev, RPM_ACTIVE);
> >             if (retval == -EAGAIN || retval == -EBUSY) {
> >                     if (dev->power.timer_expires == 0)
> >                             notify = true;
> > @@ -237,7 +276,7 @@ int __pm_runtime_suspend(struct device *dev, bool from_wq)
> >                     pm_runtime_cancel_pending(dev);
> >             }
> >     } else {
> > -           dev->power.runtime_status = RPM_SUSPENDED;
> > +           __update_runtime_status(dev, RPM_SUSPENDED);
> >             pm_runtime_deactivate_timer(dev);
> >
> >             if (dev->parent) {
> > @@ -381,7 +420,7 @@ int __pm_runtime_resume(struct device *dev, bool from_wq)
> >             goto repeat;
> >     }
> >
> > -   dev->power.runtime_status = RPM_RESUMING;
> > +   __update_runtime_status(dev, RPM_RESUMING);
> >
> >     if (dev->bus&&  dev->bus->pm&&  dev->bus->pm->runtime_resume) {
> >             spin_unlock_irq(&dev->power.lock);
> > @@ -411,10 +450,10 @@ int __pm_runtime_resume(struct device *dev, bool from_wq)
> >     }
> >
> >     if (retval) {
> > -           dev->power.runtime_status = RPM_SUSPENDED;
> > +           __update_runtime_status(dev, RPM_SUSPENDED);
> >             pm_runtime_cancel_pending(dev);
> >     } else {
> > -           dev->power.runtime_status = RPM_ACTIVE;
> > +           __update_runtime_status(dev, RPM_ACTIVE);
> >             if (parent)
> >                     atomic_inc(&parent->power.child_count);
> >     }
> > @@ -848,7 +887,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
> >     }
> >
> >    out_set:
> > -   dev->power.runtime_status = status;
> > +   __update_runtime_status(dev, status);
> >     dev->power.runtime_error = 0;
> >    out:
> >     spin_unlock_irqrestore(&dev->power.lock, flags);
> > @@ -1077,6 +1116,7 @@ void pm_runtime_init(struct device *dev)
> >     dev->power.request_pending = false;
> >     dev->power.request = RPM_REQ_NONE;
> >     dev->power.deferred_resume = false;
> > +   dev->power.accounting_timestamp = jiffies;
> >     INIT_WORK(&dev->power.work, pm_runtime_work);
> >
> >     dev->power.timer_expires = 0;
> > diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> > index a4c33bc..b831ec5 100644
> > --- a/drivers/base/power/sysfs.c
> > +++ b/drivers/base/power/sysfs.c
> > @@ -6,6 +6,7 @@
> >   #include<linux/string.h>
> >   #include<linux/pm_runtime.h>
> >   #include<asm/atomic.h>
> > +#include<linux/jiffies.h>
> >   #include "power.h"
> >
> >   /*
> > @@ -108,6 +109,65 @@ static ssize_t control_store(struct device * dev, struct device_attribute *attr,
> >   }
> >
> >   static DEVICE_ATTR(control, 0644, control_show, control_store);
> > +
> > +static ssize_t rtpm_active_time_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +   int ret;
> > +   spin_lock_irq(&dev->power.lock);
> > +   update_pm_runtime_accounting(dev);
> > +   ret = sprintf(buf, "%i\n", jiffies_to_msecs(dev->power.active_jiffies));
> > +   spin_unlock_irq(&dev->power.lock);
> > +   return ret;
> > +}
> > +
> > +static DEVICE_ATTR(runtime_active_time, 0444, rtpm_active_time_show, NULL);
> > +
> > +static ssize_t rtpm_suspended_time_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +   int ret;
> > +   spin_lock_irq(&dev->power.lock);
> > +   update_pm_runtime_accounting(dev);
> > +   ret = sprintf(buf, "%i\n",
> > +           jiffies_to_msecs(dev->power.suspended_jiffies));
> > +   spin_unlock_irq(&dev->power.lock);
> > +   return ret;
> > +}
> > +
> > +static DEVICE_ATTR(runtime_suspended_time, 0444, rtpm_suspended_time_show, NULL);
> > +
> > +static ssize_t rtpm_status_show(struct device *dev,
> > +                           struct device_attribute *attr, char *buf)
> > +{
> > +   const char *p;
> > +
> > +   if (dev->power.runtime_error) {
> > +           p = "error\n";
> > +   } else if (dev->power.disable_depth) {
> > +           p = "unsupported\n";
> > +   } else {
> > +           switch (dev->power.runtime_status) {
> > +           case RPM_SUSPENDED:
> > +                   p = "suspended\n";
> > +                   break;
> > +           case RPM_SUSPENDING:
> > +                   p = "suspending\n";
> > +                   break;
> > +           case RPM_RESUMING:
> > +                   p = "resuming\n";
> > +                   break;
> > +           case RPM_ACTIVE:
> > +                   p = "active\n";
> > +                   break;
> > +           default:
> > +                   return -EIO;
> > +           }
> > +   }
> > +   return sprintf(buf, p);
> > +}
> > +
> > +static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL);
> >   #endif
> >
> >   static ssize_t
> > @@ -172,27 +232,8 @@ static ssize_t rtpm_enabled_show(struct device *dev,
> >     return sprintf(buf, "enabled\n");
> >   }
> >
> > -static ssize_t rtpm_status_show(struct device *dev,
> > -                           struct device_attribute *attr, char *buf)
> > -{
> > -   if (dev->power.runtime_error)
> > -           return sprintf(buf, "error\n");
> > -   switch (dev->power.runtime_status) {
> > -   case RPM_SUSPENDED:
> > -           return sprintf(buf, "suspended\n");
> > -   case RPM_SUSPENDING:
> > -           return sprintf(buf, "suspending\n");
> > -   case RPM_RESUMING:
> > -           return sprintf(buf, "resuming\n");
> > -   case RPM_ACTIVE:
> > -           return sprintf(buf, "active\n");
> > -   }
> > -   return -EIO;
> > -}
> > -

Completely removing rtpm_status_show() doesn't appear correct, as this
is not removed in the upstream version of the patch.

> >   static DEVICE_ATTR(runtime_usage, 0444, rtpm_usagecount_show, NULL);
> >   static DEVICE_ATTR(runtime_active_kids, 0444, rtpm_children_show, NULL);
> > -static DEVICE_ATTR(runtime_status, 0444, rtpm_status_show, NULL);
> >   static DEVICE_ATTR(runtime_enabled, 0444, rtpm_enabled_show, NULL);
> >
> >   #endif
> > @@ -228,6 +269,9 @@ static DEVICE_ATTR(async, 0644, async_show, async_store);
> >   static struct attribute * power_attrs[] = {
> >   #ifdef CONFIG_PM_RUNTIME
> >     &dev_attr_control.attr,
> > +   &dev_attr_runtime_status.attr,
> > +   &dev_attr_runtime_suspended_time.attr,
> > +   &dev_attr_runtime_active_time.attr,
> >   #endif
> >     &dev_attr_wakeup.attr,
> >   #ifdef CONFIG_PM_ADVANCED_DEBUG
> > @@ -235,7 +279,6 @@ static struct attribute * power_attrs[] = {
> >   #ifdef CONFIG_PM_RUNTIME
> >     &dev_attr_runtime_usage.attr,
> >     &dev_attr_runtime_active_kids.attr,
> > -   &dev_attr_runtime_status.attr,
> >     &dev_attr_runtime_enabled.attr,
> >   #endif
> >   #endif
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 8e258c7..dca597f 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -476,9 +476,15 @@ struct dev_pm_info {
> >     enum rpm_request        request;
> >     enum rpm_status         runtime_status;
> >     int                     runtime_error;
> > +   unsigned long           active_jiffies;
> > +   unsigned long           suspended_jiffies;
> > +   unsigned long           accounting_timestamp;
> >   #endif
> >   };
> >
> > +extern void update_pm_runtime_accounting(struct device *dev);
> > +
> > +
> >   /*
> >    * The PM_EVENT_ messages are also used by drivers implementing the legacy
> >    * suspend framework, based on the ->suspend() and ->resume() callbacks common
>
> This isn't a faithful backport of
> 8d4b9d1bfef117862a2889dec4dac227068544c9. What happened to
> rtpm_status_show() ? Even if its not used I'd prefer to keep that
> function in the patch so as to cause fewer conflicts if there are
> upstream stable patches.
>
> I prefer the attached version.

Tim's patch was on the right track to provide an updated
rtpm_status_show() routine from what we currently have in Maverick, but
this still didn't originate from upstream commit 8d4b9d1b.

I've thus taken the path of least resistance and cleanly cherry-picked
the following for the Ubuntu Maverick kernel:

commit 0fcb4eef8294492c8f1de8236b1ed81f09e42922
Author: Alan Stern <stern@rowland.harvard.edu>
Date:   Thu Jul 8 00:05:37 2010 +0200

   PM / Runtime: Make runtime_status attribute not debug-only (v. 2)

commit 8d4b9d1bfef117862a2889dec4dac227068544c9
Author: Arjan van de Ven <arjan@linux.intel.com>
Date:   Mon Jul 19 02:01:06 2010 +0200

   PM / Runtime: Add runtime PM statistics (v3)

Thanks,
Leann