Patchwork [v3,7/7,SCSI] sr: adds Zero-power ODD support

login
register
mail settings
Submitter Lin Ming
Date March 28, 2012, 6:22 a.m.
Message ID <1332915722-15963-8-git-send-email-ming.m.lin@intel.com>
Download mbox | patch
Permalink /patch/149149/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Lin Ming - March 28, 2012, 6:22 a.m.
ZPODD(Zero Power Optical Disk Drive) is a new feature in
SATA 3.1 specification. It provides a way to power off unused ODD.

ZPODD support is checked in in sr_probe().
can_power_off flag is set during suspend if ZPODD is supported.

ATA port's runtime suspend callback will actually power off the ODD
and its runtime resume callback will actually power on the ODD.

When ODD is powered off(D3Cold state), inserting disk will trigger a
wakeup event(GPE). GPE AML handler notifies the associated device. Then
ODD is resumed in the notify handler.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/ata/libata-acpi.c  |    8 +++++++-
 drivers/scsi/scsi_pm.c     |    8 ++++++++
 drivers/scsi/sr.c          |   44 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/sr.h          |    2 ++
 include/scsi/scsi_device.h |    2 ++
 5 files changed, 63 insertions(+), 1 deletions(-)
Alan Stern - March 28, 2012, 2:33 p.m.
On Wed, 28 Mar 2012, Lin Ming wrote:

> ZPODD(Zero Power Optical Disk Drive) is a new feature in
> SATA 3.1 specification. It provides a way to power off unused ODD.
> 
> ZPODD support is checked in in sr_probe().
> can_power_off flag is set during suspend if ZPODD is supported.
> 
> ATA port's runtime suspend callback will actually power off the ODD
> and its runtime resume callback will actually power on the ODD.
> 
> When ODD is powered off(D3Cold state), inserting disk will trigger a
> wakeup event(GPE). GPE AML handler notifies the associated device. Then
> ODD is resumed in the notify handler.

> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c

> @@ -80,12 +82,38 @@ 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 *dev, pm_message_t mesg)
> +{
> +	struct scsi_cd *cd;
> +
> +	cd = dev_get_drvdata(dev);
> +	if (cd->device->power_off)
> +		dev->power.subsys_data->can_power_off = true;
> +
> +	return 0;
> +}
> +
> +static int sr_resume(struct device *dev)
> +{
> +	struct scsi_cd *cd;
> +
> +	cd = dev_get_drvdata(dev);
> +	if (cd->device->power_off) {
> +		dev->power.subsys_data->can_power_off = false;
> +		cd->poweroff_event = 0;
> +	}
> +
> +	return 0;
> +}

Calling this flag "can_power_off" makes these routines look very 
strange.  Either the device can power off or it can't, i.e., either it 
supports ZPODD or it doesn't.  This doesn't change over time.

If you rename the flag "may_power_off" then its meaning will be more 
clear.

> @@ -216,6 +244,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>  	unsigned int events;
>  	int ret;
>  
> +	/* Not necessary to check events if enter ZPODD state */
> +	if (cd->device->power_off &&
> +			pm_runtime_suspended(&cd->device->sdev_gendev))
> +		return 0;

The comment is wrong and the new code does the wrong thing.  You _do_
have to check for events even in the ZPODD state, which means
sr_check_events must power-up the device if necessary.  
sd_check_events in James Bottomley's scsi-misc tree now does the right
thing; see commit 4e2247b2bd289f079349d6c69755f8cff4e31f2b.

> +
>  	/* no changer support */
>  	if (CDSL_CURRENT != slot)
>  		return 0;
> @@ -260,6 +293,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>  	cd->media_present = scsi_status_is_good(ret) ||
>  		(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
>  
> +	if (!cd->media_present && cd->device->power_off && !cd->poweroff_event) {
> +		scsi_autopm_put_device(cd->device);

You can see your mistake here.  You call scsi_autopm_put_device here
without calling scsi_autopm_get_device earlier.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lin Ming - March 29, 2012, 5:45 a.m.
On Wed, Mar 28, 2012 at 10:33 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 28 Mar 2012, Lin Ming wrote:
>
>> ZPODD(Zero Power Optical Disk Drive) is a new feature in
>> SATA 3.1 specification. It provides a way to power off unused ODD.
>>
>> ZPODD support is checked in in sr_probe().
>> can_power_off flag is set during suspend if ZPODD is supported.
>>
>> ATA port's runtime suspend callback will actually power off the ODD
>> and its runtime resume callback will actually power on the ODD.
>>
>> When ODD is powered off(D3Cold state), inserting disk will trigger a
>> wakeup event(GPE). GPE AML handler notifies the associated device. Then
>> ODD is resumed in the notify handler.
>
>> --- a/drivers/scsi/sr.c
>> +++ b/drivers/scsi/sr.c
>
>> @@ -80,12 +82,38 @@ 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 *dev, pm_message_t mesg)
>> +{
>> +     struct scsi_cd *cd;
>> +
>> +     cd = dev_get_drvdata(dev);
>> +     if (cd->device->power_off)
>> +             dev->power.subsys_data->can_power_off = true;
>> +
>> +     return 0;
>> +}
>> +
>> +static int sr_resume(struct device *dev)
>> +{
>> +     struct scsi_cd *cd;
>> +
>> +     cd = dev_get_drvdata(dev);
>> +     if (cd->device->power_off) {
>> +             dev->power.subsys_data->can_power_off = false;
>> +             cd->poweroff_event = 0;
>> +     }
>> +
>> +     return 0;
>> +}
>
> Calling this flag "can_power_off" makes these routines look very
> strange.  Either the device can power off or it can't, i.e., either it
> supports ZPODD or it doesn't.  This doesn't change over time.
>
> If you rename the flag "may_power_off" then its meaning will be more
> clear.

OK, will rename it.

>
>> @@ -216,6 +244,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>>       unsigned int events;
>>       int ret;
>>
>> +     /* Not necessary to check events if enter ZPODD state */
>> +     if (cd->device->power_off &&
>> +                     pm_runtime_suspended(&cd->device->sdev_gendev))
>> +             return 0;
>
> The comment is wrong and the new code does the wrong thing.  You _do_
> have to check for events even in the ZPODD state, which means
> sr_check_events must power-up the device if necessary.
> sd_check_events in James Bottomley's scsi-misc tree now does the right
> thing; see commit 4e2247b2bd289f079349d6c69755f8cff4e31f2b.

The problem is userspace(GNOME, for example) will check for events
every seconds.
If sr is power up so frequently then we lost the expected power
savings from ZPODD.

There are 2 events:

DISK_EVENT_MEDIA_CHANGE
DISK_EVENT_EJECT_REQUEST

In current implementation, if sr is in ZPODD state, then it means
there is no disk in the CDROM.
So if sr is in ZPODD state, MEDIA_CHANGE would never happen.

EJECT_REQUEST seems can be ignored, since there is no disk in the CDROM at all.

>
>> +
>>       /* no changer support */
>>       if (CDSL_CURRENT != slot)
>>               return 0;
>> @@ -260,6 +293,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>>       cd->media_present = scsi_status_is_good(ret) ||
>>               (scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
>>
>> +     if (!cd->media_present && cd->device->power_off && !cd->poweroff_event) {
>> +             scsi_autopm_put_device(cd->device);
>
> You can see your mistake here.  You call scsi_autopm_put_device here
> without calling scsi_autopm_get_device earlier.

Let me check this.

Thanks for the review.
Lin Ming

>
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaron Lu - March 29, 2012, 1:50 p.m.
Hi,

On Thu, Mar 29, 2012 at 1:45 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> On Wed, Mar 28, 2012 at 10:33 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> On Wed, 28 Mar 2012, Lin Ming wrote:
>>
>>> ZPODD(Zero Power Optical Disk Drive) is a new feature in
>>> SATA 3.1 specification. It provides a way to power off unused ODD.
>>>
>>> ZPODD support is checked in in sr_probe().
>>> can_power_off flag is set during suspend if ZPODD is supported.
>>>
>>> ATA port's runtime suspend callback will actually power off the ODD
>>> and its runtime resume callback will actually power on the ODD.
>>>
>>> When ODD is powered off(D3Cold state), inserting disk will trigger a
>>> wakeup event(GPE). GPE AML handler notifies the associated device. Then
>>> ODD is resumed in the notify handler.
>>
>>> --- a/drivers/scsi/sr.c
>>> +++ b/drivers/scsi/sr.c
>>
>>> @@ -80,12 +82,38 @@ 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 *dev, pm_message_t mesg)
>>> +{
>>> +     struct scsi_cd *cd;
>>> +
>>> +     cd = dev_get_drvdata(dev);
>>> +     if (cd->device->power_off)
>>> +             dev->power.subsys_data->can_power_off = true;
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int sr_resume(struct device *dev)
>>> +{
>>> +     struct scsi_cd *cd;
>>> +
>>> +     cd = dev_get_drvdata(dev);
>>> +     if (cd->device->power_off) {
>>> +             dev->power.subsys_data->can_power_off = false;
>>> +             cd->poweroff_event = 0;
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>
>> Calling this flag "can_power_off" makes these routines look very
>> strange.  Either the device can power off or it can't, i.e., either it
>> supports ZPODD or it doesn't.  This doesn't change over time.
>>
>> If you rename the flag "may_power_off" then its meaning will be more
>> clear.
>
> OK, will rename it.
>
>>
>>> @@ -216,6 +244,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>>>       unsigned int events;
>>>       int ret;
>>>
>>> +     /* Not necessary to check events if enter ZPODD state */
>>> +     if (cd->device->power_off &&
>>> +                     pm_runtime_suspended(&cd->device->sdev_gendev))
>>> +             return 0;
>>
>> The comment is wrong and the new code does the wrong thing.  You _do_
>> have to check for events even in the ZPODD state, which means
>> sr_check_events must power-up the device if necessary.
>> sd_check_events in James Bottomley's scsi-misc tree now does the right
>> thing; see commit 4e2247b2bd289f079349d6c69755f8cff4e31f2b.
>
> The problem is userspace(GNOME, for example) will check for events
> every seconds.
> If sr is power up so frequently then we lost the expected power
> savings from ZPODD.

Agreed.
BTW, it's udevd on my Linux box that changed the default poll msecs kernel param
which caused sr_check_events being called every 2 seconds.

>
> There are 2 events:
>
> DISK_EVENT_MEDIA_CHANGE
> DISK_EVENT_EJECT_REQUEST
>
> In current implementation, if sr is in ZPODD state, then it means
> there is no disk in the CDROM.
> So if sr is in ZPODD state, MEDIA_CHANGE would never happen.
>
> EJECT_REQUEST seems can be ignored, since there is no disk in the CDROM at all.
>

For the ODD to be put into suspend state, the conditions should be:
1 tray closed
2 no media inside
I think we missed the condition 1 check now.

And if we follow the two conditions, the events can be safely ignored.

What do you think of blocking events for it when going to suspend and unblocking
when resume? This could erase the unnecessary calls of the check events function
when ODD is suspended.
But disk_(un)block_events are not exported and can't be used in sr
module. So I'm
not sure how to do this.

Another thing to consider is, user might want to eject the tray by
software like the
eject /dev/sr0 command or some UI mouse clicks against the cdrom icon. I'm still
thinking how to do this correctly.

>>
>>> +
>>>       /* no changer support */
>>>       if (CDSL_CURRENT != slot)
>>>               return 0;
>>> @@ -260,6 +293,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>>>       cd->media_present = scsi_status_is_good(ret) ||
>>>               (scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
>>>
>>> +     if (!cd->media_present && cd->device->power_off && !cd->poweroff_event) {
>>> +             scsi_autopm_put_device(cd->device);
>>
>> You can see your mistake here.  You call scsi_autopm_put_device here
>> without calling scsi_autopm_get_device earlier.
>
> Let me check this.

I guess the earlier call of get device is this?

 869 int scsi_sysfs_add_sdev(struct scsi_device *sdev)
... ...
 891
 892        /* The following call will keep sdev active indefinitely, until
 893         * its driver does a corresponding scsi_autopm_pm_device().  Only
 894         * drivers supporting autosuspend will do this.
 895         */
 896        scsi_autopm_get_device(sdev);
 897

-Aaron

>
> Thanks for the review.
> Lin Ming
>
>>
>> Alan Stern
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaron Lu - March 30, 2012, 1:07 a.m.
On Thu, Mar 29, 2012 at 11:00:18AM -0400, Alan Stern wrote:
> On Thu, 29 Mar 2012, Aaron Lu wrote:
> 
> > >>> @@ -216,6 +244,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
> > >>> � � � unsigned int events;
> > >>> � � � int ret;
> > >>>
> > >>> + � � /* Not necessary to check events if enter ZPODD state */
> > >>> + � � if (cd->device->power_off &&
> > >>> + � � � � � � � � � � pm_runtime_suspended(&cd->device->sdev_gendev))
> > >>> + � � � � � � return 0;
> > >>
> > >> The comment is wrong and the new code does the wrong thing. �You _do_
> > >> have to check for events even in the ZPODD state, which means
> > >> sr_check_events must power-up the device if necessary.
> > >> sd_check_events in James Bottomley's scsi-misc tree now does the right
> > >> thing; see commit 4e2247b2bd289f079349d6c69755f8cff4e31f2b.
> > >
> > > The problem is userspace(GNOME, for example) will check for events
> > > every seconds.
> > > If sr is power up so frequently then we lost the expected power
> > > savings from ZPODD.
> 
> That's true.  There's nothing you can do about it in the kernel; it's 
> up to userspace to change the frequency of event polling.
> 
> > Agreed.
> > BTW, it's udevd on my Linux box that changed the default poll msecs kernel param
> > which caused sr_check_events being called every 2 seconds.
> 
> Indeed.
> 
> > > There are 2 events:
> > >
> > > DISK_EVENT_MEDIA_CHANGE
> > > DISK_EVENT_EJECT_REQUEST
> > >
> > > In current implementation, if sr is in ZPODD state, then it means
> > > there is no disk in the CDROM.
> > > So if sr is in ZPODD state, MEDIA_CHANGE would never happen.
> 
> Sure it will: when the user inserts a disc.
> 
> > > EJECT_REQUEST seems can be ignored, since there is no disk in the CDROM at all.
> > >
> > 
> > For the ODD to be put into suspend state, the conditions should be:
> > 1 tray closed
> > 2 no media inside
> > I think we missed the condition 1 check now.
> > 
> > And if we follow the two conditions, the events can be safely ignored.
> 
> No, they can't.  Otherwise the device won't power back up when the user 
> inserts a new disc.
> 

The ODD is put to zero power state, so it can't react to the eject
button without being powered up back first ;-)
On current implementation, ACPI is used to power back up the ODD like this:
1 User press the eject button;
2 A gpe event fired, and ACPI interrupt, and the corresponding gpe
handler runs and the ODD's ACPI handle is notified about DEVICE_WAKE_UP;
3 In ODD's acpi notify handler, we power back up it by scsi_autopm_get_device.
This is handled in the following Lin Ming's patch:
[PATCH v3 2/7] libata-acpi: add ata port runtime D3Cold support

> > What do you think of blocking events for it when going to suspend and unblocking
> > when resume? This could erase the unnecessary calls of the check events function
> > when ODD is suspended.
> > But disk_(un)block_events are not exported and can't be used in sr
> > module. So I'm
> > not sure how to do this.
> > 
> > Another thing to consider is, user might want to eject the tray by
> > software like the
> > eject /dev/sr0 command or some UI mouse clicks against the cdrom icon. I'm still
> > thinking how to do this correctly.
> 
> What's the problem?  Can't the user already eject the tray via 
> software?
> 

When the ODD is put to zero power state, it will not be able to handle
commands like CDROMEJECT. We have to power it up first and then handle
the ioctl request. Currently, we only power back up the ODD when user
press the eject button as explained above.

> > >>> @@ -260,6 +293,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
> > >>> � � � cd->media_present = scsi_status_is_good(ret) ||
> > >>> � � � � � � � (scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
> > >>>
> > >>> + � � if (!cd->media_present && cd->device->power_off && !cd->poweroff_event) {
> > >>> + � � � � � � scsi_autopm_put_device(cd->device);
> > >>
> > >> You can see your mistake here. �You call scsi_autopm_put_device here
> > >> without calling scsi_autopm_get_device earlier.
> > >
> > > Let me check this.
> > 
> > I guess the earlier call of get device is this?
> > 
> >  869 int scsi_sysfs_add_sdev(struct scsi_device *sdev)
> > ... ...
> >  891
> >  892        /* The following call will keep sdev active indefinitely, until
> >  893         * its driver does a corresponding scsi_autopm_pm_device().  Only
> >  894         * drivers supporting autosuspend will do this.
> >  895         */
> >  896        scsi_autopm_get_device(sdev);
> >  897
> 
> You have to do this correctly.  Currently sr doesn't support runtime PM 
> at all.  If you do want to support it, then the driver should call 
> scsi_autopm_put_device at the end of the probe routine (or whenever it 
> is ready to allow runtime suspends) and scsi_autopm_get_device at the 
> start of the remove routine (or whenever it wants to prevent runtime 
> suspends).
> 
> The idea is that the driver is probed with the PM usage counter set to
> 1, so a runtime suspend won't occur until you do a put.  Similarly, in
> the remove routine, you must make sure that your gets and puts balance
> out.
> 

Oh, I see, thanks for your clear explanation.

-Aaron


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Stern - March 30, 2012, 2:28 p.m.
On Fri, 30 Mar 2012, Aaron Lu wrote:

> > > For the ODD to be put into suspend state, the conditions should be:
> > > 1 tray closed
> > > 2 no media inside
> > > I think we missed the condition 1 check now.
> > > 
> > > And if we follow the two conditions, the events can be safely ignored.
> > 
> > No, they can't.  Otherwise the device won't power back up when the user 
> > inserts a new disc.
> > 
> 
> The ODD is put to zero power state, so it can't react to the eject
> button without being powered up back first ;-)
> On current implementation, ACPI is used to power back up the ODD like this:
> 1 User press the eject button;
> 2 A gpe event fired, and ACPI interrupt, and the corresponding gpe
> handler runs and the ODD's ACPI handle is notified about DEVICE_WAKE_UP;
> 3 In ODD's acpi notify handler, we power back up it by scsi_autopm_get_device.
> This is handled in the following Lin Ming's patch:
> [PATCH v3 2/7] libata-acpi: add ata port runtime D3Cold support

I see.  Then you are right; it's not necessary to check for events 
while the drive is at zero power.

> When the ODD is put to zero power state, it will not be able to handle
> commands like CDROMEJECT. We have to power it up first and then handle
> the ioctl request. Currently, we only power back up the ODD when user
> press the eject button as explained above.

Then you may want to avoid going to zero-power while a program is
holding the device file open.  After all, the program might issue a
CDROMEJECT ioctl.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Ceuleers - March 31, 2012, 10:45 a.m.
On 30/03/12 03:07, Aaron Lu wrote:
> When the ODD is put to zero power state, it will not be able to handle
> commands like CDROMEJECT. We have to power it up first and then handle
> the ioctl request. Currently, we only power back up the ODD when user
> press the eject button as explained above.

What about slot-loading optical drives, where the user does not first 
have to press a button to open the tray (due to a lack of tray)?

Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaron Lu - April 1, 2012, 10:34 a.m.
On Sat, Mar 31, 2012 at 12:45:40PM +0200, Jan Ceuleers wrote:
> 
> What about slot-loading optical drives, where the user does not
> first have to press a button to open the tray (due to a lack of
> tray)?

For slot type ODD, the insert of a disk will fire the gpe event by
means of the sata slimline connector's device attention pin.

That means, the insert of a disk into a slot type ODD and the
press of the eject button of a tray type ODD has the same effect to
trigger the resume process.

Thanks,
Aaron


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lin Ming - April 5, 2012, 6 a.m.
On Thu, Mar 29, 2012 at 9:50 PM, Aaron Lu <aaron.lu@amd.com> wrote:

[.....]

> For the ODD to be put into suspend state, the conditions should be:
> 1 tray closed
> 2 no media inside
> I think we missed the condition 1 check now.
>
> And if we follow the two conditions, the events can be safely ignored.
>
> What do you think of blocking events for it when going to suspend and unblocking
> when resume? This could erase the unnecessary calls of the check events function
> when ODD is suspended.

Good idea.
Will try this.

> But disk_(un)block_events are not exported and can't be used in sr
> module. So I'm
> not sure how to do this.

We can simply export it.

>
> Another thing to consider is, user might want to eject the tray by
> software like the
> eject /dev/sr0 command or some UI mouse clicks against the cdrom icon. I'm still
> thinking how to do this correctly.

Assume eject /dev/sr0 is implemented as:

int fd = open("/dev/sr0", ...)
ioctl(fd, CDROMEJECT)

We may need to resume ODD in the ioctl handler(scsi_cmd_ioctl).

Thanks,
Lin Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 81ef0f5..3744761 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -859,8 +859,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);
-			if (acpi_state > 0)
+			if (acpi_state > 0) {
+				/* Check if the device is ready for power off */
+				if (acpi_state == ACPI_STATE_D3_COLD &&
+					!scsi_device_can_power_off(dev->sdev))
+					acpi_state = ACPI_STATE_D3;
+
 				acpi_bus_set_power(handle, acpi_state);
+			}
 			if (ap->tdev.power.request == RPM_REQ_SUSPEND)
 				acpi_pm_device_run_wake(
 					&dev->sdev->sdev_gendev, true);
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index c467064..0dc7e74 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -224,6 +224,14 @@  void scsi_autopm_put_host(struct Scsi_Host *shost)
 	pm_runtime_put_sync(&shost->shost_gendev);
 }
 
+bool scsi_device_can_power_off(struct scsi_device *sdev)
+{
+	struct device *dev = &sdev->sdev_gendev;
+
+	return !dev->power.subsys_data ? false :
+		dev->power.subsys_data->can_power_off;
+}
+
 #else
 
 #define scsi_runtime_suspend	NULL
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..265a8ea 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,8 @@ 
 #include <linux/blkdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/acpi.h>
 #include <asm/uaccess.h>
 
 #include <scsi/scsi.h>
@@ -80,12 +82,38 @@  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 *dev, pm_message_t mesg)
+{
+	struct scsi_cd *cd;
+
+	cd = dev_get_drvdata(dev);
+	if (cd->device->power_off)
+		dev->power.subsys_data->can_power_off = true;
+
+	return 0;
+}
+
+static int sr_resume(struct device *dev)
+{
+	struct scsi_cd *cd;
+
+	cd = dev_get_drvdata(dev);
+	if (cd->device->power_off) {
+		dev->power.subsys_data->can_power_off = false;
+		cd->poweroff_event = 0;
+	}
+
+	return 0;
+}
+
 static struct scsi_driver sr_template = {
 	.owner			= THIS_MODULE,
 	.gendrv = {
 		.name   	= "sr",
 		.probe		= sr_probe,
 		.remove		= sr_remove,
+		.suspend	= sr_suspend,
+		.resume		= sr_resume,
 	},
 	.done			= sr_done,
 };
@@ -216,6 +244,11 @@  static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	unsigned int events;
 	int ret;
 
+	/* Not necessary to check events if enter ZPODD state */
+	if (cd->device->power_off &&
+			pm_runtime_suspended(&cd->device->sdev_gendev))
+		return 0;
+
 	/* no changer support */
 	if (CDSL_CURRENT != slot)
 		return 0;
@@ -260,6 +293,11 @@  static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	cd->media_present = scsi_status_is_good(ret) ||
 		(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
 
+	if (!cd->media_present && cd->device->power_off && !cd->poweroff_event) {
+		scsi_autopm_put_device(cd->device);
+		cd->poweroff_event = 1;
+	}
+
 	if (last_present != cd->media_present)
 		cd->device->changed = 1;
 
@@ -716,6 +754,9 @@  static int sr_probe(struct device *dev)
 	disk->flags |= GENHD_FL_REMOVABLE;
 	add_disk(disk);
 
+	if (sdev->power_off)
+		dev_pm_get_subsys_data(dev);
+
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
 	return 0;
@@ -972,6 +1013,9 @@  static int sr_remove(struct device *dev)
 	kref_put(&cd->kref, sr_kref_release);
 	mutex_unlock(&sr_ref_mutex);
 
+	if (cd->device->power_off)
+		dev_pm_put_subsys_data(dev);
+
 	return 0;
 }
 
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 37c8f6b..8b2840b 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -42,6 +42,8 @@  typedef struct scsi_cd {
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
 	unsigned media_present:1;	/* media is present */
 
+	unsigned poweroff_event:1;
+
 	/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
 	int tur_mismatch;		/* nr of get_event TUR mismatches */
 	bool tur_changed:1;		/* changed according to TUR */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 0f69612..4805d5a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -389,9 +389,11 @@  extern int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd,
 #ifdef CONFIG_PM_RUNTIME
 extern int scsi_autopm_get_device(struct scsi_device *);
 extern void scsi_autopm_put_device(struct scsi_device *);
+extern bool scsi_device_can_power_off(struct scsi_device *);
 #else
 static inline int scsi_autopm_get_device(struct scsi_device *d) { return 0; }
 static inline void scsi_autopm_put_device(struct scsi_device *d) {}
+static inline bool scsi_device_can_power_off(struct scsi_device *d) { return false; };
 #endif /* CONFIG_PM_RUNTIME */
 
 static inline int __must_check scsi_device_reprobe(struct scsi_device *sdev)