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 <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/168231
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

2 files changed, 40 insertions, 1 deletions

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)					\