From 00682eb80fe159c1d037c721f2dd15c1be44bd12 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 1 Nov 2013 10:15:48 -0700 Subject: cleanup: Still more TODO comments More of same. Comment changes only; no code changes. BUG=chrome-os-partner:18343 BRANCH=none TEST=build all platforms; pass unit tests Change-Id: I8c42ed7d332cd9d461067e1aeac670855106cbcd Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/175405 Reviewed-by: Bill Richardson --- chip/host/uart.c | 13 ++++++++----- chip/stm32/config-stm32l100.h | 12 ++++++------ chip/stm32/config-stm32l15x.h | 12 ++++++------ common/extpower_snow.c | 2 +- common/host_command.c | 19 +++++++++++++------ common/pmu_tps65090_charger.c | 9 ++++----- common/power_button_x86.c | 18 +++++++++++++++--- driver/battery/smart.c | 4 ++-- include/extpower_spring.h | 8 ++++---- 9 files changed, 59 insertions(+), 38 deletions(-) diff --git a/chip/host/uart.c b/chip/host/uart.c index 770ea8c0ef..2a05fb686e 100644 --- a/chip/host/uart.c +++ b/chip/host/uart.c @@ -23,7 +23,10 @@ static int init_done; static pthread_t input_thread; #define INPUT_BUFFER_SIZE 16 -/* TODO: Guard these data with mutex lock when we have interrupt support. */ +/* + * TODO(crosbug.com/p/23804): Guard these data with mutex lock when we have + * interrupt support. + */ static int char_available; static char cached_char_buf[INPUT_BUFFER_SIZE]; static struct queue cached_char = { @@ -66,8 +69,8 @@ const char *test_get_captured_console(void) static void trigger_interrupt(void) { /* - * TODO: Check global interrupt status when we have - * interrupt support. + * TODO(crosbug.com/p/23804): Check global interrupt status when we + * have interrupt support. */ if (!int_disabled) { uart_process_input(); @@ -174,8 +177,8 @@ void *uart_monitor_stdin(void *d) } tcsetattr(0, TCSANOW, &org_settings); /* - * TODO: Trigger emulated interrupt when we have - * interrupt support. Also, we will need a condition + * TODO(crosbug.com/p/23804): Trigger emulated interrupt when + * we have interrupt support. Also, we will need a condition * variable to indicate the character has been read. */ trigger_interrupt(); diff --git a/chip/stm32/config-stm32l100.h b/chip/stm32/config-stm32l100.h index e22518263a..2198f6dc0a 100644 --- a/chip/stm32/config-stm32l100.h +++ b/chip/stm32/config-stm32l100.h @@ -11,12 +11,12 @@ #define CONFIG_FLASH_ERASE_SIZE 0x0100 /* erase bank size */ /* - * TODO(rspangler): Technically we can write in word-mode (4 bytes at a time), - * but that's really slow, and older host interfaces which can't ask about the - * ideal size would then end up writing in that mode instead of the faster page - * mode. So lie about the write size for now. Once all software (flashrom, - * u-boot, ectool) which cares has been updated to know about ver.1 of - * EC_CMD_GET_FLASH_INFO, we can remove this workaround. + * TODO(crosbug.com/p/23805): Technically we can write in word-mode (4 bytes at + * a time), but that's really slow, and older host interfaces which can't ask + * about the ideal size would then end up writing in that mode instead of the + * faster page mode. So lie about the write size for now. Once all software + * (flashrom, u-boot, ectool) which cares has been updated to know about ver.1 + * of EC_CMD_GET_FLASH_INFO, we can remove this workaround. */ #define CONFIG_FLASH_WRITE_SIZE 0x0080 diff --git a/chip/stm32/config-stm32l15x.h b/chip/stm32/config-stm32l15x.h index eef90576d4..7d51ca1bda 100644 --- a/chip/stm32/config-stm32l15x.h +++ b/chip/stm32/config-stm32l15x.h @@ -11,12 +11,12 @@ #define CONFIG_FLASH_ERASE_SIZE 0x0100 /* erase bank size */ /* - * TODO(rspangler): Technically we can write in word-mode (4 bytes at a time), - * but that's really slow, and older host interfaces which can't ask about the - * ideal size would then end up writing in that mode instead of the faster page - * mode. So lie about the write size for now. Once all software (flashrom, - * u-boot, ectool) which cares has been updated to know about ver.1 of - * EC_CMD_GET_FLASH_INFO, we can remove this workaround. + * TODO(crosbug.com/p/23805): Technically we can write in word-mode (4 bytes at + * a time), but that's really slow, and older host interfaces which can't ask + * about the ideal size would then end up writing in that mode instead of the + * faster page mode. So lie about the write size for now. Once all software + * (flashrom, u-boot, ectool) which cares has been updated to know about ver.1 + * of EC_CMD_GET_FLASH_INFO, we can remove this workaround. */ #define CONFIG_FLASH_WRITE_SIZE 0x0080 diff --git a/common/extpower_snow.c b/common/extpower_snow.c index 12e44696d8..1153db0058 100644 --- a/common/extpower_snow.c +++ b/common/extpower_snow.c @@ -43,4 +43,4 @@ int extpower_is_present(void) return ac_good; } -/* TODO: host events and hook notifications */ +/* TODO(crosbug.com/p/23810): host events and hook notifications */ diff --git a/common/host_command.c b/common/host_command.c index a0d703b90a..b5655d8593 100644 --- a/common/host_command.c +++ b/common/host_command.c @@ -59,6 +59,15 @@ static uint8_t command_pending; static uint8_t saved_result = EC_RES_UNAVAILABLE; #endif +/* + * Host command args passed to command handler. Static to keep it off the + * stack. Note this means we can handle only one host command at a time. + */ +static struct host_cmd_handler_args args0; + +/* Current host command packet from host, for protocol version 3+ */ +static struct host_packet *pkt0; + uint8_t *host_get_memmap(int offset) { #ifdef CONFIG_LPC @@ -120,7 +129,10 @@ test_mockable void host_send_response(struct host_cmd_handler_args *args) void host_command_received(struct host_cmd_handler_args *args) { - /* TODO: should warn if we already think we're in a command */ + /* + * TODO(crosbug.com/p/23806): should warn if we already think we're in + * a command. + */ /* * If this is the reboot command, reboot immediately. This gives the @@ -152,10 +164,6 @@ void host_command_received(struct host_cmd_handler_args *args) host_send_response(args); } -/* TODO(rspangler): less awful names. */ -static struct host_cmd_handler_args args0; -static struct host_packet *pkt0; - void host_packet_respond(struct host_cmd_handler_args *args) { struct ec_host_response *r = (struct ec_host_response *)pkt0->response; @@ -203,7 +211,6 @@ int host_request_expected_size(const struct ec_host_request *r) return 0; /* Reserved byte should be 0 */ - /* TODO: maybe we should have a header checksum instead? */ if (r->reserved) return 0; diff --git a/common/pmu_tps65090_charger.c b/common/pmu_tps65090_charger.c index 6afb72d988..68bca0a396 100644 --- a/common/pmu_tps65090_charger.c +++ b/common/pmu_tps65090_charger.c @@ -80,8 +80,8 @@ static int battery_discharging_range(int deci_k) temp_c < bat_temp_ranges.discharging_max_c); } -/* - * Turn off the host application processor +/** + * Turn off the AP */ static int system_off(void) { @@ -93,7 +93,7 @@ static int system_off(void) return ST_IDLE0; } -/* +/** * Notify the host when battery remaining charge is lower than 10% */ static int notify_battery_low(void) @@ -106,7 +106,7 @@ static int notify_battery_low(void) if (now.val - last_notify_time.val > MINUTE) { CPUTS("[pmu] notify battery low (< 4%)\n"); last_notify_time = now; - /* TODO(rongchang): notify AP ? */ + /* TODO(crosbug.com/p/23814): Actually notify AP */ } } return ST_DISCHARGING; @@ -347,7 +347,6 @@ static int calc_next_state(int state) return ST_IDLE0; } -/* TODO: Merge charge_state.h and unify charge interface */ enum charging_state charge_get_state(void) { return current_state; diff --git a/common/power_button_x86.c b/common/power_button_x86.c index e37fb6f81f..c8854e99fe 100644 --- a/common/power_button_x86.c +++ b/common/power_button_x86.c @@ -26,8 +26,21 @@ #define CPRINTF(format, args...) cprintf(CC_SWITCH, format, ## args) /* - * When chipset is on, we stretch the power button signal to it so chipset - * hard-reset is triggered at ~8 sec, not ~4 sec: + * x86 chipsets have a hardware timer on the power button input which causes + * them to reset when the button is pressed for more than 4 seconds. This is + * problematic for Chrome OS, which needs more time than that to transition + * through the lock and logout screens. So when the system is on, we need to + * stretch the power button signal so that the chipset will hard-reboot after 8 + * seconds instead of 4. + * + * When the button is pressed, we initially send a short pulse (t0); this + * allows the chipset to process its initial power button interrupt and do + * things like wake from suspend. We then deassert the power button signal to + * the chipset for (t1 = 4 sec - t0), which keeps the chipset from starting its + * hard reset timer. If the power button is still pressed after this period, + * we again assert the power button signal for the remainder of the press + * duration. Since (t0+t1) causes a 4-second offset, the hard reset timeout in + * the chipset triggers after 8 seconds as desired. * * PWRBTN# --- ---- * to EC |______________________| @@ -41,7 +54,6 @@ * to host v v * @S0 make code break code */ -/* TODO: link to full power button / lid switch state machine description. */ #define PWRBTN_DELAY_T0 (32 * MSEC) /* 32ms (PCH requires >16ms) */ #define PWRBTN_DELAY_T1 (4 * SECOND - PWRBTN_DELAY_T0) /* 4 secs - t0 */ /* diff --git a/driver/battery/smart.c b/driver/battery/smart.c index 29af6286ad..87105dfab2 100644 --- a/driver/battery/smart.c +++ b/driver/battery/smart.c @@ -124,8 +124,8 @@ int battery_charging_allowed(int *allowed) int v, c, rv; /* - * TODO(rspangler): This re-reads the battery current and voltage, - * which is silly because charge_state.c just read them. + * TODO(crosbug.com/p/23811): This re-reads the battery current and + * voltage, which is silly because charge_state.c just read them. */ rv = battery_desired_voltage(&v) | battery_desired_current(&c); if (rv) diff --git a/include/extpower_spring.h b/include/extpower_spring.h index a4716d4532..db38f2c230 100644 --- a/include/extpower_spring.h +++ b/include/extpower_spring.h @@ -11,10 +11,10 @@ #include "common.h" /* - * TODO: this currently piggy-backs on the charger task. Should be able to - * move updates to deferred functions and get rid of all the ifdef's in the - * charger task. At that point, all these APIs will be internal to the - * extpower module and this entire header file can go away. + * TODO(crosbug.com/p/23813): this currently piggy-backs on the charger task. + * Should be able to move updates to deferred functions and get rid of all the + * ifdef's in the charger task. At that point, all these APIs will be internal + * to the extpower module and this entire header file can go away. */ /** -- cgit v1.2.1