Add AP userspace scripts to tweak lightbar colorsrelease-R23-2913.B

We have yet another tweak for the lightbar, but we don't want to update the
EC. This CL adds an init script that runs on the AP at every boot and pokes
the EC to modify the lightbar settings. We have to run it at every boot
because the EC will hibernate after the AP has been off (not suspended) for
an hour on battery power and will lose its settings.

There's a corresponding CL for the ec-utils ebuild that installs the
userspace scripts into the rootfs.

NOTE that in order to cherry-pick this into R23 from ToT, we have to update
ectool as well, because the original R23 ectool doesn't implement the
"lightbar params" command.

However, the EC image that ectool talks to is built from a *different*
branch that already has the ectool update, so this change DOESN'T backport
any of the EC firmware changes that would otherwise be required. Instead, we
just hack a little bit to make the EC firmware compile.

BUG=chrome-os-partner:16827
BRANCH=link
TEST=manual

Build the image for Link, install, reboot.

Run "ectool lightbar params". The output should match what's in
/usr/share/ec/lightbar_params.txt

Original-Change-Id: Ied2304da58d6d27b4f186e621bb187ba9a574e6c
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/41167
Commit-Queue: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Tested-by: Bill Richardson <wfrichar@chromium.org>
(cherry picked from commit 024f292edf44a9223d6101edc31571df067a8dce)

Change-Id: Iee3f67848c9da92a644083133a70431efabf42aa
Reviewed-on: https://gerrit.chromium.org/gerrit/41357
Commit-Queue: Bill Richardson <wfrichar@chromium.org>
Tested-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

7 files changed, 488 insertions, 153 deletions

diff --git a/board/link/userspace/README b/board/link/userspace/README
new file mode 100644
index 0000000000..204b9a4522
--- /dev/null
+++ b/board/link/userspace/README
@@ -0,0 +1,3 @@
+Sometimes we need to perform some run-time tweaks of the EC from the AP's
+userspace. This directory holds those tweaks. They're picked up by the
+ec-utils ebuild.
diff --git a/board/link/userspace/etc/init/ec.conf b/board/link/userspace/etc/init/ec.conf
new file mode 100644
index 0000000000..738cb8b2b8
--- /dev/null
+++ b/board/link/userspace/etc/init/ec.conf
@@ -0,0 +1,13 @@
+# Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+description    "Overrides lightbar params to tweak appearance"
+author         "chromium-os-dev@chromium.org"
+
+start on startup
+
+script
+  PARAMS=/usr/share/ec/lightbar_params.txt
+  [ -f "$PARAMS" ] && ectool lightbar params "$PARAMS" >/dev/null 2>&1
+end script
diff --git a/board/link/userspace/usr/share/ec/lightbar_params.txt b/board/link/userspace/usr/share/ec/lightbar_params.txt
new file mode 100644
index 0000000000..5561ede29c
--- /dev/null
+++ b/board/link/userspace/usr/share/ec/lightbar_params.txt
@@ -0,0 +1,31 @@
+2500            # .google_ramp_up
+10000           # .google_ramp_down
+2000            # .s3s0_ramp_up
+45000           # .s0_tick_delay (battery)
+30000           # .s0_tick_delay (AC)
+5000            # .s0a_tick_delay (battery)
+3000            # .s0a_tick_delay (AC)
+2000            # .s0s3_ramp_down
+5000000 # .s3_sleep_for
+2500            # .s3_ramp_up
+10000           # .s3_ramp_down
+1               # .new_s0
+0x60 0x60       # .osc_min (battery, AC)
+0xd0 0xd0       # .osc_max (battery, AC)
+24 24           # .w_ofs (battery, AC)
+0xcc 0xff       # .bright_bl_off_fixed (battery, AC)
+0xcc 0xff       # .bright_bl_on_min (battery, AC)
+0xcc 0xff       # .bright_bl_on_max (battery, AC)
+10 40 99                # .battery_threshold
+5 4 4 4         # .s0_idx[] (battery)
+4 4 4 4         # .s0_idx[] (AC)
+5 255 255 255   # .s3_idx[] (battery)
+255 255 255 255 # .s3_idx[] (AC)
+0x33 0x69 0xe8  # color[0]
+0xd5 0x0f 0x25  # color[1]
+0xee 0xb2 0x11  # color[2]
+0x00 0x99 0x25  # color[3]
+0x00 0x00 0xff  # color[4]
+0xff 0x00 0x00  # color[5]
+0xff 0xff 0x00  # color[6]
+0x00 0xff 0x00  # color[7]
diff --git a/common/lightbar.c b/common/lightbar.c
index cfb292249d..d0933aa27f 100644
--- a/common/lightbar.c
+++ b/common/lightbar.c
@@ -5,6 +5,9 @@
  * LED controls.
  */
 
+/* Danger, Will Robinson! Danger! See crosbug.com/p/16827 for details */
+#define USE_OLD_LIGHTBAR_STRUCTS_JUST_TO_BACKPORT_BUG_16827
+
 #include "battery.h"
 #include "battery_pack.h"
 #include "charge_state.h"
@@ -267,9 +270,6 @@ static uint32_t pending_msg;
 /****************************************************************************/
 /* Demo sequence */
 
-struct rgb_s {
-	uint8_t r, g, b;
-};
 enum {
 	COLOR_LOW, COLOR_MEDIUM, COLOR_HIGH, COLOR_FULL, COLOR_BLACK,
 };
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 7e2a99ea84..a1e1ffed58 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -614,58 +614,156 @@ struct ec_params_pwm_set_fan_duty {
 
 /*****************************************************************************/
 /*
- * Lightbar commands. This looks worse than it is. Since we only use one LPC
+ * Lightbar commands. This looks worse than it is. Since we only use one HOST
  * 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_CMD_LIGHTBAR_CMD 0x28
 
+
+#ifdef USE_OLD_LIGHTBAR_STRUCTS_JUST_TO_BACKPORT_BUG_16827
+
+struct rgb_s {
+       uint8_t r, g, b;
+};
+
 struct ec_params_lightbar_cmd {
+        union {
+                union {
+                        uint8_t cmd;  /* Command (see enum lightbar_command) */
+
+                        struct {
+                                uint8_t cmd;
+                        } dump, off, on, init, get_seq;
+
+                        struct num {
+                                uint8_t cmd;
+                                uint8_t num;
+                        } brightness, seq, demo;
+
+                        struct reg {
+                                uint8_t cmd;
+                                uint8_t ctrl, reg, value;
+                        } reg;
+
+                        struct rgb {
+                                uint8_t cmd;
+                                uint8_t led, red, green, blue;
+                        } rgb;
+                } in;
+
+                union {
+                        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, demo;
+                } out;
+        };
+} __packed;
+
+#else
+
+struct rgb_s {
+	uint8_t r, g, b;
+};
+
+#define LB_BATTERY_LEVELS 4
+/* List of tweakable parameters. NOTE: It's __packed so it can be sent in a
+ * host command, but the alignment is the same regardless. Keep it that way.
+ */
+struct lightbar_params {
+	/* Timing */
+	int google_ramp_up;
+	int google_ramp_down;
+	int s3s0_ramp_up;
+	int s0_tick_delay[2];			/* AC=0/1 */
+	int s0a_tick_delay[2];			/* AC=0/1 */
+	int s0s3_ramp_down;
+	int s3_sleep_for;
+	int s3_ramp_up;
+	int s3_ramp_down;
+
+	/* Oscillation */
+	uint8_t new_s0;
+	uint8_t osc_min[2];			/* AC=0/1 */
+	uint8_t osc_max[2];			/* AC=0/1 */
+	uint8_t w_ofs[2];			/* AC=0/1 */
+
+	/* Brightness limits based on the backlight and AC. */
+	uint8_t bright_bl_off_fixed[2];		/* AC=0/1 */
+	uint8_t bright_bl_on_min[2];		/* AC=0/1 */
+	uint8_t bright_bl_on_max[2];		/* AC=0/1 */
+
+	/* Battery level thresholds */
+	uint8_t battery_threshold[LB_BATTERY_LEVELS - 1];
+
+	/* Map [AC][battery_level] to color index */
+	uint8_t s0_idx[2][LB_BATTERY_LEVELS];	/* AP is running */
+	uint8_t s3_idx[2][LB_BATTERY_LEVELS];	/* AP is sleeping */
+
+	/* Color palette */
+	struct rgb_s color[8];			/* 0-3 are Google colors */
+} __packed;
+
+struct ec_params_lightbar {
+	uint8_t cmd;		      /* Command (see enum lightbar_command) */
 	union {
-		union {
-			uint8_t cmd;  /* Command (see enum lightbar_command) */
+		struct {
+			/* no args */
+		} dump, off, on, init, get_seq, get_params;
 
-			struct {
-				uint8_t cmd;
-			} dump, off, on, init, get_seq;
-
-			struct num {
-				uint8_t cmd;
-				uint8_t num;
-			} brightness, seq, demo;
-
-			struct reg {
-				uint8_t cmd;
-				uint8_t ctrl, reg, value;
-			} reg;
-
-			struct rgb {
-				uint8_t cmd;
-				uint8_t led, red, green, blue;
-			} rgb;
-		} in;
-
-		union {
-			struct dump {
-				struct {
-					uint8_t reg;
-					uint8_t ic0;
-					uint8_t ic1;
-				} vals[23];
-			} dump;
-
-			struct get_seq {
-				uint8_t num;
-			} get_seq;
+		struct num {
+			uint8_t num;
+		} brightness, seq, demo;
+
+		struct reg {
+			uint8_t ctrl, reg, value;
+		} reg;
+
+		struct rgb {
+			uint8_t led, red, green, blue;
+		} rgb;
+
+		struct lightbar_params set_params;
+	};
+} __packed;
 
+struct ec_response_lightbar {
+	union {
+		struct dump {
 			struct {
-				/* no return params */
-			} off, on, init, brightness, seq, reg, rgb, demo;
-		} out;
+				uint8_t reg;
+				uint8_t ic0;
+				uint8_t ic1;
+			} vals[23];
+		} dump;
+
+		struct get_seq {
+			uint8_t num;
+		} get_seq;
+
+		struct lightbar_params get_params;
+
+		struct {
+			/* no return params */
+		} off, on, init, brightness, seq, reg, rgb, demo, set_params;
 	};
 } __packed;
 
+#endif /* USE_OLD_LIGHTBAR_STRUCTS_JUST_TO_BACKPORT_BUG_16827 */
+
 /* Lightbar commands */
 enum lightbar_command {
 	LIGHTBAR_CMD_DUMP = 0,
@@ -678,6 +776,8 @@ enum lightbar_command {
 	LIGHTBAR_CMD_RGB = 7,
 	LIGHTBAR_CMD_GET_SEQ = 8,
 	LIGHTBAR_CMD_DEMO = 9,
+	LIGHTBAR_CMD_GET_PARAMS = 10,
+	LIGHTBAR_CMD_SET_PARAMS = 11,
 	LIGHTBAR_NUM_CMDS
 };
 
diff --git a/util/ectool.c b/util/ectool.c
index b7d1ed89fa..ca1416048e 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -4,6 +4,7 @@
  */
 
 #include <ctype.h>
+#include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -1042,31 +1043,28 @@ 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),
+	LB_SIZES(get_params),
+	LB_SIZES(set_params),
 };
+#undef LB_SIZES
 
 static int lb_help(const char *cmd)
 {
@@ -1082,6 +1080,8 @@ static int lb_help(const char *cmd)
 	printf("  %s LED RED GREEN BLUE    - set color manually"
 		 " (LED=4 for all)\n", cmd);
 	printf("  %s demo 0|1              - turn demo mode on & off\n", cmd);
+	printf("  %s params [setfile]      - get params"
+	       " (or set from file)\n", cmd);
 	return 0;
 }
 
@@ -1096,83 +1096,268 @@ 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 lb_read_params_from_file(const char *filename,
+				    struct lightbar_params *p)
+{
+	FILE *fp;
+	char buf[80];
+	int val[4];
+	int r = 1;
+	int line = 0;
+	int want, got;
+	int i;
+
+	fp = fopen(filename, "rb");
+	if (!fp) {
+		fprintf(stderr, "Can't open %s: %s\n",
+			filename, strerror(errno));
+		return 1;
+	}
+
+	/* We must read the correct number of params from each line */
+#define READ(N) do {							\
+		line++;							\
+		want = (N);						\
+		got = -1;						\
+		if (!fgets(buf, sizeof(buf), fp))			\
+			goto done;					\
+		got = sscanf(buf, "%i %i %i %i",			\
+			     &val[0], &val[1], &val[2], &val[3]);	\
+		if (want != got)					\
+			goto done;					\
+	} while (0)
+
+
+	/* Do it */
+	READ(1); p->google_ramp_up = val[0];
+	READ(1); p->google_ramp_down = val[0];
+	READ(1); p->s3s0_ramp_up = val[0];
+	READ(1); p->s0_tick_delay[0] = val[0];
+	READ(1); p->s0_tick_delay[1] = val[0];
+	READ(1); p->s0a_tick_delay[0] = val[0];
+	READ(1); p->s0a_tick_delay[1] = val[0];
+	READ(1); p->s0s3_ramp_down = val[0];
+	READ(1); p->s3_sleep_for = val[0];
+	READ(1); p->s3_ramp_up = val[0];
+	READ(1); p->s3_ramp_down = val[0];
+	READ(1); p->new_s0 = val[0];
+
+	READ(2);
+	p->osc_min[0] = val[0];
+	p->osc_min[1] = val[1];
+	READ(2);
+	p->osc_max[0] = val[0];
+	p->osc_max[1] = val[1];
+	READ(2);
+	p->w_ofs[0] = val[0];
+	p->w_ofs[1] = val[1];
+
+	READ(2);
+	p->bright_bl_off_fixed[0] = val[0];
+	p->bright_bl_off_fixed[1] = val[1];
+
+	READ(2);
+	p->bright_bl_on_min[0] = val[0];
+	p->bright_bl_on_min[1] = val[1];
+
+	READ(2);
+	p->bright_bl_on_max[0] = val[0];
+	p->bright_bl_on_max[1] = val[1];
+
+	READ(3);
+	p->battery_threshold[0] = val[0];
+	p->battery_threshold[1] = val[1];
+	p->battery_threshold[2] = val[2];
+
+	READ(4);
+	p->s0_idx[0][0] = val[0];
+	p->s0_idx[0][1] = val[1];
+	p->s0_idx[0][2] = val[2];
+	p->s0_idx[0][3] = val[3];
+
+	READ(4);
+	p->s0_idx[1][0] = val[0];
+	p->s0_idx[1][1] = val[1];
+	p->s0_idx[1][2] = val[2];
+	p->s0_idx[1][3] = val[3];
+
+	READ(4);
+	p->s3_idx[0][0] = val[0];
+	p->s3_idx[0][1] = val[1];
+	p->s3_idx[0][2] = val[2];
+	p->s3_idx[0][3] = val[3];
+
+	READ(4);
+	p->s3_idx[1][0] = val[0];
+	p->s3_idx[1][1] = val[1];
+	p->s3_idx[1][2] = val[2];
+	p->s3_idx[1][3] = val[3];
+
+	for (i = 0; i < ARRAY_SIZE(p->color); i++) {
+		READ(3);
+		p->color[i].r = val[0];
+		p->color[i].g = val[1];
+		p->color[i].b = val[2];
+	}
+
+	/* Yay */
+	r = 0;
+done:
+	if (r)
+		fprintf(stderr, "problem with line %d: wanted %d, got %d\n",
+			line, want, got);
+	fclose(fp);
+	return r;
+}
+
+static void lb_show_params(const struct lightbar_params *p)
+{
+	int i;
+
+	printf("%d\t\t# .google_ramp_up\n", p->google_ramp_up);
+	printf("%d\t\t# .google_ramp_down\n", p->google_ramp_down);
+	printf("%d\t\t# .s3s0_ramp_up\n", p->s3s0_ramp_up);
+	printf("%d\t\t# .s0_tick_delay (battery)\n", p->s0_tick_delay[0]);
+	printf("%d\t\t# .s0_tick_delay (AC)\n", p->s0_tick_delay[1]);
+	printf("%d\t\t# .s0a_tick_delay (battery)\n", p->s0a_tick_delay[0]);
+	printf("%d\t\t# .s0a_tick_delay (AC)\n", p->s0a_tick_delay[1]);
+	printf("%d\t\t# .s0s3_ramp_down\n", p->s0s3_ramp_down);
+	printf("%d\t# .s3_sleep_for\n", p->s3_sleep_for);
+	printf("%d\t\t# .s3_ramp_up\n", p->s3_ramp_up);
+	printf("%d\t\t# .s3_ramp_down\n", p->s3_ramp_down);
+	printf("%d\t\t# .new_s0\n", p->new_s0);
+	printf("0x%02x 0x%02x\t# .osc_min (battery, AC)\n",
+	       p->osc_min[0], p->osc_min[1]);
+	printf("0x%02x 0x%02x\t# .osc_max (battery, AC)\n",
+	       p->osc_max[0], p->osc_max[1]);
+	printf("%d %d\t\t# .w_ofs (battery, AC)\n",
+	       p->w_ofs[0], p->w_ofs[1]);
+	printf("0x%02x 0x%02x\t# .bright_bl_off_fixed (battery, AC)\n",
+	       p->bright_bl_off_fixed[0], p->bright_bl_off_fixed[1]);
+	printf("0x%02x 0x%02x\t# .bright_bl_on_min (battery, AC)\n",
+	       p->bright_bl_on_min[0], p->bright_bl_on_min[1]);
+	printf("0x%02x 0x%02x\t# .bright_bl_on_max (battery, AC)\n",
+	       p->bright_bl_on_max[0], p->bright_bl_on_max[1]);
+	printf("%d %d %d\t\t# .battery_threshold\n",
+	       p->battery_threshold[0],
+	       p->battery_threshold[1],
+	       p->battery_threshold[2]);
+	printf("%d %d %d %d\t\t# .s0_idx[] (battery)\n",
+	       p->s0_idx[0][0], p->s0_idx[0][1],
+	       p->s0_idx[0][2], p->s0_idx[0][3]);
+	printf("%d %d %d %d\t\t# .s0_idx[] (AC)\n",
+	       p->s0_idx[1][0], p->s0_idx[1][1],
+	       p->s0_idx[1][2], p->s0_idx[1][3]);
+	printf("%d %d %d %d\t# .s3_idx[] (battery)\n",
+	       p->s3_idx[0][0], p->s3_idx[0][1],
+	       p->s3_idx[0][2], p->s3_idx[0][3]);
+	printf("%d %d %d %d\t# .s3_idx[] (AC)\n",
+	       p->s3_idx[1][0], p->s3_idx[1][1],
+	       p->s3_idx[1][2], p->s3_idx[1][3]);
+	for (i = 0; i < ARRAY_SIZE(p->color); i++)
+		printf("0x%02x 0x%02x 0x%02x\t# color[%d]\n",
+		       p->color[i].r,
+		       p->color[i].g,
+		       p->color[i].b, i);
 }
 
 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 (!strcasecmp(argv[1], "params")) {
+		if (argc > 2) {
+			r = lb_read_params_from_file(argv[2],
+						     &param.set_params);
+			if (r)
+				return r;
+			return lb_do_cmd(LIGHTBAR_CMD_SET_PARAMS,
+					 &param, &resp);
+		}
+		r = lb_do_cmd(LIGHTBAR_CMD_GET_PARAMS, &param, &resp);
+		if (!r)
+			lb_show_params(&resp.get_params);
+		return r;
+	}
 
 	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]);
@@ -1180,25 +1365,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..907374e9cc 100644
--- a/util/lbplay.c
+++ b/util/lbplay.c
@@ -18,40 +18,38 @@
 #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),
+	LB_SIZES(get_params),
+	LB_SIZES(set_params)
 };
+#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 +69,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)