smart_battery: Store battery info in struct battery_static_info

Currently, static battery info is stored in a host command response
struct (e.g. ec_response_battery_static_info_v1). It can contain
only up to 8 (v0) or 12 (v1) byte strings.

This patch adds struct battery_static_info, which has the full size
string fields to store static battery info.

This will make a host using virtual_battery.c see full size strings
under /sysfs/power_supply/sbs-*.

ACPI and host programs speaking EC_CMD_BATTERY_GET_STATIC will not be
affected.

BUG=b:198722643, b:203031618
BRANCH=None
TEST=Coachz. Run ectool battery.
TEST=Run cat /sys/class/power_supply/sbs-12-000b/manufacturer and
     and verify the string is longer than 8 chars.

Change-Id: Id847d66c7c846885113f6afb05a9de1788bf92ae
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3180272
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>

4 files changed, 91 insertions, 26 deletions

diff --git a/common/battery_v2.c b/common/battery_v2.c
index 3fc203b9e8..20947aac37 100644
--- a/common/battery_v2.c
+++ b/common/battery_v2.c
@@ -21,7 +21,7 @@
  * 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_v1 battery_static[CONFIG_BATTERY_COUNT];
+struct battery_static_info battery_static[CONFIG_BATTERY_COUNT];
 struct ec_response_battery_dynamic_info battery_dynamic[CONFIG_BATTERY_COUNT];
 
 #ifdef HAS_TASK_HOSTCMD
@@ -79,38 +79,53 @@ 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;
-	const struct ec_response_battery_static_info_v1 *bat;
+	const struct battery_static_info *bs;
 
 	if (p->index < 0 || p->index >= CONFIG_BATTERY_COUNT)
 		return EC_RES_INVALID_PARAM;
-	bat = &battery_static[p->index];
+	bs = &battery_static[p->index];
 
 	battery_update(p->index);
 	if (args->version == 0) {
 		struct ec_response_battery_static_info *r = args->response;
 
-		r->design_capacity = bat->design_capacity;
-		r->design_voltage = bat->design_voltage;
-		r->cycle_count = bat->cycle_count;
+		r->design_capacity = bs->design_capacity;
+		r->design_voltage = bs->design_voltage;
+		r->cycle_count = bs->cycle_count;
 
 		/* Truncate strings to reduced v0 size */
-		memcpy(&r->manufacturer, &bat->manufacturer_ext,
+		memcpy(&r->manufacturer, &bs->manufacturer_ext,
 		       sizeof(r->manufacturer));
 		r->manufacturer[sizeof(r->manufacturer) - 1] = 0;
-		memcpy(&r->model, &bat->model_ext, sizeof(r->model));
+		memcpy(&r->model, &bs->model_ext, sizeof(r->model));
 		r->model[sizeof(r->model) - 1] = 0;
-		memcpy(&r->serial, &bat->serial_ext, sizeof(r->serial));
+		memcpy(&r->serial, &bs->serial_ext, sizeof(r->serial));
 		r->serial[sizeof(r->serial) - 1] = 0;
-		memcpy(&r->type, &bat->type_ext, sizeof(r->type));
+		memcpy(&r->type, &bs->type_ext, sizeof(r->type));
 		r->type[sizeof(r->type) - 1] = 0;
 
 		args->response_size = sizeof(*r);
 	} else if (args->version == 1) {
-		/* v1 command stores the same data internally */
 		struct ec_response_battery_static_info_v1 *r = args->response;
 
+		r->design_capacity = bs->design_capacity;
+		r->design_voltage = bs->design_voltage;
+		r->cycle_count = bs->cycle_count;
+
+		/* Truncate strings to reduced size */
+		memcpy(r->manufacturer_ext, &bs->manufacturer_ext,
+		       sizeof(r->manufacturer_ext));
+		r->manufacturer_ext[sizeof(r->manufacturer_ext) - 1] = 0;
+		memcpy(r->model_ext, &bs->model_ext, sizeof(r->model_ext));
+		r->model_ext[sizeof(r->model_ext) - 1] = 0;
+		memcpy(r->serial_ext, &bs->serial_ext, sizeof(r->serial_ext));
+		r->serial_ext[sizeof(r->serial_ext) - 1] = 0;
+		memcpy(r->type_ext, &bs->type_ext, sizeof(r->type_ext));
+		r->type_ext[sizeof(r->type_ext) - 1] = 0;
+
 		args->response_size = sizeof(*r);
-		memcpy(r, bat, sizeof(*r));
+	} else {
+		return EC_RES_INVALID_VERSION;
 	}
 
 	return EC_RES_SUCCESS;
@@ -194,8 +209,7 @@ int update_static_battery_info(void)
 	 */
 	int rv, ret;
 
-	struct ec_response_battery_static_info_v1 *const bs =
-		&battery_static[BATT_IDX_MAIN];
+	struct battery_static_info *const bs = &battery_static[BATT_IDX_MAIN];
 
 	/* Clear all static information. */
 	memset(bs, 0, sizeof(*bs));
diff --git a/common/ec_ec_comm_client.c b/common/ec_ec_comm_client.c
index c92433af8c..b2f2387976 100644
--- a/common/ec_ec_comm_client.c
+++ b/common/ec_ec_comm_client.c
@@ -296,6 +296,8 @@ int ec_ec_client_base_get_static_info(void)
 			uint8_t crc8;
 		} resp;
 	} __packed data;
+	const struct ec_response_battery_static_info *info = &data.resp.info;
+	struct battery_static_info *bs = &battery_static[BATT_IDX_BASE];
 
 	data.req.param.index = 0;
 
@@ -316,8 +318,15 @@ int ec_ec_client_base_get_static_info(void)
 	CPRINTF("C-count:    %d\n", data.resp.info.cycle_count);
 #endif
 
-	memcpy(&battery_static[BATT_IDX_BASE], &data.resp.info,
-				sizeof(battery_static[BATT_IDX_BASE]));
+	bs->design_capacity = info->design_capacity;
+	bs->design_voltage = info->design_voltage;
+	bs->cycle_count = info->cycle_count;
+	strncpy(bs->manufacturer_ext, info->manufacturer,
+		sizeof(bs->manufacturer_ext));
+	strncpy(bs->model_ext, info->model, sizeof(bs->model_ext));
+	strncpy(bs->serial_ext, info->serial, sizeof(bs->serial_ext));
+	strncpy(bs->type_ext, info->type, sizeof(bs->type_ext));
+
 	return EC_RES_SUCCESS;
 }
 
diff --git a/common/virtual_battery.c b/common/virtual_battery.c
index 7c40ffb492..99dab0540e 100644
--- a/common/virtual_battery.c
+++ b/common/virtual_battery.c
@@ -171,6 +171,15 @@ void copy_memmap_string(uint8_t *dest, int offset, int len)
 	memcpy(dest + 1, memmap_str, MIN(memmap_strlen, len - 1));
 }
 
+static void copy_battery_info_string(uint8_t *dst, const uint8_t *src, int len)
+{
+	if (len == 0)
+		return;
+
+	dst[0] = strlen(src);
+	strncpy(dst + 1, src, len - 1);
+}
+
 int virtual_battery_operation(const uint8_t *batt_cmd_head,
 			      uint8_t *dest,
 			      int read_len,
@@ -192,6 +201,14 @@ int virtual_battery_operation(const uint8_t *batt_cmd_head,
 	 * are two bytes.
 	 */
 	int bounded_read_len = MIN(read_len, 2);
+	const struct battery_static_info *bs;
+
+	if (IS_ENABLED(CONFIG_BATTERY_V2))
+		/*
+		 * TODO: To support multiple batteries, we need to translate
+		 * i2c address to a battery index.
+		 */
+		bs = &battery_static[BATT_IDX_MAIN];
 
 	curr_batt = charger_current_battery_params();
 	switch (*batt_cmd_head) {
@@ -296,13 +313,24 @@ int virtual_battery_operation(const uint8_t *batt_cmd_head,
 		memcpy(dest, &val, bounded_read_len);
 		break;
 	case SB_MANUFACTURER_NAME:
-		copy_memmap_string(dest, EC_MEMMAP_BATT_MFGR, read_len);
+		if (IS_ENABLED(CONFIG_BATTERY_V2))
+			copy_battery_info_string(dest, bs->manufacturer_ext,
+						 read_len);
+		else
+			copy_memmap_string(dest, EC_MEMMAP_BATT_MFGR, read_len);
 		break;
 	case SB_DEVICE_NAME:
-		copy_memmap_string(dest, EC_MEMMAP_BATT_MODEL, read_len);
+		if (IS_ENABLED(CONFIG_BATTERY_V2))
+			copy_battery_info_string(dest, bs->model_ext, read_len);
+		else
+			copy_memmap_string(dest, EC_MEMMAP_BATT_MODEL,
+					   read_len);
 		break;
 	case SB_DEVICE_CHEMISTRY:
-		copy_memmap_string(dest, EC_MEMMAP_BATT_TYPE, read_len);
+		if (IS_ENABLED(CONFIG_BATTERY_V2))
+			copy_battery_info_string(dest, bs->type_ext, read_len);
+		else
+			copy_memmap_string(dest, EC_MEMMAP_BATT_TYPE, read_len);
 		break;
 	case SB_AVERAGE_TIME_TO_FULL:
 		/* This may cause an i2c transaction */
diff --git a/include/battery.h b/include/battery.h
index be23863c23..38ca2da91e 100644
--- a/include/battery.h
+++ b/include/battery.h
@@ -19,13 +19,6 @@ enum battery_index {
 	BATT_IDX_BASE = 1,
 };
 
-#ifdef CONFIG_BATTERY_V2
-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];
-#endif
-
 /* Stop charge when charging and battery level >= this percentage */
 #define BATTERY_LEVEL_FULL		100
 
@@ -80,6 +73,27 @@ enum battery_disconnect_state {
 	BATTERY_DISCONNECT_ERROR,
 };
 
+struct battery_static_info {
+	uint16_t design_capacity;
+	uint16_t design_voltage;
+	uint32_t cycle_count;
+	/*
+	 * TODO: The fields below should be renamed & re-typed:
+	 * uint16_t serial[32];
+	 * char manufacturer[32];
+	 * char device_name[32];
+	 * char chemistry[32];
+	 */
+	/* Max string size in the SB spec is 31. */
+	char manufacturer_ext[32];	/* SB_MANUFACTURER_NAME */
+	char model_ext[32];		/* SB_DEVICE_NAME */
+	char serial_ext[32];		/* SB_SERIAL_NUMBER */
+	char type_ext[32];		/* SB_DEVICE_CHEMISTRY */
+};
+
+extern struct battery_static_info battery_static[];
+extern struct ec_response_battery_dynamic_info battery_dynamic[];
+
 /* Battery parameters */
 struct batt_params {
 	int temperature;      /* Temperature in 0.1 K */