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. > > > > > 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? 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
