Message ID | 201106240059.38687.rjw@sisk.pl (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Fri, 24 Jun 2011, Rafael J. Wysocki wrote: > > > I'm still not clear on why the error handler needs to run at this time. > > > > Because SATA ports are suspended with the help of the SCSI error handling > > mechanism (which Tejun claims is the best way to do that). > I've carried out this exercise to see how complicated it is going to be > and it doesn't really seem to be _that_ complicated. The appended patch > illustrates this, but it hasn't been tested, so caveat emptor. The patch is straightforward enough. But will it be sufficient? Suppose a SATA port is already in runtime suspend when the system sleep starts. Will the error handler be able to do its special job? I don't know... It may turn out to be necessary for the SATA port to be runtime resumed somewhere along the line. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday, June 26, 2011, Alan Stern wrote: > On Fri, 24 Jun 2011, Rafael J. Wysocki wrote: > > > > > I'm still not clear on why the error handler needs to run at this time. > > > > > > Because SATA ports are suspended with the help of the SCSI error handling > > > mechanism (which Tejun claims is the best way to do that). > > > I've carried out this exercise to see how complicated it is going to be > > and it doesn't really seem to be _that_ complicated. The appended patch > > illustrates this, but it hasn't been tested, so caveat emptor. > > The patch is straightforward enough. But will it be sufficient? > > Suppose a SATA port is already in runtime suspend when the system sleep > starts. Will the error handler be able to do its special job? I don't > know... It may turn out to be necessary for the SATA port to be > runtime resumed somewhere along the line. That's correct, but at least in the ususal situation (i.e. sdev_gendev is not suspended at run time) the patch makes things work when runtime PM is disabled during system suspend and enabled during system resume. It still may be necessary to add code to the SATA subsystem if sdev_gendev is to be suspended at run time for SATA controllers. I'm not aware of any of such cases, though. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-2.6/drivers/base/power/runtime.c =================================================================== --- linux-2.6.orig/drivers/base/power/runtime.c +++ linux-2.6/drivers/base/power/runtime.c @@ -135,8 +135,9 @@ static int rpm_check_suspend_allowed(str if (dev->power.runtime_error) retval = -EINVAL; - else if (atomic_read(&dev->power.usage_count) > 0 - || dev->power.disable_depth > 0) + else if (dev->power.disable_depth > 0) + retval = -EACCES; + else if (atomic_read(&dev->power.usage_count) > 0) retval = -EAGAIN; else if (!pm_children_suspended(dev)) retval = -EBUSY; @@ -262,7 +263,7 @@ static int rpm_callback(int (*cb)(struct spin_lock_irq(&dev->power.lock); } dev->power.runtime_error = retval; - return retval; + return retval != -EACCES ? retval : -EIO; } /** @@ -458,7 +459,7 @@ static int rpm_resume(struct device *dev if (dev->power.runtime_error) retval = -EINVAL; else if (dev->power.disable_depth > 0) - retval = -EAGAIN; + retval = -EACCES; if (retval) goto out; Index: linux-2.6/drivers/scsi/scsi_pm.c =================================================================== --- linux-2.6.orig/drivers/scsi/scsi_pm.c +++ linux-2.6/drivers/scsi/scsi_pm.c @@ -144,9 +144,9 @@ int scsi_autopm_get_device(struct scsi_d int err; err = pm_runtime_get_sync(&sdev->sdev_gendev); - if (err < 0) + if (err < 0 && err !=-EACCES) pm_runtime_put_sync(&sdev->sdev_gendev); - else if (err > 0) + else err = 0; return err; } @@ -173,9 +173,9 @@ int scsi_autopm_get_host(struct Scsi_Hos int err; err = pm_runtime_get_sync(&shost->shost_gendev); - if (err < 0) + if (err < 0 && err !=-EACCES) pm_runtime_put_sync(&shost->shost_gendev); - else if (err > 0) + else err = 0; return err; }