Add battery_get_params()

The charge state machine asks for all of this stuff at the same time
anyway.  Bundling it into a single function removes a number of
redundant (and painfully slow) I2C reads.

Also refactor the battery debug command so it doesn't have so many
local variables all in one function; it was consuming considerably
more stack space than any other debug command.

Spring still needs low-level access to the smart battery, so move the
two functions it needs directly into the Spring implementation.

BUG=chrome-os-partner:20881
BRANCH=none
TEST=charge/discharge rambi, pit and spring; watch debug messages and
     LED and output of 'battery' debug command.  All should behave the
     same as before.  Then run 'taskinfo' and see that the console task
     has at least 20 bytes unused.

Change-Id: I951b569542e28bbbb58853d62b57b0aaaf183e3f
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/177797

3 files changed, 93 insertions, 108 deletions

diff --git a/driver/battery/bq27541.c b/driver/battery/bq27541.c
index 1060a6ba13..72ec30b6fd 100644
--- a/driver/battery/bq27541.c
+++ b/driver/battery/bq27541.c
@@ -87,24 +87,9 @@ int battery_device_name(char *device_name, int buf_size)
 	return rv;
 }
 
-int battery_temperature(int *deci_kelvin)
-{
-	return bq27541_read(REG_TEMPERATURE, deci_kelvin);
-}
-
-int battery_voltage(int *voltage)
-{
-	return bq27541_read(REG_VOLTAGE, voltage);
-}
-
-int battery_state_of_charge(int *percent)
-{
-	return bq27541_read(REG_STATE_OF_CHARGE, percent);
-}
-
 int battery_state_of_charge_abs(int *percent)
 {
-	return battery_state_of_charge(percent);
+	return EC_ERROR_UNIMPLEMENTED;
 }
 
 int battery_remaining_capacity(int *capacity)
@@ -137,18 +122,6 @@ int battery_design_capacity(int *capacity)
 	return bq27541_read(REG_DESIGN_CAPACITY, capacity);
 }
 
-int battery_average_current(int *current)
-{
-	int rv = bq27541_read(REG_AVERAGE_CURRENT, current);
-	*current = (int)((int16_t)*current);
-	return rv;
-}
-
-int battery_current(int *current)
-{
-	return battery_average_current(current);
-}
-
 int battery_time_at_rate(int rate, int *minutes)
 {
 	int rv;
@@ -174,17 +147,18 @@ int battery_serial_number(int *serial)
 	return EC_ERROR_UNIMPLEMENTED;
 }
 
-int battery_desired_voltage(int *voltage)
-{
-	return EC_ERROR_UNIMPLEMENTED;
-}
-
 int battery_design_voltage(int *voltage)
 {
 	return EC_ERROR_UNIMPLEMENTED;
 }
 
-int battery_charging_allowed(int *allowed)
+/**
+ * Check if battery allows charging.
+ *
+ * @param allowed	Non-zero if charging allowed; zero if not allowed.
+ * @return non-zero if error.
+ */
+static int battery_charging_allowed(int *allowed)
 {
 	int rv, val;
 
@@ -195,11 +169,6 @@ int battery_charging_allowed(int *allowed)
 	return EC_SUCCESS;
 }
 
-int battery_desired_current(int *current)
-{
-	return EC_ERROR_UNIMPLEMENTED;
-}
-
 int battery_get_mode(int *mode)
 {
 	return EC_ERROR_UNIMPLEMENTED;
@@ -222,3 +191,45 @@ int battery_set_10mw_mode(int enabled)
 	/* Not supported by this battery chip */
 	return EC_ERROR_INVAL;
 }
+
+void battery_get_params(struct batt_params *batt)
+{
+	int v;
+
+	/* Reset flags */
+	batt->flags = 0;
+
+	if (bq27541_read(REG_TEMPERATURE, &batt->temperature))
+		batt->flags |= BATT_FLAG_BAD_ANY;
+	else
+		batt->flags |= BATT_FLAG_RESPONSIVE; /* Battery is responding */
+
+	if (bq27541_read(REG_STATE_OF_CHARGE, &batt->state_of_charge))
+		batt->flags |= BATT_FLAG_BAD_ANY | BATT_FLAG_BAD_CHARGE_PERCENT;
+
+	if (bq27541_read(REG_VOLTAGE, &batt->voltage))
+		batt->flags |= BATT_FLAG_BAD_ANY | BATT_FLAG_BAD_VOLTAGE;
+
+	v = 0;
+	if (bq27541_read(REG_AVERAGE_CURRENT, &v))
+		batt->flags |= BATT_FLAG_BAD_ANY;
+	batt->current = (int16_t)v;
+
+	/* Default to not desiring voltage and current */
+	batt->desired_voltage = batt->desired_current = 0;
+
+	v = 0;
+	if (battery_charging_allowed(&v)) {
+		batt->flags |= BATT_FLAG_BAD_ANY;
+	} else if (v) {
+		batt->flags |= BATT_FLAG_WANT_CHARGE;
+
+		/*
+		 * Desired voltage and current are not provided by the battery.
+		 * So ask for battery's max voltage and an arbitrarily large
+		 * current.
+		 */
+		batt->desired_voltage = battery_get_info()->voltage_max;
+		batt->desired_current = 99999;
+	}
+}
diff --git a/driver/battery/link.c b/driver/battery/link.c
index 5f5cdf8dc0..891e555922 100644
--- a/driver/battery/link.c
+++ b/driver/battery/link.c
@@ -109,6 +109,7 @@ void battery_vendor_params(struct batt_params *batt)
 	/* Don't charge if outside of allowable temperature range */
 	if (bat_temp_c >= bat_temp_ranges.charging_max_c ||
 	    bat_temp_c < bat_temp_ranges.charging_min_c) {
+		batt->flags &= ~BATT_FLAG_WANT_CHARGE;
 		batt->desired_voltage = 0;
 		batt->desired_current = 0;
 		return;
diff --git a/driver/battery/smart.c b/driver/battery/smart.c
index 87105dfab2..7feca24bda 100644
--- a/driver/battery/smart.c
+++ b/driver/battery/smart.c
@@ -10,6 +10,7 @@
 #include "host_command.h"
 #include "i2c.h"
 #include "timer.h"
+#include "util.h"
 
 test_mockable int sbc_read(int cmd, int *param)
 {
@@ -64,21 +65,6 @@ int battery_set_10mw_mode(int enabled)
 	return battery_set_mode(val);
 }
 
-int battery_temperature(int *deci_kelvin)
-{
-	return sb_read(SB_TEMPERATURE, deci_kelvin);
-}
-
-int battery_voltage(int *voltage)
-{
-	return sb_read(SB_VOLTAGE, voltage);
-}
-
-int battery_state_of_charge(int *percent)
-{
-	return sb_read(SB_RELATIVE_STATE_OF_CHARGE, percent);
-}
-
 int battery_state_of_charge_abs(int *percent)
 {
 	return sb_read(SB_ABSOLUTE_STATE_OF_CHARGE, percent);
@@ -109,31 +95,6 @@ int battery_time_to_full(int *minutes)
 	return sb_read(SB_AVERAGE_TIME_TO_FULL, minutes);
 }
 
-int battery_desired_current(int *current)
-{
-	return sb_read(SB_CHARGING_CURRENT, current);
-}
-
-int battery_desired_voltage(int *voltage)
-{
-	return sb_read(SB_CHARGING_VOLTAGE, voltage);
-}
-
-int battery_charging_allowed(int *allowed)
-{
-	int v, c, rv;
-
-	/*
-	 * TODO(crosbug.com/p/23811): This re-reads the battery current and
-	 * voltage, which is silly because charge_state.c just read them.
-	 */
-	rv = battery_desired_voltage(&v) | battery_desired_current(&c);
-	if (rv)
-		return rv;
-	*allowed = (v != 0) && (c != 0);
-	return EC_SUCCESS;
-}
-
 /* Read battery status */
 int battery_status(int *status)
 {
@@ -168,35 +129,6 @@ int battery_serial_number(int *serial)
 	return sb_read(SB_SERIAL_NUMBER, serial);
 }
 
-/* Read battery discharging current
- * unit: mA
- * negative value: charging
- */
-int battery_current(int *current)
-{
-	int rv, d;
-
-	rv = sb_read(SB_CURRENT, &d);
-	if (rv)
-		return rv;
-
-	*current = (int16_t)d;
-	return EC_SUCCESS;
-}
-
-
-int battery_average_current(int *current)
-{
-	int rv, d;
-
-	rv = sb_read(SB_AVERAGE_CURRENT, &d);
-	if (rv)
-		return rv;
-
-	*current = (int16_t)d;
-	return EC_SUCCESS;
-}
-
 test_mockable int battery_time_at_rate(int rate, int *minutes)
 {
 	int rv;
@@ -278,6 +210,47 @@ test_mockable int battery_device_chemistry(char *dest, int size)
 			       SB_DEVICE_CHEMISTRY, dest, size);
 }
 
+void battery_get_params(struct batt_params *batt)
+{
+	int v;
+
+	/* Reset battery parameters */
+	memset(batt, 0, sizeof(*batt));
+
+	if (sb_read(SB_TEMPERATURE, &batt->temperature))
+		batt->flags |= BATT_FLAG_BAD_ANY;
+	else
+		batt->flags |= BATT_FLAG_RESPONSIVE; /* Battery is responding */
+
+	if (sb_read(SB_RELATIVE_STATE_OF_CHARGE, &batt->state_of_charge))
+		batt->flags |= BATT_FLAG_BAD_ANY | BATT_FLAG_BAD_CHARGE_PERCENT;
+
+	if (sb_read(SB_VOLTAGE, &batt->voltage))
+		batt->flags |= BATT_FLAG_BAD_ANY | BATT_FLAG_BAD_VOLTAGE;
+
+	v = 0;
+	if (sb_read(SB_CURRENT, &v))
+		batt->flags |= BATT_FLAG_BAD_ANY;
+	else
+		batt->current = (int16_t)v;
+
+	if (sb_read(SB_CHARGING_VOLTAGE, &batt->desired_voltage) ||
+	    sb_read(SB_CHARGING_CURRENT, &batt->desired_current))
+		batt->flags |= BATT_FLAG_BAD_ANY;
+
+	/*
+	 * Charging allowed if both desired voltage and current are nonzero
+	 * and battery isn't full.
+	 */
+	if (batt->desired_voltage && batt->desired_current &&
+	    batt->state_of_charge < BATTERY_LEVEL_FULL) {
+		batt->flags |= BATT_FLAG_WANT_CHARGE;
+	} else {
+		/* Force both to zero */
+		batt->desired_voltage = batt->desired_current = 0;
+	}
+}
+
 /*****************************************************************************/
 /* Smart battery pass-through
  */