Hi Alan,
On 2020-11-21 00:35, Alan Stern wrote:
On Fri, Nov 20, 2020 at 12:37:22AM -0800, Can Guo wrote:
Runtime resume is handled by runtime PM framework, no need to forcibly
set runtime PM status to RPM_ACTIVE during system resume/thaw/restore.
Sorry, I don't understand this explanation at all.
Sure, runtime resume is handled by the runtime PM framework. But this
patch changes the code for system resume, which is completely
different.
Following a system resume, the hardware will be at full power. We
don't
want the kernel to think that the device is still in runtime suspend;
otherwise is would never put the device back into low-power mode.
How about adding below lines to the patch?
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 908f27f..7ebe582 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -75,9 +75,11 @@ static int scsi_dev_type_resume(struct device *dev,
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm :
NULL;
int err = 0;
- err = cb(dev, pm);
- scsi_device_resume(to_scsi_device(dev));
- dev_dbg(dev, "scsi resume: %d\n", err);
+ if (pm_runtime_active(dev)) {
+ err = cb(dev, pm);
+ scsi_device_resume(to_scsi_device(dev));
+ dev_dbg(dev, "scsi resume: %d\n", err);
+ }
return err;
}
Whenever a device is accessed, the issuer or somewhere in the path
should do something like pm_runtime_get_sync (e.g. in sg_open()) or
pm_runtime_resume() (e.g. in blk_queue_enter()), in either sync or
async way. After the job (read/write/ioctl or whatever) is done,
either a pm_runtime_put_sync() or auto runtime suspend puts the device
back into runtime suspended/low-power mode. Since the func
scsi_bus_suspend_common() does nothing if device is already in runtime
suspended mode, scsi_dev_type_resume() should only resume the device
if it is runtime active.
Thanks,
Can Guo.
Alan Stern
Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
Cc: Bart Van Assche <bvanassche@xxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Can Guo <cang@xxxxxxxxxxxxxx>
---
Changes since v1:
- Incorporated Bart's comments
---
drivers/scsi/scsi_pm.c | 24 +-----------------------
1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 3717eea..908f27f 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -79,25 +79,6 @@ static int scsi_dev_type_resume(struct device *dev,
scsi_device_resume(to_scsi_device(dev));
dev_dbg(dev, "scsi resume: %d\n", err);
- if (err == 0) {
- pm_runtime_disable(dev);
- err = pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
- /*
- * Forcibly set runtime PM status of request queue to "active"
- * to make sure we can again get requests from the queue
- * (see also blk_pm_peek_request()).
- *
- * The resume hook will correct runtime PM status of the disk.
- */
- if (!err && scsi_is_sdev_device(dev)) {
- struct scsi_device *sdev = to_scsi_device(dev);
-
- blk_set_runtime_active(sdev->request_queue);
- }
- }
-
return err;
}
@@ -165,11 +146,8 @@ static int scsi_bus_resume_common(struct device
*dev,
*/
if (strncmp(scsi_scan_type, "async", 5) != 0)
async_synchronize_full_domain(&scsi_sd_pm_domain);
- } else {
- pm_runtime_disable(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
}
+
return 0;
}
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project.
