Hi Bart,
> > - 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);
>
> So we have a single new struct member, rpm_autosuspend_delay, that
> controls two different behaviors: (a) whether or not runtime suspend is
> enabled at device creation time and (b) the power management autosuspend
> delay. I don't like this. Should two separate variables be introduced
> instead of using a single variable to control both behaviors?
>
OK! Will try to separate different variables to control different things
in next version.
> > 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;
> > atomic_t disk_events_disable_depth; /* disable depth for disk events */
> >
> > DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events
> > */
> >
>
> Since the default value for the autosuspend delay is the same for all
> SCSI devices attached to a SCSI host is the same, please add a variable
> with the same name in the SCSI host template and use that value as the
> default value for SCSI devices. If the rpm_autosuspend_delay variable
> only occurs in struct scsi_device then LLD authors are forced to
> introduce a slave_configure function. Introducing such a function can be
> avoided if the default autosuspend delay can be specified in the host
> template.
>
Sounds reasonable. Will create a member indicating autosuspend delay for
the same SCSI host in SCSI host template in next version.
> Bart.
>
Thanks,
Stanley
