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

13 files changed, 291 insertions, 308 deletions

diff --git a/board/nyan/battery.c b/board/nyan/battery.c
index df29c7fd9b..12911de286 100644
--- a/board/nyan/battery.c
+++ b/board/nyan/battery.c
@@ -102,6 +102,7 @@ void battery_vendor_params(struct batt_params *batt)
 	    bat_temp_c < bat_temp_ranges.charging_min_c) {
 		batt->desired_voltage = 0;
 		batt->desired_current = 0;
+		batt->flags &= ~BATT_FLAG_WANT_CHARGE;
 		return;
 	}
 
diff --git a/board/spring/led.c b/board/spring/led.c
index 4338911237..cbb1fd9ab9 100644
--- a/board/spring/led.c
+++ b/board/spring/led.c
@@ -6,6 +6,7 @@
  */
 
 #include "battery.h"
+#include "battery_smart.h"
 #include "common.h"
 #include "driver/led/lp5562.h"
 #include "extpower.h"
@@ -70,6 +71,14 @@ static int set_led_color(enum led_state_t state)
 	return rv;
 }
 
+/**
+ * Directly read state of charge (0-100) of battery.
+ */
+static int battery_state_of_charge(int *percent)
+{
+	return sb_read(SB_RELATIVE_STATE_OF_CHARGE, percent);
+}
+
 /*****************************************************************************/
 /* Host commands */
 
diff --git a/common/battery.c b/common/battery.c
index 0d47378f94..140b7d22c7 100644
--- a/common/battery.c
+++ b/common/battery.c
@@ -31,19 +31,16 @@ static int check_print_error(int rv)
 	return rv == EC_SUCCESS;
 }
 
-static int print_battery_info(void)
+static void print_battery_status(void)
 {
-	int value;
-	int hour, minute;
-	char text[32];
-	const char *unit;
+	static const char * const st[] = {"EMPTY", "FULL", "DCHG", "INIT",};
+	static const char * const al[] =
+		{"RT", "RC", "--", "TD", "OT", "--", "TC", "OC"};
+
+	int value, i;
 
 	print_item_name("Status:");
 	if (check_print_error(battery_status(&value))) {
-		const char * const st[] = {"EMPTY", "FULL", "DCHG", "INIT",};
-		const char * const al[] = {"RT", "RC", "--", "TD",
-					   "OT", "--", "TC", "OC"};
-		int i;
 		ccprintf("0x%04x", value);
 
 		/* bits 0-3 are only valid when the previous transaction
@@ -61,11 +58,11 @@ static int print_battery_info(void)
 
 		ccprintf("\n");
 	}
+}
 
-	print_item_name("Temp:");
-	if (check_print_error(battery_temperature(&value)))
-		ccprintf("0x%04x = %.1d K (%.1d C)\n",
-			 value, value, value - 2731);
+static void print_battery_strings(void)
+{
+	char text[32];
 
 	print_item_name("Manuf:");
 	if (check_print_error(battery_manufacturer_name(text, sizeof(text))))
@@ -78,36 +75,60 @@ static int print_battery_info(void)
 	print_item_name("Chem:");
 	if (check_print_error(battery_device_chemistry(text, sizeof(text))))
 		ccprintf("%s\n", text);
+}
 
-	print_item_name("Serial:");
-	if (check_print_error(battery_serial_number(&value)))
-		ccprintf("0x%04x\n", value);
+static void print_battery_params(void)
+{
+	struct batt_params batt;
+
+	battery_get_params(&batt);
+	print_item_name("Param flags:");
+	ccprintf("%08x\n", batt.flags);
+
+	print_item_name("Temp:");
+	ccprintf("0x%04x = %.1d K (%.1d C)\n",
+		 batt.temperature, batt.temperature, batt.temperature - 2731);
 
 	print_item_name("V:");
-	if (check_print_error(battery_voltage(&value)))
-		ccprintf("0x%04x = %d mV\n", value, value);
+	ccprintf("0x%04x = %d mV\n", batt.voltage, batt.voltage);
 
 	print_item_name("V-desired:");
-	if (check_print_error(battery_desired_voltage(&value)))
-		ccprintf("0x%04x = %d mV\n", value, value);
-
-	print_item_name("V-deisgn:");
-	if (check_print_error(battery_design_voltage(&value)))
-		ccprintf("0x%04x = %d mV\n", value, value);
+	ccprintf("0x%04x = %d mV\n", batt.desired_voltage,
+		 batt.desired_voltage);
 
 	print_item_name("I:");
-	if (check_print_error(battery_current(&value))) {
-		ccprintf("0x%04x = %d mA", value & 0xffff, value);
-		if (value > 0)
-			ccputs("(CHG)");
-		else if (value < 0)
-			ccputs("(DISCHG)");
-		ccputs("\n");
-	}
+	ccprintf("0x%04x = %d mA", batt.current & 0xffff, batt.current);
+	if (batt.current > 0)
+		ccputs("(CHG)");
+	else if (batt.current < 0)
+		ccputs("(DISCHG)");
+	ccputs("\n");
 
 	print_item_name("I-desired:");
-	if (check_print_error(battery_desired_current(&value)))
-		ccprintf("0x%04x = %d mA\n", value, value);
+	ccprintf("0x%04x = %d mA\n", batt.desired_current,
+		 batt.desired_current);
+
+	print_item_name("Charging:");
+	ccprintf("%sAllowed\n",
+		 batt.flags & BATT_FLAG_WANT_CHARGE ? "" : "Not ");
+
+	print_item_name("Charge:");
+		ccprintf("%d %%\n", batt.state_of_charge);
+}
+
+static void print_battery_info(void)
+{
+	int value;
+	int hour, minute;
+	const char *unit;
+
+	print_item_name("Serial:");
+	if (check_print_error(battery_serial_number(&value)))
+		ccprintf("0x%04x\n", value);
+
+	print_item_name("V-design:");
+	if (check_print_error(battery_design_voltage(&value)))
+		ccprintf("0x%04x = %d mV\n", value, value);
 
 	print_item_name("Mode:");
 	if (check_print_error(battery_get_mode(&value)))
@@ -116,15 +137,7 @@ static int print_battery_info(void)
 	battery_is_in_10mw_mode(&value);
 	unit = value ? "0 mW" : " mAh";
 
-	print_item_name("Charging:");
-	if (check_print_error(battery_charging_allowed(&value)))
-		ccprintf("%sAllowed\n", value ? "" : "Not ");
-
-	print_item_name("Charge:");
-	if (check_print_error(battery_state_of_charge(&value)))
-		ccprintf("%d %%\n", value);
-
-	print_item_name("Abs:");
+	print_item_name("Abs charge:");
 	if (check_print_error(battery_state_of_charge_abs(&value)))
 		ccprintf("%d %%\n", value);
 
@@ -163,14 +176,11 @@ static int print_battery_info(void)
 		}
 		ccprintf("%dh:%d\n", hour, minute);
 	}
-
-	return 0;
 }
 
 static int command_battery(int argc, char **argv)
 {
 	int repeat = 1;
-	int rv = 0;
 	int loop;
 	int sleep_ms = 0;
 	char *e;
@@ -192,7 +202,10 @@ static int command_battery(int argc, char **argv)
 	}
 
 	for (loop = 0; loop < repeat; loop++) {
-		rv = print_battery_info();
+		print_battery_status();
+		print_battery_params();
+		print_battery_strings();
+		print_battery_info();
 
 		/*
 		 * Running with a high repeat count will take so long the
@@ -203,15 +216,9 @@ static int command_battery(int argc, char **argv)
 
 		if (sleep_ms)
 			msleep(sleep_ms);
-
-		if (rv)
-			break;
 	}
 
-	if (rv)
-		ccprintf("Failed - error %d\n", rv);
-
-	return rv ? EC_ERROR_UNKNOWN : EC_SUCCESS;
+	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(battery, command_battery,
 			"<repeat_count> <sleep_ms>",
diff --git a/common/charge_state.c b/common/charge_state.c
index 8b7f0e9919..8f93248c05 100644
--- a/common/charge_state.c
+++ b/common/charge_state.c
@@ -255,9 +255,9 @@ static int state_common(struct power_state_context *ctx)
 	}
 #endif /* CONFIG_BATTERY_CHECK_CONNECTED */
 
-	/* Read temperature and see if battery is responsive */
-	rv = battery_temperature(&batt->temperature);
-	if (rv) {
+	/* Read params and see if battery is responsive */
+	battery_get_params(batt);
+	if (!(batt->flags & BATT_FLAG_RESPONSIVE)) {
 		/* Check low battery condition and retry */
 		if (curr->ac && ctx->battery_responsive &&
 		    !(curr->error & F_CHARGER_MASK)) {
@@ -271,8 +271,8 @@ static int state_common(struct power_state_context *ctx)
 				       ctx->battery->precharge_current);
 			for (d = 0; d < 30; d++) {
 				sleep(1);
-				rv = battery_temperature(&batt->temperature);
-				if (rv == 0) {
+				battery_get_params(batt);
+				if (batt->flags & BATT_FLAG_RESPONSIVE) {
 					ctx->battery_responsive = 1;
 					break;
 				}
@@ -280,7 +280,7 @@ static int state_common(struct power_state_context *ctx)
 		}
 
 		/* Set error if battery is still unresponsive */
-		if (rv) {
+		if (!(batt->flags & BATT_FLAG_RESPONSIVE)) {
 			curr->error |= F_BATTERY_UNRESPONSIVE;
 			return curr->error;
 		}
@@ -288,40 +288,25 @@ static int state_common(struct power_state_context *ctx)
 		ctx->battery_responsive = 1;
 	}
 
-	if (battery_voltage(&batt->voltage))
+	/* Translate flags */
+	if (batt->flags & BATT_FLAG_BAD_ANY)
+		curr->error |= F_BATTERY_GET_PARAMS;
+	if (batt->flags & BATT_FLAG_BAD_VOLTAGE)
 		curr->error |= F_BATTERY_VOLTAGE;
+	if (batt->flags & BATT_FLAG_BAD_CHARGE_PERCENT)
+		curr->error |= F_BATTERY_STATE_OF_CHARGE;
+
 	*ctx->memmap_batt_volt = batt->voltage;
 
-	if (battery_current(&batt->current))
-		curr->error |= F_BATTERY_CURRENT;
 	/* Memory mapped value: discharge rate */
 	*ctx->memmap_batt_rate = batt->current < 0 ?
 		-batt->current : batt->current;
 
-	if (battery_charging_allowed(&d)) {
-		curr->error |= F_DESIRED_VOLTAGE | F_DESIRED_CURRENT;
-	} else if (d) {
-		rv = battery_desired_voltage(&batt->desired_voltage);
-		if (rv == EC_ERROR_UNIMPLEMENTED)
-			batt->desired_voltage = MIN(ctx->charger->voltage_max,
-						    ctx->battery->voltage_max);
-		else if (rv != EC_SUCCESS)
-			curr->error |= F_DESIRED_VOLTAGE;
-
-		rv = battery_desired_current(&batt->desired_current);
-		if (rv == EC_ERROR_UNIMPLEMENTED)
-			batt->desired_current = ctx->charger->current_max;
-		else if (rv != EC_SUCCESS)
-			curr->error |= F_DESIRED_CURRENT;
-	} else { /* Charging not allowed */
-		batt->desired_voltage = 0;
-		batt->desired_current = 0;
-	}
-
-	if (fake_state_of_charge >= 0)
+	/* Fake state of charge if necessary */
+	if (fake_state_of_charge >= 0) {
 		batt->state_of_charge = fake_state_of_charge;
-	else if (battery_state_of_charge(&batt->state_of_charge))
-		curr->error |= F_BATTERY_STATE_OF_CHARGE;
+		curr->error &= ~F_BATTERY_STATE_OF_CHARGE;
+	}
 
 	if (batt->state_of_charge != prev->batt.state_of_charge) {
 		rv = battery_full_charge_capacity(&d);
@@ -458,7 +443,7 @@ static enum power_state state_idle(struct power_state_context *ctx)
 		return PWR_STATE_UNCHANGE;
 
 	/* Configure init charger state and switch to charge state */
-	if (batt->desired_voltage && batt->desired_current) {
+	if (batt->flags & BATT_FLAG_WANT_CHARGE) {
 		int want_current =
 			charger_closest_current(batt->desired_current);
 
@@ -497,7 +482,10 @@ static enum power_state state_charge(struct power_state_context *ctx)
 	if (curr->error)
 		return PWR_STATE_ERROR;
 
-	/* Check charger reset */
+	/*
+	 * Some chargers will reset out from underneath us.  If this happens,
+	 * reinitialize charging.
+	 */
 	if (curr->charging_voltage == 0 ||
 	    curr->charging_current == 0)
 		return PWR_STATE_REINIT;
@@ -505,7 +493,8 @@ static enum power_state state_charge(struct power_state_context *ctx)
 	if (!curr->ac)
 		return PWR_STATE_REINIT;
 
-	if (batt->state_of_charge >= BATTERY_LEVEL_FULL) {
+	/* Stop charging if charging is no longer allowed */
+	if (!(batt->flags & BATT_FLAG_WANT_CHARGE)) {
 		if (charge_request(0, 0))
 			return PWR_STATE_ERROR;
 		return PWR_STATE_IDLE;
diff --git a/common/charger.c b/common/charger.c
index 15bdf8f73c..ebac3896c1 100644
--- a/common/charger.c
+++ b/common/charger.c
@@ -18,9 +18,20 @@
 
 int charger_closest_voltage(int voltage)
 {
-	const struct charger_info *info;
+	const struct charger_info *info = charger_get_info();
+
+	/*
+	 * If the requested voltage is non-zero but below our minimum,
+	 * return the minimum.  See crosbug.com/p/8662.
+	 */
+	if (voltage > 0 && voltage < info->voltage_min)
+		return info->voltage_min;
+
+	/* Clip to max */
+	if (voltage > info->voltage_max)
+		return info->voltage_max;
 
-	info = charger_get_info();
+	/* Otherwise round down to nearest voltage step */
 	return voltage - (voltage % info->voltage_step);
 }
 
diff --git a/common/extpower_spring.c b/common/extpower_spring.c
index 22b52ce80b..2e11e30d47 100644
--- a/common/extpower_spring.c
+++ b/common/extpower_spring.c
@@ -8,6 +8,7 @@
 #include "adc.h"
 #include "adc_chip.h"
 #include "battery.h"
+#include "battery_smart.h"
 #include "chipset.h"
 #include "clock.h"
 #include "console.h"
@@ -164,6 +165,23 @@ static enum {
 } charger_need_redetect = NO_REDETECT;
 static timestamp_t charger_redetection_time;
 
+/**
+ * Directly read discharge current in mA; negative = charging.
+ */
+static int battery_current(int *current)
+{
+	int rv, d;
+
+	rv = sb_read(SB_CURRENT, &d);
+	if (rv) {
+		*current = 0;
+		return rv;
+	}
+
+	*current = (int16_t)d;
+	return EC_SUCCESS;
+}
+
 static int get_video_power(void)
 {
 	return video_power_enabled;
@@ -895,7 +913,10 @@ DECLARE_CONSOLE_COMMAND(ilim, command_ilim,
 #ifdef CONFIG_CMD_BATDEBUG
 static int command_batdebug(int argc, char **argv)
 {
-	int val;
+	struct batt_params batt;
+
+	battery_get_params(&batt);
+
 	ccprintf("VBUS = %d mV\n", adc_read_channel(ADC_CH_USB_VBUS_SNS));
 	ccprintf("VAC = %d mV\n", pmu_adc_read(ADC_VAC, ADC_FLAG_KEEP_ON)
 				  * 17000 / 1024);
@@ -906,10 +927,8 @@ static int command_batdebug(int argc, char **argv)
 	ccprintf("IBAT = %d mA\n", pmu_adc_read(ADC_IBAT, 0)
 				  * (1000 / R_BATTERY_MOHM) * 40 / 1024);
 	ccprintf("PWM = %d%%\n", pwm_get_duty(PWM_CH_ILIM));
-	battery_current(&val);
-	ccprintf("Battery Current = %d mA\n", val);
-	battery_voltage(&val);
-	ccprintf("Battery Voltage= %d mV\n", val);
+	ccprintf("Battery Current = %d mA\n", batt.current);
+	ccprintf("Battery Voltage= %d mV\n", batt.voltage);
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(batdebug, command_batdebug,
diff --git a/common/pmu_tps65090_charger.c b/common/pmu_tps65090_charger.c
index 68bca0a396..974d0b4b0f 100644
--- a/common/pmu_tps65090_charger.c
+++ b/common/pmu_tps65090_charger.c
@@ -158,7 +158,10 @@ static int rsoc_moving_average(int state_of_charge)
 
 static int calc_next_state(int state)
 {
-	int batt_temp, alarm, capacity, charge;
+	struct batt_params batt;
+	int alarm;
+
+	battery_get_params(&batt);
 
 	switch (state) {
 	case ST_IDLE0:
@@ -176,13 +179,13 @@ static int calc_next_state(int state)
 			return ST_BAD_COND;
 
 		/* Enable charging when battery doesn't respond */
-		if (battery_temperature(&batt_temp))
+		if (!(batt.flags & BATT_FLAG_RESPONSIVE))
 			return ST_PRE_CHARGING;
 
 		/* Turn off charger when battery temperature is out
 		 * of the start charging range.
 		 */
-		if (!battery_start_charging_range(batt_temp))
+		if (!battery_start_charging_range(batt.temperature))
 			return ST_BAD_COND;
 
 		/* Turn off charger on battery over temperature alarm */
@@ -194,8 +197,8 @@ static int calc_next_state(int state)
 			return ST_IDLE;
 
 		/* Start charging only when battery charge lower than 100% */
-		if (!battery_state_of_charge(&charge)) {
-			if (charge < 100)
+		if (!(batt.flags & BATT_FLAG_BAD_CHARGE_PERCENT)) {
+			if (batt.state_of_charge < 100)
 				return ST_CHARGING;
 		}
 
@@ -205,14 +208,15 @@ static int calc_next_state(int state)
 		if (!extpower_is_present())
 			return ST_IDLE0;
 
-		/* If the battery goes online after enable the charger,
-		 * go into charging state.
+		/*
+		 * If the battery goes online after enabling the charger, go
+		 * into charging state.
 		 */
-		if (battery_temperature(&batt_temp) == EC_SUCCESS) {
-			if (!battery_start_charging_range(batt_temp))
+		if (batt.flags & BATT_FLAG_RESPONSIVE) {
+			if (!battery_start_charging_range(batt.temperature))
 				return ST_IDLE0;
-			if (!battery_state_of_charge(&charge)) {
-				if (charge >= 100)
+			if (!(batt.flags & BATT_FLAG_BAD_CHARGE_PERCENT)) {
+				if (batt.state_of_charge >= 100)
 					return ST_IDLE0;
 			}
 			return ST_CHARGING;
@@ -229,14 +233,14 @@ static int calc_next_state(int state)
 		 * Disable charging on battery access error, or charging
 		 * temperature out of range.
 		 */
-		if (battery_temperature(&batt_temp)) {
+		if (!(batt.flags & BATT_FLAG_RESPONSIVE)) {
 			CPUTS("[pmu] charging: unable to get battery "
 			      "temperature\n");
 			return ST_IDLE0;
-		} else if (!battery_charging_range(batt_temp)) {
+		} else if (!battery_charging_range(batt.temperature)) {
 			CPRINTF("[pmu] charging: temperature out of range "
 				"%dC\n",
-				DECI_KELVIN_TO_CELSIUS(batt_temp));
+				DECI_KELVIN_TO_CELSIUS(batt.temperature));
 			return ST_CHARGING_ERROR;
 		}
 
@@ -292,10 +296,10 @@ static int calc_next_state(int state)
 			if (alarm & ALARM_OVER_TEMP)
 				return ST_CHARGING_ERROR;
 
-			if (battery_temperature(&batt_temp))
+			if (!(batt.flags & BATT_FLAG_RESPONSIVE))
 				return ST_CHARGING_ERROR;
 
-			if (!battery_charging_range(batt_temp))
+			if (!battery_charging_range(batt.temperature))
 				return ST_CHARGING_ERROR;
 
 			return ST_CHARGING;
@@ -314,11 +318,12 @@ static int calc_next_state(int state)
 			return ST_IDLE0;
 
 		/* Check battery discharging temperature range */
-		if (battery_temperature(&batt_temp) == 0) {
-			if (!battery_discharging_range(batt_temp)) {
+		if (batt.flags & BATT_FLAG_RESPONSIVE) {
+			if (!battery_discharging_range(batt.temperature)) {
 				CPRINTF("[pmu] discharging: temperature out of"
 					"range %dC\n",
-					DECI_KELVIN_TO_CELSIUS(batt_temp));
+					DECI_KELVIN_TO_CELSIUS(
+							batt.temperature));
 				return system_off();
 			}
 		}
@@ -329,14 +334,14 @@ static int calc_next_state(int state)
 			return system_off();
 		}
 		/* Check remaining charge % */
-		if (battery_state_of_charge(&capacity) == 0) {
+		if (!(batt.flags & BATT_FLAG_BAD_CHARGE_PERCENT)) {
 			/*
 			 * Shutdown AP when state of charge < 1.5%.
 			 * Moving average is rounded to integer.
 			 */
-			if (rsoc_moving_average(capacity) < 2) {
+			if (rsoc_moving_average(batt.state_of_charge) < 2) {
 				return system_off();
-			} else if (capacity < 4) {
+			} else if (batt.state_of_charge < 4) {
 				notify_battery_low();
 			}
 		}
diff --git a/common/pmu_tps65090_powerinfo.c b/common/pmu_tps65090_powerinfo.c
index 30fe53fa4a..e53e49a660 100644
--- a/common/pmu_tps65090_powerinfo.c
+++ b/common/pmu_tps65090_powerinfo.c
@@ -126,7 +126,6 @@ DECLARE_CONSOLE_COMMAND(powerinfo, command_powerinfo,
  */
 static int power_command_info(struct host_cmd_handler_args *args)
 {
-	int bat_charging_current;
 	struct ec_response_power_info *r = args->response;
 
 	r->voltage_ac = calc_voltage(
@@ -140,14 +139,17 @@ static int power_command_info(struct host_cmd_handler_args *args)
 			pmu_sense_resistor_ac, pmu_ac_sense_range_mv);
 	} else {
 		/* Power source == battery */
+		struct batt_params batt;
+
 		r->voltage_system = calc_voltage(
 			pmu_adc_read(ADC_VBAT, ADC_FLAG_KEEP_ON),
 			pmu_voltage_range_mv);
-		/* PMU reads charging current. When battery is discharging,
-		 * ADC returns 0. Use battery gas guage output instead.
+		/*
+		 * PMU reads charging current. When battery is discharging, ADC
+		 * returns 0. Use battery gas gauge output instead.
 		 */
-		battery_current(&bat_charging_current);
-		r->current_system = -bat_charging_current;
+		battery_get_params(&batt);
+		r->current_system = -batt.current;
 	}
 
 	/* Ignore USB powerinfo fields. */
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
  */
diff --git a/include/battery.h b/include/battery.h
index b5056cf9d4..cb2a5c32ef 100644
--- a/include/battery.h
+++ b/include/battery.h
@@ -38,11 +38,27 @@ struct batt_params {
 	int temperature;      /* Temperature in 0.1 K */
 	int state_of_charge;  /* State of charge (percent, 0-100) */
 	int voltage;          /* Battery voltage (mV) */
-	int current;          /* Battery current (mA) */
+	int current;          /* Battery current (mA); negative=discharging */
 	int desired_voltage;  /* Charging voltage desired by battery (mV) */
 	int desired_current;  /* Charging current desired by battery (mA) */
+	int flags;            /* Flags */
 };
 
+/* Flags for batt_params */
+
+/* Battery wants to be charged */
+#define BATT_FLAG_WANT_CHARGE			(1 << 0)
+
+/* Battery is responsive (talking to us via I2C) */
+#define BATT_FLAG_RESPONSIVE			(1 << 1)
+
+/* Able to talk to battery, but it won't tell us voltage or charge percent */
+#define BATT_FLAG_BAD_VOLTAGE			(1 << 2)
+#define BATT_FLAG_BAD_CHARGE_PERCENT		(1 << 3)
+
+/* Battery couldn't tell us every params we want */
+#define BATT_FLAG_BAD_ANY			(1 << 4)
+
 /* Working temperature ranges in degrees C */
 struct battery_temperature_ranges {
 	int8_t start_charging_min_c;
@@ -70,6 +86,15 @@ struct battery_info {
 const struct battery_info *battery_get_info(void);
 
 /**
+ * Get current battery parameters.
+ *
+ * Error conditions are reported via batt.flags.
+ *
+ * @param batt		Destination for battery data
+ */
+void battery_get_params(struct batt_params *batt);
+
+/**
  * Modify battery parameters to match vendor charging profile.
  *
  * @param batt		Battery parameters to modify
@@ -121,22 +146,6 @@ int battery_is_in_10mw_mode(int *val);
 int battery_set_10mw_mode(int enabled);
 
 /**
- * Read battery temperature.
- *
- * @param deci_kelvin	Destination for battery temperature in units of 0.1 K
- * @return non-zero if error.
- */
-int battery_temperature(int *deci_kelvin);
-
-/**
- * Read battery voltage.
- *
- * @param voltage	Destination for voltage in mW
- * @return non-zero if error.
- */
-int battery_voltage(int *voltage);
-
-/**
  * Read nominal voltage battery is designed to supply.
  *
  * @param voltage	Destination for voltage in mW
@@ -145,48 +154,6 @@ int battery_voltage(int *voltage);
 int battery_design_voltage(int *voltage);
 
 /**
- * Read charging voltage desired by battery.
- *
- * @param voltage	Destination for voltage in mV.
- * @return non-zero if error.
- */
-int battery_desired_voltage(int *voltage);
-
-/**
- * Read battery discharging current.
- *
- * @param current	Destination for discharge current in mA; negative
- *			value indicates charging.
- * @return non-zero if error.
- */
-int battery_current(int *current);
-
-/**
- * Read averaged battery discharging current.
- *
- * @param current	Destination for discharge current in mA; negative
- *			value indicates charging.
- * @return non-zero if error.
- */
-int battery_average_current(int *current);
-
-/**
- * Read charging current desired by battery.
- *
- * @param current	Destination for current in mA.
- * @return non-zero if error.
- */
-int battery_desired_current(int *current);
-
-/**
- * Read battery relative state of charge.
- *
- * @param percent	Destination for charge in percent
- * @return non-zero if error.
- */
-int battery_state_of_charge(int *percent);
-
-/**
  * Read absolute state of charge.
  *
  * @param percent	Destination for charge in percent
@@ -257,14 +224,6 @@ int battery_time_to_full(int *minutes);
 int battery_time_at_rate(int rate, int *minutes);
 
 /**
- * Check if battery allows charging.
- *
- * @param allowed	Non-zero if charging allowed; zero if not allowed.
- * @return non-zero if error.
- */
-int battery_charging_allowed(int *allowed);
-
-/**
  * Read battery status.
  *
  * @param status	Destination for status; see STATUS_* in battery_smart.h.
@@ -326,4 +285,3 @@ int battery_device_chemistry(char *dest, int size);
 int battery_manufacturer_date(int *year, int *month, int *day);
 
 #endif /* __CROS_EC_BATTERY_H */
-
diff --git a/include/charge_state.h b/include/charge_state.h
index bd8e5d05e6..5a176c1b40 100644
--- a/include/charge_state.h
+++ b/include/charge_state.h
@@ -22,24 +22,21 @@
 
 /* Power state error flags */
 #define F_CHARGER_INIT        (1 << 0) /* Charger initialization */
-#define F_CHARGER_VOLTAGE     (1 << 1) /* Charger maximun output voltage */
+#define F_CHARGER_VOLTAGE     (1 << 1) /* Charger maximum output voltage */
 #define F_CHARGER_CURRENT     (1 << 2) /* Charger maximum output current */
 #define F_BATTERY_VOLTAGE     (1 << 3) /* Battery voltage */
-#define F_BATTERY_CURRENT     (1 << 4) /* Battery charging current */
-#define F_DESIRED_VOLTAGE     (1 << 5) /* Battery desired voltage */
-#define F_DESIRED_CURRENT     (1 << 6) /* Battery desired current */
-#define F_BATTERY_TEMPERATURE (1 << 7) /* Battery temperature */
 #define F_BATTERY_MODE        (1 << 8) /* Battery mode */
 #define F_BATTERY_CAPACITY    (1 << 9) /* Battery capacity */
 #define F_BATTERY_STATE_OF_CHARGE (1 << 10) /* State of charge, percentage */
 #define F_BATTERY_UNRESPONSIVE    (1 << 11) /* Battery not responding */
 #define F_BATTERY_NOT_CONNECTED   (1 << 12) /* Battery not connected */
+#define F_BATTERY_GET_PARAMS  (1 << 13) /* Any battery parameter bad */
 
-#define F_BATTERY_MASK (F_BATTERY_VOLTAGE | F_BATTERY_CURRENT |  \
-			F_DESIRED_VOLTAGE | F_DESIRED_CURRENT |  \
-			F_BATTERY_TEMPERATURE | F_BATTERY_MODE | \
+#define F_BATTERY_MASK (F_BATTERY_VOLTAGE | \
+			F_BATTERY_MODE | \
 			F_BATTERY_CAPACITY | F_BATTERY_STATE_OF_CHARGE | \
-			F_BATTERY_UNRESPONSIVE | F_BATTERY_NOT_CONNECTED)
+			F_BATTERY_UNRESPONSIVE | F_BATTERY_NOT_CONNECTED | \
+			F_BATTERY_GET_PARAMS)
 #define F_CHARGER_MASK (F_CHARGER_VOLTAGE | F_CHARGER_CURRENT | \
 			F_CHARGER_INIT)