All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Aaron Lu <aaron.lu@intel.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Oliver Neukum <oneukum@suse.de>,
	'James Bottomley' <James.Bottomley@hansenpartnership.com>,
	linux-pm@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-ide@vger.kernel.org
Subject: Re: [PATCH v7 2/6] scsi: sr: support runtime pm
Date: Sun, 30 Sep 2012 15:08:04 -0400	[thread overview]
Message-ID: <50689894.30004@pobox.com> (raw)
In-Reply-To: <201209300044.51017.rjw@sisk.pl>

On 09/29/2012 06:44 PM, Rafael J. Wysocki wrote:
> On Saturday, September 29, 2012, Aaron Lu wrote:
>> On 09/29/2012 10:29 PM, Alan Stern wrote:
>>> On Sat, 29 Sep 2012, Aaron Lu wrote:
>>>
>>>>> I don't think this is a good idea, quite frankly.  sr seems to be a too
>>>>> generic place for that.
>>>>
>>>> Does this mean sr can only have code that is useful to all devices it
>>>> manages? i.e. If a piece of code enables a feature for a special kind of
>>>> ODD(like the sata based ZPODD), it shouldn't be done in sr?
>>>
>>> Drivers are allowed to have special features and quirks that apply only
>>> to some devices.
>>
>> I think SATA based zero power capable ODDs are the "some devices".
>>
>>>
>>>> There is nothing in theory stopping us from doing this in ata layer.
>>>> For the loading mechanism, we can always send an ATAPI command to figure
>>>> it out.
>>>>
>>>> So gentlemen, I need your opinions on where this ZPODD code should live
>>>> before I can continue this work, thanks.
>>>
>>> Can arbitrary SCSI devices be ZP, or does this notion apply only to
>>> ATAPI-based drives?  That's the key question, and the answer determines
>>> where the ZP support belongs.
>>
>> I don't know if arbitrary SCSI devices can be ZP or not, the SPC spec
>> doesn't seem to define such a power state.
>>
>> ZPODD is defined for sata based ATAPI ODD which supports device
>> attention, but doesn't specify how the ODD is powered off and how the
>> device attention pin notifies OS. On x86 systems, these are implemented
>> by ACPI.
>>
>> Though SCSI devices may not have a general notion of ZP, the fact that
>> ZPODD are managed by sr driver is enough to make the decision that ZPODD
>> code can live in sr?
>
> Well, only a part of it is handled by sr, the high-level part so to speak.
> The low-level handling is done by the ATA layer.  Now, since sr is the
> high-level part and is supposed to be generic, I don't think it's appropriate
> to make it care about some low-level things specific to ATA (because there is
> another layer of code supposed to handle those).
>
>>> On the other hand, the sr driver certainly deserves to have some form
>>> of runtime PM support, even for devices that aren't ZP.
>>
>> Agree.
>>
>> And the following codes need to find a home:
>> - Code used to handle ACPI wake notification(currently done in ATA, but
>>    causes the "annoying" need_eject flag in scsi_device);
>
> And quite frankly I'd more and more convinced that this flag isn't really
> necessary.
>
> What you really want to achieve is to eject the tray of a tray-type ODD
> if the eject is signaled through a GPE.  I don't see anything for sr to
> do in that.  Moreover, you probably would like to do that even if the
> drive is not powered down, right?
>
> I wonder if this mechanism can be used for user space notification
> without polling regardless of whether or not the zero-power feature is
> used?

Fair questions, and I think this is finally getting to the heart of the 
matter/solution.


>> - Code to check if the ODD is ready to be powered off per the ZPODD
>>    spec.
>
> If that can be done at the ATA level, I'd prefer it too.

Does the ready-to-poweroff check involve SCSI/MMC command set commands?

If no, it probably belongs in libata.

If yes, it probably belongs in sr.

	Jeff





  parent reply	other threads:[~2012-09-30 19:08 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-12  8:29 [PATCH v7 0/6] ZPODD patches Aaron Lu
2012-09-12  8:29 ` [PATCH v7 1/6] block: genhd: add an interface to set disk poll interval Aaron Lu
2012-09-20 20:35   ` Rafael J. Wysocki
2012-09-12  8:29 ` [PATCH v7 2/6] scsi: sr: support runtime pm Aaron Lu
2012-09-20 20:48   ` Rafael J. Wysocki
2012-09-20 20:54     ` Alan Stern
2012-09-21  1:02     ` Aaron Lu
2012-09-21 20:49       ` Rafael J. Wysocki
2012-09-24  1:20         ` Aaron Lu
2012-09-24 12:55           ` Rafael J. Wysocki
2012-09-24 14:52             ` Aaron Lu
2012-09-24 21:40               ` Rafael J. Wysocki
2012-09-25  8:01                 ` Aaron Lu
2012-09-25 11:47                   ` Rafael J. Wysocki
2012-09-25 14:20                     ` Aaron Lu
2012-09-25 14:23                       ` Oliver Neukum
2012-09-25 14:46                         ` Aaron Lu
2012-09-25 21:45                           ` Rafael J. Wysocki
2012-09-26  1:03                             ` Aaron Lu
2012-09-26 11:18                               ` Rafael J. Wysocki
2012-09-26 14:52                                 ` Aaron Lu
2012-09-26  7:20                           ` Oliver Neukum
2012-09-27 10:46                   ` Oliver Neukum
2012-09-28  8:20                     ` Aaron Lu
2012-09-12  8:29 ` [PATCH v7 3/6] scsi: sr: support zero power ODD(ZPODD) Aaron Lu
2012-09-20 22:07   ` Rafael J. Wysocki
2012-09-21  1:39     ` Aaron Lu
2012-09-21 21:02       ` Rafael J. Wysocki
2012-09-27  9:26         ` Aaron Lu
2012-09-27 14:42           ` Alan Stern
2012-09-27 14:55             ` Aaron Lu
2012-09-27 23:29               ` Rafael J. Wysocki
2012-09-24 21:55   ` Jeff Garzik
2012-09-12  8:29 ` [PATCH v7 4/6] scsi: pm: add may_power_off flag Aaron Lu
2012-09-12  8:29 ` [PATCH v7 5/6] scsi: sr: use may_power_off Aaron Lu
2012-09-12  8:29 ` [PATCH v7 6/6] libata: acpi: respect may_power_off flag Aaron Lu
2012-09-24 21:55   ` Jeff Garzik
2012-09-19  8:03 ` [PATCH v7 0/6] ZPODD patches Aaron Lu
2012-09-19 12:27   ` James Bottomley
2012-09-19 12:50     ` Rafael J. Wysocki
2012-09-19 14:19       ` Aaron Lu
2012-09-20 20:00         ` Rafael J. Wysocki
2012-09-21  5:48           ` Aaron Lu
2012-09-21 21:18             ` Rafael J. Wysocki
2012-09-22  7:32               ` Oliver Neukum
2012-09-22 11:28                 ` Rafael J. Wysocki
2012-09-22 15:38                   ` Alan Stern
2012-09-22 19:46                     ` Rafael J. Wysocki
2012-09-22 20:23                       ` Alan Stern
2012-09-22 21:48                         ` Rafael J. Wysocki
2012-09-24  2:55               ` Aaron Lu
2012-09-24 13:06                 ` Rafael J. Wysocki
2012-09-24 15:04                   ` Aaron Lu
2012-09-24 21:46                     ` Rafael J. Wysocki
2012-09-25  8:18                       ` Aaron Lu
2012-09-25 11:02                         ` James Bottomley
2012-09-25 13:56                           ` Aaron Lu
2012-09-27  9:43                           ` Aaron Lu
2012-09-24 15:47                 ` Alan Stern
2012-09-19 14:52       ` James Bottomley
2012-09-20 21:46         ` Rafael J. Wysocki
2012-09-19 13:05     ` Oliver Neukum
2012-09-19 15:19     ` David Woodhouse
2012-09-20  0:34       ` Jack Wang
     [not found] ` <201209280115.06964.rjw@sisk.pl>
     [not found]   ` <5064FA08.6030005@intel.com>
     [not found]     ` <201209282346.15872.rjw@sisk.pl>
2012-09-29  2:10       ` [PATCH v7 2/6] scsi: sr: support runtime pm Aaron Lu
2012-09-29 14:29         ` Alan Stern
2012-09-29 15:03           ` Aaron Lu
2012-09-29 22:44             ` Rafael J. Wysocki
2012-09-30 12:32               ` Aaron Lu
2012-09-30 14:47                 ` Alan Stern
2012-09-30 15:39                   ` Aaron Lu
2012-09-30 19:15                   ` Jeff Garzik
2012-09-30 19:08               ` Jeff Garzik [this message]
2012-09-29 22:31           ` Rafael J. Wysocki
2012-09-30 19:03             ` Jeff Garzik
2012-09-30 19:43               ` Alan Stern
2012-10-01  4:57                 ` Jeff Garzik
2012-10-08  9:27                 ` Aaron Lu
2012-10-08 10:21                   ` James Bottomley
2012-10-09  7:20                     ` Aaron Lu
2012-10-09 14:58                       ` James Bottomley
2012-10-11  7:49                         ` Aaron Lu
2012-10-09 23:26                       ` Rafael J. Wysocki
2012-09-29 22:27         ` Rafael J. Wysocki
2012-09-30 12:38           ` Aaron Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50689894.30004@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=aaron.lu@intel.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=oneukum@suse.de \
    --cc=rjw@sisk.pl \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.