On Fri, Sep 14, 2012 at 09:54:16PM +0800, Aaron Lu wrote:

On Fri, Sep 14, 2012 at 11:26:29AM +0100, James Bottomley wrote:

quoted

On Fri, 2012-09-14 at 16:48 +0800, Aaron Lu wrote:

quoted

So if we program the device to let it enter standby/stopped power
condition with the start_stop_unit command, do we need to sync the
cache?

No, that's what the spec says.  The device must manage the cache in both
the forced (start stop unit) and timed (power control mode page) cases.

The reason is the spec doesn't define what idle and standby actually
mean (just that they're "lower" power states).  So the device
implementers get to choose if they stop the platter or power off the
motor.  The spec just means that if they do anything that causes danger
to data in the cache, they have to deal with it themselves.

Thanks for the clear explanation.

So what about the following change? In sd_suspend, if device supports
start_stop command, then we just need issue this command then both
runtime suspend case and system S3/S4 case are OK, since when the device
enters stopped power condition, the internal cache should be taken care
of by the device and it is also ready to be powered off. And if device

This is not the case for ata device, so scsi stop command should
translate to 2 ata commands: flush cache + enter standby.

If flush cache is skipped in sd driver, then ata scsi translate will
need to update to address this issue.

A possible change(not tested) like this:

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 8ec81ca..2de5fac 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c

@@ -1759,6 +1759,19 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 	ata_qc_free(qc);
 }
 
+static int ata_flush_cache(struct ata_device *dev)
+{
+	u8 cmd;
+
+	if (dev->flags & ATA_DFLAG_FLUSH_EXT)
+		cmd = ATA_CMD_FLUSH_EXT;
+	else
+		cmd = ATA_CMD_FLUSH;
+
+	return ata_do_simple_cmd(dev, cmd);
+}
+
+
 /**
  *	ata_scsi_translate - Translate then issue SCSI command to ATA device
  *	@dev: ATA device to which the command is addressed

@@ -1816,6 +1829,13 @@ static int ata_scsi_translate(struct ata_device *dev, struct scsi_cmnd *cmd,
 	if (xlat_func(qc))
 		goto early_finish;
 
+	/* scsi stop cmd = flush cache + standby */
+	if (qc->tf.command == ATA_CMD_STANDBYNOW1 && ata_try_flush_cache(dev)) {
+		rc = ata_flush_cache(dev);
+		if (rc)
+			goto err_did;
+	}
+
 	if (ap->ops->qc_defer) {
 		if ((rc = ap->ops->qc_defer(qc)))
 			goto defer;

Thanks,
Aaron