tcpc: wait for TCPC wake up upon first access

Previously we tried to perform i2c communication with the TCPC when it
was in low power mode and only if the i2c transaction failed did we wait
for the device to wake up. Now that the PS8751 will respond to i2c
transaction (ACK them) when it is in LPM, ensure the init happers before
we start talking to the device to handle an interrupt.

BRANCH=none
BUG=b:118063849,b:121109893
TEST=verify that plug and unplug for PS8751, ANX3429, and ANX3447 LPM
still works.

Change-Id: I8c18195e55ee6d04af7d4ff24230a3bd2d147d53
Signed-off-by: Jett Rink <jettrink@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1375102
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Edward Hill <ecgh@chromium.org>

3 files changed, 98 insertions, 90 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 4ab395afad..e3eb258556 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -410,25 +410,7 @@ static void handle_device_access(int port)
 	}
 }
 
-/*
- * This can be called from any task. If we are in the PD task, we can handle
- * immediately. Otherwise, we need to notify the PD task via event.
- */
-void pd_device_accessed(int port)
-{
-	if (port == TASK_ID_TO_PD_PORT(task_get_current())) {
-		/* Ignore any access to device while it is waking up */
-		if (pd[port].flags & PD_FLAGS_LPM_TRANSITION)
-			return;
-
-		handle_device_access(port);
-	}
-	else
-		task_set_event(PD_PORT_TO_TASK_ID(port),
-			       PD_EVENT_DEVICE_ACCESSED, 0);
-}
-
-int pd_device_in_low_power(int port)
+static int pd_device_in_low_power(int port)
 {
 	/*
 	 * If we are actively waking the device up in the PD task, do not
@@ -458,6 +440,20 @@ static int reset_device_and_notify(int port)
 	else
 		CPRINTS("TCPC p%d init failed!", port);
 
+	/*
+	 * Before getting the other tasks that are waiting, clear the reset
+	 * event from this PD task to prevent multiple reset/init events
+	 * occurring.
+	 *
+	 * The double reset event happens when the higher priority PD interrupt
+	 * task gets an interrupt during the above tcpm_init function. When that
+	 * occurs, the higher priority task waits correctly for us to finish
+	 * waking the TCPC, but it has also set PD_EVENT_TCPC_RESET again, which
+	 * would result in a second, unnecessary init.
+	 */
+	atomic_clear(task_get_event_bitmap(task_get_current()),
+		     PD_EVENT_TCPC_RESET);
+
 	waiting_tasks = atomic_read_clear(&pd[port].tasks_waiting_on_reset);
 
 	/*
@@ -479,7 +475,7 @@ static int reset_device_and_notify(int port)
 	return rv;
 }
 
-void pd_wait_for_wakeup(int port)
+static void pd_wait_for_wakeup(int port)
 {
 	if (port == TASK_ID_TO_PD_PORT(task_get_current())) {
 		/* If we are in the PD task, we can directly reset */
@@ -502,6 +498,30 @@ void pd_wait_for_wakeup(int port)
 	}
 }
 
+void pd_wait_exit_low_power(int port)
+{
+	if (pd_device_in_low_power(port))
+		pd_wait_for_wakeup(port);
+}
+
+/*
+ * This can be called from any task. If we are in the PD task, we can handle
+ * immediately. Otherwise, we need to notify the PD task via event.
+ */
+void pd_device_accessed(int port)
+{
+	if (port == TASK_ID_TO_PD_PORT(task_get_current())) {
+		/* Ignore any access to device while it is waking up */
+		if (pd[port].flags & PD_FLAGS_LPM_TRANSITION)
+			return;
+
+		handle_device_access(port);
+	} else {
+		task_set_event(PD_PORT_TO_TASK_ID(port),
+			       PD_EVENT_DEVICE_ACCESSED, 0);
+	}
+}
+
 void pd_prevent_low_power_mode(int port, int prevent)
 {
 	const int current_task_mask = (1 << task_get_current());
diff --git a/driver/tcpm/tcpci.c b/driver/tcpm/tcpci.c
index 00067c18d6..e3c266a478 100644
--- a/driver/tcpm/tcpci.c
+++ b/driver/tcpm/tcpci.c
@@ -32,80 +32,78 @@ static int selected_rp[CONFIG_USB_PD_PORT_COUNT];
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 int tcpc_write(int port, int reg, int val)
 {
-	int rv = i2c_write8(tcpc_config[port].i2c_host_port,
-			    tcpc_config[port].i2c_slave_addr, reg, val);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_write8(tcpc_config[port].i2c_host_port,
-				tcpc_config[port].i2c_slave_addr, reg, val);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_write8(tcpc_config[port].i2c_host_port,
+			tcpc_config[port].i2c_slave_addr, reg, val);
+
 	pd_device_accessed(port);
 	return rv;
 }
 
 int tcpc_write16(int port, int reg, int val)
 {
-	int rv = i2c_write16(tcpc_config[port].i2c_host_port,
-			     tcpc_config[port].i2c_slave_addr, reg, val);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_write16(tcpc_config[port].i2c_host_port,
-				 tcpc_config[port].i2c_slave_addr, reg, val);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_write16(tcpc_config[port].i2c_host_port,
+			 tcpc_config[port].i2c_slave_addr, reg, val);
+
 	pd_device_accessed(port);
 	return rv;
 }
 
 int tcpc_read(int port, int reg, int *val)
 {
-	int rv = i2c_read8(tcpc_config[port].i2c_host_port,
-			   tcpc_config[port].i2c_slave_addr, reg, val);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_read8(tcpc_config[port].i2c_host_port,
-			       tcpc_config[port].i2c_slave_addr, reg, val);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_read8(tcpc_config[port].i2c_host_port,
+		       tcpc_config[port].i2c_slave_addr, reg, val);
+
 	pd_device_accessed(port);
 	return rv;
 }
 
 int tcpc_read16(int port, int reg, int *val)
 {
-	int rv = i2c_read16(tcpc_config[port].i2c_host_port,
-			    tcpc_config[port].i2c_slave_addr, reg, val);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_read16(tcpc_config[port].i2c_host_port,
-				tcpc_config[port].i2c_slave_addr, reg, val);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_read16(tcpc_config[port].i2c_host_port,
+			tcpc_config[port].i2c_slave_addr, reg, val);
+
 	pd_device_accessed(port);
 	return rv;
 }
 
 int tcpc_read_block(int port, int reg, uint8_t *in, int size)
 {
-	int rv = i2c_read_block(tcpc_config[port].i2c_host_port,
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_read_block(tcpc_config[port].i2c_host_port,
 			    tcpc_config[port].i2c_slave_addr, reg, in, size);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_read_block(tcpc_config[port].i2c_host_port,
-				tcpc_config[port].i2c_slave_addr, reg,
-				in, size);
-	}
+
 	pd_device_accessed(port);
 	return rv;
 }
 
 int tcpc_write_block(int port, int reg, const uint8_t *out, int size)
 {
-	int rv = i2c_write_block(tcpc_config[port].i2c_host_port,
-			    tcpc_config[port].i2c_slave_addr, reg, out, size);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_write_block(tcpc_config[port].i2c_host_port,
-				tcpc_config[port].i2c_slave_addr, reg,
-				out, size);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_write_block(tcpc_config[port].i2c_host_port,
+			     tcpc_config[port].i2c_slave_addr, reg, out, size);
+
 	pd_device_accessed(port);
 	return rv;
 }
@@ -125,15 +123,14 @@ int tcpc_xfer(int port, const uint8_t *out, int out_size,
 int tcpc_xfer_unlocked(int port, const uint8_t *out, int out_size,
 			    uint8_t *in, int in_size, int flags)
 {
-	int rv = i2c_xfer_unlocked(tcpc_config[port].i2c_host_port,
-			  tcpc_config[port].i2c_slave_addr, out, out_size,
-			  in, in_size, flags);
-	if (rv && pd_device_in_low_power(port)) {
-		pd_wait_for_wakeup(port);
-		rv = i2c_xfer_unlocked(tcpc_config[port].i2c_host_port,
-			      tcpc_config[port].i2c_slave_addr, out, out_size,
-			      in, in_size, flags);
-	}
+	int rv;
+
+	pd_wait_exit_low_power(port);
+
+	rv = i2c_xfer_unlocked(tcpc_config[port].i2c_host_port,
+				   tcpc_config[port].i2c_slave_addr, out,
+				   out_size, in, in_size, flags);
+
 	pd_device_accessed(port);
 	return rv;
 }
diff --git a/include/usb_pd.h b/include/usb_pd.h
index f451206b4b..ceafd18c9d 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -1039,6 +1039,15 @@ int pd_build_request(int port, uint32_t *rdo, uint32_t *ma, uint32_t *mv,
 int pd_is_max_request_allowed(void);
 
 /**
+ * Waits for the TCPC to exit low power mode (including re-initializing) if it
+ * is currently in low power mode. If not, then the function immediately
+ * returns.
+ *
+ * @param port USB-C port number
+ */
+void pd_wait_exit_low_power(int port);
+
+/**
  * Informs the TCPM state machine that code within the EC has accessed the TCPC
  * via its communication bus (e.g. i2c). This is important to keep track of as
  * accessing a TCPC may pull the hardware out of low-power mode.
@@ -1062,24 +1071,6 @@ void pd_device_accessed(int port);
 void pd_prevent_low_power_mode(int port, int prevent);
 
 /**
- * Returns true if this TCPC is in low power mode and a failed i2c transaction
- * should be retried after waiting for the device to wake up via
- * pd_wait_for_wakeup()
- *
- * @param port USB-C port number
- * @return True if device is in LPM and i2c transaction should be retried
- */
-int pd_device_in_low_power(int port);
-
-/**
- * Requests that the PD task wakeup the TCPC out of low power mode properly, and
- * waits for the wakeup operation to complete.
- *
- * @param port USB-C port number
- */
-void pd_wait_for_wakeup(int port);
-
-/**
  * Process source capabilities packet
  *
  * @param port USB-C port number