Thread (1 message) 1 message, 1 author, 2014-06-23

[PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume

From: Kevin Hilman <hidden>
Date: 2014-06-23 23:35:26
Also in: linux-i2c, linux-samsung-soc, lkml

Tomasz Figa [off-list ref] writes:
On 24.06.2014 00:27, Doug Anderson wrote:
quoted
Kevin,

On Mon, Jun 23, 2014 at 3:19 PM, Kevin Hilman [off-list ref] wrote:
quoted
Doug Anderson [off-list ref] writes:

[...]
quoted
On Fri, Jun 20, 2014 at 4:59 PM, Tomasz Figa [off-list ref] wrote:
quoted
I'm not sure noirq is going to work correctly, at least not with current
callbacks. I can see a call to clk_prepare_enable() there which needs to
acquire a mutex.
Nice catch, thanks!  :)

OK, looking at that now.  Interestingly this doesn't seem to cause us
problems in our ChromeOS 3.8 tree.  I just tried enabling:
  CONFIG_DEBUG_ATOMIC_SLEEP=y

...and confirmed that I got it on right:

# zgrep -i atomic /proc/config.gz
CONFIG_DEBUG_ATOMIC_SLEEP=y

I can suspend/resume with no problems.  My bet is that it works fine because:

* resume_noirq is not considered "atomic" in the sense enforced by
CONFIG_DEBUG_ATOMIC_SLEEP (at least not in 3.8--I haven't tried on
ToT)
The reason is because "noirq" in the suspend/resume path actually means
no *device* IRQs for that specific device.

It's often assumed that the "noirq" callbacks are called with *all*
interrupts disabled, but that's not the case.  Only the IRQs for that
specific device are disabled when its noirq callbacks run.
Ah, so even with my fix of moving to noirq we could still be broken if
the system decided to enable interrupts for the device before the i2c
controller get resumed then we'd still be SOL.

...oh, but if it matches probe order then maybe we're guaranteed for
that not to happen?  We know that we will probe the i2c bus before the
devices on it, right?
If the mentioned device is a child of the I2C controller then the
parent-child relation determines the order. Otherwise (e.g. another,
non-I2C interrupt source that just triggers some operation on an I2C
device like voltage regulator) we're doomed. ;)
Exactly.  There are lots of dragons hiding here.   

Runtime PM is your friend. ;)

Kevin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help