Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1730525 > unrolled thread

Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

Started byBenson Leung <bleung@google.com>
First post2017-09-11 22:10 +0200
Last post2017-09-20 03:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup Benson Leung <bleung@google.com> - 2017-09-11 22:10 +0200
    Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup Benson Leung <bleung@google.com> - 2017-09-12 01:30 +0200
      Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-09-20 03:40 +0200

#1730525 — Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup

FromBenson Leung <bleung@google.com>
Date2017-09-11 22:10 +0200
SubjectRe: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
Message-ID<uoDbQ-84z-33@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

Hi Oliver,

On Sat, Sep 09, 2017 at 09:35:52AM +0200, Oliver Neukum wrote:
> Am Freitag, den 08.09.2017, 10:43 -0700 schrieb Dmitry Torokhov:
> > From: Benson Leung <bleung@chromium.org>
> > 
> > usbhid->intf->needs_remote_wakeup is set when a device is opened, and is
> > cleared when a device is closed.
> > 
> > In usbhid_open, usb_autopm_get_interface is called before setting the
> > needs_remote_wakeup flag, and usb_autopm_put_interface is called after
> > hid_start_in. However, when the device is closed in usbhid_close, we
> > simply reset the flag and the device stays awake even though it could be
> > suspended.
> 
> Hi,
> 
> but if the device is asleep, we do not want to wake it just to reset
> the flag. Please use the no resume varieties. They did not exist when this
> code was written and that is the reason behind the current code.
> 

Thanks for the pointer. I'll respin the patch with the no_resume version of
usb_autopm_get_interface and retest.

Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[toc] | [next] | [standalone]


#1730580

FromBenson Leung <bleung@google.com>
Date2017-09-12 01:30 +0200
Message-ID<uoGjo-1Cf-17@gated-at.bofh.it>
In reply to#1730525

[Multipart message — attachments visible in raw view] — view raw

Hi Oliver,

On Mon, Sep 11, 2017 at 01:02:52PM -0700, Benson Leung wrote:
> Thanks for the pointer. I'll respin the patch with the no_resume version of
> usb_autopm_get_interface and retest.
> 

I went and tried this patch but with usb_autopm_get_interface_no_resume instead
and the original bug I was trying to fix with a Yubikey hid security key came
back, so something is still wrong here.

I went ahead and filed a bug to track this here:
https://bugs.chromium.org/p/chromium/issues/detail?id=764112

I'll find some time to dig into this deeper and understand why this device
ends up getting stuck in active instead of suspending when all handles are
closed.

Thanks,
Benson

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[toc] | [prev] | [next] | [standalone]


#1735389

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2017-09-20 03:40 +0200
Message-ID<urC9A-4zJ-7@gated-at.bofh.it>
In reply to#1730580
On Mon, Sep 11, 2017 at 4:29 PM, Benson Leung <bleung@google.com> wrote:
> Hi Oliver,
>
> On Mon, Sep 11, 2017 at 01:02:52PM -0700, Benson Leung wrote:
>> Thanks for the pointer. I'll respin the patch with the no_resume version of
>> usb_autopm_get_interface and retest.
>>
>
> I went and tried this patch but with usb_autopm_get_interface_no_resume instead
> and the original bug I was trying to fix with a Yubikey hid security key came
> back, so something is still wrong here.
>
> I went ahead and filed a bug to track this here:
> https://bugs.chromium.org/p/chromium/issues/detail?id=764112
>
> I'll find some time to dig into this deeper and understand why this device
> ends up getting stuck in active instead of suspending when all handles are
> closed.

Meh, it is problem in hidraw that basically does
usb_autopm_put_interface() before letting usbhid_close() to run. Once
I swap  hid_hw_power() and hid_hw_close() it autosuspends properly.

I just sent out a patch.

Thanks.

-- 
Dmitry

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web