Add EC_CMD_BATTERY_GET_STATIC v1 for zork

Some zork variants have battery model names that differ only beyond
the 7th character, which cannot be differentiated with the current
limitation of 8 characters per battery string. Introduce a new hostcmd
version that allows longer battery strings and enable it on Zork.

Because allowing longer strings through the host memory map is more
difficult and not required (because getting the full longer string
is mostly only useful for servicing), the host memory map is unchanged.
ectool is updated to use hostcmd (rather than memory map) if the new
command version is available, in order to take advantage of it.

BUG=b:171854783
TEST=ectool battery prints longer strings when supported by the EC;
     a hacked EC on morphius can return 11 characters of text.
     An EC running older firmware still works with a new ectool.
BRANCH=zork

Change-Id: I63d20d4f690b6945cb1d423aafaf55dafc039211
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2519243
Reviewed-by: Edward Hill <ecgh@chromium.org>
(cherry picked from commit 6bc9bb622a31845277d5513fa80fb6766ee68f6c)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2578616
Reviewed-by: Pin-yen Lin <treapking@chromium.org>
Commit-Queue: Pin-yen Lin <treapking@chromium.org>
Tested-by: Pin-yen Lin <treapking@chromium.org>

6 files changed, 94 insertions, 31 deletions

diff --git a/baseboard/zork/baseboard.h b/baseboard/zork/baseboard.h
index f5b46483d3..cd8df675c7 100644
--- a/baseboard/zork/baseboard.h
+++ b/baseboard/zork/baseboard.h
@@ -61,6 +61,16 @@
 #define CONFIG_BATTERY_FUEL_GAUGE
 #define CONFIG_BATTERY_REVIVE_DISCONNECT
 #define CONFIG_BATTERY_SMART
+/*
+ * Enable support for battery hostcmd, supporting longer strings.
+ *
+ * Vilboz battery options' model names vary in the 8th character, which is
+ * truncated in the memory mapped battery info; differentiating them requires
+ * support for EC_CMD_BATTERY_GET_STATIC version 1.
+ */
+#define CONFIG_BATTERY_V2
+#define CONFIG_BATTERY_COUNT 1
+#define CONFIG_HOSTCMD_BATTERY_V2
 
 #define CONFIG_BC12_DETECT_PI3USB9201
 
diff --git a/common/battery.c b/common/battery.c
index 9e03bb83a2..abb7ec3d0c 100644
--- a/common/battery.c
+++ b/common/battery.c
@@ -32,7 +32,7 @@ const static int batt_host_shutdown_pct = CONFIG_BATT_HOST_SHUTDOWN_PERCENTAGE;
  * Store battery information in these 2 structures. Main (lid) battery is always
  * at index 0, and secondary (base) battery at index 1.
  */
-struct ec_response_battery_static_info battery_static[CONFIG_BATTERY_COUNT];
+struct ec_response_battery_static_info_v1 battery_static[CONFIG_BATTERY_COUNT];
 struct ec_response_battery_dynamic_info battery_dynamic[CONFIG_BATTERY_COUNT];
 #endif
 
@@ -467,19 +467,44 @@ static enum ec_status
 host_command_battery_get_static(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_battery_static_info *p = args->params;
-	struct ec_response_battery_static_info *r = args->response;
+	struct ec_response_battery_static_info_v1 *bat;
 
 	if (p->index < 0 || p->index >= CONFIG_BATTERY_COUNT)
 		return EC_RES_INVALID_PARAM;
+	bat = &battery_static[p->index];
+
 	battery_update(p->index);
-	args->response_size = sizeof(*r);
-	memcpy(r, &battery_static[p->index], sizeof(*r));
+	if (args->version == 0) {
+		struct ec_response_battery_static_info *r = args->response;
+
+		args->response_size = sizeof(*r);
+		r->design_capacity = bat->design_capacity;
+		r->design_voltage = bat->design_voltage;
+		r->cycle_count = bat->cycle_count;
+
+		/* Truncate strings to reduced v0 size */
+		memcpy(&r->manufacturer, &bat->manufacturer_ext,
+		       sizeof(r->manufacturer));
+		r->manufacturer[sizeof(r->manufacturer) - 1] = 0;
+		memcpy(&r->model, &bat->model_ext, sizeof(r->model));
+		r->model[sizeof(r->model) - 1] = 0;
+		memcpy(&r->serial, &bat->serial_ext, sizeof(r->serial));
+		r->serial[sizeof(r->serial) - 1] = 0;
+		memcpy(&r->type, &bat->type_ext, sizeof(r->type));
+		r->type[sizeof(r->type) - 1] = 0;
+	} else {
+		/* v1 command stores the same data internally */
+		struct ec_response_battery_static_info_v1 *r = args->response;
+
+		args->response_size = sizeof(*r);
+		memcpy(r, bat, sizeof(*r));
+	}
 
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_BATTERY_GET_STATIC,
 		     host_command_battery_get_static,
-		     EC_VER_MASK(0));
+		     EC_VER_MASK(0) | EC_VER_MASK(1));
 
 static enum ec_status
 host_command_battery_get_dynamic(struct host_cmd_handler_args *args)
@@ -515,7 +540,8 @@ static void battery_update(enum battery_index i)
 
 	/* Smart battery serial number is 16 bits */
 	batt_str = (char *)host_get_memmap(EC_MEMMAP_BATT_SERIAL);
-	memcpy(batt_str, battery_static[i].serial, EC_MEMMAP_TEXT_MAX);
+	memcpy(batt_str, battery_static[i].serial_ext, EC_MEMMAP_TEXT_MAX);
+	batt_str[EC_MEMMAP_TEXT_MAX - 1] = 0;
 
 	/* Design Capacity of Full */
 	*memmap_dcap = battery_static[i].design_capacity;
@@ -528,15 +554,19 @@ static void battery_update(enum battery_index i)
 
 	/* Battery Manufacturer string */
 	batt_str = (char *)host_get_memmap(EC_MEMMAP_BATT_MFGR);
-	memcpy(batt_str, battery_static[i].manufacturer, EC_MEMMAP_TEXT_MAX);
+	memcpy(batt_str, battery_static[i].manufacturer_ext,
+	       EC_MEMMAP_TEXT_MAX);
+	batt_str[EC_MEMMAP_TEXT_MAX - 1] = 0;
 
 	/* Battery Model string */
 	batt_str = (char *)host_get_memmap(EC_MEMMAP_BATT_MODEL);
-	memcpy(batt_str, battery_static[i].model, EC_MEMMAP_TEXT_MAX);
+	memcpy(batt_str, battery_static[i].model_ext, EC_MEMMAP_TEXT_MAX);
+	batt_str[EC_MEMMAP_TEXT_MAX - 1] = 0;
 
 	/* Battery Type string */
 	batt_str = (char *)host_get_memmap(EC_MEMMAP_BATT_TYPE);
-	memcpy(batt_str, battery_static[i].type, EC_MEMMAP_TEXT_MAX);
+	memcpy(batt_str, battery_static[i].type_ext, EC_MEMMAP_TEXT_MAX);
+	batt_str[EC_MEMMAP_TEXT_MAX - 1] = 0;
 
 	*memmap_volt = battery_dynamic[i].actual_voltage;
 	*memmap_rate = battery_dynamic[i].actual_current;
diff --git a/common/charge_state_v2.c b/common/charge_state_v2.c
index 2796dd9804..88e4b7da7b 100644
--- a/common/charge_state_v2.c
+++ b/common/charge_state_v2.c
@@ -892,7 +892,7 @@ static int update_static_battery_info(void)
 	 */
 	int rv, ret;
 
-	struct ec_response_battery_static_info *const bs =
+	struct ec_response_battery_static_info_v1 *const bs =
 		&battery_static[BATT_IDX_MAIN];
 
 	/* Clear all static information. */
@@ -901,7 +901,8 @@ static int update_static_battery_info(void)
 	/* Smart battery serial number is 16 bits */
 	rv = battery_serial_number(&batt_serial);
 	if (!rv)
-		snprintf(bs->serial, sizeof(bs->serial), "%04X", batt_serial);
+		snprintf(bs->serial_ext, sizeof(bs->serial_ext),
+			 "%04X", batt_serial);
 
 	/* Design Capacity of Full */
 	ret = battery_design_capacity(&val);
@@ -922,14 +923,14 @@ static int update_static_battery_info(void)
 	rv |= ret;
 
 	/* Battery Manufacturer string */
-	rv |= battery_manufacturer_name(bs->manufacturer,
-					sizeof(bs->manufacturer));
+	rv |= battery_manufacturer_name(bs->manufacturer_ext,
+					sizeof(bs->manufacturer_ext));
 
 	/* Battery Model string */
-	rv |= battery_device_name(bs->model, sizeof(bs->model));
+	rv |= battery_device_name(bs->model_ext, sizeof(bs->model_ext));
 
 	/* Battery Type string */
-	rv |= battery_device_chemistry(bs->type, sizeof(bs->type));
+	rv |= battery_device_chemistry(bs->type_ext, sizeof(bs->type_ext));
 
 	/* Zero the dynamic entries. They'll come next. */
 	memset(&battery_dynamic[BATT_IDX_MAIN], 0,
diff --git a/include/battery.h b/include/battery.h
index 96fa900d4f..aed1e1c5cf 100644
--- a/include/battery.h
+++ b/include/battery.h
@@ -19,7 +19,7 @@ enum battery_index {
 };
 
 #ifdef CONFIG_BATTERY_V2
-extern struct ec_response_battery_static_info
+extern struct ec_response_battery_static_info_v1
 	battery_static[CONFIG_BATTERY_COUNT];
 extern struct ec_response_battery_dynamic_info
 	battery_dynamic[CONFIG_BATTERY_COUNT];
diff --git a/include/ec_commands.h b/include/ec_commands.h
index edc0d8010f..12ae80fb09 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -6653,6 +6653,28 @@ struct ec_response_battery_static_info {
 	uint32_t cycle_count;
 } __ec_align4;
 
+/**
+ * struct ec_response_battery_static_info_v1 - hostcmd v1 battery static info
+ * Equivalent to struct ec_response_battery_static_info, but with longer
+ * strings.
+ * @design_capacity: battery design capacity (in mAh)
+ * @design_voltage: battery design voltage (in mV)
+ * @cycle_count: battery cycle count
+ * @manufacturer_ext: battery manufacturer string
+ * @model_ext: battery model string
+ * @serial_ext: battery serial number string
+ * @type_ext: battery type string
+ */
+struct ec_response_battery_static_info_v1 {
+	uint16_t design_capacity;
+	uint16_t design_voltage;
+	uint32_t cycle_count;
+	char manufacturer_ext[12];
+	char model_ext[12];
+	char serial_ext[12];
+	char type_ext[12];
+} __ec_align4;
+
 /*
  * Get battery dynamic information, i.e. information that is likely to change
  * every time it is read.
diff --git a/util/ectool.c b/util/ectool.c
index 81c4da9957..7dd6a901db 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -7599,7 +7599,7 @@ void print_battery_flags(int flags)
 int get_battery_command(int index)
 {
 	struct ec_params_battery_static_info static_p;
-	struct ec_response_battery_static_info static_r;
+	struct ec_response_battery_static_info_v1 static_r;
 	struct ec_params_battery_dynamic_info dynamic_p;
 	struct ec_response_battery_dynamic_info dynamic_r;
 	int rv;
@@ -7607,7 +7607,7 @@ int get_battery_command(int index)
 	printf("Battery %d info:\n", index);
 
 	static_p.index = index;
-	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 0,
+	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 1,
 			&static_p, sizeof(static_p),
 			&static_r, sizeof(static_r));
 	if (rv < 0)
@@ -7625,21 +7625,21 @@ int get_battery_command(int index)
 		return -1;
 	}
 
-	if (!is_string_printable(static_r.manufacturer))
+	if (!is_string_printable(static_r.manufacturer_ext))
 		goto cmd_error;
-	printf("  OEM name:               %s\n", static_r.manufacturer);
+	printf("  OEM name:               %s\n", static_r.manufacturer_ext);
 
-	if (!is_string_printable(static_r.model))
+	if (!is_string_printable(static_r.model_ext))
 		goto cmd_error;
-	printf("  Model number:           %s\n", static_r.model);
+	printf("  Model number:           %s\n", static_r.model_ext);
 
-	if (!is_string_printable(static_r.type))
+	if (!is_string_printable(static_r.type_ext))
 		goto cmd_error;
-	printf("  Chemistry   :           %s\n", static_r.type);
+	printf("  Chemistry   :           %s\n", static_r.type_ext);
 
-	if (!is_string_printable(static_r.serial))
+	if (!is_string_printable(static_r.serial_ext))
 		goto cmd_error;
-	printf("  Serial number:          %s\n", static_r.serial);
+	printf("  Serial number:          %s\n", static_r.serial_ext);
 
 	if (!is_battery_range(static_r.design_capacity))
 		goto cmd_error;
@@ -7701,15 +7701,15 @@ int cmd_battery(int argc, char *argv[])
 			fprintf(stderr, "Bad battery index.\n");
 			return -1;
 		}
-
-		if (index > 0)
-			return get_battery_command(index);
 	}
 
 	/*
-	 * TODO(b:65697620): When supported/required, read battery 0 information
-	 * through EC commands as well.
+	 * Read non-primary batteries through hostcmd, and all batteries
+	 * if longer strings are supported for static info.
 	 */
+	if (index > 0 ||
+	    ec_cmd_version_supported(EC_CMD_BATTERY_GET_STATIC, 1))
+		return get_battery_command(index);
 
 	val = read_mapped_mem8(EC_MEMMAP_BATTERY_VERSION);
 	if (val < 1) {