battery: extend BATTERY_GET_STATIC hostcmd for 32-byte stringsfactory-nissa-15199.B-main

The Smart Battery Specification defines a string as up to 31 characters,
and nissa project has encountered some batteries where the existing
11-character strings for v1 of BATTERY_GET_STATIC are insufficient to
differentiate some batteries that can be used on a given device. This
change adds a new version of this host command that extends the strings
fields to each accommodate up to 31 characters of data, which should be
enough to support all smart batteries compliant with the latest version
of the specification.

BUG=b:263920422
TEST=`ectool battery` with updated ectool works on Nereid and uses the
     new hostcmd version. If EC support for v2 or v1 is disabled, older
     command versions are used.
BRANCH=nissa

Change-Id: Ia6a3a917051d5e641e7d38ef6936ec7174cb8aab
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4150669
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>
(cherry picked from commit 19b4b94a0589a15fa40f7694ec79b3136e3219c6)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4173089
Commit-Queue: Clark Chung <ckclark@chromium.org>
Tested-by: Clark Chung <ckclark@chromium.org>

3 files changed, 158 insertions, 32 deletions

diff --git a/common/battery_v2.c b/common/battery_v2.c
index 57ae07196b..9096cbb72c 100644
--- a/common/battery_v2.c
+++ b/common/battery_v2.c
@@ -129,6 +129,20 @@ host_command_battery_get_static(struct host_cmd_handler_args *args)
 		r->type_ext[sizeof(r->type_ext) - 1] = 0;
 
 		args->response_size = sizeof(*r);
+	} else if (args->version == 2) {
+		struct ec_response_battery_static_info_v2 *r = args->response;
+
+		r->design_capacity = bs->design_capacity;
+		r->design_voltage = bs->design_voltage;
+		r->cycle_count = bs->cycle_count;
+
+		strzcpy(r->manufacturer, bs->manufacturer_ext,
+			sizeof(r->manufacturer));
+		strzcpy(r->device_name, bs->model_ext, sizeof(r->device_name));
+		strzcpy(r->serial, bs->serial_ext, sizeof(r->serial));
+		strzcpy(r->chemistry, bs->type_ext, sizeof(r->chemistry));
+
+		args->response_size = sizeof(*r);
 	} else {
 		return EC_RES_INVALID_VERSION;
 	}
@@ -136,7 +150,7 @@ host_command_battery_get_static(struct host_cmd_handler_args *args)
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_BATTERY_GET_STATIC, host_command_battery_get_static,
-		     EC_VER_MASK(0) | EC_VER_MASK(1));
+		     EC_VER_MASK(0) | EC_VER_MASK(1) | EC_VER_MASK(2));
 
 static enum ec_status
 host_command_battery_get_dynamic(struct host_cmd_handler_args *args)
diff --git a/include/ec_commands.h b/include/ec_commands.h
index acb502ca99..5b76f369bc 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -7667,6 +7667,32 @@ struct ec_response_battery_static_info_v1 {
 	char type_ext[12];
 } __ec_align4;
 
+/**
+ * struct ec_response_battery_static_info_v2 - hostcmd v2 battery static info
+ *
+ * Equivalent to struct ec_response_battery_static_info, but with strings
+ * further lengthened (relative to v1) to accommodate the maximum string length
+ * permitted by the Smart Battery Data Specification revision 1.1 and fields
+ * renamed to better match that specification.
+ *
+ * @design_capacity: battery design capacity (in mAh)
+ * @design_voltage: battery design voltage (in mV)
+ * @cycle_count: battery cycle count
+ * @manufacturer: battery manufacturer string
+ * @device_name: battery model string
+ * @serial: battery serial number string
+ * @chemistry: battery type string
+ */
+struct ec_response_battery_static_info_v2 {
+	uint16_t design_capacity;
+	uint16_t design_voltage;
+	uint32_t cycle_count;
+	char manufacturer[32];
+	char device_name[32];
+	char serial[32];
+	char chemistry[32];
+} __ec_align4;
+
 /*
  * Get battery dynamic information, i.e. information that is likely to change
  * every time it is read.
diff --git a/util/ectool.cc b/util/ectool.cc
index c116b35fcc..213de566b8 100644
--- a/util/ectool.cc
+++ b/util/ectool.cc
@@ -8035,64 +8035,59 @@ void print_battery_flags(int flags)
 	printf("\n");
 }
 
-int get_battery_command(int index)
+static int get_battery_command_print_info(
+	uint8_t index,
+	const struct ec_response_battery_static_info_v2 *const static_r)
 {
-	struct ec_params_battery_static_info static_p;
-	struct ec_response_battery_static_info_v1 static_r;
-	struct ec_params_battery_dynamic_info dynamic_p;
+	struct ec_params_battery_dynamic_info dynamic_p = {
+		.index = index,
+	};
 	struct ec_response_battery_dynamic_info dynamic_r;
 	int rv;
 
-	printf("Battery %d info:\n", index);
-
-	static_p.index = index;
-	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 1, &static_p,
-			sizeof(static_p), &static_r, sizeof(static_r));
-	if (rv < 0)
-		return -1;
-
-	dynamic_p.index = index;
 	rv = ec_command(EC_CMD_BATTERY_GET_DYNAMIC, 0, &dynamic_p,
 			sizeof(dynamic_p), &dynamic_r, sizeof(dynamic_r));
 	if (rv < 0)
 		return -1;
 
+	printf("Battery %d info:\n", index);
+
 	if (dynamic_r.flags & EC_BATT_FLAG_INVALID_DATA) {
 		printf("  Invalid data (not present?)\n");
 		return -1;
 	}
 
-	if (!is_string_printable(static_r.manufacturer_ext))
+	if (!is_string_printable(static_r->manufacturer))
 		goto cmd_error;
-	printf("  OEM name:               %s\n", static_r.manufacturer_ext);
+	printf("  OEM name:               %s\n", static_r->manufacturer);
 
-	if (!is_string_printable(static_r.model_ext))
+	if (!is_string_printable(static_r->device_name))
 		goto cmd_error;
-	printf("  Model number:           %s\n", static_r.model_ext);
+	printf("  Model number:           %s\n", static_r->device_name);
 
-	if (!is_string_printable(static_r.type_ext))
+	if (!is_string_printable(static_r->chemistry))
 		goto cmd_error;
-	printf("  Chemistry   :           %s\n", static_r.type_ext);
+	printf("  Chemistry   :           %s\n", static_r->chemistry);
 
-	if (!is_string_printable(static_r.serial_ext))
+	if (!is_string_printable(static_r->serial))
 		goto cmd_error;
-	printf("  Serial number:          %s\n", static_r.serial_ext);
+	printf("  Serial number:          %s\n", static_r->serial);
 
-	if (!is_battery_range(static_r.design_capacity))
+	if (!is_battery_range(static_r->design_capacity))
 		goto cmd_error;
-	printf("  Design capacity:        %u mAh\n", static_r.design_capacity);
+	printf("  Design capacity:        %u mAh\n", static_r->design_capacity);
 
 	if (!is_battery_range(dynamic_r.full_capacity))
 		goto cmd_error;
 	printf("  Last full charge:       %u mAh\n", dynamic_r.full_capacity);
 
-	if (!is_battery_range(static_r.design_voltage))
+	if (!is_battery_range(static_r->design_voltage))
 		goto cmd_error;
-	printf("  Design output voltage   %u mV\n", static_r.design_voltage);
+	printf("  Design output voltage   %u mV\n", static_r->design_voltage);
 
-	if (!is_battery_range(static_r.cycle_count))
+	if (!is_battery_range(static_r->cycle_count))
 		goto cmd_error;
-	printf("  Cycle count             %u\n", static_r.cycle_count);
+	printf("  Cycle count             %u\n", static_r->cycle_count);
 
 	if (!is_battery_range(dynamic_r.actual_voltage))
 		goto cmd_error;
@@ -8122,6 +8117,89 @@ cmd_error:
 	return -1;
 }
 
+static int get_battery_command_v2(uint8_t index)
+{
+	struct ec_params_battery_static_info static_p = {
+		.index = index,
+	};
+	struct ec_response_battery_static_info_v2 static_r;
+	int rv;
+
+	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 2, &static_p,
+			sizeof(static_p), &static_r, sizeof(static_r));
+	if (rv < 0) {
+		fprintf(stderr, "CMD_BATTERY_GET_STATIC v2 failed: %d\n", rv);
+		return -1;
+	}
+
+	return get_battery_command_print_info(index, &static_r);
+}
+
+static int get_battery_command_v1(uint8_t index)
+{
+	struct ec_params_battery_static_info static_p {
+		.index = index,
+	};
+	struct ec_response_battery_static_info_v1 static_r;
+	int rv;
+
+	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 1, &static_p,
+			sizeof(static_p), &static_r, sizeof(static_r));
+	if (rv < 0) {
+		fprintf(stderr, "CMD_BATTERY_GET_STATIC v1 failed: %d\n", rv);
+		return -1;
+	}
+
+	/* Translate v1 response into v2 to display it */
+	struct ec_response_battery_static_info_v2 static_v2 = {
+		.design_capacity = static_r.design_capacity,
+		.design_voltage = static_r.design_voltage,
+		.cycle_count = static_r.cycle_count,
+	};
+	strncpy(static_v2.manufacturer, static_r.manufacturer_ext,
+		sizeof(static_v2.manufacturer) - 1);
+	strncpy(static_v2.device_name, static_r.model_ext,
+		sizeof(static_v2.device_name) - 1);
+	strncpy(static_v2.serial, static_r.serial_ext,
+		sizeof(static_v2.serial) - 1);
+	strncpy(static_v2.chemistry, static_r.type_ext,
+		sizeof(static_v2.chemistry) - 1);
+
+	return get_battery_command_print_info(index, &static_v2);
+}
+static int get_battery_command_v0(uint8_t index)
+{
+	struct ec_params_battery_static_info static_p = {
+		.index = index,
+	};
+	struct ec_response_battery_static_info static_r;
+	int rv;
+
+	rv = ec_command(EC_CMD_BATTERY_GET_STATIC, 0, &static_p,
+			sizeof(static_p), &static_r, sizeof(static_r));
+	if (rv < 0) {
+		fprintf(stderr, "CMD_BATTERY_GET_STATIC v0 failed: %d\n", rv);
+		return -1;
+	}
+
+	/* Translate v0 response into v2 to display it */
+	struct ec_response_battery_static_info_v2 static_v2 = {
+		.design_capacity = static_r.design_capacity,
+		.design_voltage = static_r.design_voltage,
+		.cycle_count = static_r.cycle_count,
+	};
+	strncpy(static_v2.manufacturer, static_r.manufacturer,
+		sizeof(static_v2.manufacturer) - 1);
+	strncpy(static_v2.device_name, static_r.model,
+		sizeof(static_v2.device_name) - 1);
+	strncpy(static_v2.serial, static_r.serial,
+		sizeof(static_v2.serial) - 1);
+	strncpy(static_v2.chemistry, static_r.type,
+		sizeof(static_v2.chemistry) - 1);
+
+	return get_battery_command_print_info(index, &static_v2);
+}
+
 int cmd_battery(int argc, char *argv[])
 {
 	char batt_text[EC_MEMMAP_TEXT_MAX];
@@ -8142,11 +8220,19 @@ int cmd_battery(int argc, char *argv[])
 	}
 
 	/*
-	 * Read non-primary batteries through hostcmd, and all batteries
-	 * if longer strings are supported for static info.
+	 * Prefer to use newer hostcmd versions if supported because these allow
+	 * us to read longer strings, and always use hostcmd for non-primary
+	 * batteries because memmap doesn't export that data.
 	 */
-	if (index > 0 || ec_cmd_version_supported(EC_CMD_BATTERY_GET_STATIC, 1))
-		return get_battery_command(index);
+	uint32_t versions;
+	ec_get_cmd_versions(EC_CMD_BATTERY_GET_STATIC, &versions);
+
+	if (versions & EC_VER_MASK(2))
+		return get_battery_command_v2(index);
+	else if (versions & EC_VER_MASK(1))
+		return get_battery_command_v1(index);
+	else if (index > 0)
+		return get_battery_command_v0(index);
 
 	val = read_mapped_mem8(EC_MEMMAP_BATTERY_VERSION);
 	if (val < 1) {