Re: [PATCH RESEND] ahci: implement aggressive SATA device sleep support

[Jeff Garzik <jgarzik@pobox.com>]

On 08/07/2012 01:44 PM, Shane Huang wrote:
> @@ -702,6 +708,16 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
>   		}
>   	}
>
> +	/* set aggressive device sleep */
> +	if ((hpriv->cap2 & HOST_CAP2_SDS) &&
> +	    (hpriv->cap2 & HOST_CAP2_SADM) &&
> +	    (link->device->flags & ATA_DFLAG_DEVSLP)) {
> +		if (policy == ATA_LPM_MIN_POWER)
> +			ahci_set_aggressive_devslp(ap, true);
> +		else
> +			ahci_set_aggressive_devslp(ap, false);
> +	}
> +
>   	if (policy == ATA_LPM_MAX_POWER) {
>   		sata_link_scr_lpm(link, policy, false);
>
> @@ -1889,6 +1905,55 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
>   		ahci_kick_engine(ap);
>   }
>
> +static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
> +{
> +	void __iomem *port_mmio = ahci_port_base(ap);
> +	u32 devslp, dm, dito;
> +	int rc;
> +	unsigned int err_mask;
> +
> +	devslp = readl(port_mmio + PORT_DEVSLP);
> +	if (!(devslp & PORT_DEVSLP_DSP)) {
> +		dev_err(ap->host->dev, "port does not support device sleep\n");
> +		return;
> +	}
> +
> +	/* disable device sleep */
> +	if (!sleep) {
> +		writel(devslp & ~PORT_DEVSLP_ADSE, port_mmio + PORT_DEVSLP);
> +		return;
> +	}
> +
> +	/* device sleep was already enabled */
> +	if (devslp & PORT_DEVSLP_ADSE)
> +		return;

Mostly OK.  A question and a comment.

* for the !sleep case, don't writel() if the devslp value is unchanged

* if we are disabling sleep -- a valid case where host & device both 
support it, but policy denies it -- do we need to stop the ahci engine 
as is done in the enabling case?