From: Mayuresh Kulkarni <mkulkarni@opensource.cirrus.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.com>,
Greg KH <gregkh@linuxfoundation.org>,
<patches@opensource.cirrus.com>, <linux-usb@vger.kernel.org>
Subject: Re: [PATCH] usb: core: devio: add ioctls for suspend and resume
Date: Wed, 3 Jul 2019 15:44:35 +0100 [thread overview]
Message-ID: <1562165075.7481.27.camel@opensource.cirrus.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1906211319260.1471-100000@iolanthe.rowland.org>
On Fri, 2019-06-21 at 15:28 -0400, Alan Stern wrote:
> On Fri, 21 Jun 2019, Mayuresh Kulkarni wrote:
>
> However, I have been thinking about how to do all this in light of
> Oliver's comments, and it seems like we should make some changes.
>
> First, let's change the ioctls' names. Instead of RESUME and SUSPEND,
> I'll call them FORBID_SUSPEND and ALLOW_SUSPEND. The way they work
> should be clear: ALLOW_SUSPEND will permit the device to be suspended
> but might not cause a suspend to happen immediately. FORBID_SUSPEND
> will cause an immediate resume if the device is currently suspended
> and
> will prevent the device from being suspended in the future. The new
> names more accurately reflect what the ioctls actually do.
>
> Second, the WAIT_FOR_RESUME ioctl will wait only until a resume has
> occurred more recently than the most recent ALLOW_SUSPEND ioctl. So
> for example, if the program calls ALLOW_SUSPEND, and the device
> suspends, and then the device resumes, and then the device suspends
> again, and then the program calls WAIT_FOR_RESUME, the ioctl will
> return immediately even though the device is currently suspended.
> Thus you don't have to worry about missing a remote resume. This also
> means that when WAIT_FOR_RESUME returns, the program should call
> FORBID_SUSPEND to ensure that the device is active and doesn't go
> back
> into suspend.
>
> In practice, your program would use the ioctls as follows:
>
> When the device file is opened, the kernel will automatically
> do the equivalent of FORBID_SUSPEND (to remain compatible
> with the current behavior).
>
> When the program is ready for the device to suspend, it will
> call the ALLOW_SUSPEND ioctl. But it won't cancel the
> outstanding URBs; instead it will continue to interact
> normally with the device, because the device might not be
> suspended for some time.
>
> When the device does go into suspend, URBs will start failing
> with an appropriate error code (EHOSTUNREACH, ESHUTDOWN,
> EPROTO, or something similar). In this way the program will
> realize the device is suspended. At that point the program
> should call the WAIT_FOR_RESUME ioctl and stop trying to
> communicate with the device.
>
> When WAIT_FOR_RESUME returns, the program should call the
> FORBID_SUSPEND ioctl and resume normal communication with the
> device.
>
> How does that sound?
>
> The proposed patch is below.
>
> Alan Stern
>
Hi Alan,
With the 3-ioctl patch you had send, I checked the behaviour using my
test program on our reference platform.
AFAIU, the patch seems to work fine for our use-cases of: remote-wake
and host-wake.
For our typical use-cases, the user-space does not need to communicate
with the device even after calling ALLOW_SUSPEND, so I am not in
position to verify this point.
The test does the below steps -
----------------
A. REMOTE-WAKE -
----------------
1. Open the device file.
2. Find our interface and bind to it.
3. Send multiple commands to our interface.
4. Call ALLOW_SUSPEND.
5. Call WAIT_FOR_RESUME.
6. Wait for sometime (10-20 sec).
7. Do the activity on device which I know causes remote-wake to host.
8. Waiter in (5) above, calls FORBID_SUSPEND and reads the cause of
resume.
9. Release the interface and close the device file.
After (5) + auto-suspend time-out expires, I can see device, root-hub
and host-controller (DWC-3) going into suspend.
After (7), host-controller, root-hub and device resume are seen.
--------------
B. HOST-WAKE -
--------------
1. Open the device file.
2. Find our interface and bind to it.
3. Send multiple commands to our interface.
4. Spawn a thread to cause host-wake.
5. Call ALLOW_SUSPEND.
6. Call WAIT_FOR_RESUME.
7. Signal thread after a delay (10-20 sec).
8. Thread calls FORBID_SUSPEND and sends a command to device.
9. Release the interface, wait for thread-join and close the device
file.
After (6) + auto-suspend time-out expires, device, root-hub and host
controller goes into suspend.
After (8), host-controller, root-hub and device resume are seen. Also
the response to command is correct.
With that said, at this point, the above observations are in-sync with
what I had verified when I had send-out the old patch (with 2-ioctls and
interface based suspend-resume). I am checking if I can verify a bit
more complex use-cases. But not sure, if I can do that with current
setup.
As you had mentioned in one of the comment before, the only addition to
the patch I have locally is -
usbfs_notify_resume() has usbfs_mutex lock around list traversal.
Could you please send the patch for review? Please note, I think I am
not a part of linux-usb mailing-list, so probably need to be in cc to
get the patch email. Do let me know if something else is needed from me.
Thanks to You/Oliver/Greg KH for the all the comments and reviews.
next prev parent reply other threads:[~2019-07-03 14:44 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-10 10:01 [PATCH] usb: core: devio: add ioctls for suspend and resume Mayuresh Kulkarni
2019-06-05 9:41 ` Greg KH
2019-06-05 21:02 ` Alan Stern
2019-06-13 14:00 ` Mayuresh Kulkarni
2019-06-13 20:54 ` Alan Stern
2019-06-17 11:38 ` Mayuresh Kulkarni
2019-06-17 15:55 ` Alan Stern
2019-06-18 14:00 ` Mayuresh Kulkarni
2019-06-18 15:50 ` Alan Stern
2019-06-19 9:19 ` Oliver Neukum
2019-06-19 14:40 ` Alan Stern
2019-06-19 15:12 ` Oliver Neukum
2019-06-19 16:07 ` Alan Stern
2019-06-20 15:11 ` Mayuresh Kulkarni
2019-06-20 15:49 ` Alan Stern
2019-06-21 16:16 ` Mayuresh Kulkarni
2019-06-21 19:28 ` Alan Stern
2019-06-24 16:02 ` Mayuresh Kulkarni
2019-06-24 17:48 ` Alan Stern
2019-06-25 10:41 ` Mayuresh Kulkarni
2019-06-25 14:08 ` Alan Stern
2019-06-26 7:42 ` Oliver Neukum
2019-06-26 14:35 ` Alan Stern
2019-06-26 14:15 ` Mayuresh Kulkarni
2019-06-26 15:07 ` Alan Stern
2019-06-27 13:20 ` Mayuresh Kulkarni
2019-06-27 13:52 ` Alan Stern
2019-07-01 9:18 ` Oliver Neukum
2019-07-01 14:17 ` Alan Stern
2019-07-02 9:21 ` Oliver Neukum
2019-07-02 14:29 ` Alan Stern
2019-07-03 14:44 ` Mayuresh Kulkarni [this message]
2019-07-05 18:51 ` [RFC] usbfs: Add ioctls for runtime " Alan Stern
2019-07-11 9:16 ` Mayuresh Kulkarni
2019-07-11 14:20 ` Alan Stern
2019-07-11 14:36 ` Greg KH
2019-07-25 9:10 ` Mayuresh Kulkarni
2019-07-25 9:18 ` Greg KH
2019-07-25 15:18 ` Alan Stern
2019-07-25 16:05 ` Greg KH
2019-06-20 14:34 ` [PATCH] usb: core: devio: add ioctls for " Mayuresh Kulkarni
2019-06-20 14:43 ` Alan Stern
2019-06-13 13:32 ` Mayuresh Kulkarni
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=1562165075.7481.27.camel@opensource.cirrus.com \
--to=mkulkarni@opensource.cirrus.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.org \
--cc=oneukum@suse.com \
--cc=patches@opensource.cirrus.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).