On 07/07/2011 09:37 AM, Somebody in the thread at some point said:
Hi Kan-Ru -
They have a single wakelock to cover delayed work in there, however there are multiple delayed works possible to be queued, eg delayed disable and delayed detect actions, and although they wrap scheduling the delayed work to also lock the wakelock, they don't wrap cancelling it, eg -->
Yes, they unlock the wakelock when the delayed work is done, in the handler.
Right so cancelling the delayed work means the wakelock is left locked / asserted and we will never do the work to unlock it; the code for this case is broken.
So here when preparing for suspend we can cancel the delayed work we presumably arranged wakelock coverage for, without unlocking the wakelock.
When the delayed work was canceled and handled immediately the wakelock should have been unlocked.
Not certain you meant this by "handled immediately" but to be clear cancel_delayed_work() deletes the timer that creates the delay, and stops the work ever from being executed. In that case the wakelock will be left locked with nothing coming to unlock him except any private wakelock timeout. And I am guessing private wakelock timeouts are there to hide a whole bunch of immortal wakelocks due to broken code that would otherwise kill suspend from ever happening.
The problem is here:
int mmc_pm_notify(struct notifier_block *notify_block, unsigned long mode, void *unused) { ... switch (mode) { case PM_HIBERNATION_PREPARE: case PM_SUSPEND_PREPARE: ... mmc_release_host(host);
Which calls
void mmc_release_host(struct mmc_host *host) { ... mmc_host_lazy_disable(host); ... }
Since omap_hsmmc has non-zero host->disable_delay by default, this will schedule a new delayed_work thus acquire new wakelock.
Okay, I see: thanks for sharing. I will look at the code.
I tried to fix this by ignore the host->disable_delay if host->rescan_disable is non-zero, which means we are in suspend phase, and it worked.
I got the idea from looking at core.c it's full of bad corner cases and potential problems wrt wakelock. I just wanted to confirm it really does suck and it's not just me missing the point somewhere ^^
-Andy