From 9dd6dabc8fcb85bc80bccd5c0f387b85e174ce61 Mon Sep 17 00:00:00 2001 From: Eric Peers Date: Wed, 26 Apr 2023 16:21:33 -0600 Subject: 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4481407 Reviewed-by: Aaron Massey Reviewed-by: Jeremy Bettis --- driver/battery/smart.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) (limited to 'driver/battery/smart.c') 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); -- cgit v1.2.1