From c1e02ca2203a7889539a88570e142f16cfd265a8 Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 1 Nov 2013 11:39:00 -0700 Subject: cleanup: Yes, even more TODO comments Almost done. Comment changes only. BUG=chrome-os-partner:18343 BRANCH=none TEST=build all platforms Change-Id: I974dfc12aa264e2035b3bae35a089c19344e7d45 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/175484 Reviewed-by: Bill Richardson --- board/samus/board.c | 8 +++++--- common/build.mk | 11 ++++++----- power/gaia.c | 13 +++++++++---- power/tegra.c | 10 ++++++++-- util/comm-dev.c | 10 +++++----- util/comm-i2c.c | 3 ++- util/comm-lpc.c | 9 ++++----- util/ectool.c | 29 +++++++++++++++++++++-------- util/ectool_keyscan.c | 2 +- 9 files changed, 61 insertions(+), 34 deletions(-) diff --git a/board/samus/board.c b/board/samus/board.c index 7e3aa5c51e..db8639acb9 100644 --- a/board/samus/board.c +++ b/board/samus/board.c @@ -179,9 +179,11 @@ const struct adc_t adc_channels[] = { {"ChargerCurrent", LM4_ADC_SEQ1, 33000, ADC_READ_MAX * 4, 0, LM4_AIN(11), 0x06 /* IE0 | END0 */, LM4_GPIO_B, (1<<5)}, - /* TODO: We don't know what to expect here, but it's an analog input - * that's pulled high. We're only using it as a battery presence - * indicator for now, so we'll just return 0 - ADC_READ_MAX. + /* + * TODO(crosbug.com/p/23827): We don't know what to expect here, but + * it's an analog input that's pulled high. We're using it as a battery + * presence indicator for now. We'll return just 0 - ADC_READ_MAX for + * now. */ {"BatteryTemp", LM4_ADC_SEQ2, 1, 1, 0, LM4_AIN(10), 0x06 /* IE0 | END0 */, LM4_GPIO_B, (1<<4)}, diff --git a/common/build.mk b/common/build.mk index 540626e0d7..132fd62783 100644 --- a/common/build.mk +++ b/common/build.mk @@ -11,14 +11,15 @@ common-y+=memory_commands.o shared_mem.o system.o hooks.o common-y+=gpio.o version.o printf.o queue.o common-$(CONFIG_BACKLIGHT_LID)+=backlight_lid.o -# TODO(rspangler): Why do these include battery_common but the other batteries -# don't? Perhaps should use CONFIG_CMD_BATTERY instead, since all that's in -# battery.c is the battery console command? +# TODO(crosbug.com/p/23821): Why do these include battery_common but +# the other batteries don't? Perhaps should use CONFIG_CMD_BATTERY +# instead, since all that's in battery.c is the battery console +# command? common-$(CONFIG_BATTERY_BQ27541)+=battery.o common-$(CONFIG_BATTERY_SMART)+=battery.o common-$(CONFIG_CHARGER)+=charge_state.o charger.o -# TODO(rspangler): This is really the charge state machine for ARM, not the -# charger driver for the tps65090. Rename. +# TODO(crosbug.com/p/23815): This is really the charge state machine +# for ARM, not the charger driver for the tps65090. Rename. common-$(CONFIG_CHARGER_TPS65090)+=pmu_tps65090_charger.o common-$(CONFIG_PMU_POWERINFO)+=pmu_tps65090_powerinfo.o common-$(CONFIG_PMU_TPS65090)+=pmu_tps65090.o diff --git a/power/gaia.c b/power/gaia.c index fd5d5fb7a6..c6077c921f 100644 --- a/power/gaia.c +++ b/power/gaia.c @@ -369,12 +369,18 @@ int chipset_in_state(int state_mask) void chipset_exit_hard_off(void) { - /* TODO: implement, if/when we take the AP down to a hard-off state */ + /* + * TODO(crosbug.com/p/23822): Implement, if/when we take the AP down to + * a hard-off state. + */ } void chipset_reset(int is_cold) { - /* TODO: implement cold reset. For now, all resets are warm resets. */ + /* + * TODO(crosbug.com/p/23822): Implement cold reset. For now, all + * resets are warm resets. + */ CPRINTF("[%T EC triggered warm reboot]\n"); /* @@ -438,8 +444,7 @@ void chipset_force_shutdown(void) */ static int check_for_power_on_event(void) { - /* the system is already ON */ - /* TODO: this isn't the right check for pit */ + /* Check if we've already powered the system on */ if (gpio_get_level(GPIO_EN_PP3300)) { CPRINTF("[%T system is on, thus clear auto_power_on]\n"); auto_power_on = 0; /* no need to arrange another power on */ diff --git a/power/tegra.c b/power/tegra.c index 3484a266ae..18b39ea7fe 100644 --- a/power/tegra.c +++ b/power/tegra.c @@ -309,12 +309,18 @@ int chipset_in_state(int state_mask) void chipset_exit_hard_off(void) { - /* TODO: implement, if/when we take the AP down to a hard-off state */ + /* + * TODO(crosbug.com/p/23822): Implement, if/when we take the AP down to + * a hard-off state. + */ } void chipset_reset(int is_cold) { - /* TODO: implement cold reset. For now, all resets are warm resets. */ + /* + * TODO(crosbug.com/p/23822): Implement cold reset. For now, all + * resets are warm resets. + */ CPRINTF("[%T EC triggered warm reboot]\n"); /* diff --git a/util/comm-dev.c b/util/comm-dev.c index 1175f92555..eb3a4f9b47 100644 --- a/util/comm-dev.c +++ b/util/comm-dev.c @@ -108,11 +108,11 @@ int comm_init_dev(void) ec_readmem = ec_readmem_dev; /* - * TODO: need a way to get this from the driver and EC. For now, - * pick a magic lowest common denominator value. The ec_max_outsize - * is set to handle v3 EC protocol. The ec_max_insize needs to be - * set to the largest value that can be returned from the EC, - * EC_PROTO2_MAX_PARAM_SIZE. + * TODO(crosbug.com/p/23823): Need a way to get this from the driver + * and EC. For now, pick a magic lowest common denominator value. The + * ec_max_outsize is set to handle v3 EC protocol. The ec_max_insize + * needs to be set to the largest value that can be returned from the + * EC, EC_PROTO2_MAX_PARAM_SIZE. */ ec_max_outsize = EC_PROTO2_MAX_PARAM_SIZE - 8; ec_max_insize = EC_PROTO2_MAX_PARAM_SIZE; diff --git a/util/comm-i2c.c b/util/comm-i2c.c index 91ceb555e2..b305823198 100644 --- a/util/comm-i2c.c +++ b/util/comm-i2c.c @@ -126,7 +126,8 @@ static int ec_command_i2c(int command, int version, /* check response error code */ ret = resp_buf[0]; - /* TODO: handle EC_RES_IN_PROGRESS case. */ + + /* TODO(crosbug.com/p/23824): handle EC_RES_IN_PROGRESS case. */ resp_len = resp_buf[1]; if (resp_len > insize) { diff --git a/util/comm-lpc.c b/util/comm-lpc.c index 8ea4e9ddd8..25f0bea4c6 100644 --- a/util/comm-lpc.c +++ b/util/comm-lpc.c @@ -26,11 +26,10 @@ static int wait_for_ec(int status_addr, int timeout_usec) for (i = 0; i < timeout_usec; i += delay) { /* * Delay first, in case we just sent out a command but the EC - * hasn't raise the busy flag. However, I think this doesn't + * hasn't raised the busy flag. However, I think this doesn't * happen since the LPC commands are executed in order and the - * busy flag is set by hardware. - * - * TODO: move this delay after inb(status). + * busy flag is set by hardware. Minor issue in any case, + * since the initial delay is very short. */ usleep(MIN(delay, timeout_usec - i)); @@ -142,7 +141,7 @@ static int ec_command_lpc_3(int command, int version, return -EC_RES_REQUEST_TRUNCATED; /* Fill in request packet */ - /* TODO: this should be common to all protocols */ + /* TODO(crosbug.com/p/23825): This should be common to all protocols */ rq.struct_version = EC_HOST_REQUEST_VERSION; rq.checksum = 0; rq.command = command; diff --git a/util/ectool.c b/util/ectool.c index 382dd556a6..23244e54fb 100644 --- a/util/ectool.c +++ b/util/ectool.c @@ -496,7 +496,10 @@ int cmd_reboot_ec(int argc, char *argv[]) else if (!strcmp(argv[1], "RO")) p.cmd = EC_REBOOT_JUMP_RO; else if (!strcmp(argv[1], "RW") || !strcmp(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". + */ p.cmd = EC_REBOOT_JUMP_RW; } else if (!strcmp(argv[1], "cold")) p.cmd = EC_REBOOT_COLD; @@ -1080,7 +1083,11 @@ int cmd_thermal_set_threshold_v1(int argc, char *argv[]) return rv; } - +/** + * Detect the version of EC_CMD_THERMAL_GET_THRESHOLD that the EC supports. + * + * @return The version, or -1 if error. + */ static int thermal_threshold_version(void) { struct ec_params_thermal_get_threshold v0_p; @@ -1092,8 +1099,11 @@ static int thermal_threshold_version(void) v1_p.sensor_num = 0; rv = ec_command(EC_CMD_THERMAL_GET_THRESHOLD, 1, &v1_p, sizeof(v1_p), &v1_r, sizeof(v1_r)); - /* FIXME: Verson 1 will only return these responses */ - /* FIXME: if (??? == EC_RES_SUCCESS || ??? == EC_RES_INVALID_PARAM) */ + + /* + * TODO(crosbug.com/p/23828): Version 1 of the threshold command will + * only return EC_RES_SUCCESS or EC_RES_INVALID_PARAM? + */ if (rv > 0) return 1; @@ -1101,13 +1111,16 @@ static int thermal_threshold_version(void) v0_p.threshold_id = 0; rv = ec_command(EC_CMD_THERMAL_GET_THRESHOLD, 0, &v0_p, sizeof(v0_p), &v0_r, sizeof(v0_r)); - /* FIXME: Verson 0 will only return these responses */ - /* FIXME: if (??? == EC_RES_SUCCESS || ??? == EC_RES_ERROR) */ + /* + * TODO(crosbug.com/p/23828): Version 0 of the threshold command will + * only return EC_RES_SUCCESS or EC_RES_ERROR? + */ if (rv > 0) return 0; - /* Anything else is most likely EC_RES_INVALID_COMMAND, - * but we don't care because it's nothing we can use. + /* + * Anything else is most likely EC_RES_INVALID_COMMAND, but we don't + * care because it's nothing we can use. */ return -1; } diff --git a/util/ectool_keyscan.c b/util/ectool_keyscan.c index 868fbd11ab..6029e0b09a 100644 --- a/util/ectool_keyscan.c +++ b/util/ectool_keyscan.c @@ -663,7 +663,7 @@ int cmd_keyscan(int argc, char *argv[]) return -1; } - /* TODO(sjg@chromium.org): Read key matrix from fdt */ + /* TODO(crosbug.com/p/23826): Read key matrix from fdt */ err = keyscan_read_fdt_matrix(&keyscan, "test/test-matrix.bin"); if (!err) err = keyscan_process_file(f, &keyscan); -- cgit v1.2.1