On 09/07/2012 02:50 AM, Alan Stern wrote:
> On Thu, 6 Sep 2012, Oliver Neukum wrote:
>
>>>>> But in the long run that wouldn't be a good solution. What I'd really
>>>>> like is a way to do the status polling without having it reset the
>>>>> idle timer.
>>>>>
>>>>> Oliver, what do you think? Would that be a good solution?
>>>>
>>>> Well, we could introduce a flag into the requests for the polls.
>>>> But best would be to simply declare a device immediately idle
>>>> as soon as we learn that it has no medium. No special casing
>>>> would be needed.
>>>
>>> We could do that, but what about idle drives that do have media?
>>
>> Then we do have a problem. To handle this optimally we'd have to make
>> a difference between the first time a new medium is noticed and later
>> polls.
>
> That's right. It shouldn't be too difficult to accomplish.
>
> One case to watch out for is a ZPODD with no media and an open door.
> We should put the drive into runtime suspend immediately, but continue
> polling and leave the power on. The runtime suspend after each poll
> will to see if the door is shut; when it is then polling can be turned
> off and power removed.
>
> This leads to a question: How should the sr device tell its parent
> whether or not to turn off the power? Aaron's current patches simply
> rely on the device being runtime suspended, but that won't work with
> this scheme. Do we need a "ready_to_power_down" flag?
Thanks for the suggestions, I've tried to use these ideas and please
take a look to see if below code did the job.
Note that I didn't call disk_block_events in sr_suspend, as there is a
race that I didn't know how to resolve:
sr_suspend sr_check_events
disk_block_events scsi_autopm_get_device
wait for all delayed wait for suspend finish
events checking work
to finish
So it's possible when sr_suspend is executing, sr_check_events is also
executing and disk_block_events will wait for sr_check_events to finish
while sr_check_events waits for this device's suspend routine finish
before the scsi_autopm_get_device can proceed.
I used a flag called powered_off, if it is set, the sr_check_events will
simply return.
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..f91c922 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -869,9 +869,14 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
if (state.event != PM_EVENT_ON) {
acpi_state = acpi_pm_device_sleep_state(
- &dev->sdev->sdev_gendev, NULL, ACPI_STATE_D3);
- if (acpi_state > 0)
+ &dev->sdev->sdev_gendev, NULL,
+ dev->sdev->ready_to_power_off ?
+ ACPI_STATE_D3 : ACPI_STATE_D0);
+ if (acpi_state > 0) {
acpi_bus_set_power(handle, acpi_state);
+ if (acpi_state == ACPI_STATE_D3)
+ dev->sdev->powered_off = 1;
+ }
/* TBD: need to check if it's runtime pm request */
acpi_pm_device_run_wake(
&dev->sdev->sdev_gendev, true);
@@ -880,6 +885,7 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
acpi_pm_device_run_wake(
&dev->sdev->sdev_gendev, false);
acpi_bus_set_power(handle, ACPI_STATE_D0);
+ dev->sdev->powered_off = 0;
}
}
@@ -985,8 +991,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->need_eject = 1;
+ pm_runtime_resume(&ata_dev->sdev->sdev_gendev);
+ }
}
static void ata_acpi_add_pm_notifier(struct ata_device *dev)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..890cee2 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,7 @@
#include <linux/blkdev.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
#include <asm/uaccess.h>
#include <scsi/scsi.h>
@@ -79,6 +80,8 @@ static DEFINE_MUTEX(sr_mutex);
static int sr_probe(struct device *);
static int sr_remove(struct device *);
static int sr_done(struct scsi_cmnd *);
+static int sr_suspend(struct device *, pm_message_t msg);
+static int sr_resume(struct device *);
static struct scsi_driver sr_template = {
.owner = THIS_MODULE,
@@ -86,6 +89,8 @@ static struct scsi_driver sr_template = {
.name = "sr",
.probe = sr_probe,
.remove = sr_remove,
+ .suspend = sr_suspend,
+ .resume = sr_resume,
},
.done = sr_done,
};
@@ -146,8 +151,12 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)
kref_get(&cd->kref);
if (scsi_device_get(cd->device))
goto out_put;
+ if (scsi_autopm_get_device(cd->device))
+ goto out_pm;
goto out;
+ out_pm:
+ scsi_device_put(cd->device);
out_put:
kref_put(&cd->kref, sr_kref_release);
cd = NULL;
@@ -163,9 +172,38 @@ static void scsi_cd_put(struct scsi_cd *cd)
mutex_lock(&sr_ref_mutex);
kref_put(&cd->kref, sr_kref_release);
scsi_device_put(sdev);
+ scsi_autopm_put_device(sdev);
mutex_unlock(&sr_ref_mutex);
}
+static int sr_suspend(struct device *dev, pm_message_t msg)
+{
+ return 0;
+}
+
+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);
+ }
+
+ return 0;
+}
+
static unsigned int sr_get_events(struct scsi_device *sdev)
{
u8 buf[8];
@@ -211,7 +249,7 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
unsigned int clearing, int slot)
{
struct scsi_cd *cd = cdi->handle;
- bool last_present;
+ bool last_present = cd->media_present;
struct scsi_sense_hdr sshdr;
unsigned int events;
int ret;
@@ -220,6 +258,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
if (CDSL_CURRENT != slot)
return 0;
+ if (cd->device->powered_off)
+ return 0;
+
+ scsi_autopm_get_device(cd->device);
+
events = sr_get_events(cd->device);
cd->get_event_changed |= events & DISK_EVENT_MEDIA_CHANGE;
@@ -246,10 +289,9 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
}
if (!(clearing & DISK_EVENT_MEDIA_CHANGE))
- return events;
+ goto out;
do_tur:
/* let's see whether the media is there with TUR */
- last_present = cd->media_present;
ret = scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
/*
@@ -269,8 +311,19 @@ do_tur:
cd->tur_changed = true;
}
+ /* See if we can power off this ZPODD device */
+ if (cd->device->can_power_off) {
+ if (cd->cdi.mask & CDC_CLOSE_TRAY)
+ /* no media for caddy/slot type ODD */
+ cd->device->ready_to_power_off = !cd->media_present;
+ else
+ /* no media and door closed for tray type ODD */
+ cd->device->ready_to_power_off = !cd->media_present &&
+ sshdr.ascq == 0x01;
+ }
+
if (cd->ignore_get_event)
- return events;
+ goto out;
/* check whether GET_EVENT is reporting spurious MEDIA_CHANGE */
if (!cd->tur_changed) {
@@ -287,6 +340,12 @@ do_tur:
cd->tur_changed = false;
cd->get_event_changed = false;
+out:
+ if (cd->media_present && !last_present)
+ pm_runtime_put_noidle(&cd->device->sdev_gendev);
+ else
+ scsi_autopm_put_device(cd->device);
+
return events;
}
@@ -715,9 +774,14 @@ static int sr_probe(struct device *dev)
dev_set_drvdata(dev, cd);
disk->flags |= GENHD_FL_REMOVABLE;
add_disk(disk);
+ disk_events_set_poll_msecs(disk, 5000);
sdev_printk(KERN_DEBUG, sdev,
"Attached scsi CD-ROM %s\n", cd->cdi.name);
+
+ /* enable runtime pm */
+ scsi_autopm_put_device(cd->device);
+
return 0;
fail_put:
@@ -965,6 +1029,9 @@ static int sr_remove(struct device *dev)
{
struct scsi_cd *cd = dev_get_drvdata(dev);
+ /* disable runtime pm */
+ scsi_autopm_get_device(cd->device);
+
blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
del_gendisk(cd->disk);
Thanks,
Aaron
--
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
