Update EC_CMD_CHARGE_CONTROL to version 2

Version 2 of EC_CMD_CHARGE_CONTROL can control battery sustainer.
It allows the host to set the upper and lower thresholds between
which the EC tries to keep the battery state of charge.

Version 2 of EC_CMD_CHARGE_CONTROL also supports 'GET' request.
It allows the host to query the current charge control settings.

localhost ~ # ectool chargecontrol
Charge mode = NORMAL (0)
Battery sustainer = off (-1% ~ -1%)

localhost ~ # ectool chargecontrol normal 66 66
Charge state machine is in normal mode with sustainer enabled.

localhost ~ # ectool chargecontrol
Charge mode = NORMAL (0)
Battery sustainer = on (66% ~ 66%)

localhost ~ # ectool chargecontrol normal
Charge state machine is in normal mode.

BUG=b:188457962
BRANCH=none
TEST=Atlas. See above.

Change-Id: I81ec62172b4f159c46334fc0f940a2adae3f2b8a
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2929340
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

Author: dnojiri

common/charge_state_v2.c

@@ -205,6 +205,39 @@ static void problem(enum problem_type p, int v)
 }
 
 #ifdef CONFIG_EC_EC_COMM_BATTERY_MASTER
+static int battery_sustainer_set(int8_t lower, int8_t upper)
+{
+	if (lower == -1 || upper == -1) {
+		CPRINTS("Sustain mode disabled");
+		sustain_soc.lower = -1;
+		sustain_soc.upper = -1;
+		return EC_SUCCESS;
+	}
+
+	if (lower <= upper && 0 <= lower && upper <= 100) {
+		/* Currently sustainer requires discharge_on_ac. */
+		if (!IS_ENABLED(CONFIG_CHARGER_DISCHARGE_ON_AC))
+			return EC_RES_UNAVAILABLE;
+		sustain_soc.lower = lower;
+		sustain_soc.upper = upper;
+		return EC_SUCCESS;
+	}
+
+	CPRINTS("Invalid param: %s(%d, %d)", __func__, lower, upper);
+	return EC_ERROR_INVAL;
+}
+
+static void battery_sustainer_disable(void)
+{
+	battery_sustainer_set(-1, -1);
+}
+
+static bool battery_sustainer_enabled(void)
+{
+	return sustain_soc.lower != -1 && sustain_soc.upper != -1;
+}
+
+#ifdef CONFIG_EC_EC_COMM_BATTERY_CLIENT
 /*
  * Parameters for dual-battery policy.
  * TODO(b:71881017): This should be made configurable by AP in the future.
@@ -1156,7 +1189,8 @@ static void dump_charge_state(void)
 		 battery_seems_to_be_disconnected);
 	ccprintf("battery_was_removed = %d\n", battery_was_removed);
 	ccprintf("debug output = %s\n", debugging ? "on" : "off");
-	ccprintf("sustain charge = %d%% ~ %d%%\n",
+	ccprintf("Battery sustainer = %s (%d%% ~ %d%%)\n",
+		 battery_sustainer_enabled() ? "on" : "off",
 		 sustain_soc.lower, sustain_soc.upper);
 #undef DUMP
 }
@@ -2658,8 +2692,33 @@ static enum ec_status
 charge_command_charge_control(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_charge_control *p = args->params;
+	struct ec_response_charge_control *r = args->response;
 	int rv;
 
+	if (args->version >= 2) {
+		if (p->cmd == EC_CHARGE_CONTROL_CMD_SET) {
+			if (chg_ctl_mode == CHARGE_CONTROL_NORMAL) {
+				rv = battery_sustainer_set(
+						p->sustain_soc.lower,
+						p->sustain_soc.upper);
+				if (rv == EC_RES_UNAVAILABLE)
+					return EC_RES_UNAVAILABLE;
+				if (rv)
+					return EC_RES_INVALID_PARAM;
+			} else {
+				battery_sustainer_disable();
+			}
+		} else if (p->cmd == EC_CHARGE_CONTROL_CMD_GET) {
+			r->mode = chg_ctl_mode;
+			r->sustain_soc.lower = sustain_soc.lower;
+			r->sustain_soc.upper = sustain_soc.upper;
+			args->response_size = sizeof(*r);
+			return EC_RES_SUCCESS;
+		} else {
+			return EC_RES_INVALID_PARAM;
+		}
+	}
+
 	rv = set_chg_ctrl_mode(p->mode);
 	if (rv != EC_SUCCESS)
 		return EC_RES_ERROR;
@@ -2693,7 +2752,7 @@ charge_command_charge_control(struct host_cmd_handler_args *args)
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_CHARGE_CONTROL, charge_command_charge_control,
-		     EC_VER_MASK(1));
+		     EC_VER_MASK(1) | EC_VER_MASK(2));
 
 static void reset_current_limit(void)
 {

include/charge_state_v2.h

@@ -52,8 +52,8 @@ struct charge_state_data {
 };
 
 struct sustain_soc {
-	int16_t lower;
-	int16_t upper;
+	int8_t lower;
+	int8_t upper;
 };
 
 /**

include/ec_commands.h

@@ -4167,21 +4167,53 @@ struct ec_params_i2c_write {
  * discharge the battery.
  */
 #define EC_CMD_CHARGE_CONTROL 0x0096
-#define EC_VER_CHARGE_CONTROL 1
+#define EC_VER_CHARGE_CONTROL 2
 
 enum ec_charge_control_mode {
 	CHARGE_CONTROL_NORMAL = 0,
 	CHARGE_CONTROL_IDLE,
 	CHARGE_CONTROL_DISCHARGE,
 };
 
+#define EC_CHARGE_MODE_TEXT { \
+	[CHARGE_CONTROL_NORMAL] = "NORMAL", \
+	[CHARGE_CONTROL_IDLE] = "IDLE", \
+	[CHARGE_CONTROL_DISCHARGE] = "DISCHARGE", \
+	}
+
+enum ec_charge_control_cmd {
+	EC_CHARGE_CONTROL_CMD_SET = 0,
+	EC_CHARGE_CONTROL_CMD_GET,
+};
+
 struct ec_params_charge_control {
 	uint32_t mode;  /* enum charge_control_mode */
-	/* For sustain charge (v2). The struct doesn't need to be exported. */
+
+	/* Below are the fields added in V2. */
+	uint8_t cmd;    /* enum ec_charge_control_cmd. */
+	uint8_t reserved;
+	/*
+	 * Lower and upper thresholds for battery sustainer. This struct isn't
+	 * named to avoid tainting foreign projects' name spaces.
+	 *
+	 * If charge mode is explicitly set (e.g. DISCHARGE), battery sustainer
+	 * will be disabled. To disable battery sustainer, set mode=NORMAL,
+	 * lower=-1, upper=-1.
+	 */
 	struct {
-		int16_t lower;
-		int16_t upper;
-	} sustain_charge;
+		int8_t lower;	/* Display SoC in percentage. */
+		int8_t upper;	/* Display SoC in percentage. */
+	} sustain_soc;
+} __ec_align4;
+
+/* Added in v2 */
+struct ec_response_charge_control {
+	uint32_t mode;  /* enum charge_control_mode */
+	struct {        /* Battery sustainer thresholds */
+		int8_t lower;
+		int8_t upper;
+	} sustain_soc;
+	uint16_t reserved;
 } __ec_align4;
 
 /*****************************************************************************/

util/ectool.c

@@ -7378,38 +7378,115 @@ int cmd_charge_current_limit(int argc, char *argv[])
 	return rv;
 }
 
+static void cmd_charge_control_help(const char *cmd, const char *msg)
+{
+	if (msg)
+		fprintf(stderr, "ERROR: %s\n", msg);
+
+	fprintf(stderr,
+	"\n"
+	"  Usage: %s\n"
+	"    Get current settings.\n"
+	"  Usage: %s normal|idle|discharge\n"
+	"    Set charge mode (and disable battery sustainer).\n"
+	"  Usage: %s normal <lower> <upper>\n"
+	"    Enable battery sustainer. <lower> and <upper> are battery SoC\n"
+	"    between which EC tries to keep the battery level.\n"
+	"\n",
+	cmd, cmd, cmd);
+}
 
 int cmd_charge_control(int argc, char *argv[])
 {
 	struct ec_params_charge_control p;
+	struct ec_response_charge_control r;
+	int version = 2;
+	const char * const charge_mode_text[] = EC_CHARGE_MODE_TEXT;
+	char *e;
 	int rv;
 
-	if (argc != 2) {
-		fprintf(stderr, "Usage: %s <normal | idle | discharge>\n",
-			argv[0]);
-		return -1;
+	if (!ec_cmd_version_supported(EC_CMD_CHARGE_CONTROL, 2))
+		version = 1;
+
+	if (argc == 1) {
+		if (version < 2) {
+			cmd_charge_control_help(argv[0],
+						"Old EC doesn't support GET.");
+			return -1;
+		}
+		p.cmd = EC_CHARGE_CONTROL_CMD_GET;
+		rv = ec_command(EC_CMD_CHARGE_CONTROL, version,
+				&p, sizeof(p), &r, sizeof(r));
+		if (rv < 0) {
+			fprintf(stderr, "Command failed.\n");
+			return rv;
+		}
+		printf("Charge mode = %s (%d)\n",
+		       r.mode < ARRAY_SIZE(charge_mode_text)
+		       		? charge_mode_text[r.mode] : "UNDEFINED",
+		       r.mode);
+		printf("Battery sustainer = %s (%d%% ~ %d%%)\n",
+		       (r.sustain_soc.lower != -1 && r.sustain_soc.upper != -1)
+				? "on" : "off",
+		       r.sustain_soc.lower, r.sustain_soc.upper);
+		return 0;
 	}
 
 	if (!strcasecmp(argv[1], "normal")) {
 		p.mode = CHARGE_CONTROL_NORMAL;
+		if (argc == 2) {
+			p.sustain_soc.lower = -1;
+			p.sustain_soc.upper = -1;
+		} else if (argc == 4) {
+			if (version < 2) {
+				cmd_charge_control_help(argv[0],
+					"Old EC doesn't support sustainer.");
+				return -1;
+			}
+			p.sustain_soc.lower = strtol(argv[2], &e, 0);
+			if (e && *e) {
+				cmd_charge_control_help(argv[0],
+						"Bad character in <lower>");
+				return -1;
+			}
+			p.sustain_soc.upper = strtol(argv[3], &e, 0);
+			if (e && *e) {
+				cmd_charge_control_help(argv[0],
+						"Bad character in <upper>");
+				return -1;
+			}
+		} else {
+			cmd_charge_control_help(argv[0], "Bad arguments");
+			return -1;
+		}
 	} else if (!strcasecmp(argv[1], "idle")) {
+		if (argc != 2) {
+			cmd_charge_control_help(argv[0], "Bad arguments");
+			return -1;
+		}
 		p.mode = CHARGE_CONTROL_IDLE;
 	} else if (!strcasecmp(argv[1], "discharge")) {
+		if (argc != 2) {
+			cmd_charge_control_help(argv[0], "Bad arguments");
+			return -1;
+		}
 		p.mode = CHARGE_CONTROL_DISCHARGE;
 	} else {
-		fprintf(stderr, "Bad value.\n");
+		cmd_charge_control_help(argv[0], "Bad sub-command");
 		return -1;
 	}
 
-	rv = ec_command(EC_CMD_CHARGE_CONTROL, 1, &p, sizeof(p), NULL, 0);
+	rv = ec_command(EC_CMD_CHARGE_CONTROL, version, &p, sizeof(p), NULL, 0);
 	if (rv < 0) {
 		fprintf(stderr, "Is AC connected?\n");
 		return rv;
 	}
 
 	switch (p.mode) {
 	case CHARGE_CONTROL_NORMAL:
-		printf("Charge state machine normal mode.\n");
+		printf("Charge state machine is in normal mode%s.\n",
+		       (p.sustain_soc.lower == -1 || p.sustain_soc.upper == -1)
+		       		? "" : " with sustainer enabled");
 		break;
 	case CHARGE_CONTROL_IDLE:
 		printf("Charge state machine force idle.\n");