Re: [PATCH 1/2] scsi: sd: set ready_to_power_off for scsi disk

[Aaron Lu <aaron.lu@intel.com>]

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:
> > On Fri, 2012-09-14 at 16:48 +0800, Aaron Lu wrote:
> > > 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