samus: fix lightbar tap sequence doesn't always show on AC event

Fix bug on samus that the battery status doesn't always show on
lightbar when AC is plugged/unplugged. It doesn't show when the
battery is full is S3 or S5 because in these states we turn off
CHARGE_EN so that ACOK to the EC never toggles.

Instead, what we want to do is display battery status whenever the
active charge port changes. This will happen when AC is plugged or
unplugged OR if a user has AC on both ports and toggles between
them using the charge override hot-keys.

BUG=chrome-os-partner:36317
BRANCH=samus
TEST=test plugging and unplugging AC on both sides when battery is
full and unit is in S0, and when unit is in S5. also tested lightbar
flashes battery percentage when two zingers are plugged in and you
switch between them using Ctrl+Search+0|1|2.

Change-Id: I5cd7fff4f466adf857f1e63f07f3b0c7ae8422c7
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/245922
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

4 files changed, 35 insertions, 26 deletions

diff --git a/board/samus/board.c b/board/samus/board.c
index 7f606516b0..704b3102b8 100644
--- a/board/samus/board.c
+++ b/board/samus/board.c
@@ -230,15 +230,6 @@ enum battery_present battery_is_present(void)
 #endif
 
 /**
- * Show battery status on lightbar when AC status changes
- */
-void show_battery_status(void)
-{
-	lightbar_sequence(LIGHTBAR_TAP);
-}
-DECLARE_HOOK(HOOK_AC_CHANGE, show_battery_status, HOOK_PRIO_DEFAULT);
-
-/**
  * Discharge battery when on AC power for factory test.
  */
 int board_discharge_on_ac(int enable)
diff --git a/common/host_command_pd.c b/common/host_command_pd.c
index e9282abee4..ca315c574f 100644
--- a/common/host_command_pd.c
+++ b/common/host_command_pd.c
@@ -9,6 +9,7 @@
 #include "common.h"
 #include "console.h"
 #include "host_command.h"
+#include "lightbar.h"
 #include "task.h"
 #include "timer.h"
 #include "util.h"
@@ -20,6 +21,9 @@
 /* By default allow 5V charging only for the dead battery case */
 static enum pd_charge_state charge_state = PD_CHARGE_5V;
 
+#define CHARGE_PORT_UNINITIALIZED -2
+static int charge_port = CHARGE_PORT_UNINITIALIZED;
+
 void host_command_pd_send_status(enum pd_charge_state new_chg_state)
 {
 	/* Update PD MCU charge state if necessary */
@@ -29,13 +33,15 @@ void host_command_pd_send_status(enum pd_charge_state new_chg_state)
 	task_set_event(TASK_ID_PDCMD, TASK_EVENT_EXCHANGE_PD_STATUS, 0);
 }
 
-void pd_exchange_status(int *charge_port)
+int pd_get_active_charge_port(void)
+{
+	return charge_port;
+}
+
+static void pd_exchange_status(void)
 {
 	struct ec_params_pd_status ec_status;
-	struct ec_response_pd_status pd_status = {
-		/* default for when the PD isn't cooperating */
-		.active_charge_port = -1,
-	};
+	struct ec_response_pd_status pd_status;
 	int rv = 0;
 
 	/* Send PD charge state and battery state of charge */
@@ -55,14 +61,29 @@ void pd_exchange_status(int *charge_port)
 			     sizeof(struct ec_params_pd_status), &pd_status,
 			     sizeof(struct ec_response_pd_status));
 
-	if (charge_port)
-		*charge_port = pd_status.active_charge_port;
-
 	if (rv < 0) {
 		CPRINTS("Host command to PD MCU failed");
 		return;
 	}
 
+#ifdef HAS_TASK_LIGHTBAR
+	/*
+	 * If charge port has changed, and it was initialized, then show
+	 * battery status on lightbar.
+	 */
+	if (pd_status.active_charge_port != charge_port) {
+		if (charge_port != CHARGE_PORT_UNINITIALIZED) {
+			charge_port = pd_status.active_charge_port;
+			lightbar_sequence(LIGHTBAR_TAP);
+		} else {
+			charge_port = pd_status.active_charge_port;
+		}
+	}
+#else
+	/* Store the active charge port */
+	charge_port = pd_status.active_charge_port;
+#endif
+
 	/* Set input current limit */
 	rv = charge_set_input_current_limit(MAX(pd_status.curr_lim_ma,
 					CONFIG_CHARGER_INPUT_CURRENT));
@@ -77,7 +98,7 @@ void pd_exchange_status(int *charge_port)
 void pd_command_task(void)
 {
 	/* On startup exchange status with the PD */
-	pd_exchange_status(0);
+	pd_exchange_status();
 
 	while (1) {
 		/* Wait for the next command event */
@@ -85,6 +106,6 @@ void pd_command_task(void)
 
 		/* Process event to send status to PD */
 		if (evt & TASK_EVENT_EXCHANGE_PD_STATUS)
-			pd_exchange_status(0);
+			pd_exchange_status();
 	}
 }
diff --git a/common/lightbar.c b/common/lightbar.c
index 6a72e4458f..662c75a787 100644
--- a/common/lightbar.c
+++ b/common/lightbar.c
@@ -984,7 +984,7 @@ static int get_tap_direction(void)
 		dir = force_dir;
 #ifdef HAS_TASK_PDCMD
 	else
-		pd_exchange_status(&dir);
+		dir = pd_get_active_charge_port();
 #endif
 	if (dir < 0)
 		dir = last_dir;
diff --git a/include/host_command.h b/include/host_command.h
index 4661e59dd6..4bf903e6ef 100644
--- a/include/host_command.h
+++ b/include/host_command.h
@@ -216,14 +216,11 @@ void host_throttle_cpu(int throttle);
 void host_command_pd_send_status(enum pd_charge_state new_chg_state);
 
 /**
- * Ask the PD MCU for its status, obtaining the current charge_port as a
- * side-effect (-1 means none or don't know).
+ * Get the active charge port from the PD
  *
- * @param charge_port   If present, updated with the current charge port:
- *                        -1 == none/unknown, 0 == left, 1 == right.
+ * @return -1 == none/unknown, 0 == left, 1 == right.
  */
-void pd_exchange_status(int *charge_port);
-
+int pd_get_active_charge_port(void);
 
 /**
  * Send host command to PD MCU.