Clean up labels and TODOs

Signed-off-by: Randall Spangler <rspangler@chromium.org>

BUG=none
TEST=none

Change-Id: I8d6e99b3e2b60d32ea3719468590b055e692a67e

10 files changed, 68 insertions, 50 deletions

diff --git a/chip/lm4/clock.c b/chip/lm4/clock.c
index b9f739d703..3ce8637397 100644
--- a/chip/lm4/clock.c
+++ b/chip/lm4/clock.c
@@ -63,7 +63,7 @@ static int command_sleep(int argc, char **argv)
 		clock = strtoi(argv[2], NULL, 10);
 	}
 	/* remove LED current sink  */
-	gpio_set(EC_GPIO_DEBUG_LED, 0);
+	gpio_set_level(EC_GPIO_DEBUG_LED, 0);
 
 	uart_printf("Going to sleep : level %d clock %d...\n", level, clock);
 	uart_flush_output();
diff --git a/chip/lm4/flash.c b/chip/lm4/flash.c
index 10a07343d2..43f5d5923b 100644
--- a/chip/lm4/flash.c
+++ b/chip/lm4/flash.c
@@ -50,9 +50,9 @@ int flash_read(int offset, int size, char *data)
 		return EC_ERROR_UNKNOWN;  /* Invalid range */
 
 	/* Just read the flash from its memory window. */
-	/* TODO: is this affected by data cache?  That is, if we read a
-	 * block, then alter it, then read it again, do we get the old
-	 * data? */
+	/* TODO: (crosbug.com/p/7473) is this affected by data cache?
+	 * That is, if we read a block, then alter it, then read it
+	 * again, do we get the old data? */
 	memcpy(data, (char *)offset, size);
 	return EC_SUCCESS;
 }
@@ -60,7 +60,7 @@ int flash_read(int offset, int size, char *data)
 
 /* Performs a write-buffer operation.  Buffer (FWB) and address (FMA)
  * must be pre-loaded. */
-static int WriteBuffer(void)
+static int write_buffer(void)
 {
 	if (!LM4_FLASH_FWBVAL)
 		return EC_SUCCESS;  /* Nothing to do */
@@ -94,8 +94,8 @@ int flash_write(int offset, int size, const char *data)
 	    (offset | size) & (FLASH_WRITE_BYTES - 1))
 		return EC_ERROR_UNKNOWN;  /* Invalid range */
 
-	/* TODO - safety check - don't allow writing to the image we're
-	 * running from */
+	/* TODO (crosbug.com/p/7478) - safety check - don't allow writing to
+	 * the image we're running from */
 
 	/* Get initial page and write buffer index */
 	LM4_FLASH_FMA = offset & ~(FLASH_FWB_BYTES - 1);
@@ -105,7 +105,7 @@ int flash_write(int offset, int size, const char *data)
 	for ( ; size > 0; size -= 4) {
 		LM4_FLASH_FWB[i++] = *data32++;
 		if (i == FLASH_FWB_WORDS) {
-			rv = WriteBuffer();
+			rv = write_buffer();
 			if (rv != EC_SUCCESS)
 				return rv;
 
@@ -117,7 +117,7 @@ int flash_write(int offset, int size, const char *data)
 
 	/* Handle final partial page, if any */
 	if (i > 0) {
-		rv = WriteBuffer();
+		rv = write_buffer();
 		if (rv != EC_SUCCESS)
 			return rv;
 	}
@@ -132,8 +132,8 @@ int flash_erase(int offset, int size)
 	    (offset | size) & (FLASH_ERASE_BYTES - 1))
 		return EC_ERROR_UNKNOWN;  /* Invalid range */
 
-	/* TODO - safety check - don't allow erasing the image we're running
-	 * from */
+	/* TODO (crosbug.com/p/7478) - safety check - don't allow erasing the
+	 * image we're running from */
 
 	LM4_FLASH_FCMISC = LM4_FLASH_FCRIS;  /* Clear previous error status */
 	LM4_FLASH_FMA = offset;
@@ -295,7 +295,10 @@ int flash_init(void)
 	 * returns one less than the number of protection pages. */
 	usable_flash_size = LM4_FLASH_FSIZE * FLASH_PROTECT_BYTES;
 
-	/* TODO - check WP# GPIO.  If it's set and the flash protect range
-	 * is set, write the flash protection registers. */
+	/* TODO (crosbug.com/p/7453) - check WP# GPIO.  If it's set and the
+	 * flash protect range is set, write the flash protection registers.
+	 * Probably cleaner to do this in vboot, since we're going to need to
+	 * use the same last block of flash to hold the firmware rollback
+	 * counters. */
 	return EC_SUCCESS;
 }
diff --git a/chip/lm4/gpio.c b/chip/lm4/gpio.c
index b39fbaac48..9a6e8429bf 100644
--- a/chip/lm4/gpio.c
+++ b/chip/lm4/gpio.c
@@ -154,7 +154,7 @@ int gpio_pre_init(void)
 	LM4_SYSTEM_RCGCGPIO |= 0x0001;
 
 	/* Turn off the LED before we make it an output */
-	gpio_set(EC_GPIO_DEBUG_LED, 0);
+	gpio_set_level(EC_GPIO_DEBUG_LED, 0);
 
 	/* Clear GPIOAFSEL bits for block A pin 7 */
 	LM4_GPIO_AFSEL(LM4_GPIO_A) &= ~(0x80);
@@ -223,19 +223,18 @@ int gpio_init(void)
 }
 
 
-int gpio_get(enum gpio_signal signal, int *value_ptr)
+int gpio_get_level(enum gpio_signal signal)
 {
 	switch (signal) {
 	case EC_GPIO_DEBUG_LED:
-		*value_ptr = (LM4_GPIO_DATA(LM4_GPIO_A, 0x80) & 0x80 ? 1 : 0);
-		return EC_SUCCESS;
+		return LM4_GPIO_DATA(LM4_GPIO_A, 0x80) & 0x80 ? 1 : 0;
 	default:
-		return EC_ERROR_UNKNOWN;
+		return 0;
 	}
 }
 
 
-int gpio_set(enum gpio_signal signal, int value)
+int gpio_set_level(enum gpio_signal signal, int value)
 {
 	switch (signal) {
 	case EC_GPIO_DEBUG_LED:
diff --git a/chip/lm4/i2c.c b/chip/lm4/i2c.c
index 340ece84dc..096ef19255 100644
--- a/chip/lm4/i2c.c
+++ b/chip/lm4/i2c.c
@@ -3,7 +3,7 @@
  * found in the LICENSE file.
  */
 
-/* Temperature sensor module for Chrome EC */
+/* I2C port module for Chrome EC */
 
 #include "board.h"
 #include "console.h"
diff --git a/chip/lm4/keyboard_scan.c b/chip/lm4/keyboard_scan.c
index 889c535d24..f7a5872a16 100644
--- a/chip/lm4/keyboard_scan.c
+++ b/chip/lm4/keyboard_scan.c
@@ -63,7 +63,8 @@ static uint8_t raw_state[KB_COLS];
 static const uint8_t *actual_key_mask;
 
 /* All actual key masks (todo: move to keyboard matrix definition */
-/* TODO: fill in real key mask with 0-bits for coords that aren't keys */
+/* TODO: (crosbug.com/p/7485) fill in real key mask with 0-bits for coords that
+   aren't keys */
 static const uint8_t actual_key_masks[4][KB_COLS] = {
 	{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
 	 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
@@ -342,7 +343,7 @@ void keyboard_scan_task(void)
 				}
 			}
 		}
-		/* TODO: A race condition here.
+		/* TODO: (crosbug.com/p/7484) A race condition here.
 		 *       If a key state is changed here (before interrupt is
 		 *       enabled), it will be lost.
 		 */
diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c
index 9f14b8d4aa..521bd754ad 100644
--- a/chip/lm4/lpc.c
+++ b/chip/lm4/lpc.c
@@ -37,9 +37,9 @@ static void configure_gpio(void)
 	LM4_GPIO_DEN(LM4_GPIO_M) |= 0x37;
 
 	/* Set the drive strength to 8mA for serirq only */
-	/* TODO: Only necessary on BDS because the cabling to the x86
-	 * is long and flaky; remove this for Link.  Setting this for all
-	 * I/O lines seems to hang the x86 during boot. */
+	/* TODO: (crosbug.com/p/7495) Only necessary on BDS because the cabling
+	 * to the x86 is long and flaky; remove this for Link.  Setting this
+	 * for all I/O lines seems to hang the x86 during boot. */
 	LM4_GPIO_DR8R(LM4_GPIO_M) |= 0x00000010;
 }
 
@@ -143,7 +143,8 @@ void lpc_send_host_response(int slot, int status)
 	/* Write dummy value to data byte.  This sets the TOH bit in the
 	 * status byte and triggers an IRQ on the host so the host can read
 	 * the status. */
-	/* TODO: or it would, if we actually set up host IRQs */
+	/* TODO: (crosbug.com/p/7496) or it would, if we actually set up host
+	 * IRQs */
 	if (slot)
 		LPC_POOL_USER[1] = 0;
 	else
@@ -159,8 +160,9 @@ int lpc_comx_has_char(void)
 
 int lpc_comx_get_char(void)
 {
-	/* TODO: this clears the receive-ready interrupt too, which will be ok
-	 * once we're handing output to COMx as well.  But we're not yet. */
+	/* TODO: (crosbug.com/p/7488) this clears the receive-ready interrupt
+	 * too, which will be ok once we're handing output to COMx as well.
+	 * But we're not yet. */
 	LM4_LPC_LPCDMACX = LM4_LPC_LPCDMACX;
 	/* Copy the next byte */
 	return LPC_POOL_COMX[0];
@@ -229,9 +231,9 @@ static void lpc_interrupt(void)
 				uart_comx_putc(lpc_comx_get_char());
 		}
 
-		/* TODO: handle UART input to host - if host read the
-		 * to-host data, see if there's another byte still
-		 * waiting on UART1. */
+		/* TODO: (crosbug.com/p/7488) handle UART input to host - if
+		 * host read the to-host data, see if there's another byte
+		 * still waiting on UART1. */
 	}
 }
 
diff --git a/chip/lm4/system.c b/chip/lm4/system.c
index a6d74e32cd..c1ff285187 100644
--- a/chip/lm4/system.c
+++ b/chip/lm4/system.c
@@ -217,8 +217,6 @@ int system_run_image_copy(enum system_image_copy_t copy)
 	if (system_get_image_copy() != SYSTEM_IMAGE_RO)
 		return EC_ERROR_UNKNOWN;
 
-	/* TODO: fail if we've already started up interrupts, etc. */
-
 	/* Load the appropriate reset vector */
 	if (copy == SYSTEM_IMAGE_RW_A)
 		init_addr = *(uint32_t *)(CONFIG_FW_A_OFF + 4);
@@ -227,8 +225,16 @@ int system_run_image_copy(enum system_image_copy_t copy)
 	else
 		return EC_ERROR_UNKNOWN;
 
-	/* TODO: sanity check reset vector; must be inside the
-	 * appropriate image. */
+	/* TODO: sanity checks (crosbug.com/p/7468)
+	 *
+	 * Fail if called outside of pre-init.
+	 *
+	 * Fail if reboot reason is not soft reboot.  Power-on
+	 * reset cause must run RO firmware; if it wants to move to RW
+	 * firmware, it must go through a soft reboot first
+	 *
+	 * Sanity check reset vector; must be inside the appropriate
+	 * image. */
 
 	/* Jump to the reset vector */
 	resetvec = (void(*)(void))init_addr;
@@ -241,11 +247,13 @@ int system_run_image_copy(enum system_image_copy_t copy)
 
 int system_reset(int is_cold)
 {
-	/* TODO - warm vs. cold */
+	/* TODO: (crosbug.com/p/7470) support cold boot; this is a
+	   warm boot. */
 	LM4_NVIC_APINT = 0x05fa0004;
 
 	/* Spin and wait for reboot; should never return */
-	/* TODO: should disable task swaps while waiting */
+	/* TODO: (crosbug.com/p/7471) should disable task swaps while
+	   waiting */
 	while (1) {}
 
 	return EC_ERROR_UNKNOWN;
@@ -262,8 +270,10 @@ int system_set_scratchpad(uint32_t value)
 		return rv;
 
 	/* Write scratchpad */
-	/* TODO: might be more elegant to have a write_hibernate_reg() method
-	 * which takes an address and data and does the delays */
+	/* TODO: (crosbug.com/p/7472) might be more elegant to have a
+	 * write_hibernate_reg() method which takes an address and
+	 * data and does the delays.  Then we could move the hibernate
+	 * register accesses to a separate module. */
 	LM4_HIBERNATE_HIBDATA = value;
 
 	/* Wait for write-complete */
@@ -302,9 +312,10 @@ const char *system_get_version(enum system_image_copy_t copy)
 	}
 
 	/* Search for version cookies in target image */
-	/* TODO: could be smarter about where to search if we stuffed
-	 * the version data into a predefined area of the image - for
-	 * example, immediately following the reset vectors. */
+	/* TODO: (crosbug.com/p/7469) could be smarter about where to
+	 * search if we stuffed the version data into a predefined
+	 * area of the image - for example, immediately following the
+	 * reset vectors. */
 	pend = (uint32_t *)(imoffset + CONFIG_FW_IMAGE_SIZE
 			    - sizeof(version_data));
 	for (p = (uint32_t *)imoffset; p <= pend; p++) {
diff --git a/chip/lm4/uart.c b/chip/lm4/uart.c
index d9f86e9800..26ac058e1d 100644
--- a/chip/lm4/uart.c
+++ b/chip/lm4/uart.c
@@ -137,7 +137,7 @@ static void uart_1_interrupt(void)
 	/* Clear transmit and receive interrupt status */
 	LM4_UART_ICR(1) = 0x70;
 
-	/* TODO: handle input */
+	/* TODO: (crosbug.com/p/7488) handle input */
 
 	/* If we have space in our FIFO and a character is pending in LPC,
 	 * handle that character. */
@@ -330,7 +330,8 @@ int uart_printf(const char *format, ...)
 			int is_negative = 0;
 			int base = 10;
 
-			/* TODO: handle "%l" prefix for uint64_t */
+			/* TODO: (crosbug.com/p/7490) handle "%l" prefix for
+			 * uint64_t */
 
 			v = va_arg(args, uint32_t);
 
diff --git a/common/main.c b/common/main.c
index 5ea2607b14..47f3e0f0e9 100644
--- a/common/main.c
+++ b/common/main.c
@@ -36,10 +36,10 @@
 void UserLedBlink(void)
 {
 	while (1) {
-		gpio_set(EC_GPIO_DEBUG_LED, 1);
+		gpio_set_level(EC_GPIO_DEBUG_LED, 1);
 		usleep(500000);
 		watchdog_reload();
-		gpio_set(EC_GPIO_DEBUG_LED, 0);
+		gpio_set_level(EC_GPIO_DEBUG_LED, 0);
 		usleep(500000);
 		watchdog_reload();
 	}
@@ -61,8 +61,9 @@ int main(void)
 	gpio_pre_init();
 	vboot_pre_init();
 
-	/* TODO - race condition on enabling interrupts.  Module inits
-	 * should call task_IntEnable(int) when they're ready... */
+	/* TODO (crosbug.com/p/7456)- race condition on enabling
+	 * interrupts.  Module inits should call task_IntEnable(int)
+	 * when they're ready... */
 	task_init();
 
 	watchdog_init(1100);
diff --git a/include/gpio.h b/include/gpio.h
index 05123c4eb6..5e4edf0593 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -32,10 +32,10 @@ int gpio_init(void);
  * supported / not present on the board. */
 
 /* Gets the current value of a signal (0=low, 1=hi). */
-int gpio_get(enum gpio_signal signal, int *value_ptr);
+int gpio_get_level(enum gpio_signal signal);
 
 /* Sets the current value of a signal.  Returns error if the signal is
  * not supported or is an input signal. */
-int gpio_set(enum gpio_signal signal, int value);
+int gpio_set_level(enum gpio_signal signal, int value);
 
 #endif  /* __CROS_EC_GPIO_H */