Merge pull request #389 from hathach/fix-samg

Fix SAMG

examples/device/net_lwip_webserver/src/usb_descriptors.c

@@ -103,9 +103,21 @@ uint8_t const * tud_descriptor_device_cb(void)
 #if CFG_TUSB_MCU == OPT_MCU_LPC175X_6X || CFG_TUSB_MCU == OPT_MCU_LPC177X_8X || CFG_TUSB_MCU == OPT_MCU_LPC40XX
   // LPC 17xx and 40xx endpoint type (bulk/interrupt/iso) are fixed by its number
   // 0 control, 1 In, 2 Bulk, 3 Iso, 4 In etc ...
-  #define EPNUM_CDC     2
+  #define EPNUM_NET_NOTIF   0x81
+  #define EPNUM_NET_OUT     0x02
+  #define EPNUM_NET_IN      0x82
+
+#elif CFG_TUSB_MCU == OPT_MCU_SAMG
+  // SAMG doesn't support a same endpoint number with different direction IN and OUT
+  //    e.g EP1 OUT & EP1 IN cannot exist together
+  #define EPNUM_NET_NOTIF   0x81
+  #define EPNUM_NET_OUT     0x02
+  #define EPNUM_NET_IN      0x83
+
 #else
-  #define EPNUM_CDC     2
+  #define EPNUM_NET_NOTIF   0x81
+  #define EPNUM_NET_OUT     0x02
+  #define EPNUM_NET_IN      0x82
 #endif
 
 static uint8_t const rndis_configuration[] =
@@ -114,7 +126,7 @@ static uint8_t const rndis_configuration[] =
   TUD_CONFIG_DESCRIPTOR(CONFIG_ID_RNDIS+1, ITF_NUM_TOTAL, 0, MAIN_CONFIG_TOTAL_LEN, 0, 100),
 
   // Interface number, string index, EP notification address and size, EP data address (out, in) and size.
-  TUD_RNDIS_DESCRIPTOR(ITF_NUM_CDC, STRID_INTERFACE, 0x81, 8, EPNUM_CDC, 0x80 | EPNUM_CDC, CFG_TUD_NET_ENDPOINT_SIZE),
+  TUD_RNDIS_DESCRIPTOR(ITF_NUM_CDC, STRID_INTERFACE, EPNUM_NET_NOTIF, 8, EPNUM_NET_OUT, EPNUM_NET_IN, CFG_TUD_NET_ENDPOINT_SIZE),
 };
 
 static uint8_t const ecm_configuration[] =
@@ -123,7 +135,7 @@ static uint8_t const ecm_configuration[] =
   TUD_CONFIG_DESCRIPTOR(CONFIG_ID_ECM+1, ITF_NUM_TOTAL, 0, ALT_CONFIG_TOTAL_LEN, 0, 100),
 
   // Interface number, description string index, MAC address string index, EP notification address and size, EP data address (out, in), and size, max segment size.
-  TUD_CDC_ECM_DESCRIPTOR(ITF_NUM_CDC, STRID_INTERFACE, STRID_MAC, 0x81, 64, EPNUM_CDC, 0x80 | EPNUM_CDC, CFG_TUD_NET_ENDPOINT_SIZE, CFG_TUD_NET_MTU),
+  TUD_CDC_ECM_DESCRIPTOR(ITF_NUM_CDC, STRID_INTERFACE, STRID_MAC, EPNUM_NET_NOTIF, 64, EPNUM_NET_OUT, EPNUM_NET_IN, CFG_TUD_NET_ENDPOINT_SIZE, CFG_TUD_NET_MTU),
 };
 
 // Configuration array: RNDIS and CDC-ECM

src/portable/microchip/samg/dcd_samg.c

@@ -51,12 +51,6 @@ typedef struct
 // Endpoint 0-5, each can only be either OUT or In
 xfer_desc_t _dcd_xfer[EP_COUNT];
 
-// Indicate that DATA Toggle for Control Status is incorrect, which must always be DATA1 by USB Specs.
-// However SAMG DToggle is read-only, therefore we must duplicate the status phase ( D0 then D1 )
-// as walk-around to resolve this. The D0 status packet is likely to be discarded by USB Host safely.
-// Note: Only needed for IN Status e.g CDC_SET_LINE_CODING, since out data is sent by host
-volatile bool _walkaround_incorrect_dtoggle_control_status;
-
 void xfer_epsize_set(xfer_desc_t* xfer, uint16_t epsize)
 {
   xfer->epsize = epsize;
@@ -110,6 +104,32 @@ static void xact_ep_read(uint8_t epnum, uint8_t* buffer, uint16_t xact_len)
   }
 }
 
+
+//! Bitmap for all status bits in CSR that are not affected by a value 1.
+#define CSR_NO_EFFECT_1_ALL (UDP_CSR_RX_DATA_BK0 | UDP_CSR_RX_DATA_BK1 | UDP_CSR_STALLSENT | UDP_CSR_RXSETUP | UDP_CSR_TXCOMP)
+
+// Per Specs: CSR need synchronization each write
+static inline void csr_write(uint8_t epnum, uint32_t value)
+{
+  uint32_t const csr = value;
+  UDP->UDP_CSR[epnum] = csr;
+
+  volatile uint32_t nop_count;
+  for (nop_count = 0; nop_count < 20; nop_count ++) __NOP();
+}
+
+// Per Specs: CSR need synchronization each write
+static inline void csr_set(uint8_t epnum, uint32_t mask)
+{
+  csr_write(epnum, UDP->UDP_CSR[epnum] | CSR_NO_EFFECT_1_ALL | mask);
+}
+
+// Per Specs: CSR need synchronization each write
+static inline void csr_clear(uint8_t epnum, uint32_t mask)
+{
+  csr_write(epnum, (UDP->UDP_CSR[epnum] | CSR_NO_EFFECT_1_ALL) & ~mask);
+}
+
 /*------------------------------------------------------------------*/
 /* Device API
  *------------------------------------------------------------------*/
@@ -117,13 +137,12 @@ static void xact_ep_read(uint8_t epnum, uint8_t* buffer, uint16_t xact_len)
 // Set up endpoint 0, clear all other endpoints
 static void bus_reset(void)
 {
-  _walkaround_incorrect_dtoggle_control_status = false;
   tu_memclr(_dcd_xfer, sizeof(_dcd_xfer));
 
   xfer_epsize_set(&_dcd_xfer[0], CFG_TUD_ENDPOINT0_SIZE);
 
   // Enable EP0 control
-  UDP->UDP_CSR[0] = UDP_CSR_EPEDS_Msk;
+  csr_write(0, UDP_CSR_EPEDS_Msk);
 
   // Enable interrupt : EP0, Suspend, Resume, Wakeup
   UDP->UDP_IER = UDP_IER_EP0INT_Msk | UDP_IER_RXSUSP_Msk | UDP_IER_RXRSM_Msk | UDP_IER_WAKEUP_Msk;
@@ -189,7 +208,6 @@ void dcd_disconnect(uint8_t rhport)
   UDP->UDP_TXVC = UDP_TXVC_TXVDIS_Msk;
 }
 
-
 //--------------------------------------------------------------------+
 // Endpoint API
 //--------------------------------------------------------------------+
@@ -240,8 +258,8 @@ bool dcd_edpt_open (uint8_t rhport, tusb_desc_endpoint_t const * ep_desc)
 
   xfer_epsize_set(&_dcd_xfer[epnum], ep_desc->wMaxPacketSize.size);
 
-  // Configure type and eanble EP
+  // Configure type and enable EP
-  UDP->UDP_CSR[epnum] = UDP_CSR_EPEDS_Msk | UDP_CSR_EPTYPE(ep_desc->bmAttributes.xfer + 4*dir);
+  csr_write(epnum, UDP_CSR_EPEDS_Msk | UDP_CSR_EPTYPE(ep_desc->bmAttributes.xfer + 4*dir));
 
   // Enable EP Interrupt for IN
   if (dir == TUSB_DIR_IN) UDP->UDP_IER |= (1 << epnum);
@@ -262,47 +280,15 @@ bool dcd_edpt_xfer (uint8_t rhport, uint8_t ep_addr, uint8_t * buffer, uint16_t
 
   if (dir == TUSB_DIR_OUT)
   {
-    // Clear EP0 direction bit
-    if (epnum == 0) UDP->UDP_CSR[epnum] &= ~UDP_CSR_DIR_Msk;
-
     // Enable interrupt when starting OUT transfer
     if (epnum != 0) UDP->UDP_IER |= (1 << epnum);
   }
   else
   {
-    if (epnum == 0)
-    {
-      // Previous EP0 direction is OUT --> This transfer is ZLP control status.
-      if ( !(UDP->UDP_CSR[epnum] & UDP_CSR_DIR_Msk) )
-      {
-        // Set EP0 dir bit
-        UDP->UDP_CSR[epnum] |= UDP_CSR_DIR_Msk;
-
-        // DATA Toggle is 0, USB Specs requires Status Stage must be DATA1
-        // Since SAMG DToggle is read-only, we mark this and implement the walk-around
-        if ( !(UDP->UDP_CSR[epnum] & UDP_CSR_DTGLE_Msk) )
-        {
-          TU_LOG2("Incorrect DATA TOGGLE, Control Status must be DATA1\n");
-
-          // DTGLE is read-only on SAMG, this statement has no effect
-          UDP->UDP_CSR[epnum] |= UDP_CSR_DTGLE_Msk;
-
-          // WALKROUND: duplicate IN transfer to send DATA1 status packet
-          // set flag for irq to skip reporting first incorrect packet
-          _walkaround_incorrect_dtoggle_control_status = true;
-
-          UDP->UDP_CSR[epnum] |= UDP_CSR_TXPKTRDY_Msk;
-          while ( UDP->UDP_CSR[epnum] & UDP_CSR_TXPKTRDY_Msk ) {}
-
-          _walkaround_incorrect_dtoggle_control_status = false;
-        }
-      }
-    }
-
     xact_ep_write(epnum, xfer->buffer, xfer_packet_len(xfer));
 
     // TX ready for transfer
-    UDP->UDP_CSR[epnum] |= UDP_CSR_TXPKTRDY_Msk;
+    csr_set(epnum, UDP_CSR_TXPKTRDY_Msk);
   }
 
   return true;
@@ -313,10 +299,14 @@ void dcd_edpt_stall (uint8_t rhport, uint8_t ep_addr)
 {
   (void) rhport;
 
+  // For EP0 USBD will stall both EP0 Out and In with 0x00 and 0x80
+  // only handle one by skipping 0x80
+  if ( ep_addr == tu_edpt_addr(0, TUSB_DIR_IN_MASK) ) return;
+
   uint8_t const epnum = tu_edpt_number(ep_addr);
 
   // Set force stall bit
-  UDP->UDP_CSR[epnum] |= UDP_CSR_FORCESTALL_Msk;
+  csr_set(epnum, UDP_CSR_FORCESTALL_Msk);
 }
 
 // clear stall, data toggle is also reset to DATA0
@@ -327,11 +317,11 @@ void dcd_edpt_clear_stall (uint8_t rhport, uint8_t ep_addr)
   uint8_t const epnum = tu_edpt_number(ep_addr);
 
   // clear stall
-  UDP->UDP_CSR[epnum] &= ~UDP_CSR_FORCESTALL_Msk;
+  csr_clear(epnum, UDP_CSR_FORCESTALL_Msk);
 
   // must also reset EP to clear data toggle
-  UDP->UDP_RST_EP = tu_bit_set(UDP->UDP_RST_EP, epnum);
+  UDP->UDP_RST_EP |= (1 << epnum);
-  UDP->UDP_RST_EP = tu_bit_clear(UDP->UDP_RST_EP, epnum);
+  UDP->UDP_RST_EP &= ~(1 << epnum);
 }
 
 //--------------------------------------------------------------------+
@@ -369,7 +359,7 @@ void dcd_int_handler(uint8_t rhport)
   if ( intr_status & TU_BIT(0) )
   {
     // setup packet
-    if (UDP->UDP_CSR[0] & UDP_CSR_RXSETUP)
+    if ( UDP->UDP_CSR[0] & UDP_CSR_RXSETUP )
     {
       // get setup from FIFO
       uint8_t setup[8];
@@ -382,18 +372,18 @@ void dcd_int_handler(uint8_t rhport)
       dcd_event_setup_received(rhport, setup, true);
 
       // Set EP direction bit according to DATA stage
-      if (setup[0] & 0x80)
+      // MUST only be set before RXSETUP is clear per specs
+      if ( tu_edpt_dir(setup[0]) )
       {
-        UDP->UDP_CSR[0] |= UDP_CSR_DIR_Msk;
+        csr_set(0, UDP_CSR_DIR_Msk);
-      }else
+      }
+      else
       {
-        UDP->UDP_CSR[0] &= ~UDP_CSR_DIR_Msk;
+        csr_clear(0, UDP_CSR_DIR_Msk);
       }
 
-      // Clear Setup bit & stall bit if needed
+      // Clear Setup, stall and other on-going transfer bits
-      UDP->UDP_CSR[0] &= ~(UDP_CSR_RXSETUP_Msk | UDP_CSR_FORCESTALL_Msk);
+      csr_clear(0, UDP_CSR_RXSETUP_Msk | UDP_CSR_TXPKTRDY_Msk | UDP_CSR_TXCOMP_Msk | UDP_CSR_RX_DATA_BK0 | UDP_CSR_RX_DATA_BK1 | UDP_CSR_STALLSENT_Msk | UDP_CSR_FORCESTALL_Msk);
-
-      return;
     }
   }
 
@@ -416,22 +406,18 @@ void dcd_int_handler(uint8_t rhport)
           xact_ep_write(epnum, xfer->buffer, xact_len);
 
           // TX ready for transfer
-          UDP->UDP_CSR[epnum] |= UDP_CSR_TXPKTRDY_Msk;
+          csr_set(epnum, UDP_CSR_TXPKTRDY_Msk);
         }else
         {
-          // WALKAROUND: Skip reporting this incorrect DATA Toggle status IN transfer
+          // xfer is complete
-          if ( !(_walkaround_incorrect_dtoggle_control_status && (epnum == 0) && (xfer->actual_len == 0)) )
+          dcd_event_xfer_complete(rhport, epnum | TUSB_DIR_IN_MASK, xfer->actual_len, XFER_RESULT_SUCCESS, true);
-          {
-            // xfer is complete
-            dcd_event_xfer_complete(rhport, epnum | TUSB_DIR_IN_MASK, xfer->actual_len, XFER_RESULT_SUCCESS, true);
 
-            // Required since control OUT can happen right after before stack handle this event
+          // Required since control OUT can happen right after before stack handle this event
-            xfer_end(xfer);
+          xfer_end(xfer);
-          }
         }
 
         // Clear TX Complete bit
-        UDP->UDP_CSR[epnum] &= ~UDP_CSR_TXCOMP_Msk;
+        csr_clear(epnum, UDP_CSR_TXCOMP_Msk);
       }
 
       //------------- Endpoint OUT -------------//
@@ -456,13 +442,13 @@ void dcd_int_handler(uint8_t rhport)
         }
 
         // Clear DATA Bank0/1 bit
-        UDP->UDP_CSR[epnum] &= ~banks_complete;
+        csr_clear(epnum, banks_complete);
       }
 
       // Stall sent to host
       if (UDP->UDP_CSR[epnum] & UDP_CSR_STALLSENT_Msk)
       {
-        UDP->UDP_CSR[epnum] &= ~UDP_CSR_STALLSENT_Msk;
+        csr_clear(epnum, UDP_CSR_STALLSENT_Msk);
       }
     }
   }