Mika, On Tue, Mar 10, 2015 at 5:23 AM, Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxx> >> --- >> 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-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
