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]

Message-ID: <20150310122307.GM1563@lahna.fi.intel.com>
Date:	Tue, 10 Mar 2015 14:23:07 +0200
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	Doug Anderson <dianders@...omium.org>
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-kernel@...r.kernel.org
Subject: Re: [PATCH v2] HID: i2c-hid: Fix suspend/resume when already runtime
 suspended

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
  [   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).
--
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/