cleanup: Even more TODO comments

Update comments with more info, or remove if no longer applicable.

No code changes.

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

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

13 files changed, 38 insertions, 32 deletions

diff --git a/board/kirby/board.c b/board/kirby/board.c
index 9413e74ce6..42241b308d 100644
--- a/board/kirby/board.c
+++ b/board/kirby/board.c
@@ -88,8 +88,9 @@ const struct gpio_info gpio_list[] = {
 	{"KB_OUT11",      GPIO_D, (1<<11), GPIO_KB_OUTPUT, NULL},
 	{"KB_OUT12",      GPIO_D, (1<<12), GPIO_KB_OUTPUT, NULL},
 	/*
-	 * Unimplemented
-	 * TODO: Remove these when codes compile without them
+	 * TODO(crosbug.com/p/23802): These pins are never used for I2C on
+	 * Kirby, but the I2C code won't currently compile unless they're
+	 * defined.
 	 */
 	GPIO_SIGNAL_NOT_IMPLEMENTED("I2C2_SCL"),
 	GPIO_SIGNAL_NOT_IMPLEMENTED("I2C2_SDA"),
diff --git a/board/rambi/ec.tasklist b/board/rambi/ec.tasklist
index f6e061cb4c..9d016bbce5 100644
--- a/board/rambi/ec.tasklist
+++ b/board/rambi/ec.tasklist
@@ -17,7 +17,6 @@
  * 'd' is an opaque parameter passed to the routine at startup
  * 's' is the stack size in bytes; must be a multiple of 8
  */
-/* TODO(rspangler): re-enable tasks once initial bringup is done */
 #define CONFIG_TASK_LIST \
 	TASK_ALWAYS(HOOKS, hook_task, NULL, TASK_STACK_SIZE) \
 	TASK_NOTEST(VBOOTHASH, vboot_hash_task, NULL, LARGER_TASK_STACK_SIZE) \
diff --git a/board/snow/board.c b/board/snow/board.c
index 9b7a0fcd56..1540612252 100644
--- a/board/snow/board.c
+++ b/board/snow/board.c
@@ -99,8 +99,8 @@ BUILD_ASSERT(ARRAY_SIZE(gpio_list) == GPIO_COUNT);
 /* Pins with alternate functions */
 const struct gpio_alt_func gpio_alt_funcs[] = {
 	/*
-	 * TODO(rspangler): use this instead of hard-coded register writes in
-	 * board_config_pre_init().
+	 * TODO(crosbug.com/p/21618): use this instead of hard-coded register
+	 * writes in board_config_pre_init().
 	 */
 };
 const int gpio_alt_funcs_count = ARRAY_SIZE(gpio_alt_funcs);
@@ -132,9 +132,7 @@ void board_config_pre_init(void)
 {
 	uint32_t val;
 
-	/* Enable all GPIOs clocks
-	 * TODO: more fine-grained enabling for power saving
-	 */
+	/* Enable all GPIOs clocks */
 	STM32_RCC_APB2ENR |= 0x1fd;
 
 	/* remap OSC_IN/OSC_OUT to PD0/PD1 */
diff --git a/board/spring/board.c b/board/spring/board.c
index 29505bb4aa..faa0c179c0 100644
--- a/board/spring/board.c
+++ b/board/spring/board.c
@@ -98,8 +98,8 @@ BUILD_ASSERT(ARRAY_SIZE(gpio_list) == GPIO_COUNT);
 /* Pins with alternate functions */
 const struct gpio_alt_func gpio_alt_funcs[] = {
 	/*
-	 * TODO(rspangler): use this instead of hard-coded register writes in
-	 * board_config_pre_init().
+	 * TODO(crosbug.com/p/21618): use this instead of hard-coded register
+	 * writes in board_config_pre_init().
 	 */
 };
 const int gpio_alt_funcs_count = ARRAY_SIZE(gpio_alt_funcs);
@@ -135,9 +135,7 @@ void board_config_pre_init(void)
 {
 	uint32_t val;
 
-	/* Enable all GPIOs clocks
-	 * TODO: more fine-grained enabling for power saving
-	 */
+	/* Enable all GPIOs clocks */
 	STM32_RCC_APB2ENR |= 0x1fd;
 
 	/* remap OSC_IN/OSC_OUT to PD0/PD1 */
diff --git a/chip/lm4/clock.c b/chip/lm4/clock.c
index bbd2e503d1..3f40264ef5 100644
--- a/chip/lm4/clock.c
+++ b/chip/lm4/clock.c
@@ -168,10 +168,11 @@ void clock_init(void)
 #endif
 
 	/*
-	 * TODO: UART seems to glitch unless we wait 500k cycles before
-	 * enabling the PLL, but only if this is a cold boot.  Why?  UART
-	 * doesn't even use the PLL'd system clock.  I've heard rumors the
-	 * Stellaris ROM library does this too, but why?
+	 * TODO(crosbug.com/p/23794): UART seems to glitch unless we wait 500k
+	 * cycles before enabling the PLL, but only if this is a cold boot.
+	 * Why?  UART doesn't even use the PLL'd system clock.  I've heard
+	 * rumors the Stellaris ROM library does this too, but why?  Revisit on
+	 * current systems to see if this is is still needed.
 	 */
 	if (!system_jumped_to_this_image())
 		clock_wait_cycles(500000);
@@ -457,9 +458,10 @@ static int command_sleep(int argc, char **argv)
 		}
 
 		/*
-		 * TODO: move this to the UART module; ugly to have
-		 * UARTisms here.  Also note this only fixes UART0,
-		 * not UART1.
+		 * TODO(crosbug.com/p/23795): move this to the UART module;
+		 * ugly to have UARTisms here.  Also note this only fixes
+		 * UART0, not UART1.  Should just be able to trigger
+		 * HOOK_FREQ_CHANGE and have that take care of it.
 		 */
 		if (uartfbrd) {
 			/* Disable the port via UARTCTL and add HSE. */
diff --git a/chip/lm4/config_chip.h b/chip/lm4/config_chip.h
index a534bb7a57..81ceed1b09 100644
--- a/chip/lm4/config_chip.h
+++ b/chip/lm4/config_chip.h
@@ -98,7 +98,7 @@
 #define CONFIG_FW_RW_OFF		CONFIG_FW_IMAGE_SIZE
 #define CONFIG_FW_RW_SIZE		CONFIG_FW_IMAGE_SIZE
 
-/* TODO: why 2 sets of configs with the same numbers? */
+/* TODO(crosbug.com/p/23796): why 2 sets of configs with the same numbers? */
 #define CONFIG_FW_WP_RO_OFF		CONFIG_FW_RO_OFF
 #define CONFIG_FW_WP_RO_SIZE		CONFIG_FW_RO_SIZE
 
diff --git a/chip/stm32/flash-stm32f.c b/chip/stm32/flash-stm32f.c
index c0f5ae4cd3..c14e37044c 100644
--- a/chip/stm32/flash-stm32f.c
+++ b/chip/stm32/flash-stm32f.c
@@ -48,8 +48,9 @@ static int entire_flash_locked;
 #define FLASH_HOOK_VERSION 1
 /* The previous write protect state before sys jump */
 /*
- * TODO: check if STM32L code works here too - that is, check if entire flash
- * is locked by attempting to lock it rather than keeping a global variable.
+ * TODO(crosbug.com/p/23798): check if STM32L code works here too - that is,
+ * check if entire flash is locked by attempting to lock it rather than keeping
+ * a global variable.
  */
 struct flash_wp_state {
 	int entire_flash_locked;
@@ -478,8 +479,9 @@ int flash_pre_init(void)
 			 * Write protect register was in an inconsistent state.
 			 * Set it back to a good state and reboot.
 			 *
-			 * TODO: this seems really similar to the check above.
-			 * One of them should be able to go away.
+			 * TODO(crosbug.com/p/23798): this seems really similar
+			 * to the check above.  One of them should be able to
+			 * go away.
 			 */
 			flash_protect_ro_at_boot(
 				prot_flags & EC_FLASH_PROTECT_RO_AT_BOOT);
diff --git a/chip/stm32/i2c-stm32f.c b/chip/stm32/i2c-stm32f.c
index 29d88b5660..52b56c363a 100644
--- a/chip/stm32/i2c-stm32f.c
+++ b/chip/stm32/i2c-stm32f.c
@@ -377,6 +377,10 @@ static void unwedge_i2c_bus(int port)
 
 	ASSERT(port == I2C1 || port == I2C2);
 
+	/*
+	 * TODO(crosbug.com/p/23802): This requires defining GPIOs for both
+	 * ports even if the board only supports one port.
+	 */
 	if (port == I2C1) {
 		sda = GPIO_I2C1_SDA;
 		scl = GPIO_I2C1_SCL;
diff --git a/chip/stm32/i2c-stm32l.c b/chip/stm32/i2c-stm32l.c
index 27f6eb92bb..6ff355c0ff 100644
--- a/chip/stm32/i2c-stm32l.c
+++ b/chip/stm32/i2c-stm32l.c
@@ -178,6 +178,10 @@ static void i2c_try_unwedge(int port, int force_unwedge)
 	enum gpio_signal scl, sda;
 	int i;
 
+	/*
+	 * TODO(crosbug.com/p/23802): This requires defining GPIOs for both
+	 * ports even if the board only supports one port.
+	 */
 	if (port == I2C1) {
 		sda = GPIO_I2C1_SDA;
 		scl = GPIO_I2C1_SCL;
diff --git a/common/switch.c b/common/switch.c
index a8cf4eaebd..7a15fb144f 100644
--- a/common/switch.c
+++ b/common/switch.c
@@ -84,8 +84,9 @@ static void switch_init(void)
 #endif
 
 	/*
-	 * TODO(rspangler): It's weird that flash_common.c owns reading the
-	 * write protect signal, but we enable the interrupt for it here.
+	 * TODO(crosbug.com/p/23793): It's weird that flash_common.c owns
+	 * reading the write protect signal, but we enable the interrupt for it
+	 * here.  Take ownership of WP back, or refactor it to its own module.
 	 */
 #ifdef CONFIG_WP_ACTIVE_HIGH
 	gpio_enable_interrupt(GPIO_WP);
diff --git a/power/build.mk b/power/build.mk
index 1c176a3db9..b2cd14fe66 100644
--- a/power/build.mk
+++ b/power/build.mk
@@ -6,7 +6,6 @@
 # Power management for application processor and peripherals
 #
 
-# TODO(rspangler): rename _CHIPSET to _POWER
 power-$(CONFIG_CHIPSET_BAYTRAIL)+=baytrail.o
 power-$(CONFIG_CHIPSET_GAIA)+=gaia.o
 power-$(CONFIG_CHIPSET_HASWELL)+=haswell.o
diff --git a/test/stress.c b/test/stress.c
index d5c6f3a901..d357c389c5 100644
--- a/test/stress.c
+++ b/test/stress.c
@@ -56,8 +56,6 @@ struct i2c_test_param_t {
 /* ADC test */
 #define ADC_TEST_ITERATION 2000
 
-/* TODO(victoryang): PECI test */
-
 /*****************************************************************************/
 /* Test utilities */
 
diff --git a/test/timer_dos.py b/test/timer_dos.py
index 86d9b7ab49..5b7e1f7d40 100644
--- a/test/timer_dos.py
+++ b/test/timer_dos.py
@@ -20,7 +20,7 @@ def period_us(num):
 
 # build the same pseudo random sequence as the target
 def build_sequence():
-    #TODO
+    # TODO(crosbug.com/p/23800): implement
     return []
 
 def test(helper):
@@ -34,7 +34,7 @@ def test(helper):
 
       # Check the results
       model = build_sequence()
-      #TODO
+      # TODO(crosbug.com/p/23800): implement
 
       helper.trace("Got %d timer IRQ\n" % len(seq))