Re: [patch 3/4] Enable link power management for ata drivers

2007-08-13 Thread Kristen Carlson Accardi
Andrew, can you fold this into this patch


change to KERN_INFO for failure to set pm policy

Signed-off-by:  Kristen Carlson Accardi <[EMAIL PROTECTED]>
---
 drivers/ata/libata-scsi.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c  2007-08-13 09:40:18.0 
-0700
+++ 2.6-git/drivers/ata/libata-scsi.c   2007-08-13 10:23:04.0 -0700
@@ -3009,7 +3009,7 @@ int ata_scsi_set_link_pm_policy(struct a
 */
if ((dev->horkage & ATA_HORKAGE_IPM) ||
!(dev->flags & ATA_DFLAG_IPM)) {
-   ata_dev_printk(dev, KERN_ERR,
+   ata_dev_printk(dev, KERN_INFO,
"Unable to set Link PM policy\n");
ap->pm_policy = MAX_PERFORMANCE;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-13 Thread Kristen Carlson Accardi
Andrew, can you fold this into this patch


change to KERN_INFO for failure to set pm policy

Signed-off-by:  Kristen Carlson Accardi [EMAIL PROTECTED]
---
 drivers/ata/libata-scsi.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c  2007-08-13 09:40:18.0 
-0700
+++ 2.6-git/drivers/ata/libata-scsi.c   2007-08-13 10:23:04.0 -0700
@@ -3009,7 +3009,7 @@ int ata_scsi_set_link_pm_policy(struct a
 */
if ((dev-horkage  ATA_HORKAGE_IPM) ||
!(dev-flags  ATA_DFLAG_IPM)) {
-   ata_dev_printk(dev, KERN_ERR,
+   ata_dev_printk(dev, KERN_INFO,
Unable to set Link PM policy\n);
ap-pm_policy = MAX_PERFORMANCE;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-10 Thread Valdis . Kletnieks
On Thu, 09 Aug 2007 14:24:16 PDT, Kristen Carlson Accardi said:
> +++ 2.6-git/drivers/ata/libata-scsi.c
> @@ -2904,6 +2976,52 @@ void ata_scsi_simulate(struct ata_device

> + if ((dev->horkage & ATA_HORKAGE_IPM) ||
> + !(dev->flags & ATA_DFLAG_IPM)) {
> + ata_dev_printk(dev, KERN_ERR,
> + "Unable to set Link PM policy\n");
> + ap->pm_policy = MAX_PERFORMANCE;
> + }

KERN_INFO please, or KERN_WARNING at the highest, at least until such time
as enough drivers support enough hardware that it really *does* qualify for
"this should not fail" status.

(OK, so I'm just cranky because I'm tired of seeing a KERN_ERR thrown at every
reboot, just because the ata_piix driver doesn't know how to set this stuff up
for the DVD?RW drive in my laptop.  But when this goes upstream, lots of
*other* people are going to get hit by the exact same thing and think there's
something actually *wrong* with their hardware.)


pgpOEeHllsO1p.pgp
Description: PGP signature


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-10 Thread Valdis . Kletnieks
On Thu, 09 Aug 2007 14:24:16 PDT, Kristen Carlson Accardi said:
 +++ 2.6-git/drivers/ata/libata-scsi.c
 @@ -2904,6 +2976,52 @@ void ata_scsi_simulate(struct ata_device

 + if ((dev-horkage  ATA_HORKAGE_IPM) ||
 + !(dev-flags  ATA_DFLAG_IPM)) {
 + ata_dev_printk(dev, KERN_ERR,
 + Unable to set Link PM policy\n);
 + ap-pm_policy = MAX_PERFORMANCE;
 + }

KERN_INFO please, or KERN_WARNING at the highest, at least until such time
as enough drivers support enough hardware that it really *does* qualify for
this should not fail status.

(OK, so I'm just cranky because I'm tired of seeing a KERN_ERR thrown at every
reboot, just because the ata_piix driver doesn't know how to set this stuff up
for the DVD?RW drive in my laptop.  But when this goes upstream, lots of
*other* people are going to get hit by the exact same thing and think there's
something actually *wrong* with their hardware.)


pgpOEeHllsO1p.pgp
Description: PGP signature


[patch 3/4] Enable link power management for ata drivers

2007-08-09 Thread Kristen Carlson Accardi
libata drivers can define a function (enable_pm) that will
perform hardware specific actions to enable whatever power
management policy the user set up from the scsi sysfs 
interface if the driver supports it.  This power management 
policy will be activated after all disks have been 
enumerated and intialized.  Drivers should also define
disable_pm, which will turn off link power management, but
not change link power management policy.

Signed-off-by:  Kristen Carlson Accardi <[EMAIL PROTECTED]>
Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -110,6 +110,78 @@ static struct scsi_transport_template at
.user_scan  = ata_scsi_user_scan,
 };
 
+static const struct {
+   enum link_pmvalue;
+   char*name;
+} link_pm_policy[] = {
+   { NOT_AVAILABLE, "max_performance" },
+   { MIN_POWER, "min_power" },
+   { MAX_PERFORMANCE, "max_performance" },
+   { MEDIUM_POWER, "medium_power" },
+};
+
+const char *ata_scsi_link_pm_policy(enum link_pm policy)
+{
+   int i;
+   char *name = NULL;
+
+   for (i = 0; i < ARRAY_SIZE(link_pm_policy); i++) {
+   if (link_pm_policy[i].value == policy) {
+   name = link_pm_policy[i].name;
+   break;
+   }
+   }
+   return name;
+}
+
+static ssize_t store_link_pm_policy(struct class_device *class_dev,
+   const char *buf, size_t count)
+{
+   struct Scsi_Host *shost = class_to_shost(class_dev);
+   struct ata_port *ap = ata_shost_to_port(shost);
+   enum link_pm policy = 0;
+   int i;
+
+   /*
+* we are skipping array location 0 on purpose - this
+* is because a value of NOT_AVAILABLE is displayed
+* to the user as max_performance, but when the user
+* writes "max_performance", they actually want the
+* value to match MAX_PERFORMANCE.
+*/
+   for (i = 1; i < ARRAY_SIZE(link_pm_policy); i++) {
+   const int len = strlen(link_pm_policy[i].name);
+   if (strncmp(link_pm_policy[i].name, buf, len) == 0 &&
+  buf[len] == '\n') {
+   policy = link_pm_policy[i].value;
+   break;
+   }
+   }
+   if (!policy)
+   return -EINVAL;
+
+   if (ata_scsi_set_link_pm_policy(ap, policy))
+   return -EINVAL;
+   return count;
+}
+
+static ssize_t
+show_link_pm_policy(struct class_device *class_dev, char *buf)
+{
+   struct Scsi_Host *shost = class_to_shost(class_dev);
+   struct ata_port *ap = ata_shost_to_port(shost);
+   const char *policy =
+   ata_scsi_link_pm_policy(ap->pm_policy);
+
+   if (!policy)
+   return -EINVAL;
+
+   return snprintf(buf, 23, "%s\n", policy);
+}
+CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
+   show_link_pm_policy, store_link_pm_policy);
+EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
   void (*done)(struct scsi_cmnd *))
 {
@@ -2904,6 +2976,52 @@ void ata_scsi_simulate(struct ata_device
}
 }
 
+int ata_scsi_set_link_pm_policy(struct ata_port *ap,
+   enum link_pm policy)
+{
+   int rc = -EINVAL;
+   int i;
+
+   /*
+* make sure no broken devices are on this port,
+* and that all devices support interface power
+* management
+*/
+   for (i = 0; i < ATA_MAX_DEVICES; i++) {
+   struct ata_device *dev = >device[i];
+
+   /* only check drives which exist */
+   if (!ata_dev_enabled(dev))
+   continue;
+
+   /*
+* do we need to handle the case where we've hotplugged
+* a broken drive (since hotplug and ALPM are mutually
+* exclusive) ?
+*
+* If so, if we detect a broken drive on a port with
+* alpm already enabled, then we should reset the policy
+* to off for the entire port.
+*/
+   if ((dev->horkage & ATA_HORKAGE_IPM) ||
+   !(dev->flags & ATA_DFLAG_IPM)) {
+   ata_dev_printk(dev, KERN_ERR,
+   "Unable to set Link PM policy\n");
+   ap->pm_policy = MAX_PERFORMANCE;
+   }
+   }
+
+   if (ap->ops->enable_pm) {
+   ap->pm_policy = policy;
+   ap->eh_info.action |= ATA_EHI_LPM;
+   ap->eh_info.flags |= ATA_EHI_NO_AUTOPSY;
+   ata_port_schedule_eh(ap);
+   rc = 0;
+   }
+   return rc;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
+
 int 

[patch 3/4] Enable link power management for ata drivers

2007-08-09 Thread Kristen Carlson Accardi
libata drivers can define a function (enable_pm) that will
perform hardware specific actions to enable whatever power
management policy the user set up from the scsi sysfs 
interface if the driver supports it.  This power management 
policy will be activated after all disks have been 
enumerated and intialized.  Drivers should also define
disable_pm, which will turn off link power management, but
not change link power management policy.

Signed-off-by:  Kristen Carlson Accardi [EMAIL PROTECTED]
Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -110,6 +110,78 @@ static struct scsi_transport_template at
.user_scan  = ata_scsi_user_scan,
 };
 
+static const struct {
+   enum link_pmvalue;
+   char*name;
+} link_pm_policy[] = {
+   { NOT_AVAILABLE, max_performance },
+   { MIN_POWER, min_power },
+   { MAX_PERFORMANCE, max_performance },
+   { MEDIUM_POWER, medium_power },
+};
+
+const char *ata_scsi_link_pm_policy(enum link_pm policy)
+{
+   int i;
+   char *name = NULL;
+
+   for (i = 0; i  ARRAY_SIZE(link_pm_policy); i++) {
+   if (link_pm_policy[i].value == policy) {
+   name = link_pm_policy[i].name;
+   break;
+   }
+   }
+   return name;
+}
+
+static ssize_t store_link_pm_policy(struct class_device *class_dev,
+   const char *buf, size_t count)
+{
+   struct Scsi_Host *shost = class_to_shost(class_dev);
+   struct ata_port *ap = ata_shost_to_port(shost);
+   enum link_pm policy = 0;
+   int i;
+
+   /*
+* we are skipping array location 0 on purpose - this
+* is because a value of NOT_AVAILABLE is displayed
+* to the user as max_performance, but when the user
+* writes max_performance, they actually want the
+* value to match MAX_PERFORMANCE.
+*/
+   for (i = 1; i  ARRAY_SIZE(link_pm_policy); i++) {
+   const int len = strlen(link_pm_policy[i].name);
+   if (strncmp(link_pm_policy[i].name, buf, len) == 0 
+  buf[len] == '\n') {
+   policy = link_pm_policy[i].value;
+   break;
+   }
+   }
+   if (!policy)
+   return -EINVAL;
+
+   if (ata_scsi_set_link_pm_policy(ap, policy))
+   return -EINVAL;
+   return count;
+}
+
+static ssize_t
+show_link_pm_policy(struct class_device *class_dev, char *buf)
+{
+   struct Scsi_Host *shost = class_to_shost(class_dev);
+   struct ata_port *ap = ata_shost_to_port(shost);
+   const char *policy =
+   ata_scsi_link_pm_policy(ap-pm_policy);
+
+   if (!policy)
+   return -EINVAL;
+
+   return snprintf(buf, 23, %s\n, policy);
+}
+CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR,
+   show_link_pm_policy, store_link_pm_policy);
+EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy);
+
 static void ata_scsi_invalid_field(struct scsi_cmnd *cmd,
   void (*done)(struct scsi_cmnd *))
 {
@@ -2904,6 +2976,52 @@ void ata_scsi_simulate(struct ata_device
}
 }
 
+int ata_scsi_set_link_pm_policy(struct ata_port *ap,
+   enum link_pm policy)
+{
+   int rc = -EINVAL;
+   int i;
+
+   /*
+* make sure no broken devices are on this port,
+* and that all devices support interface power
+* management
+*/
+   for (i = 0; i  ATA_MAX_DEVICES; i++) {
+   struct ata_device *dev = ap-device[i];
+
+   /* only check drives which exist */
+   if (!ata_dev_enabled(dev))
+   continue;
+
+   /*
+* do we need to handle the case where we've hotplugged
+* a broken drive (since hotplug and ALPM are mutually
+* exclusive) ?
+*
+* If so, if we detect a broken drive on a port with
+* alpm already enabled, then we should reset the policy
+* to off for the entire port.
+*/
+   if ((dev-horkage  ATA_HORKAGE_IPM) ||
+   !(dev-flags  ATA_DFLAG_IPM)) {
+   ata_dev_printk(dev, KERN_ERR,
+   Unable to set Link PM policy\n);
+   ap-pm_policy = MAX_PERFORMANCE;
+   }
+   }
+
+   if (ap-ops-enable_pm) {
+   ap-pm_policy = policy;
+   ap-eh_info.action |= ATA_EHI_LPM;
+   ap-eh_info.flags |= ATA_EHI_NO_AUTOPSY;
+   ata_port_schedule_eh(ap);
+   rc = 0;
+   }
+   return rc;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
+
 int ata_scsi_add_hosts(struct ata_host *host, struct 

Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Tejun Heo
Kristen Carlson Accardi wrote:
> On Wed, 01 Aug 2007 17:27:39 +0900
> Tejun Heo <[EMAIL PROTECTED]> wrote:
> 
>> Kristen Carlson Accardi wrote:
> 
>> Is it safe to use ALPM on a device which only claims to support DIPM?
> 
> Yes - I doubled checked this with the AHCI people - and of course you
> have Edvin's testing to prove it does fine.

Alright.

> As far as moving the enable/disable_pm calls to EH - can you take
> a look at the other patch I sent which implements the shost_attrs
> to see if I still need to do this?  I really don't know much about
> the EH stuff - can you explain why we need to use it to set the
> link pm?

Unfortunately, yes, you still need to.  Only two threads of execution
(one is not a real thread tho) are allowed to access a libata port -
command execution and EH, and the two are mutually exclusive.  Invoking
something from the outside is done by doing the following.

1. recording what to do in ehi->[dev_]action, ap->pflags or dev->flags

2. schedule EH by calling either ata_port_schedule_eh(),
ata_port_abort() or ata_port_freeze().  The first one waits for the
currently in-flight commands to finish before entering EH.  The second
one aborts all in-flight commands and enters EH.  The third one aborts
all commands and freezes the port and enters EH.

3. wait for EH to finish by calling ata_port_wait_eh().

This achieves correct synchronization and other EH functionalities can
be easily used.  e.g. Resuming requires resetting the bus and
revalidating the attached devices, so the resume handler can just
request such actions together.  For link PS, I think it would probably
be a good idea to revalidate after mode change to make sure the device
works in the new mode.  If revalidation fails, it can reset and back off.

EH is done in three large steps - autopsy, report and recover.  To
implement an action, the 'recover' stage needs to be extended.  It
basically comes down to hooking the enable/disable functions into the
right places in ata_eh_recover().  Unconditionally disabling link PS
prior to reset and enabling it back again before revalidation would be a
pretty good choice, but haven't thought about it too hard so take it
with a grain of salt.

I'm not sure whether it would be necessary now but it would be nice to
have a proper recovery logic later.  e.g. If more than certain number of
ATA bus occurs in certain mount of time, disable link PS.  This kind of
logic is used during autopsy to determine whether stepping down
link/transfer speed is needed.  Please take a look at
ata_eh_speed_down().  It might be enough to piggy back on
ata_eh_speed_down() tho such that the first step of speeding down is
turning off link PS.

Hope the brief introduction to libata-EH-hacking helps.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Kristen Carlson Accardi
On Wed, 01 Aug 2007 17:27:39 +0900
Tejun Heo <[EMAIL PROTECTED]> wrote:

> Kristen Carlson Accardi wrote:

> Is it safe to use ALPM on a device which only claims to support DIPM?

Yes - I doubled checked this with the AHCI people - and of course you
have Edvin's testing to prove it does fine.

> I think this should be ATA_HORKAGE_IPM.

OK - I changed it.

> > @@ -321,6 +321,8 @@ struct ata_taskfile {
> >   ((u64) (id)[(n) + 0]) )
> >  
> >  #define ata_id_cdb_intr(id)(((id)[0] & 0x60) == 0x20)
> > +#define ata_id_has_hipm(id)((id)[76] & (1 << 9))
> > +#define ata_id_has_dipm(id)((id)[78] & (1 << 3))
> 
> We probably need !0x test here.

Thanks, I fixed that too.

As far as moving the enable/disable_pm calls to EH - can you take
a look at the other patch I sent which implements the shost_attrs
to see if I still need to do this?  I really don't know much about
the EH stuff - can you explain why we need to use it to set the
link pm?

thanks for your review, 
Kristen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread edwintorok
On 8/1/07, Tejun Heo <[EMAIL PROTECTED]> wrote:
> >
> > +   if (ata_id_has_hipm(dev->id) || ata_id_has_dipm(dev->id))
> > +   dev->flags |= ATA_DFLAG_IPM;
>
> Is it safe to use ALPM on a device which only claims to support DIPM?

I have tested on a Dell Inspiron 6400, it has ICH7-M (82801GBM) chipset.
The harddisk claims to support only DIPM, and the ALPM patches work with it.
Kristen said either DIPM or HIPM will work with ALPM accord to the
spec, see this email:
http://www.bughost.org/pipermail/power/2007-June/000691.html

--Edwin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Tejun Heo
Kristen Carlson Accardi wrote:
> libata drivers can define a function (enable_pm) that will
> perform hardware specific actions to enable whatever power
> management policy the user set up from the scsi sysfs 
> interface if the driver supports it.  This power management 
> policy will be activated after all disks have been 
> enumerated and intialized.  Drivers should also define
> disable_pm, which will turn off link power management, but
> not change link power management policy.
> 
> Signed-off-by:  Kristen Carlson Accardi <[EMAIL PROTECTED]>

Please update the patch against the current libata-dev#upstream and
there are several lines where tab follows space and git-applymbox isn't
happy about it.  Those lines are in other patches too.

> +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost,
> + enum scsi_host_link_pm policy)
> +{
> + struct ata_port *ap = ata_shost_to_port(shost);
> + int rc = -EINVAL;
> + int i;
> +
> + /*
> +  * make sure no broken devices are on this port,
> +  * and that all devices support interface power
> +  * management
> +  */
> + for (i = 0; i < ATA_MAX_DEVICES; i++) {
> + struct ata_device *dev = >device[i];
> +
> + /* only check drives which exist */
> + if (!ata_dev_enabled(dev))
> + continue;
> +
> + /*
> +  * do we need to handle the case where we've hotplugged
> +  * a broken drive (since hotplug and ALPM are mutually
> +  * exclusive) ?
> +  *
> +  * If so, if we detect a broken drive on a port with
> +  * alpm already enabled, then we should reset the policy
> +  * to off for the entire port.
> +  */
> + if ((dev->horkage & ATA_HORKAGE_ALPM) ||
> + !(dev->flags & ATA_DFLAG_IPM)) {
> + ata_dev_printk(dev, KERN_ERR,
> + "Unable to set Link PM policy\n");
> + ap->pm_policy = SHOST_MAX_PERFORMANCE;
> + }
> + }
> +
> + if (ap->ops->enable_pm)
> + rc = ap->ops->enable_pm(ap, policy);
> +
> + if (!rc)
> + shost->shost_link_pm_policy = ap->pm_policy;
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);

This function is directly called from SCSI sysfs write, right?  You
can't do this without synchronizing with EH.  The best way is to define
an EH action and ask EH to transit between powersave states.

> @@ -2021,6 +2021,9 @@ int ata_dev_configure(struct ata_device 
>   if (dev->flags & ATA_DFLAG_LBA48)
>   dev->max_sectors = ATA_MAX_SECTORS_LBA48;
>  
> + if (ata_id_has_hipm(dev->id) || ata_id_has_dipm(dev->id))
> + dev->flags |= ATA_DFLAG_IPM;

Is it safe to use ALPM on a device which only claims to support DIPM?

> @@ -2046,6 +2049,13 @@ int ata_dev_configure(struct ata_device 
>   dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
>dev->max_sectors);
>  
> + if (ata_device_blacklisted(dev) & ATA_HORKAGE_ALPM) {
> + dev->horkage |= ATA_HORKAGE_ALPM;

I think this should be ATA_HORKAGE_IPM.

> @@ -6385,6 +6426,8 @@ int ata_host_register(struct ata_host *h
>   struct ata_port *ap = host->ports[i];
>  
>   ata_scsi_scan_host(ap);
> + ata_scsi_set_link_pm_policy(ap->scsi_host,
> + ap->pm_policy);

Again, this should be done from EH.

> @@ -321,6 +321,8 @@ struct ata_taskfile {
> ((u64) (id)[(n) + 0]) )
>  
>  #define ata_id_cdb_intr(id)  (((id)[0] & 0x60) == 0x20)
> +#define ata_id_has_hipm(id)  ((id)[76] & (1 << 9))
> +#define ata_id_has_dipm(id)  ((id)[78] & (1 << 3))

We probably need !0x test here.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Tejun Heo
Kristen Carlson Accardi wrote:
 libata drivers can define a function (enable_pm) that will
 perform hardware specific actions to enable whatever power
 management policy the user set up from the scsi sysfs 
 interface if the driver supports it.  This power management 
 policy will be activated after all disks have been 
 enumerated and intialized.  Drivers should also define
 disable_pm, which will turn off link power management, but
 not change link power management policy.
 
 Signed-off-by:  Kristen Carlson Accardi [EMAIL PROTECTED]

Please update the patch against the current libata-dev#upstream and
there are several lines where tab follows space and git-applymbox isn't
happy about it.  Those lines are in other patches too.

 +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost,
 + enum scsi_host_link_pm policy)
 +{
 + struct ata_port *ap = ata_shost_to_port(shost);
 + int rc = -EINVAL;
 + int i;
 +
 + /*
 +  * make sure no broken devices are on this port,
 +  * and that all devices support interface power
 +  * management
 +  */
 + for (i = 0; i  ATA_MAX_DEVICES; i++) {
 + struct ata_device *dev = ap-device[i];
 +
 + /* only check drives which exist */
 + if (!ata_dev_enabled(dev))
 + continue;
 +
 + /*
 +  * do we need to handle the case where we've hotplugged
 +  * a broken drive (since hotplug and ALPM are mutually
 +  * exclusive) ?
 +  *
 +  * If so, if we detect a broken drive on a port with
 +  * alpm already enabled, then we should reset the policy
 +  * to off for the entire port.
 +  */
 + if ((dev-horkage  ATA_HORKAGE_ALPM) ||
 + !(dev-flags  ATA_DFLAG_IPM)) {
 + ata_dev_printk(dev, KERN_ERR,
 + Unable to set Link PM policy\n);
 + ap-pm_policy = SHOST_MAX_PERFORMANCE;
 + }
 + }
 +
 + if (ap-ops-enable_pm)
 + rc = ap-ops-enable_pm(ap, policy);
 +
 + if (!rc)
 + shost-shost_link_pm_policy = ap-pm_policy;
 + return rc;
 +}
 +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);

This function is directly called from SCSI sysfs write, right?  You
can't do this without synchronizing with EH.  The best way is to define
an EH action and ask EH to transit between powersave states.

 @@ -2021,6 +2021,9 @@ int ata_dev_configure(struct ata_device 
   if (dev-flags  ATA_DFLAG_LBA48)
   dev-max_sectors = ATA_MAX_SECTORS_LBA48;
  
 + if (ata_id_has_hipm(dev-id) || ata_id_has_dipm(dev-id))
 + dev-flags |= ATA_DFLAG_IPM;

Is it safe to use ALPM on a device which only claims to support DIPM?

 @@ -2046,6 +2049,13 @@ int ata_dev_configure(struct ata_device 
   dev-max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
dev-max_sectors);
  
 + if (ata_device_blacklisted(dev)  ATA_HORKAGE_ALPM) {
 + dev-horkage |= ATA_HORKAGE_ALPM;

I think this should be ATA_HORKAGE_IPM.

 @@ -6385,6 +6426,8 @@ int ata_host_register(struct ata_host *h
   struct ata_port *ap = host-ports[i];
  
   ata_scsi_scan_host(ap);
 + ata_scsi_set_link_pm_policy(ap-scsi_host,
 + ap-pm_policy);

Again, this should be done from EH.

 @@ -321,6 +321,8 @@ struct ata_taskfile {
 ((u64) (id)[(n) + 0]) )
  
  #define ata_id_cdb_intr(id)  (((id)[0]  0x60) == 0x20)
 +#define ata_id_has_hipm(id)  ((id)[76]  (1  9))
 +#define ata_id_has_dipm(id)  ((id)[78]  (1  3))

We probably need !0x test here.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread edwintorok
On 8/1/07, Tejun Heo [EMAIL PROTECTED] wrote:
 
  +   if (ata_id_has_hipm(dev-id) || ata_id_has_dipm(dev-id))
  +   dev-flags |= ATA_DFLAG_IPM;

 Is it safe to use ALPM on a device which only claims to support DIPM?

I have tested on a Dell Inspiron 6400, it has ICH7-M (82801GBM) chipset.
The harddisk claims to support only DIPM, and the ALPM patches work with it.
Kristen said either DIPM or HIPM will work with ALPM accord to the
spec, see this email:
http://www.bughost.org/pipermail/power/2007-June/000691.html

--Edwin
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Kristen Carlson Accardi
On Wed, 01 Aug 2007 17:27:39 +0900
Tejun Heo [EMAIL PROTECTED] wrote:

 Kristen Carlson Accardi wrote:
snippy
 Is it safe to use ALPM on a device which only claims to support DIPM?

Yes - I doubled checked this with the AHCI people - and of course you
have Edvin's testing to prove it does fine.

 I think this should be ATA_HORKAGE_IPM.

OK - I changed it.

  @@ -321,6 +321,8 @@ struct ata_taskfile {
((u64) (id)[(n) + 0]) )
   
   #define ata_id_cdb_intr(id)(((id)[0]  0x60) == 0x20)
  +#define ata_id_has_hipm(id)((id)[76]  (1  9))
  +#define ata_id_has_dipm(id)((id)[78]  (1  3))
 
 We probably need !0x test here.

Thanks, I fixed that too.

As far as moving the enable/disable_pm calls to EH - can you take
a look at the other patch I sent which implements the shost_attrs
to see if I still need to do this?  I really don't know much about
the EH stuff - can you explain why we need to use it to set the
link pm?

thanks for your review, 
Kristen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-08-01 Thread Tejun Heo
Kristen Carlson Accardi wrote:
 On Wed, 01 Aug 2007 17:27:39 +0900
 Tejun Heo [EMAIL PROTECTED] wrote:
 
 Kristen Carlson Accardi wrote:
 snippy
 Is it safe to use ALPM on a device which only claims to support DIPM?
 
 Yes - I doubled checked this with the AHCI people - and of course you
 have Edvin's testing to prove it does fine.

Alright.

 As far as moving the enable/disable_pm calls to EH - can you take
 a look at the other patch I sent which implements the shost_attrs
 to see if I still need to do this?  I really don't know much about
 the EH stuff - can you explain why we need to use it to set the
 link pm?

Unfortunately, yes, you still need to.  Only two threads of execution
(one is not a real thread tho) are allowed to access a libata port -
command execution and EH, and the two are mutually exclusive.  Invoking
something from the outside is done by doing the following.

1. recording what to do in ehi-[dev_]action, ap-pflags or dev-flags

2. schedule EH by calling either ata_port_schedule_eh(),
ata_port_abort() or ata_port_freeze().  The first one waits for the
currently in-flight commands to finish before entering EH.  The second
one aborts all in-flight commands and enters EH.  The third one aborts
all commands and freezes the port and enters EH.

3. wait for EH to finish by calling ata_port_wait_eh().

This achieves correct synchronization and other EH functionalities can
be easily used.  e.g. Resuming requires resetting the bus and
revalidating the attached devices, so the resume handler can just
request such actions together.  For link PS, I think it would probably
be a good idea to revalidate after mode change to make sure the device
works in the new mode.  If revalidation fails, it can reset and back off.

EH is done in three large steps - autopsy, report and recover.  To
implement an action, the 'recover' stage needs to be extended.  It
basically comes down to hooking the enable/disable functions into the
right places in ata_eh_recover().  Unconditionally disabling link PS
prior to reset and enabling it back again before revalidation would be a
pretty good choice, but haven't thought about it too hard so take it
with a grain of salt.

I'm not sure whether it would be necessary now but it would be nice to
have a proper recovery logic later.  e.g. If more than certain number of
ATA bus occurs in certain mount of time, disable link PS.  This kind of
logic is used during autopsy to determine whether stepping down
link/transfer speed is needed.  Please take a look at
ata_eh_speed_down().  It might be enough to piggy back on
ata_eh_speed_down() tho such that the first step of speeding down is
turning off link PS.

Hope the brief introduction to libata-EH-hacking helps.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 05 Jul 2007 20:02:08 -0400
Jeff Garzik <[EMAIL PROTECTED]> wrote:

> May I assume that I may delete the patches from Kristen, and assume that 
> you will resend an updated version of her AN and ALPM patches to me?
> 

Sure.  But I have a sneaking feeling that Kristen sneaks sneaky fixes into
her patches without telling me, so I won't be 100% confident about their
uptodateness (hint).


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik
May I assume that I may delete the patches from Kristen, and assume that 
you will resend an updated version of her AN and ALPM patches to me?


Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik

Andrew Morton wrote:

I guess we can bump ATA_DFLAG_CFG_MASK up to 12, like this?


Yep

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik

Andrew Morton wrote:

On Thu, 5 Jul 2007 13:05:30 -0700
Kristen Carlson Accardi <[EMAIL PROTECTED]> wrote:


+   ATA_DFLAG_IPM   = (1 << 6), /* device supports interface PM */
ATA_DFLAG_CFG_MASK  = (1 << 8) - 1,


I had to bump this to (1<<7), so we've run out.


You can shuffle the numbers a bit, as long as the masks (*_MASK) stay 
correct for their purpose


Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 5 Jul 2007 15:33:34 -0700
Andrew Morton <[EMAIL PROTECTED]> wrote:

> On Thu, 5 Jul 2007 13:05:30 -0700
> Kristen Carlson Accardi <[EMAIL PROTECTED]> wrote:
> 
> > +   ATA_DFLAG_IPM   = (1 << 6), /* device supports interface PM */
> > ATA_DFLAG_CFG_MASK  = (1 << 8) - 1,
> 
> I had to bump this to (1<<7), so we've run out.

err, no, we've more than run out because you AN patches took the last one.


I guess we can bump ATA_DFLAG_CFG_MASK up to 12, like this?

--- 
a/include/linux/libata.h~ata-ahci-alpm-enable-link-power-management-for-ata-drivers
+++ a/include/linux/libata.h
@@ -140,11 +140,12 @@ enum {
ATA_DFLAG_ACPI_PENDING  = (1 << 5), /* ACPI resume action pending */
ATA_DFLAG_ACPI_FAILED   = (1 << 6), /* ACPI on devcfg has failed */
ATA_DFLAG_AN= (1 << 7), /* device supports Async 
notification */
-   ATA_DFLAG_CFG_MASK  = (1 << 8) - 1,
+   ATA_DFLAG_IPM   = (1 << 8), /* device supports interface PM */
+   ATA_DFLAG_CFG_MASK  = (1 << 12) - 1,
 
-   ATA_DFLAG_PIO   = (1 << 8), /* device limited to PIO mode */
-   ATA_DFLAG_NCQ_OFF   = (1 << 9), /* device limited to non-NCQ mode */
-   ATA_DFLAG_SPUNDOWN  = (1 << 10), /* XXX: for spindown_compat */
+   ATA_DFLAG_PIO   = (1 << 12), /* device limited to PIO mode */
+   ATA_DFLAG_NCQ_OFF   = (1 << 13), /* device limited to non-NCQ mode 
*/
+   ATA_DFLAG_SPUNDOWN  = (1 << 14), /* XXX: for spindown_compat */
ATA_DFLAG_INIT_MASK = (1 << 16) - 1,
 
ATA_DFLAG_DETACH= (1 << 16),


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 5 Jul 2007 13:05:30 -0700
Kristen Carlson Accardi <[EMAIL PROTECTED]> wrote:

> + ATA_DFLAG_IPM   = (1 << 6), /* device supports interface PM */
>   ATA_DFLAG_CFG_MASK  = (1 << 8) - 1,

I had to bump this to (1<<7), so we've run out.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Kristen Carlson Accardi
libata drivers can define a function (enable_pm) that will
perform hardware specific actions to enable whatever power
management policy the user set up from the scsi sysfs 
interface if the driver supports it.  This power management 
policy will be activated after all disks have been 
enumerated and intialized.  Drivers should also define
disable_pm, which will turn off link power management, but
not change link power management policy.

Signed-off-by:  Kristen Carlson Accardi <[EMAIL PROTECTED]>
Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -2905,6 +2905,51 @@ void ata_scsi_simulate(struct ata_device
}
 }
 
+int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost,
+   enum scsi_host_link_pm policy)
+{
+   struct ata_port *ap = ata_shost_to_port(shost);
+   int rc = -EINVAL;
+   int i;
+
+   /*
+* make sure no broken devices are on this port,
+* and that all devices support interface power
+* management
+*/
+   for (i = 0; i < ATA_MAX_DEVICES; i++) {
+   struct ata_device *dev = >device[i];
+
+   /* only check drives which exist */
+   if (!ata_dev_enabled(dev))
+   continue;
+
+   /*
+* do we need to handle the case where we've hotplugged
+* a broken drive (since hotplug and ALPM are mutually
+* exclusive) ?
+*
+* If so, if we detect a broken drive on a port with
+* alpm already enabled, then we should reset the policy
+* to off for the entire port.
+*/
+   if ((dev->horkage & ATA_HORKAGE_ALPM) ||
+   !(dev->flags & ATA_DFLAG_IPM)) {
+   ata_dev_printk(dev, KERN_ERR,
+   "Unable to set Link PM policy\n");
+   ap->pm_policy = SHOST_MAX_PERFORMANCE;
+   }
+   }
+
+   if (ap->ops->enable_pm)
+   rc = ap->ops->enable_pm(ap, policy);
+
+   if (!rc)
+   shost->shost_link_pm_policy = ap->pm_policy;
+   return rc;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
+
 int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 {
int i, rc;
@@ -2927,7 +2972,7 @@ int ata_scsi_add_hosts(struct ata_host *
shost->max_lun = 1;
shost->max_channel = 1;
shost->max_cmd_len = 16;
-
+   shost->shost_link_pm_policy = ap->pm_policy;
rc = scsi_add_host(ap->scsi_host, ap->host->dev);
if (rc)
goto err_add;
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1 << 2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1 << 3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1 << 4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_IPM   = (1 << 6), /* device supports interface PM */
ATA_DFLAG_CFG_MASK  = (1 << 8) - 1,
 
ATA_DFLAG_PIO   = (1 << 8), /* device limited to PIO mode */
@@ -298,6 +299,7 @@ enum {
ATA_HORKAGE_NODMA   = (1 << 1), /* DMA problems */
ATA_HORKAGE_NONCQ   = (1 << 2), /* Don't use NCQ */
ATA_HORKAGE_MAX_SEC_128 = (1 << 3), /* Limit max sects to 128 */
+   ATA_HORKAGE_ALPM= (1 << 4), /* ALPM problems */
 };
 
 enum hsm_task_states {
@@ -546,6 +548,7 @@ struct ata_port {
 
pm_message_tpm_mesg;
int *pm_result;
+   enum scsi_host_link_pm  pm_policy;
 
void*private_data;
 
@@ -605,7 +608,8 @@ struct ata_port_operations {
 
int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
int (*port_resume) (struct ata_port *ap);
-
+   int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy);
+   int (*disable_pm) (struct ata_port *ap);
int (*port_start) (struct ata_port *ap);
void (*port_stop) (struct ata_port *ap);
 
@@ -811,7 +815,7 @@ extern int ata_cable_40wire(struct ata_p
 extern int ata_cable_80wire(struct ata_port *ap);
 extern int ata_cable_sata(struct ata_port *ap);
 extern int ata_cable_unknown(struct ata_port *ap);
-
+extern int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, enum 
scsi_host_link_pm);
 /*
  * Timing helpers
  */
Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -2021,6 +2021,9 @@ int 

Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 05 Jul 2007 20:02:08 -0400
Jeff Garzik [EMAIL PROTECTED] wrote:

 May I assume that I may delete the patches from Kristen, and assume that 
 you will resend an updated version of her AN and ALPM patches to me?
 

Sure.  But I have a sneaking feeling that Kristen sneaks sneaky fixes into
her patches without telling me, so I won't be 100% confident about their
uptodateness (hint).


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik

Andrew Morton wrote:

I guess we can bump ATA_DFLAG_CFG_MASK up to 12, like this?


Yep

Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik

Andrew Morton wrote:

On Thu, 5 Jul 2007 13:05:30 -0700
Kristen Carlson Accardi [EMAIL PROTECTED] wrote:


+   ATA_DFLAG_IPM   = (1  6), /* device supports interface PM */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,


I had to bump this to (17), so we've run out.


You can shuffle the numbers a bit, as long as the masks (*_MASK) stay 
correct for their purpose


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Jeff Garzik
May I assume that I may delete the patches from Kristen, and assume that 
you will resend an updated version of her AN and ALPM patches to me?


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Kristen Carlson Accardi
libata drivers can define a function (enable_pm) that will
perform hardware specific actions to enable whatever power
management policy the user set up from the scsi sysfs 
interface if the driver supports it.  This power management 
policy will be activated after all disks have been 
enumerated and intialized.  Drivers should also define
disable_pm, which will turn off link power management, but
not change link power management policy.

Signed-off-by:  Kristen Carlson Accardi [EMAIL PROTECTED]
Index: 2.6-git/drivers/ata/libata-scsi.c
===
--- 2.6-git.orig/drivers/ata/libata-scsi.c
+++ 2.6-git/drivers/ata/libata-scsi.c
@@ -2905,6 +2905,51 @@ void ata_scsi_simulate(struct ata_device
}
 }
 
+int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost,
+   enum scsi_host_link_pm policy)
+{
+   struct ata_port *ap = ata_shost_to_port(shost);
+   int rc = -EINVAL;
+   int i;
+
+   /*
+* make sure no broken devices are on this port,
+* and that all devices support interface power
+* management
+*/
+   for (i = 0; i  ATA_MAX_DEVICES; i++) {
+   struct ata_device *dev = ap-device[i];
+
+   /* only check drives which exist */
+   if (!ata_dev_enabled(dev))
+   continue;
+
+   /*
+* do we need to handle the case where we've hotplugged
+* a broken drive (since hotplug and ALPM are mutually
+* exclusive) ?
+*
+* If so, if we detect a broken drive on a port with
+* alpm already enabled, then we should reset the policy
+* to off for the entire port.
+*/
+   if ((dev-horkage  ATA_HORKAGE_ALPM) ||
+   !(dev-flags  ATA_DFLAG_IPM)) {
+   ata_dev_printk(dev, KERN_ERR,
+   Unable to set Link PM policy\n);
+   ap-pm_policy = SHOST_MAX_PERFORMANCE;
+   }
+   }
+
+   if (ap-ops-enable_pm)
+   rc = ap-ops-enable_pm(ap, policy);
+
+   if (!rc)
+   shost-shost_link_pm_policy = ap-pm_policy;
+   return rc;
+}
+EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy);
+
 int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 {
int i, rc;
@@ -2927,7 +2972,7 @@ int ata_scsi_add_hosts(struct ata_host *
shost-max_lun = 1;
shost-max_channel = 1;
shost-max_cmd_len = 16;
-
+   shost-shost_link_pm_policy = ap-pm_policy;
rc = scsi_add_host(ap-scsi_host, ap-host-dev);
if (rc)
goto err_add;
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_IPM   = (1  6), /* device supports interface PM */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -298,6 +299,7 @@ enum {
ATA_HORKAGE_NODMA   = (1  1), /* DMA problems */
ATA_HORKAGE_NONCQ   = (1  2), /* Don't use NCQ */
ATA_HORKAGE_MAX_SEC_128 = (1  3), /* Limit max sects to 128 */
+   ATA_HORKAGE_ALPM= (1  4), /* ALPM problems */
 };
 
 enum hsm_task_states {
@@ -546,6 +548,7 @@ struct ata_port {
 
pm_message_tpm_mesg;
int *pm_result;
+   enum scsi_host_link_pm  pm_policy;
 
void*private_data;
 
@@ -605,7 +608,8 @@ struct ata_port_operations {
 
int (*port_suspend) (struct ata_port *ap, pm_message_t mesg);
int (*port_resume) (struct ata_port *ap);
-
+   int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy);
+   int (*disable_pm) (struct ata_port *ap);
int (*port_start) (struct ata_port *ap);
void (*port_stop) (struct ata_port *ap);
 
@@ -811,7 +815,7 @@ extern int ata_cable_40wire(struct ata_p
 extern int ata_cable_80wire(struct ata_port *ap);
 extern int ata_cable_sata(struct ata_port *ap);
 extern int ata_cable_unknown(struct ata_port *ap);
-
+extern int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, enum 
scsi_host_link_pm);
 /*
  * Timing helpers
  */
Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -2021,6 +2021,9 @@ int ata_dev_configure(struct ata_device 
if 

Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 5 Jul 2007 13:05:30 -0700
Kristen Carlson Accardi [EMAIL PROTECTED] wrote:

 + ATA_DFLAG_IPM   = (1  6), /* device supports interface PM */
   ATA_DFLAG_CFG_MASK  = (1  8) - 1,

I had to bump this to (17), so we've run out.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 3/4] Enable link power management for ata drivers

2007-07-05 Thread Andrew Morton
On Thu, 5 Jul 2007 15:33:34 -0700
Andrew Morton [EMAIL PROTECTED] wrote:

 On Thu, 5 Jul 2007 13:05:30 -0700
 Kristen Carlson Accardi [EMAIL PROTECTED] wrote:
 
  +   ATA_DFLAG_IPM   = (1  6), /* device supports interface PM */
  ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
 I had to bump this to (17), so we've run out.

err, no, we've more than run out because you AN patches took the last one.


I guess we can bump ATA_DFLAG_CFG_MASK up to 12, like this?

--- 
a/include/linux/libata.h~ata-ahci-alpm-enable-link-power-management-for-ata-drivers
+++ a/include/linux/libata.h
@@ -140,11 +140,12 @@ enum {
ATA_DFLAG_ACPI_PENDING  = (1  5), /* ACPI resume action pending */
ATA_DFLAG_ACPI_FAILED   = (1  6), /* ACPI on devcfg has failed */
ATA_DFLAG_AN= (1  7), /* device supports Async 
notification */
-   ATA_DFLAG_CFG_MASK  = (1  8) - 1,
+   ATA_DFLAG_IPM   = (1  8), /* device supports interface PM */
+   ATA_DFLAG_CFG_MASK  = (1  12) - 1,
 
-   ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
-   ATA_DFLAG_NCQ_OFF   = (1  9), /* device limited to non-NCQ mode */
-   ATA_DFLAG_SPUNDOWN  = (1  10), /* XXX: for spindown_compat */
+   ATA_DFLAG_PIO   = (1  12), /* device limited to PIO mode */
+   ATA_DFLAG_NCQ_OFF   = (1  13), /* device limited to non-NCQ mode 
*/
+   ATA_DFLAG_SPUNDOWN  = (1  14), /* XXX: for spindown_compat */
ATA_DFLAG_INIT_MASK = (1  16) - 1,
 
ATA_DFLAG_DETACH= (1  16),


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/