From 8d921815bf0d3424fec28dc13be0294fae3978d1 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Thu, 5 Sep 2013 09:22:07 -0700 Subject: Add comments about hooks and deferred functions No code changes, only comments, and making gaia_suspend_deferred() static. BUG=none TEST=build pit BRANCH=none Change-Id: I96448bd3b7457f2a0ec25276167d2740d0cd0c52 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/168231 Reviewed-by: Bill Richardson --- common/chipset_gaia.c | 13 ++++++++++++- include/hooks.h | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/common/chipset_gaia.c b/common/chipset_gaia.c index baeb7d1251..19619d601b 100644 --- a/common/chipset_gaia.c +++ b/common/chipset_gaia.c @@ -244,7 +244,18 @@ static int check_for_power_off_event(void) return 0; } -void gaia_suspend_deferred(void) +/** + * Deferred handling for suspend events + * + * The suspend event needs to be able to call the suspend and resume hooks. + * This cannot be done from interrupt level, since the handlers from those + * hooks may need to use mutexes or other functionality not present at + * interrupt level. Use a deferred function instead. + * + * Deferred functions are called from the hook task and not the chipset task, + * so that's a slight deviation from the spec in hooks.h, but a minor one. + */ +static void gaia_suspend_deferred(void) { int new_ap_suspended; diff --git a/include/hooks.h b/include/hooks.h index 6862005379..da46610505 100644 --- a/include/hooks.h +++ b/include/hooks.h @@ -162,6 +162,13 @@ void hook_init(void); /** * Call all the hook routines of a specified type. * + * This function must be called from the correct type-specific context (task); + * see enum hook_type for details. hook_notify() should NEVER be called from + * interrupt context unless specifically allowed for a hook type, because hook + * routines may need to perform task-level calls like usleep() and mutex + * operations that are not valid in interrupt context. Instead of calling a + * hook from interrupt context, use a deferred function. + * * @param type Type of hook routines to call. */ void hook_notify(enum hook_type type); @@ -186,6 +193,21 @@ int hook_call_deferred(void (*routine)(void), int us); /** * Register a hook routine. * + * NOTE: Hook routines must be careful not to leave resources locked which may + * be needed by other hook routines or deferred function calls. This can cause + * a deadlock, because most hooks and all deferred functions are called from + * the same hook task. For example: + * + * hook1(): lock foo + * deferred1(): lock foo, use foo, unlock foo + * hook2(): unlock foo + * + * In this example, hook1() and hook2() lock and unlock a shared resource foo + * (for example, a mutex). If deferred1() attempts to lock the resource, it + * will stall waiting for the resource to be unlocked. But the unlock will + * never happen, because hook2() won't be called by the hook task until + * deferred1() returns. + * * @param hooktype Type of hook for routine (enum hook_type) * @param routine Hook routine, with prototype void routine(void) * @param priority Priority for determining when routine is called vs. @@ -211,6 +233,12 @@ struct deferred_data { * Note that if you declare a bunch of these, you may need to override * DEFERRABLE_MAX_COUNT in your board.h. * + * NOTE: Deferred function call routines must be careful not to leave resources + * locked which may be needed by other hook routines or deferred function + * calls. This can cause a deadlock, because most hooks and all deferred + * functions are called from the same hook task. See DECLARE_HOOK() for an + * example. + * * @param routine Function pointer, with prototype void routine(void) */ #define DECLARE_DEFERRED(routine) \ -- cgit v1.2.1