stm32/usb: Handle spurious wakes in usb_resume

Sometimes, usb_resume gets called, but the D+/D- lines do not
indicate an actual resume event (e.g. during ESD discharge).

Reference manual tells we should go back to sleep if state is 10 or
11. However, setting FSUSP and LP_MODE in this interrupt routine
seems to lock the USB controller (see b/35775088 and b/71688150).
Instead, we do it in a deferred routine. The host must assert the
reset condition for 20ms, so reading D+/D- after ~3ms should be safe
(there is no chance we end up sampling during a bus transaction).

BRANCH=none
BUG=b:35775088
BUG=b:67766202
BUG=b:71688150
TEST=On staff, with USB autosuspend enabled on soraka, short
     D+/D- lines, which causes a spurious wake event. After that
     remote wake-up still works.
TEST=Repeat test with ESD discharge.
TEST=Repeat test with plugging/unplugging of USB-C monitor.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Change-Id: I0f2697d1fa5b68356fd8a4fc16eaab5eadad9086
Reviewed-on: https://chromium-review.googlesource.com/868093
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

1 files changed, 34 insertions, 7 deletions

diff --git a/chip/stm32/usb.c b/chip/stm32/usb.c
index dfec42b5f8..4aec16f8d3 100644
--- a/chip/stm32/usb.c
+++ b/chip/stm32/usb.c
@@ -384,11 +384,17 @@ static void usb_suspend(void)
 {
 	CPRINTF("SUS\n");
 
+	/*
+	 * usb_suspend can be called from hook task, make sure no interrupt is
+	 * modifying CNTR at the same time.
+	 */
+	interrupt_disable();
 	/* Set FSUSP bit to activate suspend mode */
 	STM32_USB_CNTR |= STM32_USB_CNTR_FSUSP;
 
 	/* Set USB low power mode */
 	STM32_USB_CNTR |= STM32_USB_CNTR_LP_MODE;
+	interrupt_enable();
 
 	clock_enable_module(MODULE_USB, 0);
 
@@ -396,15 +402,20 @@ static void usb_suspend(void)
 	enable_sleep(SLEEP_MASK_USB_DEVICE);
 }
 
-static void usb_resume(void)
+static void usb_resume_deferred(void)
 {
-	CPRINTF("RSM\n");
+	uint32_t state = (STM32_USB_FNR & STM32_USB_FNR_RXDP_RXDM_MASK)
+		>> STM32_USB_FNR_RXDP_RXDM_SHIFT;
 
-	/*
-	 * TODO(crosbug.com/p/63273): Reference manual suggests going back to
-	 * sleep if state is 10 or 11, but this seems to cause other problems
-	 * (see bug). Ignore them for now.
-	 */
+	CPRINTF("RSMd %d %04x\n", state, STM32_USB_CNTR);
+	if (state == 2 || state == 3)
+		usb_suspend();
+}
+DECLARE_DEFERRED(usb_resume_deferred);
+
+static void usb_resume(void)
+{
+	uint32_t state;
 
 	clock_enable_module(MODULE_USB, 1);
 
@@ -413,6 +424,22 @@ static void usb_resume(void)
 
 	/* USB is in use again */
 	disable_sleep(SLEEP_MASK_USB_DEVICE);
+
+	state = (STM32_USB_FNR & STM32_USB_FNR_RXDP_RXDM_MASK)
+		>> STM32_USB_FNR_RXDP_RXDM_SHIFT;
+
+	CPRINTF("RSM %d %04x\n", state, STM32_USB_CNTR);
+
+	/*
+	 * Reference manual tells we should go back to sleep if state is 10 or
+	 * 11. However, setting FSUSP and LP_MODE in this interrupt routine
+	 * seems to lock the USB controller (see b/35775088 and b/71688150).
+	 * Instead, we do it in a deferred routine. The host must assert the
+	 * reset condition for 20ms, so reading D+/D- after ~3ms should be safe
+	 * (there is no chance we end up sampling during a bus transaction).
+	 */
+	if (state == 2 || state == 3)
+		hook_call_deferred(&usb_resume_deferred_data, 3 * MSEC);
 }
 
 #ifdef CONFIG_USB_REMOTE_WAKEUP