2007-06-11 18:52:44

by Kristen Carlson Accardi

[permalink] [raw]
Subject: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

This patch will set the correct bits to turn on Aggressive
Link Power Management (ALPM) for the ahci driver. This
will cause the controller and disk to negotiate a lower
power state for the link when there is no activity (see
the AHCI 1.x spec for details). This feature is mutually
exclusive with Hot Plug, so when ALPM is enabled, Hot Plug
is disabled. ALPM will be enabled by default, but it is
settable via the scsi host syfs interface. Possible
settings for this feature are:

Setting Effect
----------------------------------------------------------
min_power ALPM is enabled, and link set to enter
lowest power state (SLUMBER) when idle
Hot plug not allowed.

max_performance ALPM is disabled, Hot Plug is allowed

medium_power ALPM is enabled, and link set to enter
second lowest power state (PARTIAL) when
idle. Hot plug not allowed.

Signed-off-by: Kristen Carlson Accardi <[email protected]>

Index: 2.6-git/drivers/ata/ahci.c
===================================================================
--- 2.6-git.orig/drivers/ata/ahci.c
+++ 2.6-git/drivers/ata/ahci.c
@@ -48,6 +48,9 @@
#define DRV_NAME "ahci"
#define DRV_VERSION "2.2"

+static int ahci_enable_alpm(struct ata_port *ap,
+ enum scsi_host_link_pm policy);
+static int ahci_disable_alpm(struct ata_port *ap);

enum {
AHCI_PCI_BAR = 5,
@@ -97,6 +100,7 @@ enum {
/* HOST_CAP bits */
HOST_CAP_SSC = (1 << 14), /* Slumber capable */
HOST_CAP_CLO = (1 << 24), /* Command List Override support */
+ HOST_CAP_ALPM = (1 << 26), /* Aggressive Link PM support */
HOST_CAP_SSS = (1 << 27), /* Staggered Spin-up */
HOST_CAP_NCQ = (1 << 30), /* Native Command Queueing */
HOST_CAP_64 = (1 << 31), /* PCI DAC (64-bit DMA) support */
@@ -151,6 +155,8 @@ enum {
PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS,

/* PORT_CMD bits */
+ 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_LIST_ON = (1 << 15), /* cmd list DMA engine running */
PORT_CMD_FIS_ON = (1 << 14), /* FIS DMA engine running */
@@ -171,6 +177,7 @@ enum {
AHCI_FLAG_HONOR_PI = (1 << 26), /* honor PORTS_IMPL */
AHCI_FLAG_IGN_SERR_INTERNAL = (1 << 27), /* ignore SERR_INTERNAL */
AHCI_FLAG_32BIT_ONLY = (1 << 28), /* force 32bit */
+ AHCI_FLAG_NO_HOTPLUG = (1 << 29), /* ignore PxSERR.DIAG.N */

AHCI_FLAG_COMMON = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
@@ -253,6 +260,7 @@ static struct scsi_host_template ahci_sh
.slave_configure = ata_scsi_slave_config,
.slave_destroy = ata_scsi_slave_destroy,
.bios_param = ata_std_bios_param,
+ .set_link_pm_policy = ata_scsi_set_link_pm_policy,
};

static const struct ata_port_operations ahci_ops = {
@@ -284,6 +292,7 @@ static const struct ata_port_operations
.port_suspend = ahci_port_suspend,
.port_resume = ahci_port_resume,
#endif
+ .enable_pm = ahci_enable_alpm,

.port_start = ahci_port_start,
.port_stop = ahci_port_stop,
@@ -695,6 +704,152 @@ static void ahci_power_up(struct ata_por
writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD);
}

+static int ahci_disable_alpm(struct ata_port *ap)
+{
+ void __iomem *port_mmio = ahci_port_base(ap);
+ u32 cmd, scontrol;
+ struct ahci_port_priv *pp = ap->private_data;
+
+ /*
+ * disable Interface Power Management State Transitions
+ * This is accomplished by setting bits 8:11 of the
+ * SATA Control register
+ */
+ scontrol = readl(port_mmio + PORT_SCR_CTL);
+ scontrol |= (0x3 << 8);
+ writel(scontrol, port_mmio + PORT_SCR_CTL);
+
+ /* get the existing command bits */
+ cmd = readl(port_mmio + PORT_CMD);
+
+ /* disable ALPM and ASP */
+ cmd &= ~PORT_CMD_ASP;
+ cmd &= ~PORT_CMD_ALPE;
+
+ /* force the interface back to active */
+ cmd |= PORT_CMD_ICC_ACTIVE;
+
+ /* write out new cmd value */
+ writel(cmd, port_mmio + PORT_CMD);
+ cmd = readl(port_mmio + PORT_CMD);
+
+ /* wait 10ms to be sure we've come out of any low power state */
+ msleep(10);
+
+ /* clear out any PhyRdy stuff from interrupt status */
+ writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT);
+
+ /* go ahead and clean out PhyRdy Change from Serror too */
+ ahci_scr_write(ap, SCR_ERROR, (1 << 16));
+
+ /*
+ * Clear flag to indicate that we should ignore all PhyRdy
+ * state changes
+ */
+ ap->flags &= ~AHCI_FLAG_NO_HOTPLUG;
+
+ /*
+ * Enable interrupts on Phy Ready.
+ */
+ pp->intr_mask |= PORT_IRQ_PHYRDY;
+ writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+ ap->pm_policy = SHOST_MAX_PERFORMANCE;
+ return 0;
+}
+
+static int ahci_enable_alpm(struct ata_port *ap,
+ enum scsi_host_link_pm policy)
+{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+ void __iomem *port_mmio = ahci_port_base(ap);
+ u32 cmd, scontrol, sstatus;
+ struct ahci_port_priv *pp = ap->private_data;
+ u32 asp;
+
+ /* Make sure the host is capable of link power management */
+ if (!(hpriv->cap & HOST_CAP_ALPM)) {
+ ap->pm_policy = SHOST_NOT_AVAILABLE;
+ return -EINVAL;
+ }
+
+ /* make sure we have a device attached */
+ sstatus = readl(port_mmio + PORT_SCR_STAT);
+ if (!(sstatus & 0xf00)) {
+ ap->pm_policy = SHOST_NOT_AVAILABLE;
+ return -EINVAL;
+ }
+
+ switch(policy) {
+ case SHOST_MAX_PERFORMANCE:
+ return ahci_disable_alpm(ap);
+ case SHOST_NOT_AVAILABLE:
+ case SHOST_MIN_POWER:
+ /*
+ * if we came here with SHOST_NOT_AVAILABLE,
+ * it just means this is the first time we
+ * have tried to enable - so try to do
+ * min_power
+ */
+ ap->pm_policy = SHOST_MIN_POWER;
+
+ /* configure HBA to enter SLUMBER */
+ asp = PORT_CMD_ASP;
+ break;
+ case SHOST_MEDIUM_POWER:
+ /* configure HBA to enter PARTIAL */
+ ap->pm_policy = policy;
+ asp = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /*
+ * Disable interrupts on Phy Ready. This keeps us from
+ * getting woken up due to spurious phy ready interrupts
+ * TBD - Hot plug should be done via polling now, is
+ * that even supported?
+ */
+ 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
+ * state changes since these can happen now whenever we
+ * change link state
+ */
+ ap->flags |= AHCI_FLAG_NO_HOTPLUG;
+
+ /* get the existing command bits */
+ cmd = readl(port_mmio + PORT_CMD);
+
+ /*
+ * enable Interface Power Management State Transitions
+ * This is accomplished by clearing bits 8:11 of the
+ * SATA Control register
+ */
+ scontrol = readl(port_mmio + PORT_SCR_CTL);
+ scontrol &= ~(0x3 << 8);
+ writel(scontrol, port_mmio + PORT_SCR_CTL);
+
+ /*
+ * Set ASP based on Policy
+ */
+ cmd |= asp;
+
+ /*
+ * Setting this bit will instruct the HBA to aggressively
+ * enter a lower power link state when it's appropriate and
+ * based on the value set above for ASP
+ */
+ cmd |= PORT_CMD_ALPE;
+
+ /* write out new cmd value */
+ writel(cmd, port_mmio + PORT_CMD);
+ cmd = readl(port_mmio + PORT_CMD);
+ return 0;
+}
+
#ifdef CONFIG_PM
static void ahci_power_down(struct ata_port *ap)
{
@@ -1220,6 +1375,10 @@ static void ahci_host_intr(struct ata_po
status = readl(port_mmio + PORT_IRQ_STAT);
writel(status, port_mmio + PORT_IRQ_STAT);

+ if ((ap->flags & AHCI_FLAG_NO_HOTPLUG) &&
+ (status & PORT_IRQ_PHYRDY))
+ status &= ~PORT_IRQ_PHYRDY;
+
if (unlikely(status & PORT_IRQ_ERROR)) {
ahci_error_intr(ap, status);
return;
@@ -1735,6 +1894,7 @@ static int ahci_init_one(struct pci_dev

ap->ioaddr.cmd_addr = port_mmio;
ap->ioaddr.scr_addr = port_mmio + PORT_SCR;
+ ap->pm_policy = SHOST_NOT_AVAILABLE;
} else
host->ports[i]->ops = &ata_dummy_port_ops;
}

--


2007-06-11 20:01:27

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Kristen Carlson Accardi wrote:
> This patch will set the correct bits to turn on Aggressive
> Link Power Management (ALPM) for the ahci driver. This
> will cause the controller and disk to negotiate a lower
> power state for the link when there is no activity (see
> the AHCI 1.x spec for details). This feature is mutually
> exclusive with Hot Plug, so when ALPM is enabled, Hot Plug
> is disabled. ALPM will be enabled by default, but it is
> settable via the scsi host syfs interface. Possible
> settings for this feature are:
>
> Setting Effect
> ----------------------------------------------------------
> min_power ALPM is enabled, and link set to enter
> lowest power state (SLUMBER) when idle
> Hot plug not allowed.
>
> max_performance ALPM is disabled, Hot Plug is allowed
>
> medium_power ALPM is enabled, and link set to enter
> second lowest power state (PARTIAL) when
> idle. Hot plug not allowed.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>

seems OK at first glance, though I'll have questions about hardware
behavior once I get off this day-long Intel conference call :)


Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Mon, 11 Jun 2007, Kristen Carlson Accardi wrote:
> Setting Effect
> ----------------------------------------------------------
> min_power ALPM is enabled, and link set to enter
> lowest power state (SLUMBER) when idle
> Hot plug not allowed.
>
> max_performance ALPM is disabled, Hot Plug is allowed
>
> medium_power ALPM is enabled, and link set to enter
> second lowest power state (PARTIAL) when
> idle. Hot plug not allowed.

Just some food for thought:

If you split it into a enable/disable (0/1) attribute, and a level attribute
(some sort of integer scale, or "min", "medium", "max" if you must use
strings. You could use four levels to mimic the PCI device power state, for
example), it might make its usage more generic, and easier from userspace,
as it decouples the need to turn it on/off from the need to know which level
the user wants it set to when you turn it on.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2007-06-12 01:18:18

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Henrique de Moraes Holschuh wrote:
> On Mon, 11 Jun 2007, Kristen Carlson Accardi wrote:
>> Setting Effect
>> ----------------------------------------------------------
>> min_power ALPM is enabled, and link set to enter
>> lowest power state (SLUMBER) when idle
>> Hot plug not allowed.
>>
>> max_performance ALPM is disabled, Hot Plug is allowed
>>
>> medium_power ALPM is enabled, and link set to enter
>> second lowest power state (PARTIAL) when
>> idle. Hot plug not allowed.
>
> Just some food for thought:
>
> If you split it into a enable/disable (0/1) attribute, and a level attribute

on/off doesn't really make sense if the question is "do you favor
power or do you favor performance".......

2007-06-12 01:59:57

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Dagfinn Ilmari Manns?ker wrote:
> Arjan van de Ven <[email protected]> writes:
>
>> Henrique de Moraes Holschuh wrote:
>>> On Mon, 11 Jun 2007, Kristen Carlson Accardi wrote:
>>>> Setting Effect
>>>> ----------------------------------------------------------
>>>> min_power ALPM is enabled, and link set to enter lowest
>>>> power state (SLUMBER) when idle
>>>> Hot plug not allowed.
>>>>
>>>> max_performance ALPM is disabled, Hot Plug is allowed
>>>>
>>>> medium_power ALPM is enabled, and link set to enter
>>>> second lowest power state (PARTIAL) when
>>>> idle. Hot plug not allowed.
>>> Just some food for thought:
>>> If you split it into a enable/disable (0/1) attribute, and a level
>>> attribute
>> on/off doesn't really make sense if the question is "do you favor power
>> or do you favor performance".......
>
> How about just making it a numeric scale with 0 meaning no power saving
> and then some fixed number of levels (e.g 0-9)?

The original proposal seems far more intuitive than these alternatives.

Jeff



Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Arjan van de Ven <[email protected]> writes:

> Henrique de Moraes Holschuh wrote:
>> On Mon, 11 Jun 2007, Kristen Carlson Accardi wrote:
>>> Setting Effect
>>> ----------------------------------------------------------
>>> min_power ALPM is enabled, and link set to enter lowest
>>> power state (SLUMBER) when idle
>>> Hot plug not allowed.
>>>
>>> max_performance ALPM is disabled, Hot Plug is allowed
>>>
>>> medium_power ALPM is enabled, and link set to enter
>>> second lowest power state (PARTIAL) when
>>> idle. Hot plug not allowed.
>> Just some food for thought:
>> If you split it into a enable/disable (0/1) attribute, and a level
>> attribute
>
> on/off doesn't really make sense if the question is "do you favor power
> or do you favor performance".......

How about just making it a numeric scale with 0 meaning no power saving
and then some fixed number of levels (e.g 0-9)?

--
ilmari
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Mon, 11 Jun 2007, Jeff Garzik wrote:
> >>on/off doesn't really make sense if the question is "do you favor power
> >>or do you favor performance".......

Actually, it does if you think of it as "do you need hotplug right now or
not?".

> >How about just making it a numeric scale with 0 meaning no power saving
> >and then some fixed number of levels (e.g 0-9)?
>
> The original proposal seems far more intuitive than these alternatives.

There is nothing intuitive about the text or the levels. All cases need
proper documentation. I'd never expect link powersaving to kill hotplug,
unless I read the AHCI docs.

And enable/disable ain't intuitive either :( But enable/disable is useful
to get stuff like SATA bay hotplug, dock/undock and other stuff that needs
hotplug to be working right, unless we can make that automatic so that power
saving is always disabled in all situations we'd need hotplug to be working?

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2007-06-12 04:01:31

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Henrique de Moraes Holschuh wrote:
> On Mon, 11 Jun 2007, Jeff Garzik wrote:
>>>> on/off doesn't really make sense if the question is "do you favor power
>>>> or do you favor performance".......
>
> Actually, it does if you think of it as "do you need hotplug right now or
> not?".

that's a temporary shortcoming; even with these power savings you can
do hotplug as long as you're willing to poll for it at a reasonable
interval and are willing to wait the time between polls for a hotplug
to take effect..

2007-06-12 09:10:26

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Mon, Jun 11, 2007 at 08:59:46PM -0700, Arjan van de Ven wrote:

> that's a temporary shortcoming; even with these power savings you can
> do hotplug as long as you're willing to poll for it at a reasonable
> interval and are willing to wait the time between polls for a hotplug
> to take effect..

On laptops, I suspect that we'll probably get an ACPI interrupt even if
the AHCI hotplug pathway can't manage.

--
Matthew Garrett | [email protected]

Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, 12 Jun 2007, Matthew Garrett wrote:
> On Mon, Jun 11, 2007 at 08:59:46PM -0700, Arjan van de Ven wrote:
> > that's a temporary shortcoming; even with these power savings you can
> > do hotplug as long as you're willing to poll for it at a reasonable
> > interval and are willing to wait the time between polls for a hotplug
> > to take effect..
>
> On laptops, I suspect that we'll probably get an ACPI interrupt even if
> the AHCI hotplug pathway can't manage.

As long as we don't crash the drive or AHCI controller because we hotplugged
it in a way it didn't like (like trying to hotplug a ICH5R would cause).

It is not like 1Hz would not be fast enough for laptop bays and docks, so
the polling frequency would not be a problem. I still prefer when levels
and on/off are kept separate, but if it won't stand in the way of hotplug, I
certainly don't care enough to bother.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2007-06-12 13:50:47

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, Jun 12, 2007 at 09:18:19AM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 12 Jun 2007, Matthew Garrett wrote:
> >
> > On laptops, I suspect that we'll probably get an ACPI interrupt even if
> > the AHCI hotplug pathway can't manage.
>
> As long as we don't crash the drive or AHCI controller because we hotplugged
> it in a way it didn't like (like trying to hotplug a ICH5R would cause).

Laptop bays are designed to deal with hotplugging PATA - I don't think
this is too much of an issue :)

--
Matthew Garrett | [email protected]

Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, 12 Jun 2007, Matthew Garrett wrote:
> On Tue, Jun 12, 2007 at 09:18:19AM -0300, Henrique de Moraes Holschuh wrote:
> > On Tue, 12 Jun 2007, Matthew Garrett wrote:
> > >
> > > On laptops, I suspect that we'll probably get an ACPI interrupt even if
> > > the AHCI hotplug pathway can't manage.
> >
> > As long as we don't crash the drive or AHCI controller because we hotplugged
> > it in a way it didn't like (like trying to hotplug a ICH5R would cause).
>
> Laptop bays are designed to deal with hotplugging PATA - I don't think
> this is too much of an issue :)

The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
cards use usb2.0 and pci-e hotplug...

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2007-06-12 15:38:57

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 12 Jun 2007, Matthew Garrett wrote:
> > Laptop bays are designed to deal with hotplugging PATA - I don't think
> > this is too much of an issue :)
>
> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
> cards use usb2.0 and pci-e hotplug...

Yes, but they'll also send an ACPI interrupt even if the SATA host
controller doesn't - it's part of the spec for bays.

--
Matthew Garrett | [email protected]

2007-06-12 15:45:45

by Tejun Heo

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Matthew Garrett wrote:
> On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote:
>> On Tue, 12 Jun 2007, Matthew Garrett wrote:
>>> Laptop bays are designed to deal with hotplugging PATA - I don't think
>>> this is too much of an issue :)
>> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
>> cards use usb2.0 and pci-e hotplug...
>
> Yes, but they'll also send an ACPI interrupt even if the SATA host
> controller doesn't - it's part of the spec for bays.

Does the spec mandate that the ACPI interrupt shouldn't depend on SATA
phy status? I don't think vendors are likely to implement separate
mechanism when SATA phy status can do the job fine.

--
tejun

2007-06-12 15:47:49

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Matthew Garrett wrote:
> On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote:
>> On Tue, 12 Jun 2007, Matthew Garrett wrote:
>>> Laptop bays are designed to deal with hotplugging PATA - I don't think
>>> this is too much of an issue :)
>> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
>> cards use usb2.0 and pci-e hotplug...
>
> Yes, but they'll also send an ACPI interrupt even if the SATA host
> controller doesn't - it's part of the spec for bays.

Regardless, having a laptop does not imply having a docking bay.

Jeff



2007-06-12 15:56:56

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Wed, Jun 13, 2007 at 12:45:21AM +0900, Tejun Heo wrote:
> Matthew Garrett wrote:
> > Yes, but they'll also send an ACPI interrupt even if the SATA host
> > controller doesn't - it's part of the spec for bays.
>
> Does the spec mandate that the ACPI interrupt shouldn't depend on SATA
> phy status? I don't think vendors are likely to implement separate
> mechanism when SATA phy status can do the job fine.

I suspect that the spec allows them to do that, but think that it's
unlikely to actually happen in most cases. Bear in mind that Windows
doesn't tend to drive the hardware in AHCI mode, and that their
implementation is likely to be very similar to the implementation they
used for PATA devices.

--
Matthew Garrett | [email protected]

2007-06-12 15:58:50

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, Jun 12, 2007 at 11:46:56AM -0400, Jeff Garzik wrote:
> Matthew Garrett wrote:
> >On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh
> >wrote:
> >>On Tue, 12 Jun 2007, Matthew Garrett wrote:
> >>>Laptop bays are designed to deal with hotplugging PATA - I don't think
> >>>this is too much of an issue :)
> >>The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
> >>cards use usb2.0 and pci-e hotplug...
> >
> >Yes, but they'll also send an ACPI interrupt even if the SATA host
> >controller doesn't - it's part of the spec for bays.
>
> Regardless, having a laptop does not imply having a docking bay.

Excluding the corner case of an Expresscard SATA controller (where I
suspect you'd want different policy), I doubt there are any cases where
you have a laptop with hotplug capabilities without it being implemented
as an ACPI bay.

--
Matthew Garrett | [email protected]

2007-06-12 16:19:20

by Jeff Garzik

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

Matthew Garrett wrote:
> Excluding the corner case of an Expresscard SATA controller (where I
> suspect you'd want different policy), I doubt there are any cases where
> you have a laptop with hotplug capabilities without it being implemented
> as an ACPI bay.

Cardbus card.

Jeff



2007-06-12 16:31:27

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

On Tue, 12 Jun 2007 11:46:56 -0400
Jeff Garzik <[email protected]> wrote:

> Matthew Garrett wrote:
> > On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote:
> >> On Tue, 12 Jun 2007, Matthew Garrett wrote:
> >>> Laptop bays are designed to deal with hotplugging PATA - I don't think
> >>> this is too much of an issue :)
> >> The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e
> >> cards use usb2.0 and pci-e hotplug...
> >
> > Yes, but they'll also send an ACPI interrupt even if the SATA host
> > controller doesn't - it's part of the spec for bays.
>
> Regardless, having a laptop does not imply having a docking bay.
>
> Jeff
>

For bay devices, we can use ACPI just like we do now. For non-bay devices,
we can implement hotplug via polling when ALPM is enabled. In my experience
most laptop vendors implement extra drive as either PATA in a dock station,
USB in a dock station, or a bay device either on the dock station, or
on the laptop itself.


Kristen