On 02/06/2013 12:51 AM, Alan Stern wrote: > On Tue, 5 Feb 2013, Aaron Lu wrote: > >> From: Lin Ming <ming.m.lin@xxxxxxxxx> >> >> Uses block layer runtime pm helper functions in >> scsi_runtime_suspend/resume for devices that take advantage of it. >> >> Remove scsi_autopm_* from sd open/release path and check_events path. > > One thing is a little odd here... > >> --- a/drivers/scsi/scsi_pm.c >> +++ b/drivers/scsi/scsi_pm.c >> @@ -144,33 +144,78 @@ static int scsi_bus_restore(struct device *dev) >> >> #ifdef CONFIG_PM_RUNTIME >> >> +static int sdev_blk_runtime_suspend(struct scsi_device *sdev, >> + int (*cb)(struct device *)) >> +{ >> + int err; >> + >> + err = blk_pre_runtime_suspend(sdev->request_queue); >> + if (err) >> + return err; >> + if (cb) >> + err = cb(&sdev->sdev_gendev); >> + blk_post_runtime_suspend(sdev->request_queue, err); >> + >> + return err; >> +} >> + >> +static int sdev_runtime_suspend(struct device *dev) >> +{ >> + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; >> + int (*cb)(struct device *) = pm ? pm->runtime_suspend : NULL; >> + struct scsi_device *sdev = to_scsi_device(dev); >> + >> + if (sdev->request_queue->dev) >> + return sdev_blk_runtime_suspend(sdev, cb); >> + else >> + return scsi_dev_type_suspend(dev, cb); >> +} >> + >> static int scsi_runtime_suspend(struct device *dev) >> { >> int err = 0; >> - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; >> >> dev_dbg(dev, "scsi_runtime_suspend\n"); >> - if (scsi_is_sdev_device(dev)) { >> - err = scsi_dev_type_suspend(dev, >> - pm ? pm->runtime_suspend : NULL); >> - if (err == -EAGAIN) >> - pm_schedule_suspend(dev, jiffies_to_msecs( >> - round_jiffies_up_relative(HZ/10))); >> - } >> + if (scsi_is_sdev_device(dev)) >> + err = sdev_runtime_suspend(dev); > > The "if (err == -EAGAIN)" test and the call to pm_schedule_suspend seem > to have been dropped since v8 of this series. It looks like they ought > to be moved into sdev_runtime_suspend. I thought they were no longer needed... For sd, we have request based rutime PM and the PM core will always try to autosuspend the device with the timer; and for sr, the poll will trigger suspend constantly. And for both, we don't return -EAGAIN anyway. So I suppose that code is not necessary? BTW, I'll be on vocation till 02/17, and I don't have access to the internet in my hometown, but please feel free to drop any comments and I'll check them when I get back. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html