On Tuesday, September 25, 2012, Aaron Lu wrote: > On Mon, Sep 24, 2012 at 11:40:18PM +0200, Rafael J. Wysocki wrote: > > On Monday, September 24, 2012, Aaron Lu wrote: > > > On Mon, Sep 24, 2012 at 02:55:31PM +0200, Rafael J. Wysocki wrote: > > > > And I'd add a comment about the next poll. > > > > > > > > This appears somewhat racy, though, because in theory a media may be inserted > > > > while we are removing power from the device. Isn't that a problem? > > > > > > Yes, this is a problem. > > > To avoid this race, I think the following needs to be done: > > > - For slot type ODD, make it such that user can't insert a disc; > > > - For tray type ODD, make it such that when user presses the eject > > > button, the tray doesn't open. > > > I'll see if this is possible, thanks for the remind. > > > > OK > > Looks like this is not the right thing to do, if I lock the door user > will be confused. > > > > > > > > The poll runs periodically, until the device is powered off(reflected by > > > > > the powered_off flag), in which case, the poll will simply return > > > > > 0 without touching this device. > > > > > > > > Is it useful to poll the device after it has been suspended, but not powered > > > > off? > > > > > > Yes, it is necessary to poll the device when it has been suspended with > > > power left. The reason is, we need to check if there is a media change > > > event happened, and the way to check this is to issue a > > > GET_EVENT_STATUS_NOTIFICATION comand. > > > > > > Please note that when the drive is not powered off, it will not send us > > > a notification when its eject button is pressed. > > > > I'm not sure about that, actually. If it doesn't notify us, that whole thing > > is inherently racy pretty much beyond fixing, because it is always possible > > that the user will press the eject button right after we've checked the > > status last time and right before power removal. We're going to lose that > > event, so the user will have to push the button once again in that case. > > I just checked the spec again and tested, when the ODD has power, it > will also send out notifications on pressing the eject button/inserting > a disc. So we should be able to capture such a event. Good! > > > > > That's correct. > > > > > AFAIK, user space does not care how often the device is polled, it > > > > > cares only one thing: when there is a media change event, please let me > > > > > know(through uevent). > > > > > > > > So that's why we do the polling, right? > > > > > > Yes. > > > > Well, that's difficult. > > > > If the remote wakeup is signaled through a GPE, it should be possible to > > enable it before we actually put the device into D3cold. That may allow us > > to eliminate the races. > > Thanks for the suggestion, I'll update the code. > > I'm thinking of enabling this GPE in sr_suspend once we decided that it > is ready to be powered off, so the time frame between sr_suspend and > when the power is actually removed in libata should be taken care of by > the GPE. If GPE fires, the notification function will request a runtime > resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Thanks, Rafael -- 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
