Subject: Re: [PATCH v2 1/3] scsi: core: allow auto suspend override by low-level driver

From: Stanley Chu <[email-masked]>
Date: Mon, 16 Sep 2019 14:38:25 +0800

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

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel