On 09/22/2012 05:02 AM, Rafael J. Wysocki wrote:

On Friday, September 21, 2012, Aaron Lu wrote:

quoted

On Fri, Sep 21, 2012 at 12:07:23AM +0200, Rafael J. Wysocki wrote:

quoted

On Wednesday, September 12, 2012, Aaron Lu wrote:

quoted

 static struct scsi_driver sr_template = {
 	.owner			= THIS_MODULE,

@@ -87,6 +89,8 @@ static struct scsi_driver sr_template = {
 		.name   	= "sr",
 		.probe		= sr_probe,
 		.remove		= sr_remove,
+		.suspend        = sr_suspend,
+		.resume         = sr_resume,
 	},
 	.done			= sr_done,
 };

@@ -172,6 +176,52 @@ static void scsi_cd_put(struct scsi_cd *cd)
 	mutex_unlock(&sr_ref_mutex);
 }

Besides, I need some help to understand how this is supposed to work.

Do I think correctly that sr_suspend(), for example, will be run by the
SCSI bus type layer in case of a CD device runtime suspend?  However,

Yes.

quoted

won't this routine be used during system suspend as well and won't it cause
problems to happen if so?

On system suspend, nothing needs to be done.
I'll add the following code in next version.

	if (!PMSG_IS_AUTO(msg))
		return 0;

Please don't.  The pm_message_t thing is obsolete and shoulnd't really be
used by device drivers.  I know that ATA relies on it internally, but that's
just something that needs to be changed at one point.

Moreover, I'd like to migrate SCSI drivers to the PM handling based on struct
dev_pm_ops eventually and your change is kind of going in the opposite
direction.  I don't know how much effort the migration is going to take,
though, so perhaps we can just make this change first.

Does the following change look OK?

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index dc0ad85..1fb7ccc 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c

@@ -143,7 +143,15 @@ static int scsi_runtime_suspend(struct device *dev)
 
 	dev_dbg(dev, "scsi_runtime_suspend\n");
 	if (scsi_is_sdev_device(dev)) {
-		err = scsi_dev_type_suspend(dev, PMSG_AUTO_SUSPEND);
+		err = scsi_device_quiesce(to_scsi_device(dev));
+		if (err)
+			goto out;
+
+		err = pm_generic_runtime_suspend(dev);
+		if (!err)
+			goto out;
+
+		scsi_device_resume(to_scsi_device(dev));
 		if (err == -EAGAIN)
 			pm_schedule_suspend(dev, jiffies_to_msecs(
 				round_jiffies_up_relative(HZ/10)));

@@ -151,6 +159,7 @@ static int scsi_runtime_suspend(struct device *dev)
 
 	/* Insert hooks here for targets, hosts, and transport classes */
 
+out:
 	return err;
 }
 

@@ -159,11 +168,17 @@ static int scsi_runtime_resume(struct device *dev)
 	int err = 0;
 
 	dev_dbg(dev, "scsi_runtime_resume\n");
-	if (scsi_is_sdev_device(dev))
+	if (scsi_is_sdev_device(dev)) {
+		err = pm_generic_runtime_resume(dev);
+		if (err)
+			goto out;
+
 		err = scsi_dev_type_resume(dev);
+	}
 
 	/* Insert hooks here for targets, hosts, and transport classes */
 
+out:
 	return err;
 }
 

And I'll define runtime callbacks for sr and sd.

Thanks,
Aaron