linux-kernel - Re: [PATCH v2] HID: i2c-hid: Fix suspend/resume when already runtime suspended

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Mar 2015 09:12:36 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:	Jiri Kosina <jkosina@...e.cz>,
	Andrew Duggan <aduggan@...aptics.com>,
	Vincent Huang <vincent.huang@...synaptics.com>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	jmaneyrol@...ensense.com, borneo.antonio@...il.com,
	seth.forshee@...onical.com, archana.patni@...ux.intel.com,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] HID: i2c-hid: Fix suspend/resume when already runtime suspended

Mika,

On Tue, Mar 10, 2015 at 5:23 AM, Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> On Mon, Mar 09, 2015 at 12:44:47PM -0700, Doug Anderson wrote:
>> If the i2c-hid device was runtime suspended and then the system
>> suspended itself we'd end up disabling interrupts twice (in
>> i2c_hid_runtime_suspend and i2c_hid_suspend) and not reenabling them
>> until later when the i2c-hid device was runtime resumed.
>> Unfortunately the i2c_hid_resume() calls i2c_hid_hwreset() and that
>> only works properly if interrupts are enabled.
>>
>> We can fix this by taking the advice from "runtime_pm.txt".
>> Specifically we'll change i2c-hid to always resume to full power.
>> This only works if our parents are also resumed to full power, but
>> given the suggestion in "runtime_pm.txt" this seems a reasonable
>> assumption.
>>
>> Signed-off-by: Doug Anderson <dianders@...omium.org>
>> ---
>> Note that this was tested on a 3.14 kernel with backports.  Any
>> testing that folks can do on ToT is appreciated.
>>
>> Changes in v2:
>> - Move pm_runtime_enable() higher in resume
>
> Hmm, I tried on one machine with Atmel touch panel. I modified i2c-hid.c
> a bit to print out when it is running suspend/resume hooks. Here is what
> I got:
>
>   [    7.506455] i2c_hid i2c-ATML1000:00: PM: i2c-hid runtime suspend
>
> This is the initialization and after that the device is runtime
> suspended.
>
> Now I put the machine to system sleep (using echo mem >
> /sys/power/state):
>
>   [   26.988867] i2c_hid i2c-ATML1000:00: PM: i2c-hid runtime resume

Thanks for testing!  Can you do a "dump_stack()" here?  I'm curious
why it's deciding to runtime resume.  Maybe something changed between
3.14 and ToT?

>   [   27.002957] i2c_hid i2c-ATML1000:00: PM: i2c-hid suspend
>
> So the PM core will kick the device out of runtime suspend before it
> suspends it again.
>
> Pressing power button will resume the device:
>
>   [   28.238934] i2c_hid i2c-ATML1000:00: PM: i2c-hid resume
>   [   29.941579] i2c_hid i2c-ATML1000:00: PM: i2c-hid runtime suspend
>
> and since the device is not being used, it will runtime suspend itself.
>
> In other words, I'm not sure if this patch is needed. It would be good
> if you could do similar testing using mainline kernel (if possible at
> all).

Unfortunately it's not so easy.  For my platform suspend/resume is
just barely functional as it is, but then I'm on a board that's
further not supported upstream.

If folks upstream can't reproduce the problems then there's no reason
to take my patch...

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/