summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Zieba <robertzieba@google.com>2022-04-26 11:22:06 -0600
committerChromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com>2022-05-05 17:19:43 +0000
commitab0976af3371547511fa37c9e769faa509eddd1c (patch)
treea79b45a5fc0a73e68ae77673a70da8ab384a4667
parent8f7ec2f4d15b811d37dddad4f25334a41fc5f113 (diff)
downloadchrome-ec-ab0976af3371547511fa37c9e769faa509eddd1c.tar.gz
smart battery: Fix mfgacc access function reading wrong number of bytes
`sb_read_mfgacc` currently uses `sb_read_string`. This function reads a null-terminated string. However, the data that is being read is not a null-terminated string. This is causing N-1 bytes to be read instead of N bytes. This commit adds an `sb_read_sized_block` function that is used instead. Only PARAM_SAFETY_STATUS and PARAM_OPERATION_STATUS are read with `sb_read_mfgacc` and neither of these values are a string. BUG=b:228589840 TEST=Ran i2ctrace to verify that right number of bytes are now being read, tested with CL:3578293 to ensure no errors BRANCH=guybrush Change-Id: Iaedb4d28274bda8b6d1ee6d2021ebd04f83767f8 Signed-off-by: Robert Zieba <robertzieba@google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3606146 Reviewed-by: Rob Barnes <robbarnes@google.com> Reviewed-by: Diana Z <dzigterman@chromium.org> Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
-rw-r--r--driver/battery/smart.c25
-rw-r--r--include/battery_smart.h15
2 files changed, 37 insertions, 3 deletions
diff --git a/driver/battery/smart.c b/driver/battery/smart.c
index 62a7d61258..c87a1a6cdc 100644
--- a/driver/battery/smart.c
+++ b/driver/battery/smart.c
@@ -92,7 +92,28 @@ int sb_read_string(int offset, uint8_t *data, int len)
if (battery_supports_pec())
addr_flags |= I2C_FLAG_PEC;
- return i2c_read_string(I2C_PORT_BATTERY, addr_flags, offset, data, len);
+ return i2c_read_string(I2C_PORT_BATTERY, addr_flags, offset, data,
+ len);
+}
+
+int sb_read_sized_block(int offset, uint8_t *data, int len)
+{
+ uint16_t addr_flags = BATTERY_ADDR_FLAGS;
+ int read_len = 0;
+
+ if (IS_ENABLED(CONFIG_BATTERY_CUT_OFF)) {
+ /*
+ * Some batteries would wake up after cut-off if we talk to it.
+ */
+ if (battery_is_cut_off())
+ return EC_RES_ACCESS_DENIED;
+ }
+
+ if (battery_supports_pec())
+ addr_flags |= I2C_FLAG_PEC;
+
+ return i2c_read_sized_block(I2C_PORT_BATTERY, addr_flags, offset, data,
+ len, &read_len);
}
int sb_read_mfgacc(int cmd, int block, uint8_t *data, int len)
@@ -116,7 +137,7 @@ int sb_read_mfgacc(int cmd, int block, uint8_t *data, int len)
* First two bytes returned are command sent,
* rest are actual data LSB to MSB.
*/
- rv = sb_read_string(block, data, len);
+ rv = sb_read_sized_block(block, data, len);
if (rv)
return rv;
if ((data[0] | data[1] << 8) != cmd)
diff --git a/include/battery_smart.h b/include/battery_smart.h
index b97d681223..9b373f7c8a 100644
--- a/include/battery_smart.h
+++ b/include/battery_smart.h
@@ -169,9 +169,22 @@
/* Read from battery */
int sb_read(int cmd, int *param);
-/* Read sequence from battery */
+/**
+ * Read null-terminated string from battery
+ * @param offset Battery register to read from
+ * @param data Buffer to hold the string
+ * @param len Length of data buffer
+ */
int sb_read_string(int offset, uint8_t *data, int len);
+/**
+ * Read sized block of data from battery
+ * @param offset Battery register to read from
+ * @param data Buffer to hold read data
+ * @param len Length of data buffer
+ */
+int sb_read_sized_block(int offset, uint8_t *data, int len);
+
/* Write to battery */
int sb_write(int cmd, int param);