On 09/22/2012 05:02 AM, Rafael J. Wysocki wrote: > On Friday, September 21, 2012, Aaron Lu wrote: >> On Fri, Sep 21, 2012 at 12:07:23AM +0200, Rafael J. Wysocki wrote: >>> On Wednesday, September 12, 2012, Aaron Lu wrote: >>>> 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. >> >>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html