Re: debug needed: twl4030 RTC wakeups: repeated attempts fail on Beagle

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3768 bytes --]

On Fri, 10 Aug 2012 11:49:27 -0700 Kevin Hilman <khilman@ti.com> wrote:

> Hello,
> 
> In doing some automated testing of suspend/resume I noticed that
> repeated attempts to suspend and resume via RTC wakeup fail on
> 3530/Beagle and 3730/Beagle-xM, but work fine on 3430/n900, 3530/Overo,
> 3730/OveroSTORM and 4430/Panda.
> 
> When RTC wakeup fails, a UART wakeup will work, and in the logs, you'll
> see this:
> 
> [  316.036132] twl: i2c_read failed to transfer all messages
> [  316.036163] twl4030: I2C error -13 reading PIH ISR
> 
> My guess about what might be happening is that very late in the suspend
> process (during the noirq hooks), a PMIC interrupt fires, but by this
> time the I2C driver is runtime suspended (and clock gated.)  Since
> runtime PM is disabled at this point, I2C reads fail, so the twl4030 IRQ
> driver cannot talk over I2C to the PMIC to determine the interrupt
> source.

This area seems to be rife with opportunity for bugs.  I wrote about some of
it here:  https://lwn.net/Articles/482345/

I don't know that I saw quite what you are seeing though.

If a PMIC interrupt fires during the noirq phase, the interrupt handler
shouldn't be run (it isn't marked NOSUSPEND).  However there is probably room
for a race between the 'suspend' phase and the 'noirq' phase.

When the suspend processing handles the I2C device, the last thing that
__device_suspend does is

		__pm_runtime_disable(dev, false);

which will freeze the current runtime_pm state of the I2C device.  If it is
off, it stays off.  If on, it stays on.

As the noirq phase hasn't been entered yet an interrupt from the PMIC could
still be handled.  If it is, you get exactly the error you see.

I'm not convinced that the __pm_runtime_disable call is correct.  It think we
need to stop async runtime_suspends, but we don't need to stop sync
runtime_resumes. So just a pm_runtime_get should be enough.  But there  is
possibly an important point I am missing.

However if my analysis is correct, then this can be 'fixed' by changing the
omap i2c suspend routine to do a pm_runtime_get, and the resume routine to do
a pm_runtime_put.  The I2C will still be put to sleep during suspend by the
noirq suspend handler, but we will be sure of it being awake during the
crucial suspend and resume transition.
See also https://lwn.net/Articles/505683/.  Particularly (towards the end)

    If the device might be needed to power down other devices, such as an I2C
    controller that might be needed to tell some regulator to turn off, then the
    device should be activated for runtime PM purposes so that it will still be
    active when runtime PM is disabled. 

(Rafael reviewed this article so it shouldn't be very far from the mark).


> 
> The real mystery is why this happens on Beagle and Beagle-xM, but none
> of the other OMAP3 boards (at least the ones I have.)

Maybe didn't components of the PMIC are active and have the potential to
generate an interrupt at an awkward time.  USB and battery chargers seem good
at that.

Or maybe due to the particular components active and the particular timing,
the pm_runtime_disable ends up freezing the runtime_pm state in 'on' rather
than 'off'.

> 
> Reproducing is easy.  Simply run rtcwake in a loop:
> 
>   # while true; do rtcwake -m mem -s 1; done
> 
> In my tests, this happens using omap2plus_defconfig (+ initramfs) on
> v3.6-rc1, v3.5, v3.4, v3.3 but seems to work fine on v3.2.
> 
> I'm going on vacation for a few weeks, so any help debugging this would
> be greatly appreciated. 

Enjoy your vacation!   I don't suppose it ends up in San Diego in late August
for one of the multitude of conferences there?

NeilBrown



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]