USB MUX: Generalize mux ACK

Currently, only the virtual mux driver uses the mux ACK feature, but the
actual wait for the host command ACK is a part of the usb_mux general
code.  Generalize this mux ACK wait so it's available if needed in the
future for more muxes.

Additionally, moving this wait out of the mux set will allow us to lock
the muxes intelligently between tasks, without keeping the muxes locked
during the inactive ACK wait.

BRANCH=None
BUG=b:172222942,b:186777984
TEST=tast typec.Mode*.manual on voxel

Signed-off-by: Diana Z <dzigterman@chromium.org>
Change-Id: I61a043425a482cc6f3170548c888d91ec20c2a82
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3078411
Reviewed-by: Keith Short <keithshort@chromium.org>

13 files changed, 105 insertions, 29 deletions

diff --git a/driver/usb_mux/amd_fp5.c b/driver/usb_mux/amd_fp5.c
index 92c7e682a4..b77edf2826 100644
--- a/driver/usb_mux/amd_fp5.c
+++ b/driver/usb_mux/amd_fp5.c
@@ -42,10 +42,14 @@ static int amd_fp5_init(const struct usb_mux *me)
 	return EC_SUCCESS;
 }
 
-static int amd_fp5_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int amd_fp5_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	uint8_t val = 0;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	saved_mux_state[me->usb_port] = mux_state;
 
 	/*
@@ -128,9 +132,11 @@ static void amd_fp5_chipset_reset_delay(void)
 {
 	struct usb_mux *me;
 	int rv;
+	bool unused;
 
 	while (queue_remove_unit(&chipset_reset_queue, &me)) {
-		rv = amd_fp5_set_mux(me, saved_mux_state[me->usb_port]);
+		rv = amd_fp5_set_mux(me, saved_mux_state[me->usb_port],
+				     &unused);
 		if (rv)
 			ccprints("C%d restore mux rv:%d", me->usb_port, rv);
 	}
diff --git a/driver/usb_mux/amd_fp6.c b/driver/usb_mux/amd_fp6.c
index 88f6b5aff5..b2d5ae1fb4 100644
--- a/driver/usb_mux/amd_fp6.c
+++ b/driver/usb_mux/amd_fp6.c
@@ -134,10 +134,14 @@ static void amd_fp6_set_mux_retry(void)
 }
 
 
-static int amd_fp6_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int amd_fp6_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	uint8_t val;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	if (mux_state == USB_PD_MUX_NONE)
 		/*
 		 * LOW_POWER must be set when connection mode is
diff --git a/driver/usb_mux/anx3443.c b/driver/usb_mux/anx3443.c
index dc7fd3a3c0..2e57f4d30c 100644
--- a/driver/usb_mux/anx3443.c
+++ b/driver/usb_mux/anx3443.c
@@ -76,10 +76,14 @@ static int anx3443_wake_up(const struct usb_mux *me)
 	return EC_SUCCESS;
 }
 
-static int anx3443_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int anx3443_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	int reg;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	/* Mux is not powered in Z1 */
 	if (chipset_in_state(CHIPSET_STATE_HARD_OFF))
 		return (mux_state == USB_PD_MUX_NONE) ? EC_SUCCESS
@@ -129,6 +133,7 @@ static int anx3443_get_mux(const struct usb_mux *me, mux_state_t *mux_state)
 static int anx3443_init(const struct usb_mux *me)
 {
 	uint64_t now;
+	bool unused;
 
 	/*
 	 * ANX3443 requires 30ms to power on. EC and ANX3443 are on the same
@@ -140,8 +145,12 @@ static int anx3443_init(const struct usb_mux *me)
 
 	RETURN_ERROR(anx3443_wake_up(me));
 
+	/*
+	 * Note that bypassing the usb_mux API is okay for internal driver calls
+	 * since the task calling init already holds this port's mux lock.
+	 */
 	/* Default to USB mode */
-	RETURN_ERROR(anx3443_set_mux(me, USB_PD_MUX_USB_ENABLED));
+	RETURN_ERROR(anx3443_set_mux(me, USB_PD_MUX_USB_ENABLED, &unused));
 
 	return EC_SUCCESS;
 }
diff --git a/driver/usb_mux/anx7440.c b/driver/usb_mux/anx7440.c
index df3a9c2682..89e593217d 100644
--- a/driver/usb_mux/anx7440.c
+++ b/driver/usb_mux/anx7440.c
@@ -61,10 +61,14 @@ static int anx7440_init(const struct usb_mux *me)
 }
 
 /* Writes control register to set switch mode */
-static int anx7440_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int anx7440_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	int reg, res;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	res = anx7440_read(me, ANX7440_REG_CHIP_CTRL, &reg);
 	if (res)
 		return res;
diff --git a/driver/usb_mux/anx7451.c b/driver/usb_mux/anx7451.c
index ccf09f61be..52318eee29 100644
--- a/driver/usb_mux/anx7451.c
+++ b/driver/usb_mux/anx7451.c
@@ -89,10 +89,14 @@ static int anx7451_wake_up(const struct usb_mux *me)
 	return EC_SUCCESS;
 }
 
-static int anx7451_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int anx7451_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	int reg;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	/* Mux is not powered in Z1 */
 	if (chipset_in_state(CHIPSET_STATE_HARD_OFF))
 		return (mux_state == USB_PD_MUX_NONE) ? EC_SUCCESS
diff --git a/driver/usb_mux/it5205.c b/driver/usb_mux/it5205.c
index f275568313..0cfecdeda0 100644
--- a/driver/usb_mux/it5205.c
+++ b/driver/usb_mux/it5205.c
@@ -91,10 +91,14 @@ enum ec_error_list it5205h_enable_csbu_switch(const struct usb_mux *me, bool en)
 }
 
 /* Writes control register to set switch mode */
-static int it5205_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int it5205_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			  bool *ack_required)
 {
 	uint8_t reg;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	switch (mux_state & MUX_STATE_DP_USB_MASK) {
 	case USB_PD_MUX_USB_ENABLED:
 		reg = IT5205_USB;
diff --git a/driver/usb_mux/pi3usb3x532.c b/driver/usb_mux/pi3usb3x532.c
index 7e74157d17..2435157967 100644
--- a/driver/usb_mux/pi3usb3x532.c
+++ b/driver/usb_mux/pi3usb3x532.c
@@ -84,10 +84,14 @@ static int pi3usb3x532_init(const struct usb_mux *me)
 
 /* Writes control register to set switch mode */
 static int pi3usb3x532_set_mux(const struct usb_mux *me,
-			       mux_state_t mux_state)
+			       mux_state_t mux_state,
+			       bool *ack_required)
 {
 	uint8_t reg = 0;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	if (mux_state & USB_PD_MUX_USB_ENABLED)
 		reg |= PI3USB3X532_MODE_USB;
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
diff --git a/driver/usb_mux/ps8740.c b/driver/usb_mux/ps8740.c
index 4d03cb5cf3..618c74cd65 100644
--- a/driver/usb_mux/ps8740.c
+++ b/driver/usb_mux/ps8740.c
@@ -70,10 +70,14 @@ static int ps8740_init(const struct usb_mux *me)
 }
 
 /* Writes control register to set switch mode */
-static int ps8740_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int ps8740_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			  bool *ack_required)
 {
 	uint8_t reg = 0;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	if (mux_state & USB_PD_MUX_USB_ENABLED)
 		reg |= PS8740_MODE_USB_ENABLED;
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
diff --git a/driver/usb_mux/ps8743.c b/driver/usb_mux/ps8743.c
index 68bb0428d3..f618bb009f 100644
--- a/driver/usb_mux/ps8743.c
+++ b/driver/usb_mux/ps8743.c
@@ -94,7 +94,8 @@ static int ps8743_init(const struct usb_mux *me)
 }
 
 /* Writes control register to set switch mode */
-static int ps8743_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int ps8743_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			  bool *ack_required)
 {
 	/*
 	 * For CE_DP, CE_USB, and FLIP, disable pin control and enable I2C
@@ -105,6 +106,9 @@ static int ps8743_set_mux(const struct usb_mux *me, mux_state_t mux_state)
 		       PS8743_MODE_USB_REG_CONTROL |
 		       PS8743_MODE_FLIP_REG_CONTROL);
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	if (mux_state & USB_PD_MUX_USB_ENABLED)
 		reg |= PS8743_MODE_USB_ENABLE;
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
diff --git a/driver/usb_mux/ps8822.c b/driver/usb_mux/ps8822.c
index f7a60df6fa..7f25db37f4 100644
--- a/driver/usb_mux/ps8822.c
+++ b/driver/usb_mux/ps8822.c
@@ -79,11 +79,15 @@ static int ps8822_init(const struct usb_mux *me)
 }
 
 /* Writes control register to set switch mode */
-static int ps8822_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int ps8822_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			  bool *ack_required)
 {
 	int reg;
 	int rv;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	rv = ps8822_read(me, PS8822_REG_PAGE0, PS8822_REG_MODE, &reg);
 	if (rv)
 		return rv;
diff --git a/driver/usb_mux/tusb1064.c b/driver/usb_mux/tusb1064.c
index 6bbf323b3c..1c0f0e4701 100644
--- a/driver/usb_mux/tusb1064.c
+++ b/driver/usb_mux/tusb1064.c
@@ -32,10 +32,14 @@ static int tusb1064_write(const struct usb_mux *me, uint8_t reg, uint8_t val)
 }
 
 /* Writes control register to set switch mode */
-static int tusb1064_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int tusb1064_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			    bool *ack_required)
 {
 	int reg = REG_GENERAL_STATIC_BITS;
 
+	/* This driver does not use host command ACKs */
+	*ack_required = false;
+
 	if (mux_state & USB_PD_MUX_USB_ENABLED)
 		reg |= REG_GENERAL_CTLSEL_USB3;
 	if (mux_state & USB_PD_MUX_DP_ENABLED)
@@ -72,6 +76,7 @@ static int tusb1064_init(const struct usb_mux *me)
 {
 	int res;
 	uint8_t reg;
+	bool unused;
 
 	/* Default to "Floating Pin" DP Equalization */
 	reg = TUSB1064_DP1EQ(TUSB1064_DP_EQ_RX_10_0_DB) |
@@ -86,8 +91,12 @@ static int tusb1064_init(const struct usb_mux *me)
 	if (res)
 		return res;
 
+	/*
+	 * Note that bypassing the usb_mux API is okay for internal driver calls
+	 * since the task calling init already holds this port's mux lock.
+	 */
 	/* Disconnect USB3.1 and DP */
-	res = tusb1064_set_mux(me, USB_PD_MUX_NONE);
+	res = tusb1064_set_mux(me, USB_PD_MUX_NONE, &unused);
 	if (res)
 		return res;
 
diff --git a/driver/usb_mux/usb_mux.c b/driver/usb_mux/usb_mux.c
index f3418556d0..cf2e660a4f 100644
--- a/driver/usb_mux/usb_mux.c
+++ b/driver/usb_mux/usb_mux.c
@@ -12,6 +12,7 @@
 #include "hooks.h"
 #include "host_command.h"
 #include "task.h"
+#include "timer.h"
 #include "usb_mux.h"
 #include "usbc_ppc.h"
 #include "util.h"
@@ -73,6 +74,7 @@ static int configure_mux(int port,
 	     mux_ptr = mux_ptr->next_mux) {
 		mux_state_t lcl_state;
 		const struct usb_mux_driver *drv = mux_ptr->driver;
+		bool ack_required = false;
 
 		switch (config) {
 		case USB_MUX_INIT:
@@ -107,7 +109,8 @@ static int configure_mux(int port,
 				lcl_state &= ~USB_PD_MUX_POLARITY_INVERTED;
 
 			if (drv && drv->set) {
-				rv = drv->set(mux_ptr, lcl_state);
+				rv = drv->set(mux_ptr, lcl_state,
+					      &ack_required);
 				if (rv)
 					break;
 			}
@@ -133,6 +136,21 @@ static int configure_mux(int port,
 			}
 			break;
 		}
+
+		if (ack_required) {
+			/* This should only be called from the PD task */
+			assert(port == TASK_ID_TO_PD_PORT(task_get_current()));
+
+			/*
+			 * Note: This task event could be generalized for more
+			 * purposes beyond host command ACKs.  For now, these
+			 * wait times are tuned for the purposes of the TCSS
+			 * mux, but could be made configurable for other
+			 * purposes.
+			 */
+			task_wait_event_mask(PD_EVENT_AP_MUX_DONE, 100*MSEC);
+			usleep(12.5 * MSEC);
+		}
 	}
 
 	if (rv)
diff --git a/driver/usb_mux/virtual.c b/driver/usb_mux/virtual.c
index 9fbcbd2643..72020dda03 100644
--- a/driver/usb_mux/virtual.c
+++ b/driver/usb_mux/virtual.c
@@ -26,12 +26,19 @@
 
 static mux_state_t virtual_mux_state[CONFIG_USB_PD_PORT_MAX_COUNT];
 
-static inline void virtual_mux_update_state(int port, mux_state_t mux_state)
+static inline void virtual_mux_update_state(int port, mux_state_t mux_state,
+					    bool *ack_required)
 {
 	mux_state_t previous_mux_state = virtual_mux_state[port];
 
 	virtual_mux_state[port] = mux_state;
 
+	/*
+	 * Initialize ack_required to false to start, and set on necessary
+	 * conditions
+	 */
+	*ack_required = false;
+
 	if (!IS_ENABLED(CONFIG_HOSTCMD_EVENTS))
 		return;
 
@@ -48,9 +55,6 @@ static inline void virtual_mux_update_state(int port, mux_state_t mux_state)
 	 * TCSS Mux to allow better synchronization between them and thereby
 	 * remain in the same state for achieving proper safe state
 	 * terminations.
-	 *
-	 * Note: While the EC waits for the ACK, the value of usb_mux_get
-	 * won't match the most recently set value with usb_mux_set.
 	 */
 
 	/* TODO(b/186777984): Wait for an ACK for all mux state change */
@@ -60,13 +64,8 @@ static inline void virtual_mux_update_state(int port, mux_state_t mux_state)
 	   ((previous_mux_state & USB_PD_MUX_SAFE_MODE) &&
 	    !(mux_state & USB_PD_MUX_SAFE_MODE)) ||
 	   ((previous_mux_state != USB_PD_MUX_NONE) &&
-	    (mux_state == USB_PD_MUX_NONE))) {
-		/* This should only be called from the PD task */
-		assert(port == TASK_ID_TO_PD_PORT(task_get_current()));
-
-		task_wait_event_mask(PD_EVENT_AP_MUX_DONE, 100*MSEC);
-		usleep(12.5 * MSEC);
-	}
+	    (mux_state == USB_PD_MUX_NONE)))
+		*ack_required = true;
 }
 
 static int virtual_init(const struct usb_mux *me)
@@ -78,7 +77,8 @@ static int virtual_init(const struct usb_mux *me)
  * Set the state of our 'virtual' mux. The EC does not actually control this
  * mux, so update the desired state, then notify the host of the update.
  */
-static int virtual_set_mux(const struct usb_mux *me, mux_state_t mux_state)
+static int virtual_set_mux(const struct usb_mux *me, mux_state_t mux_state,
+			   bool *ack_required)
 {
 	int port = me->usb_port;
 
@@ -86,7 +86,7 @@ static int virtual_set_mux(const struct usb_mux *me, mux_state_t mux_state)
 	mux_state_t new_mux_state = (mux_state & ~USB_PD_MUX_HPD_STATE) |
 		(virtual_mux_state[port] & USB_PD_MUX_HPD_STATE);
 
-	virtual_mux_update_state(port, new_mux_state);
+	virtual_mux_update_state(port, new_mux_state, ack_required);
 
 	return EC_SUCCESS;
 }
@@ -108,13 +108,15 @@ static int virtual_get_mux(const struct usb_mux *me, mux_state_t *mux_state)
 void virtual_hpd_update(const struct usb_mux *me, int hpd_lvl, int hpd_irq)
 {
 	int port = me->usb_port;
+	bool unused;
 
 	/* Current HPD related mux status + existing USB & DP mux status */
 	mux_state_t new_mux_state = (hpd_lvl ? USB_PD_MUX_HPD_LVL : 0) |
 			(hpd_irq ? USB_PD_MUX_HPD_IRQ : 0) |
 			(virtual_mux_state[port] & USB_PD_MUX_USB_DP_STATE);
 
-	virtual_mux_update_state(port, new_mux_state);
+	/* HPD ACK isn't required for the EC to continue with its tasks */
+	virtual_mux_update_state(port, new_mux_state, &unused);
 }
 
 const struct usb_mux_driver virtual_usb_mux_driver = {