Tejun Heo wrote:
> Arjan van de Ven wrote:
>>> They were hardware problems. I don't think any amount of proper
>>> implementation can fix them. I have one DVD RAM somewhere in my pile of
>>> hardware which locks up solidly if any link PS mode is used and had a
>> and the AHCI ALPM code decides to use power savings on this device? if
>> so, please give us the idents so that we can add it to the blacklist as
>> the first entry... (or can buy it to check it in detail first)
>
> Yeap, lemme check.
>
> It's "TSSTcorpCD/DVDW SH-S183A" with firmware revision "SB01". Wanna
> check ID capability bits but 'hdparm -I /dev/sr0' is still broken and
> it's already past 3 am here. I'll report back tomorrow.
Oops, that was the wrong one. Locking up one was HL-DT-STDVD-RAM
GSA-H30N and it correctly reports that it doesn't support IPM. Here
are some test results.
Controller is ICH9.
00:1f.2 SATA controller [Class 0106]: Intel Corporation Unknown device [8086:2922] (rev 02)
====
1. HL-DT-STDVD-RAM GSA-H30N
ATAPI CD-ROM, with removable media
Model Number: HL-DT-STDVD-RAM GSA-H30N
Serial Number:
Firmware Revision: 1.00
Standards:
Likely used CD-ROM ATAPI-1
Configuration:
DRQ response: 50us.
Packet size: 12 bytes
Capabilities:
LBA, IORDY(can be disabled)
DMA: sdma0 sdma1 sdma2 mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 *udma5
Cycle time: min=120ns recommended=120ns
PIO: pio0 pio1 pio2 pio3 pio4
Cycle time: no flow control=120ns IORDY flow control=120
This device doesn't claim to support HIPM nor DIPM.
# echo min_power > link_power_management_policy
[ 752.761751] ata1.00: Unable to set Link PM policy
[ 784.510218] ata1.00: exception Emask 0x50 SAct 0x0 SErr 0xd0800 action 0x6 frozen
[ 784.530266] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in
[ 784.530270] res 40/00:03:00:00:20/00:00:00:00:00/a0 Emask 0x54 (ATA bus error)
[ 784.571175] ata1: hard resetting port
[ 790.489486] ata1: port is slow to respond, please be patient (Status 0x80)
[ 794.594247] ata1: COMRESET failed (errno=-16)
[ 794.611174] ata1: hard resetting port
[ 800.541562] ata1: port is slow to respond, please be patient (Status 0x80)
[ 804.646316] ata1: COMRESET failed (errno=-16)
[ 804.663284] ata1: hard resetting port
[ 810.593623] ata1: port is slow to respond, please be patient (Status 0x80)
[ 839.654644] ata1: COMRESET failed (errno=-16)
[ 839.672576] ata1: limiting SATA link speed to 1.5 Gbps
[ 839.691024] ata1: hard resetting port
[ 844.726659] ata1: COMRESET failed (errno=-16)
[ 844.744064] ata1: reset failed, giving up
[ 844.761614] ata1.00: disabled
[ 844.761639] ata1: EH complete
The device doesn't respond till power is physically removed and
restored. It seems something in ahci_disable_alpm() path upsets the
device.
2. TSSTcorpCD/DVDW SH-S183A
ATAPI CD-ROM, with removable media
Model Number: TSSTcorpCD/DVDW SH-S183A
Serial Number:
Firmware Revision: SB01
Standards:
Likely used CD-ROM ATAPI-1
Configuration:
DRQ response: 50us.
Packet size: 12 bytes
Capabilities:
LBA, IORDY(can be disabled)
DMA: mdma0 mdma1 mdma2 udma0 udma1 *udma2
Cycle time: min=120ns recommended=120ns
PIO: pio0 pio1 pio2 pio3 pio4
Cycle time: no flow control=120ns IORDY flow control=120ns
Commands/features:
Enabled Supported:
* SATA-I signaling speed (1.5Gb/s)
* Host-initiated interface power management
* Phy event counters
Device-initiated interface power management
unknown 78[5]
* Software settings preservation
This one claims to support HIPS.
# echo min_power > link_power_management_policy
[ 1301.917248] ata1.00: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2
[ 1301.938338] ata1.00: irq_stat 0x40000001
[ 1301.956955] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in
[ 1301.956959] res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x10 (ATA bus error)
[ 1304.189565] ata1: soft resetting port
[ 1304.207745] ata1: SATA link down (SStatus 611 SControl 300)
[ 1304.228076] ata1: failed to recover some devices, retrying in 5 secs
[ 1309.245599] ata1: hard resetting port
[ 1314.773227] ata1: port is slow to respond, please be patient (Status 0x80)
[ 1319.269677] ata1: COMRESET failed (errno=-16)
[ 1319.289639] ata1: hard resetting port
[ 1319.781285] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[ 1320.115569] ata1.00: configured for UDMA/33
[ 1320.134780] ata1: EH complete
And hotplug works fine after EH is done with itself. Dunno why.
3. PLEXTOR DVDR PX-716A
ATAPI CD-ROM, with removable media
Model Number: PLEXTOR DVDR PX-716A
Serial Number: 127377
Firmware Revision: 1.09
Standards:
Likely used CD-ROM ATAPI-1
Configuration:
DRQ response: 50us.
Packet size: 12 bytes
Capabilities:
LBA, IORDY(can be disabled)
DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 *udma4
Cycle time: min=120ns recommended=120ns
PIO: pio0 pio1 pio2 pio3 pio4
Cycle time: no flow control=120ns IORDY flow control=120ns
HW reset results:
CBLID- below Vih
Device num = 0
# echo min_power > link_power_management_policy
[ 2102.655765] ata1.00: Unable to set Link PM policy
[ 2104.505926] ata1.00: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2
[ 2104.527256] ata1.00: irq_stat 0x40000001
[ 2104.545868] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in
[ 2104.545870] res 51/24:03:00:00:20/00:00:00:00:00/a0 Emask 0x10 (ATA bus error)
[ 2106.810252] ata1: soft resetting port
[ 2106.828106] ata1: SATA link down (SStatus 611 SControl 300)
[ 2106.847957] ata1: failed to recover some devices, retrying in 5 secs
[ 2111.870285] ata1: hard resetting port
[ 2117.401917] ata1: port is slow to respond, please be patient (Status 0x80)
[ 2121.902365] ata1: COMRESET failed (errno=-16)
[ 2121.920313] ata1: hard resetting port
[ 2122.413973] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[ 2122.795507] ata1.00: configured for UDMA/66
[ 2122.813234] ata1: EH complete
4. Harddisks
All the harddisks I have behave themselves. Impressive.
====
In all cases, hotplug works well even after ALPM is configured. Have
no idea why. It might be that PARTIAL/SLUMBER mode didn't kick in or
ICH9 has some magic feature to detect PHY status changes even in PS
mode (wishful thinking).
Will repeat the test with ICH7 when I get some time. Anyone up for
testing JMB and VIA?
With some massging to ahci_disable_alpm(), I think it can be fairly
safe on this chipset at least.
--
tejun
On Wed, 01 Aug 2007 18:23:21 +0900
Tejun Heo <[email protected]> wrote:
> Tejun Heo wrote:
> > Arjan van de Ven wrote:
> >>> They were hardware problems. I don't think any amount of proper
> >>> implementation can fix them. I have one DVD RAM somewhere in my pile of
> >>> hardware which locks up solidly if any link PS mode is used and had a
> >> and the AHCI ALPM code decides to use power savings on this device? if
> >> so, please give us the idents so that we can add it to the blacklist as
> >> the first entry... (or can buy it to check it in detail first)
> >
> > Yeap, lemme check.
> >
> > It's "TSSTcorpCD/DVDW SH-S183A" with firmware revision "SB01". Wanna
> > check ID capability bits but 'hdparm -I /dev/sr0' is still broken and
> > it's already past 3 am here. I'll report back tomorrow.
>
> Oops, that was the wrong one. Locking up one was HL-DT-STDVD-RAM
> GSA-H30N and it correctly reports that it doesn't support IPM. Here
> are some test results.
>
> Controller is ICH9.
>
> 00:1f.2 SATA controller [Class 0106]: Intel Corporation Unknown device [8086:2922] (rev 02)
>
> ====
>
> 1. HL-DT-STDVD-RAM GSA-H30N
>
> ATAPI CD-ROM, with removable media
> Model Number: HL-DT-STDVD-RAM GSA-H30N
> Serial Number:
> Firmware Revision: 1.00
> Standards:
> Likely used CD-ROM ATAPI-1
> Configuration:
> DRQ response: 50us.
> Packet size: 12 bytes
> Capabilities:
> LBA, IORDY(can be disabled)
> DMA: sdma0 sdma1 sdma2 mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 *udma5
> Cycle time: min=120ns recommended=120ns
> PIO: pio0 pio1 pio2 pio3 pio4
> Cycle time: no flow control=120ns IORDY flow control=120
>
> This device doesn't claim to support HIPM nor DIPM.
Are you sure you are using the latest version of the patch? This seems
like a bug, if the device doesn't support HIPM or DIPM it shouldn't attempt
to use ALPM. There was a check for this put into the patch on about the
second or third version - maybe I'm not doing it right.
(from the patch located here:
http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm/libata-enable-pm
if (ata_id_has_hipm(dev->id) || ata_id_has_dipm(dev->id))
+ dev->flags |= ATA_DFLAG_IPM;
+
<snip>
>
> 2. TSSTcorpCD/DVDW SH-S183A
>
> ATAPI CD-ROM, with removable media
> Model Number: TSSTcorpCD/DVDW SH-S183A
> Serial Number:
> Firmware Revision: SB01
> Standards:
> Likely used CD-ROM ATAPI-1
> Configuration:
> DRQ response: 50us.
> Packet size: 12 bytes
> Capabilities:
> LBA, IORDY(can be disabled)
> DMA: mdma0 mdma1 mdma2 udma0 udma1 *udma2
> Cycle time: min=120ns recommended=120ns
> PIO: pio0 pio1 pio2 pio3 pio4
> Cycle time: no flow control=120ns IORDY flow control=120ns
> Commands/features:
> Enabled Supported:
> * SATA-I signaling speed (1.5Gb/s)
> * Host-initiated interface power management
> * Phy event counters
> Device-initiated interface power management
> unknown 78[5]
> * Software settings preservation
>
> This one claims to support HIPS.
>
> # echo min_power > link_power_management_policy
> [ 1301.917248] ata1.00: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2
> [ 1301.938338] ata1.00: irq_stat 0x40000001
> [ 1301.956955] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in
> [ 1301.956959] res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x10 (ATA bus error)
> [ 1304.189565] ata1: soft resetting port
> [ 1304.207745] ata1: SATA link down (SStatus 611 SControl 300)
> [ 1304.228076] ata1: failed to recover some devices, retrying in 5 secs
> [ 1309.245599] ata1: hard resetting port
> [ 1314.773227] ata1: port is slow to respond, please be patient (Status 0x80)
> [ 1319.269677] ata1: COMRESET failed (errno=-16)
> [ 1319.289639] ata1: hard resetting port
> [ 1319.781285] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> [ 1320.115569] ata1.00: configured for UDMA/33
> [ 1320.134780] ata1: EH complete
>
> And hotplug works fine after EH is done with itself. Dunno why.
It works because after EH you've reset the port, and when you reset the
port you turn off ipm (note the value of SControl). I left this the way
it was without attempting to renable link pm after a reset because I figured
if there was something bad happening and we needed to reset we should leave
ipm off.
As far as the failure you are seeing goes - this may not actually be a
hardware problem but just a case of us not understanding which bits we
need to clear out of the Interrupt status register once we enable
ALPM. The value of SErr indicates that this device has gotten PhyRdy -
which is normal for power state changes, and the interrupt status indicates
that a d2h FIS was received which is also normal - the TFES bit seems to be
set - it may be that we need to modify the interrupt code to not only ignore
PhyRdy when ALPM is enabled, but also clear out bit 0 - this is definitely
something we can try. But, meanwhile, please make sure you are testing
with the latest version of the patches.
>
> 3. PLEXTOR DVDR PX-716A
>
> ATAPI CD-ROM, with removable media
> Model Number: PLEXTOR DVDR PX-716A
> Serial Number: 127377
> Firmware Revision: 1.09
> Standards:
> Likely used CD-ROM ATAPI-1
> Configuration:
> DRQ response: 50us.
> Packet size: 12 bytes
> Capabilities:
> LBA, IORDY(can be disabled)
> DMA: mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 *udma4
> Cycle time: min=120ns recommended=120ns
> PIO: pio0 pio1 pio2 pio3 pio4
> Cycle time: no flow control=120ns IORDY flow control=120ns
> HW reset results:
> CBLID- below Vih
> Device num = 0
>
> # echo min_power > link_power_management_policy
> [ 2102.655765] ata1.00: Unable to set Link PM policy
> [ 2104.505926] ata1.00: exception Emask 0x10 SAct 0x0 SErr 0x50000 action 0x2
> [ 2104.527256] ata1.00: irq_stat 0x40000001
> [ 2104.545868] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in
> [ 2104.545870] res 51/24:03:00:00:20/00:00:00:00:00/a0 Emask 0x10 (ATA bus error)
> [ 2106.810252] ata1: soft resetting port
> [ 2106.828106] ata1: SATA link down (SStatus 611 SControl 300)
> [ 2106.847957] ata1: failed to recover some devices, retrying in 5 secs
> [ 2111.870285] ata1: hard resetting port
> [ 2117.401917] ata1: port is slow to respond, please be patient (Status 0x80)
> [ 2121.902365] ata1: COMRESET failed (errno=-16)
> [ 2121.920313] ata1: hard resetting port
> [ 2122.413973] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> [ 2122.795507] ata1.00: configured for UDMA/66
> [ 2122.813234] ata1: EH complete
this looks like the same issue as above.
>
> 4. Harddisks
>
> All the harddisks I have behave themselves. Impressive.
>
> ====
>
> In all cases, hotplug works well even after ALPM is configured. Have
> no idea why. It might be that PARTIAL/SLUMBER mode didn't kick in or
> ICH9 has some magic feature to detect PHY status changes even in PS
> mode (wishful thinking).
Make sure you are using the latest version of the patches - and that
the devices are not resetting themselves for some reason. I don't see
how hotplug can work if we are indeed ignoring PhyRdy.
> With some massging to ahci_disable_alpm(), I think it can be fairly
> safe on this chipset at least.
Not sure what part of disable_alpm you think is broken here - I see
the issue to be more likely in the interrupt handler. I'll
regenerate the patches today against the latest kernel and you
should rerun your tests just to make sure we aren't trying to fix
something that's already been fixed.
Thanks,
Kristen
I was able to duplicate Tejun's problem on an ATAPI device I had here.
this updated patch fixed my problem. These devices are just setting
PhyReady (N) and CommWake (W) in Serror on power state changes, ignoring
them seems to be fine, and fixed the problem for me.
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.3"
+static int ahci_enable_alpm(struct ata_port *ap,
+ enum link_pm policy);
+static int ahci_disable_alpm(struct ata_port *ap);
enum {
AHCI_PCI_BAR = 5,
@@ -98,6 +101,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_SNTF = (1 << 29), /* SNotification register */
HOST_CAP_NCQ = (1 << 30), /* Native Command Queueing */
@@ -153,6 +157,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 */
@@ -244,6 +250,11 @@ static int ahci_pci_device_suspend(struc
static int ahci_pci_device_resume(struct pci_dev *pdev);
#endif
+static struct class_device_attribute *ahci_shost_attrs[] = {
+ &class_device_attr_link_power_management_policy,
+ NULL
+};
+
static struct scsi_host_template ahci_sht = {
.module = THIS_MODULE,
.name = DRV_NAME,
@@ -261,6 +272,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,
+ .shost_attrs = ahci_shost_attrs,
};
static const struct ata_port_operations ahci_ops = {
@@ -292,6 +304,8 @@ static const struct ata_port_operations
.port_suspend = ahci_port_suspend,
.port_resume = ahci_port_resume,
#endif
+ .enable_pm = ahci_enable_alpm,
+ .disable_pm = ahci_disable_alpm,
.port_start = ahci_port_start,
.port_stop = ahci_port_stop,
@@ -778,6 +792,156 @@ 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) | (1 << 18)));
+
+ /*
+ * 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);
+
+ /*
+ * don't change the link pm policy - we can be called
+ * just to turn of link pm temporarily
+ */
+ return 0;
+}
+
+static int ahci_enable_alpm(struct ata_port *ap,
+ enum 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 = NOT_AVAILABLE;
+ return -EINVAL;
+ }
+
+ /* make sure we have a device attached */
+ sstatus = readl(port_mmio + PORT_SCR_STAT);
+ if (!(sstatus & 0xf00)) {
+ ap->pm_policy = NOT_AVAILABLE;
+ return -EINVAL;
+ }
+
+ switch (policy) {
+ case MAX_PERFORMANCE:
+ case NOT_AVAILABLE:
+ /*
+ * if we came here with NOT_AVAILABLE,
+ * it just means this is the first time we
+ * have tried to enable - default to max performance,
+ * and let the user go to lower power modes on request.
+ */
+ ahci_disable_alpm(ap);
+ ap->pm_policy = MAX_PERFORMANCE;
+ return 0;
+ case MIN_POWER:
+ /* configure HBA to enter SLUMBER */
+ asp = PORT_CMD_ASP;
+ break;
+ case MEDIUM_POWER:
+ /* configure HBA to enter PARTIAL */
+ asp = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+ ap->pm_policy = policy;
+
+ /*
+ * 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)
{
@@ -1355,6 +1519,17 @@ static void ahci_port_intr(struct ata_po
status = readl(port_mmio + PORT_IRQ_STAT);
writel(status, port_mmio + PORT_IRQ_STAT);
+ /* If we are getting PhyRdy, this is
+ * just a power state change, we should
+ * clear out this, plus the PhyRdy/Comm
+ * Wake bits from Serror
+ */
+ if ((ap->flags & AHCI_FLAG_NO_HOTPLUG) &&
+ (status & PORT_IRQ_PHYRDY)) {
+ status &= ~PORT_IRQ_PHYRDY;
+ ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
+ }
+
if (unlikely(status & PORT_IRQ_ERROR)) {
ahci_error_intr(ap, status);
return;
@@ -1861,6 +2036,9 @@ static int ahci_init_one(struct pci_dev
struct ata_port *ap = host->ports[i];
void __iomem *port_mmio = ahci_port_base(ap);
+ /* set initial link pm policy */
+ ap->pm_policy = NOT_AVAILABLE;
+
/* standard SATA port setup */
if (hpriv->port_map & (1 << i))
ap->ioaddr.cmd_addr = port_mmio;
On Wed, 1 Aug 2007 14:16:51 -0700
Kristen Carlson Accardi <[email protected]> wrote:
> I was able to duplicate Tejun's problem on an ATAPI device I had here.
> this updated patch fixed my problem. These devices are just setting
> PhyReady (N) and CommWake (W) in Serror on power state changes, ignoring
> them seems to be fine, and fixed the problem for me.
Hi Tejun,
Were you able to test out this patch and see if it solved your ATAPI
device/ALPM issues?
Thanks,
Kristen
>
>
> 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.3"
>
> +static int ahci_enable_alpm(struct ata_port *ap,
> + enum link_pm policy);
> +static int ahci_disable_alpm(struct ata_port *ap);
>
> enum {
> AHCI_PCI_BAR = 5,
> @@ -98,6 +101,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_SNTF = (1 << 29), /* SNotification register */
> HOST_CAP_NCQ = (1 << 30), /* Native Command Queueing */
> @@ -153,6 +157,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 */
> @@ -244,6 +250,11 @@ static int ahci_pci_device_suspend(struc
> static int ahci_pci_device_resume(struct pci_dev *pdev);
> #endif
>
> +static struct class_device_attribute *ahci_shost_attrs[] = {
> + &class_device_attr_link_power_management_policy,
> + NULL
> +};
> +
> static struct scsi_host_template ahci_sht = {
> .module = THIS_MODULE,
> .name = DRV_NAME,
> @@ -261,6 +272,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,
> + .shost_attrs = ahci_shost_attrs,
> };
>
> static const struct ata_port_operations ahci_ops = {
> @@ -292,6 +304,8 @@ static const struct ata_port_operations
> .port_suspend = ahci_port_suspend,
> .port_resume = ahci_port_resume,
> #endif
> + .enable_pm = ahci_enable_alpm,
> + .disable_pm = ahci_disable_alpm,
>
> .port_start = ahci_port_start,
> .port_stop = ahci_port_stop,
> @@ -778,6 +792,156 @@ 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) | (1 << 18)));
> +
> + /*
> + * 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);
> +
> + /*
> + * don't change the link pm policy - we can be called
> + * just to turn of link pm temporarily
> + */
> + return 0;
> +}
> +
> +static int ahci_enable_alpm(struct ata_port *ap,
> + enum 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 = NOT_AVAILABLE;
> + return -EINVAL;
> + }
> +
> + /* make sure we have a device attached */
> + sstatus = readl(port_mmio + PORT_SCR_STAT);
> + if (!(sstatus & 0xf00)) {
> + ap->pm_policy = NOT_AVAILABLE;
> + return -EINVAL;
> + }
> +
> + switch (policy) {
> + case MAX_PERFORMANCE:
> + case NOT_AVAILABLE:
> + /*
> + * if we came here with NOT_AVAILABLE,
> + * it just means this is the first time we
> + * have tried to enable - default to max performance,
> + * and let the user go to lower power modes on request.
> + */
> + ahci_disable_alpm(ap);
> + ap->pm_policy = MAX_PERFORMANCE;
> + return 0;
> + case MIN_POWER:
> + /* configure HBA to enter SLUMBER */
> + asp = PORT_CMD_ASP;
> + break;
> + case MEDIUM_POWER:
> + /* configure HBA to enter PARTIAL */
> + asp = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> + ap->pm_policy = policy;
> +
> + /*
> + * 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)
> {
> @@ -1355,6 +1519,17 @@ static void ahci_port_intr(struct ata_po
> status = readl(port_mmio + PORT_IRQ_STAT);
> writel(status, port_mmio + PORT_IRQ_STAT);
>
> + /* If we are getting PhyRdy, this is
> + * just a power state change, we should
> + * clear out this, plus the PhyRdy/Comm
> + * Wake bits from Serror
> + */
> + if ((ap->flags & AHCI_FLAG_NO_HOTPLUG) &&
> + (status & PORT_IRQ_PHYRDY)) {
> + status &= ~PORT_IRQ_PHYRDY;
> + ahci_scr_write(ap, SCR_ERROR, ((1 << 16) | (1 << 18)));
> + }
> +
> if (unlikely(status & PORT_IRQ_ERROR)) {
> ahci_error_intr(ap, status);
> return;
> @@ -1861,6 +2036,9 @@ static int ahci_init_one(struct pci_dev
> struct ata_port *ap = host->ports[i];
> void __iomem *port_mmio = ahci_port_base(ap);
>
> + /* set initial link pm policy */
> + ap->pm_policy = NOT_AVAILABLE;
> +
> /* standard SATA port setup */
> if (hpriv->port_map & (1 << i))
> ap->ioaddr.cmd_addr = port_mmio;