Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD

[Aaron Lu <aaron.lu@intel.com>]

On Tue, Sep 11, 2012 at 11:30:35AM +0200, Oliver Neukum wrote:
> On Tuesday 11 September 2012 17:24:13 Aaron Lu wrote:
> > On Tue, Sep 11, 2012 at 10:55:44AM +0200, Oliver Neukum wrote:
> > > On Tuesday 11 September 2012 14:44:30 Aaron Lu wrote:
> > > > On Mon, Sep 10, 2012 at 12:45:51PM +0200, Oliver Neukum wrote:
> > > > > On Monday 10 September 2012 17:16:22 Aaron Lu wrote:
> > > > > 
> > > > > > +static int sr_resume(struct device *dev)
> > > > > > +{
> > > > > > +       struct scsi_cd *cd;
> > > > > > +       struct scsi_sense_hdr sshdr;
> > > > > > +
> > > > > > +       cd = dev_get_drvdata(dev);
> > > > > > +
> > > > > > +       if (!cd->device->powered_off)
> > > > > > +               return 0;
> > > > > > +
> > > > > > +       /* get the disk ready */
> > > > > > +       scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
> > > > > > +
> > > > > > +       /* if user wakes up the ODD, eject the tray */
> > > > > > +       if (cd->device->need_eject) {
> > > > > > +               cd->device->need_eject = 0;
> > > > > > +               if (!(cd->cdi.mask & CDC_CLOSE_TRAY))
> > > > > > +                       sr_tray_move(&cd->cdi, 1);
> > > > > 
> > > > > 1. Even if the door is locked?
> > > > 
> > > > This piece of code of sr_resume is intended for ZPODD devices, for
> > > > normal devices it will simply return as the powered_off flag will never
> > > > be set for them.
> > > > 
> > > > And for ZPODD devices, the door shouldn't be in locked state here since
> > > > for it to enter power off state, "no medium inside" has to be satisfied
> > > 
> > > This is true only if the device is autosuspended. But sr_resume()
> > > will also be called if the whole system was suspended.
> > 
> > You mean when system resumes, right?
> 
> Yes, but because the whole system had been suspended.
> In that case you can have a locked door.
> 
> > > What happens if the user presses the door button on the drive
> > > while the system was suspended?
> > 
> > The drive does not have the capability to wake up the system from S3, so
> > nothing happens.
> 
> You are assuming that the system is resumed because the door button is pressed.
> That need not be the case. What happens if the door button is pressed while
> the system is resuming?

I think a little more description about sr_resume is required.

For non-ZPODD devices, sr_resume will always do nothing no matter it is
called due to system resume or runtime resume since the powered_off flag
will never be set.

And for ZPODD devices, when powered_off flag is set, it will check if
this resume is caused by user pressing the eject button, this is reflected
by the need_eject flag. This flag will only be set if an ACPI gpe event
is received and the event is generated by user pressing the eject button
when the ODD is in powered off state and the system is in S0 state. If
this flag is set, the tray will be ejected for tray type ODD; and if it
is not set, nothing is done there.

So only when the system is in S0, ODD is in powered off state and user
pressed the eject button will the tray be ejected on resume. Is there a
problem with this?

Thanks,
Aaron