cleanup: Assorted TODO comments

Remove comments if no longer applicable, or assign bug numbers if they
still are.  Tidy some debug output.  No code changes other than the
debug output.

BUG=chrome-os-partner:18343
BRANCH=none
TEST=build all platforms, pass unit tests

Change-Id: I2277e73fbf8cc93f3b1b35ee115e0f2f52eb8cf9
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/175215
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

chip/lm4/gpio.c

@@ -47,8 +47,9 @@ void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func)
 	int port_index = find_gpio_port_index(port);
 	int cgmask;
 
+	/* Ignore (do nothing for) invalid port values */
 	if (port_index < 0)
-		return;  /* TODO: assert */
+		return;
 
 	/* Enable the GPIO port in run and sleep. */
 	cgmask = 1 << port_index;

chip/lm4/peci.c

@@ -16,8 +16,13 @@
 #include "temp_sensor.h"
 #include "util.h"
 
-/* Max junction temperature for processor in degrees C */
-/* TODO: read TjMax from processor via PECI */
+/*
+ * Max junction temperature for processor in degrees C.  This is correct for
+ * Ivy Bridge and Haswell; future chips don't have PECI.
+ *
+ * In theory we could read TjMax from the processor via PECI, but that requires
+ * closed-source Intel PECI commands.
+ */
 #define PECI_TJMAX 105
 
 /* Initial PECI baud rate */
@@ -26,8 +31,12 @@
 /* Polling interval for PECI, in ms */
 #define PECI_POLL_INTERVAL_MS 250
 
-/* Internal and external path delays, in ns */
-#define PECI_TD_FET_NS 60  /* Guess; TODO: what is real delay */
+/*
+ * Internal and external path delays, in ns.  The external delay is a
+ * best-guess measurement, but we're fairly tolerant of a bad guess because
+ * PECI_BAUD_RATE is slow compared to PECI's actual maximum baud rate.
+ */
+#define PECI_TD_FET_NS 60
 #define PECI_TD_INT_NS 80
 
 /* Number of controller retries. Should be between 0 and 7. */

common/extpower_spring.c

@@ -600,8 +600,8 @@ static void usb_device_change(int dev_type)
 /* External API */
 
 /*
- * TODO: Init here until we can do with HOOK_INIT.  Just need to set prio so we
- * init before the charger task does.
+ * TODO(crosbug.com/p/23741): Init here until we can do with HOOK_INIT.  Just
+ * need to set prio so we init before the charger task does.
  */
 void extpower_charge_init(void)
 {

common/host_command.c

@@ -72,44 +72,46 @@ test_mockable void host_send_response(struct host_cmd_handler_args *args)
 {
 #ifdef CONFIG_HOST_COMMAND_STATUS
 	/*
-	 * TODO(sjg@chromium.org):
-	 * If we got an 'in progress' previously, then this
-	 * must be the completion of that command, so stash the result
-	 * code. We can't send it back to the host now since we already sent
-	 * the in-progress response and the host is on to other things now.
 	 *
-	 * If we are in interrupt context, then we are handling a
-	 * get_status response or an immediate error which prevented us
-	 * from processing the command. Note we can't check for the
-	 * GET_COMMS_STATUS command in args->command because the original
-	 * command value has now been overwritten.
+	 * If we are in interrupt context, then we are handling a get_status
+	 * response or an immediate error which prevented us from processing
+	 * the command. Note we can't check for the GET_COMMS_STATUS command in
+	 * args->command because the original command value has now been
+	 * overwritten.
 	 *
 	 * When a EC_CMD_RESEND_RESPONSE arrives we will supply this response
 	 * to that command.
-	 *
-	 * We don't support stashing response data, so mark the response as
-	 * unavailable in that case.
-	 *
-	 * TODO(sjg@chromium.org): If we stashed the command in host_command
-	 * before processing it, then it would not get overwritten by a
-	 * subsequent command and we could simplify the logic here by adding
-	 * a flag to host_cmd_handler_args to indicate that the command had
-	 * an interim response. We would have to make this stashing dependent
-	 * on CONFIG_HOST_COMMAND_STATUS also.
 	 */
 	if (!in_interrupt_context()) {
 		if (command_pending) {
-			CPRINTF("pending complete, size=%d, result=%d\n",
+			/*
+			 * We previously got EC_RES_IN_PROGRESS.  This must be
+			 * the completion of that command, so stash the result
+			 * code.
+			 */
+			CPRINTF("[%T HC pending done, size=%d, result=%d]\n",
 				args->response_size, args->result);
+
+			/*
+			 * We don't support stashing response data, so mark the
+			 * response as unavailable in that case.
+			 */
 			if (args->response_size != 0)
 				saved_result = EC_RES_UNAVAILABLE;
 			else
 				saved_result = args->result;
+
+			/*
+			 * We can't send the response back to the host now
+			 * since we already sent the in-progress response and
+			 * the host is on to other things now.
+			 */
 			command_pending = 0;
 			return;
+
 		} else if (args->result == EC_RES_IN_PROGRESS) {
 			command_pending = 1;
-			CPRINTF("Command pending\n");
+			CPRINTF("[HC pending]\n");
 		}
 	}
 #endif

common/keyboard_scan.c

@@ -301,12 +301,7 @@ static int check_keys_changed(uint8_t *state)
 	/* Read the raw key state */
 	any_pressed = read_matrix(new_state);
 
-	/* Ignore if so many keys are pressed that we're ghosting */
-	/*
-	 * TODO: maybe in this case we should reset all the debounce times,
-	 * because in the ghosting case we're not paying attention to any of
-	 * the keys which aren't ghosting.
-	 */
+	/* Ignore if so many keys are pressed that we're ghosting. */
 	if (has_ghosting(new_state))
 		return any_pressed;
 

common/system.c

@@ -220,8 +220,11 @@ void system_disable_jump(void)
 
 #ifdef CONFIG_MPU
 	/*
-	 * Lock down memory
-	 * TODO: Lock down other images (RO or RW) not running.
+	 * Lock down memory to prevent code execution from data areas.
+	 *
+	 * TODO(crosbug.com/p/16904): Also lock down the image which isn't
+	 * running (RO if RW, or vice versa), so a bad or malicious jump can't
+	 * execute code from that image.
 	 */
 	{
 		int mpu_error = mpu_protect_ram();

common/util.c

@@ -171,7 +171,10 @@ int memcmp(const void *s1, const void *s2, int len)
 
 void *memcpy(void *dest, const void *src, int len)
 {
-	/* TODO: optimized version using LDM/STM would be much faster */
+	/*
+	 * TODO(crosbug.com/p/23720): if src/dest are aligned, copy a word at a
+	 * time instead.
+	 */
 	char *d = (char *)dest;
 	const char *s = (const char *)src;
 	while (len > 0) {
@@ -184,7 +187,10 @@ void *memcpy(void *dest, const void *src, int len)
 
 void *memset(void *dest, int c, int len)
 {
-	/* TODO: optimized version using STM would be much faster */
+	/*
+	 * TODO(crosbug.com/p/23720): if dest is aligned, copy a word at a time
+	 * instead.
+	 */
 	char *d = (char *)dest;
 	while (len > 0) {
 		*(d++) = c;
@@ -205,7 +211,10 @@ void *memmove(void *dest, const void *src, int len)
 		/* Copy from end, so we don't overwrite the source */
 		char *d = (char *)dest + len;
 		const char *s = (const char *)src + len;
-		/* TODO: optimized version using LDM/STM would be much faster */
+		/*
+		 * TODO(crosbug.com/p/23720): if src/dest are aligned, copy a
+		 * word at a time instead.
+		 */
 		while (len > 0) {
 			*(--d) = *(--s);
 			len--;

driver/usb_switch_tsu6721.c

@@ -168,9 +168,9 @@ int tsu6721_init(void)
 	return res ? EC_ERROR_UNKNOWN : EC_SUCCESS;
 }
 /*
- * TODO(vpalatin): using the I2C early in the HOOK_INIT
- * currently triggers all sort of badness, I need to debug
- * this before re-activatin this initialization.
+ * TODO(crosbug.com/p/23741): Using I2C early in the HOOK_INIT currently
+ * triggers all sort of badness.  Debug this before re-activating
+ * initialization in HOOK_INIT.
  */
 #if 0
 DECLARE_HOOK(HOOK_INIT, tsu6721_init, HOOK_PRIO_DEFAULT);

power/baytrail.c

@@ -60,7 +60,6 @@ void chipset_force_shutdown(void)
 	 * Force x86 off. This condition will reset once the state machine
 	 * transitions to G3.
 	 */
-	/* TODO(rspangler): verify this works */
 	gpio_set_level(GPIO_PCH_SYS_PWROK, 0);
 	gpio_set_level(GPIO_PCH_RSMRST_L, 0);
 }