Re: Query on usb/core/devio.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2018-11-08 at 10:26 -0500, Alan Stern wrote:
> On Thu, 8 Nov 2018, Mayuresh Kulkarni wrote:
> 
> > 
> > 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.
> > > > 
> > We reviewed the proposal internally and looks like we have following cases:
> > 1. Host wake from L2: This needs to be done when end-user wants to interact
> > with
> > device && device is in suspend/link in L2.
> > 2. Remote wake from L2 by device: This happens when USB device detects some
> > activity while link was in L2.
> > 
> > As per my understanding, the current ioctl proposal seems to address (2)
> > above.
> > But (1) seems to have issue which is, the end-user has to wait for remote-
> > wake
> > from device to interact with it (since device is already open and blocked in
> > new ioctl).
> This conclusion does not follow.
> 
> In fact, we have two choices.  An access while the device is suspended
> could be forced to fail -- this could cause unexpected errors on
> unrelated threads if they happen to share the same file descriptor for
> the device.  Or an access while the device is suspended could first
> cause it to wake up, terminating the ioctl call -- in which case there
> is no problem, right?
> 

Yes, it will be better to have the latter choice of - when device is suspended
and user wants to interact with it, there should be wake-up followed by
termination of ioctl call to wait-for-remote-wake.

> Alternatively, the user can simply send a signal to the thread that is
> blocked in the ioctl; that will terminate the ioctl call and allow the
> interaction to proceed.
> 

I am not sure, if this will be a good UX or not. Need to check more on this one.

> > 
> > With that said, it looks like we would need 3 ioctls as below:
> > 1. suspend: to be called by user-space when it knows it is done using the
> > device. Should be non-blocking. It should just drop the PM ref-count on
> > device.
> > 2. resume: to be called by user-space when it wants to actively interact
> > with
> > the device. Should be non-blocking. It should just get the PM ref-count on
> > device.
> > 3. wait-for-remote-wake: to be called by user-space when it wants to wait
> > for
> > remote wake of device && end user not actively use it. Should be blocking.
> > Unblocked by resume. It should have different return value to indicate
> > resume-by-host or resume-by-remote-wake (apart from signal info).
> The driver has no way to tell whether the resume was caused by the 
> host or by the device.  (In fact, it's possible for a resume to be 
> caused by _both_ the host and the device, if they request it at the 
> same time.)  In the end, it doesn't matter anyway.
> 

Yes agreed to the point that driver as such does not know if its resume is
called due to host or remote wake.
But based on ioctl call, it should be possible to know, if resume happened due
to resume-ioctl or remote-wake (e.g.: using a flag in resume-ioctl), right?

> > 
> > Does this sound reasonable (considering one of my previous comments :-/)?
> Doesn't (3) suffer from the same problem as (1) in the first list 
> above?
> 

As I understand, the calling sequence by user-space would be like -

ioctl(fd, USBDEVFS_SUSPEND); ==> put-down PM ref-count and return

ret = ioctl(fd, USBDVEFS_WAIT_FOR_REMOTE_WAKE); ==> will block for resume
if (ret == EHOST_WAKE)
	/* handle host wake case */
else if (ret == EREMOTE_WAKE)
	/* handle remote wake case */
else
	/* handle other return values */

When user wants to interact with the device, user space should call -

ioctl(fd, USBDEVFS_RESUME); ==> gets PM ref-count, unblock wait-for-remote and
return

Here the "get PM ref-count" should cause the resume of root-hub and its port
followed by resume of device, right?
As I understand, as a result of this operation, the host controller should bring
back the link to L0.

Am I missing some important aspect here? If yes, could you please help explain?

> Alan Stern



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux