From 3266ae6a7e7539f7e124c15197118fc0d9e68489 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 1 Nov 2013 08:49:17 -0700 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/175418 Reviewed-by: Bill Richardson --- board/kirby/board.c | 5 +++-- board/rambi/ec.tasklist | 1 - board/snow/board.c | 8 +++----- board/spring/board.c | 8 +++----- chip/lm4/clock.c | 16 +++++++++------- chip/lm4/config_chip.h | 2 +- chip/stm32/flash-stm32f.c | 10 ++++++---- chip/stm32/i2c-stm32f.c | 4 ++++ chip/stm32/i2c-stm32l.c | 4 ++++ common/switch.c | 5 +++-- power/build.mk | 1 - test/stress.c | 2 -- test/timer_dos.py | 4 ++-- 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)) -- cgit v1.2.1