STM32L476 USB HAL errors Topic is solved

[lilvinz]

Yes, its exactly the same as before.

Code: Select all

  case USB_EP0_SENDING_STS:
    /* All the above are invalid states in the IN phase.*/
    osalDbgAssert(false, "EP0 state machine error");


Code: Select all

Thread #7 536882416 (usb_lld_pump : CURRENT) (Suspended : Signal : SIGINT:Interrupt)   
   chSysHalt() at chsys.c:182 0x802ba42   
   _usb_ep0out() at usb.c:950 0x802ba42   
   usb_lld_serve_interrupt() at usb_lld.c:605 0x8025558   
   Vector14C() at usb_lld.c:654 0x8025558   
   <signal handler called>() at 0xfffffffd   
   0x55555554


Modified usb_lld.c:

Code: Select all

/**
 * @brief   OTG shared ISR.
 *
 * @param[in] usbp      pointer to the @p USBDriver object
 *
 * @notapi
 */
static void usb_lld_serve_interrupt(USBDriver *usbp) {
  stm32_otg_t *otgp = usbp->otg;
  uint32_t sts, src;

  sts  = otgp->GINTSTS;
  sts &= otgp->GINTMSK;
  otgp->GINTSTS = sts;

  /* Reset interrupt handling.*/
  if (sts & GINTSTS_USBRST) {

    /* Resetting pending operations.*/
    usbp->txpending = 0;

    /* Default reset action.*/
    _usb_reset(usbp);

    /* Preventing execution of more handlers, the core has been reset.*/
    return;
  }

  /* Wake-up handling.*/
  if (sts & GINTSTS_WKUPINT) {
    /* If clocks are gated off, turn them back on (may be the case if
       coming out of suspend mode).*/
    if (otgp->PCGCCTL & (PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK)) {
      /* Set to zero to un-gate the USB core clocks.*/
      otgp->PCGCCTL &= ~(PCGCCTL_STPPCLK | PCGCCTL_GATEHCLK);
    }

    /* Clear the Remote Wake-up Signaling.*/
    otgp->DCTL |= DCTL_RWUSIG;

    _usb_wakeup(usbp);
  }

  /* Suspend handling.*/
  if (sts & GINTSTS_USBSUSP) {

    /* Resetting pending operations.*/
    usbp->txpending = 0;

    /* Default suspend action.*/
    _usb_suspend(usbp);
  }

  /* Enumeration done.*/
  if (sts & GINTSTS_ENUMDNE) {
    /* Full or High speed timing selection.*/
    if ((otgp->DSTS & DSTS_ENUMSPD_MASK) == DSTS_ENUMSPD_HS_480) {
      otgp->GUSBCFG = (otgp->GUSBCFG & ~(GUSBCFG_TRDT_MASK)) |
                      GUSBCFG_TRDT(TRDT_VALUE_HS);
    }
    else {
      otgp->GUSBCFG = (otgp->GUSBCFG & ~(GUSBCFG_TRDT_MASK)) |
                      GUSBCFG_TRDT(TRDT_VALUE_FS);
    }
  }

  /* SOF interrupt handling.*/
  if (sts & GINTSTS_SOF) {
    _usb_isr_invoke_sof_cb(usbp);
  }

  /* Isochronous IN failed handling */
  if (sts & GINTSTS_IISOIXFR) {
    otg_isoc_in_failed_handler(usbp);
  }

  /* Isochronous OUT failed handling */
  if (sts & GINTSTS_IISOOXFR) {
    otg_isoc_out_failed_handler(usbp);
  }

  /* RX FIFO not empty handling.*/
  if (sts & GINTSTS_RXFLVL) {
    /* The interrupt is masked while the thread has control or it would
       be triggered again.*/
    osalSysLockFromISR();
    otgp->GINTMSK &= ~GINTMSK_RXFLVLM;
    osalThreadResumeI(&usbp->wait, MSG_OK);
    osalSysUnlockFromISR();
  }

  /* IN/OUT endpoints event handling.*/
  src = otgp->DAINT;
  if (sts & GINTSTS_OEPINT) {
    if (src & (1 << 16))
      otg_epout_handler(usbp, 0);
    if (src & (1 << 17))
      otg_epout_handler(usbp, 1);
    if (src & (1 << 18))
      otg_epout_handler(usbp, 2);
    if (src & (1 << 19))
      otg_epout_handler(usbp, 3);
#if STM32_USB_USE_OTG2
    if (src & (1 << 20))
      otg_epout_handler(usbp, 4);
    if (src & (1 << 21))
      otg_epout_handler(usbp, 5);
#endif
  }
  if (sts & GINTSTS_IEPINT) {
    if (src & (1 << 0))
      otg_epin_handler(usbp, 0);
    if (src & (1 << 1))
      otg_epin_handler(usbp, 1);
    if (src & (1 << 2))
      otg_epin_handler(usbp, 2);
    if (src & (1 << 3))
      otg_epin_handler(usbp, 3);
#if STM32_USB_USE_OTG2
    if (src & (1 << 4))
      otg_epin_handler(usbp, 4);
    if (src & (1 << 5))
      otg_epin_handler(usbp, 5);
#endif
  }
}


Cheers

Vinz

[Giovanni]

Giving up on this for now, the L4 seems to throw interrupts out of order for no apparent reason, there is no way to make the driver resilient to this. There must be some basic problem but I have not found it.

Giovanni

[lilvinz]

Is there any way to support you in finding the root cause?
This really makes usb on the l4 unusable which is a pity.

Cheers

Vinz

[Giovanni]

I'll try to find somebody to ask questions in ST about it, the effect is clear, the root cause is not.

Giovanni

[Giovanni]

Hi,

I finally had a chance to speak with the right person and had a quick answer:

Code: Select all

The USB IP on STM32L4 is slightly different than F4.
While the RM is being edited, you should look at the HAL for L4 and you should be seeing the specific USB code for L4 (the HAL read the USB IP version to branch to specifics)

Let me know if this information is enough to unlock the situation.

On my side, I will ask for a IP / SW compatibility matrix between products to avoir people waste time….


I will look into those code differences when I will have some time, of course everybody is welcome to try.

Giovanni

[lilvinz]

Hi Giovanni,

after some days of investigation i can present a patch against 16.1.8 which is working for me.
In general the OTG peripheral and its documentation seems to be a huge mess. People are complaining about this issue at st forums as well.

Code: Select all

diff --git a/os/hal/ports/STM32/LLD/OTGv1/stm32_otg.h b/os/hal/ports/STM32/LLD/OTGv1/stm32_otg.h
index 2d0ae3b..d017ef3 100644
--- a/os/hal/ports/STM32/LLD/OTGv1/stm32_otg.h
+++ b/os/hal/ports/STM32/LLD/OTGv1/stm32_otg.h
@@ -110,8 +110,17 @@ typedef struct {
   volatile uint32_t resvd30;
   volatile uint32_t resvd34;
   volatile uint32_t GCCFG;      /**< @brief General core configuration.     */
-  volatile uint32_t CID;        /**< @brief Core ID register.               */
-  volatile uint32_t resvd58[48];
+  volatile uint32_t CID;        /**< @brief User ID register.               */
+  volatile uint32_t GSNPSID;    /**< @brief USB_OTG core ID.                */
+  volatile uint32_t GHWCFG1;    /**< @brief User HW config1.                */
+  volatile uint32_t GHWCFG2;    /**< @brief User HW config2.                */
+  volatile uint32_t GHWCFG3;    /**< @brief User HW config3.                */
+  volatile uint32_t resvd50;
+  volatile uint32_t GLPMCFG;    /**< @brief LPM register.                   */
+  volatile uint32_t GPWRDN;     /**< @brief Power down register.            */
+  volatile uint32_t GDFIFOCFG;  /**< @brief DFIFO software config register. */
+  volatile uint32_t GADPCTL;    /**< @brief APD timer, control and status.  */
+  volatile uint32_t resvd64[39];
   volatile uint32_t HPTXFSIZ;   /**< @brief Host periodic transmit FIFO size
                                             register.                       */
   volatile uint32_t DIEPTXF[15];/**< @brief Device IN endpoint transmit FIFO
diff --git a/os/hal/ports/STM32/LLD/OTGv1/usb_lld.c b/os/hal/ports/STM32/LLD/OTGv1/usb_lld.c
index a341706..cab46a4 100644
--- a/os/hal/ports/STM32/LLD/OTGv1/usb_lld.c
+++ b/os/hal/ports/STM32/LLD/OTGv1/usb_lld.c
@@ -403,9 +403,13 @@ static void otg_epin_handler(USBDriver *usbp, usbep_t ep) {
  *
  * @notapi
  */
+#define USB_OTG_CORE_ID_310A          0x4F54310A
+#define USB_OTG_CORE_ID_320A          0x4F54320A
+
 static void otg_epout_handler(USBDriver *usbp, usbep_t ep) {
   stm32_otg_t *otgp = usbp->otg;
   uint32_t epint = otgp->oe[ep].DOEPINT;
+  uint32_t ep0int = otgp->oe[0].DOEPINT;
 
   /* Resets all EP IRQ sources.*/
   otgp->oe[ep].DOEPINT = epint;
@@ -418,31 +422,38 @@ static void otg_epout_handler(USBDriver *usbp, usbep_t ep) {
   if ((epint & DOEPINT_XFRC) && (otgp->DOEPMSK & DOEPMSK_XFRCM)) {
     USBOutEndpointState *osp;
 
-    /* Receive transfer complete, checking if it is a SETUP transfer on EP0,
-       that it must be ignored, the STUPM handler will take care of it.*/
-    if ((ep == 0) && (usbp->ep0state == USB_EP0_WAITING_SETUP))
-      return;
-
-    /* OUT state structure pointer for this endpoint.*/
-    osp = usbp->epc[ep]->out_state;
-
-    /* A short packet always terminates a transaction.*/
-    if ((ep == 0) &&
-        ((osp->rxcnt % usbp->epc[ep]->out_maxsize) == 0) &&
-        (osp->rxsize < osp->totsize)) {
-      /* For EP 0 only, in case the transaction covered only part of the total
-         transfer then another transaction is immediately started in order to
-         cover the remaining.*/
-      osp->rxsize = osp->totsize - osp->rxsize;
-      osp->rxcnt  = 0;
-      osalSysLockFromISR();
-      usb_lld_start_out(usbp, ep);
-      osalSysUnlockFromISR();
-    }
-    else {
-      /* End on OUT transfer.*/
-      _usb_isr_invoke_out_cb(usbp, ep);
+    /* Ignore transfer for core 310a when oep0 interrupt bit 15 is set. */
+    if ((otgp->GSNPSID != USB_OTG_CORE_ID_310A) ||
+        (otgp->GSNPSID == USB_OTG_CORE_ID_310A && (ep0int & (1U << 15)) == 0U)) {
+
+      /* Receive transfer complete, checking if it is a SETUP transfer on EP0,
+         that it must be ignored, the STUPM handler will take care of it.*/
+      if ((ep == 0) && (usbp->ep0state == USB_EP0_WAITING_SETUP))
+        return;
+
+      /* OUT state structure pointer for this endpoint.*/
+      osp = usbp->epc[ep]->out_state;
+      /* A short packet always terminates a transaction.*/
+      if ((ep == 0) &&
+          ((osp->rxcnt % usbp->epc[ep]->out_maxsize) == 0) &&
+          (osp->rxsize < osp->totsize)) {
+        /* For EP 0 only, in case the transaction covered only part of the total
+           transfer then another transaction is immediately started in order to
+           cover the remaining.*/
+        osp->rxsize = osp->totsize - osp->rxsize;
+        osp->rxcnt  = 0;
+        osalSysLockFromISR();
+        usb_lld_start_out(usbp, ep);
+        osalSysUnlockFromISR();
+      }
+      else {
+        /* End on OUT transfer.*/
+        _usb_isr_invoke_out_cb(usbp, ep);
+      }
     }
+
+    if (otgp->GSNPSID == USB_OTG_CORE_ID_310A)
+      otgp->oe[0].DOEPINT |= (1U << 15) | (1U << 5);
   }
 }


I could imagine that the otg version is constant among a family of stm32 and could thus be known at compile time (registry) but i am not sure about this. The patch is based on findings in the st drivers.

Cheers

Vinz

[Giovanni]

Great!! thanks for finding.

I think that the ID check could be implemented as a "stepping" 3 using the existing registry key, there is no need to check the version at runtime.

Giovanni

[lilvinz]

I think that the ID check could be implemented as a "stepping" 3 using the existing registry key, there is no need to check the version at runtime.

I am a bit hesitating as this looks more like a workaround for a bug in the core than a new feature. They are explicitly checking for one core version and there seems to be a newer one which is not affected. I don't know about ST product update policy but as the L4 series is quite new, they could make a newer revision with a fixed (updated) OTG core.

Cheers

Vinz

[Giovanni]

So you think it is a bug, not just a new OTG IP version, interesting.

Can you confirm there are newer L4s not affected?

Giovanni

[lilvinz]

I can't confirm but ST is specifically checking for IP version 310A and has defined a constant for 320A which does not trigger the "workaround".

Cheers

Vinz