Merge pull request #1086 from kkitayam/impl_close_all_for_khci

Implement dcd_edpt_close_all() and fix dcd_edpt_clear_stall() for frdm_kl25z

hw/bsp/frdm_kl25z/board.mk

@@ -8,8 +8,12 @@ CFLAGS += \
   -DCPU_MKL25Z128VLK4 \
   -DCFG_TUSB_MCU=OPT_MCU_MKL25ZXX
 
+LDFLAGS += \
+  -Wl,--defsym,__stack_size__=0x400 \
+  -Wl,--defsym,__heap_size__=0
+
 # mcu driver cause following warnings
-CFLAGS += -Wno-error=unused-parameter
+CFLAGS += -Wno-error=unused-parameter -Wno-error=format
 
 MCU_DIR = $(SDK_DIR)/devices/MKL25Z4
 

hw/bsp/frdm_kl25z/frdm_kl25z.c

@@ -54,6 +54,14 @@ void USB0_IRQHandler(void)
 #define LED_PIN_FUNCTION      kPORT_MuxAsGpio
 #define LED_STATE_ON          0
 
+// Button
+#define BUTTON_PORT           GPIOC
+#define BUTTON_PIN_CLOCK      kCLOCK_PortC
+#define BUTTON_PIN_PORT       PORTC
+#define BUTTON_PIN            9U
+#define BUTTON_PIN_FUNCTION   kPORT_MuxAsGpio
+#define BUTTON_STATE_ACTIVE   0
+
 // UART
 #define UART_PORT             UART0
 #define UART_PIN_CLOCK        kCLOCK_PortA
@@ -84,7 +92,19 @@ void board_init(void)
   PORT_SetPinMux(LED_PIN_PORT, LED_PIN, LED_PIN_FUNCTION);
   gpio_pin_config_t led_config = { kGPIO_DigitalOutput, 0 };
   GPIO_PinInit(LED_PORT, LED_PIN, &led_config);
-  board_led_write(true);
+  board_led_write(false);
+
+#if defined(BUTTON_PORT) && defined(BUTTON_PIN)
+  // Button
+  CLOCK_EnableClock(BUTTON_PIN_CLOCK);
+  port_pin_config_t button_port = {
+    .pullSelect = kPORT_PullUp, 
+    .mux = BUTTON_PIN_FUNCTION,
+  };
+  PORT_SetPinConfig(BUTTON_PIN_PORT, BUTTON_PIN, &button_port);
+  gpio_pin_config_t button_config = { kGPIO_DigitalInput, 0 };
+  GPIO_PinInit(BUTTON_PORT, BUTTON_PIN, &button_config);
+#endif
 
   // UART
   CLOCK_EnableClock(UART_PIN_CLOCK);
@@ -119,6 +139,9 @@ void board_led_write(bool state)
 
 uint32_t board_button_read(void)
 {
+#if defined(BUTTON_PORT) && defined(BUTTON_PIN)
+  return BUTTON_STATE_ACTIVE == GPIO_ReadPinInput(BUTTON_PORT, BUTTON_PIN);
+#endif
   return 0;
 }
 

src/portable/nxp/khci/dcd_khci.c

@@ -52,23 +52,23 @@ typedef struct TU_ATTR_PACKED
     struct {
       union {
         struct {
-          uint16_t          :  2;
-          uint16_t tok_pid  :  4;
-          uint16_t data     :  1;
-          uint16_t own      :  1;
-          uint16_t          :  8;
+               uint16_t           :  2;
+          __IO uint16_t tok_pid   :  4;
+               uint16_t data      :  1;
+          __IO uint16_t own       :  1;
+               uint16_t           :  8;
         };
         struct {
-          uint16_t          :  2;
-          uint16_t bdt_stall:  1;
-          uint16_t dts      :  1;
-          uint16_t ninc     :  1;
-          uint16_t keep     :  1;
-          uint16_t          : 10;
+               uint16_t           :  2;
+               uint16_t bdt_stall :  1;
+               uint16_t dts       :  1;
+               uint16_t ninc      :  1;
+               uint16_t keep      :  1;
+               uint16_t           : 10;
         };
       };
-      uint16_t bc          : 10;
-      uint16_t             :  6;
+      __IO uint16_t bc : 10;
+           uint16_t    :  6;
     };
   };
   uint8_t *addr;
@@ -120,10 +120,8 @@ static void prepare_next_setup_packet(uint8_t rhport)
 {
   const unsigned out_odd = _dcd.endpoint[0][0].odd;
   const unsigned in_odd  = _dcd.endpoint[0][1].odd;
-  if (_dcd.bdt[0][0][out_odd].own) {
-    TU_LOG1("DCD fail to prepare the next SETUP %d %d\r\n", out_odd, in_odd);
-    return;
-  }
+  TU_ASSERT(0 == _dcd.bdt[0][0][out_odd].own, );
+
   _dcd.bdt[0][0][out_odd].data     = 0;
   _dcd.bdt[0][0][out_odd ^ 1].data = 1;
   _dcd.bdt[0][1][in_odd].data      = 1;
@@ -134,10 +132,16 @@ static void prepare_next_setup_packet(uint8_t rhport)
 
 static void process_stall(uint8_t rhport)
 {
-  if (KHCI->ENDPOINT[0].ENDPT & USB_ENDPT_EPSTALL_MASK) {
-    /* clear stall condition of the control pipe */
-    prepare_next_setup_packet(rhport);
-    KHCI->ENDPOINT[0].ENDPT &= ~USB_ENDPT_EPSTALL_MASK;
+  for (int i = 0; i < 16; ++i) {
+    unsigned const endpt = KHCI->ENDPOINT[i].ENDPT;
+
+    if (endpt & USB_ENDPT_EPSTALL_MASK) {
+      // prepare next setup if endpoint0
+      if ( i == 0 ) prepare_next_setup_packet(rhport);
+
+      // clear stall bit
+      KHCI->ENDPOINT[i].ENDPT = endpt & ~USB_ENDPT_EPSTALL_MASK;
+    }
   }
 }
 
@@ -145,12 +149,17 @@ static void process_tokdne(uint8_t rhport)
 {
   const unsigned s = KHCI->STAT;
   KHCI->ISTAT = USB_ISTAT_TOKDNE_MASK; /* fetch the next token if received */
+
+  uint8_t const epnum = (s >> USB_STAT_ENDP_SHIFT);
+  uint8_t const dir   = (s & USB_STAT_TX_MASK) >> USB_STAT_TX_SHIFT;
+  unsigned const odd  = (s & USB_STAT_ODD_MASK) ? 1 : 0;
+
   buffer_descriptor_t *bd = (buffer_descriptor_t *)&_dcd.bda[s];
   endpoint_state_t    *ep = &_dcd.endpoint_unified[s >> 3];
-  unsigned odd = (s & USB_STAT_ODD_MASK) ? 1 : 0;
 
   /* fetch pid before discarded by the next steps */
   const unsigned pid = bd->tok_pid;
+
   /* reset values for a next transfer */
   bd->bdt_stall = 0;
   bd->dts       = 1;
@@ -163,9 +172,6 @@ static void process_tokdne(uint8_t rhport)
     KHCI->CTL &= ~USB_CTL_TXSUSPENDTOKENBUSY_MASK;
     return;
   }
-  if (s >> 4) {
-    TU_LOG1("TKDNE %x\r\n", s);
-  }
 
   const unsigned bc = bd->bc;
   const unsigned remaining = ep->remaining - bc;
@@ -184,9 +190,9 @@ static void process_tokdne(uint8_t rhport)
   }
   const unsigned length = ep->length;
   dcd_event_xfer_complete(rhport,
-                          ((s & USB_STAT_TX_MASK) << 4) | (s >> USB_STAT_ENDP_SHIFT),
+                          tu_edpt_addr(epnum, dir),
                           length - remaining, XFER_RESULT_SUCCESS, true);
-  if (0 == (s & USB_STAT_ENDP_MASK) && 0 == length) {
+  if (0 == epnum && 0 == length) {
     /* After completion a ZLP of control transfer,
      * it prepares for the next steup transfer. */
     if (_dcd.addr) {
@@ -204,7 +210,8 @@ static void process_bus_reset(uint8_t rhport)
   KHCI->USBCTRL &= ~USB_USBCTRL_SUSP_MASK;
   KHCI->CTL     |= USB_CTL_ODDRST_MASK;
   KHCI->ADDR     = 0;
-  KHCI->INTEN    = (KHCI->INTEN & ~USB_INTEN_RESUMEEN_MASK) | USB_INTEN_SLEEPEN_MASK;
+  KHCI->INTEN    = USB_INTEN_USBRSTEN_MASK | USB_INTEN_TOKDNEEN_MASK | USB_INTEN_SLEEPEN_MASK |
+                   USB_INTEN_ERROREN_MASK  | USB_INTEN_STALLEN_MASK;
 
   KHCI->ENDPOINT[0].ENDPT = USB_ENDPT_EPHSHK_MASK | USB_ENDPT_EPRXEN_MASK | USB_ENDPT_EPTXEN_MASK;
   for (unsigned i = 1; i < 16; ++i) {
@@ -229,21 +236,27 @@ static void process_bus_reset(uint8_t rhport)
   dcd_event_bus_reset(rhport, TUSB_SPEED_FULL, true);
 }
 
-static void process_bus_inactive(uint8_t rhport)
+static void process_bus_sleep(uint8_t rhport)
 {
-  (void) rhport;
+  // Enable resume & disable suspend interrupt
   const unsigned inten = KHCI->INTEN;
+
   KHCI->INTEN    = (inten & ~USB_INTEN_SLEEPEN_MASK) | USB_INTEN_RESUMEEN_MASK;
+  KHCI->USBTRC0 |= USB_USBTRC0_USBRESMEN_MASK;
   KHCI->USBCTRL |= USB_USBCTRL_SUSP_MASK;
+
   dcd_event_bus_signal(rhport, DCD_EVENT_SUSPEND, true);
 }
 
-static void process_bus_active(uint8_t rhport)
+static void process_bus_resume(uint8_t rhport)
 {
-  (void) rhport;
-  KHCI->USBCTRL &= ~USB_USBCTRL_SUSP_MASK;
+  // Enable suspend & disable resume interrupt
   const unsigned inten = KHCI->INTEN;
+
+  KHCI->USBCTRL &= ~USB_USBCTRL_SUSP_MASK; // will also clear USB_USBTRC0_USB_RESUME_INT_MASK
+  KHCI->USBTRC0 &= ~USB_USBTRC0_USBRESMEN_MASK;
   KHCI->INTEN    = (inten & ~USB_INTEN_RESUMEEN_MASK) | USB_INTEN_SLEEPEN_MASK;
+
   dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true);
 }
 
@@ -256,12 +269,15 @@ void dcd_init(uint8_t rhport)
 
   KHCI->USBTRC0 |= USB_USBTRC0_USBRESET_MASK;
   while (KHCI->USBTRC0 & USB_USBTRC0_USBRESET_MASK);
+
   tu_memclr(&_dcd, sizeof(_dcd));
   KHCI->USBTRC0 |= TU_BIT(6); /* software must set this bit to 1 */
   KHCI->BDTPAGE1 = (uint8_t)((uintptr_t)_dcd.bdt >>  8);
   KHCI->BDTPAGE2 = (uint8_t)((uintptr_t)_dcd.bdt >> 16);
   KHCI->BDTPAGE3 = (uint8_t)((uintptr_t)_dcd.bdt >> 24);
 
+  KHCI->INTEN = USB_INTEN_USBRSTEN_MASK;
+
   dcd_connect(rhport);
   NVIC_ClearPendingIRQ(USB0_IRQn);
 }
@@ -269,8 +285,6 @@ void dcd_init(uint8_t rhport)
 void dcd_int_enable(uint8_t rhport)
 {
   (void) rhport;
-  KHCI->INTEN = USB_INTEN_USBRSTEN_MASK | USB_INTEN_TOKDNEEN_MASK |
-    USB_INTEN_SLEEPEN_MASK | USB_INTEN_ERROREN_MASK | USB_INTEN_STALLEN_MASK;
   NVIC_EnableIRQ(USB0_IRQn);
 }
 
@@ -278,13 +292,11 @@ void dcd_int_disable(uint8_t rhport)
 {
   (void) rhport;
   NVIC_DisableIRQ(USB0_IRQn);
-  KHCI->INTEN = 0;
 }
 
 void dcd_set_address(uint8_t rhport, uint8_t dev_addr)
 {
-  (void) rhport;
-  _dcd.addr = dev_addr & 0x7F;
+  _dcd.addr = dev_addr & 0x7F; 
   /* Response with status first before changing device address */
   dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0);
 }
@@ -292,9 +304,12 @@ void dcd_set_address(uint8_t rhport, uint8_t dev_addr)
 void dcd_remote_wakeup(uint8_t rhport)
 {
   (void) rhport;
-  unsigned cnt = SystemCoreClock / 100;
+
   KHCI->CTL |= USB_CTL_RESUME_MASK;
+
+  unsigned cnt = SystemCoreClock / 1000;
   while (cnt--) __NOP();
+
   KHCI->CTL &= ~USB_CTL_RESUME_MASK;
 }
 
@@ -321,12 +336,12 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc)
   (void) rhport;
 
   const unsigned ep_addr  = ep_desc->bEndpointAddress;
-  const unsigned epn      = ep_addr & 0xFu;
-  const unsigned dir      = (ep_addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
+  const unsigned epn      = tu_edpt_number(ep_addr);
+  const unsigned dir      = tu_edpt_dir(ep_addr);
   const unsigned xfer     = ep_desc->bmAttributes.xfer;
   endpoint_state_t *ep    = &_dcd.endpoint[epn][dir];
   const unsigned odd      = ep->odd;
-  buffer_descriptor_t *bd = &_dcd.bdt[epn][dir][0];
+  buffer_descriptor_t *bd = _dcd.bdt[epn][dir];
 
   /* No support for control transfer */
   TU_ASSERT(epn && (xfer != TUSB_XFER_CONTROL));
@@ -347,41 +362,60 @@ bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc)
   return true;
 }
 
-void dcd_edpt_close_all (uint8_t rhport)
+void dcd_edpt_close_all(uint8_t rhport)
 {
   (void) rhport;
-  // TODO implement dcd_edpt_close_all()
+  const unsigned ie = NVIC_GetEnableIRQ(USB0_IRQn);
+  NVIC_DisableIRQ(USB0_IRQn);
+  for (unsigned i = 1; i < 16; ++i) {
+    KHCI->ENDPOINT[i].ENDPT = 0;
+  }
+  if (ie) NVIC_EnableIRQ(USB0_IRQn);
+  buffer_descriptor_t *bd = _dcd.bdt[1][0];
+  for (unsigned i = 2; i < sizeof(_dcd.bdt)/sizeof(*bd); ++i, ++bd) {
+    bd->head = 0;
+  }
+  endpoint_state_t *ep = &_dcd.endpoint[1][0];
+  for (unsigned i = 2; i < sizeof(_dcd.endpoint)/sizeof(*ep); ++i, ++ep) {
+    /* Clear except the odd */
+    ep->max_packet_size = 0;
+    ep->length          = 0;
+    ep->remaining       = 0;
+  }
 }
 
 void dcd_edpt_close(uint8_t rhport, uint8_t ep_addr)
 {
   (void) rhport;
 
-  const unsigned epn      = ep_addr & 0xFu;
-  const unsigned dir      = (ep_addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
+  const unsigned epn      = tu_edpt_number(ep_addr);
+  const unsigned dir      = tu_edpt_dir(ep_addr);
   endpoint_state_t *ep    = &_dcd.endpoint[epn][dir];
-  buffer_descriptor_t *bd = &_dcd.bdt[epn][dir][0];
+  buffer_descriptor_t *bd = _dcd.bdt[epn][dir];
   const unsigned msk      = dir ? USB_ENDPT_EPTXEN_MASK : USB_ENDPT_EPRXEN_MASK;
+  const unsigned ie       = NVIC_GetEnableIRQ(USB0_IRQn);
+  NVIC_DisableIRQ(USB0_IRQn);
   KHCI->ENDPOINT[epn].ENDPT &= ~msk;
   ep->max_packet_size = 0;
   ep->length          = 0;
   ep->remaining       = 0;
-  bd->head            = 0;
+  bd[0].head          = 0;
+  bd[1].head          = 0;
+  if (ie) NVIC_EnableIRQ(USB0_IRQn);
 }
 
 bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t total_bytes)
 {
   (void) rhport;
-  NVIC_DisableIRQ(USB0_IRQn);
-  const unsigned epn = ep_addr & 0xFu;
-  const unsigned dir = (ep_addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
+  const unsigned epn      = tu_edpt_number(ep_addr);
+  const unsigned dir      = tu_edpt_dir(ep_addr);
   endpoint_state_t    *ep = &_dcd.endpoint[epn][dir];
   buffer_descriptor_t *bd = &_dcd.bdt[epn][dir][ep->odd];
+  TU_ASSERT(0 == bd->own);
+
+  const unsigned ie = NVIC_GetEnableIRQ(USB0_IRQn);
+  NVIC_DisableIRQ(USB0_IRQn);
 
-  if (bd->own) {
-    TU_LOG1("DCD XFER fail %x %d %lx %lx\r\n", ep_addr, total_bytes, ep->state, bd->head);
-    return false; /* The last transfer has not completed */
-  }
   ep->length    = total_bytes;
   ep->remaining = total_bytes;
 
@@ -394,42 +428,69 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t to
     next->addr = buffer + mps;
     next->own  = 1;
   }
-  bd->bc        = total_bytes >= mps ? mps: total_bytes;
-  bd->addr      = buffer;
+  bd->bc   = total_bytes >= mps ? mps: total_bytes;
+  bd->addr = buffer;
   __DSB();
-  bd->own  = 1; /* the own bit must set after addr */
-  NVIC_EnableIRQ(USB0_IRQn);
+  bd->own  = 1; /* This bit must be set last */
+
+  if (ie) NVIC_EnableIRQ(USB0_IRQn);
   return true;
 }
 
 void dcd_edpt_stall(uint8_t rhport, uint8_t ep_addr)
 {
   (void) rhport;
-  const unsigned epn = ep_addr & 0xFu;
+  const unsigned epn = tu_edpt_number(ep_addr);
+
   if (0 == epn) {
     KHCI->ENDPOINT[epn].ENDPT |=  USB_ENDPT_EPSTALL_MASK;
   } else {
-    const unsigned dir      = (ep_addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
-    buffer_descriptor_t *bd = _dcd.bdt[epn][dir];
-    bd[0].bdt_stall = 1;
-    bd[1].bdt_stall = 1;
+    const unsigned dir      = tu_edpt_dir(ep_addr);
+    const unsigned odd      = _dcd.endpoint[epn][dir].odd;
+    buffer_descriptor_t *bd = &_dcd.bdt[epn][dir][odd];
+    TU_ASSERT(0 == bd->own,);
+
+    const unsigned ie       = NVIC_GetEnableIRQ(USB0_IRQn);
+    NVIC_DisableIRQ(USB0_IRQn);
+
+    bd->bdt_stall = 1;
+    __DSB();
+    bd->own       = 1; /* This bit must be set last */
+
+    if (ie) NVIC_EnableIRQ(USB0_IRQn);
   }
 }
 
 void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr)
 {
   (void) rhport;
-  const unsigned epn      = ep_addr & 0xFu;
-  const unsigned dir      = (ep_addr & TUSB_DIR_IN_MASK) ? TUSB_DIR_IN : TUSB_DIR_OUT;
+  const unsigned epn      = tu_edpt_number(ep_addr);
+  TU_VERIFY(epn,);
+  const unsigned dir      = tu_edpt_dir(ep_addr);
   const unsigned odd      = _dcd.endpoint[epn][dir].odd;
   buffer_descriptor_t *bd = _dcd.bdt[epn][dir];
+  TU_VERIFY(bd[odd].own,);
+
+  const unsigned ie       = NVIC_GetEnableIRQ(USB0_IRQn);
+  NVIC_DisableIRQ(USB0_IRQn);
+
+  bd[odd].own = 0;
+  __DSB();
+
+  // clear stall
+  bd[odd].bdt_stall  = 0;
+
+  // Reset data toggle
+  bd[odd    ].data = 0;
+  bd[odd ^ 1].data = 1;
 
-  bd[odd ^ 1].own       = 0;
-  bd[odd ^ 1].data      = 1;
-  bd[odd ^ 1].bdt_stall = 0;
-  bd[odd].own           = 0;
-  bd[odd].data          = 0;
-  bd[odd].bdt_stall     = 0;
+  // We already cleared this in ISR, but just clear it here to be safe
+  const unsigned endpt = KHCI->ENDPOINT[epn].ENDPT;
+  if (endpt & USB_ENDPT_EPSTALL_MASK) {
+    KHCI->ENDPOINT[epn].ENDPT = endpt & ~USB_ENDPT_EPSTALL_MASK;
+  }
+
+  if (ie) NVIC_EnableIRQ(USB0_IRQn);
 }
 
 //--------------------------------------------------------------------+
@@ -437,48 +498,59 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr)
 //--------------------------------------------------------------------+
 void dcd_int_handler(uint8_t rhport)
 {
-  (void) rhport;
-
   uint32_t is  = KHCI->ISTAT;
   uint32_t msk = KHCI->INTEN;
+
+  // clear non-enabled interrupts
   KHCI->ISTAT = is & ~msk;
   is &= msk;
+
   if (is & USB_ISTAT_ERROR_MASK) {
     /* TODO: */
     uint32_t es = KHCI->ERRSTAT;
     KHCI->ERRSTAT = es;
     KHCI->ISTAT   = is; /* discard any pending events */
-    return;
   }
 
   if (is & USB_ISTAT_USBRST_MASK) {
     KHCI->ISTAT = is; /* discard any pending events */
     process_bus_reset(rhport);
-    return;
   }
+
   if (is & USB_ISTAT_SLEEP_MASK) {
+    // TU_LOG2("Suspend: "); TU_LOG2_HEX(is);
+
+    // Note Host usually has extra delay after bus reset (without SOF), which could falsely 
+    // detected as Sleep event. Though usbd has debouncing logic so we are good
     KHCI->ISTAT = USB_ISTAT_SLEEP_MASK;
-    process_bus_inactive(rhport);
-    return;
+    process_bus_sleep(rhport);
   }
+
+#if 0 // ISTAT_RESUME never trigger, probably for host mode ?
   if (is & USB_ISTAT_RESUME_MASK) {
+    // TU_LOG2("ISTAT Resume: "); TU_LOG2_HEX(is);
     KHCI->ISTAT = USB_ISTAT_RESUME_MASK;
-    process_bus_active(rhport);
-    return;
+    process_bus_resume(rhport);
   }
+#endif
+
+  if (KHCI->USBTRC0 & USB_USBTRC0_USB_RESUME_INT_MASK) {
+     // TU_LOG2("USBTRC0 Resume: "); TU_LOG2_HEX(is); TU_LOG2_HEX(KHCI->USBTRC0);
+    process_bus_resume(rhport);
+  }
+
   if (is & USB_ISTAT_SOFTOK_MASK) {
     KHCI->ISTAT = USB_ISTAT_SOFTOK_MASK;
     dcd_event_bus_signal(rhport, DCD_EVENT_SOF, true);
-    return;
   }
+
   if (is & USB_ISTAT_STALL_MASK) {
     KHCI->ISTAT = USB_ISTAT_STALL_MASK;
     process_stall(rhport);
-    return;
   }
+
   if (is & USB_ISTAT_TOKDNE_MASK) {
     process_tokdne(rhport);
-    return;
   }
 }