All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
To: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: jkosina-AlSwsSmVLrQ@public.gmane.org,
	bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org,
	olofj-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/3] HID: usbhid: hid-core: submit queued urbs before suspend
Date: Wed, 14 Dec 2011 10:14:33 +0100	[thread overview]
Message-ID: <201112141014.33696.oneukum@suse.de> (raw)
In-Reply-To: <CAGS+omDMipRTVOGV9fG0ciWpuwMRBL-eTY8uDwaiSOW==CkQ-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am Mittwoch, 14. Dezember 2011, 09:00:18 schrieb Daniel Kurtz:
> On Wed, Dec 14, 2011 at 3:55 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> 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.

	Regards
		Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Oliver Neukum <oneukum@suse.de>
To: Daniel Kurtz <djkurtz@chromium.org>
Cc: jkosina@suse.cz, bleung@chromium.org, stern@rowland.harvard.edu,
	olofj@chromium.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH 2/3] HID: usbhid: hid-core: submit queued urbs before suspend
Date: Wed, 14 Dec 2011 10:14:33 +0100	[thread overview]
Message-ID: <201112141014.33696.oneukum@suse.de> (raw)
In-Reply-To: <CAGS+omDMipRTVOGV9fG0ciWpuwMRBL-eTY8uDwaiSOW==CkQ-w@mail.gmail.com>

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.

	Regards
		Oliver

  parent reply	other threads:[~2011-12-14  9:14 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-17 11:23 [PATCH 0/3 v2] usb/hid-core: drain URB queue when going to suspend Daniel Kurtz
2011-11-17 11:23 ` [PATCH 1/3] HID: usbhid: remove LED_ON Daniel Kurtz
2011-11-17 11:23 ` [PATCH 2/3] HID: usbhid: hid-core: submit queued urbs before suspend Daniel Kurtz
2011-12-14  7:55   ` Oliver Neukum
2011-12-14  8:00     ` Daniel Kurtz
2011-12-14  8:00       ` Daniel Kurtz
     [not found]       ` <CAGS+omDMipRTVOGV9fG0ciWpuwMRBL-eTY8uDwaiSOW==CkQ-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-12-14  9:14         ` Oliver Neukum [this message]
2011-12-14  9:14           ` Oliver Neukum
2011-12-14  9:41           ` Daniel Kurtz
2011-12-14  9:41             ` Daniel Kurtz
2011-12-14 11:36             ` Oliver Neukum
2011-11-17 11:23 ` [PATCH 3/3] HID: usbhid: defer LED setting to a workqueue Daniel Kurtz
2011-12-14  8:01   ` Oliver Neukum
2011-12-14  8:19     ` Daniel Kurtz
2011-12-14  8:19       ` Daniel Kurtz
2011-12-14  9:25       ` Oliver Neukum
     [not found]     ` <201112140901.35203.oneukum-l3A5Bk7waGM@public.gmane.org>
2011-12-14 10:33       ` Daniel Kurtz
2011-12-14 10:33         ` Daniel Kurtz
2011-12-14 11:22         ` Oliver Neukum
2011-12-15  6:43           ` Daniel Kurtz
2011-12-15  6:43             ` Daniel Kurtz
2011-12-15  9:01             ` Oliver Neukum
     [not found]               ` <201112151001.52761.oneukum-l3A5Bk7waGM@public.gmane.org>
2011-12-20 10:12                 ` Daniel Kurtz
2011-12-20 10:12                   ` Daniel Kurtz
2011-12-20 10:18                   ` Oliver Neukum
2011-12-20 10:18                     ` Oliver Neukum
2011-12-21 10:19                     ` Jiri Kosina
     [not found] ` <1321529030-7845-1-git-send-email-djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2011-12-07  2:42   ` [PATCH 0/3 v2] usb/hid-core: drain URB queue when going to suspend Daniel Kurtz
2011-12-07  2:42     ` Daniel Kurtz
2011-12-07  9:27     ` Jiri Kosina
  -- strict thread matches above, loose matches on Subject: below --
2011-11-01  9:25 [PATCH 0/3] " Daniel Kurtz
2011-11-01  9:25 ` [PATCH 2/3] HID: usbhid: hid-core: submit queued urbs before suspend Daniel Kurtz

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=201112141014.33696.oneukum@suse.de \
    --to=oneukum-l3a5bk7wagm@public.gmane.org \
    --cc=bleung-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=jkosina-AlSwsSmVLrQ@public.gmane.org \
    --cc=linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=olofj-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.