On Thu, Oct 11, 2012 at 10:57:26AM -0400, Alan Stern wrote:
> I have a couple of small changes to suggest. Nothing major.
>
> On Thu, 11 Oct 2012, Aaron Lu wrote:
>
> > @@ -102,26 +77,87 @@ static int scsi_bus_prepare(struct device *dev)
> >
> > static int scsi_bus_suspend(struct device *dev)
> > {
> > - return scsi_bus_suspend_common(dev, PMSG_SUSPEND);
> > + int err = 0;
> > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> > +
> > + if (scsi_is_sdev_device(dev)) {
> > + /*
> > + * sd is the only high-level SCSI driver to implement runtime
> > + * PM, and sd treats runtime suspend, system suspend, and
> > + * system hibernate identically.
> > + */
>
> It would be better if we don't refer to sd specifically. The comment
> could be changed to something like this:
>
> /*
> * All the high-level SCSI drivers that implement runtime PM
> * treat runtime suspend, system suspend, and system
> * hibernate identically.
> */
OK, will do this.
>
> > static int scsi_bus_freeze(struct device *dev)
> > {
> > - return scsi_bus_suspend_common(dev, PMSG_FREEZE);
> > + int err = 0;
> > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> > +
> > + if (scsi_is_sdev_device(dev)) {
> > + /* wake up device so that FREEZE will succeed */
> > + if (pm_runtime_suspended(dev))
> > + pm_runtime_resume(dev);
>
> I'm not sure why this is here. If none of the high-level drivers
> implement FREEZE then it's not needed.
Agree.
I should have checked the git log to see why it is here.
>From the log, it doesn't seem to have a reason.
Thanks for pointing this out.
>
>
> > static int scsi_bus_poweroff(struct device *dev)
> > {
> > - return scsi_bus_suspend_common(dev, PMSG_HIBERNATE);
> > + int err = 0;
> > + const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> > +
> > + if (scsi_is_sdev_device(dev)) {
> > + /*
> > + * sd is the only high-level SCSI driver to implement runtime
> > + * PM, and sd treats runtime suspend, system suspend, and
> > + * system hibernate identically.
> > + */
>
> Same change as above for this comment.
OK.
And thanks for reviewing.
-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
