Refactor lightbar host command to pass messages the new way.

We recently changed the way host messages are passed to the EC to make it
work nicer across I2C. When we did, we updated all the internal structs
except those used for lightbar commands. This CL updates the lightbar
commands too.

BUG=chrome-os-partner:11277
BRANCH=all
TEST=manual

This shouldn't change anything, but you can ensure that by poking at the
lightbar manually. On Link, run this from a root shell:

  ectool lightbar seq stop
  ectool lightbar 4 ff 00 ff
  ectool lightbar seq run

With the first command, the lightbar pattern should freeze.
With the second command, it should turn magenta.
With the third command, it should resume pulsing as before.

Change-Id: Ic5dc4c827b3b4459288d7d9bd7d06af8a5176b3c
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/33798
Reviewed-by: Randall Spangler <rspangler@chromium.org>

2 files changed, 113 insertions, 112 deletions

diff --git a/util/ectool.c b/util/ectool.c
index b4e9dd20a0..7ddaa07c06 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -1033,31 +1033,26 @@ static const char const *lightbar_cmds[] = {
  * 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.
  */
+#define LB_SIZES(SUBCMD) { \
+		sizeof(((struct ec_params_lightbar *)0)->SUBCMD) \
+		+ sizeof(((struct ec_params_lightbar *)0)->cmd), \
+		sizeof(((struct ec_response_lightbar *)0)->SUBCMD) }
 static const struct {
 	uint8_t insize;
 	uint8_t outsize;
 } lb_command_paramcount[] = {
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.dump),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.dump) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.off),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.off) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.on),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.on) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.init),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.init) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.brightness),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.brightness) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.seq),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.seq) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.reg),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.reg) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.rgb),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.rgb) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.get_seq),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.get_seq) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.demo),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.demo) },
+	LB_SIZES(dump),
+	LB_SIZES(off),
+	LB_SIZES(on),
+	LB_SIZES(init),
+	LB_SIZES(brightness),
+	LB_SIZES(seq),
+	LB_SIZES(reg),
+	LB_SIZES(rgb),
+	LB_SIZES(get_seq),
+	LB_SIZES(demo)
 };
+#undef LB_SIZES
 
 static int lb_help(const char *cmd)
 {
@@ -1087,83 +1082,88 @@ static uint8_t lb_find_msg_by_name(const char *str)
 }
 
 static int lb_do_cmd(enum lightbar_command cmd,
-		     struct ec_params_lightbar_cmd *ptr)
+		     struct ec_params_lightbar *in,
+		     struct ec_response_lightbar *out)
 {
 	int rv;
-	ptr->in.cmd = cmd;
+	in->cmd = cmd;
 	rv = ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-			ptr, lb_command_paramcount[cmd].insize,
-			ptr, lb_command_paramcount[cmd].outsize);
+			in, lb_command_paramcount[cmd].insize,
+			out, lb_command_paramcount[cmd].outsize);
 	return (rv < 0 ? rv : 0);
 }
 
-static void lb_show_msg_names(void)
+static int lb_show_msg_names(void)
 {
 	int i, current_state;
-	struct ec_params_lightbar_cmd param;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
 
-	(void)lb_do_cmd(LIGHTBAR_CMD_GET_SEQ, &param);
-	current_state = param.out.get_seq.num;
+	i = lb_do_cmd(LIGHTBAR_CMD_GET_SEQ, &param, &resp);
+	if (i < 0)
+		return i;
+	current_state = resp.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]);
+
+	return 0;
 }
 
 static int cmd_lightbar(int argc, char **argv)
 {
 	int i, r;
-	struct ec_params_lightbar_cmd param;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
 
 	if (1 == argc) {		/* no args = dump 'em all */
-		r = lb_do_cmd(LIGHTBAR_CMD_DUMP, &param);
+		r = lb_do_cmd(LIGHTBAR_CMD_DUMP, &param, &resp);
 		if (r)
 			return r;
-		for (i = 0; i < ARRAY_SIZE(param.out.dump.vals); i++) {
+		for (i = 0; i < ARRAY_SIZE(resp.dump.vals); i++) {
 			printf(" %02x     %02x     %02x\n",
-			       param.out.dump.vals[i].reg,
-			       param.out.dump.vals[i].ic0,
-			       param.out.dump.vals[i].ic1);
+			       resp.dump.vals[i].reg,
+			       resp.dump.vals[i].ic0,
+			       resp.dump.vals[i].ic1);
 		}
 		return 0;
 	}
 
 	if (argc == 2 && !strcasecmp(argv[1], "init"))
-		return lb_do_cmd(LIGHTBAR_CMD_INIT, &param);
+		return lb_do_cmd(LIGHTBAR_CMD_INIT, &param, &resp);
 
 	if (argc == 2 && !strcasecmp(argv[1], "off"))
-		return lb_do_cmd(LIGHTBAR_CMD_OFF, &param);
+		return lb_do_cmd(LIGHTBAR_CMD_OFF, &param, &resp);
 
 	if (argc == 2 && !strcasecmp(argv[1], "on"))
-		return lb_do_cmd(LIGHTBAR_CMD_ON, &param);
+		return lb_do_cmd(LIGHTBAR_CMD_ON, &param, &resp);
 
 	if (argc == 3 && !strcasecmp(argv[1], "brightness")) {
 		char *e;
-		param.in.brightness.num = 0xff & strtoul(argv[2], &e, 16);
-		return lb_do_cmd(LIGHTBAR_CMD_BRIGHTNESS, &param);
+		param.brightness.num = 0xff & strtoul(argv[2], &e, 16);
+		return lb_do_cmd(LIGHTBAR_CMD_BRIGHTNESS, &param, &resp);
 	}
 
 	if (argc == 3 && !strcasecmp(argv[1], "demo")) {
 		if (!strcasecmp(argv[2], "on") || argv[2][0] == '1')
-			param.in.demo.num = 1;
+			param.demo.num = 1;
 		else if (!strcasecmp(argv[2], "off") || argv[2][0] == '0')
-			param.in.demo.num = 0;
+			param.demo.num = 0;
 		else {
 			fprintf(stderr, "Invalid arg\n");
 			return -1;
 		}
-		return lb_do_cmd(LIGHTBAR_CMD_DEMO, &param);
+		return lb_do_cmd(LIGHTBAR_CMD_DEMO, &param, &resp);
 	}
 
 	if (argc >= 2 && !strcasecmp(argv[1], "seq")) {
 		char *e;
 		uint8_t num;
-		if (argc == 2) {
-			lb_show_msg_names();
-			return 0;
-		}
+		if (argc == 2)
+			return lb_show_msg_names();
 		num = 0xff & strtoul(argv[2], &e, 16);
 		if (e && *e)
 			num = lb_find_msg_by_name(argv[2]);
@@ -1171,25 +1171,25 @@ static int cmd_lightbar(int argc, char **argv)
 			fprintf(stderr, "Invalid arg\n");
 			return -1;
 		}
-		param.in.seq.num = num;
-		return lb_do_cmd(LIGHTBAR_CMD_SEQ, &param);
+		param.seq.num = num;
+		return lb_do_cmd(LIGHTBAR_CMD_SEQ, &param, &resp);
 	}
 
 	if (argc == 4) {
 		char *e;
-		param.in.reg.ctrl = 0xff & strtoul(argv[1], &e, 16);
-		param.in.reg.reg = 0xff & strtoul(argv[2], &e, 16);
-		param.in.reg.value = 0xff & strtoul(argv[3], &e, 16);
-		return lb_do_cmd(LIGHTBAR_CMD_REG, &param);
+		param.reg.ctrl = 0xff & strtoul(argv[1], &e, 16);
+		param.reg.reg = 0xff & strtoul(argv[2], &e, 16);
+		param.reg.value = 0xff & strtoul(argv[3], &e, 16);
+		return lb_do_cmd(LIGHTBAR_CMD_REG, &param, &resp);
 	}
 
 	if (argc == 5) {
 		char *e;
-		param.in.rgb.led = strtoul(argv[1], &e, 16);
-		param.in.rgb.red = strtoul(argv[2], &e, 16);
-		param.in.rgb.green = strtoul(argv[3], &e, 16);
-		param.in.rgb.blue = strtoul(argv[4], &e, 16);
-		return lb_do_cmd(LIGHTBAR_CMD_RGB, &param);
+		param.rgb.led = strtoul(argv[1], &e, 16);
+		param.rgb.red = strtoul(argv[2], &e, 16);
+		param.rgb.green = strtoul(argv[3], &e, 16);
+		param.rgb.blue = strtoul(argv[4], &e, 16);
+		return lb_do_cmd(LIGHTBAR_CMD_RGB, &param, &resp);
 	}
 
 	return lb_help(argv[0]);
diff --git a/util/lbplay.c b/util/lbplay.c
index 9acabaddfb..8658368160 100644
--- a/util/lbplay.c
+++ b/util/lbplay.c
@@ -18,40 +18,36 @@
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 
+#define LB_SIZES(SUBCMD) { \
+		sizeof(((struct ec_params_lightbar *)0)->SUBCMD) \
+		+ sizeof(((struct ec_params_lightbar *)0)->cmd), \
+		sizeof(((struct ec_response_lightbar *)0)->SUBCMD) }
 static const struct {
 	uint8_t insize;
 	uint8_t outsize;
 } lb_command_paramcount[] = {
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.dump),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.dump) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.off),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.off) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.on),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.on) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.init),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.init) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.brightness),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.brightness) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.seq),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.seq) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.reg),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.reg) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.rgb),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.rgb) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.get_seq),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.get_seq) },
-	{ sizeof(((struct ec_params_lightbar_cmd *)0)->in.demo),
-	  sizeof(((struct ec_params_lightbar_cmd *)0)->out.demo) },
+	LB_SIZES(dump),
+	LB_SIZES(off),
+	LB_SIZES(on),
+	LB_SIZES(init),
+	LB_SIZES(brightness),
+	LB_SIZES(seq),
+	LB_SIZES(reg),
+	LB_SIZES(rgb),
+	LB_SIZES(get_seq),
+	LB_SIZES(demo)
 };
+#undef LB_SIZES
 
 
 static void lb_cmd_noargs(enum lightbar_command cmd)
 {
-	struct ec_params_lightbar_cmd param;
-	param.in.cmd = cmd;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
+	param.cmd = cmd;
 	ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-		   &param, lb_command_paramcount[param.in.cmd].insize,
-		   &param, lb_command_paramcount[param.in.cmd].outsize);
+		   &param, lb_command_paramcount[param.cmd].insize,
+		   &resp, lb_command_paramcount[param.cmd].outsize);
 }
 
 inline void lightbar_off(void)
@@ -71,68 +67,73 @@ inline void lightbar_init_vals(void)
 
 void lightbar_brightness(int newval)
 {
-	struct ec_params_lightbar_cmd param;
-	param.in.cmd = LIGHTBAR_CMD_BRIGHTNESS;
-	param.in.brightness.num = newval;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
+	param.cmd = LIGHTBAR_CMD_BRIGHTNESS;
+	param.brightness.num = newval;
 	ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-		   &param, lb_command_paramcount[param.in.cmd].insize,
-		   &param, lb_command_paramcount[param.in.cmd].outsize);
+		   &param, lb_command_paramcount[param.cmd].insize,
+		   &resp, lb_command_paramcount[param.cmd].outsize);
 }
 
 void lightbar_sequence(enum lightbar_sequence num)
 {
-	struct ec_params_lightbar_cmd param;
-	param.in.cmd = LIGHTBAR_CMD_SEQ;
-	param.in.seq.num = num;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
+	param.cmd = LIGHTBAR_CMD_SEQ;
+	param.seq.num = num;
 	ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-		   &param, lb_command_paramcount[param.in.cmd].insize,
-		   &param, lb_command_paramcount[param.in.cmd].outsize);
+		   &param, lb_command_paramcount[param.cmd].insize,
+		   &resp, lb_command_paramcount[param.cmd].outsize);
 }
 
 void lightbar_reg(uint8_t ctrl, uint8_t reg, uint8_t val)
 {
-	struct ec_params_lightbar_cmd param;
-	param.in.cmd = LIGHTBAR_CMD_REG;
-	param.in.reg.ctrl = ctrl;
-	param.in.reg.reg = reg;
-	param.in.reg.value = val;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
+	param.cmd = LIGHTBAR_CMD_REG;
+	param.reg.ctrl = ctrl;
+	param.reg.reg = reg;
+	param.reg.value = val;
 	ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-		   &param, lb_command_paramcount[param.in.cmd].insize,
-		   &param, lb_command_paramcount[param.in.cmd].outsize);
+		   &param, lb_command_paramcount[param.cmd].insize,
+		   &resp, lb_command_paramcount[param.cmd].outsize);
 }
 
 void lightbar_rgb(int led, int red, int green, int blue)
 {
-	struct ec_params_lightbar_cmd param;
-	param.in.cmd = LIGHTBAR_CMD_RGB;
-	param.in.rgb.led = led;
-	param.in.rgb.red = red;
-	param.in.rgb.green = green;
-	param.in.rgb.blue = blue;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
+	param.cmd = LIGHTBAR_CMD_RGB;
+	param.rgb.led = led;
+	param.rgb.red = red;
+	param.rgb.green = green;
+	param.rgb.blue = blue;
 	ec_command(EC_CMD_LIGHTBAR_CMD, 0,
-		   &param, lb_command_paramcount[param.in.cmd].insize,
-		   &param, lb_command_paramcount[param.in.cmd].outsize);
+		   &param, lb_command_paramcount[param.cmd].insize,
+		   &resp, lb_command_paramcount[param.cmd].outsize);
 }
 
 void wait_for_ec_to_stop(void)
 {
 	int r;
-	struct ec_params_lightbar_cmd param;
+	struct ec_params_lightbar param;
+	struct ec_response_lightbar resp;
 	int count = 0;
 
 	do {
 		usleep(100000);
-		param.in.cmd = LIGHTBAR_CMD_GET_SEQ;
+		param.cmd = LIGHTBAR_CMD_GET_SEQ;
 		r = ec_command(EC_CMD_LIGHTBAR_CMD, 0,
 			       &param,
-			       lb_command_paramcount[param.in.cmd].insize,
-			       &param,
-			       lb_command_paramcount[param.in.cmd].outsize);
+			       lb_command_paramcount[param.cmd].insize,
+			       &resp,
+			       lb_command_paramcount[param.cmd].outsize);
 		if (count++ > 10) {
 			fprintf(stderr, "EC isn't responding\n");
 			exit(1);
 		}
-	} while (r < 0 && param.out.get_seq.num != LIGHTBAR_STOP);
+	} while (r < 0 && resp.get_seq.num != LIGHTBAR_STOP);
 }
 
 int main(int argc, char **argv)