Add LPC lightbar command to get the current sequence.

Instead of making the STOP command synchronous, we can just have the
host-side app tell the EC to stop, then poll until it has.

BUG=chrome-os-partner:9349
TEST=manual

"make BOARD=link", then
copy build/link/util/lbplay to the host and run it.

Change-Id: I846924ae7994a498e0089197785cf239898fe2a3
Signed-off-by: Bill Richardson <wfrichar@chromium.org>

5 files changed, 110 insertions, 54 deletions

diff --git a/common/lightbar.c b/common/lightbar.c
index d81b75c298..3c90e834c5 100644
--- a/common/lightbar.c
+++ b/common/lightbar.c
@@ -590,10 +590,11 @@ static struct lightbar_cmd_t lightbar_cmds[] = {
 };
 #undef LBMSG
 
+static enum lightbar_sequence current_state, previous_state;
+
 void lightbar_task(void)
 {
 	uint32_t msg;
-	enum lightbar_sequence state, previous_state;
 
 	/* Keep the controllers out of reset. The reset pullup uses more power
 	 * than leaving them in standby. */
@@ -605,36 +606,36 @@ void lightbar_task(void)
 
 	/* FIXME: What to do first? For now, nothing, followed by more
 	   nothing. */
-	state = LIGHTBAR_STOP;
+	current_state = LIGHTBAR_STOP;
 	previous_state = LIGHTBAR_S5;
 
 	while (1) {
-		msg = lightbar_cmds[state].sequence();
+		msg = lightbar_cmds[current_state].sequence();
 		CPRINTF("[%s(%d)]\n", __func__, msg);
 		msg = TASK_EVENT_CUSTOM(msg);
 		if (msg && msg < LIGHTBAR_NUM_SEQUENCES) {
-			previous_state = state;
-			state = TASK_EVENT_CUSTOM(msg);
+			previous_state = current_state;
+			current_state = TASK_EVENT_CUSTOM(msg);
 		} else {
-			switch (state) {
+			switch (current_state) {
 			case LIGHTBAR_S5S3:
-				state = LIGHTBAR_S3;
+				current_state = LIGHTBAR_S3;
 				break;
 			case LIGHTBAR_S3S0:
-				state = LIGHTBAR_S0;
+				current_state = LIGHTBAR_S0;
 				break;
 			case LIGHTBAR_S0S3:
-				state = LIGHTBAR_S3;
+				current_state = LIGHTBAR_S3;
 				break;
 			case LIGHTBAR_S3S5:
-				state = LIGHTBAR_S5;
+				current_state = LIGHTBAR_S5;
 				break;
 			case LIGHTBAR_TEST:
 			case LIGHTBAR_STOP:
 			case LIGHTBAR_RUN:
 			case LIGHTBAR_ERROR:
 			case LIGHTBAR_KONAMI:
-				state = previous_state;
+				current_state = previous_state;
 			default:
 				break;
 			}
@@ -663,20 +664,19 @@ static const uint8_t dump_reglist[] = {
 	0x18, 0x19, 0x1a
 };
 
-static void do_cmd_dump(uint8_t *outptr)
+static void do_cmd_dump(struct lpc_params_lightbar_cmd *ptr)
 {
-	int i, n;
+	int i;
 	uint8_t reg;
 
-	BUILD_ASSERT(3 * ARRAY_SIZE(dump_reglist) ==
-		     sizeof(((struct lpc_params_lightbar_cmd *)0)->out.dump));
+	BUILD_ASSERT(ARRAY_SIZE(dump_reglist) ==
+		     ARRAY_SIZE(ptr->out.dump.vals));
 
-	n = ARRAY_SIZE(dump_reglist);
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < ARRAY_SIZE(dump_reglist); i++) {
 		reg = dump_reglist[i];
-		*outptr++ = reg;
-		*outptr++ = controller_read(0, reg);
-		*outptr++ = controller_read(1, reg);
+		ptr->out.dump.vals[i].reg = reg;
+		ptr->out.dump.vals[i].ic0 = controller_read(0, reg);
+		ptr->out.dump.vals[i].ic1 = controller_read(1, reg);
 	}
 }
 
@@ -704,7 +704,7 @@ static enum lpc_status lpc_cmd_lightbar(uint8_t *data)
 
 	switch (ptr->in.cmd) {
 	case LIGHTBAR_CMD_DUMP:
-		do_cmd_dump(ptr->out.dump);
+		do_cmd_dump(ptr);
 		break;
 	case LIGHTBAR_CMD_OFF:
 		lightbar_off();
@@ -732,7 +732,11 @@ static enum lpc_status lpc_cmd_lightbar(uint8_t *data)
 			   ptr->in.rgb.green,
 			   ptr->in.rgb.blue);
 		break;
+	case LIGHTBAR_CMD_GET_SEQ:
+		ptr->out.get_seq.num = current_state;
+		break;
 	default:
+		CPRINTF("[invalid lightbar cmd 0x%x]\n", ptr->in.cmd);
 		return EC_LPC_RESULT_INVALID_PARAM;
 	}
 
@@ -778,19 +782,24 @@ static void show_msg_names(void)
 	ccprintf("sequence names:");
 	for (i = 0; i < LIGHTBAR_NUM_SEQUENCES; i++)
 		ccprintf(" %s", lightbar_cmds[i].string);
-	ccprintf("\n");
+	ccprintf("\nCurrent = 0x%x %s\n", current_state,
+		 lightbar_cmds[current_state].string);
 }
 
 static int command_lightbar(int argc, char **argv)
 {
-	int i, j;
-	uint8_t num, buf[128];
+	int i;
+	uint8_t num;
+	struct lpc_params_lightbar_cmd params;
 
 	if (1 == argc) {		/* no args = dump 'em all */
-		do_cmd_dump(buf);
-		for (i = j = 0; i < ARRAY_SIZE(dump_reglist); i++, j += 3)
+		do_cmd_dump(&params);
+		for (i = 0; i < ARRAY_SIZE(dump_reglist); i++)
 			ccprintf(" %02x     %02x     %02x\n",
-				 buf[j], buf[j+1], buf[j+2]);
+				 params.out.dump.vals[i].reg,
+				 params.out.dump.vals[i].ic0,
+				 params.out.dump.vals[i].ic1);
+
 		return EC_SUCCESS;
 	}
 
diff --git a/include/lightbar.h b/include/lightbar.h
index 0cc19e2b9b..395166191d 100644
--- a/include/lightbar.h
+++ b/include/lightbar.h
@@ -35,6 +35,7 @@ enum lightbar_command {
 	LIGHTBAR_CMD_SEQ,
 	LIGHTBAR_CMD_REG,
 	LIGHTBAR_CMD_RGB,
+	LIGHTBAR_CMD_GET_SEQ,
 	LIGHTBAR_NUM_CMDS
 };
 
diff --git a/include/lpc_commands.h b/include/lpc_commands.h
index 354c059b1f..c7f2530df1 100644
--- a/include/lpc_commands.h
+++ b/include/lpc_commands.h
@@ -349,7 +349,6 @@ struct lpc_params_pwm_set_keyboard_backlight {
  * command to say "talk to the lightbar", we put the "and tell it to do X"
  * part into a subcommand. We'll make separate structs for subcommands with
  * different input args, so that we know how much to expect. */
-
 #define EC_LPC_COMMAND_LIGHTBAR_CMD 0x28
 struct lpc_params_lightbar_cmd {
 	union {
@@ -357,7 +356,7 @@ struct lpc_params_lightbar_cmd {
 			uint8_t cmd;
 			struct {
 				uint8_t cmd;
-			} dump, off, on, init;
+			} dump, off, on, init, get_seq;
 			struct num {
 				uint8_t cmd;
 				uint8_t num;
@@ -373,14 +372,19 @@ struct lpc_params_lightbar_cmd {
 			} rgb;
 		} in;
 		union {
-			uint8_t dump[69];
-			uint8_t off[0];
-			uint8_t on[0];
-			uint8_t init[0];
-			uint8_t brightness[0];
-			uint8_t seq[0];
-			uint8_t reg[0];
-			uint8_t rgb[0];
+			struct dump {
+				struct {
+					uint8_t reg;
+					uint8_t ic0;
+					uint8_t ic1;
+				} vals[23];
+			} dump;
+			struct get_seq {
+				uint8_t num;
+			} get_seq;
+			struct {
+				/* no return params */
+			} off, on, init, brightness, seq, reg, rgb;
 		} out;
 	};
 } __attribute__ ((packed));
diff --git a/util/ectool.c b/util/ectool.c
index d90db5dee5..c395a525af 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -11,10 +11,12 @@
 #include <sys/io.h>
 #include <unistd.h>
 
+#include "battery.h"
 #include "lightbar.h"
 #include "lpc_commands.h"
-#include "battery.h"
 
+/* Handy tricks */
+#define BUILD_ASSERT(cond) ((void)sizeof(char[1 - 2*!(cond)]))
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 /* Don't use a macro where an inline will do... */
 static inline int MIN(int a, int b) { return a < b ? a : b; }
@@ -846,14 +848,14 @@ static const char const *lightbar_cmds[] = {
 };
 #undef LBMSG
 
-/* This needs to match the values used in common/lightbar.c.  I'd like to
+/* This needs to match the values defined in lightbar.h. I'd like to
  * define this in one and only one place, but I can't think of a good way to do
  * that without adding bunch of complexity. This will do for now.
  */
 static const struct {
 	uint8_t insize;
 	uint8_t outsize;
-} lb_command_paramcount[LIGHTBAR_NUM_CMDS] = {
+} lb_command_paramcount[] = {
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.dump),
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.dump) },
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.off),
@@ -870,6 +872,8 @@ static const struct {
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.reg) },
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.rgb),
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.rgb) },
+	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.get_seq),
+	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.get_seq) },
 };
 
 static int lb_help(const char *cmd)
@@ -898,15 +902,6 @@ static uint8_t lb_find_msg_by_name(const char *str)
 	return LIGHTBAR_NUM_SEQUENCES;
 }
 
-static void lb_show_msg_names(void)
-{
-	int i;
-	printf("sequence names:");
-	for (i = 0; i < LIGHTBAR_NUM_SEQUENCES; i++)
-		printf(" %s", lightbar_cmds[i]);
-	printf("\n");
-}
-
 static int lb_do_cmd(enum lightbar_command cmd,
 		     struct lpc_params_lightbar_cmd *ptr)
 {
@@ -918,6 +913,21 @@ static int lb_do_cmd(enum lightbar_command cmd,
 	return r;
 }
 
+static void lb_show_msg_names(void)
+{
+	int i, current_state;
+	struct lpc_params_lightbar_cmd param;
+
+	(void)lb_do_cmd(LIGHTBAR_CMD_GET_SEQ, &param);
+	current_state = param.out.get_seq.num;
+
+	printf("sequence names:");
+	for (i = 0; i < LIGHTBAR_NUM_SEQUENCES; i++)
+		printf(" %s", lightbar_cmds[i]);
+	printf("\nCurrent = 0x%x %s\n", current_state,
+	       lightbar_cmds[current_state]);
+}
+
 static int cmd_lightbar(int argc, char **argv)
 {
 	int i, r;
@@ -927,11 +937,11 @@ static int cmd_lightbar(int argc, char **argv)
 		r = lb_do_cmd(LIGHTBAR_CMD_DUMP, &param);
 		if (r)
 			return r;
-		for (i = 0; i < sizeof(param.out.dump); i += 3) {
+		for (i = 0; i < ARRAY_SIZE(param.out.dump.vals); i++) {
 			printf(" %02x     %02x     %02x\n",
-			       param.out.dump[i],
-			       param.out.dump[i+1],
-			       param.out.dump[i+2]);
+			       param.out.dump.vals[i].reg,
+			       param.out.dump.vals[i].ic0,
+			       param.out.dump.vals[i].ic1);
 		}
 		return 0;
 	}
@@ -1465,6 +1475,8 @@ int main(int argc, char *argv[])
 {
 	const struct command *cmd;
 
+	BUILD_ASSERT(ARRAY_SIZE(lb_command_paramcount) == LIGHTBAR_NUM_CMDS);
+
 	if (argc < 2 || !strcasecmp(argv[1], "-?") ||
 	    !strcasecmp(argv[1], "help")) {
 		print_help(argv[0]);
diff --git a/util/lbplay.c b/util/lbplay.c
index 33d8bb731e..56934ebea2 100644
--- a/util/lbplay.c
+++ b/util/lbplay.c
@@ -13,6 +13,9 @@
 #include "lightbar.h"
 #include "lpc_commands.h"
 
+/* Handy tricks */
+#define BUILD_ASSERT(cond) ((void)sizeof(char[1 - 2*!(cond)]))
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 /* Waits for the EC to be unbusy.  Returns 0 if unbusy, non-zero if
  * timeout. */
@@ -80,7 +83,7 @@ static int ec_command(int command, const void *indata, int insize,
 static const struct {
 	uint8_t insize;
 	uint8_t outsize;
-} lb_command_paramcount[LIGHTBAR_NUM_CMDS] = {
+} lb_command_paramcount[] = {
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.dump),
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.dump) },
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.off),
@@ -97,6 +100,8 @@ static const struct {
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.reg) },
 	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.rgb),
 	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.rgb) },
+	{ sizeof(((struct lpc_params_lightbar_cmd *)0)->in.get_seq),
+	  sizeof(((struct lpc_params_lightbar_cmd *)0)->out.get_seq) },
 };
 
 
@@ -169,11 +174,33 @@ void lightbar_rgb(int led, int red, int green, int blue)
 		   &param, lb_command_paramcount[param.in.cmd].outsize);
 }
 
+void wait_for_ec_to_stop(void)
+{
+	int r;
+	struct lpc_params_lightbar_cmd param;
+	int count = 0;
+
+	do {
+		usleep(100000);
+		param.in.cmd = LIGHTBAR_CMD_GET_SEQ;
+		r = ec_command(EC_LPC_COMMAND_LIGHTBAR_CMD,
+			       &param,
+			       lb_command_paramcount[param.in.cmd].insize,
+			       &param,
+			       lb_command_paramcount[param.in.cmd].outsize);
+		if (count++ > 10) {
+			fprintf(stderr, "EC isn't responding\n");
+			exit(1);
+		}
+	} while (r != 0 && param.out.get_seq.num != LIGHTBAR_STOP);
+}
 
 int main(int argc, char **argv)
 {
 	int i;
 
+	BUILD_ASSERT(ARRAY_SIZE(lb_command_paramcount) == LIGHTBAR_NUM_CMDS);
+
 	/* Request I/O privilege */
 	if (iopl(3) < 0) {
 		perror("Error getting I/O privilege");
@@ -184,6 +211,9 @@ int main(int argc, char **argv)
 	/* Tell the EC to let us drive. */
 	lightbar_sequence(LIGHTBAR_STOP);
 
+	/* Wait until it's listening */
+	wait_for_ec_to_stop();
+
 	/* Initialize it */
 	lightbar_off();
 	lightbar_init_vals();