STM32 OTGv1 Suspend and Resume Fixes

[KarlK90]

Hi Giovanni,

I have another patch for the resume and suspend handling in the OTGv1 driver.

1. Problem:

After resuming from suspend, either by being woken by the host or by waking the host, the already setup'ed in and out endpoints remain disabled. The cause is that otg_disable_ep is called on the suspend condition - disabling the irqs for all endpoints except ep0 and they are never reactivated after coming out of suspend.

See: https://github.com/ChibiOS/ChibiOS/blob ... lld.c#L170

The only other place in the code where the endpoints are explicitly enabled is usb_lld_init_endpoint, but it doesn't make sense to call that in the wakeup callback.

See: https://github.com/ChibiOS/ChibiOS/blob ... ld.c#L1022

My solution so far is to enable the endpoints after coming out of suspend again:

Code: Select all

diff --git a/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.c b/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.c
index c52ba5c5dc..54f40ec302 100644
--- a/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.c
+++ b/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.c
@@ -170,6 +170,20 @@ static void otg_disable_ep(USBDriver *usbp) {
   otgp->DAINTMSK = DAINTMSK_OEPM(0) | DAINTMSK_IEPM(0);
 }
 
+static void otg_enable_ep(USBDriver *usbp) {
+  stm32_otg_t *otgp = usbp->otg;
+  unsigned i;
+
+  for (i = 0; i <= usbp->otgparams->num_endpoints; i++) {
+    if (usbp->epc[i]->out_state != NULL) {
+      otgp->DAINTMSK |= DAINTMSK_OEPM(i);
+    }
+    if (usbp->epc[i]->in_state != NULL) {
+      otgp->DAINTMSK |= DAINTMSK_IEPM(i);
+    }
+  }
+}
+
 static void otg_rxfifo_flush(USBDriver *usbp) {
   stm32_otg_t *otgp = usbp->otg;
 
@@ -553,6 +567,9 @@ static void usb_lld_serve_interrupt(USBDriver *usbp) {
       otgp->PCGCCTL &= ~(PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK);
     }
 
+    /* Re-enable endpoint irqs if they have been disabled by suspend before. */
+    otg_enable_ep(usbp);
+
     /* Clear the Remote Wake-up Signaling.*/
     otgp->DCTL &= ~DCTL_RWUSIG;
 
@@ -595,6 +612,10 @@ static void usb_lld_serve_interrupt(USBDriver *usbp) {
         /* Set to zero to un-gate the USB core clocks.*/
         otgp->PCGCCTL &= ~(PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK);
       }
+
+      /* Re-enable endpoint irqs if they have been disabled by suspend before. */
+      otg_enable_ep(usbp);
+
       _usb_wakeup(usbp);
     }


[KarlK90]

2. Problem

If the application disables the OTG clocks in low power mode in order to save some power, these clocks have to be enabled again if they want to wake the host - otherwise nothing happens:

Code: Select all

diff --git a/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.h b/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.h
index c9d2be1649..ddaa751ae0 100644
--- a/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.h
+++ b/os/hal/ports/STM32/LLD/OTGv1/hal_usb_lld.h
@@ -557,6 +557,12 @@ struct USBDriver {
  */
 #define usb_lld_wakeup_host(usbp)                                           \
   do {                                                                      \
+    /* If clocks are gated off, turn them back on (may be the case if
+       coming out of suspend mode).*/                                       \
+    if ((usbp)->otg->PCGCCTL & (PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK)) {      \
+      /* Set to zero to un-gate the USB core clocks.*/                      \
+      (usbp)->otg->PCGCCTL &= ~(PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK);        \
+    }                                                                       \
     (usbp)->otg->DCTL |= DCTL_RWUSIG;                                       \
     /* remote wakeup doesn't trigger the wakeup interrupt, therefore
        we use the SOF interrupt to detect resume of the bus.*/              \