Merge pull request #416 from hathach/fix-nrf-called-within-critical

fix nrf hanged (blocking wait) when called within critical section

2 files changed, 73 insertions, 24 deletions

diff --git a/examples/device/cdc_msc/ses/nrf5x/nrf5x.emProject b/examples/device/cdc_msc/ses/nrf5x/nrf5x.emProject
index fa5534635..f7714f269 100644
--- a/examples/device/cdc_msc/ses/nrf5x/nrf5x.emProject
+++ b/examples/device/cdc_msc/ses/nrf5x/nrf5x.emProject
@@ -19,8 +19,9 @@
       arm_target_device_name="nRF52840_xxAA"
       arm_target_interface_type="SWD"
       build_treat_warnings_as_errors="Yes"
-      c_preprocessor_definitions="NRF52840_XXAA;__nRF_FAMILY;ARM_MATH_CM4;FLASH_PLACEMENT=1;CFG_TUSB_MCU=OPT_MCU_NRF5X"
-      c_user_include_directories="../../src;$(rootDir)/hw/mcu/nordic/cmsis/Include;$(rootDir)/hw;$(rootDir)/src;$(nrfxDir)/..;$(nrfxDir);$(nrfxDir)/mdk;$(nrfxDir)/hal;$(nrfxDir)/drivers/include;$(nrfxDir)/drivers/src"
+      c_additional_options="-Wno-error=undef;-Wno-error=unused-parameter;-Wno-error=cast-align;-Wno-error=cast-function-type"
+      c_preprocessor_definitions="NRF52840_XXAA;__nRF_FAMILY;ARM_MATH_CM4;FLASH_PLACEMENT=1;CFG_TUSB_MCU=OPT_MCU_NRF5X;CFG_TUSB_DEBUG=1"
+      c_user_include_directories="../../src;$(rootDir)/lib/CMSIS_4/CMSIS/Include;$(rootDir)/hw;$(rootDir)/src;$(nrfxDir)/..;$(nrfxDir);$(nrfxDir)/mdk;$(nrfxDir)/hal;$(nrfxDir)/drivers/include;$(nrfxDir)/drivers/src"
       debug_register_definition_file="nrf52840_Registers.xml"
       debug_target_connection="J-Link"
       gcc_enable_all_warnings="Yes"
@@ -42,11 +43,12 @@
       recurse="Yes" />
     <folder Name="hw">
       <folder Name="bsp">
-        <folder Name="pca10056">
-          <file file_name="../../../../../hw/bsp/pca10056/pca10056.c" />
-        </folder>
         <file file_name="../../../../../hw/bsp/ansi_escape.h" />
         <file file_name="../../../../../hw/bsp/board.h" />
+        <file file_name="../../../../../hw/bsp/board.c" />
+        <folder Name="feather_nrf52840_express">
+          <file file_name="../../../../../hw/bsp/feather_nrf52840_express/feather_nrf52840_express.c" />
+        </folder>
       </folder>
       <folder Name="mcu">
         <folder Name="nordic">
@@ -103,16 +105,10 @@
       <file file_name="thumb_crt0.s" />
       <file file_name="nRF52840_xxAA_s140v6_MemoryMap.xml" />
     </folder>
-    <folder
-      Name="segger_rtt"
-      exclude=""
-      filter="*.c;*.h"
-      path="../../../../../lib/segger_rtt"
-      recurse="No" />
     <configuration
       Name="pca10056"
       build_treat_warnings_as_errors="No"
-      c_preprocessor_definitions="BOARD_PCA10056"
+      c_preprocessor_definitions=""
       linker_memory_map_file="nRF52840_xxAA_MemoryMap.xml" />
     <configuration
       Name="pca10056 s140v6"
@@ -121,7 +117,16 @@
       c_user_include_directories="$(nrfxDir)/../nrf5x/s140_nrf52_6.1.1_API/include;$(nrfxDir)/../nrf5x/s140_nrf52_6.1.1_API/include/nrf52"
       debug_start_from_entry_point_symbol="No"
       linker_memory_map_file="nRF52840_xxAA_s140v6_MemoryMap.xml" />
+    <folder Name="SEGGER_RTT">
+      <folder Name="RTT">
+        <file file_name="../../../../../lib/SEGGER_RTT/RTT/SEGGER_RTT.c" />
+        <file file_name="../../../../../lib/SEGGER_RTT/RTT/SEGGER_RTT.h" />
+        <file file_name="../../../../../lib/SEGGER_RTT/RTT/SEGGER_RTT_Conf.h" />
+        <file file_name="../../../../../lib/SEGGER_RTT/RTT/SEGGER_RTT_printf.c" />
+      </folder>
+    </folder>
   </project>
   <configuration Name="pca10056" />
   <configuration Name="pca10056 s140v6" />
+  <configuration Name="Feather nRF52840" />
 </solution>
diff --git a/src/portable/nordic/nrf5x/dcd_nrf5x.c b/src/portable/nordic/nrf5x/dcd_nrf5x.c
index d788547b8..7ca32c4f6 100644
--- a/src/portable/nordic/nrf5x/dcd_nrf5x.c
+++ b/src/portable/nordic/nrf5x/dcd_nrf5x.c
@@ -51,6 +51,11 @@ enum
                       USBD_INTENCLR_ENDISOIN_Msk | USBD_INTEN_ENDISOOUT_Msk
 };
 
+enum
+{
+  EP_COUNT = 8
+};
+
 // Transfer descriptor
 typedef struct
 {
@@ -69,36 +74,73 @@ typedef struct
 static struct
 {
   // All 8 endpoints including control IN & OUT (offset 1)
-  xfer_td_t xfer[8][2];
+  xfer_td_t xfer[EP_COUNT][2];
 
-  // Only one DMA can run at a time
-  volatile bool dma_running;
+  // Number of pending DMA that is started but not handled yet by dcd_int_handler().
+  // Since nRF can only carry one DMA can run at a time, this value is normally be either 0 or 1.
+  // However, in critical section with interrupt disabled, the DMA can be finished and added up
+  // until handled by dcd_init_handler() when exiting critical section.
+  volatile uint8_t dma_pending;
 }_dcd;
 
 /*------------------------------------------------------------------*/
 /* Control / Bulk / Interrupt (CBI) Transfer
  *------------------------------------------------------------------*/
 
+// NVIC_GetEnableIRQ is only available in CMSIS v5
+#ifndef NVIC_GetEnableIRQ
+static inline uint32_t NVIC_GetEnableIRQ(IRQn_Type IRQn)
+{
+  if ((int32_t)(IRQn) >= 0)
+  {
+    return((uint32_t)(((NVIC->ISER[(((uint32_t)(int32_t)IRQn) >> 5UL)] & (1UL << (((uint32_t)(int32_t)IRQn) & 0x1FUL))) != 0UL) ? 1UL : 0UL));
+  }
+  else
+  {
+    return(0U);
+  }
+}
+#endif
+
 // helper to start DMA
 static void edpt_dma_start(volatile uint32_t* reg_startep)
 {
   // Only one dma can be active
-  if ( _dcd.dma_running )
+  if ( _dcd.dma_pending )
   {
     if (SCB->ICSR & SCB_ICSR_VECTACTIVE_Msk)
     {
-      // If called within ISR, use usbd task to defer later
+      // Called within ISR, use usbd task to defer later
       usbd_defer_func( (osal_task_func_t) edpt_dma_start, (void*) reg_startep, true );
       return;
     }
     else
     {
-      // Otherwise simply block wait
-      while ( _dcd.dma_running )  { }
+      if ( __get_PRIMASK() || !NVIC_GetEnableIRQ(USBD_IRQn) )
+      {
+        // Called in critical section with interrupt disabled. We have to manually check
+        // for the DMA complete by comparing current pending DMA with number of ENDED Events
+        uint32_t ended = 0;
+
+        while ( _dcd.dma_pending < ((uint8_t) ended) )
+        {
+          ended = NRF_USBD->EVENTS_ENDISOIN + NRF_USBD->EVENTS_ENDISOOUT;
+
+          for (uint8_t i=0; i<EP_COUNT; i++)
+          {
+            ended += NRF_USBD->EVENTS_ENDEPIN[i] + NRF_USBD->EVENTS_ENDEPOUT[i];
+          }
+        }
+      }else
+      {
+        // Called in non-critical thread-mode, should be 99% of the time.
+        // Should be safe to blocking wait until previous DMA transfer complete
+        while ( _dcd.dma_pending ) { }
+      }
     }
   }
 
-  _dcd.dma_running = true;
+  _dcd.dma_pending++;
 
   (*reg_startep) = 1;
   __ISB(); __DSB();
@@ -107,8 +149,8 @@ static void edpt_dma_start(volatile uint32_t* reg_startep)
 // DMA is complete
 static void edpt_dma_end(void)
 {
-  TU_ASSERT(_dcd.dma_running, );
-  _dcd.dma_running = false;
+  TU_ASSERT(_dcd.dma_pending, );
+  _dcd.dma_pending = 0;
 }
 
 // helper getting td
@@ -282,9 +324,11 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t
 
   if ( control_status )
   {
-    // Status Phase also require Easy DMA has to be free as well !!!!
+    // Status Phase also requires Easy DMA has to be available as well !!!!
+    // However TASKS_EP0STATUS doesn't trigger any DMA transfer and got ENDED event subsequently
+    // Therefore dma_running state will be corrected right away
     edpt_dma_start(&NRF_USBD->TASKS_EP0STATUS);
-    edpt_dma_end();
+    if (_dcd.dma_pending) _dcd.dma_pending--; // correct the dma_running++ in dma start
 
     // The nRF doesn't interrupt on status transmit so we queue up a success response.
     dcd_event_xfer_complete(0, ep_addr, 0, XFER_RESULT_SUCCESS, false);