From d7004207e5d809ff6bc4b6a0c6969fd3b277b886 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Thu, 31 Oct 2013 15:27:19 -0700 Subject: cleanup: Update more TODO comments Add bug links, reword, or remove as applicable. No code changes, just comments. BUG=chrome-os-partner:18343 BRANCH=none TEST=build all boards; pass unit tests Change-Id: Id55dd530c10091d7ab9d0f942f750168fca793b4 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/175326 Reviewed-by: Bill Richardson --- chip/lm4/lpc.c | 34 +++++++++++++++++++++------------- chip/stm32/gpio-stm32f.c | 11 ++++++++--- chip/stm32/gpio-stm32l.c | 4 +++- chip/stm32/spi.c | 13 +++++++------ common/system.c | 5 ++++- power/x86_common.c | 14 +++++++------- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c index 8a99247c74..7bfcc35dd0 100644 --- a/chip/lm4/lpc.c +++ b/chip/lm4/lpc.c @@ -78,12 +78,11 @@ static struct ec_lpc_host_args * const lpc_host_args = static void wait_irq_sent(void) { /* - * TODO(yjlou): udelay() is not graceful. Since the SIRQRIS is almost - * not cleared in continuous mode and EC has problem to file more than - * 1 frame in the quiet mode, this is the best way we can do right now. - * - * 4 us is the time of 2 SERIRQ frames, which is long enough to - * guarantee the IRQ has been sent out. + * A hard-coded delay here isn't very elegant, but it's the best we can + * manage (and it's a short delay, so it's not that horrible). We need + * this because SIRQRIS isn't cleared in continuous mode, and the EC + * has trouble sending more than 1 frame in quiet mode. Waiting 4 us = + * 2 SERIRQ frames ensures the IRQ has been sent out. */ udelay(4); } @@ -284,7 +283,12 @@ int lpc_comx_get_char(void) void lpc_comx_put_char(int c) { LPC_POOL_COMX[1] = c; - /* TODO: manually trigger IRQ, like we do for keyboard? */ + + /* + * We could in theory manually trigger an IRQ, like we do for the 8042 + * keyboard interface, but neither the kernel nor BIOS seems to require + * this. + */ } #endif /* CONFIG_UART_HOST */ @@ -401,10 +405,12 @@ static void handle_acpi_write(int is_cmd) #ifdef CONFIG_PWM_KBLIGHT case EC_ACPI_MEM_KEYBOARD_BACKLIGHT: /* - * TODO: not very satisfying that LPC knows directly - * about the keyboard backlight, but for now this is - * good enough and less code than defining a new - * console command interface just for ACPI read/write. + * TODO(crosbug.com/p/23774): not very satisfying that + * LPC knows directly about the keyboard backlight, but + * for now this is good enough and less code than + * defining a new API for ACPI commands. If we start + * adding more commands, or need to support LPC on more + * than just LM4, fix this. */ result = pwm_get_duty(PWM_CH_KBLIGHT); break; @@ -751,8 +757,10 @@ static void lpc_init(void) */ LM4_LPC_ADR(LPC_CH_COMX) = LPC_COMX_ADDR; /* - * TODO: could configure IRQSELs and set IRQEN2/CX, and then the host - * can enable IRQs on its own. + * In theory we could configure IRQSELs and set IRQEN2/CX, and then the + * host could enable IRQs on its own. So far that hasn't been + * necessary, and due to the issues with IRQs (see wait_irq_sent() + * above) it might not work anyway. */ LM4_LPC_CTL(LPC_CH_COMX) = 0x0004 | (LPC_POOL_OFFS_COMX << (5 - 1)); /* Enable COMx emulation for reads and writes. */ diff --git a/chip/stm32/gpio-stm32f.c b/chip/stm32/gpio-stm32f.c index f917d00a0d..eba75d48e0 100644 --- a/chip/stm32/gpio-stm32f.c +++ b/chip/stm32/gpio-stm32f.c @@ -67,7 +67,11 @@ void gpio_set_flags_by_mask(uint32_t port, uint32_t pmask, uint32_t flags) * output, or alternate function. */ if (flags & GPIO_OUTPUT) { - /* TODO: This assumes output max speed of 10MHz */ + /* + * This sets output max speed to 10MHz. That should be + * sufficient for most GPIO needs; the only thing that needs to + * go faster is SPI, which overrides the port speed on its own. + */ mask |= 0x11111111 & mode; if (flags & GPIO_OPEN_DRAIN) mask |= 0x44444444 & cnf; @@ -113,7 +117,7 @@ void gpio_set_flags_by_mask(uint32_t port, uint32_t pmask, uint32_t flags) void gpio_set_alternate_function(uint32_t port, uint32_t mask, int func) { - /* TODO(rspangler): implement me! */ + /* TODO(crosbug.com/p/21618): implement me! */ } void gpio_pre_init(void) @@ -128,7 +132,8 @@ void gpio_pre_init(void) /* * Enable all GPIOs clocks * - * TODO: more fine-grained enabling for power saving + * TODO(crosbug.com/p/23770): only enable the banks we need to, + * and support disabling some of them in low-power idle. */ STM32_RCC_APB2ENR |= 0x1fd; } diff --git a/chip/stm32/gpio-stm32l.c b/chip/stm32/gpio-stm32l.c index e5b07f23c3..727c4c1d99 100644 --- a/chip/stm32/gpio-stm32l.c +++ b/chip/stm32/gpio-stm32l.c @@ -96,7 +96,9 @@ void gpio_pre_init(void) } else { /* * Enable all GPIOs clocks - * TODO: more fine-grained enabling for power saving + * + * TODO(crosbug.com/p/23770): only enable the banks we need to, + * and support disabling some of them in low-power idle. */ STM32_RCC_AHBENR |= 0x3f; } diff --git a/chip/stm32/spi.c b/chip/stm32/spi.c index d11b696672..1314e0e0d9 100644 --- a/chip/stm32/spi.c +++ b/chip/stm32/spi.c @@ -36,11 +36,11 @@ static const struct dma_option dma_rx_option = { /* * Timeout to wait for SPI request packet * - * TODO(sjg@chromium.org): Support much slower SPI clocks. For 4096 we have a - * delay of 4ms. For the largest message (68 bytes) this is 130KhZ, assuming - * that the master starts sending bytes as soon as it drops NSS. In practice, - * this timeout seems adequately high for a 1MHz clock which is as slow as we - * would reasonably want it. + * This affects the slowest SPI clock we can support. A delay of 8192 us + * permits a 512-byte request at 500 KHz, assuming the master starts sending + * bytes as soon as it asserts chip select. That's as slow as we would + * practically want to run the SPI interface, since running it slower + * significantly impacts firmware update times. */ #define SPI_CMD_RX_TIMEOUT_US 8192 @@ -410,7 +410,8 @@ void spi_event(enum gpio_signal signal) /* * Protocol version 2 * - * TODO: remove once all systems upgraded to version 3 + * TODO(crosbug.com/p/20257): Remove once kernel supports + * version 3. */ args.version = in_msg[0] - EC_CMD_VERSION0; args.command = in_msg[1]; diff --git a/common/system.c b/common/system.c index dc965aea1e..db91d1a6b6 100644 --- a/common/system.c +++ b/common/system.c @@ -705,7 +705,10 @@ static int command_sysjump(int argc, char **argv) if (!strcasecmp(argv[1], "RO")) return system_run_image_copy(SYSTEM_IMAGE_RO); else if (!strcasecmp(argv[1], "RW") || !strcasecmp(argv[1], "A")) { - /* TODO: remove "A" once all scripts are updated to use "RW" */ + /* + * TODO(crosbug.com/p/11149): remove "A" once all scripts are + * updated to use "RW". + */ return system_run_image_copy(SYSTEM_IMAGE_RW); } else if (!strcasecmp(argv[1], "disable")) { system_disable_jump(); diff --git a/power/x86_common.c b/power/x86_common.c index add033906e..d0b1088845 100644 --- a/power/x86_common.c +++ b/power/x86_common.c @@ -105,10 +105,10 @@ int x86_wait_signals(uint32_t want) return EC_ERROR_TIMEOUT; } /* - * TODO: should really shrink the remaining timeout if we woke - * up but didn't have all the signals we wanted. Also need to - * handle aborts if we're no longer in the same state we were - * when we started waiting. + * TODO(crosbug.com/p/23772): should really shrink the + * remaining timeout if we woke up but didn't have all the + * signals we wanted. Also need to handle aborts if we're no + * longer in the same state we were when we started waiting. */ } return EC_SUCCESS; @@ -207,9 +207,9 @@ int chipset_in_state(int state_mask) int need_mask = 0; /* - * TODO: what to do about state transitions? If the caller wants - * HARD_OFF|SOFT_OFF and we're in G3S5, we could still return - * non-zero. + * TODO(crosbug.com/p/23773): what to do about state transitions? If + * the caller wants HARD_OFF|SOFT_OFF and we're in G3S5, we could still + * return non-zero. */ switch (state) { case X86_G3: -- cgit v1.2.1