Add host command to control charge state v2

This replaces the obsolete and temporary (ha!) EC_CMD_CHARGE_DUMP host
command with EC_CMD_CHARGE_STATE. This is used to monitor and adjust the new
charge state implementation, including any board-specific customizations.

This command is a single catch-all command with multiple subcommands
(similar to EC_CMD_LIGHTBAR_CMD) so that we don't have to keep adding new
top-level host commands just to support incremental changes.

BUG=chrome-os-partner:23776
BRANCH=ToT
TEST=manual

From the AP, try these commands:

  ectool chargestate show
  ectool chargestate param
  ectool chargestate param <NUM>
  ectool chargestate param <NUM> <VALUE>

Watch the EC console and use its "chg" command to verify the effects of
setting various params.

Note: the Samus-specific fast-charging profile override is param 0x10000.
You can check it with the EC console "fastcharge" command.

Change-Id: Iad2f773a085bc25c05073b3eed9866f122ae9d78
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/193305
Reviewed-by: Randall Spangler <rspangler@chromium.org>

common/charge_state_v1.c

@@ -979,23 +979,6 @@ static int charge_command_charge_control(struct host_cmd_handler_args *args)
 DECLARE_HOST_COMMAND(EC_CMD_CHARGE_CONTROL, charge_command_charge_control,
 		     EC_VER_MASK(0) | EC_VER_MASK(1));
 
-static int charge_command_dump(struct host_cmd_handler_args *args)
-{
-	char *dest = (char *)args->response;
-
-	if (system_is_locked())
-		return EC_RES_ACCESS_DENIED;
-
-	ASSERT(sizeof(task_ctx) <= args->response_max);
-
-	memcpy(dest, &task_ctx, sizeof(task_ctx));
-	args->response_size = sizeof(task_ctx);
-
-	return EC_RES_SUCCESS;
-}
-DECLARE_HOST_COMMAND(EC_CMD_CHARGE_DUMP, charge_command_dump,
-		     EC_VER_MASK(0));
-
 static void reset_current_limit(void)
 {
 	user_current_limit = -1;

common/charge_state_v2.c

@@ -271,11 +271,11 @@ static void show_charging_progress(void)
 
 /*
  * Ask the charger for some voltage and current. If either value is 0,
- * charging is disabled; otherwise it's enabled.
+ * charging is disabled; otherwise it's enabled. Negative values are ignored.
  */
 static int charge_request(int voltage, int current)
 {
-	int r1, r2;
+	int r1 = EC_SUCCESS, r2 = EC_SUCCESS;
 
 	/* TODO(crosbug.com/p/27640): should we call charger_set_mode() too? */
 	if (!voltage || !current)
@@ -283,11 +283,13 @@ static int charge_request(int voltage, int current)
 
 	CPRINTF("[%T %s(%dmV, %dmA)]\n", __func__, voltage, current);
 
-	r1 = charger_set_voltage(voltage);
+	if (voltage > 0)
+		r1 = charger_set_voltage(voltage);
 	if (r1 != EC_SUCCESS)
 		problem(PR_SET_VOLTAGE, r1);
 
-	r2 = charger_set_current(current);
+	if (current > 0)
+		r2 = charger_set_current(current);
 	if (r2 != EC_SUCCESS)
 		problem(PR_SET_CURRENT, r2);
 
@@ -743,6 +745,108 @@ static int charge_command_current_limit(struct host_cmd_handler_args *args)
 DECLARE_HOST_COMMAND(EC_CMD_CHARGE_CURRENT_LIMIT, charge_command_current_limit,
 		     EC_VER_MASK(0));
 
+static int charge_command_charge_state(struct host_cmd_handler_args *args)
+{
+	const struct ec_params_charge_state *in = args->params;
+	struct ec_response_charge_state *out = args->response;
+	uint32_t val;
+	int rv = EC_RES_SUCCESS;
+
+	switch (in->cmd) {
+
+	case CHARGE_STATE_CMD_GET_STATE:
+		out->get_state.ac = curr.ac;
+		out->get_state.chg_voltage = curr.chg.voltage;
+		out->get_state.chg_current = curr.chg.current;
+		out->get_state.chg_input_current = curr.chg.input_current;
+		out->get_state.batt_state_of_charge = curr.batt.state_of_charge;
+		args->response_size = sizeof(out->get_state);
+		break;
+
+	case CHARGE_STATE_CMD_GET_PARAM:
+		val = 0;
+#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
+		/* custom profile params */
+		if (in->get_param.param >= CS_PARAM_CUSTOM_PROFILE_MIN &&
+		    in->get_param.param <= CS_PARAM_CUSTOM_PROFILE_MAX) {
+			rv  = charger_profile_override_get_param(
+				in->get_param.param, &val);
+		} else
+#endif
+			/* standard params */
+			switch (in->get_param.param) {
+			case CS_PARAM_CHG_VOLTAGE:
+				val = curr.chg.voltage;
+				break;
+			case CS_PARAM_CHG_CURRENT:
+				val = curr.chg.current;
+				break;
+			case CS_PARAM_CHG_INPUT_CURRENT:
+				val = curr.chg.input_current;
+				break;
+			case CS_PARAM_CHG_STATUS:
+				val = curr.chg.status;
+				break;
+			case CS_PARAM_CHG_OPTION:
+				val = curr.chg.option;
+				break;
+			default:
+				rv = EC_RES_INVALID_PARAM;
+			}
+
+		/* got something */
+		out->get_param.value = val;
+		args->response_size = sizeof(out->get_param);
+		break;
+
+	case CHARGE_STATE_CMD_SET_PARAM:
+		val = in->set_param.value;
+#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
+		/* custom profile params */
+		if (in->set_param.param >= CS_PARAM_CUSTOM_PROFILE_MIN &&
+		    in->set_param.param <= CS_PARAM_CUSTOM_PROFILE_MAX) {
+			rv  = charger_profile_override_set_param(
+				in->set_param.param, val);
+		} else
+#endif
+			switch (in->set_param.param) {
+			case CS_PARAM_CHG_VOLTAGE:
+				if (charge_request(val, -1))
+					rv = EC_RES_ERROR;
+				break;
+			case CS_PARAM_CHG_CURRENT:
+				if (charge_request(-1, val))
+					rv = EC_RES_ERROR;
+				break;
+			case CS_PARAM_CHG_INPUT_CURRENT:
+				if (charger_set_input_current(val))
+					rv = EC_RES_ERROR;
+				break;
+			case CS_PARAM_CHG_STATUS:
+				/* Can't set this */
+				rv = EC_RES_ACCESS_DENIED;
+				break;
+			case CS_PARAM_CHG_OPTION:
+				if (charger_set_option(val))
+					rv = EC_RES_ERROR;
+				break;
+			default:
+				rv = EC_RES_INVALID_PARAM;
+
+			}
+		break;
+
+	default:
+		CPRINTF("[%T EC_CMD_CHARGE_STATE: bad cmd 0x%x]\n", in->cmd);
+		rv = EC_RES_INVALID_PARAM;
+	}
+
+	return rv;
+}
+
+DECLARE_HOST_COMMAND(EC_CMD_CHARGE_STATE, charge_command_charge_state,
+		     EC_VER_MASK(0));
+
 /*****************************************************************************/
 /* Console commands */
 

driver/battery/samus.c

@@ -7,6 +7,7 @@
 
 #include "charge_state.h"
 #include "console.h"
+#include "ec_commands.h"
 #include "util.h"
 
 static const struct battery_info info = {
@@ -112,6 +113,29 @@ int charger_profile_override(struct charge_state_data *curr)
 	return 0;
 }
 
+/* Customs options controllable by host command. */
+#define PARAM_FASTCHARGE (CS_PARAM_CUSTOM_PROFILE_MIN + 0)
+
+enum ec_status charger_profile_override_get_param(uint32_t param,
+						  uint32_t *value)
+{
+	if (param == PARAM_FASTCHARGE) {
+		*value = fast_charging_allowed;
+		return EC_RES_SUCCESS;
+	}
+	return EC_RES_INVALID_PARAM;
+}
+
+enum ec_status charger_profile_override_set_param(uint32_t param,
+						  uint32_t value)
+{
+	if (param == PARAM_FASTCHARGE) {
+		fast_charging_allowed = value;
+		return EC_RES_SUCCESS;
+	}
+	return EC_RES_INVALID_PARAM;
+}
+
 static int command_fastcharge(int argc, char **argv)
 {
 	if (argc > 1 && !parse_bool(argv[1], &fast_charging_allowed))

include/charge_state_v2.h

@@ -48,5 +48,14 @@ struct charge_state_data {
  */
 int charger_profile_override(struct charge_state_data *);
 
+/*
+ * Access to custom profile params through host commands.
+ * What this does is up to the implementation.
+ */
+enum ec_status charger_profile_override_get_param(uint32_t param,
+						  uint32_t *value);
+enum ec_status charger_profile_override_set_param(uint32_t param,
+						  uint32_t value);
+
 #endif /* __CROS_EC_CHARGE_STATE_V2_H */
 

include/ec_commands.h

@@ -1827,14 +1827,79 @@ struct ec_params_hang_detect {
 } __packed;
 
 /*****************************************************************************/
-/* Debug commands for battery charging */
+/* Commands for battery charging */
 
 /*
- * Dump charge state machine context.
- *
- * Response is a binary dump of charge state machine context.
+ * This is the single catch-all host command to exchange data regarding the
+ * charge state machine (v2 and up).
  */
-#define EC_CMD_CHARGE_DUMP 0xa0
+#define EC_CMD_CHARGE_STATE 0xa0
+
+/* Subcommands for this host command */
+enum charge_state_command {
+	CHARGE_STATE_CMD_GET_STATE,
+	CHARGE_STATE_CMD_GET_PARAM,
+	CHARGE_STATE_CMD_SET_PARAM,
+	CHARGE_STATE_NUM_CMDS
+};
+
+/*
+ * Known param numbers are defined here. Ranges are reserved for board-specific
+ * params, which are handled by the particular implementations.
+ */
+enum charge_state_params {
+	CS_PARAM_CHG_VOLTAGE,	      /* charger voltage limit */
+	CS_PARAM_CHG_CURRENT,	      /* charger current limit */
+	CS_PARAM_CHG_INPUT_CURRENT,   /* charger input current limit */
+	CS_PARAM_CHG_STATUS,	      /* charger-specific status */
+	CS_PARAM_CHG_OPTION,	      /* charger-specific options */
+	/* How many so far? */
+	CS_NUM_BASE_PARAMS,
+
+	/* Range for CONFIG_CHARGER_PROFILE_OVERRIDE params */
+	CS_PARAM_CUSTOM_PROFILE_MIN = 0x10000,
+	CS_PARAM_CUSTOM_PROFILE_MAX = 0x1ffff,
+
+	/* Other custom param ranges go here... */
+};
+
+struct ec_params_charge_state {
+	uint8_t cmd;				/* enum charge_state_command */
+	union {
+		struct {
+			/* no args */
+		} get_state;
+
+		struct {
+			uint32_t param;		/* enum charge_state_param */
+		} get_param;
+
+		struct {
+			uint32_t param;		/* param to set */
+			uint32_t value;		/* value to set */
+		} set_param;
+	};
+} __packed;
+
+struct ec_response_charge_state {
+	union {
+		struct {
+			int ac;
+			int chg_voltage;
+			int chg_current;
+			int chg_input_current;
+			int batt_state_of_charge;
+		} get_state;
+
+		struct {
+			uint32_t value;
+		} get_param;
+		struct {
+			/* no return values */
+		} set_param;
+	};
+} __packed;
+
 
 /*
  * Set maximum battery charging current.

util/ectool.c

@@ -40,10 +40,10 @@ const char help_str[] =
 	"      Prints the board version\n"
 	"  chargecurrentlimit\n"
 	"      Set the maximum battery charging current\n"
-	"  chargedump\n"
-	"      Dump the context of charge state machine\n"
 	"  chargecontrol\n"
 	"      Force the battery to stop charging or discharge\n"
+	"  chargestate\n"
+	"      Handle commands related to charge state v2 (and later)\n"
 	"  chipinfo\n"
 	"      Prints chip info\n"
 	"  cmdversions <cmd>\n"
@@ -2757,26 +2757,124 @@ int cmd_charge_control(int argc, char *argv[])
 }
 
 
-int cmd_charge_dump(int argc, char *argv[])
+
+/* Table of subcommand sizes for EC_CMD_CHARGE_STATE */
+#define CB_SIZES(SUBCMD) { \
+		sizeof(((struct ec_params_charge_state *)0)->SUBCMD) \
+		+ sizeof(((struct ec_params_charge_state *)0)->cmd), \
+		sizeof(((struct ec_response_charge_state *)0)->SUBCMD) }
+static const struct {
+	uint8_t to_ec_size;
+	uint8_t from_ec_size;
+} cs_paramcount[] = {
+	/* Order must match enum charge_state_command */
+	CB_SIZES(get_state),
+	CB_SIZES(get_param),
+	CB_SIZES(set_param),
+};
+#undef CB_SIZES
+BUILD_ASSERT(ARRAY_SIZE(cs_paramcount) == CHARGE_STATE_NUM_CMDS);
+
+static int cs_do_cmd(struct ec_params_charge_state *to_ec,
+		     struct ec_response_charge_state *from_ec)
 {
-	unsigned char *out = ec_inbuf;
-	int rv, i;
+	int rv;
+	int cmd = to_ec->cmd;
 
-	rv = ec_command(EC_CMD_CHARGE_DUMP, 0, NULL, 0,
-			ec_inbuf, ec_max_insize);
+	rv = ec_command(EC_CMD_CHARGE_STATE, 0,
+			to_ec, cs_paramcount[cmd].to_ec_size,
+			from_ec, cs_paramcount[cmd].from_ec_size);
 
-	if (rv < 0)
-		return rv;
-
-	for (i = 0; i < rv; ++i) {
-		printf("%02X", out[i]);
-		if ((i & 31) == 31)
-			printf("\n");
-	}
-	printf("\n");
-	return 0;
+	return (rv < 0 ? 1 : 0);
 }
 
+static const char * const base_params[] = {
+	"chg_voltage",
+	"chg_current",
+	"chg_input_current",
+	"chg_status",
+	"chg_option",
+};
+BUILD_ASSERT(ARRAY_SIZE(base_params) == CS_NUM_BASE_PARAMS);
+
+static int cmd_charge_state(int argc, char **argv)
+{
+	struct ec_params_charge_state param;
+	struct ec_response_charge_state resp;
+	uint32_t p, v;
+	int i, r;
+	char *e;
+
+	if (argc > 1 && !strcasecmp(argv[1], "show")) {
+		param.cmd = CHARGE_STATE_CMD_GET_STATE;
+		r = cs_do_cmd(&param, &resp);
+		if (r)
+			return r;
+		printf("ac = %d\n", resp.get_state.ac);
+		printf("chg_voltage = %dmV\n", resp.get_state.chg_voltage);
+		printf("chg_current = %dmA\n", resp.get_state.chg_current);
+		printf("chg_input_current = %dmA\n",
+		       resp.get_state.chg_input_current);
+		printf("batt_state_of_charge = %d%%\n",
+		       resp.get_state.batt_state_of_charge);
+		return 0;
+	}
+
+	if (argc > 1 && !strcasecmp(argv[1], "param")) {
+		switch (argc) {
+		case 3:
+			if (!strcasecmp(argv[2], "help"))
+				break;
+			param.cmd = CHARGE_STATE_CMD_GET_PARAM;
+			p = strtoul(argv[2], &e, 0);
+			if (e && *e) {
+				fprintf(stderr, "Bad param: %s\n", argv[2]);
+				return -1;
+			}
+			param.get_param.param = p;
+			r = cs_do_cmd(&param, &resp);
+			if (r)
+				return r;
+			v = resp.get_param.value;
+			if (p < CS_NUM_BASE_PARAMS)
+				printf("%d (0x%x)       # %s\n", v, v,
+				       base_params[p]);
+			else
+				printf("%d (0x%x)\n", v, v);
+			return 0;
+		case 4:
+			param.cmd = CHARGE_STATE_CMD_SET_PARAM;
+			p = strtoul(argv[2], &e, 0);
+			if (e && *e) {
+				fprintf(stderr, "Bad param: %s\n", argv[2]);
+				return -1;
+			}
+			v = strtoul(argv[3], &e, 0);
+			if (e && *e) {
+				fprintf(stderr, "Bad value: %s\n", argv[3]);
+				return -1;
+			}
+			param.set_param.param = p;
+			param.set_param.value = v;
+			return cs_do_cmd(&param, &resp);
+		}
+
+		printf("base params:\n");
+		for (i = 0; i < CS_NUM_BASE_PARAMS; i++)
+			printf("  %d   %s\n", i, base_params[i]);
+		printf("custom profile params:\n");
+		printf("  0x%x - 0x%x\n", CS_PARAM_CUSTOM_PROFILE_MIN,
+		       CS_PARAM_CUSTOM_PROFILE_MAX);
+
+		return 0;
+	}
+
+	printf("Usage:\n");
+	printf("  %s show                  - show current state\n", argv[0]);
+	printf("  %s param NUM [VALUE]     - get/set param NUM\n", argv[0]);
+	printf("  %s param help            - show known param NUMs\n", argv[0]);
+	return 0;
+}
 
 int cmd_gpio_get(int argc, char *argv[])
 {
@@ -3549,8 +3647,8 @@ const struct command commands[] = {
 	{"batterycutoff", cmd_battery_cut_off},
 	{"boardversion", cmd_board_version},
 	{"chargecurrentlimit", cmd_charge_current_limit},
-	{"chargedump", cmd_charge_dump},
 	{"chargecontrol", cmd_charge_control},
+	{"chargestate", cmd_charge_state},
 	{"chipinfo", cmd_chipinfo},
 	{"cmdversions", cmd_cmdversions},
 	{"console", cmd_console},