On Tue, 4 Sep 2012, Oliver Neukum wrote:
> On Tuesday 04 September 2012 22:24:34 Aaron Lu wrote:
> > From: Aaron Lu <aaron.lu@xxxxxxxxx>
> >
> > The ODD will be placed into suspend state when:
> > 1 For tray type ODD, no media inside and door closed;
> > 2 For slot type ODD, no media inside;
> > And together with ACPI, when we suspend the ODD's parent(the port it
> > attached to), we will omit the power altogether to reduce power
> > consumption(done in libata-acpi.c).
>
> Well, this is quite a layering violation. You encode that specific requirement
> in the generic sr_suspend()
What requirement are you talking about? The "no media and door closed"
thing? How is that a layering violation? Are you suggesting this
should go into the CD layer instead?
> > @@ -985,8 +985,10 @@ static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
> > struct ata_device *ata_dev = context;
> >
> > if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev &&
> > - pm_runtime_suspended(&ata_dev->sdev->sdev_gendev))
> > - scsi_autopm_get_device(ata_dev->sdev);
> > + pm_runtime_suspended(&ata_dev->sdev->sdev_gendev)) {
> > + ata_dev->sdev->wakeup_by_user = 1;
>
> That flag is badly named. Something like "insert_event_during_suspend"
> would be better.
What happens on non-ACPI systems when a new disc is inserted into a
suspended ODD? How does the drive let the computer know that an insert
event has occurred?
> > + if (cd->cdi.mask & CDC_CLOSE_TRAY)
> > + /* no media for caddy/slot type ODD */
> > + suspend = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a;
> > + else
> > + /* no media and door closed for tray type ODD */
> > + suspend = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a &&
> > + sshdr.ascq == 0x01;
>
> That requirement is valid for a specific type of disk. You cannot put it
> into generic sd_suspend().
You mean sr_suspend(). What's not generic about it?
> And even so I don't see why you wouldn't
> want to suspend for example a drive with an inserted but unopened disk.
I assume that Aaron wanted to handle the easiest case first. Adding
suspend/resume handling to the open/close routines can be done later.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
