On Wed, 24 Oct 2018 10:10:32 -0400 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > On Mon, 22 Oct 2018 10:24:46 -0400 > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > On Mon, 22 Oct 2018, Oliver Neukum wrote: > > > > > > > On Do, 2018-10-18 at 13:42 -0400, Alan Stern wrote: > > > > > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > > > > > > > > > The only way to make the ioctl work properly is to have it do a > > > > > > > runtime-PM put at the start and then a runtime-PM get before it > > > > > > > > If and only if you want to do this with one ioctl() > > > > If you separate the runtime-PM put and the get, you can do it without > > > > the waiting part. > > > > > > Sure, but you still need a runtime-PM put at the start and a runtime-PM > > > get at the end. In fact, if you separate this into two calls then you > > > run the risk that the user may never perform the second call, and you > > > would end up with an unbalanced put. > > > > > > > I am tending to agree towards having a single ioctl call here. It is better > > to give minimal control to user-space w.r.t. runtime PM. The current > > proposal of user-space giving an hint to USB-FS/core that - it is not using > > the device, sounds better. > > > > > > > > /* > > > > > > * There are 3 possibilities here: > > > > > > * 1. Device did suspend and resume (success) > > > > > > * 2. Signal was received (failed suspend) > > > > > > * 3. Time-out happened (failed suspend) > > > > > > > > > > 4. Device did suspend but a signal was received before the device > > > > > resumed. > > > > > > > > > > > * In any of above cases, we need to resume device. > > > > > > */ > > > > > > usb_autoresume_device(dev); > > > > > > > > Yes and that is the problem. Why do you want to wait for the result > > > > of runtime-PM put ? If we need a channel for notifying user space > > > > about resume of a device, why wait for the result of suspend instead > > > > of using the same channel? > > > > > > This is not meant to be a general-purpose channel for notifying > > > userspace when a device resumes. Such a channel should be defined in > > > the runtime-PM layer, not in the USB layer. > > > > > > This is instead meant to be a special-purpose mechanism for adding a > > > runtime-suspend/resume interface to usbfs. > > > > > > > Just to be clear here - the worst case wait-time from user-space > > perspective will be <time-out - 1 HZ> + <time-to-resume>, right? > > That's right. > > > If yes then, timeout argument means "wait at-least timeout sec". > > Yes. Unless for a few unlikely cases, including: > > A signal is received before the timeout expires; > > The device suspends and then resumes before the timeout > expires. > > > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > We spend time internally to go over the "new" ioctl proposal. Overall > > it looks promising. > > > > However, we still have an issue as below - > > Consider a use-case where, user-space calls "new" ioctl, but suspend > > never happen (for various reasons) && async event happens on USB > > device side by the end-user. > > > > In such a case, since user-space is waiting in "new" ioctl, it is not > > in position to queue a request to read-out the async event info. It > > will be able to queue a request when the "new" ioctl returns which > > will be "time-out" later (in this case). Due to auto-suspend > > time-out's default's value of 2 sec, the user-space has to choose the > > time-out to "new" ioctl > 2 sec. > > Not so. That "2 second" value can be adjusted by the user; it can be > reduced to as little as 1 ms. > Thanks for the tip. But an issue with changing the module-param "usb_autosuspend_delay" is, the new value will be applicable to all USB devices connected after that. This may or may-not be optimum. I am not sure, but need to check if it is possible to set "usb_autosuspend_delay" based on connected device as well as current use-case running in the user-space we are targeting (which is Android, FYI). > Alan Stern
