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
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
