host_command_pd: loop over sending PD status host command

Modify host_command_pd.c to loop over sending of PD status
host command and processing the response as long as the
alert GPIO is active.

This fixes a potential bug that if the alert line is held
low for more than one PD status host command, then we would
not process the return status.

Also, fix a bug in which we could call alert() for a
non-existent port.

BUG=none
BRANCH=strago
TEST=verified on samus and glados. connected charger and verified
that we negotiate a contract and set appropriate input currnet limit.

Change-Id: I3b2db87b51f55fc2b20a4695bd466ff8bb09ea55
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/288819
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Trybot-Ready: David James <davidjames@chromium.org>

1 files changed, 83 insertions, 69 deletions

diff --git a/common/host_command_pd.c b/common/host_command_pd.c
index 0ece50a468..fe1ccc014c 100644
--- a/common/host_command_pd.c
+++ b/common/host_command_pd.c
@@ -22,6 +22,11 @@
 
 #define TASK_EVENT_EXCHANGE_PD_STATUS  TASK_EVENT_CUSTOM(1)
 
+/* Define local option for if we are a TCPM with an off chip TCPC */
+#if defined(CONFIG_USB_POWER_DELIVERY) && !defined(CONFIG_USB_PD_TCPM_STUB)
+#define USB_TCPM_WITH_OFF_CHIP_TCPC
+#endif
+
 #ifdef CONFIG_HOSTCMD_PD_CHG_CTRL
 /* By default allow 5V charging only for the dead battery case */
 static enum pd_charge_state charge_state = PD_CHARGE_5V;
@@ -33,7 +38,7 @@ int pd_get_active_charge_port(void)
 {
 	return charge_port;
 }
-#endif
+#endif /* CONFIG_HOSTCMD_PD_CHG_CTRL */
 
 void host_command_pd_send_status(enum pd_charge_state new_chg_state)
 {
@@ -46,6 +51,7 @@ 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);
 }
 
+
 #ifdef CONFIG_HOSTCMD_PD
 static int pd_send_host_command(struct ec_params_pd_status *ec_status,
 	struct ec_response_pd_status *pd_status)
@@ -63,120 +69,128 @@ static int pd_send_host_command(struct ec_params_pd_status *ec_status,
 			     sizeof(struct ec_response_pd_status));
 	return rv;
 }
-#endif
 
-static void pd_exchange_status(void)
+static void pd_exchange_update_ec_status(struct ec_params_pd_status *ec_status)
 {
-#ifdef CONFIG_HOSTCMD_PD
-	struct ec_params_pd_status ec_status;
-	struct ec_response_pd_status pd_status;
-	int rv = 0;
-#ifdef CONFIG_HOSTCMD_PD_PANIC
-	static int pd_in_rw;
-#endif
-
 	/* Send PD charge state and battery state of charge */
 #ifdef CONFIG_HOSTCMD_PD_CHG_CTRL
-	ec_status.charge_state = charge_state;
+	ec_status->charge_state = charge_state;
 #endif
 	if (charge_get_flags() & CHARGE_FLAG_BATT_RESPONSIVE)
-		ec_status.batt_soc = charge_get_percent();
+		ec_status->batt_soc = charge_get_percent();
 	else
-		ec_status.batt_soc = -1;
-
-	rv = pd_send_host_command(&ec_status, &pd_status);
-
-	if (rv < 0) {
-		CPRINTS("Host command to PD MCU failed");
-		return;
-	}
+		ec_status->batt_soc = -1;
+}
 
 #ifdef CONFIG_HOSTCMD_PD_PANIC
+static void pd_check_panic(struct ec_response_pd_status *pd_status)
+{
+	static int pd_in_rw;
+
 	/*
-	 * Check if PD MCU is in RW. If PD MCU was in RW and is now in RO
+	 * Check if PD MCU is in RW. If PD MCU was in RW, is now in RO,
 	 * AND it did not sysjump to RO, then it must have crashed, and
 	 * therefore we should panic as well.
 	 */
-	if (pd_status.status & PD_STATUS_IN_RW) {
+	if (pd_status->status & PD_STATUS_IN_RW) {
 		pd_in_rw = 1;
 	} else if (pd_in_rw &&
-		   !(pd_status.status & PD_STATUS_JUMPED_TO_IMAGE)) {
+		   !(pd_status->status & PD_STATUS_JUMPED_TO_IMAGE)) {
 		panic_printf("PD crash");
 		software_panic(PANIC_SW_PD_CRASH, 0);
 	}
-#endif
+}
+#endif /* CONFIG_HOSTCMD_PD_PANIC */
 
 #ifdef CONFIG_HOSTCMD_PD_CHG_CTRL
+static void pd_check_chg_status(struct ec_response_pd_status *pd_status)
+{
+	int rv;
 #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 (pd_status->active_charge_port != charge_port) {
 		if (charge_port != CHARGE_PORT_UNINITIALIZED) {
-			charge_port = pd_status.active_charge_port;
+			charge_port = pd_status->active_charge_port;
 			lightbar_sequence(LIGHTBAR_TAP);
 		} else {
-			charge_port = pd_status.active_charge_port;
+			charge_port = pd_status->active_charge_port;
 		}
 	}
 #else
 	/* Store the active charge port */
-	charge_port = pd_status.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,
+	rv = charge_set_input_current_limit(MAX(pd_status->curr_lim_ma,
 					CONFIG_CHARGER_INPUT_CURRENT));
 	if (rv < 0)
-		CPRINTS("Failed to set input current limit from PD MCU");
+		CPRINTS("Failed to set input curr limit from PD MCU");
+}
 #endif /* CONFIG_HOSTCMD_PD_CHG_CTRL */
+#endif /* CONFIG_HOSTCMD_PD */
+
+#ifdef USB_TCPM_WITH_OFF_CHIP_TCPC
+static void pd_check_tcpc_alert(struct ec_response_pd_status *pd_status)
+{
+	int i;
+
+	for (i = 0; i < CONFIG_USB_PD_PORT_COUNT; i++) {
+		if (!pd_status ||
+		    (pd_status->status & (PD_STATUS_TCPC_ALERT_0 << i)))
+			tcpc_alert(i);
+	}
+}
+#endif /* USB_TCPM_WITH_OFF_CHIP_TCPC */
+
+static void pd_exchange_status(void)
+{
+#ifdef CONFIG_HOSTCMD_PD
+	struct ec_params_pd_status ec_status;
+	struct ec_response_pd_status pd_status;
+	int rv;
+
+	pd_exchange_update_ec_status(&ec_status);
+#endif
+
+#ifdef USB_TCPM_WITH_OFF_CHIP_TCPC
+	/* Loop until the alert gpio is not active */
+	do {
+		int first_exchange = 1;
+#endif
+
+#ifdef CONFIG_HOSTCMD_PD
+		rv = pd_send_host_command(&ec_status, &pd_status);
+		if (rv < 0) {
+			CPRINTS("Host command to PD MCU failed");
+			return;
+		}
 
-	/* If PD is signalling host event, then pass it up to AP */
-	if (pd_status.status & PD_STATUS_HOST_EVENT)
-		host_set_single_event(EC_HOST_EVENT_PD_MCU);
+#ifdef CONFIG_HOSTCMD_PD_PANIC
+		pd_check_panic(&pd_status);
+#endif
+
+#ifdef CONFIG_HOSTCMD_PD_CHG_CTRL
+		pd_check_chg_status(&pd_status);
+#endif
 #endif /* CONFIG_HOSTCMD_PD */
 
-	/*
-	 * If we are TCPM, connected to an off chip TCPC, then check
-	 * TCPC alert status.
-	 */
-#if defined(CONFIG_USB_POWER_DELIVERY) && !defined(CONFIG_USB_PD_TCPM_STUB)
-	/*
-	 * Loop here until all Alerts from either port have been handled.
-	 * This is necessary to prevent the case where Alert bits are set
-	 * and the GPIO line is held low, which would prevent a new edge
-	 * event which prevents tcpc_alert() from being called and that
-	 * in turn prevents the GPIO line from being released.
-	 */
-	while (!gpio_get_level(GPIO_PD_MCU_INT)) {
-		int loop_count = 0;
-		/*
-		 * If TCPC is not present on this MCU, then check
-		 * to see if either PD port is signallng an
-		 * Alert# to the TCPM.
-		 */
+#ifdef USB_TCPM_WITH_OFF_CHIP_TCPC
 #ifdef CONFIG_HOSTCMD_PD
-		if (pd_status.status & PD_STATUS_TCPC_ALERT_0)
-			tcpc_alert(0);
-		if (pd_status.status & PD_STATUS_TCPC_ALERT_1)
-			tcpc_alert(1);
+		pd_check_tcpc_alert(&pd_status);
 #else
-		tcpc_alert(0);
-		tcpc_alert(1);
+		pd_check_tcpc_alert(NULL);
 #endif
-		if (loop_count++) {
+
+		if (!first_exchange) {
 			usleep(50*MSEC);
-#ifdef CONFIG_HOSTCMD_PD
-			rv = pd_send_host_command(&ec_status, &pd_status);
-			if (rv < 0) {
-				CPRINTS("Host command to PD MCU failed");
-				return;
-			}
-#endif
+			first_exchange = 0;
 		}
-	}
-#endif
+	} while (!gpio_get_level(GPIO_PD_MCU_INT));
+#endif /* USB_TCPM_WITH_OFF_CHIP_TCPC */
 }
 
 void pd_command_task(void)