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;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
