spring: More stable battery LED controlstabilize-3881.0.B

This includes:
  - Discharging with AC present now recognized as battery assist mode
  - Trigger LED update every second instead of from PMU task
  - Minor breathing effect optimization
  - Control LED power and color from the same task to prevent
    simultaneous access to LED driver

BUG=chrome-os-partner:18244
TEST=Manual
BRANCH=spring

Change-Id: I9b34934f46cd4f04aaa7ca30cbe90316aec3e61e
Signed-off-by: Vic Yang <victoryang@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/45632
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

5 files changed, 52 insertions, 39 deletions

diff --git a/board/spring/board.c b/board/spring/board.c
index 227fc10f10..1dc0707203 100644
--- a/board/spring/board.c
+++ b/board/spring/board.c
@@ -330,12 +330,12 @@ int board_led_breathing(int enabled)
 		ret |= lp5562_engine_load(LP5562_ENG_SEL_1,
 					  breathing_prog,
 					  sizeof(breathing_prog));
-		ret |= lp5562_set_engine(LP5562_ENG_SEL_NONE,
-					 LP5562_ENG_SEL_NONE,
-					 LP5562_ENG_SEL_1);
 		ret |= lp5562_engine_control(LP5562_ENG_RUN,
 					     LP5562_ENG_HOLD,
 					     LP5562_ENG_HOLD);
+		ret |= lp5562_set_engine(LP5562_ENG_SEL_NONE,
+					 LP5562_ENG_SEL_NONE,
+					 LP5562_ENG_SEL_1);
 	} else {
 		ret |= lp5562_engine_control(LP5562_ENG_HOLD,
 					     LP5562_ENG_HOLD,
@@ -348,26 +348,49 @@ int board_led_breathing(int enabled)
 	return ret;
 }
 
-int board_battery_led(enum charging_state state)
+static void board_battery_led_update(void)
 {
 	int current;
 	int desired_current;
+
+	/* Current states and next states */
 	static uint32_t color = LED_COLOR_RED;
 	static int breathing;
+	static int led_power;
 	int new_color = LED_COLOR_RED;
 	int new_breathing = 0;
-	int ret = 0;
+	int new_led_power;
+
+	/* Determine LED power */
+	new_led_power = board_get_ac();
+	if (new_led_power != led_power) {
+		led_power = new_led_power;
+		if (new_led_power) {
+			lp5562_poweron();
+		} else {
+			color = LED_COLOR_NONE;
+			if (breathing) {
+				board_led_breathing(0);
+				breathing = 0;
+			}
+			lp5562_poweroff();
+		}
+	}
+	if (!new_led_power)
+		return;
 
 	/*
 	 * LED power is controlled by accessory detection. We only
 	 * set color here.
 	 */
-	switch (state) {
+	switch (charge_get_state()) {
 	case ST_IDLE:
 		new_color = LED_COLOR_GREEN;
 		break;
 	case ST_DISCHARGING:
-		new_color = LED_COLOR_NONE;
+		/* Discharging with AC, must be battery assist */
+		new_color = LED_COLOR_YELLOW;
+		new_breathing = 1;
 		break;
 	case ST_PRE_CHARGING:
 		new_color = LED_COLOR_YELLOW;
@@ -382,7 +405,7 @@ int board_battery_led(enum charging_state state)
 
 		if (current < 0 && desired_current > 0) { /* Battery assist */
 			new_breathing = 1;
-			new_color = LED_COLOR_NONE;
+			new_color = LED_COLOR_YELLOW;
 			break;
 		}
 
@@ -396,16 +419,16 @@ int board_battery_led(enum charging_state state)
 		break;
 	}
 
-	if (new_breathing != breathing) {
-		ret |= board_led_breathing(new_breathing);
-		breathing = new_breathing;
-	}
 	if (new_color != color) {
-		ret |= lp5562_set_color(new_color);
+		lp5562_set_color(new_color);
 		color = new_color;
 	}
-	return ret;
+	if (new_breathing != breathing) {
+		board_led_breathing(new_breathing);
+		breathing = new_breathing;
+	}
 }
+DECLARE_HOOK(HOOK_SECOND, board_battery_led_update, HOOK_PRIO_DEFAULT);
 
 /*****************************************************************************/
 /* Host commands */
diff --git a/board/spring/board.h b/board/spring/board.h
index 7a6d4c8f05..2b0f62e54a 100644
--- a/board/spring/board.h
+++ b/board/spring/board.h
@@ -164,9 +164,6 @@ void board_pwm_duty_cycle(int percent);
 /* Update USB port status */
 void board_usb_charge_update(int force_update);
 
-/* Update battery LED color */
-int board_battery_led(enum charging_state state);
-
 /* Get USB port device type */
 int board_get_usb_dev_type(void);
 
diff --git a/board/spring/usb_charging.c b/board/spring/usb_charging.c
index 5c29af3e69..ba60e4dbaf 100644
--- a/board/spring/usb_charging.c
+++ b/board/spring/usb_charging.c
@@ -386,12 +386,8 @@ static void usb_device_change(int dev_type)
 			current_limit = I_LIMIT_1500MA;
 
 		board_pwm_nominal_duty_cycle(current_limit);
-
-		/* Turns on battery LED */
-		lp5562_poweron();
 	} else {
 		board_ilim_config(ILIM_CONFIG_MANUAL_ON);
-		lp5562_poweroff();
 	}
 
 	/* Log to console */
diff --git a/common/pmu_tps65090_charger.c b/common/pmu_tps65090_charger.c
index 9d3fea4630..732a5b6d98 100644
--- a/common/pmu_tps65090_charger.c
+++ b/common/pmu_tps65090_charger.c
@@ -54,6 +54,8 @@ static const char * const state_list[] = {
 static timestamp_t last_waken; /* Initialized to 0 */
 static int has_pending_event;
 
+static enum charging_state current_state = ST_IDLE;
+
 static void enable_charging(int enable)
 {
 	enable = enable ? 1 : 0;
@@ -365,17 +367,14 @@ static int calc_next_state(int state)
 	return ST_IDLE;
 }
 
-int __board_battery_led(enum charging_state state)
+/* TODO: Merge charge_state.h and unify charge interface */
+enum charging_state charge_get_state(void)
 {
-	return EC_SUCCESS;
+	return current_state;
 }
 
-int board_battery_led(enum charging_state state)
-	__attribute__((weak, alias("__board_battery_led")));
-
 void pmu_charger_task(void)
 {
-	int state = ST_IDLE;
 	int next_state;
 	int wait_time = T1_USEC;
 	unsigned int pre_charging_count = 0;
@@ -421,20 +420,20 @@ void pmu_charger_task(void)
 		 * failed.
 		 */
 		next_state = pre_charging_count > PRE_CHARGING_RETRY ?
-			ST_CHARGING_ERROR : calc_next_state(state);
+			ST_CHARGING_ERROR : calc_next_state(current_state);
 
-		if (next_state != state) {
+		if (next_state != current_state) {
 			/* Reset state of charge moving average window */
 			rsoc_moving_average(-1);
 
 			pre_charging_count = 0;
 			CPRINTF("[batt] state %s -> %s\n",
-				state_list[state],
+				state_list[current_state],
 				state_list[next_state]);
 
-			state = next_state;
+			current_state = next_state;
 
-			switch (state) {
+			switch (current_state) {
 			case ST_PRE_CHARGING:
 			case ST_CHARGING:
 				if (pmu_blink_led(0))
@@ -461,9 +460,7 @@ void pmu_charger_task(void)
 			}
 		}
 
-		board_battery_led(state);
-
-		switch (state) {
+		switch (current_state) {
 		case ST_CHARGING:
 		case ST_CHARGING_ERROR:
 			wait_time = T2_USEC;
diff --git a/include/pmu_tpschrome.h b/include/pmu_tpschrome.h
index 9b9c0ff0d5..0af241682f 100644
--- a/include/pmu_tpschrome.h
+++ b/include/pmu_tpschrome.h
@@ -260,14 +260,14 @@ int board_get_ac(void);
 void board_hard_reset(void);
 
 /**
- * Update battery LED according to charger state.
+ * Wake TPS65090 charger task, but throttled to at most one call per tick
  */
-int board_battery_led(enum charging_state state);
+void pmu_task_throttled_wake(void);
 
 /**
- * Wake TPS65090 charger task, but throttled to at most one call per tick
+ * Get current charge state
  */
-void pmu_task_throttled_wake(void);
+enum charging_state charge_get_state(void);
 
 #endif /* __CROS_EC_TPSCHROME_H */