Better help for console commands

Additional help messages and usage are gated by
CONFIG_CONSOLE_CMDHELP, so we can turn it on if there's space (adds
about 3KB to image size) and turn it off when there isn't.

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

BUG=none
TEST=manual

1) help
2) help list
3) help gpioset
4) gpioset -> wrong number of params
5) gpioset fred 0 -> param1 bad
6) gpioset cpu_prochot fred -> param2 bad

Change-Id: Ibe99f37212020f763ebe65a068e6aa83a809a370

19 files changed, 329 insertions, 211 deletions

diff --git a/common/charger_bq24725.c b/common/charger_bq24725.c
index 6693a1bebe..84dc8897a2 100644
--- a/common/charger_bq24725.c
+++ b/common/charger_bq24725.c
@@ -234,29 +234,30 @@ static int print_info(void)
 static int command_charger(int argc, char **argv)
 {
 	int d;
-	char *endptr;
+	char *e;
 
 	if (argc != 3)
 		return print_info();
 
 	if (strcasecmp(argv[1], "input") == 0) {
-		d = strtoi(argv[2], &endptr, 0);
-		if (*endptr)
-			return EC_ERROR_INVAL;
+		d = strtoi(argv[2], &e, 0);
+		if (*e)
+			return EC_ERROR_PARAM2;
 		return charger_set_input_current(d);
 	} else if (strcasecmp(argv[1], "current") == 0) {
-		d = strtoi(argv[2], &endptr, 0);
-		if (*endptr)
-			return EC_ERROR_INVAL;
+		d = strtoi(argv[2], &e, 0);
+		if (*e)
+			return EC_ERROR_PARAM2;
 		return charger_set_current(d);
 	} else if (strcasecmp(argv[1], "voltage") == 0) {
-		d = strtoi(argv[2], &endptr, 0);
-		if (*endptr) {
-			return EC_ERROR_INVAL;
-		}
+		d = strtoi(argv[2], &e, 0);
+		if (*e)
+			return EC_ERROR_PARAM2;
 		return charger_set_voltage(d);
 	} else
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 }
-DECLARE_CONSOLE_COMMAND(charger, command_charger);
-
+DECLARE_CONSOLE_COMMAND(charger, command_charger,
+			"[input | current | voltage] [newval]",
+			"Get or set charger param(s)",
+			NULL);
diff --git a/common/console.c b/common/console.c
index 16a17fade7..e1c1365f11 100644
--- a/common/console.c
+++ b/common/console.c
@@ -124,8 +124,8 @@ static int split_words(char *input, int max_argc, int *argc, char **argv)
 }
 
 
-/* Finds a command by name.  Returns the command structure, or NULL if
- * no match found. */
+/* Find a command by name.  Returns the command structure, or NULL if no match
+ * found. */
 static const struct console_command *find_command(char *name)
 {
 	const struct console_command *cmd, *match = NULL;
@@ -143,14 +143,14 @@ static const struct console_command *find_command(char *name)
 }
 
 
-/* Handles a line of input containing a single command.
- *
- * Modifies the input string during parsing. */
+/* Handle a line of input containing a single command.  Modifies the input
+ * string during parsing. */
 static int handle_command(char *input)
 {
 	const struct console_command *cmd;
 	char *argv[MAX_ARGS_PER_COMMAND];
 	int argc = 0;
+	int rv;
 
 	/* Split input into words.  Ignore words past our limit. */
 	split_words(input, MAX_ARGS_PER_COMMAND, &argc, argv);
@@ -160,11 +160,32 @@ static int handle_command(char *input)
 		return EC_SUCCESS;
 
 	cmd = find_command(argv[0]);
-	if (cmd)
-		return cmd->handler(argc, argv);
+	if (!cmd) {
+		ccprintf("Command '%s' not found or ambiguous.\n", argv[0]);
+		return EC_ERROR_UNKNOWN;
+	}
 
-	ccprintf("Command '%s' not found or ambiguous.\n", argv[0]);
-	return EC_ERROR_UNKNOWN;
+	rv = cmd->handler(argc, argv);
+	if (rv == EC_SUCCESS)
+		return rv;
+
+	/* Print more info for errors */
+	if (rv == EC_ERROR_INVAL)
+		ccputs("Command usage/param invalid.\n");
+	else if (rv == EC_ERROR_PARAM_COUNT)
+		ccputs("Wrong number of params.\n");
+	else if (rv >= EC_ERROR_PARAM1 && rv <= EC_ERROR_PARAM9)
+		ccprintf("Parameter %d invalid.\n", rv - EC_ERROR_PARAM1 + 1);
+	else if (rv != EC_SUCCESS) {
+		ccprintf("Command returned error %d\n", rv);
+		return rv;
+	}
+
+#ifdef CONFIG_CONSOLE_CMDHELP
+	if (cmd->argdesc)
+		ccprintf("Usage: %s %s\n", cmd->name, cmd->argdesc);
+#endif
+	return rv;
 }
 
 
@@ -184,20 +205,11 @@ static int console_init(void)
 /* handle a console command */
 static void console_process(void)
 {
-        int rv;
-
 	/* Process all the pending commands.  Need to do this all at once
 	 * since our interrupt may have been triggered multiple times. */
-	/* TODO: Go to sleep briefly between commands to give lower
-	 * priority tasks a chance to run? */
 	while (uart_peek('\n') >= 0) {
 		uart_gets(input_buf, sizeof(input_buf));
-
-		rv = handle_command(input_buf);
-		if (rv == EC_ERROR_INVAL)
-			ccputs("Command usage/param invalid.\n");
-		else if (rv != EC_SUCCESS)
-			ccprintf("Command returned error %d\n", rv);
+		handle_command(input_buf);
 		ccputs(PROMPT);
 	}
 }
@@ -216,7 +228,7 @@ void console_task(void)
 
 	while (1) {
 		console_process();
-		/* wait for the next command message */
+		/* Wait for the next command message */
 		task_wait_event(-1);
 	}
 }
@@ -233,8 +245,35 @@ static int command_help(int argc, char **argv)
 	const int rows = (ncmds + cols - 1) / cols;
 	int i, j;
 
-	ccputs("Known commands:\n");
+#ifdef CONFIG_CONSOLE_CMDHELP
+	if (argc == 2) {
+		const struct console_command *cmd;
+
+		if (!strcasecmp(argv[1], "list")) {
+			ccputs("Known commands:\n");
+			for (i = 0; i < ncmds; i++) {
+				ccprintf("  %-15s%s\n",
+					 __cmds[i].name, __cmds[i].shorthelp);
+				cflush();
+			}
+			ccputs("HELP CMD = help on CMD.\n");
+			return EC_SUCCESS;
+		}
+		cmd = find_command(argv[1]);
+		if (!cmd) {
+			ccprintf("Command '%s' not found or ambiguous.\n",
+				 argv[1]);
+			return EC_ERROR_UNKNOWN;
+		}
+		ccprintf("Usage: %s %s\n", cmd->name,
+			 (cmd->argdesc ? cmd->argdesc : ""));
+		if (cmd->shorthelp)
+			ccprintf("%s\n", cmd->shorthelp);
+		return EC_SUCCESS;
+	}
+#endif
 
+	ccputs("Known commands:\n");
 	for (i = 0; i < rows; i++) {
 		ccputs("  ");
 		for (j = 0; j < cols; j++) {
@@ -247,9 +286,17 @@ static int command_help(int argc, char **argv)
 		cflush();
 	}
 
+#ifdef CONFIG_CONSOLE_CMDHELP
+	ccputs("HELP LIST = more info; ");
+	ccputs("HELP CMD = help on CMD.\n");
+#endif
+
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(help, command_help);
+DECLARE_CONSOLE_COMMAND(help, command_help,
+			"[ list | <name> ]",
+			"Print command help",
+			NULL);
 
 
 /* Set active channels */
@@ -262,7 +309,7 @@ static int command_ch(int argc, char **argv)
 	if (argc == 2) {
 		int m = strtoi(argv[1], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 
 		/* No disabling the command output channel */
 		channel_mask = m | CC_MASK(CC_COMMAND);
@@ -283,4 +330,7 @@ static int command_ch(int argc, char **argv)
 	}
 	return EC_SUCCESS;
 };
-DECLARE_CONSOLE_COMMAND(chan, command_ch);
+DECLARE_CONSOLE_COMMAND(chan, command_ch,
+			"[mask]",
+			"Get or set console channel mask",
+			NULL);
diff --git a/common/eoption.c b/common/eoption.c
index 83a94647fc..58d0ce22e0 100644
--- a/common/eoption.c
+++ b/common/eoption.c
@@ -141,7 +141,7 @@ static int command_eoption_get(int argc, char **argv)
 	if (argc == 2) {
 		i = find_option_by_name(argv[1], bool_opts);
 		if (i == -1)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 		d = bool_opts + i;
 		ccprintf("  %d %s\n", eoption_get_bool(i), d->name);
 		return EC_SUCCESS;
@@ -157,7 +157,10 @@ static int command_eoption_get(int argc, char **argv)
 	}
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(optget, command_eoption_get);
+DECLARE_CONSOLE_COMMAND(optget, command_eoption_get,
+			"[name]",
+			"Print EC option(s)",
+			NULL);
 
 
 static int command_eoption_set(int argc, char **argv)
@@ -166,16 +169,19 @@ static int command_eoption_set(int argc, char **argv)
 	int v, i;
 
 	if (argc < 3)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	v = strtoi(argv[2], &e, 0);
 	if (*e)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM2;
 
 	i = find_option_by_name(argv[1], bool_opts);
 	if (i == -1)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	return eoption_set_bool(i, v);
 }
-DECLARE_CONSOLE_COMMAND(optset, command_eoption_set);
+DECLARE_CONSOLE_COMMAND(optset, command_eoption_set,
+			"name value",
+			"Set EC option",
+			NULL);
diff --git a/common/flash_commands.c b/common/flash_commands.c
index ade35b15c9..cfae2124a3 100644
--- a/common/flash_commands.c
+++ b/common/flash_commands.c
@@ -13,30 +13,32 @@
 #include "system.h"
 #include "util.h"
 
-/* Parse offset and size from command line argv[0] and argv[1].
+/* Parse offset and size from command line argv[shift] and argv[shift+1]
  *
- * Default values: If argc<1, leaves offset unchanged, returning error if
- * *offset<0.  If argc<2, leaves size unchanged, returning error if *size<0. */
-static int parse_offset_size(int argc, char **argv, int *offset, int *size)
+ * Default values: If argc<=shift, leaves offset unchanged, returning error if
+ * *offset<0.  If argc<shift+1, leaves size unchanged, returning error if
+ * *size<0. */
+static int parse_offset_size(int argc, char **argv, int shift,
+			     int *offset, int *size)
 {
 	char *e;
 	int i;
 
-	if (argc >= 1) {
-		i = (uint32_t)strtoi(argv[0], &e, 0);
+	if (argc > shift) {
+		i = (uint32_t)strtoi(argv[shift], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 		*offset = i;
 	} else if (*offset < 0)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
-	if (argc >= 2) {
-		i = (uint32_t)strtoi(argv[1], &e, 0);
+	if (argc > shift + 1) {
+		i = (uint32_t)strtoi(argv[shift + 1], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM2;
 		*size = i;
 	} else if (*size < 0)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	return EC_SUCCESS;
 }
@@ -82,7 +84,10 @@ static int command_flash_info(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(flashinfo, command_flash_info);
+DECLARE_CONSOLE_COMMAND(flashinfo, command_flash_info,
+			NULL,
+			"Print flash info",
+			NULL);
 
 
 static int command_flash_erase(int argc, char **argv)
@@ -91,14 +96,17 @@ static int command_flash_erase(int argc, char **argv)
 	int size = flash_get_erase_block_size();
 	int rv;
 
-	rv = parse_offset_size(argc - 1, argv + 1, &offset, &size);
+	rv = parse_offset_size(argc, argv, 1, &offset, &size);
 	if (rv)
 		return rv;
 
 	ccprintf("Erasing %d bytes at 0x%x...\n", size, offset, offset);
 	return flash_erase(offset, size);
 }
-DECLARE_CONSOLE_COMMAND(flasherase, command_flash_erase);
+DECLARE_CONSOLE_COMMAND(flasherase, command_flash_erase,
+			"offset [size]",
+			"Erase flash",
+			NULL);
 
 
 static int command_flash_write(int argc, char **argv)
@@ -110,7 +118,7 @@ static int command_flash_write(int argc, char **argv)
 	int i;
 
 
-	rv = parse_offset_size(argc - 1, argv + 1, &offset, &size);
+	rv = parse_offset_size(argc, argv, 1, &offset, &size);
 	if (rv)
 		return rv;
 
@@ -137,7 +145,10 @@ static int command_flash_write(int argc, char **argv)
 
 	return rv;
 }
-DECLARE_CONSOLE_COMMAND(flashwrite, command_flash_write);
+DECLARE_CONSOLE_COMMAND(flashwrite, command_flash_write,
+			"offset [size]",
+			"Write pattern to flash",
+			NULL);
 
 
 static int command_flash_wp(int argc, char **argv)
@@ -147,7 +158,7 @@ static int command_flash_wp(int argc, char **argv)
 	int rv;
 
 	if (argc < 2)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	/* Commands that don't need offset and size */
 	if (!strcasecmp(argv[1], "lock"))
@@ -156,7 +167,7 @@ static int command_flash_wp(int argc, char **argv)
 		return flash_lock_protect(0);
 
 	/* All remaining commands need offset and size */
-	rv = parse_offset_size(argc - 2, argv + 2, &offset, &size);
+	rv = parse_offset_size(argc, argv, 2, &offset, &size);
 	if (rv)
 		return rv;
 
@@ -167,9 +178,12 @@ static int command_flash_wp(int argc, char **argv)
 	else if (!strcasecmp(argv[1], "clear"))
 		return flash_set_protect(offset, size, 0);
 	else
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 }
-DECLARE_CONSOLE_COMMAND(flashwp, command_flash_wp);
+DECLARE_CONSOLE_COMMAND(flashwp, command_flash_wp,
+			"<lock | unlock | now | set | clear> offset [size]",
+			"Print or modify flash write protect",
+			NULL);
 
 /*****************************************************************************/
 /* Host commands */
diff --git a/common/gaia_power.c b/common/gaia_power.c
index 18e319368b..5a9f5bde3d 100644
--- a/common/gaia_power.c
+++ b/common/gaia_power.c
@@ -402,4 +402,7 @@ static int command_force_power(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(forcepower, command_force_power);
+DECLARE_CONSOLE_COMMAND(forcepower, command_force_power,
+			NULL,
+			"Force power on",
+			NULL);
diff --git a/common/gpio_commands.c b/common/gpio_commands.c
index ca8487533d..3327e9ae9c 100644
--- a/common/gpio_commands.c
+++ b/common/gpio_commands.c
@@ -58,7 +58,7 @@ static int command_gpio_get(int argc, char **argv)
 	if (argc == 2) {
 		i = find_signal_by_name(argv[1]);
 		if (i == GPIO_COUNT)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 		g = gpio_list + i;
 		v = gpio_get_level(i);
 		changed = last_val_changed(i, v);
@@ -82,7 +82,10 @@ static int command_gpio_get(int argc, char **argv)
 	}
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(gpioget, command_gpio_get);
+DECLARE_CONSOLE_COMMAND(gpioget, command_gpio_get,
+			"[name]",
+			"Read GPIO value(s)",
+			NULL);
 
 
 static int command_gpio_set(int argc, char **argv)
@@ -92,23 +95,26 @@ static int command_gpio_set(int argc, char **argv)
 	int v, i;
 
 	if (argc < 3)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	i = find_signal_by_name(argv[1]);
 	if (i == GPIO_COUNT)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 	g = gpio_list + i;
 
 	if (!g->mask)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	if (!(g->flags & GPIO_OUTPUT))
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	v = strtoi(argv[2], &e, 0);
 	if (*e)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM2;
 
 	return gpio_set_level(i, v);
 }
-DECLARE_CONSOLE_COMMAND(gpioset, command_gpio_set);
+DECLARE_CONSOLE_COMMAND(gpioset, command_gpio_set,
+			"name <0 | 1>",
+			"Set a GPIO",
+			NULL);
diff --git a/common/host_event_commands.c b/common/host_event_commands.c
index 70ee5a102c..507d603f7b 100644
--- a/common/host_event_commands.c
+++ b/common/host_event_commands.c
@@ -20,7 +20,7 @@ static int command_host_event(int argc, char **argv)
 		char *e;
 		int i = strtoi(argv[2], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM2;
 
 		if (!strcasecmp(argv[1], "set"))
 			lpc_set_host_events(i);
@@ -33,7 +33,7 @@ static int command_host_event(int argc, char **argv)
 		else if (!strcasecmp(argv[1], "wake"))
 			lpc_set_host_event_mask(LPC_HOST_EVENT_WAKE, i);
 		else
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 	}
 
 	/* Print current SMI/SCI status */
@@ -46,7 +46,10 @@ static int command_host_event(int argc, char **argv)
 		 lpc_get_host_event_mask(LPC_HOST_EVENT_WAKE));
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(hostevent, command_host_event);
+DECLARE_CONSOLE_COMMAND(hostevent, command_host_event,
+			"[set | clear | smi | sci | wake] [mask]",
+			"Print / set host event state",
+			NULL);
 
 /*****************************************************************************/
 /* Host commands */
diff --git a/common/keyboard.c b/common/keyboard.c
index 6b2dc48088..8b05485d2b 100644
--- a/common/keyboard.c
+++ b/common/keyboard.c
@@ -727,7 +727,10 @@ static int command_typematic(int argc, char **argv)
 	ccputs("\n");
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(typematic, command_typematic);
+DECLARE_CONSOLE_COMMAND(typematic, command_typematic,
+			"[first] [inter]",
+			"Get/set typematic delays",
+			NULL);
 
 
 static int command_codeset(int argc, char **argv)
@@ -740,7 +743,7 @@ static int command_codeset(int argc, char **argv)
 			scancode_set = set;
 			break;
 		default:
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 		}
 	}
 
@@ -748,7 +751,10 @@ static int command_codeset(int argc, char **argv)
 	ccprintf("I8042_XLATE: %d\n", controller_ram[0] & I8042_XLATE ? 1 : 0);
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(codeset, command_codeset);
+DECLARE_CONSOLE_COMMAND(codeset, command_codeset,
+			"[set]",
+			"Get/set keyboard codeset",
+			NULL);
 
 
 static int command_controller_ram(int argc, char **argv)
@@ -756,11 +762,11 @@ static int command_controller_ram(int argc, char **argv)
 	int index;
 
 	if (argc < 2)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	index = strtoi(argv[1], NULL, 0);
 	if (index >= 0x20)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	if (argc >= 3)
 		update_ctl_ram(index, strtoi(argv[2], NULL, 0));
@@ -768,7 +774,10 @@ static int command_controller_ram(int argc, char **argv)
 	ccprintf("%d = 0x%02x\n", index, controller_ram[index]);
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(ctrlram, command_controller_ram);
+DECLARE_CONSOLE_COMMAND(ctrlram, command_controller_ram,
+			"index [value]",
+			"Get/set keyboard controller RAM",
+			NULL);
 
 
 static int command_keyboard_press(int argc, char **argv)
@@ -791,15 +800,15 @@ static int command_keyboard_press(int argc, char **argv)
 
 		c = strtoi(argv[1], &e, 0);
 		if (*e || c < 0 || c >= CROS_COL_NUM)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 
 		r = strtoi(argv[2], &e, 0);
 		if (*e || r < 0 || r >= CROS_ROW_NUM)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM2;
 
 		p = strtoi(argv[3], &e, 0);
 		if (*e || p < 0 || p > 1)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM3;
 
 		if ((simulated_key[c] & (1 << r)) == (p << r))
 			return EC_SUCCESS;
@@ -811,7 +820,10 @@ static int command_keyboard_press(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(kbpress, command_keyboard_press);
+DECLARE_CONSOLE_COMMAND(kbpress, command_keyboard_press,
+			"[col] [row] [0 | 1]",
+			"Simulate keypress",
+			NULL);
 
 
 static int command_keyboard_log(int argc, char **argv)
@@ -841,11 +853,14 @@ static int command_keyboard_log(int argc, char **argv)
 			shared_mem_release(kblog);
 		kblog = NULL;
 	} else
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(kblog, command_keyboard_log);
+DECLARE_CONSOLE_COMMAND(kblog, command_keyboard_log,
+			"[on | off]",
+			"Print or toggle keyboard event log",
+			NULL);
 
 
 /* Preserves the states of keyboard controller to keep the initialized states
diff --git a/common/lightbar.c b/common/lightbar.c
index 40adeff920..e440b04620 100644
--- a/common/lightbar.c
+++ b/common/lightbar.c
@@ -832,10 +832,10 @@ static int command_lightbar(int argc, char **argv)
 			return 0;
 		}
 		num = 0xff & strtoi(argv[2], &e, 16);
-		if (e && *e)
+		if (*e)
 			num = find_msg_by_name(argv[2]);
 		if (num >= LIGHTBAR_NUM_SEQUENCES)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM2;
 		lightbar_sequence(num);
 		return EC_SUCCESS;
 	}
@@ -867,4 +867,7 @@ static int command_lightbar(int argc, char **argv)
 
 	return EC_ERROR_INVAL;
 }
-DECLARE_CONSOLE_COMMAND(lightbar, command_lightbar);
+DECLARE_CONSOLE_COMMAND(lightbar, command_lightbar,
+			"[on | off | init | brightness | seq] | [ctrl reg val]",
+			"Get/set lightbar state",
+			NULL);
diff --git a/common/memory_commands.c b/common/memory_commands.c
index a313379e86..46df50e74d 100644
--- a/common/memory_commands.c
+++ b/common/memory_commands.c
@@ -13,11 +13,18 @@ static int command_write_word(int argc, char **argv)
 {
 	volatile uint32_t *address;
 	uint32_t value;
+	char *e;
 
 	if (argc != 3)
-		return EC_ERROR_INVAL;
-	address = (uint32_t*)strtoi(argv[1], NULL, 0);
-	value = strtoi(argv[2], NULL, 0);
+		return EC_ERROR_PARAM_COUNT;
+
+	address = (uint32_t *)strtoi(argv[1], &e, 0);
+	if (*e)
+		return EC_ERROR_PARAM1;
+
+	value = strtoi(argv[2], &e, 0);
+	if (*e)
+		return EC_ERROR_PARAM2;
 
 	ccprintf("write 0x%p = 0x%08x\n", address, value);
 	cflush();  /* Flush before writing in case this crashes */
@@ -26,24 +33,32 @@ static int command_write_word(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(ww, command_write_word);
-DECLARE_CONSOLE_COMMAND(writeword, command_write_word);
+DECLARE_CONSOLE_COMMAND(ww, command_write_word,
+			"addr value",
+			"Write a word to memory",
+			NULL);
 
 
 static int command_read_word(int argc, char **argv)
 {
 	volatile uint32_t *address;
 	uint32_t value;
+	char *e;
 
 	if (argc != 2)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
+
+	address = (uint32_t *)strtoi(argv[1], &e, 0);
+	if (*e)
+		return EC_ERROR_PARAM1;
 
-	address = (uint32_t*)strtoi(argv[1], NULL, 0);
 	value = *address;
 
 	ccprintf("read 0x%p = 0x%08x\n", address, value);
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(rw, command_read_word);
-DECLARE_CONSOLE_COMMAND(readword, command_read_word);
+DECLARE_CONSOLE_COMMAND(rw, command_read_word,
+			"addr",
+			"Read a word from memory",
+			NULL);
diff --git a/common/port80.c b/common/port80.c
index 8c5e5924aa..b879ed2043 100644
--- a/common/port80.c
+++ b/common/port80.c
@@ -55,4 +55,7 @@ static int command_port80(int argc, char **argv)
 	ccputs(" <--new\n");
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(port80, command_port80);
+DECLARE_CONSOLE_COMMAND(port80, command_port80,
+			"[scroll]",
+			"Print port80 writes or toggle port80 scrolling",
+			NULL);
diff --git a/common/power_led.c b/common/power_led.c
index f666a76dd9..8647619f39 100644
--- a/common/power_led.c
+++ b/common/power_led.c
@@ -83,6 +83,9 @@ static int command_powerled(int argc, char **argv)
 		if (!strcasecmp(argv[1], color_names[i]))
 			return powerled_set(i);
 	}
-	return EC_ERROR_INVAL;
+	return EC_ERROR_PARAM1;
 }
-DECLARE_CONSOLE_COMMAND(powerled, command_powerled);
+DECLARE_CONSOLE_COMMAND(powerled, command_powerled,
+			"<off | red | yellow | green>",
+			"Set power LED color",
+			NULL);
diff --git a/common/smart_battery.c b/common/smart_battery.c
index eaae395777..fb73f28ac3 100644
--- a/common/smart_battery.c
+++ b/common/smart_battery.c
@@ -201,7 +201,10 @@ static int command_battery(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(battery, command_battery);
+DECLARE_CONSOLE_COMMAND(battery, command_battery,
+			NULL,
+			"Print battery info",
+			NULL);
 
 
 /* Usage:sb <r/w> cmd [uint16_t w_word]
@@ -217,11 +220,11 @@ static int command_sb(int argc, char **argv)
 	char *e;
 
 	if (argc < 3)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	cmd = strtoi(argv[2], &e, 0);
 	if (*e)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM2;
 
 	if (argv[1][0] == 'r') {
 		rv = i2c_read16(I2C_PORT_BATTERY, BATTERY_ADDR, cmd, &d);
@@ -233,7 +236,7 @@ static int command_sb(int argc, char **argv)
 	} else if (argc >= 4 && argv[1][0] == 'w') {
 		d = strtoi(argv[3], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM3;
 
 		ccprintf("W SBCMD[%04x] 0x%04x (%d)\n", cmd, d, d);
 		rv = i2c_write16(I2C_PORT_BATTERY, BATTERY_ADDR, cmd, d);
@@ -246,5 +249,8 @@ static int command_sb(int argc, char **argv)
 
 
 }
-DECLARE_CONSOLE_COMMAND(sb, command_sb);
+DECLARE_CONSOLE_COMMAND(sb, command_sb,
+			"[r addr | w addr value]",
+			"Read/write smart battery data",
+			NULL);
 
diff --git a/common/system_common.c b/common/system_common.c
index f80db6f86c..9d93cdef25 100644
--- a/common/system_common.c
+++ b/common/system_common.c
@@ -455,17 +455,10 @@ static int command_sysinfo(int argc, char **argv)
 	ccprintf("Jump: %s\n", system_jumped_to_this_image() ? "yes" : "no");
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(sysinfo, command_sysinfo);
-
-
-static int command_chipinfo(int argc, char **argv)
-{
-	ccprintf("Vendor:   %s\n", system_get_chip_vendor());
-	ccprintf("Name:     %s\n", system_get_chip_name());
-	ccprintf("Revision: %s\n", system_get_chip_revision());
-	return EC_SUCCESS;
-}
-DECLARE_CONSOLE_COMMAND(chipinfo, command_chipinfo);
+DECLARE_CONSOLE_COMMAND(sysinfo, command_sysinfo,
+			NULL,
+			"Print system info",
+			NULL);
 
 
 #ifdef CONSOLE_COMMAND_SCRATCHPAD
@@ -477,14 +470,17 @@ static int command_scratchpad(int argc, char **argv)
 		char *e;
 		int s = strtoi(argv[1], &e, 0);
 		if (*e)
-			return EC_ERROR_INVAL;
+			return EC_ERROR_PARAM1;
 		rv = system_set_scratchpad(s);
 	}
 
 	ccprintf("Scratchpad: 0x%08x\n", system_get_scratchpad());
 	return rv;
 }
-DECLARE_CONSOLE_COMMAND(scratchpad, command_scratchpad);
+DECLARE_CONSOLE_COMMAND(scratchpad, command_scratchpad,
+			"[val]",
+			"Get or set scratchpad value",
+			NULL);
 #endif
 
 
@@ -494,7 +490,7 @@ static int command_hibernate(int argc, char **argv)
 	int microseconds = 0;
 
 	if (argc < 2)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 	seconds = strtoi(argv[1], NULL, 0);
 	if (argc >= 3)
 		microseconds = strtoi(argv[2], NULL, 0);
@@ -506,11 +502,16 @@ static int command_hibernate(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(hibernate, command_hibernate);
+DECLARE_CONSOLE_COMMAND(hibernate, command_hibernate,
+			"sec [usec]",
+			"Hibernate the EC",
+			NULL);
 
 
 static int command_version(int argc, char **argv)
 {
+	ccprintf("Chip:  %s %s %s\n", system_get_chip_vendor(),
+		 system_get_chip_name(), system_get_chip_revision());
 	ccprintf("Board: %d\n", system_get_board_version());
 	ccprintf("RO:    %s\n", system_get_version(SYSTEM_IMAGE_RO));
 	ccprintf("RW-A:  %s\n", system_get_version(SYSTEM_IMAGE_RW_A));
@@ -518,7 +519,10 @@ static int command_version(int argc, char **argv)
 	ccprintf("Build: %s\n", system_get_build_info());
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(version, command_version);
+DECLARE_CONSOLE_COMMAND(version, command_version,
+			NULL,
+			"Print versions",
+			NULL);
 
 
 static int command_sysjump(int argc, char **argv)
@@ -530,30 +534,32 @@ static int command_sysjump(int argc, char **argv)
 	 * be disabled. */
 
 	if (argc < 2)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM_COUNT;
 
 	ccputs("Processing sysjump command\n");
 
 	/* Handle named images */
-	if (!strcasecmp(argv[1], "RO")) {
+	if (!strcasecmp(argv[1], "RO"))
 		return system_run_image_copy(SYSTEM_IMAGE_RO, 0);
-	} else if (!strcasecmp(argv[1], "A")) {
+	else if (!strcasecmp(argv[1], "A"))
 		return system_run_image_copy(SYSTEM_IMAGE_RW_A, 0);
-	} else if (!strcasecmp(argv[1], "B")) {
+	else if (!strcasecmp(argv[1], "B"))
 		return system_run_image_copy(SYSTEM_IMAGE_RW_B, 0);
-	}
 
 	/* Check for arbitrary address */
 	addr = strtoi(argv[1], &e, 0);
 	if (*e)
-		return EC_ERROR_INVAL;
+		return EC_ERROR_PARAM1;
 
 	ccprintf("Jumping to 0x%08x\n", addr);
 	cflush();
 	jump_to_image(addr, 0);
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(sysjump, command_sysjump);
+DECLARE_CONSOLE_COMMAND(sysjump, command_sysjump,
+			"[RO | A | B | addr]",
+			"Jump to a system image or address",
+			NULL);
 
 
 static int command_reboot(int argc, char **argv)
@@ -570,7 +576,10 @@ static int command_reboot(int argc, char **argv)
 	system_reset(is_hard);
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(reboot, command_reboot);
+DECLARE_CONSOLE_COMMAND(reboot, command_reboot,
+			"[hard]",
+			"Reboot the EC",
+			NULL);
 
 /*****************************************************************************/
 /* Host commands */
diff --git a/common/temp_sensor.c b/common/temp_sensor.c
index 980d62d13b..c027fb560b 100644
--- a/common/temp_sensor.c
+++ b/common/temp_sensor.c
@@ -119,4 +119,7 @@ static int command_temps(int argc, char **argv)
 
 	return rv;
 }
-DECLARE_CONSOLE_COMMAND(temps, command_temps);
+DECLARE_CONSOLE_COMMAND(temps, command_temps,
+			NULL,
+			"Print temp sensors",
+			NULL);
diff --git a/common/thermal.c b/common/thermal.c
index 72709cd8ec..4f14ca3f50 100644
--- a/common/thermal.c
+++ b/common/thermal.c
@@ -241,18 +241,12 @@ static int command_thermal_config(int argc, char **argv)
 	char *e;
 	int sensor_type, threshold_id, value;
 
-	if (argc != 2 && argc != 4) {
-		ccputs("Usage: thermalconf <sensor_type> "
-		       "[<threshold_id> <value>]\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (argc != 2 && argc != 4)
+		return EC_ERROR_PARAM_COUNT;
 
 	sensor_type = strtoi(argv[1], &e, 0);
-	if ((e && *e) || sensor_type < 0 ||
-	    sensor_type >= TEMP_SENSOR_TYPE_COUNT) {
-		ccputs("Bad sensor type ID.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (*e || sensor_type < 0 || sensor_type >= TEMP_SENSOR_TYPE_COUNT)
+		return EC_ERROR_PARAM1;
 
 	if (argc == 2) {
 		print_thermal_config(sensor_type);
@@ -260,16 +254,12 @@ static int command_thermal_config(int argc, char **argv)
 	}
 
 	threshold_id = strtoi(argv[2], &e, 0);
-	if ((e && *e) || threshold_id < 0 || threshold_id >= THRESHOLD_COUNT) {
-		ccputs("Bad threshold ID.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (*e || threshold_id < 0 || threshold_id >= THRESHOLD_COUNT)
+		return EC_ERROR_PARAM2;
 
 	value = strtoi(argv[3], &e, 0);
-	if ((e && *e) || value < 0) {
-		ccputs("Bad threshold value.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (*e || value < 0)
+		return EC_ERROR_PARAM3;
 
 	thermal_config[sensor_type].thresholds[threshold_id] = value;
 	ccprintf("Setting threshold %d of sensor type %d to %d\n",
@@ -277,7 +267,10 @@ static int command_thermal_config(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(thermalconf, command_thermal_config);
+DECLARE_CONSOLE_COMMAND(thermalconf, command_thermal_config,
+			"sensortype [threshold_id temp]",
+			"Get/set thermal threshold temp",
+			NULL);
 
 
 static int command_fan_config(int argc, char **argv)
@@ -285,18 +278,13 @@ static int command_fan_config(int argc, char **argv)
 	char *e;
 	int sensor_type, stepping_id, value;
 
-	if (argc != 2 && argc != 4) {
-		ccputs("Usage: thermalfan <sensor_type> "
-		       "[<stepping_id> <value>]\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (argc != 2 && argc != 4)
+		return EC_ERROR_PARAM_COUNT;
 
 	sensor_type = strtoi(argv[1], &e, 0);
 	if ((e && *e) || sensor_type < 0 ||
-	    sensor_type >= TEMP_SENSOR_TYPE_COUNT) {
-		ccputs("Bad sensor type ID.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	    sensor_type >= TEMP_SENSOR_TYPE_COUNT)
+		return EC_ERROR_PARAM1;
 
 	if (argc == 2) {
 		print_fan_stepping(sensor_type);
@@ -304,16 +292,12 @@ static int command_fan_config(int argc, char **argv)
 	}
 
 	stepping_id = strtoi(argv[2], &e, 0);
-	if ((e && *e) || stepping_id < 0 || stepping_id >= THERMAL_FAN_STEPS) {
-		ccputs("Bad stepping ID.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if ((e && *e) || stepping_id < 0 || stepping_id >= THERMAL_FAN_STEPS)
+		return EC_ERROR_PARAM2;
 
 	value = strtoi(argv[3], &e, 0);
-	if ((e && *e) || value < 0) {
-		ccputs("Bad threshold value.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (*e || value < 0)
+		return EC_ERROR_PARAM3;
 
 	thermal_config[sensor_type].thresholds[THRESHOLD_COUNT + stepping_id] =
 		value;
@@ -322,11 +306,17 @@ static int command_fan_config(int argc, char **argv)
 
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(thermalfan, command_fan_config);
+DECLARE_CONSOLE_COMMAND(thermalfan, command_fan_config,
+			"sensortype [threshold_id rpm]",
+			"Get/set thermal threshold fan rpm",
+			NULL);
 
 
 static int command_thermal_auto_fan_ctrl(int argc, char **argv)
 {
 	return thermal_toggle_auto_fan_ctrl(1);
 }
-DECLARE_CONSOLE_COMMAND(autofan, command_thermal_auto_fan_ctrl);
+DECLARE_CONSOLE_COMMAND(autofan, command_thermal_auto_fan_ctrl,
+			NULL,
+			"Enable thermal fan control",
+			NULL);
diff --git a/common/tmp006.c b/common/tmp006.c
index 9ecc8084e3..18882c5fde 100644
--- a/common/tmp006.c
+++ b/common/tmp006.c
@@ -291,4 +291,7 @@ static int command_sensor_info(int argc, char **argv)
 
 	return rv1;
 }
-DECLARE_CONSOLE_COMMAND(tmp006, command_sensor_info);
+DECLARE_CONSOLE_COMMAND(tmp006, command_sensor_info,
+			NULL,
+			"Print TMP006 sensors",
+			NULL);
diff --git a/common/usb_charge.c b/common/usb_charge.c
index 175717fadf..6495a491c0 100644
--- a/common/usb_charge.c
+++ b/common/usb_charge.c
@@ -120,34 +120,30 @@ static int command_set_mode(int argc, char **argv)
 {
 	int port_id = -1;
 	int mode = -1;
-	char* endptr;
-
-	if (argc != 3) {
-		ccputs("Usage: usbchargemode <port_id> <mode>\n");
-		ccputs("Modes: 0=Disabled.\n"
-		       "       1=Dedicated charging. Auto select.\n"
-		       "       2=Dedicated charging. BC 1.2.\n"
-		       "       3=Downstream. Max 500mA.\n"
-		       "       4=Downstream. Max 1.5A.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	char *e;
 
-	port_id = strtoi(argv[1], &endptr, 0);
-	if (*endptr || port_id < 0 || port_id >= USB_CHARGE_PORT_COUNT) {
-		ccputs("Invalid port ID.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	if (argc != 3)
+		return EC_ERROR_PARAM_COUNT;
 
-	mode = strtoi(argv[2], &endptr, 0);
-	if (*endptr || mode < 0 || mode >= USB_CHARGE_MODE_COUNT) {
-		ccputs("Invalid mode.\n");
-		return EC_ERROR_UNKNOWN;
-	}
+	port_id = strtoi(argv[1], &e, 0);
+	if (*e || port_id < 0 || port_id >= USB_CHARGE_PORT_COUNT)
+		return EC_ERROR_PARAM1;
+
+	mode = strtoi(argv[2], &e, 0);
+	if (*e || mode < 0 || mode >= USB_CHARGE_MODE_COUNT)
+		return EC_ERROR_PARAM2;
 
-	ccprintf("Setting USB mode...\n");
 	return usb_charge_set_mode(port_id, mode);
 }
-DECLARE_CONSOLE_COMMAND(usbchargemode, command_set_mode);
+DECLARE_CONSOLE_COMMAND(usbchargemode, command_set_mode,
+			"<port> <0 | 1 | 2 | 3 | 4>",
+			"Set USB charge mode",
+			"Modes: 0=Disabled.\n"
+			"       1=Dedicated charging. Auto select.\n"
+			"       2=Dedicated charging. BC 1.2.\n"
+			"       3=Downstream. Max 500mA.\n"
+			"       4=Downstream. Max 1.5A.\n");
+
 
 /*****************************************************************************/
 /* Hooks */
diff --git a/common/x86_power.c b/common/x86_power.c
index 9f1fd175d5..48d05b6d6d 100644
--- a/common/x86_power.c
+++ b/common/x86_power.c
@@ -588,20 +588,6 @@ void x86_power_task(void)
 /*****************************************************************************/
 /* Console commands */
 
-static int command_x86power(int argc, char **argv)
-{
-	/* Print current state */
-	ccprintf("Current X86 state: %d (%s)\n", state, state_names[state]);
-
-	/* Forcing a power state from EC is deprecated */
-	if (argc > 1)
-		ccputs("Use 'powerbtn' instead of 'x86power s0'.\n");
-
-	return EC_SUCCESS;
-}
-DECLARE_CONSOLE_COMMAND(x86power, command_x86power);
-
-
 static int command_x86reset(int argc, char **argv)
 {
 	int is_cold = 1;
@@ -616,4 +602,7 @@ static int command_x86reset(int argc, char **argv)
 	x86_power_reset(is_cold);
 	return EC_SUCCESS;
 }
-DECLARE_CONSOLE_COMMAND(x86reset, command_x86reset);
+DECLARE_CONSOLE_COMMAND(x86reset, command_x86reset,
+			"[warm | cold]",
+			"Issue x86 reset",
+			NULL);