Re: [PATCH 2/3] HID: usbhid: hid-core: submit queued urbs before suspend

[Oliver Neukum <oneukum@suse.de>]

Am Mittwoch, 14. Dezember 2011, 10:41:25 schrieben Sie:
> On Wed, Dec 14, 2011 at 5:14 PM, Oliver Neukum <oneukum@suse.de> wrote:
> > Am Mittwoch, 14. Dezember 2011, 09:00:18 schrieb Daniel Kurtz:
> >> On Wed, Dec 14, 2011 at 3:55 PM, Oliver Neukum <oneukum@suse.de> wrote:
> >> > Am Donnerstag, 17. November 2011, 12:23:49 schrieb Daniel Kurtz:
> >> >> Unfortunately, this usually happens while the first LED request is
> >> >> actually still being processed.  Thus when the completion handler tries
> >> >> to submit the second LED request it fails, since REPORTED_IDLE is
> >> >> already set!  This REPORTED_IDLE check failure causes the completion
> >> >> handler to complete, however without clearing the CTRL_RUNNING flag.
> >> >> This, in turn, means that the suspend() handler's wait_io() condition
> >> >> is never satisfied, and instead it times out after 10 seconds, aborting
> >> >> the original system suspend.
> >> >>
> >> >> This patch changes the behavior to the following:
> >> >>   (1) allow completion handler to finish submitting all queued URBs, even if
> >> >>       REPORTED_IDLE is set.  This guarantees that all URBs queued before the
> >> >>       hid-core suspend() call will be submitted before the system is
> >> >>       suspended.
> >> >
> >> > Hi,
> >> >
> >> > why is this desirable? You'd want to requests to be executed at resumption.
> >> > A system suspend will alter the LED state anyway.
> >>
> >> This is how a system suspend 'alters' the LED state.  The input layer
> >> sends those LED off commands down through HID as it is trying to
> >> suspend.  We want to make sure the usbhid layer actually finishes
> >> forwarding those requests on to the device before the system is
> >> suspended.
> >
> > You know the HID layer will send those commands. You don't know which
> > other commands may already be queued. So I don't know whether you really want
> > to execute them all while the system is asuspending.
> 
> I'm confused.  The HID layer is sending which commands?

That depends on the state of the system. The system may suspend
right after you pressed CAPS_LOCK for example.

> This patch fixes this problem, and synchronizes queued URBs with
> system suspend by sending all urbs that were queued before the usbhid
> suspend() was called.

Your solution is correct. But you have two choices to solve this problem.

a) submit and wait for all IO before you suspend
b) queue all requests and restart IO as you resume

You chose solution a. Is that the best solution?

	Regards
		Oliver