| Message ID | 1456359748-22838-3-git-send-email-dbasehore@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Feb 24, 2016 at 04:22:28PM -0800, Derek Basehore wrote: > This allows scsi devices to remain runtime suspended for system > suspend. Since runtime suspend is stricter than system suspend > callbacks, this is just returning a positive number for the prepare > callback. AFAICT SCSI layer already leaves devices runtime suspended during system suspend (see scsi_bus_suspend_common()). What's the benefit using direct_complete over the current implementation? > Signed-off-by: Derek Basehore <dbasehore@chromium.org> > Reviewed-by: Eric Caruso <ejcaruso@chromium.org> > --- > drivers/scsi/scsi_pm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c > index b44c1bb..7af76ad 100644 > --- a/drivers/scsi/scsi_pm.c > +++ b/drivers/scsi/scsi_pm.c > @@ -178,7 +178,7 @@ static int scsi_bus_prepare(struct device *dev) > /* Wait until async scanning is finished */ > scsi_complete_async_scans(); > } > - return 0; > + return 1; > } > > static int scsi_bus_suspend(struct device *dev) > -- > 2.7.0.rc3.207.g0ac5344 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
A device is not able to use direct complete if its children do not also use direct complete. Even though the SCSI layer leaves devices runtime suspended, the way it does it still prevents its parent from using direct complete. On Fri, Feb 26, 2016 at 11:26 PM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Wed, Feb 24, 2016 at 04:22:28PM -0800, Derek Basehore wrote: >> This allows scsi devices to remain runtime suspended for system >> suspend. Since runtime suspend is stricter than system suspend >> callbacks, this is just returning a positive number for the prepare >> callback. > > AFAICT SCSI layer already leaves devices runtime suspended during system > suspend (see scsi_bus_suspend_common()). What's the benefit using > direct_complete over the current implementation? > >> Signed-off-by: Derek Basehore <dbasehore@chromium.org> >> Reviewed-by: Eric Caruso <ejcaruso@chromium.org> >> --- >> drivers/scsi/scsi_pm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c >> index b44c1bb..7af76ad 100644 >> --- a/drivers/scsi/scsi_pm.c >> +++ b/drivers/scsi/scsi_pm.c >> @@ -178,7 +178,7 @@ static int scsi_bus_prepare(struct device *dev) >> /* Wait until async scanning is finished */ >> scsi_complete_async_scans(); >> } >> - return 0; >> + return 1; >> } >> >> static int scsi_bus_suspend(struct device *dev) >> -- >> 2.7.0.rc3.207.g0ac5344 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 27, 2016 at 12:10:03AM -0800, dbasehore . wrote: > A device is not able to use direct complete if its children do not > also use direct complete. Even though the SCSI layer leaves devices > runtime suspended, the way it does it still prevents its parent from > using direct complete. Okay. Do you need to provide ->complete() hook that then resumes the device when system is resumed (since the device resume hook is never called when direct_complete is set)? Along the lines of pm_complete_with_resume_check(). -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 27, 2016 at 12:38 AM, Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > On Sat, Feb 27, 2016 at 12:10:03AM -0800, dbasehore . wrote: >> A device is not able to use direct complete if its children do not >> also use direct complete. Even though the SCSI layer leaves devices >> runtime suspended, the way it does it still prevents its parent from >> using direct complete. > > Okay. > > Do you need to provide ->complete() hook that then resumes the device > when system is resumed (since the device resume hook is never called > when direct_complete is set)? Along the lines of > pm_complete_with_resume_check(). That's an interesting question. Part of direct complete is to leave the device runtime suspended even after the system resumes if possible. The comments in pm_complete_with_resume_check indicate that the firmware may resume a device. I could see this happening with some kind of SCSI device. If this is possible, are we able to put the device back into a consistent state (runtime suspended) by calling suspend for the scsi device? If not, we might need to use pm_complete_with_resume_check for the complete callback. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 27, 2016 at 01:07:06AM -0800, dbasehore . wrote: > That's an interesting question. Part of direct complete is to leave > the device runtime suspended even after the system resumes if > possible. The comments in pm_complete_with_resume_check indicate that > the firmware may resume a device. I could see this happening with some > kind of SCSI device. > > If this is possible, are we able to put the device back into a > consistent state (runtime suspended) by calling suspend for the scsi > device? If not, we might need to use pm_complete_with_resume_check for > the complete callback. To me the latter option sounds safer. I tried this series (as I'm interested in AHCI host controller runtime PM) and noticed possible issue. I have following two additional patches from linux-next applied to get system resume working (otherwise resume is never finished): d07ab6d11477 block: Add blk_set_runtime_active() 356fd2663cff scsi: Set request queue runtime PM status back to active on resume Then I do this: # echo auto > /sys/block/sda/device/power/control # echo 5000 > /sys/block/sda/device/power/autosuspend_delay_ms # echo auto > /sys/bus/pci/devices/0000\:00\:12.0/ata1/power/control The last line unblocks runtime PM from the parent device. After 5 seconds of idle I see that the disk gets runtime suspended: [ 566.858971] sd 0:0:0:0: [sda] Synchronizing SCSI cache [ 567.049288] sd 0:0:0:0: [sda] Stopping disk Now suspend the machine: # rtcwake -s10 -mmem Once the system resumes I get this: [ 668.879149] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 668.927319] ata1.00: configured for UDMA/133 [ 669.392246] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 669.440427] ata1.00: configured for UDMA/133 [ 669.445352] sd 0:0:0:0: [sda] Starting disk [ 669.464647] sda: detected capacity change from 0 to 240057409536 [ 669.482596] sda: detected capacity change from 0 to 240057409536 It looks like we get resumed two times. If I have this patch reverted I can see it happening only once. Also with the patch applied and if the parent device has runtime PM blocked, it happens only once. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index b44c1bb..7af76ad 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -178,7 +178,7 @@ static int scsi_bus_prepare(struct device *dev) /* Wait until async scanning is finished */ scsi_complete_async_scans(); } - return 0; + return 1; } static int scsi_bus_suspend(struct device *dev)