You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "raiden00pl (via GitHub)" <gi...@apache.org> on 2023/06/10 09:26:34 UTC

[GitHub] [nuttx] raiden00pl opened a new pull request, #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

raiden00pl opened a new pull request, #9508:
URL: https://github.com/apache/nuttx/pull/9508

   ## Summary
   - arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability
     USBD has many limitation that make it hard to work properly:
     - only one EasyDMA channel which must be shared for all EPs
     - only one DMA transfer can take place at a time
     - some registers are unavailable during DMA transfer
     - in case of any problems, the peripheral silently blocks, or lose the transmitted bytes without information for the user
   
     This PR is trying to fix these problem and makes the USBS stack more reliable.
   
   ## Impact
   
   ## Testing
   Tested with high-speed CDCACM data transfers
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9508:
URL: https://github.com/apache/nuttx/pull/9508#discussion_r1225442043


##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf52_endepoutinterrupt(struct nrf52_usbdev_s *priv)
 
 static int nrf52_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf52_usbdev_s *priv = &g_usbdev;
+  struct nrf52_usbdev_s *priv   = &g_usbdev;
+  struct nrf52_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF52_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF52_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf52_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf52_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_DEVRESET), 0);
       nrf52_usbreset(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_SOF), 0);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_USBEVENT), 0);
       nrf52_eventinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF52_USBD_DMAIRQ)
+    {
+      nrf52_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0SETUP), 0);
       nrf52_ep0setupinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0DATADONE), 0);
       nrf52_ep0datainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf52_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf52_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EPDATA), 0);
       nrf52_epdatainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf52_endepininterrupt(priv);
-  nrf52_endepoutinterrupt(priv);
+  if (priv->dmanow == false)
+    {
+      if (priv->dmaepoutwait)
+        {
+          for (i = 0; i < NRF52_NENDPOINTS; i++)
+            {
+              if (priv->dmaepoutwait & (1 << i))
+                {
+                  priv->dmaepoutwait &= ~(1 << i);
+
+                  privep = &priv->epout[i];
+                  nrf52_epout_handle(priv, privep);
+                  break;
+                }
+            }
+        }
+    }
+
+  /* Try to handle waiting IN DMA requests */
+
+  if (priv->dmanow == false)

Review Comment:
   Thank you for explanation!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9508:
URL: https://github.com/apache/nuttx/pull/9508#discussion_r1225473694


##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1020,18 +1032,26 @@ static void nrf52_epin_transfer(struct nrf52_ep_s *privep, uint8_t *buf,
   DEBUGASSERT(privep->eptype != USB_EP_ATTR_XFER_ISOC);
   DEBUGASSERT(nbytes <= 64);
 
-  if (nbytes > 0)
-    {
-      /* Configure EasyDMA */
+  /* Configure EasyDMA */
 
+  if (buf)

Review Comment:
   ```suggestion
     if (buf != NULL)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] acassis merged pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #9508:
URL: https://github.com/apache/nuttx/pull/9508


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] raiden00pl commented on a diff in pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on code in PR #9508:
URL: https://github.com/apache/nuttx/pull/9508#discussion_r1225250145


##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf52_endepoutinterrupt(struct nrf52_usbdev_s *priv)
 
 static int nrf52_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf52_usbdev_s *priv = &g_usbdev;
+  struct nrf52_usbdev_s *priv   = &g_usbdev;
+  struct nrf52_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF52_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF52_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf52_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf52_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_DEVRESET), 0);
       nrf52_usbreset(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_SOF), 0);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_USBEVENT), 0);
       nrf52_eventinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF52_USBD_DMAIRQ)
+    {
+      nrf52_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0SETUP), 0);
       nrf52_ep0setupinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0DATADONE), 0);
       nrf52_ep0datainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf52_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf52_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EPDATA), 0);
       nrf52_epdatainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf52_endepininterrupt(priv);
-  nrf52_endepoutinterrupt(priv);
+  if (priv->dmanow == false)
+    {
+      if (priv->dmaepoutwait)
+        {
+          for (i = 0; i < NRF52_NENDPOINTS; i++)
+            {
+              if (priv->dmaepoutwait & (1 << i))
+                {
+                  priv->dmaepoutwait &= ~(1 << i);
+
+                  privep = &priv->epout[i];
+                  nrf52_epout_handle(priv, privep);
+                  break;
+                }
+            }
+        }
+    }
+
+  /* Try to handle waiting IN DMA requests */
+
+  if (priv->dmanow == false)

Review Comment:
   It can't be. EPOUT must be handled first and if there is a pending EPOUT then the DMA will be taken (`priv->dmanow` set to `true` by `nrf52_epout_handle()`) and the pending EPIN must be skipped in this interrupt cycle.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] raiden00pl commented on a diff in pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "raiden00pl (via GitHub)" <gi...@apache.org>.
raiden00pl commented on code in PR #9508:
URL: https://github.com/apache/nuttx/pull/9508#discussion_r1225251814


##########
arch/arm/src/nrf53/nrf53_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf53_endepoutinterrupt(struct nrf53_usbdev_s *priv)
 
 static int nrf53_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf53_usbdev_s *priv = &g_usbdev;
+  struct nrf53_usbdev_s *priv   = &g_usbdev;
+  struct nrf53_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF53_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF53_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf53_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf53_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_DEVRESET), 0);
       nrf53_usbreset(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_SOF), 0);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_USBEVENT), 0);
       nrf53_eventinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF53_USBD_DMAIRQ)
+    {
+      nrf53_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0SETUP), 0);
       nrf53_ep0setupinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0DATADONE), 0);
       nrf53_ep0datainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf53_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf53_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EPDATA), 0);
       nrf53_epdatainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf53_endepininterrupt(priv);
-  nrf53_endepoutinterrupt(priv);
+  if (priv->dmanow == false)
+    {
+      if (priv->dmaepoutwait)
+        {
+          for (i = 0; i < NRF53_NENDPOINTS; i++)
+            {
+              if (priv->dmaepoutwait & (1 << i))
+                {
+                  priv->dmaepoutwait &= ~(1 << i);
+
+                  privep = &priv->epout[i];
+                  nrf53_epout_handle(priv, privep);
+                  break;
+                }
+            }
+        }
+    }
+
+  /* Try to handle waiting IN DMA requests */
+
+  if (priv->dmanow == false)

Review Comment:
   same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #9508: arch/{nrf52|nrf53}/usbd: various fixes to improve USB stability

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #9508:
URL: https://github.com/apache/nuttx/pull/9508#discussion_r1225237516


##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf52_endepoutinterrupt(struct nrf52_usbdev_s *priv)
 
 static int nrf52_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf52_usbdev_s *priv = &g_usbdev;
+  struct nrf52_usbdev_s *priv   = &g_usbdev;
+  struct nrf52_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF52_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF52_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf52_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf52_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_DEVRESET), 0);
       nrf52_usbreset(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_SOF), 0);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_USBEVENT), 0);
       nrf52_eventinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF52_USBD_DMAIRQ)
+    {
+      nrf52_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0SETUP), 0);
       nrf52_ep0setupinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0DATADONE), 0);
       nrf52_ep0datainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf52_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf52_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EPDATA), 0);
       nrf52_epdatainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf52_endepininterrupt(priv);
-  nrf52_endepoutinterrupt(priv);
+  if (priv->dmanow == false)

Review Comment:
   ```suggestion
     if (!priv->dmanow)
   ```
   



##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf52_endepoutinterrupt(struct nrf52_usbdev_s *priv)
 
 static int nrf52_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf52_usbdev_s *priv = &g_usbdev;
+  struct nrf52_usbdev_s *priv   = &g_usbdev;
+  struct nrf52_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF52_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF52_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf52_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf52_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_DEVRESET), 0);
       nrf52_usbreset(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_SOF), 0);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_USBEVENT), 0);
       nrf52_eventinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF52_USBD_DMAIRQ)
+    {
+      nrf52_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0SETUP), 0);
       nrf52_ep0setupinterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EP0DATADONE), 0);
       nrf52_ep0datainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf52_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf52_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf52_getreg(NRF52_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF52_TRACEINTID_EPDATA), 0);
       nrf52_epdatainterrupt(priv);
-      nrf52_putreg(0, NRF52_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf52_endepininterrupt(priv);
-  nrf52_endepoutinterrupt(priv);
+  if (priv->dmanow == false)
+    {
+      if (priv->dmaepoutwait)
+        {
+          for (i = 0; i < NRF52_NENDPOINTS; i++)
+            {
+              if (priv->dmaepoutwait & (1 << i))
+                {
+                  priv->dmaepoutwait &= ~(1 << i);
+
+                  privep = &priv->epout[i];
+                  nrf52_epout_handle(priv, privep);
+                  break;
+                }
+            }
+        }
+    }
+
+  /* Try to handle waiting IN DMA requests */
+
+  if (priv->dmanow == false)

Review Comment:
   ```suggestion
     if (!priv->dmanow)
   ```
   Can be combined into one if with above loop



##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1342,6 +1392,15 @@ static void nrf52_epout_handle(struct nrf52_usbdev_s *priv,
 
   DEBUGASSERT(privep->epphy != EP0);
 
+  /* If DMA is busy, add EP OUT to the waiting list */
+
+  if (priv->dmanow == true)

Review Comment:
   ```suggestion
     if (priv->dmanow)
   ```
   



##########
arch/arm/src/nrf53/nrf53_usbd.c:
##########
@@ -1020,18 +1032,26 @@ static void nrf53_epin_transfer(struct nrf53_ep_s *privep, uint8_t *buf,
   DEBUGASSERT(privep->eptype != USB_EP_ATTR_XFER_ISOC);
   DEBUGASSERT(nbytes <= 64);
 
-  if (nbytes > 0)
-    {
-      /* Configure EasyDMA */
+  /* Configure EasyDMA */
 
+  if (buf)

Review Comment:
   ```suggestion
     if (buf != NULL)
   ```
   



##########
arch/arm/src/nrf53/nrf53_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf53_endepoutinterrupt(struct nrf53_usbdev_s *priv)
 
 static int nrf53_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf53_usbdev_s *priv = &g_usbdev;
+  struct nrf53_usbdev_s *priv   = &g_usbdev;
+  struct nrf53_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF53_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF53_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf53_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf53_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_DEVRESET), 0);
       nrf53_usbreset(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_SOF), 0);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_USBEVENT), 0);
       nrf53_eventinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF53_USBD_DMAIRQ)
+    {
+      nrf53_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0SETUP), 0);
       nrf53_ep0setupinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0DATADONE), 0);
       nrf53_ep0datainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf53_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf53_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EPDATA), 0);
       nrf53_epdatainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf53_endepininterrupt(priv);
-  nrf53_endepoutinterrupt(priv);
+  if (priv->dmanow == false)

Review Comment:
   ```suggestion
     if (!priv->dmanow)
   ```
   



##########
arch/arm/src/nrf53/nrf53_usbd.c:
##########
@@ -1943,70 +1990,135 @@ static void nrf53_endepoutinterrupt(struct nrf53_usbdev_s *priv)
 
 static int nrf53_usbinterrupt(int irq, void *context, void *arg)
 {
-  struct nrf53_usbdev_s *priv = &g_usbdev;
+  struct nrf53_usbdev_s *priv   = &g_usbdev;
+  struct nrf53_ep_s     *privep = NULL;
+  uint32_t               irqnow = 0;
+  uint32_t               offset = 0;
+  int                    i      = 0;
 
   usbtrace(TRACE_INTENTRY(NRF53_TRACEINTID_USB), 0);
 
+  /* Handle all events */
+
+  for (i = 0; i < USBD_INT_ALL_NUM; i++)
+    {
+      /* Get EVENT offset */
+
+      offset = NRF53_USBD_EVENTS_USBRESET + 0x04 * i;
+
+      /* Get EVENT state */
+
+      irqnow |= nrf53_getreg(offset) << i;
+
+      /* Clear EVENT */
+
+      nrf53_putreg(0, offset);
+    }
+
   /* USB reset interrupt */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBRESET))
+  if (irqnow & USBD_INT_USBRESET)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_DEVRESET), 0);
       nrf53_usbreset(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBRESET);
       goto intout;
     }
 
 #ifdef CONFIG_USBDEV_SOFINTERRUPT
   /* Handle SOF */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_SOF))
+  if (irqnow & USBD_INT_SOF)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_SOF), 0);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_SOF);
     }
 #endif
 
   /* Handle USBEVENT */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_USBEVENT))
+  if (irqnow & USBD_INT_USBEVENT)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_USBEVENT), 0);
       nrf53_eventinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_USBEVENT);
+    }
+
+  /* DMA transfer complete */
+
+  if (irqnow & NRF53_USBD_DMAIRQ)
+    {
+      nrf53_startdma_ack(priv);
     }
 
   /* Handle EP0SETUP */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0SETUP))
+  if (irqnow & USBD_INT_EP0SETUP)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0SETUP), 0);
       nrf53_ep0setupinterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0SETUP);
     }
 
   /* Handle EP0DATADONE */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EP0DATADONE))
+  if (irqnow & USBD_INT_EP0DATADONE)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EP0DATADONE), 0);
       nrf53_ep0datainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EP0DATADONE);
     }
 
+  /* Handle ENDEPOUT */
+
+  nrf53_endepoutinterrupt(priv, irqnow);
+
+  /* Handle ENDEPIN */
+
+  nrf53_endepininterrupt(priv, irqnow);
+
   /* Handle EPDATA */
 
-  if (nrf53_getreg(NRF53_USBD_EVENTS_EPDATA))
+  if (irqnow & USBD_INT_EPDATA)
     {
       usbtrace(TRACE_INTDECODE(NRF53_TRACEINTID_EPDATA), 0);
       nrf53_epdatainterrupt(priv);
-      nrf53_putreg(0, NRF53_USBD_EVENTS_EPDATA);
     }
 
-  /* Handle all END events */
+  /* Try to handle waiting OUT DMA requests */
 
-  nrf53_endepininterrupt(priv);
-  nrf53_endepoutinterrupt(priv);
+  if (priv->dmanow == false)
+    {
+      if (priv->dmaepoutwait)
+        {
+          for (i = 0; i < NRF53_NENDPOINTS; i++)
+            {
+              if (priv->dmaepoutwait & (1 << i))
+                {
+                  priv->dmaepoutwait &= ~(1 << i);
+
+                  privep = &priv->epout[i];
+                  nrf53_epout_handle(priv, privep);
+                  break;
+                }
+            }
+        }
+    }
+
+  /* Try to handle waiting IN DMA requests */
+
+  if (priv->dmanow == false)

Review Comment:
   ```suggestion
     if (!priv->dmanow)
   ```
   I think this can be combined with same if condition above



##########
arch/arm/src/nrf52/nrf52_usbd.c:
##########
@@ -1099,6 +1135,15 @@ static void nrf52_epin_request(struct nrf52_usbdev_s *priv,
   int                 bytesleft = 0;
   int                 nbytes    = 0;
 
+  /* If DMA is busy, add EP IN to the waiting list */
+
+  if (priv->dmanow == true)

Review Comment:
   ```suggestion
     if (priv->dmanow)
   ```
   



##########
arch/arm/src/nrf53/nrf53_usbd.c:
##########
@@ -1099,6 +1135,15 @@ static void nrf53_epin_request(struct nrf53_usbdev_s *priv,
   int                 bytesleft = 0;
   int                 nbytes    = 0;
 
+  /* If DMA is busy, add EP IN to the waiting list */
+
+  if (priv->dmanow == true)

Review Comment:
   ```suggestion
     if (priv->dmanow)
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org