Clean up core routines - cpu, task, watchdog

No functional changes.

BUG=chrome-os-partner:15579
BRANCH=none
TEST=boot system

Change-Id: I55cf9c60e92177fd441614a8f9fce2d3acca3d0e
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36706

4 files changed, 185 insertions, 130 deletions

diff --git a/core/cortex-m/cpu.c b/core/cortex-m/cpu.c
index edf2272256..2571a4a17c 100644
--- a/core/cortex-m/cpu.c
+++ b/core/cortex-m/cpu.c
@@ -7,7 +7,6 @@
 
 #include "cpu.h"
 
-
 void cpu_init(void)
 {
 	/* Catch divide by 0 and unaligned access */
diff --git a/core/cortex-m/task.c b/core/cortex-m/task.c
index c48a5e3068..485e58e06d 100644
--- a/core/cortex-m/task.c
+++ b/core/cortex-m/task.c
@@ -5,8 +5,8 @@
 
 /* Task scheduling / events module for Chrome EC operating system */
 
-#include "config.h"
 #include "atomic.h"
+#include "common.h"
 #include "console.h"
 #include "cpu.h"
 #include "link_defs.h"
@@ -64,20 +64,23 @@ extern int __task_start(int *task_stack_ready);
 /* Idle task.  Executed when no tasks are ready to be scheduled. */
 void __idle(void)
 {
-	/* Print when the idle task starts.  This is the lowest priority task,
+	/*
+	 * Print when the idle task starts.  This is the lowest priority task,
 	 * so this only starts once all other tasks have gotten a chance to do
-	 * their task inits and have gone to sleep. */
+	 * their task inits and have gone to sleep.
+	 */
 	cprintf(CC_TASK, "[%T idle task started]\n");
 
 	while (1) {
-		/* Wait for the next irq event.  This stops the CPU clock
-		 * (sleep / deep sleep, depending on chip config). */
+		/*
+		 * Wait for the next irq event.  This stops the CPU clock
+		 * (sleep / deep sleep, depending on chip config).
+		 */
 		asm("wfi");
 	}
 }
 #endif /* !CONFIG_LOW_POWER_IDLE */
 
-
 static void task_exit_trap(void)
 {
 	int i = task_get_current();
@@ -87,7 +90,6 @@ static void task_exit_trap(void)
 		task_wait_event(-1);
 }
 
-
 /* Startup parameters for all tasks. */
 #define TASK(n, r, d, s)  {	\
 	.r0 = (uint32_t)d,	\
@@ -109,7 +111,6 @@ static const struct {
 static task_ tasks[TASK_ID_COUNT];
 
 /* Stacks for all tasks */
-/* TODO: variable-size stacks */
 #define TASK(n, r, d, s)  + s
 #include TASK_LIST
 uint8_t task_stacks[0
@@ -128,7 +129,8 @@ uint32_t scratchpad[17];
 
 static task_ *current_task = (task_ *)scratchpad;
 
-/* Should IRQs chain to svc_handler()?  This should be set if either of the
+/*
+ * Should IRQs chain to svc_handler()?  This should be set if either of the
  * following is true:
  *
  * 1) Task scheduling has started, and task profiling is enabled.  Task
@@ -136,11 +138,12 @@ static task_ *current_task = (task_ *)scratchpad;
  *
  * 2) An event was set by an interrupt; this could result in a higher-priority
  * task unblocking.  After checking for a task switch, svc_handler() will clear
- * the flag (unless profiling is also enabled; then the flag remains set). */
+ * the flag (unless profiling is also enabled; then the flag remains set).
+ */
 static int need_resched_or_profiling = 0;
 
-/**
- * bitmap of all tasks ready to be run
+/*
+ * Bitmap of all tasks ready to be run.
  *
  * Currently all tasks are enabled at startup.
  */
@@ -153,19 +156,16 @@ static inline task_ *__task_id_to_ptr(task_id_t id)
 	return tasks + id;
 }
 
-
 void interrupt_disable(void)
 {
 	asm("cpsid i");
 }
 
-
 void interrupt_enable(void)
 {
 	asm("cpsie i");
 }
 
-
 inline int in_interrupt_context(void)
 {
 	int ret;
@@ -174,12 +174,11 @@ inline int in_interrupt_context(void)
 	return ret;
 }
 
-
 inline int get_interrupt_context(void)
 {
 	int ret;
 	asm("mrs %0, ipsr \n":"=r"(ret)); /* read exception number */
-	return ret & 0x1ff;                /* exception bits are the 9 LSB */
+	return ret & 0x1ff;               /* exception bits are the 9 LSB */
 }
 
 task_id_t task_get_current(void)
@@ -193,14 +192,14 @@ uint32_t *task_get_event_bitmap(task_id_t tskid)
 	return &tsk->events;
 }
 
-
 int task_start_called(void)
 {
 	return start_called;
 }
 
-
-/* Scheduling system call */
+/**
+ * Scheduling system call
+ */
 void svc_handler(int desched, task_id_t resched)
 {
 	task_ *current, *next;
@@ -209,14 +208,18 @@ void svc_handler(int desched, task_id_t resched)
 	uint64_t t;
 #endif
 
-	/* Push the priority to -1 until the return, to avoid being
-	 * interrupted */
+	/*
+	 * Push the priority to -1 until the return, to avoid being
+	 * interrupted.
+	 */
 	asm volatile("cpsid f\n"
 		     "isb\n");
 
 #ifdef CONFIG_TASK_PROFILING
-	/* SVCall isn't triggered via DECLARE_IRQ(), so it needs to track its
-	 * start time explicitly. */
+	/*
+	 * SVCall isn't triggered via DECLARE_IRQ(), so it needs to track its
+	 * start time explicitly.
+	 */
 	if (exc == 0xb) {
 		exc_start_time = get_time().val;
 		svc_calls++;
@@ -245,13 +248,17 @@ void svc_handler(int desched, task_id_t resched)
 	t = get_time().val;
 	exc_total_time += (t - exc_start_time);
 
-	/* Bill the current task for time between the end of the last interrupt
-	 * and the start of this one. */
+	/*
+	 * Bill the current task for time between the end of the last interrupt
+	 * and the start of this one.
+	 */
 	current->runtime += (exc_start_time - exc_end_time);
 	exc_end_time = t;
 #else
-	/* Don't chain here from interrupts until the next time an interrupt
-	 * sets an event. */
+	/*
+	 * Don't chain here from interrupts until the next time an interrupt
+	 * sets an event.
+	 */
 	need_resched_or_profiling = 0;
 #endif
 
@@ -267,13 +274,14 @@ void svc_handler(int desched, task_id_t resched)
 	__switchto(current, next);
 }
 
-
 void __schedule(int desched, int resched)
 {
 	register int p0 asm("r0") = desched;
 	register int p1 asm("r1") = resched;
-	/* TODO: remove hardcoded opcode.  SWI is not compiled properly for
-	 * ARMv7-M on our current chroot toolchain. */
+	/*
+	 * TODO: remove hardcoded opcode.  SWI is not compiled properly for
+	 * ARMv7-M on our current chroot toolchain.
+	 */
 	asm(".hword 0xdf00 @swi 0"::"r"(p0),"r"(p1));
 }
 
@@ -281,19 +289,25 @@ void __schedule(int desched, int resched)
 #ifdef CONFIG_TASK_PROFILING
 void task_start_irq_handler(void *excep_return)
 {
-	/* Get time before checking depth, in case this handler is
-	 * pre-empted */
+	/*
+	 * Get time before checking depth, in case this handler is
+	 * pre-empted.
+	 */
 	uint64_t t = get_time().val;
 	int irq = get_interrupt_context() - 16;
 
-	/* Track IRQ distribution.  No need for atomic add, because an IRQ
-	* can't pre-empt itself. */
+	/*
+	 * Track IRQ distribution.  No need for atomic add, because an IRQ
+	 * can't pre-empt itself.
+	 */
 	if (irq < ARRAY_SIZE(irq_dist))
 		irq_dist[irq]++;
 
-	/* Continue iff a rescheduling event happened or profiling is active,
+	/*
+	 * Continue iff a rescheduling event happened or profiling is active,
 	 * and we are not called from another exception (this must match the
-	 * logic for when we chain to svc_handler() below). */
+	 * logic for when we chain to svc_handler() below).
+	 */
 	if (!need_resched_or_profiling || (((uint32_t)excep_return & 0xf) == 1))
 		return;
 
@@ -301,18 +315,18 @@ void task_start_irq_handler(void *excep_return)
 }
 #endif
 
-
 void task_resched_if_needed(void *excep_return)
 {
-	/* Continue iff a rescheduling event happened or profiling is active,
-	 * and we are not called from another exception. */
+	/*
+	 * Continue iff a rescheduling event happened or profiling is active,
+	 * and we are not called from another exception.
+	 */
 	if (!need_resched_or_profiling || (((uint32_t)excep_return & 0xf) == 1))
 		return;
 
 	svc_handler(0, 0);
 }
 
-
 static uint32_t __wait_evt(int timeout_us, task_id_t resched)
 {
 	task_ *tsk = current_task;
@@ -339,7 +353,6 @@ static uint32_t __wait_evt(int timeout_us, task_id_t resched)
 	return evt;
 }
 
-
 uint32_t task_set_event(task_id_t tskid, uint32_t event, int wait)
 {
 	task_ *receiver = __task_id_to_ptr(tskid);
@@ -365,39 +378,35 @@ uint32_t task_set_event(task_id_t tskid, uint32_t event, int wait)
 	return 0;
 }
 
-
 uint32_t task_wait_event(int timeout_us)
 {
 	return __wait_evt(timeout_us, TASK_ID_IDLE);
 }
 
-
 void task_enable_irq(int irq)
 {
 	CPU_NVIC_EN(irq / 32) = 1 << (irq % 32);
 }
 
-
 void task_disable_irq(int irq)
 {
 	CPU_NVIC_DIS(irq / 32) = 1 << (irq % 32);
 }
 
-
 void task_clear_pending_irq(int irq)
 {
 	CPU_NVIC_UNPEND(irq / 32) = 1 << (irq % 32);
 }
 
-
 void task_trigger_irq(int irq)
 {
 	CPU_NVIC_SWTRIG = irq;
 }
 
-
-/* Initialize IRQs in the NVIC and set their priorities as defined by the
- * DECLARE_IRQ statements. */
+/*
+ * Initialize IRQs in the NVIC and set their priorities as defined by the
+ * DECLARE_IRQ statements.
+ */
 static void __nvic_init_irqs(void)
 {
 	/* Get the IRQ priorities section from the linker */
@@ -410,9 +419,11 @@ static void __nvic_init_irqs(void)
 		CPU_NVIC_UNPEND(i) = 0xffffffff;
 	}
 
-	/* Re-enable global interrupts in case they're disabled.  On a reboot,
+	/*
+	 * Re-enable global interrupts in case they're disabled.  On a reboot,
 	 * they're already enabled; if we've jumped here from another image,
-	 * they're not. */
+	 * they're not.
+	 */
 	interrupt_enable();
 
 	/* Set priorities */
@@ -427,7 +438,6 @@ static void __nvic_init_irqs(void)
 	}
 }
 
-
 void mutex_lock(struct mutex *mtx)
 {
 	uint32_t value;
@@ -437,26 +447,24 @@ void mutex_lock(struct mutex *mtx)
 	atomic_or(&mtx->waiters, id);
 
 	do {
-		/* try to get the lock (set 1 into the lock field) */
+		/* Try to get the lock (set 1 into the lock field) */
 		__asm__ __volatile__("   ldrex   %0, [%1]\n"
 				     "   teq     %0, #0\n"
 				     "   it eq\n"
 				     "   strexeq %0, %2, [%1]\n"
 				     : "=&r" (value)
 				     : "r" (&mtx->lock), "r" (2) : "cc");
-		/* "value" is equals to 1 if the store conditional failed,
+		/*
+		 * "value" is equals to 1 if the store conditional failed,
 		 * 2 if somebody else owns the mutex, 0 else.
 		 */
-		if (value == 2) {
-			/* contention on the mutex */
-			task_wait_event(0);
-		}
+		if (value == 2)
+			task_wait_event(0);  /* Contention on the mutex */
 	} while (value);
 
 	atomic_clear(&mtx->waiters, id);
 }
 
-
 void mutex_unlock(struct mutex *mtx)
 {
 	uint32_t waiters;
@@ -469,10 +477,12 @@ void mutex_unlock(struct mutex *mtx)
 			     : "cc");
 	while (waiters) {
 		task_id_t id = 31 - __builtin_clz(waiters);
-		/* somebody is waiting on the mutex */
+
+		/* Somebody is waiting on the mutex */
 		task_set_event(id, TASK_EVENT_MUTEX, 0);
 		waiters &= ~(1 << id);
 	}
+
 	/* Ensure no event is remaining from mutex wake-up */
 	atomic_clear(&tsk->events, TASK_EVENT_MUTEX);
 }
@@ -501,10 +511,8 @@ void task_print_list(void)
 	}
 }
 
-
 #ifdef CONFIG_DEBUG
 
-
 int command_task_info(int argc, char **argv)
 {
 #ifdef CONFIG_TASK_PROFILING
@@ -539,7 +547,6 @@ DECLARE_CONSOLE_COMMAND(taskinfo, command_task_info,
 			"Print task info",
 			NULL);
 
-
 static int command_task_ready(int argc, char **argv)
 {
 	if (argc < 2) {
@@ -557,11 +564,9 @@ DECLARE_CONSOLE_COMMAND(taskready, command_task_ready,
 			"Print/set ready tasks",
 			NULL);
 
-
 #endif  /* CONFIG_DEBUG */
 
-
-int task_pre_init(void)
+void task_pre_init(void)
 {
 	uint32_t *stack_next = (uint32_t *)task_stacks;
 	int i;
@@ -614,11 +619,8 @@ int task_pre_init(void)
 
 	/* Initialize IRQs */
 	__nvic_init_irqs();
-
-	return EC_SUCCESS;
 }
 
-
 int task_start(void)
 {
 #ifdef CONFIG_TASK_PROFILING
diff --git a/core/cortex-m/watchdog.c b/core/cortex-m/watchdog.c
index ae26734692..36e0ae43e1 100644
--- a/core/cortex-m/watchdog.c
+++ b/core/cortex-m/watchdog.c
@@ -40,7 +40,6 @@ void watchdog_trace(uint32_t excep_lr, uint32_t excep_sp)
 	task_print_list();
 }
 
-
 /* Low priority task to reload the watchdog */
 void watchdog_task(void)
 {
diff --git a/include/task.h b/include/task.h
index c6b8b4591d..1a7fb1b86f 100644
--- a/include/task.h
+++ b/include/task.h
@@ -5,120 +5,167 @@
 
 /* Task scheduling / events module for Chrome EC operating system */
 
-#ifndef __EC_TASK_H
-#define __EC_TASK_H
+#ifndef __CROS_EC_TASK_H
+#define __CROS_EC_TASK_H
 
-#include "board.h"
 #include "common.h"
 #include "task_id.h"
 
 /* Task event bitmasks */
-#define TASK_EVENT_CUSTOM(x) (x & 0x0fffffff)
-#define TASK_EVENT_I2C_IDLE (1 << 28) /* I2C interrupt handler event. */
-#define TASK_EVENT_WAKE   (1 << 29)  /* task_wake() called on task */
-#define TASK_EVENT_MUTEX  (1 << 30)  /* Mutex unlocking */
-#define TASK_EVENT_TIMER  (1 << 31)  /* Timer expired.  For example,
-				      * task_wait_event() timed out before
-				      * receiving another event. */
-
-/* Disable CPU interrupt bit. This might break the system so think really hard
- * before using these. There are usually better ways of accomplishing this. */
+/* Tasks may use the bits in TASK_EVENT_CUSTOM for their own events */
+#define TASK_EVENT_CUSTOM(x)	(x & 0x0fffffff)
+/* I2C interrupt handler event */
+#define TASK_EVENT_I2C_IDLE	(1 << 28)
+/* task_wake() called on task */
+#define TASK_EVENT_WAKE		(1 << 29)
+/* Mutex unlocking */
+#define TASK_EVENT_MUTEX	(1 << 30)
+/*
+ * Timer expired.  For example, task_wait_event() timed out before receiving
+ * another event.
+ */
+#define TASK_EVENT_TIMER	(1U << 31)
+
+/**
+ * Disable CPU interrupt bit.
+ *
+ * This might break the system so think really hard before using these. There
+ * are usually better ways of accomplishing this.
+ */
 void interrupt_disable(void);
 
-/* Enable CPU interrupt bit. */
+/**
+ * Enable CPU interrupt bit.
+ */
 void interrupt_enable(void);
 
-/* Return true if we are in interrupt context. */
+/**
+ * Return true if we are in interrupt context.
+ */
 inline int in_interrupt_context(void);
 
-/* Set an event for task <tskid> and wake it up if it is higher priority than
- * the current task.
- *
- * event : event bitmap to set (TASK_EVENT_*)
+/**
+ * Set a task event.
  *
- * If wait!=0, after setting the event, de-schedule the calling task to wait
- * for a response event, then return the bitmap of events which have occured
- * (same as task_wait_event()).  Ignored in interrupt context.
+ * If the task is higher priority than the current task, this will cause an
+ * immediate context switch to the new task.
  *
- * If wait==0, returns 0.
+ * Can be called both in interrupt context and task context.
  *
- * Can be called both in interrupt context and task context. */
+ * @param tskid		Task to set event for
+ * @param event		Event bitmap to set (TASK_EVENT_*)
+ * @param wait		If non-zero, after setting the event, de-schedule the
+ *			calling task to wait for a response event.  Ignored in
+ *			interrupt context.
+ * @return		The bitmap of events which occurred if wait!=0, else 0.
+ */
 uint32_t task_set_event(task_id_t tskid, uint32_t event, int wait);
 
-/* Wake a task.  This sends it the TASK_EVENT_WAKE event. */
+/**
+ * Wake a task.  This sends it the TASK_EVENT_WAKE event.
+ *
+ * @param tskid		Task to wake
+ */
 static inline void task_wake(task_id_t tskid)
 {
 	task_set_event(tskid, TASK_EVENT_WAKE, 0);
 }
 
-/* Return the identifier of the task currently running. */
+/**
+ * Return the identifier of the task currently running.
+ */
 task_id_t task_get_current(void);
 
-/* Return a pointer to the bitmap of events of the task. */
-uint32_t *task_get_event_bitmap(task_id_t tsk);
+/**
+ * Return a pointer to the bitmap of events of the task.
+ */
+uint32_t *task_get_event_bitmap(task_id_t tskid);
 
-/* Wait for the next event.
+/**
+ * Wait for the next event.
  *
  * If one or more events are already pending, returns immediately.  Otherwise,
  * it de-schedules the calling task and wakes up the next one in the priority
- * order.
+ * order.  Automatically clears the bitmap of received events before returning
+ * the events which are set.
  *
- * If timeout_us > 0, it also sets a timer to produce the TASK_EVENT_TIMER
- * event after the specified micro-second duration.
+ * @param timeout_us	If > 0, sets a timer to produce the TASK_EVENT_TIMER
+ *			event after the specified micro-second duration.
  *
- * Returns the bitmap of received events (and clears it atomically). */
+ * @return The bitmap of received events. */
 uint32_t task_wait_event(int timeout_us);
 
-/* Prints the list of tasks using the command output channel.  This may be
- * called from interrupt level. */
+/**
+ * Prints the list of tasks.
+ *
+ * Uses the command output channel.  May be called from interrupt level.
+ */
 void task_print_list(void);
 
 #ifdef CONFIG_TASK_PROFILING
-/* Start tracking an interrupt.
+/**
+ * Start tracking an interrupt.
  *
- * This must be called from interrupt context(!) before the interrupt routine
- * is called. */
+ * This must be called from interrupt context (!) before the interrupt routine
+ * is called.
+ */
 void task_start_irq_handler(void *excep_return);
 #else
 #define task_start_irq_handler(excep_return)
 #endif
 
-/* Change the task scheduled after returning from the exception.
+/**
+ * Change the task scheduled to run after returning from the exception.
  *
  * If task_send_event() has been called and has set need_resched flag,
  * re-computes which task is running and eventually swaps the context
  * saved on the process stack to restore the new one at exception exit.
  *
- * This must be called from interrupt context(!) and is designed to be the
- * last call of the interrupt handler. */
+ * This must be called from interrupt context (!) and is designed to be the
+ * last call of the interrupt handler.
+ */
 void task_resched_if_needed(void *excep_return);
 
-/* Initialize tasks and interrupt controller. */
-int task_pre_init(void);
+/**
+ * Initialize tasks and interrupt controller.
+ */
+void task_pre_init(void);
 
-/* Start task scheduling.  Does not normally return. */
+/**
+ * Start task scheduling.  Does not normally return.
+ */
 int task_start(void);
 
-/* Return non-zero if task_start() has been called and task scheduling has
- * started. */
+/**
+ * Return non-zero if task_start() has been called and task scheduling has
+ * started.
+ */
 int task_start_called(void);
 
-/* Enable an interrupt. */
+/**
+ * Enable an interrupt.
+ */
 void task_enable_irq(int irq);
 
-/* Disable an interrupt. */
+/**
+ * Disable an interrupt.
+ */
 void task_disable_irq(int irq);
 
-/* Software-trigger an interrupt. */
+/**
+ * Software-trigger an interrupt.
+ */
 void task_trigger_irq(int irq);
 
-/* Clear a pending interrupt.
+/**
+ * Clear a pending interrupt.
  *
  * Note that most interrupts can be removed from the pending state simply by
  * handling whatever caused the interrupt in the first place.  This only needs
  * to be called if an interrupt handler disables itself without clearing the
  * reason for the interrupt, and then the interrupt is re-enabled from a
- * different context. */
+ * different context.
+ */
 void task_clear_pending_irq(int irq);
 
 struct mutex {
@@ -126,13 +173,19 @@ struct mutex {
 	uint32_t waiters;
 };
 
-/* Try to lock the mutex mtx and de-schedule the current task if mtx is already
- * locked by another task.
+/**
+ * Lock a mutex.
+ *
+ * This tries to lock the mutex mtx.  If the mutex is already locked by another
+ * task, de-schedules the current task until the mutex is again unlocked.
  *
- * Must not be used in interrupt context! */
+ * Must not be used in interrupt context!
+ */
 void mutex_lock(struct mutex *mtx);
 
-/* Release a mutex previously locked by the same task. */
+/**
+ * Release a mutex previously locked by the same task.
+ */
 void mutex_unlock(struct mutex *mtx);
 
 struct irq_priority {
@@ -144,8 +197,10 @@ struct irq_priority {
 #define IRQ_BUILD_NAME(prefix, irqnum, postfix) prefix ## irqnum ## postfix
 #define IRQ_HANDLER(irqname)  IRQ_BUILD_NAME(irq_,irqname,_handler)
 
-/* Connects the interrupt handler "routine" to the irq number "irq" and ensures
- * it is enabled in the interrupt controller with the right priority. */
+/*
+ * Macro to connect the interrupt handler "routine" to the irq number "irq" and
+ * ensure it is enabled in the interrupt controller with the right priority.
+ */
 #define DECLARE_IRQ(irq, routine, priority)                     \
 	void IRQ_HANDLER(irq)(void)				\
 	{							\
@@ -158,4 +213,4 @@ struct irq_priority {
 	__attribute__((section(".rodata.irqprio")))		\
 			= {irq, priority}
 
-#endif  /* __EC_TASK_H */
+#endif  /* __CROS_EC_TASK_H */