SBS: Make battery_get_params clear flags selectively

Currently, battery_get_params clear all flags in batt.flags. This patch
makes the function clear only the flags which are explicitly set or
unset by battery_get_params. With this change, the charger task can add
a flag to batt.flags.

There isn't yet functionality change by this patch.

BUG=b:263921114
BRANCH=None
TEST=run-battery_get_params_smart run-sbs_charging_v2

Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Change-Id: I5708ab6de81bc0a7c28961b13960fd89460b2e1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4136966
Code-Coverage: Zoss <zoss-cl-coverage@prod.google.com>
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Bob Moragues <moragues@chromium.org>
(cherry picked from commit 9e73d7d01bccf2cf87e01abb1dd1a4f216c569e4)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4164177
Reviewed-by: Wai-Hong Tam <waihong@google.com>
Commit-Queue: Bob Moragues <moragues@chromium.org>
Tested-by: Bob Moragues <moragues@chromium.org>

3 files changed, 46 insertions, 30 deletions

diff --git a/driver/battery/smart.c b/driver/battery/smart.c
index c84aa87b85..868ef49733 100644
--- a/driver/battery/smart.c
+++ b/driver/battery/smart.c
@@ -360,6 +360,34 @@ static void apply_fake_state_of_charge(struct batt_params *batt)
 	batt->flags &= ~BATT_FLAG_BAD_REMAINING_CAPACITY;
 }
 
+static bool battery_want_charge(struct batt_params *batt)
+{
+	if (batt->flags &
+	    (BATT_FLAG_BAD_DESIRED_VOLTAGE | BATT_FLAG_BAD_DESIRED_CURRENT |
+	     BATT_FLAG_BAD_STATE_OF_CHARGE))
+		return false;
+
+	/*
+	 * Charging allowed if both desired voltage and current are nonzero
+	 * and battery isn't full (and we read them all correctly).
+	 */
+	if (batt->desired_voltage && batt->desired_current &&
+	    batt->state_of_charge < BATTERY_LEVEL_FULL)
+		return true;
+
+	/*
+	 * TODO (crosbug.com/p/29467): remove this workaround for dead battery
+	 * that requests no voltage/current
+	 */
+	if (IS_ENABLED(CONFIG_BATTERY_REQUESTS_NIL_WHEN_DEAD)) {
+		if (batt->desired_voltage == 0 && batt->desired_current == 0 &&
+		    batt->state_of_charge == 0)
+			return true;
+	}
+
+	return false;
+}
+
 void battery_get_params(struct batt_params *batt)
 {
 	struct batt_params batt_new;
@@ -371,7 +399,7 @@ void battery_get_params(struct batt_params *batt)
 	 * will be preserved.
 	 */
 	memcpy(&batt_new, batt, sizeof(*batt));
-	batt_new.flags = 0;
+	batt_new.flags &= ~BATT_FLAG_VOLATILE;
 
 	if (sb_read(SB_TEMPERATURE, &batt_new.temperature)
 			&& fake_temperature < 0)
@@ -393,9 +421,9 @@ void battery_get_params(struct batt_params *batt)
 		batt_new.flags |= BATT_FLAG_BAD_CURRENT;
 	else
 		batt_new.current = (int16_t)v;
-
 	if (sb_read(SB_AVERAGE_CURRENT, &v))
 		batt_new.flags |= BATT_FLAG_BAD_AVERAGE_CURRENT;
+
 	if (sb_read(SB_CHARGING_VOLTAGE, &batt_new.desired_voltage))
 		batt_new.flags |= BATT_FLAG_BAD_DESIRED_VOLTAGE;
 
@@ -432,33 +460,7 @@ void battery_get_params(struct batt_params *batt)
 		batt_new.is_present = BP_NOT_SURE;
 #endif
 
-	/*
-	 * Charging allowed if both desired voltage and current are nonzero
-	 * and battery isn't full (and we read them all correctly).
-	 */
-	if (!(batt_new.flags & (BATT_FLAG_BAD_DESIRED_VOLTAGE |
-				BATT_FLAG_BAD_DESIRED_CURRENT |
-				BATT_FLAG_BAD_STATE_OF_CHARGE)) &&
-#ifdef CONFIG_BATTERY_REQUESTS_NIL_WHEN_DEAD
-		/*
-		 * TODO (crosbug.com/p/29467): remove this workaround
-		 * for dead battery that requests no voltage/current
-		 */
-		((batt_new.desired_voltage &&
-			batt_new.desired_current &&
-			batt_new.state_of_charge < BATTERY_LEVEL_FULL) ||
-		(batt_new.desired_voltage == 0 &&
-			batt_new.desired_current == 0 &&
-#ifdef CONFIG_BATTERY_DEAD_UNTIL_VALUE
-			batt_new.state_of_charge < CONFIG_BATTERY_DEAD_UNTIL_VALUE)))
-#else
-			batt_new.state_of_charge == 0)))
-#endif
-#else
-	    batt_new.desired_voltage &&
-	    batt_new.desired_current &&
-	    batt_new.state_of_charge < BATTERY_LEVEL_FULL)
-#endif
+	if (battery_want_charge(&batt_new))
 		batt_new.flags |= BATT_FLAG_WANT_CHARGE;
 	else
 		/* Force both to zero */
diff --git a/include/battery.h b/include/battery.h
index 75a7d48da0..6d3c92a8a5 100644
--- a/include/battery.h
+++ b/include/battery.h
@@ -142,7 +142,11 @@ int battery_get_avg_voltage(void); /* in mV */
 #define BATT_FLAG_IMBALANCED_CELL		0x00000800
 #define BATT_FLAG_BAD_AVERAGE_CURRENT		0x00001000
 /* All of the above BATT_FLAG_BAD_* bits */
-#define BATT_FLAG_BAD_ANY			0x000017fc
+#define BATT_FLAG_BAD_ANY 0x000017fc
+/* Flags which are set or unset on every access (via battery_get_params) */
+#define BATT_FLAG_VOLATILE                                                  \
+	(BATT_FLAG_BAD_ANY | BATT_FLAG_WANT_CHARGE | BATT_FLAG_RESPONSIVE | \
+	 BATT_FLAG_IMBALANCED_CELL)
 
 /* Battery constants */
 struct battery_info {
diff --git a/test/battery_get_params_smart.c b/test/battery_get_params_smart.c
index 11e6a3cfe8..6122b8d2e9 100644
--- a/test/battery_get_params_smart.c
+++ b/test/battery_get_params_smart.c
@@ -163,6 +163,16 @@ static int test_flags(void)
 	test_flag(SB_BATTERY_STATUS, BATT_FLAG_BAD_STATUS);
 
 	/*
+	 * Volatile flags should be cleared and other flags should be preserved.
+	 */
+	reset_and_fail_on(0, 0, -1);
+	batt.flags |= BATT_FLAG_BAD_TEMPERATURE;
+	batt.flags |= BIT(31);
+	battery_get_params(&batt);
+	TEST_ASSERT(batt.flags & BIT(31));
+	TEST_ASSERT(!(batt.flags & BATT_FLAG_BAD_ANY));
+
+	/*
 	 * All reads succeed. BATT_FLAG_RESPONSIVE should be set. Then, all
 	 * reads fail. BATT_FLAG_RESPONSIVE should be cleared.
 	 */