From: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, Pavel Machek <pavel@ucw.cz>,
Theodore Tso <tytso@mit.edu>,
linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ata: ahci: power off unused ports
Date: Mon, 2 Jun 2008 09:55:23 -0700 [thread overview]
Message-ID: <20080602095523.0d2647eb@appleyard> (raw)
In-Reply-To: <4843C1E0.3080900@garzik.org>
On Mon, 02 Jun 2008 05:48:16 -0400
Jeff Garzik <jeff@garzik.org> wrote:
> Alan Cox wrote:
> >> 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.
> >
> > 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.
>
> 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
>
>
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(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 DMA engine running */
@@ -1900,6 +1906,18 @@ static int ahci_pci_device_resume(struct
}
#endif
+static int ahci_is_hotplug_capable(struct ata_port *ap)
+{
+ void __iomem *port_mmio = ahci_port_base(ap);
+ u8 cmd;
+
+ if (!ahci_power_save)
+ return 1;
+
+ 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 +1969,9 @@ static int ahci_port_start(struct ata_po
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);
}
Index: linux-ahci-phy/drivers/ata/libata-core.c
===================================================================
--- linux-ahci-phy.orig/drivers/ata/libata-core.c 2008-05-08 14:28:57.000000000 -0700
+++ linux-ahci-phy/drivers/ata/libata-core.c 2008-05-08 14:29:50.000000000 -0700
@@ -162,6 +162,19 @@ MODULE_DESCRIPTION("Library module for A
MODULE_LICENSE("GPL");
MODULE_VERSION(DRV_VERSION);
+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);
+}
/**
* ata_force_cbl - force cable type according to libata.force
@@ -2671,6 +2684,7 @@ void ata_port_disable(struct ata_port *a
ap->link.device[0].class = ATA_DEV_NONE;
ap->link.device[1].class = ATA_DEV_NONE;
ap->flags |= ATA_FLAG_DISABLED;
+ ata_phy_offline(&ap->link);
}
/**
@@ -5609,6 +5623,8 @@ int ata_host_register(struct ata_host *h
if (ap->ops->error_handler) {
struct ata_eh_info *ehi = &ap->link.eh_info;
unsigned long flags;
+ int device_attached = 0;
+ struct ata_device *dev;
ata_port_probe(ap);
@@ -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 {
DPRINTK("ata%u: bus probe begin\n", ap->print_id);
rc = ata_bus_probe(ap);
Index: linux-ahci-phy/include/linux/libata.h
===================================================================
--- linux-ahci-phy.orig/include/linux/libata.h 2008-05-08 14:28:57.000000000 -0700
+++ linux-ahci-phy/include/linux/libata.h 2008-05-08 14:29:50.000000000 -0700
@@ -193,6 +193,7 @@ enum {
ATA_FLAG_AN = (1 << 18), /* controller supports AN */
ATA_FLAG_PMP = (1 << 19), /* controller supports PMP */
ATA_FLAG_IPM = (1 << 20), /* driver can handle IPM */
+ ATA_FLAG_NO_HOTPLUG = (1 << 21), /* port doesn't support HP */
/* The following flag belongs to ap->pflags but is kept in
* ap->flags because it's referenced in many LLDs and will be
next prev parent reply other threads:[~2008-06-02 17:04 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-08 23:10 [PATCH] ata: ahci: power off unused ports Kristen Carlson Accardi
2008-05-08 23:37 ` Matthew Garrett
2008-05-08 23:35 ` Kristen Carlson Accardi
2008-05-09 0:14 ` Matthew Garrett
2008-05-09 0:28 ` Kristen Carlson Accardi
2008-05-09 15:58 ` Lennart Sorensen
2008-05-09 16:06 ` Matthew Garrett
2008-05-09 16:14 ` Lennart Sorensen
2008-05-09 17:14 ` Kristen Carlson Accardi
2008-05-09 17:14 ` Kristen Carlson Accardi
2008-05-09 15:06 ` Mark Lord
2008-05-09 15:28 ` Port control interface (was Re: [PATCH] ata: ahci: power off unused ports) Jeff Garzik
2008-05-27 3:08 ` [PATCH] ata: ahci: power off unused ports Theodore Tso
2008-05-27 21:32 ` Kristen Carlson Accardi
2008-05-27 22:59 ` Theodore Tso
2008-05-27 23:32 ` Kristen Carlson Accardi
2008-05-31 8:00 ` Pavel Machek
2008-06-01 19:16 ` Jeff Garzik
2008-06-02 7:04 ` Alan Cox
2008-06-02 7:43 ` Jeff Garzik
2008-06-02 8:22 ` Alan Cox
2008-06-02 9:48 ` Jeff Garzik
2008-06-02 13:54 ` Alan Cox
2008-06-02 16:55 ` Kristen Carlson Accardi [this message]
2008-06-02 13:03 ` Mark Lord
2008-06-02 16:07 ` Jeff Garzik
2008-06-02 17:00 ` Kristen Carlson Accardi
2008-06-02 17:45 ` Jeff Garzik
2008-06-02 17:47 ` Kristen Carlson Accardi
2008-06-02 18:15 ` Jeff Garzik
2008-06-02 18:16 ` Kristen Carlson Accardi
2008-06-02 18:30 ` Ric Wheeler
2008-06-02 18:40 ` Jeff Garzik
2008-06-02 18:49 ` Ric Wheeler
2008-06-02 18:52 ` Jeff Garzik
2008-06-02 20:00 ` Matthew Garrett
2008-06-02 18:38 ` Jeff Garzik
2008-06-03 16:49 ` Kristen Carlson Accardi
2008-06-02 17:07 ` Greg Freemyer
2008-06-02 16:57 ` Kristen Carlson Accardi
2008-06-02 17:44 ` Jeff Garzik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080602095523.0d2647eb@appleyard \
--to=kristen.c.accardi@intel.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=jeff@garzik.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pavel@ucw.cz \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.