> > Rework from previous work by: > Sujit Reddy Thumma <sthumma@xxxxxxxxxxxxxx> > > Until now the scsi mid-layer forbids runtime suspend till userspace enables it. > This is mainly to quarantine some disks with broken runtime power > management or have high latencies executing suspend resume callbacks. If > the userspace doesn't enable the runtime suspend the underlying hardware > will be always on even when it is not doing any useful work and thus wasting > power. > > Some low-level drivers for the controllers can efficiently use runtime power > management to reduce power consumption and improve battery life. > Allow runtime suspend parameters override within the LLD itself instead of > waiting for userspace to control the power management. > > Signed-off-by: Stanley Chu <stanley.chu@xxxxxxxxxxxx> Reviewed-by: Avri Altman <avri.altman@xxxxxxx> Stanley hi, Your series looks fine. I added some comments/questions - feel free to ignore it. Thanks, Avri > --- > drivers/scsi/scsi_sysfs.c | 3 ++- > drivers/scsi/sd.c | 3 +++ > include/scsi/scsi_device.h | 2 +- > 3 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index > 64c96c7828ee..66a8a5c74352 100644 > --- a/drivers/scsi/scsi_sysfs.c > +++ b/drivers/scsi/scsi_sysfs.c > @@ -1300,7 +1300,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev) > device_enable_async_suspend(&sdev->sdev_gendev); > scsi_autopm_get_target(starget); > pm_runtime_set_active(&sdev->sdev_gendev); > - pm_runtime_forbid(&sdev->sdev_gendev); > + if (sdev->rpm_autosuspend_delay <= 0) > + pm_runtime_forbid(&sdev->sdev_gendev); > pm_runtime_enable(&sdev->sdev_gendev); > scsi_autopm_put_target(starget); > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index > 149d406aacc9..2218d57c4c0c 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -3371,6 +3371,9 @@ static int sd_probe(struct device *dev) > } > > blk_pm_runtime_init(sdp->request_queue, dev); > + if (sdp->rpm_autosuspend_delay > 0) > + pm_runtime_set_autosuspend_delay(dev, > + Redundant line ? > + sdp->rpm_autosuspend_delay); Don't you need to call now pm_runtime_use_autosuspend() ? > device_add_disk(dev, gd, NULL); > if (sdkp->capacity) > sd_dif_config_host(sdkp); diff --git a/include/scsi/scsi_device.h > b/include/scsi/scsi_device.h index 202f4d6a4342..133b282fae5a 100644 > --- a/include/scsi/scsi_device.h > +++ b/include/scsi/scsi_device.h > @@ -199,7 +199,7 @@ struct scsi_device { > unsigned broken_fua:1; /* Don't set FUA bit */ > unsigned lun_in_cdb:1; /* Store LUN bits in CDB[1] */ > unsigned unmap_limit_for_ws:1; /* Use the UNMAP limit for WRITE > SAME */ > - > + int rpm_autosuspend_delay; Can suspend be negative? > atomic_t disk_events_disable_depth; /* disable depth for disk events */ > > DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* > supported events */ > -- > 2.18.0