battery: Refactor for coverage

Platforms that don't set CONFIG_SMBUS_PEC don't need a function call to
battery_supports_pec, nor do they need to set the high order address
bit. Coverage runs cannot fully excise/optimize out this logic, so it
reports a lower coverage due to an unreachable conditional. This
refactor corrects the unreachable code to be behind the CONFIG.

This is a derivative CL from jbettis' prototype in CL:4460056.
It additionally pulls the address logic into the macro/routine.

BRANCH=None
BUG=b:279215911
TEST=./twister -ivc -s drivers/drivers.default --coverage
  Check the coverage both before (239/271) and after (232/259).

Change-Id: Ida381e987c854bfb7cfc48fa6b20f67db9da9fba
Signed-off-by: Eric Peers <epeers@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4481407
Reviewed-by: Aaron Massey <aaronmassey@google.com>
Reviewed-by: Jeremy Bettis <jbettis@chromium.org>

1 files changed, 26 insertions, 17 deletions

diff --git a/driver/battery/smart.c b/driver/battery/smart.c
index b68e037133..b3d1787338 100644
--- a/driver/battery/smart.c
+++ b/driver/battery/smart.c
@@ -22,27 +22,39 @@
 static int fake_state_of_charge = -1;
 static int fake_temperature = -1;
 
-static int battery_supports_pec(void)
+#ifdef CONFIG_SMBUS_PEC
+static void addr_flags_for_pec(uint16_t *addr_flags)
 {
 	static int supports_pec = -1;
 
-	if (!IS_ENABLED(CONFIG_SMBUS_PEC))
-		return 0;
-
 	if (supports_pec < 0) {
 		int spec_info;
 		int rv = i2c_read16(I2C_PORT_BATTERY, BATTERY_ADDR_FLAGS,
 				    SB_SPECIFICATION_INFO, &spec_info);
-		/* failed, assuming not support and try again later */
+		/* failed, assuming not supported and try again later */
 		if (rv)
-			return 0;
+			return;
 
 		supports_pec = (BATTERY_SPEC_VERSION(spec_info) ==
 				BATTERY_SPEC_VER_1_1_WITH_PEC);
 		CPRINTS("battery supports pec: %d", supports_pec);
 	}
-	return supports_pec;
+
+	if (supports_pec) {
+		*addr_flags |= I2C_FLAG_PEC;
+	}
 }
+/* macro to avoid calling a routine when config is disabled */
+#define ADDR_FLAGS_FOR_PEC(ADDR_FLAGS) addr_flags_for_pec(ADDR_FLAGS)
+
+#else
+/*
+ * don't call it at all - this allows compiler optimization to prune
+ * PEC address code
+ */
+#define ADDR_FLAGS_FOR_PEC(ADDR_FLAGS)
+
+#endif
 
 test_mockable int sb_read(int cmd, int *param)
 {
@@ -62,9 +74,8 @@ test_mockable int sb_read(int cmd, int *param)
 	if (battery_is_cut_off())
 		return EC_RES_ACCESS_DENIED;
 #endif
-	if (battery_supports_pec())
-		addr_flags |= I2C_FLAG_PEC;
 
+	ADDR_FLAGS_FOR_PEC(&addr_flags);
 	return i2c_read16(I2C_PORT_BATTERY, addr_flags, cmd, param);
 }
 
@@ -79,8 +90,8 @@ test_mockable int sb_write(int cmd, int param)
 	if (battery_is_cut_off())
 		return EC_RES_ACCESS_DENIED;
 #endif
-	if (battery_supports_pec())
-		addr_flags |= I2C_FLAG_PEC;
+
+	ADDR_FLAGS_FOR_PEC(&addr_flags);
 
 	return i2c_write16(I2C_PORT_BATTERY, addr_flags, cmd, param);
 }
@@ -103,8 +114,8 @@ int sb_read_string(int offset, uint8_t *data, int len)
 	if (battery_is_cut_off())
 		return EC_RES_ACCESS_DENIED;
 #endif
-	if (battery_supports_pec())
-		addr_flags |= I2C_FLAG_PEC;
+
+	ADDR_FLAGS_FOR_PEC(&addr_flags);
 
 	return i2c_read_string(I2C_PORT_BATTERY, addr_flags, offset, data, len);
 }
@@ -129,8 +140,7 @@ int sb_read_sized_block(int offset, uint8_t *data, int len)
 			return EC_RES_ACCESS_DENIED;
 	}
 
-	if (battery_supports_pec())
-		addr_flags |= I2C_FLAG_PEC;
+	ADDR_FLAGS_FOR_PEC(&addr_flags);
 
 	return i2c_read_sized_block(I2C_PORT_BATTERY, addr_flags, offset, data,
 				    len, &read_len);
@@ -214,8 +224,7 @@ int sb_write_block(int reg, const uint8_t *val, int len)
 		return EC_RES_ACCESS_DENIED;
 #endif
 
-	if (battery_supports_pec())
-		addr_flags |= I2C_FLAG_PEC;
+	ADDR_FLAGS_FOR_PEC(&addr_flags);
 
 	/* TODO: implement smbus_write_block. */
 	return i2c_write_block(I2C_PORT_BATTERY, addr_flags, reg, val, len);