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

6 files changed, 156 insertions, 123 deletions

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. */