diff mbox

[2/2] libata: Add ALPM power state accounting to the AHCI driver

Message ID 1284359477-2177-3-git-send-email-yong.shen@linaro.org
State Accepted
Delegated to: Leann Ogasawara
Headers show

Commit Message

yong.shen@linaro.org Sept. 13, 2010, 6:31 a.m. UTC 
From: Arjan van de Ven <arjan@linux.intel.com>

PowerTOP wants to be able to show the user how effective the ALPM link
power management is for the user. ALPM is worth around 0.5W on a quiet
link; PowerTOP wants to be able to find cases where the "quiet link" isn't
actually quiet.

This patch adds state accounting functionality to the AHCI driver for
PowerTOP to use.
The parts of the patch are
1) the sysfs logic of exposing the stats for each state in sysfs
2) the basic accounting logic that gets update on link change interrupts
   (or when the user accesses the info from sysfs)
3) a "accounting enable" flag; in order to get the accounting to work,
   the driver needs to get phyrdy interrupts on link status changes.
   Normally and currently this is disabled by the driver when ALPM is
   on (to reduce overhead); when PowerTOP is running this will need
   to be on to get usable statistics... hence the sysfs tunable.

The PowerTOP output currently looks like this:

Recent SATA AHCI link activity statistics
Active	Partial	Slumber	Device name
  0.5%	 99.5%	  0.0%	host0

(work to resolve "host0" to a more human readable name is in progress)

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Shen Yong <yong.shen@linaro.org>
---
 drivers/ata/ahci.h    |   15 ++++
 drivers/ata/libahci.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 203 insertions(+), 2 deletions(-)

Comments

Tim Gardner Sept. 14, 2010, 4:37 p.m. UTC | #1 
On 09/13/2010 12:31 AM, yong.shen@linaro.org wrote:
> From: Arjan van de Ven<arjan@linux.intel.com>
>
> PowerTOP wants to be able to show the user how effective the ALPM link
> power management is for the user. ALPM is worth around 0.5W on a quiet
> link; PowerTOP wants to be able to find cases where the "quiet link" isn't
> actually quiet.
>
> This patch adds state accounting functionality to the AHCI driver for
> PowerTOP to use.
> The parts of the patch are
> 1) the sysfs logic of exposing the stats for each state in sysfs
> 2) the basic accounting logic that gets update on link change interrupts
>     (or when the user accesses the info from sysfs)
> 3) a "accounting enable" flag; in order to get the accounting to work,
>     the driver needs to get phyrdy interrupts on link status changes.
>     Normally and currently this is disabled by the driver when ALPM is
>     on (to reduce overhead); when PowerTOP is running this will need
>     to be on to get usable statistics... hence the sysfs tunable.
>
> The PowerTOP output currently looks like this:
>
> Recent SATA AHCI link activity statistics
> Active	Partial	Slumber	Device name
>    0.5%	 99.5%	  0.0%	host0
>
> (work to resolve "host0" to a more human readable name is in progress)
>
> Signed-off-by: Arjan van de Ven<arjan@linux.intel.com>
> Signed-off-by: Shen Yong<yong.shen@linaro.org>
> ---
>   drivers/ata/ahci.h    |   15 ++++
>   drivers/ata/libahci.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 7113c57..6a3a291 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -261,6 +261,13 @@ struct ahci_em_priv {
>   	unsigned long led_state;
>   };
>
> +enum ahci_port_states {
> +	AHCI_PORT_NOLINK = 0,
> +	AHCI_PORT_ACTIVE = 1,
> +	AHCI_PORT_PARTIAL = 2,
> +	AHCI_PORT_SLUMBER = 3
> +};
> +
>   struct ahci_port_priv {
>   	struct ata_link		*active_link;
>   	struct ahci_cmd_hdr	*cmd_slot;
> @@ -279,6 +286,14 @@ struct ahci_port_priv {
>   	int			fbs_last_dev;	/* save FBS.DEV of last FIS */
>   	/* enclosure management info per PM slot */
>   	struct ahci_em_priv	em_priv[EM_MAX_SLOTS];
> +
> +	/* ALPM accounting state and stats */
> +	unsigned int 		accounting_active:1;
> +	u64			active_jiffies;
> +	u64			partial_jiffies;
> +	u64			slumber_jiffies;
> +	int			previous_state;
> +	int			previous_jiffies;
>   };
>
>   struct ahci_host_priv {
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 81e772a..c3250ee 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -59,6 +59,20 @@ MODULE_PARM_DESC(ignore_sss, "Ignore staggered spinup flag (0=don't ignore, 1=ig
>   static int ahci_enable_alpm(struct ata_port *ap,
>   		enum link_pm policy);
>   static void ahci_disable_alpm(struct ata_port *ap);
> +static ssize_t ahci_alpm_show_active(struct device *dev,
> +				  struct device_attribute *attr, char *buf);
> +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> +				  struct device_attribute *attr, char *buf);
> +static ssize_t ahci_alpm_show_partial(struct device *dev,
> +				  struct device_attribute *attr, char *buf);
> +
> +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> +				  struct device_attribute *attr, char *buf);
> +
> +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count);
> +
>   static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
>   static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
>   			      size_t size);
> @@ -118,6 +132,12 @@ static DEVICE_ATTR(ahci_host_caps, S_IRUGO, ahci_show_host_caps, NULL);
>   static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2, NULL);
>   static DEVICE_ATTR(ahci_host_version, S_IRUGO, ahci_show_host_version, NULL);
>   static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
> +static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active, NULL);
> +static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial, NULL);
> +static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber, NULL);
> +static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
> +		ahci_alpm_show_accounting, ahci_alpm_set_accounting);
> +
>   static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
>   		   ahci_read_em_buffer, ahci_store_em_buffer);
>
> @@ -129,6 +149,10 @@ static struct device_attribute *ahci_shost_attrs[] = {
>   	&dev_attr_ahci_host_cap2,
>   	&dev_attr_ahci_host_version,
>   	&dev_attr_ahci_port_cmd,
> +	&dev_attr_ahci_alpm_active,
> +	&dev_attr_ahci_alpm_partial,
> +	&dev_attr_ahci_alpm_slumber,
> +	&dev_attr_ahci_alpm_accounting,
>   	&dev_attr_em_buffer,
>   	NULL
>   };
> @@ -734,9 +758,14 @@ static int ahci_enable_alpm(struct ata_port *ap,
>   	 * getting woken up due to spurious phy ready interrupts
>   	 * TBD - Hot plug should be done via polling now, is
>   	 * that even supported?
> +	 *
> +	 * However, when accounting_active is set, we do want
> +	 * the interrupts for accounting purposes.
>   	 */
> -	pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> -	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> +	if (!pp->accounting_active) {
> +		pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> +	}
>
>   	/*
>   	 * Set a flag to indicate that we should ignore all PhyRdy
> @@ -1645,6 +1674,162 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
>   		ata_port_abort(ap);
>   }
>
> +static int get_current_alpm_state(struct ata_port *ap)
> +{
> +	u32 status = 0;
> +
> +	ahci_scr_read(&ap->link, SCR_STATUS,&status);
> +
> +	/* link status is in bits 11-8 */
> +	status = status>>  8;
> +	status = status&  0x7;
> +
> +	if (status == 6)
> +		return AHCI_PORT_SLUMBER;
> +	if (status == 2)
> +		return AHCI_PORT_PARTIAL;
> +	if (status == 1)
> +		return AHCI_PORT_ACTIVE;
> +	return AHCI_PORT_NOLINK;
> +}
> +
> +static void account_alpm_stats(struct ata_port *ap)
> +{
> +	struct ahci_port_priv *pp;
> +
> +	int new_state;
> +	u64 new_jiffies, jiffies_delta;
> +
> +	if (ap == NULL)
> +		return;
> +	pp = ap->private_data;
> +
> +	if (!pp) return;
> +
> +	new_state = get_current_alpm_state(ap);
> +	new_jiffies = jiffies;
> +
> +	jiffies_delta = new_jiffies - pp->previous_jiffies;
> +
> +	switch (pp->previous_state) {
> +	case AHCI_PORT_NOLINK:
> +		pp->active_jiffies = 0;
> +		pp->partial_jiffies = 0;
> +		pp->slumber_jiffies = 0;
> +		break;
> +	case AHCI_PORT_ACTIVE:
> +		pp->active_jiffies += jiffies_delta;
> +		break;
> +	case AHCI_PORT_PARTIAL:
> +		pp->partial_jiffies += jiffies_delta;
> +		break;
> +	case AHCI_PORT_SLUMBER:
> +		pp->slumber_jiffies += jiffies_delta;
> +		break;
> +	default:
> +		break;
> +	}
> +	pp->previous_state = new_state;
> +	pp->previous_jiffies = new_jiffies;
> +}
> +
> +static ssize_t ahci_alpm_show_active(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	struct ahci_port_priv *pp;
> +
> +	if (!ap || ata_port_is_dummy(ap))
> +		return -EINVAL;
> +	pp = ap->private_data;
> +	account_alpm_stats(ap);
> +
> +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
> +}
> +
> +static ssize_t ahci_alpm_show_partial(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	struct ahci_port_priv *pp;
> +
> +	if (!ap || ata_port_is_dummy(ap))
> +		return -EINVAL;
> +
> +	pp = ap->private_data;
> +	account_alpm_stats(ap);
> +
> +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
> +}
> +
> +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	struct ahci_port_priv *pp;
> +
> +	if (!ap || ata_port_is_dummy(ap))
> +		return -EINVAL;
> +
> +	pp = ap->private_data;
> +
> +	account_alpm_stats(ap);
> +
> +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
> +}
> +
> +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> +				  struct device_attribute *attr, char *buf)
> +{
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	struct ahci_port_priv *pp;
> +
> +	if (!ap || ata_port_is_dummy(ap))
> +		return -EINVAL;
> +
> +	pp = ap->private_data;
> +
> +	return sprintf(buf, "%u\n", pp->accounting_active);
> +}
> +
> +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	unsigned long flags;
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct ata_port *ap = ata_shost_to_port(shost);
> +	struct ahci_port_priv *pp;
> +	void __iomem *port_mmio;
> +
> +	if (!ap || ata_port_is_dummy(ap))
> +		return 1;
> +
> +	pp = ap->private_data;
> +	port_mmio = ahci_port_base(ap);
> +
> +	if (!pp)
> +		return 1;
> +	if (buf[0] == '0')
> +		pp->accounting_active = 0;
> +	if (buf[0] == '1')
> +		pp->accounting_active = 1;
> +
> +	/* we need to enable the PHYRDY interrupt when we want accounting */
> +	if (pp->accounting_active) {
> +		spin_lock_irqsave(ap->lock, flags);
> +		pp->intr_mask |= PORT_IRQ_PHYRDY;
> +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> +		spin_unlock_irqrestore(ap->lock, flags);
> +	}
> +	return count;
> +}
> +
> +
>   static void ahci_port_intr(struct ata_port *ap)
>   {
>   	void __iomem *port_mmio = ahci_port_base(ap);
> @@ -1670,6 +1855,7 @@ static void ahci_port_intr(struct ata_port *ap)
>   	if ((hpriv->flags&  AHCI_HFLAG_NO_HOTPLUG)&&
>   		(status&  PORT_IRQ_PHYRDY)) {
>   		status&= ~PORT_IRQ_PHYRDY;
> +		account_alpm_stats(ap);
>   		ahci_scr_write(&ap->link, SCR_ERROR, ((1<<  16) | (1<<  18)));
>   	}
>

Yong Shen - is this patch faithful to the version released by Arjan? 
Since its not upstream I can't really tell. Otherwise this patch looks 
OK since accounting is not on by default. Note that statistics gathering 
within the PHY interrupt handler could have an impact on throughput. 
This is definitely a SAUCE patch since its not upstream, and is likely 
to suffer some edits before acceptance.

Does Maverick have a version of powertop that uses any of these sysfs 
entries?

Acked-by: Tim Gardner <tim.gardner@canoncial.com>
Leann Ogasawara Sept. 14, 2010, 10:33 p.m. UTC | #2 
On Tue, 2010-09-14 at 10:37 -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>
> >
> > PowerTOP wants to be able to show the user how effective the ALPM link
> > power management is for the user. ALPM is worth around 0.5W on a quiet
> > link; PowerTOP wants to be able to find cases where the "quiet link" isn't
> > actually quiet.
> >
> > This patch adds state accounting functionality to the AHCI driver for
> > PowerTOP to use.
> > The parts of the patch are
> > 1) the sysfs logic of exposing the stats for each state in sysfs
> > 2) the basic accounting logic that gets update on link change interrupts
> >     (or when the user accesses the info from sysfs)
> > 3) a "accounting enable" flag; in order to get the accounting to work,
> >     the driver needs to get phyrdy interrupts on link status changes.
> >     Normally and currently this is disabled by the driver when ALPM is
> >     on (to reduce overhead); when PowerTOP is running this will need
> >     to be on to get usable statistics... hence the sysfs tunable.
> >
> > The PowerTOP output currently looks like this:
> >
> > Recent SATA AHCI link activity statistics
> > Active	Partial	Slumber	Device name
> >    0.5%	 99.5%	  0.0%	host0
> >
> > (work to resolve "host0" to a more human readable name is in progress)
> >
> > Signed-off-by: Arjan van de Ven<arjan@linux.intel.com>
> > Signed-off-by: Shen Yong<yong.shen@linaro.org>
> > ---
> >   drivers/ata/ahci.h    |   15 ++++
> >   drivers/ata/libahci.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >   2 files changed, 203 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> > index 7113c57..6a3a291 100644
> > --- a/drivers/ata/ahci.h
> > +++ b/drivers/ata/ahci.h
> > @@ -261,6 +261,13 @@ struct ahci_em_priv {
> >   	unsigned long led_state;
> >   };
> >
> > +enum ahci_port_states {
> > +	AHCI_PORT_NOLINK = 0,
> > +	AHCI_PORT_ACTIVE = 1,
> > +	AHCI_PORT_PARTIAL = 2,
> > +	AHCI_PORT_SLUMBER = 3
> > +};
> > +
> >   struct ahci_port_priv {
> >   	struct ata_link		*active_link;
> >   	struct ahci_cmd_hdr	*cmd_slot;
> > @@ -279,6 +286,14 @@ struct ahci_port_priv {
> >   	int			fbs_last_dev;	/* save FBS.DEV of last FIS */
> >   	/* enclosure management info per PM slot */
> >   	struct ahci_em_priv	em_priv[EM_MAX_SLOTS];
> > +
> > +	/* ALPM accounting state and stats */
> > +	unsigned int 		accounting_active:1;
> > +	u64			active_jiffies;
> > +	u64			partial_jiffies;
> > +	u64			slumber_jiffies;
> > +	int			previous_state;
> > +	int			previous_jiffies;
> >   };
> >
> >   struct ahci_host_priv {
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index 81e772a..c3250ee 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -59,6 +59,20 @@ MODULE_PARM_DESC(ignore_sss, "Ignore staggered spinup flag (0=don't ignore, 1=ig
> >   static int ahci_enable_alpm(struct ata_port *ap,
> >   		enum link_pm policy);
> >   static void ahci_disable_alpm(struct ata_port *ap);
> > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > +				  struct device_attribute *attr, char *buf);
> > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > +				  struct device_attribute *attr, char *buf);
> > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > +				  struct device_attribute *attr, char *buf);
> > +
> > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > +				  struct device_attribute *attr, char *buf);
> > +
> > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  const char *buf, size_t count);
> > +
> >   static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
> >   static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
> >   			      size_t size);
> > @@ -118,6 +132,12 @@ static DEVICE_ATTR(ahci_host_caps, S_IRUGO, ahci_show_host_caps, NULL);
> >   static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2, NULL);
> >   static DEVICE_ATTR(ahci_host_version, S_IRUGO, ahci_show_host_version, NULL);
> >   static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
> > +static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active, NULL);
> > +static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial, NULL);
> > +static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber, NULL);
> > +static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
> > +		ahci_alpm_show_accounting, ahci_alpm_set_accounting);
> > +
> >   static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
> >   		   ahci_read_em_buffer, ahci_store_em_buffer);
> >
> > @@ -129,6 +149,10 @@ static struct device_attribute *ahci_shost_attrs[] = {
> >   	&dev_attr_ahci_host_cap2,
> >   	&dev_attr_ahci_host_version,
> >   	&dev_attr_ahci_port_cmd,
> > +	&dev_attr_ahci_alpm_active,
> > +	&dev_attr_ahci_alpm_partial,
> > +	&dev_attr_ahci_alpm_slumber,
> > +	&dev_attr_ahci_alpm_accounting,
> >   	&dev_attr_em_buffer,
> >   	NULL
> >   };
> > @@ -734,9 +758,14 @@ static int ahci_enable_alpm(struct ata_port *ap,
> >   	 * getting woken up due to spurious phy ready interrupts
> >   	 * TBD - Hot plug should be done via polling now, is
> >   	 * that even supported?
> > +	 *
> > +	 * However, when accounting_active is set, we do want
> > +	 * the interrupts for accounting purposes.
> >   	 */
> > -	pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > -	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > +	if (!pp->accounting_active) {
> > +		pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > +	}
> >
> >   	/*
> >   	 * Set a flag to indicate that we should ignore all PhyRdy
> > @@ -1645,6 +1674,162 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
> >   		ata_port_abort(ap);
> >   }
> >
> > +static int get_current_alpm_state(struct ata_port *ap)
> > +{
> > +	u32 status = 0;
> > +
> > +	ahci_scr_read(&ap->link, SCR_STATUS,&status);
> > +
> > +	/* link status is in bits 11-8 */
> > +	status = status>>  8;
> > +	status = status&  0x7;
> > +
> > +	if (status == 6)
> > +		return AHCI_PORT_SLUMBER;
> > +	if (status == 2)
> > +		return AHCI_PORT_PARTIAL;
> > +	if (status == 1)
> > +		return AHCI_PORT_ACTIVE;
> > +	return AHCI_PORT_NOLINK;
> > +}
> > +
> > +static void account_alpm_stats(struct ata_port *ap)
> > +{
> > +	struct ahci_port_priv *pp;
> > +
> > +	int new_state;
> > +	u64 new_jiffies, jiffies_delta;
> > +
> > +	if (ap == NULL)
> > +		return;
> > +	pp = ap->private_data;
> > +
> > +	if (!pp) return;
> > +
> > +	new_state = get_current_alpm_state(ap);
> > +	new_jiffies = jiffies;
> > +
> > +	jiffies_delta = new_jiffies - pp->previous_jiffies;
> > +
> > +	switch (pp->previous_state) {
> > +	case AHCI_PORT_NOLINK:
> > +		pp->active_jiffies = 0;
> > +		pp->partial_jiffies = 0;
> > +		pp->slumber_jiffies = 0;
> > +		break;
> > +	case AHCI_PORT_ACTIVE:
> > +		pp->active_jiffies += jiffies_delta;
> > +		break;
> > +	case AHCI_PORT_PARTIAL:
> > +		pp->partial_jiffies += jiffies_delta;
> > +		break;
> > +	case AHCI_PORT_SLUMBER:
> > +		pp->slumber_jiffies += jiffies_delta;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +	pp->previous_state = new_state;
> > +	pp->previous_jiffies = new_jiffies;
> > +}
> > +
> > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > +				   struct device_attribute *attr, char *buf)
> > +{
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +	struct ata_port *ap = ata_shost_to_port(shost);
> > +	struct ahci_port_priv *pp;
> > +
> > +	if (!ap || ata_port_is_dummy(ap))
> > +		return -EINVAL;
> > +	pp = ap->private_data;
> > +	account_alpm_stats(ap);
> > +
> > +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
> > +}
> > +
> > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > +				   struct device_attribute *attr, char *buf)
> > +{
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +	struct ata_port *ap = ata_shost_to_port(shost);
> > +	struct ahci_port_priv *pp;
> > +
> > +	if (!ap || ata_port_is_dummy(ap))
> > +		return -EINVAL;
> > +
> > +	pp = ap->private_data;
> > +	account_alpm_stats(ap);
> > +
> > +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
> > +}
> > +
> > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > +				   struct device_attribute *attr, char *buf)
> > +{
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +	struct ata_port *ap = ata_shost_to_port(shost);
> > +	struct ahci_port_priv *pp;
> > +
> > +	if (!ap || ata_port_is_dummy(ap))
> > +		return -EINVAL;
> > +
> > +	pp = ap->private_data;
> > +
> > +	account_alpm_stats(ap);
> > +
> > +	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
> > +}
> > +
> > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > +				  struct device_attribute *attr, char *buf)
> > +{
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +	struct ata_port *ap = ata_shost_to_port(shost);
> > +	struct ahci_port_priv *pp;
> > +
> > +	if (!ap || ata_port_is_dummy(ap))
> > +		return -EINVAL;
> > +
> > +	pp = ap->private_data;
> > +
> > +	return sprintf(buf, "%u\n", pp->accounting_active);
> > +}
> > +
> > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  const char *buf, size_t count)
> > +{
> > +	unsigned long flags;
> > +	struct Scsi_Host *shost = class_to_shost(dev);
> > +	struct ata_port *ap = ata_shost_to_port(shost);
> > +	struct ahci_port_priv *pp;
> > +	void __iomem *port_mmio;
> > +
> > +	if (!ap || ata_port_is_dummy(ap))
> > +		return 1;
> > +
> > +	pp = ap->private_data;
> > +	port_mmio = ahci_port_base(ap);
> > +
> > +	if (!pp)
> > +		return 1;
> > +	if (buf[0] == '0')
> > +		pp->accounting_active = 0;
> > +	if (buf[0] == '1')
> > +		pp->accounting_active = 1;
> > +
> > +	/* we need to enable the PHYRDY interrupt when we want accounting */
> > +	if (pp->accounting_active) {
> > +		spin_lock_irqsave(ap->lock, flags);
> > +		pp->intr_mask |= PORT_IRQ_PHYRDY;
> > +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > +		spin_unlock_irqrestore(ap->lock, flags);
> > +	}
> > +	return count;
> > +}
> > +
> > +
> >   static void ahci_port_intr(struct ata_port *ap)
> >   {
> >   	void __iomem *port_mmio = ahci_port_base(ap);
> > @@ -1670,6 +1855,7 @@ static void ahci_port_intr(struct ata_port *ap)
> >   	if ((hpriv->flags&  AHCI_HFLAG_NO_HOTPLUG)&&
> >   		(status&  PORT_IRQ_PHYRDY)) {
> >   		status&= ~PORT_IRQ_PHYRDY;
> > +		account_alpm_stats(ap);
> >   		ahci_scr_write(&ap->link, SCR_ERROR, ((1<<  16) | (1<<  18)));
> >   	}
> >
> 
> Yong Shen - is this patch faithful to the version released by Arjan?

This looks like a modified verison of the patch which Arjan submitted
upstream a while back, ie Nov 2009:

http://thread.gmane.org/gmane.linux.ide/43337

Has there been any recent follow up to get this accepted upstream?

> Since its not upstream I can't really tell. Otherwise this patch looks 
> OK since accounting is not on by default. Note that statistics gathering 
> within the PHY interrupt handler could have an impact on throughput. 
> This is definitely a SAUCE patch since its not upstream, and is likely 
> to suffer some edits before acceptance.
> 
> Does Maverick have a version of powertop that uses any of these sysfs 
> entries?
> 
> Acked-by: Tim Gardner <tim.gardner@canoncial.com>

Applied to Maverick linux master but marked as SAUCE.

Thanks,
Leann
yong.shen@linaro.org Sept. 15, 2010, 3:27 a.m. UTC | #3 
Hi there,

Well, I did not modify the content of the patch. It can be 'git am' on
ubuntu kernel without conflict.
This patch is directly from git://git.linaro.org/amitarora/powertop.git
commit cc1aaff10ee9046b835441d295fda498f68cb1ac
Author: Arjan van de Ven <arjan@linux.intel.com>
Date:   Mon Jul 12 17:25:49 2010 -0400

    alpm kernel patch

Based on my understanding, powertop only uses it to collect harddisk's
status. so if it is risky to merge to current ubuntu kernel, we may wait a
better version of this from Arjan.

Yong

On Wed, Sep 15, 2010 at 6:33 AM, Leann Ogasawara <
leann.ogasawara@canonical.com> wrote:

> On Tue, 2010-09-14 at 10:37 -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>
> > >
> > > PowerTOP wants to be able to show the user how effective the ALPM link
> > > power management is for the user. ALPM is worth around 0.5W on a quiet
> > > link; PowerTOP wants to be able to find cases where the "quiet link"
> isn't
> > > actually quiet.
> > >
> > > This patch adds state accounting functionality to the AHCI driver for
> > > PowerTOP to use.
> > > The parts of the patch are
> > > 1) the sysfs logic of exposing the stats for each state in sysfs
> > > 2) the basic accounting logic that gets update on link change
> interrupts
> > >     (or when the user accesses the info from sysfs)
> > > 3) a "accounting enable" flag; in order to get the accounting to work,
> > >     the driver needs to get phyrdy interrupts on link status changes.
> > >     Normally and currently this is disabled by the driver when ALPM is
> > >     on (to reduce overhead); when PowerTOP is running this will need
> > >     to be on to get usable statistics... hence the sysfs tunable.
> > >
> > > The PowerTOP output currently looks like this:
> > >
> > > Recent SATA AHCI link activity statistics
> > > Active      Partial Slumber Device name
> > >    0.5%      99.5%    0.0%  host0
> > >
> > > (work to resolve "host0" to a more human readable name is in progress)
> > >
> > > Signed-off-by: Arjan van de Ven<arjan@linux.intel.com>
> > > Signed-off-by: Shen Yong<yong.shen@linaro.org>
> > > ---
> > >   drivers/ata/ahci.h    |   15 ++++
> > >   drivers/ata/libahci.c |  190
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> > >   2 files changed, 203 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> > > index 7113c57..6a3a291 100644
> > > --- a/drivers/ata/ahci.h
> > > +++ b/drivers/ata/ahci.h
> > > @@ -261,6 +261,13 @@ struct ahci_em_priv {
> > >     unsigned long led_state;
> > >   };
> > >
> > > +enum ahci_port_states {
> > > +   AHCI_PORT_NOLINK = 0,
> > > +   AHCI_PORT_ACTIVE = 1,
> > > +   AHCI_PORT_PARTIAL = 2,
> > > +   AHCI_PORT_SLUMBER = 3
> > > +};
> > > +
> > >   struct ahci_port_priv {
> > >     struct ata_link         *active_link;
> > >     struct ahci_cmd_hdr     *cmd_slot;
> > > @@ -279,6 +286,14 @@ struct ahci_port_priv {
> > >     int                     fbs_last_dev;   /* save FBS.DEV of last FIS
> */
> > >     /* enclosure management info per PM slot */
> > >     struct ahci_em_priv     em_priv[EM_MAX_SLOTS];
> > > +
> > > +   /* ALPM accounting state and stats */
> > > +   unsigned int            accounting_active:1;
> > > +   u64                     active_jiffies;
> > > +   u64                     partial_jiffies;
> > > +   u64                     slumber_jiffies;
> > > +   int                     previous_state;
> > > +   int                     previous_jiffies;
> > >   };
> > >
> > >   struct ahci_host_priv {
> > > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > > index 81e772a..c3250ee 100644
> > > --- a/drivers/ata/libahci.c
> > > +++ b/drivers/ata/libahci.c
> > > @@ -59,6 +59,20 @@ MODULE_PARM_DESC(ignore_sss, "Ignore staggered
> spinup flag (0=don't ignore, 1=ig
> > >   static int ahci_enable_alpm(struct ata_port *ap,
> > >             enum link_pm policy);
> > >   static void ahci_disable_alpm(struct ata_port *ap);
> > > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > > +                             struct device_attribute *attr, char
> *buf);
> > > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > > +                             struct device_attribute *attr, char
> *buf);
> > > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > > +                             struct device_attribute *attr, char
> *buf);
> > > +
> > > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > > +                             struct device_attribute *attr, char
> *buf);
> > > +
> > > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > > +                             struct device_attribute *attr,
> > > +                             const char *buf, size_t count);
> > > +
> > >   static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
> > >   static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
> > >                           size_t size);
> > > @@ -118,6 +132,12 @@ static DEVICE_ATTR(ahci_host_caps, S_IRUGO,
> ahci_show_host_caps, NULL);
> > >   static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2,
> NULL);
> > >   static DEVICE_ATTR(ahci_host_version, S_IRUGO,
> ahci_show_host_version, NULL);
> > >   static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
> > > +static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
> > > +           ahci_alpm_show_accounting, ahci_alpm_set_accounting);
> > > +
> > >   static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
> > >                ahci_read_em_buffer, ahci_store_em_buffer);
> > >
> > > @@ -129,6 +149,10 @@ static struct device_attribute *ahci_shost_attrs[]
> = {
> > >     &dev_attr_ahci_host_cap2,
> > >     &dev_attr_ahci_host_version,
> > >     &dev_attr_ahci_port_cmd,
> > > +   &dev_attr_ahci_alpm_active,
> > > +   &dev_attr_ahci_alpm_partial,
> > > +   &dev_attr_ahci_alpm_slumber,
> > > +   &dev_attr_ahci_alpm_accounting,
> > >     &dev_attr_em_buffer,
> > >     NULL
> > >   };
> > > @@ -734,9 +758,14 @@ static int ahci_enable_alpm(struct ata_port *ap,
> > >      * getting woken up due to spurious phy ready interrupts
> > >      * TBD - Hot plug should be done via polling now, is
> > >      * that even supported?
> > > +    *
> > > +    * However, when accounting_active is set, we do want
> > > +    * the interrupts for accounting purposes.
> > >      */
> > > -   pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > > -   writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > +   if (!pp->accounting_active) {
> > > +           pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > > +           writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > +   }
> > >
> > >     /*
> > >      * Set a flag to indicate that we should ignore all PhyRdy
> > > @@ -1645,6 +1674,162 @@ static void ahci_error_intr(struct ata_port
> *ap, u32 irq_stat)
> > >             ata_port_abort(ap);
> > >   }
> > >
> > > +static int get_current_alpm_state(struct ata_port *ap)
> > > +{
> > > +   u32 status = 0;
> > > +
> > > +   ahci_scr_read(&ap->link, SCR_STATUS,&status);
> > > +
> > > +   /* link status is in bits 11-8 */
> > > +   status = status>>  8;
> > > +   status = status&  0x7;
> > > +
> > > +   if (status == 6)
> > > +           return AHCI_PORT_SLUMBER;
> > > +   if (status == 2)
> > > +           return AHCI_PORT_PARTIAL;
> > > +   if (status == 1)
> > > +           return AHCI_PORT_ACTIVE;
> > > +   return AHCI_PORT_NOLINK;
> > > +}
> > > +
> > > +static void account_alpm_stats(struct ata_port *ap)
> > > +{
> > > +   struct ahci_port_priv *pp;
> > > +
> > > +   int new_state;
> > > +   u64 new_jiffies, jiffies_delta;
> > > +
> > > +   if (ap == NULL)
> > > +           return;
> > > +   pp = ap->private_data;
> > > +
> > > +   if (!pp) return;
> > > +
> > > +   new_state = get_current_alpm_state(ap);
> > > +   new_jiffies = jiffies;
> > > +
> > > +   jiffies_delta = new_jiffies - pp->previous_jiffies;
> > > +
> > > +   switch (pp->previous_state) {
> > > +   case AHCI_PORT_NOLINK:
> > > +           pp->active_jiffies = 0;
> > > +           pp->partial_jiffies = 0;
> > > +           pp->slumber_jiffies = 0;
> > > +           break;
> > > +   case AHCI_PORT_ACTIVE:
> > > +           pp->active_jiffies += jiffies_delta;
> > > +           break;
> > > +   case AHCI_PORT_PARTIAL:
> > > +           pp->partial_jiffies += jiffies_delta;
> > > +           break;
> > > +   case AHCI_PORT_SLUMBER:
> > > +           pp->slumber_jiffies += jiffies_delta;
> > > +           break;
> > > +   default:
> > > +           break;
> > > +   }
> > > +   pp->previous_state = new_state;
> > > +   pp->previous_jiffies = new_jiffies;
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > > +                              struct device_attribute *attr, char
> *buf)
> > > +{
> > > +   struct Scsi_Host *shost = class_to_shost(dev);
> > > +   struct ata_port *ap = ata_shost_to_port(shost);
> > > +   struct ahci_port_priv *pp;
> > > +
> > > +   if (!ap || ata_port_is_dummy(ap))
> > > +           return -EINVAL;
> > > +   pp = ap->private_data;
> > > +   account_alpm_stats(ap);
> > > +
> > > +   return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > > +                              struct device_attribute *attr, char
> *buf)
> > > +{
> > > +   struct Scsi_Host *shost = class_to_shost(dev);
> > > +   struct ata_port *ap = ata_shost_to_port(shost);
> > > +   struct ahci_port_priv *pp;
> > > +
> > > +   if (!ap || ata_port_is_dummy(ap))
> > > +           return -EINVAL;
> > > +
> > > +   pp = ap->private_data;
> > > +   account_alpm_stats(ap);
> > > +
> > > +   return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > > +                              struct device_attribute *attr, char
> *buf)
> > > +{
> > > +   struct Scsi_Host *shost = class_to_shost(dev);
> > > +   struct ata_port *ap = ata_shost_to_port(shost);
> > > +   struct ahci_port_priv *pp;
> > > +
> > > +   if (!ap || ata_port_is_dummy(ap))
> > > +           return -EINVAL;
> > > +
> > > +   pp = ap->private_data;
> > > +
> > > +   account_alpm_stats(ap);
> > > +
> > > +   return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > > +                             struct device_attribute *attr, char *buf)
> > > +{
> > > +   struct Scsi_Host *shost = class_to_shost(dev);
> > > +   struct ata_port *ap = ata_shost_to_port(shost);
> > > +   struct ahci_port_priv *pp;
> > > +
> > > +   if (!ap || ata_port_is_dummy(ap))
> > > +           return -EINVAL;
> > > +
> > > +   pp = ap->private_data;
> > > +
> > > +   return sprintf(buf, "%u\n", pp->accounting_active);
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > > +                             struct device_attribute *attr,
> > > +                             const char *buf, size_t count)
> > > +{
> > > +   unsigned long flags;
> > > +   struct Scsi_Host *shost = class_to_shost(dev);
> > > +   struct ata_port *ap = ata_shost_to_port(shost);
> > > +   struct ahci_port_priv *pp;
> > > +   void __iomem *port_mmio;
> > > +
> > > +   if (!ap || ata_port_is_dummy(ap))
> > > +           return 1;
> > > +
> > > +   pp = ap->private_data;
> > > +   port_mmio = ahci_port_base(ap);
> > > +
> > > +   if (!pp)
> > > +           return 1;
> > > +   if (buf[0] == '0')
> > > +           pp->accounting_active = 0;
> > > +   if (buf[0] == '1')
> > > +           pp->accounting_active = 1;
> > > +
> > > +   /* we need to enable the PHYRDY interrupt when we want accounting
> */
> > > +   if (pp->accounting_active) {
> > > +           spin_lock_irqsave(ap->lock, flags);
> > > +           pp->intr_mask |= PORT_IRQ_PHYRDY;
> > > +           writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > +           spin_unlock_irqrestore(ap->lock, flags);
> > > +   }
> > > +   return count;
> > > +}
> > > +
> > > +
> > >   static void ahci_port_intr(struct ata_port *ap)
> > >   {
> > >     void __iomem *port_mmio = ahci_port_base(ap);
> > > @@ -1670,6 +1855,7 @@ static void ahci_port_intr(struct ata_port *ap)
> > >     if ((hpriv->flags&  AHCI_HFLAG_NO_HOTPLUG)&&
> > >             (status&  PORT_IRQ_PHYRDY)) {
> > >             status&= ~PORT_IRQ_PHYRDY;
> > > +           account_alpm_stats(ap);
> > >             ahci_scr_write(&ap->link, SCR_ERROR, ((1<<  16) | (1<<
>  18)));
> > >     }
> > >
> >
> > Yong Shen - is this patch faithful to the version released by Arjan?
>
> This looks like a modified verison of the patch which Arjan submitted
> upstream a while back, ie Nov 2009:
>
> http://thread.gmane.org/gmane.linux.ide/43337
>
> Has there been any recent follow up to get this accepted upstream?
>
> > Since its not upstream I can't really tell. Otherwise this patch looks
> > OK since accounting is not on by default. Note that statistics gathering
> > within the PHY interrupt handler could have an impact on throughput.
> > This is definitely a SAUCE patch since its not upstream, and is likely
> > to suffer some edits before acceptance.
> >
> > Does Maverick have a version of powertop that uses any of these sysfs
> > entries?
> >
> > Acked-by: Tim Gardner <tim.gardner@canoncial.com>
>
> Applied to Maverick linux master but marked as SAUCE.
>
> Thanks,
> Leann
>
>
Amit Kucheria Sept. 15, 2010, 6:56 a.m. UTC | #4 
On 10 Sep 14, Leann Ogasawara wrote:
> > 
> > Yong Shen - is this patch faithful to the version released by Arjan?
> 
> This looks like a modified verison of the patch which Arjan submitted
> upstream a while back, ie Nov 2009:
> 
> http://thread.gmane.org/gmane.linux.ide/43337
> 
> Has there been any recent follow up to get this accepted upstream?
> 
> > Since its not upstream I can't really tell. Otherwise this patch looks 
> > OK since accounting is not on by default. Note that statistics gathering 
> > within the PHY interrupt handler could have an impact on throughput. 
> > This is definitely a SAUCE patch since its not upstream, and is likely 
> > to suffer some edits before acceptance.
> > 
> > Does Maverick have a version of powertop that uses any of these sysfs 
> > entries?
> > 
> > Acked-by: Tim Gardner <tim.gardner@canoncial.com>

The confusion seems to stem from the fact that Yong got the patches from the
powertop git tree (where they are present as patch files).

So where available, an upstream verion should be applied. We should've done
our homework on this though.

Cheers,
Amit
diff mbox

Patch

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 7113c57..6a3a291 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -261,6 +261,13 @@  struct ahci_em_priv {
 	unsigned long led_state;
 };
 
+enum ahci_port_states {
+	AHCI_PORT_NOLINK = 0,
+	AHCI_PORT_ACTIVE = 1,
+	AHCI_PORT_PARTIAL = 2,
+	AHCI_PORT_SLUMBER = 3
+};
+
 struct ahci_port_priv {
 	struct ata_link		*active_link;
 	struct ahci_cmd_hdr	*cmd_slot;
@@ -279,6 +286,14 @@  struct ahci_port_priv {
 	int			fbs_last_dev;	/* save FBS.DEV of last FIS */
 	/* enclosure management info per PM slot */
 	struct ahci_em_priv	em_priv[EM_MAX_SLOTS];
+
+	/* ALPM accounting state and stats */
+	unsigned int 		accounting_active:1;
+	u64			active_jiffies;
+	u64			partial_jiffies;
+	u64			slumber_jiffies;
+	int			previous_state;
+	int			previous_jiffies;
 };
 
 struct ahci_host_priv {
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 81e772a..c3250ee 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -59,6 +59,20 @@  MODULE_PARM_DESC(ignore_sss, "Ignore staggered spinup flag (0=don't ignore, 1=ig
 static int ahci_enable_alpm(struct ata_port *ap,
 		enum link_pm policy);
 static void ahci_disable_alpm(struct ata_port *ap);
+static ssize_t ahci_alpm_show_active(struct device *dev,
+				  struct device_attribute *attr, char *buf);
+static ssize_t ahci_alpm_show_slumber(struct device *dev,
+				  struct device_attribute *attr, char *buf);
+static ssize_t ahci_alpm_show_partial(struct device *dev,
+				  struct device_attribute *attr, char *buf);
+
+static ssize_t ahci_alpm_show_accounting(struct device *dev,
+				  struct device_attribute *attr, char *buf);
+
+static ssize_t ahci_alpm_set_accounting(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count);
+
 static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
 static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
 			      size_t size);
@@ -118,6 +132,12 @@  static DEVICE_ATTR(ahci_host_caps, S_IRUGO, ahci_show_host_caps, NULL);
 static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2, NULL);
 static DEVICE_ATTR(ahci_host_version, S_IRUGO, ahci_show_host_version, NULL);
 static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
+static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active, NULL);
+static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial, NULL);
+static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber, NULL);
+static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
+		ahci_alpm_show_accounting, ahci_alpm_set_accounting);
+
 static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
 		   ahci_read_em_buffer, ahci_store_em_buffer);
 
@@ -129,6 +149,10 @@  static struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_ahci_host_cap2,
 	&dev_attr_ahci_host_version,
 	&dev_attr_ahci_port_cmd,
+	&dev_attr_ahci_alpm_active,
+	&dev_attr_ahci_alpm_partial,
+	&dev_attr_ahci_alpm_slumber,
+	&dev_attr_ahci_alpm_accounting,
 	&dev_attr_em_buffer,
 	NULL
 };
@@ -734,9 +758,14 @@  static int ahci_enable_alpm(struct ata_port *ap,
 	 * getting woken up due to spurious phy ready interrupts
 	 * TBD - Hot plug should be done via polling now, is
 	 * that even supported?
+	 *
+	 * However, when accounting_active is set, we do want
+	 * the interrupts for accounting purposes.
 	 */
-	pp->intr_mask &= ~PORT_IRQ_PHYRDY;
-	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+	if (!pp->accounting_active) {
+		pp->intr_mask &= ~PORT_IRQ_PHYRDY;
+		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+	}
 
 	/*
 	 * Set a flag to indicate that we should ignore all PhyRdy
@@ -1645,6 +1674,162 @@  static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
 		ata_port_abort(ap);
 }
 
+static int get_current_alpm_state(struct ata_port *ap)
+{
+	u32 status = 0;
+
+	ahci_scr_read(&ap->link, SCR_STATUS, &status);
+
+	/* link status is in bits 11-8 */
+	status = status >> 8;
+	status = status & 0x7;
+
+	if (status == 6)
+		return AHCI_PORT_SLUMBER;
+	if (status == 2)
+		return AHCI_PORT_PARTIAL;
+	if (status == 1)
+		return AHCI_PORT_ACTIVE;
+	return AHCI_PORT_NOLINK;
+}
+
+static void account_alpm_stats(struct ata_port *ap)
+{
+	struct ahci_port_priv *pp;
+
+	int new_state;
+	u64 new_jiffies, jiffies_delta;
+
+	if (ap == NULL)
+		return;
+	pp = ap->private_data;
+
+	if (!pp) return;
+
+	new_state = get_current_alpm_state(ap);
+	new_jiffies = jiffies;
+
+	jiffies_delta = new_jiffies - pp->previous_jiffies;
+
+	switch (pp->previous_state) {
+	case AHCI_PORT_NOLINK:
+		pp->active_jiffies = 0;
+		pp->partial_jiffies = 0;
+		pp->slumber_jiffies = 0;
+		break;
+	case AHCI_PORT_ACTIVE:
+		pp->active_jiffies += jiffies_delta;
+		break;
+	case AHCI_PORT_PARTIAL:
+		pp->partial_jiffies += jiffies_delta;
+		break;
+	case AHCI_PORT_SLUMBER:
+		pp->slumber_jiffies += jiffies_delta;
+		break;
+	default:
+		break;
+	}
+	pp->previous_state = new_state;
+	pp->previous_jiffies = new_jiffies;
+}
+
+static ssize_t ahci_alpm_show_active(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	struct ahci_port_priv *pp;
+
+	if (!ap || ata_port_is_dummy(ap))
+		return -EINVAL;
+	pp = ap->private_data;
+	account_alpm_stats(ap);
+
+	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
+}
+
+static ssize_t ahci_alpm_show_partial(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	struct ahci_port_priv *pp;
+
+	if (!ap || ata_port_is_dummy(ap))
+		return -EINVAL;
+
+	pp = ap->private_data;
+	account_alpm_stats(ap);
+
+	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
+}
+
+static ssize_t ahci_alpm_show_slumber(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	struct ahci_port_priv *pp;
+
+	if (!ap || ata_port_is_dummy(ap))
+		return -EINVAL;
+
+	pp = ap->private_data;
+
+	account_alpm_stats(ap);
+
+	return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
+}
+
+static ssize_t ahci_alpm_show_accounting(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	struct ahci_port_priv *pp;
+
+	if (!ap || ata_port_is_dummy(ap))
+		return -EINVAL;
+
+	pp = ap->private_data;
+
+	return sprintf(buf, "%u\n", pp->accounting_active);
+}
+
+static ssize_t ahci_alpm_set_accounting(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	unsigned long flags;
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct ata_port *ap = ata_shost_to_port(shost);
+	struct ahci_port_priv *pp;
+	void __iomem *port_mmio;
+
+	if (!ap || ata_port_is_dummy(ap))
+		return 1;
+
+	pp = ap->private_data;
+	port_mmio = ahci_port_base(ap);
+
+	if (!pp)
+		return 1;
+	if (buf[0] == '0')
+		pp->accounting_active = 0;
+	if (buf[0] == '1')
+		pp->accounting_active = 1;
+
+	/* we need to enable the PHYRDY interrupt when we want accounting */
+	if (pp->accounting_active) {
+		spin_lock_irqsave(ap->lock, flags);
+		pp->intr_mask |= PORT_IRQ_PHYRDY;
+		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
+	return count;
+}
+
+
 static void ahci_port_intr(struct ata_port *ap)
 {
 	void __iomem *port_mmio = ahci_port_base(ap);
@@ -1670,6 +1855,7 @@  static void ahci_port_intr(struct ata_port *ap)
 	if ((hpriv->flags & AHCI_HFLAG_NO_HOTPLUG) &&
 		(status & PORT_IRQ_PHYRDY)) {
 		status &= ~PORT_IRQ_PHYRDY;
+		account_alpm_stats(ap);
 		ahci_scr_write(&ap->link, SCR_ERROR, ((1 << 16) | (1 << 18)));
 	}