I think I mentioned on the list a couple weeks ago that I favor just
expanding the definition of link power management to include
the notion of simply powering the entire port off rather
than adding new knobs to sysfs. I wrote this completely untested and
very incomplete patch to give you a better idea of what I am proposing.
This patch adds a new valid value of "power_off" for the existing
link_power_management_policy sysfs entry:
power_off: phy is not on at all.
min_power: driver uses minimum possible power. hotplug
may or may not be available.
medium_power: best power/performance tradeoff. hotplug
may or may not be available
max_performance: max performance without regard to power
hot plug is available.
the default value for link_power_management_policy
can be determined by the core and overriden by the
driver, or just solely determined by the driver.
min_power and medium_power are only valid values for
occupied ports. right now, using min_power or medium_power
mode means that hotplug is not available, although that's
not necessarily got to be true in the future, since we
could add polling or low power presence detect or even
use mechanical interlock switch events.
We could further add a "power_on" value to indicate that
there's no device attached but we can detect one if it
becomes available if you think that using max_performance
to indicate this state is too confusing.
---
drivers/ata/libata-core.c | 10 ++++++++++
drivers/ata/libata-scsi.c | 1 +
include/linux/libata.h | 1 +
3 files changed, 12 insertions(+)
Index: linux-ahci-phy/drivers/ata/libata-core.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/libata-core.c 2008-06-03 16:52:05.000000000 -0700
+++ linux-ahci-phy/drivers/ata/libata-core.c 2008-06-03 17:02:34.000000000 -0700
@@ -993,6 +993,16 @@ void ata_dev_enable_pm(struct ata_device
int rc = 0;
struct ata_port *ap = dev->link->ap;
+ /*
+ * if we don't have a device attached, all we can
+ * do is power off
+ */
+ if (!ata_dev_enabled(dev) && policy == POWER_OFF) {
+ ap->flags |= ATA_FLAG_NO_HOTPLUG;
+ ata_phy_offline(ap);
+ return;
+ }
+
/* set HIPM first, then DIPM */
if (ap->ops->enable_pm)
rc = ap->ops->enable_pm(ap, policy);
Index: linux-ahci-phy/drivers/ata/libata-scsi.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/libata-scsi.c 2008-06-03 16:53:17.000000000 -0700
+++ linux-ahci-phy/drivers/ata/libata-scsi.c 2008-06-03 16:53:34.000000000 -0700
@@ -122,6 +122,7 @@ static const struct {
{ MIN_POWER, "min_power" },
{ MAX_PERFORMANCE, "max_performance" },
{ MEDIUM_POWER, "medium_power" },
+ { POWER_OFF, "power_off" },
};
static const char *ata_scsi_lpm_get(enum link_pm policy)
Index: linux-ahci-phy/include/linux/libata.h
===================================================================
--- linux-ahci-phy.orig/include/linux/libata.h 2008-06-03 16:52:32.000000000 -0700
+++ linux-ahci-phy/include/linux/libata.h 2008-06-03 16:52:54.000000000 -0700
@@ -443,6 +443,7 @@ enum link_pm {
MIN_POWER,
MAX_PERFORMANCE,
MEDIUM_POWER,
+ POWER_OFF,
};
extern struct device_attribute dev_attr_link_power_management_policy;
Kristen Carlson Accardi wrote:
> I think I mentioned on the list a couple weeks ago that I favor just
> expanding the definition of link power management to include
> the notion of simply powering the entire port off rather
> than adding new knobs to sysfs. I wrote this completely untested and
> very incomplete patch to give you a better idea of what I am proposing.
> This patch adds a new valid value of "power_off" for the existing
> link_power_management_policy sysfs entry:
Looks fine to me... that would work nicely.
I think your patch is missing code to handle the transition from
power_off to <something else>, though, right?
I'm quite happy with this approach.
Jeff
On Tue, 03 Jun 2008 20:52:49 -0400
Jeff Garzik <[email protected]> wrote:
> Kristen Carlson Accardi wrote:
> > I think I mentioned on the list a couple weeks ago that I favor just
> > expanding the definition of link power management to include
> > the notion of simply powering the entire port off rather
> > than adding new knobs to sysfs. I wrote this completely untested and
> > very incomplete patch to give you a better idea of what I am proposing.
> > This patch adds a new valid value of "power_off" for the existing
> > link_power_management_policy sysfs entry:
>
> Looks fine to me... that would work nicely.
>
> I think your patch is missing code to handle the transition from
> power_off to <something else>, though, right?
>
> I'm quite happy with this approach.
>
> Jeff
>
>
>
yeah - it's missing a lot. I'll flesh it out some more and
send a real one.
Jeff Garzik wrote:
> Kristen Carlson Accardi wrote:
>> I think I mentioned on the list a couple weeks ago that I favor just
>> expanding the definition of link power management to include
>> the notion of simply powering the entire port off rather
>> than adding new knobs to sysfs. I wrote this completely untested and
>> very incomplete patch to give you a better idea of what I am proposing.
>> This patch adds a new valid value of "power_off" for the existing
>> link_power_management_policy sysfs entry:
>
> Looks fine to me... that would work nicely.
>
> I think your patch is missing code to handle the transition from
> power_off to <something else>, though, right?
>
> I'm quite happy with this approach.
I agree this is the right place to implement the control.
Thanks.
--
tejun