power off unused ports
If the port isn't either a drive bay or an external SATA port, do not

keep the phy powered on if the port is unoccupied. This saves .75 watts

on most laptops.  A module parameter can be used to override this

behavior and allow the phy's to be powered up regardless of whether it

has externally accessible SATA ports. 
Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---

 drivers/ata/ahci.c        |   21 +++++++++++++++++++++

 drivers/ata/libata-core.c |   24 ++++++++++++++++++++++++

 include/linux/libata.h    |    1 +

 3 files changed, 46 insertions(+)
Index: linux-ahci-phy/drivers/ata/ahci.c

===================================================================

--- linux-ahci-phy.orig/drivers/ata/ahci.c	2008-05-08 14:29:02.000000000 -0700

+++ linux-ahci-phy/drivers/ata/ahci.c	2008-05-08 14:31:05.000000000 -0700

@@ -53,9 +53,13 @@ static int ahci_skip_host_reset;

 module_param_named(skip_host_reset, ahci_skip_host_reset, int, 0444);

 MODULE_PARM_DESC(skip_host_reset, "skip global host reset (0=don't skip, 1=skip)");
+static int ahci_power_save = 1;

+module_param_named(power_save, ahci_power_save, int, 0444);

+MODULE_PARM_DESC(power_save, "Power off unused ports (0=don't power off, 1=power off)");

 static int ahci_enable_alpm(struct ata_port *ap,

 		enum link_pm policy);

 static void ahci_disable_alpm(struct ata_port *ap);

+static int ahci_is_hotplug_capable(struct ata_port *ap);
 enum {

 	AHCI_PCI_BAR		= 5,

@@ -166,6 +170,8 @@ enum {

 	PORT_CMD_ASP		= (1 << 27), /* Aggressive Slumber/Partial */

 	PORT_CMD_ALPE		= (1 << 26), /* Aggressive Link PM enable */

 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */

+	PORT_CMD_ESP		= (1 << 21), /* External SATA Port */

+	PORT_CMD_HPCP		= (1 << 18), /* port is hot plug capable */

 	PORT_CMD_PMP		= (1 << 17), /* PMP attached */

 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */

 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS ...
[ message continues ]

Hi,
I'm wondering what's the status of this patch?  
if this has hit a subsystem tree yet?  0.75 watts power savings sounds

really cool, but it doesn't seem to be in Linus's mainline yet.  Has

it hit a subsystem tree yet, or is there some issue that still needs

to be resolved before it can go it?
Thanks, regards,
							- Ted

--

On Mon, 26 May 2008 23:08:54 -0400
Hi Ted,

As far as I know the patch has gone nowhere.  I believe that

Jeff wanted something more flexible than the module parameter that

I provided to override the BIOS options.  I am not working on this,

I figured he had a pretty firm idea what he wanted so he was better

equipped to write the patch.
Kristen

--

Thanks Kristen,
Can you say which laptops you had tested this on where it saved power?

(Did you test any Thinkpads, in particular?)  I'm wondering if it's

worth trying to forward port your patch as a private mod to my kernel;

30 to 40 minutes of extra battery life is nothing to sneeze at!
						- Ted

--

On Tue, 27 May 2008 18:59:26 -0400
I tested this on an Intel mobile software development platform

with a newer mobile ICH - the power savings were measured at the actual

component (via probes on the ICH), so I did not measure the power

savings at the wall socket, although I would expect the power savings

to be even greater on the other side of the power supply.  So in short,

yes, I think it's worth it to give it a try - the patch is pretty

unintrusive, so it should be that difficult a port to do.
--

Can you repost the patch? I believe we should push it and only add

complex enable/disable functionality if someone needs it...
--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--

If you are talking about SATA -- incorrect.
The patch deals with policy, and the user MUST have the ability to

control this stuff.  Otherwise you create a situation where the user

might be denied hotplug use in valid cases, or similar negative situations.
	Jeff
--

The policy isn't however complicated. Tejun added the stuff for forcing

cable type and mode on setup and has therefore written all the per device

setup code we might need. Alternatively a single
	foo=1/0
option has been fine for acpi and will do fine for this. Total additional

cost - 1 line.
Alan

--

The key requirement is per-port control.  Ideally via hdparm or another

userspace tool, but kernel command line (module options) or sysfs would

be just fine too.  And agreed, the minimal you need is simply 1/0 for

the port's policy.
	Jeff
--

..
Btw.. hdparm-8.7 (unreleased) can grok /sys now, so that interface is

as good as any from a userspace viewpoint now.
For the power-off of unused ports, the current patch still sounds

extremely vendor-specific (Intel).
Does it actually work (demonstrate, please) on any other hardware ?
I would still like to see a far more generic solution, with periodic polling

and the like, which would permit use on *any* machines (eg. data centers)

without loss of hotplug capability on those ports.
But that's probably just wishful thinking at this point.
Cheers

--

On Mon, 02 Jun 2008 09:03:04 -0400
Wrong - this patch is implemented according to the AHCI spec and
If someone would like to test it on another AHCI compliant chipset

that would be great.  I have none.

--

ITHM AHCI-specific, and not usable by non-AHCI SATA controllers -- which

we would want in any solution.
	Jeff
--

Ding...  correct.  We need something per-port that's not Intel specific.
	Jeff
--

On Mon, 02 Jun 2008 12:07:02 -0400
Can you tell me what part of this patch is vendor specific?  Maybe

I am misunderstanding what you are saying.  Here's that patch again

in case you need to look at it again.
power off unused ports
If the port isn't either a drive bay or an external SATA port, do not

keep the phy powered on if the port is unoccupied. This saves .75 watts

on most laptops.  A module parameter can be used to override this

behavior and allow the phy's to be powered up regardless of whether it

has externally accessible SATA ports. 
Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---

 drivers/ata/ahci.c        |   21 +++++++++++++++++++++

 drivers/ata/libata-core.c |   24 ++++++++++++++++++++++++

 include/linux/libata.h    |    1 +

 3 files changed, 46 insertions(+)
Index: linux-ahci-phy/drivers/ata/ahci.c

===================================================================

--- linux-ahci-phy.orig/drivers/ata/ahci.c	2008-05-08 14:29:02.000000000 -0700

+++ linux-ahci-phy/drivers/ata/ahci.c	2008-05-08 14:31:05.000000000 -0700

@@ -53,9 +53,13 @@ static int ahci_skip_host_reset;

 module_param_named(skip_host_reset, ahci_skip_host_reset, int, 0444);

 MODULE_PARM_DESC(skip_host_reset, "skip global host reset (0=don't skip, 1=skip)");
+static int ahci_power_save = 1;

+module_param_named(power_save, ahci_power_save, int, 0444);

+MODULE_PARM_DESC(power_save, "Power off unused ports (0=don't power off, 1=power off)");

 static int ahci_enable_alpm(struct ata_port *ap,

 		enum link_pm policy);

 static void ahci_disable_alpm(struct ata_port *ap);

+static int ahci_is_hotplug_capable(struct ata_port *ap);
 enum {

 	AHCI_PCI_BAR		= 5,

@@ -166,6 +170,8 @@ enum {

 	PORT_CMD_ASP		= (1 << 27), /* Aggressive Slumber/Partial */

 	PORT_CMD_ALPE		= (1 << 26), /* Aggressive Link PM enable */

 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */

+	PORT_CMD_ESP		= (1 << 21), /* External SATA Port */

+	PORT_CMD_HPCP		= (1 &lt...
[ message continues ]

It's specific to AHCI, and does not cover other controllers.
Certainly PORT_CMD_HPCP must be, but the concept itself is not at all

AHCI-specific.
	Jeff
--

On Mon, 02 Jun 2008 13:45:15 -0400
@@ -5627,6 +5643,14 @@ int ata_host_register(struct ata_host *h
                        /* wait for EH to finish */

                        ata_port_wait_eh(ap);

+                       ata_link_for_each_dev(dev, &ap->link)

+                               if (ata_dev_enabled(dev))

+                                       device_attached++;

+                       if (!device_attached &&

+                           (ap->flags & ATA_FLAG_NO_HOTPLUG)) {

+                               /* no device present, disable port */

+                               ata_port_disable(ap);

+                       }

                } else {
Here you can see that it would be very easy for another driver

to just add code to set the NO_HOTPLUG flag and then this code

will work for them as well, since we power off the phy using

DET which is specified by SATA.
here's that code:

+static void ata_phy_offline(struct ata_link *link)

+{

+       u32 scontrol;

+       int rc;

+

+       /* set DET to 4 */

+       rc = sata_scr_read(link, SCR_CONTROL, &scontrol);

+       if (rc)

+               return;

+       scontrol &= ~0xf;

+       scontrol |= (1 << 2);

+       sata_scr_write(link, SCR_CONTROL, scontrol);

+}
while it's true that I only modified ahci.c to set

the flag, any other driver writer can do that for the

other drivers based on whatever they want to use to

determine whether to set the NO_HOTPLUG flag.
--

Agreed.  The main discussion in this sub-thread is more about user

interface.  The user interface in this case, a module option, is

specific to AHCI.
Surely you can see how it is a bit silly to force each driver writer to

create the same user interface in each driver, to support a generic concept.
	Jeff
--

On Mon, 02 Jun 2008 14:15:27 -0400

Not all drivers will need a user interface to turn off hotplug

I would think.  At any rate - I would think it'd be better to let

driver writers decide how they want their drivers to behave wrt

hotplug and power instead of forcing a generic policy on everyone.
This patch would provide users of AHCI controllers a way to save

power now, while you work on the grand scheme for polling/turning on off

hotplug via sysfs.  It's an interim solution that impacts nobody but

ahci users and is can be easily integrated into whatever solution you

eventually work out.
--

It's a one-off driver-specific interface that will have to be supported

even after the same feature is available via libata core...  that's not

the path to scalable, sustainable engineering.
I know user interfaces are annoying because you have to think about

chips other than your own, but that's life.  Other hardware vendors have

to do it too.
Letting each driver have a different user interface is /unfriendly/ to

both developers users.  It's easiest for Intel kernel developers, but

that is not our target audience :)
The biggest power savings for the largest amount of users can be had if

you take a moment and figure out what's best for Linux, rather than what

is best for Intel.
Because you can be damned sure SATA users with non-AHCI chips want this

power savings too...  let's not put roadblocks to that in place in the

beginning (by adding one-off interfaces).
	Jeff
--

On Mon, 02 Jun 2008 14:38:55 -0400
Jeff - I think you are misunderstanding what the module parameter

is doing.  It is not a substitute for the sysfs interface that you

proposed.  You would not want to get rid of it ever.  What it does

is set the default of the AHCI driver to favor the BIOS settings

for which ports are hotpluggable.  the parameter is there to allow

users to override the BIOS settings at module load time.  Once you

get your sysfs interface in, you would then be able to override
User interfaces are annoying because it takes 2 maintainers to agree

on them, that is all.  Surely you remember how long it took AN to

get in?  It was about 9 months.  This patch can go in now while we
As I've been trying to explain to you.  a) this isn't a one off interface.

b) this patch doesn't prevent you at all from adding a generic interface

later.  Perhaps you have a non-technical reason for not wanting this

patch, but I'd like to make sure there's nothing here that you are

just not understanding or something that I can fix.  I obviously can't

fix your idea that I only care about Intel chips, not matter how utterly

wrong and not supported by the facts that is.
--

I like the patch -  it seems that it will help a lot of users out near

term in a very positive way while we iterate on the broader solution,
ric
--

A better patch would enable the _possibility_ of power savings on

non-AHCI chips, and not add a one-off AHCI-specific user interface that

must be supported for years to come.
	Jeff
--

So how about somethig like the following, where the module parameter

just gets moved to libata rather than ahci? Entirely untested, I don't

have ahci on this machine.
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c

index 97f83fb..5913ebb 100644

--- a/drivers/ata/ahci.c

+++ b/drivers/ata/ahci.c

@@ -56,6 +56,7 @@ MODULE_PARM_DESC(skip_host_reset, "skip global host reset (0=don't skip, 1=skip)

 static int ahci_enable_alpm(struct ata_port *ap,

 		enum link_pm policy);

 static void ahci_disable_alpm(struct ata_port *ap);

+static int ahci_is_hotplug_capable(struct ata_port *ap);
 enum {

 	AHCI_PCI_BAR		= 5,

@@ -166,6 +167,8 @@ enum {

 	PORT_CMD_ASP		= (1 << 27), /* Aggressive Slumber/Partial */

 	PORT_CMD_ALPE		= (1 << 26), /* Aggressive Link PM enable */

 	PORT_CMD_ATAPI		= (1 << 24), /* Device is ATAPI */

+	PORT_CMD_ESP		= (1 << 21), /* External SATA Port */

+	PORT_CMD_HPCP		= (1 << 18), /* port is hot plug capable */

 	PORT_CMD_PMP		= (1 << 17), /* PMP attached */

 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */

 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */

@@ -1900,6 +1903,15 @@ static int ahci_pci_device_resume(struct pci_dev *pdev)

 }

 #endif
+static int ahci_is_hotplug_capable(struct ata_port *ap)

+{

+	void __iomem *port_mmio = ahci_port_base(ap);

+	u8 cmd;

+

+	cmd = readl(port_mmio + PORT_CMD);

+	return ((cmd & PORT_CMD_HPCP) || (cmd & PORT_CMD_ESP));

+}

+

 static int ahci_port_start(struct ata_port *ap)

 {

 	struct device *dev = ap->host->dev;

@@ -1951,6 +1963,9 @@ static int ahci_port_start(struct ata_port *ap)
 	ap->private_data = pp;
+	/* set some flags based on port capabilities */

+	if (!ahci_is_hotplug_capable(ap))

+		ap->flags |= ATA_FLAG_NO_HOTPLUG;

 	/* engage engines, captain */

 	return ahci_port_resume(ap);

 }

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c

index 3c89f20..209efe5 100644

--- a/drivers/ata/libata-...
[ message continues ]

I think that the patch does that first part pretty reasonably for all

chips as Kristen explained before.
If the user interface is the only obstacle and the base code seems to be

flexible enough for any device to take advantage of, it would still seem

to be a positive step forward to put in that base functionality (even

without the module param) to enable power saving.
It is always good to get the complete solution in (how hard can the user

interface be to code once we agree on what it should be ;-)), but this

seems to be a good first step.
ric
--

A prep step without the module param (ie. user interface) is fine.

That's how we deployed Async Notification.  Kristen's AN stuff went into

libata well before the SCSI API was settled upon.
	Jeff
--

But, isn't Alan correct that the new logic is useful to some and with

a simple boot option is can be used to enable / disable it.
So if it is defaulted it to disabled, and the end user is allowed to

have a boot option to enable it, you get a reasonable first step for

now.
Greg

--

Greg Freemyer

Litigation Triage Solutions Specialist

http://www.linkedin.com/in/gregfreemyer

First 99 Days Litigation White Paper -

http://www.norcrossgroup.com/forms/whitepapers/99%20Days%20whitepaper.pdf
The Norcross Group

The Intersection of Evidence & Technology

http://www.norcrossgroup.com

--

We don't need it per port Jeff, you are being quite silly here. Right now

its permanently foo=0 for all ports and nobody has suffered anything too

horrible so being able to turn it off for all ports is clearly quite

sufficient for the neat future. If someone wants it per port they'll send

you a patch later.
Alan
--

Er, huh?  foo=0 means hotplug continues to work on the unused ports.

Nobody has suffered because we default to enabling all the goodies.
	Jeff
--

On Mon, 02 Jun 2008 05:48:16 -0400
well - I disagree about nobody suffering.  I would argue that most

users don't use hotplug and would prefer power savings to be the

default, but the patch I sent implemented a module parameter to allow

disable power savings if the user desires hotplug on every port.

Also keep in mind that this patch would still allow hotplug on a per

port basis if the BIOS tells us that this is either an external SATA

port or a port with some kind of externally accessible mechanism.

Here's the patch I sent so you can all review it again.
power off unused ports
If the port isn't either a drive bay or an external SATA port, do not

keep the phy powered on if the port is unoccupied. This saves .75 watts

on most laptops.  A module parameter can be used to override this

behavior and allow the phy's to be powered up regardless of whether it

has externally accessible SATA ports. 
Signed-off-by:  Kristen Carlson Accardi <kristen.c.accardi@intel.com>
---

 drivers/ata/ahci.c        |   21 +++++++++++++++++++++

 drivers/ata/libata-core.c |   24 ++++++++++++++++++++++++

 include/linux/libata.h    |    1 +

 3 files changed, 46 insertions(+)
Index: linux-ahci-phy/drivers/ata/ahci.c

===================================================================

--- linux-ahci-phy.orig/drivers/ata/ahci.c	2008-05-08 14:29:02.000000000 -0700

+++ linux-ahci-phy/drivers/ata/ahci.c	2008-05-08 14:31:05.000000000 -0700

@@ -53,9 +53,13 @@ static int ahci_skip_host_reset;

 module_param_named(skip_host_reset, ahci_skip_host_reset, int, 0444);

 MODULE_PARM_DESC(skip_host_reset, "skip global host reset (0=don't skip, 1=skip)");
+static int ahci_power_save = 1;

+module_param_named(power_save, ahci_power_save, int, 0444);

+MODULE_PARM_DESC(power_save, "Power off unused ports (0=don't power off, 1=power off)");

 static int ahci_enable_alpm(struct ata_port *ap,

 		enum link_pm policy);

 static void ahci_disable_alpm(struct ata_port *ap);

+static int ahci_is_hotplug_capable(s...
[ message continues ]

Thats a simple question of what "=0" means - is it "disable=1" or

"enable=1" or to quote Alice in Wonderland
	'When I use a word,' Humpty Dumpty said, in a rather scornful

	tone,' it means just what I choose it to mean, neither more nor

	less
It really doesn't matter if it is on by default or off by default,

whether 0 means off or on, what matters is that the single switch is

quite sufficient initially to improve matters for everyone wanting to use

the feature without harming anyone else.
Sure you might want to make it per port later but someone who wants to

optimise that peculiar case, has a problem and can test it can deal with

that.
Alan
--

..
I don't like to be picky, but we already have an "ata_link_offline"

function in libata, which tests *whether* a link is offline,

as opposed to *setting* a link to be offline.
So in that context, I find the name ata_phy_offline slightly confusing.

Perhaps something like ata_set_phy_offline would make it more clear?
And a more general note:  I still believe we should have a follow-up

feature to this one, to enable polling for newly inserted drives.
That would allow powering down idle ports to save money/planet/whatever,

but still with hotplug capability.  The polling interval should be

tunable in /sys, with a default of, say, once every couple of seconds.
Thanks for working on this stuff.
Cheers

--

In the grand open source tradition of "pass the buck", I've long been

hoping that someone would take a few days, sit down, and hammer out the

policy side of this.
We -don't- want to do hotplug-active all the time -- the current default

in all drivers that support device hotplug -- because it needlessly

keeps parts active that are unused 99.9% of the time [when they are empty].
Admins need a generic way to control SATA ports and links from

userspace.  Within that, admins need to be able to set a link's hotplug

state among three choices:  hotplug-active [when supported],

hotplug-poll, and hotplug-never.  And of course, hotplug-poll's interval

should be tunable.
And of course this control interface needs to be usable on SAS/SATA

cards that will be common in the future (broadsas, mvsas are early

examples).
This is why I look askance at an AHCI BIOS flag.  That's merely a hint,

and potentially unreliable one at that.  It could just be describing

what the BIOS writer felt were the ports that _should_ be hotpluggable

-- i.e. not even describing what is _possible_ but someone's definition

of "reasonable."  The BIOS flag might even be filled with fuzz (AHCI's

BIOS-written registers have occasionally been shipped into the field

uninitialized).
A better solution involves taking the BIOS flag and using that to set a

default policy that an admin can easily override using the

above-mentioned control interface.
Because in some cases, that BIOS flag might do the wrong thing, and we

need to give the admin an ability to undo the damage.
	Jeff
--

Just out of interest, is this set on laptop bays? (Are any of those even

native SATA yet?)

--

Matthew Garrett | mjg59@srcf.ucam.org

--

My wife's Asus R1F does in fact have SATA in use for the drive bay and

allows swapping between the DVD-RW and a second HD (or another battery

if she had one) in that bay.  I would think many laptops do that now.
--

Len Sorensen

--

Genuinely SATA, or PATA bridged to SATA?

--

Matthew Garrett | mjg59@srcf.ucam.org

--

SATA drives using intel 945GM chipset which has two native SATA ports.
--

Len Sorensen

--

On Fri, 9 May 2008 12:14:56 -0400
Most 945s I've seen in implementation have the DVD/drive bay as PATA

bridged to SATA.
But, this is an irrelevant conversation.  
For drives controlled by ata_piix, they will continue with the

"hotplug all the time" policy that the rest of libata has until

someone else patches that driver to mark the drives not hotpluggable

based on whatever method they want to use to determine that.  As we

get more laptops implementing pure SATA drive bays we can refine

the patch to use ACPI events to turn the phy back on if

laptop vendors continue to use ACPI to generate insert/remove events.

--

On Fri, 9 May 2008 00:37:13 +0100
I haven't seen any yet that are.  Most of them are PATA.  I expect

that will change in the future.
--

Yeah. My vague concern would be that on laptops, we can probably power

down the port even if it's declared as being hotplug capable - we'll

probably get an out of band signal from ACPI anyway. Not an issue at the

moment, but possibly something we'll hit in future.
--

Matthew Garrett | mjg59@srcf.ucam.org

--

On Fri, 9 May 2008 01:14:58 +0100
well - when drive bays on laptops are able to be controlled by

AHCI, they may not actually use _EJ0 or send notifications anymore,

since ACPI will not longer be necessary for hotplugging.  We'll have

to see what happens.

--